Skip to content

fix(node:os): observe runtime HOME changes - #42599

Open
steipete wants to merge 1 commit into
oven-sh:mainfrom
steipete:codex/os-homedir-runtime-home
Open

steipete wants to merge 1 commit into
oven-sh:mainfrom
steipete:codex/os-homedir-runtime-home

Conversation

@steipete

@steipete steipete commented Sep 13, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Makes POSIX os.homedir() observe the current HOME value, matching Node when user code changes or deletes process.env.HOME after startup.

Bun's native POSIX binding read a startup-cached environment value before falling back to the passwd database. Runtime process.env and Bun.env mutations live in the JavaScript environment object, so os.homedir() kept returning the old home. The JavaScript adapter now owns the live POSIX environment lookup, including explicitly empty and whitespace-only values, while the native binding provides a passwd-only fallback. os.userInfo().homedir remains passwd-derived. Windows stays on the existing libuv binding.

Fixes #29244.

AI-assisted: yes. Codex helped isolate the environment ownership boundary, implement the patch, and design the cross-runtime regression. I reviewed the code and validation results.

How did you verify your code works?

  • Bun 1.4.2 fails the focused runtime-HOME regression for the intended stale-value reason.
  • Patched debug runtime: test/js/node/os/os.test.js — 55 passed, 0 failed.
  • Vendored Node test-os.js and test-os-homedir-no-envvar.js both exit successfully.
  • Host and x86_64-pc-windows-msvc bun_runtime Rust checks pass.
  • bun run lint, targeted Prettier, cargo fmt --check, root tsc --noEmit, and git diff --check pass.
  • A custom integration build at 1.4.3-canary.1+91b28c4bf contains a patch-equivalent copy of this exact change. The PR commit and integration-stack commit have the same stable patch ID.
  • The prior Node-compatible node:test conformance suite plus a set/empty/whitespace/unset spawn matrix passes 7/7 under Node and 7/7 under that Bun build.
  • OpenClaw's adjacent iMessage path, account, runtime, and remote-host tests pass 76/76 under that Bun build. This includes the real os.userInfo().homedir regression with whitespace-only HOME.
  • A fresh independent P0-P2 review of the exact PR head found no actionable findings.

A clean full debug build of current main is independently blocked on this macOS host by existing SDK/compiler failures. The source patch passed the current-main Rust checks, and runtime validation used previously built debug and integration binaries containing the patch.

OpenClaw integration context: openclaw/openclaw#146807

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b6be0f4e-4934-4dfc-8ae2-fa9a565c1e6b

📥 Commits

Reviewing files that changed from the base of the PR and between 09bb546 and 772e4ac.

📒 Files selected for processing (3)
  • src/js/node/os.ts
  • src/runtime/node/node_os.rs
  • test/js/node/os/os.test.js
💤 Files with no reviewable changes (1)
  • src/runtime/node/node_os.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


Walkthrough

The JavaScript os.homedir() implementation now uses non-Windows Bun.env.HOME values before falling back to the runtime binding. The runtime binding no longer has a HOME fast path. Tests cover HOME changes and userInfo().homedir.

Changes

Homedir behavior

Layer / File(s) Summary
Update homedir resolution
src/js/node/os.ts, src/runtime/node/node_os.rs
The JavaScript wrapper checks Bun.env.HOME on non-Windows platforms and falls back to binding.homedir(). The runtime implementation proceeds directly to passwd lookup. Windows behavior remains unchanged.
Validate environment changes
test/js/node/os/os.test.js
Tests cover changed, empty, and deleted HOME values. The test also verifies that userInfo().homedir remains unchanged and restores the environment.

Suggested reviewers: robobun

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 772e4

The updated homedir behavior honors current HOME values on POSIX while retaining the passwd fallback and existing Windows behavior. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: making node:os observe runtime HOME changes.
Description check ✅ Passed The description includes both required sections. It explains the behavior change, implementation, verification results, known build limitation, and related issue.

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes #29244. Two older PRs covered the same bug. #29248 made the same change to os.ts and node_os.rs. #34629 fixed only the HOME="" and userInfo().homedir cases. I closed both in favor of this one: it is the smallest diff and it is based on current main.

I built this branch (772e4ac, debug build with ASAN, Linux x64) and checked it:

