feat: add GitHub adapter for coding agent bot#263
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds merge-all-PRs action and PR card UI, expands callback status handling (including "updated", "no_changes", "failed"), updates thread state transitions, introduces PR/Task card builders, and requires GITHUB_TOKEN + Redis readiness checks before callback processing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Thread
participant CodingAgentBot
participant GitHubAPI
participant ThreadState
User->>Thread: Click "Merge All PRs"
Thread->>CodingAgentBot: Trigger action "merge_all_prs"
CodingAgentBot->>ThreadState: Read PR list from thread state
loop For each PR
CodingAgentBot->>GitHubAPI: POST /merges (squash) with token
alt success
GitHubAPI-->>CodingAgentBot: 200 OK
else failure
GitHubAPI-->>CodingAgentBot: error
end
end
CodingAgentBot->>ThreadState: Set status = "merged"
CodingAgentBot->>Thread: Post consolidated results card/message
Thread-->>User: Display merge results
sequenceDiagram
participant User
participant Thread
participant OnNewMention
participant TriggerUpdatePR
participant CallbackAPI
User->>Thread: Post mention with feedback
Thread->>OnNewMention: onNewMention handler invoked
alt state is "running" or "updating"
OnNewMention->>Thread: Post busy message and exit
else state is "pr_created" with snapshotId & prs
OnNewMention->>Thread: Post "Starting update" task card
OnNewMention->>TriggerUpdatePR: triggerUpdatePR(feedback, snapshotId, branch, repo, callbackThreadId)
TriggerUpdatePR->>CallbackAPI: Send update request
CallbackAPI-->>OnNewMention: Ack
else new task
OnNewMention->>Thread: Post "Task Started" card
OnNewMention->>Thread: Update state to "running"
end
sequenceDiagram
participant CallbackAPI
participant HandleCallback
participant ThreadState
participant Thread
CallbackAPI->>HandleCallback: POST callback (status)
alt status == "pr_created"
HandleCallback->>ThreadState: Save PRs, set status
HandleCallback->>Thread: Post PR card (buildPRCard)
else status == "updated"
HandleCallback->>ThreadState: Set status "pr_created" with snapshotId
HandleCallback->>Thread: Post PR update card
else status == "no_changes"
HandleCallback->>ThreadState: Set status "no_changes"
HandleCallback->>Thread: Post "No changes" message
else status == "failed"
HandleCallback->>ThreadState: Set status "failed"
HandleCallback->>Thread: Post failure message
end
HandleCallback-->>CallbackAPI: 200
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
- Add GitHubAdapter to bot with token, webhook secret, and username config - Add onMergeAction handler to squash-merge PRs via GitHub API - Add updated callback status for PR feedback flow - Add merged status to thread state - Add GITHUB_TOKEN, GITHUB_WEBHOOK_SECRET, GITHUB_BOT_USERNAME env vars - 35 tests across 10 test files MYC-4431 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f599676 to
0d8300f
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On GitHub, follow-up comments with @mention are routed to onNewMention instead of onSubscribedMessage. Now onNewMention checks thread state: - If running/updating: tells user to wait - If pr_created with snapshot: triggers update-pr with feedback - Otherwise: starts new coding agent task Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Without this, the thread stays in "running" state after a no_changes or failed callback, blocking all future mentions with "still working." Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/coding-agent/validateCodingAgentCallback.ts (1)
12-20:⚠️ Potential issue | 🟠 Major
updatedcallbacks need a requiredsnapshotId.The new status is valid even when
snapshotIdis omitted, butlib/coding-agent/handleCodingAgentCallback.ts:56-58stores that value andlib/coding-agent/handlers/onNewMention.ts:22-31requires it for the next update cycle. A malformed callback can therefore clear the saved snapshot and make later feedback impossible. This schema needs status-aware validation here, at minimum requiringsnapshotIdforupdated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/validateCodingAgentCallback.ts` around lines 12 - 20, The schema codingAgentCallbackSchema must enforce that snapshotId is present when status is "updated"; update the Zod definition (e.g., replace the current flat object with a discriminated union on status or add a .refine) so that for status: "updated" snapshotId is required, while keeping other statuses unchanged; reference codingAgentCallbackSchema as the symbol to change so saved snapshot handling in handleCodingAgentCallback and the expectations in handlers/onNewMention remain consistent.
🤖 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/handleCodingAgentCallback.ts`:
- Around line 56-59: The "updated" case only sets snapshotId and leaves the
thread stuck in the "updating" (busy) state; after a successful update you
should reset the thread state to "pr_created" so it can accept new feedback. In
the case "updated" branch of handleCodingAgentCallback (where thread.setState
and thread.post are called), add a call to thread.setState({ state: "pr_created"
}) after persisting the snapshotId (or replace the existing setState to include
both snapshotId and state) so the thread is returned to a reusable state for
onNewMention.ts to handle future mentions.
In `@lib/coding-agent/handlers/onMergeAction.ts`:
- Around line 28-52: The loop merging PRs should catch fetch/response.json
errors per-PR and only set thread state to "merged" if every PR succeeded;
update the code in onMergeAction.ts by introducing an overallSuccess boolean
(initialized true) and inside the for (const pr of state.prs) loop wrap the
fetch/response.json in a try/catch so transport or parsing errors push a failure
message to results and set overallSuccess = false, and set overallSuccess =
false for any non-ok response; after the loop always call thread.post with the
results summary, and call thread.setState({ status: "merged" }) only when
overallSuccess is still true (otherwise set an appropriate non-merged status or
leave unchanged).
In `@lib/coding-agent/handlers/onNewMention.ts`:
- Around line 23-31: The handler currently sets thread.setState({ status:
"updating" }) before calling triggerUpdatePR, which can leave the thread stuck
in "updating" if triggerUpdatePR throws; change the flow to either call
triggerUpdatePR(...) first and only set thread.setState({ status: "updating" })
after it succeeds, or wrap triggerUpdatePR(...) in try/catch and on error call
thread.setState(...) to revert status (e.g., to previous state), post a failure
message via thread.post(...) and rethrow or handle the error; update the code
paths around triggerUpdatePR, thread.setState, and thread.post to ensure state
is rolled back on failure and that callbackThreadId/arguments (snapshotId,
branch, repo) are preserved when queueing.
---
Outside diff comments:
In `@lib/coding-agent/validateCodingAgentCallback.ts`:
- Around line 12-20: The schema codingAgentCallbackSchema must enforce that
snapshotId is present when status is "updated"; update the Zod definition (e.g.,
replace the current flat object with a discriminated union on status or add a
.refine) so that for status: "updated" snapshotId is required, while keeping
other statuses unchanged; reference codingAgentCallbackSchema as the symbol to
change so saved snapshot handling in handleCodingAgentCallback and the
expectations in handlers/onNewMention remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3fac340d-fe3e-4b96-8d58-fd4397d33d8e
⛔ Files ignored due to path filters (6)
lib/coding-agent/__tests__/bot.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/handleCodingAgentCallback.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/handlers.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/onMergeAction.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/validateCodingAgentCallback.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/validateEnv.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (9)
app/api/coding-agent/[platform]/route.tslib/coding-agent/bot.tslib/coding-agent/handleCodingAgentCallback.tslib/coding-agent/handlers/onMergeAction.tslib/coding-agent/handlers/onNewMention.tslib/coding-agent/handlers/registerHandlers.tslib/coding-agent/types.tslib/coding-agent/validateCodingAgentCallback.tslib/coding-agent/validateEnv.ts
| for (const pr of state.prs) { | ||
| const [owner, repo] = pr.repo.split("/"); | ||
| const response = await fetch( | ||
| `https://api.github.com/repos/${owner}/${repo}/pulls/${pr.number}/merge`, | ||
| { | ||
| method: "PUT", | ||
| headers: { | ||
| Authorization: `Bearer ${token}`, | ||
| Accept: "application/vnd.github+json", | ||
| "X-GitHub-Api-Version": "2022-11-28", | ||
| }, | ||
| body: JSON.stringify({ merge_method: "squash" }), | ||
| }, | ||
| ); | ||
|
|
||
| if (response.ok) { | ||
| results.push(`${pr.repo}#${pr.number} merged`); | ||
| } else { | ||
| const error = await response.json(); | ||
| results.push(`${pr.repo}#${pr.number} failed: ${error.message}`); | ||
| } | ||
| } | ||
|
|
||
| await thread.setState({ status: "merged" }); | ||
| await thread.post(`Merge results:\n${results.map(r => `- ${r}`).join("\n")}`); |
There was a problem hiding this comment.
Only mark the thread as merged after every PR merge succeeds.
This loop records per-PR failures, but Line 51 still sets status: "merged" unconditionally. Also, a thrown fetch() or response.json() error aborts the handler before any summary is posted. Track overall success, catch transport/parsing failures inside the loop, and only flip the state when all merges complete cleanly.
Suggested fix
- const results: string[] = [];
+ const results: string[] = [];
+ let allMerged = true;
for (const pr of state.prs) {
const [owner, repo] = pr.repo.split("/");
- const response = await fetch(
- `https://api.github.com/repos/${owner}/${repo}/pulls/${pr.number}/merge`,
- {
- method: "PUT",
- headers: {
- Authorization: `Bearer ${token}`,
- Accept: "application/vnd.github+json",
- "X-GitHub-Api-Version": "2022-11-28",
- },
- body: JSON.stringify({ merge_method: "squash" }),
- },
- );
-
- if (response.ok) {
- results.push(`${pr.repo}#${pr.number} merged`);
- } else {
- const error = await response.json();
- results.push(`${pr.repo}#${pr.number} failed: ${error.message}`);
- }
+ try {
+ const response = await fetch(
+ `https://api.github.com/repos/${owner}/${repo}/pulls/${pr.number}/merge`,
+ {
+ method: "PUT",
+ headers: {
+ Authorization: `Bearer ${token}`,
+ Accept: "application/vnd.github+json",
+ "X-GitHub-Api-Version": "2022-11-28",
+ },
+ body: JSON.stringify({ merge_method: "squash" }),
+ },
+ );
+
+ if (response.ok) {
+ results.push(`${pr.repo}#${pr.number} merged`);
+ } else {
+ allMerged = false;
+ const error = await response.json().catch(() => ({ message: response.statusText }));
+ results.push(`${pr.repo}#${pr.number} failed: ${error.message}`);
+ }
+ } catch (error) {
+ allMerged = false;
+ results.push(
+ `${pr.repo}#${pr.number} failed: ${error instanceof Error ? error.message : "Unknown error"}`,
+ );
+ }
}
- await thread.setState({ status: "merged" });
+ if (allMerged) {
+ await thread.setState({ status: "merged" });
+ }
await thread.post(`Merge results:\n${results.map(r => `- ${r}`).join("\n")}`);🤖 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 28 - 52, The loop
merging PRs should catch fetch/response.json errors per-PR and only set thread
state to "merged" if every PR succeeded; update the code in onMergeAction.ts by
introducing an overallSuccess boolean (initialized true) and inside the for
(const pr of state.prs) loop wrap the fetch/response.json in a try/catch so
transport or parsing errors push a failure message to results and set
overallSuccess = false, and set overallSuccess = false for any non-ok response;
after the loop always call thread.post with the results summary, and call
thread.setState({ status: "merged" }) only when overallSuccess is still true
(otherwise set an appropriate non-merged status or leave unchanged).
The callback route uses ThreadImpl which requires the Chat singleton to be registered. Without importing the bot, setState/post calls fail with "No Chat singleton registered." Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On cold starts, the callback route may execute before the lazy Redis connection is established. Explicitly await redis.connect() if not ready. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Check for "wait" (not yet started) and "connecting" (in progress) states separately to avoid "Redis is already connected" error. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/coding-agent/handleCodingAgentCallback.ts (1)
58-61:⚠️ Potential issue | 🟠 MajorReset the thread status to
pr_createdafter a successful update.The
setStatecall only persistssnapshotId, leaving the thread stuck in theupdatingstate. According tolib/coding-agent/types.ts, valid statuses includepr_created, which should be set here to allow the thread to receive future feedback.Suggested fix
case "updated": - await thread.setState({ snapshotId: validated.snapshotId }); + await thread.setState({ + status: "pr_created", + snapshotId: validated.snapshotId, + }); await thread.post("PRs updated with your feedback. Review the latest commits."); break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handleCodingAgentCallback.ts` around lines 58 - 61, The thread remains in "updating" because setState only persists snapshotId; update the case "updated" handler to call thread.setState with both snapshotId: validated.snapshotId and status: "pr_created" (i.e., setState({ snapshotId: validated.snapshotId, status: "pr_created" })) so the thread transitions back to pr_created and can accept future feedback; keep the existing await thread.post(...) call to notify the user.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/coding-agent/handleCodingAgentCallback.ts`:
- Around line 58-61: The thread remains in "updating" because setState only
persists snapshotId; update the case "updated" handler to call thread.setState
with both snapshotId: validated.snapshotId and status: "pr_created" (i.e.,
setState({ snapshotId: validated.snapshotId, status: "pr_created" })) so the
thread transitions back to pr_created and can accept future feedback; keep the
existing await thread.post(...) call to notify the user.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b96fb771-bdfe-4bf6-b7c4-9ea56deadf42
⛔ Files ignored due to path filters (1)
lib/coding-agent/__tests__/handleCodingAgentCallback.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (3)
app/api/coding-agent/callback/route.tslib/coding-agent/handleCodingAgentCallback.tslib/coding-agent/types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/coding-agent/types.ts
Use Chat SDK Card with Actions to post a rich message with: - Link buttons to review each PR - "Merge All PRs" button that triggers the onMergeAction handler Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Posts a Card with a LinkButton to https://chat.recoupable.com/tasks/{runId} so users can monitor task progress directly from the Slack thread. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/coding-agent/handlePRCreated.ts (1)
30-36:⚠️ Potential issue | 🟠 MajorPersist thread state before rendering the merge action.
Line 30 makes the button visible before the PR metadata is stored. The merge handler reads
thread.state, so a fast click can reach it with missing or staleprsdata and make merge-all fail intermittently.Suggested fix
+ const prs = body.prs ?? []; + - const prLinks = (body.prs ?? []) + const prLinks = prs .map(pr => `- [${pr.repo}#${pr.number}](${pr.url}) → \`${pr.baseBranch}\``) .join("\n"); ... - ...((body.prs ?? []).map(pr => + ...(prs.map(pr => LinkButton({ url: pr.url, label: `Review ${pr.repo}#${pr.number}` }), )), Button({ id: "merge_all_prs", label: "Merge All PRs", style: "primary" }), ]), ], }); - await thread.post({ card }); - await thread.setState({ status: "pr_created", branch: body.branch, snapshotId: body.snapshotId, - prs: body.prs, + prs, }); + + await thread.post({ card });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlePRCreated.ts` around lines 30 - 36, The merge action is rendered before thread state is persisted, so a fast click can hit the merge handler which reads thread.state.prs before it's set; in handlePRCreated.ts move the await thread.setState({...}) call to before the await thread.post({ card }) (or otherwise await state persistence before rendering the merge button) so that thread.state (including prs, branch, snapshotId, status) is guaranteed to be stored when the merge handler runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/coding-agent/handlePRCreated.ts`:
- Around line 30-36: The merge action is rendered before thread state is
persisted, so a fast click can hit the merge handler which reads
thread.state.prs before it's set; in handlePRCreated.ts move the await
thread.setState({...}) call to before the await thread.post({ card }) (or
otherwise await state persistence before rendering the merge button) so that
thread.state (including prs, branch, snapshotId, status) is guaranteed to be
stored when the merge handler runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5bc9bde0-8576-4321-b83c-3628a25fc563
⛔ Files ignored due to path filters (2)
lib/coding-agent/__tests__/handleCodingAgentCallback.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/handlePRCreated.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
app/api/coding-agent/callback/route.tslib/coding-agent/handlePRCreated.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/coding-agent/callback/route.ts
Strip GitHubAdapter and GitHub webhook env vars from coding agent bot. GITHUB_TOKEN retained for PR merge functionality via Slack button. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/coding-agent/handlers/onNewMention.ts (1)
62-64:⚠️ Potential issue | 🟡 MinorSilent failure leaves user uninformed.
The catch block logs the error but doesn't notify the user. If
triggerCodingAgentorthread.subscribe()fails after posting the "Task Started" card, the user sees a successful card but the task never runs. Consider posting a failure message:♻️ Suggested improvement
} catch (error) { console.error("[coding-agent] onNewMention error:", error); + await thread.post("Something went wrong starting the task. Please try again."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlers/onNewMention.ts` around lines 62 - 64, The catch in onNewMention currently only logs errors, leaving the user with a "Task Started" card even if triggerCodingAgent or thread.subscribe() fails; update the catch to notify the thread/user by posting a clear failure message (or updating the "Task Started" card to a "Task Failed" state) that includes the error details, e.g., call the same thread posting helper used to create the start card (or a new postTaskFailedCard helper) and include error.message/stack; ensure this runs when triggerCodingAgent or thread.subscribe throws so users are informed of the failure.
♻️ Duplicate comments (1)
lib/coding-agent/handlers/onNewMention.ts (1)
23-34:⚠️ Potential issue | 🟠 MajorThread state remains stuck after
triggerUpdatePRcompletes successfully.There are two interrelated issues in this flow:
State set before async operation (previously flagged): If
triggerUpdatePRthrows, the thread remains inupdatingstatus and rejects all future mentions.Callback doesn't reset status: Per
handleCodingAgentCallback.tslines 58-61, the"updated"callback case only callssetState({ snapshotId })— it does not transition the status back to"pr_created". This means even on success, the thread stays stuck at"updating"forever.The callback handler should reset the status:
// In handleCodingAgentCallback.ts, "updated" case: await thread.setState({ snapshotId: validated.snapshotId, status: "pr_created" });And as suggested in the prior review, wrap
triggerUpdatePRto handle failures gracefully here.
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlers/onNewMention.ts` around lines 23 - 34, The thread can get stuck in "updating" because we set status before the async and the callback never restores it; change onNewMention.ts so triggerUpdatePR is invoked inside a try/catch (or set status to "updating" only after a successful trigger) and on error call thread.setState({ status: "pr_created" }) and post an error message; additionally update handleCodingAgentCallback.ts in the "updated" case to call thread.setState({ snapshotId: validated.snapshotId, status: "pr_created" }) so successful callbacks restore the PR-created status.
🧹 Nitpick comments (1)
lib/coding-agent/handlers/onNewMention.ts (1)
44-52: Consider extracting the base URL to configuration.The hardcoded domain
https://chat.recoupable.comcould be extracted to an environment variable for flexibility across environments (staging, production). This is a minor suggestion for maintainability.const taskUrl = `${process.env.CHAT_APP_URL}/tasks/${handle.id}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlers/onNewMention.ts` around lines 44 - 52, Replace the hardcoded domain used when building the task link in the Card (currently `https://chat.recoupable.com/tasks/${handle.id}`) with a configurable base URL read from an environment/config variable (e.g. process.env.CHAT_APP_URL or a config helper) and use that to construct taskUrl for the LinkButton; ensure you provide a sensible default or throw a clear error if the config is missing so the Card/LinkButton creation (in the Card/LinkButton usage) remains robust across environments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/coding-agent/handlers/onNewMention.ts`:
- Around line 62-64: The catch in onNewMention currently only logs errors,
leaving the user with a "Task Started" card even if triggerCodingAgent or
thread.subscribe() fails; update the catch to notify the thread/user by posting
a clear failure message (or updating the "Task Started" card to a "Task Failed"
state) that includes the error details, e.g., call the same thread posting
helper used to create the start card (or a new postTaskFailedCard helper) and
include error.message/stack; ensure this runs when triggerCodingAgent or
thread.subscribe throws so users are informed of the failure.
---
Duplicate comments:
In `@lib/coding-agent/handlers/onNewMention.ts`:
- Around line 23-34: The thread can get stuck in "updating" because we set
status before the async and the callback never restores it; change
onNewMention.ts so triggerUpdatePR is invoked inside a try/catch (or set status
to "updating" only after a successful trigger) and on error call
thread.setState({ status: "pr_created" }) and post an error message;
additionally update handleCodingAgentCallback.ts in the "updated" case to call
thread.setState({ snapshotId: validated.snapshotId, status: "pr_created" }) so
successful callbacks restore the PR-created status.
---
Nitpick comments:
In `@lib/coding-agent/handlers/onNewMention.ts`:
- Around line 44-52: Replace the hardcoded domain used when building the task
link in the Card (currently `https://chat.recoupable.com/tasks/${handle.id}`)
with a configurable base URL read from an environment/config variable (e.g.
process.env.CHAT_APP_URL or a config helper) and use that to construct taskUrl
for the LinkButton; ensure you provide a sensible default or throw a clear error
if the config is missing so the Card/LinkButton creation (in the Card/LinkButton
usage) remains robust across environments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7af5a3d5-e3e9-46d4-90bc-36c4a5e4b815
⛔ Files ignored due to path filters (6)
app/api/coding-agent/__tests__/route.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included byapp/**lib/coding-agent/__tests__/bot.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/handleCodingAgentCallback.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/handlePRCreated.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/handlers.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/validateEnv.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (4)
app/api/coding-agent/[platform]/route.tslib/coding-agent/handlePRCreated.tslib/coding-agent/handlers/onNewMention.tslib/coding-agent/validateEnv.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/coding-agent/validateEnv.ts
Extract buildPRCard to share PR review links + Merge button between pr_created and updated callbacks. Updated status now resets to pr_created and shows the same card. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The review buttons already link to each PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show "repo#number → baseBranch" as plain text, buttons handle linking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The follow-up messages in a thread hit onSubscribedMessage, not onNewMention. This was still posting plain text instead of a card with the View Task button. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract busy check and update-pr trigger logic into handleFeedback, used by both onNewMention and onSubscribedMessage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/coding-agent/handlePRCreated.ts (1)
13-21:⚠️ Potential issue | 🟡 MinorDon't treat an empty
body.prsas a validpr_createdresult.Line 13 hides an empty callback behind
?? [], and Lines 17-21 still persiststatus: "pr_created".lib/coding-agent/handlers/onNewMention.tsLines 23-31 only enter the PR-update path whenstate.prs?.lengthexists, so this leaves the thread in a state that can't support the PR follow-up flow it advertises.As per coding guidelines, "Proper error handling" is required for domain functions.Suggested fix
export async function handlePRCreated(threadId: string, body: CodingAgentCallbackBody) { const thread = getThread(threadId); - const card = buildPRCard("PRs Created", body.prs ?? []); + const prs = body.prs; + if (!prs?.length) { + await thread.post("The run completed without any pull requests to review."); + await thread.setState({ status: "no_changes" }); + return; + } + + const card = buildPRCard("PRs Created", prs); await thread.post({ card }); await thread.setState({ status: "pr_created", branch: body.branch, snapshotId: body.snapshotId, - prs: body.prs, + prs, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlePRCreated.ts` around lines 13 - 21, The code treats an empty body.prs as a valid PR-created result by using the fallback "?? []" and unconditionally setting status: "pr_created" in thread.setState; change buildPRCard/thread.post and thread.setState to only run when body.prs is a non-empty array (e.g., if (Array.isArray(body.prs) && body.prs.length > 0) { const card = buildPRCard("PRs Created", body.prs); await thread.post({ card }); await thread.setState({ status: "pr_created", branch: body.branch, snapshotId: body.snapshotId, prs: body.prs }); } else { await thread.setState({ status: "no_prs", branch: body.branch, snapshotId: body.snapshotId }); } so you stop using "?? []", persist prs only when non-empty, and remain compatible with the onNewMention.ts check that expects state.prs?.length.
♻️ Duplicate comments (1)
lib/coding-agent/handlers/onNewMention.ts (1)
23-37:⚠️ Potential issue | 🟠 MajorDon't mark the thread
updatinguntil the update job is actually queued.Line 24 flips the state before
triggerUpdatePR()succeeds. If the enqueue throws, the outer catch only logs, and Lines 18-20 will block every later mention as “still working.”As per coding guidelines, "Proper error handling" is required for domain functions.Suggested fix
if (state?.status === "pr_created" && state.snapshotId && state.branch && state.prs?.length) { - await thread.setState({ status: "updating" }); - const handle = await triggerUpdatePR({ - feedback: message.text, - snapshotId: state.snapshotId, - branch: state.branch, - repo: state.prs[0].repo, - callbackThreadId: thread.id, - }); - - console.log("[coding-agent] triggerUpdatePR handle:", JSON.stringify(handle)); - const card = buildTaskCard("Updating PRs", "Got your feedback. Updating the PRs...", handle.id); - console.log("[coding-agent] posting card:", JSON.stringify({ card })); - await thread.post({ card }); + try { + const handle = await triggerUpdatePR({ + feedback: message.text, + snapshotId: state.snapshotId, + branch: state.branch, + repo: state.prs[0].repo, + callbackThreadId: thread.id, + }); + await thread.setState({ status: "updating" }); + + console.log("[coding-agent] triggerUpdatePR handle:", JSON.stringify(handle)); + const card = buildTaskCard("Updating PRs", "Got your feedback. Updating the PRs...", handle.id); + console.log("[coding-agent] posting card:", JSON.stringify({ card })); + await thread.post({ card }); + } catch (error) { + await thread.post("I couldn't start the PR update. Please try again."); + throw error; + } return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/coding-agent/handlers/onNewMention.ts` around lines 23 - 37, The code sets thread state to "updating" before calling triggerUpdatePR, so if triggerUpdatePR fails the thread remains stuck; change the sequence in onNewMention: call triggerUpdatePR(...) first, ensure it returns a successful handle (and handle enqueue errors), then call thread.setState({ status: "updating" }) and only after that buildTaskCard and thread.post; also add error handling around triggerUpdatePR to log/report the failure and avoid changing state when enqueueing fails (use the existing catch path to leave state unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/coding-agent/handlePRCreated.ts`:
- Around line 13-21: The code treats an empty body.prs as a valid PR-created
result by using the fallback "?? []" and unconditionally setting status:
"pr_created" in thread.setState; change buildPRCard/thread.post and
thread.setState to only run when body.prs is a non-empty array (e.g., if
(Array.isArray(body.prs) && body.prs.length > 0) { const card = buildPRCard("PRs
Created", body.prs); await thread.post({ card }); await thread.setState({
status: "pr_created", branch: body.branch, snapshotId: body.snapshotId, prs:
body.prs }); } else { await thread.setState({ status: "no_prs", branch:
body.branch, snapshotId: body.snapshotId }); } so you stop using "?? []",
persist prs only when non-empty, and remain compatible with the onNewMention.ts
check that expects state.prs?.length.
---
Duplicate comments:
In `@lib/coding-agent/handlers/onNewMention.ts`:
- Around line 23-37: The code sets thread state to "updating" before calling
triggerUpdatePR, so if triggerUpdatePR fails the thread remains stuck; change
the sequence in onNewMention: call triggerUpdatePR(...) first, ensure it returns
a successful handle (and handle enqueue errors), then call thread.setState({
status: "updating" }) and only after that buildTaskCard and thread.post; also
add error handling around triggerUpdatePR to log/report the failure and avoid
changing state when enqueueing fails (use the existing catch path to leave state
unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3b6ebd95-b6d5-4c58-bac4-7fbf54721cfe
⛔ Files ignored due to path filters (2)
lib/coding-agent/__tests__/handleCodingAgentCallback.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/coding-agent/__tests__/handlers.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (5)
lib/coding-agent/buildPRCard.tslib/coding-agent/buildTaskCard.tslib/coding-agent/handleCodingAgentCallback.tslib/coding-agent/handlePRCreated.tslib/coding-agent/handlers/onNewMention.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/coding-agent/handleCodingAgentCallback.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wait until Redis status is "ready" before handling callbacks. Handles all intermediate states (wait, connecting, reconnecting) to prevent IoRedisStateAdapter errors on cold starts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
GitHubAdapterto the coding agent bot (Chat SDK) for webhook-based GitHub interactionsonSubscribedMessagehandler to forward thread replies as PR update feedbackonMergeActionhandler to squash-merge PRs via GitHub APItriggerUpdatePRto trigger the update-pr background taskSUBMODULE_CONFIGfor GitHub repo/branch mappingCodingAgentPRtype and extended thread state (pr_created,updating,merged)Depends on: PR #251 (Slack adapter) being merged first
Linear: MYC-4431
Test plan
pnpm test)/api/coding-agent/github🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements