Skip to content

Commit 62dd736

Browse files
committed
fix(web): recover stale thread routes
1 parent c8f5862 commit 62dd736

10 files changed

Lines changed: 266 additions & 12 deletions

apps/web/src/components/EventRouter.browser.tsx

Lines changed: 128 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,13 +351,16 @@ const worker = setupWorker(
351351
);
352352

353353
async function mountApp(options?: {
354+
initialEntry?: string;
354355
routeThreadId?: ThreadId;
355356
waitForThreadId?: ThreadId | null;
356-
}): Promise<{ cleanup: () => Promise<void> }> {
357+
}): Promise<{ cleanup: () => Promise<void>; router: ReturnType<typeof getRouter> }> {
357358
const host = createFullscreenTestHost();
358359

359360
const routeThreadId = options?.routeThreadId ?? THREAD_ID;
360-
const router = getRouter(createMemoryHistory({ initialEntries: [`/${routeThreadId}`] }));
361+
const router = getRouter(
362+
createMemoryHistory({ initialEntries: [options?.initialEntry ?? `/${routeThreadId}`] }),
363+
);
361364
const screen = await render(<RouterProvider router={router} />, { container: host });
362365

363366
try {
@@ -391,6 +394,7 @@ async function mountApp(options?: {
391394
let cleanedUp = false;
392395

393396
return {
397+
router,
394398
cleanup: async () => {
395399
if (cleanedUp) return;
396400
cleanedUp = true;
@@ -522,6 +526,128 @@ describe("EventRouter scoped orchestration sync", () => {
522526
}
523527
});
524528

529+
it("recovers a stale remembered route without opening a missing detail subscription", async () => {
530+
const staleThreadId = ThreadId.makeUnsafe("thread-stale-route");
531+
localStorage.setItem(
532+
"synara:sidebar-ui:v1",
533+
JSON.stringify({
534+
chatSectionExpanded: true,
535+
chatThreadListExtraPages: 2,
536+
projectThreadListExtraPagesByCwd: { "/repo/project": 1 },
537+
dismissedThreadStatusKeyByThreadId: {},
538+
lastThreadRoute: { threadId: staleThreadId },
539+
activityViewEnabled: false,
540+
}),
541+
);
542+
const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
543+
const mounted = await mountApp({ routeThreadId: staleThreadId, waitForThreadId: null });
544+
545+
try {
546+
await vi.waitFor(
547+
() => {
548+
expect(mounted.router.state.location.pathname).not.toBe(`/${staleThreadId}`);
549+
expect(
550+
JSON.parse(localStorage.getItem("synara:sidebar-ui:v1") ?? "null")?.lastThreadRoute,
551+
).toBeNull();
552+
},
553+
{ timeout: 4_000, interval: 16 },
554+
);
555+
await new Promise((resolve) => window.setTimeout(resolve, 120));
556+
expect(subscribeThreadRequestCountById.get(staleThreadId)).toBeUndefined();
557+
expect(consoleError).not.toHaveBeenCalled();
558+
} finally {
559+
consoleError.mockRestore();
560+
await mounted.cleanup();
561+
}
562+
});
563+
564+
it("restores a valid remembered thread from the root route", async () => {
565+
localStorage.setItem(
566+
"synara:sidebar-ui:v1",
567+
JSON.stringify({
568+
lastThreadRoute: { threadId: THREAD_ID },
569+
}),
570+
);
571+
const mounted = await mountApp({ initialEntry: "/" });
572+
573+
try {
574+
await vi.waitFor(
575+
() => {
576+
expect(mounted.router.state.location.pathname).toBe(`/${THREAD_ID}`);
577+
expect(subscribeThreadRequestCountById.get(THREAD_ID)).toBeGreaterThanOrEqual(1);
578+
},
579+
{ timeout: 4_000, interval: 16 },
580+
);
581+
} finally {
582+
await mounted.cleanup();
583+
}
584+
});
585+
586+
it("clears a missing remembered thread while resolving the root route", async () => {
587+
const staleThreadId = ThreadId.makeUnsafe("thread-stale-root-restore");
588+
localStorage.setItem(
589+
"synara:sidebar-ui:v1",
590+
JSON.stringify({
591+
chatSectionExpanded: true,
592+
lastThreadRoute: { threadId: staleThreadId },
593+
}),
594+
);
595+
const mounted = await mountApp({ initialEntry: "/", waitForThreadId: null });
596+
597+
try {
598+
await vi.waitFor(
599+
() => {
600+
expect(
601+
JSON.parse(localStorage.getItem("synara:sidebar-ui:v1") ?? "null")?.lastThreadRoute,
602+
).toBeNull();
603+
expect(mounted.router.state.location.pathname).not.toBe(`/${staleThreadId}`);
604+
},
605+
{ timeout: 4_000, interval: 16 },
606+
);
607+
expect(subscribeThreadRequestCountById.get(staleThreadId)).toBeUndefined();
608+
} finally {
609+
await mounted.cleanup();
610+
}
611+
});
612+
613+
it("stops leasing a deleted route and clears its remembered location after recovery", async () => {
614+
localStorage.setItem(
615+
"synara:sidebar-ui:v1",
616+
JSON.stringify({
617+
lastThreadRoute: { threadId: THREAD_ID },
618+
}),
619+
);
620+
const mounted = await mountApp();
621+
622+
try {
623+
const subscribeCountBeforeDelete = subscribeThreadRequestCountById.get(THREAD_ID) ?? 0;
624+
fixture.snapshot = {
625+
...fixture.snapshot,
626+
snapshotSequence: 2,
627+
threads: [],
628+
};
629+
sendShellEventPush({
630+
kind: "thread-removed",
631+
sequence: 2,
632+
threadId: THREAD_ID,
633+
});
634+
635+
await vi.waitFor(
636+
() => {
637+
expect(mounted.router.state.location.pathname).not.toBe(`/${THREAD_ID}`);
638+
expect(
639+
JSON.parse(localStorage.getItem("synara:sidebar-ui:v1") ?? "null")?.lastThreadRoute,
640+
).toBeNull();
641+
},
642+
{ timeout: 5_000, interval: 16 },
643+
);
644+
await new Promise((resolve) => window.setTimeout(resolve, 120));
645+
expect(subscribeThreadRequestCountById.get(THREAD_ID)).toBe(subscribeCountBeforeDelete);
646+
} finally {
647+
await mounted.cleanup();
648+
}
649+
});
650+
525651
it("drops duplicate thread events after the thread snapshot sequence advances", async () => {
526652
const mounted = await mountApp();
527653

apps/web/src/components/RestoreOrCreateChatRoute.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,15 @@ export type RestoreRouteResolver = (input: RestoreRouteResolverInput) => LastThr
4040
export function RestoreOrCreateChatRoute({
4141
resolveRestoreRoute,
4242
createFreshChat,
43+
onUnrestorableRememberedRoute,
4344
}: {
4445
// Surface-specific policy for picking the thread route to restore (e.g. the last-visited route
4546
// for home chats, the latest Studio thread or draft for Studio). The remembered-route recovery
4647
// below still keys off the total thread count, which is shared across surfaces.
4748
readonly resolveRestoreRoute: RestoreRouteResolver;
4849
readonly createFreshChat: () => Promise<StartContainerChatResult>;
50+
/** Called only after hydration/recovery has made fallback authoritative. */
51+
readonly onUnrestorableRememberedRoute?: (route: LastThreadRoute) => void;
4952
}) {
5053
const navigate = useNavigate();
5154
const threadsHydrated = useStore((store) => store.threadsHydrated);
@@ -150,6 +153,9 @@ export function RestoreOrCreateChatRoute({
150153
if (cancelled || createFreshChatInFlightRef.current) {
151154
return;
152155
}
156+
if (lastThreadRoute) {
157+
onUnrestorableRememberedRoute?.(lastThreadRoute);
158+
}
153159
createFreshChatInFlightRef.current = true;
154160
// .finally instead of try/finally: React Compiler does not yet support
155161
// try/finally and would skip optimizing this whole component.
@@ -170,6 +176,7 @@ export function RestoreOrCreateChatRoute({
170176
createFreshChat,
171177
emptyRestoreRecoveryState,
172178
navigate,
179+
onUnrestorableRememberedRoute,
173180
resolveRestoreRoute,
174181
splitViewIds,
175182
splitViewsHydrated,

apps/web/src/components/Sidebar.uiState.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { afterEach, beforeEach, describe, expect, it } from "vitest";
22

33
import {
4+
clearLastThreadRouteIfMatches,
45
normalizeSidebarProjectThreadListCwd,
56
persistSidebarUiState,
67
readSidebarUiState,
8+
subscribeSidebarUiState,
79
} from "./Sidebar.uiState";
810

911
describe("Sidebar.uiState", () => {
@@ -14,6 +16,8 @@ describe("Sidebar.uiState", () => {
1416
Object.defineProperty(globalThis, "window", {
1517
configurable: true,
1618
value: {
19+
addEventListener: () => {},
20+
removeEventListener: () => {},
1721
localStorage: {
1822
clear: () => {
1923
storage.clear();
@@ -163,4 +167,48 @@ describe("Sidebar.uiState", () => {
163167
activityViewEnabled: false,
164168
});
165169
});
170+
171+
it("atomically clears only a matching remembered route and preserves unrelated state", () => {
172+
persistSidebarUiState({
173+
chatSectionExpanded: true,
174+
chatThreadListExtraPages: 3,
175+
projectThreadListExtraPagesByCwd: { "/repo": 2 },
176+
dismissedThreadStatusKeyByThreadId: { "thread-other": "ready:turn-1" },
177+
lastThreadRoute: { threadId: "thread-stale", splitViewId: "split-stale" },
178+
activityViewEnabled: true,
179+
});
180+
181+
expect(clearLastThreadRouteIfMatches("thread-other")).toBe(false);
182+
expect(readSidebarUiState().lastThreadRoute?.threadId).toBe("thread-stale");
183+
expect(clearLastThreadRouteIfMatches("thread-stale")).toBe(true);
184+
expect(readSidebarUiState()).toEqual({
185+
chatSectionExpanded: true,
186+
chatThreadListExtraPages: 3,
187+
projectThreadListExtraPagesByCwd: {
188+
[normalizeSidebarProjectThreadListCwd("/repo")]: 2,
189+
},
190+
dismissedThreadStatusKeyByThreadId: { "thread-other": "ready:turn-1" },
191+
lastThreadRoute: null,
192+
activityViewEnabled: true,
193+
});
194+
});
195+
196+
it("notifies same-window sidebar consumers after a matching clear", () => {
197+
persistSidebarUiState({
198+
chatSectionExpanded: false,
199+
chatThreadListExtraPages: 0,
200+
projectThreadListExtraPagesByCwd: {},
201+
dismissedThreadStatusKeyByThreadId: {},
202+
lastThreadRoute: { threadId: "thread-stale" },
203+
activityViewEnabled: false,
204+
});
205+
const observed: Array<ReturnType<typeof readSidebarUiState>> = [];
206+
const unsubscribe = subscribeSidebarUiState((state) => observed.push(state));
207+
208+
expect(clearLastThreadRouteIfMatches("thread-stale")).toBe(true);
209+
expect(observed).toHaveLength(1);
210+
expect(observed[0]?.lastThreadRoute).toBeNull();
211+
212+
unsubscribe();
213+
});
166214
});

apps/web/src/components/Sidebar.uiState.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ export type SidebarUiState = {
1818
activityViewEnabled: boolean;
1919
};
2020

21+
const sameWindowListeners = new Set<(state: SidebarUiState) => void>();
22+
2123
const DEFAULT_SIDEBAR_UI_STATE: SidebarUiState = {
2224
chatSectionExpanded: false,
2325
chatThreadListExtraPages: 0,
@@ -153,13 +155,17 @@ export function subscribeSidebarUiState(listener: (state: SidebarUiState) => voi
153155
if (event.key !== SIDEBAR_UI_STATE_STORAGE_KEY) return;
154156
listener(readSidebarUiState());
155157
};
158+
sameWindowListeners.add(listener);
156159
window.addEventListener("storage", handleStorage);
157-
return () => window.removeEventListener("storage", handleStorage);
160+
return () => {
161+
sameWindowListeners.delete(listener);
162+
window.removeEventListener("storage", handleStorage);
163+
};
158164
}
159165

160-
export function persistSidebarUiState(input: SidebarUiState): void {
166+
function writeSidebarUiState(input: SidebarUiState): boolean {
161167
if (typeof window === "undefined") {
162-
return;
168+
return false;
163169
}
164170

165171
try {
@@ -187,7 +193,34 @@ export function persistSidebarUiState(input: SidebarUiState): void {
187193
activityViewEnabled: input.activityViewEnabled,
188194
}),
189195
);
196+
return true;
190197
} catch {
191198
// Ignore storage errors so sidebar rendering keeps working when persistence is unavailable.
199+
return false;
200+
}
201+
}
202+
203+
export function persistSidebarUiState(input: SidebarUiState): void {
204+
writeSidebarUiState(input);
205+
}
206+
207+
/**
208+
* Atomically clears a stale remembered route without overwriting unrelated sidebar state.
209+
* Storage events do not fire in the window that performed the write, so same-window consumers
210+
* receive an explicit notification after the persisted compare-and-clear succeeds.
211+
*/
212+
export function clearLastThreadRouteIfMatches(threadId: string): boolean {
213+
const current = readSidebarUiState();
214+
if (current.lastThreadRoute?.threadId !== threadId) {
215+
return false;
216+
}
217+
218+
const next: SidebarUiState = { ...current, lastThreadRoute: null };
219+
if (!writeSidebarUiState(next)) {
220+
return false;
221+
}
222+
for (const listener of sameWindowListeners) {
223+
listener(next);
192224
}
225+
return true;
193226
}

apps/web/src/hooks/usePreloadRouteChunks.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ export function usePreloadRouteChunks() {
1717
// New-task navigation is a primary startup action. Warm that route as soon
1818
// as the root commits so an immediate click never waits for the browser's
1919
// idle callback (which can be delayed for several seconds during hydration).
20-
router.preloadRoute({ to: "/$threadId", params: { threadId: "chunk-preload" } }).catch(() => {
20+
router.loadRouteChunk(router.routesByPath["/$threadId"]).catch(() => {
2121
// Preloading is best-effort; navigation falls back to loading on demand.
2222
});
2323

2424
const preloadSettings = () => {
25-
router.preloadRoute({ to: "/settings" }).catch(() => {
25+
router.loadRouteChunk(router.routesByPath["/settings"]).catch(() => {
2626
// Preloading is best-effort; navigation falls back to loading on demand.
2727
});
2828
};

apps/web/src/routes/__root.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,13 +1100,26 @@ function EventRouter() {
11001100
);
11011101
const retainedThreadIds = useRetainedThreadDetailIds();
11021102
const serverThreadIdSet = useMemo(() => new Set(serverThreadIds), [serverThreadIds]);
1103+
const localDraftThreadIdKey = useComposerDraftStore((state) =>
1104+
Object.keys(state.draftThreadsByThreadId).join("\0"),
1105+
);
1106+
const localDraftThreadIdSet = useMemo(
1107+
() =>
1108+
new Set(
1109+
localDraftThreadIdKey.length === 0
1110+
? []
1111+
: localDraftThreadIdKey.split("\0").map((threadId) => ThreadId.makeUnsafe(threadId)),
1112+
),
1113+
[localDraftThreadIdKey],
1114+
);
11031115
// Stabilize the lease array by content: `serverThreads` re-emits on every
11041116
// streaming update, and an identity-changing lease list would enqueue a no-op
11051117
// subscription reconcile per render onto the serialized subscribe chain.
11061118
const nextSubscribedThreadIds = resolveThreadDetailSubscriptionLeaseIds({
11071119
visibleThreadIds,
11081120
retainedThreadIds,
11091121
serverThreadIds: serverThreadIdSet,
1122+
localDraftThreadIds: localDraftThreadIdSet,
11101123
});
11111124
const subscribedThreadIdsRef = useRef(nextSubscribedThreadIds);
11121125
const subscribedThreadIds = arraysShallowEqual(

apps/web/src/routes/_chat.$threadId.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { useStore } from "../store";
2424
import { createThreadExistsSelector, createThreadProjectIdSelector } from "../storeSelectors";
2525
import { SingleChatSurface } from "../components/chat/SingleChatSurface";
2626
import { SplitChatSurface } from "../components/chat/SplitChatSurface";
27+
import { clearLastThreadRouteIfMatches } from "../components/Sidebar.uiState";
2728
import { resolveSingleProjectId } from "./-chatThreadRoute.logic";
2829

2930
function ChatThreadRouteView() {
@@ -150,6 +151,7 @@ function ChatThreadRouteView() {
150151
}
151152

152153
if (!routeThreadExists) {
154+
clearLastThreadRouteIfMatches(threadId);
153155
void navigate({ to: "/", replace: true });
154156
}
155157
}, [

0 commit comments

Comments
 (0)