fix(acp): let the model continue when a question can't be shown (#52740)

packages/cli/src/acp/elicitation.ts

@@ -16,8 +16,9 @@ type InputField = Exclude<Form.Field, Form.ExternalField>
 type SelectField = Form.StringField | Form.MultiselectField
 
 // Form mode must not collect secrets, so only forms from flows known not to ask for credentials are elicited.
-const ElicitedKind = Schema.Struct({ kind: Schema.Literals(["question", "websearch.provider"]) })
-const Credential = /password|passphrase|secret|token|api[_-]?key|credential|private[_-]?key/i
+const QuestionKind = "question"
+const ElicitedKind = Schema.Struct({ kind: Schema.Literals([QuestionKind, "websearch.provider"]) })
+const Credential = /password|passphrase|secret|token|api[_ -]?key|credential|private[_ -]?key/i
 const ToolSource = Schema.Struct({ tool: Schema.Struct({ id: Schema.String }) })
 
 type Input = {
@@ -57,9 +58,12 @@ export const reply = Effect.fn("cli.acp.elicitation.reply")(function* (input: In
   )
 })
 
+export const UnshownQuestionMessage =
+  "The question couldn't be shown to the user in this client. Continue without an answer: make reasonable assumptions and state them, or ask the user in your reply if you can't proceed."
+
 /** Cancels a form, interrupting its session when the server can't cancel it. */
