Skip to content

Commit 165ec27

Browse files
committed
fix: guard workspace runtime reattach races
1 parent 7537fe4 commit 165ec27

6 files changed

Lines changed: 225 additions & 13 deletions

File tree

apps/web/src/features/app/WorkbenchRuntimeCoordinator.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ import {
5252
} from "../workspace/workspace-controller";
5353
import { attachWorkspaceRuntimeWithRetry } from "../workspace/runtime-attach";
5454
import { createWorkspaceSessionActions } from "../workspace/session-actions";
55+
import {
56+
advanceWorkspaceSyncVersion,
57+
isWorkspaceSyncVersionCurrent,
58+
} from "../workspace/workspace-sync-version.ts";
5559
import { useWorkspaceTransportSync } from "../workspace/workspace-sync-hooks";
5660

5761
const withServiceFallback = async <T,>(
@@ -400,17 +404,25 @@ export const WorkbenchRuntimeCoordinator = ({
400404
}
401405

402406
const task = (async () => {
407+
const syncVersion = advanceWorkspaceSyncVersion(workspaceId);
403408
const runtimeSnapshot = await attachWorkspaceRuntimeWithRetry(
404409
workspaceId,
405410
deviceId,
406411
clientId,
407412
withServiceFallback,
408413
);
409-
if (!runtimeSnapshot || !hasLiveWorkspaceTab(workspaceId)) {
414+
if (
415+
!runtimeSnapshot
416+
|| !hasLiveWorkspaceTab(workspaceId)
417+
|| !isWorkspaceSyncVersionCurrent(workspaceId, syncVersion)
418+
) {
410419
return;
411420
}
412421
updateState((current) => {
413-
if (!current.tabs.some((tab) => tab.id === workspaceId)) {
422+
if (
423+
!current.tabs.some((tab) => tab.id === workspaceId)
424+
|| !isWorkspaceSyncVersionCurrent(workspaceId, syncVersion)
425+
) {
414426
return current;
415427
}
416428
return applyWorkspaceRuntimeSnapshot(

apps/web/src/features/workspace/WorkspaceScreen.tsx

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ import {
120120
updateWorkspaceOverlayInput,
121121
updateWorkspaceOverlayTarget
122122
} from "./workspace-overlay-actions";
123+
import {
124+
advanceWorkspaceSyncVersion,
125+
isWorkspaceSyncVersionCurrent,
126+
} from "./workspace-sync-version.ts";
123127
import {
124128
buildWorkspaceFileSearchResults,
125129
closeWorkspaceFileSearch,
@@ -461,7 +465,6 @@ export default function WorkspaceScreen({ locale, appSettings, onOpenSettings }:
461465
const validatedRuntimeTargetsRef = useRef(new Set<string>());
462466
const runtimeValidationRequestIdRef = useRef(0);
463467
const overlayBrowseRequestIdRef = useRef(0);
464-
const workspaceSyncVersionRef = useRef(new Map<string, number>());
465468
const persistedLayoutRef = useRef<string>("");
466469
const agentStartupStateRef = useRef(new Map<string, {
467470
token: number;
@@ -579,16 +582,6 @@ export default function WorkspaceScreen({ locale, appSettings, onOpenSettings }:
579582
setState(next);
580583
};
581584

582-
const advanceWorkspaceSyncVersion = useCallback((workspaceId: string) => {
583-
const nextVersion = (workspaceSyncVersionRef.current.get(workspaceId) ?? 0) + 1;
584-
workspaceSyncVersionRef.current.set(workspaceId, nextVersion);
585-
return nextVersion;
586-
}, []);
587-
588-
const isWorkspaceSyncVersionCurrent = useCallback((workspaceId: string, version: number) => (
589-
(workspaceSyncVersionRef.current.get(workspaceId) ?? 0) === version
590-
), []);
591-
592585
const agentRuntimeRefs = useMemo(() => ({
593586
draftPromptInputRefs,
594587
agentTerminalRefs,

apps/web/src/features/workspace/session-actions.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import { createTabFromWorkspaceSnapshot } from "../../shared/utils/workspace.ts"
4343
import type { AppSettings, BackendArchiveEntry, BackendSession, SessionPatch, Toast, WorkspaceSnapshot } from "../../types/app.ts";
4444

4545
import type { CompletionReminderTarget } from "./completion-reminders.ts";
46+
import { advanceWorkspaceSyncVersion } from "./workspace-sync-version.ts";
4647

4748
type UpdateTab = (tabId: string, updater: (tab: Tab) => Tab) => void;
4849
type WithServiceFallback = <T>(operation: () => Promise<T>, fallback: T) => Promise<T>;
@@ -495,6 +496,7 @@ export const createWorkspaceSessionActions = ({
495496
const target = ensureRestorePane(tabId, preferredPaneId);
496497
if (!target) return null;
497498

499+
advanceWorkspaceSyncVersion(tabId);
498500
const restored = await withServiceFallback(
499501
() => restoreSessionRequest(tabId, numericSessionId, controllerForTab(tabId)),
500502
null,
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
export type WorkspaceSyncVersionTracker = {
2+
advance: (workspaceId: string) => number;
3+
read: (workspaceId: string) => number;
4+
isCurrent: (workspaceId: string, version: number) => boolean;
5+
};
6+
7+
export const createWorkspaceSyncVersionTracker = (): WorkspaceSyncVersionTracker => {
8+
const versions = new Map<string, number>();
9+
10+
const read = (workspaceId: string) => versions.get(workspaceId) ?? 0;
11+
12+
return {
13+
advance: (workspaceId: string) => {
14+
const nextVersion = read(workspaceId) + 1;
15+
versions.set(workspaceId, nextVersion);
16+
return nextVersion;
17+
},
18+
read,
19+
isCurrent: (workspaceId: string, version: number) => read(workspaceId) === version,
20+
};
21+
};
22+
23+
const workspaceSyncVersionTracker = createWorkspaceSyncVersionTracker();
24+
25+
export const advanceWorkspaceSyncVersion = (workspaceId: string) => (
26+
workspaceSyncVersionTracker.advance(workspaceId)
27+
);
28+
29+
export const readWorkspaceSyncVersion = (workspaceId: string) => (
30+
workspaceSyncVersionTracker.read(workspaceId)
31+
);
32+
33+
export const isWorkspaceSyncVersionCurrent = (workspaceId: string, version: number) => (
34+
workspaceSyncVersionTracker.isCurrent(workspaceId, version)
35+
);

tests/session-actions.test.ts

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import test from 'node:test';
22
import assert from 'node:assert/strict';
33
import { createTranslator } from '../apps/web/src/i18n.ts';
44
import { createWorkspaceSessionActions } from '../apps/web/src/features/workspace/session-actions.ts';
5+
import { readWorkspaceSyncVersion } from '../apps/web/src/features/workspace/workspace-sync-version.ts';
56
import type { AppSettings, Toast } from '../apps/web/src/types/app.ts';
67
import type { WorkbenchState } from '../apps/web/src/state/workbench.ts';
78

@@ -200,3 +201,134 @@ test('markSessionIdle does not trigger completion reminder for agent exit notes'
200201

201202
assert.deepEqual(reminders, []);
202203
});
204+
205+
test('restoreSessionIntoPane bumps the workspace sync version before applying the restored session', async () => {
206+
const locale = 'en';
207+
const t = createTranslator(locale);
208+
const workspaceId = 'ws-restore-sync';
209+
const beforeVersion = readWorkspaceSyncVersion(workspaceId);
210+
const stateRef = {
211+
current: {
212+
activeTabId: workspaceId,
213+
layout: {
214+
leftWidth: 320,
215+
rightWidth: 320,
216+
rightSplit: 64,
217+
showCodePanel: false,
218+
showTerminalPanel: false,
219+
},
220+
overlay: {
221+
visible: false,
222+
mode: 'local' as const,
223+
input: '',
224+
target: { type: 'native' as const },
225+
},
226+
tabs: [
227+
{
228+
id: workspaceId,
229+
title: 'Workspace Restore',
230+
status: 'ready' as const,
231+
controller: {
232+
role: 'controller' as const,
233+
deviceId: 'device-a',
234+
clientId: 'client-a',
235+
fencingToken: 1,
236+
takeoverPending: false,
237+
takeoverRequestedBySelf: false,
238+
},
239+
agent: {
240+
provider: 'claude' as const,
241+
command: 'claude',
242+
useWsl: false,
243+
},
244+
git: { branch: 'main', changes: 0, lastCommit: 'abc123' },
245+
gitChanges: [],
246+
worktrees: [],
247+
sessions: [
248+
{
249+
id: 'draft-restore',
250+
title: 'Session 1',
251+
status: 'idle' as const,
252+
mode: 'branch' as const,
253+
autoFeed: true,
254+
queue: [],
255+
messages: [],
256+
stream: '',
257+
unread: 0,
258+
lastActiveAt: 1,
259+
isDraft: true,
260+
},
261+
],
262+
activeSessionId: 'draft-restore',
263+
archive: [],
264+
terminals: [],
265+
activeTerminalId: '',
266+
fileTree: [],
267+
changesTree: [],
268+
filePreview: {
269+
path: '',
270+
content: '',
271+
mode: 'preview' as const,
272+
originalContent: '',
273+
modifiedContent: '',
274+
dirty: false,
275+
},
276+
paneLayout: {
277+
type: 'leaf' as const,
278+
id: 'pane-draft',
279+
sessionId: 'draft-restore',
280+
},
281+
activePaneId: 'pane-draft',
282+
idlePolicy: {
283+
enabled: true,
284+
idleMinutes: 10,
285+
maxActive: 3,
286+
pressure: true,
287+
},
288+
},
289+
],
290+
} satisfies WorkbenchState,
291+
};
292+
293+
const actions = createWorkspaceSessionActions({
294+
appSettings: defaultAppSettings(),
295+
locale,
296+
t,
297+
stateRef,
298+
updateTab: (tabId, updater) => {
299+
stateRef.current = {
300+
...stateRef.current,
301+
tabs: stateRef.current.tabs.map((tab) => (tab.id === tabId ? updater(tab) : tab)),
302+
};
303+
},
304+
withServiceFallback: async (_operation, fallback) => {
305+
if (fallback === null) {
306+
return {
307+
session: {
308+
id: 7,
309+
title: 'History Restore Session',
310+
status: 'idle' as const,
311+
mode: 'branch' as const,
312+
auto_feed: true,
313+
queue: [],
314+
messages: [],
315+
stream: '',
316+
unread: 0,
317+
last_active_at: 10,
318+
claude_session_id: 'claude-restore-sync',
319+
},
320+
alreadyActive: false,
321+
};
322+
}
323+
return fallback;
324+
},
325+
addToast: () => {},
326+
});
327+
328+
const restored = await actions.restoreSessionIntoPane(workspaceId, '7');
329+
330+
assert.equal(restored?.id, 7);
331+
assert.equal(readWorkspaceSyncVersion(workspaceId), beforeVersion + 1);
332+
assert.equal(stateRef.current.tabs[0]?.activeSessionId, '7');
333+
assert.equal(stateRef.current.tabs[0]?.sessions[0]?.title, 'History Restore Session');
334+
});
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import test from "node:test";
2+
import assert from "node:assert/strict";
3+
import { createWorkspaceSyncVersionTracker } from "../apps/web/src/features/workspace/workspace-sync-version.ts";
4+
5+
test("later sync versions invalidate earlier in-flight syncs for the same workspace", () => {
6+
const tracker = createWorkspaceSyncVersionTracker();
7+
8+
const firstVersion = tracker.advance("ws-sync-race");
9+
const secondVersion = tracker.advance("ws-sync-race");
10+
11+
assert.equal(firstVersion, 1);
12+
assert.equal(secondVersion, 2);
13+
assert.equal(tracker.isCurrent("ws-sync-race", firstVersion), false);
14+
assert.equal(tracker.isCurrent("ws-sync-race", secondVersion), true);
15+
});
16+
17+
test("sync versions stay isolated per workspace", () => {
18+
const tracker = createWorkspaceSyncVersionTracker();
19+
20+
const leftVersion = tracker.advance("ws-sync-left");
21+
const rightVersion = tracker.advance("ws-sync-right");
22+
23+
assert.equal(leftVersion, 1);
24+
assert.equal(rightVersion, 1);
25+
assert.equal(tracker.isCurrent("ws-sync-left", leftVersion), true);
26+
assert.equal(tracker.isCurrent("ws-sync-right", rightVersion), true);
27+
});
28+
29+
test("a local mutation bump invalidates an older attach version", () => {
30+
const tracker = createWorkspaceSyncVersionTracker();
31+
32+
const attachVersion = tracker.advance("ws-sync-restore");
33+
const restoreVersion = tracker.advance("ws-sync-restore");
34+
35+
assert.equal(restoreVersion, attachVersion + 1);
36+
assert.equal(tracker.isCurrent("ws-sync-restore", attachVersion), false);
37+
assert.equal(tracker.isCurrent("ws-sync-restore", restoreVersion), true);
38+
});

0 commit comments

Comments
 (0)