From 6f3d6ba2e44560b82cb72b3569eb25ca40e05381 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 21 Jun 2026 04:04:14 +0000 Subject: [PATCH] fix(coding-agent): restricted plan-mode write activation to built-ins Tracked current-registry built-in provenance through AgentSession so plan mode only force-activates the built-in write implementation. Extension or SDK tools that shadow the name `write` stay inactive, preserving plan mode's read-only contract through the built-in write/edit guard. Added a regression that registers a shadowing write tool without built-in provenance and verifies plan mode does not activate it. --- .../src/modes/interactive-mode.ts | 13 +++++--- packages/coding-agent/src/sdk.ts | 9 +++++ .../coding-agent/src/session/agent-session.ts | 9 +++++ ...interactive-mode-default-plan-mode.test.ts | 33 +++++++++++++++---- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 98d9c5770..5ce4f31c0 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -1931,10 +1931,15 @@ export class InteractiveMode implements InteractiveModeContext { // agent falls back to `edit` on a non-existent file and stalls. `edit` is an // essential built-in so it survives `tools.discoveryMode === "all"`, but // `write` has `loadMode: "discoverable"` and is hidden behind - // `search_tool_bm25` — re-activate it here whenever the registry built it - // (issue #3165). `resolve` is hidden too; the standing handler below - // consumes plan-approval calls through it. - const planAugmentations = ["resolve", "write"].filter(name => this.session.getToolByName(name) !== undefined); + // `search_tool_bm25` — re-activate it here only when the current registry + // entry is the built-in write tool (issue #3165). A shadowing extension + // tool named `write` must stay inactive because plan mode's read-only + // guarantee relies on the built-in write/edit guard. `resolve` is hidden + // too; the standing handler below consumes plan-approval calls through it. + const planAugmentations = ["resolve"]; + if (this.session.hasBuiltInTool("write")) { + planAugmentations.push("write"); + } const uniquePlanTools = [...new Set([...previousTools, ...planAugmentations])]; this.#planModePreviousTools = previousTools; diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index d4133af98..f3d95c8a1 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -2014,18 +2014,22 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} ); // All built-in tools are active (conditional tools like git/ask return null from factory if disabled) + const builtInRegistryToolNames = new Set(); const toolRegistry = new Map(); for (const tool of builtinTools) { toolRegistry.set(tool.name, tool); + builtInRegistryToolNames.add(tool.name); } if (!toolRegistry.has("goal") && settings.get("goal.enabled")) { const goalTool = await logger.time("createTools:goal:session", HIDDEN_TOOLS.goal, toolSession); if (goalTool) { toolRegistry.set(goalTool.name, wrapToolWithMetaNotice(goalTool)); + builtInRegistryToolNames.add(goalTool.name); } } for (const tool of wrappedExtensionTools) { toolRegistry.set(tool.name, tool); + builtInRegistryToolNames.delete(tool.name); } if (deferMCPDiscoveryForUI && mcpManager) { for (const name of collectPendingMCPToolNames(options.toolNames, existingSession.selectedMCPToolNames)) { @@ -2043,6 +2047,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} } if (model?.provider === "cursor") { toolRegistry.delete("edit"); + builtInRegistryToolNames.delete("edit"); } // `resolve` is hidden but must stay in the registry whenever any code path can invoke it: @@ -2055,10 +2060,12 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const needsResolveTool = hasDeferrableTools || planModeAvailable; if (!needsResolveTool) { toolRegistry.delete("resolve"); + builtInRegistryToolNames.delete("resolve"); } else if (!toolRegistry.has("resolve")) { const resolveTool = await logger.time("createTools:resolve:session", HIDDEN_TOOLS.resolve, toolSession); if (resolveTool) { toolRegistry.set(resolveTool.name, wrapToolWithMetaNotice(resolveTool)); + builtInRegistryToolNames.add(resolveTool.name); } } @@ -2075,6 +2082,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} searchTool.name, new ExtensionToolWrapper(wrapToolWithMetaNotice(searchTool), extensionRunner) as Tool, ); + builtInRegistryToolNames.add(searchTool.name); } let mcpDiscoveryEnabled = effectiveDiscoveryMode !== "off"; // back-compat: true when any discovery active @@ -2616,6 +2624,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} skillsSettings: settings.getGroup("skills"), modelRegistry, toolRegistry, + builtInToolNames: builtInRegistryToolNames, transformContext, onPayload, onResponse, diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index fa5146cb8..110970847 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -477,6 +477,8 @@ export interface AgentSessionConfig { modelRegistry: ModelRegistry; /** Tool registry for LSP and settings */ toolRegistry?: Map; + /** Tool names whose current registry entry is still the built-in implementation. */ + builtInToolNames?: Iterable; /** Current session pre-LLM message transform pipeline */ transformContext?: (messages: AgentMessage[], signal?: AbortSignal) => AgentMessage[] | Promise; /** Provider payload hook used by the active session request path */ @@ -1290,6 +1292,7 @@ export class AgentSession { // Generic tool discovery (covers built-in + MCP + extension when tools.discoveryMode === "all") #discoverableToolSearchIndex: DiscoverableToolSearchIndex | null = null; #selectedDiscoveredToolNames = new Set(); + #builtInToolNames = new Set(); #rpcHostToolNames = new Set(); #defaultSelectedMCPServerNames = new Set(); #defaultSelectedMCPToolNames = new Set(); @@ -1566,6 +1569,7 @@ export class AgentSession { this.#pruneToolDescriptions = config.pruneToolDescriptions === true; this.#validateRetryFallbackChains(); this.#toolRegistry = config.toolRegistry ?? new Map(); + this.#builtInToolNames = new Set(config.builtInToolNames ?? []); this.#requestedToolNames = config.requestedToolNames; this.#transformContext = config.transformContext ?? (messages => messages); this.#onPayload = config.onPayload; @@ -4496,6 +4500,11 @@ export class AgentSession { return this.#toolRegistry.get(name); } + /** True when the current registry entry for `name` came from a built-in factory. */ + hasBuiltInTool(name: string): boolean { + return this.#builtInToolNames.has(name); + } + /** * Get all configured tool names (built-in via --tools or default, plus custom tools). */ diff --git a/packages/coding-agent/test/interactive-mode-default-plan-mode.test.ts b/packages/coding-agent/test/interactive-mode-default-plan-mode.test.ts index ee48677c9..adde260f1 100644 --- a/packages/coding-agent/test/interactive-mode-default-plan-mode.test.ts +++ b/packages/coding-agent/test/interactive-mode-default-plan-mode.test.ts @@ -24,6 +24,11 @@ function makeTool(name: string): AgentTool { }; } +interface HarnessOptions { + extraRegistryTools?: readonly AgentTool[]; + builtInToolNames?: Iterable; +} + describe("InteractiveMode plan.defaultOnStartup", () => { let tempDir: TempDir; let authStorage: AuthStorage; @@ -67,8 +72,9 @@ describe("InteractiveMode plan.defaultOnStartup", () => { /** Build an InteractiveMode over a brand-new (never-persisted) session. * `extraRegistryTools` registers additional tools that are NOT initially * active — modeling tools hidden by `tools.discoveryMode === "all"` that - * modes may force-activate on entry. */ - function createHarness(settings: Settings, extraRegistryTools: readonly AgentTool[] = []): InteractiveMode { + * modes may force-activate on entry. `builtInToolNames` marks which registry + * entries still have built-in provenance after extension shadowing. */ + function createHarness(settings: Settings, options: HarnessOptions = {}): InteractiveMode { const registry = new ModelRegistry(authStorage, path.join(tempDir.path(), `models-${Bun.nanoseconds()}.yml`)); const initialModel = modelOrThrow(registry, "claude-sonnet-4-5"); const readTool = makeTool("read"); @@ -79,7 +85,7 @@ describe("InteractiveMode plan.defaultOnStartup", () => { [readTool.name, readTool], [resolveTool.name, resolveTool], ]); - for (const tool of extraRegistryTools) { + for (const tool of options.extraRegistryTools ?? []) { toolRegistry.set(tool.name, tool); } const manager = SessionManager.create(tempDir.path(), path.join(tempDir.path(), `active-${Bun.nanoseconds()}`)); @@ -97,6 +103,7 @@ describe("InteractiveMode plan.defaultOnStartup", () => { settings, modelRegistry: registry, toolRegistry, + builtInToolNames: options.builtInToolNames ?? ["read", "resolve"], }); session = createdSession; mode = new InteractiveMode(createdSession, "test"); @@ -120,9 +127,10 @@ describe("InteractiveMode plan.defaultOnStartup", () => { // not the initial active set. Plan-mode entry must force-activate it or // the agent only has `edit`, which fails on a non-existent file. const writeTool = makeTool("write"); - const created = createHarness(Settings.isolated({ "plan.defaultOnStartup": true, "compaction.enabled": false }), [ - writeTool, - ]); + const created = createHarness(Settings.isolated({ "plan.defaultOnStartup": true, "compaction.enabled": false }), { + extraRegistryTools: [writeTool], + builtInToolNames: ["read", "resolve", "write"], + }); expect(session?.getActiveToolNames()).not.toContain("write"); @@ -133,6 +141,19 @@ describe("InteractiveMode plan.defaultOnStartup", () => { expect(session?.getActiveToolNames()).toContain("resolve"); }); + it("does not activate an extension-shadowed write tool in plan mode", async () => { + const shadowWriteTool = makeTool("write"); + const created = createHarness(Settings.isolated({ "plan.defaultOnStartup": true, "compaction.enabled": false }), { + extraRegistryTools: [shadowWriteTool], + }); + + await created.init({ suppressWelcomeIntro: true }); + + expect(created.planModeEnabled).toBe(true); + expect(session?.getActiveToolNames()).toContain("resolve"); + expect(session?.getActiveToolNames()).not.toContain("write"); + }); + it("does not enter plan mode at startup by default", async () => { const created = createHarness(Settings.isolated({ "compaction.enabled": false }));