fix(maestro): never open a live pane for a session that isn't running
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.
This commit is contained in:
+29
-1
@@ -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<string, boolean>();
|
||||
store.subscribe((state) => {
|
||||
for (const id of mv?.openSessions() ?? []) {
|
||||
if (!state.sessions.has(id)) closePane(id);
|
||||
}
|
||||
const open = mv?.openSessions() ?? new Set<string>();
|
||||
const nextRunning = new Map<string, boolean>();
|
||||
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
|
||||
|
||||
@@ -45,7 +45,7 @@ function fakeSidebar(): SidebarHandle {
|
||||
};
|
||||
}
|
||||
|
||||
function makeMv(layout: Layout): MultiView {
|
||||
function makeMv(layout: Layout, overrides: Partial<Parameters<typeof createMultiView>[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<HTMLButtonElement>(".mv-dormant-resume")?.click();
|
||||
expect(onResume).toHaveBeenCalledWith("d1");
|
||||
});
|
||||
});
|
||||
|
||||
describe("multiview pane-drag swap", () => {
|
||||
beforeEach(() => {
|
||||
stubFetch();
|
||||
|
||||
+61
-2
@@ -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<unknown>;
|
||||
/** 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 = `
|
||||
<div class="mv-dormant-glyph" aria-hidden="true">◌</div>
|
||||
<div class="mv-dormant-name">${escapeHtml(sessionName(session))}</div>
|
||||
<div class="mv-dormant-cwd" title="${escapeHtml(session.cwd)}">${escapeHtml(session.cwd)}</div>
|
||||
<div class="mv-dormant-detail">dormant${age ? ` for ${escapeHtml(age)}` : ""} — nothing running behind it</div>
|
||||
<button type="button" class="btn btn-primary mv-dormant-resume">Resume</button>
|
||||
`;
|
||||
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<Node, { kind: "leaf" }>): {
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user