From cc1e7b5f982964138efa090246d92e6eade4bacf Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Fri, 4 Sep 2026 09:19:19 +0100 Subject: [PATCH] fix(maestro): never open a live pane for a session that isn't running MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integration review (#168): the content pane opened a pane for whatever the sidebar highlighted with no status check. After a host reboot every session is dormant, so arrowing down the sidebar opened a pane per session that immediately died with WS 1006 (the daemon answers a bridgeless attach with 404/409, never a completed handshake). Gate every path that fills a pane (preview, open, mobile's single pane) on the session's own status: a session that isn't Running gets the same metadata-and-resume view its sidebar card shows (docs/SIDEBAR.md, docs/PREVIEW.md) instead of an attach attempt. The shell also reacts to onNeverAttached (previous commit) by dropping the dead connection and re-rendering, and to an open session's Running-ness flipping — most commonly a resume completing — so a placeholder and a live pane swap in step with the catalog. --- web/src/main.ts | 30 +++++++++++- web/src/multiview.test.ts | 99 ++++++++++++++++++++++++++++++++++++++- web/src/multiview.ts | 63 ++++++++++++++++++++++++- 3 files changed, 188 insertions(+), 4 deletions(-) diff --git a/web/src/main.ts b/web/src/main.ts index 8bc489f..240f83c 100644 --- a/web/src/main.ts +++ b/web/src/main.ts @@ -19,6 +19,7 @@ import { getDaemonInfo, gitRoot, projects as fetchProjects, + resumeSession, } from "./api.js"; import { buildFastForwardRequest, @@ -172,17 +173,42 @@ const pool = new ConnectionPool({ // pane is never left lying around. onRequestClose: (id) => closePane(id), onClosed: (id) => closePane(id), - shouldReconnect: (id, code) => shouldRetryClose(id, code, store.snapshot().sessions), + // #168 review: a handshake that never opened (a dormant session's `409 + // not_attachable`, most likely) isn't "the session is gone" — drop the dead + // connection and re-render so the leaf gets a fresh attempt if the catalog + // still says Running, or the resume affordance if it doesn't. + onNeverAttached: (id) => { + pool.close(id); + mv?.render(); + }, + shouldReconnect: (id, code, everOpened) => + shouldRetryClose(id, code, store.snapshot().sessions, everOpened), }); // Session death used to reach the UI on two uncoordinated channels — the row // from the events socket, the pane from `ws.onclose` — so either could happen // without the other. The catalog is the authority: a pane for a session that is // no longer listed gets closed here. +// +// A pane can also be showing the dormant placeholder (or, briefly, a dead +// socket) rather than a live attach (#168): when an *open* session's +// Running-ness flips — most commonly a resume completing — the pane and the +// placeholder need to swap without waiting on an unrelated layout change. +let lastOpenRunning = new Map(); store.subscribe((state) => { for (const id of mv?.openSessions() ?? []) { if (!state.sessions.has(id)) closePane(id); } + const open = mv?.openSessions() ?? new Set(); + const nextRunning = new Map(); + let runningChanged = false; + for (const id of open) { + const running = state.sessions.get(id)?.status === "Running"; + nextRunning.set(id, running); + if (lastOpenRunning.get(id) !== running) runningChanged = true; + } + lastOpenRunning = nextRunning; + if (runningChanged) mv?.render(); }); function closePane(id: string): void { @@ -275,6 +301,8 @@ mv = createMultiView({ // catalog (no sessions) to attach. onOpenSessionsChange: updateShellRoute, onLayoutChange: scheduleSave, + sessionFor: (id) => store.snapshot().sessions.get(id), + onResume: (id) => resumeSession(id), }); let contentShowsMv = false; /** Put the multi-view in the content pane, without rendering it. A preview diff --git a/web/src/multiview.test.ts b/web/src/multiview.test.ts index d95a047..c9d44d2 100644 --- a/web/src/multiview.test.ts +++ b/web/src/multiview.test.ts @@ -45,7 +45,7 @@ function fakeSidebar(): SidebarHandle { }; } -function makeMv(layout: Layout): MultiView { +function makeMv(layout: Layout, overrides: Partial[0]> = {}): MultiView { return createMultiView({ pool: fakePool(), layout, @@ -53,6 +53,7 @@ function makeMv(layout: Layout): MultiView { infoFor: () => ({ name: "sess", command: "bash" }), onDisconnect: () => {}, renderWelcome: () => document.createElement("div"), + ...overrides, }); } @@ -87,6 +88,102 @@ describe("multiview empty panes", () => { }); }); +// #168 review: highlighting a dormant session used to open a pane and attempt +// a live attach, which the daemon answers with 404/409 and the socket dies +// with WS 1006. A dormant leaf must never reach the pool at all — it gets the +// resume affordance instead. +describe("multiview dormant gating (#168)", () => { + beforeEach(() => { + stubFetch(); + document.body.replaceChildren(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function dormantSession(id: string) { + return { + id, + cwd: "/home/n/work", + command: "claude", + env: {}, + metadata: { name: "worker" }, + status: "Dormant" as const, + created_at: "2026-08-24T10:00:00Z", + }; + } + + it("never hands a dormant session's leaf to the pool, and shows a resume affordance", async () => { + const layout = emptyLayout(); + const tab = layout.tabs[0]; + tab.focusedLeafId = splitLeaf(tab, "", "d1", "row", true); + const setVisible = vi.fn(); + const pool = { setVisible, setFocus() {} } as unknown as ConnectionPool; + const mv = createMultiView({ + pool, + layout, + sidebar: fakeSidebar(), + infoFor: () => ({ name: "worker", command: "claude" }), + onDisconnect: () => {}, + renderWelcome: () => document.createElement("div"), + sessionFor: (id) => (id === "d1" ? dormantSession("d1") : undefined), + }); + document.body.appendChild(mv.el); + mv.render(); + await flush(); + + expect(setVisible).toHaveBeenCalledWith([], expect.anything()); + const pane = mv.el.querySelector('[data-leaf-id]') as HTMLElement; + expect(pane.querySelector(".mv-pane-dormant")).toBeTruthy(); + expect(pane.textContent).toContain("worker"); + expect(pane.textContent).toContain("/home/n/work"); + expect(pane.querySelector(".mv-dormant-resume")).toBeTruthy(); + }); + + it("still attaches a Running session's leaf normally", async () => { + const layout = emptyLayout(); + const tab = layout.tabs[0]; + tab.focusedLeafId = splitLeaf(tab, "", "r1", "row", true); + const setVisible = vi.fn(); + const pool = { setVisible, setFocus() {} } as unknown as ConnectionPool; + const mv = createMultiView({ + pool, + layout, + sidebar: fakeSidebar(), + infoFor: () => ({ name: "worker", command: "claude" }), + onDisconnect: () => {}, + renderWelcome: () => document.createElement("div"), + sessionFor: (id) => ({ ...dormantSession(id), status: "Running" }), + }); + document.body.appendChild(mv.el); + mv.render(); + await flush(); + + expect(setVisible).toHaveBeenCalledWith( + [expect.objectContaining({ id: "r1" })], + expect.anything(), + ); + }); + + it("clicking resume calls back with the session id", async () => { + const layout = emptyLayout(); + const tab = layout.tabs[0]; + tab.focusedLeafId = splitLeaf(tab, "", "d1", "row", true); + const onResume = vi.fn().mockResolvedValue(undefined); + const mv = makeMv(layout, { + sessionFor: (id) => (id === "d1" ? dormantSession("d1") : undefined), + onResume, + }); + document.body.appendChild(mv.el); + mv.render(); + await flush(); + + mv.el.querySelector(".mv-dormant-resume")?.click(); + expect(onResume).toHaveBeenCalledWith("d1"); + }); +}); + describe("multiview pane-drag swap", () => { beforeEach(() => { stubFetch(); diff --git a/web/src/multiview.ts b/web/src/multiview.ts index aad876e..6f5b70b 100644 --- a/web/src/multiview.ts +++ b/web/src/multiview.ts @@ -6,6 +6,10 @@ import type { SidebarHandle } from "./catalog.js"; import type { ConnectionPool } from "./connections.js"; +import { activityDurationLabel, activityHeldMs } from "./activity.js"; +import { escapeHtml } from "./html.js"; +import { sessionName } from "./types.js"; +import type { Session } from "./types.js"; import { activeTab, addTab, @@ -34,6 +38,14 @@ export interface MultiViewOptions { onDisconnect: (sessionId: string) => void; /** Welcome content for an empty tab. */ renderWelcome: () => HTMLElement; + /** The session behind a leaf, when known — used to gate a live attach on + * `status === "Running"` (#168 review) and to fill in the dormant pane's + * resume affordance. A leaf whose session this returns `undefined` for + * (unknown to the caller) attaches as before, so callers that don't wire + * this up see no change in behaviour. */ + sessionFor?: (sessionId: string) => Session | undefined; + /** Resume a dormant session (#166/#168), from its pane's own affordance. */ + onResume?: (sessionId: string) => Promise; /** Called when a spawn fills a pane with a new session, so the shell can * re-evaluate its route (sidebar vs attach layout). */ onOpenSessionsChange?: () => void; @@ -262,6 +274,44 @@ export function createMultiView(opts: MultiViewOptions): MultiView { persist(); } + // ---- dormant panes (#168 review) ---- + + /** The session behind `sessionId` when it is known and not `Running` — + * what gates a leaf away from a live attach and towards the resume + * affordance instead. */ + function dormantSessionFor(sessionId: string): Session | undefined { + const s = opts.sessionFor?.(sessionId); + return s && s.status !== "Running" ? s : undefined; + } + + /** docs/PREVIEW.md / docs/SIDEBAR.md: what the content pane can honestly + * show for a session with no pane behind it — its metadata and a resume + * affordance, never a live terminal that would just die with WS 1006. */ + function renderDormantPane(host: HTMLElement, session: Session): void { + const heldMs = activityHeldMs(session.last_activity_at ?? session.created_at, Date.now()); + const age = heldMs !== null ? activityDurationLabel(heldMs) : null; + const wrap = document.createElement("div"); + wrap.className = "mv-pane-dormant"; + wrap.innerHTML = ` + +
${escapeHtml(sessionName(session))}
+
${escapeHtml(session.cwd)}
+
dormant${age ? ` for ${escapeHtml(age)}` : ""} — nothing running behind it
+ + `; + const btn = wrap.querySelector(".mv-dormant-resume") as HTMLButtonElement; + btn.addEventListener("click", () => { + btn.disabled = true; + btn.textContent = "Resuming…"; + opts.onResume?.(session.id)?.catch((e: Error) => { + btn.disabled = false; + btn.textContent = "Resume"; + alert(`Failed to resume: ${e.message}`); + }); + }); + host.replaceChildren(wrap); + } + // ---- pane + split DOM ---- function buildPane(leaf: Extract): { @@ -378,7 +428,12 @@ export function createMultiView(opts: MultiViewOptions): MultiView { if (node.kind === "leaf") { const pane = buildPane(node); if (node.sessionId) { - hosts.push({ id: node.sessionId, host: pane.host, info: infoFor(node.sessionId) }); + const dormant = dormantSessionFor(node.sessionId); + if (dormant) { + renderDormantPane(pane.host, dormant); + } else { + hosts.push({ id: node.sessionId, host: pane.host, info: infoFor(node.sessionId) }); + } } return pane.el; } @@ -616,9 +671,13 @@ export function createMultiView(opts: MultiViewOptions): MultiView { wrap.appendChild(pane.el); canvas.appendChild(wrap); + const dormant = focused.sessionId ? dormantSessionFor(focused.sessionId) : undefined; + if (dormant) renderDormantPane(pane.host, dormant); // Only the focused leaf's session is live on mobile; the rest park. pool.setVisible( - focused.sessionId ? [{ id: focused.sessionId, pane: pane.host, info: infoFor(focused.sessionId) }] : [], + focused.sessionId && !dormant + ? [{ id: focused.sessionId, pane: pane.host, info: infoFor(focused.sessionId) }] + : [], { focus: !preview && !quietRender }, ); if (!preview) pool.setFocus(focused.sessionId ?? null);