PR #3: journal recovery is not ready to merge
8 findings · 6 P1 · 2 P2
294 repo tests and 59 prior probes pass · 12 new assertions fail
+885 / -40 8d9cc15The journal approach fits Circe, but this implementation does not yet preserve the kernel's behavior across restarts. I found eight actionable issues, six P1 and two P2, backed by 12 failing assertions. Several appear on clean restart; the others use actual prefixes of valid journals, without fabricated corruption.
Reviewed on 23 September 2026 at PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. The accepted-turn review remains closed separately. No numeric score, as requested.
Rebase status
PR #3 had already been rebased by the time I inspected it. Its direct parent is 84da27e3a628d25851f669ff81a59c9212daef02, the merge of PR #7 and the current GitHub main head. I checked out its branch in /tmp/circe-pr3-review and ran git rebase origin/main; Git reported it was up to date. There were no conflicts to resolve, no commits to rewrite and nothing to push. The user's active inspector worktree was left unchanged.
The first git fetch hit a DNS failure. GitHub API reads succeeded and independently confirmed main and the PR head, both identical to the local objects and remote-tracking refs. The review uses those exact commits.
Assessment
The basic boundaries are sensible: core owns reconstruction, hosts provide an append/read journal, and uncertain external effects become UNKNOWN for reconciliation. Full intent and transaction records are justified because counters and a transcript cannot reconstruct authorized work. Moving TurnResult to shared types is also reasonable.
The problem is the recovery protocol. The writer appends related state in separate records while the reader combines whichever snapshots survived. It then guesses completion, ownership or receipt identity in places where the journal has not recorded enough information. That is the source of the failures, not the absence of a disk implementation in this PR.
For example, a two-target intent produces two separate effect-frozen entries. A crash after the first leaves one effect. Recovery reattaches it and permits resume; the transaction then reports success for only A, silently losing B. In another prefix, the effect outcome is SUCCEEDED but the final transaction snapshot has not been appended. Recovery exposes the still-live transaction and admits a named continuation that the uninterrupted runtime rejects.
The durable schema needs to identify complete operations before recovery can safely expose them. A complete freeze record or manifest, a pre-dispatch envelope, scoped reservation/progress records and one shared settlement rule address demonstrated failures. They do not require a generic event-sourcing framework or a rewrite of semantic perception.
Verification
- Repository baseline: 294 tests pass, including 15 existing journal-recovery tests.
pnpm typecheck: passes.- All 59 earlier independent reviewer probes pass on this PR head.
- New recovery suite: 12 failing assertions and three passing controls across 15 tests.
- New recovery test file passes strict TypeScript checking.
The controls cover exact same-payload replay without invoking, resumption of a complete paused two-target effect set, and rejection of a settled named continuation without restart. The tests use the public runtime, real emitted journals and writable copies of valid prefixes. They test logical reconstruction, not physical fsync durability, which belongs to the later file-journal PR.
One exploratory failed-effect dispatch case was replaced before the final suite because it overlapped pre-existing live dispatch behavior. The retained terminal-state case specifically contrasts a correct uninterrupted runtime against incorrect recovery. No claim depends on that discarded experiment.
Test coverage gap
The PR's tests show that whole completed commands replay, that one frozen effect reattaches, and that a RUNNING effect becomes UNKNOWN. They do not test completeness of a multi-effect freeze, shared batch identity after a missing acknowledgement, command payload conflicts without the final outcome, or the last presented gate across a restart.
Use a prefix-cut matrix over representative workflows: complete proposal, multi-target freeze, paused confirmation, batch dispatch, terminal outcome, revision and cancellation. At every append boundary, recovery should either expose equivalent state or an explicit incomplete/UNKNOWN state. It must never silently discard authorized work, invent a sent operation key, revive settled work, or let another interaction consume a command ID.
Scope and next step
Fix the durable record contracts before layering the file adapter on them. First preserve exact interaction/command identity and the presented anchor. Then make freeze completeness and dispatch identity durable. Finally restore complete outcome data, normalize ephemeral reservations and derive coherent transaction settlement before accepting new commands. Keep the passing controls and the 59 earlier probes throughout.
Full-result command caching and whole discourse snapshots have a real growth cost, as the PR acknowledges. Compaction can remain a follow-up; it cannot compensate for missing information or ambiguous boundaries. I would not block on adding compaction or a disk adapter here.
This review covers PR #3 only. It neither changes the accepted PR #2/#7 verdict nor approves PRs #4 through #6. No Circe source edits, GitHub comments, approvals or merges were submitted.
cd /tmp/circe-pr3-review
pnpm exec vitest run --config /tmp/circe-pr3-evidence/vitest.config.mjs recovery.test.ts --reporter=verbose
At the reviewed head this yields 12 failed and three passed. Downloaded configs contain local paths and need alias updates elsewhere.
Implementation walkthrough
PR #3 adds startup reconstruction to the journal port. It records full intents and transaction snapshots, discourse snapshots, accepted commands and final command results; recoverState reconstructs maps and marks unresolved dispatches UNKNOWN.
- createRuntime calls recoverState before exposing command and ingestion entry points.
- Mutation writers append intent, transaction, effect and outcome records at different points in each command.
- Recovery loads the latest snapshots, reattaches frozen effects and caches reconstructed command results.
- Reconciliation operates on recovered UNKNOWN effects without an automatic retry.
Changed files
packages/core/src/index.ts+1 / -0packages/core/src/journal-recovery.ts+271 / -0packages/core/src/runtime.ts+107 / -36packages/core/src/types.ts+91 / -4packages/evals/src/journal-recovery.test.ts+415 / -0
Journal ordering and recovery
flowchart TD A[Accepted command] --> B[Intent and transaction snapshots] B --> C[Per-effect freeze records] C --> D[Dispatch request records] D --> E[External invoke] E --> F[Effect outcome records] F --> G[Transaction settlement snapshot] G --> H[Discourse snapshot and command outcome] J[Restart from any durable prefix] --> K[recoverState] K --> L[Restore runtime maps and accept new commands]
New recovery probes 12 failures and 3 controls 59 prior cases Repo baseline Repo typecheck Test config Review sidecar
Findings
- P1 F46. Restart loses the presented gate and can redirect unaddressed actions
- P1 F47. Recovery is not fully scoped to an exact interaction identity
- P1 F48. Acceptance is treated as completion and reconstructed commands lose fingerprints
- P1 F49. Crash recovery invents batch receipt keys that were never dispatched
- P2 F50. Clean recovery drops completed effect results and failure details
- P1 F51. A partially recorded effect set can execute as if it were complete
- P2 F52. Recovery restores a validation reservation whose worker no longer exists
- P1 F53. Recovered effect outcomes do not settle the transaction before new commands
Prompt for all findings · Ctrl+`
Prompt to rerun · Ctrl+Space
P1 F47. Recovery is not fully scoped to an exact interaction identitypackages/core/src/journal-recovery.ts:200-203logic data-integrity
The command-accepted branch populates acceptedKeys without checking entry.interactionId, although the field is available. Later, recovery synthesizes cached results for these foreign keys. Effect records use startsWith(effectPrefix) instead of an exact interaction identity, so valid IDs A and A:child also overlap.
D3 writes command same in interaction A, then starts interaction B on the shared journal. B's own command same returns an empty reconstructed result and never invokes. D4 records an effect under A:child and recovers A; A imports the other interaction's effect despite having none of its intents or transactions. The existing isolation test uses different command IDs and does not assert effect isolation.
The journal is advertised as supporting multiple interactions, so every recovered namespace must apply the same exact boundary. The effect overlap can also collide with local transaction IDs such as tx:1 when those records exist in both interactions.
Fix direction. Filter command acceptance by exact interactionId. Give effect, dispatch and outcome records an explicit interactionId or use a provably unambiguous identity encoding and exact ownership check. Test identical local command IDs across interactions and delimiter-containing interaction IDs.
Source at reviewed head · Probes D3, D4
Captured source
200: case "command-accepted":
201: if (entry.commandId) {
202: acceptedKeys.set(`command:${entry.commandId}`, entry.commandId);
203: }Prompt for AI agents
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/journal-recovery.ts` around lines 200-203, The command-accepted branch populates acceptedKeys without checking entry.interactionId, although the field is available. Later, recovery synthesizes cached results for these foreign keys. Effect records use startsWith(effectPrefix) instead of an exact interaction identity, so valid IDs A and A:child also overlap. Filter command acceptance by exact interactionId. Give effect, dispatch and outcome records an explicit interactionId or use a provably unambiguous identity encoding and exact ownership check. Test identical local command IDs across interactions and delimiter-containing interaction IDs. Reproduce D3, D4 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario.
P1 F51. A partially recorded effect set can execute as if it were completepackages/core/src/journal-recovery.ts:210-215logic data-integrity
commit writes one effect-frozen record per target and only later writes confirmation and the transaction transition containing the full effect list. Recovery reattaches whichever effects survived, but there is no durable manifest or completion marker proving that the recovered list is the complete authorized set.
D9 proposes targets A and B and restarts from the actual journal prefix ending after the first effect-frozen record. Resume executes only A and settles the transaction as SUCCEEDED. B vanishes from the effect set even though the recovered intent still contains both targets. A complete-freeze control correctly resumes both targets.
Reattaching an orphan effect is useful, but it cannot establish that no sibling is missing. Before dispatch, recovery must know whether the freeze was complete. Otherwise it silently changes the operation the user requested.
Fix direction. Record the complete frozen effect set and its transaction/revision association as one durable unit, or use a manifest plus completion record. On recovery, incomplete freezes must be completed deterministically with stable identities or remain explicitly blocked. Never mark a subset successful as the whole requested operation.
Source at reviewed head · Probes D9
Captured source
210: // A crash between freezing and the next transaction transition must not
211: // orphan authorized effects or allow a second effect set for the revision.
212: for (const effect of effects.values()) {
213: const tx = transactions.get(effect.transactionId);
214: if (tx && !tx.effects.includes(effect.id)) tx.effects.push(effect.id);
215: }Prompt for AI agents
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/journal-recovery.ts` around lines 210-215, commit writes one effect-frozen record per target and only later writes confirmation and the transaction transition containing the full effect list. Recovery reattaches whichever effects survived, but there is no durable manifest or completion marker proving that the recovered list is the complete authorized set. Record the complete frozen effect set and its transaction/revision association as one durable unit, or use a manifest plus completion record. On recovery, incomplete freezes must be completed deterministically with stable identities or remain explicitly blocked. Never mark a subset successful as the whole requested operation. Reproduce D9 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario.
P1 F49. Crash recovery invents batch receipt keys that were never dispatchedpackages/core/src/journal-recovery.ts:229-238logic data-integrity
dispatch-requested stores only effectId and a timestamp. When acknowledgement is absent, recoverState fabricates a receipt with idempotencyKey equal to each member effect ID and executor set to unknown. The runtime actually submits a batch under the first member's key, so the persisted record is insufficient to reconstruct what was sent.
D7 runs a two-target batch and restarts from the prefix before its first acknowledgement. Both effects correctly become UNKNOWN. Reconciling the second effect asks about its own ID, although only the first effect's ID was used for the external operation. The recorded executor is also lost. A real executor cannot reliably find the second operation because it never existed under that key.
This is precisely the lost-response boundary that the PR promises to handle without retry. The dispatch identity has to be durable before the external call; an acknowledgement cannot be the first place the true batch identity is recorded.
Fix direction. Write the actual dispatch envelope before invoking: interaction, executor, operation/idempotency key, and full member effect IDs. Build synthetic lost-ack receipts from that record for every member. Verify all recovered receipts use the actual batch key and no recovery path reinvokes uncertain work.
Source at reviewed head · Probes D7
Captured source
229: for (const effect of effects.values()) {
230: if (!dispatchedAt.has(effect.id) || terminal.has(effect.id)) continue;
231: if (effect.status !== "PENDING" && effect.status !== "RUNNING") continue;
232: effect.status = "UNKNOWN";
233: effect.failure = undefined;
234: effect.receipt ??= {
235: effectId: effect.id,
236: executor: "unknown",
237: idempotencyKey: effect.id,
238: dispatchedAt: dispatchedAt.get(effect.id)!,Prompt for AI agents
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/journal-recovery.ts` around lines 229-238, dispatch-requested stores only effectId and a timestamp. When acknowledgement is absent, recoverState fabricates a receipt with idempotencyKey equal to each member effect ID and executor set to unknown. The runtime actually submits a batch under the first member's key, so the persisted record is insufficient to reconstruct what was sent. Write the actual dispatch envelope before invoking: interaction, executor, operation/idempotency key, and full member effect IDs. Build synthetic lost-ack receipts from that record for every member. Verify all recovered receipts use the actual batch key and no recovery path reinvokes uncertain work. Reproduce D7 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario.
P1 F48. Acceptance is treated as completion and reconstructed commands lose fingerprintspackages/core/src/journal-recovery.ts:242-252logic data-integrity
Recovery turns every accepted key without a command-outcome into a completed cached TurnResult, even if no intent or transaction was written. It also restores fingerprints only from command-outcome or command-failed, ignoring the accepted command payload. Acceptance, execution progress and completion are therefore conflated.
D6 keeps a real journal prefix ending immediately after command-accepted. On restart, redelivery returns an empty result with no errors, creates no intent and does no work; that key is consumed permanently. D5 cuts only the final command-outcome after A already succeeded. Redelivering the same ID with target B is accepted as a cached A result instead of raising the required payload-conflict error.
A conservative no-retry policy is appropriate after uncertain external execution, but acceptance alone is not evidence of execution. Recovery needs a visible incomplete state or a safe resumption path, plus the immutable accepted fingerprint. Silently synthesizing completion hides lost work.
Fix direction. Persist a scoped reservation with the original fingerprint before applying the command, and represent accepted, partially applied and completed states separately. Recover never-started commands or surface an explicit recoverable failure; retain no-reinvoke behavior after dispatch. Restore and compare fingerprints before any cached result is returned.
Source at reviewed head · Probes D5, D6
Captured source
242: // Accepted commands whose outcome was never recorded are reconstructed
243: // from the state they produced, so a replay cannot execute them again.
244: for (const tx of transactions.values()) {
245: if (tx.commandKey) acceptedKeys.set(tx.commandKey, tx.commandId ?? tx.commandKey);
246: }
247: for (const key of acceptedKeys.keys()) {
248: if (commandResults.has(key)) continue;
249: commandResults.set(
250: key,
251: synthesizeResult(key, transactions, intents, effects),
252: );Prompt for AI agents
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/journal-recovery.ts` around lines 242-252, Recovery turns every accepted key without a command-outcome into a completed cached TurnResult, even if no intent or transaction was written. It also restores fingerprints only from command-outcome or command-failed, ignoring the accepted command payload. Acceptance, execution progress and completion are therefore conflated. Persist a scoped reservation with the original fingerprint before applying the command, and represent accepted, partially applied and completed states separately. Recover never-started commands or surface an explicit recoverable failure; retain no-reinvoke behavior after dispatch. Restore and compare fingerprints before any cached result is returned. Reproduce D5, D6 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario.
P1 F46. Restart loses the presented gate and can redirect unaddressed actionspackages/core/src/types.ts:560-570logic correctness
DiscourseSnapshot omits presentedGateId and the fact that a gate has already been presented. Neither finalize nor the startup restore preserves these fields. Transactions are reconstructed, but the runtime forgets which one the user saw and incorrectly treats the interaction as never having presented a gate.
After proposing A and then B, a clean restart makes an unaddressed confirmation do nothing, instead of confirming B. If B has already succeeded before restart, an unaddressed cancellation falls back to older A and cancels it. D1 and D2 reproduce both behaviors from complete, valid journals; no crash truncation is needed.
This reintroduces the transaction-targeting behavior that PR #2 corrected. Persist and restore the presented identity even after that transaction settles. Keep the no-fallback semantics; do not infer the anchor from whichever live transaction happens to be first.
Fix direction. Extend the durable discourse contract and its writer/reader with the presented anchor and its lifetime semantics. Preserve the settled anchor tombstone or an equivalent durable fact. Verify both current-gate confirmation and no fallback to older work after settlement.
Source at reviewed head · Probes D1, D2
Captured source
560: export interface DiscourseSnapshot {
561: scopes: Scope[];
562: scopeStack: ScopeId[];
563: retainedScopes: ScopeId[];
564: views: WorldView[];
565: referents: Referent[];
566: currentReferentId?: ReferentId;
567: recentReferentIds: ReferentId[];
568: pendingQuestion?: PendingQuestion;
569: pendingFrame?: { capability: CapabilityId };
570: metrics: MetricsBucket;Prompt for AI agents
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 560-570, DiscourseSnapshot omits presentedGateId and the fact that a gate has already been presented. Neither finalize nor the startup restore preserves these fields. Transactions are reconstructed, but the runtime forgets which one the user saw and incorrectly treats the interaction as never having presented a gate. Extend the durable discourse contract and its writer/reader with the presented anchor and its lifetime semantics. Preserve the settled anchor tombstone or an equivalent durable fact. Verify both current-gate confirmation and no fallback to older work after settlement. Reproduce D1, D2 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario.
P2 F52. Recovery restores a validation reservation whose worker no longer existspackages/core/src/journal-recovery.ts:138-142logic correctness
Transaction snapshots include pendingCommitRevision and recoverState restores it verbatim. This field is an in-process compare-and-set reservation for an outstanding authority await, not a durable confirmation. No validation worker survives createRuntime after a restart.
D10 starts confirmation with validation still awaiting, then records an interrupt. touchedTx journals the paused transaction with pendingCommitRevision still set. After restart, even explicitly resuming and confirming that transaction cannot invoke: commit sees the stale reservation and returns. There are no frozen effects and no surviving operation that could clear the reservation.
The reservation was necessary for live concurrency safety, but restoring it as an active lock makes recoverable work permanently uncommittable. Durable authorization and ephemeral in-flight ownership need different recovery rules.
Fix direction. Normalize in-flight validation reservations during recovery. Clear or recoverably abandon pendingCommitRevision when no durable committed effect set owns it, then require fresh validation before commit. Preserve duplicate-confirm protection within a live runtime and do not treat the stale lock as authorization.
Source at reviewed head · Probes D10
Captured source
138: case "transaction-transition":
139: if (entry.interactionId === interactionId) {
140: transactions.set(entry.transaction.id, structuredClone(entry.transaction));
141: lastTransitionIndex.set(entry.transaction.id, index);
142: }Prompt for AI agents
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/journal-recovery.ts` around lines 138-142, Transaction snapshots include pendingCommitRevision and recoverState restores it verbatim. This field is an in-process compare-and-set reservation for an outstanding authority await, not a durable confirmation. No validation worker survives createRuntime after a restart. Normalize in-flight validation reservations during recovery. Clear or recoverably abandon pendingCommitRevision when no durable committed effect set owns it, then require fresh validation before commit. Preserve duplicate-confirm protection within a live runtime and do not treat the stale lock as authorization. Reproduce D10 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario.
P2 F50. Clean recovery drops completed effect results and failure detailspackages/core/src/journal-recovery.ts:174-185logic data-integrity
Recovery reconstructs effects from effect-frozen and status-only outcome records. invokeBatch updates effect.result or effect.failure in memory but emits outcome without those payloads; reconciliation also omits successful results and failure details. The full command-outcome is loaded into the result cache, not used to reconstruct current effects.
The two D8 cases complete successfully and fail with a structured executor error, then restart from an intact journal. The recovered effect retains its status but loses the success result or failure object. Cached replay can still contain data that runtime.state() no longer has, giving different answers for the same durable outcome.
This is a data-fidelity defect on ordinary clean restart, independent of disk corruption or torn writes. Outcome records should be sufficient to rebuild the effect itself rather than relying on an unrelated command response cache.
Fix direction. Journal the complete outcome data needed by Effect, including structured result, failure and timestamps, for invoke, observation and reconciliation writers. Reconstruct those fields consistently and test success/failure state equality after clean recovery.
Source at reviewed head · Probes D8 success and failure
Captured source
174: case "outcome":
175: case "reconciled": {
176: const effect = effects.get(entry.effectId);
177: if (!effect) break;
178: const status: EffectStatus = VALID_EFFECT_STATUSES.has(entry.status)
179: ? entry.status
180: : "UNKNOWN";
181: effect.status = status;
182: if (entry.detail !== undefined) effect.result = structuredClone(entry.detail);
183: if (TERMINAL_EFFECT_STATUSES.has(status)) {
184: effect.endedAt = entry.at;
185: terminal.add(entry.effectId);Prompt for AI agents
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/journal-recovery.ts` around lines 174-185, Recovery reconstructs effects from effect-frozen and status-only outcome records. invokeBatch updates effect.result or effect.failure in memory but emits outcome without those payloads; reconciliation also omits successful results and failure details. The full command-outcome is loaded into the result cache, not used to reconstruct current effects. Journal the complete outcome data needed by Effect, including structured result, failure and timestamps, for invoke, observation and reconciliation writers. Reconstruct those fields consistently and test success/failure state equality after clean recovery. Reproduce D8 success and failure in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario.
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Sidecar: /home/abstergo/Documents/reviews/Circe-journal-recovery-abbyview.json HTML: /home/abstergo/Documents/reviews/Circe-journal-recovery-abbyview.html Reviewed PR #3 head: 8d9cc151f6f62fb825d041b3a2b546773a8a3fff In `packages/core/src/types.ts` around lines 560-570, DiscourseSnapshot omits presentedGateId and the fact that a gate has already been presented. Neither finalize nor the startup restore preserves these fields. Transactions are reconstructed, but the runtime forgets which one the user saw and incorrectly treats the interaction as never having presented a gate. Extend the durable discourse contract and its writer/reader with the presented anchor and its lifetime semantics. Preserve the settled anchor tombstone or an equivalent durable fact. Verify both current-gate confirmation and no fallback to older work after settlement. Reproduce D1, D2 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario. In `packages/core/src/journal-recovery.ts` around lines 200-203, The command-accepted branch populates acceptedKeys without checking entry.interactionId, although the field is available. Later, recovery synthesizes cached results for these foreign keys. Effect records use startsWith(effectPrefix) instead of an exact interaction identity, so valid IDs A and A:child also overlap. Filter command acceptance by exact interactionId. Give effect, dispatch and outcome records an explicit interactionId or use a provably unambiguous identity encoding and exact ownership check. Test identical local command IDs across interactions and delimiter-containing interaction IDs. Reproduce D3, D4 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario. In `packages/core/src/journal-recovery.ts` around lines 242-252, Recovery turns every accepted key without a command-outcome into a completed cached TurnResult, even if no intent or transaction was written. It also restores fingerprints only from command-outcome or command-failed, ignoring the accepted command payload. Acceptance, execution progress and completion are therefore conflated. Persist a scoped reservation with the original fingerprint before applying the command, and represent accepted, partially applied and completed states separately. Recover never-started commands or surface an explicit recoverable failure; retain no-reinvoke behavior after dispatch. Restore and compare fingerprints before any cached result is returned. Reproduce D5, D6 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario. In `packages/core/src/journal-recovery.ts` around lines 229-238, dispatch-requested stores only effectId and a timestamp. When acknowledgement is absent, recoverState fabricates a receipt with idempotencyKey equal to each member effect ID and executor set to unknown. The runtime actually submits a batch under the first member's key, so the persisted record is insufficient to reconstruct what was sent. Write the actual dispatch envelope before invoking: interaction, executor, operation/idempotency key, and full member effect IDs. Build synthetic lost-ack receipts from that record for every member. Verify all recovered receipts use the actual batch key and no recovery path reinvokes uncertain work. Reproduce D7 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario. In `packages/core/src/journal-recovery.ts` around lines 174-185, Recovery reconstructs effects from effect-frozen and status-only outcome records. invokeBatch updates effect.result or effect.failure in memory but emits outcome without those payloads; reconciliation also omits successful results and failure details. The full command-outcome is loaded into the result cache, not used to reconstruct current effects. Journal the complete outcome data needed by Effect, including structured result, failure and timestamps, for invoke, observation and reconciliation writers. Reconstruct those fields consistently and test success/failure state equality after clean recovery. Reproduce D8 success and failure in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario. In `packages/core/src/journal-recovery.ts` around lines 210-215, commit writes one effect-frozen record per target and only later writes confirmation and the transaction transition containing the full effect list. Recovery reattaches whichever effects survived, but there is no durable manifest or completion marker proving that the recovered list is the complete authorized set. Record the complete frozen effect set and its transaction/revision association as one durable unit, or use a manifest plus completion record. On recovery, incomplete freezes must be completed deterministically with stable identities or remain explicitly blocked. Never mark a subset successful as the whole requested operation. Reproduce D9 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario. In `packages/core/src/journal-recovery.ts` around lines 138-142, Transaction snapshots include pendingCommitRevision and recoverState restores it verbatim. This field is an in-process compare-and-set reservation for an outstanding authority await, not a durable confirmation. No validation worker survives createRuntime after a restart. Normalize in-flight validation reservations during recovery. Clear or recoverably abandon pendingCommitRevision when no durable committed effect set owns it, then require fresh validation before commit. Preserve duplicate-confirm protection within a live runtime and do not treat the stale lock as authorization. Reproduce D10 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario. In `packages/core/src/journal-recovery.ts` around lines 174-185, The reducer updates effect status but leaves transaction state at the last separately written transaction-transition. After scanning the journal it reattaches effects, yet never derives the transaction lifecycle from those recovered outcomes. A valid crash prefix can therefore contain SUCCEEDED effects with a transaction still marked COMMITTED. After rebuilding verified complete effect sets, derive transaction settlement from recovered effect outcomes using the same lifecycle rules as the runtime. Preserve cancellations and superseding revisions. Expose a coherent state before accepting continuations or resumes; add an outcome-before-transition crash-prefix regression. Reproduce D11 in `/tmp/circe-pr3-evidence/recovery.test.ts` against PR #3 head 8d9cc151f6f62fb825d041b3a2b546773a8a3fff. Keep the three recovery controls and all 59 earlier reviewer probes passing. Run the repo test suite and typecheck. Change the durable record and recovery rule together; do not patch only the test scenario.
/abbyview journal-recovery Round 2. Sidecar: /home/abstergo/Documents/reviews/Circe-journal-recovery-abbyview.json Incremental: preserve baseline and original finding bodies. Review only changes since 8d9cc151f6f62fb825d041b3a2b546773a8a3fff and affected paths. Verify F46-F53 using the recovery suite and retain all 59 earlier probes. Append only new findings. Do not conflate this with disk-adapter or later stack approval.
P1 F53. Recovered effect outcomes do not settle the transaction before new commandspackages/core/src/journal-recovery.ts:174-185logic correctness
The reducer updates effect status but leaves transaction state at the last separately written transaction-transition. After scanning the journal it reattaches effects, yet never derives the transaction lifecycle from those recovered outcomes. A valid crash prefix can therefore contain SUCCEEDED effects with a transaction still marked COMMITTED.
D11 keeps the prefix through a single effect's successful outcome, immediately before the final transaction transition. After restart, a named continuation of that completed intent passes the live-intent check and executes a second operation under a new key. The same continuation is correctly rejected once the uninterrupted runtime has settled; D14 is the positive control for that rejection.
Effect completion is already durable at this boundary. Recovery must reconcile the owning transaction before exposing state to command selection. It must also distinguish complete effect sets from partial freezes, which is the separate F51 requirement.
Fix direction. After rebuilding verified complete effect sets, derive transaction settlement from recovered effect outcomes using the same lifecycle rules as the runtime. Preserve cancellations and superseding revisions. Expose a coherent state before accepting continuations or resumes; add an outcome-before-transition crash-prefix regression.
Source at reviewed head · Probes D11
Captured source
174: case "outcome": 175: case "reconciled": { 176: const effect = effects.get(entry.effectId); 177: if (!effect) break; 178: const status: EffectStatus = VALID_EFFECT_STATUSES.has(entry.status) 179: ? entry.status 180: : "UNKNOWN"; 181: effect.status = status; 182: if (entry.detail !== undefined) effect.result = structuredClone(entry.detail); 183: if (TERMINAL_EFFECT_STATUSES.has(status)) { 184: effect.endedAt = entry.at; 185: terminal.add(entry.effectId);Prompt for AI agents