sscoble/pi-mono
1
0
Fork 0
mirror of https://github.com/badlogic/pi-mono.git synced 2026-11-05 23:37:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

fix(agent): complete JSONL crash and corruption handling

Browse source
This commit is contained in:
David Brailovsky 2026-08-07 13:47:01 +02:00
parent 14ad9801be
commit 4a0e2f115a
4 changed files with 125 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

16
packages/agent/docs/harness-v2-test-matrix.md
View file

@ -16,10 +16,10 @@ No production or test changes are part of QA1.
| Area | Removed cases | Status |
|---|---:|---|
| Harness runtime and stream behavior | 37 | Mostly uncovered by design while `AgentHarness` is scaffolded; assigned to H/L/I/C/N packages. Scaffold-safe configuration is covered by F0. |
| Branch query and corruption behavior | 6 | Core query semantics are covered; bounded SQLite validation gaps were ported by QA2, and remaining JSONL corruption gaps are assigned to J3. |
| Branch query and corruption behavior | 6 | Core query semantics are covered; bounded SQLite validation gaps were ported by QA2, and JSONL corruption behavior is covered by J3. |
| Compaction helper behavior | 2 | Covered by current compaction/context tests. |
| Memory/SQLite v4 conformance entrypoints | 3 | Ported to `packages/agent/test/harness/session/*` and `packages/session-backends/sqlite-node/test/conformance.test.ts`. |
| Repository/backend lifecycle and JSONL behavior | 38 | Most covered by v4 conformance or J0–J2; QA2 lifecycle/query audits are resolved, with remaining crash/corruption/v3 gaps assigned to J3–J5. |
| Repository/backend lifecycle and JSONL behavior | 38 | Format-4 lifecycle and crash/corruption behavior are covered by v4 conformance and J0–J3; format-3 normalization and conversion remain assigned to J4–J5. |
| Session aggregate/context behavior | 17 | Covered by v4 conformance plus current context tests. |
| SQLite search | 1 | Ported to SQLite package search tests; old scanning backend is inapplicable. |
@ -122,7 +122,7 @@ Removed files:
| builds context from the branch storage without loading complete history | Inapplicable / Covered | Old branch-storage optimization was deleted; v4 context behavior is covered by `session/context.test.ts`. |
| rejects repository operations and session writes after disposal | Covered / Inapplicable | The v4 core `SessionRepo` contract has no disposable state, and the in-memory/JSONL repos do not implement permanent disposal. SQLite disposal is resource release rather than repo poisoning; `packages/session-backends/sqlite-node/test/repository.test.ts` covers the remaining applicable behavior in `closes active sessions when the repository is disposed`, proving active session writes reject after repository disposal. |
| supports lexical ownership with await using | Inapplicable | The old test covered permanent disposal on the deleted in-memory repository. The v4 core `SessionRepo` contract has no disposable surface, and memory/JSONL repos do not implement lexical ownership. SQLite `await using` is resource cleanup rather than repo poisoning; active-session closure is covered by `closes active sessions when the repository is disposed` in `packages/session-backends/sqlite-node/test/repository.test.ts`. |
| serializes conflicting create and fork destinations | Uncovered / J3 | The old test covered JSONL backend-wide serialization for concurrent create/create and create/fork operations targeting the same id. V4 intentionally removed global repository serialization, but the remaining format-4 lifecycle/concurrency question is whether conflicting destination creation can duplicate files or silently overwrite; assign to J3 lifecycle/concurrency edge cases. |
| serializes conflicting create and fork destinations | Covered | JSONL tests cover concurrent create/create, create/fork, and fork/fork operations targeting the same destination, plus reservation release after failed operations. |
| encodes custom session IDs used in filenames | Covered | J2 JSONL repository lifecycle validates file-safe ids; `jsonl.test.ts` rejects invalid coding-agent filenames. |
| allows appends to different sessions to run concurrently | Covered | J2/v4 repository conformance and JSONL concurrent write tests cover accepted concurrent writes without the old keyed queue. |
| caps concurrent operations across JSONL sessions at four by default | Inapplicable | Old JSONL keyed-operation-queue implementation detail was deleted. |
@ -135,8 +135,8 @@ Removed files:
| waits for accepted appends before disposal and rejects later writes | Inapplicable | The old test covered deleted JSONL repository disposal: drain accepted appends, enter a permanent disposed state, then reject later writes through existing sessions. V4 JSONL repos are not disposable, do not retain opened storages, and have no repo-level closed state. Per-session append serialization remains covered by backend conformance `linearizes concurrent writes across two lanes` and JSONL-specific `persists concurrent cross-lane writes in shared sequence order`; close/drain/reject-after-close semantics belong to harness H5/O3, not `SessionRepo` disposal. |
| parses once when opened and retains state across appends | Inapplicable | Old JSONL in-memory aggregate implementation detail; v4 correctness is covered by reopen/shared-sequence tests. |
| collects sessions below encoded cwd directories and lists by cwd | Covered | J2 metadata lifecycle and listing tests cover v4 JSONL metadata and cwd filtering. |
| fails loudly when listing a malformed session file | Uncovered | J3 owns JSONL crash/corruption behavior for malformed files. |
| rejects a missing active leaf when opened | Uncovered | J3 owns JSONL missing-reference rejection. SQLite equivalent is covered in `repository.test.ts`. |
| fails loudly when listing a malformed session file | Inapplicable / Covered | Section 13 now requires best-effort listing: malformed headers are skipped without opening or replaying the session. JSONL tests cover skipping the malformed file while retaining valid results; direct `open()` still rejects it. |
| rejects a missing active leaf when opened | Covered | JSONL and SQLite tests cover missing-reference rejection. |
| opens, deletes, and forks by metadata (JSONL) | Covered | J2 JSONL repo conformance. |
| persists header metadata through create, list, and fork | Covered | J0 codec and J2 repository metadata tests. |
| repository disposal closes its owned storage | Covered / Inapplicable | Old in-memory repo disposal is inapplicable because v4 memory/JSONL repos are not disposable and do not own returned session storage lifetimes. SQLite is the only disposable repository because it owns DB/lease resources; active-session closure is covered by `closes active sessions when the repository is disposed`, and DB close behavior is covered by existing SQLite connection lifecycle tests. |
@ -145,10 +145,10 @@ Removed files:
| includes assistant and summary usage in statistics | Covered | v4 conformance `keeps latest-value facts and computes ledger statistics across lanes`, JSONL storage, and SQLite repository statistics tests. |
| stops branch traversal at retained-tail compaction | Covered / Inapplicable | Branch-query stop semantics are still required outside context projection and are covered explicitly by backend conformance `supports bounded filtered and cursor-based queries` via `findEntriesOnBranch({ stopAtType: "compaction" })` across memory, JSONL, and SQLite. Retained-tail materialization is covered by context test `starts at the latest compaction and materializes its retained tail`. The old implicit `getBranch()` auto-stop-at-retained-tail-compaction behavior is inapplicable because v4 uses explicit branch bounds plus context projection. |
| writes headers and entries and reopens the aggregate | Covered | J1/J2 JSONL storage/repository tests. |
| fails loudly for malformed headers and entries | Covered / J3 | J3 owns malformed physical file behavior; current JSONL tests already cover malformed tail/middle lines. |
| fails loudly for malformed headers and entries | Covered | Direct JSONL opens reject malformed headers and complete invalid mutations without modifying the file; only a torn final JSON fragment is repaired. Listing separately skips malformed headers as required by section 13. |
| enforces entry uniqueness and does not recreate deleted files | Covered | v4 conformance rejects duplicate ids; J2 lifecycle covers delete/reopen behavior. |
| scopes entry uniqueness to the session path | Covered | v4 repository/session isolation conformance. |
| rejects non-object header metadata | Uncovered | J3/J4 should cover malformed JSONL header metadata for format-4 and v3 normalization. |
| rejects non-object header metadata | Covered | Format-4 open rejects non-object header metadata, while listing skips that malformed file. Format-3 normalization remains assigned to J4. |
## Session aggregate and context tests
@ -187,7 +187,7 @@ Removed case from `packages/agent/test/harness/sqlite-node.test.ts`.
These packages must land before QA3 can re-evaluate the uncovered rows above. They do not use this matrix as their test plan.
- **QA2**: completed storage/query audit and ports for bounded-query corruption/validation behavior, repository/session disposal lifecycle, listing/disposal barriers, and branch-query retained-tail semantics outside context projection.
- **J3**: JSONL malformed file, torn-tail, missing-reference, and lifecycle/concurrency edge cases.
- **J3**: completed JSONL malformed-file, torn-tail, missing-reference, and lifecycle/concurrency coverage.
- **J4/J5**: v3 read-only normalization and first-write conversion; include malformed v3/header metadata cases.
- **I1/I2/I3/I4/L1-L3**: hook/event/mutation/effects/loop primitive coverage required before runtime harness tests can return.
- **H1-H8**: durable run, queue, configuration, wait/abort, tool, recovery, and deferred-provider runtime behavior formerly covered by legacy `agent-harness*.test.ts`.

