diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d78affddf..77bc0c605 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,15 +1,17 @@ # Changelog ## [Unreleased] - ### Added +- Added support for embedded URL selectors (`:raw` and `:L#-L#` line ranges) in read command paths +- Added `parseReadUrlTarget` function to parse and validate URL read targets with line range support - Added `decl` region to chunk selector for targeting declarations without leading trivia - Exported `hooks` subpath for extensibility API access - Added `build` script for compiling binary artifacts ### Changed +- Updated read CLI to delegate URL inputs through the read tool pipeline instead of treating them as local file paths - Updated chunk edit documentation to clarify region semantics and emphasize using the narrowest region for edits - Improved chunk selector guidance with visual diagram showing region boundaries - Renamed `build:binary` script to `build` diff --git a/packages/coding-agent/src/cli/read-cli.ts b/packages/coding-agent/src/cli/read-cli.ts index 7d2bde50f..12e8aa9b2 100644 --- a/packages/coding-agent/src/cli/read-cli.ts +++ b/packages/coding-agent/src/cli/read-cli.ts @@ -1,21 +1,47 @@ /** * Read CLI command handler. * - * Handles `omp read` subcommand — emits chunk-mode read output for a file. + * Handles `omp read` subcommand — emits chunk-mode read output for files, + * and delegates URL reads through the read tool pipeline. */ import * as path from "node:path"; import chalk from "chalk"; +import { Settings } from "../config/settings"; import { formatChunkedRead, resolveAnchorStyle } from "../edit/modes/chunk"; import { getLanguageFromPath } from "../modes/theme/theme"; +import type { ToolSession } from "../tools"; +import { parseReadUrlTarget } from "../tools/fetch"; +import { ReadTool } from "../tools/read"; export interface ReadCommandArgs { path: string; sel?: string; } -export async function runReadCommand(cmd: ReadCommandArgs): Promise { - const filePath = path.resolve(cmd.path); +function createCliReadSession(cwd: string, settings: Settings): ToolSession { + return { + cwd, + hasUI: false, + hasEditTool: true, + getSessionFile: () => null, + getSessionSpawns: () => null, + settings, + }; +} +export async function runReadCommand(cmd: ReadCommandArgs): Promise { + const cwd = process.cwd(); + const parsedUrlTarget = parseReadUrlTarget(cmd.path, cmd.sel); + if (parsedUrlTarget) { + const settings = await Settings.init({ cwd }); + const tool = new ReadTool(createCliReadSession(cwd, settings)); + const result = await tool.execute("cli-read", { path: cmd.path, sel: cmd.sel }); + const text = result.content.find((content): content is { type: "text"; text: string } => content.type === "text"); + console.log(text?.text ?? ""); + return; + } + + const filePath = path.resolve(cmd.path); const file = Bun.file(filePath); if (!(await file.exists())) { console.error(chalk.red(`Error: File not found: ${cmd.path}`)); @@ -24,7 +50,6 @@ export async function runReadCommand(cmd: ReadCommandArgs): Promise { const readPath = cmd.sel ? `${filePath}:${cmd.sel}` : filePath; const language = getLanguageFromPath(filePath); - const cwd = process.cwd(); try { const result = await formatChunkedRead({ diff --git a/packages/coding-agent/src/tools/fetch.ts b/packages/coding-agent/src/tools/fetch.ts index 72c37997a..5adb2cbcb 100644 --- a/packages/coding-agent/src/tools/fetch.ts +++ b/packages/coding-agent/src/tools/fetch.ts @@ -23,7 +23,7 @@ import { convertWithMarkit, fetchBinary } from "../web/scrapers/utils"; import { applyListLimit } from "./list-limit"; import { formatStyledArtifactReference, type OutputMeta } from "./output-meta"; import { formatExpandHint, getDomain } from "./render-utils"; -import { ToolAbortError } from "./tool-errors"; +import { ToolAbortError, ToolError } from "./tool-errors"; import { toolResult } from "./tool-result"; import { clampTimeout } from "./tool-timeouts"; @@ -137,6 +137,70 @@ export function isReadableUrlPath(value: string): boolean { return /^https?:\/\//i.test(value) || /^www\./i.test(value); } +const URL_LINE_RANGE_RE = /^L(\d+)(?:-L?(\d+))?$/i; + +export interface ParsedReadUrlTarget { + path: string; + raw: boolean; + offset?: number; + limit?: number; +} + +export function parseReadUrlTarget(readPath: string, sel?: string): ParsedReadUrlTarget | null { + const embedded = sel ? undefined : tryExtractEmbeddedUrlSelector(readPath); + const urlPath = embedded?.path ?? readPath; + if (!isReadableUrlPath(urlPath)) { + return null; + } + + const selector = sel ?? embedded?.sel; + const raw = selector === "raw"; + const lineMatch = selector ? URL_LINE_RANGE_RE.exec(selector) : null; + if (lineMatch) { + const startLine = Number.parseInt(lineMatch[1]!, 10); + if (startLine < 1) { + throw new ToolError("L0 is invalid; lines are 1-indexed. Use sel=L1."); + } + const endLine = lineMatch[2] ? Number.parseInt(lineMatch[2], 10) : undefined; + if (endLine !== undefined && endLine < startLine) { + throw new ToolError(`Invalid range L${startLine}-L${endLine}: end must be >= start.`); + } + return { + path: urlPath, + raw: false, + offset: startLine, + limit: endLine !== undefined ? endLine - startLine + 1 : undefined, + }; + } + + return { path: urlPath, raw }; +} + +function tryExtractEmbeddedUrlSelector(readPath: string): { path: string; sel?: string } | null { + const lastColonIndex = readPath.lastIndexOf(":"); + if (lastColonIndex <= 0) { + return null; + } + + const candidateSelector = readPath.slice(lastColonIndex + 1); + const isEmbeddedSelector = candidateSelector === "raw" || URL_LINE_RANGE_RE.test(candidateSelector); + if (!isEmbeddedSelector) { + return null; + } + + const basePath = readPath.slice(0, lastColonIndex); + if (!isReadableUrlPath(basePath)) { + return null; + } + + try { + new URL(basePath.startsWith("http://") || basePath.startsWith("https://") ? basePath : `https://${basePath}`); + return { path: basePath, sel: candidateSelector }; + } catch { + return null; + } +} + /** * Normalize MIME type (lowercase, strip charset/params) */ diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index 380edd2ef..bc4123c1e 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -37,10 +37,12 @@ import { resolveFileDisplayMode } from "../utils/file-display-mode"; import { ImageInputTooLargeError, loadImageInput, MAX_IMAGE_INPUT_BYTES } from "../utils/image-loading"; import { convertFileWithMarkit } from "../utils/markit"; import { type ArchiveReader, openArchive, parseArchivePathCandidates } from "./archive-reader"; + import { executeReadUrl, isReadableUrlPath, loadReadUrlCacheEntry, + parseReadUrlTarget, type ReadUrlToolDetails, renderReadUrlCall, renderReadUrlResult, @@ -727,25 +729,28 @@ export class ReadTool implements AgentTool { return this.#handleInternalUrl(readPath, offset, limit); } - if (isReadableUrlPath(readPath)) { - const parsed = parseSel(sel); + const parsedUrlTarget = parseReadUrlTarget(readPath, sel); + if (parsedUrlTarget) { if (!this.session.settings.get("fetch.enabled")) { throw new ToolError("URL reads are disabled by settings."); } - const raw = parsed.kind === "raw"; - const { offset, limit } = selToOffsetLimit(parsed); - if (offset !== undefined || limit !== undefined) { - const cached = await loadReadUrlCacheEntry(this.session, { path: readPath, timeout, raw }, signal, { - ensureArtifact: true, - preferCached: true, - }); - return this.#buildInMemoryTextResult(cached.output, offset, limit, { + if (parsedUrlTarget.offset !== undefined || parsedUrlTarget.limit !== undefined) { + const cached = await loadReadUrlCacheEntry( + this.session, + { path: parsedUrlTarget.path, timeout, raw: parsedUrlTarget.raw }, + signal, + { + ensureArtifact: true, + preferCached: true, + }, + ); + return this.#buildInMemoryTextResult(cached.output, parsedUrlTarget.offset, parsedUrlTarget.limit, { details: { ...cached.details }, sourceUrl: cached.details.finalUrl, entityLabel: "URL output", }); } - return executeReadUrl(this.session, { path: readPath, timeout, raw }, signal); + return executeReadUrl(this.session, { path: parsedUrlTarget.path, timeout, raw: parsedUrlTarget.raw }, signal); } const parsedReadPath = chunkMode ? parseChunkReadPath(readPath) : { filePath: readPath }; @@ -858,7 +863,7 @@ export class ReadTool implements AgentTool { } // Read the file based on type - let content: (TextContent | ImageContent)[]; + let content: Array; let details: ReadToolDetails = {}; let sourcePath: string | undefined; let truncationInfo: @@ -967,11 +972,13 @@ export class ReadTool implements AgentTool { // Raw text or line-range mode const { offset, limit } = selToOffsetLimit(parsed); const startLine = offset ? Math.max(0, offset - 1) : 0; - const startLineDisplay = startLine + 1; // For display (1-indexed) + const startLineDisplay = startLine + 1; - const effectiveLimit = limit ?? this.#defaultLimit; + const DEFAULT_LIMIT = this.#defaultLimit; + const effectiveLimit = limit ?? DEFAULT_LIMIT; const maxLinesToCollect = Math.min(effectiveLimit, DEFAULT_MAX_LINES); const selectedLineLimit = effectiveLimit; + const streamResult = await streamLinesFromFile( absolutePath, startLine, diff --git a/packages/coding-agent/test/cli/read-cli.test.ts b/packages/coding-agent/test/cli/read-cli.test.ts new file mode 100644 index 000000000..34ec5fb46 --- /dev/null +++ b/packages/coding-agent/test/cli/read-cli.test.ts @@ -0,0 +1,33 @@ +import { afterEach, describe, expect, it, vi } from "bun:test"; +import * as os from "node:os"; +import * as path from "node:path"; +import { runReadCommand } from "@oh-my-pi/pi-coding-agent/cli/read-cli"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import * as scrapers from "@oh-my-pi/pi-coding-agent/web/scrapers/types"; + +describe("runReadCommand URL handling", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("delegates URL inputs through the read tool pipeline", async () => { + const cwd = path.join(os.tmpdir(), "read-cli-url-test"); + const settings = Settings.isolated({ "fetch.enabled": true }); + const pageUrl = "https://example.com/cli-read"; + const consoleLogSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + vi.spyOn(Settings, "init").mockResolvedValue(settings); + vi.spyOn(scrapers, "loadPage").mockResolvedValue({ + ok: true, + status: 200, + contentType: "text/plain", + finalUrl: pageUrl, + content: "CLI URL content", + }); + const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(cwd); + + await runReadCommand({ path: pageUrl }); + + expect(cwdSpy).toHaveBeenCalled(); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining("CLI URL content")); + }); +}); diff --git a/packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts b/packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts index dc6ef5a0f..31d5a618e 100644 --- a/packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts +++ b/packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts @@ -21,6 +21,97 @@ const withMissingSystemPython = () => { }; }; +describe("read tool URL selector shorthands", () => { + let testDir: string; + + beforeEach(() => { + testDir = path.join(os.tmpdir(), `fetch-kagi-toggle-shorthand-${Snowflake.next()}`); + fs.mkdirSync(testDir, { recursive: true }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + const createSession = (): 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, + }), + }; + }; + + it("supports embedded raw selectors in URL paths", async () => { + const session = createSession(); + const tool = new ReadTool(session); + const pageUrl = "https://example.com/embedded-raw"; + const loadPageSpy = vi.spyOn(scrapers, "loadPage").mockImplementation(async requestedUrl => { + if (requestedUrl !== pageUrl) { + throw new Error(`Unexpected URL: ${requestedUrl}`); + } + return { + ok: true, + status: 200, + contentType: "text/html", + finalUrl: pageUrl, + content: "

