Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe 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. ChangesParse gate validation
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
Suggested reviewers: Merge Risk: ⚪ Minimal · up to 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)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. A rabbit reads each line, Comment |
Why
On 4.3.3,
tweakcc --applycannot apply any customization to a Claude Code install whose bundle is an ES module. Every apply ends with:The parse gate added in #908 writes the fully patched bundle to a temp
bundle.cjsand runsnode --checkon it.node --checkchooses its goal symbol — Script or Module — from the file extension alone, so a.cjsfile containing a top-levelimportis rejected however valid the bundle actually is. Claude Code's npmcli.jshas 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
.mjsinstead would only move the breakage. Module code is strict-mode and reservesawait, so a Bun-compiled native install (@bun-cjs) that happens to containvar await = 1fails an.mjscheck just as reliably: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
assertPatchedBundleParsesnow runs thenode --checkprobe once per module kind, stopping at the first mode that accepts the bundle. The single-run logic moved into arunParseCheckhelper returningparsed/failed/skipped.looksLikeEsModule, which looks for a top-levelimport/exportat the start of the file or after a;,}, or newline. A dynamicimport(...)is not counted, since it is legal in CommonJS. The predicate only chooses which mode is attempted first.importstatement.bundle.mjs/bundle.cjswith per-mode stderr captures, inside the same singlemkdtempdirectory removed in the existingfinally.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 filespnpm lint—tsc --noEmit && eslint src, cleanpnpm build— cleansrc/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, andlooksLikeEsModuleitself.HEAD'sparseGate.ts, the ESM bundle is rejected withSyntaxError: 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
extractClaudeJsFromNativeInstallationreturns only the ~20 KB/$bunfs/root/clientry 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