Enable terminals for local draft threads - #143

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals
Closed

Enable terminals for local draft threads#143
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Mar 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds local terminal state management for draft threads before they're promoted to server threads
  • Terminal operations (open/close, split, create, activate, resize) now work on draft threads
  • When a draft thread is promoted to a server thread, terminal state is hydrated via new HYDRATE_THREAD_TERMINALS action
  • Introduces draftThreadTerminalState.ts module with reducer pattern for managing draft terminal state

Test plan

  • Open a new draft thread and verify terminals can be opened/closed
  • Split and create new terminals in a draft thread
  • Send a message to promote the draft to a server thread and verify terminal state persists
  • Verify terminal height changes are preserved after promotion
  • Run bun test to verify new unit tests pass

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because this refactors core client state management (React reducer/context → Zustand) and moves terminal UI state out of Thread into a new persisted store, which could cause state sync/persistence regressions across threads and drafts.

Overview
Moves app state from a reducer/context to a Zustand store.store.ts now exposes pure transition helpers (e.g. markThreadUnread, setThreadBranch, setRuntimeMode) plus a Zustand-backed useStore() whose dispatch is a method API (dispatch.setError(...), dispatch.toggleProject(...), etc.).

Decouples terminal UI state from Thread and persists it separately. Terminal fields are removed from Thread (types.ts), and a new terminalStateStore.ts persists per-threadId terminal state in localStorage using a shared reducer in threadTerminalState.ts.

Updates UI to use the new dispatch API and terminal store.ChatView and Sidebar switch terminal open/height/tab/group/active/running indicators to useTerminalStateStore, while other actions are updated to call the new dispatch.* methods; root websocket terminal activity events now write directly to the terminal store. Tests are adjusted accordingly, with new unit coverage for threadTerminalState behavior.

Written by Cursor Bugbot for commit c283d2a. This will update automatically on new commits. Configure here.

Note

Move terminal UI state for local draft threads to a dedicated Zustand store and update ChatView and Sidebar to read/write via useTerminalStateStore with MAX_THREAD_TERMINAL_COUNT enforcement

Introduce useTerminalStateStore for per-thread terminal state and refactor app state to a single Zustand store; update ChatView and Sidebar to consume terminal state from the new store and remove terminal fields from Thread. Core terminal logic lives in reduceThreadTerminalState with normalization helpers.

📍Where to Start

Start with the terminal reducer and helpers in apps/web/src/threadTerminalState.ts, then see the store wiring in apps/web/src/terminalStateStore.ts and usage in apps/web/src/components/ChatView.tsx.

Macroscope summarized c283d2a.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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.

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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch t3code/enable-draft-thread-terminals

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

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Close terminal selects next active terminal differently than store
    • Added same-group preference logic (closedTerminalGroup lookup and remainingTerminalsInClosedGroup fallback) to closeDraftTerminal, matching the store's closeThreadTerminal 3-level fallback behavior.

Create PR

Or push these changes by commenting:

@cursor push 5002774b80
Preview (5002774b80)
diff --git a/apps/web/src/draftThreadTerminalState.ts b/apps/web/src/draftThreadTerminalState.ts--- a/apps/web/src/draftThreadTerminalState.ts+++ b/apps/web/src/draftThreadTerminalState.ts@@ -161,9 +161,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;

Comment threadapps/web/src/draftThreadTerminalState.ts
Comment on lines +51 to +53
return [...new Set(runningTerminalIds)]
.map((id) => id.trim())
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:51

Consider trimming before deduplicating—currently " term1 " and "term1" both survive the Set, then become identical after .trim(), producing duplicates.

Suggested change
return[...newSet(runningTerminalIds)]
.map((id)=>id.trim())
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
return[...newSet(runningTerminalIds.map((id)=>id.trim()))]
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 51-53:
Consider trimming before deduplicating—currently `" term1 "` and `"term1"` both survive the `Set`, then become identical after `.trim()`, producing duplicates.
Evidence trail:
File: apps/web/src/threadTerminalState.ts lines 51-53 at REVIEWED_COMMIT. The function `normalizeRunningTerminalIds` uses `[...new Set(runningTerminalIds)].map((id) => id.trim())`. The Set deduplicates before trim, so `" term1 "` and `"term1"` both survive the Set, then both become `"term1"` after trim, producing duplicates.

Comment threadapps/web/src/composerDraftStore.ts Outdated
juliusmarmingeand others added 3 commits March 2, 2026 10:07
Adds local terminal state management for draft threads before they're
promoted to server threads. Terminal operations (open, split, create,
close, activate, resize) now work on draft threads and hydrate into
the server thread state when the draft is promoted.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add shared threadTerminalState.ts: ThreadTerminalState type, default state,
normalize helpers, and single reduceThreadTerminalState() used by both flows
- Refactor store.ts to use shared terminal reducer and helpers; terminal
actions map to reduceThreadTerminalState(threadTerminalSlice(thread), action)
- Make draftThreadTerminalState.ts re-export shared module (same shape/actions)
- Add terminalState to composer draft store: getDraftThreadTerminalState,
setDraftThreadTerminalAction, clearDraftThreadTerminalState; draft thread
shape matches Thread terminal slice
- ChatView: move draft terminal from useState into composer draft store; use
same actions for draft and persisted; hydrate then clear draft terminal
when reconciling server state
Made-with: Cursor
…irectly
- Delete draftThreadTerminalState.ts (was only re-exporting shared module)
- Replace draftThreadTerminalState.test.ts with threadTerminalState.test.ts
using createDefaultThreadTerminalState and reduceThreadTerminalState
- Draft threads are first-class: same helpers and types as persisted threads
Made-with: Cursor
Comment on lines +149 to +151
export function createDefaultThreadTerminalState(): ThreadTerminalState {
return { ...DEFAULT_THREAD_TERMINAL_STATE };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:149

Defaults are shallow. Mutating nested arrays (e.g. terminalIds, terminalGroups) can leak into DEFAULT_THREAD_TERMINAL_STATE. Suggest deep-freezing the default and/or returning deep clones from createDefaultThreadTerminalState and getDefaultThreadTerminalState to avoid shared references.

-export function createDefaultThreadTerminalState(): ThreadTerminalState {- return { ...DEFAULT_THREAD_TERMINAL_STATE };+export function createDefaultThreadTerminalState(): ThreadTerminalState {+ return {+ ...DEFAULT_THREAD_TERMINAL_STATE,+ terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds],+ runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds],+ terminalGroups: DEFAULT_THREAD_TERMINAL_STATE.terminalGroups.map((g) => ({+ ...g,+ terminalIds: [...g.terminalIds],+ })),+ };
}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 149-151:
Defaults are shallow. Mutating nested arrays (e.g. `terminalIds`, `terminalGroups`) can leak into `DEFAULT_THREAD_TERMINAL_STATE`. Suggest deep-freezing the default and/or returning deep clones from `createDefaultThreadTerminalState` and `getDefaultThreadTerminalState` to avoid shared references.
Evidence trail:
apps/web/src/threadTerminalState.ts lines 132-155: `DEFAULT_THREAD_TERMINAL_STATE` is defined with `Object.freeze()` containing nested arrays (`terminalIds`, `runningTerminalIds`, `terminalGroups`). `createDefaultThreadTerminalState()` returns `{ ...DEFAULT_THREAD_TERMINAL_STATE }` (shallow copy). `getDefaultThreadTerminalState()` returns the frozen object directly. JavaScript's `Object.freeze()` is documented as shallow-freezing only top-level properties (MDN documentation). Usage sites in `composerDraftStore.ts` lines 683-697 show these functions being called and their results potentially stored/used.

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Close terminal lost group-aware active terminal selection
    • Restored group-aware active terminal selection in closeTerminal by first looking for siblings in the same group (remainingTerminalsInClosedGroup) before falling back to the global remainingTerminalIds list, matching the old closeThreadTerminal behavior.
  • ✅ Fixed: Dead reference equality check never short-circuits store updates
    • Replaced the dead next === current reference equality check with a structural comparison via a new threadTerminalStateEquals helper, since reduceThreadTerminalState always returns a new object through normalizeThreadTerminalState.

Create PR

Or push these changes by commenting:

@cursor push a82c89a4f7
Preview (a82c89a4f7)
diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts--- a/apps/web/src/composerDraftStore.ts+++ b/apps/web/src/composerDraftStore.ts@@ -10,6 +10,7 @@
createDefaultThreadTerminalState,
getDefaultThreadTerminalState,
reduceThreadTerminalState,
+ threadTerminalStateEquals,
type ThreadTerminalAction,
type ThreadTerminalState,
} from "./threadTerminalState";
@@ -696,7 +697,7 @@
}
const current = existing.terminalState ?? createDefaultThreadTerminalState();
const next = reduceThreadTerminalState(current, action);
- if (next === current) {+ if (threadTerminalStateEquals(next, current)) {
return state;
}
return {
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -205,9 +205,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;
@@ -242,6 +254,36 @@
}));
}
+export function threadTerminalStateEquals(a: ThreadTerminalState, b: ThreadTerminalState): boolean {+ if (a === b) return true;+ if (+ a.terminalOpen !== b.terminalOpen ||+ a.terminalHeight !== b.terminalHeight ||+ a.activeTerminalId !== b.activeTerminalId ||+ a.activeTerminalGroupId !== b.activeTerminalGroupId ||+ a.terminalIds.length !== b.terminalIds.length ||+ a.runningTerminalIds.length !== b.runningTerminalIds.length ||+ a.terminalGroups.length !== b.terminalGroups.length+ ) {+ return false;+ }+ for (let i = 0; i < a.terminalIds.length; i++) {+ if (a.terminalIds[i] !== b.terminalIds[i]) return false;+ }+ for (let i = 0; i < a.runningTerminalIds.length; i++) {+ if (a.runningTerminalIds[i] !== b.runningTerminalIds[i]) return false;+ }+ for (let i = 0; i < a.terminalGroups.length; i++) {+ const ga = a.terminalGroups[i]!;+ const gb = b.terminalGroups[i]!;+ if (ga.id !== gb.id || ga.terminalIds.length !== gb.terminalIds.length) return false;+ for (let j = 0; j < ga.terminalIds.length; j++) {+ if (ga.terminalIds[j] !== gb.terminalIds[j]) return false;+ }+ }+ return true;+}+
// ─── Single reducer for both draft and persisted thread ────────────────────
export function reduceThreadTerminalState(

Comment threadapps/web/src/threadTerminalState.ts
Comment threadapps/web/src/composerDraftStore.ts Outdated
- Main store: replace useReducer with Zustand; pure state transition
functions (syncServerReadModel, markThreadVisited, setError, etc.)
called via store methods. Persist via subscribe.
- Terminal store: single store keyed by threadId; replace
dispatchTerminalAction with direct methods (setTerminalOpen,
setTerminalHeight, splitTerminal, newTerminal, setActiveTerminal,
closeTerminal, setTerminalActivity). Pure reduceThreadTerminalState
used internally.
- Update all consumers to call dispatch.method(...) and terminal
store methods. Fix store tests to use markThreadUnread pure fn.
- Fix Fragment import in store.ts; fix unused get param for lint.
Made-with: Cursor
@juliusmarminge
juliusmarmingeforce-pushed the t3code/enable-draft-thread-terminals branch from eef2781 to c283d2aCompareMarch 2, 2026 18:24
Comment threadapps/web/src/store.ts
if (!thread.latestTurn?.completedAt) return thread;
const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt);
if (Number.isNaN(latestTurnCompletedAtMs)) return thread;
const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/store.ts:303

If latestTurn.completedAt is in the future (clock skew), unreadVisitedAt will also be in the future, preventing markThreadVisited from updating lastVisitedAt until client time catches up. Consider using Math.min(latestTurnCompletedAtMs - 1, Date.now()) to cap the timestamp.

Suggested change
constunreadVisitedAt=newDate(latestTurnCompletedAtMs-1).toISOString();
constunreadVisitedAt=newDate(Math.min(latestTurnCompletedAtMs-1,Date.now())).toISOString();
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around line 303:
If `latestTurn.completedAt` is in the future (clock skew), `unreadVisitedAt` will also be in the future, preventing `markThreadVisited` from updating `lastVisitedAt` until client time catches up. Consider using `Math.min(latestTurnCompletedAtMs - 1, Date.now())` to cap the timestamp.
Evidence trail:
apps/web/src/store.ts lines 296-306 (markThreadUnread function, line 303 sets unreadVisitedAt from latestTurn.completedAt - 1); apps/web/src/store.ts lines 273-291 (markThreadVisited function, line 278 defaults to Date.now(), lines 284-287 comparison that prevents update if previousVisitedAtMs >= visitedAtMs)

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumcomponents/Sidebar.tsx:240

The component subscribes to getTerminalState (a stable function reference), not the underlying terminalStateByThreadId data. When terminal state changes, this component won't re-render, causing stale terminalStatus display. Consider subscribing to the data directly, e.g., useTerminalStateStore((s) => s.terminalStateByThreadId), then access state in the render loop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Sidebar.tsx around line 240:
The component subscribes to `getTerminalState` (a stable function reference), not the underlying `terminalStateByThreadId` data. When terminal state changes, this component won't re-render, causing stale `terminalStatus` display. Consider subscribing to the data directly, e.g., `useTerminalStateStore((s) => s.terminalStateByThreadId)`, then access state in the render loop.
Evidence trail:
apps/web/src/components/Sidebar.tsx line 240: `const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);` - subscribes to function reference
apps/web/src/components/Sidebar.tsx lines 818-820: `const terminalStatus = terminalStatusFromRunningIds(getTerminalState(thread.id).runningTerminalIds);` - uses function to get data during render
apps/web/src/terminalStateStore.ts lines 51-66: Shows `getTerminalState` is a stable function that calls `get().terminalStateByThreadId[threadId]` - the function reference doesn't change when the data it accesses changes

juliusmarminge added a commit that referenced this pull request Mar 2, 2026
Integrate the draft and persisted terminal-state flow from #143 (shared thread terminal reducer plus dedicated terminal store), while preserving selector compatibility and runtime-mode switch robustness on this branch.
Made-with: Cursor
@juliusmarminge

Copy link
Copy Markdown
MemberAuthor

Superseded by #141. I merged the terminal-store work into that branch/PR so the three-store architecture now lives there (composer draft store + app state store + terminal store).

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Unstable dispatch reference causes infinite effect loop
    • Wrapped dispatch action functions in useMemo with empty deps so the dispatch object is referentially stable across renders, breaking the infinite re-render/effect cycle.
  • ✅ Fixed: Sidebar terminal status indicator not reactive to changes
    • Changed the Sidebar's terminal store subscription from the stable getTerminalState function reference to the terminalStateByThreadId data object, so changes to terminal running state trigger re-renders.
  • ✅ Fixed: No-op terminal actions always trigger store updates
    • Changed all no-op early-return paths in reduceThreadTerminalState to return the original state parameter instead of the normalized copy, allowing the reference equality check in applyTerminalAction to succeed and skip unnecessary store updates.

Create PR

Or push these changes by commenting:

@cursor push 004658797e
Preview (004658797e)
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@@ -237,7 +237,7 @@
(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
- const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);+ const terminalStateByThreadId = useTerminalStateStore((s) => s.terminalStateByThreadId);
const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId);
const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext);
const clearProjectDraftThreadId = useComposerDraftStore(
@@ -816,7 +816,7 @@
);
const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null);
const terminalStatus = terminalStatusFromRunningIds(
- getTerminalState(thread.id).runningTerminalIds,+ terminalStateByThreadId[thread.id]?.runningTerminalIds ?? [],
);
return (
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts--- a/apps/web/src/store.ts+++ b/apps/web/src/store.ts@@ -1,4 +1,4 @@-import { Fragment, type ReactNode, createElement, useEffect } from "react";+import { Fragment, type ReactNode, createElement, useEffect, useMemo } from "react";
import {
DEFAULT_MODEL,
ProviderSessionId,
@@ -416,8 +416,18 @@
threadsHydrated: s.threadsHydrated,
runtimeMode: s.runtimeMode,
}));
- return {- state,- dispatch: useAppStore.getState(),- };+ const dispatch = useMemo(() => {+ const s = useAppStore.getState();+ return {+ syncServerReadModel: s.syncServerReadModel,+ markThreadVisited: s.markThreadVisited,+ markThreadUnread: s.markThreadUnread,+ toggleProject: s.toggleProject,+ setProjectExpanded: s.setProjectExpanded,+ setError: s.setError,+ setThreadBranch: s.setThreadBranch,+ setRuntimeMode: s.setRuntimeMode,+ };+ }, []);+ return { state, dispatch };
}
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -251,7 +251,7 @@
const normalized = normalizeThreadTerminalState(state);
if (action.type === "set-open") {
- if (normalized.terminalOpen === action.open) return normalized;+ if (normalized.terminalOpen === action.open) return state;
return { ...normalized, terminalOpen: action.open };
}
@@ -261,14 +261,14 @@
action.height <= 0 ||
normalized.terminalHeight === action.height
) {
- return normalized;+ return state;
}
return { ...normalized, terminalHeight: action.height };
}
if (action.type === "set-active") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const activeTerminalGroupId =
normalized.terminalGroups.find((group) => group.terminalIds.includes(action.terminalId))?.id ??
@@ -286,7 +286,7 @@
if (action.type === "set-activity") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const runningTerminalIds = new Set(normalized.runningTerminalIds);
if (action.hasRunningSubprocess) {
@@ -298,12 +298,12 @@
}
if (!action.terminalId || action.terminalId.trim().length === 0) {
- return normalized;+ return state;
}
const isNewTerminal = !normalized.terminalIds.includes(action.terminalId);
if (isNewTerminal && normalized.terminalIds.length >= MAX_THREAD_TERMINAL_COUNT) {
- return normalized;+ return state;
}
const terminalIds = isNewTerminal

Comment threadapps/web/src/store.ts

