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-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-identity-review.test.ts b/src/main/agent-hooks/server-codex-session-identity-review.test.ts new file mode 100644 index 000000000000..16e6563864ac --- /dev/null +++ b/src/main/agent-hooks/server-codex-session-identity-review.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it, vi } from 'vitest' +import { normalizeHookPayload } from '../../shared/agent-hook-listener' +import { + createHookListenerState, + seedLegacyAgentStatusForTests +} from '../../shared/agent-hook-listener/listener-state' +import { PANE_KEY } from '../../shared/agent-hook-listener-test-harness' +import { AgentHookServer } from './server' + +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', () => { + 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') + } + seedLegacyAgentStatusForTests(state, 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') + }) +}) 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-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..8e86c50f19a4 --- /dev/null +++ b/src/main/agent-hooks/server-codex-session-start-wire-semantics.test.ts @@ -0,0 +1,251 @@ +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, + stateHistory: [] + } +} + +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-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/agent-hooks/server/server-ingest-remote.ts b/src/main/agent-hooks/server/server-ingest-remote.ts index 0bc8d7947a9a..eb25b9689a4c 100644 --- a/src/main/agent-hooks/server/server-ingest-remote.ts +++ b/src/main/agent-hooks/server/server-ingest-remote.ts @@ -254,6 +254,25 @@ 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. + // 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, previousStatus, expectedConnectionId) + 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..e7ed71411b3f 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 || !('paneKey' in body)) { + return null + } + const paneKey = body.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..3126fbf65f1b 100644 --- a/src/main/agent-hooks/server/server-status-retries.ts +++ b/src/main/agent-hooks/server/server-status-retries.ts @@ -6,6 +6,8 @@ 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 { deleteLegacyAgentStatus } from '../../../shared/agent-hook-listener/listener-state' import type { EnrichedAgentHookEventPayload } from './server-types' import { ASSISTANT_MESSAGE_RETRY_ATTEMPTS, @@ -30,6 +32,41 @@ 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) + if (!status || !this.canApplyCodexSessionStart(status, expectedConnectionId)) { + return + } + 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) + 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..5e51286faec5 --- /dev/null +++ b/src/main/codex/codex-hook-remote-install.test.ts @@ -0,0 +1,213 @@ +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}` + }) + // 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) + 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 } +} + +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) + 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 = 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') + 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`) + }) + + 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 = 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) + 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/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..a1bca0be12f6 --- /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 type { 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..e00ea4dc1091 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,42 @@ 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 [{ fromKey, toKey, trustedHash: state?.trustedHash, enabled: state?.enabled }] + }) + 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) { + if (trustedHash === undefined && enabled === undefined) { + continue + } + 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) @@ -53,7 +95,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( @@ -86,7 +128,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) @@ -97,16 +139,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-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() +}) 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` } + ]) + }) +}) 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..9433a0c072f2 100644 --- a/src/relay/agent-hook-envelope-build.ts +++ b/src/relay/agent-hook-envelope-build.ts @@ -41,11 +41,19 @@ export function buildRelayHookEnvelope( } } +export function hookBodyPaneKey(body: unknown): string | null { + if (typeof body !== 'object' || body === null || !('paneKey' in body)) { + return null + } + 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 } @@ -53,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-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.ts b/src/relay/agent-hook-server.ts index 378f81cf0229..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 @@ -28,23 +27,26 @@ 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, 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' +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`. */ @@ -123,7 +125,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; @@ -275,6 +277,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 +289,18 @@ 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) + ) { + forwardSessionStartClear( + this.eventApplyHost(), + previousStatus, + hookBodyEnv(hookBody), + hookBodyVersion(hookBody) + ) } res.writeHead(204) res.end() @@ -303,6 +319,19 @@ export class RelayAgentHookServer { } } + 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 + } + } + private applyEvent( event: AgentHookEventPayload, source: AgentHookSource, @@ -310,45 +339,6 @@ 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)) - } - - 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 - }) + applyRelayHookEvent(this.eventApplyHost(), event, source, env, version, options) } } 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..b3118ee56bc5 --- /dev/null +++ b/src/shared/agent-hook-listener-codex-session-start.test.ts @@ -0,0 +1,324 @@ +import { describe, expect, it } from 'vitest' +import { normalizeHookPayload } from './agent-hook-listener' +import { + createHookListenerState, + seedLegacyAgentStatusForTests +} 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') + } + seedLegacyAgentStatusForTests(state, current) + seedLegacyAgentStatusForTests(state, 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') + } + seedLegacyAgentStatusForTests(state, 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..76c3538926d5 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,14 @@ 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. - const providerSession = - source === 'codex' && readString(hookPayloadRecord, 'agent_id') - ? null - : extractAgentProviderSession(source, hookPayloadRecord) + // 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. + // 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) @@ -134,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, @@ -181,3 +193,16 @@ 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 + } + return extracted ?? 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..f00e56f8c06e 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,7 +18,11 @@ import { reconcileCodexSubagentReviewer } from '../../codex-subagent-reviewer' import { readFirstString } from '../interactive-tool' -import type { HookListenerState } from '../listener-state' +import { + clearPaneTurnCacheState, + 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' @@ -86,19 +91,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 +164,41 @@ 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. + // 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. + // 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 +212,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 +285,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' }