mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 10:39:32 +01:00
fix(adopt): bring the docker and ssh transports up to master
Docker discovery now spawns dockerEngineArgv(), not the shell-quoted buildDockerBaseArgs(): execFile passed those quotes through, so docker --context "'default'" failed and every container behind a named context or remote daemon read as unreachable. The ssh probe and attach gain withRemoteTmuxPath() like every other remote tmux call, run in an explicit sh because the guard chain is POSIX and the remote login shell may be fish, and the probe holds a slot of the remote-ssh limiter and execs ssh so a timeout reaches it. Notes from a remote scan are no longer dropped, and every note now opens with its location. The adopted branch of killSession no longer blocks the event loop on docker or tmux, kills the in-container view by exact name, and clears the pane-exit and reconnect state the non-owned remote branch does. An adopted wrapper reports no injected statusline, since nothing could carry it. Key tables on the foreign server are left alone on purpose, and the invocation now says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a0e21109b2
commit
f464a22bc3
5 files changed
+237
-35
No files matched your search
+1
-1
@@ -1014,7 +1014,7 @@ export function resolveDockerCredentialArtifacts(
|
||||
* string; argv arrays must NOT carry literal quotes (mirror of docker-export's
|
||||
* dockerArgv).
|
||||
*/
|
||||
function dockerEngineArgv(docker: Pick<SessionDocker, 'engine' | 'context' | 'daemonHost'>): string[] {
|
||||
export function dockerEngineArgv(docker: Pick<SessionDocker, 'engine' | 'context' | 'daemonHost'>): string[] {
|
||||
const argv: string[] = [docker.engine === 'podman' ? 'podman' : 'docker'];
|
||||
if (docker.context) argv.push('--context', docker.context);
|
||||
if (docker.daemonHost) argv.push('-H', docker.daemonHost);
|
||||
|
||||
@@ -34,16 +34,16 @@ import { promisify } from 'node:util';
|
||||
import {
|
||||
buildForeignProbeScript,
|
||||
classifyForeignPaneMode,
|
||||
foreignRemoteShellCommand,
|
||||
foreignSessionId,
|
||||
isAdoptableSessionName,
|
||||
isAdoptableSocketPath,
|
||||
isCodemanOwnedPane,
|
||||
parseForeignProbeOutput,
|
||||
} from './foreign-tmux.js';
|
||||
import { buildDockerBaseArgs, shellescape } from './docker-hosts.js';
|
||||
import { buildSshConnectionArgs, remoteSshTarget } from './remote-hosts.js';
|
||||
import { readDockerCases, readDockerHosts } from './docker-hosts.js';
|
||||
import { readRemoteHosts } from './remote-hosts.js';
|
||||
import { dockerEngineArgv, readDockerCases, readDockerHosts } from './docker-hosts.js';
|
||||
import { buildSshConnectionArgs, readRemoteHosts, remoteSshTarget, shellescape } from './remote-hosts.js';
|
||||
import { runWithRemoteSshLimit } from './remote-ssh-limiter.js';
|
||||
import { resolveTmuxSocketName, getDataDir } from './config/instance.js';
|
||||
import { FOREIGN_CACHE_TTL_MS, FOREIGN_PROBE_MAX_BYTES, FOREIGN_PROBE_TIMEOUT_MS } from './config/foreign-tmux.js';
|
||||
import type { ForeignDiscoveryResult, ForeignTmuxSession } from './types/foreign-tmux.js';
|
||||
@@ -63,13 +63,18 @@ const PROBE_OPTS = {
|
||||
/**
|
||||
* Turn one probe's stdout into candidates. Shared by all three transports, so a
|
||||
* container's sessions and the host's are classified by identical rules.
|
||||
*
|
||||
* `where` opens every note this scan writes (`local`, `docker <name>`, `remote
|
||||
* <label>`). The browser tells the two kinds apart by it: a local note is re-read on
|
||||
* every poll, a container or host note only by the next explicit scan.
|
||||
*/
|
||||
function candidatesFrom(
|
||||
stdout: string,
|
||||
location: ForeignTmuxSession['location'],
|
||||
hostKey: string,
|
||||
extra: Pick<ForeignTmuxSession, 'hostId' | 'hostLabel' | 'containerName'>,
|
||||
notes?: string[]
|
||||
where: string,
|
||||
notes: string[]
|
||||
): ForeignTmuxSession[] {
|
||||
const probe = parseForeignProbeOutput(stdout);
|
||||
const ownSocket = resolveTmuxSocketName();
|
||||
@@ -112,9 +117,9 @@ function candidatesFrom(
|
||||
workingDir: pane.paneCurrentPath,
|
||||
});
|
||||
}
|
||||
if (unsafe && notes) {
|
||||
if (unsafe) {
|
||||
notes.push(
|
||||
`${unsafe} session(s) skipped: the tmux session name or socket path contains characters Codeman will not pass to a shell`
|
||||
`${where}: ${unsafe} session(s) skipped, the tmux session name or socket path contains characters Codeman will not pass to a shell`
|
||||
);
|
||||
}
|
||||
return out;
|
||||
@@ -145,7 +150,7 @@ export async function discoverLocalForeign(force = false): Promise<ForeignDiscov
|
||||
// argv array: no outer shell, so the script's own `$TMUX_TMPDIR`/`$(id -u)`
|
||||
// reach the inner sh intact (see @fileoverview).
|
||||
const { stdout } = await execFileAsync('sh', ['-c', buildForeignProbeScript()], PROBE_OPTS);
|
||||
sessions = candidatesFrom(stdout, 'local', 'local', {}, notes);
|
||||
sessions = candidatesFrom(stdout, 'local', 'local', {}, 'local', notes);
|
||||
} catch {
|
||||
// No tmux, no sockets, or a probe that timed out. "Nothing to adopt here"
|
||||
// is the only honest answer and it is never an error to the caller.
|
||||
@@ -171,7 +176,7 @@ export function invalidateForeignCache(): void {
|
||||
// ===========================================================================
|
||||
|
||||
/** Connection facts for one adoptable container, resolved from the registries. */
|
||||
interface DockerTargetInfo {
|
||||
export interface DockerTargetInfo {
|
||||
hostId: string;
|
||||
label: string;
|
||||
containerName: string;
|
||||
@@ -204,6 +209,18 @@ function dockerTargets(cases: DockerCase[], hosts: DockerHost[]): DockerTargetIn
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* The argv of one in-container probe.
|
||||
*
|
||||
* ⚠️ `dockerEngineArgv`, never `buildDockerBaseArgs`, whose `--context` and `-H`
|
||||
* values are shell-quoted for the launch string. `execFile` passes those quotes
|
||||
* through literally, so `docker --context "'default'"` answered "context not found"
|
||||
* and every container behind a named context or a remote daemon read as unreachable.
|
||||
*/
|
||||
export function foreignDockerProbeArgv(target: DockerTargetInfo): string[] {
|
||||
return [...dockerEngineArgv(target.base), 'exec', target.containerName, 'sh', '-lc', buildForeignProbeScript()];
|
||||
}
|
||||
|
||||
/**
|
||||
* Scan the containers behind the caller's docker cases. On demand only.
|
||||
*
|
||||
@@ -222,19 +239,16 @@ export async function discoverDockerForeign(cases: DockerCase[]): Promise<Foreig
|
||||
|
||||
await Promise.all(
|
||||
targets.map(async (t) => {
|
||||
const [engine, ...baseFlags] = buildDockerBaseArgs(t.base);
|
||||
const [engine, ...args] = foreignDockerProbeArgv(t);
|
||||
try {
|
||||
const { stdout } = await execFileAsync(
|
||||
engine,
|
||||
[...baseFlags, 'exec', t.containerName, 'sh', '-lc', buildForeignProbeScript()],
|
||||
PROBE_OPTS
|
||||
);
|
||||
const { stdout } = await execFileAsync(engine, args, PROBE_OPTS);
|
||||
sessions.push(
|
||||
...candidatesFrom(
|
||||
stdout,
|
||||
'docker',
|
||||
`${t.hostId} ${t.containerName}`,
|
||||
{ hostId: t.hostId, hostLabel: t.label, containerName: t.containerName },
|
||||
`docker ${t.containerName}`,
|
||||
notes
|
||||
)
|
||||
);
|
||||
@@ -251,6 +265,20 @@ export async function discoverDockerForeign(cases: DockerCase[]): Promise<Foreig
|
||||
// Remote
|
||||
// ===========================================================================
|
||||
|
||||
/**
|
||||
* The ssh command line of one remote probe.
|
||||
*
|
||||
* `exec` so a timeout kill reaches ssh itself, not only the local `sh -c` around it.
|
||||
*/
|
||||
export function foreignRemoteProbeCommand(host: RemoteHost): string {
|
||||
return [
|
||||
'exec',
|
||||
...buildSshConnectionArgs(host),
|
||||
remoteSshTarget(host),
|
||||
shellescape(foreignRemoteShellCommand(buildForeignProbeScript())),
|
||||
].join(' ');
|
||||
}
|
||||
|
||||
/**
|
||||
* Scan saved ssh hosts. On demand only — this is the expensive one.
|
||||
*
|
||||
@@ -258,6 +286,10 @@ export async function discoverDockerForeign(cases: DockerCase[]): Promise<Foreig
|
||||
* probe crosses one extra shell and must be single-quoted. `shellescape` does
|
||||
* that, and `buildForeignProbeScript()` is single-quote-free precisely so the
|
||||
* escaping stays a wrap rather than a rewrite.
|
||||
*
|
||||
* Each host holds a slot of the shared remote-ssh limiter: one saved host per
|
||||
* connection, all at once, is the burst OpenSSH's `MaxStartups` starts dropping.
|
||||
* A host that waits too long for a slot reads as unreachable this round.
|
||||
*/
|
||||
export async function discoverRemoteForeign(hosts: RemoteHost[]): Promise<ForeignDiscoveryResult> {
|
||||
if (IS_TEST_MODE) return { sessions: [], scannedAt: Date.now(), notes: [] };
|
||||
@@ -267,14 +299,14 @@ export async function discoverRemoteForeign(hosts: RemoteHost[]): Promise<Foreig
|
||||
|
||||
await Promise.all(
|
||||
hosts.map(async (host) => {
|
||||
const cmd = [...buildSshConnectionArgs(host), remoteSshTarget(host), shellescape(buildForeignProbeScript())].join(
|
||||
' '
|
||||
);
|
||||
const where = `remote ${host.label}`;
|
||||
try {
|
||||
const { stdout } = await execAsync(cmd, PROBE_OPTS);
|
||||
sessions.push(...candidatesFrom(stdout, 'remote', host.id, { hostId: host.id, hostLabel: host.label }));
|
||||
const { stdout } = await runWithRemoteSshLimit(() => execAsync(foreignRemoteProbeCommand(host), PROBE_OPTS), {
|
||||
maxWaitMs: FOREIGN_PROBE_TIMEOUT_MS,
|
||||
});
|
||||
sessions.push(...candidatesFrom(stdout, 'remote', host.id, { hostId: host.id, hostLabel: host.label }, where, notes));
|
||||
} catch {
|
||||
notes.push(`remote ${host.label}: unreachable, or no tmux`);
|
||||
notes.push(`${where}: unreachable, or no tmux`);
|
||||
}
|
||||
})
|
||||
);
|
||||
|
||||
+25
-3
@@ -70,6 +70,7 @@
|
||||
|
||||
import { collectDescendants } from './proc-tree.js';
|
||||
import { listClis } from './config/cli-registry/registry.js';
|
||||
import { withRemoteTmuxPath } from './remote-hosts.js';
|
||||
import { FOREIGN_MAX_PANES, FOREIGN_MAX_PROCS, FOREIGN_MAX_SOCKETS } from './config/foreign-tmux.js';
|
||||
import type { ForeignPaneProbe, ForeignTmuxLocationKind } from './types/foreign-tmux.js';
|
||||
import type { SessionAdopt, SessionMode } from './types/session.js';
|
||||
@@ -597,7 +598,7 @@ export interface ForeignAttachTarget {
|
||||
* noise out of the pane — the message we print says it better.
|
||||
* - ⚠️ The `=` prefix cannot go on `new-session -t` itself: tmux reads it as part of
|
||||
* the name and creates a group literally called `=work`. It has to be a separate
|
||||
* `has-session`. The read-only `attach-session` fallback takes `=` too.
|
||||
* `has-session`.
|
||||
* - `new-session -t` creates a session in the target's GROUP: same windows, our
|
||||
* own session options and current window (NOT our own size — see @fileoverview
|
||||
* for the measurement that disproved that).
|
||||
@@ -620,6 +621,12 @@ export interface ForeignAttachTarget {
|
||||
* killing a session in a group removes only that session (measured: the owner's
|
||||
* `work` survived its grouped view being killed). `2>/dev/null` because "no such
|
||||
* session" is the normal case, not an error.
|
||||
* - ⚠️ Nothing here touches the prefix table, unlike the nested servers Codeman runs
|
||||
* itself (`nestedPrefixTableResetCommands`). Our own socket already has its table
|
||||
* reduced by `createSession`, but key tables are SERVER-WIDE, so reducing the foreign
|
||||
* one would rewrite the owner's keys for every session on their server, and it would
|
||||
* outlive our detach. Same reason `window-size` is left alone above. The owner's
|
||||
* bindings stay reachable from a tab, behind our own `send-prefix`, as theirs to use.
|
||||
* - Single line: the string crosses `bash -c "..."`, where a real newline would
|
||||
* not survive JSON escaping.
|
||||
*/
|
||||
@@ -703,10 +710,25 @@ export function buildForeignDockerViewKillCommand(opts: {
|
||||
}): string {
|
||||
return (
|
||||
`${opts.dockerBase} exec ${fshq(opts.containerName)} ` +
|
||||
`tmux -S ${fshq(opts.socketPath)} kill-session -t ${fshq(opts.viewSession)}`
|
||||
`tmux -S ${fshq(opts.socketPath)} kill-session -t ${fshq(`=${opts.viewSession}`)}`
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* A POSIX script as the command line of an ssh connection.
|
||||
*
|
||||
* ssh hands its command to the remote user's LOGIN shell, which may be fish: the
|
||||
* guard chain's `{ ...; }` and the probe's `${VAR:-x}` are POSIX, so they run in an
|
||||
* explicit `sh`. The PATH line stays outside it because it is the part written for
|
||||
* whatever that shell is (`withRemoteTmuxPath`), and sshd's non-interactive PATH can
|
||||
* miss a Homebrew or per-user tmux, the same reason every other remote tmux call has
|
||||
* it. Single-quote-free input stays single-quote-free here, and `exec` keeps the
|
||||
* process tree one level shorter.
|
||||
*/
|
||||
export function foreignRemoteShellCommand(script: string): string {
|
||||
return withRemoteTmuxPath(`exec sh -c ${fshq(script)}`);
|
||||
}
|
||||
|
||||
export interface ForeignRemoteAttachOptions extends ForeignAttachTarget {
|
||||
/** `buildSshConnectionArgs(host)` — `['ssh', '-o BatchMode=yes', ...]`. */
|
||||
sshArgs: string[];
|
||||
@@ -724,7 +746,7 @@ export interface ForeignRemoteAttachOptions extends ForeignAttachTarget {
|
||||
*/
|
||||
export function buildForeignRemoteAttachCommand(opts: ForeignRemoteAttachOptions): string {
|
||||
const [ssh, batchMode, ...rest] = opts.sshArgs;
|
||||
return [ssh, batchMode, '-t', ...rest, opts.sshTarget, fshq(buildForeignTmuxInvocation(opts))]
|
||||
return [ssh, batchMode, '-t', ...rest, opts.sshTarget, fshq(foreignRemoteShellCommand(buildForeignTmuxInvocation(opts)))]
|
||||
.filter(Boolean)
|
||||
.join(' ');
|
||||
}
|
||||
+16
-9
@@ -1917,7 +1917,7 @@ function buildRemoteSessionCommand(options: {
|
||||
* `kill-session`. It is the same reasoning that keeps a non-owned REMOTE session
|
||||
* safe (see the detach-not-kill early return in `killSession`).
|
||||
*/
|
||||
function buildAdoptSessionCommand(adopt: SessionAdopt, sessionId: string): string {
|
||||
export function buildAdoptSessionCommand(adopt: SessionAdopt, sessionId: string): string {
|
||||
const target = {
|
||||
socketPath: adopt.socketPath,
|
||||
targetSession: adopt.targetSession,
|
||||
@@ -2614,7 +2614,8 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
const { command: statusLineCommand, state: statusLine } = await this._resolveStatusLine(
|
||||
mode,
|
||||
workingDir,
|
||||
!!(remote || docker)
|
||||
// An adopted wrapper launches no agent, so nothing can carry the exporter.
|
||||
!!(remote || docker || adopt)
|
||||
);
|
||||
const idleCompactionOff = await this._resolveIdleCompactionOptOut(mode, statusLine, keepWarm, spawnedByAgent);
|
||||
// The user's own REAL statusLine, if any (walked via Claude Code's own
|
||||
@@ -2937,7 +2938,8 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
const { command: statusLineCommand, state: statusLine } = await this._resolveStatusLine(
|
||||
mode,
|
||||
workingDir,
|
||||
!!(remote || docker)
|
||||
// An adopted wrapper launches no agent, so nothing can carry the exporter.
|
||||
!!(remote || docker || adopt)
|
||||
);
|
||||
const idleCompactionOff = await this._resolveIdleCompactionOptOut(mode, statusLine, keepWarm, spawnedByAgent);
|
||||
const userStatusLineCommand = statusLineCommand ? await findEffectiveUserStatusLineCommand(workingDir) : undefined;
|
||||
@@ -3218,24 +3220,27 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
// A `docker exec` survives its client, so the in-container view would stay
|
||||
// attached forever and leak. Best-effort, and it can only reach OUR view.
|
||||
// See buildForeignDockerViewKillCommand for why local/remote need nothing.
|
||||
// Fire and forget like the owned docker branch below: a daemon that stopped
|
||||
// answering must not hold the event loop for the whole timeout.
|
||||
if (session.adopt.location === 'docker' && session.adopt.docker) {
|
||||
try {
|
||||
execSync(
|
||||
`${buildForeignDockerViewKillCommand({
|
||||
exec(
|
||||
buildForeignDockerViewKillCommand({
|
||||
dockerBase: buildDockerBaseArgs(session.adopt.docker).join(' '),
|
||||
containerName: session.adopt.docker.containerName,
|
||||
socketPath: session.adopt.socketPath,
|
||||
viewSession: session.adopt.viewSession || foreignViewSessionName(sessionId),
|
||||
})} 2>/dev/null`,
|
||||
{ timeout: EXEC_TIMEOUT_MS, stdio: 'ignore' }
|
||||
}),
|
||||
{ timeout: EXEC_TIMEOUT_MS },
|
||||
() => {}
|
||||
);
|
||||
} catch {
|
||||
// The container may be gone, or the view already collected.
|
||||
// Best-effort: the container may be gone, or the view already collected.
|
||||
}
|
||||
}
|
||||
if (isValidMuxName(session.muxName)) {
|
||||
try {
|
||||
execSync(`${this.tmux()} kill-session -t "${session.muxName}" 2>/dev/null`, {
|
||||
await execAsync(`${this.tmux()} kill-session -t "${session.muxName}" 2>/dev/null`, {
|
||||
timeout: EXEC_TIMEOUT_MS,
|
||||
});
|
||||
} catch {
|
||||
@@ -3243,7 +3248,9 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
}
|
||||
}
|
||||
this.lastPaneCount.delete(session.muxName);
|
||||
this.clearPaneExit(session.muxName);
|
||||
this.sessions.delete(sessionId);
|
||||
this.clearRemoteReconnectState(sessionId);
|
||||
this.saveSessions();
|
||||
this.emit('sessionKilled', { sessionId });
|
||||
return true;
|
||||
|
||||
Reference in new issue
Block a user