SyntheticModuleRecord: support a live exports source for namespace reads

[robobun]

Adds an optional backing object to SyntheticModuleRecord so Bun's mock.module / loader:"object" path can keep accessor exports live when accessed via the module namespace object.

Problem

Bun's mock.module(id, () => ({ get foo() { ... } })) materializes the factory's properties into a SyntheticModuleRecord's JSModuleEnvironment slots at creation time. Since those slots hold plain JSValues and JSModuleNamespaceObject::getOwnPropertySlotCommon reads them via environment->variableAt(scopeOffset), the accessor is evaluated once and frozen. See oven-sh/bun#9874.

Change

• SyntheticModuleRecord gains WriteBarrier<JSObject> m_liveExportsSource (visited in visitChildrenImpl).
• tryCreateWithExportNamesAndValues accepts a trailing exportValue with no matching exportName as the backing object. The slot-population loop already iterates by exportNames.size(), so existing callers are unchanged.
• JSModuleNamespaceObject::getOwnPropertySlotCommon checks for a SyntheticModuleRecord with liveExportsSource(); when present, the read is forwarded to source->get(propertyName) and returned as a plain uncacheable value (not setValueModuleNamespace), so the JIT's ModuleNamespaceAccessCase IC (and the DFG/FTL lowering to GetClosureVar that hangs off it) is never installed for that property.

The module environment slots still hold the first-read snapshot, so static import { x } bindings (which all tiers read as a raw variableAt(offset) load) continue to see a plain value.

All under USE(BUN_JSC_ADDITIONS).

Companion bun change: oven-sh/bun#36677.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 6 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 710451af-4e5a-4744-8e92-33c949789761

📥 Commits

Reviewing files that changed from the base of the PR and between 45e21dc and 1b4afe5.

📒 Files selected for processing (6)

• Source/JavaScriptCore/bytecode/ModuleNamespaceAccessCase.cpp
• Source/JavaScriptCore/bytecode/ModuleNamespaceAccessCase.h
• Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
• Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp
• Source/JavaScriptCore/runtime/SyntheticModuleRecord.cpp
• Source/JavaScriptCore/runtime/SyntheticModuleRecord.h

---

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

[claude]

🔴 The live-source lookup uses propertyName (the export name on this namespace) instead of exportEntry.localName (the binding name in the target synthetic module). When another module does export { foo as bar } from './mocked', propertyName is bar but the source object only has foo, so the read returns undefined — a regression vs. the environment-slot fallback. Additionally, this branch must be skipped when exportEntry.localName == vm.propertyNames->starNamespacePrivateName: for export * as X from './mocked' the resolved local name is the private star-namespace symbol, which the user's factory object cannot have, so nsB.X also becomes undefined instead of the mocked module's namespace object.

Extended reasoning...

What the bug is

m_exports on a JSModuleNamespaceObject maps this namespace's export names to ExportEntry { localName, moduleRecord }, where moduleRecord / localName are the resolved binding after following re-export chains (see the constructor, which stores resolution.localName and resolution.moduleRecord). The new live-source branch reads the source object with propertyName — the key on this namespace — instead of exportEntry.localName — the key in the target synthetic module's environment (and thus on its backing object).

For direct access on the synthetic module's own namespace these happen to coincide, because tryCreateWithExportNamesAndValues calls addExportEntry(ExportEntry::createLocal(exportName, exportName)). They diverge as soon as another module re-exports from the mocked module.

Step-by-step: renamed re-export

1. ./mocked is a SyntheticModuleRecord created via mock.module(id, () => ({ get foo() { ... } })) with a liveExportsSource that has a foo accessor.
2. Module B contains export { foo as bar } from './mocked'.
3. resolveExportImpl walks the Indirect entry on B, enqueues (mockedRecord, 'foo'), and resolves at the synthetic module's Local entry to Resolution{ Resolved, moduleRecord: mockedSyntheticRecord, localName: 'foo' }.
4. B's namespace stores m_exports['bar'] = { localName: 'foo', moduleRecord: mockedSyntheticRecord }.
5. Reading nsB.bar enters getOwnPropertySlotCommon with propertyName == 'bar' and exportEntry.localName == 'foo'.
6. dynamicDowncast<SyntheticModuleRecord>(exportEntry.moduleRecord.get()) succeeds, liveExportsSource() is non-null, and the code executes source->get(globalObject, propertyName) → source.bar → undefined.

Before this PR, step 6 fell through to setValueModuleNamespace with the environment slot value (the snapshot of foo), so this is a regression, not merely an incomplete feature. Note that the immediately preceding line already uses the correct key: getValue(environment, exportEntry.localName, scopeOffset).

