From f5d5c69544d2ac94abab4ed22d7f5bc3aa6919cd Mon Sep 17 00:00:00 2001 From: JATMN Date: Sat, 24 Oct 2026 01:31:56 -0700 Subject: [PATCH] test: synchronize retention policy phases (#2380) * test: synchronize retention policy phases * test: verify cleanup policy hook ordering Record the rejecting policy hook and threshold phase in the cleanup fixture, and assert that the hook runs first while the throttle lock is held. Preserve existing policy outcome and snapshot assertions. * test: prove cleanup reruns remove newly completed sessions --- src/utils/cleanup.test.ts | 67 +++++++------ .../cleanupBackgroundSessions.fixture.ts | 99 ++++++++++++++++++- 2 files changed, 129 insertions(+), 37 deletions(-) diff --git a/src/utils/cleanup.test.ts b/src/utils/cleanup.test.ts index 3306f6345..b89b1d066 100644 --- a/src/utils/cleanup.test.ts +++ b/src/utils/cleanup.test.ts @@ -67,6 +67,7 @@ async function runCleanupFixture( mode: | 'once' | 'pass-outcome' + | 'policy-outcome' | 'marker-failure' | 'periodic-policy-recheck' | 'periodic-recovery' @@ -400,6 +401,11 @@ describe('cleanupOldMessageFilesInBackground', () => { expect(await Bun.file(targetPath).exists()).toBe(true) await rm(join(root, '.recovery-pass'), { force: true }) + const freshPath = await createCompletedBackgroundSession( + configDir, + 'bg-progress-fresh', + new Date(Date.now() - 60_000), + ) expect( await runCleanupFixture( configDir, @@ -407,6 +413,7 @@ describe('cleanupOldMessageFilesInBackground', () => { { OPENCLAUDE_CLEANUP_RECOVERY_LIMIT: '2' }, ), ).toEqual({ result: 'ran' }) + expect(await Bun.file(freshPath).exists()).toBe(false) expect(await Bun.file(targetPath).exists()).toBe(false) }, 30_000, @@ -447,41 +454,37 @@ describe('cleanupOldMessageFilesInBackground', () => { ) const journal = join(root, '.recovery-journal') const before = await readFile(journal, 'utf8') - expect( - await runCleanupFixture(configDir, 'pass-outcome', { - OPENCLAUDE_CLEANUP_TRIGGER: trigger, - OPENCLAUDE_CLEANUP_REJECT_AFTER_LOCK: - timing === 'after' ? '1' : '0', - }), - ).toEqual({ result: 'invalid-policy' }) - expect(await Bun.file(metadataPath).exists()).toBe(true) - expect(await Bun.file(marker).exists()).toBe(false) - expect( - await Bun.file(join(root, '.recovery-cursor.json')).exists(), - ).toBe(false) - expect(await readFile(journal, 'utf8')).toBe(before) - - await writeFile(settingsPath, validPolicy) - expect( - await runCleanupFixture(configDir, 'pass-outcome', { - OPENCLAUDE_CLEANUP_TRIGGER: trigger, - }), - ).toEqual({ result: 'ran' }) + const outcomes = await runCleanupFixture(configDir, 'policy-outcome', { + OPENCLAUDE_CLEANUP_TRIGGER: trigger, + OPENCLAUDE_CLEANUP_REJECT_AFTER_LOCK: + timing === 'after' ? '1' : '0', + }) + expect(outcomes).toEqual({ + phases: timing === 'after' ? ['policy-hook', 'threshold'] : ['threshold'], + policyHookLockHeld: timing === 'after' ? true : null, + rejected: { + result: 'invalid-policy', + metadataPresent: true, + markerPresent: false, + cursorPresent: false, + journal: before, + }, + retried: { + result: 'ran', + metadataPresent: false, + journal: '', + markerPresent: true, + }, + throttled: 'skipped', + rerun: { + result: 'ran', + metadataPresent: false, + markerPresent: true, + }, + }) expect(await Bun.file(metadataPath).exists()).toBe(false) expect(await readFile(journal, 'utf8')).toBe('') expect(await Bun.file(marker).exists()).toBe(true) - expect( - await runCleanupFixture(configDir, 'pass-outcome', { - OPENCLAUDE_CLEANUP_TRIGGER: trigger, - }), - ).toEqual({ result: 'skipped' }) - await rm(marker) - expect( - await runCleanupFixture(configDir, 'pass-outcome', { - OPENCLAUDE_CLEANUP_TRIGGER: trigger, - }), - ).toEqual({ result: 'ran' }) - expect(await Bun.file(marker).exists()).toBe(true) }, 30_000) } diff --git a/src/utils/cleanupBackgroundSessions.fixture.ts b/src/utils/cleanupBackgroundSessions.fixture.ts index a9b0a838c..f6b3efe2e 100644 --- a/src/utils/cleanupBackgroundSessions.fixture.ts +++ b/src/utils/cleanupBackgroundSessions.fixture.ts @@ -1,5 +1,5 @@ import { createHash } from 'node:crypto' -import { mkdir, utimes } from 'node:fs/promises' +import { mkdir, readFile, rm, stat, utimes } from 'node:fs/promises' import { dirname, join } from 'node:path' import { createBackgroundSession, @@ -8,6 +8,7 @@ import { import { killHandler } from '../cli/bg.js' import { enableConfigs } from './config.js' import { eagerLoadSettingsFromArgs } from './settings/flagSettings.js' +import { resetSettingsCache } from './settings/settingsCache.js' import { cleanupBackgroundSessionsAfterFinalization, cleanupBackgroundSessionsInBackground, @@ -17,7 +18,7 @@ import { try { const mode = process.env.OPENCLAUDE_CLEANUP_FIXTURE_MODE - if (mode === 'pass-outcome') { + if (mode === 'pass-outcome' || mode === 'policy-outcome') { enableConfigs() const settingsPath = join( process.env.OPENCLAUDE_CONFIG_DIR!, @@ -32,15 +33,27 @@ try { process.env.OPENCLAUDE_CLEANUP_TRIGGER === 'periodic-retention' ? 'periodic-retention' : 'periodic-recovery' + let rejectAfterLock = + process.env.OPENCLAUDE_CLEANUP_REJECT_AFTER_LOCK === '1' + const phases: string[] = [] + let policyHookLockHeld: boolean | null = null const afterLock = async () => { - if (process.env.OPENCLAUDE_CLEANUP_REJECT_AFTER_LOCK === '1') { + if (rejectAfterLock) { + phases.push('policy-hook') + policyHookLockHeld = await stat(join( + process.env.OPENCLAUDE_CONFIG_DIR!, + 'bg-sessions', + trigger === 'periodic-recovery' + ? '.recovery-pass.lock' + : '.retention-pass.lock', + )).then(lock => lock.isDirectory(), () => false) await Bun.write( settingsPath, JSON.stringify({ cleanupPeriodDays: 'invalid' }), ) } } - const result = await runBackgroundSessionRetention( + const runPass = () => runBackgroundSessionRetention( trigger === 'periodic-recovery' ? { trigger, _afterThrottleLockForTesting: afterLock } : { @@ -74,7 +87,83 @@ try { }, }, ) - process.stdout.write(`${JSON.stringify({ result })}\n`) + if (mode === 'policy-outcome') { + const root = join(process.env.OPENCLAUDE_CONFIG_DIR!, 'bg-sessions') + const marker = join( + root, + trigger === 'periodic-recovery' ? '.recovery-pass' : '.retention-pass', + ) + const snapshot = async (result: Awaited>) => ({ + result, + metadataPresent: await Bun.file( + join(root, 'sessions', 'bg-rejected-policy.json'), + ).exists(), + markerPresent: await Bun.file(marker).exists(), + cursorPresent: await Bun.file( + join(root, '.recovery-cursor.json'), + ).exists(), + journal: await readFile(join(root, '.recovery-journal'), 'utf8'), + }) + const freshPass = () => { + // Match a fresh process's settings read at each phase boundary. + resetSettingsCache() + return runPass() + } + const originalDateNow = Date.now + const now = Date.now() + try { + // Scheduling must not age the recovery marker between these phases. + Date.now = () => now + const rejected = await snapshot(await freshPass()) + rejectAfterLock = false + await Bun.write(settingsPath, JSON.stringify({ cleanupPeriodDays: 0 })) + const retriedState = await snapshot(await freshPass()) + const retried = { + result: retriedState.result, + metadataPresent: retriedState.metadataPresent, + journal: retriedState.journal, + markerPresent: retriedState.markerPresent, + } + // Anchor the throttle check to the published marker's time. + const markerNow = Math.max(now, Math.ceil((await stat(marker)).mtimeMs)) + Date.now = () => markerNow + phases.push('threshold') + const throttled = await freshPass() + await rm(marker) + const rerunSession = await createBackgroundSession({ + id: 'bg-policy-rerun', + pid: process.pid, + cwd: process.cwd(), + command: ['openclaude', '--print', 'fixture'], + sessionId: 'bg-policy-rerun-conversation', + }) + recordBackgroundSessionNaturalTerminationSync( + rerunSession.id, + { exitCode: 0 }, + { + ownerPid: process.pid, + expectedSession: rerunSession, + now: new Date(markerNow - 1_000), + }, + ) + const rerunState = await snapshot(await freshPass()) + const rerun = { + result: rerunState.result, + metadataPresent: await Bun.file( + join(root, 'sessions', `${rerunSession.id}.json`), + ).exists(), + markerPresent: rerunState.markerPresent, + } + process.stdout.write( + `${JSON.stringify({ rejected, retried, throttled, rerun, phases, policyHookLockHeld })}\n`, + ) + } finally { + Date.now = originalDateNow + } + } else { + const result = await runPass() + process.stdout.write(`${JSON.stringify({ result })}\n`) + } } else if ( mode === 'periodic-retention' || mode === 'periodic-retention-policy-recheck'