Skip to content

fix: preserve gate status during cleanup - #273

Merged
randomparity merged 11 commits into
mainfrom
feat/fix-cleanup-status-77
Aug 28, 2026
Merged

fix: preserve gate status during cleanup#273
randomparity merged 11 commits into
mainfrom
feat/fix-cleanup-status-77

Conversation

@randomparity

@randomparity randomparity commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • preserve an earned finding or CI status when scratch cleanup also fails
  • return exit 2 when cleanup is the only failure and report the retained path
  • cover removal and diagnostic-write failures in both gate suites
  • document the cleanup precedence contract and bump the plugin patch version

Verification

  • just test check-ripgrep-config verify-push
  • git diff --check main...HEAD
  • just verify
  • adversarial review: approved after two fixed findings
  • security review: approved with no findings

Closes #77

Review exit payloads

  • Suppressed by governing ADR: the plugin manifest patch bump was required by ADR 0022 and was not re-litigated as cleanup-scope expansion.

@randomparity

Copy link
Copy Markdown
Owner Author

verdict: approve
exit: none
findings: 0
iterations: 2
security: approve
delivered-head-sha: f9bbcf0

Forge whole-branch review

### Strengths

- The implementation matches the plan and ADR: both EXIT traps capture the earned status before
  cleanup, map only a clean cleanup fault to exit 2, preserve every pre-existing nonzero status,
  and emit the gate-specific retained-path diagnostic.
- The production changes stay gate-local and small. `check-ripgrep-config.sh` keeps its unsafe-path
  refusal separate, while `verify-push.sh` folds ordinary scratch removal into its existing
  worktree-cleanup accounting without changing the successful path.
- Regression coverage exercises both clean and already-failing runs for failing `rm`, and the
  existing failing-worktree-removal case now pins the same exit-2 contract. The shims are scoped to
  child verifier invocations, so fixture teardown retains its real removal path.
- The patch bump is aligned with the behavior-only, installable plugin change.
- Bounded integration reads: the current full cleanup functions and relevant fixture setup,
  invocation, and teardown context were read beyond the package to verify EXIT-trap flow,
  PATH-scoped fault injection, and ownership of intentional residue. No diff was rederived and no
  extra check was run because the caller supplied exact-HEAD focused/full-suite evidence and no
  unresolved concern remained.

### Issues

#### critical

None.

#### high

None.

#### medium

None.

#### low

None.

#### Low triage

None; low ledger is none.

### Recommendations

None.

### Assessment

**Verdict:** approve

**Reasoning:** The branch implements the specified cleanup-status contract with bounded,
Bash-3.2-compatible changes; covers the two removal failure paths and both clean/nonzero incoming
statuses; preserves existing diagnostics and safety behavior; and introduces no defensible
correctness, compatibility, architecture, test, or production-readiness finding.

Review exit payloads

  • Suppressed by governing ADR: the plugin manifest patch bump was required by ADR 0022 and was not re-litigated as cleanup-scope expansion.

@randomparity
randomparity merged commit c246979 into main Aug 28, 2026
5 checks passed
@randomparity
randomparity deleted the feat/fix-cleanup-status-77 branch August 28, 2026 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Two gates turn a failed scratch cleanup into a phantom finding

1 participant