fix(cjs): expose live exports at CommonJS cycle re-entry

[proggeramlug]

Summary

An esbuild CommonJS module assigns module.exports = __toCommonJS(target) before requiring its cyclic peer. The owner record contains the getters, but the path registry retained the initial empty exports object. The peer kept that stale object and an indirect call later threw TypeError: value is not a function. Export-name extraction was already working; #10156's live named getters did not address this replaced object.

Changes

• Publish the existing module record at the partial boundary, as at final publication. The existing runtime adapter reads its current exports when require re-enters the module.
• Check speculative circular-property warnings against the returned exports value with in, without invoking getters.
• Add an ESM-imported esbuild CJS cycle fixture that records both views at re-entry, calls through the cycle, changes the live binding, counts getter reads, and asserts empty stderr. Add class-getter and genuine-missing-property controls.
• Keep the publication state machine and runtime ABI unchanged. Publication adds no allocations, scans, copies, or calls; it removes one property read for ordinary modules.

Related issue

Fixes #10178. Refs #10107. Related to #10153 / #10156.

Test plan

All builds, tests, and measurements ran through ./remote.sh on Linux, with two build jobs, matching isolated compiler/static archives, and --no-auto-optimize for native probes.

Baseline production sources were built from 1cd160f3d, with no compiler/runtime edits. Both snapshots used cargo build --release -j2 -p perry -p perry-runtime-static -p perry-stdlib-static; compiler and archives were copied together before test builds could replace them.

Check	Baseline	Fixed
In-tree esbuild fixture	Owner getter exists; recursive view lacks it, identity differs; indirect call throws	Same object and getter at re-entry; calls return 42, then 52; zero early getter reads, two actual reads; empty stderr
Unmodified @ai-sdk/gateway@3.0.104 entry, @vercel/oidc@3.2.0	Exact getVercelDataDir circular-dependency warning	gateway: function, empty stderr
OIDC private path call exposed in an isolated package copy	Consumer getter undefined; value is not a function	Consumer getter callable; correct .../com.vercel.cli/auth.json path; empty stderr
OIDC public getVercelToken() through the opposite cycle entry	Indirect call throws	Returns the isolated test token; empty stderr; no network request
semver 7.8.1 and 7.8.5, comparator-first	ANY warning; symbol and intersection result already correct	No warning; same symbol/intersection behavior
Both semver versions, range-first	No warning; wildcard range matches	Same result and empty stderr

The installed, unmodified gateway entry compiled 125 native modules; the isolated source-copy/call probe compiled 280 native modules plus 57 pruned forwarding modules. Node 26.5.1 passed the same probes with empty stderr. Only the lane-owned OIDC copy received diagnostic globals exposing the private function and owner record; installed packages were untouched. Test auth data stayed under the lane directory.

Commands and results:

• cargo test --release -j2 -p perry --bin perry cjs_wrap: 121 passed.
• cargo test --release -j2 -p perry --test source_graph_export_regressions: 61 passed, including fix(cjs): preserve live getter re-export bindings #10156's sibling cases, all three new controls, and unrelated source-graph regressions.
• CARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_TEST_DEBUG=0 CARGO_INCREMENTAL=0 RUST_TEST_THREADS=1 cargo test -j2 -p perry-runtime: 3,931 passed, 1 failed, 4 ignored. Debug assertions remained enabled; debug info was disabled to bound disk use.
• The remaining failure is native_stack::tests::stack_top_respects_custom_thread_stack_sizes (bound must belong to this worker). It also fails alone. Its source is unchanged from 1cd160f3d; the same Linux failure was reported in fix(cjs): preserve live getter re-export bindings #10156. This PR changes only comments in runtime Rust code.
• An initial release-profile runtime run had 3,929 passed, 2 failed, 4 ignored: the same stack test plus a test that unconditionally expects a disabled debug assertion. The assertion test passed in the default-profile run. Runtime doctests completed successfully with 8 ignored.
• cargo fmt --all -- --check, scripts/check_file_size.sh, scripts/check_test_registration.py, and scripts/check_node_version_consistency.py --list: passed.
• SHA-256 comparison confirmed all 11 final files match the remotely tested files.

Require-heavy benchmark

111.55 ms before → 111.36 ms after (−0.17%, within measurement noise). No measured runtime regression.

The workload has 96 getter-free CommonJS leaves (module.exports = { value: i }) and one CJS driver. For 1,000 rounds the driver sums each literal require("./mN.cjs").value; ESM imports the driver and prints the asserted checksum 4560000. Every fresh process performs module publication and 96,000 cached requires.

