agent: @U0AJM7X8FBR Slack Action: Merge test to main in the slack client in the#279
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 (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a complete "Merge test to main" workflow by adding a card builder, GitHub branch merge utility, action handler, and integrating it into the existing merge action flow. The implementation includes validation, GitHub API integration, and user-facing feedback messages. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Bot as Coding Agent Bot
participant Handler as Merge Handler
participant GitHub as GitHub API
participant Thread as Chat Thread
User->>Bot: Click "Merge test → main" button
Bot->>Handler: Trigger merge_test_to_main action
Handler->>Handler: Validate & parse action ID
Handler->>Handler: Retrieve GITHUB_TOKEN
Handler->>GitHub: POST /repos/{owner}/{repo}/merges
GitHub-->>Handler: Success response (ok)
Handler->>Thread: Post success message
Thread-->>User: Display "Test → Main merge successful!"
Note over Handler,GitHub: On failure path:
GitHub-->>Handler: Error response
Handler->>Thread: Post error message with details
Thread-->>User: Display failure notification
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 |
in the slack client in the
sweetmantech
force-pushed
the
agent/-u0ajm7x8fbr-slack-action--mer-1773153587086
branch
from
c4dc579 to
ad471ec
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/coding-agent/handlers/onMergeTestToMainAction.ts (1)
8-13: Consider extracting parser to separate file for SRP compliance.Per coding guidelines, each file should have one exported function. This file exports both
parseMergeTestToMainActionIdandregisterOnMergeTestToMainAction.However, I see this follows the pattern established with
parseMergeActionIdbeing in its own file whileonMergeAction.tsimports it. For consistency, consider movingparseMergeTestToMainActionIdtolib/coding-agent/parseMergeTestToMainActionId.ts.That said, the tight coupling makes co-location pragmatic—this is a minor nit.
♻️ Optional: Extract parser to separate file
Create
lib/coding-agent/parseMergeTestToMainActionId.ts:/** * Parses a merge_test_to_main action ID like "merge_test_to_main:recoupable/api" * into the repo string, or null if the format doesn't match. */ export function parseMergeTestToMainActionId(actionId: string): string | null { const prefix = "merge_test_to_main:"; if (!actionId.startsWith(prefix)) return null; const repo = actionId.slice(prefix.length); return repo.includes("/") ? repo : null; }Then import it in
onMergeTestToMainAction.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlers/onMergeTestToMainAction.ts` around lines 8 - 13, Move the parser function out of onMergeTestToMainAction.ts into a new module named lib/coding-agent/parseMergeTestToMainActionId.ts and export it there as parseMergeTestToMainActionId, keeping the exact implementation; then update lib/coding-agent/handlers/onMergeTestToMainAction.ts to import parseMergeTestToMainActionId and leave registerOnMergeTestToMainAction as the sole exported symbol in that file to satisfy SRP and match the parseMergeActionId pattern used elsewhere.lib/coding-agent/mergeGithubBranch.ts (1)
26-26: Consider defensive validation for malformedrepoparameter.While
parseMergeTestToMainActionIdvalidates the repo format upstream, this function could be called from other contexts. Ifrepodoesn't contain "/",ownerandrepoNamewill beundefinedor incorrect, leading to a confusing API error.🛡️ Optional: Add defensive check
): Promise<MergeGithubBranchResult> { const [owner, repoName] = repo.split("/"); + if (!owner || !repoName) { + return { ok: false, message: `Invalid repo format: ${repo}` }; + } const response = await fetch(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/mergeGithubBranch.ts` at line 26, The destructuring const [owner, repoName] = repo.split("/") in mergeGithubBranch.ts can break if repo is malformed; add a defensive validation before that line (within the mergeGithubBranch function) to ensure repo is a non-empty string containing exactly one "/" (e.g., check typeof repo === "string" and repo.split("/").length === 2 after trimming) and if invalid throw or return a clear error (include repo value) so callers get a helpful message; reference parseMergeTestToMainActionId to note upstream validation but still perform this local check for safety.
🤖 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/mergeGithubBranch.ts`:
- Around line 49-54: The branch-merge error handler currently calls
JSON.parse(errorBody) without protection; wrap that parse in a try-catch inside
the mergeGithubBranch error handling block so non-JSON responses (HTML, proxy
pages, rate-limit text) don't throw; if JSON.parse succeeds extract
error.message, otherwise fall back to using the raw errorBody (or a generic
message) when returning { ok: false, message: ... } and ensure the process log
still includes the original errorBody; update the code around the existing
variables response, errorBody, and the return that builds the message
accordingly.
---
Nitpick comments:
In `@lib/coding-agent/handlers/onMergeTestToMainAction.ts`:
- Around line 8-13: Move the parser function out of onMergeTestToMainAction.ts
into a new module named lib/coding-agent/parseMergeTestToMainActionId.ts and
export it there as parseMergeTestToMainActionId, keeping the exact
implementation; then update lib/coding-agent/handlers/onMergeTestToMainAction.ts
to import parseMergeTestToMainActionId and leave registerOnMergeTestToMainAction
as the sole exported symbol in that file to satisfy SRP and match the
parseMergeActionId pattern used elsewhere.
In `@lib/coding-agent/mergeGithubBranch.ts`:
- Line 26: The destructuring const [owner, repoName] = repo.split("/") in
mergeGithubBranch.ts can break if repo is malformed; add a defensive validation
before that line (within the mergeGithubBranch function) to ensure repo is a
non-empty string containing exactly one "/" (e.g., check typeof repo ===
"string" and repo.split("/").length === 2 after trimming) and if invalid throw
or return a clear error (include repo value) so callers get a helpful message;
reference parseMergeTestToMainActionId to note upstream validation but still
perform this local check for safety.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c42e9190-5419-4af5-9972-309d1b484c97
⛔ Files ignored due to path filters (2)
lib/coding-agent/__tests__/mergeGithubBranch.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/onMergeTestToMainAction.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (5)
lib/coding-agent/buildMergeTestToMainCard.tslib/coding-agent/handlers/onMergeAction.tslib/coding-agent/handlers/onMergeTestToMainAction.tslib/coding-agent/handlers/registerHandlers.tslib/coding-agent/mergeGithubBranch.ts
| const errorBody = await response.text(); | ||
| console.error( | ||
| `[coding-agent] branch merge failed for ${repo} (${head} → ${base}): ${response.status} ${errorBody}`, | ||
| ); | ||
| const error = JSON.parse(errorBody); | ||
| return { ok: false, message: error.message }; |
There was a problem hiding this comment.
Missing try-catch around JSON.parse can cause unhandled exceptions.
If the GitHub API returns a non-JSON response (e.g., HTML error page during outage, proxy errors, or rate-limit pages), JSON.parse(errorBody) will throw a SyntaxError, crashing the handler and leaving the user without feedback.
🐛 Proposed fix with fallback for non-JSON responses
const errorBody = await response.text();
console.error(
`[coding-agent] branch merge failed for ${repo} (${head} → ${base}): ${response.status} ${errorBody}`,
);
- const error = JSON.parse(errorBody);
- return { ok: false, message: error.message };
+ try {
+ const error = JSON.parse(errorBody);
+ return { ok: false, message: error.message ?? errorBody };
+ } catch {
+ return { ok: false, message: `GitHub API error: ${response.status}` };
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/coding-agent/mergeGithubBranch.ts` around lines 49 - 54, The branch-merge
error handler currently calls JSON.parse(errorBody) without protection; wrap
that parse in a try-catch inside the mergeGithubBranch error handling block so
non-JSON responses (HTML, proxy pages, rate-limit text) don't throw; if
JSON.parse succeeds extract error.message, otherwise fall back to using the raw
errorBody (or a generic message) when returning { ok: false, message: ... } and
ensure the process log still includes the original errorBody; update the code
around the existing variables response, errorBody, and the return that builds
the message accordingly.
2 participants
Automated PR from coding agent.
Summary by CodeRabbit