diff --git a/src/commands/coordinator.ts b/src/commands/coordinator.ts index bb91435c..534bc310 100644 --- a/src/commands/coordinator.ts +++ b/src/commands/coordinator.ts @@ -486,11 +486,18 @@ export async function startCoordinatorSession( ...(profileFlag ? { OVERSTORY_PROFILE: profileFlag } : {}), }, }); - const pid = await tmux.createSession(tmuxSession, projectRoot, spawnCmd, { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: coordinatorName, - ...(profileFlag ? { OVERSTORY_PROFILE: profileFlag } : {}), - }); + const pid = await tmux.createSession( + tmuxSession, + projectRoot, + spawnCmd, + { + ...runtime.buildEnv(resolvedModel), + OVERSTORY_AGENT_NAME: coordinatorName, + ...(profileFlag ? { OVERSTORY_PROFILE: profileFlag } : {}), + }, + undefined, + config.runtime?.bashWrapper ?? "auto", + ); // Create a run for this coordinator session BEFORE recording the session, // so the session can reference the run ID from the start. diff --git a/src/commands/monitor.ts b/src/commands/monitor.ts index 37a74fb4..e66ab2e8 100644 --- a/src/commands/monitor.ts +++ b/src/commands/monitor.ts @@ -165,10 +165,17 @@ async function startMonitor(opts: { json: boolean; attach: boolean }): Promiseworking. diff --git a/src/commands/sling.ts b/src/commands/sling.ts index 152eb6f9..eff3fceb 100644 --- a/src/commands/sling.ts +++ b/src/commands/sling.ts @@ -1017,12 +1017,19 @@ export async function slingCommand(taskId: string, opts: SlingOptions): Promise< OVERSTORY_TASK_ID: taskId, }, }); - const pid = await createSession(tmuxSessionName, worktreePath, spawnCmd, { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: name, - OVERSTORY_WORKTREE_PATH: worktreePath, - OVERSTORY_TASK_ID: taskId, - }); + const pid = await createSession( + tmuxSessionName, + worktreePath, + spawnCmd, + { + ...runtime.buildEnv(resolvedModel), + OVERSTORY_AGENT_NAME: name, + OVERSTORY_WORKTREE_PATH: worktreePath, + OVERSTORY_TASK_ID: taskId, + }, + undefined, + config.runtime?.bashWrapper ?? "auto", + ); // 13. Record session BEFORE sending the beacon so that hook-triggered // updateLastActivity() can find the entry and transition booting->working. diff --git a/src/commands/supervisor.ts b/src/commands/supervisor.ts index b1a6d7c5..dc380672 100644 --- a/src/commands/supervisor.ts +++ b/src/commands/supervisor.ts @@ -188,11 +188,18 @@ async function startSupervisor(opts: { OVERSTORY_TASK_ID: opts.task, }, }); - const pid = await createSession(tmuxSession, projectRoot, spawnCmd, { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: opts.name, - OVERSTORY_TASK_ID: opts.task, - }); + const pid = await createSession( + tmuxSession, + projectRoot, + spawnCmd, + { + ...runtime.buildEnv(resolvedModel), + OVERSTORY_AGENT_NAME: opts.name, + OVERSTORY_TASK_ID: opts.task, + }, + undefined, + config.runtime?.bashWrapper ?? "auto", + ); // Wait for Claude Code TUI to render before sending input await waitForTuiReady(tmuxSession, (content) => runtime.detectReady(content)); diff --git a/src/config.ts b/src/config.ts index 6c97008b..9645299c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -759,6 +759,17 @@ function validateConfig(config: OverstoryConfig): void { } } + // runtime.bashWrapper: validate if present + if (config.runtime?.bashWrapper !== undefined) { + const valid = ["auto", "always", "never"]; + if (!valid.includes(config.runtime.bashWrapper)) { + process.stderr.write( + `[overstory] WARNING: runtime.bashWrapper must be one of ${valid.join(", ")}. Got: "${config.runtime.bashWrapper}". Using default ("auto").\n`, + ); + config.runtime.bashWrapper = "auto"; + } + } + // runtime.shellInitDelayMs: validate if present if (config.runtime?.shellInitDelayMs !== undefined) { const delay = config.runtime.shellInitDelayMs; diff --git a/src/runtimes/pi.test.ts b/src/runtimes/pi.test.ts index 09f2e7c6..def721b5 100644 --- a/src/runtimes/pi.test.ts +++ b/src/runtimes/pi.test.ts @@ -265,9 +265,9 @@ describe("PiRuntime", () => { expect(state).toEqual({ phase: "loading" }); }); - test("returns loading when only status bar present (no header)", () => { + test("returns ready when only status bar present (Pi >=0.55 no longer shows header)", () => { const state = runtime.detectReady("0.0%/200k (auto) (anthropic) claude-opus-4-6"); - expect(state).toEqual({ phase: "loading" }); + expect(state).toEqual({ phase: "ready" }); }); test("returns ready for real Pi TUI pane content", () => { @@ -327,9 +327,9 @@ describe("PiRuntime", () => { expect(state).toEqual({ phase: "ready" }); }); - test("returns loading when only 1.0M status bar present (no header)", () => { + test("returns ready when only 1.0M status bar present (Pi >=0.55 no header)", () => { const state = runtime.detectReady("0.0%/1.0M (auto) (anthropic) claude-opus-4-6"); - expect(state).toEqual({ phase: "loading" }); + expect(state).toEqual({ phase: "ready" }); }); test("returns ready for 2.0M context window", () => { diff --git a/src/runtimes/pi.ts b/src/runtimes/pi.ts index 4ddc04e2..42f1950c 100644 --- a/src/runtimes/pi.ts +++ b/src/runtimes/pi.ts @@ -150,8 +150,8 @@ export class PiRuntime implements AgentRuntime { * Claude Code's TUI sometimes swallows Enter during late initialization, so the * orchestrator resends the beacon until the pane leaves the "idle" state. Pi's TUI * does not have this issue AND its idle vs. processing states are indistinguishable - * via detectReady (the header "pi v..." and status bar token counter are visible in - * both states). Enabling the resend loop would spam Pi with duplicate beacon messages. + * via detectReady (the status bar token counter is visible in both states). + * Enabling the resend loop would spam Pi with duplicate beacon messages. */ requiresBeaconVerification(): boolean { return false; @@ -167,13 +167,13 @@ export class PiRuntime implements AgentRuntime { * @returns Current readiness phase */ detectReady(paneContent: string): ReadyState { - // Pi's TUI shows "pi v" in the header and a status bar with - // a token usage indicator like "0.0%/200k" or "0.0%/1.0M" when fully rendered. - // The context window size uses k-scale (e.g. 200k) for smaller models and - // M-scale (e.g. 1.0M) for Opus/Sonnet with 1M+ context windows. - const hasHeader = paneContent.includes("pi v"); + // Pi's TUI shows a status bar with a token usage indicator like + // "0.0%/200k" or "0.0%/1.0M" when fully rendered. Older Pi versions + // also showed a "pi v" header, but newer versions (>=0.55) + // omit it. The status bar alone is a reliable readiness signal. + // The context window uses k-scale (200k) or M-scale (1.0M). const hasStatusBar = /\d+\.\d+%\/[\d.]+[kKmM]/.test(paneContent); - if (hasHeader && hasStatusBar) { + if (hasStatusBar) { return { phase: "ready" }; } return { phase: "loading" }; diff --git a/src/types.ts b/src/types.ts index 9f3e7eaa..076dae8a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -134,6 +134,16 @@ export interface OverstoryConfig { printCommand?: string; /** Pi runtime configuration for model alias expansion. */ pi?: PiRuntimeConfig; + /** + * Controls whether tmux sessions wrap the startup command in `/bin/bash -c '...'`. + * + * - "auto" (default): wrap only when the user's $SHELL is non-POSIX (e.g. fish). + * Bash/zsh/sh handle inline exports natively and wrapping can break commands + * with complex quoting (e.g. Pi runtime's $(cat '...') subshells). + * - "always": always wrap (legacy behavior, needed for fish shell users). + * - "never": never wrap (assume POSIX-compatible shell). + */ + bashWrapper?: "auto" | "always" | "never"; /** * Delay in milliseconds between creating a tmux session and polling * for TUI readiness. Gives slow shells (oh-my-zsh, starship, etc.) diff --git a/src/worktree/tmux.test.ts b/src/worktree/tmux.test.ts index 98f7a9e8..56833a81 100644 --- a/src/worktree/tmux.test.ts +++ b/src/worktree/tmux.test.ts @@ -82,7 +82,9 @@ describe("createSession", () => { expect(pid).toBe(42); }); - test("passes correct args to tmux new-session with PATH wrapping", async () => { + test("passes correct args to tmux new-session with startup script", async () => { + const originalShell = process.env.SHELL; + process.env.SHELL = "/bin/zsh"; let callCount = 0; spawnSpy.mockImplementation(() => { callCount++; @@ -96,7 +98,11 @@ describe("createSession", () => { return mockSpawnResult("1234\n", "", 0); }); - await createSession("my-session", "/work/dir", "echo hello"); + try { + await createSession("my-session", "/work/dir", "echo hello"); + } finally { + process.env.SHELL = originalShell; + } // Call 0 is 'which overstory', call 1 is 'tmux new-session' const tmuxCallArgs = spawnSpy.mock.calls[1] as unknown[]; @@ -107,10 +113,10 @@ describe("createSession", () => { expect(cmd[6]).toBe("my-session"); expect(cmd[7]).toBe("-c"); expect(cmd[8]).toBe("/work/dir"); - // The command should be wrapped with PATH export const wrappedCmd = cmd[9] as string; expect(wrappedCmd).toContain("echo hello"); expect(wrappedCmd).toContain("export PATH="); + expect(wrappedCmd).not.toContain("/bin/bash -c"); const opts = tmuxCallArgs[1] as { cwd: string }; expect(opts.cwd).toBe("/work/dir"); @@ -148,6 +154,78 @@ describe("createSession", () => { ]); }); + test("auto mode wraps startup command for fish shells", async () => { + const originalShell = process.env.SHELL; + process.env.SHELL = "/opt/homebrew/bin/fish"; + let callCount = 0; + spawnSpy.mockImplementation(() => { + callCount++; + if (callCount === 1) return mockSpawnResult("/usr/local/bin/overstory\n", "", 0); + if (callCount === 2) return mockSpawnResult("", "", 0); + return mockSpawnResult("7777\n", "", 0); + }); + + try { + await createSession("fish-agent", "/tmp", "echo hello", { TEST: "1" }); + } finally { + process.env.SHELL = originalShell; + } + + const tmuxCallArgs = spawnSpy.mock.calls[1] as unknown[]; + const cmd = tmuxCallArgs[0] as string[]; + const startupCmd = cmd[9] as string; + expect(startupCmd).toContain("/bin/bash -c"); + expect(startupCmd).toContain("export TEST=\"1\""); + }); + + test("always mode wraps startup command even for POSIX shells", async () => { + const originalShell = process.env.SHELL; + process.env.SHELL = "/bin/zsh"; + let callCount = 0; + spawnSpy.mockImplementation(() => { + callCount++; + if (callCount === 1) return mockSpawnResult("/usr/local/bin/overstory\n", "", 0); + if (callCount === 2) return mockSpawnResult("", "", 0); + return mockSpawnResult("7777\n", "", 0); + }); + + try { + await createSession("zsh-agent", "/tmp", "echo hello", { TEST: "1" }, undefined, "always"); + } finally { + process.env.SHELL = originalShell; + } + + const tmuxCallArgs = spawnSpy.mock.calls[1] as unknown[]; + const cmd = tmuxCallArgs[0] as string[]; + const startupCmd = cmd[9] as string; + expect(startupCmd).toContain("/bin/bash -c"); + expect(startupCmd).toContain("export TEST=\"1\""); + }); + + test("never mode skips bash wrapping even for fish shells", async () => { + const originalShell = process.env.SHELL; + process.env.SHELL = "/opt/homebrew/bin/fish"; + let callCount = 0; + spawnSpy.mockImplementation(() => { + callCount++; + if (callCount === 1) return mockSpawnResult("/usr/local/bin/overstory\n", "", 0); + if (callCount === 2) return mockSpawnResult("", "", 0); + return mockSpawnResult("7777\n", "", 0); + }); + + try { + await createSession("fish-agent", "/tmp", "echo hello", { TEST: "1" }, undefined, "never"); + } finally { + process.env.SHELL = originalShell; + } + + const tmuxCallArgs = spawnSpy.mock.calls[1] as unknown[]; + const cmd = tmuxCallArgs[0] as string[]; + const startupCmd = cmd[9] as string; + expect(startupCmd).not.toContain("/bin/bash -c"); + expect(startupCmd).toContain("export TEST=\"1\""); + }); + test("throws AgentError if session creation fails", async () => { let callCount = 0; spawnSpy.mockImplementation(() => { diff --git a/src/worktree/tmux.ts b/src/worktree/tmux.ts index 696b1c96..55a4a1c9 100644 --- a/src/worktree/tmux.ts +++ b/src/worktree/tmux.ts @@ -106,6 +106,7 @@ export async function createSession( command: string, env?: Record, maxRetries = 3, + bashWrapper: "auto" | "always" | "never" = "auto", ): Promise { // Build environment exports for the tmux session const exports: string[] = []; @@ -129,16 +130,36 @@ export async function createSession( } } - // Build the startup script using bash syntax (export/unset). - // Then wrap it in `/bin/bash -c '...'` so it always runs in bash, - // regardless of the user's $SHELL. Without this, tmux uses the user's - // default shell (e.g. fish), which rejects bash export/unset syntax and - // causes the session to die instantly. Single-quote wrapping with escaped - // single quotes prevents any intermediate shell from expanding variables - // before bash receives them. (GitHub #86) + // Build the startup script with inline exports prepended to the command. + // + // Bash wrapping (GitHub #86): fish shell rejects bash export/unset syntax, + // so we optionally wrap in `/bin/bash -c '...'`. However, the single-quote + // escaping breaks commands with complex quoting — notably Pi runtime's + // `$(cat '...')` subshells — so we only wrap when actually needed. + // + // bashWrapper modes: + // "auto" — detect $SHELL; wrap only for non-POSIX shells (fish) + // "always" — always wrap (legacy behavior) + // "never" — never wrap (assume POSIX-compatible shell) const startupScript = exports.length > 0 ? `${exports.join(" && ")} && ${command}` : command; - const wrappedCommand = - exports.length > 0 ? `/bin/bash -c '${startupScript.replace(/'/g, "'\\''")}'` : command; + + let needsWrapper = false; + if (exports.length > 0) { + if (bashWrapper === "always") { + needsWrapper = true; + } else if (bashWrapper === "auto") { + // Detect fish shell by checking for "/fish" in $SHELL path. + // Using "/fish" (with slash) avoids false positives on hypothetical + // shells with "fish" elsewhere in the name. + const shell = process.env.SHELL ?? ""; + needsWrapper = shell.includes("/fish"); + } + // "never" → needsWrapper stays false + } + + const wrappedCommand = needsWrapper + ? `/bin/bash -c '${startupScript.replace(/'/g, "'\\''")}'` + : startupScript; const { exitCode, stderr } = await runCommand( tmuxCmd("new-session", "-d", "-s", name, "-c", cwd, wrappedCommand),