fix(governance): reject rate-limited review status as semantic evidence - #1347
fix(governance): reject rate-limited review status as semantic evidence#1347seonghobae wants to merge 24 commits into
Conversation
|
Warning Review limit reachedNext included review available in 58 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR separates the governance gate wrapper from its evaluator. It adds trusted CLI and repository validation, current-head governance checks, gate result publication, and deterministic tests for API failures and required-check states. ChangesPR governance gate
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to The PR improves rate-limit handling and lookup failures, but a current failure path can still report success after status filtering fails, potentially making required semantic review evidence appear unblocked; this should be fixed before merge. Repository-name validation also needs a minor tightening follow-up. Sequence Diagram(s)sequenceDiagram
participant Test as regression test
participant Gate as pr_governance_gate.sh
participant GH as GitHub CLI
participant Impl as pr_governance_gate_impl.sh
participant Check as GitHub check run
Test->>Gate: run governance gate
Gate->>GH: read PR comments and current-head statuses
GH-->>Gate: return configured responses
Gate->>Impl: execute with filtered status data
Impl->>GH: evaluate PR state and review evidence
Impl->>Check: publish blocker, waiting, or ready result
Impl-->>Test: return gate outcome
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
PR governance metadata gate is not ready for
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai review |
|
Stale predecessor-head review. It evaluated e9ca94e; both actionable review threads were subsequently marked addressed by CodeRabbit on later commits. Current exact head 166c8e9 has fresh same-head repository CI/security/coverage/opencode success. Dismissing the predecessor review does not count as current independent approval; a fresh qualifying review remains required.
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The requested fixture-isolation finding was addressed on current head 51ae919, and the corresponding CodeRabbit thread is resolved. Dismissing this predecessor-head CHANGES_REQUESTED state removes only stale blocking evidence; it does not constitute approval or satisfy the independent-review requirement.
|
@opencode-agent review current exact head |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head6336b9e6439f1e32f2c5719cc60be97446aab93e. -
Head SHA:
6336b9e6439f1e32f2c5719cc60be97446aab93e -
Workflow run: 32120046109
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend: test_pr_governance_rate_limit.py"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend: test_pr_governance_rate_limit.py"]
R1 --> V1["backend tests"]
Evidence --> S2["CI script (2 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script (2 files)"]
R2 --> V2["bash -n plus Strix self-test"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend: test_pr_governance_rate_limit.py"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend: test_pr_governance_rate_limit.py"]
R1 --> V1["backend tests"]
Evidence --> S2["CI script (2 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script (2 files)"]
R2 --> V2["bash -n plus Strix self-test"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headb67ac4c70d83cd952c97c1ff13bb4af7bfc3e5d3. -
Head SHA:
b67ac4c70d83cd952c97c1ff13bb4af7bfc3e5d3 -
Workflow run: 32250930904
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend: test_pr_governance_rate_limit.py"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend: test_pr_governance_rate_limit.py"]
R1 --> V1["backend tests"]
Evidence --> S2["CI script (2 files)"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script (2 files)"]
R2 --> V2["bash -n plus Strix self-test"]
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
opencode-agent
Bot
added
priority: medium
… normalization notes (#1347) - Cache the entrypoint's issue-comments snapshot to a shared temp file and have the evaluator reuse it, cutting one issues/<pr>/comments API call per gate run on a rate-sensitive path. - A transient comments-endpoint failure inside the status interception no longer fails the commit-status read; it passes the original payload through and the evaluator's own lookup still fails closed. - Surface wrapper stderr diagnostics (ignored-status note) on the success path instead of swallowing them in the error-file capture. - Document the exported-function constraint next to 'export -f gh'. - Add three regression tests: comments-outage decoupling, diagnostic surfacing, and one-fetch-per-run dedupe.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Current-head verification
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Exact-head maintenance evidence
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Exact-head validation update
|
|
Maintainer disposition for the current exact head f9e1751: the three Devin observations are informational and do not identify a source defect.
The current diff already includes the required cleanup trap and its regression test. No code change is warranted for these non-blocking observations; the threads can be resolved. |
1 participant
Summary
Fail closed when a CodeRabbit legacy commit status says
successbut authoritative current-head PR evidence says semantic review did not start because the review quota was exhausted. The gate removes only that contradictory successful legacy status before the unchanged structured-review evaluator consumes evidence; branch protection, required checks, and approval policy are not weakened.Root cause and remediation
scripts/ci/pr_governance_gate.shaccepted current-head CodeRabbit evidence from either check-runs or legacy commit statuses. A quota-limited CodeRabbit attempt can leave a successful legacy status while a current-head CodeRabbit comment explicitly saysReview limit reached/we couldn't start this review. That contradictory success suppressed the structured OpenCode fallback and could make readiness evidence look stronger than the semantic review that actually ran.The branch now:
PATHbefore resolving the real GitHub CLI under GitHub Actions;All previously actionable CodeRabbit review threads are resolved. The remaining formal CodeRabbit reviews are dismissed predecessor-head evidence and do not constitute current-head approval.
Current-base reconciliation
The prior exact candidate was synchronized only to
develop@c14c4912149e65af520843b1321b654d08469e24. Protecteddevelopsubsequently advanced by 26 commits tobc98789521d21271e84789888413c182aa111b4d. That protected-base delta changes topic-intelligence/backend-tool documentation and implementation paths but does not touch any of this PR's three paths.The branch was therefore reconciled by a normal non-forced two-parent merge that preserves protected
developeverywhere except the three already-reviewed governance paths. No destructive rebase or force-push was used.Exact candidate
develop@bc98789521d21271e84789888413c182aa111b4d.85e0711ac0af1f592f0b59ad8942af3b19d8b14c.0; merge-base is the exact protected base.backend/tests/test_pr_governance_rate_limit.pyscripts/ci/pr_governance_gate.shscripts/ci/pr_governance_gate_impl.shaf89a6536227c13ef56d5bc29c4291d0392e0be2had Application CI, SAST Semgrep, Security Scan, Dependency Review, Docker, and Bandit terminal-success. Those results are historical after the current-base merge and are not counted for this head.Merge boundary
Merge only after the unchanged exact current head satisfies every live required CI/security/coverage/container/review context, all actionable current-head review threads are resolved, and a qualifying independent non-author current-head approval satisfies the live organization ruleset and last-push requirement. Pending, queued, skipped-required, stale, predecessor-head, rate-limited, status-only, author-only, or model-only evidence is non-passing.
Summary by CodeRabbit
Bug Fixes
Reliability