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) }) })