Repository navigation
Implement native review-state query - #19
Merged
Merged
Conversation
flyingrobots
marked this pull request as ready for review
April 9, 2026 08:24
flyingrobots
changed the title
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 2 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a repo-native "review-state" feature: new engine (src/review-state.ts) that analyzes PR review/check/thread/bot signals, CLI and MCP surfaces to invoke it, supporting modules ( Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI/MCP
participant Git as Git
participant GH as "GitHub API"
participant Renderer
User->>CLI/MCP: review-state [--pr NUM | --current-branch] [--json]
CLI/MCP->>CLI/MCP: validate selectors (mutual exclusion)
alt current-branch
CLI/MCP->>Git: resolve current branch
Git-->>CLI/MCP: branch name
CLI/MCP->>GH: list PRs for branch
GH-->>CLI/MCP: pr list
else explicit PR
CLI/MCP->>GH: fetch PR details (--pr NUM)
GH-->>CLI/MCP: pr metadata
end
CLI/MCP->>GH: fetch reviews, comments, checks, unresolved thread count
GH-->>CLI/MCP: reviews[], comments[], checks[], threadCount
CLI/MCP->>CLI/MCP: classify checks, derive reviewer decisions, detect bot signals/cooldown
CLI/MCP->>CLI/MCP: assemble blockers[] and merge_ready
CLI/MCP->>Renderer: ReviewStateResult
Renderer->>Renderer: renderReviewStateText() or JSON
Renderer-->>User: output (text or JSON)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/0034-review-state-query/review-state-query.md`:
- Around line 77-80: Remove the first placeholder "## Non-goals" section that
contains the "- [ ] TBD" item; locate the initial header "## Non-goals" (the one
at lines shown in the diff) and delete that header and its bullet so only the
later concrete "## Non-goals" block remains, avoiding duplicate/conflicting
non-goal statements.
In `@docs/MCP.md`:
- Around line 30-31: Update the docs in MCP.md to explicitly state that the
selector fields for method_review_state are mutually exclusive: the `pr`
(optional) `number` and `currentBranch` (optional) `boolean` cannot both be
provided at the same time; clarify that when `pr` is omitted `currentBranch` is
treated as the default behavior, and add a short note that supplying both will
be rejected at runtime to prevent client misuse. Reference the `pr` and
`currentBranch` selector names in the text so readers can locate the constraint
easily.
In `@src/mcp.ts`:
- Around line 35-39: The MCP schema currently declares the property pr as type
'number' but the handler only accepts positive integers; update the property
definition for pr in the schema used in src/mcp.ts (the object that spreads
workspaceProperty and defines pr/currentBranch) to declare pr as type 'integer'
and add a minimum constraint of 1 so the schema enforces a positive integer at
validation time rather than letting the handler reject invalid values at
runtime.
In `@src/review-state.ts`:
- Around line 154-164: The code erroneously allows options.currentBranch ===
false with no PR and still calls resolveCurrentBranchPr; in queryReviewState
adjust the validation and selection logic: after computing currentBranch (using
options.currentBranch ?? pr === undefined), add a check that if pr === undefined
&& currentBranch === false throw a clear Error (e.g., "review-state requires
either --pr or --current-branch; cannot use currentBranch:false with no PR"),
and change the selection expression to depend on currentBranch (use selection =
currentBranch ? await resolveCurrentBranchPr(options.cwd, client) : { kind:
'selected' as const, prNumber: pr } or an appropriate none/selected variant) so
resolveCurrentBranchPr is only called when currentBranch is true; reference
symbols: queryReviewState, ReviewStateQueryOptions, currentBranch,
resolveCurrentBranchPr, selection.
- Around line 500-515: latestDecisiveStates currently ignores
non-APPROVED/CHANGES_REQUESTED states so a later DISMISSED review doesn’t clear
an earlier vote; update latestDecisiveStates to handle state === 'DISMISSED' by
checking the parsed timestamp (use parseTimestamp(review.submittedAt)) and, if
the dismissal is newer or there was no previous entry, remove the author from
the latest map (or ensure their entry is not carried forward), otherwise keep
the existing newer APPROVED/CHANGES_REQUESTED; keep existing normalization
(normalizeOptionalString) and the timestamp comparison logic when deciding
whether to replace or clear the stored entry.
In `@tests/mcp.test.ts`:
- Around line 95-109: The test stubs reviewStateQuery but never asserts it was
called with the correct selector/cwd from method_review_state; update the test
(both the call at createCallToolHarness and the similar one at lines 121-126) to
assert the mock was invoked and received the expected arguments—e.g., after
invoking the handler, add expectations like
expect(reviewStateQuery).toHaveBeenCalled() and
expect(reviewStateQuery).toHaveBeenCalledWith(expect.objectContaining({
selector: <expected selector>, cwd: <expected cwd> })) so the test verifies the
wiring from method_review_state into reviewStateQuery.
- Around line 129-160: The test promises to verify rejection "before querying
GitHub" but never asserts the GitHub query was skipped; add a spy for the GitHub
query helper used by method_review_state (e.g., vi.spyOn(yourGitHubClient,
'query') or the module function that method_review_state calls) before invoking
callToolHandler from createCallToolHarness, then after the invalid-selector
calls (the calls to callToolHandler for pr as string and currentBranch=true)
assert that that spy was not called (expect(spy).not.toHaveBeenCalled()), and
keep vi.restoreAllMocks() to clean up.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81f23be5-b544-46a5-a9e8-4d4bbae96632
📒 Files selected for processing (14)
ARCHITECTURE.mdREADME.mddocs/CLI.mddocs/MCP.mddocs/design/0034-review-state-query/review-state-query.mddocs/method/process.mdsrc/cli-args.tssrc/cli.tssrc/mcp.tssrc/review-state.tstests/cli.test.tstests/docs.test.tstests/mcp.test.tstests/review-state.test.ts
Owner
Author
|
Addressed the latest CodeRabbit round in Verification:
All seven review threads are now resolved on the PR. CodeRabbit is still re-running on the new head. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/mcp.ts`:
- Around line 138-149: The handler for request.params.name ===
'method_review_state' is passing explicit false for currentBranch into
reviewStateQuery, causing queryReviewState to error; normalize currentBranch
(from validateOptionalBoolean) to undefined when it's false (or when pr is
undefined and currentBranch === false) before calling reviewStateQuery so the
default "resolve current branch" path is used. Update the block that
assigns/uses currentBranch (the variable validated by validateOptionalBoolean
and the call to reviewStateQuery/options.reviewStateQuery) to transform false ->
undefined (or omit currentBranch) prior to dispatch.
In `@src/review-state.ts`:
- Around line 449-469: The classifyChecks loop currently treats any non-pending,
non-failing check as passing (which will include UNKNOWN); change the logic in
classifyChecks to only classify checks as passing when they explicitly match
known success states (e.g., conclusion/status values that represent success
according to GitHub API) and otherwise leave them out of the passing bucket (so
they block merge readiness); update the checks conversion path (functions
referenced: classifyChecks, toReviewCheck, isPendingCheck, isFailingCheck) to
use a whitelist of allowed success states instead of the current fallback, and
apply the same whitelist fix to the other, similar classification block
elsewhere in this file that follows the same pattern.
- Around line 245-252: The runCommand function's execFileAsync call (in
runCommand) can hit Node's default 1MB stdout limit when running gh pr view
--json reviews,comments; add a larger maxBuffer option to the execFileAsync
options (e.g., maxBuffer: 10 * 1024 * 1024 or larger) so large PR review/comment
payloads don't trigger ERR_CHILD_PROCESS_STDIO_MAXBUFFER and cause the command
to fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0173025f-3fd3-4a60-b864-2bfa62f70b8e
📒 Files selected for processing (6)
docs/MCP.mddocs/design/0034-review-state-query/review-state-query.mdsrc/mcp.tssrc/review-state.tstests/mcp.test.tstests/review-state.test.ts
Owner
Author
|
Addressed the latest follow-up review round in Verification:
The latest CodeRabbit threads are now replied-to with SHA-backed outcomes, and the branch remains green locally. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/review-state.ts`:
- Line 591: The variable latestHumanMeaningfulTimestamp is misnamed because it
actually stores the max timestamp of the latest bot review or comment; rename it
(e.g., latestBotMeaningfulTimestamp or latestBotActivityTimestamp) and update
all references (notably where it's read at the places corresponding to the
original lines ~601 and ~611) to use the new identifier so the name matches its
contents everywhere in the module (search for latestHumanMeaningfulTimestamp and
replace with the new name).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56c45f62-013d-4cfa-bb64-10a05244895e
📒 Files selected for processing (5)
src/mcp.tssrc/review-state.tstests/mcp.test.tstests/review-state-exec.test.tstests/review-state.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Implemented a native METHOD review-state query so PR merge-readiness can be inspected without reconstructing
ghstate by hand.This adds a shared
src/review-state.tsengine that:--prIt also wires that engine into:
method review-state [--pr NUMBER | --current-branch] [--json]method_review_statein the MCP serverThe docs and active cycle design were updated to reflect that review visibility is now a repo-native METHOD query surface.
Why it changed
Recent PR review work made it obvious that METHOD had a gap here: the repo could answer backlog and cycle questions, but not the simple coordination question of what is under review and whether it is actually merge-ready.
Doghouse already demonstrated the right shape of solution. This PR ports the useful semantics into METHOD natively instead of depending on an external tool.
Impact
Root cause
Review-state knowledge existed only as ad hoc forge inspection, so every merge-readiness check had to be rebuilt from
ghoutput and reviewer heuristics.Validation
npm run buildnpm testgit diff --check./node_modules/.bin/tsx src/cli.ts review-state --pr 18 --json