mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-27 19:52:49 +00:00
Report failed terminal launches and remove early exits (#6792)
* 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. * fix(terminal): check exact executable names and verify launch output --------- Co-authored-by: KiraPC <pasqualecarmine.carbone@gmail.com> Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
import { createRequire } from "node:module";
|
||||
import { existsSync } from "node:fs";
|
||||
import { delimiter, resolve } from "node:path";
|
||||
import type { ProcessEnvRecord } from "../server/paseo-env.js";
|
||||
import { execCommand } from "../utils/spawn.js";
|
||||
import { windowsExecutableResolution } from "./windows.js";
|
||||
@@ -120,6 +121,29 @@ 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; cwd: string },
|
||||
): Promise<string | null> {
|
||||
if (!name) {
|
||||
return null;
|
||||
}
|
||||
const path =
|
||||
options.path === undefined
|
||||
? undefined
|
||||
: options.path
|
||||
.split(delimiter)
|
||||
.map((entry) => resolve(options.cwd, entry))
|
||||
.join(delimiter);
|
||||
const candidates = await enumerateCandidatesViaLibrary(name, { PATH: path });
|
||||
return candidates[0] ?? null;
|
||||
}
|
||||
|
||||
export interface ExecutableResolutionOptions {
|
||||
probeTimeoutMs?: number;
|
||||
env?: ProcessEnvRecord;
|
||||
|
||||
@@ -244,3 +244,28 @@ async function observeScriptLifecycles(
|
||||
await subscription.ready;
|
||||
return lifecycles;
|
||||
}
|
||||
|
||||
test.runIf(process.platform !== "win32")(
|
||||
"a missing command fails creation without leaving a terminal listed",
|
||||
async () => {
|
||||
const workspaceId = await createWorkspace("Missing command");
|
||||
await expect(
|
||||
sdk.terminals.create({
|
||||
workspaceId,
|
||||
name: "Missing command",
|
||||
command: "paseo-test-absent-terminal-command",
|
||||
}),
|
||||
).rejects.toThrow("paseo-test-absent-terminal-command: command not found");
|
||||
expect((await sdk.terminals.list({ workspaceId })).entries).toEqual([]);
|
||||
const control = await sdk.terminals.create({
|
||||
workspaceId,
|
||||
command: process.execPath,
|
||||
args: ["-e", "process.stdin.resume()"],
|
||||
});
|
||||
expect((await sdk.terminals.list({ workspaceId })).entries.map((entry) => entry.id)).toEqual([
|
||||
control.id,
|
||||
]);
|
||||
await control.kill();
|
||||
expect((await sdk.terminals.list({ workspaceId })).entries).toEqual([]);
|
||||
},
|
||||
);
|
||||
|
||||
@@ -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,
|
||||
@@ -26,7 +27,7 @@ import { spawnSync } from "node:child_process";
|
||||
import * as pty from "node-pty";
|
||||
import xterm from "@xterm/headless";
|
||||
import { renderTerminalSnapshotToAnsi } from "@getpaseo/protocol/terminal-snapshot";
|
||||
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";
|
||||
@@ -637,6 +638,151 @@ 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\nprintf 'paseo-launch-ok\\n'\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 ?? ""}` },
|
||||
}),
|
||||
);
|
||||
|
||||
await waitForLines(session, ["paseo-launch-ok"]);
|
||||
expect(getLines(session.getState())[0]).toBe("paseo-launch-ok");
|
||||
},
|
||||
);
|
||||
|
||||
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\nprintf 'paseo-launch-ok\\n'\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: [],
|
||||
}),
|
||||
);
|
||||
|
||||
await waitForLines(session, ["paseo-launch-ok"]);
|
||||
expect(getLines(session.getState())[0]).toBe("paseo-launch-ok");
|
||||
},
|
||||
);
|
||||
|
||||
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\nprintf 'paseo-launch-ok\\n'\nsleep 30\n");
|
||||
chmodSync(scriptPath, 0o755);
|
||||
|
||||
const session = trackSession(
|
||||
await createTerminal({
|
||||
workspaceId: "ws-test",
|
||||
cwd: realpathSync(tmpdir()),
|
||||
name: "Absolute",
|
||||
command: scriptPath,
|
||||
args: [],
|
||||
}),
|
||||
);
|
||||
|
||||
await waitForLines(session, ["paseo-launch-ok"]);
|
||||
expect(getLines(session.getState())[0]).toBe("paseo-launch-ok");
|
||||
});
|
||||
|
||||
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", () => {
|
||||
@@ -1388,3 +1534,43 @@ describe.runIf(isPlatform("win32"))(".cmd shim argv round-trip on Windows", () =
|
||||
expect(await roundTrip("first\nsecond")).toEqual(["first second"]);
|
||||
});
|
||||
});
|
||||
|
||||
it.runIf(isPlatform("linux", "darwin"))(
|
||||
"opens a profile command from a PATH entry relative to the terminal cwd",
|
||||
async () => {
|
||||
const projectDir = mkdtempSync(join(tmpdir(), "terminal-relative-path-"));
|
||||
mkdirSync(join(projectDir, "bin"));
|
||||
const script = join(projectDir, "bin", "paseo-relative-path-tool");
|
||||
writeFileSync(script, "#!/bin/sh\nprintf 'paseo-launch-ok\\n'\nexec /bin/sleep 30\n");
|
||||
chmodSync(script, 0o755);
|
||||
let session: TerminalSession | undefined;
|
||||
try {
|
||||
session = await createTerminal({
|
||||
workspaceId: "ws-test",
|
||||
cwd: realpathSync(projectDir),
|
||||
command: "paseo-relative-path-tool",
|
||||
env: { PATH: "bin" },
|
||||
});
|
||||
await waitForLines(session, ["paseo-launch-ok"]);
|
||||
expect(getLines(session.getState())[0]).toBe("paseo-launch-ok");
|
||||
expect(session.cwd).toBe(realpathSync(projectDir));
|
||||
expect(session.getExitInfo()).toBeNull();
|
||||
} finally {
|
||||
await session?.killAndWait({ gracefulTimeoutMs: 1000, forceTimeoutMs: 500 });
|
||||
rmSync(projectDir, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(isPlatform("linux", "darwin"))(
|
||||
"checks the exact command rather than accepting a trimmed executable name",
|
||||
async () => {
|
||||
await expect(
|
||||
createTerminal({
|
||||
workspaceId: "ws-test",
|
||||
cwd: realpathSync(tmpdir()),
|
||||
command: " node ",
|
||||
}),
|
||||
).rejects.toThrow(" node : command not found");
|
||||
},
|
||||
);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
@@ -886,6 +911,62 @@ 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)],
|
||||
cwd,
|
||||
});
|
||||
if (!resolved) {
|
||||
throw new TerminalCommandNotFoundError(command);
|
||||
}
|
||||
}
|
||||
|
||||
export async function createTerminal(options: CreateTerminalOptions): Promise<TerminalSession> {
|
||||
const {
|
||||
cwd,
|
||||
@@ -945,19 +1026,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 {
|
||||
|
||||
@@ -968,3 +968,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,9 @@ export function createWorkerTerminalManager(
|
||||
const recordsById = new Map<string, WorkerTerminalRecord>();
|
||||
const terminalIdsByCwd = new Map<string, Set<string>>();
|
||||
const terminalActivityTokenById = new Map<string, string>();
|
||||
// Creation owns a reference to the session independently of the live-terminal
|
||||
// index. Only terminalCreated registers it; terminalExit removes it permanently.
|
||||
const pendingCreationsById = new Map<string, { session: TerminalSession | null }>();
|
||||
const terminalsChangedListeners = new Set<TerminalsChangedListener>();
|
||||
const terminalActivityListeners = new Set<TerminalActivityListener>();
|
||||
const terminalWorkspaceContributionChangedListeners =
|
||||
@@ -542,10 +545,14 @@ export function createWorkerTerminalManager(
|
||||
function handleWorkerEvent(message: TerminalWorkerToParentMessage): void {
|
||||
switch (message.type) {
|
||||
case "terminalCreated": {
|
||||
registerRecord({
|
||||
const session = registerRecord({
|
||||
info: asRequiredWorkerTerminalInfo(message.terminal),
|
||||
state: message.state,
|
||||
});
|
||||
const pendingCreation = pendingCreationsById.get(message.terminal.id);
|
||||
if (pendingCreation) {
|
||||
pendingCreation.session = session;
|
||||
}
|
||||
emitTerminalsChanged({
|
||||
cwd: message.terminal.cwd,
|
||||
terminals: listTerminalItemsForCwd(message.terminal.cwd),
|
||||
@@ -686,12 +693,10 @@ export function createWorkerTerminalManager(
|
||||
const activityToken = createActivityToken();
|
||||
const terminalActivityUrl = managerOptions.getTerminalActivityUrl?.() ?? null;
|
||||
terminalActivityTokenById.set(terminalId, activityToken);
|
||||
let result: {
|
||||
terminal: RequiredWorkerTerminalInfo;
|
||||
state: TerminalState;
|
||||
};
|
||||
const creation = { session: null as TerminalSession | null };
|
||||
pendingCreationsById.set(terminalId, creation);
|
||||
try {
|
||||
result = (await sendRequest({
|
||||
await sendRequest({
|
||||
type: "createTerminal",
|
||||
options: {
|
||||
...options,
|
||||
@@ -699,16 +704,17 @@ export function createWorkerTerminalManager(
|
||||
activityToken,
|
||||
activityUrl: terminalActivityUrl,
|
||||
},
|
||||
})) as {
|
||||
terminal: RequiredWorkerTerminalInfo;
|
||||
state: TerminalState;
|
||||
};
|
||||
});
|
||||
if (!creation.session) {
|
||||
throw new Error("Terminal worker completed creation without a terminalCreated event");
|
||||
}
|
||||
return creation.session;
|
||||
} catch (error) {
|
||||
terminalActivityTokenById.delete(terminalId);
|
||||
throw error;
|
||||
} finally {
|
||||
pendingCreationsById.delete(terminalId);
|
||||
}
|
||||
const session = registerRecord({ info: result.terminal, state: result.state });
|
||||
return session;
|
||||
},
|
||||
|
||||
registerCwdEnv(options: { cwd: string; env: Record<string, string> }): void {
|
||||
|
||||
Reference in New Issue
Block a user