Skip to content

fix: route ExitPlanMode through request_permission instead of #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
william0wang merged 1 commit into from
Jul 27, 2026
Merged

fix: route ExitPlanMode through request_permission instead of #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 7 additions & 25 deletions src/handlers/server-requests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,12 @@ import {
acpPermissionResponseToZcode,
buildAskUserAcpParams,
buildAskUserElicitationForm,
buildExitPlanModeElicitationForm,
exitPlanModeToAcpPermission,
isAskUserQuestion,
isExitPlanMode,
isPermissionRequest,
parseAskUserElicitationResponse,
parseAskUserResponse,
parseExitPlanModeElicitationResponse,
splitAskUserQuestions,
zcodePermissionToAcp,
} from "../interaction/adapter.js";
Expand Down Expand Up @@ -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);
Expand Down
99 changes: 5 additions & 94 deletions src/interaction/adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
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<string, unknown>;
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.
69 changes: 5 additions & 64 deletions tests/interaction.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,13 @@ import {
acpPermissionResponseToZcode,
buildAskUserAcpParams,
buildAskUserElicitationForm,
buildExitPlanModeElicitationForm,
exitPlanModeToAcpPermission,
isAskUserQuestion,
isExitPlanMode,
isPermissionRequest,
isUserInputRequest,
parseAskUserElicitationResponse,
parseAskUserResponse,
parseExitPlanModeElicitationResponse,
splitAskUserQuestions,
zcodePermissionToAcp,
} from "../src/interaction/adapter.js";
Expand Down Expand Up @@ -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.
Loading