From 3d89096e0d26a6a7daaf42b32e4af77265951717 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Sat, 12 Sep 2026 00:24:08 +0200 Subject: [PATCH 1/2] (remote): stop a remote session for real, behind the same dialog as a local one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #246 (lifecycle verbs). A remote row's stop control now goes through remote-stop-session: the descriptor pid is re-probed on the host (refused if it no longer names a claude CLI), then the narrowest tmux scope is killed (pane, else window — never the shared session), else TERM then KILL by pid. Closing a remote tab stays a detach. The descriptor is dropped and the host refreshed at once; the adapter marks the row dead. --- .ai/contexts/ipc-bridge.md | 3 +- .ai/contexts/session-state.md | 70 ++++++++++ docs/session-browser.md | 6 + eslint.config.js | 5 +- main.js | 29 ++++ preload.js | 2 + public/app.js | 13 +- public/index.html | 2 + public/remote-activity-ui.js | 14 ++ public/stop-session-ui.js | 14 ++ remote-attach.js | 3 + remote-index.js | 11 ++ remote-stop.js | 110 +++++++++++++++ test/remote-activity-ui.test.js | 39 ++++++ test/remote-index.test.js | 29 ++++ test/remote-stop.test.js | 174 ++++++++++++++++++++++++ test/stop-session-ui.test.js | 28 ++++ test/terminal-manager-lifecycle.test.js | 27 ++++ 18 files changed, 575 insertions(+), 4 deletions(-) create mode 100644 public/stop-session-ui.js create mode 100644 remote-stop.js create mode 100644 test/remote-stop.test.js create mode 100644 test/stop-session-ui.test.js diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index c66a5d87..2f6eb264 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -22,7 +22,8 @@ This file is the **canonical inventory** of the IPC surface. When you add a new | `get-active-sessions` | — | `{sessionId, busy}[]` | Currently open PTY sessions plus each one's live `_cliBusy` flag — see "Busy-state reconciliation" below. | | `get-active-terminals` | — | `Terminal[]` | Active PTY identifiers | | `open-terminal` | `(id, projectPath, isNew, sessionOptions)` | `{ok, error?, mcpActive}` | Spawn or attach a PTY. | -| `stop-session` | `(id)` | `{ok}` | Kill the PTY for `id`. | +| `stop-session` | `(id)` | `{ok}` | Kill the PTY for `id`. Local only — a remote-attach session's PTY is the local ssh attach client, so this only detaches it; see `remote-stop-session` for the real remote "stop". | +| `remote-stop-session` | `({alias, sessionId})` | `{ok, method?, error?}` | The "stop" verb for a remote-ssh session (`.ai/contexts/session-state.md`, "Lifecycle decisions") — kills the process on the host (`tmux kill-session` when the descriptor names a tmux target, else `kill -TERM` then `kill -KILL`), refusing with the pid-reuse message when the pid now belongs to a non-claude process. `method` is `'tmux' \| 'pid-term' \| 'pid-kill'`. On success also drops the host's in-memory descriptor, forces a host refresh, and detaches the local attach PTY if one was open. Handler in `main.js`, adapter in `remote-stop.js` (reuses `remote-attach.js`'s pid-reuse probe, not a fork). | | `toggle-star` | `(id)` | `{ok}` | Star/unstar in session_meta. | | `rename-session` | `(id, name)` | `{ok}` | Set customTitle. | | `archive-session` | `(id, archived)` | `{ok}` | Move to archive. | diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index 6ecc0430..e95e8ecb 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -30,6 +30,76 @@ what actually shipped, not the whole plan. `localTranscriptStates`) so `has-busy-agents` survives a full `renderProjects()` re-render for those two kinds, the same way it already did for local-pty via `activeSubagentsByParent`. +- **Lifecycle decisions (2026-09-11): done.** The two verbs — detach and + stop — are both real now; see "The two lifecycle verbs: detach and stop" + below. + +## The two lifecycle verbs: detach and stop + +Two facts the domain carries separately: **process liveness** (the CLI is +running — local pid, or a remote descriptor with an ALIVE marker, #262) and +**attached view** (Switchboard holds a PTY / an ssh attach for it). A row is +active because the process is alive, not because a tab is open. + +| verb | local-pty | remote-ssh | +|---|---|---| +| detach | not offered — closing a session's view is a stop (unchanged) | `stop-session`'s pre-existing behavior: `killPty` → the tmux adapter's `detach()` (`remote-attach.js`) — ends the local ssh client, optionally restores tmux options (solo attach, #256). Still reachable today wherever `activeSessions` cleanup calls `killPty` on a `kind: 'remote-attach'` session without a preceding `remote-stop-session` call, and via `close-terminal`'s ordinary detach (marks `rendererAttached=false`, kills nothing). | +| stop | kill the PTY (`stop-session`, unchanged) | **new**: `remote-stop-session` IPC (`{alias, sessionId}`) — kills the process on the host itself, same control and same confirmation dialog as local. No session locked by name or role. | + +**No new dialog component.** `public/stop-session-ui.js`'s `resolveSessionStop(session)` +is the only thing that differs between a local and a remote stop: which IPC +to call, and the `confirm()` text (the host alias is named for a remote +session). `app.js`'s `confirmAndStopSession` is still the single call site of +the dialog and the single stop control (the sidebar row's `.session-stop-btn`, +the terminal header's stop button, and the grid card's stop button all funnel +through it) — it now asks `resolveSessionStop` which IPC to call instead of +always calling `stopSession`. + +**The remote stop, main-side (`remote-stop.js`).** `createRemoteStopAdapter().stop(alias, descriptor)` +builds one non-interactive ssh command (same `buildRemoteCommandArgs` transport +as `remote-attach.js`'s probe/restore calls) that: (1) reuses +`remote-attach.js`'s `buildProcCmdlineCheck`/pid-reuse guard verbatim — a +recycled pid is refused with the exact wording the attach path uses, not a +forked copy; (2) when the descriptor's `tmux` field parses, discovers the +socket from `/proc//environ` (identical to the attach probe) and kills at +the **narrowest matching scope, never the session**: `tmux kill-pane -t +` when the target names a pane, `tmux kill-window -t ` when it +names only a window. `kill-session` is never emitted — the VPS harness runs +several CLIs as windows/panes of one shared tmux session, and a session-wide +kill would take every sibling down with the one being stopped; (3) otherwise, +or if the tmux kill fails, falls back to `kill -TERM `, polls +`/proc/` for up to ~3s (six 0.5s ticks), then `kill -KILL` once if it is +still there. Returns `{ok, method}` where `method` is +`'tmux-pane' | 'tmux-window' | 'pid-term' | 'pid-kill'`, or `{ok:false, error}`. +`targetHasPane()` reads the pane/window distinction off the target string +itself (a "." after the session prefix means a pane component follows, +matching the grammar `TMUX_FIELD_RE` already validates) — no new parsing of +the descriptor is added. + +**On a successful stop, `main.js`'s `remote-stop-session` handler**: drops the +descriptor from `remote-index.js`'s in-memory list (`dropRemoteSession(alias, +sessionId)`) and calls `notifyRendererProjectsChanged()` directly — a forced +`refreshHostNow` alone does not reliably `notify()` (only a folder-level jsonl +change does), so the row would otherwise wait for the next real host cycle +to reflect the kill; `refreshHostNow(alias, {force:true})` still runs +afterward, fire-and-forget, as the authoritative reconciliation once the host's +own next descriptor list confirms the process is gone. If this app held a +local ssh attach for the now-dead session (`session.kind === 'remote-attach'`), +`killPty` closes it too — the remote process is already gone, so there is +nothing left to detach *from*, but the local ssh client would otherwise linger +until it notices the far end closed on its own. + +**The renderer side, immediately.** On a successful remote stop, +`public/remote-activity-ui.js`'s `applyRemoteStopped(sessionId)` applies +`liveness:'dead'`, `attached:false`, and — beyond what the issue text names, +needed so the adapter's own snapshot does not keep claiming a dead process is +still doing something — clears `busy`/`attention`/`agentsBusy` too, cancels +both of the adapter's own decay timers (activity and subagent-attribution), +and calls `purgeActivityFor(sessionId, 'remote-stop')` to drop the +parallel-fed `sessionBusyState`/`responseReadySessions`/`attentionSessions` +Map entries (see "migration status" above — two readers still consume those +Maps directly). This repaints the row's icon slot before the next +`get-projects` round-trip lands. ### The remote-ssh adapter (step 3) diff --git a/docs/session-browser.md b/docs/session-browser.md index ffe6fdc6..6cc9737a 100644 --- a/docs/session-browser.md +++ b/docs/session-browser.md @@ -44,6 +44,12 @@ The confirmation dialog states what will be removed: the project, how many files If the session is still running it is stopped first when Switchboard knows it is live; otherwise the deletion is refused with a reason rather than pulling a transcript out from under a running process. Anything that resolves outside `~/.claude/projects` — a symlinked transcript, for instance — is refused and logged. A session that never started has no transcript to remove, so deleting it just clears the leftover card. +## Stop a running session + +The stop button on a running session's card ends its process — for a local session and for a session on a declared remote host alike, with the same confirmation dialog. For a remote host session the dialog names the host, and stopping kills the process on that host, not just Switchboard's view of it. + +A remote session you are not currently viewing keeps running on the host even though Switchboard is not attached to it; opening it again reattaches to the same process instead of starting a new one. Only the stop button ends it. + ## Star and archive - **Star** — right-click a session and choose Star, or use the star icon in the session header. Starred sessions appear at the top of their project group. diff --git a/eslint.config.js b/eslint.config.js index b9c1e5ae..624de022 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -165,6 +165,9 @@ const rendererCrossFileGlobals = { paintSessionIcon: 'readonly', // public/remote-activity-ui.js (remote-ssh adapter, see .ai/contexts/session-state.md) setRemoteAttached: 'readonly', + applyRemoteStopped: 'readonly', + // public/stop-session-ui.js (pure stop-vs-detach decision, see .ai/contexts/session-state.md) + resolveSessionStop: 'readonly', // read by sidebar.js's parentHasActiveSubagent() — see .ai/contexts/subagent-observability.md remoteSessionStates: 'readonly', // public/local-transcript-adapter.js (local-transcript adapter, see .ai/contexts/session-state.md) @@ -317,7 +320,7 @@ module.exports = [ // Dual-mode helper: classic + + diff --git a/public/remote-activity-ui.js b/public/remote-activity-ui.js index 42e557a8..7fc72e62 100644 --- a/public/remote-activity-ui.js +++ b/public/remote-activity-ui.js @@ -131,6 +131,20 @@ function setRemoteAttached(sessionId, attached) { projectRemoteState(sessionId); } +// see .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop") +function applyRemoteStopped(sessionId) { + clearRemoteActivityTimer(sessionId); + clearRemoteAgentsTimer(sessionId); + const state = remoteState(sessionId); + state.apply({ type: 'busy', active: false, armReady: false }); + state.apply({ type: 'attention', active: false }); + state.apply({ type: 'subagentCompleted', stillActive: false }); + state.apply({ type: 'liveness', value: 'dead' }); + state.apply({ type: 'attached', value: false }); + projectRemoteState(sessionId); + purgeActivityFor(sessionId, 'remote-stop'); +} + function seedRemoteActivity(session) { if (!session || !session.remoteAlias) return; applyRemoteDescriptor(session); diff --git a/public/stop-session-ui.js b/public/stop-session-ui.js new file mode 100644 index 00000000..ce3d3c40 --- /dev/null +++ b/public/stop-session-ui.js @@ -0,0 +1,14 @@ +// Dual-mode helper — see .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop") + +// session: the sessionMap entry for the row being stopped, or undefined. +function resolveSessionStop(session) { + const alias = session && session.remoteAlias; + if (alias) { + return { remote: true, alias, confirmText: `Stop this session on ${alias}?` }; + } + return { remote: false, alias: null, confirmText: 'Stop this session?' }; +} + +if (typeof module !== 'undefined' && module.exports) { + module.exports = { resolveSessionStop }; +} diff --git a/remote-attach.js b/remote-attach.js index 02c57a11..629d33bf 100644 --- a/remote-attach.js +++ b/remote-attach.js @@ -351,4 +351,7 @@ module.exports = { buildAttachCommand, buildRestoreCommand, buildRemoteCommandArgs, + isValidPid, + buildProcCmdlineCheck, + defaultRunRemoteCommand, }; diff --git a/remote-index.js b/remote-index.js index eb3c94f6..7cd926bc 100644 --- a/remote-index.js +++ b/remote-index.js @@ -317,10 +317,21 @@ function createRemoteIndexer(ctx) { }; } + // see .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop") + function dropRemoteSession(alias, sessionId) { + const list = remoteSessions.get(alias); + if (!list || list.length === 0) return false; + const next = list.filter(s => s.sessionId !== sessionId); + if (next.length === list.length) return false; + remoteSessions.set(alias, next); + return true; + } + return { start, stop, dispose, restart, refreshNow, refreshHostNow, isRunning: () => timer !== null, getRemoteSessions, + dropRemoteSession, getRemoteHostState, }; } diff --git a/remote-stop.js b/remote-stop.js new file mode 100644 index 00000000..3e348cd0 --- /dev/null +++ b/remote-stop.js @@ -0,0 +1,110 @@ +// remote-stop.js — see .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop") +'use strict'; + +const { + isValidPid, + parseTmuxField, + buildProcCmdlineCheck, + buildRemoteCommandArgs, + defaultRunRemoteCommand, +} = require('./remote-attach'); + +const DEFAULT_STOP_TIMEOUT_MS = 15000; +const NOT_CLAUDE_EXIT_CODE = 7; +const NOT_CLAUDE_MARKER = 'NOT_CLAUDE'; +const TMUX_PANE_KILLED_MARKER = 'TMUX_PANE_KILLED'; +const TMUX_WINDOW_KILLED_MARKER = 'TMUX_WINDOW_KILLED'; +const PID_TERM_MARKER = 'PID_KILLED_TERM'; +const PID_FORCE_MARKER = 'PID_KILLED_FORCE'; +// ~3s: kill -TERM, then poll /proc/ six times at 0.5s before kill -KILL. +const TERM_WAIT_TICKS = 6; +const TERM_WAIT_STEP_S = '0.5'; + +// see .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop") +function buildRefusalGuard(pid) { + const check = buildProcCmdlineCheck(pid); + return `alive=$(${check}); if [ "$alive" != "1" ]; then echo ${NOT_CLAUDE_MARKER}; exit ${NOT_CLAUDE_EXIT_CODE}; fi;`; +} + +function buildKillByPidSegment(pid) { + return `kill -TERM ${pid} 2>/dev/null; i=0; ` + + `while [ -d /proc/${pid} ] && [ $i -lt ${TERM_WAIT_TICKS} ]; do sleep ${TERM_WAIT_STEP_S}; i=$((i+1)); done; ` + + `if [ -d /proc/${pid} ]; then kill -KILL ${pid} 2>/dev/null; echo ${PID_FORCE_MARKER}; else echo ${PID_TERM_MARKER}; fi; exit 0`; +} + +// pane present only when a "." follows the window component — see .ai/contexts/session-state.md +function targetHasPane(target) { + const idx = target.indexOf(':'); + const winPane = idx >= 0 ? target.slice(idx + 1) : target; + return winPane.includes('.'); +} + +// see .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop") +function buildStopCommand(pid, tmuxTarget) { + const guard = buildRefusalGuard(pid); + const killByPid = buildKillByPidSegment(pid); + if (!tmuxTarget) return `${guard} ${killByPid}`; + + // never kill-session, siblings share the tmux session — see .ai/contexts/session-state.md + const hasPane = targetHasPane(tmuxTarget); + const tmuxSubcommand = hasPane ? 'kill-pane' : 'kill-window'; + const tmuxMarker = hasPane ? TMUX_PANE_KILLED_MARKER : TMUX_WINDOW_KILLED_MARKER; + + const sockDiscovery = `sock=$(tr '\\0' '\\n' < /proc/${pid}/environ 2>/dev/null | grep -m1 '^TMUX=' | cut -d= -f2- | cut -d, -f1);`; + return `${guard} ${sockDiscovery} ` + + `if [ -n "$sock" ] && tmux -S "$sock" ${tmuxSubcommand} -t ${tmuxTarget} 2>/dev/null; then echo ${tmuxMarker}; exit 0; fi; ` + + killByPid; +} + +// see .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop") +function createRemoteStopAdapter(opts = {}) { + const runRemoteCommand = opts.runRemoteCommand || defaultRunRemoteCommand; + const log = opts.log || { info() {}, warn() {}, error() {} }; + + async function stop(alias, descriptor) { + if (!isValidPid(descriptor && descriptor.pid)) { + return { ok: false, error: 'session carries no readable pid — cannot stop it' }; + } + const parsedTmux = descriptor.tmux ? parseTmuxField(descriptor.tmux) : null; + const command = buildStopCommand(descriptor.pid, parsedTmux ? parsedTmux.target : null); + + let result; + try { + result = await runRemoteCommand(alias, command, { timeoutMs: DEFAULT_STOP_TIMEOUT_MS }); + } catch (err) { + return { ok: false, error: `stop failed: ${err.message}` }; + } + if (!result) return { ok: false, error: 'stop failed: no response' }; + + const stdout = result.stdout || ''; + if (result.code === NOT_CLAUDE_EXIT_CODE || stdout.includes(NOT_CLAUDE_MARKER)) { + return { ok: false, error: `pid ${descriptor.pid} now belongs to a process that is not a claude CLI — the session is gone` }; + } + if (result.code !== 0) { + const reason = (result.stderr || '').trim() || 'no stderr'; + return { ok: false, error: `stop failed (exit ${result.code}): ${reason}` }; + } + + const method = stdout.includes(TMUX_PANE_KILLED_MARKER) ? 'tmux-pane' + : stdout.includes(TMUX_WINDOW_KILLED_MARKER) ? 'tmux-window' + : stdout.includes(PID_FORCE_MARKER) ? 'pid-kill' + : stdout.includes(PID_TERM_MARKER) ? 'pid-term' + : 'unknown'; + log.info(`[remote-stop:${alias}] stopped pid ${descriptor.pid} via ${method}`); + return { ok: true, method }; + } + + return { stop }; +} + +module.exports = { + createRemoteStopAdapter, + buildStopCommand, + buildRemoteCommandArgs, + NOT_CLAUDE_EXIT_CODE, + NOT_CLAUDE_MARKER, + TMUX_PANE_KILLED_MARKER, + TMUX_WINDOW_KILLED_MARKER, + PID_TERM_MARKER, + PID_FORCE_MARKER, +}; diff --git a/test/remote-activity-ui.test.js b/test/remote-activity-ui.test.js index e7b7e70a..2357e6a6 100644 --- a/test/remote-activity-ui.test.js +++ b/test/remote-activity-ui.test.js @@ -79,8 +79,12 @@ function setup(sessionIds = ['s1']) { pending: () => scheduled.filter(h => !h.cleared), pruneRemoteActivityTimers: read('pruneRemoteActivityTimers'), remoteActivityDecayTimers: read('remoteActivityDecayTimers'), + remoteAgentsDecayTimers: read('remoteAgentsDecayTimers'), + remoteSessionStates: read('remoteSessionStates'), + applyRemoteStopped: read('applyRemoteStopped'), sessionBusyState: read('sessionBusyState'), responseReadySessions: read('responseReadySessions'), + attentionSessions: read('attentionSessions'), destroy: () => window.close(), }; } @@ -162,6 +166,41 @@ test('a second event before decay resets the timer instead of stacking one', () t.destroy(); }); +// Lifecycle decisions (2026-09-11): a successful remote-stop-session must +// reflect immediately — liveness dead, attached false, no lingering busy +// state or decay timer waiting to repaint the row as busy again. +test('applyRemoteStopped marks the row dead/detached, clears busy state, and cancels its decay timers', () => { + const t = setup(['s1']); + t.emit({ sessionId: 's1' }); // busy, with a live decay timer + assert.ok(t.item('s1').classList.contains('cli-busy'), 'precondition: row is busy'); + assert.equal(t.pending().length, 1, 'precondition: a decay timer is armed'); + + t.applyRemoteStopped('s1'); + + const snapshot = t.remoteSessionStates.get('s1').snapshot(); + assert.equal(snapshot.liveness, 'dead'); + assert.equal(snapshot.attached, false); + assert.equal(snapshot.busy, false); + + assert.ok(!t.item('s1').classList.contains('cli-busy'), 'the row must stop looking busy'); + assert.ok(!t.item('s1').classList.contains('response-ready'), 'a killed process never claims a finished response'); + assert.equal(t.pending().length, 0, 'the decay timer must be cancelled, not left to fire later'); + assert.equal(t.sessionBusyState.has('s1'), false, 'purgeActivityFor must drop the parallel-fed busy Map entry'); + assert.equal(t.responseReadySessions.has('s1'), false); +}); + +test('applyRemoteStopped also cancels a pending subagent-attribution decay timer', () => { + const t = setup(['s1']); + t.emit({ sessionId: 's1', kind: 'subagent', parentSessionId: 's1' }); + assert.equal(t.remoteAgentsDecayTimers.has('s1'), true, 'precondition: an agents-busy decay timer is armed'); + + t.applyRemoteStopped('s1'); + + assert.equal(t.remoteAgentsDecayTimers.has('s1'), false); + assert.equal(t.remoteSessionStates.get('s1').snapshot().agentsBusy, false, + 'a dead session cannot still have subagents running under it'); +}); + test('pruneRemoteActivityTimers cancels a timer whose row no longer exists', () => { const t = setup(['s1', 's2']); t.emit({ sessionId: 's1' }); diff --git a/test/remote-index.test.js b/test/remote-index.test.js index 266a2771..8a2bb719 100644 --- a/test/remote-index.test.js +++ b/test/remote-index.test.js @@ -233,6 +233,35 @@ test('getRemoteSessions keeps the last known descriptors, not wiped, after a cyc } finally { fs.rmSync(dataDir, { recursive: true, force: true }); } }); +// Lifecycle decisions (2026-09-11): a successful remote-stop-session drops +// the descriptor from the in-memory list immediately, ahead of the next poll. +test('dropRemoteSession removes exactly the named descriptor and reports whether it found one', async () => { + const dataDir = tmp('idx-drop'); + try { + const indexer = createRemoteIndexer({ + getHosts: () => [{ alias: 'vps' }], + dataDir, + transport: {}, + scanFolders: () => Promise.resolve({ ok: true }), + listIndexedFolderKeys: () => [], + timers: fakeTimers(), + sync: async () => ({ + fetched: 0, unchanged: 0, removed: 0, failed: 0, total: 0, + changedFolders: new Set(), + sessions: [{ pid: 1, sessionId: 'a' }, { pid: 2, sessionId: 'b' }], + }), + }); + await indexer.refreshNow(); + + assert.equal(indexer.dropRemoteSession('vps', 'a'), true); + assert.deepEqual(indexer.getRemoteSessions('vps').sessions, [{ pid: 2, sessionId: 'b' }]); + + assert.equal(indexer.dropRemoteSession('vps', 'a'), false, 'already gone — nothing to drop a second time'); + assert.equal(indexer.dropRemoteSession('vps', 'never-seen'), false); + assert.equal(indexer.dropRemoteSession('unknown-alias', 'a'), false, 'an alias never refreshed has nothing to drop'); + } finally { fs.rmSync(dataDir, { recursive: true, force: true }); } +}); + test('a mirror already on disk but absent from the cache is indexed once', async () => { const dataDir = tmp('idx-cold'); try { diff --git a/test/remote-stop.test.js b/test/remote-stop.test.js new file mode 100644 index 00000000..de2434d9 --- /dev/null +++ b/test/remote-stop.test.js @@ -0,0 +1,174 @@ +'use strict'; + +// The remote "stop" verb, fully injected: no ssh, no network. Mirrors +// test/remote-attach.test.js's style for the sibling "detach" verb — see +// .ai/contexts/session-state.md ("Lifecycle decisions (2026-09-11)"). +// +// Four properties matter: +// 1. A recycled pid is refused before anything is killed — same wording, +// same guard, as remote-attach's pid-reuse check (not forked). +// 2. A tmux target is killed at the narrowest matching scope — the pane +// when the target names one, else the window — NEVER the whole session: +// the VPS harness runs several CLIs as windows/panes of one tmux +// session, and kill-session would take down every sibling. +// 3. No tmux field falls back to kill -TERM then kill -KILL after a +// bounded wait. +// 4. No command string ever contains a backtick, and kill-session is +// never emitted under any input. + +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { + createRemoteStopAdapter, + buildStopCommand, + NOT_CLAUDE_EXIT_CODE, + NOT_CLAUDE_MARKER, + TMUX_PANE_KILLED_MARKER, + TMUX_WINDOW_KILLED_MARKER, + PID_TERM_MARKER, + PID_FORCE_MARKER, +} = require('../remote-stop'); + +const silentLog = { info() {}, warn() {}, error() {} }; + +function makeAdapter(runRemoteCommand) { + return createRemoteStopAdapter({ runRemoteCommand, log: silentLog }); +} + +// --- buildStopCommand ------------------------------------------------------ + +test('buildStopCommand: no tmux target kills by pid only, no tmux mention', () => { + const cmd = buildStopCommand(4242, null); + assert.match(cmd, /kill -TERM 4242/); + assert.match(cmd, /kill -KILL 4242/); + assert.ok(!cmd.includes('tmux'), 'no tmux invocation when the descriptor names no target'); +}); + +test('buildStopCommand: a target naming a pane kills only that pane, never the session', () => { + const cmd = buildStopCommand(4242, 'main:@0.%0'); + assert.match(cmd, /tmux -S "\$sock" kill-pane -t main:@0\.%0/); + assert.match(cmd, /\/proc\/4242\/environ/, 'socket must be discovered from the pid\'s own environ, like remote-attach\'s probe'); + assert.match(cmd, /kill -TERM 4242/, 'the pid fallback must still be present if tmux kill-pane fails'); + assert.ok(!cmd.includes('kill-session'), 'never kill-session — siblings share the session on the VPS harness'); +}); + +test('buildStopCommand: a target naming a window only kills the window, never the session', () => { + const cmd = buildStopCommand(4242, 'main:@0'); + assert.match(cmd, /tmux -S "\$sock" kill-window -t main:@0\b/); + assert.ok(!cmd.includes('kill-pane'), 'a window-only target must not be treated as a pane target'); + assert.ok(!cmd.includes('kill-session'), 'never kill-session — siblings share the session on the VPS harness'); +}); + +test('buildStopCommand: the pid-reuse guard runs before either kill path', () => { + const cmd = buildStopCommand(4242, 'main:@0.%0'); + const guardIdx = cmd.indexOf(NOT_CLAUDE_MARKER); + const tmuxIdx = cmd.indexOf('kill-pane'); + const killIdx = cmd.indexOf('kill -TERM'); + assert.ok(guardIdx >= 0 && guardIdx < tmuxIdx && guardIdx < killIdx, 'the refusal guard must precede both kill paths'); + assert.match(cmd, /\/proc\/4242\/cmdline/, 'must reuse remote-attach\'s cmdline check, not a forked copy'); +}); + +test('kill-session never appears in any built command, for any input', () => { + const inputs = [null, 'main:@0.%0', 'main:@0', 'main:0.0', 'main:0']; + for (const tmuxTarget of inputs) { + assert.ok(!buildStopCommand(4242, tmuxTarget).includes('kill-session'), + `kill-session must never appear (input: ${tmuxTarget})`); + } +}); + +test('buildStopCommand waits roughly 3s before escalating to kill -KILL', () => { + const cmd = buildStopCommand(4242, null); + assert.match(cmd, /sleep 0\.5/); + assert.match(cmd, /-lt 6/, '6 ticks of 0.5s bounds the wait at ~3s'); +}); + +test('no builder ever emits a backtick', () => { + assert.ok(!buildStopCommand(4242, null).includes('`')); + assert.ok(!buildStopCommand(4242, 'main:@0.%0').includes('`')); + assert.ok(!buildStopCommand(4242, 'main:@0').includes('`')); +}); + +// --- createRemoteStopAdapter().stop() -------------------------------------- + +test('stop() refuses a recycled pid without reporting success (mutation target: removing the guard)', async () => { + const calls = []; + const adapter = makeAdapter(async (alias, command) => { + calls.push(command); + return { code: NOT_CLAUDE_EXIT_CODE, stdout: NOT_CLAUDE_MARKER + '\n', stderr: '' }; + }); + const result = await adapter.stop('vps', { sessionId: 's1', pid: 4242 }); + assert.equal(result.ok, false); + assert.match(result.error, /pid 4242 now belongs to a process that is not a claude CLI/); + assert.equal(calls.length, 1); +}); + +test('stop() succeeds via tmux kill-pane when the descriptor names a pane target', async () => { + const commands = []; + const adapter = makeAdapter(async (alias, command) => { + commands.push(command); + return { code: 0, stdout: TMUX_PANE_KILLED_MARKER + '\n', stderr: '' }; + }); + const result = await adapter.stop('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0' }); + assert.deepEqual(result, { ok: true, method: 'tmux-pane' }); + assert.match(commands[0], /kill-pane/); + assert.ok(!commands[0].includes('kill-session')); +}); + +test('stop() succeeds via tmux kill-window when the descriptor names a window-only target', async () => { + const commands = []; + const adapter = makeAdapter(async (alias, command) => { + commands.push(command); + return { code: 0, stdout: TMUX_WINDOW_KILLED_MARKER + '\n', stderr: '' }; + }); + const result = await adapter.stop('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0' }); + assert.deepEqual(result, { ok: true, method: 'tmux-window' }); + assert.match(commands[0], /kill-window/); + assert.ok(!commands[0].includes('kill-session')); +}); + +test('stop() succeeds via a plain kill -TERM when no tmux target is named', async () => { + const adapter = makeAdapter(async () => ({ code: 0, stdout: PID_TERM_MARKER + '\n', stderr: '' })); + const result = await adapter.stop('vps', { sessionId: 's1', pid: 4242 }); + assert.deepEqual(result, { ok: true, method: 'pid-term' }); +}); + +test('stop() reports the forced kill -KILL escalation as its own method', async () => { + const adapter = makeAdapter(async () => ({ code: 0, stdout: PID_FORCE_MARKER + '\n', stderr: '' })); + const result = await adapter.stop('vps', { sessionId: 's1', pid: 4242 }); + assert.deepEqual(result, { ok: true, method: 'pid-kill' }); +}); + +test('stop() surfaces an ssh failure without claiming success', async () => { + const adapter = makeAdapter(async () => ({ code: 255, stdout: '', stderr: 'ssh: connection refused' })); + const result = await adapter.stop('vps', { sessionId: 's1', pid: 4242 }); + assert.equal(result.ok, false); + assert.match(result.error, /connection refused/); +}); + +test('stop() surfaces a thrown ssh runner without throwing itself', async () => { + const adapter = makeAdapter(async () => { throw new Error('ECONNRESET'); }); + const result = await adapter.stop('vps', { sessionId: 's1', pid: 4242 }); + assert.equal(result.ok, false); + assert.match(result.error, /ECONNRESET/); +}); + +test('stop() refuses a descriptor with no readable pid, before any ssh call', async () => { + let calls = 0; + const adapter = makeAdapter(async () => { calls++; return { code: 0, stdout: '' }; }); + const result = await adapter.stop('vps', { sessionId: 's1' }); + assert.equal(result.ok, false); + assert.match(result.error, /pid/i); + assert.equal(calls, 0); +}); + +test('stop() ignores a descriptor tmux field that fails validation and falls back to pid-kill semantics', async () => { + const commands = []; + const adapter = makeAdapter(async (alias, command) => { + commands.push(command); + return { code: 0, stdout: PID_TERM_MARKER + '\n', stderr: '' }; + }); + const result = await adapter.stop('vps', { sessionId: 's1', pid: 4242, tmux: 'not valid; rm -rf /' }); + assert.equal(result.ok, true); + assert.ok(!commands[0].includes('tmux'), 'an unparseable tmux field must never reach the command string'); +}); diff --git a/test/stop-session-ui.test.js b/test/stop-session-ui.test.js new file mode 100644 index 00000000..a12f01a3 --- /dev/null +++ b/test/stop-session-ui.test.js @@ -0,0 +1,28 @@ +'use strict'; + +// Pure decision behind the shared stop control — see +// .ai/contexts/session-state.md ("Lifecycle decisions (2026-09-11)"). No DOM, +// no IPC: this is the only thing that differs between a local and a remote +// stop, so app.js's confirmAndStopSession has a single confirm() call site. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { resolveSessionStop } = require('../public/stop-session-ui'); + +test('a local session (no remoteAlias) resolves to the plain stop dialog and the local IPC', () => { + const plan = resolveSessionStop({ sessionId: 's1' }); + assert.deepEqual(plan, { remote: false, alias: null, confirmText: 'Stop this session?' }); +}); + +test('an undefined session (row not found) still resolves to the local stop, never throws', () => { + const plan = resolveSessionStop(undefined); + assert.deepEqual(plan, { remote: false, alias: null, confirmText: 'Stop this session?' }); +}); + +test('a remote session resolves to the remote IPC with the host alias named in the dialog text', () => { + const plan = resolveSessionStop({ sessionId: 's1', remoteAlias: 'vps' }); + assert.equal(plan.remote, true); + assert.equal(plan.alias, 'vps'); + assert.match(plan.confirmText, /vps/, 'the host alias must be shown, same text shape as the local dialog'); + assert.match(plan.confirmText, /^Stop this session/, 'same text shape as the local confirm()'); +}); diff --git a/test/terminal-manager-lifecycle.test.js b/test/terminal-manager-lifecycle.test.js index 5047d02e..427df6fe 100644 --- a/test/terminal-manager-lifecycle.test.js +++ b/test/terminal-manager-lifecycle.test.js @@ -48,6 +48,33 @@ test('flushTerminalBuffer after destroySession is a safe no-op', () => { } }); +// Lifecycle decisions (2026-09-11): closing a tab is a detach for a remote +// session (current close-terminal behavior, unchanged), never the new "stop" +// path — only the dedicated stop control (app.js's confirmAndStopSession, +// via resolveSessionStop) may call stopSession/remoteStopSession. +// See .ai/contexts/session-state.md. +test('destroySession never calls stopSession or remoteStopSession — only closeTerminal (detach)', () => { + const { window, spies, destroy } = setupTerminalDom(); + try { + let stopCalls = 0; + let remoteStopCalls = 0; + window.api.stopSession = () => { stopCalls++; return Promise.resolve({ ok: true }); }; + window.api.remoteStopSession = () => { remoteStopCalls++; return Promise.resolve({ ok: true }); }; + + // A remote session's entry carries the same shape terminal-manager.js + // already handles — remoteAlias is opaque to this file, by design: the + // tab-close path never branches on it. + window.createTerminalEntry({ sessionId: 's1', remoteAlias: 'vps' }); + window.destroySession('s1'); + + assert.strictEqual(spies.closeTerminal, 1, 'closing a tab still detaches via close-terminal'); + assert.strictEqual(stopCalls, 0, 'destroySession must never stop the local pty'); + assert.strictEqual(remoteStopCalls, 0, 'destroySession must never kill the remote process'); + } finally { + destroy(); + } +}); + test('destroySession on unknown sessionId is a no-op', () => { const { window, spies, destroy } = setupTerminalDom(); try { From bf9e19bd015befd5def9cc2a10d408285a7bb244 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Sat, 12 Sep 2026 01:18:21 +0200 Subject: [PATCH 2/2] (remote): show the stop control for any live row, keep state on a failed stop Review follow-up on the remote stop. The projection marks a row whose process is alive (.is-alive) and the stop control shows for it, attached or not; a failed stop leaves activePtyIds and the open view untouched and flashes the button instead of logging only; the tmux kill path polls /proc before claiming success and falls through to the pid path; the main handler's ok-guarded fan-out is pinned by a source check. --- .ai/contexts/ipc-bridge.md | 2 +- .ai/contexts/session-state.md | 33 ++- eslint.config.js | 2 + public/app.js | 22 +- public/grid-view.js | 5 +- public/session-activity-dom.js | 15 +- public/sidebar.js | 4 +- public/style.css | 3 +- remote-stop.js | 13 +- test/confirm-and-stop-session.test.js | 172 ++++++++++++++++ test/dom-grid-group-by-project.test.js | 2 + test/dom-grid-sidebar-prune-collision.test.js | 2 + test/dom-subagent-transcript.test.js | 2 + test/dom-subagent-ttl-tick.test.js | 2 + test/main-wiring-source-check.test.js | 17 ++ test/remote-stop-reachability.test.js | 190 ++++++++++++++++++ test/remote-stop.test.js | 20 ++ test/terminal-manager-harness.js | 2 + 18 files changed, 489 insertions(+), 19 deletions(-) create mode 100644 test/confirm-and-stop-session.test.js create mode 100644 test/remote-stop-reachability.test.js diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index 2f6eb264..21e6b156 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -23,7 +23,7 @@ This file is the **canonical inventory** of the IPC surface. When you add a new | `get-active-terminals` | — | `Terminal[]` | Active PTY identifiers | | `open-terminal` | `(id, projectPath, isNew, sessionOptions)` | `{ok, error?, mcpActive}` | Spawn or attach a PTY. | | `stop-session` | `(id)` | `{ok}` | Kill the PTY for `id`. Local only — a remote-attach session's PTY is the local ssh attach client, so this only detaches it; see `remote-stop-session` for the real remote "stop". | -| `remote-stop-session` | `({alias, sessionId})` | `{ok, method?, error?}` | The "stop" verb for a remote-ssh session (`.ai/contexts/session-state.md`, "Lifecycle decisions") — kills the process on the host (`tmux kill-session` when the descriptor names a tmux target, else `kill -TERM` then `kill -KILL`), refusing with the pid-reuse message when the pid now belongs to a non-claude process. `method` is `'tmux' \| 'pid-term' \| 'pid-kill'`. On success also drops the host's in-memory descriptor, forces a host refresh, and detaches the local attach PTY if one was open. Handler in `main.js`, adapter in `remote-stop.js` (reuses `remote-attach.js`'s pid-reuse probe, not a fork). | +| `remote-stop-session` | `({alias, sessionId})` | `{ok, method?, error?}` | The "stop" verb for a remote-ssh session (`.ai/contexts/session-state.md`, "Lifecycle decisions") — kills the process on the host at the narrowest matching tmux scope when the descriptor names a target (`kill-pane` when it names a pane, `kill-window` when it names only a window — never `kill-session`, siblings share the tmux session), confirms death with a `/proc` poll, and falls back to `kill -TERM` then `kill -KILL` if the tmux kill didn't stick or no tmux target was named; refuses with the pid-reuse message when the pid now belongs to a non-claude process. `method` is `'tmux-pane' \| 'tmux-window' \| 'pid-term' \| 'pid-kill'`. On success also drops the host's in-memory descriptor, forces a host refresh, and detaches the local attach PTY if one was open. Handler in `main.js`, adapter in `remote-stop.js` (reuses `remote-attach.js`'s pid-reuse probe, not a fork). | | `toggle-star` | `(id)` | `{ok}` | Star/unstar in session_meta. | | `rename-session` | `(id, name)` | `{ok}` | Set customTitle. | | `archive-session` | `(id, archived)` | `{ok}` | Move to archive. | diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index e95e8ecb..f793368b 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -66,15 +66,24 @@ the **narrowest matching scope, never the session**: `tmux kill-pane -t ` when the target names a pane, `tmux kill-window -t ` when it names only a window. `kill-session` is never emitted — the VPS harness runs several CLIs as windows/panes of one shared tmux session, and a session-wide -kill would take every sibling down with the one being stopped; (3) otherwise, -or if the tmux kill fails, falls back to `kill -TERM `, polls -`/proc/` for up to ~3s (six 0.5s ticks), then `kill -KILL` once if it is -still there. Returns `{ok, method}` where `method` is +kill would take every sibling down with the one being stopped; a tmux exit +code of 0 only means tmux accepted the request, so this is confirmed with the +same `/proc/` poll as step (3) below before the tmux success marker is +reported — a survivor falls through to (3) instead; (3) otherwise, or if the +tmux kill fails (or its target survives the poll), falls back to `kill -TERM +`, polls `/proc/` for up to ~3s (six 0.5s ticks), then `kill -KILL` +once if it is still there. Returns `{ok, method}` where `method` is `'tmux-pane' | 'tmux-window' | 'pid-term' | 'pid-kill'`, or `{ok:false, error}`. `targetHasPane()` reads the pane/window distinction off the target string itself (a "." after the session prefix means a pane component follows, matching the grammar `TMUX_FIELD_RE` already validates) — no new parsing of -the descriptor is added. +the descriptor is added. The pane-vs-window suffix convention itself comes +from the CLI's own descriptor writer on the VPS side, not measured against +that writer's source from here; the `/proc/` death poll after the tmux +kill (above) is what bounds the blast radius if that assumption is ever +wrong — a wrongly-classified target still ends up TERM'd/KILL'd by pid once +the poll finds it still alive, instead of the stop silently reporting +success on a process the tmux call never actually touched. **On a successful stop, `main.js`'s `remote-stop-session` handler**: drops the descriptor from `remote-index.js`'s in-memory list (`dropRemoteSession(alias, @@ -450,3 +459,17 @@ from a completion signal it cannot verify — that is why the remote-ssh and local-transcript `busy: false` transitions always pass `armReady: false` (see "The remote-ssh adapter" and "The local-transcript adapter" above), not a tri-state `busy: unknown`. + +## Known limits + +- **The remote-stop pid-reuse guard is weak.** `remote-stop.js`'s + `buildRefusalGuard` (and `remote-attach.js`'s probe it reuses verbatim) + decides "is this still the claude CLI" with `grep -qi claude` against + `/proc//cmdline` — a process a user happens to launch with "claude" + anywhere in its argv (not the CLI itself) passes the same guard and can be + killed. Deferred, not implemented: hardening candidates are the `comm` + field from `/proc//stat` (the kernel-recorded executable basename, + harder to spoof by argv alone) and the process start time (`/proc//stat` + field 22, jiffies since boot) compared against the descriptor's own + recorded start time — a pid recycled fast enough to still say "claude" in + argv is caught by a start-time mismatch even when the cmdline check is not. diff --git a/eslint.config.js b/eslint.config.js index 624de022..6a9e1912 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -162,6 +162,8 @@ const rendererCrossFileGlobals = { setResponseReady: 'readonly', setCliBusy: 'readonly', setHasBusyAgents: 'readonly', + setIsAlive: 'readonly', + isSessionAlive: 'readonly', paintSessionIcon: 'readonly', // public/remote-activity-ui.js (remote-ssh adapter, see .ai/contexts/session-state.md) setRemoteAttached: 'readonly', diff --git a/public/app.js b/public/app.js index 328b4d56..9b0b2229 100644 --- a/public/app.js +++ b/public/app.js @@ -742,15 +742,26 @@ async function triggerRebuildAndSearch() { // --- Stop session helper --- // see .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop") -async function confirmAndStopSession(sessionId) { +// btn (optional): the clicked control, flashed on failure instead of alert() — see sidebar.js's session-delete-btn +async function confirmAndStopSession(sessionId, btn) { const plan = resolveSessionStop(sessionMap.get(sessionId)); if (!confirm(plan.confirmText)) return; const result = plan.remote ? await window.api.remoteStopSession(plan.alias, sessionId) : await window.api.stopSession(sessionId); if (result && result.ok === false) { - console.error('[stop-session]', result.error || 'unknown error'); - } else if (plan.remote && typeof applyRemoteStopped === 'function') { + const message = result.error || 'unknown error'; + console.error('[stop-session]', message); + // leave activePtyIds/the open view untouched on failure + if (btn) { + if (typeof window.flashButtonText === 'function') window.flashButtonText(btn, 'Failed', 1500); + const originalTitle = btn.title; + btn.title = message; + setTimeout(() => { btn.title = originalTitle; }, 3000); + } + return; + } + if (plan.remote && typeof applyRemoteStopped === 'function') { applyRemoteStopped(sessionId); } activePtyIds.delete(sessionId); @@ -764,7 +775,7 @@ async function confirmAndStopSession(sessionId) { // --- Terminal header controls --- terminalStopBtn.addEventListener('click', () => { - if (activeSessionId) confirmAndStopSession(activeSessionId); + if (activeSessionId) confirmAndStopSession(activeSessionId, terminalStopBtn); }); @@ -860,7 +871,8 @@ function updateRunningIndicators() { const footer = card.querySelector('.grid-card-footer'); if (footer) footer.children[0].textContent = running ? 'Running' : 'Stopped'; const stopBtn = card.querySelector('.grid-card-stop-btn'); - if (stopBtn) stopBtn.style.display = running ? '' : 'none'; + // is-alive: process alive on its host though unattached — see .ai/contexts/session-state.md + if (stopBtn) stopBtn.style.display = (running || isSessionAlive(sid)) ? '' : 'none'; } } diff --git a/public/grid-view.js b/public/grid-view.js index cb4d2785..4125a8e6 100644 --- a/public/grid-view.js +++ b/public/grid-view.js @@ -191,10 +191,11 @@ function wrapInGridCard(sessionId) { stopBtn.className = 'grid-card-stop-btn'; stopBtn.title = 'Stop session'; stopBtn.innerHTML = ''; - stopBtn.style.display = activePtyIds.has(sessionId) ? '' : 'none'; + // is-alive: process alive on its host though unattached — see .ai/contexts/session-state.md + stopBtn.style.display = (activePtyIds.has(sessionId) || isSessionAlive(sessionId)) ? '' : 'none'; stopBtn.onclick = (e) => { e.stopPropagation(); - confirmAndStopSession(sessionId); + confirmAndStopSession(sessionId, stopBtn); }; header.appendChild(stopBtn); diff --git a/public/session-activity-dom.js b/public/session-activity-dom.js index a55f9764..e9a47eed 100644 --- a/public/session-activity-dom.js +++ b/public/session-activity-dom.js @@ -20,6 +20,17 @@ function setHasBusyAgents(el, on) { if (el) el.classList.toggle('has-busy-agents', !!on); } +// process-alive-on-its-host signal, independent of attach state — see .ai/contexts/session-state.md +function setIsAlive(el, on) { + if (el) el.classList.toggle('is-alive', !!on); +} + +// the only reader other call sites (grid-view.js, app.js) should use +function isSessionAlive(sessionId) { + const el = sessionItemEl(sessionId); + return !!(el && el.classList.contains('is-alive')); +} + // local-pty only for now — see session-state.md "migration status". function computeBusyReadyClasses(sessionId) { const busy = sessionBusyState.get(sessionId) === true; @@ -54,11 +65,13 @@ function applyStateClasses(sessionId, snapshot) { const ready = icon.classes.includes('response-ready'); const busy = icon.classes.includes('cli-busy'); const agentsBusy = !!(snapshot && snapshot.agentsBusy); + const alive = !!(snapshot && snapshot.liveness === 'alive'); setResponseReady(item, ready); setCliBusy(item, busy); setHasBusyAgents(item, agentsBusy); + setIsAlive(item, alive); writeIconSlot(item.querySelector('.session-icon'), icon); - if (window.ATRACE) window.atrace('class.apply', sessionId, { el: item.id || null, 'response-ready': ready, 'cli-busy': busy, 'has-busy-agents': agentsBusy, fn: 'applyStateClasses', kind: snapshot && snapshot.kind }); + if (window.ATRACE) window.atrace('class.apply', sessionId, { el: item.id || null, 'response-ready': ready, 'cli-busy': busy, 'has-busy-agents': agentsBusy, 'is-alive': alive, fn: 'applyStateClasses', kind: snapshot && snapshot.kind }); } // One icon slot per row, written here and nowhere else — see .ai/contexts/session-state.md diff --git a/public/sidebar.js b/public/sidebar.js index c942df6c..a92ad56e 100644 --- a/public/sidebar.js +++ b/public/sidebar.js @@ -1224,7 +1224,7 @@ function rebindSidebarEvents(projects) { if (stopBtn) { stopBtn.onclick = (e) => { e.stopPropagation(); - confirmAndStopSession(session.sessionId); + confirmAndStopSession(session.sessionId, stopBtn); }; } @@ -1330,6 +1330,8 @@ function buildSessionItem(session) { setResponseReady(item, responseReadySessions.has(session.sessionId)); setCliBusy(item, !!sessionBusyState.get(session.sessionId)); setHasBusyAgents(item, parentHasActiveSubagent(session.sessionId)); + // initial-paint mirror of the remote-ssh adapter's liveness — see .ai/contexts/session-state.md + setIsAlive(item, !!session.remoteDescriptorSeen); if (window.ATRACE && item.className !== 'session-item js-stateful') window.atrace('class.render', session.sessionId, { el: item.id, cls: item.className, fn: 'buildSessionItem' }); item.dataset.sessionId = session.sessionId; if (session.remoteAlias) item.dataset.remoteAlias = session.remoteAlias; diff --git a/public/style.css b/public/style.css index 2c451307..a05f7690 100644 --- a/public/style.css +++ b/public/style.css @@ -1210,7 +1210,8 @@ body { display: flex; flex-direction: column; } display: none; } -.session-item.has-running-pty .session-stop-btn { +.session-item.has-running-pty .session-stop-btn, +.session-item.is-alive .session-stop-btn { display: flex; } diff --git a/remote-stop.js b/remote-stop.js index 3e348cd0..4a9586ab 100644 --- a/remote-stop.js +++ b/remote-stop.js @@ -26,9 +26,13 @@ function buildRefusalGuard(pid) { return `alive=$(${check}); if [ "$alive" != "1" ]; then echo ${NOT_CLAUDE_MARKER}; exit ${NOT_CLAUDE_EXIT_CODE}; fi;`; } +// shared /proc poll, no signal sent here — see .ai/contexts/session-state.md +function buildDeathPoll(pid) { + return `i=0; while [ -d /proc/${pid} ] && [ $i -lt ${TERM_WAIT_TICKS} ]; do sleep ${TERM_WAIT_STEP_S}; i=$((i+1)); done;`; +} + function buildKillByPidSegment(pid) { - return `kill -TERM ${pid} 2>/dev/null; i=0; ` + - `while [ -d /proc/${pid} ] && [ $i -lt ${TERM_WAIT_TICKS} ]; do sleep ${TERM_WAIT_STEP_S}; i=$((i+1)); done; ` + + return `kill -TERM ${pid} 2>/dev/null; ${buildDeathPoll(pid)} ` + `if [ -d /proc/${pid} ]; then kill -KILL ${pid} 2>/dev/null; echo ${PID_FORCE_MARKER}; else echo ${PID_TERM_MARKER}; fi; exit 0`; } @@ -51,8 +55,11 @@ function buildStopCommand(pid, tmuxTarget) { const tmuxMarker = hasPane ? TMUX_PANE_KILLED_MARKER : TMUX_WINDOW_KILLED_MARKER; const sockDiscovery = `sock=$(tr '\\0' '\\n' < /proc/${pid}/environ 2>/dev/null | grep -m1 '^TMUX=' | cut -d= -f2- | cut -d, -f1);`; + // confirm death before declaring success — see .ai/contexts/session-state.md + const deathPoll = buildDeathPoll(pid); return `${guard} ${sockDiscovery} ` + - `if [ -n "$sock" ] && tmux -S "$sock" ${tmuxSubcommand} -t ${tmuxTarget} 2>/dev/null; then echo ${tmuxMarker}; exit 0; fi; ` + + `if [ -n "$sock" ] && tmux -S "$sock" ${tmuxSubcommand} -t ${tmuxTarget} 2>/dev/null; then ` + + `${deathPoll} if [ ! -d /proc/${pid} ]; then echo ${tmuxMarker}; exit 0; fi; fi; ` + killByPid; } diff --git a/test/confirm-and-stop-session.test.js b/test/confirm-and-stop-session.test.js new file mode 100644 index 00000000..6bf8a1e8 --- /dev/null +++ b/test/confirm-and-stop-session.test.js @@ -0,0 +1,172 @@ +// Tests for app.js's confirmAndStopSession — a failed stop must not touch +// local state (activePtyIds, the open terminal view) and must surface the +// failure on the clicked button instead of alert(). See +// .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop"). +// +// app.js cannot be eval-ed in jsdom (module-scope `new ViewerPanel(...)` etc. +// — see test/running-indicators.test.js's file header for the full reason). +// `makeConfirmAndStopSession` below is therefore a HAND-MAINTAINED MIRROR of +// the real function, not the shipped code — it pins the *decision* logic in +// isolation. The source-level pin test at the bottom catches the one +// regression that matters most (the guard silently dropped from the shipped +// file) without needing a full eval — same two-layer technique already used +// in test/running-indicators.test.js. + +'use strict'; +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const APP_SRC = fs.readFileSync(path.join(__dirname, '..', 'public', 'app.js'), 'utf8'); + +// Mirrors public/app.js's confirmAndStopSession(sessionId, btn), with every +// external dependency injected instead of read off globals. +function makeConfirmAndStopSession(deps) { + return async function confirmAndStopSession(sessionId, btn) { + const plan = deps.resolveSessionStop(deps.sessionMap.get(sessionId)); + if (!deps.confirm(plan.confirmText)) return; + const result = plan.remote + ? await deps.api.remoteStopSession(plan.alias, sessionId) + : await deps.api.stopSession(sessionId); + if (result && result.ok === false) { + const message = result.error || 'unknown error'; + deps.logError('[stop-session]', message); + if (btn) { + if (typeof deps.flashButtonText === 'function') deps.flashButtonText(btn, 'Failed', 1500); + const originalTitle = btn.title; + btn.title = message; + deps.setTimeout(() => { btn.title = originalTitle; }, 3000); + } + return; + } + if (plan.remote && typeof deps.applyRemoteStopped === 'function') { + deps.applyRemoteStopped(sessionId); + } + deps.activePtyIds.delete(sessionId); + if (!deps.gridViewActive && deps.activeSessionId === sessionId) { + deps.setActiveSession(null); + deps.closeTerminalView(); + } + }; +} + +function makeDeps(overrides = {}) { + const activePtyIds = new Set(['s1']); + return { + resolveSessionStop: (session) => (session && session.remoteAlias + ? { remote: true, alias: session.remoteAlias, confirmText: `Stop this session on ${session.remoteAlias}?` } + : { remote: false, alias: null, confirmText: 'Stop this session?' }), + sessionMap: new Map([['s1', { sessionId: 's1', remoteAlias: 'vps' }]]), + confirm: () => true, + api: { stopSession: async () => ({ ok: true }), remoteStopSession: async () => ({ ok: true }) }, + logError: () => {}, + flashButtonText: () => {}, + applyRemoteStopped: () => {}, + activePtyIds, + gridViewActive: false, + activeSessionId: 's1', + setActiveSession: () => {}, + closeTerminalView: () => {}, + setTimeout: (fn) => fn(), // fire immediately — tests don't care about the 3s delay itself + ...overrides, + }; +} + +test('a successful remote stop deletes the pty id and closes the active view', async () => { + const closed = []; + const deps = makeDeps({ + closeTerminalView: () => closed.push('closed'), + }); + const confirmAndStopSession = makeConfirmAndStopSession(deps); + + await confirmAndStopSession('s1', null); + + assert.ok(!deps.activePtyIds.has('s1'), 'activePtyIds must be cleared on success'); + assert.equal(closed.length, 1, 'the active terminal view must be closed on success'); +}); + +test('a failed remote stop must not delete the pty id (mutation target: unconditional delete)', async () => { + const closed = []; + const deps = makeDeps({ + api: { stopSession: async () => ({ ok: true }), remoteStopSession: async () => ({ ok: false, error: 'pid now belongs to a non-claude process' }) }, + closeTerminalView: () => closed.push('closed'), + }); + const confirmAndStopSession = makeConfirmAndStopSession(deps); + + await confirmAndStopSession('s1', null); + + assert.ok(deps.activePtyIds.has('s1'), 'a failed stop must leave activePtyIds untouched'); + assert.equal(closed.length, 0, 'a failed stop must not close the terminal view'); +}); + +test('a failed stop flashes the clicked button and sets its title to the error, restoring it after', async () => { + const flashCalls = []; + let restoredTitle = null; + const btn = { title: 'Stop session' }; + const deps = makeDeps({ + api: { remoteStopSession: async () => ({ ok: false, error: 'ssh: connection refused' }) }, + flashButtonText: (b, text, ms) => flashCalls.push({ b, text, ms }), + setTimeout: (fn) => { fn(); restoredTitle = btn.title; }, // simulate the restore firing later, capture it before we assert + }); + const confirmAndStopSession = makeConfirmAndStopSession(deps); + + await confirmAndStopSession('s1', btn); + + assert.equal(flashCalls.length, 1, 'flashButtonText must be called on failure'); + assert.equal(flashCalls[0].text, 'Failed'); + // btn.title was set to the error message synchronously before the restore timer fires. + assert.equal(restoredTitle, 'Stop session', 'the original title is restored after the flash window'); +}); + +test('a failed stop with no button element does not throw (terminal header stop passes a real btn, but be defensive)', async () => { + const deps = makeDeps({ api: { remoteStopSession: async () => ({ ok: false, error: 'boom' }) } }); + const confirmAndStopSession = makeConfirmAndStopSession(deps); + await assert.doesNotReject(confirmAndStopSession('s1', null)); +}); + +test('declining the confirm() dialog calls neither IPC', async () => { + const calls = []; + const deps = makeDeps({ + confirm: () => false, + api: { + stopSession: async () => { calls.push('stop'); return { ok: true }; }, + remoteStopSession: async () => { calls.push('remote-stop'); return { ok: true }; }, + }, + }); + const confirmAndStopSession = makeConfirmAndStopSession(deps); + await confirmAndStopSession('s1', null); + assert.deepEqual(calls, []); +}); + +// --------------------------------------------------------------------------- +// Source-level pin for the REAL public/app.js. +// --------------------------------------------------------------------------- + +test('public/app.js: confirmAndStopSession still exists with the (sessionId, btn) signature', () => { + assert.match(APP_SRC, /async function confirmAndStopSession\(sessionId,\s*btn\)/, + 'confirmAndStopSession must accept the clicked button as its second argument'); +}); + +test('public/app.js: a failed stop returns before touching activePtyIds or closing the view (mutation target: unconditional delete)', () => { + const start = APP_SRC.indexOf('async function confirmAndStopSession(sessionId, btn)'); + assert.notEqual(start, -1); + const body = APP_SRC.slice(start, start + 1200); + + const failIdx = body.indexOf("result.ok === false"); + assert.notEqual(failIdx, -1, 'the ok === false branch must still exist'); + const returnIdx = body.indexOf('return;', failIdx); + const deleteIdx = body.indexOf('activePtyIds.delete(sessionId)'); + assert.notEqual(returnIdx, -1, 'the failure branch must return before falling through to the success path'); + assert.ok(returnIdx < deleteIdx, + 'the failure branch\'s return must precede activePtyIds.delete — a failed stop must not clear it'); +}); + +test('public/app.js: a failed stop flashes the button and sets its title to the error text', () => { + const start = APP_SRC.indexOf('async function confirmAndStopSession(sessionId, btn)'); + const body = APP_SRC.slice(start, start + 1200); + assert.match(body, /window\.flashButtonText\(btn,\s*'Failed',\s*1500\)/, + 'a failed stop must flash the button, same pattern as session-delete-btn'); + assert.match(body, /btn\.title\s*=\s*message/, 'the error text must be surfaced via the button title'); + assert.doesNotMatch(body, /\balert\(/, 'no alert() for a failed stop'); +}); diff --git a/test/dom-grid-group-by-project.test.js b/test/dom-grid-group-by-project.test.js index 24695c60..8e1a9fca 100644 --- a/test/dom-grid-group-by-project.test.js +++ b/test/dom-grid-group-by-project.test.js @@ -107,6 +107,8 @@ function setupGridDom({ stored = null } = {}) { lruTouch: noop, updateRunningIndicators: noop, confirmAndStopSession: noop, + // public/session-activity-dom.js is not loaded in this minimal harness — grid-view.js calls this directly. + isSessionAlive: () => false, suspendTerminalWebgl: noop, restoreTerminalWebgl: noop, formatDate: () => '', diff --git a/test/dom-grid-sidebar-prune-collision.test.js b/test/dom-grid-sidebar-prune-collision.test.js index 2c15599d..e0099ca3 100644 --- a/test/dom-grid-sidebar-prune-collision.test.js +++ b/test/dom-grid-sidebar-prune-collision.test.js @@ -75,6 +75,8 @@ function setupCombinedDom() { setResponseReady: (el, on) => { if (el) el.classList.toggle('response-ready', !!on); }, setCliBusy: (el, on) => { if (el) el.classList.toggle('cli-busy', !!on); }, setHasBusyAgents: (el, on) => { if (el) el.classList.toggle('has-busy-agents', !!on); }, + setIsAlive: (el, on) => { if (el) el.classList.toggle('is-alive', !!on); }, + isSessionAlive: () => false, // issue #246 step 3b — sidebar.js calls this to paint the .session-icon // slot; a no-op here is fine, this harness doesn't assert on it. paintSessionIcon: () => {}, diff --git a/test/dom-subagent-transcript.test.js b/test/dom-subagent-transcript.test.js index 480b1929..f6c088e3 100644 --- a/test/dom-subagent-transcript.test.js +++ b/test/dom-subagent-transcript.test.js @@ -123,6 +123,8 @@ function setupDom({ readSubagentJsonlResult = { entries: SAMPLE_ENTRIES }, readS setResponseReady: (el, on) => { if (el) el.classList.toggle('response-ready', !!on); }, setCliBusy: (el, on) => { if (el) el.classList.toggle('cli-busy', !!on); }, setHasBusyAgents: (el, on) => { if (el) el.classList.toggle('has-busy-agents', !!on); }, + setIsAlive: (el, on) => { if (el) el.classList.toggle('is-alive', !!on); }, + isSessionAlive: () => false, // issue #246 step 3b — sidebar.js calls this to paint the .session-icon // slot; a no-op here is fine, this harness doesn't assert on it. paintSessionIcon: () => {}, diff --git a/test/dom-subagent-ttl-tick.test.js b/test/dom-subagent-ttl-tick.test.js index 2f630594..e8146de8 100644 --- a/test/dom-subagent-ttl-tick.test.js +++ b/test/dom-subagent-ttl-tick.test.js @@ -72,6 +72,8 @@ function setupCombinedDom() { setResponseReady: (el, on) => { if (el) el.classList.toggle('response-ready', !!on); }, setCliBusy: (el, on) => { if (el) el.classList.toggle('cli-busy', !!on); }, setHasBusyAgents: (el, on) => { if (el) el.classList.toggle('has-busy-agents', !!on); }, + setIsAlive: (el, on) => { if (el) el.classList.toggle('is-alive', !!on); }, + isSessionAlive: () => false, // issue #246 step 3b — sidebar.js calls this to paint the .session-icon // slot; a no-op here is fine, this harness doesn't assert on it. paintSessionIcon: () => {}, diff --git a/test/main-wiring-source-check.test.js b/test/main-wiring-source-check.test.js index b054f2c7..f72465b5 100644 --- a/test/main-wiring-source-check.test.js +++ b/test/main-wiring-source-check.test.js @@ -191,6 +191,23 @@ test('main-wiring source check: stop-session kills through killPty', () => { ); }); +test('main-wiring source check: remote-stop-session only drops/refreshes/kills on a successful stop', () => { + const args = argsOf("ipcMain.handle('remote-stop-session'", 'ipcMain.handle'); + const guardIdx = args.indexOf('if (result.ok)'); + assert.notEqual(guardIdx, -1, 'remote-stop-session must gate its cleanup on result.ok'); + const before = args.slice(0, guardIdx); + const after = args.slice(guardIdx); + for (const marker of ['dropRemoteSession(', 'refreshHostNow(', 'killPty(']) { + assert.ok(!before.includes(marker), + `${marker} must not run before the result.ok guard — a failed stop must not drop or refresh anything`); + assert.ok(after.includes(marker), `${marker} must run inside the result.ok guard`); + } + assert.match( + after, /refreshHostNow\([^;]*\{\s*force:\s*true\s*\}/, + 'the post-stop reconciliation must force a host refresh', + ); +}); + test('main-wiring source check: the keystroke path writes through pty-ops', () => { const inputSrc = fs.readFileSync(path.join(__dirname, '..', 'terminal-input.js'), 'utf8'); assert.doesNotMatch( diff --git a/test/remote-stop-reachability.test.js b/test/remote-stop-reachability.test.js new file mode 100644 index 00000000..dbd36993 --- /dev/null +++ b/test/remote-stop-reachability.test.js @@ -0,0 +1,190 @@ +// A remote row whose process is alive on the host must show the stop control +// even when nobody has attached to it — session.remoteDescriptorSeen / +// snapshot.liveness === 'alive' is a fact independent of attach state. See +// .ai/contexts/session-state.md ("The two lifecycle verbs: detach and stop"). +// +// Three layers are covered: +// 1. The CSS "show rule" itself (public/style.css) — a real cascade test, +// not a source-grep, so a selector typo would also be caught. +// 2. The class writer (public/session-activity-dom.js's applyStateClasses), +// via the real remote-ssh adapter — same technique as +// test/remote-session-adapter.test.js. +// 3. The sidebar's initial paint (public/sidebar.js's buildSessionItem), +// via the real dom-setup.js harness, so a first render (before any +// live event fires) is covered too. +// 4. Source pins for grid-view.js / app.js, which cannot be eval'd in +// jsdom (see test/running-indicators.test.js's file header) — same +// pin technique used there. + +'use strict'; +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { JSDOM } = require('jsdom'); +const { setupSidebarDom, makeSampleProject } = require('./dom-setup'); + +const STYLE_PATH = path.join(__dirname, '..', 'public', 'style.css'); +const APP_SRC = fs.readFileSync(path.join(__dirname, '..', 'public', 'app.js'), 'utf8'); +const GRID_SRC = fs.readFileSync(path.join(__dirname, '..', 'public', 'grid-view.js'), 'utf8'); + +// --- 1. The CSS show rule, real cascade ------------------------------------- + +test('style.css: .session-stop-btn shows for .is-alive alone, not only .has-running-pty (mutation target: dropping .is-alive from the show rule)', () => { + const css = fs.readFileSync(STYLE_PATH, 'utf8'); + const dom = new JSDOM( + ` +
+ `, + { pretendToBeVisual: true, url: 'http://localhost/' }, + ); + const { window } = dom; + const item = window.document.querySelector('.session-item'); + const btn = window.document.querySelector('.session-stop-btn'); + + assert.equal(window.getComputedStyle(btn).display, 'none', 'hidden by default'); + + item.classList.add('has-running-pty'); + assert.equal(window.getComputedStyle(btn).display, 'flex', 'has-running-pty still shows it (unchanged behavior)'); + item.classList.remove('has-running-pty'); + + item.classList.add('is-alive'); + assert.equal(window.getComputedStyle(btn).display, 'flex', + 'is-alive alone must also show it — an unattached-but-alive remote row'); + + window.close(); +}); + +// --- 2. The class writer, via the real remote-ssh adapter ------------------- + +const STATE_SRC = path.join(__dirname, '..', 'public', 'session-state.js'); +const DOM_SRC = path.join(__dirname, '..', 'public', 'session-activity-dom.js'); +const ACTIVITY_SRC = path.join(__dirname, '..', 'public', 'session-activity.js'); +const REMOTE_SRC = path.join(__dirname, '..', 'public', 'remote-activity-ui.js'); + +function setupRemoteAdapter(sessionIds = ['s1']) { + const items = sessionIds + .map(id => `
`) + .join(''); + const dom = new JSDOM(`${items}`, + { url: 'http://localhost/', runScripts: 'outside-only' }); + const { window } = dom; + Object.defineProperty(window, 'activeSessionId', { value: null, writable: true, configurable: true }); + Object.defineProperty(window, 'api', { value: { onRemoteActivity: () => {} }, writable: true, configurable: true }); + Object.defineProperty(window, 'setTimeout', { value: () => 1, writable: true, configurable: true }); + Object.defineProperty(window, 'clearTimeout', { value: () => {}, writable: true, configurable: true }); + + const ctx = dom.getInternalVMContext(); + vm.runInContext(fs.readFileSync(STATE_SRC, 'utf8'), ctx, { filename: STATE_SRC }); + vm.runInContext(fs.readFileSync(DOM_SRC, 'utf8'), ctx, { filename: DOM_SRC }); + vm.runInContext(fs.readFileSync(ACTIVITY_SRC, 'utf8'), ctx, { filename: ACTIVITY_SRC }); + vm.runInContext(fs.readFileSync(REMOTE_SRC, 'utf8'), ctx, { filename: REMOTE_SRC }); + + const call = (fnName, arg) => vm.runInContext(`${fnName}(${JSON.stringify(arg)})`, ctx); + return { + window, + item: (id) => window.document.querySelector(`.session-item[data-session-id="${id}"]`), + applyRemoteDescriptor: (session) => call('applyRemoteDescriptor', session), + applyRemoteStopped: (id) => call('applyRemoteStopped', id), + destroy: () => window.close(), + }; +} + +test('applyRemoteDescriptor marks an alive, unattached row .is-alive', () => { + const t = setupRemoteAdapter(['s1']); + t.applyRemoteDescriptor({ sessionId: 's1', remoteAlias: 'planificator', remoteDescriptorSeen: true, status: 'idle' }); + assert.ok(t.item('s1').classList.contains('is-alive'), 'a matched descriptor must mark the row is-alive'); + t.destroy(); +}); + +test('applyRemoteDescriptor leaves a row not is-alive when the descriptor was not seen', () => { + const t = setupRemoteAdapter(['s1']); + t.applyRemoteDescriptor({ sessionId: 's1', remoteAlias: 'planificator', remoteDescriptorSeen: false, status: null }); + assert.ok(!t.item('s1').classList.contains('is-alive'), 'liveness stays unknown, never treated as alive'); + t.destroy(); +}); + +test('applyRemoteStopped clears is-alive once the host confirms the process is gone', () => { + const t = setupRemoteAdapter(['s1']); + t.applyRemoteDescriptor({ sessionId: 's1', remoteAlias: 'planificator', remoteDescriptorSeen: true, status: 'idle' }); + assert.ok(t.item('s1').classList.contains('is-alive'), 'precondition: alive before the stop'); + t.applyRemoteStopped('s1'); + assert.ok(!t.item('s1').classList.contains('is-alive'), 'a successful stop must clear is-alive immediately'); + t.destroy(); +}); + +// --- 3. The sidebar's initial paint (first render, no prior live event) ---- + +function remoteProject(session) { + return makeSampleProject({ + projectPath: '/srv/supervision', + folder: 'planificator::-srv-supervision', + remoteAlias: 'planificator', + sessions: [session], + }); +} + +test('buildSessionItem: an unattached remote session with a live descriptor paints is-alive on first render', () => { + const ctx = setupSidebarDom(); + try { + const session = { + sessionId: 'remote-alive', summary: 'alive, unattached', modified: '2026-09-06T10:00:00.000Z', + starred: false, archived: 0, messageCount: 1, + remoteAlias: 'planificator', remoteDescriptorSeen: true, status: 'idle', + }; + ctx.sidebar.renderProjects([remoteProject(session)], true); + + const item = ctx.document.querySelector('#si-remote-alive'); + assert.ok(item, 'the row must exist'); + assert.ok(item.classList.contains('is-alive'), 'a live, unattached remote row must show is-alive on its very first paint'); + } finally { ctx.destroy(); } +}); + +test('buildSessionItem: a remote session with no matching descriptor is not painted is-alive', () => { + const ctx = setupSidebarDom(); + try { + const session = { + sessionId: 'remote-dead', summary: 'no live process', modified: '2026-09-06T10:00:00.000Z', + starred: false, archived: 0, messageCount: 1, + remoteAlias: 'planificator', remoteDescriptorSeen: false, status: null, + }; + ctx.sidebar.renderProjects([remoteProject(session)], true); + + const item = ctx.document.querySelector('#si-remote-dead'); + assert.ok(item); + assert.ok(!item.classList.contains('is-alive'), 'no descriptor match must never be painted is-alive'); + } finally { ctx.destroy(); } +}); + +test('buildSessionItem: a plain local session is never painted is-alive by this path', () => { + const ctx = setupSidebarDom(); + try { + const project = makeSampleProject({ + sessions: [{ + sessionId: 'local-1', summary: 'local work', modified: '2026-09-06T10:00:00.000Z', + starred: false, archived: 0, messageCount: 2, + }], + }); + ctx.sidebar.renderProjects([project], true); + + const item = ctx.document.querySelector('#si-local-1'); + assert.ok(item); + assert.ok(!item.classList.contains('is-alive')); + } finally { ctx.destroy(); } +}); + +// --- 4. grid-view.js / app.js source pins (cannot be eval'd in jsdom — see +// test/running-indicators.test.js's file header for why) -------------------- + +test('public/grid-view.js: the stop button visibility check also honors isSessionAlive (mutation target: reverting to activePtyIds alone)', () => { + const marker = "stopBtn.style.display = (activePtyIds.has(sessionId) || isSessionAlive(sessionId)) ? '' : 'none';"; + assert.ok(GRID_SRC.includes(marker), + 'wrapInGridCard must show the stop button for an alive-but-unattached remote row too'); +}); + +test('public/app.js: updateRunningIndicators\' grid-card stop button also honors isSessionAlive (mutation target: reverting to running alone)', () => { + const marker = "if (stopBtn) stopBtn.style.display = (running || isSessionAlive(sid)) ? '' : 'none';"; + assert.ok(APP_SRC.includes(marker), + 'the periodic grid-card refresh must not un-hide the stop button only for running local sessions'); +}); diff --git a/test/remote-stop.test.js b/test/remote-stop.test.js index de2434d9..04f1435b 100644 --- a/test/remote-stop.test.js +++ b/test/remote-stop.test.js @@ -83,6 +83,26 @@ test('buildStopCommand waits roughly 3s before escalating to kill -KILL', () => assert.match(cmd, /-lt 6/, '6 ticks of 0.5s bounds the wait at ~3s'); }); +test('buildStopCommand: a successful tmux kill-pane still polls /proc before declaring success (mutation target: dropping the post-tmux poll)', () => { + const cmd = buildStopCommand(4242, 'main:@0.%0'); + const killPaneIdx = cmd.indexOf('kill-pane -t main:@0.%0'); + const thenIdx = cmd.indexOf('then', killPaneIdx); + const markerIdx = cmd.indexOf(TMUX_PANE_KILLED_MARKER, thenIdx); + const pollIdx = cmd.indexOf('while [ -d /proc/4242 ]', thenIdx); + assert.ok(thenIdx > killPaneIdx && markerIdx > thenIdx, 'sanity: tmux kill precedes its own success marker'); + assert.ok(pollIdx > thenIdx && pollIdx < markerIdx, + 'a /proc poll must run between the tmux kill and its success marker'); + assert.match(cmd.slice(thenIdx, markerIdx), /if \[ ! -d \/proc\/4242 \]/, + 'the tmux success marker must be gated on the pid actually being gone, not just on tmux\'s exit code'); +}); + +test('buildStopCommand: a tmux-killed pid that survives the poll falls through to the pid-kill path (mutation target: dropping the fallthrough)', () => { + const cmd = buildStopCommand(4242, 'main:@0.%0'); + const tail = cmd.slice(cmd.indexOf('kill-pane')); + assert.match(tail, /kill -TERM 4242 2>\/dev\/null;[\s\S]*kill -KILL 4242/, + 'the pid TERM->KILL fallback must still be reachable after a tmux kill whose target never actually dies'); +}); + test('no builder ever emits a backtick', () => { assert.ok(!buildStopCommand(4242, null).includes('`')); assert.ok(!buildStopCommand(4242, 'main:@0.%0').includes('`')); diff --git a/test/terminal-manager-harness.js b/test/terminal-manager-harness.js index e29a879c..00c87789 100644 --- a/test/terminal-manager-harness.js +++ b/test/terminal-manager-harness.js @@ -146,6 +146,8 @@ function setupTerminalDom(opts = {}) { hideAllViewers: () => {}, showTerminalHeader: () => {}, updateRunningIndicators: () => {}, + // public/session-activity-dom.js is not loaded in this harness — grid-view.js calls this directly. + isSessionAlive: () => false, placeholder: window.document.createElement('div'), terminalHeader: window.document.createElement('div'), gridViewer: window.document.createElement('div'),