function setThreadTerminalActivity(
thread: Thread,
terminalId: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable dispatch reference causes infinite effect loop

High Severity

useStore() returns dispatch: useAppStore.getState(), which produces a new object reference after every store update. The selector also creates a new inline object on every call, so every store change triggers a re-render. In EventRouter, this dispatch is in the useEffect dependency array. Each effect run calls syncSnapshot(), which calls dispatch.syncServerReadModel(snapshot), which updates the store, which triggers a re-render, which changes dispatch, which re-runs the effect — creating an infinite loop of network requests and event re-subscriptions.

Additional Locations (1)

Fix in CursorFix in Web

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar terminal status indicator not reactive to changes

Medium Severity

The Sidebar subscribes to useTerminalStateStore((s) => s.getTerminalState), which returns the getTerminalState function reference — a stable closure that never changes. This means the Sidebar never re-renders when terminal state (like runningTerminalIds) changes in the terminal store. The terminal running indicator in the sidebar will be stale until the component re-renders for an unrelated reason.

Additional Locations (1)

Fix in CursorFix in Web

...state,
[threadId]: next,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No-op terminal actions always trigger store updates

Low Severity

applyTerminalAction checks if (next === current) to avoid unnecessary updates, but this never succeeds. reduceThreadTerminalState always calls normalizeThreadTerminalState first, which creates a new object. So even for no-op actions (e.g., setting terminalOpen to its current value), next is a different reference than current, and the store always updates. The intended optimization is defeated.

Additional Locations (1)

Fix in CursorFix in Web

logancsack added a commit to logancsack/t3code that referenced this pull request Aug 4, 2026
Aldo Review lost 27% of its lead reviewers in the field (12 of 44 slots across remote-dev PRs pingdotgg#135-pingdotgg#143): 8 Grok Build structured-output rejections and 4 timeouts. Nothing retried, so one transient blip deleted a whole reviewer for the entire run.
Retry every failure a fresh sample could plausibly fix, against a new subprocess. Output that overflows the collection limit stays terminal because it recurs identically.
Give the swarm one wall-clock budget split into stage deadlines, so a retrying lead can never starve the verification that produces the report. The worst-case serial path was already 12 minutes against a 10-minute service cap. Delegation, the only optional stage, is skipped and reported rather than started with too little time to finish.
Carry the Grok Build error cause into reviewer diagnostics so the next failure is diagnosable.
Sign up for freeto 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.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Enable terminals for local draft threads - #143

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals
Closed

Enable terminals for local draft threads#143
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Mar 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds local terminal state management for draft threads before they're promoted to server threads
  • Terminal operations (open/close, split, create, activate, resize) now work on draft threads
  • When a draft thread is promoted to a server thread, terminal state is hydrated via new HYDRATE_THREAD_TERMINALS action
  • Introduces draftThreadTerminalState.ts module with reducer pattern for managing draft terminal state

Test plan

  • Open a new draft thread and verify terminals can be opened/closed
  • Split and create new terminals in a draft thread
  • Send a message to promote the draft to a server thread and verify terminal state persists
  • Verify terminal height changes are preserved after promotion
  • Run bun test to verify new unit tests pass

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because this refactors core client state management (React reducer/context → Zustand) and moves terminal UI state out of Thread into a new persisted store, which could cause state sync/persistence regressions across threads and drafts.

Overview
Moves app state from a reducer/context to a Zustand store.store.ts now exposes pure transition helpers (e.g. markThreadUnread, setThreadBranch, setRuntimeMode) plus a Zustand-backed useStore() whose dispatch is a method API (dispatch.setError(...), dispatch.toggleProject(...), etc.).

Decouples terminal UI state from Thread and persists it separately. Terminal fields are removed from Thread (types.ts), and a new terminalStateStore.ts persists per-threadId terminal state in localStorage using a shared reducer in threadTerminalState.ts.

Updates UI to use the new dispatch API and terminal store.ChatView and Sidebar switch terminal open/height/tab/group/active/running indicators to useTerminalStateStore, while other actions are updated to call the new dispatch.* methods; root websocket terminal activity events now write directly to the terminal store. Tests are adjusted accordingly, with new unit coverage for threadTerminalState behavior.

Written by Cursor Bugbot for commit c283d2a. This will update automatically on new commits. Configure here.

Note

Move terminal UI state for local draft threads to a dedicated Zustand store and update ChatView and Sidebar to read/write via useTerminalStateStore with MAX_THREAD_TERMINAL_COUNT enforcement

Introduce useTerminalStateStore for per-thread terminal state and refactor app state to a single Zustand store; update ChatView and Sidebar to consume terminal state from the new store and remove terminal fields from Thread. Core terminal logic lives in reduceThreadTerminalState with normalization helpers.

📍Where to Start

Start with the terminal reducer and helpers in apps/web/src/threadTerminalState.ts, then see the store wiring in apps/web/src/terminalStateStore.ts and usage in apps/web/src/components/ChatView.tsx.

Macroscope summarized c283d2a.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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.

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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch t3code/enable-draft-thread-terminals

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

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Close terminal selects next active terminal differently than store
    • Added same-group preference logic (closedTerminalGroup lookup and remainingTerminalsInClosedGroup fallback) to closeDraftTerminal, matching the store's closeThreadTerminal 3-level fallback behavior.

Create PR

Or push these changes by commenting:

@cursor push 5002774b80
Preview (5002774b80)
diff --git a/apps/web/src/draftThreadTerminalState.ts b/apps/web/src/draftThreadTerminalState.ts--- a/apps/web/src/draftThreadTerminalState.ts+++ b/apps/web/src/draftThreadTerminalState.ts@@ -161,9 +161,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;

Comment threadapps/web/src/draftThreadTerminalState.ts
Comment on lines +51 to +53
return [...new Set(runningTerminalIds)]
.map((id) => id.trim())
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:51

Consider trimming before deduplicating—currently " term1 " and "term1" both survive the Set, then become identical after .trim(), producing duplicates.

Suggested change
return[...newSet(runningTerminalIds)]
.map((id)=>id.trim())
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
return[...newSet(runningTerminalIds.map((id)=>id.trim()))]
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 51-53:
Consider trimming before deduplicating—currently `" term1 "` and `"term1"` both survive the `Set`, then become identical after `.trim()`, producing duplicates.
Evidence trail:
File: apps/web/src/threadTerminalState.ts lines 51-53 at REVIEWED_COMMIT. The function `normalizeRunningTerminalIds` uses `[...new Set(runningTerminalIds)].map((id) => id.trim())`. The Set deduplicates before trim, so `" term1 "` and `"term1"` both survive the Set, then both become `"term1"` after trim, producing duplicates.

Comment threadapps/web/src/composerDraftStore.ts Outdated
juliusmarmingeand others added 3 commits March 2, 2026 10:07
Adds local terminal state management for draft threads before they're
promoted to server threads. Terminal operations (open, split, create,
close, activate, resize) now work on draft threads and hydrate into
the server thread state when the draft is promoted.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add shared threadTerminalState.ts: ThreadTerminalState type, default state,
normalize helpers, and single reduceThreadTerminalState() used by both flows
- Refactor store.ts to use shared terminal reducer and helpers; terminal
actions map to reduceThreadTerminalState(threadTerminalSlice(thread), action)
- Make draftThreadTerminalState.ts re-export shared module (same shape/actions)
- Add terminalState to composer draft store: getDraftThreadTerminalState,
setDraftThreadTerminalAction, clearDraftThreadTerminalState; draft thread
shape matches Thread terminal slice
- ChatView: move draft terminal from useState into composer draft store; use
same actions for draft and persisted; hydrate then clear draft terminal
when reconciling server state
Made-with: Cursor
…irectly
- Delete draftThreadTerminalState.ts (was only re-exporting shared module)
- Replace draftThreadTerminalState.test.ts with threadTerminalState.test.ts
using createDefaultThreadTerminalState and reduceThreadTerminalState
- Draft threads are first-class: same helpers and types as persisted threads
Made-with: Cursor
Comment on lines +149 to +151
export function createDefaultThreadTerminalState(): ThreadTerminalState {
return { ...DEFAULT_THREAD_TERMINAL_STATE };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:149

Defaults are shallow. Mutating nested arrays (e.g. terminalIds, terminalGroups) can leak into DEFAULT_THREAD_TERMINAL_STATE. Suggest deep-freezing the default and/or returning deep clones from createDefaultThreadTerminalState and getDefaultThreadTerminalState to avoid shared references.

-export function createDefaultThreadTerminalState(): ThreadTerminalState {- return { ...DEFAULT_THREAD_TERMINAL_STATE };+export function createDefaultThreadTerminalState(): ThreadTerminalState {+ return {+ ...DEFAULT_THREAD_TERMINAL_STATE,+ terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds],+ runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds],+ terminalGroups: DEFAULT_THREAD_TERMINAL_STATE.terminalGroups.map((g) => ({+ ...g,+ terminalIds: [...g.terminalIds],+ })),+ };
}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 149-151:
Defaults are shallow. Mutating nested arrays (e.g. `terminalIds`, `terminalGroups`) can leak into `DEFAULT_THREAD_TERMINAL_STATE`. Suggest deep-freezing the default and/or returning deep clones from `createDefaultThreadTerminalState` and `getDefaultThreadTerminalState` to avoid shared references.
Evidence trail:
apps/web/src/threadTerminalState.ts lines 132-155: `DEFAULT_THREAD_TERMINAL_STATE` is defined with `Object.freeze()` containing nested arrays (`terminalIds`, `runningTerminalIds`, `terminalGroups`). `createDefaultThreadTerminalState()` returns `{ ...DEFAULT_THREAD_TERMINAL_STATE }` (shallow copy). `getDefaultThreadTerminalState()` returns the frozen object directly. JavaScript's `Object.freeze()` is documented as shallow-freezing only top-level properties (MDN documentation). Usage sites in `composerDraftStore.ts` lines 683-697 show these functions being called and their results potentially stored/used.

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Close terminal lost group-aware active terminal selection
    • Restored group-aware active terminal selection in closeTerminal by first looking for siblings in the same group (remainingTerminalsInClosedGroup) before falling back to the global remainingTerminalIds list, matching the old closeThreadTerminal behavior.
  • ✅ Fixed: Dead reference equality check never short-circuits store updates
    • Replaced the dead next === current reference equality check with a structural comparison via a new threadTerminalStateEquals helper, since reduceThreadTerminalState always returns a new object through normalizeThreadTerminalState.

Create PR

Or push these changes by commenting:

@cursor push a82c89a4f7
Preview (a82c89a4f7)
diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts--- a/apps/web/src/composerDraftStore.ts+++ b/apps/web/src/composerDraftStore.ts@@ -10,6 +10,7 @@
createDefaultThreadTerminalState,
getDefaultThreadTerminalState,
reduceThreadTerminalState,
+ threadTerminalStateEquals,
type ThreadTerminalAction,
type ThreadTerminalState,
} from "./threadTerminalState";
@@ -696,7 +697,7 @@
}
const current = existing.terminalState ?? createDefaultThreadTerminalState();
const next = reduceThreadTerminalState(current, action);
- if (next === current) {+ if (threadTerminalStateEquals(next, current)) {
return state;
}
return {
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -205,9 +205,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;
@@ -242,6 +254,36 @@
}));
}
+export function threadTerminalStateEquals(a: ThreadTerminalState, b: ThreadTerminalState): boolean {+ if (a === b) return true;+ if (+ a.terminalOpen !== b.terminalOpen ||+ a.terminalHeight !== b.terminalHeight ||+ a.activeTerminalId !== b.activeTerminalId ||+ a.activeTerminalGroupId !== b.activeTerminalGroupId ||+ a.terminalIds.length !== b.terminalIds.length ||+ a.runningTerminalIds.length !== b.runningTerminalIds.length ||+ a.terminalGroups.length !== b.terminalGroups.length+ ) {+ return false;+ }+ for (let i = 0; i < a.terminalIds.length; i++) {+ if (a.terminalIds[i] !== b.terminalIds[i]) return false;+ }+ for (let i = 0; i < a.runningTerminalIds.length; i++) {+ if (a.runningTerminalIds[i] !== b.runningTerminalIds[i]) return false;+ }+ for (let i = 0; i < a.terminalGroups.length; i++) {+ const ga = a.terminalGroups[i]!;+ const gb = b.terminalGroups[i]!;+ if (ga.id !== gb.id || ga.terminalIds.length !== gb.terminalIds.length) return false;+ for (let j = 0; j < ga.terminalIds.length; j++) {+ if (ga.terminalIds[j] !== gb.terminalIds[j]) return false;+ }+ }+ return true;+}+
// ─── Single reducer for both draft and persisted thread ────────────────────
export function reduceThreadTerminalState(

Comment threadapps/web/src/threadTerminalState.ts
Comment threadapps/web/src/composerDraftStore.ts Outdated
- Main store: replace useReducer with Zustand; pure state transition
functions (syncServerReadModel, markThreadVisited, setError, etc.)
called via store methods. Persist via subscribe.
- Terminal store: single store keyed by threadId; replace
dispatchTerminalAction with direct methods (setTerminalOpen,
setTerminalHeight, splitTerminal, newTerminal, setActiveTerminal,
closeTerminal, setTerminalActivity). Pure reduceThreadTerminalState
used internally.
- Update all consumers to call dispatch.method(...) and terminal
store methods. Fix store tests to use markThreadUnread pure fn.
- Fix Fragment import in store.ts; fix unused get param for lint.
Made-with: Cursor
@juliusmarminge
juliusmarmingeforce-pushed the t3code/enable-draft-thread-terminals branch from eef2781 to c283d2aCompareMarch 2, 2026 18:24
Comment threadapps/web/src/store.ts
if (!thread.latestTurn?.completedAt) return thread;
const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt);
if (Number.isNaN(latestTurnCompletedAtMs)) return thread;
const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/store.ts:303

If latestTurn.completedAt is in the future (clock skew), unreadVisitedAt will also be in the future, preventing markThreadVisited from updating lastVisitedAt until client time catches up. Consider using Math.min(latestTurnCompletedAtMs - 1, Date.now()) to cap the timestamp.

Suggested change
constunreadVisitedAt=newDate(latestTurnCompletedAtMs-1).toISOString();
constunreadVisitedAt=newDate(Math.min(latestTurnCompletedAtMs-1,Date.now())).toISOString();
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around line 303:
If `latestTurn.completedAt` is in the future (clock skew), `unreadVisitedAt` will also be in the future, preventing `markThreadVisited` from updating `lastVisitedAt` until client time catches up. Consider using `Math.min(latestTurnCompletedAtMs - 1, Date.now())` to cap the timestamp.
Evidence trail:
apps/web/src/store.ts lines 296-306 (markThreadUnread function, line 303 sets unreadVisitedAt from latestTurn.completedAt - 1); apps/web/src/store.ts lines 273-291 (markThreadVisited function, line 278 defaults to Date.now(), lines 284-287 comparison that prevents update if previousVisitedAtMs >= visitedAtMs)

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumcomponents/Sidebar.tsx:240

The component subscribes to getTerminalState (a stable function reference), not the underlying terminalStateByThreadId data. When terminal state changes, this component won't re-render, causing stale terminalStatus display. Consider subscribing to the data directly, e.g., useTerminalStateStore((s) => s.terminalStateByThreadId), then access state in the render loop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Sidebar.tsx around line 240:
The component subscribes to `getTerminalState` (a stable function reference), not the underlying `terminalStateByThreadId` data. When terminal state changes, this component won't re-render, causing stale `terminalStatus` display. Consider subscribing to the data directly, e.g., `useTerminalStateStore((s) => s.terminalStateByThreadId)`, then access state in the render loop.
Evidence trail:
apps/web/src/components/Sidebar.tsx line 240: `const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);` - subscribes to function reference
apps/web/src/components/Sidebar.tsx lines 818-820: `const terminalStatus = terminalStatusFromRunningIds(getTerminalState(thread.id).runningTerminalIds);` - uses function to get data during render
apps/web/src/terminalStateStore.ts lines 51-66: Shows `getTerminalState` is a stable function that calls `get().terminalStateByThreadId[threadId]` - the function reference doesn't change when the data it accesses changes

juliusmarminge added a commit that referenced this pull request Mar 2, 2026
Integrate the draft and persisted terminal-state flow from #143 (shared thread terminal reducer plus dedicated terminal store), while preserving selector compatibility and runtime-mode switch robustness on this branch.
Made-with: Cursor
@juliusmarminge

Copy link
Copy Markdown
MemberAuthor

Superseded by #141. I merged the terminal-store work into that branch/PR so the three-store architecture now lives there (composer draft store + app state store + terminal store).

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Unstable dispatch reference causes infinite effect loop
    • Wrapped dispatch action functions in useMemo with empty deps so the dispatch object is referentially stable across renders, breaking the infinite re-render/effect cycle.
  • ✅ Fixed: Sidebar terminal status indicator not reactive to changes
    • Changed the Sidebar's terminal store subscription from the stable getTerminalState function reference to the terminalStateByThreadId data object, so changes to terminal running state trigger re-renders.
  • ✅ Fixed: No-op terminal actions always trigger store updates
    • Changed all no-op early-return paths in reduceThreadTerminalState to return the original state parameter instead of the normalized copy, allowing the reference equality check in applyTerminalAction to succeed and skip unnecessary store updates.

Create PR

Or push these changes by commenting:

@cursor push 004658797e
Preview (004658797e)
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@@ -237,7 +237,7 @@
(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
- const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);+ const terminalStateByThreadId = useTerminalStateStore((s) => s.terminalStateByThreadId);
const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId);
const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext);
const clearProjectDraftThreadId = useComposerDraftStore(
@@ -816,7 +816,7 @@
);
const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null);
const terminalStatus = terminalStatusFromRunningIds(
- getTerminalState(thread.id).runningTerminalIds,+ terminalStateByThreadId[thread.id]?.runningTerminalIds ?? [],
);
return (
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts--- a/apps/web/src/store.ts+++ b/apps/web/src/store.ts@@ -1,4 +1,4 @@-import { Fragment, type ReactNode, createElement, useEffect } from "react";+import { Fragment, type ReactNode, createElement, useEffect, useMemo } from "react";
import {
DEFAULT_MODEL,
ProviderSessionId,
@@ -416,8 +416,18 @@
threadsHydrated: s.threadsHydrated,
runtimeMode: s.runtimeMode,
}));
- return {- state,- dispatch: useAppStore.getState(),- };+ const dispatch = useMemo(() => {+ const s = useAppStore.getState();+ return {+ syncServerReadModel: s.syncServerReadModel,+ markThreadVisited: s.markThreadVisited,+ markThreadUnread: s.markThreadUnread,+ toggleProject: s.toggleProject,+ setProjectExpanded: s.setProjectExpanded,+ setError: s.setError,+ setThreadBranch: s.setThreadBranch,+ setRuntimeMode: s.setRuntimeMode,+ };+ }, []);+ return { state, dispatch };
}
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -251,7 +251,7 @@
const normalized = normalizeThreadTerminalState(state);
if (action.type === "set-open") {
- if (normalized.terminalOpen === action.open) return normalized;+ if (normalized.terminalOpen === action.open) return state;
return { ...normalized, terminalOpen: action.open };
}
@@ -261,14 +261,14 @@
action.height <= 0 ||
normalized.terminalHeight === action.height
) {
- return normalized;+ return state;
}
return { ...normalized, terminalHeight: action.height };
}
if (action.type === "set-active") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const activeTerminalGroupId =
normalized.terminalGroups.find((group) => group.terminalIds.includes(action.terminalId))?.id ??
@@ -286,7 +286,7 @@
if (action.type === "set-activity") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const runningTerminalIds = new Set(normalized.runningTerminalIds);
if (action.hasRunningSubprocess) {
@@ -298,12 +298,12 @@
}
if (!action.terminalId || action.terminalId.trim().length === 0) {
- return normalized;+ return state;
}
const isNewTerminal = !normalized.terminalIds.includes(action.terminalId);
if (isNewTerminal && normalized.terminalIds.length >= MAX_THREAD_TERMINAL_COUNT) {
- return normalized;+ return state;
}
const terminalIds = isNewTerminal

Comment threadapps/web/src/store.ts

