fix(cursor): give advisors the replace-mode edit pi_edit needs

The primary bridge builds a `replace`-mode `EditTool` because
`PiEditExecArgs` carries `old_text`/`new_text` pairs that no other mode
accepts. The advisor roster passed its own instances straight through,
and those follow the session's configured `edit.mode` - `hashline` by
default, whose schema is a single `input` string - so every native
advisor edit failed validation instead of touching the file.

Both bridge-only tools now come from `cursor-bridge-tools.ts`:
`createBridgeEditTool` builds the wrapped `replace` instance, and
`bridgeToolMap` substitutes it into a granted map. The substitution is
gated on `edit` actually having been granted, since the tool is
constructed rather than looked up - handing one to a read-only roster is
the #5680 escalation. The advisor's own loop keeps its instance; only
the exec map is swapped.

(cherry picked from commit e6cf9f8046c595cab9793b4b2a5795d8488d8d22)

packages/coding-agent/CHANGELOG.md

@@ -139,6 +139,7 @@
 - Fixed every Cursor `pi_edit` frame failing instead of editing. Two independent causes: the session drops `edit` from the tool registry for Cursor so the model uses full-file `write`, but that registry is also the exec bridge's tool source, so the native frame — which the server sends regardless of the advertised catalog — found no tool; and the retained instance followed the session's configured edit mode, while `PiEditExecArgs` carries `old_text`/`new_text` pairs that only `replace` accepts (the default `hashline` takes a single `input` string). The bridge now resolves a `replace`-mode instance through its fallback resolver, still wrapped for approval.
 - Fixed a `pi_grep` frame carrying `context` or `limit` escaping the approval gate. Honoring those fields needs a per-call `grep`, and the per-call instance was built raw while every registry tool is wrapped, so such calls bypassed `tools.approval.grep` and the exec-tier check for SSH-targeted paths. Both bridge callsites now build it through one shared factory that applies the same wrapper.
 - Fixed Cursor advisors ignoring `pi_grep`'s `context` and `limit`. Only the primary session supplied the per-call `grep` factory, so advisor frames silently fell back to session defaults. Advisors now receive the same factory, gated on the advisor actually having been granted `grep`.
+- Fixed Cursor advisors failing every `pi_edit`. The advisor roster handed the bridge the `edit` instance built for the advisor's own loop, which follows the configured `edit.mode` (`hashline` by default) and rejects the frame's `old_text`/`new_text` pairs — the same mode mismatch the primary bridge already fixed, on the path it missed. The exec map now substitutes a `replace`-mode instance, gated on the advisor actually having been granted `edit`, while the advisor's own loop keeps the tool it was given.
 - Fixed `pi_bash` killing commands that explicitly asked for no deadline. `timeout` is `optional int32` and `bash` documents `0` as "disables the command deadline", but a truthiness check folded a supplied `0` into unset, applying the 300s default instead. A present `0` now passes through; negatives, which have no local meaning and would otherwise clamp to the 1s floor, still fall back to the default.
 - Fixed the Cursor exec bridge granting `edit` and `grep` to sessions that withheld them. Both bridge-only tools are constructed rather than looked up, and `executeTool` prefers a constructed override over the registry, so a restricted tool set (`toolNames` without them, or `restrictToolNames`) still got a working `pi_edit`/`pi_grep` — native frames arrive regardless of the advertised catalog. Both are now gated on the session having actually granted the tool, matching the `delete` frame's existing check (issue #5680).
 

packages/coding-agent/src/cursor-bridge-tools.ts

@@ -9,6 +9,7 @@
  */
 
 import type { AgentTool } from "@oh-my-pi/pi-agent-core";
+import { EditTool } from "./edit";
 import type { ExtensionRunner } from "./extensibility/extensions";
 import { ExtensionToolWrapper } from "./extensibility/extensions";
 import type { GrepToolOptions, Tool, ToolSession } from "./tools";
