mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-08 16:39:42 +02:00
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 <id>` to create, with the `|| --resume <id>` 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.
This commit is contained in:
+21
-5
@@ -770,11 +770,27 @@ export function buildRemoteLaunchCommand(options: {
|
|||||||
// `defaultRemoteCommandForMode`: `claude` lives under a per-user PATH entry that
|
// `defaultRemoteCommandForMode`: `claude` lives under a per-user PATH entry that
|
||||||
// only an interactive login shell resolves (see that function's comment).
|
// only an interactive login shell resolves (see that function's comment).
|
||||||
const override = remote.commands?.[mode];
|
const override = remote.commands?.[mode];
|
||||||
const modeCommand = override
|
let modeCommand: string;
|
||||||
? override
|
if (override) {
|
||||||
: mode === 'claude'
|
modeCommand = override;
|
||||||
? remoteLoginShellCommand(`claude${buildClaudePermissionFlags(claudeMode, allowedTools)}`)
|
} else if (mode === 'claude') {
|
||||||
: defaultRemoteCommandForMode(mode);
|
// Deterministic conversation pinning for SSH-remote claude (mirrors the
|
||||||
|
// docker-claude shape in claudeDockerPaneCommand): 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);
|
||||||
|
modeCommand = remoteLoginShellCommand(
|
||||||
|
`claude${permFlags} --session-id ${sessionId} || claude${permFlags} --resume ${sessionId}`
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
modeCommand = defaultRemoteCommandForMode(mode);
|
||||||
|
}
|
||||||
const remoteName = remoteTmuxSessionName(sessionId);
|
const remoteName = remoteTmuxSessionName(sessionId);
|
||||||
|
|
||||||
// Innermost: the command tmux runs in the new pane. Run via `/bin/sh -c` by
|
// Innermost: the command tmux runs in the new pane. Run via `/bin/sh -c` by
|
||||||
|
|||||||
@@ -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. */
|
/** 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$/;
|
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
|
* Mirrors `omp`'s own directory mangling. Confirmed empirically against real
|
||||||
* `~/.omp/agent/sessions/` directory names (2026-08-27): unlike Claude Code's
|
* `~/.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
|
// omp's actual behavior on a symlinked-home setup; guessing wrong here would
|
||||||
// trade one silent mismatch for a different one.
|
// trade one silent mismatch for a different one.
|
||||||
const home = homedir();
|
const home = homedir();
|
||||||
|
const normalized = stripTrailingSlash(workingDir);
|
||||||
const relative =
|
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, '-');
|
return relative.replace(/\//g, '-');
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -183,7 +201,9 @@ export function resolveAndClaimOmpSessionId(workingDir: string): string | null {
|
|||||||
}
|
}
|
||||||
if (mtimeMs <= newestMtime) continue;
|
if (mtimeMs <= newestMtime) continue;
|
||||||
const header = readOmpSessionHeader(filePath);
|
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;
|
newestMtime = mtimeMs;
|
||||||
newestId = header.id;
|
newestId = header.id;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -41,6 +41,15 @@ describe('mangleOmpWorkingDir', () => {
|
|||||||
const sibling = `${homedir()}-other/dev/foo`;
|
const sibling = `${homedir()}-other/dev/foo`;
|
||||||
expect(mangleOmpWorkingDir(sibling)).toBe(sibling.replace(/\//g, '-'));
|
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', () => {
|
describe('findLatestOmpSessionId', () => {
|
||||||
|
|||||||
@@ -172,6 +172,21 @@ describe('TmuxManager (unit)', () => {
|
|||||||
expect(command).toContain('exec "${SHELL:-/bin/sh}" -i -l -c');
|
expect(command).toContain('exec "${SHELL:-/bin/sh}" -i -l -c');
|
||||||
expect(command).toContain('claude --dangerously-skip-permissions');
|
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');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('remote kill command builder', () => {
|
describe('remote kill command builder', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user