fix(skill-start): unknown proactive values read as core; first-run tips name core skills and stay quiet under off

A hand-edited config can hold any value and only set validates, so STATUS normalizes anything but core|all|off|true|false to core. The first-run and first-loop tips named /spec, /investigate and /plan-eng-review whatever the setting; they now name the core four and are skipped under off. The fallback regression test folds into test/proactive-skill-start.test.ts, the duplicate header assertion leaves proactive-config, and the frozen-length preamble test is retired (the size guard enforces it).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-skill-start

@@ -133,6 +133,9 @@ find "$_GH/sessions" -mmin +120 -type f -exec rm {} + 2>/dev/null || true
 _SESSIONS=$(find "$_GH/sessions" -mmin -120 -type f 2>/dev/null | wc -l | tr -d ' ')
 
 _PROACTIVE=$("$_BIN/gstack-config" get proactive 2>/dev/null || echo "core")
+# A hand-edited config can hold anything; the router and preambles only know
+# core | all | off (true = all, false = off), so anything else reads as core.
+case "$_PROACTIVE" in core|all|off|true|false) ;; *) _PROACTIVE="core" ;; esac
 _PROACTIVE_PROMPTED=$([ -f "$_GH/.proactive-prompted" ] && echo "yes" || echo "no")
 # Branch name is embedded in concatenated JSON below; strip anything outside
 # the gstack-slug BRANCH alphabet so a hostile ref name can't forge fields.
@@ -562,12 +565,14 @@ fi
 if [ "$_ACTIVATED" = "no" ]; then
   _FT_TIP=""
   case "$_FIRST_TASK" in
-    greenfield) _FT_TIP="Fresh repo — shape it first with \`/spec\` or \`/office-hours\`." ;;
-    code_node|code_python|code_rust|code_go|code_ruby|code_ios) _FT_TIP="There's code here — \`/qa\` to see it work, or \`/investigate\` if something's off." ;;
+    greenfield) _FT_TIP="Fresh repo — shape it first with \`/office-hours\`." ;;
+    code_node|code_python|code_rust|code_go|code_ruby|code_ios) _FT_TIP="There's code here — \`/qa\` to see it work, or \`/review\` on your latest diff." ;;
     branch_ahead) _FT_TIP="Unshipped work on this branch — \`/review\` then \`/ship\`." ;;
     dirty_default) _FT_TIP="Uncommitted changes — \`/review\` before committing." ;;
-    clean_default) _FT_TIP="Pick one: \`/spec\`, \`/investigate\`, or \`/qa\`." ;;
+    clean_default) _FT_TIP="Pick one: \`/office-hours\`, \`/qa\`, or \`/review\`." ;;
   esac
+  # Under off the tips are the suggestions the user turned off.
+  case "$_PROACTIVE" in off|false) _FT_TIP="" ;; esac
   if [ -n "$_FT_TIP" ]; then
     _emit_block first-run-tip <<EOI
 Show this one project-specific line as a heads-up, then CONTINUE with whatever the user actually asked — do NOT halt their task: $_FT_TIP
@@ -576,10 +581,12 @@ EOI
   fi
   touch "$_GH/.activated" 2>/dev/null || true
 elif [ "$_FIRST_LOOP_SHOWN" = "no" ]; then
-  _emit_block first-loop-tip <<EOI
-Say once as a heads-up (then continue): Tip: gstack pays off when you complete one loop — **plan → review → ship**. A common first loop: \`/office-hours\` or \`/spec\` to shape it, \`/plan-eng-review\` to lock it, then \`/ship\`.
+  if [ "$_PROACTIVE" != "off" ] && [ "$_PROACTIVE" != "false" ]; then
+    _emit_block first-loop-tip <<EOI
+Say once as a heads-up (then continue): Tip: gstack pays off when you complete one loop — **plan → review → ship**. A common first loop: \`/office-hours\` to shape it, \`/review\` to check it, then \`/ship\`.
 EOI
-  touch "$_GH/.first-loop-tip-shown" 2>/dev/null || true
+    touch "$_GH/.first-loop-tip-shown" 2>/dev/null || true
+  fi
 fi
 
 # Routing injection (interactive: AUQ + CLAUDE.md append + commit). The body

test/gstack-config-defaults.test.ts

@@ -62,7 +62,7 @@ function defaultArms(): string[] {
   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 "" ;;`
