Uh oh!
There was an error while loading. Please reload this page.
feat(web): add cross-provider model handoff with structured context - #7101
feat(web): add cross-provider model handoff with structured context#7101kbrianps wants to merge 1 commit into
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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.
UI consistency review found three issues in the new ThreadHandoffDialog, all in apps/web/src/components/chat/ThreadHandoffDialog.tsx: a non-existent Button variant, a hand-rolled dialog header that bypasses the Dialog* primitives (and their aria wiring/spacing contract), and a textarea label that is not associated with its control plus a no-op resize-y on the Textarea wrapper.
Posted via Macroscope — UI Consistency
| <div className="flex flex-col space-y-1.5 p-6 pb-0"> | ||
| <h2 className="text-lg font-semibold leading-none tracking-tight"> | ||
| Continue in new thread | ||
| </h2> | ||
| <p className="text-sm text-muted-foreground"> | ||
| Switch to a different model with a structured handoff of your current progress. The | ||
| original thread remains untouched. | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
The header is hand-rolled instead of using the shared DialogHeader / DialogTitle / DialogDescription primitives. Two concrete consequences: Base UI only wires aria-labelledby / aria-describedby on the popup when Dialog.Title / Dialog.Description are rendered, so this dialog ships with no accessible name; and without a data-slot="dialog-header" element the DialogPanel spacing contract (in-[[data-slot=dialog-popup]:has([data-slot=dialog-header])]:pt-1) never applies, which is why the call site has to re-specify p-6 pb-0 and px-6 py-4. The title typography also diverges from DialogTitle (text-lg vs font-heading text-xl).
Suggested fix: import DialogHeader, DialogTitle, DialogDescription from ../ui/dialog and drop the manual padding overrides on the header and DialogPanel. Note Dialog.Title requires a dialog root, so ThreadHandoffDialog.test.tsx should render ThreadHandoffContent inside <Dialog open> / DialogPopup.
- <div className="flex flex-col space-y-1.5 p-6 pb-0">- <h2 className="text-lg font-semibold leading-none tracking-tight">- Continue in new thread- </h2>- <p className="text-sm text-muted-foreground">- Switch to a different model with a structured handoff of your current progress. The- original thread remains untouched.- </p>- </div>+ <DialogHeader>+ <DialogTitle>Continue in new thread</DialogTitle>+ <DialogDescription>+ Switch to a different model with a structured handoff of your current progress. The+ original thread remains untouched.+ </DialogDescription>+ </DialogHeader>Posted via Macroscope — UI Consistency
| Cancel | ||
| </Button> | ||
| <Button | ||
| variant="primary" |
There was a problem hiding this comment.
Button has no primary variant (default | destructive | destructive-outline | ghost | ghost-muted | glass | link | outline | secondary), so this fails typecheck and, at runtime, cva applies no variant classes — the confirm CTA renders with only the base classes (no background, no primary border/shadow) next to the outlined Cancel button. The filled primary style is variant="default".
| variant="primary" | |
| variant="default" |
Posted via Macroscope — UI Consistency
| <div className="space-y-1.5"> | ||
| <label className="text-xs font-medium text-muted-foreground"> | ||
| Handoff context (editable) | ||
| </label> | ||
| <Textarea | ||
| value={handoffText} | ||
| onChange={(e) => setHandoffText(e.target.value)} | ||
| rows={12} | ||
| className="font-mono text-xs leading-relaxed resize-y" | ||
| placeholder="Structured context to be sent to the new model..." | ||
| /> |
There was a problem hiding this comment.
Two issues here. The raw <label> has no htmlFor and the Textarea has no id, so clicking the label does not focus the control and the textarea has no accessible name; elsewhere the repo pairs the Label primitive with an id (e.g. projectScriptEditor.tsx). Also resize-y lands on the Textarea wrapper <span>, not the inner textarea, so it has no effect — the inner element is field-sizing-content (which also makes rows={12} non-authoritative); target it via [&_[data-slot=textarea]]: as DiffCommentAnnotation.tsx does.
Suggested fix (also import Label from ../ui/label):
- <label className="text-xs font-medium text-muted-foreground">- Handoff context (editable)- </label>+ <Label htmlFor="thread-handoff-context" className="text-xs text-muted-foreground">+ Handoff context (editable)+ </Label>
<Textarea
+ id="thread-handoff-context"
value={handoffText}
onChange={(e) => setHandoffText(e.target.value)}
- rows={12}- className="font-mono text-xs leading-relaxed resize-y"+ className="font-mono text-xs leading-relaxed [&_[data-slot=textarea]]:max-h-80 [&_[data-slot=textarea]]:resize-y"Posted via Macroscope — UI Consistency
| ); | ||
| }, [providerInstanceEntries, targetModelSelection]); | ||
| useEffect(() => { |
There was a problem hiding this comment.
🟡 Mediumchat/ThreadHandoffDialog.tsx:52
Routine sourceThread refreshes overwrite the user's edited handoffText while the dialog is open. The effect depends on the entire sourceThread object, so any store update that replaces it regenerates the markdown and calls setHandoffText; only reset for a newly opened handoff or changed thread/target, while preserving dirty input.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ThreadHandoffDialog.tsx around line 52:
Routine `sourceThread` refreshes overwrite the user's edited `handoffText` while the dialog is open. The effect depends on the entire `sourceThread` object, so any store update that replaces it regenerates the markdown and calls `setHandoffText`; only reset for a newly opened handoff or changed thread/target, while preserving dirty input.
| setHandoffText(generated); | ||
| }, [sourceThread, targetModelSelection]); | ||
| const handleConfirm = useCallback(() => { |
There was a problem hiding this comment.
🟡 Mediumchat/ThreadHandoffDialog.tsx:60
A rapid double activation of handleConfirm invokes onConfirmHandoff twice before isSubmitting disables the button, so the caller can create two fork threads. Add an in-flight ref or synchronous state check inside handleConfirm; relying on the next render's disabled value does not prevent duplicate submissions.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ThreadHandoffDialog.tsx around line 60:
A rapid double activation of `handleConfirm` invokes `onConfirmHandoff` twice before `isSubmitting` disables the button, so the caller can create two fork threads. Add an in-flight ref or synchronous state check inside `handleConfirm`; relying on the next render's `disabled` value does not prevent duplicate submissions.
| failure = startResult._tag === "Failure" ? startResult : null; | ||
| } | ||
| if (failure === null) { |
There was a problem hiding this comment.
🟠 Highcomponents/ChatView.tsx:5935
A handoff whose new thread never starts is still navigated to and reported as successful. waitForStartedServerThread resolves false on timeout, but this code treats every Success as success; handle the false value as a failure before navigation.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatView.tsx around line 5935:
A handoff whose new thread never starts is still navigated to and reported as successful. `waitForStartedServerThread` resolves `false` on timeout, but this code treats every `Success` as success; handle the `false` value as a failure before navigation.
| keybindings: providedKeybindings, | ||
| modelOptionsByInstance, | ||
| instanceEntries, | ||
| allowHandoff = true, |
There was a problem hiding this comment.
🟡 Mediumchat/ModelPickerContent.tsx:101
Locked pickers default to showing an enabled cross-provider Handoff row, but selecting that row can still be rejected by handleModelSelect when getModelDisabledReason reports that handoff is unsupported, so the UI presents an action that silently does nothing. Default allowHandoff to false and require callers to opt in only when their selection handler supports handoff, keeping the row filtering and handler decision consistent.
| allowHandoff=true, | |
| allowHandoff=false, |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ModelPickerContent.tsx around line 101:
Locked pickers default to showing an enabled cross-provider `Handoff` row, but selecting that row can still be rejected by `handleModelSelect` when `getModelDisabledReason` reports that handoff is unsupported, so the UI presents an action that silently does nothing. Default `allowHandoff` to `false` and require callers to opt in only when their selection handler supports handoff, keeping the row filtering and handler decision consistent.
| // ignoring sidebar selection so account-scoped searches can find a | ||
| // model before the user chooses a specific instance rail item. | ||
| if (props.lockedProvider !== null) { | ||
| if (!allowHandoff && props.lockedProvider !== null) { |
There was a problem hiding this comment.
🟡 Mediumchat/ModelPickerContent.tsx:319
filteredModels can use a stale model list when allowHandoff changes, leaving cross-provider models visible after handoffs are disabled or filtering them after handoffs are enabled. Because allowHandoff is read in the memo but omitted from its dependency array, React does not recompute until another dependency changes. Add allowHandoff to the dependency array.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ModelPickerContent.tsx around line 319:
`filteredModels` can use a stale model list when `allowHandoff` changes, leaving cross-provider models visible after handoffs are disabled or filtering them after handoffs are enabled. Because `allowHandoff` is read in the memo but omitted from its dependency array, React does not recompute until another dependency changes. Add `allowHandoff` to the dependency array.
| `> Source thread: "${thread.title}" (ID: \`${thread.id}\`)`, | ||
| ); | ||
| const messages = (thread.messages ?? []).filter((m) => m.text.trim().length > 0); |
There was a problem hiding this comment.
🟠 Highhandoff/threadHandoff.ts:56
Requests conveyed through message attachments are dropped, so a message with blank text produces no original goal or latest request and the new provider receives no user-supplied task context. The filter only retains non-empty message.text, and the handoff never reads attachments; include the attachment data (and preserve messages whose task is conveyed only by attachments) when building the handoff.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/handoff/threadHandoff.ts around line 56:
Requests conveyed through message `attachments` are dropped, so a message with blank `text` produces no original goal or latest request and the new provider receives no user-supplied task context. The filter only retains non-empty `message.text`, and the handoff never reads attachments; include the attachment data (and preserve messages whose task is conveyed only by attachments) when building the handoff.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e55a6cc. Configure here.
| onOpenChange(false); | ||
| } finally { | ||
| setIsSubmitting(false); | ||
| } |
There was a problem hiding this comment.
Dialog closes after failed handoff
High Severity
ThreadHandoffDialog always calls onOpenChange(false) after await onConfirmHandoff(...), but handleConfirmHandoff swallows failures with a toast and returns normally. Failed creates/starts therefore dismiss the dialog and discard the user’s edited handoff text, unlike PullRequestThreadDialog which only closes on success.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e55a6cc. Configure here.
| targetModelSelection, | ||
| }); | ||
| setHandoffText(generated); | ||
| }, [sourceThread, targetModelSelection]); |
There was a problem hiding this comment.
Handoff edits wiped by updates
High Severity
ThreadHandoffContent regenerates and replaces handoffText whenever sourceThread changes. activeServerThread from useThread gets a new object on shell/detail updates (streaming, session, settle, title), so live thread churn overwrites user edits in the textarea while the dialog is open.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e55a6cc. Configure here.
| onCancel={() => onOpenChange(false)} | ||
| onConfirmHandoff={handleConfirm} | ||
| /> | ||
| </DialogPopup> |
There was a problem hiding this comment.
Dialog dismissible while submitting
Medium Severity
While isSubmitting is true, Cancel is disabled but Dialog still forwards Escape, backdrop, and the close button through onOpenChange. The handoff create/start continues in the background after dismiss, so a later failure only surfaces as a toast with the edited context already gone.
Reviewed by Cursor Bugbot for commit e55a6cc. Configure here.
ApprovabilityVerdict: Needs human review 6 blocking correctness issues found. This PR introduces a significant new feature (cross-provider model handoff) with new UI components, thread creation workflows, and user-facing behavior. Multiple High and Medium severity findings remain unresolved, including issues with timeout handling, attachment preservation, and dialog state management. You can customize Macroscope's approvability policy. Learn more. |
closing this because it will be included with orchestrator v2. |
Summary
This PR implements cross-provider model switching via structured thread handoffs.
Problem
Previously, once a thread started, switching to a different provider or incompatible driver (e.g. Claude Code ↔ Codex, Antigravity, etc.) was blocked because runtime session cursors and subprocess lifecycles are mutually incompatible across providers. Removing the lock directly was unsafe and could cause empty sessions or out-of-sync transcripts.
Solution
ThreadHandoffDialog) instead of being blocked.buildThreadHandoffMarkdownin@t3tools/client-runtime/handoffto compile canonical goal, decisions, modified files, terminal commands, active plans, and latest requests without relying on external LLM summarization.Verification
@t3tools/client-runtimefor Claude ↔ Codex transitions and offline source threads.ThreadHandoffDialogin@t3tools/web.vp fmt,vp lint, and test suites.Note
Medium Risk
Touches thread create/start/navigation and model-lock behavior on live conversations; failures are partially mitigated by cleanup and toasts, but a bad handoff could still start the wrong model or lose context if synthesis is incomplete.
Overview
Cross-provider model switching on active server threads no longer hard-blocks incompatible picks in the composer. Choosing a different driver/continuation group—or a model change that would normally be blocked on a started thread—opens
ThreadHandoffDialogso the user can review and edit context, then continue in a new thread; the original thread is left unchanged.buildThreadHandoffMarkdownin@t3tools/client-runtime/handoffdeterministically assembles handoff markdown from the source thread (messages, activities, checkpoints, plans, etc.). Confirming handoff creates a titled fork (… (Handoff)), starts the first turn with the markdown as the user message, waits for the thread to start, navigates to it, and deletes the new thread if anything fails.The model picker gains an optional
allowHandoffmode: cross-provider rows show a Handoff badge and stay selectable instead of being filtered/disabled when the provider is locked. Unit and component tests cover markdown generation and dialog rendering.Reviewed by Cursor Bugbot for commit e55a6cc. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add cross-provider model handoff with structured context in chat threads
buildThreadHandoffMarkdowninpackages/client-runtime/src/handoff/threadHandoff.ts) that captures thread context such as the original goal, decisions, modified files, and latest request.allowHandoffis true.📊 Macroscope summarized e55a6cc. 7 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.