[GH-ISSUE #564] replay: CJS interop loses live bindings and stale exports on reassignment #290

New issue

Closed

opened 2026-10-15 18:23:31 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:23:31 +00:00

Owner

Originally created by @morluto on GitHub (Oct 5, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/564

Summary

Two replay-sandbox fidelity gaps remain after #539–#541. Both were identified in the same sweep and deliberately deferred; recording them here so they are not rediscovered as if new.

Findings

1. SyntheticModule snapshots CommonJS exports and invokes getters eagerly — src/replay/JavaScriptReplayWorker.ts:221-232. The synthetic module copies exports[name] once at link time. Two consequences:

Later mutation of exports is invisible to an importing ESM module, so live bindings are lost.
A getter is invoked during linking even if the importing module never reads that export. A require.d(exports, { lazy: () => { throw … } }) helper therefore fails the whole import, when Node would only throw if the binding were actually accessed.

2. module.exports = … reassignment and circular requires return a stale object — src/replay/JavaScriptReplayWorker.ts:188-214. commonJsCache.set(alias, module.exports) caches the initial {} before the factory runs. A factory that reassigns module.exports (a very common pattern) leaves circular requirers holding the original empty object, and the cache type claims Record<string, unknown> even when the value is a function.

3. Non-enumerable and Symbol exports are not enumerated — Object.keys misses both, so a module's exported surface is under-reported. Note this is the enumeration side of #540, which addressed the projection side; the two together would make symbol-keyed and non-enumerable exports either unrepresentable or invisible depending on which path runs.

Why this matters

Each produces a replay result that differs from real Node behaviour while reporting outcome: "return". That is the failure mode most costly for a differential tool: a confidently wrong answer rather than an explicit unknown. #539 fixed the same class of problem for the seed and the builtin identities.

Suggested direction

Export through accessors on the synthetic module so reads are lazy and reflect current values, mirroring Node's CommonJS-ESM interop.
Re-cache after factory execution and type the cache to admit a function export.
Enumerate Object.keys plus getOwnPropertyNames (and decide explicitly what to do with symbols, consistent with #540).

Note

These interact with the open design question in #543 about how much fidelity the sandbox should attempt. If the decision there is to narrow the sandbox's contract, this issue should be closed as covered rather than implemented.

Originally created by @morluto on GitHub (Oct 5, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/564 ## Summary Two replay-sandbox fidelity gaps remain after #539–#541. Both were identified in the same sweep and deliberately deferred; recording them here so they are not rediscovered as if new. ## Findings **1. `SyntheticModule` snapshots CommonJS exports and invokes getters eagerly** — `src/replay/JavaScriptReplayWorker.ts:221-232`. The synthetic module copies `exports[name]` once at link time. Two consequences: - Later mutation of `exports` is invisible to an importing ESM module, so live bindings are lost. - A getter is invoked during linking even if the importing module never reads that export. A `require.d(exports, { lazy: () => { throw … } })` helper therefore fails the whole import, when Node would only throw if the binding were actually accessed. **2. `module.exports = …` reassignment and circular requires return a stale object** — `src/replay/JavaScriptReplayWorker.ts:188-214`. `commonJsCache.set(alias, module.exports)` caches the initial `{}` before the factory runs. A factory that reassigns `module.exports` (a very common pattern) leaves circular requirers holding the original empty object, and the cache type claims `Record<string, unknown>` even when the value is a function. **3. Non-enumerable and `Symbol` exports are not enumerated** — `Object.keys` misses both, so a module's exported surface is under-reported. Note this is the *enumeration* side of #540, which addressed the *projection* side; the two together would make symbol-keyed and non-enumerable exports either unrepresentable or invisible depending on which path runs. ## Why this matters Each produces a replay result that differs from real Node behaviour while reporting `outcome: "return"`. That is the failure mode most costly for a differential tool: a confidently wrong answer rather than an explicit unknown. #539 fixed the same class of problem for the seed and the builtin identities. ## Suggested direction - Export through accessors on the synthetic module so reads are lazy and reflect current values, mirroring Node's CommonJS-ESM interop. - Re-cache after factory execution and type the cache to admit a function export. - Enumerate `Object.keys` plus `getOwnPropertyNames` (and decide explicitly what to do with symbols, consistent with #540). ## Note These interact with the open design question in #543 about how much fidelity the sandbox should attempt. If the decision there is to narrow the sandbox's contract, this issue should be closed as covered rather than implemented.

bsakaguchi

2026-10-15 18:23:31 +00:00

closed this issue
added the
area: native

bug
labels

bsakaguchi commented

2026-10-15 18:23:32 +00:00

Author

Owner

@morluto commented on GitHub (Oct 5, 2026):

Closing as superseded by merged #572.

The reported CJS/ESM fidelity defects belong to src/replay/JavaScriptReplayWorker.ts. That worker and the controlled replay tool have been removed, so these paths no longer produce misleading replay outcomes. This is removal of the affected feature, not an implementation of the proposed interop fixes. Agents can run experiments with the actual runtime and collect evidence through direct capture tools.

@morluto commented on GitHub (Oct 5, 2026): Closing as superseded by merged [#572](https://github.com/morluto/rea/pull/572). The reported CJS/ESM fidelity defects belong to `src/replay/JavaScriptReplayWorker.ts`. That worker and the controlled replay tool have been removed, so these paths no longer produce misleading replay outcomes. This is removal of the affected feature, not an implementation of the proposed interop fixes. Agents can run experiments with the actual runtime and collect evidence through direct capture tools.

bsakaguchi referenced this issue

2026-10-15 18:24:05 +00:00

[GH-ISSUE #786] [Bug] Windows native boundaries reject ordinary forward-slash drive paths #342

bsakaguchi referenced this issue

2026-10-16 21:05:48 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

bsakaguchi referenced this issue

2026-10-16 21:05:50 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

bsakaguchi referenced this issue

2026-10-16 21:12:42 +00:00

[PR #305] [MERGED] feat: harden authority and runtime conformance boundaries #1941