+  // e.g. `    proactive) echo "core" ;; # core | all | off ...` 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]);
   }

test/proactive-config.test.ts

@@ -59,13 +59,4 @@ describe('gstack-config proactive', () => {
     expect(junk.stderr).toContain('core, all, off');
     expect(run(['get', 'proactive']).stdout).toBe('core');
   });
-
-  test('the config header documents the three values and names the core four', () => {
-    const src = fs.readFileSync(BIN_CONFIG, 'utf-8');
-    const header = src.slice(src.indexOf('# proactive:'), src.indexOf('# routing_declined:'));
-    expect(header).toMatch(/^# proactive: core/m);
-    for (const skill of ['/office-hours', '/review', '/qa', '/ship']) expect(header).toContain(skill);
-    expect(header).toContain('all = ');
-    expect(header).toContain('off = ');
-  });
 });

test/proactive-skill-start-fallback.test.ts

@@ -1,79 +0,0 @@
-/**
- * The PROACTIVE STATUS line (GSTA-93 fix 12, proactive core default): every
- * skill reads `PROACTIVE:` from bin/gstack-skill-start, which asks
- * bin/gstack-config and falls back to a literal when the config binary is
- * missing. The default moved from `true` to `core`.
- *
- * A fallback that still says `true` means a broken install (no gstack-config
- * beside the script) auto-invokes every skill while the documented default is
- * the core four.
- *
- * Value: protects=a skill run whose bin has no gstack-config reports the same
- * proactive mode as the config default; fails_when=lookup_default moves (true
- * to core) without the skill-start fallback literal following; why_new=no test
- * ran skill-start without gstack-config beside it; seam=none (the script is
- * copied next to an empty bin, the OV4 sanitize test's pattern); no_claim=how
- * the model applies the value (prose, not executed here)
- *
- * Regression proof — fails at HEAD: yes · passes at base: yes · passes after fix: yes
- */
-import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
-import * as fs from 'fs';
-import * as os from 'os';
-import * as path from 'path';
-
-import { runBin } from './helpers/run-bin';
-
-const ROOT = path.resolve(import.meta.dir, '..');
-const BIN = path.join(ROOT, 'bin');
-
-let tmpHome: string;
-
-beforeAll(() => {
-  tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-proactive-ss-home-'));
-});
-
-afterAll(() => {
-  fs.rmSync(tmpHome, { recursive: true, force: true });
-});
-
-function freshState(config: string): string {
-  const state = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-proactive-ss-state-'));
-  fs.writeFileSync(path.join(state, 'config.yaml'), config);
-  return state;
-}
-
-function runStart(script: string, state: string): string {
-  const r = runBin(script, ['--skill', 'testskill'], {
-    cwd: tmpHome,
-    timeoutMs: 30_000,
-    env: {
-      PATH: process.env.PATH,
-      HOME: tmpHome,
-      GSTACK_HOME: state,
-      GSTACK_STATE_DIR: state,
-      GSTACK_LEASE_DIR: path.join(state, 'leases'),
-    },
-  });
-  expect(r.status, r.stderr).toBe(0);
-  return r.stdout;
-}
-
-const proactiveLine = (out: string) => out.match(/^PROACTIVE: (.*)$/m)?.[1];
-
-describe('gstack-skill-start PROACTIVE fallback', () => {
-  test('regression: with no gstack-config beside it, the fallback reports the config default, not a stale literal', () => {
-    const fakeBin = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-proactive-ss-bin-'));
-    const state = freshState('update_check: false\n');
-    try {
-      for (const f of ['gstack-skill-start', 'gstack-state-root.sh']) fs.copyFileSync(path.join(BIN, f), path.join(fakeBin, f));
-      fs.chmodSync(path.join(fakeBin, 'gstack-skill-start'), 0o755);
-      const configDefault = runBin(path.join(BIN, 'gstack-config'), ['get', 'proactive'], { gstackHome: state, cwd: ROOT, trim: true }).stdout;
-      expect(configDefault).not.toBe('');
-      expect(proactiveLine(runStart(path.join(fakeBin, 'gstack-skill-start'), state))).toBe(configDefault);
-    } finally {
-      fs.rmSync(fakeBin, { recursive: true, force: true });
-      fs.rmSync(state, { recursive: true, force: true });
-    }
-  });
-});

