diff --git a/docs/tools/read.md b/docs/tools/read.md index adc99a408..87022718c 100644 --- a/docs/tools/read.md +++ b/docs/tools/read.md @@ -249,7 +249,7 @@ Notes: ... - Uses `session.internalRouter` for internal URLs. - Uses `session.allocateOutputArtifact()` for cached/truncated URL output. - Background work / cancellation - - Most branches honor `AbortSignal`; helper paths call `throwIfAborted(signal)` to stop promptly. + - Only the deterministic disk reads are non-abortable: plain-file line/range reads (`streamLinesFromFile`, multi-range) and directory listings (`#readDirectory`) are called with `undefined` instead of the `AbortSignal`, so an interrupt mid-read can't surface a misleading "Operation aborted" on a read that would have finished instantly. Every other branch keeps the signal and its helpers call `throwIfAborted(signal)` to stop promptly: URL/internal-URL reads (network), archive, sqlite, document conversion, image decode, structural summary, conflict scan, and the suffix-glob path resolution. ## Limits & Caps - Shared text truncation defaults from `packages/coding-agent/src/session/streaming-output.ts`: diff --git a/packages/agent/test/scratch-build-history-bench.test.ts b/packages/agent/test/scratch-build-history-bench.test.ts new file mode 100644 index 000000000..00a8f4b6b --- /dev/null +++ b/packages/agent/test/scratch-build-history-bench.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "bun:test"; +import type { Message, Model } from "@oh-my-pi/pi-ai/types"; +import { buildOpenAiNativeHistory } from "../src/compaction/openai"; + +const model = { + provider: "openai-codex", + id: "gpt-5.5", + api: "openai-responses", + contextWindow: 524288, + input: ["text"], +} as unknown as Model; + +function makeCodexHistory(turns: number): Message[] { + const messages: Message[] = []; + for (let i = 0; i < turns; i++) { + messages.push({ + role: "user", + content: [{ type: "text", text: `user turn ${i}` }], + timestamp: i, + } as unknown as Message); + messages.push({ + role: "assistant", + provider: "openai-codex", + model: "gpt-5.5", + api: "openai-responses", + content: [{ type: "text", text: `assistant turn ${i}` }], + providerPayload: { + type: "openaiResponsesHistory", + provider: "openai-codex", + dt: true, + items: [ + { type: "reasoning", id: `rs_${i}`, summary: [] }, + { + type: "message", + role: "assistant", + id: `msg_${i}`, + status: "completed", + content: [{ type: "output_text", text: `assistant turn ${i}`, annotations: [] }], + }, + { type: "function_call", id: `fc_${i}`, call_id: `call_${i}`, name: "read", arguments: "{}" }, + ], + }, + timestamp: i, + } as unknown as Message); + messages.push({ + role: "toolResult", + toolCallId: `call_${i}`, + content: [{ type: "text", text: `result ${i}` }], + timestamp: i, + } as unknown as Message); + } + return messages; +} + +describe("buildOpenAiNativeHistory perf", () => { + it("times native history build for large codex contexts", () => { + for (const turns of [500, 1000, 2000, 4000]) { + const messages = makeCodexHistory(turns); + const t0 = performance.now(); + const out = buildOpenAiNativeHistory(messages, model); + const dt = performance.now() - t0; + console.log(`turns=${turns} items=${out.length} ms=${dt.toFixed(1)}`); + expect(out.length).toBeGreaterThan(0); + } + }); +}); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fe6f545ed..4d92c1c8c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,27 +1,28 @@ # Changelog ## [Unreleased] +### Added + +- Added clickable file path hyperlinks to read tool outputs (read-call rows, grouped summaries, and inline previews) using resolved or absolute file targets with selector-based line anchors for quick navigation ### Changed - Changed hidden custom messages and file-mention context to reach providers as `developer` messages instead of user-authored turns, so system reminders no longer pollute compacted user history. - - Rewrote the plan-mode active prompt (`prompts/system/plan-mode-active.md`) from scratch to stop producing shallow plans. Reframed the artifact as an **execution spec** a fresh agent runs after the planning conversation is cleared/compacted (zero design decisions for the implementer) rather than a brevity-capped summary. Folded high-consensus requirements into the existing sections as inline, conditional rules — no new boilerplate sections: ordered Approach steps that keep the build/tests green after each step (sequencing); exact signatures/literals for new or load-bearing symbols (contracts); full callsite list + clean cutover for renames/signature-changes/removals; Verification that must exercise the new behavior (input → observable output) with run preconditions, not just build/typecheck; Assumptions restricted to user-overridable choices plus pre-decided fallbacks for load-bearing assumptions; a provenance rule (plan facts must come from a read this session; unverified claims flagged inline); and bans on conversation back-references and decision-free sections (Non-Goals/Alternatives/Risks/Future Work). Kept the decision-complete self-check and the brevity-vs-completeness tiebreak (completeness wins). Render contract (Handlebars vars/conditionals) unchanged; verified across all `planExists`/`reentry`/`iterative` branch combinations. ### Removed - Removed the animated pending border ("shimmer") on running `bash`, `eval`, and `ssh` execution blocks. While pending, a block now shows a static accent border instead of sweeping a dark segment around its bottom edge; `display.shimmer` still governs the working-status line and `task` row animations. -- Read, write, and edit tools now honor the active turn `AbortSignal`. +- Removed the tool-level `nonAbortable` bypass so `write` and `edit` honor the active turn `AbortSignal`. `read` is abortable for everything that is slow or non-deterministic (URL/internal-URL reads, archive, sqlite, document conversion, image decode, structural summary, conflict scan, suffix glob); only the deterministic plain-file line/range reads and directory listings run to completion. ### Fixed -- Fixed edit tool headers to hide first-change line suffixes, middle-elide long paths, and show compact change stats. - +- Fixed read output paths so selector suffixes are preserved when corrected paths were returned without selectors +- Fixed `read` surfacing a misleading red "Operation aborted" on a plain-file or directory read when a turn was interrupted mid-read. Those reads are deterministic and fast, so `execute` now runs them to completion instead of cancelling them; slower/non-deterministic reads (archive, sqlite, document, image, summary, conflict scan, URL) stay cancellable. +- Fixed edit tool headers to hide first-change line suffixes, middle-elide long paths only when the header width needs it, show compact change stats, and target encoded `file://` hyperlinks. - Fixed Esc interrupts rendering a redundant `Interrupted by user` assistant transcript line while preserving the interrupt reason for tool-result placeholders and continuation logic. - - LSP writethrough no longer burns the full diagnostics poll on every edit/write. `typescript-language-server` never echoes the document version in `publishDiagnostics` ([upstream #983](https://github.com/typescript-language-server/typescript-language-server/issues/983)), so the exact-version gate never passed; `waitForDiagnostics` now accepts an exact version match instantly and otherwise settles on the latest publish after a short quiescence window, dropping superseded in-flight diagnostics. - LSP writethrough no longer blocks the whole edit/write on slow diagnostics: it now waits only a short inline window (~500ms) for a settled result, then hands the in-flight fetch to the deferred channel so a slow or cold language server (e.g. a large-project `tsserver`) delivers its diagnostics as a follow-up message instead of stalling the tool 3–5s on every edit. The background fetch also gets a longer budget so slow servers still surface late rather than being dropped. - - Fixed the `c`/`.` continue shortcut making the agent second-guess itself after an Esc interrupt. Continuing used to submit an *empty* user turn, which left the model with only the aborted-turn context — so it tended to restate the halted state and ask whether to proceed rather than just continuing. The shortcut now resumes with a hidden agent-authored `developer` directive ("keep going — don't stop to summarize or re-confirm the plan") instead of an empty turn. It still produces no visible transcript entry, same as before. - Fixed native scrollback commit boundaries to be computed generically from finalized transcript blocks and observed append-only live growth, so tall final tool results and streaming previews keep their scrolled-off heads on ED3-risk terminals without per-tool append-only predicates; live blocks that re-layout remain deferred until finalization or the next checkpoint. - Fixed read-group summaries for multi-path `read` results to use result-provided display targets so each resolved path is shown as its own row diff --git a/packages/coding-agent/src/modes/components/read-tool-group.ts b/packages/coding-agent/src/modes/components/read-tool-group.ts index a543cf504..eb8aeb553 100644 --- a/packages/coding-agent/src/modes/components/read-tool-group.ts +++ b/packages/coding-agent/src/modes/components/read-tool-group.ts @@ -1,10 +1,11 @@ +import * as path from "node:path"; import type { Component } from "@oh-my-pi/pi-tui"; import { Container, Text } from "@oh-my-pi/pi-tui"; import { InternalUrlRouter } from "../../internal-urls"; import { getLanguageFromPath, theme } from "../../modes/theme/theme"; -import { parseLineRanges, splitPathAndSel } from "../../tools/path-utils"; +import { parseLineRanges, selectorLineRanges, splitPathAndSel } from "../../tools/path-utils"; import { PREVIEW_LIMITS, shortenPath } from "../../tools/render-utils"; -import { renderCodeCell } from "../../tui"; +import { fileHyperlink, renderCodeCell, tryResolveInternalUrlSync } from "../../tui"; import type { ToolExecutionHandle } from "./tool-execution"; /** @@ -46,12 +47,19 @@ type ReadToolSuffixResolution = { }; type ReadToolResultDetails = { + resolvedPath?: string; suffixResolution?: { from?: string; to?: string; }; conflictCount?: number; displayReadTargets?: unknown; + meta?: { + source?: { + type?: string; + value?: string; + }; + }; }; type ReadToolGroupOptions = { @@ -69,6 +77,7 @@ type ReadEntry = { toolCallId: string; path: string; displayPaths?: string[]; + linkPath?: string; status: "pending" | "success" | "warning" | "error"; correctedFrom?: string; contentText?: string; @@ -82,6 +91,7 @@ type ReadDisplayTarget = { entry: ReadEntry; targetPath: string; basePath: string; + linkPath?: string; selector?: string; }; @@ -109,6 +119,53 @@ function getDisplayReadTargets(details: ReadToolResultDetails | undefined): stri return targets.length > 0 ? targets : undefined; } +function displayPathWithSuffixResolution(currentPath: string, suffixResolution: ReadToolSuffixResolution): string { + const currentSelector = splitPathAndSel(currentPath).sel; + if (!currentSelector || splitPathAndSel(suffixResolution.to).sel) return suffixResolution.to; + return `${suffixResolution.to}:${currentSelector}`; +} + +function readSourceFsPath(details: ReadToolResultDetails | undefined): string | undefined { + const source = details?.meta?.source; + return source?.type === "path" && typeof source.value === "string" ? source.value : undefined; +} + +function readResultLinkPath(details: ReadToolResultDetails | undefined): string | undefined { + return typeof details?.resolvedPath === "string" ? details.resolvedPath : readSourceFsPath(details); +} + +function readTargetLinkPath(basePath: string, entryLinkPath: string | undefined): string | undefined { + if (entryLinkPath) return entryLinkPath; + const resolvedInternalPath = tryResolveInternalUrlSync(basePath); + if (resolvedInternalPath) return resolvedInternalPath; + return path.isAbsolute(basePath) ? basePath : undefined; +} + +function firstSelectorLine(selector: string | undefined): number | undefined { + try { + return selectorLineRanges(selector)?.[0].startLine; + } catch { + return undefined; + } +} + +function firstSelectorLineForTargets(targets: ReadDisplayTarget[]): number | undefined { + let line: number | undefined; + for (const target of targets) { + const targetLine = firstSelectorLine(target.selector); + if (targetLine === undefined) continue; + if (line === undefined || targetLine < line) line = targetLine; + } + return line; +} + +function linkPathForTargets(targets: ReadDisplayTarget[]): string | undefined { + for (const target of targets) { + if (target.linkPath) return target.linkPath; + } + return undefined; +} + function selectorChunkIsLineRangeList(chunk: string): boolean { const trimmed = chunk.trim(); if (!trimmed) return false; @@ -277,8 +334,9 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa const details = result.details as ReadToolResultDetails | undefined; const suffixResolution = getSuffixResolution(details); const displayPaths = getDisplayReadTargets(details); + entry.linkPath = readResultLinkPath(details); if (suffixResolution) { - entry.path = suffixResolution.to; + entry.path = displayPathWithSuffixResolution(entry.path, suffixResolution); entry.correctedFrom = suffixResolution.from; entry.displayPaths = undefined; } else { @@ -364,13 +422,16 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa const targets: ReadDisplayTarget[] = []; for (const entry of entries) { const pathSpecs = entry.displayPaths ?? splitReadDisplayPathSpecs(entry.path); + const useEntryLinkPath = pathSpecs.length === 1; for (const pathSpec of pathSpecs) { const split = splitPathAndSel(pathSpec); + const linkPath = readTargetLinkPath(split.path, useEntryLinkPath ? entry.linkPath : undefined); for (const selector of splitSelectorDisplayParts(split.sel)) { targets.push({ entry, targetPath: selector ? `${split.path}:${selector}` : pathSpec, basePath: split.path, + linkPath, selector, }); } @@ -434,6 +495,8 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa return this.#formatPathValue(row.targetPath, { correctedFrom: this.#correctedFromForTargets(row.targets), conflictCount: this.#conflictCountForTargets(row.targets), + line: firstSelectorLineForTargets(row.targets), + linkPath: linkPathForTargets(row.targets), }); } @@ -479,9 +542,22 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa return this.#previewEntriesForRow(row).length > 0; } - #formatPathValue(value: string, options: { correctedFrom?: string; conflictCount?: number } = {}): string { - const filePath = shortenPath(value); + #formatPathValue( + value: string, + options: { correctedFrom?: string; conflictCount?: number; line?: number; linkPath?: string } = {}, + ): string { + const split = splitPathAndSel(value); + const selectorSuffix = split.sel ? `:${split.sel}` : ""; + const baseValue = split.sel ? split.path : value; + const filePath = shortenPath(baseValue); let pathDisplay = filePath ? theme.fg("accent", filePath) : theme.fg("toolOutput", "…"); + if (filePath && options.linkPath) { + const linkOptions = options.line !== undefined ? { line: options.line } : undefined; + pathDisplay = fileHyperlink(options.linkPath, pathDisplay, linkOptions); + } + if (selectorSuffix) { + pathDisplay += theme.fg("accent", selectorSuffix); + } if (options.correctedFrom) { pathDisplay += theme.fg("dim", ` (corrected from ${shortenPath(options.correctedFrom)})`); } @@ -501,10 +577,18 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa * When expanded: shows full content. */ #addContentPreview(entry: ReadEntry): void { - const lang = getLanguageFromPath(splitPathAndSel(entry.path).path); - const filePath = shortenPath(entry.path); - const correctionSuffix = entry.correctedFrom ? ` (corrected from ${shortenPath(entry.correctedFrom)})` : ""; - const title = filePath ? `Read ${filePath}${correctionSuffix}` : "Read"; + const split = splitPathAndSel(entry.path); + const lang = getLanguageFromPath(split.path); + const pathValue = shortenPath(entry.path); + const pathDisplay = pathValue + ? this.#formatPathValue(entry.path, { + correctedFrom: entry.correctedFrom, + conflictCount: entry.conflictCount, + line: firstSelectorLine(split.sel), + linkPath: readTargetLinkPath(split.path, entry.linkPath), + }) + : ""; + const title = pathDisplay ? `Read ${pathDisplay}` : "Read"; let cachedWidth: number | undefined; let cachedLines: string[] | undefined; const expanded = this.#expanded; diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index c5973a2ae..86124528f 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -1652,7 +1652,9 @@ export class ReadTool implements AgentTool { throw new ToolError("Multi-range line selectors are not supported for directory listings."); } const { offset, limit } = selToOffsetLimit(parsed); - const dirResult = await this.#readDirectory(absolutePath, offset, limit, signal); + // Directory listings are deterministic and fast; never abort them mid-scan + // (an interrupt would otherwise surface a misleading "Operation aborted"). + const dirResult = await this.#readDirectory(absolutePath, offset, limit, undefined); if (suffixResolution) { dirResult.details ??= {}; dirResult.details.suffixResolution = suffixResolution; @@ -1819,7 +1821,7 @@ export class ReadTool implements AgentTool { parsed, displayMode, suffixResolution, - signal, + undefined, // plain-file read: deterministic and fast, never abort mid-read ); if (multiResult.bridgeResult) return multiResult.bridgeResult; content = [{ type: "text", text: multiResult.outputText }]; @@ -1878,7 +1880,7 @@ export class ReadTool implements AgentTool { maxLinesToCollect, maxBytesForRead, selectedLineLimit, - signal, + undefined, // plain-file read: deterministic and fast, never abort mid-read ); const { @@ -2372,11 +2374,13 @@ function formatReadPathLink( const plainDisplayPath = options.suffixResolution ? shortenPath(options.suffixResolution.to) : shortenPath(basePath || options.resolvedPath || options.fallbackLabel || rawPath); - const target = options.resolvedPath ?? options.sourcePath ?? tryResolveInternalUrlSync(basePath); + const absoluteInputPath = path.isAbsolute(basePath) ? basePath : undefined; + const target = + options.resolvedPath ?? options.sourcePath ?? tryResolveInternalUrlSync(basePath) ?? absoluteInputPath; const line = firstReadSelectorLine(split.sel) ?? options.offset; const linkOptions = line !== undefined ? { line } : undefined; - const displayPath = target ? fileHyperlink(target, plainDisplayPath, linkOptions) : plainDisplayPath; - return `${displayPath}${selectorSuffix}`; + const linkedPath = target ? fileHyperlink(target, plainDisplayPath, linkOptions) : plainDisplayPath; + return `${linkedPath}${selectorSuffix}`; } export const readToolRenderer = { diff --git a/packages/coding-agent/test/read-tool-group.test.ts b/packages/coding-agent/test/read-tool-group.test.ts index 25c75f3cb..907c08ec4 100644 --- a/packages/coding-agent/test/read-tool-group.test.ts +++ b/packages/coding-agent/test/read-tool-group.test.ts @@ -1,17 +1,35 @@ -import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { resetSettingsForTest, Settings, settings } from "../src/config/settings"; import { getDefault } from "../src/config/settings-schema"; import { ReadToolGroupComponent, readArgsTargetInternalUrl } from "../src/modes/components/read-tool-group"; import * as themeModule from "../src/modes/theme/theme"; +function extractLinkUris(text: string): string[] { + return [...text.matchAll(/\x1b\]8;[^;]*;([^\x1b]+)\x1b\\/g)].map(match => match[1]!); +} + +function extractLinkTexts(text: string): string[] { + return [...text.matchAll(/\x1b\]8;[^;]*;[^\x1b]+\x1b\\([\s\S]*?)\x1b\]8;;\x1b\\/g)].map(match => + Bun.stripANSI(match[1]!), + ); +} + describe("ReadToolGroupComponent", () => { beforeAll(async () => { + resetSettingsForTest(); + await Settings.init({ inMemory: true }); await themeModule.initTheme(false, undefined, undefined, "dark", "light"); }); afterEach(() => { + settings.clearOverride("tui.hyperlinks"); vi.restoreAllMocks(); }); + afterAll(() => { + resetSettingsForTest(); + }); + it("keeps inline read previews disabled by default", () => { expect(getDefault("read.toolResultPreview")).toBe(false); @@ -188,6 +206,48 @@ describe("ReadToolGroupComponent", () => { expect(matches).toHaveLength(1); }); + + it("links grouped summary paths to resolved filesystem paths and selector lines", () => { + settings.override("tui.hyperlinks", "always"); + const component = new ReadToolGroupComponent(); + component.updateArgs({ path: "src/example.ts:7-9" }, "read-link"); + component.updateResult( + { + content: [{ type: "text", text: "line 7" }], + details: { meta: { source: { type: "path", value: "/workspace/src/example.ts" } } }, + }, + false, + "read-link", + ); + + const rendered = component.render(120).join("\n"); + + expect(Bun.stripANSI(rendered)).toContain("Read src/example.ts:7-9"); + expect(extractLinkUris(rendered)).toContain("file:///workspace/src/example.ts?line=7"); + expect(extractLinkTexts(rendered)).toContain("src/example.ts"); + expect(extractLinkTexts(rendered)).not.toContain("src/example.ts:7-9"); + }); + + it("links inline preview titles when the summary row is suppressed", () => { + settings.override("tui.hyperlinks", "always"); + const component = new ReadToolGroupComponent({ showContentPreview: true }); + component.updateArgs({ path: "src/preview.ts:20-22" }, "read-preview-link"); + component.updateResult( + { + content: [{ type: "text", text: "line 20\nline 21\nline 22" }], + details: { resolvedPath: "/workspace/src/preview.ts" }, + }, + false, + "read-preview-link", + ); + + const rendered = component.render(120).join("\n"); + + expect(Bun.stripANSI(rendered)).toContain("Read src/preview.ts:20-22"); + expect(extractLinkUris(rendered)).toContain("file:///workspace/src/preview.ts?line=20"); + expect(extractLinkTexts(rendered)).toContain("src/preview.ts"); + expect(extractLinkTexts(rendered)).not.toContain("src/preview.ts:20-22"); + }); }); describe("readArgsTargetInternalUrl", () => { diff --git a/packages/coding-agent/test/tools/read-fs-not-abortable.test.ts b/packages/coding-agent/test/tools/read-fs-not-abortable.test.ts new file mode 100644 index 000000000..f33fdad73 --- /dev/null +++ b/packages/coding-agent/test/tools/read-fs-not-abortable.test.ts @@ -0,0 +1,107 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; +import { ReadTool } from "@oh-my-pi/pi-coding-agent/tools/read"; +import { ToolAbortError } from "@oh-my-pi/pi-coding-agent/tools/tool-errors"; +import { Snowflake } from "@oh-my-pi/pi-utils"; + +function getTextOutput(result: { content: Array<{ type: string; text?: string }> }): string { + return result.content + .filter(c => c.type === "text" && typeof c.text === "string") + .map(c => c.text as string) + .join("\n"); +} + +function makeSession(cwd: string): ToolSession { + return { + cwd, + hasUI: false, + getSessionFile: () => path.join(cwd, "session.jsonl"), + getSessionSpawns: () => "*", + getArtifactsDir: () => path.join(cwd, "session"), + allocateOutputArtifact: async (toolType: string) => ({ + id: "a1", + path: path.join(cwd, "session", `a1.${toolType}.log`), + }), + settings: Settings.isolated(), + }; +} + +function abortedSignal(): AbortSignal { + const controller = new AbortController(); + controller.abort(); + return controller.signal; +} + +// Only the deterministic, fast disk reads — plain file line/range reads and +// directory listings — are non-abortable: a turn interrupt that fires mid-read +// must not surface "Operation aborted" on a read that would have completed +// instantly. Non-deterministic reads (archive, sqlite, document conversion, +// image decode, structural summary, conflict scan) stay cancellable. +describe("plain-file and directory reads ignore an already-aborted signal", () => { + let testDir: string; + let tool: ReadTool; + + beforeEach(() => { + testDir = path.join(os.tmpdir(), `read-fs-noabort-${Snowflake.next()}`); + fs.mkdirSync(testDir, { recursive: true }); + tool = new ReadTool(makeSession(testDir)); + }); + + afterEach(() => { + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + it("returns a plain-file line range with an aborted signal", async () => { + const filePath = path.join(testDir, "range.txt"); + fs.writeFileSync(filePath, Array.from({ length: 40 }, (_, i) => `line-${i + 1}`).join("\n")); + + const result = await tool.execute("call-range", { path: `${filePath}:20-22` }, abortedSignal()); + const output = getTextOutput(result); + + expect(result.isError).toBeFalsy(); + expect(output).toContain("line-20"); + expect(output).toContain("line-22"); + }); + + it("returns a multi-range plain-file read with an aborted signal", async () => { + const filePath = path.join(testDir, "multi.txt"); + fs.writeFileSync(filePath, Array.from({ length: 40 }, (_, i) => `line-${i + 1}`).join("\n")); + + const result = await tool.execute("call-multi", { path: `${filePath}:2-3,30-31` }, abortedSignal()); + const output = getTextOutput(result); + + expect(result.isError).toBeFalsy(); + expect(output).toContain("line-2"); + expect(output).toContain("line-30"); + }); + + it("returns a directory listing with an aborted signal", async () => { + for (let i = 1; i <= 5; i++) { + fs.writeFileSync(path.join(testDir, `f-${i}.txt`), ""); + } + + const result = await tool.execute("call-dir", { path: testDir }, abortedSignal()); + const output = getTextOutput(result); + + expect(result.isError).toBeFalsy(); + expect(result.details?.isDirectory).toBe(true); + expect(output).toContain("f-1.txt"); + expect(output).toContain("f-5.txt"); + }); + + // Boundary: non-deterministic reads must remain cancellable. The conflict + // scan honors the abort signal, so an already-aborted read still fails fast + // instead of being forced to run to completion like a plain file read. + it("still aborts a non-plain read (`:conflicts`) when the signal is aborted", async () => { + const filePath = path.join(testDir, "conflicted.txt"); + fs.writeFileSync(filePath, "hello\nworld\n"); + + await expect( + tool.execute("call-conflicts", { path: `${filePath}:conflicts` }, abortedSignal()), + ).rejects.toBeInstanceOf(ToolAbortError); + }); +}); diff --git a/packages/coding-agent/test/tools/read-renderer.test.ts b/packages/coding-agent/test/tools/read-renderer.test.ts index 0c7991235..8d4cb981f 100644 --- a/packages/coding-agent/test/tools/read-renderer.test.ts +++ b/packages/coding-agent/test/tools/read-renderer.test.ts @@ -9,6 +9,12 @@ function extractLinkUris(text: string): string[] { return [...text.matchAll(/\x1b\]8;[^;]*;([^\x1b]+)\x1b\\/g)].map(match => match[1]!); } +function extractLinkTexts(text: string): string[] { + return [...text.matchAll(/\x1b\]8;[^;]*;[^\x1b]+\x1b\\([\s\S]*?)\x1b\]8;;\x1b\\/g)].map(match => + Bun.stripANSI(match[1]!), + ); +} + beforeAll(async () => { await initTheme(); resetSettingsForTest(); @@ -47,6 +53,26 @@ describe("readToolRenderer hyperlinks", () => { expect(rendered).toContain("local://handoff.md"); expect(rendered).toContain(":2"); expect(extractLinkUris(rendered)).toContain("file:///tmp/omp-local/handoff.md?line=2"); + expect(extractLinkTexts(rendered)).toContain("local://handoff.md"); + expect(extractLinkTexts(rendered)).not.toContain("local://handoff.md:2"); + }); + + it("links absolute read call paths to file URIs with selector lines", async () => { + settings.override("tui.hyperlinks", "always"); + const theme = await getThemeByName("dark"); + expect(theme).toBeDefined(); + + const component = readToolRenderer.renderCall( + { path: "/tmp/omp-read/example.ts:10-12" }, + { expanded: false, isPartial: false }, + theme!, + ); + + const rendered = component.render(200).join("\n"); + expect(Bun.stripANSI(rendered)).toContain("/tmp/omp-read/example.ts:10-12"); + expect(extractLinkUris(rendered)).toContain("file:///tmp/omp-read/example.ts?line=10"); + expect(extractLinkTexts(rendered)).toContain("/tmp/omp-read/example.ts"); + expect(extractLinkTexts(rendered)).not.toContain("/tmp/omp-read/example.ts:10-12"); }); it("links HTTP read result headers to the final URL", async () => {