From 48396fc15ce19df0d4f8f6085a8a27cce159aca2 Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Wed, 9 Sep 2026 02:16:18 +0800 Subject: [PATCH 1/7] fix(codex): keep SessionStart as a clear, not a completed turn SessionStart is an idle TUI/resume boundary. Clear the pane instead of publishing working/done, prepend remote hooks with index-addressed trust moves, and forward a working tombstone so old-main cannot treat this as completion. Co-authored-by: Cursor --- .../server-codex-normalization.test.ts | 28 +- ...codex-session-start-wire-semantics.test.ts | 250 ++++++++++++++ .../server/server-ingest-remote.ts | 13 + .../agent-hooks/server/server-lifecycle.ts | 5 + .../server/server-status-identity.ts | 11 + .../server/server-status-retries.ts | 26 ++ .../agent-hooks/server/server-tab-cleanup.ts | 5 + .../codex/codex-hook-remote-install.test.ts | 136 ++++++++ src/main/codex/codex-hook-remote-install.ts | 29 +- .../codex-hook-remote-user-trust-moves.ts | 68 ++++ src/main/codex/config-toml-hook-trust-edit.ts | 39 +++ src/main/codex/config-toml-trust.ts | 16 +- src/relay/agent-hook-envelope-build.ts | 8 + src/relay/agent-hook-server.test.ts | 107 ++++++ src/relay/agent-hook-server.ts | 55 ++- ...-approval-notification-suppression.test.ts | 22 ++ ...-auto-approval-notification-suppression.ts | 6 +- src/renderer/src/lib/agent-status.test.ts | 7 + src/shared/agent-detection.ts | 2 + ...-hook-listener-codex-session-start.test.ts | 321 ++++++++++++++++++ src/shared/agent-hook-listener.ts | 28 +- .../agent-hook-listener/listener-state.ts | 8 + .../providers/codex-events.ts | 84 ++++- .../providers/codex-tool-fields.ts | 33 +- .../agent-hook-listener/tool-input-preview.ts | 7 +- src/shared/agent-title-status.ts | 12 + 26 files changed, 1283 insertions(+), 43 deletions(-) create mode 100644 src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts create mode 100644 src/main/codex/codex-hook-remote-install.test.ts create mode 100644 src/main/codex/codex-hook-remote-user-trust-moves.ts create mode 100644 src/shared/agent-hook-listener-codex-session-start.test.ts diff --git a/src/main/agent-hooks/server-codex-normalization.test.ts b/src/main/agent-hooks/server-codex-normalization.test.ts index b400a3f2dacc..323659074cfb 100644 --- a/src/main/agent-hooks/server-codex-normalization.test.ts +++ b/src/main/agent-hooks/server-codex-normalization.test.ts @@ -193,8 +193,8 @@ describe('Codex hook normalization', () => { expect(result?.payload.toolInput).toBeUndefined() }) - it('SessionStart clears cached tool state from a prior session', () => { - // Seed a Stop snapshot with an assistant message. + it('SessionStart clears cached tool state without reporting working', () => { + // Why: SessionStart is an idle TUI/resume boundary, not an active turn. _internals.normalizeHookPayload( 'codex', buildBody({ @@ -205,11 +205,18 @@ describe('Codex hook normalization', () => { ) const result = _internals.normalizeHookPayload( 'codex', - buildBody({ hook_event_name: 'SessionStart' }), + buildBody({ hook_event_name: 'SessionStart', session_id: 'codex-session-next' }), 'production' ) - expect(result?.payload.state).toBe('working') - expect(result?.payload.lastAssistantMessage).toBeUndefined() + const prompted = _internals.normalizeHookPayload( + 'codex', + buildBody({ hook_event_name: 'UserPromptSubmit', prompt: 'next turn' }), + 'production' + ) + expect(result).toBeNull() + expect(prompted?.payload.state).toBe('working') + expect(prompted?.payload.lastAssistantMessage).toBeUndefined() + expect(prompted?.providerSession).toEqual({ key: 'session_id', id: 'codex-session-next' }) }) it('SessionStart clears the cached prompt from a prior session until a new prompt arrives', () => { @@ -223,10 +230,15 @@ describe('Codex hook normalization', () => { ) const result = _internals.normalizeHookPayload( 'codex', - buildBody({ hook_event_name: 'SessionStart' }), + buildBody({ hook_event_name: 'SessionStart', session_id: 'codex-session-fresh' }), 'production' ) - expect(result?.payload.state).toBe('working') - expect(result?.payload.prompt).toBe('') + const prompted = _internals.normalizeHookPayload( + 'codex', + buildBody({ hook_event_name: 'UserPromptSubmit', prompt: 'fresh prompt' }), + 'production' + ) + expect(result).toBeNull() + expect(prompted?.payload.prompt).toBe('fresh prompt') }) }) diff --git a/src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts b/src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts new file mode 100644 index 000000000000..4440b413615e --- /dev/null +++ b/src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts @@ -0,0 +1,250 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { agentEntryCompletionAt } from '../../shared/agent-completion-time' +import { AgentHookServer, _internals } from './server' +import { buildBody, PANE } from './server.test-fixtures' + +const { getCohortAtEmitMock, trackMock } = vi.hoisted(() => ({ + getCohortAtEmitMock: vi.fn(), + trackMock: vi.fn() +})) + +vi.mock('../telemetry/client', () => ({ + track: trackMock +})) + +vi.mock('../telemetry/cohort-classifier', () => ({ + getCohortAtEmit: getCohortAtEmitMock +})) + +beforeEach(() => { + _internals.resetCachesForTests() + trackMock.mockReset() + getCohortAtEmitMock.mockReset() + getCohortAtEmitMock.mockReturnValue({ nth_repo_added: 2 }) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +function completionEntry(overrides: { + state: 'done' | 'working' + sessionBoundary?: boolean + stateStartedAt?: number + updatedAt?: number +}) { + return { + paneKey: PANE, + state: overrides.state, + updatedAt: overrides.updatedAt ?? 2_000, + stateStartedAt: overrides.stateStartedAt ?? 1_500, + sessionBoundary: overrides.sessionBoundary + } +} + +describe('Codex SessionStart mixed-version content semantics', () => { + it('treats plain done without sessionBoundary as a finished turn', () => { + const plainDone = completionEntry({ state: 'done' }) + // Same predicate as automation-run-completion-evidence and + // automation-dispatch-completion: sessionBoundary is the only suppressor. + const isAutomationCompletion = + plainDone.state === 'done' && + plainDone.sessionBoundary !== true && + plainDone.updatedAt >= 1_000 + expect(agentEntryCompletionAt(plainDone)).toBe(1_500) + expect(isAutomationCompletion).toBe(true) + }) + + it('does not treat sessionBoundary done as a finished turn', () => { + const boundary = completionEntry({ state: 'done', sessionBoundary: true }) + const isAutomationCompletion = + boundary.state === 'done' && boundary.sessionBoundary !== true && boundary.updatedAt >= 1_000 + expect(agentEntryCompletionAt(boundary)).toBeNull() + expect(isAutomationCompletion).toBe(false) + }) + + it('clears an old-relay working SessionStart without applying a done row', () => { + const server = new AgentHookServer() + const clearListener = vi.fn() + server.setPaneStatusClearListener(clearListener) + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + hookEventName: 'UserPromptSubmit', + source: 'codex', + payload: { state: 'working', prompt: 'old session', agentType: 'codex' } + }, + 'conn-1' + ) + clearListener.mockClear() + + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + hookEventName: 'SessionStart', + source: 'codex', + providerSession: { key: 'session_id', id: 'relay-new-session' }, + payload: { state: 'working', prompt: '', agentType: 'codex' } + }, + 'conn-1' + ) + + expect(clearListener).toHaveBeenCalledTimes(1) + expect(server.getStatusSnapshot()).toEqual([]) + expect(server.getStatusSnapshot().some((row) => row.state === 'done')).toBe(false) + expect(server._getStateForTests().lastProviderSessionByPaneKey.get(PANE)).toEqual({ + key: 'session_id', + id: 'relay-new-session' + }) + }) + + it('does not apply a legacy plain-done SessionStart as a completed turn', () => { + const server = new AgentHookServer() + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + hookEventName: 'UserPromptSubmit', + source: 'codex', + payload: { state: 'working', prompt: 'old session', agentType: 'codex' } + }, + 'conn-1' + ) + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + hookEventName: 'SessionStart', + source: 'codex', + payload: { state: 'done', prompt: '', agentType: 'codex' } + }, + 'conn-1' + ) + + const snapshot = server.getStatusSnapshot() + expect(snapshot).toEqual([]) + expect(agentEntryCompletionAt(completionEntry({ state: 'done' }))).toBe(1_500) + expect(snapshot.some((row) => row.state === 'done' && row.sessionBoundary !== true)).toBe(false) + }) + + it('treats a duplicate relayed SessionStart as an idempotent clear', () => { + const server = new AgentHookServer() + const clearListener = vi.fn() + server.setPaneStatusClearListener(clearListener) + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'UserPromptSubmit', + source: 'codex', + payload: { state: 'working', prompt: 'old session', agentType: 'codex' } + }, + 'conn-1' + ) + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'SessionStart', + source: 'codex', + payload: { state: 'working', prompt: '', agentType: 'codex' } + }, + 'conn-1' + ) + clearListener.mockClear() + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'SessionStart', + source: 'codex', + isReplay: true, + payload: { state: 'working', prompt: '', agentType: 'codex' } + }, + 'conn-1' + ) + + expect(clearListener).not.toHaveBeenCalled() + expect(server.getStatusSnapshot()).toEqual([]) + }) + + it('does not let a relayed Codex SessionStart clear a different agent status', () => { + const server = new AgentHookServer() + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'UserPromptSubmit', + source: 'claude', + payload: { state: 'working', prompt: 'parent session', agentType: 'claude' } + }, + 'conn-1' + ) + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'SessionStart', + source: 'codex', + payload: { state: 'working', prompt: '', agentType: 'codex' } + }, + 'conn-1' + ) + + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ + paneKey: PANE, + state: 'working', + agentType: 'claude', + prompt: 'parent session' + }) + ]) + }) + + it('broadcasts a local HTTP SessionStart clear once and keeps the next prompt working', async () => { + const server = new AgentHookServer() + await server.start({ env: 'production' }) + try { + const env = server.buildPtyEnv() + const clearListener = vi.fn() + server.setPaneStatusClearListener(clearListener) + const post = (payload: Record): Promise => + fetch(`http://127.0.0.1:${env.ORCA_AGENT_HOOK_PORT}/hook/codex`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': env.ORCA_AGENT_HOOK_TOKEN + }, + body: JSON.stringify(buildBody(payload)) + }) + + expect( + (await post({ hook_event_name: 'UserPromptSubmit', prompt: 'old session' })).status + ).toBe(204) + clearListener.mockClear() + expect( + (await post({ hook_event_name: 'SessionStart', session_id: 'new-session' })).status + ).toBe(204) + expect( + (await post({ hook_event_name: 'SessionStart', session_id: 'new-session' })).status + ).toBe(204) + expect(clearListener).toHaveBeenCalledTimes(1) + expect(server.getStatusSnapshot()).toEqual([]) + + expect( + (await post({ hook_event_name: 'UserPromptSubmit', prompt: 'next turn' })).status + ).toBe(204) + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ + paneKey: PANE, + state: 'working', + prompt: 'next turn', + providerSession: { key: 'session_id', id: 'new-session' } + }) + ]) + } finally { + server.stop() + } + }) +}) diff --git a/src/main/agent-hooks/server/server-ingest-remote.ts b/src/main/agent-hooks/server/server-ingest-remote.ts index 0bc8d7947a9a..96d7ca39e4bc 100644 --- a/src/main/agent-hooks/server/server-ingest-remote.ts +++ b/src/main/agent-hooks/server/server-ingest-remote.ts @@ -254,6 +254,19 @@ export abstract class AgentHookServerIngestRemote extends AgentHookServerIngestS env: envelope.env, expectedEnv: this.env }) + if (hookEventName === 'SessionStart' && normalizedPayload.agentType === 'codex') { + // Why: SessionStart is an idle metadata boundary. Old relays encode it as + // working; the original tombstone used plain done. New main must not apply + // either — plain done is a finished-turn signal to completion-reactive + // consumers unless sessionBoundary is stamped, and that flag is optional + // so old clients ignore it (remote-wire Rule 3). Clear the stale row and + // keep session identity for the next real status event. + if (providerSession) { + this.state.lastProviderSessionByPaneKey.set(paneKey, providerSession) + } + this.clearStatusForSessionStart(paneKey, undefined, trimmedConnectionId ?? undefined) + return + } const event: AgentHookEventPayload & { authorityRestartId?: string } = { paneKey, source: effectiveSource, diff --git a/src/main/agent-hooks/server/server-lifecycle.ts b/src/main/agent-hooks/server/server-lifecycle.ts index 5b8bf908d552..aa3ad2ae824f 100644 --- a/src/main/agent-hooks/server/server-lifecycle.ts +++ b/src/main/agent-hooks/server/server-lifecycle.ts @@ -13,6 +13,7 @@ import { HOOK_REQUEST_SLOWLORIS_MS } from '../../../shared/agent-hook-listener/l import { isHookRequestTruncatedError } from '../../../shared/agent-hook-transport-interference' import { drainAgentHookSpool, type SpoolRecord } from '../../../shared/agent-hook-spool' import { clearAllListenerCaches } from '../../../shared/agent-hook-listener/listener-state' +import { hookBodyPaneKey } from './server-status-identity' import { trackEmptyPaneKeyHook } from './server-transport-rules' import { AgentHookServerRuntimeEnv } from './server-runtime-env' @@ -95,6 +96,8 @@ export abstract class AgentHookServerLifecycle extends AgentHookServerRuntimeEnv const hookBody = mergeAgentHookRequestHeaders(body, req.headers) trackEmptyPaneKeyHook(hookBody) const aliasedBody = this.normalizeHookBodyPaneKeyAlias(hookBody) + const paneKey = hookBodyPaneKey(aliasedBody) + const previousStatus = paneKey ? this.state.lastStatusByPaneKey.get(paneKey) : undefined const normalized = this.normalizeLocalHookPayload(source, aliasedBody) const statusDisposition = normalized.event ? this.getAgentStatusDisposition(normalized.event.paneKey, { @@ -134,6 +137,8 @@ export abstract class AgentHookServerLifecycle extends AgentHookServerRuntimeEnv this.scheduleAssistantMessageRetry(source, aliasedBody, enriched) this.scheduleCodexSubagentPoll(source, aliasedBody, enriched) } + } else if (paneKey && previousStatus && !this.state.lastStatusByPaneKey.has(paneKey)) { + this.clearStatusForSessionStart(paneKey, previousStatus) } res.writeHead(204) res.end() diff --git a/src/main/agent-hooks/server/server-status-identity.ts b/src/main/agent-hooks/server/server-status-identity.ts index 1694c4b1b676..580b4aef9d35 100644 --- a/src/main/agent-hooks/server/server-status-identity.ts +++ b/src/main/agent-hooks/server/server-status-identity.ts @@ -35,6 +35,17 @@ export function equivalentInterruptAgentType( } // Why: validate the durable `${tabId}:${leafUuid}` leaf suffix at write/hydrate so legacy numeric rows fail closed. +export function hookBodyPaneKey(body: unknown): string | null { + if (typeof body !== 'object' || body === null) { + return null + } + const paneKey = (body as Record).paneKey + if (typeof paneKey !== 'string') { + return null + } + return paneKey.trim() || null +} + export function isValidPaneKey(value: unknown): value is string { return ( typeof value === 'string' && value.length <= MAX_PANE_KEY_LEN && parsePaneKey(value) !== null diff --git a/src/main/agent-hooks/server/server-status-retries.ts b/src/main/agent-hooks/server/server-status-retries.ts index 4620506b210e..8a81f2ccd583 100644 --- a/src/main/agent-hooks/server/server-status-retries.ts +++ b/src/main/agent-hooks/server/server-status-retries.ts @@ -6,6 +6,7 @@ import { } from '../../../shared/agent-hook-listener/grok-result-discovery' import type { AgentHookSource } from '../../../shared/agent-hook-relay' import { CodexSubagentPollScheduler } from '../../../shared/codex-subagent-poll-scheduler' +import type { AgentHookEventPayload } from '../../../shared/agent-hook-listener/listener-event' import type { EnrichedAgentHookEventPayload } from './server-types' import { ASSISTANT_MESSAGE_RETRY_ATTEMPTS, @@ -30,6 +31,31 @@ export abstract class AgentHookServerStatusRetries extends AgentHookServerStatus this.codexSubagentPollScheduler.clearAll() } + protected clearStatusForSessionStart( + paneKey: string, + previousStatus?: AgentHookEventPayload, + expectedConnectionId?: string + ): void { + const status = previousStatus ?? this.state.lastStatusByPaneKey.get(paneKey) + // Why: a delayed relay notification must not clear a newer remote target's + // status if pane identity is ever reused across logical connections. + if ( + status?.payload.agentType !== 'codex' || + (expectedConnectionId !== undefined && status.connectionId !== expectedConnectionId) + ) { + return + } + this.state.lastStatusByPaneKey.delete(paneKey) + // Why: SessionStart is an idle metadata boundary, so remove the stale row + // without emitting a synthetic visible status for the new session. + this.clearAssistantMessageRetry(paneKey) + this.runtimeObservedStatusPaneKeys.delete(paneKey) + this.promptSentDedupeByPaneKey.delete(paneKey) + this.scheduleStatusPersist() + this.notifyStatusChangeListeners() + this.emitPaneStatusCleared({ paneKey }) + } + protected clearAssistantMessageRetry(paneKey: string): void { const timer = this.assistantMessageRetryTimers.get(paneKey) if (!timer) { diff --git a/src/main/agent-hooks/server/server-tab-cleanup.ts b/src/main/agent-hooks/server/server-tab-cleanup.ts index a108bdbbd22f..bc0b5b45a35a 100644 --- a/src/main/agent-hooks/server/server-tab-cleanup.ts +++ b/src/main/agent-hooks/server/server-tab-cleanup.ts @@ -32,6 +32,11 @@ export abstract class AgentHookServerTabCleanup extends AgentHookServerCleanup { paneKeysToClear.add(key.split('\0', 1)[0] ?? key) } } + for (const key of this.state.lastProviderSessionByPaneKey.keys()) { + if (paneCacheKeyMatchesTab(key, tabId)) { + paneKeysToClear.add(key.split('\0', 1)[0] ?? key) + } + } for (const key of this.state.antigravityCompletedTranscriptByPaneKey.keys()) { if (paneCacheKeyMatchesTab(key, tabId)) { paneKeysToClear.add(key.split('\0', 1)[0] ?? key) diff --git a/src/main/codex/codex-hook-remote-install.test.ts b/src/main/codex/codex-hook-remote-install.test.ts new file mode 100644 index 000000000000..6ba5de549736 --- /dev/null +++ b/src/main/codex/codex-hook-remote-install.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it, vi } from 'vitest' +import type { SFTPWrapper } from 'ssh2' +import { CodexHookService } from './hook-service' +import { upsertHookTrustEntriesInContent } from './config-toml-trust' + +vi.mock('electron', () => ({ + app: { + getPath: () => '/tmp/orca-user-data' + } +})) + +function createFakeSftp(initialFiles: Record = {}): { + sftp: SFTPWrapper + files: Map +} { + const files = new Map(Object.entries(initialFiles)) + const dirs = new Set(['/']) + const noEntry = (path: string): { code: number; message: string } => ({ + code: 2, + message: `ENOENT ${path}` + }) + const sftp = { + readFile: (path: string, _enc: string, cb: (err: unknown, data?: string) => void): void => { + const value = files.get(path) + if (value === undefined) { + cb(noEntry(path)) + return + } + cb(null, value) + }, + writeFile: ( + path: string, + content: string, + _options: string | { mode?: number }, + cb: (err: unknown) => void + ): void => { + files.set(path, content) + cb(null) + }, + rename: (src: string, dst: string, cb: (err: unknown) => void): void => { + const value = files.get(src) + if (value === undefined) { + cb(noEntry(src)) + return + } + files.set(dst, value) + files.delete(src) + cb(null) + }, + unlink: (path: string, cb: (err: unknown) => void): void => { + files.delete(path) + cb(null) + }, + chmod: (_path: string, _mode: number, cb: (err: unknown) => void): void => { + cb(null) + }, + stat: (path: string, cb: (err: unknown, stats?: { mode: number }) => void): void => { + if (!files.has(path)) { + cb(noEntry(path)) + return + } + cb(null, { mode: 0o100644 }) + }, + readdir: (path: string, cb: (err: unknown, list?: { filename: string }[]) => void): void => { + if (!dirs.has(path)) { + cb(noEntry(path)) + return + } + cb(null, []) + }, + mkdir: (path: string, cb: (err: unknown) => void): void => { + dirs.add(path) + cb(null) + } + } as unknown as SFTPWrapper + return { sftp, files } +} + +function hookTrustBlock(content: string, key: string): string { + const header = `[hooks.state."${key}"]` + const start = content.indexOf(header) + if (start === -1) { + return '' + } + const nextHeader = content.indexOf('\n[', start + header.length) + return content.slice(start, nextHeader === -1 ? content.length : nextHeader) +} + +describe('Codex remote hook prepend + trust migration', () => { + it('prepends remote hooks without invalidating existing user hook trust', async () => { + const remoteHooksPath = '/home/dev/.codex/hooks.json' + const userStopCommand = 'echo user-stop-hook' + const userTrustedHash = 'sha256:user-approved-stop-hook' + const { sftp, files } = createFakeSftp({ + [remoteHooksPath]: `${JSON.stringify({ + hooks: { + Stop: [{ hooks: [{ type: 'command', command: userStopCommand }] }] + } + })}\n`, + '/home/dev/.codex/config.toml': upsertHookTrustEntriesInContent('', [ + { + sourcePath: remoteHooksPath, + eventLabel: 'stop', + groupIndex: 0, + handlerIndex: 0, + command: userStopCommand, + trustedHash: userTrustedHash, + enabled: false + } + ]) + }) + + const service = new CodexHookService() + const status = await service.installRemote(sftp, '/home/dev') + const repeatedStatus = await service.installRemote(sftp, '/home/dev') + + expect(status.state).toBe('installed') + expect(repeatedStatus.state).toBe('installed') + const hooks = JSON.parse(files.get(remoteHooksPath)!) as { + hooks: Record + } + expect(hooks.hooks.Stop?.[0]?.hooks?.[0]?.command).toContain('codex-hook.sh') + expect(hooks.hooks.Stop?.[1]?.hooks?.[0]?.command).toBe(userStopCommand) + expect(hooks.hooks.SubagentStart?.[0]?.hooks?.[0]?.command).toContain('codex-hook.sh') + expect(hooks.hooks.SubagentStop?.[0]?.hooks?.[0]?.command).toContain('codex-hook.sh') + const toml = files.get('/home/dev/.codex/config.toml') ?? '' + expect(toml).toContain(':stop:0:0') + expect(toml).toContain(':subagent_start:0:0') + expect(toml).toContain(':subagent_stop:0:0') + const userStopTrust = hookTrustBlock(toml, `${remoteHooksPath}:stop:1:0`) + expect(userStopTrust).toContain('enabled = false') + expect(userStopTrust).toContain(`trusted_hash = "${userTrustedHash}"`) + expect(hookTrustBlock(toml, `${remoteHooksPath}:stop:0:0`)).not.toContain(userTrustedHash) + expect(toml).not.toContain(`${remoteHooksPath}:stop:2:0`) + }) +}) diff --git a/src/main/codex/codex-hook-remote-install.ts b/src/main/codex/codex-hook-remote-install.ts index 2b6105cdc3df..7149b1e94bdb 100644 --- a/src/main/codex/codex-hook-remote-install.ts +++ b/src/main/codex/codex-hook-remote-install.ts @@ -15,7 +15,13 @@ import { writeManagedScriptRemote, writeTextFileRemoteAtomic } from '../agent-hooks/installer-utils-remote' -import { upsertHookTrustEntriesInContent, type CodexTrustEntry } from './config-toml-trust' +import { + moveHookTrustEntriesInContent, + upsertHookTrustEntriesInContent, + type CodexHookTrustKeyMove, + type CodexTrustEntry +} from './config-toml-trust' +import { collectPrependedRemoteUserTrustMoves } from './codex-hook-remote-user-trust-moves' import { CODEX_EVENTS, CODEX_EVENT_LABEL, @@ -71,19 +77,29 @@ export async function installCodexHooksRemote( } const trustEntries: CodexTrustEntry[] = [] + const userTrustMoves: CodexHookTrustKeyMove[] = [] for (const eventName of CODEX_EVENTS) { const current = Array.isArray(nextHooks[eventName]) ? nextHooks[eventName] : [] const cleaned = removeManagedCommands(current, isManagedCommand) const definition: HookDefinition = { hooks: [buildManagedCommandHook(command)] } - nextHooks[eventName] = redirectedCodexHome - ? [definition, ...cleaned] - : [...cleaned, definition] + // Why: local installs already place Orca first; remote installs must + // not wait behind slow user hooks before publishing terminal status. + nextHooks[eventName] = [definition, ...cleaned] + userTrustMoves.push( + ...collectPrependedRemoteUserTrustMoves( + remoteConfigPath, + eventName, + current, + cleaned, + isManagedCommand + ) + ) trustEntries.push({ sourcePath: remoteConfigPath, eventLabel: CODEX_EVENT_LABEL[eventName], - groupIndex: redirectedCodexHome ? 0 : cleaned.length, + groupIndex: 0, handlerIndex: 0, command, timeoutSec: MANAGED_HOOK_TIMEOUT_SECONDS @@ -108,7 +124,8 @@ export async function installCodexHooksRemote( } } const existingToml = existingTomlRaw ?? '' - const updatedToml = upsertHookTrustEntriesInContent(existingToml, trustEntries) + const movedUserTrust = moveHookTrustEntriesInContent(existingToml, userTrustMoves) + const updatedToml = upsertHookTrustEntriesInContent(movedUserTrust, trustEntries) if (updatedToml !== existingToml) { await writeTextFileRemoteAtomic(sftp, remoteTomlPath, updatedToml) } diff --git a/src/main/codex/codex-hook-remote-user-trust-moves.ts b/src/main/codex/codex-hook-remote-user-trust-moves.ts new file mode 100644 index 000000000000..aac9301a9eed --- /dev/null +++ b/src/main/codex/codex-hook-remote-user-trust-moves.ts @@ -0,0 +1,68 @@ +import type { HookDefinition } from '../agent-hooks/installer-utils' +import { createCodexHookTrustEntry, getCodexHookTrustSignature } from './codex-hook-identity' +import { CODEX_EVENTS } from './codex-hook-definition' +import { + computeTrustKey, + type CodexHookTrustKeyMove, + type CodexTrustEntry +} from './config-toml-trust' + +// Why: repeated remote installs may find Orca before or after user hooks. Match +// user content across cleanup so only approvals whose real index changed move. +export function collectPrependedRemoteUserTrustMoves( + sourcePath: string, + eventName: (typeof CODEX_EVENTS)[number], + current: readonly HookDefinition[], + cleaned: readonly HookDefinition[], + isManagedCommand: (command: string | undefined) => boolean +): CodexHookTrustKeyMove[] { + const oldEntriesBySignature = new Map() + current.forEach((definition, groupIndex) => { + const hooks = Array.isArray(definition.hooks) ? definition.hooks : [] + hooks.forEach((hook, handlerIndex) => { + if (isManagedCommand(hook.command)) { + return + } + const entry = createCodexHookTrustEntry( + sourcePath, + eventName, + groupIndex, + handlerIndex, + definition, + hook + ) + if (!entry) { + return + } + const signature = getCodexHookTrustSignature(entry) + const entries = oldEntriesBySignature.get(signature) ?? [] + entries.push(entry) + oldEntriesBySignature.set(signature, entries) + }) + }) + + const moves: CodexHookTrustKeyMove[] = [] + cleaned.forEach((definition, cleanedGroupIndex) => { + const hooks = Array.isArray(definition.hooks) ? definition.hooks : [] + hooks.forEach((hook, handlerIndex) => { + const nextEntry = createCodexHookTrustEntry( + sourcePath, + eventName, + cleanedGroupIndex + 1, + handlerIndex, + definition, + hook + ) + if (!nextEntry) { + return + } + const oldEntries = oldEntriesBySignature.get(getCodexHookTrustSignature(nextEntry)) + const oldEntry = oldEntries?.shift() + if (!oldEntry) { + return + } + moves.push({ fromKey: computeTrustKey(oldEntry), toKey: computeTrustKey(nextEntry) }) + }) + }) + return moves +} diff --git a/src/main/codex/config-toml-hook-trust-edit.ts b/src/main/codex/config-toml-hook-trust-edit.ts index a8487012f058..b783cf64bb10 100644 --- a/src/main/codex/config-toml-hook-trust-edit.ts +++ b/src/main/codex/config-toml-hook-trust-edit.ts @@ -1,4 +1,10 @@ import type { CodexTrustEntry } from './config-toml-trust' +import { readHookTrustContent } from './config-toml-hook-trust-read' + +export type CodexHookTrustKeyMove = { + fromKey: string + toKey: string +} import { computeCodexTrustedHash, computeCodexTrustKey, @@ -35,6 +41,39 @@ export function upsertHookTrustContent( return updated } +export function moveHookTrustContent( + existingContent: string, + moves: readonly CodexHookTrustKeyMove[] +): string { + const existing = stripLeadingBom(existingContent) + const states = readHookTrustContent(existing) + const resolvedMoves = moves.flatMap(({ fromKey, toKey }) => { + if (normalizeCodexHookTrustLookupKey(fromKey) === normalizeCodexHookTrustLookupKey(toKey)) { + return [] + } + const state = states.get(fromKey) + return state?.trustedHash + ? [{ fromKey, toKey, trustedHash: state.trustedHash, enabled: state.enabled }] + : [] + }) + let updated = existing + // Why: remove old index-addressed user trust blocks before writing the shifted keys so remote prepend does not leave duplicate approvals. + if (resolvedMoves.length > 0) { + updated = removeHookTrustContent(updated, [ + ...new Set(resolvedMoves.map(({ fromKey }) => fromKey)) + ]) + } + for (const { toKey, trustedHash, enabled } of resolvedMoves) { + updated = upsertTrustBlocks( + updated, + getTrustKeyWriteVariants(toKey), + trustedHash, + enabled ?? true + ) + } + return updated +} + export function removeHookTrustContent(content: string, keys: readonly string[]): string { const normalizedKeys = new Set(keys.map(normalizeCodexHookTrustLookupKey)) const ranges = findHookTrustBlockRanges(content, normalizedKeys) diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index 8b1943c058e2..4df38371eba2 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -11,7 +11,14 @@ import { parseCodexTrustKey } from './codex-trust-identity' import { writeTomlConfigAtomically } from './config-toml-atomic-write' -import { removeHookTrustContent, upsertHookTrustContent } from './config-toml-hook-trust-edit' +import { + moveHookTrustContent, + removeHookTrustContent, + upsertHookTrustContent, + type CodexHookTrustKeyMove +} from './config-toml-hook-trust-edit' + +export type { CodexHookTrustKeyMove } import { CodexHookTrustEntryMap, readHookTrustContent } from './config-toml-hook-trust-read' import { upsertProjectTrustContent } from './config-toml-project-trust' import { escapeTomlBasicString, parseProjectTomlHeaderPath } from './config-toml-syntax' @@ -117,6 +124,13 @@ export function upsertHookTrustEntriesInContent( return upsertHookTrustContent(existingContent, entries) } +export function moveHookTrustEntriesInContent( + existingContent: string, + moves: readonly CodexHookTrustKeyMove[] +): string { + return moveHookTrustContent(existingContent, moves) +} + export function upsertProjectTrustLevel( configPath: string, projectPath: string, diff --git a/src/relay/agent-hook-envelope-build.ts b/src/relay/agent-hook-envelope-build.ts index 6b1b45432d8b..0c83f5c6448c 100644 --- a/src/relay/agent-hook-envelope-build.ts +++ b/src/relay/agent-hook-envelope-build.ts @@ -41,6 +41,14 @@ export function buildRelayHookEnvelope( } } +export function hookBodyPaneKey(body: unknown): string | null { + if (typeof body !== 'object' || body === null) { + return null + } + const value = (body as Record).paneKey + return typeof value === 'string' && value.trim().length > 0 ? value.trim() : null +} + export function hookBodyEnv(body: unknown): string | undefined { if (typeof body !== 'object' || body === null) { return undefined diff --git a/src/relay/agent-hook-server.test.ts b/src/relay/agent-hook-server.test.ts index c3a99b8a20c1..83bf71bb4e34 100644 --- a/src/relay/agent-hook-server.test.ts +++ b/src/relay/agent-hook-server.test.ts @@ -385,6 +385,113 @@ describe('RelayAgentHookServer', () => { } }) + it('forwards Codex SessionStart identity with the next real status event', async () => { + const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + await server.start() + try { + const { port, token } = server.getCoordinates() + const post = (payload: Record): Promise => + fetch(`http://127.0.0.1:${port}/hook/codex`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ paneKey: PANE_KEY, tabId: 'tab-1', payload }) + }) + + expect( + ( + await post({ + hook_event_name: 'SessionStart', + session_id: 'codex-relay-session' + }) + ).status + ).toBe(204) + expect(forward).not.toHaveBeenCalled() + + expect( + (await post({ hook_event_name: 'UserPromptSubmit', prompt: 'relay this status' })).status + ).toBe(204) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + source: 'codex', + providerSession: { key: 'session_id', id: 'codex-relay-session' }, + payload: { state: 'working', prompt: 'relay this status' } + }) + } finally { + server.stop() + } + }) + + it('deduplicates Codex SessionStart while retaining replay until working resumes', async () => { + const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + await server.start() + try { + const { port, token } = server.getCoordinates() + const post = (payload: Record): Promise => + fetch(`http://127.0.0.1:${port}/hook/codex`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ paneKey: PANE_KEY, tabId: 'tab-1', payload }) + }) + + await post({ hook_event_name: 'UserPromptSubmit', prompt: 'remote old session' }) + forward.mockClear() + await post({ hook_event_name: 'SessionStart', session_id: 'relay-new-session' }) + await post({ hook_event_name: 'SessionStart', session_id: 'relay-new-session' }) + + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + source: 'codex', + paneKey: PANE_KEY, + hookEventName: 'SessionStart', + providerSession: { key: 'session_id', id: 'relay-new-session' }, + payload: { state: 'working', prompt: '', agentType: 'codex' } + }) + expect(forward.mock.calls[0][0].payload).not.toMatchObject({ state: 'done' }) + + forward.mockClear() + expect(server.replayCachedPayloadsForPanes()).toBe(1) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + source: 'codex', + paneKey: PANE_KEY, + hookEventName: 'SessionStart', + isReplay: true, + payload: { state: 'working', prompt: '', agentType: 'codex' } + }) + expect(forward.mock.calls[0][0].payload).not.toMatchObject({ state: 'done' }) + + forward.mockClear() + await post({ hook_event_name: 'UserPromptSubmit', prompt: 'new session working' }) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + source: 'codex', + paneKey: PANE_KEY, + hookEventName: 'UserPromptSubmit', + providerSession: { key: 'session_id', id: 'relay-new-session' }, + payload: { state: 'working', prompt: 'new session working', agentType: 'codex' } + }) + + forward.mockClear() + expect(server.replayCachedPayloadsForPanes()).toBe(1) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + hookEventName: 'UserPromptSubmit', + isReplay: true, + payload: { state: 'working', prompt: 'new session working', agentType: 'codex' } + }) + } finally { + server.stop() + } + }) + it('forwards and replays Pi session identity as metadata-only', async () => { const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() const server = new RelayAgentHookServer({ endpointDir: dir, forward }) diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 378f81cf0229..728917ae0dd7 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -40,7 +40,12 @@ import { type SpoolRecord } from '../shared/agent-hook-spool' import { buildRelayHookPtyEnv, defaultEndpointDir } from './agent-hook-endpoint-coordinates' -import { buildRelayHookEnvelope, hookBodyEnv, hookBodyVersion } from './agent-hook-envelope-build' +import { + buildRelayHookEnvelope, + hookBodyEnv, + hookBodyPaneKey, + hookBodyVersion +} from './agent-hook-envelope-build' import { AgentHookResultRetryScheduler } from './agent-hook-result-retry-scheduler' import { MAX_CACHED_PANES, selectReplayableCachedPanes } from './agent-hook-cached-pane-status' @@ -275,6 +280,8 @@ export class RelayAgentHookServer { } const body = await readRequestBody(req) const hookBody = mergeAgentHookRequestHeaders(body, req.headers) + const paneKey = hookBodyPaneKey(hookBody) + const previousStatus = paneKey ? this.state.lastStatusByPaneKey.get(paneKey) : undefined const event = normalizeHookPayload(this.state, source, hookBody, this.env, { deferCompactOwnershipToClient: true }) @@ -285,6 +292,17 @@ export class RelayAgentHookServer { this.applyEvent(event, source, env, version) this.retryScheduler.scheduleAssistantMessageRetry(source, hookBody, event, env, version) this.retryScheduler.scheduleCodexSubagentPoll(source, hookBody, event, env, version) + } else if ( + source === 'codex' && + previousStatus && + paneKey && + !this.state.lastStatusByPaneKey.has(paneKey) + ) { + this.forwardSessionStartClear( + previousStatus, + hookBodyEnv(hookBody), + hookBodyVersion(hookBody) + ) } res.writeHead(204) res.end() @@ -303,6 +321,41 @@ export class RelayAgentHookServer { } } + private forwardSessionStartClear( + previous: AgentHookEventPayload, + env?: string, + version?: string + ): void { + if (previous.hookEventName === 'SessionStart') { + // Why: normalization removes the prior Codex status before returning null; + // restore its tombstone so duplicate hooks neither rebroadcast nor erase replay. + this.state.lastStatusByPaneKey.set(previous.paneKey, previous) + return + } + this.retryScheduler.clearAssistantMessageRetry(previous.paneKey) + const providerSession = this.state.lastProviderSessionByPaneKey.get(previous.paneKey) + // Why: do not publish state:done. Current completion-reactive consumers treat + // plain done as a finished turn unless sessionBoundary is set + // (agent-completion-hook-observer, automation-dispatch-completion, + // agent-completion-time). sessionBoundary is Rule 1 optional — old mains + // ignore it and still complete. New main clears on hookEventName SessionStart + // and never applies this payload. Old main applies it as working, which is + // today's SessionStart mapping, not a new completed-turn signal. + // Compatibility limit: old main cannot receive a SessionStart clear without + // applying some status; we refuse a false completion over a stale working row. + this.applyEvent( + { + ...previous, + hookEventName: 'SessionStart', + ...(providerSession ? { providerSession } : {}), + payload: { state: 'working', prompt: '', agentType: 'codex' } + }, + 'codex', + env, + version + ) + } + private applyEvent( event: AgentHookEventPayload, source: AgentHookSource, diff --git a/src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.test.ts b/src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.test.ts index 28a9cca5bf7f..d55d482b0b24 100644 --- a/src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.test.ts +++ b/src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.test.ts @@ -256,4 +256,26 @@ describe('Codex auto-approval status suppression', () => { }) ).toBe(false) }) + + it('suppresses Codex native Action Required titles when launch is yolo', () => { + registerCodexLaunchConfig({ + agentArgs: YOLO_TUI_AGENT_ARGS.codex ?? '', + launchToken + }) + + expect( + shouldSuppressCodexAutoApprovalSyntheticTitle('[ ! ] Action Required | my-project', { + paneKey, + tabId: 'tab-1', + launchToken + }) + ).toBe(true) + expect( + shouldSuppressCodexAutoApprovalSyntheticTitle('[ . ] Action Required | my-project', { + paneKey, + tabId: 'tab-1', + launchToken + }) + ).toBe(true) + }) }) diff --git a/src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.ts b/src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.ts index ffd2ee6ee185..68b70ad6c1a7 100644 --- a/src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.ts +++ b/src/renderer/src/components/terminal-pane/codex-auto-approval-notification-suppression.ts @@ -1,4 +1,5 @@ import { isAskUserQuestionTool } from '../../../../shared/agent-question-answered-intent' +import { isCodexNativeActionRequiredTitle } from '../../../../shared/agent-detection' import type { AgentProviderSessionMetadata } from '../../../../shared/agent-session-resume' import { getSyntheticAgentTitleProfile } from '../../../../shared/synthetic-agent-title' import { resolveTuiAgentPermissionMode } from '../../../../shared/tui-agent-permissions' @@ -65,7 +66,10 @@ export function shouldSuppressCodexAutoApprovalSyntheticTitle( title: string, context: CodexAutoApprovalStatusContext ): boolean { - if (title !== getSyntheticAgentTitleProfile('codex')?.permissionLabel) { + const isPermissionTitle = + title === getSyntheticAgentTitleProfile('codex')?.permissionLabel || + isCodexNativeActionRequiredTitle(title) + if (!isPermissionTitle) { return false } diff --git a/src/renderer/src/lib/agent-status.test.ts b/src/renderer/src/lib/agent-status.test.ts index 8bd1314d5774..4fd6102abc4d 100644 --- a/src/renderer/src/lib/agent-status.test.ts +++ b/src/renderer/src/lib/agent-status.test.ts @@ -84,6 +84,13 @@ describe('detectAgentStatusFromTitle', () => { expect(detectAgentStatusFromTitle('Claude Code - action required')).toBe('permission') }) + it('detects Codex native Action Required titles without a codex token', () => { + expect(detectAgentStatusFromTitle('[ ! ] Action Required | my-project')).toBe('permission') + expect(detectAgentStatusFromTitle('[ . ] Action Required | my-project')).toBe('permission') + expect(detectAgentStatusFromTitle('[!] Action Required | project')).toBe('permission') + expect(detectAgentStatusFromTitle('Action Required | project')).toBeNull() + }) + it('detects "permission" keyword with agent name', () => { expect(detectAgentStatusFromTitle('codex - permission needed')).toBe('permission') }) diff --git a/src/shared/agent-detection.ts b/src/shared/agent-detection.ts index 297bac12d06a..6e3748b5dd4d 100644 --- a/src/shared/agent-detection.ts +++ b/src/shared/agent-detection.ts @@ -22,9 +22,11 @@ export { export { isOpenCodeNativeTitle, isMeaningfulOpenCodeTerminalTitle } from './opencode-terminal-title' export { getAgentLabel, isClaudeAgent } from './agent-title-identity' export { + CODEX_NATIVE_ACTION_REQUIRED_TITLE_RE, clearWorkingIndicators, createAgentStatusTracker, detectAgentStatusFromTitle, + isCodexNativeActionRequiredTitle, isQuarterCircleSpinnerOnlyAgentTitle, normalizeTerminalTitle } from './agent-title-status' diff --git a/src/shared/agent-hook-listener-codex-session-start.test.ts b/src/shared/agent-hook-listener-codex-session-start.test.ts new file mode 100644 index 000000000000..af84f063a9e8 --- /dev/null +++ b/src/shared/agent-hook-listener-codex-session-start.test.ts @@ -0,0 +1,321 @@ +import { describe, expect, it } from 'vitest' +import { normalizeHookPayload } from './agent-hook-listener' +import { createHookListenerState } from './agent-hook-listener/listener-state' +import { makePaneKey } from './stable-pane-id' +import { PANE_KEY } from './agent-hook-listener-test-harness' + +describe('Codex SessionStart listener obligations', () => { + it('keeps Codex SessionStart metadata without treating it as working', () => { + const state = createHookListenerState() + const started = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'SessionStart', + source: 'startup', + session_id: 'codex-session-start' + } + }, + 'production' + ) + const prompted = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'ship the fix' } + }, + 'production' + ) + + expect(started).toBeNull() + expect(prompted?.payload).toMatchObject({ state: 'working', prompt: 'ship the fix' }) + expect(prompted?.providerSession).toEqual({ key: 'session_id', id: 'codex-session-start' }) + }) + + it('clears only the same-pane cached Codex status on SessionStart', () => { + const state = createHookListenerState() + const otherPane = makePaneKey('tab-2', '22222222-2222-4222-8222-222222222222') + const current = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'old session' } + }, + 'production' + ) + const other = normalizeHookPayload( + state, + 'codex', + { + paneKey: otherPane, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'other pane' } + }, + 'production' + ) + if (!current || !other) { + throw new Error('expected working status fixtures') + } + state.lastStatusByPaneKey.set(PANE_KEY, current) + state.lastStatusByPaneKey.set(otherPane, other) + + const started = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'SessionStart', session_id: 'new-session' } + }, + 'production' + ) + + expect(started).toBeNull() + expect(state.lastStatusByPaneKey.has(PANE_KEY)).toBe(false) + expect(state.lastStatusByPaneKey.get(otherPane)).toBe(other) + }) + + it('does not clear a different agent status on Codex SessionStart', () => { + const state = createHookListenerState() + const claude = normalizeHookPayload( + state, + 'claude', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'parent session' } + }, + 'production' + ) + if (!claude) { + throw new Error('expected Claude working status fixture') + } + state.lastStatusByPaneKey.set(PANE_KEY, claude) + + normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'SessionStart', session_id: 'nested-codex' } + }, + 'production' + ) + + expect(state.lastStatusByPaneKey.get(PANE_KEY)).toBe(claude) + }) + + it('does not let a child hook replace the root SessionStart session id', () => { + const state = createHookListenerState() + normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'SessionStart', session_id: 'root-session' } + }, + 'production' + ) + const child = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'SubagentStart', + session_id: 'child-session', + agent_id: 'child-session', + agent_type: 'explorer' + } + }, + 'production' + ) + const prompted = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'continue' } + }, + 'production' + ) + + expect(child?.providerSession).toBeUndefined() + expect(prompted?.providerSession).toEqual({ key: 'session_id', id: 'root-session' }) + }) + + it('keeps Codex subagent events working and only root Stop done', () => { + const state = createHookListenerState() + normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'review this PR' } + }, + 'production' + ) + const subStart = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'SubagentStart', + agent_id: 'agent-1', + agent_type: 'explorer' + } + }, + 'production' + ) + const subStop = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'SubagentStop', + agent_id: 'agent-1', + agent_type: 'explorer', + last_assistant_message: 'Found 3 call sites' + } + }, + 'production' + ) + const stop = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'PR review complete' + } + }, + 'production' + ) + + expect(subStart?.payload).toMatchObject({ + state: 'working', + agentType: 'codex', + toolName: 'explorer', + prompt: 'review this PR' + }) + expect(subStart?.toolAgentId).toBe('agent-1') + expect(subStop?.payload).toMatchObject({ + state: 'working', + lastAssistantMessage: 'Found 3 call sites', + prompt: 'review this PR' + }) + expect(stop?.payload).toMatchObject({ + state: 'done', + lastAssistantMessage: 'PR review complete' + }) + }) + + it('promotes Codex PostToolUse tool_response into lastAssistantMessage', () => { + const state = createHookListenerState() + const event = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'PostToolUse', + tool_name: 'Bash', + tool_input: { command: 'ls' }, + tool_response: 'README.md\nsrc\n' + } + }, + 'production' + ) + + expect(event?.payload).toMatchObject({ + state: 'working', + toolName: 'Bash', + toolInput: 'ls', + lastAssistantMessage: 'README.md\nsrc' + }) + }) + + it('previews Codex Bash cmd, apply_patch command, and spawn_agent prompt inputs', () => { + const state = createHookListenerState() + const bash = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'PreToolUse', + tool_name: 'Bash', + tool_input: { cmd: 'pwd' } + } + }, + 'production' + ) + const patch = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'PreToolUse', + tool_name: 'apply_patch', + tool_input: { command: '*** Begin Patch\n*** Update File: a.ts\n*** End Patch' } + } + }, + 'production' + ) + const spawn = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'PreToolUse', + tool_name: 'spawn_agent', + tool_input: { agent_type: 'explorer', prompt: 'find auth handlers' } + } + }, + 'production' + ) + + expect(bash?.payload.toolInput).toBe('pwd') + expect(patch?.payload.toolInput).toContain('Begin Patch') + expect(spawn?.payload.toolInput).toBe('find auth handlers') + }) + + it('clears sticky Codex working when a tool event carries an interrupt marker', () => { + const state = createHookListenerState() + normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'run tests' } + }, + 'production' + ) + const interrupted = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'PostToolUse', + tool_name: 'Bash', + tool_input: { command: 'pnpm test' }, + is_interrupt: true + } + }, + 'production' + ) + + expect(interrupted?.payload).toMatchObject({ + state: 'done', + interrupted: true, + prompt: 'run tests' + }) + }) +}) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 7df46e74cbc0..2bb7d9960d2d 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -1,6 +1,9 @@ import { normalizeAgentStatusPayload } from './agent-status-types' import type { AgentHookSource } from './agent-hook-relay' -import { extractAgentProviderSession } from './agent-session-resume' +import { + extractAgentProviderSession, + type AgentProviderSessionMetadata +} from './agent-session-resume' import { canAcceptClaudeCompactCompletion, isClaudeCompactCompletionConsumed, @@ -39,11 +42,13 @@ export function normalizeHookPayload( readFirstString(record, ['hook_event_name', 'hookEventName', 'hook_type', 'hookType']) ?? hookPayloadRecord.hook_event_name ?? hookPayloadRecord.hookEventName - // Codex child hooks expose the child's session_id on the parent's pane. + // Why: Codex child hooks expose the child's session_id on the parent's pane; + // treating it as the root resume id would replace the terminal's real session. + // SessionStart may cache the root session id before any visible status event. const providerSession = source === 'codex' && readString(hookPayloadRecord, 'agent_id') ? null - : extractAgentProviderSession(source, hookPayloadRecord) + : (resolveHookProviderSession(state, source, paneKey, hookPayloadRecord) ?? null) const providerPromptId = source === 'claude' ? normalizeClaudePromptId(hookPayloadRecord.prompt_id) @@ -181,3 +186,20 @@ export function normalizeHookPayload( payload: transportPayload } } + +function resolveHookProviderSession( + state: HookListenerState, + source: AgentHookSource, + paneKey: string, + hookPayload: Record +): AgentProviderSessionMetadata | undefined { + const extracted = extractAgentProviderSession(source, hookPayload) ?? undefined + if (source !== 'codex') { + return extracted + } + if (extracted) { + state.lastProviderSessionByPaneKey.set(paneKey, extracted) + return extracted + } + return state.lastProviderSessionByPaneKey.get(paneKey) +} diff --git a/src/shared/agent-hook-listener/listener-state.ts b/src/shared/agent-hook-listener/listener-state.ts index d4daeaa04b20..5610ca5d10df 100644 --- a/src/shared/agent-hook-listener/listener-state.ts +++ b/src/shared/agent-hook-listener/listener-state.ts @@ -7,6 +7,7 @@ import { type AgentStatusLegacyAdmissionMode } from '../agent-status-legacy-adapter' import type { AgentStatusLegacyIngressCaller } from '../agent-status-legacy-ingress-manifest' +import type { AgentProviderSessionMetadata } from '../agent-session-resume' import type { ClaudeSubagentRoster } from '../claude-subagent-roster' import type { CodexSubagentRoster } from '../codex-subagent-roster' import type { CodexSubagentTranscriptState } from '../codex-subagent-transcript' @@ -18,6 +19,9 @@ export type HookListenerState = { warnedEnvs: Set lastPromptByPaneKey: Map lastToolByPaneKey: Map + /** Provider session identity can arrive on a metadata-only SessionStart before + * the first visible status event. Keep it pane-scoped until that event. */ + lastProviderSessionByPaneKey: Map /** Read-only compatibility view. All writes pass through the isolated legacy adapter. */ lastStatusByPaneKey: ReadonlyMap antigravityCompletedTranscriptByPaneKey: Map @@ -90,6 +94,7 @@ export function createHookListenerState( warnedEnvs: new Set(), lastPromptByPaneKey: new Map(), lastToolByPaneKey: new Map(), + lastProviderSessionByPaneKey: new Map(), lastStatusByPaneKey: adapter.view, antigravityCompletedTranscriptByPaneKey: new Map(), ampCompletedCacheKeys: new Set(), @@ -171,6 +176,7 @@ export function seedLegacyAgentStatusForTests( export function clearPaneCacheState(state: HookListenerState, paneKey: string): void { deletePaneScopedCacheEntry(state.lastPromptByPaneKey, paneKey) deletePaneScopedCacheEntry(state.lastToolByPaneKey, paneKey) + deletePaneScopedCacheEntry(state.lastProviderSessionByPaneKey, paneKey) deleteLegacyAgentStatus(state, paneKey) for (const key of state.lastStatusByPaneKey.keys()) { if (key.startsWith(`${paneKey}\0`)) { @@ -250,6 +256,7 @@ export function movePaneCacheState( } movePaneScopedMapEntries(state.lastPromptByPaneKey, fromPaneKey, toPaneKey) movePaneScopedMapEntries(state.lastToolByPaneKey, fromPaneKey, toPaneKey) + movePaneScopedMapEntries(state.lastProviderSessionByPaneKey, fromPaneKey, toPaneKey) moveLegacyAgentStatuses(state, fromPaneKey, toPaneKey) movePaneScopedMapEntries(state.antigravityCompletedTranscriptByPaneKey, fromPaneKey, toPaneKey) movePaneScopedSetEntries(state.ampCompletedCacheKeys, fromPaneKey, toPaneKey) @@ -297,6 +304,7 @@ export function deletePaneScopedSetEntry(set: Set, paneKey: string): voi export function clearAllListenerCaches(state: HookListenerState): void { state.lastPromptByPaneKey.clear() state.lastToolByPaneKey.clear() + state.lastProviderSessionByPaneKey.clear() clearLegacyAgentStatuses(state) state.antigravityCompletedTranscriptByPaneKey.clear() state.ampCompletedCacheKeys.clear() diff --git a/src/shared/agent-hook-listener/providers/codex-events.ts b/src/shared/agent-hook-listener/providers/codex-events.ts index ad2ce3db92f2..b9e902caf154 100644 --- a/src/shared/agent-hook-listener/providers/codex-events.ts +++ b/src/shared/agent-hook-listener/providers/codex-events.ts @@ -5,6 +5,7 @@ import { } from '../../agent-status-types' import { normalizeOptionalField } from '../../agent-status-field-normalization' import { isAskUserQuestionTool } from '../../agent-question-answered-intent' +import { extractAgentProviderSession } from '../../agent-session-resume' import { codexRosterEffectiveState, codexRosterToSnapshots, @@ -17,8 +18,11 @@ import { reconcileCodexSubagentReviewer } from '../../codex-subagent-reviewer' import { readFirstString } from '../interactive-tool' -import type { HookListenerState } from '../listener-state' -import { resolvePrompt, resolveToolState } from '../prompt-fields' +import { + clearPaneTurnCacheState, + deleteLegacyAgentStatus, + type HookListenerState +} from '../listener-state' import { extractToolFields, isNewTurnEvent } from '../provider-event-routing' import { readString } from '../tool-input-preview' import { @@ -86,19 +90,35 @@ export function normalizeCodexSubagentLifecycleEvent( return null } const roster = getOrCreateCodexSubagentRoster(state, paneKey) + const agentType = readString(hookPayload, 'agent_type') if (eventName === 'SubagentStart') { upsertCodexSubagent( roster, agentId, { - agentType: readString(hookPayload, 'agent_type'), + agentType, model: readString(hookPayload, 'model'), state: 'working' }, Date.now() ) + // Why: child start has no tool_name; show the agent type in the status tool row. + if (agentType) { + resolveToolState( + state, + paneKey, + { toolName: agentType, hasToolUpdate: true }, + { + resetOnNewTurn: false + } + ) + } } else { finishCodexSubagent(roster, agentId) + const message = readString(hookPayload, 'last_assistant_message') + if (message) { + resolveToolState(state, paneKey, { lastAssistantMessage: message }, { resetOnNewTurn: false }) + } } return buildCodexChildDrivenStatusPayload(state, eventName, paneKey, hookPayload) } @@ -143,12 +163,33 @@ export function normalizeCodexEvent( return normalizeCodexSubagentLifecycleEvent(state, eventName, paneKey, hookPayload) } - // Why: Codex's request_user_input (0.145+) is auto-allowed, so it fires PreToolUse while blocked on a human answer; map to waiting like grok's ask_user_question. + if (eventName === 'SessionStart') { + // Why: Codex fires SessionStart when opening or resuming an idle TUI, before + // a user prompt exists; reset stale turn/session cache without emitting state. + // Cache provider session identity for the next real status event. + clearPaneTurnCacheState(state, paneKey) + state.codexSubagentRosterByPaneKey.delete(paneKey) + state.codexSubagentTranscriptByPaneKey.delete(paneKey) + state.codexLeadStateByPaneKey.delete(paneKey) + // Why: retain session id from SessionStart for the subsequent working event. + const extracted = extractAgentProviderSession('codex', hookPayload) + if (extracted) { + state.lastProviderSessionByPaneKey.set(paneKey, extracted) + } else { + state.lastProviderSessionByPaneKey.delete(paneKey) + } + if (state.lastStatusByPaneKey.get(paneKey)?.payload.agentType === 'codex') { + deleteLegacyAgentStatus(state, paneKey) + } + return null + } + + // Why: Codex's request_user_input (0.145+) is auto-allowed, so it fires PreToolUse while + // blocked on a human answer; map to waiting like generic ask-user-question tools. const isUserInputPreTool = eventName === 'PreToolUse' && isAskUserQuestionTool(readString(hookPayload, 'tool_name') ?? readString(hookPayload, 'name')) - const stateName = - eventName === 'SessionStart' || + let stateName: 'working' | 'waiting' | 'done' | null = eventName === 'UserPromptSubmit' || (eventName === 'PreToolUse' && !isUserInputPreTool) || eventName === 'PostToolUse' @@ -162,6 +203,14 @@ export function normalizeCodexEvent( return null } + if ( + stateName === 'working' && + eventName !== 'UserPromptSubmit' && + codexPayloadIsInterrupted(hookPayload) + ) { + stateName = 'done' + } + const agentId = readString(hookPayload, 'agent_id') const transcriptPath = readFirstString(hookPayload, ['transcript_path', 'transcriptPath']) if (eventName === 'SessionStart' && !agentId) { @@ -227,15 +276,32 @@ export function normalizeCodexEvent( state.codexLeadStateByPaneKey.set(paneKey, { state: ownedState, model: - normalizeOptionalField(hookPayload['model'], AGENT_MODEL_MAX_LENGTH) ?? - (eventName === 'SessionStart' ? undefined : previousLead?.model) + normalizeOptionalField(hookPayload['model'], AGENT_MODEL_MAX_LENGTH) ?? previousLead?.model }) const effectiveState = codexRosterEffectiveState( state.codexSubagentRosterByPaneKey.get(paneKey), ownedState ) - return buildCodexStatusPayload(state, eventName, promptText, paneKey, hookPayload, { + const payload = buildCodexStatusPayload(state, eventName, promptText, paneKey, hookPayload, { stateName: effectiveState, updateLead: true }) + if (!payload) { + return null + } + const interrupted = + stateName === 'done' && codexPayloadIsInterrupted(hookPayload) ? true : undefined + return interrupted ? { ...payload, interrupted } : payload +} + +function codexPayloadIsInterrupted(hookPayload: Record): boolean { + if (hookPayload['is_interrupt'] === true || hookPayload['interrupted'] === true) { + return true + } + const stopReason = readFirstString(hookPayload, ['stop_reason', 'stopReason'])?.toLowerCase() + return ( + stopReason?.includes('interrupt') === true || + stopReason?.includes('abort') === true || + stopReason?.includes('cancel') === true + ) } diff --git a/src/shared/agent-hook-listener/providers/codex-tool-fields.ts b/src/shared/agent-hook-listener/providers/codex-tool-fields.ts index 09b87c794a50..0d8dfaf53000 100644 --- a/src/shared/agent-hook-listener/providers/codex-tool-fields.ts +++ b/src/shared/agent-hook-listener/providers/codex-tool-fields.ts @@ -5,12 +5,13 @@ import { readString, toolUpdate } from '../tool-input-preview' -import { deriveInteractivePrompt } from '../interactive-tool' +import { deriveInteractivePrompt, extractToolResponseText } from '../interactive-tool' export function extractCodexToolFields( eventName: unknown, hookPayload: Record ): ToolSnapshot { + const update: ToolSnapshot = {} if ( eventName === 'PreToolUse' || eventName === 'PermissionRequest' || @@ -22,20 +23,30 @@ export function extractCodexToolFields( deriveToolInputPreview(toolName, hookPayload.tool_input) ?? deriveToolInputPreview(toolName, hookPayload.input) ?? deriveToolInputPreview(toolName, hookPayload.arguments) - return toolUpdate( - { - toolName, - toolInput, - interactivePrompt: deriveInteractivePrompt(toolName, rawInput, eventName) - }, - { hasToolInputField: hasAnyOwnField(hookPayload, ['tool_input', 'input', 'arguments']) } + Object.assign( + update, + toolUpdate( + { + toolName, + toolInput, + interactivePrompt: deriveInteractivePrompt(toolName, rawInput, eventName) + }, + { hasToolInputField: hasAnyOwnField(hookPayload, ['tool_input', 'input', 'arguments']) } + ) ) } - if (eventName === 'Stop') { + if (eventName === 'PostToolUse') { + // Why: Codex posts the tool result on PostToolUse; surface it as the status row summary. + const responseText = extractToolResponseText(hookPayload.tool_response) + if (responseText) { + update.lastAssistantMessage = responseText + } + } + if (eventName === 'Stop' || eventName === 'SubagentStop') { const message = readString(hookPayload, 'last_assistant_message') if (message) { - return { lastAssistantMessage: message } + update.lastAssistantMessage = message } } - return {} + return update } diff --git a/src/shared/agent-hook-listener/tool-input-preview.ts b/src/shared/agent-hook-listener/tool-input-preview.ts index 7499763b63dd..5b56726f0857 100644 --- a/src/shared/agent-hook-listener/tool-input-preview.ts +++ b/src/shared/agent-hook-listener/tool-input-preview.ts @@ -8,7 +8,7 @@ const TOOL_INPUT_KEYS_BY_TOOL: Record = { Execute: ['command'], MultiEdit: ['file_path', 'filePath', 'path'], NotebookEdit: ['file_path', 'filePath', 'path'], - Bash: ['command'], + Bash: ['command', 'cmd'], Glob: ['pattern'], Grep: ['pattern'], WebFetch: ['url'], @@ -33,13 +33,14 @@ const TOOL_INPUT_KEYS_BY_TOOL: Record = { search_replace: ['file_path', 'path', 'filePath'], write_to_file: ['TargetFile', 'path', 'file_path'], execute_code: ['code', 'command', 'cmd'], - apply_patch: ['path', 'file_path'], + apply_patch: ['command', 'path', 'file_path'], + spawn_agent: ['prompt', 'description', 'agent_type'], view_image: ['path', 'file_path'], AskUser: ['question', 'prompt', 'message'], ask_user: ['question', 'prompt', 'message'], AskUserQuestion: ['questions', 'question', 'prompt', 'message'], ask_user_question: ['questions', 'question', 'prompt', 'message'], - bash: ['command'], + bash: ['command', 'cmd'], powershell: ['command'], create: ['path', 'file_path'], read: ['path', 'file_path'], diff --git a/src/shared/agent-title-status.ts b/src/shared/agent-title-status.ts index a74ae15d6bf0..e200a3969ddf 100644 --- a/src/shared/agent-title-status.ts +++ b/src/shared/agent-title-status.ts @@ -34,6 +34,14 @@ import { getWrapperTitleSegments } from './terminal-title-wrapper-segments' import { isGrokRotatingWorkingTitle } from './terminal-title-agent-type' import { memoizeTitleClassification } from './terminal-title-classification-memo' +// Why: Codex permission titles omit the agent name, so the fixed native prefix +// is the only reliable title-only signal for its waiting state. +export const CODEX_NATIVE_ACTION_REQUIRED_TITLE_RE = /^\[\s*[!.]\s*\]\s*Action Required\b/i + +export function isCodexNativeActionRequiredTitle(title: string): boolean { + return CODEX_NATIVE_ACTION_REQUIRED_TITLE_RE.test(title) +} + /** * Strip working-status indicators so stale exit titles stop reporting working. */ @@ -198,6 +206,10 @@ function computeAgentStatusFromTitle(title: string): AgentStatus | null { return piStateStatus } + if (isCodexNativeActionRequiredTitle(title)) { + return 'permission' + } + if (title.includes(GEMINI_PERMISSION)) { return 'permission' } From 97a083b74aac96d8a10942d292915439fc84bab6 Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Wed, 9 Sep 2026 02:20:25 +0800 Subject: [PATCH 2/7] fix(codex): split SessionStart relay apply to satisfy typecheck and lint Keep the working tombstone semantics, but move apply/clear helpers and SessionStart tests out of the over-budget relay files and supply the required completion stateHistory field. Co-authored-by: Cursor --- ...codex-session-start-wire-semantics.test.ts | 3 +- .../codex-hook-remote-user-trust-moves.ts | 2 +- src/relay/agent-hook-relay-event-apply.ts | 109 +++++++++++++++ .../agent-hook-server-session-start.test.ts | 126 ++++++++++++++++++ src/relay/agent-hook-server.test.ts | 107 --------------- src/relay/agent-hook-server.ts | 110 ++++----------- 6 files changed, 264 insertions(+), 193 deletions(-) create mode 100644 src/relay/agent-hook-relay-event-apply.ts create mode 100644 src/relay/agent-hook-server-session-start.test.ts diff --git a/src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts b/src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts index 4440b413615e..8e86c50f19a4 100644 --- a/src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts +++ b/src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts @@ -38,7 +38,8 @@ function completionEntry(overrides: { state: overrides.state, updatedAt: overrides.updatedAt ?? 2_000, stateStartedAt: overrides.stateStartedAt ?? 1_500, - sessionBoundary: overrides.sessionBoundary + sessionBoundary: overrides.sessionBoundary, + stateHistory: [] } } diff --git a/src/main/codex/codex-hook-remote-user-trust-moves.ts b/src/main/codex/codex-hook-remote-user-trust-moves.ts index aac9301a9eed..a1bca0be12f6 100644 --- a/src/main/codex/codex-hook-remote-user-trust-moves.ts +++ b/src/main/codex/codex-hook-remote-user-trust-moves.ts @@ -1,6 +1,6 @@ import type { HookDefinition } from '../agent-hooks/installer-utils' import { createCodexHookTrustEntry, getCodexHookTrustSignature } from './codex-hook-identity' -import { CODEX_EVENTS } from './codex-hook-definition' +import type { CODEX_EVENTS } from './codex-hook-definition' import { computeTrustKey, type CodexHookTrustKeyMove, diff --git a/src/relay/agent-hook-relay-event-apply.ts b/src/relay/agent-hook-relay-event-apply.ts new file mode 100644 index 000000000000..89686644878c --- /dev/null +++ b/src/relay/agent-hook-relay-event-apply.ts @@ -0,0 +1,109 @@ +import type { AgentHookEventPayload } from '../shared/agent-hook-listener/listener-event' +import { normalizeHookPayload } from '../shared/agent-hook-listener' +import { + isAgentHookSource, + type AgentHookRelayEnvelope, + type AgentHookSource +} from '../shared/agent-hook-relay' +import { buildSpoolHookBody, type SpoolRecord } from '../shared/agent-hook-spool' +import type { HookListenerState } from '../shared/agent-hook-listener/listener-state' +import { buildRelayHookEnvelope, hookBodyEnv, hookBodyVersion } from './agent-hook-envelope-build' +import { MAX_CACHED_PANES } from './agent-hook-cached-pane-status' +import { cacheRelayLegacyAgentStatus } from '../shared/agent-status-legacy-relay-cache' + +export type RelayHookForward = (envelope: AgentHookRelayEnvelope) => void + +export type RelayHookEventApplyHost = { + state: HookListenerState + lastEnvelopeMetaByPaneKey: Map< + string, + { source: AgentHookSource; env?: string; version?: string } + > + isPaneSurfaceRetired: (paneKey: string) => boolean + clearPaneState: (paneKey: string) => void + clearAssistantMessageRetry: (paneKey: string) => void + forward: RelayHookForward + env: string +} + +export function applyRelayHookEvent( + host: RelayHookEventApplyHost, + event: AgentHookEventPayload, + source: AgentHookSource, + env?: string, + version?: string, + options: { isReplay?: boolean } = {} +): void { + if (host.isPaneSurfaceRetired(event.paneKey)) { + host.clearPaneState(event.paneKey) + return + } + if (event.payload.state !== 'done' || event.payload.lastAssistantMessage) { + host.clearAssistantMessageRetry(event.paneKey) + } + if ( + !cacheRelayLegacyAgentStatus(host.state, event, MAX_CACHED_PANES, (paneKey) => + host.clearPaneState(paneKey) + ) + ) { + return + } + host.lastEnvelopeMetaByPaneKey.delete(event.paneKey) + host.lastEnvelopeMetaByPaneKey.set(event.paneKey, { source, env, version }) + host.forward(buildRelayHookEnvelope(event, source, env, version, options)) +} + +export function ingestRelaySpoolRecord(host: RelayHookEventApplyHost, record: SpoolRecord): void { + if (!isAgentHookSource(record.source)) { + return + } + const body = buildSpoolHookBody(record) + const event = normalizeHookPayload(host.state, record.source, body, host.env, { + deferCompactOwnershipToClient: true + }) + if (!event) { + return + } + applyRelayHookEvent(host, event, record.source, hookBodyEnv(body), hookBodyVersion(body), { + isReplay: true + }) +} + +export function forwardSessionStartClear( + host: RelayHookEventApplyHost, + previous: AgentHookEventPayload, + env?: string, + version?: string +): void { + if (previous.hookEventName === 'SessionStart') { + // Why: normalization removes the prior Codex status before returning null; + // restore its tombstone so duplicate hooks neither rebroadcast nor erase replay. + cacheRelayLegacyAgentStatus(host.state, previous, MAX_CACHED_PANES, (paneKey) => + host.clearPaneState(paneKey) + ) + return + } + host.clearAssistantMessageRetry(previous.paneKey) + const providerSession = host.state.lastProviderSessionByPaneKey.get(previous.paneKey) + // Why: do not publish state:done. Current completion-reactive consumers treat + // plain done as a finished turn unless sessionBoundary is set + // (agent-completion-hook-observer, automation-dispatch-completion, + // agent-completion-time). sessionBoundary is Rule 1 optional — old mains + // ignore it and still complete. New main clears on hookEventName SessionStart + // and never applies this payload. Old main applies it as working, which is + // today's SessionStart mapping, not a new completed-turn signal. + // Compatibility limit: old main cannot receive a SessionStart clear without + // applying some status; we refuse a false completion over a stale working row. + applyRelayHookEvent( + host, + { + ...previous, + hookEventName: 'SessionStart', + ...(providerSession ? { providerSession } : {}), + payload: { state: 'working', prompt: '', agentType: 'codex' } + }, + 'codex', + env, + version + ) +} diff --git a/src/relay/agent-hook-server-session-start.test.ts b/src/relay/agent-hook-server-session-start.test.ts new file mode 100644 index 000000000000..38a56de15608 --- /dev/null +++ b/src/relay/agent-hook-server-session-start.test.ts @@ -0,0 +1,126 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { RelayAgentHookServer } from './agent-hook-server' +import type { AgentHookRelayEnvelope } from '../shared/agent-hook-relay' +import { makePaneKey } from '../shared/stable-pane-id' + +const PANE_KEY = makePaneKey('tab-1', '11111111-1111-4111-8111-111111111111') + +describe('RelayAgentHookServer Codex SessionStart', () => { + let dir: string + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'relay-hook-session-start-')) + }) + afterEach(() => { + rmSync(dir, { recursive: true, force: true }) + }) + + it('forwards Codex SessionStart identity with the next real status event', async () => { + const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + await server.start() + try { + const { port, token } = server.getCoordinates() + const post = (payload: Record): Promise => + fetch(`http://127.0.0.1:${port}/hook/codex`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ paneKey: PANE_KEY, tabId: 'tab-1', payload }) + }) + + expect( + ( + await post({ + hook_event_name: 'SessionStart', + session_id: 'codex-relay-session' + }) + ).status + ).toBe(204) + expect(forward).not.toHaveBeenCalled() + + expect( + (await post({ hook_event_name: 'UserPromptSubmit', prompt: 'relay this status' })).status + ).toBe(204) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + source: 'codex', + providerSession: { key: 'session_id', id: 'codex-relay-session' }, + payload: { state: 'working', prompt: 'relay this status' } + }) + } finally { + server.stop() + } + }) + + it('deduplicates Codex SessionStart while retaining replay until working resumes', async () => { + const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + await server.start() + try { + const { port, token } = server.getCoordinates() + const post = (payload: Record): Promise => + fetch(`http://127.0.0.1:${port}/hook/codex`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': token + }, + body: JSON.stringify({ paneKey: PANE_KEY, tabId: 'tab-1', payload }) + }) + + await post({ hook_event_name: 'UserPromptSubmit', prompt: 'remote old session' }) + forward.mockClear() + await post({ hook_event_name: 'SessionStart', session_id: 'relay-new-session' }) + await post({ hook_event_name: 'SessionStart', session_id: 'relay-new-session' }) + + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + source: 'codex', + paneKey: PANE_KEY, + hookEventName: 'SessionStart', + providerSession: { key: 'session_id', id: 'relay-new-session' }, + payload: { state: 'working', prompt: '', agentType: 'codex' } + }) + expect(forward.mock.calls[0][0].payload).not.toMatchObject({ state: 'done' }) + + forward.mockClear() + expect(server.replayCachedPayloadsForPanes()).toBe(1) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + source: 'codex', + paneKey: PANE_KEY, + hookEventName: 'SessionStart', + isReplay: true, + payload: { state: 'working', prompt: '', agentType: 'codex' } + }) + expect(forward.mock.calls[0][0].payload).not.toMatchObject({ state: 'done' }) + + forward.mockClear() + await post({ hook_event_name: 'UserPromptSubmit', prompt: 'new session working' }) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + source: 'codex', + paneKey: PANE_KEY, + hookEventName: 'UserPromptSubmit', + providerSession: { key: 'session_id', id: 'relay-new-session' }, + payload: { state: 'working', prompt: 'new session working', agentType: 'codex' } + }) + + forward.mockClear() + expect(server.replayCachedPayloadsForPanes()).toBe(1) + expect(forward).toHaveBeenCalledTimes(1) + expect(forward.mock.calls[0][0]).toMatchObject({ + hookEventName: 'UserPromptSubmit', + isReplay: true, + payload: { state: 'working', prompt: 'new session working', agentType: 'codex' } + }) + } finally { + server.stop() + } + }) +}) diff --git a/src/relay/agent-hook-server.test.ts b/src/relay/agent-hook-server.test.ts index 83bf71bb4e34..c3a99b8a20c1 100644 --- a/src/relay/agent-hook-server.test.ts +++ b/src/relay/agent-hook-server.test.ts @@ -385,113 +385,6 @@ describe('RelayAgentHookServer', () => { } }) - it('forwards Codex SessionStart identity with the next real status event', async () => { - const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() - const server = new RelayAgentHookServer({ endpointDir: dir, forward }) - await server.start() - try { - const { port, token } = server.getCoordinates() - const post = (payload: Record): Promise => - fetch(`http://127.0.0.1:${port}/hook/codex`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Orca-Agent-Hook-Token': token - }, - body: JSON.stringify({ paneKey: PANE_KEY, tabId: 'tab-1', payload }) - }) - - expect( - ( - await post({ - hook_event_name: 'SessionStart', - session_id: 'codex-relay-session' - }) - ).status - ).toBe(204) - expect(forward).not.toHaveBeenCalled() - - expect( - (await post({ hook_event_name: 'UserPromptSubmit', prompt: 'relay this status' })).status - ).toBe(204) - expect(forward).toHaveBeenCalledTimes(1) - expect(forward.mock.calls[0][0]).toMatchObject({ - source: 'codex', - providerSession: { key: 'session_id', id: 'codex-relay-session' }, - payload: { state: 'working', prompt: 'relay this status' } - }) - } finally { - server.stop() - } - }) - - it('deduplicates Codex SessionStart while retaining replay until working resumes', async () => { - const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() - const server = new RelayAgentHookServer({ endpointDir: dir, forward }) - await server.start() - try { - const { port, token } = server.getCoordinates() - const post = (payload: Record): Promise => - fetch(`http://127.0.0.1:${port}/hook/codex`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Orca-Agent-Hook-Token': token - }, - body: JSON.stringify({ paneKey: PANE_KEY, tabId: 'tab-1', payload }) - }) - - await post({ hook_event_name: 'UserPromptSubmit', prompt: 'remote old session' }) - forward.mockClear() - await post({ hook_event_name: 'SessionStart', session_id: 'relay-new-session' }) - await post({ hook_event_name: 'SessionStart', session_id: 'relay-new-session' }) - - expect(forward).toHaveBeenCalledTimes(1) - expect(forward.mock.calls[0][0]).toMatchObject({ - source: 'codex', - paneKey: PANE_KEY, - hookEventName: 'SessionStart', - providerSession: { key: 'session_id', id: 'relay-new-session' }, - payload: { state: 'working', prompt: '', agentType: 'codex' } - }) - expect(forward.mock.calls[0][0].payload).not.toMatchObject({ state: 'done' }) - - forward.mockClear() - expect(server.replayCachedPayloadsForPanes()).toBe(1) - expect(forward).toHaveBeenCalledTimes(1) - expect(forward.mock.calls[0][0]).toMatchObject({ - source: 'codex', - paneKey: PANE_KEY, - hookEventName: 'SessionStart', - isReplay: true, - payload: { state: 'working', prompt: '', agentType: 'codex' } - }) - expect(forward.mock.calls[0][0].payload).not.toMatchObject({ state: 'done' }) - - forward.mockClear() - await post({ hook_event_name: 'UserPromptSubmit', prompt: 'new session working' }) - expect(forward).toHaveBeenCalledTimes(1) - expect(forward.mock.calls[0][0]).toMatchObject({ - source: 'codex', - paneKey: PANE_KEY, - hookEventName: 'UserPromptSubmit', - providerSession: { key: 'session_id', id: 'relay-new-session' }, - payload: { state: 'working', prompt: 'new session working', agentType: 'codex' } - }) - - forward.mockClear() - expect(server.replayCachedPayloadsForPanes()).toBe(1) - expect(forward).toHaveBeenCalledTimes(1) - expect(forward.mock.calls[0][0]).toMatchObject({ - hookEventName: 'UserPromptSubmit', - isReplay: true, - payload: { state: 'working', prompt: 'new session working', agentType: 'codex' } - }) - } finally { - server.stop() - } - }) - it('forwards and replays Pi session identity as metadata-only', async () => { const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>() const server = new RelayAgentHookServer({ endpointDir: dir, forward }) diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 728917ae0dd7..937664e0d0f8 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -28,17 +28,8 @@ import { describeHookTransportInterference, isHookRequestTruncatedError } from '../shared/agent-hook-transport-interference' -import { - isAgentHookSource, - REMOTE_AGENT_HOOK_ENV, - type AgentHookRelayEnvelope, - type AgentHookSource -} from '../shared/agent-hook-relay' -import { - buildSpoolHookBody, - drainAgentHookSpool, - type SpoolRecord -} from '../shared/agent-hook-spool' +import { REMOTE_AGENT_HOOK_ENV, type AgentHookSource } from '../shared/agent-hook-relay' +import { drainAgentHookSpool } from '../shared/agent-hook-spool' import { buildRelayHookPtyEnv, defaultEndpointDir } from './agent-hook-endpoint-coordinates' import { buildRelayHookEnvelope, @@ -47,9 +38,16 @@ import { hookBodyVersion } from './agent-hook-envelope-build' import { AgentHookResultRetryScheduler } from './agent-hook-result-retry-scheduler' -import { MAX_CACHED_PANES, selectReplayableCachedPanes } from './agent-hook-cached-pane-status' +import { selectReplayableCachedPanes } from './agent-hook-cached-pane-status' +import { + applyRelayHookEvent, + forwardSessionStartClear, + ingestRelaySpoolRecord, + type RelayHookEventApplyHost, + type RelayHookForward +} from './agent-hook-relay-event-apply' -export type RelayHookForward = (envelope: AgentHookRelayEnvelope) => void +export type { RelayHookForward } export type RelayHookServerOptions = { /** Where to put endpoint.env / endpoint.cmd. Defaults to `$HOME/.orca-relay/agent-hooks`. */ @@ -128,7 +126,7 @@ export class RelayAgentHookServer { drainAgentHookSpool({ endpointDir: this.endpointDir, getPersistedLaunchTokenHash: () => undefined, - ingest: (record) => this.ingestSpoolRecord(record) + ingest: (record) => ingestRelaySpoolRecord(this.eventApplyHost(), record) }) } catch (err) { // Why: a downstream relay failure must not prevent the loopback listener from starting; @@ -298,7 +296,8 @@ export class RelayAgentHookServer { paneKey && !this.state.lastStatusByPaneKey.has(paneKey) ) { - this.forwardSessionStartClear( + forwardSessionStartClear( + this.eventApplyHost(), previousStatus, hookBodyEnv(hookBody), hookBodyVersion(hookBody) @@ -321,39 +320,17 @@ export class RelayAgentHookServer { } } - private forwardSessionStartClear( - previous: AgentHookEventPayload, - env?: string, - version?: string - ): void { - if (previous.hookEventName === 'SessionStart') { - // Why: normalization removes the prior Codex status before returning null; - // restore its tombstone so duplicate hooks neither rebroadcast nor erase replay. - this.state.lastStatusByPaneKey.set(previous.paneKey, previous) - return + private eventApplyHost(): RelayHookEventApplyHost { + return { + state: this.state, + lastEnvelopeMetaByPaneKey: this.lastEnvelopeMetaByPaneKey, + isPaneSurfaceRetired: this.isPaneSurfaceRetired, + clearPaneState: (paneKey) => this.clearPaneState(paneKey), + clearAssistantMessageRetry: (paneKey) => + this.retryScheduler.clearAssistantMessageRetry(paneKey), + forward: this.forward, + env: this.env } - this.retryScheduler.clearAssistantMessageRetry(previous.paneKey) - const providerSession = this.state.lastProviderSessionByPaneKey.get(previous.paneKey) - // Why: do not publish state:done. Current completion-reactive consumers treat - // plain done as a finished turn unless sessionBoundary is set - // (agent-completion-hook-observer, automation-dispatch-completion, - // agent-completion-time). sessionBoundary is Rule 1 optional — old mains - // ignore it and still complete. New main clears on hookEventName SessionStart - // and never applies this payload. Old main applies it as working, which is - // today's SessionStart mapping, not a new completed-turn signal. - // Compatibility limit: old main cannot receive a SessionStart clear without - // applying some status; we refuse a false completion over a stale working row. - this.applyEvent( - { - ...previous, - hookEventName: 'SessionStart', - ...(providerSession ? { providerSession } : {}), - payload: { state: 'working', prompt: '', agentType: 'codex' } - }, - 'codex', - env, - version - ) } private applyEvent( @@ -363,45 +340,10 @@ export class RelayAgentHookServer { version?: string, options: { isReplay?: boolean } = {} ): void { - // Why: this post came from a process still running inside a pane whose tab the user closed. - // Caching or forwarding it makes every connected client advertise a live, resumable agent pane - // that no tab owns — the advertisement that ends up auto-typing a second `--resume` onto a - // transcript the orphan is still writing (#12447). Drop the stale cache with it. - if (this.isPaneSurfaceRetired(event.paneKey)) { - this.clearPaneState(event.paneKey) - return - } - if (event.payload.state !== 'done' || event.payload.lastAssistantMessage) { - this.retryScheduler.clearAssistantMessageRetry(event.paneKey) - } - // Why: keep PostCompact identity in the replay cache so the client can re-run ownership when - // it reconnects. Stripping it would let a cold relay replay a completion as an ordinary `done` - // row and resurrect a pane that the client had already retired. - if ( - !cacheRelayLegacyAgentStatus(this.state, event, MAX_CACHED_PANES, (paneKey) => - this.clearPaneState(paneKey) - ) - ) { - return - } - this.lastEnvelopeMetaByPaneKey.delete(event.paneKey) - this.lastEnvelopeMetaByPaneKey.set(event.paneKey, { source, env, version }) - this.forward(buildRelayHookEnvelope(event, source, env, version, options)) + applyRelayHookEvent(this.eventApplyHost(), event, source, env, version, options) } private ingestSpoolRecord(record: SpoolRecord): void { - if (!isAgentHookSource(record.source)) { - return - } - const body = buildSpoolHookBody(record) - const event = normalizeHookPayload(this.state, record.source, body, this.env, { - deferCompactOwnershipToClient: true - }) - if (!event) { - return - } - this.applyEvent(event, record.source, hookBodyEnv(body), hookBodyVersion(body), { - isReplay: true - }) + ingestRelaySpoolRecord(this.eventApplyHost(), record) } } From 2766777aa8fe197f8f4bebf2fcd1ed388453bc33 Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Wed, 9 Sep 2026 02:24:10 +0800 Subject: [PATCH 3/7] fix(codex): fence SessionStart identity to accepted root owners Child SessionStart, foreign-connection SessionStart, and unrecognized hooks must not replace the root resume id. Cache writes wait until a root event is accepted, except the intentional SessionStart metadata write. Co-authored-by: Cursor --- .../server/server-ingest-remote.ts | 8 +- .../server/server-status-retries.ts | 22 ++- src/shared/agent-hook-listener.ts | 23 +-- .../providers/codex-events.ts | 8 ++ ...agent-hook-session-identity-review.test.ts | 136 ++++++++++++++++++ 5 files changed, 180 insertions(+), 17 deletions(-) create mode 100644 src/shared/agent-hook-session-identity-review.test.ts diff --git a/src/main/agent-hooks/server/server-ingest-remote.ts b/src/main/agent-hooks/server/server-ingest-remote.ts index 96d7ca39e4bc..eb25b9689a4c 100644 --- a/src/main/agent-hooks/server/server-ingest-remote.ts +++ b/src/main/agent-hooks/server/server-ingest-remote.ts @@ -261,10 +261,16 @@ export abstract class AgentHookServerIngestRemote extends AgentHookServerIngestS // consumers unless sessionBoundary is stamped, and that flag is optional // so old clients ignore it (remote-wire Rule 3). Clear the stale row and // keep session identity for the next real status event. + // Identity and clear share one owner fence: a foreign connection must not + // replace the current resume id while leaving the visible row intact. + const expectedConnectionId = trimmedConnectionId ?? undefined + if (!this.canApplyCodexSessionStart(previousStatus, expectedConnectionId)) { + return + } if (providerSession) { this.state.lastProviderSessionByPaneKey.set(paneKey, providerSession) } - this.clearStatusForSessionStart(paneKey, undefined, trimmedConnectionId ?? undefined) + this.clearStatusForSessionStart(paneKey, previousStatus, expectedConnectionId) return } const event: AgentHookEventPayload & { authorityRestartId?: string } = { diff --git a/src/main/agent-hooks/server/server-status-retries.ts b/src/main/agent-hooks/server/server-status-retries.ts index 8a81f2ccd583..38ea80b2de37 100644 --- a/src/main/agent-hooks/server/server-status-retries.ts +++ b/src/main/agent-hooks/server/server-status-retries.ts @@ -31,18 +31,28 @@ export abstract class AgentHookServerStatusRetries extends AgentHookServerStatus this.codexSubagentPollScheduler.clearAll() } + protected canApplyCodexSessionStart( + status: AgentHookEventPayload | undefined, + expectedConnectionId?: string + ): boolean { + if (!status) { + return true + } + if (status.payload.agentType !== 'codex') { + return false + } + // Why: a delayed relay notification must not clear a newer remote target's + // status if pane identity is ever reused across logical connections. + return expectedConnectionId === undefined || status.connectionId === expectedConnectionId + } + protected clearStatusForSessionStart( paneKey: string, previousStatus?: AgentHookEventPayload, expectedConnectionId?: string ): void { const status = previousStatus ?? this.state.lastStatusByPaneKey.get(paneKey) - // Why: a delayed relay notification must not clear a newer remote target's - // status if pane identity is ever reused across logical connections. - if ( - status?.payload.agentType !== 'codex' || - (expectedConnectionId !== undefined && status.connectionId !== expectedConnectionId) - ) { + if (!status || !this.canApplyCodexSessionStart(status, expectedConnectionId)) { return } this.state.lastStatusByPaneKey.delete(paneKey) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 2bb7d9960d2d..76c3538926d5 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -44,11 +44,12 @@ export function normalizeHookPayload( hookPayloadRecord.hookEventName // Why: Codex child hooks expose the child's session_id on the parent's pane; // treating it as the root resume id would replace the terminal's real session. - // SessionStart may cache the root session id before any visible status event. - const providerSession = - source === 'codex' && readString(hookPayloadRecord, 'agent_id') - ? null - : (resolveHookProviderSession(state, source, paneKey, hookPayloadRecord) ?? null) + // Peek only — cache writes wait until a root event is accepted, except the + // intentional SessionStart metadata write inside normalizeCodexEvent. + const isCodexChild = source === 'codex' && Boolean(readString(hookPayloadRecord, 'agent_id')) + const providerSession = isCodexChild + ? null + : (resolveHookProviderSession(state, source, paneKey, hookPayloadRecord) ?? null) const providerPromptId = source === 'claude' ? normalizeClaudePromptId(hookPayloadRecord.prompt_id) @@ -139,6 +140,12 @@ export function normalizeHookPayload( return null } const grokActiveTurn = source === 'grok' ? state.grokActiveTurnByPaneKey.get(paneKey) : undefined + if (source === 'codex' && !isCodexChild) { + const extracted = extractAgentProviderSession(source, hookPayloadRecord) + if (extracted) { + state.lastProviderSessionByPaneKey.set(paneKey, extracted) + } + } return { paneKey, @@ -197,9 +204,5 @@ function resolveHookProviderSession( if (source !== 'codex') { return extracted } - if (extracted) { - state.lastProviderSessionByPaneKey.set(paneKey, extracted) - return extracted - } - return state.lastProviderSessionByPaneKey.get(paneKey) + return extracted ?? state.lastProviderSessionByPaneKey.get(paneKey) } diff --git a/src/shared/agent-hook-listener/providers/codex-events.ts b/src/shared/agent-hook-listener/providers/codex-events.ts index b9e902caf154..d58421f8996a 100644 --- a/src/shared/agent-hook-listener/providers/codex-events.ts +++ b/src/shared/agent-hook-listener/providers/codex-events.ts @@ -163,6 +163,14 @@ export function normalizeCodexEvent( return normalizeCodexSubagentLifecycleEvent(state, eventName, paneKey, hookPayload) } + // Why: child hooks share the parent paneKey and carry their own session_id. + // Root SessionStart reset must not run for them — it would drop parent + // status/roster and store the child session as the resume id. + const childAgentId = readString(hookPayload, 'agent_id') + if (childAgentId && eventName === 'SessionStart') { + return null + } + if (eventName === 'SessionStart') { // Why: Codex fires SessionStart when opening or resuming an idle TUI, before // a user prompt exists; reset stale turn/session cache without emitting state. diff --git a/src/shared/agent-hook-session-identity-review.test.ts b/src/shared/agent-hook-session-identity-review.test.ts new file mode 100644 index 000000000000..81624346c85e --- /dev/null +++ b/src/shared/agent-hook-session-identity-review.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it, vi } from 'vitest' +import { normalizeHookPayload } from './agent-hook-listener' +import { createHookListenerState } from './agent-hook-listener/listener-state' +import { PANE_KEY } from './agent-hook-listener-test-harness' +import { AgentHookServer } from '../main/agent-hooks/server' + +vi.mock('../main/telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../main/telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn(() => ({})) })) + +describe('review: Codex root identity isolation', () => { + it('keeps the parent status and session when a child emits SessionStart', () => { + const state = createHookListenerState() + const parent = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'UserPromptSubmit', + prompt: 'parent work', + session_id: 'parent-session' + } + }, + 'production' + ) + if (!parent) throw new Error('missing parent fixture') + state.lastStatusByPaneKey.set(PANE_KEY, parent) + normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'SessionStart', + agent_id: 'child-agent', + session_id: 'child-session' + } + }, + 'production' + ) + expect.soft(state.lastStatusByPaneKey.get(PANE_KEY)).toBe(parent) + expect.soft(state.lastProviderSessionByPaneKey.get(PANE_KEY)?.id).toBe('parent-session') + const next = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'PostToolUse', + tool_name: 'Bash', + tool_input: { command: 'pwd' } + } + }, + 'production' + ) + expect.soft(next?.providerSession?.id).toBe('parent-session') + }) + + it('does not replace the current connection session with a delayed foreign SessionStart', () => { + const server = new AgentHookServer() + server.ingestRemote( + { + paneKey: PANE_KEY, + source: 'codex', + hookEventName: 'SessionStart', + providerSession: { key: 'session_id', id: 'current-session' }, + payload: { agentType: 'codex', state: 'working', prompt: '' } + }, + 'current-connection' + ) + server.ingestRemote( + { + paneKey: PANE_KEY, + source: 'codex', + hookEventName: 'UserPromptSubmit', + providerSession: { key: 'session_id', id: 'current-session' }, + payload: { agentType: 'codex', state: 'working', prompt: 'current work' } + }, + 'current-connection' + ) + server.ingestRemote( + { + paneKey: PANE_KEY, + source: 'codex', + hookEventName: 'SessionStart', + providerSession: { key: 'session_id', id: 'stale-session' }, + payload: { agentType: 'codex', state: 'working', prompt: '' } + }, + 'stale-connection' + ) + expect + .soft(server.getStatusSnapshot()) + .toEqual([ + expect.objectContaining({ connectionId: 'current-connection', prompt: 'current work' }) + ]) + expect + .soft(server._getStateForTests().lastProviderSessionByPaneKey.get(PANE_KEY)?.id) + .toBe('current-session') + }) + it('ignores session identity from an unrecognized hook', () => { + const state = createHookListenerState() + normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { + hook_event_name: 'UserPromptSubmit', + prompt: 'real work', + session_id: 'real-session' + } + }, + 'production' + ) + const ignored = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'UnknownEvent', session_id: 'ignored-session' } + }, + 'production' + ) + expect(ignored).toBeNull() + const next = normalizeHookPayload( + state, + 'codex', + { + paneKey: PANE_KEY, + payload: { hook_event_name: 'PostToolUse', tool_name: 'Bash' } + }, + 'production' + ) + expect(next?.providerSession?.id).toBe('real-session') + }) +}) From 97b563c1be21a193337f7aa6a3c0da43235dffe2 Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Wed, 9 Sep 2026 02:25:53 +0800 Subject: [PATCH 4/7] test(codex): keep identity-review cases out of the web typecheck graph The three root identity regressions stay intact. They live under main so tsconfig.web does not follow AgentHookServer into telemetry defines. Co-authored-by: Cursor --- ...erver-codex-session-identity-review.test.ts} | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) rename src/{shared/agent-hook-session-identity-review.test.ts => main/agent-hooks/server-codex-session-identity-review.test.ts} (87%) diff --git a/src/shared/agent-hook-session-identity-review.test.ts b/src/main/agent-hooks/server-codex-session-identity-review.test.ts similarity index 87% rename from src/shared/agent-hook-session-identity-review.test.ts rename to src/main/agent-hooks/server-codex-session-identity-review.test.ts index 81624346c85e..ff59c463db47 100644 --- a/src/shared/agent-hook-session-identity-review.test.ts +++ b/src/main/agent-hooks/server-codex-session-identity-review.test.ts @@ -1,11 +1,11 @@ import { describe, expect, it, vi } from 'vitest' -import { normalizeHookPayload } from './agent-hook-listener' -import { createHookListenerState } from './agent-hook-listener/listener-state' -import { PANE_KEY } from './agent-hook-listener-test-harness' -import { AgentHookServer } from '../main/agent-hooks/server' +import { normalizeHookPayload } from '../../shared/agent-hook-listener' +import { createHookListenerState } from '../../shared/agent-hook-listener/listener-state' +import { PANE_KEY } from '../../shared/agent-hook-listener-test-harness' +import { AgentHookServer } from './server' -vi.mock('../main/telemetry/client', () => ({ track: vi.fn() })) -vi.mock('../main/telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn(() => ({})) })) +vi.mock('../telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn(() => ({})) })) describe('review: Codex root identity isolation', () => { it('keeps the parent status and session when a child emits SessionStart', () => { @@ -23,7 +23,9 @@ describe('review: Codex root identity isolation', () => { }, 'production' ) - if (!parent) throw new Error('missing parent fixture') + if (!parent) { + throw new Error('missing parent fixture') + } state.lastStatusByPaneKey.set(PANE_KEY, parent) normalizeHookPayload( state, @@ -97,6 +99,7 @@ describe('review: Codex root identity isolation', () => { .soft(server._getStateForTests().lastProviderSessionByPaneKey.get(PANE_KEY)?.id) .toBe('current-session') }) + it('ignores session identity from an unrecognized hook', () => { const state = createHookListenerState() normalizeHookPayload( From 08b8ab36b20d90a1e174e8393acfac3fa751ab95 Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Wed, 9 Sep 2026 02:36:04 +0800 Subject: [PATCH 5/7] fix(codex): move disabled no-hash user trust across remote prepends A user hook with enabled=false and no trusted_hash must travel with its index shift. Leaving it behind disables the managed hook and inventing a hash would synthesize approval. Co-authored-by: Cursor --- .../codex/codex-hook-remote-install.test.ts | 71 +++++++++ src/main/codex/config-toml-hook-trust-edit.ts | 39 ++--- .../config-toml-trust-api-parity.test.ts | 1 + .../config-toml-trust-move-review.test.ts | 136 ++++++++++++++++++ 4 files changed, 229 insertions(+), 18 deletions(-) create mode 100644 src/main/codex/config-toml-trust-move-review.test.ts diff --git a/src/main/codex/codex-hook-remote-install.test.ts b/src/main/codex/codex-hook-remote-install.test.ts index 6ba5de549736..fa4e29d6f5d6 100644 --- a/src/main/codex/codex-hook-remote-install.test.ts +++ b/src/main/codex/codex-hook-remote-install.test.ts @@ -133,4 +133,75 @@ describe('Codex remote hook prepend + trust migration', () => { expect(hookTrustBlock(toml, `${remoteHooksPath}:stop:0:0`)).not.toContain(userTrustedHash) expect(toml).not.toContain(`${remoteHooksPath}:stop:2:0`) }) + + it('moves a disabled no-hash user hook on SSH and WSL-redirected prepend, including repeats', async () => { + const posixHooksPath = '/home/dev/.codex/hooks.json' + const wslHooksPath = '/mnt/c/Users/me/.codex/hooks.json' + const userA = 'echo user-stop-a' + const userB = 'echo user-stop-b' + const userBHash = 'sha256:user-b' + const seedToml = (hooksPath: string) => + [ + `[hooks.state."${hooksPath}:stop:0:0"]`, + 'enabled = false', + '', + `[hooks.state."${hooksPath}:stop:1:0"]`, + 'enabled = false', + `trusted_hash = "${userBHash}"`, + '' + ].join('\n') + const hooksJson = `${JSON.stringify({ + hooks: { + Stop: [ + { hooks: [{ type: 'command', command: userA }] }, + { hooks: [{ type: 'command', command: userB }] } + ] + } + })}\n` + + const { sftp: sshSftp, files: sshFiles } = createFakeSftp({ + [posixHooksPath]: hooksJson, + '/home/dev/.codex/config.toml': seedToml(posixHooksPath) + }) + const { sftp: wslSftp, files: wslFiles } = createFakeSftp({ + [wslHooksPath]: hooksJson, + '/mnt/c/Users/me/.codex/config.toml': seedToml(wslHooksPath) + }) + + const service = new CodexHookService() + const sshFirst = await service.installRemote(sshSftp, '/home/dev') + const sshRepeat = await service.installRemote(sshSftp, '/home/dev') + const wslFirst = await service.installRemote(wslSftp, '/home/dev', { + codexHomeDir: '/mnt/c/Users/me/.codex' + }) + const wslRepeat = await service.installRemote(wslSftp, '/home/dev', { + codexHomeDir: '/mnt/c/Users/me/.codex' + }) + + expect(sshFirst.state).toBe('installed') + expect(sshRepeat.state).toBe('installed') + expect(wslFirst.state).toBe('installed') + expect(wslRepeat.state).toBe('installed') + + for (const [hooksPath, files, tomlPath] of [ + [posixHooksPath, sshFiles, '/home/dev/.codex/config.toml'], + [wslHooksPath, wslFiles, '/mnt/c/Users/me/.codex/config.toml'] + ] as const) { + const hooks = JSON.parse(files.get(hooksPath)!) as { + hooks: Record + } + expect(hooks.hooks.Stop?.[0]?.hooks?.[0]?.command).toContain('codex-hook.sh') + expect(hooks.hooks.Stop?.[1]?.hooks?.[0]?.command).toBe(userA) + expect(hooks.hooks.Stop?.[2]?.hooks?.[0]?.command).toBe(userB) + const toml = files.get(tomlPath) ?? '' + const movedA = hookTrustBlock(toml, `${hooksPath}:stop:1:0`) + const movedB = hookTrustBlock(toml, `${hooksPath}:stop:2:0`) + expect(movedA).toContain('enabled = false') + expect(movedA).not.toContain('trusted_hash') + expect(movedB).toContain('enabled = false') + expect(movedB).toContain(`trusted_hash = "${userBHash}"`) + expect(hookTrustBlock(toml, `${hooksPath}:stop:0:0`)).not.toContain(userBHash) + expect(toml).not.toContain(`${hooksPath}:stop:3:0`) + } + }) }) diff --git a/src/main/codex/config-toml-hook-trust-edit.ts b/src/main/codex/config-toml-hook-trust-edit.ts index b783cf64bb10..5388c371e7d1 100644 --- a/src/main/codex/config-toml-hook-trust-edit.ts +++ b/src/main/codex/config-toml-hook-trust-edit.ts @@ -52,9 +52,13 @@ export function moveHookTrustContent( return [] } const state = states.get(fromKey) - return state?.trustedHash - ? [{ fromKey, toKey, trustedHash: state.trustedHash, enabled: state.enabled }] - : [] + // Why: disabled user hooks often have no trusted_hash. Skipping them leaves + // enablement at the old index (now the managed hook) and drops the user's + // disablement. Move any enabled/hash row without inventing a hash. + if (!state || (state.trustedHash === undefined && state.enabled === undefined)) { + return [] + } + return [{ fromKey, toKey, trustedHash: state.trustedHash, enabled: state.enabled }] }) let updated = existing // Why: remove old index-addressed user trust blocks before writing the shifted keys so remote prepend does not leave duplicate approvals. @@ -64,12 +68,7 @@ export function moveHookTrustContent( ]) } for (const { toKey, trustedHash, enabled } of resolvedMoves) { - updated = upsertTrustBlocks( - updated, - getTrustKeyWriteVariants(toKey), - trustedHash, - enabled ?? true - ) + updated = upsertTrustBlocks(updated, getTrustKeyWriteVariants(toKey), trustedHash, enabled) } return updated } @@ -92,7 +91,7 @@ export function removeHookTrustContent(content: string, keys: readonly string[]) function upsertTrustBlocks( content: string, keys: readonly string[], - hash: string, + hash?: string, explicitEnabled?: boolean ): string { const ranges = findHookTrustBlockRanges( @@ -125,7 +124,7 @@ function isBlockDisabled(content: string, range: HookTrustBlockRange): boolean { function appendTrustBlocks( content: string, keys: readonly string[], - hash: string, + hash: string | undefined, enabled: boolean ): string { const block = buildTrustBlocks(keys, hash, enabled) @@ -136,16 +135,20 @@ function appendTrustBlocks( return `${content}${separator}${block}\n` } -function buildTrustBlocks(keys: readonly string[], hash: string, enabled: boolean): string { +function buildTrustBlocks( + keys: readonly string[], + hash: string | undefined, + enabled: boolean +): string { return keys.map((key) => buildTrustBlock(key, hash, enabled)).join('\n\n') } -function buildTrustBlock(key: string, hash: string, enabled: boolean): string { - return [ - `[hooks.state.${formatHookStateTableKey(key)}]`, - `enabled = ${enabled}`, - `trusted_hash = "${escapeTomlBasicString(hash)}"` - ].join('\n') +function buildTrustBlock(key: string, hash: string | undefined, enabled: boolean): string { + const lines = [`[hooks.state.${formatHookStateTableKey(key)}]`, `enabled = ${enabled}`] + if (hash) { + lines.push(`trusted_hash = "${escapeTomlBasicString(hash)}"`) + } + return lines.join('\n') } function formatHookStateTableKey(key: string): string { diff --git a/src/main/codex/config-toml-trust-api-parity.test.ts b/src/main/codex/config-toml-trust-api-parity.test.ts index d20cde89b8e2..b1bb467acce5 100644 --- a/src/main/codex/config-toml-trust-api-parity.test.ts +++ b/src/main/codex/config-toml-trust-api-parity.test.ts @@ -32,6 +32,7 @@ describe('config-toml-trust public API', () => { 'normalizeCodexHookSourcePath', 'normalizeCodexProjectPathForLookup', 'normalizeCodexProjectPathForRevocationLookup', + 'moveHookTrustEntriesInContent', 'normalizeHookTrustKeyForLookup', 'parseCodexProjectHeaderPath', 'parseTrustKey', diff --git a/src/main/codex/config-toml-trust-move-review.test.ts b/src/main/codex/config-toml-trust-move-review.test.ts new file mode 100644 index 000000000000..7280729a18fd --- /dev/null +++ b/src/main/codex/config-toml-trust-move-review.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest' +import { collectPrependedRemoteUserTrustMoves } from './codex-hook-remote-user-trust-moves' +import { moveHookTrustEntriesInContent, readHookTrustEntriesFromContent } from './config-toml-trust' + +describe('review: index moves preserve approval absence and disablement', () => { + it('moves a disabled user hook even when it has no trusted hash', () => { + const source = '/home/dev/.codex/hooks.json:stop:0:0' + const target = '/home/dev/.codex/hooks.json:stop:1:0' + const input = `[hooks.state."${source}"]\nenabled = false\n` + const output = moveHookTrustEntriesInContent(input, [{ fromKey: source, toKey: target }]) + const states = readHookTrustEntriesFromContent(output) + expect.soft(states.get(target)).toMatchObject({ enabled: false }) + expect.soft(states.get(target)?.trustedHash).toBeUndefined() + expect.soft(states.has(source)).toBe(false) + }) +}) + +describe('index moves: simultaneous shifts, collisions, and no synthesized approval', () => { + const base = '/home/dev/.codex/hooks.json:stop' + + it('shifts two adjacent rows without giving the disabled no-hash hook a hash', () => { + const input = [ + `[hooks.state."${base}:0:0"]`, + 'enabled = false', + '', + `[hooks.state."${base}:1:0"]`, + 'enabled = false', + 'trusted_hash = "sha256:user-b"', + '' + ].join('\n') + const output = moveHookTrustEntriesInContent(input, [ + { fromKey: `${base}:0:0`, toKey: `${base}:1:0` }, + { fromKey: `${base}:1:0`, toKey: `${base}:2:0` } + ]) + const states = readHookTrustEntriesFromContent(output) + expect(states.get(`${base}:1:0`)).toMatchObject({ enabled: false }) + expect(states.get(`${base}:1:0`)?.trustedHash).toBeUndefined() + expect(states.get(`${base}:2:0`)).toEqual({ enabled: false, trustedHash: 'sha256:user-b' }) + expect(states.has(`${base}:0:0`)).toBe(false) + expect(output).toContain(`[hooks.state."${base}:1:0"]\nenabled = false\n`) + expect(output).not.toContain(`[hooks.state."${base}:1:0"]\nenabled = false\ntrusted_hash`) + }) + + it('applies the same chain when move order is reversed', () => { + const input = [ + `[hooks.state."${base}:0:0"]`, + 'enabled = false', + '', + `[hooks.state."${base}:1:0"]`, + 'enabled = true', + 'trusted_hash = "sha256:user-b"', + '' + ].join('\n') + const moves = [ + { fromKey: `${base}:0:0`, toKey: `${base}:1:0` }, + { fromKey: `${base}:1:0`, toKey: `${base}:2:0` } + ] + const forward = readHookTrustEntriesFromContent(moveHookTrustEntriesInContent(input, moves)) + const reverse = readHookTrustEntriesFromContent( + moveHookTrustEntriesInContent(input, moves.toReversed()) + ) + expect(forward.get(`${base}:1:0`)).toEqual(reverse.get(`${base}:1:0`)) + expect(forward.get(`${base}:2:0`)).toEqual(reverse.get(`${base}:2:0`)) + expect(forward.has(`${base}:0:0`)).toBe(false) + expect(reverse.has(`${base}:0:0`)).toBe(false) + }) + + it('does not adopt a colliding destination hash when moving a disabled no-hash row', () => { + const source = `${base}:0:0` + const target = `${base}:1:0` + const input = [ + `[hooks.state."${source}"]`, + 'enabled = false', + '', + `[hooks.state."${target}"]`, + 'enabled = true', + 'trusted_hash = "sha256:destination"', + '' + ].join('\n') + const output = moveHookTrustEntriesInContent(input, [{ fromKey: source, toKey: target }]) + const states = readHookTrustEntriesFromContent(output) + expect(states.get(target)).toMatchObject({ enabled: false }) + expect(states.get(target)?.trustedHash).toBeUndefined() + expect(states.has(source)).toBe(false) + expect(output).not.toContain('sha256:destination') + }) + + it('keeps a disabled hashed approval intact across the same index shift', () => { + const source = `${base}:0:0` + const target = `${base}:1:0` + const input = [ + `[hooks.state."${source}"]`, + 'enabled = false', + 'trusted_hash = "sha256:user-approved"', + '' + ].join('\n') + const states = readHookTrustEntriesFromContent( + moveHookTrustEntriesInContent(input, [{ fromKey: source, toKey: target }]) + ) + expect(states.get(target)).toEqual({ enabled: false, trustedHash: 'sha256:user-approved' }) + expect(states.has(source)).toBe(false) + }) + + it('is a no-op when the collected prepend move is already at the target index', () => { + const key = `${base}:1:0` + const input = `[hooks.state."${key}"]\nenabled = false\n` + expect(moveHookTrustEntriesInContent(input, [{ fromKey: key, toKey: key }])).toBe(input) + }) + + it('matches repeated remote prepend collection to a same-index no-op', () => { + const sourcePath = '/home/dev/.codex/hooks.json' + const userHook = { hooks: [{ type: 'command' as const, command: 'echo user-stop' }] } + const managed = { + hooks: [{ type: 'command' as const, command: '/home/dev/.orca/codex-hook.sh' }] + } + const isManaged = (command: string | undefined) => Boolean(command?.includes('codex-hook.sh')) + const first = collectPrependedRemoteUserTrustMoves( + sourcePath, + 'Stop', + [userHook], + [userHook], + isManaged + ) + expect(first).toEqual([{ fromKey: `${sourcePath}:stop:0:0`, toKey: `${sourcePath}:stop:1:0` }]) + const repeated = collectPrependedRemoteUserTrustMoves( + sourcePath, + 'Stop', + [managed, userHook], + [userHook], + isManaged + ) + expect(repeated).toEqual([ + { fromKey: `${sourcePath}:stop:1:0`, toKey: `${sourcePath}:stop:1:0` } + ]) + }) +}) From bc81e9d7c9f5cdec0bb8cf744309a2aaa40dff7f Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Wed, 9 Sep 2026 02:50:28 +0800 Subject: [PATCH 6/7] fix(codex): snapshot source trust on index moves, including absence Destination enabled/hash must come only from the source. Vacate both keys before writes so a missing source cannot reuse a stale dest approval, and a missing enabled cannot inherit dest disablement. Adapt the Codex retired-pane case: SessionStart stays idle metadata and the first real event supplies the visible working row. Co-authored-by: Cursor --- ...er-codex-session-retirement-review.test.ts | 38 ++++++++++++++++ .../server-retired-pane-new-turn.test.ts | 45 ++++++++++++++++++- src/main/codex/config-toml-hook-trust-edit.ts | 32 +++++++------ ...-toml-trust-default-enabled-review.test.ts | 23 ++++++++++ 4 files changed, 122 insertions(+), 16 deletions(-) create mode 100644 src/main/agent-hooks/server-codex-session-retirement-review.test.ts create mode 100644 src/main/codex/config-toml-trust-default-enabled-review.test.ts diff --git a/src/main/agent-hooks/server-codex-session-retirement-review.test.ts b/src/main/agent-hooks/server-codex-session-retirement-review.test.ts new file mode 100644 index 000000000000..2cb47637c2a6 --- /dev/null +++ b/src/main/agent-hooks/server-codex-session-retirement-review.test.ts @@ -0,0 +1,38 @@ +import { expect, it, vi } from 'vitest' +import { AgentHookServer } from './server' +import { PANE } from './server.test-fixtures' +vi.mock('../telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn(() => ({})) })) +it('accepts the first real tool event after an idle SessionStart revives a retired pane', () => { + const server = new AgentHookServer() + server.retirePaneAuthority(PANE) + server.ingestRemote( + { + paneKey: PANE, + source: 'codex', + hookEventName: 'SessionStart', + providerSession: { key: 'session_id', id: 'new-session' }, + payload: { state: 'working', prompt: '', agentType: 'codex' } + }, + 'conn-1' + ) + expect(server.getStatusSnapshot()).toEqual([]) + server.ingestRemote( + { + paneKey: PANE, + source: 'codex', + hookEventName: 'PostToolUse', + payload: { + state: 'working', + prompt: '', + agentType: 'codex', + toolName: 'Bash', + toolInput: 'pwd' + } + }, + 'conn-1' + ) + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: PANE, state: 'working', agentType: 'codex' }) + ]) +}) diff --git a/src/main/agent-hooks/server-retired-pane-new-turn.test.ts b/src/main/agent-hooks/server-retired-pane-new-turn.test.ts index 3466cb52af9d..1529c1fb5093 100644 --- a/src/main/agent-hooks/server-retired-pane-new-turn.test.ts +++ b/src/main/agent-hooks/server-retired-pane-new-turn.test.ts @@ -20,7 +20,9 @@ afterEach(() => vi.restoreAllMocks()) /** Each source's own new-turn boundary, as `isNewTurnEvent` classifies it. `null` means the * classifier names no boundary for that source. That is not the same as "can never revive": - * mimo-code's boundary is an explicit-prompt MessagePart, which the gate handles separately. */ + * mimo-code's boundary is an explicit-prompt MessagePart, which the gate handles separately. + * Codex SessionStart still names a boundary (it un-retires the fence) but is idle metadata, + * not a visible working row — the dedicated Codex case covers first-real-event revival. */ const NEW_TURN_EVENT: Record = { claude: 'SessionStart', kimi: 'UserPromptSubmit', @@ -71,7 +73,7 @@ describe("retired pane un-retires on each provider's own new-turn event", () => // Why: keys of a Record — a new source fails typecheck here rather than // silently skipping coverage, which is the same guarantee the runtime list would give. const revivable = (Object.keys(NEW_TURN_EVENT) as AgentHookSource[]).filter( - (source) => NEW_TURN_EVENT[source] !== null + (source) => NEW_TURN_EVENT[source] !== null && source !== 'codex' ) it.each(revivable)('%s', (source) => { @@ -80,6 +82,45 @@ describe("retired pane un-retires on each provider's own new-turn event", () => expect(reviveRetiredPane(source, hookEventName as string)).toBe(true) }) + it('codex un-retires on SessionStart without a synthetic working row, then the first real event supplies the row', () => { + // Why this assertion changed: the old `%s` table treated Codex SessionStart like + // Claude and required a visible working row. SessionStart is an idle metadata + // clear; a synthetic working row is the mixed-version bug this PR closes. The + // fence still opens, and the first real event (PostToolUse) is the visible row. + const server = new AgentHookServer() + server.retirePaneAuthority(PANE) + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + source: 'codex', + hookEventName: 'SessionStart', + payload: { state: 'working', prompt: 'after reuse', agentType: 'codex' } + }, + 'conn-1' + ) + expect(server.getStatusSnapshot().some((entry) => entry.paneKey === PANE)).toBe(false) + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + source: 'codex', + hookEventName: 'PostToolUse', + payload: { + state: 'working', + prompt: '', + agentType: 'codex', + toolName: 'Bash', + toolInput: 'pwd' + } + }, + 'conn-1' + ) + expect(server.getStatusSnapshot().some((entry) => entry.paneKey === PANE)).toBe(true) + }) + // Why these two: every case above passes `source`, so the source-less compatibility path — // the branch added for older relays — would otherwise ship with no coverage at all. it('revives on a literal boundary when an older relay omits source', () => { diff --git a/src/main/codex/config-toml-hook-trust-edit.ts b/src/main/codex/config-toml-hook-trust-edit.ts index 5388c371e7d1..e00ea4dc1091 100644 --- a/src/main/codex/config-toml-hook-trust-edit.ts +++ b/src/main/codex/config-toml-hook-trust-edit.ts @@ -52,23 +52,27 @@ export function moveHookTrustContent( return [] } const state = states.get(fromKey) - // Why: disabled user hooks often have no trusted_hash. Skipping them leaves - // enablement at the old index (now the managed hook) and drops the user's - // disablement. Move any enabled/hash row without inventing a hash. - if (!state || (state.trustedHash === undefined && state.enabled === undefined)) { - return [] - } - return [{ fromKey, toKey, trustedHash: state.trustedHash, enabled: state.enabled }] + return [{ fromKey, toKey, trustedHash: state?.trustedHash, enabled: state?.enabled }] }) - let updated = existing - // Why: remove old index-addressed user trust blocks before writing the shifted keys so remote prepend does not leave duplicate approvals. - if (resolvedMoves.length > 0) { - updated = removeHookTrustContent(updated, [ - ...new Set(resolvedMoves.map(({ fromKey }) => fromKey)) - ]) + if (resolvedMoves.length === 0) { + return existing } + // Why: destination is the source snapshot only. Vacate every non-identity + // from and to key before writes so a missing source clears a stale dest + // approval and a missing enabled cannot inherit dest disablement. + let updated = removeHookTrustContent(existing, [ + ...new Set(resolvedMoves.flatMap(({ fromKey, toKey }) => [fromKey, toKey])) + ]) for (const { toKey, trustedHash, enabled } of resolvedMoves) { - updated = upsertTrustBlocks(updated, getTrustKeyWriteVariants(toKey), trustedHash, enabled) + if (trustedHash === undefined && enabled === undefined) { + continue + } + updated = upsertTrustBlocks( + updated, + getTrustKeyWriteVariants(toKey), + trustedHash, + enabled ?? true + ) } return updated } diff --git a/src/main/codex/config-toml-trust-default-enabled-review.test.ts b/src/main/codex/config-toml-trust-default-enabled-review.test.ts new file mode 100644 index 000000000000..46d21ed9f1cb --- /dev/null +++ b/src/main/codex/config-toml-trust-default-enabled-review.test.ts @@ -0,0 +1,23 @@ +import { expect, it } from 'vitest' +import { moveHookTrustEntriesInContent, readHookTrustEntriesFromContent } from './config-toml-trust' + +it('does not inherit a stale destination disablement when the source has no explicit enabled flag', () => { + const source = '/home/dev/.codex/hooks.json:stop:0:0' + const target = '/home/dev/.codex/hooks.json:stop:1:0' + const input = `[hooks.state."${source}"]\ntrusted_hash = "sha256:source"\n\n[hooks.state."${target}"]\nenabled = false\ntrusted_hash = "sha256:stale-destination"\n` + const output = readHookTrustEntriesFromContent( + moveHookTrustEntriesInContent(input, [{ fromKey: source, toKey: target }]) + ) + expect(output.get(target)?.trustedHash).toBe('sha256:source') + expect(output.get(target)?.enabled).not.toBe(false) +}) + +it('does not inherit a stale destination approval when the source is unapproved', () => { + const source = '/home/dev/.codex/hooks.json:stop:0:0' + const target = '/home/dev/.codex/hooks.json:stop:1:0' + const input = `[hooks.state."${target}"]\nenabled = true\ntrusted_hash = "sha256:old-approval-for-same-command"\n` + const output = readHookTrustEntriesFromContent( + moveHookTrustEntriesInContent(input, [{ fromKey: source, toKey: target }]) + ) + expect(output.get(target)?.trustedHash).toBeUndefined() +}) From 13ab19acda22a954671096841c9e005fb5338a3f Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Sun, 20 Sep 2026 19:21:52 +0800 Subject: [PATCH 7/7] refactor(codex): adapt hooks status to latest legacy agent status adapter and eliminate type assertions --- config/tsconfig.cli.json | 1 + ...erver-codex-session-identity-review.test.ts | 7 +++++-- .../server/server-status-identity.ts | 4 ++-- .../server/server-status-retries.ts | 3 ++- .../codex/codex-hook-remote-install.test.ts | 18 ++++++++++++------ src/relay/agent-hook-envelope-build.ts | 12 ++++++------ src/relay/agent-hook-server.ts | 5 ----- ...t-hook-listener-codex-session-start.test.ts | 11 +++++++---- .../providers/codex-events.ts | 1 + 9 files changed, 36 insertions(+), 26 deletions(-) diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 43eac7cb024d..62e3342d6d66 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -54,6 +54,7 @@ "../src/main/codex/codex-hook-local-install.ts", "../src/main/codex/codex-hook-local-maintenance.ts", "../src/main/codex/codex-hook-remote-install.ts", + "../src/main/codex/codex-hook-remote-user-trust-moves.ts", "../src/main/codex/codex-hook-script.ts", "../src/main/codex/codex-hook-service-implementation.ts", "../src/main/codex/codex-hook-status.ts", diff --git a/src/main/agent-hooks/server-codex-session-identity-review.test.ts b/src/main/agent-hooks/server-codex-session-identity-review.test.ts index ff59c463db47..16e6563864ac 100644 --- a/src/main/agent-hooks/server-codex-session-identity-review.test.ts +++ b/src/main/agent-hooks/server-codex-session-identity-review.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it, vi } from 'vitest' import { normalizeHookPayload } from '../../shared/agent-hook-listener' -import { createHookListenerState } from '../../shared/agent-hook-listener/listener-state' +import { + createHookListenerState, + seedLegacyAgentStatusForTests +} from '../../shared/agent-hook-listener/listener-state' import { PANE_KEY } from '../../shared/agent-hook-listener-test-harness' import { AgentHookServer } from './server' @@ -26,7 +29,7 @@ describe('review: Codex root identity isolation', () => { if (!parent) { throw new Error('missing parent fixture') } - state.lastStatusByPaneKey.set(PANE_KEY, parent) + seedLegacyAgentStatusForTests(state, parent) normalizeHookPayload( state, 'codex', diff --git a/src/main/agent-hooks/server/server-status-identity.ts b/src/main/agent-hooks/server/server-status-identity.ts index 580b4aef9d35..e7ed71411b3f 100644 --- a/src/main/agent-hooks/server/server-status-identity.ts +++ b/src/main/agent-hooks/server/server-status-identity.ts @@ -36,10 +36,10 @@ export function equivalentInterruptAgentType( // Why: validate the durable `${tabId}:${leafUuid}` leaf suffix at write/hydrate so legacy numeric rows fail closed. export function hookBodyPaneKey(body: unknown): string | null { - if (typeof body !== 'object' || body === null) { + if (typeof body !== 'object' || body === null || !('paneKey' in body)) { return null } - const paneKey = (body as Record).paneKey + const paneKey = body.paneKey if (typeof paneKey !== 'string') { return null } diff --git a/src/main/agent-hooks/server/server-status-retries.ts b/src/main/agent-hooks/server/server-status-retries.ts index 38ea80b2de37..3126fbf65f1b 100644 --- a/src/main/agent-hooks/server/server-status-retries.ts +++ b/src/main/agent-hooks/server/server-status-retries.ts @@ -7,6 +7,7 @@ import { import type { AgentHookSource } from '../../../shared/agent-hook-relay' import { CodexSubagentPollScheduler } from '../../../shared/codex-subagent-poll-scheduler' import type { AgentHookEventPayload } from '../../../shared/agent-hook-listener/listener-event' +import { deleteLegacyAgentStatus } from '../../../shared/agent-hook-listener/listener-state' import type { EnrichedAgentHookEventPayload } from './server-types' import { ASSISTANT_MESSAGE_RETRY_ATTEMPTS, @@ -55,7 +56,7 @@ export abstract class AgentHookServerStatusRetries extends AgentHookServerStatus if (!status || !this.canApplyCodexSessionStart(status, expectedConnectionId)) { return } - this.state.lastStatusByPaneKey.delete(paneKey) + deleteLegacyAgentStatus(this.state, paneKey) // Why: SessionStart is an idle metadata boundary, so remove the stale row // without emitting a synthetic visible status for the new session. this.clearAssistantMessageRetry(paneKey) diff --git a/src/main/codex/codex-hook-remote-install.test.ts b/src/main/codex/codex-hook-remote-install.test.ts index fa4e29d6f5d6..5e51286faec5 100644 --- a/src/main/codex/codex-hook-remote-install.test.ts +++ b/src/main/codex/codex-hook-remote-install.test.ts @@ -19,6 +19,7 @@ function createFakeSftp(initialFiles: Record = {}): { code: 2, message: `ENOENT ${path}` }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Test mock implementing minimal SFTPWrapper surface. const sftp = { readFile: (path: string, _enc: string, cb: (err: unknown, data?: string) => void): void => { const value = files.get(path) @@ -76,6 +77,15 @@ function createFakeSftp(initialFiles: Record = {}): { return { sftp, files } } +type ParsedHooksFile = { + hooks: Record +} + +function parseHooksFile(raw: string | undefined): ParsedHooksFile { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Test helper parsing remote hook json shape. + return JSON.parse(raw ?? '{}') as ParsedHooksFile +} + function hookTrustBlock(content: string, key: string): string { const header = `[hooks.state."${key}"]` const start = content.indexOf(header) @@ -116,9 +126,7 @@ describe('Codex remote hook prepend + trust migration', () => { expect(status.state).toBe('installed') expect(repeatedStatus.state).toBe('installed') - const hooks = JSON.parse(files.get(remoteHooksPath)!) as { - hooks: Record - } + const hooks = parseHooksFile(files.get(remoteHooksPath)) expect(hooks.hooks.Stop?.[0]?.hooks?.[0]?.command).toContain('codex-hook.sh') expect(hooks.hooks.Stop?.[1]?.hooks?.[0]?.command).toBe(userStopCommand) expect(hooks.hooks.SubagentStart?.[0]?.hooks?.[0]?.command).toContain('codex-hook.sh') @@ -187,9 +195,7 @@ describe('Codex remote hook prepend + trust migration', () => { [posixHooksPath, sshFiles, '/home/dev/.codex/config.toml'], [wslHooksPath, wslFiles, '/mnt/c/Users/me/.codex/config.toml'] ] as const) { - const hooks = JSON.parse(files.get(hooksPath)!) as { - hooks: Record - } + const hooks = parseHooksFile(files.get(hooksPath)) expect(hooks.hooks.Stop?.[0]?.hooks?.[0]?.command).toContain('codex-hook.sh') expect(hooks.hooks.Stop?.[1]?.hooks?.[0]?.command).toBe(userA) expect(hooks.hooks.Stop?.[2]?.hooks?.[0]?.command).toBe(userB) diff --git a/src/relay/agent-hook-envelope-build.ts b/src/relay/agent-hook-envelope-build.ts index 0c83f5c6448c..9433a0c072f2 100644 --- a/src/relay/agent-hook-envelope-build.ts +++ b/src/relay/agent-hook-envelope-build.ts @@ -42,18 +42,18 @@ export function buildRelayHookEnvelope( } export function hookBodyPaneKey(body: unknown): string | null { - if (typeof body !== 'object' || body === null) { + if (typeof body !== 'object' || body === null || !('paneKey' in body)) { return null } - const value = (body as Record).paneKey + const value = body.paneKey return typeof value === 'string' && value.trim().length > 0 ? value.trim() : null } export function hookBodyEnv(body: unknown): string | undefined { - if (typeof body !== 'object' || body === null) { + if (typeof body !== 'object' || body === null || !('env' in body)) { return undefined } - const v = (body as Record).env + const v = body.env if (typeof v !== 'string' || v.length === 0 || v.length > MAX_HOOK_META_LEN) { return undefined } @@ -61,10 +61,10 @@ export function hookBodyEnv(body: unknown): string | undefined { } export function hookBodyVersion(body: unknown): string | undefined { - if (typeof body !== 'object' || body === null) { + if (typeof body !== 'object' || body === null || !('version' in body)) { return undefined } - const v = (body as Record).version + const v = body.version if (typeof v !== 'string' || v.length === 0 || v.length > MAX_HOOK_META_LEN) { return undefined } diff --git a/src/relay/agent-hook-server.ts b/src/relay/agent-hook-server.ts index 937664e0d0f8..d7d5157edc1e 100644 --- a/src/relay/agent-hook-server.ts +++ b/src/relay/agent-hook-server.ts @@ -12,7 +12,6 @@ import { createHookListenerState, type HookListenerState } from '../shared/agent-hook-listener/listener-state' -import { cacheRelayLegacyAgentStatus } from '../shared/agent-status-legacy-relay-cache' import { getEndpointFileName, writeEndpointFile @@ -342,8 +341,4 @@ export class RelayAgentHookServer { ): void { applyRelayHookEvent(this.eventApplyHost(), event, source, env, version, options) } - - private ingestSpoolRecord(record: SpoolRecord): void { - ingestRelaySpoolRecord(this.eventApplyHost(), record) - } } diff --git a/src/shared/agent-hook-listener-codex-session-start.test.ts b/src/shared/agent-hook-listener-codex-session-start.test.ts index af84f063a9e8..b3118ee56bc5 100644 --- a/src/shared/agent-hook-listener-codex-session-start.test.ts +++ b/src/shared/agent-hook-listener-codex-session-start.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from 'vitest' import { normalizeHookPayload } from './agent-hook-listener' -import { createHookListenerState } from './agent-hook-listener/listener-state' +import { + createHookListenerState, + seedLegacyAgentStatusForTests +} from './agent-hook-listener/listener-state' import { makePaneKey } from './stable-pane-id' import { PANE_KEY } from './agent-hook-listener-test-harness' @@ -59,8 +62,8 @@ describe('Codex SessionStart listener obligations', () => { if (!current || !other) { throw new Error('expected working status fixtures') } - state.lastStatusByPaneKey.set(PANE_KEY, current) - state.lastStatusByPaneKey.set(otherPane, other) + seedLegacyAgentStatusForTests(state, current) + seedLegacyAgentStatusForTests(state, other) const started = normalizeHookPayload( state, @@ -91,7 +94,7 @@ describe('Codex SessionStart listener obligations', () => { if (!claude) { throw new Error('expected Claude working status fixture') } - state.lastStatusByPaneKey.set(PANE_KEY, claude) + seedLegacyAgentStatusForTests(state, claude) normalizeHookPayload( state, diff --git a/src/shared/agent-hook-listener/providers/codex-events.ts b/src/shared/agent-hook-listener/providers/codex-events.ts index d58421f8996a..f00e56f8c06e 100644 --- a/src/shared/agent-hook-listener/providers/codex-events.ts +++ b/src/shared/agent-hook-listener/providers/codex-events.ts @@ -23,6 +23,7 @@ import { deleteLegacyAgentStatus, type HookListenerState } from '../listener-state' +import { resolvePrompt, resolveToolState } from '../prompt-fields' import { extractToolFields, isNewTurnEvent } from '../provider-event-routing' import { readString } from '../tool-input-preview' import {