Step-by-step: export * as X

1. Module B contains export * as X from './mocked'.
2. resolveExport / getModuleNamespace produce Resolution{ Resolved, moduleRecord: mockedSyntheticRecord, localName: starNamespacePrivateName } (AbstractModuleRecord.cpp handling for star-namespace bindings).
3. B's namespace stores m_exports['X'] = { localName: starNamespacePrivateName, moduleRecord: mockedSyntheticRecord }.
4. Reading nsB.X: the earlier if (exportEntry.localName == starNamespacePrivateName) block materializes the namespace, and getValue(environment, starNamespacePrivateName, ...) correctly fetches the mocked module's JSModuleNamespaceObject from the slot.
5. But then the live-source branch matches (exportEntry.moduleRecord is the synthetic record with a live source) and calls source->get(globalObject, 'X') on the user's plain factory object → undefined.

Fixing case 1 by switching to exportEntry.localName does not fix case 2: it would call source->get(starNamespacePrivateName), a private symbol the user object cannot have, still yielding undefined.

Why nothing prevents it

The branch is guarded only by dynamicDowncast<SyntheticModuleRecord>(exportEntry.moduleRecord) and liveExportsSource(). It does not check whether this namespace is the synthetic module's own namespace, nor whether the resolved local name is the star-namespace sentinel. Because exportEntry.moduleRecord is the resolved target record (not m_moduleRecord), any namespace whose export chain terminates at the mocked module hits this path.

Impact

Both cases return undefined where the pre-PR code returned the correct value (snapshot for the rename case, the namespace object for the star case). This is user-visible incorrect behavior for anyone who re-exports from a mock.module'd module — a common pattern when mocking a package that is itself re-exported through a barrel file.

Suggested fix

if (auto* synthetic = dynamicDowncast<SyntheticModuleRecord>(exportEntry.moduleRecord.get())) {
    if (JSObject* source = synthetic->liveExportsSource();
        source && exportEntry.localName != vm.propertyNames->starNamespacePrivateName) [[unlikely]] {
        slot.disableCaching();
        JSValue liveValue = source->get(globalObject, exportEntry.localName);
        RETURN_IF_EXCEPTION(scope, false);
        slot.setValue(this, static_cast<unsigned>(PropertyAttribute::DontDelete), liveValue);
        return true;
    }
}

i.e. (a) look up by exportEntry.localName, and (b) skip the branch entirely for the star-namespace sentinel so it falls through to setValueModuleNamespace with the already-correct slot value.

[claude]

The hasOwn fallback in 3cf0ca7 removes the undefined regression for both the renamed-re-export and export * as X cases (the source lacks bar/X, so we drop through to the env slot), but the lookup key is still propertyName rather than exportEntry.localName, so the underlying wrong-key issue remains:

• Live behavior is lost through a rename. export { foo as bar } from './mocked' → nsB.bar misses on source.bar and returns the env-slot snapshot, while nsMocked.foo returns the live value. The feature silently stops working across a barrel/re-export.
• Wrong value when the outer name collides. If the mocked module also exports bar, the source does own bar, so hasOwn is true and nsB.bar returns source.bar — but the resolved binding is foo. That's a correctness bug, not just a stale snapshot.

Switching both getOwnPropertySlot and getValue calls to exportEntry.localName fixes both. With that change the hasOwn fallback also happens to cover the star-namespace case (the source can't own starNamespacePrivateName), so the explicit sentinel guard from the original suggestion becomes optional — though keeping it saves a pointless method-table dispatch on every export * as X read.

[github-actions]

Preview Builds

Commit	Release	Date
1b4afe55	autobuild-preview-pr-380-1b4afe55	2026-08-01 18:10:42 UTC
abed2dbe	autobuild-preview-pr-380-abed2dbe	2026-08-01 16:36:32 UTC
3cf0ca7b	autobuild-preview-pr-380-3cf0ca7b	2026-08-01 15:02:37 UTC
1ee717ba	autobuild-preview-pr-380-1ee717ba	2026-08-01 09:44:05 UTC
a59646ca	autobuild-preview-pr-380-a59646ca	2026-08-01 07:52:45 UTC

[claude]

🔴 The write-through uses name (the export name on this namespace) instead of resolution.localName (the binding name in the target synthetic module — the same key the env-slot write on line 470 already uses). With export { foo as bar } from './mocked', an override on B's namespace with name='bar' writes a stray source.bar while source.foo stays stale, so once the read side is fixed to use localName the override is silently lost for namespace reads. Separately, putDirect is a fragile primitive for a user-supplied backing object: it asserts !parseIndex(propertyName) (numeric export names like "0" trip a debug ASSERT and desync named/indexed storage in release), it overwrites an accessor's GetterSetter in place and transitions it to a plain data property (destroying the live getter this feature exists to preserve), and it is non-virtual so a Proxy source's traps are bypassed and the write is invisible to the read side's source->get() — consider a method-table put (or skip the write-through when the existing own descriptor is an accessor) and handle the index case.

Extended reasoning...

What the bug is

overrideExportValue follows re-export chains via resolveExport, so record is resolution.moduleRecord — potentially a different module than the one whose namespace was passed in — and the binding key inside that target module is resolution.localName. The env-slot write on line 470 gets this right:

symbolTablePutTouchWatchpointSet(moduleEnvironment, globalObject, resolution.localName, value, ...);

but the new live-source write-through on line 483 uses name — the export name on this namespace — instead:

source->putDirect(vm, name, value, 0);

The source object's keys are the synthetic module's own export names (its export entries are createLocal(exportName, exportName), so localName == exportName == the property key on the factory object). When name != resolution.localName, the write lands on a key the read side never looks at.

Step-by-step: renamed re-export

1. ./mocked is a SyntheticModuleRecord created via mock.module(id, () => ({ get foo() { ... } })) with a liveExportsSource that has foo.
2. Module B contains export { foo as bar } from './mocked'.
3. spyOn(nsB, 'bar') (or any Bun-side caller) invokes overrideExportValue(globalObject, 'bar', spy) on B's namespace.
4. resolveExport walks B's Indirect entry and returns { moduleRecord: mockedSyntheticRecord, localName: 'foo' }.
5. Line 470 correctly writes the env slot foo in the mocked module's environment.
6. Line 483 executes source->putDirect(vm, 'bar', spy, 0) — a stray bar property is added to the factory object; source.foo is untouched.
7. Once the read-side bug at line 216 is fixed to look up source[exportEntry.localName], reading nsB.bar forwards to source.foo — which still holds the original getter — and the spy is never observed via the namespace.

Even without the line-216 fix this is observable today: after step 6, reading nsMocked.foo on the mocked module's own namespace (where propertyName == 'foo') forwards to source.foo and returns the stale value, while the env slot already holds the spy — the two paths this hunk was added to keep consistent have diverged.

This is the write-side analogue of the read-side issue already flagged at line 216, but at a distinct code location and needing its own fix: use resolution.localName instead of name.

Secondary: putDirect is the wrong primitive here

While touching this line, note that putDirect(VM&, PropertyName, JSValue, unsigned) (JSObject.h:1231 → putDirectInternal<PutModeDefineOwnProperty>) makes assumptions the user-supplied backing object need not satisfy:

• Numeric export names. putDirectInternal has ASSERT(!parseIndex(propertyName)) (JSObjectInlines.h:501) and the header documents "the property name is assumed to not be an index". A factory like mock.module(id, () => ({ '0': v })) or () => ['a','b'] yields export '0'; overrideExportValue on that key trips the debug ASSERT and, in release, adds '0' to named storage while the live value sits in indexed storage — the read side's source->get(globalObject, '0') parses the index and misses the write. Switching to resolution.localName does not help (it's still '0'). putDirectMayBeIndex (JSObject.cpp:3951) or a parseIndex branch handles this.

• Accessor destruction. If source has get foo() { ... } — the exact shape this PR exists to support — putDirectInternal finds the existing offset with PropertyAttribute::Accessor, overwrites the GetterSetter cell via putDirectOffset, and (since newAttributes(0) != currentAttributes) takes Structure::attributeChangeTransition to a plain data property (JSObjectInlines.h:588–603). The live accessor is gone permanently; a later restore that writes the original snapshot back through the same path leaves ns.foo frozen — the pre-PR behavior this feature was meant to eliminate. If clobbering is the intended spyOn semantic that's fine, but if not, skip the write-through when the existing own descriptor is an accessor.

• Proxy sources. liveExportsSource accepts any JSObject, so mock.module(id, () => new Proxy(target, { get(t,k){...} })) reaches here. putDirect is non-virtual and writes to the ProxyObject cell's own structure/butterfly, never invoking [[Set]]/[[DefineOwnProperty]] traps or touching the target. The read side's source->get() dispatches through ProxyObject::getOwnPropertySlot → the get trap → the target, so the value just written is invisible and the write-through is silently lost. A method-table-dispatched put (e.g. source->methodTable()->put(...) with a strict PutPropertySlot, or defineOwnProperty) would respect traps.

These three are edge cases and arguably nit-level on their own, but they all flow from the same choice of low-level define-own-property on an object whose shape the engine does not control, and can be addressed in the same one-line hunk as the localName fix.

(Minor: putDirect(VM&, ...) never throws a JS exception, so the RETURN_IF_EXCEPTION on line 484 is dead.)

[claude]

🟡 The sibling batch API overrideExports() (lines 426-448) performs the same env-slot write via symbolTablePutTouchWatchpointSet but was not given the matching write-through to liveExportsSource() added here — so a bulk override on a live-source synthetic module updates only the env slot while namespace reads (lines 211-224) keep returning the source object's stale value. Before this PR both read and write agreed on the env slot, so this is a regression for that path; mirror this block inside the overrideExports() loop after line 441, keyed on pair.value.localName and dynamicDowncast<SyntheticModuleRecord>(pair.value.moduleRecord.get()). (Distinct from the wrong-key issue already noted on this hunk — here the write-through is absent from a separate function.)

Extended reasoning...

What the bug is

Commit 1ee717b added a write-through to the live-exports backing object in overrideExportValue() (lines 488-493) precisely because this PR redirected namespace reads on a SyntheticModuleRecord with a liveExportsSource away from the env slot and onto the source object (lines 211-224). But the sibling batch API overrideExports() at lines 426-448 — which performs the identical env-slot write via symbolTablePutTouchWatchpointSet at line 441 — was not given the same write-through. Its writes are now invisible to namespace reads whenever the target export resolves to a live-source synthetic module.

Step-by-step

1. mock.module('m', () => ({ get foo() { return orig } })) creates a SyntheticModuleRecord whose m_liveExportsSource is the factory object with an own foo accessor, and whose env slot foo holds the first-read snapshot.
2. Bun-side code calls ns->overrideExports(...) to bulk-replace exports (this Bun-specific API exists for re-mock / HMR paths). For the foo entry, pair.value.moduleRecord is the mocked SyntheticModuleRecord and pair.value.localName == 'foo'.
3. Line 441 writes value into the env slot for foo. The source object's own foo accessor is untouched.
4. A subsequent ns.foo read enters getOwnPropertySlotCommon, reaches line 211, dynamicDowncast<SyntheticModuleRecord> succeeds, liveExportsSource() is non-null, and source->getOwnPropertySlot(..., 'foo', ...) finds the old own accessor → hasOwn == true.
5. Line 218 returns sourceSlot.getValue(...) — the original getter's result — and never consults the env slot. The override is silently lost.

Because pair.value.moduleRecord is the resolved target record (stored from resolution.moduleRecord in the constructor), this applies whether overrideExports() is called on the mocked module's own namespace or on a namespace that re-exports from it.

Why this is a regression introduced by this PR

Before this PR, both the write path (symbolTablePutTouchWatchpointSet → env slot) and the read path (getValue(environment, exportEntry.localName, scopeOffset) → env slot) agreed, so overrideExports() worked correctly on synthetic modules. This PR moved the read path to the source object but only taught overrideExportValue() to write there. The author's own commit message for 1ee717b ("overrideExportValue: write through to the live-exports source") states exactly the invariant being broken here for the batch variant.

Not a duplicate

The two existing comments on this PR are (a) line 223 — read side uses propertyName instead of exportEntry.localName, and (b) line 493 — overrideExportValue's write-through uses the wrong key / putDirect primitive. This finding is about a separate function where the write-through is absent entirely, and needs its own fix regardless of how (b) is resolved.

Fix

Mirror the block at 488-493 inside the overrideExports() loop, after line 441:

if (auto* synthetic = dynamicDowncast<SyntheticModuleRecord>(pair.value.moduleRecord.get())) {
    if (JSObject* source = synthetic->liveExportsSource())
        source->putDirect(vm, pair.value.localName, value, 0); // subject to the same putDirect caveats noted at line 493
}

(keyed on pair.value.localName, since that is the binding name in the target synthetic module — the same key line 441 already uses for the env-slot write).

Severity note

Marked nit rather than blocking because overrideExports() has no in-tree callers — it is invoked from the Bun side, and whether the companion change (oven-sh/bun#36677) exercises it against a live-source synthetic module cannot be confirmed from this repo. If it does, this is a user-visible regression and should be treated as blocking; either way the JSC-side inconsistency is real and cheap to fix alongside the line-493 change.