Both native executables used the same source, release-profile compiler/archive package set, --no-cache --no-auto-optimize, two module jobs and one LLVM worker per module. Measurements used 51 randomly ordered before/after pairs (seed 10178), three warmups per executable, and CPU affinity 15 on the shared Linux host. Compilation is excluded; process startup and module initialization are included. No getter-specific work was added to publication; the existing registry receives the record instead of reading its initial exports property.

The full OpenCode build and terminal interface were not run, as requested. CI is informational for this lane; this PR does not wait for unrelated main failures.

Checklist

• Added an in-tree regression fixture and registered integration tests.
• Added a changelog fragment.
• No workspace version, CLAUDE.md version, or CHANGELOG.md edits.
• Commit follows the fix: convention; read CONTRIBUTING.md.

Summary by CodeRabbit

• Bug Fixes

  • Fixed CommonJS circular dependencies so modules observe current exports, including replacements made during initialization.
  • Improved compatibility with getter-based exports and live bindings when CommonJS modules are imported from ESM.
  • Refined circular-dependency warnings to avoid unnecessary accessor evaluation and preserve warnings for genuinely missing properties.

• Tests

  • Added regression coverage for CommonJS/ESM cycles, getter-backed exports, live updates, class exports, and expected warning output.

• Documentation

  • Updated documentation describing CommonJS module publication and circular dependency behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 00d3b77a-fcf2-4dce-bd17-1a24e64b0d97

📥 Commits

Reviewing files that changed from the base of the PR and between 1cd160f and 93571fa.

📒 Files selected for processing (11)

• changelog.d/10178-cjs-cycle-exports.md
• crates/perry-codegen/src/lower_call/native/native_runtime_branch.rs
• crates/perry-runtime/src/module_require.rs
• crates/perry-runtime/src/module_require/path_registry.rs
• crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs
• crates/perry/src/commands/compile/cjs_wrap/wrap.rs
• crates/perry/tests/source_graph_export_regressions.rs
• crates/perry/tests/source_graph_export_regressions/issue_10178.rs
• test-files/cjs_esbuild_cycle/consumer.cjs
• test-files/cjs_esbuild_cycle/token.cjs
• test-files/test_cjs_esbuild_cycle.ts

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

---

📝 Walkthrough

Walkthrough

The CJS wrapper now publishes the full module record during partial initialization, so cyclic readers observe current module.exports replacements. Circular-dependency warnings avoid primitive values and accessor invocation. New fixtures test esbuild getters, live bindings, class getters, and missing-property warnings.

Changes

CommonJS cycle export handling

Layer / File(s)	Summary
Runtime cycle handling crates/perry/src/commands/compile/cjs_wrap/wrap.rs, crates/perry-runtime/src/..., crates/perry-codegen/src/..., crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs	Partial registration now passes __cjs_module. Cyclic reads use the current module record. Circular-dependency checks guard null and primitive values and test property absence without invoking accessors. Related documentation and the preamble assertion were updated.
Cycle regression coverage crates/perry/tests/source_graph_export_regressions.rs, crates/perry/tests/source_graph_export_regressions/issue_10178.rs, test-files/cjs_esbuild_cycle/*, test-files/test_cjs_esbuild_cycle.ts, changelog.d/10178-cjs-cycle-exports.md	New tests verify esbuild getter exports, live updates, comparator class getters, and expected missing-property warnings. The test helper now preserves process output for stderr assertions. The changelog records the fix.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant ESM
  participant CJSWrapper
  participant ModuleRegistry
  participant CyclicCJS
  ESM->>CJSWrapper: import token.cjs
  CJSWrapper->>ModuleRegistry: publish module record
  CJSWrapper->>CyclicCJS: require consumer.cjs
  CyclicCJS->>ModuleRegistry: read current module.exports
  ModuleRegistry-->>CyclicCJS: getter-backed exports
  CyclicCJS-->>ESM: live values and cycle results

Loading

Merge Risk: ⚪ Minimal · up to 93571

The CommonJS cycle handling change is covered by targeted regression fixtures for export replacement, getters, warnings, and live bindings. No merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 47.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 10 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: exposing live CommonJS exports during cycle re-entry.
Description check	✅ Passed	The description includes Summary, Changes, Related issue, Test plan, and Checklist sections. It provides detailed behavior, affected scenarios, test results, known baseline failures, and validation co…
Linked Issues check	✅ Passed	The PR addresses #10178. CJS cycle re-entry now publishes the module record before dependency loading, so readers observe the current module.exports value and live esbuild getters. Warning checks us…
Out of Scope Changes check	✅ Passed	The changed runtime and wrapper code directly implements #10178. The documentation, canary assertion, changelog, fixtures, and regression tests support or verify that implementation. No unrelated prod…

Full details: Docstring Coverage

Explanation

Docstring coverage is 47.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 10 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[proggeramlug]

Landed via merge train #10291 (v0.5.1573). All source commits preserve authorship; merged main matches the validated train exactly.