Fix: gstack-config get returns "" with exit 0 for keys that have no default

Skill preambles read configuration with

    VAR=$(gstack-config get <key> 2>/dev/null || echo "<default>")

and that fallback only fires on a non-zero exit. lookup_default ended in a
catch-all that echoed "" and returned 0, so for any key missing from the table
VAR came back empty and the default written right there in the preamble was
unreachable. The skill then branched on a value it never specified: "skip
entirely if QUESTION_TUNING is false", reached with QUESTION_TUNING="".

Four keys that skills actually read had no entry and took that path:

    question_tuning         -> callers assume "false"
    repo_mode               -> callers assume "unknown"
    team_mode               -> callers assume "false"
    transcript_ingest_mode  -> callers assume "off"

Each default above is the value the call sites already substitute in their own
`|| echo` fallback, so this only makes reachable what was already intended.

The catch-all now returns non-zero. That is deliberately scoped to the
unknown-key arm alone: keys whose default is intentionally empty still exit 0,
because "" is their real answer and their callers depend on it --
cross_project_learnings ("unset triggers the first-time prompt"),
redact_repo_visibility ("empty falls through to gh/glab detection"),
salience_allowlist, user_slug_at_*. Making every empty answer an error would
have broken those.

test/gstack-config-defaults.test.ts pins the class rather than the four
instances: it parses the case arms and asserts every `gstack-config get <key>`
site in the tree is covered, so adding a read without a default fails CI. It
also pins the exit-code contract in both directions. Verified failing against
the pre-fix script, where it names exactly those four keys.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

bin/gstack-config

@@ -161,7 +161,23 @@ lookup_default() {
     brain_trust_policy*) echo "unset" ;;
     salience_allowlist) echo "" ;;
     user_slug_at_*) echo "" ;;
-    *) echo "" ;;
+    # Read by skill preambles but missing from this table, so they fell through
+    # to the catch-all and came back "" with exit 0. Values below are the ones
+    # the callers already assume in their own `|| echo "<default>"` fallback.
+    question_tuning) echo "false" ;;
+    team_mode) echo "false" ;;
+    transcript_ingest_mode) echo "off" ;;
+    repo_mode) echo "unknown" ;;
+    # Unknown key: exit non-zero instead of printing "". The fallback pattern
+    # the preambles use,
+    #   VAR=$(gstack-config get <key> 2>/dev/null || echo "<default>")
+    # only fires on a non-zero exit, so a catch-all echoing "" with exit 0 left
+    # VAR empty and the written default unreachable.
+    # Deliberately *only* the unknown-key path: the keys above whose default is
+    # intentionally empty (cross_project_learnings, salience_allowlist,
+    # user_slug_at_*, redact_repo_visibility) keep exit 0, because "" is their
+    # real answer and their callers rely on it.
+    *) return 1 ;;
   esac
 }
 
@@ -297,7 +313,12 @@ case "${1:-}" in
     fi
     VALUE=$(read_config_value "$KEY" || true)
     if [ -z "$VALUE" ]; then
-      VALUE=$(lookup_default "$KEY")
+      # lookup_default exits non-zero for a key it does not know. Propagate
+      # that, so the caller's `|| echo "<default>"` can fire. A known key whose
+      # default is empty still exits 0 and prints "".
+      if ! VALUE=$(lookup_default "$KEY"); then
+        exit 1
+      fi
     fi
     printf '%s' "$VALUE"
     ;;

test/gstack-config-defaults.test.ts

