Stop daemon crashes from terminal-list worker timeouts
getTerminals was the only terminal read that round-tripped the terminal worker process. Under load the IPC call could hit its 10s timeout, and the rejection from a fire-and-forget workspace-update emit propagated as an unhandled rejection — taking down the whole daemon. The daemon already maintains an authoritative mirror of terminal state (every other read is served from it), so getTerminals now reads that mirror locally, preserving the worker's descendant-path aggregation and absolute-path guard. The getTerminals worker IPC request is removed entirely, so no terminal read can stall on the worker anymore. Also adds a defensive .catch to the two fire-and-forget workspace-update sites, and hoists the duplicated assertAbsolutePath guard into one shared helper in path-utils.
This commit is contained in:
@@ -1,5 +1,11 @@
|
||||
import { homedir } from "node:os";
|
||||
import { isAbsolute, resolve } from "node:path";
|
||||
import { isAbsolute, posix, resolve, win32 } from "node:path";
|
||||
|
||||
export function assertAbsolutePath(cwd: string): void {
|
||||
if (!posix.isAbsolute(cwd) && !win32.isAbsolute(cwd)) {
|
||||
throw new Error("cwd must be absolute path");
|
||||
}
|
||||
}
|
||||
|
||||
function hasHomePrefix(value: string): boolean {
|
||||
return value === "~" || value.startsWith("~/");
|
||||
|
||||
@@ -1297,7 +1297,12 @@ export class Session {
|
||||
if (this.terminalManager) {
|
||||
this.unsubscribeTerminalActivityEvents = this.terminalManager.subscribeTerminalsChanged(
|
||||
(event) => {
|
||||
void this.emitWorkspaceUpdateForCwd(event.cwd);
|
||||
void this.emitWorkspaceUpdateForCwd(event.cwd).catch((error) => {
|
||||
this.sessionLogger.warn(
|
||||
{ err: error, cwd: event.cwd },
|
||||
"Failed to emit workspace update after terminals changed",
|
||||
);
|
||||
});
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -5082,7 +5087,12 @@ export class Session {
|
||||
{ cwd: normalizedCwd },
|
||||
(snapshot) => {
|
||||
this.handleWorkspaceGitBranchSnapshot(normalizedCwd, snapshot.git.currentBranch ?? null);
|
||||
void this.emitWorkspaceUpdateForCwd(normalizedCwd);
|
||||
void this.emitWorkspaceUpdateForCwd(normalizedCwd).catch((error) => {
|
||||
this.sessionLogger.warn(
|
||||
{ err: error, cwd: normalizedCwd },
|
||||
"Failed to emit workspace update after git branch snapshot",
|
||||
);
|
||||
});
|
||||
this.emitCheckoutStatusUpdate(normalizedCwd, snapshot);
|
||||
},
|
||||
);
|
||||
|
||||
@@ -7,8 +7,8 @@ import {
|
||||
} from "./terminal.js";
|
||||
import { captureTerminalLines, type CaptureTerminalLinesResult } from "./terminal-capture.js";
|
||||
import { randomBytes, randomUUID } from "node:crypto";
|
||||
import { resolve, sep, win32, posix } from "node:path";
|
||||
import { isSameOrDescendantPath } from "../server/path-utils.js";
|
||||
import { resolve, sep } from "node:path";
|
||||
import { assertAbsolutePath, isSameOrDescendantPath } from "../server/path-utils.js";
|
||||
import type { TerminalActivity, TerminalActivityState } from "@getpaseo/protocol/terminal-activity";
|
||||
|
||||
export interface TerminalListItem {
|
||||
@@ -97,12 +97,6 @@ export function createTerminalManager(
|
||||
const terminalActivityListeners = new Set<TerminalActivityListener>();
|
||||
const defaultEnvByRootCwd = new Map<string, Record<string, string>>();
|
||||
|
||||
function assertAbsolutePath(cwd: string): void {
|
||||
if (!posix.isAbsolute(cwd) && !win32.isAbsolute(cwd)) {
|
||||
throw new Error("cwd must be absolute path");
|
||||
}
|
||||
}
|
||||
|
||||
function removeSessionById(id: string, options: { kill: boolean }): void {
|
||||
const session = terminalsById.get(id);
|
||||
if (!session) {
|
||||
|
||||
@@ -241,19 +241,6 @@ async function handleCreateTerminalRequest(message: TerminalCreateRequest): Prom
|
||||
|
||||
async function handleRequest(message: TerminalWorkerRequest): Promise<void> {
|
||||
switch (message.type) {
|
||||
case "getTerminals": {
|
||||
const terminals = await manager.getTerminals(message.cwd, {
|
||||
workspaceId: message.workspaceId,
|
||||
});
|
||||
sendToParent({
|
||||
type: "response",
|
||||
requestId: message.requestId,
|
||||
ok: true,
|
||||
result: terminals.map(toTerminalInfo),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
case "createTerminal": {
|
||||
await enqueueCreateTerminalRequest(message);
|
||||
return;
|
||||
|
||||
@@ -37,12 +37,6 @@ export interface WorkerKillAndWaitOptions {
|
||||
}
|
||||
|
||||
export type TerminalWorkerRequest =
|
||||
| {
|
||||
type: "getTerminals";
|
||||
requestId: string;
|
||||
cwd: string;
|
||||
workspaceId?: string;
|
||||
}
|
||||
| {
|
||||
type: "createTerminal";
|
||||
requestId: string;
|
||||
|
||||
@@ -523,6 +523,55 @@ it("lists subdirectory terminals when querying the workspace root", async () =>
|
||||
expect(rootTerminals.map((terminal) => terminal.id)).toEqual([created.id]);
|
||||
});
|
||||
|
||||
it("lists terminals locally without waiting on the worker", async () => {
|
||||
const worker = new FakeTerminalWorker();
|
||||
manager = createWorkerTerminalManager({
|
||||
requestTimeoutMs: 5,
|
||||
forkWorker: () => worker,
|
||||
});
|
||||
|
||||
worker.emitWorkerMessage({
|
||||
type: "terminalCreated",
|
||||
terminal: {
|
||||
id: "terminal-root",
|
||||
name: "Shell",
|
||||
cwd: "/workspace",
|
||||
activity: { state: "idle", changedAt: 0 },
|
||||
},
|
||||
state: createTerminalState(),
|
||||
});
|
||||
worker.emitWorkerMessage({
|
||||
type: "terminalCreated",
|
||||
terminal: {
|
||||
id: "terminal-subdir",
|
||||
name: "Shell",
|
||||
cwd: "/workspace/apps/mobile",
|
||||
activity: { state: "idle", changedAt: 0 },
|
||||
},
|
||||
state: createTerminalState(),
|
||||
});
|
||||
|
||||
// The fake worker never answers requests, so a round-trip would reject at the
|
||||
// 5ms timeout. A local mirror read must resolve regardless.
|
||||
const terminals = await manager.getTerminals("/workspace");
|
||||
|
||||
expect(terminals.map((terminal) => terminal.id).sort()).toEqual([
|
||||
"terminal-root",
|
||||
"terminal-subdir",
|
||||
]);
|
||||
expect(worker.sentMessages.some((message) => message.type === "getTerminals")).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects non-absolute cwd in getTerminals", async () => {
|
||||
const worker = new FakeTerminalWorker();
|
||||
manager = createWorkerTerminalManager({
|
||||
requestTimeoutMs: 5,
|
||||
forkWorker: () => worker,
|
||||
});
|
||||
|
||||
await expect(manager.getTerminals("relative/path")).rejects.toThrow("cwd must be absolute path");
|
||||
});
|
||||
|
||||
it("surfaces worker activity changes via getActivity, onActivityChange, and terminalsChanged", async () => {
|
||||
const worker = new FakeTerminalWorker();
|
||||
manager = createWorkerTerminalManager({
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { fork } from "node:child_process";
|
||||
import { randomBytes, randomUUID } from "node:crypto";
|
||||
import { assertAbsolutePath, isSameOrDescendantPath } from "../server/path-utils.js";
|
||||
import type { TerminalState } from "@getpaseo/protocol/messages";
|
||||
import type { TerminalActivity, TerminalActivityState } from "@getpaseo/protocol/terminal-activity";
|
||||
import type {
|
||||
@@ -624,23 +625,41 @@ export function createWorkerTerminalManager(
|
||||
});
|
||||
}
|
||||
|
||||
function toSessions(terminals: WorkerTerminalInfo[]): TerminalSession[] {
|
||||
return terminals
|
||||
.map((terminal) => recordsById.get(terminal.id)?.session)
|
||||
.filter((session): session is TerminalSession => Boolean(session));
|
||||
}
|
||||
|
||||
return {
|
||||
async getTerminals(
|
||||
cwd: string,
|
||||
options?: { workspaceId?: string },
|
||||
): Promise<TerminalSession[]> {
|
||||
const result = (await sendRequest({
|
||||
type: "getTerminals",
|
||||
cwd,
|
||||
...(options?.workspaceId !== undefined ? { workspaceId: options.workspaceId } : {}),
|
||||
})) as WorkerTerminalInfo[];
|
||||
return toSessions(result);
|
||||
assertAbsolutePath(cwd);
|
||||
|
||||
// Served from the local mirror, exactly like every other parent read.
|
||||
// Terminals are bucketed by exact cwd, but an agent can open a terminal in
|
||||
// a subdirectory of the workspace. A query for the workspace root must
|
||||
// surface those too, so aggregate every bucket at or below `cwd`.
|
||||
const sessions: TerminalSession[] = [];
|
||||
for (const [bucketCwd, terminalIds] of terminalIdsByCwd) {
|
||||
if (!isSameOrDescendantPath(cwd, bucketCwd)) {
|
||||
continue;
|
||||
}
|
||||
for (const terminalId of terminalIds) {
|
||||
const session = recordsById.get(terminalId)?.session;
|
||||
if (session) {
|
||||
sessions.push(session);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// When the query carries a workspaceId, two workspaces sharing a cwd must
|
||||
// not see each other's terminals. Exclude sessions owned by a different
|
||||
// workspace; keep sessions without an owner (COMPAT: created by clients
|
||||
// that predate terminal workspace ownership).
|
||||
if (options?.workspaceId !== undefined) {
|
||||
return sessions.filter(
|
||||
(session) =>
|
||||
session.workspaceId === undefined || session.workspaceId === options.workspaceId,
|
||||
);
|
||||
}
|
||||
return sessions;
|
||||
},
|
||||
|
||||
async createTerminal(options: WorkerCreateTerminalOptions): Promise<TerminalSession> {
|
||||
|
||||
Reference in New Issue
Block a user