fix(server): report a terminal whose command cannot start instead of leaving a dead tab

Opening a terminal profile whose command is not installed left a tab that could
never be closed. Two problems sat on the same path.

A terminal that dies during creation was resurrected by its own create. The
worker sends terminalCreated, the createTerminal response and terminalExit in
the same tick, so the parent drains all three before the awaiting create
continuation resumes; that continuation called registerRecord a second time and
put back a terminal the exit had just removed. Removal is owned solely by the
terminalExit event and that event never comes twice, so the record outlived
every close attempt: getTerminals kept listing it, capture returned nothing,
kill was a silent no-op that still reported success, and only a daemon restart
cleared it.

The parent now tracks the ids with a create in flight and keeps the finished
record when an exit removes one of them, so the create continuation returns the
exited session instead of registering a live one. Removal still happens exactly
once, in the terminalExit path.

With the stuck tab gone, a missing command produced nothing at all: no tab and
no message. node-pty spawns directly through execve on POSIX, with no shell, so
a command that does not exist fails inside the spawn helper and surfaces as exit
code 1 with no output -- exactly what `false` produces, so the exit carries no
usable signal. createTerminal now resolves the command before spawning and fails
the create when it cannot be found, so the daemon answers create_terminal with a
null terminal and "<command>: command not found", which the app already renders.
A command that starts and then exits non-zero still opens its tab.