function setThreadTerminalActivity(
thread: Thread,
terminalId: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable dispatch reference causes infinite effect loop

High Severity

useStore() returns dispatch: useAppStore.getState(), which produces a new object reference after every store update. The selector also creates a new inline object on every call, so every store change triggers a re-render. In EventRouter, this dispatch is in the useEffect dependency array. Each effect run calls syncSnapshot(), which calls dispatch.syncServerReadModel(snapshot), which updates the store, which triggers a re-render, which changes dispatch, which re-runs the effect — creating an infinite loop of network requests and event re-subscriptions.

Additional Locations (1)

Fix in CursorFix in Web

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar terminal status indicator not reactive to changes

Medium Severity

The Sidebar subscribes to useTerminalStateStore((s) => s.getTerminalState), which returns the getTerminalState function reference — a stable closure that never changes. This means the Sidebar never re-renders when terminal state (like runningTerminalIds) changes in the terminal store. The terminal running indicator in the sidebar will be stale until the component re-renders for an unrelated reason.

Additional Locations (1)

Fix in CursorFix in Web

...state,
[threadId]: next,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No-op terminal actions always trigger store updates

Low Severity

applyTerminalAction checks if (next === current) to avoid unnecessary updates, but this never succeeds. reduceThreadTerminalState always calls normalizeThreadTerminalState first, which creates a new object. So even for no-op actions (e.g., setting terminalOpen to its current value), next is a different reference than current, and the store always updates. The intended optimization is defeated.

Additional Locations (1)

Fix in CursorFix in Web

logancsack added a commit to logancsack/t3code that referenced this pull request Aug 4, 2026
Aldo Review lost 27% of its lead reviewers in the field (12 of 44 slots across remote-dev PRs pingdotgg#135-pingdotgg#143): 8 Grok Build structured-output rejections and 4 timeouts. Nothing retried, so one transient blip deleted a whole reviewer for the entire run.
Retry every failure a fresh sample could plausibly fix, against a new subprocess. Output that overflows the collection limit stays terminal because it recurs identically.
Give the swarm one wall-clock budget split into stage deadlines, so a retrying lead can never starve the verification that produces the report. The worst-case serial path was already 12 minutes against a 10-minute service cap. Delegation, the only optional stage, is skipped and reported rather than started with too little time to finish.
Carry the Grok Build error cause into reviewer diagnostics so the next failure is diagnosable.
Sign up for freeto 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.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enable terminals for local draft threads - #143

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals
Closed

Enable terminals for local draft threads#143
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Mar 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds local terminal state management for draft threads before they're promoted to server threads
  • Terminal operations (open/close, split, create, activate, resize) now work on draft threads
  • When a draft thread is promoted to a server thread, terminal state is hydrated via new HYDRATE_THREAD_TERMINALS action
  • Introduces draftThreadTerminalState.ts module with reducer pattern for managing draft terminal state

Test plan

  • Open a new draft thread and verify terminals can be opened/closed
  • Split and create new terminals in a draft thread
  • Send a message to promote the draft to a server thread and verify terminal state persists
  • Verify terminal height changes are preserved after promotion
  • Run bun test to verify new unit tests pass

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because this refactors core client state management (React reducer/context → Zustand) and moves terminal UI state out of Thread into a new persisted store, which could cause state sync/persistence regressions across threads and drafts.

Overview
Moves app state from a reducer/context to a Zustand store.store.ts now exposes pure transition helpers (e.g. markThreadUnread, setThreadBranch, setRuntimeMode) plus a Zustand-backed useStore() whose dispatch is a method API (dispatch.setError(...), dispatch.toggleProject(...), etc.).

Decouples terminal UI state from Thread and persists it separately. Terminal fields are removed from Thread (types.ts), and a new terminalStateStore.ts persists per-threadId terminal state in localStorage using a shared reducer in threadTerminalState.ts.

Updates UI to use the new dispatch API and terminal store.ChatView and Sidebar switch terminal open/height/tab/group/active/running indicators to useTerminalStateStore, while other actions are updated to call the new dispatch.* methods; root websocket terminal activity events now write directly to the terminal store. Tests are adjusted accordingly, with new unit coverage for threadTerminalState behavior.

Written by Cursor Bugbot for commit c283d2a. This will update automatically on new commits. Configure here.

Note

Move terminal UI state for local draft threads to a dedicated Zustand store and update ChatView and Sidebar to read/write via useTerminalStateStore with MAX_THREAD_TERMINAL_COUNT enforcement

Introduce useTerminalStateStore for per-thread terminal state and refactor app state to a single Zustand store; update ChatView and Sidebar to consume terminal state from the new store and remove terminal fields from Thread. Core terminal logic lives in reduceThreadTerminalState with normalization helpers.

📍Where to Start

Start with the terminal reducer and helpers in apps/web/src/threadTerminalState.ts, then see the store wiring in apps/web/src/terminalStateStore.ts and usage in apps/web/src/components/ChatView.tsx.

Macroscope summarized c283d2a.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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.

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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch t3code/enable-draft-thread-terminals

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

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Close terminal selects next active terminal differently than store
    • Added same-group preference logic (closedTerminalGroup lookup and remainingTerminalsInClosedGroup fallback) to closeDraftTerminal, matching the store's closeThreadTerminal 3-level fallback behavior.

Create PR

Or push these changes by commenting:

@cursor push 5002774b80
Preview (5002774b80)
diff --git a/apps/web/src/draftThreadTerminalState.ts b/apps/web/src/draftThreadTerminalState.ts--- a/apps/web/src/draftThreadTerminalState.ts+++ b/apps/web/src/draftThreadTerminalState.ts@@ -161,9 +161,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;

Comment threadapps/web/src/draftThreadTerminalState.ts
Comment on lines +51 to +53
return [...new Set(runningTerminalIds)]
.map((id) => id.trim())
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:51

Consider trimming before deduplicating—currently " term1 " and "term1" both survive the Set, then become identical after .trim(), producing duplicates.

Suggested change
return[...newSet(runningTerminalIds)]
.map((id)=>id.trim())
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
return[...newSet(runningTerminalIds.map((id)=>id.trim()))]
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 51-53:
Consider trimming before deduplicating—currently `" term1 "` and `"term1"` both survive the `Set`, then become identical after `.trim()`, producing duplicates.
Evidence trail:
File: apps/web/src/threadTerminalState.ts lines 51-53 at REVIEWED_COMMIT. The function `normalizeRunningTerminalIds` uses `[...new Set(runningTerminalIds)].map((id) => id.trim())`. The Set deduplicates before trim, so `" term1 "` and `"term1"` both survive the Set, then both become `"term1"` after trim, producing duplicates.

Comment threadapps/web/src/composerDraftStore.ts Outdated
juliusmarmingeand others added 3 commits March 2, 2026 10:07
Adds local terminal state management for draft threads before they're
promoted to server threads. Terminal operations (open, split, create,
close, activate, resize) now work on draft threads and hydrate into
the server thread state when the draft is promoted.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add shared threadTerminalState.ts: ThreadTerminalState type, default state,
normalize helpers, and single reduceThreadTerminalState() used by both flows
- Refactor store.ts to use shared terminal reducer and helpers; terminal
actions map to reduceThreadTerminalState(threadTerminalSlice(thread), action)
- Make draftThreadTerminalState.ts re-export shared module (same shape/actions)
- Add terminalState to composer draft store: getDraftThreadTerminalState,
setDraftThreadTerminalAction, clearDraftThreadTerminalState; draft thread
shape matches Thread terminal slice
- ChatView: move draft terminal from useState into composer draft store; use
same actions for draft and persisted; hydrate then clear draft terminal
when reconciling server state
Made-with: Cursor
…irectly
- Delete draftThreadTerminalState.ts (was only re-exporting shared module)
- Replace draftThreadTerminalState.test.ts with threadTerminalState.test.ts
using createDefaultThreadTerminalState and reduceThreadTerminalState
- Draft threads are first-class: same helpers and types as persisted threads
Made-with: Cursor
Comment on lines +149 to +151
export function createDefaultThreadTerminalState(): ThreadTerminalState {
return { ...DEFAULT_THREAD_TERMINAL_STATE };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:149

Defaults are shallow. Mutating nested arrays (e.g. terminalIds, terminalGroups) can leak into DEFAULT_THREAD_TERMINAL_STATE. Suggest deep-freezing the default and/or returning deep clones from createDefaultThreadTerminalState and getDefaultThreadTerminalState to avoid shared references.

-export function createDefaultThreadTerminalState(): ThreadTerminalState {- return { ...DEFAULT_THREAD_TERMINAL_STATE };+export function createDefaultThreadTerminalState(): ThreadTerminalState {+ return {+ ...DEFAULT_THREAD_TERMINAL_STATE,+ terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds],+ runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds],+ terminalGroups: DEFAULT_THREAD_TERMINAL_STATE.terminalGroups.map((g) => ({+ ...g,+ terminalIds: [...g.terminalIds],+ })),+ };
}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 149-151:
Defaults are shallow. Mutating nested arrays (e.g. `terminalIds`, `terminalGroups`) can leak into `DEFAULT_THREAD_TERMINAL_STATE`. Suggest deep-freezing the default and/or returning deep clones from `createDefaultThreadTerminalState` and `getDefaultThreadTerminalState` to avoid shared references.
Evidence trail:
apps/web/src/threadTerminalState.ts lines 132-155: `DEFAULT_THREAD_TERMINAL_STATE` is defined with `Object.freeze()` containing nested arrays (`terminalIds`, `runningTerminalIds`, `terminalGroups`). `createDefaultThreadTerminalState()` returns `{ ...DEFAULT_THREAD_TERMINAL_STATE }` (shallow copy). `getDefaultThreadTerminalState()` returns the frozen object directly. JavaScript's `Object.freeze()` is documented as shallow-freezing only top-level properties (MDN documentation). Usage sites in `composerDraftStore.ts` lines 683-697 show these functions being called and their results potentially stored/used.

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Close terminal lost group-aware active terminal selection
    • Restored group-aware active terminal selection in closeTerminal by first looking for siblings in the same group (remainingTerminalsInClosedGroup) before falling back to the global remainingTerminalIds list, matching the old closeThreadTerminal behavior.
  • ✅ Fixed: Dead reference equality check never short-circuits store updates
    • Replaced the dead next === current reference equality check with a structural comparison via a new threadTerminalStateEquals helper, since reduceThreadTerminalState always returns a new object through normalizeThreadTerminalState.

Create PR

Or push these changes by commenting:

@cursor push a82c89a4f7
Preview (a82c89a4f7)
diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts--- a/apps/web/src/composerDraftStore.ts+++ b/apps/web/src/composerDraftStore.ts@@ -10,6 +10,7 @@
createDefaultThreadTerminalState,
getDefaultThreadTerminalState,
reduceThreadTerminalState,
+ threadTerminalStateEquals,
type ThreadTerminalAction,
type ThreadTerminalState,
} from "./threadTerminalState";
@@ -696,7 +697,7 @@
}
const current = existing.terminalState ?? createDefaultThreadTerminalState();
const next = reduceThreadTerminalState(current, action);
- if (next === current) {+ if (threadTerminalStateEquals(next, current)) {
return state;
}
return {
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -205,9 +205,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;
@@ -242,6 +254,36 @@
}));
}
+export function threadTerminalStateEquals(a: ThreadTerminalState, b: ThreadTerminalState): boolean {+ if (a === b) return true;+ if (+ a.terminalOpen !== b.terminalOpen ||+ a.terminalHeight !== b.terminalHeight ||+ a.activeTerminalId !== b.activeTerminalId ||+ a.activeTerminalGroupId !== b.activeTerminalGroupId ||+ a.terminalIds.length !== b.terminalIds.length ||+ a.runningTerminalIds.length !== b.runningTerminalIds.length ||+ a.terminalGroups.length !== b.terminalGroups.length+ ) {+ return false;+ }+ for (let i = 0; i < a.terminalIds.length; i++) {+ if (a.terminalIds[i] !== b.terminalIds[i]) return false;+ }+ for (let i = 0; i < a.runningTerminalIds.length; i++) {+ if (a.runningTerminalIds[i] !== b.runningTerminalIds[i]) return false;+ }+ for (let i = 0; i < a.terminalGroups.length; i++) {+ const ga = a.terminalGroups[i]!;+ const gb = b.terminalGroups[i]!;+ if (ga.id !== gb.id || ga.terminalIds.length !== gb.terminalIds.length) return false;+ for (let j = 0; j < ga.terminalIds.length; j++) {+ if (ga.terminalIds[j] !== gb.terminalIds[j]) return false;+ }+ }+ return true;+}+
// ─── Single reducer for both draft and persisted thread ────────────────────
export function reduceThreadTerminalState(

Comment threadapps/web/src/threadTerminalState.ts
Comment threadapps/web/src/composerDraftStore.ts Outdated
- Main store: replace useReducer with Zustand; pure state transition
functions (syncServerReadModel, markThreadVisited, setError, etc.)
called via store methods. Persist via subscribe.
- Terminal store: single store keyed by threadId; replace
dispatchTerminalAction with direct methods (setTerminalOpen,
setTerminalHeight, splitTerminal, newTerminal, setActiveTerminal,
closeTerminal, setTerminalActivity). Pure reduceThreadTerminalState
used internally.
- Update all consumers to call dispatch.method(...) and terminal
store methods. Fix store tests to use markThreadUnread pure fn.
- Fix Fragment import in store.ts; fix unused get param for lint.
Made-with: Cursor
@juliusmarminge
juliusmarmingeforce-pushed the t3code/enable-draft-thread-terminals branch from eef2781 to c283d2aCompareMarch 2, 2026 18:24
Comment threadapps/web/src/store.ts
if (!thread.latestTurn?.completedAt) return thread;
const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt);
if (Number.isNaN(latestTurnCompletedAtMs)) return thread;
const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/store.ts:303

If latestTurn.completedAt is in the future (clock skew), unreadVisitedAt will also be in the future, preventing markThreadVisited from updating lastVisitedAt until client time catches up. Consider using Math.min(latestTurnCompletedAtMs - 1, Date.now()) to cap the timestamp.

Suggested change
constunreadVisitedAt=newDate(latestTurnCompletedAtMs-1).toISOString();
constunreadVisitedAt=newDate(Math.min(latestTurnCompletedAtMs-1,Date.now())).toISOString();
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around line 303:
If `latestTurn.completedAt` is in the future (clock skew), `unreadVisitedAt` will also be in the future, preventing `markThreadVisited` from updating `lastVisitedAt` until client time catches up. Consider using `Math.min(latestTurnCompletedAtMs - 1, Date.now())` to cap the timestamp.
Evidence trail:
apps/web/src/store.ts lines 296-306 (markThreadUnread function, line 303 sets unreadVisitedAt from latestTurn.completedAt - 1); apps/web/src/store.ts lines 273-291 (markThreadVisited function, line 278 defaults to Date.now(), lines 284-287 comparison that prevents update if previousVisitedAtMs >= visitedAtMs)

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumcomponents/Sidebar.tsx:240

The component subscribes to getTerminalState (a stable function reference), not the underlying terminalStateByThreadId data. When terminal state changes, this component won't re-render, causing stale terminalStatus display. Consider subscribing to the data directly, e.g., useTerminalStateStore((s) => s.terminalStateByThreadId), then access state in the render loop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Sidebar.tsx around line 240:
The component subscribes to `getTerminalState` (a stable function reference), not the underlying `terminalStateByThreadId` data. When terminal state changes, this component won't re-render, causing stale `terminalStatus` display. Consider subscribing to the data directly, e.g., `useTerminalStateStore((s) => s.terminalStateByThreadId)`, then access state in the render loop.
Evidence trail:
apps/web/src/components/Sidebar.tsx line 240: `const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);` - subscribes to function reference
apps/web/src/components/Sidebar.tsx lines 818-820: `const terminalStatus = terminalStatusFromRunningIds(getTerminalState(thread.id).runningTerminalIds);` - uses function to get data during render
apps/web/src/terminalStateStore.ts lines 51-66: Shows `getTerminalState` is a stable function that calls `get().terminalStateByThreadId[threadId]` - the function reference doesn't change when the data it accesses changes

juliusmarminge added a commit that referenced this pull request Mar 2, 2026
Integrate the draft and persisted terminal-state flow from #143 (shared thread terminal reducer plus dedicated terminal store), while preserving selector compatibility and runtime-mode switch robustness on this branch.
Made-with: Cursor
@juliusmarminge

Copy link
Copy Markdown
MemberAuthor

Superseded by #141. I merged the terminal-store work into that branch/PR so the three-store architecture now lives there (composer draft store + app state store + terminal store).

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Unstable dispatch reference causes infinite effect loop
    • Wrapped dispatch action functions in useMemo with empty deps so the dispatch object is referentially stable across renders, breaking the infinite re-render/effect cycle.
  • ✅ Fixed: Sidebar terminal status indicator not reactive to changes
    • Changed the Sidebar's terminal store subscription from the stable getTerminalState function reference to the terminalStateByThreadId data object, so changes to terminal running state trigger re-renders.
  • ✅ Fixed: No-op terminal actions always trigger store updates
    • Changed all no-op early-return paths in reduceThreadTerminalState to return the original state parameter instead of the normalized copy, allowing the reference equality check in applyTerminalAction to succeed and skip unnecessary store updates.

Create PR

Or push these changes by commenting:

@cursor push 004658797e
Preview (004658797e)
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@@ -237,7 +237,7 @@
(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
- const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);+ const terminalStateByThreadId = useTerminalStateStore((s) => s.terminalStateByThreadId);
const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId);
const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext);
const clearProjectDraftThreadId = useComposerDraftStore(
@@ -816,7 +816,7 @@
);
const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null);
const terminalStatus = terminalStatusFromRunningIds(
- getTerminalState(thread.id).runningTerminalIds,+ terminalStateByThreadId[thread.id]?.runningTerminalIds ?? [],
);
return (
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts--- a/apps/web/src/store.ts+++ b/apps/web/src/store.ts@@ -1,4 +1,4 @@-import { Fragment, type ReactNode, createElement, useEffect } from "react";+import { Fragment, type ReactNode, createElement, useEffect, useMemo } from "react";
import {
DEFAULT_MODEL,
ProviderSessionId,
@@ -416,8 +416,18 @@
threadsHydrated: s.threadsHydrated,
runtimeMode: s.runtimeMode,
}));
- return {- state,- dispatch: useAppStore.getState(),- };+ const dispatch = useMemo(() => {+ const s = useAppStore.getState();+ return {+ syncServerReadModel: s.syncServerReadModel,+ markThreadVisited: s.markThreadVisited,+ markThreadUnread: s.markThreadUnread,+ toggleProject: s.toggleProject,+ setProjectExpanded: s.setProjectExpanded,+ setError: s.setError,+ setThreadBranch: s.setThreadBranch,+ setRuntimeMode: s.setRuntimeMode,+ };+ }, []);+ return { state, dispatch };
}
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -251,7 +251,7 @@
const normalized = normalizeThreadTerminalState(state);
if (action.type === "set-open") {
- if (normalized.terminalOpen === action.open) return normalized;+ if (normalized.terminalOpen === action.open) return state;
return { ...normalized, terminalOpen: action.open };
}
@@ -261,14 +261,14 @@
action.height <= 0 ||
normalized.terminalHeight === action.height
) {
- return normalized;+ return state;
}
return { ...normalized, terminalHeight: action.height };
}
if (action.type === "set-active") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const activeTerminalGroupId =
normalized.terminalGroups.find((group) => group.terminalIds.includes(action.terminalId))?.id ??
@@ -286,7 +286,7 @@
if (action.type === "set-activity") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const runningTerminalIds = new Set(normalized.runningTerminalIds);
if (action.hasRunningSubprocess) {
@@ -298,12 +298,12 @@
}
if (!action.terminalId || action.terminalId.trim().length === 0) {
- return normalized;+ return state;
}
const isNewTerminal = !normalized.terminalIds.includes(action.terminalId);
if (isNewTerminal && normalized.terminalIds.length >= MAX_THREAD_TERMINAL_COUNT) {
- return normalized;+ return state;
}
const terminalIds = isNewTerminal

Comment threadapps/web/src/store.ts

