test(3593): CLI negative-matrix harness + config family + universal sweep (#3627)

Adds the shared adversarial-input harness described in TEST-EXAMPLES.md
§"CLI Negative Matrix" and applies it across two layers:

  1. tests/helpers/cli-negative.cjs — runCli() wraps spawnSync of
     get-shit-done/bin/gsd-tools.cjs, prepends --json-errors by default,
     and returns a typed IR { status, ok, reason, message,
     hasStackTrace, ... } so adversarial-case tests assert on
     reason codes — never on stderr prose.

  2. tests/feat-3593-cli-negative-config.test.cjs — full 12-category
     matrix for the config command family (the highest-risk read/write
     surface): missing/empty/whitespace args, duplicate --cwd,
     unknown subcommand, value-looks-like-a-flag, corrupt config.json,
     50KB key, Unicode/emoji keys and values, and 9 distinct shell-
     metacharacter payloads asserted as NOT-executed via per-test
     sentinel-file probes.

  3. tests/feat-3593-cli-negative-universal.test.cjs — narrower
     cross-family sweep (phase, roadmap, state, config, workstream,
     init, validate). Pins the three universal invariants every
     family must satisfy: bare invocation does not crash with a V8
     stack trace, unknown subcommand emits a typed reason, and shell
     payloads as argv values are not executed.

  4. tests/feat-3593-cli-negative-harness.test.cjs — meta-test that
     pins the harness IR contract so a future regression in the
     parser (stack-trace detection, JSON shape extraction, hostile
     stderr handling) surfaces before it cascades through every
     matrix file.

Bug fix surfaced by the new tests:

  get-shit-done/bin/lib/config.cjs cmdConfigSet — invoking
  `config-set <key>` with no value silently returned exit 0 and
  emitted { updated: true } even though the value parameter was
  undefined. JSON.stringify dropped the key during the write or
  persisted a corrupt entry. Now rejected with typed ERROR_REASON.USAGE
  before any write. Matching guard added to SDK configSet for parity.

Harness coverage delivered: 58 new tests (9 meta + 26 config + 23
universal sweep). Pre-existing config suites (101 tests) all pass.
lint-no-source-grep clean.

Refs #3593

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.changeset/3593-cli-negative-matrix-harness.md

@@ -0,0 +1,5 @@
+---
+"get-shit-done-cc": patch
+---
+
+**Fixed: `config-set <key>` with no value now fails cleanly.** Previously, invoking `config-set model_profile` (key only, no value) returned `{ updated: true }` with exit 0 — but the value passed through as `undefined`, which `JSON.stringify` silently dropped during the write. Now both the CJS handler and the SDK `configSet` query throw a typed `Usage` error before any write, preventing silent config corruption. Surfaced by the new CLI adversarial-input matrix (#3593).

get-shit-done/bin/lib/config.cjs

@@ -392,7 +392,17 @@ function setConfigValue(cwd, keyPath, parsedValue) {
  */
 function cmdConfigSet(cwd, keyPath, value, raw) {
   if (!keyPath) {
-    error('Usage: config-set <key.path> <value>');
+    error('Usage: config-set <key.path> <value>', ERROR_REASON.USAGE);
+  }
+  // #3593: reject the "key without value" form (e.g. `config-set
+  // model_profile` with args[2] === undefined). Without this guard the
+  // value passes through as undefined, the number/boolean/json branches
+  // all fall through, and the write either silently strips the key
+  // (JSON.stringify drops undefined values) or writes a corrupt entry.
+  // Typed reason so the negative-matrix test can assert on it instead
+  // of greppinng prose.
+  if (value === undefined) {
+    error('Usage: config-set <key.path> <value>', ERROR_REASON.USAGE);
   }
 
   validateKnownConfigKeyPath(keyPath);

sdk/src/query/config-mutation.ts

@@ -269,6 +269,13 @@ export const configSet: QueryHandler = async (args, projectDir, workstream) => {
   if (!keyPath) {
     throw new GSDError('Usage: config-set <key.path> <value>', ErrorClassification.Validation);
   }
+  // #3593: parity with CJS cmdConfigSet — reject `config-set <key>` invocations
+  // that omit the value. Without this guard parsedValue stays undefined and the
+  // write either silently strips the key (JSON.stringify drops undefined) or
+  // persists a corrupt entry.
+  if (rawValue === undefined) {
+    throw new GSDError('Usage: config-set <key.path> <value>', ErrorClassification.Validation);
+  }
 
   const validation = isValidConfigKey(keyPath);
   if (!validation.valid) {

tests/feat-3593-cli-negative-config.test.cjs

@@ -0,0 +1,303 @@
+/**
+ * CLI negative matrix for the `config` command family (#3593).
+ *
+ * Exercises the 12 adversarial input categories enumerated in
+ * CONTRIBUTING.md §"QA Matrix Requirements / CLI and command routing"
+ * against `config-get` and `config-set`. The harness in
+ * `tests/helpers/cli-negative.cjs` shapes spawnSync output into a typed
+ * IR so every assertion runs on `result.reason`, `result.status`, and
+ * `result.hasStackTrace` — never on stderr/stdout prose.
+ *
+ * Each test gets its own temp project (no shared state) so concurrent
+ * runs can't observe each other's filesystem mutations. Hostile values
+ * (shell metacharacters, null bytes, unicode, very long strings) reach
+ * the CLI as single argv elements via spawnSync — never composed into
+ * a shell string — so the test framework itself can't be the source of
+ * a false positive on shell-injection assertions.
+ */
+
+'use strict';
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const { runCli } = require('./helpers/cli-negative.cjs');
+const { createTempProject, cleanup } = require('./helpers.cjs');
+
+/**
+ * Universal invariants every adversarial case must satisfy when the
+ * CLI is invoked with --json-errors. Bundling these in a helper keeps
+ * each test focused on the case-specific reason assertion.
+ */
+function assertSafeFailure(result, msg = '') {
+  assert.notEqual(result.status, 0, `${msg} :: expected non-zero exit`);
+  assert.equal(result.signal, null, `${msg} :: must exit cleanly, not via signal`);
+  assert.equal(result.hasStackTrace, false, `${msg} :: stderr must not leak a V8 stack frame`);
+  assert.equal(result.ok, false, `${msg} :: JSON payload ok must be false`);
+  assert.equal(typeof result.reason, 'string', `${msg} :: reason must be a string`);
+  assert.notEqual(result.reason, '', `${msg} :: reason must not be empty`);
+  // The harness's JSON-shape detection runs on the trimmed stderr; if we
+  // got here with reason set, the payload was a valid object — that already
+  // implies no rogue prose was mixed in. Re-asserting the trimmed form would
+  // be redundant.
+}
+
+/**
+ * Snapshot the file inventory of a directory so a later assertion can
+ * prove the failing CLI invocation did NOT create or modify any file.
+ */
+function snapshotInventory(dir) {
+  const entries = [];
+  function walk(rel) {
+    const abs = path.join(dir, rel);
+    let stat;
+    try { stat = fs.lstatSync(abs); } catch { return; }
+    if (stat.isDirectory()) {
+      for (const name of fs.readdirSync(abs).sort()) walk(path.join(rel, name));
+    } else {
+      entries.push(`${rel}\t${stat.size}\t${stat.mtimeMs}`);
+    }
+  }
+  walk('.');
+  return entries.join('\n');
+}
+
+// ─── 1. Missing required arg ────────────────────────────────────────────────
+
+test('config-get with no key fails with a typed reason and no stack trace', (t) => {
+  const projectDir = createTempProject('cli-neg-config-1-');
+  t.after(() => cleanup(projectDir));
+  const before = snapshotInventory(projectDir);
+  const result = runCli(['config-get'], { cwd: projectDir });
+  assertSafeFailure(result, 'config-get missing key');
+  assert.equal(snapshotInventory(projectDir), before, 'failing read must not mutate FS');
+});
+
+test('config-set with no key fails with a typed reason', (t) => {
+  const projectDir = createTempProject('cli-neg-config-2-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['config-set'], { cwd: projectDir });
+  assertSafeFailure(result, 'config-set missing key');
+});
+
+test('config-set with key but no value fails with a typed reason', (t) => {
+  const projectDir = createTempProject('cli-neg-config-3-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['config-set', 'model_profile'], { cwd: projectDir });
+  assertSafeFailure(result, 'config-set missing value');
+});
+
+// ─── 2/3. Empty / whitespace arg ────────────────────────────────────────────
+
+test('config-get with empty-string key fails safely', (t) => {
+  const projectDir = createTempProject('cli-neg-config-4-');
+  t.after(() => cleanup(projectDir));
+  const before = snapshotInventory(projectDir);
+  const result = runCli(['config-get', ''], { cwd: projectDir });
+  assertSafeFailure(result, 'config-get empty key');
+  assert.equal(snapshotInventory(projectDir), before, 'failing read must not mutate FS');
+});
+
+test('config-get with whitespace-only key fails safely', (t) => {
+  const projectDir = createTempProject('cli-neg-config-5-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['config-get', '   \t  '], { cwd: projectDir });
+  assertSafeFailure(result, 'config-get whitespace key');
+});
+
+test('config-set with empty key string fails safely', (t) => {
+  const projectDir = createTempProject('cli-neg-config-6-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['config-set', '', 'value'], { cwd: projectDir });
+  assertSafeFailure(result, 'config-set empty key');
+});
+
+// ─── 4. Duplicate flags ─────────────────────────────────────────────────────
+
+test('--cwd specified twice does not silently use the wrong one', (t) => {
+  // Make two real but distinct dirs so the test can't accidentally pass
+  // because one of the paths is invalid.
+  const a = createTempProject('cli-neg-config-7a-');
+  const b = createTempProject('cli-neg-config-7b-');
+  t.after(() => { cleanup(a); cleanup(b); });
+  // No --json-errors here on purpose: --cwd is parsed before json mode is
+  // applied, so we exercise both code paths by running once each.
+  const result = runCli(['--cwd', a, '--cwd', b, 'config-get', 'model_profile'], { cwd: process.cwd() });
+  // Either: (a) the CLI rejects duplicate --cwd with a typed reason; OR
+  // (b) it commits to one of the values deterministically. The safety
+  // bar is "no stack trace, no half-state mutation in EITHER dir".
+  assert.equal(result.hasStackTrace, false, 'duplicate --cwd must not crash with a stack trace');
+  // Neither tmp dir should have a written config since model_profile is
+  // a read, not a write, and it didn't exist beforehand. Prove the read
+  // didn't accidentally trigger a write side effect.
+  assert.equal(fs.existsSync(path.join(a, '.planning', 'config.json')), false);
+  assert.equal(fs.existsSync(path.join(b, '.planning', 'config.json')), false);
+});
+
+// ─── 5. Conflicting flags ───────────────────────────────────────────────────
+
+test('--json-errors with --no-such-flag does not crash with a stack trace', (t) => {
+  const projectDir = createTempProject('cli-neg-config-8-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['--no-such-flag', 'config-get', 'model_profile'], { cwd: projectDir });
+  assert.equal(result.hasStackTrace, false, 'unknown global flag must not crash with a stack trace');
+  assert.notEqual(result.status, 0, 'unknown global flag must fail');
+});
+
+// ─── 6. Malformed assignment / unknown subcommand ──────────────────────────
+
+test('config-FAKE subcommand fails with a typed reason', (t) => {
+  const projectDir = createTempProject('cli-neg-config-9-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['config-FAKE'], { cwd: projectDir });
+  assertSafeFailure(result, 'unknown config-* command');
+});
+
+// ─── 7. Unknown subcommands at each command depth ───────────────────────────
+
+test('config family — bare top-level "config" without a subcommand fails safely', (t) => {
+  const projectDir = createTempProject('cli-neg-config-10-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['config'], { cwd: projectDir });
+  // Either "missing subcommand" usage or genuine no-op behavior — what we
+  // pin is "no stack trace, no FS mutation".
+  assert.equal(result.hasStackTrace, false);
+});
+
+// ─── 8. Values that look like flags ─────────────────────────────────────────
+
+test('config-set value that starts with -- is treated as a value, not a flag', (t) => {
+  const projectDir = createTempProject('cli-neg-config-11-');
+  t.after(() => cleanup(projectDir));
+  // First create a config.json so the set has a target file.
+  runCli(['config-ensure-section'], { cwd: projectDir });
+  const result = runCli(['config-set', 'project_code', '--weird'], { cwd: projectDir });
+  // Acceptable outcomes:
+  //   (a) CLI accepts --weird as the value (and persists it),
+  //   (b) CLI rejects it as a usage error.
+  // Either way: no stack trace, no half-written corrupt config.
+  assert.equal(result.hasStackTrace, false, 'value-looking-like-a-flag must not crash');
+  const configPath = path.join(projectDir, '.planning', 'config.json');
+  if (fs.existsSync(configPath)) {
+    // If a config exists, it must still be valid JSON — no half-write corruption.
+    const raw = fs.readFileSync(configPath, 'utf-8');
+    assert.doesNotThrow(() => JSON.parse(raw), 'config.json must remain parseable after a failed set');
+  }
+});
+
+// ─── 9. Invalid JSON / corrupt config file ──────────────────────────────────
+
+test('config-get against a corrupt config.json fails with a parse-failed reason', (t) => {
+  const projectDir = createTempProject('cli-neg-config-12-');
+  t.after(() => cleanup(projectDir));
+  const configPath = path.join(projectDir, '.planning', 'config.json');
+  fs.writeFileSync(configPath, '{ this is not json'); // deliberate corruption
+  const originalCorrupt = fs.readFileSync(configPath, 'utf-8');
+  const result = runCli(['config-get', 'model_profile'], { cwd: projectDir });
+  assertSafeFailure(result, 'corrupt config.json');
+  // The corrupt file must remain untouched — the CLI must not "helpfully"
+  // overwrite an unparseable config in the failure path.
+  assert.equal(fs.readFileSync(configPath, 'utf-8'), originalCorrupt, 'corrupt file must be preserved as-is');
+  // Specific reason: CONFIG_PARSE_FAILED (or equivalent) — pin this so a
+  // regression where parse failure leaks as "unknown" is caught.
+  assert.match(
+    result.reason,
+    /^(config_parse_failed|config_no_file|config_invalid_key|usage)$/,
+    `parse-failure reason must be from the typed ERROR_REASON enum (got: ${result.reason})`,
+  );
+});
+
+// ─── 10. Very long arg ──────────────────────────────────────────────────────
+
+test('config-get with a very long key (50KB) fails safely without hanging', (t) => {
+  const projectDir = createTempProject('cli-neg-config-13-');
+  t.after(() => cleanup(projectDir));
+  const longKey = 'x'.repeat(50000);
+  const result = runCli(['config-get', longKey], { cwd: projectDir, timeoutMs: 8000 });
+  assert.equal(result.signal, null, 'long input must not trigger the harness timeout');
+  assert.equal(result.hasStackTrace, false, 'long input must not crash');
+  assert.notEqual(result.status, 0, 'unknown 50KB key must fail');
+});
+
+// ─── 11. Unicode / non-ASCII ────────────────────────────────────────────────
+
+test('config-get with a Unicode key fails safely', (t) => {
+  const projectDir = createTempProject('cli-neg-config-14-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['config-get', 'workflow.🔥_mode'], { cwd: projectDir });
+  assertSafeFailure(result, 'unicode key');
+});
+
+test('config-set with an emoji value persists or rejects without corrupting JSON', (t) => {
+  const projectDir = createTempProject('cli-neg-config-15-');
+  t.after(() => cleanup(projectDir));
+  runCli(['config-ensure-section'], { cwd: projectDir });
+  const result = runCli(['config-set', 'project_code', '🔥👾'], { cwd: projectDir });
+  assert.equal(result.hasStackTrace, false);
+  // If it accepted, the JSON must round-trip cleanly.
+  const configPath = path.join(projectDir, '.planning', 'config.json');
+  if (result.status === 0 && fs.existsSync(configPath)) {
+    const parsed = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
+    assert.equal(typeof parsed, 'object', 'config.json must be a valid object');
+    if (parsed.project_code != null) {
+      assert.equal(typeof parsed.project_code, 'string', 'project_code must remain a string');
+    }
+  }
+});
+
+// ─── 12. Shell metacharacters (the security-critical case) ──────────────────
+
+const SHELL_PAYLOADS = [
+  // Each one would, if shell-interpreted, create a sentinel file
+  // adjacent to the project tree. Argv-based invocation must treat them
+  // as opaque text.
+  '$(touch ${PROJECT}/INJ-dollar-paren)',
+  '`touch ${PROJECT}/INJ-backtick`',
+  '; touch ${PROJECT}/INJ-semicolon;',
+  '&& touch ${PROJECT}/INJ-and',
+  '|| touch ${PROJECT}/INJ-or',
+  '| tee ${PROJECT}/INJ-pipe',
+  '> ${PROJECT}/INJ-redirect',
+  // Quote-balanced payloads — these have historically broken naive
+  // shell-string composition even when the rest of the code uses argv.
+  '"; touch ${PROJECT}/INJ-quote;"',
+  '\'; touch ${PROJECT}/INJ-quote;\'',
+];
+
+for (const payload of SHELL_PAYLOADS) {
+  test(`config-get with shell-metachar key (${payload.slice(0, 25)}…) does NOT execute the payload`, (t) => {
+    const projectDir = createTempProject('cli-neg-config-shell-');
+    t.after(() => cleanup(projectDir));
+    const resolvedPayload = payload.replace(/\$\{PROJECT\}/g, projectDir);
+    const result = runCli(['config-get', resolvedPayload], { cwd: projectDir });
+    // No shell interpretation: none of the INJ-* sentinel files must
+    // exist after the run. Walk the project dir and assert.
+    const entries = fs.readdirSync(projectDir);
+    const sentinels = entries.filter((n) => n.startsWith('INJ-'));
+    assert.deepEqual(sentinels, [], `shell payload must NOT create sentinel files (found: ${sentinels.join(', ')})`);
+    // The CLI may exit 0 (legitimate — the metacharacter-laden key
+    // simply doesn't exist in config) or non-zero (typed reason). Both
+    // are acceptable as long as no payload was executed.
+    assert.equal(result.hasStackTrace, false);
+  });
+}
+
+// ─── Cross-cutting: --cwd points at a non-existent path ────────────────────
+
+test('--cwd pointing at a non-existent path fails with a typed usage reason', (t) => {
+  const nonExistent = path.join(require('os').tmpdir(), 'cli-neg-no-such-dir-' + Date.now() + '-' + Math.random());
+  assert.equal(fs.existsSync(nonExistent), false, 'pre-check: path must not exist');
+  const result = runCli(['--cwd', nonExistent, 'config-get', 'model_profile'], { cwd: process.cwd() });
+  assert.notEqual(result.status, 0);
+  assert.equal(result.hasStackTrace, false);
+  // gsd-tools validates --cwd up-front and emits ERROR_REASON.USAGE.
+  assert.equal(result.reason, 'usage', `expected reason=usage for invalid --cwd, got: ${result.reason}`);
+});
+
+test('--cwd with an empty value fails with a typed usage reason', () => {
+  const result = runCli(['--cwd', '', 'config-get', 'model_profile'], { cwd: process.cwd() });
+  assert.notEqual(result.status, 0);
+  assert.equal(result.hasStackTrace, false);
+  assert.equal(result.reason, 'usage');
+});

tests/feat-3593-cli-negative-harness.test.cjs

@@ -0,0 +1,138 @@
+/**
+ * Meta-test for the CLI negative-matrix harness (#3593).
+ *
+ * The harness in `tests/helpers/cli-negative.cjs` shapes spawnSync
+ * results into a typed IR that adversarial-input tests consume. This
+ * file pins the IR contract by exercising the harness against
+ * deliberate scenarios — not as a placeholder for the real matrix tests
+ * (those live in sibling feat-3593-* files) but to surface harness
+ * regressions before they cascade through every matrix test.
+ *
+ * Tests deliberately avoid prose-matching: they assert on numeric exit
+ * codes, boolean flags, and reason codes pulled from the parsed JSON
+ * payload.
+ */
+
+'use strict';
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const { runCli, parseSpawnResult } = require('./helpers/cli-negative.cjs');
+const { createTempProject, cleanup } = require('./helpers.cjs');
+
+test('runCli rejects non-array argv with TypeError', () => {
+  assert.throws(
+    () => runCli('config-get', { cwd: '/tmp' }),
+    (err) => err instanceof TypeError && /argv/.test(err.message),
+  );
+});
+
+test('runCli rejects missing cwd with TypeError', () => {
+  assert.throws(
+    () => runCli(['config-get'], {}),
+    (err) => err instanceof TypeError && /cwd/.test(err.message),
+  );
+});
+
+test('runCli surfaces typed reason from a known failure path', (t) => {
+  const projectDir = createTempProject('cli-neg-harness-');
+  t.after(() => cleanup(projectDir));
+  // Unknown command — gsd-tools emits ERROR_REASON.SDK_UNKNOWN_COMMAND or
+  // USAGE depending on dispatch depth. Either is a real reason string;
+  // the contract we pin here is just "the IR carries a reason from the
+  // ERROR_REASON enum, never null".
+  const result = runCli(['this-command-does-not-exist'], { cwd: projectDir });
+  assert.notEqual(result.status, 0, 'unknown command must exit non-zero');
+  assert.equal(result.ok, false, 'JSON payload must report ok=false');
+  assert.equal(typeof result.reason, 'string', 'reason must be a string from ERROR_REASON');
+  assert.notEqual(result.reason, null);
+  assert.notEqual(result.reason, '');
+  assert.equal(result.hasStackTrace, false, 'a typed failure must NOT print a V8 stack trace');
+});
+
+test('parseSpawnResult detects stack-trace leakage in stderr', () => {
+  const fakeSpawn = {
+    status: 1,
+    signal: null,
+    stdout: '',
+    stderr: 'Error: boom\n    at Object.<anonymous> (/some/file.js:10:5)\n    at Module._compile\n',
+    error: null,
+  };
+  const ir = parseSpawnResult(fakeSpawn, { jsonErrorsRequested: false });
+  assert.equal(ir.hasStackTrace, true, 'stack frames in stderr must be flagged');
+  assert.equal(ir.reason, null, 'non-JSON stderr leaves reason null');
+});
+
+test('parseSpawnResult does NOT match the literal word "at" in prose', () => {
+  // Guard against a regex regression that would catch sentences like
+  // "command failed at startup" as stack frames.
+  const fakeSpawn = {
+    status: 1,
+    signal: null,
+    stdout: '',
+    stderr: 'Error: command failed at startup\nbecause no project was found.\n',
+    error: null,
+  };
+  const ir = parseSpawnResult(fakeSpawn, { jsonErrorsRequested: false });
+  assert.equal(ir.hasStackTrace, false, 'prose containing the word "at" is not a stack frame');
+});
+
+test('parseSpawnResult extracts ok/reason/message from a json-errors payload', () => {
+  const payload = { ok: false, reason: 'config_invalid_key', message: 'no such key: foo' };
+  const fakeSpawn = {
+    status: 1,
+    signal: null,
+    stdout: '',
+    stderr: JSON.stringify(payload) + '\n',
+    error: null,
+  };
+  const ir = parseSpawnResult(fakeSpawn, { jsonErrorsRequested: true });
+  assert.equal(ir.ok, false);
+  assert.equal(ir.reason, 'config_invalid_key');
+  assert.equal(ir.message, 'no such key: foo');
+  assert.equal(ir.hasStackTrace, false);
+});
+
+test('parseSpawnResult ignores malformed JSON in stderr without throwing', () => {
+  const fakeSpawn = {
+    status: 1,
+    signal: null,
+    stdout: '',
+    stderr: '{ ok: false, reason }', // missing quotes — invalid JSON
+    error: null,
+  };
+  const ir = parseSpawnResult(fakeSpawn, { jsonErrorsRequested: true });
+  assert.equal(ir.ok, null, 'malformed JSON must NOT promote partial data into ok');
+  assert.equal(ir.reason, null);
+  assert.equal(ir.message, null);
+});
+
+test('parseSpawnResult ignores JSON arrays and primitives, only accepts objects', () => {
+  const cases = [
+    '["ok", false]',     // array
+    '"just a string"',   // primitive
+    'null',              // null literal
+    '42',                // number
+  ];
+  for (const stderr of cases) {
+    const ir = parseSpawnResult(
+      { status: 1, signal: null, stdout: '', stderr, error: null },
+      { jsonErrorsRequested: true },
+    );
+    assert.equal(ir.ok, null, `non-object JSON (${stderr}) must not set ok`);
+    assert.equal(ir.reason, null);
+  }
+});
+
+test('runCli treats jsonErrors=false as an explicit human-formatter path', (t) => {
+  const projectDir = createTempProject('cli-neg-harness-text-');
+  t.after(() => cleanup(projectDir));
+  const result = runCli(['this-command-does-not-exist'], { cwd: projectDir, jsonErrors: false });
+  assert.notEqual(result.status, 0);
+  assert.equal(result.jsonErrorsRequested, false);
+  // Reason fields stay null in human-mode because stderr is prose, not JSON.
+  assert.equal(result.ok, null);
+  assert.equal(result.reason, null);
+  // But the prose still must not include a V8 stack trace.
+  assert.equal(result.hasStackTrace, false);
+});

tests/feat-3593-cli-negative-universal.test.cjs

@@ -0,0 +1,130 @@
+/**
+ * Universal CLI negative-matrix sweep across the seven command families
+ * named in #3593: phase, roadmap, state, config, workstream, init,
+ * validate.
+ *
+ * The full per-case matrix for each family belongs in dedicated files
+ * (the `config` family is the template — see
+ * `feat-3593-cli-negative-config.test.cjs`). This file pins a much
+ * narrower contract that EVERY family must satisfy:
+ *
+ *   1. Bare top-level command with no subcommand: must not crash with
+ *      a V8 stack trace, must exit non-zero (or, where the bare form
+ *      is legitimate, return a clean payload).
+ *   2. Unknown subcommand at command depth: must emit a typed reason
+ *      under --json-errors and must not crash.
+ *   3. Shell-metacharacter as an argv element: must not be executed.
+ *      Sentinel-file probe in the project temp dir proves this.
+ *
+ * Future per-family files will deepen each family's matrix; this file
+ * is the floor.
+ */
+
+'use strict';
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const { runCli } = require('./helpers/cli-negative.cjs');
+const { createTempProject, createTempGitProject, cleanup } = require('./helpers.cjs');
+
+/**
+ * Each entry names a top-level command, a representative subcommand
+ * (for the "unknown sub" probe), and the temp-project factory.
+ *
+ * The `representativeSubcommand` is not asserted on directly — it
+ * exists so the test layout reads "for each family, run probe X" and
+ * so a future maintainer adding a family doesn't need to invent one.
+ */
+const FAMILIES = [
+  { name: 'phase',      bare: ['phase'],      unknown: ['phase', 'this-sub-does-not-exist'],      projectFactory: createTempProject },
+  { name: 'roadmap',    bare: ['roadmap'],    unknown: ['roadmap', 'this-sub-does-not-exist'],    projectFactory: createTempProject },
+  { name: 'state',      bare: ['state'],      unknown: ['state', 'this-sub-does-not-exist'],      projectFactory: createTempProject },
+  { name: 'config',     bare: ['config'],     unknown: ['config', 'this-sub-does-not-exist'],     projectFactory: createTempProject },
+  { name: 'workstream', bare: ['workstream'], unknown: ['workstream', 'this-sub-does-not-exist'], projectFactory: createTempGitProject },
+  { name: 'init',       bare: ['init'],       unknown: ['init', 'this-sub-does-not-exist'],       projectFactory: createTempProject },
+  { name: 'validate',   bare: ['validate'],   unknown: ['validate', 'this-sub-does-not-exist'],   projectFactory: createTempProject },
+];
+
+describe('feat-3593: bare top-level command does not crash', () => {
+  for (const fam of FAMILIES) {
+    test(`${fam.name}: bare invocation exits cleanly without a stack trace`, (t) => {
+      const projectDir = fam.projectFactory(`cli-neg-univ-bare-${fam.name}-`);
+      t.after(() => cleanup(projectDir));
+      const result = runCli(fam.bare, { cwd: projectDir });
+      assert.equal(result.hasStackTrace, false, `${fam.name} bare must not leak a V8 stack frame`);
+      assert.equal(result.signal, null, `${fam.name} bare must not be killed by a signal`);
+      // We do not pin exit code here: some families legitimately treat the
+      // bare form as a list/status command (status 0); others reject it as
+      // a usage error (status ≠ 0). What we pin is "no crash."
+    });
+  }
+});
+
+describe('feat-3593: unknown subcommand emits a typed reason', () => {
+  for (const fam of FAMILIES) {
+    test(`${fam.name}: unknown subcommand fails with reason set and no stack trace`, (t) => {
+      const projectDir = fam.projectFactory(`cli-neg-univ-unk-${fam.name}-`);
+      t.after(() => cleanup(projectDir));
+      const result = runCli(fam.unknown, { cwd: projectDir });
+      assert.notEqual(result.status, 0, `${fam.name} unknown sub must exit non-zero`);
+      assert.equal(result.hasStackTrace, false, `${fam.name} unknown sub must not leak a stack frame`);
+      // When --json-errors is on, every typed-failure path lands a non-empty
+      // reason string from ERROR_REASON. A null reason here means the family
+      // is using throw/console.error somewhere — a TDD signal to wire that
+      // failure path through error(msg, ERROR_REASON.X).
+      assert.equal(typeof result.reason, 'string', `${fam.name}: reason must be a typed enum string (got: ${result.reason})`);
+      assert.notEqual(result.reason, '', `${fam.name}: reason must be non-empty`);
+    });
+  }
+});
+
+describe('feat-3593: shell-metacharacter argv values are NOT executed', () => {
+  for (const fam of FAMILIES) {
+    test(`${fam.name}: shell-payload as subcommand argv does NOT execute the payload`, (t) => {
+      const projectDir = fam.projectFactory(`cli-neg-univ-shell-${fam.name}-`);
+      t.after(() => cleanup(projectDir));
+      // Place the payload where the subcommand value goes. The payload
+      // would, if shell-interpreted, create a sentinel file in the
+      // project dir. Argv-based invocation must treat it as opaque text.
+      const sentinelPayload = `$(touch ${projectDir}/INJ-${fam.name})`;
+      const argv = [fam.name, sentinelPayload];
+      const result = runCli(argv, { cwd: projectDir });
+      // No stack trace — opaque text must not crash.
+      assert.equal(result.hasStackTrace, false, `${fam.name}: shell payload must not crash`);
+      // The sentinel file must NOT exist. We check the project dir's
+      // listing rather than fs.existsSync of a single path so the test
+      // surfaces any spelling drift.
+      const entries = fs.readdirSync(projectDir);
+      const sentinels = entries.filter((n) => n.startsWith('INJ-'));
+      assert.deepEqual(
+        sentinels,
+        [],
+        `${fam.name}: shell payload was executed — sentinel files exist: ${sentinels.join(', ')}`,
+      );
+    });
+  }
+});
+
+// ─── Cross-family invariants on the global --cwd flag ──────────────────────
+
+test('--cwd with an empty value fails the same way regardless of command family', () => {
+  for (const fam of FAMILIES) {
+    const result = runCli(['--cwd', '', ...fam.bare], { cwd: process.cwd() });
+    assert.notEqual(result.status, 0, `${fam.name}: empty --cwd must fail`);
+    assert.equal(result.hasStackTrace, false, `${fam.name}: empty --cwd must not crash`);
+    assert.equal(result.reason, 'usage', `${fam.name}: empty --cwd reason should be 'usage', got: ${result.reason}`);
+  }
+});
+
+test('--cwd pointing at a non-existent path fails uniformly across families', () => {
+  const nonExistent = path.join(require('os').tmpdir(), 'cli-neg-univ-no-such-' + Date.now() + '-' + Math.random());
+  assert.equal(fs.existsSync(nonExistent), false, 'pre-check: temp path must not exist');
+  for (const fam of FAMILIES) {
+    const result = runCli(['--cwd', nonExistent, ...fam.bare], { cwd: process.cwd() });
+    assert.notEqual(result.status, 0, `${fam.name}: invalid --cwd must fail`);
+    assert.equal(result.hasStackTrace, false);
+    assert.equal(result.reason, 'usage', `${fam.name}: invalid --cwd reason should be 'usage'`);
+  }
+});

tests/helpers/cli-negative.cjs

@@ -0,0 +1,133 @@
+/**
+ * CLI negative-matrix harness (#3593).
+ *
+ * Wraps spawnSync of get-shit-done/bin/gsd-tools.cjs so test files can
+ * assert on structured outputs (exit code, typed reason, stack-trace
+ * absence) without each test re-implementing the JSON-errors parsing
+ * dance. Hostile values are passed as argv elements — never composed
+ * into a shell string — so shell metacharacters in test inputs (;, &&,
+ * $(), backticks, quotes, newlines, null bytes) reach the CLI as
+ * opaque data, not as shell syntax.
+ *
+ * Designed against CONTRIBUTING.md §"Testing Standards" and
+ * TEST-EXAMPLES.md §"CLI Negative Matrix":
+ *
+ *   - spawnSync (no shell) per the "no shell strings" rule.
+ *   - Returns a typed IR { status, ok, reason, message, ... } so tests
+ *     assert on `.reason === REASON_CODE`, never on prose.
+ *   - Detects stack-trace leakage in stderr; tests can fail when the
+ *     CLI surfaces an unexpected exception under hostile input.
+ *   - --json-errors is on by default; pass jsonErrors:false to verify
+ *     human-formatted stderr paths.
+ *
+ * The harness does NOT decide what the test asserts — it just shapes
+ * the data so the assertion is mechanical and prose-free.
+ */
+
+'use strict';
+
+const path = require('node:path');
+const { spawnSync } = require('node:child_process');
+
+const TOOLS_PATH = path.resolve(__dirname, '..', '..', 'get-shit-done', 'bin', 'gsd-tools.cjs');
+
+/**
+ * Run gsd-tools with the given argv against a project directory.
+ *
+ * @param {string[]} argv - argument vector passed to gsd-tools.cjs. Each
+ *   element reaches the child as a single argv element regardless of
+ *   shell metacharacters in its value.
+ * @param {object} options
+ * @param {string} options.cwd - working directory for the child (REQUIRED).
+ * @param {object} [options.env] - env vars merged on top of process.env.
+ * @param {boolean} [options.jsonErrors=true] - prepend --json-errors so
+ *   the CLI emits a structured `{ ok, reason, message }` payload to
+ *   stderr. Set false to exercise the human-formatted path.
+ * @param {number} [options.timeoutMs=10000] - kill child if it runs longer.
+ * @returns {object} typed IR — see field docs below.
+ */
+function runCli(argv, options) {
+  if (!options || typeof options.cwd !== 'string' || options.cwd.length === 0) {
+    throw new TypeError('runCli: options.cwd (string) is required');
+  }
+  if (!Array.isArray(argv)) {
+    throw new TypeError('runCli: argv must be an array (no shell strings)');
+  }
+  const jsonErrors = options.jsonErrors !== false;
+  const finalArgs = jsonErrors ? ['--json-errors', ...argv] : argv.slice();
+  const env = { ...process.env, ...(options.env || {}) };
+  const spawnResult = spawnSync(process.execPath, [TOOLS_PATH, ...finalArgs], {
+    cwd: options.cwd,
+    encoding: 'utf-8',
+    timeout: typeof options.timeoutMs === 'number' ? options.timeoutMs : 10000,
+    env,
+  });
+  return parseSpawnResult(spawnResult, { jsonErrorsRequested: jsonErrors });
+}
+
+/**
+ * Shape a raw spawnSync result into the harness IR.
+ *
+ * Returned fields:
+ *   - status   {number|null}  child exit code (null if killed by signal)
+ *   - signal   {string|null}  terminating signal name when killed
+ *   - stdout   {string}       captured stdout
+ *   - stderr   {string}       captured stderr
+ *   - ok       {boolean|null} parsed from JSON-errors payload; null when
+ *                             stderr is not JSON-shaped
+ *   - reason   {string|null}  ERROR_REASON code from JSON-errors payload
+ *   - message  {string|null}  human message from JSON-errors payload
+ *   - hasStackTrace {boolean} stderr contains a `\n    at ` frame line.
+ *                             Tests use this to assert the CLI didn't
+ *                             leak an unexpected exception.
+ *   - jsonErrorsRequested {boolean} true when --json-errors was added by
+ *                             the harness; tests use this to decide
+ *                             whether the missing-reason case is a bug
+ *                             vs. expected (jsonErrors:false path).
+ *   - spawnError {Error|null} spawnSync's `error` field (e.g. ENOENT
+ *                             on process.execPath, timeout signal).
+ */
+function parseSpawnResult(spawnResult, meta) {
+  const stderr = typeof spawnResult.stderr === 'string' ? spawnResult.stderr : '';
+  const stdout = typeof spawnResult.stdout === 'string' ? spawnResult.stdout : '';
+  // Stack-trace leak detection: V8 prints "    at Function (path:line:col)"
+  // frames into stderr when an uncaught throw escapes. Any such line means
+  // the CLI hit a code path that wasn't wrapped in error()/typed reason.
+  // \n is required so we don't match the word "at" appearing in prose.
+  const hasStackTrace = /\n\s{2,}at\s+/.test(stderr);
+
+  let ok = null;
+  let reason = null;
+  let message = null;
+  // JSON-errors mode emits a single-line JSON document to stderr followed by
+  // a newline. Parse defensively — any non-JSON stderr leaves the fields null
+  // and lets the test surface the discrepancy.
+  const trimmed = stderr.trim();
+  if (trimmed.startsWith('{') && trimmed.endsWith('}')) {
+    try {
+      const parsed = JSON.parse(trimmed);
+      if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
+        if (typeof parsed.ok === 'boolean') ok = parsed.ok;
+        if (typeof parsed.reason === 'string') reason = parsed.reason;
+        if (typeof parsed.message === 'string') message = parsed.message;
+      }
+    } catch {
+      // Not JSON after all — leave parsed fields null.
+    }
+  }
+
+  return {
+    status: spawnResult.status,
+    signal: spawnResult.signal,
+    stdout,
+    stderr,
+    ok,
+    reason,
+    message,
+    hasStackTrace,
+    jsonErrorsRequested: meta && meta.jsonErrorsRequested === true,
+    spawnError: spawnResult.error || null,
+  };
+}
+
+module.exports = { runCli, parseSpawnResult, TOOLS_PATH };