# Circe PR #6 review, round 3

COMMENT at e7743111bea382d634bd492a831f445f8e9833b8. No merge blockers; F65-F70 fixed, one nonblocking part of F71 remains.

No remaining merge blockers. The unique option lookup fixes the host identity ambiguity, and the operation registry now handles ordinary replay, payload conflicts and caller-owned membership arrays. The changes are localized and use the existing host/executor contracts. A nonblocking receipt-identity gap remains in the demo replay branch. Returning the original recorded outcome would also make that branch simpler.

All 394 repository tests pass: 323 evals, 40 journal-file, 17 perception-jev and 14 inspector. Repository and probe typechecks, CLI and git diff --check pass. All 28 cases in the previous inspector review suite pass, including the cleanup assertion. The 140 prior independent probes retain 138 passes and two inherited status-getter failures. Two new probes have one pass and one failure for the remaining F71 gap. Combined independent runs: 167 passes and three failures across 170 cases; two failures are inherited and one is the nonblocking demo receipt issue.

## Current F71 verification

F71 is substantially fixed: replay after drift, conflicting target payloads and caller-mutated effect membership now pass. One identity gap remains in the known-key branch. Its fingerprint omits effect identity, and its success receipt uses the incoming request.effectId. R3-1 repeats a recorded key and payload with a new effect ID and receives a success receipt for that never-recorded effect. Reject changed effect membership or return the original recorded receipt; do not mint a receipt from the replay request. This remains a nonblocking P2 in the demo executor.

## F65 P1 Constrain “the other one” to the bounded decision domain [fixed]

apps/inspector/src/scenario.ts:90-107

The scenario asks a referent decision whose options are A, B and none, but propositionsFromDecisions builds the executable include set from every candidate in the frame. Subtracting the risky choice from that larger set is only equivalent to “the other one” when the frame contains exactly the intended pair.

P6-2 adds an unrelated task C to the pipeline scope while leaving the accepted decisions unchanged: sketchy=B and operation=task.resume. The run reports RESOLVED and the pipeline invokes both A and C. C was never in the bounded referent question. The existing answer-inversion test proves the A/B case, but it does not establish that the host preserves the decision domain or singular cardinality.

For this deliberately fixed scenario, reject a scope that cannot identify a unique remaining member of the intended pair. Alternatively derive decision options and target selection from an explicitly identified domain. Do not broaden a singular request to every retrieved candidate. This needs a small contract check, not an English interpreter.

Fix: Bind the reference scenario to a unique, explicit A/B candidate domain and require one remaining target. Reject or clarify an incompatible scope before emitting executable propositions.

## F66 P1 Honor the atomic precondition contract at invocation [fixed]

apps/inspector/src/demo.ts:105-111

taskResume advertises conditionalApply="atomic", but the demo executor ignores InvokeRequest.preconditions and immediately records every binding as invoked. The earlier authority validation cannot provide atomic execution: the world may move after that validation and before the executor applies the request.

P6-3 wraps the authority validator so it returns current for A version 1, then changes A to version 2 before returning control to the runtime. The executor still records A and returns SUCCEEDED. The retained drift test moves the world before validation, so it exercises only the earlier check and misses this boundary.

The inspector is intended to demonstrate the full contract, and its executor is the part responsible for this guarantee. Compare all frozen preconditions against the demo world immediately before recording or applying any effect, with no asynchronous gap. On mismatch, fail without touching any target. If the demo does not implement that contract, it must not advertise atomic application.

Fix: Check every frozen precondition in the demo executor before any application; return a stale or invalidated failure on disagreement. Keep the check and apply synchronous within the in-process world.

## F67 P1 Reconcile from recorded executor outcomes [fixed]

apps/inspector/src/demo.ts:109-120

The demo reconciler returns SUCCEEDED for every receipt and keeps no operation records. A receipt proves that the runtime attempted dispatch; it does not prove that the executor applied the operation. The executor also ignores repeated idempotency keys while advertising runtime-key idempotency.

P6-8 runs the real pipeline with a dispatch wrapper that throws before calling the demo executor. The runtime correctly journals UNKNOWN, and world.invoked is empty. A fresh runtime recovers that journal and calls the demo reconciler, which changes the effect to SUCCEEDED even though no invocation occurred. P6-4 confirms the same result for a completely unknown receipt. P6-9 invokes the same request twice and records A twice.

Keep a small operation-result map in the demo world, keyed and checked by the dispatched operation identity. Known duplicate keys should return the recorded result without another application. Reconciliation must return UNKNOWN for absent or mismatched operations. No durable external executor service is needed here; after a fresh demo world loses its operation records, UNKNOWN is the truthful result.

Fix: Record applied operations and results in the demo executor, deduplicate known idempotency keys, and reconcile only matching recorded outcomes. Unknown receipts must remain UNKNOWN.