function setThreadTerminalActivity(
thread: Thread,
terminalId: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable dispatch reference causes infinite effect loop

High Severity

useStore() returns dispatch: useAppStore.getState(), which produces a new object reference after every store update. The selector also creates a new inline object on every call, so every store change triggers a re-render. In EventRouter, this dispatch is in the useEffect dependency array. Each effect run calls syncSnapshot(), which calls dispatch.syncServerReadModel(snapshot), which updates the store, which triggers a re-render, which changes dispatch, which re-runs the effect — creating an infinite loop of network requests and event re-subscriptions.

Additional Locations (1)

Fix in CursorFix in Web

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar terminal status indicator not reactive to changes

Medium Severity

The Sidebar subscribes to useTerminalStateStore((s) => s.getTerminalState), which returns the getTerminalState function reference — a stable closure that never changes. This means the Sidebar never re-renders when terminal state (like runningTerminalIds) changes in the terminal store. The terminal running indicator in the sidebar will be stale until the component re-renders for an unrelated reason.

Additional Locations (1)

Fix in CursorFix in Web

...state,
[threadId]: next,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No-op terminal actions always trigger store updates

Low Severity

applyTerminalAction checks if (next === current) to avoid unnecessary updates, but this never succeeds. reduceThreadTerminalState always calls normalizeThreadTerminalState first, which creates a new object. So even for no-op actions (e.g., setting terminalOpen to its current value), next is a different reference than current, and the store always updates. The intended optimization is defeated.

Additional Locations (1)

Fix in CursorFix in Web

logancsack added a commit to logancsack/t3code that referenced this pull request Aug 4, 2026
Aldo Review lost 27% of its lead reviewers in the field (12 of 44 slots across remote-dev PRs pingdotgg#135-pingdotgg#143): 8 Grok Build structured-output rejections and 4 timeouts. Nothing retried, so one transient blip deleted a whole reviewer for the entire run.
Retry every failure a fresh sample could plausibly fix, against a new subprocess. Output that overflows the collection limit stays terminal because it recurs identically.
Give the swarm one wall-clock budget split into stage deadlines, so a retrying lead can never starve the verification that produces the report. The worst-case serial path was already 12 minutes against a 10-minute service cap. Delegation, the only optional stage, is skipped and reported rather than started with too little time to finish.
Carry the Grok Build error cause into reviewer diagnostics so the next failure is diagnosable.
Sign up for freeto 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.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enable terminals for local draft threads - #143

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals
Closed

Enable terminals for local draft threads#143
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Mar 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds local terminal state management for draft threads before they're promoted to server threads
  • Terminal operations (open/close, split, create, activate, resize) now work on draft threads
  • When a draft thread is promoted to a server thread, terminal state is hydrated via new HYDRATE_THREAD_TERMINALS action
  • Introduces draftThreadTerminalState.ts module with reducer pattern for managing draft terminal state

Test plan

  • Open a new draft thread and verify terminals can be opened/closed
  • Split and create new terminals in a draft thread
  • Send a message to promote the draft to a server thread and verify terminal state persists
  • Verify terminal height changes are preserved after promotion
  • Run bun test to verify new unit tests pass

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because this refactors core client state management (React reducer/context → Zustand) and moves terminal UI state out of Thread into a new persisted store, which could cause state sync/persistence regressions across threads and drafts.

Overview
Moves app state from a reducer/context to a Zustand store.store.ts now exposes pure transition helpers (e.g. markThreadUnread, setThreadBranch, setRuntimeMode) plus a Zustand-backed useStore() whose dispatch is a method API (dispatch.setError(...), dispatch.toggleProject(...), etc.).

Decouples terminal UI state from Thread and persists it separately. Terminal fields are removed from Thread (types.ts), and a new terminalStateStore.ts persists per-threadId terminal state in localStorage using a shared reducer in threadTerminalState.ts.

Updates UI to use the new dispatch API and terminal store.ChatView and Sidebar switch terminal open/height/tab/group/active/running indicators to useTerminalStateStore, while other actions are updated to call the new dispatch.* methods; root websocket terminal activity events now write directly to the terminal store. Tests are adjusted accordingly, with new unit coverage for threadTerminalState behavior.

Written by Cursor Bugbot for commit c283d2a. This will update automatically on new commits. Configure here.

Note

Move terminal UI state for local draft threads to a dedicated Zustand store and update ChatView and Sidebar to read/write via useTerminalStateStore with MAX_THREAD_TERMINAL_COUNT enforcement

Introduce useTerminalStateStore for per-thread terminal state and refactor app state to a single Zustand store; update ChatView and Sidebar to consume terminal state from the new store and remove terminal fields from Thread. Core terminal logic lives in reduceThreadTerminalState with normalization helpers.

📍Where to Start

Start with the terminal reducer and helpers in apps/web/src/threadTerminalState.ts, then see the store wiring in apps/web/src/terminalStateStore.ts and usage in apps/web/src/components/ChatView.tsx.

Macroscope summarized c283d2a.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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.

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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch t3code/enable-draft-thread-terminals

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

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Close terminal selects next active terminal differently than store
    • Added same-group preference logic (closedTerminalGroup lookup and remainingTerminalsInClosedGroup fallback) to closeDraftTerminal, matching the store's closeThreadTerminal 3-level fallback behavior.

Create PR

Or push these changes by commenting:

@cursor push 5002774b80
Preview (5002774b80)
diff --git a/apps/web/src/draftThreadTerminalState.ts b/apps/web/src/draftThreadTerminalState.ts--- a/apps/web/src/draftThreadTerminalState.ts+++ b/apps/web/src/draftThreadTerminalState.ts@@ -161,9 +161,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;

Comment threadapps/web/src/draftThreadTerminalState.ts
Comment on lines +51 to +53
return [...new Set(runningTerminalIds)]
.map((id) => id.trim())
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:51

Consider trimming before deduplicating—currently " term1 " and "term1" both survive the Set, then become identical after .trim(), producing duplicates.

Suggested change
return[...newSet(runningTerminalIds)]
.map((id)=>id.trim())
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
return[...newSet(runningTerminalIds.map((id)=>id.trim()))]
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 51-53:
Consider trimming before deduplicating—currently `" term1 "` and `"term1"` both survive the `Set`, then become identical after `.trim()`, producing duplicates.
Evidence trail:
File: apps/web/src/threadTerminalState.ts lines 51-53 at REVIEWED_COMMIT. The function `normalizeRunningTerminalIds` uses `[...new Set(runningTerminalIds)].map((id) => id.trim())`. The Set deduplicates before trim, so `" term1 "` and `"term1"` both survive the Set, then both become `"term1"` after trim, producing duplicates.

Comment threadapps/web/src/composerDraftStore.ts Outdated
juliusmarmingeand others added 3 commits March 2, 2026 10:07
Adds local terminal state management for draft threads before they're
promoted to server threads. Terminal operations (open, split, create,
close, activate, resize) now work on draft threads and hydrate into
the server thread state when the draft is promoted.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add shared threadTerminalState.ts: ThreadTerminalState type, default state,
normalize helpers, and single reduceThreadTerminalState() used by both flows
- Refactor store.ts to use shared terminal reducer and helpers; terminal
actions map to reduceThreadTerminalState(threadTerminalSlice(thread), action)
- Make draftThreadTerminalState.ts re-export shared module (same shape/actions)
- Add terminalState to composer draft store: getDraftThreadTerminalState,
setDraftThreadTerminalAction, clearDraftThreadTerminalState; draft thread
shape matches Thread terminal slice
- ChatView: move draft terminal from useState into composer draft store; use
same actions for draft and persisted; hydrate then clear draft terminal
when reconciling server state
Made-with: Cursor
…irectly
- Delete draftThreadTerminalState.ts (was only re-exporting shared module)
- Replace draftThreadTerminalState.test.ts with threadTerminalState.test.ts
using createDefaultThreadTerminalState and reduceThreadTerminalState
- Draft threads are first-class: same helpers and types as persisted threads
Made-with: Cursor
Comment on lines +149 to +151
export function createDefaultThreadTerminalState(): ThreadTerminalState {
return { ...DEFAULT_THREAD_TERMINAL_STATE };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:149

Defaults are shallow. Mutating nested arrays (e.g. terminalIds, terminalGroups) can leak into DEFAULT_THREAD_TERMINAL_STATE. Suggest deep-freezing the default and/or returning deep clones from createDefaultThreadTerminalState and getDefaultThreadTerminalState to avoid shared references.

-export function createDefaultThreadTerminalState(): ThreadTerminalState {- return { ...DEFAULT_THREAD_TERMINAL_STATE };+export function createDefaultThreadTerminalState(): ThreadTerminalState {+ return {+ ...DEFAULT_THREAD_TERMINAL_STATE,+ terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds],+ runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds],+ terminalGroups: DEFAULT_THREAD_TERMINAL_STATE.terminalGroups.map((g) => ({+ ...g,+ terminalIds: [...g.terminalIds],+ })),+ };
}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 149-151:
Defaults are shallow. Mutating nested arrays (e.g. `terminalIds`, `terminalGroups`) can leak into `DEFAULT_THREAD_TERMINAL_STATE`. Suggest deep-freezing the default and/or returning deep clones from `createDefaultThreadTerminalState` and `getDefaultThreadTerminalState` to avoid shared references.
Evidence trail:
apps/web/src/threadTerminalState.ts lines 132-155: `DEFAULT_THREAD_TERMINAL_STATE` is defined with `Object.freeze()` containing nested arrays (`terminalIds`, `runningTerminalIds`, `terminalGroups`). `createDefaultThreadTerminalState()` returns `{ ...DEFAULT_THREAD_TERMINAL_STATE }` (shallow copy). `getDefaultThreadTerminalState()` returns the frozen object directly. JavaScript's `Object.freeze()` is documented as shallow-freezing only top-level properties (MDN documentation). Usage sites in `composerDraftStore.ts` lines 683-697 show these functions being called and their results potentially stored/used.

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Close terminal lost group-aware active terminal selection
    • Restored group-aware active terminal selection in closeTerminal by first looking for siblings in the same group (remainingTerminalsInClosedGroup) before falling back to the global remainingTerminalIds list, matching the old closeThreadTerminal behavior.
  • ✅ Fixed: Dead reference equality check never short-circuits store updates
    • Replaced the dead next === current reference equality check with a structural comparison via a new threadTerminalStateEquals helper, since reduceThreadTerminalState always returns a new object through normalizeThreadTerminalState.

Create PR

Or push these changes by commenting:

@cursor push a82c89a4f7
Preview (a82c89a4f7)
diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts--- a/apps/web/src/composerDraftStore.ts+++ b/apps/web/src/composerDraftStore.ts@@ -10,6 +10,7 @@
createDefaultThreadTerminalState,
getDefaultThreadTerminalState,
reduceThreadTerminalState,
+ threadTerminalStateEquals,
type ThreadTerminalAction,
type ThreadTerminalState,
} from "./threadTerminalState";
@@ -696,7 +697,7 @@
}
const current = existing.terminalState ?? createDefaultThreadTerminalState();
const next = reduceThreadTerminalState(current, action);
- if (next === current) {+ if (threadTerminalStateEquals(next, current)) {
return state;
}
return {
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -205,9 +205,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;
@@ -242,6 +254,36 @@
}));
}
+export function threadTerminalStateEquals(a: ThreadTerminalState, b: ThreadTerminalState): boolean {+ if (a === b) return true;+ if (+ a.terminalOpen !== b.terminalOpen ||+ a.terminalHeight !== b.terminalHeight ||+ a.activeTerminalId !== b.activeTerminalId ||+ a.activeTerminalGroupId !== b.activeTerminalGroupId ||+ a.terminalIds.length !== b.terminalIds.length ||+ a.runningTerminalIds.length !== b.runningTerminalIds.length ||+ a.terminalGroups.length !== b.terminalGroups.length+ ) {+ return false;+ }+ for (let i = 0; i < a.terminalIds.length; i++) {+ if (a.terminalIds[i] !== b.terminalIds[i]) return false;+ }+ for (let i = 0; i < a.runningTerminalIds.length; i++) {+ if (a.runningTerminalIds[i] !== b.runningTerminalIds[i]) return false;+ }+ for (let i = 0; i < a.terminalGroups.length; i++) {+ const ga = a.terminalGroups[i]!;+ const gb = b.terminalGroups[i]!;+ if (ga.id !== gb.id || ga.terminalIds.length !== gb.terminalIds.length) return false;+ for (let j = 0; j < ga.terminalIds.length; j++) {+ if (ga.terminalIds[j] !== gb.terminalIds[j]) return false;+ }+ }+ return true;+}+
// ─── Single reducer for both draft and persisted thread ────────────────────
export function reduceThreadTerminalState(

Comment threadapps/web/src/threadTerminalState.ts
Comment threadapps/web/src/composerDraftStore.ts Outdated
- Main store: replace useReducer with Zustand; pure state transition
functions (syncServerReadModel, markThreadVisited, setError, etc.)
called via store methods. Persist via subscribe.
- Terminal store: single store keyed by threadId; replace
dispatchTerminalAction with direct methods (setTerminalOpen,
setTerminalHeight, splitTerminal, newTerminal, setActiveTerminal,
closeTerminal, setTerminalActivity). Pure reduceThreadTerminalState
used internally.
- Update all consumers to call dispatch.method(...) and terminal
store methods. Fix store tests to use markThreadUnread pure fn.
- Fix Fragment import in store.ts; fix unused get param for lint.
Made-with: Cursor
@juliusmarminge
juliusmarmingeforce-pushed the t3code/enable-draft-thread-terminals branch from eef2781 to c283d2aCompareMarch 2, 2026 18:24
Comment threadapps/web/src/store.ts
if (!thread.latestTurn?.completedAt) return thread;
const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt);
if (Number.isNaN(latestTurnCompletedAtMs)) return thread;
const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/store.ts:303

If latestTurn.completedAt is in the future (clock skew), unreadVisitedAt will also be in the future, preventing markThreadVisited from updating lastVisitedAt until client time catches up. Consider using Math.min(latestTurnCompletedAtMs - 1, Date.now()) to cap the timestamp.

Suggested change
constunreadVisitedAt=newDate(latestTurnCompletedAtMs-1).toISOString();
constunreadVisitedAt=newDate(Math.min(latestTurnCompletedAtMs-1,Date.now())).toISOString();
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around line 303:
If `latestTurn.completedAt` is in the future (clock skew), `unreadVisitedAt` will also be in the future, preventing `markThreadVisited` from updating `lastVisitedAt` until client time catches up. Consider using `Math.min(latestTurnCompletedAtMs - 1, Date.now())` to cap the timestamp.
Evidence trail:
apps/web/src/store.ts lines 296-306 (markThreadUnread function, line 303 sets unreadVisitedAt from latestTurn.completedAt - 1); apps/web/src/store.ts lines 273-291 (markThreadVisited function, line 278 defaults to Date.now(), lines 284-287 comparison that prevents update if previousVisitedAtMs >= visitedAtMs)

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumcomponents/Sidebar.tsx:240

The component subscribes to getTerminalState (a stable function reference), not the underlying terminalStateByThreadId data. When terminal state changes, this component won't re-render, causing stale terminalStatus display. Consider subscribing to the data directly, e.g., useTerminalStateStore((s) => s.terminalStateByThreadId), then access state in the render loop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Sidebar.tsx around line 240:
The component subscribes to `getTerminalState` (a stable function reference), not the underlying `terminalStateByThreadId` data. When terminal state changes, this component won't re-render, causing stale `terminalStatus` display. Consider subscribing to the data directly, e.g., `useTerminalStateStore((s) => s.terminalStateByThreadId)`, then access state in the render loop.
Evidence trail:
apps/web/src/components/Sidebar.tsx line 240: `const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);` - subscribes to function reference
apps/web/src/components/Sidebar.tsx lines 818-820: `const terminalStatus = terminalStatusFromRunningIds(getTerminalState(thread.id).runningTerminalIds);` - uses function to get data during render
apps/web/src/terminalStateStore.ts lines 51-66: Shows `getTerminalState` is a stable function that calls `get().terminalStateByThreadId[threadId]` - the function reference doesn't change when the data it accesses changes

juliusmarminge added a commit that referenced this pull request Mar 2, 2026
Integrate the draft and persisted terminal-state flow from #143 (shared thread terminal reducer plus dedicated terminal store), while preserving selector compatibility and runtime-mode switch robustness on this branch.
Made-with: Cursor
@juliusmarminge

Copy link
Copy Markdown
MemberAuthor

Superseded by #141. I merged the terminal-store work into that branch/PR so the three-store architecture now lives there (composer draft store + app state store + terminal store).

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Unstable dispatch reference causes infinite effect loop
    • Wrapped dispatch action functions in useMemo with empty deps so the dispatch object is referentially stable across renders, breaking the infinite re-render/effect cycle.
  • ✅ Fixed: Sidebar terminal status indicator not reactive to changes
    • Changed the Sidebar's terminal store subscription from the stable getTerminalState function reference to the terminalStateByThreadId data object, so changes to terminal running state trigger re-renders.
  • ✅ Fixed: No-op terminal actions always trigger store updates
    • Changed all no-op early-return paths in reduceThreadTerminalState to return the original state parameter instead of the normalized copy, allowing the reference equality check in applyTerminalAction to succeed and skip unnecessary store updates.

Create PR

Or push these changes by commenting:

@cursor push 004658797e
Preview (004658797e)
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@@ -237,7 +237,7 @@
(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
- const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);+ const terminalStateByThreadId = useTerminalStateStore((s) => s.terminalStateByThreadId);
const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId);
const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext);
const clearProjectDraftThreadId = useComposerDraftStore(
@@ -816,7 +816,7 @@
);
const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null);
const terminalStatus = terminalStatusFromRunningIds(
- getTerminalState(thread.id).runningTerminalIds,+ terminalStateByThreadId[thread.id]?.runningTerminalIds ?? [],
);
return (
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts--- a/apps/web/src/store.ts+++ b/apps/web/src/store.ts@@ -1,4 +1,4 @@-import { Fragment, type ReactNode, createElement, useEffect } from "react";+import { Fragment, type ReactNode, createElement, useEffect, useMemo } from "react";
import {
DEFAULT_MODEL,
ProviderSessionId,
@@ -416,8 +416,18 @@
threadsHydrated: s.threadsHydrated,
runtimeMode: s.runtimeMode,
}));
- return {- state,- dispatch: useAppStore.getState(),- };+ const dispatch = useMemo(() => {+ const s = useAppStore.getState();+ return {+ syncServerReadModel: s.syncServerReadModel,+ markThreadVisited: s.markThreadVisited,+ markThreadUnread: s.markThreadUnread,+ toggleProject: s.toggleProject,+ setProjectExpanded: s.setProjectExpanded,+ setError: s.setError,+ setThreadBranch: s.setThreadBranch,+ setRuntimeMode: s.setRuntimeMode,+ };+ }, []);+ return { state, dispatch };
}
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -251,7 +251,7 @@
const normalized = normalizeThreadTerminalState(state);
if (action.type === "set-open") {
- if (normalized.terminalOpen === action.open) return normalized;+ if (normalized.terminalOpen === action.open) return state;
return { ...normalized, terminalOpen: action.open };
}
@@ -261,14 +261,14 @@
action.height <= 0 ||
normalized.terminalHeight === action.height
) {
- return normalized;+ return state;
}
return { ...normalized, terminalHeight: action.height };
}
if (action.type === "set-active") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const activeTerminalGroupId =
normalized.terminalGroups.find((group) => group.terminalIds.includes(action.terminalId))?.id ??
@@ -286,7 +286,7 @@
if (action.type === "set-activity") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const runningTerminalIds = new Set(normalized.runningTerminalIds);
if (action.hasRunningSubprocess) {
@@ -298,12 +298,12 @@
}
if (!action.terminalId || action.terminalId.trim().length === 0) {
- return normalized;+ return state;
}
const isNewTerminal = !normalized.terminalIds.includes(action.terminalId);
if (isNewTerminal && normalized.terminalIds.length >= MAX_THREAD_TERMINAL_COUNT) {
- return normalized;+ return state;
}
const terminalIds = isNewTerminal

Comment threadapps/web/src/store.ts

