fix: checkpoint malformed maintainer decisions during apply#416
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 9:12 AM ET / 13:12 UTC. Summary Reproducibility: yes. Current main sorts missing Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the narrow catch-path checkpoint with regression coverage and live proof if maintainers accept the intentionally direct report write for malformed frontmatter. Do we have a high-confidence way to reproduce the issue? Yes. Current main sorts missing Is this the best way to solve the issue? Yes. The direct timestamp write is the narrowest maintainable fix because the normal report writer would reparse the malformed AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 44d3b656300f. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (5 earlier review cycles)
|
|
@clawsweeper re-review |
|
@clawsweeper re-review |
Keep automatic apply windows responsive by running fast candidates first and admitting at most one expensive PR close-coverage proof per window. Fast and proof cursors advance independently from the exact executor trace, so interrupted or partial windows preserve unexamined work. Coverage proof, freshness checks, protected-label gates, and explicit targeted applies remain unchanged. Includes the maintainer changelog credit for #416–#419.
Summary
maintainer_decisionapply records withapply_checked_atbefore recording the kept-open result.--processed-limit 1advance from the first record to the second on the next apply run.Problem
The decision-packet rewrite in #358 added a
maintainer_decisionparse step in the apply loop. When frontmatter contained malformed maintainer decision JSON, the catch path returned a kept-open result and advancedprocessedCount, but did not persistapply_checked_at. That let the same malformed report keep sorting as unchecked and repeatedly consume bounded apply queue slots.Implementation
maintainer_decisioncatch path, writeapply_checked_atdirectly to the report markdown when not in dry-run mode.writeReportMarkdownin this specific path because decision-packet sync reparses the malformed frontmatter and would fail again.kept_open/invalid maintainer_decision: ...and do not call GitHub.Validation
pnpm run build:allpassed.node --test test\clawsweeper.test.tspassed: 69 tests.pnpm run lint:srcpassed.pnpm run lint:scriptspassed.pnpm exec oxfmt --check src\clawsweeper.ts test\clawsweeper.test.tspassed.git diff --checkpassed.$env:Path = 'C:\msys64\usr\bin;' + $env:Path; node --test --test-skip-pattern "dispatch receipt gate|--local-range" test\*.test.tspassed: 663 tests, 662 pass, 1 skipped.node dist/clawsweeper.js apply-decisionsagainst two malformed reports and a fatal mockgh.Proof
The updated malformed-decision test creates
321.mdand322.md, both with invalidmaintainer_decisionJSON, and runs apply twice with--processed-limit 1while the mockghexits if called. The first run stamps321.mdwithapply_checked_at; the second run processes322.md, proving the malformed first record no longer monopolizes the bounded apply slot and no GitHub mutation is attempted.Live apply-run proof from a local built checkout, with temp paths redacted:
I also attempted unfiltered
pnpm run test:unit. On this Windows host it fails in environment-sensitive tests unrelated to this change:dispatch-receipt-ownerneedsjqin the shell path, and twolocal-range-reviewtests do not produce their fake Codex marker files. The skip-pattern run above covers the rest of the top-level suite.Risks / Rollout
Links
Codex Review Closeout
C:\msys64\usr\bin\bash.exe C:/Users/marti/.codex/skills/codex-review/scripts/codex-review --mode branch --base origin/main --codex-bin C:/Users/marti/AppData/Local/Temp/codex-fast-wrapper.sh --output C:\Users\marti\AppData\Local\Temp\codex-review-pr416.txtcodex -c 'service_tier="fast"' review --base origin/mainfromcodex/fix-maintainer-decision-apply-checkpointagainst actual PR baseorigin/main(44d3b656300f647a20e1c82a48e84f8ad24225f9). The wrapper only corrected the local CLI config's rejectedservice_tier=default; model remainedgpt-5.5withxhighreasoning.codex-review clean: no accepted/actionable findings reported. Codex review summary: malformed decision records now stampapply_checked_at, processed-limit runs can advance to later items, and the focused test covers that rotation.pnpm run build:allpassed;node --test test\clawsweeper.test.tspassed (69 tests); directnode dist/clawsweeper.js apply-decisions ... --processed-limit 1 --min-age-days 0 --close-delay-ms 0proof passed twice. The proof showed run 1 processed/stamped only item 321, run 2 processed/stamped item 322, and the fatalghmock was not called.