Two requests:

  1. Add Fixes #29244 to the description, so that the merge closes the issue.

  2. Consider the node:test file from Honor live $HOME mutations in os.homedir() #29248. In review there, alii asked for a test that uses node:test, so that the same file runs in Node.js. The file is test/js/node/os/os-homedir-env.test.js. Each of its 6 cases runs in a child process: HOME changed after and before require("node:os"), HOME from the parent environment, HOME="", HOME deleted (seeded with a sentinel and compared with userInfo().homedir), and userInfo().homedir after a HOME change. It passes 6/6 on this branch and on Node v26.3.0. Without the fix, 4 of 6 fail.

    git fetch https://github.com/oven-sh/bun refs/pull/29248/head
    git checkout FETCH_HEAD -- test/js/node/os/os-homedir-env.test.js

    os: return empty-but-set $HOME from homedir(); read userInfo().homedir from passwd #34629 tested one thing that neither test has: HOME set, empty, and unset when the child starts, for both os.homedir() and os.userInfo().homedir. The case below does that in the same file. It passes on this branch and on Node v26.3.0, and it fails without the fix.

    Extra case for os-homedir-env.test.js
    test("homedir() and userInfo().homedir with HOME set, empty, and unset at spawn", { skip: isWindows }, () => {
      // uv_os_homedir: HOME whenever it is present (including ""), passwd only when absent.
      // uv_os_get_passwd (userInfo().homedir): passwd regardless of HOME.
      const source = `
        const os = require("node:os");
        console.log(JSON.stringify({ homedir: os.homedir(), userInfo: os.userInfo().homedir }));
      `;
      const unset = runWithEnv(source, { HOME: undefined });
      const set = runWithEnv(source, { HOME: "/tmp/spawn-set-29244" });
      const empty = runWithEnv(source, { HOME: "" });
      const passwd = unset.userInfo;
      assert.ok(passwd.startsWith("/"));
      assert.deepStrictEqual(
        { unset, set, empty },
        {
          unset: { homedir: passwd, userInfo: passwd },
          set: { homedir: "/tmp/spawn-set-29244", userInfo: passwd },
          empty: { homedir: "", userInfo: passwd },
        },
      );
    });

    If you prefer, I can push both to this branch as one separate commit.

The change has one side effect that differs from Node, and #29248 had it too. In a Worker started with a different HOME in its env option, os.homedir() now returns the worker's value. Node returns the process value there, because libuv reads the C environment and not the worker's copy.

The Buildkite build for this PR (build 115128) is blocked until a maintainer unblocks it. CI has not run yet.

@steipete

Copy link
Copy Markdown
Collaborator Author

Addressed both points from the review:

That integration build contains commit 4bcb6c80082e, whose stable patch ID is identical to this PR head (772e4acb9263). I also ran OpenClaw's adjacent iMessage CLI-path, account, runtime, and remote-host tests against it: 76/76 pass, including uses the real OS account home when HOME is whitespace-only, which was the downstream failure this fixes.

I kept the PR head unchanged: the committed focused regression already covers live assignment, empty HOME, deletion/passwd fallback, and userInfo().homedir independence. The larger Node-compatible matrix is now recorded as cross-runtime integration evidence without duplicating those subprocess cases in this minimal patch.

Fresh P0-P2 review of the exact PR head is clean. The PR description still includes the AI-assisted disclosure.

steipete added a commit to openclaw/bun that referenced this pull request Sep 14, 2026
### What does this PR do?

Integrates the 19 captured upstream compatibility PRs into the OpenClaw Bun fork, retaining their original commits as merge parents. The base is upstream `86771d09fd486a7256790d6f36602b683f7a19de`. This integration is separate from upstream PR review and does not publish a Bun release.