4
packages/agent/docs/harness-v2.md
View file

@ -1660,7 +1660,7 @@ interface JsonlSessionListOptions { cwd?: string; }
A v3 `parentSession` path resolves to the parent header's id when that file is available. If it is unavailable, metadata retains `legacyParentSessionPath`; first-write conversion preserves that optional header field rather than silently dropping the relationship. Format-4 code uses `parentSessionId` for repository relationships. `modifiedAt` is read from the filesystem and is not a sequenced session mutation.
The repository layout matches coding-agent v3. Under `sessionsRoot`, each resolved cwd uses a directory named `--${resolvedCwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`. New files are named `${createdAtIso.replace(/[:.]/g, "-")}_${sessionId}.jsonl`. `list({ cwd })` scans that cwd's directory; `list()` scans every direct child directory. First-write v3 conversion replaces the original file in place and never changes its directory or filename.
The repository layout matches coding-agent v3. Under `sessionsRoot`, each resolved cwd uses a directory named `--${resolvedCwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`. New files are named `${createdAtIso.replace(/[:.]/g, "-")}_${sessionId}.jsonl`. `list({ cwd })` scans that cwd's directory; `list()` scans every direct child directory. Listing reads only each file's header and filesystem metadata; it does not open or replay the session. A file with a missing or malformed header is omitted from the result. First-write v3 conversion replaces the original file in place and never changes its directory or filename.
One file per session: a header line, then one JSON object per line, in `seq` order. Every logical mutation is exactly one line; a line is the atomic unit.
@ -3265,7 +3265,7 @@ These packages own `packages/agent/src/harness/session/jsonl/**`, the concrete `
- [x] **J2 — format-4 repository lifecycle and forks.** Dependencies: J1.
- Add create/open/list/delete, one writer queue per session, metadata ordering/filtering, branch/tree forks, and the concrete public `JsonlSessionRepo` export.
- Acceptance: the complete backend-neutral conformance suite passes against JSONL, including concurrent lane writes and forks.
- [ ] **J3 — format-4 crash and corruption behavior.** Dependencies: J2.
- [x] **J3 — format-4 crash and corruption behavior.** Dependencies: J2.
- Add torn-tail truncation, malformed-interior rejection, missing-reference rejection, and lifecycle/concurrency edge cases.
- Acceptance: acknowledged writes survive reopen and malformed non-tail data is never silently repaired.
- [ ] **J4 — read-only v3 normalization.** Dependencies: J3.

11
packages/agent/src/harness/session/jsonl/repo.ts
View file

@ -2,7 +2,7 @@ import { uuidv7 } from "@earendil-works/pi-ai";
import { assertJsonSerializable, Session } from "../session.ts";
import { type ForkOptions, SessionError, type SessionRepo } from "../types.ts";
import { metadataFromHeader, parseHeader } from "./codec.ts";
import { fileResult, invalidFile } from "./errors.ts";
import { fileResult } from "./errors.ts";
import { JsonlSessionStorage } from "./storage.ts";
import type {
JsonlSessionCreateOptions,
@ -167,8 +167,13 @@ export class JsonlSessionRepo
await this.fs.readTextLines(file.path, { maxLines: 1 }),
`Failed to read session header ${file.path}`,
);
if (!firstLine) throw invalidFile(file.path, 1, "is missing a header");
metadata.push(metadataFromHeader(parseHeader(firstLine, file.path), file.path, file.mtimeMs));
if (!firstLine) continue;
try {
metadata.push(metadataFromHeader(parseHeader(firstLine, file.path), file.path, file.mtimeMs));
} catch (error) {
if (error instanceof SessionError && error.code === "invalid_entry") continue;
throw error;
}
}
}
return metadata.sort((left, right) => right.modifiedAt - left.modifiedAt);

113
packages/agent/test/harness/session/jsonl.test.ts
View file

@ -122,6 +122,41 @@ describe("JSONL v4 persistence", () => {
expect(await repository.list({ cwd: join(root, "other", "project") })).toEqual([]);
});
it("rejects a malformed JSON header on open and skips it when listing", async () => {
const root = createTempDir();
const repository = createRepository(root);
await repository.create({ id: "valid", cwd: root });
const session = await repository.create({ id: "malformed-header", cwd: root });
const metadata = await session.getMetadata();
const malformed = "not json\n";
writeFileSync(metadata.path, malformed);
await expect(repository.open(metadata)).rejects.toMatchObject({ code: "invalid_entry" });
expect((await repository.list({ cwd: root })).map((listed) => listed.id)).toEqual(["valid"]);
expect(readFileSync(metadata.path, "utf8")).toBe(malformed);
});
it("rejects non-object header metadata on open and skips it when listing", async () => {
const root = createTempDir();
const repository = createRepository(root);
await repository.create({ id: "valid", cwd: root });
const session = await repository.create({ id: "invalid-header-metadata", cwd: root });
const metadata = await session.getMetadata();
const malformed = `${JSON.stringify({
kind: "header",
version: 4,
id: metadata.id,
createdAt: metadata.createdAt,
cwd: metadata.cwd,
metadata: "invalid",
})}\n`;
writeFileSync(metadata.path, malformed);
await expect(repository.open(metadata)).rejects.toMatchObject({ code: "invalid_entry" });
expect((await repository.list({ cwd: root })).map((listed) => listed.id)).toEqual(["valid"]);
expect(readFileSync(metadata.path, "utf8")).toBe(malformed);
});
it("rejects session ids that cannot be used in coding-agent filenames", async () => {
const root = createTempDir();
const repository = createRepository(root);
@ -145,30 +180,66 @@ describe("JSONL v4 persistence", () => {
expect((await repository.list()).map((metadata) => metadata.id)).toEqual(["shared", "shared"]);
});
it("rejects concurrent create calls for the same destination", async () => {
it.each([
["create", "create"],
["create", "fork"],
["fork", "fork"],
] as const)("rejects concurrent %s and %s calls for the same destination", async (firstKind, secondKind) => {
vi.useFakeTimers({ toFake: ["Date"] });
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
try {
const root = createTempDir();
const repository = createRepository(root);
const cwd = join(root, "workspace");
const source = await repository.create({ id: "source", cwd });
const sourceMetadata = await source.getMetadata();
const run = (kind: "create" | "fork") =>
kind === "create"
? repository.create({ id: "same", cwd })
: repository.fork(sourceMetadata, { id: "same", cwd });
const results = await Promise.allSettled([
repository.create({ id: "same", cwd }),
repository.create({ id: "same", cwd }),
]);
const results = await Promise.allSettled([run(firstKind), run(secondKind)]);
const successes = results.flatMap((result) => (result.status === "fulfilled" ? [result.value] : []));
const failures = results.flatMap((result) => (result.status === "rejected" ? [result.reason] : []));
expect(successes).toHaveLength(1);
expect(failures).toHaveLength(1);
expect(failures[0]).toMatchObject({ code: "already_exists" });
expect((await repository.list({ cwd })).map((listed) => listed.id)).toEqual(["same"]);
expect((await repository.list({ cwd })).filter((listed) => listed.id === "same")).toHaveLength(1);
} finally {
vi.useRealTimers();
}
});
it.each(["create", "fork"] as const)("releases a destination reservation after a failed %s", async (kind) => {
const root = createTempDir();
const env = new NodeExecutionEnv({ cwd: root });
const repository = new JsonlSessionRepo({ fs: env, sessionsRoot: root });
const cwd = join(root, "workspace");
const source = await repository.create({ id: "source", cwd });
const sourceMetadata = await source.getMetadata();
const run = () =>
kind === "create"
? repository.create({ id: "retry", cwd })
: repository.fork(sourceMetadata, { id: "retry", cwd });
if (kind === "create") {
vi.spyOn(env, "writeFile").mockResolvedValueOnce({
ok: false,
error: new FileError("unknown", "injected creation failure"),
});
} else {
vi.spyOn(env, "renameFile").mockResolvedValueOnce({
ok: false,
error: new FileError("unknown", "injected fork failure"),
});
}
await expect(run()).rejects.toMatchObject({ code: "storage" });
await expect(run()).resolves.toBeDefined();
expect((await repository.list({ cwd })).filter((listed) => listed.id === "retry")).toHaveLength(1);
});
it("sorts listed sessions by current filesystem modification time", async () => {
const root = createTempDir();
const repository = createRepository(root);
@ -415,6 +486,15 @@ describe("JSONL v4 persistence", () => {
expect((await reopened.getEntry(appendedId))?.seq).toBe(2);
});
it("rejects a complete invalid final mutation without modifying the file", async () => {
const root = createTempDir();
const metadata = writeRawSession(root, "invalid-final-mutation", [{ kind: "unknown", seq: 1 }]);
const corrupted = readFileSync(metadata.path, "utf8");
await expect(createRepository(root).open(metadata)).rejects.toMatchObject({ code: "invalid_entry" });
expect(readFileSync(metadata.path, "utf8")).toBe(corrupted);
});
it("rejects a malformed middle line without modifying the file", async () => {
const root = createTempDir();
const repository = createRepository(root);
@ -662,4 +742,25 @@ describe("JSONL v4 persistence", () => {
expect(readFileSync(metadata.path, "utf8")).toBe(original);
expect(existsSync(`${metadata.path}.tmp`)).toBe(false);
});
it("preserves the session when torn-tail repair cannot be published", async () => {
const root = createTempDir();
const repository = createRepository(root);
const session = await repository.create({ id: "repair-rename-failure", cwd: root });
const metadata = await session.getMetadata();
await session.appendCustomEntry("kept");
appendFileSync(metadata.path, '{"kind":"entry"');
const original = readFileSync(metadata.path, "utf8");
const env = new NodeExecutionEnv({ cwd: root });
vi.spyOn(env, "renameFile").mockResolvedValueOnce({
ok: false,
error: new FileError("unknown", "injected repair rename failure"),
});
const failingRepository = new JsonlSessionRepo({ fs: env, sessionsRoot: root });
await expect(failingRepository.open(metadata)).rejects.toMatchObject({ code: "storage" });
expect(readFileSync(metadata.path, "utf8")).toBe(original);
expect(existsSync(`${metadata.path}.tmp`)).toBe(false);
});
});