fix(review): reject completed tasks with empty reviewer results#1550
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds durable reviewer-slot acquisition, acquisition-bound capture and disposal, distinct empty-result and nested-envelope classifications, class-aware incident artifacts, and recovery tests covering validation, idempotence, retries, and escalation. ChangesBound acquisition and classified incident recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Plugin as OpenCode plugin
participant Acquire as review acquire-result
participant Capture as review capture-result
participant Preserve as review preserve-result
participant Dispose as review dispose-result
participant Store as CompactStore
Plugin->>Acquire: reserve reviewer slot
Acquire->>Store: persist acquisition
Plugin->>Capture: submit result with acquisition ID
Capture->>Store: validate exact binding
Plugin->>Preserve: preserve classified failure
Preserve->>Store: write incident artifact
Dispose->>Store: resolve artifact and validate acquisition
Store-->>Dispose: escalation disposition
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
dnlrsls
left a comment
dnlrsls
left a comment
There was a problem hiding this comment.
Request changes: this currently proves idempotent result publication, not exactly-once recovery acquisition.
tool.execute.before only runs the non-mutating capture preflight. The slot is not acquired or durably marked until CaptureReviewerResult, which runs after the reviewer has already executed. Two concurrent recoveries can therefore both pass preflight and launch the same lens, and a crash after launch but before capture leaves the slot immediately re-runnable. Even after a successful capture, another preflight still passes because it does not check or reserve the captured slot.
The duplicate/interrupted recovery tests start at RunReviewCaptureResult, so they do not exercise duplicate reviewer launches. This conflicts with #1455's explicit requirement that concurrent or repeated recovery attempts cannot acquire the slot twice, and with its warning that retry is unsafe once execution may have occurred.
Please add an atomic, durable acquisition/unknown-outcome transition before task launch, bind capture or disposition to that acquisition, reject concurrent/repeated acquisition, and test the launch and crash boundaries. The incident classification and raw-preservation work otherwise looks solid.
|
Thanks for the review — you were right, and this is fixed. What changedAdded durable pre-launch acquisition, per your request:
Net effect: two concurrent recoveries can no longer both launch the same lens, and a crash after launch but before capture leaves an auditable SizeThis landed as 3 separate commits instead of amending the original 6 — the full remediation is 747 changed lines (685 additions, 62 deletions), over the 400-line single-PR budget, so it's delivered as a commit-per-unit chain on the same branch/PR rather than forced into one oversized diff:
Test plan
Happy to split the preserve-result format-only validation differently if you'd rather it fail without live-store access too — wanted to flag that trade-off explicitly rather than bury it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/reviewtransaction/compact_result_disposition.go (1)
114-134: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid globbing the full incidents path.
CompactIncidentsDiris derived from the repo’s on-disk Git common dir, so a checkout path containing[or]can makefilepath.Glob(filepath.Join(incidents, ...))miss every artifact. Match basenames fromos.ReadDir, or escape the parent path before globbing.🤖 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 `@internal/reviewtransaction/compact_result_disposition.go` around lines 114 - 134, Update the artifact lookup around filepath.Glob in the compact-result disposition function to avoid interpreting the incidents directory path as a glob pattern. Enumerate entries with os.ReadDir and match candidate basenames against the existing order/lens/digest naming pattern, preserving the zero-, one-, and multiple-match behavior.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@internal/reviewtransaction/compact_result_disposition.go`:
- Around line 114-134: Update the artifact lookup around filepath.Glob in the
compact-result disposition function to avoid interpreting the incidents
directory path as a glob pattern. Enumerate entries with os.ReadDir and match
candidate basenames against the existing order/lens/digest naming pattern,
preserving the zero-, one-, and multiple-match behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d049e750-3b86-4a35-a4d7-aced9d5ac9ed
📒 Files selected for processing (16)
internal/assets/assets_test.gointernal/assets/opencode/plugins/review-result-artifacts.tsinternal/cli/review_artifact.gointernal/cli/review_artifact_test.gointernal/cli/review_capabilities.gointernal/cli/review_dispose_result.gointernal/cli/review_dispose_result_test.gointernal/cli/review_facade.gointernal/cli/review_incident.gointernal/cli/review_incident_test.gointernal/reviewtransaction/compact_reclaim.gointernal/reviewtransaction/compact_result_acquisition.gointernal/reviewtransaction/compact_result_acquisition_test.gointernal/reviewtransaction/compact_result_disposition.gointernal/reviewtransaction/compact_result_disposition_test.gointernal/reviewtransaction/compact_store.go
|
Flagging a process overlap rather than acting on it unilaterally — this needs a maintainer call, not something I should decide from this side. #1455 was closed by #1549 merging ( @dnlrsls left
It was merged anyway, closing #1455. This PR (#1550), including the remediation just pushed, implements the full contract that objection describes: durable pre-launch acquisition, exactly-once launch enforcement, and the crash/duplicate-recovery boundary tests. None of that landed in #1549. Rather than guess at the right outcome, flagging the options for you to pick:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/cli/review_artifact.go (2)
56-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRead errors are masked behind a generic "required" message.
If
readFacadeBytesfails for a real reason (permission denied, path not found), the error is discarded and replaced with "final verification evidence is required," hiding the actual cause from callers/automation.🐛 Proposed fix to preserve the underlying error
payload, err := readFacadeBytes(*input) - if err != nil || len(payload) == 0 || len(payload) > reviewResultArtifactLimit { - return reviewPreflightError(errors.New("final verification evidence is required")) - } + if err != nil { + return reviewPreflightError(fmt.Errorf("read final verification evidence: %w", err)) + } + if len(payload) == 0 || len(payload) > reviewResultArtifactLimit { + return reviewPreflightError(errors.New("final verification evidence is required")) + }🤖 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 `@internal/cli/review_artifact.go` around lines 56 - 59, Update the validation around readFacadeBytes in the review artifact preflight flow to preserve and return the underlying read error when err is non-nil, while retaining the existing generic required-evidence error for empty or oversized payloads. Keep the current reviewPreflightError wrapping behavior and size checks unchanged.
371-395: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winLeaked
.capture-digest-*temp file on error paths.Unlike the sibling temp-file patterns in this same file (lines 72-77, 329-330),
persistReviewerArtifactDigestnever registers a cleanup for its temp file. IfChmod,WriteString,Sync, orClosefails, the.capture-digest-*file is left on disk permanently (only the fd is closed via the deferredtemp.Close()).🐛 Proposed fix matching the sibling cleanup pattern
temp, err := os.CreateTemp(dir, ".capture-digest-*") if err != nil { return err } + owned, _ := temp.Stat() + defer removeOwnedArtifact(temp.Name(), owned) defer temp.Close() if err := temp.Chmod(0o600); err != nil { return err }🤖 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 `@internal/cli/review_artifact.go` around lines 371 - 395, Update persistReviewerArtifactDigest to register cleanup for the temporary file immediately after os.CreateTemp succeeds, using the existing sibling temp-file cleanup pattern. Ensure the cleanup removes temp.Name() on Chmod, WriteString, Sync, Close, and publish error paths while preserving the successful publish flow.internal/reviewtransaction/compact_reclaim.go (1)
203-235: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winClose the remaining quarantine-root swap window.
ensureCanonicalReviewQuarantineRootstill leaves a path-based TOCTOU gap between the last check andos.MkdirTemp(quarantineRoot, ...); a symlink swap there can redirect where the quarantine directory is created. Useos.Root/os.OpenInRootto make the quarantine operations fd-scoped.🤖 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 `@internal/reviewtransaction/compact_reclaim.go` around lines 203 - 235, Replace the path-based quarantine creation and subsequent operations guarded by ensureCanonicalReviewQuarantineRoot with fd-scoped operations using os.Root and os.OpenInRoot, rooted at the validated review-transactions authority directory. Ensure os.MkdirTemp and quarantine inspection/rename operations resolve through the opened root so a symlink swap cannot redirect them, while preserving the existing canonical-root validation behavior.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@internal/cli/review_artifact.go`:
- Line 233: Update the upfront validation in the review artifact command to
require a non-empty acquisition flag whenever preflight is disabled, matching
the contract declared by the acquisition flag help text. Add this check
alongside the existing input validation before store.ReadResultAcquisition,
while preserving the current preflight exception and specific required-flag
error behavior.
---
Outside diff comments:
In `@internal/cli/review_artifact.go`:
- Around line 56-59: Update the validation around readFacadeBytes in the review
artifact preflight flow to preserve and return the underlying read error when
err is non-nil, while retaining the existing generic required-evidence error for
empty or oversized payloads. Keep the current reviewPreflightError wrapping
behavior and size checks unchanged.
- Around line 371-395: Update persistReviewerArtifactDigest to register cleanup
for the temporary file immediately after os.CreateTemp succeeds, using the
existing sibling temp-file cleanup pattern. Ensure the cleanup removes
temp.Name() on Chmod, WriteString, Sync, Close, and publish error paths while
preserving the successful publish flow.
In `@internal/reviewtransaction/compact_reclaim.go`:
- Around line 203-235: Replace the path-based quarantine creation and subsequent
operations guarded by ensureCanonicalReviewQuarantineRoot with fd-scoped
operations using os.Root and os.OpenInRoot, rooted at the validated
review-transactions authority directory. Ensure os.MkdirTemp and quarantine
inspection/rename operations resolve through the opened root so a symlink swap
cannot redirect them, while preserving the existing canonical-root validation
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f4347045-e17d-4043-bbd3-ed3591f423ac
📒 Files selected for processing (6)
internal/assets/assets_test.gointernal/cli/review_artifact.gointernal/cli/review_artifact_test.gointernal/cli/review_capabilities.gointernal/cli/review_facade.gointernal/reviewtransaction/compact_reclaim.go
Match the flag's own documented "required unless --preflight" contract with an explicit check, instead of relying on ReadResultAcquisition's internal empty-ID error deep in the function. Checked only after binding/repository validation so a wrong --cwd or stale lineage/target/lens/order still surfaces its own actionable error first (CodeRabbit finding on PR Gentleman-Programming#1550).
|
Verified each finding against current code before touching anything:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/cli/review_artifact.go (1)
175-222: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAdd a recovery path for failed acquisition publication.
review acquire-resultreserves the slot beforeencodeReviewJSONwrites the ID. If stdout closes or the write fails, the durable acquisition stays consumed but the caller never gets its ID, and a retry is rejected as a duplicate. Surface the existing acquisition through a read/replay path or another durable handoff, and add a broken-stdout test.🤖 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 `@internal/cli/review_artifact.go` around lines 175 - 222, Update RunReviewAcquireResult so a failure from encodeReviewJSON does not leave the acquired slot inaccessible: retain the acquisition ID and expose an existing read/replay or durable handoff path that callers can use to recover it, while preserving duplicate protection. Add a test covering broken stdout and verifying the acquisition remains recoverable through that path.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@internal/cli/review_artifact.go`:
- Around line 175-222: Update RunReviewAcquireResult so a failure from
encodeReviewJSON does not leave the acquired slot inaccessible: retain the
acquisition ID and expose an existing read/replay or durable handoff path that
callers can use to recover it, while preserving duplicate protection. Add a test
covering broken stdout and verifying the acquisition remains recoverable through
that path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1cc83d19-33c9-4575-a7f9-0cf6d27ae63e
📒 Files selected for processing (2)
internal/cli/review_artifact.gointernal/cli/review_artifact_test.go
Add a dedicated ResultIncidentClass type (empty_result, nested_envelope) distinct from ResultDispositionClass, since disposition classes judge candidate-inapplicability of a decodable payload while incident classes describe why the plugin could never extract a payload at all.
Add the optional --class flag to review preserve-result and reject any value outside the closed ResultIncidentClass enum (or empty) before any filepath.Join or incident-directory write, so a bogus or path-traversal class (e.g. --class ../evil) is rejected without touching disk.
Thread --class into preserveIncidentArtifact and reviewIncidentArtifact.Class, and extend the content-addressed incident filename from order-lens-digest12 to order-lens-class-digest12 when a class is present. Omitting --class keeps the old filename shape and empty Class, so existing callers stay backward-compatible.
Replace the single combined throw in reviewerResult() with two classified errors (reviewClass: empty_result | nested_envelope), add an extractionClass(cause) reader, and thread the class through preserveResult into --class so preservedCaptureFailure forwards it on extraction failure.
Assert the old conflated message ("reviewer task result is empty or
contains a nested envelope") is absent from the plugin source, so a
future edit cannot silently regress the two classified throws back into
one indistinguishable free-text error.
…rios Add regression coverage for AC scenarios 6 and 7: a duplicate recovery capture for the identical slot is idempotent with no revision bump, and a recovery capture interrupted by a rejected payload leaves the slot cleanly retryable through the existing store-lock + CAS in CaptureReviewerResult. Both pass against the existing atomicity with no new primitive.
preserveIncidentArtifact writes incident filenames with a class suffix (order-lens-class-digest12) whenever --class is non-empty, but CompactPreservedResultPath used by DisposeUnreplayablePreservedResult only reconstructed the old class-less shape, causing review dispose-result to fail to locate any incident preserved with empty_result/nested_envelope. The lookup now checks the class-less path first, then falls back to a glob over the incidents directory to resolve the class-suffixed shape without needing the class as an input, preserving backward compatibility with existing class-less incidents. Found by native bounded review (lineage review-083dd7c132820384), confirmed by review-resilience and review-reliability lenses.
review acquire-result's launch slot needs an exactly-once authority: the existing capture-result --preflight only verifies a binding read-only after the reviewer already ran, so a crash or duplicate recovery attempt could relaunch the identical bound task with no durable trace. CompactStore.AcquireReviewerResult atomically publishes an immutable review-acquisitions/NN-lens.json unknown_outcome record under the store lock after revalidating the exact lineage, target, lens, and order. Existence of that record consumes the slot: a stale binding is refused before any write, an already-captured slot is refused, and a duplicate or post-crash retry of the identical binding is refused while leaving the original record readable. ReadResultAcquisition gives terminal callers (capture, disposal) a lock-free exact-match lookup by ID, lineage, target, lens, and order.
…ispose Exposes review acquire-result as a new CLI subcommand and makes it the exactly-once pre-launch authority: capture-result now requires --acquisition (exempt only for the existing read-only --preflight verification) and refuses unless it exactly matches the live acquisition record for the bound lineage, target, lens, and order. dispose-result requires the same exact binding equality via the acquisition record before it will terminally escalate a lineage. preserve-result requires --acquisition in the correct generated-ID shape, but validates it as a format guard only (not against a live store), since it must keep durably saving evidence even from a repository that cannot resolve the reviewing lineage's compact store (for example a nested-worktree misconfiguration) -- only the store-bound terminal operations (capture-result, dispose-result) prove exact binding equality against the live acquisition. Existing capture-result, preserve-result, and dispose-result tests are updated to acquire the exact slot before exercising a real success path; error-path tests that fail on an earlier binding check are unaffected, since the acquisition check runs after binding validation and before payload handling.
Replaces the read-only preflightCapture check in tool.execute.before with an atomic review acquire-result call: it durably reserves the launch slot before the bound reviewer task runs, and any failure -- including an older gentle-ai binary that does not support acquire-result -- now fails closed instead of degrading gracefully into a launch with no pre-launch authority. The acquired ID is injected as GENTLE_AI_REVIEW_ACQUISITION on its own line immediately after the unchanged GENTLE_AI_REVIEW_BINDING, so the launched prompt carries it through to tool.execute.after, which recovers it and threads it into capture-result and preserve-result via --acquisition. A missing or malformed acquisition surviving launch fails closed with the raw output embedded in the error rather than silently discarding it.
Match the flag's own documented "required unless --preflight" contract with an explicit check, instead of relying on ReadResultAcquisition's internal empty-ID error deep in the function. Checked only after binding/repository validation so a wrong --cwd or stale lineage/target/lens/order still surfaces its own actionable error first (CodeRabbit finding on PR Gentleman-Programming#1550).
… rejection These two tests were fixed to pass --acquisition after the origin/main merge (capture-result now requires it before payload validation), but the fix was left in the working tree only — the merge commit captured the pre-fix version of this file from its own auto-merge staging. Tests already ran green against the working tree; this just makes the history match what was actually tested.
RunReviewAcquireResult durably records the pre-launch acquisition before encoding its ID to stdout. If that final write fails (broken pipe, crashed caller), the slot is consumed but the caller never learns its ID, and a retry over the identical binding is correctly rejected as a duplicate — leaving the slot permanently unbindable. Add CompactStore.ReadResultAcquisitionByBinding, a lock-free read that recovers an existing acquisition by lineage/target/lens/order alone (no ID required), and wire it behind a new `--replay` flag on `review acquire-result`. Replay never creates or mutates an acquisition and never authorizes a new launch: a genuine acquire attempt over an already-acquired binding still refuses identically, with or without a prior replay.
b0b0914 to
362c272
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/cli/review_incident.go (1)
117-135: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winFilename-shape contract is duplicated with the reader in
compact_result_disposition.go.
preserveIncidentArtifacthard-codes the two incident filename shapes (%02d-%s-%s.rawand%02d-%s-%s-%s.raw) thatCompactPreservedResultPathininternal/reviewtransaction/compact_result_disposition.gomust independently reconstruct viaos.Stat/filepath.Globto locate the same artifact later. Nothing enforces these two format strings stay in sync across packages; a future edit to one side (e.g. reordering fields, changing the separator) would silently break dispose-result lookups with no compile-time signal, only a runtime "no such file" error. Consider hoisting a single shared helper (e.g. inreviewtransaction, since it already ownsCompactIncidentsDir) that both builds and matches the filename, so cli only calls a documented API instead of duplicating the shape.🤖 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 `@internal/cli/review_incident.go` around lines 117 - 135, The incident artifact filename contract is duplicated between preserveIncidentArtifact and CompactPreservedResultPath. Add a shared reviewtransaction helper that builds and/or matches incident artifact filenames, then update preserveIncidentArtifact and CompactPreservedResultPath to use that API instead of hard-coded format strings or independent matching logic, preserving both class and non-class filename shapes.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@internal/cli/review_incident.go`:
- Around line 117-135: The incident artifact filename contract is duplicated
between preserveIncidentArtifact and CompactPreservedResultPath. Add a shared
reviewtransaction helper that builds and/or matches incident artifact filenames,
then update preserveIncidentArtifact and CompactPreservedResultPath to use that
API instead of hard-coded format strings or independent matching logic,
preserving both class and non-class filename shapes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: af8c853f-3aa5-44ed-85aa-3a1b7cf88e55
📒 Files selected for processing (17)
internal/assets/assets_test.gointernal/assets/opencode/plugins/review-result-artifacts.tsinternal/cli/review_artifact.gointernal/cli/review_artifact_test.gointernal/cli/review_capabilities.gointernal/cli/review_dispose_result.gointernal/cli/review_dispose_result_test.gointernal/cli/review_facade.gointernal/cli/review_incident.gointernal/cli/review_incident_test.gointernal/reviewtransaction/compact.gointernal/reviewtransaction/compact_reclaim.gointernal/reviewtransaction/compact_result_acquisition.gointernal/reviewtransaction/compact_result_acquisition_test.gointernal/reviewtransaction/compact_result_disposition.gointernal/reviewtransaction/compact_result_disposition_test.gointernal/reviewtransaction/compact_store.go
# Conflicts: # internal/assets/assets_test.go # internal/assets/opencode/plugins/review-result-artifacts.ts # internal/cli/review_facade.go # internal/cli/review_incident.go
Bind reviewer artifacts to provider-owned subjects and admit only completed, in-scope inspections. Add exact validating-result reopen and narrowly gated correction-evidence recovery while preserving strict legacy contracts through versioned schemas.
Alan-TheGentleman
added
size:exception
7f934dc
into
Gentleman-Programming:main
3 participants
🔗 Linked Issues
Closes #1454
Closes #1555
Closes #1575
Closes #1579
Closes #1259
Closes #1264
Closes #1609
Adds regression proof for #1215 and preserves the original contributor work from already-closed #1455.
🏷️ PR Type
type:bugtype:featuretype:docstype:refactortype:choretype:breaking-change📝 Summary
review reopen-resultsrecovery that retains valid admitted slots and quarantines unusable artifacts without resetting lineage, scope, risk, lenses, or budget.📂 Changes
ArtifactSubject, admitted-result envelope, raw/canonical digests, exact lens/order/revision/target/correction bindings.review reopen-resultswith maintainer authorization, store-lock re-admission, selective retention/quarantine, exact replay and mixed-slot refinalization.🧪 Test Plan
go test ./... -count=1go vet ./...go run ./internal/gofmtcheck✅ Contributor Checklist
status:approvedtype:*labelsize:exceptionapplied because admission, recovery and versioned contracts share one authority invariantReview Evidence
bfdffbca31c3d13bd8653a416ccebb329d5ee4f347daaa50483481968fd3e93519cab66a7630d8ce1f9af714+56720ef0