@@ -34,3 +35,45 @@ export function createBridgeGrepFactory(
 		return new ExtensionToolWrapper(grepTool, extensionRunner);
 	};
 }
+
+/**
+ * Build the `replace`-mode `edit` the bridge answers `pi_edit` with.
+ *
+ * `PiEditExecArgs` carries `old_text`/`new_text` pairs, which is exactly
+ * `replace`'s schema and nothing else's. The session's own instance follows the
+ * configured `edit.mode` — `hashline` by default, whose schema is a single
+ * `input` string — so a frame handed that instance fails validation instead of
+ * editing the file.
+ *
+ * Callers MUST gate this on the session having actually granted `edit`: the
+ * tool is constructed rather than looked up, so building one unconditionally
+ * hands a restricted agent a mutating tool it was denied (issue #5680).
+ */
+export function createBridgeEditTool(session: ToolSession, extensionRunner: ExtensionRunner): AgentTool {
+	const editTool: Tool = new EditTool(session, "replace");
+	return new ExtensionToolWrapper(editTool, extensionRunner);
+}
+
+/**
+ * The tool map the exec bridge should run, given the map a caller granted.
+ *
+ * `pi_edit` needs a `replace`-mode instance, but only when `edit` was granted:
+ * the tool is constructed rather than looked up, so substituting
+ * unconditionally would hand a restricted roster a mutating tool it was denied
+ * (issue #5680). The granted map is never mutated: an unsubstituted result is a
+ * copy, so a caller without an `edit` grant cannot accidentally gain one.
+ *
+ * The advisor roster passes its granted map here; the primary session keeps its
+ * instance out of the registry entirely (Cursor does not advertise `edit`) and
+ * serves it through the bridge's `getTool` fallback instead.
+ */
+export function bridgeToolMap(
+	granted: ReadonlyMap<string, AgentTool>,
+	createEditTool: (() => AgentTool | undefined) | undefined,
+): Map<string, AgentTool> {
+	const bridged = new Map(granted);
+	if (!granted.has("edit") || !createEditTool) return bridged;
+	const bridgeEdit = createEditTool();
+	if (bridgeEdit) bridged.set("edit", bridgeEdit);
+	return bridged;
+}

packages/coding-agent/src/sdk.ts

@@ -62,7 +62,7 @@ import { applyProviderGlobalsFromSettings } from "./config/provider-globals";
 import { buildServiceTierByFamily } from "./config/service-tier";
 import { Settings, type SkillsSettings } from "./config/settings";
 import { CursorExecHandlers } from "./cursor";
-import { createBridgeGrepFactory } from "./cursor-bridge-tools";
+import { createBridgeEditTool, createBridgeGrepFactory } from "./cursor-bridge-tools";
 import "./discovery";
 import { initializeWithSettings } from "./discovery";
 import { disposeAllJuliaKernelSessions, disposeJuliaKernelSessionsByOwner } from "./eval/jl/executor";
@@ -2607,8 +2607,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 			toolRegistry.delete("edit");
 			builtInRegistryToolNames.delete("edit");
 			if (editWasGranted) {
-				const bridgeEdit: Tool = new EditTool(toolSession, "replace");
-				cursorBridgeEditTool = new ExtensionToolWrapper(bridgeEdit, extensionRunner);
+				cursorBridgeEditTool = createBridgeEditTool(toolSession, extensionRunner);
 			}
 		}
 
@@ -3273,6 +3272,9 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 			// advisor's own tool session so a `pi_grep` frame's context width and
 			// match cap are honored there too.
 			advisorCreateGrepTool: createBridgeGrepFactory(advisorToolSession, extensionRunner),
+			// Same `replace`-mode requirement as the primary bridge; the advisor
+			// path gates it on the advisor's own `edit` grant.
+			advisorCreateEditTool: () => createBridgeEditTool(advisorToolSession, extensionRunner),
 			titleSystemPrompt: options.titleSystemPrompt,
 		});
 		hasSession = true;

