Refactor chat into a composable workspace shell#2011
Refactor chat into a composable workspace shell#2011juliusmarminge wants to merge 1 commit intomainfrom
Conversation
- Add workspace document storage and store tests - Route terminal and command-palette actions through workspace surfaces - Update chat layout, sidebar, and thread terminal handling
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
vouch:trusted
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Unused state variable
nowTickadded to ChatView- Removed the unused
nowTickandsetNowTickstate declaration from ChatView.tsx as it was never referenced anywhere in the component.
- Removed the unused
- ✅ Fixed:
WorkspaceRouteSynccomponent exported but never used- Deleted the entire WorkspaceRouteSync.tsx file since the component was never imported or rendered anywhere in the codebase.
- ✅ Fixed: Unused
terminalFocusRequestIdstate after refactor- Removed the unused
terminalFocusRequestIdandsetTerminalFocusRequestIdstate declaration from ChatView.tsx as all consumers were removed in a prior refactor.
- Removed the unused
- ✅ Fixed: Sidebar thread context menu navigates after split open
- Removed the redundant
router.navigate()calls afteropenThreadInSplit/openThreadInNewTabin the sidebar context menu handlers, which were causing the route component'suseEffectto callopenThreadSurfacewithfocus-or-tabdisposition and potentially refocus the surface into the wrong pane.
- Removed the redundant
Or push these changes by commenting:
@cursor push dc4ee0b932
Preview (dc4ee0b932)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -498,8 +498,6 @@
// When set, the thread-change reset effect will open the sidebar instead of closing it.
// Used by "Implement in a new thread" to carry the sidebar-open intent across navigation.
const planSidebarOpenOnNextThreadRef = useRef(false);
- const [terminalFocusRequestId, setTerminalFocusRequestId] = useState(0);
- const [nowTick, setNowTick] = useState(() => Date.now());
const [pullRequestDialogState, setPullRequestDialogState] =
useState<PullRequestDialogState | null>(null);
const [attachmentPreviewHandoffByMessageId, setAttachmentPreviewHandoffByMessageId] = useState<
diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -1654,28 +1654,16 @@
if (clicked === "open-new-tab") {
openThreadInNewTab(serverThreadSurfaceInput(threadRef));
- void router.navigate({
- to: "/$environmentId/$threadId",
- params: buildThreadRouteParams(threadRef),
- });
return;
}
if (clicked === "open-split-right") {
openThreadInSplit(serverThreadSurfaceInput(threadRef), "x");
- void router.navigate({
- to: "/$environmentId/$threadId",
- params: buildThreadRouteParams(threadRef),
- });
return;
}
if (clicked === "open-split-down") {
openThreadInSplit(serverThreadSurfaceInput(threadRef), "y");
- void router.navigate({
- to: "/$environmentId/$threadId",
- params: buildThreadRouteParams(threadRef),
- });
return;
}
@@ -1729,7 +1717,6 @@
openThreadInNewTab,
openThreadInSplit,
project.cwd,
- router,
],
);
diff --git a/apps/web/src/components/workspace/WorkspaceRouteSync.tsx b/apps/web/src/components/workspace/WorkspaceRouteSync.tsx
deleted file mode 100644
--- a/apps/web/src/components/workspace/WorkspaceRouteSync.tsx
+++ /dev/null
@@ -1,130 +1,0 @@
-import { useLocation, useNavigate, useParams } from "@tanstack/react-router";
-import { useEffect, useMemo, useRef } from "react";
-
-import { useComposerDraftStore } from "../../composerDraftStore";
-import { resolveThreadRouteTarget } from "../../threadRoutes";
-import { useFocusedWorkspaceRouteTarget, useWorkspaceStore } from "../../workspace/store";
-import type { ThreadSurfaceInput } from "../../workspace/types";
-
-function sameRouteTarget(
- left: ReturnType<typeof resolveThreadRouteTarget>,
- right: ReturnType<typeof resolveThreadRouteTarget>,
-): boolean {
- if (!left && !right) {
- return true;
- }
- if (!left || !right || left.kind !== right.kind) {
- return false;
- }
- if (left.kind === "server" && right.kind === "server") {
- return (
- left.threadRef.environmentId === right.threadRef.environmentId &&
- left.threadRef.threadId === right.threadRef.threadId
- );
- }
- if (left.kind === "draft" && right.kind === "draft") {
- return left.draftId === right.draftId;
- }
- return false;
-}
-
-export function WorkspaceRouteSync() {
- const navigate = useNavigate();
- const openThreadSurface = useWorkspaceStore((state) => state.openThreadSurface);
- const currentRouteTarget = useParams({
- strict: false,
- select: (params) => resolveThreadRouteTarget(params),
- });
- const focusedRouteTarget = useFocusedWorkspaceRouteTarget();
- const pathname = useLocation({
- select: (location) => location.pathname,
- });
- const previousPathnameRef = useRef(pathname);
- const draftSession = useComposerDraftStore((store) =>
- currentRouteTarget?.kind === "draft" ? store.getDraftSession(currentRouteTarget.draftId) : null,
- );
- const currentRouteSurfaceInput = useMemo<ThreadSurfaceInput | null>(() => {
- if (!currentRouteTarget) {
- return null;
- }
- if (currentRouteTarget.kind === "server") {
- return {
- scope: "server",
- threadRef: currentRouteTarget.threadRef,
- };
- }
- if (!draftSession) {
- return null;
- }
- return {
- scope: "draft",
- draftId: currentRouteTarget.draftId,
- environmentId: draftSession.environmentId,
- threadId: draftSession.threadId,
- };
- }, [currentRouteTarget, draftSession]);
-
- useEffect(() => {
- const pathnameChanged = previousPathnameRef.current !== pathname;
- previousPathnameRef.current = pathname;
-
- if (currentRouteTarget) {
- if (!currentRouteSurfaceInput) {
- return;
- }
-
- if (!focusedRouteTarget || pathnameChanged) {
- openThreadSurface(currentRouteSurfaceInput, "focus-or-tab");
- return;
- }
-
- if (sameRouteTarget(currentRouteTarget, focusedRouteTarget)) {
- return;
- }
-
- void navigateToRouteTarget(navigate, focusedRouteTarget);
- return;
- }
-
- if (!focusedRouteTarget) {
- return;
- }
-
- void navigateToRouteTarget(navigate, focusedRouteTarget);
- }, [
- currentRouteSurfaceInput,
- currentRouteTarget,
- focusedRouteTarget,
- navigate,
- openThreadSurface,
- pathname,
- ]);
-
- return null;
-}
-
-function navigateToRouteTarget(
- navigate: ReturnType<typeof useNavigate>,
- target: NonNullable<ReturnType<typeof resolveThreadRouteTarget>>,
-) {
- if (target.kind === "server") {
- return navigate({
- to: "/$environmentId/$threadId",
- params: {
- environmentId: target.threadRef.environmentId,
- threadId: target.threadRef.threadId,
- },
- replace: true,
- search: {},
- });
- }
-
- return navigate({
- to: "/draft/$draftId",
- params: {
- draftId: target.draftId,
- },
- replace: true,
- search: {},
- });
-}
\ No newline at end of fileYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit d3d096c. Configure here.
| // Used by "Implement in a new thread" to carry the sidebar-open intent across navigation. | ||
| const planSidebarOpenOnNextThreadRef = useRef(false); | ||
| const [terminalFocusRequestId, setTerminalFocusRequestId] = useState(0); | ||
| const [nowTick, setNowTick] = useState(() => Date.now()); |
There was a problem hiding this comment.
Unused state variable nowTick added to ChatView
Low Severity
The state variable nowTick and its setter setNowTick are declared but never read or called anywhere in the component. This appears to be accidentally committed scaffolding for a feature that was never wired up, adding dead code to an already large component.
Reviewed by Cursor Bugbot for commit d3d096c. Configure here.
| replace: true, | ||
| search: {}, | ||
| }); | ||
| } |
There was a problem hiding this comment.
WorkspaceRouteSync component exported but never used
Low Severity
WorkspaceRouteSync is a fully implemented component (130 lines) that is exported but never imported or rendered anywhere in the codebase. The route-to-workspace synchronization it would perform is instead duplicated across WorkspaceRouteFallback in WorkspaceShell.tsx and the individual route view components. This is dead code that adds maintenance burden.
Reviewed by Cursor Bugbot for commit d3d096c. Configure here.
|
|
||
| const activateTerminal = useCallback( | ||
| (terminalId: string) => { | ||
| storeSetActiveTerminal(threadRef, terminalId); |
There was a problem hiding this comment.
Unused terminalFocusRequestId state after refactor
Low Severity
terminalFocusRequestId state and its setter setTerminalFocusRequestId are still declared, but all setTerminalFocusRequestId calls were removed in this diff along with the PersistentThreadTerminalDrawer that consumed terminalFocusRequestId. Both the state value and setter are now dead code.
Reviewed by Cursor Bugbot for commit d3d096c. Configure here.
| params: buildThreadRouteParams(threadRef), | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Sidebar thread context menu navigates after split open
Medium Severity
The "Open in split right/down" and "Open in new tab" context menu handlers call openThreadInSplit/openThreadInNewTab to create a new workspace pane, then immediately navigate the route to that thread. The navigation triggers the route view's useEffect which calls openThreadSurface with "focus-or-tab" disposition, potentially refocusing the surface into the original pane instead of the newly created split, undermining the user's intent to keep both panes visible.
Reviewed by Cursor Bugbot for commit d3d096c. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟢 Low
t3code/apps/web/src/components/ChatView.tsx
Line 1290 in d3d096c
The focusComposer callback at line 1290 captures composerRef in a closure with an empty dependency array []. Since composerRef is a local variable (not a React ref itself) that is conditionally assigned based on bindSharedComposerHandle and sharedComposerRef, if sharedComposerRef transitions from null to a valid ref after the initial render, focusComposer (and scheduleComposerFocus which depends on it) will permanently call .focusAtEnd() on the stale initial ref (likely localComposerRef) instead of the now-active sharedComposerRef. This means focus requests from many callbacks (e.g., handleRuntimeModeChange, onProviderModelSelect, onEnvModeChange, terminal close) would silently fail to focus the correct composer.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatView.tsx around line 1290:
The `focusComposer` callback at line 1290 captures `composerRef` in a closure with an empty dependency array `[]`. Since `composerRef` is a local variable (not a React ref itself) that is conditionally assigned based on `bindSharedComposerHandle` and `sharedComposerRef`, if `sharedComposerRef` transitions from `null` to a valid ref after the initial render, `focusComposer` (and `scheduleComposerFocus` which depends on it) will permanently call `.focusAtEnd()` on the stale initial ref (likely `localComposerRef`) instead of the now-active `sharedComposerRef`. This means focus requests from many callbacks (e.g., `handleRuntimeModeChange`, `onProviderModelSelect`, `onEnvModeChange`, terminal close) would silently fail to focus the correct composer.
Evidence trail:
apps/web/src/components/ChatView.tsx lines 471-475 (composerRef definition), lines 1291-1293 (focusComposer with empty deps), apps/web/src/composerHandleContext.ts (shows useComposerHandleContext returns ComposerHandleRef | null). Commit: REVIEWED_COMMIT
| if (!activeThreadRef) { | ||
| return; | ||
| } | ||
| storeSetTerminalLaunchContext(activeThreadRef, { |
There was a problem hiding this comment.
🟢 Low components/ChatView.tsx:1390
When a draft thread's environment is changed via onEnvironmentChange, routeThreadRef (using route environmentId) diverges from activeThreadRef (using draftThread.environmentId). The terminal launch context is written and cleared using activeThreadRef (lines 1390, 1851, 1868) but read using routeThreadRef at line 531. This causes the clearing logic to target a different store key than the reading logic, leaving the launch context orphaned and never visible to the component that needs it.
- storeSetTerminalLaunchContext(activeThreadRef, {
+ storeSetTerminalLaunchContext(routeThreadRef, {🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/ChatView.tsx around line 1390:
When a draft thread's environment is changed via `onEnvironmentChange`, `routeThreadRef` (using route `environmentId`) diverges from `activeThreadRef` (using `draftThread.environmentId`). The terminal launch context is written and cleared using `activeThreadRef` (lines 1390, 1851, 1868) but read using `routeThreadRef` at line 531. This causes the clearing logic to target a different store key than the reading logic, leaving the launch context orphaned and never visible to the component that needs it.
Evidence trail:
- apps/web/src/components/ChatView.tsx lines 403-406: routeThreadRef definition uses route environmentId
- apps/web/src/components/ChatView.tsx lines 572-575: activeThreadRef definition uses activeThread.environmentId
- apps/web/src/components/ChatView.tsx line 564: activeThread = localDraftThread for draft threads
- apps/web/src/components/ChatView.tsx lines 1239-1250: onEnvironmentChange calls setDraftThreadContext but does not navigate
- apps/web/src/composerDraftStore.ts lines 1919-1963: setDraftThreadContext updates draftThread.environmentId
- apps/web/src/components/ChatView.tsx line 1390: storeSetTerminalLaunchContext(activeThreadRef, ...)
- apps/web/src/components/ChatView.tsx line 532: reads from scopedThreadKey(routeThreadRef)
- apps/web/src/components/ChatView.tsx lines 1851-1853, 1868-1870: storeClearTerminalLaunchContext(activeThreadRef)
- apps/web/src/terminalStateStore.ts lines 264-266: terminalThreadKey calls scopedThreadKey
- packages/client-runtime/src/scoped.ts lines 20-22: scopedRefKey produces `{environmentId}:{localId}` keys
ApprovabilityVerdict: Needs human review This PR introduces a new composable workspace shell with split-pane management, terminal surfaces, and multi-window functionality — a substantial new feature rather than a simple refactor. The ~2000+ lines of new logic, new user-facing workflows, localStorage persistence, and a medium-severity open comment about sidebar context menu behavior conflicting with route sync warrant careful human review. You can customize Macroscope's approvability policy. Learn more. |
1 participant
CleanShot.2026-04-13.at.19.42.25.mp4
CleanShot.2026-04-13.at.21.41.01.mp4
Summary
Testing
bun fmt,bun lint,bun typecheck, andbun run testbefore merge.Note
Medium Risk
Moderate risk: re-routes core chat/terminal UI through a new workspace layout/store and adds new keybinding/command-palette behaviors, which could affect navigation, focus, and terminal lifecycle across the app.
Overview
Refactors chat rendering to run inside a new workspace shell that manages a split-pane/tabbed layout of thread and terminal surfaces, including pane resizing, focusing, tab close/split controls, and syncing focused surface to the URL.
Moves terminal visibility from per-thread “drawer open” state to workspace terminal surfaces, updates
ChatViewkeyboard handling and toolbar labeling accordingly, and adds a dedicatedThreadTerminalSurfacethat embedsThreadTerminalDrawerwithout resize handling.Expands command palette + keybindings with a set of
workspace.*commands (pane split/close, focus/move panes, move tabs, open terminal in split/tab), including a targeted “Open in split…” mode, and adds localStorage persistence for the workspace document with new unit tests covering store behavior and persistence.Reviewed by Cursor Bugbot for commit d3d096c. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Refactor chat routes into a composable multi-pane workspace shell with split layout support
ChatViewdirectly; instead they open/focus a workspace surface viaopenThreadSurface, with WorkspaceRouteSync keeping the URL and focused surface in sync.ChatViewinto a standalone ThreadTerminalSurface component managed by workspace-level surface hooks, removing per-thread terminal drawer mounting fromChatView.workspaceTargetdisposition mode for opening threads into a specific split from CommandPalette.tsx.📊 Macroscope summarized d3d096c. 20 files reviewed, 4 issues evaluated, 0 issues filtered, 2 comments posted
🗂️ Filtered Issues