bake: require app.plugins and framework.plugins to be arrays

[robobun]

Problem

• Bun.serve({ app: { plugins: <non-array> } }), the same key inside app.framework, and bun build --app with either one never check that plugins is an array.
• plugins: 123, plugins: true, plugins: { length: 0 } or a function are accepted and silently do nothing.
• plugins: "abc" or plugins: { name, setup } (one plugin passed without the surrounding array) throw Expected plugin to be an object, which describes the first "element" of the value, not the mistake.
• Cause: both call sites (src/runtime/bake/bake_body.rs:247 for app.plugins, :1097 for framework.plugins) hand the value straight to SplitBundlerOptions::parse_plugin_array, which calls array_iterator() on it (:329). JSArrayIterator works on any value: it reads length and indexes, so a string iterates its characters and a value without a usable length iterates nothing.
• The { name, setup } case only throws today because JSValue::get_length returns 2^51-1 for objects with no length property (the C++ side returns infinity, the Rust side checks for f64::MAX); once that is fixed a single plugin object would be silently ignored too.
• Same check was missing in the Zig version (bake.zig parsePluginArray), so this is long-standing rather than a port regression.

Fix

• parse_plugin_array rejects a non-array with plugins must be an array before doing anything else. Both call sites go through it, so app.plugins, framework.plugins, Bun.serve and bun build --app are all covered by the one check.
• The message and the is_array() test are the ones Bun.build already uses for its plugins option (JSValue::get_array, src/runtime/api/JSBundler.rs:488), so the same mistake now reports the same error in both APIs.
• get_array itself is not used here because it also returns None for an empty array, and plugins: [] has to keep creating the (empty) plugin set: the dev server uses its presence to decide whether to fall back to the bunfig [serve.static] plugins (src/runtime/bake/DevServer.rs:1978).
• The check runs before the plugin set is created, so the new error path does not allocate a GC-protected JSBundlerPlugin of its own. A plugin set already created by a valid framework.plugins is still leaked when a later option (including this new app.plugins error) fails: from_js error paths have never released it, and bake: release the app plugins cell when the dev server is torn down #37837 fixes that class by making the handle release itself on drop.
• app.plugins is now read with the same get_optional_value helper framework.plugins uses, so plugins: null means "not provided" at both sites, as it already does for Bun.build. Without this, app.plugins: null (accepted today) would start throwing.
• Verified with test/bake/dev/plugins.test.ts, app.plugins and framework.plugins must be arrays: one process runs the same seven values through both sites via Bun.serve. A string, a single plugin object, a number and an array-like all get plugins must be an array; [plugin], [] and null are still accepted, and setup() still runs for the real arrays. It fails on the released bun with the accepted/misleading results listed above and passes with this build; the existing tests in the file still pass.
• bun build --app reaches the same UserOptions::from_js (src/runtime/bake/production.rs:395) and is covered by test/bake/dev/production.test.ts, production > rejects a non-array plugins option (a bun.app.ts with plugins: 123): the release build bundles with the option ignored, this build prints TypeError: plugins must be an array and exits 1.
• That test sits in production.test.ts rather than next to the matrix because every bun build --app run trips a pre-existing unchecked-exception assertion in BakeSourceProvider.cpp (BakeGetDefaultExportFromModule) under the exception-check validation the ASAN lane enables, and production.test.ts is the bake file listed in test/no-validate-exceptions.txt. The assertion is independent of this change and is being fixed separately. Run the test alone with -t "rejects a non-array plugins option": the file's other tests build a React app and exceed the default per-test timeout on a debug build.
• Intentionally left out: the other shape checks missing from the same file, bundlerOptions.{server,client,ssr} being objects (bake: validate bundlerOptions values are objects before property access #30125) and fileSystemRouterTypes[i] being objects (bake: validate fileSystemRouterTypes array elements are objects #30401), already have their own open PRs. Note for sequencing: bake: release the app plugins cell when the dev server is torn down #37837 rewrites the body of parse_plugin_array directly below the inserted check, so whichever of the two lands second needs a trivial rebase.

Background

• Bake is the app option of Bun.serve and the bun build --app command. Both parse the user's options object with UserOptions::from_js in bake_body.rs; framework can be the string "react" or an object parsed by Framework::from_js, and both the outer object and the framework object accept a plugins array of Bun bundler plugins.
• The plugins from both arrays are registered into one native JSBundlerPlugin object (created on the first plugins key seen, even an empty one), which every bundle run by the dev server or the production build consults.
• JSArrayIterator is Bun's helper for walking a JS array from native code. It does not require an actual array: it reads a length and indexes, so callers are expected to validate the value first; JSValue::get_array is the helper that does that for object properties.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ee06baf7-d79d-4e7f-a0ae-6169c480191c

📥 Commits

Reviewing files that changed from the base of the PR and between 428fb85 and 7f3a824.

📒 Files selected for processing (1)

• test/bake/dev/plugins.test.ts

💤 Files with no reviewable changes (1)

• test/bake/dev/plugins.test.ts

---

Walkthrough

The bake runtime now skips nullish plugin values and rejects non-array values. Tests cover both plugin configuration sites, plugin setup behavior, and the corresponding build-time error.

Changes

Plugin validation

Layer / File(s)	Summary
Runtime plugin validation src/runtime/bake/bake_body.rs	UserOptions::from_js skips nullish plugins values. parse_plugin_array rejects non-array values with an invalid-arguments error.
Plugin validation regression coverage test/bake/dev/plugins.test.ts	Tests cover valid arrays, null, invalid values, plugin setup behavior, and the bun build --app error and exit status.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly and concisely summarizes the primary change: requiring Bake plugin options to be arrays.
Description check	✅ Passed	The description clearly explains the problem, fix, scope, behavior, and verification details, despite using different headings from the template.

---

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

[robobun]

Reproduced on the released bun (1.4.0) with Bun.serve({ app: { framework, plugins } }) and a minimal framework object, for both app.plugins and framework.plugins:

• plugins: 123, true, { length: 0 }, a function: accepted, server starts, option ignored
• plugins: "abc", { name, setup } (single plugin object), {}, a Set: throws Expected plugin to be an object
• bun build --app with plugins: 123 builds successfully with the option ignored

With this branch every one of those reports plugins must be an array (the Bun.build message); [plugin], [] and null are still accepted.

Tests, both failing on the released binary and passing with this branch:

• test/bake/dev/plugins.test.ts, app.plugins and framework.plugins must be arrays (the value matrix through Bun.serve)
• test/bake/dev/production.test.ts, production > rejects a non-array plugins option (bun build --app)

[claude]

LGTM — adds the missing is_array() guard in the shared parse_plugin_array helper and aligns app.plugins null-handling with framework.plugins.

What was reviewed:

• is_array() uses the same JSType::Array | DerivedArray check as JSValue::get_array (JSBundler's path), so the message and semantics match Bun.build.
• Switching app.plugins from get() to get_optional_value() is required because get() returns Some(null) for plugins: null, which would now fail the new check; verified against JSValue::get at src/jsc/JSValue.rs:1129.
• The guard runs before Plugin::create, so the error path allocates nothing that UserOptions::drop would need to release.
• Tests cover both call sites, both entry points (Bun.serve and bun build --app), and the accepted cases ([], null, [plugin]) alongside the rejected ones.

Extended reasoning...

Overview

Two-line runtime change in src/runtime/bake/bake_body.rs: (1) parse_plugin_array now rejects a non-array value with plugins must be an array before creating the plugin object or iterating; (2) the app.plugins read switches from config.get() to the local get_optional_value() shim so null is treated as unset. Two new subprocess tests in test/bake/dev/plugins.test.ts exercise seven values against both app.plugins and framework.plugins in one process, plus the bun build --app CLI path.

Security risks

None. This tightens input validation on a user-provided options object; nothing new is parsed, allocated, or executed on the added path — it only rejects earlier. No auth, crypto, or filesystem surface touched.

Level of scrutiny

Low-to-medium. Bake is experimental (behind a feature flag and prints a warning), and the change is a defensive guard placed at the shared helper both call sites route through — the recommended layer per REVIEW.md. I confirmed JSValue::is_array() is the same predicate get_array uses (is_cell() && js_type().is_array()), so behavior matches Bun.build's existing plugins validation, and confirmed JSValue::get() returns Some(null) (only filters undefined/missing), which is why the get_optional_value swap is needed to keep plugins: null accepted.

Other factors

The PR description explicitly justifies not using get_array (it filters empty arrays, but plugins: [] must still create the plugin set for the DevServer bunfig-fallback check) — that reasoning checks out against the code. The guard is placed before Plugin::create, so no GC-protected handle leaks on the error path. Tests are hermetic (tempDir, port: 0, subprocess with drained pipes, test.concurrent), assert exact error messages and the positive contract (setupCalls: 2), and were verified by the author to fail on the released binary. print_warning() is only called from bun build --app (not Bun.serve), and the CLI test uses toContain on stderr, so the experimental-warning banner does not interfere with either assertion.

[robobun]

^{Updated 11:40 PM PT - Aug 14th, 2026}

@robobun, your commit 7f3a824 is building: #97518

[claude]

LGTM — small, well-scoped validation fix with matching test coverage.

What was reviewed:

• is_array() is an infallible JSType check (no JS execution / exception scope needed); error message matches Bun.build's existing wording.
• Switching app.plugins to get_optional_value keeps plugins: null accepted (would otherwise start throwing) and now matches the framework.plugins site.
• Confirmed get_array filters empty arrays, so the direct is_array() check is deliberate to preserve plugins: [] creating the plugin set.
• Test covers both sites × 7 value shapes in one subprocess, drains pipes concurrently, asserts setup() still runs for real arrays.

Extended reasoning...

Overview

Two-file change: src/runtime/bake/bake_body.rs gains a 3-line guard at the top of SplitBundlerOptions::parse_plugin_array rejecting non-arrays with plugins must be an array, and UserOptions::from_js reads app.plugins via the existing get_optional_value helper (filtering null/undefined) instead of raw config.get. test/bake/dev/plugins.test.ts gains one test.concurrent case that runs 7 value shapes (string, single plugin object, number, array-like, array, empty array, null) through both app.plugins and framework.plugins in a single spawned process and asserts the full result matrix plus that setup() fired exactly twice.

Security risks

None. This tightens input validation on a config option; is_array() is a pure JSType check that runs no user JS, so there's no new coercion surface or reentrancy.

Level of scrutiny

Low. The guard is placed in the shared helper both call sites already flow through, mirrors the check Bun.build performs for the same option name, and the get_optional_value swap brings app.plugins in line with how framework.plugins was already read. I verified the PR's rationale for not using get_array (it drops empty arrays, and plugins: [] must still create the plugin set for the DevServer's bunfig-fallback decision).

Other factors

The one inline finding is a pre-existing JSBundlerPlugin leak on the framework.plugins: [valid] + app.plugins: <invalid> error path — the leak class predates this PR (already reachable via app.plugins: "abc" and several other inputs) and the fix belongs on SplitBundlerOptions's drop, which is orthogonal. The new test follows the harness conventions (tempDir, bunEnv, port: 0, concurrent pipe drain, stderr/stdout asserted before exitCode) and the PR description documents that it fails on the released binary. No outstanding reviewer comments.

[claude]

LGTM — the four-line guard in parse_plugin_array plus the get_optional_value swap are the minimal, correct fix, and the pre-existing leak note from the earlier pass is resolved (#37837).

What was reviewed:

• is_array() guard placed in the shared helper so both app.plugins and framework.plugins (and both Bun.serve/bun build --app entry points) are covered; message matches Bun.build's.
• get_optional_value swap keeps app.plugins: null accepted (would otherwise regress) and matches the framework.plugins site.
• New plugins.test.ts case: hermetic subprocess, port: 0, drains pipes concurrently, exact-value assertions across 7 values × 2 sites, verifies setup() still runs; production.test.ts case sits alongside existing bun build --app tests using the same Bun.$/bunEnv shape.

Extended reasoning...

Overview

Two-line behavioral change in src/runtime/bake/bake_body.rs: parse_plugin_array now rejects non-array input with plugins must be an array before creating the JSBundlerPlugin or iterating, and UserOptions::from_js reads app.plugins via get_optional_value (filters null/undefined) instead of raw .get(). Two test files gain coverage: test/bake/dev/plugins.test.ts adds a subprocess test exercising 7 value shapes across both app.plugins and framework.plugins, and test/bake/dev/production.test.ts adds a bun build --app test with plugins: 123.

Security risks

None. This tightens input validation on a config option; no new parsing of untrusted data, no auth/crypto/permissions surface. The only observable behavior change is that previously-silent bad config now throws a TypeError at startup.

Level of scrutiny

Low-to-medium. The Rust change is a four-line guard placed at the shared helper (the correct layer per REVIEW.md — fixes both call sites at once), reusing the exact is_array() check and message string Bun.build already uses for the same option. The get_optional_value swap is defensive: without it, app.plugins: null (accepted today) would newly throw, so it preserves existing behavior. get_array was correctly avoided because it also filters empty arrays, and plugins: [] must still create the plugin set (DevServer uses its presence to gate the bunfig fallback). Bake is experimental (per the print_warning in the same file), further lowering the risk of the stricter validation.

Other factors

• My earlier pass flagged the pre-existing SplitBundlerOptions error-path leak as non-blocking; the author confirmed #37837 fixes that class via RAII, resolved the thread, and updated the PR description accordingly. This PR neither introduces nor worsens the mechanism (and actually narrows one trigger: framework.plugins: "abc" now throws before creating the Plugin instead of after).
• Since that review, commit 43a3a6c added the bun build --app test in production.test.ts. It follows the exact shape of the neighboring tests in that file (Bun.$, .env(bunEnv), .throws(false), stderr-then-exitCode assertion order) and errors out at options parsing before any bundling runs, so it does not share the full-build code path the description flagged for the ASAN exception-check lane.
• The plugins.test.ts test is well-constructed per REVIEW.md: tempDir + using, bunEnv/bunExe, port: 0, concurrent pipe draining, exact toEqual on a JSON result object, setupCalls: 2 proves valid arrays still register plugins, and test.concurrent since it's an independent subprocess.
• No bugs surfaced by the bug-hunting system this run.

[robobun]

Ordering note: #38910 makes JSValue::get_length return 0 for objects without a length property, which is the change that turns the accidental rejection described here into a silent no-op, so #38910 is marked as depending on this PR landing first.