That resolution has to match what execve will do in the child, or a working
profile is refused: it uses the terminal's own environment, built first so the
check sees the per-cwd registered PATH and the entries buildTerminalEnvironment
prepends, and it resolves a command containing a separator against the
terminal's cwd rather than through PATH. Windows keeps resolving inside
resolveTerminalSpawnCommand, which has its own shim handling and fallback.
This commit is contained in:
KiraPC
2026-08-24 16:58:06 +02:00
parent 7db654aa0a
commit a92dc56639
5 changed files with 412 additions and 14 deletions
@@ -6,7 +6,7 @@ import { windowsExecutableResolution } from "./windows.js";
export { quoteWindowsArgument, quoteWindowsCommand } from "../utils/windows-command.js";
type Which = (command: string, options: { all: true }) => Promise<string[]>;
type Which = (command: string, options: { all: true; path?: string }) => Promise<string[]>;
const require = createRequire(import.meta.url);
const which = require("which") as Which;
@@ -35,10 +35,10 @@ async function enumerateCandidatesViaSystemWhich(name: string): Promise<string[]
}
}
async function enumerateCandidatesViaLibrary(name: string): Promise<string[]> {
async function enumerateCandidatesViaLibrary(name: string, path?: string): Promise<string[]> {
let candidates: string[];
try {
candidates = await which(name, { all: true });
candidates = await which(name, { all: true, ...(path === undefined ? {} : { path }) });
} catch (error) {
// `which` throws ENOENT when the command is absent from PATH.
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
@@ -108,6 +108,23 @@ export function executableExists(
return exists(executablePath) ? executablePath : null;
}
// PATH lookup that never runs the candidate, unlike findExecutable which probes
// each one with `--version`. A terminal spawn path cannot afford that probe: it
// would execute the profile command once before the terminal even opens.
// `path` overrides the PATH to search, so a caller can resolve against the
// environment its child process will actually get rather than the daemon's own.
export async function findExecutableWithoutProbing(
name: string,
options: { path?: string } = {},
): Promise<string | null> {
const trimmed = name.trim();
if (!trimmed) {
return null;
}
const candidates = await enumerateCandidatesViaLibrary(trimmed, options.path);
return candidates[0] ?? null;
}
export async function findExecutable(
name: string,
probeTimeoutMs = PROBE_TIMEOUT_MS,
+144 -1
View File
@@ -2,6 +2,7 @@ import { describe, it, expect, afterEach, vi } from "vitest";
import { isPlatform } from "../test-utils/platform.js";
import {
buildTerminalEnvironment,
assertTerminalCommandCanStart,
createTerminal,
ensureNodePtySpawnHelperExecutableForCurrentPlatform,
resolveDefaultTerminalShell,
@@ -24,7 +25,7 @@ import {
} from "node:fs";
import { spawnSync } from "node:child_process";
import * as pty from "node-pty";
import { join } from "node:path";
import { delimiter, join } from "node:path";
import { tmpdir } from "node:os";
import { setImmediate as waitForImmediate, setTimeout as delay } from "node:timers/promises";
import { stripVTControlCharacters } from "node:util";
@@ -593,6 +594,148 @@ describe("createTerminal", () => {
expect(exitInfo.lastOutputLines.length).toBeGreaterThan(0);
expect(exitInfo.lastOutputLines[exitInfo.lastOutputLines.length - 1]).toBe("line-2999");
});
it.runIf(isPlatform("linux", "darwin"))(
"rejects a profile command that is not installed instead of opening a terminal",
async () => {
await expect(
createTerminal({
workspaceId: "ws-test",
cwd: realpathSync(tmpdir()),
name: "Codex",
command: "paseo-qa-absent-codex",
args: [],
}),
).rejects.toThrow("paseo-qa-absent-codex: command not found");
},
);
it.runIf(isPlatform("linux", "darwin"))(
"opens a command found only on the PATH carried by the env option",
async () => {
// execve resolves the command against the child's PATH, which carries the
// per-cwd registered environment. A version-manager shim lives there and
// not necessarily on the daemon's own PATH.
const binDir = mkdtempSync(join(tmpdir(), "terminal-envpath-bin-"));
temporaryDirs.push(binDir);
const toolPath = join(binDir, "paseo-qa-envpath-tool");
writeFileSync(toolPath, "#!/usr/bin/env bash\nsleep 30\n");
chmodSync(toolPath, 0o755);
const session = trackSession(
await createTerminal({
workspaceId: "ws-test",
cwd: realpathSync(tmpdir()),
name: "Shim",
command: "paseo-qa-envpath-tool",
args: [],
env: { PATH: `${binDir}${delimiter}${process.env.PATH ?? ""}` },
}),
);
expect(session.id).toBeDefined();
},
);
it.runIf(isPlatform("linux", "darwin"))(
"reports command not found when the command is on neither PATH",
async () => {
await expect(
createTerminal({
workspaceId: "ws-test",
cwd: realpathSync(tmpdir()),
name: "Codex",
command: "paseo-qa-absent-codex",
args: [],
env: { PATH: `/paseo-qa-empty${delimiter}${process.env.PATH ?? ""}` },
}),
).rejects.toThrow("paseo-qa-absent-codex: command not found");
},
);
it.runIf(isPlatform("linux", "darwin"))(
"opens a relative command path resolved against the terminal cwd",
async () => {
// execve resolves a path-ish command against the child's cwd, never the
// daemon's own working directory.
const projectDir = mkdtempSync(join(tmpdir(), "terminal-relative-cmd-"));
temporaryDirs.push(projectDir);
mkdirSync(join(projectDir, "scripts"));
const scriptPath = join(projectDir, "scripts", "dev.sh");
writeFileSync(scriptPath, "#!/usr/bin/env bash\nsleep 30\n");
chmodSync(scriptPath, 0o755);
const session = trackSession(
await createTerminal({
workspaceId: "ws-test",
cwd: realpathSync(projectDir),
name: "Dev script",
command: "./scripts/dev.sh",
args: [],
}),
);
expect(session.id).toBeDefined();
},
);
it.runIf(isPlatform("linux", "darwin"))("opens an absolute command path", async () => {
const projectDir = mkdtempSync(join(tmpdir(), "terminal-absolute-cmd-"));
temporaryDirs.push(projectDir);
const scriptPath = join(projectDir, "run.sh");
writeFileSync(scriptPath, "#!/usr/bin/env bash\nsleep 30\n");
chmodSync(scriptPath, 0o755);
const session = trackSession(
await createTerminal({
workspaceId: "ws-test",
cwd: realpathSync(tmpdir()),
name: "Absolute",
command: scriptPath,
args: [],
}),
);
expect(session.id).toBeDefined();
});
it("leaves the command check to the terminal itself on Windows", async () => {
// Windows resolves inside resolveTerminalSpawnCommand, which has its own
// shim handling and its own fallback for an unresolved command.
await expect(
assertTerminalCommandCanStart({
command: "paseo-qa-absent-codex",
cwd: realpathSync(tmpdir()),
env: { PATH: "/paseo-qa-empty" },
platform: "win32",
}),
).resolves.toBeUndefined();
});
it.runIf(isPlatform("linux", "darwin"))(
"still opens a terminal for an installed command that exits non-zero",
async () => {
// A profile running a failing test suite must keep opening a tab with its
// output. Only a command that cannot start at all is reported as an error,
// and "exited non-zero" does not distinguish the two: a missing binary and
// `false` both surface as exit code 1 from node-pty.
const session = trackSession(
await createTerminal({
workspaceId: "ws-test",
cwd: realpathSync(tmpdir()),
name: "Failing suite",
command: "false",
args: [],
}),
);
const exitInfo = await new Promise<{ exitCode: number | null }>((resolve) => {
session.onExit((info) => resolve({ exitCode: info.exitCode }));
});
expect(exitInfo).toEqual({ exitCode: 1 });
},
);
});
describe.skipIf(isPlatform("win32"))("send input", () => {
+92 -10
View File
@@ -1,14 +1,25 @@
import * as pty from "node-pty";
import xterm, { type Terminal as TerminalType } from "@xterm/headless";
import { randomUUID } from "crypto";
import { chmodSync, existsSync, mkdirSync, readFileSync, statSync } from "node:fs";
import {
accessSync,
chmodSync,
constants,
existsSync,
mkdirSync,
readFileSync,
statSync,
} from "node:fs";
import { tmpdir, userInfo } from "node:os";
import { basename, delimiter, dirname, extname, join, resolve as resolvePath } from "node:path";
import { createRequire } from "node:module";
import { fileURLToPath } from "node:url";
import { createExternalProcessEnv } from "../server/paseo-env.js";
import { writePrivateFileAtomicSync } from "../server/private-files.js";
import { findExecutable } from "../executable-resolution/executable-resolution.js";
import {
findExecutable,
findExecutableWithoutProbing,
} from "../executable-resolution/executable-resolution.js";
import type { TerminalCell, TerminalState } from "@getpaseo/protocol/messages";
import { TerminalInputModeTracker } from "@getpaseo/protocol/terminal-input-mode";
import { TerminalActivityTracker } from "./activity/terminal-activity-tracker.js";
@@ -33,6 +44,20 @@ export interface TerminalExitInfo {
lastOutputLines: string[];
}
// The field is assigned explicitly rather than declared as a constructor
// parameter property: the terminal worker loads this module under
// --experimental-strip-types, which erases types but cannot emit the
// assignment a parameter property implies.
export class TerminalCommandNotFoundError extends Error {
readonly command: string;
constructor(command: string) {
super(`${command}: command not found`);
this.name = "TerminalCommandNotFoundError";
this.command = command;
}
}
export interface TerminalCommandFinishedInfo {
exitCode: number | null;
}
@@ -882,6 +907,59 @@ function extractLastOutputLinesFromText(text: string, limit: number): string[] {
return lines.slice(-limit);
}
function isExecutableFile(path: string): boolean {
try {
if (!statSync(path).isFile()) {
return false;
}
accessSync(path, constants.X_OK);
return true;
} catch {
return false;
}
}
export interface AssertTerminalCommandCanStartInput {
command: string | undefined;
// The terminal's own cwd and environment, not the daemon's: execve in the
// child resolves against those, so anything else produces false negatives.
cwd: string;
env: Record<string, string>;
platform?: NodeJS.Platform;
}
// node-pty spawns directly through execve on POSIX, with no shell, so a command
// that does not exist fails inside the spawn helper and surfaces as exit code 1
// with no output -- indistinguishable from `false` or any program that simply
// failed. Resolving it up front is the only signal that separates "cannot start"
// from "ran and exited non-zero", and it lets the create report the failure
// instead of handing back a terminal that dies immediately. Windows keeps
// resolving inside resolveTerminalSpawnCommand, which has its own shim handling
// and its own fallback for an unresolved command.
export async function assertTerminalCommandCanStart(
input: AssertTerminalCommandCanStartInput,
): Promise<void> {
const { command, cwd, env } = input;
const platform = input.platform ?? process.platform;
if (command === undefined || platform === "win32") {
return;
}
// A path-ish command is resolved by execve against the child's cwd, never
// through PATH and never against the daemon's working directory.
if (command.includes("/")) {
if (isExecutableFile(resolvePath(cwd, command))) {
return;
}
throw new TerminalCommandNotFoundError(command);
}
const resolved = await findExecutableWithoutProbing(command, { path: env[getPathEnvKey(env)] });
if (!resolved) {
throw new TerminalCommandNotFoundError(command);
}
}
export async function createTerminal(options: CreateTerminalOptions): Promise<TerminalSession> {
const {
cwd,
@@ -941,19 +1019,23 @@ export async function createTerminal(options: CreateTerminalOptions): Promise<Te
const { command: spawnCommand, args: spawnArgs } = command
? await resolveTerminalSpawnCommand(command, args)
: { command: resolvedShell, args: [] as string[] };
const terminalEnv = buildTerminalEnvironment({
shell: spawnCommand,
env: {
...env,
...activityEnv,
PASEO_WORKSPACE_ID: workspaceId,
},
});
await assertTerminalCommandCanStart({ command, cwd, env: terminalEnv });
const ptyProcess = pty.spawn(spawnCommand, spawnArgs, {
name: "xterm-256color",
cols,
rows,
cwd,
env: buildTerminalEnvironment({
shell: spawnCommand,
env: {
...env,
...activityEnv,
PASEO_WORKSPACE_ID: workspaceId,
},
}),
env: terminalEnv,
});
function emitTitleChange(nextTitle: string | undefined): void {
@@ -970,3 +970,139 @@ it("removes a killed worker terminal from terminalExit without duplicate snapsho
},
]);
});
it("does not resurrect a terminal that exited before its create response was handled", async () => {
const worker = new FakeTerminalWorker();
manager = createWorkerTerminalManager({
requestTimeoutMs: 50,
forkWorker: () => worker,
});
const snapshots: Array<{ cwd: string; terminalIds: string[] }> = [];
manager.subscribeTerminalsChanged((event) => {
snapshots.push({ cwd: event.cwd, terminalIds: event.terminals.map((terminal) => terminal.id) });
});
const created = manager.createTerminal({
id: "terminal-a",
cwd: "/workspace",
workspaceId: "ws-test",
});
const request = worker.sentMessages.find((message) => message.type === "createTerminal");
if (!request) {
throw new Error("createTerminal request not sent");
}
// The worker delivers all three messages in one tick, so the awaiting
// createTerminal continuation resumes only after the exit was processed.
const terminal = {
id: "terminal-a",
name: "Shell",
cwd: "/workspace",
workspaceId: "ws-test",
activity: null,
};
const state = createTerminalState();
const exitInfo = { exitCode: 0, signal: null, lastOutputLines: [] };
worker.emitWorkerMessage({ type: "terminalCreated", terminal, state });
worker.emitWorkerMessage({
type: "response",
requestId: request.requestId,
ok: true,
result: { terminal, state },
});
worker.emitWorkerMessage({ type: "terminalExit", terminalId: "terminal-a", info: exitInfo });
const session = await created;
expect(manager.getTerminal("terminal-a")).toBeUndefined();
expect(await manager.getTerminals("/workspace")).toEqual([]);
expect(session.getExitInfo()).toEqual(exitInfo);
expect(snapshots).toEqual([
{ cwd: "/workspace", terminalIds: ["terminal-a"] },
{ cwd: "/workspace", terminalIds: [] },
]);
});
it("replays the exit to a listener attached after a create that lost the race", async () => {
const worker = new FakeTerminalWorker();
manager = createWorkerTerminalManager({
requestTimeoutMs: 50,
forkWorker: () => worker,
});
const created = manager.createTerminal({
id: "terminal-a",
cwd: "/workspace",
workspaceId: "ws-test",
});
const request = worker.sentMessages.find((message) => message.type === "createTerminal");
if (!request) {
throw new Error("createTerminal request not sent");
}
const terminal = {
id: "terminal-a",
name: "Shell",
cwd: "/workspace",
workspaceId: "ws-test",
activity: null,
};
const state = createTerminalState();
const exitInfo = { exitCode: 0, signal: null, lastOutputLines: [] };
worker.emitWorkerMessage({ type: "terminalCreated", terminal, state });
worker.emitWorkerMessage({
type: "response",
requestId: request.requestId,
ok: true,
result: { terminal, state },
});
worker.emitWorkerMessage({ type: "terminalExit", terminalId: "terminal-a", info: exitInfo });
const session = await created;
// The session controller subscribes only after createTerminal resolves; that
// subscription is what turns into a terminal_exit for the client.
const observedExits: Array<{ exitCode: number | null }> = [];
session.onExit((info) => {
observedExits.push({ exitCode: info.exitCode });
});
await new Promise((resolve) => setImmediate(resolve));
expect(observedExits).toEqual([{ exitCode: 0 }]);
});
it("rejects createTerminal and registers nothing when the worker cannot start the command", async () => {
const worker = new FakeTerminalWorker();
manager = createWorkerTerminalManager({
requestTimeoutMs: 50,
forkWorker: () => worker,
});
const snapshots: Array<{ cwd: string; terminalIds: string[] }> = [];
manager.subscribeTerminalsChanged((event) => {
snapshots.push({ cwd: event.cwd, terminalIds: event.terminals.map((terminal) => terminal.id) });
});
const created = manager.createTerminal({
id: "terminal-a",
cwd: "/workspace",
workspaceId: "ws-test",
command: "paseo-qa-absent-codex",
});
const request = worker.sentMessages.find((message) => message.type === "createTerminal");
if (!request) {
throw new Error("createTerminal request not sent");
}
worker.emitWorkerMessage({
type: "response",
requestId: request.requestId,
ok: false,
error: "paseo-qa-absent-codex: command not found",
});
await expect(created).rejects.toThrow("paseo-qa-absent-codex: command not found");
expect(manager.getTerminal("terminal-a")).toBeUndefined();
expect(await manager.getTerminals("/workspace")).toEqual([]);
expect(snapshots).toEqual([]);
});
@@ -156,6 +156,14 @@ export function createWorkerTerminalManager(
const recordsById = new Map<string, WorkerTerminalRecord>();
const terminalIdsByCwd = new Map<string, Set<string>>();
const terminalActivityTokenById = new Map<string, string>();
// A terminal can die before its createTerminal response is handled: the worker
// sends terminalCreated, the response, and terminalExit in the same tick, so
// the awaiting create continuation resumes after the exit was already
// processed. These hold the ids being created and the finished records their
// exit removed, so that continuation returns the exited session instead of
// registering a live one for a terminal the worker no longer owns.
const creatingTerminalIds = new Set<string>();
const exitedDuringCreateById = new Map<string, WorkerTerminalRecord>();
const terminalsChangedListeners = new Set<TerminalsChangedListener>();
const terminalActivityListeners = new Set<TerminalActivityListener>();
const terminalWorkspaceContributionChangedListeners =
@@ -448,6 +456,9 @@ export function createWorkerTerminalManager(
record.exitListeners.clear();
const previousBucket = deriveTerminalActivityStatusBucket(record.activity);
const removedRecord = removeRecord(message.terminalId);
if (removedRecord && creatingTerminalIds.has(message.terminalId)) {
exitedDuringCreateById.set(message.terminalId, removedRecord);
}
if (previousBucket !== null && removedRecord) {
emitTerminalWorkspaceContributionChanged({
terminalId: removedRecord.info.id,
@@ -686,6 +697,7 @@ export function createWorkerTerminalManager(
const activityToken = createActivityToken();
const terminalActivityUrl = managerOptions.getTerminalActivityUrl?.() ?? null;
terminalActivityTokenById.set(terminalId, activityToken);
creatingTerminalIds.add(terminalId);
let result: {
terminal: RequiredWorkerTerminalInfo;
state: TerminalState;
@@ -705,7 +717,15 @@ export function createWorkerTerminalManager(
};
} catch (error) {
terminalActivityTokenById.delete(terminalId);
exitedDuringCreateById.delete(terminalId);
throw error;
} finally {
creatingTerminalIds.delete(terminalId);
}
const exitedDuringCreate = exitedDuringCreateById.get(terminalId);
if (exitedDuringCreate) {
exitedDuringCreateById.delete(terminalId);
return exitedDuringCreate.session;
}
const session = registerRecord({ info: result.terminal, state: result.state });
return session;