diff --git a/src/agents/agent-tools.read.ts b/src/agents/agent-tools.read.ts index 892e165f7cf2..ca478b6e4439 100644 --- a/src/agents/agent-tools.read.ts +++ b/src/agents/agent-tools.read.ts @@ -1004,12 +1004,16 @@ async function statHostFile(absolutePath: string) { async function writeWorkspaceFile( root: string, - rootPromise: ReturnType, + getRoot: () => ReturnType, absolutePath: string, content: string, ) { + // Validate the path before starting the fs-safe root: call getRoot() (which opens the + // root dir, rejecting if the workspace is missing) only after toCanonicalRelativeWorkspacePath + // succeeds. Eagerly starting it would orphan a rejecting root promise as an unhandled + // rejection when validation fails first — the readFile/access paths already defer the same way. const relative = await toCanonicalRelativeWorkspacePath(root, absolutePath); - await (await rootPromise).write(relative, content, { mkdir: true }); + await (await getRoot()).write(relative, content, { mkdir: true }); } function createHostWriteOperations(root: string, options?: { workspaceOnly?: boolean }) { @@ -1030,8 +1034,12 @@ function createHostWriteOperations(root: string, options?: { workspaceOnly?: boo } as const; } - // When workspaceOnly is true, enforce workspace boundary - const rootPromise = fsRoot(root); + // When workspaceOnly is true, enforce workspace boundary. Resolve the fs-safe + // root lazily on first use: constructing the tool (e.g. doctor projecting tool + // schemas) must not open an fs handle, and a missing workspace dir must not + // orphan a rejecting promise as "Unhandled promise rejection: root dir not found". + let rootPromise: ReturnType | undefined; + const getRoot = () => (rootPromise ??= fsRoot(root)); return { mkdir: async (dir: string) => { const relative = toRelativeWorkspacePath(root, dir, { allowRoot: true }); @@ -1040,10 +1048,10 @@ function createHostWriteOperations(root: string, options?: { workspaceOnly?: boo await fs.mkdir(resolved, { recursive: true }); }, writeFile: (absolutePath: string, content: string) => - writeWorkspaceFile(root, rootPromise, absolutePath, content), + writeWorkspaceFile(root, getRoot, absolutePath, content), readFile: async (absolutePath: string) => { const relative = toRelativeWorkspacePath(root, absolutePath); - return (await (await rootPromise).read(relative)).buffer; + return (await (await getRoot()).read(relative)).buffer; }, statFile: async (absolutePath: string) => { const relative = toRelativeWorkspacePath(root, absolutePath); @@ -1068,16 +1076,20 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool } as const; } - // When workspaceOnly is true, enforce workspace boundary - const rootPromise = fsRoot(root); + // When workspaceOnly is true, enforce workspace boundary. Resolve the fs-safe + // root lazily on first use: constructing the tool (e.g. doctor projecting tool + // schemas) must not open an fs handle, and a missing workspace dir must not + // orphan a rejecting promise as "Unhandled promise rejection: root dir not found". + let rootPromise: ReturnType | undefined; + const getRoot = () => (rootPromise ??= fsRoot(root)); return { readFile: async (absolutePath: string) => { const relative = toRelativeWorkspacePath(root, absolutePath); - const safeRead = await (await rootPromise).read(relative); + const safeRead = await (await getRoot()).read(relative); return safeRead.buffer; }, writeFile: (absolutePath: string, content: string) => - writeWorkspaceFile(root, rootPromise, absolutePath, content), + writeWorkspaceFile(root, getRoot, absolutePath, content), access: async (absolutePath: string) => { let relative: string; try { @@ -1091,7 +1103,7 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool return; } try { - const opened = await (await rootPromise).open(relative); + const opened = await (await getRoot()).open(relative); await opened.handle.close().catch(() => {}); } catch (error) { if (error instanceof FsSafeError && error.code === "not-found") { diff --git a/src/agents/agent-tools.read.workspace-root-lazy.test.ts b/src/agents/agent-tools.read.workspace-root-lazy.test.ts new file mode 100644 index 000000000000..3f732c4dd957 --- /dev/null +++ b/src/agents/agent-tools.read.workspace-root-lazy.test.ts @@ -0,0 +1,110 @@ +import path from "node:path"; +import { describe, expect, it, vi } from "vitest"; + +// Regression guard: doctor's active-tool schema projection constructs the full +// coding toolset to inspect tool input schemas. Workspace-scoped edit/write +// tools used to resolve their fs-safe root eagerly at construction. When the +// agent's workspace dir does not exist yet (e.g. an unresolved `${ENV}` +// placeholder in the authored config), that orphaned a rejecting promise: +// "[openclaw] Unhandled promise rejection: FsSafeError: root dir not found" +// The root must only be opened when a read/write/access operation actually runs. + +const rootSpy = vi.hoisted(() => vi.fn()); + +type WorkspaceFileOps = { + writeFile: (absolutePath: string, content: string) => Promise; +}; +const captured = vi.hoisted(() => ({ + write: undefined as WorkspaceFileOps | undefined, + edit: undefined as WorkspaceFileOps | undefined, +})); + +vi.mock("../infra/fs-safe.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, root: rootSpy }; +}); + +// Capture the operations object handed to the underlying write/edit tools so the +// regression can drive a single workspace write operation directly. +vi.mock("./sessions/index.js", async (importOriginal) => { + const actual = await importOriginal(); + const stub = (name: string) => ({ + name, + description: `test ${name} tool`, + parameters: { type: "object", properties: {} }, + execute: async () => ({ content: [{ type: "text" as const, text: "ok" }] }), + }); + return { + ...actual, + createWriteTool: (_cwd: string, options?: { operations?: WorkspaceFileOps }) => { + captured.write = options?.operations; + return stub("write"); + }, + createEditTool: (_cwd: string, options?: { operations?: WorkspaceFileOps }) => { + captured.edit = options?.operations; + return stub("edit"); + }, + }; +}); + +const { createHostWorkspaceEditTool, createHostWorkspaceWriteTool } = + await import("./agent-tools.read.js"); + +function requireOps(ops: WorkspaceFileOps | undefined, label: string): WorkspaceFileOps { + if (!ops) { + throw new Error(`expected captured ${label} operations`); + } + return ops; +} + +describe("workspace-scoped coding tools resolve their fs root lazily", () => { + it("does not open the fs-safe root while only constructing the tool", () => { + // Resolve to a stub root handle so a lazy call would not reject, yet assert + // construction never reaches it. + rootSpy.mockReset().mockResolvedValue({ + read: vi.fn(), + write: vi.fn(), + open: vi.fn(), + }); + const missingWorkspace = "/openclaw-nonexistent-workspace-zzz/does/not/exist"; + + createHostWorkspaceEditTool(missingWorkspace, { workspaceOnly: true }); + createHostWorkspaceWriteTool(missingWorkspace, { workspaceOnly: true }); + + expect(rootSpy).not.toHaveBeenCalled(); + }); + + it("does not orphan a rejecting fs-safe root when a write/edit targets a missing root", async () => { + // A workspace-only write/edit against an absent root fails path validation first + // (realpath on the missing root). The fs-safe root must only be started after that + // validation succeeds, so a failed write never leaves a rejecting root promise + // unawaited and surfacing as an unhandled rejection. + rootSpy.mockReset().mockRejectedValue(new Error("root dir not found")); + const missingWorkspace = "/openclaw-nonexistent-workspace-zzz/does/not/exist"; + const missingFile = path.join(missingWorkspace, "out.txt"); + + createHostWorkspaceWriteTool(missingWorkspace, { workspaceOnly: true }); + createHostWorkspaceEditTool(missingWorkspace, { workspaceOnly: true }); + const writeOps = requireOps(captured.write, "write"); + const editOps = requireOps(captured.edit, "edit"); + + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => unhandled.push(reason); + process.on("unhandledRejection", onUnhandled); + try { + await expect(writeOps.writeFile(missingFile, "x")).rejects.toThrow(); + await expect(editOps.writeFile(missingFile, "x")).rejects.toThrow(); + // Let any orphaned rejection settle into the listener before asserting. + await new Promise((resolve) => { + setImmediate(resolve); + }); + } finally { + process.off("unhandledRejection", onUnhandled); + } + + // Validation failed before the fs-safe root was started, so there is no orphaned + // rejecting promise — neither a rootSpy call nor an unhandled rejection. + expect(rootSpy).not.toHaveBeenCalled(); + expect(unhandled).toEqual([]); + }); +});