From 4aeb8486c46df72771369e1b702f36d244366b0a Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:08:02 +0530 Subject: [PATCH 1/2] Startup picker: distinct Resume, New and Kill; counted kill confirmations With several detached sessions, Enter resumes the highlighted session (or the selected ones) instead of starting a new one; N or Esc starts new. X kills the selection or the highlighted session after a confirmation that names the count and lists them, so A then X is Kill all; sessions attached in another window are never touched and each outcome is reported. Rows and the one-session prompt show name, directory, shell and the same evidence based status as /sessions. The picker redraws on resize. Nothing about launching or leaving NMSh ends a session. --- CHANGELOG.md | 4 + docs/design/session-interaction-ux.md | 6 +- src/session/StartupPicker.ts | 116 ++++++++++++++++++-------- tests/startupRestore.test.ts | 72 ++++++++++++++-- 4 files changed, 152 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3e165a6..8fa3b9a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **Markdown editing helpers in `/edit`** ([#463](https://github.com/raiseCatError/notMyShell/issues/463), first slice; Markdown files only). **Enter** continues a bullet, numbered or task list (same marker and indentation, next number, unchecked box) and splits an item at the caret; on an empty item it ends the list (a nested one steps out a level). **Tab / Shift+Tab** on list items indent or outdent the item with its children (selected items together); elsewhere Tab is a tab as before. **F1** opens the Markdown actions: toggle a task checkbox, renumber the list at the caret, insert a link or image around the selection, wrap the selection in bold, italic, code or strikethrough (again to unwrap). Every action changes only the lines it names and is one undo step; text is never reflowed or regenerated. - **Crash recovery for `/edit` and the document workspace** ([#469](https://github.com/raiseCatError/notMyShell/issues/469)). Unsaved changes are written, a moment after you stop typing, to a private draft in NMSh's data folder (`recovery/`, mode 0700; files 0600). After a crash, a kill or a closed window, opening the file offers **Restore draft**, **Compare with file**, **Discard draft** or **Keep for later**; nothing is applied until you choose, a draft made from an older version of the file says so, and the file itself is only ever written by the editor's normal checked save (which also retires the draft). Two windows on one file keep separate drafts, and a draft of a window that is still running is never offered. Drafts are bounded (2 MiB each, 50 in all, 30 days) and are never sent anywhere. +### Changed + +- **Startup session picker** ([design](docs/design/session-interaction-ux.md)). With several detached sessions, Enter now resumes the highlighted session when none is selected (it used to start a new one), N or Esc starts a new session, and the footer says what Enter and X will act on. X kills the selected sessions, or the highlighted one: the confirmation names the count and lists them, so A then X is Kill all; sessions attached in another window are still never touched, and a line reports what happened to each. Rows and the one-session prompt show the session's name and shell and the same evidence-based status as `/resume` (idle, what runs, waiting for input) instead of always saying `zsh`, and the screen redraws on resize. + ## [0.23.0] - 2026-10-21 — Vespyr Arcade The Arcade grows from five games to 33, and it can now be played while a command keeps running. diff --git a/docs/design/session-interaction-ux.md b/docs/design/session-interaction-ux.md index 029fff7c..eb580d33 100644 --- a/docs/design/session-interaction-ux.md +++ b/docs/design/session-interaction-ux.md @@ -135,11 +135,11 @@ Live sessions are owned by the per-user session service (`nmshd`), which listens - **Session limit:** at most 16 live sessions per service (`NMSH_MAX_SESSIONS`). When the limit is reached, a new window falls back to an in-process shell with a notice. That window's shell ends when the window closes; it cannot be detached or reattached. Detached sessions are never ended to make room; end one, or kill it from `/resume`. - **Startup restore:** two settings in Config → Sessions. - **Startup restore:** Ask (default), Always or Never. - - Ask: with one detached session, the launch shows its directory, what it is running and its age, then offers Resume (R/Enter), Not now (N/Esc), Always resume (A) or Don't resume at startup (D). A and D also save the setting. + - Ask: with one detached session, the launch shows its name, directory and shell, and the same evidence-based status as `/resume` (idle or what it runs, for how long, whether it waits for input), then offers Resume (R/Enter), Not now (N/Esc), Always resume (A) or Don't resume at startup (D). A and D also save the setting. - Never only skips restoring at launch. It never ends a session, and `/resume`, detach and reattach are unaffected. - **Multiple detached sessions:** Ask which (default) or Open all. - - The picker uses ↑↓ to move, Space to select, A to select all (A again clears), and Enter to resume the selected sessions. Esc, or Enter with nothing selected, starts fresh. - - Both screens have one destructive key, `X`, which only opens a confirmation (Enter/Y kills, Esc/N/Ctrl+C cancels). The kill is the same confirmed termination `/resume` uses; the transcript is archived and stays in `/resume`. Sessions attached elsewhere are never touched, and killing the last one starts a fresh session. `--new` still skips restoring, and `--attach ` attaches one session explicitly. + - Each row shows the session's name, directory, shell and status. The picker uses ↑↓ to move, Space to select, A to select all (A again clears), and Enter to resume the selected sessions, or the highlighted one when none is selected; the footer says which ("Enter resume this", "Enter resume 2 selected"). N or Esc starts a new session and leaves every listed one running. The screen redraws when the terminal is resized. + - Both screens have one destructive key, `X`, which only opens a confirmation (Enter/Y kills, Esc/N/Ctrl+C cancels). In the picker it targets the selected sessions, or the highlighted one when none is selected; the confirmation names how many and lists them, so A then X is Kill all. Each kill is the same confirmed termination `/resume` uses, performed one at a time; the transcript is archived and stays in `/resume`. Sessions attached elsewhere are never touched (a line says how many were left), and killing the last one starts a fresh session. Launching or quitting NMSh never ends a session by itself. `--new` still skips restoring, and `--attach ` attaches one session explicitly. - When several sessions are restored, this window attaches the first. Each other one opens in a new window of the host terminal, running `nmsh --attach `: - Ghostty on macOS: Ghostty's AppleScript API (`new surface configuration`, then `new window with configuration`). It opens a normal window in the running Ghostty app. The command words are passed as `osascript` arguments and shell-quoted with `quoted form of`, never written into the script. macOS asks once for Automation permission. If AppleScript is disabled or permission is denied, NMSh names the session's `nmsh --attach` command instead. - Ghostty on Linux: `ghostty -e`. diff --git a/src/session/StartupPicker.ts b/src/session/StartupPicker.ts index 3155a0a7..5fee6e56 100644 --- a/src/session/StartupPicker.ts +++ b/src/session/StartupPicker.ts @@ -1,6 +1,7 @@ import {KeyDecoder, type Key} from '../terminal/keys.js'; import type {SessionInfo} from './SessionProtocol.js'; import {formatAge, sessionText, tildePath} from './sessionList.js'; +import {liveStatusParts} from './liveStatus.js'; import {GLYPHS, checkMark} from '../ui/glyphs.js'; // Launch-time restore screens, shown before any session is attached. Each has @@ -9,8 +10,14 @@ import {GLYPHS, checkMark} from '../ui/glyphs.js'; // transcript is archived, never discarded. const clipTo = (columns: number) => (raw: string) => { const text = sessionText(raw); return text.length > columns - 1 ? `${text.slice(0, Math.max(0, columns - 2))}…` : text; }; -const activity = (session: SessionInfo) => (session.running === undefined ? undefined : sessionText(session.running.replace(/\s+/gu, ' ')).slice(0, 60)) ?? 'zsh'; -const age = (session: SessionInfo, now: number) => formatAge(now - (session.runningSince ?? session.idleSince ?? session.createdAt)); +const shellName = (session: SessionInfo) => session.shell ?? 'zsh'; +/** What the session is doing, from the service's evidence only: the first one or two facts /sessions shows. */ +const activity = (session: SessionInfo, now: number) => sessionText(liveStatusParts(session, now).slice(0, session.running ? 2 : 1).join(' · ')); +/** The one-session screens have room to say it in words: idle at the prompt (for how long), or what runs. */ +const spelledActivity = (session: SessionInfo, now: number) => session.running ? activity(session, now) + : `idle at the prompt${session.idleSince ? ` · ${formatAge(now - session.idleSince)}` : ''}`; +/** Its own name, when the person gave one or the service assigned a signature. */ +const identity = (session: SessionInfo) => sessionText(session.name ?? session.signature ?? ''); // ── One detached session ───────────────────────────────────────────────────── @@ -19,9 +26,10 @@ export type SinglePromptChoice = 'resume' | 'not-now' | 'always' | 'never' | 'ki export function renderSinglePrompt(session: SessionInfo, columns: number, now: number): string[] { const clip = clipTo(columns); + const name = identity(session); return ['NMSh · 1 detached live session', '', - clip(` ${tildePath(session.cwd)}`), - clip(` ${session.running ? `running ${activity(session)}` : 'idle at the prompt'} · ${age(session, now)}`), + clip(` ${name ? `${name} · ` : ''}${tildePath(session.cwd)} · ${shellName(session)}`), + clip(` ${spelledActivity(session, now)}`), clip(` started ${formatAge(now - session.createdAt)} ago`), '', ' R Resume', ' N Not now (start a new session; it keeps running)', ' X Kill session (ends the shell; transcript stays in /resume)', @@ -38,15 +46,24 @@ export function singlePromptKey(key: Key): SinglePromptChoice | undefined { // ── Kill confirmation (shared by both screens) ────────────────────────────── -export function renderKillConfirm(session: SessionInfo, columns: number, now: number): string[] { +export function renderKillConfirm(target: SessionInfo | readonly SessionInfo[], columns: number, now: number): string[] { const clip = clipTo(columns); - return ['Kill detached session?', '', - clip(` ${tildePath(session.cwd)}`), - clip(` ${session.running ? `running ${activity(session)}` : 'idle at the prompt'} · ${age(session, now)}`), '', - ' This ends the live shell.', ' Its transcript will remain available in /resume.', '', - clip('Enter/Y kill · Esc/N cancel')]; + const sessions = Array.isArray(target) ? target as readonly SessionInfo[] : [target as SessionInfo]; + const many = sessions.length > 1; + const shown = sessions.slice(0, KILL_LIST_LIMIT); + return [many ? `Kill ${sessions.length} detached sessions?` : 'Kill detached session?', '', + ...shown.flatMap(session => many + ? [clip(` ${tildePath(session.cwd)} · ${activity(session, now)}`)] + : [clip(` ${tildePath(session.cwd)} · ${shellName(session)}`), clip(` ${spelledActivity(session, now)}`)]), + ...(sessions.length > shown.length ? [` and ${sessions.length - shown.length} more`] : []), '', + many ? ` This ends ${sessions.length} live shells and whatever runs in them.` : ' This ends the live shell and whatever runs in it.', + ` ${many ? 'Their transcripts remain' : 'Its transcript will remain'} available in /resume. Sessions open in another window are not touched.`, '', + clip(`Enter/Y kill${many ? ` all ${sessions.length}` : ''} · Esc/N cancel`)]; } +/** Sessions named one per line in a kill confirmation; the rest are counted. */ +const KILL_LIST_LIMIT = 6; + /** Enter/Y confirm, Esc/N/Ctrl+C cancel; anything else is ignored. */ export function killConfirmKey(key: Key): 'confirm' | 'cancel' | undefined { if (key.kind === 'enter') return 'confirm'; @@ -62,14 +79,14 @@ export interface MultiPickerState { sessions: SessionInfo[]; cursor: number; selected: Set; - /** The session a kill confirmation is open for. */ - confirming?: string; + /** The sessions a kill confirmation is open for: the selected ones, or the highlighted one. */ + confirming?: string[]; /** One factual line about the last kill, shown under the list. */ message?: string; } /** Returned by multiPickerKey once a kill is confirmed; the caller performs it and re-enters the picker. */ -export interface KillRequest {kill: string} +export interface KillRequest {kill: string[]} export function createMultiPicker(sessions: SessionInfo[]): MultiPickerState { return {sessions, cursor: 0, selected: new Set()}; @@ -78,35 +95,45 @@ export function createMultiPicker(sessions: SessionInfo[]): MultiPickerState { export function renderMultiPicker(state: MultiPickerState, columns: number, now: number): string[] { const clip = clipTo(columns); const width = Math.min(32, Math.max(...state.sessions.map(session => tildePath(session.cwd).length), 4)); - const commandWidth = Math.min(20, Math.max(...state.sessions.map(session => activity(session).length), 3)); - const confirming = state.sessions.find(session => session.id === state.confirming); - if (confirming) return renderKillConfirm(confirming, columns, now); + const named = Math.min(16, Math.max(0, ...state.sessions.map(session => identity(session).length))); + const confirming = state.confirming ? state.sessions.filter(session => state.confirming!.includes(session.id)) : []; + if (confirming.length) return renderKillConfirm(confirming, columns, now); const rows = [`NMSh · ${state.sessions.length} detached live session${state.sessions.length === 1 ? '' : 's'}`, '']; state.sessions.forEach((session, index) => { const mark = checkMark(state.selected.has(session.id)); - rows.push(clip(`${index === state.cursor ? GLYPHS.selection : ' '} ${mark} ${tildePath(session.cwd).padEnd(width)} ${activity(session).padEnd(commandWidth)} ${age(session, now)}`)); + const name = named ? `${identity(session).slice(0, named).padEnd(named)} ` : ''; + rows.push(clip(`${index === state.cursor ? GLYPHS.selection : ' '} ${mark} ${name}${tildePath(session.cwd).padEnd(width)} ${shellName(session).padEnd(4)} ${activity(session, now)}`)); }); const count = state.selected.size; - rows.push('', clip(`↑↓ move · Space select · A all · X kill · Enter resume ${count === 0 ? 'none' : count} · Esc none`), - clip('Unselected sessions keep running and stay in /resume.'), ...(state.message ? [clip(state.message)] : [])); + // What Enter does is spelled out, and so is what X would end: the action under the cursor is never a guess. + const enter = count === 0 ? 'Enter resume this' : `Enter resume ${count} selected`; + const kill = count === 0 ? 'X kill this' : `X kill ${count} selected`; + rows.push('', clip(`${enter} · N new session · ↑↓ move · Space select · A all`), + clip(`${kill} · Esc new session · sessions you leave keep running (/resume)`), ...(state.message ? ['', clip(state.message)] : [])); return rows; } -/** Apply one key; returns the ids to resume (in list order) once the user is done, [] for none. */ +/** + * Apply one key; returns the ids to resume (in list order) once the user is done, [] for a new session. Enter resumes + * the selected sessions, or the highlighted one when none is selected; N and Esc start a new session and leave every + * listed one running. X asks to kill the selected sessions, or the highlighted one. + */ export function multiPickerKey(state: MultiPickerState, key: Key): string[] | KillRequest | undefined { const count = state.sessions.length; if (state.confirming) { const decision = killConfirmKey(key); - const id = state.confirming; + const ids = state.confirming; if (decision) state.confirming = undefined; - return decision === 'confirm' ? {kill: id} : undefined; + return decision === 'confirm' ? {kill: [...ids]} : undefined; } state.message = undefined; - if (key.kind === 'text' && key.value.toLowerCase() === 'x') state.confirming = state.sessions[state.cursor]?.id; + const highlighted = state.sessions[state.cursor]; + const chosen = () => state.selected.size ? state.sessions.filter(session => state.selected.has(session.id)).map(session => session.id) : highlighted ? [highlighted.id] : []; + if (key.kind === 'text' && key.value.toLowerCase() === 'x') { const ids = chosen(); if (ids.length) state.confirming = ids; } else if (key.kind === 'up') state.cursor = (state.cursor - 1 + count) % count; else if (key.kind === 'down') state.cursor = (state.cursor + 1) % count; - else if (key.kind === 'escape' || key.kind === 'interrupt') return []; - else if (key.kind === 'enter') return state.sessions.filter(session => state.selected.has(session.id)).map(session => session.id); + else if (key.kind === 'escape' || key.kind === 'interrupt' || (key.kind === 'text' && key.value.toLowerCase() === 'n')) return []; + else if (key.kind === 'enter') return chosen(); else if (key.kind === 'text' && key.value === ' ') { const id = state.sessions[state.cursor]!.id; if (!state.selected.delete(id)) state.selected.add(id); @@ -122,7 +149,7 @@ export function multiPickerKey(state: MultiPickerState, key: Key): string[] | Ki export function removeFromMultiPicker(state: MultiPickerState, id: string): void { state.sessions = state.sessions.filter(session => session.id !== id); state.selected.delete(id); - if (state.confirming === id) state.confirming = undefined; + if (state.confirming) { state.confirming = state.confirming.filter(item => item !== id); if (!state.confirming.length) state.confirming = undefined; } state.cursor = Math.max(0, Math.min(state.cursor, state.sessions.length - 1)); } @@ -144,18 +171,35 @@ export async function pickWithKill(sessions: SessionInfo[], io: { for (;;) { const result = await io.run(columns => renderMultiPicker(state, columns, now()), key => multiPickerKey(state, key)); if (Array.isArray(result)) return result; - const target = state.sessions.find(session => session.id === result.kill); - if (!target) continue; - let outcome: KillResult; - try { outcome = await io.kill(target); } - catch (error) { state.message = `Could not end that session: ${error instanceof Error ? error.message : String(error)}`; continue; } - removeFromMultiPicker(state, target.id); - state.message = outcome === 'killed' ? 'Session ended; its transcript is in /resume.' - : outcome === 'gone' ? 'That session had already ended; it is in /resume.' : 'That session is attached in another window and was not touched.'; + const targets = state.sessions.filter(session => result.kill.includes(session.id)); + if (!targets.length) continue; + // One at a time, each re-checked by the service: a session attached elsewhere meanwhile is left alone. + const tally = {killed: 0, gone: 0, attached: 0, failed: [] as string[]}; + for (const target of targets) { + let outcome: KillResult; + try { outcome = await io.kill(target); } + catch (error) { tally.failed.push(error instanceof Error ? error.message : String(error)); continue; } + tally[outcome] += 1; + if (outcome !== 'attached') removeFromMultiPicker(state, target.id); + else state.selected.delete(target.id); + } + state.message = killSummary(tally); if (state.sessions.length === 0) return []; } } +/** One factual line about what a kill did, session by session. */ +function killSummary(tally: {killed: number; gone: number; attached: number; failed: string[]}): string { + const parts: string[] = []; + const sessions = (count: number) => `${count} session${count === 1 ? '' : 's'}`; + if (tally.killed) parts.push(tally.killed === 1 && !tally.gone && !tally.attached && !tally.failed.length ? 'Session ended; its transcript is in /resume.' : `${sessions(tally.killed)} ended`); + if (tally.gone) parts.push(tally.gone === 1 && parts.length === 0 && !tally.attached && !tally.failed.length ? 'That session had already ended; it is in /resume.' : `${sessions(tally.gone)} had already ended`); + if (tally.attached) parts.push(tally.attached === 1 && parts.length === 0 && !tally.failed.length ? 'That session is attached in another window and was not touched.' : `${sessions(tally.attached)} attached in another window ${tally.attached === 1 ? 'was' : 'were'} not touched`); + if (tally.failed.length) parts.push(tally.failed.length === 1 && parts.length === 0 ? `Could not end that session: ${tally.failed[0]}` : `${sessions(tally.failed.length)} could not be ended (${tally.failed[0]})`); + const line = parts.join('; '); + return /[.)]$/u.test(line) ? line : `${line}; transcripts stay in /resume.`; +} + /** The one-session prompt with in-place kill; confirmation cancels back to the prompt. */ export async function askWithKill(session: SessionInfo, io: { run: (render: (columns: number) => string[], onKey: (key: Key) => T | undefined) => Promise; @@ -194,6 +238,7 @@ export function runStartupScreen(render: (columns: number) => string[], onKey const result = onKey(key); if (result !== undefined) { process.stdin.off('data', onData); + process.stdout.off('resize', onResize); process.stdin.setRawMode(wasRaw); process.stdin.pause(); process.stdout.write('\u001b[?25h\u001b[?1049l'); @@ -203,6 +248,9 @@ export function runStartupScreen(render: (columns: number) => string[], onKey } draw(); }; + // A resize redraws at the new width (narrow terminals keep every line clipped, never wrapped). + const onResize = () => draw(); + process.stdout.on('resize', onResize); process.stdin.on('data', onData); process.stdin.resume(); draw(); diff --git a/tests/startupRestore.test.ts b/tests/startupRestore.test.ts index 179bd04b..7ae19ab7 100644 --- a/tests/startupRestore.test.ts +++ b/tests/startupRestore.test.ts @@ -59,7 +59,7 @@ test('one-session prompt: identifying details, and R/Enter, N/Esc, A, D, and X t assert.equal(singlePromptKey(key('down')), undefined); }); -test('multi picker: move, Space toggles, A selects all then clears, Enter resumes selected in order, Esc none', () => { +test('multi picker: move, Space toggles, A selects all then clears, Enter resumes selected in order or else the highlighted one; N and Esc start new', () => { const sessions = [info('s1', 'detached'), info('s2', 'detached', {running: 'npm test'}), info('s3', 'detached')]; const state = createMultiPicker(sessions); assert.match(renderMultiPicker(state, 100, 61_000).join('\n'), /3 detached live sessions[\s\S]*› · \/w\/s1[\s\S]*npm test/); @@ -77,9 +77,11 @@ test('multi picker: move, Space toggles, A selects all then clears, Enter resume assert.equal(state.selected.size, 3); multiPickerKey(state, text('A')); assert.equal(state.selected.size, 0, 'A again clears everything'); - assert.deepEqual(multiPickerKey(state, key('enter')), [], 'Enter with nothing selected resumes none'); + assert.deepEqual(multiPickerKey(state, key('enter')), ['s1'], 'Enter with nothing selected resumes the highlighted session, as the footer says'); + assert.match(renderMultiPicker(state, 100, 61_000).join('\n'), /Enter resume this · N new session/); multiPickerKey(state, text('a')); - assert.deepEqual(multiPickerKey(state, key('escape')), [], 'Esc resumes none even with a selection'); + assert.deepEqual(multiPickerKey(state, key('escape')), [], 'Esc starts a new session even with a selection'); + assert.deepEqual(multiPickerKey(state, text('n')), [], 'N starts a new session; every listed one keeps running'); assert.equal(multiPickerKey(state, text('k')), undefined, 'K is not a key'); }); @@ -207,7 +209,7 @@ test('multi picker: X only opens a confirmation; Esc, N and Ctrl+C cancel; Enter for (const cancel of [key('escape'), text('n'), text('N'), key('interrupt')]) { const state = createMultiPicker(three()); assert.equal(multiPickerKey(state, text('x')), undefined, 'X alone kills nothing'); - assert.equal(state.confirming, 'a'); + assert.deepEqual(state.confirming, ['a'], 'nothing selected: the highlighted session'); assert.match(renderMultiPicker(state, 80, 5_000).join('\n'), /Kill detached session\?[\s\S]*remain available in \/resume[\s\S]*Enter\/Y kill · Esc\/N cancel/); assert.equal(multiPickerKey(state, cancel), undefined); assert.equal(state.confirming, undefined); assert.equal(state.sessions.length, 3); @@ -215,14 +217,14 @@ test('multi picker: X only opens a confirmation; Esc, N and Ctrl+C cancel; Enter for (const confirm of [key('enter'), text('y'), text('Y')]) { const state = createMultiPicker(three()); state.cursor = 1; multiPickerKey(state, text('X')); - assert.deepEqual(multiPickerKey(state, confirm), {kill: 'b'}); + assert.deepEqual(multiPickerKey(state, confirm), {kill: ['b']}); } const state = createMultiPicker(three()); multiPickerKey(state, text('x')); assert.equal(multiPickerKey(state, text(' ')), undefined, 'Space is inert while confirming'); assert.equal(multiPickerKey(state, text('a')), undefined, 'A is inert while confirming'); assert.equal(state.selected.size, 0); - assert.match(renderMultiPicker(createMultiPicker(three()), 100, 0).join('\n'), /↑↓ move · Space select · A all · X kill · Enter resume none · Esc none/); + assert.match(renderMultiPicker(createMultiPicker(three()), 100, 0).join('\n'), /Enter resume this · N new session · ↑↓ move · Space select · A all[\s\S]*X kill this · Esc new session/); assert.equal(killConfirmKey(text('k')), undefined); }); @@ -246,7 +248,8 @@ test('pickWithKill: kills through the injected path, drops selection, and a fina throw new Error('script ended without a result'); }; const ids = await pickWithKill(three(), {run, kill: async session => { killed.push(session.id); return 'killed'; }, now: () => 5_000}); - assert.deepEqual(killed, ['b', 'c', 'a'], 'every kill went through the injected kill, in order'); + // The first X has a selection (a), so it kills the selection, not the highlighted b; then the highlighted ones. + assert.deepEqual(killed, ['a', 'c', 'b'], 'every kill went through the injected kill, in order'); assert.deepEqual(ids, [], 'no sessions left: resolves with none so startup continues fresh'); assert.ok(screens.some(screen => /Session ended; its transcript is in \/resume/.test(screen))); }); @@ -255,7 +258,7 @@ test('pickWithKill: already-gone and attached-elsewhere sessions refresh safely; const outcomes: KillResult[] = ['gone', 'attached']; let call = 0; const run = async (_render: unknown, onKey: (key: Key) => T | undefined): Promise => { - const keys = call === 0 ? [text('x'), text('y')] : call === 1 ? [text('x'), key('enter')] : call === 2 ? [text('x'), text('y')] : [key('enter')]; + const keys = call === 0 ? [text('x'), text('y')] : call === 1 ? [text('x'), key('enter')] : call === 2 ? [text('x'), text('y')] : [text('n')]; call += 1; for (const k of keys) { const result = onKey(k); if (result !== undefined) return result; } throw new Error('no result'); @@ -268,7 +271,7 @@ test('pickWithKill: already-gone and attached-elsewhere sessions refresh safely; throw new Error('permission denied'); }}); assert.equal(attempts, 3); - assert.deepEqual(ids, [], 'Enter with nothing selected; the failed row was kept, not lost'); + assert.deepEqual(ids, [], 'N: a new session; the failed row was kept, not lost'); }); test('one-session prompt: X confirms before killing; cancel returns to the prompt; Enter is still Resume', async () => { @@ -290,3 +293,54 @@ test('restoreAtStartup: a killed single session starts fresh and attaches nothin askOne: async () => 'kill', pick: async () => assert.fail('not used'), host: detectTerminalHost({}), selfCommand: ['nmsh']}; assert.deepEqual(await restoreAtStartup([info('a', 'detached')], deps), {}); }); + +test('multi picker: X with a selection asks once for all of them, naming the count; A then X is Kill all', async () => { + const state = createMultiPicker(three()); + multiPickerKey(state, text(' ')); + multiPickerKey(state, key('down')); + multiPickerKey(state, key('down')); + multiPickerKey(state, text(' ')); + assert.match(renderMultiPicker(state, 100, 0).join('\n'), /Enter resume 2 selected[\s\S]*X kill 2 selected/); + multiPickerKey(state, text('x')); + assert.deepEqual(state.confirming, ['a', 'c'], 'the selected sessions, never the highlighted one alone'); + const screen = renderMultiPicker(state, 100, 0).join('\n'); + assert.match(screen, /Kill 2 detached sessions\?/); + assert.match(screen, /\/w\/a[\s\S]*\/w\/c/); + assert.match(screen, /ends 2 live shells[\s\S]*not touched[\s\S]*Enter\/Y kill all 2 · Esc\/N cancel/); + assert.deepEqual(multiPickerKey(state, text('y')), {kill: ['a', 'c']}); + const all = createMultiPicker(three()); + multiPickerKey(all, text('a')); + multiPickerKey(all, text('x')); + assert.match(renderMultiPicker(all, 100, 0).join('\n'), /Kill 3 detached sessions\?[\s\S]*Enter\/Y kill all 3/); + assert.deepEqual(multiPickerKey(all, key('enter')), {kill: ['a', 'b', 'c']}); + // Performed one at a time; an attached-elsewhere one stays listed, and the line says what happened to each. + const outcomes: KillResult[] = ['killed', 'attached', 'killed']; + const screens: string[] = []; + let call = 0; + const run = async (render: (columns: number) => string[], onKey: (key: Key) => T | undefined): Promise => { + const keys = call === 0 ? [text('a'), text('x'), text('y')] : [text('n')]; + call += 1; + for (const k of keys) { screens.push(render(100).join('\n')); const result = onKey(k); if (result !== undefined) return result; } + screens.push(render(100).join('\n')); + throw new Error('no result'); + }; + const killed: string[] = []; + const ids = await pickWithKill(three(), {run, now: () => 0, kill: async session => { killed.push(session.id); return outcomes.shift()!; }}); + assert.deepEqual(killed, ['a', 'b', 'c']); + assert.deepEqual(ids, []); + assert.ok(screens.some(screen => /2 sessions ended; 1 session attached in another window was not touched/.test(screen)), screens.at(-1)); + assert.ok(screens.some(screen => /\/w\/b/.test(screen) && /1 detached live session\b/.test(screen)), 'the attached one is still listed'); +}); + +test('picker rows name the shell and say what the service saw: idle, running, waiting for input; never a guessed zsh', () => { + const sessions = [info('a', 'detached', {shell: 'fish', idleSince: 1, lastExit: 0, signature: 'Lemon'}), + info('b', 'detached', {shell: 'bash', running: 'read -r name', runningSince: 0, inputSince: 0, inputConfidence: 'likely'}), + info('c', 'detached', {running: 'npm test', runningSince: 0, lastOutputAt: 59_000})]; + const screen = renderMultiPicker(createMultiPicker(sessions), 120, 60_001).join('\n'); + assert.match(screen, /Lemon\s+\/w\/a\s+fish\s+idle 1m/); + assert.match(screen, /\/w\/b\s+bash\s+running read -r name · 1m · probably waiting for input 1m/); + assert.match(screen, /\/w\/c\s+zsh\s+running npm test · 1m · active/); + const single = renderSinglePrompt(sessions[0]!, 100, 60_001).join('\n'); + assert.match(single, /Lemon · \/w\/a · fish/); + assert.match(single, /idle at the prompt · 1m/); +}); From b63cc3b12ba3ede5f2226ba51e6164a8c555d5ec Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Wed, 11 Nov 2026 22:18:08 +0530 Subject: [PATCH 2/2] Startup kill: report only what the service proved; exact targets The one-session prompt treated a kill the service refused (attached in another window) as done and swallowed failures; it now returns to the prompt with the reason. Ending a session goes through one function, endDetachedSession: an unreachable service is a failure (it used to read as "already ended"), a refusal because a window attached meanwhile is "not touched", and a shell this window ended whose archive failed is reported as ended but not archived yet, with the spool kept. The service refuses to attach a session between its confirmed kill and the shell's exit. Bulk confirmations list every target by name, and the outcome line puts failures first. --- CHANGELOG.md | 2 +- docs/design/session-interaction-ux.md | 2 +- src/index.ts | 18 +-- src/session/SessionService.ts | 6 + src/session/StartupPicker.ts | 62 +++++--- src/session/liveSessions.ts | 48 +++++- tests/startupKillSafety.test.ts | 215 ++++++++++++++++++++++++++ tests/startupRestore.test.ts | 2 +- 8 files changed, 317 insertions(+), 38 deletions(-) create mode 100644 tests/startupKillSafety.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fa3b9a2..246b0875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed -- **Startup session picker** ([design](docs/design/session-interaction-ux.md)). With several detached sessions, Enter now resumes the highlighted session when none is selected (it used to start a new one), N or Esc starts a new session, and the footer says what Enter and X will act on. X kills the selected sessions, or the highlighted one: the confirmation names the count and lists them, so A then X is Kill all; sessions attached in another window are still never touched, and a line reports what happened to each. Rows and the one-session prompt show the session's name and shell and the same evidence-based status as `/resume` (idle, what runs, waiting for input) instead of always saying `zsh`, and the screen redraws on resize. +- **Startup session picker** ([design](docs/design/session-interaction-ux.md)). With several detached sessions, Enter now resumes the highlighted session when none is selected (it used to start a new one), N or Esc starts a new session, and the footer says what Enter and X will act on. X kills the selected sessions, or the highlighted one: the confirmation names the count and lists them, so A then X is Kill all; every target is listed by name (never "and N more"); sessions attached in another window, or attached while the confirmation was open, are never touched, and a line reports what happened to each, failures first. A service that cannot be reached is reported as a failure, never as "already ended"; a session this window ended whose transcript could not be archived yet says so (the spool is kept and the next launch archives it); and a session being ended can no longer be attached by another window in the moment before its shell exits. In the one-session prompt, a kill the service refused or could not perform returns to the prompt with the reason instead of starting fresh as if it had worked. Rows and the one-session prompt show the session's name and shell and the same evidence-based status as `/resume` (idle, what runs, waiting for input) instead of always saying `zsh`, and the screen redraws on resize. ## [0.23.0] - 2026-10-21 — Vespyr Arcade diff --git a/docs/design/session-interaction-ux.md b/docs/design/session-interaction-ux.md index eb580d33..194b423d 100644 --- a/docs/design/session-interaction-ux.md +++ b/docs/design/session-interaction-ux.md @@ -139,7 +139,7 @@ Live sessions are owned by the per-user session service (`nmshd`), which listens - Never only skips restoring at launch. It never ends a session, and `/resume`, detach and reattach are unaffected. - **Multiple detached sessions:** Ask which (default) or Open all. - Each row shows the session's name, directory, shell and status. The picker uses ↑↓ to move, Space to select, A to select all (A again clears), and Enter to resume the selected sessions, or the highlighted one when none is selected; the footer says which ("Enter resume this", "Enter resume 2 selected"). N or Esc starts a new session and leaves every listed one running. The screen redraws when the terminal is resized. - - Both screens have one destructive key, `X`, which only opens a confirmation (Enter/Y kills, Esc/N/Ctrl+C cancels). In the picker it targets the selected sessions, or the highlighted one when none is selected; the confirmation names how many and lists them, so A then X is Kill all. Each kill is the same confirmed termination `/resume` uses, performed one at a time; the transcript is archived and stays in `/resume`. Sessions attached elsewhere are never touched (a line says how many were left), and killing the last one starts a fresh session. Launching or quitting NMSh never ends a session by itself. `--new` still skips restoring, and `--attach ` attaches one session explicitly. + - Both screens have one destructive key, `X`, which only opens a confirmation (Enter/Y kills, Esc/N/Ctrl+C cancels). In the picker it targets the selected sessions, or the highlighted one when none is selected; the confirmation names how many and lists them, so A then X is Kill all. Every target is listed by name and nothing else is ended. Each kill is the same confirmed termination `/resume` uses, performed one at a time and addressed by the service's session id (never a PID): the service's list is read again first, and the service itself refuses the kill if a window holds the session at that moment. The transcript is archived and stays in `/resume`; if archiving fails after the shell ended, the line says so and the spool is kept for the next launch. Once a kill is sent the service refuses to attach that session until its shell exits. An unreachable service is reported as a failure, never as "already ended". Sessions attached elsewhere are never touched (a line says how many were left), and killing the last one starts a fresh session. Launching or quitting NMSh never ends a session by itself. `--new` still skips restoring, and `--attach ` attaches one session explicitly. - When several sessions are restored, this window attaches the first. Each other one opens in a new window of the host terminal, running `nmsh --attach `: - Ghostty on macOS: Ghostty's AppleScript API (`new surface configuration`, then `new window with configuration`). It opens a normal window in the running Ghostty app. The command words are passed as `osascript` arguments and shell-quoted with `quoted form of`, never written into the script. macOS asks once for Automation permission. If AppleScript is disabled or permission is denied, NMSh names the session's `nmsh --attach` command instead. - Ghostty on Linux: `ghostty -e`. diff --git a/src/index.ts b/src/index.ts index 244c5c9b..cf4b42da 100644 --- a/src/index.ts +++ b/src/index.ts @@ -174,16 +174,14 @@ if (args[0] === 'packs') { const config = loadPromptConfiguration(); // Same confirmed termination as /resume. Service truth is re-read first: a session that ended // meanwhile is simply gone, and one attached elsewhere is never touched. - const killDetached = async (session: (typeof live)[number]): Promise<'killed' | 'gone' | 'attached'> => { - const [{killAndArchive}, {TranscriptStore}] = await Promise.all([import('./session/liveSessions.js'), import('./sessions/TranscriptStore.js')]); - const current = (await listLiveSessions().catch(() => [])).find(item => item.id === session.id); - if (!current) return 'gone'; - if (current.state !== 'detached') return 'attached'; - try { await killAndArchive(current, {store: new TranscriptStore(), origin: 'the startup picker'}); return 'killed'; } - catch (error) { - if (!(await listLiveSessions().catch(() => [current])).some(item => item.id === session.id)) return 'gone'; - throw error; - } + // An unreachable service is a failure, never "already ended"; only a shell this call ended counts as killed. + const killDetached = async (session: (typeof live)[number]) => { + const [{endDetachedSession, archiveKilled}, {killSession}, {defaultRuntimeDir, socketPathFor}, {TranscriptStore}] = await Promise.all([ + import('./session/liveSessions.js'), import('./session/SocketSessionClient.js'), import('./session/runtimeDir.js'), import('./sessions/TranscriptStore.js')]); + const store = new TranscriptStore(); + return endDetachedSession(session, {list: () => listLiveSessions(), + kill: current => killSession(socketPathFor(defaultRuntimeDir()), current.id), + archive: current => archiveKilled(current, {store, origin: 'the startup picker'})}); }; const restored = await restoreAtStartup(live, { // After a failed return, never silently attach a different session: ask instead of Always. diff --git a/src/session/SessionService.ts b/src/session/SessionService.ts index 4c7b459d..3d382249 100644 --- a/src/session/SessionService.ts +++ b/src/session/SessionService.ts @@ -72,6 +72,8 @@ interface ManagedSession { tasks: SessionTasks; /** The attached frontend's task listener, when it listed 'background-tasks'. */ taskListener?: TaskListener; + /** A confirmed kill was sent and the shell has not reported its exit yet: nothing may attach to it now. */ + ending?: true; } export {AlternateScreenTracker} from './TerminalModes.js'; @@ -247,6 +249,7 @@ export class SessionService { const session = this.sessions.get(message.sessionId); if (!session) { send({type: 'error', code: 'unknown', message: 'no live session with that id'}); break; } if (session.controller) { send({type: 'error', code: 'attached', message: 'session is attached to another frontend'}); break; } + if (session.ending) { send({type: 'error', code: 'unknown', message: 'that session is ending'}); break; } owned = session; this.bind(session, send, features); // Opening a session is focusing it: its notice is no longer news in any window. @@ -369,6 +372,9 @@ export class SessionService { // Never pull a shell out from under a frontend that is using it. if (target.controller) { send({type: 'error', code: 'attached', message: 'session is attached to another frontend'}); break; } target.shell.once('exit', () => send({type: 'killed', sessionId: message.sessionId})); + // Between the kill and the shell's exit event no window may attach; a second kill just waits for the same exit. + if (target.ending) break; + target.ending = true; target.shell.kill(); break; } diff --git a/src/session/StartupPicker.ts b/src/session/StartupPicker.ts index 5fee6e56..56fd5d50 100644 --- a/src/session/StartupPicker.ts +++ b/src/session/StartupPicker.ts @@ -2,6 +2,7 @@ import {KeyDecoder, type Key} from '../terminal/keys.js'; import type {SessionInfo} from './SessionProtocol.js'; import {formatAge, sessionText, tildePath} from './sessionList.js'; import {liveStatusParts} from './liveStatus.js'; +import type {EndOutcome} from './liveSessions.js'; import {GLYPHS, checkMark} from '../ui/glyphs.js'; // Launch-time restore screens, shown before any session is attached. Each has @@ -24,7 +25,7 @@ const identity = (session: SessionInfo) => sessionText(session.name ?? session.s /** resume: attach · not-now: fresh session, this launch only · always/never: also persist the startup setting. */ export type SinglePromptChoice = 'resume' | 'not-now' | 'always' | 'never' | 'kill'; -export function renderSinglePrompt(session: SessionInfo, columns: number, now: number): string[] { +export function renderSinglePrompt(session: SessionInfo, columns: number, now: number, message?: string): string[] { const clip = clipTo(columns); const name = identity(session); return ['NMSh · 1 detached live session', '', @@ -34,7 +35,7 @@ export function renderSinglePrompt(session: SessionInfo, columns: number, now: n ' R Resume', ' N Not now (start a new session; it keeps running)', ' X Kill session (ends the shell; transcript stays in /resume)', ' A Always resume', ' D Don\'t resume at startup (it keeps running)', '', - clip('Enter resume · Esc not now · sessions stay in /resume')]; + clip('Enter resume · Esc not now · sessions stay in /resume'), ...(message ? ['', clip(message)] : [])]; } export function singlePromptKey(key: Key): SinglePromptChoice | undefined { @@ -50,20 +51,18 @@ export function renderKillConfirm(target: SessionInfo | readonly SessionInfo[], const clip = clipTo(columns); const sessions = Array.isArray(target) ? target as readonly SessionInfo[] : [target as SessionInfo]; const many = sessions.length > 1; - const shown = sessions.slice(0, KILL_LIST_LIMIT); + // Every target is named on its own line (the service caps live sessions, so the list is bounded): the confirmation + // is the exact set that will be ended, never "and N more", and two sessions in one directory stay distinguishable. + const named = (session: SessionInfo) => { const name = identity(session); return name ? `${name} · ` : ''; }; return [many ? `Kill ${sessions.length} detached sessions?` : 'Kill detached session?', '', - ...shown.flatMap(session => many - ? [clip(` ${tildePath(session.cwd)} · ${activity(session, now)}`)] - : [clip(` ${tildePath(session.cwd)} · ${shellName(session)}`), clip(` ${spelledActivity(session, now)}`)]), - ...(sessions.length > shown.length ? [` and ${sessions.length - shown.length} more`] : []), '', + ...sessions.flatMap(session => many + ? [clip(` ${named(session)}${tildePath(session.cwd)} · ${shellName(session)} · ${activity(session, now)}`)] + : [clip(` ${named(session)}${tildePath(session.cwd)} · ${shellName(session)}`), clip(` ${spelledActivity(session, now)}`)]), '', many ? ` This ends ${sessions.length} live shells and whatever runs in them.` : ' This ends the live shell and whatever runs in it.', ` ${many ? 'Their transcripts remain' : 'Its transcript will remain'} available in /resume. Sessions open in another window are not touched.`, '', clip(`Enter/Y kill${many ? ` all ${sessions.length}` : ''} · Esc/N cancel`)]; } -/** Sessions named one per line in a kill confirmation; the rest are counted. */ -const KILL_LIST_LIMIT = 6; - /** Enter/Y confirm, Esc/N/Ctrl+C cancel; anything else is ignored. */ export function killConfirmKey(key: Key): 'confirm' | 'cancel' | undefined { if (key.kind === 'enter') return 'confirm'; @@ -153,7 +152,7 @@ export function removeFromMultiPicker(state: MultiPickerState, id: string): void state.cursor = Math.max(0, Math.min(state.cursor, state.sessions.length - 1)); } -export type KillResult = 'killed' | 'gone' | 'attached'; +export type KillResult = EndOutcome; /** * The multi-session screen with in-place kill. `run` shows one screen until @@ -174,7 +173,7 @@ export async function pickWithKill(sessions: SessionInfo[], io: { const targets = state.sessions.filter(session => result.kill.includes(session.id)); if (!targets.length) continue; // One at a time, each re-checked by the service: a session attached elsewhere meanwhile is left alone. - const tally = {killed: 0, gone: 0, attached: 0, failed: [] as string[]}; + const tally: KillTally = {killed: 0, 'killed-unarchived': 0, gone: 0, attached: 0, failed: []}; for (const target of targets) { let outcome: KillResult; try { outcome = await io.kill(target); } @@ -188,16 +187,27 @@ export async function pickWithKill(sessions: SessionInfo[], io: { } } -/** One factual line about what a kill did, session by session. */ -function killSummary(tally: {killed: number; gone: number; attached: number; failed: string[]}): string { - const parts: string[] = []; +type KillTally = Record & {failed: string[]}; + +/** One factual line about what a kill did, session by session; what failed comes first, so a narrow screen never clips it. */ +function killSummary(tally: KillTally): string { const sessions = (count: number) => `${count} session${count === 1 ? '' : 's'}`; - if (tally.killed) parts.push(tally.killed === 1 && !tally.gone && !tally.attached && !tally.failed.length ? 'Session ended; its transcript is in /resume.' : `${sessions(tally.killed)} ended`); - if (tally.gone) parts.push(tally.gone === 1 && parts.length === 0 && !tally.attached && !tally.failed.length ? 'That session had already ended; it is in /resume.' : `${sessions(tally.gone)} had already ended`); - if (tally.attached) parts.push(tally.attached === 1 && parts.length === 0 && !tally.failed.length ? 'That session is attached in another window and was not touched.' : `${sessions(tally.attached)} attached in another window ${tally.attached === 1 ? 'was' : 'were'} not touched`); - if (tally.failed.length) parts.push(tally.failed.length === 1 && parts.length === 0 ? `Could not end that session: ${tally.failed[0]}` : `${sessions(tally.failed.length)} could not be ended (${tally.failed[0]})`); - const line = parts.join('; '); - return /[.)]$/u.test(line) ? line : `${line}; transcripts stay in /resume.`; + const unarchived = tally['killed-unarchived']; + const kinds = [tally.failed.length, tally.attached, tally.killed, unarchived, tally.gone].filter(Boolean).length; + if (kinds === 1) { + const one = (count: number) => count === 1; + if (one(tally.failed.length)) return `Could not end that session: ${tally.failed[0]}`; + if (one(tally.attached)) return 'That session is attached in another window and was not touched.'; + if (one(tally.killed)) return 'Session ended; its transcript is in /resume.'; + if (one(tally.gone)) return 'That session had already ended; it is in /resume.'; + } + const parts: string[] = []; + if (tally.failed.length) parts.push(`${sessions(tally.failed.length)} could not be ended (${tally.failed[0]})`); + if (tally.attached) parts.push(`${sessions(tally.attached)} attached in another window ${tally.attached === 1 ? 'was' : 'were'} not touched`); + if (tally.killed) parts.push(`${sessions(tally.killed)} ended`); + if (unarchived) parts.push(`${sessions(unarchived)} ended but ${unarchived === 1 ? 'its transcript was' : 'their transcripts were'} not archived yet (kept; the next launch archives ${unarchived === 1 ? 'it' : 'them'})`); + if (tally.gone) parts.push(`${sessions(tally.gone)} had already ended`); + return `${parts.join('; ')}${tally.killed || tally.gone ? '; transcripts stay in /resume.' : '.'}`; } /** The one-session prompt with in-place kill; confirmation cancels back to the prompt. */ @@ -207,12 +217,18 @@ export async function askWithKill(session: SessionInfo, io: { now?: () => number; }): Promise { const now = io.now ?? Date.now; + let message: string | undefined; for (;;) { - const choice = await io.run(columns => renderSinglePrompt(session, columns, now()), singlePromptKey); + const choice = await io.run(columns => renderSinglePrompt(session, columns, now(), message), singlePromptKey); + message = undefined; if (choice !== 'kill') return choice; const decision = await io.run<'confirm' | 'cancel'>(columns => renderKillConfirm(session, columns, now()), killConfirmKey); if (decision === 'cancel') continue; - try { await io.kill(session); } catch { continue; } + // Only an ended session lets startup go on fresh; anything else is said on the prompt, never treated as killed. + let outcome: KillResult; + try { outcome = await io.kill(session); } + catch (error) { message = `Could not end that session: ${error instanceof Error ? error.message : String(error)}`; continue; } + if (outcome === 'attached') { message = 'That session is attached in another window and was not touched.'; continue; } return 'kill'; } } diff --git a/src/session/liveSessions.ts b/src/session/liveSessions.ts index 9b8b2507..e009ad32 100644 --- a/src/session/liveSessions.ts +++ b/src/session/liveSessions.ts @@ -45,11 +45,55 @@ export interface KillOptions { export async function killAndArchive(session: SessionInfo, options: KillOptions = {}): Promise { const runtimeDir = options.runtimeDir ?? defaultRuntimeDir(options.env ?? process.env); await killSession(socketPathFor(runtimeDir), session.id); - // Same ownership boundary as launch recovery, which may be finalizing this - // session concurrently now that the service no longer lists it. + return archiveKilled(session, {...options, runtimeDir}); +} + +/** + * Archive a session whose shell this process just ended. Same ownership boundary as launch recovery, which may be + * finalizing it concurrently now that the service no longer lists it. On failure the spool is kept for a later launch. + */ +export async function archiveKilled(session: SessionInfo, options: KillOptions = {}): Promise { + const runtimeDir = options.runtimeDir ?? defaultRuntimeDir(options.env ?? process.env); const outcome = await finalizeLiveSession({store: options.store ?? new TranscriptStore(), runtimeDir, sessionId: session.id, ...(session.journalId ? {journalId: session.journalId} : {}), cwd: session.cwd, waitMs: 10_000, note: () => `Session killed from ${options.origin ?? '/resume'}; its shell has ended.`}); if (outcome.kind === 'claimed-elsewhere') throw new Error('Another NMSh window is archiving this session.'); return outcome.session; } + +/** + * What ending one listed detached session did, from the service's own answers only: + * - killed: this call ended its shell and the transcript was archived; + * - killed-unarchived: this call ended its shell; the transcript stays in the spool and a later launch archives it; + * - gone: the service no longer had it before this call could end it; + * - attached: it is attached in a window (or became attached), and was not touched. + */ +export type EndOutcome = 'killed' | 'killed-unarchived' | 'gone' | 'attached'; + +export interface EndDetachedDeps { + /** The service's current list. Throws when the service cannot be asked: that is never "already ended". */ + list: () => Promise; + /** Ask the service to end the session by id; it refuses ('attached') if a window holds it at that moment. */ + kill: (session: SessionInfo) => Promise; + archive: (session: SessionInfo) => Promise; +} + +/** + * End one session the person confirmed. Targets are session ids the service issued (never a PID); the service's list + * is read again first, and the service itself re-checks attachment when it receives the kill. Anything this cannot + * prove is reported as it is: a failure throws, and only a shell this call ended counts as killed. + */ +export async function endDetachedSession(session: SessionInfo, deps: EndDetachedDeps): Promise { + const current = (await deps.list()).find(item => item.id === session.id); + if (!current) return 'gone'; + if (current.state !== 'detached') return 'attached'; + try { await deps.kill(current); } + catch (error) { + const code = (error as {code?: string}).code; + if (code === 'attached') return 'attached'; + if (code === 'unknown') return 'gone'; + throw error; + } + try { await deps.archive(current); return 'killed'; } + catch { return 'killed-unarchived'; } +} diff --git a/tests/startupKillSafety.test.ts b/tests/startupKillSafety.test.ts new file mode 100644 index 00000000..aff27c89 --- /dev/null +++ b/tests/startupKillSafety.test.ts @@ -0,0 +1,215 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import {chmodSync, mkdtempSync, realpathSync, rmSync} from 'node:fs'; +import {connect, type Socket} from 'node:net'; +import {tmpdir} from 'node:os'; +import {join} from 'node:path'; +import {endDetachedSession, type EndDetachedDeps} from '../src/session/liveSessions.js'; +import {askWithKill, createMultiPicker, multiPickerKey, pickWithKill, renderMultiPicker, type KillResult} from '../src/session/StartupPicker.js'; +import {SessionConnectError} from '../src/session/SocketSessionClient.js'; +import {SessionService} from '../src/session/SessionService.js'; +import {FrameDecoder, PROTOCOL_VERSION, encodeMessage, type ServerMessage, type SessionInfo} from '../src/session/SessionProtocol.js'; +import type {Key} from '../src/terminal/keys.js'; + +const info = (id: string, state: SessionInfo['state'], extra: Partial = {}): SessionInfo => + ({id, pid: 100, state, cwd: `/w/${id}`, createdAt: 1_000, ...extra}); +const text = (value: string): Key => ({kind: 'text', value}); +const key = (kind: string): Key => ({kind} as Key); + +/** A scripted service: what it lists, how a kill answers, and every call made to it. */ +function fakeService(listed: SessionInfo[] | Error, kill: (session: SessionInfo) => Promise = async () => {}, archive: () => Promise = async () => ({})) { + const calls: string[] = []; + const deps: EndDetachedDeps = { + list: async () => { calls.push('list'); if (listed instanceof Error) throw listed; return listed; }, + kill: async session => { calls.push(`kill ${session.id}`); await kill(session); }, + archive: async () => { calls.push('archive'); return archive(); }, + }; + return {deps, calls}; +} + +test('ending a session: only a shell this call ended counts as killed; every other answer is reported as it is', async () => { + // The service cannot be asked: a failure, never "already ended". + const unreachable = fakeService(new SessionConnectError('session service did not respond', 'timeout')); + await assert.rejects(endDetachedSession(info('a', 'detached'), unreachable.deps), /did not respond/); + assert.deepEqual(unreachable.calls, ['list'], 'nothing is killed when the service could not be asked'); + + // No longer listed: gone, and nothing is sent. + const gone = fakeService([info('b', 'detached')]); + assert.equal(await endDetachedSession(info('a', 'detached'), gone.deps), 'gone'); + assert.deepEqual(gone.calls, ['list']); + + // Attached since the picker opened: never touched. + const attached = fakeService([info('a', 'attached')]); + assert.equal(await endDetachedSession(info('a', 'detached'), attached.deps), 'attached'); + assert.deepEqual(attached.calls, ['list']); + + // Attached between the re-read and the kill: the service refuses, and that is "attached", not a failure or a kill. + const raced = fakeService([info('a', 'detached')], async () => { throw new SessionConnectError('session service refused: session is attached to another frontend', 'attached'); }); + assert.equal(await endDetachedSession(info('a', 'detached'), raced.deps), 'attached'); + assert.deepEqual(raced.calls, ['list', 'kill a']); + + // Ended between the re-read and the kill. + const ended = fakeService([info('a', 'detached')], async () => { throw new SessionConnectError('session service refused: no live session with that id', 'unknown'); }); + assert.equal(await endDetachedSession(info('a', 'detached'), ended.deps), 'gone'); + + // Any other kill failure propagates; the archive is never attempted. + const timeout = fakeService([info('a', 'detached')], async () => { throw new SessionConnectError('session service did not respond', 'timeout'); }); + await assert.rejects(endDetachedSession(info('a', 'detached'), timeout.deps), /did not respond/); + assert.deepEqual(timeout.calls, ['list', 'kill a']); + + // Killed, archived. + const killed = fakeService([info('a', 'detached')]); + assert.equal(await endDetachedSession(info('a', 'detached'), killed.deps), 'killed'); + assert.deepEqual(killed.calls, ['list', 'kill a', 'archive']); + + // Killed by this call, but archiving failed: still ended by us (never "already ended"), and said so. + const unarchived = fakeService([info('a', 'detached')], async () => {}, async () => { throw new Error('Another NMSh window is archiving this session.'); }); + assert.equal(await endDetachedSession(info('a', 'detached'), unarchived.deps), 'killed-unarchived'); +}); + +test('the kill targets the service\'s current record of the same id, never a remembered pid', async () => { + const seen: SessionInfo[] = []; + const {deps} = fakeService([info('a', 'detached', {pid: 999})], async session => { seen.push(session); }); + await endDetachedSession(info('a', 'detached', {pid: 100}), deps); + assert.equal(seen[0]!.pid, 999, 'the re-read record is what is sent; the kill itself is addressed by id'); +}); + +test('bulk confirmation names every target, by name, with no "and N more"', () => { + const sessions = Array.from({length: 12}, (_, index) => info(`s${index}`, 'detached', {cwd: '/w/same', signature: `Fruit${index}`})); + const state = createMultiPicker(sessions); + multiPickerKey(state, text('a')); + multiPickerKey(state, text('x')); + const screen = renderMultiPicker(state, 120, 5_000).join('\n'); + assert.match(screen, /Kill 12 detached sessions\?/); + for (let index = 0; index < 12; index += 1) assert.match(screen, new RegExp(`Fruit${index} · /w/same`), `session ${index} is listed`); + assert.doesNotMatch(screen, /more/); +}); + +test('bulk end with partial failures: each outcome is counted, failures stay listed and selected, nothing claims success', async () => { + const outcomes: Array = ['killed', new Error('session service did not respond'), 'killed-unarchived', 'gone']; + const screens: string[] = []; + let call = 0; + const run = async (render: (columns: number) => string[], onKey: (key: Key) => T | undefined): Promise => { + const keys = call === 0 ? [text('a'), text('x'), text('y')] : [text('n')]; + call += 1; + for (const k of keys) { const result = onKey(k); if (result !== undefined) return result; } + throw new Error('no result'); + }; + const sessions = ['a', 'b', 'c', 'd'].map(id => info(id, 'detached')); + const ids = await pickWithKill(sessions, {run: async (render, onKey) => { screens.push(render(400).join('\n')); return run(render, onKey); }, + now: () => 0, kill: async () => { const next = outcomes.shift()!; if (next instanceof Error) throw next; return next; }}); + assert.deepEqual(ids, []); + const summary = screens.find(screen => /ended/.test(screen)) ?? ''; + assert.match(summary, /1 session ended/); + assert.match(summary, /1 session ended but its transcript was not archived yet/); + assert.match(summary, /1 session had already ended/); + assert.match(summary, /1 session could not be ended \(session service did not respond\)/); + assert.match(summary, /1 detached live session\b/, 'only the failed one is still listed'); + assert.match(summary, /\/w\/b/); +}); + +test('one-session prompt: a kill the service refused or could not perform returns to the prompt and says why', async () => { + const outcomes: Array = ['attached', new Error('session service did not respond')]; + const screens: string[] = []; + const script: Key[][] = [[text('x')], [key('enter')], [text('x')], [key('enter')], [text('n')]]; + const run = async (render: (columns: number) => string[], onKey: (key: Key) => T | undefined): Promise => { + screens.push(render(120).join('\n')); + for (const k of script.shift() ?? []) { const result = onKey(k); if (result !== undefined) return result; } + throw new Error('no result'); + }; + const choice = await askWithKill(info('a', 'detached'), {run, now: () => 0, + kill: async () => { const next = outcomes.shift()!; if (next instanceof Error) throw next; return next; }}); + assert.equal(choice, 'not-now', 'neither refused kill was treated as a kill'); + assert.ok(screens.some(screen => /1 detached live session[\s\S]*attached in another window and was not touched/.test(screen))); + assert.ok(screens.some(screen => /Could not end that session: session service did not respond/.test(screen))); +}); + +test('one-session prompt: a killed or already-ended session lets startup continue fresh', async () => { + for (const outcome of ['killed', 'killed-unarchived', 'gone'] as const) { + const script: Key[][] = [[text('x')], [key('enter')]]; + const run = async (_render: unknown, onKey: (key: Key) => T | undefined): Promise => { + for (const k of script.shift() ?? []) { const result = onKey(k); if (result !== undefined) return result; } + throw new Error('no result'); + }; + assert.equal(await askWithKill(info('a', 'detached'), {run, kill: async () => outcome}), 'kill'); + } +}); + +class RawPeer { + readonly messages: ServerMessage[] = []; + readonly socket: Socket; + private readonly decoder = new FrameDecoder(); + constructor(path: string) { + this.socket = connect(path); + this.socket.setEncoding('utf8'); + this.socket.on('data', chunk => { for (const r of this.decoder.push(chunk as unknown as string)) if (r.ok) this.messages.push(r.message as ServerMessage); }); + this.socket.on('error', () => {}); + this.send({type: 'hello', version: PROTOCOL_VERSION, client: 'test'}); + } + send(message: Parameters[0]): void { this.socket.write(encodeMessage(message)); } + count(type: string): number { return this.messages.filter(m => m.type === type).length; } + async waitFor(type: string, count = 1): Promise { + await until(() => this.count(type) >= count); + return this.messages.filter(m => m.type === type)[count - 1]!; + } +} + +async function until(check: () => boolean, timeoutMs = 15000): Promise { + const deadline = Date.now() + timeoutMs; + while (!check()) { + if (Date.now() > deadline) throw new Error('condition not reached'); + await new Promise(resolve => setTimeout(resolve, 25)); + } +} + +function scratch(prefix: string): string { + const dir = realpathSync(mkdtempSync(join(tmpdir(), prefix))); + chmodSync(dir, 0o700); + return dir; +} + +test('service: once a confirmed kill is sent, no window can attach to the ending session before its shell exits', async () => { + const runtimeDir = scratch('nmsh-kr-'); + const home = scratch('nmsh-kh-'); + const service = new SessionService({runtimeDir, startupIdleMs: 60_000}); + await service.start(); + const keeper = new RawPeer(service.socketPath); + let pid = 0; + try { + const owner = new RawPeer(service.socketPath); + owner.send({type: 'create', cwd: home, env: {HOME: home, PATH: process.env.PATH ?? ''}, columns: 80, rows: 24}); + const created = await owner.waitFor('created'); + const id = created.type === 'created' ? created.sessionId : ''; + pid = created.type === 'created' ? created.pid : 0; + await owner.waitFor('prompt'); + // The shell takes a moment to exit after SIGHUP: the window in which an attach used to succeed. + owner.send({type: 'input', data: 'trap "sleep 1; exit 0" HUP\r'}); + await owner.waitFor('prompt', 2); + owner.socket.destroy(); + await until(() => service.registry[0]?.state === 'detached'); + + const killer = new RawPeer(service.socketPath); + killer.send({type: 'kill', sessionId: id}); + await until(() => (service as unknown as {sessions: Map}).sessions.get(id)?.ending === true); + const attacher = new RawPeer(service.socketPath); + attacher.send({type: 'attach', sessionId: id, columns: 80, rows: 24}); + const refused = await attacher.waitFor('error'); + assert.equal(refused.type === 'error' && refused.code, 'unknown'); + assert.match(refused.type === 'error' ? refused.message : '', /ending/); + assert.equal(attacher.count('attached'), 0); + // A second kill for the same session waits for the same exit. + const again = new RawPeer(service.socketPath); + again.send({type: 'kill', sessionId: id}); + await killer.waitFor('killed'); + await again.waitFor('killed'); + assert.equal(service.registry.length, 0); + for (const peer of [killer, attacher, again]) peer.socket.destroy(); + } finally { + keeper.socket.destroy(); + await service.close(); + if (pid) await until(() => { try { process.kill(pid, 0); return false; } catch { return true; } }, 5000).catch(() => {}); + await new Promise(resolve => setTimeout(resolve, 50)); + rmSync(runtimeDir, {recursive: true, force: true}); + rmSync(home, {recursive: true, force: true}); + } +}); diff --git a/tests/startupRestore.test.ts b/tests/startupRestore.test.ts index 7ae19ab7..e5c69c38 100644 --- a/tests/startupRestore.test.ts +++ b/tests/startupRestore.test.ts @@ -328,7 +328,7 @@ test('multi picker: X with a selection asks once for all of them, naming the cou const ids = await pickWithKill(three(), {run, now: () => 0, kill: async session => { killed.push(session.id); return outcomes.shift()!; }}); assert.deepEqual(killed, ['a', 'b', 'c']); assert.deepEqual(ids, []); - assert.ok(screens.some(screen => /2 sessions ended; 1 session attached in another window was not touched/.test(screen)), screens.at(-1)); + assert.ok(screens.some(screen => /1 session attached in another window was not touched; 2 sessions ended/.test(screen)), screens.at(-1)); assert.ok(screens.some(screen => /\/w\/b/.test(screen) && /1 detached live session\b/.test(screen)), 'the attached one is still listed'); });