diff --git a/src/interaction/adapter.ts b/src/interaction/adapter.ts index a370be3..d78068f 100644 --- a/src/interaction/adapter.ts +++ b/src/interaction/adapter.ts @@ -48,6 +48,36 @@ export function isAskUserQuestion(method: string, params: unknown): boolean { // ---------- zcode permission → ACP requestPermission ---------- +/** + * Normalize a zcode permission-option `kind` to one of the 4 ACP-legal values + * (`allow_once` | `allow_always` | `reject_once` | `reject_always`). + * + * zcode's backend emits a wider/legacy enum that includes `deny`, `allow`, + * `reject`, etc. The ACP schema (and Zed's deserializer) only accepts the four + * canonical values — anything else makes the *whole* `requestPermission` fail + * at the client with "unknown variant", so the popup never renders and the + * request defaults to decline. `optionId` is free text and stays untouched. + * + * Fail-safe: any unrecognized kind maps to `reject_once` (never auto-allow). + */ +function normalizeKind(kind: string | undefined): PermissionOption["kind"] { + switch (kind) { + case "allow_once": + case "allow_always": + case "reject_once": + case "reject_always": + return kind; + case "allow": + case "allow_project": + return "allow_always"; + case "reject": + case "deny": + return "reject_once"; + default: + return "reject_once"; + } +} + /** Convert a zcode tool-permission request into ACP requestPermission params. */ export function zcodePermissionToAcp( params: ZcodeInteractionPermissionParams, @@ -61,7 +91,7 @@ export function zcodePermissionToAcp( for (const opt of params.options ?? []) { options.push({ optionId: opt.optionId ?? "", - kind: (opt.kind as PermissionOption["kind"]) ?? "allow_once", + kind: normalizeKind(opt.kind), name: opt.name ?? opt.optionId ?? "", }); } @@ -73,6 +103,14 @@ export function zcodePermissionToAcp( }; } +/** + * Allow-class optionIds that zcode's backend emits. The response mapper must + * recognize ALL of them — selecting "Allow once" (optionId "allow_once") in + * the popup otherwise gets reported back as a rejection. optionId is free + * text defined by the backend, so this set mirrors the backend's naming. + * Any optionId not in this set is treated as deny (fail-safe). */ +const ALLOW_OPTION_IDS = new Set(["allow", "allow_once", "allow_always", "allow_project"]); + /** Convert an ACP requestPermission response → zcode {decision, reason?}. */ export function acpPermissionResponseToZcode( acpResp: unknown, @@ -83,7 +121,7 @@ export function acpPermissionResponseToZcode( const outcome = (acpResp as { outcome?: { outcome?: string; optionId?: string } }).outcome ?? {}; if (outcome.outcome === "cancelled") return { decision: "deny", reason: "cancelled by user" }; const optionId = outcome.optionId ?? ""; - if (optionId === "allow" || optionId === "allow_always") return { decision: "allow" }; + if (ALLOW_OPTION_IDS.has(optionId)) return { decision: "allow" }; return { decision: "deny", reason: `rejected (${optionId})` }; } @@ -246,7 +284,7 @@ export function buildAskUserElicitationForm( required: string[]; }; } { - const questions = params.questions?.length ? params.questions : params.input?.questions ?? []; + const questions = params.questions?.length ? params.questions : (params.input?.questions ?? []); const properties: Record = {}; const required: string[] = []; for (let i = 0; i < questions.length; i++) { @@ -317,7 +355,7 @@ export function parseAskUserElicitationResponse( if (!acpResp || typeof acpResp !== "object") return null; const resp = acpResp as { action?: string; content?: Record | null }; if (resp.action !== "accept" || !resp.content) return null; - const questions = params.questions?.length ? params.questions : params.input?.questions ?? []; + const questions = params.questions?.length ? params.questions : (params.input?.questions ?? []); const answers: Record = {}; for (let i = 0; i < questions.length; i++) { const q = questions[i]; @@ -362,9 +400,7 @@ export function buildExitPlanModeElicitationForm( params.input && typeof params.input === "object" ? ((params.input as { plan?: string }).plan ?? "") : ""; - const message = planText - ? `Ready to code?\n\n${planText}` - : "Ready to code?"; + const message = planText ? `Ready to code?\n\n${planText}` : "Ready to code?"; const form: { mode: "form"; sessionId: string; @@ -397,9 +433,10 @@ export function buildExitPlanModeElicitationForm( } /** Parse an ExitPlanMode elicitation response → zcode accept/decline. */ -export function parseExitPlanModeElicitationResponse( - acpResp: unknown, -): { action: "accept" | "decline"; reason?: string } { +export function parseExitPlanModeElicitationResponse(acpResp: unknown): { + action: "accept" | "decline"; + reason?: string; +} { if (!acpResp || typeof acpResp !== "object") { return { action: "decline", reason: "invalid client response" }; } diff --git a/tests/interaction.test.ts b/tests/interaction.test.ts index e039455..3dce084 100644 --- a/tests/interaction.test.ts +++ b/tests/interaction.test.ts @@ -76,6 +76,45 @@ describe("zcode permission → ACP", () => { it("returns null when no valid options", () => { expect(zcodePermissionToAcp({ toolCallId: "t", options: [] } as never, "acp_1")).toBeNull(); }); + + // Regression: zcode backend emits `kind: "deny"` (and other non-ACP values) + // which Zed rejects at schema deserialization ("unknown variant `deny`"), + // making every permission popup fail instantly without user action. + // The adapter MUST normalize kinds to the 4 ACP-legal values. optionId is + // free text and must be preserved verbatim (zcode identifies the choice by it). + it("normalizes non-ACP kinds (deny → reject_once), preserves optionId", () => { + // Real payload observed from zcode backend (Zed.log deserialization error). + const params = { + requestId: "r1", + toolCallId: "tc_1", + toolName: "Bash", + input: { command: "ls" }, + options: [ + { optionId: "allow_once", kind: "allow_once", name: "Allow once" }, + { optionId: "allow_project", kind: "allow_always", name: "Always allow in this project" }, + { optionId: "deny", kind: "deny", name: "Deny" }, + ], + }; + const acp = zcodePermissionToAcp(params, "acp_1"); + expect(acp).not.toBeNull(); + expect(acp!.options.map((o) => o.kind)).toEqual(["allow_once", "allow_always", "reject_once"]); + // optionId untouched so the response mapping still recognizes the choice. + expect(acp!.options.map((o) => o.optionId)).toEqual(["allow_once", "allow_project", "deny"]); + }); + + it("maps legacy allow/reject/unknown kinds to ACP-legal values", () => { + const params = { + toolCallId: "tc", + options: [ + { optionId: "a", kind: "allow", name: "a" }, + { optionId: "r", kind: "reject", name: "r" }, + { optionId: "u", kind: "something_weird", name: "u" }, + ], + }; + const acp = zcodePermissionToAcp(params, "acp_1"); + // allow→allow_always; reject/unknown→reject_once (fail-safe, never auto-allow) + expect(acp!.options.map((o) => o.kind)).toEqual(["allow_always", "reject_once", "reject_once"]); + }); }); describe("ACP response → zcode permission", () => { @@ -96,6 +135,27 @@ describe("ACP response → zcode permission", () => { "deny", ); }); + + // Regression: zcode backend's allow-class options carry optionId "allow_once" + // / "allow_project" (not "allow"/"allow_always"). The response mapper must + // recognize these as allow, otherwise selecting "Allow once" in the popup + // is reported back to zcode as a rejection. + it("allow_once / allow_project optionId → decision allow", () => { + expect( + acpPermissionResponseToZcode({ outcome: { outcome: "selected", optionId: "allow_once" } }) + .decision, + ).toBe("allow"); + expect( + acpPermissionResponseToZcode({ outcome: { outcome: "selected", optionId: "allow_project" } }) + .decision, + ).toBe("allow"); + }); + + it("deny optionId → decision deny", () => { + expect( + acpPermissionResponseToZcode({ outcome: { outcome: "selected", optionId: "deny" } }).decision, + ).toBe("deny"); + }); }); describe("ExitPlanMode", () => {