From cf38839ee7f8412e3e403078796e92d212630e94 Mon Sep 17 00:00:00 2001 From: abose Date: Sat, 5 Sep 2026 10:36:22 +0530 Subject: [PATCH 1/2] chore(ai): keep Bash off the permission allow list and log permission asks Verified against the CLI's own debug log that Auto mode's classifier does decide each Bash command (classifier_request_started / _finished, with permissionDecisionMs going from 10ms on a fast path to ~1.6s when the classifier actually runs), and that its asks reach our canUseTool handler and surface as the panel's Allow/Deny card. The CLI already discards a Bash allow rule passed via --allowedTools ("Ignoring dangerous permission Bash(*) from cliArg (bypasses classifier)"), so dropping it here changes nothing today. It removes the dependency on that behaviour: nothing in this list can pre-approve a shell command should a future CLI stop discarding it. - Bash off allowedTools, with the reasoning recorded beside the list. - Edit Mode's Bash confirm returns an explicit allow rather than {}, so a command the user just approved is not then re-checked by the CLI. - Log the tool, mode and the SDK's decisionReason whenever the CLI asks, so a genuine ask can be told apart from a silent auto-allow. --- src-node/claude-code-agent.js | 43 +++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/src-node/claude-code-agent.js b/src-node/claude-code-agent.js index 583016d059..8a5cb61154 100644 --- a/src-node/claude-code-agent.js +++ b/src-node/claude-code-agent.js @@ -1215,6 +1215,17 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, // always land here regardless of allowedTools. async function _onPermissionRequest(toolName, input, opts) { const promptSignal = (opts && opts.signal) || signal; + // Why the CLI is asking. In Auto this is the classifier deciding it + // wants a human, which is the whole point of the mode — logging it + // tells a genuine ask apart from a silent auto-allow. + const askParts = ["Permission ask:", toolName, "mode=" + _runtimePermissionMode]; + if (opts && opts.decisionReason) { + askParts.push("reason=" + opts.decisionReason); + } + if (opts && opts.blockedPath) { + askParts.push("blockedPath=" + opts.blockedPath); + } + _log.apply(null, askParts); if (toolName === "ExitPlanMode") { return _onExitPlanModeRequest(input, promptSignal); } @@ -1254,11 +1265,10 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, return { behavior: "allow", updatedInput: input }; } // Anything else the CLI wants a human decision on: Bash or a - // non-read-only MCP tool in Plan Mode, a classifier fallback in - // Auto, a tool outside allowedTools. With no prompt tool the CLI - // used to deny these on its own and nothing ever reached the - // panel — the user just saw the model give up. Put up the card. - _log("Permission request:", toolName, "mode=" + _runtimePermissionMode); + // non-read-only MCP tool in Plan Mode, a classifier ask in Auto, a + // tool outside allowedTools. With no prompt tool the CLI used to + // deny these on its own and nothing ever reached the panel — the + // user just saw the model give up. Put up the card. const allowed = await _askToolConfirm(requestId, toolName, input, promptSignal); if (allowed) { return { behavior: "allow", updatedInput: input }; @@ -1336,8 +1346,18 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, _hookErrorTimer = setTimeout(_flushHookError, HOOK_ERROR_FLUSH_MS); } }, + // Permission allow-rules, not a tool availability list. Bash is + // deliberately absent so that nothing here can pre-approve a shell + // command: every one is judged by the permission pipeline, and in + // Auto that means the SDK's classifier, whose "ask" verdicts reach + // canUseTool below as the panel's Allow/Deny card. The CLI happens + // to ignore a Bash allow rule anyway ("Ignoring dangerous permission + // Bash(*) from cliArg (bypasses classifier)"), so leaving it out + // simply stops the list from implying otherwise. Edit Mode still + // uses the manual confirm in the Bash PreToolUse hook below, and + // Allow Everything (bypassPermissions) skips permission checks. allowedTools: [ - "Read", "Edit", "Write", "Glob", "Grep", "Bash", + "Read", "Edit", "Write", "Glob", "Grep", "AskUserQuestion", "Task", "Agent", // Background-subagent plumbing: lets the main agent relay a // user follow-up to a running subagent (SendMessage), read its @@ -1812,7 +1832,16 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, const allowed = await _askToolConfirm( requestId, "Bash", input.tool_input, signal); if (allowed) { - return {}; + // Explicit allow, not {}: with Bash off the + // allow list, "no opinion" would send a + // command the user just approved on to the + // CLI's own permission check. + return { + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "allow" + } + }; } return { hookSpecificOutput: { From ce0b6e8caaf9b6befc1f8b1bdabec31030e199cb Mon Sep 17 00:00:00 2001 From: abose Date: Sat, 5 Sep 2026 10:44:07 +0530 Subject: [PATCH 2/2] feat(ai): classifier weighs preference writes, reads never prompt editorPreferences carried a permission allow rule, so in Auto every call went straight through, `set` included, without the classifier ever seeing it. Dropping the rule puts each write in front of the classifier, and a PreToolUse hook allows `get` and `list` outright so a read never prompts and never sits through a classifier round-trip either. The tool reads and writes through one entry point, so it cannot carry a static readOnlyHint the way getEditorState does; the operation has to be inspected per call, which is why this needs a hook rather than an annotation. Verified in the desktop app against the CLI's debug log (permissionDecisionMs, and whether a classifier request was made): Auto + get 0ms, no card (was 1957ms through the classifier) Auto + set 1386ms, classified: "operation=set id=fontSize ..." Plan + get 0ms, no card (previously raised a card) Edit Mode + get 0ms, no card --- src-node/claude-code-agent.js | 36 ++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src-node/claude-code-agent.js b/src-node/claude-code-agent.js index 8a5cb61154..c8bf7a6140 100644 --- a/src-node/claude-code-agent.js +++ b/src-node/claude-code-agent.js @@ -181,6 +181,16 @@ const nodeConnector = global.createNodeConnector(CONNECTOR_ID, exports); // start editing user files" — they share the plan-mode write-confirm card. const FILE_WRITE_TOOLS = ["Edit", "Write", "MultiEdit", "NotebookEdit"]; +// The preferences tool both reads and writes, so it cannot carry a static +// readOnlyHint the way getEditorState does — whether a call is harmless +// depends on its `operation`. +const EDITOR_PREFS_TOOL = "mcp__phoenix-editor__editorPreferences"; + +function _isPreferenceRead(input) { + const op = input && input.operation; + return op === "get" || op === "list"; +} + // Handed to the model right after the user approves a plan. The CLI leaves // plan mode on approval and the model carries on in the same turn, so this // is where "proceed" gets spelled out for Phoenix. @@ -1371,7 +1381,10 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, "mcp__phoenix-editor__takeScreenshot", "mcp__phoenix-editor__execJsInLivePreview", "mcp__phoenix-editor__execJsInEditor", - "mcp__phoenix-editor__editorPreferences", + // editorPreferences is absent for the same reason as Bash: it can + // write, so in Auto the classifier should weigh each call rather + // than a rule waving all of them through. Reads never reach a + // prompt — the PreToolUse hook below allows them outright. "mcp__phoenix-editor__editorDocs", "mcp__phoenix-editor__controlEditor", "mcp__phoenix-editor__resizeLivePreview", @@ -1853,6 +1866,27 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, } ] }, + { + // Reading a preference is free of side effects, so allow + // it outright: no card, and no classifier round-trip to + // sit through either. Writes return {} and take the + // normal route — the classifier in Auto, the card + // elsewhere. + matcher: EDITOR_PREFS_TOOL, + hooks: [ + async (input) => { + if (!_isPreferenceRead(input && input.tool_input)) { + return {}; + } + return { + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "allow" + } + }; + } + ] + }, { // Built-in agents (Explore, Plan, general-purpose) inherit // every tool, including this one. Keep the user's follow-up