diff --git a/packages/coding-agent/src/cli/file-processor.ts b/packages/coding-agent/src/cli/file-processor.ts index 4b3bf0e20fe..19220514158 100644 --- a/packages/coding-agent/src/cli/file-processor.ts +++ b/packages/coding-agent/src/cli/file-processor.ts @@ -2,11 +2,11 @@ * Process @file CLI arguments into text content and image attachments */ -import { access, readFile, stat } from "node:fs/promises"; +import { readFile, stat } from "node:fs/promises"; import type { ImageContent } from "@earendil-works/pi-ai"; import chalk from "chalk"; import { resolve } from "path"; -import { resolveReadPath } from "../core/tools/path-utils.ts"; +import { pathExists, resolveReadPath } from "../core/tools/path-utils.ts"; import { processImage } from "../utils/image-process.ts"; import { detectSupportedImageMimeTypeFromFile } from "../utils/mime.ts"; @@ -20,6 +20,49 @@ export interface ProcessFileOptions { autoResizeImages?: boolean; } +interface LineRange { + start: number; + end: number; +} + +interface ResolvedFileArgument { + absolutePath: string; + lineRange?: LineRange; +} + +const LINE_RANGE_SUFFIX = /#L(\d+)-L(\d+)$/; + +function exitWithError(message: string): never { + console.error(chalk.red(`Error: ${message}`)); + process.exit(1); +} + +async function resolveFileArgument(fileArg: string): Promise { + const match = LINE_RANGE_SUFFIX.exec(fileArg); + const literalPath = resolve(resolveReadPath(fileArg, process.cwd())); + const hasFileUrlRange = fileArg.startsWith("file://") && match !== null; + if (!hasFileUrlRange && (await pathExists(literalPath))) { + return { absolutePath: literalPath }; + } + + if (!match || match.index === 0) { + exitWithError(`File not found: ${literalPath}`); + } + + const start = Number(match[1]); + const end = Number(match[2]); + if (!Number.isSafeInteger(start) || !Number.isSafeInteger(end) || start < 1 || end < start) { + exitWithError(`Invalid line range: #L${match[1]}-L${match[2]}`); + } + + const basePath = resolve(resolveReadPath(fileArg.slice(0, match.index), process.cwd())); + if (!(await pathExists(basePath))) { + exitWithError(`File not found: ${basePath}`); + } + + return { absolutePath: basePath, lineRange: { start, end } }; +} + /** Process @file arguments into text content and image attachments */ export async function processFileArguments(fileArgs: string[], options?: ProcessFileOptions): Promise { const autoResizeImages = options?.autoResizeImages ?? true; @@ -27,20 +70,11 @@ export async function processFileArguments(fileArgs: string[], options?: Process const images: ImageContent[] = []; for (const fileArg of fileArgs) { - // Expand and resolve path (handles ~ expansion and macOS screenshot Unicode spaces) - const absolutePath = resolve(resolveReadPath(fileArg, process.cwd())); - - // Check if file exists - try { - await access(absolutePath); - } catch { - console.error(chalk.red(`Error: File not found: ${absolutePath}`)); - process.exit(1); - } + const { absolutePath, lineRange } = await resolveFileArgument(fileArg); // Check if file is empty const stats = await stat(absolutePath); - if (stats.size === 0) { + if (stats.size === 0 && !lineRange) { // Skip empty files continue; } @@ -48,6 +82,10 @@ export async function processFileArguments(fileArgs: string[], options?: Process const mimeType = await detectSupportedImageMimeTypeFromFile(absolutePath); if (mimeType) { + if (lineRange) { + exitWithError(`Line ranges are only supported for text files: ${absolutePath}`); + } + // Handle image file const content = await readFile(absolutePath); const processed = await processImage(content, mimeType, { autoResizeImages }); @@ -72,13 +110,29 @@ export async function processFileArguments(fileArgs: string[], options?: Process } } else { // Handle text file + let content: string; try { - const content = await readFile(absolutePath, "utf-8"); - text += `\n${content}\n\n`; + content = await readFile(absolutePath, "utf-8"); } catch (error: unknown) { const message = error instanceof Error ? error.message : String(error); - console.error(chalk.red(`Error: Could not read file ${absolutePath}: ${message}`)); - process.exit(1); + exitWithError(`Could not read file ${absolutePath}: ${message}`); + } + + if (lineRange) { + const lines = content.split("\n"); + if (lines.length > 1 && lines.at(-1) === "") { + lines.pop(); + } + if (lineRange.start > lines.length) { + exitWithError( + `Line range start ${lineRange.start} is beyond end of file (${lines.length} lines total): ${absolutePath}`, + ); + } + const effectiveEnd = Math.min(lineRange.end, lines.length); + const selectedContent = lines.slice(lineRange.start - 1, effectiveEnd).join("\n"); + text += `\n${selectedContent}\n\n`; + } else { + text += `\n${content}\n\n`; } } } diff --git a/packages/coding-agent/test/suite/regressions/7673-file-line-ranges.test.ts b/packages/coding-agent/test/suite/regressions/7673-file-line-ranges.test.ts new file mode 100644 index 00000000000..2015b00e7db --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/7673-file-line-ranges.test.ts @@ -0,0 +1,161 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { fauxAssistantMessage } from "@earendil-works/pi-ai"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { processFileArguments } from "../../../src/cli/file-processor.ts"; +import { createHarness, getUserTexts, type Harness } from "../harness.ts"; + +const TINY_PNG_BASE64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8DwHwAFBQIAX8jx0gAAAABJRU5ErkJggg=="; + +class ProcessExitError extends Error { + readonly code: string | number | null | undefined; + + constructor(code: string | number | null | undefined) { + super(`process.exit(${code})`); + this.code = code; + } +} + +describe("issue #7673 CLI file line ranges", () => { + let testDir: string; + let consoleError: ReturnType; + let harness: Harness; + + beforeEach(async () => { + testDir = mkdtempSync(join(tmpdir(), "pi-7673-")); + consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(process, "exit").mockImplementation(((code?: string | number | null) => { + throw new ProcessExitError(code); + }) as typeof process.exit); + harness = await createHarness(); + }); + + afterEach(() => { + harness.cleanup(); + vi.restoreAllMocks(); + rmSync(testDir, { recursive: true, force: true }); + }); + + async function processAndPrompt(fileArg: string) { + const result = await processFileArguments([fileArg]); + harness.setResponses([fauxAssistantMessage("done")]); + await harness.session.prompt(result.text); + expect(getUserTexts(harness)).toEqual([result.text]); + return result; + } + + async function expectFatal(fileArg: string, message: string): Promise { + await expect(processFileArguments([fileArg])).rejects.toMatchObject({ + code: 1, + }); + expect(consoleError.mock.calls.flat().join("\n")).toContain(message); + } + + it("includes the requested 1-based inclusive range", async () => { + const filePath = join(testDir, "example.txt"); + writeFileSync(filePath, "one\ntwo\nthree\nfour"); + + const result = await processAndPrompt(`${filePath}#L2-L3`); + + expect(result.images).toEqual([]); + expect(result.text).toBe(`\ntwo\nthree\n\n`); + }); + + it("parses a line range from a file URL fragment", async () => { + const filePath = join(testDir, "url.txt"); + writeFileSync(filePath, "one\ntwo\nthree\nfour"); + + const result = await processAndPrompt(`${pathToFileURL(filePath).href}#L2-L3`); + + expect(result.text).toBe(`\ntwo\nthree\n\n`); + }); + + it("prefers an existing literal path over parsing a range suffix", async () => { + const basePath = join(testDir, "example.txt"); + const literalPath = `${basePath}#L2-L3`; + writeFileSync(basePath, "base one\nbase two\nbase three"); + writeFileSync(literalPath, "literal filename"); + + const result = await processAndPrompt(literalPath); + + expect(result.text).toBe(`\nliteral filename\n\n`); + }); + + it("clamps the end to EOF and reports the effective range", async () => { + const filePath = join(testDir, "short.txt"); + writeFileSync(filePath, "one\ntwo\nthree"); + + const result = await processAndPrompt(`${filePath}#L2-L99`); + + expect(result.text).toBe(`\ntwo\nthree\n\n`); + }); + + it("preserves CRLF line endings without counting the terminator as another line", async () => { + const filePath = join(testDir, "windows.txt"); + writeFileSync(filePath, "one\r\ntwo\r\nthree\r\nfour\r\n"); + + const result = await processAndPrompt(`${filePath}#L2-L99`); + + expect(result.text).toBe(`\ntwo\r\nthree\r\nfour\r\n\n`); + }); + + it("does not count a terminating LF as an extra line", async () => { + const filePath = join(testDir, "terminated.txt"); + writeFileSync(filePath, "one\ntwo\nthree\n"); + + const result = await processAndPrompt(`${filePath}#L2-L99`); + + expect(result.text).toBe(`\ntwo\nthree\n\n`); + await expectFatal(`${filePath}#L4-L4`, "Line range start 4 is beyond end of file (3 lines total)"); + }); + + it("leaves whole-file references unchanged", async () => { + const filePath = join(testDir, "whole.txt"); + writeFileSync(filePath, "one\ntwo"); + + const result = await processAndPrompt(filePath); + + expect(result.text).toBe(`\none\ntwo\n\n`); + }); + + it("treats an empty ranged file as an empty first line", async () => { + const filePath = join(testDir, "empty.txt"); + writeFileSync(filePath, ""); + + const result = await processAndPrompt(`${filePath}#L1-L1`); + + expect(result.text).toBe(`\n\n\n`); + }); + + it("rejects a range starting after the empty file's first line", async () => { + const filePath = join(testDir, "empty.txt"); + writeFileSync(filePath, ""); + + await expectFatal(`${filePath}#L2-L2`, "Line range start 2 is beyond end of file (1 lines total)"); + }); + + it("rejects non-positive and reversed ranges", async () => { + const filePath = join(testDir, "invalid.txt"); + writeFileSync(filePath, "one\ntwo\nthree"); + + await expectFatal(`${filePath}#L0-L2`, "Invalid line range: #L0-L2"); + await expectFatal(`${filePath}#L3-L2`, "Invalid line range: #L3-L2"); + }); + + it("rejects a range starting beyond EOF", async () => { + const filePath = join(testDir, "short.txt"); + writeFileSync(filePath, "one\ntwo\nthree"); + + await expectFatal(`${filePath}#L4-L5`, "Line range start 4 is beyond end of file (3 lines total)"); + }); + + it("rejects line ranges for images", async () => { + const filePath = join(testDir, "image.png"); + writeFileSync(filePath, Buffer.from(TINY_PNG_BASE64, "base64")); + + await expectFatal(`${filePath}#L1-L2`, "Line ranges are only supported for text files"); + }); +});