From add99c908b410017cb040a9782f4613fbb86d24f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:54:46 -0700 Subject: [PATCH] fix(native-chat): send typed question answers as structured answers, not an option id (#22793) * fix(native-chat): send typed question answers as structured answers, not an option id A typed "Other" answer was packed into the `optionId` of agentSession.respondToQuestion, a field capped at 1024 characters, so a long answer failed with "Invalid option id" and never reached the agent. respondToQuestion now carries per-question `answers` in their own field, bounded like a typed answer, and a host advertises agent-session.question-answers.v1 when it takes them. Clients fall back to the packed option id for older hosts. The host reads either form once into a typed response, records the structured answers on the resolution (and keeps the packed form older clients read), and the Claude and Codex adapters build their reply from the typed answers before the journal commits, so an answer the agent cannot take is refused rather than recorded unanswered. Co-Authored-By: Claude * fix(native-chat): hold one answer per single-select question card Typing an answer deselects a picked option, and picking an option leaves the typed text in the field without sending it, so the card never shows two answers while sending one. Multi-select still sends picked options and typed text together. * fix(native-chat): keep keyboard tabbing from re-choosing a typed answer; accept untrimmed question ids Clicking or typing in the answer field chooses the typed answer; focus alone no longer does, so tabbing to Submit keeps the option the user picked. A question id is matched exactly by the host, so the wire no longer rejects agent-written ids with edge spaces, which older builds accepted. * fix(native-chat): choose the typed answer on click so a disabled or scrolled field cannot * test(native-chat): cover pointer events on a disabled answer field * refactor(native-chat): record the typed answer as a choice in the question card Choosing the typed answer is now an entry in the question's selection, set by typing or clicking the field and replaced by picking an option, instead of being inferred from an empty selection. Unpicking an option no longer silently chooses kept text. --------- Co-authored-by: Claude --- src/main/claude/claude-prompt-journaling.ts | 2 +- src/main/claude/claude-prompt-registry.ts | 22 +-- .../claude-structured-control-actions.test.ts | 8 +- .../claude-structured-control-actions.ts | 12 +- ...de-structured-journal-prompt-retry.test.ts | 1 - .../claude-structured-journal-prompts.ts | 2 +- ...ude-structured-journal-translation.test.ts | 1 - .../claude-structured-journal-translation.ts | 5 +- .../claude-structured-prompt-items.test.ts | 61 ++++--- ...claude-structured-prompt-ownership.test.ts | 15 +- .../claude-structured-prompt-ownership.ts | 20 ++- .../claude-structured-prompt-replies.ts | 81 +++------ .../claude-structured-session-adapter.test.ts | 74 +++++--- .../claude-structured-session-adapter.ts | 8 +- .../codex-structured-prompt-ownership.test.ts | 18 +- .../codex-structured-prompt-ownership.ts | 24 ++- .../codex-structured-prompt-replies.test.ts | 59 +++++-- .../codex/codex-structured-prompt-replies.ts | 73 +++++--- ...ructured-session-adapter-lifecycle.test.ts | 2 +- .../codex-structured-session-adapter.test.ts | 27 +-- .../structured-agent-session-adapter.ts | 16 +- ...tured-agent-session-grouped-prompt.test.ts | 65 ++++++- ...structured-agent-session-host-mutations.ts | 9 +- ...structured-agent-session-mutation-plans.ts | 14 +- .../structured-agent-session-turns-prompt.ts | 100 +++++++---- ...tured-agent-session-respond-params.test.ts | 106 +++++++++++ .../structured-agent-session-schemas.ts | 1 + .../methods/structured-agent-session.test.ts | 35 ---- .../rpc/methods/structured-agent-session.ts | 3 +- .../NativeChatQuestionCard.test.tsx | 134 ++++++++++++++ .../native-chat/NativeChatQuestionCard.tsx | 70 +++++++- .../NativeChatResolutionReceipt.test.tsx | 18 ++ .../NativeChatStructuredSession.test.tsx | 30 +++- .../NativeChatStructuredSession.tsx | 62 ++----- .../native-chat-resolution-receipt.ts | 33 ++-- ...ed-agent-session-question-answers.test.tsx | 164 ++++++++++++++++++ .../use-structured-agent-session.ts | 35 +++- .../structured-agent-session-client.ts | 10 ++ src/shared/agent-session-journal-schemas.ts | 9 + src/shared/agent-session-journal-types.ts | 6 +- src/shared/agent-session-question-answer.ts | 98 ++++++++++- src/shared/protocol-version.ts | 5 + .../rpc-params-catalog.generated.ts | 3 +- .../structured-agent-session-params.ts | 49 ++++++ ...ss-version-agent-session-wire.unit.test.ts | 37 ++++ 45 files changed, 1222 insertions(+), 405 deletions(-) create mode 100644 src/main/runtime/rpc/methods/structured-agent-session-respond-params.test.ts create mode 100644 src/renderer/src/components/native-chat/use-structured-agent-session-question-answers.test.tsx diff --git a/src/main/claude/claude-prompt-journaling.ts b/src/main/claude/claude-prompt-journaling.ts index 99b163fe197..6d3bfe148cf 100644 --- a/src/main/claude/claude-prompt-journaling.ts +++ b/src/main/claude/claude-prompt-journaling.ts @@ -13,7 +13,7 @@ import { export type ClaudePromptJournalDeps = { sink: StructuredAgentSessionEventSink - bindPromptItemId?: (journalItemId: string, promptKey: string, questionId?: string) => void + bindPromptItemId?: (journalItemId: string, promptKey: string) => void /** Prompt key → the rows it wrote, owned by the translator so a cancel can sweep them. */ promptItems: Map } diff --git a/src/main/claude/claude-prompt-registry.ts b/src/main/claude/claude-prompt-registry.ts index 3437dba93dc..5cd2f6b1a7b 100644 --- a/src/main/claude/claude-prompt-registry.ts +++ b/src/main/claude/claude-prompt-registry.ts @@ -26,7 +26,6 @@ export type ClaudePendingPrompt = ClaudePromptPresentation & { input: Record suggestions: PermissionUpdate[] questionIds: readonly string[] - answers: Map settle: ClaudePromptSettle turnId?: string | null } @@ -43,13 +42,12 @@ export type ClaudePromptRegistration = ClaudePromptPresentation & { type PromptBinding = { address: string - questionId?: string turnId: string | null } export type ClaudePromptClaim = { readonly itemId: string - readonly found: { prompt: ClaudePendingPrompt; questionId?: string } + readonly found: { prompt: ClaudePendingPrompt } } type ClaudePromptCancellationObservation = { @@ -111,7 +109,6 @@ export class ClaudePromptRegistry { ...(registration.matchedAskRule ? { matchedAskRule: registration.matchedAskRule } : {}), ...(registration.subject ? { subject: registration.subject } : {}), questionIds: questions.map(questionId), - answers: new Map(), settle: registration.settle, turnId: registration.turnId ?? null } @@ -130,26 +127,18 @@ export class ClaudePromptRegistry { return true } - bindJournalItemId( - journalItemId: string, - promptKey: string, - questionIdForItem?: string, - turnId: string | null = null - ): void { + bindJournalItemId(journalItemId: string, promptKey: string, turnId: string | null = null): void { const prompt = this.prompts.get(promptKey) this.journalBindings.set(journalItemId, { address: promptKey, - ...(questionIdForItem ? { questionId: questionIdForItem } : {}), turnId: turnId ?? prompt?.turnId ?? null }) } - find(itemId: string): { prompt: ClaudePendingPrompt; questionId?: string } | null { + find(itemId: string): { prompt: ClaudePendingPrompt } | null { const binding = this.journalBindings.get(itemId) const prompt = this.prompts.get(binding?.address ?? itemId) - return prompt - ? { prompt, ...(binding?.questionId ? { questionId: binding.questionId } : {}) } - : null + return prompt ? { prompt } : null } claim(itemId: string, kind?: 'approval' | 'question'): ClaudePromptClaim | null { @@ -168,8 +157,7 @@ export class ClaudePromptRegistry { if (!binding || !prompt || binding.turnId !== turnId || this.claims.has(prompt)) { return null } - const found = { prompt, ...(binding.questionId ? { questionId: binding.questionId } : {}) } - const claim = { itemId, found } + const claim = { itemId, found: { prompt } } this.claims.set(prompt, claim) return claim } diff --git a/src/main/claude/claude-structured-control-actions.test.ts b/src/main/claude/claude-structured-control-actions.test.ts index f92a04a8003..a04273ac126 100644 --- a/src/main/claude/claude-structured-control-actions.test.ts +++ b/src/main/claude/claude-structured-control-actions.test.ts @@ -6,7 +6,7 @@ import { } from './claude-structured-control-actions' import { dispatchClaudeTurn } from './claude-structured-dispatch' import { ClaudeControlRequestError } from './claude-stream-json-connection' -import { ClaudePromptRegistry } from './claude-structured-prompt-replies' +import { buildClaudePromptReply, ClaudePromptRegistry } from './claude-structured-prompt-replies' import type { ClaudeDispatchWaiter, ClaudeSession } from './claude-structured-session-state' import { ClaudeBackgroundTaskTracker } from './claude-background-task-tracker' import { sessionFor, userMessage } from './claude-structured-dispatch-test-support' @@ -214,7 +214,11 @@ describe('answerClaudePrompt', () => { if (!claim) { throw new Error('expected prompt claim') } - await answerClaudePrompt(session, claim, 'allow') + await answerClaudePrompt( + session, + claim, + buildClaudePromptReply(prompt, { kind: 'option', optionId: 'allow' }) + ) expect(settle).toHaveBeenCalledWith( expect.objectContaining({ behavior: 'allow', toolUseID: 'tool-1' }) diff --git a/src/main/claude/claude-structured-control-actions.ts b/src/main/claude/claude-structured-control-actions.ts index 961b9aee7ba..5e5906c4216 100644 --- a/src/main/claude/claude-structured-control-actions.ts +++ b/src/main/claude/claude-structured-control-actions.ts @@ -1,4 +1,5 @@ -import { applyClaudePromptAnswer, type ClaudePromptClaim } from './claude-structured-prompt-replies' +import type { PermissionResult } from '@anthropic-ai/claude-agent-sdk' +import type { ClaudePromptClaim } from './claude-structured-prompt-replies' import { ClaudeControlRequestError } from './claude-stream-json-connection' import { settleCancelledClaudeDispatchWaiters, @@ -83,17 +84,12 @@ export async function stopClaudeBackgroundTasks( export async function answerClaudePrompt( session: ClaudeSession, claim: ClaudePromptClaim, - optionId: string + reply: PermissionResult ): Promise { if (!session.prompts.ownsClaim(claim)) { throw new Error(`claude is no longer waiting on ${claim.itemId}`) } - const response = applyClaudePromptAnswer(claim.found, optionId) - if (response === null) { - session.prompts.releaseClaim(claim) - return - } session.prompts.forget(claim.found.prompt) - claim.found.prompt.settle(response) + claim.found.prompt.settle(reply) session.translator?.journalPrompts.resolve(claim.found.prompt.promptKey) } diff --git a/src/main/claude/claude-structured-journal-prompt-retry.test.ts b/src/main/claude/claude-structured-journal-prompt-retry.test.ts index 491152668ed..d22362467e9 100644 --- a/src/main/claude/claude-structured-journal-prompt-retry.test.ts +++ b/src/main/claude/claude-structured-journal-prompt-retry.test.ts @@ -18,7 +18,6 @@ function approval(promptKey: string): ClaudePendingPrompt { input: { command: 'git status' }, suggestions: [], questionIds: [], - answers: new Map(), settle: () => {} } } diff --git a/src/main/claude/claude-structured-journal-prompts.ts b/src/main/claude/claude-structured-journal-prompts.ts index 0d2509723f6..73dd35071ba 100644 --- a/src/main/claude/claude-structured-journal-prompts.ts +++ b/src/main/claude/claude-structured-journal-prompts.ts @@ -54,7 +54,7 @@ export class ClaudeJournalPrompts { constructor( private readonly deps: { sink: StructuredAgentSessionEventSink - bindPromptItemId?: (journalItemId: string, promptKey: string, questionId?: string) => void + bindPromptItemId?: (journalItemId: string, promptKey: string) => void questionItems?: (input: { sessionId: string prompt: Extract['prompt'] diff --git a/src/main/claude/claude-structured-journal-translation.test.ts b/src/main/claude/claude-structured-journal-translation.test.ts index 574547ee731..d868b3816c4 100644 --- a/src/main/claude/claude-structured-journal-translation.test.ts +++ b/src/main/claude/claude-structured-journal-translation.test.ts @@ -869,7 +869,6 @@ function prompt( return { ...input, suggestions: [], - answers: new Map(), settle: () => {} } } diff --git a/src/main/claude/claude-structured-journal-translation.ts b/src/main/claude/claude-structured-journal-translation.ts index c18392e1aa9..16c054945a1 100644 --- a/src/main/claude/claude-structured-journal-translation.ts +++ b/src/main/claude/claude-structured-journal-translation.ts @@ -38,7 +38,7 @@ export type { ClaudeJournalTranslator } from './claude-journal-translator-contra export type ClaudeJournalTranslatorDeps = { sink: StructuredAgentSessionEventSink - bindPromptItemId?: (journalItemId: string, promptKey: string, questionId?: string) => void + bindPromptItemId?: (journalItemId: string, promptKey: string) => void coalesceMs?: number schedule?: AgentSessionDeltaCoalescerDeps['schedule'] fallbackIdPrefix?: string @@ -56,8 +56,7 @@ export function createClaudeSessionJournalTranslator( sink, fallbackIdPrefix, ...(onBackgroundTaskJournalFailure ? { onBackgroundTaskJournalFailure } : {}), - bindPromptItemId: (itemId, promptKey, questionId) => - prompts.bindJournalItemId(itemId, promptKey, questionId) + bindPromptItemId: (itemId, promptKey) => prompts.bindJournalItemId(itemId, promptKey) }) : null } diff --git a/src/main/claude/claude-structured-prompt-items.test.ts b/src/main/claude/claude-structured-prompt-items.test.ts index 297bf405435..f032d4bcf8d 100644 --- a/src/main/claude/claude-structured-prompt-items.test.ts +++ b/src/main/claude/claude-structured-prompt-items.test.ts @@ -1,13 +1,11 @@ import { describe, expect, it } from 'vitest' import { agentJournalItemKey } from '../../shared/agent-session-journal-item-key' -import { encodeAgentSessionQuestionAnswers } from '../../shared/agent-session-question-answer' import { cancelledJournalPromptBody } from '../native-chat/agent-session-journal/journal-prompt-body-bounds' import { MAX_JOURNAL_LIFECYCLE_BATCH_BYTES } from '../native-chat/agent-session-journal/journal-row-schema' import { MAX_TOOL_DETAIL_LENGTH } from '../../shared/native-chat-tool-summary' import { claudeApprovalItem, claudeQuestionItems } from './claude-structured-prompt-items' import { - applyClaudePromptAnswer, - encodeClaudeQuestionOptionId, + buildClaudePromptReply, type ClaudePendingPrompt } from './claude-structured-prompt-replies' @@ -24,7 +22,6 @@ function approvalPrompt( input, suggestions: [], questionIds: [], - answers: new Map(), settle: () => {}, ...presentation } @@ -121,19 +118,21 @@ describe('Claude structured approval presentation', () => { } ) - expect(applyClaudePromptAnswer({ prompt }, 'deny')).toEqual({ + expect(buildClaudePromptReply(prompt, { kind: 'option', optionId: 'deny' })).toEqual({ behavior: 'deny', message: 'User denied this action.', toolUseID: 'tool-approval' }) - expect(applyClaudePromptAnswer({ prompt }, 'allowForSession')).toEqual({ - behavior: 'allow', - updatedInput: { command: 'rm output.txt' }, - updatedPermissions: [ - { type: 'addRules', rules: [], behavior: 'allow', destination: 'session' } - ], - toolUseID: 'tool-approval' - }) + expect(buildClaudePromptReply(prompt, { kind: 'option', optionId: 'allowForSession' })).toEqual( + { + behavior: 'allow', + updatedInput: { command: 'rm output.txt' }, + updatedPermissions: [ + { type: 'addRules', rules: [], behavior: 'allow', destination: 'session' } + ], + toolUseID: 'tool-approval' + } + ) }) it('asks Claude to revise a rejected plan while accepting legacy session replies', () => { @@ -145,16 +144,18 @@ describe('Claude structured approval presentation', () => { } ) - expect(applyClaudePromptAnswer({ prompt }, 'deny')).toEqual({ + expect(buildClaudePromptReply(prompt, { kind: 'option', optionId: 'deny' })).toEqual({ behavior: 'deny', message: 'The user asked you to keep planning. Revise the plan and call ExitPlanMode again.', toolUseID: 'tool-approval' }) - expect(applyClaudePromptAnswer({ prompt }, 'allowForSession')).toEqual({ - behavior: 'allow', - updatedInput: { plan: '# Release' }, - toolUseID: 'tool-approval' - }) + expect(buildClaudePromptReply(prompt, { kind: 'option', optionId: 'allowForSession' })).toEqual( + { + behavior: 'allow', + updatedInput: { plan: '# Release' }, + toolUseID: 'tool-approval' + } + ) }) }) @@ -178,7 +179,6 @@ describe('Claude structured question addressing', () => { input: { questions }, suggestions: [], questionIds: questions.map((question) => question.question), - answers: new Map(), settle: () => {} } @@ -211,7 +211,6 @@ describe('Claude structured question addressing', () => { input: { questions: [{ question: questionId, options: [{ label }] }] }, suggestions: [], questionIds: [questionId], - answers: new Map(), settle: () => {} } @@ -219,7 +218,12 @@ describe('Claude structured question addressing', () => { expect(agentJournalItemKey(item.identity).length).toBeLessThan(512) expect(item.body.options[0]!.id.length).toBeLessThan(512) expect(item.body.freeTextQuestionId).toBe('q1') - expect(applyClaudePromptAnswer({ prompt }, item.body.options[0]!.id)).toMatchObject({ + expect( + buildClaudePromptReply(prompt, { + kind: 'answers', + answers: [{ questionId: 'q1', optionIds: [item.body.options[0]!.id] }] + }) + ).toMatchObject({ updatedInput: { answers: { [questionId]: label } } }) }) @@ -235,13 +239,15 @@ describe('Claude structured question addressing', () => { input: { questions: [{ question: questionId }] }, suggestions: [], questionIds: [questionId], - answers: new Map(), settle: () => {} } const answer = 'https://example.test:8443/path' expect( - applyClaudePromptAnswer({ prompt }, encodeClaudeQuestionOptionId('q1', answer)) + buildClaudePromptReply(prompt, { + kind: 'answers', + answers: [{ questionId: 'q1', optionIds: [], other: answer }] + }) ).toMatchObject({ updatedInput: { answers: { [questionId]: answer } } }) @@ -273,21 +279,20 @@ describe('Claude structured question addressing', () => { }, suggestions: [], questionIds: [multiQuestion, singleQuestion, otherQuestion], - answers: new Map(), settle: () => {} } const item = claudeQuestionItems({ sessionId: 'session-1', prompt })[0]! const questions = item.body.questions! - const encoded = encodeAgentSessionQuestionAnswers([ + const answers = [ { questionId: 'q1', optionIds: [questions[0]!.options[0]!.id, questions[0]!.options[1]!.id] }, { questionId: 'q2', optionIds: [questions[1]!.options[1]!.id] }, { questionId: 'q3', optionIds: [], other: 'remote host' } - ]) + ] - expect(applyClaudePromptAnswer({ prompt }, encoded)).toMatchObject({ + expect(buildClaudePromptReply(prompt, { kind: 'answers', answers })).toMatchObject({ updatedInput: { answers: { [multiQuestion]: ['frontend', 'backend'], diff --git a/src/main/claude/claude-structured-prompt-ownership.test.ts b/src/main/claude/claude-structured-prompt-ownership.test.ts index 7a06a58e881..678c6a4f1e2 100644 --- a/src/main/claude/claude-structured-prompt-ownership.test.ts +++ b/src/main/claude/claude-structured-prompt-ownership.test.ts @@ -120,7 +120,7 @@ describe('Claude live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'allow', + response: { kind: 'option', optionId: 'allow' }, fence: 7, commit: async () => { expect(answered.settled()).toBe(false) @@ -185,7 +185,7 @@ describe('Claude live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'allow', + response: { kind: 'option', optionId: 'allow' }, fence: 7, commit }) @@ -204,7 +204,7 @@ describe('Claude live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'allow', + response: { kind: 'option', optionId: 'allow' }, fence: 7, commit }) @@ -356,7 +356,6 @@ describe('Claude live prompt ownership', () => { input: { command: 'git status' }, suggestions: [], questionIds: [], - answers: new Map(), settle: vi.fn() } }) @@ -398,7 +397,7 @@ describe('Claude live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'allow', + response: { kind: 'option', optionId: 'allow' }, fence: 7, commit: async () => undefined }) @@ -557,7 +556,7 @@ describe('Claude live prompt ownership', () => { sessionId: 'session-1', itemId: promptItemId, kind: 'approval', - optionId: 'allow', + response: { kind: 'option', optionId: 'allow' }, fence: 7, commit }) @@ -610,7 +609,7 @@ describe('Claude live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'allow', + response: { kind: 'option', optionId: 'allow' }, fence: 8, commit }) @@ -689,7 +688,6 @@ describe('Claude live prompt ownership', () => { }, suggestions: [], questionIds: ['First?', 'Second?'], - answers: new Map(), settle: vi.fn() } prompts.handle({ type: 'prompt', sessionId: 'session-1', prompt }) @@ -728,7 +726,6 @@ describe('Claude live prompt ownership', () => { input: { command: 'git status' }, suggestions: [], questionIds: [], - answers: new Map(), settle: vi.fn() } prompts.handle({ type: 'prompt', sessionId: 'session-1', prompt }) diff --git a/src/main/claude/claude-structured-prompt-ownership.ts b/src/main/claude/claude-structured-prompt-ownership.ts index e4c9de982d0..885ea2dfa74 100644 --- a/src/main/claude/claude-structured-prompt-ownership.ts +++ b/src/main/claude/claude-structured-prompt-ownership.ts @@ -1,4 +1,5 @@ import { + AgentSessionPromptAnswerRejectedError, AgentSessionPromptUnavailableError, type StructuredAgentSessionAdapter } from '../native-chat/agent-session-wire/structured-agent-session-adapter' @@ -10,7 +11,10 @@ import { supportsClaudeQueuedInterruptCancellation } from './claude-structured-control-actions' import type { ClaudeLateDispatchSettlement } from './claude-structured-dispatch' +import { buildClaudePromptReply } from './claude-structured-prompt-replies' import type { ClaudeSession } from './claude-structured-session-state' +import type { ClaudePendingPrompt } from './claude-prompt-registry' +import type { PermissionResult } from '@anthropic-ai/claude-agent-sdk' import { claudeStartupHoldsWrites, rejectClaudeStartupWrites @@ -202,6 +206,19 @@ export async function cancelClaudeStructuredTurn(input: { } } +function prepareClaudePromptReply( + prompt: ClaudePendingPrompt, + response: AnswerInput['response'] +): PermissionResult { + try { + return buildClaudePromptReply(prompt, response) + } catch (error) { + throw new AgentSessionPromptAnswerRejectedError( + error instanceof Error ? error.message : String(error) + ) + } +} + export async function answerClaudeStructuredPrompt(input: { request: AnswerInput sessions: Map @@ -217,6 +234,7 @@ export async function answerClaudeStructuredPrompt(input: { throw new AgentSessionPromptUnavailableError(request.itemId) } try { + const reply = prepareClaudePromptReply(claim.found.prompt, request.response) await request.commit() if ( sessions.get(request.sessionId) !== session || @@ -226,7 +244,7 @@ export async function answerClaudeStructuredPrompt(input: { ) { throw new AgentSessionPromptUnavailableError(request.itemId) } - await answerClaudePrompt(session, claim, request.optionId) + await answerClaudePrompt(session, claim, reply) } catch (error) { session.prompts.releaseClaim(claim) throw error diff --git a/src/main/claude/claude-structured-prompt-replies.ts b/src/main/claude/claude-structured-prompt-replies.ts index 96bc83d0876..3bb99e3638a 100644 --- a/src/main/claude/claude-structured-prompt-replies.ts +++ b/src/main/claude/claude-structured-prompt-replies.ts @@ -1,5 +1,8 @@ import type { PermissionResult } from '@anthropic-ai/claude-agent-sdk' -import { decodeAgentSessionQuestionAnswers } from '../../shared/agent-session-question-answer' +import type { + AgentSessionPromptResponse, + AgentSessionQuestionAnswer +} from '../../shared/agent-session-question-answer' import { claudePromptQuestions, isClaudePromptRecord, @@ -22,12 +25,6 @@ function isClaudeApprovalDecision(optionId: string): optionId is ClaudeApprovalD return CLAUDE_APPROVAL_DECISIONS.some((decision) => decision === optionId) } -function questionIdFromAddress(prompt: ClaudePendingPrompt, address: string): string | null { - const match = /^q([1-9]\d*)$/.exec(address) - const index = match ? Number(match[1]) - 1 : -1 - return index >= 0 ? (prompt.questionIds[index] ?? null) : null -} - function questionAnswer(prompt: ClaudePendingPrompt, questionId: string, optionId: string): string { const decoded = decodeClaudeQuestionOptionId(optionId) if (!decoded) { @@ -111,49 +108,8 @@ function approvalResponse(prompt: ClaudePendingPrompt, optionId: string): Permis function questionResponse( prompt: ClaudePendingPrompt, - optionId: string, - boundQuestionId?: string -): PermissionResult | null { - const decoded = decodeClaudeQuestionOptionId(optionId) - const decodedQuestionId = decoded - ? (questionIdFromAddress(prompt, decoded.questionId) ?? - (prompt.questionIds.includes(decoded.questionId) ? decoded.questionId : null)) - : null - const selectedQuestionId = - boundQuestionId ?? - decodedQuestionId ?? - (prompt.questionIds.length === 1 ? prompt.questionIds[0] : null) - if (!selectedQuestionId || !prompt.questionIds.includes(selectedQuestionId)) { - throw new Error(`${optionId} does not name a question on Claude prompt ${prompt.promptKey}`) - } - const answer = questionAnswer(prompt, selectedQuestionId, optionId) - prompt.answers.set(selectedQuestionId, answer) - if (prompt.questionIds.some((id) => !prompt.answers.has(id))) { - return null - } - const answers: Record = {} - for (const id of prompt.questionIds) { - const answer = prompt.answers.get(id) - if (answer === undefined) { - return null - } - answers[id] = answer - } - return { - behavior: 'allow', - updatedInput: { ...prompt.input, answers }, - toolUseID: prompt.toolUseId - } -} - -function groupedQuestionResponse( - prompt: ClaudePendingPrompt, - optionId: string -): PermissionResult | null { - const grouped = decodeAgentSessionQuestionAnswers(optionId) - if (!grouped) { - return null - } + grouped: readonly AgentSessionQuestionAnswer[] +): PermissionResult { const questions = claudePromptQuestions(prompt.input) if (grouped.length !== prompt.questionIds.length) { throw new Error(`Grouped answer does not match Claude prompt ${prompt.promptKey}`) @@ -191,15 +147,20 @@ function groupedQuestionResponse( } } -export function applyClaudePromptAnswer( - found: { prompt: ClaudePendingPrompt; questionId?: string }, - optionId: string -): PermissionResult | null { - if (found.prompt.kind === 'approval') { - return approvalResponse(found.prompt, optionId) +/** Builds Claude's reply without touching the prompt, so a reply that cannot be built refuses the + * answer before anything is recorded. */ +export function buildClaudePromptReply( + prompt: ClaudePendingPrompt, + response: AgentSessionPromptResponse +): PermissionResult { + if (prompt.kind === 'approval') { + if (response.kind !== 'option') { + throw new Error(`Claude prompt ${prompt.promptKey} takes a decision, not answers`) + } + return approvalResponse(prompt, response.optionId) } - return ( - groupedQuestionResponse(found.prompt, optionId) ?? - questionResponse(found.prompt, optionId, found.questionId) - ) + if (response.kind !== 'answers') { + throw new Error(`Claude prompt ${prompt.promptKey} takes answers, not a decision`) + } + return questionResponse(prompt, response.answers) } diff --git a/src/main/claude/claude-structured-session-adapter.test.ts b/src/main/claude/claude-structured-session-adapter.test.ts index a879878436a..04bc6d5ffff 100644 --- a/src/main/claude/claude-structured-session-adapter.test.ts +++ b/src/main/claude/claude-structured-session-adapter.test.ts @@ -3,12 +3,12 @@ import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' import { AgentSessionAcquisitionExitUnprovenError, - AgentSessionAcquisitionRootExitObservedError + AgentSessionAcquisitionRootExitObservedError, + AgentSessionPromptAnswerRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { ClaudeStreamJsonConnection } from './claude-stream-json-connection' import { ClaudeControlRequestError } from './claude-stream-json-connection' import { CLAUDE_SPAWN_TOKEN_ENV } from './claude-structured-owner-identity' -import { encodeClaudeQuestionOptionId } from './claude-structured-prompt-replies' import type { ClaudeStructuredSessionAdapter, ClaudeStructuredSessionEvent @@ -780,7 +780,7 @@ describe('ClaudeStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId: 'journal-approval', kind: 'approval', - optionId: 'allowForSession', + response: { kind: 'option', optionId: 'allowForSession' }, fence: 7, commit: async () => undefined }) @@ -793,7 +793,7 @@ describe('ClaudeStructuredSessionAdapter prompts', () => { }) }) - it('collects every AskUserQuestion card before settling the one callback', async () => { + it('settles the one AskUserQuestion callback from structured answers, including a long typed answer', async () => { const claude = fakeClaude() const adapter = await acquired(claude) const answered = invokeCanUseTool( @@ -810,34 +810,68 @@ describe('ClaudeStructuredSessionAdapter prompts', () => { } } ) - adapter.bindPromptItemId('session-1', 'journal-q1', 'question-1', 'Library?') - adapter.bindPromptItemId('session-1', 'journal-q2', 'question-1', 'Ship now?') + adapter.bindPromptItemId('session-1', 'journal-question', 'question-1') + const typed = 'Wait for the capture to finish first. '.repeat(60) await adapter.answerPrompt({ sessionId: 'session-1', - itemId: 'journal-q1', + itemId: 'journal-question', kind: 'question', - optionId: encodeClaudeQuestionOptionId('Library?', 'Luxon'), - fence: 7, - commit: async () => undefined - }) - await tick() - expect(answered.settled()).toBe(false) - await adapter.answerPrompt({ - sessionId: 'session-1', - itemId: 'journal-q2', - kind: 'question', - optionId: encodeClaudeQuestionOptionId('Ship now?', 'Yes'), + response: { + kind: 'answers', + answers: [ + { questionId: 'q1', optionIds: ['q1:choice-1'] }, + { questionId: 'q2', optionIds: [], other: typed } + ] + }, fence: 7, commit: async () => undefined }) await expect(answered.promise).resolves.toMatchObject({ behavior: 'allow', - updatedInput: { answers: { 'Library?': 'Luxon', 'Ship now?': 'Yes' } }, + updatedInput: { answers: { 'Library?': 'Luxon', 'Ship now?': typed.trim() } }, toolUseID: 'tool-question' }) }) + it('refuses answers Claude cannot take before the journal commits them', async () => { + const claude = fakeClaude() + const adapter = await acquired(claude) + const answered = invokeCanUseTool( + claude.connections[0], + 'AskUserQuestion', + 'question-1', + 'tool-question', + { input: { questions: [{ question: 'Library?', options: [{ label: 'Luxon' }] }] } } + ) + adapter.bindPromptItemId('session-1', 'journal-question', 'question-1') + const commit = vi.fn(async () => undefined) + + await expect( + adapter.answerPrompt({ + sessionId: 'session-1', + itemId: 'journal-question', + kind: 'question', + response: { kind: 'option', optionId: 'allow' }, + fence: 7, + commit + }) + ).rejects.toBeInstanceOf(AgentSessionPromptAnswerRejectedError) + expect(commit).not.toHaveBeenCalled() + + await adapter.answerPrompt({ + sessionId: 'session-1', + itemId: 'journal-question', + kind: 'question', + response: { kind: 'answers', answers: [{ questionId: 'q1', optionIds: ['q1:choice-1'] }] }, + fence: 7, + commit + }) + await expect(answered.promise).resolves.toMatchObject({ + updatedInput: { answers: { 'Library?': 'Luxon' } } + }) + }) + it('leaves a prompt cancelled and unanswerable once the SDK abort signal fires', async () => { const claude = fakeClaude() const events: ClaudeStructuredSessionEvent[] = [] @@ -859,7 +893,7 @@ describe('ClaudeStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId: 'journal-9', kind: 'approval', - optionId: 'allow', + response: { kind: 'option', optionId: 'allow' }, fence: 7, commit: async () => undefined }) diff --git a/src/main/claude/claude-structured-session-adapter.ts b/src/main/claude/claude-structured-session-adapter.ts index 835757dc730..e9e233e4934 100644 --- a/src/main/claude/claude-structured-session-adapter.ts +++ b/src/main/claude/claude-structured-session-adapter.ts @@ -169,17 +169,11 @@ export class ClaudeStructuredSessionAdapter implements StructuredAgentSessionAda } } - bindPromptItemId( - sessionId: string, - journalItemId: string, - promptKey: string, - questionId?: string - ): void { + bindPromptItemId(sessionId: string, journalItemId: string, promptKey: string): void { const session = this.sessions.get(sessionId) session?.prompts.bindJournalItemId( journalItemId, promptKey, - questionId, session.translator?.currentTurnId ?? null ) } diff --git a/src/main/codex/codex-structured-prompt-ownership.test.ts b/src/main/codex/codex-structured-prompt-ownership.test.ts index cfb77e628a7..ecdc4c9a4be 100644 --- a/src/main/codex/codex-structured-prompt-ownership.test.ts +++ b/src/main/codex/codex-structured-prompt-ownership.test.ts @@ -159,7 +159,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit: async () => { expect(codex.connections[0]?.replies).toEqual([]) @@ -210,7 +210,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit }) @@ -224,7 +224,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit }) @@ -237,7 +237,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 8, commit }) @@ -266,7 +266,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: 'journal-prompt', kind: 'approval', - optionId: 'decline', + response: { kind: 'option', optionId: 'decline' }, fence: 7, commit: async () => undefined }) @@ -382,7 +382,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: siblingItemId, kind: 'question', - optionId: 'no', + response: { kind: 'answers', answers: [{ questionId: 'q2', optionIds: [], other: 'no' }] }, fence: 7, commit: async () => undefined }) @@ -497,7 +497,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: promptItemId, kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit }) @@ -545,7 +545,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: promptItemId, kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit }) @@ -587,7 +587,7 @@ describe('Codex live prompt ownership', () => { sessionId: 'session-1', itemId: promptItemId, kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit }) diff --git a/src/main/codex/codex-structured-prompt-ownership.ts b/src/main/codex/codex-structured-prompt-ownership.ts index 28d060d955f..d58f08e731d 100644 --- a/src/main/codex/codex-structured-prompt-ownership.ts +++ b/src/main/codex/codex-structured-prompt-ownership.ts @@ -1,9 +1,15 @@ import { + AgentSessionPromptAnswerRejectedError, AgentSessionPromptUnavailableError, type StructuredAgentSessionAdapter } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { StructuredSessionCompaction } from '../native-chat/agent-session-wire/structured-session-compaction' -import { answerCodexPrompt } from './codex-structured-prompt-replies' +import { + answerCodexPrompt, + prepareCodexPromptAnswer, + type CodexPendingPrompt, + type CodexPreparedAnswer +} from './codex-structured-prompt-replies' import { requireLiveCodexSession, type CodexSession } from './codex-structured-session-state' import type { CodexStructuredTurnCancellation } from './codex-structured-turn-cancellation' @@ -69,6 +75,19 @@ export async function cancelCodexStructuredTurn(input: { } } +function prepareCodexAnswer( + prompt: CodexPendingPrompt, + response: AnswerInput['response'] +): CodexPreparedAnswer { + try { + return prepareCodexPromptAnswer(prompt, response) + } catch (error) { + throw new AgentSessionPromptAnswerRejectedError( + error instanceof Error ? error.message : String(error) + ) + } +} + export async function answerCodexStructuredPrompt(input: { request: AnswerInput sessions: Map @@ -84,6 +103,7 @@ export async function answerCodexStructuredPrompt(input: { throw new AgentSessionPromptUnavailableError(request.itemId) } try { + const prepared = prepareCodexAnswer(claim.prompt, request.response) await request.commit() if ( sessions.get(request.sessionId) !== session || @@ -95,7 +115,7 @@ export async function answerCodexStructuredPrompt(input: { throw new AgentSessionPromptUnavailableError(request.itemId) } session.translator?.resolvePrompt(request.itemId) - answerCodexPrompt(session.prompts, session.connection, claim, request.optionId) + answerCodexPrompt(session.prompts, session.connection, claim, prepared) } catch (error) { session.prompts.releaseClaim(claim) throw error diff --git a/src/main/codex/codex-structured-prompt-replies.test.ts b/src/main/codex/codex-structured-prompt-replies.test.ts index e575f27632b..9f17a3efb4c 100644 --- a/src/main/codex/codex-structured-prompt-replies.test.ts +++ b/src/main/codex/codex-structured-prompt-replies.test.ts @@ -1,8 +1,11 @@ import { describe, expect, it } from 'vitest' import { AGENT_SESSION_ID_MAX_LENGTH } from '../../shared/agent-session-wire' +import type { AgentSessionPromptResponse } from '../../shared/agent-session-question-answer' import { applyCodexPromptAnswer, CodexPromptRegistry, + prepareCodexPromptAnswer, + type CodexPendingPrompt, MAX_CODEX_PROMPT_REGISTRY_BYTES, MAX_CODEX_PROMPT_REGISTRY_ENTRIES, codexJournalPromptIdPart, @@ -11,6 +14,29 @@ import { encodeCodexQuestionOptionId } from './codex-structured-prompt-replies' +function picked(optionId: string): AgentSessionPromptResponse { + return { kind: 'answers', answers: [{ questionId: 'q1', optionIds: [optionId] }] } +} + +function typed(questionId: string, other: string): AgentSessionPromptResponse { + return { kind: 'answers', answers: [{ questionId, optionIds: [], other }] } +} + +function registered(prompt: CodexPendingPrompt | null): CodexPendingPrompt { + if (!prompt) { + throw new Error('expected the request to register') + } + return prompt +} + +function answer( + prompt: CodexPendingPrompt | null, + response: AgentSessionPromptResponse +): Record | null { + const live = registered(prompt) + return applyCodexPromptAnswer(live, prepareCodexPromptAnswer(live, response)) +} + function userInputRequest(questionIds: string[]): { id: number method: string @@ -59,7 +85,7 @@ describe('codex question option ids', () => { expect(Buffer.byteLength(optionId, 'utf8')).toBeLessThan(1024) expect(codexJournalPromptIdPart(longQuestionId)).not.toBe(longQuestionId) - expect(applyCodexPromptAnswer(prompt as NonNullable, optionId)).toEqual({ + expect(answer(prompt, picked(optionId))).toEqual({ answers: { [longQuestionId]: { answers: [longAnswer] } } }) }) @@ -219,11 +245,11 @@ describe('CodexPromptRegistry', () => { }) describe('applyCodexPromptAnswer', () => { - it('accepts a bare answer only when the request has one question', () => { + it('answers the lone question of a single-question request with typed text', () => { const registry = new CodexPromptRegistry() const single = registry.register(userInputRequest(['q1'])) - expect(applyCodexPromptAnswer(single as NonNullable, 'sure')).toEqual({ + expect(answer(single, typed('q1', 'sure'))).toEqual({ answers: { q1: { answers: ['sure'] } } }) }) @@ -232,29 +258,32 @@ describe('applyCodexPromptAnswer', () => { const registry = new CodexPromptRegistry() const many = registry.register(userInputRequest(['q1', 'q2'])) - expect(() => applyCodexPromptAnswer(many as NonNullable, 'sure')).toThrow( - 'does not name a question' - ) - expect(() => - applyCodexPromptAnswer( - many as NonNullable, - encodeCodexQuestionOptionId('q3', 'sure') - ) - ).toThrow('does not name a question') + expect(() => answer(many, picked('sure'))).toThrow('does not name a question') + expect(() => answer(many, typed('q3', 'sure'))).toThrow('does not name a question') }) it('keeps the last answer when a question is answered twice', () => { const registry = new CodexPromptRegistry() const single = registry.register(userInputRequest(['q1'])) - const prompt = single as NonNullable - applyCodexPromptAnswer(prompt, encodeCodexQuestionOptionId('q1', 'first')) + answer(single, typed('q1', 'first')) - expect(applyCodexPromptAnswer(prompt, encodeCodexQuestionOptionId('q1', 'second'))).toEqual({ + expect(answer(single, typed('q1', 'second'))).toEqual({ answers: { q1: { answers: ['second'] } } }) }) + it('refuses an answer over the registry bound before recording anything', () => { + const registry = new CodexPromptRegistry() + const single = registry.register(userInputRequest(['q1'])) + const live = registered(single) + + expect(() => prepareCodexPromptAnswer(live, typed('q1', 'x'.repeat(64 * 1024 + 1)))).toThrow( + 'exceeds bounded registry state' + ) + expect(live.answers.size).toBe(0) + }) + it('refuses question and option collections that exceed bounded live state', () => { const registry = new CodexPromptRegistry() const tooManyQuestions = registry.register( diff --git a/src/main/codex/codex-structured-prompt-replies.ts b/src/main/codex/codex-structured-prompt-replies.ts index 6e742bf827c..d4301d8f27c 100644 --- a/src/main/codex/codex-structured-prompt-replies.ts +++ b/src/main/codex/codex-structured-prompt-replies.ts @@ -1,3 +1,4 @@ +import type { AgentSessionPromptResponse } from '../../shared/agent-session-question-answer' import type { CodexAppServerConnection } from './codex-app-server-connection' import { CODEX_PROMPT_MAX_ANSWER_BYTES } from './codex-prompt-registry-bounds' import { @@ -55,35 +56,61 @@ export function decodeCodexQuestionOptionId( } } +/** One answer, checked against the prompt but not yet recorded on it. */ +export type CodexPreparedAnswer = + | { kind: 'decision'; decision: CodexApprovalDecision } + | { kind: 'answer'; questionId: string; answer: string } + +/** Validates a client's choice against the prompt without recording it, so an answer Codex + * cannot take is refused before the journal commits it. */ +export function prepareCodexPromptAnswer( + prompt: CodexPendingPrompt, + response: AgentSessionPromptResponse +): CodexPreparedAnswer { + if (prompt.method !== CODEX_USER_INPUT_METHOD) { + if (response.kind !== 'option' || !isCodexApprovalDecision(response.optionId)) { + throw new Error(`Codex item ${prompt.codexItemId} takes an approval decision`) + } + return { kind: 'decision', decision: response.optionId } + } + // Each Codex question is its own journal item, so an answer names exactly one question. + const entry = + response.kind === 'answers' && response.answers.length === 1 ? response.answers[0] : null + if (!entry) { + throw new Error(`Codex item ${prompt.codexItemId} takes one question answer`) + } + const optionId = entry.optionIds[0] + const decoded = + optionId === undefined + ? { questionId: entry.questionId, answer: entry.other?.trim() ?? '' } + : (prompt.optionAnswers.get(optionId) ?? decodeCodexQuestionOptionId(optionId)) + const questionId = + (decoded?.questionId + ? (prompt.questionIdAliases.get(decoded.questionId) ?? decoded.questionId) + : null) ?? (prompt.questionIds.length === 1 ? prompt.questionIds[0] : null) + const answer = decoded?.answer ?? optionId ?? '' + if (!questionId || !prompt.questionIds.includes(questionId)) { + throw new Error(`The answer does not name a question on Codex item ${prompt.codexItemId}`) + } + if (Buffer.byteLength(answer, 'utf8') > CODEX_PROMPT_MAX_ANSWER_BYTES) { + throw new Error('codex prompt answer exceeds bounded registry state') + } + return { kind: 'answer', questionId, answer } +} + /** - * Records one answer and returns the reply payload once the request is fully + * Records one prepared answer and returns the reply payload once the request is fully * answered. A multi-question user-input request stays pending until every * question has an answer, because Codex takes one reply for all of them. */ export function applyCodexPromptAnswer( prompt: CodexPendingPrompt, - optionId: string + prepared: CodexPreparedAnswer ): Record | null { - if (prompt.method !== CODEX_USER_INPUT_METHOD) { - if (!isCodexApprovalDecision(optionId)) { - throw new Error(`${optionId} is not a Codex approval decision`) - } - return { decision: optionId } + if (prepared.kind === 'decision') { + return { decision: prepared.decision } } - const mapped = prompt.optionAnswers.get(optionId) - const decoded = mapped ?? decodeCodexQuestionOptionId(optionId) - const questionId = - (decoded?.questionId - ? (prompt.questionIdAliases.get(decoded.questionId) ?? decoded.questionId) - : null) ?? (prompt.questionIds.length === 1 ? prompt.questionIds[0] : null) - const answer = decoded?.answer ?? optionId - if (!questionId || !prompt.questionIds.includes(questionId)) { - throw new Error(`${optionId} does not name a question on Codex item ${prompt.codexItemId}`) - } - if (Buffer.byteLength(answer, 'utf8') > CODEX_PROMPT_MAX_ANSWER_BYTES) { - throw new Error('codex prompt answer exceeds bounded registry state') - } - prompt.answers.set(questionId, answer) + prompt.answers.set(prepared.questionId, prepared.answer) if (prompt.questionIds.some((id) => !prompt.answers.has(id))) { return null } @@ -104,13 +131,13 @@ export function answerCodexPrompt( registry: CodexPromptRegistry, connection: Pick, claim: CodexPromptClaim, - optionId: string + prepared: CodexPreparedAnswer ): void { if (!registry.ownsClaim(claim)) { throw new Error(`codex app-server is no longer waiting on ${claim.itemId}`) } const prompt = claim.prompt - const reply = applyCodexPromptAnswer(prompt, optionId) + const reply = applyCodexPromptAnswer(prompt, prepared) if (reply === null) { registry.releaseClaim(claim) return diff --git a/src/main/codex/codex-structured-session-adapter-lifecycle.test.ts b/src/main/codex/codex-structured-session-adapter-lifecycle.test.ts index fc458304dc8..6389cbe70d5 100644 --- a/src/main/codex/codex-structured-session-adapter-lifecycle.test.ts +++ b/src/main/codex/codex-structured-session-adapter-lifecycle.test.ts @@ -152,7 +152,7 @@ describe('CodexStructuredSessionAdapter lifecycle', () => { sessionId: 'session-2', itemId: 'codex-item-1', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 1, commit: async () => undefined }) diff --git a/src/main/codex/codex-structured-session-adapter.test.ts b/src/main/codex/codex-structured-session-adapter.test.ts index f17003f7e40..2dd4b8d78af 100644 --- a/src/main/codex/codex-structured-session-adapter.test.ts +++ b/src/main/codex/codex-structured-session-adapter.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest' +import { AgentSessionPromptAnswerRejectedError } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import { CodexAppServerRequestError, type openCodexAppServerConnection @@ -6,7 +7,6 @@ import { import type { StructuredAgentSessionEventSink } from '../native-chat/agent-session-wire/structured-agent-session-event-sink' import { CODEX_SPAWN_TOKEN_ENV } from './codex-structured-owner-identity' import { ORCA_STRUCTURED_SESSION_ENV } from '../../shared/structured-session-marker' -import { encodeCodexQuestionOptionId } from './codex-structured-prompt-replies' import { CodexStructuredSessionAdapter, type CodexStructuredLaunch, @@ -174,7 +174,7 @@ describe('CodexStructuredSessionAdapter.acquire', () => { sessionId: 'session-1', itemId: 'codex-item-early', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit: async () => undefined }) @@ -535,7 +535,7 @@ describe('CodexStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId: 'codex:thread-abc:turn-1:3', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit: async () => undefined }) @@ -548,7 +548,7 @@ describe('CodexStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId: 'codex:thread-abc:turn-1:3', kind: 'approval', - optionId: 'decline', + response: { kind: 'option', optionId: 'decline' }, fence: 7, commit: async () => undefined }) @@ -589,7 +589,7 @@ describe('CodexStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId: 'codex-item-1', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit: async () => undefined }) @@ -676,7 +676,7 @@ describe('CodexStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId, kind: 'approval', - optionId, + response: { kind: 'option', optionId }, fence: 7, commit: async () => undefined }) @@ -697,17 +697,20 @@ describe('CodexStructuredSessionAdapter prompts', () => { const adapter = await acquired(codex) askApproval(codex) + const commit = vi.fn(async () => undefined) await expect( adapter.answerPrompt({ sessionId: 'session-1', itemId: 'codex-item-1', kind: 'approval', - optionId: 'yolo', + response: { kind: 'option', optionId: 'yolo' }, fence: 7, - commit: async () => undefined + commit }) - ).rejects.toThrow('is not a Codex approval decision') + ).rejects.toThrow(AgentSessionPromptAnswerRejectedError) + // Refused before the journal records an answer the agent never receives. + expect(commit).not.toHaveBeenCalled() expect(codex.connections[0].replies).toEqual([]) }) @@ -732,7 +735,7 @@ describe('CodexStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId: 'codex-item-2', kind: 'question', - optionId: encodeCodexQuestionOptionId('q1', 'yes'), + response: { kind: 'answers', answers: [{ questionId: 'q1', optionIds: [], other: 'yes' }] }, fence: 7, commit: async () => undefined }) @@ -742,7 +745,7 @@ describe('CodexStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId: 'codex-item-2', kind: 'question', - optionId: encodeCodexQuestionOptionId('q2', 'no'), + response: { kind: 'answers', answers: [{ questionId: 'q2', optionIds: [], other: 'no' }] }, fence: 7, commit: async () => undefined }) @@ -778,7 +781,7 @@ describe('CodexStructuredSessionAdapter prompts', () => { sessionId: 'session-1', itemId: 'codex-item-gone', kind: 'approval', - optionId: 'accept', + response: { kind: 'option', optionId: 'accept' }, fence: 7, commit: async () => undefined }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts index 7085700cdcb..27cd3c30240 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts @@ -31,6 +31,7 @@ import type { AgentSessionWireRefusalCode } from '../../../shared/agent-session-wire' import { isAgentSessionWireRefusalCode } from '../../../shared/agent-session-wire-refusals' +import type { AgentSessionPromptResponse } from '../../../shared/agent-session-question-answer' import type { ProviderHistoryWindow } from '../agent-session-journal/journal-submission-reconciler' import type { StructuredAgentSessionEventSink } from './structured-agent-session-event-sink' import type { AgentSessionCreatePhaseRecorder } from '../../observability/agent-session-instrumentation' @@ -52,6 +53,14 @@ export class AgentSessionPromptUnavailableError extends Error { } } +/** The provider cannot take this answer. Thrown before the journal commit, so nothing is recorded. */ +export class AgentSessionPromptAnswerRejectedError extends Error { + constructor(message: string) { + super(message) + this.name = 'AgentSessionPromptAnswerRejectedError' + } +} + /** * The provider's own root process was observed to exit, but its descendant tree * could not be verified. The lease keys on the root's pid and start time, so its @@ -263,13 +272,14 @@ export type StructuredAgentSessionAdapter = { /** The `/` surface the running provider reports for itself. Undefined when the * provider never reports one, which is what keeps the client on its catalog. */ readCommands?(sessionId: string): AgentSessionSlashCommand[] | undefined - /** Claims the live callback, commits the journal CAS while that claim is held, then answers it. - * A prompt cancel claims the same callback, so only one operation can commit. */ + /** Claims the live callback, builds the provider reply, commits the journal CAS while that claim is + * held, then answers it. A reply that cannot be built throws `AgentSessionPromptAnswerRejectedError` + * before the commit. A prompt cancel claims the same callback, so only one operation can commit. */ answerPrompt(input: { sessionId: string itemId: string kind: 'approval' | 'question' - optionId: string + response: AgentSessionPromptResponse fence: number commit: () => Promise }): Promise diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts index bbc1ec49d4c..013f4191c30 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts @@ -141,23 +141,78 @@ afterEach(async () => { }) describe('grouped question admission', () => { - it('admits renderer question-group payloads with child ids and multi-select answers', async () => { + it('reads the packed answer an older client sends into structured answers', async () => { const attached = await host.attach(CALLER, attachParams()) expect(attached.ok).toBe(true) const prompt = await seedGroupedQuestion() - const optionId = encodeAgentSessionQuestionAnswers([ + const answers = [ { questionId: 'q1', optionIds: ['target-web', 'target-mobile'] }, { questionId: 'q2', optionIds: [], other: 'SSH host' } - ]) + ] + const optionId = encodeAgentSessionQuestionAnswers(answers) const fields = { itemId: prompt.itemId, expectedRevision: prompt.revision, optionId } const result = await host.respondToPrompt(CALLER, { envelope: envelope('agentSession.respondTo:question', fields), kind: 'question', ...fields }) - expect(result).toMatchObject({ ok: true, value: { resolution: { state: 'resolved' } } }) + expect(result).toMatchObject({ + ok: true, + value: { resolution: { state: 'resolved', selectedOptionId: optionId, answers } } + }) expect(answerPrompt).toHaveBeenCalledWith( - expect.objectContaining({ itemId: prompt.itemId, optionId }) + expect.objectContaining({ itemId: prompt.itemId, response: { kind: 'answers', answers } }) ) }) + + it('takes structured answers past the old option-id bound and keeps the packed form for older readers', async () => { + const attached = await host.attach(CALLER, attachParams()) + expect(attached.ok).toBe(true) + const prompt = await seedGroupedQuestion() + const typed = 'Proceed with the replacement, but wait for the capture. '.repeat(40) + const answers = [ + { questionId: 'q1', optionIds: ['target-web'] }, + { questionId: 'q2', optionIds: [], other: typed } + ] + const fields = { itemId: prompt.itemId, expectedRevision: prompt.revision, answers } + const result = await host.respondToPrompt(CALLER, { + envelope: envelope('agentSession.respondTo:question', fields), + kind: 'question', + ...fields + }) + + expect(typed.length).toBeGreaterThan(1024) + expect(result).toMatchObject({ + ok: true, + value: { + resolution: { + state: 'resolved', + selectedOptionId: encodeAgentSessionQuestionAnswers(answers), + answers + } + } + }) + expect(answerPrompt).toHaveBeenCalledWith( + expect.objectContaining({ response: { kind: 'answers', answers } }) + ) + }) + + it('refuses answers that do not match the questions without reaching the provider', async () => { + const attached = await host.attach(CALLER, attachParams()) + expect(attached.ok).toBe(true) + const prompt = await seedGroupedQuestion() + const answers = [{ questionId: 'q1', optionIds: ['target-web'] }] + const fields = { itemId: prompt.itemId, expectedRevision: prompt.revision, answers } + const result = await host.respondToPrompt(CALLER, { + envelope: envelope('agentSession.respondTo:question', fields), + kind: 'question', + ...fields + }) + + expect(result).toMatchObject({ + ok: false, + refusal: { code: 'agent_session_operation_invalid' } + }) + expect(answerPrompt).not.toHaveBeenCalled() + }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts index 364f1e7c0dd..f6a5770cfac 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-mutations.ts @@ -22,6 +22,7 @@ import type { AgentSessionThreadGoalResult } from '../../../shared/agent-session-wire' import type { StructuredAgentSessionHolds } from './structured-agent-session-holds' +import type { AgentSessionPromptRequest } from './structured-agent-session-turns-prompt' import { threadGoalPlan } from './structured-agent-session-thread-goal' import { admitAndRunAgentSessionMutation, @@ -136,13 +137,7 @@ export function cancelStructuredAgentSessionTurn( export function respondToStructuredAgentSessionPrompt( context: StructuredAgentSessionMutationContext, caller: StructuredAgentSessionCaller, - params: { - envelope: AgentSessionMutationEnvelope - kind: 'approval' | 'question' - itemId: string - expectedRevision: number - optionId: string - } + params: AgentSessionPromptRequest & { envelope: AgentSessionMutationEnvelope } ): Promise> { return mutate(context, caller, params.envelope, promptPlan(params)) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-plans.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-plans.ts index d8d2876d272..226881d38bf 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-plans.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-mutation-plans.ts @@ -23,6 +23,7 @@ import { type AgentSessionTurnContext, type TurnOutcome } from './structured-agent-session-turns' +import type { AgentSessionPromptRequest } from './structured-agent-session-turns-prompt' export type MutationPlan = { method: string @@ -120,18 +121,17 @@ export function cancelPlan(params: { } } -export function promptPlan(params: { - kind: 'approval' | 'question' - itemId: string - expectedRevision: number - optionId: string -}): MutationPlan { +export function promptPlan( + params: AgentSessionPromptRequest +): MutationPlan { return { method: `agentSession.respondTo:${params.kind}`, + // The client hashes exactly what it sent; the absent one of these two drops out of the digest. fields: { itemId: params.itemId, expectedRevision: params.expectedRevision, - optionId: params.optionId + optionId: params.optionId, + answers: params.answers }, run: (ctx) => performPrompt(ctx, params), replay: (ctx) => { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts index 6ac29cab0ab..4cacf96ba89 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts @@ -1,51 +1,84 @@ import { parseAgentJournalItemKey } from '../../../shared/agent-session-journal-item-key' import { - decodeAgentSessionQuestionAnswers, - isValidAgentSessionQuestionAnswers + agentSessionPromptQuestions, + isValidAgentSessionQuestionAnswers, + legacyAgentSessionQuestionAnswers, + legacyAgentSessionSelectedOptionId, + type AgentSessionPromptResponse, + type AgentSessionQuestionAnswer } from '../../../shared/agent-session-question-answer' -import type { AgentJournalResolution } from '../../../shared/agent-session-journal-types' +import type { + AgentJournalApprovalItem, + AgentJournalQuestionItem, + AgentJournalResolution +} from '../../../shared/agent-session-journal-types' import type { AgentSessionPromptResult } from '../../../shared/agent-session-wire' -import { decodeCodexQuestionOptionId } from '../../codex/codex-structured-prompt-replies' -import { AgentSessionPromptUnavailableError } from './structured-agent-session-adapter' +import { + AgentSessionPromptAnswerRejectedError, + AgentSessionPromptUnavailableError +} from './structured-agent-session-adapter' import { validatePendingPrompt } from './structured-agent-session-prompt-state' import type { AgentSessionTurnContext, TurnOutcome } from './structured-agent-session-turns' +export type AgentSessionPromptRequest = { + itemId: string + expectedRevision: number + kind: 'approval' | 'question' + /** A decision id; or, from a client that predates `answers`, a question answer packed into one id. */ + optionId?: string + answers?: AgentSessionQuestionAnswer[] +} + function invalid(message: string): TurnOutcome { return { ok: false, refusal: { code: 'agent_session_operation_invalid', message } } } +/** The one place a client's choice is read; an answer an older client packed into `optionId` is unpacked here, once. */ +function readPromptChoice( + prompt: AgentJournalApprovalItem | AgentJournalQuestionItem, + input: AgentSessionPromptRequest +): { response: AgentSessionPromptResponse; selectedOptionId: string } | null { + if (prompt.kind === 'approval') { + const optionId = input.optionId + return optionId !== undefined && prompt.options.some((option) => option.id === optionId) + ? { response: { kind: 'option', optionId }, selectedOptionId: optionId } + : null + } + const answers = + input.answers ?? + (input.optionId === undefined + ? null + : legacyAgentSessionQuestionAnswers(prompt, input.optionId)) + if ( + !answers || + !isValidAgentSessionQuestionAnswers(agentSessionPromptQuestions(prompt), answers) + ) { + return null + } + const selectedOptionId = legacyAgentSessionSelectedOptionId(prompt, answers) + return selectedOptionId === null + ? null + : { response: { kind: 'answers', answers }, selectedOptionId } +} + export async function performPrompt( ctx: AgentSessionTurnContext, - input: { - itemId: string - expectedRevision: number - optionId: string - kind: 'approval' | 'question' - } + input: AgentSessionPromptRequest ): Promise> { const validated = validatePendingPrompt(ctx, input) if (!validated.ok) { return validated } const { prompt } = validated - const question = prompt.kind === 'question' ? prompt : null - const freeText = decodeCodexQuestionOptionId(input.optionId) - const acceptsFreeText = - question?.freeTextQuestionId !== undefined && - freeText?.questionId === question.freeTextQuestionId && - freeText.answer.trim().length > 0 - const grouped = question?.questions ? decodeAgentSessionQuestionAnswers(input.optionId) : null - const acceptsGrouped = - grouped !== null && - question?.questions !== undefined && - isValidAgentSessionQuestionAnswers(question.questions, grouped) - if ( - !acceptsFreeText && - !acceptsGrouped && - !prompt.options.some((option) => option.id === input.optionId) - ) { - return invalid(`Option ${input.optionId} is not offered by item ${input.itemId}.`) + const choice = readPromptChoice(prompt, input) + if (!choice) { + return invalid( + input.optionId !== undefined + ? `Option ${input.optionId} is not offered by item ${input.itemId}.` + : `The answers do not match the questions on item ${input.itemId}.` + ) } + const { response } = choice const identity = parseAgentJournalItemKey(input.itemId) if (!identity) { return invalid(`Item id ${input.itemId} is not a well-formed item key.`) @@ -53,7 +86,8 @@ export async function performPrompt( const resolution: AgentJournalResolution = { state: 'resolved', - selectedOptionId: input.optionId, + selectedOptionId: choice.selectedOptionId, + ...(response.kind === 'answers' ? { answers: response.answers } : {}), resolvedBy: ctx.resolvedBy, resolvedAt: ctx.now() } @@ -63,7 +97,7 @@ export async function performPrompt( sessionId: ctx.sessionId, itemId: input.itemId, kind: input.kind, - optionId: input.optionId, + response, fence: ctx.fence, commit: async () => { committed.item = await ctx.journal.appendItem( @@ -77,7 +111,11 @@ export async function performPrompt( } }) } catch (error) { - if (!committed.item && error instanceof AgentSessionPromptUnavailableError) { + if ( + !committed.item && + (error instanceof AgentSessionPromptUnavailableError || + error instanceof AgentSessionPromptAnswerRejectedError) + ) { return invalid(error.message) } if (!committed.item) { diff --git a/src/main/runtime/rpc/methods/structured-agent-session-respond-params.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-respond-params.test.ts new file mode 100644 index 00000000000..3eb7ad3db61 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-respond-params.test.ts @@ -0,0 +1,106 @@ +// Wire bounds for prompt responses: a decision id for an approval, structured answers for a question. + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { + call, + clearStructuredHostStub, + envelope, + hostCalls, + installStructuredHostStub, + STRUCTURED_CLIENT +} from './structured-agent-session-rpc.test-fixture' + +beforeEach(() => { + installStructuredHostStub() +}) + +afterEach(() => { + clearStructuredHostStub() +}) + +describe('prompt response parameters', () => { + const rejects = async (method: string, params: unknown): Promise => { + const response = await call(method, params, STRUCTURED_CLIENT) + expect(response).toMatchObject({ ok: false, error: { code: 'invalid_argument' } }) + } + + it('accepts the maximum fully encoded Claude choice group and retains a finite bound', async () => { + const maximumSelections = Array.from({ length: 4 }, (_, questionIndex) => ({ + questionId: `q${questionIndex + 1}`, + optionIds: Array.from( + { length: 4 }, + (_, optionIndex) => `q${questionIndex + 1}:choice-${optionIndex + 1}` + ) + })) + const optionId = `question-group:${encodeURIComponent(JSON.stringify(maximumSelections))}` + expect(optionId.length).toBe(610) + + const response = await call( + 'agentSession.respondToQuestion', + { + envelope: envelope(), + itemId: 'item-1', + expectedRevision: 1, + optionId + }, + STRUCTURED_CLIENT + ) + expect(response).toMatchObject({ ok: true }) + expect(hostCalls.respondToPrompt).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ optionId }) + ) + + await rejects('agentSession.respondToQuestion', { + envelope: envelope(), + itemId: 'item-1', + expectedRevision: 1, + optionId: 'x'.repeat(1025) + }) + }) + + it('takes a long typed answer as structured answers and bounds each field', async () => { + const answers = [ + { questionId: 'q1', optionIds: ['q1:choice-1'] }, + { questionId: 'q2', optionIds: [], other: 'Proceed with the replacement. '.repeat(100) } + ] + const response = await call( + 'agentSession.respondToQuestion', + { envelope: envelope(), itemId: 'item-1', expectedRevision: 1, answers }, + STRUCTURED_CLIENT + ) + expect(response).toMatchObject({ ok: true }) + expect(hostCalls.respondToPrompt).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ kind: 'question', answers }) + ) + + const base = { envelope: envelope(), itemId: 'item-1', expectedRevision: 1 } + await rejects('agentSession.respondToQuestion', base) + await rejects('agentSession.respondToQuestion', { ...base, optionId: 'q1:choice-1', answers }) + await rejects('agentSession.respondToQuestion', { + ...base, + answers: [{ questionId: 'q1', optionIds: [], other: 'x'.repeat(64 * 1024 + 1) }] + }) + await rejects('agentSession.respondToQuestion', { + ...base, + answers: [{ questionId: 'q1', optionIds: [], other: 'é'.repeat(40 * 1024) }] + }) + await rejects('agentSession.respondToApproval', { ...base, answers }) + await rejects('agentSession.respondToApproval', { ...base, optionId: 'x'.repeat(1025) }) + }) + + it('takes a question id exactly as the agent wrote it, including edge spaces', async () => { + const answers = [{ questionId: 'scope ', optionIds: [], other: 'mine' }] + const response = await call( + 'agentSession.respondToQuestion', + { envelope: envelope(), itemId: 'item-1', expectedRevision: 1, answers }, + STRUCTURED_CLIENT + ) + expect(response).toMatchObject({ ok: true }) + expect(hostCalls.respondToPrompt).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ kind: 'question', answers }) + ) + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts b/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts index 9daf97d55af..345fdf5bf47 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-schemas.ts @@ -17,6 +17,7 @@ export { MutationEnvelope, OptionsParams, RespondParams, + RespondToQuestionParams, RestartResumableParams, RestartResumeParams, RewindParams, diff --git a/src/main/runtime/rpc/methods/structured-agent-session.test.ts b/src/main/runtime/rpc/methods/structured-agent-session.test.ts index 2d2760832ad..38b55fd70eb 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.test.ts @@ -783,41 +783,6 @@ describe('parameter validation', () => { }) }) - it('accepts the maximum fully encoded Claude choice group and retains a finite bound', async () => { - const maximumSelections = Array.from({ length: 4 }, (_, questionIndex) => ({ - questionId: `q${questionIndex + 1}`, - optionIds: Array.from( - { length: 4 }, - (_, optionIndex) => `q${questionIndex + 1}:choice-${optionIndex + 1}` - ) - })) - const optionId = `question-group:${encodeURIComponent(JSON.stringify(maximumSelections))}` - expect(optionId.length).toBe(610) - - const response = await call( - 'agentSession.respondToQuestion', - { - envelope: envelope(), - itemId: 'item-1', - expectedRevision: 1, - optionId - }, - STRUCTURED_CLIENT - ) - expect(response).toMatchObject({ ok: true }) - expect(hostCalls.respondToPrompt).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ optionId }) - ) - - await rejects('agentSession.respondToQuestion', { - envelope: envelope(), - itemId: 'item-1', - expectedRevision: 1, - optionId: 'x'.repeat(1025) - }) - }) - it('bounds a history page and validates its cursor', async () => { await rejects('agentSession.history', { sessionId: SESSION, diff --git a/src/main/runtime/rpc/methods/structured-agent-session.ts b/src/main/runtime/rpc/methods/structured-agent-session.ts index 66d44d4b782..ebff420cb00 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session.ts @@ -56,6 +56,7 @@ import { HandoffStatusParams, OptionsParams, RespondParams, + RespondToQuestionParams, RewindParams, SendParams, SetOptionParams, @@ -222,7 +223,7 @@ export const STRUCTURED_AGENT_SESSION_METHODS = [ }), defineMethod({ name: 'agentSession.respondToQuestion', - params: RespondParams, + params: RespondToQuestionParams, handler: async (params, ctx) => requireHost(ctx).respondToPrompt(callerFor(ctx), { ...params, kind: 'question' }) }), diff --git a/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx b/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx index 9b1a2967682..189815f2478 100644 --- a/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsx @@ -69,6 +69,21 @@ function clickAction(text: string): void { click(button, text) } +function typeAnswer(value: string): void { + const input = container.querySelector('input')! + act(() => { + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + setter.call(input, value) + input.dispatchEvent(new Event('input', { bubbles: true })) + }) +} + +function optionPressed(label: string): string | null | undefined { + return [...container.querySelectorAll('button[aria-pressed]')] + .find((b) => b.textContent?.includes(label)) + ?.getAttribute('aria-pressed') +} + const tabsOrSpaces: AskPrompt = { questions: [ { @@ -222,4 +237,123 @@ describe('NativeChatQuestionCard', () => { { indices: [], other: 'SSH host' } ]) }) + + it('replaces a picked option with a typed answer on a single-select question', () => { + const onAnswer = vi.fn() + render(tabsOrSpaces, onAnswer) + + clickOption('Spaces') + typeAnswer('two spaces') + expect(optionPressed('Spaces')).toBe('false') + clickAction('Submit') + + expect(onAnswer).toHaveBeenCalledWith([{ indices: [], other: 'two spaces' }]) + }) + + it('keeps typed text in the field but sends a later-picked option', () => { + const onAnswer = vi.fn() + render(tabsOrSpaces, onAnswer) + + typeAnswer('two spaces') + clickOption('Tabs') + expect(container.querySelector('input')!.value).toBe('two spaces') + clickAction('Submit') + + expect(onAnswer).toHaveBeenCalledWith([{ indices: [0], other: '' }]) + }) + + it('chooses the kept typed text again when its field is clicked', () => { + const onAnswer = vi.fn() + render(tabsOrSpaces, onAnswer) + + typeAnswer('two spaces') + clickOption('Tabs') + act(() => { + container.querySelector('input')!.dispatchEvent(new MouseEvent('click', { bubbles: true })) + }) + expect(optionPressed('Tabs')).toBe('false') + clickAction('Submit') + + expect(onAnswer).toHaveBeenCalledWith([{ indices: [], other: 'two spaces' }]) + }) + + it('ignores a click on the answer field while the answer is sending', () => { + const renderCard = (isSubmitting: boolean): void => { + act(() => { + root.render( + {}} + isSubmitting={isSubmitting} + /> + ) + }) + } + renderCard(false) + typeAnswer('two spaces') + clickOption('Tabs') + renderCard(true) + // Chromium still delivers pointer events to a disabled input. + const input = container.querySelector('input')! + act(() => { + input.dispatchEvent(new MouseEvent('pointerdown', { bubbles: true })) + input.dispatchEvent(new MouseEvent('click', { bubbles: true })) + }) + + expect(optionPressed('Tabs')).toBe('true') + }) + + it('keeps the picked option when keyboard focus passes through the field', () => { + const onAnswer = vi.fn() + render(tabsOrSpaces, onAnswer) + + typeAnswer('two spaces') + clickOption('Tabs') + act(() => { + container.querySelector('input')!.dispatchEvent(new FocusEvent('focusin', { bubbles: true })) + }) + clickAction('Submit') + + expect(onAnswer).toHaveBeenCalledWith([{ indices: [0], other: '' }]) + }) + + it('leaves nothing chosen when a picked option is unpicked over kept text', () => { + const onAnswer = vi.fn() + render(tabsOrSpaces, onAnswer) + + typeAnswer('two spaces') + clickOption('Tabs') + clickOption('Tabs') + + expect(optionPressed('Tabs')).toBe('false') + expect( + [...container.querySelectorAll('button')].some((b) => b.textContent?.trim() === 'Skip') + ).toBe(true) + expect(onAnswer).not.toHaveBeenCalled() + }) + + it('sends picked options and typed text together on a multi-select question', () => { + const onAnswer = vi.fn() + render( + { + questions: [ + { + question: 'Which targets?', + multiSelect: true, + options: [{ label: 'Web' }, { label: 'Mobile' }] + } + ] + }, + onAnswer + ) + + clickOption('Mobile') + typeAnswer('Desktop') + typeAnswer('') + typeAnswer('Desktop app') + clickAction('Submit') + + expect(onAnswer).toHaveBeenCalledWith([{ indices: [1], other: 'Desktop app' }]) + }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx b/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx index 0167fbb61f3..e26131d96f0 100644 --- a/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatQuestionCard.tsx @@ -18,11 +18,16 @@ export type NativeChatQuestionCardProps = { answerInputRef?: RefObject } +// Selection entry for the typed answer (never a real option index), so a single-select +// question holds exactly one choice: an option or the typed answer. +const TYPED_ANSWER = -1 + /** * Native renderer for an agent's AskUserQuestion prompt: a numbered pick-list * (mobile/Claude-Code parity) with a header + close, a hover-highlighted row per * option, and an optional free-text row for a custom answer. Single-select - * commits on click; multi-select toggles and confirms via the trailing action. + * holds one answer (an option or the typed text, whichever was chosen last); + * multi-select toggles options, adds any typed text, and confirms via the trailing action. * Multi-question prompts step through tabs across the top. Neutral shadcn tokens. */ export function NativeChatQuestionCard({ @@ -44,29 +49,55 @@ export function NativeChatQuestionCard({ const q = prompt.questions[index]! const questionAllowsOther = Array.isArray(allowOther) ? (allowOther[index] ?? false) : allowOther + // Picking an option replaces a chosen typed answer on single-select; the text stays in + // the field, unsent, until the user types or clicks there again. + const typedAnswerChosen = (qi: number, sel = selections, oth = otherText): boolean => + (sel[qi] ?? []).includes(TYPED_ANSWER) && (oth[qi] ?? '').trim().length > 0 + + const chooseTypedAnswer = (qi: number): void => { + setSelections((prev) => { + const cur = prev[qi] ?? [] + if (cur.includes(TYPED_ANSWER)) { + return prev + } + const chosen = prompt.questions[qi]?.multiSelect ? [...cur, TYPED_ANSWER] : [TYPED_ANSWER] + return prev.map((s, i) => (i === qi ? chosen : s)) + }) + } + + const pickedOptions = (qi: number, sel = selections): number[] => + (sel[qi] ?? []).filter((choice) => choice !== TYPED_ANSWER) + const setOther = (qi: number, value: string): void => { setOtherText((prev) => { const next = [...prev] next[qi] = value return next }) + if (value.trim().length > 0) { + chooseTypedAnswer(qi) + } } - // The resolved answer for a question: picked labels plus any typed free-text. + // The resolved answer for a question: picked labels plus the typed answer when chosen. const answerFor = (qi: number, sel = selections, oth = otherText): string => { const question = prompt.questions[qi] - const picked = (sel[qi] ?? []) + const picked = pickedOptions(qi, sel) .map((optionIndex) => question?.options[optionIndex]?.label ?? '') .filter((label) => label.length > 0) - const other = (oth[qi] ?? '').trim() + const other = typedAnswerChosen(qi, sel, oth) ? (oth[qi] ?? '').trim() : '' return [...picked, ...(other ? [other] : [])].join(', ') } const currentAnswered = answerFor(index).length > 0 + const currentTypedAnswerChosen = typedAnswerChosen(index) const submitAll = (sel: number[][], oth: string[]): void => { const resolved: AskAnswerSelection[] = prompt.questions.map((_, i) => { - return { indices: [...(sel[i] ?? [])], other: (oth[i] ?? '').trim() } + return { + indices: pickedOptions(i, sel), + other: typedAnswerChosen(i, sel, oth) ? (oth[i] ?? '').trim() : '' + } }) const anyAnswered = resolved.some((s) => s.indices.length > 0 || (s.other ?? '').length > 0) if (anyAnswered) { @@ -192,8 +223,19 @@ export function NativeChatQuestionCard({
{questionAllowsOther ? ( <> - - + + {currentTypedAnswerChosen ? ( + + ) : ( + + )} {/* No `/` or `@` picker here — that autocomplete belongs to the composer, which this card replaces. What you type is delivered verbatim as the @@ -205,6 +247,13 @@ export function NativeChatQuestionCard({ disabled={isSubmitting} value={otherText[index]} onChange={(e) => setOther(index, e.target.value)} + // Click, not focus: tabbing through the field toward Submit must not + // replace the option the user just picked. + onClick={() => { + if ((otherText[index] ?? '').trim().length > 0) { + chooseTypedAnswer(index) + } + }} onKeyDown={(e) => { if (e.key === 'Enter') { e.preventDefault() @@ -215,7 +264,12 @@ export function NativeChatQuestionCard({ 'components.native-chat.question.otherPlaceholder', 'Type your answer' )} - className="min-w-0 flex-1 bg-transparent text-sm text-foreground outline-none placeholder:text-muted-foreground/60 disabled:cursor-default disabled:opacity-50" + className={cn( + 'min-w-0 flex-1 bg-transparent text-sm outline-none placeholder:text-muted-foreground/60 disabled:cursor-default disabled:opacity-50', + currentTypedAnswerChosen || !otherText[index] + ? 'text-foreground' + : 'text-muted-foreground' + )} /> ) : ( diff --git a/src/renderer/src/components/native-chat/NativeChatResolutionReceipt.test.tsx b/src/renderer/src/components/native-chat/NativeChatResolutionReceipt.test.tsx index 8dc70aab15e..fcf13956b9c 100644 --- a/src/renderer/src/components/native-chat/NativeChatResolutionReceipt.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatResolutionReceipt.test.tsx @@ -225,4 +225,22 @@ describe('resolution receipts', () => { ).toEqual([{ question: null, answer: null }]) } }) + + it('reads the recorded structured answers before the packed form', () => { + const typed = 'Wait for the capture to finish. '.repeat(50).trim() + const body: AgentJournalQuestionItem = { + kind: 'question', + question: 'Name?', + options: [{ id: 'q1:choice-1', label: 'Default' }], + freeTextQuestionId: 'q1', + resolution: { + ...approval.resolution, + // The packed copy only older clients read; it must not win over the recorded answer. + selectedOptionId: 'q1:choice-1', + answers: [{ questionId: 'q1', optionIds: [], other: typed }] + } + } + + expect(nativeChatReceiptAnswers(body)).toEqual([{ question: null, answer: typed }]) + }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx index 286ae50a3c8..e766955b83e 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx @@ -2,7 +2,6 @@ import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' -import { decodeAgentSessionQuestionAnswers } from '../../../../shared/agent-session-question-answer' import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' import { useAppStore } from '@/store' import { @@ -293,7 +292,10 @@ describe('NativeChatStructuredSession', () => { expect(document.querySelector('[data-native-chat-background-tasks="true"]')).not.toBeNull() act(() => mocks.approvalCardProps?.onChoose('allow')) - expect(mocks.respond).toHaveBeenCalledWith(approvalItems[0], 'allow') + expect(mocks.respond).toHaveBeenCalledWith(approvalItems[0], { + kind: 'option', + optionId: 'allow' + }) expect(mocks.messageListProps?.showLiveTurnActivity).toBe(false) act(() => mocks.approvalCardProps?.onCancel?.()) @@ -535,14 +537,16 @@ describe('NativeChatStructuredSession', () => { { indices: [0, 1], other: '' }, { indices: [], other: 'SSH host' } ]) - const encoded = mocks.respond.mock.calls[0]?.[1] - expect(decodeAgentSessionQuestionAnswers(encoded)).toEqual([ - { questionId: 'q1', optionIds: ['target-web', 'target-mobile'] }, - { questionId: 'q2', optionIds: [], other: 'SSH host' } - ]) + expect(mocks.respond).toHaveBeenCalledWith(mocks.promptItems[0], { + kind: 'answers', + answers: [ + { questionId: 'q1', optionIds: ['target-web', 'target-mobile'] }, + { questionId: 'q2', optionIds: [], other: 'SSH host' } + ] + }) }) - it('keeps legacy single-question option ids and free text behavior', () => { + it('answers a single-question item as its one question', () => { mocks.promptItems = legacySingleQuestionPromptItems render( @@ -568,6 +572,14 @@ describe('NativeChatStructuredSession', () => { } ]) card.onAnswer([{ indices: [1], other: '' }]) - expect(mocks.respond).toHaveBeenCalledWith(mocks.promptItems[0], 'q1:choice-2') + expect(mocks.respond).toHaveBeenLastCalledWith(mocks.promptItems[0], { + kind: 'answers', + answers: [{ questionId: 'q1', optionIds: ['q1:choice-2'] }] + }) + card.onAnswer([{ indices: [], other: ' Svelte ' }]) + expect(mocks.respond).toHaveBeenLastCalledWith(mocks.promptItems[0], { + kind: 'answers', + answers: [{ questionId: 'q1', optionIds: [], other: 'Svelte' }] + }) }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx index 4e18f0bf9c8..d4fbbbf62ba 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx @@ -1,5 +1,5 @@ import { useMemo, useRef, useState } from 'react' -import { encodeAgentSessionQuestionAnswers } from '../../../../shared/agent-session-question-answer' +import { agentSessionPromptQuestions } from '../../../../shared/agent-session-question-answer' import { dispatchStructuredAgentSessionComposerCommand } from '../../../../shared/structured-agent-session-composer' import { structuredAgentSessionPaneKey } from '../../../../shared/structured-agent-session-projection' import type { NativeChatLiveSession } from './use-native-chat-live-session' @@ -27,10 +27,6 @@ import { useStructuredAgentSessionHostExecutionPhase } from './StructuredAgentSe import { structuredAgentLabel } from '@/lib/structured-agent-session-launch-label' import { NativeChatThreadGoalBanner } from './NativeChatThreadGoalBanner' -function encodeQuestionAnswer(questionId: string, answer: string): string { - return `${encodeURIComponent(questionId)}:${encodeURIComponent(answer)}` -} - export function NativeChatStructuredSession( props: Omit ): React.JSX.Element { @@ -146,21 +142,7 @@ export function NativeChatStructuredSession( composerReady: prompt === null }) const questionBody = prompt?.body.kind === 'question' ? prompt.body : null - const questions = - questionBody?.questions ?? - (questionBody - ? [ - { - id: questionBody.freeTextQuestionId ?? 'q1', - question: questionBody.question, - options: questionBody.options, - multiSelect: false, - ...(questionBody.freeTextQuestionId - ? { freeTextQuestionId: questionBody.freeTextQuestionId } - : {}) - } - ] - : []) + const questions = questionBody ? agentSessionPromptQuestions(questionBody) : [] const structuredTransport = useMemo(() => { const threadGoal = controller.threadGoal const setThreadGoalObjective = threadGoal @@ -295,7 +277,7 @@ export function NativeChatStructuredSession( void controller.respond(prompt, optionId)} + onChoose={(optionId) => void controller.respond(prompt, { kind: 'option', optionId })} onCancel={cancelPrompt} shouldFocus={props.isVisible && props.isFocusedGroup} onLinkClick={onLinkClick} @@ -318,35 +300,17 @@ export function NativeChatStructuredSession( }} allowOther={questions.map((question) => Boolean(question.freeTextQuestionId))} onAnswer={(answers) => { - if (questionBody.questions) { - const grouped = questions.map((question, questionIndex) => { - const answer = answers[questionIndex] - const other = answer?.other?.trim() - const optionIds = (answer?.indices ?? []).flatMap((optionIndex) => { - const optionId = question.options[optionIndex]?.id - return optionId ? [optionId] : [] - }) - return { - questionId: question.id, - optionIds: question.multiSelect || !other ? optionIds : [], - ...(other ? { other } : {}) - } + const chosen = questions.map((question, questionIndex) => { + const answer = answers[questionIndex] + const other = answer?.other?.trim() + const optionIds = (answer?.indices ?? []).flatMap((optionIndex) => { + const optionId = question.options[optionIndex]?.id + return optionId ? [optionId] : [] }) - if (grouped.every((answer) => answer.optionIds.length > 0 || answer.other)) { - void controller.respond(prompt, encodeAgentSessionQuestionAnswers(grouped)) - } - return - } - const index = answers[0]?.indices[0] - const other = answers[0]?.other?.trim() - const optionId = - typeof index === 'number' - ? questionBody.options[index]?.id - : questionBody.freeTextQuestionId && other - ? encodeQuestionAnswer(questionBody.freeTextQuestionId, other) - : undefined - if (optionId) { - void controller.respond(prompt, optionId) + return { questionId: question.id, optionIds, ...(other ? { other } : {}) } + }) + if (chosen.every((answer) => answer.optionIds.length > 0 || answer.other)) { + void controller.respond(prompt, { kind: 'answers', answers: chosen }) } }} onCancel={cancelPrompt} diff --git a/src/renderer/src/components/native-chat/native-chat-resolution-receipt.ts b/src/renderer/src/components/native-chat/native-chat-resolution-receipt.ts index f9a0c64437d..475966a1b03 100644 --- a/src/renderer/src/components/native-chat/native-chat-resolution-receipt.ts +++ b/src/renderer/src/components/native-chat/native-chat-resolution-receipt.ts @@ -2,7 +2,10 @@ import type { AgentJournalApprovalItem, AgentJournalQuestionItem } from '../../../../shared/agent-session-journal-types' -import { decodeAgentSessionQuestionAnswers } from '../../../../shared/agent-session-question-answer' +import { + agentSessionPromptQuestions, + legacyAgentSessionQuestionAnswers +} from '../../../../shared/agent-session-question-answer' export type NativeChatResolvedPrompt = AgentJournalApprovalItem | AgentJournalQuestionItem export type NativeChatReceiptAnswer = { question: string | null; answer: string | null } @@ -14,16 +17,19 @@ export function nativeChatReceiptAnswers( return [] } const selected = body.resolution.selectedOptionId - if (body.kind === 'question' && body.questions) { - const answers = selected ? decodeAgentSessionQuestionAnswers(selected) : null - return body.questions.map((question) => { + if (body.kind === 'question') { + // Rows written before hosts recorded structured answers carry only the packed form. + const answers = + body.resolution.answers ?? + (selected ? legacyAgentSessionQuestionAnswers(body, selected) : null) + return agentSessionPromptQuestions(body).map((question) => { const answer = answers?.find((entry) => entry.questionId === question.id) const labels = answer?.optionIds.map( (id) => question.options.find((option) => option.id === id)?.label ) const valid = labels?.every((label) => label !== undefined) return { - question: question.question, + question: body.questions ? question.question : null, answer: valid ? [...(labels ?? []), ...(answer?.other ? [answer.other] : [])].join(' · ') || null : null @@ -31,20 +37,5 @@ export function nativeChatReceiptAnswers( }) } const option = body.options.find((option) => option.id === selected) - if (option) { - return [{ question: null, answer: option.label }] - } - if (body.kind === 'question' && body.freeTextQuestionId && selected) { - const prefix = `${encodeURIComponent(body.freeTextQuestionId)}:` - if (selected.startsWith(prefix)) { - try { - return [ - { question: null, answer: decodeURIComponent(selected.slice(prefix.length)) || null } - ] - } catch { - // Malformed persisted answers remain readable as an unavailable selection. - } - } - } - return [{ question: null, answer: null }] + return [{ question: null, answer: option?.label ?? null }] } diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session-question-answers.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session-question-answers.test.tsx new file mode 100644 index 00000000000..337b22b4f5e --- /dev/null +++ b/src/renderer/src/components/native-chat/use-structured-agent-session-question-answers.test.tsx @@ -0,0 +1,164 @@ +// @vitest-environment happy-dom + +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + call: vi.fn(), + promptCancelSupported: vi.fn(), + questionAnswersSupported: vi.fn(), + operationId: vi.fn(() => 'operation-1') +})) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.call, + supportsStructuredAgentSessionPromptCancel: mocks.promptCancelSupported, + supportsStructuredAgentSessionQuestionAnswers: mocks.questionAnswersSupported +})) +vi.mock('./use-structured-agent-session-read', () => ({ + useStructuredAgentSessionRead: () => ({ + state: { + fence: 3, + items, + submissions: [], + status: 'ready', + error: null, + hasOlder: false, + handoff: null + }, + loadingOlder: false, + loadOlder: vi.fn() + }) +})) +vi.mock('./use-structured-agent-session-outbox', () => ({ + structuredSessionOperationId: mocks.operationId, + useStructuredAgentSessionOutbox: () => ({ + outbox: [], + blockedClientMessageId: null, + error: null, + send: vi.fn(), + retry: vi.fn() + }) +})) + +import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types' +import { encodeAgentSessionQuestionAnswers } from '../../../../shared/agent-session-question-answer' +import { + useStructuredAgentSession, + type StructuredPromptItem +} from './use-structured-agent-session' + +let items: AgentJournalRenderItem[] = [] +const target = { kind: 'local' } as const + +const question: StructuredPromptItem = { + itemId: 'question-1', + revision: 2, + sequence: 2, + observedAt: 2, + body: { + kind: 'question', + question: '1 grouped question from Claude', + options: [], + questions: [ + { + id: 'q1', + question: 'Which option?', + multiSelect: false, + options: [{ id: 'q1:choice-1', label: 'Alpha' }], + freeTextQuestionId: 'q1' + } + ], + resolution: { state: 'pending', selectedOptionId: null, resolvedBy: null, resolvedAt: null } + } +} + +const approval: StructuredPromptItem = { + itemId: 'approval-1', + revision: 3, + sequence: 3, + observedAt: 3, + body: { + kind: 'approval', + title: 'Allow Bash?', + detail: null, + options: [{ id: 'allow', label: 'Allow' }], + resolution: { state: 'pending', selectedOptionId: null, resolvedBy: null, resolvedAt: null } + } +} + +const answers = [{ questionId: 'q1', optionIds: [], other: 'Wait for the capture. '.repeat(80) }] + +function respondCall(): Record | undefined { + return mocks.call.mock.calls.find(([, method]) => + String(method).startsWith('agentSession.respondTo') + )?.[2] +} + +describe('desktop structured prompt answers', () => { + beforeEach(() => { + vi.clearAllMocks() + items = [question, approval] + mocks.call.mockResolvedValue({ ok: true, value: { itemId: 'question-1', revision: 3 } }) + }) + + it('sends structured answers to a host that takes them', async () => { + mocks.questionAnswersSupported.mockResolvedValue(true) + const { result } = renderHook(() => + useStructuredAgentSession({ + sessionId: 'session-1', + target, + agent: 'claude', + isVisible: true + }) + ) + + await act(async () => { + await result.current.respond(question, { kind: 'answers', answers }) + }) + + expect(mocks.questionAnswersSupported).toHaveBeenCalledWith(target) + expect(respondCall()).toMatchObject({ itemId: 'question-1', expectedRevision: 2, answers }) + expect(respondCall()).not.toHaveProperty('optionId') + }) + + it('packs the answer into an option id for a host that predates structured answers', async () => { + mocks.questionAnswersSupported.mockResolvedValue(false) + const { result } = renderHook(() => + useStructuredAgentSession({ + sessionId: 'session-1', + target, + agent: 'claude', + isVisible: true + }) + ) + + await act(async () => { + await result.current.respond(question, { kind: 'answers', answers }) + }) + + expect(respondCall()).toMatchObject({ + itemId: 'question-1', + optionId: encodeAgentSessionQuestionAnswers(answers) + }) + expect(respondCall()).not.toHaveProperty('answers') + }) + + it('sends an approval decision without probing the host', async () => { + const { result } = renderHook(() => + useStructuredAgentSession({ + sessionId: 'session-1', + target, + agent: 'claude', + isVisible: true + }) + ) + + await act(async () => { + await result.current.respond(approval, { kind: 'option', optionId: 'allow' }) + }) + + expect(mocks.questionAnswersSupported).not.toHaveBeenCalled() + expect(respondCall()).toMatchObject({ itemId: 'approval-1', optionId: 'allow' }) + }) +}) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.ts b/src/renderer/src/components/native-chat/use-structured-agent-session.ts index 640b3b2a7c4..9db1e6ff075 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.ts @@ -8,7 +8,14 @@ import type { } from '../../../../shared/agent-session-conversation-command' import type { AgentType } from '../../../../shared/agent-status-types' import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' -import { supportsStructuredAgentSessionPromptCancel } from '@/runtime/structured-agent-session-client' +import { + supportsStructuredAgentSessionPromptCancel, + supportsStructuredAgentSessionQuestionAnswers +} from '@/runtime/structured-agent-session-client' +import { + legacyAgentSessionSelectedOptionId, + type AgentSessionPromptResponse +} from '../../../../shared/agent-session-question-answer' import { pendingStructuredSessionPrompts, type StructuredPromptItem @@ -174,14 +181,32 @@ export function useStructuredAgentSession(args: { scope: 'background-tasks', ...(taskId ? { taskId } : {}) }), - respond: (item: StructuredPromptItem, optionId: string) => - mutate( + respond: async (item: StructuredPromptItem, response: AgentSessionPromptResponse) => { + const promptTarget = { itemId: item.itemId, expectedRevision: item.revision } + let fields: Record + if (response.kind === 'option') { + fields = { ...promptTarget, optionId: response.optionId } + } else if (await supportsStructuredAgentSessionQuestionAnswers(target)) { + // Negotiated before mutate fingerprints the call: older hosts reject the strict field. + fields = { ...promptTarget, answers: response.answers } + } else { + const optionId = + item.body.kind === 'question' + ? legacyAgentSessionSelectedOptionId(item.body, response.answers) + : null + if (optionId === null) { + return null + } + fields = { ...promptTarget, optionId } + } + return mutate( item.body.kind === 'approval' ? 'agentSession.respondToApproval' : 'agentSession.respondToQuestion', `agentSession.respondTo:${item.body.kind}`, - { itemId: item.itemId, expectedRevision: item.revision, optionId } - ), + fields + ) + }, optionSnapshot, optionSurface, sessionCommands: transportEnabled ? (state.commands ?? undefined) : undefined, diff --git a/src/renderer/src/runtime/structured-agent-session-client.ts b/src/renderer/src/runtime/structured-agent-session-client.ts index 446f419d1f9..d50fed2b4c3 100644 --- a/src/renderer/src/runtime/structured-agent-session-client.ts +++ b/src/renderer/src/runtime/structured-agent-session-client.ts @@ -9,6 +9,7 @@ import type { AgentSessionConversationOutline } from '../../../shared/agent-sess import { AGENT_SESSION_CONVERSATION_OUTLINE_RUNTIME_CAPABILITY, AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, + AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, AGENT_SESSION_REWIND_RUNTIME_CAPABILITY, type RuntimeCapability } from '../../../shared/protocol-version' @@ -46,6 +47,15 @@ export function supportsStructuredAgentSessionPromptCancel( return structuredAgentSessionHostSupports(target, AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY) } +export function supportsStructuredAgentSessionQuestionAnswers( + target: RuntimeClientTarget +): Promise { + return structuredAgentSessionHostSupports( + target, + AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY + ) +} + /** Null when the host predates the outline, without calling it. A failed read * rejects, so the caller can retry it; the rail maps loaded messages meanwhile. */ export async function readStructuredAgentSessionConversationOutline( diff --git a/src/shared/agent-session-journal-schemas.ts b/src/shared/agent-session-journal-schemas.ts index 29271faa2b1..b7a80634bf8 100644 --- a/src/shared/agent-session-journal-schemas.ts +++ b/src/shared/agent-session-journal-schemas.ts @@ -147,6 +147,15 @@ const Question = z const Resolution = z.object({ state: z.string().min(1), selectedOptionId: z.string().nullable(), + answers: z + .array( + z.object({ + questionId: z.string(), + optionIds: z.array(z.string()), + other: z.string().optional() + }) + ) + .optional(), resolvedBy: z.string().nullable(), resolvedAt: z.number().nullable() }) diff --git a/src/shared/agent-session-journal-types.ts b/src/shared/agent-session-journal-types.ts index a13f3b1f676..f5bd250c0c9 100644 --- a/src/shared/agent-session-journal-types.ts +++ b/src/shared/agent-session-journal-types.ts @@ -8,6 +8,7 @@ // journal rather than skipping or compacting past it. import type { AgentType } from './agent-status-types' +import type { AgentSessionQuestionAnswer } from './agent-session-question-answer' import type { AgentJournalTurnOutcome } from './agent-turn-outcome' import type { NativeChatToolMetadata } from './native-chat-tool-identity' import type { AgentSessionContextUsage } from './agent-session-context-usage' @@ -128,8 +129,11 @@ export type AgentJournalResolutionState = (typeof AGENT_JOURNAL_RESOLUTION_STATE * invoking the provider callback twice. */ export type AgentJournalResolution = { state: AgentJournalResolutionState - /** Option id the winner picked; null while pending or cancelled. */ + /** Option id the winner picked; null while pending or cancelled. For a question, the answer in the + * packed form older clients read; `answers` is the same answer structured. */ selectedOptionId: string | null + /** Question answers. Absent on approvals and on rows written before hosts recorded it. */ + answers?: AgentSessionQuestionAnswer[] /** Opaque client identity of the resolver, for "answered on ". */ resolvedBy: string | null resolvedAt: number | null diff --git a/src/shared/agent-session-question-answer.ts b/src/shared/agent-session-question-answer.ts index 072dfce200a..cc815acf542 100644 --- a/src/shared/agent-session-question-answer.ts +++ b/src/shared/agent-session-question-answer.ts @@ -1,13 +1,109 @@ -import type { AgentJournalQuestion } from './agent-session-journal-types' +import type { AgentJournalQuestion, AgentJournalQuestionItem } from './agent-session-journal-types' const GROUP_ANSWER_PREFIX = 'question-group:' +/** A single-question item has no question list; the client and host must agree on its id. */ +const SINGLE_QUESTION_ID = 'q1' + +/** Largest typed answer to one question, in UTF-8 bytes. */ +export const AGENT_SESSION_QUESTION_ANSWER_MAX_BYTES = 64 * 1024 + export type AgentSessionQuestionAnswer = { questionId: string optionIds: string[] other?: string } +/** What a client chose on a prompt: a decision id for an approval, per-question answers for a question. */ +export type AgentSessionPromptResponse = + | { kind: 'option'; optionId: string } + | { kind: 'answers'; answers: AgentSessionQuestionAnswer[] } + +/** The questions an item asks: its grouped list, or the item itself as one question. */ +export function agentSessionPromptQuestions( + body: Pick +): AgentJournalQuestion[] { + if (body.questions) { + return body.questions + } + return [ + { + id: body.freeTextQuestionId ?? SINGLE_QUESTION_ID, + question: body.question, + options: body.options, + multiSelect: false, + ...(body.freeTextQuestionId ? { freeTextQuestionId: body.freeTextQuestionId } : {}) + } + ] +} + +function encodeLegacyFreeTextAnswer(questionId: string, answer: string): string { + return `${encodeURIComponent(questionId)}:${encodeURIComponent(answer)}` +} + +function decodeLegacyFreeTextAnswer( + optionId: string +): { questionId: string; answer: string } | null { + const separator = optionId.indexOf(':') + if (separator <= 0) { + return null + } + try { + return { + questionId: decodeURIComponent(optionId.slice(0, separator)), + answer: decodeURIComponent(optionId.slice(separator + 1)) + } + } catch { + return null + } +} + +/** + * The answer packed into one option-id string, the only form older peers read: hosts that + * predate structured answers take it as `optionId`, and older clients render receipts from it. + */ +export function legacyAgentSessionSelectedOptionId( + body: Pick, + answers: readonly AgentSessionQuestionAnswer[] +): string | null { + if (body.questions) { + return encodeAgentSessionQuestionAnswers(answers) + } + const [answer] = answers + if (!answer || answers.length !== 1) { + return null + } + const other = answer.other?.trim() + return ( + answer.optionIds[0] ?? (other ? encodeLegacyFreeTextAnswer(answer.questionId, other) : null) + ) +} + +/** Reads an answer an older client packed into `optionId`. */ +export function legacyAgentSessionQuestionAnswers( + body: Pick, + optionId: string +): AgentSessionQuestionAnswer[] | null { + const grouped = body.questions ? decodeAgentSessionQuestionAnswers(optionId) : null + if (grouped) { + return grouped + } + const questions = agentSessionPromptQuestions(body) + const questionId = questions.length === 1 ? questions[0]!.id : null + if (!questionId) { + return null + } + if (body.options.some((option) => option.id === optionId)) { + return [{ questionId, optionIds: [optionId] }] + } + const freeText = decodeLegacyFreeTextAnswer(optionId) + return body.freeTextQuestionId && + freeText?.questionId === body.freeTextQuestionId && + freeText.answer.trim().length > 0 + ? [{ questionId, optionIds: [], other: freeText.answer }] + : null +} + export function encodeAgentSessionQuestionAnswers( answers: readonly AgentSessionQuestionAnswer[] ): string { diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index dac7fa084bd..db9d7f1b544 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -211,6 +211,10 @@ export const AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY = // older host that would reject the whole cancellation instead of falling back to turn stop. export const AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY = 'agent-session.prompt-cancel.v1' as const +// Why: agentSession.respondToQuestion has a strict schema, so clients must not send structured +// `answers` to an older host; they fall back to the answer packed into `optionId`. +export const AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY = + 'agent-session.question-answers.v1' as const // Why: the host now publishes rows for work that is live inside a turn, and such // a row carries `stoppable: false` because no targeted stop can reach it. A // reader that predates the field draws a per-row Stop on every row it is given, @@ -377,6 +381,7 @@ export const RUNTIME_CAPABILITIES = [ AGENT_SESSION_CONVERSATION_OUTLINE_RUNTIME_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_STOP_CAPABILITY, AGENT_SESSION_PROMPT_CANCEL_RUNTIME_CAPABILITY, + AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, AGENT_SESSION_TURN_ITEM_CAPABILITY, AGENT_SESSION_BACKGROUND_TASK_ROW_STOP_CAPABILITY, AGENT_SESSION_KIMI_RESUME_RUNTIME_CAPABILITY, diff --git a/src/shared/rpc-contract/rpc-params-catalog.generated.ts b/src/shared/rpc-contract/rpc-params-catalog.generated.ts index 57d90d2218a..df05a234a88 100644 --- a/src/shared/rpc-contract/rpc-params-catalog.generated.ts +++ b/src/shared/rpc-contract/rpc-params-catalog.generated.ts @@ -474,6 +474,7 @@ import { ModelCatalogParams, OptionsParams, RespondParams, + RespondToQuestionParams, RestartResumableParams, RestartResumeParams, RewindParams, @@ -579,7 +580,7 @@ export const RPC_PARAMS_BY_METHOD = { 'agentSession.options': OptionsParams, 'agentSession.release': HoldParams, 'agentSession.respondToApproval': RespondParams, - 'agentSession.respondToQuestion': RespondParams, + 'agentSession.respondToQuestion': RespondToQuestionParams, 'agentSession.restartContinue': RestartResumeParams, 'agentSession.restartResumable': RestartResumableParams, 'agentSession.restartResumableDismiss': RestartResumableParams, diff --git a/src/shared/rpc-contract/structured-agent-session-params.ts b/src/shared/rpc-contract/structured-agent-session-params.ts index 7f41bd2c4b8..bd9ce3db82a 100644 --- a/src/shared/rpc-contract/structured-agent-session-params.ts +++ b/src/shared/rpc-contract/structured-agent-session-params.ts @@ -2,6 +2,7 @@ import { z } from 'zod' import { isAgentSessionSurfaceTabId } from '../agent-session-surface-tab-id' import { isAgentSessionId } from '../agent-session-record' import { normalizeExecutionHostId } from '../execution-host' +import { AGENT_SESSION_QUESTION_ANSWER_MAX_BYTES } from '../agent-session-question-answer' import { AGENT_SESSION_ID_MAX_LENGTH, AGENT_SESSION_HISTORY_DIRECTIONS, @@ -16,6 +17,10 @@ export const MAX_RESPONSE_OPTION_ID_LENGTH = 1024 export const MAX_PROMPT_BYTES = 256 * 1024 +/** Matches the journal's bounds on one grouped prompt. */ +const MAX_QUESTION_ANSWER_QUESTIONS = 4 +const MAX_QUESTION_ANSWER_OPTIONS = 64 + export const MAX_BLOCKS = 64 export const MAX_OPTION_LABEL = 512 @@ -208,6 +213,50 @@ export const RespondParams = z }) .strict() +const QuestionAnswer = z + .object({ + // Codex question ids are model-written and untrimmed; the host matches them exactly. + questionId: z + .string() + .min(1, 'Invalid question id') + .max(MAX_RESPONSE_OPTION_ID_LENGTH, 'Invalid question id'), + optionIds: z + .array(Identifier('Invalid option id', MAX_RESPONSE_OPTION_ID_LENGTH)) + .max(MAX_QUESTION_ANSWER_OPTIONS), + // Hashed verbatim by both peers, so no trim or transform here. + other: z + .string() + .max(AGENT_SESSION_QUESTION_ANSWER_MAX_BYTES) + .refine( + (value) => Buffer.byteLength(value, 'utf8') <= AGENT_SESSION_QUESTION_ANSWER_MAX_BYTES, + 'Answer is too large' + ) + .optional() + }) + .strict() + +export const RespondToQuestionParams = z + .object({ + envelope: MutationEnvelope, + itemId: Identifier('Invalid item id'), + expectedRevision: z.number().int().positive(), + /** An answer packed into one id, from clients that predate `answers`. */ + optionId: Identifier('Invalid option id', MAX_RESPONSE_OPTION_ID_LENGTH).optional(), + answers: z.array(QuestionAnswer).min(1).max(MAX_QUESTION_ANSWER_QUESTIONS).optional() + }) + .strict() + .superRefine((value, ctx) => { + if ((value.optionId === undefined) === (value.answers === undefined)) { + ctx.addIssue({ code: 'custom', message: 'Send exactly one of an option id or answers' }) + } + if ( + value.answers !== undefined && + Buffer.byteLength(JSON.stringify(value.answers), 'utf8') > MAX_PROMPT_BYTES + ) { + ctx.addIssue({ code: 'custom', message: 'Answer is too large' }) + } + }) + export const SetOptionParams = z .object({ envelope: MutationEnvelope, diff --git a/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts b/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts index d244fba1ba6..0e3a6ae06c9 100644 --- a/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts +++ b/tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts @@ -23,6 +23,7 @@ import { RuntimeSubscriptionRegistry } from '../../../src/main/runtime/runtime-s import type { AgentSessionSubscribeEvent } from '../../../src/shared/agent-session-wire' import { AGENT_SESSION_PENDING_SEND_RESULT_RUNTIME_CAPABILITY, + AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY, AGENT_SESSION_REWIND_RUNTIME_CAPABILITY, AGENT_SESSION_CONVERSATION_OUTLINE_RUNTIME_CAPABILITY, AGENT_SESSION_STATUS_FEED_RUNTIME_CAPABILITY, @@ -42,6 +43,7 @@ import { resetOperationIds, REWIND_METHOD, CONVERSATION_OUTLINE_METHOD, + envelope, STATUS_FEED_METHOD, sendParams, SESSION, @@ -313,6 +315,41 @@ describe('cross-version structured agent sessions', () => { } }) + it('takes structured question answers exactly where the host advertises them', async () => { + // A client sends `answers` only on this capability, so the two must never disagree: + // a strict older schema refuses the field and the answer is lost rather than degraded. + const method = 'agentSession.respondToQuestion' + const fields = { + itemId: 'item-1', + expectedRevision: 1, + answers: [{ questionId: 'q1', optionIds: [], other: 'Wait for the capture. '.repeat(80) }] + } + const params = { envelope: envelope({ method, fields, fence: 1 }), ...fields } + expect(current.capabilities).toContain(AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY) + for (const build of [current, baseline]) { + const advertised = build.capabilities.includes( + AGENT_SESSION_QUESTION_ANSWERS_RUNTIME_CAPABILITY + ) + if (!build.methodNames.includes(method)) { + expect(advertised, `${build.label} advertises answers without the method`).toBe(false) + continue + } + const hostCalls = structuredHostStub(SESSION, WORKSPACE) + await build.installStructuredHost(installableHost(hostCalls)) + try { + const replies = await callBuild(build, method, params, { + clientKind: 'runtime', + clientCapabilities: current.capabilities + }) + expect(replies, `${build.label}: ${method} must answer exactly once`).toHaveLength(1) + expect(replies[0]?.ok, `${build.label}: ${JSON.stringify(replies[0])}`).toBe(advertised) + expect(hostCalls.respondToPrompt).toHaveBeenCalledTimes(advertised ? 1 : 0) + } finally { + await build.installStructuredHost(null) + } + } + }) + it( 'executes every method a release-shaped checkout registers', async () => {