worker_threads: honor --no-addons in execArgv after value-taking flags

[robobun]

What does this PR do?

WebWorker.startVM parses the worker's execArgv to pick up --no-addons. The Zig implementation runs bun.clap.parseEx against the full RunCommand param table with stop_after_positional_at = 1, so value-taking flags like -r <preload>, --title <name>, --port <n> and -e <code> correctly consume their value token before the first positional ends parsing.

The Rust port's parse_worker_exec_argv_allow_addons replaced this with a hand-rolled scanner that breaks at the first token not prefixed with -, without knowing the previous flag may consume that token as its value. For execArgv: ['-r', './preload.js', '--no-addons'] it stops at './preload.js' and never sees --no-addons, so transform_options.allow_addons stays true and process.dlopen is not disabled.

Repro

const { Worker } = require('node:worker_threads');
const w = new Worker(
  `try { process.dlopen({exports:{}}, '/x.node') } catch (e) { require('node:worker_threads').parentPort.postMessage(e.code) }`,
  { eval: true, execArgv: ['-r', './preload.js', '--no-addons'] },
);
w.on('message', m => { console.log(m); process.exit(0); });

Before: prints ERR_DLOPEN_FAILED (addons enabled, dlopen attempted).
After: prints ERR_DLOPEN_DISABLED.

Same divergence with --title, --port, -e, --require, chained shorts (-br), and --prefixed values.

Fix

