fix(cursor): repair pi_edit and close the scoped-grep approval bypass

Three defects the exec bridge shipped with, all found by review. `pi_edit` never worked. The session removes `edit` from the tool registry for Cursor so the model is steered to full-file `write` (8ba0498eb), but that same registry is the bridge's tool source, so the native frame — which the server sends regardless of the advertised catalog — resolved nothing and answered `Tool "edit" not available`. Retaining the instance is not enough either: `PiEditExecArgs` carries `old_text`/`new_text` pairs, which only `replace` accepts, while the default mode is `hashline` (`{ input: string }`). `EditTool` now takes an optional mode, and the bridge resolves a pinned `replace` instance through its fallback resolver. A `pi_grep` frame carrying `context` or `limit` escaped the approval gate. Honoring those needs a per-call tool, and the per-call instance was built raw while every registry tool is wrapped — so exactly those calls skipped `tools.approval.grep` and the exec-tier SSH check. Both callsites now go through one `createBridgeGrepFactory`. Advisors ignored the same two fields: only the primary session supplied the factory. They now get it too, gated on the advisor actually holding `grep` so the factory cannot grant a denied tool. Also moves the pure Pi arg translation to `providers/cursor-pi-args`. The legacy shim shares it and is compiled into the bundled virtual registry, where `./providers/*` cannot match a nested specifier — it fell through to `Bun.resolveSync`, unsatisfiable under bunfs (#3442) — and the exec module would have dragged the protobuf graph along. Verified against real files and the real module graph: `pi_edit` mutates a temp file, the bundled probe executes the shim's shared module in a subprocess, and the grep test drives the shared factory. Mutation- checked: returning a raw tool from the factory, ignoring the pinned edit mode, dropping the `getTool` fallback, or moving the helpers back to a nested path each fails a test. (cherry picked from commit e46ba22b634e449005f7c22b6d0efd19a45ce1f8)
2026-07-27 10:57:17 -03:00
parent ab7b457af0
commit 6eaf090ccc
14 changed files with 340 additions and 74 deletions
@@ -53,6 +53,9 @@

 - Added `getProxyForUrl()` for transports that need provider-specific and standard proxy environment resolution with `NO_PROXY` support ([#6770](https://github.com/can1357/oh-my-pi/issues/6770)).
 - Added SiliconFlow and SiliconFlow (China) to the built-in API-key login provider catalog so `omp login siliconflow` / `omp login siliconflow-cn` stores a reusable credential validated against each region's `/v1/models` endpoint.
+### Changed
+
+- The Cursor Pi arg translation (`piReadPath`, `piJoinPath`, `piLsPath`, `piEscapeRegexLiteral`, `piLimit`) moved to `providers/cursor-pi-args`, re-exported from `providers/cursor/exec-modern` so existing imports are unaffected. The legacy pi shim shares these helpers and is compiled into the bundled virtual module registry, where a nested `providers/<dir>/<mod>` specifier is unresolvable under bunfs — and importing them from the exec module would drag the whole protobuf graph in for two string functions.

 ## [17.1.5] - 2026-07-27

@@ -0,0 +1,81 @@
+/**
+ * Translate a Pi frame's args into the local tool kwargs that run it.
+ *
+ * Shared deliberately by three consumers: the provider synthesizes a display
+ * block from these, the coding-agent bridge executes with them, and the legacy
+ * pi shim performs the identical translation for the old wire. Separate
+ * hand-rolled copies drift, and the drift is invisible — the transcript shows
+ * one operation while a different one runs.
+ *
+ * Kept apart from `cursor/exec-modern.ts` on purpose: these are pure
+ * string/path functions with no protobuf coupling, while that module pulls in
+ * `@bufbuild/protobuf` and the generated `agent_pb` graph. The legacy shim is
+ * compiled into the bundled virtual module registry, so importing it from a
+ * nested path would drag the whole exec implementation in with it — and
+ * `./providers/*` is a single-segment wildcard export that cannot serve a
+ * nested specifier under bunfs (issue #3442).
+ *
+ * Every `optional int32` here is presence-sensitive: `0` is a supplied value,
+ * not "unset", so it must never be folded into a default.
+ */
+
+import * as path from "node:path";
+
+/**
+ * A `pi_read` range composed onto the path as `read`'s inline `:N+K` selector.
+ *
+ * `read` exposes no range kwargs, so an uncomposed range reads the whole file.
+ * `offset` is a 1-indexed start clamped like the reference's
+ * `Math.max(0, offset - 1)` over 0-indexed lines; `limit` is a line count.
+ * `null` marks a present `limit: 0` — zero lines, which no selector expresses
+ * and which must not degrade into a whole-file read.
+ */
+export function piReadPath(readPath: string, offset?: number, limit?: number): string | null {
+	if (limit !== undefined && Math.floor(limit) <= 0) return null;
+	const start = offset !== undefined ? Math.max(1, Math.floor(offset)) : undefined;
+	const count = limit !== undefined ? Math.floor(limit) : undefined;
+	if (start === undefined && count === undefined) return readPath;
+	if (start === undefined) return `${readPath}:1+${count}`;
+	return count === undefined ? `${readPath}:${start}-` : `${readPath}:${start}+${count}`;
+}
+
+/**
+ * Join a Pi frame's optional `path` with the `glob`/`pattern` it scopes.
+ *
+ * The local `grep`/`glob` tools take one combined path spec. An absolute
+ * pattern ignores the path, and an absent or `.` path leaves the pattern
+ * standing alone rather than building a `./`- or `//`-prefixed spec.
+ *
+ * Uses `node:path` rather than string surgery so Windows absolutes (`C:\…`,
+ * UNC) are recognised and separators stay normalized.
+ */
+export function piJoinPath(basePath: string | undefined, pattern: string): string {
+	if (path.isAbsolute(pattern)) return pattern;
+	if (!basePath || basePath === ".") return pattern;
+	return path.join(basePath, pattern);
+}
+
+/**
+ * The path a `pi_ls` frame lists.
+ *
+ * The frame's `limit` is deliberately NOT mapped. It caps directory *entries*
+ * (the reference does a flat `readdir` and slices the entry array), while the
+ * local `read` tool renders a depth-2 tree with per-directory caps and elision
+ * summaries and applies a selector as a *rendered line* slice. Nested rows,
+ * headers and "N more" lines all count toward that slice, so `:1+K` would cap
+ * a different unit while looking honored — worse than leaving it unset, which
+ * at least reports the local listing's own truncation faithfully.
+ */
+export function piLsPath(basePath: string | undefined): string {
+	return basePath || ".";
+}
+
+/** Escape a literal string so the regex-only local `grep` tool matches it verbatim. */
+export function piEscapeRegexLiteral(value: string): string {
+	return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+}
+
+/** Clamp a present `optional int32` result cap the way the reference does; `undefined` stays unset. */
+export function piLimit(limit: number | undefined): number | undefined {
+	return limit === undefined ? undefined : Math.max(1, Math.floor(limit));
+}
@@ -10,7 +10,6 @@
 * the server reads as "the tool ran and produced nothing".
 */

-import * as path from "node:path";
 import { create } from "@bufbuild/protobuf";
 import {
 	AfterAgentResponseRequestResponseSchema,
@@ -67,76 +66,18 @@ import {
 import type { ToolResultMessage } from "../../types";

 /**
- * Translate a Pi frame's args into the local tool kwargs that run it.
- *
- * Shared deliberately: the provider synthesizes a display block from these and
- * the coding-agent bridge executes with them. Two hand-rolled translations of
- * one frame drift, and the drift is invisible — the transcript shows one
- * operation while a different one runs.
- *
- * Every `optional int32` here is presence-sensitive: `0` is a supplied value,
- * not "unset", so it must never be folded into a default.
+ * The pure arg translation lives in `../cursor-pi-args` so the legacy pi shim
+ * can share it without pulling this module's protobuf graph into the bundled
+ * virtual registry. Re-exported here because this is where the frame builders
+ * and their translation are consumed together.
 */
-
-/**
- * A `pi_read` range composed onto the path as `read`'s inline `:N+K` selector.
- *
- * `read` exposes no range kwargs, so an uncomposed range reads the whole file.
- * `offset` is a 1-indexed start clamped like the reference's
- * `Math.max(0, offset - 1)` over 0-indexed lines; `limit` is a line count.
- * `null` marks a present `limit: 0` — zero lines, which no selector expresses
- * and which must not degrade into a whole-file read.
- */
-export function piReadPath(path: string, offset?: number, limit?: number): string | null {
-	if (limit !== undefined && Math.floor(limit) <= 0) return null;
-	const start = offset !== undefined ? Math.max(1, Math.floor(offset)) : undefined;
-	const count = limit !== undefined ? Math.floor(limit) : undefined;
-	if (start === undefined && count === undefined) return path;
-	if (start === undefined) return `${path}:1+${count}`;
-	return count === undefined ? `${path}:${start}-` : `${path}:${start}+${count}`;
-}
-
-/**
- * Join a Pi frame's optional `path` with the `glob`/`pattern` it scopes.
- *
- * The local `grep`/`glob` tools take one combined path spec. An absolute
- * pattern ignores the path, and an absent or `.` path leaves the pattern
- * standing alone rather than building a `./`- or `//`-prefixed spec.
- *
- * Uses `node:path` rather than string surgery so Windows absolutes (`C:\…`,
- * UNC) are recognised and separators stay normalized. The legacy pi shim's
- * identical path/glob pair calls this too, so both stay in step.
- */
-export function piJoinPath(basePath: string | undefined, pattern: string): string {
-	if (path.isAbsolute(pattern)) return pattern;
-	if (!basePath || basePath === ".") return pattern;
-	return path.join(basePath, pattern);
-}
-
-/**
- * The path a `pi_ls` frame lists.
- *
- * The frame's `limit` is deliberately NOT mapped. It caps directory *entries*
- * (the reference does a flat `readdir` and slices the entry array), while the
- * local `read` tool renders a depth-2 tree with per-directory caps and elision
- * summaries and applies a selector as a *rendered line* slice. Nested rows,
- * headers and "N more" lines all count toward that slice, so `:1+K` would cap
- * a different unit while looking honored — worse than leaving it unset, which
- * at least reports the local listing's own truncation faithfully.
- */
-export function piLsPath(basePath: string | undefined): string {
-	return basePath || ".";
-}
-
-/** Escape a literal string so the regex-only local `grep` tool matches it verbatim. */
-export function piEscapeRegexLiteral(value: string): string {
-	return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
-}
-
-/** Clamp a present `optional int32` result cap the way the reference does; `undefined` stays unset. */
-export function piLimit(limit: number | undefined): number | undefined {
-	return limit === undefined ? undefined : Math.max(1, Math.floor(limit));
-}
+export {
+	piEscapeRegexLiteral,
+	piJoinPath,
+	piLimit,
+	piLsPath,
+	piReadPath,
+} from "../cursor-pi-args";

 /** Flatten a tool result's content into the single `output` string the Pi frames carry. */
 export function piOutputText(toolResult: ToolResultMessage): string {
@@ -131,6 +131,9 @@
 - Fixed `/live` sideband WebSockets ignoring standard proxy environment variables and `NO_PROXY`, which left proxied sessions stuck while the rest of the Codex connection succeeded ([#6770](https://github.com/can1357/oh-my-pi/issues/6770)).
 - Fixed the bash tool's `kill` builtin rejecting numeric signals and multiple process operands, stopping after the first failed target, and defaulting to `SIGKILL` instead of the standard `SIGTERM`. Negative PID operands (process groups per `kill(2)`) and the `--` end-of-options marker are now handled instead of being misparsed as signals ([#6779](https://github.com/can1357/oh-my-pi/issues/6779)).
 - Fixed `learned.md` saves growing a blank line on every write (trailing-newline split artifact) and hoisting all headings/prose above all bullets, which re-scoped lessons under the wrong heading in hand-organized files. Saves are now byte-idempotent and preserve mixed Markdown ordering: non-list lines keep their positions, new lessons insert newest-first at the head of the first bullet run, and dedupe/cap operate on bullet lines in place.
+- 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`.

 ## [17.1.5] - 2026-07-27

@@ -0,0 +1,36 @@
+/**
+ * Per-call tools the Cursor exec bridge needs but the model-facing registry
+ * cannot supply.
+ *
+ * Both bridge callsites — the primary session and the advisor roster — build
+ * the same instances, and both must apply the session's approval wrapper. A
+ * raw tool here silently escapes the gate every registry call goes through, so
+ * the construction lives in one place rather than being repeated per callsite.
+ */
+
+import type { AgentTool } from "@oh-my-pi/pi-agent-core";
+import type { ExtensionRunner } from "./extensibility/extensions";
+import { ExtensionToolWrapper } from "./extensibility/extensions";
+import type { GrepToolOptions, Tool, ToolSession } from "./tools";
+import { GrepTool } from "./tools";
+
+/**
+ * Build the bridge's `createGrepTool` factory for one tool session.
+ *
+ * A `pi_grep` frame carries its own context width and total match cap. Neither
+ * is expressible in the model-facing `grep` schema — context comes from
+ * `grep.contextBefore`/`grep.contextAfter`, fixed when the shared instance is
+ * constructed — so honoring them needs a fresh tool per call.
+ *
+ * The result is wrapped exactly like a registry tool: the approval gate runs on
+ * every call site, and a per-call instance is no exception.
+ */
+export function createBridgeGrepFactory(
+	session: ToolSession,
+	extensionRunner: ExtensionRunner,
+): (options: GrepToolOptions) => AgentTool {
+	return options => {
+		const grepTool: Tool = new GrepTool(session, options);
+		return new ExtensionToolWrapper(grepTool, extensionRunner);
+	};
+}
@@ -71,6 +71,11 @@ interface CursorExecBridgeOptions {
 	 * is fixed to the session settings at construction — so without this the
 	 * two fields are silently dropped. Callers that cannot supply it keep the
 	 * shared instance and the session's defaults.
+	 *
+	 * The returned tool is executed as-is. Callers whose registry tools carry an
+	 * approval wrapper MUST apply the same wrapper here, or a frame supplying
+	 * either field silently escapes the approval gate that every other call
+	 * goes through.
 	 */
 	createGrepTool?(options: { context?: number; totalMatchLimit?: number }): CursorBridgeTool | undefined;
 }
@@ -399,14 +399,24 @@ export class EditTool implements AgentTool<TInput> {
 	readonly #editMode?: EditMode;
 	readonly #deferredDiagnostics: DeferredDiagnostics;

-	constructor(private readonly session: ToolSession) {
+	/**
+	 * `mode` pins the edit variant for this instance, for callers whose protocol
+	 * fixes the shape of an edit. The Cursor `pi_edit` frame carries
+	 * `old_text`/`new_text` pairs, which only `replace` accepts — under the
+	 * default `hashline` mode those args do not match the schema at all. Left
+	 * unset, the env/settings resolution applies as before.
+	 */
+	constructor(
+		private readonly session: ToolSession,
+		mode?: EditMode,
+	) {
 		const {
 			PI_EDIT_FUZZY: editFuzzy = "auto",
 			PI_EDIT_FUZZY_THRESHOLD: editFuzzyThreshold = "auto",
 			PI_EDIT_VARIANT: envEditVariant = "auto",
 		} = Bun.env;

-		this.#editMode = resolveConfiguredEditMode(envEditVariant);
+		this.#editMode = mode ?? resolveConfiguredEditMode(envEditVariant);
 		this.#allowFuzzy = resolveAllowFuzzy(session, editFuzzy);
 		this.#fuzzyThreshold = resolveFuzzyThreshold(session, editFuzzyThreshold);
 		const deduplicateDiagnostics =
@@ -17,7 +17,7 @@ import * as fs from "node:fs";
 import * as path from "node:path";
 import type { AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core";
 import { type AuthCredential, SqliteAuthCredentialStore, type TSchema } from "@oh-my-pi/pi-ai";
-import { piEscapeRegexLiteral, piJoinPath } from "@oh-my-pi/pi-ai/providers/cursor/exec-modern";
+import { piEscapeRegexLiteral, piJoinPath } from "@oh-my-pi/pi-ai/providers/cursor-pi-args";
 import { getKeybindings, type Keybinding, Text } from "@oh-my-pi/pi-tui";
 import {
 	getAgentDbPath,
@@ -62,6 +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 "./discovery";
 import { initializeWithSettings } from "./discovery";
 import { disposeAllJuliaKernelSessions, disposeJuliaKernelSessionsByOwner } from "./eval/jl/executor";
@@ -2584,7 +2585,21 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 		for (const tool of toolRegistry.values()) {
 			toolRegistry.set(tool.name, new ExtensionToolWrapper(tool, extensionRunner));
 		}
+		// Cursor's own client owns file edits, so `edit` is not advertised to the
+		// model (commit 8ba0498eb: full-file `write` is used instead). The exec
+		// bridge is a different consumer: the server sends native `pi_edit`
+		// frames regardless of the advertised catalog, and answering them needs
+		// a real tool.
+		//
+		// It must be a `replace`-mode instance. `PiEditExecArgs` carries
+		// `old_text`/`new_text` pairs, which is exactly `replace`'s schema and
+		// nothing else's — under the default `hashline` mode the frame's args do
+		// not match the tool's parameters at all. The registry instance follows
+		// the session's configured mode, so the bridge builds its own.
+		let cursorBridgeEditTool: AgentTool | undefined;
 		if (model?.provider === "cursor") {
+			const bridgeEdit: Tool = new EditTool(toolSession, "replace");
+			cursorBridgeEditTool = new ExtensionToolWrapper(bridgeEdit, extensionRunner);
 			toolRegistry.delete("edit");
 			builtInRegistryToolNames.delete("edit");
 		}
@@ -2622,6 +2637,9 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 		// execution-only ACP decorator used by `write xd://<tool>`; docs and
 		// renderer lookup continue to use the undecorated canonical instance.
 		const resolveDeviceTool = (name: string): AgentTool | undefined => {
+			// `edit` is withheld from the model-facing registry for Cursor but the
+			// native `pi_edit` frame still needs it; see the retention above.
+			if (name === "edit" && cursorBridgeEditTool) return cursorBridgeEditTool;
 			const state = toolSession.xdev;
 			if (!state) return undefined;
 			return resolveMountedXdevExecutable(state, name);
@@ -2637,7 +2655,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 			persistTodoPhases: phases => sessionManager.appendCustomEntry(USER_TODO_EDIT_CUSTOM_TYPE, { phases }),
 			// `pi_grep` carries its own context width and match cap, which the
 			// shared grep instance fixed at construction cannot express.
-			createGrepTool: grepOptions => new GrepTool(toolSession, grepOptions),
+			createGrepTool: createBridgeGrepFactory(toolSession, extensionRunner),
 		});

 		// Resolve the inline-descriptors setting against the session-start model.
@@ -3240,6 +3258,10 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 			providerPromptCacheKeySource,
 			parentEvalSessionId: options.parentEvalSessionId,
 			advisorTools,
+			// Same per-call `grep` seam the primary bridge gets, built against the
+			// advisor's own tool session so a `pi_grep` frame's context width and
+			// match cap are honored there too.
+			advisorCreateGrepTool: createBridgeGrepFactory(advisorToolSession, extensionRunner),
 			titleSystemPrompt: options.titleSystemPrompt,
 		});
 		hasSession = true;
@@ -209,6 +209,12 @@ export interface AgentSessionConfig {
 	providerPromptCacheKeySource?: "explicit" | "fork";
 	/** Full advisor toolset built against an advisor-scoped tool session. */
 	advisorTools?: AgentTool[];
+	/**
+	 * Build a `grep` honoring a Cursor `pi_grep` frame's own context width and
+	 * match cap, against the advisor-scoped tool session. Without it an advisor
+	 * running on Cursor silently drops both fields.
+	 */
+	advisorCreateGrepTool?(options: { context?: number; totalMatchLimit?: number }): AgentTool | undefined;
 	/** Preloaded watchdog prompt content for the advisor. */
 	advisorWatchdogPrompt?: string;
 	/** Shared advisor instructions loaded from WATCHDOG.yml. */
@@ -1290,6 +1290,7 @@ export class AgentSession {
 		this.#advisors = new SessionAdvisors(advisorsHost, {
 			enabled: this.settings.get("advisor.enabled"),
 			tools: config.advisorTools,
+			createGrepTool: config.advisorCreateGrepTool,
 			watchdogPrompt: config.advisorWatchdogPrompt,
 			sharedInstructions: config.advisorSharedInstructions,
 			contextPrompt: config.advisorContextPrompt,
@@ -177,6 +177,13 @@ interface AdvisorRuntimeDescriptor {
 export interface SessionAdvisorsOptions {
 	enabled: boolean;
 	tools?: AgentTool[];
+	/**
+	 * Build a `grep` honoring a Cursor `pi_grep` frame's own context width and
+	 * match cap. The advisor's tools are fixed instances carrying session
+	 * defaults, so without this an advisor running against Cursor silently
+	 * drops both fields — the same gap the primary bridge closes.
+	 */
+	createGrepTool?(options: { context?: number; totalMatchLimit?: number }): AgentTool | undefined;
 	watchdogPrompt?: string;
 	sharedInstructions?: string;
 	contextPrompt?: string;
@@ -250,6 +257,7 @@ export class SessionAdvisors {
 	readonly #host: SessionAdvisorsHost;
 	#advisorEnabled: boolean;
 	#advisorTools: AgentTool[] | undefined;
+	#advisorCreateGrepTool: SessionAdvisorsOptions["createGrepTool"];
 	#advisorWatchdogPrompt: string | undefined;
 	#advisorSharedInstructions: string | undefined;
 	#advisorContextPrompt: string | undefined;
@@ -272,6 +280,7 @@ export class SessionAdvisors {
 		this.#host = host;
 		this.#advisorEnabled = options.enabled;
 		this.#advisorTools = options.tools;
+		this.#advisorCreateGrepTool = options.createGrepTool;
 		this.#advisorWatchdogPrompt = options.watchdogPrompt;
 		this.#advisorSharedInstructions = options.sharedInstructions;
 		this.#advisorContextPrompt = options.contextPrompt;
@@ -723,6 +732,10 @@ export class SessionAdvisors {
 				getCwd: () => this.#host.sessionManager.getCwd(),
 				tools: advisorToolMap,
 				allowNativeDelete: advisorCanMutateFiles,
+				// Gated on the advisor's own grant: the factory builds a fresh
+				// tool, so handing it over unconditionally would give a roster
+				// without `grep` a search tool it was denied.
+				createGrepTool: advisorToolMap.has("grep") ? this.#advisorCreateGrepTool : undefined,
 			});
 			const baseAdvisorStreamFn = this.#advisorStreamFn ?? streamSimple;
 			const advisorStreamFn: StreamFn = (requestModel, context, options) =>
@@ -19,6 +19,8 @@ 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 { 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";
 import { GrepTool, type Tool, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
@@ -202,6 +204,94 @@ describe("pi_bash truncation reaches the wire from a real BashTool result", () =
 	});
 });

+describe("bridge tool resolution beyond the model-facing registry", () => {
+	let cwd: string;
+
+	beforeEach(async () => {
+		cwd = await fs.mkdtemp(path.join(os.tmpdir(), "cursor-bridge-resolve-"));
+	});
+
+	afterEach(async () => {
+		await removeWithRetries(cwd);
+	});
+
+	it("edits a real file from a pi_edit frame when `edit` is withheld from the model", async () => {
+		// For Cursor the session drops `edit` from the tool registry so the model
+		// is steered to full-file `write`. The native `pi_edit` frame arrives
+		// regardless of the advertised catalog, so the bridge must still reach a
+		// real edit tool through the `getTool` fallback — otherwise every modern
+		// 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");
+
+		const withheld = new CursorExecHandlers({
+			cwd,
+			tools: new Map<string, Tool>(),
+			getTool: name => (name === "edit" ? editTool : undefined),
+		});
+		const result = await withheld.piEdit({
+			toolCallId: "e1",
+			args: { path: target, edits: [{ oldText: "beta", newText: "gamma" }] },
+		} as never);
+
+		expect(result.isError).toBeFalsy();
+		expect(await Bun.file(target).text()).toBe("alpha\ngamma\n");
+	});
+
+	it("reports the failure instead of editing when no edit tool is reachable", async () => {
+		const target = path.join(cwd, "sample.txt");
+		await Bun.write(target, "alpha\nbeta\n");
+		const unreachable = new CursorExecHandlers({ cwd, tools: new Map<string, Tool>() });
+		const result = await unreachable.piEdit({
+			toolCallId: "e2",
+			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("wraps the per-call grep the real bridge factory builds", async () => {
+		// The reviewed bypass was in the factory the session hands the bridge,
+		// not in the bridge: a raw `new GrepTool(...)` there skips the approval
+		// gate every registry tool goes through. Exercise the shared factory
+		// 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 built = factory({ context: 0, totalMatchLimit: 5 });
+		expect(built).toBeInstanceOf(ExtensionToolWrapper);
+
+		const handlers = new CursorExecHandlers({
+			cwd,
+			tools: new Map<string, Tool>(),
+			createGrepTool: factory,
+		});
+		const result = await handlers.piGrep({
+			toolCallId: "g1",
+			args: { pattern: "needle", path: cwd, limit: 5 },
+		} as never);
+
+		// The wrapper ran (its extension hook fired) and the frame's cap still
+		// reached the underlying tool.
+		expect(intercepted).toEqual(["grep"]);
+		expect((result.details as { matchCount?: number } | undefined)?.matchCount).toBe(1);
+	});
+});
+
 describe("CursorExecHandlers error results", () => {
 	const rewrittenErrorTool = (name: string): AgentTool => ({
 		name,
@@ -1,4 +1,5 @@
 import { describe, expect, it } from "bun:test";
+import * as fs from "node:fs/promises";
 import * as path from "node:path";
 import * as url from "node:url";
 import { __buildLegacyPiPackageRootOverrides } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/legacy-pi-compat";
@@ -83,6 +84,60 @@ process.stdout.write(JSON.stringify([
 		expect(bundledModuleKeys.has("@oh-my-pi/pi-ai/oauth/openai-codex")).toBe(true);
 	});

+	it("actually loads the shim's shared Pi translation through the bundled registry", async () => {
+		// The legacy shim performs the same Pi arg translation as the modern
+		// bridge and imports the shared helpers rather than copying them. Those
+		// live in a single-segment `providers/` module on purpose: `./providers/*`
+		// cannot match a nested `providers/<dir>/<mod>` specifier, which would
+		// fall through to `Bun.resolveSync` and fail under bunfs (issue #3442).
+		//
+		// Executing the generated registry is the contract — a key present in the
+		// override map still proves nothing if the module cannot be imported.
+		const key = "@oh-my-pi/pi-ai/providers/cursor-pi-args";
+		const entry = (await collectBundledPiEntries()).find(candidate => candidate.key === key);
+		expect(entry).toBeDefined();
+
+		// The rendered registry imports by bare specifier, exactly as the real
+		// bundle does, so it must run somewhere those specifiers resolve — the
+		// package itself. A temp dir has no workspace links and would fail for
+		// a reason unrelated to the export map.
+		const packageRoot = path.join(path.dirname(url.fileURLToPath(import.meta.url)), "..", "..");
+		const registryPath = path.join(packageRoot, `.probe-legacy-pi-args-${Bun.randomUUIDv7()}.ts`);
+		await Bun.write(
+			registryPath,
+			`${__renderLegacyPiVirtualModule([entry!])}
+const mod = await BUNDLED_PI_MODULE_LOADERS[${JSON.stringify(key)}]();
+process.stdout.write(JSON.stringify([
+	mod.piEscapeRegexLiteral("a.b*c"),
+	mod.piJoinPath("src", "*.ts"),
+]));
+`,
+		);
+		let exitCode: number;
+		let stdout: string;
+		let stderr: string;
+		try {
+			const proc = Bun.spawn([process.execPath, registryPath], {
+				cwd: packageRoot,
+				stdout: "pipe",
+				stderr: "pipe",
+			});
+			[exitCode, stdout, stderr] = await Promise.all([
+				proc.exited,
+				new Response(proc.stdout).text(),
+				new Response(proc.stderr).text(),
+			]);
+		} finally {
+			await fs.rm(registryPath, { force: true });
+		}
+		expect(stderr).toBe("");
+		expect(exitCode).toBe(0);
+		expect(JSON.parse(stdout)).toEqual(["a\\.b\\*c", path.join("src", "*.ts")]);
+
+		const overrides = __buildLegacyPiPackageRootOverrides(true, bundledModuleKeys);
+		expect(overrides[key]).toBe(`omp-legacy-pi-bundled:${key}`);
+	});
+
 	it("expands web search provider wildcard exports for compiled plugin imports", () => {
 		const overrides = __buildLegacyPiPackageRootOverrides(true, bundledModuleKeys);
 		const providerKeys = [