Skip to content

Commit 3dd7fbf

Browse files
committed
Fix mobile session pane cleanup
1 parent 5545e65 commit 3dd7fbf

8 files changed

Lines changed: 708 additions & 29 deletions

File tree

packages/web/src/features/agent-panes/actions/use-pane-actions.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
assignSessionToPane,
99
closeDraftPaneById,
1010
closePaneBySessionId,
11+
removePaneBySessionId,
1112
splitPaneByPaneId,
1213
splitPaneBySessionId,
1314
} from "../pane-layout-tree";
@@ -56,6 +57,13 @@ export function usePaneActions(workspaceId: string) {
5657
[applyLayout]
5758
);
5859

60+
const removeSessionPane = useCallback(
61+
(sessionId: string) => {
62+
applyLayout((current) => removePaneBySessionId(current, sessionId));
63+
},
64+
[applyLayout]
65+
);
66+
5967
const assignSession = useCallback(
6068
(paneId: string, sessionId: string) => {
6169
applyLayout((current) => assignSessionToPane(current, paneId, sessionId));
@@ -92,6 +100,7 @@ export function usePaneActions(workspaceId: string) {
92100
assignSession,
93101
closeDraftPane,
94102
closeSessionPane,
103+
removeSessionPane,
95104
replaceWithSession,
96105
splitDraftPane,
97106
splitSessionPane,

packages/web/src/features/agent-panes/actions/use-session-actions.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ describe("useSessionActions", () => {
121121
});
122122

123123
await vi.advanceTimersByTimeAsync(100);
124-
await expect(closePromise).resolves.toBeUndefined();
124+
await expect(closePromise).resolves.toBe(true);
125125
} finally {
126126
if (windowDescriptor) {
127127
Object.defineProperty(globalThis, "window", windowDescriptor);

packages/web/src/features/agent-panes/actions/use-session-actions.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,40 +32,43 @@ export function useSessionActions() {
3232
async (sessionId: string) => {
3333
const session = store.get(sessionByIdAtomFamily(sessionId));
3434
if (!session) {
35-
return;
35+
return false;
3636
}
3737

3838
if (session.state === "ended") {
3939
const removeResult = await dispatch<void>("session.remove", { sessionId });
4040
if (!removeResult.ok) {
4141
console.error("Failed to remove ended session:", removeResult.error?.message);
42+
return false;
4243
}
43-
return;
44+
return true;
4445
}
4546

4647
const stopResult = await dispatch<void>("session.stop", { sessionId });
4748
if (!stopResult.ok && stopResult.error?.code !== "invalid_state") {
4849
console.error("Failed to stop session before removal:", stopResult.error?.message);
49-
return;
50+
return false;
5051
}
5152

5253
const deadline = Date.now() + SESSION_REMOVAL_TIMEOUT_MS;
5354
while (Date.now() < deadline) {
5455
const current = store.get(sessionByIdAtomFamily(sessionId));
5556
if (!current) {
56-
return;
57+
return true;
5758
}
5859
if (current.state === "ended") {
5960
const removeResult = await dispatch<void>("session.remove", { sessionId });
6061
if (!removeResult.ok) {
6162
console.error("Failed to remove ended session:", removeResult.error?.message);
63+
return false;
6264
}
63-
return;
65+
return true;
6466
}
6567
await delay(SESSION_REMOVAL_POLL_INTERVAL_MS);
6668
}
6769

6870
console.error("Timed out waiting for session to end before removal:", sessionId);
71+
return false;
6972
},
7073
[dispatch, store]
7174
);

packages/web/src/features/agent-panes/actions/use-workspace-sessions.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,9 @@ export function useWorkspaceSessions(
9494

9595
const currentLayout = store.get(paneLayoutAtomFamily(workspaceId));
9696
const workspacePaneLayout = normalizePaneLayout(workspace?.uiState.paneLayout);
97-
const legacyPaneLayout = workspacePaneLayout ? null : readLegacyPaneLayout(workspace.id);
97+
const legacyPaneLayout = workspacePaneLayout
98+
? null
99+
: normalizePaneLayout(readLegacyPaneLayout(workspace.id));
98100
const baseLayout =
99101
workspacePaneLayout ?? legacyPaneLayout ?? currentLayout ?? defaultPaneLayout;
100102
const displayableSessionIds = new Set(
@@ -172,13 +174,19 @@ export function useWorkspaceSessions(
172174
};
173175
}
174176

175-
function normalizePaneLayout(layout: Workspace["uiState"]["paneLayout"]): PaneNode | null {
177+
function normalizePaneLayout(
178+
layout: Workspace["uiState"]["paneLayout"] | PaneNode | null | undefined
179+
): PaneNode | null {
176180
if (!layout) {
177181
return null;
178182
}
179183

180184
return {
181-
...layout,
185+
id: layout.id,
186+
type: layout.type,
187+
sessionId: layout.sessionId,
188+
direction: layout.direction,
189+
ratio: "ratio" in layout ? layout.ratio : undefined,
182190
children: layout.children?.map((child) => normalizePaneLayout(child) ?? defaultPaneLayout),
183191
};
184192
}

packages/web/src/features/agent-panes/pane-layout-tree.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
closeDraftPaneById,
77
closePaneBySessionId,
88
createFallbackPaneLayout,
9+
removePaneBySessionId,
910
splitPaneByPaneId,
1011
splitPaneBySessionId,
1112
} from "./pane-layout-tree";
@@ -59,6 +60,25 @@ describe("pane-layout-tree", () => {
5960
});
6061
});
6162

63+
it("removes a session pane and collapses the split when explicitly requested", () => {
64+
const layout: PaneNode = {
65+
id: "root",
66+
type: "split",
67+
direction: "vertical",
68+
ratio: 0.5,
69+
children: [
70+
{ id: "left", type: "leaf", sessionId: "sess_1" },
71+
{ id: "right", type: "leaf", sessionId: "sess_2" },
72+
],
73+
};
74+
75+
expect(removePaneBySessionId(layout, "sess_2")).toEqual({
76+
id: "left",
77+
type: "leaf",
78+
sessionId: "sess_1",
79+
});
80+
});
81+
6282
it("assigns a session to the matching draft pane without touching siblings", () => {
6383
const layout: PaneNode = {
6484
id: "root",

packages/web/src/features/agent-panes/pane-layout-tree.ts

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ export function closeDraftPaneById(node: PaneNode, paneId: string): PaneNode {
186186

187187
/**
188188
* Close a session pane by turning it into a draft leaf while preserving the
189-
* existing split structure. This matches the session-card close behavior:
189+
* existing split structure. This matches the desktop session-card close behavior:
190190
* the session ends, but the workspace layout remains stable so the user can
191191
* immediately launch a replacement session in the same pane.
192192
*/
@@ -221,6 +221,49 @@ function replaceSessionWithDraft(node: PaneNode, sessionId: string): PaneNode {
221221
};
222222
}
223223

224+
export function removePaneBySessionId(node: PaneNode, sessionId: string): PaneNode {
225+
return removeSessionPane(node, sessionId) ?? { id: node.id, type: "leaf" };
226+
}
227+
228+
function removeSessionPane(node: PaneNode, sessionId: string): PaneNode | null {
229+
if (node.type === "leaf") {
230+
if (node.sessionId === sessionId) {
231+
return null;
232+
}
233+
return node;
234+
}
235+
236+
const children = node.children ?? [];
237+
let changed = false;
238+
const nextChildren: PaneNode[] = [];
239+
for (const child of children) {
240+
const nextChild = removeSessionPane(child, sessionId);
241+
if (nextChild !== child) {
242+
changed = true;
243+
}
244+
if (nextChild !== null) {
245+
nextChildren.push(nextChild);
246+
}
247+
}
248+
249+
if (!changed) {
250+
return node;
251+
}
252+
253+
if (nextChildren.length === 1) {
254+
return nextChildren[0]!;
255+
}
256+
257+
if (nextChildren.length === 0) {
258+
return null;
259+
}
260+
261+
return {
262+
...node,
263+
children: nextChildren,
264+
};
265+
}
266+
224267
export function paneLayoutHasSession(node: PaneNode, sessionIds: Set<string>): boolean {
225268
if (node.type === "leaf") {
226269
return node.sessionId ? sessionIds.has(node.sessionId) : false;
@@ -317,14 +360,13 @@ export function createFallbackPaneLayout(sessionIds: string[]): PaneNode {
317360
*/
318361
export function sanitizePaneLayout(node: PaneNode, liveSessionIds: Set<string>): PaneNode {
319362
if (node.type === "leaf") {
320-
// If this leaf references a session that is ended or removed, turn it into a draft
363+
// If this leaf references a session that is ended or removed, turn it into a draft.
321364
if (node.sessionId && !liveSessionIds.has(node.sessionId)) {
322365
return { id: node.id, type: "leaf" };
323366
}
324367
return node;
325368
}
326369

327-
// For splits, recursively sanitize all children and keep the structure intact
328370
const children = node.children ?? [];
329371
let changed = false;
330372
const nextChildren = children.map((child) => {
@@ -335,16 +377,10 @@ export function sanitizePaneLayout(node: PaneNode, liveSessionIds: Set<string>):
335377
return nextChild;
336378
});
337379

338-
// If no children changed, return the same node to preserve reference equality
339380
if (!changed) {
340381
return node;
341382
}
342383

343-
// If all children collapsed to a single leaf, simplify
344-
if (nextChildren.length === 1) {
345-
return nextChildren[0]!;
346-
}
347-
348384
return {
349385
...node,
350386
children: nextChildren,

packages/web/src/features/workspace/actions/use-workspace-screen-model.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { GitStatus, Session } from "@coder-studio/core";
22
import { useAtomValue, useSetAtom, useStore } from "jotai";
3-
import { useCallback, useEffect, useMemo, useState } from "react";
3+
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
44
import { dispatchCommandAtom } from "../../../atoms/connection";
55
import {
66
activeWorkspaceAtom,
@@ -63,6 +63,7 @@ export function useWorkspaceScreenModel() {
6363
const [mobileSheet, setMobileSheet] = useState<MobileWorkspaceSheetKind>(null);
6464
const [mobileFilesRoute, setMobileFilesRoute] = useState<MobileFilesRoute>({ kind: "root" });
6565
const [mobileActiveSessionId, setMobileActiveSessionId] = useState<string | null>(null);
66+
const mobileSelectionVersionRef = useRef(0);
6667

6768
useEffect(() => {
6869
if (!workspace) {
@@ -170,6 +171,7 @@ export function useWorkspaceScreenModel() {
170171
paneActions.appendSession(sessionId, mobileActiveSessionId, "vertical");
171172
}
172173

174+
mobileSelectionVersionRef.current += 1;
173175
setMobileActiveSessionId(sessionId);
174176
},
175177
[mobileActiveSessionId, orderedSessions, paneActions, sessions]
@@ -178,23 +180,38 @@ export function useWorkspaceScreenModel() {
178180
const handleMobileSessionCreated = useCallback(
179181
(sessionId: string) => {
180182
paneActions.appendSession(sessionId, mobileActiveSessionId, "vertical");
183+
mobileSelectionVersionRef.current += 1;
181184
setMobileActiveSessionId(sessionId);
182185
},
183186
[mobileActiveSessionId, paneActions]
184187
);
185188

186189
const closeMobileSession = useCallback(
187190
async (sessionId: string) => {
191+
const wasActive = mobileActiveSessionId === sessionId;
192+
const selectionVersionAtCloseStart = mobileSelectionVersionRef.current;
188193
const remainingSessions = mobileAgentSessions.filter((session) => session.id !== sessionId);
189194
const nextActiveSessionId = remainingSessions[0]?.id ?? null;
190195

191-
paneActions.closeSessionPane(sessionId);
192-
setMobileActiveSessionId((current) =>
193-
current === sessionId ? nextActiveSessionId : current
194-
);
195-
await sessionActions.closeSession(sessionId);
196+
if (wasActive) {
197+
setMobileActiveSessionId(nextActiveSessionId);
198+
}
199+
200+
const closed = await sessionActions.closeSession(sessionId);
201+
if (!closed) {
202+
if (!wasActive || mobileSelectionVersionRef.current !== selectionVersionAtCloseStart) {
203+
return;
204+
}
205+
206+
setMobileActiveSessionId((current) =>
207+
current === nextActiveSessionId ? sessionId : current
208+
);
209+
return;
210+
}
211+
212+
paneActions.removeSessionPane(sessionId);
196213
},
197-
[mobileAgentSessions, paneActions, sessionActions]
214+
[mobileActiveSessionId, mobileAgentSessions, paneActions, sessionActions]
198215
);
199216

200217
const restoreMobileSession = useCallback(
@@ -207,6 +224,7 @@ export function useWorkspaceScreenModel() {
207224
paneActions.appendSession(sessionId, mobileActiveSessionId, "vertical");
208225
}
209226

227+
mobileSelectionVersionRef.current += 1;
210228
setMobileActiveSessionId(sessionId);
211229
},
212230
[mobileActiveSessionId, orderedSessions, paneActions, sessions]

0 commit comments

Comments
 (0)