diff --git a/cli/src/codex/codexLocalLauncher.test.ts b/cli/src/codex/codexLocalLauncher.test.ts index 9459449b..088ec696 100644 --- a/cli/src/codex/codexLocalLauncher.test.ts +++ b/cli/src/codex/codexLocalLauncher.test.ts @@ -361,7 +361,7 @@ describe('codexLocalLauncher', () => { }); }); - it('falls back to fresh transcript activity when SessionStart does not arrive', async () => { + it('falls back to the top-level review transcript when a review subagent is active', async () => { const originalCodexHome = process.env.CODEX_HOME; process.env.CODEX_HOME = tempDir; const now = new Date(); @@ -373,8 +373,9 @@ describe('codexLocalLauncher', () => { String(now.getUTCDate()).padStart(2, '0') ); await mkdir(sessionDirectory, { recursive: true }); - const transcriptPath = join(sessionDirectory, 'rollout-fallback-thread.jsonl'); - const { session, userMessages } = createSessionStub( + const transcriptPath = join(sessionDirectory, 'rollout-review-primary.jsonl'); + const reviewSubagentPath = join(sessionDirectory, 'rollout-review-subagent.jsonl'); + const { session, userMessages, agentMessages } = createSessionStub( 'default', ['--cd', '/tmp/effective-codex-cwd'], '/tmp/worktree', @@ -392,27 +393,66 @@ describe('codexLocalLauncher', () => { await vi.waitFor(() => expect(harness.launches).toHaveLength(1)); expect(session.sessionId).toBeNull(); - await writeFile(transcriptPath, [ - JSON.stringify({ - type: 'session_meta', - payload: { id: 'fallback-thread', cwd: '/tmp/effective-codex-cwd' } - }), - JSON.stringify({ - timestamp: new Date().toISOString(), - type: 'event_msg', - payload: { type: 'user_message', message: 'fallback prompt' } - }) - ].join('\n') + '\n'); + await Promise.all([ + writeFile(transcriptPath, [ + JSON.stringify({ + type: 'session_meta', + payload: { + id: 'review-primary', + cwd: '/tmp/effective-codex-cwd', + source: 'cli' + } + }), + JSON.stringify({ + timestamp: new Date().toISOString(), + type: 'event_msg', + payload: { type: 'user_message', message: '/review' } + }), + JSON.stringify({ + timestamp: new Date().toISOString(), + type: 'event_msg', + payload: { type: 'agent_message', message: 'final review result' } + }) + ].join('\n') + '\n'), + writeFile(reviewSubagentPath, [ + JSON.stringify({ + type: 'session_meta', + payload: { + id: 'review-subagent', + cwd: '/tmp/effective-codex-cwd', + source: { subagent: 'review' } + } + }), + JSON.stringify({ + timestamp: new Date().toISOString(), + type: 'event_msg', + payload: { type: 'user_message', message: 'review instructions' } + }), + JSON.stringify({ + timestamp: new Date().toISOString(), + type: 'event_msg', + payload: { type: 'agent_message', message: 'internal review work' } + }) + ].join('\n') + '\n') + ]); await vi.waitFor( - () => expect(session.sessionId).toBe('fallback-thread'), + () => expect(session.sessionId).toBe('review-primary'), { timeout: 3_000, interval: 50 } ); if (releaseRunBarrier) releaseRunBarrier(); await launcherPromise; expect(session.transcriptPath).toBe(transcriptPath); - expect(userMessages).toContain('fallback prompt'); + expect(userMessages).toContain('/review'); + expect(agentMessages).toContainEqual(expect.objectContaining({ + type: 'message', + message: 'final review result' + })); + expect(agentMessages).not.toContainEqual(expect.objectContaining({ + type: 'message', + message: 'internal review work' + })); } finally { if (releaseRunBarrier) releaseRunBarrier(); if (originalCodexHome === undefined) { diff --git a/cli/src/codex/utils/codexSessionMetadata.ts b/cli/src/codex/utils/codexSessionMetadata.ts new file mode 100644 index 00000000..3471a9be --- /dev/null +++ b/cli/src/codex/utils/codexSessionMetadata.ts @@ -0,0 +1,6 @@ +export function isCodexSubagentSource(value: unknown): boolean { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return false; + } + return Object.prototype.hasOwnProperty.call(value, 'subagent'); +} diff --git a/cli/src/codex/utils/codexTranscriptLocator.test.ts b/cli/src/codex/utils/codexTranscriptLocator.test.ts index 0a75bc14..1cf58274 100644 --- a/cli/src/codex/utils/codexTranscriptLocator.test.ts +++ b/cli/src/codex/utils/codexTranscriptLocator.test.ts @@ -129,6 +129,42 @@ describe('codexTranscriptLocator', () => { expect(new Set(ambiguous[0])).toEqual(new Set([first, second])); }); + it('ignores a review subagent and waits for the top-level review transcript', async () => { + const located: string[] = []; + const ambiguous: string[][] = []; + locator = createCodexTranscriptLocator({ + cwd: '/tmp/project', + startupTimestampMs: Date.now(), + intervalMs: 25, + settlementMs: 50, + onLocated: (result) => located.push(result.transcriptPath), + onAmbiguous: (paths) => ambiguous.push(paths) + }); + await locator.ready; + const reviewSubagent = await createTranscript( + 'thread-review-subagent', + '/tmp/project', + { subagent: 'review' } + ); + const userEvent = (message: string) => `${JSON.stringify({ + timestamp: new Date().toISOString(), + type: 'event_msg', + payload: { type: 'user_message', message } + })}\n`; + + await appendFile(reviewSubagent, userEvent('review instructions')); + await wait(100); + expect(located).toEqual([]); + expect(ambiguous).toEqual([]); + + const primary = await createTranscript('thread-review-primary', '/tmp/project', 'cli'); + await appendFile(primary, userEvent('/review')); + await wait(150); + + expect(located).toEqual([primary]); + expect(ambiguous).toEqual([]); + }); + it('rejects candidates whose activity arrives in adjacent polling cycles', async () => { const located: string[] = []; const ambiguous: string[][] = []; @@ -208,9 +244,13 @@ describe('codexTranscriptLocator', () => { expect(located).toEqual([]); }); - it('polls only the exact resume transcript once it is found', async () => { + it('polls only the exact resume transcript once it is found, including a subagent', async () => { const unrelated = await createTranscript('thread-unrelated', '/tmp/project'); - const target = await createTranscript('thread-resume', '/tmp/original-project'); + const target = await createTranscript( + 'thread-resume', + '/tmp/original-project', + { subagent: 'review' } + ); await appendFile(unrelated, `${JSON.stringify({ timestamp: new Date().toISOString(), type: 'event_msg', @@ -239,11 +279,15 @@ describe('codexTranscriptLocator', () => { expect(ambiguous).toEqual([]); }); - async function createTranscript(sessionId: string, cwd: string): Promise { + async function createTranscript(sessionId: string, cwd: string, source?: unknown): Promise { const transcriptPath = join(sessionDirectory, `rollout-${sessionId}.jsonl`); await writeFile(transcriptPath, `${JSON.stringify({ type: 'session_meta', - payload: { id: sessionId, cwd } + payload: { + id: sessionId, + cwd, + ...(source === undefined ? {} : { source }) + } })}\n`); return transcriptPath; } diff --git a/cli/src/codex/utils/codexTranscriptLocator.ts b/cli/src/codex/utils/codexTranscriptLocator.ts index 6d5bcd50..0e3a497b 100644 --- a/cli/src/codex/utils/codexTranscriptLocator.ts +++ b/cli/src/codex/utils/codexTranscriptLocator.ts @@ -3,6 +3,7 @@ import { join, resolve } from 'node:path'; import { open, readdir, stat } from 'node:fs/promises'; import { logger } from '@/ui/logger'; import { convertCodexEvent, type CodexSessionEvent } from './codexEventConverter'; +import { isCodexSubagentSource } from './codexSessionMetadata'; export type LocatedCodexTranscript = { sessionId: string; @@ -21,6 +22,7 @@ type TranscriptState = { ino: number; sessionId: string | null; cwd: string | null; + isSubagent: boolean; }; type CodexTranscriptLocatorOptions = { @@ -170,7 +172,8 @@ class CodexTranscriptLocatorImpl { mtimeMs: 0, ino: fileStats.ino, sessionId: null, - cwd: null + cwd: null, + isSubagent: false }; const replaced = previous && previous.ino !== fileStats.ino; @@ -186,7 +189,8 @@ class CodexTranscriptLocatorImpl { mtimeMs: 0, ino: fileStats.ino, sessionId: null, - cwd: null + cwd: null, + isSubagent: false }; } else if (previous && fileStats.size === previous.size @@ -238,6 +242,9 @@ class CodexTranscriptLocatorImpl { state.sessionId = asString(metadata?.id) ?? state.sessionId; const eventCwd = asString(metadata?.cwd); state.cwd = eventCwd ? normalizePath(eventCwd) : state.cwd; + if (metadata && Object.prototype.hasOwnProperty.call(metadata, 'source')) { + state.isSubagent = isCodexSubagentSource(metadata.source); + } } if (convertCodexEvent(event)?.userActivity) { @@ -263,8 +270,10 @@ class CodexTranscriptLocatorImpl { if (state.sessionId !== this.resumeSessionId) { return null; } - } else if (state.cwd !== this.targetCwd) { - return null; + } else { + if (state.isSubagent || state.cwd !== this.targetCwd) { + return null; + } } return { sessionId: state.sessionId, transcriptPath }; diff --git a/cli/src/modules/common/codexSessions.ts b/cli/src/modules/common/codexSessions.ts index 40297c0a..b0dcda98 100644 --- a/cli/src/modules/common/codexSessions.ts +++ b/cli/src/modules/common/codexSessions.ts @@ -3,6 +3,7 @@ import { randomUUID } from 'node:crypto' import { basename, dirname, join, relative } from 'node:path' import { homedir } from 'node:os' import { AGENT_MESSAGE_PAYLOAD_TYPE } from '@hapi/protocol' +import { isCodexSubagentSource } from '@/codex/utils/codexSessionMetadata' const DEFAULT_CODEX_SESSION_SCAN_LIMIT = 200 @@ -78,11 +79,6 @@ function shouldIgnoreSyntheticUserMessage(text: string): boolean { return normalized.startsWith('# AGENTS.md instructions') || normalized.startsWith('') } -function isSubagentSource(value: unknown): boolean { - const record = asRecord(value) - return Boolean(record && Object.prototype.hasOwnProperty.call(record, 'subagent')) -} - function inferSessionIdFromFileName(filePath: string): string | null { return /([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})/.exec(filePath)?.[1] ?? null } @@ -338,7 +334,7 @@ function parseCodexLocalSession( if (!record) continue if (record.type === 'session_meta') { const payload = asRecord(record.payload) - if (isSubagentSource(payload?.source)) return null + if (isCodexSubagentSource(payload?.source)) return null if (!sessionId && typeof payload?.id === 'string') sessionId = payload.id if (!cwd && typeof payload?.cwd === 'string') cwd = payload.cwd if (!originator && typeof payload?.originator === 'string') originator = payload.originator