-export function cancel(client: OpenCodeClient, form: AskedForm) {
-  return settle(() => client.session.form.cancel({ sessionID: form.sessionID, formID: form.id })).pipe(
+function cancel(client: OpenCodeClient, form: AskedForm, message?: string) {
+  return settle(() => client.session.form.cancel({ sessionID: form.sessionID, formID: form.id, message })).pipe(
     Effect.catch(() =>
       Effect.tryPromise(() => client.session.interrupt({ sessionID: form.sessionID })).pipe(Effect.ignore),
     ),
@@ -68,15 +72,16 @@ export function cancel(client: OpenCodeClient, form: AskedForm) {
 
 /**
  * The form-mode schema for a form, or undefined when the form is cancelled instead: the client lacks form
- * elicitation, the form is not from an allowed flow or has a field that looks like a credential, or ACP can't
- * represent it faithfully. Unrepresentable forms have `external` fields, `when` conditions, a hidden required field
- * without a default, a default outside a field's options, or a free-text answer alongside options that must also
- * satisfy `required` or item bounds across both inputs. Hidden fields are not asked and answer with their default.
+ * elicitation, the form is not from an allowed flow or has a field whose key, title, description, or option label
+ * looks like a credential, or ACP can't represent it faithfully. Unrepresentable forms have `external` fields, `when`
+ * conditions, a hidden required field without a default, a default outside a field's options, or a free-text answer
+ * alongside options that must also satisfy `required` or item bounds across both inputs. Hidden fields are not asked
+ * and answer with their default.
  */
 export function requestedSchema(form: AskedForm, capabilities: ACPService.Capabilities): ElicitationSchema | undefined {
   if (!capabilities.formElicitation) return undefined
   if (Option.isNone(Schema.decodeUnknownOption(ElicitedKind)(form.metadata))) return undefined
-  if (form.fields.some((field) => Credential.test(field.key) || Credential.test(field.title ?? ""))) return undefined
+  if (form.fields.some(credentialLike)) return undefined
   const fields = form.fields.filter((field): field is InputField => field.type !== "external")
   if (fields.length !== form.fields.length || fields.some((field) => field.when?.length)) return undefined
   if (fields.some((field) => field.hidden && field.required && field.default === undefined)) return undefined
@@ -90,6 +95,14 @@ export function requestedSchema(form: AskedForm, capabilities: ACPService.Capabi
   }
 }
 
+/**
+ * Cancels a form that `requestedSchema` won't show. A question is cancelled with a message the question tool returns
+ * to the model, so the turn continues instead of ending as interrupted; other forms are cancelled silently.
+ */
+export function cancelUnshown(client: OpenCodeClient, form: AskedForm) {
+  return cancel(client, form, form.metadata?.kind === QuestionKind ? UnshownQuestionMessage : undefined)
+}
+
 /** The answer for an accepted response, or undefined when the user declined, cancelled, or sent invalid content. */
 export function answer(form: AskedForm, response: CreateElicitationResponse): Form.Answer | undefined {
   if (response.action !== "accept") return undefined
@@ -137,6 +150,14 @@ function settle(evaluate: () => Promise<void>) {
   )
 }
 
+function credentialLike(field: Form.Field) {
+  const labels =
+    field.type === "string" || field.type === "multiselect" ? (field.options ?? []).map((option) => option.label) : []
+  return [field.key, field.title, field.description, ...labels].some(
+    (text) => text !== undefined && Credential.test(text),
+  )
+}
+
 function representable(field: InputField, keys: ReadonlySet<string>) {
   if (field.type !== "string" && field.type !== "multiselect") return true
   if (!hasOptions(field)) return true

packages/cli/src/acp/turn.ts

@@ -161,7 +161,7 @@ export const make = Effect.fnUntraced(function* (input: {
         return Effect.gen(function* () {
           const capabilities = yield* Ref.get(input.capabilities)
           const requestedSchema = ACPElicitation.requestedSchema(output.form, capabilities)
-          if (!requestedSchema) return yield* ACPElicitation.cancel(input.client, output.form)
+          if (!requestedSchema) return yield* ACPElicitation.cancelUnshown(input.client, output.form)
           const settled = yield* Deferred.make<void>()
           subscription.forms.set(output.form.id, settled)
           yield* Queue.offer(

packages/cli/test/acp/additional-directories.subprocess.test.ts

@@ -2,30 +2,16 @@ import type { NewSessionResponse, PromptResponse } from "@agentclientprotocol/sd
 import { describe, expect, test } from "bun:test"
 import fs from "node:fs/promises"
 import path from "node:path"
-import { createAcpFixture, expectOk, initialize } from "./subprocess"
+import { createAcpFixture, expectOk, initialize, toolCallStream } from "./subprocess"
 
 // The first completion reads the file outside cwd; the follow-up completion ends the turn.
 function readingModel(file: () => string) {
   return (request: unknown) => {
     if (JSON.stringify(request).includes('"role":"tool"')) return "done"
-    return new Response(toolCall(file()), { headers: { "content-type": "text/event-stream" } })
+    return toolCallStream("call_read", "read", { path: file() })
   }
 }
 
-function toolCall(file: string) {
-  const call = { index: 0, id: "call_read", type: "function", function: { name: "read", arguments: "" } }
-  const chunks = [
-    { choices: [{ delta: { role: "assistant", tool_calls: [call] }, finish_reason: null }], usage: null },
-    {
-      choices: [{ delta: { tool_calls: [{ index: 0, function: { arguments: JSON.stringify({ path: file }) } }] } }],
-      usage: null,
-    },
-    { choices: [{ delta: {}, finish_reason: "tool_calls" }], usage: null },
-    { choices: [], usage: { prompt_tokens: 10, completion_tokens: 1, total_tokens: 11 } },
-  ]
-  return `${chunks.map((chunk) => `data: ${JSON.stringify(chunk)}\n\n`).join("")}data: [DONE]\n\n`
-}
-
 describe("acp additional directories subprocess", () => {
   test("tools read files in an additional directory without an external directory ask", async () => {
     const target = { file: "" }

packages/cli/test/acp/elicitation.test.ts

@@ -191,6 +191,8 @@ describe("acp elicitation mapping", () => {
       [{ key: "api_key", type: "string" }],
       [{ key: "q0", title: "GitHub token", type: "string" }],
       [{ key: "q0", title: "Password", type: "string", hidden: true, default: "" }],
+      [{ key: "q0", title: "Setup", description: "Paste your API key", type: "string" }],
+      [{ key: "q0", title: "Setup", type: "string", options: [{ value: "a", label: "Use my access token" }] }],
     ]
     expect(credentials.map((fields) => ACPElicitation.requestedSchema(form(fields), capable))).toEqual(
       credentials.map(() => undefined),
@@ -332,6 +334,7 @@ describe("acp elicitation over the wire", () => {
     expect((await acp.prompt(acp.sessionId, "hello")).stopReason).toBe("end_turn")
     expect(acp.elicitations).toHaveLength(ids.length)
     expect(acp.server.cancelledForms.map((item) => item.formID)).toEqual(ids)
+    expect(acp.server.cancelledForms.map((item) => item.message)).toEqual(ids.map(() => undefined))
     expect(acp.server.repliedForms).toEqual([])
     expect(acp.server.interrupts).toEqual([])
   })
@@ -359,6 +362,62 @@ describe("acp elicitation over the wire", () => {
     expect(acp.server.cancelledForms).toEqual([{ sessionID: acp.sessionId, formID: "frm_plugin" }])
   })
 
+  test("cancels a question that looks like it asks for a credential with a message for the model", async () => {
+    await using acp = await startSession({
+      capabilities: { elicitation: true },
+      onPrompt: ({ sessionID, id }) => [
+        delivered(sessionID, id),
+        ephemeralEvent("form.created", {
+          form: {
+            id: "frm_token",
+            sessionID,
+            title: "Questions",
+            metadata: { kind: "question", tool: { messageID: "msg_tools", id: "call_question" } },
+            fields: [
+              {
+                key: "q0",
+                title: "Setup",
+                description: "Paste your API key",
+                type: "string",
+                options: [{ value: "Later", label: "Later" }],
+                custom: true,
+              },
+            ],
+          },
+        }),
+      ],
+      onFormCancel: ({ sessionID }) => [succeeded(sessionID)],
+    })
+
+    expect((await acp.prompt(acp.sessionId, "hello")).stopReason).toBe("end_turn")
+    expect(acp.elicitations).toEqual([])
+    expect(acp.server.cancelledForms).toEqual([
+      { sessionID: acp.sessionId, formID: "frm_token", message: ACPElicitation.UnshownQuestionMessage },
+    ])
+  })
+
+  test("cancels other forms it cannot show without a message", async () => {
+    await using acp = await startSession({
+      onPrompt: ({ sessionID, id }) => [
+        delivered(sessionID, id),
+        ephemeralEvent("form.created", {
+          form: {
+            id: "frm_websearch",
+            sessionID,
+            title: "Web search provider",
+            metadata: { kind: "websearch.provider" },
+            fields: [{ key: "provider", type: "string", options: [{ value: "exa", label: "Exa" }] }],
+          },
+        }),
+      ],
+      onFormCancel: ({ sessionID }) => [succeeded(sessionID)],
+    })
+
+    expect((await acp.prompt(acp.sessionId, "hello")).stopReason).toBe("end_turn")
+    expect(acp.elicitations).toEqual([])
+    expect(acp.server.cancelledForms).toEqual([{ sessionID: acp.sessionId, formID: "frm_websearch" }])
+  })
+
   test("cancelling the turn cancels its pending elicitation and the form, and never sends queued ones", async () => {
     await using acp = await startSession({
       capabilities: { elicitation: true },

packages/cli/test/acp/question.subprocess.test.ts

@@ -0,0 +1,52 @@
+import type { PromptResponse } from "@agentclientprotocol/sdk"
+import { describe, expect, test } from "bun:test"
+import { Option, Schema } from "effect"
+import { ACPElicitation } from "../../src/acp/elicitation"
+import { createAcpFixture, expectOk, initialize, newSession, toolCallStream } from "./subprocess"
+
+const ChatRequest = Schema.Struct({
+  messages: Schema.Array(
+    Schema.Struct({ role: Schema.String, tool_call_id: Schema.optional(Schema.String), content: Schema.Unknown }),
+  ),
+})
+
+// The first completion asks a question; the follow-up completion ends the turn.
+function askingModel(request: unknown) {
+  if (JSON.stringify(request).includes('"role":"tool"')) return "done"
+  return toolCallStream("call_question", "question", {
+    questions: [
+      {
+        header: "Runtime",
+        question: "Which runtime?",
+        options: [
+          { label: "Bun", description: "Fast" },
+          { label: "Node", description: "Stable" },
+        ],
+      },
+    ],
+  })
+}
+
+describe("acp question subprocess", () => {
+  test("a question the client cannot show returns to the model and the turn continues", async () => {
+    await using fixture = await createAcpFixture({ respond: askingModel })
+    const acp = fixture.spawn()
+    await initialize(acp)
+    const session = await newSession(acp, fixture.home)
+
+    const result = expectOk(
+      await acp.request<PromptResponse>("session/prompt", {
+        sessionId: session.sessionId,
+        prompt: [{ type: "text", text: "ask me something" }],
+      }),
+    )
+
+    expect(result.stopReason).toBe("end_turn")
+    const results = fixture.llm.requests
+      .flatMap((request) => Option.toArray(Schema.decodeUnknownOption(ChatRequest)(request)))
+      .flatMap((request) => request.messages)
+      .filter((message) => message.role === "tool" && message.tool_call_id === "call_question")
+    expect(results).toHaveLength(1)
+    expect(String(results[0]?.content)).toContain(ACPElicitation.UnshownQuestionMessage)
+  }, 60_000)
+})

packages/cli/test/acp/subprocess.ts

@@ -161,6 +161,30 @@ export function expectOk<T>(response: JsonRpcResponse<T>) {
   return response.result
 }
 
+/** A streamed completion that calls one tool. */
+export function toolCallStream(id: string, name: string, input: unknown) {
+  const chunks = [
+    {
+      choices: [
+        {
+          delta: { role: "assistant", tool_calls: [{ index: 0, id, type: "function", function: { name, arguments: "" } }] },
+          finish_reason: null,
+        },
+      ],
+      usage: null,
+    },
+    {
+      choices: [{ delta: { tool_calls: [{ index: 0, function: { arguments: JSON.stringify(input) } }] } }],
+      usage: null,
+    },
+    { choices: [{ delta: {}, finish_reason: "tool_calls" }], usage: null },
+    { choices: [], usage: { prompt_tokens: 10, completion_tokens: 1, total_tokens: 11 } },
+  ]
+  return new Response(`${chunks.map((chunk) => `data: ${JSON.stringify(chunk)}\n\n`).join("")}data: [DONE]\n\n`, {
+    headers: { "content-type": "text/event-stream" },
+  })
+}
+
 function verifierConfig(llmUrl: string, skills?: string) {
   const model = {
     capabilities: { tools: true, input: ["text", "image"], output: ["text"] },

packages/cli/test/acp/turn.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, test } from "bun:test"
 import type { SessionNotification } from "@agentclientprotocol/sdk"
 import { resolve } from "node:path"
+import { ACPElicitation } from "../../src/acp/elicitation"
 import {
   assistantMessage,
   childCreated,
@@ -363,7 +364,9 @@ describe("acp turn events over the wire", () => {
     })
 
     expect((await acp.prompt(acp.sessionId, "hello")).stopReason).toBe("end_turn")
-    expect(acp.server.cancelledForms).toEqual([{ sessionID: acp.sessionId, formID: "frm_question" }])
+    expect(acp.server.cancelledForms).toEqual([
+      { sessionID: acp.sessionId, formID: "frm_question", message: ACPElicitation.UnshownQuestionMessage },
+    ])
     expect(acp.elicitations).toEqual([])
   })
 

packages/cli/test/acp/wire-fixture.ts

@@ -569,7 +569,7 @@ function startServer(options: WireOptions, changed: () => void) {
   const selections: Selection[] = []
   const interrupts: string[] = []
   const replies: Array<{ readonly sessionID: string; readonly requestID: string; readonly decision: string }> = []
-  const cancelledForms: Array<{ readonly sessionID: string; readonly formID: string }> = []
+  const cancelledForms: Array<{ readonly sessionID: string; readonly formID: string; readonly message?: string }> = []
   const repliedForms: FormReply[] = []
   const mcp: Array<{ readonly name: string; readonly directory?: string; readonly config: unknown }> = []
   const fake = {
@@ -831,8 +831,8 @@ function startServer(options: WireOptions, changed: () => void) {
         }),
       },
       "/api/session/:sessionID/form/:formID": {
-        DELETE: route(async (req) => {
-          const form = { sessionID: req.params.sessionID, formID: req.params.formID }
+        DELETE: route(async (req, query) => {
+          const form = { sessionID: req.params.sessionID, formID: req.params.formID, message: query.message }
           fake.cancelledForms.push(form)
           await emit(options.onFormCancel?.(form))
           return noContent()