From e4d5169da0423c8149d9b0061de23822b1bd49fc Mon Sep 17 00:00:00 2001 From: t Date: Thu, 3 Sep 2026 09:32:04 +0800 Subject: [PATCH] feat: Grep and Glob results render as clickable locations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DSH round specified a fourth render intent — locations — and deferred it: deriving entries by parsing the formatted result text is unsound, because the ':' separator is ambiguous the moment a path contains one. That is the same reason the withhold filter runs on parsed rows. So the entries are attached at the source instead. Grep and Glob already hold exact rows (Grep from ripgrep's --null output) and now publish them as ToolResult.data.locations — absolute path for opening, the printed form for display, line and matched text where the mode knows them, capped at MAX_TOOL_LOCATIONS. The intent stays a pure function of the call's arguments; the entries ride the result's existing data channel, which the server has always serialized verbatim (the protocol type now says so instead of narrowing it away). The desktop reads them through one shared validating extractor (readToolLocations) and renders a search's card as one openable row per hit, wired to the same file-panel opener the card header already uses. A session restored from its log has only the text the model saw, so replayed cards degrade to today's plain-text body — same characters, minus the click. Grep's head_limit now slices rows before formatting (output unchanged byte for byte) so entries line up with shown lines, and the contract tests pin that withheld paths never leak through the structured channel. Verified in the vite preview harness: line-numbered Grep rows with match previews, Glob's relative rows, clicks reporting absolute paths, and the restored-session card falling back to plain text. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 11 +++ apps/desktop/src/components/ToolBody.tsx | 51 +++++++++++++- apps/desktop/src/components/ToolCard.tsx | 7 +- apps/desktop/src/index.css | 25 +++++++ apps/desktop/src/lib/repl-stream.test.ts | 18 +++++ apps/desktop/src/lib/repl-stream.ts | 15 +++- apps/desktop/src/preview-toolcards.tsx | 56 +++++++++++++-- apps/desktop/src/screens/Repl.tsx | 14 +++- packages/core/src/index.ts | 3 + packages/core/src/tools/glob.test.ts | 18 +++++ packages/core/src/tools/glob.ts | 16 ++++- packages/core/src/tools/grep.test.ts | 65 ++++++++++++++++++ packages/core/src/tools/grep.ts | 43 +++++++++--- packages/core/src/tools/index.ts | 3 + packages/core/src/tools/presentation.test.ts | 55 +++++++++++++++ packages/core/src/tools/presentation.ts | 72 +++++++++++++++++++- packages/protocol/src/types.ts | 9 ++- 17 files changed, 457 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4567472..dcf056d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### ✨ Added +- **Grep and Glob results in the desktop are now clickable locations.** Both + tools attach the places they found as structured data — resolved from their + own parsed rows, where a path containing `:` is still unambiguous — and the + desktop renders a search's card as a list of openable paths (with line and + matched text in content mode) instead of a grey text blob. Clicking one opens + it in the file panel. This is the `locations` render intent the DSH round + specified and deferred: re-deriving entries by parsing the formatted result + text was the rejected design, and attaching them at the source is what makes + the entries exact. A session restored from its log has only the text the + model saw, so replayed cards degrade to today's plain-text body. + - **Persistent shells now last a whole CLI session, and `/shells` shows them.** The registry landed in #273 owned by a single `runAgent` call, which meant a shell opened in one turn was gone by the next — a slower `Bash` with extra diff --git a/apps/desktop/src/components/ToolBody.tsx b/apps/desktop/src/components/ToolBody.tsx index 798853a..1e25045 100644 --- a/apps/desktop/src/components/ToolBody.tsx +++ b/apps/desktop/src/components/ToolBody.tsx @@ -7,7 +7,7 @@ import type { JSX } from 'react'; import { computeLineDiff } from '../lib/diff.js'; -import type { ToolPresentation } from '@deepcode/core/dist/tools/presentation.js'; +import type { ToolLocation, ToolPresentation } from '@deepcode/core/dist/tools/presentation.js'; /** How much of a tool's text output a card shows before cutting it off. */ const MAX_BODY_CHARS = 1500; @@ -42,6 +42,44 @@ function DiffBody({ before, after }: { before: string; after: string }): JSX.Ele ); } +/** + * The places a search found, each one openable. + * + * Entries come from the tool's structured result data, resolved by the tool + * itself from its own parsed rows — never re-parsed out of the formatted text, + * whose `:` separator is ambiguous when a path contains one. Without an + * `onOpenFile` the list still renders; the rows are just not buttons. + */ +function LocationsBody({ + locations, + onOpenFile, +}: { + locations: ToolLocation[]; + onOpenFile?: (path: string) => void; +}): JSX.Element { + return ( + <> + {locations.map((loc, i) => { + const label = `${loc.display ?? loc.path}${loc.line !== undefined ? `:${loc.line}` : ''}`; + return ( +
+ {onOpenFile ? ( + + ) : ( + {label} + )} + {loc.preview !== undefined && loc.preview !== '' && ( + {loc.preview} + )} +
+ ); + })} + + ); +} + /** A command and what it printed, styled as a shell transcript. */ function TerminalBody({ command, output }: { command: string; output?: string }): JSX.Element { return ( @@ -59,14 +97,22 @@ function TerminalBody({ command, output }: { command: string; output?: string }) * * @param presentation What core derived from the call's arguments. * @param resultText The tool's output, once it has any. + * @param locations Openable entries from the result's structured data. A call + * whose intent is `locations` but that has none — still running, restored + * from a session log, or genuinely empty — falls back to the text body. + * @param onOpenFile Opens a path in the file panel, when the host offers one. * @returns The body, or null when there is nothing to show yet. */ export function ToolBody({ presentation, resultText, + locations, + onOpenFile, }: { presentation: ToolPresentation; resultText?: string; + locations?: ToolLocation[]; + onOpenFile?: (path: string) => void; }): JSX.Element | null { if (presentation.kind === 'diff' && presentation.diff) { return ; @@ -74,5 +120,8 @@ export function ToolBody({ if (presentation.kind === 'terminal' && presentation.command !== undefined) { return ; } + if (presentation.kind === 'locations' && locations && locations.length > 0) { + return ; + } return resultText ? <>{clip(resultText)} : null; } diff --git a/apps/desktop/src/components/ToolCard.tsx b/apps/desktop/src/components/ToolCard.tsx index b923727..3b32258 100644 --- a/apps/desktop/src/components/ToolCard.tsx +++ b/apps/desktop/src/components/ToolCard.tsx @@ -22,10 +22,11 @@ interface ToolCardProps { body?: ReactNode; /** * How the body is laid out. `diff` and `terminal` preserve columns strictly; - * `generic` wraps. Chosen from the tool's own declared render intent — see - * core's `tools/presentation.ts`. + * `locations` is a one-entry-per-line list of openable paths; `generic` + * wraps. Chosen from the tool's own declared render intent — see core's + * `tools/presentation.ts`. */ - layout?: 'generic' | 'diff' | 'terminal'; + layout?: 'generic' | 'diff' | 'terminal' | 'locations'; /** * If set, the target becomes a clickable "open preview" affordance — used for * file tools (Read/Write/Edit) to load the file into the right-side panel. diff --git a/apps/desktop/src/index.css b/apps/desktop/src/index.css index 793a5fe..17a3b80 100644 --- a/apps/desktop/src/index.css +++ b/apps/desktop/src/index.css @@ -1000,6 +1000,31 @@ select { border-top: 1px solid var(--border); padding-top: 6px; } +/* One found location per row: an openable path, then the matched text. */ +.tool-card .tc-body.locations { + white-space: pre; +} +.tool-card .tc-loc-row { + overflow: hidden; + text-overflow: ellipsis; +} +.tool-card button.tc-loc { + background: none; + border: none; + padding: 0; + font: inherit; + color: var(--brand); + cursor: pointer; +} +.tool-card button.tc-loc:hover { + text-decoration: underline; +} +.tool-card span.tc-loc { + color: var(--text-1); +} +.tool-card .tc-loc-preview { + color: var(--text-2); +} /* Inline approval — sits right under a tool card */ .approval-row { diff --git a/apps/desktop/src/lib/repl-stream.test.ts b/apps/desktop/src/lib/repl-stream.test.ts index ceb5d7d..4eade5c 100644 --- a/apps/desktop/src/lib/repl-stream.test.ts +++ b/apps/desktop/src/lib/repl-stream.test.ts @@ -72,6 +72,24 @@ describe('repl-stream mutators', () => { }); }); + it('keeps a result’s locations on the card, and omits the key when there are none', () => { + let m: Msg[] = []; + m = appendToolUse(m, tool('a', 'Grep')); + m = appendToolUse(m, tool('b', 'Read')); + m = attachToolResult(m, 'a', '/repo/x.ts:1:hit', 'ok', [ + { path: '/repo/x.ts', line: 1, preview: 'hit' }, + ]); + // A restored session replays results without structured data — the card + // must not grow a `locations: []` that reads as "searched, found nothing". + m = attachToolResult(m, 'b', 'file contents', 'ok', []); + const turn = m[0]!; + if (turn.role !== 'assistant') throw new Error('expected assistant'); + expect(turn.turn.tools[0]!.locations).toEqual([ + { path: '/repo/x.ts', line: 1, preview: 'hit' }, + ]); + expect('locations' in turn.turn.tools[1]!).toBe(false); + }); + it('falls back to only the newest running tool across resumed turns', () => { const m: Msg[] = [ { diff --git a/apps/desktop/src/lib/repl-stream.ts b/apps/desktop/src/lib/repl-stream.ts index 630cab4..7d3d8f3 100644 --- a/apps/desktop/src/lib/repl-stream.ts +++ b/apps/desktop/src/lib/repl-stream.ts @@ -12,6 +12,7 @@ // The card header's label comes from core, so the CLI and the extension read // the same answer rather than each keeping their own key list. import { pickTarget } from '@deepcode/core/dist/tools/presentation.js'; +import type { ToolLocation } from '@deepcode/core/dist/tools/presentation.js'; export { pickTarget }; @@ -22,6 +23,12 @@ export interface ToolInvocation { input: Record; status: 'running' | 'ok' | 'err'; resultText?: string; + /** + * Openable places the call found, from the result's structured data. Only + * live results carry them — a session restored from its log has just the + * text, and its cards degrade to the plain-text body. + */ + locations?: ToolLocation[]; } export interface AssistantTurn { @@ -123,6 +130,7 @@ export function attachToolResult( toolId: string, content: string, status: 'ok' | 'err', + locations?: ToolLocation[], ): Msg[] { let messageIndex = -1; let toolIndex = -1; @@ -153,7 +161,12 @@ export function attachToolResult( return msgs.map((message, index): Msg => { if (index !== messageIndex || message.role !== 'assistant') return message; const tools = [...message.turn.tools]; - tools[toolIndex] = { ...tools[toolIndex]!, status, resultText: content }; + tools[toolIndex] = { + ...tools[toolIndex]!, + status, + resultText: content, + ...(locations && locations.length > 0 ? { locations } : {}), + }; return { ...message, turn: { ...message.turn, tools } }; }); } diff --git a/apps/desktop/src/preview-toolcards.tsx b/apps/desktop/src/preview-toolcards.tsx index 0231752..4145daa 100644 --- a/apps/desktop/src/preview-toolcards.tsx +++ b/apps/desktop/src/preview-toolcards.tsx @@ -2,12 +2,13 @@ // build input is pinned to index.html, so this page exists only under // `vite dev` (served at /preview-toolcards.html) for visual iteration. // -// Renders one card per render intent so the three layouts can be compared side +// Renders one card per render intent so the four layouts can be compared side // by side without running an agent. import type { JSX } from 'react'; import { createRoot } from 'react-dom/client'; import { presentToolCall } from '@deepcode/core/dist/tools/presentation.js'; +import type { ToolLocation } from '@deepcode/core/dist/tools/presentation.js'; import { ToolBody } from './components/ToolBody.js'; import { ToolCard } from './components/ToolCard.js'; import './index.css'; @@ -16,6 +17,7 @@ const CALLS: Array<{ name: string; input: Record; result?: string; + locations?: ToolLocation[]; status: 'ok' | 'err' | 'running'; }> = [ { @@ -55,9 +57,48 @@ const CALLS: Array<{ }, { name: 'Grep', - input: { pattern: 'applySpillPolicy', path: 'packages/core/src' }, + input: { pattern: 'applySpillPolicy', path: 'packages/core/src', '-n': true }, result: - 'packages/core/src/agent.ts:16\npackages/core/src/spill/policy.ts:71\npackages/core/src/index.ts:213', + 'packages/core/src/agent.ts:16:import { applySpillPolicy } from...\npackages/core/src/spill/policy.ts:71:export function applySpillPolicy(\npackages/core/src/index.ts:213: applySpillPolicy,', + locations: [ + { + path: '/repo/packages/core/src/agent.ts', + display: 'packages/core/src/agent.ts', + line: 16, + preview: "import { applySpillPolicy } from './spill/policy.js';", + }, + { + path: '/repo/packages/core/src/spill/policy.ts', + display: 'packages/core/src/spill/policy.ts', + line: 71, + preview: 'export function applySpillPolicy(', + }, + { + path: '/repo/packages/core/src/index.ts', + display: 'packages/core/src/index.ts', + line: 213, + preview: ' applySpillPolicy,', + }, + ], + status: 'ok', + }, + { + name: 'Glob', + input: { pattern: 'src/sandbox/*.ts', path: 'packages/core' }, + result: 'src/sandbox/dns-proxy.ts\nsrc/sandbox/netns.ts\nsrc/sandbox/profiles.ts', + locations: [ + { path: '/repo/packages/core/src/sandbox/dns-proxy.ts', display: 'src/sandbox/dns-proxy.ts' }, + { path: '/repo/packages/core/src/sandbox/netns.ts', display: 'src/sandbox/netns.ts' }, + { path: '/repo/packages/core/src/sandbox/profiles.ts', display: 'src/sandbox/profiles.ts' }, + ], + status: 'ok', + }, + { + // A restored session has only the result text — the same call degrades to + // the generic body, no buttons. + name: 'Grep', + input: { pattern: 'applySpillPolicy', path: 'packages/core/src' }, + result: 'packages/core/src/agent.ts:16\npackages/core/src/spill/policy.ts:71', status: 'ok', }, { @@ -93,7 +134,14 @@ function Preview(): JSX.Element { ? '✓ done' : '✕ error', }} - body={} + body={ + console.log('open', path)} + /> + } /> ); diff --git a/apps/desktop/src/screens/Repl.tsx b/apps/desktop/src/screens/Repl.tsx index 73fc6cc..41b5bfe 100644 --- a/apps/desktop/src/screens/Repl.tsx +++ b/apps/desktop/src/screens/Repl.tsx @@ -43,7 +43,7 @@ import { } from '../lib/slash-commands.js'; import { ToolCard } from '../components/ToolCard.js'; import { ToolBody } from '../components/ToolBody.js'; -import { presentToolCall } from '@deepcode/core/dist/tools/presentation.js'; +import { presentToolCall, readToolLocations } from '@deepcode/core/dist/tools/presentation.js'; import { projectName } from '../lib/project.js'; import { useVoice } from '../lib/use-voice.js'; import { insertTranscript } from '../lib/voice.js'; @@ -197,7 +197,7 @@ interface AgentEvt { text?: string; name?: string; input?: Record; - result?: { content: string; isError?: boolean }; + result?: { content: string; isError?: boolean; data?: Record }; error?: string; stopReason?: string; // usage event — emitted per provider round-trip with that turn's token counts @@ -404,6 +404,7 @@ export function ReplScreen({ e.id ?? '', e.result?.content ?? '', e.result?.isError ? 'err' : 'ok', + readToolLocations(e.result?.data), ), ); break; @@ -1151,7 +1152,14 @@ function renderMessage( : '✕ error', }} layout={presentation.kind} - body={} + body={ + + } onOpen={ onOpenFile && typeof t.input?.file_path === 'string' ? () => onOpenFile(String(t.input.file_path)) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 19eb54b..ab1ac13 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -59,10 +59,13 @@ export { BUILTIN_TOOLS, presentToolCall, pickTarget, + readToolLocations, BUILTIN_RENDER_INTENTS, + MAX_TOOL_LOCATIONS, type ToolRenderKind, type ToolPresentation, type ToolDiffIntent, + type ToolLocation, type TodoItem, type TodoStatus, type SearchHit, diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index 9e3e756..e5f8a4b 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -39,6 +39,24 @@ describe('GlobTool', () => { expect(lines.length).toBeLessThanOrEqual(2); }); + it('attaches locations: absolute for opening, relative as displayed', async () => { + const r = await GlobTool.execute({ pattern: '**/*.ts', path: tmp }, { cwd: tmp }); + const locs = (r.data?.locations ?? []) as Array<{ path: string; display?: string }>; + expect(locs.length).toBe(3); + for (const loc of locs) { + expect(loc.path.startsWith(tmp)).toBe(true); + expect(loc.display).toBe(loc.path.slice(tmp.length + 1)); + } + // Entries line up with the listing, in the same (mtime) order. + const listed = (r.content as string).split('\n').filter(Boolean); + expect(locs.map((l) => l.display)).toEqual(listed); + }); + + it('slices locations with limit, so the marker line has no entry', async () => { + const r = await GlobTool.execute({ pattern: '**/*.ts', path: tmp, limit: 1 }, { cwd: tmp }); + expect((r.data?.locations as unknown[]).length).toBe(1); + }); + it('returns (no matches) cleanly', async () => { const r = await GlobTool.execute({ pattern: '**/*.xyz', path: tmp }, { cwd: tmp }); expect(r.isError).toBeFalsy(); diff --git a/packages/core/src/tools/glob.ts b/packages/core/src/tools/glob.ts index 9b996ef..fe97284 100644 --- a/packages/core/src/tools/glob.ts +++ b/packages/core/src/tools/glob.ts @@ -4,6 +4,7 @@ import { glob } from 'node:fs/promises'; import { isAbsolute, relative, resolve } from 'node:path'; import { withheldNotice, withholdDeniedReads } from '../config/contract-dispatch.js'; +import { MAX_TOOL_LOCATIONS, type ToolLocation } from './presentation.js'; import type { ToolContext, ToolHandler, ToolResult } from '../types.js'; interface GlobInput { @@ -18,6 +19,7 @@ export const GlobTool: ToolHandler = { name: 'Glob', definition: { name: 'Glob', + render: 'locations', description: 'Finds files matching a glob pattern (e.g. "src/**/*.ts"). Returns paths sorted by mtime (most recent first).', inputSchema: { @@ -86,9 +88,21 @@ export const GlobTool: ToolHandler = { const notice = withheldNotice(withheld); if (notice) lines.push(notice); + // Structured entries for clients that render results as openable locations: + // the absolute path for opening, plus the relative form the text printed. + const locations: ToolLocation[] = top.slice(0, MAX_TOOL_LOCATIONS).map((s) => ({ + path: s.p, + display: relative(ctx.cwd, s.p) || s.p, + })); + return { content: lines.join('\n') || '(no matches)', - data: { count: top.length, total: stamped.length, ...(withheld > 0 ? { withheld } : {}) }, + data: { + count: top.length, + total: stamped.length, + locations, + ...(withheld > 0 ? { withheld } : {}), + }, }; }, }; diff --git a/packages/core/src/tools/grep.test.ts b/packages/core/src/tools/grep.test.ts index ece8710..37299c7 100644 --- a/packages/core/src/tools/grep.test.ts +++ b/packages/core/src/tools/grep.test.ts @@ -149,6 +149,68 @@ describe('GrepTool', async () => { expect(r.data?.mode).toBe('files_with_matches'); }); + // Locations are built from the parsed rows, never re-parsed out of the + // formatted text — the `:` separator there is ambiguous when a path contains + // one, which is the same reason the withhold filter runs on rows. + describe('locations', () => { + const locs = (r: { data?: Record }) => + (r.data?.locations ?? []) as Array<{ + path: string; + line?: number; + preview?: string; + }>; + + it.skipIf(skipReason)('carries path, line and preview in content mode with -n', async () => { + const r = await GrepTool.execute( + { pattern: 'verifyToken', path: tmp, type: 'ts', '-n': true }, + { cwd: tmp }, + ); + const sorted = locs(r).sort((a, b) => a.path.localeCompare(b.path)); + expect(sorted).toEqual([ + { path: join(tmp, 'a.ts'), line: 1, preview: 'function verifyToken() {}' }, + { path: join(tmp, 'b.ts'), line: 1, preview: 'verifyToken(); // call site' }, + ]); + }); + + it.skipIf(skipReason)('attributes a single-file search to the search path', async () => { + // rg printed no filename at all here; the row's path is null and the + // file it came from is the search path itself. + const r = await GrepTool.execute( + { pattern: 'verifyToken', path: join(tmp, 'a.ts'), '-n': true }, + { cwd: tmp }, + ); + expect(locs(r)).toEqual([ + { path: join(tmp, 'a.ts'), line: 1, preview: 'function verifyToken() {}' }, + ]); + }); + + it.skipIf(skipReason)('lists bare paths in files_with_matches mode', async () => { + const r = await GrepTool.execute( + { pattern: 'verifyToken', path: tmp, output_mode: 'files_with_matches', type: 'ts' }, + { cwd: tmp }, + ); + const paths = locs(r).map((l) => l.path); + expect(paths.sort()).toEqual([join(tmp, 'a.ts'), join(tmp, 'b.ts')]); + expect(locs(r).every((l) => l.line === undefined && l.preview === undefined)).toBe(true); + }); + + it.skipIf(skipReason)( + 'slices with head_limit so entries line up with shown lines', + async () => { + const r = await GrepTool.execute( + { pattern: 'verifyToken', path: tmp, head_limit: 1 }, + { cwd: tmp }, + ); + expect(locs(r)).toHaveLength(1); + // The truncation marker counts everything found, not everything listed. + expect(r.content).toMatch(/\.\.\. \[1 of \d+\]/); + // The one location is the one shown. + const firstLine = (r.content as string).split('\n')[0]!; + expect(firstLine.startsWith(locs(r)[0]!.path)).toBe(true); + }, + ); + }); + // The pre-call gate adjudicates the *search root*. A search rooted at the // workspace is allowed, and then hands back the contents of every file it // matched — including the ones the contract says must never be read. @@ -168,6 +230,9 @@ describe('GrepTool', async () => { expect(r.content).not.toMatch(/hunter2/); // the secret itself expect(r.content).toMatch(/1 result withheld by the file contract/); expect(r.data?.withheld).toBe(1); + // The structured entries must not leak what the text withheld. + const paths = (r.data?.locations as Array<{ path: string }>).map((l) => l.path); + expect(paths.some((p) => p.includes('prod.key'))).toBe(false); }); it.skipIf(skipReason)('withholds in files_with_matches mode too', async () => { diff --git a/packages/core/src/tools/grep.ts b/packages/core/src/tools/grep.ts index 8cbd3ac..551e0a0 100644 --- a/packages/core/src/tools/grep.ts +++ b/packages/core/src/tools/grep.ts @@ -5,6 +5,7 @@ import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; import { isAbsolute, resolve } from 'node:path'; import { withheldNotice, withholdDeniedReads } from '../config/contract-dispatch.js'; +import { MAX_TOOL_LOCATIONS, type ToolLocation } from './presentation.js'; import type { ToolContext, ToolHandler, ToolResult } from '../types.js'; const execFileAsync = promisify(execFile); @@ -90,6 +91,7 @@ export const GrepTool: ToolHandler = { name: 'Grep', definition: { name: 'Grep', + render: 'locations', description: 'Searches for a regex pattern using ripgrep (rg). Supports globs, file types, case-insensitive matching.', inputSchema: { @@ -194,14 +196,15 @@ export const GrepTool: ToolHandler = { (row) => row.path ?? searchPath, ); - let lines = kept.map(formatRipgrepRow); - const matched = lines.length; - - if (input.head_limit && input.head_limit > 0) { - const truncated = lines.length > input.head_limit; - lines = lines.slice(0, input.head_limit); - if (truncated) lines.push(`... [${lines.length} of ${matched}]`); - } + const matched = kept.length; + // Slice the ROWS, not the formatted lines: the locations below must line up + // with what is shown, and rows still carry the exact path `--null` gave us. + const shown = + input.head_limit && input.head_limit > 0 && kept.length > input.head_limit + ? kept.slice(0, input.head_limit) + : kept; + const lines = shown.map(formatRipgrepRow); + if (shown.length < matched) lines.push(`... [${lines.length} of ${matched}]`); // Say that something was withheld, never what. Silence is worse than the // count: an agent that finds nothing goes looking through Bash, which the @@ -209,9 +212,31 @@ export const GrepTool: ToolHandler = { const notice = withheldNotice(withheld); if (notice) lines.push(notice); + // Structured entries for clients that render results as openable locations. + // Built from the parsed rows — the formatted text's `:` separator is not + // reversible (a path may contain one), which is why the text is never + // parsed back. A row without a path came from a single-file search, so the + // search path is the file. Line numbers exist only when rg printed them. + const locations: ToolLocation[] = shown.slice(0, MAX_TOOL_LOCATIONS).map((row) => { + const path = row.path ?? searchPath; + const entry: ToolLocation = { path }; + if (mode === 'content' && row.text !== null) { + if (input['-n']) { + const numbered = /^(\d+):(.*)$/s.exec(row.text); + if (numbered) { + entry.line = Number(numbered[1]); + entry.preview = numbered[2]; + return entry; + } + } + entry.preview = row.text; + } + return entry; + }); + return { content: lines.join('\n') || '(no matches)', - data: { mode, matches: matched, ...(withheld > 0 ? { withheld } : {}) }, + data: { mode, matches: matched, locations, ...(withheld > 0 ? { withheld } : {}) }, }; }, }; diff --git a/packages/core/src/tools/index.ts b/packages/core/src/tools/index.ts index b720135..e78dcc8 100644 --- a/packages/core/src/tools/index.ts +++ b/packages/core/src/tools/index.ts @@ -38,9 +38,12 @@ export { ToolRegistry, BUILTIN_TOOLS } from './registry.js'; export { presentToolCall, pickTarget, + readToolLocations, BUILTIN_RENDER_INTENTS, + MAX_TOOL_LOCATIONS, type ToolRenderKind, type ToolPresentation, type ToolDiffIntent, + type ToolLocation, } from './presentation.js'; export type { ToolDefinition, ToolContext, ToolResult, ToolHandler } from './types.js'; diff --git a/packages/core/src/tools/presentation.test.ts b/packages/core/src/tools/presentation.test.ts index 36c36f7..c84a7f1 100644 --- a/packages/core/src/tools/presentation.test.ts +++ b/packages/core/src/tools/presentation.test.ts @@ -2,8 +2,10 @@ import { describe, it, expect } from 'vitest'; import { BUILTIN_TOOLS } from './registry.js'; import { BUILTIN_RENDER_INTENTS, + MAX_TOOL_LOCATIONS, pickTarget, presentToolCall, + readToolLocations, type ToolRenderKind, } from './presentation.js'; @@ -53,6 +55,19 @@ describe('presentToolCall', () => { expect(presentToolCall('Edit', input)).toEqual(presentToolCall('Edit', input)); }); + it('renders Grep and Glob as location lists, labelled by their pattern', () => { + // The intent alone: what was found lives in the result's data channel, so + // the presentation carries no entries — arguments cannot know the matches. + expect(presentToolCall('Grep', { pattern: 'TODO', path: 'src' })).toEqual({ + kind: 'locations', + target: 'TODO', + }); + expect(presentToolCall('Glob', { pattern: '**/*.ts' })).toEqual({ + kind: 'locations', + target: '**/*.ts', + }); + }); + it('ignores non-string arguments where it expects text', () => { expect(presentToolCall('Bash', { command: 42 }).kind).toBe('generic'); expect(presentToolCall('Edit', { file_path: '/a', old_string: 1, new_string: 2 }).kind).toBe( @@ -72,6 +87,46 @@ describe('pickTarget', () => { }); }); +describe('readToolLocations', () => { + it('reads well-formed entries and keeps only their known fields', () => { + expect( + readToolLocations({ + locations: [ + { path: '/a/b.ts', display: 'b.ts', line: 3, preview: 'hit', extra: 'dropped' }, + { path: '/c.ts' }, + ], + }), + ).toEqual([{ path: '/a/b.ts', display: 'b.ts', line: 3, preview: 'hit' }, { path: '/c.ts' }]); + }); + + it('degrades malformed payloads to no locations instead of throwing', () => { + // This is the one validating extractor every client shares, so a bad + // payload must fail the same quiet way everywhere. + expect(readToolLocations(undefined)).toEqual([]); + expect(readToolLocations('locations')).toEqual([]); + expect(readToolLocations({ locations: 'nope' })).toEqual([]); + expect( + readToolLocations({ locations: [null, 42, { display: 'no path' }, { path: '' }] }), + ).toEqual([]); + }); + + it('ignores a non-numeric line rather than inventing one', () => { + expect(readToolLocations({ locations: [{ path: '/a', line: 'seven' }] })).toEqual([ + { path: '/a' }, + ]); + expect(readToolLocations({ locations: [{ path: '/a', line: Infinity }] })).toEqual([ + { path: '/a' }, + ]); + }); + + it('caps the list at MAX_TOOL_LOCATIONS', () => { + const locations = Array.from({ length: MAX_TOOL_LOCATIONS + 50 }, (_, i) => ({ + path: `/f${i}`, + })); + expect(readToolLocations({ locations })).toHaveLength(MAX_TOOL_LOCATIONS); + }); +}); + describe('BUILTIN_RENDER_INTENTS', () => { it('agrees with what each built-in tool declares', () => { // The table is what clients read when they hold only a name; the definition diff --git a/packages/core/src/tools/presentation.ts b/packages/core/src/tools/presentation.ts index b46ab33..33d6edd 100644 --- a/packages/core/src/tools/presentation.ts +++ b/packages/core/src/tools/presentation.ts @@ -22,7 +22,70 @@ export type ToolRenderKind = /** A shell command and its output. */ | 'terminal' /** A change to a file, shown as added and removed lines. */ - | 'diff'; + | 'diff' + /** A list of file locations the call found, each one openable. */ + | 'locations'; + +/** + * One place a search-shaped tool found, exact enough to open. + * + * The intent stays a pure function of the call's ARGUMENTS — a pattern does not + * say what it will match — so the entries ride the result's `data` channel + * instead, attached by the tool at execute time from its own parsed rows. That + * is the whole point: Grep resolves paths from ripgrep's `--null` output, where + * a path containing `:` is still unambiguous, and re-deriving the entries from + * the formatted result text would reintroduce exactly that ambiguity. + * + * A client replaying a session log has only the result text (session history + * stores what the model saw), so a replayed call falls back to the generic text + * body — same characters, minus the affordance to click them. + */ +export interface ToolLocation { + /** Absolute path, for opening. */ + path: string; + /** The path as the result text printed it (e.g. relative). Defaults to `path`. */ + display?: string; + /** 1-based line, when the tool knows one. */ + line?: number; + /** The matched text, when the tool has it (Grep content mode). */ + preview?: string; +} + +/** + * Upper bound on locations a tool attaches to one result. + * + * An unbounded content-mode Grep can match thousands of lines; the text output + * already carries them all for the model, and duplicating every one as a + * structured entry only bloats the wire for a list no card will usefully show. + */ +export const MAX_TOOL_LOCATIONS = 200; + +/** + * Read `data.locations` back out of a tool result's structured payload. + * + * The one validating extractor every client shares, so a malformed or + * hand-crafted payload degrades to "no locations" everywhere instead of + * crashing one renderer. Entries missing a string path are dropped; + * non-numeric lines are ignored rather than invented. + */ +export function readToolLocations(data: unknown): ToolLocation[] { + if (!data || typeof data !== 'object') return []; + const raw = (data as { locations?: unknown }).locations; + if (!Array.isArray(raw)) return []; + const out: ToolLocation[] = []; + for (const entry of raw.slice(0, MAX_TOOL_LOCATIONS)) { + if (!entry || typeof entry !== 'object') continue; + const { path, display, line, preview } = entry as Record; + if (typeof path !== 'string' || path === '') continue; + out.push({ + path, + ...(typeof display === 'string' ? { display } : {}), + ...(typeof line === 'number' && Number.isFinite(line) ? { line } : {}), + ...(typeof preview === 'string' ? { preview } : {}), + }); + } + return out; +} /** A file change derivable from a call's arguments alone. */ export interface ToolDiffIntent { @@ -73,6 +136,8 @@ export const BUILTIN_RENDER_INTENTS: Readonly> = Edit: 'diff', Write: 'diff', NotebookEdit: 'diff', + Grep: 'locations', + Glob: 'locations', }; /** @@ -115,5 +180,10 @@ export function presentToolCall( return { kind, target, diff: { path, before, after } }; } + // `locations` has no argument-derived payload: a pattern does not say what it + // will match. The entries arrive on the result's `data` channel, and a client + // that has none (a replay from the session log) falls back to the text body. + if (kind === 'locations') return { kind, target }; + return { kind: 'generic', target }; } diff --git a/packages/protocol/src/types.ts b/packages/protocol/src/types.ts index e0a5be7..61c2dbe 100644 --- a/packages/protocol/src/types.ts +++ b/packages/protocol/src/types.ts @@ -68,7 +68,14 @@ export interface ToolCompletedEvent { threadId: string; turnId: string; itemId: string; - result: { content: string; isError?: boolean }; + /** + * `data` is the tool's structured payload (core `ToolResult.data`), passed + * through verbatim — the server has always serialized the whole result, so + * declaring it here documents the wire rather than widening it. Clients use + * it for richer rendering (e.g. Grep/Glob attach `locations`); it is + * transient and not part of the persisted thread snapshot. + */ + result: { content: string; isError?: boolean; data?: Record }; } export interface UsageUpdatedEvent {