mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(remote): classify the has-session probe by exit status, and forget it once the pane is back
#355 made the remote auto-reconnect watcher revive a dead pane only when the durable remote tmux session is verifiably still alive, which is the right rule: a clean Ctrl-C / Ctrl-D / exit tears that session down and must never relaunch a fresh agent. Its probe, though, read `has-session`'s stdout and treated an empty string as "gone". `tmux has-session` prints NOTHING on success (measured on a scratch socket: exit 0, empty stdout, the failure message goes to stderr), so every live remote session classified as gone and transport-drop reconnects were silently disabled along with the clean-exit revives. The probe now goes by exit status through a pure, unit-tested mapping (`classifyRemoteAliveExit`): 0 is alive; ssh's own 255, a timeout (`killed`, no numeric code) and a spawn failure are unknown, which the watcher already treats as do-not-revive; any other status is the remote command's and means gone (tmux's 1 for a missing session, 127 when tmux is not installed there). Two smaller things in the same area: - The cached answer was never invalidated, so after one successful reattach a stale `true` would have revived the NEXT clean exit (the original bug back after the first transport drop), and a cached `false` from a clean exit would have left a manually restarted session with auto-reconnect permanently off. The tick now forgets the cache entry whenever the pane is seen alive. - The fire-and-forget probe has a 15s timeout against a 5s tick, so an unreachable host stacked up to three ssh processes per dead session. An in-flight set caps it at one. The probe command is pinned as a literal string, and the reattach-then-clean-exit sequence is driven through the watcher in the tests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qg6bcATm1pNNY4kQWGwzgu
This commit is contained in:
@@ -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-<id>`, 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-<id>`, 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)
|
||||
|
||||
|
||||
+31
-6
@@ -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.
|
||||
|
||||
+23
-1
@@ -1368,6 +1368,13 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
* transport drop).
|
||||
*/
|
||||
private remoteAliveCache: Map<string, boolean | undefined> = 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<string> = 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<void> {
|
||||
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 {
|
||||
|
||||
@@ -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<string, boolean | undefined> }).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<string, boolean | undefined> }).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');
|
||||
|
||||
Reference in New Issue
Block a user