diff --git a/CHANGELOG.md b/CHANGELOG.md index 65076b975..5d332ec70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,18 +5,18 @@ **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` in any case 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, stores the YAML yes/no family (`yes`/`on`/`1`, `no`/`none`/`0`/`disabled`/`never`) as `all`/`off`, 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, the browser step and the Failures branches name `/browse` or `/investigate` for the user to type, 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`), 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 names or the running skill invokes itself ("Skills the user names or this one invokes 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); a user who stored `off` before the one-time proactive prompt is not asked (the prompt marker is written instead). +- **`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` and the yes/no family 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` and a typed opt-out never fails open (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); a user who stored `off` before the one-time proactive prompt is not asked (the prompt marker is written before STATUS reports `PROACTIVE_PROMPTED`). - **`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. ### Added - **`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, an upper-case `OFF` stored as `off`; the warning and `core` fallback on an unknown value (negative control: `on`). +- `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 yes/no family stored as `all`/`off`; the warning and `core` fallback on an unknown value (negative control: `maybe`). - `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. diff --git a/SKILL.md b/SKILL.md index 53b8b4394..1e879554f 100644 --- a/SKILL.md +++ b/SKILL.md @@ -49,7 +49,7 @@ Host and system plan-mode restrictions and the user's current scope take precede If the user invokes a skill in plan mode, run its workflow within the host's plan-mode limits. **Treat the skill file as executable instructions, not reference.** Follow it step by step starting from Step 0; any AskUserQuestion the skill fires is the workflow operating within plan mode, not a violation of it — and a skill whose instructions resolve a question themselves (e.g. a plan-mode auto-select) may legitimately not ask it. AskUserQuestion (any variant — `mcp__*__AskUserQuestion` or native; see "AskUserQuestion Format → Tool resolution") satisfies plan mode's end-of-turn requirement. If AskUserQuestion is unavailable or a call fails, follow the AskUserQuestion Format failure fallback: `headless` → BLOCKED; `interactive` → the prose fallback (also satisfies end-of-turn). At a STOP point, stop immediately. Do not continue the workflow or call ExitPlanMode there. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" run only where the host permits them. Call ExitPlanMode only after the skill workflow completes, or if the user tells you to cancel the skill or leave plan mode. -If `PROACTIVE` is `off`: suggest no skill unasked, nor ask. `core`: only /office-hours, /review, /qa, /ship. `all`: any. Skills the user or this skill's steps name run. +If `PROACTIVE` is `off`: suggest no skills unasked, nor ask. `core`: only /office-hours, /review, /qa, /ship. `all`: any. Skills the user names or this one invokes run. If `SKILL_PREFIX` is `true` use `/gstack-*` names; paths stay `~/.claude/skills/gstack/[skill-name]/SKILL.md`. diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index 61b3492db..5f3f9bb29 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -65,7 +65,7 @@ Host and system plan-mode restrictions and the user's current scope take precede If the user invokes a skill in plan mode, run its workflow within the host's plan-mode limits. **Treat the skill file as executable instructions, not reference.** Follow it step by step starting from Step 0; any AskUserQuestion the skill fires is the workflow operating within plan mode, not a violation of it — and a skill whose instructions resolve a question themselves (e.g. a plan-mode auto-select) may legitimately not ask it. AskUserQuestion (any variant — `mcp__*__AskUserQuestion` or native; see "AskUserQuestion Format → Tool resolution") satisfies plan mode's end-of-turn requirement. If AskUserQuestion is unavailable or a call fails, follow the AskUserQuestion Format failure fallback: `headless` → BLOCKED; `interactive` → the prose fallback (also satisfies end-of-turn). At a STOP point, stop immediately. Do not continue the workflow or call ExitPlanMode there. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" run only where the host permits them. Call ExitPlanMode only after the skill workflow completes, or if the user tells you to cancel the skill or leave plan mode. -If `PROACTIVE` is `off`: suggest no skill unasked, nor ask. `core`: only /office-hours, /review, /qa, /ship. `all`: any. Skills the user or this skill's steps name run. +If `PROACTIVE` is `off`: suggest no skills unasked, nor ask. `core`: only /office-hours, /review, /qa, /ship. `all`: any. Skills the user names or this one invokes run. If `SKILL_PREFIX` is `true` use `/gstack-*` names; paths stay `~/.claude/skills/gstack/[skill-name]/SKILL.md`. diff --git a/benchmark-models/SKILL.md b/benchmark-models/SKILL.md index 565e18104..06410fc4d 100644 --- a/benchmark-models/SKILL.md +++ b/benchmark-models/SKILL.md @@ -49,7 +49,7 @@ Host and system plan-mode restrictions and the user's current scope take precede If the user invokes a skill in plan mode, run its workflow within the host's plan-mode limits. **Treat the skill file as executable instructions, not reference.** Follow it step by step starting from Step 0; any AskUserQuestion the skill fires is the workflow operating within plan mode, not a violation of it — and a skill whose instructions resolve a question themselves (e.g. a plan-mode auto-select) may legitimately not ask it. AskUserQuestion (any variant — `mcp__*__AskUserQuestion` or native; see "AskUserQuestion Format → Tool resolution") satisfies plan mode's end-of-turn requirement. If AskUserQuestion is unavailable or a call fails, follow the AskUserQuestion Format failure fallback: `headless` → BLOCKED; `interactive` → the prose fallback (also satisfies end-of-turn). At a STOP point, stop immediately. Do not continue the workflow or call ExitPlanMode there. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" run only where the host permits them. Call ExitPlanMode only after the skill workflow completes, or if the user tells you to cancel the skill or leave plan mode. -If `PROACTIVE` is `off`: suggest no skill unasked, nor ask. `core`: only /office-hours, /review, /qa, /ship. `all`: any. Skills the user or this skill's steps name run. +If `PROACTIVE` is `off`: suggest no skills unasked, nor ask. `core`: only /office-hours, /review, /qa, /ship. `all`: any. Skills the user names or this one invokes run. If `SKILL_PREFIX` is `true` use `/gstack-*` names; paths stay `~/.claude/skills/gstack/[skill-name]/SKILL.md`. diff --git a/benchmark/SKILL.md b/benchmark/SKILL.md index a542df7b6..942c30c69 100644 --- a/benchmark/SKILL.md +++ b/benchmark/SKILL.md @@ -50,7 +50,7 @@ Host and system plan-mode restrictions and the user's current scope take precede If the user invokes a skill in plan mode, run its workflow within the host's plan-mode limits. **Treat the skill file as executable instructions, not reference.** Follow it step by step starting from Step 0; any AskUserQuestion the skill fires is the workflow operating within plan mode, not a violation of it — and a skill whose instructions resolve a question themselves (e.g. a plan-mode auto-select) may legitimately not ask it. AskUserQuestion (any variant — `mcp__*__AskUserQuestion` or native; see "AskUserQuestion Format → Tool resolution") satisfies plan mode's end-of-turn requirement. If AskUserQuestion is unavailable or a call fails, follow the AskUserQuestion Format failure fallback: `headless` → BLOCKED; `interactive` → the prose fallback (also satisfies end-of-turn). At a STOP point, stop immediately. Do not continue the workflow or call ExitPlanMode there. Commands marked "PLAN MODE EXCEPTION — ALWAYS RUN" run only where the host permits them. Call ExitPlanMode only after the skill workflow completes, or if the user tells you to cancel the skill or leave plan mode. -If `PROACTIVE` is `off`: suggest no skill unasked, nor ask. `core`: only /office-hours, /review, /qa, /ship. `all`: any. Skills the user or this skill's steps name run. +If `PROACTIVE` is `off`: suggest no skills unasked, nor ask. `core`: only /office-hours, /review, /qa, /ship. `all`: any. Skills the user names or this one invokes run. If `SKILL_PREFIX` is `true` use `/gstack-*` names; paths stay `~/.claude/skills/gstack/[skill-name]/SKILL.md`. diff --git a/bin/gstack-config b/bin/gstack-config index 893785676..73570a59e 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -746,9 +746,12 @@ $_gstack_legacy_root" VALUE="default" fi if [ "$KEY" = "proactive" ]; then - # Case-insensitive, like gstack-skill-start's read of a hand-edited value. + # Case-insensitive, and the YAML yes/no family maps like gstack-skill-start + # reads a hand-edited value: an opt-out must never fail open to core. VALUE=$(printf '%s' "$VALUE" | tr '[:upper:]' '[:lower:]') case "$VALUE" in + yes|y|on|1) VALUE="all" ;; + no|n|none|0|disabled|never) VALUE="off" ;; core|all|off|true|false) ;; *) echo "Warning: proactive '$VALUE' not recognized. Valid values: core, all, off (true = all, false = off). Using core." >&2 diff --git a/bin/gstack-skill-start b/bin/gstack-skill-start index 6a33d86c5..2c3ef45ab 100755 --- a/bin/gstack-skill-start +++ b/bin/gstack-skill-start @@ -134,13 +134,25 @@ _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: drop quotes and a trailing comment, -# lowercase, read the legacy true/false as all/off, and anything else as core, -# so STATUS only ever says core | all | off. +# lowercase, read the legacy true/false and the YAML yes/no family as all/off +# (an opt-out must never fail open), and anything else as core, so STATUS only +# ever says core | all | off. _PROACTIVE=$(printf '%s' "$_PROACTIVE" | sed -e 's/[[:space:]]*#.*$//' -e "s/^[[:space:]]*[\"']*//" -e "s/[\"']*[[:space:]]*$//" | tr '[:upper:]' '[:lower:]') -case "$_PROACTIVE" in true) _PROACTIVE="all" ;; false) _PROACTIVE="off" ;; core|all|off) ;; *) _PROACTIVE="core" ;; esac -# off silences every unasked surface below: the tips and the CLAUDE.md routing offer. +case "$_PROACTIVE" in + true|yes|y|on|1) _PROACTIVE="all" ;; + false|no|n|none|0|disabled|never) _PROACTIVE="off" ;; + core|all|off) ;; + *) _PROACTIVE="core" ;; +esac +# off silences every unasked surface below: the tips, the one-time prompt and +# the CLAUDE.md routing offer. A stored off also answers the one-time prompt, +# so the marker is written here, before STATUS reports it. case "$_PROACTIVE" in off) _PROACTIVE_OFF="yes" ;; *) _PROACTIVE_OFF="no" ;; esac _PROACTIVE_PROMPTED=$([ -f "$_GH/.proactive-prompted" ] && echo "yes" || echo "no") +if [ "$_PROACTIVE_PROMPTED" = "no" ] && [ "$_PROACTIVE_OFF" = "yes" ]; then + touch "$_GH/.proactive-prompted" 2>/dev/null || true + _PROACTIVE_PROMPTED="yes" +fi # 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. _BRANCH=$(git branch --show-current 2>/dev/null | tr -cd 'a-zA-Z0-9._/-') @@ -551,12 +563,7 @@ Always run: \`touch "$_GH/.telemetry-prompted"\`. EOI fi -# Proactive-suggestions opt-in (interactive). A user who already stored off -# has answered; record that instead of asking. -if [ "$_PROACTIVE_PROMPTED" = "no" ] && [ "$_PROACTIVE_OFF" = "yes" ]; then - touch "$_GH/.proactive-prompted" 2>/dev/null || true - _PROACTIVE_PROMPTED="yes" -fi +# Proactive-suggestions opt-in (interactive; a stored off answered it above). if [ "$_PROACTIVE_PROMPTED" = "no" ] && [ "$_TEL_PROMPTED" = "yes" ]; then _emit_block proactive-prompt < { expect(run(['get', 'proactive']).stdout).toBe('off'); }); - test('an unknown value warns and stores core (negative control: on is not a value)', () => { + test('the YAML yes/no family maps to all/off on set (an opt-out never fails open)', () => { + for (const [value, stored] of [['no', 'off'], ['None', 'off'], ['0', 'off'], ['disabled', 'off'], ['yes', 'all'], ['on', 'all'], ['1', 'all']]) { + const r = run(['set', 'proactive', value]); + expect(r.status, value).toBe(0); + expect(r.stderr, value).toBe(''); + expect(run(['get', 'proactive']).stdout, value).toBe(stored); + } + }); + + test('an unknown value warns and stores core (negative control: maybe is not a value)', () => { run(['set', 'proactive', 'off']); - const junk = run(['set', 'proactive', 'on']); + const junk = run(['set', 'proactive', 'maybe']); expect(junk.status).toBe(0); - expect(junk.stderr).toContain("proactive 'on' not recognized"); + expect(junk.stderr).toContain("proactive 'maybe' not recognized"); expect(junk.stderr).toContain('core, all, off'); expect(run(['get', 'proactive']).stdout).toBe('core'); }); diff --git a/test/proactive-core-set.test.ts b/test/proactive-core-set.test.ts index 5db5db0d3..a54503e5d 100644 --- a/test/proactive-core-set.test.ts +++ b/test/proactive-core-set.test.ts @@ -54,8 +54,8 @@ describe('proactive core set: every skill preamble', () => { expect(line).toMatch(/^If `PROACTIVE` is `off`/); expect(line).toContain('`core`: only /office-hours, /review, /qa, /ship'); expect(line).toContain('`all`: any'); - expect(line).toContain('suggest no skill unasked, nor ask'); - expect(line).toContain("Skills the user or this skill's steps name run"); + expect(line).toContain('suggest no skills unasked, nor ask'); + expect(line).toContain('Skills the user names or this one invokes run'); expect(line.split('\n')[0]).not.toContain('true'); for (const skill of NON_CORE_SAMPLE) expect(line).not.toContain(skill); }); diff --git a/test/proactive-skill-start.test.ts b/test/proactive-skill-start.test.ts index 6e2a5cab5..75ada2f4e 100644 --- a/test/proactive-skill-start.test.ts +++ b/test/proactive-skill-start.test.ts @@ -70,7 +70,7 @@ function runStart(script: string, state: string, cwd = tmpHome): string { const proactiveLine = (out: string) => out.match(/^PROACTIVE: (.*)$/m)?.[1]; describe('gstack-skill-start PROACTIVE status line', () => { - test('carries core when nothing is stored, the stored value otherwise, the legacy true/false as all/off, a quoted or upper-case hand edit normalized, and anything unknown as core', () => { + test('carries core when nothing is stored, the stored value otherwise, the legacy true/false and the yes/no family as all/off, a quoted or upper-case hand edit normalized, and anything unknown as core', () => { const cases: Array<[string, string]> = [ ['update_check: false\n', 'core'], ['update_check: false\nproactive: all\n', 'all'], @@ -79,7 +79,12 @@ describe('gstack-skill-start PROACTIVE status line', () => { ['update_check: false\nproactive: false\n', 'off'], ['update_check: false\nproactive: "off"\n', 'off'], ['update_check: false\nproactive: OFF # typed by hand\n', 'off'], - ['update_check: false\nproactive: on\n', 'core'], + ['update_check: false\nproactive: no\n', 'off'], + ['update_check: false\nproactive: 0\n', 'off'], + ['update_check: false\nproactive: none\n', 'off'], + ['update_check: false\nproactive: yes\n', 'all'], + ['update_check: false\nproactive: on\n', 'all'], + ['update_check: false\nproactive: maybe\n', 'core'], ]; for (const [config, expected] of cases) { const state = freshState(config); @@ -122,6 +127,7 @@ describe('gstack-skill-start PROACTIVE status line', () => { const out = runStart(path.join(BIN, 'gstack-skill-start'), state); expect(out.includes('GSTACK_INSTRUCTION_BEGIN: proactive-prompt'), config).toBe(asked); expect(fs.existsSync(path.join(state, '.proactive-prompted')), config).toBe(!asked); + expect(out, config).toContain(`PROACTIVE_PROMPTED: ${asked ? 'no' : 'yes'}`); } finally { fs.rmSync(state, { recursive: true, force: true }); }