Skip to content

Honor CLI precedence and isolate JSP bootstrap state (Fixes #3083) - #3096

Merged
acoliver merged 10 commits into
mainfrom
issue3083
Aug 7, 2026
Merged

Honor CLI precedence and isolate JSP bootstrap state (Fixes #3083)#3096
acoliver merged 10 commits into
mainfrom
issue3083

Conversation

@acoliver

@acoliver acoliver commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

TLDR

Enforces explicit CLI precedence for sandbox selection and adds a public --jsp-bootstrap <path> option so supervisors no longer need to leave per-session JSP bootstrap state in descendant environments.

The bootstrap environment variable is captured and scrubbed as the first executable startup action. Its nonsecret file path is transported explicitly across memory relaunches and sandbox hops, while the credential-bearing bootstrap content remains in its mode-0600 file.

Dive Deeper

  • Resolves sandbox configuration in the order CLI flag > LLXPRT_SANDBOX > settings.sandbox > disabled.
    • Explicit --no-sandbox and --sandbox false now override inherited sandbox environment values.
    • Existing value normalization, nested-sandbox behavior, profile handling, image resolution, and engine selection remain intact.
    • Sandbox failures identify the source that supplied the effective value.
  • Adds --jsp-bootstrap <path> with precedence over the deprecated LLXPRT_JSP_BOOTSTRAP_FILE environment variable.
    • Rejects bare, empty, and repeated public or internal bootstrap options instead of silently falling through.
    • Captures and deletes the deprecated environment variable before help/version handling, settings loading, memory relaunch, parsing, or other child-capable startup work.
    • Carries env-origin paths through a hidden argv transport option across direct-replacement memory and sandbox hops, including argv containing an exact -- terminator.
    • Preserves source provenance so fail-fast diagnostics name either --jsp-bootstrap or LLXPRT_JSP_BOOTSTRAP_FILE accurately without exposing bootstrap contents.
  • Adds Bun-native behavioral coverage for parser behavior, precedence, lifecycle scrubbing, real child inheritance, relaunch/sandbox argv transport, source ordering, and real HTTP observation setup.
  • Documents both precedence contracts and the JSP bootstrap migration path.

Reviewer Test Plan

  1. Run the Bun-native issue tests:

    cd packages/cli
    bun test src/config/cliArgParser.jspBootstrap.test.ts src/config/sandboxConfig.precedence.test.ts src/observation/jspBootstrapStartup.test.ts src/observation/jspWiring.test.ts src/utils/relaunch.test.ts
    
  2. Set LLXPRT_SANDBOX=docker and verify --no-sandbox or --sandbox false leaves sandboxing disabled.

  3. Point LLXPRT_JSP_BOOTSTRAP_FILE at a valid bootstrap and verify a child spawned after CLI startup does not inherit that variable.

  4. Supply a valid bootstrap through --jsp-bootstrap while also setting the deprecated environment variable; verify the flag path is selected.

  5. Exercise memory relaunch or sandbox hopping and verify the env-origin path survives through argv while the child environment remains scrubbed.

  6. Supply missing or malformed bootstrap files through both sources and verify diagnostics name the actual source without printing path contents or credentials.

Local verification completed:

  • npm run test workspace coverage; all issue-related and CLI suites pass. The full serial workspace runner encountered documented unrelated concurrent Bun/agent test instability, while affected workspaces passed in isolation.
  • npm run lint
  • npm run typecheck
  • npm run format
  • npm run build
  • npm run lint:eslint-guard
  • bun scripts/start.ts --profile-load stepfun-37 "write me a haiku and nothing else"
  • DeepThinker review and Open Code Review remediation

Testing Matrix

🍏 🪟 🐧
npm run
npx
Docker
Podman - -
Seatbelt - -

Linked issues / bugs

Fixes #3083

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c49b5287-d3d4-47b4-aad7-9415742a23dc

📥 Commits

Reviewing files that changed from the base of the PR and between e408e1f and 3334ee6.

⛔ Files ignored due to path filters (1)
  • project-plans/issue-3083-flag-precedence-and-jsp-bootstrap-flag.md is excluded by !project-plans/**
📒 Files selected for processing (16)
  • docs/cli/configuration.md
  • docs/sandbox.md
  • packages/cli/src/cli.tsx
  • packages/cli/src/cliBootstrap.tsx
  • packages/cli/src/cliSandbox.ts
  • packages/cli/src/cliSessionBootstrap.ts
  • packages/cli/src/config/cliArgParser.jspBootstrap.test.ts
  • packages/cli/src/config/cliArgParser.ts
  • packages/cli/src/config/profileBootstrap.ts
  • packages/cli/src/config/sandboxConfig.precedence.test.ts
  • packages/cli/src/config/sandboxConfig.ts
  • packages/cli/src/config/yargsOptions.ts
  • packages/cli/src/observation/jspBootstrapStartup.test.ts
  • packages/cli/src/observation/jspWiring.test.ts
  • packages/cli/src/observation/jspWiring.ts
  • packages/cli/src/utils/relaunch.ts

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

@github-actions github-actions Bot added the maintainer:e2e:ok Trusted contributor; maintainer-approved E2E run label Aug 6, 2026
# Conflicts:
#	docs/cli/configuration.md
#	packages/cli/src/cliSessionBootstrap.ts
#	packages/cli/src/observation/jspWiring.test.ts
#	packages/cli/src/observation/jspWiring.ts
#	scripts/bun-test-manifest.ts
# Conflicts:
#	packages/cli/vitest.test-groups.ts
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Before this PR, sandbox configuration could be silently overridden by inherited environment variables, so setting --sandbox, --sandbox-engine, or --sandbox-image on the command line did not reliably win over LLXPRT_SANDBOX* env or settings, and diagnostics did not make the winning source obvious. JSP bootstrap state was also not isolated: inherited bootstrap-related environment could leak into the CLI process at startup, and there was no explicit CLI control for bootstrap selection, so relaunch or observation paths could pick up unintended inherited state. After this PR, CLI flags take clear precedence over env and settings for sandbox configuration, with source-labeled diagnostics surfacing where each value came from. A new --jsp-bootstrap flag lets users explicitly choose bootstrap behavior, inherited bootstrap env is scrubbed at startup, and bootstrap selection is consistently threaded through relaunch and observation wiring so sessions start from a clean, deterministic state.

Release Notes

New Features

  • Added --jsp-bootstrap CLI flag to explicitly control JSP bootstrap behavior.
  • Sandbox configuration now honors explicit CLI flag precedence over environment variables and settings, with source-labeled diagnostics.

Bug Fixes

  • Fixed sandbox option resolution so CLI flags reliably override inherited LLXPRT_SANDBOX* environment state instead of silently losing precedence.
  • Scrubbed inherited JSP bootstrap environment at startup and threaded bootstrap selection through relaunch and observation wiring to prevent cross-session state leakage.

Tests

  • Added behavioral coverage for sandbox precedence rules.
  • Added tests for JSP bootstrap startup and observation wiring.

Changes

Layer File(s) Summary
sandbox-precedence packages/cli/src/config/sandboxConfig.ts, packages/cli/src/config/sandboxConfig.precedence.test.ts, packages/cli/src/cliSandbox.ts, packages/cli/src/config/profileBootstrap.ts, docs/sandbox.md Honor CLI flag precedence over env and settings for sandbox configuration, with source-labeled diagnostics
jsp-bootstrap packages/cli/src/config/cliArgParser.ts, packages/cli/src/config/cliArgParser.jspBootstrap.test.ts, packages/cli/src/config/yargsOptions.ts, packages/cli/src/observation/jspWiring.ts, packages/cli/src/observation/jspWiring.test.ts, packages/cli/src/observation/jspBootstrapStartup.test.ts, packages/cli/src/cli.tsx, packages/cli/src/cliSessionBootstrap.ts, packages/cli/src/cliBootstrap.tsx, packages/cli/src/utils/relaunch.ts, docs/cli/configuration.md Add --jsp-bootstrap CLI flag, scrub inherited env at startup, and thread bootstrap selection through relaunch and observation wiring
project-plan project-plans/issue-3083-flag-precedence-and-jsp-bootstrap-flag.md Plan document defining acceptance criteria and behavioral test strategy for CLI precedence and JSP bootstrap isolation

Magnitude

🎯 2 (M)
1970 additions, 138 deletions, 17 changed files across 1 package, 0 acceptance criteria

Related

Pre-merge Checks

Check Status Note
Title Clear and descriptive: identifies the two core behaviors being changed (CLI precedence and JSP bootstrap isolation) and explicitly references the issue being fixed.
Description Contains all required template sections: TLDR, Dive Deeper, Reviewer Test Plan, Testing Matrix, and Linked issues / bugs. The test plan is actionable with specific bun test commands and manual verification steps.
Linked Issues The changes fulfill issue #3083's acceptance criteria: sandbox resolution now honors CLI flag > env > settings precedence with source attribution; a new --jsp-bootstrap flag is added with precedence over the deprecated LLXPRT_JSP_BOOTSTRAP_FILE; the env var is scrubbed at startup to prevent child inheritance; and bootstrap state is isolated across memory relaunches and sandbox hops via hidden argv transport. Behavioral tests cover parser behavior, precedence, scrubbing, real child inheritance, relaunch/sandbox transport, and HTTP observation setup.
Out of Scope Testing matrix shows most platforms as untested (), though this is a documentation gap rather than missing functionality. No other out-of-scope items are evident from the change set.

Walkthrough generated by LLxprt PR Review. Planner issue: #2256

Comment thread packages/cli/src/cliSessionBootstrap.ts
Comment thread packages/cli/src/cli.tsx
Comment thread packages/cli/src/observation/jspWiring.test.ts
Comment thread packages/cli/src/config/sandboxConfig.ts
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

OpenCodeReview — automatic reviews suspended

Automatic OCR reviews are suspended for this PR after 2 of 2 automatic reviews.

To get more reviews you can:

  • Check the box below to re-enable automatic reviews (resets the counter), or

  • Comment /review, /ocr, or /open-code-review to request a single review on demand.

  • Re-enable automatic reviews

Comment thread packages/cli/src/observation/jspBootstrapStartup.test.ts
Comment thread packages/cli/src/observation/jspBootstrapStartup.test.ts Outdated
Comment thread packages/cli/src/observation/jspBootstrapStartup.test.ts Outdated
Comment thread packages/cli/src/observation/jspWiring.test.ts
@acoliver
acoliver merged commit 0544ba8 into main Aug 7, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer:e2e:ok Trusted contributor; maintainer-approved E2E run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI flags must outrank environment variables, and the JSP bootstrap needs a --jsp-bootstrap flag

1 participant