[don-petry]

Problem

dev-lead claim retraction does not cover issue-comment dispositions: a fixed reply for a commit that never landed stays up

From the issue: When a dev-lead pass answers a PR issue comment it posts a reply marked disposition=fixed that cites a commit. If that commit never lands, the reply stays up saying "fixed". The claim-retraction sweep from #2013 does not look at these replies: retract_unlanded_claims (scripts/dev-lead-fix-reviews.sh) lists only pulls/{pr}/comments, the review-thread replies.

Risk

Low — changes automation shell logic under scripts/, covered by shellcheck (--severity=warning) and the bats suite.

Test plan

Tests added/updated: tests/dev-lead/unit/test_claim_landing.bats,tests/dev-lead/unit/test_claim_sweep.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

This PR's Lint (shellcheck) and bats checks show pass/fail; watch subsequent dev-lead / pr-review runs for behavioral regressions.

Closes #2156

[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.

[gemini-code-assist]

Code Review

This pull request adds support for sweeping and verifying fixed disposition replies posted as PR issue comments, ensuring that unlanded commits are retracted and rebased commits are updated. Feedback on the changes includes improving the portability of base64 decoding by adding a fallback command, and using prefix/suffix parameter expansion instead of pattern replacement in cl_rewrite_disposition_sha to robustly handle special characters in user-supplied comment bodies.

[don-petry]

QA Lead — test-risk advisory

Risk tier: LOW. This only touches the bot's own PR-comment automation. If it misfires, the worst case is a wrongly kept or wrongly retracted "fixed" reply, which is recoverable. Its pure helpers (cl_select_fixed_dispositions, cl_retract_disposition_body and cl_rewrite_disposition_sha) have unit tests, and the bats sweep suite exercises both windows against a real bare remote.

What I'd shore up (highest leverage first):

• The "fixed" sweep is looser than the review-thread sweep. The review-thread sweep runs acv_claim_in_pass against RESOLUTION_BASE_SHA, so a claim that cites a commit from before the pass gets retracted. sweep_fixed_dispositions in the pass window only checks merge-base --is-ancestor $sha $ref, so a fixed reply citing an older commit that is already on the branch survives. Add a test that pins whichever behaviour you intend.
• Untested failure paths. Add a test where the retract PATCH fails, which should give ::error and a non-zero return. Add another where the remap PATCH fails, which should fall back to retracting. Both are new branches with no coverage.
• Status folding in sweep_earlier_claims. Make the issue-comment listing fail while a review-thread claim is pending. Assert that the thread claim is still swept and the function returns non-zero. The current failed-listing test only checks retract_unlanded_claims, and only its exit status.
• Unverifiable and lookup-error paths at sweep level. Cover a fixed reply with an empty or malformed sha, which should be retracted as unverifiable. Also cover GH_COMMITS_ERROR in the earlier window, which should leave the reply as is. Right now the unverifiable case is only tested at the selector level.

Escalate? no

Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture
(bmad-tea v1.19.0). Opt out on this item with the qa-lead:hands-off label.

[don-petry]

Dev-Lead — review-changes (applied)

Changes committed and pushed. Requested items addressed:

• scripts/lib/claim-landing.sh:485 — applied
• scripts/lib/claim-landing.sh:537 — applied

[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]

No description provided.

[donpetry-bot]

No description provided.

[don-petry]

Neutral overview — the PR appears safe to merge. No actionable findings to address.

[don-petry]

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

Agent reasoning

## Summary
**Bot:** codeant-ai[bot]
**Issues addressed:** 0 (neutral overview)
**Review threads:** 2 from gemini-code-assist (already resolved and outdated)
- Thread 1: Base64 fallback suggestion — already implemented in commit `f7b0cf1:485`
- Thread 2: Slice-based SHA rewrite suggestion — already implemented in commit `f7b0cf1:534-540`
**CodeAnt comment disposition:** Posted `informational` disposition on IC_kwDOR9SdIs8AAAABa8kuSQ (neutral overview with no actionable findings)
**PR State:** All CI checks passing, no Tier 1 blockers
The PR improvements addressing the bot's suggestions from earlier passes have been merged into the current commit. The CodeAnt overview comment has been dispositioned to clear the maintainer-comment gate.

[don-petry]

This is a usage-limit notice from the bot's service — no code changes needed. The bot will resume reviewing after the quota resets.

[don-petry]

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

Agent reasoning

**Bot:** `gemini-code-assist[bot]`
**Issues analyzed:**
1. **Quota limit notice** — "You have reached your daily quota limit": non-actionable operational notice indicating the bot's daily review limit was reached
2. **Code review comment** — neutral overview describing the PR's changes and offering style suggestions (base64 portability fallback is already implemented; parameter expansion is a style preference, not a defect)
**Tier 1 blockers:** None — all CI checks pass, no `CHANGES_REQUESTED` reviews
**Action taken:**
- Dispositioned the quota limit notice as `informational` with the required `<!-- dev-lead:comment-disposition id=IC_kwDOR9SdIs8AAAABa8-t9A disposition=informational -->` marker
- No code changes needed — the PR's changes are sound and the bot's suggestions are either already addressed or stylistic preferences
**Files changed:** None
**Outcome:** Disposition posted. The harness will resolve the minimized comment and clear the maintainer-comment gate.

[don-petry]

This is a quota limit notice from the code review bot — an operational notice, not a code finding. No changes to the PR are needed.

[don-petry]

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

Agent reasoning

## Summary
**Bot:** `gemini-code-assist[bot]`
**Issues addressed:** 0 code findings
**Disposition:** 1 non-actionable notice
- **Quota limit warning** → Dispositioned as `informational` on the original comment
  - The bot's daily quota limit was reached; this is an operational notice, not a code defect
  - PR state shows all CI checks passing and no actionable findings in the review comment
  - Posted disposition reply: https://github.com/petry-projects/.github-private/pull/2231#issuecomment-6103874407
**Files changed:** None
The maintainer-comment gate is now cleared — the original quota limit comment has been dispositioned with `informational`, and the harness will resolve it on the next run.

[don-petry]

Rate-limit notice — no code changes needed. The PR is ready for review once the bot's quota resets in 24 hours.

[don-petry]

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

Agent reasoning

- The bot comment is a **rate-limit notice** — a non-actionable operational notice indicating the bot has exhausted its daily quota
- The bot's code review (COMMENTED state) is a summary providing feedback about the PR implementation, but contains no change requests or specific line-level thread findings
- All CI checks pass (55 success, 3 skipped, 3 cancelled)
- No Tier 1 blockers present (no failures, no CHANGES_REQUESTED reviews)
**Action taken:**
- Posted disposition reply to the original quota-limit warning comment with `informational` marker
- The harness will minimize the original comment as RESOLVED
**Files changed:** None (no code changes needed)
**Skipped (informational):** 0 actionable findings in code
The PR is ready for review once the bot's daily quota resets.

[don-petry]

No description provided.

[don-petry]

Rate-limit notice only: CodeRabbit reached its free OSS review quota and paused the code review. This is an operational throttle message with no code findings to address. The PR's CI checks all pass, and there are no actionable issues in the changed files.

[don-petry]

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

Agent reasoning

Issues addressed: 0 (no actionable code findings)
Files changed: (none — notice only)
Disposition: informational
- Rate-limit notice: Pure operational throttle message with no code findings. Posted disposition reply on PR comment.
```
The bot comment is a pure **rate-limit notice** — CodeRabbit hit its free OSS review quota and paused the code review. There are no Security Architecture Review findings, no actionable comments, and no other findings in any section. All CI checks pass. The comment has been dispositioned as `informational` and will be minimized by the harness once verified.

[donpetry-bot]

Automated review — APPROVED ✓

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

Summary

Extends both dev-lead claim sweeps (the end-of-pass retract_unlanded_claims and the start-of-pass sweep_earlier_claims) to PR issue-comment replies marked disposition=fixed. A new sweep_fixed_dispositions uses the same landing rules as review-thread claims: a reply whose cited commit is not on the remote head is retracted, a reply whose commit was rebased by the push guard is re-pointed at the landed commit, and an earlier reply is skipped when the checkout is shallow or GitHub knows the commit. The logic is fully contained in the bot's own automation. A wrong decision costs one kept or retracted reply, which can be recovered. Rated MEDIUM because the logic change is non-trivial.

Linked issue analysis

Closes #2156. All four acceptance criteria are met:

1. Retraction shape: cl_retract_disposition_body removes every comment-disposition marker, adds a retraction notice, quotes the original text and appends <!-- dev-lead:retracted reason=… -->.
2. Both sweeps: the sweep is wired into retract_unlanded_claims (pass) and sweep_earlier_claims (earlier), against the same reference head as the review-thread claims.
3. Other dispositions untouched: the selector matches only disposition=fixed. Tests cover invalid, answered, informational and out-of-scope replies in both windows.
4. Tests: bats tests with a gh stub cover an unlanded commit (retracted), a landed commit (kept), a commit GitHub knows (kept), a rebase (re-pointed), a failed listing (reported) and other dispositions (left alone). There are also unit tests for the pure helpers.

Findings

No blocking findings.

• Correctness: failed is now declared before the new sweep runs, and the sweep's result is folded into it. Both callers return it, including the [ -z "$rows" ] && return "$failed" path in sweep_earlier_claims, so a failed issue-comment listing is not read as a clean pass. retract_unlanded_claims has no early return on empty rows, so the folded status survives there too.
• Downstream behaviour: removing the marker is the right thing for the other consumers. bot-comment-retry.sh treats a comment with no comment-disposition id= reply as un-dispositioned, so it is answered again, and maintainer-comment-gate.sh stops counting it as answered.
• Both earlier Gemini threads are resolved and fixed in code: base64 --decode now falls back to -d, and the SHA rewrite uses a quoted literal slice instead of a pattern substitution on the whole body.
• Source order: moving comment-disposition-verify.sh ahead of claim-landing.sh is safe. claim-landing.sh also guards with declare -F cdv_parse_disposition.
• Security: the retraction reason is restricted to [a-z-], so it cannot break out of the HTML comment. Bodies go to gh api -f body= as arguments, not through the shell. No secrets, workflow or permission changes.
• Non-blocking: for review-thread claims, cl_restore_body restores a wrongly retracted reply. The earlier sweep has no such step for disposition replies. The issue does not ask for it, and the comment is answered again anyway, so this is acceptable as a follow-up.
• shellcheck --severity=warning is clean locally on both changed scripts. bats is not installed on this runner; the CI bats and unit-tests jobs pass.

CI status

All required checks pass: Lint, ShellCheck, shellcheck, bats, unit-tests, CodeQL, SonarCloud (quality gate passed), gitleaks, AgentShield, actionlint and the stub/standards validators. The dev-lead / dispatch|ci-relay|resume runs were cancelled by the orchestration's concurrency control; these are not failures. The dependency-audit ecosystems that do not apply were skipped, and cubic reported neutral.

---

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).