diff --git a/src-node/claude-code-agent.js b/src-node/claude-code-agent.js index 583016d059..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. @@ -1215,6 +1225,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 +1275,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 +1356,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 @@ -1351,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", @@ -1812,7 +1845,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: { @@ -1824,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