diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aa7141..deae5d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,13 @@ All notable changes to **basou** are recorded here. The project follows promises this (see Changed). A signal Node has no name for (`SIGEMT` on macOS, the real-time signals on Linux) still reaches basou as an exit code of `0`, so basou still exits `0` for it. +- **`basou decision capture` and `basou review record` no longer print the + path of a `--file` they cannot read.** `Input file not found: ` + carried the path as given, which can be absolute, and most other failures + to read it (a file without read permission, say) passed on Node's own + message, which names the path. They now say `--file names a file that does not exist`, + `--file names a directory, not a file` or `Could not read the file --file + names`, as `basou note` does, and `--verbose` adds the cause's code. ## 0.68.4 — 2026-10-15 diff --git a/packages/cli/src/commands/decision.test.ts b/packages/cli/src/commands/decision.test.ts index c1fd352..2e152f2 100644 --- a/packages/cli/src/commands/decision.test.ts +++ b/packages/cli/src/commands/decision.test.ts @@ -1,5 +1,6 @@ import { execFile } from "node:child_process"; import { + chmod, mkdir, mkdtemp, readdir, @@ -1333,17 +1334,73 @@ describe("doRunDecisionCapture (input validation, agent-facing errors)", () => { expect(process.exitCode).toBe(1); }); - it("cap-err-9: --file that does not exist is rejected without hanging", async () => { + it("cap-err-9: a --file that cannot be read is refused with a fixed message that carries no path", async () => { + const repo = await setupInitedRepo(); + await mkdir(join(repo, "a-directory")); + await writeFile(join(repo, "locked.json"), "[]"); + await chmod(join(repo, "locked.json"), 0o000); + const cases: Array<[string, string]> = [ + [join(repo, "missing.json"), "--file names a file that does not exist"], + [join(repo, "a-directory"), "--file names a directory, not a file"], + ]; + // Root reads a 0o000 file anyway, so the permission case only means + // something for an ordinary user. + if (process.getuid?.() !== 0) { + cases.push([join(repo, "locked.json"), "Could not read the file --file names"]); + } + const err = captureStderr(); + for (const [file, message] of cases) { + err.mockClear(); + process.exitCode = 0; + await runDecisionCapture({ file }, { cwd: repo, nowProvider: () => FIXED_NOW }); + expect(joinCalls(err), file).toBe(message); + expect(process.exitCode, file).toBe(1); + } + const dirs = await readdir(basouPaths(repo).sessions); + expect(dirs.filter((d) => d.startsWith("ses_"))).toEqual([]); + }); + + it("cap-err-9c: --verbose adds the code of the cause, still with no path", async () => { const repo = await setupInitedRepo(); const err = captureStderr(); await runDecisionCapture( - { file: join(repo, "missing.json") }, + { file: join(repo, "missing.json"), verbose: true }, { cwd: repo, nowProvider: () => FIXED_NOW }, ); - expect(joinCalls(err)).toContain("Input file not found:"); + expect(joinCalls(err)).toBe("--file names a file that does not exist\nCaused by: ENOENT"); expect(process.exitCode).toBe(1); }); + it("cap-err-9d: a relative --file is read from the working directory, not the workspace root", async () => { + const repo = await setupInitedRepo(); + const sub = join(repo, "sub"); + await mkdir(sub); + await writeFile( + join(sub, "rel.json"), + JSON.stringify([{ title: "from sub", kind: "decision" }]), + ); + await writeFile( + join(repo, "rel.json"), + JSON.stringify([{ title: "from root", kind: "decision" }]), + ); + captureStdout(); + const previous = process.cwd(); + process.chdir(sub); + try { + await doRunDecisionCapture({ file: "rel.json" }, { cwd: sub, nowProvider: () => FIXED_NOW }); + } finally { + process.chdir(previous); + } + const sid = await findAdHocSessionId(repo); + const events = (await readFile(join(basouPaths(repo).sessions, sid, "events.jsonl"), "utf8")) + .split("\n") + .filter((l) => l.length > 0) + .map((l) => JSON.parse(l) as Record); + expect(events.filter((e) => e.type === "decision_recorded").map((e) => e.title)).toEqual([ + "from sub", + ]); + }); + it("cap-err-9b: --file - is refused as reserved for stdin, before the root lookup and whatever is on disk", async () => { const input = JSON.stringify([{ title: "x", kind: "decision" }]); const readInput = vi.fn(async () => input); diff --git a/packages/cli/src/commands/decision.ts b/packages/cli/src/commands/decision.ts index bb98dc7..0723049 100644 --- a/packages/cli/src/commands/decision.ts +++ b/packages/cli/src/commands/decision.ts @@ -1,4 +1,3 @@ -import { readFile } from "node:fs/promises"; import { homedir } from "node:os"; import { join, resolve } from "node:path"; import { @@ -32,7 +31,7 @@ import { renderCliError, shortSessionId, } from "../lib/error-render.js"; -import { refuseDashInputFile } from "../lib/input-file.js"; +import { readInputFileText, refuseDashInputFile } from "../lib/input-file.js"; import { resolveBasouRootForCommand } from "../lib/repo-root.js"; import { registerDecisionGapsCommand } from "./decision-gaps.js"; @@ -737,14 +736,7 @@ async function readCaptureInput( ctx: DecisionCaptureContext, ): Promise { if (options.file !== undefined) { - try { - return await readFile(options.file, "utf8"); - } catch (error: unknown) { - if (findErrorCode(error, "ENOENT")) { - throw new Error(`Input file not found: ${options.file}`); - } - throw error; - } + return await readInputFileText("--file", options.file); } if (ctx.readInput !== undefined) { return await ctx.readInput(); diff --git a/packages/cli/src/commands/note.ts b/packages/cli/src/commands/note.ts index e7ce38a..4533519 100644 --- a/packages/cli/src/commands/note.ts +++ b/packages/cli/src/commands/note.ts @@ -1,4 +1,3 @@ -import { readFile } from "node:fs/promises"; import { homedir } from "node:os"; import { resolve } from "node:path"; import { @@ -24,7 +23,7 @@ import { renderCliError, shortSessionId, } from "../lib/error-render.js"; -import { refuseDashInputFile } from "../lib/input-file.js"; +import { readInputFileText, refuseDashInputFile } from "../lib/input-file.js"; import { resolveBasouRootForCommand } from "../lib/repo-root.js"; // Single words that are almost certainly a mistyped subcommand rather than a @@ -317,20 +316,7 @@ async function readNoteBody( return { body: bodyArgument, source: "argument" }; } if (options.file !== undefined) { - let raw: string; - try { - raw = await readFile(resolve(cwd, options.file), "utf8"); - } catch (error: unknown) { - // Fixed messages, like `basou task new --from-file`: the error surface - // never carries a path. The cause (shown with --verbose) has the detail. - if (findErrorCode(error, "ENOENT")) { - throw new Error("--file names a file that does not exist", { cause: error }); - } - if (findErrorCode(error, "EISDIR")) { - throw new Error("--file names a directory, not a file", { cause: error }); - } - throw new Error("Could not read the file --file names", { cause: error }); - } + const raw = await readInputFileText("--file", resolve(cwd, options.file)); return { body: dropTrailingNewlines(raw), source: "file" }; } if (ctx.readInput !== undefined) { diff --git a/packages/cli/src/commands/review.test.ts b/packages/cli/src/commands/review.test.ts index 2611b3b..b8df71b 100644 --- a/packages/cli/src/commands/review.test.ts +++ b/packages/cli/src/commands/review.test.ts @@ -1,5 +1,15 @@ import { execFile } from "node:child_process"; -import { mkdtemp, readdir, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises"; +import { + chmod, + mkdir, + mkdtemp, + readdir, + readFile, + realpath, + rm, + symlink, + writeFile, +} from "node:fs/promises"; import { devNull, tmpdir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; @@ -387,6 +397,48 @@ describe("doRunReviewRecord (validation errors surface to the agent)", () => { expect(readInput).not.toHaveBeenCalled(); }); + it("rev-err-6: a --file that cannot be read is refused with a fixed message that carries no path", async () => { + const repo = await setupInitedRepo(); + await mkdir(join(repo, "a-directory")); + await writeFile(join(repo, "locked.json"), "[]"); + await chmod(join(repo, "locked.json"), 0o000); + const cases: Array<[string, string]> = [ + [join(repo, "missing.json"), "--file names a file that does not exist"], + [join(repo, "a-directory"), "--file names a directory, not a file"], + ]; + // Root reads a 0o000 file anyway, so the permission case only means + // something for an ordinary user. + if (process.getuid?.() !== 0) { + cases.push([join(repo, "locked.json"), "Could not read the file --file names"]); + } + for (const [file, message] of cases) { + await expect( + doRunReviewRecord({ file }, { cwd: repo, nowProvider: () => FIXED_NOW }), + file, + ).rejects.toHaveProperty("message", message); + } + const dirs = (await readdir(basouPaths(repo).sessions)).filter((d) => d.startsWith("ses_")); + expect(dirs).toHaveLength(0); + }); + + it("rev-err-7: a relative --file is read from the working directory, not the workspace root", async () => { + const repo = await setupInitedRepo(); + const sub = join(repo, "sub"); + await mkdir(sub); + await writeFile(join(sub, "rev.json"), '{ "reviewer": "from-sub", "target": "wt" }'); + await writeFile(join(repo, "rev.json"), '{ "reviewer": "from-root", "target": "wt" }'); + captureStdout(); + const previous = process.cwd(); + process.chdir(sub); + try { + await doRunReviewRecord({ file: "rev.json" }, { cwd: sub, nowProvider: () => FIXED_NOW }); + } finally { + process.chdir(previous); + } + const reviews = (await readAdHocEvents(repo)).filter((e) => e.type === "review_recorded"); + expect(reviews.map((e) => e.reviewer)).toEqual(["from-sub"]); + }); + it("rev-err-5: a --file path that names a file called - is read", async () => { const repo = await setupInitedRepo(); captureStdout(); diff --git a/packages/cli/src/commands/review.ts b/packages/cli/src/commands/review.ts index 28b64d6..1b0c2d7 100644 --- a/packages/cli/src/commands/review.ts +++ b/packages/cli/src/commands/review.ts @@ -1,4 +1,3 @@ -import { readFile } from "node:fs/promises"; import { homedir } from "node:os"; import { resolve } from "node:path"; import { @@ -24,7 +23,7 @@ import { renderCliError, shortSessionId, } from "../lib/error-render.js"; -import { refuseDashInputFile } from "../lib/input-file.js"; +import { readInputFileText, refuseDashInputFile } from "../lib/input-file.js"; import { resolveBasouRootForCommand } from "../lib/repo-root.js"; /** @@ -260,14 +259,7 @@ async function readReviewInput( ctx: ReviewRecordContext, ): Promise { if (options.file !== undefined) { - try { - return await readFile(options.file, "utf8"); - } catch (error: unknown) { - if (findErrorCode(error, "ENOENT")) { - throw new Error(`Input file not found: ${options.file}`); - } - throw error; - } + return await readInputFileText("--file", options.file); } if (ctx.readInput !== undefined) { return await ctx.readInput(); diff --git a/packages/cli/src/lib/input-file.test.ts b/packages/cli/src/lib/input-file.test.ts index 26330a7..ea0fb3a 100644 --- a/packages/cli/src/lib/input-file.test.ts +++ b/packages/cli/src/lib/input-file.test.ts @@ -1,7 +1,11 @@ +import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { type InputFileAlternative, type InputFileFlag, + readInputFileText, refuseDashInputFile, } from "./input-file.js"; @@ -47,3 +51,45 @@ describe("refuseDashInputFile", () => { } }); }); + +describe("readInputFileText", () => { + it("reads the file, and refuses one it cannot read with a message that names the flag, not the path", async () => { + const dir = await mkdtemp(join(tmpdir(), "basou-input-file-")); + try { + await writeFile(join(dir, "in.json"), "[1]\n"); + await mkdir(join(dir, "a-directory")); + await writeFile(join(dir, "locked.json"), "[]"); + await chmod(join(dir, "locked.json"), 0o000); + expect(await readInputFileText("--file", join(dir, "in.json"))).toBe("[1]\n"); + const cases: Array<[InputFileFlag, string, string, string]> = [ + ["--file", "missing.json", "--file names a file that does not exist", "ENOENT"], + ["--from", "a-directory", "--from names a directory, not a file", "EISDIR"], + ]; + // Root reads a 0o000 file anyway. + if (process.getuid?.() !== 0) { + cases.push([ + "--from-file", + "locked.json", + "Could not read the file --from-file names", + "EACCES", + ]); + } + for (const [flag, name, message, code] of cases) { + const error = await readInputFileText(flag, join(dir, name)).then( + () => undefined, + (e: unknown) => e, + ); + expect(error, name).toBeInstanceOf(Error); + expect((error as Error).message, name).toBe(message); + expect((error as Error).message, name).not.toContain(dir); + // An Error, so that --verbose can print its code (renderCliError reads + // the code only off an Error). + expect((error as Error).cause, name).toBeInstanceOf(Error); + expect(((error as Error).cause as NodeJS.ErrnoException).code, name).toBe(code); + } + } finally { + await chmod(join(dir, "locked.json"), 0o600).catch(() => undefined); + await rm(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/cli/src/lib/input-file.ts b/packages/cli/src/lib/input-file.ts index 0f61703..d4762a2 100644 --- a/packages/cli/src/lib/input-file.ts +++ b/packages/cli/src/lib/input-file.ts @@ -1,3 +1,6 @@ +import { readFile } from "node:fs/promises"; +import { findErrorCode } from "@basou/core"; + /** * The flags that name a file to read a command's input from: `--file` * (`basou decision capture`, `basou review record`, `basou note`), @@ -52,3 +55,23 @@ export function refuseDashInputFile( `${toFile}; a file named - is read with ${flag} ./-.`, ); } + +/** + * Read the text of the file an input-file flag names. A failure is a fixed + * message that names the flag and never the path, per the CLI's pathless error + * contract (a path the caller gave can be absolute); the cause, shown with + * `--verbose` as its code, has the detail. + */ +export async function readInputFileText(flag: InputFileFlag, path: string): Promise { + try { + return await readFile(path, "utf8"); + } catch (error: unknown) { + if (findErrorCode(error, "ENOENT")) { + throw new Error(`${flag} names a file that does not exist`, { cause: error }); + } + if (findErrorCode(error, "EISDIR")) { + throw new Error(`${flag} names a directory, not a file`, { cause: error }); + } + throw new Error(`Could not read the file ${flag} names`, { cause: error }); + } +}