Skip to content

Commit 69ab955

Browse files
nedtwiggclaude
andcommitted
Give browser surfaces a first-class activity slot; centralize browser pane record
- Move browser-surface TODO out of the transient primedActivityStates map into a dedicated localSurfaceActivity store (cleared on kill/replace, never consumed by terminal creation, live PTY outranks a stale reused-id entry) - Extract shared restoreBrowserSurfaceTodo helper for the restore/resume paths - Centralize the persisted browser-pane shape into browserPersistedPane() in lib, shared by session-save and the VS Code host refresh Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 05a5eb2 commit 69ab955

11 files changed

Lines changed: 138 additions & 80 deletions

lib/src/components/Wall.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { markAgentBrowserSessionClosed } from './wall/agent-browser-sessions';
1515
import { KILL_CONFIRM_MS, KILL_SHAKE_MS, KillConfirmOverlay, randomKillChar, type ConfirmKill } from './KillConfirm';
1616
import {
1717
clearSessionAttention,
18-
clearPrimedActivity,
18+
clearLocalSurfaceActivity,
1919
disposeSession,
2020
dismissOrToggleAlert,
2121
focusSession,
@@ -598,7 +598,7 @@ export function Wall({
598598
if (!api || !panel) return;
599599
closeAgentBrowserSession(panel.params);
600600
orchestrateKill(api, id, selectPane, setSelectedId, killInProgressRef, overlayElRef);
601-
clearPrimedActivity(id);
601+
clearLocalSurfaceActivity(id);
602602
fireEvent({ type: 'kill', id });
603603
}, [fireEvent, selectPane]);
604604

@@ -1093,7 +1093,7 @@ export function Wall({
10931093
position: { referencePanel: panel, direction: 'within' },
10941094
});
10951095
api.removePanel(panel);
1096-
clearPrimedActivity(oldId);
1096+
clearLocalSurfaceActivity(oldId);
10971097
selectPane(newId);
10981098
return newId;
10991099
}, [generatePaneId, selectPane]);

lib/src/lib/reconnect.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@ import type { PlatformAdapter, PtyInfo } from './platform/types';
33
import type { PersistedSession } from './session-types';
44

55
const terminalRegistryMocks = vi.hoisted(() => ({
6-
primeActivity: vi.fn(),
6+
restoreBrowserSurfaceTodo: vi.fn(),
77
resumeTerminal: vi.fn(),
88
restoreTerminal: vi.fn(),
99
}));
1010

1111
vi.mock('./terminal-registry', () => ({
12-
primeActivity: terminalRegistryMocks.primeActivity,
12+
restoreBrowserSurfaceTodo: terminalRegistryMocks.restoreBrowserSurfaceTodo,
1313
resumeTerminal: terminalRegistryMocks.resumeTerminal,
1414
restoreTerminal: terminalRegistryMocks.restoreTerminal,
1515
}));
@@ -320,7 +320,7 @@ describe('resumeOrRestore', () => {
320320
expect(terminalRegistryMocks.resumeTerminal).toHaveBeenCalledWith('pane-term', 'pane-term-replay', expect.anything());
321321
});
322322