The two stacked PRs also bring their prerequisites: [worker support oven-sh#34424](oven-sh#34424) and [file-URL query handling oven-sh#35601](oven-sh#35601).

| Upstream PR | Captured head |
| --- | --- |
| [42349: fix(sqlite): allow workers to reuse custom library](oven-sh#42349) | `65924882863e` |
| [42374: fix(node:fs): preserve POSIX locks in realpath](oven-sh#42374) | `4df5e0600308` |
| [42446: fix(node:fs): preserve child rm permission errors](oven-sh#42446) | `17d1237bcbac` |
| [42469: fix(runtime): preserve encoded file URL path delimiters](oven-sh#42469) | `cc5b9fb06de9` |
| [42576: fix(node:https): support live secure context updates](oven-sh#42576) | `b8666fde28e6` |
| [42593: fix(worker_threads): preserve async context for worker events](oven-sh#42593) | `f72285db962b` |
| [42594: fix(node:https): wrap injected raw connections with TLS](oven-sh#42594) | `82a9d26cf2cc` |
| [42599: fix(node:os): observe runtime HOME changes](oven-sh#42599) | `772e4acb9263` |
| [42600: fix(worker_threads): preserve cloned error metadata](oven-sh#42600) | `7254eaec568c` |
| [42601: fix(node:path): honor replaced process.cwd](oven-sh#42601) | `e040ec4cf1c0` |
| [42607: fix(process): allow clearing exitCode](oven-sh#42607) | `bacfa9ee3cb3` |
| [42610: fix(node:http): uncork reused upgrade sockets](oven-sh#42610) | `33f89359c50a` |
| [42614: fix(node): resolve listen hosts before binding](oven-sh#42614) | `5b9ab5644122` |
| [42616: fix(node:module): synchronize builtin ESM exports](oven-sh#42616) | `aa78523549c1` |
| [42620: fix(worker_threads): apply execArgv preloads](oven-sh#42620) | `60fbb60c9a16` |
| [42621: fix(node:async_hooks): report timer lifecycles](oven-sh#42621) | `6e044db91d6b` |
| [42622: fix(node:http): align shutdown transport lifecycle](oven-sh#42622) | `98d5f813e8fe` |
| [42635: fix(node:fs): preserve Win32 semantics in recursive mkdir checks](oven-sh#42635) | `891eb8df52f3` |
| [42636: fix(runtime): derive data URL loaders from MIME](oven-sh#42636) | `4570e105f422` |

Integration repairs preserve newer upstream loop-init error handling, use current Rust loader/string-view interfaces, coordinate WORKER init hook mutations with timer/nextTick dispatch, apply TLS context updates made during pending listen, retain draining native listeners for force-close, and preserve literal filename delimiters across ESM/CommonJS resolution and lookup paths. Superseded C++ CommonJS key reconstruction is removed in favor of the shared resolver owner.

### How did you verify your code works?

- Fresh optimized macOS arm64 build: 1,687 passed, 37 existing skips, one existing todo, zero failures across the 22 selected suites, including standalone compilation.
- Debug/ASAN build and focused integration regressions passed. Its earlier full run passed 1,681 tests but hit an inherited standalone-compilation fixture limitation: the large debug template exceeded that test budget, and relocated output needs its ASAN sidecar. The optimized run covers that production flow; no sanitizer setting, test timeout, or skip was weakened.
- Ten directly affected vendored Node conformance files passed with retries disabled.
- All twelve Rust targets passed: zero failed and zero skipped. These are compilation checks, not native execution claims for every target.
- Oxlint, root TypeScript, Rust formatting, and `git diff --check` passed.
- Independent review is clean through P2. Confirmed integration regressions were repaired; an empty-query/fragment review claim was rejected using actual Node 26.8.2 behavior and protected by a regression.
- Repeated recursive-directory testing keeps its 200 optimized-build iterations and descriptor-leak checks, with a fixed nested fixture instead of scanning the growing source tree.

### Final CI corrections

The follow-up removes MIME decoding and response cork adapters whose last callers were replaced by the integrated PRs, documents raw-slice ownership immediately above the unsafe operations, and sorts HTTP exports. Workspace Clippy and formatting pass locally. The final debug/ASAN check passes 392 tests across the data-URL, worker-thread, and HTTP suites, with one existing skip and no failures. Independent review of this follow-up is clean through P2.

The first CI run also exposed two fork-service limitations: the issue-linking bot has no Anthropic credentials, and autofix.ci cannot push formatter changes without its GitHub App. The formatting change was applied locally.

Mordant's advisory `unchecked_construction` warning points to the existing server reload assignment of `user_routes_to_build`. That assignment moves fields from `new_config`, which `on_reload` obtains through `ServerConfig::from_js` before calling `on_reload_from_zig`; the integrated TLS setter also parses its replacement through `SSLConfig::from_js`. This is not an unchecked user-input path. Its baseline and enforcement were left intact; the three unused-helper findings were repaired.

The final optimized macOS arm64 build passes all four affected suites: **433 passed, one existing skip, zero failures** in 9.11 seconds, including standalone compilation. This supplements the initial 22-suite run (1,687 passed), ten vendored Node conformance files, and twelve Rust compilation targets. The final cleanup also passes **392 debug/ASAN tests** and workspace Clippy.

The reload validation path discussed above is visible at [ServerConfig::from_js before reload](https://github.com/openclaw/bun/blob/597b78c2c4b6a0e15b4b1724ab0e5ebff80f5678/src/runtime/server/server_body.rs#L2262), while [the flagged assignment](https://github.com/openclaw/bun/blob/597b78c2c4b6a0e15b4b1724ab0e5ebff80f5678/src/runtime/server/server_body.rs#L2208) transfers that parsed configuration.

Final hosted validation on `597b78c2c4b6a0e15b4b1724ab0e5ebff80f5678`: formatting, JavaScript/source lint, TypeScript types, package tests, Clippy, Miri, and lol-html tests passed. The [Rust workflow](https://github.com/openclaw/bun/actions/runs/34808804630) succeeded; its advisory Mordant job retains only the documented reload-validation false positive.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os.homedir() uses process-start HOME snapshot instead of current process.env.HOME

2 participants