Skip to content

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 #2008

Closed

Description

@don-petry

Problem

CodeRabbit now posts two independently throttled outputs in one summary issue comment, which it edits in place:

  1. Code review. This part can be rate-limited ("Review limit reached — You've used all free OSS reviews for now…").
  2. Security Architecture Review. This part is not throttled with the code review and can carry real findings while the code review shows the rate-limit block.

Our automation treats that comment as a single unit. It also only reacts to newly created comments. As a result, a security finding that CodeRabbit adds by editing an already-dispositioned summary is never addressed and never re-blocks the gate.

Live evidence: PR #2000, comment 5938681830 (coderabbitai[bot], created 19:12Z, edited in place through 21:15Z)

The section markers, in order:

<!-- This is an auto-generated comment: summarize by coderabbit.ai -->
<!-- This is an auto-generated comment: rate limited by coderabbit.ai --> … <!-- end of auto-generated comment: rate limited by coderabbit.ai -->
<!-- recent_review_start --> … <!-- recent_review_end -->
<!-- walkthrough_start --> … <!-- walkthrough_end -->
<!-- final_review_risk_start --> … <!-- final_review_risk_end -->
<!-- architecture_review_start -->
### Security Architecture Review
**Security architecture risk:** _🟡 Moderate_ · up to `769f3`
…
**Retained concerns**
- **Medium · security · inferred:** New exemptions classify an entire registered-reviewer comment as clean when only its opening notice matches. …
<details><summary>Security review details</summary> … **Hardening Proposals** … </details>
<!-- architecture_review_end -->

At the same moment, the rate-limit block covered commits 769f3dd→2c77e41, while the security section still reported a finding at 769f3.

Timeline:

  • 19:23Z: Dev-Lead posted disposition 5938896990, disposition=informational ("rate-limit/summary notice — no actionable review finding"). The comment was then minimized RESOLVED.
  • ~20:35Z: CodeRabbit edited the same comment to add the Moderate security finding.
  • After that: no automation re-opened, re-dispositioned or addressed it. It was fixed only because a human-driven session acted on it manually (reply 5940203474).

CodeRabbit had no review threads on #2000, and its two PR reviews were DISMISSED with empty bodies. The security finding existed only inside the edited summary comment.

Similar stale dispositions:

Root causes (code on main)

  1. Edits are invisible.
    • dev-lead.yml:42–43 triggers on issue_comment: types: [created] only. So do persona-mention.yml and pr-review-mention.yml.
    • Neither scripts/lib/maintainer-comment-gate.sh (:164–186) nor scripts/lib/comment-disposition-verify.sh compares the comment's updatedAt/lastEditedAt with the disposition's time.
    • Once a comment is minimized RESOLVED it stays cleared, whatever the bot appends later.
  2. No section awareness.
    • prompts/dev-lead/fix-bot-comment.md:113–125 dispositions a "rate-limit / 'review limit reached' notice" as informational, and :36 says "A neutral overview is not an actionable finding".
    • Nothing tells the agent to look inside CodeRabbit's sections: architecture_review/Security, Actionable comments posted, Outside diff range comments, nitpicks.
    • The rate-limit notice is what clears the whole comment, findings included.
  3. Security findings never become threads, so fix-reviews Phase 1, required_review_thread_resolution and notify_coderabbit_resolve (dev-lead-fix-reviews.sh:308–324) never see them.
  4. The registry pattern targets text CodeRabbit doesn't post.
  5. Rate-limit detection matches the whole body and keys on creation time.
    • ADVISORY_RATE_LIMIT_RE (scripts/lib/advisory-review-gate.sh:94) is matched against the entire summary, so a walkthrough that merely mentions rate limits counts.
    • detect_advisory_rate_limit (:301–326) picks the latest entry by createdAt/submittedAt, not by edit time.
    • Nothing tells "code review throttled" apart from "security review completed with findings".

Acceptance criteria

  1. Edits re-open dispositioned comments.
    • If a registered bot's comment was edited after its covering disposition (lastEditedAt/updatedAt later than the disposition's createdAt), the maintainer comment gate treats it as un-dispositioned. It is not cleared until a fresh disposition covers the current body.
    • Dev-Lead is triggered for such edits, either via issue_comment: [edited] for trusted bots (deduplicated so CodeRabbit's frequent progress edits don't spawn a run each time) or a sweep check.
    • Fail closed when the timestamps can't be read.
  2. Both CodeRabbit sections are addressed.
    • The fix-bot-comment and fix-reviews prompts treat a CodeRabbit summary as a set of sections. A rate-limit block covers only its own section.
    • Security Architecture Review findings ("Retained concerns", severity-labelled items, Hardening Proposals) and any "Actionable comments posted" / "Outside diff range" items each get a real disposition: fixed with a verifying commit, or answered/out-of-scope with a reason.
    • Only a body whose every finding-bearing section is empty or "no issues" may be dispositioned informational.
  3. A rate-limit notice never clears findings.
  4. Section-aware rate-limit detection.
    • CodeRabbit's rate-limit detection looks only inside the rate limited by coderabbit.ai marker block, not the whole body, and uses the comment's last-edit time.
    • When the code review is throttled but the security section is present, the PR still waits on, or addresses, the security findings and is not treated as "no CodeRabbit evidence".
  5. Tests:

