From 153ecd4d36f7e29a638b85f55a41a2d10d46df1e Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Wed, 23 Sep 2026 16:30:13 +0200 Subject: [PATCH 1/5] fix(cleanup): keep .claude-images while a sibling session uses the same dir cleanupSession() recursively removes {workingDir}/.claude-images. That directory belongs to the working directory rather than to the session, and several sessions routinely share one case directory, so closing one session deleted the pasted images a live sibling still referred to. The removal now runs only when no other live session has the same working directory. A session that is itself being cleaned up does not count as live, so two sessions of one case closed together still remove the dir. Split out ahead of the exited-agent sweep for Ark0N/Codeman#446, which closes sessions unattended and would otherwise make the loss routine. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/paste-image-gc.ts | 34 ++++++++- src/web/server.ts | 12 +++- test/paste-image-dir-shared.test.ts | 108 ++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 test/paste-image-dir-shared.test.ts diff --git a/src/web/paste-image-gc.ts b/src/web/paste-image-gc.ts index 0711d3e7e..7325bdff5 100644 --- a/src/web/paste-image-gc.ts +++ b/src/web/paste-image-gc.ts @@ -12,7 +12,7 @@ * image dir. */ import fs from 'node:fs/promises'; -import { join } from 'node:path'; +import { join, resolve } from 'node:path'; import type { SessionPort } from './ports/index.js'; const MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; // 7 days @@ -53,6 +53,38 @@ export async function sweepPasteImagesOnce( return { scanned, deleted }; } +/** + * Does another live session still use this working directory's paste-image + * dir? Deleting a session removes `{workingDir}/.claude-images` recursively, + * and several sessions routinely share one case directory, so without this + * check closing one session deletes the pasted images a sibling in the same + * case still refers to. + * + * A session that is itself being cleaned up does not count as live. Without + * that exemption, closing two sessions of one case concurrently (a bulk + * delete, or the exited-agent sweep closing two panes on one tick) would have + * each defer to the other, and neither would remove the dir. + * + * Paths are compared after `resolve()`, which normalises a trailing slash and + * `..` segments. Symlinks are not resolved: a sibling that reaches the same + * directory through a symlink only costs a missed deletion here, and the + * periodic sweep above still ages those files out. + */ +export function pasteImageDirInUseByOtherSession( + sessions: Iterable<{ id: string; workingDir: string }>, + closingId: string, + workingDir: string, + closing: ReadonlySet +): boolean { + const target = resolve(workingDir); + for (const session of sessions) { + if (session.id === closingId || closing.has(session.id)) continue; + if (!session.workingDir) continue; + if (resolve(session.workingDir) === target) return true; + } + return false; +} + export function startPasteImageGc(ctx: Pick): () => void { const initial = setTimeout(() => { void sweepPasteImagesOnce(ctx); diff --git a/src/web/server.ts b/src/web/server.ts index efcaded6b..0ead5c5bd 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -33,7 +33,7 @@ import fastifyCookie from '@fastify/cookie'; import fastifyStatic from '@fastify/static'; import fastifyWebsocket from '@fastify/websocket'; import fastifyMultipart from '@fastify/multipart'; -import { startPasteImageGc } from './paste-image-gc.js'; +import { pasteImageDirInUseByOtherSession, startPasteImageGc } from './paste-image-gc.js'; import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; import { existsSync, mkdirSync, readFileSync, chmodSync, rmSync, statSync } from 'node:fs'; @@ -1482,8 +1482,14 @@ export class WebServer extends EventEmitter { attachmentRegistry.clearSession(sessionId); // Stop watching for images in this session's directory imageWatcher.unwatchSession(sessionId); - // Clean up pasted images directory for this session - if (killMux && session.workingDir) { + // Clean up pasted images directory for this session. The dir belongs to the + // working directory rather than the session, so it stays while another live + // session in the same case still uses it (Ark0N/Codeman#446). + if ( + killMux && + session.workingDir && + !pasteImageDirInUseByOtherSession(this.sessions.values(), sessionId, session.workingDir, this.cleaningUp) + ) { const pasteImageDir = join(session.workingDir, '.claude-images'); try { rmSync(pasteImageDir, { recursive: true, force: true }); diff --git a/test/paste-image-dir-shared.test.ts b/test/paste-image-dir-shared.test.ts new file mode 100644 index 000000000..71c6cfdc3 --- /dev/null +++ b/test/paste-image-dir-shared.test.ts @@ -0,0 +1,108 @@ +/** + * @fileoverview Deleting a session keeps `.claude-images` while a sibling in the + * same working directory is still live (Ark0N/Codeman#446). + * + * `cleanupSession()` removes `{workingDir}/.claude-images` recursively. That + * dir belongs to the working directory, not to the session, and several + * sessions routinely share one case directory, so closing one used to delete + * the pasted images a live sibling still referred to. The exited-agent sweep + * closes sessions unattended, which turns that from an occasional loss into a + * routine one. + * + * Port: 3188 + */ +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { WebServer } from '../src/web/server.js'; +import { pasteImageDirInUseByOtherSession } from '../src/web/paste-image-gc.js'; + +const PORT = 3188; + +describe('pasteImageDirInUseByOtherSession', () => { + const none = new Set(); + + it('finds a live sibling in the same working directory', () => { + const sessions = [ + { id: 'a', workingDir: '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/work/case' }, + { id: 'b', workingDir: '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/work/case' }, + ]; + expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', none)).toBe(true); + }); + + it('ignores the session being closed', () => { + const sessions = [{ id: 'a', workingDir: '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/work/case' }]; + expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', none)).toBe(false); + }); + + it('ignores sessions in other directories, including a subdirectory', () => { + const sessions = [ + { id: 'a', workingDir: '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/work/case' }, + { id: 'b', workingDir: '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/work/other' }, + { id: 'c', workingDir: '/work/case/sub' }, + ]; + expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', none)).toBe(false); + }); + + it('normalises a trailing slash and dot segments', () => { + const sessions = [ + { id: 'a', workingDir: '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/work/case' }, + { id: 'b', workingDir: '/work/x/../case/' }, + ]; + expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', none)).toBe(true); + }); + + it('does not count a sibling that is being closed too', () => { + // Two sessions of one case closed together must not each defer to the + // other, or neither removes the dir. + const sessions = [ + { id: 'a', workingDir: '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/work/case' }, + { id: 'b', workingDir: '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/work/case' }, + ]; + expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', new Set(['a', 'b']))).toBe(false); + }); +}); + +describe('deleting a session that shares its working directory', () => { + let server: WebServer; + let workingDir: string; + const base = `http://localhost:${PORT}`; + + beforeAll(async () => { + workingDir = mkdtempSync(join(tmpdir(), 'codeman-paste-shared-')); + server = new WebServer(PORT, false, true); + await server.start(); + }); + + afterAll(async () => { + await server.stop(); + rmSync(workingDir, { recursive: true, force: true }); + }, 60000); + + const create = async (): Promise => { + const res = await fetch(`${base}/api/sessions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ workingDir }), + }); + const body = await res.json(); + return body.data.session.id as string; + }; + + const remove = (id: string) => fetch(`${base}/api/sessions/${id}`, { method: 'DELETE' }); + + it('keeps the images while a sibling is live, and removes them with the last session', async () => { + const first = await create(); + const second = await create(); + const imageDir = join(workingDir, '.claude-images'); + mkdirSync(imageDir, { recursive: true }); + writeFileSync(join(imageDir, 'paste-1.png'), 'x'); + + expect((await remove(first)).status).toBe(200); + expect(existsSync(join(imageDir, 'paste-1.png'))).toBe(true); + + expect((await remove(second)).status).toBe(200); + expect(existsSync(imageDir)).toBe(false); + }); +}); From 7626539f88ef76a6ffc21e0c14fbecacc0bf388d Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Wed, 23 Sep 2026 16:36:47 +0200 Subject: [PATCH 2/5] feat(session): close sessions whose agent exited cleanly (#446) Part 2 of Ark0N/Codeman#446. Part 1 records an exited agent as SessionState.paneExit. A session whose agent the user ended with /exit is now closed through cleanupSession(), the same path the X button takes, so finished sessions stop piling up on the board. The lifecycle log records the reason as "agent exited cleanly (status 0)", and the conversation stays resumable from the Resume list. shouldCloseCleanlyExitedSession() in the new pure module pane-exit-sweep.ts holds the rule. It closes a session only when all of these hold: - The exit status is an explicit numeric 0 with no signal. An absent status is how a SIGKILL presents on tmux 3.2a, so it counts as unknown and the row stays. A non-zero status or any signal also keeps the row, with the exit code on the tab. - Two authoritative pane reads agreed on that exit. TmuxManager.getPaneExitReadCount() counts them, and a failed, empty or skipped read neither confirms nor resets the count. - No start, attach or relaunch is running for the pane. Session.paneLifecycleInFlight covers _setupOrAttachMuxSession(), whose dead-pane branch revives an exited pane on purpose, and restartCli(). setPaneExit() already scopes paneExit to local mux-backed sessions, so remote, docker and direct-PTY sessions are never closed. planRebootRestore() now refuses a record whose persisted paneExit is a clean exit. That covers an agent that exited just before a reboot, before the sweep reached it. A crashed agent's record stays eligible, like its row. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 8 ++ src/mux-interface.ts | 8 ++ src/pane-exit-sweep.ts | 74 ++++++++++ src/reboot-restore.ts | 41 ++++-- src/session.ts | 40 ++++++ src/tmux-manager.ts | 30 +++- src/types/session.ts | 13 +- src/web/server.ts | 44 ++++++ test/pane-exit-sweep.test.ts | 240 ++++++++++++++++++++++++++++++++ test/reboot-restore.test.ts | 19 +++ test/tmux-manager.test.ts | 45 ++++++ 12 files changed, 541 insertions(+), 23 deletions(-) create mode 100644 src/pane-exit-sweep.ts create mode 100644 test/pane-exit-sweep.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 1257c6897..97f2fcd14 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -205,7 +205,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph ⚠️ **A quiet pane is not always a pane that wants you.** A CLI can declare an optional `capabilities.workDetect.watchingLine` (a monitor, background shell or cloud hand-off it is still running); the idle probe reads it into `Session.watching` and `notePrompt()` opens that idle item ALREADY acknowledged, so no surface alerts. Only `idle` is eligible, and the label is pane-derived and prompt-injectable, so a pattern must anchor on chrome only that CLI draws. → [architecture-invariants#the-watching-signal-a-quiet-pane-that-is-not-waiting-for-you](docs/architecture-invariants.md#the-watching-signal-a-quiet-pane-that-is-not-waiting-for-you). Tests: `test/session-watching.test.ts`, `test/watching-no-alert.test.ts`. -**An exited agent in a live pane** (`paneExit`, #446): panes use `remain-on-exit on`, so `/exit` leaves a pane, session and pid that look alive; `TmuxManager.startPaneExitWatcher()` publishes `SessionState.paneExit` via `session:updated`. ⚠️ Never set `status: 'error'` or null the `pid` for it; the field is TRI-STATE (absent = UNKNOWN, never alive, scoped by `Session.paneExitApplies`); an absent `#{pane_dead_status}` is not 0; a path that starts a command in a pane must clear the record AND persist. → [architecture-invariants#an-exited-agent-in-a-live-pane-paneexit](docs/architecture-invariants.md#an-exited-agent-in-a-live-pane-paneexit) +**An exited agent in a live pane** (`paneExit`, #446): panes use `remain-on-exit on`, so `/exit` leaves a pane, session and pid that look alive; `TmuxManager.startPaneExitWatcher()` publishes `SessionState.paneExit` via `session:updated`. ⚠️ Never set `status: 'error'` or null the `pid` for it; the field is TRI-STATE (absent = UNKNOWN, never alive, scoped by `Session.paneExitApplies`); an absent `#{pane_dead_status}` is not 0; a path that starts a command in a pane must clear the record AND persist. A clean exit is CLOSED via `cleanupSession()` (`pane-exit-sweep.ts`): only an explicit numeric status 0 with no signal, confirmed by 2 reads, with no start/attach in flight (`paneLifecycleInFlight`); a crashed agent keeps its row. → [architecture-invariants#an-exited-agent-in-a-live-pane-paneexit](docs/architecture-invariants.md#an-exited-agent-in-a-live-pane-paneexit) **Dead-pane respawn resume pin** (`_buildRespawnPaneOptionsWithResumePin()`, session.ts): recovering a dead pane, like a custom-model `restartCli()`, must pin the conversation or claude refuses the reused `--session-id`. The pin takes the first transcript-backed candidate (chain tail, launch seed, own id), never `_claudeSessionId`, adds nothing when none is backed, and is never applied to remote or docker sessions. → [architecture-invariants#dead-pane-respawn-the-resume-pin](docs/architecture-invariants.md#dead-pane-respawn-the-resume-pin) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index fbe15b5e3..804be7a91 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -241,6 +241,14 @@ Further detail, closing: ⚠️ **Closing has the mirror-image race and one owne **Codeman creates every pane with `remain-on-exit on`, so a session whose agent exited still looks alive.** `/exit` ends the CLI, tmux keeps the pane and the tmux session, and the `tmux attach-session` process Codeman records as `Session.pid` runs on, so no PTY exit handler fires and the record keeps its pid and `status: 'idle'` (Ark0N/Codeman#446). `SessionState.paneExit` (`{status?, signal?, at}`) is the fact tmux already knows, published through `toState()` so it rides `session:updated` and lands in `state.json` on the same persist — there is no SSE event for it. One batched `tmux list-panes -a` per tick fills it, from `TmuxManager.startPaneExitWatcher()`, which has its OWN always-on interval: the stats collector cannot carry it, because the browser arms and disarms that one with the Monitor panel (`panels-ui.js`) and boot skips it entirely when no session was recovered. ⚠️ **The field is TRI-STATE and its third state is absence**, meaning UNKNOWN, which renders as nothing and must NEVER read as alive; it covers a running pane, a session the read did not list, a failed probe, and every session shape a dead local pane does not describe. `Session.paneExitApplies` is the single place that scoping lives, and it fails closed for four shapes: a direct-PTY session (no pane), a remote SSH session (the local pane is the ssh client, whose death is a transport drop OR an exit — the whole of #355), a docker case (the local pane is a `docker exec` into the container's own tmux), and a session rebuilt from the socket (`MuxSession.discovered`: its synthetic `restored-` id matches no `state.json` entry, so a remote session rediscovered after `mux-sessions.json` was lost would arrive looking local). ⚠️ **Never set `status: 'error'`** for an exited pane — that value is the PTY-exit breaker's and the browser answers it with a "restart it?" confirm — and **never null the `pid`**, which is what makes `selectSession()` re-attach and launch a fresh CLI. Local panes keep `remain-on-exit on`; flipping them to `failed` ends the tmux session, nulls the pid and reintroduces the auto-revive #355 removed. ⚠️ **An absent `#{pane_dead_status}` is not 0**: measured on tmux 3.2a a SIGKILLed pane reports neither a status nor a signal (`#{pane_dead_signal}` did not exist before tmux 3.4), so folding it into 0 would turn an unexplained death into a clean exit. A session answers only when the read listed EXACTLY ONE pane for it, since Codeman never splits a pane and a session the user split by hand has none that speaks for the agent. The three synchronous `isPaneDead()` callers (the `/wait` route, the TUI, the attach path) keep their own probes — this watcher is never fresh enough for them. ⚠️ **A path that starts a command in a pane must clear the record AND persist**, since the watcher's next tick sees the field already cleared and writes nothing. ⚠️ **The always-on timer gates the READ, never the tick.** `hasObservablePaneSession()` (`tmux-manager.ts`) skips the tmux exec while every session on the manager is one of the shapes `paneExitApplies` forces to UNKNOWN, so an instance running only remote or Docker work keeps ticking and costs nothing; the two predicates are two copies of one rule, and `test/session-pane-exit.test.ts` pins them against each other for the four session shapes that exist today — a FIFTH condition added to one and not the other still fails nothing, so change them together. Skipping retracts nothing, for the same reason a failed read does not. ⚠️ **The muted status dot is a specificity fight, and it is fought on three surfaces.** The tab renders `status` as before, and `tab-agent-exited` only quiets the dot, so the rule excludes three states BY HAND: `.tab-alert-action` and `.tab-alert-idle` on the tab, and `.tab-status.error` on the dot itself. Each of those colours means "this needs you" — the two alerts because a human is blocked, `error` because the browser answers it with a "restart it?" confirm — and each must survive the exit. The rich tab rail needs a SECOND copy of the rule, because its own `tab-state-*` dot rules are (0,9,1) against the strip's (0,5,0) — measured, an exited session on a detailed rail kept a full green dot and the working halo beside a badge reading "exited". Its twin matches that specificity exactly and therefore must stay BELOW those rules in source order. mobile.css needs a THIRD copy, with `!important`, because the phone block enlarges a `busy` dot and gives it a green glow that way, and `status` stays `busy` for a pane whose agent died mid-turn — without it a phone renders a grey dot still wearing the green halo. `test/session-pane-exit-ui.test.ts` resolves the real stylesheets in jsdom rather than matching selector text — styles.css for the desktop cases and both files for the phone ones — so the ordering, the hand-written exclusions and a missing phone rule all fail there. Tests: `test/session-pane-exit.test.ts`, `test/tmux-manager.test.ts`, `test/session-pane-exit-ui.test.ts`. +**A session whose agent exited cleanly is closed, and a crashed one is kept** (Ark0N/Codeman#446, part 2). After every pane read, `closeCleanlyExitedSessions()` (`server.ts`) closes each session that `shouldCloseCleanlyExitedSession()` (`pane-exit-sweep.ts`, pure) accepts, through `cleanupSession(id, true, CLEAN_EXIT_CLOSE_REASON)`. That is the X button's path, so an unpinned session is removed, a pinned one is demoted to `status: 'stopped'`, the lifecycle log records why, and the conversation stays resumable from the Resume list, which reads the lifecycle log and the transcripts rather than the pane. The rule has three parts, and each guards against a wrong close: + +- ⚠️ **The status must be an explicit numeric 0 with no signal** (`isCleanPaneExit()`). An absent status is how a SIGKILL presents on tmux 3.2a, so `status ?? 0` would close an agent the OOM killer took. A non-zero status or any signal keeps the row, marked with the exit, as the crash evidence #210 was filed to keep. +- **At least `CLEAN_EXIT_CONFIRMING_READS` (2) authoritative reads must agree.** `TmuxManager.getPaneExitReadCount()` counts them. A repeat of the same pane pid, status and signal adds one, anything else starts again at 1, and a failed, empty or skipped read never reaches `applyPaneExits()`, so it neither confirms nor resets. A mux without the method never has a session closed. +- **No start, attach or relaunch may be in flight** (`Session.paneLifecycleInFlight`, raised for the whole of `_setupOrAttachMuxSession()` and `restartCli()`). The dead-pane respawn revives an exited pane on purpose, and the pane reads as dead until `clearPaneExitForNewPane()` runs after its startup delay. + +Scoping needs no check of its own here: `setPaneExit()` already forces `paneExit` to UNKNOWN for direct-PTY, remote, docker and discovered sessions. There is no setting, by the maintainer's decision on #446. ⚠️ Do not flip local panes to `remain-on-exit failed` to get the same effect: a destroyed pane ends the tmux session, the PTY exit nulls the pid, and the browser's `selectSession()` then launches a fresh CLI. `cleanupSession()` keeps `{workingDir}/.claude-images` while another live session shares that working directory (`pasteImageDirInUseByOtherSession()`, `paste-image-gc.ts`), since the sweep would otherwise routinely delete a live sibling's pasted images. `planRebootRestore()` refuses a record whose persisted `paneExit` is clean (`agent-exited`), which covers an agent that exited just before the power went, before the sweep reached it. Tests: `test/pane-exit-sweep.test.ts`, `test/paste-image-dir-shared.test.ts`, `test/reboot-restore.test.ts`, `test/tmux-manager.test.ts`. + ### Dead-pane respawn: the resume pin **The dead-pane respawn needs the same resume pin as a custom-model `restartCli()` and shares it** (`_buildRespawnPaneOptionsWithResumePin()` in `session.ts`, used by `restartCli()`, the dead-pane respawn in `_setupOrAttachMuxSession()`, and its create path when that path RELAUNCHES a CLI: after a failed respawn, or when tmux lost the whole session rather than the pane): a pane whose agent EXITED owns a transcript too, so recovering one with the bare launch line hit the same refusal and the conversation was stranded behind a tab that looked merely idle. The pin walks three candidates in order — the conversation chain's tail, the launch seed, then the session's own id — and takes the first one a transcript backs, never `_claudeSessionId` (which also holds history-correlated GUESSES keyed on the working directory, and launching from one would open and write to a conversation that was never this pane's). ⚠️ The create-path pin is written to `_resumeSessionId` as well, so unlike `restartCli()`'s one-respawn pin it PERSISTS through `toState()` as `resumeSessionId`: that field means "what the user asked to resume at creation, or what recovery pinned", and after a dead-pane respawn `_claudeSessionId` names whatever the walk actually pinned rather than the chain tail. ⚠️ A candidate no transcript backs is passed over, and falling off the end of the walk ADDS no pin (the options keep whatever launch seed they already carried): a divergent pin leaves `--session-id ` in the fallback branch, where a failed resume collides all over again, while pinning an id with no transcript prints claude's "No conversation found" into a brand-new session's scrollback and costs the running branch its `nice` priority (`wrapWithNice()` prefixes only the first branch of an `a || b`). ⚠️ Remote and docker sessions are never pinned: their pane commands are already self-healing, the conversation lives on the far side, and a local id resolves to nothing there. diff --git a/src/mux-interface.ts b/src/mux-interface.ts index f5d3124e9..8461c6608 100644 --- a/src/mux-interface.ts +++ b/src/mux-interface.ts @@ -336,6 +336,14 @@ export interface TerminalMultiplexer extends EventEmitter { */ getPaneExit?(muxName: string): PaneExit | undefined; + /** + * How many authoritative pane reads have agreed on the exit `getPaneExit()` + * reports, or 0 when it reports none. The exited-agent sweep closes a session + * only once this reaches `CLEAN_EXIT_CONFIRMING_READS` (`pane-exit-sweep.ts`), + * and a multiplexer without this method never has a session closed by it. + */ + getPaneExitReadCount?(muxName: string): number; + /** Forget a session's exit observation, e.g. once its pane has been respawned. */ clearPaneExit?(muxName: string): void; diff --git a/src/pane-exit-sweep.ts b/src/pane-exit-sweep.ts new file mode 100644 index 000000000..c334d9a81 --- /dev/null +++ b/src/pane-exit-sweep.ts @@ -0,0 +1,74 @@ +/** + * @fileoverview The exited-agent sweep's decision rule (Ark0N/Codeman#446). + * + * Codeman creates every tmux pane with `remain-on-exit on`, so `/exit` ends the + * CLI while the pane, the tmux session and the `tmux attach-session` process + * all live on. Part 1 of #446 records that as `SessionState.paneExit`. This + * module decides when such a session is closed, the way the X button closes + * it, so finished sessions stop piling up on the board. + * + * The rule closes a session only on a POSITIVE observation of a clean exit: + * + * - The exit status must be an explicit numeric 0 with no signal. An absent + * status is UNKNOWN, never 0: on tmux 3.2a a SIGKILLed pane reports neither a + * status nor a signal, so reading absence as clean would sweep an agent the + * OOM killer took. A non-zero status or any signal keeps the row, marked with + * the exit, as the crash evidence #210 was filed to keep. + * - At least {@link CLEAN_EXIT_CONFIRMING_READS} authoritative pane reads must + * have agreed on that exit. A failed, empty or skipped read counts for + * nothing, because unknown never closes anything. + * - No start, attach or relaunch may be in flight for the session. The + * dead-pane branch of `Session._setupOrAttachMuxSession()` respawns an exited + * pane on purpose, and for a few seconds that pane still reads as dead. + * + * Scoping to local mux-backed sessions happens before this rule runs: + * `Session.setPaneExit()` forces the field to UNKNOWN for direct-PTY, remote, + * docker and discovered sessions, so their `paneExit` never reaches here. + * + * Pure, so the rule is unit-tested without a server (test/pane-exit-sweep.test.ts). + */ +import type { PaneExit } from './types/index.js'; + +/** + * How many authoritative pane reads must agree on a clean exit before the + * session is closed. At the watcher's 2 s cadence two reads mean a finished + * session disappears within about four seconds of its agent exiting. + */ +export const CLEAN_EXIT_CONFIRMING_READS = 2; + +/** The lifecycle-log reason recorded when the sweep closes a session. */ +export const CLEAN_EXIT_CLOSE_REASON = 'agent exited cleanly (status 0)'; + +/** + * Is this exit a clean one? True only for an explicit numeric status of 0 with + * no signal reported. + * + * ⚠ Never widen this to `(exit.status ?? 0) === 0` or to "no signal, so it was + * clean". An absent status is how a signal death presents on tmux 3.2a, and + * that shortcut would close crashed agents with nothing failing to warn you. + */ +export function isCleanPaneExit(exit: PaneExit | undefined): boolean { + if (!exit) return false; + if (exit.signal !== undefined) return false; + return exit.status === 0; +} + +/** Everything the sweep needs to know about one session. */ +export interface CleanExitSweepCandidate { + /** The session's published exit, already scoped by `Session.setPaneExit()`. */ + paneExit: PaneExit | undefined; + /** Authoritative pane reads that agreed on that exit (`getPaneExitReadCount()`). */ + confirmingReads: number; + /** A start, attach or relaunch is running for this session's pane. */ + paneLifecycleInFlight: boolean; + /** The session is already being closed or detached. */ + closing: boolean; +} + +/** Should the sweep close this session now? See the file overview for the rule. */ +export function shouldCloseCleanlyExitedSession(candidate: CleanExitSweepCandidate): boolean { + if (candidate.closing) return false; + if (candidate.paneLifecycleInFlight) return false; + if (!isCleanPaneExit(candidate.paneExit)) return false; + return candidate.confirmingReads >= CLEAN_EXIT_CONFIRMING_READS; +} diff --git a/src/reboot-restore.ts b/src/reboot-restore.ts index 40b29b082..055bd1a38 100644 --- a/src/reboot-restore.ts +++ b/src/reboot-restore.ts @@ -19,19 +19,22 @@ * touching its status, so a pinned session a reboot killed still reads `idle` or * `busy` and stays eligible. * - * ⚠️ Ending the AGENT rather than the session is a shape this module CANNOT - * recognise today, and a reboot restores it. `/exit` ends the CLI inside the - * pane, `remain-on-exit` keeps the pane, and the PTY Codeman owns is the - * `tmux attach-session` process, which stays alive throughout — so no exit - * handler runs, no lifecycle `exit` is logged, and the record keeps both its pid - * and `status: 'idle'`. Nothing durable distinguishes it from a session that was - * simply idle when the power went. Ark0N/Codeman#446 covers making Codeman - * notice the dead pane; until a record can say the agent is gone, this pass will - * offer those sessions back, and the user dismisses or closes them. + * ⚠️ Ending the AGENT rather than the session leaves no trace in `status` or + * `pid`. `/exit` ends the CLI inside the pane, `remain-on-exit` keeps the pane, + * and the PTY Codeman owns is the `tmux attach-session` process, which stays + * alive throughout — so no exit handler runs, no lifecycle `exit` is logged, + * and the record keeps both its pid and `status: 'idle'`. Ark0N/Codeman#446 + * handles it in two steps. The pane-exit watcher persists `paneExit`, and the + * clean-exit sweep (`pane-exit-sweep.ts`) closes a session whose agent exited + * with status 0 through `cleanupSession()`, which leaves the durable record + * described above. This module also refuses a record whose persisted + * `paneExit` is a clean exit, which covers a session that exited moments + * before the power went, before the sweep reached it. A crashed agent's record + * stays eligible, like the row the sweep leaves on the board for it. * - * The `pid` check below is therefore NOT that rule. It refuses a record whose - * attach process was already gone, which is a session that never started or - * whose pane died outright. + * The `pid` check below is NOT that rule. It refuses a record whose attach + * process was already gone, which is a session that never started or whose + * pane died outright. * * @dependencies types (SessionState), config/cli-registry * @consumedby web/server (plan build at boot), web/routes/reboot-restore-routes @@ -41,6 +44,7 @@ import type { SessionState } from './types.js'; import { getCli } from './config/cli-registry/registry.js'; +import { isCleanPaneExit } from './pane-exit-sweep.js'; /** Session statuses a reboot restore may rebuild. `stopped` is the kill marker. */ const RESTORABLE_STATUSES: ReadonlySet = new Set(['idle', 'busy', 'error']); @@ -109,7 +113,7 @@ export function resolveResumeConversationId(state: SessionState): string { /** * Why one session was passed over. Reported for logging and shown to the user. * - * The first seven are decided before anything is built. `capacity-reached` and + * All but the last two are decided before anything is built. `capacity-reached` and * `rebuild-failed` can only happen once a click is spending the plan, and they * are the two the banner must not confuse with a missing workspace: one means * "try again after closing something", the other means the CLI would not start. @@ -120,6 +124,7 @@ export interface RebootRestoreRejection { | 'no-persisted-record' | 'intentionally-ended' | 'not-running' + | 'agent-exited' | 'respawn-blocked' | 'remote-or-docker' | 'unsupported-mode' @@ -191,7 +196,8 @@ export function planRebootRestore( // // ⚠️ This does NOT catch a session the user ended with `/exit`. See the // module header: that leaves the pid in place, because the pid is the tmux - // attach process and `remain-on-exit` keeps it alive. + // attach process and `remain-on-exit` keeps it alive. The `paneExit` check + // below catches it instead. // // Conservative on purpose. A session that somehow persisted no pid while // genuinely running is not offered, and its conversation stays reachable @@ -200,6 +206,13 @@ export function planRebootRestore( skipped.push({ sessionId, reason: 'not-running' }); continue; } + if (isCleanPaneExit(state.paneExit)) { + // The user ended the agent, and the clean-exit sweep would have closed the + // session had the power not gone first (Ark0N/Codeman#446). The same + // explicit-0 rule applies: an absent status is unknown, not clean. + skipped.push({ sessionId, reason: 'agent-exited' }); + continue; + } if (state.respawnBlocked === true) { // The crash-loop breaker tripped on this pane. Re-creating it restarts the loop. skipped.push({ sessionId, reason: 'respawn-blocked' }); diff --git a/src/session.ts b/src/session.ts index d5e6ba164..77952348f 100644 --- a/src/session.ts +++ b/src/session.ts @@ -582,6 +582,14 @@ export class Session extends EventEmitter { * rendered as "alive". */ private _paneExit: PaneExit | null = null; + /** + * How many starts, attaches or relaunches are running for this session's + * pane. While one is, a dead-pane reading may describe a pane that is being + * revived on purpose, so the exited-agent sweep leaves the session alone + * (Ark0N/Codeman#446). A counter rather than a flag, so two overlapping + * operations cannot clear each other's mark. + */ + private _paneLifecycleOps = 0; /** * This session was rebuilt from the tmux socket rather than from Codeman's * own records, so its `remote`/`docker` metadata is missing rather than known @@ -1183,6 +1191,26 @@ export class Session extends EventEmitter { return this._paneExit ?? undefined; } + /** + * True while a start, attach or relaunch is running for this session's pane. + * The exited-agent sweep reads it (see `pane-exit-sweep.ts`): the dead-pane + * branch of {@link _setupOrAttachMuxSession} respawns an exited pane, and + * until it finishes and clears the exit, the pane still reads as dead. + */ + get paneLifecycleInFlight(): boolean { + return this._paneLifecycleOps > 0; + } + + /** Run one pane start, attach or relaunch with {@link paneLifecycleInFlight} raised. */ + private async _withPaneLifecycle(op: () => Promise): Promise { + this._paneLifecycleOps++; + try { + return await op(); + } finally { + this._paneLifecycleOps--; + } + } + /** * Forget this pane's exit, on both this record and the mux layer's cache. * Every path that starts or relaunches a command in the pane calls it, and @@ -1888,6 +1916,14 @@ export class Session extends EventEmitter { respawnPaneOptions: import('./mux-interface.js').RespawnPaneOptions; createSessionOptions: import('./mux-interface.js').CreateSessionOptions; spawnErrLabel: string; + }): Promise<{ isRestored: boolean; respawnedResumeId?: string; respawnedDeadPane: boolean }> { + return this._withPaneLifecycle(() => this._doSetupOrAttachMuxSession(options)); + } + + private async _doSetupOrAttachMuxSession(options: { + respawnPaneOptions: import('./mux-interface.js').RespawnPaneOptions; + createSessionOptions: import('./mux-interface.js').CreateSessionOptions; + spawnErrLabel: string; }): Promise<{ isRestored: boolean; respawnedResumeId?: string; respawnedDeadPane: boolean }> { const mux = this._mux!; @@ -2071,6 +2107,10 @@ export class Session extends EventEmitter { * the mux session is gone — see {@link reattachRemote} for that reasoning). */ async restartCli(): Promise { + return this._withPaneLifecycle(() => this._doRestartCli()); + } + + private async _doRestartCli(): Promise { if (!this._useMux || !this._mux || !this._muxSession) return false; const mux = this._mux; diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index a0ad8d76a..77078ec27 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -287,6 +287,19 @@ export interface PaneExitObservation { exit: PaneExit; } +/** + * A {@link PaneExitObservation} as the manager stores it, with a count of the + * authoritative reads that have seen this same exit. The count is what lets + * the exited-agent sweep act only on a death that more than one read agreed on + * (`CLEAN_EXIT_CONFIRMING_READS` in `pane-exit-sweep.ts`). A failed or skipped + * read never reaches {@link TmuxManager.applyPaneExits}, so it neither raises + * the count nor resets it. + */ +interface TrackedPaneExit extends PaneExitObservation { + /** Authoritative reads that saw this exit, counting the first. */ + reads: number; +} + /** Read one optional numeric field; a blank or non-numeric value is "not reported". */ function paneField(fields: string[], index: number): number | undefined { const raw = fields[index]; @@ -1672,7 +1685,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { * lives on `Session`, because the remote-reconnect watcher above needs the * raw pane reading. */ - private paneExits: Map = new Map(); + private paneExits: Map = new Map(); /** The pane-exit watcher's own interval. Runs whether or not stats are on. */ private paneExitInterval: NodeJS.Timeout | null = null; /** @@ -3075,6 +3088,16 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { return this.paneExits.get(muxName)?.exit; } + /** + * How many authoritative pane reads have agreed on the exit that + * {@link getPaneExit} reports, or 0 when it reports none. A new observation + * starts at 1, and every later read that sees the same pane with the same + * status and signal adds one. + */ + getPaneExitReadCount(muxName: string): number { + return this.paneExits.get(muxName)?.reads ?? 0; + } + /** * Re-read every pane on the socket and refresh {@link paneExits}. ONE batched * `tmux list-panes -a` answers for every session at once, which is why this @@ -3170,6 +3193,9 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { * changed status, a changed signal, or a different pane pid all start a new * observation — the pid is what catches a second command in the same pane * that happened to exit the same way. + * + * The same rule decides the read count: a repeat of the stored exit adds one, + * and anything that starts a new observation starts the count again at 1. */ applyPaneExits(observed: Map): void { for (const muxName of [...this.paneExits.keys()]) { @@ -3182,7 +3208,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { prev.panePid === next.panePid && prev.exit.status === next.exit.status && prev.exit.signal === next.exit.signal; - this.paneExits.set(muxName, sameExit ? prev : next); + this.paneExits.set(muxName, sameExit ? { ...prev, reads: prev.reads + 1 } : { ...next, reads: 1 }); } } diff --git a/src/types/session.ts b/src/types/session.ts index 529ba448e..49249aff8 100644 --- a/src/types/session.ts +++ b/src/types/session.ts @@ -649,12 +649,13 @@ export interface CustomModelBookkeeping extends CustomModelSelection { * ⚠ AN ABSENT `status` STAYS ABSENT. Never write `status ?? 0`, and never read * "no signal was reported" as "the exit must have been clean". On tmux 3.2a * the absent status IS how a signal death presents, so absent-stays-absent is - * the only thing keeping a future clean-exit sweep away from crashed agents: - * an agent SIGKILLed by the OOM killer would otherwise read as a user typing - * `/exit` and be swept. Nothing here fails when somebody adds that `??` — the - * types allow it, the label still renders, and the damage shows up only once - * the sweep lands. The rule is enforced in `derivePaneExits()` - * (`tmux-manager.ts`), which omits the key rather than defaulting it. + * the only thing keeping the clean-exit sweep (`pane-exit-sweep.ts`) away + * from crashed agents: an agent SIGKILLed by the OOM killer would otherwise + * read as a user typing `/exit` and be closed. Nothing here fails when + * somebody adds that `??` — the types allow it and the label still renders. + * The rule is enforced in `derivePaneExits()` (`tmux-manager.ts`), which omits + * the key rather than defaulting it, and again in `isCleanPaneExit()`, which + * accepts only an explicit 0. */ export interface PaneExit { /** diff --git a/src/web/server.ts b/src/web/server.ts index 0ead5c5bd..19d1cab1d 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -34,6 +34,7 @@ import fastifyStatic from '@fastify/static'; import fastifyWebsocket from '@fastify/websocket'; import fastifyMultipart from '@fastify/multipart'; import { pasteImageDirInUseByOtherSession, startPasteImageGc } from './paste-image-gc.js'; +import { CLEAN_EXIT_CLOSE_REASON, shouldCloseCleanlyExitedSession } from '../pane-exit-sweep.js'; import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; import { existsSync, mkdirSync, readFileSync, chmodSync, rmSync, statSync } from 'node:fs'; @@ -2508,6 +2509,9 @@ export class WebServer extends EventEmitter { * Nothing here touches `status` or `pid`. `status: 'error'` belongs to the * PTY-exit breaker and makes the browser offer a restart, and a null `pid` is * what makes the browser re-attach and launch a fresh CLI. + * + * Once the records are current, {@link closeCleanlyExitedSessions} closes the + * sessions whose agent the user ended. */ private applyPaneExits(): void { const getPaneExit = this.mux.getPaneExit?.bind(this.mux); @@ -2521,6 +2525,46 @@ export class WebServer extends EventEmitter { this.persistSessionState(session); this.broadcastSessionStateDebounced(session.id); } + this.closeCleanlyExitedSessions(); + } + + /** + * Close every session whose agent exited cleanly, through the same + * `cleanupSession()` the X button uses (Ark0N/Codeman#446). A pinned session + * is demoted to `status: 'stopped'` there rather than removed, and either way + * the reboot restore stops offering it back. The conversation stays + * resumable, since the Resume list reads the lifecycle log and the transcript + * files, and the pane owns neither. + * + * `shouldCloseCleanlyExitedSession()` (`pane-exit-sweep.ts`) holds the rule: + * an explicit status of 0, confirmed by more than one pane read, with no + * start or attach in flight. A crashed agent keeps its row with the exit + * code on it. `session.paneExit` is already scoped to local mux-backed + * sessions by `setPaneExit()`, so a remote, docker or direct-PTY session is + * never closed here. + * + * The close runs in the background. `cleanupSession()` ignores a second call + * for a session it is already closing, and the `closing` check below keeps + * the next tick from queueing one. + */ + private closeCleanlyExitedSessions(): void { + const readCount = this.mux.getPaneExitReadCount?.bind(this.mux); + if (!readCount) return; + for (const session of [...this.sessions.values()]) { + const muxName = session.muxName; + if (!muxName) continue; + const close = shouldCloseCleanlyExitedSession({ + paneExit: session.paneExit, + confirmingReads: readCount(muxName), + paneLifecycleInFlight: session.paneLifecycleInFlight, + closing: this.cleaningUp.has(session.id), + }); + if (!close) continue; + console.log(`[Server] Closing session ${session.id} (${session.name}): ${CLEAN_EXIT_CLOSE_REASON}`); + void this.cleanupSession(session.id, true, CLEAN_EXIT_CLOSE_REASON).catch((err) => { + console.error(`[Server] Failed to close cleanly exited session ${session.id}:`, err); + }); + } } // ========== Web Push ========== diff --git a/test/pane-exit-sweep.test.ts b/test/pane-exit-sweep.test.ts new file mode 100644 index 000000000..c8869ea70 --- /dev/null +++ b/test/pane-exit-sweep.test.ts @@ -0,0 +1,240 @@ +/** + * @fileoverview The clean-exit sweep (Ark0N/Codeman#446, part 2). + * + * A session whose agent the user ended with `/exit` is closed the way the X + * button closes it, so finished sessions stop piling up on the board. A crashed + * agent keeps its row, marked with the exit code. + * + * Each rule pinned here guards against a specific wrong close: + * + * 1. **Only an explicit numeric 0 is clean.** On tmux 3.2a a SIGKILLed pane + * reports neither a status nor a signal, so an absent status is the + * crashed-agent case, not the clean one. + * 2. **Two agreeing reads, not one.** A single reading can describe a pane + * that is about to be revived. + * 3. **Never while a start or attach is in flight.** The dead-pane respawn in + * `_setupOrAttachMuxSession()` revives an exited pane on purpose, and the + * pane reads as dead until it finishes. + * 4. **Only local mux-backed sessions.** A remote session's local pane is its + * ssh client, whose death may be a transport drop. + * + * Port: 3189 + */ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { Session } from '../src/session.js'; +import { WebServer } from '../src/web/server.js'; +import type { PaneExit, SessionRemote } from '../src/types.js'; +import type { MuxSession, TerminalMultiplexer } from '../src/mux-interface.js'; +import { + CLEAN_EXIT_CLOSE_REASON, + CLEAN_EXIT_CONFIRMING_READS, + isCleanPaneExit, + shouldCloseCleanlyExitedSession, +} from '../src/pane-exit-sweep.js'; + +const PORT = 3189; +const AT = 1_700_000_000_000; + +describe('isCleanPaneExit', () => { + it('accepts an explicit status of 0 with no signal', () => { + expect(isCleanPaneExit({ status: 0, at: AT })).toBe(true); + }); + + it('refuses an absent status, which is how a SIGKILL presents on tmux 3.2a', () => { + expect(isCleanPaneExit({ at: AT })).toBe(false); + }); + + it('refuses a non-zero status', () => { + expect(isCleanPaneExit({ status: 1, at: AT })).toBe(false); + expect(isCleanPaneExit({ status: 137, at: AT })).toBe(false); + }); + + it('refuses any reported signal, even beside a 0', () => { + expect(isCleanPaneExit({ signal: 9, at: AT })).toBe(false); + expect(isCleanPaneExit({ status: 0, signal: 15, at: AT })).toBe(false); + }); + + it('refuses an unknown exit', () => { + expect(isCleanPaneExit(undefined)).toBe(false); + }); +}); + +describe('shouldCloseCleanlyExitedSession', () => { + const clean = { + paneExit: { status: 0, at: AT } as PaneExit, + confirmingReads: CLEAN_EXIT_CONFIRMING_READS, + paneLifecycleInFlight: false, + closing: false, + }; + + it('closes a clean exit that enough reads agreed on', () => { + expect(CLEAN_EXIT_CONFIRMING_READS).toBe(2); + expect(shouldCloseCleanlyExitedSession(clean)).toBe(true); + expect(shouldCloseCleanlyExitedSession({ ...clean, confirmingReads: 5 })).toBe(true); + }); + + it('waits for the second read', () => { + expect(shouldCloseCleanlyExitedSession({ ...clean, confirmingReads: 1 })).toBe(false); + expect(shouldCloseCleanlyExitedSession({ ...clean, confirmingReads: 0 })).toBe(false); + }); + + it('leaves a session alone while its pane is being started or revived', () => { + expect(shouldCloseCleanlyExitedSession({ ...clean, paneLifecycleInFlight: true })).toBe(false); + }); + + it('does not queue a second close for a session already closing', () => { + expect(shouldCloseCleanlyExitedSession({ ...clean, closing: true })).toBe(false); + }); + + it('keeps a crashed agent however many reads saw it', () => { + expect(shouldCloseCleanlyExitedSession({ ...clean, paneExit: { status: 137, at: AT }, confirmingReads: 9 })).toBe( + false + ); + expect(shouldCloseCleanlyExitedSession({ ...clean, paneExit: { at: AT }, confirmingReads: 9 })).toBe(false); + }); +}); + +describe('the sweep on a real server', () => { + // Drives the real `paneExitsUpdated` wiring: the mux reports a reading, the + // event fires, and the test checks whether the server closed the session. + let server: WebServer | null = null; + + afterEach(async () => { + vi.restoreAllMocks(); + await server?.stop?.(); + server = null; + }); + + const remote: SessionRemote = { hostId: 'h1', label: 'box', host: 'box', user: 'dev' } as SessionRemote; + + /** The mux members `Session` needs to be a local mux-backed session that can be stopped. */ + const sessionMux = () => + ({ + isAvailable: () => true, + killSession: async () => true, + }) as unknown as TerminalMultiplexer; + + const addSession = (web: WebServer, extra: Record = {}) => { + const muxName = `codeman-${Math.random().toString(16).slice(2, 10)}`; + const session = new Session({ + workingDir: '/tmp', + mode: 'claude', + useMux: true, + mux: sessionMux(), + muxSession: { muxName, sessionId: 'x' } as unknown as MuxSession, + ...extra, + }); + (web as unknown as { sessions: Map }).sessions.set(session.id, session); + return { session, muxName }; + }; + + /** A server whose mux reports `exit` for every pane, seen by `reads` reads. */ + const build = (exit: PaneExit | undefined, reads: number) => { + const web = new WebServer(PORT, false, true); + server = web; + const mux = (web as unknown as { mux: Record }).mux; + const state = { exit, reads }; + mux.getPaneExit = () => state.exit; + mux.getPaneExitReadCount = () => (state.exit ? state.reads : 0); + const tick = () => (mux as unknown as { emit: (e: string) => void }).emit('paneExitsUpdated'); + const cleanup = vi + .spyOn(web as unknown as { cleanupSession: (...a: unknown[]) => Promise }, 'cleanupSession') + .mockResolvedValue(undefined); + return { web, state, tick, cleanup }; + }; + + it('closes a cleanly exited session on the second read, with a reason in the lifecycle log', () => { + const { web, state, tick, cleanup } = build({ status: 0, at: AT }, 1); + const { session } = addSession(web); + + tick(); + expect(cleanup).not.toHaveBeenCalled(); + // The exit is on the record already, so the tab says "exited (0)" meanwhile. + expect(session.paneExit).toEqual({ status: 0, at: AT }); + + state.reads = 2; + tick(); + expect(cleanup).toHaveBeenCalledTimes(1); + expect(cleanup).toHaveBeenCalledWith(session.id, true, CLEAN_EXIT_CLOSE_REASON); + }); + + it('keeps a crashed agent on the board', () => { + const { web, tick, cleanup } = build({ status: 137, at: AT }, 5); + addSession(web); + tick(); + expect(cleanup).not.toHaveBeenCalled(); + }); + + it('keeps an agent whose exit status tmux did not report', () => { + const { web, tick, cleanup } = build({ at: AT }, 5); + addSession(web); + tick(); + expect(cleanup).not.toHaveBeenCalled(); + }); + + it('never closes a remote session, whose local pane is only the ssh client', () => { + const { web, tick, cleanup } = build({ status: 0, at: AT }, 5); + addSession(web, { remote }); + tick(); + expect(cleanup).not.toHaveBeenCalled(); + }); + + it('leaves a session alone while its pane is being revived', () => { + const { web, tick, cleanup } = build({ status: 0, at: AT }, 5); + const { session } = addSession(web); + vi.spyOn(session, 'paneLifecycleInFlight', 'get').mockReturnValue(true); + tick(); + expect(cleanup).not.toHaveBeenCalled(); + }); + + it('closes nothing when the mux cannot count its reads', () => { + const { web, tick, cleanup } = build({ status: 0, at: AT }, 5); + delete (web as unknown as { mux: Record }).mux.getPaneExitReadCount; + addSession(web); + tick(); + expect(cleanup).not.toHaveBeenCalled(); + }); + + it('removes the session for real through cleanupSession', async () => { + // No spy on cleanupSession here: the close runs the same path as the X + // button, and the session leaves the server's map. + const web = new WebServer(PORT, false, true); + server = web; + const mux = (web as unknown as { mux: Record }).mux; + mux.getPaneExit = () => ({ status: 0, at: AT }); + mux.getPaneExitReadCount = () => 2; + const { session } = addSession(web); + const sessions = (web as unknown as { sessions: Map }).sessions; + + (mux as unknown as { emit: (e: string) => void }).emit('paneExitsUpdated'); + await vi.waitFor(() => expect(sessions.has(session.id)).toBe(false)); + }); +}); + +describe('the pane lifecycle mark on Session', () => { + it('is raised for the whole of restartCli and lowered afterwards, even on failure', async () => { + let seenDuring: boolean | null = null; + let session: Session; + const mux = { + isAvailable: () => true, + muxSessionExists: () => true, + respawnPane: async () => { + seenDuring = session.paneLifecycleInFlight; + throw new Error('respawn failed'); + }, + clearPaneExit: () => {}, + } as unknown as TerminalMultiplexer; + session = new Session({ + workingDir: '/tmp', + mode: 'claude', + useMux: true, + mux, + muxSession: { muxName: 'codeman-aaaa', sessionId: 'aaaa' } as unknown as MuxSession, + }); + + expect(session.paneLifecycleInFlight).toBe(false); + await expect(session.restartCli()).rejects.toThrow('respawn failed'); + expect(seenDuring).toBe(true); + expect(session.paneLifecycleInFlight).toBe(false); + }); +}); diff --git a/test/reboot-restore.test.ts b/test/reboot-restore.test.ts index 67bc34e94..e1f3b5c4e 100644 --- a/test/reboot-restore.test.ts +++ b/test/reboot-restore.test.ts @@ -108,6 +108,25 @@ describe('which dead sessions may be rebuilt', () => { expect(plan.skipped).toEqual([{ sessionId: 'gone', reason: 'no-persisted-record' }]); }); + it('never revives a session whose agent the user ended with a clean exit (#446)', () => { + // The clean-exit sweep would have closed it, had the power not gone first. + const persisted = { exited: persistedSession({ id: 'exited', paneExit: { status: 0, at: NOW - HOUR } }) }; + const plan = planRebootRestore(['exited'], persisted, () => true); + expect(plan.restore).toEqual([]); + expect(plan.skipped).toEqual([{ sessionId: 'exited', reason: 'agent-exited' }]); + }); + + it('still offers a crashed agent, and one whose exit status tmux never reported', () => { + // Same explicit-0 rule as the sweep: an absent status is unknown, not clean, + // and a crash keeps its row on the board, so it stays eligible here too. + const persisted = { + crashed: persistedSession({ id: 'crashed', paneExit: { status: 137, at: NOW - HOUR } }), + killed: persistedSession({ id: 'killed', paneExit: { at: NOW - HOUR } }), + }; + const plan = planRebootRestore(['crashed', 'killed'], persisted, () => true); + expect(plan.restore.map((s) => s.sessionId)).toEqual(['crashed', 'killed']); + }); + it('never revives a pane whose PTY-exit breaker had tripped', () => { const persisted = { crashy: persistedSession({ id: 'crashy', respawnBlocked: true }) }; expect(planRebootRestore(['crashy'], persisted, () => true).skipped[0].reason).toBe('respawn-blocked'); diff --git a/test/tmux-manager.test.ts b/test/tmux-manager.test.ts index 0fd963e18..24b0659fe 100644 --- a/test/tmux-manager.test.ts +++ b/test/tmux-manager.test.ts @@ -1126,6 +1126,37 @@ describe('TmuxManager pane-exit bookkeeping', () => { manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW)); manager.clearPaneExit('codeman-aaaa'); expect(manager.getPaneExit('codeman-aaaa')).toBeUndefined(); + expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(0); + }); + + // The clean-exit sweep closes a session only once two reads agreed on its + // exit (Ark0N/Codeman#446), so the count must rise only on an exact repeat. + it('counts the reads that agreed on one exit', () => { + const manager = new TmuxManager(); + expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(0); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW)); + expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(1); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW + 2000)); + expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(2); + }); + + it('starts the count again when the status or the pane pid changes', () => { + const manager = new TmuxManager(); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW)); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW + 2000)); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|1|'), NOW + 4000)); + expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(1); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|101|1|1|'), NOW + 6000)); + expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(1); + }); + + it('drops the count once a read sees the pane alive again', () => { + const manager = new TmuxManager(); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW)); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|101|0|||'), NOW + 2000)); + expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(0); + manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|101|1|0|'), NOW + 4000)); + expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(1); }); }); @@ -1214,6 +1245,20 @@ describe('the pane-exit watcher tick', () => { expect(manager.getPaneExit('codeman-s1')).toEqual({ status: 137, at: NOW }); }); + it('does not count an empty read as confirming an exit', async () => { + // The clean-exit sweep closes on the second agreeing read. A read tmux did + // not answer agrees with nothing, so it must not supply that second read. + const manager = withLocalSession(); + manager.rows = parsePaneRows('codeman-s1|100|1|0|'); + await manager.refreshPaneExits(NOW); + manager.rows = []; + await manager.refreshPaneExits(NOW + 2000); + expect(manager.getPaneExitReadCount('codeman-s1')).toBe(1); + manager.rows = parsePaneRows('codeman-s1|100|1|0|'); + await manager.refreshPaneExits(NOW + 4000); + expect(manager.getPaneExitReadCount('codeman-s1')).toBe(2); + }); + it('discards a read that started before the pane was cleared', async () => { // The guard that stops an in-flight read from republishing a death over // the pane that has just replaced it. From b91ed853b7b70a96013d12b7ee9e57e143748c85 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Wed, 23 Sep 2026 16:40:42 +0200 Subject: [PATCH 3/5] fix(web): show "exited" on the phone overview and desktop home rail (#446) Part 1 of Ark0N/Codeman#446 taught the tab strip and the rich rail rows to say that a session's agent has exited. The phone overview and the desktop home rail still said "idle", beside a green or pulsing dot. _mobileOverviewExit() in mobile-overview.js is now the one rule for all three surfaces, and _sidebarRichRow() uses it as well. It changes what a row shows and leaves the row's state alone, because the state still picks the section and the sort order. An exited row gets an "exited" pill, a neutral dot and row accent, and a duration measured from when the server first saw the pane dead. A pending permission prompt or question still wins, as it does on the tab. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/public/app.js | 15 +-- src/web/public/home-sessions.js | 16 ++- src/web/public/mobile-overview.js | 45 ++++++- src/web/public/mobile.css | 7 + src/web/public/styles.css | 9 ++ test/home-screen-exited-rows.test.ts | 194 +++++++++++++++++++++++++++ test/session-pane-exit-ui.test.ts | 10 +- 7 files changed, 273 insertions(+), 23 deletions(-) create mode 100644 test/home-screen-exited-rows.test.ts diff --git a/src/web/public/app.js b/src/web/public/app.js index 5bfdfe436..1e6ba949d 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -4878,9 +4878,10 @@ class CodemanApp { // `state` keys SESSION_ACTIVITY_RANK and the sort, while `status` stays idle // or busy for an exited pane by design, so without this the muted dot sits // beside a pill saying "idle". A pending alert still wins, exactly as it - // does for the dot. - const exited = !!paneExitLabel(session.paneExit) && (state === 'idle' || state === 'working'); - const exitAt = exited ? Number(session.paneExit.at) || 0 : 0; + // does for the dot. The rule is `_mobileOverviewExit()`, shared with both + // home screens so the three surfaces agree on which sessions have exited. + const exit = this._mobileOverviewExit ? this._mobileOverviewExit(state, session) : null; + const exited = !!exit; return { state, exited, @@ -4891,13 +4892,7 @@ class CodemanApp { // state pill and never replaces it. watching: typeof session.watching === 'string' ? session.watching : '', createdAt: Number(session.createdAt) || 0, - since: exitAt - ? { key: 'exited', at: exitAt } - : exited - ? null - : this._mobileOverviewSince - ? this._mobileOverviewSince(state, session) - : null, + since: exit ? exit.since : this._mobileOverviewSince ? this._mobileOverviewSince(state, session) : null, }; } diff --git a/src/web/public/home-sessions.js b/src/web/public/home-sessions.js index 66430d9a2..eeb12c536 100644 --- a/src/web/public/home-sessions.js +++ b/src/web/public/home-sessions.js @@ -201,6 +201,8 @@ Object.assign(CodemanApp.prototype, { const session = this.sessions.get(id); const matched = this._mobileOverviewCaseFor(session.workingDir, cases); const state = this._mobileOverviewState(session, this.pendingHooks?.get(id)); + // Guarded: a stale cached mobile-overview.js may predate the helper. + const exit = this._mobileOverviewExit ? this._mobileOverviewExit(state, session) : null; const mode = session.mode || 'claude'; return { id, @@ -211,7 +213,10 @@ Object.assign(CodemanApp.prototype, { caseName: matched ? matched.name : '', dir: this._shortenHomePath ? this._shortenHomePath(session.workingDir) : session.workingDir || '', state, - pill: HOME_SESSIONS_PILL_LABEL[state] || state, + // What the row's dot, accent and pill show. It differs from `state` only + // for an exited agent (Ark0N/Codeman#446), whose state still sorts it. + display: exit ? 'exited' : state, + pill: exit ? 'exited' : HOME_SESSIONS_PILL_LABEL[state] || state, // What the pane's footer says is still running in the background, straight off // the session payload. Same field, same meaning as on the phone overview. watching: typeof session.watching === 'string' ? session.watching : '', @@ -224,7 +229,7 @@ Object.assign(CodemanApp.prototype, { lastSubmitAt: Number(session.lastSubmitAt) || 0, // "how long has it been like this", resolved by the phone overview's // helper so both home screens label the same stamp with the same word. - since: this._mobileOverviewSince(state, session), + since: exit ? exit.since : this._mobileOverviewSince(state, session), }; }); @@ -392,7 +397,8 @@ Object.assign(CodemanApp.prototype, { _buildHomeSessionRow(row) { const item = document.createElement('button'); item.type = 'button'; - item.className = 'home-sessions-row home-sessions-row--' + row.state; + const display = row.display || row.state; + item.className = 'home-sessions-row home-sessions-row--' + display; item.dataset.hsAction = 'session'; item.dataset.hsSession = row.id; item.title = row.dir ? `${row.name} (${row.dir})` : row.name; @@ -409,7 +415,7 @@ Object.assign(CodemanApp.prototype, { } const dot = document.createElement('span'); - dot.className = 'home-sessions-dot home-sessions-dot--' + row.state; + dot.className = 'home-sessions-dot home-sessions-dot--' + display; dot.setAttribute('aria-hidden', 'true'); item.appendChild(dot); @@ -441,7 +447,7 @@ Object.assign(CodemanApp.prototype, { item.appendChild(body); const pill = document.createElement('span'); - pill.className = 'home-sessions-pill home-sessions-pill--' + row.state; + pill.className = 'home-sessions-pill home-sessions-pill--' + display; // Skipped by i18n on purpose: generic single words ("idle", "done", "error") // that collide with state strings on other surfaces. pill.setAttribute('data-i18n-skip', ''); diff --git a/src/web/public/mobile-overview.js b/src/web/public/mobile-overview.js index 833177aa1..764697a68 100644 --- a/src/web/public/mobile-overview.js +++ b/src/web/public/mobile-overview.js @@ -161,6 +161,36 @@ Object.assign(CodemanApp.prototype, { return { key: MOBILE_OVERVIEW_SINCE_LABEL[state] || state, at }; }, + /** + * The exited-agent override for one row (Ark0N/Codeman#446), or null when + * the row shows its state as usual. + * + * The server publishes `session.paneExit` once the agent inside a local tmux + * pane has exited, while `status` stays `idle` or `busy` by design. So a row + * classified as idle or working may really be a pane with nothing running + * in it. This overrides what the row SHOWS, never its `state`: `state` still + * picks the section and the sort, the way `_sidebarRichRow()` (app.js) does + * for the detailed sidebar and rail. A pending alert still wins, because a + * human being blocked outranks the agent having exited. + * + * Shared by the phone overview, the desktop home rail and the rich tab rows, + * so the three cannot disagree about which sessions have exited. + * + * Guarded like every other cross-file call: `paneExitLabel()` lives in + * app.js, and a stale cached app.js must degrade to no override, not throw. + * + * @returns {{since: {key: string, at: number}|null}|null} + */ + _mobileOverviewExit(state, session) { + if (state !== 'idle' && state !== 'working') return null; + if (typeof paneExitLabel !== 'function' || !paneExitLabel(session.paneExit)) return null; + // `at` is when this server first saw the pane dead, which is what "exited + // 2m" should measure. A row without it shows no duration at all rather + // than a working or idle stamp that no longer describes the pane. + const at = Number(session.paneExit.at) || 0; + return { since: at ? { key: 'exited', at } : null }; + }, + /** * Longest-prefix match of a workingDir against the case list, so a session * started in a subdirectory still belongs to its case. Mirrors the matching in @@ -202,6 +232,7 @@ Object.assign(CodemanApp.prototype, { const rows = sessions.map((session) => { const matched = this._mobileOverviewCaseFor(session.workingDir, cases); const state = this._mobileOverviewState(session, pendingHooks.get && pendingHooks.get(session.id)); + const exit = this._mobileOverviewExit(state, session); const orderIndex = order.indexOf(session.id); return { id: session.id, @@ -210,7 +241,10 @@ Object.assign(CodemanApp.prototype, { caseName: matched ? matched.name : '', dir: this._shortenHomePath ? this._shortenHomePath(session.workingDir) : session.workingDir || '', state, - pill: MOBILE_OVERVIEW_PILL_LABEL[state] || state, + // What the row's dot, accent and pill show. It differs from `state` only + // for an exited agent, whose state still decides the section and sort. + display: exit ? 'exited' : state, + pill: exit ? 'exited' : MOBILE_OVERVIEW_PILL_LABEL[state] || state, // What the pane's own footer says is still running in the background ("1 monitor", // "2 shells"), straight off the session payload. A row that has one is quiet // because the agent is waiting for that, not because it is waiting for you. @@ -222,7 +256,7 @@ Object.assign(CodemanApp.prototype, { // pair resolved for DISPLAY, and the two must not drift apart. lastActivityAt: Number(session.lastActivityAt) || 0, lastSubmitAt: Number(session.lastSubmitAt) || 0, - since: this._mobileOverviewSince(state, session), + since: exit ? exit.since : this._mobileOverviewSince(state, session), orderIndex: orderIndex === -1 ? Number.MAX_SAFE_INTEGER : orderIndex, }; }); @@ -698,12 +732,13 @@ Object.assign(CodemanApp.prototype, { _buildMobileOverviewRow(row) { const item = document.createElement('button'); item.type = 'button'; - item.className = 'mobile-overview-row mobile-overview-row--' + row.state; + const display = row.display || row.state; + item.className = 'mobile-overview-row mobile-overview-row--' + display; item.dataset.moAction = 'session'; item.dataset.moSession = row.id; const dot = document.createElement('span'); - dot.className = 'mobile-overview-dot mobile-overview-dot--' + row.state; + dot.className = 'mobile-overview-dot mobile-overview-dot--' + display; dot.setAttribute('aria-hidden', 'true'); item.appendChild(dot); @@ -736,7 +771,7 @@ Object.assign(CodemanApp.prototype, { item.appendChild(body); const pill = document.createElement('span'); - pill.className = 'mobile-overview-pill mobile-overview-pill--' + row.state; + pill.className = 'mobile-overview-pill mobile-overview-pill--' + display; // Skipped by i18n on purpose: the labels are generic single words ("idle", // "done", "error") that collide with state strings on other surfaces. pill.setAttribute('data-i18n-skip', ''); diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index f4588d218..4565a91b1 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -2978,6 +2978,13 @@ html.mobile-init .file-browser-panel { color: var(--green); } + /* An exited agent (Ark0N/Codeman#446): neutral, since nothing is running behind + the row. The dot and the row take `--exited` too and keep their base rules. */ + .mobile-overview-pill--exited { + border-color: var(--text-muted); + color: var(--text-muted); + } + /* Accent, and none of the three above: a session watching work it started itself is not asking the user for anything, and red and yellow are what say it is. */ .mobile-overview-pill--watching { diff --git a/src/web/public/styles.css b/src/web/public/styles.css index a7379b72f..c3be0d0b2 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -16392,6 +16392,15 @@ html[data-tab-orientation='vertical'] .home-sessions { color: color-mix(in srgb, var(--green) 45%, var(--text-muted)); } +/* An exited agent (Ark0N/Codeman#446): neutral, like the rich rail's exited pill. + No green at all, since nothing is running behind this row. The dot and the row + take the `--exited` class too and fall back to their neutral base rules. */ +.home-sessions-pill--exited { + background: color-mix(in srgb, var(--text-muted) 10%, transparent); + border-color: color-mix(in srgb, var(--text-muted) 30%, var(--border)); + color: var(--text-muted); +} + /* Accent, deliberately none of the three above: a session that is watching something it started (a monitor, a backgrounded shell, a cloud session) is not asking for anything, so it must not borrow the red or the yellow that mean it is. This badge diff --git a/test/home-screen-exited-rows.test.ts b/test/home-screen-exited-rows.test.ts new file mode 100644 index 000000000..0541bc5a3 --- /dev/null +++ b/test/home-screen-exited-rows.test.ts @@ -0,0 +1,194 @@ +// Port: none (pure model + fake-DOM row builders — no browser, no server). +// +// The phone overview and the desktop home rail showing an exited agent as +// "exited" rather than "idle" (Ark0N/Codeman#446). +// +// The server publishes `session.paneExit` when the agent inside a local tmux +// pane has exited, while `status` stays `idle` or `busy` by design. Part 1 of +// #446 taught the tab strip and the rich rail rows to say so; both home screens +// still said "idle" beside nothing running. `_mobileOverviewExit()` +// (mobile-overview.js) is now the one rule all three surfaces read. It changes +// what a row SHOWS and leaves its `state` alone, because `state` picks the +// section and the sort order. +// +// `paneExitLabel()` is lifted out of the shipped app.js rather than restated +// here, so a change to what counts as "exited" there reaches these tests. +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it } from 'vitest'; + +const PUBLIC = resolve(import.meta.dirname, '../src/web/public'); + +/** The shipped `paneExitLabel()` from app.js, as source text. */ +function paneExitLabelSource(): string { + const appJs = readFileSync(resolve(PUBLIC, 'app.js'), 'utf8'); + const match = appJs.match(/function paneExitLabel\(paneExit\) \{[\s\S]*?\n\}\n/); + if (!match) throw new Error('paneExitLabel() not found in app.js'); + return match[0]; +} + +function fakeElement(): any { + const el: any = { + className: '', + type: '', + title: '', + textContent: '', + dataset: {}, + style: {}, + children: [] as any[], + setAttribute() {}, + appendChild(child: any) { + el.children.push(child); + return child; + }, + }; + return el; +} + +/** Every className in a fake-DOM subtree, depth first. */ +function classNames(el: any): string[] { + return [el.className, ...(el.children || []).flatMap(classNames)].filter(Boolean); +} + +function loadApp({ withPaneExitLabel = true } = {}) { + const CodemanApp = function CodemanApp(this: any) {}; + const context = vm.createContext({ + CodemanApp, + console, + window: { innerWidth: 1512 }, + document: { + documentElement: { getAttribute: () => null }, + getElementById: () => null, + createElement: () => fakeElement(), + createElementNS: () => fakeElement(), + }, + MobileDetection: { getDeviceType: () => 'desktop' }, + }); + if (withPaneExitLabel) vm.runInContext(paneExitLabelSource(), context, { filename: 'app.js' }); + for (const file of ['constants.js', 'mobile-overview.js', 'home-sessions.js']) { + vm.runInContext(readFileSync(resolve(PUBLIC, file), 'utf8'), context, { filename: file }); + } + const app = new (CodemanApp as any)(); + app.getSessionName = (session: any) => session.name || session.id.slice(0, 8); + app._shortenHomePath = (p: string) => p || ''; + app.loadAppSettingsFromStorage = () => ({}); + app.cases = []; + app.pendingHooks = new Map(); + return app; +} + +const EXIT_AT = 1_700_000_000_000; + +function sessions(list: Array>) { + return new Map(list.map((s) => [s.id, { status: 'idle', mode: 'claude', workingDir: '/w', ...s }])); +} + +describe('the desktop home rail', () => { + it('says "exited" for an idle session whose agent exited, and measures from the exit', () => { + const app = loadApp(); + app.sessions = sessions([{ id: 'gone', paneExit: { status: 3, at: EXIT_AT }, lastActivityAt: 5 }]); + app.sessionOrder = ['gone']; + + const [row] = app.buildHomeSessionRows(); + expect(row.state).toBe('idle'); + expect(row.display).toBe('exited'); + expect(row.pill).toBe('exited'); + expect(row.since).toEqual({ key: 'exited', at: EXIT_AT }); + }); + + it('draws a neutral row: no idle or working class on the row, the dot or the pill', () => { + // A pane whose agent died mid-turn still has `status: 'busy'`, so without + // the display class its dot would pulse green beside "exited". + const app = loadApp(); + app.sessions = sessions([{ id: 'gone', status: 'busy', paneExit: { at: EXIT_AT } }]); + app.sessionOrder = ['gone']; + + const [row] = app.buildHomeSessionRows(); + expect(row.state).toBe('working'); + const classes = classNames(app._buildHomeSessionRow(row)); + expect(classes).toEqual( + expect.arrayContaining([ + 'home-sessions-row home-sessions-row--exited', + 'home-sessions-dot home-sessions-dot--exited', + 'home-sessions-pill home-sessions-pill--exited', + ]) + ); + expect(classes.join(' ')).not.toMatch(/--(working|idle)\b/); + }); + + it('lets a pending permission prompt win over the exit', () => { + const app = loadApp(); + app.sessions = sessions([{ id: 'blocked', paneExit: { status: 0, at: EXIT_AT } }]); + app.sessionOrder = ['blocked']; + app.pendingHooks = new Map([['blocked', new Set(['permission_prompt'])]]); + + const [row] = app.buildHomeSessionRows(); + expect(row.display).toBe('needs'); + expect(row.pill).toBe('needs you'); + }); + + it('leaves a session without an exit exactly as it was', () => { + const app = loadApp(); + app.sessions = sessions([{ id: 'live', lastActivityAt: 5 }]); + app.sessionOrder = ['live']; + + const [row] = app.buildHomeSessionRows(); + expect(row.display).toBe('idle'); + expect(row.pill).toBe('idle'); + expect(row.since).toEqual({ key: 'idle', at: 5 }); + }); + + it('degrades to no override when a stale cached app.js lacks paneExitLabel', () => { + const app = loadApp({ withPaneExitLabel: false }); + app.sessions = sessions([{ id: 'gone', paneExit: { status: 3, at: EXIT_AT } }]); + app.sessionOrder = ['gone']; + + expect(app.buildHomeSessionRows()[0].pill).toBe('idle'); + }); +}); + +describe('the phone overview', () => { + it('says "exited" and keeps the row in its section', () => { + const app = loadApp(); + const model = app.buildMobileOverviewModel({ + sessions: sessions([ + { id: 'gone', paneExit: { status: 137, at: EXIT_AT } }, + { id: 'live', lastActivityAt: 5 }, + ]), + cases: [], + sessionOrder: ['gone', 'live'], + }); + + const byId = Object.fromEntries(model.current.map((r: any) => [r.id, r])); + expect(byId.gone).toMatchObject({ state: 'idle', display: 'exited', pill: 'exited' }); + expect(byId.gone.since).toEqual({ key: 'exited', at: EXIT_AT }); + expect(byId.live).toMatchObject({ display: 'idle', pill: 'idle' }); + }); + + it('draws a neutral row', () => { + const app = loadApp(); + const model = app.buildMobileOverviewModel({ + sessions: sessions([{ id: 'gone', status: 'busy', paneExit: { at: EXIT_AT } }]), + cases: [], + }); + app._pendingApprovalForSession = () => null; + + const classes = classNames(app._buildMobileOverviewRow(model.current[0])); + expect(classes).toEqual( + expect.arrayContaining([ + 'mobile-overview-row mobile-overview-row--exited', + 'mobile-overview-dot mobile-overview-dot--exited', + 'mobile-overview-pill mobile-overview-pill--exited', + ]) + ); + expect(classes.join(' ')).not.toMatch(/--(working|idle)\b/); + }); +}); + +describe('the exited pill styles', () => { + it('gives both home screens a neutral exited pill', () => { + expect(readFileSync(resolve(PUBLIC, 'styles.css'), 'utf8')).toMatch(/\.home-sessions-pill--exited \{/); + expect(readFileSync(resolve(PUBLIC, 'mobile.css'), 'utf8')).toMatch(/\.mobile-overview-pill--exited \{/); + }); +}); diff --git a/test/session-pane-exit-ui.test.ts b/test/session-pane-exit-ui.test.ts index 84a71032a..cd7ffeb1e 100644 --- a/test/session-pane-exit-ui.test.ts +++ b/test/session-pane-exit-ui.test.ts @@ -163,9 +163,12 @@ describe('the rich row pill of an exited session', () => { // which reads `status` and knows nothing about the exit, so without an override // the muted dot sat beside a pill saying "idle". const appJs = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8'); - const fn = (re: RegExp, name: string) => { - const m = appJs.match(re)?.[0]; - if (!m) throw new Error(`${name} not found in app.js`); + // The exit rule itself is shared with both home screens and lives in + // mobile-overview.js, so it is lifted from there rather than stubbed. + const overviewJs = readFileSync(resolve(import.meta.dirname, '../src/web/public/mobile-overview.js'), 'utf8'); + const fn = (re: RegExp, name: string, source = appJs) => { + const m = source.match(re)?.[0]; + if (!m) throw new Error(`${name} not found`); return m; }; type Row = { state: string; exited: boolean; pill: string; since: { key: string; at: number } | null }; @@ -174,6 +177,7 @@ describe('the rich row pill of an exited session', () => { return { ${fn(/ {2}_sidebarRichPillLabel\(state\) \{[\s\S]*?\n {2}\}/, '_sidebarRichPillLabel')}, ${fn(/ {2}_sidebarRichRow\(id, session\) \{[\s\S]*?\n {2}\}/, '_sidebarRichRow')}, + ${fn(/ {2}_mobileOverviewExit\(state, session\) \{[\s\S]*?\n {2}\}/, '_mobileOverviewExit', overviewJs)}, _mobileOverviewState(session, hooks) { if (hooks && hooks.has('permission_prompt')) return 'needs'; if (hooks && hooks.has('idle_prompt')) return 'waiting'; From ea18bc668651ad4b34ab9678f245b222cad8f8eb Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Wed, 23 Sep 2026 18:34:39 +0200 Subject: [PATCH 4/5] fix(cleanup): close the gaps review found in the #446 sweep and image guard Four fixes from a dual review of Ark0N/Codeman#446 part 2. - The .claude-images guard compares canonical paths, so a sibling that reaches the same directory through a symlink keeps it. Its comment used to say that case only missed a deletion; it caused one. - A detached session counts as a live sibling. DELETE ?killMux=false removes it from the server's map while its pane keeps running, so the guard now reads persisted records too, and exempts only sessions being killed rather than every session in cleaningUp. - A session being closed refuses startInteractive() and startShell(). The /interactive route awaits listener setup before the start, and a start that raced the close could launch a CLI in a tmux session whose record was then deleted. A failed close clears the mark again. - The clean-exit sweep tries each exit once, keyed by session id and the exit's at stamp, so a close that fails is not retried and logged every two seconds. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/architecture-invariants.md | 2 +- src/session.ts | 21 ++++++ src/web/paste-image-gc.ts | 74 ++++++++++++++----- src/web/server.ts | 45 ++++++++++- test/pane-exit-sweep.test.ts | 66 +++++++++++++++++ test/paste-image-dir-shared.test.ts | 111 +++++++++++++++++++++------- 6 files changed, 271 insertions(+), 48 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 804be7a91..e77e626ce 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -247,7 +247,7 @@ Further detail, closing: ⚠️ **Closing has the mirror-image race and one owne - **At least `CLEAN_EXIT_CONFIRMING_READS` (2) authoritative reads must agree.** `TmuxManager.getPaneExitReadCount()` counts them. A repeat of the same pane pid, status and signal adds one, anything else starts again at 1, and a failed, empty or skipped read never reaches `applyPaneExits()`, so it neither confirms nor resets. A mux without the method never has a session closed. - **No start, attach or relaunch may be in flight** (`Session.paneLifecycleInFlight`, raised for the whole of `_setupOrAttachMuxSession()` and `restartCli()`). The dead-pane respawn revives an exited pane on purpose, and the pane reads as dead until `clearPaneExitForNewPane()` runs after its startup delay. -Scoping needs no check of its own here: `setPaneExit()` already forces `paneExit` to UNKNOWN for direct-PTY, remote, docker and discovered sessions. There is no setting, by the maintainer's decision on #446. ⚠️ Do not flip local panes to `remain-on-exit failed` to get the same effect: a destroyed pane ends the tmux session, the PTY exit nulls the pid, and the browser's `selectSession()` then launches a fresh CLI. `cleanupSession()` keeps `{workingDir}/.claude-images` while another live session shares that working directory (`pasteImageDirInUseByOtherSession()`, `paste-image-gc.ts`), since the sweep would otherwise routinely delete a live sibling's pasted images. `planRebootRestore()` refuses a record whose persisted `paneExit` is clean (`agent-exited`), which covers an agent that exited just before the power went, before the sweep reached it. Tests: `test/pane-exit-sweep.test.ts`, `test/paste-image-dir-shared.test.ts`, `test/reboot-restore.test.ts`, `test/tmux-manager.test.ts`. +Scoping needs no check of its own here: `setPaneExit()` already forces `paneExit` to UNKNOWN for direct-PTY, remote, docker and discovered sessions. There is no setting, by the maintainer's decision on #446. ⚠️ Do not flip local panes to `remain-on-exit failed` to get the same effect: a destroyed pane ends the tmux session, the PTY exit nulls the pid, and the browser's `selectSession()` then launches a fresh CLI. `cleanupSession()` keeps `{workingDir}/.claude-images` while another session still uses that directory (`pasteImageDirInUseByOtherSession()`, `paste-image-gc.ts`), since the sweep would otherwise routinely delete a live sibling's pasted images. Paths are compared by `realpath`, and a detached session counts through its persisted record, because `killMux=false` removes it from the map while its pane keeps running; only a session being KILLED is exempt. Each exit gets ONE close attempt (keyed by session id and `at`), and a session being closed refuses `startInteractive()`/`startShell()` (`Session.markClosing()`), so a start that races the close cannot orphan a tmux session. `planRebootRestore()` refuses a record whose persisted `paneExit` is clean (`agent-exited`), which covers an agent that exited just before the power went, before the sweep reached it. Tests: `test/pane-exit-sweep.test.ts`, `test/paste-image-dir-shared.test.ts`, `test/reboot-restore.test.ts`, `test/tmux-manager.test.ts`. ### Dead-pane respawn: the resume pin diff --git a/src/session.ts b/src/session.ts index 77952348f..1c61e50ed 100644 --- a/src/session.ts +++ b/src/session.ts @@ -590,6 +590,11 @@ export class Session extends EventEmitter { * operations cannot clear each other's mark. */ private _paneLifecycleOps = 0; + /** + * The server has started closing this session, so no start or attach may + * begin (see {@link markClosing}). + */ + private _closing = false; /** * This session was rebuilt from the tmux socket rather than from Codeman's * own records, so its `remote`/`docker` metadata is missing rather than known @@ -1201,6 +1206,16 @@ export class Session extends EventEmitter { return this._paneLifecycleOps > 0; } + /** + * Mark this session as being closed, or clear the mark after a close that + * failed. While it is set, {@link startInteractive} and {@link startShell} + * refuse to run. A start that raced a close would otherwise launch a CLI in a + * tmux session whose record is about to be deleted (Ark0N/Codeman#446). + */ + markClosing(closing: boolean): void { + this._closing = closing; + } + /** Run one pane start, attach or relaunch with {@link paneLifecycleInFlight} raised. */ private async _withPaneLifecycle(op: () => Promise): Promise { this._paneLifecycleOps++; @@ -2499,6 +2514,9 @@ export class Session extends EventEmitter { if (this.ptyProcess) { throw new Error('Session already has a running process'); } + if (this._closing) { + throw new Error('Session is being closed'); + } // Bounds the workspace-trust scan (see _maybeAcceptTrustDialog). Stamped here // rather than at PTY spawn so a slow mux attach still counts as startup. @@ -3320,6 +3338,9 @@ export class Session extends EventEmitter { if (this.ptyProcess) { throw new Error('Session already has a running process'); } + if (this._closing) { + throw new Error('Session is being closed'); + } this._resetBuffers(); diff --git a/src/web/paste-image-gc.ts b/src/web/paste-image-gc.ts index 7325bdff5..8cee31ce3 100644 --- a/src/web/paste-image-gc.ts +++ b/src/web/paste-image-gc.ts @@ -12,6 +12,7 @@ * image dir. */ import fs from 'node:fs/promises'; +import { realpathSync } from 'node:fs'; import { join, resolve } from 'node:path'; import type { SessionPort } from './ports/index.js'; @@ -53,6 +54,27 @@ export async function sweepPasteImagesOnce( return { scanned, deleted }; } +/** + * The path two sessions must share to share a paste-image dir: the canonical + * path when it can be resolved, so a sibling that reaches the same directory + * through a symlink matches, and the normalised path otherwise (a directory + * that no longer exists has nothing left to protect). + */ +function canonicalDir(dir: string): string { + try { + return realpathSync(dir); + } catch { + return resolve(dir); + } +} + +/** One session the paste-image guard weighs: its id, directory and, for a persisted record, its status. */ +export interface PasteImageDirUser { + id: string; + workingDir: string; + status?: string; +} + /** * Does another live session still use this working directory's paste-image * dir? Deleting a session removes `{workingDir}/.claude-images` recursively, @@ -60,27 +82,41 @@ export async function sweepPasteImagesOnce( * check closing one session deletes the pasted images a sibling in the same * case still refers to. * - * A session that is itself being cleaned up does not count as live. Without - * that exemption, closing two sessions of one case concurrently (a bulk - * delete, or the exited-agent sweep closing two panes on one tick) would have - * each defer to the other, and neither would remove the dir. + * Two kinds of sibling count as live: + * + * - a session in the server's map, unless it is itself being killed; + * - a persisted record whose status is not `stopped`. That covers a session + * detached with `killMux=false`, which leaves the server's map while its + * tmux pane keeps running, and a session whose detach is still in progress. * - * Paths are compared after `resolve()`, which normalises a trailing slash and - * `..` segments. Symlinks are not resolved: a sibling that reaches the same - * directory through a symlink only costs a missed deletion here, and the - * periodic sweep above still ages those files out. + * A session being KILLED does not count. Without that exemption, killing two + * sessions of one case concurrently (a bulk delete, or the exited-agent sweep + * closing two panes on one tick) would have each defer to the other, and + * neither would remove the dir. + * + * Erring toward "in use" only costs a missed deletion, which the periodic + * sweep above ages out. A pinned record whose tmux session is gone keeps its + * status through boot pruning, so it holds the dir this way until unpinned. */ -export function pasteImageDirInUseByOtherSession( - sessions: Iterable<{ id: string; workingDir: string }>, - closingId: string, - workingDir: string, - closing: ReadonlySet -): boolean { - const target = resolve(workingDir); - for (const session of sessions) { - if (session.id === closingId || closing.has(session.id)) continue; - if (!session.workingDir) continue; - if (resolve(session.workingDir) === target) return true; +export function pasteImageDirInUseByOtherSession(input: { + live: Iterable; + persisted: Iterable; + closingId: string; + workingDir: string; + killing: ReadonlySet; +}): boolean { + const target = canonicalDir(input.workingDir); + const matches = (user: PasteImageDirUser): boolean => + user.id !== input.closingId && + !input.killing.has(user.id) && + !!user.workingDir && + canonicalDir(user.workingDir) === target; + for (const user of input.live) { + if (matches(user)) return true; + } + for (const user of input.persisted) { + if (user.status === 'stopped') continue; + if (matches(user)) return true; } return false; } diff --git a/src/web/server.ts b/src/web/server.ts index 19d1cab1d..c6e808537 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1322,16 +1322,32 @@ export class WebServer extends EventEmitter { // Clean up all resources associated with a session // Track sessions currently being cleaned up to prevent concurrent cleanup races private cleaningUp: Set = new Set(); + /** + * The subset of {@link cleaningUp} whose tmux session is being KILLED rather + * than detached. The paste-image guard needs the difference: a detaching + * session keeps running in tmux and still uses its working directory. + */ + private killingSessions: Set = new Set(); private async cleanupSession(sessionId: string, killMux: boolean = true, reason?: string): Promise { // Guard against concurrent cleanup of the same session if (this.cleaningUp.has(sessionId)) return; this.cleaningUp.add(sessionId); + if (killMux) this.killingSessions.add(sessionId); + // Refuse a start or attach from here on (Ark0N/Codeman#446): a start that + // raced this cleanup would launch a CLI in a tmux session whose record is + // about to be deleted, leaving an orphan the next boot rediscovers. + const session = this.sessions.get(sessionId); + session?.markClosing(true); try { await this._doCleanupSession(sessionId, killMux, reason); } finally { this.cleaningUp.delete(sessionId); + this.killingSessions.delete(sessionId); + // A cleanup that failed leaves the session on the board, so it must be + // startable again. + if (this.sessions.get(sessionId) === session) session?.markClosing(false); } } @@ -1489,7 +1505,17 @@ export class WebServer extends EventEmitter { if ( killMux && session.workingDir && - !pasteImageDirInUseByOtherSession(this.sessions.values(), sessionId, session.workingDir, this.cleaningUp) + !pasteImageDirInUseByOtherSession({ + live: this.sessions.values(), + persisted: Object.entries(this.store.getSessions()).map(([id, record]) => ({ + id, + workingDir: record.workingDir, + status: record.status, + })), + closingId: sessionId, + workingDir: session.workingDir, + killing: this.killingSessions, + }) ) { const pasteImageDir = join(session.workingDir, '.claude-images'); try { @@ -2546,10 +2572,23 @@ export class WebServer extends EventEmitter { * The close runs in the background. `cleanupSession()` ignores a second call * for a session it is already closing, and the `closing` check below keeps * the next tick from queueing one. + * + * Each exit is attempted ONCE, keyed by session id and the exit's `at` + * stamp. A close that fails leaves the session on the board with its exit + * badge, which is where a crashed agent's row would be too, rather than + * retrying and logging every two seconds. A new exit in the same pane has a + * new `at` and gets its own attempt. */ + /** Exits the clean-exit sweep has already tried to close, as `:`. */ + private cleanExitCloseAttempts: Set = new Set(); + private closeCleanlyExitedSessions(): void { const readCount = this.mux.getPaneExitReadCount?.bind(this.mux); if (!readCount) return; + // Forget attempts for sessions that are gone, so the set stays bounded. + for (const key of this.cleanExitCloseAttempts) { + if (!this.sessions.has(key.slice(0, key.lastIndexOf(':')))) this.cleanExitCloseAttempts.delete(key); + } for (const session of [...this.sessions.values()]) { const muxName = session.muxName; if (!muxName) continue; @@ -2560,6 +2599,9 @@ export class WebServer extends EventEmitter { closing: this.cleaningUp.has(session.id), }); if (!close) continue; + const attempt = `${session.id}:${session.paneExit?.at ?? 0}`; + if (this.cleanExitCloseAttempts.has(attempt)) continue; + this.cleanExitCloseAttempts.add(attempt); console.log(`[Server] Closing session ${session.id} (${session.name}): ${CLEAN_EXIT_CLOSE_REASON}`); void this.cleanupSession(session.id, true, CLEAN_EXIT_CLOSE_REASON).catch((err) => { console.error(`[Server] Failed to close cleanly exited session ${session.id}:`, err); @@ -4013,6 +4055,7 @@ export class WebServer extends EventEmitter { } this.activePlanOrchestrators.clear(); this.cleaningUp.clear(); + this.killingSessions.clear(); // Dispose push store (flush pending saves) this.pushStore.dispose(); diff --git a/test/pane-exit-sweep.test.ts b/test/pane-exit-sweep.test.ts index c8869ea70..9b67831d4 100644 --- a/test/pane-exit-sweep.test.ts +++ b/test/pane-exit-sweep.test.ts @@ -158,6 +158,29 @@ describe('the sweep on a real server', () => { expect(cleanup).toHaveBeenCalledWith(session.id, true, CLEAN_EXIT_CLOSE_REASON); }); + it('tries each exit once, so a failed close is not retried every tick', () => { + const { web, tick, cleanup } = build({ status: 0, at: AT }, 2); + const { session } = addSession(web); + + tick(); + tick(); + tick(); + expect(cleanup).toHaveBeenCalledTimes(1); + // The spy resolved without removing the session, which is what a failed + // close looks like from here: the row stays, with its exit badge. + expect(session.paneExit).toEqual({ status: 0, at: AT }); + }); + + it('gives a new exit in the same pane its own attempt', () => { + const { web, state, tick, cleanup } = build({ status: 0, at: AT }, 2); + addSession(web); + + tick(); + state.exit = { status: 0, at: AT + 60_000 }; + tick(); + expect(cleanup).toHaveBeenCalledTimes(2); + }); + it('keeps a crashed agent on the board', () => { const { web, tick, cleanup } = build({ status: 137, at: AT }, 5); addSession(web); @@ -211,6 +234,49 @@ describe('the sweep on a real server', () => { }); }); +describe('a session the server is closing', () => { + const session = () => + new Session({ + workingDir: '/tmp', + mode: 'shell', + useMux: true, + mux: { isAvailable: () => true } as unknown as TerminalMultiplexer, + muxSession: { muxName: 'codeman-aaaa', sessionId: 'aaaa' } as unknown as MuxSession, + }); + + it('refuses to start, so a racing start cannot orphan a tmux session', async () => { + const s = session(); + s.markClosing(true); + await expect(s.startInteractive()).rejects.toThrow('Session is being closed'); + await expect(s.startShell()).rejects.toThrow('Session is being closed'); + }); + + it('is cleared again when the server gives the close up', async () => { + const web = new WebServer(PORT, false, true); + try { + const s = session(); + const sessions = (web as unknown as { sessions: Map }).sessions; + sessions.set(s.id, s); + const internals = web as unknown as { + _doCleanupSession: (...a: unknown[]) => Promise; + cleanupSession: (id: string, kill: boolean, reason: string) => Promise; + }; + let closingDuring = false; + vi.spyOn(internals, '_doCleanupSession').mockImplementation(async () => { + closingDuring = (s as unknown as { _closing: boolean })._closing; + throw new Error('layout prune unavailable'); + }); + + await expect(internals.cleanupSession(s.id, true, 'test')).rejects.toThrow('layout prune unavailable'); + expect(closingDuring).toBe(true); + expect((s as unknown as { _closing: boolean })._closing).toBe(false); + } finally { + vi.restoreAllMocks(); + await web.stop(); + } + }); +}); + describe('the pane lifecycle mark on Session', () => { it('is raised for the whole of restartCli and lowered afterwards, even on failure', async () => { let seenDuring: boolean | null = null; diff --git a/test/paste-image-dir-shared.test.ts b/test/paste-image-dir-shared.test.ts index 71c6cfdc3..1adc5a539 100644 --- a/test/paste-image-dir-shared.test.ts +++ b/test/paste-image-dir-shared.test.ts @@ -11,10 +11,10 @@ * * Port: 3188 */ -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { WebServer } from '../src/web/server.js'; import { pasteImageDirInUseByOtherSession } from '../src/web/paste-image-gc.js'; @@ -22,45 +22,86 @@ const PORT = 3188; describe('pasteImageDirInUseByOtherSession', () => { const none = new Set(); + const check = ( + live: Array<{ id: string; workingDir: string }>, + opts: { persisted?: Array<{ id: string; workingDir: string; status?: string }>; killing?: Set } = {} + ) => + pasteImageDirInUseByOtherSession({ + live, + persisted: opts.persisted ?? [], + closingId: 'a', + workingDir: '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/work/case', + killing: opts.killing ?? none, + }); it('finds a live sibling in the same working directory', () => { - const sessions = [ - { id: 'a', workingDir: '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/work/case' }, - { id: 'b', workingDir: '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/work/case' }, - ]; - expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', none)).toBe(true); + expect( + check([ + { id: 'a', workingDir: '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/work/case' }, + { id: 'b', workingDir: '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/work/case' }, + ]) + ).toBe(true); }); it('ignores the session being closed', () => { - const sessions = [{ id: 'a', workingDir: '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/work/case' }]; - expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', none)).toBe(false); + expect(check([{ id: 'a', workingDir: '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/work/case' }])).toBe(false); }); it('ignores sessions in other directories, including a subdirectory', () => { - const sessions = [ - { id: 'a', workingDir: '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/work/case' }, - { id: 'b', workingDir: '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/work/other' }, - { id: 'c', workingDir: '/work/case/sub' }, - ]; - expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', none)).toBe(false); + expect( + check([ + { id: 'a', workingDir: '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/work/case' }, + { id: 'b', workingDir: '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/work/other' }, + { id: 'c', workingDir: '/work/case/sub' }, + ]) + ).toBe(false); }); it('normalises a trailing slash and dot segments', () => { - const sessions = [ - { id: 'a', workingDir: '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/work/case' }, - { id: 'b', workingDir: '/work/x/../case/' }, - ]; - expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', none)).toBe(true); + expect(check([{ id: 'b', workingDir: '/work/x/../case/' }])).toBe(true); }); - it('does not count a sibling that is being closed too', () => { - // Two sessions of one case closed together must not each defer to the + it('does not count a sibling that is being killed too', () => { + // Two sessions of one case killed together must not each defer to the // other, or neither removes the dir. - const sessions = [ - { id: 'a', workingDir: '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/work/case' }, - { id: 'b', workingDir: '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/work/case' }, - ]; - expect(pasteImageDirInUseByOtherSession(sessions, 'a', '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/work/case', new Set(['a', 'b']))).toBe(false); + expect(check([{ id: 'b', workingDir: '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/work/case' }], { killing: new Set(['a', 'b']) })).toBe(false); + }); + + it('counts a detached session, which left the map but still runs in tmux', () => { + // `DELETE ?killMux=false` removes the session from the server's map and + // keeps its persisted record and its pane. + expect(check([], { persisted: [{ id: 'b', workingDir: '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/work/case', status: 'idle' }] })).toBe(true); + }); + + it('does not count a persisted record demoted to stopped, or the closing session', () => { + expect( + check([], { + persisted: [ + { id: 'a', workingDir: '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/work/case', status: 'idle' }, + { id: 'b', workingDir: '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/work/case', status: 'stopped' }, + ], + }) + ).toBe(false); + }); + + it('matches a sibling that reaches the same directory through a symlink', () => { + const root = mkdtempSync(join(tmpdir(), 'codeman-paste-link-')); + try { + const real = join(root, 'case'); + mkdirSync(real); + symlinkSync(real, join(root, 'current')); + expect( + pasteImageDirInUseByOtherSession({ + live: [{ id: 'b', workingDir: join(root, 'current') }], + persisted: [], + closingId: 'a', + workingDir: real, + killing: none, + }) + ).toBe(true); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); }); @@ -105,4 +146,20 @@ describe('deleting a session that shares its working directory', () => { expect((await remove(second)).status).toBe(200); expect(existsSync(imageDir)).toBe(false); }); + + it('keeps the images while a sibling is only detached, since it still runs in tmux', async () => { + const detached = await create(); + const deleted = await create(); + // Persisting a new record is debounced, and a detach cancels the pending + // write, so wait for the record a long-running session would already have. + const store = (server as unknown as { store: { getSession: (id: string) => unknown } }).store; + await vi.waitFor(() => expect(store.getSession(detached)).toBeTruthy(), { timeout: 10_000 }); + const imageDir = join(workingDir, '.claude-images'); + mkdirSync(imageDir, { recursive: true }); + writeFileSync(join(imageDir, 'paste-2.png'), 'x'); + + expect((await fetch(`${base}/api/sessions/${detached}?killMux=false`, { method: 'DELETE' })).status).toBe(200); + expect((await remove(deleted)).status).toBe(200); + expect(existsSync(join(imageDir, 'paste-2.png'))).toBe(true); + }); }); From e114ce9385b254c02e34db7ad836c5817d13719d Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Thu, 24 Sep 2026 22:09:06 +0200 Subject: [PATCH 5/5] fix(session): keep a clean exit that lands within 10 s of a pane start (#446) A CLI that prints a startup error ("not logged in", a bad profile, a config error) and exits 0 used to lose its tab, and the error with it, about 4 s after launch. The sweep now keeps any clean exit that lands within CLEAN_EXIT_MIN_PANE_LIFETIME_MS (10 s) of the last start, attach or relaunch finishing (Session.paneStartedAt, stamped when _withPaneLifecycle ends). The row stays as "exited (0)" for the user to read and close. Verified on an isolated instance: a shell that ran `exit 0` 2 s after start kept its row, one that exited after 13 s was closed. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 3 ++- src/pane-exit-sweep.ts | 25 +++++++++++++++++++++++++ src/session.ts | 15 +++++++++++++++ src/web/server.ts | 3 ++- test/pane-exit-sweep.test.ts | 24 ++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 97f2fcd14..bc9cc407f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -205,7 +205,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph ⚠️ **A quiet pane is not always a pane that wants you.** A CLI can declare an optional `capabilities.workDetect.watchingLine` (a monitor, background shell or cloud hand-off it is still running); the idle probe reads it into `Session.watching` and `notePrompt()` opens that idle item ALREADY acknowledged, so no surface alerts. Only `idle` is eligible, and the label is pane-derived and prompt-injectable, so a pattern must anchor on chrome only that CLI draws. → [architecture-invariants#the-watching-signal-a-quiet-pane-that-is-not-waiting-for-you](docs/architecture-invariants.md#the-watching-signal-a-quiet-pane-that-is-not-waiting-for-you). Tests: `test/session-watching.test.ts`, `test/watching-no-alert.test.ts`. -**An exited agent in a live pane** (`paneExit`, #446): panes use `remain-on-exit on`, so `/exit` leaves a pane, session and pid that look alive; `TmuxManager.startPaneExitWatcher()` publishes `SessionState.paneExit` via `session:updated`. ⚠️ Never set `status: 'error'` or null the `pid` for it; the field is TRI-STATE (absent = UNKNOWN, never alive, scoped by `Session.paneExitApplies`); an absent `#{pane_dead_status}` is not 0; a path that starts a command in a pane must clear the record AND persist. A clean exit is CLOSED via `cleanupSession()` (`pane-exit-sweep.ts`): only an explicit numeric status 0 with no signal, confirmed by 2 reads, with no start/attach in flight (`paneLifecycleInFlight`); a crashed agent keeps its row. → [architecture-invariants#an-exited-agent-in-a-live-pane-paneexit](docs/architecture-invariants.md#an-exited-agent-in-a-live-pane-paneexit) +**An exited agent in a live pane** (`paneExit`, #446): panes use `remain-on-exit on`, so `/exit` leaves a pane, session and pid that look alive; `TmuxManager.startPaneExitWatcher()` publishes `SessionState.paneExit` via `session:updated`. ⚠️ Never set `status: 'error'` or null the `pid` for it; the field is TRI-STATE (absent = UNKNOWN, never alive, scoped by `Session.paneExitApplies`); an absent `#{pane_dead_status}` is not 0; a path that starts a command in a pane must clear the record AND persist. A clean exit is CLOSED via `cleanupSession()` (`pane-exit-sweep.ts`): only an explicit numeric status 0 with no signal, confirmed by 2 reads, with no start/attach in flight (`paneLifecycleInFlight`) and not within 10 s of one (a startup error keeps its row); a crashed agent keeps its row. → [architecture-invariants#an-exited-agent-in-a-live-pane-paneexit](docs/architecture-invariants.md#an-exited-agent-in-a-live-pane-paneexit) **Dead-pane respawn resume pin** (`_buildRespawnPaneOptionsWithResumePin()`, session.ts): recovering a dead pane, like a custom-model `restartCli()`, must pin the conversation or claude refuses the reused `--session-id`. The pin takes the first transcript-backed candidate (chain tail, launch seed, own id), never `_claudeSessionId`, adds nothing when none is backed, and is never applied to remote or docker sessions. → [architecture-invariants#dead-pane-respawn-the-resume-pin](docs/architecture-invariants.md#dead-pane-respawn-the-resume-pin) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index e77e626ce..6a25c5b34 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -241,11 +241,12 @@ Further detail, closing: ⚠️ **Closing has the mirror-image race and one owne **Codeman creates every pane with `remain-on-exit on`, so a session whose agent exited still looks alive.** `/exit` ends the CLI, tmux keeps the pane and the tmux session, and the `tmux attach-session` process Codeman records as `Session.pid` runs on, so no PTY exit handler fires and the record keeps its pid and `status: 'idle'` (Ark0N/Codeman#446). `SessionState.paneExit` (`{status?, signal?, at}`) is the fact tmux already knows, published through `toState()` so it rides `session:updated` and lands in `state.json` on the same persist — there is no SSE event for it. One batched `tmux list-panes -a` per tick fills it, from `TmuxManager.startPaneExitWatcher()`, which has its OWN always-on interval: the stats collector cannot carry it, because the browser arms and disarms that one with the Monitor panel (`panels-ui.js`) and boot skips it entirely when no session was recovered. ⚠️ **The field is TRI-STATE and its third state is absence**, meaning UNKNOWN, which renders as nothing and must NEVER read as alive; it covers a running pane, a session the read did not list, a failed probe, and every session shape a dead local pane does not describe. `Session.paneExitApplies` is the single place that scoping lives, and it fails closed for four shapes: a direct-PTY session (no pane), a remote SSH session (the local pane is the ssh client, whose death is a transport drop OR an exit — the whole of #355), a docker case (the local pane is a `docker exec` into the container's own tmux), and a session rebuilt from the socket (`MuxSession.discovered`: its synthetic `restored-` id matches no `state.json` entry, so a remote session rediscovered after `mux-sessions.json` was lost would arrive looking local). ⚠️ **Never set `status: 'error'`** for an exited pane — that value is the PTY-exit breaker's and the browser answers it with a "restart it?" confirm — and **never null the `pid`**, which is what makes `selectSession()` re-attach and launch a fresh CLI. Local panes keep `remain-on-exit on`; flipping them to `failed` ends the tmux session, nulls the pid and reintroduces the auto-revive #355 removed. ⚠️ **An absent `#{pane_dead_status}` is not 0**: measured on tmux 3.2a a SIGKILLed pane reports neither a status nor a signal (`#{pane_dead_signal}` did not exist before tmux 3.4), so folding it into 0 would turn an unexplained death into a clean exit. A session answers only when the read listed EXACTLY ONE pane for it, since Codeman never splits a pane and a session the user split by hand has none that speaks for the agent. The three synchronous `isPaneDead()` callers (the `/wait` route, the TUI, the attach path) keep their own probes — this watcher is never fresh enough for them. ⚠️ **A path that starts a command in a pane must clear the record AND persist**, since the watcher's next tick sees the field already cleared and writes nothing. ⚠️ **The always-on timer gates the READ, never the tick.** `hasObservablePaneSession()` (`tmux-manager.ts`) skips the tmux exec while every session on the manager is one of the shapes `paneExitApplies` forces to UNKNOWN, so an instance running only remote or Docker work keeps ticking and costs nothing; the two predicates are two copies of one rule, and `test/session-pane-exit.test.ts` pins them against each other for the four session shapes that exist today — a FIFTH condition added to one and not the other still fails nothing, so change them together. Skipping retracts nothing, for the same reason a failed read does not. ⚠️ **The muted status dot is a specificity fight, and it is fought on three surfaces.** The tab renders `status` as before, and `tab-agent-exited` only quiets the dot, so the rule excludes three states BY HAND: `.tab-alert-action` and `.tab-alert-idle` on the tab, and `.tab-status.error` on the dot itself. Each of those colours means "this needs you" — the two alerts because a human is blocked, `error` because the browser answers it with a "restart it?" confirm — and each must survive the exit. The rich tab rail needs a SECOND copy of the rule, because its own `tab-state-*` dot rules are (0,9,1) against the strip's (0,5,0) — measured, an exited session on a detailed rail kept a full green dot and the working halo beside a badge reading "exited". Its twin matches that specificity exactly and therefore must stay BELOW those rules in source order. mobile.css needs a THIRD copy, with `!important`, because the phone block enlarges a `busy` dot and gives it a green glow that way, and `status` stays `busy` for a pane whose agent died mid-turn — without it a phone renders a grey dot still wearing the green halo. `test/session-pane-exit-ui.test.ts` resolves the real stylesheets in jsdom rather than matching selector text — styles.css for the desktop cases and both files for the phone ones — so the ordering, the hand-written exclusions and a missing phone rule all fail there. Tests: `test/session-pane-exit.test.ts`, `test/tmux-manager.test.ts`, `test/session-pane-exit-ui.test.ts`. -**A session whose agent exited cleanly is closed, and a crashed one is kept** (Ark0N/Codeman#446, part 2). After every pane read, `closeCleanlyExitedSessions()` (`server.ts`) closes each session that `shouldCloseCleanlyExitedSession()` (`pane-exit-sweep.ts`, pure) accepts, through `cleanupSession(id, true, CLEAN_EXIT_CLOSE_REASON)`. That is the X button's path, so an unpinned session is removed, a pinned one is demoted to `status: 'stopped'`, the lifecycle log records why, and the conversation stays resumable from the Resume list, which reads the lifecycle log and the transcripts rather than the pane. The rule has three parts, and each guards against a wrong close: +**A session whose agent exited cleanly is closed, and a crashed one is kept** (Ark0N/Codeman#446, part 2). After every pane read, `closeCleanlyExitedSessions()` (`server.ts`) closes each session that `shouldCloseCleanlyExitedSession()` (`pane-exit-sweep.ts`, pure) accepts, through `cleanupSession(id, true, CLEAN_EXIT_CLOSE_REASON)`. That is the X button's path, so an unpinned session is removed, a pinned one is demoted to `status: 'stopped'`, the lifecycle log records why, and the conversation stays resumable from the Resume list, which reads the lifecycle log and the transcripts rather than the pane. The rule has four parts, and each guards against a wrong close: - ⚠️ **The status must be an explicit numeric 0 with no signal** (`isCleanPaneExit()`). An absent status is how a SIGKILL presents on tmux 3.2a, so `status ?? 0` would close an agent the OOM killer took. A non-zero status or any signal keeps the row, marked with the exit, as the crash evidence #210 was filed to keep. - **At least `CLEAN_EXIT_CONFIRMING_READS` (2) authoritative reads must agree.** `TmuxManager.getPaneExitReadCount()` counts them. A repeat of the same pane pid, status and signal adds one, anything else starts again at 1, and a failed, empty or skipped read never reaches `applyPaneExits()`, so it neither confirms nor resets. A mux without the method never has a session closed. - **No start, attach or relaunch may be in flight** (`Session.paneLifecycleInFlight`, raised for the whole of `_setupOrAttachMuxSession()` and `restartCli()`). The dead-pane respawn revives an exited pane on purpose, and the pane reads as dead until `clearPaneExitForNewPane()` runs after its startup delay. +- **The pane must have been up for `CLEAN_EXIT_MIN_PANE_LIFETIME_MS` (10 s)** since the last start, attach or relaunch finished (`Session.paneStartedAt`). A CLI that prints a startup error and exits 0 would otherwise lose its tab, and the error with it, seconds after launch; its row stays as `exited (0)` instead. An attach to an already running pane stamps it too, so an `/exit` within seconds of a server restart leaves a row to close by hand. Scoping needs no check of its own here: `setPaneExit()` already forces `paneExit` to UNKNOWN for direct-PTY, remote, docker and discovered sessions. There is no setting, by the maintainer's decision on #446. ⚠️ Do not flip local panes to `remain-on-exit failed` to get the same effect: a destroyed pane ends the tmux session, the PTY exit nulls the pid, and the browser's `selectSession()` then launches a fresh CLI. `cleanupSession()` keeps `{workingDir}/.claude-images` while another session still uses that directory (`pasteImageDirInUseByOtherSession()`, `paste-image-gc.ts`), since the sweep would otherwise routinely delete a live sibling's pasted images. Paths are compared by `realpath`, and a detached session counts through its persisted record, because `killMux=false` removes it from the map while its pane keeps running; only a session being KILLED is exempt. Each exit gets ONE close attempt (keyed by session id and `at`), and a session being closed refuses `startInteractive()`/`startShell()` (`Session.markClosing()`), so a start that races the close cannot orphan a tmux session. `planRebootRestore()` refuses a record whose persisted `paneExit` is clean (`agent-exited`), which covers an agent that exited just before the power went, before the sweep reached it. Tests: `test/pane-exit-sweep.test.ts`, `test/paste-image-dir-shared.test.ts`, `test/reboot-restore.test.ts`, `test/tmux-manager.test.ts`. diff --git a/src/pane-exit-sweep.ts b/src/pane-exit-sweep.ts index c334d9a81..3d71708eb 100644 --- a/src/pane-exit-sweep.ts +++ b/src/pane-exit-sweep.ts @@ -20,6 +20,11 @@ * - No start, attach or relaunch may be in flight for the session. The * dead-pane branch of `Session._setupOrAttachMuxSession()` respawns an exited * pane on purpose, and for a few seconds that pane still reads as dead. + * - The exit must land at least {@link CLEAN_EXIT_MIN_PANE_LIFETIME_MS} after + * the last start, attach or relaunch finished. A CLI that prints a startup + * error ("not logged in", a bad profile, a config error) and exits 0 would + * otherwise lose its tab, and the error with it, seconds after launch. Its + * row stays, marked `exited (0)`, for the user to read and close. * * Scoping to local mux-backed sessions happens before this rule runs: * `Session.setPaneExit()` forces the field to UNKNOWN for direct-PTY, remote, @@ -36,6 +41,13 @@ import type { PaneExit } from './types/index.js'; */ export const CLEAN_EXIT_CONFIRMING_READS = 2; +/** + * How long a pane must have been up before a clean exit closes its session. + * An exit sooner than this after the last pane start is read as a startup + * failure rather than a user ending the agent, and the row is kept. + */ +export const CLEAN_EXIT_MIN_PANE_LIFETIME_MS = 10_000; + /** The lifecycle-log reason recorded when the sweep closes a session. */ export const CLEAN_EXIT_CLOSE_REASON = 'agent exited cleanly (status 0)'; @@ -63,6 +75,11 @@ export interface CleanExitSweepCandidate { paneLifecycleInFlight: boolean; /** The session is already being closed or detached. */ closing: boolean; + /** + * When the last start, attach or relaunch of this pane finished + * (`Session.paneStartedAt`), or 0 when none has run in this process. + */ + paneStartedAt: number; } /** Should the sweep close this session now? See the file overview for the rule. */ @@ -70,5 +87,13 @@ export function shouldCloseCleanlyExitedSession(candidate: CleanExitSweepCandida if (candidate.closing) return false; if (candidate.paneLifecycleInFlight) return false; if (!isCleanPaneExit(candidate.paneExit)) return false; + // `at` is when this server first read the pane dead, so an exit during the + // start itself lands BEFORE `paneStartedAt` and is kept too. + if ( + candidate.paneStartedAt > 0 && + candidate.paneExit!.at - candidate.paneStartedAt < CLEAN_EXIT_MIN_PANE_LIFETIME_MS + ) { + return false; + } return candidate.confirmingReads >= CLEAN_EXIT_CONFIRMING_READS; } diff --git a/src/session.ts b/src/session.ts index 1c61e50ed..e8d1c4a3c 100644 --- a/src/session.ts +++ b/src/session.ts @@ -590,6 +590,8 @@ export class Session extends EventEmitter { * operations cannot clear each other's mark. */ private _paneLifecycleOps = 0; + /** When the last pane start, attach or relaunch finished (ms), 0 when none has run. */ + private _paneStartedAt = 0; /** * The server has started closing this session, so no start or attach may * begin (see {@link markClosing}). @@ -1206,6 +1208,18 @@ export class Session extends EventEmitter { return this._paneLifecycleOps > 0; } + /** + * When the last start, attach or relaunch of this pane finished, or 0 when + * none has run in this process. The exited-agent sweep keeps an exit that + * lands within `CLEAN_EXIT_MIN_PANE_LIFETIME_MS` of it, since that reads as a + * CLI failing at startup rather than a user ending it. An attach to a pane + * that was already running stamps it too, which only costs a user who + * `/exit`s within seconds of a server restart a row to close by hand. + */ + get paneStartedAt(): number { + return this._paneStartedAt; + } + /** * Mark this session as being closed, or clear the mark after a close that * failed. While it is set, {@link startInteractive} and {@link startShell} @@ -1223,6 +1237,7 @@ export class Session extends EventEmitter { return await op(); } finally { this._paneLifecycleOps--; + this._paneStartedAt = Date.now(); } } diff --git a/src/web/server.ts b/src/web/server.ts index c6e808537..c9de58959 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -2564,7 +2564,7 @@ export class WebServer extends EventEmitter { * * `shouldCloseCleanlyExitedSession()` (`pane-exit-sweep.ts`) holds the rule: * an explicit status of 0, confirmed by more than one pane read, with no - * start or attach in flight. A crashed agent keeps its row with the exit + * start or attach in flight and not within seconds of one (a startup error). A crashed agent keeps its row with the exit * code on it. `session.paneExit` is already scoped to local mux-backed * sessions by `setPaneExit()`, so a remote, docker or direct-PTY session is * never closed here. @@ -2597,6 +2597,7 @@ export class WebServer extends EventEmitter { confirmingReads: readCount(muxName), paneLifecycleInFlight: session.paneLifecycleInFlight, closing: this.cleaningUp.has(session.id), + paneStartedAt: session.paneStartedAt, }); if (!close) continue; const attempt = `${session.id}:${session.paneExit?.at ?? 0}`; diff --git a/test/pane-exit-sweep.test.ts b/test/pane-exit-sweep.test.ts index 9b67831d4..6839e846f 100644 --- a/test/pane-exit-sweep.test.ts +++ b/test/pane-exit-sweep.test.ts @@ -28,6 +28,7 @@ import type { MuxSession, TerminalMultiplexer } from '../src/mux-interface.js'; import { CLEAN_EXIT_CLOSE_REASON, CLEAN_EXIT_CONFIRMING_READS, + CLEAN_EXIT_MIN_PANE_LIFETIME_MS, isCleanPaneExit, shouldCloseCleanlyExitedSession, } from '../src/pane-exit-sweep.js'; @@ -65,8 +66,21 @@ describe('shouldCloseCleanlyExitedSession', () => { confirmingReads: CLEAN_EXIT_CONFIRMING_READS, paneLifecycleInFlight: false, closing: false, + paneStartedAt: 0, }; + it('keeps a clean exit that lands within the startup window, as a startup failure', () => { + const justStarted = AT - CLEAN_EXIT_MIN_PANE_LIFETIME_MS + 1; + expect(shouldCloseCleanlyExitedSession({ ...clean, paneStartedAt: justStarted, confirmingReads: 9 })).toBe(false); + // An exit read during the start itself is earlier than the stamp. + expect(shouldCloseCleanlyExitedSession({ ...clean, paneStartedAt: AT + 500, confirmingReads: 9 })).toBe(false); + }); + + it('closes a clean exit once the pane has outlived the startup window', () => { + const settled = AT - CLEAN_EXIT_MIN_PANE_LIFETIME_MS; + expect(shouldCloseCleanlyExitedSession({ ...clean, paneStartedAt: settled })).toBe(true); + }); + it('closes a clean exit that enough reads agreed on', () => { expect(CLEAN_EXIT_CONFIRMING_READS).toBe(2); expect(shouldCloseCleanlyExitedSession(clean)).toBe(true); @@ -158,6 +172,16 @@ describe('the sweep on a real server', () => { expect(cleanup).toHaveBeenCalledWith(session.id, true, CLEAN_EXIT_CLOSE_REASON); }); + it('keeps a session whose pane started moments before the clean exit', () => { + const { web, tick, cleanup } = build({ status: 0, at: AT }, 2); + const { session } = addSession(web); + (session as unknown as { _paneStartedAt: number })._paneStartedAt = AT - 2_000; + + tick(); + expect(cleanup).not.toHaveBeenCalled(); + expect(session.paneExit).toEqual({ status: 0, at: AT }); + }); + it('tries each exit once, so a failed close is not retried every tick', () => { const { web, tick, cleanup } = build({ status: 0, at: AT }, 2); const { session } = addSession(web);