Skip to content

Commit 3503291

Browse files
committed
Persist session commit keys with exclusive create
Write first-time commit keys with wx and reload on EEXIST. Wrap JSON.parse failures as Invalid commit signing key. Stop listIndexPaths from swallowing every error. Treat SessionStores as an interface. Drop the IMPLEMENTATION.md claim that exclusive-delta blob staging is in place.
1 parent 35d0f61 commit 3503291

6 files changed

Lines changed: 115 additions & 23 deletions

File tree

docs/IMPLEMENTATION.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -403,9 +403,10 @@ segments (`turns-0001.jsonl`, ...) that seal at 256KB, so `git add` re-hashes on
403403
the small active segment instead of the whole growing file. Segment zero keeps the
404404
original filename, so a legacy monolithic `turns.jsonl` reads back as its own first
405405
segment. `load` and `readAt` concatenate every segment in order; a torn final line
406-
in the active segment (from a crash mid-write) is dropped on resume. Only tool-output
407-
blobs new since the last commit are staged, and stale segments deleted by a
408-
history rewrite (compaction) are removed from the tree on the next commit. The
406+
in the active segment (from a crash mid-write) is dropped on resume. The wrapper
407+
stages extra tool-output blobs it wrote since the last commit; vendor
408+
`base.commit()` also stages the whole `tool-output/` tree. Stale segments deleted
409+
by a history rewrite (compaction) are removed from the tree on the next commit. The
409410
per-commit git tree still grows one entry per spilled tool-output blob across the
410411
session; that tree re-write is inherent to git and left as residual cost.
411412

src/session/assemble-runtime.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ describe("assembleChatAgent", () => {
223223
expect(agentSessionIds).toEqual(["build-session"]);
224224
expect(agentStorages).toEqual([fakeStorage]);
225225
expect(agentAudits).toEqual([fakeStorage]);
226-
expect(agentAudits[0]).toBe(agentStorages[0]);
226+
expect(Object.is(agentAudits[0], agentStorages[0])).toBe(true);
227227
expect(agentCompactors).toEqual([builtCompactor]);
228228
},
229229
);

src/session/commit-signer.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { describe, test, expect } from "bun:test";
2+
import fs from "node:fs";
3+
import os from "node:os";
4+
import path from "node:path";
5+
6+
import { loadOrCreateCommitSigner } from "./commit-signer.js";
7+
8+
const KEY_FILE = path.join("keys", "commit-ed25519.json");
9+
10+
function tempDir(): string {
11+
return fs.mkdtempSync(path.join(os.tmpdir(), "commit-signer-"));
12+
}
13+
14+
function keyPath(dir: string): string {
15+
return path.join(dir, KEY_FILE);
16+
}
17+
18+
function publicKeyInFile(dir: string): string {
19+
const parsed = JSON.parse(fs.readFileSync(keyPath(dir), "utf8")) as { publicKey: string };
20+
return parsed.publicKey;
21+
}
22+
23+
describe("loadOrCreateCommitSigner", () => {
24+
test("first call creates a signed-able signer and a 0600 key file", async () => {
25+
const dir = tempDir();
26+
const signer = await loadOrCreateCommitSigner(dir);
27+
const signature = await signer("payload");
28+
expect(typeof signature).toBe("string");
29+
expect(signature.length).toBeGreaterThan(0);
30+
31+
const st = fs.statSync(keyPath(dir));
32+
expect(st.mode & 0o777).toBe(0o600);
33+
});
34+
35+
test("second call reloads the same key (same publicKey bytes in the file)", async () => {
36+
const dir = tempDir();
37+
await loadOrCreateCommitSigner(dir);
38+
const firstPublic = publicKeyInFile(dir);
39+
await loadOrCreateCommitSigner(dir);
40+
expect(publicKeyInFile(dir)).toBe(firstPublic);
41+
});
42+
43+
test("concurrent first-time create: both succeed, only one key file, both signers work", async () => {
44+
const dir = tempDir();
45+
const [a, b] = await Promise.all([
46+
loadOrCreateCommitSigner(dir),
47+
loadOrCreateCommitSigner(dir),
48+
]);
49+
const sigA = await a("a");
50+
const sigB = await b("b");
51+
expect(typeof sigA).toBe("string");
52+
expect(typeof sigB).toBe("string");
53+
expect(sigA.length).toBeGreaterThan(0);
54+
expect(sigB.length).toBeGreaterThan(0);
55+
expect(fs.readdirSync(path.join(dir, "keys"))).toEqual(["commit-ed25519.json"]);
56+
});
57+
58+
test("corrupt JSON throws Invalid commit signing key", async () => {
59+
const dir = tempDir();
60+
fs.mkdirSync(path.join(dir, "keys"));
61+
fs.writeFileSync(keyPath(dir), "{not-json");
62+
await expect(loadOrCreateCommitSigner(dir)).rejects.toThrow(
63+
`Invalid commit signing key at ${keyPath(dir)}`,
64+
);
65+
});
66+
67+
test("invalid arktype shape throws Invalid commit signing key", async () => {
68+
const dir = tempDir();
69+
fs.mkdirSync(path.join(dir, "keys"));
70+
fs.writeFileSync(keyPath(dir), JSON.stringify({ privateKey: 1 }));
71+
await expect(loadOrCreateCommitSigner(dir)).rejects.toThrow(
72+
`Invalid commit signing key at ${keyPath(dir)}`,
73+
);
74+
});
75+
});

