From 66ee251e9fa8877a33baf8699b546534d25fd3e7 Mon Sep 17 00:00:00 2001 From: Solenta Date: Sun, 18 Oct 2026 21:50:51 +0200 Subject: [PATCH] Answer permission and plan cards with y / n / a; always-allow a command prefix, not all of Bash (#477) While a tool permission or plan card is pending, y accepts/approves, n denies/keeps planning, a always-allows. Pending permission outranks typing: the keys are taken from a focused composer (capture phase, never typed). The card's own fields keep their keys, and guardrail-flagged cards never take keys from the composer. Allow-always on a Claude shell command now adds a session rule for the command prefix (npm test:*), editable on the card, instead of the whole tool. "Allow any Bash" stays as an explicit button with no key. Co-Authored-By: Claude Opus 5.5 --- electron/permissionCommand.js | 26 ++-- electron/runner-permissions.js | 18 +-- electron/test/claude.test.js | 5 +- electron/test/permissionCommand.test.js | 27 ++-- site/docs.html | 17 ++- src/coder/useCoderThreadActions.ts | 2 + src/components/Composer.tsx | 1 + src/components/ThreadView.module.css | 41 ++++++ src/components/ThreadView.tsx | 1 + src/components/thread/cards.tsx | 138 ++++++++++++++++++-- src/permissionRule.ts | 41 ++++++ src/shared/ipc.ts | 12 +- src/useCoder.ts | 1 + test/permissionPrompt.test.tsx | 160 +++++++++++++++++++++--- test/planPrompt.test.tsx | 16 +++ 15 files changed, 444 insertions(+), 62 deletions(-) create mode 100644 src/permissionRule.ts diff --git a/electron/permissionCommand.js b/electron/permissionCommand.js index 5e592b82d..bf6882e01 100644 --- a/electron/permissionCommand.js +++ b/electron/permissionCommand.js @@ -7,7 +7,8 @@ * Claude's addRules `ruleContent` for Bash is a prefix glob (`npm test:*`), * not the whole tool. After a human amends the command, that glob must key * on the *edited* command — never the original, and never a blank cheque - * for Bash. Unedited allow-always stays whole-tool; narrowing that is #479. + * for Bash. The renderer mirrors commandPrefix in src/permissionRule.ts to + * show the rule before it is sent (#477); keep the two in step. */ const COMMAND_KEYS = ["command", "cmd", "script"]; @@ -128,20 +129,25 @@ function commandPrefix(command) { } /** - * Session addRules payload. After an edit, `ruleContent` is the edited - * command's prefix so allow-always never keys on the original. Unedited - * stays `{ toolName }` (whole tool) — #479 owns narrowing that default. + * Session addRules payload for allow-always (#477/#479). A command tool keys + * on a prefix glob: the user's rule when given, else the prefix of the + * command that actually runs (the edited one after #509, never the + * original). Whole tool only when asked: `alwaysRule === null` is the + * explicit "any Bash". Tools without a command (Edit/Write) stay whole-tool, + * like the CLI's "allow all edits this session". * * @param {string} toolName - * @param {string | null} command - * @param {{ edited: boolean }} opts + * @param {string | null} command null for tools without a command field + * @param {string | null} [alwaysRule] */ -function sessionAllowRule(toolName, command, opts) { +function sessionAllowRule(toolName, command, alwaysRule) { /** @type {{ toolName: string, ruleContent?: string }} */ const rule = { toolName }; - if (opts && opts.edited) { - const prefix = commandPrefix(command); - if (prefix) rule.ruleContent = prefix; + if (command !== null && alwaysRule !== null) { + const content = + (typeof alwaysRule === "string" && alwaysRule.trim()) || + commandPrefix(command); + if (content) rule.ruleContent = content; } return { type: "addRules", diff --git a/electron/runner-permissions.js b/electron/runner-permissions.js index 6e430c7df..2a39be087 100644 --- a/electron/runner-permissions.js +++ b/electron/runner-permissions.js @@ -313,9 +313,10 @@ function createPermissions(ctx) { * (question text -> chosen label) rides back as updatedInput.answers. * `updatedCommand` (#509) replaces the shell command in updatedInput; * allow-always after an edit keys the session rule on the edited prefix. - * Codex JSON-RPC ignores `updatedCommand` (the reply cannot rewrite the - * command). - * @param {{ threadId: string, requestId: string, decision: "allow" | "allowAlways" | "allowFolder" | "deny", answers?: Record, updatedCommand?: string }} input + * `alwaysRule` (#477) overrides that prefix; null asks for the whole tool. + * Codex JSON-RPC ignores `updatedCommand` and `alwaysRule` (the reply + * cannot rewrite the command). + * @param {{ threadId: string, requestId: string, decision: "allow" | "allowAlways" | "allowFolder" | "deny", answers?: Record, updatedCommand?: string, alwaysRule?: string | null }} input */ function respondPermission(input) { const { threadId, requestId, decision, answers, updatedCommand } = @@ -510,12 +511,13 @@ function createPermissions(ctx) { // A governed thread keeps the CLI asking: a Claude session rule would // stop can_use_tool for that tool and skip the matrix next time. if (decision === "allowAlways" && !governed) { - // Unedited: whole-tool session rule (matches today's Accept all). - // Edited: prefix of the *edited* command, never the original (#509). + // Command prefix (or the user's glob), not the whole tool (#477). response.updatedPermissions = [ - sessionAllowRule(pending.toolName, resolved.next, { - edited: resolved.edited, - }), + sessionAllowRule( + pending.toolName, + resolved.field ? resolved.next : null, + input.alwaysRule, + ), ]; } } else { diff --git a/electron/test/claude.test.js b/electron/test/claude.test.js index 9d8899869..f020779eb 100644 --- a/electron/test/claude.test.js +++ b/electron/test/claude.test.js @@ -1539,7 +1539,8 @@ describe("runner claude provider", () => { await waitFor(() => store.getThread(thread.id).status === "done"); - // The CLI received a well-formed allow with a session-scoped rule. + // The CLI received a well-formed allow with a session-scoped rule on + // the command prefix, not all of Bash (#477). const ctrl = JSON.parse(fs.readFileSync(ctrlFile, "utf8")); assert.equal(ctrl.type, "control_response"); assert.equal(ctrl.response.subtype, "success"); @@ -1551,7 +1552,7 @@ describe("runner claude provider", () => { assert.deepEqual(ctrl.response.response.updatedPermissions, [ { type: "addRules", - rules: [{ toolName: "Bash" }], + rules: [{ toolName: "Bash", ruleContent: "npm test:*" }], behavior: "allow", destination: "session", }, diff --git a/electron/test/permissionCommand.test.js b/electron/test/permissionCommand.test.js index 35a63e617..4b8788c4e 100644 --- a/electron/test/permissionCommand.test.js +++ b/electron/test/permissionCommand.test.js @@ -73,21 +73,24 @@ describe("permissionCommand", () => { assert.equal(commandPrefix(" "), null); }); - it("sessionAllowRule keys edited allow-always on the edited prefix", () => { - assert.deepEqual(sessionAllowRule("Bash", "npm test", { edited: false }), { + it("sessionAllowRule keys allow-always on the command prefix, not the tool (#477)", () => { + const rules = (toolName, command, alwaysRule) => + sessionAllowRule(toolName, command, alwaysRule).rules; + assert.deepEqual(sessionAllowRule("Bash", "npm test -- foo"), { type: "addRules", - rules: [{ toolName: "Bash" }], + rules: [{ toolName: "Bash", ruleContent: "npm test:*" }], behavior: "allow", destination: "session", }); - assert.deepEqual( - sessionAllowRule("Bash", "npm build", { edited: true }), - { - type: "addRules", - rules: [{ toolName: "Bash", ruleContent: "npm build:*" }], - behavior: "allow", - destination: "session", - }, - ); + // The user's glob wins; blank falls back to the prefix. + assert.deepEqual(rules("Bash", "npm test", "npm *"), [ + { toolName: "Bash", ruleContent: "npm *" }, + ]); + assert.deepEqual(rules("Bash", "npm test", " "), [ + { toolName: "Bash", ruleContent: "npm test:*" }, + ]); + // Whole tool only when asked, or when the tool has no command. + assert.deepEqual(rules("Bash", "npm test", null), [{ toolName: "Bash" }]); + assert.deepEqual(rules("Edit", null), [{ toolName: "Edit" }]); }); }); diff --git a/site/docs.html b/site/docs.html index 465318ac6..31d379868 100644 --- a/site/docs.html +++ b/site/docs.html @@ -549,8 +549,8 @@

Codex sessions

A Codex command approval shows Accept and - Deny. Accept all appears only when - Codex offers approval for the session. The displayed command is + Deny. Accept for session appears only + when Codex offers approval for the session. The displayed command is read-only because this provider cannot change it through the approval response. Stop cancels pending requests. Unsupported approval request types are rejected rather than silently approved. @@ -669,6 +669,19 @@

Worktrees and pull requests

Codex command cards are read-only; see Codex sessions for their approval controls.

+

+ Answer permission and plan cards from the keyboard, like the CLIs: + y accepts (or approves the plan), n denies (or + keeps planning), and a always allows for the rest of the + session. A pending permission outranks typing: the keys work even + while the composer has focus, and are not typed into it. The card's + own fields (the command, the rule, plan notes) keep their keys, and a + card the guardrail flagged as risky never takes keys from the + composer. + Always-allow on a shell command is a prefix rule such as + npm test:*, not all of Bash. Edit the rule on the card, + or pick allow any Bash explicitly. +

PR state is live: checks show as badges on the thread card, and merging is a button, not a terminal command. Worktree merge and delete are diff --git a/src/coder/useCoderThreadActions.ts b/src/coder/useCoderThreadActions.ts index 81daf383d..d593d08b9 100644 --- a/src/coder/useCoderThreadActions.ts +++ b/src/coder/useCoderThreadActions.ts @@ -94,6 +94,7 @@ export function useCoderThreadActions({ updatedCommand?: string, inputValues?: InputValues, feedback?: string, + alwaysRule?: string | null, ) => { if (!selectedThreadId) return; const threadId = selectedThreadId; @@ -108,6 +109,7 @@ export function useCoderThreadActions({ updatedCommand, inputValues, ...(feedback ? { feedback } : {}), + ...(alwaysRule !== undefined ? { alwaysRule } : {}), }); setError(null); } catch (err) { diff --git a/src/components/Composer.tsx b/src/components/Composer.tsx index 06e6bf390..c499507e2 100644 --- a/src/components/Composer.tsx +++ b/src/components/Composer.tsx @@ -1959,6 +1959,7 @@ export const Composer = memo(function Composer({ disabled={disabled || sending} readOnly={dictating} data-vim-mode={vimEnabled ? vimMode : undefined} + data-composer-input="" />

void | Promise; /** "Implement in a new thread" from the plan prompt (#1501). */ onImplementPlan?: (plan: string, provider?: string) => void | Promise; diff --git a/src/components/thread/cards.tsx b/src/components/thread/cards.tsx index cb414a6ed..c81d06940 100644 --- a/src/components/thread/cards.tsx +++ b/src/components/thread/cards.tsx @@ -29,9 +29,60 @@ import { import { Markdown } from "../Markdown"; import { FileHunks } from "../TurnDiffPanel"; import { parsePatch } from "../../reviewItinerary"; +import { commandPrefix } from "../../permissionRule"; import styles from "../ThreadView.module.css"; import diffStyles from "../TurnDiffPanel.module.css"; +type PermissionKey = "y" | "n" | "a"; + +/** + * y / n / a answer a pending permission or plan card, like the CLIs (#477). + * Pending permission outranks typing: the keys are taken from a focused + * composer too, in the capture phase so the composer never sees them. + * Every other text field (the card's own command / rule / notes, search + * boxes) keeps its keys. A key with no handler (answered, empty command) + * passes through untouched. `fromComposer: false` (guardrail-flagged calls, + * CI sign-off) leaves the composer alone too: a stray `y` mid-sentence must + * not approve something the guardrail called risky. + */ +function usePermissionKeys( + keys: Partial void>>, + fromComposer = true, +) { + useEffect(() => { + const onKey = (ev: KeyboardEvent) => { + if (ev.metaKey || ev.ctrlKey || ev.altKey || ev.isComposing) return; + const run = keys[ev.key as PermissionKey]; + if (!run) return; + const t = ev.target as HTMLElement | null; + if ( + t && + !(fromComposer && t.hasAttribute?.("data-composer-input")) && + (t.tagName === "INPUT" || + t.tagName === "TEXTAREA" || + t.tagName === "SELECT" || + t.isContentEditable) + ) { + return; + } + ev.preventDefault(); + ev.stopPropagation(); + run(); + }; + window.addEventListener("keydown", onKey, true); + return () => window.removeEventListener("keydown", onKey, true); + }); +} + +/** Key hint on a card button: `y` Accept. */ +function KeyHint({ k }: { k: PermissionKey }) { + return ( + + ); +} + /** * Plan approval (ExitPlanMode): the plan rendered as markdown in the prompt * panel, approve or send the agent back to planning (with optional notes), @@ -80,6 +131,9 @@ export function PlanPrompt({ notes || undefined, ); }; + usePermissionKeys( + sent ? {} : { y: () => answer("allow"), n: () => answer("deny") }, + ); const save = async () => { if (!onSave || saving) return; setSaving(true); @@ -127,17 +181,19 @@ export function PlanPrompt({ className={styles.permissionAllow} disabled={sent} onClick={() => answer("allow")} + aria-keyshortcuts="y" > - Approve plan + Approve plan {onImplement ? ( {acceptAlways ? ( ) : null} {pending.folderGrant ? ( @@ -367,10 +459,36 @@ export function PermissionPrompt({ className={styles.permissionDeny} disabled={sent} onClick={() => answer("deny")} + aria-keyshortcuts="n" > - Deny + Deny
+ {ruleEditable ? ( +
+ + +
+ ) : null} ); } diff --git a/src/permissionRule.ts b/src/permissionRule.ts new file mode 100644 index 000000000..fe342aa1b --- /dev/null +++ b/src/permissionRule.ts @@ -0,0 +1,41 @@ +/** + * Default allow-always rule shown on the permission card (#477): the prefix + * glob the main process would derive (`npm test:*`). Mirrors commandPrefix + * in electron/permissionCommand.js; keep the two in step. + */ + +/** Bins whose second token is the verb (`npm test`, `git status`). */ +const TWO_TOKEN_BINS = new Set([ + "npm", + "npx", + "yarn", + "pnpm", + "bun", + "bunx", + "cargo", + "go", + "git", + "docker", + "kubectl", + "composer", + "pip", + "pip3", + "poetry", + "uv", + "make", + "mise", +]); + +export function commandPrefix(command: string): string | null { + const withoutEnv = command + .trim() + .replace(/^(?:[A-Za-z_][A-Za-z0-9_]*=\S*\s+)+/, ""); + const tokens = withoutEnv.split(/\s+/).filter(Boolean); + if (tokens.length === 0) return null; + const bin = tokens[0].replace(/^.*[/\\]/, ""); + let prefix = tokens[0]; + if (TWO_TOKEN_BINS.has(bin) && tokens[1] && !tokens[1].startsWith("-")) { + prefix = `${tokens[0]} ${tokens[1]}`; + } + return `${prefix}:*`; +} diff --git a/src/shared/ipc.ts b/src/shared/ipc.ts index a261047fa..718726c58 100644 --- a/src/shared/ipc.ts +++ b/src/shared/ipc.ts @@ -2064,7 +2064,11 @@ export interface PendingQuestion { customAnswer?: boolean; } -/** User decision on a PendingPermissionInfo. "allowAlways" also allows the tool for the rest of the CLI session. */ +/** + * User decision on a PendingPermissionInfo. "allowAlways" also allows + * matching calls for the rest of the CLI session: the command prefix for + * command tools (see respondPermission's alwaysRule), else the whole tool. + */ export type PermissionDecision = | "allow" | "allowAlways" @@ -5095,6 +5099,12 @@ export interface CoderApi { */ updatedCommand?: string; inputValues?: InputValues; + /** + * allowAlways session rule for a command tool (#477), e.g. `npm test:*`. + * Omitted: the prefix of the command that runs. null: the whole tool + * (the explicit "any Bash"). Ignored by Codex and non-command tools. + */ + alwaysRule?: string | null; /** * "Keep planning" notes (#1501). Deny on a plan prompt only: a live * CLI gets them in the rejection; a persisted plan card sends them diff --git a/src/useCoder.ts b/src/useCoder.ts index 4c65bfd7c..c23f4a825 100644 --- a/src/useCoder.ts +++ b/src/useCoder.ts @@ -338,6 +338,7 @@ export interface UseCoderResult { updatedCommand?: string, inputValues?: InputValues, feedback?: string, + alwaysRule?: string | null, ) => Promise; /** Write plan markdown into the thread's checkout; resolves the path (#1501). */ savePlan: (plan: string, threadId?: string) => Promise; diff --git a/test/permissionPrompt.test.tsx b/test/permissionPrompt.test.tsx index 57c947159..9b5b5630f 100644 --- a/test/permissionPrompt.test.tsx +++ b/test/permissionPrompt.test.tsx @@ -1,7 +1,8 @@ /** * Tool permission card (#509): the proposed command is an editable field; - * Accept / Accept all send the edited command, not the original. Non-command - * tools keep the JSON preview. + * Accept / Always allow send the edited command, not the original. Non-command + * tools keep the JSON preview. y / n / a answer from the keyboard, even from + * a focused composer (#477). * * Run: node --import=./test/support/render.mjs --test test/permissionPrompt.test.tsx */ @@ -99,6 +100,7 @@ interface Spy { decision: PermissionDecision; answers?: Record; updatedCommand?: string; + alwaysRule?: string | null; }>; } @@ -121,8 +123,22 @@ function mountView(pending: PendingPermissionInfo): { onRemoveWorkflow={async () => {}} onStopRun={() => {}} onSetPermissionMode={() => {}} - onRespondPermission={(requestId, decision, answers, updatedCommand) => { - spy.calls.push({ requestId, decision, answers, updatedCommand }); + onRespondPermission={( + requestId, + decision, + answers, + updatedCommand, + _inputValues, + _feedback, + alwaysRule, + ) => { + spy.calls.push({ + requestId, + decision, + answers, + updatedCommand, + ...(alwaysRule !== undefined ? { alwaysRule } : {}), + }); }} onSetProvider={() => {}} onSetReasoningEffort={() => {}} @@ -143,7 +159,7 @@ describe("PermissionPrompt (#509)", () => { assert.ok(ta, "command textarea"); assert.equal(ta.value, "rm -rf dist && npm build"); assert.ok(view.byText("Accept")); - assert.ok(view.byText("Accept all")); + assert.ok(view.byText("Always allow")); assert.ok(view.byText("Deny")); assert.equal(view.query("[data-edited]"), null); }); @@ -180,7 +196,7 @@ describe("PermissionPrompt (#509)", () => { ]); }); - it("Accept all after an edit sends allowAlways with the edited command", async () => { + it("Always allow after an edit sends allowAlways with the edited command and its prefix", async () => { const { m, spy } = mountView(bashPending); const view = await m; const ta = view.query( @@ -188,9 +204,11 @@ describe("PermissionPrompt (#509)", () => { ) as HTMLTextAreaElement | null; assert.ok(ta); await view.type(ta, "npm build"); - await view.click(view.byText("Accept all")); + assert.match(view.byText("Always allow")!.textContent!, /npm build:\*/); + await view.click(view.byText("Always allow")); assert.equal(spy.calls[0].decision, "allowAlways"); assert.equal(spy.calls[0].updatedCommand, "npm build"); + assert.equal(spy.calls[0].alwaysRule, "npm build:*"); }); it("Reset restores the original command", async () => { @@ -206,7 +224,7 @@ describe("PermissionPrompt (#509)", () => { assert.equal(view.query("[data-edited]"), null); }); - it("empty command disables Accept / Accept all; Deny still works", async () => { + it("empty command disables Accept / Always allow; Deny still works", async () => { const { m, spy } = mountView(bashPending); const view = await m; const ta = view.query( @@ -215,7 +233,7 @@ describe("PermissionPrompt (#509)", () => { assert.ok(ta); await view.type(ta, " "); const accept = view.byText("Accept") as HTMLButtonElement; - const always = view.byText("Accept all") as HTMLButtonElement; + const always = view.byText("Always allow") as HTMLButtonElement; assert.equal(accept.disabled, true); assert.equal(always.disabled, true); await view.click(view.byText("Deny")); @@ -241,14 +259,15 @@ describe("PermissionPrompt (#509)", () => { assert.equal(spy.calls[0].updatedCommand, undefined); }); - it("hides Accept all when acceptAlways is false (#1171)", async () => { + it("hides allow-always when acceptAlways is false (#1171)", async () => { const { m } = mountView({ ...bashPending, acceptAlways: false, }); const view = await m; assert.ok(view.byText("Accept")); - assert.ok(!view.text().includes("Accept all")); + assert.equal(view.query("[data-permission-always]"), null); + assert.equal(view.query("[data-permission-rule]"), null); }); it("Codex fileChange shows the diff, reason and grantRoot instead of JSON (#1189)", async () => { @@ -273,7 +292,7 @@ describe("PermissionPrompt (#509)", () => { assert.ok(!view.text().includes('"changes"')); assert.match(view.query("[data-permission-reason]")?.textContent || "", /needs write access/); assert.match(view.query("[data-permission-grant-root]")?.textContent || "", /\/tmp\/other/); - await view.click(view.byText("Accept all")); + await view.click(view.query("[data-permission-always]")); assert.equal(spy.calls[0].decision, "allowAlways"); assert.equal(spy.calls[0].updatedCommand, undefined); }); @@ -287,6 +306,113 @@ describe("PermissionPrompt (#509)", () => { assert.equal(spy.calls[0].decision, "allow"); assert.equal(spy.calls[0].updatedCommand, undefined); }); + + it("Codex and non-command tools keep a plain session accept, no rule (#477)", async () => { + for (const pending of [ + { ...bashPending, commandEditable: false }, + editPending, + ]) { + const { m, spy } = mountView(pending); + const view = await m; + assert.equal(view.query("[data-permission-rule]"), null); + await view.click(view.byText("Accept for session")); + assert.equal(spy.calls[0].decision, "allowAlways"); + assert.equal("alwaysRule" in spy.calls[0], false); + view.unmount(); + } + }); +}); + +/** Press a key; true when the card took it (the field never saw the keydown). */ +async function key(view: Mounted, el: Element, k: string): Promise { + let reached = false; + const seen = () => { + reached = true; + }; + el.addEventListener("keydown", seen); + await view.press(el, k); + el.removeEventListener("keydown", seen); + return !reached; +} + +describe("PermissionPrompt keyboard (#477)", () => { + const npmPending: PendingPermissionInfo = { + ...bashPending, + summary: "Bash: npm test -- foo", + command: "npm test -- foo", + }; + + for (const [k, decision] of [ + ["y", "allow"], + ["n", "deny"], + ["a", "allowAlways"], + ] as const) { + it(`${k} answers ${decision} from a focused composer and is not typed`, async () => { + const { m, spy } = mountView(npmPending); + const view = await m; + const composer = view.query( + "[data-composer-input]", + ) as HTMLTextAreaElement; + assert.ok(composer, "composer textarea"); + composer.focus(); + assert.equal(await key(view, composer, k), true); + assert.equal(composer.value, ""); + assert.equal(spy.calls.length, 1); + assert.equal(spy.calls[0].decision, decision); + if (k === "a") assert.equal(spy.calls[0].alwaysRule, "npm test:*"); + }); + } + + it("the card's own fields keep their keys; answered cards release them", async () => { + const { m, spy } = mountView(npmPending); + const view = await m; + const ta = view.query("[data-permission-command]")!; + const rule = view.query("[data-permission-rule]")!; + assert.equal(await key(view, ta, "y"), false); + assert.equal(await key(view, rule, "a"), false); + assert.equal(spy.calls.length, 0); + const composer = view.query("[data-composer-input]")!; + assert.equal(await key(view, composer, "n"), true); + assert.equal(await key(view, composer, "y"), false); + assert.equal(spy.calls.length, 1); + }); + + it("a guardrail-flagged call never takes keys from the composer", async () => { + const { m, spy } = mountView({ + ...npmPending, + guardrail: { rule: "RISKY", reason: "touches prod" }, + }); + const view = await m; + assert.equal(await key(view, view.query("[data-composer-input]")!, "y"), false); + assert.equal(spy.calls.length, 0); + // Outside any text field the keys still answer. + assert.equal(await key(view, view.query("[data-permission-card]")!, "y"), true); + assert.equal(spy.calls[0].decision, "allow"); + }); + + it("modifiers pass through (Cmd+A still selects all)", async () => { + const { m, spy } = mountView(npmPending); + const view = await m; + await view.press(view.query("[data-composer-input]"), "a", { metaKey: true }); + assert.equal(spy.calls.length, 0); + }); + + it("an edited rule is sent; any-tool sends null (#479)", async () => { + const { m, spy } = mountView(npmPending); + const view = await m; + const rule = view.query("[data-permission-rule]") as HTMLInputElement; + assert.equal(rule.value, "npm test:*"); + await view.type(rule, "npm *"); + assert.match(view.byText("Always allow")!.textContent!, /npm \*/); + await view.click(view.byText("Always allow")); + assert.equal(spy.calls[0].alwaysRule, "npm *"); + + const second = mountView(npmPending); + const v2 = await second.m; + await v2.click(v2.query("[data-permission-any-tool]")); + assert.equal(second.spy.calls[0].decision, "allowAlways"); + assert.equal(second.spy.calls[0].alwaysRule, null); + }); }); describe("PermissionPrompt governance strip (#262)", () => { @@ -298,10 +424,10 @@ describe("PermissionPrompt governance strip (#262)", () => { view.query("[data-permission-guardrail]")?.textContent, "⚠ Deletes files recursively (rm-rf)", ); - assert.ok(view.text().includes("Accept all")); + assert.ok(view.query("[data-permission-always]")); }); - it("shows reason (layer / rule) and keeps Accept all for builtin", async () => { + it("shows reason (layer / rule) and keeps allow-always for builtin", async () => { const view = await mountView({ ...bashPending, guardrail: { ...risky, layer: "builtin" }, @@ -310,10 +436,10 @@ describe("PermissionPrompt governance strip (#262)", () => { view.query("[data-permission-guardrail]")?.textContent, "⚠ Deletes files recursively (builtin / rm-rf)", ); - assert.ok(view.text().includes("Accept all")); + assert.ok(view.query("[data-permission-always]")); }); - it("hides Accept all on profile.ask", async () => { + it("hides allow-always on profile.ask", async () => { const view = await mountView({ ...bashPending, guardrail: { @@ -327,7 +453,7 @@ describe("PermissionPrompt governance strip (#262)", () => { /\(profile\.ask \/ bash:git push\*\)/, ); assert.ok(view.byText("Accept")); - assert.ok(!view.text().includes("Accept all")); + assert.equal(view.query("[data-permission-always]"), null); }); }); diff --git a/test/planPrompt.test.tsx b/test/planPrompt.test.tsx index 78ca32034..d9efac8ca 100644 --- a/test/planPrompt.test.tsx +++ b/test/planPrompt.test.tsx @@ -166,6 +166,22 @@ describe("PlanPrompt", () => { ]); }); + it("y approves and n keeps planning, even from the composer (#477)", async () => { + for (const [k, decision] of [["y", "allow"], ["n", "deny"]] as const) { + const { m, spy } = mountView(); + const view = await m; + const composer = view.query("[data-composer-input]")!; + await view.press(composer, k); + assert.deepEqual(spy.calls, [{ requestId: "req-plan-1", decision }]); + view.unmount(); + } + // Typing notes keeps its keys. + const { m, spy } = mountView(); + const view = await m; + await view.press(view.query("textarea[data-plan-feedback]"), "n"); + assert.deepEqual(spy.calls, []); + }); + it("sends Keep planning notes, and Approve ignores them (#1501)", async () => { const { m, spy } = mountView(); const view = await m;