fix(ai): replace raw require(node:fs) with loadNodeBuiltinModule in getProcEnv by wm0018 · Pull Request #102947 · openclaw/openclaw

wm0018 · 2026-07-09T15:08:11Z

What Problem This Solves

Fixes #102934 — getProcEnv used a raw require("node:fs") string literal on line 92, which static bundlers such as Vite scan and attempt to resolve even when guarded by try/catch. The same file already has a safe loadNodeBuiltinModule wrapper (line 17) used by loadNodeHelpersSync (line 32) for the same purpose — getProcEnv was the only remaining raw require.

Why This Change Was Made

The file header explicitly states: // NEVER convert to top-level imports - breaks browser/Vite builds. Using loadNodeBuiltinModule and NODE_FS_SPECIFIER (both already defined in the file) eliminates the static require("node:fs") literal without changing runtime behavior.

User Impact

No user-visible impact. Fixes a static analysis / bundler compatibility gap for packages consuming @openclaw/ai in browser contexts.

Evidence

Before — packages/ai/src/env-api-keys.ts:92 (raw static require literal):

const { readFileSync } = require("node:fs") as typeof import("node:fs");
const data = readFileSync("/proc/self/environ", "utf-8");

After — same behavior via loadNodeBuiltinModule:

const fsModule = loadNodeBuiltinModule(NODE_FS_SPECIFIER) as typeof import("node:fs") | null;
if (!fsModule) {
  return undefined;
}
const data = fsModule.readFileSync("/proc/self/environ", "utf-8");

loadNodeBuiltinModule is runtime-equivalent — packages/ai/src/env-api-keys.ts:17-28:

function loadNodeBuiltinModule(specifier: string): NodeBuiltinModule | null {
  const getBuiltinModule = (typeof process !== "undefined" ? process : undefined) as
    | (NodeJS.Process & { getBuiltinModule?: (id: string) => unknown })
    | undefined;
  if (typeof getBuiltinModule?.getBuiltinModule === "function") {
    return getBuiltinModule.getBuiltinModule(specifier) as NodeBuiltinModule;
  }
  if (typeof require === "function") {
    return require(specifier) as NodeBuiltinModule;  // ← same require call, but indirect
  }
  return null;
}

The wrapper first tries process.getBuiltinModule (Node 22+), then falls back to require(specifier). The key difference for bundlers: require is called with a variable (specifier) rather than a string literal ("node:fs"), so static analyzers cannot trace the dependency. The added null guard is also stricter — getProcEnv now returns undefined when fs is unavailable, where previously it would have thrown.

clawsweeper · 2026-07-09T15:28:56Z

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 11:37 AM ET / 15:37 UTC.

Summary
This PR replaces the literal require("node:fs") in packages/ai/src/env-api-keys.ts#getProcEnv with the module's existing loadNodeBuiltinModule(NODE_FS_SPECIFIER) path and a null guard.

PR surface: Source +3. Total +3 across 1 file.

Reproducibility: yes. source-reproducible: current main still contains the exact require("node:fs") literal in getProcEnv on the browser-sensitive AI package path. I did not establish a live Vite/browser failure in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #102934
Summary: The linked issue is the canonical report for the static node:fs require in getProcEnv; this PR and another open PR are candidate fixes for that same source-visible problem.

Members:

canonical: fix(ai): direct require("node:fs") in getProcEnv breaks browser bundlers #102934 - The PR body uses closing syntax for this issue, and the issue body reports the same getProcEnv static require("node:fs") problem.
fixed_by_candidate: fix(ai): direct require('node:fs') in getProcEnv breaks browser bundlers #102955 - This later open PR changes the same lines in packages/ai/src/env-api-keys.ts for the same reported problem, but it lacks positive real behavior proof and is not a safer replacement target.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted real behavior proof for the reported bundler or Bun-consumer path; terminal output or logs are fine for this non-visual change.
After proof is added, make sure maintainers land only one of the duplicate candidate PRs for the linked issue.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR has code snippets but no after-fix real behavior proof; please add redacted terminal output, logs, a linked artifact, or a short recording showing a real Vite/browser or Bun compiled consumer path after the fix, and update the PR body to trigger re-review.