src/session/commit-signer.ts

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,16 @@ async function loadPersistedKeyPair(
3535
if (cause instanceof Error && "code" in cause && cause.code === "ENOENT") return null;
3636
throw cause;
3737
}
38-
const parsed = PersistedKeyPair(JSON.parse(raw) as unknown);
38+
let parsedJson: unknown;
39+
try {
40+
parsedJson = JSON.parse(raw);
41+
} catch (cause) {
42+
if (cause instanceof SyntaxError) {
43+
throw new Error(`Invalid commit signing key at ${filePath}`, { cause });
44+
}
45+
throw cause;
46+
}
47+
const parsed = PersistedKeyPair(parsedJson);
3948
if (parsed instanceof type.errors) {
4049
throw new Error(`Invalid commit signing key at ${filePath}: ${parsed.summary}`);
4150
}
@@ -52,16 +61,27 @@ export async function loadOrCreateCommitSigner(dir: string): Promise<CommitSigne
5261
if (keyPair === null) {
5362
const generated = await generateKeyPair();
5463
await fs.promises.mkdir(keyDir, { recursive: true });
55-
await fs.promises.writeFile(
56-
filePath,
57-
JSON.stringify({
58-
privateKey: Buffer.from(generated.privateKey).toString("base64"),
59-
publicKey: Buffer.from(generated.publicKey).toString("base64"),
60-
}),
61-
{ encoding: "utf8", mode: 0o600 },
62-
);
63-
log.debug?.("wrote session commit signing key");
64-
keyPair = generated;
64+
try {
65+
await fs.promises.writeFile(
66+
filePath,
67+
JSON.stringify({
68+
privateKey: Buffer.from(generated.privateKey).toString("base64"),
69+
publicKey: Buffer.from(generated.publicKey).toString("base64"),
70+
}),
71+
{ encoding: "utf8", mode: 0o600, flag: "wx" },
72+
);
73+
log.debug?.("wrote session commit signing key");
74+
keyPair = generated;
75+
} catch (cause) {
76+
if (cause instanceof Error && "code" in cause && cause.code === "EEXIST") {
77+
keyPair = await loadPersistedKeyPair(filePath);
78+
if (keyPair === null) {
79+
throw new Error(`Commit signing key missing after EEXIST at ${filePath}`);
80+
}
81+
} else {
82+
throw cause;
83+
}
84+
}
6585
}
6686
return (payload) => createSSHSignature(payload, keyPair.privateKey, keyPair.publicKey);
6787
}

src/session/optimized-context-store.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ describe("createSessionStores", () => {
539539
test("exposes the same object as ContextStore and AuditStore", async () => {
540540
const dir = tempDir();
541541
const { storage, audit } = await createSessionStores(dir);
542-
expect(storage).toBe(audit);
542+
expect(Object.is(storage, audit)).toBe(true);
543543
expect(typeof audit.commitAudit).toBe("function");
544544
expect(typeof audit.commitErrors).toBe("function");
545545
expect(typeof audit.loadAudit).toBe("function");

src/session/optimized-context-store.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,11 +307,7 @@ export async function loadRecentTurns(dir: string, minTurns: number): Promise<Co
307307
}
308308

309309
async function listIndexPaths(dir: string): Promise<Set<string>> {
310-
try {
311-
return new Set(await git.listFiles({ fs, dir }));
312-
} catch {
313-
return new Set();
314-
}
310+
return new Set(await git.listFiles({ fs, dir }));
315311
}
316312

317313
async function extraSegmentNamesAtCommit(dir: string, hash: string): Promise<string[]> {
@@ -383,10 +379,10 @@ function extraCommitPaths(paths: readonly string[]): string[] {
383379
return paths.filter((filepath) => !VENDOR_COMMIT_ROOT_FILES.has(filepath));
384380
}
385381

386-
export type SessionStores = {
382+
export interface SessionStores {
387383
storage: ContextStore;
388384
audit: AuditStore;
389-
};
385+
}
390386

391387
/**
392388
* Local wrapper around the Interchange git store that avoids O(session length)

0 commit comments

Comments
 (0)