feat: implement issue #1617 — Follow-up to #1609 (slice 2): gate dev-lead thread resolution on the pass having advanced the PR head

[don-petry]

User description

Closes #1617

Implemented by dev-lead agent. Please review.

---

CodeAnt-AI Description

Prevent review threads from being resolved without a new commit

What Changed

• Review threads are automatically resolved only when the pass advances the PR head.
• Passes that make no commit now leave all review threads open and report that resolution was skipped.
• Dry-run behavior remains unchanged, and tests cover no-commit passes across all review workflows.

Impact

✅ Fewer incorrectly closed review threads
✅ Preserved visibility of unresolved findings after no-change passes
✅ Safer review automation

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[codeant-ai]

🤖 CodeAnt AI — Review Status

Status	Commit	Started (UTC)	Finished (UTC)
✅ Reviewed your PR	0a818e9	Sep 01, 2026 · 01:53	01:55

[qodo-code-review]

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

[codeant-ai]

---

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

[coderabbitai]

Warning

Review limit reached

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: e052ed97-3329-4b8f-bf41-0a5a3388fd17

📥 Commits

Reviewing files that changed from the base of the PR and between 642abf4 and 09cef7c.

📒 Files selected for processing (2)

• scripts/dev-lead-fix-reviews.sh
• tests/dev-lead/unit/test_fix_reviews.bats

---

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

[don-petry]

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

No changes were needed for this PR.

[gemini-code-assist]

Code Review

This pull request introduces a resolution gate (resolution_gate_open) in scripts/dev-lead-fix-reviews.sh to ensure review threads are only auto-resolved when a pass actually advances the PR head. It also adds comprehensive unit tests in tests/dev-lead/unit/test_fix_reviews.bats to validate this logic. Feedback was provided to replace the standalone conditional check [ "$cp_rc" -eq 0 ] with a direct return "$cp_rc" to prevent potential premature script termination under set -e.

[codeant-ai]

CodeAnt Nitpicks

2 code suggestions

1. The head check is not atomic with thread resolution. A concurrent push can occur afterward, so these calls may resolve threads using a head state this pass did not validate.

Race condition · scripts/dev-lead-fix-reviews.sh:1517-1521

---

2. The positive tests treat every push as successful and keep the mocked PR head at the old SHA, so they cannot detect resolution after a failed or local-only push.

Code quality · tests/dev-lead/unit/test_fix_reviews.bats:930-931

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 0a818e9afdd8eb606aa3535111f8c75b0e5eeede
Cascade: triage → deep (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5)

Summary

PR #1625 wires the pure #1609 head-movement predicate (ri_may_resolve) into dev-lead's resolve_* thread nets via resolution_gate_open, so a fix pass auto-resolves review threads ONLY when it advanced the PR head — closing the #1024 no-commit-resolution vector across all three intent branches, with three new no-commit bats tests plus updated existing tests. The Gemini high-priority finding on [ "$cp_rc" -eq 0 ] under set -e is a false positive: all three call sites invoke the function as an if condition, which suspends errexit for the whole function call, so the bare return correctly propagates the test result without terminating the script. CI is fully green (bats/unit/shellcheck/CodeQL/SonarCloud/gitleaks/AgentShield), downstream impact is (none), and the change touches only automation logic + tests with no auth/secrets/crypto/migration surface.

Findings

• INFO: Advisory bot (Gemini) flagged [ "$cp_rc" -eq 0 ] followed by bare return in resolution_gate_open as a set -e premature-termination risk. Verified false positive: all three call sites are if resolution_gate_open "$cp_rc"; then, and bash suspends errexit for the entire duration of a function invoked as a condition, so a failing test only sets $?=1 which the bare return propagates. Boolean-correct and fail-closed (cp_rc defaults to 1). Suggested return "$cp_rc" is an equivalent, marginally more robust style change but not required. (scripts/dev-lead-fix-reviews.sh:1454)
• INFO: Gate design is sound: ri_may_resolve compares pre-pass HEAD_SHA against post-pass git rev-parse HEAD, opening only when the head genuinely moved; dry-run passes through to preserve announce-only behaviour; SHA-unavailable falls back to cp_rc==0. The review-changes branch refactor (cp_rc=0; commit_and_push || cp_rc=$?) preserves prior semantics while exposing cp_rc for the gate. (scripts/dev-lead-fix-reviews.sh:1624)
• INFO: MCP run_secret_scanning tool is not available in this environment; skipped per instructions. Diff contains only shell logic and bats tests — no credential-bearing content. gitleaks CI check passed. (n/a)

---

Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5). Reply if you need a human review.

[don-petry]

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

Agent reasoning