function setThreadTerminalActivity(
thread: Thread,
terminalId: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable dispatch reference causes infinite effect loop

High Severity

useStore() returns dispatch: useAppStore.getState(), which produces a new object reference after every store update. The selector also creates a new inline object on every call, so every store change triggers a re-render. In EventRouter, this dispatch is in the useEffect dependency array. Each effect run calls syncSnapshot(), which calls dispatch.syncServerReadModel(snapshot), which updates the store, which triggers a re-render, which changes dispatch, which re-runs the effect — creating an infinite loop of network requests and event re-subscriptions.

Additional Locations (1)

Fix in CursorFix in Web

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar terminal status indicator not reactive to changes

Medium Severity

The Sidebar subscribes to useTerminalStateStore((s) => s.getTerminalState), which returns the getTerminalState function reference — a stable closure that never changes. This means the Sidebar never re-renders when terminal state (like runningTerminalIds) changes in the terminal store. The terminal running indicator in the sidebar will be stale until the component re-renders for an unrelated reason.

Additional Locations (1)

Fix in CursorFix in Web

...state,
[threadId]: next,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No-op terminal actions always trigger store updates

Low Severity

applyTerminalAction checks if (next === current) to avoid unnecessary updates, but this never succeeds. reduceThreadTerminalState always calls normalizeThreadTerminalState first, which creates a new object. So even for no-op actions (e.g., setting terminalOpen to its current value), next is a different reference than current, and the store always updates. The intended optimization is defeated.

Additional Locations (1)

Fix in CursorFix in Web

logancsack added a commit to logancsack/t3code that referenced this pull request Aug 4, 2026
Aldo Review lost 27% of its lead reviewers in the field (12 of 44 slots across remote-dev PRs pingdotgg#135-pingdotgg#143): 8 Grok Build structured-output rejections and 4 timeouts. Nothing retried, so one transient blip deleted a whole reviewer for the entire run.
Retry every failure a fresh sample could plausibly fix, against a new subprocess. Output that overflows the collection limit stays terminal because it recurs identically.
Give the swarm one wall-clock budget split into stage deadlines, so a retrying lead can never starve the verification that produces the report. The worst-case serial path was already 12 minutes against a 10-minute service cap. Delegation, the only optional stage, is skipped and reported rather than started with too little time to finish.
Carry the Grok Build error cause into reviewer diagnostics so the next failure is diagnosable.
Sign up for freeto 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.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Enable terminals for local draft threads - #143

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals
Closed

Enable terminals for local draft threads#143
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Mar 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds local terminal state management for draft threads before they're promoted to server threads
  • Terminal operations (open/close, split, create, activate, resize) now work on draft threads
  • When a draft thread is promoted to a server thread, terminal state is hydrated via new HYDRATE_THREAD_TERMINALS action
  • Introduces draftThreadTerminalState.ts module with reducer pattern for managing draft terminal state

Test plan

  • Open a new draft thread and verify terminals can be opened/closed
  • Split and create new terminals in a draft thread
  • Send a message to promote the draft to a server thread and verify terminal state persists
  • Verify terminal height changes are preserved after promotion
  • Run bun test to verify new unit tests pass

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because this refactors core client state management (React reducer/context → Zustand) and moves terminal UI state out of Thread into a new persisted store, which could cause state sync/persistence regressions across threads and drafts.

Overview
Moves app state from a reducer/context to a Zustand store.store.ts now exposes pure transition helpers (e.g. markThreadUnread, setThreadBranch, setRuntimeMode) plus a Zustand-backed useStore() whose dispatch is a method API (dispatch.setError(...), dispatch.toggleProject(...), etc.).

Decouples terminal UI state from Thread and persists it separately. Terminal fields are removed from Thread (types.ts), and a new terminalStateStore.ts persists per-threadId terminal state in localStorage using a shared reducer in threadTerminalState.ts.

Updates UI to use the new dispatch API and terminal store.ChatView and Sidebar switch terminal open/height/tab/group/active/running indicators to useTerminalStateStore, while other actions are updated to call the new dispatch.* methods; root websocket terminal activity events now write directly to the terminal store. Tests are adjusted accordingly, with new unit coverage for threadTerminalState behavior.

Written by Cursor Bugbot for commit c283d2a. This will update automatically on new commits. Configure here.

Note

Move terminal UI state for local draft threads to a dedicated Zustand store and update ChatView and Sidebar to read/write via useTerminalStateStore with MAX_THREAD_TERMINAL_COUNT enforcement

Introduce useTerminalStateStore for per-thread terminal state and refactor app state to a single Zustand store; update ChatView and Sidebar to consume terminal state from the new store and remove terminal fields from Thread. Core terminal logic lives in reduceThreadTerminalState with normalization helpers.

📍Where to Start

Start with the terminal reducer and helpers in apps/web/src/threadTerminalState.ts, then see the store wiring in apps/web/src/terminalStateStore.ts and usage in apps/web/src/components/ChatView.tsx.

Macroscope summarized c283d2a.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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.

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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch t3code/enable-draft-thread-terminals

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

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Close terminal selects next active terminal differently than store
    • Added same-group preference logic (closedTerminalGroup lookup and remainingTerminalsInClosedGroup fallback) to closeDraftTerminal, matching the store's closeThreadTerminal 3-level fallback behavior.

Create PR

Or push these changes by commenting:

@cursor push 5002774b80
Preview (5002774b80)
diff --git a/apps/web/src/draftThreadTerminalState.ts b/apps/web/src/draftThreadTerminalState.ts--- a/apps/web/src/draftThreadTerminalState.ts+++ b/apps/web/src/draftThreadTerminalState.ts@@ -161,9 +161,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;

Comment threadapps/web/src/draftThreadTerminalState.ts
Comment on lines +51 to +53
return [...new Set(runningTerminalIds)]
.map((id) => id.trim())
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:51

Consider trimming before deduplicating—currently " term1 " and "term1" both survive the Set, then become identical after .trim(), producing duplicates.

Suggested change
return[...newSet(runningTerminalIds)]
.map((id)=>id.trim())
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
return[...newSet(runningTerminalIds.map((id)=>id.trim()))]
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 51-53:
Consider trimming before deduplicating—currently `" term1 "` and `"term1"` both survive the `Set`, then become identical after `.trim()`, producing duplicates.
Evidence trail:
File: apps/web/src/threadTerminalState.ts lines 51-53 at REVIEWED_COMMIT. The function `normalizeRunningTerminalIds` uses `[...new Set(runningTerminalIds)].map((id) => id.trim())`. The Set deduplicates before trim, so `" term1 "` and `"term1"` both survive the Set, then both become `"term1"` after trim, producing duplicates.

Comment threadapps/web/src/composerDraftStore.ts Outdated
juliusmarmingeand others added 3 commits March 2, 2026 10:07
Adds local terminal state management for draft threads before they're
promoted to server threads. Terminal operations (open, split, create,
close, activate, resize) now work on draft threads and hydrate into
the server thread state when the draft is promoted.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add shared threadTerminalState.ts: ThreadTerminalState type, default state,
normalize helpers, and single reduceThreadTerminalState() used by both flows
- Refactor store.ts to use shared terminal reducer and helpers; terminal
actions map to reduceThreadTerminalState(threadTerminalSlice(thread), action)
- Make draftThreadTerminalState.ts re-export shared module (same shape/actions)
- Add terminalState to composer draft store: getDraftThreadTerminalState,
setDraftThreadTerminalAction, clearDraftThreadTerminalState; draft thread
shape matches Thread terminal slice
- ChatView: move draft terminal from useState into composer draft store; use
same actions for draft and persisted; hydrate then clear draft terminal
when reconciling server state
Made-with: Cursor
…irectly
- Delete draftThreadTerminalState.ts (was only re-exporting shared module)
- Replace draftThreadTerminalState.test.ts with threadTerminalState.test.ts
using createDefaultThreadTerminalState and reduceThreadTerminalState
- Draft threads are first-class: same helpers and types as persisted threads
Made-with: Cursor
Comment on lines +149 to +151
export function createDefaultThreadTerminalState(): ThreadTerminalState {
return { ...DEFAULT_THREAD_TERMINAL_STATE };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:149

Defaults are shallow. Mutating nested arrays (e.g. terminalIds, terminalGroups) can leak into DEFAULT_THREAD_TERMINAL_STATE. Suggest deep-freezing the default and/or returning deep clones from createDefaultThreadTerminalState and getDefaultThreadTerminalState to avoid shared references.

-export function createDefaultThreadTerminalState(): ThreadTerminalState {- return { ...DEFAULT_THREAD_TERMINAL_STATE };+export function createDefaultThreadTerminalState(): ThreadTerminalState {+ return {+ ...DEFAULT_THREAD_TERMINAL_STATE,+ terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds],+ runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds],+ terminalGroups: DEFAULT_THREAD_TERMINAL_STATE.terminalGroups.map((g) => ({+ ...g,+ terminalIds: [...g.terminalIds],+ })),+ };
}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 149-151:
Defaults are shallow. Mutating nested arrays (e.g. `terminalIds`, `terminalGroups`) can leak into `DEFAULT_THREAD_TERMINAL_STATE`. Suggest deep-freezing the default and/or returning deep clones from `createDefaultThreadTerminalState` and `getDefaultThreadTerminalState` to avoid shared references.
Evidence trail:
apps/web/src/threadTerminalState.ts lines 132-155: `DEFAULT_THREAD_TERMINAL_STATE` is defined with `Object.freeze()` containing nested arrays (`terminalIds`, `runningTerminalIds`, `terminalGroups`). `createDefaultThreadTerminalState()` returns `{ ...DEFAULT_THREAD_TERMINAL_STATE }` (shallow copy). `getDefaultThreadTerminalState()` returns the frozen object directly. JavaScript's `Object.freeze()` is documented as shallow-freezing only top-level properties (MDN documentation). Usage sites in `composerDraftStore.ts` lines 683-697 show these functions being called and their results potentially stored/used.

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Close terminal lost group-aware active terminal selection
    • Restored group-aware active terminal selection in closeTerminal by first looking for siblings in the same group (remainingTerminalsInClosedGroup) before falling back to the global remainingTerminalIds list, matching the old closeThreadTerminal behavior.
  • ✅ Fixed: Dead reference equality check never short-circuits store updates
    • Replaced the dead next === current reference equality check with a structural comparison via a new threadTerminalStateEquals helper, since reduceThreadTerminalState always returns a new object through normalizeThreadTerminalState.

Create PR

Or push these changes by commenting:

@cursor push a82c89a4f7
Preview (a82c89a4f7)
diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts--- a/apps/web/src/composerDraftStore.ts+++ b/apps/web/src/composerDraftStore.ts@@ -10,6 +10,7 @@
createDefaultThreadTerminalState,
getDefaultThreadTerminalState,
reduceThreadTerminalState,
+ threadTerminalStateEquals,
type ThreadTerminalAction,
type ThreadTerminalState,
} from "./threadTerminalState";
@@ -696,7 +697,7 @@
}
const current = existing.terminalState ?? createDefaultThreadTerminalState();
const next = reduceThreadTerminalState(current, action);
- if (next === current) {+ if (threadTerminalStateEquals(next, current)) {
return state;
}
return {
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -205,9 +205,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;
@@ -242,6 +254,36 @@
}));
}
+export function threadTerminalStateEquals(a: ThreadTerminalState, b: ThreadTerminalState): boolean {+ if (a === b) return true;+ if (+ a.terminalOpen !== b.terminalOpen ||+ a.terminalHeight !== b.terminalHeight ||+ a.activeTerminalId !== b.activeTerminalId ||+ a.activeTerminalGroupId !== b.activeTerminalGroupId ||+ a.terminalIds.length !== b.terminalIds.length ||+ a.runningTerminalIds.length !== b.runningTerminalIds.length ||+ a.terminalGroups.length !== b.terminalGroups.length+ ) {+ return false;+ }+ for (let i = 0; i < a.terminalIds.length; i++) {+ if (a.terminalIds[i] !== b.terminalIds[i]) return false;+ }+ for (let i = 0; i < a.runningTerminalIds.length; i++) {+ if (a.runningTerminalIds[i] !== b.runningTerminalIds[i]) return false;+ }+ for (let i = 0; i < a.terminalGroups.length; i++) {+ const ga = a.terminalGroups[i]!;+ const gb = b.terminalGroups[i]!;+ if (ga.id !== gb.id || ga.terminalIds.length !== gb.terminalIds.length) return false;+ for (let j = 0; j < ga.terminalIds.length; j++) {+ if (ga.terminalIds[j] !== gb.terminalIds[j]) return false;+ }+ }+ return true;+}+
// ─── Single reducer for both draft and persisted thread ────────────────────
export function reduceThreadTerminalState(

Comment threadapps/web/src/threadTerminalState.ts
Comment threadapps/web/src/composerDraftStore.ts Outdated
- Main store: replace useReducer with Zustand; pure state transition
functions (syncServerReadModel, markThreadVisited, setError, etc.)
called via store methods. Persist via subscribe.
- Terminal store: single store keyed by threadId; replace
dispatchTerminalAction with direct methods (setTerminalOpen,
setTerminalHeight, splitTerminal, newTerminal, setActiveTerminal,
closeTerminal, setTerminalActivity). Pure reduceThreadTerminalState
used internally.
- Update all consumers to call dispatch.method(...) and terminal
store methods. Fix store tests to use markThreadUnread pure fn.
- Fix Fragment import in store.ts; fix unused get param for lint.
Made-with: Cursor
@juliusmarminge
juliusmarmingeforce-pushed the t3code/enable-draft-thread-terminals branch from eef2781 to c283d2aCompareMarch 2, 2026 18:24
Comment threadapps/web/src/store.ts
if (!thread.latestTurn?.completedAt) return thread;
const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt);
if (Number.isNaN(latestTurnCompletedAtMs)) return thread;
const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/store.ts:303

If latestTurn.completedAt is in the future (clock skew), unreadVisitedAt will also be in the future, preventing markThreadVisited from updating lastVisitedAt until client time catches up. Consider using Math.min(latestTurnCompletedAtMs - 1, Date.now()) to cap the timestamp.

Suggested change
constunreadVisitedAt=newDate(latestTurnCompletedAtMs-1).toISOString();
constunreadVisitedAt=newDate(Math.min(latestTurnCompletedAtMs-1,Date.now())).toISOString();
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around line 303:
If `latestTurn.completedAt` is in the future (clock skew), `unreadVisitedAt` will also be in the future, preventing `markThreadVisited` from updating `lastVisitedAt` until client time catches up. Consider using `Math.min(latestTurnCompletedAtMs - 1, Date.now())` to cap the timestamp.
Evidence trail:
apps/web/src/store.ts lines 296-306 (markThreadUnread function, line 303 sets unreadVisitedAt from latestTurn.completedAt - 1); apps/web/src/store.ts lines 273-291 (markThreadVisited function, line 278 defaults to Date.now(), lines 284-287 comparison that prevents update if previousVisitedAtMs >= visitedAtMs)

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumcomponents/Sidebar.tsx:240

The component subscribes to getTerminalState (a stable function reference), not the underlying terminalStateByThreadId data. When terminal state changes, this component won't re-render, causing stale terminalStatus display. Consider subscribing to the data directly, e.g., useTerminalStateStore((s) => s.terminalStateByThreadId), then access state in the render loop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Sidebar.tsx around line 240:
The component subscribes to `getTerminalState` (a stable function reference), not the underlying `terminalStateByThreadId` data. When terminal state changes, this component won't re-render, causing stale `terminalStatus` display. Consider subscribing to the data directly, e.g., `useTerminalStateStore((s) => s.terminalStateByThreadId)`, then access state in the render loop.
Evidence trail:
apps/web/src/components/Sidebar.tsx line 240: `const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);` - subscribes to function reference
apps/web/src/components/Sidebar.tsx lines 818-820: `const terminalStatus = terminalStatusFromRunningIds(getTerminalState(thread.id).runningTerminalIds);` - uses function to get data during render
apps/web/src/terminalStateStore.ts lines 51-66: Shows `getTerminalState` is a stable function that calls `get().terminalStateByThreadId[threadId]` - the function reference doesn't change when the data it accesses changes

juliusmarminge added a commit that referenced this pull request Mar 2, 2026
Integrate the draft and persisted terminal-state flow from #143 (shared thread terminal reducer plus dedicated terminal store), while preserving selector compatibility and runtime-mode switch robustness on this branch.
Made-with: Cursor
@juliusmarminge

Copy link
Copy Markdown
MemberAuthor

Superseded by #141. I merged the terminal-store work into that branch/PR so the three-store architecture now lives there (composer draft store + app state store + terminal store).

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Unstable dispatch reference causes infinite effect loop
    • Wrapped dispatch action functions in useMemo with empty deps so the dispatch object is referentially stable across renders, breaking the infinite re-render/effect cycle.
  • ✅ Fixed: Sidebar terminal status indicator not reactive to changes
    • Changed the Sidebar's terminal store subscription from the stable getTerminalState function reference to the terminalStateByThreadId data object, so changes to terminal running state trigger re-renders.
  • ✅ Fixed: No-op terminal actions always trigger store updates
    • Changed all no-op early-return paths in reduceThreadTerminalState to return the original state parameter instead of the normalized copy, allowing the reference equality check in applyTerminalAction to succeed and skip unnecessary store updates.

Create PR

Or push these changes by commenting:

@cursor push 004658797e
Preview (004658797e)
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@@ -237,7 +237,7 @@
(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
- const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);+ const terminalStateByThreadId = useTerminalStateStore((s) => s.terminalStateByThreadId);
const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId);
const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext);
const clearProjectDraftThreadId = useComposerDraftStore(
@@ -816,7 +816,7 @@
);
const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null);
const terminalStatus = terminalStatusFromRunningIds(
- getTerminalState(thread.id).runningTerminalIds,+ terminalStateByThreadId[thread.id]?.runningTerminalIds ?? [],
);
return (
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts--- a/apps/web/src/store.ts+++ b/apps/web/src/store.ts@@ -1,4 +1,4 @@-import { Fragment, type ReactNode, createElement, useEffect } from "react";+import { Fragment, type ReactNode, createElement, useEffect, useMemo } from "react";
import {
DEFAULT_MODEL,
ProviderSessionId,
@@ -416,8 +416,18 @@
threadsHydrated: s.threadsHydrated,
runtimeMode: s.runtimeMode,
}));
- return {- state,- dispatch: useAppStore.getState(),- };+ const dispatch = useMemo(() => {+ const s = useAppStore.getState();+ return {+ syncServerReadModel: s.syncServerReadModel,+ markThreadVisited: s.markThreadVisited,+ markThreadUnread: s.markThreadUnread,+ toggleProject: s.toggleProject,+ setProjectExpanded: s.setProjectExpanded,+ setError: s.setError,+ setThreadBranch: s.setThreadBranch,+ setRuntimeMode: s.setRuntimeMode,+ };+ }, []);+ return { state, dispatch };
}
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -251,7 +251,7 @@
const normalized = normalizeThreadTerminalState(state);
if (action.type === "set-open") {
- if (normalized.terminalOpen === action.open) return normalized;+ if (normalized.terminalOpen === action.open) return state;
return { ...normalized, terminalOpen: action.open };
}
@@ -261,14 +261,14 @@
action.height <= 0 ||
normalized.terminalHeight === action.height
) {
- return normalized;+ return state;
}
return { ...normalized, terminalHeight: action.height };
}
if (action.type === "set-active") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const activeTerminalGroupId =
normalized.terminalGroups.find((group) => group.terminalIds.includes(action.terminalId))?.id ??
@@ -286,7 +286,7 @@
if (action.type === "set-activity") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const runningTerminalIds = new Set(normalized.runningTerminalIds);
if (action.hasRunningSubprocess) {
@@ -298,12 +298,12 @@
}
if (!action.terminalId || action.terminalId.trim().length === 0) {
- return normalized;+ return state;
}
const isNewTerminal = !normalized.terminalIds.includes(action.terminalId);
if (isNewTerminal && normalized.terminalIds.length >= MAX_THREAD_TERMINAL_COUNT) {
- return normalized;+ return state;
}
const terminalIds = isNewTerminal

Comment threadapps/web/src/store.ts