## F68 P2 Decode canonical identity components before returning refs [fixed]

apps/inspector/src/demo.ts:66-70

The authority stores entities under entityRefKey, then splits that escaped key on pipes and treats each component as the original string. entityRefKey escapes colons, pipes and percent signs. Splitting identifies component boundaries but does not undo those escapes.

P6-1 observes the valid ID A:pipe|percent%. The returned ID is A%3Apipe%7Cpercent%25, a different entity. When observation ingestion keys it again, it is escaped a second time. The originally requested reference no longer matches its evidence, causing valid targets to disappear or lack usable version evidence.

Use the existing decodeIdentityComponent helper for all four components, or retain the original EntityRef alongside the demo entity. This is an identity fix; the simple A/B fixtures currently hide it.

Fix: Decode all split components with the core identity helper, or store original refs instead of reconstructing them from keys.

## F69 P2 Close writable test journals before deleting their directories [fixed]

apps/inspector/src/pipeline.test.ts:29-49

The input helper creates a FileJournal but returns it through the narrower InteractionJournal field. Tests pass it inline to the pipeline and then remove the temporary directory in finally without closing the writable handle. The pipeline correctly treats its injected journal as caller-owned, so it cannot clean up the lost reference for these tests.

The independent cleanup probe imports and runs all six retained inspector tests, then inspects /proc/self/fd. Four descriptors still point to deleted circe-inspector journal files. Linux permits the deletion and the tests pass, but repeated runs leak descriptors and hold the unlinked files until process exit. Read-only recovery handles are lazy and do not account for these four leaks.

Retain each FileJournal in the test fixture and close it in finally before rmSync. Apply the same explicit ownership pattern to any handle that can append. The CLI closes its writable journal already; this finding concerns the test fixture.

Fix: Have the fixture retain and expose the FileJournal or a cleanup callback; close writable journals before removing temporary directories in every finally block.

## F70 P2 Remove the unused recursive reference walker [fixed]

packages/perception/src/proposals.ts:298-316

The only substantive shared perception change in this PR adds collectRefKeys, but nothing calls it outside its own recursive body. The active argument exclusion and evidence paths already use argumentRefsOf. This new traversal contributes no protection to the inspector pipeline.

Keeping an unused second implementation makes the security-sensitive argument handling harder to assess and suggests a check is active when it is not. The new helper also lacks the safeguards of the existing argument validation path. It is dead code, so that difference is not currently an execution defect.

Delete the unused helper and the stray blank line added in compileSemanticTurn. The inspector PR can then leave this shared compiler file unchanged. This cleanup is nonblocking, but it directly reduces unnecessary code in an already reviewed boundary.

Fix: Remove collectRefKeys and its incidental whitespace change; retain the existing argumentRefsOf-based validation.

## F71 P2 Store immutable request identity and the original operation result [open]

apps/inspector/src/demo.ts:120-155

The new operation map stores effect IDs and refs, but no request identity or result. invoke validates current preconditions before checking the map; when a key already exists, it skips application and synthesizes success for the incoming request. These paths do not implement replay of a recorded operation.

R2-1 applies A at version 1, changes the world to version 2, then repeats the identical request. The repeated call returns STALE_REFERENCE despite the recorded success. R2-2 reuses the same key and effect ID with target B: the executor returns success even though only A was applied. Reconciliation and invoke can therefore disagree about the same operation, or success can be attached to a different payload.

R2-3 exposes a related ownership gap: request.effectIds is stored by reference. Appending never-applied to that array after invocation causes reconciliation to report SUCCEEDED for that effect. The record must be a detached account of what happened, rather than mutable caller data.

Keep the small in-process map, but store a detached operation fingerprint, effect membership and original result. On a known key, check identity and return the recorded result before consulting current world versions; reject conflicting payloads. Check live preconditions only for a new operation. This is a nonblocking P2 in the demo, and needs no additional service or generic retry framework.

Fix: Snapshot request identity, effect IDs and the applied result. Resolve known matching keys from that record before revalidating world state; reject conflicting key reuse and detach stored arrays.

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-inspector-e2e-abbyview.json
HTML: /home/abstergo/Documents/reviews/Circe-inspector-e2e-abbyview.html

In `apps/inspector/src/demo.ts` around lines 137-156, finish F71 by keeping the known-key receipt tied to recorded effect identity. The original R2-1/R2-2/R2-3 cases now pass. R3-1 in `/tmp/circe-pr6-evidence/round3.test.ts` repeats the same key and target payload with a different effectId/effectIds and receives SUCCEEDED with a receipt for that never-recorded effect. The fingerprint excludes effect identity and the replay branch constructs its receipt from the incoming request. Reject changed effect membership or return the original recorded receipt, rather than synthesizing a new successful receipt. Preserve matching replay after drift and known batch-member reconciliation; keep this a small in-process registry fix.