@@ -0,0 +1,137 @@
+/**
+ * gstack-config default-table completeness (gate, free).
+ *
+ * Skill preambles read configuration with
+ *
+ *     VAR=$(gstack-config get <key> 2>/dev/null || echo "<default>")
+ *
+ * and that fallback only fires on a NON-ZERO exit. `get` used to answer a key
+ * it did not know with "" and exit 0, so VAR came back empty and the default
+ * written right there in the preamble was unreachable. The skill then branched
+ * on a value it never specified -- "skip entirely if QUESTION_TUNING is false"
+ * reached with QUESTION_TUNING="".
+ *
+ * Four keys skills actually read had no entry in lookup_default and took that
+ * path: question_tuning, repo_mode, team_mode, transcript_ingest_mode.
+ *
+ * Three invariants are pinned so the class cannot reopen:
+ *
+ *   1. every key read anywhere in the tree is matched by an arm of the DEFAULTS
+ *      table. Add a `gstack-config get some_new_key` to a preamble without
+ *      adding its default and this test fails. Checked by parsing the case arms
+ *      rather than shelling out per key, which keeps it fast and makes the
+ *      failure name the key.
+ *   2. a genuinely unknown key exits non-zero, so the caller fallback fires.
+ *   3. a known key whose default is intentionally empty still exits 0 --
+ *      cross_project_learnings ("unset triggers the first-time prompt") and
+ *      redact_repo_visibility ("empty falls through to gh/glab detection")
+ *      depend on receiving "" successfully.
+ */
+
+import { describe, test, expect } from 'bun:test';
+import { spawnSync } from 'child_process';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+
+const ROOT = path.resolve(import.meta.dir, '..');
+const CONFIG_BIN = path.join(ROOT, 'bin', 'gstack-config');
+const SELF = 'gstack-config-defaults.test.ts';
+
+// Isolated state dir, so a value the developer happens to have set in their own
+// ~/.gstack/config.yaml cannot mask a missing default.
+const STATE = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-config-test-'));
+
+function get(key: string): { out: string; code: number } {
+  const r = spawnSync('bash', [CONFIG_BIN, 'get', key], {
+    encoding: 'utf-8',
+    env: { ...process.env, GSTACK_STATE_ROOT: STATE },
+  });
+  return { out: r.stdout ?? '', code: r.status ?? -1 };
+}
+
+/** Case-arm patterns of lookup_default, in order, excluding the catch-all. */
+function defaultArms(): string[] {
+  const src = fs.readFileSync(CONFIG_BIN, 'utf-8');
+  const body = src.slice(src.indexOf('lookup_default()'));
+  const end = body.indexOf('\n}');
+  const arms: string[] = [];
+  // e.g. `    proactive) echo "true" ;;` or `    user_slug_at_*) echo "" ;;`
+  for (const m of body.slice(0, end).matchAll(/^\s{4}([a-zA-Z0-9_*]+)\)/gm)) {
+    if (m[1] !== '*') arms.push(m[1]);
+  }
+  return arms;
+}
+
+function isCovered(key: string, arms: string[]): boolean {
+  return arms.some((a) =>
+    a.endsWith('*') ? key.startsWith(a.slice(0, -1)) : key === a,
+  );
+}
+
+const SKIP_DIRS = new Set(['node_modules', '.git', 'dist', 'build', '.next']);
+
+/** Every `gstack-config get <key>` call site in the tree. */
+function keysReadInTree(): string[] {
+  const keys = new Set<string>();
+  // [ \t]+ rather than \s+: \s crosses newlines and would pair a trailing
+  // "gstack-config get" with the first word of the next line.
+  const re = /gstack-config["']?[ \t]+get[ \t]+([a-zA-Z0-9_]+)/g;
+  const stack = [ROOT];
+  while (stack.length) {
+    const cur = stack.pop()!;
+    let entries: fs.Dirent[];
+    try {
+      entries = fs.readdirSync(cur, { withFileTypes: true });
+    } catch {
+      continue;
+    }
+    for (const ent of entries) {
+      if (SKIP_DIRS.has(ent.name) || ent.isSymbolicLink()) continue;
+      const full = path.join(cur, ent.name);
+      if (ent.isDirectory()) {
+        stack.push(full);
+        continue;
+      }
+      // Skip this file: its own prose cites example keys.
+      if (ent.name === SELF) continue;
+      if (!/\.(md|ts|sh)$|^gstack-[a-z-]+$/.test(ent.name)) continue;
+      let text: string;
+      try {
+        text = fs.readFileSync(full, 'utf-8');
+      } catch {
+        continue;
+      }
+      for (const m of text.matchAll(re)) keys.add(m[1]);
+    }
+  }
+  return [...keys].sort();
+}
+
+describe('gstack-config defaults (gate, free)', () => {
+  test('every key read in the tree is covered by the DEFAULTS table', () => {
+    const arms = defaultArms();
+    expect(arms.length).toBeGreaterThan(10); // the parse actually found the table
+    const uncovered = keysReadInTree().filter((k) => !isCovered(k, arms));
+    expect(uncovered).toEqual([]);
+  });
+
+  test('an unknown key exits non-zero, so the caller fallback fires', () => {
+    const r = get('definitely_not_a_gstack_key_9f3a');
+    expect(r.code).not.toBe(0);
+    expect(r.out).toBe('');
+  });
+
+  test('a known key whose default is intentionally empty still exits 0', () => {
+    for (const key of ['cross_project_learnings', 'salience_allowlist', 'redact_repo_visibility']) {
+      expect({ key, ...get(key) }).toEqual({ key, out: '', code: 0 });
+    }
+  });
+
+  test('the four keys that regressed resolve to the values their callers assume', () => {
+    expect(get('question_tuning').out).toBe('false');
+    expect(get('team_mode').out).toBe('false');
+    expect(get('transcript_ingest_mode').out).toBe('off');
+    expect(get('repo_mode').out).toBe('unknown');
+  });
+});