From 6639847425a36bf2a3bab4c91a7a1b94bbdceda0 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sat, 26 Sep 2026 19:50:04 +0200 Subject: [PATCH] fix: answer questions with structured selections The question form sent each answer as comma-joined text, so Codex, OpenCode, and the rpiv-ask-user-question adapter split labels that contain ", " into separate answers. OpenCode v2 forms rejected multi-select answers outright and published `multiple` instead of `multiSelect`. The allow response now carries optional `questionAnswers`, one `{ selected, text? }` per question. AgentManager resolves both forms before the provider sees the response: text-only callers (CLI, MCP, older apps) are parsed by option label, and the header-keyed text answers Claude and plugin providers read are derived when absent. The app still sends the text answers for older daemons (COMPAT(questionAnswers)). --- .../question-form-card-core.test.ts | 41 +++++++ .../src/components/question-form-card-core.ts | 55 ++++++--- .../app/src/components/question-form-card.tsx | 22 ++-- packages/protocol/src/agent-types.ts | 10 ++ packages/protocol/src/messages.ts | 6 + .../server/src/server/agent/agent-manager.ts | 7 +- .../src/server/agent/agent-sdk-types.ts | 10 ++ .../providers/codex-app-server-agent.test.ts | 29 +++++ .../agent/providers/codex-app-server-agent.ts | 33 ++--- .../opencode-agent.full-access.test.ts | 4 +- .../agent/providers/opencode-agent.test.ts | 46 ++++++- .../server/agent/providers/opencode-agent.ts | 16 +-- .../providers/opencode/v2/permissions.test.ts | 26 ++-- .../providers/opencode/v2/permissions.ts | 41 +++---- .../rpiv-ask-user-question/index.test.ts | 39 +++--- .../rpiv-ask-user-question/index.ts | 41 +++---- .../src/server/agent/question-answers.test.ts | 89 ++++++++++++++ .../src/server/agent/question-answers.ts | 114 ++++++++++++++++++ 18 files changed, 490 insertions(+), 139 deletions(-) create mode 100644 packages/server/src/server/agent/question-answers.test.ts create mode 100644 packages/server/src/server/agent/question-answers.ts diff --git a/packages/app/src/components/question-form-card-core.test.ts b/packages/app/src/components/question-form-card-core.test.ts index 40692d4acf..4cfccce6c7 100644 --- a/packages/app/src/components/question-form-card-core.test.ts +++ b/packages/app/src/components/question-form-card-core.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "vitest"; import { areQuestionsAnswered, buildQuestionFormAnswers, + buildQuestionFormResponse, parseQuestionFormQuestions, questionShowsTextInput, resolveDismissLabel, @@ -147,4 +148,44 @@ describe("question form card core", () => { Response: "custom", }); }); + test("submits each question's answer as structured labels so commas stay unambiguous", () => { + const input = { + questions: [ + { + question: "Which colors?", + header: "Colors", + options: [{ label: "Red, bright" }, { label: "Blue" }], + multiSelect: true, + allowOther: true, + }, + { + question: "Which city?", + header: "City", + options: [{ label: "Paris, France" }, { label: "Rome" }], + allowOther: true, + }, + ], + }; + const questions = parseQuestionFormQuestions(input); + if (!questions) throw new Error("questions did not parse"); + + expect( + buildQuestionFormResponse( + input, + questions, + { 0: new Set([0, 1]), 1: new Set([1]) }, + { 0: " Green, dark ", 1: "Lyon, France" }, + ), + ).toEqual({ + behavior: "allow", + questionAnswers: [ + { selected: ["Red, bright", "Blue"], text: "Green, dark" }, + { selected: [], text: "Lyon, France" }, + ], + updatedInput: { + ...input, + answers: { Colors: "Red, bright, Blue, Green, dark", City: "Lyon, France" }, + }, + }); + }); }); diff --git a/packages/app/src/components/question-form-card-core.ts b/packages/app/src/components/question-form-card-core.ts index 0dea6c706e..8d67933b63 100644 --- a/packages/app/src/components/question-form-card-core.ts +++ b/packages/app/src/components/question-form-card-core.ts @@ -1,3 +1,5 @@ +import type { AgentPermissionResponse, AgentQuestionAnswer } from "@getpaseo/protocol/agent-types"; + export interface QuestionOption { label: string; description?: string; @@ -101,35 +103,60 @@ export function areQuestionsAnswered( ); } -export function buildQuestionFormAnswers( +export function buildQuestionFormResponse( + input: Record | undefined, questions: QuestionFormQuestion[], selections: QuestionSelections, otherTexts: QuestionOtherTexts, -): Record { - const answers: Record = {}; - for (let i = 0; i < questions.length; i++) { - const q = questions[i]; +): AgentPermissionResponse { + return { + behavior: "allow", + questionAnswers: buildQuestionAnswers(questions, selections, otherTexts), + updatedInput: { + ...input, + // COMPAT(questionAnswers): added in v0.9.3, remove after 2027-03-26 once daemon floor >= v0.9.3. + // Older daemons read only these text answers; newer daemons derive them from questionAnswers. + answers: buildQuestionFormAnswers(questions, selections, otherTexts), + }, + }; +} + +function buildQuestionAnswers( + questions: QuestionFormQuestion[], + selections: QuestionSelections, + otherTexts: QuestionOtherTexts, +): AgentQuestionAnswer[] { + return questions.map((q, i) => { const selected = selections[i]; const otherText = otherTexts[i]?.trim(); const labels = selected ? Array.from(selected).map((idx) => q.options[idx].label) : []; if (questionShowsTextInput(q)) { if (otherText && otherText.length > 0) { - // Multi-select keeps the checked options and appends the custom answer, the way + // Multi-select keeps the checked options alongside the custom answer, the way // Claude Code's own AskUserQuestion UI does. Single-select replaces the option. - answers[q.header] = q.multiSelect ? [...labels, otherText].join(", ") : otherText; - continue; + return { selected: q.multiSelect ? labels : [], text: otherText }; } if (q.allowEmpty && q.options.length === 0) { - answers[q.header] = ""; - continue; + return { selected: [], text: "" }; } } + return { selected: labels }; + }); +} - if (labels.length > 0) { - answers[q.header] = labels.join(", "); - } - } +export function buildQuestionFormAnswers( + questions: QuestionFormQuestion[], + selections: QuestionSelections, + otherTexts: QuestionOtherTexts, +): Record { + const answers: Record = {}; + buildQuestionAnswers(questions, selections, otherTexts).forEach((answer, i) => { + if (answer.selected.length === 0 && answer.text === undefined) return; + answers[questions[i].header] = [...answer.selected, ...(answer.text ? [answer.text] : [])].join( + ", ", + ); + }); return answers; } diff --git a/packages/app/src/components/question-form-card.tsx b/packages/app/src/components/question-form-card.tsx index 5b3b146f4e..71cbe0f310 100644 --- a/packages/app/src/components/question-form-card.tsx +++ b/packages/app/src/components/question-form-card.tsx @@ -12,7 +12,7 @@ import { EditingTextInput as TextInput } from "@/components/ui/text-input"; import type { EditingTextInputHandle } from "@/components/ui/text-input/types"; import { areQuestionsAnswered, - buildQuestionFormAnswers, + buildQuestionFormResponse, isQuestionAnswered, parseQuestionFormQuestions, questionShowsTextInput, @@ -402,13 +402,9 @@ export function QuestionFormCard({ permission, onRespond, isResponding }: Questi const handleSubmit = useCallback(() => { if (!questions || !allAnswered || isResponding) return; setRespondingAction("submit"); - onRespond({ - behavior: "allow", - updatedInput: { - ...permission.request.input, - answers: buildQuestionFormAnswers(questions, selections, otherTexts), - }, - }); + onRespond( + buildQuestionFormResponse(permission.request.input, questions, selections, otherTexts), + ); }, [ questions, allAnswered, @@ -423,13 +419,9 @@ export function QuestionFormCard({ permission, onRespond, isResponding }: Questi if (!questions) return; setRespondingAction("dismiss"); if (shouldSubmitEmptyOnDismiss(questions)) { - onRespond({ - behavior: "allow", - updatedInput: { - ...permission.request.input, - answers: buildQuestionFormAnswers(questions, selections, otherTexts), - }, - }); + onRespond( + buildQuestionFormResponse(permission.request.input, questions, selections, otherTexts), + ); return; } onRespond({ diff --git a/packages/protocol/src/agent-types.ts b/packages/protocol/src/agent-types.ts index ea412c16be..96ede8ae99 100644 --- a/packages/protocol/src/agent-types.ts +++ b/packages/protocol/src/agent-types.ts @@ -466,12 +466,22 @@ export interface AgentPermissionRequest { metadata?: AgentMetadata; } +/** + * The answer to one question of a `question` permission request, aligned by index with + * `input.questions`. `selected` holds the chosen option labels; `text` holds a typed answer. + */ +export interface AgentQuestionAnswer { + selected: string[]; + text?: string; +} + export type AgentPermissionResponse = | { behavior: "allow"; selectedActionId?: string; updatedInput?: AgentMetadata; updatedPermissions?: AgentPermissionUpdate[]; + questionAnswers?: AgentQuestionAnswer[]; } | { behavior: "deny"; diff --git a/packages/protocol/src/messages.ts b/packages/protocol/src/messages.ts index db72cd3cb6..03bd61a304 100644 --- a/packages/protocol/src/messages.ts +++ b/packages/protocol/src/messages.ts @@ -456,6 +456,11 @@ const AgentPermissionActionSchema = z.object({ intent: z.enum(["implement", "implement_resume", "dismiss"]).optional(), }); +const AgentQuestionAnswerSchema = z.object({ + selected: z.array(z.string()), + text: z.string().optional(), +}); + export const AgentPermissionResponseSchema: z.ZodType = z.discriminatedUnion("behavior", [ z.object({ @@ -463,6 +468,7 @@ export const AgentPermissionResponseSchema: z.ZodType = selectedActionId: z.string().optional(), updatedInput: z.record(z.string(), z.unknown()).optional(), updatedPermissions: z.array(AgentPermissionUpdateSchema).optional(), + questionAnswers: z.array(AgentQuestionAnswerSchema).optional(), }), z.object({ behavior: z.literal("deny"), diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index 061c00e2ca..78ea871d0e 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -54,6 +54,7 @@ import { type ImportableProviderSession, type ListImportableSessionsOptions, } from "./agent-sdk-types.js"; +import { resolveQuestionAnswers } from "./question-answers.js"; import { buildArchivedAgentRecord, type ArchivedStoredAgentRecord } from "./agent-archive.js"; import type { StoredAgentRecord, AgentStorage } from "./agent-storage.js"; import type { AgentOwner } from "./agent-owner.js"; @@ -2981,7 +2982,11 @@ export class AgentManager { agent.inFlightPermissionResponses.add(requestId); try { - const result = await agent.session.respondToPermission(requestId, response); + const request = agent.pendingPermissions.get(requestId); + const result = await agent.session.respondToPermission( + requestId, + request ? resolveQuestionAnswers(request, response) : response, + ); agent.pendingPermissions.delete(requestId); try { diff --git a/packages/server/src/server/agent/agent-sdk-types.ts b/packages/server/src/server/agent/agent-sdk-types.ts index e2fb9779b9..bb4f3f9147 100644 --- a/packages/server/src/server/agent/agent-sdk-types.ts +++ b/packages/server/src/server/agent/agent-sdk-types.ts @@ -504,12 +504,22 @@ export interface AgentPermissionRequest { metadata?: AgentMetadata; } +/** + * The answer to one question of a `question` permission request, aligned by index with + * `input.questions`. `selected` holds the chosen option labels; `text` holds a typed answer. + */ +export interface AgentQuestionAnswer { + selected: string[]; + text?: string; +} + export type AgentPermissionResponse = | { behavior: "allow"; selectedActionId?: string; updatedInput?: AgentMetadata; updatedPermissions?: AgentPermissionUpdate[]; + questionAnswers?: AgentQuestionAnswer[]; } | { behavior: "deny"; diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index 00dc26652c..f47d7eb4bc 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -5042,6 +5042,7 @@ describe("Codex app-server provider", () => { await session.respondToPermission("permission-call-question-2", { behavior: "allow", + questionAnswers: [{ selected: ["Tea"] }], updatedInput: { answers: { Drink: "Tea", @@ -5061,6 +5062,7 @@ describe("Codex app-server provider", () => { requestId: "permission-call-question-2", resolution: { behavior: "allow", + questionAnswers: [{ selected: ["Tea"] }], updatedInput: { answers: { Drink: "Tea", @@ -5100,6 +5102,33 @@ describe("Codex app-server provider", () => { }); }); + test("answers multi-select questions with option labels that contain commas", async () => { + const session = createSession(); + const pendingResponse = asInternals(session).handleToolApprovalRequest({ + itemId: "call-question-3", + threadId: "thread-1", + turnId: "turn-1", + questions: [ + { + id: "colors", + header: "Colors", + question: "Which colors?", + multiSelect: true, + options: [{ label: "Red, bright" }, { label: "Blue" }], + }, + ], + }); + + await session.respondToPermission("permission-call-question-3", { + behavior: "allow", + questionAnswers: [{ selected: ["Red, bright", "Blue"], text: "Green, dark" }], + }); + + await expect(pendingResponse).resolves.toEqual({ + answers: { colors: { answers: ["Red, bright", "Blue", "Green, dark"] } }, + }); + }); + test("emits a synthetic plan approval permission after a successful Codex plan turn", () => { const session = createSession({ featureValues: { plan_mode: true, fast_mode: true }, diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index b7fc721f24..732ac6b40a 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -38,6 +38,7 @@ import { type ProviderRefreshContext, type ResolveAgentDefaultModeInput, } from "../agent-sdk-types.js"; +import { questionAnswerValues } from "../question-answers.js"; import { importSessionFromPersistence } from "../provider-session-import.js"; import { runProviderRefreshActivity } from "../provider-refresh-deadline.js"; import type { Logger } from "pino"; @@ -1264,39 +1265,21 @@ export function mapCodexQuestionRequestToToolCall(params: { }; } -function mapCodexQuestionResponseByHeader(params: { +function mapCodexQuestionResponse(params: { questions: CodexQuestionPrompt[]; response: AgentPermissionResponse; }): Record | null { - if (params.response.behavior !== "allow") { + if (params.response.behavior !== "allow" || !params.response.questionAnswers) { return null; } - const updatedInputRecord = toObjectRecord(params.response.updatedInput); - const answersRecord = toObjectRecord(updatedInputRecord?.answers); - if (!answersRecord) { - return null; - } - + const questionAnswers = params.response.questionAnswers; const answers: Record = {}; - for (const question of params.questions) { - const rawAnswer = answersRecord[question.header]; - if (typeof rawAnswer !== "string") { - continue; - } - const normalizedAnswer = rawAnswer.trim(); - if (!normalizedAnswer) { - continue; - } - const values = question.multiSelect - ? normalizedAnswer - .split(",") - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0) - : [normalizedAnswer]; + params.questions.forEach((question, index) => { + const values = questionAnswerValues(questionAnswers[index]); if (values.length > 0) { answers[question.id] = { answers: values }; } - } + }); return Object.keys(answers).length > 0 ? answers : null; } @@ -4604,7 +4587,7 @@ export class CodexAppServerAgentSession implements AgentSession { ? pendingRequest.metadata.itemId : requestId; if (response.behavior === "allow") { - const mappedAnswers = mapCodexQuestionResponseByHeader({ + const mappedAnswers = mapCodexQuestionResponse({ questions, response, }); diff --git a/packages/server/src/server/agent/providers/opencode-agent.full-access.test.ts b/packages/server/src/server/agent/providers/opencode-agent.full-access.test.ts index d1a5de6f8b..b13b443ef0 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.full-access.test.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.full-access.test.ts @@ -332,7 +332,7 @@ describe("OpenCode auto_accept feature", () => { await session.respondToPermission("question-1", { behavior: "allow", - updatedInput: { answers: { Decision: "Proceed" } }, + questionAnswers: [{ selected: ["Proceed"] }], }); expect(openCodeClient.calls.questionReply).toHaveLength(1); @@ -386,7 +386,7 @@ describe("OpenCode auto_accept feature", () => { await session.respondToPermission("question-1", { behavior: "allow", - updatedInput: { answers: { Decision: "Use another answer" } }, + questionAnswers: [{ selected: [], text: "Use another answer" }], }); expect(openCodeClient.calls.questionReply).toEqual([ diff --git a/packages/server/src/server/agent/providers/opencode-agent.test.ts b/packages/server/src/server/agent/providers/opencode-agent.test.ts index 1ba8b282a9..9c5c1ef602 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.test.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.test.ts @@ -5296,7 +5296,7 @@ describe("OpenCode provider subagent contract", () => { await parent.respondToPermission("question_provider_child", { behavior: "allow", - updatedInput: { answers: { Path: "A" } }, + questionAnswers: [{ selected: ["A"] }], }); expect(parentClient.calls.questionReply).toContainEqual( expect.objectContaining({ @@ -5308,6 +5308,50 @@ describe("OpenCode provider subagent contract", () => { await parent.close(); }); + test("replies to questions with option labels and custom answers that contain commas", async () => { + const { parent, openCode } = await createParentSession("ses_parent_comma_question"); + openCode.emitEvent({ + type: "question.asked", + properties: { + id: "question_comma", + sessionID: "ses_parent_comma_question", + questions: [ + { + question: "Which colors?", + header: "Colors", + multiple: true, + options: [ + { label: "Red, bright", description: "Warm" }, + { label: "Blue", description: "Cool" }, + ], + }, + { + question: "Which city?", + header: "City", + options: [{ label: "Paris, France", description: "Europe" }], + }, + ], + }, + }); + await vi.waitFor(() => expect(parent.getPendingPermissions()).toHaveLength(1)); + + await parent.respondToPermission("question_comma", { + behavior: "allow", + questionAnswers: [ + { selected: ["Red, bright", "Blue"], text: "Green, dark" }, + { selected: ["Paris, France"] }, + ], + }); + + expect(openCode.calls.questionReply).toContainEqual( + expect.objectContaining({ + requestID: "question_comma", + answers: [["Red, bright", "Blue", "Green, dark"], ["Paris, France"]], + }), + ); + await parent.close(); + }); + test("emits the same child status when descriptor semantics change", async () => { const { parent, openCode } = await createParentSession("ses_parent_child_changed_status"); const events: AgentStreamEvent[] = []; diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index 2276e85786..6aa353989f 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -57,6 +57,7 @@ import { type ToolCallDetail, type ToolCallTimelineItem, } from "../agent-sdk-types.js"; +import { questionAnswerValues } from "../question-answers.js"; import { importSessionFromPersistence } from "../provider-session-import.js"; import { raceProviderRefreshAbort, @@ -4957,19 +4958,10 @@ class OpenCodeAgentSession implements AgentSession { directory, }); } else { - const answersRecord = readOpenCodeRecord(response.updatedInput?.answers); const questions = Array.isArray(pending.input?.questions) ? pending.input.questions : []; - const answers = questions.map((item) => { - const header = readNonEmptyString(readOpenCodeRecord(item)?.header); - const rawAnswer = header ? readNonEmptyString(answersRecord?.[header]) : null; - if (!rawAnswer) { - return []; - } - return rawAnswer - .split(",") - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0); - }); + const answers = questions.map((_, index) => + questionAnswerValues(response.questionAnswers?.[index]), + ); await this.client.question.reply({ requestID: requestId, diff --git a/packages/server/src/server/agent/providers/opencode/v2/permissions.test.ts b/packages/server/src/server/agent/providers/opencode/v2/permissions.test.ts index 451bdf81c6..510d6bc1c0 100644 --- a/packages/server/src/server/agent/providers/opencode/v2/permissions.test.ts +++ b/packages/server/src/server/agent/providers/opencode/v2/permissions.test.ts @@ -35,17 +35,23 @@ describe("OpenCode v2 questions", () => { }); const session = await client.createSession({ provider: "opencode", cwd: "/tmp/project" }); try { - expect(session.getPendingPermissions()).toHaveLength(1); + expect(session.getPendingPermissions()).toEqual([ + expect.objectContaining({ + input: { + questions: expect.arrayContaining([ + expect.objectContaining({ header: "features", multiSelect: true }), + ]), + }, + }), + ]); await session.respondToPermission("question", { behavior: "allow", - updatedInput: { - answers: { - color: "Blue", - features: ["Search"], - count: "3", - enabled: "false", - }, - }, + questionAnswers: [ + { selected: ["Blue"] }, + { selected: ["Search"], text: "Web, images" }, + { selected: [], text: "3" }, + { selected: [], text: "false" }, + ], }); expect(answers).toEqual([ { @@ -53,7 +59,7 @@ describe("OpenCode v2 questions", () => { formID: "question", answer: { color: "blue-id", - features: ["search-id"], + features: ["search-id", "Web, images"], count: 3, enabled: false, }, diff --git a/packages/server/src/server/agent/providers/opencode/v2/permissions.ts b/packages/server/src/server/agent/providers/opencode/v2/permissions.ts index 12965a6d7e..943469cc45 100644 --- a/packages/server/src/server/agent/providers/opencode/v2/permissions.ts +++ b/packages/server/src/server/agent/providers/opencode/v2/permissions.ts @@ -4,9 +4,11 @@ import type { V2Api } from "./api.js"; import type { AgentPermissionRequest, AgentPermissionResponse, + AgentQuestionAnswer, AgentSessionConfig, AgentStreamEvent, } from "../../../agent-sdk-types.js"; +import { questionAnswerValues } from "../../../question-answers.js"; export class SessionPermissions { private readonly pending = new Map(); @@ -39,16 +41,13 @@ export class SessionPermissions { if (response.behavior === "deny") await this.client.session.form.cancel({ sessionID: form.sessionID, formID: form.id }); else { - const raw = response.updatedInput?.answers; + const answers = response.questionAnswers; const answer: Record = {}; - if (!raw || typeof raw !== "object" || Array.isArray(raw)) - throw new Error("OpenCode question response requires answers"); - for (const field of form.fields) { - const value: unknown = - Reflect.get(raw, field.key) ?? Reflect.get(raw, field.title ?? field.key); - const normalized = formAnswer(field, value); + if (!answers) throw new Error("OpenCode question response requires answers"); + form.fields.forEach((field, index) => { + const normalized = formAnswer(field, answers[index]); if (normalized !== undefined) answer[field.key] = normalized; - } + }); await this.client.session.form.reply({ sessionID: form.sessionID, formID: form.id, @@ -103,7 +102,7 @@ export class SessionPermissions { header: field.key, question: field.title ?? field.key, options: "options" in field ? field.options : undefined, - multiple: field.type === "multiselect", + multiSelect: field.type === "multiselect", })), }, }; @@ -124,9 +123,13 @@ function permissionReply(response: AgentPermissionResponse): "reject" | "once" | return response.selectedActionId === "always" ? "always" : "once"; } -function formAnswer(field: FormInfo["fields"][number], value: unknown): FormValue | undefined { - if (value === undefined) return undefined; - if (field.type === "external") return undefined; +function formAnswer( + field: FormInfo["fields"][number], + answer: AgentQuestionAnswer | undefined, +): FormValue | undefined { + const values = questionAnswerValues(answer); + const [value] = values; + if (value === undefined || field.type === "external") return undefined; const labelValue = (label: string) => { const options = "options" in field ? field.options : undefined; return ( @@ -136,21 +139,15 @@ function formAnswer(field: FormInfo["fields"][number], value: unknown): FormValu ); }; if (field.type === "multiselect") { - if (Array.isArray(value) && value.every((item: unknown) => typeof item === "string")) - return value.map(labelValue); - } else if (field.type === "string" && typeof value === "string") { + return values.map(labelValue); + } else if (field.type === "string") { return labelValue(value); } else if (field.type === "boolean") { - if (typeof value === "boolean") return value; if (value === "true") return true; if (value === "false") return false; } else if (field.type === "number" || field.type === "integer") { - const numeric = typeof value === "string" && value.trim() ? Number(value) : value; - if ( - typeof numeric === "number" && - Number.isFinite(numeric) && - (field.type !== "integer" || Number.isInteger(numeric)) - ) + const numeric = value.trim() ? Number(value) : Number.NaN; + if (Number.isFinite(numeric) && (field.type !== "integer" || Number.isInteger(numeric))) return numeric; } throw new Error(`Invalid answer for OpenCode question ${field.key}`); diff --git a/packages/server/src/server/agent/providers/pi/extensions/rpiv-ask-user-question/index.test.ts b/packages/server/src/server/agent/providers/pi/extensions/rpiv-ask-user-question/index.test.ts index 3ec9256a29..141963a9d9 100644 --- a/packages/server/src/server/agent/providers/pi/extensions/rpiv-ask-user-question/index.test.ts +++ b/packages/server/src/server/agent/providers/pi/extensions/rpiv-ask-user-question/index.test.ts @@ -15,12 +15,18 @@ const [start, ...rest] = fixture.frames; const dialogs = rest.filter((frame: { type: string }) => frame.type === "extension_ui_request"); const end = rest.find((frame: { type: string }) => frame.type === "tool_execution_end"); -function startedHost() { +const answers = [ + { selected: ["Cookie"] }, + { selected: ["Red", "Blue"] }, + { selected: [], text: "Nebula" }, +]; + +function startedHost(args: unknown = start.args) { const host = createPiExtensionHost(); const permission = host.onToolStart({ callId: start.toolCallId, toolName: start.toolName, - args: start.args, + args, status: "running", result: null, }); @@ -47,12 +53,7 @@ test("real rpiv RPC dialogs use one form and preserve choices, multi-select, and }); expect(host.mapDialog(dialogs[0], "pi")).toEqual({ type: "deferred" }); expect( - host.respondToPermission(permission, { - behavior: "allow", - updatedInput: { - answers: { Snack: "Cookie", Colors: "Red, Blue", Project: "Nebula" }, - }, - }), + host.respondToPermission(permission, { behavior: "allow", questionAnswers: answers }), ).toEqual({ responses: [{ id: dialogs[0].id, response: { value: dialogs[0].options[1] } }] }); expect(host.mapDialog(dialogs[1], "pi")).toEqual({ type: "response", @@ -68,6 +69,21 @@ test("real rpiv RPC dialogs use one form and preserve choices, multi-select, and }); }); +test("multi-select labels containing a comma map back to their option numbers", () => { + const args = structuredClone(start.args); + args.questions[1].options[0].label = "Red, warm"; + const { host, permission } = startedHost(args); + host.respondToPermission(permission, { + behavior: "allow", + questionAnswers: [answers[0], { selected: ["Red, warm", "Blue"] }, answers[2]], + }); + host.mapDialog(dialogs[0], "pi"); + expect(host.mapDialog(dialogs[1], "pi")).toEqual({ + type: "response", + response: { value: "1,2" }, + }); +}); + test("cancellation declines each remaining real RPC dialog", () => { const { host, permission } = startedHost(); expect(host.respondToPermission(permission, { behavior: "deny" })).toEqual({ responses: [] }); @@ -153,12 +169,7 @@ test("the Pi agent releases a deferred captured dialog when the form is answered expect(permission.input?.questions).toHaveLength(3); runtime.emit(dialogs[0]); expect(runtime.extensionUiResponses).toEqual([]); - await session.respondToPermission(permission.id, { - behavior: "allow", - updatedInput: { - answers: { Snack: "Cookie", Colors: "Red, Blue", Project: "Nebula" }, - }, - }); + await session.respondToPermission(permission.id, { behavior: "allow", questionAnswers: answers }); for (const dialog of dialogs.slice(1)) runtime.emit(dialog); expect(runtime.extensionUiResponses).toEqual([ { id: dialogs[0].id, response: { value: dialogs[0].options[1] } }, diff --git a/packages/server/src/server/agent/providers/pi/extensions/rpiv-ask-user-question/index.ts b/packages/server/src/server/agent/providers/pi/extensions/rpiv-ask-user-question/index.ts index 07906cc955..a06003c166 100644 --- a/packages/server/src/server/agent/providers/pi/extensions/rpiv-ask-user-question/index.ts +++ b/packages/server/src/server/agent/providers/pi/extensions/rpiv-ask-user-question/index.ts @@ -1,5 +1,6 @@ import { z } from "zod"; -import type { AgentPermissionRequest } from "../../../../agent-sdk-types.js"; +import type { AgentPermissionRequest, AgentQuestionAnswer } from "../../../../agent-sdk-types.js"; +import { questionAnswerValues } from "../../../../question-answers.js"; import type { PiExtension, PiExtensionDialog, PiExtensionUiResponse } from "../contract.js"; const Params = z @@ -55,38 +56,34 @@ const Result = z.object({ type Question = z.infer["questions"][number]; const PERMISSION_PREFIX = "rpiv-question:"; -function answerFor(answers: Record, question: Question): string | null { - const answer = answers[question.header]; - return typeof answer === "string" ? answer : null; -} - function replyToDialog( dialog: PiExtensionDialog, question: Question, - answer: string, + answer: AgentQuestionAnswer, customInput: boolean, ): { response: PiExtensionUiResponse; customInput: boolean } { const options = z.array(z.string()).safeParse(dialog.options).data; - if (customInput) return { response: { value: answer }, customInput: false }; + const text = questionAnswerValues(answer).join(", "); + if (customInput) return { response: { value: text }, customInput: false }; if (question.multiSelect) { - const labels = answer.split(", "); - const indices = labels.map((label) => + const indices = answer.selected.map((label) => question.options.findIndex((option) => option.label === label), ); - if (indices.every((index) => index >= 0)) { + if (!answer.text && indices.every((index) => index >= 0)) { return { response: { value: indices.map((index) => index + 1).join(",") }, customInput: false, }; } - return { response: { value: answer }, customInput: false }; + return { response: { value: text }, customInput: false }; } - const index = question.options.findIndex((option) => option.label === answer); - if (index >= 0) { + const [label] = answer.selected; + const index = question.options.findIndex((option) => option.label === label); + if (!answer.text && index >= 0) { return { response: { value: - options?.[index] ?? `${index + 1}. ${answer} — ${question.options[index].description}`, + options?.[index] ?? `${index + 1}. ${label} — ${question.options[index].description}`, }, customInput: false, }; @@ -106,7 +103,7 @@ export const rpivAskUserQuestion: PiExtension = { let active: { callId: string; questions: Question[]; - answers: Record | null; + answers: AgentQuestionAnswer[] | null; cancelled: boolean; index: number; customInput: boolean; @@ -117,8 +114,10 @@ export const rpivAskUserQuestion: PiExtension = { if (!active || active.cancelled) return { cancelled: true }; const question = active.questions[active.index]; if (!question || !active.answers) return { cancelled: true }; - const answer = answerFor(active.answers, question); - if (answer === null) return { cancelled: true }; + const answer = active.answers[active.index]; + if (!answer || (answer.selected.length === 0 && answer.text === undefined)) { + return { cancelled: true }; + } const mapped = replyToDialog(dialog, question, answer, active.customInput); active.customInput = mapped.customInput; if (!active.customInput) active.index++; @@ -169,11 +168,7 @@ export const rpivAskUserQuestion: PiExtension = { }, respondToPermission(request, response) { if (!active || request.id !== `${PERMISSION_PREFIX}${active.callId}`) return undefined; - const answers = response.behavior === "allow" ? response.updatedInput?.answers : undefined; - active.answers = - answers && typeof answers === "object" && !Array.isArray(answers) - ? (answers as Record) - : null; + active.answers = (response.behavior === "allow" && response.questionAnswers) || null; active.cancelled = response.behavior === "deny" || !active.answers; const deferred = active.deferred; active.deferred = null; diff --git a/packages/server/src/server/agent/question-answers.test.ts b/packages/server/src/server/agent/question-answers.test.ts new file mode 100644 index 0000000000..c5879a5ab4 --- /dev/null +++ b/packages/server/src/server/agent/question-answers.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, test } from "vitest"; +import type { AgentPermissionRequest } from "./agent-sdk-types.js"; +import { resolveQuestionAnswers } from "./question-answers.js"; + +const request: AgentPermissionRequest = { + id: "question-1", + provider: "codex", + name: "request_user_input", + kind: "question", + input: { + questions: [ + { + question: "Which colors?", + header: "Colors", + options: [{ label: "Red, bright" }, { label: "Blue" }], + multiSelect: true, + allowOther: true, + }, + { + question: "Which snack?", + header: "Snack", + options: [{ label: "Cookie" }, { label: "Apple" }], + multiSelect: false, + allowOther: true, + }, + { question: "Anything else?", header: "Notes", options: [], allowEmpty: true }, + ], + }, +}; + +describe("resolveQuestionAnswers", () => { + test("keeps structured answers and derives the header-keyed text answers", () => { + const resolved = resolveQuestionAnswers(request, { + behavior: "allow", + questionAnswers: [ + { selected: ["Red, bright", "Blue"], text: "Green, dark" }, + { selected: ["Cookie"] }, + { selected: [], text: "" }, + ], + }); + + expect(resolved).toEqual({ + behavior: "allow", + questionAnswers: [ + { selected: ["Red, bright", "Blue"], text: "Green, dark" }, + { selected: ["Cookie"] }, + { selected: [], text: "" }, + ], + updatedInput: { + answers: { Colors: "Red, bright, Blue, Green, dark", Snack: "Cookie", Notes: "" }, + }, + }); + }); + + test("reads text answers from clients without structured answers by matching option labels", () => { + const resolved = resolveQuestionAnswers(request, { + behavior: "allow", + updatedInput: { + answers: { Colors: "Red, bright, Blue, Green", Snack: "Pretzel", Notes: "" }, + }, + }); + + expect(resolved).toMatchObject({ + questionAnswers: [ + { selected: ["Red, bright", "Blue"], text: "Green" }, + { selected: [], text: "Pretzel" }, + { selected: [], text: "" }, + ], + updatedInput: { + answers: { Colors: "Red, bright, Blue, Green", Snack: "Pretzel", Notes: "" }, + }, + }); + }); + + test("rejects structured answers that do not line up with the questions", () => { + expect(() => + resolveQuestionAnswers(request, { + behavior: "allow", + questionAnswers: [{ selected: ["Blue"] }], + }), + ).toThrow("3 questions"); + }); + + test("leaves denials and non-question requests unchanged", () => { + expect(resolveQuestionAnswers(request, { behavior: "deny" })).toEqual({ behavior: "deny" }); + const tool = { ...request, kind: "tool" as const }; + expect(resolveQuestionAnswers(tool, { behavior: "allow" })).toEqual({ behavior: "allow" }); + }); +}); diff --git a/packages/server/src/server/agent/question-answers.ts b/packages/server/src/server/agent/question-answers.ts new file mode 100644 index 0000000000..5d74a3743d --- /dev/null +++ b/packages/server/src/server/agent/question-answers.ts @@ -0,0 +1,114 @@ +import { z } from "zod"; +import type { + AgentPermissionRequest, + AgentPermissionResponse, + AgentQuestionAnswer, +} from "./agent-sdk-types.js"; + +// Every provider publishes `question` requests in the shape the app's question form reads. +const QuestionsSchema = z.array( + z + .object({ + header: z.string(), + options: z.array(z.object({ label: z.string() }).passthrough()).optional(), + multiSelect: z.boolean().optional(), + }) + .passthrough(), +); +type Question = z.infer[number]; + +const TextAnswersSchema = z.record(z.string(), z.unknown()); + +// Text answers join a multi-select's labels and typed answer with this separator. +const TEXT_SEPARATOR = ", "; + +/** + * Gives an answered `question` request both answer forms before it reaches the provider: + * `questionAnswers`, one structured answer per question, and `updatedInput.answers`, the + * header-keyed text that Claude's AskUserQuestion and plugin providers read. Clients that + * send only text (the CLI, MCP, and apps before structured answers) are parsed by option label. + */ +export function resolveQuestionAnswers( + request: AgentPermissionRequest, + response: AgentPermissionResponse, +): AgentPermissionResponse { + if (request.kind !== "question" || response.behavior !== "allow") return response; + const parsed = QuestionsSchema.safeParse(request.input?.questions); + if (!parsed.success) return response; + const questions = parsed.data; + + if (response.questionAnswers && response.questionAnswers.length !== questions.length) { + throw new Error( + `Question answers must answer all ${questions.length} questions; received ${response.questionAnswers.length}`, + ); + } + const textAnswers = TextAnswersSchema.safeParse(response.updatedInput?.answers).data; + const questionAnswers = + response.questionAnswers ?? + (textAnswers ? questions.map((question) => parseTextAnswer(question, textAnswers)) : null); + if (!questionAnswers) return response; + + return { + ...response, + questionAnswers, + updatedInput: { + ...response.updatedInput, + answers: textAnswers ?? formatTextAnswers(questions, questionAnswers), + }, + }; +} + +/** The chosen option labels followed by the typed answer, if there is one. */ +export function questionAnswerValues(answer: AgentQuestionAnswer | undefined): string[] { + if (!answer) return []; + const text = answer.text?.trim(); + return text ? [...answer.selected, text] : answer.selected; +} + +function parseTextAnswer( + question: Question, + textAnswers: Record, +): AgentQuestionAnswer { + const raw = textAnswers[question.header]; + if (Array.isArray(raw)) return { selected: raw.filter((item) => typeof item === "string") }; + if (typeof raw === "number" || typeof raw === "boolean") { + return { selected: [], text: String(raw) }; + } + if (typeof raw !== "string") return { selected: [] }; + const text = raw.trim(); + const labels = new Set(question.options?.map((option) => option.label)); + if (!question.multiSelect) { + return labels.has(text) ? { selected: [text] } : { selected: [], text }; + } + + // Labels can contain the separator, so match the longest run of segments that is a label. + const segments = text.split(TEXT_SEPARATOR); + const selected: string[] = []; + const typed: string[] = []; + let start = 0; + while (start < segments.length) { + let end = segments.length; + while (end > start && !labels.has(segments.slice(start, end).join(TEXT_SEPARATOR))) end--; + if (end > start) { + selected.push(segments.slice(start, end).join(TEXT_SEPARATOR)); + start = end; + } else { + typed.push(segments[start]); + start++; + } + } + return typed.length > 0 ? { selected, text: typed.join(TEXT_SEPARATOR) } : { selected }; +} + +function formatTextAnswers( + questions: Question[], + questionAnswers: AgentQuestionAnswer[], +): Record { + const answers: Record = {}; + questions.forEach((question, index) => { + const answer = questionAnswers[index]; + if (answer.selected.length === 0 && answer.text === undefined) return; + answers[question.header] = questionAnswerValues(answer).join(TEXT_SEPARATOR); + }); + return answers; +}