fix(runtime-tmux): kill session if set-option fails
Move the `set-option ... status off` call inside the existing try/catch so a failure (e.g. the 5-second tmux command timeout firing on a slow host) triggers `kill-session` cleanup instead of leaving an orphaned tmux session behind. Renames the surfaced error to "Failed to configure or launch session" since the try block now covers both configuration and the launch send-keys. Adds a regression test that asserts kill-session is called when set-option throws. Addresses review feedback on #1711. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
0e8e1a9e16
commit
fc9e4bfdd8
|
|
@ -238,7 +238,7 @@ describe("runtime.create()", () => {
|
|||
launchCommand: "bad-command",
|
||||
environment: {},
|
||||
}),
|
||||
).rejects.toThrow('Failed to send launch command to session "fail-session"');
|
||||
).rejects.toThrow('Failed to configure or launch session "fail-session"');
|
||||
|
||||
// Verify kill-session was called for cleanup
|
||||
expect(mockExecFileCustom).toHaveBeenCalledWith(
|
||||
|
|
@ -248,6 +248,33 @@ describe("runtime.create()", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("cleans up session if set-option fails", async () => {
|
||||
const runtime = create();
|
||||
|
||||
// 1: new-session succeeds
|
||||
mockTmuxSuccess();
|
||||
// 2: set-option fails (e.g. tmux command timeout on a slow host)
|
||||
mockTmuxError("set-option timed out");
|
||||
// 3: kill-session (cleanup attempt)
|
||||
mockTmuxSuccess();
|
||||
|
||||
await expect(
|
||||
runtime.create({
|
||||
sessionId: "setopt-fail",
|
||||
workspacePath: "/tmp/ws",
|
||||
launchCommand: "echo hi",
|
||||
environment: {},
|
||||
}),
|
||||
).rejects.toThrow('Failed to configure or launch session "setopt-fail"');
|
||||
|
||||
// kill-session must run so we don't leave an orphaned tmux session
|
||||
expect(mockExecFileCustom).toHaveBeenCalledWith(
|
||||
"tmux",
|
||||
["kill-session", "-t", "setopt-fail"],
|
||||
expectedTmuxOptions,
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects invalid session IDs with special characters", async () => {
|
||||
const runtime = create();
|
||||
|
||||
|
|
|
|||
|
|
@ -66,11 +66,6 @@ export function create(): Runtime {
|
|||
// Create tmux session in detached mode
|
||||
await tmux("new-session", "-d", "-s", sessionName, "-c", config.workspacePath, ...envArgs);
|
||||
|
||||
// Hide the tmux status bar — sessions are embedded in the web terminal,
|
||||
// and the green bar at the bottom is visual noise (and racy with the
|
||||
// web layer's own set-option call, which only fires on WebSocket connect).
|
||||
await tmux("set-option", "-t", sessionName, "status", "off");
|
||||
|
||||
// Re-export PATH inside the launch script. macOS zsh runs path_helper
|
||||
// during shell startup which resets PATH, wiping entries set via tmux -e.
|
||||
// Including the export in the script file (not send-keys) avoids terminal
|
||||
|
|
@ -83,10 +78,16 @@ export function create(): Runtime {
|
|||
launchCommand = `export PATH=$(printf '%s' ${JSON.stringify(pathValue)})\n${launchCommand}`;
|
||||
}
|
||||
|
||||
// Send the launch command — clean up the session if this fails.
|
||||
// Use a temp script for long commands so the pane shows a short
|
||||
// Configure the session and send the launch command — kill the session
|
||||
// if any of these fail so we don't leave an orphaned tmux process.
|
||||
// Use a temp script for long launch commands so the pane shows a short
|
||||
// invocation instead of a pasted wall of shell.
|
||||
try {
|
||||
// Hide the tmux status bar — sessions are embedded in the web terminal,
|
||||
// and the green bar at the bottom is visual noise (and racy with the
|
||||
// web layer's own set-option call, which only fires on WebSocket connect).
|
||||
await tmux("set-option", "-t", sessionName, "status", "off");
|
||||
|
||||
if (launchCommand.length > 200) {
|
||||
const invocation = writeLaunchScript(launchCommand);
|
||||
await tmux("send-keys", "-t", sessionName, "-l", invocation);
|
||||
|
|
@ -102,7 +103,7 @@ export function create(): Runtime {
|
|||
// Best-effort cleanup
|
||||
}
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
throw new Error(`Failed to send launch command to session "${sessionName}": ${msg}`, {
|
||||
throw new Error(`Failed to configure or launch session "${sessionName}": ${msg}`, {
|
||||
cause: err,
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue