Skip to content

fix(cli): emit station IDL factory as CommonJS#639

Merged
MRmarioruci merged 1 commit into
mainfrom
fix/cli-station-idl-cjs
Jun 24, 2026
Merged

fix(cli): emit station IDL factory as CommonJS#639
MRmarioruci merged 1 commit into
mainfrom
fix/cli-station-idl-cjs

Conversation

@MRmarioruci

Copy link
Copy Markdown
Contributor

Summary

  • cli/src/audit/generated/station.did.js (vendored from didc bind --target js) uses ES module syntax. The cli package is plain CommonJS.
  • On Node ≤ 20.18 — the version pinned by .nvmrc and the one CI uses — require() of that file throws SyntaxError: Unexpected token 'export' at module-load time. Every orbit-cli invocation fails: registry publish, release publish, audit, even --help.
  • This is also the underlying cause of feat(cli): audit subcommand for station configuration sanity checks #638's e2e regression (disaster-recovery.spec.ts › can recover uninstalled station failing at `execSync(`orbit-cli registry publish --app station`)`).

Motivation

Newer Node versions (≥ 20.20, backported from Node 22's `--experimental-detect-module`) silently auto-detect ESM in `.js` files, which masked the problem locally during development. CI's pinned Node 20.18 has no such fallback.

What changed

  • cli/src/audit/generated/station.did.js — transformed via a single sed substitution: `export const X = ...` → `exports.X = ...`. Two top-level exports affected: `idlFactory` and `init`.
  • cli/package.json (`generate-station-types`) — same sed pipe added to the regen script so future re-generations from `spec.did` don't reintroduce the breakage.

Test plan

  • `pnpm --filter orbit-cli build` — clean.
  • `pnpm --filter orbit-cli test` — 52 vitest cases pass (no test surface touched).
  • `node cli/dist/cli.js registry --help` — loads cleanly (was failing on Node 20.18 with the syntax error).
  • `node cli/dist/cli.js audit --help` — loads cleanly.
  • CI `e2e-tests:required` — should now get past `orbit-cli registry publish` to whatever the actual disaster-recovery test does.

Follow-up to #638.

The generated `cli/src/audit/generated/station.did.js` (produced by
`didc bind --target js`) uses ES module syntax (`export const idlFactory
= ...`), but the cli package is plain CommonJS. On Node ≤ 20.18 (the
version pinned via `.nvmrc`), `require('./generated/station.did.js')`
throws `SyntaxError: Unexpected token 'export'` at module-load time —
breaking every orbit-cli invocation, including `registry publish`,
`release publish`, and the new `audit` subcommand.

Transform the file to `exports.X = ...` form so any require()-loading
Node version accepts it. The transform is one sed substitution and is
wired into the `generate-station-types` script so future re-generations
don't reintroduce the breakage. Local Node 20.20+ silently auto-detects
ESM in .js files, which is why this didn't surface during development.

Follow-up to #638.
@MRmarioruci MRmarioruci requested a review from a team as a code owner June 24, 2026 17:23
@zeropath-ai

zeropath-ai Bot commented Jun 24, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to e7c6f15.

Security Overview
Detected Code Changes
Change Type Relevant files
Refactor ► package.json
    Modify station types generation script to use CommonJS exports
► station.did.js
    Update generated JavaScript file to use CommonJS exports

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a Node 20.18 runtime failure in orbit-cli caused by an ESM-generated Candid JS binding being loaded via require() in a CommonJS package, which prevented all CLI commands from starting.

Changes:

  • Converted the generated Station Candid JS bindings (station.did.js) from export const … to exports.… for CommonJS compatibility.
  • Updated the generate-station-types script to apply the same transformation during regeneration to prevent regressions.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
cli/src/audit/generated/station.did.js Switches top-level exports to CommonJS (exports.idlFactory, exports.init) so Node 20.18 can load the module.
cli/package.json Ensures regenerated JS bindings are emitted as CommonJS via a sed transform in generate-station-types.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MRmarioruci MRmarioruci requested a review from aterga June 24, 2026 18:07
@MRmarioruci MRmarioruci enabled auto-merge (squash) June 24, 2026 18:08
@MRmarioruci MRmarioruci merged commit d23075d into main Jun 24, 2026
30 checks passed
@MRmarioruci MRmarioruci deleted the fix/cli-station-idl-cjs branch June 24, 2026 19:58
aterga added a commit that referenced this pull request Jun 25, 2026
#639 (merged) emits the station IDL factory as CommonJS, so the built
CLI no longer crashes with `Unexpected token 'export'`. Verified after
merging main: `pnpm --filter orbit-cli build` produces a CLI that runs
`audit` directly with no repair.

Removes the now-dead repair step from run-audit.sh, drops the ESM gotcha
and its troubleshooting entry from SKILL.md (four gotchas → three), and
simplifies the "don't trust the global CLI" note accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants