From 4892fa7b4981f8d49fce41f8da2703ab0c2152d9 Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Thu, 30 Jul 2026 10:40:10 +0200 Subject: [PATCH 1/2] feat(sessions): surface tmux sessions the server does not track MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `web_*` tmux session that outlives the process which created it was unreachable from the UI, permanently. Nothing enumerated tmux — src/session/tmux.ts had hasSession but no list — so recovery only worked if a client still had the id in localStorage. list() walks the in-memory table, and so does reapIdle, so such a session could not be listed, previewed, joined or killed, and never aged out. On this host that was 69 tmux sessions with 5 clients: 64 unreachable, the oldest from 26 Jun, one of them still running a Claude Code session with 7 agents. This is the "catalogue" approach: make them visible and actionable WITHOUT adopting them. Nothing is attached, because attaching makes tmux resize the window to the new client's dimensions and SIGWINCH whatever is running inside — doing that to dozens of live TUIs at startup is exactly the outcome to avoid. Adoption stays an explicit user action: opening an orphan re-attaches by id through the existing re-attach path, and it becomes an ordinary live session. src/session/tmux.ts listSessions() + parseSessionList() (pure, so the filter rules are unit-testable) and capturePane(), which reads a session's current screen via `capture-pane -p -e` — no client, no resize, colour preserved. src/session/manager.ts listOrphans/captureOrphan/killOrphan/killOrphansIdleSince src/server.ts GET /orphan-sessions, GET /orphan-sessions/:id/preview, DELETE /orphan-sessions/:id, DELETE /orphan-sessions ?idleDays=N public/ a "Recoverable" section under the home grid, reusing the existing card/thumbnail machinery via orphanAsLive() Guards, all enforced in the manager so no route can forget one: - the tmux name must be `web_` + a UUID v4 (SESSION_ID_RE, the same gate the attach protocol applies). This is what stops a hostile or malformed name from reaching `tmux -t` — a literal `-t`, or path traversal — and it means a tmux session the user created for their own work is never enumerated, never previewed and never offered for deletion. - an id the table already owns is refused everywhere: killing it via tmux would end the shell behind a live PTY's back and leave a zombie table entry. Those go through killById. - every function is inert when cfg.useTmux is off, so the non-tmux deployment is byte-for-byte unchanged. - bulk cleanup skips ATTACHED sessions and requires idleDays >= 1: there is no "delete everything" form of the route. "This server does not track it" is not evidence that it is abandoned — a plain `tmux attach` and a second server process both report as attached. - reads carry no Origin guard (same threat model as /live-sessions); both DELETEs do. Preview gets its own rate-limit bucket since each call spawns a process. Two things the live run exposed and this fixes: previews were loading sequentially (69 tmux spawns per 5 s refresh) — now once per card, fire-and-forget — and the grid is capped at 24 cards because each thumbnail is an xterm instance. The remainder is stated in the UI with the tmux command to reach it, never silently truncated. Separate wire type from LiveSessionInfo on purpose: an orphan supports none of the live-session operations, and the Android/iOS clients decode /live-sessions, so a variant shape there would break them. Tests: 2203 unit (+42: parse/filter rules, all four manager guards, tmux-off inertness) and 8 integration against real tmux — create a throwaway session, list it, capture it while asserting it stays unattached, reject non-UUID ids, reject a foreign Origin, kill it, 404 the second time. The integration file never issues the bulk DELETE: it runs against the host's real tmux server and would end the developer's own sessions. Verified live against all 69 with none harmed. --- public/launcher.ts | 134 ++++++++++- public/preview-grid.ts | 88 ++++++- public/style.css | 54 +++++ src/server.ts | 66 ++++++ src/session/manager.ts | 75 +++++- src/session/tmux.ts | 123 ++++++++++ src/types.ts | 45 ++++ test/integration/orphan-sessions.test.ts | 208 +++++++++++++++++ test/manager-orphans.test.ts | 280 +++++++++++++++++++++++ test/tmux.test.ts | 95 +++++++- 10 files changed, 1162 insertions(+), 6 deletions(-) create mode 100644 test/integration/orphan-sessions.test.ts create mode 100644 test/manager-orphans.test.ts diff --git a/public/launcher.ts b/public/launcher.ts index b778349..38f8325 100644 --- a/public/launcher.ts +++ b/public/launcher.ts @@ -9,7 +9,7 @@ * manage page via public/preview-grid.ts (DRY). */ -import type { LiveSessionInfo } from '../src/types.js' +import type { LiveSessionInfo, OrphanSessionInfo } from '../src/types.js' import { el, relTime, @@ -17,6 +17,12 @@ import { updatePreviewCard, loadPreviewInto, fetchLiveSessions, + fetchOrphanSessions, + fetchOrphanPreview, + renderPreview, + orphanAsLive, + killOrphanReq, + cleanupOrphansReq, type PreviewCard, } from './preview-grid.js' @@ -48,6 +54,24 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher const grid = el('div', 'mg-grid') root.append(grid) + // A: tmux sessions that outlived the server process. Its own section, below the + // live grid, because these are recoveries rather than things you are working on + // — and it stays hidden entirely when there are none (the common case, and the + // only case when tmux is off). + const orphanSection = el('div', 'launcher-orphans') + orphanSection.style.display = 'none' + const orphanHead = el('div', 'launcher-orphans-head') + const orphanTitle = el('div', 'launcher-orphans-title', 'Recoverable') + const orphanSub = el('div', 'launcher-orphans-sub', '') + const cleanupBtn = el('button', 'launcher-cleanup', 'Clean up idle 7d+') + cleanupBtn.title = 'Kill unattached recoverable sessions with no tmux activity for 7 days' + orphanHead.append(orphanTitle, orphanSub, cleanupBtn) + const orphanGrid = el('div', 'mg-grid') + const orphanMore = el('div', 'launcher-orphans-more', '') + orphanMore.style.display = 'none' + orphanSection.append(orphanHead, orphanGrid, orphanMore) + root.append(orphanSection) + // "New session" is a tile that leads the grid (matches the session cards), // instead of a heavy header button. Re-prepended on every refresh. const newTile = el('button', 'mg-new-card') @@ -57,8 +81,14 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher newTile.addEventListener('click', () => hooks.onNew()) const cards = new Map() + const orphanCards = new Map() let timer: ReturnType | null = null + const CLEANUP_IDLE_DAYS = 7 + /** Thumbnails are xterm instances; a host with dozens of stale tmux sessions + * would otherwise mount dozens of terminals on the home screen. */ + const MAX_ORPHAN_CARDS = 24 + /** Kill a session (DELETE /live-sessions/:id) then refresh the grid. */ async function killOne(id: string): Promise { await fetch(`/live-sessions/${encodeURIComponent(id)}`, { method: 'DELETE' }).catch(() => {}) @@ -106,8 +136,106 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher } } + await refreshOrphans() } + /** Kill one orphan, then refresh. */ + async function killOrphan(id: string): Promise { + await killOrphanReq(id) + void refresh() + } + + /** Opening an orphan revives it: attaching with its id re-adopts the tmux + * session in place, after which it appears in the live grid above. */ + function makeOrphanCard(o: OrphanSessionInfo): PreviewCard { + const kill = el('button', 'mg-kill', 'Kill ✕') + kill.title = 'End this session’s shell' + kill.addEventListener('click', () => void killOrphan(o.id)) + return makePreviewCard(orphanAsLive(o), { + onOpen: (id) => hooks.onOpen(id), + extraActions: () => [kill], + }) + } + + async function refreshOrphans(): Promise { + const orphans = await fetchOrphanSessions() + + if (orphans.length === 0) { + orphanSection.style.display = 'none' + for (const card of orphanCards.values()) { + card.term.dispose() + card.el.remove() + } + orphanCards.clear() + return + } + + orphanSection.style.display = '' + const idle = orphans.filter((o) => !o.attached).length + orphanSub.textContent = + `${orphans.length} tmux session${orphans.length === 1 ? '' : 's'} not open here — ` + + `open one to take it over${idle > 0 ? `, ${idle} unattended` : ''}` + + // Server-sorted most-recently-active first, so the cap keeps the ones you are + // plausibly looking for. The tail is what the cleanup button is for. + const shown = orphans.slice(0, MAX_ORPHAN_CARDS) + const hidden = orphans.length - shown.length + + const seen = new Set() + for (const o of shown) { + seen.add(o.id) + let card = orphanCards.get(o.id) + const isNew = card === undefined + if (card === undefined) { + card = makeOrphanCard(o) + orphanCards.set(o.id, card) + orphanGrid.append(card.el) + } + updatePreviewCard(card, orphanAsLive(o)) + card.meta.textContent = + `${o.cols}×${o.rows} · active ${relTime(o.lastActivityAt)} ago · ` + + `${relTime(o.createdAt)} old` + // ONCE per card, not on every 5 s tick: each preview spawns a tmux + // capture-pane, and re-capturing every card every tick would mean dozens of + // processes a minute for screens that are, by definition, not being driven + // from here. Fire-and-forget so a slow capture never stalls the next card. + if (isNew) { + const target = card + void fetchOrphanPreview(o.id).then((p) => { + if (p) renderPreview(target, p, THUMB_W, THUMB_MAX_H) + }) + } + } + for (const [id, card] of orphanCards) { + if (!seen.has(id)) { + card.term.dispose() + card.el.remove() + orphanCards.delete(id) + } + } + + // Never truncate silently. + orphanMore.textContent = + hidden > 0 + ? `${hidden} older session${hidden === 1 ? '' : 's'} not shown — clean up, or attach by id from a terminal with: tmux attach -t web_` + : '' + orphanMore.style.display = hidden > 0 ? '' : 'none' + } + + cleanupBtn.addEventListener('click', () => { + const n = orphanCards.size + if ( + !window.confirm( + `Kill unattached recoverable sessions with no activity for ${CLEANUP_IDLE_DAYS} days?\n\n` + + `This ends their shells. Sessions someone is attached to are never touched. ` + + `(${n} recoverable session${n === 1 ? '' : 's'} listed.)`, + ) + ) { + return + } + void cleanupOrphansReq(CLEANUP_IDLE_DAYS).then(() => refresh()) + }) + return { setVisible(v: boolean): void { root.style.display = v ? 'block' : 'none' @@ -126,6 +254,10 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher for (const card of cards.values()) card.term.dispose() cards.clear() grid.replaceChildren() + for (const card of orphanCards.values()) card.term.dispose() + orphanCards.clear() + orphanGrid.replaceChildren() + orphanSection.style.display = 'none' } }, refresh(): void { diff --git a/public/preview-grid.ts b/public/preview-grid.ts index 48419c5..2947d6f 100644 --- a/public/preview-grid.ts +++ b/public/preview-grid.ts @@ -9,7 +9,12 @@ */ import { Terminal } from '@xterm/xterm' -import type { LiveSessionInfo, StatusTelemetry, ClaudeStatus } from '../src/types.js' +import type { + LiveSessionInfo, + OrphanSessionInfo, + StatusTelemetry, + ClaudeStatus, +} from '../src/types.js' /** Shape of GET /live-sessions/:id/preview. */ export interface SessionPreview { @@ -152,9 +157,12 @@ export async function fetchPreview(id: string): Promise { } } -/** Render a preview into a card's xterm, resizing + scaling to fit. */ +/** Render a preview into a card's xterm, resizing + scaling to fit. + * cols/rows of 0 mean "size unknown, keep what the card already has" — the + * orphan preview omits them because its dimensions came with the list, and + * resizing to Math.max(2, 0) would collapse the thumbnail to 2×2. */ export function renderPreview(card: PreviewCard, p: SessionPreview, thumbW: number, thumbMaxH: number): void { - if (card.term.cols !== Math.max(2, p.cols) || card.term.rows !== Math.max(2, p.rows)) { + if (p.cols > 0 && p.rows > 0 && (card.term.cols !== p.cols || card.term.rows !== p.rows)) { card.term.resize(Math.max(2, p.cols), Math.max(2, p.rows)) } card.term.reset() @@ -183,6 +191,80 @@ export async function fetchLiveSessions(): Promise { } } +/* ── A: orphan tmux sessions ───────────────────────────────────────────────── + Sessions that outlived the server process which created them. They reuse the + card machinery above through orphanAsLive, so there is one thumbnail + implementation rather than two that drift. */ + +/** Fetch untracked tmux sessions, returning [] on failure. */ +export async function fetchOrphanSessions(): Promise { + try { + const res = await fetch('/orphan-sessions') + const data: unknown = await res.json() + return Array.isArray(data) ? (data as OrphanSessionInfo[]) : [] + } catch { + return [] + } +} + +/** Fetch an orphan's screen (tmux capture-pane). null on any failure. */ +export async function fetchOrphanPreview(id: string): Promise { + try { + const res = await fetch(`/orphan-sessions/${encodeURIComponent(id)}/preview`) + if (!res.ok) return null + return (await res.json()) as SessionPreview + } catch { + return null + } +} + +/** + * Present an orphan as a LiveSessionInfo so makePreviewCard can render it. + * + * `status` is 'unknown', never 'idle': status comes from Claude Code's hooks into + * a session this server is streaming, and for an orphan we have not looked — so + * claiming 'idle' would assert something we do not know. `cwd` is null for the + * same reason. `clientCount` mirrors tmux's client count, so a session someone is + * attached to from a real terminal reads as watched rather than abandoned. + */ +export function orphanAsLive(o: OrphanSessionInfo): LiveSessionInfo { + return { + id: o.id, + createdAt: o.createdAt, + clientCount: o.attached ? 1 : 0, + status: 'unknown', + exited: false, + cwd: null, + cols: o.cols, + rows: o.rows, + lastOutputAt: o.lastActivityAt, + } +} + +/** Kill one orphan. Resolves true on 204. */ +export async function killOrphanReq(id: string): Promise { + try { + const res = await fetch(`/orphan-sessions/${encodeURIComponent(id)}`, { method: 'DELETE' }) + return res.status === 204 + } catch { + return false + } +} + +/** Bulk-kill unattached orphans idle longer than `days`. Returns ids killed. */ +export async function cleanupOrphansReq(days: number): Promise { + try { + const res = await fetch(`/orphan-sessions?idleDays=${encodeURIComponent(String(days))}`, { + method: 'DELETE', + }) + if (!res.ok) return [] + const body = (await res.json()) as { ids?: unknown } + return Array.isArray(body.ids) ? (body.ids as string[]) : [] + } catch { + return [] + } +} + /** * Render a telemetry gauge into `container` (clears first). * diff --git a/public/style.css b/public/style.css index 7db0aa1..7b3ba29 100644 --- a/public/style.css +++ b/public/style.css @@ -1192,6 +1192,60 @@ body { color: var(--text-dim); font-size: 13px; } +/* A: recoverable (orphan) tmux sessions — a secondary section under the live grid, + separated by a rule so it never competes with what you are actually working on. + Hidden entirely when the list is empty. */ +.launcher-orphans { + margin-top: 30px; + padding-top: 20px; + border-top: 1px solid var(--border); +} +.launcher-orphans-head { + display: flex; + align-items: center; + flex-wrap: wrap; + gap: 10px 16px; + margin-bottom: 16px; +} +.launcher-orphans-title { + font-size: 11px; + letter-spacing: 0.15em; + text-transform: uppercase; + color: var(--text-faint); +} +.launcher-orphans-sub { + flex: 1 1 auto; + color: var(--text-dim); + font-size: 13px; +} +/* Destructive, so it is the ghost role rather than a filled button — the design + reserves the accent fill for the action you actually came to perform. */ +.launcher-cleanup { + font: inherit; + font-size: 12.5px; + cursor: pointer; + padding: 7px 15px; + border-radius: 7px; + border: 1px solid var(--border-strong); + background: transparent; + color: var(--text-dim); + flex: none; +} +.launcher-cleanup:hover { + border-color: var(--red); + color: var(--red); +} +.launcher-cleanup:focus-visible { + outline: 2px solid var(--text); + outline-offset: 2px; +} +/* The grid is capped, so say so rather than let the tail vanish silently. */ +.launcher-orphans-more { + margin-top: 14px; + font-family: var(--mono-font); + font-size: 11.5px; + color: var(--text-faint); +} .launcher-actions { display: flex; gap: 8px; diff --git a/src/server.ts b/src/server.ts index 7a934c2..c1294ea 100644 --- a/src/server.ts +++ b/src/server.ts @@ -95,6 +95,9 @@ const RATE_LIMIT_WINDOW_MS = 60_000 const DECISION_RATE_MAX = 10 // POST /hook/decision ≤ 10/min/IP const SUBSCRIBE_RATE_MAX = 5 // POST/DELETE /push/subscribe ≤ 5/min/IP const QUEUE_RATE_MAX = 20 // W2: POST/DELETE /live-sessions/:id/queue ≤ 20/min/IP +// A: GET /orphan-sessions/:id/preview ≤ 240/min/IP — one tmux capture-pane each, +// and a full cleanup grid refreshes every visible card at once. +const ORPHAN_PREVIEW_RATE_MAX = 240 const GIT_WRITE_RATE_MAX = 30 // W4: POST /projects/git/{stage,commit} ≤ 30/min/IP const GIT_PUSH_RATE_MAX = 6 // W4: POST /projects/git/push ≤ 6/min/IP (network-bound) const AUTH_RATE_MAX = 10 // w5-access-token: POST /auth + GET /?token= ≤ 10/min/IP (brute-force guard) @@ -251,6 +254,10 @@ export function startServer(cfg: Config): { close(): Promise } { const decisionLimiter = createRateLimiter(DECISION_RATE_MAX, RATE_LIMIT_WINDOW_MS) const subscribeLimiter = createRateLimiter(SUBSCRIBE_RATE_MAX, RATE_LIMIT_WINDOW_MS) const queueLimiter = createRateLimiter(QUEUE_RATE_MAX, RATE_LIMIT_WINDOW_MS) // W2 + // A: /orphan-sessions/:id/preview spawns one `tmux capture-pane` per call, and a + // cleanup grid can ask for many at once — its own bucket, sized for a full grid + // refresh, so it never eats the queue or git budgets. + const orphanLimiter = createRateLimiter(ORPHAN_PREVIEW_RATE_MAX, RATE_LIMIT_WINDOW_MS) const gitWriteLimiter = createRateLimiter(GIT_WRITE_RATE_MAX, RATE_LIMIT_WINDOW_MS) // W4 stage/commit const gitPushLimiter = createRateLimiter(GIT_PUSH_RATE_MAX, RATE_LIMIT_WINDOW_MS) // W4 push // w6/G2 fetch gets its OWN bucket: it is the panel's refresh button, so a burst @@ -582,6 +589,65 @@ export function startServer(cfg: Config): { close(): Promise } { // Preview (v0.4 manage page) — the tail of a session's scrollback so the grid // can render a live read-only thumbnail of its current screen. No client/attach. + // ── Orphan tmux sessions (A) — `web_*` tmux sessions this server does NOT + // track, because they outlived the process that created them. Read-only routes + // carry no Origin guard (same threat model as /live-sessions); the DELETEs do, + // and every route re-validates the id via manager, which requires UUID v4 and + // refuses anything the table already owns. + // + // Deliberately a separate path from /live-sessions rather than an extra field on + // it: an orphan supports none of the live-session operations (no replay, status, + // telemetry or inject queue), and the Android/iOS clients decode + // /live-sessions — adding a variant shape there would break them. + app.get('/orphan-sessions', (_req, res) => { + res.json(manager.listOrphans()) + }) + + // Each call spawns a short-lived `tmux capture-pane`, so it gets the same per-IP + // bucket discipline as the other process-spawning routes. + app.get('/orphan-sessions/:id/preview', (req, res) => { + if (!orphanLimiter(req.socket.remoteAddress ?? '', Date.now())) { + res.status(429).end() + return + } + const id = req.params.id + if (!SESSION_ID_RE.test(id)) { + res.status(400).json({ error: 'invalid session id' }) + return + } + const data = manager.captureOrphan(id) + if (data === null) { + res.status(404).end() + return + } + // Same shape as the live preview so the frontend renders both with one path. + res.json({ id, cols: 0, rows: 0, data }) + }) + + // Bulk cleanup — kill UNATTACHED orphans idle longer than `idleDays`. Registered + // BEFORE /orphan-sessions/:id so it is not captured as an :id. `idleDays` is + // required and must be >= 1: there is no "delete everything" form of this route. + app.delete('/orphan-sessions', (req, res) => { + if (!requireAllowedOrigin(req, res)) return + const days = Number(req.query['idleDays']) + if (!Number.isFinite(days) || days < 1) { + res.status(400).json({ error: 'idleDays must be a number >= 1' }) + return + } + const killed = manager.killOrphansIdleSince(Date.now() - days * 86_400_000) + res.json({ killed: killed.length, ids: killed }) + }) + + app.delete('/orphan-sessions/:id', (req, res) => { + if (!requireAllowedOrigin(req, res)) return + const id = req.params.id + if (!SESSION_ID_RE.test(id)) { + res.status(400).json({ error: 'invalid session id' }) + return + } + res.status(manager.killOrphan(id) ? 204 : 404).end() + }) + app.get('/live-sessions/:id/preview', (req, res) => { const session = manager.get(req.params.id) if (session === undefined) { diff --git a/src/session/manager.ts b/src/session/manager.ts index cf44005..880a6d4 100644 --- a/src/session/manager.ts +++ b/src/session/manager.ts @@ -32,6 +32,7 @@ import type { Dims, EnqueueResult, LiveSessionInfo, + OrphanSessionInfo, NotifyService, PermissionGate, ServerMessage, @@ -44,7 +45,8 @@ import { WS_OPEN } from '../types.js'; import { serialize } from '../protocol.js'; import { createSession, attachWs, broadcast, kill, writeInput } from './session.js'; import { appendEvent, makeTimelineEvent } from './timeline.js'; -import { hasSession, tmuxName } from './tmux.js'; +import { hasSession, tmuxName, killSession, listSessions, capturePane } from './tmux.js'; +import { SESSION_ID_RE } from '../protocol.js'; /** * Send a serialised ServerMessage to `ws` only when it is OPEN (M5). @@ -415,10 +417,81 @@ export function createSessionManager( sessions = new Map(); } + /* ── A: tmux sessions this server does not track ─────────────────────────── + A `web_*` tmux session that outlived the process which made it is invisible + to everything above: list() only walks our own table, and so does reapIdle. + These four functions are the whole of the read/act surface for them. They + never adopt: no PTY is spawned and nothing is attached, because attaching + would resize the tmux window to the new client and SIGWINCH whatever is + running inside it. Joining (which DOES adopt, via handleAttach's re-attach + path) stays an explicit user action. + + One guard, applied identically everywhere: the id must satisfy + SESSION_ID_RE and must NOT be in our table. The first is what stops a + hostile or malformed name reaching `tmux -t`; the second keeps a live + session's shell from being killed behind its own PTY's back. */ + + /** True when `id` is a real, untracked tmux session of ours that we may act on. */ + function isActionableOrphan(id: string): boolean { + if (!cfg.useTmux) return false; + if (!SESSION_ID_RE.test(id)) return false; + if (sessions.has(id)) return false; // tracked ⇒ use get()/killById instead + return hasSession(tmuxName(id)); + } + + function listOrphans(): OrphanSessionInfo[] { + if (!cfg.useTmux) return []; + return listSessions() + .filter((t) => !sessions.has(t.id)) + .map((t) => ({ + id: t.id, + createdAt: t.createdAtMs, + lastActivityAt: t.lastActivityAtMs, + attached: t.attached, + cols: t.cols, + rows: t.rows, + })); + } + + /** The orphan's current screen, or null. Read-only: never attaches. */ + function captureOrphan(id: string): string | null { + if (!isActionableOrphan(id)) return null; + return capturePane(tmuxName(id)); + } + + /** End an orphan's shell. False when it is not ours, not there, or is tracked. */ + function killOrphan(id: string): boolean { + if (!isActionableOrphan(id)) return false; + killSession(tmuxName(id)); + return true; + } + + /** + * Bulk cleanup: kill every UNATTACHED orphan whose last tmux activity predates + * `cutoffMs`. Returns the ids killed. + * + * Attached sessions are skipped on purpose — tmux reporting a client means + * someone is in there from a real terminal or another server process, and + * "this server does not track it" is not evidence that it is abandoned. + */ + function killOrphansIdleSince(cutoffMs: number): string[] { + const killed: string[] = []; + for (const o of listOrphans()) { + if (o.attached) continue; + if (o.lastActivityAt >= cutoffMs) continue; + if (killOrphan(o.id)) killed.push(o.id); + } + return killed; + } + return { handleAttach, get, list, + listOrphans, + captureOrphan, + killOrphan, + killOrphansIdleSince, killById, handleHookEvent, handleStatusLine, diff --git a/src/session/tmux.ts b/src/session/tmux.ts index 451120f..b64a271 100644 --- a/src/session/tmux.ts +++ b/src/session/tmux.ts @@ -10,6 +10,38 @@ */ import { execFileSync } from 'node:child_process' +import { SESSION_ID_RE } from '../protocol.js' + +/** Upper bound on one `capture-pane` payload. A pane is one screen, so this is + * generous; it exists so a wedged tmux can never hand us unbounded output. */ +const CAPTURE_MAX_BUFFER = 512 * 1024 + +/** Tab-separated, machine-readable. tmux reports the two clocks in unix SECONDS. */ +const LIST_FORMAT = [ + '#{session_name}', + '#{session_created}', + '#{session_activity}', + '#{session_attached}', + '#{window_width}', + '#{window_height}', +].join('\t') + +/** + * One `web_*` tmux session as tmux itself describes it (A). + * + * `attached` is tmux's own client count, NOT "this server is streaming it": a + * session can be attached from a plain terminal, or by a second web-terminal + * process. So "we do not track it" and "nobody is using it" are different + * questions, and the UI must not conflate them. + */ +export interface TmuxSessionSummary { + readonly id: string + readonly createdAtMs: number + readonly lastActivityAtMs: number + readonly attached: boolean + readonly cols: number + readonly rows: number +} /** Is the tmux binary available on PATH? */ export function tmuxAvailable(): boolean { @@ -44,3 +76,94 @@ export function killSession(name: string): void { // already gone — fine } } + +/** + * Parse `tmux list-sessions -F LIST_FORMAT` output. Pure, so the parsing rules are + * testable without a tmux binary. + * + * Two filters, both load-bearing: + * - the name must start with `web_`, so we never enumerate (and therefore never + * offer to kill) a tmux session the user created for their own work; + * - the remainder must be a UUID v4 per SESSION_ID_RE — the same gate the attach + * protocol applies (M7). Anything else cannot have been created by this app, and + * refusing it here is also what keeps a hostile name from reaching `tmux -t` + * (e.g. a literal `-t`, or a name full of path traversal). + */ +export function parseSessionList(stdout: string): TmuxSessionSummary[] { + const rows: TmuxSessionSummary[] = [] + for (const line of stdout.split('\n')) { + if (line === '') continue + const parts = line.split('\t') + if (parts.length < 6) continue + + const [name, created, activity, attached, width, height] = parts as [ + string, + string, + string, + string, + string, + string, + ] + if (!name.startsWith('web_')) continue + const id = name.slice('web_'.length) + if (!SESSION_ID_RE.test(id)) continue + + const createdSec = Number(created) + const activitySec = Number(activity) + const cols = Number(width) + const rows_ = Number(height) + if ( + !Number.isFinite(createdSec) || + !Number.isFinite(activitySec) || + !Number.isFinite(cols) || + !Number.isFinite(rows_) + ) { + continue + } + + rows.push({ + id, + createdAtMs: createdSec * 1000, + lastActivityAtMs: activitySec * 1000, + attached: attached !== '0' && attached !== '', + cols, + rows: rows_, + }) + } + return rows.sort((a, b) => b.lastActivityAtMs - a.lastActivityAtMs) +} + +/** Every `web_*` tmux session on the host, most recently active first. Never + * throws; [] when tmux is missing or no tmux server is running. */ +export function listSessions(): TmuxSessionSummary[] { + try { + const out = execFileSync('tmux', ['list-sessions', '-F', LIST_FORMAT], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + maxBuffer: CAPTURE_MAX_BUFFER, + }) + return parseSessionList(out) + } catch { + return [] + } +} + +/** + * The session's CURRENT screen, without attaching. Returns null if it is gone. + * + * `-p` prints to stdout and `-e` keeps the escape sequences, so the bytes can go + * straight into a read-only xterm and keep their colour. Deliberately NOT + * `attach-session`: attaching would make tmux resize the window to the new + * client's dimensions and SIGWINCH whatever is running inside it. + */ +export function capturePane(name: string): string | null { + try { + return execFileSync('tmux', ['capture-pane', '-p', '-e', '-t', name], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + maxBuffer: CAPTURE_MAX_BUFFER, + }) + } catch { + return null + } +} diff --git a/src/types.ts b/src/types.ts index c7e5f96..f38fffd 100644 --- a/src/types.ts +++ b/src/types.ts @@ -318,6 +318,37 @@ export interface LiveSessionInfo { readonly queueLength?: number; } +/** + * A tmux session on this host that the running server does NOT have in its table + * (A). It outlived the process that created it — a server restart, a different + * server process, or the desktop app quitting. + * + * It has no PTY, no ring buffer and no live stream here, so it is deliberately a + * DIFFERENT type from LiveSessionInfo rather than a flag on it: everything the + * manage grid does with a live session (replay, status, telemetry, inject queue) + * is unavailable, and a shared shape would invite callers to assume otherwise. + * What it does support: being listed, previewed via `tmux capture-pane` (no + * attach), killed, and joined — attaching with this id revives it in place + * through the existing re-attach path, at which point it becomes a real + * LiveSessionInfo. + * + * Before this existed such a session was unreachable from the UI forever: nothing + * enumerated tmux, so an id no client still remembered could not be listed, + * previewed, joined or killed, and the idle reaper never saw it either. + */ +export interface OrphanSessionInfo { + id: string; + createdAt: number; // epoch ms + /** tmux's OWN activity clock (epoch ms) — survives server restarts, unlike the + * in-memory lastOutputAt, so it is the only usable age for a cleanup policy. */ + lastActivityAt: number; + /** tmux client count > 0: attached from a real terminal or another server + * process. "Untracked here" does NOT mean "abandoned". */ + attached: boolean; + cols: number; + rows: number; +} + /* ───────────────── W5 fan-out board (parallel agent lanes) ───────────────── */ /** A group of running sessions sharing a repo/worktree-root (fan-out discovery). @@ -442,6 +473,20 @@ export interface SessionManager { get(id: string): Session | undefined; /** Live sessions for multi-device discovery (newest first). */ list(): LiveSessionInfo[]; + /* ── A: tmux sessions NOT in this server's table ────────────────────────── + All four are no-ops (empty / null / false) when cfg.useTmux is off, and all + reject an id that is not a UUID v4 or that the table already owns — a + tracked session must go through get()/killById so its PTY and table entry + stay in step. None of them adopt: nothing is attached, so a running TUI is + never resized. */ + /** Untracked `web_*` tmux sessions, most recently active first. */ + listOrphans(): OrphanSessionInfo[]; + /** An orphan's current screen via `tmux capture-pane` (no attach). */ + captureOrphan(id: string): string | null; + /** End an orphan's shell. False when it is not ours, gone, or tracked. */ + killOrphan(id: string): boolean; + /** Kill every UNATTACHED orphan last active before `cutoffMs`; returns the ids. */ + killOrphansIdleSince(cutoffMs: number): string[]; /** Kill a session by id (manage page): close its clients, kill the PTY, drop it. * Returns true if a session was found and killed. */ killById(id: string): boolean; diff --git a/test/integration/orphan-sessions.test.ts b/test/integration/orphan-sessions.test.ts new file mode 100644 index 0000000..5d5aa6f --- /dev/null +++ b/test/integration/orphan-sessions.test.ts @@ -0,0 +1,208 @@ +/** + * Integration test for the orphan-session routes (A): + * GET /orphan-sessions + * GET /orphan-sessions/:id/preview + * DELETE /orphan-sessions/:id + * + * SAFETY: this file NEVER issues `DELETE /orphan-sessions` (the bulk cleanup), + * because these tests run against the host's real tmux server and would end the + * developer's own long-running sessions. Bulk cleanup is covered with mocks in + * test/manager-orphans.test.ts. The only session touched here is one this file + * creates under a fresh UUID and tears down itself. + */ + +import net from 'node:net' +import { execFileSync } from 'node:child_process' +import { randomUUID } from 'node:crypto' + +import { afterAll, beforeAll, describe, expect, it } from 'vitest' + +import { loadConfig } from '../../src/config.js' +import { startServer } from '../../src/server.js' +import type { OrphanSessionInfo } from '../../src/types.js' + +const TMUX_AVAILABLE = (() => { + try { + execFileSync('tmux', ['-V'], { stdio: 'ignore' }) + return true + } catch { + return false + } +})() +const itTmux = TMUX_AVAILABLE ? it : it.skip + +function getFreePort(): Promise { + return new Promise((resolve, reject) => { + const srv = net.createServer() + srv.listen(0, '127.0.0.1', () => { + const addr = srv.address() + if (addr === null || typeof addr === 'string') { + srv.close() + reject(new Error('unexpected address type')) + return + } + const port = addr.port + srv.close(() => resolve(port)) + }) + srv.on('error', reject) + }) +} + +/* ── tmux off: the whole feature must be inert ──────────────────────────────── */ + +describe('orphan sessions — tmux disabled', () => { + let port: number + let handle: { close(): Promise } + const origin = (): string => `http://127.0.0.1:${port}` + + beforeAll(async () => { + port = await getFreePort() + handle = startServer( + loadConfig({ + PORT: String(port), + BIND_HOST: '127.0.0.1', + SHELL_PATH: process.env['SHELL'] ?? '/bin/zsh', + ALLOWED_ORIGINS: `http://127.0.0.1:${port}`, + USE_TMUX: '0', + IDLE_TTL: '86400', + }), + ) + await new Promise((r) => setTimeout(r, 100)) + }) + + afterAll(async () => { + await handle.close() + }) + + it('lists nothing, and does not require an Origin header (read-only)', async () => { + const res = await fetch(`${origin()}/orphan-sessions`) + expect(res.status).toBe(200) + expect(await res.json()).toEqual([]) + }) + + it('404s a preview instead of shelling out to tmux', async () => { + const res = await fetch(`${origin()}/orphan-sessions/${randomUUID()}/preview`) + expect(res.status).toBe(404) + }) + + it('404s a kill', async () => { + const res = await fetch(`${origin()}/orphan-sessions/${randomUUID()}`, { + method: 'DELETE', + headers: { Origin: origin() }, + }) + expect(res.status).toBe(404) + }) +}) + +/* ── tmux on: real session, real capture, real kill ─────────────────────────── */ + +describe('orphan sessions — tmux enabled', () => { + let port: number + let handle: { close(): Promise } + const origin = (): string => `http://127.0.0.1:${port}` + + /** Our throwaway session. Never any other. */ + const id = randomUUID() + const name = `web_${id}` + let created = false + + beforeAll(async () => { + port = await getFreePort() + handle = startServer( + loadConfig({ + PORT: String(port), + BIND_HOST: '127.0.0.1', + SHELL_PATH: process.env['SHELL'] ?? '/bin/zsh', + ALLOWED_ORIGINS: `http://127.0.0.1:${port}`, + USE_TMUX: '1', + IDLE_TTL: '86400', + }), + ) + await new Promise((r) => setTimeout(r, 100)) + + if (TMUX_AVAILABLE) { + // Detached, printing a known marker so capture-pane has something to find. + execFileSync( + 'tmux', + ['new-session', '-d', '-s', name, `printf 'ORPHAN_MARKER_%s' OK; sleep 300`], + { stdio: 'ignore' }, + ) + created = true + await new Promise((r) => setTimeout(r, 400)) + } + }) + + afterAll(async () => { + if (created) { + try { + execFileSync('tmux', ['kill-session', '-t', name], { stdio: 'ignore' }) + } catch { + // the kill test already removed it — fine + } + } + await handle.close() + }) + + itTmux('lists our untracked tmux session as an orphan', async () => { + const res = await fetch(`${origin()}/orphan-sessions`) + expect(res.status).toBe(200) + const body = (await res.json()) as OrphanSessionInfo[] + const mine = body.find((o) => o.id === id) + expect(mine).toBeDefined() + expect(mine!.attached).toBe(false) // created with -d, nobody attached + expect(mine!.cols).toBeGreaterThan(0) + expect(mine!.lastActivityAt).toBeGreaterThan(0) + }) + + itTmux('previews the live screen without attaching to it', async () => { + const res = await fetch(`${origin()}/orphan-sessions/${id}/preview`) + expect(res.status).toBe(200) + const body = (await res.json()) as { id: string; data: string } + expect(body.id).toBe(id) + expect(body.data).toContain('ORPHAN_MARKER_OK') + + // The capture must not have left a client behind: still unattached. + const after = execFileSync('tmux', ['ls', '-F', '#{session_name} #{session_attached}'], { + encoding: 'utf8', + }) + expect(after).toContain(`${name} 0`) + }) + + itTmux('rejects a non-UUID id with 400, never reaching tmux', async () => { + for (const bad of ['-t', 'not-a-uuid', '../../etc/passwd']) { + const res = await fetch(`${origin()}/orphan-sessions/${encodeURIComponent(bad)}/preview`) + expect(res.status).toBe(400) + } + }) + + itTmux('refuses a kill with a foreign Origin (CSRF guard)', async () => { + const res = await fetch(`${origin()}/orphan-sessions/${id}`, { + method: 'DELETE', + headers: { Origin: 'http://evil.example' }, + }) + expect(res.status).toBe(403) + // and it is still alive + expect( + execFileSync('tmux', ['ls', '-F', '#{session_name}'], { encoding: 'utf8' }), + ).toContain(name) + }) + + itTmux('kills it, and 404s the second time', async () => { + const first = await fetch(`${origin()}/orphan-sessions/${id}`, { + method: 'DELETE', + headers: { Origin: origin() }, + }) + expect(first.status).toBe(204) + + const listing = execFileSync('tmux', ['ls', '-F', '#{session_name}'], { + encoding: 'utf8', + }).split('\n') + expect(listing).not.toContain(name) + + const second = await fetch(`${origin()}/orphan-sessions/${id}`, { + method: 'DELETE', + headers: { Origin: origin() }, + }) + expect(second.status).toBe(404) + }) +}) diff --git a/test/manager-orphans.test.ts b/test/manager-orphans.test.ts new file mode 100644 index 0000000..149ebac --- /dev/null +++ b/test/manager-orphans.test.ts @@ -0,0 +1,280 @@ +/** + * A — orphan tmux sessions: the manager side. + * + * An "orphan" is a `web_*` tmux session on the host that this server does NOT have + * in its table (it outlived the process that made it). These tests pin the three + * rules that keep the feature safe: + * + * 1. never enumerate or act on a tmux session that is not ours (`web_` + UUID v4); + * 2. never act on an id the table already owns — that path must stay killById, or + * we would kill the shell out from under a live PTY and leave a zombie entry; + * 3. do nothing at all when tmux is off, so the non-tmux deployment is unchanged. + * + * Both node-pty and the tmux CLI wrappers are mocked: no shell, no tmux binary. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +import type { Config, Dims, WebSocketLike } from '../src/types.js'; +import { WS_OPEN } from '../src/types.js'; +import { createMockPty, type MockIPty } from './helpers/mock-pty.js'; + +let nextPty: MockIPty = createMockPty(); + +vi.mock('node-pty', () => ({ + spawn: () => nextPty, +})); + +// ── tmux CLI mock ──────────────────────────────────────────────────────────── +const tmuxList = vi.fn<() => unknown[]>(() => []); +const tmuxCapture = vi.fn<(name: string) => string | null>(() => null); +const tmuxKill = vi.fn<(name: string) => void>(() => {}); +const tmuxHas = vi.fn<(name: string) => boolean>(() => true); + +vi.mock('../src/session/tmux.js', () => ({ + tmuxAvailable: () => true, + tmuxName: (id: string) => `web_${id}`, + hasSession: (n: string) => tmuxHas(n), + killSession: (n: string) => tmuxKill(n), + listSessions: () => tmuxList(), + capturePane: (n: string) => tmuxCapture(n), +})); + +const { createSessionManager } = await import('../src/session/manager.js'); + +const U1 = '05caa88b-1f5f-45d9-bbbe-fc5955314870'; +const U2 = '3a63f3c7-acfe-4081-992f-c4dbd55b3d6b'; + +const BASE_CFG = { + port: 3000, + bindHost: '0.0.0.0', + shellPath: '/bin/zsh', + homeDir: '/home/tester', + idleTtlMs: 10_000, + scrollbackBytes: 2 * 1024 * 1024, + maxPayloadBytes: 1024 * 1024, + wsPath: '/term', + maxSessions: 50, + maxMsgsPerSec: 2000, + permTimeoutMs: 300_000, + reapIntervalMs: 60_000, + previewBytes: 24 * 1024, + useTmux: true, + allowedOrigins: [], + projectRoots: ['/home/tester'], + projectScanDepth: 4, + projectScanTtlMs: 10_000, + projectDirtyCheck: true, + editorCmd: 'code', + vapidPublicKey: undefined, + vapidPrivateKey: undefined, + vapidSubject: undefined, + pushStorePath: '/home/tester/.push.json', + pushMaxSubs: 50, + notifyDone: true, + notifyDnd: false, + decisionTokenTtlMs: 300_000, + timelineMax: 200, + timelineEnabled: true, + stuckTtlMs: 10_000, + stuckAlert: true, + diffTimeoutMs: 2000, + diffMaxBytes: 2 * 1024 * 1024, + diffMaxFiles: 300, + statuslineTtlMs: 30_000, + worktreeEnabled: true, + worktreeRoot: undefined, + worktreeTimeoutMs: 10_000, + maxFanoutLanes: 6, + defaultPermissionMode: 'default' as const, + allowAutoMode: false, + queueEnabled: true, + queueMaxItems: 10, + queueItemMaxBytes: 4096, + queueSettleMs: 1500, +} satisfies Config; + +const CFG: Config = BASE_CFG; +const CFG_NO_TMUX: Config = { ...BASE_CFG, useTmux: false }; +const DIMS: Dims = { cols: 80, rows: 24 }; + +function createMockWs(): WebSocketLike & { readyState: number } { + return { + readyState: WS_OPEN, + send() {}, + close() {}, + }; +} + +/** A tmux summary row as src/session/tmux.ts would produce it. */ +function summary(id: string, activityMs = 5_000, attached = false) { + return { + id, + createdAtMs: 1_000, + lastActivityAtMs: activityMs, + attached, + cols: 120, + rows: 40, + }; +} + +beforeEach(() => { + nextPty = createMockPty(); + tmuxList.mockReset().mockReturnValue([]); + tmuxCapture.mockReset().mockReturnValue(null); + tmuxKill.mockReset(); + tmuxHas.mockReset().mockReturnValue(true); +}); + +describe('listOrphans', () => { + it('reports a tmux session the table does not know about', () => { + tmuxList.mockReturnValue([summary(U1, 9_000, true)]); + const mgr = createSessionManager(CFG); + + expect(mgr.listOrphans()).toEqual([ + { + id: U1, + createdAt: 1_000, + lastActivityAt: 9_000, + attached: true, + cols: 120, + rows: 40, + }, + ]); + }); + + it('excludes sessions the table already owns — those are live, not orphans', () => { + const mgr = createSessionManager(CFG); + const live = mgr.handleAttach(createMockWs(), null, DIMS, 1_000); + tmuxList.mockReturnValue([summary(live.meta.id), summary(U2)]); + + expect(mgr.listOrphans().map((o) => o.id)).toEqual([U2]); + }); + + it('is empty when tmux is off — the non-tmux deployment is untouched', () => { + tmuxList.mockReturnValue([summary(U1)]); + const mgr = createSessionManager(CFG_NO_TMUX); + + expect(mgr.listOrphans()).toEqual([]); + expect(tmuxList).not.toHaveBeenCalled(); + }); +}); + +describe('captureOrphan', () => { + it('returns the captured screen for an untracked session', () => { + tmuxCapture.mockReturnValue('screen contents'); + const mgr = createSessionManager(CFG); + + expect(mgr.captureOrphan(U1)).toBe('screen contents'); + expect(tmuxCapture).toHaveBeenCalledWith(`web_${U1}`); + }); + + it('refuses an id that is not a UUID v4 without invoking tmux at all', () => { + const mgr = createSessionManager(CFG); + + expect(mgr.captureOrphan('../../etc/passwd')).toBeNull(); + expect(mgr.captureOrphan('-t')).toBeNull(); + expect(tmuxCapture).not.toHaveBeenCalled(); + }); + + it('refuses when no such tmux session exists', () => { + tmuxHas.mockReturnValue(false); + const mgr = createSessionManager(CFG); + + expect(mgr.captureOrphan(U1)).toBeNull(); + expect(tmuxCapture).not.toHaveBeenCalled(); + }); + + it('refuses an id the table owns — a live session previews from its ring buffer', () => { + const mgr = createSessionManager(CFG); + const live = mgr.handleAttach(createMockWs(), null, DIMS, 1_000); + + expect(mgr.captureOrphan(live.meta.id)).toBeNull(); + expect(tmuxCapture).not.toHaveBeenCalled(); + }); + + it('returns null when tmux is off', () => { + const mgr = createSessionManager(CFG_NO_TMUX); + expect(mgr.captureOrphan(U1)).toBeNull(); + expect(tmuxCapture).not.toHaveBeenCalled(); + }); +}); + +describe('killOrphan', () => { + it('kills the tmux session by its prefixed name', () => { + const mgr = createSessionManager(CFG); + + expect(mgr.killOrphan(U1)).toBe(true); + expect(tmuxKill).toHaveBeenCalledWith(`web_${U1}`); + }); + + it('refuses a non-UUID id without invoking tmux', () => { + const mgr = createSessionManager(CFG); + + expect(mgr.killOrphan('web_x')).toBe(false); + expect(mgr.killOrphan('-t')).toBe(false); + expect(mgr.killOrphan('')).toBe(false); + expect(tmuxKill).not.toHaveBeenCalled(); + }); + + it('refuses an id the table owns — that is killById’s job', () => { + const mgr = createSessionManager(CFG); + const live = mgr.handleAttach(createMockWs(), null, DIMS, 1_000); + + expect(mgr.killOrphan(live.meta.id)).toBe(false); + expect(tmuxKill).not.toHaveBeenCalled(); + }); + + it('reports false when the session is already gone', () => { + tmuxHas.mockReturnValue(false); + const mgr = createSessionManager(CFG); + + expect(mgr.killOrphan(U1)).toBe(false); + expect(tmuxKill).not.toHaveBeenCalled(); + }); + + it('does nothing when tmux is off', () => { + const mgr = createSessionManager(CFG_NO_TMUX); + expect(mgr.killOrphan(U1)).toBe(false); + expect(tmuxKill).not.toHaveBeenCalled(); + }); +}); + +describe('killOrphansIdleSince', () => { + it('kills only sessions whose last activity is older than the cutoff', () => { + tmuxList.mockReturnValue([summary(U1, 1_000), summary(U2, 9_000)]); + const mgr = createSessionManager(CFG); + + expect(mgr.killOrphansIdleSince(5_000)).toEqual([U1]); + expect(tmuxKill).toHaveBeenCalledWith(`web_${U1}`); + expect(tmuxKill).toHaveBeenCalledTimes(1); + }); + + it('never kills a session someone is attached to from a terminal', () => { + // "Untracked by this server" is not "abandoned" — a plain `tmux attach` or a + // second server process both show up as attached, and bulk cleanup must not + // reach into either. + tmuxList.mockReturnValue([summary(U1, 1_000, true)]); + const mgr = createSessionManager(CFG); + + expect(mgr.killOrphansIdleSince(5_000)).toEqual([]); + expect(tmuxKill).not.toHaveBeenCalled(); + }); + + it('never kills a session the table owns, however idle tmux thinks it is', () => { + const mgr = createSessionManager(CFG); + const live = mgr.handleAttach(createMockWs(), null, DIMS, 1_000); + tmuxList.mockReturnValue([summary(live.meta.id, 0)]); + + expect(mgr.killOrphansIdleSince(5_000)).toEqual([]); + expect(tmuxKill).not.toHaveBeenCalled(); + }); + + it('does nothing when tmux is off', () => { + tmuxList.mockReturnValue([summary(U1, 0)]); + const mgr = createSessionManager(CFG_NO_TMUX); + + expect(mgr.killOrphansIdleSince(5_000)).toEqual([]); + expect(tmuxKill).not.toHaveBeenCalled(); + }); +}); diff --git a/test/tmux.test.ts b/test/tmux.test.ts index cea0dea..f4814c2 100644 --- a/test/tmux.test.ts +++ b/test/tmux.test.ts @@ -12,7 +12,8 @@ vi.mock('node:child_process', () => ({ execFileSync: (...a: unknown[]) => mockExec(...a), })) -const { tmuxAvailable, tmuxName, hasSession, killSession } = await import('../src/session/tmux.js') +const { tmuxAvailable, tmuxName, hasSession, killSession, parseSessionList, listSessions, capturePane } = + await import('../src/session/tmux.js') beforeEach(() => { mockExec.mockReset() @@ -68,3 +69,95 @@ describe('killSession', () => { expect(() => killSession('web_gone')).not.toThrow() }) }) + +/* ── A: enumerate + preview tmux sessions the server does not track ─────────── */ + +const U1 = '05caa88b-1f5f-45d9-bbbe-fc5955314870' +const U2 = '3a63f3c7-acfe-4081-992f-c4dbd55b3d6b' + +describe('parseSessionList', () => { + it('parses tmux rows into summaries, converting seconds to ms', () => { + const out = `web_${U1}\t1783911991\t1783911992\t0\t80\t23\n` + expect(parseSessionList(out)).toEqual([ + { + id: U1, + createdAtMs: 1783911991_000, + lastActivityAtMs: 1783911992_000, + attached: false, + cols: 80, + rows: 23, + }, + ]) + }) + + it('sorts most-recently-active first', () => { + const out = [`web_${U1}\t100\t100\t0\t80\t24`, `web_${U2}\t100\t900\t1\t80\t24`].join('\n') + expect(parseSessionList(out).map((s) => s.id)).toEqual([U2, U1]) + }) + + it('reports a session attached from a real terminal', () => { + const out = `web_${U1}\t100\t100\t1\t80\t24` + expect(parseSessionList(out)[0]!.attached).toBe(true) + }) + + it('ignores sessions that are not ours — never touch the user’s own tmux', () => { + const out = ['mywork\t100\t100\t0\t80\t24', `web_${U1}\t100\t100\t0\t80\t24`].join('\n') + expect(parseSessionList(out).map((s) => s.id)).toEqual([U1]) + }) + + it('ignores a web_-prefixed name whose suffix is not a UUID v4', () => { + // Only ids the protocol could have produced are actionable: anything else is + // someone else's session that merely starts with web_. + const out = ['web_../../etc\t100\t100\t0\t80\t24', 'web_-t\t100\t100\t0\t80\t24'].join('\n') + expect(parseSessionList(out)).toEqual([]) + }) + + it('skips malformed and blank rows instead of throwing', () => { + const out = ['', 'garbage', `web_${U1}\tNaN\t100\t0\t80\t24`, `web_${U2}\t100\t100\t0\t80\t24`].join('\n') + expect(parseSessionList(out).map((s) => s.id)).toEqual([U2]) + }) + + it('returns an empty list for empty output (no tmux server)', () => { + expect(parseSessionList('')).toEqual([]) + }) +}) + +describe('listSessions', () => { + it('asks tmux for a machine-readable list', () => { + mockExec.mockReturnValue(`web_${U1}\t100\t100\t0\t80\t24\n`) + expect(listSessions().map((s) => s.id)).toEqual([U1]) + const [file, args] = mockExec.mock.calls[0] as [string, string[]] + expect(file).toBe('tmux') + expect(args[0]).toBe('list-sessions') + expect(args).toContain('-F') + }) + + it('returns [] when there is no tmux server (exec throws)', () => { + mockExec.mockImplementation(() => { + throw new Error('no server running') + }) + expect(listSessions()).toEqual([]) + }) +}) + +describe('capturePane', () => { + it('captures the current screen WITHOUT attaching', () => { + mockExec.mockReturnValue('hello\n') + expect(capturePane('web_x')).toBe('hello\n') + const [file, args] = mockExec.mock.calls[0] as [string, string[]] + expect(file).toBe('tmux') + expect(args[0]).toBe('capture-pane') + expect(args).toContain('-p') // print to stdout — read-only, no client + expect(args).toContain('-e') // keep escape sequences so colour survives + expect(args).toContain('web_x') + // Attaching would resize the session and SIGWINCH whatever is running in it. + expect(args).not.toContain('attach-session') + }) + + it('returns null when the session is gone', () => { + mockExec.mockImplementation(() => { + throw new Error("can't find session") + }) + expect(capturePane('web_gone')).toBeNull() + }) +}) From f6ef19ebf69d9de6dd5fc496865109f0d7db79f6 Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Thu, 30 Jul 2026 11:28:39 +0200 Subject: [PATCH 2/2] fix(sessions): orphan cleanup was reading the wrong tmux clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review of the previous commit found ten defects. The critical one would have destroyed exactly the work this feature exists to protect. CRITICAL — `#{session_activity}` is a CLIENT clock, not an output clock. It advances on attach and on keypresses, and never moves for a detached session however much its shell is printing. Measured on tmux 3.6a: a detached session running `while true; do echo tick; sleep 1; done` still reported session_activity == session_created after 6 s, while `#{window_activity}` tracked the output exactly. So "idle for 7 days" was really "nobody has typed for 7 days", and `Clean up idle 7d+` would have killed long-running unattended sessions that were busy working — the walk-away case the whole app is built around. Measured against this host's 69 sessions: the old clock condemned 36, the new one condemns 33, so three sessions doing real work were one click from deletion. Now reads window_activity, with a test that fails if the format string regresses. HIGH — no timeout on any tmux call. maxBuffer bounds a flood but nothing bounded the wait, and a synchronous exec cannot be interrupted by the event loop, so a tmux server stopped mid-syscall hung the whole server permanently. All five calls (including the three that predate this feature) now carry one. HIGH — listSessions and capturePane were synchronous on a POLLED path. Measured 73 ms and 58 ms per call on this host; every home screen on every device refreshes on a 5 s timer, and each of those was 73 ms with the event loop stopped, i.e. 73 ms in which no PTY byte reached any terminal anywhere. Both are async now, so the manager's listOrphans/captureOrphan/killOrphansIdleSince are too. The server is a byte-shuttle first: nothing polled may block it. HIGH — GET /orphan-sessions had no rate limiter although, unlike every other read route, it spawns a subprocess. It shares the preview bucket now. HIGH — the cleanup confirmation counted CARDS, and the grid is capped at 24 while the cleanup matches across every session the server enumerates. On this host it would have said "24" and ended 33 shells. New GET /orphan-sessions/count-idle answers the real question, and the dialog states that number, or refuses to proceed if it cannot be determined. MEDIUM — fetchOrphanSessions collapsed every failure into [], so one 429 or 500 read as "all recovered sessions are gone", tearing down every mounted xterm and rebuilding it on the next success. It returns null for failure now, and the refresh leaves the section untouched. MEDIUM — the preview latch was "card was created", not "preview succeeded", so a card that lost its single request stayed blank forever. Latches on success. MEDIUM — overlapping refreshes. refresh() is entered from a 5 s timer, from killOrphan and from cleanup, with awaits inside and no guard, so a slow poll could resurrect cards for sessions already killed or re-mount them into a root that setVisible(false) had just cleared. A generation counter, bumped on teardown and checked after every await, invalidates stale runs. MEDIUM — renderPreview could write to a disposed xterm (it throws "Object has been disposed"). PreviewCard carries an explicit `disposed` flag set by a new disposeCard() helper, so no caller can dispose a terminal and forget the flag. Deliberately NOT el.isConnected: a card not yet appended is alive, and two existing preview-grid tests correctly caught that conflation. LOW — kill and cleanup discarded their results, so a refusal (403 behind a proxy whose Origin differs, 404 on a lost race) looked like a dead button. Both report. Tests: 2209 unit, 27 e2e, 8 orphan integration. Verified live against all 69 real sessions with none harmed; the window_activity fix confirmed on an isolated tmux socket (-L) so the developer's sessions were never involved. --- public/launcher.ts | 91 +++++++++++++++++++++++---------- public/preview-grid.ts | 45 +++++++++++++++-- src/server.ts | 33 +++++++++--- src/session/manager.ts | 23 +++++++-- src/session/tmux.ts | 65 ++++++++++++++++++------ src/types.ts | 14 ++++-- test/manager-orphans.test.ts | 98 ++++++++++++++++++++++-------------- test/tmux.test.ts | 81 +++++++++++++++++++++-------- 8 files changed, 330 insertions(+), 120 deletions(-) diff --git a/public/launcher.ts b/public/launcher.ts index 38f8325..e54bd2a 100644 --- a/public/launcher.ts +++ b/public/launcher.ts @@ -16,9 +16,11 @@ import { makePreviewCard, updatePreviewCard, loadPreviewInto, + disposeCard, fetchLiveSessions, fetchOrphanSessions, fetchOrphanPreview, + countIdleOrphans, renderPreview, orphanAsLive, killOrphanReq, @@ -82,7 +84,16 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher const cards = new Map() const orphanCards = new Map() + /** Ids whose preview has actually RENDERED. Latching on "card exists" instead + * left a card that lost its one request (429, or a 404 racing the list) blank + * forever, because nothing ever asked again. */ + const orphanPreviewed = new Set() let timer: ReturnType | null = null + /** Bumped by every teardown and every new refresh. An in-flight refresh compares + * it after each await and bails if it is stale — otherwise a slow poll could + * resurrect cards for sessions already killed, or re-mount them into a root that + * setVisible(false) has just cleared. */ + let generation = 0 const CLEANUP_IDLE_DAYS = 7 /** Thumbnails are xterm instances; a host with dozens of stale tmux sessions @@ -130,8 +141,7 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher } for (const [id, card] of cards) { if (!seen.has(id)) { - card.term.dispose() - card.el.remove() + disposeCard(card) cards.delete(id) } } @@ -139,9 +149,12 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher await refreshOrphans() } - /** Kill one orphan, then refresh. */ + /** Kill one orphan, then refresh. A refusal (403 from the Origin guard behind a + * proxy, 404 from a lost race) must be visible — silently doing nothing reads as + * a broken button. */ async function killOrphan(id: string): Promise { - await killOrphanReq(id) + const ok = await killOrphanReq(id) + if (!ok) orphanSub.textContent = `Could not end session ${id.slice(0, 8)} — it may already be gone.` void refresh() } @@ -158,15 +171,19 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher } async function refreshOrphans(): Promise { + const myGen = generation const orphans = await fetchOrphanSessions() + if (myGen !== generation) return // torn down or superseded while fetching + + // null = the request failed. Leave the section exactly as it is: reporting + // "none" would tear down every mounted xterm and rebuild it on recovery. + if (orphans === null) return if (orphans.length === 0) { orphanSection.style.display = 'none' - for (const card of orphanCards.values()) { - card.term.dispose() - card.el.remove() - } + for (const card of orphanCards.values()) disposeCard(card) orphanCards.clear() + orphanPreviewed.clear() return } @@ -185,7 +202,6 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher for (const o of shown) { seen.add(o.id) let card = orphanCards.get(o.id) - const isNew = card === undefined if (card === undefined) { card = makeOrphanCard(o) orphanCards.set(o.id, card) @@ -199,18 +215,21 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher // capture-pane, and re-capturing every card every tick would mean dozens of // processes a minute for screens that are, by definition, not being driven // from here. Fire-and-forget so a slow capture never stalls the next card. - if (isNew) { + if (!orphanPreviewed.has(o.id)) { const target = card - void fetchOrphanPreview(o.id).then((p) => { - if (p) renderPreview(target, p, THUMB_W, THUMB_MAX_H) + const targetId = o.id + void fetchOrphanPreview(targetId).then((p) => { + if (p === null || myGen !== generation) return + renderPreview(target, p, THUMB_W, THUMB_MAX_H) + orphanPreviewed.add(targetId) // only now stop asking }) } } for (const [id, card] of orphanCards) { if (!seen.has(id)) { - card.term.dispose() - card.el.remove() + disposeCard(card) orphanCards.delete(id) + orphanPreviewed.delete(id) } } @@ -223,17 +242,33 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher } cleanupBtn.addEventListener('click', () => { - const n = orphanCards.size - if ( - !window.confirm( - `Kill unattached recoverable sessions with no activity for ${CLEANUP_IDLE_DAYS} days?\n\n` + - `This ends their shells. Sessions someone is attached to are never touched. ` + - `(${n} recoverable session${n === 1 ? '' : 's'} listed.)`, - ) - ) { - return - } - void cleanupOrphansReq(CLEANUP_IDLE_DAYS).then(() => refresh()) + // Ask the SERVER how many it would end. orphanCards is capped at + // MAX_ORPHAN_CARDS while the cleanup matches across every session the server + // enumerates, so counting cards would understate the blast radius — on a host + // with 69 orphans the dialog would have said 24. + void countIdleOrphans(CLEANUP_IDLE_DAYS).then((n) => { + if (n === null) { + orphanSub.textContent = 'Could not check how many sessions are idle — not cleaning up.' + return + } + if (n === 0) { + orphanSub.textContent = `Nothing to clean up: no unattended session has been idle ${CLEANUP_IDLE_DAYS} days.` + return + } + if ( + !window.confirm( + `End ${n} session${n === 1 ? '' : 's'}?\n\n` + + `These are recoverable sessions with no tmux activity for ${CLEANUP_IDLE_DAYS} days. ` + + `Their shells will be terminated. Sessions someone is attached to are never touched.`, + ) + ) { + return + } + void cleanupOrphansReq(CLEANUP_IDLE_DAYS).then((ids) => { + if (ids.length === 0) orphanSub.textContent = 'Cleanup ended no sessions.' + void refresh() + }) + }) }) return { @@ -251,11 +286,13 @@ export function mountLauncher(host: HTMLElement, hooks: LauncherHooks): Launcher clearInterval(timer) timer = null } - for (const card of cards.values()) card.term.dispose() + for (const card of cards.values()) disposeCard(card) cards.clear() grid.replaceChildren() - for (const card of orphanCards.values()) card.term.dispose() + generation += 1 // invalidate any refresh still in flight + for (const card of orphanCards.values()) disposeCard(card) orphanCards.clear() + orphanPreviewed.clear() orphanGrid.replaceChildren() orphanSection.style.display = 'none' } diff --git a/public/preview-grid.ts b/public/preview-grid.ts index 2947d6f..214f371 100644 --- a/public/preview-grid.ts +++ b/public/preview-grid.ts @@ -67,6 +67,10 @@ export function sessionName(s: LiveSessionInfo): string { export interface PreviewCard { el: HTMLElement term: Terminal + /** Set by disposeCard. A preview is fetched fire-and-forget, so a card can be + * torn down while its request is in flight, and xterm throws "Object has been + * disposed" on a disposed terminal. Checked by renderPreview. */ + disposed: boolean inner: HTMLElement thumb: HTMLElement watch: HTMLElement @@ -125,7 +129,7 @@ export function makePreviewCard(s: LiveSessionInfo, opts: MakeCardOpts): Preview term.open(inner) cardEl.append(head, thumb, meta, actions) - return { el: cardEl, term, inner, thumb, watch, status, meta } + return { el: cardEl, term, inner, thumb, watch, status, meta, disposed: false } } /** Re-apply a session's live status/watch/meta fields to its card in place. */ @@ -136,6 +140,14 @@ export function updatePreviewCard(card: PreviewCard, s: LiveSessionInfo): void { card.watch.textContent = `👁 ${s.clientCount}` } +/** Tear a card down: mark it, dispose its terminal, detach its element. One place, + * so no caller can dispose the terminal and forget to set the flag. */ +export function disposeCard(card: PreviewCard): void { + card.disposed = true + card.term.dispose() + card.el.remove() +} + /** Scale the rendered xterm down so the full screen fits `thumbW` px wide. */ export function fitThumb(card: PreviewCard, thumbW: number, thumbMaxH: number): void { const w = card.inner.offsetWidth @@ -162,6 +174,11 @@ export async function fetchPreview(id: string): Promise { * orphan preview omits them because its dimensions came with the list, and * resizing to Math.max(2, 0) would collapse the thumbnail to 2×2. */ export function renderPreview(card: PreviewCard, p: SessionPreview, thumbW: number, thumbMaxH: number): void { + // A preview is fetched fire-and-forget, so the card can be torn down while its + // request is in flight, and xterm throws "Object has been disposed". An explicit + // flag, not el.isConnected: a card that has not been appended YET is alive, and + // conflating the two would drop the first render. + if (card.disposed) return if (p.cols > 0 && p.rows > 0 && (card.term.cols !== p.cols || card.term.rows !== p.rows)) { card.term.resize(Math.max(2, p.cols), Math.max(2, p.rows)) } @@ -196,14 +213,32 @@ export async function fetchLiveSessions(): Promise { card machinery above through orphanAsLive, so there is one thumbnail implementation rather than two that drift. */ -/** Fetch untracked tmux sessions, returning [] on failure. */ -export async function fetchOrphanSessions(): Promise { +/** + * Fetch untracked tmux sessions. Returns null on FAILURE, distinct from [] meaning + * "there are none" — collapsing the two would let a 429, a 500 or a dropped request + * read as "all recovered sessions are gone", tearing the grid down and re-mounting + * every xterm on the next success. + */ +export async function fetchOrphanSessions(): Promise { try { const res = await fetch('/orphan-sessions') + if (!res.ok) return null const data: unknown = await res.json() - return Array.isArray(data) ? (data as OrphanSessionInfo[]) : [] + return Array.isArray(data) ? (data as OrphanSessionInfo[]) : null } catch { - return [] + return null + } +} + +/** How many sessions the bulk cleanup would actually end. null on failure. */ +export async function countIdleOrphans(days: number): Promise { + try { + const res = await fetch(`/orphan-sessions/count-idle?idleDays=${encodeURIComponent(String(days))}`) + if (!res.ok) return null + const body = (await res.json()) as { count?: unknown } + return typeof body.count === 'number' ? body.count : null + } catch { + return null } } diff --git a/src/server.ts b/src/server.ts index c1294ea..ebf497d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -599,13 +599,34 @@ export function startServer(cfg: Config): { close(): Promise } { // it: an orphan supports none of the live-session operations (no replay, status, // telemetry or inject queue), and the Android/iOS clients decode // /live-sessions — adding a variant shape there would break them. - app.get('/orphan-sessions', (_req, res) => { - res.json(manager.listOrphans()) + // Rate-limited even though it is read-only: unlike the other read routes this one + // spawns a subprocess, so an unbounded caller is a spawn loop against the host. + app.get('/orphan-sessions', async (req, res) => { + if (!orphanLimiter(req.socket.remoteAddress ?? '', Date.now())) { + res.status(429).end() + return + } + res.json(await manager.listOrphans()) + }) + + // How many the bulk cleanup would kill, so the client can confirm with a real + // number instead of counting the cards it happens to be showing. + app.get('/orphan-sessions/count-idle', async (req, res) => { + if (!orphanLimiter(req.socket.remoteAddress ?? '', Date.now())) { + res.status(429).end() + return + } + const days = Number(req.query['idleDays']) + if (!Number.isFinite(days) || days < 1) { + res.status(400).json({ error: 'idleDays must be a number >= 1' }) + return + } + res.json({ count: await manager.countOrphansIdleSince(Date.now() - days * 86_400_000) }) }) // Each call spawns a short-lived `tmux capture-pane`, so it gets the same per-IP // bucket discipline as the other process-spawning routes. - app.get('/orphan-sessions/:id/preview', (req, res) => { + app.get('/orphan-sessions/:id/preview', async (req, res) => { if (!orphanLimiter(req.socket.remoteAddress ?? '', Date.now())) { res.status(429).end() return @@ -615,7 +636,7 @@ export function startServer(cfg: Config): { close(): Promise } { res.status(400).json({ error: 'invalid session id' }) return } - const data = manager.captureOrphan(id) + const data = await manager.captureOrphan(id) if (data === null) { res.status(404).end() return @@ -627,14 +648,14 @@ export function startServer(cfg: Config): { close(): Promise } { // Bulk cleanup — kill UNATTACHED orphans idle longer than `idleDays`. Registered // BEFORE /orphan-sessions/:id so it is not captured as an :id. `idleDays` is // required and must be >= 1: there is no "delete everything" form of this route. - app.delete('/orphan-sessions', (req, res) => { + app.delete('/orphan-sessions', async (req, res) => { if (!requireAllowedOrigin(req, res)) return const days = Number(req.query['idleDays']) if (!Number.isFinite(days) || days < 1) { res.status(400).json({ error: 'idleDays must be a number >= 1' }) return } - const killed = manager.killOrphansIdleSince(Date.now() - days * 86_400_000) + const killed = await manager.killOrphansIdleSince(Date.now() - days * 86_400_000) res.json({ killed: killed.length, ids: killed }) }) diff --git a/src/session/manager.ts b/src/session/manager.ts index 880a6d4..3f86574 100644 --- a/src/session/manager.ts +++ b/src/session/manager.ts @@ -439,9 +439,10 @@ export function createSessionManager( return hasSession(tmuxName(id)); } - function listOrphans(): OrphanSessionInfo[] { + async function listOrphans(): Promise { if (!cfg.useTmux) return []; - return listSessions() + const all = await listSessions(); + return all .filter((t) => !sessions.has(t.id)) .map((t) => ({ id: t.id, @@ -454,7 +455,7 @@ export function createSessionManager( } /** The orphan's current screen, or null. Read-only: never attaches. */ - function captureOrphan(id: string): string | null { + async function captureOrphan(id: string): Promise { if (!isActionableOrphan(id)) return null; return capturePane(tmuxName(id)); } @@ -474,9 +475,9 @@ export function createSessionManager( * someone is in there from a real terminal or another server process, and * "this server does not track it" is not evidence that it is abandoned. */ - function killOrphansIdleSince(cutoffMs: number): string[] { + async function killOrphansIdleSince(cutoffMs: number): Promise { const killed: string[] = []; - for (const o of listOrphans()) { + for (const o of await listOrphans()) { if (o.attached) continue; if (o.lastActivityAt >= cutoffMs) continue; if (killOrphan(o.id)) killed.push(o.id); @@ -484,6 +485,17 @@ export function createSessionManager( return killed; } + /** How many orphans `killOrphansIdleSince(cutoffMs)` would kill, without killing + * anything. The UI needs this to state a truthful number in its confirmation: + * the card grid is capped, so counting cards understates the blast radius. */ + async function countOrphansIdleSince(cutoffMs: number): Promise { + let n = 0; + for (const o of await listOrphans()) { + if (!o.attached && o.lastActivityAt < cutoffMs) n += 1; + } + return n; + } + return { handleAttach, get, @@ -492,6 +504,7 @@ export function createSessionManager( captureOrphan, killOrphan, killOrphansIdleSince, + countOrphansIdleSince, killById, handleHookEvent, handleStatusLine, diff --git a/src/session/tmux.ts b/src/session/tmux.ts index b64a271..d4d23e1 100644 --- a/src/session/tmux.ts +++ b/src/session/tmux.ts @@ -9,18 +9,38 @@ * treated as "false"/no-op). */ -import { execFileSync } from 'node:child_process' +import { execFile, execFileSync } from 'node:child_process' +import { promisify } from 'node:util' import { SESSION_ID_RE } from '../protocol.js' +const execFileAsync = promisify(execFile) + /** Upper bound on one `capture-pane` payload. A pane is one screen, so this is * generous; it exists so a wedged tmux can never hand us unbounded output. */ const CAPTURE_MAX_BUFFER = 512 * 1024 -/** Tab-separated, machine-readable. tmux reports the two clocks in unix SECONDS. */ +/** Every tmux call is bounded. maxBuffer caps a flood but nothing caps the WAIT, + * and a tmux server stopped mid-syscall (SIGSTOP, a hung filesystem) would + * otherwise block forever — fatally so for the synchronous calls, which the event + * loop cannot interrupt. */ +const TMUX_TIMEOUT_MS = 3000 + +/** + * Tab-separated, machine-readable. tmux reports both clocks in unix SECONDS. + * + * The activity clock is `window_activity`, NOT `session_activity`. That is not a + * detail: `session_activity` tracks CLIENT activity — it advances on attach and on + * keypresses — and never moves for a detached session no matter how much its shell + * is printing. Measured on tmux 3.6a with a detached session in a `while true; do + * echo tick; sleep 1; done` loop: after 6s, session_activity was still equal to + * session_created while window_activity had advanced. Using session_activity as the + * liveness signal would make the idle-cleanup kill precisely the long-running, + * unattended, actively-working sessions this whole feature exists to protect. + */ const LIST_FORMAT = [ '#{session_name}', '#{session_created}', - '#{session_activity}', + '#{window_activity}', '#{session_attached}', '#{window_width}', '#{window_height}', @@ -46,7 +66,7 @@ export interface TmuxSessionSummary { /** Is the tmux binary available on PATH? */ export function tmuxAvailable(): boolean { try { - execFileSync('tmux', ['-V'], { stdio: 'ignore' }) + execFileSync('tmux', ['-V'], { stdio: 'ignore', timeout: TMUX_TIMEOUT_MS }) return true } catch { return false @@ -61,7 +81,10 @@ export function tmuxName(sessionId: string): string { /** Does a tmux session with this name already exist (e.g. after a restart)? */ export function hasSession(name: string): boolean { try { - execFileSync('tmux', ['has-session', '-t', name], { stdio: 'ignore' }) + execFileSync('tmux', ['has-session', '-t', name], { + stdio: 'ignore', + timeout: TMUX_TIMEOUT_MS, + }) return true } catch { return false @@ -71,7 +94,10 @@ export function hasSession(name: string): boolean { /** Kill a tmux session (ends the shell). No-op if it's already gone. */ export function killSession(name: string): void { try { - execFileSync('tmux', ['kill-session', '-t', name], { stdio: 'ignore' }) + execFileSync('tmux', ['kill-session', '-t', name], { + stdio: 'ignore', + timeout: TMUX_TIMEOUT_MS, + }) } catch { // already gone — fine } @@ -133,16 +159,24 @@ export function parseSessionList(stdout: string): TmuxSessionSummary[] { return rows.sort((a, b) => b.lastActivityAtMs - a.lastActivityAtMs) } -/** Every `web_*` tmux session on the host, most recently active first. Never - * throws; [] when tmux is missing or no tmux server is running. */ -export function listSessions(): TmuxSessionSummary[] { +/** + * Every `web_*` tmux session on the host, most recently active first. Never + * throws; [] when tmux is missing or no tmux server is running. + * + * ASYNC on purpose. This is polled — every device's home screen refreshes on a + * timer — and the spawn costs ~73 ms on a host with 69 sessions. Done + * synchronously that is 73 ms with the event loop stopped, i.e. 73 ms during which + * no PTY byte reaches any terminal on any device. The server is a byte-shuttle + * first; nothing on a polling path may block it. + */ +export async function listSessions(): Promise { try { - const out = execFileSync('tmux', ['list-sessions', '-F', LIST_FORMAT], { + const { stdout } = await execFileAsync('tmux', ['list-sessions', '-F', LIST_FORMAT], { encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], maxBuffer: CAPTURE_MAX_BUFFER, + timeout: TMUX_TIMEOUT_MS, }) - return parseSessionList(out) + return parseSessionList(stdout) } catch { return [] } @@ -156,13 +190,14 @@ export function listSessions(): TmuxSessionSummary[] { * `attach-session`: attaching would make tmux resize the window to the new * client's dimensions and SIGWINCH whatever is running inside it. */ -export function capturePane(name: string): string | null { +export async function capturePane(name: string): Promise { try { - return execFileSync('tmux', ['capture-pane', '-p', '-e', '-t', name], { + const { stdout } = await execFileAsync('tmux', ['capture-pane', '-p', '-e', '-t', name], { encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], maxBuffer: CAPTURE_MAX_BUFFER, + timeout: TMUX_TIMEOUT_MS, }) + return stdout } catch { return null } diff --git a/src/types.ts b/src/types.ts index f38fffd..5c70277 100644 --- a/src/types.ts +++ b/src/types.ts @@ -478,15 +478,21 @@ export interface SessionManager { reject an id that is not a UUID v4 or that the table already owns — a tracked session must go through get()/killById so its PTY and table entry stay in step. None of them adopt: nothing is attached, so a running TUI is - never resized. */ + never resized. + + The tmux-spawning ones are ASYNC: they sit on a polled path and each spawn + costs tens of milliseconds, which done synchronously is time the event loop + is stopped and no PTY byte reaches any terminal. */ /** Untracked `web_*` tmux sessions, most recently active first. */ - listOrphans(): OrphanSessionInfo[]; + listOrphans(): Promise; /** An orphan's current screen via `tmux capture-pane` (no attach). */ - captureOrphan(id: string): string | null; + captureOrphan(id: string): Promise; /** End an orphan's shell. False when it is not ours, gone, or tracked. */ killOrphan(id: string): boolean; /** Kill every UNATTACHED orphan last active before `cutoffMs`; returns the ids. */ - killOrphansIdleSince(cutoffMs: number): string[]; + killOrphansIdleSince(cutoffMs: number): Promise; + /** How many killOrphansIdleSince would kill — for a truthful confirmation. */ + countOrphansIdleSince(cutoffMs: number): Promise; /** Kill a session by id (manage page): close its clients, kill the PTY, drop it. * Returns true if a session was found and killed. */ killById(id: string): boolean; diff --git a/test/manager-orphans.test.ts b/test/manager-orphans.test.ts index 149ebac..928c07f 100644 --- a/test/manager-orphans.test.ts +++ b/test/manager-orphans.test.ts @@ -26,8 +26,8 @@ vi.mock('node-pty', () => ({ })); // ── tmux CLI mock ──────────────────────────────────────────────────────────── -const tmuxList = vi.fn<() => unknown[]>(() => []); -const tmuxCapture = vi.fn<(name: string) => string | null>(() => null); +const tmuxList = vi.fn<() => Promise>(async () => []); +const tmuxCapture = vi.fn<(name: string) => Promise>(async () => null); const tmuxKill = vi.fn<(name: string) => void>(() => {}); const tmuxHas = vi.fn<(name: string) => boolean>(() => true); @@ -120,18 +120,18 @@ function summary(id: string, activityMs = 5_000, attached = false) { beforeEach(() => { nextPty = createMockPty(); - tmuxList.mockReset().mockReturnValue([]); - tmuxCapture.mockReset().mockReturnValue(null); + tmuxList.mockReset().mockResolvedValue([]); + tmuxCapture.mockReset().mockResolvedValue(null); tmuxKill.mockReset(); tmuxHas.mockReset().mockReturnValue(true); }); describe('listOrphans', () => { - it('reports a tmux session the table does not know about', () => { - tmuxList.mockReturnValue([summary(U1, 9_000, true)]); + it('reports a tmux session the table does not know about', async () => { + tmuxList.mockResolvedValue([summary(U1, 9_000, true)]); const mgr = createSessionManager(CFG); - expect(mgr.listOrphans()).toEqual([ + expect(await mgr.listOrphans()).toEqual([ { id: U1, createdAt: 1_000, @@ -143,59 +143,59 @@ describe('listOrphans', () => { ]); }); - it('excludes sessions the table already owns — those are live, not orphans', () => { + it('excludes sessions the table already owns — those are live, not orphans', async () => { const mgr = createSessionManager(CFG); const live = mgr.handleAttach(createMockWs(), null, DIMS, 1_000); - tmuxList.mockReturnValue([summary(live.meta.id), summary(U2)]); + tmuxList.mockResolvedValue([summary(live.meta.id), summary(U2)]); - expect(mgr.listOrphans().map((o) => o.id)).toEqual([U2]); + expect((await mgr.listOrphans()).map((o) => o.id)).toEqual([U2]); }); - it('is empty when tmux is off — the non-tmux deployment is untouched', () => { - tmuxList.mockReturnValue([summary(U1)]); + it('is empty when tmux is off — the non-tmux deployment is untouched', async () => { + tmuxList.mockResolvedValue([summary(U1)]); const mgr = createSessionManager(CFG_NO_TMUX); - expect(mgr.listOrphans()).toEqual([]); + expect(await mgr.listOrphans()).toEqual([]); expect(tmuxList).not.toHaveBeenCalled(); }); }); describe('captureOrphan', () => { - it('returns the captured screen for an untracked session', () => { - tmuxCapture.mockReturnValue('screen contents'); + it('returns the captured screen for an untracked session', async () => { + tmuxCapture.mockResolvedValue('screen contents'); const mgr = createSessionManager(CFG); - expect(mgr.captureOrphan(U1)).toBe('screen contents'); + expect(await mgr.captureOrphan(U1)).toBe('screen contents'); expect(tmuxCapture).toHaveBeenCalledWith(`web_${U1}`); }); - it('refuses an id that is not a UUID v4 without invoking tmux at all', () => { + it('refuses an id that is not a UUID v4 without invoking tmux at all', async () => { const mgr = createSessionManager(CFG); - expect(mgr.captureOrphan('../../etc/passwd')).toBeNull(); - expect(mgr.captureOrphan('-t')).toBeNull(); + expect(await mgr.captureOrphan('../../etc/passwd')).toBeNull(); + expect(await mgr.captureOrphan('-t')).toBeNull(); expect(tmuxCapture).not.toHaveBeenCalled(); }); - it('refuses when no such tmux session exists', () => { + it('refuses when no such tmux session exists', async () => { tmuxHas.mockReturnValue(false); const mgr = createSessionManager(CFG); - expect(mgr.captureOrphan(U1)).toBeNull(); + expect(await mgr.captureOrphan(U1)).toBeNull(); expect(tmuxCapture).not.toHaveBeenCalled(); }); - it('refuses an id the table owns — a live session previews from its ring buffer', () => { + it('refuses an id the table owns — a live session previews from its ring buffer', async () => { const mgr = createSessionManager(CFG); const live = mgr.handleAttach(createMockWs(), null, DIMS, 1_000); - expect(mgr.captureOrphan(live.meta.id)).toBeNull(); + expect(await mgr.captureOrphan(live.meta.id)).toBeNull(); expect(tmuxCapture).not.toHaveBeenCalled(); }); - it('returns null when tmux is off', () => { + it('returns null when tmux is off', async () => { const mgr = createSessionManager(CFG_NO_TMUX); - expect(mgr.captureOrphan(U1)).toBeNull(); + expect(await mgr.captureOrphan(U1)).toBeNull(); expect(tmuxCapture).not.toHaveBeenCalled(); }); }); @@ -241,40 +241,64 @@ describe('killOrphan', () => { }); describe('killOrphansIdleSince', () => { - it('kills only sessions whose last activity is older than the cutoff', () => { - tmuxList.mockReturnValue([summary(U1, 1_000), summary(U2, 9_000)]); + it('kills only sessions whose last activity is older than the cutoff', async () => { + tmuxList.mockResolvedValue([summary(U1, 1_000), summary(U2, 9_000)]); const mgr = createSessionManager(CFG); - expect(mgr.killOrphansIdleSince(5_000)).toEqual([U1]); + expect(await mgr.killOrphansIdleSince(5_000)).toEqual([U1]); expect(tmuxKill).toHaveBeenCalledWith(`web_${U1}`); expect(tmuxKill).toHaveBeenCalledTimes(1); }); - it('never kills a session someone is attached to from a terminal', () => { + it('never kills a session someone is attached to from a terminal', async () => { // "Untracked by this server" is not "abandoned" — a plain `tmux attach` or a // second server process both show up as attached, and bulk cleanup must not // reach into either. - tmuxList.mockReturnValue([summary(U1, 1_000, true)]); + tmuxList.mockResolvedValue([summary(U1, 1_000, true)]); const mgr = createSessionManager(CFG); - expect(mgr.killOrphansIdleSince(5_000)).toEqual([]); + expect(await mgr.killOrphansIdleSince(5_000)).toEqual([]); expect(tmuxKill).not.toHaveBeenCalled(); }); - it('never kills a session the table owns, however idle tmux thinks it is', () => { + it('never kills a session the table owns, however idle tmux thinks it is', async () => { const mgr = createSessionManager(CFG); const live = mgr.handleAttach(createMockWs(), null, DIMS, 1_000); - tmuxList.mockReturnValue([summary(live.meta.id, 0)]); + tmuxList.mockResolvedValue([summary(live.meta.id, 0)]); - expect(mgr.killOrphansIdleSince(5_000)).toEqual([]); + expect(await mgr.killOrphansIdleSince(5_000)).toEqual([]); expect(tmuxKill).not.toHaveBeenCalled(); }); - it('does nothing when tmux is off', () => { - tmuxList.mockReturnValue([summary(U1, 0)]); + it('does nothing when tmux is off', async () => { + tmuxList.mockResolvedValue([summary(U1, 0)]); const mgr = createSessionManager(CFG_NO_TMUX); - expect(mgr.killOrphansIdleSince(5_000)).toEqual([]); + expect(await mgr.killOrphansIdleSince(5_000)).toEqual([]); expect(tmuxKill).not.toHaveBeenCalled(); }); }); + +describe('countOrphansIdleSince', () => { + it('counts exactly what killOrphansIdleSince would kill, killing nothing', async () => { + // The UI confirmation must state this number, not the number of cards it is + // showing: the grid is capped, so counting cards understates the blast radius. + tmuxList.mockResolvedValue([summary(U1, 1_000), summary(U2, 9_000)]); + const mgr = createSessionManager(CFG); + + expect(await mgr.countOrphansIdleSince(5_000)).toBe(1); + expect(tmuxKill).not.toHaveBeenCalled(); + }); + + it('does not count attached sessions', async () => { + tmuxList.mockResolvedValue([summary(U1, 1_000, true), summary(U2, 1_000)]); + const mgr = createSessionManager(CFG); + + expect(await mgr.countOrphansIdleSince(5_000)).toBe(1); + }); + + it('is 0 when tmux is off', async () => { + const mgr = createSessionManager(CFG_NO_TMUX); + expect(await mgr.countOrphansIdleSince(5_000)).toBe(0); + }); +}); diff --git a/test/tmux.test.ts b/test/tmux.test.ts index f4814c2..626dd90 100644 --- a/test/tmux.test.ts +++ b/test/tmux.test.ts @@ -8,8 +8,15 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' const mockExec = vi.fn() +// promisify(execFile) reads the custom symbol, so give it one that returns our mock. +const mockExecAsync = vi.fn() +const fakeExecFile = Object.assign( + (...a: unknown[]) => mockExecAsync(...a), + { [Symbol.for('nodejs.util.promisify.custom')]: (...a: unknown[]) => mockExecAsync(...a) }, +) vi.mock('node:child_process', () => ({ execFileSync: (...a: unknown[]) => mockExec(...a), + execFile: fakeExecFile, })) const { tmuxAvailable, tmuxName, hasSession, killSession, parseSessionList, listSessions, capturePane } = @@ -17,6 +24,7 @@ const { tmuxAvailable, tmuxName, hasSession, killSession, parseSessionList, list beforeEach(() => { mockExec.mockReset() + mockExecAsync.mockReset() }) describe('tmuxName', () => { @@ -29,7 +37,7 @@ describe('tmuxAvailable', () => { it('returns true when `tmux -V` succeeds', () => { mockExec.mockReturnValue(Buffer.from('tmux 3.4')) expect(tmuxAvailable()).toBe(true) - expect(mockExec).toHaveBeenCalledWith('tmux', ['-V'], { stdio: 'ignore' }) + expect(mockExec).toHaveBeenCalledWith('tmux', ['-V'], expect.objectContaining({ stdio: 'ignore' })) }) it('returns false when tmux is missing (exec throws)', () => { @@ -44,7 +52,11 @@ describe('hasSession', () => { it('returns true when has-session exits 0', () => { mockExec.mockReturnValue(Buffer.from('')) expect(hasSession('web_x')).toBe(true) - expect(mockExec).toHaveBeenCalledWith('tmux', ['has-session', '-t', 'web_x'], { stdio: 'ignore' }) + expect(mockExec).toHaveBeenCalledWith( + 'tmux', + ['has-session', '-t', 'web_x'], + expect.objectContaining({ stdio: 'ignore' }), + ) }) it('returns false when has-session throws (no such session)', () => { @@ -59,7 +71,11 @@ describe('killSession', () => { it('invokes tmux kill-session for the name', () => { mockExec.mockReturnValue(Buffer.from('')) killSession('web_x') - expect(mockExec).toHaveBeenCalledWith('tmux', ['kill-session', '-t', 'web_x'], { stdio: 'ignore' }) + expect(mockExec).toHaveBeenCalledWith( + 'tmux', + ['kill-session', '-t', 'web_x'], + expect.objectContaining({ stdio: 'ignore' }), + ) }) it('swallows errors when the session is already gone', () => { @@ -123,28 +139,46 @@ describe('parseSessionList', () => { }) describe('listSessions', () => { - it('asks tmux for a machine-readable list', () => { - mockExec.mockReturnValue(`web_${U1}\t100\t100\t0\t80\t24\n`) - expect(listSessions().map((s) => s.id)).toEqual([U1]) - const [file, args] = mockExec.mock.calls[0] as [string, string[]] + it('asks tmux for a machine-readable list', async () => { + mockExecAsync.mockResolvedValue({ stdout: `web_${U1}\t100\t100\t0\t80\t24\n`, stderr: '' }) + expect((await listSessions()).map((s) => s.id)).toEqual([U1]) + const [file, args] = mockExecAsync.mock.calls[0] as [string, string[]] expect(file).toBe('tmux') expect(args[0]).toBe('list-sessions') expect(args).toContain('-F') }) - it('returns [] when there is no tmux server (exec throws)', () => { - mockExec.mockImplementation(() => { - throw new Error('no server running') - }) - expect(listSessions()).toEqual([]) + it('asks for window_activity, NOT session_activity', async () => { + // session_activity is a CLIENT clock: it never advances for a detached session + // however much its shell prints. Using it would make idle-cleanup kill exactly + // the unattended, actively-working sessions this feature exists to protect. + mockExecAsync.mockResolvedValue({ stdout: '', stderr: '' }) + await listSessions() + const [, args] = mockExecAsync.mock.calls[0] as [string, string[]] + const fmt = args[args.indexOf('-F') + 1]! + expect(fmt).toContain('#{window_activity}') + expect(fmt).not.toContain('#{session_activity}') + }) + + it('bounds the call so a wedged tmux cannot hang the server', async () => { + mockExecAsync.mockResolvedValue({ stdout: '', stderr: '' }) + await listSessions() + const opts = mockExecAsync.mock.calls[0]![2] as { timeout?: number; maxBuffer?: number } + expect(opts.timeout).toBeGreaterThan(0) + expect(opts.maxBuffer).toBeGreaterThan(0) + }) + + it('returns [] when there is no tmux server (exec rejects)', async () => { + mockExecAsync.mockRejectedValue(new Error('no server running')) + expect(await listSessions()).toEqual([]) }) }) describe('capturePane', () => { - it('captures the current screen WITHOUT attaching', () => { - mockExec.mockReturnValue('hello\n') - expect(capturePane('web_x')).toBe('hello\n') - const [file, args] = mockExec.mock.calls[0] as [string, string[]] + it('captures the current screen WITHOUT attaching', async () => { + mockExecAsync.mockResolvedValue({ stdout: 'hello\n', stderr: '' }) + expect(await capturePane('web_x')).toBe('hello\n') + const [file, args] = mockExecAsync.mock.calls[0] as [string, string[]] expect(file).toBe('tmux') expect(args[0]).toBe('capture-pane') expect(args).toContain('-p') // print to stdout — read-only, no client @@ -154,10 +188,15 @@ describe('capturePane', () => { expect(args).not.toContain('attach-session') }) - it('returns null when the session is gone', () => { - mockExec.mockImplementation(() => { - throw new Error("can't find session") - }) - expect(capturePane('web_gone')).toBeNull() + it('returns null when the session is gone', async () => { + mockExecAsync.mockRejectedValue(new Error("can't find session")) + expect(await capturePane('web_gone')).toBeNull() + }) + + it('bounds the call', async () => { + mockExecAsync.mockResolvedValue({ stdout: '', stderr: '' }) + await capturePane('web_x') + const opts = mockExecAsync.mock.calls[0]![2] as { timeout?: number } + expect(opts.timeout).toBeGreaterThan(0) }) })