mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(shell,remote-ssh): allowlist the login flags, and keep only CRASHED remote panes
Follow-up to #209 and #210. Both land a real fix (a pane that is a login shell picks up /etc/profile and the per-user PATH entries an ssh remote command never sees, which is what was failing agent CLIs with exit 127). Three corrections: 1. `-i -l` is no longer hardcoded onto the resolved shell. That path ultimately comes from the passwd entry, which is user data and can name anything, and a shell that rejects an unknown flag exits on the spot: nushell, elvish and xonsh take neither flag, so a user with one of those in passwd would have gotten a dead pane on arrival, which is exactly the #208 failure #209 builds on top of. loginShellArgs() applies them only to the POSIX-family shells verified to accept both, and a test really launches every allowlisted shell present on the machine rather than trusting the set. csh/tcsh are excluded deliberately: tcsh honors -l only when it is the ONLY flag. 2. `remain-on-exit on` -> `failed`, moved LAST in the tmux command chain. `on` keeps the pane after a CLEAN exit too, so typing `exit` in a remote shell stranded a dead pane, the session outlived it, and the next launch's `-A` reattached to that corpse: "Pane is dead (status 0)" instead of a shell, permanently, on the DEFAULT path. Verified against a real tmux, as was the fix: `failed` tears the session down on status 0 and keeps the pane on 127 with the "command not found" still on screen, which is the case #210 wanted. It is last because tmux aborts the remaining commands of a `\;` sequence once one errors (also verified) and `failed` needs tmux >= 3.2 on the REMOTE host; leading, a rejection there would have silently dropped status/mouse/prefix/ escape-time/window-size along with it. 3. `$SHELL` -> `"${SHELL:-/bin/sh}"`, via one shared remoteLoginShellCommand() helper instead of the string being rebuilt in tmux-manager as well. Also corrects the rationale both PRs carried: a tmux pane already hands the shell a tty, so it was interactive all along ($- contains i for a bare /bin/bash in a pane) and ~/.bashrc was always being sourced. `-l` is the flag doing the work. End-to-end verified, not just unit-tested: the emitted remote pane command was run through all three quoting layers under a minimal sshd-style PATH with the CLI installed only on a login-shell PATH entry, and it resolved and launched the CLI with its arguments intact and a space-containing remote path preserved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+37
-6
@@ -58,6 +58,37 @@ export async function writeRemoteCases(configDir: string, cases: RemoteCase[]):
|
|||||||
await writeJsonArray(configDir, remoteCasesPath(configDir), cases);
|
await writeJsonArray(configDir, remoteCasesPath(configDir), cases);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The remote user's login shell, defaulted and quoted.
|
||||||
|
*
|
||||||
|
* The default is belt-and-braces, not a live bug: an empty `$SHELL` would expand
|
||||||
|
* to `exec -i -l`, which the shell reads as `exec -i` — "not found", pane dead on
|
||||||
|
* arrival, the #208 failure all over again (verified: `sh -c 'exec $SHELL -i -l'`
|
||||||
|
* with SHELL unset prints `exec: -i: not found`). In practice tmux always exports
|
||||||
|
* SHELL into a pane from its own `default-shell` option, so the command as USED
|
||||||
|
* here is safe either way (also verified). The default matters because these
|
||||||
|
* strings are the seed values a per-host `commands.*` override is edited from, and
|
||||||
|
* nothing constrains where an edited one ends up running. Quoted for a shell path
|
||||||
|
* containing spaces. `/bin/sh` exists on every POSIX host.
|
||||||
|
*/
|
||||||
|
const REMOTE_LOGIN_SHELL = '"${SHELL:-/bin/sh}"';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Run `command` through the remote user's interactive login shell, so per-user
|
||||||
|
* PATH entries (~/.local/bin, ~/.opencode/bin, …) are resolved before the CLI name
|
||||||
|
* is looked up. ssh's remote-command execution is neither interactive nor login,
|
||||||
|
* so a bare `exec claude` sees only sshd's minimal default PATH and dies with
|
||||||
|
* "command not found" (exit 127).
|
||||||
|
*
|
||||||
|
* Shells that take neither flag (nushell, elvish, …) cannot be detected from here
|
||||||
|
* the way `loginShellArgs()` detects them locally, since the shell is whatever the
|
||||||
|
* REMOTE passwd says. A host like that is what the per-host `commands.*` override
|
||||||
|
* is for.
|
||||||
|
*/
|
||||||
|
export function remoteLoginShellCommand(command: string): string {
|
||||||
|
return `exec ${REMOTE_LOGIN_SHELL} -i -l -c ${shellescape(command)}`;
|
||||||
|
}
|
||||||
|
|
||||||
export function defaultRemoteCommandForMode(mode: SessionMode): string {
|
export function defaultRemoteCommandForMode(mode: SessionMode): string {
|
||||||
// Agent CLIs (claude/opencode/codex/gemini/antigravity) are typically installed
|
// Agent CLIs (claude/opencode/codex/gemini/antigravity) are typically installed
|
||||||
// under per-user paths like ~/.local/bin or ~/.opencode/bin, added to PATH only by
|
// under per-user paths like ~/.local/bin or ~/.opencode/bin, added to PATH only by
|
||||||
@@ -73,15 +104,15 @@ export function defaultRemoteCommandForMode(mode: SessionMode): string {
|
|||||||
// /etc/passwd entry, so this launches their actual login shell (zsh,
|
// /etc/passwd entry, so this launches their actual login shell (zsh,
|
||||||
// fish, etc.). -i -l so it sources rc files (~/.zshrc etc.), matching
|
// fish, etc.). -i -l so it sources rc files (~/.zshrc etc.), matching
|
||||||
// the local shell-mode launch.
|
// the local shell-mode launch.
|
||||||
shell: 'exec $SHELL -i -l',
|
shell: `exec ${REMOTE_LOGIN_SHELL} -i -l`,
|
||||||
// Mirror the LOCAL claude default so the remote agent runs non-interactively
|
// Mirror the LOCAL claude default so the remote agent runs non-interactively
|
||||||
// (no trust-folder/permission prompt that nothing on the remote answers). The
|
// (no trust-folder/permission prompt that nothing on the remote answers). The
|
||||||
// per-host `commands.claude` override stays the escape hatch.
|
// per-host `commands.claude` override stays the escape hatch.
|
||||||
claude: `exec $SHELL -i -l -c ${shellescape('claude --dangerously-skip-permissions')}`,
|
claude: remoteLoginShellCommand('claude --dangerously-skip-permissions'),
|
||||||
opencode: `exec $SHELL -i -l -c ${shellescape('opencode')}`,
|
opencode: remoteLoginShellCommand('opencode'),
|
||||||
codex: `exec $SHELL -i -l -c ${shellescape('codex')}`,
|
codex: remoteLoginShellCommand('codex'),
|
||||||
gemini: `exec $SHELL -i -l -c ${shellescape('gemini')}`,
|
gemini: remoteLoginShellCommand('gemini'),
|
||||||
antigravity: `exec $SHELL -i -l -c ${shellescape('agy')}`,
|
antigravity: remoteLoginShellCommand('agy'),
|
||||||
};
|
};
|
||||||
return commands[mode as RemoteCommandMode] || commands.shell;
|
return commands[mode as RemoteCommandMode] || commands.shell;
|
||||||
}
|
}
|
||||||
|
|||||||
+35
-15
@@ -49,7 +49,12 @@ import {
|
|||||||
type DockerCommandMode,
|
type DockerCommandMode,
|
||||||
} from './types.js';
|
} from './types.js';
|
||||||
import { buildEffortCliArgs } from './session-cli-builder.js';
|
import { buildEffortCliArgs } from './session-cli-builder.js';
|
||||||
import { buildSshConnectionArgs, defaultRemoteCommandForMode, remoteSshTarget } from './remote-hosts.js';
|
import {
|
||||||
|
buildSshConnectionArgs,
|
||||||
|
defaultRemoteCommandForMode,
|
||||||
|
remoteLoginShellCommand,
|
||||||
|
remoteSshTarget,
|
||||||
|
} from './remote-hosts.js';
|
||||||
import {
|
import {
|
||||||
buildDockerBaseArgs,
|
buildDockerBaseArgs,
|
||||||
buildDockerCreateArgs,
|
buildDockerCreateArgs,
|
||||||
@@ -72,6 +77,7 @@ import {
|
|||||||
resolveGeminiDir,
|
resolveGeminiDir,
|
||||||
resolveAntigravityDir,
|
resolveAntigravityDir,
|
||||||
resolveLocalShell,
|
resolveLocalShell,
|
||||||
|
loginShellArgs,
|
||||||
} from './utils/index.js';
|
} from './utils/index.js';
|
||||||
import type {
|
import type {
|
||||||
TerminalMultiplexer,
|
TerminalMultiplexer,
|
||||||
@@ -784,11 +790,15 @@ export function buildSpawnCommand(options: {
|
|||||||
// SERVER process's env — empty in containers and system systemd units, leaving
|
// SERVER process's env — empty in containers and system systemd units, leaving
|
||||||
// the pane command ending in a dangling `&&` ("syntax error: unexpected end of
|
// the pane command ending in a dangling `&&` ("syntax error: unexpected end of
|
||||||
// file", pane dead on arrival). Resolve it in Node and quote the result.
|
// file", pane dead on arrival). Resolve it in Node and quote the result.
|
||||||
// -i -l: interactive login shell, so the resolved shell sources the user's rc
|
// #209: launch it as a LOGIN shell, which is what tmux itself does for a pane
|
||||||
// files (~/.zshrc, ~/.bashrc, etc.) instead of running as a bare non-interactive
|
// with no `default-command`, so a Codeman shell tab matches a hand-started tmux
|
||||||
// child of the non-interactive `bash -c` that launches the pane — without this,
|
// one. That is what picks up /etc/profile and /etc/profile.d/* — a systemd
|
||||||
// aliases, PATH additions, and tool init (zoxide, nvm, etc.) are silently dropped.
|
// --user service never sourced them, so its PATH is what every pane inherited.
|
||||||
return `${shellescape(resolveLocalShell())} -i -l`;
|
// The flags come from loginShellArgs() rather than being hardcoded: they are
|
||||||
|
// appended to a path that ultimately comes from the passwd entry, and a shell
|
||||||
|
// that rejects an unknown flag exits on the spot, which is #208 all over again.
|
||||||
|
const shell = resolveLocalShell();
|
||||||
|
return `${shellescape(shell)}${loginShellArgs(shell)}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -867,7 +877,7 @@ export function buildRemoteLaunchCommand(options: {
|
|||||||
const modeCommand = override
|
const modeCommand = override
|
||||||
? override
|
? override
|
||||||
: mode === 'claude'
|
: mode === 'claude'
|
||||||
? `exec $SHELL -i -l -c ${shellescape(`claude${buildClaudePermissionFlags(claudeMode, allowedTools)}`)}`
|
? remoteLoginShellCommand(`claude${buildClaudePermissionFlags(claudeMode, allowedTools)}`)
|
||||||
: defaultRemoteCommandForMode(mode);
|
: defaultRemoteCommandForMode(mode);
|
||||||
const remoteName = remoteTmuxSessionName(sessionId);
|
const remoteName = remoteTmuxSessionName(sessionId);
|
||||||
|
|
||||||
@@ -882,14 +892,6 @@ export function buildRemoteLaunchCommand(options: {
|
|||||||
// shared remote tmux server's other sessions keep their own prefix/mouse.
|
// shared remote tmux server's other sessions keep their own prefix/mouse.
|
||||||
const tmuxInvocation = [
|
const tmuxInvocation = [
|
||||||
`tmux -L ${REMOTE_TMUX_SOCKET} new-session -A -s ${remoteName} -c ${shellescape(remote.remotePath)} ${shellescape(paneCommand)}`,
|
`tmux -L ${REMOTE_TMUX_SOCKET} new-session -A -s ${remoteName} -c ${shellescape(remote.remotePath)} ${shellescape(paneCommand)}`,
|
||||||
// Without this, tmux's default behavior destroys the pane -> window ->
|
|
||||||
// session (and, being the only session, the whole remote server) the
|
|
||||||
// instant paneCommand exits for ANY reason -- even something transient.
|
|
||||||
// That tears down the local ssh -t attach along with it (dead pane,
|
|
||||||
// status whatever ssh reported), and reconnect's `-A` then creates a
|
|
||||||
// fresh session with no trace of what actually happened. Set first, so
|
|
||||||
// it applies as early as possible after the pane starts.
|
|
||||||
`set -t ${remoteName} remain-on-exit on`,
|
|
||||||
`set -t ${remoteName} status off`,
|
`set -t ${remoteName} status off`,
|
||||||
`set -t ${remoteName} mouse off`,
|
`set -t ${remoteName} mouse off`,
|
||||||
`set -t ${remoteName} prefix C-q`,
|
`set -t ${remoteName} prefix C-q`,
|
||||||
@@ -901,6 +903,24 @@ export function buildRemoteLaunchCommand(options: {
|
|||||||
// Per-session scoped (`set -t <name>`, matching #145's hardening) so a shared
|
// Per-session scoped (`set -t <name>`, matching #145's hardening) so a shared
|
||||||
// remote tmux server's other sessions keep their own sizing behavior.
|
// remote tmux server's other sessions keep their own sizing behavior.
|
||||||
`set -t ${remoteName} window-size latest`,
|
`set -t ${remoteName} window-size latest`,
|
||||||
|
// #210: keep a CRASHED pane so the failure is still on screen. Without this,
|
||||||
|
// tmux destroys the pane -> window -> session (and, being the only session,
|
||||||
|
// the whole remote server) the instant the pane command exits, which tears the
|
||||||
|
// local `ssh -t` attach down with it; reconnect's `-A` then builds a fresh
|
||||||
|
// session and the cycle can repeat as a flap loop with no evidence surviving.
|
||||||
|
// That is how the exit-127 PATH bug fixed above stayed invisible.
|
||||||
|
//
|
||||||
|
// `failed`, NOT `on`: `on` keeps the pane on a CLEAN exit too, so typing
|
||||||
|
// `exit` in a remote shell leaves a dead pane behind, the session outlives it,
|
||||||
|
// and the next launch's `-A` reattaches to that corpse ("Pane is dead (status
|
||||||
|
// 0)") instead of starting a shell — verified against a real tmux. `failed`
|
||||||
|
// keeps the pane only on a non-zero exit, which is exactly the diagnostic case.
|
||||||
|
//
|
||||||
|
// LAST in the chain on purpose: tmux aborts the remaining commands of a `\;`
|
||||||
|
// sequence once one errors (also verified), and `failed` needs tmux >= 3.2 on
|
||||||
|
// the REMOTE host. Trailing, a rejection costs only this option; leading, it
|
||||||
|
// would silently drop status/mouse/prefix/escape-time/window-size with it.
|
||||||
|
`set -t ${remoteName} remain-on-exit failed`,
|
||||||
].join(' \\; ');
|
].join(' \\; ');
|
||||||
|
|
||||||
// ssh runs its trailing args through the remote login shell, so the entire
|
// ssh runs its trailing args through the remote login shell, so the entire
|
||||||
|
|||||||
+1
-1
@@ -26,7 +26,7 @@ export { isSafePushEndpoint } from './push-endpoint-validation.js';
|
|||||||
export { stringSimilarity, fuzzyPhraseMatch, todoContentHash } from './string-similarity.js';
|
export { stringSimilarity, fuzzyPhraseMatch, todoContentHash } from './string-similarity.js';
|
||||||
export { assertNever } from './type-safety.js';
|
export { assertNever } from './type-safety.js';
|
||||||
export { wrapWithNice } from './nice-wrapper.js';
|
export { wrapWithNice } from './nice-wrapper.js';
|
||||||
export { resolveLocalShell } from './shell-resolver.js';
|
export { resolveLocalShell, loginShellArgs } from './shell-resolver.js';
|
||||||
export { findClaudeDir, getAugmentedPath, getClaudeCliVersion, getClaudeBinaryPath } from './claude-cli-resolver.js';
|
export { findClaudeDir, getAugmentedPath, getClaudeCliVersion, getClaudeBinaryPath } from './claude-cli-resolver.js';
|
||||||
export { spawnPtyWithHelperRepair } from './node-pty-repair.js';
|
export { spawnPtyWithHelperRepair } from './node-pty-repair.js';
|
||||||
export { resolveOpenCodeDir } from './opencode-cli-resolver.js';
|
export { resolveOpenCodeDir } from './opencode-cli-resolver.js';
|
||||||
|
|||||||
@@ -33,6 +33,18 @@ const FALLBACK_SHELLS = ['/bin/bash', '/bin/zsh', '/bin/sh'];
|
|||||||
*/
|
*/
|
||||||
const NON_INTERACTIVE_SHELLS = new Set(['nologin', 'false', 'true', 'sync']);
|
const NON_INTERACTIVE_SHELLS = new Set(['nologin', 'false', 'true', 'sync']);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Shells verified to accept BOTH `-i` and `-l`. Deliberately an allowlist, not a
|
||||||
|
* blocklist: a shell that rejects an unknown flag exits immediately, which is the
|
||||||
|
* dead-pane-on-arrival failure this module exists to prevent (#208). The passwd
|
||||||
|
* entry is user data and can name anything — nushell, elvish, and xonsh all take
|
||||||
|
* neither flag in this form, so they get a bare launch instead of a dead tab.
|
||||||
|
*
|
||||||
|
* csh/tcsh are excluded on purpose: tcsh honors `-l` only when it is the ONLY
|
||||||
|
* flag, so `-i -l` would silently not be a login shell there anyway.
|
||||||
|
*/
|
||||||
|
const LOGIN_FLAG_SHELLS = new Set(['sh', 'bash', 'dash', 'ash', 'zsh', 'ksh', 'ksh93', 'mksh', 'pdksh', 'fish']);
|
||||||
|
|
||||||
function isUsableShell(candidate: string): boolean {
|
function isUsableShell(candidate: string): boolean {
|
||||||
if (!candidate.startsWith('/')) return false;
|
if (!candidate.startsWith('/')) return false;
|
||||||
const base = candidate.slice(candidate.lastIndexOf('/') + 1);
|
const base = candidate.slice(candidate.lastIndexOf('/') + 1);
|
||||||
@@ -76,3 +88,24 @@ export function resolveLocalShell(): string {
|
|||||||
// best guess and is far better than emitting an empty command.
|
// best guess and is far better than emitting an empty command.
|
||||||
return '/bin/sh';
|
return '/bin/sh';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Flags that make `shellPath` a login shell, or `''` when it takes none we trust.
|
||||||
|
*
|
||||||
|
* A tmux pane already hands the shell a tty, so it is interactive with or without
|
||||||
|
* `-i` (verified: `$-` contains `i` for a bare `/bin/bash` in a pane, which is why
|
||||||
|
* `~/.bashrc` has always been sourced). The flag that actually changes anything is
|
||||||
|
* `-l`: it makes the pane a LOGIN shell, matching what tmux itself does when it
|
||||||
|
* spawns a pane with no `default-command`, and picking up the `/etc/profile` and
|
||||||
|
* `/etc/profile.d/*` PATH entries that a systemd-spawned server never sourced.
|
||||||
|
*
|
||||||
|
* `-i` is kept alongside it because for bash the two select different files —
|
||||||
|
* login reads `~/.bash_profile`, interactive-non-login reads `~/.bashrc` — and
|
||||||
|
* asking for both is the closest thing to "the shell the user actually gets".
|
||||||
|
*
|
||||||
|
* Returns a string ready to append to an already-escaped shell path.
|
||||||
|
*/
|
||||||
|
export function loginShellArgs(shellPath: string): string {
|
||||||
|
const base = shellPath.slice(shellPath.lastIndexOf('/') + 1);
|
||||||
|
return LOGIN_FLAG_SHELLS.has(base) ? ' -i -l' : '';
|
||||||
|
}
|
||||||
|
|||||||
@@ -120,6 +120,6 @@ describe('Antigravity mode gates', () => {
|
|||||||
expect(defaultDockerCommandForMode('antigravity')).toBe('exec agy');
|
expect(defaultDockerCommandForMode('antigravity')).toBe('exec agy');
|
||||||
// Routed through an interactive login shell so per-user PATH entries resolve —
|
// Routed through an interactive login shell so per-user PATH entries resolve —
|
||||||
// same fix as the other remote agent CLIs (see defaultRemoteCommandForMode).
|
// same fix as the other remote agent CLIs (see defaultRemoteCommandForMode).
|
||||||
expect(defaultRemoteCommandForMode('antigravity')).toBe("exec $SHELL -i -l -c 'agy'");
|
expect(defaultRemoteCommandForMode('antigravity')).toBe('exec "${SHELL:-/bin/sh}" -i -l -c \'agy\'');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -55,13 +55,15 @@ describe('remote-hosts domain', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('returns safe mode defaults and remote display values', () => {
|
it('returns safe mode defaults and remote display values', () => {
|
||||||
expect(defaultRemoteCommandForMode('shell')).toBe('exec $SHELL -i -l');
|
expect(defaultRemoteCommandForMode('shell')).toBe('exec "${SHELL:-/bin/sh}" -i -l');
|
||||||
// Routed through an interactive login shell so per-user PATH entries (e.g.
|
// Routed through an interactive login shell so per-user PATH entries (e.g.
|
||||||
// ~/.local/bin, ~/.opencode/bin) resolve — a bare `exec codex` sees only
|
// ~/.local/bin, ~/.opencode/bin) resolve — a bare `exec codex` sees only
|
||||||
// sshd's minimal default PATH and fails with "command not found".
|
// sshd's minimal default PATH and fails with "command not found".
|
||||||
expect(defaultRemoteCommandForMode('codex')).toBe("exec $SHELL -i -l -c 'codex'");
|
expect(defaultRemoteCommandForMode('codex')).toBe('exec "${SHELL:-/bin/sh}" -i -l -c \'codex\'');
|
||||||
// Mirrors the local claude default so the remote agent runs non-interactively.
|
// Mirrors the local claude default so the remote agent runs non-interactively.
|
||||||
expect(defaultRemoteCommandForMode('claude')).toBe("exec $SHELL -i -l -c 'claude --dangerously-skip-permissions'");
|
expect(defaultRemoteCommandForMode('claude')).toBe(
|
||||||
|
'exec "${SHELL:-/bin/sh}" -i -l -c \'claude --dangerously-skip-permissions\''
|
||||||
|
);
|
||||||
expect(remoteSshTarget({ id: 'h1', label: 'H1', host: 'box.local', username: 'aamer' })).toBe('aamer@box.local');
|
expect(remoteSshTarget({ id: 'h1', label: 'H1', host: 'box.local', username: 'aamer' })).toBe('aamer@box.local');
|
||||||
expect(remoteDisplayPath({ username: 'aamer', host: 'box.local', path: '/opt/work' })).toBe(
|
expect(remoteDisplayPath({ username: 'aamer', host: 'box.local', path: '/opt/work' })).toBe(
|
||||||
'aamer@box.local:/opt/work'
|
'aamer@box.local:/opt/work'
|
||||||
|
|||||||
@@ -150,16 +150,19 @@ describe('COD-107 buildRemoteLaunchCommand — threads connection args', () => {
|
|||||||
const sh = (s: string) => "'" + s.replace(/'/g, "'\\''") + "'";
|
const sh = (s: string) => "'" + s.replace(/'/g, "'\\''") + "'";
|
||||||
const remoteName = `codeman-ssh-${SESSION_ID.slice(0, 8)}`;
|
const remoteName = `codeman-ssh-${SESSION_ID.slice(0, 8)}`;
|
||||||
const path = sh('/home/ubuntu/work');
|
const path = sh('/home/ubuntu/work');
|
||||||
const paneCommand = `cd ${path} && exec $SHELL -i -l`;
|
const paneCommand = `cd ${path} && exec "\${SHELL:-/bin/sh}" -i -l`;
|
||||||
const tmuxInvocation = [
|
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} ${sh(paneCommand)}`,
|
||||||
`set -t ${remoteName} remain-on-exit on`,
|
|
||||||
`set -t ${remoteName} status off`,
|
`set -t ${remoteName} status off`,
|
||||||
`set -t ${remoteName} mouse off`,
|
`set -t ${remoteName} mouse off`,
|
||||||
`set -t ${remoteName} prefix C-q`,
|
`set -t ${remoteName} prefix C-q`,
|
||||||
'set -s escape-time 0',
|
'set -s escape-time 0',
|
||||||
// COD-106 — shared/collaborative sizing, per-session scoped (never -g).
|
// COD-106 — shared/collaborative sizing, per-session scoped (never -g).
|
||||||
`set -t ${remoteName} window-size latest`,
|
`set -t ${remoteName} window-size latest`,
|
||||||
|
// #210 — keep a CRASHED pane for diagnosis. `failed` (not `on`, which would
|
||||||
|
// also strand a pane after a clean `exit`), and LAST because tmux aborts the
|
||||||
|
// remaining commands of a `\;` chain on error and `failed` needs tmux >= 3.2.
|
||||||
|
`set -t ${remoteName} remain-on-exit failed`,
|
||||||
].join(' \\; ');
|
].join(' \\; ');
|
||||||
// Connection args (with the default -o ConnectTimeout=10) sit after -t.
|
// Connection args (with the default -o ConnectTimeout=10) sit after -t.
|
||||||
const expected = `ssh -o BatchMode=yes -t -o ConnectTimeout=10 ${remoteSshTarget(baseRemote)} ${sh(tmuxInvocation)}`;
|
const expected = `ssh -o BatchMode=yes -t -o ConnectTimeout=10 ${remoteSshTarget(baseRemote)} ${sh(tmuxInvocation)}`;
|
||||||
|
|||||||
@@ -17,7 +17,7 @@
|
|||||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||||
import { execFileSync } from 'node:child_process';
|
import { execFileSync } from 'node:child_process';
|
||||||
import { buildSpawnCommand } from '../src/tmux-manager.js';
|
import { buildSpawnCommand } from '../src/tmux-manager.js';
|
||||||
import { resolveLocalShell } from '../src/utils/shell-resolver.js';
|
import { loginShellArgs, resolveLocalShell } from '../src/utils/shell-resolver.js';
|
||||||
|
|
||||||
describe('resolveLocalShell', () => {
|
describe('resolveLocalShell', () => {
|
||||||
const originalShell = process.env.SHELL;
|
const originalShell = process.env.SHELL;
|
||||||
@@ -70,6 +70,40 @@ describe('resolveLocalShell', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('loginShellArgs (#209 login flags, allowlisted)', () => {
|
||||||
|
it('asks for a login shell on the POSIX-family shells that accept the flags', () => {
|
||||||
|
for (const shell of ['/bin/sh', '/bin/bash', '/bin/dash', '/usr/bin/zsh', '/usr/local/bin/fish', '/bin/ksh']) {
|
||||||
|
expect(loginShellArgs(shell)).toBe(' -i -l');
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('adds nothing for shells that take neither flag, so the pane cannot die on arrival', () => {
|
||||||
|
// The shell path can come from the passwd entry, which is user data and can
|
||||||
|
// name anything. A shell that rejects an unknown flag exits immediately —
|
||||||
|
// indistinguishable from the #208 dead-pane-on-arrival this module prevents.
|
||||||
|
// csh/tcsh are here too: tcsh honors -l only when it is the ONLY flag.
|
||||||
|
for (const shell of ['/usr/bin/nu', '/usr/bin/elvish', '/usr/bin/xonsh', '/bin/tcsh', '/bin/csh']) {
|
||||||
|
expect(loginShellArgs(shell)).toBe('');
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('really launches for every allowlisted shell present on this machine', () => {
|
||||||
|
// The whole point of the allowlist is that the flags are ACCEPTED, so prove it
|
||||||
|
// against the real binaries rather than trusting the set.
|
||||||
|
for (const shell of ['/bin/sh', '/bin/bash', '/bin/dash', '/usr/bin/zsh', '/bin/ksh']) {
|
||||||
|
let exists = true;
|
||||||
|
try {
|
||||||
|
execFileSync('/bin/sh', ['-c', `test -x ${shell}`]);
|
||||||
|
} catch {
|
||||||
|
exists = false;
|
||||||
|
}
|
||||||
|
if (!exists) continue;
|
||||||
|
const out = execFileSync('/bin/sh', ['-c', `${shell} -i -l -c 'echo ok' 2>/dev/null`], { encoding: 'utf8' });
|
||||||
|
expect(out).toContain('ok');
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('shell-mode spawn command (issue #208)', () => {
|
describe('shell-mode spawn command (issue #208)', () => {
|
||||||
const originalShell = process.env.SHELL;
|
const originalShell = process.env.SHELL;
|
||||||
|
|
||||||
@@ -88,10 +122,12 @@ describe('shell-mode spawn command (issue #208)', () => {
|
|||||||
expect(cmd.trim()).not.toBe('');
|
expect(cmd.trim()).not.toBe('');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('launches an interactive login shell, so rc files (~/.zshrc, ~/.bashrc, etc.) are sourced', () => {
|
it('launches a LOGIN shell, matching what tmux does for a pane with no default-command', () => {
|
||||||
// Without -i -l, the resolved shell runs as a bare non-interactive child of
|
// A tmux pane already hands the shell a tty, so it is interactive either way
|
||||||
// the non-interactive `bash -c` that launches the pane, silently dropping
|
// (`$-` contains `i` for a bare /bin/bash in a pane, which is why ~/.bashrc has
|
||||||
// aliases, PATH additions, and tool init (zoxide, nvm, etc.).
|
// always been sourced). `-l` is the flag that changes anything: it is what
|
||||||
|
// picks up /etc/profile and /etc/profile.d/*, which a systemd --user service
|
||||||
|
// never sourced, so its minimal PATH is what every pane used to inherit.
|
||||||
const cmd = buildSpawnCommand({ mode: 'shell', sessionId: 'abc123de-0000-0000-0000-000000000000' });
|
const cmd = buildSpawnCommand({ mode: 'shell', sessionId: 'abc123de-0000-0000-0000-000000000000' });
|
||||||
expect(cmd.trim().endsWith('-i -l')).toBe(true);
|
expect(cmd.trim().endsWith('-i -l')).toBe(true);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -146,8 +146,16 @@ describe('TmuxManager (unit)', () => {
|
|||||||
sessionId: 'abc123def456',
|
sessionId: 'abc123def456',
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(command).toContain('exec $SHELL -i -l');
|
expect(command).toContain('exec "${SHELL:-/bin/sh}" -i -l');
|
||||||
expect(command).toContain('remain-on-exit on');
|
// `failed`, not `on`: `on` also keeps the pane after a CLEAN exit, so typing
|
||||||
|
// `exit` in a remote shell strands a dead pane that the next launch's `-A`
|
||||||
|
// reattaches to instead of starting a shell.
|
||||||
|
expect(command).toContain('remain-on-exit failed');
|
||||||
|
expect(command).not.toContain('remain-on-exit on');
|
||||||
|
// Last in the chain: tmux aborts the rest of a `\;` sequence after an error,
|
||||||
|
// and `failed` needs tmux >= 3.2 on the REMOTE host. Trailing, a rejection
|
||||||
|
// costs only this option instead of every setting after it.
|
||||||
|
expect(command.trimEnd().endsWith("remain-on-exit failed'")).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('defaults claude to a non-interactive launch (--dangerously-skip-permissions)', () => {
|
it('defaults claude to a non-interactive launch (--dangerously-skip-permissions)', () => {
|
||||||
@@ -161,7 +169,7 @@ describe('TmuxManager (unit)', () => {
|
|||||||
// interactive nor login, so a bare `exec claude` fails with "command not found".
|
// interactive nor login, so a bare `exec claude` fails with "command not found".
|
||||||
// The inner quoting is escaped twice over (once per shellescape() layer), so
|
// The inner quoting is escaped twice over (once per shellescape() layer), so
|
||||||
// assert on the unescaped substrings rather than the literal quoted form.
|
// assert on the unescaped substrings rather than the literal quoted form.
|
||||||
expect(command).toContain('exec $SHELL -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');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user