Repository navigation
feat(review): safe diagnostics, one bounded relaunch, and lost-output recovery - #275
Alan-TheGentleman wants to merge 3 commits into
Conversation
…e-slot record types decodeSafeAdmissionDiagnostic returns a value only for its exact known shape and undefined for anything else -- unknown code, an extra key, an absolute/private location, .., ~, control characters, an over-length reason, or raw prose. A denylist or sanitize-then-forward default was explicitly rejected in design: a wrong default here is exactly how private paths and user content leak into an issue report. Also adds CAPTURE_SLOT_STATE/CaptureSlotRecord, sha256Hex, clearCaptureSlotRecordsForLineage, and isCaptureSlotCommitted -- the pure primitives extensions/gentle-ai.ts wires into the FINALIZE capture phase for one-shot relaunch, unreplayability, lost-output recovery, and cleanup.
…up into FINALIZE capture The FINALIZE capture phase now distinguishes three capture-result failure classes instead of bubbling every one to the generic native mutation failure path: - Rejected with a safe admission diagnostic: one relaunch grant per slotKey. The controller never launches the reviewer itself -- it returns a blocked capture-relaunch-required envelope for the parent to act on. A second rejection on the same slot is terminal exhausted; a mismatched reoffer or an unsafe diagnostic is terminal unavailable. Resubmitting the exact rejected bytes is refused before any native call, by digest membership, never by keeping the bytes. - Lost output (ambiguous mutation outcome): one fresh target-scoped STATUS query. Proof that the slot is no longer offered under unchanged authority marks it committed (no recapture, no lens rerun); otherwise only the existing reconcileNativeMutationFailure reconciliation is surfaced, reusing the already-fetched status so no second query is made for the same failure. - Everything else: unchanged from Wave 1, bubbles to the outer native mutation failure handler. The capture-slot record map is parameter-injected beside correctionEvidenceByLineage, and is cleared on all three triggers: all slots for a lineage reaching a terminal per-slot outcome, FINALIZE itself going terminal, and session shutdown.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing as obsoleted by the atomic review lifecycle: this wave1 chain instruments the pre-atomic relay protocol (slot capture, bounded relaunch, and lost-output recovery). The atomic relay (#386) reworked capture and closure ownership, and upstream gentle-ai main has since removed FINALIZE entirely (close on the last causal event), so the recovery and diagnostics surfaces this chain hardens no longer exist in the current model. Anything still relevant should restart as fresh slices against the atomic relay. |
1 participant
Second slice of Wave 1, stacked on #274.
Diagnostics are an allowlist, because a wrong default leaks user data
decodeSafeAdmissionDiagnosticreturns a value only for its exact known shape andundefinedfor everything else. Denylist and sanitize-then-forward were both rejected in design: a denylist's wrong default is precisely how private paths and user content end up pasted into an issue report.Rejection cases proven: unknown code, extra key, missing required key, malformed finding id, absolute or home-directory location,
..and backslash escapes, control characters and newlines, over-length reason or location, untrimmed raw prose, and non-object input.One relaunch, and the controller does not launch it
One grant per slot. An identical six-field STATUS reoffer plus a safe diagnostic returns
blocked: capture-relaunch-required— the controller hands back a blocked envelope and the parent decides. An already-granted record is terminalexhausted. Any field mismatched, or an undecodable diagnostic, is terminalunavailable.Unreplayability stores
sha256(rejectedDocument), never the bytes. Retaining candidate-derived content across turns to compare later would be its own problem. Membership is checked before every relaunch capture attempt, not only inside the rejection handler, because re-entry resubmits through the normal per-slot loop.Lost output: ambiguity fails closed
Proof that a capture committed is that the slot is no longer offered under an unchanged lineage id and revision. When that holds, mark committed, do not recapture, do not rerun the lens.
When the proof is absent, take only the declared reconciliation action. When STATUS is ambiguous, it is not proven. Guessing there means either a duplicate capture or a silently lost result, and neither is acceptable.
reconcileNativeMutationFailuregained an optional prefetched-status parameter so the "one fresh STATUS query per failure" rule holds instead of double-querying.Stale grants are structurally impossible
Cleanup fires on all-slots-settled, on FINALIZE terminal, and on session shutdown. But the guarantee that matters is not the cleanup:
slotKeyembeds the authority revision and subject hash, so a grant from a superseded authority cannot key into a current lookup at all. The test asserts that as a property, not as a scenario.Receipt-driven development stays disabled
Verified independently across the whole Wave 1 diff: no start, recover, reclaim, reset or reconcile-authority call was introduced. The only native calls are capture and a read-only status query.
One thing worth knowing about the pinned provider
Today's pinned version has no real admission-diagnostic wire field, so every capture failure reports an unknown outcome regardless of cause. The rejected branch is therefore distinguished by first checking for a decodable diagnostic-shaped property, before the generic lost-output path. That is a defensive decode against the frozen shape and should be revisited at the next pin.
One placement deviation
The integration tests went into the routing test file rather than the recovery one the task named, because the recovery file has none of the git-repo, candidate-view or manifest fixtures these need, while the routing file already hosts every W1 capture fixture. Recorded in
tasks.md.Tests
35 focused, all green. Full suite 1046 pass, 3 fail — the receipt-driven-development-disabled failures, byte-identical to a stashed baseline.
check:transaction-runnergreen.Rollback
The two feature commits revert together. W1's capture-only behavior remains valid standalone.