mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 10:59:32 +01:00
Merge #770 fix(remote): stop the local shell expanding the remote launch (exit 127 since 1.47.0)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
commit
9e9031214e
14 files changed
+332
-47
No files matched your search
@@ -2,6 +2,7 @@ import { mkdtempSync, rmSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { listClis } from '../src/config/cli-registry/registry.js';
|
||||
import {
|
||||
defaultRemoteCommandForMode,
|
||||
readRemoteCases,
|
||||
@@ -13,7 +14,7 @@ import {
|
||||
writeRemoteCases,
|
||||
writeRemoteHosts,
|
||||
} from '../src/remote-hosts.js';
|
||||
import { RemoteHostSchema } from '../src/web/schemas.js';
|
||||
import { DockerHostSchema, RemoteHostSchema } from '../src/web/schemas.js';
|
||||
|
||||
describe('remote-hosts domain', () => {
|
||||
let dir: string | null = null;
|
||||
@@ -126,6 +127,24 @@ describe('remote-hosts domain', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('per-mode command overrides', () => {
|
||||
const modes = ['shell', ...listClis().map((cli) => cli.id)];
|
||||
const host = { id: 'h', label: 'H', host: '10.0.0.1', username: 'u' };
|
||||
|
||||
// `.strict()` refuses an unknown key, so a mode missing here cannot take an override, and
|
||||
// `commands.*` is the documented way out for a login shell the stock launch cannot run.
|
||||
it.each(modes)('accepts a %s override on a remote host and a docker host', (mode) => {
|
||||
const commands = { [mode]: 'exec my-agent' };
|
||||
const docker = { id: 'h', label: 'H', image: 'codeman/agent:base', commands };
|
||||
expect(RemoteHostSchema.safeParse({ ...host, commands }).success).toBe(true);
|
||||
expect(DockerHostSchema.safeParse(docker).success).toBe(true);
|
||||
});
|
||||
|
||||
it('still refuses a key that is no mode', () => {
|
||||
expect(RemoteHostSchema.safeParse({ ...host, commands: { nope: 'exec my-agent' } }).success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('rehydrateRemoteHostFields', () => {
|
||||
const persisted = {
|
||||
hostId: 'hufflepuff',
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* @fileoverview Which shell parses a remote pane command (#760).
|
||||
*
|
||||
* tmux runs a ONE-argument command through the remote default-shell, the user's login shell, and
|
||||
* bypasses it for two or more arguments. Codeman's own text carries `"${SHELL:-/bin/sh}"`, a parse
|
||||
* error in fish that kills the pane with status 127, so it must reach `/bin/sh -c` as separate
|
||||
* arguments. A per-host `commands.*` override is the user's text for their own shell: `source` and
|
||||
* `[[` work in bash and zsh and die under a dash `/bin/sh`, so it must stay one argument.
|
||||
*
|
||||
* CI has no fish, so this reads the argv tmux would receive instead: the real builder's string goes
|
||||
* through a real sh with `ssh` and `tmux` defined as functions that report their arguments.
|
||||
*/
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { listClis } from '../src/config/cli-registry/registry.js';
|
||||
import { buildRemoteLaunchCommand } from '../src/tmux-manager.js';
|
||||
import type { SessionMode, SessionRemote } from '../src/types.js';
|
||||
|
||||
const SESSION_ID = 'abcd1234-0000-0000-0000-000000000000';
|
||||
const remote: SessionRemote = {
|
||||
hostId: 'gpu',
|
||||
label: 'GPU',
|
||||
host: '10.0.0.42',
|
||||
username: 'ubuntu',
|
||||
remotePath: '/home/ubuntu/it is work',
|
||||
};
|
||||
|
||||
/** The one argument ssh would send, as the login shell on the far end receives it. */
|
||||
function sshArgument(launch: string): string {
|
||||
return execFileSync('/bin/sh', ['-c', `ssh() { for a; do l="$a"; done; printf '%s' "$l"; }; ${launch}`], {
|
||||
encoding: 'utf8',
|
||||
});
|
||||
}
|
||||
|
||||
/** What follows `new-session … -c <dir>`, up to the end of that tmux command. */
|
||||
function paneArgv(launch: string): string[] {
|
||||
const argv = execFileSync('/bin/sh', ['-c', `tmux() { printf '%s\\0' "$@"; }; ${sshArgument(launch)}`], {
|
||||
encoding: 'utf8',
|
||||
})
|
||||
.split('\0')
|
||||
.slice(0, -1);
|
||||
const head = argv.slice(0, argv.indexOf(';'));
|
||||
return head.slice(head.indexOf('-c', head.indexOf('new-session')) + 2);
|
||||
}
|
||||
|
||||
const launches: Array<[string, Parameters<typeof buildRemoteLaunchCommand>[0]]> = [
|
||||
...listClis().map((cli): [string, Parameters<typeof buildRemoteLaunchCommand>[0]] => [
|
||||
cli.id,
|
||||
{ mode: cli.id as SessionMode, remote, sessionId: SESSION_ID },
|
||||
]),
|
||||
['shell', { mode: 'shell', remote, sessionId: SESSION_ID }],
|
||||
['claude resume', { mode: 'claude', remote, sessionId: SESSION_ID, resumeSessionId: SESSION_ID }],
|
||||
['omp resume', { mode: 'omp', remote, sessionId: SESSION_ID, resumeSessionId: 'abc123' }],
|
||||
];
|
||||
|
||||
describe('the shell that parses a remote pane command', () => {
|
||||
it.each(launches)('%s: Codeman text goes to /bin/sh, never the default-shell', (_name, options) => {
|
||||
const argv = paneArgv(buildRemoteLaunchCommand(options));
|
||||
expect(argv.slice(0, 2)).toEqual(['/bin/sh', '-c']);
|
||||
expect(argv).toHaveLength(3);
|
||||
expect(argv[2]).toContain(`cd '/home/ubuntu/it is work' && `);
|
||||
});
|
||||
|
||||
it('leaves an override as one argument, parsed by the remote default-shell', () => {
|
||||
const override = 'source ~/env.sh && [[ -x ~/.local/bin/codx ]] && exec codx';
|
||||
const argv = paneArgv(
|
||||
buildRemoteLaunchCommand({
|
||||
mode: 'codex',
|
||||
remote: { ...remote, commands: { codex: override } },
|
||||
sessionId: SESSION_ID,
|
||||
})
|
||||
);
|
||||
expect(argv).toEqual([`cd '/home/ubuntu/it is work' && ${override}`]);
|
||||
});
|
||||
|
||||
it('keeps the rest of the tmux chain behind the longer pane command', () => {
|
||||
const sent = sshArgument(buildRemoteLaunchCommand({ mode: 'shell', remote, sessionId: SESSION_ID }));
|
||||
expect(sent).toContain(String.raw` \; set -t codeman-ssh-abcd1234 status off \; `);
|
||||
expect(sent).toMatch(/set -t codeman-ssh-abcd1234 remain-on-exit failed$/);
|
||||
});
|
||||
});
|
||||
@@ -159,7 +159,7 @@ describe('COD-107 buildRemoteLaunchCommand — threads connection args', () => {
|
||||
const path = sh('/home/ubuntu/work');
|
||||
const paneCommand = `cd ${path} && exec "\${SHELL:-/bin/sh}" -i -l`;
|
||||
const tmuxInvocation = [
|
||||
`tmux -L codeman-remote new-session -A -s ${remoteName} -c ${path} ${sh(paneCommand)}`,
|
||||
`tmux -L codeman-remote new-session -A -s ${remoteName} -c ${path} /bin/sh -c ${sh(paneCommand)}`,
|
||||
`set -t ${remoteName} status off`,
|
||||
`set -t ${remoteName} mouse off`,
|
||||
`set -t ${remoteName} prefix C-q`,
|
||||
|
||||
@@ -28,6 +28,20 @@ import type { MuxSession } from '../src/mux-interface.js';
|
||||
// ============================================================================
|
||||
|
||||
// Mock child_process
|
||||
/**
|
||||
* Runs a captured `… respawn-pane … bash -c <arg>` line through a real local sh (with a
|
||||
* foreign HOME/PATH) and returns the string bash would get, i.e. what ssh sends on.
|
||||
*/
|
||||
async function bashArgAfterLocalShell(respawnCmd: string): Promise<string> {
|
||||
const real = await vi.importActual<typeof import('node:child_process')>('node:child_process');
|
||||
const at = respawnCmd.indexOf(' bash -c ');
|
||||
expect(at).toBeGreaterThan(0);
|
||||
return real.execFileSync('/bin/sh', ['-c', `printf '%s' ${respawnCmd.slice(at + ' bash -c '.length)}`], {
|
||||
env: { HOME: '/local-home-xyz', PATH: '/local-path-xyz:/usr/bin:/bin' },
|
||||
encoding: 'utf8',
|
||||
});
|
||||
}
|
||||
|
||||
vi.mock('node:child_process', async () => {
|
||||
const actual = await vi.importActual('node:child_process');
|
||||
return {
|
||||
@@ -865,6 +879,98 @@ describe('TmuxManager (unit)', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('hands a remote launch to ssh with $HOME and $PATH unexpanded', async () => {
|
||||
// respawn-pane runs `bash -c <arg>` through the local sh. In double quotes that sh
|
||||
// expanded withRemoteTmuxPath's `$HOME/…:$PATH` to the Codeman host's values, so a
|
||||
// remote user with another home found no tmux and the pane exited 127.
|
||||
const NonTestTmuxManager = await importWithTmuxCommandsEnabled();
|
||||
const nonTestManager = new NonTestTmuxManager();
|
||||
|
||||
try {
|
||||
await nonTestManager.createSession({
|
||||
sessionId: 'abc12345-1234-5678-90ab-cdef12345679',
|
||||
workingDir: '/w',
|
||||
mode: 'shell',
|
||||
remote: { hostId: 'gpu-box', label: 'GPU Box', host: '10.0.0.42', username: 'ubuntu', remotePath: '/w' },
|
||||
});
|
||||
const launch = [...mockedExecSync.mock.calls, ...vi.mocked(exec).mock.calls]
|
||||
.map(([cmd]) => cmd)
|
||||
.find((cmd) => typeof cmd === 'string' && cmd.includes(' respawn-pane '));
|
||||
const sent = await bashArgAfterLocalShell(launch as string);
|
||||
expect(sent).toContain('ssh ');
|
||||
expect(sent).toContain('export PATH="$HOME/');
|
||||
expect(sent).toContain(':$PATH"; tmux ');
|
||||
expect(sent).not.toContain('local-home-xyz');
|
||||
expect(sent).not.toContain('local-path-xyz');
|
||||
} finally {
|
||||
nonTestManager.destroy();
|
||||
}
|
||||
});
|
||||
|
||||
it('respawns a remote pane with $HOME unexpanded as well', async () => {
|
||||
const NonTestTmuxManager = await importWithTmuxCommandsEnabled();
|
||||
const nonTestManager = new NonTestTmuxManager();
|
||||
nonTestManager.registerSession({
|
||||
sessionId: 'respawnremote2',
|
||||
muxName: 'codeman-abcd8888',
|
||||
pid: 1000,
|
||||
createdAt: Date.now(),
|
||||
workingDir: '/w',
|
||||
mode: 'shell',
|
||||
attached: false,
|
||||
});
|
||||
|
||||
try {
|
||||
await nonTestManager.respawnPane({
|
||||
sessionId: 'respawnremote2',
|
||||
workingDir: '/w',
|
||||
mode: 'shell',
|
||||
remote: { hostId: 'gpu-box', label: 'GPU Box', host: '10.0.0.42', username: 'ubuntu', remotePath: '/w' },
|
||||
});
|
||||
const { exec: currentExec } = await import('node:child_process');
|
||||
const respawnCall = vi
|
||||
.mocked(currentExec)
|
||||
.mock.calls.find(([cmd]) => typeof cmd === 'string' && cmd.includes(' respawn-pane '));
|
||||
const sent = await bashArgAfterLocalShell(respawnCall?.[0] as string);
|
||||
expect(sent).toContain('export PATH="$HOME/');
|
||||
expect(sent).not.toContain('local-home-xyz');
|
||||
} finally {
|
||||
nonTestManager.destroy();
|
||||
}
|
||||
});
|
||||
|
||||
it('hands a docker override to bash with $HOME unexpanded', async () => {
|
||||
const NonTestTmuxManager = await importWithTmuxCommandsEnabled();
|
||||
const nonTestManager = new NonTestTmuxManager();
|
||||
|
||||
try {
|
||||
await nonTestManager.createSession({
|
||||
sessionId: 'abc12345-1234-5678-90ab-cdef12345680',
|
||||
workingDir: '/w',
|
||||
mode: 'shell',
|
||||
docker: {
|
||||
hostId: 'h',
|
||||
label: 'L',
|
||||
engine: 'docker',
|
||||
image: 'codeman/agent:base',
|
||||
containerName: 'codeman-case-x',
|
||||
hostWorkspacePath: '/w',
|
||||
containerWorkdir: '/w',
|
||||
commands: { shell: 'echo CONTAINER_HOME=$HOME; exec sleep 30' },
|
||||
} as never,
|
||||
});
|
||||
const launch = [...mockedExecSync.mock.calls, ...vi.mocked(exec).mock.calls]
|
||||
.map(([cmd]) => cmd)
|
||||
.find((cmd) => typeof cmd === 'string' && cmd.includes(' respawn-pane '));
|
||||
const sent = await bashArgAfterLocalShell(launch as string);
|
||||
expect(sent).toContain('docker');
|
||||
expect(sent).toContain('CONTAINER_HOME=$HOME');
|
||||
expect(sent).not.toContain('local-home-xyz');
|
||||
} finally {
|
||||
nonTestManager.destroy();
|
||||
}
|
||||
});
|
||||
|
||||
it('changes the global history default on tmux versions that cannot resize panes', async () => {
|
||||
const NonTestTmuxManager = await importWithTmuxCommandsEnabled();
|
||||
const nonTestManager = new NonTestTmuxManager();
|
||||
|
||||
Reference in new issue
Block a user