From 529d8fa8ea9ad99ed47aaa03335b26e8ce8d7f78 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 4 Aug 2026 23:09:00 +0200 Subject: [PATCH] chore: version packages --- CHANGELOG.md | 10 +++ CLAUDE.md | 2 +- package-lock.json | 4 +- package.json | 2 +- src/session.ts | 6 +- src/tmux-manager.ts | 9 ++- src/utils/index.ts | 1 + src/utils/shell-resolver.ts | 78 +++++++++++++++++++++++ test/shell-session-launch.test.ts | 102 ++++++++++++++++++++++++++++++ 9 files changed, 207 insertions(+), 7 deletions(-) create mode 100644 src/utils/shell-resolver.ts create mode 100644 test/shell-session-launch.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f1c521..01e8e0c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # aicodeman +## 1.9.9 + +### Patch Changes + +- Two bug fixes. + + **Plain shell sessions could not start when the server process had no `SHELL` (#208).** The tmux pane command for `mode: 'shell'` was the literal string `$SHELL`. That string is embedded in the `bash -c "..."` argument of the `respawn-pane` line, which is run through `/bin/sh -c`, so it was expanded by the _server_ process's shell against the _server_ process's environment rather than inside the pane. Containers and system-level systemd units do not set `SHELL`, so it expanded to nothing and the pane command ended in a dangling `&&`, giving `bash: -c: line 1: syntax error: unexpected end of file` and a pane that died instantly (status 2) while tmux session creation still reported success. The shell is now resolved in Node (`$SHELL`, then the passwd entry, then `/bin/bash`, `/bin/zsh`, `/bin/sh`), requiring an absolute path to an executable and skipping `nologin`-style stubs, then shell-quoted. Only local shell sessions were affected: agent CLI modes emit a real command, and Docker/remote-SSH cases already used a literal `exec bash -l`. + + **A session name typed into the tab options could be silently dropped.** Two independent paths. In the Session Options modal, the Session Name input saves on blur while every autosave handler bails on a null `editingSessionId`, and `closeSessionOptions()` cleared that id before hiding the modal (hiding is what blurs the input), so the save always ran too late; Escape and backdrop-click lost the name with no PUT at all, and only the X button worked because mousedown blurs first. The focused modal field is now blurred before the id is cleared, which also covers the auto-compact prompt. Separately, the right-click inline rename could be destroyed mid-keystroke: the `_inlineRenameActive` guard was missing from `_renderSessionTabsImmediate()`, so a render queued just before the rename opened still rewrote the tab name's innerHTML, committing a truncated name or closing the rename outright. The debounced executor is now guarded too. + ## 1.9.8 ### Patch Changes diff --git a/CLAUDE.md b/CLAUDE.md index bc5d3c6f..35c64aac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,7 +74,7 @@ When user says "COM": CI runs `npm run check:lockfile` on every push/PR, so lockfile drift fails the build even if the `version-packages` script is bypassed. -**Version**: 1.9.8 (must match `package.json`) +**Version**: 1.9.9 (must match `package.json`) ## Project Overview diff --git a/package-lock.json b/package-lock.json index 5357ac81..647845a7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "aicodeman", - "version": "1.9.8", + "version": "1.9.9", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "aicodeman", - "version": "1.9.8", + "version": "1.9.9", "hasInstallScript": true, "license": "MIT", "workspaces": [ diff --git a/package.json b/package.json index e315fd12..4344ac1a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "aicodeman", - "version": "1.9.8", + "version": "1.9.9", "description": "Mission control for AI coding agents - run 20 autonomous agents with real-time monitoring and session persistence", "type": "module", "main": "dist/index.js", diff --git a/src/session.ts b/src/session.ts index 9a5ca134..d2053104 100644 --- a/src/session.ts +++ b/src/session.ts @@ -68,6 +68,7 @@ import { getClaudeCliVersion, getClaudeBinaryPath, spawnPtyWithHelperRepair, + resolveLocalShell, } from './utils/index.js'; import { MAX_TERMINAL_BUFFER_SIZE, @@ -1898,8 +1899,9 @@ export class Session extends EventEmitter { this._resetBuffers(); - // Use user's default shell or bash - const shell = process.env.SHELL || '/bin/bash'; + // Use user's default shell, falling back to a shell that actually exists. + // Shared with the tmux pane command so both paths launch the same binary. + const shell = resolveLocalShell(); console.log( '[Session] Starting shell session with:', shell + (this._useMux ? ` (with ${this._mux!.backend})` : '') diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 2747d8f2..acfecb3a 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -71,6 +71,7 @@ import { resolveCodexDir, resolveGeminiDir, resolveAntigravityDir, + resolveLocalShell, } from './utils/index.js'; import type { TerminalMultiplexer, @@ -773,7 +774,13 @@ export function buildSpawnCommand(options: { if (options.mode === 'antigravity') { return buildAntigravityCommand(options.antigravityConfig); } - return '$SHELL'; + // #208: NOT the literal '$SHELL'. This string is embedded in the `bash -c "…"` + // argument of the respawn-pane line, which execSync runs through `/bin/sh -c`, + // so a `$SHELL` here is expanded by the SERVER process's shell against the + // SERVER process's env — empty in containers and system systemd units, leaving + // 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. + return shellescape(resolveLocalShell()); } /** diff --git a/src/utils/index.ts b/src/utils/index.ts index 160ca712..98a34dd0 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -26,6 +26,7 @@ export { isSafePushEndpoint } from './push-endpoint-validation.js'; export { stringSimilarity, fuzzyPhraseMatch, todoContentHash } from './string-similarity.js'; export { assertNever } from './type-safety.js'; export { wrapWithNice } from './nice-wrapper.js'; +export { resolveLocalShell } from './shell-resolver.js'; export { findClaudeDir, getAugmentedPath, getClaudeCliVersion, getClaudeBinaryPath } from './claude-cli-resolver.js'; export { spawnPtyWithHelperRepair } from './node-pty-repair.js'; export { resolveOpenCodeDir } from './opencode-cli-resolver.js'; diff --git a/src/utils/shell-resolver.ts b/src/utils/shell-resolver.ts new file mode 100644 index 00000000..8d20fde6 --- /dev/null +++ b/src/utils/shell-resolver.ts @@ -0,0 +1,78 @@ +/** + * @fileoverview Resolve a real, launchable login shell for `mode: 'shell'` sessions. + * + * The tmux pane command for a local shell session used to be the literal string + * `$SHELL`. That string is embedded in the `bash -c "…"` argument of the + * `respawn-pane` line, which `execSync` hands to `/bin/sh -c` — so `$SHELL` was + * expanded by the SERVER process's shell (not the pane's), against the SERVER + * process's env. Containers and system-level systemd units do not set `SHELL`, + * so the expansion produced an empty string and the pane command ended in a + * dangling `&&`: + * + * bash -c "cd \"/case\" && ulimit … && export … && " + * -> bash: -c: line 1: syntax error: unexpected end of file + * + * The pane then died instantly (status 2) while tmux creation itself reported + * success, which is exactly what issue #208 saw. Resolving the shell HERE, in + * Node, removes the shell-expansion layer entirely and guarantees a non-empty + * absolute path. + * + * @module utils/shell-resolver + */ + +import { accessSync, constants } from 'node:fs'; +import { userInfo } from 'node:os'; + +/** Last-resort shells, in preference order. `/bin/sh` exists on every POSIX host. */ +const FALLBACK_SHELLS = ['/bin/bash', '/bin/zsh', '/bin/sh']; + +/** + * Shells that exist and are executable but immediately exit — a service account's + * passwd entry commonly points at one, which would look identical to the crash + * this module exists to prevent. + */ +const NON_INTERACTIVE_SHELLS = new Set(['nologin', 'false', 'true', 'sync']); + +function isUsableShell(candidate: string): boolean { + if (!candidate.startsWith('/')) return false; + const base = candidate.slice(candidate.lastIndexOf('/') + 1); + if (NON_INTERACTIVE_SHELLS.has(base)) return false; + try { + accessSync(candidate, constants.X_OK); + return true; + } catch { + return false; + } +} + +/** + * Resolve an absolute path to an interactive shell, preferring the user's own. + * + * Order: `$SHELL` -> the passwd entry -> `/bin/bash` -> `/bin/zsh` -> `/bin/sh`. + * Every candidate must be an absolute path to an executable that is not a + * nologin-style stub. Always returns a non-empty string. + */ +export function resolveLocalShell(): string { + const candidates: string[] = []; + + const envShell = process.env.SHELL?.trim(); + if (envShell) candidates.push(envShell); + + try { + // Throws when the uid has no /etc/passwd entry (common for `--user` containers). + const passwdShell = userInfo().shell?.trim(); + if (passwdShell) candidates.push(passwdShell); + } catch { + /* no passwd entry — fall through to the static fallbacks */ + } + + candidates.push(...FALLBACK_SHELLS); + + for (const candidate of candidates) { + if (isUsableShell(candidate)) return candidate; + } + + // Nothing was verifiable (exotic/read-restricted image). /bin/sh is still the + // best guess and is far better than emitting an empty command. + return '/bin/sh'; +} diff --git a/test/shell-session-launch.test.ts b/test/shell-session-launch.test.ts new file mode 100644 index 00000000..e3a1ba42 --- /dev/null +++ b/test/shell-session-launch.test.ts @@ -0,0 +1,102 @@ +/** + * Regression tests for issue #208 — "Plain shell PTY exits with code 1 after + * successful tmux creation in Docker". + * + * The shell-mode pane command used to be the literal string `$SHELL`. It ends up + * inside the `bash -c "…"` argument of the respawn-pane line, which execSync runs + * through `/bin/sh -c`, so it was expanded by the SERVER process's shell against + * the SERVER process's env. Containers (and system-level systemd units) do not set + * SHELL, so it expanded to nothing and the pane command ended in a dangling `&&`: + * + * bash: -c: line 1: syntax error: unexpected end of file + * + * These tests pin the resolver's guarantees and assert that a shell launch command + * survives the outer `sh -c` layer with an unset SHELL. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { buildSpawnCommand } from '../src/tmux-manager.js'; +import { resolveLocalShell } from '../src/utils/shell-resolver.js'; + +describe('resolveLocalShell', () => { + const originalShell = process.env.SHELL; + + afterEach(() => { + if (originalShell === undefined) delete process.env.SHELL; + else process.env.SHELL = originalShell; + }); + + it('returns an absolute executable path when SHELL is unset (container case)', () => { + delete process.env.SHELL; + const shell = resolveLocalShell(); + expect(shell).not.toBe(''); + expect(shell.startsWith('/')).toBe(true); + // Proves the resolved path is really launchable, not just a plausible string. + expect(execFileSync(shell, ['-c', 'echo ok'], { encoding: 'utf8' }).trim()).toBe('ok'); + }); + + it('returns an absolute executable path when SHELL is empty or whitespace', () => { + for (const value of ['', ' ']) { + process.env.SHELL = value; + const shell = resolveLocalShell(); + expect(shell.startsWith('/')).toBe(true); + expect(execFileSync(shell, ['-c', 'echo ok'], { encoding: 'utf8' }).trim()).toBe('ok'); + } + }); + + it('honors a valid $SHELL', () => { + process.env.SHELL = '/bin/sh'; + expect(resolveLocalShell()).toBe('/bin/sh'); + }); + + it('ignores a $SHELL that does not exist', () => { + process.env.SHELL = '/nonexistent/shell-that-is-not-here'; + const shell = resolveLocalShell(); + expect(shell).not.toBe('/nonexistent/shell-that-is-not-here'); + expect(shell.startsWith('/')).toBe(true); + }); + + it('ignores a relative $SHELL (never emits a bare word into the launch command)', () => { + process.env.SHELL = 'bash'; + expect(resolveLocalShell().startsWith('/')).toBe(true); + }); + + it('ignores nologin-style stubs that would exit instantly', () => { + process.env.SHELL = '/usr/sbin/nologin'; + expect(resolveLocalShell()).not.toContain('nologin'); + process.env.SHELL = '/bin/false'; + expect(resolveLocalShell()).not.toBe('/bin/false'); + }); +}); + +describe('shell-mode spawn command (issue #208)', () => { + const originalShell = process.env.SHELL; + + beforeEach(() => { + delete process.env.SHELL; + }); + + afterEach(() => { + if (originalShell === undefined) delete process.env.SHELL; + else process.env.SHELL = originalShell; + }); + + it('never emits an unexpanded $SHELL into the pane command', () => { + const cmd = buildSpawnCommand({ mode: 'shell', sessionId: 'abc123de-0000-0000-0000-000000000000' }); + expect(cmd).not.toContain('$SHELL'); + expect(cmd.trim()).not.toBe(''); + }); + + it('produces a launch command that parses after the outer sh -c expansion layer', () => { + const cmd = buildSpawnCommand({ mode: 'shell', sessionId: 'abc123de-0000-0000-0000-000000000000' }); + // Mirrors tmux-manager: `… bash -c ${JSON.stringify(launchCmd)}` handed to `sh -c`. + const launchCmd = `cd ${JSON.stringify('/tmp')} && export CODEMAN_MUX=1 && ${cmd}`; + const outer = `bash -n -c ${JSON.stringify(launchCmd)}`; + + // `bash -n` parses without executing: exits 0 on the fix, 2 with the dangling `&&`. + const result = execFileSync('/bin/sh', ['-c', `${outer}; echo "rc=$?"`], { encoding: 'utf8' }); + expect(result).toContain('rc=0'); + expect(result).not.toContain('unexpected end of file'); + }); +});