mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 02:29:33 +01:00
fix(remote): keep Codeman's pane command away from the remote default-shell
Handing the launch to bash verbatim exposed `"${SHELL:-/bin/sh}"` to the far
end for the first time. tmux runs a one-argument command through the remote
default-shell, not /bin/sh as the comments said, and fish refuses `${...}`:
on a fish host, or a bash login with `set -g default-shell fish`, every new
pane died with status 127. master only survived because the local sh had
already rewritten that token to this host's shell path.
Codeman's own text now goes to `/bin/sh -c` as separate tmux arguments, which
tmux execs without a shell. A commands.* override stays one argument and keeps
the default-shell it was written for: under `/bin/sh` a `source` or `[[` in an
override dies with 127 on bash and zsh hosts.
The tests read the argv tmux would receive through a real sh, so they need no
fish: every stock launch must reach /bin/sh and an override must not. The
docker half of bashCArg was pinned by nothing, so it gets its own test.
Comments that described the removed double-quote layer are corrected, and
remote-hosts.ts no longer claims overrides are seeded from the default text
or that they rescue nushell and elvish.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
21256202ba
commit
012ca2e971
9 files changed
+190
-37
No files matched your search
@@ -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`,
|
||||
|
||||
@@ -939,6 +939,38 @@ describe('TmuxManager (unit)', () => {
|
||||
}
|
||||
});
|
||||
|
||||
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