Repository navigation
fix(parser): fix bytecode CJS pragma detection after shebang - #25868
Merged
Merged
Conversation
The pragma parser was using self.lexer.end instead of the local end variable (contents.len) when scanning for @bun-cjs and @bytecode pragmas. This caused the parser to fail detecting pragmas when a shebang was present, as the cursor would exceed the wrong boundary. Files with shebang + bytecode CJS wrapper would silently fail to execute, returning exit code 0 with no output.
Contributor
|
Caution Review failedThe pull request is closed. WalkthroughReplaced loop boundary in Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a parser bug where bytecode CJS pragma detection failed when the source file contained a shebang. The issue caused bundled bytecode output with shebangs to silently exit without executing. The fix corrects the loop boundary check in hasBunPragma() to use the actual content length instead of the lexer's end position.
Key Changes
- Fixed parser to correctly scan for
@bun-cjsand@bytecodetokens after a shebang by usingcontents.leninstead ofself.lexer.end - Added bundler test for source files with shebangs that are bundled with bytecode
- Added regression tests verifying pragma detection and execution with shebangs
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ast/Parser.zig | Changed loop condition in hasBunPragma() to use end (contents.len) instead of self.lexer.end when scanning for pragma tokens |
| test/regression/issue/bun-bytecode-shebang.test.ts | Added regression tests for CJS pragma detection with shebangs and end-to-end bytecode bundling |
| test/bundler/bundler_banner.test.ts | Added bundler test case for source files with shebangs bundled with bytecode and CJS target |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dylan-conway
approved these changes
Jan 7, 2026
dylan-conway
left a comment
dylan-conway
left a comment
Member
There was a problem hiding this comment.
Nice fix. Will merge after CI finishes
dylan-conway
requested changes
Jan 7, 2026
dylan-conway
left a comment
dylan-conway
left a comment
Member
There was a problem hiding this comment.
Actually could you remove the regression test? I think only bundler_banner.test.ts is necessary
Signed-off-by: Kaj Kowalski <info@kajkowalski.nl>
dylan-conway
approved these changes
Jan 7, 2026
dylan-conway
reviewed
Jan 7, 2026
Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/bundler/bundler_banner.test.ts:
- Around line 207-234: In the itBundled test
"banner/SourceHashbangWithBytecodeAndCJSTargetBun" add explicit order assertions
after the snapshot by computing indices from the content (e.g. shebangIndex =
content.indexOf("#!/usr/bin/env bun"), pragmaIndex = content.indexOf("// @bun
@bytecode @bun-cjs"), wrapperIndex = content.indexOf("(function(exports,
require, module, __filename, __dirname) {"), bannerIndex = content.indexOf("//
Copyright 2024 Example Corp")) and assert shebangIndex === 0, pragmaIndex >
shebangIndex, wrapperIndex > pragmaIndex, and bannerIndex > wrapperIndex to
mirror the ordering checks used in BannerWithBytecodeAndCJSTargetBun and give
clearer failure messages.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
test/bundler/bundler_banner.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/bundler/bundler_banner.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/bundler/bundler_banner.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/bundler/bundler_banner.test.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Learnt from: nektro
Repo: oven-sh/bun PR: 22806
File: scripts/runner.node.mjs:687-689
Timestamp: 2025-09-24T05:48:59.872Z
Learning: In the Bun codebase, the `startGroup` utility function in scripts/runner.node.mjs automatically closes any previously open group when called, so `startGroup("End")` correctly closes the final test group and keeps subsequent output ungrouped.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25462
File: src/ast/visitExpr.zig:1644-1695
Timestamp: 2025-12-11T02:11:47.024Z
Learning: In Bun's bundler feature flag implementation (src/ast/visitExpr.zig), the validation for feature() flag names intentionally only rejects UTF-16 strings (checking `is_utf16`) while allowing UTF-8 strings, even though the error message says "must be an ASCII string". This is the intended behavior and should not be changed to enforce strict ASCII validation.
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Use `normalizeBunSnapshot` to normalize snapshot output of tests
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures rather than tests themselves.
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Organize unit tests by module in directories like `/test/js/bun/` and `/test/js/node/`.
Applied to files:
test/bundler/bundler_banner.test.ts
📚 Learning: 2025-09-30T22:53:19.887Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23117
File: src/bun.js/test/snapshot.zig:265-276
Timestamp: 2025-09-30T22:53:19.887Z
Learning: In Bun's snapshot testing (src/bun.js/test/snapshot.zig), multiple inline snapshots at the same line and column (same call position) must have identical values. However, multiple inline snapshots on the same line at different columns are allowed to have different values. The check is position-specific (line+col), not line-wide.
Applied to files:
test/bundler/bundler_banner.test.ts
🧬 Code graph analysis (1)
test/bundler/bundler_banner.test.ts (1)
test/bundler/expectBundled.ts (1)
itBundled(1763-1797)
Comment on lines
+207
to
+234
| itBundled("banner/SourceHashbangWithBytecodeAndCJSTargetBun", { | ||
| banner: "// Copyright 2024 Example Corp", | ||
| format: "cjs", | ||
| target: "bun", | ||
| bytecode: true, | ||
| outdir: "/out", | ||
| minifyWhitespace: true, | ||
| backend: "api", | ||
| files: { | ||
| "/a.js": `#!/usr/bin/env bun | ||
| module.exports = 1; | ||
| console.log("bun!");`, | ||
| }, | ||
| onAfterBundle(api) { | ||
| const content = api.readFile("/out/a.js"); | ||
| // Shebang from source should come first, then @bun pragma | ||
| expect(content).toMatchInlineSnapshot(` | ||
| "#!/usr/bin/env bun | ||
| // @bun @bytecode @bun-cjs | ||
| (function(exports, require, module, __filename, __dirname) {// Copyright 2024 Example Corp | ||
| module.exports=1;console.log("bun!")}) | ||
| " | ||
| `); | ||
| }, | ||
| run: { | ||
| stdout: "bun!\n", | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding explicit order verification.
The test includes a comment (line 222) emphasizing that "Shebang from source should come first, then @Bun pragma," which suggests order is critical for this bug fix. Consider adding explicit order checks similar to the BannerWithBytecodeAndCJSTargetBun test (lines 173-179) to make the ordering requirement more explicit and provide clearer failure messages:
// After the snapshot assertion
const shebangIndex = content.indexOf("#!/usr/bin/env bun");
const pragmaIndex = content.indexOf("// @bun @bytecode @bun-cjs");
const wrapperIndex = content.indexOf("(function(exports, require, module, __filename, __dirname) {");
const bannerIndex = content.indexOf("// Copyright 2024 Example Corp");
expect(shebangIndex).toBe(0);
expect(pragmaIndex).toBeGreaterThan(shebangIndex);
expect(wrapperIndex).toBeGreaterThan(pragmaIndex);
expect(bannerIndex).toBeGreaterThan(wrapperIndex);This would provide more specific failure messages if the ordering regresses and be consistent with similar tests in the file.
🤖 Prompt for AI Agents
In @test/bundler/bundler_banner.test.ts around lines 207 - 234, In the itBundled
test "banner/SourceHashbangWithBytecodeAndCJSTargetBun" add explicit order
assertions after the snapshot by computing indices from the content (e.g.
shebangIndex = content.indexOf("#!/usr/bin/env bun"), pragmaIndex =
content.indexOf("// @bun @bytecode @bun-cjs"), wrapperIndex =
content.indexOf("(function(exports, require, module, __filename, __dirname) {"),
bannerIndex = content.indexOf("// Copyright 2024 Example Corp")) and assert
shebangIndex === 0, pragmaIndex > shebangIndex, wrapperIndex > pragmaIndex, and
bannerIndex > wrapperIndex to mirror the ordering checks used in
BannerWithBytecodeAndCJSTargetBun and give clearer failure messages.
dylan-conway
reviewed
Jan 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fix bytecode CJS pragma detection when source file contains a shebang.
When bundling with
--bytecodeand the source file has a shebang, the output silently fails to execute (exits 0, no output).Reproduction: github.com/kjanat/bun-bytecode-banner-bug
The pragma parser in
hasBunPragma()correctly skips the shebang line, but usesself.lexer.endinstead ofcontents.lenwhen scanning for@bun-cjs/@bytecodetokens. This causes the pragma to not be recognized.Fix:
Where
endis already defined ascontents.lenat the top of the function.How did you verify your code works?
banner/SourceHashbangWithBytecodeAndCJSTargetBunintest/bundler/bundler_banner.test.tstest/regression/issue/bun-bytecode-shebang.test.tsthat verify: