# Circe PR #6 review, round 2

CHANGES_REQUESTED at 0d9e510b77a44c8f9f522a017876f27c60630453. F65 and F71 open; F66-F70 fixed.

The fixes stay within the right modules. Atomic precondition checks run synchronously in the demo executor; the file-journal fixture now owns its handles; the unused compiler traversal is gone. A small operation map is appropriate. Two contracts remain incomplete: option-to-entity identity must be unique, and an operation record must preserve the original request and outcome. These need local checks and snapshots, not more architecture.

All 390 repository tests pass: 323 evals, 40 journal-file, 17 perception-jev and 10 inspector. Typecheck, probe typecheck, CLI and git diff --check pass. All nine original behavioral probes and the cleanup assertion over ten retained tests pass, 19 tests total. The 140 prior probes retain 138 passes and two inherited status-getter failures. Five new probes have four failures and one passing control; the failures support the remaining F65 gap and new F71.

## F65 verification update

The new domain guard rejects C and checks the number of remaining entities, but it does not require each option to identify a unique canonical ref. R2-4 assembles two observed candidates with local ID B under different authorities and no A. Both pass the option-membership check. refForChoice picks the first B, and the other B becomes an executable resume target. Keep F65 open: require an unambiguous A/B option-to-ref mapping, or reject this incompatible scope.

## 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.

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

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.

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/scenario.ts` around lines 93-108, finish F65 by checking a unique canonical entity mapping for each bounded option. The C expansion case now passes, but R2-4 in `/tmp/circe-pr6-evidence/round2.test.ts` supplies two B refs with different authorities and no A. Both pass the membership check, the first is excluded, and the second gets an executable resume command. For this fixed scenario, require exactly one intended A and B with unambiguous identities, or reject incompatible scopes. Do not infer identity from a bare local ID with first-match selection. Preserve the normal A/B inversion control and the extra-C rejection.

In `apps/inspector/src/demo.ts` around lines 120-155, make the new operation map a detached record of request identity and result. R2-1 in `/tmp/circe-pr6-evidence/round2.test.ts` changes the world after success and gets a stale failure on an identical replay. R2-2 reuses the key with target B and gets success without applying B. R2-3 mutates the supplied effectIds array after invoke and can then reconcile a never-applied effect as successful. For an existing key, compare a stable operation fingerprint and return the original result before live precondition checks; reject conflicting payloads. Copy effect membership and results when storing them. Keep the in-process map and the absent-operation UNKNOWN behavior; no new framework is needed.
