mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-06 00:44:07 +01:00
fix(skill-start): no CLAUDE.md routing offer under proactive off
The one-time offer to append ## Skill routing to the project's CLAUDE.md fired whatever the proactive value, and its rules are the auto-invocation an off user turned off. The offer is skipped under off and the legacy false; a section already in CLAUDE.md is left alone. Under core the offered section still routes every skill, which docs/proactive.md records as the follow-up. Found by /ship's red team. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
cd861ac7f2
commit
c0caf45803
5 files changed
+36
-12
No files matched your search
+2
-2
@@ -9,7 +9,7 @@
|
||||
- **`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 ("This skill's steps still run", so a handoff or a dispatched `/document-release` inside an invoked skill is not a suggestion), 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 first-run and first-loop tips name only core skills and stay quiet under `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 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.
|
||||
|
||||
### Added
|
||||
@@ -19,7 +19,7 @@
|
||||
- `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`); the header names the four skills.
|
||||
- `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 and did not grow past the old line; 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, and the missing-`gstack-config` fallback equals the config default (regression test: failed at the first head). `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: the `## Skill routing` section gstack injects into 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.
|
||||
- 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.
|
||||
|
||||
### Gate integrity
|
||||
- No threshold, budget, timeout, trial count or case kind changed. Size pins unchanged (the preamble is byte-for-byte the same length).
|
||||
|
||||
@@ -591,8 +591,10 @@ fi
|
||||
|
||||
# Routing injection (interactive: AUQ + CLAUDE.md append + commit). The body
|
||||
# is a quoted heredoc (markdown backticks must stay literal), so the resolved
|
||||
# bin path is substituted via sed on the __BIN__ placeholder.
|
||||
if [ "$_HAS_ROUTING" = "no" ] && [ "$_ROUTING_DECLINED" = "false" ] && [ "$_PROACTIVE_PROMPTED" = "yes" ]; then
|
||||
# bin path is substituted via sed on the __BIN__ placeholder. Under off the
|
||||
# routing rules are the auto-invocation the user turned off, so no offer.
|
||||
case "$_PROACTIVE" in off|false) _ROUTING_OFFER="no" ;; *) _ROUTING_OFFER="yes" ;; esac
|
||||
if [ "$_HAS_ROUTING" = "no" ] && [ "$_ROUTING_DECLINED" = "false" ] && [ "$_PROACTIVE_PROMPTED" = "yes" ] && [ "$_ROUTING_OFFER" = "yes" ]; then
|
||||
sed "s|__BIN__|$_BIN|g" <<'EOI' | _emit_block routing-injection
|
||||
gstack works best when the project CLAUDE.md includes skill routing rules. If no CLAUDE.md exists in the project root, create it. Use AskUserQuestion:
|
||||
|
||||
|
||||
+8
-6
@@ -30,12 +30,14 @@ needed, because every skill reads `PROACTIVE` from `gstack-skill-start` at the
|
||||
start of each run and the router (`/gstack`) applies it to its routing rules. A
|
||||
value gstack does not know (a hand-edited `proactive: maybe`) reads as `core`.
|
||||
|
||||
One surface does not follow the setting yet: the `## Skill routing` section
|
||||
gstack offers to append to your project's CLAUDE.md 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 injected section follow `proactive` is
|
||||
tracked as a follow-up.
|
||||
One surface follows the setting only in part: the `## Skill routing` section
|
||||
gstack offers to append to your project's CLAUDE.md. Under `off` the offer is
|
||||
not made, because those rules are the auto-invocation you turned off (a section
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
@@ -2276,10 +2276,11 @@ describe('preamble routing injection (bin/gstack-skill-start emission layer)', (
|
||||
});
|
||||
|
||||
test('routing injection only fires when all conditions met', () => {
|
||||
// Must be: HAS_ROUTING=no AND ROUTING_DECLINED=false AND PROACTIVE_PROMPTED=yes
|
||||
// Must be: HAS_ROUTING=no AND ROUTING_DECLINED=false AND PROACTIVE_PROMPTED=yes AND the offer is not switched off by proactive off/false
|
||||
expect(SKILL_START_SCRIPT).toContain(
|
||||
'if [ "$_HAS_ROUTING" = "no" ] && [ "$_ROUTING_DECLINED" = "false" ] && [ "$_PROACTIVE_PROMPTED" = "yes" ]; then',
|
||||
'if [ "$_HAS_ROUTING" = "no" ] && [ "$_ROUTING_DECLINED" = "false" ] && [ "$_PROACTIVE_PROMPTED" = "yes" ] && [ "$_ROUTING_OFFER" = "yes" ]; then',
|
||||
);
|
||||
expect(SKILL_START_SCRIPT).toContain('case "$_PROACTIVE" in off|false) _ROUTING_OFFER="no" ;; *) _ROUTING_OFFER="yes" ;; esac');
|
||||
});
|
||||
|
||||
test('routing section content includes key routing rules', () => {
|
||||
|
||||
@@ -85,6 +85,25 @@ describe('gstack-skill-start PROACTIVE status line', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('the CLAUDE.md routing offer fires under core and not under off (the routing rules are the auto-invocation off turns off)', () => {
|
||||
const cases: Array<[string, boolean]> = [
|
||||
['update_check: false\n', true],
|
||||
['update_check: false\nproactive: off\n', false],
|
||||
['update_check: false\nproactive: false\n', false],
|
||||
];
|
||||
for (const [config, offered] of cases) {
|
||||
const state = freshState(config);
|
||||
fs.writeFileSync(path.join(state, '.proactive-prompted'), '');
|
||||
try {
|
||||
const out = runStart(path.join(BIN, 'gstack-skill-start'), state);
|
||||
expect(out, config).toContain('HAS_ROUTING: no');
|
||||
expect(out.includes('GSTACK_INSTRUCTION_BEGIN: routing-injection '), config).toBe(offered);
|
||||
} finally {
|
||||
fs.rmSync(state, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
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');
|
||||
|
||||
Reference in new issue
Block a user