Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/commands/validate.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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[]>([]),
Expand Down
11 changes: 9 additions & 2 deletions src/utils/interactive.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
type InteractiveOptions = {
export type InteractiveOptions = {
/**
* Explicit "disable prompts" flag passed by internal callers.
*/
Expand All @@ -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;
}

14 changes: 14 additions & 0 deletions test/commands/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?');
});
});
125 changes: 125 additions & 0 deletions test/utils/interactive.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
Loading