PR #4: append safety fixed, decoder issues remain
PR #3 is merged. PR #4 is rebased onto main, but the disk adapter has three reproduced issues.
2 open P2 findings: F59 and new F60. F57 and F58 are verified fixed. No open P1 findings. No GitHub review comments posted.
+1053 −0 total 5f3f755Round 3 reviewed exact head 5f3f755160b89d5283b04c371116d8576cccdc46 on 23 September 2026. The latest patch adds 318 lines and removes 84 across three files. PR #4 remains open on main.
What changed the verdict
The append-loss issue is closed. Each append inspects the current file, so a handle opened before another handle's partial failure repairs the new tail correctly. Both handles also preserve each other's successful repair. Directory durability remains fixed. All 112 previous independent cases now pass.
The decoder still has two P2 issues. F59 accepts null members of discourse arrays and an intent binding without its required expectations. The null members crash runtime restoration. New F60 is the opposite mismatch: a valid receipt without an optional idempotency key is accepted by append, then rejected as corruption on the next append.
Architecture and complexity
The current-boundary check is a clear correction. Full-file parsing before every append costs work proportional to the journal's size, but it is straightforward within the small reference-adapter scope. Compaction and larger-scale performance remain separate follow-ups.
The decoder now adds 325 lines to core/types.ts. Its reusable helpers are reasonable, but validation is drifting from the TypeScript definitions in both directions: optional receipt data became required, and required binding data remains unchecked. That is the remaining concern about complexity. Align validation with the owned types, distinguish SlotBinding from FrozenSlotBinding, and reuse structure validators for collection members. A new persistence framework would not help these errors.
Verification
354 repository tests pass: 323 evals and 31 journal-file tests. Repository typecheck passes. All 112 prior independent cases pass. Seven new cases produce 5 failures and 2 passing controls; their TypeScript check passes too.
L1 is a valid typed journal round-trip regression. L2 has three deliberately corrupted discourse collections; each reaches runtime restoration and throws a null-property TypeError. L3 removes expectations from one otherwise valid intent binding, which is wrongly accepted. L4 and L5 confirm a valid discourse snapshot and a keyed receipt still work. Filesystem fault injection is not physical power-loss certification.
Current review JSONReview textNew reproductionsFailure output112 prior checks354 repository testsRound 2 reviewOriginal review
Round 3 changes
Every append now inspects the current boundary, closing F57. The decoder gained nested validators and previous cases pass. F59 remains incomplete for owned collection members and binding fields. New F60 rejects an optional receipt field when omitted.
Round 2 changes
Tail repair and first-open directory syncing were added. A shared journal decoder now checks entry kinds and top-level fields. All original reproductions pass, but constructor-cached tail state and shallow nested decoding leave F57 and F59 open. F58 is verified fixed.
Changed: journal-file/src/index.ts, core/src/types.ts, and a new file-journal-durability.test.ts.
Walkthrough
PR #4 implements the InteractionJournal port with a synchronous JSONL file. It keeps Node filesystem dependencies outside the kernel and relies on the already merged recovery reader.
- append serializes one record, completes short writes, fsyncs the file and syncs the parent directory on initial creation. Write or sync failures poison that handle.
- entries reads through to disk and returns a detached frozen array. Loading rejects interior parse holes and tolerates a damaged final fragment.
- close releases the descriptor. A later append reopens it. Fourteen retained tests cover basic files, corruption and runtime restart behavior.
Files changed
| File | Change |
|---|---|
| packages/journal-file/src/index.ts | +155 |
| packages/journal-file/src/file-journal.test.ts | +259 |
| packages/journal-file/package.json | +24 |
| packages/journal-file/tsconfig.json | +8 |
| pnpm-lock.yaml | +16 |
Issues
| ID | Priority | Finding |
|---|---|---|
| F57 · fixed | P1 | Repair the append boundary after tolerating a torn tail |
| F58 · fixed | P1 | Do not infer directory durability from file existence |
| F59 | P2 | Separate torn JSON from invalid journal records |
| F60 | P2 | Preserve the optional idempotency key in receipt decoding |
Persistence flow
Create/load -> validate JSONL -> append -> complete writes -> fsync file -> sync parent directory -> return. Recovery must also establish a safe append boundary.
Prompt for all review comments · Ctrl+`
Prompt to re-run review · Ctrl+Space
Review comments
P1 F57. Repair the append boundary after tolerating a torn tailpackages/journal-file/src/index.ts:93-96logicdata-integrity
Verified fixed. repairBoundary now inspects the current file before every append. K1 preserves the surviving handle's acknowledged append after another handle leaves a partial write. K4 preserves both successful appends after two handles open on an existing torn tail. All original tail and separator cases also pass.
Current fix prompt
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `packages/journal-file/src/index.ts` around lines 160-165, do not use constructor-time tailRepair absence as proof that the current append boundary is clean. Another supported handle can leave a partial write after this handle was created. Before accepting the next append, inspect the current boundary or enforce equivalent shared invalidation. Preserve prior complete records and the next acknowledged record, or fail closed before writing. K1 in `/tmp/circe-pr4-evidence/disk-round2.test.ts` reproduces the failure using sequential real-file writes and one injected write failure. Keep K4, which verifies that a later handle does not truncate another handle's successful repair, and all earlier disk checks passing.
Partially fixed. J1 and J2 now pass when the handle is opened after the torn tail exists. K1 opens two handles on a clean file, injects a partial write and failure through the first, then appends through the surviving handle. Its constructor cached no repair; normalizeTail returns before rereading. The append succeeds but its record disappears on reopen. All operations are sequential in one process. K4 passes when both handles were opened after the tail was already torn.
Current location: packages/journal-file/src/index.ts:160-165
Current fix direction. In `packages/journal-file/src/index.ts` around lines 160-165, do not use constructor-time tailRepair absence as proof that the current append boundary is clean. Another supported handle can leave a partial write after this handle was created. Before accepting the next append, inspect the current boundary or enforce equivalent shared invalidation. Preserve prior complete records and the next acknowledged record, or fail closed before writing. K1 in `/tmp/circe-pr4-evidence/disk-round2.test.ts` reproduces the failure using sequential real-file writes and one injected write failure. Keep K4, which verifies that a later handle does not truncate another handle's successful repair, and all earlier disk checks passing.
Original finding below, preserved from round 1.
The constructor validates the file with parseEntries, but discards the location of the tolerated final fragment. handle then opens the same bytes with O_APPEND. Ignoring a broken tail while reading does not make the file safe to append to.
J1 loads an old valid record followed by an incomplete JSON fragment, then appends a new record successfully. Without a trailing newline, the new record joins the fragment and disappears on the next read. With a trailing newline, the fragment becomes an interior hole and reopening rejects the entire journal. J2 starts with a complete final JSON record missing only its newline; the next append concatenates two JSON objects and both records disappear.
These are supported recovery inputs, not concurrent-writer scenarios. The current tests stop after reading the tolerated tail. A recovery adapter must preserve the next acknowledged append too. Otherwise a successful write-ahead append can be missing when the kernel next reconstructs state.
Fix direction. Retain the last safe byte boundary and normalize the tail before accepting another append, with the repair included in the durability boundary. Alternatively fail closed until an explicit repair completes. Preserve complete records and ensure a final complete record without a newline stays separate from the next record.
J1, J2: three failing cases
93: parseEntries(path); 94: 95: let descriptor: number | undefined; 96: let directorySynced = existed;
Prompt for AI agents
P1 F58. Do not infer directory durability from file existencepackages/journal-file/src/index.ts:90-96logicdata-integrity
Remains fixed. Directory-sync failure/reopen and normal durability controls pass.
Verified fixed. directorySynced starts false for a new journal object, so reopening after a directory-fsync failure establishes directory durability on the next append. Original J3 passes, along with normal-create and reopen controls.
Current location: packages/journal-file/src/index.ts:133-136
Original finding below, preserved from round 1.
directorySynced is initialized from existsSync. That proves the name is currently visible, not that a previous creation synced its directory. A failed first append can leave a complete, file-synced journal behind before the directory sync succeeds.
J3 injects a failure only into the first parent-directory fsync. The initial append correctly throws and poisons its handle. After close and reopen, a second append returns success, but no parent-directory fsync is attempted because the path now exists. The reopened handle has forgotten the unfinished creation durability step.
File fsync does not establish durability of its directory entry. A later successful append therefore lacks the documented guarantee that the filename survives power loss. This probe establishes the missing syscall after a real-file failure sequence; it does not simulate a physical power loss.
Fix direction. Establish parent-directory durability on the first durable append of a newly opened journal even when its path already exists, or use a protocol that proves creation durability. An existence check alone cannot carry that proof across handles.
J3: one failing case
90: const existed = existsSync(path); 91: // Validate before opening: a directory, device or FIFO at this path must not 92: // become a blocking or redirecting append target. 93: parseEntries(path); 94: 95: let descriptor: number | undefined; 96: let directorySynced = existed;
File fsync does not guarantee directory-entry persistence; the parent directory needs its own sync. Linux fsync(2).
Prompt for AI agents
P2 F59. Separate torn JSON from invalid journal recordspackages/journal-file/src/index.ts:64-78logicdata-integrity
Round 3 fix prompt
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `packages/core/src/types.ts` around lines 812-817, 903-925, finish the owned nested-data checks behind `decodeJournalEntry`. Array shape alone does not validate the scopes, views and referents that restoration dereferences, and a SlotBinding needs expectations and origins that a FrozenSlotBinding does not. L2 and L3 in `/tmp/circe-pr4-evidence/disk-round3.test.ts` cover null discourse members and a binding missing expectations. Use distinct validators for the two binding types and validate collection members with the corresponding owned structure validators, including result arrays where applicable. Preserve line-numbered corruption errors and keep application payloads opaque. Keep all 112 earlier probes and the valid snapshot control passing; avoid per-example rejection rules.
Partially fixed. All previous nested-record cases now reject. However, validateDiscourseSnapshot checks only that scopes, views and referents are arrays; each accepts [null]. L2 then reaches runtime restoration and throws Cannot read properties of null (reading id). L3 removes only expectations from an otherwise valid intent binding and the decoder accepts it, although live commit iterates binding.expectations. These are deliberate malformed-file probes of owned kernel data, not normal-writer failures.
Current fix prompt
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `packages/core/src/types.ts` around lines 785-881, complete `decodeJournalEntry` for the owned nested data consumed by recovery. Checking that transaction, intent or result is a non-null object does not establish the InteractionLogEntry type. Reject missing or wrongly typed required nested fields with the journal loader's line-numbered corruption context; keep arbitrary application payloads opaque. K2 and K3 in `/tmp/circe-pr4-evidence/disk-round2.test.ts` cover a transaction without effects, an empty intent, an empty command result and the resulting recovery crash. Reuse compact validators for owned structures instead of adding per-example exceptions or a second divergent schema. Keep valid runtime journal round trips and the original rejection cases passing.
Partially fixed. Complete invalid top-level records and unknown kinds now reject, and the original J4/J5 cases pass. The new decoder only checks that nested payloads are objects. K2 admits a transaction with only id, an empty intent and an empty command result. K3 feeds the admitted transaction to recoverState and gets tx.effects is not iterable instead of a line-numbered journal corruption error. These are deliberately malformed files testing the documented decoding boundary, not a claim that normal runtime writes produce them.
Current location: packages/core/src/types.ts:814-818
Current fix direction. In `packages/core/src/types.ts` around lines 785-881, complete `decodeJournalEntry` for the owned nested data consumed by recovery. Checking that transaction, intent or result is a non-null object does not establish the InteractionLogEntry type. Reject missing or wrongly typed required nested fields with the journal loader's line-numbered corruption context; keep arbitrary application payloads opaque. K2 and K3 in `/tmp/circe-pr4-evidence/disk-round2.test.ts` cover a transaction without effects, an empty intent, an empty command result and the resulting recovery crash. Reuse compact validators for owned structures instead of adding per-example exceptions or a second divergent schema. Keep valid runtime journal round trips and the original rejection cases passing.
Original finding below, preserved from round 1.
The catch that tolerates a torn final JSON fragment also catches the loader's own record-shape errors. A complete final null, number or unrelated object is silently discarded. Separately, an object with any string kind is cast to InteractionLogEntry without checking the required data for that entry.
J4 writes a valid record followed by each of null, 42 and an unrelated object, all newline-terminated. All three open successfully instead of reporting corruption. J5 places an effect-frozen object with no effect in the interior of the file; it is also accepted. For the matching interaction, the core recovery reader accesses entry.effect.id directly at journal-recovery.ts:172.
This contradicts the adapter's stated refusal of non-entry lines and moves malformed persisted data past the typed journal boundary. These probes deliberately corrupt files; they do not claim the current runtime normally emits those records. Keep syntax truncation tolerance distinct from record validation, and report a useful line-numbered corruption error.
Fix direction. Catch JSON syntax errors separately from entry decoding. Reject complete invalid records at any position and validate required fields of recognized entry kinds before exposing InteractionLogEntry values. Share a small decoder at the journal boundary if appropriate; avoid maintaining a second divergent journal schema.
J4, J5: four failing cases
64: try {
65: const parsed: unknown = JSON.parse(trimmed);
66: if (
67: typeof parsed !== "object" ||
68: parsed === null ||
69: Array.isArray(parsed) ||
70: typeof (parsed as { kind?: unknown }).kind !== "string"
71: ) {
72: throw new Error("entry is not an object with a string kind");
73: }
74: entries.push(parsed as InteractionLogEntry);
75: } catch (error) {
76: // A crash mid-append can tear the final line (with or without its
77: // newline). A damaged interior line is a hole and must be refused.
78: if (index === lastNonEmpty) continue;
P2 F60. Preserve the optional idempotency key in receipt decodingpackages/core/src/types.ts:878-883logiccorrectness
DispatchReceipt declares idempotencyKey as optional, but validateReceipt now requires a string unconditionally. This makes the persisted decoder stricter than the exported InteractionJournal contract. A receipt identified by its externalOperationId is a valid typed value without an idempotency key.
L1 constructs a dispatch-acknowledged entry using InteractionLogEntry with no unsafe type assertion. The journal successfully appends it. The next ordinary append rereads that record, rejects it as corrupt because receipt.idempotencyKey is missing, and poisons the handle. A keyed receipt follows the same path successfully in L5.
This is a regression introduced by the new validator. The kernel's current invoke path supplies a key, which explains why runtime fixtures pass, but the public journal port accepts the wider declared receipt type. A valid acknowledged record must remain readable and appendable. Validate optional fields when present instead of silently narrowing the port contract.
Fix direction. Check receipt.idempotencyKey only when it is present. Keep the required receipt fields required, and cover both keyed and external-operation receipts in a typed write/read/append round trip.
L1 fails; L5 is the keyed-receipt control.
878: function validateReceipt(value: unknown, context: string): void { 879: const record = journalRecord(value, context); 880: journalString(record.effectId, `${context}.effectId`); 881: journalString(record.executor, `${context}.executor`); 882: journalString(record.idempotencyKey, `${context}.idempotencyKey`); 883: journalString(record.dispatchedAt, `${context}.dispatchedAt`);Prompt for AI agents