Skip to content

Intersect injected flags with the Node binary's accepted set - #398

Merged
colinhacks merged 1 commit into
mainfrom
flag-probe-intersect
Jul 10, 2026
Merged

Intersect injected flags with the Node binary's accepted set#398
colinhacks merged 1 commit into
mainfrom
flag-probe-intersect

Conversation

@colinhacks

Copy link
Copy Markdown
Contributor

nub injects --experimental-* flags to unflag Node features early. Several of those bands are open-ended ([lo, ∞), e.g. --experimental-import-text on [26.5.0, ∞)) on the assumption that once a flag exists Node keeps accepting it forever.

That assumption is false. Node keeps some unflagged flags as accepted no-ops (--experimental-fetch, --experimental-modules) but hard-removes others: --experimental-policy and --experimental-network-imports are gone, and --experimental-permission was accepted through Node 23.11 then deleted at 24.0 (it stabilized to --permission). Passing a removed flag is a node: bad option startup abort (exit 9), before any code runs. So an open-ended band would crash nub on whatever future Node drops the flag — and runtime feature-detection can't help, since the flag is parsed before any preload loads.

Fix

Make injection self-correcting by intersecting the version-band set with the target Node's actual accepted-flag set:

  • discovery::accepted_env_flags(node_path) probes process.allowedNodeEnvironmentFlags once and caches it per (path, mtime) in ~/.cache/nub/node-env-flags.json. Amortized cost ≈ 0 (one probe per binary, ever; spawn-free on the warm path).
  • compute_inject_flags gained a Stage-4 intersection: a flag the running Node doesn't accept is dropped. None (probe unavailable) preserves the prior version-band behavior, so a failed probe never regresses.
  • Sound because nub injects flags via argv and propagates them to children via NODE_OPTIONS, so every injected flag is already envvar-allowed — exactly what allowedNodeEnvironmentFlags enumerates. A host_node_accepts_every_injected_flag test guards the invariant (the intersection drops nothing on a current, supported Node).

A future Node that removes a flag now needs no nub release — the probe stops offering it and Stage 4 drops it.

Verification

  • Empirically confirmed the removal behavior against installed Node 18.20–26.5 and node_options.cc (no-op vs hard-removed flags; the --experimental-permission--permission deletion at 24.0).
  • e2e: --experimental-import-text is injected on Node 26.5 (present in its probed set) and not on 26.2 (absent); warm runs hit the cache with no re-probe.
  • cargo clippy --all-targets --all-features -D warnings, cargo fmt --check, and the nub-core node test suite pass.

Refs #395.

…d set

nub injects `--experimental-*` flags to unflag Node features early, with
several open-ended version bands on the premise that once a flag exists Node
accepts it forever. That premise is false: Node keeps some unflagged flags as
no-ops but hard-removes others (`--experimental-policy`,
`--experimental-network-imports`; `--experimental-permission` was deleted at
24.0 when it stabilized to `--permission`). Injecting a removed flag is a
`node: bad option` startup abort, so an open-ended band would crash nub on a
future Node that drops the flag.

Probe the target Node's `process.allowedNodeEnvironmentFlags` once, cache it per
(path, mtime), and intersect it with the version-band inject set in
`compute_inject_flags` (Stage 4). A flag the running Node does not accept is
dropped, so injection self-corrects with no nub release when Node removes a
flag. Probe unavailable falls back to the prior version-band behavior. The
probe clears NODE_OPTIONS so an inherited preload can't pollute or side-effect
it.

A host-Node invariant test asserts the intersection drops nothing on a current,
supported Node (nub only injects env-allowed flags).

Refs #395.
Copilot AI review requested due to automatic review settings July 9, 2026 20:26
@vercel

vercel Bot commented Jul 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nub Ready Ready Preview, Comment Jul 9, 2026 8:28pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — makes --experimental-* flag injection self-correcting by intersecting the version-band inject set with the target Node's actual allowedNodeEnvironmentFlags, so a flag a future Node hard-removes is dropped instead of aborting startup with node: bad option.

  • Probe + cache the accepted-flag setdiscovery::accepted_env_flags(node_path) runs one node -e probe of process.allowedNodeEnvironmentFlags and caches it per (path, mtime) in node-env-flags.json; warm path is spawn-free. Clears inherited NODE_OPTIONS for the probe so it stays deterministic and side-effect-free.
  • Stage-4 intersection in compute_inject_flags — new accepted_env_flags: Option<&BTreeSet<String>> param drops any flag the binary doesn't accept, matched on the flag NAME (split_once('=')) so value-bearing flags like --disable-warning=… match the bare accepted name. None preserves pure version-band behavior, so a failed probe never regresses.
  • Threaded through the spawn pathscompute_augmentation_env gained a node_path param (all three call sites updated), and spawn_node + run_watch pass the probed set.
  • Webstorage injection stays outside the intersection deliberately — its band is closed (22.4–<25) and the flag stabilized at 25, so there's no open-ended-removal hazard; documented inline.
  • Invariant testhost_node_accepts_every_injected_flag asserts the intersection drops nothing on the host Node, encoding the design's load-bearing property (nub only injects envvar-allowed flags).

The soundness argument holds: nub propagates every injected flag via NODE_OPTIONS, and allowedNodeEnvironmentFlags enumerates exactly the envvar-allowed universe, so the intersection can only ever bite a future Node that removed a flag. The fail-safe None fallback, the name-based match, and the real-Node invariant test are all correct. The non-atomic read-modify-write in write_env_flags_cache mirrors the pre-existing write_version_cache pattern, so it's consistent with convention rather than a new concern. Mergeable as-is.

Pullfrog  | View workflow run | Using Claude Opus𝕏

@colinhacks
colinhacks merged commit 4b3a85c into main Jul 10, 2026
59 checks passed
@colinhacks

Copy link
Copy Markdown
Contributor Author

Shipped in v0.4.6: https://github.com/nubjs/nub/releases/tag/v0.4.6

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.

2 participants