function setThreadTerminalActivity(
thread: Thread,
terminalId: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable dispatch reference causes infinite effect loop

High Severity

useStore() returns dispatch: useAppStore.getState(), which produces a new object reference after every store update. The selector also creates a new inline object on every call, so every store change triggers a re-render. In EventRouter, this dispatch is in the useEffect dependency array. Each effect run calls syncSnapshot(), which calls dispatch.syncServerReadModel(snapshot), which updates the store, which triggers a re-render, which changes dispatch, which re-runs the effect — creating an infinite loop of network requests and event re-subscriptions.

Additional Locations (1)

Fix in CursorFix in Web

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar terminal status indicator not reactive to changes

Medium Severity

The Sidebar subscribes to useTerminalStateStore((s) => s.getTerminalState), which returns the getTerminalState function reference — a stable closure that never changes. This means the Sidebar never re-renders when terminal state (like runningTerminalIds) changes in the terminal store. The terminal running indicator in the sidebar will be stale until the component re-renders for an unrelated reason.

Additional Locations (1)

Fix in CursorFix in Web

...state,
[threadId]: next,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No-op terminal actions always trigger store updates

Low Severity

applyTerminalAction checks if (next === current) to avoid unnecessary updates, but this never succeeds. reduceThreadTerminalState always calls normalizeThreadTerminalState first, which creates a new object. So even for no-op actions (e.g., setting terminalOpen to its current value), next is a different reference than current, and the store always updates. The intended optimization is defeated.

Additional Locations (1)

Fix in CursorFix in Web

logancsack added a commit to logancsack/t3code that referenced this pull request Aug 4, 2026
Aldo Review lost 27% of its lead reviewers in the field (12 of 44 slots across remote-dev PRs pingdotgg#135-pingdotgg#143): 8 Grok Build structured-output rejections and 4 timeouts. Nothing retried, so one transient blip deleted a whole reviewer for the entire run.
Retry every failure a fresh sample could plausibly fix, against a new subprocess. Output that overflows the collection limit stays terminal because it recurs identically.
Give the swarm one wall-clock budget split into stage deadlines, so a retrying lead can never starve the verification that produces the report. The worst-case serial path was already 12 minutes against a 10-minute service cap. Delegation, the only optional stage, is skipped and reported rather than started with too little time to finish.
Carry the Grok Build error cause into reviewer diagnostics so the next failure is diagnosable.
Sign up for freeto 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.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enable terminals for local draft threads - #143

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals
Closed

Enable terminals for local draft threads#143
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Mar 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds local terminal state management for draft threads before they're promoted to server threads
  • Terminal operations (open/close, split, create, activate, resize) now work on draft threads
  • When a draft thread is promoted to a server thread, terminal state is hydrated via new HYDRATE_THREAD_TERMINALS action
  • Introduces draftThreadTerminalState.ts module with reducer pattern for managing draft terminal state

Test plan

  • Open a new draft thread and verify terminals can be opened/closed
  • Split and create new terminals in a draft thread
  • Send a message to promote the draft to a server thread and verify terminal state persists
  • Verify terminal height changes are preserved after promotion
  • Run bun test to verify new unit tests pass

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because this refactors core client state management (React reducer/context → Zustand) and moves terminal UI state out of Thread into a new persisted store, which could cause state sync/persistence regressions across threads and drafts.

Overview
Moves app state from a reducer/context to a Zustand store.store.ts now exposes pure transition helpers (e.g. markThreadUnread, setThreadBranch, setRuntimeMode) plus a Zustand-backed useStore() whose dispatch is a method API (dispatch.setError(...), dispatch.toggleProject(...), etc.).

Decouples terminal UI state from Thread and persists it separately. Terminal fields are removed from Thread (types.ts), and a new terminalStateStore.ts persists per-threadId terminal state in localStorage using a shared reducer in threadTerminalState.ts.

Updates UI to use the new dispatch API and terminal store.ChatView and Sidebar switch terminal open/height/tab/group/active/running indicators to useTerminalStateStore, while other actions are updated to call the new dispatch.* methods; root websocket terminal activity events now write directly to the terminal store. Tests are adjusted accordingly, with new unit coverage for threadTerminalState behavior.

Written by Cursor Bugbot for commit c283d2a. This will update automatically on new commits. Configure here.

Note

Move terminal UI state for local draft threads to a dedicated Zustand store and update ChatView and Sidebar to read/write via useTerminalStateStore with MAX_THREAD_TERMINAL_COUNT enforcement

Introduce useTerminalStateStore for per-thread terminal state and refactor app state to a single Zustand store; update ChatView and Sidebar to consume terminal state from the new store and remove terminal fields from Thread. Core terminal logic lives in reduceThreadTerminalState with normalization helpers.

📍Where to Start

Start with the terminal reducer and helpers in apps/web/src/threadTerminalState.ts, then see the store wiring in apps/web/src/terminalStateStore.ts and usage in apps/web/src/components/ChatView.tsx.

Macroscope summarized c283d2a.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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.

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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch t3code/enable-draft-thread-terminals

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

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Close terminal selects next active terminal differently than store
    • Added same-group preference logic (closedTerminalGroup lookup and remainingTerminalsInClosedGroup fallback) to closeDraftTerminal, matching the store's closeThreadTerminal 3-level fallback behavior.

Create PR

Or push these changes by commenting:

@cursor push 5002774b80
Preview (5002774b80)
diff --git a/apps/web/src/draftThreadTerminalState.ts b/apps/web/src/draftThreadTerminalState.ts--- a/apps/web/src/draftThreadTerminalState.ts+++ b/apps/web/src/draftThreadTerminalState.ts@@ -161,9 +161,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;

Comment threadapps/web/src/draftThreadTerminalState.ts
Comment on lines +51 to +53
return [...new Set(runningTerminalIds)]
.map((id) => id.trim())
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:51

Consider trimming before deduplicating—currently " term1 " and "term1" both survive the Set, then become identical after .trim(), producing duplicates.

Suggested change
return[...newSet(runningTerminalIds)]
.map((id)=>id.trim())
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
return[...newSet(runningTerminalIds.map((id)=>id.trim()))]
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 51-53:
Consider trimming before deduplicating—currently `" term1 "` and `"term1"` both survive the `Set`, then become identical after `.trim()`, producing duplicates.
Evidence trail:
File: apps/web/src/threadTerminalState.ts lines 51-53 at REVIEWED_COMMIT. The function `normalizeRunningTerminalIds` uses `[...new Set(runningTerminalIds)].map((id) => id.trim())`. The Set deduplicates before trim, so `" term1 "` and `"term1"` both survive the Set, then both become `"term1"` after trim, producing duplicates.

Comment threadapps/web/src/composerDraftStore.ts Outdated
juliusmarmingeand others added 3 commits March 2, 2026 10:07
Adds local terminal state management for draft threads before they're
promoted to server threads. Terminal operations (open, split, create,
close, activate, resize) now work on draft threads and hydrate into
the server thread state when the draft is promoted.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add shared threadTerminalState.ts: ThreadTerminalState type, default state,
normalize helpers, and single reduceThreadTerminalState() used by both flows
- Refactor store.ts to use shared terminal reducer and helpers; terminal
actions map to reduceThreadTerminalState(threadTerminalSlice(thread), action)
- Make draftThreadTerminalState.ts re-export shared module (same shape/actions)
- Add terminalState to composer draft store: getDraftThreadTerminalState,
setDraftThreadTerminalAction, clearDraftThreadTerminalState; draft thread
shape matches Thread terminal slice
- ChatView: move draft terminal from useState into composer draft store; use
same actions for draft and persisted; hydrate then clear draft terminal
when reconciling server state
Made-with: Cursor
…irectly
- Delete draftThreadTerminalState.ts (was only re-exporting shared module)
- Replace draftThreadTerminalState.test.ts with threadTerminalState.test.ts
using createDefaultThreadTerminalState and reduceThreadTerminalState
- Draft threads are first-class: same helpers and types as persisted threads
Made-with: Cursor
Comment on lines +149 to +151
export function createDefaultThreadTerminalState(): ThreadTerminalState {
return { ...DEFAULT_THREAD_TERMINAL_STATE };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:149

Defaults are shallow. Mutating nested arrays (e.g. terminalIds, terminalGroups) can leak into DEFAULT_THREAD_TERMINAL_STATE. Suggest deep-freezing the default and/or returning deep clones from createDefaultThreadTerminalState and getDefaultThreadTerminalState to avoid shared references.

-export function createDefaultThreadTerminalState(): ThreadTerminalState {- return { ...DEFAULT_THREAD_TERMINAL_STATE };+export function createDefaultThreadTerminalState(): ThreadTerminalState {+ return {+ ...DEFAULT_THREAD_TERMINAL_STATE,+ terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds],+ runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds],+ terminalGroups: DEFAULT_THREAD_TERMINAL_STATE.terminalGroups.map((g) => ({+ ...g,+ terminalIds: [...g.terminalIds],+ })),+ };
}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 149-151:
Defaults are shallow. Mutating nested arrays (e.g. `terminalIds`, `terminalGroups`) can leak into `DEFAULT_THREAD_TERMINAL_STATE`. Suggest deep-freezing the default and/or returning deep clones from `createDefaultThreadTerminalState` and `getDefaultThreadTerminalState` to avoid shared references.
Evidence trail:
apps/web/src/threadTerminalState.ts lines 132-155: `DEFAULT_THREAD_TERMINAL_STATE` is defined with `Object.freeze()` containing nested arrays (`terminalIds`, `runningTerminalIds`, `terminalGroups`). `createDefaultThreadTerminalState()` returns `{ ...DEFAULT_THREAD_TERMINAL_STATE }` (shallow copy). `getDefaultThreadTerminalState()` returns the frozen object directly. JavaScript's `Object.freeze()` is documented as shallow-freezing only top-level properties (MDN documentation). Usage sites in `composerDraftStore.ts` lines 683-697 show these functions being called and their results potentially stored/used.

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Close terminal lost group-aware active terminal selection
    • Restored group-aware active terminal selection in closeTerminal by first looking for siblings in the same group (remainingTerminalsInClosedGroup) before falling back to the global remainingTerminalIds list, matching the old closeThreadTerminal behavior.
  • ✅ Fixed: Dead reference equality check never short-circuits store updates
    • Replaced the dead next === current reference equality check with a structural comparison via a new threadTerminalStateEquals helper, since reduceThreadTerminalState always returns a new object through normalizeThreadTerminalState.

Create PR

Or push these changes by commenting:

@cursor push a82c89a4f7
Preview (a82c89a4f7)
diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts--- a/apps/web/src/composerDraftStore.ts+++ b/apps/web/src/composerDraftStore.ts@@ -10,6 +10,7 @@
createDefaultThreadTerminalState,
getDefaultThreadTerminalState,
reduceThreadTerminalState,
+ threadTerminalStateEquals,
type ThreadTerminalAction,
type ThreadTerminalState,
} from "./threadTerminalState";
@@ -696,7 +697,7 @@
}
const current = existing.terminalState ?? createDefaultThreadTerminalState();
const next = reduceThreadTerminalState(current, action);
- if (next === current) {+ if (threadTerminalStateEquals(next, current)) {
return state;
}
return {
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -205,9 +205,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;
@@ -242,6 +254,36 @@
}));
}
+export function threadTerminalStateEquals(a: ThreadTerminalState, b: ThreadTerminalState): boolean {+ if (a === b) return true;+ if (+ a.terminalOpen !== b.terminalOpen ||+ a.terminalHeight !== b.terminalHeight ||+ a.activeTerminalId !== b.activeTerminalId ||+ a.activeTerminalGroupId !== b.activeTerminalGroupId ||+ a.terminalIds.length !== b.terminalIds.length ||+ a.runningTerminalIds.length !== b.runningTerminalIds.length ||+ a.terminalGroups.length !== b.terminalGroups.length+ ) {+ return false;+ }+ for (let i = 0; i < a.terminalIds.length; i++) {+ if (a.terminalIds[i] !== b.terminalIds[i]) return false;+ }+ for (let i = 0; i < a.runningTerminalIds.length; i++) {+ if (a.runningTerminalIds[i] !== b.runningTerminalIds[i]) return false;+ }+ for (let i = 0; i < a.terminalGroups.length; i++) {+ const ga = a.terminalGroups[i]!;+ const gb = b.terminalGroups[i]!;+ if (ga.id !== gb.id || ga.terminalIds.length !== gb.terminalIds.length) return false;+ for (let j = 0; j < ga.terminalIds.length; j++) {+ if (ga.terminalIds[j] !== gb.terminalIds[j]) return false;+ }+ }+ return true;+}+
// ─── Single reducer for both draft and persisted thread ────────────────────
export function reduceThreadTerminalState(

Comment threadapps/web/src/threadTerminalState.ts
Comment threadapps/web/src/composerDraftStore.ts Outdated
- Main store: replace useReducer with Zustand; pure state transition
functions (syncServerReadModel, markThreadVisited, setError, etc.)
called via store methods. Persist via subscribe.
- Terminal store: single store keyed by threadId; replace
dispatchTerminalAction with direct methods (setTerminalOpen,
setTerminalHeight, splitTerminal, newTerminal, setActiveTerminal,
closeTerminal, setTerminalActivity). Pure reduceThreadTerminalState
used internally.
- Update all consumers to call dispatch.method(...) and terminal
store methods. Fix store tests to use markThreadUnread pure fn.
- Fix Fragment import in store.ts; fix unused get param for lint.
Made-with: Cursor
@juliusmarminge
juliusmarmingeforce-pushed the t3code/enable-draft-thread-terminals branch from eef2781 to c283d2aCompareMarch 2, 2026 18:24
Comment threadapps/web/src/store.ts
if (!thread.latestTurn?.completedAt) return thread;
const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt);
if (Number.isNaN(latestTurnCompletedAtMs)) return thread;
const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/store.ts:303

If latestTurn.completedAt is in the future (clock skew), unreadVisitedAt will also be in the future, preventing markThreadVisited from updating lastVisitedAt until client time catches up. Consider using Math.min(latestTurnCompletedAtMs - 1, Date.now()) to cap the timestamp.

Suggested change
constunreadVisitedAt=newDate(latestTurnCompletedAtMs-1).toISOString();
constunreadVisitedAt=newDate(Math.min(latestTurnCompletedAtMs-1,Date.now())).toISOString();
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around line 303:
If `latestTurn.completedAt` is in the future (clock skew), `unreadVisitedAt` will also be in the future, preventing `markThreadVisited` from updating `lastVisitedAt` until client time catches up. Consider using `Math.min(latestTurnCompletedAtMs - 1, Date.now())` to cap the timestamp.
Evidence trail:
apps/web/src/store.ts lines 296-306 (markThreadUnread function, line 303 sets unreadVisitedAt from latestTurn.completedAt - 1); apps/web/src/store.ts lines 273-291 (markThreadVisited function, line 278 defaults to Date.now(), lines 284-287 comparison that prevents update if previousVisitedAtMs >= visitedAtMs)

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumcomponents/Sidebar.tsx:240

The component subscribes to getTerminalState (a stable function reference), not the underlying terminalStateByThreadId data. When terminal state changes, this component won't re-render, causing stale terminalStatus display. Consider subscribing to the data directly, e.g., useTerminalStateStore((s) => s.terminalStateByThreadId), then access state in the render loop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Sidebar.tsx around line 240:
The component subscribes to `getTerminalState` (a stable function reference), not the underlying `terminalStateByThreadId` data. When terminal state changes, this component won't re-render, causing stale `terminalStatus` display. Consider subscribing to the data directly, e.g., `useTerminalStateStore((s) => s.terminalStateByThreadId)`, then access state in the render loop.
Evidence trail:
apps/web/src/components/Sidebar.tsx line 240: `const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);` - subscribes to function reference
apps/web/src/components/Sidebar.tsx lines 818-820: `const terminalStatus = terminalStatusFromRunningIds(getTerminalState(thread.id).runningTerminalIds);` - uses function to get data during render
apps/web/src/terminalStateStore.ts lines 51-66: Shows `getTerminalState` is a stable function that calls `get().terminalStateByThreadId[threadId]` - the function reference doesn't change when the data it accesses changes

juliusmarminge added a commit that referenced this pull request Mar 2, 2026
Integrate the draft and persisted terminal-state flow from #143 (shared thread terminal reducer plus dedicated terminal store), while preserving selector compatibility and runtime-mode switch robustness on this branch.
Made-with: Cursor
@juliusmarminge

Copy link
Copy Markdown
MemberAuthor

Superseded by #141. I merged the terminal-store work into that branch/PR so the three-store architecture now lives there (composer draft store + app state store + terminal store).

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Unstable dispatch reference causes infinite effect loop
    • Wrapped dispatch action functions in useMemo with empty deps so the dispatch object is referentially stable across renders, breaking the infinite re-render/effect cycle.
  • ✅ Fixed: Sidebar terminal status indicator not reactive to changes
    • Changed the Sidebar's terminal store subscription from the stable getTerminalState function reference to the terminalStateByThreadId data object, so changes to terminal running state trigger re-renders.
  • ✅ Fixed: No-op terminal actions always trigger store updates
    • Changed all no-op early-return paths in reduceThreadTerminalState to return the original state parameter instead of the normalized copy, allowing the reference equality check in applyTerminalAction to succeed and skip unnecessary store updates.

Create PR

Or push these changes by commenting:

@cursor push 004658797e
Preview (004658797e)
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@@ -237,7 +237,7 @@
(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
- const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);+ const terminalStateByThreadId = useTerminalStateStore((s) => s.terminalStateByThreadId);
const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId);
const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext);
const clearProjectDraftThreadId = useComposerDraftStore(
@@ -816,7 +816,7 @@
);
const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null);
const terminalStatus = terminalStatusFromRunningIds(
- getTerminalState(thread.id).runningTerminalIds,+ terminalStateByThreadId[thread.id]?.runningTerminalIds ?? [],
);
return (
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts--- a/apps/web/src/store.ts+++ b/apps/web/src/store.ts@@ -1,4 +1,4 @@-import { Fragment, type ReactNode, createElement, useEffect } from "react";+import { Fragment, type ReactNode, createElement, useEffect, useMemo } from "react";
import {
DEFAULT_MODEL,
ProviderSessionId,
@@ -416,8 +416,18 @@
threadsHydrated: s.threadsHydrated,
runtimeMode: s.runtimeMode,
}));
- return {- state,- dispatch: useAppStore.getState(),- };+ const dispatch = useMemo(() => {+ const s = useAppStore.getState();+ return {+ syncServerReadModel: s.syncServerReadModel,+ markThreadVisited: s.markThreadVisited,+ markThreadUnread: s.markThreadUnread,+ toggleProject: s.toggleProject,+ setProjectExpanded: s.setProjectExpanded,+ setError: s.setError,+ setThreadBranch: s.setThreadBranch,+ setRuntimeMode: s.setRuntimeMode,+ };+ }, []);+ return { state, dispatch };
}
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -251,7 +251,7 @@
const normalized = normalizeThreadTerminalState(state);
if (action.type === "set-open") {
- if (normalized.terminalOpen === action.open) return normalized;+ if (normalized.terminalOpen === action.open) return state;
return { ...normalized, terminalOpen: action.open };
}
@@ -261,14 +261,14 @@
action.height <= 0 ||
normalized.terminalHeight === action.height
) {
- return normalized;+ return state;
}
return { ...normalized, terminalHeight: action.height };
}
if (action.type === "set-active") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const activeTerminalGroupId =
normalized.terminalGroups.find((group) => group.terminalIds.includes(action.terminalId))?.id ??
@@ -286,7 +286,7 @@
if (action.type === "set-activity") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const runningTerminalIds = new Set(normalized.runningTerminalIds);
if (action.hasRunningSubprocess) {
@@ -298,12 +298,12 @@
}
if (!action.terminalId || action.terminalId.trim().length === 0) {
- return normalized;+ return state;
}
const isNewTerminal = !normalized.terminalIds.includes(action.terminalId);
if (isNewTerminal && normalized.terminalIds.length >= MAX_THREAD_TERMINAL_COUNT) {
- return normalized;+ return state;
}
const terminalIds = isNewTerminal

Comment threadapps/web/src/store.ts

