Skip to content

Try both module kinds in the patched-bundle parse gate - #1000

Open
mike1858 wants to merge 2 commits into
mainfrom
fix/parse-gate-esm-bundles
Open

mike1858 wants to merge 2 commits into
mainfrom
fix/parse-gate-esm-bundles

Conversation

@mike1858

@mike1858 mike1858 commented Sep 17, 2026

Copy link
Copy Markdown
Member

Why

On 4.3.3, tweakcc --apply cannot apply any customization to a Claude Code install whose bundle is an ES module. Every apply ends with:

✖ The patched bundle failed to parse. Your customizations were not applied; Claude Code is running the original unmodified version.
import{createRequire as vP5}from"node:module";var MP5=Object.create;...
^

SyntaxError: Cannot use import statement outside a module

The parse gate added in #908 writes the fully patched bundle to a temp bundle.cjs and runs node --check on it. node --check chooses its goal symbol — Script or Module — from the file extension alone, so a .cjs file containing a top-level import is rejected however valid the bundle actually is. Claude Code's npm cli.js has shipped "type": "module" since v1.0.20, so the gate fails those installs unconditionally, and correct patches are thrown away with a message that blames the bundle.

Pinning .mjs instead would only move the breakage. Module code is strict-mode and reserves await, so a Bun-compiled native install (@bun-cjs) that happens to contain var await = 1 fails an .mjs check just as reliably:

$ printf 'import fs from "node:fs";\nexport const a = 1;\n' > b.cjs
$ node --check b.cjs
SyntaxError: Cannot use import statement outside a module

$ printf 'var await = 1;\nmodule.exports = await;\n' > c.mjs
$ node --check c.mjs
SyntaxError: Unexpected reserved word

No single extension can validate both bundle shapes, so the gate now checks the mode the bundle looks like first and falls back to the other, treating the bundle as valid when either parse accepts it. The module-kind guess is deliberately not load-bearing — it only picks which mode is tried first, so a bundle misdetected by the heuristic still passes.

Reported by @rgutzen in #981.

What changed

  • assertPatchedBundleParses now runs the node --check probe once per module kind, stopping at the first mode that accepts the bundle. The single-run logic moved into a runParseCheck helper returning parsed / failed / skipped.
  • Added looksLikeEsModule, which looks for a top-level import/export at the start of the file or after a ;, }, or newline. A dynamic import(...) is not counted, since it is legal in CommonJS. The predicate only chooses which mode is attempted first.
  • When both modes reject the bundle, the reported diagnostic is the one from the mode the bundle resembles, so an ESM bundle with a real syntax error reports that error rather than the CommonJS parse's complaint about the import statement.
  • Operational failures keep their existing meaning: an unwritable temp file, a spawn failure, or a timeout warns once and skips the check rather than falling through to the other mode, so an environment problem still never blocks an otherwise-valid apply.
  • Temp files are now named bundle.mjs / bundle.cjs with per-mode stderr captures, inside the same single mkdtemp directory removed in the existing finally.

The CommonJS path is unchanged for bundles that are CommonJS: they are still checked as .cjs, on the first attempt, with the same diagnostic sanitization, the same 30s timeout, and the same skip-on-operational-failure behavior.

Validation

  • pnpm test — 531 passed, 5 skipped, 45 files
  • pnpm linttsc --noEmit && eslint src, clean
  • pnpm build — clean
  • Nine tests added to src/patches/parseGate.test.ts, covering the ESM bundle from tweakcc 4.3.3: new CJS parse-check breaks all ESM bundles (node --check bundle.cjs) #981, CommonJS-only syntax that a module parse rejects, a bundle whose ;import{ lives inside a string literal so the heuristic guesses wrong, the diagnostic chosen when both modes fail, and looksLikeEsModule itself.
  • Confirmed the new tests fail against the pre-fix gate rather than passing vacuously: run against HEAD's parseGate.ts, the ESM bundle is rejected with SyntaxError: Cannot use import statement outside a module, while the fixed gate accepts both it and the CommonJS-only sample.

Out of scope

The second issue raised in #981 is untouched here: native installs on Claude Code ≥ 2.1.247, whose Bun bundle is split into ESM chunks, where extractClaudeJsFromNativeInstallation returns only the ~20 KB /$bunfs/root/cli entry stub and every patch reports "Could not find … in cli.js". That needs the chunk modules to be read and inlined, which is a separate change. This PR only stops the parse gate from rejecting valid ESM bundles.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed bundle validation incorrectly rejecting valid ES module bundles with “Cannot use import statement outside a module.”
    • Bundle checks now recognize both ES module and CommonJS formats and retry using the alternate parsing mode when needed.
    • Improved diagnostics for genuinely invalid bundles, including clearer module-specific errors.
    • Operational parse-check failures are now handled safely without blocking validation.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7d0e7801-1c52-4610-9819-45e029ffc6e6

📥 Commits

Reviewing files that changed from the base of the PR and between 2a4ac73 and 6e5de7d.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/patches/parseGate.test.ts
  • src/patches/parseGate.ts

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


📝 Walkthrough

Walkthrough

The parse gate now detects likely ESM bundles, checks both ESM and CommonJS parse modes, falls back when needed, and reports relevant diagnostics. Tests cover valid bundles, syntax differences, heuristic boundaries, and operational failures.

Changes

Parse gate validation

Layer / File(s) Summary
Module-kind detection and fixtures
src/patches/parseGate.ts, src/patches/parseGate.test.ts
The parse gate maps module modes to .mjs and .cjs files. looksLikeEsModule detects static top-level import and export syntax. Tests cover ESM fixtures, statement boundaries, dynamic imports, and misleading substrings.
Dual-mode parse execution and validation
src/patches/parseGate.ts, src/patches/parseGate.test.ts, CHANGELOG.md
assertPatchedBundleParses tries the likely mode first and falls back to the other mode. runParseCheck classifies parse and operational outcomes. Failed checks report sanitized diagnostics, while operational failures warn and skip. Tests and the changelog record the ESM parsing fix.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant assertPatchedBundleParses
  participant runParseCheck
  participant nodeCheck
  assertPatchedBundleParses->>runParseCheck: try likely module mode
  runParseCheck->>nodeCheck: check mode-specific temporary file
  nodeCheck-->>runParseCheck: parse result or diagnostic
  runParseCheck-->>assertPatchedBundleParses: return or try fallback mode
Loading

Suggested reviewers: xrevenantpulse

Merge Risk: ⚪ Minimal · up to 6e5de

The dual-mode parse gate and its fallback behavior have targeted coverage, with no concrete merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: the parse gate now tries both CommonJS and ES module modes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/parse-gate-esm-bundles

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

A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.

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

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.

1 participant