diff --git a/packages/server/src/server/agent/providers/claude/agent.ts b/packages/server/src/server/agent/providers/claude/agent.ts index 61ac0e3ae0..8c5cc1a41c 100644 --- a/packages/server/src/server/agent/providers/claude/agent.ts +++ b/packages/server/src/server/agent/providers/claude/agent.ts @@ -203,24 +203,26 @@ function stripClaudeAskUserQuestionUiMetadata(input: AgentMetadata): AgentMetada }; } -export function normalizeClaudeAskUserQuestionUpdatedInput( +/** + * The answers this response will actually deliver, keyed by full question text, + * or null when there is no question list and answers map to work from. + * + * Shared by the normalizer and by the check that rejects an undeliverable + * answer, so the two can never disagree about what "deliverable" means. + */ +function resolveClaudeAskUserQuestionAnswers( updatedInput: AgentMetadata | undefined, fallbackInput: AgentMetadata | undefined, -): AgentMetadata { +): Record | null { const fallback = isMetadata(fallbackInput) ? fallbackInput : {}; const base = isMetadata(updatedInput) ? updatedInput : {}; - // Paseo's shared question UI serializes answers by question header, but Claude's - // AskUserQuestion tool expects answer keys to match the full question text. Merge - // the original request payload back in so provider callbacks that only return - // `{ answers }` still satisfy Claude's full tool input schema. - const merged = stripClaudeAskUserQuestionUiMetadata({ ...fallback, ...base }); const questions = (Array.isArray(base.questions) ? base.questions : null) ?? (Array.isArray(fallback.questions) ? fallback.questions : null); const answers = isMetadata(base.answers) ? base.answers : null; if (!questions || !answers) { - return merged; + return null; } const normalizedAnswers: Record = {}; @@ -244,7 +246,23 @@ export function normalizeClaudeAskUserQuestionUpdatedInput( } } - if (Object.keys(normalizedAnswers).length === 0) { + return normalizedAnswers; +} + +export function normalizeClaudeAskUserQuestionUpdatedInput( + updatedInput: AgentMetadata | undefined, + fallbackInput: AgentMetadata | undefined, +): AgentMetadata { + const fallback = isMetadata(fallbackInput) ? fallbackInput : {}; + const base = isMetadata(updatedInput) ? updatedInput : {}; + // Paseo's shared question UI serializes answers by question header, but Claude's + // AskUserQuestion tool expects answer keys to match the full question text. Merge + // the original request payload back in so provider callbacks that only return + // `{ answers }` still satisfy Claude's full tool input schema. + const merged = stripClaudeAskUserQuestionUiMetadata({ ...fallback, ...base }); + const normalizedAnswers = resolveClaudeAskUserQuestionAnswers(updatedInput, fallbackInput); + + if (!normalizedAnswers || Object.keys(normalizedAnswers).length === 0) { return merged; } @@ -254,6 +272,32 @@ export function normalizeClaudeAskUserQuestionUpdatedInput( }; } +class ClaudeQuestionAnswerRequiredError extends Error { + readonly code = "question_answer_required" as const; + + constructor(readonly requestId: string) { + super( + `Permission request '${requestId}' requires updatedInput.answers keyed by a ` + + `question's full text or header, with a non-empty string value. ` + + `The request is still pending; retry with that shape.`, + ); + this.name = "ClaudeQuestionAnswerRequiredError"; + } +} + +function assertClaudeQuestionAnswerDeliverable( + request: AgentPermissionRequest, + response: AgentPermissionResponse, +): void { + if (request.kind !== "question" || response.behavior !== "allow") { + return; + } + const answers = resolveClaudeAskUserQuestionAnswers(response.updatedInput, request.input); + if (!answers || Object.keys(answers).length === 0) { + throw new ClaudeQuestionAnswerRequiredError(request.id); + } +} + function isObjectRecord(value: unknown): value is Record { return Boolean(value) && typeof value === "object" && !Array.isArray(value); } @@ -2734,6 +2778,12 @@ class ClaudeAgentSession implements AgentSession { if (!pending) { throw new Error(`No pending permission request with id '${requestId}'`); } + + // Before the delete below, so a rejected answer leaves the request pending + // and answerable rather than consuming it for nothing. AgentManager only + // drops its own copy once this resolves, so both maps survive the throw. + assertClaudeQuestionAnswerDeliverable(pending.request, response); + this.pendingPermissions.delete(requestId); pending.cleanup?.(); diff --git a/packages/server/src/server/agent/providers/claude/question-response.test.ts b/packages/server/src/server/agent/providers/claude/question-response.test.ts new file mode 100644 index 0000000000..26909faab7 --- /dev/null +++ b/packages/server/src/server/agent/providers/claude/question-response.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, test } from "vitest"; +import type { CanUseTool, Query, SDKMessage } from "@anthropic-ai/claude-agent-sdk"; +import { ClaudeAgentClient } from "./agent.js"; +import type { ClaudeQueryFactory } from "./query.js"; +import type { AgentPermissionResponse } from "../../agent-sdk-types.js"; +import { createTestLogger } from "../../../../test-utils/test-logger.js"; + +// The SDK dependency receives the same permission callback as a real Claude process. +// Keep its stream open until close; permission state belongs entirely to the session. +function createSdkFake() { + let canUseTool: CanUseTool | undefined; + let finish!: () => void; + const finished = new Promise((resolve) => { + finish = resolve; + }); + const queryFactory: ClaudeQueryFactory = ({ options }) => { + canUseTool = options.canUseTool; + return { + close: finish, + return: async () => { + finish(); + return { value: undefined, done: true }; + }, + [Symbol.asyncIterator]() { + return { + next: async (): Promise> => { + await finished; + return { value: undefined, done: true }; + }, + }; + }, + } as Query; + }; + return { + queryFactory, + ask: (input: Record) => { + if (!canUseTool) throw new Error("SDK permission callback was not configured"); + return canUseTool("AskUserQuestion", input, { + signal: new AbortController().signal, + toolUseID: "question-tool-call", + }); + }, + }; +} + +const questionInput = { + questions: [{ question: "Which colour?", header: "Colour", options: [], multiSelect: false }], +}; + +const invalidResponses: Array<[string, AgentPermissionResponse]> = [ + ["selectedActionId", { behavior: "allow", selectedActionId: "Viridian" }], + ["missing answers", { behavior: "allow", updatedInput: {} }], + ["unknown question", { behavior: "allow", updatedInput: { answers: { Hue: "Viridian" } } }], + ["blank answer", { behavior: "allow", updatedInput: { answers: { Colour: " " } } }], + ["non-string answer", { behavior: "allow", updatedInput: { answers: { Colour: 1 } } }], + [ + "empty echoed questions", + { behavior: "allow", updatedInput: { questions: [], answers: { Colour: "Viridian" } } }, + ], +]; + +describe("Claude question permission responses", () => { + test.each(invalidResponses)( + "rejects %s and delivers a corrected retry", + async (_label, response) => { + const sdk = createSdkFake(); + const session = await new ClaudeAgentClient({ + logger: createTestLogger(), + queryFactory: sdk.queryFactory, + resolveBinary: async () => "/test/claude/bin", + }).createSession({ provider: "claude", cwd: process.cwd(), modeId: "default" }); + try { + await session.startTurn("Ask which colour"); + const answer = sdk.ask(questionInput); + void answer.catch(() => undefined); + const request = session.getPendingPermissions()[0]!; + expect(request.kind).toBe("question"); + await expect(session.respondToPermission(request.id, response)).rejects.toMatchObject({ + code: "question_answer_required", + requestId: request.id, + }); + expect(session.getPendingPermissions()).toEqual([request]); + await session.respondToPermission(request.id, { + behavior: "allow", + updatedInput: { answers: { Colour: "Viridian" } }, + }); + await expect(answer).resolves.toMatchObject({ + behavior: "allow", + updatedInput: { answers: { "Which colour?": "Viridian" } }, + }); + expect(session.getPendingPermissions()).toEqual([]); + } finally { + await session.close(); + } + }, + ); + + test.each([ + ["header", questionInput, { Colour: "Viridian" }, { "Which colour?": "Viridian" }], + ["question text", questionInput, { "Which colour?": "Ochre" }, { "Which colour?": "Ochre" }], + [ + "padded question text", + { questions: [{ question: " Which colour? ", header: " Colour " }] }, + { " Which colour? ": "Ochre" }, + { " Which colour? ": "Ochre" }, + ], + [ + "padded header", + { questions: [{ question: " Which colour? ", header: " Colour " }] }, + { " Colour ": "Viridian" }, + { " Which colour? ": "Viridian" }, + ], + [ + "partial answer", + { questions: [...questionInput.questions, { question: "Ship it?", header: "Ship" }] }, + { Ship: "Yes" }, + { "Ship it?": "Yes" }, + ], + ])("delivers answers keyed by %s", async (_label, input, answers, expected) => { + const sdk = createSdkFake(); + const session = await new ClaudeAgentClient({ + logger: createTestLogger(), + queryFactory: sdk.queryFactory, + resolveBinary: async () => "/test/claude/bin", + }).createSession({ provider: "claude", cwd: process.cwd(), modeId: "default" }); + try { + await session.startTurn("Ask which colour"); + const answer = sdk.ask(input); + void answer.catch(() => undefined); + const request = session.getPendingPermissions()[0]!; + await session.respondToPermission(request.id, { + behavior: "allow", + updatedInput: { answers }, + }); + await expect(answer).resolves.toMatchObject({ + behavior: "allow", + updatedInput: { answers: expected }, + }); + expect(session.getPendingPermissions()).toEqual([]); + } finally { + await session.close(); + } + }); + + test("allows denying a question without answers", async () => { + const sdk = createSdkFake(); + const session = await new ClaudeAgentClient({ + logger: createTestLogger(), + queryFactory: sdk.queryFactory, + resolveBinary: async () => "/test/claude/bin", + }).createSession({ provider: "claude", cwd: process.cwd(), modeId: "default" }); + try { + await session.startTurn("Ask which colour"); + const answer = sdk.ask(questionInput); + void answer.catch(() => undefined); + const request = session.getPendingPermissions()[0]!; + await session.respondToPermission(request.id, { behavior: "deny", message: "No answer" }); + await expect(answer).resolves.toMatchObject({ behavior: "deny", message: "No answer" }); + expect(session.getPendingPermissions()).toEqual([]); + } finally { + await session.close(); + } + }); +});