feat: enabled clickable read links and made deterministic reads non-abortable
- Enabled clickable read-path output for result rows, summaries, and previews. - Resolved read links from result paths, source metadata, internal URLs, and absolutes. - Preserved selector suffixes while rendering line-anchor hyperlinks. - Ignored aborted signals for plain-file and directory reads while keeping conflicts-cancel behavior. - Added tests for non-abortable read behavior and link-label rendering regression coverage.
This commit is contained in:
can1357
+1
-1
@@ -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`:
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -1652,7 +1652,9 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
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<typeof readSchema, ReadToolDetails> {
|
||||
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<typeof readSchema, ReadToolDetails> {
|
||||
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 = {
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in New Issue
Block a user