test/proactive-skill-start.test.ts

@@ -5,11 +5,22 @@
  * missing. The default moved from `true` to `core`.
  *
  * Value: protects=the PROACTIVE STATUS line a skill reads carries the stored
- * proactive value and core when unset; fails_when=gstack-skill-start stops
- * consulting gstack-config for proactive or the default moves back to true;
- * why_new=proactive-config.test.ts exercises gstack-config alone and
+ * proactive value, core when unset, and core for a value the router does not
+ * know; fails_when=gstack-skill-start stops consulting gstack-config for
+ * proactive, the default moves back to true, or a hand-edited junk value leaks
+ * into STATUS; why_new=proactive-config.test.ts exercises gstack-config alone and
  * gstack-skill-start.test.ts asserts only that the key is emitted; seam=none;
  * no_claim=how the model applies the value (prose, not executed here)
+ *
+ * The fallback test: a broken install (no gstack-config beside the script)
+ * must report the same mode as the config default.
+ * Value: protects=a skill run whose bin has no gstack-config reports the same
+ * proactive mode as the config default; fails_when=lookup_default moves (true
+ * to core) without the skill-start fallback literal following; why_new=no test
+ * ran skill-start without gstack-config beside it; seam=none (the script is
+ * copied next to an empty bin, the OV4 sanitize test's pattern); no_claim=how
+ * the model applies the value (prose, not executed here)
+ * Regression proof — fails at HEAD (b115142): yes · passes at base: yes · passes after fix (9e22d7f): yes
  */
 import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
 import * as fs from 'fs';
@@ -62,6 +73,7 @@ describe('gstack-skill-start PROACTIVE status line', () => {
       ['update_check: false\nproactive: all\n', 'all'],
       ['update_check: false\nproactive: off\n', 'off'],
       ['update_check: false\nproactive: true\n', 'true'],
+      ['update_check: false\nproactive: on\n', 'core'],
     ];
     for (const [config, expected] of cases) {
       const state = freshState(config);
@@ -72,4 +84,19 @@ describe('gstack-skill-start PROACTIVE status line', () => {
       }
     }
   });
+
+  test('regression: with no gstack-config beside it, the fallback reports the config default, not a stale literal', () => {
+    const fakeBin = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-proactive-ss-bin-'));
+    const state = freshState('update_check: false\n');
+    try {
+      for (const f of ['gstack-skill-start', 'gstack-state-root.sh']) fs.copyFileSync(path.join(BIN, f), path.join(fakeBin, f));
+      fs.chmodSync(path.join(fakeBin, 'gstack-skill-start'), 0o755);
+      const configDefault = runBin(path.join(BIN, 'gstack-config'), ['get', 'proactive'], { gstackHome: state, cwd: ROOT, trim: true }).stdout;
+      expect(configDefault).not.toBe('');
+      expect(proactiveLine(runStart(path.join(fakeBin, 'gstack-skill-start'), state))).toBe(configDefault);
+    } finally {
+      fs.rmSync(fakeBin, { recursive: true, force: true });
+      fs.rmSync(state, { recursive: true, force: true });
+    }
+  });
 });