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 () => {