diff --git a/CLAUDE.md b/CLAUDE.md index 1d77d340..c647f7aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -207,7 +207,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Cron (`CronJob`s)**: saved, named jobs on a recurring schedule (`once`/`interval`/`daily`/`weekly`) with per-job run history. ⚠️ **Distinct from the legacy `ScheduledRun`** (`/api/scheduled`, a run-now duration-bounded loop); the two never interact and keep separate `Scheduled*` / `Cron*` names. `CronService` **reuses the existing session layer** rather than rebuilding tmux logic. Next-run math is pure and unit-tested in `cron-time.ts` (server-local timezone). The schedule is advanced BEFORE launch so a slow launch cannot re-trigger. → [architecture-invariants#cron-jobs](docs/architecture-invariants.md#cron-jobs), `docs/cron-discovery.md` -**Remote sessions + remote SSH cases**: a case can point at a remote host. The agent runs inside a durable remote `tmux -L codeman-remote` (session name `codeman-ssh-`, deliberately failing the remote Codeman's `SAFE_MUX_NAME_PATTERN` so an instance on the target host never adopts it), fronted by a LOCAL tmux pane running `ssh`. Attached (`owned:false`) sessions **detach, never kill** on tab close; owned ones propagate `kill-session`. A bounded-backoff watcher auto-reconnects dropped sessions (`remoteAutoReconnect`, default ON). ⚠️ **Command-injection surface: every ssh command line must flow through `buildSshConnectionArgs()`**, which `shellescape`s every user field. Never hand-build an ssh line elsewhere. ⚠️ Run flows must route remote cases through `POST /api/quick-start`, not `POST /api/sessions` (which stat-validates `workingDir` locally and has no `caseName`). → [architecture-invariants#remote-sessions-over-ssh](docs/architecture-invariants.md#remote-sessions-over-ssh), [#remote-ssh-cases](docs/architecture-invariants.md#remote-ssh-cases), `docs/remote-sessions.md` +**Remote sessions + remote SSH cases**: a case can point at a remote host. The agent runs inside a durable remote `tmux -L codeman-remote` (session name `codeman-ssh-`, deliberately failing the remote Codeman's `SAFE_MUX_NAME_PATTERN` so an instance on the target host never adopts it), fronted by a LOCAL tmux pane running `ssh`. Attached (`owned:false`) sessions **detach, never kill** on tab close; owned ones propagate `kill-session`. A bounded-backoff watcher auto-reconnects dropped sessions (`remoteAutoReconnect`, default ON). ⚠️ **It revives ONLY when the durable remote tmux session is verifiably still alive** (`remoteTmuxSessionAlive()`, a `has-session` probe over ssh, #355): a clean agent exit (Ctrl-C, Ctrl-D, `exit`) tears that session down, and `isPaneDead()` cannot tell it from a transport drop, so the watcher used to relaunch a FRESH agent after every clean exit (claude only looked fine because its `|| --resume` fallback masked it). An unreachable host answers `undefined`, which also means do not revive. ⚠️ `has-session` prints NOTHING on success, so the probe is classified by EXIT STATUS (`classifyRemoteAliveExit`: 0 alive, ssh's 255 or a timeout unknown, anything else gone); reading stdout classified every live session as gone and silently disabled transport-drop reconnects. The answer is cached per session and forgotten whenever the pane is seen alive again, or a stale `true` from one transport drop would revive the next clean exit. ⚠️ **Command-injection surface: every ssh command line must flow through `buildSshConnectionArgs()`**, which `shellescape`s every user field. Never hand-build an ssh line elsewhere. ⚠️ Run flows must route remote cases through `POST /api/quick-start`, not `POST /api/sessions` (which stat-validates `workingDir` locally and has no `caseName`). → [architecture-invariants#remote-sessions-over-ssh](docs/architecture-invariants.md#remote-sessions-over-ssh), [#remote-ssh-cases](docs/architecture-invariants.md#remote-ssh-cases), `docs/remote-sessions.md` **Docker cases**: a case can point at a **container**, with any of the CLI run modes running inside it. Like remote-SSH this is a **LOCATION OVERLAY on cases, never a `SessionMode` of its own**. Exactly one long-lived container **per case**, shared by all its sessions, so killing a session kills only that session's in-container tmux and **never** `docker stop` while siblings remain. The workspace is a real host dir bind-mounted at the **same absolute path**, which is what keeps file-routes/watchers on real host bytes and makes the in-container transcript projHash match the host. Credentials are **seeded** (RO mount, copied into the container once) rather than shared RW, so in-container CLIs never write refreshed tokens back to the host, and bind mounts are excluded from `docker commit` so exports stay secret-free. **NEVER a create-time `-e` for secrets, NEVER `--privileged`, NEVER the docker socket.** Config drift is detected via a label hash and a drifted launch is REFUSED rather than silently launched with stale config. ⚠️ On the loopback-only prod bind a container cannot reach 127.0.0.1, so in-container hooks need `CODEMAN_DOCKER_BRIDGE_HOOKS=1`; otherwise idle detection falls back to output-based. → [architecture-invariants#docker-cases](docs/architecture-invariants.md#docker-cases), `docs/docker-cases.md` (user guide), `docs/docker-cases-plan.md` (design) diff --git a/src/remote-hosts.ts b/src/remote-hosts.ts index f8cc0ba5..cb14166c 100644 --- a/src/remote-hosts.ts +++ b/src/remote-hosts.ts @@ -390,15 +390,40 @@ export async function remoteTmuxSessionAlive( if (process.env.VITEST) return true; const command = buildRemoteSessionAliveCommand(remote, remoteSessionName); try { - const { stdout } = await execAsync(command, { timeout: 15_000 }); - // has-session prints the session name on success (exit 0). Anything else is - // a non-zero exit → the session is gone. - return stdout.trim().length > 0; - } catch { - return undefined; + await execAsync(command, { timeout: 15_000 }); + return classifyRemoteAliveExit(0, false); + } catch (err) { + const e = err as { code?: unknown; killed?: boolean }; + return classifyRemoteAliveExit(typeof e.code === 'number' ? e.code : null, e.killed === true); } } +/** + * Map the `has-session` probe's exit status onto the tri-state the watcher + * reads. Pure, so the mapping is unit-tested even though the probe itself is + * VITEST-guarded. + * + * ⚠️ `tmux has-session` prints NOTHING on success (measured: exit 0, empty + * stdout; the failure message goes to stderr), so the exit status is the ONLY + * signal. An earlier version read stdout and therefore classified every live + * remote session as gone, which silently disabled transport-drop reconnects. + * + * - exit 0 → the durable remote session exists → `true`. + * - exit 255 is ssh's own failure (unreachable host, auth, proxy/jump error) + * and a timeout arrives as `killed` with no numeric code: we learned + * nothing about the session → `undefined`, which the watcher treats as + * "do not revive". + * - any other non-zero status is the REMOTE command's: tmux's 1 for a missing + * session, or 127 when tmux is not installed there (no durable session can + * exist without it) → `false`. + */ +export function classifyRemoteAliveExit(code: number | null, killed: boolean): boolean | undefined { + if (killed) return undefined; + if (code === 0) return true; + if (code === null || code === 255) return undefined; + return false; +} + /** * COD-105 — build the SSH command that lists `codeman-*` tmux sessions on a * remote host's canonical `-L codeman` socket. diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 492bcd94..8633bb51 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -1368,6 +1368,13 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { * transport drop). */ private remoteAliveCache: Map = new Map(); + /** + * Sessions with a `has-session` probe currently in flight. The probe is a + * fire-and-forget ssh round-trip with a 15s timeout against a 5s tick, so + * without this an unreachable host would accumulate three overlapping ssh + * processes per dead session. + */ + private remoteAliveInFlight: Set = new Set(); private trueColorConfigured = false; /** tmux 3.7+ can resize pane history after creation; older releases cannot. */ @@ -2732,12 +2739,16 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { */ private async refreshRemoteAlive(session: MuxSession): Promise { if (!session.remote) return; + if (this.remoteAliveInFlight.has(session.sessionId)) return; + this.remoteAliveInFlight.add(session.sessionId); const remoteName = session.remote.remoteSessionName || remoteTmuxSessionName(session.sessionId); try { const alive = await remoteTmuxSessionAlive(session.remote, remoteName); this.remoteAliveCache.set(session.sessionId, alive); } catch { this.remoteAliveCache.set(session.sessionId, undefined); + } finally { + this.remoteAliveInFlight.delete(session.sessionId); } } @@ -2751,7 +2762,15 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { // refreshed lazily so a clean exit (remote tmux gone) flips it to false // on the next tick and stops the auto-revive. const paneDead = this.isPaneDead(session.muxName); - if (paneDead && this.remoteAliveCache.get(sessionId) === undefined) { + if (!paneDead) { + // A live pane makes whatever the probe last said STALE, so forget it: + // after a successful reattach (or a manual restart) the next dead pane + // must be probed afresh. A cached `true` from the transport drop would + // otherwise revive a later CLEAN exit, the exact bug this cache exists + // to prevent, and a cached `false` from a clean exit would leave a + // manually restarted session with auto-reconnect permanently off. + this.remoteAliveCache.delete(sessionId); + } else if (this.remoteAliveCache.get(sessionId) === undefined) { void this.refreshRemoteAlive(session); } const action = decideReconnect({ @@ -2808,6 +2827,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { this.reconnectGuard.add(sessionId); this.reconnectState.delete(sessionId); this.remoteAliveCache.delete(sessionId); + this.remoteAliveInFlight.delete(sessionId); } /** Clear all per-session reconnect + guard state (e.g. when a session is removed). */ @@ -2815,6 +2835,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { this.reconnectState.delete(sessionId); this.reconnectGuard.delete(sessionId); this.remoteAliveCache.delete(sessionId); + this.remoteAliveInFlight.delete(sessionId); } destroy(): void { @@ -2824,6 +2845,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { this.reconnectState.clear(); this.reconnectGuard.clear(); this.remoteAliveCache.clear(); + this.remoteAliveInFlight.clear(); } registerSession(session: MuxSession): void { diff --git a/test/remote-auto-reconnect.test.ts b/test/remote-auto-reconnect.test.ts index b09d2d61..00b2c0fc 100644 --- a/test/remote-auto-reconnect.test.ts +++ b/test/remote-auto-reconnect.test.ts @@ -26,6 +26,7 @@ import { decideReconnect, } from '../src/remote-reconnect.js'; import type { ReconnectSessionView } from '../src/remote-reconnect.js'; +import { buildRemoteSessionAliveCommand, classifyRemoteAliveExit } from '../src/remote-hosts.js'; import { TmuxManager } from '../src/tmux-manager.js'; import type { SessionRemote } from '../src/types.js'; @@ -220,6 +221,34 @@ describe('decideReconnect (pure eligibility)', () => { // (c) MANAGER integration — drive ticks with a stubbed pane-death + clock // ──────────────────────────────────────────────────────────────────────────── +describe('remote has-session probe (pure)', () => { + it('builds the probe through the shared ssh connection args, has-session by name', () => { + const cmd = buildRemoteSessionAliveCommand({ username: 'dev', host: 'box', port: 2222 }, 'codeman-ssh-abc'); + // Literal pin: the session name is shellescaped inside the remote command, + // which is itself one shellescaped ssh argument. + expect(cmd).toBe( + "ssh -o BatchMode=yes -o ConnectTimeout=10 -p 2222 dev@box 'tmux -L codeman-remote has-session -t '\\''codeman-ssh-abc'\\'' 2>/dev/null'" + ); + }); + + // `tmux has-session` prints NOTHING on success (exit 0), so the exit status is + // the only signal; reading stdout classified every live session as gone. + it('exit 0 = alive', () => { + expect(classifyRemoteAliveExit(0, false)).toBe(true); + }); + + it("tmux's 1 (missing session) and 127 (no tmux on the remote) = gone", () => { + expect(classifyRemoteAliveExit(1, false)).toBe(false); + expect(classifyRemoteAliveExit(127, false)).toBe(false); + }); + + it("ssh's 255, a timeout, and a spawn failure = unknown (never revive)", () => { + expect(classifyRemoteAliveExit(255, false)).toBeUndefined(); + expect(classifyRemoteAliveExit(null, true)).toBeUndefined(); + expect(classifyRemoteAliveExit(null, false)).toBeUndefined(); + }); +}); + describe('TmuxManager remote reconnect watcher (integration)', () => { let manager: TmuxManager; @@ -315,6 +344,38 @@ describe('TmuxManager remote reconnect watcher (integration)', () => { expect(dropped).toEqual([]); }); + it('forgets the cached liveness once the pane is alive again, so a later dead pane is probed afresh', async () => { + registerRemote('ffff6666'); + const cache = (manager as unknown as { remoteAliveCache: Map }).remoteAliveCache; + // A clean exit was observed earlier (remote gone) ... + cache.set('ffff6666', false); + // ... then the user restarted the session by hand: the pane is alive. + const paneDead = vi.spyOn(manager, 'isPaneDead').mockReturnValue(false); + manager.runRemoteReconnectTick(0, true); + expect(cache.has('ffff6666')).toBe(false); + + // Now a transport drop. The first dead-pane tick only fires the probe + // (stubbed alive under VITEST); the tick after it sees the fresh answer. + paneDead.mockReturnValue(true); + const dropped: unknown[] = []; + manager.on('remoteSessionDropped', (d) => dropped.push(d)); + manager.runRemoteReconnectTick(1000, true); + expect(dropped).toEqual([]); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(cache.get('ffff6666')).toBe(true); + manager.runRemoteReconnectTick(2000, true); + expect(dropped).toEqual([{ sessionId: 'ffff6666', attempt: 1 }]); + }); + + it('never revives from a stale "alive" answer after the pane came back: a later clean exit re-probes', () => { + registerRemote('abab7777'); + const cache = (manager as unknown as { remoteAliveCache: Map }).remoteAliveCache; + cache.set('abab7777', true); // learned during a transport drop + vi.spyOn(manager, 'isPaneDead').mockReturnValue(false); // reattach succeeded + manager.runRemoteReconnectTick(0, true); + expect(cache.has('abab7777')).toBe(false); + }); + it('clears per-session reconnect/guard state when the session is removed', () => { registerRemote('eeee5555'); manager.guardRemoteReconnect('eeee5555');