function setThreadTerminalActivity(
thread: Thread,
terminalId: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable dispatch reference causes infinite effect loop

High Severity

useStore() returns dispatch: useAppStore.getState(), which produces a new object reference after every store update. The selector also creates a new inline object on every call, so every store change triggers a re-render. In EventRouter, this dispatch is in the useEffect dependency array. Each effect run calls syncSnapshot(), which calls dispatch.syncServerReadModel(snapshot), which updates the store, which triggers a re-render, which changes dispatch, which re-runs the effect — creating an infinite loop of network requests and event re-subscriptions.

Additional Locations (1)

Fix in CursorFix in Web

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar terminal status indicator not reactive to changes

Medium Severity

The Sidebar subscribes to useTerminalStateStore((s) => s.getTerminalState), which returns the getTerminalState function reference — a stable closure that never changes. This means the Sidebar never re-renders when terminal state (like runningTerminalIds) changes in the terminal store. The terminal running indicator in the sidebar will be stale until the component re-renders for an unrelated reason.

Additional Locations (1)

Fix in CursorFix in Web

...state,
[threadId]: next,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No-op terminal actions always trigger store updates

Low Severity

applyTerminalAction checks if (next === current) to avoid unnecessary updates, but this never succeeds. reduceThreadTerminalState always calls normalizeThreadTerminalState first, which creates a new object. So even for no-op actions (e.g., setting terminalOpen to its current value), next is a different reference than current, and the store always updates. The intended optimization is defeated.

Additional Locations (1)

Fix in CursorFix in Web

logancsack added a commit to logancsack/t3code that referenced this pull request Aug 4, 2026
Aldo Review lost 27% of its lead reviewers in the field (12 of 44 slots across remote-dev PRs pingdotgg#135-pingdotgg#143): 8 Grok Build structured-output rejections and 4 timeouts. Nothing retried, so one transient blip deleted a whole reviewer for the entire run.
Retry every failure a fresh sample could plausibly fix, against a new subprocess. Output that overflows the collection limit stays terminal because it recurs identically.
Give the swarm one wall-clock budget split into stage deadlines, so a retrying lead can never starve the verification that produces the report. The worst-case serial path was already 12 minutes against a 10-minute service cap. Delegation, the only optional stage, is skipped and reported rather than started with too little time to finish.
Carry the Grok Build error cause into reviewer diagnostics so the next failure is diagnosable.
Sign up for freeto 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.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enable terminals for local draft threads - #143

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals
Closed

Enable terminals for local draft threads#143
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Mar 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds local terminal state management for draft threads before they're promoted to server threads
  • Terminal operations (open/close, split, create, activate, resize) now work on draft threads
  • When a draft thread is promoted to a server thread, terminal state is hydrated via new HYDRATE_THREAD_TERMINALS action
  • Introduces draftThreadTerminalState.ts module with reducer pattern for managing draft terminal state

Test plan

  • Open a new draft thread and verify terminals can be opened/closed
  • Split and create new terminals in a draft thread
  • Send a message to promote the draft to a server thread and verify terminal state persists
  • Verify terminal height changes are preserved after promotion
  • Run bun test to verify new unit tests pass

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because this refactors core client state management (React reducer/context → Zustand) and moves terminal UI state out of Thread into a new persisted store, which could cause state sync/persistence regressions across threads and drafts.

Overview
Moves app state from a reducer/context to a Zustand store.store.ts now exposes pure transition helpers (e.g. markThreadUnread, setThreadBranch, setRuntimeMode) plus a Zustand-backed useStore() whose dispatch is a method API (dispatch.setError(...), dispatch.toggleProject(...), etc.).

Decouples terminal UI state from Thread and persists it separately. Terminal fields are removed from Thread (types.ts), and a new terminalStateStore.ts persists per-threadId terminal state in localStorage using a shared reducer in threadTerminalState.ts.

Updates UI to use the new dispatch API and terminal store.ChatView and Sidebar switch terminal open/height/tab/group/active/running indicators to useTerminalStateStore, while other actions are updated to call the new dispatch.* methods; root websocket terminal activity events now write directly to the terminal store. Tests are adjusted accordingly, with new unit coverage for threadTerminalState behavior.

Written by Cursor Bugbot for commit c283d2a. This will update automatically on new commits. Configure here.

Note

Move terminal UI state for local draft threads to a dedicated Zustand store and update ChatView and Sidebar to read/write via useTerminalStateStore with MAX_THREAD_TERMINAL_COUNT enforcement

Introduce useTerminalStateStore for per-thread terminal state and refactor app state to a single Zustand store; update ChatView and Sidebar to consume terminal state from the new store and remove terminal fields from Thread. Core terminal logic lives in reduceThreadTerminalState with normalization helpers.

📍Where to Start

Start with the terminal reducer and helpers in apps/web/src/threadTerminalState.ts, then see the store wiring in apps/web/src/terminalStateStore.ts and usage in apps/web/src/components/ChatView.tsx.

Macroscope summarized c283d2a.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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.

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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch t3code/enable-draft-thread-terminals

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

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Close terminal selects next active terminal differently than store
    • Added same-group preference logic (closedTerminalGroup lookup and remainingTerminalsInClosedGroup fallback) to closeDraftTerminal, matching the store's closeThreadTerminal 3-level fallback behavior.

Create PR

Or push these changes by commenting:

@cursor push 5002774b80
Preview (5002774b80)
diff --git a/apps/web/src/draftThreadTerminalState.ts b/apps/web/src/draftThreadTerminalState.ts--- a/apps/web/src/draftThreadTerminalState.ts+++ b/apps/web/src/draftThreadTerminalState.ts@@ -161,9 +161,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;

Comment threadapps/web/src/draftThreadTerminalState.ts
Comment on lines +51 to +53
return [...new Set(runningTerminalIds)]
.map((id) => id.trim())
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:51

Consider trimming before deduplicating—currently " term1 " and "term1" both survive the Set, then become identical after .trim(), producing duplicates.

Suggested change
return[...newSet(runningTerminalIds)]
.map((id)=>id.trim())
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
return[...newSet(runningTerminalIds.map((id)=>id.trim()))]
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 51-53:
Consider trimming before deduplicating—currently `" term1 "` and `"term1"` both survive the `Set`, then become identical after `.trim()`, producing duplicates.
Evidence trail:
File: apps/web/src/threadTerminalState.ts lines 51-53 at REVIEWED_COMMIT. The function `normalizeRunningTerminalIds` uses `[...new Set(runningTerminalIds)].map((id) => id.trim())`. The Set deduplicates before trim, so `" term1 "` and `"term1"` both survive the Set, then both become `"term1"` after trim, producing duplicates.

Comment threadapps/web/src/composerDraftStore.ts Outdated
juliusmarmingeand others added 3 commits March 2, 2026 10:07
Adds local terminal state management for draft threads before they're
promoted to server threads. Terminal operations (open, split, create,
close, activate, resize) now work on draft threads and hydrate into
the server thread state when the draft is promoted.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add shared threadTerminalState.ts: ThreadTerminalState type, default state,
normalize helpers, and single reduceThreadTerminalState() used by both flows
- Refactor store.ts to use shared terminal reducer and helpers; terminal
actions map to reduceThreadTerminalState(threadTerminalSlice(thread), action)
- Make draftThreadTerminalState.ts re-export shared module (same shape/actions)
- Add terminalState to composer draft store: getDraftThreadTerminalState,
setDraftThreadTerminalAction, clearDraftThreadTerminalState; draft thread
shape matches Thread terminal slice
- ChatView: move draft terminal from useState into composer draft store; use
same actions for draft and persisted; hydrate then clear draft terminal
when reconciling server state
Made-with: Cursor
…irectly
- Delete draftThreadTerminalState.ts (was only re-exporting shared module)
- Replace draftThreadTerminalState.test.ts with threadTerminalState.test.ts
using createDefaultThreadTerminalState and reduceThreadTerminalState
- Draft threads are first-class: same helpers and types as persisted threads
Made-with: Cursor
Comment on lines +149 to +151
export function createDefaultThreadTerminalState(): ThreadTerminalState {
return { ...DEFAULT_THREAD_TERMINAL_STATE };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:149

Defaults are shallow. Mutating nested arrays (e.g. terminalIds, terminalGroups) can leak into DEFAULT_THREAD_TERMINAL_STATE. Suggest deep-freezing the default and/or returning deep clones from createDefaultThreadTerminalState and getDefaultThreadTerminalState to avoid shared references.

-export function createDefaultThreadTerminalState(): ThreadTerminalState {- return { ...DEFAULT_THREAD_TERMINAL_STATE };+export function createDefaultThreadTerminalState(): ThreadTerminalState {+ return {+ ...DEFAULT_THREAD_TERMINAL_STATE,+ terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds],+ runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds],+ terminalGroups: DEFAULT_THREAD_TERMINAL_STATE.terminalGroups.map((g) => ({+ ...g,+ terminalIds: [...g.terminalIds],+ })),+ };
}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 149-151:
Defaults are shallow. Mutating nested arrays (e.g. `terminalIds`, `terminalGroups`) can leak into `DEFAULT_THREAD_TERMINAL_STATE`. Suggest deep-freezing the default and/or returning deep clones from `createDefaultThreadTerminalState` and `getDefaultThreadTerminalState` to avoid shared references.
Evidence trail:
apps/web/src/threadTerminalState.ts lines 132-155: `DEFAULT_THREAD_TERMINAL_STATE` is defined with `Object.freeze()` containing nested arrays (`terminalIds`, `runningTerminalIds`, `terminalGroups`). `createDefaultThreadTerminalState()` returns `{ ...DEFAULT_THREAD_TERMINAL_STATE }` (shallow copy). `getDefaultThreadTerminalState()` returns the frozen object directly. JavaScript's `Object.freeze()` is documented as shallow-freezing only top-level properties (MDN documentation). Usage sites in `composerDraftStore.ts` lines 683-697 show these functions being called and their results potentially stored/used.

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Close terminal lost group-aware active terminal selection
    • Restored group-aware active terminal selection in closeTerminal by first looking for siblings in the same group (remainingTerminalsInClosedGroup) before falling back to the global remainingTerminalIds list, matching the old closeThreadTerminal behavior.
  • ✅ Fixed: Dead reference equality check never short-circuits store updates
    • Replaced the dead next === current reference equality check with a structural comparison via a new threadTerminalStateEquals helper, since reduceThreadTerminalState always returns a new object through normalizeThreadTerminalState.

Create PR

Or push these changes by commenting:

@cursor push a82c89a4f7
Preview (a82c89a4f7)
diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts--- a/apps/web/src/composerDraftStore.ts+++ b/apps/web/src/composerDraftStore.ts@@ -10,6 +10,7 @@
createDefaultThreadTerminalState,
getDefaultThreadTerminalState,
reduceThreadTerminalState,
+ threadTerminalStateEquals,
type ThreadTerminalAction,
type ThreadTerminalState,
} from "./threadTerminalState";
@@ -696,7 +697,7 @@
}
const current = existing.terminalState ?? createDefaultThreadTerminalState();
const next = reduceThreadTerminalState(current, action);
- if (next === current) {+ if (threadTerminalStateEquals(next, current)) {
return state;
}
return {
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -205,9 +205,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;
@@ -242,6 +254,36 @@
}));
}
+export function threadTerminalStateEquals(a: ThreadTerminalState, b: ThreadTerminalState): boolean {+ if (a === b) return true;+ if (+ a.terminalOpen !== b.terminalOpen ||+ a.terminalHeight !== b.terminalHeight ||+ a.activeTerminalId !== b.activeTerminalId ||+ a.activeTerminalGroupId !== b.activeTerminalGroupId ||+ a.terminalIds.length !== b.terminalIds.length ||+ a.runningTerminalIds.length !== b.runningTerminalIds.length ||+ a.terminalGroups.length !== b.terminalGroups.length+ ) {+ return false;+ }+ for (let i = 0; i < a.terminalIds.length; i++) {+ if (a.terminalIds[i] !== b.terminalIds[i]) return false;+ }+ for (let i = 0; i < a.runningTerminalIds.length; i++) {+ if (a.runningTerminalIds[i] !== b.runningTerminalIds[i]) return false;+ }+ for (let i = 0; i < a.terminalGroups.length; i++) {+ const ga = a.terminalGroups[i]!;+ const gb = b.terminalGroups[i]!;+ if (ga.id !== gb.id || ga.terminalIds.length !== gb.terminalIds.length) return false;+ for (let j = 0; j < ga.terminalIds.length; j++) {+ if (ga.terminalIds[j] !== gb.terminalIds[j]) return false;+ }+ }+ return true;+}+
// ─── Single reducer for both draft and persisted thread ────────────────────
export function reduceThreadTerminalState(

Comment threadapps/web/src/threadTerminalState.ts
Comment threadapps/web/src/composerDraftStore.ts Outdated
- Main store: replace useReducer with Zustand; pure state transition
functions (syncServerReadModel, markThreadVisited, setError, etc.)
called via store methods. Persist via subscribe.
- Terminal store: single store keyed by threadId; replace
dispatchTerminalAction with direct methods (setTerminalOpen,
setTerminalHeight, splitTerminal, newTerminal, setActiveTerminal,
closeTerminal, setTerminalActivity). Pure reduceThreadTerminalState
used internally.
- Update all consumers to call dispatch.method(...) and terminal
store methods. Fix store tests to use markThreadUnread pure fn.
- Fix Fragment import in store.ts; fix unused get param for lint.
Made-with: Cursor
@juliusmarminge
juliusmarmingeforce-pushed the t3code/enable-draft-thread-terminals branch from eef2781 to c283d2aCompareMarch 2, 2026 18:24
Comment threadapps/web/src/store.ts
if (!thread.latestTurn?.completedAt) return thread;
const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt);
if (Number.isNaN(latestTurnCompletedAtMs)) return thread;
const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/store.ts:303

If latestTurn.completedAt is in the future (clock skew), unreadVisitedAt will also be in the future, preventing markThreadVisited from updating lastVisitedAt until client time catches up. Consider using Math.min(latestTurnCompletedAtMs - 1, Date.now()) to cap the timestamp.

Suggested change
constunreadVisitedAt=newDate(latestTurnCompletedAtMs-1).toISOString();
constunreadVisitedAt=newDate(Math.min(latestTurnCompletedAtMs-1,Date.now())).toISOString();
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around line 303:
If `latestTurn.completedAt` is in the future (clock skew), `unreadVisitedAt` will also be in the future, preventing `markThreadVisited` from updating `lastVisitedAt` until client time catches up. Consider using `Math.min(latestTurnCompletedAtMs - 1, Date.now())` to cap the timestamp.
Evidence trail:
apps/web/src/store.ts lines 296-306 (markThreadUnread function, line 303 sets unreadVisitedAt from latestTurn.completedAt - 1); apps/web/src/store.ts lines 273-291 (markThreadVisited function, line 278 defaults to Date.now(), lines 284-287 comparison that prevents update if previousVisitedAtMs >= visitedAtMs)

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumcomponents/Sidebar.tsx:240

The component subscribes to getTerminalState (a stable function reference), not the underlying terminalStateByThreadId data. When terminal state changes, this component won't re-render, causing stale terminalStatus display. Consider subscribing to the data directly, e.g., useTerminalStateStore((s) => s.terminalStateByThreadId), then access state in the render loop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Sidebar.tsx around line 240:
The component subscribes to `getTerminalState` (a stable function reference), not the underlying `terminalStateByThreadId` data. When terminal state changes, this component won't re-render, causing stale `terminalStatus` display. Consider subscribing to the data directly, e.g., `useTerminalStateStore((s) => s.terminalStateByThreadId)`, then access state in the render loop.
Evidence trail:
apps/web/src/components/Sidebar.tsx line 240: `const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);` - subscribes to function reference
apps/web/src/components/Sidebar.tsx lines 818-820: `const terminalStatus = terminalStatusFromRunningIds(getTerminalState(thread.id).runningTerminalIds);` - uses function to get data during render
apps/web/src/terminalStateStore.ts lines 51-66: Shows `getTerminalState` is a stable function that calls `get().terminalStateByThreadId[threadId]` - the function reference doesn't change when the data it accesses changes

juliusmarminge added a commit that referenced this pull request Mar 2, 2026
Integrate the draft and persisted terminal-state flow from #143 (shared thread terminal reducer plus dedicated terminal store), while preserving selector compatibility and runtime-mode switch robustness on this branch.
Made-with: Cursor
@juliusmarminge

Copy link
Copy Markdown
MemberAuthor

Superseded by #141. I merged the terminal-store work into that branch/PR so the three-store architecture now lives there (composer draft store + app state store + terminal store).

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Unstable dispatch reference causes infinite effect loop
    • Wrapped dispatch action functions in useMemo with empty deps so the dispatch object is referentially stable across renders, breaking the infinite re-render/effect cycle.
  • ✅ Fixed: Sidebar terminal status indicator not reactive to changes
    • Changed the Sidebar's terminal store subscription from the stable getTerminalState function reference to the terminalStateByThreadId data object, so changes to terminal running state trigger re-renders.
  • ✅ Fixed: No-op terminal actions always trigger store updates
    • Changed all no-op early-return paths in reduceThreadTerminalState to return the original state parameter instead of the normalized copy, allowing the reference equality check in applyTerminalAction to succeed and skip unnecessary store updates.

Create PR

Or push these changes by commenting:

@cursor push 004658797e
Preview (004658797e)
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@@ -237,7 +237,7 @@
(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
- const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);+ const terminalStateByThreadId = useTerminalStateStore((s) => s.terminalStateByThreadId);
const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId);
const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext);
const clearProjectDraftThreadId = useComposerDraftStore(
@@ -816,7 +816,7 @@
);
const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null);
const terminalStatus = terminalStatusFromRunningIds(
- getTerminalState(thread.id).runningTerminalIds,+ terminalStateByThreadId[thread.id]?.runningTerminalIds ?? [],
);
return (
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts--- a/apps/web/src/store.ts+++ b/apps/web/src/store.ts@@ -1,4 +1,4 @@-import { Fragment, type ReactNode, createElement, useEffect } from "react";+import { Fragment, type ReactNode, createElement, useEffect, useMemo } from "react";
import {
DEFAULT_MODEL,
ProviderSessionId,
@@ -416,8 +416,18 @@
threadsHydrated: s.threadsHydrated,
runtimeMode: s.runtimeMode,
}));
- return {- state,- dispatch: useAppStore.getState(),- };+ const dispatch = useMemo(() => {+ const s = useAppStore.getState();+ return {+ syncServerReadModel: s.syncServerReadModel,+ markThreadVisited: s.markThreadVisited,+ markThreadUnread: s.markThreadUnread,+ toggleProject: s.toggleProject,+ setProjectExpanded: s.setProjectExpanded,+ setError: s.setError,+ setThreadBranch: s.setThreadBranch,+ setRuntimeMode: s.setRuntimeMode,+ };+ }, []);+ return { state, dispatch };
}
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -251,7 +251,7 @@
const normalized = normalizeThreadTerminalState(state);
if (action.type === "set-open") {
- if (normalized.terminalOpen === action.open) return normalized;+ if (normalized.terminalOpen === action.open) return state;
return { ...normalized, terminalOpen: action.open };
}
@@ -261,14 +261,14 @@
action.height <= 0 ||
normalized.terminalHeight === action.height
) {
- return normalized;+ return state;
}
return { ...normalized, terminalHeight: action.height };
}
if (action.type === "set-active") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const activeTerminalGroupId =
normalized.terminalGroups.find((group) => group.terminalIds.includes(action.terminalId))?.id ??
@@ -286,7 +286,7 @@
if (action.type === "set-activity") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const runningTerminalIds = new Set(normalized.runningTerminalIds);
if (action.hasRunningSubprocess) {
@@ -298,12 +298,12 @@
}
if (!action.terminalId || action.terminalId.trim().length === 0) {
- return normalized;+ return state;
}
const isNewTerminal = !normalized.terminalIds.includes(action.terminalId);
if (isNewTerminal && normalized.terminalIds.length >= MAX_THREAD_TERMINAL_COUNT) {
- return normalized;+ return state;
}
const terminalIds = isNewTerminal

Comment threadapps/web/src/store.ts

