fix(cli): respect --no-interactive flag in validate command

[TabishB]

Summary

• Fixes validate command hanging when run via pre-commit hooks
• The spinner was starting regardless of --no-interactive flag
• Correctly handles Commander.js --no-* flag syntax (sets interactive=false, not noInteractive=true)
• Adds CI environment variable check for industry standard compliance

Root Cause

When using --no-interactive, Commander.js sets options.interactive = false, NOT options.noInteractive = true. The runBulkValidation method was:

1. Not receiving the interactive state at all
2. Starting the ora spinner unconditionally (only checking --json flag)

Test plan

• Run openspec validate --specs --strict --no-interactive directly - completes without hanging
• Run pre-commit run openspec-validate -a - completes without hanging
• Build passes

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Explicit support for interactive and non-interactive modes for validation commands, including handling of the CLI --no-interactive flag.
  • Automatic CI environment detection disables interactive behavior for smoother automated runs and suppresses interactive UI (spinners/prompts) when non-interactive.

• Tests

  • Added tests verifying non-interactive flag behavior and interactive-utility logic across environments.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds CLI handling for --no-interactive via a new resolveNoInteractive export; threads a noInteractive flag into ValidateCommand.runBulkValidation and spinner logic; isInteractive now short-circuits to non-interactive when CI env is detected. Tests added for CLI flag and interactive utilities.

Changes

Cohort / File(s)	Summary
Validate command src/commands/validate.ts	Added interactive?: boolean to ExecuteOptions; pass noInteractive: resolveNoInteractive(options) into runBulkValidation; extended runBulkValidation opts to accept noInteractive?: boolean; spinner rendering now suppressed when json or noInteractive is true.
Interactive utilities src/utils/interactive.ts	Exported InteractiveOptions type and resolveNoInteractive helper; isInteractive() now returns false early when a CI environment variable is present; preserved existing OPEN_SPEC_INTERACTIVE and stdin TTY checks.
Tests — validate CLI test/commands/validate.test.ts	Added test verifying --no-interactive CLI flag is parsed and prevents interactive prompt when running validate with --specs.
Tests — interactive utils test/utils/interactive.test.ts	New suite covering resolveNoInteractive and isInteractive across combinations of flags, environment vars (OPEN_SPEC_INTERACTIVE, CI), and mocked stdin.isTTY.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI (commander)
    participant Validate as ValidateCommand
    participant Utils as interactive utils
    participant Spinner as Spinner/Renderer

    CLI->>Validate: parse options (includes --no-interactive / interactive)
    Validate->>Utils: resolveNoInteractive(options, env)
    note right of Utils `#f8f3d7`: resolveNoInteractive\nchecks explicit flags,\nOPEN_SPEC_INTERACTIVE,\nCI, stdin.isTTY
    Utils-->>Validate: boolean noInteractive
    Validate->>Validate: decide bulk run path
    alt noInteractive or json
        Validate->>Spinner: suppress spinner rendering
        Spinner-->>Validate: no spinner shown
    else interactive allowed
        Validate->>Spinner: show spinner during bulk validation
        Spinner-->>Validate: spinner updates
    end
    Validate->>Validate: runBulkValidation(..., noInteractive)
    Validate-->>CLI: exit code / output

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix(cli): prevent hang in pre-commit hooks by using dynamic imports #380: Modifies interactivity handling (resolveNoInteractive/isInteractive) and validation command control flow, touching the same files and non-interactive validation paths.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly summarizes the main fix: making the validate command respect the --no-interactive flag to prevent hanging in non-interactive contexts.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/pre-commit-hook-hanging

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/utils/interactive.test.ts (1)

40-80: Comprehensive test coverage for resolveNoInteractive.

The tests cover all input shapes (object with noInteractive/interactive, boolean, undefined) and correctly verify the OR logic.

Optional: Clarify the "prioritize" comment

The comment at line 74 says "should prioritize noInteractive over interactive when both set," but the function uses OR logic (noInteractive === true || interactive === false), so both test cases return true. Consider rephrasing:

-    it('should prioritize noInteractive over interactive when both set', () => {
-      // noInteractive: true should win
+    it('should return true when either condition for non-interactive is met', () => {
+      // Either noInteractive: true OR interactive: false triggers non-interactive mode
       expect(resolveNoInteractive({ noInteractive: true, interactive: true })).toBe(true);
-      // If noInteractive is false, check interactive
       expect(resolveNoInteractive({ noInteractive: false, interactive: false })).toBe(true);
     });

The current assertions are correct; this just clarifies the intent.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2c2799 and 6b3f64e.

📒 Files selected for processing (4)

• src/commands/validate.ts
• src/utils/interactive.ts
• test/commands/validate.test.ts
• test/utils/interactive.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/utils/interactive.ts

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval

📚 Learning: 2025-11-25T01:08:19.004Z

Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-11-25T01:08:19.004Z
Learning: Run `openspec validate [change-id] --strict` before requesting approval

Applied to files:

• src/commands/validate.ts

🧬 Code graph analysis (3)

test/commands/validate.test.ts (1)

test/helpers/run-cli.ts (1)

• runCLI (75-137)

test/utils/interactive.test.ts (1)

src/utils/interactive.ts (2)

• resolveNoInteractive (17-20)
• isInteractive (22-28)

src/commands/validate.ts (1)

src/utils/interactive.ts (1)

• resolveNoInteractive (17-20)

🔇 Additional comments (7)

test/commands/validate.test.ts (1)

134-146: LGTM! Excellent test coverage for the CLI flag.

The test correctly verifies that --no-interactive prevents the command from hanging and prompting. The comment explaining Commander.js behavior is helpful, and explicitly unsetting OPEN_SPEC_INTERACTIVE ensures the test validates the flag itself rather than environment-derived behavior.

test/utils/interactive.test.ts (2)

1-38: LGTM! Clean environment isolation.

The setup and teardown properly preserve and restore environment variables and stdin.isTTY, ensuring test isolation.

---

82-124: LGTM! Thorough coverage of isInteractive behavior.

The tests validate all the short-circuit conditions (explicit flags, OPEN_SPEC_INTERACTIVE, CI env var, stdin.isTTY) and confirm the fallback to TTY detection.

src/commands/validate.ts (4)

4-4: LGTM! Correct import for the fix.

---

18-18: LGTM! Matches Commander.js --no- convention.*

The comment correctly documents that Commander.js sets interactive = false (not noInteractive = true) for the --no-interactive flag.

---

39-39: LGTM! Correctly threads noInteractive into bulk validation.

Using resolveNoInteractive(options) properly handles both Commander.js style (interactive: false) and direct noInteractive flags.

---

184-185: LGTM! Spinner is correctly gated for non-interactive mode.

The spinner only starts when both conditions are met:

• Not in JSON output mode
• Not in non-interactive mode

This prevents the hang reported in pre-commit hooks. All subsequent spinner usages (lines 217, 250, 272) safely handle undefined via optional chaining or conditionals.