feat(chat): wire stop button to POST /api/chat/{chatId}/stop#1770
feat(chat): wire stop button to POST /api/chat/{chatId}/stop#1770arpitgupta1214 wants to merge 9 commits into
Conversation
The stop button only called the AI SDK stop() (local fetch abort), leaving
the durable workflow run streaming and billing server-side. Wrap stop so it
also fires POST /api/chat/{chatId}/stop (fire-and-forget, workflow chats only)
to cancel the run.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a backend-aware stop flow: new stopChatWorkflow POSTs to /api/chat/{chatId}/stop; useStopChatWorkflow wraps it as a React Query mutation; useVercelChat conditionally calls workflow stop when sessionId exists (falls back to legacy stop) and exposes isStopping; UI/context/tool UI updated. ChangesChat Workflow Cancellation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 4/5
- This PR is likely safe to merge, with mainly maintainability risk rather than a clear functional breakage.
- The main concern is in
hooks/useVercelChat.ts: adding workflow-cancellation logic inline to an already large (~400-line) hook increases complexity and raises regression risk when modifying chat flow behavior later. - Given the medium severity/confidence signal (6/10, 7/10), this looks like a design/readability issue that should be refactored soon, but it is not clearly merge-blocking on its own.
- Pay close attention to
hooks/useVercelChat.ts- large, mixed-responsibility logic can make cancellation behavior harder to reason about and safely change.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/useVercelChat.ts">
<violation number="1" location="hooks/useVercelChat.ts:207">
P2: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
New workflow-cancellation logic added inline to an already 400-line hook instead of being extracted into a focused unit, violating the 100-line limit and single-responsibility rule.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Calling aiStop() immediately after firing the stop POST tore down the local SSE before the server had finished cancelling — chunks the server kept emitting (and persisting via onStepFinish/onFinish) in that 1–1.5s window never reached the UI, so the assistant message visible at stop time differed from what got persisted. Reload showed more. Pair with the api-side change that holds /stop until the workflow run is terminal: await stopChatWorkflow, then return without aiStop. The server's runAgentWorkflow.finally closes the workflow writable, the SSE drains the remaining chunks to the client, and useChat transitions to "ready" on its own. Frontend at stop-complete now matches the DB. Legacy chats (no sessionId) keep the AI-SDK local abort — there's no backend endpoint to drive their teardown. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
useVercelChat exposes a local isStopping state set immediately on click and cleared in finally. VercelChatProvider threads it through context. ChatInput overrides the submit button to a spinner (status="submitted") and disables re-clicks while the backend round trip is in flight, so the UI no longer looks dead for ~1-2s before SSE closes. SSE close still happens naturally via the backend watcher, preserving frontend == DB on reload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/useVercelChat.ts">
<violation number="1" location="hooks/useVercelChat.ts:218">
P2: Awaiting workflow stop without a timeout can freeze the chat input in a long-lived "stopping" state when the stop request hangs.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| setIsStopping(true); | ||
| try { | ||
| const token = await getAccessToken().catch(() => null); | ||
| await stopChatWorkflow(id, token); |
There was a problem hiding this comment.
P2: Awaiting workflow stop without a timeout can freeze the chat input in a long-lived "stopping" state when the stop request hangs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useVercelChat.ts, line 218:
<comment>Awaiting workflow stop without a timeout can freeze the chat input in a long-lived "stopping" state when the stop request hangs.</comment>
<file context>
@@ -204,10 +204,21 @@ export function useVercelChat({
+ setIsStopping(true);
+ try {
+ const token = await getAccessToken().catch(() => null);
+ await stopChatWorkflow(id, token);
+ } finally {
+ setIsStopping(false);
</file context>
| await stopChatWorkflow(id, token); | |
| await Promise.race([ | |
| stopChatWorkflow(id, token), | |
| new Promise<void>((resolve) => setTimeout(resolve, 5000)), | |
| ]); |
MessageParts only distinguished output-available vs everything-else,
so any tool part in state output-error or output-denied fell through
to getToolCallComponent and painted the spinning skeleton — making a
cancelled tool look like it was still running, both live and on
reload.
Add a getCancelledToolComponent renderer (OctagonX icon + "Cancelled
{toolName}" / "Failed {toolName}" label) and route output-error /
output-denied parts to it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
useVercelChat was managing isStopping state, token fetch, and the backend round-trip inline. Pull it out into a dedicated hook so the stop concern is encapsulated, useVercelChat thins out, and the hook is reusable from any consumer that needs workflow-stop semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
Match the established hook pattern (useDeleteChat, useCreateTask,
usePulseToggle, etc.) and drop the hand-rolled useState. Same public
shape — { stop, isStopping } — so useVercelChat is untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
…-to-backend # Conflicts: # components/VercelChat/ChatInput.tsx # hooks/useVercelChat.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
components/VercelChat/ToolComponents.tsx (1)
441-446: 💤 Low value
getCancelledToolComponentis missing from theToolComponentsaggregate.
MessagePartsimports the new renderer directly as a named export, so nothing is broken today. But theToolComponentsobject advertises itself as the canonical bundle of these renderers, and now it's inconsistent —getCancelledToolComponentis the odd one out. For DRY/discoverability, either add it to the object or skip the comment if you intend the aggregate to be deprecated in favor of named imports.♻️ Optional: keep the aggregate complete
export const ToolComponents = { getToolCallComponent, getToolResultComponent, + getCancelledToolComponent, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/VercelChat/ToolComponents.tsx` around lines 441 - 446, The ToolComponents export is missing getCancelledToolComponent, making the aggregate incomplete; update the export object to include getCancelledToolComponent alongside getToolCallComponent and getToolResultComponent so the aggregate remains canonical (referencing getCancelledToolComponent, getToolCallComponent, getToolResultComponent and the default export ToolComponents) or remove/mark the aggregate as deprecated if you intend to rely only on named exports.hooks/useVercelChat.ts (1)
219-225: 💤 Low valueClean branch — one resilience nit worth a glance.
The
sessionIdsplit reads well and keeps each path single-purpose. One thing to keep an eye on:ChatInputinvokesstop()fire-and-forget (noawait/catch). Today that's safe becausestopChatWorkflowswallows its own errors, so neitherstopWorkflow()nor this callback rejects. But the safety net lives a couple of layers away — if a future refactor lets either path reject, this becomes an unhandled rejection with no UI recovery. A defensivetry/catchhere would make the contract self-contained rather than relying on the helper's internals.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hooks/useVercelChat.ts` around lines 219 - 225, The stop callback currently awaits stopWorkflow() or aiStop() but relies on those helpers to swallow errors; make stop self-defensive by wrapping the await calls in a try/catch so it never propagates a rejection to the caller (e.g., ChatInput's fire-and-forget). Locate the stop function (useCallback) and add a single try/catch around the branch so both branches call await inside the try and any thrown error is caught and handled (log via console or existing logger) to prevent unhandled promise rejections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@components/VercelChat/ToolComponents.tsx`:
- Around line 441-446: The ToolComponents export is missing
getCancelledToolComponent, making the aggregate incomplete; update the export
object to include getCancelledToolComponent alongside getToolCallComponent and
getToolResultComponent so the aggregate remains canonical (referencing
getCancelledToolComponent, getToolCallComponent, getToolResultComponent and the
default export ToolComponents) or remove/mark the aggregate as deprecated if you
intend to rely only on named exports.
In `@hooks/useVercelChat.ts`:
- Around line 219-225: The stop callback currently awaits stopWorkflow() or
aiStop() but relies on those helpers to swallow errors; make stop self-defensive
by wrapping the await calls in a try/catch so it never propagates a rejection to
the caller (e.g., ChatInput's fire-and-forget). Locate the stop function
(useCallback) and add a single try/catch around the branch so both branches call
await inside the try and any thrown error is caught and handled (log via console
or existing logger) to prevent unhandled promise rejections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5d8a3c3a-7b09-4266-b15c-ab3404e42484
📒 Files selected for processing (6)
components/VercelChat/ChatInput.tsxcomponents/VercelChat/MessageParts.tsxcomponents/VercelChat/ToolComponents.tsxhooks/useStopChatWorkflow.tshooks/useVercelChat.tsproviders/VercelChatProvider.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- providers/VercelChatProvider.tsx
- components/VercelChat/ChatInput.tsx
E2E verified on previewChat preview: this PR's head (post test-branch merge) Live UI (no reload)
Reload (DB-sourced render)Same state — "⊘ Cancelled bash" pill rendered from the persisted snapshot. Persisted snapshot (
|
|
Wires the chat stop button to the backend so stopping actually cancels the durable workflow run, not just the local stream.
Previously the stop button only called the AI SDK
stop()(aborts the client fetch). The Vercel Workflow run kept streaming and billing server-side. Nowstop()also firesPOST /api/chat/{chatId}/stop(recoupable/api#590) — fire-and-forget so the UI stops instantly — for workflow chats (sessionIdpresent). Legacy chats are unchanged.Test plan
sessionId): stop still aborts locally, no backend call made.lib/chat/__tests__/stopChatWorkflow.test.ts(bearer header, unauth, url-encoding, never-throws).Depends on recoupable/api#590 being live.
Summary by cubic
Wires the chat Stop button to POST
/api/chat/{chatId}/stopfor workflow chats and waits for the server to cancel, letting SSE drain so the UI matches the DB and avoids extra billing. Legacy chats still abort locally; the UI shows instant stop feedback, and cancelled/denied/errored tool-calls render as stopped.New Features
useStopChatWorkflowon@tanstack/react-query;useVercelChat/VercelChatProviderexposeisStopping.ChatInputdisables re-clicks and forces a spinner (status="submitted") while stopping.Bug Fixes
sessionId) await the stop POST and skipaiStop, letting the server close SSE so the stream drains and UI == DB. AddedstopChatWorkflowwith unit tests (bearer header, unauth, URL-encode, never-throws). Tool-calls inoutput-error/output-deniedrender with a stop icon and “Cancelled/Failed” label instead of a spinner. Depends onrecoupable/api#590.Written for commit 82c88a2. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
New Features