function setThreadTerminalActivity(
thread: Thread,
terminalId: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable dispatch reference causes infinite effect loop

High Severity

useStore() returns dispatch: useAppStore.getState(), which produces a new object reference after every store update. The selector also creates a new inline object on every call, so every store change triggers a re-render. In EventRouter, this dispatch is in the useEffect dependency array. Each effect run calls syncSnapshot(), which calls dispatch.syncServerReadModel(snapshot), which updates the store, which triggers a re-render, which changes dispatch, which re-runs the effect — creating an infinite loop of network requests and event re-subscriptions.

Additional Locations (1)

Fix in CursorFix in Web

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar terminal status indicator not reactive to changes

Medium Severity

The Sidebar subscribes to useTerminalStateStore((s) => s.getTerminalState), which returns the getTerminalState function reference — a stable closure that never changes. This means the Sidebar never re-renders when terminal state (like runningTerminalIds) changes in the terminal store. The terminal running indicator in the sidebar will be stale until the component re-renders for an unrelated reason.

Additional Locations (1)

Fix in CursorFix in Web

...state,
[threadId]: next,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No-op terminal actions always trigger store updates

Low Severity

applyTerminalAction checks if (next === current) to avoid unnecessary updates, but this never succeeds. reduceThreadTerminalState always calls normalizeThreadTerminalState first, which creates a new object. So even for no-op actions (e.g., setting terminalOpen to its current value), next is a different reference than current, and the store always updates. The intended optimization is defeated.

Additional Locations (1)

Fix in CursorFix in Web

logancsack added a commit to logancsack/t3code that referenced this pull request Aug 4, 2026
Aldo Review lost 27% of its lead reviewers in the field (12 of 44 slots across remote-dev PRs pingdotgg#135-pingdotgg#143): 8 Grok Build structured-output rejections and 4 timeouts. Nothing retried, so one transient blip deleted a whole reviewer for the entire run.
Retry every failure a fresh sample could plausibly fix, against a new subprocess. Output that overflows the collection limit stays terminal because it recurs identically.
Give the swarm one wall-clock budget split into stage deadlines, so a retrying lead can never starve the verification that produces the report. The worst-case serial path was already 12 minutes against a 10-minute service cap. Delegation, the only optional stage, is skipped and reported rather than started with too little time to finish.
Carry the Grok Build error cause into reviewer diagnostics so the next failure is diagnosable.
Sign up for freeto 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.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Enable terminals for local draft threads - #143

Closed
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals
Closed

Enable terminals for local draft threads#143
juliusmarminge wants to merge 4 commits into
mainfrom
t3code/enable-draft-thread-terminals

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Mar 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds local terminal state management for draft threads before they're promoted to server threads
  • Terminal operations (open/close, split, create, activate, resize) now work on draft threads
  • When a draft thread is promoted to a server thread, terminal state is hydrated via new HYDRATE_THREAD_TERMINALS action
  • Introduces draftThreadTerminalState.ts module with reducer pattern for managing draft terminal state

Test plan

  • Open a new draft thread and verify terminals can be opened/closed
  • Split and create new terminals in a draft thread
  • Send a message to promote the draft to a server thread and verify terminal state persists
  • Verify terminal height changes are preserved after promotion
  • Run bun test to verify new unit tests pass

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because this refactors core client state management (React reducer/context → Zustand) and moves terminal UI state out of Thread into a new persisted store, which could cause state sync/persistence regressions across threads and drafts.

Overview
Moves app state from a reducer/context to a Zustand store.store.ts now exposes pure transition helpers (e.g. markThreadUnread, setThreadBranch, setRuntimeMode) plus a Zustand-backed useStore() whose dispatch is a method API (dispatch.setError(...), dispatch.toggleProject(...), etc.).

Decouples terminal UI state from Thread and persists it separately. Terminal fields are removed from Thread (types.ts), and a new terminalStateStore.ts persists per-threadId terminal state in localStorage using a shared reducer in threadTerminalState.ts.

Updates UI to use the new dispatch API and terminal store.ChatView and Sidebar switch terminal open/height/tab/group/active/running indicators to useTerminalStateStore, while other actions are updated to call the new dispatch.* methods; root websocket terminal activity events now write directly to the terminal store. Tests are adjusted accordingly, with new unit coverage for threadTerminalState behavior.

Written by Cursor Bugbot for commit c283d2a. This will update automatically on new commits. Configure here.

Note

Move terminal UI state for local draft threads to a dedicated Zustand store and update ChatView and Sidebar to read/write via useTerminalStateStore with MAX_THREAD_TERMINAL_COUNT enforcement

Introduce useTerminalStateStore for per-thread terminal state and refactor app state to a single Zustand store; update ChatView and Sidebar to consume terminal state from the new store and remove terminal fields from Thread. Core terminal logic lives in reduceThreadTerminalState with normalization helpers.

📍Where to Start

Start with the terminal reducer and helpers in apps/web/src/threadTerminalState.ts, then see the store wiring in apps/web/src/terminalStateStore.ts and usage in apps/web/src/components/ChatView.tsx.

Macroscope summarized c283d2a.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. 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.

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
  • Post copyable unit tests in a comment
  • Commit unit tests in branch t3code/enable-draft-thread-terminals

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

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Close terminal selects next active terminal differently than store
    • Added same-group preference logic (closedTerminalGroup lookup and remainingTerminalsInClosedGroup fallback) to closeDraftTerminal, matching the store's closeThreadTerminal 3-level fallback behavior.

Create PR

Or push these changes by commenting:

@cursor push 5002774b80
Preview (5002774b80)
diff --git a/apps/web/src/draftThreadTerminalState.ts b/apps/web/src/draftThreadTerminalState.ts--- a/apps/web/src/draftThreadTerminalState.ts+++ b/apps/web/src/draftThreadTerminalState.ts@@ -161,9 +161,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;

Comment threadapps/web/src/draftThreadTerminalState.ts
Comment on lines +51 to +53
return [...new Set(runningTerminalIds)]
.map((id) => id.trim())
.filter((id) => id.length > 0 && validTerminalIdSet.has(id));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:51

Consider trimming before deduplicating—currently " term1 " and "term1" both survive the Set, then become identical after .trim(), producing duplicates.

Suggested change
return[...newSet(runningTerminalIds)]
.map((id)=>id.trim())
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
return[...newSet(runningTerminalIds.map((id)=>id.trim()))]
.filter((id)=>id.length>0&&validTerminalIdSet.has(id));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 51-53:
Consider trimming before deduplicating—currently `" term1 "` and `"term1"` both survive the `Set`, then become identical after `.trim()`, producing duplicates.
Evidence trail:
File: apps/web/src/threadTerminalState.ts lines 51-53 at REVIEWED_COMMIT. The function `normalizeRunningTerminalIds` uses `[...new Set(runningTerminalIds)].map((id) => id.trim())`. The Set deduplicates before trim, so `" term1 "` and `"term1"` both survive the Set, then both become `"term1"` after trim, producing duplicates.

Comment threadapps/web/src/composerDraftStore.ts Outdated
juliusmarmingeand others added 3 commits March 2, 2026 10:07
Adds local terminal state management for draft threads before they're
promoted to server threads. Terminal operations (open, split, create,
close, activate, resize) now work on draft threads and hydrate into
the server thread state when the draft is promoted.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add shared threadTerminalState.ts: ThreadTerminalState type, default state,
normalize helpers, and single reduceThreadTerminalState() used by both flows
- Refactor store.ts to use shared terminal reducer and helpers; terminal
actions map to reduceThreadTerminalState(threadTerminalSlice(thread), action)
- Make draftThreadTerminalState.ts re-export shared module (same shape/actions)
- Add terminalState to composer draft store: getDraftThreadTerminalState,
setDraftThreadTerminalAction, clearDraftThreadTerminalState; draft thread
shape matches Thread terminal slice
- ChatView: move draft terminal from useState into composer draft store; use
same actions for draft and persisted; hydrate then clear draft terminal
when reconciling server state
Made-with: Cursor
…irectly
- Delete draftThreadTerminalState.ts (was only re-exporting shared module)
- Replace draftThreadTerminalState.test.ts with threadTerminalState.test.ts
using createDefaultThreadTerminalState and reduceThreadTerminalState
- Draft threads are first-class: same helpers and types as persisted threads
Made-with: Cursor
Comment on lines +149 to +151
export function createDefaultThreadTerminalState(): ThreadTerminalState {
return { ...DEFAULT_THREAD_TERMINAL_STATE };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/threadTerminalState.ts:149

Defaults are shallow. Mutating nested arrays (e.g. terminalIds, terminalGroups) can leak into DEFAULT_THREAD_TERMINAL_STATE. Suggest deep-freezing the default and/or returning deep clones from createDefaultThreadTerminalState and getDefaultThreadTerminalState to avoid shared references.

-export function createDefaultThreadTerminalState(): ThreadTerminalState {- return { ...DEFAULT_THREAD_TERMINAL_STATE };+export function createDefaultThreadTerminalState(): ThreadTerminalState {+ return {+ ...DEFAULT_THREAD_TERMINAL_STATE,+ terminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.terminalIds],+ runningTerminalIds: [...DEFAULT_THREAD_TERMINAL_STATE.runningTerminalIds],+ terminalGroups: DEFAULT_THREAD_TERMINAL_STATE.terminalGroups.map((g) => ({+ ...g,+ terminalIds: [...g.terminalIds],+ })),+ };
}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/threadTerminalState.ts around lines 149-151:
Defaults are shallow. Mutating nested arrays (e.g. `terminalIds`, `terminalGroups`) can leak into `DEFAULT_THREAD_TERMINAL_STATE`. Suggest deep-freezing the default and/or returning deep clones from `createDefaultThreadTerminalState` and `getDefaultThreadTerminalState` to avoid shared references.
Evidence trail:
apps/web/src/threadTerminalState.ts lines 132-155: `DEFAULT_THREAD_TERMINAL_STATE` is defined with `Object.freeze()` containing nested arrays (`terminalIds`, `runningTerminalIds`, `terminalGroups`). `createDefaultThreadTerminalState()` returns `{ ...DEFAULT_THREAD_TERMINAL_STATE }` (shallow copy). `getDefaultThreadTerminalState()` returns the frozen object directly. JavaScript's `Object.freeze()` is documented as shallow-freezing only top-level properties (MDN documentation). Usage sites in `composerDraftStore.ts` lines 683-697 show these functions being called and their results potentially stored/used.

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Close terminal lost group-aware active terminal selection
    • Restored group-aware active terminal selection in closeTerminal by first looking for siblings in the same group (remainingTerminalsInClosedGroup) before falling back to the global remainingTerminalIds list, matching the old closeThreadTerminal behavior.
  • ✅ Fixed: Dead reference equality check never short-circuits store updates
    • Replaced the dead next === current reference equality check with a structural comparison via a new threadTerminalStateEquals helper, since reduceThreadTerminalState always returns a new object through normalizeThreadTerminalState.

Create PR

Or push these changes by commenting:

@cursor push a82c89a4f7
Preview (a82c89a4f7)
diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts--- a/apps/web/src/composerDraftStore.ts+++ b/apps/web/src/composerDraftStore.ts@@ -10,6 +10,7 @@
createDefaultThreadTerminalState,
getDefaultThreadTerminalState,
reduceThreadTerminalState,
+ threadTerminalStateEquals,
type ThreadTerminalAction,
type ThreadTerminalState,
} from "./threadTerminalState";
@@ -696,7 +697,7 @@
}
const current = existing.terminalState ?? createDefaultThreadTerminalState();
const next = reduceThreadTerminalState(current, action);
- if (next === current) {+ if (threadTerminalStateEquals(next, current)) {
return state;
}
return {
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -205,9 +205,21 @@
}
const closedTerminalIndex = state.terminalIds.indexOf(terminalId);
+ const closedTerminalGroup = state.terminalGroups.find((group) =>+ group.terminalIds.includes(terminalId),+ );+ const closedTerminalGroupIndex = closedTerminalGroup+ ? closedTerminalGroup.terminalIds.indexOf(terminalId)+ : -1;+ const remainingTerminalsInClosedGroup = (closedTerminalGroup?.terminalIds ?? []).filter(+ (id) => id !== terminalId,+ );
const nextActiveTerminalId =
state.activeTerminalId === terminalId
- ? (remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??+ ? (remainingTerminalsInClosedGroup[+ Math.min(closedTerminalGroupIndex, remainingTerminalsInClosedGroup.length - 1)+ ] ??+ remainingTerminalIds[Math.min(closedTerminalIndex, remainingTerminalIds.length - 1)] ??
remainingTerminalIds[0] ??
DEFAULT_THREAD_TERMINAL_ID)
: state.activeTerminalId;
@@ -242,6 +254,36 @@
}));
}
+export function threadTerminalStateEquals(a: ThreadTerminalState, b: ThreadTerminalState): boolean {+ if (a === b) return true;+ if (+ a.terminalOpen !== b.terminalOpen ||+ a.terminalHeight !== b.terminalHeight ||+ a.activeTerminalId !== b.activeTerminalId ||+ a.activeTerminalGroupId !== b.activeTerminalGroupId ||+ a.terminalIds.length !== b.terminalIds.length ||+ a.runningTerminalIds.length !== b.runningTerminalIds.length ||+ a.terminalGroups.length !== b.terminalGroups.length+ ) {+ return false;+ }+ for (let i = 0; i < a.terminalIds.length; i++) {+ if (a.terminalIds[i] !== b.terminalIds[i]) return false;+ }+ for (let i = 0; i < a.runningTerminalIds.length; i++) {+ if (a.runningTerminalIds[i] !== b.runningTerminalIds[i]) return false;+ }+ for (let i = 0; i < a.terminalGroups.length; i++) {+ const ga = a.terminalGroups[i]!;+ const gb = b.terminalGroups[i]!;+ if (ga.id !== gb.id || ga.terminalIds.length !== gb.terminalIds.length) return false;+ for (let j = 0; j < ga.terminalIds.length; j++) {+ if (ga.terminalIds[j] !== gb.terminalIds[j]) return false;+ }+ }+ return true;+}+
// ─── Single reducer for both draft and persisted thread ────────────────────
export function reduceThreadTerminalState(

Comment threadapps/web/src/threadTerminalState.ts
Comment threadapps/web/src/composerDraftStore.ts Outdated
- Main store: replace useReducer with Zustand; pure state transition
functions (syncServerReadModel, markThreadVisited, setError, etc.)
called via store methods. Persist via subscribe.
- Terminal store: single store keyed by threadId; replace
dispatchTerminalAction with direct methods (setTerminalOpen,
setTerminalHeight, splitTerminal, newTerminal, setActiveTerminal,
closeTerminal, setTerminalActivity). Pure reduceThreadTerminalState
used internally.
- Update all consumers to call dispatch.method(...) and terminal
store methods. Fix store tests to use markThreadUnread pure fn.
- Fix Fragment import in store.ts; fix unused get param for lint.
Made-with: Cursor
@juliusmarminge
juliusmarmingeforce-pushed the t3code/enable-draft-thread-terminals branch from eef2781 to c283d2aCompareMarch 2, 2026 18:24
Comment threadapps/web/src/store.ts
if (!thread.latestTurn?.completedAt) return thread;
const latestTurnCompletedAtMs = Date.parse(thread.latestTurn.completedAt);
if (Number.isNaN(latestTurnCompletedAtMs)) return thread;
const unreadVisitedAt = new Date(latestTurnCompletedAtMs - 1).toISOString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Lowsrc/store.ts:303

If latestTurn.completedAt is in the future (clock skew), unreadVisitedAt will also be in the future, preventing markThreadVisited from updating lastVisitedAt until client time catches up. Consider using Math.min(latestTurnCompletedAtMs - 1, Date.now()) to cap the timestamp.

Suggested change
constunreadVisitedAt=newDate(latestTurnCompletedAtMs-1).toISOString();
constunreadVisitedAt=newDate(Math.min(latestTurnCompletedAtMs-1,Date.now())).toISOString();
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/store.ts around line 303:
If `latestTurn.completedAt` is in the future (clock skew), `unreadVisitedAt` will also be in the future, preventing `markThreadVisited` from updating `lastVisitedAt` until client time catches up. Consider using `Math.min(latestTurnCompletedAtMs - 1, Date.now())` to cap the timestamp.
Evidence trail:
apps/web/src/store.ts lines 296-306 (markThreadUnread function, line 303 sets unreadVisitedAt from latestTurn.completedAt - 1); apps/web/src/store.ts lines 273-291 (markThreadVisited function, line 278 defaults to Date.now(), lines 284-287 comparison that prevents update if previousVisitedAtMs >= visitedAtMs)

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mediumcomponents/Sidebar.tsx:240

The component subscribes to getTerminalState (a stable function reference), not the underlying terminalStateByThreadId data. When terminal state changes, this component won't re-render, causing stale terminalStatus display. Consider subscribing to the data directly, e.g., useTerminalStateStore((s) => s.terminalStateByThreadId), then access state in the render loop.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/Sidebar.tsx around line 240:
The component subscribes to `getTerminalState` (a stable function reference), not the underlying `terminalStateByThreadId` data. When terminal state changes, this component won't re-render, causing stale `terminalStatus` display. Consider subscribing to the data directly, e.g., `useTerminalStateStore((s) => s.terminalStateByThreadId)`, then access state in the render loop.
Evidence trail:
apps/web/src/components/Sidebar.tsx line 240: `const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);` - subscribes to function reference
apps/web/src/components/Sidebar.tsx lines 818-820: `const terminalStatus = terminalStatusFromRunningIds(getTerminalState(thread.id).runningTerminalIds);` - uses function to get data during render
apps/web/src/terminalStateStore.ts lines 51-66: Shows `getTerminalState` is a stable function that calls `get().terminalStateByThreadId[threadId]` - the function reference doesn't change when the data it accesses changes

juliusmarminge added a commit that referenced this pull request Mar 2, 2026
Integrate the draft and persisted terminal-state flow from #143 (shared thread terminal reducer plus dedicated terminal store), while preserving selector compatibility and runtime-mode switch robustness on this branch.
Made-with: Cursor
@juliusmarminge

Copy link
Copy Markdown
MemberAuthor

Superseded by #141. I merged the terminal-store work into that branch/PR so the three-store architecture now lives there (composer draft store + app state store + terminal store).

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

  • ✅ Fixed: Unstable dispatch reference causes infinite effect loop
    • Wrapped dispatch action functions in useMemo with empty deps so the dispatch object is referentially stable across renders, breaking the infinite re-render/effect cycle.
  • ✅ Fixed: Sidebar terminal status indicator not reactive to changes
    • Changed the Sidebar's terminal store subscription from the stable getTerminalState function reference to the terminalStateByThreadId data object, so changes to terminal running state trigger re-renders.
  • ✅ Fixed: No-op terminal actions always trigger store updates
    • Changed all no-op early-return paths in reduceThreadTerminalState to return the original state parameter instead of the normalized copy, allowing the reference equality check in applyTerminalAction to succeed and skip unnecessary store updates.

Create PR

Or push these changes by commenting:

@cursor push 004658797e
Preview (004658797e)
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@@ -237,7 +237,7 @@
(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
- const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);+ const terminalStateByThreadId = useTerminalStateStore((s) => s.terminalStateByThreadId);
const setProjectDraftThreadId = useComposerDraftStore((store) => store.setProjectDraftThreadId);
const setDraftThreadContext = useComposerDraftStore((store) => store.setDraftThreadContext);
const clearProjectDraftThreadId = useComposerDraftStore(
@@ -816,7 +816,7 @@
);
const prStatus = prStatusIndicator(prByThreadId.get(thread.id) ?? null);
const terminalStatus = terminalStatusFromRunningIds(
- getTerminalState(thread.id).runningTerminalIds,+ terminalStateByThreadId[thread.id]?.runningTerminalIds ?? [],
);
return (
diff --git a/apps/web/src/store.ts b/apps/web/src/store.ts--- a/apps/web/src/store.ts+++ b/apps/web/src/store.ts@@ -1,4 +1,4 @@-import { Fragment, type ReactNode, createElement, useEffect } from "react";+import { Fragment, type ReactNode, createElement, useEffect, useMemo } from "react";
import {
DEFAULT_MODEL,
ProviderSessionId,
@@ -416,8 +416,18 @@
threadsHydrated: s.threadsHydrated,
runtimeMode: s.runtimeMode,
}));
- return {- state,- dispatch: useAppStore.getState(),- };+ const dispatch = useMemo(() => {+ const s = useAppStore.getState();+ return {+ syncServerReadModel: s.syncServerReadModel,+ markThreadVisited: s.markThreadVisited,+ markThreadUnread: s.markThreadUnread,+ toggleProject: s.toggleProject,+ setProjectExpanded: s.setProjectExpanded,+ setError: s.setError,+ setThreadBranch: s.setThreadBranch,+ setRuntimeMode: s.setRuntimeMode,+ };+ }, []);+ return { state, dispatch };
}
diff --git a/apps/web/src/threadTerminalState.ts b/apps/web/src/threadTerminalState.ts--- a/apps/web/src/threadTerminalState.ts+++ b/apps/web/src/threadTerminalState.ts@@ -251,7 +251,7 @@
const normalized = normalizeThreadTerminalState(state);
if (action.type === "set-open") {
- if (normalized.terminalOpen === action.open) return normalized;+ if (normalized.terminalOpen === action.open) return state;
return { ...normalized, terminalOpen: action.open };
}
@@ -261,14 +261,14 @@
action.height <= 0 ||
normalized.terminalHeight === action.height
) {
- return normalized;+ return state;
}
return { ...normalized, terminalHeight: action.height };
}
if (action.type === "set-active") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const activeTerminalGroupId =
normalized.terminalGroups.find((group) => group.terminalIds.includes(action.terminalId))?.id ??
@@ -286,7 +286,7 @@
if (action.type === "set-activity") {
if (!normalized.terminalIds.includes(action.terminalId)) {
- return normalized;+ return state;
}
const runningTerminalIds = new Set(normalized.runningTerminalIds);
if (action.hasRunningSubprocess) {
@@ -298,12 +298,12 @@
}
if (!action.terminalId || action.terminalId.trim().length === 0) {
- return normalized;+ return state;
}
const isNewTerminal = !normalized.terminalIds.includes(action.terminalId);
if (isNewTerminal && normalized.terminalIds.length >= MAX_THREAD_TERMINAL_COUNT) {
- return normalized;+ return state;
}
const terminalIds = isNewTerminal

Comment threadapps/web/src/store.ts

function setThreadTerminalActivity(
thread: Thread,
terminalId: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unstable dispatch reference causes infinite effect loop

High Severity

useStore() returns dispatch: useAppStore.getState(), which produces a new object reference after every store update. The selector also creates a new inline object on every call, so every store change triggers a re-render. In EventRouter, this dispatch is in the useEffect dependency array. Each effect run calls syncSnapshot(), which calls dispatch.syncServerReadModel(snapshot), which updates the store, which triggers a re-render, which changes dispatch, which re-runs the effect — creating an infinite loop of network requests and event re-subscriptions.

Additional Locations (1)

Fix in CursorFix in Web

(store) => store.getDraftThreadByProjectId,
);
const getDraftThread = useComposerDraftStore((store) => store.getDraftThread);
const getTerminalState = useTerminalStateStore((s) => s.getTerminalState);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidebar terminal status indicator not reactive to changes

Medium Severity

The Sidebar subscribes to useTerminalStateStore((s) => s.getTerminalState), which returns the getTerminalState function reference — a stable closure that never changes. This means the Sidebar never re-renders when terminal state (like runningTerminalIds) changes in the terminal store. The terminal running indicator in the sidebar will be stale until the component re-renders for an unrelated reason.

Additional Locations (1)

Fix in CursorFix in Web

...state,
[threadId]: next,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No-op terminal actions always trigger store updates

Low Severity

applyTerminalAction checks if (next === current) to avoid unnecessary updates, but this never succeeds. reduceThreadTerminalState always calls normalizeThreadTerminalState first, which creates a new object. So even for no-op actions (e.g., setting terminalOpen to its current value), next is a different reference than current, and the store always updates. The intended optimization is defeated.

Additional Locations (1)

Fix in CursorFix in Web

logancsack added a commit to logancsack/t3code that referenced this pull request Aug 4, 2026
Aldo Review lost 27% of its lead reviewers in the field (12 of 44 slots across remote-dev PRs pingdotgg#135-pingdotgg#143): 8 Grok Build structured-output rejections and 4 timeouts. Nothing retried, so one transient blip deleted a whole reviewer for the entire run.
Retry every failure a fresh sample could plausibly fix, against a new subprocess. Output that overflows the collection limit stays terminal because it recurs identically.
Give the swarm one wall-clock budget split into stage deadlines, so a retrying lead can never starve the verification that produces the report. The worst-case serial path was already 12 minutes against a 10-minute service cap. Delegation, the only optional stage, is skipped and reported rather than started with too little time to finish.
Carry the Grok Build error cause into reviewer diagnostics so the next failure is diagnosable.
Sign up for freeto 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.

1 participant

@juliusmarminge