fix(agent-tools): resolve workspace-scoped tool fs root lazily (#89226)

* fix(agent-tools): resolve workspace-scoped tool fs root lazily

Workspace-scoped edit/write tools resolved their fs-safe root eagerly at
construction. Doctor's active-tool schema projection builds the full coding
toolset just to read tool schemas; when an agent workspace dir is absent at
that time (e.g. an unresolved ${ENV} placeholder in the authored config the
legacy-migration path operates on), the eager fsRoot orphaned a rejecting
promise as 'Unhandled promise rejection: FsSafeError: root dir not found'.

Resolve the root lazily and memoized so construction never opens an fs handle;
the root is only opened on the first real read/write/access operation.

* fix(agent-tools): defer fs-safe root start until workspace write validation succeeds

A workspace-only write/edit against an absent root started fsRoot(root) eagerly
(passed as getRoot() into writeWorkspaceFile) before toCanonicalRelativeWorkspacePath
ran. When validation failed (realpath on the missing root), the rejecting root promise
was left unawaited and surfaced as an unhandled rejection — the readFile/access paths
already defer getRoot() the same way.

writeWorkspaceFile now takes a getRoot thunk and calls it only after validation; the
write and edit-write callers pass the thunk instead of a started promise. Adds a
regression that a missing-root write/edit rejects without starting the fs-safe root or
emitting an unhandled rejection.

---------

Co-authored-by: Sasan <sasan.sotoodehfar@gmail.com>
Co-authored-by: Gio Della-Libera <giodl73@gmail.com>
This commit is contained in:
Sasan 2026-06-28 16:26:21 -04:00 committed by GitHub
parent 19617b0b45
commit 201686d9e3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 133 additions and 11 deletions

View file

@ -1004,12 +1004,16 @@ async function statHostFile(absolutePath: string) {
async function writeWorkspaceFile(
root: string,
rootPromise: ReturnType<typeof fsRoot>,
getRoot: () => ReturnType<typeof fsRoot>,
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<typeof fsRoot> | 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<typeof fsRoot> | 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") {

View file

@ -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<void>;
};
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<typeof import("../infra/fs-safe.js")>();
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<typeof import("./sessions/index.js")>();
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<void>((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([]);
});
});