shell: reassigning an exported variable updates the child environment

[robobun]

What

In Bun Shell, a plain assignment to a variable that is already exported (inherited or set with export) updated only the shell's own view. Child processes spawned afterward still received the original value, and the common PATH=/opt/x/bin:$PATH tool / LC_ALL=C sort idioms silently ran with the stale value.

Repro

import { $ } from "bun";
$.nothrow();
const E = { PATH: "/usr/bin:/bin", OUTER: "fromenv" };
const run = async s => (await $`${{ raw: s }}`.env(E).quiet()).stdout.toString();

// OUTER is exported; POSIX keeps the export attribute on reassignment.
await run("OUTER=changed; /usr/bin/printenv OUTER");
//   before: "fromenv"   (bash: "changed")
await run('OUTER="$OUTER+more"; /usr/bin/printenv OUTER');
//   before: "fromenv"   (bash: "fromenv+more")

echo $OUTER inside the shell reported the new value while the child got the old one, with no diagnostics and exit 0.

Cause

The interpreter keeps three env maps: shell_env (shell-local vars), cmd_local_env (NAME=value cmd prefixes), and export_env (exported/inherited vars). A bare NAME=value always wrote into shell_env (AssignCtx::Shell), but the child environment is built from export_env + cmd_local_env only. So an assignment to an already-exported name landed in shell_env, which children never see, while $NAME expansion (which checks shell_env first) picked up the new value. The two maps disagreed.

Fix

assign_var now checks, for a bare assignment, whether the name already lives in export_env. If it does, it updates the exported binding in place (POSIX: a variable that carries the export attribute keeps it on reassignment). Names that are not already exported still become shell-local, so a fresh NEWV=nv is correctly not exported.

Verification

Added three tests to test/js/bun/shell/bunshell.test.ts under variables:

• reassigning an exported var updates both the shell and the child
• the append idiom VAR="$VAR+more" reaches the child
• a bare assignment to a non-exported name stays shell-local (control)

The first two fail on the released binary and pass with the fix; the control passes both ways.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1d3a4e7c-3f0f-45da-9ee4-9a71681a7340

📥 Commits

Reviewing files that changed from the base of the PR and between 99929fc and 7bb237c.

📒 Files selected for processing (1)

• test/js/bun/shell/bunshell.test.ts

Walkthrough

This PR fixes POSIX shell variable semantics for export and plain assignments. EnvMap gains a remove method; assign_var and the export builtin now update export_env directly for already-exported variables instead of shadowing them in shell_env. Tests cover these interactions.

Changes

Shell export and assignment semantics

Layer / File(s)	Summary
EnvMap remove helper src/runtime/shell/EnvMap.rs	Adds remove method to conditionally delete an entry and deref the stored key/value.
Assignment respects existing exports src/runtime/shell/interpreter.rs	assign_var updates export_env and clears shell_env shadowing when a variable is already exported, rather than always writing to shell_env.
Export builtin conditional insertion src/runtime/shell/builtin/export.rs	Export::start branches on NAME=value vs export NAME, removing shell-local shadows, promoting shell-local values, or preserving/defaulting exported values.
Export/assignment behavior tests test/js/bun/shell/bunshell.test.ts	Adds tests validating reassignment, appending, shadowing removal, and promotion behavior across shell and child-process environments.

Sequence Diagram(s)

sequenceDiagram
  participant Script as Shell Script
  participant Interpreter as ShellExecEnv
  participant ExportBuiltin as Export::start
  participant EnvMap as EnvMap

  Script->>Interpreter: assign_var(NAME, value, AssignCtx::Shell)
  Interpreter->>EnvMap: check export_env for NAME
  alt NAME already exported
    Interpreter->>EnvMap: remove(NAME) from shell_env
    Interpreter->>EnvMap: insert into export_env
  else NAME not exported
    Interpreter->>EnvMap: insert into shell_env
  end

  Script->>ExportBuiltin: export NAME[=value]
  alt NAME=value
    ExportBuiltin->>EnvMap: remove(NAME) from shell_env
    ExportBuiltin->>EnvMap: insert into export_env
  else NAME only
    ExportBuiltin->>EnvMap: check shell_env / export_env
    ExportBuiltin->>EnvMap: promote or preserve value in export_env
  end

Loading

Compact metadata

• Estimated code review effort: High
• Related issues: None found
• Related PRs: None found
• Suggested labels: shell, bug-fix
• Suggested reviewers: None found

Poem

A rabbit hopped through shell and pipe,
Untangling shadows, exports ripe,
No more shy vars hiding in the dark,
Now export NAME leaves a proper mark,
Hop, test, and merge — the burrow's bright! 🐇

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main fix: reassigning an exported variable now updates what child processes receive.
Description check	✅ Passed	The description covers the PR purpose and verification, though it uses different headings than the template.
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.

---

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

[robobun]

^{Updated 9:55 PM PT - Jul 6th, 2026}

❌ @robobun, your commit 7bb237c has 2 failures in Build #69429 (All Failures):

• test/js/sql/local-sql.test.ts - timeout on 🍎 14 x64
• test/js/bun/terminal/terminal.test.ts - code 1 on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 33550

That installs a local version of the PR into your bun-33550 executable, so you can run:

bun-33550 --bun

[github-actions]

Found 1 issue this PR may fix:

1. Bun Shell: prefix assignment of an existing env var duplicates the environ entry (child sees the old value) #32202 - PR fixes the export FOO=first; FOO=second env scenario where bare reassignment of an already-exported variable wasn't propagated to child processes

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #32202

🤖 Generated with Claude Code

[robobun]

Good catch, fixed in f3db27c. The AssignCtx::Shell branch now removes any shell-local entry before writing to export_env, so a stale shell_env value can't shadow the exported one (expansion checks shell_env first). Added a test for the exact sequence:

FOO=a; export FOO=b; FOO=c; echo $FOO now yields shell=c and the child also sees c.

[robobun]

Note on the auto-linked issue: this PR does not fix #32202. That issue is about the VAR=value cmd prefix form duplicating the environ entry in SpawnArgs::fill_env (child ends up with two entries and glibc returns the stale first one). This PR only covers the bare NAME=value assignment path routing to the wrong map. Verified B (HOME=/tmp/zz env) and C (export FOO=first; FOO=second env) from that issue still show duplicate lines with this change, so Fixes #32202 should not be added here.

[robobun]

Fixed in b2561c9. The export builtin now uses the same shadow-clearing logic:

• export NAME=value writes export_env and removes any shell_env shadow, so FOO=a; export FOO=b; echo $FOO prints b (and the child sees b).
• export NAME (no value) promotes an existing shell-local value instead of blanking it, and leaves an already-exported value intact. This also fixes export FOO=b; export FOO which previously blanked FOO to the empty string.

Added four tests covering these forms, all matching bash. The AssignCtx::Exported arm you noted is indeed never constructed (only Cmd and Shell are), so it's left as-is.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/shell/bunshell.test.ts`:
- Around line 1006-1051: These independent subprocess-spawning shell tests
should run concurrently instead of sequentially. Update each of the seven test
declarations in bunshell.test.ts to use test.concurrent while keeping the
existing bodies and env isolation intact, so the cases around
reassigning/exporting FOO and OUTER can execute in parallel without shared-state
issues.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3109313d-dfa7-4641-ba99-8d82dc8b1d16

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and 99929fc.

📒 Files selected for processing (4)

• src/runtime/shell/EnvMap.rs
• src/runtime/shell/builtin/export.rs
• src/runtime/shell/interpreter.rs
• test/js/bun/shell/bunshell.test.ts

[claude]

Both earlier findings are addressed and the refcount flow traces cleanly on every path, but the final shape — manual ref/deref across an unsafe get→remove→insert sequence plus the new export NAME promote/preserve semantics in b2561c9 — is the kind of change a shell maintainer should sign off on rather than me auto-approving.

Extended reasoning...

Overview

Touches four files: adds EnvMap::remove (derefs the owned key/value on swap-remove), reworks ShellExecEnv::assign_var's AssignCtx::Shell arm to route reassignments of already-exported names into export_env and clear any shell-local shadow, and substantially expands the export builtin to (a) clear the shell-local shadow on export NAME=value and (b) handle export NAME with no = by promoting a shell-local value / preserving an already-exported one instead of unconditionally blanking it. Seven new tests in bunshell.test.ts cover shell-view and child-view for each form.

Security risks

None identified. This only changes which of three in-process EnvMaps a shell variable lands in; no new external input parsing, no path handling, no privilege boundaries.

Level of scrutiny

Medium-high. The logic itself is small and well-tested, but it lives in unsafe blocks doing manual EnvStr refcounting — exactly the category CLAUDE.md calls out as most-blocked. I traced every path (including the get→remove→insert→deref ordering in the export NAME promote branch, where the get's +1 keeps the value alive across remove's -1) and the counts balance. But this PR has already needed two correction rounds, and b2561c9 expanded scope beyond the original fix into a semantic redesign of export NAME — that's a design decision a maintainer should confirm, not something to rubber-stamp.

Other factors

Both of my prior inline findings are resolved with dedicated tests. The bug-hunting pass on the latest revision found nothing. fetch_swap_remove exists on ArrayHashMap and returns owned (K, V), so EnvMap::remove is sound. The AssignCtx::Exported arm remains unpatched but is confirmed dead code. CI on 99929fc is still building. Net: I have no outstanding concerns, just not enough confidence in an unsafe-refcounting change with expanded scope to skip human review.

[robobun]

Status: the diff is green. All shell tests pass locally (full test/js/bun/shell/bunshell.test.ts, 393 pass) and the changed files (shell env handling) have no failures in CI.

The only red lane on the latest run (build 69429) is test/napi/napi.test.ts under the "flaky" annotation, which is unrelated to this change and also flakes on main. An earlier run's unrelated failures (windows napi, windows update_interactive_install, aarch64 bun init timeout) were already re-rolled once. Needs a maintainer to merge.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-06, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.