diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b2a9167c..e0455811f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,9 @@ **Proactive skill suggestions default to the four core skills.** `proactive` now takes `core` (the default: only /office-hours, /review, /qa and /ship auto-invoke when a request matches), `all` (any matching skill, the old behavior) or `off`. `true` and `false` keep working as `all` and `off`. The sentiment research (GSTA-93) found auto-invocation of every skill is what makes gstack collide with other skill packs; the four core skills are where the praise is, so they keep the nudge. Garry, 2026-10-25: "do it". ### Changed -- **`bin/gstack-config`**: `proactive` defaults to `core`; `set proactive` accepts `core`, `all`, `off`, `true` and `false` and warns on anything else, storing `core`. The config header documents the three values. +- **`bin/gstack-config`**: `proactive` defaults to `core`; `set proactive` accepts `core`, `all`, `off`, `true` and `false` in any case and warns on anything else, storing `core`. The config header documents the three values. - **`SKILL.md.tmpl`** (the router): three branches, `off`, `core` and `all`; under `core` only the four core routing rules auto-invoke and a skill the user names is always invoked. The opt-out line writes `off`, and `all` and `core` have their own lines. -- **Every skill's preamble** (`scripts/resolvers/preamble/generate-upgrade-check.ts`): the steady-state rule names the four core skills under `core`, scopes the setting to unasked suggestions, carves out only the skills the user or a step of the invoked skill names ("Skills the user or this skill's steps name run", so `/ship` dispatching `/document-release` is not a suggestion, while an in-skill offer to run another skill still counts as one under `off`; the first wording, "This skill's steps still run", licensed any in-skill offer, found by /ship's red team), reads only `core`, `all` and `off` because `gstack-skill-start` now reports the legacy spellings as those, and is exactly as long as before, so the 89 size pins in `test/fixtures/skill-md-size-allowlist.json` are unchanged. +- **Every skill's preamble** (`scripts/resolvers/preamble/generate-upgrade-check.ts`): the steady-state rule names the four core skills under `core`, scopes the setting to unasked suggestions, carves out only the skills the user or a step of the invoked skill names ("Skills the user or this skill's steps name run", so `/ship` dispatching `/document-release` is not a suggestion, while an in-skill offer to run another skill still counts as one under `off`), reads only `core`, `all` and `off` because `gstack-skill-start` now reports the legacy spellings as those, and is exactly as long as before, so the 89 size pins in `test/fixtures/skill-md-size-allowlist.json` are unchanged. - **`bin/gstack-skill-start`**: the one-time prompt offers "Keep the core four (recommended)", "Suggest every matching skill" and "Turn it off", writing `core`, `all` or `off`. `scripts/question-registry.ts` lists the same three options. - **`bin/gstack-skill-start`**: the `PROACTIVE:` STATUS fallback when `gstack-config` is missing is `core`, not `true` (found by /ship's coverage audit, red at the first head); a stored value gstack does not know reads as `core`, the legacy `true`/`false` read as `all`/`off`, and a hand-edited value is unquoted, lowercased and stripped of a trailing comment first, so STATUS only ever says `core`, `all` or `off` (found by /ship's red team: `proactive: "off"` read as `core`); the first-run and first-loop tips name only core skills and stay quiet under `off`; the one-time offer to add `## Skill routing` to the project's CLAUDE.md is not made under `off` (found by /ship's red team: the routing rules are the auto-invocation the user turned off). - **`office-hours/sections/design-and-handoff.md.tmpl`**: the next-review handoff takes the recommend-only path under `off` as well as the legacy `false` (found by /ship's review); under `core` it still offers the review, because a step inside an invoked skill is part of that skill, which the router now says in so many words. @@ -16,10 +16,10 @@ - **`docs/proactive.md`**: the three values, who each is for, the legacy spellings, the commands, and the approved fallback (`off` if collisions continue). Linked from docs/skills.md. ### Tests -- `test/proactive-config.test.ts`: the default in get, list and defaults; the five accepted values stored verbatim; the warning and `core` fallback on an unknown value (negative control: `on`). -- `test/proactive-core-set.test.ts`: the router template and its render name `core` as the default and exactly the four core skills (negative control: no non-core skill in the core block, `true` is no longer "the default", no `set proactive false` left); the preamble rule carries the same four; the prompt and registry agree on `core`, `all`, `off`. -- `test/proactive-skill-start.test.ts`: the `PROACTIVE:` STATUS line carries the stored value, `core` when unset or unknown, the missing-`gstack-config` fallback equals the config default (regression test: failed at the first head), the CLAUDE.md routing offer fires under `core` and not under `off`/`false`, and the first-run and first-loop tips are shown under `core` and suppressed (marker unwritten) under `off`/`false`, all by running the script. `test/proactive-core-set.test.ts` also checks every surface (config header and default, router, preamble, prompt, docs) for the same four skills and three values, the office-hours gate, the router's in-skill carve-out, and the tips. -- Known gap, follow-up: under `core`, the `## Skill routing` section gstack offers for a project's CLAUDE.md, and the "Proactively suggest" sentences in non-core skill descriptions (full catalog mode), still route every skill; docs/proactive.md says so. +- `test/proactive-config.test.ts`: the default in get, list and defaults; the five accepted values stored verbatim, an upper-case `OFF` stored as `off`; the warning and `core` fallback on an unknown value (negative control: `on`). +- `test/proactive-core-set.test.ts`: every surface (config header and default, router template and render, preamble, one-time prompt and registry, docs, office-hours gate, tips) names the same four core skills and the same three values, with negative controls (no non-core skill in the core block, `true` no longer "the default", no `set proactive false` left). +- `test/proactive-skill-start.test.ts` runs the script: STATUS is `core` when unset or unknown, the stored value otherwise, `all`/`off` for the legacy spellings and for a quoted, upper-case or commented hand edit; the missing-`gstack-config` fallback equals the config default (regression test: failed at the first head); the CLAUDE.md routing offer and the first-run and first-loop tips fire under `core` and not under `off` (marker unwritten). +- Known gap, follow-up #3446: under `core`, the `## Skill routing` section gstack offers for a project's CLAUDE.md, and the "Proactively suggest" sentences in non-core skill descriptions (full catalog mode), still route every skill; docs/proactive.md says so. ### Gate integrity - No threshold, budget, timeout, trial count or case kind changed. Size pins unchanged (the preamble is byte-for-byte the same length). diff --git a/bin/gstack-config b/bin/gstack-config index 3479f7602..893785676 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -746,6 +746,8 @@ $_gstack_legacy_root" VALUE="default" fi if [ "$KEY" = "proactive" ]; then + # Case-insensitive, like gstack-skill-start's read of a hand-edited value. + VALUE=$(printf '%s' "$VALUE" | tr '[:upper:]' '[:lower:]') case "$VALUE" in core|all|off|true|false) ;; *) diff --git a/docs/proactive.md b/docs/proactive.md index 547db95e1..5830a85cd 100644 --- a/docs/proactive.md +++ b/docs/proactive.md @@ -39,7 +39,7 @@ already in CLAUDE.md stays; delete it yourself). Under `core` the offered section still lists every routing rule and tells the host to invoke any matching skill. If you accepted it before the `core` default, trim that section to the four core rules (or delete it and let gstack offer it again); making the -offered section follow `core` is tracked as a follow-up. +offered section follow `core` is tracked in #3446. gstack asks once, after the telemetry question, which of the three you want. "Say stop suggesting" in a session runs `gstack-config set proactive off` for you. diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index aa9b1239b..9f31be7b6 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -2280,7 +2280,6 @@ describe('preamble routing injection (bin/gstack-skill-start emission layer)', ( expect(SKILL_START_SCRIPT).toContain( 'if [ "$_HAS_ROUTING" = "no" ] && [ "$_ROUTING_DECLINED" = "false" ] && [ "$_PROACTIVE_PROMPTED" = "yes" ] && [ "$_PROACTIVE_OFF" = "no" ]; then', ); - expect(SKILL_START_SCRIPT).toContain('case "$_PROACTIVE" in off) _PROACTIVE_OFF="yes" ;; *) _PROACTIVE_OFF="no" ;; esac'); }); test('routing section content includes key routing rules', () => { diff --git a/test/proactive-config.test.ts b/test/proactive-config.test.ts index 47981f779..12788b1aa 100644 --- a/test/proactive-config.test.ts +++ b/test/proactive-config.test.ts @@ -51,6 +51,13 @@ describe('gstack-config proactive', () => { expect(run(['get', 'proactive']).stdout).toBe('false'); }); + test('an upper-case opt-out stores off, not core (set is case-insensitive like skill-start)', () => { + const r = run(['set', 'proactive', 'OFF']); + expect(r.status).toBe(0); + expect(r.stderr).toBe(''); + expect(run(['get', 'proactive']).stdout).toBe('off'); + }); + test('an unknown value warns and stores core (negative control: on is not a value)', () => { run(['set', 'proactive', 'off']); const junk = run(['set', 'proactive', 'on']); diff --git a/test/proactive-skill-start.test.ts b/test/proactive-skill-start.test.ts index 3e78dc8ff..f1b59502e 100644 --- a/test/proactive-skill-start.test.ts +++ b/test/proactive-skill-start.test.ts @@ -116,7 +116,7 @@ describe('gstack-skill-start PROACTIVE status line', () => { for (const [config, shown] of cases) { const state = freshState(config); const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-proactive-ss-repo-')); - execFileSync('git', ['init', '-q'], { cwd: repo }); + execFileSync('git', ['init', '-q'], { cwd: repo, timeout: 30_000 }); try { const first = runStart(path.join(BIN, 'gstack-skill-start'), state, repo); expect(first, config).toContain('FIRST_TASK: greenfield');