Risk before merge

[P1] No redacted real Vite/browser/Bun consumer proof is attached, so code inspection and CI do not yet prove the reported bundler behavior is fixed in a real setup.
[P1] A second open PR targets the same linked issue; maintainers should land only one narrow fix and close or supersede the duplicate candidate.

Maintainer options:

Decide the mitigation before merge
Land one narrow, proof-backed PR that removes the remaining static node:fs require through the existing AI package helper, then close the linked issue and retire duplicate candidates.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] The remaining blocker is contributor-supplied real behavior proof and duplicate-PR coordination, not an automated code repair.

Security
Cleared: The diff only changes an existing Node builtin loading path in the AI package and adds no dependencies, workflow changes, lockfile changes, credential handling expansion, or new code-execution source.

Review details

Best possible solution:

Land one narrow, proof-backed PR that removes the remaining static node:fs require through the existing AI package helper, then close the linked issue and retire duplicate candidates.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: current main still contains the exact require("node:fs") literal in getProcEnv on the browser-sensitive AI package path. I did not establish a live Vite/browser failure in this read-only review.

Is this the best way to solve the issue?

Yes for the code shape: reusing loadNodeBuiltinModule(NODE_FS_SPECIFIER) is the narrowest local fix because the same module and sibling AI provider code already use indirect Node builtin loading for browser/Vite compatibility. Merge readiness still depends on real behavior proof from the contributor.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3c048ef05297.

Label changes

Label changes:

add P2: This is a focused package compatibility bugfix with limited blast radius for @openclaw/ai browser-bundler consumers.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR has code snippets but no after-fix real behavior proof; please add redacted terminal output, logs, a linked artifact, or a short recording showing a real Vite/browser or Bun compiled consumer path after the fix, and update the PR body to trigger re-review.

Label justifications:

P2: This is a focused package compatibility bugfix with limited blast radius for @openclaw/ai browser-bundler consumers.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR has code snippets but no after-fix real behavior proof; please add redacted terminal output, logs, a linked artifact, or a short recording showing a real Vite/browser or Bun compiled consumer path after the fix, and update the PR body to trigger re-review.

Evidence reviewed

PR surface:

Source +3. Total +3 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	2	+3
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	5	2	+3

What I checked:

Current main still has the reported literal: Current main still calls require("node:fs") inside getProcEnv, while the same module already defines NODE_FS_SPECIFIER and loadNodeBuiltinModule. (packages/ai/src/env-api-keys.ts:92, 3c048ef05297)
PR patch removes the literal at the affected line: The PR replaces the direct literal require with loadNodeBuiltinModule(NODE_FS_SPECIFIER), returns undefined when no fs module is available, and keeps the /proc/self/environ parsing path otherwise. (packages/ai/src/env-api-keys.ts:92, 310867db6a63)
Existing module contract favors browser-safe loading: The file's top comment says not to convert Node modules to top-level imports because browser/Vite builds break, and the nearby helper already loads Node builtins indirectly. (packages/ai/src/env-api-keys.ts:1, 3c048ef05297)
Affected API key path is reachable from package providers: getEnvApiKey resolves environment-backed provider keys through getEnvValue, and package providers such as OpenAI Responses import it directly. (packages/ai/src/providers/openai-responses.ts:4, 3c048ef05297)
The AI package is a publishable library surface: Docs describe @openclaw/ai as a publishable library that external consumers can depend on directly, and packages/ai/package.json exposes built ESM entrypoints. Public docs: docs/reference/openclaw-ai.md. (docs/reference/openclaw-ai.md:10, 3c048ef05297)
Sibling AI code uses the same browser-safe pattern: openai-chatgpt-responses.ts avoids top-level runtime imports and uses process.getBuiltinModule for Node builtins under the same browser/Vite constraint. (packages/ai/src/providers/openai-chatgpt-responses.ts:19, 3c048ef05297)

Likely related people:

huangjianxiong: git blame attributes the current env-api-keys.ts file, the browser/Vite-safe helper, and the remaining static require("node:fs") line to this commit. (role: introduced behavior; confidence: high; commits: 0e626690cd5d; files: packages/ai/src/env-api-keys.ts, packages/ai/src/providers/openai-chatgpt-responses.ts)
LiLan0125: Recent packages/ai history shows adjacent provider/package work after the AI package split, making this a plausible secondary routing candidate for package-level review. (role: recent adjacent area contributor; confidence: medium; commits: db0f6f09dcf2; files: packages/ai/src)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

…etProcEnv

steipete · 2026-07-09T23:14:43Z

Maintainer fixup and land-ready proof for exact head 62485c21b9e08795147f06d0a4897fec9b163435:

Reused the owning module's existing loadNodeBuiltinModule(NODE_FS_SPECIFIER) path; no second loader, top-level Node import, dependency, or public API change.
Added packages/ai/src/env-api-keys.browser-bundle.test.ts, which bundles the real module for the browser and rejects an emitted static node:fs require.
Blacksmith Testbox through Crabbox tbx_01kx4he284dbm90f6q4z6480hh: focused regression 1/1; complete packages/ai suite 28 files / 350 tests; check:changed passed for core and coreTests.
Live Vite 8.1.3 production builds on the exact head: pre-fix node:fs externalization warnings = 1; final warnings = 0; both chunks = 4180 bytes.
Exact-head canonical CI run 29056319772 is green, including build, type, lint, dependency, architecture, and compact Node lanes.
Fresh full-diff autoreview: clean (0.86 confidence). Review artifacts and OPENCLAW_TESTBOX=1 scripts/pr prepare-run 102947 both passed.

Known proof gap: the Testbox image has no Bun executable, so a direct Bun /proc/self/environ scenario could not start. The fallback's gates and parsing are unchanged; the only runtime change routes node:fs through the same synchronous builtin loader already used in this module. Node documents process.getBuiltinModule for exactly this conditional cross-environment builtin-loading use case.

Best-fix verdict: best. A top-level import breaks the package's browser contract, another helper duplicates policy, and deleting or async-converting the fallback would regress the tracked Bun sandbox workaround.

steipete · 2026-07-09T23:15:02Z

Merged via squash.

Prepared head SHA: 62485c21b9e08795147f06d0a4897fec9b163435
Landed commit: 50cb0d66b8e63e5c2baea8f7ce065007d6659521

…etProcEnv (openclaw#102947) * fix(ai): replace raw require(node:fs) with loadNodeBuiltinModule in getProcEnv * test(ai): cover browser-safe env key bundle --------- Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Jul 9, 2026

wm0018 force-pushed the fix/env-api-keys-load-node-builtin-module branch from aae01c1 to 310867d Compare July 9, 2026 15:09

openclaw-barnacle Bot removed the gateway Gateway runtime label Jul 9, 2026

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(ai): direct require("node:fs") in getProcEnv breaks browser bundlers #102934

Closed

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix(ai): direct require('node:fs') in getProcEnv breaks browser bundlers #102955

Closed

steipete self-assigned this Jul 9, 2026

wm0018 and others added 2 commits July 9, 2026 19:01

fix(ai): replace raw require(node:fs) with loadNodeBuiltinModule in g…

80a0e37

…etProcEnv

test(ai): cover browser-safe env key bundle

62485c2

steipete force-pushed the fix/env-api-keys-load-node-builtin-module branch from 310867d to 62485c2 Compare July 9, 2026 23:03

steipete merged commit 50cb0d6 into openclaw:main Jul 9, 2026
81 checks passed

github-actions Bot mentioned this pull request Jul 10, 2026

📡 Upstream Digest — 2026-07-10 02:07 UTC curtismercier/openclaw-mods#1248

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(ai): replace raw require(node:fs) with loadNodeBuiltinModule in getProcEnv#102947

fix(ai): replace raw require(node:fs) with loadNodeBuiltinModule in getProcEnv#102947
steipete merged 2 commits into
openclaw:mainfrom
wm0018:fix/env-api-keys-load-node-builtin-module

wm0018 commented Jul 9, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wm0018 commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wm0018 commented Jul 9, 2026 •

edited

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading