diff --git a/packages/cli/src/commands/history.test.ts b/packages/cli/src/commands/history.test.ts index e254f680e58..4b448f94a49 100644 --- a/packages/cli/src/commands/history.test.ts +++ b/packages/cli/src/commands/history.test.ts @@ -56,6 +56,7 @@ function project() { historyDeps.historyRoot = tempDir("hf-history-cli-root-"); historyDeps.findServer = async () => null; historyDeps.turnIdleMs = 60_000; + historyDeps.ownerWaitMs = undefined; clock.at = Date.now() + 86_400_000; historyDeps.now = () => clock.at; const write = (path: string, text: string) => { @@ -524,15 +525,63 @@ describe("hyperframes history, refusals", () => { expect(readdirSync(historyDeps.historyRoot)).toEqual([]); }); - it("exits 2 with the holder's pid when another process keeps the history past the wait", async () => { - const { dir, hf } = project(); - await openProjectHistory({ projectDir: dir, historyRoot: historyDeps.historyRoot }).then( - (held) => onTestFinished(() => held.close()), - ); - const busy = await hf(); - expect([busy.code, busy.err]).toEqual([ + it("reads the history while another app keeps it open, and refuses to change it there", async () => { + const { dir, read, hf, turn } = project(); + await hf(); + const first = await turn("claude", "Retitle", "index.html", "A2"); + await turn("claude", "Again", "index.html", "A3"); + await hf("begin", "--who", "codex", "--label", "Open turn"); + const marker = () => readFileSync(join(dir, ".hyperframes", "history-turn.json"), "utf-8"); + const before = marker(); + const held = await openProjectHistory({ + projectDir: dir, + historyRoot: historyDeps.historyRoot, + }); + onTestFinished(() => held.close()); + historyDeps.ownerWaitMs = 0; + + const listed = await hf("--json", "--limit", "2"); + expect(listed.code).toBe(0); + const labels = JSON.parse(listed.out).entries.map((entry: { label: string }) => entry.label); + expect(labels).toEqual(["Again", "Retitle"]); + expect((await hf("show", first.id)).out).toContain("M index.html"); + const stdout = vi.spyOn(process.stdout, "write").mockImplementation(() => true); + await hf("peek", first.id, "index.html"); + expect(String(stdout.mock.calls[0]?.[0])).toBe("A2"); + stdout.mockRestore(); + expect(marker()).toBe(before); + + const refused = await hf("undo", first.id); + expect([refused.code, refused.err]).toEqual([ 2, - `This project's history is open in another process (pid ${process.pid}).`, + `This project is open in another app (pid ${process.pid}), which keeps its history: undo, restore or pin there.`, ]); - }, 15_000); + expect(read("index.html")).toBe("A3"); + }); + + it("still refuses a history id it cannot read, instead of reading the history as empty", async () => { + const { dir, hf } = project(); + mkdirSync(join(dir, ".hyperframes"), { recursive: true }); + writeFileSync(join(dir, ".hyperframes", "history-id"), "garbage\n"); + const refused = await hf(); + expect(refused.code).toBe(2); + expect(refused.err).toContain("holds no history id this version can read"); + }); + + it("peek refuses cleanly when another app pruned the point's file after its log was read", async () => { + const { dir, hf, turn } = project(); + await hf(); + const first = await turn("claude", "Retitle", "index.html", "A2"); + const held = await openProjectHistory({ + projectDir: dir, + historyRoot: historyDeps.historyRoot, + }); + onTestFinished(() => held.close()); + historyDeps.ownerWaitMs = 0; + const hash = held.peek(first.id)!["index.html"]!; + rmSync(join(historyDeps.historyRoot, held.projectId, "blobs", hash.slice(0, 2), hash)); + + const refused = await hf("peek", first.id, "index.html"); + expect([refused.code, refused.err]).toEqual([2, "That point is no longer kept"]); + }); }); diff --git a/packages/cli/src/commands/history.ts b/packages/cli/src/commands/history.ts index e0e0c24c7af..b4ab0352a96 100644 --- a/packages/cli/src/commands/history.ts +++ b/packages/cli/src/commands/history.ts @@ -21,8 +21,10 @@ import { historyDeps, lastTurnParts, withOwner as withHistoryOwner, + withReader as withHistoryReader, writeTurn, type Owner, + type Reader, type Turn, type UndoMode, } from "../utils/historyOwner.js"; @@ -54,6 +56,16 @@ const withOwner = ( return task(owner, turn, projectDir); }); +const withReader = ( + action: string, + dir: string | undefined, + task: (reader: Reader) => Promise, +) => + withHistoryReader(dir, (reader) => { + trackHistoryAction({ action, via: reader.via }); + return task(reader); + }); + /** An agent names itself with --who; without it the caller is the person, even during an agent's turn. */ const whoOf = (name: string | undefined): HistoryWho => (name ? { kind: "agent", name } : YOU); @@ -138,13 +150,13 @@ function print(json: boolean, data: object, text: string): void { console.log(json ? JSON.stringify(withMeta(data), null, 2) : text); } -async function textDiff(owner: Owner, entry: HistoryEntry): Promise { +async function textDiff(reader: Reader, entry: HistoryEntry): Promise { const work = mkdtempSync(join(tmpdir(), "hf-history-diff-")); try { const out: string[] = []; for (const file of entry.files) { const sides = await Promise.all( - [file.before, file.after].map((hash) => (hash ? owner.blob(hash) : Buffer.alloc(0))), + [file.before, file.after].map((hash) => (hash ? reader.blob(hash) : Buffer.alloc(0))), ); if (sides.some((bytes) => bytes.subarray(0, 8000).includes(0))) { out.push(`Binary ${file.path} changed`); @@ -283,7 +295,10 @@ function guarded(run: (args: A) => Promise) { error instanceof HistoryIdError; if (!refused) throw error; setCommandExitCode(2); - const { message } = error as Error; + const message = + error instanceof HistoryBusyError + ? `This project is open in another app (pid ${error.pid}), which keeps its history: undo, restore or pin there.` + : (error as Error).message; if ((args as { json?: boolean }).json) print(true, { ok: false, error: message }, message); else console.error(message); } @@ -316,8 +331,8 @@ const listEntries = async (args: { json: boolean; }) => { if (args._?.[0]) return; - await withOwner("list", args.dir, async (owner) => { - const all = await owner.list(); + await withReader("list", args.dir, async (reader) => { + const all = await reader.list(); const picked = args.since ? since(all, args.since, whoOf(args.who)) : all; const limit = Number(args.limit ?? 20); if (!Number.isInteger(limit) || limit < 1) @@ -348,13 +363,13 @@ export default defineCommand({ "One entry's files, before and after", { ref: { type: "positional", required: true }, diff: { type: "boolean", default: false } }, (args) => - withOwner("show", args.dir, async (owner) => { - const entry = entryOf(await owner.list(), args.ref); + withReader("show", args.dir, async (reader) => { + const entry = entryOf(await reader.list(), args.ref); const changes = entry.files.map((file) => ({ path: file.path, change: changeOf(file), })); - const diff = args.diff ? await textDiff(owner, entry) : undefined; + const diff = args.diff ? await textDiff(reader, entry) : undefined; const marks = changes.map((file) => ` ${file.change[0]!.toUpperCase()} ${file.path}`); print( args.json, @@ -402,13 +417,13 @@ export default defineCommand({ path: { type: "positional", required: false }, }, (args) => - withOwner("peek", args.dir, async (owner) => { - const files = await owner.peek(pointOf(await owner.list(), args.ref)); + withReader("peek", args.dir, async (reader) => { + const files = await reader.peek(pointOf(await reader.list(), args.ref)); if (!files) throw new Refusal("That point is no longer kept"); if (!args.path) return print(args.json, { files }, Object.keys(files).join("\n")); const hash = files[args.path]; if (!hash) throw new Refusal(`${args.path} did not exist then`); - process.stdout.write(await owner.blob(hash)); + process.stdout.write(await reader.blob(hash)); }), ), pin: () => diff --git a/packages/cli/src/utils/historyOwner.ts b/packages/cli/src/utils/historyOwner.ts index b4e5cc575e6..de2eb1039a5 100644 --- a/packages/cli/src/utils/historyOwner.ts +++ b/packages/cli/src/utils/historyOwner.ts @@ -3,8 +3,10 @@ import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node import { dirname, join } from "node:path"; import { DEFAULT_HISTORY_ROOT, + HistoryBusyError, MAX_WINDOW_IDLE_MS, openProjectHistory, + readProjectHistory, type HistoryEntry, type HistoryListItem, type HistoryResult, @@ -43,12 +45,15 @@ export interface Owner { close(): Promise; } +export type Reader = Pick; + /** Swapped by tests. */ export const historyDeps = { historyRoot: DEFAULT_HISTORY_ROOT, findServer: (projectDir: string) => findPreviewServerForProject(projectDir), /** A turn with no write for this long has ended, through a preview or not. */ turnIdleMs: MAX_WINDOW_IDLE_MS, + ownerWaitMs: undefined as number | undefined, now: () => Date.now(), }; @@ -158,6 +163,7 @@ async function directOwner(projectDir: string, turn: Turn | null): Promise historyDeps.now(), pruneGoneProjectsBudgetMs: 1000, + ownerWaitMs: historyDeps.ownerWaitMs, // A turn begun through a preview that has since stopped is still the agent's, until its idle limit. ...(turn && { closedWindow: { @@ -226,3 +232,29 @@ export async function withOwner( } } } + +export async function withReader( + dir: string | undefined, + task: (reader: Reader) => Promise, +): Promise { + try { + return await withOwner(dir, (owner) => task(owner)); + } catch (error) { + if (!(error instanceof HistoryBusyError)) throw error; + } + // Busy is only thrown on opening, 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, + }); + return task({ + via: "direct", + list: async () => view.list(), + peek: async (point) => view.peek(point), + // The owner may have pruned a blob since its log was read. + blob: (hash) => + view.readBlob(hash).catch((error: NodeJS.ErrnoException) => { + throw error.code === "ENOENT" ? new Refusal("That point is no longer kept") : error; + }), + }); +} diff --git a/packages/studio-server/src/history/blobStore.ts b/packages/studio-server/src/history/blobStore.ts index a7e923697ef..6c78df7ee9d 100644 --- a/packages/studio-server/src/history/blobStore.ts +++ b/packages/studio-server/src/history/blobStore.ts @@ -19,6 +19,14 @@ export interface BlobStore { const BLOB_HASH = /^[0-9a-f]{64}$/; +function blobPath(dir: string, hash: string): string { + // Checked where the path is joined, so no caller can read or write outside the store. + if (!BLOB_HASH.test(hash)) throw new Error("That is not a history blob."); + return join(dir, hash.slice(0, 2), hash); +} + +export const readBlob = (dir: string, hash: string) => readFile(blobPath(dir, hash)); + async function hashFile(path: string): Promise { const hash = createHash("sha256"); for await (const chunk of createReadStream(path)) hash.update(chunk); @@ -44,11 +52,7 @@ export async function openBlobStore(dir: string): Promise { for (const shard of await readdir(dir)) for (const hash of await readdir(join(dir, shard)).catch(() => [])) sizes.set(hash, (await stat(join(dir, shard, hash))).size); - const pathOf = (hash: string) => { - // Checked where the path is joined, so no caller can read or write outside the store. - if (!BLOB_HASH.test(hash)) throw new Error("That is not a history blob."); - return join(dir, hash.slice(0, 2), hash); - }; + const pathOf = (hash: string) => blobPath(dir, hash); let total = [...sizes.values()].reduce((sum, size) => sum + size, 0); return { @@ -71,7 +75,7 @@ export async function openBlobStore(dir: string): Promise { return hash; }, has: (hash) => sizes.has(hash), - read: async (hash) => readFile(pathOf(hash)), + read: async (hash) => readBlob(dir, hash), writeTo: async (hash, absPath, beforeReplace) => cloneOrCopy(pathOf(hash), absPath, beforeReplace), bytes: () => total, diff --git a/packages/studio-server/src/history/index.ts b/packages/studio-server/src/history/index.ts index 86000a9fb97..f8970d6fee5 100644 --- a/packages/studio-server/src/history/index.ts +++ b/packages/studio-server/src/history/index.ts @@ -1,12 +1,14 @@ export { DEFAULT_HISTORY_ROOT, openProjectHistory, + readProjectHistory, MAX_WINDOW_IDLE_MS, UNDO_MODES, type UndoMode, type ClosedWindow, type ProjectHistory, type ProjectHistoryOptions, + type ProjectHistoryView, type HistoryListItem, type HistoryResult, type HistoryWindow, diff --git a/packages/studio-server/src/history/projectHistory.test.ts b/packages/studio-server/src/history/projectHistory.test.ts index 9f2eb295939..d98610d0726 100644 --- a/packages/studio-server/src/history/projectHistory.test.ts +++ b/packages/studio-server/src/history/projectHistory.test.ts @@ -1,6 +1,7 @@ // @vitest-environment node // fallow-ignore-file code-duplication import { + appendFileSync, cpSync, existsSync, mkdirSync, @@ -21,7 +22,12 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { fileContentVersion, hashOfVersion, recordFileWriteReceipt } from "../helpers/fileVersion"; import { HistoryBusyError } from "./ownerLock"; import { HistoryIdError } from "./historyId"; -import { HistoryClosedError, openProjectHistory, type ProjectHistory } from "./projectHistory"; +import { + HistoryClosedError, + openProjectHistory, + readProjectHistory, + type ProjectHistory, +} from "./projectHistory"; import { START, type HistoryWho } from "./historyLog"; // A test can hold a media copy, and run work after its blob is stored but before history records it. @@ -1234,6 +1240,43 @@ describe("openProjectHistory", () => { expect((await waiting).projectId).toBe(history.projectId); }); + it("reads a history another process owns without its lock, past a half-appended last line", async () => { + const { history, write, projectDir, historyRoot } = await project({ "index.html": "v1" }); + const second = await change(history, you, "Second", () => write("index.html", "v2")); + const home = join(historyRoot, history.projectId); + appendFileSync(join(home, "log.jsonl"), '{"type":"entry","entry":{"id":"torn"'); + const files = () => + readdirSync(home).map((name) => + statSync(join(home, name)).isFile() ? [name, inside(home, name)] : [name], + ); + const before = files(); + + const view = readProjectHistory({ projectDir, historyRoot }); + expect(view.list()).toEqual(history.list()); + expect(view.list().map((entry) => entry.label)).toEqual(["Second"]); + expect(view.peek(START)).toEqual(history.peek(START)); + expect((await view.readBlob(view.peek(second.id)!["index.html"]!)).toString()).toBe("v2"); + expect(files()).toEqual(before); + }); + + it("reads a copied folder as having no history, and leaves hidden paths out as the owner does", async () => { + const { history, write, projectDir, historyRoot } = await project({ "index.html": "v1" }); + await change(history, you, "Second", () => write("index.html", "v2")); + const copy = tempDir("hf-history-copy-"); + cpSync(projectDir, copy, { recursive: true }); + expect(readProjectHistory({ projectDir: copy, historyRoot }).list()).toEqual([]); + + const [entry] = history.list(); + const hidden = { path: ".hidden/x", before: null, after: entry!.files[0]!.after }; + const older = { ...entry!, id: "older", files: [...entry!.files, hidden] }; + const logFile = join(historyRoot, history.projectId, "log.jsonl"); + appendFileSync(logFile, `${JSON.stringify({ type: "entry", entry: older })}\n`); + const paths = readProjectHistory({ projectDir, historyRoot }) + .list() + .map((item) => item.files.map((file) => file.path)); + expect(paths).toEqual([["index.html"], ["index.html"]]); + }); + it("files what changed while closed to a window begun on an earlier open, under its id", async () => { const { history, write, projectDir, historyRoot } = await project({ "index.html": "v1" }); await history.close(); diff --git a/packages/studio-server/src/history/projectHistory.ts b/packages/studio-server/src/history/projectHistory.ts index c894c7510e1..74b94c13fd2 100644 --- a/packages/studio-server/src/history/projectHistory.ts +++ b/packages/studio-server/src/history/projectHistory.ts @@ -29,7 +29,7 @@ import { affectsProjectSignature, listProjectFiles, } from "../helpers/projectSignature.js"; -import { openBlobStore, type BlobStore } from "./blobStore.js"; +import { openBlobStore, readBlob, type BlobStore } from "./blobStore.js"; import { pruneGoneProjectHistoriesDaily } from "./pruneHistories.js"; import { ID_PATH, @@ -213,6 +213,22 @@ function isHistoryPath(path: string): boolean { return !path.split("/").some((segment) => segment.startsWith(".")); } +const logFileIn = (home: string) => join(home, "log.jsonl"); + +function listOf(log: HistoryLog): HistoryListItem[] { + const undone = undoneIds(log.entries); + return log.entries.map((entry) => ({ + ...entry, + pinned: log.pins.has(entry.id), + undone: undone.has(entry.id), + })); +} + +function peekOf(log: HistoryLog, point: string): Record | null { + const files = manifestAt(log, point); + return files && Object.fromEntries(files); +} + function withoutHiddenPaths(log: HistoryLog): HistoryLog { for (const path of log.baseline.keys()) if (!isHistoryPath(path)) log.baseline.delete(path); for (const entry of log.entries) @@ -356,7 +372,7 @@ class Engine { } get logFile() { - return join(this.home, "log.jsonl"); + return logFileIn(this.home); } now(): number { @@ -1170,12 +1186,7 @@ class Engine { }, list: () => { this.assertOpen(); - const undone = undoneIds(this.log.entries); - return this.log.entries.map((entry) => ({ - ...entry, - pinned: this.log.pins.has(entry.id), - undone: undone.has(entry.id), - })); + return listOf(this.log); }, step: (direction, who, { writeToken } = {}) => this.operation(writeToken, async () => { @@ -1190,8 +1201,7 @@ class Engine { ), peek: (point) => { this.assertOpen(); - const files = manifestAt(this.log, point); - return files && Object.fromEntries(files); + return peekOf(this.log, point); }, checkout: (entryId, side, emptyDir) => this.queue(async () => { @@ -1297,3 +1307,32 @@ export async function openProjectHistory(options: ProjectHistoryOptions): Promis throw error; } } + +/** A project's history as its log stands now, read without the owner lock: it reads while another process owns it. */ +export interface ProjectHistoryView { + list(): HistoryListItem[]; + peek(point: string): Record | null; + readBlob(hash: string): Promise; +} + +/** Reads only: takes no lock and files, prunes or writes nothing; an owner's half-appended last line is skipped. */ +export function readProjectHistory( + options: Pick, +): ProjectHistoryView { + const dir = resolve(options.projectDir); + const id = readId(dir); + const home = + id && isRecordedFolder(join(options.historyRoot, id), statSync(dir)) + ? join(options.historyRoot, id) + : null; + const read = home && readLog(logFileIn(home), () => {}); + const log = withoutHiddenPaths(read || { baseline: new Map(), entries: [], pins: new Set() }); + return { + list: () => listOf(log), + peek: (point) => peekOf(log, point), + readBlob: async (hash) => { + if (!home) throw new Error("That is not a history blob."); + return readBlob(join(home, "blobs"), hash); + }, + }; +}