From 42969e40d59259f2ab2eb5f156ed8353deb8a066 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 15 Jun 2026 13:49:04 +0700 Subject: [PATCH] Stop daemon crashes from terminal-list worker timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/server/src/server/path-utils.ts | 8 ++- packages/server/src/server/session.ts | 14 +++++- .../server/src/terminal/terminal-manager.ts | 10 +--- .../src/terminal/terminal-worker-process.ts | 13 ----- .../src/terminal/terminal-worker-protocol.ts | 6 --- .../terminal/worker-terminal-manager.test.ts | 49 +++++++++++++++++++ .../src/terminal/worker-terminal-manager.ts | 43 +++++++++++----- 7 files changed, 101 insertions(+), 42 deletions(-) diff --git a/packages/server/src/server/path-utils.ts b/packages/server/src/server/path-utils.ts index 84fe069ac..f85c092f9 100644 --- a/packages/server/src/server/path-utils.ts +++ b/packages/server/src/server/path-utils.ts @@ -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("~/"); diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 7f191aeaa..37bba12f0 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -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); }, ); diff --git a/packages/server/src/terminal/terminal-manager.ts b/packages/server/src/terminal/terminal-manager.ts index 612691fa8..f9c7c500a 100644 --- a/packages/server/src/terminal/terminal-manager.ts +++ b/packages/server/src/terminal/terminal-manager.ts @@ -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(); const defaultEnvByRootCwd = new Map>(); - 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) { diff --git a/packages/server/src/terminal/terminal-worker-process.ts b/packages/server/src/terminal/terminal-worker-process.ts index 165f4bd67..cf21ee279 100644 --- a/packages/server/src/terminal/terminal-worker-process.ts +++ b/packages/server/src/terminal/terminal-worker-process.ts @@ -241,19 +241,6 @@ async function handleCreateTerminalRequest(message: TerminalCreateRequest): Prom async function handleRequest(message: TerminalWorkerRequest): Promise { 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; diff --git a/packages/server/src/terminal/terminal-worker-protocol.ts b/packages/server/src/terminal/terminal-worker-protocol.ts index c4589d99d..4de6438e0 100644 --- a/packages/server/src/terminal/terminal-worker-protocol.ts +++ b/packages/server/src/terminal/terminal-worker-protocol.ts @@ -37,12 +37,6 @@ export interface WorkerKillAndWaitOptions { } export type TerminalWorkerRequest = - | { - type: "getTerminals"; - requestId: string; - cwd: string; - workspaceId?: string; - } | { type: "createTerminal"; requestId: string; diff --git a/packages/server/src/terminal/worker-terminal-manager.test.ts b/packages/server/src/terminal/worker-terminal-manager.test.ts index a2220a5b0..3ed00c349 100644 --- a/packages/server/src/terminal/worker-terminal-manager.test.ts +++ b/packages/server/src/terminal/worker-terminal-manager.test.ts @@ -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({ diff --git a/packages/server/src/terminal/worker-terminal-manager.ts b/packages/server/src/terminal/worker-terminal-manager.ts index c53ed613d..591123708 100644 --- a/packages/server/src/terminal/worker-terminal-manager.ts +++ b/packages/server/src/terminal/worker-terminal-manager.ts @@ -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 { - 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 {