Issues addressed: 0
- SonarCloud quality gate passed (no actionable defects found)
- Gemini code-assist false-positive dismissed in approved review as not-required
Files changed: none (no fixes needed)
Skipped (informational): 1 (SonarCloud quality gate summary)
```
**No action required.** All CI checks pass, the PR is approved, and the SonarCloud comment contains no specific, actionable issues to address.

[donpetry-bot]

Superseded by automated re-review at 01ccd95b89d4e2cdfe754fc4c24e7ff64f48a732 — click to expand prior review.

Review — fix requested (cycle 2/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

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

Summary

Re-review after the prior cascade approval at 0a818e9: the single new commit applies exactly the Gemini-suggested return "$cp_rc" hardening in resolution_gate_open — semantically equivalent, slightly more robust; no other changes. The gate implementation itself is verified sound. The only blocker is one unresolved CodeAnt review thread (Major) that no one has answered.

Linked issue analysis

Issue #1617 (slice 2 of #1609) is substantively addressed — every acceptance criterion is met: resolution-integrity.sh is sourced; resolution_gate_open compares pre-pass HEAD_SHA vs post-pass git rev-parse HEAD via ri_may_resolve, falls back to cp_rc only when a SHA is unavailable, and passes dry-run through; all three resolve_* nets are gated in all three intent branches (review-changes now captures cp_rc); the closed gate emits a ::notice:: naming #1609; existing no-commit tests were updated and three new per-intent no-commit tests assert zero resolutions; resolution_gate_open is declared exactly once. Ordering verified: no try_enable_auto_merge call site (the only HEAD_SHA mutator, line 841) precedes any of the three gate evaluations.

Findings

• BLOCKING — unresolved review thread: CodeAnt (Major · Sometimes) at scripts/dev-lead-fix-reviews.sh:1449-1455 — HEAD_SHA is the event-time SHA while checkout loads the current PR head, so if another actor pushes between event dispatch and the pass, a no-commit pass sees differing SHAs and the gate opens. The implemented check is "head differs from event-time SHA", weaker than the PR's stated invariant "this pass advanced the head". Narrow race, no regression vs pre-PR behaviour (which resolved unconditionally), and the head did genuinely move in that scenario — but the thread is unanswered. Resolve by either (a) snapshotting pre_sha from git rev-parse HEAD immediately after checkout instead of trusting event-time HEAD_SHA, or (b) replying in-thread that this is accepted slice-3 (Follow-up to #1609 (slice 2): gate dev-lead thread resolution on the pass having advanced the PR head #1617 out-of-scope) territory and resolving the thread.
• INFO: 0a818e9→553202a diff is a single 1-file/+1−2 change replacing [ "$cp_rc" -eq 0 ]; return with return "$cp_rc" — the prior cascade already assessed the original as a false positive; the new form is equivalent and addresses the Gemini thread, which is now resolved.
• INFO: run_secret_scanning MCP tool unavailable in this environment; skipped. Diff is shell logic + bats tests only; gitleaks CI passed.

CI status

All required checks green at 553202a: bats, unit-tests, shellcheck/ShellCheck, actionlint, CodeQL, SonarCloud (quality gate passed, 0 new issues), gitleaks, AgentShield, duplicate-decl-gate, holdout-guard, and all validate-* checks SUCCESS. CANCELLED entries (dev-lead/dispatch, review/review, ci-relay, resume) are superseded runs with SUCCESS duplicates. mergeStateStatus BLOCKED reflects the pending required review only.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

Additional tasks

1. Resolve all unresolved review thread comments from other reviewers
2. Ensure all CI checks pass after your changes
3. Rebase on the target branch if behind
4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

Superseded by automated re-review at 553202a.

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

[donpetry-bot]

Automated review — APPROVED ✓

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

Summary

Cycle-3 re-review after the cycle-2 fix request at 553202a. The sole blocker there — an unresolved CodeAnt thread on the event-time HEAD_SHA race in resolution_gate_open — is fully resolved: the new commits implement the stronger of the two suggested remedies, snapshotting an immutable RESOLUTION_BASE_SHA from git rev-parse HEAD immediately after worktree checkout and gating on that instead of the event-time HEAD_SHA, plus a dedicated stale-HEAD_SHA regression test proving a no-commit pass resolves zero threads even when HEAD_SHA differs from the checked-out head. The remaining delta since 553202a is a clean merge of main (#1622 few-shot work, untouched by this PR). Both review threads are resolved; no new issues found.

Linked issue analysis

Issue #1617 (slice 2 of #1609) is substantively addressed — every acceptance criterion verified at 01ccd95: scripts/lib/resolution-integrity.sh is sourced (line 15); resolution_gate_open compares the pre-pass snapshot against post-pass git rev-parse HEAD via ri_may_resolve (pure, fail-closed helper confirmed present at PR head), falls back to cp_rc only when a SHA is genuinely unavailable, and passes dry-run through; all three resolve_* nets are gated in all three intent branches, with review-changes now capturing cp_rc; the closed gate emits a ::notice:: naming #1609; existing no-commit tests were updated to advance the head; four new tests cover no-commit passes in each intent branch plus the stale-HEAD_SHA race; resolution_gate_open is declared exactly once (line 1457) and RESOLUTION_BASE_SHA is assigned exactly once (line 79), immune to the HEAD_SHA reassignment in try_enable_auto_merge (line 850).

Findings

• RESOLVED (prior cycle-2 blocker): CodeAnt thread on the event-time HEAD_SHA race — fixed via the immutable RESOLUTION_BASE_SHA snapshot (option (a) from the prior review, the stronger remedy) and covered by the new stale-SHA regression test. Thread is resolved and outdated.
• RESOLVED (prior): Gemini return "$cp_rc" hardening — already applied at 553202a; thread resolved.
• INFO: The 553202a→01ccd95 delta beyond the gate fix is a merge of main bringing in #1622 (few-shot eval work); the PR itself still touches only scripts/dev-lead-fix-reviews.sh and tests/dev-lead/unit/test_fix_reviews.bats.
• INFO: run_secret_scanning MCP tool unavailable in this environment; skipped. Diff is shell logic + bats tests only; gitleaks CI passed.

CI status

All checks green at 01ccd95: bats, unit-tests, unit, shellcheck/ShellCheck, actionlint, CodeQL (actions + python), SonarCloud quality gate passed, gitleaks, AgentShield, Agent Security Scan, duplicate-decl-gate, holdout-guard, template-drift, caller-stub-freeze, and all validate-* checks SUCCESS; remaining entries SKIPPED by design. mergeStateStatus BLOCKED reflects only the pending required review.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

[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

[don-petry]

Dev-Lead — rate-limited (intent: fix-bot-comment)

PR: #1625
Please re-trigger manually (re-mention @dev-lead) when the rate limit clears — the original request cannot be reconstructed automatically.

[donpetry-bot]

Automated review — APPROVED ✓

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

Summary

Implements issue #1617 (slice 2): gates dev-lead review-thread auto-resolution on the pass having actually advanced the PR head. Adds resolution_gate_open wired to the pure ri_may_resolve predicate from #1609 (verified present on main), gates all three resolve_* nets in all three intent branches (fix-reviews, fix-bot-comment, review-changes), captures cp_rc in review-changes, preserves dry-run announce-only behavior, and emits a ::notice:: naming #1609 when the gate closes. Exceeds the issue spec by snapshotting an immutable RESOLUTION_BASE_SHA at checkout to close a stale-HEAD_SHA hole, with a dedicated regression test. Triage assessment confirmed correct.

Linked issue analysis

Closes #1617. All acceptance criteria are substantively met: (1) sources scripts/lib/resolution-integrity.sh; (2) resolution_gate_open compares pre-pass vs post-pass SHA via ri_may_resolve, falls back to cp_rc only when a SHA is genuinely unavailable, and passes dry-run through; (3) all three resolve_* call sites gated in all three intent branches, with review-changes now capturing cp_rc; (4) gate-closed path emits ::notice:: naming #1609; (5) existing no-commit resolution tests updated to open the gate legitimately; (6) new tests assert zero threads resolved on a no-commit pass in each of the three intent branches, plus a stale-HEAD_SHA regression test; (7) duplicate-decl-gate CI check green. The RESOLUTION_BASE_SHA snapshot is a justified deviation from the issue's literal HEAD_SHA wording — HEAD_SHA is event-time and reassigned by try_enable_auto_merge, so comparing it could wrongly open the gate on a no-commit pass; the deviation is documented in-code and regression-tested.

Findings

No blocking findings.

• Secret scan (MCP): run_secret_scanning tool not available in this run; gitleaks CI check is green. No secrets or credentials in the diff.
• Since the prior automated approval at 01ccd95, the only new commit is a merge of main (bringing in unrelated #1626 files); the PR's own two files are unchanged.
• Fail-closed semantics verified: ri_may_resolve returns non-zero on empty or unchanged SHAs, so the #1024 no-commit vector (18 threads incl. a Critical finding resolved with no commit) is closed.
• Minor pre-existing quirk (not introduced here): fix-bot-comment calls try_enable_auto_merge both inside and after the cp_rc branch; unchanged by this PR.
• 0 unresolved review threads; no unanswered human-reviewer questions (don-petry comments are the dev-lead automation persona).

CI status

All required checks green: shellcheck, ShellCheck, bats, unit-tests, unit, duplicate-decl-gate, CodeQL (actions + python), Secret scan (gitleaks), SonarCloud, agent-shield, actionlint, Lint, holdout-guard, validate-fixtures, and the full stub/permissions/persona validation suite. CANCELLED entries are superseded dev-lead dispatch/relay/resume runs and an earlier review run replaced by a successful one; SKIPPED entries are ecosystem audits not applicable to this repo.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-09-01T03:40:36Z.