Refactor web orchestration sync to incremental events and isolated recovery#1560
Merged
juliusmarminge merged 23 commits intomainfrom Mar 30, 2026
Merged
Refactor web orchestration sync to incremental events and isolated recovery#1560juliusmarminge merged 23 commits intomainfrom
juliusmarminge merged 23 commits intomainfrom
Conversation
- Apply orchestration events incrementally instead of resyncing full snapshots - Reduce store subscription churn with cached thread snapshots and selectors - Update store tests for incremental event handling
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
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: Filter identity check always fails, causing unnecessary updates
- Replaced the always-false
filter() === originalidentity checks with length comparisons to correctly detect when no element was actually removed.
- Replaced the always-false
Or push these changes by commenting:
@cursor push a1f6b8993a
Preview (a1f6b8993a)
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
@@ -568,7 +568,9 @@
case "project.deleted": {
const projects = state.projects.filter((project) => project.id !== event.payload.projectId);
- return projects === state.projects ? state : { ...state, projects, threadsHydrated: true };
+ return projects.length === state.projects.length
+ ? state
+ : { ...state, projects, threadsHydrated: true };
}
case "thread.created": {
@@ -604,7 +606,9 @@
case "thread.deleted": {
const threads = state.threads.filter((thread) => thread.id !== event.payload.threadId);
- return threads === state.threads ? state : { ...state, threads, threadsHydrated: true };
+ return threads.length === state.threads.length
+ ? state
+ : { ...state, threads, threadsHydrated: true };
}
case "thread.archived": {Co-authored-by: codex <codex@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Turn-diff-completed can regress latestTurn to older turn
- Added a guard so the turn-diff-completed handler only updates latestTurn when the event's turnId matches the current latestTurn or latestTurn is null, preventing delayed checkpoint events for older turns from regressing latestTurn.
Or push these changes by commenting:
@cursor push 0927687131
Preview (0927687131)
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
@@ -876,24 +876,22 @@
(right.checkpointTurnCount ?? Number.MAX_SAFE_INTEGER),
)
.slice(-MAX_THREAD_CHECKPOINTS);
+ const latestTurn =
+ thread.latestTurn === null || thread.latestTurn.turnId === event.payload.turnId
+ ? buildLatestTurn({
+ previous: thread.latestTurn,
+ turnId: event.payload.turnId,
+ state: checkpointStatusToLatestTurnState(event.payload.status),
+ requestedAt: thread.latestTurn?.requestedAt ?? event.payload.completedAt,
+ startedAt: thread.latestTurn?.startedAt ?? event.payload.completedAt,
+ completedAt: event.payload.completedAt,
+ assistantMessageId: event.payload.assistantMessageId,
+ })
+ : thread.latestTurn;
return {
...thread,
turnDiffSummaries,
- latestTurn: buildLatestTurn({
- previous: thread.latestTurn,
- turnId: event.payload.turnId,
- state: checkpointStatusToLatestTurnState(event.payload.status),
- requestedAt:
- thread.latestTurn?.turnId === event.payload.turnId
- ? thread.latestTurn.requestedAt
- : event.payload.completedAt,
- startedAt:
- thread.latestTurn?.turnId === event.payload.turnId
- ? (thread.latestTurn.startedAt ?? event.payload.completedAt)
- : event.payload.completedAt,
- completedAt: event.payload.completedAt,
- assistantMessageId: event.payload.assistantMessageId,
- }),
+ latestTurn,
updatedAt: event.occurredAt,
};
});- Keep store identity unchanged for missing project/thread deletes - Avoid regressing latestTurn when an older turn diff completes late
Contributor
There was a problem hiding this comment.
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: Module-level Map cache never cleaned up, unlike WeakMap
- Added a useEffect that prunes entries from threadPlanCatalogCache for thread IDs no longer present in the store's thread list.
- ✅ Fixed: Removed snapshot sync causes race with navigation
- Restored the getSnapshot + syncServerReadModel calls before navigation and in the error cleanup path, ensuring the new thread exists in the store before ChatView renders.
Or push these changes by commenting:
@cursor push 342dacc8c1
Preview (342dacc8c1)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -494,6 +494,14 @@
const threadPlanCatalog = useStore(
useShallow((store) => store.threads.map(toThreadPlanCatalogEntry)),
);
+ useEffect(() => {
+ const activeIds = new Set(threadPlanCatalog.map((t) => t.id));
+ for (const id of threadPlanCatalogCache.keys()) {
+ if (!activeIds.has(id)) {
+ threadPlanCatalogCache.delete(id);
+ }
+ }
+ }, [threadPlanCatalog]);
const localDraftError = serverThread ? null : (localDraftErrorsByThreadId[threadId] ?? null);
const localDraftThread = useMemo(
() =>
@@ -3158,7 +3166,9 @@
createdAt,
});
})
- .then(() => {
+ .then(() => api.orchestration.getSnapshot())
+ .then((snapshot) => {
+ useStore.getState().syncServerReadModel(snapshot);
// Signal that the plan sidebar should open on the new thread.
planSidebarOpenOnNextThreadRef.current = true;
return navigate({
@@ -3174,6 +3184,12 @@
threadId: nextThreadId,
})
.catch(() => undefined);
+ await api.orchestration
+ .getSnapshot()
+ .then((snapshot) => {
+ useStore.getState().syncServerReadModel(snapshot);
+ })
+ .catch(() => undefined);
toastManager.add({
type: "error",
title: "Could not start implementation thread",- Derive batch effects for draft and terminal-state cleanup - Remove terminal state entries on thread delete - Add tests for lifecycle effect handling
- Split orchestration effects for promoted vs deleted threads - Add single-thread draft cleanup helper and update route handling - Cover promotion cleanup behavior with store and effect tests
- Rename store hydration flag to bootstrapComplete - Only clear missing-thread redirects after snapshot sync
- Split project and thread UI state from server data - Preserve sidebar ordering and unread tracking - Co-authored-by: codex <codex@users.noreply.github.com>
- Read threads and projects from store state on demand - Expose a default project id from the new-thread hook - Skip extra work in global chat shortcut handling
- Simulate thread.created pushes in browser tests - Verify promoted drafts clear via live batch effects
- Replace transient send phase state with local dispatch snapshots - Clear the busy state only after the server reflects the turn/session update - Cover the acknowledgment rules with logic tests
- Move snapshot/replay sequencing state into a shared coordinator - Add tests for deferred replay, gap recovery, and replay fallback
- Replace unbounded Map cache with LRU limits - Estimate per-thread plan entry size before caching
- Add thread-start detection helper and wait logic - Cover immediate, subscription-driven, and timeout cases
Contributor
There was a problem hiding this comment.
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: Duplicated project ordering logic across two files
- Extracted a shared
orderByPriorityutility inlib/utils.tsand replaced both independent implementations inuseHandleNewThread.tsandSidebar.tsxwith calls to it.
- Extracted a shared
- ✅ Fixed: Selector factories cause unnecessary per-render function allocations
- Added a simple Map-based cache to both
selectProjectByIdandselectThreadByIdso the same argument always returns the identical selector reference, enabling Zustand's fast-path identity check.
- Added a simple Map-based cache to both
Or push these changes by commenting:
@cursor push c83a7972fc
Preview (c83a7972fc)
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
@@ -54,7 +54,13 @@
import { isElectron } from "../env";
import { APP_STAGE_LABEL, APP_VERSION } from "../branding";
import { isTerminalFocused } from "../lib/terminalFocus";
-import { isLinuxPlatform, isMacPlatform, newCommandId, newProjectId } from "../lib/utils";
+import {
+ isLinuxPlatform,
+ isMacPlatform,
+ newCommandId,
+ newProjectId,
+ orderByPriority,
+} from "../lib/utils";
import { useStore } from "../store";
import { useUiStateStore } from "../uiStateStore";
import {
@@ -500,18 +506,10 @@
const platform = navigator.platform;
const shouldBrowseForProjectImmediately = isElectron && !isLinuxDesktop;
const shouldShowProjectPathEntry = addingProject && !shouldBrowseForProjectImmediately;
- const orderedProjects = useMemo(() => {
- if (projectOrder.length === 0) {
- return projects;
- }
- const projectsById = new Map(projects.map((project) => [project.id, project] as const));
- const ordered = projectOrder.flatMap((projectId) => {
- const project = projectsById.get(projectId);
- return project ? [project] : [];
- });
- const remaining = projects.filter((project) => !projectOrder.includes(project.id));
- return [...ordered, ...remaining];
- }, [projectOrder, projects]);
+ const orderedProjects = useMemo(
+ () => orderByPriority(projects, projectOrder, (p) => p.id),
+ [projectOrder, projects],
+ );
const sidebarProjects = useMemo<SidebarProjectSnapshot[]>(
() =>
orderedProjects.map((project) => ({
diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts
--- a/apps/web/src/hooks/useHandleNewThread.ts
+++ b/apps/web/src/hooks/useHandleNewThread.ts
@@ -7,7 +7,7 @@
type DraftThreadState,
useComposerDraftStore,
} from "../composerDraftStore";
-import { newThreadId } from "../lib/utils";
+import { newThreadId, orderByPriority } from "../lib/utils";
import { selectThreadById, useStore } from "../store";
import { useUiStateStore } from "../uiStateStore";
@@ -23,15 +23,10 @@
const activeDraftThread = useComposerDraftStore((store) =>
routeThreadId ? (store.draftThreadsByThreadId[routeThreadId] ?? null) : null,
);
- const orderedProjects = useMemo(() => {
- if (projectOrder.length === 0) {
- return projectIds;
- }
- const projectIdsSet = new Set(projectIds);
- const ordered = projectOrder.filter((projectId) => projectIdsSet.has(projectId));
- const remaining = projectIds.filter((projectId) => !projectOrder.includes(projectId));
- return [...ordered, ...remaining];
- }, [projectIds, projectOrder]);
+ const orderedProjects = useMemo(
+ () => orderByPriority(projectIds, projectOrder, (id) => id),
+ [projectIds, projectOrder],
+ );
const handleNewThread = useCallback(
(
diff --git a/apps/web/src/lib/utils.ts b/apps/web/src/lib/utils.ts
--- a/apps/web/src/lib/utils.ts
+++ b/apps/web/src/lib/utils.ts
@@ -34,3 +34,23 @@
export const newThreadId = (): ThreadId => ThreadId.makeUnsafe(randomUUID());
export const newMessageId = (): MessageId => MessageId.makeUnsafe(randomUUID());
+
+/**
+ * Reorder `items` so that those whose key appears in `orderedKeys` come first
+ * (in `orderedKeys` order), followed by the remaining items in their original order.
+ */
+export function orderByPriority<T>(
+ items: readonly T[],
+ orderedKeys: readonly string[],
+ getKey: (item: T) => string,
+): T[] {
+ if (orderedKeys.length === 0) return items.slice();
+ const itemsByKey = new Map(items.map((item) => [getKey(item), item] as const));
+ const ordered = orderedKeys.flatMap((key) => {
+ const item = itemsByKey.get(key);
+ return item ? [item] : [];
+ });
+ const orderedKeySet = new Set(orderedKeys);
+ const remaining = items.filter((item) => !orderedKeySet.has(getKey(item)));
+ return [...ordered, ...remaining];
+}
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
@@ -852,15 +852,39 @@
return events.reduce((nextState, event) => applyOrchestrationEvent(nextState, event), state);
}
-export const selectProjectById =
- (projectId: Project["id"] | null | undefined) =>
- (state: AppState): Project | undefined =>
- projectId ? state.projects.find((project) => project.id === projectId) : undefined;
+const _projectSelectorCache = new Map<
+ string | null | undefined,
+ (state: AppState) => Project | undefined
+>();
+export function selectProjectById(
+ projectId: Project["id"] | null | undefined,
+): (state: AppState) => Project | undefined {
+ const key = projectId ?? null;
+ let selector = _projectSelectorCache.get(key);
+ if (!selector) {
+ selector = (state: AppState) =>
+ projectId ? state.projects.find((project) => project.id === projectId) : undefined;
+ _projectSelectorCache.set(key, selector);
+ }
+ return selector;
+}
-export const selectThreadById =
- (threadId: ThreadId | null | undefined) =>
- (state: AppState): Thread | undefined =>
- threadId ? state.threads.find((thread) => thread.id === threadId) : undefined;
+const _threadSelectorCache = new Map<
+ string | null | undefined,
+ (state: AppState) => Thread | undefined
+>();
+export function selectThreadById(
+ threadId: ThreadId | null | undefined,
+): (state: AppState) => Thread | undefined {
+ const key = threadId ?? null;
+ let selector = _threadSelectorCache.get(key);
+ if (!selector) {
+ selector = (state: AppState) =>
+ threadId ? state.threads.find((thread) => thread.id === threadId) : undefined;
+ _threadSelectorCache.set(key, selector);
+ }
+ return selector;
+}
export function setError(state: AppState, threadId: ThreadId, error: string | null): AppState {
const threads = updateThread(state.threads, threadId, (t) => {- Share preferred-id ordering across sidebar and new-thread flows - Add resilient thread startup wait logic and selector hooks - Cover the new ordering helper and race condition in tests
- Skip duplicate preferred IDs when ordering sidebar items - Add regression test for repeated preferred IDs
- Replace inferred local types with explicit shared contract exports - Keep router, chat, and sidebar tests aligned with contract shapes
- Reorder ChatView helper declarations for readability - No functional change
Contributor
There was a problem hiding this comment.
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: Empty replay response causes infinite recovery loop
- Added a no-progress check: if latestSequence doesn't advance after a replay batch, the recovery loop now falls back to snapshot recovery instead of looping infinitely.
- ✅ Fixed: Thread plan catalog recomputes on every store change
- Replaced the useShallow+map selector with a direct store.threads subscription and useMemo, so the O(n) mapping only runs when the threads array reference changes rather than on every store update.
Or push these changes by commenting:
@cursor push dde2fc9315
Preview (dde2fc9315)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -28,7 +28,6 @@
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import { useDebouncedValue } from "@tanstack/react-pacer";
import { useNavigate, useSearch } from "@tanstack/react-router";
-import { useShallow } from "zustand/react/shallow";
import { gitBranchesQueryOptions, gitCreateWorktreeMutationOptions } from "~/lib/gitReactQuery";
import { projectSearchEntriesQueryOptions } from "~/lib/projectReactQuery";
import { serverConfigQueryOptions, serverQueryKeys } from "~/lib/serverReactQuery";
@@ -591,9 +590,8 @@
);
const fallbackDraftProject = useProjectById(draftThread?.projectId);
- const threadPlanCatalog = useStore(
- useShallow((store) => store.threads.map(toThreadPlanCatalogEntry)),
- );
+ const threads = useStore((store) => store.threads);
+ const threadPlanCatalog = useMemo(() => threads.map(toThreadPlanCatalogEntry), [threads]);
const localDraftError = serverThread ? null : (localDraftErrorsByThreadId[threadId] ?? null);
const localDraftThread = useMemo(
() =>
diff --git a/apps/web/src/routes/__root.tsx b/apps/web/src/routes/__root.tsx
--- a/apps/web/src/routes/__root.tsx
+++ b/apps/web/src/routes/__root.tsx
@@ -250,8 +250,10 @@
return;
}
+ const sequenceBefore = recovery.getState().latestSequence;
+
try {
- const events = await api.orchestration.replayEvents(recovery.getState().latestSequence);
+ const events = await api.orchestration.replayEvents(sequenceBefore);
if (!disposed) {
applyEventBatch(events);
}
@@ -261,7 +263,17 @@
return;
}
- if (!disposed && recovery.completeReplayRecovery()) {
+ if (disposed) {
+ return;
+ }
+
+ if (recovery.getState().latestSequence === sequenceBefore) {
+ recovery.failReplayRecovery();
+ void fallbackToSnapshotRecovery();
+ return;
+ }
+
+ if (recovery.completeReplayRecovery()) {
void recoverFromSequenceGap();
}
};- Scope thread plan catalog selection to active/source threads - Avoid immediate replay retries when recovery makes no progress
Contributor
There was a problem hiding this comment.
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: Missing latestTurn regression guard in message-sent handler
- Added the same
thread.latestTurn === null || thread.latestTurn.turnId === event.payload.turnIdguard used bythread.turn-diff-completedto thethread.message-senthandler, preventing late-arriving older-turn messages from regressinglatestTurn.
- Added the same
Or push these changes by commenting:
@cursor push 9a9ecd8f0d
Preview (9a9ecd8f0d)
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
@@ -608,7 +608,9 @@
: [...thread.messages, message];
const cappedMessages = messages.slice(-MAX_THREAD_MESSAGES);
const latestTurn: Thread["latestTurn"] =
- event.payload.role === "assistant" && event.payload.turnId !== null
+ event.payload.role === "assistant" &&
+ event.payload.turnId !== null &&
+ (thread.latestTurn === null || thread.latestTurn.turnId === event.payload.turnId)
? buildLatestTurn({
previous: thread.latestTurn,
turnId: event.payload.turnId,
Member
Author
Add the same defensive guard that thread.turn-diff-completed uses to prevent a late-arriving assistant message from an older turn from overwriting latestTurn, which could regress the current turn's progress. Applied via @cursor push command
Contributor
There was a problem hiding this comment.
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: Incremental event path drops
sourceProposedPlanfromlatestTurn- Added a
pendingSourceProposedPlanfield toThread, storedsourceProposedPlanfrom theturn-start-requestedevent, and passed it as a fallback tobuildLatestTurnso new turns on the incremental path preserve the plan reference.
- Added a
Or push these changes by commenting:
@cursor push 921270aa86
Preview (921270aa86)
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
@@ -158,6 +158,7 @@
archivedAt: thread.archivedAt,
updatedAt: thread.updatedAt,
latestTurn: thread.latestTurn,
+ pendingSourceProposedPlan: thread.latestTurn?.sourceProposedPlan,
branch: thread.branch,
worktreePath: thread.worktreePath,
turnDiffSummaries: thread.checkpoints.map(mapTurnDiffSummary),
@@ -214,7 +215,12 @@
startedAt: string | null;
completedAt: string | null;
assistantMessageId: NonNullable<Thread["latestTurn"]>["assistantMessageId"];
+ sourceProposedPlan?: Thread["pendingSourceProposedPlan"];
}): NonNullable<Thread["latestTurn"]> {
+ const resolvedPlan =
+ params.previous?.turnId === params.turnId
+ ? params.previous.sourceProposedPlan
+ : params.sourceProposedPlan;
return {
turnId: params.turnId,
state: params.state,
@@ -222,9 +228,7 @@
startedAt: params.startedAt,
completedAt: params.completedAt,
assistantMessageId: params.assistantMessageId,
- ...(params.previous?.turnId === params.turnId && params.previous.sourceProposedPlan
- ? { sourceProposedPlan: params.previous.sourceProposedPlan }
- : {}),
+ ...(resolvedPlan ? { sourceProposedPlan: resolvedPlan } : {}),
};
}
@@ -534,6 +538,7 @@
: {}),
runtimeMode: event.payload.runtimeMode,
interactionMode: event.payload.interactionMode,
+ pendingSourceProposedPlan: event.payload.sourceProposedPlan,
updatedAt: event.occurredAt,
}));
return threads === state.threads ? state : { ...state, threads };
@@ -629,6 +634,7 @@
thread.latestTurn?.turnId === event.payload.turnId
? (thread.latestTurn.startedAt ?? event.payload.createdAt)
: event.payload.createdAt,
+ sourceProposedPlan: thread.pendingSourceProposedPlan,
completedAt: event.payload.streaming
? thread.latestTurn?.turnId === event.payload.turnId
? (thread.latestTurn.completedAt ?? null)
@@ -671,6 +677,7 @@
thread.latestTurn?.turnId === event.payload.session.activeTurnId
? thread.latestTurn.assistantMessageId
: null,
+ sourceProposedPlan: thread.pendingSourceProposedPlan,
})
: thread.latestTurn,
updatedAt: event.occurredAt,
@@ -755,6 +762,7 @@
startedAt: thread.latestTurn?.startedAt ?? event.payload.completedAt,
completedAt: event.payload.completedAt,
assistantMessageId: event.payload.assistantMessageId,
+ sourceProposedPlan: thread.pendingSourceProposedPlan,
})
: thread.latestTurn;
return {
diff --git a/apps/web/src/types.ts b/apps/web/src/types.ts
--- a/apps/web/src/types.ts
+++ b/apps/web/src/types.ts
@@ -104,6 +104,7 @@
archivedAt: string | null;
updatedAt?: string | undefined;
latestTurn: OrchestrationLatestTurn | null;
+ pendingSourceProposedPlan?: OrchestrationLatestTurn["sourceProposedPlan"];
branch: string | null;
worktreePath: string | null;
turnDiffSummaries: TurnDiffSummary[];
Member
Author
The thread.turn-start-requested handler updated modelSelection, runtimeMode, and interactionMode but never stored the event's sourceProposedPlan field. Because buildLatestTurn only carries forward sourceProposedPlan from the previous latestTurn when turnIds match, the field was permanently lost for new turns created via the incremental event path. Fix: - Add pendingSourceProposedPlan field to Thread to bridge the gap between turn-start-requested (which has the plan reference but no turnId) and later events that call buildLatestTurn (which create the turn). - Store event.payload.sourceProposedPlan in the turn-start-requested handler. - Pass pendingSourceProposedPlan to buildLatestTurn as a fallback for new turns where the previous latestTurn turnId doesn't match. - Seed pendingSourceProposedPlan in mapThread (snapshot path) for consistency. Applied via @cursor push command
- Reset pending source proposed plans on revert - Avoid carrying stale plan state into the next session-set event - Add regression coverage for revert/session transitions
Contributor
There was a problem hiding this comment.
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: Stale
activeThreadclosure inbeginLocalDispatchcallback- Replaced the closed-over
input.activeThreadwith a ref (activeThreadRef) that is updated on every render, ensuringbeginLocalDispatchalways reads the latest value at invocation time and removing the stale closure risk.
- Replaced the closed-over
Or push these changes by commenting:
@cursor push d708ae814e
Preview (d708ae814e)
diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -341,21 +341,21 @@
}) {
const [localDispatch, setLocalDispatch] = useState<LocalDispatchSnapshot | null>(null);
- const beginLocalDispatch = useCallback(
- (options?: { preparingWorktree?: boolean }) => {
- const preparingWorktree = Boolean(options?.preparingWorktree);
- setLocalDispatch((current) => {
- if (current) {
- return current.preparingWorktree === preparingWorktree
- ? current
- : { ...current, preparingWorktree };
- }
- return createLocalDispatchSnapshot(input.activeThread, options);
- });
- },
- [input.activeThread],
- );
+ const activeThreadRef = useRef(input.activeThread);
+ activeThreadRef.current = input.activeThread;
+ const beginLocalDispatch = useCallback((options?: { preparingWorktree?: boolean }) => {
+ const preparingWorktree = Boolean(options?.preparingWorktree);
+ setLocalDispatch((current) => {
+ if (current) {
+ return current.preparingWorktree === preparingWorktree
+ ? current
+ : { ...current, preparingWorktree };
+ }
+ return createLocalDispatchSnapshot(activeThreadRef.current, options);
+ });
+ }, []);
+
const resetLocalDispatch = useCallback(() => {
setLocalDispatch(null);
}, []);| }); | ||
| }, | ||
| [input.activeThread], | ||
| ); |
Contributor
There was a problem hiding this comment.
Stale activeThread closure in beginLocalDispatch callback
Low Severity
The beginLocalDispatch callback captures input.activeThread via its useCallback dependency, but this value can become stale during an async send flow. When the callback is first invoked (creating a new snapshot from createLocalDispatchSnapshot), it uses the activeThread from the render when beginLocalDispatch was last created rather than the current store state. In cases where thread state changes between renders during an async operation, the snapshot could capture outdated turn/session data, causing hasServerAcknowledgedLocalDispatch to mismatch and delay or prevent clearing the busy state.
Contributor
|
Bugbot Autofix prepared a fix for the issue found in the latest run.
Or push these changes by commenting: Preview (05e3110b0e)diff --git a/apps/web/src/routes/__root.tsx b/apps/web/src/routes/__root.tsx
--- a/apps/web/src/routes/__root.tsx
+++ b/apps/web/src/routes/__root.tsx
@@ -156,13 +156,10 @@
const navigate = useNavigate();
const pathname = useRouterState({ select: (state) => state.location.pathname });
const pathnameRef = useRef(pathname);
+ pathnameRef.current = pathname;
const handledBootstrapThreadIdRef = useRef<string | null>(null);
useEffect(() => {
- pathnameRef.current = pathname;
- }, [pathname]);
-
- useEffect(() => {
const api = readNativeApi();
if (!api) return;
let disposed = false; |
- Prefer the authoritative assistant message ID when anchoring completion dividers - Rebind live turn diff summaries when the final assistant message arrives
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Incremental thread events skip UI state seeding
- Added syncThreads call in applyEventBatch for thread.created and thread.deleted events so incrementally created threads get their seedVisitedAt populated in threadLastVisitedAtById, matching the snapshot recovery path.
- ✅ Fixed: Redundant
useShallowon simple store property selectors- Removed useShallow wrappers from the store.projects and store.threads selectors in Sidebar since Zustand's default Object.is comparison already correctly skips re-renders when these array references are unchanged.
Or push these changes by commenting:
@cursor push d700700e4f
Preview (d700700e4f)
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
@@ -436,8 +436,8 @@
}
export default function Sidebar() {
- const projects = useStore(useShallow((store) => store.projects));
- const serverThreads = useStore(useShallow((store) => store.threads));
+ const projects = useStore((store) => store.projects);
+ const serverThreads = useStore((store) => store.threads);
const { projectExpandedById, projectOrder, threadLastVisitedAtById } = useUiStateStore(
useShallow((store) => ({
projectExpandedById: store.projectExpandedById,
diff --git a/apps/web/src/routes/__root.tsx b/apps/web/src/routes/__root.tsx
--- a/apps/web/src/routes/__root.tsx
+++ b/apps/web/src/routes/__root.tsx
@@ -230,6 +230,18 @@
const projects = useStore.getState().projects;
syncProjects(projects.map((project) => ({ id: project.id, cwd: project.cwd })));
}
+ const needsThreadUiSync = nextEvents.some(
+ (event) => event.type === "thread.created" || event.type === "thread.deleted",
+ );
+ if (needsThreadUiSync) {
+ const threads = useStore.getState().threads;
+ syncThreads(
+ threads.map((thread) => ({
+ id: thread.id,
+ seedVisitedAt: thread.updatedAt ?? thread.createdAt,
+ })),
+ );
+ }
const draftStore = useComposerDraftStore.getState();
for (const threadId of batchEffects.clearPromotedDraftThreadIds) {
clearPromotedDraftThread(threadId);You can send follow-ups to this agent here.
Member
Author
…eShallow - Call syncThreads in applyEventBatch for thread.created/thread.deleted events so incrementally created threads get their seedVisitedAt populated in threadLastVisitedAtById, matching the snapshot recovery path behavior. - Remove unnecessary useShallow wrapper from store.projects and store.threads selectors in Sidebar. Zustand's default Object.is comparison already skips re-renders when these array references are unchanged, making the O(n) shallow comparison pure overhead. Applied via @cursor push command
| }, | ||
| { timeout: 8_000, interval: 16 }, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Test event sequence equals bootstrap sequence, causing "ignore"
Medium Severity
promoteDraftThreadViaDomainEvent sends an event with sequence set to fixture.snapshot.snapshotSequence, but addThreadToSnapshot does not increment the snapshot sequence. After bootstrap, the recovery coordinator sets latestSequence = snapshotSequence, so when the domain event arrives with the same sequence, classifyDomainEvent returns "ignore" (since sequence <= latestSequence). The draft is never cleared and the vi.waitFor assertion times out. The event sequence needs to be greater than the bootstrap snapshot's snapshotSequence.
Additional Locations (1)
Contributor
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
The addThreadToSnapshot function already increments snapshotSequence (line 306: snapshot.snapshotSequence + 1), so the event sequence (2) is always greater than the bootstrap latestSequence (1), and classifyDomainEvent returns "apply" not "ignore" — the bug report's premise that the sequence is not incremented is incorrect.
You can send follow-ups to this agent here.
aaditagrawal
added a commit
to aaditagrawal/t3code
that referenced
this pull request
Mar 30, 2026
Integrates upstream changes including: - Refactor terminal manager onto Effect runtime (pingdotgg#1525) - Refactor web orchestration sync to incremental events and isolated recovery (pingdotgg#1560) - Remove redundant add-project cancel button (pingdotgg#1302) - README documentation updates (pingdotgg#1406, pingdotgg#1564, pingdotgg#1565) Conflict resolution across 8 files: adopted upstream's incremental event store architecture, terminal Effect runtime, and batched orchestration effects while preserving fork's multi-provider state.
5 tasks
Chrono-byte
pushed a commit
to Chrono-byte/t3code
that referenced
this pull request
Mar 31, 2026
…covery (pingdotgg#1560) Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
xddinside
referenced
this pull request
in xddinside/t3code
Apr 4, 2026
…covery (#1560) Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
gigq
pushed a commit
to gigq/t3code
that referenced
this pull request
Apr 6, 2026
…covery (pingdotgg#1560) Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getSnapshot()calls.Testing
bun fmtbun lintbun typecheckbun run test --filter=@t3tools/webcd apps/web && bun run test:browser -- src/components/ChatView.browser.tsxNote
High Risk
Replaces core client state synchronization with incremental event application plus replay/snapshot recovery, and moves persisted UI metadata into a new store; regressions could impact thread/project rendering, navigation guards, and cleanup side effects during reconnects or sequence gaps.
Overview
Refactors orchestration synchronization from snapshot refreshes to incremental domain-event application. The web store now applies
OrchestrationEvents via newapplyOrchestrationEvent(s)reducers, whileEventRouteruses a newcreateOrchestrationRecoveryCoordinatorto bootstrap from snapshot, detect sequence gaps, replay missing events, and fall back to snapshot on replay failure.Splits persisted UI-only state into
uiStateStore. Project expansion/order and thread last-visited/unread metadata are removed from the main store/types and persisted separately; sidebar/chat now consume these via selectors (useProjectById/useThreadById) and updated logic (orderItemsByPreferredIds,latestUserMessageAt) to reduce rerenders.Aligns chat/side effects with event-driven flows. Draft-thread promotion and thread/terminal cleanup are driven by per-batch effects (
deriveOrchestrationBatchEffects) and new draft helpers (clearPromotedDraftThread(s)), ChatView replacesSendPhasewith a server-acknowledgement-based local dispatch tracker, implementation-thread navigation waits for live thread start instead ofgetSnapshot(), and routing readiness switches fromthreadsHydratedtobootstrapComplete.Written by Cursor Bugbot for commit a16234f. This will update automatically on new commits. Configure here.
Note
Refactor web store to apply orchestration events incrementally with gap-based recovery
applyOrchestrationEvent/applyOrchestrationEventsreducers in store.ts, driven by a newcreateOrchestrationRecoveryCoordinatorin orchestrationRecovery.ts that detects sequence gaps and triggers replay or snapshot fallback.lastVisitedAt) into a dedicated uiStateStore.ts with localStorage persistence, removing it from the core store along with all prior persistence helpers andStoreProvider.deriveOrchestrationBatchEffectsin orchestrationEventEffects.ts to compute per-batch side effects: clearing promoted draft threads, purging deleted thread UI/terminal state, and throttled provider query invalidation.ChatView'sSendPhaseenum with auseLocalDispatchStatehook backed byhasServerAcknowledgedLocalDispatch, so busy indicators clear only when the server acknowledges the dispatch rather than on a local timer.deriveCompletionDividerBeforeEntryIdto anchor the completion divider toassistantMessageIdwhen available, falling back to timestamp-based search.StoreProvideris removed entirely — any remaining import sites will fail at build time, and the store no longer hydrates from or persists to localStorage.Macroscope summarized a16234f.