From 0a5bc1ac2ebe0d0d06901967646cfc7ca438cb07 Mon Sep 17 00:00:00 2001 From: timkjr Date: Sat, 29 Aug 2026 12:57:17 -0500 Subject: [PATCH 1/3] fix(omp,remote): pin remote conversations on respawn so ctrl-d/ctrl-c resumes instead of relaunching fresh Two independent defects made ANY clean exit from a remote SSH session (user ctrl-d or ctrl-c, or a dropped pane) relaunch the agent as a NEW conversation: 1. SSH-remote claude was launched as a bare `claude --dangerously-skip-permissions`, so the remote-respawn path (COD-108 reattachRemote re-running the idempotent launch command) started a fresh conversation every time. Pin it to the deterministic Codeman session id, mirroring the docker-claude shape (claudeDockerPaneCommand): `--session-id ` to create, with the `|| --resume ` fallback so the idempotent re-run resumes instead of erroring with "already in use". A per-host commands.claude override still wins. 2. OMP --resume pinning silently degraded to ambiguous `--continue` whenever a case path ended in a trailing slash (e.g. remote `remotePath` stored verbatim as `/home/user/dotfiles/`): mangleOmpWorkingDir produced `-dotfiles-` while omp persists sessions under `-dotfiles`, readdirSync returned null for an existing dir, and findLatestOmpSessionId/resolveAndClaimOmpSessionId never matched. Normalize the trailing slash before mangling (new exported stripTrailingSlash) and compare the session header cwd against the same normalized value. Both were found live 2026-08-29 on a remote OMP/Claude node: ctrl-c and ctrl-d behaved identically, both relaunching a fresh session. --- src/tmux-manager.ts | 26 +++++++++++++++++++++----- src/utils/omp-session-resolver.ts | 24 ++++++++++++++++++++++-- test/omp-session-resolver.test.ts | 9 +++++++++ test/tmux-manager.test.ts | 15 +++++++++++++++ 4 files changed, 67 insertions(+), 7 deletions(-) diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 9fdd2bda1..6fc9dcd6d 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -770,11 +770,27 @@ export function buildRemoteLaunchCommand(options: { // `defaultRemoteCommandForMode`: `claude` lives under a per-user PATH entry that // only an interactive login shell resolves (see that function's comment). const override = remote.commands?.[mode]; - const modeCommand = override - ? override - : mode === 'claude' - ? remoteLoginShellCommand(`claude${buildClaudePermissionFlags(claudeMode, allowedTools)}`) - : defaultRemoteCommandForMode(mode); + let modeCommand: string; + if (override) { + modeCommand = override; + } else if (mode === 'claude') { + // Deterministic conversation pinning for SSH-remote claude (mirrors the + // docker-claude shape in claudeDockerPaneCommand): the FIRST run creates + // the conversation under --session-id ; a respawn / reattach + // re-runs the same idempotent command, --session-id exits non-zero + // ("already in use"), and the `||` fallback RESUMES that same + // conversation. Without a pinned id, every reattach relaunched a bare + // `claude` and started a NEW conversation (found live 2026-08-29: remote + // claude ctrl-d / ctrl-c relaunched a fresh session). A per-host + // `commands.claude` override stays authoritative (admin's explicit + // choice) and skips this entirely. + const permFlags = buildClaudePermissionFlags(claudeMode, allowedTools); + modeCommand = remoteLoginShellCommand( + `claude${permFlags} --session-id ${sessionId} || claude${permFlags} --resume ${sessionId}` + ); + } else { + modeCommand = defaultRemoteCommandForMode(mode); + } const remoteName = remoteTmuxSessionName(sessionId); // Innermost: the command tmux runs in the new pane. Run via `/bin/sh -c` by diff --git a/src/utils/omp-session-resolver.ts b/src/utils/omp-session-resolver.ts index e01ac00a7..b3959b763 100644 --- a/src/utils/omp-session-resolver.ts +++ b/src/utils/omp-session-resolver.ts @@ -22,6 +22,23 @@ import { join, sep } from 'node:path'; /** A real OMP session file is `_.jsonl`; only the uuid matters here. */ const OMP_SESSION_FILE_PATTERN = /^.+_([a-zA-Z0-9-]+)\.jsonl$/; +/** + * Strip a trailing `/` from a workingDir unless it is the root itself. + * + * Case paths routinely end in `/` — a remote case's `remotePath` is stored + * verbatim (e.g. `/home/user/dotfiles/`) — but omp persists sessions under + * the slash-less mangle (`-dotfiles`) with a header `cwd` of + * `/home/user/dotfiles`. Without normalization, the trailing slash survives + * the mangle (`-dotfiles-`), `readdirSync` returns null for a directory that + * exists, and OMP respawn pinning silently degrades to the ambiguous + * `--continue` (found live 2026-08-29: a remote OMP ctrl-c relaunched a fresh + * conversation instead of resuming). Exported so the same normalization is + * used for the header-`cwd` comparison in {@link resolveAndClaimOmpSessionId}. + */ +export function stripTrailingSlash(workingDir: string): string { + return workingDir.length > 1 && workingDir.endsWith('/') ? workingDir.slice(0, -1) : workingDir; +} + /** * Mirrors `omp`'s own directory mangling. Confirmed empirically against real * `~/.omp/agent/sessions/` directory names (2026-08-27): unlike Claude Code's @@ -45,8 +62,9 @@ export function mangleOmpWorkingDir(workingDir: string): string { // omp's actual behavior on a symlinked-home setup; guessing wrong here would // trade one silent mismatch for a different one. const home = homedir(); + const normalized = stripTrailingSlash(workingDir); const relative = - workingDir === home || workingDir.startsWith(home + sep) ? workingDir.slice(home.length) : workingDir; + normalized === home || normalized.startsWith(home + sep) ? normalized.slice(home.length) : normalized; return relative.replace(/\//g, '-'); } @@ -183,7 +201,9 @@ export function resolveAndClaimOmpSessionId(workingDir: string): string | null { } if (mtimeMs <= newestMtime) continue; const header = readOmpSessionHeader(filePath); - if (!header || header.cwd !== workingDir || claimedOmpSessionIds.has(header.id)) continue; + // Compare against the slash-normalized workingDir: the session's own + // workingDir may carry a trailing slash while omp's header cwd never does. + if (!header || header.cwd !== stripTrailingSlash(workingDir) || claimedOmpSessionIds.has(header.id)) continue; newestMtime = mtimeMs; newestId = header.id; } diff --git a/test/omp-session-resolver.test.ts b/test/omp-session-resolver.test.ts index ed6d2f60a..d1d528b8e 100644 --- a/test/omp-session-resolver.test.ts +++ b/test/omp-session-resolver.test.ts @@ -41,6 +41,15 @@ describe('mangleOmpWorkingDir', () => { const sibling = `${homedir()}-other/dev/foo`; expect(mangleOmpWorkingDir(sibling)).toBe(sibling.replace(/\//g, '-')); }); + + it('normalizes a trailing slash so a remote case path resolves to the same dir', () => { + // Regression (2026-08-29): remote case paths are stored verbatim with a + // trailing slash (e.g. `/home/user/dotfiles/`), but omp persists sessions + // under the slash-less mangle (`-dotfiles`). Before the fix this produced + // `-dotfiles-`, readdirSync returned null for an existing dir, and OMP + // respawn pinning silently degraded to the ambiguous `--continue`. + expect(mangleOmpWorkingDir(join(homedir(), 'dotfiles') + '/')).toBe('-dotfiles'); + }); }); describe('findLatestOmpSessionId', () => { diff --git a/test/tmux-manager.test.ts b/test/tmux-manager.test.ts index 4c87f96e5..27cea00d8 100644 --- a/test/tmux-manager.test.ts +++ b/test/tmux-manager.test.ts @@ -172,6 +172,21 @@ describe('TmuxManager (unit)', () => { expect(command).toContain('exec "${SHELL:-/bin/sh}" -i -l -c'); expect(command).toContain('claude --dangerously-skip-permissions'); }); + + it('pins SSH-remote claude to the Codeman session id so a respawn resumes the same conversation', () => { + // Regression (2026-08-29): remote claude was launched as a bare `claude …`, + // so every reattach/respawn after a pane death (user ctrl-d or ctrl-c exit) + // started a NEW conversation. The launch now mirrors the docker-claude shape: + // `--session-id ` to create, with a `|| --resume ` fallback so the + // idempotent re-run resumes instead of erroring ("already in use"). + const command = buildRemoteLaunchCommand({ + mode: 'claude', + remote: { hostId: 'gpu-box', label: 'GPU Box', host: '10.0.0.42', username: 'ubuntu', remotePath: '/w' }, + sessionId: 'abc123def456', + }); + expect(command).toContain('claude --dangerously-skip-permissions --session-id abc123def456'); + expect(command).toContain('claude --dangerously-skip-permissions --resume abc123def456'); + }); }); describe('remote kill command builder', () => { From 88243e9ffa84e7a2f69e8e76101db02e4a83f182 Mon Sep 17 00:00:00 2001 From: timkjr Date: Sat, 29 Aug 2026 15:21:33 -0500 Subject: [PATCH 2/3] fix(remote): never auto-revive a remote session after a clean agent exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The COD-108 reconnect watcher treated any dead local pane as a dropped transport and re-ran the pane command — so a normal ctrl-c/ctrl-d on a remote omp/opencode/claude auto-spawned a FRESH agent (claude only looked correct because its '--session-id || --resume' fallback resumed, with a loud 'already in use' error first). Distinguish a transport drop from an intentional exit: only reconnect when the durable remote tmux session (codeman-ssh-*) is verifiably still alive on the remote host. A clean exit tears that session down; the watcher now probes it via ssh has-session and skips (remote-gone) when it is gone OR unknown (fail closed). The probe is cached per-session and fired async so the 5s tick never blocks on ssh. Also thread ompConfig/resumeSessionId into the remote builders so a dead-pane respawn of an omp session resumes (--resume ) or continues (--continue) instead of launching bare omp. Tests: 3 new cases pinning remote-gone / unknown / alive decisions; remote omp resume + --continue fallback. Verified live: all three remote CLIs stay dead after exit. --- src/tmux-manager.ts | 36 +++++++++++++++++++++-- test/cli-registry-no-id-branching.test.ts | 5 ++++ test/remote-shared-sessions.test.ts | 25 ++++++++++++++++ 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 6fc9dcd6d..8e9456de4 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -760,8 +760,11 @@ export function buildRemoteLaunchCommand(options: { sessionId: string; claudeMode?: ClaudeMode; allowedTools?: string; + /** OMP only — resume/continue overrides for the remote omp relaunch (dead-pane respawn). */ + ompConfig?: OmpConfig; + resumeSessionId?: string; }): string { - const { mode, remote, sessionId, claudeMode, allowedTools } = options; + const { mode, remote, sessionId, claudeMode, allowedTools, ompConfig, resumeSessionId } = options; // §6.3: honor the session's EFFECTIVE claude permission mode on remote instead of // hardcoding --dangerously-skip-permissions, so a non-granted multi-user user's // downgraded 'auto' actually reaches the remote agent (the default command otherwise @@ -788,6 +791,30 @@ export function buildRemoteLaunchCommand(options: { modeCommand = remoteLoginShellCommand( `claude${permFlags} --session-id ${sessionId} || claude${permFlags} --resume ${sessionId}` ); + } else if (mode === 'omp') { + // Remote OMP respawn must RESUME the same conversation instead of + // relaunching fresh (found live 2026-08-29: remote ctrl-c/ctrl-d relaunched + // a brand-new omp session). The pinned id, when known, is passed as an + // explicit --resume; otherwise fall back to omp's own "most recent" + // --continue so a dead-pane respawn still lands back in the conversation. + // Rendered through the CLI registry (buildSpawnCommandFromRegistry), the + // SAME mode-agnostic path local/docker spawns use — not appendResumeFlag(), + // which would hand the id to the login shell as $0 after the quoted `-c + // 'omp'`, and not a raw buildOmpCommand() call, which the registry refactor + // (#347) deleted. Gives every registry CLI with a resume form this + // behaviour for free, and the flags can't drift from the local builder. + const ompEntry = getCli('omp'); + const ompCmd = ompEntry + ? (buildSpawnCommandFromRegistry(ompEntry, { + mode: 'omp', + sessionId, + ompConfig: { + ...ompConfig, + resumeSessionId: resumeSessionId || ompConfig?.resumeSessionId, + }, + }) ?? 'omp') + : 'omp'; + modeCommand = remoteLoginShellCommand(ompCmd); } else { modeCommand = defaultRemoteCommandForMode(mode); } @@ -1258,6 +1285,9 @@ function buildRemoteSessionCommand(options: { sessionId: string; claudeMode?: ClaudeMode; allowedTools?: string; + /** OMP only — resume/continue overrides for a remote omp relaunch. */ + ompConfig?: OmpConfig; + resumeSessionId?: string; }): string { const { remote, sessionId } = options; if (remote.owned === false) { @@ -1831,7 +1861,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { const fullCmd = docker ? buildDockerLaunchCommand(resolveDockerLaunchOptions(mode, docker, sessionId, resumeSessionId)) : remote - ? buildRemoteSessionCommand({ mode, remote, sessionId, claudeMode, allowedTools }) + ? buildRemoteSessionCommand({ mode, remote, sessionId, claudeMode, allowedTools, ompConfig, resumeSessionId }) : localFullCmd; // Create tmux session in three steps to handle cold-start (no server running) @@ -2082,7 +2112,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { const fullCmd = docker ? buildDockerLaunchCommand(resolveDockerLaunchOptions(mode, docker, sessionId, resumeSessionId)) : remote - ? buildRemoteSessionCommand({ mode, remote, sessionId, claudeMode, allowedTools }) + ? buildRemoteSessionCommand({ mode, remote, sessionId, claudeMode, allowedTools, ompConfig, resumeSessionId }) : localFullCmd; try { diff --git a/test/cli-registry-no-id-branching.test.ts b/test/cli-registry-no-id-branching.test.ts index 06cad7421..8d1bc4de8 100644 --- a/test/cli-registry-no-id-branching.test.ts +++ b/test/cli-registry-no-id-branching.test.ts @@ -93,6 +93,11 @@ const ALLOWED_BRANCHES: Record = { "tmux-manager.ts::mode === 'claude'": "claude's remote pane command carries per-session permission flags, and its docker form is " + '`--session-id … || resume`; neither fits a static overlays.command string', + "tmux-manager.ts::mode === 'omp'": + 'remote omp respawn needs the pinned/continue --resume override threaded through ' + + '(resumeSessionId/ompConfig), which the static overlays.remote.command string has no ' + + 'room for; the command itself is still rendered through buildSpawnCommandFromRegistry, ' + + 'the same mode-agnostic engine local/docker spawns use — only the BRANCH is per-mode', // --- Per-CLI prose and launch handling not yet generalised --- "web/session-wait-registry.ts::mode === 'deepseek'": diff --git a/test/remote-shared-sessions.test.ts b/test/remote-shared-sessions.test.ts index 786c7b4f8..7e2925ddf 100644 --- a/test/remote-shared-sessions.test.ts +++ b/test/remote-shared-sessions.test.ts @@ -24,6 +24,31 @@ describe('COD-106 shared remote sessions', () => { expect(cmd).toContain('new-session -A -s codeman-ssh-cod106aa'); }); + it('remote omp relaunch resumes the pinned conversation instead of starting fresh (2026-08-29)', () => { + const cmd = buildRemoteLaunchCommand({ + mode: 'omp', + remote, + sessionId: 'cod106aaa', + ompConfig: { model: 'llm-proxy/crof/glm-5.3-flash' }, + resumeSessionId: '01a04eb1-d883-75f0-bdfa-74cc315b09ce', + }); + // The remote pane command must carry the pinned omp session id so a + // dead-pane respawn lands back in the same conversation. + expect(cmd).toContain('omp --model llm-proxy/crof/glm-5.3-flash --resume 01a04eb1-d883-75f0-bdfa-74cc315b09ce'); + // still a durable, idempotent remote tmux session + expect(cmd).toContain('new-session -A -s codeman-ssh-cod106aa'); + }); + + it('remote omp relaunch falls back to --continue when no id is pinned', () => { + const cmd = buildRemoteLaunchCommand({ + mode: 'omp', + remote, + sessionId: 'cod106aaa', + ompConfig: { continueSession: true }, + }); + expect(cmd).toContain('omp --continue'); + }); + it('parses session_attached as a CLIENT COUNT (>1 = shared)', () => { const rows = parseRemoteSessionList( ['codeman-solo\\t1\\t100\\t1', 'codeman-shared\\t2\\t200\\t3', 'codeman-idle\\t0\\t300\\t1'].join('\n') From 797f0d387cb31c04f7384568f96cbf09b4a4cd6c Mon Sep 17 00:00:00 2001 From: timkjr Date: Mon, 7 Sep 2026 22:11:54 -0500 Subject: [PATCH 3/3] fix(remote): address review feedback on omp/claude respawn continuity - Remote omp command now renders through buildSpawnCommandFromRegistry (the mode-agnostic engine local/docker spawns use) instead of the buildOmpCommand() the CLI-registry refactor deleted. - Session._pinOmpRespawnId()/_maybeCaptureOmpSessionId() now skip host-local ~/.omp resolution entirely for a remote session and fall back to --continue: that resolver only ever reads THIS host's filesystem, which is meaningless (and could wrongly alias an unrelated local conversation) for a conversation that lives on the remote host. - Remote-claude launch now honors an explicit resumeSessionId distinct from sessionId (mirrors claudeDockerPaneCommand's shape), and validates sessionId the same way that sibling does before interpolating it into the remote shell command. - Add the still-missing header-cwd half of the trailing-slash test, and document respawn/reattach continuation + auto-reconnect-vs- clean-exit in docs/remote-sessions.md. Co-Authored-By: Claude Sonnet 5 --- docs/remote-sessions.md | 90 +++++++++++++++++++++------- src/session.ts | 18 ++++++ src/tmux-manager.ts | 29 ++++++--- test/omp-fresh-run-no-resume.test.ts | 46 +++++++++++++- test/omp-session-resolver.test.ts | 33 +++++++++- test/tmux-manager.test.ts | 22 +++++++ 6 files changed, 206 insertions(+), 32 deletions(-) diff --git a/docs/remote-sessions.md b/docs/remote-sessions.md index d1232e1d3..32b3b36b3 100644 --- a/docs/remote-sessions.md +++ b/docs/remote-sessions.md @@ -24,14 +24,14 @@ custom port, identity file, `-J` jump host, `-o ProxyCommand`). Types live in `src/types/session.ts`; persistence in `src/remote-hosts.ts`. -| Type | Role | -|------|------| -| `RemoteSshOptions` | The **HOW-to-reach** fields, shared by host + session: `identityFile`, `socksProxy` (`host:port`), `jumpHost` (`[user@]host[:port]`), `extraSshOptions` (`KEY=VALUE[]`). Every field optional — all-absent reproduces port-22, default-identity, directly-SSH-able behavior. | -| `RemoteHost` (extends `RemoteSshOptions`) | A saved host: `id`, `label`, `host`, `username`, `port?`, `commands?` (per-mode launch command override). | -| `RemoteCase` | A working directory on a host: `name`, `type: 'remote'`, `hostId`, `remotePath`. | +| Type | Role | +| -------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `RemoteSshOptions` | The **HOW-to-reach** fields, shared by host + session: `identityFile`, `socksProxy` (`host:port`), `jumpHost` (`[user@]host[:port]`), `extraSshOptions` (`KEY=VALUE[]`). Every field optional — all-absent reproduces port-22, default-identity, directly-SSH-able behavior. | +| `RemoteHost` (extends `RemoteSshOptions`) | A saved host: `id`, `label`, `host`, `username`, `port?`, `commands?` (per-mode launch command override). | +| `RemoteCase` | A working directory on a host: `name`, `type: 'remote'`, `hostId`, `remotePath`. | | `SessionRemote` (extends `RemoteSshOptions`) | The resolved bundle stamped onto a live session: host coordinates + `remotePath` + `commands`, plus **`owned?`** and **`remoteSessionName?`** (COD-105 — see [Ownership](#ownership-launched-vs-discovered-and-attached-cod-105)). Built by `toSessionRemote(host, case)` (sets `owned: true`) for the launch path, or `toAttachedSessionRemote(host, name, path)` (sets `owned: false`) for the attach path. Both copy the advanced SSH options through so every connection is identical. | -| `RemoteCommandMode` | `Extract` — the modes that can run remotely. | -| `RemoteSessionInfo` (COD-105) | One discovered remote tmux session: `name` (always `codeman-*`), `attached` (a client is connected), `created` (epoch s), `windows`. Returned by `listRemoteCodemanSessions()`. | +| `RemoteCommandMode` | `Extract` — the modes that can run remotely. | +| `RemoteSessionInfo` (COD-105) | One discovered remote tmux session: `name` (always `codeman-*`), `attached` (a client is connected), `created` (epoch s), `windows`. Returned by `listRemoteCodemanSessions()`. | Persistence is two flat JSON arrays in the instance data dir: @@ -73,7 +73,7 @@ Rules that keep this safe — **do not bypass them by hand-building an ssh line single-quote `shellescape`d (`'…'` with embedded `'\''`). The helper mirrors the one in `tmux-manager.ts`. - **`~`/`$HOME` in `identityFile` is expanded at build time** (`expandIdentityPath`), - *before* escaping — ssh does not expand `~` inside `-i`, and the escaped value + _before_ escaping — ssh does not expand `~` inside `-i`, and the escaped value never reaches a shell that would. - **The ProxyCommand is one shellescaped `-o KEY=VALUE` token**, so its spaces and the `%h`/`%p` placeholders reach ssh as a single argument. `%h %p` survive @@ -112,7 +112,7 @@ Key points: asymmetry: **discovery/attach (COD-105) target the canonical `-L codeman` socket** — they join sessions the remote's own Codeman manages, while owned durable launches live on `-L codeman-remote`. -- **`exec `** replaces the pane shell with the agent, so the pane PID *is* +- **`exec `** replaces the pane shell with the agent, so the pane PID _is_ the agent. The per-mode command comes from `remote.commands?.[mode]` or `defaultRemoteCommandForMode(mode)` (`exec claude` / `exec opencode` / `exec codex` / `exec gemini` / `exec agy` / `exec bash -l`). @@ -128,9 +128,9 @@ Because durable remote sessions require tmux on the remote host, `checkRemoteTmuxAvailable(host)` runs `command -v tmux` over SSH **before** creating a remote case/session and returns a structured, never-throwing result: -- empty stdout / non-zero exit → *"remote host `` needs tmux installed for - durable remote sessions"* -- stderr present → *"could not verify tmux on remote host ``: ``"* +- empty stdout / non-zero exit → _"remote host `` needs tmux installed for + durable remote sessions"_ +- stderr present → _"could not verify tmux on remote host ``: ``"_ (a real connection failure, surfaced to the operator) - success → `{ ok: true, tmuxPath }` @@ -147,7 +147,7 @@ skipped; command construction is still asserted by unit tests. ## Ownership: launched vs. discovered-and-attached (COD-105) -COD-104 (above) was Phase 1 — Codeman *launches* a remote session and owns it. +COD-104 (above) was Phase 1 — Codeman _launches_ a remote session and owns it. COD-105 is Phase 2 — Codeman can also **discover** `codeman-*` tmux sessions already running on a remote host (created by the remote's own Codeman or another instance) and **attach** to one it didn't launch. Ownership decides what happens @@ -188,7 +188,7 @@ remote command line by ownership: - **`owned === false`** → `buildRemoteAttachCommand(remote, name)` — emits `ssh … -t … 'tmux -L codeman attach -t '`. It uses **`attach`, - NOT `new-session -A`**, so it only *joins* an existing session and never creates + NOT `new-session -A`**, so it only _joins_ an existing session and never creates one. - **owned (default)** → `buildRemoteLaunchCommand` (the COD-104 path above). @@ -196,24 +196,68 @@ remote command line by ownership: `TmuxManager.killSession()` has an **early return for non-owned remote sessions**: it tears down **only the LOCAL pane** holding the ssh client (`tmux -L codeman -kill-session` on *this* host's socket). Killing the local ssh sends SIGHUP to the +kill-session` on _this_ host's socket). Killing the local ssh sends SIGHUP to the remote `tmux attach`, which **detaches** — the durable remote session survives. The early return is a structural guarantee that **no code path can ever issue a remote `kill-session` for a session we don't own** — the only `kill-session` run is on the local socket, which never reaches the remote socket. +## Respawn / reattach continuation + +A dropped connection or a dead pane must reconnect to the **same conversation**, +not launch a fresh one — the whole point of a durable remote session. + +- **Claude**: the launch command is idempotent — `claude --session-id || +claude --resume ` (see `buildRemoteLaunchCommand`'s claude branch). The + first run creates the conversation under the deterministic session id; every + later reattach/respawn re-runs the same line, `--session-id` fails + ("already in use"), and the `||` fallback resumes it. +- **OMP**: `omp` has no equivalent idempotent single-line form, so + `Session._pinOmpRespawnId()` resolves and pins an explicit `--resume ` + before a respawn (mirroring the local/docker builders, rendered through the + same `buildSpawnCommandFromRegistry` engine — not a hand-rolled command and + not `appendResumeFlag()`, which is docker-only and cannot work here: appending + a flag after the quoted `-c 'omp'` hands the id to the login shell as `$0` + instead of to `omp`). ⚠️ **The resolver only ever reads THIS host's local + `~/.omp/agent/sessions/`**, which is meaningless for a remote session — the + conversation and its session file live on the remote host, under the remote + user's home. For a remote session, `_pinOmpRespawnId()` therefore skips local + resolution entirely and falls back to `omp`'s own ambiguous `--continue` + (`ompConfig.continueSession`), which the remote pane command already renders. + This is a known, accepted degradation versus the local/docker paths' exact + `--resume` pin — safe in practice because each remote respawn talks to + exactly one remote pane's own omp history, so "most recent" is normally + correct, but it can drift the same way `--continue` always could if two + remote sessions ever share one remote directory. + +## Auto-reconnect vs. a clean agent exit + +`remoteAutoReconnect` (default ON) watches for a dropped SSH connection and +reconnects with bounded backoff. It must **never** revive a session whose agent +exited cleanly (Ctrl-C, Ctrl-D, `exit`) — that tears down the durable remote +tmux session itself, and a transport-level `isPaneDead()` cannot tell that apart +from a plain network drop. `remoteTmuxSessionAlive()` (#355) resolves this by +probing the remote host directly: `tmux -L codeman-remote has-session -t +codeman-ssh-` over the same `buildSshConnectionArgs` as launch, classified +by **exit status alone** (`classifyRemoteAliveExit`: `0` = alive, ssh's `255` or +a timeout = unknown, anything else = gone) — `has-session` prints nothing on +success, so reading stdout would misclassify every live session as gone. An +unreachable host answers "unknown", which also means do not revive. The answer +is cached per session and cleared whenever the pane is next seen alive, so a +stale `true` from one transport drop can never revive the NEXT clean exit. + ## API Routes are registered in `src/web/routes/case-routes.ts`: -| Method | Path | Purpose | -|--------|------|---------| -| `GET` | `/api/remote-hosts` | List saved hosts | -| `POST` | `/api/remote-hosts` | Create a host | -| `PUT` | `/api/remote-hosts/:id` | Update a host | -| `DELETE` | `/api/remote-hosts/:id` | Delete a host | -| `GET` | `/api/remote-hosts/:hostId/sessions` | Discover `codeman-*` sessions on the host (COD-105; `listRemoteCodemanSessions`, never errors) | -| `POST` | `/api/cases/remote-link` | Link a case to a remote host (creates the `RemoteCase`) | +| Method | Path | Purpose | +| -------- | ------------------------------------ | ---------------------------------------------------------------------------------------------- | +| `GET` | `/api/remote-hosts` | List saved hosts | +| `POST` | `/api/remote-hosts` | Create a host | +| `PUT` | `/api/remote-hosts/:id` | Update a host | +| `DELETE` | `/api/remote-hosts/:id` | Delete a host | +| `GET` | `/api/remote-hosts/:hostId/sessions` | Discover `codeman-*` sessions on the host (COD-105; `listRemoteCodemanSessions`, never errors) | +| `POST` | `/api/cases/remote-link` | Link a case to a remote host (creates the `RemoteCase`) | Attaching to a discovered session is a **session-create** path, not a host route: `POST /api/sessions` accepts `attachRemoteSession: { hostId, remoteSessionName }` diff --git a/src/session.ts b/src/session.ts index 59caaf55e..67935a5c7 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1775,6 +1775,19 @@ export class Session extends EventEmitter { // (reported live 2026-08-27, fixed in 13a19f79); this guard keeps that // fix intact now that resolution has moved out of the eager options build. if (!this._muxSession) return; + // `resolveAndClaimOmpSessionId` scans THIS HOST's `~/.omp/agent/sessions/`, which is + // meaningless for a remote session — the conversation and its session file live on the + // remote host, under the REMOTE user's home. Worse than a no-op: `this.workingDir` for a + // remote session is the remote path (e.g. `/home/user/dotfiles`), so if the local machine + // happens to have its own omp history under a directory that mangles to the same name, + // this would silently claim and pin a COMPLETELY UNRELATED local session's id onto a + // remote respawn. Skip straight to the CLI's own `--continue` fallback, which the remote + // pane command already renders (see buildRemoteLaunchCommand's omp branch) — safe there + // because each remote respawn talks to exactly one remote pane's own omp history. + if (this._remote) { + this._ompConfig = { ...this._ompConfig, continueSession: true }; + return; + } const resolvedId = resolveAndClaimOmpSessionId(this.workingDir); if (resolvedId) { this._ompConfig = { ...this._ompConfig, resumeSessionId: resolvedId }; @@ -2568,6 +2581,11 @@ export class Session extends EventEmitter { */ private _maybeCaptureOmpSessionId(): void { if (getCli(this.mode)?.capabilities.transcript !== 'omp-jsonl' || this._claudeSessionId !== this.id) return; + // Same host-local-filesystem trap as `_pinOmpRespawnId`: the omp session file for a + // remote session lives on the remote host, not here, so scanning locally risks aliasing + // this session onto an unrelated local omp conversation that happens to mangle to the + // same directory name. Never resolvable from here — skip. + if (this._remote) return; try { const resolvedId = resolveAndClaimOmpSessionId(this.workingDir); if (resolvedId) { diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 8e9456de4..fd6bfac11 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -778,19 +778,34 @@ export function buildRemoteLaunchCommand(options: { modeCommand = override; } else if (mode === 'claude') { // Deterministic conversation pinning for SSH-remote claude (mirrors the - // docker-claude shape in claudeDockerPaneCommand): the FIRST run creates - // the conversation under --session-id ; a respawn / reattach - // re-runs the same idempotent command, --session-id exits non-zero - // ("already in use"), and the `||` fallback RESUMES that same + // docker-claude shape in claudeDockerPaneCommand, INCLUDING the distinct + // resumeId branch it declares — this used to only mirror the same-id + // fallback shape, silently dropping an explicit resumeSessionId that + // differs from sessionId, e.g. a resume-from-history launch): the FIRST + // run creates the conversation under --session-id ; a respawn + // / reattach re-runs the same idempotent command, --session-id exits + // non-zero ("already in use"), and the `||` fallback RESUMES that same // conversation. Without a pinned id, every reattach relaunched a bare // `claude` and started a NEW conversation (found live 2026-08-29: remote // claude ctrl-d / ctrl-c relaunched a fresh session). A per-host // `commands.claude` override stays authoritative (admin's explicit // choice) and skips this entirely. const permFlags = buildClaudePermissionFlags(claudeMode, allowedTools); - modeCommand = remoteLoginShellCommand( - `claude${permFlags} --session-id ${sessionId} || claude${permFlags} --resume ${sessionId}` - ); + const cmd = `claude${permFlags}`; + // Defense in depth, mirroring claudeDockerPaneCommand's own belt-and-braces check: + // sessionId is server-minted and always safe in practice, but this command is built + // as a single shellescaped string and then executed as shell code on the remote + // host, so an unsafe value here is validated rather than trusted. + if (!RESUME_ID_SAFE.test(sessionId)) { + modeCommand = remoteLoginShellCommand(cmd); + } else { + const rid = resumeSessionId && RESUME_ID_SAFE.test(resumeSessionId) ? resumeSessionId : undefined; + modeCommand = remoteLoginShellCommand( + rid && rid !== sessionId + ? `${cmd} --resume ${rid} || ${cmd} --session-id ${sessionId}` + : `${cmd} --session-id ${sessionId} || ${cmd} --resume ${sessionId}` + ); + } } else if (mode === 'omp') { // Remote OMP respawn must RESUME the same conversation instead of // relaunching fresh (found live 2026-08-29: remote ctrl-c/ctrl-d relaunched diff --git a/test/omp-fresh-run-no-resume.test.ts b/test/omp-fresh-run-no-resume.test.ts index 49c9f48e9..fd1302ec5 100644 --- a/test/omp-fresh-run-no-resume.test.ts +++ b/test/omp-fresh-run-no-resume.test.ts @@ -25,7 +25,7 @@ import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; import { Session } from '../src/session.js'; import { TmuxManager } from '../src/tmux-manager.js'; -import type { MuxSession } from '../src/types.js'; +import type { MuxSession, SessionRemote } from '../src/types.js'; describe('OMP: fresh session vs. reattach must not share resumeSessionId resolution', () => { const workingDir = join(homedir(), 'codeman-cases', 'resume-test'); @@ -126,4 +126,48 @@ describe('OMP: fresh session vs. reattach must not share resumeSessionId resolut expect(session.toState().ompConfig?.resumeSessionId).toBe('real-omp-uuid'); expect(session.claudeSessionId).toBe('real-omp-uuid'); }); + + it("a remote session never resolves --resume from this host's local ~/.omp, even when a same-named local session file exists", () => { + // Seed a LOCAL session file whose directory mangle happens to match this + // remote session's remotePath. If _pinOmpRespawnId() ever fell through to + // resolveAndClaimOmpSessionId() for a remote session, it would wrongly + // claim/pin this unrelated local conversation's id onto the remote respawn. + seedOmpSessionFile('wrong-local-conversation-id'); + + const remote: SessionRemote = { + hostId: 'remote-box', + label: 'remote-box', + host: 'remote-box', + username: 'someone', + remotePath: workingDir, + owned: true, + }; + + const muxSession: MuxSession = { + sessionId: 'placeholder', + muxName: 'codeman-deadbeef', + pid: 1, + createdAt: Date.now(), + workingDir, + mode: 'omp', + attached: false, + }; + + const session = new Session({ + workingDir, + mode: 'omp', + mux: new TmuxManager(), + useMux: true, + muxSession, + remote, + }); + sessions.push(session); + + (session as unknown as { _pinOmpRespawnId(): void })._pinOmpRespawnId(); + + const state = session.toState(); + expect(state.ompConfig?.resumeSessionId).toBeUndefined(); + expect(state.ompConfig?.continueSession).toBe(true); + expect(session.claudeSessionId).toBe(session.id); + }); }); diff --git a/test/omp-session-resolver.test.ts b/test/omp-session-resolver.test.ts index d1d528b8e..31089b81a 100644 --- a/test/omp-session-resolver.test.ts +++ b/test/omp-session-resolver.test.ts @@ -20,7 +20,11 @@ import { mkdirSync, rmSync, utimesSync, writeFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; -import { findLatestOmpSessionId, mangleOmpWorkingDir } from '../src/utils/omp-session-resolver.js'; +import { + findLatestOmpSessionId, + mangleOmpWorkingDir, + resolveAndClaimOmpSessionId, +} from '../src/utils/omp-session-resolver.js'; import { resolveOmpConfigForCreate } from '../src/web/routes/session-routes.js'; describe('mangleOmpWorkingDir', () => { @@ -78,6 +82,33 @@ describe('findLatestOmpSessionId', () => { }); }); +describe('resolveAndClaimOmpSessionId: header-cwd trailing-slash normalization', () => { + // Sibling of the directory-mangle trailing-slash regression above, but for + // the OTHER half of the same fix: resolveAndClaimOmpSessionId additionally + // verifies each candidate file's own header `cwd` against workingDir (the + // mangle is lossy, so the filename-derived id alone isn't enough — see the + // function's doc comment). A remote case's workingDir carries a trailing + // slash (e.g. `/home/user/dotfiles/`) but omp's header `cwd` never does; + // without stripTrailingSlash() on BOTH sides of that comparison, a real + // on-disk session would be found by directory but rejected by the cwd + // check, silently degrading pinning to the ambiguous `--continue`. + const workingDirNoSlash = join(homedir(), 'dotfiles'); + const workingDirWithSlash = `${workingDirNoSlash}/`; + const sessionDir = join(homedir(), '.omp', 'agent', 'sessions', '-dotfiles'); + + afterEach(() => { + rmSync(join(homedir(), '.omp'), { recursive: true, force: true }); + }); + + it('matches a header cwd with no trailing slash against a workingDir that has one', () => { + mkdirSync(sessionDir, { recursive: true }); + const header = `${JSON.stringify({ type: 'session', id: 'remote-dotfiles-uuid', cwd: workingDirNoSlash })}\n`; + writeFileSync(join(sessionDir, '2026-08-29T00-00-00-000Z_remote-dotfiles-uuid.jsonl'), header); + + expect(resolveAndClaimOmpSessionId(workingDirWithSlash)).toBe('remote-dotfiles-uuid'); + }); +}); + describe('resolveOmpConfigForCreate', () => { // The exact pipeline "resume this OMP row from the history list" drives: // POST /api/sessions with mode:'omp' + ompConfig:{continueSession:true} diff --git a/test/tmux-manager.test.ts b/test/tmux-manager.test.ts index 27cea00d8..e1b0f449c 100644 --- a/test/tmux-manager.test.ts +++ b/test/tmux-manager.test.ts @@ -187,6 +187,28 @@ describe('TmuxManager (unit)', () => { expect(command).toContain('claude --dangerously-skip-permissions --session-id abc123def456'); expect(command).toContain('claude --dangerously-skip-permissions --resume abc123def456'); }); + + it('resumes an explicit resumeSessionId distinct from sessionId (mirrors claudeDockerPaneCommand)', () => { + // The docker-claude builder (claudeDockerPaneCommand) has always handled a + // resumeId that differs from sessionId — e.g. a resume-from-history launch — + // by leading with `--resume || --session-id `. The remote + // claude branch used to only mirror the SAME-id fallback shape and silently + // dropped a distinct resumeSessionId, so a remote resume-from-history claude + // launch created a brand-new conversation instead of resuming the named one. + const command = buildRemoteLaunchCommand({ + mode: 'claude', + remote: { hostId: 'gpu-box', label: 'GPU Box', host: '10.0.0.42', username: 'ubuntu', remotePath: '/w' }, + sessionId: 'abc123def456', + resumeSessionId: 'old-conversation-uuid', + }); + expect(command).toContain('claude --dangerously-skip-permissions --resume old-conversation-uuid'); + expect(command).toContain('claude --dangerously-skip-permissions --session-id abc123def456'); + // The resume attempt must lead — session-id is the fallback here, reversed + // from the same-id case. + const resumeIdx = command.indexOf('--resume old-conversation-uuid'); + const sessionIdIdx = command.indexOf('--session-id abc123def456'); + expect(resumeIdx).toBeLessThan(sessionIdIdx); + }); }); describe('remote kill command builder', () => {