Notes

Activity

  1. github-actions commented on Oct 1, 2026

    @github-actions
    Contributor

    Thanks for the detailed write-up and the live evidence from PR #2000 — it makes the failure mode very clear. To help us scope the fix, can you confirm the expected behaviour when CodeRabbit edits an already-dispositioned summary: should Dev-Lead be re-triggered immediately on the edited event (deduplicated), or is a sweep-time check of lastEditedAt vs. the disposition's createdAt sufficient for your needs?

  2. don-petry commented on Oct 1, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #2008 — CodeRabbit's Security Architecture Review is throttled separately from its code review

    Scope

    Make CodeRabbit's summary comment section-aware and edit-aware:

    • A rate-limit notice can never clear a comment that has findings.
    • An edit after a disposition re-opens the comment until a fresh disposition covers the current body.
    • Rate-limit detection reads only the rate limited by coderabbit.ai block, using last-edit time.

    Implementation Checklist

    • scripts/lib/reviewer-sources.sh — add reviewer_sources_finding_section_pattern, the single regex for finding-bearing sections:
      • an architecture_review section with Retained concerns, severity items, Hardening Proposals or a non-minimal risk;
      • Actionable comments posted: N>0, Outside diff range comments and Nitpick comments (N>0).
    • scripts/lib/maintainer-comment-gate.sh:
      • An info-status pattern never clears a finding-bearing body.
      • A RESOLVED registered-bot comment re-blocks when its lastEditedAt is later than its latest covering disposition, or when that disposition is informational on a finding-bearing body.
      • Fail closed (rc 2) when an edited comment's edit time can't be read.
      • Add maintainer_gate_merge_edit_times (GraphQL lastEditedAt merge, because gh pr view doesn't expose it) and a pure stale-detector shared with the sweep.
    • scripts/review-one-pr.sh — merge edit times into the snapshot before the gate runs.
    • scripts/lib/comment-disposition-verify.sh — add pure cdv_disposition_is_stale <edit_ts> <disp_createdAt>, which fails closed on bad timestamps.
    • scripts/dev-lead-fix-reviews.sh (resolve_dispositioned_comments):
      • fetch lastEditedAt;
      • unminimize a RESOLVED bot comment whose latest disposition is stale;
      • re-verify a fresh disposition posted after the edit;
      • refuse an informational disposition on a finding-bearing body.
    • scripts/maintainer-resolve-comment.sh — mrc_bot_body_matches refuses a finding-bearing body.
    • scripts/lib/advisory-review-gate.sh:
      • CodeRabbit rate-limit detection matches only inside the rate-limited marker block;
      • a summary carrying an architecture_review section is evidence (COMMENTED), not RATE_LIMITED;
      • comment time uses lastEditedAt // createdAt.
    • scripts/dev-lead-retry.sh — sweep trigger: dispatch fix-reviews when a dev-lead-dispositioned bot comment was edited after its disposition and no dev-lead run has posted since the edit. This is deduplicated, so CodeRabbit's progress edits don't spawn a run each time.
      • The caller stub's issue_comment trigger is standards-owned and can't be widened here.
    • scripts/lib/reviewer-sources.tsv — fix the coderabbitai rationale: the prepaid-credits form isn't the live notice, and the live OSS notice is deliberately not matched.
    • prompts/dev-lead/fix-bot-comment.md and prompts/dev-lead/fix-reviews.md:
      • treat a CodeRabbit summary as a set of sections; a rate-limit block covers only its own section;
      • every security, actionable and outside-diff item gets a real disposition;
      • informational is allowed only when every finding-bearing section is empty;
      • a disposition older than the comment's last edit doesn't count.
    • Docs: docs/pr-review-agent/maintainer-comment-gate.md and AGENTS.md gate paragraph.

    Tests to Write

    Test Command

    bats tests/dev-lead/unit/ tests/test_reviewer_sources.bats plus bash .dev-lead/scripts/dev-lead-lint.sh

  3. don-petry commented on Oct 1, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: Implementation Complete — PR #2009

    The implementation for issue #2008 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.

    Review happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reportsdev-leadFor dev-lead agent pickupsecuritySecurity-related PRs and issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions