From f051cc6a0ef7707902f058bc054105a76aef171a Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 27 May 2026 15:03:12 +0200 Subject: [PATCH] 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. --- packages/coding-agent/CHANGELOG.md | 15 ++ packages/coding-agent/src/tools/fetch.ts | 137 +++++++++------ packages/coding-agent/src/tools/read.ts | 60 ++++++- .../test/tools/fetch-raw-mode.test.ts | 161 ++++++++++++++++++ .../test/tools/fetch-url-selectors.test.ts | 106 ++++++++++++ .../test/tools/multi-path-missing.test.ts | 8 +- .../test/tools/read-directory-range.test.ts | 88 ++++++++++ .../test/tools/search-path-lists.test.ts | 24 ++- 8 files changed, 538 insertions(+), 61 deletions(-) create mode 100644 packages/coding-agent/test/tools/fetch-raw-mode.test.ts create mode 100644 packages/coding-agent/test/tools/fetch-url-selectors.test.ts create mode 100644 packages/coding-agent/test/tools/read-directory-range.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index dabed2b50..d74647c46 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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 diff --git a/packages/coding-agent/src/tools/fetch.ts b/packages/coding-agent/src/tools/fetch.ts index 76b07ecf4..e1a15c0b8 100644 --- a/packages/coding-agent/src/tools/fetch.ts +++ b/packages/coding-agent/src/tools/fetch.ts @@ -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 { diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index 04af33cca..cda3bc23a 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -1488,6 +1488,21 @@ export class ReadTool implements AgentTool { 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 { details: { ...cached.details }, sourceUrl: cached.details.finalUrl, entityLabel: "URL output", + raw: parsedUrlTarget.raw, immutable: true, }); } @@ -1578,7 +1594,8 @@ export class ReadTool implements AgentTool { 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 { /** Read directory contents as a formatted listing */ async #readDirectory( absolutePath: string, + offset: number | undefined, limit: number | undefined, signal?: AbortSignal, ): Promise> { @@ -2149,7 +2167,9 @@ export class ReadTool implements AgentTool { 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 { 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 }); diff --git a/packages/coding-agent/test/tools/fetch-raw-mode.test.ts b/packages/coding-agent/test/tools/fetch-raw-mode.test.ts new file mode 100644 index 000000000..64724615d --- /dev/null +++ b/packages/coding-agent/test/tools/fetch-raw-mode.test.ts @@ -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 = `\nSampleOne12024-01-01T00:00:00Zbody`; +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(''); + expect(textBlock?.text).toContain(""); + 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()); + }); +}); diff --git a/packages/coding-agent/test/tools/fetch-url-selectors.test.ts b/packages/coding-agent/test/tools/fetch-url-selectors.test.ts new file mode 100644 index 000000000..1b2f7c583 --- /dev/null +++ b/packages/coding-agent/test/tools/fetch-url-selectors.test.ts @@ -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, + }); + }); +}); diff --git a/packages/coding-agent/test/tools/multi-path-missing.test.ts b/packages/coding-agent/test/tools/multi-path-missing.test.ts index 9d22be15d..ab7d3ad10 100644 --- a/packages/coding-agent/test/tools/multi-path-missing.test.ts +++ b/packages/coding-agent/test/tools/multi-path-missing.test.ts @@ -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"]); diff --git a/packages/coding-agent/test/tools/read-directory-range.test.ts b/packages/coding-agent/test/tools/read-directory-range.test.ts new file mode 100644 index 000000000..17aacfa5e --- /dev/null +++ b/packages/coding-agent/test/tools/read-directory-range.test.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/); + }); +}); diff --git a/packages/coding-agent/test/tools/search-path-lists.test.ts b/packages/coding-agent/test/tools/search-path-lists.test.ts index 5720f6be1..f1adebfd1 100644 --- a/packages/coding-agent/test/tools/search-path-lists.test.ts +++ b/packages/coding-agent/test/tools/search-path-lists.test.ts @@ -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 });