mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-02 05:29:42 +02:00
Merge pull request #362 from timkjr/feat/omp-remote-continuation
fix(omp,remote): thread remote-omp resume/continue through respawn and reattach
This commit is contained in:
Ark0N
+67
-23
@@ -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<SessionMode, 'shell' \| 'claude' \| 'opencode' \| 'codex' \| 'gemini' \| 'antigravity' \| 'pi' \| 'grok'>` — 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<SessionMode, 'shell' \| 'claude' \| 'opencode' \| 'codex' \| 'gemini' \| 'antigravity' \| 'pi' \| 'grok'>` — 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 <cli>`** replaces the pane shell with the agent, so the pane PID *is*
|
||||
- **`exec <cli>`** 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 `<host>` needs tmux installed for
|
||||
durable remote sessions"*
|
||||
- stderr present → *"could not verify tmux on remote host `<host>`: `<stderr>`"*
|
||||
- empty stdout / non-zero exit → _"remote host `<host>` needs tmux installed for
|
||||
durable remote sessions"_
|
||||
- stderr present → _"could not verify tmux on remote host `<host>`: `<stderr>`"_
|
||||
(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 <remoteSessionName>'`. 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 <id> ||
|
||||
claude --resume <id>` (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 <id>`
|
||||
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-<id8>` 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 }`
|
||||
|
||||
@@ -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) {
|
||||
|
||||
+69
-8
@@ -827,8 +827,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
|
||||
@@ -837,11 +840,66 @@ 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, 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 <sessionId>; 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);
|
||||
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
|
||||
// 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);
|
||||
}
|
||||
const remoteName = remoteTmuxSessionName(sessionId);
|
||||
|
||||
// Innermost: the command tmux runs in the new pane. Run via `/bin/sh -c` by
|
||||
@@ -1309,6 +1367,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) {
|
||||
@@ -1882,7 +1943,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)
|
||||
@@ -2133,7 +2194,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 {
|
||||
|
||||
@@ -22,6 +22,23 @@ import { join, sep } from 'node:path';
|
||||
/** A real OMP session file is `<ISO-ish-timestamp>_<uuid>.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;
|
||||
}
|
||||
|
||||
@@ -93,6 +93,11 @@ const ALLOWED_BRANCHES: Record<string, string> = {
|
||||
"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'":
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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', () => {
|
||||
@@ -41,6 +45,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', () => {
|
||||
@@ -69,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}
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -172,6 +172,43 @@ 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 <id>` to create, with a `|| --resume <id>` 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');
|
||||
});
|
||||
|
||||
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 <rid> || --session-id <sessionId>`. 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', () => {
|
||||
|
||||
Reference in New Issue
Block a user