Embedded raw page

", + }; + }); + + const result = await tool.execute("fetch-embedded-raw", { path: `${pageUrl}:raw` }); + const textBlock = result.content.find(content => content.type === "text"); + + expect(result.details?.method).toBe("raw"); + expect(textBlock?.type).toBe("text"); + expect(textBlock?.text).toContain("

Embedded raw page

"); + expect(loadPageSpy).toHaveBeenCalledWith(pageUrl, expect.anything()); + }); + + it("supports embedded line selectors in URL paths", async () => { + const session = createSession(); + const tool = new ReadTool(session); + const pageUrl = "https://example.com/embedded-lines"; + const loadPageSpy = vi.spyOn(scrapers, "loadPage").mockImplementation(async requestedUrl => { + if (requestedUrl !== pageUrl) { + throw new Error(`Unexpected URL: ${requestedUrl}`); + } + return { + ok: true, + status: 200, + contentType: "text/plain", + finalUrl: pageUrl, + content: "Line 1\nLine 2\nLine 3", + }; + }); + + const result = await tool.execute("fetch-embedded-lines", { path: `${pageUrl}:L7-L8` }); + const textBlock = result.content.find(content => content.type === "text"); + + expect(textBlock?.type).toBe("text"); + expect(textBlock?.text).toContain("Line 1"); + expect(textBlock?.text).toContain("Line 2"); + expect(textBlock?.text).not.toContain("Line 3"); + expect(loadPageSpy).toHaveBeenCalledTimes(1); + expect(loadPageSpy).toHaveBeenCalledWith(pageUrl, expect.anything()); + }); +}); + describe("read tool URL handling", () => { let testDir: string;