Skip to content

feat: implement issue #1995 — info_status_pattern covers only SonarCloud — Codex/Qodo/CodeRabbit usage-limit notices are still undispositionable and still strand PRs - #2000

Merged
don-petry merged 10 commits into
mainfrom
dev-lead/issue-1995-20261001-1858
Oct 1, 2026
Merged

don-petry merged 10 commits into
mainfrom
dev-lead/issue-1995-20261001-1858

Conversation

@don-petry

@don-petry don-petry commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

info_status_pattern covers only SonarCloud — Codex/Qodo/CodeRabbit usage-limit notices are still undispositionable and still strand PRs

From the issue: This is the generalization that #1918's review flagged ("do not special-case SonarCloud, or the next reviewer added to the council reopens it"). chatgpt-codex-connector is that next case, and it is live now.

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_maintainer_comment_gate.bats,tests/test_reviewer_sources.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 #1995

Review in cubic

…oud — Codex/Qodo/CodeRabbit usage-limit notices are still undispositionable and still strand PRs
@don-petry
don-petry requested a review from a team as a code owner October 1, 2026 19:12
@qodo-code-review

This comment has been minimized.

@chatgpt-codex-connector

This comment has been minimized.

@coderabbitai

This comment has been minimized.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #2000
No changes were committed, but the PR still can't be marked done: required checks SonarCloud, dependency-audit / Detect ecosystems are still pending. The retry cron will re-attempt automatically. Next attempt after: 2026-10-01T19:43:48Z

@don-petry

Copy link
Copy Markdown
Collaborator Author

Note

@don-petry I reviewed this PR and no code changes were needed, but I can't mark it done yet: required checks SonarCloud, dependency-audit / Detect ecosystems are still pending. I'll re-check automatically.
Next attempt after: 2026-10-01T19:43:48Z

@don-petry
don-petry enabled auto-merge (squash) October 1, 2026 19:13
@don-petry

Copy link
Copy Markdown
Collaborator Author

No description provided.

Comment thread tests/dev-lead/unit/test_maintainer_comment_gate.bats
Comment thread tests/test_reviewer_sources.bats
@donpetry-bot

Copy link
Copy Markdown
Contributor

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces generalized info-status patterns for service notices (such as usage-limit, trial-ended, and review-limit messages) from Codex, CodeRabbit, and Qodo to prevent these non-finding comments from blocking the maintainer-comment gate. It also adds comprehensive unit tests to verify that service notices are auto-cleared while actual review findings continue to block. The review feedback suggests tightening the regex patterns in reviewer-sources.tsv to prevent overly broad matching, and updating the BATS assertions to check for specific non-zero exit codes instead of using generic non-zero checks to avoid false positives on unexpected test failures.

Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread tests/test_reviewer_sources.bats Outdated
Comment thread tests/test_reviewer_sources.bats Outdated
Comment thread tests/test_reviewer_sources.bats Outdated
@don-petry
don-petry disabled auto-merge October 1, 2026 19:15
@donpetry-bot donpetry-bot added the needs-human-review Flagged by automated PR review agent label Oct 1, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread scripts/lib/reviewer-sources.tsv Outdated
Comment thread tests/test_reviewer_sources.bats
Comment thread tests/test_reviewer_sources.bats Outdated
@don-petry

Copy link
Copy Markdown
Collaborator Author