323-
it('primes browser surface TODO state from persisted alert during live resume', async () => {
323+
it('restores browser surface TODO from the persisted alert during live resume', async () => {
324324
const layout = { panels: { 'pane-term': {}, 'pane-web': {} } };
325325
const saved: PersistedSession = {
326326
version: 3,
@@ -343,12 +343,16 @@ describe('resumeOrRestore', () => {
343343
{ id: 'pane-term', alive: true },
344344
], saved));
345345

346-
expect(terminalRegistryMocks.primeActivity).toHaveBeenCalledWith('pane-web', {
347-
status: 'WATCHING_DISABLED',
348-
watchingEnabled: false,
349-
todo: true,
350-
notification: null,
351-
});
346+
// Resume delegates the browser pane to restoreBrowserSurfaceTodo, which owns
347+
// routing the persisted TODO into the local activity store (verified against
348+
// the real store in terminal-registry.alert.test.ts).
349+
expect(terminalRegistryMocks.restoreBrowserSurfaceTodo).toHaveBeenCalledWith(
350+
expect.objectContaining({
351+
id: 'pane-web',
352+
surfaceType: 'browser',
353+
alert: expect.objectContaining({ todo: true }),
354+
}),
355+
);
352356
});
353357

354358
it('drops visible browser panes from terminal fallback when the saved layout is rejected', async () => {

lib/src/lib/reconnect.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { PlatformAdapter, PtyInfo } from './platform/types';
2-
import { primeActivity, resumeTerminal } from './terminal-registry';
2+
import { restoreBrowserSurfaceTodo, resumeTerminal } from './terminal-registry';
33
import { readPersistedSession, type PersistedDoor } from './session-types';
44
import { restoreSession } from './session-restore';
55

@@ -102,14 +102,7 @@ function getSavedPaneResumeInfo(savedState: unknown, liveIds: string[]): Map<str
102102
const liveSet = new Set(liveIds);
103103
const result = new Map<string, { title: string; untouched: boolean }>();
104104
for (const pane of saved.panes) {
105-
if (pane.surfaceType === 'browser' && pane.alert?.todo === true) {
106-
primeActivity(pane.id, {
107-
status: 'WATCHING_DISABLED',
108-
watchingEnabled: false,
109-
todo: true,
110-
notification: null,
111-
});
112-
}
105+
restoreBrowserSurfaceTodo(pane);
113106
if (!liveSet.has(pane.id)) continue;
114107
result.set(pane.id, { title: pane.title, untouched: pane.untouched });
115108
}

lib/src/lib/session-activity-store.ts

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { SessionStatus } from './activity-monitor';
22
import type { AlertButtonActionResult } from './alert-manager';
33
import type { AlertStateDetail } from './platform/types';
4-
import type { PersistedAlertState } from './session-types';
4+
import type { PersistedAlertState, PersistedPane } from './session-types';
55
import { getPlatform } from './platform';
66
import {
77
getEntryByPtyId,
@@ -21,8 +21,19 @@ export const DEFAULT_ACTIVITY_STATE: ActivityState = {
2121

2222
const activityListeners = new Set<() => void>();
2323
let cachedSnapshot: Map<string, ActivityState> | null = null;
24+
25+
// Transient staging for activity that arrives *before* a terminal registry entry
26+
// exists. Consumed and deleted when the entry is minted (consumePrimedActivity).
2427
const primedActivityStates = new Map<string, Partial<ActivityState>>();
2528

29+
// Persistent activity for non-PTY surfaces (browser iframes / agent-browser).
30+
// A browser surface never gets a registry entry, so — unlike primedActivityStates
31+
// — this is its permanent home: keyed by pane id, written when its TODO toggles
32+
// or is restored, and cleared only when the pane is killed or replaced
33+
// (clearLocalSurfaceActivity). Kept separate so terminal creation never consumes
34+
// it and a no-arg primed reset never wipes it.
35+
const localSurfaceActivity = new Map<string, ActivityState>();
36+
2637
export function notifyActivityListeners(): void {
2738
cachedSnapshot = null;
2839
activityListeners.forEach((listener) => listener());
@@ -37,7 +48,7 @@ export function getActivitySnapshot(): Map<string, ActivityState> {
3748
if (cachedSnapshot) return cachedSnapshot;
3849

3950
const snapshot = new Map<string, ActivityState>();
40-
const ids = new Set<string>([...registry.keys(), ...primedActivityStates.keys()]);
51+
const ids = new Set<string>([...registry.keys(), ...primedActivityStates.keys(), ...localSurfaceActivity.keys()]);
4152
for (const id of ids) {
4253
const state = readActivity(id);
4354
if (state) {
@@ -67,10 +78,13 @@ function readLiveActivity(id: string): ActivityState | null {
6778
function readActivity(id: string): ActivityState | null {
6879
const primedState = primedActivityStates.get(id);
6980
const liveState = readLiveActivity(id);
81+
const localState = localSurfaceActivity.get(id);
7082

71-
if (!liveState && !primedState) return null;
83+
if (!liveState && !primedState && !localState) return null;
84+
// A live PTY is authoritative, so it outranks a stale local-surface entry left
85+
// behind if an id is reused; primed staging overrides on top.
7286
return {
73-
...(liveState ?? DEFAULT_ACTIVITY_STATE),
87+
...(liveState ?? localState ?? DEFAULT_ACTIVITY_STATE),
7488
...primedState,
7589
};
7690
}
@@ -103,24 +117,38 @@ export function clearPrimedActivity(id?: string): void {
103117
notifyActivityListeners();
104118
}
105119

120+
/**
121+
* Drop the activity for a non-PTY surface. Called when a browser pane is killed
122+
* or replaced (Wall.tsx) so its TODO doesn't outlive the pane or leak onto a
123+
* later terminal that reuses the id.
124+
*/
125+
export function clearLocalSurfaceActivity(id: string): void {
126+
if (!localSurfaceActivity.delete(id)) return;
127+
notifyActivityListeners();
128+
}
129+
106130
function setLocalSurfaceTodo(id: string, todo: boolean): void {
107131
if (!todo) {
108-
if (!primedActivityStates.delete(id)) return;
109-
notifyActivityListeners();
132+
clearLocalSurfaceActivity(id);
110133
return;
111134
}
112135

113-
const current = readActivity(id) ?? DEFAULT_ACTIVITY_STATE;
114-
primedActivityStates.set(id, {
115-
...current,
116-
status: 'WATCHING_DISABLED',
117-
watchingEnabled: false,
118-
todo: true,
119-
notification: null,
120-
});
136+
localSurfaceActivity.set(id, { ...DEFAULT_ACTIVITY_STATE, todo: true });
121137
notifyActivityListeners();
122138
}
123139

140+
/**
141+
* Restore a browser surface's persisted TODO into the local activity store.
142+
* Browser surfaces have no PTY, so the TODO is reconstructed from the saved pane
143+
* (the `alert` blob) rather than replayed from a PTY alert. Shared by the cold
144+
* restore (session-restore.ts) and live resume (reconnect.ts) paths.
145+
*/
146+
export function restoreBrowserSurfaceTodo(pane: Pick<PersistedPane, 'id' | 'surfaceType' | 'alert'>): void {
147+
if (pane.surfaceType === 'browser' && pane.alert?.todo === true) {
148+
setLocalSurfaceTodo(pane.id, true);
149+
}
150+
}
151+
124152
export function consumePrimedActivity(id: string): Partial<ActivityState> | undefined {
125153
const primed = primedActivityStates.get(id);
126154
if (primed) {

lib/src/lib/session-restore.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ import type { PersistedSession } from './session-types';
44

55
const terminalRegistryMocks = vi.hoisted(() => ({
66
getDefaultShellOpts: vi.fn(),
7-
primeActivity: vi.fn(),
7+
restoreBrowserSurfaceTodo: vi.fn(),
88
restoreTerminal: vi.fn(),
99
}));
1010

1111
vi.mock('./terminal-registry', () => ({
1212
getDefaultShellOpts: terminalRegistryMocks.getDefaultShellOpts,
13-
primeActivity: terminalRegistryMocks.primeActivity,
13+
restoreBrowserSurfaceTodo: terminalRegistryMocks.restoreBrowserSurfaceTodo,
1414
restoreTerminal: terminalRegistryMocks.restoreTerminal,
1515
}));
1616

@@ -123,7 +123,7 @@ describe('restoreSession', () => {
123123
expect(result?.paneIds).toEqual(['pane-term', 'pane-web']);
124124
});
125125

126-
it('primes browser surface TODO state from persisted alert during cold restore', () => {
126+
it('restores browser surface TODO from the persisted alert during cold restore', () => {
127127
const saved: PersistedSession = {
128128
version: 3,
129129
layout: { panels: { 'pane-web': {} } },
@@ -144,11 +144,15 @@ describe('restoreSession', () => {
144144
restoreSession(createPlatform(saved));
145145

146146
expect(terminalRegistryMocks.restoreTerminal).not.toHaveBeenCalled();
147-
expect(terminalRegistryMocks.primeActivity).toHaveBeenCalledWith('pane-web', {
148-
status: 'WATCHING_DISABLED',
149-
watchingEnabled: false,
150-
todo: true,
151-
notification: null,
152-
});
147+
// Cold restore delegates the browser pane to restoreBrowserSurfaceTodo, which
148+
// owns routing the persisted TODO into the local activity store (verified
149+
// against the real store in terminal-registry.alert.test.ts).
150+
expect(terminalRegistryMocks.restoreBrowserSurfaceTodo).toHaveBeenCalledWith(
151+
expect.objectContaining({
152+
id: 'pane-web',
153+
surfaceType: 'browser',
154+
alert: expect.objectContaining({ todo: true }),
155+
}),
156+
);
153157
});
154158
});

lib/src/lib/session-restore.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { PlatformAdapter } from './platform/types';
22
import { readPersistedSession, type PersistedDoor } from './session-types';
3-
import { getDefaultShellOpts, primeActivity, restoreTerminal } from './terminal-registry';
3+
import { getDefaultShellOpts, restoreBrowserSurfaceTodo, restoreTerminal } from './terminal-registry';
44

55
export interface RestoredSession {
66
paneIds: string[];
@@ -20,14 +20,7 @@ export function restoreSession(platform: PlatformAdapter): RestoredSession | nul
2020
// them via fromJSON (docs/specs/transport.md). Calling restoreTerminal here
2121
// would mint a stray PTY + xterm for the pane id that never gets mounted.
2222
if (pane.surfaceType === 'browser') {
23-
if (pane.alert?.todo === true) {
24-
primeActivity(pane.id, {
25-
status: 'WATCHING_DISABLED',
26-
watchingEnabled: false,
27-
todo: true,
28-
notification: null,
29-
});
30-
}
23+
restoreBrowserSurfaceTodo(pane);
3124
continue;
3225
}
3326
restoreTerminal(pane.id, {

lib/src/lib/session-save.ts

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { PlatformAdapter } from './platform/types';
2-
import { readPersistedSession, type PersistedDoor, type PersistedPane, type PersistedSession, type PersistedSurfaceType } from './session-types';
2+
import { browserPersistedPane, readPersistedSession, type PersistedDoor, type PersistedPane, type PersistedSession, type PersistedSurfaceType } from './session-types';
33
import { detectResumeCommand } from './resume-patterns';
44
import { getActivity, getLivePersistedAlertState, getTerminalPaneState, isUntouched, resolveTerminalSessionId } from './terminal-registry';
55
import { UNNAMED_PANEL_TITLE } from './terminal-state';
@@ -35,24 +35,10 @@ export async function saveSession(
3535
[...allPanes.values()].map(async (pane) => {
3636
const previousPane = previousPanes.get(pane.id);
3737
if (pane.surfaceType === 'browser') {
38+
// The activity store already holds this surface's TODO; persist it as the
39+
// alert blob (ActivityState is assignable to PersistedAlertState).
3840
const activity = getActivity(pane.id);
39-
return {
40-
id: pane.id,
41-
title: pane.title,
42-
cwd: null,
43-
scrollback: null,
44-
resumeCommand: null,
45-
untouched: false,
46-
alert: activity.todo
47-
? {
48-
status: 'WATCHING_DISABLED' as const,
49-
watchingEnabled: false,
50-
todo: true,
51-
notification: null,
52-
}
53-
: null,
54-
surfaceType: 'browser' as const,
55-
};
41+
return browserPersistedPane(pane, activity.todo ? activity : null);
5642
}
5743

5844
const liveAlert = getLivePersistedAlertState(pane.id);

lib/src/lib/session-types.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,29 @@ export interface PersistedPane {
2828
surfaceType?: PersistedSurfaceType;
2929
}
3030

31+
/**
32+
* Build the persisted record for a browser surface. Browser panes have no PTY,
33+
* so the terminal-only fields (cwd/scrollback/resumeCommand/untouched) are always
34+
* blank; the dockview `layout` blob reconstructs the surface and `alert` carries
35+
* the optional TODO. Single source of truth shared by the renderer save path
36+
* (`session-save.ts`) and the VS Code host refresh (`vscode-ext/session-state.ts`).
37+
*/
38+
export function browserPersistedPane(
39+
pane: { id: string; title: string },
40+
alert: PersistedAlertState | null,
41+
): PersistedPane {
42+
return {
43+
id: pane.id,
44+
title: pane.title,
45+
cwd: null,
46+
scrollback: null,
47+
resumeCommand: null,
48+
untouched: false,
49+
alert,
50+
surfaceType: 'browser',
51+
};
52+
}
53+
3154
export interface PersistedDoor {
3255
id: string;
3356
title: string;

lib/src/lib/terminal-registry.alert.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,10 @@ import {
9797
applyTerminalSemanticEvents,
9898
isPaneOscDriven,
9999
mountElement,
100+
clearLocalSurfaceActivity,
100101
clearSessionAttention,
101102
clearSessionTodo,
103+
restoreBrowserSurfaceTodo,
102104
disposeAllSessions,
103105
disposeSession,
104106
unmountElement,
@@ -488,6 +490,36 @@ describe('terminal-registry alert behavior', () => {
488490
alertToggleSpy.mockRestore();
489491
});
490492

493+
it('Browser surface: restoreBrowserSurfaceTodo replays a persisted TODO; clearLocalSurfaceActivity drops it', () => {
494+
const browserId = 'pane-browser-restore';
495+
496+
// Only browser panes whose persisted alert carries a TODO are replayed.
497+
restoreBrowserSurfaceTodo({ id: browserId, surfaceType: 'browser', alert: { status: 'WATCHING_DISABLED', todo: false } });
498+
expect(getActivity(browserId).todo).toBe(false);
499+
restoreBrowserSurfaceTodo({ id: 'pane-term-x', surfaceType: 'terminal', alert: { status: 'WATCHING_DISABLED', todo: true } });
500+
expect(getActivity('pane-term-x').todo).toBe(false);
501+
502+
restoreBrowserSurfaceTodo({ id: browserId, surfaceType: 'browser', alert: { status: 'WATCHING_DISABLED', todo: true } });
503+
expect(getActivity(browserId).todo).toBe(true);
504+
505+
// Killing/replacing the pane clears the local surface activity entirely.
506+
clearLocalSurfaceActivity(browserId);
507+
expect(getActivity(browserId).todo).toBe(false);
508+
});
509+
510+
it('Browser surface: a reused id prefers a live PTY over a stale local-surface TODO', () => {
511+
const id = 'reused-id';
512+
restoreBrowserSurfaceTodo({ id, surfaceType: 'browser', alert: { status: 'WATCHING_DISABLED', todo: true } });
513+
expect(getActivity(id).todo).toBe(true);
514+
515+
// A terminal later minted with the same id wins: the stale browser TODO is
516+
// ignored rather than leaking onto the PTY's activity.
517+
createSession(id);
518+
expect(getActivity(id).todo).toBe(false);
519+
520+
clearLocalSurfaceActivity(id);
521+
});
522+
491523
it('Story 8: disable alerts clears ring and stops tracking', () => {
492524
const id = 'story-8';
493525
createSession(id);

0 commit comments

Comments
 (0)