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

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

The validate command's spinner was starting regardless of the
--no-interactive flag, causing hangs in pre-commit hooks.

Changes:
- Pass noInteractive option to runBulkValidation
- Handle Commander.js --no-* flag syntax (sets interactive=false)
- Only start ora spinner when in interactive mode
- Add CI environment variable check to isInteractive() for industry
  standard compliance

* test: add unit tests for interactive utilities and CLI flag

- Export resolveNoInteractive() helper for reuse
- Add InteractiveOptions type export for testing
- Refactor validate.ts to use resolveNoInteractive()
- Add 17 unit tests for isInteractive() and resolveNoInteractive()
- Add CLI integration test for --no-interactive flag

This prevents future regressions where Commander.js --no-* flag
parsing is not properly handled.

Author: TabishB

src/commands/validate.ts

@@ -1,7 +1,7 @@
 import ora from 'ora';
 import path from 'path';
 import { Validator } from '../core/validation/validator.js';
-import { isInteractive } from '../utils/interactive.js';
+import { isInteractive, resolveNoInteractive } from '../utils/interactive.js';
 import { getActiveChangeIds, getSpecIds } from '../utils/item-discovery.js';
 import { nearestMatches } from '../utils/match.js';
 
@@ -15,6 +15,7 @@ interface ExecuteOptions {
   strict?: boolean;
   json?: boolean;
   noInteractive?: boolean;
+  interactive?: boolean; // Commander sets this to false when --no-interactive is used
   concurrency?: string;
 }
 
@@ -35,7 +36,7 @@ export class ValidateCommand {
       await this.runBulkValidation({
         changes: !!options.all || !!options.changes,
         specs: !!options.all || !!options.specs,
-      }, { strict: !!options.strict, json: !!options.json, concurrency: options.concurrency });
+      }, { strict: !!options.strict, json: !!options.json, concurrency: options.concurrency, noInteractive: resolveNoInteractive(options) });
       return;
     }
 
@@ -180,8 +181,8 @@ export class ValidateCommand {
     bullets.forEach(b => console.error(`  ${b}`));
   }
 