Acknowledged: this is Qodo's "trial has ended" service notice — it carries no review finding, so no code action is needed. This PR (#1995) teaches the maintainer-comment gate to auto-clear exactly this notice via the qodo-code-review info_status_pattern in scripts/lib/reviewer-sources.tsv, so it no longer strands the PR.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Acknowledged: this is Codex's "You have reached your Codex usage limits for code reviews" out-of-quota notice — no review finding, so no code action is needed. This PR (#1995) teaches the maintainer-comment gate to auto-clear this notice via the chatgpt-codex-connector info_status_pattern in scripts/lib/reviewer-sources.tsv.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Acknowledged: this is CodeRabbit's auto-generated rate-limit/summary notice — no actionable review finding, so no code action is needed. This PR (#1995) teaches the maintainer-comment gate to auto-clear CodeRabbit's "Review limit reached … prepaid credits" notice via its info_status_pattern in scripts/lib/reviewer-sources.tsv.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Acknowledged: SonarCloud reports Quality Gate Passed — a clean status re-posted on every push with no finding, so no action is needed. The sonarqubecloud info_status_pattern already auto-clears this (#1918).

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (applied)

Changes committed and pushed. Requested items addressed:

  • tests/dev-lead/unit/test_maintainer_comment_gate.bats:337 — applied
  • tests/test_reviewer_sources.bats:197 — applied
  • scripts/lib/reviewer-sources.tsv:61 — applied
  • scripts/lib/reviewer-sources.tsv:62 — applied
  • scripts/lib/reviewer-sources.tsv:64 — applied
  • tests/test_reviewer_sources.bats:201 — applied
  • tests/test_reviewer_sources.bats:211 — applied
  • tests/test_reviewer_sources.bats:221 — applied

@donpetry-bot

Copy link
Copy Markdown
Contributor

Automated review paused — review engines are unavailable.

Every configured review engine (Claude → Copilot → Gemini) reported a usage/rate limit or was otherwise unavailable, so no review could be generated for this run. This is a single, deduplicated notice: the agent will re-review automatically on its next scheduled run once capacity recovers, and will not post repeat notices in the meantime.

@don-petry
don-petry enabled auto-merge (squash) October 1, 2026 19:24
@don-petry

don-petry commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator Author
Resolved — the `needs-human-review` hold was lifted; dev-lead has picked this item up. Click to expand the prior hold notice.

dev-lead is withholding action on this item.

It is labeled needs-human-review (flagged for human review — this label is applied by automation as well as by people, so an item can become held without anyone noticing), so dev-lead will not pick it up while that label is present. This notice is posted once so the withhold is visible rather than looking like a stalled run.

To re-enable automated pickup: remove the needs-human-review label.

@donpetry-bot

Copy link
Copy Markdown
Contributor

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

@donpetry-bot

donpetry-bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 9b79eabff600a61727c5e4f50c72ef679f190639 — click to expand prior review.

Review — fix requested (cycle 1/3)

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

Findings to fix

Automated review — NEEDS HUMAN REVIEW

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

Summary

Adds info_status_pattern entries for chatgpt-codex-connector, coderabbitai and qodo-code-review (usage-limit / review-limit / trial-ended notices), documents the column as no-finding-notices-only, and adds positive/negative bats coverage. The gate-side change is sound, but registering these finding-producing bots also widens the maintainer-resolve-comment.sh bot path, which authorizes by login only. CI is still running and a review thread is unresolved.

Linked issue analysis

Closes #1995. AC1 (patterns for Codex, Qodo, CodeRabbit), AC3 (one match/no-match test per pattern), AC4 (--reason untouched) and AC5 (registry doc block) are addressed. AC2 is partly met: the qodo and coderabbit patterns are full literal sentences, but the Codex pattern is the single phrase You have reached your Codex usage limit. That phrase is specific enough, but it is not the "several required literals in sequence" shape the new doc block asks for. The issue's note "do not widen the bot path's author check" is undercut in effect: see finding 1.

Findings

  1. [MEDIUM, blocking] Registering finding-producing bots widens the manual minimize escape hatch. scripts/maintainer-resolve-comment.sh:401-418 builds the bot-path allowlist from reviewer_sources_info_status_patterns | cut -f1 and authorizes with mrc_is_registered_bot. That check compares the login only; it never tests the comment body against the pattern. Before this PR only sonarqubecloud was on that list. After it, any issue comment from chatgpt-codex-connector, coderabbitai or qodo-code-review can be minimized RESOLVED with --reason, including real findings such as a CodeRabbit walkthrough or a Qodo "PR Review" comment. That skips dev-lead's verified-fix flow. It contradicts the script's own documented invariant at lines 389-394 ("a clean-status re-poster like SonarCloud, NOT a finding-producing reviewer") and the PR's new registry text ("Never reach for this column to wave away an inconvenient review"). Fix: on the bot path, also require the comment body to match that login's info_status_pattern (the same test($p) the gate uses), and add a bats case showing a real Codex/CodeRabbit/Qodo finding comment is refused (exit 3).
  2. [LOW] Unresolved review thread: cubic-dev-ai P3 at tests/test_reviewer_sources.bats:203. None of the negative review bodies shares a token with its pattern, so the tests don't exercise AC2's real failure mode. A useful negative case would be a review that quotes part of the notice, e.g. a Qodo review mentioning "trial has ended".
  3. [LOW, non-blocking] Pattern tightness: the Codex pattern is one unanchored phrase. Consider requiring a second literal in sequence (e.g. "usage limit" followed by "code reviews", while still allowing the short form) to match the doc block's guidance. Acceptable as-is, because a real Codex review is unlikely to contain that exact phrase.
  4. Gate behaviour verified: maintainer-comment-gate.sh strips the [bot] suffix and applies an unanchored test($p), so the top-level | alternation in the Qodo pattern behaves as intended. The PR carries the needs-human-review label.
  5. Secret scan via MCP: the run_secret_scanning tool is not available in this run. The gitleaks check passed.

CI status

Not all green yet. bats and unit are IN_PROGRESS, cubic · AI code reviewer is IN_PROGRESS and CodeRabbit is PENDING. Lint, shellcheck, CodeQL, SonarCloud, gitleaks, actionlint and the other required checks passed. Several dev-lead/review runs show CANCELLED, which is normal superseded-run churn.


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.

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.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 1, 2026
@donpetry-bot

donpetry-bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 2c77e41d28d7c9fdd5c2675ba36bb490404c6c26 — click to expand prior review.

Review — fix requested (cycle 1/3)

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

Findings to fix

Automated review — NEEDS HUMAN REVIEW

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

Summary

Re-review of the same head SHA (9b79eab), so nothing has changed since the prior fix-requested review. The PR adds info_status_pattern entries for chatgpt-codex-connector, coderabbitai and qodo-code-review so their usage-limit, review-limit and trial-ended notices auto-clear the maintainer-comment gate. It also documents the column as for no-finding notices only and adds match and no-match bats tests. The gate change is correct. However, adding these logins to the info-status list also widens the manual-minimize bot path in maintainer-resolve-comment.sh. That path authorizes by login only, so the prior blocking finding still stands. CI is now fully green.

Linked issue analysis

Closes #1995.

  • AC1 (patterns for Codex, Qodo and CodeRabbit): met.
  • AC3 (a match and a no-match test per pattern): met. Both the gate-level and the registry-level tests are present.
  • AC4 (--reason stays mandatory): met. Untouched.
  • AC5 (registry doc block): met.
  • AC2 (tight anchoring): acceptable. The qodo and coderabbit patterns are full literal sentences. The Codex pattern is a single specific sentence prefix, You have reached your Codex usage limit.
  • The issue's note "Do not widen ... the bot path's author check" is not followed in effect. See finding 1.

Findings

  1. [MEDIUM, blocking — carried forward, not addressed] Registering finding-producing bots widens the manual-minimize escape hatch. In scripts/maintainer-resolve-comment.sh (around lines 401–418 at this SHA), the bot-path allowlist is reviewer_sources_info_status_patterns | cut -f1. Authorization then goes through mrc_is_registered_bot, which compares the author login only and never tests the comment body against that login's pattern.
    • Before this PR, only sonarqubecloud was on the list. After it, a maintainer can minimize-as-RESOLVED (with --reason) any issue comment from chatgpt-codex-connector, coderabbitai or qodo-code-review. That includes real findings such as a CodeRabbit walkthrough or a Qodo "PR Review" comment, which bypasses dev-lead's verified-fix flow.
    • This contradicts the script's own invariant (lines 389–394: "a clean-status re-poster like SonarCloud, NOT a finding-producing reviewer"), this PR's new registry text, and the issue's "do not widen the bot path's author check".
    • The gate now auto-clears matching notices, so the widened bot path adds nothing for notices. Its only new reach is comments that do not match, i.e. findings.
    • Fix: on the bot path, also require the comment body to match the author's info_status_pattern, using the same unanchored jq test($p) the gate uses. Add a bats case showing that a real Codex, CodeRabbit or Qodo finding comment is refused with exit 3.
  2. [LOW] Unresolved review thread: cubic-dev-ai P3 at tests/test_reviewer_sources.bats:203. None of the negative bodies shares a token with its pattern, so a regression that loosens an anchor would not be caught. Add a negative case that quotes part of a notice, for example a Qodo review body mentioning "trial has ended" or a Codex review mentioning "usage limit". Then reply to or resolve the thread.
  3. [LOW, non-blocking] Codex pattern shape: it is one unanchored phrase, not "several required literals in sequence" as the new doc block recommends. This is acceptable as-is because a real Codex review is unlikely to contain that exact sentence.
  4. Verified: maintainer-comment-gate.sh strips the [bot] suffix and applies test($p), so the top-level | alternation in the Qodo pattern behaves as intended. The AC9b test was correctly retargeted to a genuine Qodo finding.
  5. Secret scan: the run_secret_scanning MCP tool is not available in this run. The gitleaks check passed.

CI status

All checks that ran are green, including Lint, shellcheck, bats, unit, unit-tests, CodeQL, SonarCloud (Quality Gate passed), gitleaks, actionlint, CodeRabbit (approved) and cubic. The remaining CANCELLED and SKIPPED entries are superseded dev-lead, review and dependency-audit runs, which are normal churn. Merge state is BLOCKED pending review. The PR carries the needs-human-review label.


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.

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.

…dy start

Reconciles this PR with #1998 (#1993), which pins the same Codex notice.
The unanchored substring would clear any Codex comment that merely quotes
the notice further down, including one carrying a real finding. Anchoring
on ^ keeps both observed forms of the notice (short "usage limit." and long
"usage limits for code reviews") while a quoted occurrence still blocks.

Adds a negative test to the registry suite and to the gate suite; both
fail without the anchor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R
@donpetry-bot

Copy link
Copy Markdown
Contributor

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

@don-petry

Copy link
Copy Markdown
Collaborator Author

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 match — auto-cleared, no reply needed
Files changed: (none)
Tier 1 blockers: 0
```

@don-petry
don-petry enabled auto-merge (squash) October 1, 2026 20:36
…otices only

Two review findings on #2000, plus a regression I introduced in 6f05785.

1. pr-review finding 1 (MEDIUM, blocking). maintainer-resolve-comment.sh
   authorized the registered-bot path by login only. Now that Codex,
   CodeRabbit and Qodo are registered, that let a maintainer minimize any of
   their FINDINGS as RESOLVED. The bot path now also requires the comment
   body to match the author's info_status_pattern (new pure helper
   mrc_bot_body_matches, the same case-sensitive jq test() the gate uses).
   The snapshot query reads `body`. A finding, or SonarCloud's "Quality Gate
   failed", is refused with exit 3.

2. CodeRabbit security review (moderate). A matching notice prefix cleared
   the whole comment, including finding text after it. Each pattern now
   matches the bot's COMPLETE known notice body, anchored at both ends
   (^ ... \s*$), with the observed optional parts (dashboard/settings links,
   billing link, the second sentence).

3. Regression. The start anchor in 6f05785 rejected Qodo's REAL trial
   notice, which reads "<!-- qodo:billing-blocked -->\n\n**ⓘ Qodo reviews
   are paused ...**". The pattern now allows the bold and ⓘ wrapper.

Tests: the real Codex and Qodo bodies posted on these PRs clear; each
notice followed by a finding blocks; the bot-path refusals. The new
resolve-comment tests fail without the script change. The advisory_gate
projection is still 5 rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

Copy link
Copy Markdown
Collaborator Author

Addressed in 1b86c65:

  • pr-review finding 1 (MEDIUM, blocking). In maintainer-resolve-comment.sh the bot path authorized by login only, so once Codex, CodeRabbit and Qodo were registered, their findings could be minimized by hand.
    • The path now also requires the comment body to match that login's info_status_pattern. This uses the new mrc_bot_body_matches, the same jq test() the gate runs, and the snapshot query now reads body.
    • A finding, or SonarCloud's "Quality Gate failed", is refused with exit 3.
    • New bats cases fail without the change.
  • CodeRabbit security review (moderate). A matching notice prefix cleared the whole comment, so findings after the notice cleared too. Each pattern now matches the bot's complete known notice body, anchored at both ends, so a notice followed by finding text blocks. There are tests for each bot.
  • A regression I introduced in 6f05785. The start anchor rejected Qodo's real trial notice, which wraps the sentence in bold with an ⓘ after the marker comment. The pattern allows that wrapper now, and a test uses the exact body Qodo posted on this PR.

The advisory projection is still 5 rows.


Generated by Claude Code

don-petry pushed a commit that referenced this pull request Oct 1, 2026
's branch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread tests/dev-lead/unit/test_maintainer_resolve_comment.bats Outdated
cubic P3: the shim's JSON used \\n, so the body decoded to a literal
backslash-n rather than a newline, unlike a real GitHub response. The
SonarCloud pass and fail fixtures now use \n.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R
@donpetry-bot

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

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

don-petry added a commit that referenced this pull request Oct 1, 2026
…t-codex-connector "usage limits" notices so they stop blocking the maintainer gate and spawning dev-lead runs (#1998)

* feat: implement issue #1993 — Stop the Codex noise: auto-clear chatgpt-codex-connector "usage limits" notices so they stop blocking the maintainer gate and spawning dev-lead runs

* fix(reviews): address review comments [skip ci-relay]

* feat: implement issue #1995 — info_status_pattern covers only SonarCloud — Codex/Qodo/CodeRabbit usage-limit notices are still undispositionable and still strand PRs

* fix(reviews): address review comments [skip ci-relay]

* fix(reviews): address review comments [skip ci-relay]

* fix(reviewer-sources): anchor the Codex usage-limit pattern to the body start

Reconciles this PR with #1998 (#1993), which pins the same Codex notice.
The unanchored substring would clear any Codex comment that merely quotes
the notice further down, including one carrying a real finding. Anchoring
on ^ keeps both observed forms of the notice (short "usage limit." and long
"usage limits for code reviews") while a quoted occurrence still blocks.

Adds a negative test to the registry suite and to the gate suite; both
fail without the anchor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

* fix(reviewer-sources): end-anchor the Codex notice pattern; negatives reuse each pattern's literals

cubic (PRRT_kwDOR9SdIs6oGf77): the negative cases shared no tokens with
their patterns, so loosening an anchor would still pass. The Codex pattern
also matched review prose that opens with the notice's words and then
carries a finding.

- Codex pattern is now ^You have reached your Codex usage limits?( for code
  reviews)?\. — it matches both observed notice forms and nothing that
  continues the sentence.
- Each registry test gains a negative built from its own pattern's literals
  (Codex, CodeRabbit, Qodo). The new Codex negative matches under the
  previous pattern and fails the test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

* fix(dev-lead-intent): warn when the info-status classifier cannot read the registry

cubic (PRRT_kwDOR9SdIs6oGWHh): matches_info_status_pattern swallowed
registry read failures. With TRUSTED_BOTS preset the top-level registry
bootstrap never runs, so an unreadable registry silently routed every Codex
usage-limit notice back to a full fix-bot-comment engine run, which is the
spend #1993 removes.

The classifier still fails closed (routes normally) but now emits a
::warning:: on stderr when the helper is missing, the registry cannot be
read, or the pattern fails to evaluate, matching how the rest of the script
reports registry problems. New intent test covers the unreadable-registry
path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

* fix(reviewer-sources): start-anchor the CodeRabbit and Qodo notice patterns

graphite (both rows): the patterns were unanchored substrings, so any
review body that merely mentions the notice text mid-body was auto-cleared.

- Both patterns are now anchored to the body start, allowing leading HTML
  marker comments (^\s*(<!--[^>]*-->\s*)*). Qodo prefixes its real trial
  notice with <!-- qodo:billing-blocked -->, which a bare ^ would reject.
- The Qodo trial sentence is also end-anchored (\.), like the Codex one.
- Registry and gate tests: graphite's mid-body examples now block, and the
  marker-prefixed Qodo notice still clears. The CodeRabbit negatives fail
  without the anchor.
- The column docs now spell out the anchoring convention.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

* fix(info-status): bot path checks the body; patterns match complete notices only

Two review findings on #2000, plus a regression I introduced in 6f05785.

1. pr-review finding 1 (MEDIUM, blocking). maintainer-resolve-comment.sh
   authorized the registered-bot path by login only. Now that Codex,
   CodeRabbit and Qodo are registered, that let a maintainer minimize any of
   their FINDINGS as RESOLVED. The bot path now also requires the comment
   body to match the author's info_status_pattern (new pure helper
   mrc_bot_body_matches, the same case-sensitive jq test() the gate uses).
   The snapshot query reads `body`. A finding, or SonarCloud's "Quality Gate
   failed", is refused with exit 3.

2. CodeRabbit security review (moderate). A matching notice prefix cleared
   the whole comment, including finding text after it. Each pattern now
   matches the bot's COMPLETE known notice body, anchored at both ends
   (^ ... \s*$), with the observed optional parts (dashboard/settings links,
   billing link, the second sentence).

3. Regression. The start anchor in 6f05785 rejected Qodo's REAL trial
   notice, which reads "<!-- qodo:billing-blocked -->\n\n**ⓘ Qodo reviews
   are paused ...**". The pattern now allows the bold and ⓘ wrapper.

Tests: the real Codex and Qodo bodies posted on these PRs clear; each
notice followed by a finding blocks; the bot-path refusals. The new
resolve-comment tests fail without the script change. The advisory_gate
projection is still 5 rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

---------

Co-authored-by: don-petry <{}+don-petry@users.noreply.github.com>
Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@don-petry
don-petry disabled auto-merge October 1, 2026 21:14
Resolve test conflicts: keep this branch's single-escaped SonarCloud
fixture bodies (the shim's printf expands \n), and keep main's two
case-sensitivity / foreign-author Codex negative tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R
@don-petry

Copy link
Copy Markdown
Collaborator Author

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

Agent reasoning
Issues addressed: 0
- Quality Gate passed (clean-status): auto-cleared, matches info_status_pattern
Files changed: none
Skipped (informational): 0
```
No action required. PR is ready for merge.

@don-petry
don-petry enabled auto-merge (squash) October 1, 2026 21:16
@sonarqubecloud

sonarqubecloud Bot commented Oct 1, 2026

Copy link
Copy Markdown

@donpetry-bot

donpetry-bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at 2c77e41d28d7c9fdd5c2675ba36bb490404c6c26 — click to expand prior review.

Review — fix requested (cycle 3/3)

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

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: LOW
Reviewed commit: 2c77e41d28d7c9fdd5c2675ba36bb490404c6c26
Review mode: triage-approved (single reviewer)

Summary

After main was merged in following the #1998 squash, this PR's net diff against main is a 2-line test-fixture fix in tests/dev-lead/unit/test_maintainer_resolve_comment.bats. The squash already carried the substantive #1995 work into main: the info_status_pattern rows for Codex, CodeRabbit and Qodo, the registry docs, and the mrc_bot_body_matches body check on the bot path. The fixture change is correct and low-risk. The fake gh shim emits the body inside a JSON string, so \\n decoded to a literal backslash-n and \n now decodes to a real newline, which matches real SonarCloud bodies. The verdict is escalate only because bats and unit, the suites that run this file, are still in progress.

Linked issue analysis

Closes #1995 (still OPEN). The implementation of every AC is already on main via #1998 (2d65b55):

  • AC1: reviewer-sources.tsv rows for chatgpt-codex-connector, coderabbitai and qodo-code-review.
  • AC2: each pattern is anchored at both ends and matches only the complete notice.
  • AC3: match and no-match tests per pattern.
  • AC4: --reason is unchanged.
  • AC5: the registry doc block is present.

Merging this PR auto-closes #1995. That is appropriate because the work has landed, but note that this PR's own diff is now only the test-fixture follow-up.

Findings

Prior review (cycle 1, 9b79eab), now resolved:

  1. [MEDIUM, blocking] The bot path authorized by login only. Resolved. scripts/maintainer-resolve-comment.sh:172 adds mrc_bot_body_matches, and line 445 requires the body to match that login's pattern. Bats cases cover a refused Codex finding and a refused SonarCloud "Quality Gate failed" comment. This is on main.
  2. [LOW] The cubic thread on negative-case coverage. Resolved. The negative cases now reuse each pattern's literals, and there are no unresolved review threads.
  3. [LOW] The Codex pattern was too loose. Resolved. It is now anchored at both ends and allows only the known short and long notice forms.

New findings: none.

  • The fixture change '\\n' → '\n' is correct. $body is substituted verbatim into a single-quoted printf '%s' JSON payload, so \n now JSON-decodes to a real newline.
  • The SonarCloud pattern uses [\s\S]* between literals, so the pass case matches with either form. The failed case is refused because the "Quality Gate passed" literal is absent, so the test's intent holds.
  • I could not run bats locally because it is not installed on this runner.

Secret scan: the run_secret_scanning MCP tool is not available in this run. gitleaks passed.

CI status

Not all green yet.

  • Still running: bats and unit (IN_PROGRESS), which are the suites covering this change. Also pending: concurrency-config, toplevel-permissions, validate-personas, auto-rebase-retry-resilience, agents-paused-preflight, pr-review-preflight and pr-auto-review / check-and-dispatch (QUEUED).
  • Passed (49): includes shellcheck, ShellCheck, Lint-adjacent checks, CodeQL, SonarCloud (quality gate passed), gitleaks, actionlint, AgentShield and check-model-pins.
  • Cancelled (3): the dev-lead dispatch, ci-relay and resume runs were superseded, which is normal churn.

Once bats and unit pass, this PR should be approvable.


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.

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.

@don-petry
don-petry disabled auto-merge October 1, 2026 21:49
@don-petry

Copy link
Copy Markdown
Collaborator Author

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

Agent reasoning
Issues addressed: 0
- Quality Gate passed: registered clean-status pattern, auto-cleared by gate
Files changed: none
Skipped (informational): 0
```
The PR is clean. All CI checks pass, no actionable findings remain.

@don-petry
don-petry enabled auto-merge (squash) October 1, 2026 21:50

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: 2c77e41d28d7c9fdd5c2675ba36bb490404c6c26
Review mode: triage-approved (single reviewer)

Summary

After main absorbed the #1995 implementation (it landed with #1998's squash), this PR's net diff is just a 2-line test-fixture change in tests/dev-lead/unit/test_maintainer_resolve_comment.bats. The fake gh bodies now use JSON \n escapes instead of \\n, so the parsed comment body has real newlines like a real SonarCloud comment instead of a literal backslash-n. Test-only change, LOW risk. The triage-approved assessment holds.

Linked issue analysis

Closes #1995. Every acceptance criterion is met on main, which this branch is merged up to:

  1. scripts/lib/reviewer-sources.tsv declares info_status_pattern entries for chatgpt-codex-connector, coderabbitai and qodo-code-review.
  2. Each pattern is anchored at both ends (^ … \s*$) on the complete notice text.
  3. tests/test_reviewer_sources.bats and test_maintainer_comment_gate.bats cover positive and negative cases.
  4. --reason is still mandatory on the bot path.
  5. The registry header documents the rule FOR NO-FINDING SERVICE NOTICES ONLY (#1995).

mrc_bot_body_matches also stops a registered bot's finding from being minimized on the bot path. With Closes #1995, the issue will close correctly once this merges.

Findings

No blocking findings.

  • The fixture change is correct. Inside the shim's JSON string, \n decodes to a newline, while \\n decoded to a literal backslash-n, which did not represent a real comment body. The SonarCloud pattern uses [\s\S]*, so the 'passed' case still matches. The 'Quality Gate failed' case is still refused (exit 3, no writes).
  • Non-blocking: the PR title and body still describe the full feature, although the remaining diff is only a test fix. That is a cosmetic mismatch and does not need to change before merging.
  • No unresolved review threads. The bot notices (Qodo trial-ended, Codex usage-limit, CodeRabbit) are no-finding service notices that have already been acknowledged.
  • MCP secret scan: run_secret_scanning is not available in this run. gitleaks passed.

CI status

All required checks pass: bats, unit-tests, shellcheck/Lint, CodeQL, SonarCloud, gitleaks, actionlint, AgentShield, and the stub/standards guards. The dev-lead / dispatch|ci-relay|resume runs were CANCELLED; those are automation jobs superseded by newer runs, not code checks. The dependency-audit ecosystem jobs were SKIPPED because the ecosystems don't apply to this repo.


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.

@don-petry
don-petry merged commit 324813c into main Oct 1, 2026
70 of 73 checks passed
@don-petry
don-petry deleted the dev-lead/issue-1995-20261001-1858 branch October 1, 2026 21:52

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: 2c77e41d28d7c9fdd5c2675ba36bb490404c6c26
Review mode: triage-approved (single reviewer)

Summary

Confirmation review at 2c77e41. Since #1998's squash, main already holds the #1995 implementation. This PR's net diff is now a 2-line test-fixture fix in tests/dev-lead/unit/test_maintainer_resolve_comment.bats: the fake gh bodies use '\n' instead of '\n'. The shim JSON then decodes to real newlines, as in a real SonarCloud comment. The change is test-only and LOW risk. The triage-approved assessment holds.

Linked issue analysis

Closes #1995, which is already CLOSED. Its implementation reached main through #1998 (2d65b55). scripts/lib/reviewer-sources.tsv carries anchored info_status_pattern rows for chatgpt-codex-connector, coderabbitai and qodo-code-review, plus the registry doc block. The keyword on an already-closed issue is harmless. This PR is the remaining test-fixture follow-up.

Findings

No blocking findings.

  • The fixture fix is correct. The SHIM heredoc is unquoted, so $body is substituted verbatim and backslashes in the expanded value are not reprocessed. A JSON \n now decodes to a real newline. The old \\n decoded to a literal backslash-n.
  • Both affected tests still hold. The SonarCloud 'passed' fixture still matches its pattern. The 'Quality Gate failed' case is still refused with exit 3 and no writes.
  • Prior cycle findings are all resolved on main. The bot path requiring a body match through mrc_bot_body_matches and the anchored Codex, CodeRabbit and Qodo patterns are both in place. No new issues were found.
  • Review threads: there are no unresolved threads. The Qodo, Codex and CodeRabbit comments are no-finding service notices, and each has already been acknowledged.
  • Non-blocking: the PR title and body still describe the full feature, but the remaining diff is only a test fix. This is cosmetic.
  • Secret scan: the run_secret_scanning MCP tool is not available in this run. gitleaks passed.
  • Local bats run: I could not run bats locally because it is not installed on this runner. CI bats and unit both passed.

CI status

All code and quality checks are green: bats, unit, unit-tests, shellcheck/ShellCheck/Lint, CodeQL, SonarCloud (Quality Gate passed), gitleaks, actionlint, AgentShield and the stub/standards guards. The dev-lead dispatch, ci-relay and resume runs show CANCELLED or PENDING, and pr-auto-review / check-and-dispatch is QUEUED. These are automation orchestration runs, not code checks. The dependency-audit ecosystem jobs were SKIPPED because those ecosystems don't apply to this repo.


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.

don-petry pushed a commit that referenced this pull request Oct 2, 2026
… section (#2008 AC4)

rl_scope returned "" for any CodeRabbit summary that carried an
architecture_review section, so a summary with both a security review
and the `rate limited by coderabbit.ai` block was never detected as
rate-limited. pr-review then never withheld for it or scheduled the
retry, and the throttled code review was silently lost. That is the
owner's AC4 review item at 8429b59 and pr-review's cycle-1 finding.

rl_scope now returns the rate-limited block whenever it is present,
with or without a security section, and "" otherwise. The security
section is still evidence in its own right: its findings are held by
the maintainer gate and dispositioned through
reviewer_sources_finding_section_pattern, which this scope does not
touch.

Tests: PR #2000's fixture (throttled code review + security finding) is
now detected as rate-limited (fails on the previous script), and a
security section that only mentions a rate limit is not a notice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R
don-petry added a commit that referenced this pull request Oct 2, 2026
…ew is throttled separately from its code review — findings in both sections must be addressed, and an in-place edit after disposition must re-open the comment (#2009)

* feat: implement issue #2008 — CodeRabbit's Security Architecture Review is throttled separately from its code review — findings in both sections must be addressed, and an in-place edit after disposition must re-open the comment

* chore: dev-lead update (review-changes) [skip ci-relay]

* fix(reviews): address review comments [skip ci-relay]

* fix(reviews): address review comments [skip ci-relay]

* test(advisory-gate): raise line budget 600→615 for #2008 edit-time merge

The producer-path lastEditedAt merge in get_advisory_bot_states
(50bdc85) took scripts/lib/advisory-review-gate.sh to 603 lines,
failing "Advisory gate: script is minimal" in the unit job. Raise the
budget with the same rationale-comment convention as earlier raises.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

* fix(reviews): address review comments [skip ci-relay]

* fix(dev-lead-fix-reviews): converge superseded dispositions before skipping stale candidates (#2008)

When the stale gate skips a candidate (disposition predates the last edit or
timestamp is unreadable), the superseded disposition replies were not converged
to OUTDATED. This allowed a comment edited across multiple passes to accumulate
duplicate disposition replies. Now the convergence loop runs before the skip,
ensuring exactly one authorized disposition is visible even when the candidate
cannot yet be minimized.

Fixes cubic P3 finding about operational-hygiene gap in dev-lead-fix-reviews.sh.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix: count only trusted-author run markers in the stale-disposition dedup (#2008)

stale_disposition_needs_dispatch suppressed the #2008 edit re-dispatch
when any comment carried a success-shaped
`<!-- dev-lead-fix-reviews pr=<N> ... intent=fix-reviews status=... -->`
marker posted after the edit, whoever wrote it. An outside commenter
could paste one and keep a stale disposition from being re-checked.
Only markers from OWNER/MEMBER/COLLABORATOR authors (as dev-lead's own
markers are) now count. Addresses CodeRabbit's Security Architecture
finding on the #2009 summary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

* fix: honor an active rate-limit hold in the retry sweep, including the #2008 edit re-dispatch

- scan_pr_for_rate_limits read each marker's reset= and check= with
  Python-style named groups `(?P<r>...)`. jq's Oniguruma rejects that
  syntax ("undefined group option"), the `|| true` swallowed the error,
  and the reset came back empty, so is_reset_in_future never held a PR:
  every cron cycle re-dispatched a still-rate-limited fix-ci/fix-reviews
  pass. Use `(?<r>...)`.
- The #2008 stale-edit re-dispatch ran whenever nothing else was
  dispatched, including while a hold was still active, spending a pass
  that would hit the same limit. Track an active hold and skip the edit
  re-dispatch until it resets; once it resets, the retry dispatch covers
  the edit in one run. (cubic P1 on dev-lead-retry.sh:456.)
- Tests: scan-level cases for an active and an expired hold; the active
  case fails on the previous script.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

* fix: count a pass from when it started in the #2008 stale-edit dedup (cubic P1)

stale_disposition_needs_dispatch suppressed the edit re-dispatch when a
fix-reviews success marker was POSTED at/after the bot edit. A pass that
started before the edit and finished after it posted such a marker
without ever reading the edited body, so the re-dispatch was skipped
and the stale disposition stayed until some later edit.

- dev-lead-fix-reviews.sh stamps every terminal marker with read_at=,
  the time the pass started (PASS_STARTED_AT, a lower bound on when it
  read the PR's comments; validated as an ISO timestamp).
- The dedup compares read_at= against the edit, and falls back to the
  marker's createdAt only for legacy markers without the stamp.
- Tests: a pass that started before the edit but finished after it no
  longer suppresses the dispatch (fails on the previous scripts); the
  marker carries read_at=; and the COLLABORATOR trust case (cubic P3).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

* fix: detect a throttled CodeRabbit code review even beside a security section (#2008 AC4)

rl_scope returned "" for any CodeRabbit summary that carried an
architecture_review section, so a summary with both a security review
and the `rate limited by coderabbit.ai` block was never detected as
rate-limited. pr-review then never withheld for it or scheduled the
retry, and the throttled code review was silently lost. That is the
owner's AC4 review item at 8429b59 and pr-review's cycle-1 finding.

rl_scope now returns the rate-limited block whenever it is present,
with or without a security section, and "" otherwise. The security
section is still evidence in its own right: its findings are held by
the maintainer gate and dispositioned through
reviewer_sources_finding_section_pattern, which this scope does not
touch.

Tests: PR #2000's fixture (throttled code review + security finding) is
now detected as rate-limited (fails on the previous script), and a
security section that only mentions a rate limit is not a notice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015fdDZDb4mhT65MU1izeA4R

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

info_status_pattern covers only SonarCloud — Codex/Qodo/CodeRabbit usage-limit notices are still undispositionable and still strand PRs

2 participants