From e01f7e4688584aa305b8c6b38c814a95a0c46d86 Mon Sep 17 00:00:00 2001 From: William Wang Date: Mon, 27 Jul 2026 20:19:09 +0800 Subject: [PATCH] fix: route ExitPlanMode through request_permission instead of elicitation form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExitPlanMode previously went through elicitation/create when the client supported it, which surfaced a generic "input request / waiting for input" shell that reads wrong for a plan-approval flow. Plan approval is a permission decision, not structured input — route it through session/request_permission like claude-agent-acp does, so it renders as the same Approve/Reject dialog as tool permission. AskUserQuestion still uses elicitation/create (it is the actual input case). Trade-off: the user can no longer attach free-text feedback when rejecting. They can give reasons in the next normal message instead. --- src/handlers/server-requests.ts | 32 +++-------- src/interaction/adapter.ts | 99 ++------------------------------- tests/interaction.test.ts | 69 ++--------------------- 3 files changed, 17 insertions(+), 183 deletions(-) diff --git a/src/handlers/server-requests.ts b/src/handlers/server-requests.ts index a22c59e..2dc9733 100644 --- a/src/handlers/server-requests.ts +++ b/src/handlers/server-requests.ts @@ -28,14 +28,12 @@ import { acpPermissionResponseToZcode, buildAskUserAcpParams, buildAskUserElicitationForm, - buildExitPlanModeElicitationForm, exitPlanModeToAcpPermission, isAskUserQuestion, isExitPlanMode, isPermissionRequest, parseAskUserElicitationResponse, parseAskUserResponse, - parseExitPlanModeElicitationResponse, splitAskUserQuestions, zcodePermissionToAcp, } from "../interaction/adapter.js"; @@ -218,29 +216,13 @@ async function handleSinglePermission( } await sendSessionUpdate(cx, acpSid, tcUpdate); - // ExitPlanMode: prefer elicitation form when the client supports it. - if (epm && server.supportsElicitationForm()) { - const formParams = buildExitPlanModeElicitationForm( - p as ZcodeInteractionUserInputParams, - acpSid, - toolCallId || undefined, - ); - log(` ⟳ ExitPlanMode forwarding elicitation/create (form)`); - const elicResp = await requestWithTimeout( - cx, - "elicitation/create", - formParams, - "elicitation/create", - undefined, - turn, - ); - if (elicResp === null) { - return { action: "decline", reason: "elicitation failed" }; - } - return parseExitPlanModeElicitationResponse(elicResp) as ZcodeInteractionResponse; - } - - // Tool auth, or ExitPlanMode without elicitation: use request_permission. + // ExitPlanMode and tool auth both go through session/request_permission. + // ExitPlanMode intentionally does NOT use elicitation/create even when the + // client supports forms — matching claude-agent-acp's reference behaviour: + // plan approval is a permission decision (approve/reject), not a structured + // input, and rendering it through the elicitation channel surfaces a generic + // "input request" shell that reads wrong for this flow. AskUserQuestion is + // the right place for elicitation; plan approval is not. const acpParams = perm ? zcodePermissionToAcp(p as ZcodeInteractionPermissionParams, acpSid)! : exitPlanModeToAcpPermission(p as ZcodeInteractionUserInputParams, acpSid); diff --git a/src/interaction/adapter.ts b/src/interaction/adapter.ts index f61e042..63a0d26 100644 --- a/src/interaction/adapter.ts +++ b/src/interaction/adapter.ts @@ -416,97 +416,8 @@ export function parseAskUserElicitationResponse( return answers; } -/** - * Build an elicitation form for ExitPlanMode. - * - * Single `feedback` text field — no approve/reject dropdown. The ACP client - * always renders its own submit/cancel controls, so: - * - submit with feedback empty → approve (proceed with the plan) - * - submit with feedback filled → reject, and the text becomes the decline - * `reason` the agent sees when re-planning - * - cancel/decline button → plain reject (no reason) - * - * Typing into the field IS the reject signal — a separate dropdown would be - * redundant (its value is forced by whether feedback is present). - */ -export function buildExitPlanModeElicitationForm( - params: ZcodeInteractionUserInputParams, - acpSid: string, - toolCallId?: string, -): { - mode: "form"; - sessionId: string; - toolCallId?: string; - message: string; - requestedSchema: { - type: "object"; - properties: Record; - required: string[]; - }; -} { - const planText = - params.input && typeof params.input === "object" - ? ((params.input as { plan?: string }).plan ?? "") - : ""; - const suffix = "Leave the box empty and submit to approve; type feedback to reject and redirect."; - const message = planText - ? `Ready to code?\n\n${planText}\n\n${suffix}` - : `Ready to code?\n\n${suffix}`; - const form: { - mode: "form"; - sessionId: string; - toolCallId?: string; - message: string; - requestedSchema: { - type: "object"; - properties: Record; - required: string[]; - }; - } = { - mode: "form", - sessionId: acpSid, - message, - requestedSchema: { - type: "object", - properties: { - feedback: { - type: "string", - title: "Feedback", - description: - "Empty = approve the plan. Anything typed = reject and use this text as the redirection.", - }, - }, - // Not required: an empty submit is a valid "approve". - required: [], - }, - }; - if (toolCallId) form.toolCallId = toolCallId; - return form; -} - -/** - * Parse an ExitPlanMode elicitation response → zcode accept/decline. - * - * Accept with non-empty feedback → decline carrying the feedback as `reason` - * (the user typed a redirection). Accept with empty feedback → approve. - * Decline/cancel from the client → plain decline. - */ -export function parseExitPlanModeElicitationResponse(acpResp: unknown): { - action: "accept" | "decline"; - reason?: string; - content?: unknown; -} { - if (!acpResp || typeof acpResp !== "object") { - return { action: "decline", reason: "invalid client response" }; - } - const resp = acpResp as { action?: string; content?: { feedback?: string } | null }; - if (resp.action !== "accept") { - return { action: "decline", reason: "cancelled" }; - } - const feedback = typeof resp.content?.feedback === "string" ? resp.content.feedback.trim() : ""; - if (feedback) { - return { action: "decline", reason: feedback }; - } - // content must be an object with answer_0 (zcode reads content.answer_0). - return { action: "accept", content: { answer_0: "approve" } }; -} +// ExitPlanMode is handled via session/request_permission (see +// `exitPlanModeToAcpPermission` / `acpPermissionResponseToExitPlanMode` above) — +// not via elicitation/create. Plan approval is a permission decision, and +// routing it through elicitation surfaces a generic "input request" shell that +// reads wrong for this flow. AskUserQuestion is the elicitation use case. diff --git a/tests/interaction.test.ts b/tests/interaction.test.ts index c9b572b..90776de 100644 --- a/tests/interaction.test.ts +++ b/tests/interaction.test.ts @@ -11,7 +11,6 @@ import { acpPermissionResponseToZcode, buildAskUserAcpParams, buildAskUserElicitationForm, - buildExitPlanModeElicitationForm, exitPlanModeToAcpPermission, isAskUserQuestion, isExitPlanMode, @@ -19,7 +18,6 @@ import { isUserInputRequest, parseAskUserElicitationResponse, parseAskUserResponse, - parseExitPlanModeElicitationResponse, splitAskUserQuestions, zcodePermissionToAcp, } from "../src/interaction/adapter.js"; @@ -465,65 +463,8 @@ describe("elicitation form: AskUserQuestion", () => { }); }); -describe("elicitation form: ExitPlanMode", () => { - const epmParams = { - requestId: "req_2", - sessionId: "sess_1", - toolCallId: "tc_2", - schema: { interaction: "plan_approval" }, - input: { plan: "Step 1\nStep 2" }, - }; - - it("embeds the plan text + approve/reject hint in the message", () => { - const form = buildExitPlanModeElicitationForm(epmParams, "acp_1"); - expect(form.message).toContain("Step 1"); - expect(form.message).toContain("Ready to code?"); - expect(form.message).toContain("Leave the box empty and submit to approve"); - }); - - it("attaches toolCallId scope when provided", () => { - const form = buildExitPlanModeElicitationForm(epmParams, "acp_1", "tc_2"); - expect(form.toolCallId).toBe("tc_2"); - }); - - it("has only a feedback field, not required (empty submit = approve)", () => { - const form = buildExitPlanModeElicitationForm(epmParams, "acp_1"); - const keys = Object.keys(form.requestedSchema.properties); - expect(keys).toEqual(["feedback"]); - expect(form.requestedSchema.required).toEqual([]); - }); - - it("accept with empty feedback → approve", () => { - const resp = parseExitPlanModeElicitationResponse({ - action: "accept", - content: { feedback: "" }, - }); - expect(resp).toEqual({ action: "accept", content: { answer_0: "approve" } }); - }); - - it("accept with whitespace-only feedback → approve (trimmed to empty)", () => { - const resp = parseExitPlanModeElicitationResponse({ - action: "accept", - content: { feedback: " " }, - }); - expect(resp).toEqual({ action: "accept", content: { answer_0: "approve" } }); - }); - - it("accept with feedback → decline carrying the feedback as reason", () => { - const resp = parseExitPlanModeElicitationResponse({ - action: "accept", - content: { feedback: " do login first " }, - }); - expect(resp).toEqual({ action: "decline", reason: "do login first" }); - }); - - it("accept with missing content → approve (treated as empty)", () => { - const resp = parseExitPlanModeElicitationResponse({ action: "accept", content: null }); - expect(resp).toEqual({ action: "accept", content: { answer_0: "approve" } }); - }); - - it("cancel → decline", () => { - const resp = parseExitPlanModeElicitationResponse({ action: "cancel" }); - expect(resp.action).toBe("decline"); - }); -}); +// ExitPlanMode is exercised via session/request_permission (see the +// "ExitPlanMode" describe block above): `exitPlanModeToAcpPermission` + +// `acpPermissionResponseToExitPlanMode`. There is no elicitation-form path for +// it — routing plan approval through elicitation surfaces a generic "input +// request" shell that reads wrong; AskUserQuestion is the elicitation use case.