packages/coding-agent/src/session/agent-session-types.ts

@@ -215,6 +215,12 @@ export interface AgentSessionConfig {
 	 * running on Cursor silently drops both fields.
 	 */
 	advisorCreateGrepTool?(options: { context?: number; totalMatchLimit?: number }): AgentTool | undefined;
+	/**
+	 * Build the `replace`-mode `edit` a Cursor `pi_edit` frame needs, against the
+	 * advisor-scoped tool session. The advisor's ordinary instance follows the
+	 * configured `edit.mode` and rejects the frame's `old_text`/`new_text` pairs.
+	 */
+	advisorCreateEditTool?(): AgentTool | undefined;
 	/** Preloaded watchdog prompt content for the advisor. */
 	advisorWatchdogPrompt?: string;
 	/** Shared advisor instructions loaded from WATCHDOG.yml. */

packages/coding-agent/src/session/agent-session.ts

@@ -1291,6 +1291,7 @@ export class AgentSession {
 			enabled: this.settings.get("advisor.enabled"),
 			tools: config.advisorTools,
 			createGrepTool: config.advisorCreateGrepTool,
+			createEditTool: config.advisorCreateEditTool,
 			watchdogPrompt: config.advisorWatchdogPrompt,
 			sharedInstructions: config.advisorSharedInstructions,
 			contextPrompt: config.advisorContextPrompt,

packages/coding-agent/src/session/session-advisors.ts

@@ -68,6 +68,7 @@ import { MODEL_ROLES } from "../config/model-roles";
 import { serviceTierForAllFamilies, serviceTierSettingToTier } from "../config/service-tier";
 import type { Settings } from "../config/settings";
 import { CursorExecHandlers } from "../cursor";
+import { bridgeToolMap } from "../cursor-bridge-tools";
 import { estimateToolSchemaTokens } from "../modes/utils/context-usage";
 import type { PlanModeState } from "../plan-mode/state";
 import advisorSystemPrompt from "../prompts/advisor/system.md" with { type: "text" };
@@ -184,6 +185,13 @@ export interface SessionAdvisorsOptions {
 	 * drops both fields — the same gap the primary bridge closes.
 	 */
 	createGrepTool?(options: { context?: number; totalMatchLimit?: number }): AgentTool | undefined;
+	/**
+	 * Build the `replace`-mode `edit` a Cursor `pi_edit` frame needs. The
+	 * advisor's own instance follows the configured `edit.mode` (`hashline` by
+	 * default), whose schema the frame's `old_text`/`new_text` pairs do not
+	 * match, so without this every native advisor edit fails validation.
+	 */
+	createEditTool?(): AgentTool | undefined;
 	watchdogPrompt?: string;
 	sharedInstructions?: string;
 	contextPrompt?: string;
@@ -258,6 +266,7 @@ export class SessionAdvisors {
 	#advisorEnabled: boolean;
 	#advisorTools: AgentTool[] | undefined;
 	#advisorCreateGrepTool: SessionAdvisorsOptions["createGrepTool"];
+	#advisorCreateEditTool: SessionAdvisorsOptions["createEditTool"];
 	#advisorWatchdogPrompt: string | undefined;
 	#advisorSharedInstructions: string | undefined;
 	#advisorContextPrompt: string | undefined;
@@ -281,6 +290,7 @@ export class SessionAdvisors {
 		this.#advisorEnabled = options.enabled;
 		this.#advisorTools = options.tools;
 		this.#advisorCreateGrepTool = options.createGrepTool;
+		this.#advisorCreateEditTool = options.createEditTool;
 		this.#advisorWatchdogPrompt = options.watchdogPrompt;
 		this.#advisorSharedInstructions = options.sharedInstructions;
 		this.#advisorContextPrompt = options.contextPrompt;
@@ -727,10 +737,16 @@ export class SessionAdvisors {
 			// to delete workspace files it was never granted (issue #5680 review).
 			const advisorCanMutateFiles = advisorToolMap.has("write") || advisorToolMap.has("edit");
 			if (advisorCanMutateFiles) availableAdvisorToolNames.add("delete");
+			// `pi_edit` speaks `replace`'s `old_text`/`new_text` schema, which the
+			// advisor's ordinary `EditTool` (built at the session's configured
+			// `edit.mode`, `hashline` by default) does not accept. The bridge map
+			// swaps in a `replace` instance for the exec channel only — the
+			// advisor's own loop keeps the tool it was given — and only when
+			// `edit` was actually granted.
 			const advisorCursorExecHandlers = new CursorExecHandlers({
 				cwd: this.#host.sessionManager.getCwd(),
 				getCwd: () => this.#host.sessionManager.getCwd(),
-				tools: advisorToolMap,
+				tools: bridgeToolMap(advisorToolMap, this.#advisorCreateEditTool),
 				allowNativeDelete: advisorCanMutateFiles,
 				// Gated on the advisor's own grant: the factory builds a fresh
 				// tool, so handing it over unconditionally would give a roster

packages/coding-agent/test/cursor-exec.test.ts

@@ -19,7 +19,11 @@ import {
 } from "@oh-my-pi/pi-catalog/discovery/cursor-gen/agent_pb";
 import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
 import { CursorExecHandlers } from "@oh-my-pi/pi-coding-agent/cursor";
-import { createBridgeGrepFactory } from "@oh-my-pi/pi-coding-agent/cursor-bridge-tools";
+import {
+	bridgeToolMap,
+	createBridgeEditTool,
+	createBridgeGrepFactory,
+} from "@oh-my-pi/pi-coding-agent/cursor-bridge-tools";
 import { EditTool } from "@oh-my-pi/pi-coding-agent/edit";
 import type { ExtensionRunner } from "@oh-my-pi/pi-coding-agent/extensibility/extensions";
 import { ExtensionToolWrapper } from "@oh-my-pi/pi-coding-agent/extensibility/extensions";
@@ -41,6 +45,24 @@ function createTestSession(cwd: string, overrides: Partial<ToolSession> = {}): T
 	};
 }
 
+/**
+ * An `ExtensionRunner` that intercepts nothing but records that it ran.
+ *
+ * The bridge's per-call tools must carry the same wrapper as registry tools;
+ * seeing a name arrive here proves the wrapper is present, since an unwrapped
+ * tool never announces.
+ */
+function passthroughRunner(seen: string[] = []): ExtensionRunner {
+	return {
+		hasHandlers: () => true,
+		emitToolCall: async (event: { toolName: string }) => {
+			seen.push(event.toolName);
+			return undefined;
+		},
+		emitToolResult: async () => undefined,
+	} as unknown as ExtensionRunner;
+}
+
 describe("CursorExecHandlers.grep bridge", () => {
 	let cwd: string;
 	let searchTool: GrepTool;
@@ -223,10 +245,11 @@ describe("bridge tool resolution beyond the model-facing registry", () => {
 		// edit answers "Tool \"edit\" not available" and the file is untouched.
 		const target = path.join(cwd, "sample.txt");
 		await Bun.write(target, "alpha\nbeta\n");
-		const session = createTestSession(cwd);
-		// `replace` is the mode `pi_edit`'s `old_text`/`new_text` pairs speak;
-		// the session default is `hashline`, whose schema they do not match.
-		const editTool: Tool = new EditTool(session, "replace");
+		// Build it exactly as the session does. Both bridge callsites go through
+		// this factory, so a regression in it — the wrong mode, a missing
+		// approval wrapper — fails here rather than passing against a
+		// hand-constructed stand-in.
+		const editTool = createBridgeEditTool(createTestSession(cwd), passthroughRunner());
 
 		const withheld = new CursorExecHandlers({
 			cwd,
@@ -255,6 +278,57 @@ describe("bridge tool resolution beyond the model-facing registry", () => {
 		expect(await Bun.file(target).text()).toBe("alpha\nbeta\n");
 	});
 
+	it("substitutes a replace-mode edit into a granted advisor tool map", async () => {
+		// The advisor roster hands the bridge the instances it built for the
+		// advisor's own loop — default `hashline` mode, whose schema is a single
+		// `input` string. A `pi_edit` frame's `old_text`/`new_text` pairs fail
+		// validation against it, so the file goes unmodified. This is the
+		// substitution the advisor path applies before constructing handlers.
+		const target = path.join(cwd, "sample.txt");
+		await Bun.write(target, "alpha\nbeta\n");
+		const session = createTestSession(cwd);
+		const advisorEdit = new EditTool(session);
+		expect(advisorEdit.mode).not.toBe("replace");
+		const granted = new Map<string, Tool>([["edit", advisorEdit]]);
+
+		const bridged = bridgeToolMap(granted, () => createBridgeEditTool(session, passthroughRunner()));
+		const handlers = new CursorExecHandlers({ cwd, tools: bridged });
+		const result = await handlers.piEdit({
+			toolCallId: "e3",
+			args: { path: target, edits: [{ oldText: "beta", newText: "gamma" }] },
+		} as never);
+
+		expect(result.isError).toBeFalsy();
+		expect(await Bun.file(target).text()).toBe("alpha\ngamma\n");
+		// The advisor's own loop must keep the exact instance it was handed.
+		expect(granted.get("edit")).toBe(advisorEdit);
+	});
+
+	it("leaves an ungranted tool map without an edit tool", async () => {
+		// The bridge tool is constructed, not looked up, so substituting for a
+		// roster that was never granted `edit` would hand a read-only advisor a
+		// mutating tool (issue #5680). The frame must fail instead.
+		const target = path.join(cwd, "sample.txt");
+		await Bun.write(target, "alpha\nbeta\n");
+		const session = createTestSession(cwd);
+		let built = 0;
+		const withheld = bridgeToolMap(new Map<string, Tool>(), () => {
+			built++;
+			return createBridgeEditTool(session, passthroughRunner());
+		});
+		expect(withheld.has("edit")).toBe(false);
+		expect(built).toBe(0);
+
+		const handlers = new CursorExecHandlers({ cwd, tools: withheld });
+		const result = await handlers.piEdit({
+			toolCallId: "e4",
+			args: { path: target, edits: [{ oldText: "beta", newText: "gamma" }] },
+		} as never);
+
+		expect(result.isError).toBe(true);
+		expect(await Bun.file(target).text()).toBe("alpha\nbeta\n");
+	});
+
 	it("refuses a scoped pi_grep when no grep tool was granted", async () => {
 		// The factory builds a fresh tool and `executeTool` prefers that override
 		// over the registry, so a session that withheld `grep` must not install
@@ -277,16 +351,7 @@ describe("bridge tool resolution beyond the model-facing registry", () => {
 		// both callsites use, so a regression in it fails here.
 		await Bun.write(path.join(cwd, "hit.txt"), "needle\n");
 		const intercepted: string[] = [];
-		const runner = {
-			hasHandlers: () => true,
-			emitToolCall: async (event: { toolName: string }) => {
-				intercepted.push(event.toolName);
-				return undefined;
-			},
-			emitToolResult: async () => undefined,
-		} as unknown as ExtensionRunner;
-
-		const factory = createBridgeGrepFactory(createTestSession(cwd), runner);
+		const factory = createBridgeGrepFactory(createTestSession(cwd), passthroughRunner(intercepted));
 		const built = factory({ context: 0, totalMatchLimit: 5 });
 		expect(built).toBeInstanceOf(ExtensionToolWrapper);