fix(dev-lead): idempotent comment dispositions and duplicate recovery (#1992)

[don-petry]

Summary

Dev-Lead no longer stacks duplicate dev-lead:comment-disposition replies, and the resolver recovers PRs that are already stuck on duplicates.

This finishes the Dev-Lead checkpoint 1b957e3, which timed out at the 2700s action budget. It adds a fix for one defect in that checkpoint and a regression test with #1952's exact shape.

• Duplicate recovery (scripts/lib/comment-disposition-verify.sh): new pure cdv_select_disposition. Among the parseable, non-minimized dispositions authored by BOT_USER (CWE-863: nobody else's ever count), it picks the latest by createdAt, with a node-id tie-break, and reports the rest as superseded.
• Resolver (scripts/dev-lead-fix-reviews.sh): uses the selector instead of failing closed on auth_count != 1. After the original comment is minimized RESOLVED, it minimizes the superseded replies OUTDATED, so the count converges to one.
• No orphans on failure: resolve_dispositioned_comments now also runs on the failure path of fix-reviews, fix-bot-comment and review-changes. Each disposition is still verified on its own terms, so an unpushed fixed fails closed.
• Idempotent posting (prompts): the engine skips comments that already carry a non-fixed disposition from the bot account.
  • Exception for fixed: a still-unminimized fixed disposition is never verified, because the resolver only accepts a fixed sha produced by the pass that cites it. The model must re-check it and post a fresh disposition. Skipping it, as the checkpoint did, would block the PR permanently. That is the defect fixed in 0a639c3.

Problem

Closes #1992. It also covers #1934 item 1 (the fix-bot-comment path).

Dev-Lead posted a new disposition on every pass until the comment was minimized. The resolver required exactly one per comment, so any comment dispositioned but not minimized in the same pass stayed blocked for good:

• feat: implement issue #1950 — [Phase 2] Add a workflow_dispatch runner for model-ab.sh plus a liveness/effort probe, so the Opus 5.5 non-regression A/B actually executes #1952: CodeAnt's comment has three dispositions (fixed, then invalid twice).
• feat: implement issue #1951 — [Phase 2] Canary go/no-go report: compare deep-tier cost, cache-read cost and latency by model from token-usage artifacts #1953: a timed-out pass orphaned 10 dispositions.

Risk

Medium. This changes how the maintainer comment gate clears dispositioned comments.

• Authorization is unchanged: only BOT_USER replies count, and unparseable markers fail closed.
• fixed verification is unchanged: it is still bound to the pass that made the commit.

Test plan

• New cdv_select_disposition unit tests in tests/dev-lead/unit/test_comment_disposition_verify.bats. They cover a single disposition, three converging to the latest, a non-BOT_USER reply never winning, minimized and unparseable replies being excluded, zero replies failing closed, and the tie-break.
• New end-to-end resolver tests in tests/dev-lead/unit/test_fix_reviews.bats, with failing engine stubs:
  • a failed pass still resolves a single disposition;
  • three dispositions leave the latest RESOLVED and the other two OUTDATED;
  • feat: implement issue #1950 — [Phase 2] Add a workflow_dispatch runner for model-ab.sh plus a liveness/effort probe, so the Opus 5.5 non-regression A/B actually executes #1952's shape (fixed, invalid, invalid) resolves on the latest invalid;
  • a non-BOT_USER disposition can't resolve the comment.
• Ran locally:
  • bats tests/dev-lead/unit/test_comment_disposition_verify.bats, plus bats tests/dev-lead/unit/test_fix_reviews.bats (161 tests). The only failure is review-changes evidence: whitespace/cosmetic-only change, which fails identically on main.
  • The prompt and marker suites (test_agent_marker_convention, test_channel_*_guardrail, test_duplicate_decl_gate; 66 tests) and tests/dev-lead/integration/test_prompt_coverage.sh pass.
  • shellcheck --severity=warning and scripts/dev-lead-lint.sh are clean.

Rollback

Revert this PR. There are no non-revertible side effects. Replies minimized OUTDATED stay minimized but are still visible, and can be un-minimized by hand.

Monitoring

• After this reaches the Dev-Lead channel, the next pass on feat: implement issue #1950 — [Phase 2] Add a workflow_dispatch runner for model-ab.sh plus a liveness/effort probe, so the Opus 5.5 non-regression A/B actually executes #1952 and feat: implement issue #1951 — [Phase 2] Canary go/no-go report: compare deep-tier cost, cache-read cost and latency by model from token-usage artifacts #1953 should log selecting latest … marking N superseded reply(ies) OUTDATED, and the gate's undispositioned-pr-comment hold should clear.
• Watch for any new expected exactly one authorized disposition notice. That message no longer exists, so seeing it means the old code is still running.

Interaction contract

• N/A — this PR does not add or change an agentic role.

Checklist

• shellcheck scripts/*.sh passes for any changed shell scripts.
• Docs updated where behavior or standards changed (prompts and in-code docs).

🤖 Generated with Claude Code

https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

---

Generated by Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6e768009-02fd-4fe1-a4a5-1e749ff20322

📥 Commits

Reviewing files that changed from the base of the PR and between 8693ab4 and 6d35a5d.

📒 Files selected for processing (6)

• prompts/dev-lead/fix-bot-comment.md
• prompts/dev-lead/fix-reviews.md
• scripts/dev-lead-fix-reviews.sh
• scripts/lib/comment-disposition-verify.sh
• tests/dev-lead/unit/test_comment_disposition_verify.bats
• tests/dev-lead/unit/test_fix_reviews.bats

 _____________________________________________________________________
< This is 'temporary' in the same way legacy systems are 'temporary'. >
 ---------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codeant-ai]

CodeAnt Nitpicks

4 code suggestions

1. An earlier valid fixed reply may remain after an API failure, but this forces another reply; without a new commit, the resolver rejects every new fixed SHA.

Logic error · prompts/dev-lead/fix-reviews.md:130

---

2. The duplicate check reads only the first 100 comments, so older disposition replies on larger PRs are missed and duplicate replies can still be posted.

Api mismatch · prompts/dev-lead/fix-bot-comment.md:147-149

---

3. If minimizing a superseded reply fails after the original is resolved, the original is no longer a candidate, so the duplicate remains live and is never retried.

Missing cleanup · scripts/dev-lead-fix-reviews.sh:1083-1084

---

4. mktemp -d creates a repository for every test, but no cleanup removes it, leaving temporary directories after the test suite finishes.

Resource leak · tests/dev-lead/unit/test_fix_reviews.bats:4695

[gemini-code-assist]

Code Review

This pull request implements duplicate recovery and idempotency for bot comment dispositions, resolving issues #1992, #1952, and #1953. It introduces a deterministic selection helper cdv_select_disposition to collapse multiple authorized replies to the latest one, minimizes superseded replies as outdated, and ensures the resolver runs on failed passes. The feedback suggests fixing an inconsistent default value for BOT_USER in the documentation, removing a redundant newline check when reading from a here-string, and tightening test assertions in BATS to check for specific non-zero exit codes instead of generic non-zero statuses.

[cubic-dev-ai]

All reported issues were addressed across 6 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 6 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-01T22:16:41Z.

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

Issues addressed: 0
- Quality Gate passed (0 new issues, 0 security hotspots): registered clean-status pattern — auto-cleared by gate
Files changed: none
Skipped (informational): 0
```

[donpetry-bot]

CI checks on this PR are still running. The PR-review sweep re-reviews this PR automatically once the checks complete — no action is needed.

Posted by the donpetry-bot PR-review cascade.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: f50278111066dbdc0851fcc2fb06731663ba6af3
Review mode: triage-approved (single reviewer)

Summary

Fixes the Dev-Lead disposition deadlock (#1992). It adds a pure cdv_select_disposition selector. The selector collapses multiple authorized BOT_USER dispositions to the latest by createdAt (node-id tie-break). The resolver minimizes the superseded replies OUTDATED once the original is RESOLVED. The resolver now also runs on the failure path of all three intents, with fixed never certified there. Both prompts gain idempotency guidance.

Linked issue analysis

Closes #1992. All acceptance criteria are substantively addressed:

• Idempotent posting: prompt-level skip in both paths. The harness backstop is duplicate convergence after the engine returns, and the code comment records this choice (the model posts through its own shell).
• Duplicate recovery: deterministic latest-wins selection. Superseded replies are minimized OUTDATED. Unparseable or non-BOT_USER replies still fail closed.
• No orphans on a failed pass: resolve_dispositioned_comments <intent> failed runs on the fix-reviews, fix-bot-comment and review-changes failure branches. fixed is refused there because the commit may be unpushed.
• Tests: behavioural unit tests for the selector, and end-to-end resolver tests with failing engine stubs, covering the #1952 shape, the CWE-863 attacker case and fixed on a failed pass.

Findings

No blocking findings.

• Authorization is preserved: the CWE-863 author gate (BOT_USER or its [bot]-stripped login) is unchanged. Only the cardinality rule is relaxed, and only among replies the bot itself authored. The new selector also tightens input handling: isMinimized == false must be explicit, and createdAt must be strict ISO-8601.
• The resolver's comment query already fetches createdAt and isMinimized, so the selector gets real data in production.
• Fail-closed edge case: if the latest reply is a stale fixed from an earlier pass, the comment stays open. The new prompt exception (post answered naming the existing commit) lets the next pass converge.
• Non-blocking: a failed OUTDATED minimization is only warned, not retried. The author's rationale holds: once the original is RESOLVED it leaves the candidate set, so a leftover reply can't re-block the gate.
• All 17 review threads (CodeAnt, Gemini, cubic) are resolved, with fixes in 41ede87 and 42ef721.
• MCP secret scan: run_secret_scanning was not available in this run. The gitleaks CI check passed.

CI status

All required checks are green: bats, unit-tests, shellcheck, CodeQL, SonarCloud (Quality Gate passed), gitleaks, AgentShield, actionlint and others. dev-lead / dispatch|ci-relay|resume show CANCELLED (superseded concurrency runs, not test failures). Dependency-audit language jobs were skipped as not applicable.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.

[donpetry-bot]

pr-review approved on PARTIAL advisory evidence: 4/5 required advisory bots reported before the gate's head-age-timeout fallback proceeded. Recorded for the miss-rate metric (#1596).

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

Issues addressed: 0
- Quality Gate passed (0 new issues, 0 security hotspots, 0.0% coverage/duplication on new code): auto-cleared registered clean-status pattern — no action needed
Files changed: none
Skipped (informational/clean-status): 1
```
No code changes required. The PR is clean from the bot's perspective.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-01T23:04:45Z.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: e0cc2cf886d607efeea0df043a69400bea190e10
Review mode: triage-approved (single reviewer)

Summary

Fixes the Dev-Lead disposition deadlock (#1992). It adds a pure cdv_select_disposition selector, which picks the latest authorized BOT_USER disposition by createdAt (node-id tie-break). The resolver then minimizes the superseded replies OUTDATED once the original comment is RESOLVED. The resolver also runs on the failure path of fix-reviews, fix-bot-comment and review-changes, and fixed is never certified there. Both prompts gain idempotency guidance. Since the prior approved review (f502781), the only new commits are two merges from main, and neither touches this PR's six files.

Linked issue analysis

Closes #1992 (and #1934 item 1). The acceptance criteria are substantively addressed:

• Duplicate recovery: latest-wins selection is deterministic. Superseded replies are minimized OUTDATED only after the original is RESOLVED, so the count converges to one.
• Authorization unchanged: only BOT_USER (or its [bot]-stripped login) counts (CWE-863). Minimized, unparseable, mis-targeted and malformed-createdAt replies fail closed.
• No orphans on a failed pass: resolve_dispositioned_comments <intent> failed runs on all three failure branches. fixed is refused there because the commit may be unpushed.
• Idempotent posting: the prompts skip an existing non-fixed disposition. An unverified fixed is re-checked rather than skipped (the 0a639c3 fix). The fix-bot-comment check matches the node id literally and fails closed when the check can't be read.
• Tests: selector unit tests, plus end-to-end resolver tests covering #1952's exact fixed, invalid, invalid shape and a non-BOT_USER disposition.

Findings

No blocking findings.

• All 18 review threads (CodeAnt, Gemini, cubic) are resolved; cubic reports every issue addressed.
• The resolver's comment query already fetches id, isMinimized, minimizedReason and createdAt, which is everything the selector needs.
• Non-blocking: a failed OUTDATED minimize is only logged as a warning. This is self-healing: the next pass reselects, and an already-RESOLVED original stops being a candidate.
• The secret-scan MCP tool is not available in this run. The gitleaks CI check passed.

CI status

All code checks pass on e0cc2cf: shellcheck, bats, unit, unit-tests, CodeQL, SonarCloud (Quality Gate passed), gitleaks, actionlint, AgentShield, prompt-coverage, and the stub and standards validators. The only non-success jobs are three CANCELLED dev-lead orchestration jobs (dispatch, ci-relay, resume), which were superseded runs and are not code checks.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: opus 5.5 [opus 4.8, opus 4.7]). Reply if you need a human review.