smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): restricted plan-mode write activation to built-ins

Browse Source 
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.
This commit is contained in:
roboomp
2026-06-21 04:04:14 +00:00
parent a7662b4208
commit 6f3d6ba2e4
4 changed files with 54 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/modes/interactive-mode.ts
+9 -4
View File
@@ -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;
packages/coding-agent/src/sdk.ts
+9
View File
@@ -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<string>();
const toolRegistry = new Map<string, Tool>();
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,
packages/coding-agent/src/session/agent-session.ts
+9
View File
@@ -477,6 +477,8 @@ export interface AgentSessionConfig {
modelRegistry: ModelRegistry;
/** Tool registry for LSP and settings */
toolRegistry?: Map<string, AgentTool>;
/** Tool names whose current registry entry is still the built-in implementation. */
builtInToolNames?: Iterable<string>;
/** Current session pre-LLM message transform pipeline */
transformContext?: (messages: AgentMessage[], signal?: AbortSignal) => AgentMessage[] | Promise<AgentMessage[]>;
/** 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<string>();
#builtInToolNames = new Set<string>();
#rpcHostToolNames = new Set<string>();
#defaultSelectedMCPServerNames = new Set<string>();
#defaultSelectedMCPToolNames = new Set<string>();
@@ -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).
*/
packages/coding-agent/test/interactive-mode-default-plan-mode.test.ts
+27 -6
View File
@@ -24,6 +24,11 @@ function makeTool(name: string): AgentTool {
};
}
interface HarnessOptions {
extraRegistryTools?: readonly AgentTool[];
builtInToolNames?: Iterable<string>;
}
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 }));