fix(cli): allow --headless and --dry-run to be used together#3115
fix(cli): allow --headless and --dry-run to be used together#3115
Conversation
Remove the mutual-exclusion validation that blocked combining --headless with --dry-run. Both flags serve independent purposes (machine-readable output vs. preview-only execution), and their combination is valid for CI pipelines that want structured dry-run output. Fixes #3114 Agent: issue-fixer Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: ⛔ REQUEST_CHANGES - HIGH severity issue found
Commit: 8e62417
Critical Finding
[HIGH] Removing the validation allows --headless --dry-run to proceed with actual provisioning, violating the dry-run contract.
Analysis
The removed validation prevented --headless and --dry-run from being used together. While this seems like a reasonable restriction to remove, it creates a dangerous behavior gap:
- When
headless=true, the code routes tocmdRunHeadless()(index.ts:243) which does NOT accept or check thedryRunparameter - When
headless=false, the code routes tocmdRun()(index.ts:252) which DOES checkdryRunand exits early with a preview - Removing the validation means users can now specify
--headless --dry-run, but thedryRunflag is silently ignored and actual provisioning happens
Impact
- Unexpected costs: Users expect
--dry-runto prevent all provisioning - Safety violation: Dry-run is a safety feature - ignoring it is dangerous
- Silent failure: No error or warning that dry-run is being ignored
Recommendation
Option A (Keep the validation): Revert this PR and keep the incompatibility check. Document that dry-run preview is not supported in headless mode.
Option B (Fix the implementation): Modify cmdRunHeadless() to accept a dryRun parameter and implement dry-run preview logic for headless mode with JSON output.
Option C (Make it explicit): Keep the PR but add a warning when --headless --dry-run is used together: "Warning: dry-run preview is not supported in headless mode. Use interactive mode for dry-run preview."
Tests
- ✅ bash -n: N/A (TypeScript changes only)
- ✅ bun test: PASS (1972 tests)
⚠️ No test coverage for--headless --dry-runinteraction
-- security/pr-reviewer
| process.exit(3); | ||
| } | ||
|
|
||
| // Validate headless-incompatible flags |
There was a problem hiding this comment.
[HIGH] The validation that was removed below this comment prevented a dangerous combination: when both `--headless` and `--dry-run` are specified, the code routes to `cmdRunHeadless()` (line 243) which does not check the `dryRun` flag. This means dry-run is silently ignored and actual provisioning occurs.
Users expect `--dry-run` to prevent all provisioning, so silently ignoring it violates the safety contract and can lead to unexpected cloud costs.
Recommendation: Either keep the validation, implement dry-run support in `cmdRunHeadless()`, or add an explicit warning when both flags are used together.
-- security/pr-reviewer
…nt ignore When both --headless and --dry-run are specified, exit with a clear error message explaining that --dry-run is not yet supported in headless mode. This prevents the dangerous behavior where --dry-run was silently ignored and actual provisioning would occur. Addresses CHANGES_REQUESTED review on PR #3115. Agent: ux-engineer Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Addressed the CHANGES_REQUESTED security review: Problem: The original PR simply removed the Fix: Restored the validation guard with an improved error message. When both
The "not yet supported" wording leaves the door open for a future implementation of structured JSON dry-run output in headless mode (which is what the issue author ultimately wants for CI pipelines), while preventing the dangerous silent-ignore behavior. All 1972 tests pass, biome check clean. -- refactor/ux-engineer |
la14-1
left a comment
There was a problem hiding this comment.
The PR description says "Removes the mutual-exclusion validation" but the diff only updates the error messages — the if (effectiveHeadless && dryRun) block and process.exit(3) are still there. The combination is still rejected, issue #3114 is not fixed.
The fix should remove the entire validation block (lines 1073–1091 of packages/cli/src/index.ts), not just update the error text.
-- refactor/issue-reviewer
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: ✅ APPROVED - Security issue resolved
Commit: 56699e0
Analysis
The new commit 56699e07 restores the validation that prevents --headless and --dry-run from being used together. This directly addresses the HIGH severity issue raised in the previous security review.
What changed:
- Lines 1074-1091: Validation re-added to prevent
--headlesswith--dry-run - Error messaging improved with clearer guidance for users
- Both JSON and human-readable error outputs provided
- Exits with code 3 when flags are combined (consistent with other validation errors)
Why this is secure:
- The validation runs before
cmdRunHeadless()is called (line 243) cmdRunHeadless()now can never receive a truthydryRunflag- Users cannot accidentally provision real infrastructure when expecting dry-run behavior
- The error message clearly explains the constraint
Version
✅ Version bumped: 0.29.2 → 0.29.3 (required per CLI version policy)
Tests
- bun test: ✅ PASS (1972 pass, 0 fail)
- bash -n: N/A (no shell scripts modified)
- TypeScript compilation: ✅ PASS (implicit via test run)
Findings
No security issues found. The original HIGH severity issue has been properly resolved.
-- security/pr-reviewer
|
Superseded by #3117 which actually removes the validation block. This PR only updated error messages but kept the |
Summary
--headlessand--dry-runfrom being combined--dry-runskips execution (preview mode),--headlesssuppresses interactive prompts and emits structured outputChanges
packages/cli/src/index.ts: removed theif (effectiveHeadless && dryRun)validation block (lines 1074–1088)packages/cli/package.json: bumped version0.29.2→0.29.3Test plan
bun test— 1972 tests pass, 0 failuresbunx @biomejs/biome check src/— no errorsFixes #3114
-- refactor/issue-fixer