diff --git a/mobile/src/session/mobile-structured-send-delivery.test.ts b/mobile/src/session/mobile-structured-send-delivery.test.ts index 8431e32a7aa9..bea3e2fb4351 100644 --- a/mobile/src/session/mobile-structured-send-delivery.test.ts +++ b/mobile/src/session/mobile-structured-send-delivery.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from 'vitest' +import { agentSessionFailureFact } from '../../../src/shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../src/shared/agent-session-failure-words' import type { AgentJournalDispatchState } from '../../../src/shared/agent-session-journal-types' import type { AgentSessionSendResult } from '../../../src/shared/agent-session-wire' import { mobileStructuredSendDelivery } from './mobile-structured-send-delivery' @@ -125,6 +127,34 @@ describe('mobileStructuredSendDelivery', () => { }) }) + it('reports a send its own hook blocked as sent, never handing its text back', () => { + // Drawn as a sent message on every client, so a restored draft would show it twice. + const answer = structuredSendResultFixture('rejected') + if (!('submission' in answer)) { + throw new Error('the fixture answers with a submission') + } + const blocked: StructuredAgentSessionMutationCallResult = { + status: 'accepted', + value: { + ...answer, + submission: { + ...answer.submission, + ...agentSessionFailureWords( + agentSessionFailureFact('hookBlocked', { + detail: { text: 'No secrets in prompts.', audience: 'person' } + }), + { surface: 'rejection', agentName: 'Codex' } + ) + } + } + } + expect(mobileStructuredSendDelivery(blocked)).toEqual({ + outcome: 'accepted', + operationIdSpent: true, + error: null + }) + }) + it('shows a provider content rejection verbatim', () => { expect( mobileStructuredSendDelivery(accepted('rejected', 'Claude does not support .bmp')) diff --git a/mobile/src/session/mobile-structured-send-delivery.ts b/mobile/src/session/mobile-structured-send-delivery.ts index fe6b1f05ebfe..2b871af7890d 100644 --- a/mobile/src/session/mobile-structured-send-delivery.ts +++ b/mobile/src/session/mobile-structured-send-delivery.ts @@ -32,6 +32,7 @@ import type { AgentSessionSendResult } from '../../../src/shared/agent-session-w import { agentSessionRefusalOperationState } from '../../../src/shared/agent-session-refusal-retry' import { agentSessionWriteNoticeEnglish } from '../../../src/shared/agent-session-refusal-notice' import { structuredAgentSessionRejectionNotice } from '../../../src/shared/structured-agent-session-send-disposition' +import { rejectionDrawnAsSent } from '../../../src/shared/structured-agent-session-dispatch-rejection' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import type { StructuredAgentSessionMutationCallResult } from './mobile-structured-agent-session-rpc' @@ -98,6 +99,10 @@ export function mobileStructuredSendDelivery( if (!submission || submission.dispatchState === 'unknown') { return { outcome: 'unknown', operationIdSpent: false, error: null } } + if (rejectionDrawnAsSent(submission)) { + // Every client draws it as sent, so its text handed back to the composer would show it twice. + return { outcome: 'accepted', operationIdSpent: true, error: null } + } if (submission.dispatchState === 'rejected') { return { outcome: 'rejected', diff --git a/mobile/src/session/use-mobile-structured-agent-session-rejected-rows.test.tsx b/mobile/src/session/use-mobile-structured-agent-session-rejected-rows.test.tsx new file mode 100644 index 000000000000..d736d1be79fd --- /dev/null +++ b/mobile/src/session/use-mobile-structured-agent-session-rejected-rows.test.tsx @@ -0,0 +1,142 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, expect, it, vi } from 'vitest' +import { agentSessionFailureFact } from '../../../src/shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../src/shared/agent-session-failure-words' +import { agentJournalSubmissionKey } from '../../../src/shared/agent-session-journal-item-key' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../src/shared/agent-session-journal-types' +import type { AgentSessionSubscribeEvent } from '../../../src/shared/agent-session-wire' +import type { RpcClient } from '../transport/rpc-client' +import { useMobileStructuredAgentSession } from './use-mobile-structured-agent-session' + +vi.mock('@react-native-async-storage/async-storage', () => ({ + default: { getItem: vi.fn(async () => null), setItem: vi.fn(), removeItem: vi.fn() } +})) + +const MESSAGE: AgentJournalRenderItem = { + itemId: agentJournalSubmissionKey('op-undelivered'), + revision: 0, + sequence: 1, + observedAt: 1, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'never got it' }] } +} + +/** Handed over, then the agent's history showed it never got it. */ +const UNDELIVERED: AgentJournalSubmission = { + clientMessageId: 'op-undelivered', + fence: 3, + payloadFingerprint: 'fingerprint', + dispatchState: 'rejected', + providerItemId: null, + submittedAt: 1, + resolvedAt: 2, + handoverRecorded: true, + handedOverAt: 1, + recovered: true, + ...agentSessionFailureWords(agentSessionFailureFact('notDelivered'), { surface: 'rejection' }) +} + +/** A Codex hook blocked it: a rejection of its own kind, with the host's sentence beside it. */ +const HOOK_BLOCKED: AgentJournalSubmission = { + ...UNDELIVERED, + ...agentSessionFailureWords( + agentSessionFailureFact('hookBlocked', { + detail: { text: 'No secrets in prompts.', audience: 'person' } + }), + { surface: 'rejection', agentName: 'Codex' } + ) +} + +function snapshot(submission: AgentJournalSubmission = UNDELIVERED): AgentSessionSubscribeEvent { + return { + type: 'snapshot', + sessionId: 'session-1', + fence: 3, + page: { + sessionId: 'session-1', + epoch: 'epoch-1', + fence: 3, + direction: 'tail', + items: [MESSAGE], + removedItemIds: [], + submissions: [submission], + window: { + oldest: { epoch: 'epoch-1', sequence: 1 }, + newest: { epoch: 'epoch-1', sequence: 1 }, + nextCursor: { epoch: 'epoch-1', sequence: 2 } + }, + liveCursor: { epoch: 'epoch-1', sequence: 1 }, + hasOlder: false, + hasNewer: false + } + } +} + +// Module scope: the harness assigns these, which a test body's own `let` would narrow to null. +let renderer: ReactTestRenderer | null = null +let hook: ReturnType | null = null +let listener: ((value: unknown) => void) | null = null +afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + hook = null + listener = null +}) + +/** The messages the phone's chat draws once the host's page holds this submission. */ +async function phoneMessages(submission: AgentJournalSubmission) { + const client: RpcClient = { + sendRequest: async () => ({ + id: 'request-1', + ok: true, + result: {}, + _meta: { runtimeId: 'runtime-1' } + }), + subscribe: (_method, _params, onData) => { + listener = onData + return () => {} + }, + updateTerminalSubscriptionViewport: () => {}, + getState: () => 'connected', + getReconnectAttempt: () => 0, + getLastConnectedAt: () => null, + onStateChange: () => () => {}, + notifyForeground: () => {}, + close: () => {} + } + function Harness(): null { + hook = useMobileStructuredAgentSession({ + client, + sessionId: 'session-1', + sourceIdentity: 'host-a\0workspace-a', + enabled: true, + connected: true, + agent: 'claude', + hostSupport: null, + onSendError: vi.fn() + }) + return null + } + + await act(async () => { + renderer = create(createElement(Harness)) + }) + await vi.waitFor(() => expect(listener).toEqual(expect.any(Function))) + act(() => listener?.(snapshot(submission))) + return hook?.session.messages +} + +// The phone does not mark a message as unsent yet; drawn, it would look delivered. +it('keeps a message the agent never got hidden, as before', async () => { + expect(await phoneMessages(UNDELIVERED)).toEqual([]) +}) + +// Before the block was known it was drawn as sent; it stays so rather than vanish. +it('draws a message a Codex hook blocked as sent, as before', async () => { + const messages = await phoneMessages(HOOK_BLOCKED) + expect(messages?.map(({ id }) => id)).toEqual([agentJournalSubmissionKey('op-undelivered')]) + expect(messages?.[0]).not.toHaveProperty('unsent') +}) diff --git a/src/main/codex/codex-structured-dispatch-echo.ts b/src/main/codex/codex-structured-dispatch-echo.ts index 4ad6d58faedc..036cad7a053d 100644 --- a/src/main/codex/codex-structured-dispatch-echo.ts +++ b/src/main/codex/codex-structured-dispatch-echo.ts @@ -3,6 +3,7 @@ import type { AgentJournalItemIdentity, AgentJournalTurnJoin } from '../../shared/agent-session-journal-types' +import type { CodexPromptBlock } from './codex-structured-prompt-block' /** Sends awaiting their echo. One bound to a turn that ended without taking it settles from that * end; any other whose echo never arrives is retired by the journal's recovery on exit. */ @@ -11,9 +12,10 @@ export const MAX_CODEX_PENDING_DISPATCH_ECHOES = 256 * turns kept that Codex left unopened through a wait. */ export const MAX_CODEX_RECORDED_TURN_ENDS = 64 -/** How a primary-thread turn ended, as Codex reported it. */ +/** How a primary-thread turn ended, as Codex reported it. `blocked`: a Codex hook blocked a prompt + * in that turn, which Codex then never recorded. */ export type CodexTurnEnd = - | { status: 'completed' } + | { status: 'completed'; blocked?: CodexPromptBlock } | { status: 'interrupted' } | { status: 'failed'; detail?: ProviderDiagnostic } @@ -52,12 +54,18 @@ export type CodexDispatchEchoes = { answeredUnopenedTurn: (threadId: string, openTurnIds: ReadonlySet) => string | null /** Codex did not open this answered turn within a wait, so no later wait is spent on it. */ leftUnopened: (threadId: string, turnId: string) => void + /** Codex reported a Codex hook blocked a prompt in this turn, kept only until the turn ends: the + * first message to the person and the first block reason any of its hooks gave. */ + blockPrompt: (threadId: string, turnId: string, block: CodexPromptBlock) => void + /** The block reported in this turn, which its end takes; null when none was. */ + takePromptBlock: (threadId: string, turnId: string) => CodexPromptBlock | null /** The latest armed send's answered turn a wait left unopened, neither open nor ended: Codex may * still open it, though no wait is spent on it again. */ answeredTurnLeftUnopened: (threadId: string, openTurnIds: ReadonlySet) => string | null /** * Records a turn's end and returns the sends bound to it that it settles: all of them unless it - * completed, which echoes its pending input first, so one it never echoed waits for recovery. An + * completed, which echoes its pending input first, so one it never echoed waits for recovery, + * unless a hook blocked a prompt in it: Codex records nothing it blocked. An * interrupt withdraws an un-echoed send, steered or the turn's own input: neither reached history. */ endTurn: ( @@ -84,9 +92,11 @@ export function createCodexDispatchEchoes(): CodexDispatchEchoes { >() const endedTurns = new Map() const unopenedTurns = new Set() + const blockedTurns = new Map() let nextSequence = 0 const turnKey = (threadId: string, turnId: string): string => JSON.stringify([threadId, turnId]) - const settles = (end: CodexTurnEnd): boolean => end.status !== 'completed' + const settles = (end: CodexTurnEnd): boolean => + end.status !== 'completed' || end.blocked !== undefined const answeredTurn = ( threadId: string, openTurnIds: ReadonlySet, @@ -142,6 +152,28 @@ export function createCodexDispatchEchoes(): CodexDispatchEchoes { unopenedTurns.delete(oldest) } }, + blockPrompt: (threadId, turnId, block) => { + const turn = turnKey(threadId, turnId) + // Codex reports hooks in their configured order and stops for the first block reason; a + // hook's message to the person is never that reason, so each keeps its first separately. + const earlier = blockedTurns.get(turn) + const warning = earlier?.warning ?? block.warning + const stop = earlier?.stop ?? block.stop + blockedTurns.delete(turn) + blockedTurns.set(turn, { ...(warning ? { warning } : {}), ...(stop ? { stop } : {}) }) + for (const oldest of blockedTurns.keys()) { + if (blockedTurns.size <= MAX_CODEX_RECORDED_TURN_ENDS) { + break + } + blockedTurns.delete(oldest) + } + }, + takePromptBlock: (threadId, turnId) => { + const turn = turnKey(threadId, turnId) + const block = blockedTurns.get(turn) ?? null + blockedTurns.delete(turn) + return block + }, endTurn: (threadId, turnId, end) => { const turn = turnKey(threadId, turnId) endedTurns.delete(turn) @@ -176,6 +208,7 @@ export function createCodexDispatchEchoes(): CodexDispatchEchoes { armed.clear() endedTurns.clear() unopenedTurns.clear() + blockedTurns.clear() nextSequence = 0 }, get size() { diff --git a/src/main/codex/codex-structured-prompt-block.ts b/src/main/codex/codex-structured-prompt-block.ts new file mode 100644 index 000000000000..6d793cc45456 --- /dev/null +++ b/src/main/codex/codex-structured-prompt-block.ts @@ -0,0 +1,116 @@ +// Codex's report that a Codex UserPromptSubmit hook (the person's, the project's, a plugin's or a +// managed one) blocked a prompt: a `hook/completed` frame inside the turn, before that turn's +// `turn/completed`. Codex records and echoes nothing for a blocked prompt, so the frame is the only +// proof it never reached history; the turn's end settles the sends it never echoed from it. The +// hook's reason is the words it wrote for the person, kept as plain text: never markup, no control +// or bidi characters, bounded. + +import type { CodexSession } from './codex-structured-session-state' +import { readCodexThreadId } from './codex-structured-thread-facts' + +/** Long enough for a sentence or two of why, in UTF-16 units; the hook's output stays in its logs. */ +export const MAX_CODEX_HOOK_REASON_CHARS = 300 + +// C0 and C1 controls, and every bidi mark, embedding, override and isolate. +const CONTROL_OR_BIDI = new RegExp( + // oxlint-disable-next-line no-control-regex -- stripping control characters is the point. + '[\\u0000-\\u001f\\u007f-\\u009f\\u061c\\u200e\\u200f\\u202a-\\u202e\\u2066-\\u2069]+', + 'g' +) +const ENDS_A_SENTENCE = /[.!?\u2026\u3002\uff01\uff1f\uff0e]$/ +const ELLIPSIS = '\u2026' +const WARNING: ReadonlySet = new Set(['warning']) +const BLOCK_REASON: ReadonlySet = new Set(['feedback', 'stop']) + +/** What a turn's blocking hooks said, kept apart until the turn ends: the first message to the + * person (`warning`) and the first reason for the block (`feedback` or `stop`). */ +export type CodexPromptBlock = { warning?: string; stop?: string } + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +function record(value: unknown): Record | null { + return isRecord(value) ? value : null +} + +/** One line of plain text: control and bidi characters become spaces, and a long one is cut on a + * whole code point. */ +export function plainCodexHookReason(text: string): string | undefined { + const plain = text.replace(CONTROL_OR_BIDI, ' ').replace(/\s+/g, ' ').trim() + if (!plain) { + return undefined + } + if (plain.length <= MAX_CODEX_HOOK_REASON_CHARS) { + return plain + } + let cut = '' + for (const codePoint of plain) { + if (cut.length + codePoint.length > MAX_CODEX_HOOK_REASON_CHARS - ELLIPSIS.length) { + break + } + cut += codePoint + } + return `${cut.trimEnd()}${ELLIPSIS}` +} + +function entryText(entry: Record | null): string | undefined { + return typeof entry?.text === 'string' ? plainCodexHookReason(entry.text) : undefined +} + +/** The block this frame reports, or null for any other frame. */ +export function readCodexPromptBlock( + method: string, + params: unknown +): ({ threadId: string; turnId: string } & CodexPromptBlock) | null { + const root = record(params) + const run = record(root?.run) + const threadId = readCodexThreadId(params) + const turnId = typeof root?.turnId === 'string' && root.turnId ? root.turnId : null + if ( + method !== 'hook/completed' || + !run || + run.eventName !== 'userPromptSubmit' || + (run.status !== 'blocked' && run.status !== 'stopped') || + !threadId || + !turnId + ) { + return null + } + // The hook's message to the person (`warning`), and why it blocked (`feedback` for a `blocked` + // run, `stop` for a `stopped` one). + const entries = (Array.isArray(run.entries) ? run.entries : []).map((entry) => record(entry)) + const firstText = (kinds: ReadonlySet) => + entries + .filter((entry) => kinds.has(entry?.kind)) + .map(entryText) + .find((text) => text !== undefined) + const warning = firstText(WARNING) + const stop = firstText(BLOCK_REASON) + return { threadId, turnId, ...(warning ? { warning } : {}), ...(stop ? { stop } : {}) } +} + +/** A turn's block as one reason, as Codex shows it: the message to the person, then why. */ +export function codexPromptBlockReason(block: CodexPromptBlock): string | undefined { + const parts = [block.warning, block.stop].filter((part): part is string => part !== undefined) + return plainCodexHookReason( + parts + .map((part, index) => + index < parts.length - 1 && !ENDS_A_SENTENCE.test(part) ? `${part}.` : part + ) + .join(' ') + ) +} + +/** Notes the primary thread's turn whose prompt a Codex hook blocked, until that turn ends. */ +export function noteCodexPromptBlock( + session: Pick, + method: string, + params: unknown +): void { + const block = readCodexPromptBlock(method, params) + if (block && block.threadId === session.threadId) { + const { threadId, turnId, ...said } = block + session.dispatchEchoes.blockPrompt(threadId, turnId, said) + } +} diff --git a/src/main/codex/codex-structured-session-adapter.ts b/src/main/codex/codex-structured-session-adapter.ts index 1c5a034fd63c..f3e22da76ead 100644 --- a/src/main/codex/codex-structured-session-adapter.ts +++ b/src/main/codex/codex-structured-session-adapter.ts @@ -36,6 +36,7 @@ import { settleCodexSendsInEndedTurn } from './codex-structured-turn-end-settlement' import { createCodexStructuredNotificationRetry } from './codex-structured-notification-retry' +import { noteCodexPromptBlock } from './codex-structured-prompt-block' import { acquireCodexStructuredSession } from './codex-structured-session-acquire' import { changeCodexThreadGoal } from './codex-structured-thread-goal' import { @@ -129,6 +130,7 @@ export class CodexStructuredSessionAdapter implements StructuredAgentSessionAdap return admission } if (event.type === 'notification') { + noteCodexPromptBlock(session, event.method, event.params) // Only an admitted turn end settles; a refused one settles on the retry that lands. settleCodexSendsInEndedTurn(session, event, (settlement) => this.deps.onDispatchSettledLate?.({ sessionId: event.sessionId, ...settlement }) diff --git a/src/main/codex/codex-structured-turn-end-settlement.ts b/src/main/codex/codex-structured-turn-end-settlement.ts index 376d9adb6e1f..e944086c9ac9 100644 --- a/src/main/codex/codex-structured-turn-end-settlement.ts +++ b/src/main/codex/codex-structured-turn-end-settlement.ts @@ -4,8 +4,11 @@ // or reaches only the model's context, and the thread history shows no user message either way. // So it is withdrawn, as a Stop's host-side withdrawal is. Any other end records pending // input before `turn/completed`, a failed turn after its `error` frame, so only that -// frame settles: a failed turn that never echoed the send refused it, in Codex's words, -// and a completed one leaves it pending for the journal's recovery on exit. +// frame settles: a failed turn that never echoed the send refused it, in Codex's words. A +// completed one settles it only when Codex reported a hook blocked a prompt in that turn +// (codex-structured-prompt-block): Codex records nothing it blocked, so the send was not +// delivered, for the hook's reason. Otherwise it stays pending for the journal's recovery on +// exit. import { agentSessionFailureFact, @@ -20,6 +23,7 @@ import { import type { AgentJournalAnsweredTurnIdentity } from '../../shared/agent-session-journal-types' import type { CodexTurnEnd } from './codex-structured-dispatch-echo' import { codexTurnLifecycleIdentity } from './codex-structured-journal-translation-turns' +import { codexPromptBlockReason } from './codex-structured-prompt-block' import type { CodexSession } from './codex-structured-session-state' import { readCodexThreadId, @@ -70,6 +74,11 @@ export function readCodexTurnEnd(method: string, params: unknown): CodexTurnEnd /** How an ended turn settles a send it never echoed; null leaves the send to its echo. */ export function codexTurnEndRejection(end: CodexTurnEnd): AgentJournalDispatchRejection | null { + if (end.status === 'completed' && end.blocked) { + const reason = codexPromptBlockReason(end.blocked) + const detail = reason ? providerDiagnostic(reason, 'person') : undefined + return codexDispatchRejection(agentSessionFailureFact('hookBlocked', detail ? { detail } : {})) + } if (end.status === 'interrupted') { return agentSessionFailureWords(agentSessionFailureFact('cancelled'), { surface: 'rejection' }) } @@ -88,14 +97,18 @@ export function settleCodexSendsInEndedTurn( settle: (settlement: CodexTurnEndSettlement) => void ): void { const turnId = readCodexTurnId(frame.params) - const end = readCodexTurnEnd(frame.method, frame.params) + const reported = readCodexTurnEnd(frame.method, frame.params) if ( !turnId || - !end || + !reported || (readCodexThreadId(frame.params) ?? session.threadId) !== session.threadId ) { return } + // Whatever the turn's end, a block it reported ends with it. + const blocked = session.dispatchEchoes.takePromptBlock(session.threadId, turnId) + const end: CodexTurnEnd = + reported.status === 'completed' && blocked ? { ...reported, blocked } : reported const rejection = codexTurnEndRejection(end) const turn = codexTurnLifecycleIdentity(frame.sessionId, turnId) for (const { clientMessageId, via } of session.dispatchEchoes.endTurn( diff --git a/src/main/runtime/structured-agent-session-codex-hook-blocked-send.test.ts b/src/main/runtime/structured-agent-session-codex-hook-blocked-send.test.ts new file mode 100644 index 000000000000..dbd0230fc5cb --- /dev/null +++ b/src/main/runtime/structured-agent-session-codex-hook-blocked-send.test.ts @@ -0,0 +1,532 @@ +// A Codex send a Codex UserPromptSubmit hook blocked (the person's, the project's or a managed +// one). Codex records and echoes +// nothing for a blocked prompt and still completes the turn, but reports the block itself as +// `hook/completed` inside that turn. Its completed end settles the send as blocked, with the +// hook's reason, so the chat stops reading as working. With no such report a completed turn +// settles nothing, and a send only Codex's echo settles is never settled early. Driven through the +// shipped host, journal and Codex adapter; only the Codex child is fake. + +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + CodexAppServerConnection, + CodexAppServerConnectionHandlers, + openCodexAppServerConnection +} from '../codex/codex-app-server-connection' +import { codexTurnLifecycleFake } from '../codex/codex-turn-lifecycle-fake' +import { computeAgentSessionPayloadFingerprint } from '../../shared/agent-session-mutation-envelope' +import type { AgentJournalSnapshot } from '../../shared/agent-session-journal-types' +import { agentJournalSubmissionKey } from '../../shared/agent-session-journal-item-key' +import { classifyDispatchRejection } from '../../shared/structured-agent-session-dispatch-rejection' +import { CodexAppServerRequestError } from '../codex/codex-app-server-connection' +import { MAX_CODEX_HOOK_REASON_CHARS } from '../codex/codex-structured-prompt-block' +import { owesStructuredAgentSessionWork } from '../../shared/structured-agent-session-owed-work' +import { projectStructuredAgentSessionStatusSummary } from '../../shared/structured-agent-session-projection' +import { + projectStructuredAgentSessionMessages, + structuredAgentSessionRejectedShownInPlace +} from '../../shared/structured-agent-session-message-projection' +import { createStructuredAgentSessionOutboxEntry } from '../../shared/structured-agent-session-outbox' +import { reconcileStructuredAgentSessionOutbox } from '../../shared/structured-agent-session-outbox-reconcile' +import { admitStructuredAgentSessionOutboxEntry } from '../../shared/structured-agent-session-outbox-admission' +import { readWholeAgentSessionFailureFact } from '../../shared/agent-session-failure' +import { + HOST_TEST_SESSION as SESSION, + HOST_TEST_THREAD as THREAD, + hostTestAttachParams, + hostTestMessage +} from '../native-chat/agent-session-wire/structured-agent-session-host-test-data' +import type { StructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-host' +import { + ensureStructuredAgentSessionHost, + stopStructuredAgentSessionRuntime +} from './structured-agent-session-runtime' +import { createStructuredAgentSessionLogger } from '../native-chat/agent-session-wire/structured-agent-session-logger' + +const CALLER = { callerKey: 'codex-hook-blocked-send-test' } +const MODEL = { + model: 'gpt-test', + displayName: 'GPT Test', + hidden: false, + supportedReasoningEfforts: [], + defaultReasoningEffort: null, + isDefault: true +} + +let root: string +let host: StructuredAgentSessionHost +let fence: number +let handlers: CodexAppServerConnectionHandlers | undefined +let answers: number +let steers: number +let turns: ReturnType +let operations = 0 +/** How Codex answers a start or a steer; a test may answer otherwise. */ +let routes: { + start: () => unknown + steer: (params: Record | undefined) => unknown +} + +/** The durable ledger stamps its own clock and refuses an id far from it. */ +const operationId = (): string => `${Date.now()}-${(++operations).toString(16).padStart(32, '0')}` + +async function send(text: string): Promise { + const body = hostTestMessage(text) + const sent = await host.send(CALLER, { + envelope: { + sessionId: SESSION, + clientOperationId: operationId(), + expectedRuntimeFence: fence, + payloadFingerprint: computeAgentSessionPayloadFingerprint({ + method: 'agentSession.send', + sessionId: SESSION, + fields: { body } + }) + }, + body + }) + if (!sent.ok) { + throw new Error(JSON.stringify(sent.refusal)) + } + return sent.value.clientMessageId +} + +function notify(method: string, params: Record): void { + handlers?.onNotification?.(method, params) +} + +/** Codex's report that a UserPromptSubmit hook blocked the prompt in this turn. */ +function hookBlocked( + turnId: string, + status: 'blocked' | 'stopped', + entries: { kind: string; text: string }[] +): void { + notify('hook/completed', { + threadId: THREAD, + turnId, + run: { + id: 'hook-1', + eventName: 'userPromptSubmit', + handlerType: 'command', + executionMode: 'sync', + scope: 'turn', + sourcePath: '/home/person/.codex/hooks/check.sh', + source: 'user', + displayOrder: 0, + status, + statusMessage: null, + startedAt: 1, + completedAt: 2, + durationMs: 1, + entries + } + }) +} + +async function snapshot(): Promise { + await host.flushStreamedEvents(SESSION) + return host.journalSnapshot(SESSION) +} + +function verdictOf(journal: AgentJournalSnapshot, clientMessageId: string) { + const submission = journal.submissions.find((entry) => entry.clientMessageId === clientMessageId) + return submission?.dispatchState === 'rejected' + ? classifyDispatchRejection(submission).kind + : submission?.dispatchState +} + +/** The shared rule every surface reads "working" from. */ +function working(journal: AgentJournalSnapshot): boolean { + return owesStructuredAgentSessionWork(journal.items, journal.submissions, fence) +} + +/** The opening send, echoed, in a running turn. */ +async function runningTurn(): Promise { + const opening = await send('look around') + await vi.waitFor(() => expect(answers).toBe(1)) + turns.start() + turns.echo(opening) +} + +/** A follow-up Codex answered as a steer into the running turn. */ +async function steered(text: string): Promise { + const before = steers + const followUp = await send(text) + await vi.waitFor(() => expect(steers).toBe(before + 1)) + return followUp +} + +/** The send settled as a hook's block, nothing reads as working, and the next send opens a turn. */ +async function blockedAndTheChatMovesOn(clientMessageId: string) { + await vi.waitFor(async () => + expect(verdictOf(await snapshot(), clientMessageId)).toBe('hookBlocked') + ) + const journal = await snapshot() + expect(working(journal)).toBe(false) + // Its sidebar row reads as the idle chat does, never Failed: the person's own hook refused it. + expect( + projectStructuredAgentSessionStatusSummary(journal.items, journal.submissions, fence) + ).toMatchObject({ status: 'idle' }) + expect( + projectStructuredAgentSessionStatusSummary(journal.items, journal.submissions, fence) + .turnOutcome + ).not.toBe('failure') + const before = answers + await send('carry on') + await vi.waitFor(() => expect(answers).toBe(before + 1)) + return journal.submissions.find((entry) => entry.clientMessageId === clientMessageId) +} + +/** The hook's reason as the host keeps it on the rejection: a diagnostic no client shows. */ +function reasonKept(journal: AgentJournalSnapshot, clientMessageId: string): string | undefined { + const row = journal.submissions.find((entry) => entry.clientMessageId === clientMessageId) + return readWholeAgentSessionFailureFact(row?.rejection)?.detail?.text +} + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-codex-hook-blocked-send-')) + answers = 0 + steers = 0 + routes = { + start: () => turns.routes['turn/start'](), + steer: (params) => turns.routes['turn/steer'](params) + } + turns = codexTurnLifecycleFake( + THREAD, + () => (method, params) => handlers?.onNotification?.(method, params) + ) + const openConnection: typeof openCodexAppServerConnection = async ( + _launch, + connectionHandlers = {} + ) => { + handlers = connectionHandlers + const connection: CodexAppServerConnection = { + pid: 4321, + closed: false, + request: async (method, params) => { + if (method === 'thread/start' || method === 'thread/resume') { + return { thread: { id: THREAD } } + } + if (method === 'model/list') { + return { data: [MODEL], nextCursor: null } + } + if (method === 'turn/start') { + answers += 1 + return routes.start() + } + if (method === 'turn/steer') { + steers += 1 + return routes.steer(params) + } + if (method === 'turn/interrupt') { + return turns.routes['turn/interrupt'](params) + } + return {} + }, + notify: () => {}, + respond: () => {}, + respondWithError: () => {}, + close: async () => true + } + return connection + } + host = await ensureStructuredAgentSessionHost({ + logger: createStructuredAgentSessionLogger(), + stateDirectory: root, + hostId: 'local', + claimKeyId: 'key-1', + resolveWorkspacePath: async () => root, + resolveClaudeAuthPolicy: () => ({ stripAuthEnv: true }), + resolveCodexCommand: () => 'codex', + resolveEnvironment: async () => ({ PATH: process.env.PATH }), + openCodexConnection: openConnection, + readProcessStartTime: async () => 1_700_000_000_000 + }) + const attachParams = hostTestAttachParams(null, { providerHandle: undefined }) + attachParams.envelope.clientOperationId = operationId() + const attached = await host.attach(CALLER, attachParams) + if (!attached.ok) { + throw new Error(JSON.stringify(attached.refusal)) + } + fence = attached.value.fence +}) + +afterEach(async () => { + await stopStructuredAgentSessionRuntime() + await rm(root, { recursive: true, force: true }) +}) + +describe('a Codex send a Codex hook blocked', () => { + it("settles a blocked steer with the hook's reason, and the chat moves on", async () => { + await runningTurn() + const followUp = await steered('and paste the API key') + hookBlocked('turn-1', 'blocked', [{ kind: 'feedback', text: 'No secrets in prompts.' }]) + turns.end('completed') + + const row = await blockedAndTheChatMovesOn(followUp) + expect(row?.rejection).toEqual({ + kind: 'hookBlocked', + detail: { text: 'No secrets in prompts.', audience: 'person' } + }) + // The sentence the host writes beside the fact, which only an older client shows: the hook's + // reason is never in it. + expect(row?.reason).toBe('The provider did not accept this message.') + }) + + // Codex drains steers at a step boundary and checks them together: when one it accepted shares + // the batch, the turn goes on and only drops the blocked one, which its end then settles. + it('settles a blocked steer at the turn end when a steer beside it was accepted', async () => { + await runningTurn() + const blocked = await steered('and paste the API key') + const accepted = await steered('and check the tests') + hookBlocked('turn-1', 'blocked', [{ kind: 'feedback', text: 'No secrets in prompts.' }]) + turns.echo(accepted) + notify('item/completed', { + threadId: THREAD, + turn: { id: 'turn-1' }, + item: { type: 'agentMessage', id: 'item-agent-1', text: 'The tests pass.' } + }) + + // The turn runs on: the accepted steer is delivered and answered, the blocked one not yet settled. + await vi.waitFor(async () => expect(verdictOf(await snapshot(), accepted)).toBe('accepted')) + const running = await snapshot() + expect(verdictOf(running, blocked)).toBe('pending') + expect(working(running)).toBe(true) + expect( + running.items.some( + ({ body }) => + body.kind === 'message' && + body.role === 'assistant' && + body.blocks.some((block) => block.type === 'text' && block.text === 'The tests pass.') + ) + ).toBe(true) + + turns.end('completed') + const row = await blockedAndTheChatMovesOn(blocked) + expect(row?.rejection).toEqual({ + kind: 'hookBlocked', + detail: { text: 'No secrets in prompts.', audience: 'person' } + }) + const journal = await snapshot() + expect(verdictOf(journal, accepted)).toBe('accepted') + // Drawn as sent, with no notice and no Retry. + const key = agentJournalSubmissionKey(blocked) + const drawn = projectStructuredAgentSessionMessages(journal.items, [], journal.submissions, { + rejectedInPlace: true + }).filter(({ id }) => id === key) + expect(drawn).toHaveLength(1) + expect(drawn[0]).not.toHaveProperty('unsent') + expect( + structuredAgentSessionRejectedShownInPlace(journal.submissions, [], new Set()).has(key) + ).toBe(false) + }) + + it('settles a blocked send that opened its turn the same way', async () => { + const opening = await send('paste the API key') + await vi.waitFor(() => expect(answers).toBe(1)) + turns.start() + hookBlocked('turn-1', 'blocked', [{ kind: 'feedback', text: 'No secrets in prompts.' }]) + turns.end('completed') + + await blockedAndTheChatMovesOn(opening) + expect(reasonKept(await snapshot(), opening)).toBe('No secrets in prompts.') + }) + + it('keeps no reason when the hook gave none', async () => { + await runningTurn() + const followUp = await steered('and check the tests') + hookBlocked('turn-1', 'stopped', []) + turns.end('completed') + + const row = await blockedAndTheChatMovesOn(followUp) + expect(row?.rejection).toEqual({ kind: 'hookBlocked' }) + }) + + it('keeps a long or marked-up reason plain and bounded, never splitting a character', async () => { + await runningTurn() + const followUp = await steered('and check the tests') + const reason = `Denied\n**by policy**${String.fromCharCode(0x07, 0x202e, 0x061c)} ${'x'.repeat(270)}${String.fromCodePoint(0x1f600)}${'y'.repeat(40)}` + hookBlocked('turn-1', 'stopped', [{ kind: 'stop', text: reason }]) + turns.end('completed') + + const row = await blockedAndTheChatMovesOn(followUp) + const text = readWholeAgentSessionFailureFact(row?.rejection)?.detail?.text ?? '' + expect(text.startsWith('Denied **by policy** x')).toBe(true) + expect(text.length).toBeLessThanOrEqual(MAX_CODEX_HOOK_REASON_CHARS) + expect(text.endsWith('…')).toBe(true) + // The emoji the cut fell on is dropped whole, never left as half a surrogate pair. + expect(/[\ud800-\udbff](?![\udc00-\udfff])/.test(text)).toBe(false) + const controlOrBidi = new RegExp( + // oxlint-disable-next-line no-control-regex -- asserting no control characters remain. + '[\\u0000-\\u001f\\u007f-\\u009f\\u061c\\u202a-\\u202e]' + ) + expect(controlOrBidi.test(text)).toBe(false) + }) + + // Codex's own example: the hook's message to the person, then why it stopped the prompt. + it("keeps the hook's message to the person ahead of its stop reason", async () => { + await runningTurn() + const followUp = await steered('start the go-workflow') + hookBlocked('turn-1', 'stopped', [ + { kind: 'warning', text: 'go-workflow must start from PlanMode' }, + { kind: 'stop', text: 'prompt blocked' } + ]) + turns.end('completed') + + await blockedAndTheChatMovesOn(followUp) + expect(reasonKept(await snapshot(), followUp)).toBe( + 'go-workflow must start from PlanMode. prompt blocked' + ) + }) + + // A hook's message to the person is never Codex's block reason: an earlier hook that only spoke + // to the person does not hide a later one's reason. + it("keeps a later hook's reason behind an earlier hook's message", async () => { + await runningTurn() + const followUp = await steered('and paste the API key') + hookBlocked('turn-1', 'stopped', [{ kind: 'warning', text: 'Heads up' }]) + hookBlocked('turn-1', 'blocked', [{ kind: 'feedback', text: 'No secrets in prompts' }]) + turns.end('completed') + + await blockedAndTheChatMovesOn(followUp) + expect(reasonKept(await snapshot(), followUp)).toBe('Heads up. No secrets in prompts') + }) + + // Codex stops for the first block reason a turn's hooks gave, in their configured order. + it('keeps the first reason a turn gave, and never trades it for none', async () => { + await runningTurn() + const followUp = await steered('and paste the API key!') + hookBlocked('turn-1', 'blocked', [{ kind: 'feedback', text: 'Never paste keys!' }]) + hookBlocked('turn-1', 'blocked', [{ kind: 'feedback', text: 'Second hook.' }]) + hookBlocked('turn-1', 'stopped', []) + turns.end('completed') + + await blockedAndTheChatMovesOn(followUp) + expect(reasonKept(await snapshot(), followUp)).toBe('Never paste keys!') + }) + + it('leaves a send a completed turn never echoed pending when no hook blocked it', async () => { + await runningTurn() + const followUp = await steered('and check the tests') + // A hook that ran and let the prompt through, and a block in another turn, block nothing here. + notify('hook/completed', { + threadId: THREAD, + turnId: 'turn-1', + run: { eventName: 'userPromptSubmit', status: 'completed', entries: [] } + }) + hookBlocked('turn-0', 'blocked', [{ kind: 'feedback', text: 'Not this one.' }]) + turns.end('completed') + + await snapshot() + expect(verdictOf(await snapshot(), followUp)).toBe('pending') + }) + + it('leaves a steer Codex echoed as delivered', async () => { + await runningTurn() + const followUp = await steered('and check the tests') + turns.echo(followUp) + turns.end('completed') + + await vi.waitFor(async () => expect(verdictOf(await snapshot(), followUp)).toBe('accepted')) + expect(working(await snapshot())).toBe(false) + }) + + // Codex may answer a send that falls through to a new turn before it opens that turn, and report + // the old turn's end and the thread idle in between. Only Codex's echo settles that send. + it('delivers a send answered into a turn that opens after the last one ended', async () => { + await runningTurn() + routes.steer = () => { + throw new CodexAppServerRequestError( + 'turn/steer', + -32600, + 'codex app-server turn/steer failed: no active turn to steer', + 'no active turn to steer' + ) + } + routes.start = () => ({ turn: { id: 'turn-2', status: 'inProgress' } }) + const followUp = await send('and check the tests') + await vi.waitFor(() => expect(answers).toBe(2)) + + turns.end('completed') + notify('thread/status/changed', { threadId: THREAD, status: { type: 'idle' } }) + await snapshot() + expect(verdictOf(await snapshot(), followUp)).toBe('pending') + + notify('turn/started', { threadId: THREAD, turn: { id: 'turn-2', status: 'inProgress' } }) + notify('item/completed', { + threadId: THREAD, + turn: { id: 'turn-2' }, + item: { type: 'userMessage', id: 'item-user-2', clientId: followUp, content: [] } + }) + await vi.waitFor(async () => expect(verdictOf(await snapshot(), followUp)).toBe('accepted')) + }) + + // The desktop reads the host's rows as it does for any message the agent never got. + it('is drawn as a sent message, never as not sent, on every desktop and the phone', async () => { + await runningTurn() + const followUp = await steered('and paste the API key') + hookBlocked('turn-1', 'blocked', [{ kind: 'feedback', text: 'No secrets in prompts.' }]) + turns.end('completed') + await vi.waitFor(async () => expect(verdictOf(await snapshot(), followUp)).toBe('hookBlocked')) + const journal = await snapshot() + const key = agentJournalSubmissionKey(followUp) + + // The sending desktop lets its copy go once the row that draws it is loaded, as for any rejection. + const queued = (clientMessageId: string, text: string) => + createStructuredAgentSessionOutboxEntry({ + clientMessageId, + sessionId: SESSION, + text, + attachments: [], + queuedAt: 1 + }) + const next = queued('op-next', 'carry on') + const outbox = reconcileStructuredAgentSessionOutbox( + [ + { ...queued(followUp, 'and paste the API key'), state: 'dispatching', lastAttemptAt: 1 }, + next + ], + journal.submissions, + journal.items + ) + expect(outbox).toEqual([next]) + expect(admitStructuredAgentSessionOutboxEntry(outbox)).toEqual({ + state: 'dispatch', + entry: next + }) + + // So every desktop draws it from its row as a sent message, exactly once: the sender before its + // outbox lets the copy go, after, and another desktop alike. + const unreconciled = [ + { + ...queued(followUp, 'and paste the API key'), + state: 'dispatching' as const, + lastAttemptAt: 1 + }, + next + ] + for (const sentHere of [unreconciled, outbox, []]) { + const rows = projectStructuredAgentSessionMessages( + journal.items, + sentHere, + journal.submissions, + { rejectedInPlace: true } + ) + const drawn = rows.filter(({ id }) => id === key) + expect(drawn).toHaveLength(1) + expect(drawn[0]).not.toHaveProperty('unsent') + } + // Not one the chat draws in place as not sent, so it carries no notice and no Retry. + expect( + structuredAgentSessionRejectedShownInPlace(journal.submissions, [], new Set()).has(key) + ).toBe(false) + // The phone draws it the same way. + const onPhone = projectStructuredAgentSessionMessages(journal.items, [], journal.submissions, { + rejectedInPlace: false + }).filter(({ id }) => id === key) + expect(onPhone).toHaveLength(1) + expect(onPhone[0]).not.toHaveProperty('unsent') + }) +}) diff --git a/src/renderer/src/components/native-chat/structured-agent-session-hook-blocked-message.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-hook-blocked-message.test.ts new file mode 100644 index 000000000000..43ef312ba514 --- /dev/null +++ b/src/renderer/src/components/native-chat/structured-agent-session-hook-blocked-message.test.ts @@ -0,0 +1,79 @@ +// A message the person's own agent hook refused is drawn as sent on the desktop: no notice, no +// Retry, and no "not sent", whether this desktop sent it or another device did. + +import { expect, it, vi } from 'vitest' +import { agentSessionFailureFact } from '../../../../shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../../shared/agent-session-failure-words' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import { createStructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' +import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' + +const ID = 'op-blocked' +const KEY = agentJournalSubmissionKey(ID) +const ITEM: AgentJournalRenderItem = { + itemId: KEY, + revision: 0, + sequence: 5, + observedAt: 5, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'paste the key' }] } +} +const BLOCKED: AgentJournalSubmission = { + clientMessageId: ID, + fence: 1, + payloadFingerprint: 'fingerprint', + dispatchState: 'rejected', + providerItemId: null, + submittedAt: 4, + resolvedAt: 7, + handoverRecorded: true, + handedOverAt: 5, + ...agentSessionFailureWords( + agentSessionFailureFact('hookBlocked', { + detail: { text: 'No secrets in prompts.', audience: 'person' } + }), + { surface: 'rejection', agentName: 'Codex' } + ) +} +/** This desktop's own copy, as the send left it before the rejection was read. */ +const OWN_COPY = { + ...createStructuredAgentSessionOutboxEntry({ + clientMessageId: ID, + sessionId: 'session-1', + text: 'paste the key', + attachments: [], + queuedAt: 4 + }), + state: 'dispatching' as const, + lastAttemptAt: 4 +} + +it.each([ + ['another device sent it', []], + ['this desktop sent it and still holds its copy', [OWN_COPY]] +])( + 'draws a hook-blocked message as sent, with no notice and no Retry, when %s', + (_case, outbox) => { + const rows = projectStructuredAgentSessionMessages([ITEM], outbox, [BLOCKED], []) + expect(rows.map(({ id }) => id)).toEqual([KEY]) + expect(rows[0]).not.toHaveProperty('unsent') + + const notices = structuredAgentSessionDeliveryNotices( + outbox, + 'Codex', + vi.fn(), + [BLOCKED], + [], + new Set(), + [], + new Set(), + [ITEM], + true + ) + expect(notices.get(KEY)).toBeUndefined() + } +) diff --git a/src/shared/agent-session-failure-words.ts b/src/shared/agent-session-failure-words.ts index 720389fb32d2..90fdb0292389 100644 --- a/src/shared/agent-session-failure-words.ts +++ b/src/shared/agent-session-failure-words.ts @@ -240,6 +240,9 @@ const FAILURE_SENTENCES = { restartFailed: couldNot('couldNotRestart'), providerRejected: (_context, fact, _surface, say) => quotingPersonDetail(say, 'providerRejected', 'providerRejectedQuoted', fact.detail), + // The person's own hook refused it: drawn as sent, so only an older client shows this, as the + // agent's refusal. The hook's reason stays on the fact, unshown. + hookBlocked: (_context, _fact, _surface, say) => say('providerRejected'), attachmentInvalid: (context, fact, _surface, say) => fact.attachment ? ATTACHMENT_SENTENCES[fact.attachment.reason](say, context, fact.attachment) diff --git a/src/shared/agent-session-failure.ts b/src/shared/agent-session-failure.ts index e92333c2d931..b19dc1da19f2 100644 --- a/src/shared/agent-session-failure.ts +++ b/src/shared/agent-session-failure.ts @@ -28,6 +28,9 @@ export const AGENT_SESSION_FAILURE_KINDS = [ 'providerExited', 'restartFailed', 'providerRejected', + /** A hook of the agent's (the person's, the project's or a managed one) blocked the message + * before the agent recorded it. */ + 'hookBlocked', 'attachmentInvalid', 'attachmentUnreadable', 'emptyMessage', diff --git a/src/shared/structured-agent-session-dispatch-rejection.ts b/src/shared/structured-agent-session-dispatch-rejection.ts index d1334a4e0a85..b7d9970311ba 100644 --- a/src/shared/structured-agent-session-dispatch-rejection.ts +++ b/src/shared/structured-agent-session-dispatch-rejection.ts @@ -95,6 +95,7 @@ const KIND_CATEGORY = { managedAccountUnsupported: 'startFailed', restartFailed: 'startFailed', providerRejected: 'content', + hookBlocked: 'content', attachmentInvalid: 'content', attachmentUnreadable: 'content', emptyMessage: 'content', @@ -121,6 +122,8 @@ const KIND_VERDICT = { managedAccountUnsupported: 'failure', restartFailed: 'failure', providerRejected: 'failure', + // A queued card it blocked is returned for the person to edit, never re-sent into the same hook. + hookBlocked: 'failure', attachmentInvalid: 'failure', attachmentUnreadable: 'failure', emptyMessage: 'failure', @@ -252,6 +255,22 @@ export function dispatchWasWithdrawn( ) } +/** Rejected, yet every client draws it as a sent message: the person's own hook refused it once the + * agent had it, which is no failure to show, and its reason stays unshown. A queued card's + * message is the card's to show. */ +export function rejectionDrawnAsSent( + submission: Pick< + AgentJournalSubmission, + 'dispatchState' | 'reason' | 'rejection' | 'queuedMessageId' + > +): boolean { + return ( + submission.dispatchState === 'rejected' && + submission.queuedMessageId === undefined && + classifyDispatchRejection(submission).kind === 'hookBlocked' + ) +} + /** A submission that says Orca never handed it over, in any dispatch state: journals written * before this state moved hold it as `unknown` carrying the marker. */ export function isWriteFailureSubmission( diff --git a/src/shared/structured-agent-session-latest-request.test.ts b/src/shared/structured-agent-session-latest-request.test.ts index 4cab89fca213..bbda6b294009 100644 --- a/src/shared/structured-agent-session-latest-request.test.ts +++ b/src/shared/structured-agent-session-latest-request.test.ts @@ -4,7 +4,12 @@ import type { AgentJournalSubmission, AgentJournalTurnLifecycle } from './agent-session-journal-types' -import { AGENT_SESSION_FAILURE_KINDS, isSubmissionRejectionKind } from './agent-session-failure' +import { + AGENT_SESSION_FAILURE_KINDS, + agentSessionFailureFact, + isSubmissionRejectionKind +} from './agent-session-failure' +import { agentSessionFailureWords } from './agent-session-failure-words' import { agentJournalSubmissionKey } from './agent-session-journal-item-key' import { classifyDispatchRejection, @@ -83,6 +88,22 @@ const rejected = (clientMessageId: string, reason: string, handedOverAt?: number ...(handedOverAt !== undefined ? { handedOverAt } : {}) }) +/** Rejected with a typed fact, as the host writes it. */ +const rejectedWith = ( + clientMessageId: string, + fact: Parameters[0], + resolvedAt = 20 +) => + sent(clientMessageId, { + dispatchState: 'rejected', + resolvedAt, + ...agentSessionFailureWords(fact, { surface: 'rejection', agentName: 'Codex' }) + }) + +const HOOK_BLOCKED = agentSessionFailureFact('hookBlocked', { + detail: { text: 'No secrets in prompts.', audience: 'person' } +}) + type Row = { name: string items: AgentJournalRenderItem[] @@ -93,6 +114,37 @@ type Row = { } const ROWS: Row[] = [ + // Nothing failed: the person's own hook refused it, and the chat is idle. Drawn where it was + // rejected (after the turn's end), it is still not the news; the turn it was blocked in is. + { + name: 'a steer a Codex hook blocked, after the turn it joined completed (the turn, not Failed)', + items: [ + userEntry('m1', 1), + turn('t1', 2, { state: 'completed', outcome: 'success', completedAt: 15 }), + userEntry('m2', 3) + ], + submissions: [sent('m1', { dispatchState: 'accepted' }), rejectedWith('m2', HOOK_BLOCKED, 16)], + outcome: 'success', + listed: true + }, + { + name: 'a first message a Codex hook blocked, with the turn it opened (the turn, not Failed)', + items: [ + turn('t1', 2, { state: 'completed', outcome: 'success', completedAt: 15 }), + userEntry('m1', 3) + ], + submissions: [rejectedWith('m1', HOOK_BLOCKED, 16)], + outcome: 'success', + listed: true + }, + // Its tries ran out: it reads Failed, as before. + { + name: 'a start refused before it ran, its tries spent (Failed)', + items: [userEntry('m1', 1)], + submissions: [rejectedWith('m1', { kind: 'accountSwitchInProgress' })], + outcome: 'failure', + listed: true + }, { name: 'a send the agent start refused', items: [userEntry('m1', 1)], @@ -316,7 +368,10 @@ describe('the latest request and its verdict', () => { describe('the sidebar verdict agrees with the rejection classifier', () => { // A sentence as the reason, so only the typed fact can make a kind read as no verdict. - const typed = AGENT_SESSION_FAILURE_KINDS.filter(isSubmissionRejectionKind).map((kind) => ({ + // A hook's block is the one exception, pinned below. + const typed = AGENT_SESSION_FAILURE_KINDS.filter( + (kind) => isSubmissionRejectionKind(kind) && kind !== 'hookBlocked' + ).map((kind) => ({ name: `typed ${kind}`, submission: sent('m1', { dispatchState: 'rejected', @@ -350,6 +405,14 @@ describe('the sidebar verdict agrees with the rejection classifier', () => { expect(hasStructuredAgentSessionRequest(items, [submission])).toBe(verdict === 'failure') }) + // Returned as a card for the person to edit (a failure to the queue), but nothing broke, so it + // is no verdict of its own: the turn it was blocked in reads instead. + it('reads a send a hook blocked as no verdict, though the classifier calls it a failure', () => { + const submission = rejectedWith('m1', HOOK_BLOCKED) + expect(classifyDispatchRejection(submission).verdict).toBe('failure') + expect(latestStructuredAgentSessionRequest([userEntry('m1', 1)], [submission])).toBeNull() + }) + const failedNobody = new Set([ 'typed notDelivered', 'typed cancelled', diff --git a/src/shared/structured-agent-session-latest-request.ts b/src/shared/structured-agent-session-latest-request.ts index d16680f64b47..8a520e197a10 100644 --- a/src/shared/structured-agent-session-latest-request.ts +++ b/src/shared/structured-agent-session-latest-request.ts @@ -5,6 +5,7 @@ // A request is either a turn, whose record carries the provider's verdict, or a send that never // became one because the agent or its start refused it. A send its handover placed inside a // running turn (a steer) is not a request of its own: the turn it joined answers for it. Nor is a +// send the person's own hook blocked: the turn it was blocked in answers for it. Nor is a // conversation command. Nor is a message waiting out a refused start: a try is booked, so it has no // verdict yet, and the session reads as it did before it was sent. @@ -100,9 +101,12 @@ function requestOf( } } const submission = rejected.get(item.itemId) + const rejection = submission ? classifyDispatchRejection(submission) : null if ( - !submission || - classifyDispatchRejection(submission).verdict !== 'failure' || + !rejection || + rejection.verdict !== 'failure' || + // The person's own hook refused it: nothing failed, and the turn it was blocked in answers. + rejection.kind === 'hookBlocked' || // Handed into a running turn (a steer): that turn answers for it. item.turnScope?.kind === 'turn' ) { @@ -113,7 +117,7 @@ function requestOf( id: item.itemId, turnState: null, outcome: 'failure', - settledAt: submission.resolvedAt ?? undefined + settledAt: submission?.resolvedAt ?? undefined } } diff --git a/src/shared/structured-agent-session-message-projection.ts b/src/shared/structured-agent-session-message-projection.ts index ef7344444128..ec0281446a70 100644 --- a/src/shared/structured-agent-session-message-projection.ts +++ b/src/shared/structured-agent-session-message-projection.ts @@ -4,7 +4,10 @@ import { isQueuedAgentJournalSubmission } from './agent-session-queued-submissio import { isRetryingStructuredAgentSessionStart } from './structured-agent-session-start-retry' import { collapseProviderRetryRuns } from './native-chat-provider-retry-runs' import type { NativeChatMessage } from './native-chat-types' -import { dispatchWasWithdrawn } from './structured-agent-session-dispatch-rejection' +import { + dispatchWasWithdrawn, + rejectionDrawnAsSent +} from './structured-agent-session-dispatch-rejection' import type { StructuredAgentSessionOutboxEntry } from './structured-agent-session-outbox' import { structuredAgentSessionEntryHeldForRetry } from './structured-agent-session-outbox-admission' import { reconcileStructuredAgentSessionOutboxWithQueue } from './structured-agent-session-draft-hand-off' @@ -37,7 +40,8 @@ export function structuredAgentSessionCommandItemIds( * The rejected submissions the host's history shows in place as not sent, by item id; `submissions` * in submission order, as the client keeps them. A withdrawn one went back to its sender, one the * queue holds (a draft's hand-off, or a card under its id) is drawn as its card, and a command such - * as `/compact` has its rejection reported as its own reply. + * as `/compact` has its rejection reported as its own reply. One the person's own hook refused is + * drawn as sent instead (`rejectionDrawnAsSent`). */ export function structuredAgentSessionRejectedShownInPlace( submissions: readonly AgentJournalSubmission[], @@ -65,6 +69,7 @@ export function structuredAgentSessionRejectedShownInPlace( if ( submission.dispatchState !== 'rejected' || dispatchWasWithdrawn(submission) || + rejectionDrawnAsSent(submission) || submission.queuedMessageId !== undefined || cards.has(submission.clientMessageId) || commandItemIds.has(agentJournalSubmissionKey(submission.clientMessageId)) || @@ -113,12 +118,22 @@ export function projectStructuredAgentSessionMessages( ) : [] ) + // Drawn as sent on every client, where the journal placed it (`rejectionDrawnAsSent`). + const shownAsSent = new Set( + submissions + .filter(rejectionDrawnAsSent) + .map((submission) => agentJournalSubmissionKey(submission.clientMessageId)) + ) const visibleItems: AgentJournalRenderItem[] = [] const unsentItems: AgentJournalRenderItem[] = [] for (const item of items) { if (inPlace.has(item.itemId)) { unsentItems.push(item) - } else if (!rejected.has(item.itemId) || unsentElsewhere.has(item.itemId)) { + } else if ( + !rejected.has(item.itemId) || + unsentElsewhere.has(item.itemId) || + shownAsSent.has(item.itemId) + ) { visibleItems.push(item) } } diff --git a/src/shared/structured-agent-session-send-disposition.test.ts b/src/shared/structured-agent-session-send-disposition.test.ts index 9605764b0179..d6db103da25d 100644 --- a/src/shared/structured-agent-session-send-disposition.test.ts +++ b/src/shared/structured-agent-session-send-disposition.test.ts @@ -4,8 +4,10 @@ // which is the whole reason it was split out. import { describe, expect, it } from 'vitest' -import type { AgentSessionFailureFact } from './agent-session-failure' -import type { AgentJournalSubmission } from './agent-session-journal-types' +import { agentSessionFailureFact, type AgentSessionFailureFact } from './agent-session-failure' +import { agentSessionFailureWords } from './agent-session-failure-words' +import { agentJournalSubmissionKey } from './agent-session-journal-item-key' +import type { AgentJournalRenderItem, AgentJournalSubmission } from './agent-session-journal-types' import type { AgentSessionMutationResult, AgentSessionSendResult } from './agent-session-wire' import { DISPATCH_REJECTED_CANCELLED, @@ -23,6 +25,7 @@ import { } from './structured-agent-session-outbox' import { reconcileStructuredAgentSessionOutbox } from './structured-agent-session-outbox-reconcile' import { structuredAgentSessionEntryHeldForRetry } from './structured-agent-session-outbox-admission' +import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' const entry: StructuredAgentSessionOutboxEntry = createStructuredAgentSessionOutboxEntry({ clientMessageId: 'client-1', @@ -113,6 +116,45 @@ describe('a queued draft answer', () => { }) }) +describe('a send its own hook blocked', () => { + it('leaves no not-sent copy, before or after its row loads', () => { + const blocked = agentSessionFailureWords( + agentSessionFailureFact('hookBlocked', { + detail: { text: 'No secrets in prompts.', audience: 'person' } + }), + { surface: 'rejection', agentName: 'Codex' } + ) + const result = rejectedWith(blocked.reason, { rejection: blocked.rejection }) + if (!result.ok || !('submission' in result.value)) { + throw new Error('expected rejected submission fixture') + } + const disposition = disposeStructuredAgentSessionSendResult({ + entries: [entry], + entry, + result, + createOperationId: () => 'unused' + }) + // Before its row loads: no outbox copy to draw as not sent, and no notice. + expect(disposition).toEqual({ entries: [], error: null }) + // After: the row draws it once, as sent. + const item: AgentJournalRenderItem = { + itemId: agentJournalSubmissionKey('client-1'), + revision: 0, + sequence: 10, + observedAt: 10, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'hello' }] } + } + const drawn = projectStructuredAgentSessionMessages( + [item], + disposition.entries, + [result.value.submission], + { rejectedInPlace: true } + ).filter(({ id }) => id === item.itemId) + expect(drawn).toHaveLength(1) + expect(drawn[0]).not.toHaveProperty('unsent') + }) +}) + describe('what a rejection shows the user', () => { it('removes a queued message the provider confirms Stop cancelled', () => { const result = rejectedWith(DISPATCH_REJECTED_CANCELLED) diff --git a/src/shared/structured-agent-session-send-disposition.ts b/src/shared/structured-agent-session-send-disposition.ts index b4a4612b10e9..397dfac63354 100644 --- a/src/shared/structured-agent-session-send-disposition.ts +++ b/src/shared/structured-agent-session-send-disposition.ts @@ -21,7 +21,10 @@ import { } from './agent-session-write-failure' import type { AgentSessionFailureFact } from './agent-session-failure' import type { AgentSessionFailureWordsContext } from './agent-session-failure-words' -import { classifyDispatchRejection } from './structured-agent-session-dispatch-rejection' +import { + classifyDispatchRejection, + rejectionDrawnAsSent +} from './structured-agent-session-dispatch-rejection' import { classifyStructuredAgentSessionSendFailure, requeueStructuredAgentSessionSendRefusal, @@ -261,7 +264,8 @@ export function disposeStructuredAgentSessionSendResult( error: ['sendOutcomeLost'] } } - if (submission.dispatchState === 'accepted') { + // One its own hook blocked is drawn as sent from its row: a not-sent copy until then would flash. + if (submission.dispatchState === 'accepted' || rejectionDrawnAsSent(submission)) { return { entries: dropEntry(input), error: null