feat(tools): added multi-range line selectors and raw mode support for URLs and directories
- Added support for multi-range line selectors on URLs (e.g., `:5-10,20-30`) and combining `:raw` mode with line range selectors. - Added support for line range selectors on directory listings with offset and limit parameters. - Fixed `:raw` selector being ignored for JSON and feed URLs and directory listing line selectors dropping offset parameter. - Added clear error message for line offset beyond directory listing end. - Refactored URL parsing and directory reading to support multiple comma-separated ranges and improved line-based slicing logic. - Added comprehensive test coverage for multi-range selectors, raw mode combinations, and directory range operations.
This commit is contained in:
@@ -1,6 +1,21 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
### Added
|
||||
|
||||
- Support for multi-range line selectors on URLs (e.g., `:5-10,20-30`) to fetch and display multiple non-contiguous sections
|
||||
- Support for combining `:raw` mode with line range selectors on URLs (e.g., `:raw:1-120` or `:1-120:raw`)
|
||||
- Support for line range selectors on directory listings (e.g., `:30-40` to view lines 30–40 of a directory tree)
|
||||
- Clear error message when requesting a line offset beyond the end of a directory listing
|
||||
|
||||
### Changed
|
||||
|
||||
- URL selector parsing now supports multiple trailing selector tokens (e.g., `:raw:N-M`), applying them left-to-right
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `:raw` selector being ignored for JSON and feed URLs, causing them to be pretty-printed or converted to markdown instead of returning raw content
|
||||
- Fixed directory listing line selectors silently dropping the offset parameter and only applying the limit
|
||||
|
||||
## [15.5.5] - 2026-05-27
|
||||
|
||||
|
||||
@@ -24,6 +24,7 @@ import { finalizeOutput, loadPage, looksLikeHtml, MAX_OUTPUT_CHARS } from "../we
|
||||
import { convertWithMarkit, fetchBinary } from "../web/scrapers/utils";
|
||||
import { applyListLimit } from "./list-limit";
|
||||
import { formatStyledArtifactReference, type OutputMeta } from "./output-meta";
|
||||
import { type LineRange, parseLineRanges } from "./path-utils";
|
||||
import { formatExpandHint, getDomain, replaceTabs } from "./render-utils";
|
||||
import { ToolAbortError, ToolError } from "./tool-errors";
|
||||
import { toolResult } from "./tool-result";
|
||||
@@ -139,16 +140,31 @@ export function isReadableUrlPath(value: string): boolean {
|
||||
return /^https?:\/\//i.test(value) || /^www\./i.test(value);
|
||||
}
|
||||
|
||||
// URL line selectors mirror the file form: `:50`, `:50-100`, `:50+150`, `:raw`.
|
||||
// If a URL would otherwise look like `host:port`, add a trailing slash before the selector
|
||||
// (e.g. `https://example.com/:80` to read line 80 of the document at `https://example.com/`).
|
||||
const URL_LINE_RANGE_RE = /^(\d+)(?:([-+])(\d+))?$/;
|
||||
// URL line selectors mirror the file form: `:50`, `:50-100`, `:50+150`, `:5-10,20-30`, `:raw`,
|
||||
// or `:raw:N-M` / `:N-M:raw` to combine raw mode with a range. If a URL would otherwise look
|
||||
// like `host:port`, add a trailing slash before the selector (e.g. `https://example.com/:80`
|
||||
// to read line 80 of the document at `https://example.com/`).
|
||||
|
||||
export interface ParsedReadUrlTarget {
|
||||
path: string;
|
||||
raw: boolean;
|
||||
offset?: number;
|
||||
limit?: number;
|
||||
/** Populated only when the selector carries 2+ ranges. Single-range stays on offset/limit. */
|
||||
ranges?: readonly LineRange[];
|
||||
}
|
||||
|
||||
/** Recognize a single selector token (`raw` or one/many line ranges). */
|
||||
function isUrlSelectorToken(token: string): boolean {
|
||||
if (token === "raw") return true;
|
||||
try {
|
||||
return parseLineRanges(token) !== null;
|
||||
} catch {
|
||||
// `parseLineRanges` throws `ToolError` for malformed ranges (e.g. `5+0`). Only treat the
|
||||
// token as a selector when it parses cleanly so URL ports like `:80` keep flowing
|
||||
// through to the URL path.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function parseReadUrlTarget(readPath: string): ParsedReadUrlTarget | null {
|
||||
@@ -158,62 +174,71 @@ export function parseReadUrlTarget(readPath: string): ParsedReadUrlTarget | null
|
||||
return null;
|
||||
}
|
||||
|
||||
const selector = embedded?.sel;
|
||||
const raw = selector === "raw";
|
||||
const lineMatch = selector && selector !== "raw" ? URL_LINE_RANGE_RE.exec(selector) : null;
|
||||
if (lineMatch) {
|
||||
const startLine = Number.parseInt(lineMatch[1]!, 10);
|
||||
if (startLine < 1) {
|
||||
throw new ToolError("URL line selector 0 is invalid; lines are 1-indexed. Use :1.");
|
||||
let raw = false;
|
||||
let ranges: readonly LineRange[] | undefined;
|
||||
for (const sel of embedded?.sels ?? []) {
|
||||
if (sel === "raw") {
|
||||
raw = true;
|
||||
continue;
|
||||
}
|
||||
const sep = lineMatch[2];
|
||||
const rhs = lineMatch[3] ? Number.parseInt(lineMatch[3], 10) : undefined;
|
||||
let endLine: number | undefined;
|
||||
if (sep === "+") {
|
||||
if (rhs === undefined || rhs < 1) {
|
||||
throw new ToolError(`Invalid range ${startLine}+${rhs ?? 0}: count must be >= 1.`);
|
||||
}
|
||||
endLine = startLine + rhs - 1;
|
||||
} else if (sep === "-") {
|
||||
if (rhs === undefined || rhs < startLine) {
|
||||
throw new ToolError(`Invalid range ${startLine}-${rhs ?? 0}: end must be >= start.`);
|
||||
}
|
||||
endLine = rhs;
|
||||
if (ranges !== undefined) {
|
||||
// Two range groups on the same URL (`…:5-10:20-30`) — combine with commas instead.
|
||||
throw new ToolError(
|
||||
`URL selector has multiple range groups; combine them with commas (e.g. \`:5-10,20-30\`).`,
|
||||
);
|
||||
}
|
||||
const parsed = parseLineRanges(sel);
|
||||
if (parsed === null) {
|
||||
// Shouldn't happen — isUrlSelectorToken vetted it. Belt-and-suspenders.
|
||||
throw new ToolError(`Invalid URL line selector: ${sel}`);
|
||||
}
|
||||
ranges = parsed;
|
||||
}
|
||||
|
||||
if (!ranges || ranges.length === 0) return { path: urlPath, raw };
|
||||
if (ranges.length === 1) {
|
||||
const r = ranges[0];
|
||||
return {
|
||||
path: urlPath,
|
||||
raw: false,
|
||||
offset: startLine,
|
||||
limit: endLine !== undefined ? endLine - startLine + 1 : undefined,
|
||||
raw,
|
||||
offset: r.startLine,
|
||||
limit: r.endLine !== undefined ? r.endLine - r.startLine + 1 : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
return { path: urlPath, raw };
|
||||
return { path: urlPath, raw, ranges };
|
||||
}
|
||||
|
||||
function tryExtractEmbeddedUrlSelector(readPath: string): { path: string; sel?: string } | null {
|
||||
const lastColonIndex = readPath.lastIndexOf(":");
|
||||
if (lastColonIndex <= 0) {
|
||||
return null;
|
||||
}
|
||||
/**
|
||||
* Peel one or more selector tokens off the right of a URL string. Walks back through
|
||||
* trailing `:tok` segments while each token (a) looks like a selector and (b) leaves
|
||||
* behind a string that still parses as a URL. Returns selectors left-to-right so callers
|
||||
* can apply them in source order.
|
||||
*/
|
||||
function tryExtractEmbeddedUrlSelector(readPath: string): { path: string; sels: string[] } | null {
|
||||
let basePath = readPath;
|
||||
const sels: string[] = [];
|
||||
while (true) {
|
||||
const lastColonIndex = basePath.lastIndexOf(":");
|
||||
if (lastColonIndex <= 0) break;
|
||||
|
||||
const candidateSelector = readPath.slice(lastColonIndex + 1);
|
||||
const basePath = readPath.slice(0, lastColonIndex);
|
||||
if (!isReadableUrlPath(basePath)) {
|
||||
return null;
|
||||
}
|
||||
const candidate = basePath.slice(lastColonIndex + 1);
|
||||
const remainder = basePath.slice(0, lastColonIndex);
|
||||
if (!isReadableUrlPath(remainder)) break;
|
||||
if (!isUrlSelectorToken(candidate)) break;
|
||||
|
||||
const isEmbeddedSelector = candidateSelector === "raw" || URL_LINE_RANGE_RE.test(candidateSelector);
|
||||
if (!isEmbeddedSelector) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
new URL(
|
||||
remainder.startsWith("http://") || remainder.startsWith("https://") ? remainder : `https://${remainder}`,
|
||||
);
|
||||
} catch {
|
||||
break;
|
||||
}
|
||||
|
||||
try {
|
||||
new URL(basePath.startsWith("http://") || basePath.startsWith("https://") ? basePath : `https://${basePath}`);
|
||||
return { path: basePath, sel: candidateSelector };
|
||||
} catch {
|
||||
return null;
|
||||
sels.unshift(candidate);
|
||||
basePath = remainder;
|
||||
}
|
||||
if (sels.length === 0) return null;
|
||||
return { path: basePath, sels };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -932,6 +957,22 @@ async function renderUrl(
|
||||
const isText = mime.includes("text/plain") || mime.includes("text/markdown");
|
||||
const isFeed = mime.includes("rss") || mime.includes("atom") || mime.includes("feed");
|
||||
|
||||
// Raw mode skips every text-shaping branch below (JSON pretty-print, feed-to-markdown,
|
||||
// HTML extraction) and returns the response body verbatim. The image/markit branches
|
||||
// above already ran because raw isn't useful for binary payloads.
|
||||
if (raw) {
|
||||
const output = finalizeOutput(rawContent);
|
||||
return {
|
||||
url,
|
||||
finalUrl,
|
||||
contentType: mime,
|
||||
method: "raw",
|
||||
content: output.content,
|
||||
fetchedAt,
|
||||
truncated: output.truncated,
|
||||
notes,
|
||||
};
|
||||
}
|
||||
if (isJson) {
|
||||
const output = finalizeOutput(formatJson(rawContent));
|
||||
return {
|
||||
|
||||
@@ -1488,6 +1488,21 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
if (!this.session.settings.get("fetch.enabled")) {
|
||||
throw new ToolError("URL reads are disabled by settings.");
|
||||
}
|
||||
if (parsedUrlTarget.ranges !== undefined) {
|
||||
const cached = await loadReadUrlCacheEntry(
|
||||
this.session,
|
||||
{ path: parsedUrlTarget.path, raw: parsedUrlTarget.raw },
|
||||
signal,
|
||||
{ ensureArtifact: true, preferCached: true },
|
||||
);
|
||||
return this.#buildInMemoryMultiRangeResult(cached.output, parsedUrlTarget.ranges, {
|
||||
details: { ...cached.details },
|
||||
sourceUrl: cached.details.finalUrl,
|
||||
entityLabel: "URL output",
|
||||
raw: parsedUrlTarget.raw,
|
||||
immutable: true,
|
||||
});
|
||||
}
|
||||
if (parsedUrlTarget.offset !== undefined || parsedUrlTarget.limit !== undefined) {
|
||||
const cached = await loadReadUrlCacheEntry(
|
||||
this.session,
|
||||
@@ -1502,6 +1517,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
details: { ...cached.details },
|
||||
sourceUrl: cached.details.finalUrl,
|
||||
entityLabel: "URL output",
|
||||
raw: parsedUrlTarget.raw,
|
||||
immutable: true,
|
||||
});
|
||||
}
|
||||
@@ -1578,7 +1594,8 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
if (isMultiRange(parsed)) {
|
||||
throw new ToolError("Multi-range line selectors are not supported for directory listings.");
|
||||
}
|
||||
const dirResult = await this.#readDirectory(absolutePath, selToOffsetLimit(parsed).limit, signal);
|
||||
const { offset, limit } = selToOffsetLimit(parsed);
|
||||
const dirResult = await this.#readDirectory(absolutePath, offset, limit, signal);
|
||||
if (suffixResolution) {
|
||||
dirResult.details ??= {};
|
||||
dirResult.details.suffixResolution = suffixResolution;
|
||||
@@ -2136,6 +2153,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
/** Read directory contents as a formatted listing */
|
||||
async #readDirectory(
|
||||
absolutePath: string,
|
||||
offset: number | undefined,
|
||||
limit: number | undefined,
|
||||
signal?: AbortSignal,
|
||||
): Promise<AgentToolResult<ReadToolDetails>> {
|
||||
@@ -2149,7 +2167,9 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
maxDepth: READ_DIRECTORY_MAX_DEPTH,
|
||||
perDirLimit: READ_DIRECTORY_CHILD_LIMIT,
|
||||
rootLimit: null,
|
||||
lineCap: limit ?? null,
|
||||
// `lineCap` truncates the rendered tree itself, so apply it only when the caller
|
||||
// did not request an offset — otherwise we'd cap the first N lines before slicing.
|
||||
lineCap: offset === undefined && limit !== undefined ? limit : null,
|
||||
});
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
@@ -2158,12 +2178,46 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
throwIfAborted(signal);
|
||||
|
||||
const output = tree.totalLines <= 1 ? "(empty directory)" : tree.rendered;
|
||||
const truncation = truncateHead(output, { maxLines: Number.MAX_SAFE_INTEGER });
|
||||
const details: ReadToolDetails = {
|
||||
isDirectory: true,
|
||||
resolvedPath: tree.rootPath,
|
||||
};
|
||||
|
||||
// Slice the rendered listing when the caller passed an offset/limit. We do this
|
||||
// instead of passing the selector down to `buildDirectoryTree` because the tree
|
||||
// builder lays out entries hierarchically (per-dir caps, recent-then-elided
|
||||
// summaries); line-based slicing operates on the formatted text and matches what
|
||||
// users expect from `:N-M` on long listings.
|
||||
const wantsSlice = offset !== undefined || limit !== undefined;
|
||||
if (wantsSlice) {
|
||||
const allLines = output.split("\n");
|
||||
const start = offset ? Math.max(0, offset - 1) : 0;
|
||||
if (start >= allLines.length) {
|
||||
const suggestion =
|
||||
allLines.length === 0
|
||||
? "The listing is empty."
|
||||
: `Use :1 to read from the start, or :${allLines.length} to read the last line.`;
|
||||
return toolResult(details)
|
||||
.text(`Line ${start + 1} is beyond end of listing (${allLines.length} lines total). ${suggestion}`)
|
||||
.sourcePath(tree.rootPath)
|
||||
.done();
|
||||
}
|
||||
const end = limit !== undefined ? Math.min(start + limit, allLines.length) : allLines.length;
|
||||
const sliced = allLines.slice(start, end).join("\n");
|
||||
const resultBuilder = toolResult(details).sourcePath(tree.rootPath);
|
||||
let text = sliced;
|
||||
if (end < allLines.length) {
|
||||
const remaining = allLines.length - end;
|
||||
text += `\n\n[${remaining} more lines in listing. Use :${end + 1} to continue]`;
|
||||
}
|
||||
resultBuilder.text(text);
|
||||
if (tree.truncated) {
|
||||
resultBuilder.limits({ resultLimit: 1 });
|
||||
}
|
||||
return resultBuilder.done();
|
||||
}
|
||||
|
||||
const truncation = truncateHead(output, { maxLines: Number.MAX_SAFE_INTEGER });
|
||||
const resultBuilder = toolResult(details).text(truncation.content).sourcePath(tree.rootPath);
|
||||
if (tree.truncated) {
|
||||
resultBuilder.limits({ resultLimit: 1 });
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } 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 * as scrapers from "@oh-my-pi/pi-coding-agent/web/scrapers/types";
|
||||
import { Snowflake } from "@oh-my-pi/pi-utils";
|
||||
|
||||
const ATOM = `<?xml version="1.0"?>\n<feed xmlns="http://www.w3.org/2005/Atom"><title>Sample</title><entry><title>One</title><id>1</id><updated>2024-01-01T00:00:00Z</updated><content>body</content></entry></feed>`;
|
||||
const JSON_BODY = `{"alpha":1,"beta":[2,3]}`;
|
||||
|
||||
function makeSession(testDir: string): ToolSession {
|
||||
const sessionFile = path.join(testDir, "session.jsonl");
|
||||
const artifactsDir = sessionFile.slice(0, -6);
|
||||
let nextArtifactId = 0;
|
||||
return {
|
||||
cwd: testDir,
|
||||
hasUI: false,
|
||||
getSessionFile: () => sessionFile,
|
||||
getArtifactsDir: () => artifactsDir,
|
||||
getSessionSpawns: () => null,
|
||||
allocateOutputArtifact: async toolType => {
|
||||
const id = String(nextArtifactId++);
|
||||
return { id, path: path.join(artifactsDir, `${id}.${toolType}.log`) };
|
||||
},
|
||||
settings: Settings.isolated({ "fetch.enabled": true }),
|
||||
};
|
||||
}
|
||||
|
||||
function stubLoadPage(body: string, contentType: string) {
|
||||
return vi.spyOn(scrapers, "loadPage").mockImplementation(async (requestedUrl: string) => ({
|
||||
ok: true,
|
||||
status: 200,
|
||||
finalUrl: requestedUrl,
|
||||
contentType,
|
||||
content: body,
|
||||
}));
|
||||
}
|
||||
|
||||
describe("read URL with :raw selector (regression: JSON/feed parsers ignored raw flag)", () => {
|
||||
let testDir: string;
|
||||
beforeEach(() => {
|
||||
testDir = path.join(os.tmpdir(), `fetch-raw-mode-${Snowflake.next()}`);
|
||||
fs.mkdirSync(testDir, { recursive: true });
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
fs.rmSync(testDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("returns the raw atom feed body when :raw is set", async () => {
|
||||
const session = makeSession(testDir);
|
||||
const tool = new ReadTool(session);
|
||||
stubLoadPage(ATOM, "application/atom+xml");
|
||||
|
||||
const result = await tool.execute("call", { path: "https://example.com/feed.xml:raw" });
|
||||
const textBlock = result.content.find(c => c.type === "text");
|
||||
|
||||
expect(result.details?.method).toBe("raw");
|
||||
// The raw response body must round-trip verbatim — not get rewritten to "# Atom Feed".
|
||||
expect(textBlock?.text).toContain('<feed xmlns="http://www.w3.org/2005/Atom">');
|
||||
expect(textBlock?.text).toContain("<entry>");
|
||||
expect(textBlock?.text).not.toContain("# Atom Feed");
|
||||
});
|
||||
|
||||
it("returns the rendered atom feed when :raw is absent (existing behavior)", async () => {
|
||||
const session = makeSession(testDir);
|
||||
const tool = new ReadTool(session);
|
||||
stubLoadPage(ATOM, "application/atom+xml");
|
||||
|
||||
const result = await tool.execute("call", { path: "https://example.com/feed.xml" });
|
||||
expect(result.details?.method).toBe("feed");
|
||||
});
|
||||
|
||||
it("returns the raw JSON body when :raw is set", async () => {
|
||||
const session = makeSession(testDir);
|
||||
const tool = new ReadTool(session);
|
||||
stubLoadPage(JSON_BODY, "application/json");
|
||||
|
||||
const result = await tool.execute("call", { path: "https://example.com/api.json:raw" });
|
||||
const textBlock = result.content.find(c => c.type === "text");
|
||||
|
||||
expect(result.details?.method).toBe("raw");
|
||||
// Body comes back as-is, not pretty-printed.
|
||||
expect(textBlock?.text).toContain('{"alpha":1,"beta":[2,3]}');
|
||||
});
|
||||
|
||||
it("still pretty-prints JSON when :raw is absent (existing behavior)", async () => {
|
||||
const session = makeSession(testDir);
|
||||
const tool = new ReadTool(session);
|
||||
stubLoadPage(JSON_BODY, "application/json");
|
||||
|
||||
const result = await tool.execute("call", { path: "https://example.com/api.json" });
|
||||
const textBlock = result.content.find(c => c.type === "text");
|
||||
|
||||
expect(result.details?.method).toBe("json");
|
||||
expect(textBlock?.text).toContain('"alpha": 1');
|
||||
});
|
||||
|
||||
it("returns slices of raw content when :raw is combined with a range", async () => {
|
||||
const session = makeSession(testDir);
|
||||
const tool = new ReadTool(session);
|
||||
const body = Array.from({ length: 20 }, (_, i) => `raw line ${i + 1}`).join("\n");
|
||||
stubLoadPage(body, "application/json");
|
||||
|
||||
// `:raw:N-M` must skip JSON pretty-print (raw mode) AND slice. URL output is
|
||||
// prefixed with a 6-line header (URL/Content-Type/Method/blank/---/blank), so
|
||||
// body line N appears at output line N+6.
|
||||
const result = await tool.execute("call", { path: "https://example.com/api.json:raw:9-11" });
|
||||
const text =
|
||||
result.content
|
||||
.filter(c => c.type === "text")
|
||||
.map(c => c.text)
|
||||
.join("\n") ?? "";
|
||||
|
||||
expect(result.details?.method).toBe("raw");
|
||||
expect(text).toContain("raw line 3");
|
||||
expect(text).toContain("raw line 5");
|
||||
expect(text).not.toContain("raw line 15");
|
||||
});
|
||||
});
|
||||
|
||||
describe("read URL with multi-range selector (regression: was stuck on URL → 404)", () => {
|
||||
let testDir: string;
|
||||
beforeEach(() => {
|
||||
testDir = path.join(os.tmpdir(), `fetch-multi-range-${Snowflake.next()}`);
|
||||
fs.mkdirSync(testDir, { recursive: true });
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
fs.rmSync(testDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("routes :A-B,C-D to the multi-range builder against the cached body", async () => {
|
||||
const session = makeSession(testDir);
|
||||
const tool = new ReadTool(session);
|
||||
const body = Array.from({ length: 40 }, (_, i) => `content ${i + 1}`).join("\n");
|
||||
const loadSpy = stubLoadPage(body, "text/plain");
|
||||
|
||||
const result = await tool.execute("call", { path: "https://example.com/file.txt:11-13,26-28" });
|
||||
const text = result.content
|
||||
.filter(c => c.type === "text")
|
||||
.map(c => c.text)
|
||||
.join("\n");
|
||||
|
||||
// Body line N is at output line N+6 (URL header prefix). 11-13 = content 5-7,
|
||||
// 26-28 = content 20-22.
|
||||
expect(text).toContain("content 5");
|
||||
expect(text).toContain("content 7");
|
||||
expect(text).toContain("content 20");
|
||||
expect(text).toContain("content 22");
|
||||
// Lines between ranges are elided
|
||||
expect(text).not.toContain("content 14");
|
||||
// Elision marker between blocks
|
||||
expect(text).toContain("…");
|
||||
// The URL itself stays clean — no range selector ever hits the network.
|
||||
expect(loadSpy).toHaveBeenCalledWith("https://example.com/file.txt", expect.anything());
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,106 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { parseReadUrlTarget } from "@oh-my-pi/pi-coding-agent/tools/fetch";
|
||||
|
||||
describe("parseReadUrlTarget", () => {
|
||||
it("returns null for non-URL paths", () => {
|
||||
expect(parseReadUrlTarget("/etc/hosts")).toBeNull();
|
||||
expect(parseReadUrlTarget("relative/file.ts")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns a bare URL with no selectors", () => {
|
||||
expect(parseReadUrlTarget("https://example.com/foo")).toEqual({
|
||||
path: "https://example.com/foo",
|
||||
raw: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("peels :raw", () => {
|
||||
expect(parseReadUrlTarget("https://example.com/foo:raw")).toEqual({
|
||||
path: "https://example.com/foo",
|
||||
raw: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("peels a single line range as offset/limit", () => {
|
||||
expect(parseReadUrlTarget("https://example.com/foo:50-100")).toEqual({
|
||||
path: "https://example.com/foo",
|
||||
raw: false,
|
||||
offset: 50,
|
||||
limit: 51,
|
||||
});
|
||||
expect(parseReadUrlTarget("https://example.com/foo:50+10")).toEqual({
|
||||
path: "https://example.com/foo",
|
||||
raw: false,
|
||||
offset: 50,
|
||||
limit: 10,
|
||||
});
|
||||
expect(parseReadUrlTarget("https://example.com/foo:50")).toEqual({
|
||||
path: "https://example.com/foo",
|
||||
raw: false,
|
||||
offset: 50,
|
||||
});
|
||||
});
|
||||
|
||||
it("peels multi-range selectors into ranges (regression: was stuck on URL → 404)", () => {
|
||||
// Direct repro of bug report 6234.
|
||||
const result = parseReadUrlTarget("https://raw.githubusercontent.com/oven-sh/bun/main/README.md:5-10,20-30");
|
||||
expect(result).toEqual({
|
||||
path: "https://raw.githubusercontent.com/oven-sh/bun/main/README.md",
|
||||
raw: false,
|
||||
ranges: [
|
||||
{ startLine: 5, endLine: 10 },
|
||||
{ startLine: 20, endLine: 30 },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("peels raw + range combos in both orders (regression: was stuck on URL → 404)", () => {
|
||||
// Direct repro of bug report 6230.
|
||||
expect(parseReadUrlTarget("https://example.com/foo:raw:1-120")).toEqual({
|
||||
path: "https://example.com/foo",
|
||||
raw: true,
|
||||
offset: 1,
|
||||
limit: 120,
|
||||
});
|
||||
expect(parseReadUrlTarget("https://example.com/foo:1-120:raw")).toEqual({
|
||||
path: "https://example.com/foo",
|
||||
raw: true,
|
||||
offset: 1,
|
||||
limit: 120,
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects two range groups on the same URL", () => {
|
||||
expect(() => parseReadUrlTarget("https://example.com/foo:5-10:20-30")).toThrow(/range groups/);
|
||||
});
|
||||
|
||||
it("leaves URL ports intact", () => {
|
||||
// `:8080` after the host has no trailing selector character — port stays put.
|
||||
expect(parseReadUrlTarget("https://example.com:8080/foo")).toEqual({
|
||||
path: "https://example.com:8080/foo",
|
||||
raw: false,
|
||||
});
|
||||
// Port + selector combo still works because the selector sits on a path segment.
|
||||
expect(parseReadUrlTarget("https://example.com:8080/foo:raw")).toEqual({
|
||||
path: "https://example.com:8080/foo",
|
||||
raw: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("treats trailing-colon selectors that don't parse as part of the URL", () => {
|
||||
// `:abc` is not a selector token; the parser leaves it on the URL.
|
||||
expect(parseReadUrlTarget("https://example.com/foo:abc")).toEqual({
|
||||
path: "https://example.com/foo:abc",
|
||||
raw: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("supports the documented `host:port/` escape for naked-host selectors", () => {
|
||||
// `https://example.com/:80` is the documented form to read line 80 of the homepage.
|
||||
expect(parseReadUrlTarget("https://example.com/:80")).toEqual({
|
||||
path: "https://example.com/",
|
||||
raw: false,
|
||||
offset: 80,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -85,10 +85,12 @@ describe("multi-path tools tolerate missing entries", () => {
|
||||
});
|
||||
|
||||
const text = getText(result);
|
||||
const details = result.details as { fileCount?: number; missingPaths?: string[] } | undefined;
|
||||
const details = result.details as { fileCount?: number; missingPaths?: string[]; files?: string[] } | undefined;
|
||||
|
||||
expect(text).toContain("src/alpha.ts");
|
||||
expect(text).toContain("src/beta.ts");
|
||||
expect(text).toContain("# src/");
|
||||
expect(text).toContain("alpha.ts");
|
||||
expect(text).toContain("beta.ts");
|
||||
expect(details?.files).toEqual(expect.arrayContaining(["src/alpha.ts", "src/beta.ts"]));
|
||||
expect(text).toContain("Skipped missing paths: tests/**/*.ts");
|
||||
expect(details?.fileCount).toBe(2);
|
||||
expect(details?.missingPaths).toEqual(["tests/**/*.ts"]);
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
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 { 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(),
|
||||
};
|
||||
}
|
||||
|
||||
describe("read tool directory listings honor line selectors (regression: was silently dropping offset)", () => {
|
||||
let testDir: string;
|
||||
let tool: ReadTool;
|
||||
|
||||
beforeEach(() => {
|
||||
testDir = path.join(os.tmpdir(), `read-dir-range-${Snowflake.next()}`);
|
||||
fs.mkdirSync(testDir, { recursive: true });
|
||||
for (let i = 1; i <= 60; i++) {
|
||||
fs.writeFileSync(path.join(testDir, `file-${String(i).padStart(3, "0")}.txt`), "");
|
||||
}
|
||||
tool = new ReadTool(makeSession(testDir));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(testDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("returns the full listing when no selector is given", async () => {
|
||||
const result = await tool.execute("call-bare", { path: testDir });
|
||||
const output = getTextOutput(result);
|
||||
|
||||
expect(result.details?.isDirectory).toBe(true);
|
||||
expect(output).toContain("file-001.txt");
|
||||
expect(output).toContain("file-060.txt");
|
||||
});
|
||||
|
||||
it("returns a slice of the listing for `:start-end`", async () => {
|
||||
const result = await tool.execute("call-range", { path: `${testDir}:30-40` });
|
||||
const output = getTextOutput(result);
|
||||
const lines = output.split("\n").filter(line => line.includes("file-"));
|
||||
|
||||
// 11-line window (30..40 inclusive) — every line in the slice must be a file row.
|
||||
expect(lines.length).toBeLessThanOrEqual(11);
|
||||
// "Use :N to continue" pagination hint when more entries remain.
|
||||
expect(output).toContain("more lines in listing");
|
||||
expect(output).toContain("Use :41 to continue");
|
||||
});
|
||||
|
||||
it("returns from offset to end for `:start`", async () => {
|
||||
const result = await tool.execute("call-offset", { path: `${testDir}:30` });
|
||||
const output = getTextOutput(result);
|
||||
const lines = output.split("\n").filter(line => line.includes("file-"));
|
||||
|
||||
// 60 files + a header line; offset 30 leaves roughly 31 trailing lines.
|
||||
expect(lines.length).toBeGreaterThan(10);
|
||||
// No "continue" footer when we reach EOF.
|
||||
expect(output).not.toContain("Use :");
|
||||
});
|
||||
|
||||
it("emits a clear `beyond end` notice instead of returning an empty body", async () => {
|
||||
const result = await tool.execute("call-beyond", { path: `${testDir}:9999` });
|
||||
const output = getTextOutput(result);
|
||||
|
||||
expect(output).toMatch(/Line 9999 is beyond end of listing/);
|
||||
expect(output).toMatch(/lines total/);
|
||||
});
|
||||
});
|
||||
@@ -478,12 +478,21 @@ describe("tool path arrays", () => {
|
||||
paths: ["apps/", "packages/", "phases/"],
|
||||
});
|
||||
const text = getText(result);
|
||||
const details = result.details as { fileCount?: number; scopePath?: string } | undefined;
|
||||
const details = result.details as { fileCount?: number; scopePath?: string; files?: string[] } | undefined;
|
||||
|
||||
expect(text).toContain("apps/ast.ts");
|
||||
expect(text).toContain("packages/ast.ts");
|
||||
expect(text).toContain("phases/ast.ts");
|
||||
expect(text).toContain("apps/grep.txt");
|
||||
expect(text).toMatch(/^# apps\/\n(?:ast\.ts|grep\.txt)\n(?:ast\.ts|grep\.txt)$/m);
|
||||
expect(text).toMatch(/^# packages\/\n(?:ast\.ts|grep\.txt)\n(?:ast\.ts|grep\.txt)$/m);
|
||||
expect(text).toMatch(/^# phases\/\n(?:ast\.ts|grep\.txt)\n(?:ast\.ts|grep\.txt)$/m);
|
||||
expect(details?.files).toEqual(
|
||||
expect.arrayContaining([
|
||||
"apps/ast.ts",
|
||||
"packages/ast.ts",
|
||||
"phases/ast.ts",
|
||||
"apps/grep.txt",
|
||||
"packages/grep.txt",
|
||||
"phases/grep.txt",
|
||||
]),
|
||||
);
|
||||
expect(text).not.toContain("other/ast.ts");
|
||||
expect(details?.fileCount).toBe(6);
|
||||
expect(details?.scopePath).toBe("apps/, packages/, phases/");
|
||||
@@ -522,11 +531,12 @@ describe("tool path arrays", () => {
|
||||
});
|
||||
const text = getText(result);
|
||||
const expectedPath = path.join(outsideDir, "outside.txt").replace(/\\/g, "/");
|
||||
const details = result.details as { fileCount?: number; scopePath?: string } | undefined;
|
||||
const details = result.details as { fileCount?: number; scopePath?: string; files?: string[] } | undefined;
|
||||
|
||||
expect(text).toContain(expectedPath);
|
||||
expect(text).toContain(`# ${outsideDir.replace(/\\/g, "/")}/\noutside.txt`);
|
||||
expect(text).not.toContain("../");
|
||||
expect(details?.fileCount).toBe(1);
|
||||
expect(details?.files).toEqual([expectedPath]);
|
||||
expect(details?.scopePath).toBe(outsideDir.replace(/\\/g, "/"));
|
||||
} finally {
|
||||
await fs.rm(outsideDir, { recursive: true, force: true });
|
||||
|
||||
Reference in New Issue
Block a user