-  private async runBulkValidation(scope: { changes: boolean; specs: boolean }, opts: { strict: boolean; json: boolean; concurrency?: string }): Promise<void> {
-    const spinner = !opts.json ? ora('Validating...').start() : undefined;
+  private async runBulkValidation(scope: { changes: boolean; specs: boolean }, opts: { strict: boolean; json: boolean; concurrency?: string; noInteractive?: boolean }): Promise<void> {
+    const spinner = !opts.json && !opts.noInteractive ? ora('Validating...').start() : undefined;
     const [changeIds, specIds] = await Promise.all([
       scope.changes ? getActiveChangeIds() : Promise.resolve<string[]>([]),
       scope.specs ? getSpecIds() : Promise.resolve<string[]>([]),

src/utils/interactive.ts

@@ -1,4 +1,4 @@
-type InteractiveOptions = {
+export type InteractiveOptions = {
   /**
    * Explicit "disable prompts" flag passed by internal callers.
    */
@@ -9,14 +9,21 @@ type InteractiveOptions = {
   interactive?: boolean;
 };
 
-function resolveNoInteractive(value?: boolean | InteractiveOptions): boolean {
+/**
+ * Resolves whether non-interactive mode is requested.
+ * Handles both explicit `noInteractive: true` and Commander.js style `interactive: false`.
+ * Use this helper instead of manually checking options.noInteractive to avoid bugs.
+ */
+export function resolveNoInteractive(value?: boolean | InteractiveOptions): boolean {
   if (typeof value === 'boolean') return value;
   return value?.noInteractive === true || value?.interactive === false;
 }
 
 export function isInteractive(value?: boolean | InteractiveOptions): boolean {
   if (resolveNoInteractive(value)) return false;
   if (process.env.OPEN_SPEC_INTERACTIVE === '0') return false;
+  // Respect the standard CI environment variable (set by GitHub Actions, GitLab CI, Travis, etc.)
+  if ('CI' in process.env) return false;
   return !!process.stdin.isTTY;
 }
 

test/commands/validate.test.ts

@@ -130,4 +130,18 @@ describe('top-level validate command', () => {
     const result = await runCLI(['validate', changeId], { cwd: testDir });
     expect(result.exitCode).toBe(0);
   });
+
+  it('respects --no-interactive flag passed via CLI', async () => {
+    // This test ensures Commander.js --no-interactive flag is correctly parsed
+    // and passed to the validate command. The flag sets options.interactive = false
+    // (not options.noInteractive = true) due to Commander.js convention.
+    const result = await runCLI(['validate', '--specs', '--no-interactive'], {
+      cwd: testDir,
+      // Don't set OPEN_SPEC_INTERACTIVE to ensure we're testing the flag itself
+      env: { ...process.env, OPEN_SPEC_INTERACTIVE: undefined },
+    });
+    expect(result.exitCode).toBe(0);
+    // Should complete without hanging and without prompts
+    expect(result.stderr).not.toContain('What would you like to validate?');
+  });
 });

test/utils/interactive.test.ts

@@ -0,0 +1,125 @@
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { isInteractive, resolveNoInteractive, InteractiveOptions } from '../../src/utils/interactive.js';
+
+describe('interactive utilities', () => {
+  let originalOpenSpecInteractive: string | undefined;
+  let originalCI: string | undefined;
+  let originalStdinIsTTY: boolean | undefined;
+
+  beforeEach(() => {
+    // Save original environment
+    originalOpenSpecInteractive = process.env.OPEN_SPEC_INTERACTIVE;
+    originalCI = process.env.CI;
+    originalStdinIsTTY = process.stdin.isTTY;
+
+    // Clear environment for clean testing
+    delete process.env.OPEN_SPEC_INTERACTIVE;
+    delete process.env.CI;
+  });
+
+  afterEach(() => {
+    // Restore original environment
+    if (originalOpenSpecInteractive !== undefined) {
+      process.env.OPEN_SPEC_INTERACTIVE = originalOpenSpecInteractive;
+    } else {
+      delete process.env.OPEN_SPEC_INTERACTIVE;
+    }
+    if (originalCI !== undefined) {
+      process.env.CI = originalCI;
+    } else {
+      delete process.env.CI;
+    }
+    // Restore stdin.isTTY
+    Object.defineProperty(process.stdin, 'isTTY', {
+      value: originalStdinIsTTY,
+      writable: true,
+      configurable: true,
+    });
+  });
+
+  describe('resolveNoInteractive', () => {
+    it('should return true when noInteractive is true', () => {
+      expect(resolveNoInteractive({ noInteractive: true })).toBe(true);
+    });
+
+    it('should return true when interactive is false (Commander.js style)', () => {
+      // This is how Commander.js handles --no-interactive flag
+      expect(resolveNoInteractive({ interactive: false })).toBe(true);
+    });
+
+    it('should return false when noInteractive is false', () => {
+      expect(resolveNoInteractive({ noInteractive: false })).toBe(false);
+    });
+
+    it('should return false when interactive is true', () => {
+      expect(resolveNoInteractive({ interactive: true })).toBe(false);
+    });
+
+    it('should return false for empty options object', () => {
+      expect(resolveNoInteractive({})).toBe(false);
+    });
+
+    it('should return false for undefined', () => {
+      expect(resolveNoInteractive(undefined)).toBe(false);
+    });
+
+    it('should handle boolean value true', () => {
+      expect(resolveNoInteractive(true)).toBe(true);
+    });
+
+    it('should handle boolean value false', () => {
+      expect(resolveNoInteractive(false)).toBe(false);
+    });
+
+    it('should prioritize noInteractive over interactive when both set', () => {
+      // noInteractive: true should win
+      expect(resolveNoInteractive({ noInteractive: true, interactive: true })).toBe(true);
+      // If noInteractive is false, check interactive
+      expect(resolveNoInteractive({ noInteractive: false, interactive: false })).toBe(true);
+    });
+  });
+
+  describe('isInteractive', () => {
+    it('should return false when noInteractive is true', () => {
+      expect(isInteractive({ noInteractive: true })).toBe(false);
+    });
+
+    it('should return false when interactive is false (Commander.js --no-interactive)', () => {
+      expect(isInteractive({ interactive: false })).toBe(false);
+    });
+
+    it('should return false when OPEN_SPEC_INTERACTIVE env var is 0', () => {
+      process.env.OPEN_SPEC_INTERACTIVE = '0';
+      Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
+      expect(isInteractive({})).toBe(false);
+    });
+
+    it('should return false when CI env var is set', () => {
+      process.env.CI = 'true';
+      Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
+      expect(isInteractive({})).toBe(false);
+    });
+
+    it('should return false when CI env var is set to any value', () => {
+      // CI can be set to any value, not just "true"
+      process.env.CI = '1';
+      Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
+      expect(isInteractive({})).toBe(false);
+    });
+
+    it('should return false when stdin is not a TTY', () => {
+      Object.defineProperty(process.stdin, 'isTTY', { value: false, writable: true, configurable: true });
+      expect(isInteractive({})).toBe(false);
+    });
+
+    it('should return true when stdin is TTY and no flags disable it', () => {
+      Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
+      expect(isInteractive({})).toBe(true);
+    });
+
+    it('should return true when stdin is TTY and options are undefined', () => {
+      Object.defineProperty(process.stdin, 'isTTY', { value: true, writable: true, configurable: true });
+      expect(isInteractive(undefined)).toBe(true);
+    });
+  });
+});