fix(cli): explain switch values like --vulns=false instead of 'unknown option'

DOCS.md

@@ -1978,6 +1978,8 @@ vg scan [path] [--vulns] [--full] [--iac] [--format text|json|sarif|md] [--out <
 | `--force` | — | Always create a fresh ingest, even when the repository is unchanged since the last scan |
 | `--quiet` | — | Suppress promotional output; scan results are unaffected |
 
+Switches (flags that take no value, such as `--vulns`, `--offline` or `--no-graph`) are off unless you pass them. They do not accept a value or an invented `--no-` form: `--vulns=false`, `--vulns=true` and `--no-vulns` stop with exit code `5` and name the form that works. In a CI template, add or leave out the flag itself rather than passing `true` or `false`.
+
 By default, the scan writes `.vibgrate/scan_result.json`. Use `--no-local-artifacts` or `--max-privacy` to suppress local JSON artifact files.
 
 For offline drift scoring, pass `--package-manifest <file>` with a downloaded manifest bundle such as `https://github.com/vibgrate/manifests/latest-packages.zip`. The manifest shape, the fail-closed errors, and what offline mode skips are in [Offline scan with a package-version manifest](#offline-scan-with-a-package-version-manifest).

src/cli-switch-values.test.ts

@@ -0,0 +1,137 @@
+import { spawnSync } from 'node:child_process';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { Command, CommanderError } from 'commander';
+import { describe, expect, it } from 'vitest';
+import { buildProgram } from './cli.js';
+import { switchValueError } from './cli-switch-values.js';
+
+/**
+ * CI templates often spell switches out explicitly (`--vulns=false`,
+ * `--no-vulns`), usually from a templated variable. `vg` must never read such
+ * an input as "on", and when it rejects one the error must name the switch and
+ * show a form that works.
+ */
+
+const SWITCHES = ['--vulns', '--offline'] as const;
+const attr = (flag: string): string => flag.slice(2);
+
+class Parsed extends Error {
+  constructor(readonly opts: Record<string, unknown>) {
+    super('parsed');
+  }
+}
+
+/**
+ * Parse `vg scan <dir> ...args` through the real program and return the
+ * options commander resolved, or the error it rejected the input with. The
+ * scan itself never runs.
+ */
+async function parseScan(args: string[]): Promise<Record<string, unknown> | CommanderError> {
+  const scan = buildProgram().commands.find((c) => c.name() === 'scan') as Command;
+  scan.exitOverride();
+  scan.configureOutput({ writeErr: () => {}, writeOut: () => {} });
+  scan.hook('preAction', (_cmd, action) => {
+    throw new Parsed(action.opts());
+  });
+  try {
+    await scan.parseAsync([os.tmpdir(), ...args], { from: 'user' });
+  } catch (e) {
+    if (e instanceof Parsed) return e.opts;
+    if (e instanceof CommanderError) return e;
+    throw e;
+  }
+  throw new Error('scan did not reach its action');
+}
+
+describe('vg scan switches parse as documented', () => {
+  for (const flag of SWITCHES) {
+    describe(flag, () => {
+      it('is off when absent', async () => {
+        const opts = await parseScan([]);
+        expect(opts).not.toBeInstanceOf(CommanderError);
+        expect((opts as Record<string, unknown>)[attr(flag)]).toBeUndefined();
+      });
+
+      it(`is on with ${flag}`, async () => {
+        const opts = await parseScan([flag]);
+        expect((opts as Record<string, unknown>)[attr(flag)]).toBe(true);
+      });
+
+      for (const input of [`${flag}=true`, `${flag}=false`, `${flag}=maybe`, `${flag}=`, `--no-${attr(flag)}`]) {
+        it(`rejects ${input} and never turns the switch on`, async () => {
+          const res = await parseScan([input]);
+          expect(res).toBeInstanceOf(CommanderError);
+          expect((res as CommanderError).code).toBe('commander.unknownOption');
+        });
+      }
+    });
+  }
+});
+
+describe('switchValueError', () => {
+  const program = buildProgram();
+
+  for (const flag of SWITCHES) {
+    for (const value of ['true', 'false', 'maybe', '']) {
+      it(`explains ${flag}=${value} with the switch and a working example`, () => {
+        const msg = switchValueError(program, ['scan', '.', `${flag}=${value}`]);
+        expect(msg).toContain(`${flag} is an on/off switch and takes no value`);
+        expect(msg).toContain(`got ${flag}=${value}`);
+        expect(msg).toContain(`vg scan ${flag}`);
+      });
+    }
+
+    it(`explains --no-${attr(flag)} without suggesting the opposite`, () => {
+      const msg = switchValueError(program, ['scan', `--no-${attr(flag)}`, '.']);
+      expect(msg).toContain(`--no-${attr(flag)} is not an option`);
+      expect(msg).toContain(`${flag} is off unless you pass it`);
+      expect(msg).toContain(`vg scan ${flag}`);
+    });
+  }
+
+  it('explains a value on a --no- switch the command does define', () => {
+    expect(switchValueError(program, ['scan', '--no-graph=1'])).toContain(
+      '--no-graph is an on/off switch and takes no value (got --no-graph=1)',
+    );
+  });
+
+  it.each([
+    [['scan', '.', '--vulns', '--offline']],
+    [['scan', '.', '--format=json']],
+    [['scan', '.', '--bogus=1']],
+    [['scan', '.', '--no-graph']],
+    [['scan', '.', '--', '--vulns=false']],
+    [['scan', '.', '--no-such-switch']],
+  ])('leaves valid or unrelated input to commander: %j', (args) => {
+    expect(switchValueError(program, args)).toBeUndefined();
+  });
+
+  it('does not inspect arguments a pass-through command forwards', () => {
+    // `vg serve <agent> ...` hands everything after the agent name to the agent.
+    // `--http` is a serve switch, so without the pass-through rule this would match.
+    expect(switchValueError(program, ['serve', 'claude', '--http=1'])).toBeUndefined();
+  });
+});
+
+describe('vg scan with a switch value, end to end', () => {
+  const pkgRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-switch-'));
+  fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"fixture","version":"1.0.0"}\n');
+
+  const run = (flag: string) =>
+    spawnSync(
+      process.execPath,
+      ['--import', 'tsx', path.join(pkgRoot, 'src/cli.ts'), 'scan', dir, '--offline', '--no-graph', '--no-daemon', '--quiet', '--no-local-artifacts', flag],
+      { cwd: pkgRoot, encoding: 'utf8', timeout: 45_000, env: { ...process.env, NO_COLOR: '1', VIBGRATE_NO_KERNEL: '1', VIBGRATE_DSN: '' } },
+    );
+
+  it.each(['--vulns=false', '--no-vulns'])('%s exits with the usage code and a working example', (flag) => {
+    const res = run(flag);
+    expect(res.status).toBe(5);
+    expect(res.stderr).toContain('vg scan --vulns');
+    expect(res.stderr).not.toContain('Did you mean');
+  }, 60_000);
+});

src/cli-switch-values.ts

@@ -0,0 +1,83 @@
+import type { Command, Option } from 'commander';
+
+/**
+ * Explain an on/off switch written with a value (`--vulns=false`) or with a
+ * `--no-` prefix the command does not define (`--no-vulns`).
+ *
+ * Commander rejects both as an unknown option, which is safe but unhelpful:
+ * `--vulns=false` reads as "unknown option" for a switch that exists, and for
+ * `--no-vulns` it suggests `--vulns`, the opposite of what was asked. Returns
+ * a usage message for the first such argument, or `undefined` to leave the
+ * input to commander. Only inputs commander would reject anyway are matched.
+ */
+export function switchValueError(program: Command, args: string[]): string | undefined {
+  const { cmd, rest } = resolveCommand(program, args);
+  if (isPassThrough(cmd)) return undefined;
+  const usage = commandPath(cmd);
+
+  for (const arg of rest) {
+    if (arg === '--') break;
+    if (!arg.startsWith('--')) continue;
+
+    const eq = arg.indexOf('=');
+    if (eq > 0) {
+      const flag = arg.slice(0, eq);
+      const opt = switchNamed(cmd, flag);
+      if (!opt || (!opt.negate && opt.defaultValue !== undefined)) continue;
+      const hint = flag.startsWith('--no-')
+        ? `use \`${usage} ${flag}\`, or leave it out`
+        : `it is off unless you pass it: use \`${usage} ${flag}\` to turn it on, or leave it out`;
+      return `${flag} is an on/off switch and takes no value (got ${arg}); ${hint}.`;
+    }
+
+    if (arg.startsWith('--no-') && !findOption(cmd, arg)) {
+      const positive = `--${arg.slice('--no-'.length)}`;
+      const opt = switchNamed(cmd, positive);
+      if (!opt || opt.negate || opt.defaultValue !== undefined) continue;
+      return `${arg} is not an option. ${positive} is off unless you pass it, so leave it out (to turn it on: \`${usage} ${positive}\`).`;
+    }
+  }
+  return undefined;
+}
+
+/** Walk leading subcommand names (`models catalog ...`) down to the leaf. */
+function resolveCommand(program: Command, args: string[]): { cmd: Command; rest: string[] } {
+  let cmd = program;
+  let i = 0;
+  for (; i < args.length; i++) {
+    const sub = cmd.commands.find((c) => c.name() === args[i] || c.aliases().includes(args[i]));
+    if (!sub) break;
+    cmd = sub;
+  }
+  return { cmd, rest: args.slice(i) };
+}
+
+/**
+ * A pass-through command (`vg serve <agent> ...`) forwards options after its
+ * first operand to another program, so their spelling is not ours to judge.
+ * Commander exposes the setter but not a getter for this.
+ */
+function isPassThrough(cmd: Command): boolean {
+  return (cmd as unknown as { _passThroughOptions?: boolean })._passThroughOptions === true;
+}
+
+/**
+ * Only the leaf's own options count: the program enables positional options,
+ * so a parent's flag is not accepted after a subcommand name (global flags are
+ * declared on every subcommand instead, see cli-options.applyGlobalOptions).
+ */
+function findOption(cmd: Command, long: string): Option | undefined {
+  return cmd.options.find((o) => o.long === long);
+}
+
+/** The option named `long` if it takes no value: `--vulns`, and also `--no-graph`. */
+function switchNamed(cmd: Command, long: string): Option | undefined {
+  const opt = findOption(cmd, long);
+  return opt && !opt.required && !opt.optional ? opt : undefined;
+}
+
+function commandPath(cmd: Command): string {
+  const names: string[] = [];
+  for (let c: Command | null = cmd; c; c = c.parent) names.unshift(c.name());
+  return names.join(' ');
+}

src/cli.ts

@@ -45,13 +45,14 @@ import { registerReview } from './commands/review.js';
 import { ConfigFileError } from './core-open/config.js';
 import { LockfileParseError } from './core-open/utils/lockfile-parse.js';
 import { UnsafeRootError } from './core-open/utils/root-safety.js';
-import { CliError, ExitCode } from './util/exit.js';
+import { CliError, ExitCode, usageError } from './util/exit.js';
 import { c, info, disableColor, exitAfterFlush } from './util/output.js';
 
 // Drift-reporting commands (merged from the Vibgrate CLI). These run on the
 // open base engine (`@vibgrate/core-open`) — no proprietary kernel.
 import { initCommand } from './reporting/commands/init.js';
 import { scanCommand } from './reporting/commands/scan.js';
+import { switchValueError } from './cli-switch-values.js';
 import { fixCommand } from './reporting/commands/fix.js';
 import { baselineCommand } from './reporting/commands/baseline.js';
 import { reportCommand } from './reporting/commands/report.js';
@@ -398,6 +399,9 @@ export async function main(argv = process.argv): Promise<void> {
     // dispatch() can throw (e.g. a moved-command hint) — keep it inside the
     // handler so those surface as a clean `error:` line, not a raw stack.
     const dispatched = dispatch(raw, cwd);
+    // Commander would reject these as an unknown option; say what works instead.
+    const switchError = switchValueError(program, dispatched);
+    if (switchError) throw usageError(switchError);
     await program.parseAsync(dispatched, { from: 'user' });
   } catch (err) {
     handleError(err);