Skip to content

Commit ac52ec9

Browse files
committed
fix(browser): keep the Node-only fs-safe package out of the browser graph
The Workflow Runner Browser E2E leg fails all 27 specs on page.waitForFunction timeout, waiting for workflowRunnerReady. My previous attempt at this leg chased a vite build failure. That was the wrong measurement: playwright.config.ts runs the dev server, not a build, so the rollup error I was fixing is not the error CI sees. Reproduced the real one by loading the harness page against the dev server and reading pageerror: Browser stub: node:module.createRequire not supported at .../@openclaw_fs-safe.js:1057 and, once past that, "process is not defined" from the same module. Both are at module scope in @openclaw/fs-safe's native-binding loader, so having it in the graph throws before any code calls it. It is in the graph because of this branch. main's runtime defined its own FileStorageAdapter in context.ts; consolidating onto @nodetool-ai/storage's made storage-workspace.ts import that package statically, and rolldown says so directly while building web: packages/storage/src/index.ts is dynamically imported by packages/runtime/src/context.ts but also statically imported by packages/runtime/src/storage-workspace.ts, packages/runtime/src/testing.ts So context.ts's deliberate await import — its comment says storage reaches node:fs — is defeated by two static ones. Stubbed at the same seam the configs already use for Node-only modules, in both the harness and web. root() is called only from FileStorageAdapter's constructor and a browser has no local directory to point one at, so the stub throws on use like every other one there. web's bundle carried the real package until now: the chunks pulling it are lazy node bundles, so it was latent rather than a load failure. Harness: 26 of 27 pass. The one failure is sandbox-media, on "This video track cannot be decoded by this browser" — this container's codec-less Chromium build, not the change. Before the stub the page never reached workflowRunnerReady at all. Also measured and not shipped: making the createRequire stub return a throwing require instead of throwing itself. It fixes the first error on its own, but with fs-safe stubbed nothing reaches it, so there is no failing case to justify it. web build clean, typecheck web+electron clean, lint 0 errors, harness gate exit 0. Not fixed here, written up on the PR: the two static imports above mean the runtime barrel still cannot be loaded off Node without the alias.
1 parent 842c706 commit ac52ec9

4 files changed

Lines changed: 61 additions & 0 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* `@openclaw/fs-safe`, the symlink-safe filesystem wrapper
3+
* `@nodetool-ai/storage`'s `FileStorageAdapter` is built on.
4+
*
5+
* Not a Node builtin, but it belongs with the stubs for the same reason: it
6+
* is Node-only *at module scope*. Its native-binding loader calls
7+
* `createRequire(import.meta.url)` and reads `process.env` while the module
8+
* evaluates, so merely having it in a browser graph throws before any code
9+
* asks it for anything — which is what took the whole harness entry down.
10+
*
11+
* The browser never constructs a `FileStorageAdapter`: `root()` is called
12+
* only from that constructor, and a browser has no local directory to point
13+
* one at. So the stub throws on use, like every other stub here.
14+
*/
15+
export function root() {
16+
throw new Error("Browser stub: @openclaw/fs-safe.root not supported");
17+
}

packages/workflow-runner/e2e/vite.config.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,20 @@ const BARE_STUBBED = new Set([
8989
"module"
9090
]);
9191

92+
/**
93+
* npm packages — not builtins — that are Node-only *at module scope*, so
94+
* having one in a browser graph throws before any code calls into it.
95+
*
96+
* `@openclaw/fs-safe` backs `@nodetool-ai/storage`'s `FileStorageAdapter`.
97+
* Its native-binding loader builds a `createRequire` and reads `process.env`
98+
* while the module evaluates; both fail in a browser and took the harness
99+
* entry down with them. Nothing here ever constructs that adapter — a
100+
* browser has no local directory — so the stub throws on use.
101+
*/
102+
const PACKAGE_STUBS: Record<string, string> = {
103+
"@openclaw/fs-safe": `${STUBS}/fs-safe-stub.js`
104+
};
105+
92106
/**
93107
* Every specifier form that maps to a stub. Insertion order matters: Vite's
94108
* alias matches a string `find` as a path prefix, so `fs/promises` has to be
@@ -99,6 +113,7 @@ for (const [name, stub] of Object.entries(BUILTIN_STUBS)) {
99113
SPECIFIER_STUBS[`node:${name}`] = stub;
100114
if (BARE_STUBBED.has(name)) SPECIFIER_STUBS[name] = stub;
101115
}
116+
Object.assign(SPECIFIER_STUBS, PACKAGE_STUBS);
102117

103118
const STUB_SPECIFIER_FILTER = new RegExp(
104119
`^(${Object.keys(SPECIFIER_STUBS)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* `@openclaw/fs-safe`, the symlink-safe filesystem wrapper
3+
* `@nodetool-ai/storage`'s `FileStorageAdapter` is built on.
4+
*
5+
* Not a Node builtin, but it belongs with the stubs for the same reason: it
6+
* is Node-only *at module scope*. Its native-binding loader calls
7+
* `createRequire(import.meta.url)` and reads `process.env` while the module
8+
* evaluates, so merely having it in a browser graph throws on load, before
9+
* any code asks it for anything.
10+
*
11+
* The browser never constructs a `FileStorageAdapter`: `root()` is called
12+
* only from that constructor, and a browser has no local directory to point
13+
* one at. So the stub throws on use, like every other stub here.
14+
*/
15+
export function root() {
16+
throw new Error("Browser stub: @openclaw/fs-safe.root not supported");
17+
}

web/vite.config.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,17 @@ const BARE_BUILTIN_STUBS: Record<string, string> = Object.fromEntries(
8282
.map(([key, stub]) => [key.replace(/^node:/, ""), stub])
8383
);
8484

85+
// npm packages — not builtins — that are Node-only *at module scope*, so a
86+
// browser graph that merely contains one throws before any code calls into it.
87+
// `@openclaw/fs-safe` backs `@nodetool-ai/storage`'s `FileStorageAdapter`; its
88+
// native-binding loader builds a `createRequire` and reads `process.env` while
89+
// the module evaluates. The browser never constructs that adapter — `root()`
90+
// is called only from its constructor, and a browser has no local directory to
91+
// point one at — so the stub throws on use.
92+
const NODE_PACKAGE_STUBS: Record<string, string> = {
93+
"@openclaw/fs-safe": `${NODE_STUBS}/fs-safe-stub.js`
94+
};
95+
8596
// Vite's `resolve.alias` doesn't intercept the `node:` protocol — these imports
8697
// bypass the alias plugin and hit the default resolver. Catch them in a `pre`
8798
// resolveId hook before any other plugin runs. `includeBare` additionally stubs
@@ -94,6 +105,7 @@ function stubNodeProtocolPlugin(includeBare = false): Plugin {
94105
resolveId(source) {
95106
return (
96107
NODE_BUILTIN_STUBS[source] ??
108+
NODE_PACKAGE_STUBS[source] ??
97109
(includeBare ? BARE_BUILTIN_STUBS[source] : undefined) ??
98110
null
99111
);

0 commit comments

Comments
 (0)