mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-06 00:44:07 +01:00
fix(proactive): a stored off answers the one-time prompt; the router names /browse and /investigate under core instead of invoking them
Red team, full repeat pass: with proactive: off stored before the prompt, skill-start still emitted the opt-in prompt; it now writes the marker and asks nothing. The router's Route-first browser step and the Failures branches invoked non-core skills under core while the core block said to answer directly; one sentence in the core block resolves it, pinned by the core-set tests. CHANGELOG and docs/proactive.md say both. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
36a64e1b0f
commit
9b391d98bf
7 files changed
+39
-8
No files matched your search
+2
-2
@@ -6,10 +6,10 @@
|
||||
|
||||
### 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.
|
||||
- **`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.
|
||||
- **`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.
|
||||
- **`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).
|
||||
- **`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).
|
||||
- **`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
|
||||
|
||||
@@ -178,7 +178,9 @@ Only run skills the user explicitly invokes. This preference persists across ses
|
||||
If `PROACTIVE` is `core` (default): **invoke the Skill tool** only for the four core
|
||||
skills when the user's request matches their rule below: `/office-hours`, `/review`,
|
||||
`/qa` and `/ship`. For every other rule, answer directly and do not suggest the skill;
|
||||
the user types it. A skill the user names is always invoked, whatever this setting says,
|
||||
the user types it. The browser step above and the **Failures** branches below follow the
|
||||
same rule: under `core`, name `/browse` or `/investigate` for the user to type instead of
|
||||
invoking it. A skill the user names is always invoked, whatever this setting says,
|
||||
and a step inside a skill that is already running (a handoff, a review gate, a subagent it
|
||||
dispatches) is part of that skill, not a suggestion.
|
||||
|
||||
|
||||
+3
-1
@@ -54,7 +54,9 @@ Only run skills the user explicitly invokes. This preference persists across ses
|
||||
If `PROACTIVE` is `core` (default): **invoke the Skill tool** only for the four core
|
||||
skills when the user's request matches their rule below: `/office-hours`, `/review`,
|
||||
`/qa` and `/ship`. For every other rule, answer directly and do not suggest the skill;
|
||||
the user types it. A skill the user names is always invoked, whatever this setting says,
|
||||
the user types it. The browser step above and the **Failures** branches below follow the
|
||||
same rule: under `core`, name `/browse` or `/investigate` for the user to type instead of
|
||||
invoking it. A skill the user names is always invoked, whatever this setting says,
|
||||
and a step inside a skill that is already running (a handoff, a review gate, a subagent it
|
||||
dispatches) is part of that skill, not a suggestion.
|
||||
|
||||
|
||||
@@ -551,7 +551,12 @@ Always run: \`touch "$_GH/.telemetry-prompted"\`.
|
||||
EOI
|
||||
fi
|
||||
|
||||
# Proactive-suggestions opt-in (interactive).
|
||||
# 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
|
||||
if [ "$_PROACTIVE_PROMPTED" = "no" ] && [ "$_TEL_PROMPTED" = "yes" ]; then
|
||||
_emit_block proactive-prompt <<EOI
|
||||
Ask once:
|
||||
|
||||
+2
-1
@@ -41,7 +41,8 @@ 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 in #3446.
|
||||
|
||||
gstack asks once, after the telemetry question, which of the three you want.
|
||||
gstack asks once, after the telemetry question, which of the three you want;
|
||||
if you stored `off` before that, it takes the answer as given and does not ask.
|
||||
"Say stop suggesting" in a session runs `gstack-config set proactive off` for you.
|
||||
|
||||
If collisions with other skill packs keep coming up under `core`, the fallback
|
||||
|
||||
@@ -15,6 +15,8 @@ import { generateUpgradeCheck } from '../scripts/resolvers/preamble/generate-upg
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const CORE = ['/office-hours', '/review', '/qa', '/ship'];
|
||||
const NON_CORE_SAMPLE = ['/investigate', '/plan-ceo-review', '/design-review', '/retro'];
|
||||
// The one place the core block may name non-core skills: the rule that says to name them for the user to type.
|
||||
const TYPE_IT_RULE = 'under `core`, name `/browse` or `/investigate` for the user to type instead of\ninvoking it.';
|
||||
|
||||
const routerTmpl = fs.readFileSync(path.join(ROOT, 'SKILL.md.tmpl'), 'utf-8');
|
||||
const routerOut = fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf-8');
|
||||
@@ -25,7 +27,8 @@ describe('proactive core set: the router', () => {
|
||||
const block = routerTmpl.slice(routerTmpl.indexOf('If `PROACTIVE` is `core` (default)'), routerTmpl.indexOf('If `PROACTIVE` is `all`'));
|
||||
expect(block.length).toBeGreaterThan(50);
|
||||
for (const skill of CORE) expect(block).toContain(`\`${skill}\``);
|
||||
for (const skill of NON_CORE_SAMPLE) expect(block).not.toContain(`\`${skill}\``);
|
||||
expect(block).toContain(TYPE_IT_RULE);
|
||||
for (const skill of NON_CORE_SAMPLE) expect(block.replace(TYPE_IT_RULE, '')).not.toContain(`\`${skill}\``);
|
||||
expect(block).toContain('A skill the user names is always invoked');
|
||||
});
|
||||
|
||||
@@ -126,7 +129,7 @@ describe('proactive core set: every surface names the same four skills and three
|
||||
const surfaces: Record<string, string> = {
|
||||
'bin/gstack-config header': configSrc.slice(configSrc.indexOf('# proactive:'), configSrc.indexOf('# routing_declined:')),
|
||||
'bin/gstack-config lookup_default': configSrc.slice(configSrc.indexOf('lookup_default()')).match(/^\s*proactive\).*$/m)?.[0] ?? '',
|
||||
'SKILL.md.tmpl core block': routerTmpl.slice(routerTmpl.indexOf('If `PROACTIVE` is `core` (default)'), routerTmpl.indexOf('If `PROACTIVE` is `all`')),
|
||||
'SKILL.md.tmpl core block': routerTmpl.slice(routerTmpl.indexOf('If `PROACTIVE` is `core` (default)'), routerTmpl.indexOf('If `PROACTIVE` is `all`')).replace(TYPE_IT_RULE, ''),
|
||||
'preamble rule': generateUpgradeCheck({ paths: { skillRoot: '~/.claude/skills/gstack' } } as never),
|
||||
'gstack-skill-start prompt': skillStart.slice(promptAt, skillStart.indexOf('.proactive-prompted"\\`', promptAt)),
|
||||
'docs/proactive.md': fs.readFileSync(path.join(ROOT, 'docs', 'proactive.md'), 'utf-8'),
|
||||
|
||||
@@ -107,6 +107,24 @@ describe('gstack-skill-start PROACTIVE status line', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('a stored off answers the one-time proactive prompt: no prompt block, marker written; core still asks once telemetry was answered', () => {
|
||||
const cases: Array<[string, boolean]> = [
|
||||
['update_check: false\ntelemetry: off\n', true],
|
||||
['update_check: false\ntelemetry: off\nproactive: off\n', false],
|
||||
];
|
||||
for (const [config, asked] of cases) {
|
||||
const state = freshState(config);
|
||||
fs.writeFileSync(path.join(state, '.telemetry-prompted'), '');
|
||||
try {
|
||||
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);
|
||||
} finally {
|
||||
fs.rmSync(state, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('the first-run and first-loop tips are shown under core and suppressed under off/false (the marker is not written either)', () => {
|
||||
const cases: Array<[string, boolean]> = [
|
||||
['update_check: false\n', true],
|
||||
|
||||
Reference in new issue
Block a user