diff --git a/src/main/native-chat/agent-session-journal/journal-legacy-file-lock.test.ts b/src/main/native-chat/agent-session-journal/journal-legacy-file-lock.test.ts index faed62d30d8..38a893b34fa 100644 --- a/src/main/native-chat/agent-session-journal/journal-legacy-file-lock.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-legacy-file-lock.test.ts @@ -6,7 +6,7 @@ import { spawnProcess } from '@orca/process-host' import { afterEach, expect, it } from 'vitest' import { acquireJournalLegacyFileLock, - checkJournalLegacyFileLock, + createJournalLegacyFileEligibility, journalLegacyFileLockPath } from './journal-legacy-file-lock' import { openJournalDatabase } from './database/journal-database' @@ -53,7 +53,7 @@ it.skipIf(!process.env.CI)( reader.once('error', reject) }) const signal = new AbortController() - await checkJournalLegacyFileLock(filename, signal.signal) + await createJournalLegacyFileEligibility().check(filename, signal.signal) await expect(acquireJournalLegacyFileLock(filename, signal.signal)).rejects.toThrow() reader.stdin.end() await exited diff --git a/src/main/native-chat/agent-session-journal/journal-legacy-file-lock.ts b/src/main/native-chat/agent-session-journal/journal-legacy-file-lock.ts index 9bae752d3c1..67d93569cb5 100644 --- a/src/main/native-chat/agent-session-journal/journal-legacy-file-lock.ts +++ b/src/main/native-chat/agent-session-journal/journal-legacy-file-lock.ts @@ -23,7 +23,8 @@ async function startLegacyHelper( filename: string, signal: AbortSignal, check: boolean, - spawn: PipedProcessSpawner = spawnProcess + spawn: PipedProcessSpawner = spawnProcess, + track?: (exited: Promise) => void ) { signal.throwIfAborted() const child = spawn({ @@ -67,6 +68,7 @@ async function startLegacyHelper( }) }) void done.catch(() => {}) + track?.(done) const force = () => { if (!exited) { void signalProcessTree(child, 'SIGKILL') @@ -144,17 +146,26 @@ export function acquireJournalLegacyFileLock( return startLegacyHelper(filename, signal, false, spawn) } -export async function checkJournalLegacyFileLock( - filename: string, - signal: AbortSignal -): Promise { - if (!existsSync(journalLegacyFileLockPath())) { - throw new JournalLegacyRetentionError('legacy journal lock asset is unavailable') +export function createJournalLegacyFileEligibility() { + let pending: Promise | undefined + return { + present: requireJournalLegacyFileLock, + async check(filename: string, signal: AbortSignal): Promise { + if (pending) { + throw new Error('legacy journal eligibility pending') + } + await startLegacyHelper(filename, signal, true, spawnProcess, (exited) => { + pending = exited + const clear = () => { + pending = undefined + } + void exited.then(clear, clear) + }) + } } - await startLegacyHelper(filename, signal, true) } -export function requireJournalLegacyFileLock(): void { +function requireJournalLegacyFileLock(): void { if (!existsSync(journalLegacyFileLockPath())) { throw new JournalLegacyRetentionError('legacy journal lock asset is unavailable') } diff --git a/src/main/native-chat/agent-session-journal/journal-legacy-retirement.test.ts b/src/main/native-chat/agent-session-journal/journal-legacy-retirement.test.ts index dfbb4696cb6..87498a51fd8 100644 --- a/src/main/native-chat/agent-session-journal/journal-legacy-retirement.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-legacy-retirement.test.ts @@ -12,6 +12,9 @@ import { } from 'node:fs' import { join, basename } from 'node:path' import { tmpdir } from 'node:os' +import { ChildProcess } from 'node:child_process' +import { PassThrough } from 'node:stream' +import { spawnProcess } from '@orca/process-host' import { afterEach, expect, it, vi } from 'vitest' import Database from '../../sqlite/sync-database' import { openJournalDatabase } from './database/journal-database' @@ -28,7 +31,10 @@ import { import { message } from './journal-sql-message.test-fixture' import { journalDatabasePath } from './database/journal-host-database' import { JournalSqlBatch } from './journal-sql-batch' -import { acquireJournalLegacyFileLock } from './journal-legacy-file-lock' +import { + acquireJournalLegacyFileLock, + createJournalLegacyFileEligibility +} from './journal-legacy-file-lock' import { JournalLegacyRetirement } from './journal-legacy-retirement' import { advanceLegacyRetirementClock, @@ -43,8 +49,11 @@ import { prepareJournalDomainImport } from './journal-domain-import' import { journalSqlRetryRows } from './journal-sql-retry-rows.test-fixture' import { insertJournalRow, publishJournalSessionEpoch } from './durable-rows/journal-row-table' +vi.mock('@orca/process-host', { spy: true }) + const cleanup: (() => void)[] = [] afterEach(() => { + vi.useRealTimers() vi.restoreAllMocks() for (const close of cleanup.splice(0).toReversed()) { close() @@ -566,3 +575,68 @@ it('retires in the same owner after a missing helper is restored', async () => { expect(test.errors).toHaveLength(1) expect(existsSync(test.filename)).toBe(false) }) + +it('keeps eligibility pending until a termination-resistant check exits, then retires', async () => { + let now = 0 + vi.spyOn(performance, 'now').mockImplementation(() => now) + const test = await profile(undefined, { + ...createJournalLegacyFileEligibility(), + present: () => {} + }) + const proof = vi.spyOn(test.readers, 'legacyProof') + const recycle = vi.spyOn(test.readers, 'recycleSourceReaders') + const child = () => { + const helper = Object.assign(new ChildProcess(), { + stdin: new PassThrough(), + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: vi.fn(() => false) + }) + cleanup.push(() => { + helper.emit('exit', null, 'SIGKILL') + helper.stdin.destroy() + helper.stdout.destroy() + helper.stderr.destroy() + }) + return helper + } + const stuck = child() + const started = Promise.withResolvers() + const spawn = vi.mocked(spawnProcess).mockClear() + spawn.mockImplementationOnce(() => { + started.resolve() + return stuck + }) + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + const attempt = test.retirement.run() + await started.promise + await vi.advanceTimersByTimeAsync(5_250) + await attempt + vi.useRealTimers() + expect(stuck.kill).toHaveBeenCalledWith('SIGKILL') + for (now of [60_001, 180_002, 420_003]) { + await test.retirement.run() + expect(test.errors.at(-1)).toMatchObject({ message: 'legacy journal eligibility pending' }) + } + expect(spawn).toHaveBeenCalledOnce() + expect(proof).not.toHaveBeenCalled() + expect(recycle).not.toHaveBeenCalled() + expect(existsSync(test.filename)).toBe(true) + + stuck.emit('exit', null, 'SIGKILL') + const recovered = child() + spawn.mockImplementationOnce(() => { + queueMicrotask(() => { + recovered.stdout.write('eligible\n') + recovered.emit('exit', 0) + }) + return recovered + }) + now = 900_004 + await test.retirement.run() + expect(test.receiptState().phase).toBe('grace') + test.arm() + await test.retirement.run() + expect(test.errors).toHaveLength(2) + expect(existsSync(test.filename)).toBe(false) +}) diff --git a/src/main/native-chat/agent-session-journal/journal-legacy-retirement.ts b/src/main/native-chat/agent-session-journal/journal-legacy-retirement.ts index fc4a3bf1f1e..775e2537147 100644 --- a/src/main/native-chat/agent-session-journal/journal-legacy-retirement.ts +++ b/src/main/native-chat/agent-session-journal/journal-legacy-retirement.ts @@ -19,8 +19,7 @@ import { import { captureJournalLegacySourceReceipt } from './database/journal-legacy-source-receipt' import { acquireJournalLegacyFileLock, - checkJournalLegacyFileLock, - requireJournalLegacyFileLock + createJournalLegacyFileEligibility } from './journal-legacy-file-lock' import type { JournalSqlOwner } from './journal-sql-owner' import type { JournalSqlReaderPool } from './journal-sql-reader-pool' @@ -58,10 +57,7 @@ export class JournalLegacyRetirement { private readonly ownsProfile = () => ownsOrcadProfileDirectory(directory), private readonly lock = acquireJournalLegacyFileLock, private readonly rest: (started: number) => Promise = async () => {}, - private readonly eligibility = { - check: checkJournalLegacyFileLock, - present: requireJournalLegacyFileLock - } + private readonly eligibility = createJournalLegacyFileEligibility() ) { this.filename = journalDatabasePath(directory) }