Skip to content

Bun.spawn/spawnSync, Bun.which: inherit live process.env instead of the startup snapshot - #34972

Open
robobun wants to merge 7 commits into
mainfrom
claude/farm/68b2c3e5/spawn-default-env-live-process-env
Open

Bun.spawn/spawnSync, Bun.which: inherit live process.env instead of the startup snapshot#34972
robobun wants to merge 7 commits into
mainfrom
claude/farm/68b2c3e5/spawn-default-env-live-process-env

Bun.spawn/spawnSync, Bun.which: inherit live process.env instead of t…

18f60e1
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 21, 2026 in 22m 24s

Code review found 2 potential issues

Found 2 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/jsc/JSGlobalObject.rs:1170-1174 process_env_object() lacks exception scope for lazy-init path
🟡 Nit test/js/bun/spawn/spawn-env.test.ts:36 Test asserts stderr is exactly empty (REVIEW.md subprocess-test rule)

Annotations

Check warning on line 1174 in src/jsc/JSGlobalObject.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

process_env_object() lacks exception scope for lazy-init path

`process_env_object()` can trigger the `processEnvObject` lazy initializer → `createEnvironmentVariablesMap`, which opens a `DECLARE_THROW_SCOPE`, but neither the new C++ shim nor this Rust wrapper opens a scope or returns `JsResult` — under `BUN_JSC_validateExceptionChecks=1`, if `Bun.which`/`Bun.spawn` is the very first `process.env` access, the next scope-opening call (`.get("PATH")` / `JSPropertyIterator::init`) asserts on the unchecked-exception bit. Other first-access callers wrap this (`c

Check warning on line 36 in test/js/bun/spawn/spawn-env.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Test asserts stderr is exactly empty (REVIEW.md subprocess-test rule)

REVIEW.md → "Subprocess tests" says to assert a combined `{ stdout, stderr, exitCode }` object rather than `expect(stderr).toBe("")` (same at line 66). Swapping to e.g. `expect({ stdout, stderr, exitCode }).toEqual({ stdout: "runtime-value,[unset]", stderr: "", exitCode: 0 })` shows all three values in one diff when the test fails, instead of a bare stderr mismatch with no stdout/exitCode context.