diff --git a/packages/cli/src/commands/history.test.ts b/packages/cli/src/commands/history.test.ts index e0994fc2a80..3734b7d447f 100644 --- a/packages/cli/src/commands/history.test.ts +++ b/packages/cli/src/commands/history.test.ts @@ -1,6 +1,8 @@ // @vitest-environment node import { spawn } from "node:child_process"; import { + chmodSync, + existsSync, mkdirSync, mkdtempSync, readdirSync, @@ -26,7 +28,7 @@ import { runCommand } from "citty"; import { Hono } from "hono"; import { afterEach, describe, expect, it, onTestFinished, vi } from "vitest"; import { consumeCommandResult } from "../utils/commandResult.js"; -import { historyDeps, withOwner } from "../utils/historyOwner.js"; +import { historyDeps, withOwner, withReader } from "../utils/historyOwner.js"; import historyCommand from "./history.js"; /** History's clock, a day ahead of the file system's, so each write is dated by the mtime it is stamped with. */ @@ -710,3 +712,126 @@ describe("hyperframes history, refusals", () => { expect([refused.code, refused.err]).toEqual([2, "That point is no longer kept"]); }); }); + +describe.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "hyperframes history, a history this run cannot write", + () => { + /** The project's history folder (or a folder in it) set to `mode` for the rest of the test. */ + function lock(mode: number, ...parts: string[]) { + const [id] = readdirSync(historyDeps.historyRoot).filter((name) => + existsSync(join(historyDeps.historyRoot, name, "log.jsonl")), + ); + const path = join(historyDeps.historyRoot, id!, ...parts); + const dirs = [ + path, + ...readdirSync(path, { recursive: true, withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => join(entry.parentPath, entry.name)), + ]; + for (const dir of dirs.reverse()) chmodSync(dir, mode); + onTestFinished(() => dirs.forEach((dir) => chmodSync(dir, 0o755))); + } + + it("lists the history as last recorded when its folder cannot be written, and says so", async () => { + const { hf, turn } = project(); + await hf(); + const entry = await turn("claude", "Retitle", "index.html", "A2"); + lock(0o555); + const listed = await hf("--json"); + expect(listed.code).toBe(0); + expect(JSON.parse(listed.out).entries.map((item: { id: string }) => item.id)).toContain( + entry.id, + ); + expect(listed.err).toMatch(/could not write this project's history/i); + }); + + it("lists the history when its log can be read but not appended to", async () => { + const { hf, write, turn } = project(); + await hf(); + const entry = await turn("claude", "Retitle", "index.html", "A2"); + const [id] = readdirSync(historyDeps.historyRoot).filter((name) => + existsSync(join(historyDeps.historyRoot, name, "log.jsonl")), + ); + const log = join(historyDeps.historyRoot, id!, "log.jsonl"); + chmodSync(log, 0o444); + onTestFinished(() => chmodSync(log, 0o644)); + write("notes.html", "N2"); + const listed = await hf("--json"); + expect(listed.code).toBe(0); + expect(JSON.parse(listed.out).entries.map((item: { id: string }) => item.id)).toContain( + entry.id, + ); + expect(listed.err).toMatch(/could not write this project's history/i); + }); + + it("lists the history when only its stored file copies cannot be written", async () => { + const { hf, write, turn } = project(); + await hf(); + const entry = await turn("claude", "Retitle", "index.html", "A2"); + lock(0o555, "blobs"); + write("notes.html", "N2"); + const listed = await hf("--json"); + expect(listed.code).toBe(0); + expect(JSON.parse(listed.out).entries.map((item: { id: string }) => item.id)).toContain( + entry.id, + ); + }); + + it("never runs a read twice: a write error after it started is the command's own", async () => { + const { dir, hf } = project(); + await hf(); + let runs = 0; + const unwritable = Object.assign(new Error("no space"), { code: "ENOSPC", syscall: "write" }); + await expect( + withReader(dir, async () => { + runs += 1; + throw unwritable; + }), + ).rejects.toBe(unwritable); + expect(runs).toBe(1); + }); + + it("a folder it cannot read fails the read with that error, never as one it could not write", async () => { + const { hf, turn } = project(); + await hf(); + await turn("claude", "Retitle", "index.html", "A2"); + lock(0o111, "blobs"); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + await expect(hf("--json")).rejects.toMatchObject({ code: "EACCES", syscall: "scandir" }); + expect(error.mock.calls.flat().join(" ")).not.toMatch(/could not write/i); + } finally { + error.mockRestore(); + } + }); + + it("an owner file it cannot read fails the read with that error, never as one it could not write", async () => { + const { hf, turn } = project(); + await hf(); + await turn("claude", "Retitle", "index.html", "A2"); + const [id] = readdirSync(historyDeps.historyRoot).filter((name) => + existsSync(join(historyDeps.historyRoot, name, "log.jsonl")), + ); + const owner = join(historyDeps.historyRoot, id!, "owner.pid"); + writeFileSync(owner, "999999999"); + chmodSync(owner, 0o000); + onTestFinished(() => chmodSync(owner, 0o644)); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + await expect(hf("--json")).rejects.toMatchObject({ code: "EACCES", syscall: "open" }); + expect(error.mock.calls.flat().join(" ")).not.toMatch(/could not write/i); + } finally { + error.mockRestore(); + } + }); + + it("still fails an undo it cannot record, and leaves the files as they are", async () => { + const { hf, read, turn } = project(); + await hf(); + await turn("claude", "Retitle", "index.html", "A2"); + lock(0o555); + await expect(hf("undo", "--who", "claude")).rejects.toMatchObject({ code: "EACCES" }); + expect(read("index.html")).toBe("A2"); + }); + }, +); diff --git a/packages/cli/src/utils/historyOwner.ts b/packages/cli/src/utils/historyOwner.ts index 735eae6b9fe..0a1e424e003 100644 --- a/packages/cli/src/utils/historyOwner.ts +++ b/packages/cli/src/utils/historyOwner.ts @@ -1,5 +1,13 @@ import { randomUUID } from "node:crypto"; -import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { + accessSync, + constants, + mkdirSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from "node:fs"; import { dirname, join, resolve } from "node:path"; import { DEFAULT_HISTORY_ROOT, @@ -261,16 +269,58 @@ export async function withOwner( } } +const CANNOT_WRITE = new Set(["ENOSPC", "EDQUOT", "EACCES", "EPERM", "EROFS"]); +const WRITE_CALLS = new Set([ + "mkdir", + "write", + "rename", + "unlink", + "link", + "copyfile", + "rmdir", + "symlink", + "utime", + "ftruncate", + "fsync", +]); + +function openedForWriting(path: string): boolean { + try { + accessSync(path, constants.R_OK); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === "ENOENT"; + } +} + +/** A write the history could not make, never a read: a denied `open` of a missing or readable file was a write. */ +function failedWrite(error: unknown): boolean { + const { code = "", syscall = "", path } = error as NodeJS.ErrnoException; + if (!CANNOT_WRITE.has(code)) return false; + return ( + WRITE_CALLS.has(syscall) || (syscall === "open" && path !== undefined && openedForWriting(path)) + ); +} + export async function withReader( dir: string | undefined, task: (reader: Reader) => Promise, ): Promise { + let ran = false; try { - return await withOwner(dir, (owner) => task(owner)); + return await withOwner(dir, (owner) => { + ran = true; + return task(owner); + }); } catch (error) { - if (!(error instanceof HistoryBusyError)) throw error; + const unwritable = failedWrite(error); + if (ran || !(error instanceof HistoryBusyError || unwritable)) throw error; + if (unwritable) + console.error( + `[hyperframes] Could not write this project's history (${(error as Error).message}); showing it as last recorded.`, + ); } - // Busy is only thrown on opening, so the task has not run; an open turn's marker is left as it is. + // Opening failed, so the task has not run; an open turn's marker is left as it is. const view = readProjectHistory({ projectDir: resolveProject(dir).dir, historyRoot: historyDeps.historyRoot,