Generalize bun_clap::ComptimeClap / Args over the argv lifetime so parse_ex accepts a borrowed SliceIterator (the prior 'static bound was why the Rust port hand-rolled the scan). parse_worker_exec_argv_allow_addons is now a direct port of the Zig startVM path: convert the WTF strings to owned UTF-8, run parse_ex against RUN_PARAMS with stop_after_positional_at = 1, read args.flag("--no-addons"). Existing OsIterator callers infer 'static via elision, so no call sites change.

How did you verify your code works?

New parameterised test in test/js/node/no-addons.test.ts covering -r, --require, --title, --port, -e, chained shorts (-br), --prefixed values (-r --), bare -, and the case where --no-addons is itself consumed as a value. Nine of the twelve cases fail on main and all pass with the fix.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Bun's clap argument parser is refactored to accept non-static borrowed argument slices. This enables parse_worker_exec_argv_allow_addons in jsc_hooks.rs to build a locally-owned argv from WTFStringImpl entries and parse it with bun_clap::parse_ex, using the RUN_PARAMS table to correctly handle flags that consume subsequent tokens. A parameterized test suite verifies the corrected behavior across multiple execArgv orderings via an in-memory Worker.

Changes

Worker execArgv --no-addons Flag Parsing Fix

Layer / File(s)	Summary
ComptimeClap lifetime parameterization src/clap/comptime.rs	ComptimeClap<Id> becomes ComptimeClap<'a, Id> with argv-derived slice fields changed from &'static [u8] to &'a [u8]. Parsing methods update ArgIter trait bounds and value accessor return types accordingly.
Public clap API Args container lifetime src/clap/lib.rs	Args and its accessor methods are parameterized with lifetime 'a to propagate borrowed references from the parsed input. Top-level parse and parse_with_table return Args<'static, Id>, while parse_ex is generalized to accept ArgIter<'a> and return ComptimeClap<'a, Id>.
Worker execArgv --no-addons parsing via structured clap src/runtime/jsc_hooks.rs	parse_worker_exec_argv_allow_addons is rewritten to convert WTFStringImpl entries into owned UTF-8 slices, build a SliceIterator, and call bun_clap::parse_ex with RUN_PARAMS and stop_after_positional_at = 1. Flag value consumption is now handled by the parameter table instead of manual loop logic.
Worker execArgv parameterized test matrix test/js/node/no-addons.test.ts	bun:test imports include describe. A test.each suite spawns an in-memory Worker with eval: true to call process.dlopen and post error codes to the parent. Assertions verify correct --no-addons behavior across multiple execArgv orderings, including cases where the flag is consumed as a value by preceding flags.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the primary fix: honoring the --no-addons flag in execArgv after value-taking flags in worker threads.
Description check	✅ Passed	The description covers both required sections: it thoroughly explains what the PR does with concrete examples and how it was verified with a parameterized test.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[robobun]

^{Updated 9:59 PM PT - Jun 15th, 2026}

❌ @alii, your commit 86352d8 has 1 failures in Build #62753 (All Failures):

• test/js/web/fetch/fetch-leak.test.ts - code 1 on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 32362

That installs a local version of the PR into your bun-32362 executable, so you can run:

bun-32362 --bun

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/node/no-addons.test.ts`:
- Around line 43-46: The Promise in the worker message handling code only
listens for "message" to resolve and "error" to reject, leaving the "exit" event
unhandled. If the worker process exits prematurely without posting a message or
throwing an error, the promise never settles and the test hangs. Add a "exit"
event listener that rejects the promise with an appropriate error message.
Additionally, consider using "once" instead of "on" for all listeners and
implement cleanup logic to remove listeners once the promise settles, ensuring
deterministic test behavior and preventing resource leaks.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 897c6cc1-ed58-4b94-b9e8-c5c782025f65

📥 Commits

Reviewing files that changed from the base of the PR and between df55ab7 and 348c942.

📒 Files selected for processing (2)

• src/runtime/jsc_hooks.rs
• test/js/node/no-addons.test.ts

[claude]

All prior feedback is addressed and the final approach (delegating to the real ComptimeClap::parse_with_table(RUN_TABLE, …) instead of a hand-rolled scanner) is the right one — but since this generalises the core clap lifetime bounds and changes how the --no-addons capability gate is computed for workers, it's worth a quick human glance.

Extended reasoning...

Overview

This PR fixes parse_worker_exec_argv_allow_addons so a worker's execArgv: ['-r', './x', '--no-addons'] correctly disables process.dlopen. After four rounds of review the implementation evolved from a hand-rolled token scanner (which had several edge-case parity bugs vs StreamingClap) to simply running the real clap parser against the rodata RUN_TABLE. To enable that, ComptimeClap<Id>/Args<Id> were generalised to ComptimeClap<'a, Id>/Args<'a, Id> so a borrowed SliceIterator can be used; the top-level parse/parse_with_table still return Args<'static, Id>, so existing OsIterator callers are unchanged by elision. A 12-case parameterised test suite covers value-taking flags, chained shorts, --prefixed values, bare -, and the negative case where --no-addons is itself consumed as a value.

Security risks

--no-addons is a capability control — it determines whether process.dlopen (arbitrary native code loading) is permitted in the worker VM. The original bug erred in the unsafe direction (addons stayed enabled when the user asked to disable them); this fix moves in the safe direction and now uses the canonical CLI parser as the single source of truth, so the risk profile improves. I don't see new exposure, but capability-boundary changes warrant a human sanity check.

Level of scrutiny

Moderate. The clap lifetime relaxation is mechanical and borrow-checker-enforced, but it threads through ComptimeClap/Args which back every bun CLI invocation. The jsc_hooks.rs change is a net simplification (~30 lines of bespoke scanning replaced by ~20 lines of delegation to existing, tested infrastructure).

Other factors

All five of my earlier inline findings (chained-short clusters, --prefixed values, bare -, OneOptional chaining, and the cold parse_ex path) are resolved — the last two became moot once the hand-rolled scanner was dropped in favour of the real parser. CodeRabbit's worker-exit hang concern is also addressed. No new issues found in this pass; not auto-approving only because of the security-adjacent surface and the core-infrastructure refactor.

[robobun]

CI at d0ef27c: all lanes pass except two unrelated to this diff.

• darwin-26-aarch64 (both shards, exit 137): the guest VM is missing /usr/local/bin/bun during pre-warm ([guest] pre-warming node_modules (native bun) → /bin/bash: line 16: /usr/local/bin/bun: No such file or directory). VM-image issue; the test runner never starts.
• darwin-14-x64 (one shard): test/js/bun/terminal/terminal.test.ts PTY tests time out after 90s. The same two tests time out on the same lane in build 62714 (unrelated PR sql_jsc: add SQLDataCell tag constructors and replace hand-rolled struct literals #32367, branch ali/sql-datacell-ctors), so this is a lane-specific issue rather than something introduced here.

My test file (test/js/node/no-addons.test.ts) passes on every lane that ran it, including debian-13-x64-asan. The clap lifetime generalization compiles and runs identically on all platforms that reached the test stage.

[claude]

LGTM — the switch to the real clap parser via parse_with_table(RUN_TABLE, …) structurally eliminates the parity gaps from the earlier hand-rolled scanner, and the lifetime generalization is type-level only.

Extended reasoning...

Overview

This PR fixes --no-addons detection in a worker's execArgv when it follows a value-taking flag (e.g. ['-r', './preload.js', '--no-addons']). It touches four files: src/clap/comptime.rs and src/clap/lib.rs generalize ComptimeClap<Id>/Args<Id> over an argv lifetime 'a (was hardcoded 'static); src/runtime/jsc_hooks.rs rewrites parse_worker_exec_argv_allow_addons to delegate to the canonical clap parser via ComptimeClap::parse_with_table(RUN_TABLE, …) instead of a hand-rolled scanner; and test/js/node/no-addons.test.ts adds an 11-case parameterized matrix.

I reviewed earlier revisions of this PR that kept a hand-rolled flag_consumes_next_token scanner and flagged several StreamingClap-parity gaps (chained shorts, --prefixed values, bare -, OneOptional). Those were each addressed, and then the final revision dropped the hand-rolled scanner entirely in favor of the real parser — which structurally removes that whole class of divergence. The last nit (use rodata RUN_TABLE via parse_with_table instead of the cold parse_ex/for_params path) was also applied in d0ef27c.

Security risks

--no-addons gates process.dlopen, so this is security-adjacent. The fix moves strictly in the safe direction (correctly disabling native addons when the user asked for it) and now uses exactly the same parser and param table the main CLI uses, so there is no second source of truth to drift. The two negative test cases (['-r', '--no-addons'] and ['-', '--no-addons']) confirm the parser doesn't over-disable either. No new attack surface.

Level of scrutiny

Moderate. The clap changes touch core CLI infrastructure, but they are a pure type-level relaxation: &'static [u8] → &'a [u8] on stored argv slices, with the public parse/parse_with_table entry points still returning Args<'static, Id> explicitly — so every existing OsIterator caller is byte-identical and compiler-verified. The jsc_hooks.rs change is a net reduction in custom logic (~25 lines of bespoke scanning replaced by ~20 lines that build a SliceIterator and call the canonical parser).

Other factors

All prior inline comments (mine and CodeRabbit's) are resolved. The bug-hunting system found nothing in the current revision. CI is green on every lane that ran the test file, including ASAN; the two red lanes are a darwin-26 VM-image issue and a known flaky PTY test on darwin-14-x64, both documented as reproducing on unrelated PRs. No CODEOWNERS cover these paths.