Skip to content

draft: Stop button — open-agents-style (instant aiStop + void POST, no isStopping) [alt to #1770]#1786

Draft
sweetmantech wants to merge 20 commits into
testfrom
feat/stop-button-simplified
Draft

draft: Stop button — open-agents-style (instant aiStop + void POST, no isStopping) [alt to #1770]#1786
sweetmantech wants to merge 20 commits into
testfrom
feat/stop-button-simplified

Conversation

@sweetmantech

@sweetmantech sweetmantech commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Draft / alternative to #1770 — not for parallel merge. Same feature (wire the Stop button to cancel the durable workflow run), implemented the open-agents way. Open this side-by-side with #1770 to compare.

What's different vs #1770

#1770 awaits the backend POST /api/chat/{chatId}/stop and deliberately does not call the AI SDK stop() for workflow chats, so it needs an isStopping pending-state threaded through hook → provider → ChatInput to give button feedback while the stream keeps running ~1–2s.

This version matches open-agents (use-session-chat-runtime.ts):

  • stop() calls the AI SDK aiStop() immediately for all chats → the stream stops on screen instantly.
  • For workflow chats it also void-fires POST /api/chat/{chatId}/stop (best-effort) to cancel the durable run server-side.
  • Removes the entire isStopping path — the useMutation, the provider field, and the ChatInput wiring.

Net −20 lines (4 files): simpler, and the Stop button feels instant instead of lagging ~1–2s.

Why this is safe

Reload-consistency was the stated reason #1770 avoided aiStop(). That's already handled server-side by api#590: it persists the assistant message per step and finalizeAbortedAssistantMessage closes open tool-calls + re-persists on abort. So the DB shows the complete, tool-closed turn on reload regardless of local abort.

Trade-off to decide

  • feat(chat): wire stop button to POST /api/chat/{chatId}/stop #1770: trailing in-flight chunks render in the current view before the stream stops (no reload needed); costs the isStopping machinery + a ~1–2s visible lag.
  • This PR: instant on-screen stop; a couple of trailing chunks may not appear in the current view but are persisted (visible on reload).

If "trailing chunks must show without reload" isn't required, this is the better default.

Tests

  • stopChatWorkflow lib test passes; ESLint clean; no isStopping references remain.

🤖 Generated with Claude Code


Summary by cubic

Make the Stop button instant and cancel workflow runs server-side. We now call aiStop() immediately and fire a best-effort POST /api/chat/{chatId}/stop; removed isStopping and improved cancelled tool UI.

  • New Features

    • Stop now calls aiStop() for all chats and asynchronously POSTs /api/chat/{chatId}/stop for workflow chats (fire-and-forget).
    • Added useStopChatWorkflow and stopChatWorkflow; useVercelChat now uses them.
  • Bug Fixes

    • Cancelled or denied tool-calls render with a clear badge instead of a spinner.
    • Tests added for stopChatWorkflow (auth header, URL encoding, no-throw behavior).

Written for commit f0b3d7a. Summary will update on new commits.

Review in cubic

arpitgupta1214 and others added 20 commits June 2, 2026 22:52
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>
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>
#1765)

* refactor(chat): update chat components to use chatId instead of roomId

This commit removes the InstantChatRoom component and updates various hooks and components to replace references of roomId with chatId. The changes ensure that chat functionality is aligned with the new session-scoped architecture, improving consistency across the chat system. Additionally, the sessionId is now required in several components to streamline the chat transport process.

* refactor(chat): integrate sessionId and update chat path utilities

This commit enhances the chat components by introducing the `sessionId` in various hooks and updating the URL handling to utilize new utility functions for generating chat paths. The changes improve the consistency and maintainability of chat navigation, ensuring that the application correctly reflects the session-scoped architecture.

* fix(chat): ensure newRoomId is treated as a string in URL handling

This commit updates the `useCreateArtistTool` hook to explicitly cast `result.newRoomId` to a string when generating the chat path. This change prevents potential type-related issues and ensures consistent URL formatting. Additionally, it adds an import statement for testing utilities in the chat paths test file, enhancing test coverage and maintainability.

* refactor(Header, OrganizationProvider): remove usePathname and replace with window.location.pathname

This commit removes the usePathname hook from both Artist.tsx and OrganizationProvider.tsx, replacing it with direct access to window.location.pathname. This change simplifies the code and maintains the intended navigation behavior without relying on Next.js's router for pathname management.

* refactor(chat): split chatPaths per SRP, drop dead legacy branch, simplify useParams

- Remove dead `/chat/` branch from isActiveChatRoomPath — this PR makes
  /chat/{id} 404, so treating it as an active chat room contradicted the
  PR's own intent and could never fire in production.
- Split lib/chat/chatPaths.ts into one-function-per-file (getChatPath,
  getChatUrl, isActiveChatRoomPath) per repo SRP convention; mirror tests.
- Simplify useVercelChat to `const { chatId } = useParams<{ chatId?: string }>()`
  (KISS), consistent with chat.tsx.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#1767) (#1774)

* feat(chat): defer new-chat bootstrap wait from spinner to send button (#1767)

New chats landing on `/` or `/chat` blocked the whole view on a
full-screen spinner while `POST /api/sessions` + `POST /api/sandbox`
resolved (~12s cold, dominated by sandbox provisioning). Render `<Chat>`
immediately with a typeable input instead and provision in parallel; the
Send button stays disabled with a "Preparing your workspace…" cue until
the api-minted `sessionId` + `chatId` land, so the workflow transport
never fires without the ids its validator requires.

- NewChatBootstrap: render <Chat> eagerly with a stable placeholder chat
  id (survives the preparing → ready transition); drop the spinner.
- Thread `sessionId?`, `workflowChatId`, `isBootstrapPreparing` through
  Chat → VercelChatProvider → useVercelChat (provider kept inline).
- useVercelChat: `transportChatId = workflowChatId ?? id` drives the
  transport, optimistic conversation, and URL; only load persisted
  history on a canonical route so no spinner flashes over the input
  during the ready transition.
- ChatInput: gate Send on `isBootstrapPreparing`; input stays typeable.
- useChatTransport / useMessageLoader: sessionId optional + guard.
- useCreateArtistTool: guard the one context sessionId consumer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(chat): send live sessionId/chatId from workflow transport (#1767)

The new-chat flow mounts <Chat> (and useChat) during provisioning with
sessionId undefined + a placeholder chatId. useChat captures the transport
from that mount render and does not swap to the rebuilt instance when the
ids later resolve — so the first send POSTed `sessionId: undefined` to
/api/chat/workflow and got a 400 (the URL rewrote correctly because it
reads the live prop, not the transport).

Read the ids from refs inside transport.body() so a single stable
transport instance always sends the values current as of the request,
regardless of useChat's stale capture. Removed chatId/sessionId from the
memo deps since rebuilding the instance never helped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(chat): replace preparing-text cue with a workspace status dot (#1767)

Swap the flashing "Preparing your workspace…" toolbar text for a
stoplight status dot in the top-right of the chat input: red = off
(provisioning failed / unavailable), yellow = provisioning, green =
ready. Hover shows context via the shadcn tooltip.

- New `WorkspaceStatusIndicator` component in its own file (SRP) — a pure
  display component driven by a `status` prop, built on the existing
  shadcn-based `common/Tooltip`, so it's open for reuse without change.
- Thread `workspaceStatus: "off" | "provisioning" | "ready"` through
  Chat → VercelChatProvider → context (replacing the `isBootstrapPreparing`
  boolean); Send is gated while it's not `ready`.
- NewChatBootstrap now derives the status and renders `<Chat>` even on a
  provisioning error (red dot + disabled Send, input still visible)
  instead of swapping in a full-screen error view.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(chat): simplify provisioning tooltip copy (#1767)

Shorten the yellow/provisioning workspace-status tooltip to
"Preparing your workspace".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
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>
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>
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>
…-to-backend

# Conflicts:
#	components/VercelChat/ChatInput.tsx
#	hooks/useVercelChat.ts
* fix(chat): drop legacy GET /api/chats/{id}/artist caller (#1767)

* fix(chat): reactive conversations cache + user-scoped session query

* refactor(chat): drop dead /chat/:id artist cache fallback (post-#1765)

The conversations-cache fallback in useArtistFromChat only existed for the
legacy /chat/:id deep link (chatId but no sessionId). That route was
removed in #1765 — every chat is now reached via the canonical
/sessions/{sessionId}/chats/{chatId} URL, so sessionId is always present.

Reduce to the session path: GET /api/sessions/{sessionId} -> session.artistId
-> select. Removes useSyncExternalStore reactive-cache machinery,
findArtistIdInConversationsCache (+ its test), and the unused chatId param.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(chat): validate getSessionById response with zod (PR #1768 review)

Replace type-casting with boundary zod validation, matching the
getConversations convention. Drops the `as GetSessionByIdResponse`/
`as GetSessionByIdErrorResponse` casts in favor of safeParse/parse.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(chat): extract shared safeJsonParse into lib/api (PR #1768 review)

Replace the locally-defined safeJsonParse in getSessionById with a
shared helper in lib/api/, alongside the other HTTP helpers. DRY/SRP.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#1781)

* fix(chat): persist selected model before send so the workflow bills it

The chat-workflow path bills the model it reads from chats.model_id at
request time. The picker selection was never written there, so every new
chat (and any model change) billed the chats.model_id default instead of
the chosen model.

Before sending, await a PATCH of the selected model to
chats.model_id when it changed for the active chat (shouldPersistChatModel
guards redundant writes). The chat row already exists from new-chat
bootstrap, so this covers the first turn race-free without an api change.

Reuses buildPatchChatBody + updateChat(modelId) (originally from #1779).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(chat): extract model-persist-before-send into usePersistSelectedModel

Addresses OCP/SRP review feedback on #1781: instead of inlining the
persist-before-send block in useVercelChat, encapsulate it in a dedicated
usePersistSelectedModel hook (owns the lastPersisted ref + guard + PATCH).
useVercelChat now just calls `await persistSelectedModel()` before send.

Behavior unchanged; pure decision logic still covered by
shouldPersistChatModel tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(chat): persist selected model on retry/edit + append paths

Addresses review feedback on #1781:
- Retry/Edit call reload() → regenerate (and append() calls sendMessage)
  bypassing handleSubmit, so the selected model was not persisted before
  those sends — the regenerated turn could bill the previous/default model.
  Now await persistSelectedModel() in handleReload and append too.
- Clarify updateChat JSDoc: it patches title and/or modelId (not just rename).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Step 3 of the /api/chat/workflow → /api/chat rename (chat#1767), against
the merged docs contract (docs#235) and the api endpoint (api#645, on test).

useChatTransport now POSTs to ${baseUrl}/api/chat instead of
/api/chat/workflow. Comment-only refs in VercelChatProvider updated too.

⚠️ Deploy coordination: requires api#645 (which removes /api/chat/workflow)
to be live in the same environment. Must deploy together with the api
promotion and the open-agents repoint (step 4), or chat 404s.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, drop isStopping)

DRAFT for comparison vs #1770. Stop now aborts locally instantly (aiStop) for
all chats and fire-and-forgets POST /api/chat/{chatId}/stop for workflow chats;
removes the isStopping mutation/pending-state threaded through hook→provider→ChatInput.
Reload-correctness is covered server-side by api#590.
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9aa29c6c-3f4a-40c2-88b8-c0390e1531be

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/stop-button-simplified

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@vercel

vercel Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
chat Ready Ready Preview Jun 8, 2026 5:24pm

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants