Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: <path>`
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

Expand Down
63 changes: 60 additions & 3 deletions packages/cli/src/commands/decision.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { execFile } from "node:child_process";
import {
chmod,
mkdir,
mkdtemp,
readdir,
Expand Down Expand Up @@ -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<string, unknown>);
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);
Expand Down
12 changes: 2 additions & 10 deletions packages/cli/src/commands/decision.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { readFile } from "node:fs/promises";
import { homedir } from "node:os";
import { join, resolve } from "node:path";
import {
Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -737,14 +736,7 @@ async function readCaptureInput(
ctx: DecisionCaptureContext,
): Promise<string> {
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();
Expand Down
18 changes: 2 additions & 16 deletions packages/cli/src/commands/note.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { readFile } from "node:fs/promises";
import { homedir } from "node:os";
import { resolve } from "node:path";
import {
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
54 changes: 53 additions & 1 deletion packages/cli/src/commands/review.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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();
Expand Down
12 changes: 2 additions & 10 deletions packages/cli/src/commands/review.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { readFile } from "node:fs/promises";
import { homedir } from "node:os";
import { resolve } from "node:path";
import {
Expand All @@ -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";

/**
Expand Down Expand Up @@ -260,14 +259,7 @@ async function readReviewInput(
ctx: ReviewRecordContext,
): Promise<string> {
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();
Expand Down
46 changes: 46 additions & 0 deletions packages/cli/src/lib/input-file.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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 });
}
});
});
23 changes: 23 additions & 0 deletions packages/cli/src/lib/input-file.ts
Original file line number Diff line number Diff line change
@@ -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`),
Expand Down Expand Up @@ -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<string> {
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 });
}
}
Loading