diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index f9c3ddf66..bcd887980 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -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//` 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 diff --git a/packages/ai/src/providers/cursor-pi-args.ts b/packages/ai/src/providers/cursor-pi-args.ts new file mode 100644 index 000000000..09f5b3750 --- /dev/null +++ b/packages/ai/src/providers/cursor-pi-args.ts @@ -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)); +} diff --git a/packages/ai/src/providers/cursor/exec-modern.ts b/packages/ai/src/providers/cursor/exec-modern.ts index 3d2d2f2c4..a98c8d6fb 100644 --- a/packages/ai/src/providers/cursor/exec-modern.ts +++ b/packages/ai/src/providers/cursor/exec-modern.ts @@ -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 { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index dbf992d34..c85892057 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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 diff --git a/packages/coding-agent/src/cursor-bridge-tools.ts b/packages/coding-agent/src/cursor-bridge-tools.ts new file mode 100644 index 000000000..8ae54f17a --- /dev/null +++ b/packages/coding-agent/src/cursor-bridge-tools.ts @@ -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); + }; +} diff --git a/packages/coding-agent/src/cursor.ts b/packages/coding-agent/src/cursor.ts index 8dd8b6096..b7599f68f 100644 --- a/packages/coding-agent/src/cursor.ts +++ b/packages/coding-agent/src/cursor.ts @@ -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; } diff --git a/packages/coding-agent/src/edit/index.ts b/packages/coding-agent/src/edit/index.ts index 33a28c0b7..1320a35a4 100644 --- a/packages/coding-agent/src/edit/index.ts +++ b/packages/coding-agent/src/edit/index.ts @@ -399,14 +399,24 @@ export class EditTool implements AgentTool { 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 = diff --git a/packages/coding-agent/src/extensibility/legacy-pi-coding-agent-shim.ts b/packages/coding-agent/src/extensibility/legacy-pi-coding-agent-shim.ts index b9c5c97b5..36e755eab 100644 --- a/packages/coding-agent/src/extensibility/legacy-pi-coding-agent-shim.ts +++ b/packages/coding-agent/src/extensibility/legacy-pi-coding-agent-shim.ts @@ -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, diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index fe093e07e..b56291cb3 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -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://`; 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; diff --git a/packages/coding-agent/src/session/agent-session-types.ts b/packages/coding-agent/src/session/agent-session-types.ts index 75828db4f..1cd89daaa 100644 --- a/packages/coding-agent/src/session/agent-session-types.ts +++ b/packages/coding-agent/src/session/agent-session-types.ts @@ -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. */ diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 1b4191eb7..5544aa967 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -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, diff --git a/packages/coding-agent/src/session/session-advisors.ts b/packages/coding-agent/src/session/session-advisors.ts index b18d7db08..17f8e394a 100644 --- a/packages/coding-agent/src/session/session-advisors.ts +++ b/packages/coding-agent/src/session/session-advisors.ts @@ -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) => diff --git a/packages/coding-agent/test/cursor-exec.test.ts b/packages/coding-agent/test/cursor-exec.test.ts index 8c5ca1362..3c0a3df6c 100644 --- a/packages/coding-agent/test/cursor-exec.test.ts +++ b/packages/coding-agent/test/cursor-exec.test.ts @@ -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(), + 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() }); + 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(), + 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, diff --git a/packages/coding-agent/test/extensibility/legacy-pi-bundled-subpath-overrides.test.ts b/packages/coding-agent/test/extensibility/legacy-pi-bundled-subpath-overrides.test.ts index 6d2919361..bd74df6e3 100644 --- a/packages/coding-agent/test/extensibility/legacy-pi-bundled-subpath-overrides.test.ts +++ b/packages/coding-agent/test/extensibility/legacy-pi-bundled-subpath-overrides.test.ts @@ -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//` 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 = [