From 9436fb56401b50a3bed4f91f2db6f7e59be6e7c4 Mon Sep 17 00:00:00 2001 From: Diogo Soares Rodrigues Date: Mon, 27 Jul 2026 09:49:34 -0300 Subject: [PATCH] feat(cursor): honor pi_grep's context and limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pi_grep` carries a context width and a total match cap. Neither is expressible in the model-facing `grep` schema — context comes from `grep.contextBefore`/`grep.contextAfter`, fixed when the shared tool is constructed — so both were dropped. `GrepTool` now takes them as constructor options. The model-facing schema is unchanged: this is a seam for wire bridges whose protocol supplies the values, mirroring `GlobTool`'s existing options bag. The bridge builds a per-call `grep` only for frames that supply them; everything else keeps the shared instance and session defaults. `pi_ls`'s `limit` stays unmapped, now deliberately and documented. It caps directory entries, while the local `read` renders a depth-2 tree and slices rendered lines — nested rows, headers and elision summaries all count — so `:1+K` would cap a different unit while looking honored. Verified against real files in a temp dir, not captured arguments: match counts and context lines are asserted from actual search output. Mutation-checked — ignoring either option, or dropping the scoped tool in the bridge, fails a test. (cherry picked from commit 299ded5a274427c2c2d5de27c00a2056a709581c) --- packages/ai/src/providers/cursor.ts | 3 +- .../ai/src/providers/cursor/exec-modern.ts | 15 +++++ packages/coding-agent/CHANGELOG.md | 2 + packages/coding-agent/src/cursor.ts | 65 +++++++++++++++---- packages/coding-agent/src/sdk.ts | 3 + packages/coding-agent/src/tools/grep.ts | 54 +++++++++++++-- .../coding-agent/test/cursor-exec.test.ts | 54 ++++++++++++++- 7 files changed, 175 insertions(+), 21 deletions(-) diff --git a/packages/ai/src/providers/cursor.ts b/packages/ai/src/providers/cursor.ts index 8e10cba54..2ef6452d5 100644 --- a/packages/ai/src/providers/cursor.ts +++ b/packages/ai/src/providers/cursor.ts @@ -195,6 +195,7 @@ import { piEscapeRegexLiteral, piJoinPath, piLimit, + piLsPath, piReadPath, } from "./cursor/exec-modern"; @@ -1671,7 +1672,7 @@ async function handleExecServerMessage( // Same mapping as the legacy `lsArgs` frame: the local `read` tool lists // directories, so the synthesized block must name `read` to match the // bridge's own `toolResult`. - synthesizeCursorExecToolCall(output, stream, state, toolCallId, "read", { path: args.path || "." }); + synthesizeCursorExecToolCall(output, stream, state, toolCallId, "read", { path: piLsPath(args.path) }); const { execResult } = await resolveExecHandler( { args, toolCallId }, execHandlers?.piLs?.bind(execHandlers), diff --git a/packages/ai/src/providers/cursor/exec-modern.ts b/packages/ai/src/providers/cursor/exec-modern.ts index d61876c41..d5c559fef 100644 --- a/packages/ai/src/providers/cursor/exec-modern.ts +++ b/packages/ai/src/providers/cursor/exec-modern.ts @@ -113,6 +113,21 @@ export function piJoinPath(basePath: string | undefined, pattern: string): strin 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, "\\$&"); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 73021a173..533faaacf 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -168,6 +168,8 @@ ### Fixed - Fixed the Cursor Pi exec bridge silently dropping frame arguments. `pi_read`'s `offset`/`limit` were ignored, so a ranged read returned the whole file; `pi_grep`'s `literal` was ignored, so a fixed-string search ran as a regex and matched the wrong lines; and the path/glob join produced a `./`-prefixed spec. Ranges are now composed onto `read`'s `:N+K` inline selector, literal patterns are escaped, and the join uses `node:path`. These are `optional int32` fields, so a present `0` is honored rather than folded into a default: `pi_read` with `limit: 0` answers with empty output instead of the entire file, and `pi_find` with `limit: 0` clamps to 1 the way the reference client does. +- `pi_grep`'s `context` and `limit` are honored. Neither is expressible in the model-facing `grep` schema — context width comes from `grep.contextBefore`/`grep.contextAfter` fixed at tool construction — so the bridge builds a per-call `grep` for frames that supply them. `GrepTool` accepts these as constructor options; the model-facing schema is unchanged, and a frame that supplies neither keeps the shared instance and the session's defaults. +- `pi_ls`'s `limit` is still not mapped, now deliberately: it caps directory *entries*, while the local `read` tool renders a depth-2 tree with per-directory caps and elision rows and applies a selector as a *rendered line* slice. Mapping it to `:1+K` would cap a different unit while appearing honored. ## [17.1.4] - 2026-07-26 diff --git a/packages/coding-agent/src/cursor.ts b/packages/coding-agent/src/cursor.ts index 39ae87cba..8dd8b6096 100644 --- a/packages/coding-agent/src/cursor.ts +++ b/packages/coding-agent/src/cursor.ts @@ -14,7 +14,13 @@ import type { CursorExecHandlers as ICursorExecHandlers, ToolResultMessage, } from "@oh-my-pi/pi-ai"; -import { piEscapeRegexLiteral, piJoinPath, piLimit, piReadPath } from "@oh-my-pi/pi-ai/providers/cursor/exec-modern"; +import { + piEscapeRegexLiteral, + piJoinPath, + piLimit, + piLsPath, + piReadPath, +} from "@oh-my-pi/pi-ai/providers/cursor/exec-modern"; import { sanitizeText } from "@oh-my-pi/pi-utils"; import { resolveToCwd } from "./tools/path-utils"; import type { TodoPhase, TodoStatus } from "./tools/todo"; @@ -22,6 +28,13 @@ import type { TodoPhase, TodoStatus } from "./tools/todo"; /** Phase used for Cursor-owned tasks with no local phase grouping. */ const CURSOR_TODO_PHASE = "Tasks"; +/** + * A tool instance the bridge can run, matching the erased shape the session's + * tool registry stores. The concrete tools have narrower `execute` parameter + * types than the default `AgentTool`, which only unify through this alias. + */ +type CursorBridgeTool = AgentTool; + interface CursorExecBridgeOptions { cwd: string; getCwd?: () => string; @@ -51,6 +64,15 @@ interface CursorExecBridgeOptions { * Cursor emits no local `todo` toolResult, so nothing else records it. */ persistTodoPhases?: (phases: TodoPhase[]) => void; + /** + * Build a `grep` tool honoring a frame's own context width and match cap. + * + * The modern `pi_grep` frame carries both, and the shared `grep` instance + * 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. + */ + createGrepTool?(options: { context?: number; totalMatchLimit?: number }): CursorBridgeTool | undefined; } function createToolResultMessage( @@ -82,8 +104,9 @@ async function executeTool( toolName: string, toolCallId: string, args: Record, + overrideTool?: CursorBridgeTool, ): Promise { - const tool = options.getExecutableTool?.(toolName) ?? options.tools.get(toolName); + const tool = overrideTool ?? options.getExecutableTool?.(toolName) ?? options.tools.get(toolName); if (!tool) { const result = buildToolErrorResult(`Tool "${toolName}" not available`); return createToolResultMessage(toolCallId, toolName, result, true); @@ -460,20 +483,35 @@ export class CursorExecHandlers implements ICursorExecHandlers { /** * `literal` makes the pattern a fixed string; the local tool is regex-only, * so the pattern is escaped on the way in (same translation the legacy pi - * shim does). `context` and `limit` have no tool-level equivalent — context - * width comes from `grep.contextBefore`/`grep.contextAfter` settings and the - * result count is bounded by the tool's own file/match caps. + * shim does). + * + * `context` and `limit` are not expressible in the model-facing schema — + * context width comes from settings fixed at tool construction — so the + * frame's values are honored by building a per-call `grep` through + * {@link CursorExecBridgeOptions.createGrepTool}. Both are `optional int32`, + * so a present `0` context means "no context lines", not "use the default". + * Without the factory the shared instance runs with session defaults. */ async piGrep(call: Parameters>[0]) { - const { pattern, path, glob, ignoreCase, literal } = call.args; + const { pattern, path, glob, ignoreCase, literal, context, limit } = call.args; + const scoped = + context !== undefined || limit !== undefined + ? this.options.createGrepTool?.({ context, totalMatchLimit: piLimit(limit) }) + : undefined; // Same arg mapping as the legacy `grep` handler: the local tool takes one // path spec, and its `case` flag is case-SENSITIVITY, the inverse of the // frame's `ignore_case`. - return await executeTool(this.options, "grep", call.toolCallId, { - pattern: literal === true ? piEscapeRegexLiteral(pattern) : pattern, - path: glob ? piJoinPath(path, glob) : path || ".", - case: ignoreCase === true ? false : undefined, - }); + return await executeTool( + this.options, + "grep", + call.toolCallId, + { + pattern: literal === true ? piEscapeRegexLiteral(pattern) : pattern, + path: glob ? piJoinPath(path, glob) : path || ".", + case: ignoreCase === true ? false : undefined, + }, + scoped, + ); } /** @@ -495,11 +533,10 @@ export class CursorExecHandlers implements ICursorExecHandlers { /** * Redirected to `read`, which lists directories — same as the legacy `ls`. - * `limit` has no tool-level equivalent; the listing is bounded by `read`'s - * own directory-entry caps. + * The frame's entry `limit` is not mapped; see {@link piLsPath}. */ async piLs(call: Parameters>[0]) { - return await executeTool(this.options, "read", call.toolCallId, { path: call.args.path || "." }); + return await executeTool(this.options, "read", call.toolCallId, { path: piLsPath(call.args.path) }); } /** diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 49a17ddd6..fe093e07e 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -2635,6 +2635,9 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} getTodoPhases: () => session.getTodoPhases(), setTodoPhases: phases => session.setTodoPhases(phases), 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), }); // Resolve the inline-descriptors setting against the session-start model. diff --git a/packages/coding-agent/src/tools/grep.ts b/packages/coding-agent/src/tools/grep.ts index e28756a14..63377292b 100644 --- a/packages/coding-agent/src/tools/grep.ts +++ b/packages/coding-agent/src/tools/grep.ts @@ -884,6 +884,22 @@ export interface GrepToolDetails { type SearchParams = typeof searchSchema.infer; +/** + * Construction-time overrides for callers that are not the model. + * + * The model-facing schema deliberately does not grow these: they exist for + * wire bridges (the Cursor `pi_grep` frame) whose protocol carries an explicit + * context width and total match cap, and which would otherwise have to drop + * them. Unset means "use the session settings / built-in caps" — the behavior + * every model-issued call keeps. + */ +export interface GrepToolOptions { + /** Overrides `grep.contextBefore`/`grep.contextAfter` for every call on this instance. */ + context?: number; + /** Caps total surfaced matches. Applied on top of the built-in per-file and file-window caps, never above them. */ + totalMatchLimit?: number; +} + export class GrepTool implements AgentTool { readonly name = "grep"; readonly approval = (args: unknown): ToolTier => { @@ -897,7 +913,17 @@ export class GrepTool implements AgentTool readonly parameters = searchSchema; readonly strict = true; - constructor(private readonly session: ToolSession) { + readonly #contextOverride?: number; + readonly #totalMatchLimit?: number; + + constructor( + private readonly session: ToolSession, + options?: GrepToolOptions, + ) { + const context = options?.context; + this.#contextOverride = context !== undefined ? Math.max(0, Math.floor(context)) : undefined; + const total = options?.totalMatchLimit; + this.#totalMatchLimit = total !== undefined ? Math.max(1, Math.floor(total)) : undefined; const displayMode = resolveFileDisplayMode(session); this.description = prompt.render(grepDescription, { IS_HL_MODE: displayMode.hashLines, @@ -978,8 +1004,8 @@ export class GrepTool implements AgentTool `or pass a UTF-8 text member.`, ); } - const normalizedContextBefore = this.session.settings.get("grep.contextBefore"); - const normalizedContextAfter = this.session.settings.get("grep.contextAfter"); + const normalizedContextBefore = this.#contextOverride ?? this.session.settings.get("grep.contextBefore"); + const normalizedContextAfter = this.#contextOverride ?? this.session.settings.get("grep.contextAfter"); const ignoreCase = !(caseSensitive ?? true); const useGitignore = gitignore ?? true; const patternHasNewline = normalizedPattern.includes("\n") || normalizedPattern.includes("\\n"); @@ -1300,6 +1326,7 @@ export class GrepTool implements AgentTool const windowFiles = canPaginate ? fileOrder.slice(skipFiles, skipFiles + DEFAULT_FILE_LIMIT) : fileOrder; const fileLimitReached = canPaginate && totalFiles > skipFiles + DEFAULT_FILE_LIMIT; const selectedMatches: GrepMatch[] = []; + let totalMatchLimitReached = false; if (windowFiles.length > 0) { const lists = windowFiles.map(file => matchesByPath.get(file) ?? []); const cursors = new Array(lists.length).fill(0); @@ -1313,6 +1340,14 @@ export class GrepTool implements AgentTool } } } + // Round-robin above interleaves files for diversity, so the cap is + // applied after selection rather than as a per-list bound: trimming + // mid-rotation would silently favour whichever files sort first. + const cap = this.#totalMatchLimit; + if (cap !== undefined && selectedMatches.length > cap) { + selectedMatches.length = cap; + totalMatchLimitReached = true; + } } const nextSkip = skipFiles + windowFiles.length; const limitMessage = fileLimitReached @@ -1503,7 +1538,12 @@ export class GrepTool implements AgentTool const output = truncation.content; const displayText = displayLines.join("\n"); const truncated = Boolean( - fileLimitReached || perFileLimitReached || result.limitReached || truncation.truncated || linesTruncated, + fileLimitReached || + perFileLimitReached || + totalMatchLimitReached || + result.limitReached || + truncation.truncated || + linesTruncated, ); const details: GrepToolDetails = { scopePath, @@ -1518,7 +1558,11 @@ export class GrepTool implements AgentTool })), truncated, fileLimitReached: fileLimitReached ? DEFAULT_FILE_LIMIT : undefined, - perFileLimitReached: perFileLimitReached ? perFileMatchCap : undefined, + perFileLimitReached: totalMatchLimitReached + ? this.#totalMatchLimit + : perFileLimitReached + ? perFileMatchCap + : undefined, displayContent: displayText, missingPaths: missingPaths.length > 0 ? missingPaths : undefined, }; diff --git a/packages/coding-agent/test/cursor-exec.test.ts b/packages/coding-agent/test/cursor-exec.test.ts index 60454c23c..a5b248852 100644 --- a/packages/coding-agent/test/cursor-exec.test.ts +++ b/packages/coding-agent/test/cursor-exec.test.ts @@ -20,7 +20,7 @@ import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { CursorExecHandlers } from "@oh-my-pi/pi-coding-agent/cursor"; 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 ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; +import { GrepTool, type Tool, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { removeWithRetries } from "@oh-my-pi/pi-utils"; import { type } from "arktype"; import { AdviseTool } from "../src/advisor/advise-tool"; @@ -82,6 +82,58 @@ describe("CursorExecHandlers.grep bridge", () => { } as any); expect((sensitiveResult.details as { matchCount?: number } | undefined)?.matchCount).toBe(1); }); + + it("honors pi_grep's requested match limit against real files", async () => { + // The frame's `limit` caps total surfaced matches. The model-facing schema + // has no such parameter, so without a per-call tool the cap is dropped and + // the search returns everything it found. + await Bun.write(path.join(cwd, "many.txt"), Array.from({ length: 10 }, (_, i) => `needle ${i}`).join("\n")); + const scopedHandlers = new CursorExecHandlers({ + cwd, + tools: new Map([["grep", searchTool]]), + createGrepTool: options => new GrepTool(createTestSession(cwd), options), + }); + + const capped = await scopedHandlers.piGrep({ + toolCallId: "c1", + args: { pattern: "needle", path: cwd, limit: 3 }, + } as never); + expect((capped.details as { matchCount?: number } | undefined)?.matchCount).toBe(3); + + const uncapped = await scopedHandlers.piGrep({ + toolCallId: "c2", + args: { pattern: "needle", path: cwd }, + } as never); + expect((uncapped.details as { matchCount?: number } | undefined)?.matchCount).toBe(10); + }); + + it("honors pi_grep's requested context width against real files", async () => { + // `context` has no schema parameter either: the width is read from + // settings fixed at tool construction, so the frame's value only lands + // through a per-call instance. + await Bun.write(path.join(cwd, "ctx.txt"), "before line\nneedle here\nafter line\n"); + const scopedHandlers = new CursorExecHandlers({ + cwd, + tools: new Map([["grep", searchTool]]), + createGrepTool: options => new GrepTool(createTestSession(cwd), options), + }); + + const noContext = await scopedHandlers.piGrep({ + toolCallId: "c1", + args: { pattern: "needle here", path: path.join(cwd, "ctx.txt"), context: 0 }, + } as never); + const noContextText = noContext.content.map(c => (c.type === "text" ? c.text : "")).join(""); + expect(noContextText).not.toContain("before line"); + expect(noContextText).not.toContain("after line"); + + const withContext = await scopedHandlers.piGrep({ + toolCallId: "c2", + args: { pattern: "needle here", path: path.join(cwd, "ctx.txt"), context: 1 }, + } as never); + const withContextText = withContext.content.map(c => (c.type === "text" ? c.text : "")).join(""); + expect(withContextText).toContain("before line"); + expect(withContextText).toContain("after line"); + }); }); describe("CursorExecHandlers error results", () => {