Follow-up from the review of #2243. Assessed as design-shape rather than correctness — we tried to construct a wrong-result path and could not — so it was filed rather than folded into a converging stack.
What
In packages/storage/src/system-record-verified-replacement-v1-internal.ts, issueCandidate (:741-907) combines exact-shape dispatch, validation, reservation accounting, fact construction, handle registration, and a promotion path in one method. The quarantine branch is the indirect part: it creates a registered active proof and then rewrites it into a quarantine proof.
Measured specifics at 1db4bc36b:
:757 issuer.issueActive(activeInput) and :758 registeredHandle(handle) — both outside the try opening at :759.:806-814 mutate registry internals: AUTHENTIC_VERIFIED_REPLACEMENT_FACTS.add(facts), FACT_RESERVATIONS.set(facts, registered.reservation), then const superseded = registered.facts; registered.facts = facts; and .delete(superseded) on both.RegisteredReplacementV1.facts is declared non-readonly at :219 solely to permit :812; every sibling field is readonly.
Target shape
A small dispatcher calling operation-specific pure validators/fact builders — active validates active input, quarantine reuses a pure active-fact builder without registering it first, tombstone builds terminal facts — then one shared registerReplacement(bindings, facts, reservation) that registers the final facts exactly once. That removes the handle reopening, the mutable registered.facts, the WeakSet delete/re-add, and two of the three as unknown as casts (:749, :756; the third at :862 is unrelated to dispatch).
Why it is worth doing, beyond tidiness
:757-758 sit outside the try and are safe only because registeredHandle cannot throw — the handle was registered two frames earlier with the same registry identity. Nothing at the site states that. The reservation gate is non-queued single-lease and production shares one gate across runtimes, so a leak here would brick system-record materialization process-wide rather than fail locally. Today the catch at :816-819 releases correctly and it is pinned by packages/storage/test/system-record-verified-replacement-v1.test.ts:281-293 (three validation failures, each followed by issueActive on the same registry, which can only succeed if the lease was freed). But the current shape is one edit away from a real leak, and the proposed restructure removes that hazard by construction.
Constraints for whoever takes it
- Blast radius is contained to this one file: the issuer surface keeps both methods, so all ~55 call sites across the storage tests and fixtures compile unchanged.
- The pins are the risk, not the diff. The refactor deletes or moves the exact lines the reservation-lifecycle pins reference, including the invariant comment at
:808-810 and the failure-release pin above. Preserve single-lease release on every failure path and re-establish those pins deliberately — that property is the one a restructure is most likely to perturb.
Related: #2052 (umbrella), #2243.
Follow-up from the review of #2243. Assessed as design-shape rather than correctness — we tried to construct a wrong-result path and could not — so it was filed rather than folded into a converging stack.
What
In
packages/storage/src/system-record-verified-replacement-v1-internal.ts,issueCandidate(:741-907) combines exact-shape dispatch, validation, reservation accounting, fact construction, handle registration, and a promotion path in one method. The quarantine branch is the indirect part: it creates a registered active proof and then rewrites it into a quarantine proof.Measured specifics at
1db4bc36b::757issuer.issueActive(activeInput)and:758registeredHandle(handle)— both outside thetryopening at:759.:806-814mutate registry internals:AUTHENTIC_VERIFIED_REPLACEMENT_FACTS.add(facts),FACT_RESERVATIONS.set(facts, registered.reservation), thenconst superseded = registered.facts; registered.facts = facts;and.delete(superseded)on both.RegisteredReplacementV1.factsis declared non-readonlyat:219solely to permit:812; every sibling field isreadonly.Target shape
A small dispatcher calling operation-specific pure validators/fact builders — active validates active input, quarantine reuses a pure active-fact builder without registering it first, tombstone builds terminal facts — then one shared
registerReplacement(bindings, facts, reservation)that registers the final facts exactly once. That removes the handle reopening, the mutableregistered.facts, the WeakSet delete/re-add, and two of the threeas unknown ascasts (:749,:756; the third at:862is unrelated to dispatch).Why it is worth doing, beyond tidiness
:757-758sit outside thetryand are safe only becauseregisteredHandlecannot throw — the handle was registered two frames earlier with the same registry identity. Nothing at the site states that. The reservation gate is non-queued single-lease and production shares one gate across runtimes, so a leak here would brick system-record materialization process-wide rather than fail locally. Today the catch at:816-819releases correctly and it is pinned bypackages/storage/test/system-record-verified-replacement-v1.test.ts:281-293(three validation failures, each followed byissueActiveon the same registry, which can only succeed if the lease was freed). But the current shape is one edit away from a real leak, and the proposed restructure removes that hazard by construction.Constraints for whoever takes it
:808-810and the failure-release pin above. Preserve single-lease release on every failure path and re-establish those pins deliberately — that property is the one a restructure is most likely to perturb.Related: #2052 (umbrella), #2243.