Repository navigation
Stale-disposition retry cap does not count a fix-reviews pass that dies before posting any marker (follow-up to the edited-comment deadlock fix) #2247
Description
Problem
The stale-disposition retry cap added for #2166 (PR #2230) bounds how many fix-reviews passes the sweep dispatches for a bot comment that was edited after its disposition. It counts passes by their markers on the PR: any <!-- dev-lead-fix-reviews pr=<n> … intent=fix-reviews status=… --> comment newer than the edit.
A pass that starts and then dies before posting any marker (runner lost, timeout before the marker step, cancelled mid-run) leaves nothing to count. Each later sweep can dispatch another model pass once the dispatch-guard window expires, so the cap does not bound model runs in that case.
CodeRabbit raised this on PR #2230 (scripts/dev-lead-retry.sh, stale_disposition_needs_dispatch). dev-lead confirmed it and widened the count from status=(applied|no-changes) to any status= in 5fcef895, which covers failed passes that post a marker. The no-marker case was accepted for that PR by the maintainer and deferred here.
Acceptance criteria
- A
fix-reviewspass dispatched for a stale disposition records that it started before the model runs, somewhere the sweep can read on its next tick (for example a start marker comment carryingpr,shaand the edit time it was dispatched for). -
stale_disposition_needs_dispatchcounts started passes, not only finished ones, soSTALE_DISPOSITION_MAX_PASSESbounds the number of model passes per edit even when every pass dies early. - The start record carries a registered automation marker, so pr-review's comment gate does not count it as a finding.
- A bats test covers: two passes that start and post no terminal marker, then a third sweep → no dispatch, and the capped hold is posted.
Out of scope
The cap's existing behaviour for passes that do post a marker, and the hold message itself (both in PR #2230).
Filed from a Claude Code session at the maintainer's request, 2026-10-11.
Activity
github-actions commentedon Oct 11, 2026
Thanks for the detailed write-up and for tracking this follow-up from #2230. The acceptance criteria are clear. To scope the fix, where should the start record live: a new marker comment on the PR (as suggested), or would you prefer to reuse an existing marker type with an added
status=started? The choice affects how pr-review's comment gate registers it and howstale_disposition_needs_dispatchcounts passes.don-petry commentedon Oct 11, 2026
Dev-Lead Implementation Plan
Issue: #2247 — Stale-disposition retry cap does not count a fix-reviews pass that dies before posting any marker
Scope
A
fix-reviewspass that sees a stale disposition posts a start marker before the model runs.stale_disposition_needs_dispatchcounts started passes, deduplicated with their terminal markers byread_at=, soSTALE_DISPOSITION_MAX_PASSESbounds model passes even when every pass dies early.Implementation Checklist
scripts/dev-lead-fix-reviews.sh:export_stale_dispositionsalso records the latest stale edit time (STALE_LATEST_EDIT); newpost_stale_pass_startposts<!-- dev-lead-fix-reviews-start pr=<n> sha=<sha> intent=fix-reviews edit=<iso> read_at=<PASS_STARTED_AT> -->(only when a disposition is stale) in thefix-reviewsarm beforebuild_and_run.scripts/dev-lead-retry.sh:stale_disposition_needs_dispatchcounts start markers and terminal markers from trusted authors, keyed byread_at(fallbackcreatedAt), so one pass that posts both counts once.<!-- dev-lead, so it already matches_MAINTAINER_GATE_AGENT_MARKERS(registered automation marker). A test pins that. Thedev-lead-fix-reviews-startname does not match the existingdev-lead-fix-reviews pr=/markers("dev-lead-fix-reviews")readers.Tests to Write
test_dev_lead_retry.bats: two passes that post only start markers, then a third sweep → no dispatch, and the capped hold comment is posted (realstale_disposition_needs_dispatch+scan_pr_for_rate_limits).test_dev_lead_retry.bats: a start marker plus a terminal marker from the same pass (sameread_at) count once; a start marker before the edit, for another PR, or from an untrusted author does not count.test_maintainer_comment_gate.bats/test_fix_reviews.bats: the start marker is an agent marker for the gate;post_stale_pass_startposts only when stale, and runs beforebuild_and_run.Test Command
bats tests/dev-lead/unit/test_dev_lead_retry.bats tests/dev-lead/unit/test_fix_reviews.bats tests/dev-lead/unit/test_maintainer_comment_gate.bats+bash .dev-lead/scripts/dev-lead-lint.shdon-petry commentedon Oct 11, 2026
Dev-Lead: Progress (in progress — not yet durable)
Plan Execution (intended)
fix-reviewspass that sees a stale disposition posts<!-- dev-lead-fix-reviews-start pr=<n> sha=<sha> intent=fix-reviews edit=<latest edit> read_at=<pass start> -->beforebuild_and_run.stale_disposition_needs_dispatchcounts start markers as well as terminal markers, deduplicated byread_at, so a pass that posts both counts once.Files touched (this workspace, not yet pushed)
scripts/dev-lead-fix-reviews.sh:export_stale_dispositionsalso setsSTALE_LATEST_EDIT; newpost_stale_pass_start(best effort, skipped in dry-run and when nothing is stale), called in thefix-reviewsarm before the model runs.scripts/dev-lead-retry.sh:stale_disposition_needs_dispatchmatches start markers and counts distinctread_atvalues; doc comment updated.tests/dev-lead/unit/test_dev_lead_retry.bats: start-only passes reach the cap; start plus terminal counts once; pre-edit, other-PR and untrusted start markers are ignored; end-to-end: two passes that die after their start marker, then a third sweep gets no dispatch and the capped hold is posted.tests/dev-lead/unit/test_fix_reviews.bats:STALE_LATEST_EDIT, the marker format, no post when nothing is stale or in dry-run, the posted marker round-trips through the sweep's counter, and it is posted beforebuild_and_run.tests/dev-lead/unit/test_maintainer_comment_gate.bats: the start marker is exempt from the comment gate (it is covered by the existing<!-- dev-leadprefix).Notes
read_atand count once. This is unlikely given the dispatch-guard window.STALE_DISPOSITION_MAX_PASSESper edit).main(environment-only) and are unrelated to this change.don-petry commentedon Oct 11, 2026
Dev-Lead: Implementation Complete — PR #2290
The implementation for issue #2247 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.
2e4b8207763f83dc61c8d72da43bb3256527686dmain: 5 file(s), +216/-11 linesReview happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.
feat: implement issue #2247 — Stale-disposition retry cap does not co…