feat(cli): guard history turns with --exclusive, --who and --if-open (#5518)

* feat(cli): guard history turns with --exclusive, --who and --if-open

* fix(cli): make exclusive history turns atomic

Re-read the open turn only after acquiring history ownership so
concurrent exclusive begins cannot both claim the same project.
Cover the race with a deterministic two-process regression test.

* fix(cli): refuse a second exclusive turn while a preview is open

The preview's begin window now rejects another agent's live turn, and an
idle turn no longer blocks exclusive begin. The two-process test waits on
pids collected from claim drafts instead of seeing both files at once.
This commit is contained in:
Xuanru Li
2026-10-13 04:22:41 +00:00
committed by GitHub
parent 945c61ebb6
commit 7c73b3caee
7 changed files with 303 additions and 69 deletions
+8 -2
View File
@@ -1045,12 +1045,18 @@ npx hyperframes history undo --who claude # undo your newest turn
| `restore <ref> [--who]` | Make the files what they were right after an entry, a time, or `start` |
| `peek <ref> [path]` | List the files at an entry, a time, or `start` without changing anything; with a path, print that file |
| `pin <ref> [--off]` | Keep an entry, and what it needs, when old history is trimmed. `--off` unpins it. |
| `begin --who --label` | Start a turn: your writes until `end` become one entry under your name |
| `end` | End your turn and print its entry |
| `begin --who --label` | Start a turn: your writes until `end` become one entry under your name. Another agent's open turn is ended first, or with `--exclusive` the command refuses instead. |
| `end` | End your turn and print its entry. `--who <name>` refuses unless the open turn is that name's; `--if-open` succeeds when no turn is open. |
Every command also takes `--dir` and `--json`. A ref is an entry id or a unique
prefix of one; a time is a local date or date-time such as `2026-09-24T14:30`.
Without a running preview, `history` keeps its data under `~/.cache/hyperframes/history`, or under
`HYPERFRAMES_HISTORY_ROOT` when that is set, which keeps an agent run or a test apart from your own
history. That path is resolved to an absolute one, and every agent on a project has to share it: the
lock lives there, while the turn marker lives in the project. A running preview server records to
its own location.
When a file in the entry has changed since, `undo` writes nothing, exits 2 and
lists the newer entries. Run it again with `--just-this` to put back only that
entry's files over the newer edits, or `--back-to-before` to go back to before
+125
View File
@@ -504,6 +504,131 @@ describe("hyperframes history, refusals", () => {
expect(read("added.html")).toBe("new");
});
it.each(["direct", "preview"])(
"begin --exclusive starts once another agent's turn has been idle (%s)",
async (mode) => {
const { dir, hf } = project();
await hf();
if (mode === "preview") await preview(dir);
historyDeps.turnIdleMs = 300;
await hf("begin", "--who", "codex", "--label", "Retitle");
advance(1_000);
const begun = await hf("begin", "--who", "claude", "--label", "Notes", "--exclusive");
expect([begun.code, begun.err]).toEqual([0, ""]);
expect((await hf("end", "--who", "claude")).code).toBe(0);
},
);
it("begin --exclusive through a preview refuses another agent's open turn", async () => {
const { dir, hf } = project();
await hf();
await preview(dir);
await hf("begin", "--who", "codex", "--label", "Retitle");
const refused = await hf("begin", "--who", "claude", "--label", "Notes", "--exclusive");
expect([refused.code, refused.err]).toEqual([
2,
"codex's turn is open; it has to end before yours begins",
]);
});
it("begin --exclusive through a preview lets only one of two agents win", async () => {
const { dir, hf } = project();
await hf();
await preview(dir);
const begin = (who: string) =>
withOwner(dir, async (owner) => {
await owner.begin({ kind: "agent", name: who }, who, true);
}).then(
() => "won",
(error: Error) => error.message,
);
const results = await Promise.all([begin("codex"), begin("claude")]);
expect(results.filter((result) => result === "won")).toHaveLength(1);
expect(results.find((result) => result !== "won")).toMatch(
/^(codex|claude)'s turn is open; it has to end before yours begins$/,
);
});
it("begin --exclusive refuses another agent's open turn and keeps it open", async () => {
const { write, hf, json } = project();
await hf();
await hf("begin", "--who", "codex", "--label", "Retitle");
write("index.html", "A2");
const refused = await hf("begin", "--who", "claude", "--label", "Notes", "--exclusive");
expect([refused.code, refused.err]).toEqual([
2,
"codex's turn is open; it has to end before yours begins",
]);
expect((await hf("end", "--who", "codex")).code).toBe(0);
const entries = (await json()).entries as Array<{ who: { name: string }; files: string[] }>;
expect(entries.filter((entry) => entry.who.name === "codex")).toMatchObject([
{ files: ["index.html"] },
]);
});
it("serializes concurrent begin --exclusive claims under the history owner lock", async () => {
const { dir } = project();
const historyRoot = tempDir("hf-history-cli-root-");
const held = await openProjectHistory({ projectDir: dir, historyRoot });
const home = join(historyRoot, held.projectId);
const claimers = new Set<string>();
const bothWaiting = new Promise<void>((resolveWaiting) => {
const inspect = (_event: string, name: string | null) => {
const pid = name?.match(/^owner\.pid-(\d+)-/)?.[1];
if (pid) claimers.add(pid);
if (claimers.size < 2) return;
watcher.close();
resolveWaiting();
};
const watcher = watch(home, inspect);
onTestFinished(() => watcher.close());
});
const cli = resolve(fileURLToPath(import.meta.url), "..", "..", "cli.ts");
const begin = (who: string) => {
const child = spawn(
"bun",
["run", cli, "history", "begin", "--who", who, "--label", who, "--exclusive", "--dir", dir],
{
env: {
...process.env,
HYPERFRAMES_HISTORY_ROOT: historyRoot,
HYPERFRAMES_SKIP_UPDATE_CHECK: "1",
HYPERFRAMES_NO_TELEMETRY: "1",
},
},
);
onTestFinished(() => void child.kill());
return new Promise<{ code: number | null; error: string }>((done) => {
let error = "";
child.stderr.on("data", (chunk) => (error += chunk));
child.on("exit", (code) => done({ code, error: error.trim() }));
});
};
const results = [begin("codex"), begin("claude")];
await bothWaiting;
await held.close();
const settled = await Promise.all(results);
expect(settled.map((result) => result.code).sort()).toEqual([0, 2]);
expect(settled.find((result) => result.code === 2)?.error).toMatch(
/'s turn is open; it has to end before yours begins$/,
);
});
it("end --who refuses another agent's turn, and --if-open succeeds with none open", async () => {
const { hf, json } = project();
await hf();
expect(await json("end", "--if-open")).toMatchObject({ entry: null, parts: [] });
expect((await hf("end")).code).toBe(2);
await hf("begin", "--who", "codex", "--label", "Retitle");
const refused = await hf("end", "--who", "claude", "--if-open");
expect([refused.code, refused.err]).toEqual([2, "The open turn is codex's, not claude's"]);
expect((await hf("end", "--who", "codex")).code).toBe(0);
});
it("--json refusals go to stdout as {ok: false, error}", async () => {
const { hf } = project();
await hf();
+84 -20
View File
@@ -77,6 +77,39 @@ function lastIndex<T>(items: readonly T[], match: (item: T) => boolean): number
}
/** An entry id or a unique prefix of one. */
async function latestWrite(owner: Owner, turn: Turn): Promise<number> {
const entries = await owner.list();
return entries.reduce((latest, entry) => {
const same =
entry.id === turn.id ||
turn.parts.includes(entry.id) ||
(entry.who.kind === turn.who.kind &&
entry.who.name === turn.who.name &&
entry.label === turn.label &&
entry.startedAt === turn.startedAt);
return same ? Math.max(latest, entry.endedAt) : latest;
}, turn.lastWriteAt);
}
async function releaseOpenTurn(
owner: Owner,
turn: Turn | null,
who: HistoryWho,
exclusive: boolean,
projectDir: string,
): Promise<void> {
if (!turn) return;
if (owner.via !== "direct") {
if (!exclusive || turn.who.name === who.name) await endTurn(owner, turn, projectDir);
return;
}
const latest = await latestWrite(owner, turn);
const live = historyDeps.now() - latest <= historyDeps.turnIdleMs;
if (exclusive && live && turn.who.name !== who.name)
throw new Refusal(`${turn.who.name}'s turn is open; it has to end before yours begins`);
await endTurn(owner, turn, projectDir);
}
function entryOf(entries: readonly HistoryListItem[], ref: string): HistoryListItem {
const found = entries.filter((entry) => entry.id.startsWith(ref));
if (found.length === 1) return found[0]!;
@@ -123,6 +156,16 @@ function ago(at: number): string {
return `${Math.round(seconds / 86_400)}d ago`;
}
function endedTurnText(entry: HistoryEntry | null, parts: readonly string[]): string {
const earlier = parts.length - (entry ? 1 : 0);
const text = entry
? line(entry)
: earlier
? "Nothing changed since this turn's last command."
: "Nothing changed in this turn.";
return earlier ? `${text}\n(+${earlier} earlier part${earlier > 1 ? "s" : ""})` : text;
}
function line(entry: HistoryListItem | HistoryEntry): string {
const paths = entry.files.map((file) => file.path);
const files = paths.slice(0, 2).join(", ") + (paths.length > 2 ? ` +${paths.length - 2}` : "");
@@ -443,15 +486,27 @@ export default defineCommand({
}),
),
begin: () =>
sub<{ who: string; label: string; dir?: string; json: boolean }>(
sub<{ who: string; label: string; exclusive: boolean; dir?: string; json: boolean }>(
"begin",
"Start a turn: your writes until `end` become one entry under your name",
{ who: { type: "string", required: true }, label: { type: "string", required: true } },
{
who: { type: "string", required: true },
label: { type: "string", required: true },
exclusive: {
type: "boolean",
default: false,
description: "Refuse while another agent's turn is open, instead of ending it",
},
},
(args) =>
withOwner("begin", args.dir, async (owner, turn, projectDir) => {
if (turn) await endTurn(owner, turn, projectDir);
const who = whoOf(args.who);
const { id, startedAt } = await owner.begin(who, args.label);
await releaseOpenTurn(owner, turn, who, args.exclusive, projectDir);
const { id, startedAt } = await owner.begin(
who,
args.label,
args.exclusive && owner.via === "preview",
);
writeTurn(projectDir, {
via: owner.via,
id,
@@ -465,22 +520,31 @@ export default defineCommand({
}),
),
end: () =>
sub<{ dir?: string; json: boolean }>("end", "End your turn and print its entry", {}, (args) =>
withOwner("end", args.dir, async (owner, turn, projectDir) => {
if (!turn) throw new Refusal("No turn is open; start one with history begin");
const { entry, parts } = await endTurn(owner, turn, projectDir);
const earlier = parts.length - (entry ? 1 : 0);
const text = entry
? line(entry)
: earlier
? "Nothing changed since this turn's last command."
: "Nothing changed in this turn.";
print(
args.json,
{ entry: entry && publicEntry(entry), parts },
earlier ? `${text}\n(+${earlier} earlier part${earlier > 1 ? "s" : ""})` : text,
);
}),
sub<{ who?: string; ifOpen: boolean; dir?: string; json: boolean }>(
"end",
"End your turn and print its entry",
{
who: { type: "string", description: "Refuse unless the open turn is this agent's" },
"if-open": {
type: "boolean",
default: false,
description: "Succeed with no entry when no turn is open",
},
},
(args) =>
withOwner("end", args.dir, async (owner, turn, projectDir) => {
if (!turn && args.ifOpen)
return print(args.json, { entry: null, parts: [] }, "No turn is open.");
if (!turn) throw new Refusal("No turn is open; start one with history begin");
if (args.who !== undefined && turn.who.name !== args.who)
throw new Refusal(`The open turn is ${turn.who.name}'s, not ${args.who}'s`);
const { entry, parts } = await endTurn(owner, turn, projectDir);
print(
args.json,
{ entry: entry && publicEntry(entry), parts },
endedTurnText(entry, parts),
);
}),
),
},
run: ({ args }) => guarded(listEntries)(args),
+55 -41
View File
@@ -1,6 +1,6 @@
import { randomUUID } from "node:crypto";
import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs";
import { dirname, join } from "node:path";
import { dirname, join, resolve } from "node:path";
import {
DEFAULT_HISTORY_ROOT,
HistoryBusyError,
@@ -40,7 +40,11 @@ export interface Owner {
undo(id: string, who: HistoryWho, mode?: UndoMode): Promise<HistoryResult>;
restore(point: string, who: HistoryWho): Promise<HistoryEntry | null>;
pin(id: string, pinned: boolean): Promise<void>;
begin(who: HistoryWho, label: string): Promise<{ id: string; startedAt: number }>;
begin(
who: HistoryWho,
label: string,
exclusive?: boolean,
): Promise<{ id: string; startedAt: number }>;
end(id: string): Promise<HistoryEntry | null>;
close(): Promise<void>;
}
@@ -49,7 +53,7 @@ export type Reader = Pick<Owner, "via" | "list" | "peek" | "blob">;
/** Swapped by tests. */
export const historyDeps = {
historyRoot: DEFAULT_HISTORY_ROOT,
historyRoot: resolve(process.env.HYPERFRAMES_HISTORY_ROOT || 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,
@@ -144,8 +148,13 @@ function previewOwner(route: (path: string) => string): Owner {
undo: (id, who, mode) => json("/undo", { entryId: id, who, mode }),
restore: (point, who) => json("/restore", { point, who }),
pin: async (id, pinned) => void (await json("/pin", { entryId: id, pinned })),
begin: async (who, label) => {
const opened = await json("/window", { who, label, idleMs: historyDeps.turnIdleMs });
begin: async (who, label, exclusive) => {
const opened = await json("/window", {
who,
label,
idleMs: historyDeps.turnIdleMs,
...(exclusive && { exclusive: true }),
});
return { id: opened.windowId, startedAt: opened.startedAt ?? historyDeps.now() };
},
// A preview restarted since begin committed the window on its way down.
@@ -157,42 +166,48 @@ function previewOwner(route: (path: string) => string): Owner {
};
}
async function directOwner(projectDir: string, turn: Turn | null): Promise<Owner> {
async function directOwner(projectDir: string): Promise<{ owner: Owner; turn: Turn | null }> {
let turn: Turn | null = null;
const history = await openProjectHistory({
projectDir,
historyRoot: historyDeps.historyRoot,
now: () => 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: {
id: turn.id,
who: turn.who,
label: turn.label,
startedAt: turn.startedAt,
lastWriteAt: turn.lastWriteAt,
idleMs: historyDeps.turnIdleMs,
},
}),
closedWindowAfterOwnership: () => {
turn = readTurn(projectDir);
return turn
? {
id: turn.id,
who: turn.who,
label: turn.label,
startedAt: turn.startedAt,
lastWriteAt: turn.lastWriteAt,
idleMs: historyDeps.turnIdleMs,
}
: undefined;
},
});
return {
via: "direct",
list: async () => history.list(),
peek: async (point) => history.peek(point),
blob: (hash) => history.readBlob(hash),
undo: (id, who, mode) => history.undo(id, { who, ...(mode && { mode }) }),
restore: (point, who) => history.restore(point, who),
pin: async (id, pinned) => history.pin(id, pinned),
// Opening filed every earlier write; the turn's own writes are filed to it when the next open passes it.
begin: async () => ({ id: randomUUID(), startedAt: historyDeps.now() }),
end: async (id) => history.list().find((entry) => entry.id === id) ?? null,
close: () => history.close(),
turn,
owner: {
via: "direct",
list: async () => history.list(),
peek: async (point) => history.peek(point),
blob: (hash) => history.readBlob(hash),
undo: (id, who, mode) => history.undo(id, { who, ...(mode && { mode }) }),
restore: (point, who) => history.restore(point, who),
pin: async (id, pinned) => history.pin(id, pinned),
// Opening filed every earlier write; the turn's own writes are filed to it when the next open passes it.
begin: async () => ({ id: randomUUID(), startedAt: historyDeps.now() }),
end: async (id) => history.list().find((entry) => entry.id === id) ?? null,
close: () => history.close(),
},
};
}
/** One owner: a preview that keeps this project's history, else the engine, which refuses a second opener. */
async function connect(projectDir: string, turn: Turn | null): Promise<Owner> {
async function connect(projectDir: string): Promise<{ owner: Owner; turn: Turn | null }> {
const server = await historyDeps.findServer(projectDir);
if (server) {
const route = (path: string) => studioApiUrl(server, `history${path}`);
@@ -201,9 +216,9 @@ async function connect(projectDir: string, turn: Turn | null): Promise<Owner> {
(response) => response.status,
() => 404,
);
if (status !== 404) return previewOwner(route);
if (status !== 404) return { owner: previewOwner(route), turn: readTurn(projectDir) };
}
return directOwner(projectDir, turn);
return directOwner(projectDir);
}
/** Runs `task` with the project's history. ponytail: with no preview, a command mid-turn splits the turn in two. */
@@ -212,24 +227,23 @@ export async function withOwner<T>(
task: (owner: Owner, turn: Turn | null, projectDir: string) => Promise<T>,
): Promise<T> {
const { dir: projectDir } = resolveProject(dir);
const turn = readTurn(projectDir);
const owner = await connect(projectDir, turn);
const { owner, turn } = await connect(projectDir);
try {
return await task(owner, turn, projectDir);
} finally {
// This open filed the turn so far under its id; the turn goes on under a fresh one, from its last write.
const direct = owner.via === "direct" && turn;
let kept: HistoryListItem | undefined;
try {
kept = direct ? (await owner.list()).find((entry) => entry.id === turn.id) : undefined;
if (owner.via === "direct" && turn) {
const kept = (await owner.list()).find((entry) => entry.id === turn.id);
if (readTurn(projectDir)?.id === turn.id) {
const lastWriteAt = kept ? kept.endedAt : turn.lastWriteAt;
const parts = kept ? [...turn.parts, kept.id] : turn.parts;
writeTurn(projectDir, { ...turn, via: "direct", id: randomUUID(), lastWriteAt, parts });
}
}
} finally {
await owner.close();
}
if (direct && readTurn(projectDir)?.id === turn.id) {
const lastWriteAt = kept ? kept.endedAt : turn.lastWriteAt;
const parts = kept ? [...turn.parts, kept.id] : turn.parts;
writeTurn(projectDir, { ...turn, via: "direct", id: randomUUID(), lastWriteAt, parts });
}
}
}
@@ -32,7 +32,7 @@ function ownerOf(file: string): number | null {
/** Takes `file` if nobody holds it: written aside and linked in, so a reader never sees it without its pid. */
function claim(file: string): boolean {
const draft = `${file}-${randomUUID()}.tmp`;
const draft = `${file}-${process.pid}-${randomUUID()}.tmp`;
writeFileSync(draft, String(process.pid));
try {
linkSync(draft, file);
@@ -82,6 +82,7 @@ export interface ProjectHistoryOptions {
ownerWaitMs?: number;
/** A CLI turn's window from an earlier open: writes since, within its idle limit, become the entry with its id. */
closedWindow?: ClosedWindow;
closedWindowAfterOwnership?: () => ClosedWindow | undefined;
undoScope?: "own" | "everyone";
pruneGoneProjectsBudgetMs?: number;
}
@@ -135,7 +136,7 @@ export interface ProjectHistory {
beginWindow(
who: HistoryWho,
label: string,
options?: { idleMs?: number },
options?: { idleMs?: number; exclusive?: boolean },
): Promise<HistoryWindow>;
/**
* For a writer that records after writing (Studio): moves the uncommitted changes to `paths` into one entry of
@@ -945,10 +946,25 @@ class Engine {
});
}
beginWindow(who: HistoryWho, label: string, idleMs: number): Promise<HistoryWindow> {
beginWindow(
who: HistoryWho,
label: string,
idleMs: number,
exclusive = false,
): Promise<HistoryWindow> {
return this.queue(async () => {
// Writes before the window opened are not this writer's, and are logged before it.
await this.settle();
const at = this.now();
if (exclusive) {
const blocking = this.windows.find(
(open) => !sameWho(open.who, who) && takesWrite(open, at),
);
if (blocking)
throw new Error(`${blocking.who.name}'s turn is open; it has to end before yours begins`);
for (const open of [...this.windows])
if (!takesWrite(open, at) || sameWho(open.who, who)) await this.endWindow(open);
}
const window = { ...this.newGroup(who, label), idleMs };
this.windows.push(window);
this.touch(window, this.now());
@@ -1176,7 +1192,12 @@ class Engine {
return {
projectId: this.projectId,
beginWindow: (who, label, options = {}) =>
this.beginWindow(who, label, options.idleMs ?? this.options.maxGroupMs ?? 30_000),
this.beginWindow(
who,
label,
options.idleMs ?? this.options.maxGroupMs ?? 30_000,
options.exclusive === true,
),
claim: async (who, label, paths, options = {}) => {
await Promise.all(paths.map((path) => this.adopting.get(this.logPath(path))));
return this.queue(() => this.claimNow(who, label, paths, options));
@@ -1288,7 +1309,8 @@ export async function openProjectHistory(options: ProjectHistoryOptions): Promis
);
try {
const blobs = await openBlobStore(join(options.historyRoot, projectId, "blobs"));
const engine = new Engine(options, projectId, blobs);
const closedWindow = options.closedWindowAfterOwnership?.() ?? options.closedWindow;
const engine = new Engine({ ...options, closedWindow }, projectId, blobs);
await engine.queue(async () => {
await engine.start();
engine.hearWrites();
+4 -1
View File
@@ -141,7 +141,10 @@ export function registerHistoryRoutes(api: Hono, adapter: StudioApiAdapter): voi
const window = await history.beginWindow(
whoOf(body),
text(body.label) ?? "Edited in Studio",
idleMs ? { idleMs } : undefined,
{
...(idleMs ? { idleMs } : {}),
...(body.exclusive === true && { exclusive: true }),
},
);
windows.set(window.id, { history, window });
// The window's id is its last entry's id, unless nothing was left after its last part.