agent: @U0AJM7X8FBR I need individual buttons to merge each pull request in the#276
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplaces a single global merge-all button with individual per-PR merge buttons in the PR card UI. Updates the merge action handler to parse PR-specific action IDs and process merges for individual PRs, with immediate state updates and per-PR success/failure messaging instead of batch aggregation. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as PR Card
participant Handler as onMergeAction
participant API as GitHub API
participant State as Thread State
User->>UI: Click merge button (merge_pr:repo#123)
UI->>Handler: Emit action merge_pr:repo#123
Handler->>Handler: parseMergeActionId()
alt Valid Action ID
Handler->>State: Locate PR `#123`
alt PR Found
Handler->>API: POST /repos/owner/repo/pulls/123/merge
alt Merge Successful
API-->>Handler: Success response
Handler->>State: Remove PR from list
Handler->>State: Update overall status
Handler->>UI: Post success message
else Merge Failed
API-->>Handler: Error response
Handler->>UI: Post failure message with error
end
else PR Not Found
Handler->>UI: Post "PR not found" error
end
else Invalid Action ID
Handler->>UI: Post "Invalid merge action" error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ 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 |
70f4c1c to
e751f79
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/coding-agent/handlers/onMergeAction.ts (2)
9-13: Consider makingparseMergeActionIda private helper or moving to a separate file.Per coding guidelines,
lib/**/*.tsfiles should follow Single Responsibility Principle with one exported function per file. Currently, this file exports bothparseMergeActionIdandregisterOnMergeAction.Options:
- Remove the
exportkeyword if this helper is only used internally- Move to a dedicated
parseMergeActionId.tsutility file if needed elsewhere♻️ Option 1: Make it internal
-export function parseMergeActionId(actionId: string) { +function parseMergeActionId(actionId: string) {As per coding guidelines: "Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlers/onMergeAction.ts` around lines 9 - 13, The helper parseMergeActionId is exported alongside registerOnMergeAction, violating SRP; either remove the export from parseMergeActionId to make it an internal helper used by registerOnMergeAction, or move parseMergeActionId into its own module (e.g., parseMergeActionId.ts) and keep it exported there; update any imports/usages accordingly and ensure registerOnMergeAction references the internal helper or the new module.
24-89: Function exceeds 50-line guideline; consider extracting helpers.The
registerOnMergeActionfunction spans ~65 lines with multiple concerns: validation, API interaction, state management, and user messaging. Per coding guidelines, functions should stay under 50 lines.Potential extractions:
mergePullRequest(owner, repo, number, token)- encapsulate the GitHub API call- State update logic into a helper
This isn't blocking but would improve testability and readability.
As per coding guidelines: "Keep functions under 50 lines"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlers/onMergeAction.ts` around lines 24 - 89, The registerOnMergeAction function is too long and mixes validation, GitHub API interaction, and state updates; extract the API call into a new helper mergePullRequest(owner, repo, number, token) that performs the fetch/PUT and returns a success boolean and error info, and move the state mutation and post-merge behavior into a helper like applyMergeResultToThread(thread, state, pr, success) which updates thread.setState, computes remainingPrs/allMerged, calls handleMergeSuccess when appropriate, and posts the final message; replace the inline fetch block and state logic in registerOnMergeAction with calls to mergePullRequest and applyMergeResultToThread so the exported registerOnMergeAction body stays under 50 lines and is easier to test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/coding-agent/handlers/onMergeAction.ts`:
- Around line 81-88: The handler in onMergeAction.ts currently calls
JSON.parse(errorBody) which can throw if GitHub returns non-JSON; wrap the parse
in a try-catch (around JSON.parse(errorBody)) and use a safe fallback: if
parsing succeeds extract error.message, otherwise use the raw errorBody (or a
generic message) when calling thread.post and logging; keep the existing
console.error but include the safe error text so thread.post(`❌
${pr.repo}#${pr.number} failed to merge: ...`) always runs without throwing.
---
Nitpick comments:
In `@lib/coding-agent/handlers/onMergeAction.ts`:
- Around line 9-13: The helper parseMergeActionId is exported alongside
registerOnMergeAction, violating SRP; either remove the export from
parseMergeActionId to make it an internal helper used by registerOnMergeAction,
or move parseMergeActionId into its own module (e.g., parseMergeActionId.ts) and
keep it exported there; update any imports/usages accordingly and ensure
registerOnMergeAction references the internal helper or the new module.
- Around line 24-89: The registerOnMergeAction function is too long and mixes
validation, GitHub API interaction, and state updates; extract the API call into
a new helper mergePullRequest(owner, repo, number, token) that performs the
fetch/PUT and returns a success boolean and error info, and move the state
mutation and post-merge behavior into a helper like
applyMergeResultToThread(thread, state, pr, success) which updates
thread.setState, computes remainingPrs/allMerged, calls handleMergeSuccess when
appropriate, and posts the final message; replace the inline fetch block and
state logic in registerOnMergeAction with calls to mergePullRequest and
applyMergeResultToThread so the exported registerOnMergeAction body stays under
50 lines and is easier to test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a1486e23-269e-42b2-9fd5-6ec487ea16fd
⛔ Files ignored due to path filters (2)
lib/coding-agent/__tests__/handlePRCreated.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/onMergeAction.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
lib/coding-agent/buildPRCard.tslib/coding-agent/handlers/onMergeAction.ts
Chat SDK's onAction uses exact match, not prefix match. Changed to
a catch-all handler that filters with startsWith("merge_pr:") so
dynamic action IDs like "merge_pr:#42" are matched.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…est in the (#276) (#277) * agent: @U0AJM7X8FBR I need individual buttons to merge each pull request in the * agent: address feedback * agent: address feedback * agent: address feedback * agent: address feedback * agent: address feedback * agent: address feedback * agent: address feedback * fix: use catch-all action handler for merge_pr prefix matching Chat SDK's onAction uses exact match, not prefix match. Changed to a catch-all handler that filters with startsWith("merge_pr:") so dynamic action IDs like "merge_pr:#42" are matched. --------- Co-authored-by: Recoup Agent <agent@recoupable.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Automated PR from coding agent.
Summary by CodeRabbit