C8: a render given disabled skills drops them from the router (#3018)

The root router named every skill and ends 'When in doubt, invoke the skill',
so it routed to skills the user disabled. gen-skill-docs takes
--disabled-skills (runGeneration disabledSkills): the router drops their
routing rules, keeps the other side of an 'A or B' rule, and names them as
disabled. The router and gstack-upgrade cannot be disabled. With no disabled
skills the render is byte-identical.
This commit is contained in:
garrytan committed 2026-10-03 21:42:40 +00:00
1 parent 3f3655b925
commit dc7a95a4b4
2 files changed
+79

No files matched your search

+31
View File
@@ -42,6 +42,8 @@ export interface GenerationOptions {
contentLinkRoot?: string | null;
/** Install-context render contract: absolute install root this render serves. */
installRoot?: string | null;
/** Skills this install leaves unregistered (gstack-config disabled_skills); the router omits them. */
disabledSkills?: string[];
model?: Model | null;
catalogMode?: 'trim' | 'full';
explainLevel?: 'default' | 'terse';
@@ -53,6 +55,7 @@ interface RenderOptions {
outputRoot: string;
contentLinkRoot: string | null;
installRoot: string | null;
disabledSkills: string[];
model: Model | null;
catalogMode: 'trim' | 'full';
explainLevel: 'default' | 'terse';
@@ -126,6 +129,10 @@ function parseGenerationArgs(args: string[]): GenerationOptions {
}
const outDir = value('--out-dir');
const linkRoot = value('--link-root');
const disabledValue = value('--disabled-skills');
const disabledSkills = (disabledValue ?? '').split(/[\s,]+/).filter(Boolean);
const badDisabled = disabledSkills.find(n => !/^\/?[a-z0-9-]+$/.test(n));
if (badDisabled) throw new Error(`--disabled-skills takes skill names (got ${JSON.stringify(badDisabled)})`);
const installRoot = value('--install-root');
if (installRoot !== undefined && !INSTALL_ROOT_PATTERN.test(installRoot)) {
throw new Error(`--install-root must be an absolute path of letters, digits and . _ - + @ / (got ${JSON.stringify(installRoot)})`);
@@ -140,6 +147,7 @@ function parseGenerationArgs(args: string[]): GenerationOptions {
contentLinkRoot: linkRoot !== undefined ? path.resolve(linkRoot)
: outDir !== undefined ? path.resolve(outDir) : null,
installRoot: installRoot ?? null,
disabledSkills,
};
}
@@ -160,6 +168,27 @@ function rewriteInstallRoot(content: string, hostConfig: HostConfig, installRoot
return defaults.reduce((text, from) => text.split(from).join(root), content);
}
/**
* C8 (#3018): the router names every skill and ends "When in doubt, invoke the
* skill", so it routed to skills the user disabled. A per-install render that
* knows its disabled skills drops their routing rules (keeping the other side
* of an "A or B" rule) and says which skills are off. The router itself and
* the way back (gstack-upgrade) can never be disabled.
*/
function omitDisabledSkills(content: string, disabled: string[]): string {
const off = new Set(disabled.map(n => n.replace(/^\//, '')).filter(n => n !== 'gstack' && n !== 'gstack-upgrade').map(n => n.replace(/^gstack-/, '')));
if (off.size === 0) return content;
const isOff = (ref: string) => off.has(ref.replace(/^`\/(?:gstack-)?|`$/g, ''));
const routed = content.split('\n').flatMap(line => {
const m = line.match(/^(- .*→ invoke )(`\/[a-z0-9-]+`(?: or `\/[a-z0-9-]+`)*)(.*)$/);
if (!m) return [line];
const kept = m[2].split(' or ').filter(ref => !isOff(ref));
return kept.length ? [`${m[1]}${kept.join(' or ')}${m[3]}`] : [];
}).join('\n');
const names = [...off].map(n => `\`/${n}\``).join(', ');
return routed.replace('\n## Route first\n', `\n## Route first\n\nDisabled on this install (never invoke or suggest them): ${names}.\n`);
}
/** Repoint only Claude section links, retaining global bin/browse/doc paths. */
function rewriteSectionBase(content: string, linkRoot: string | null): string {
if (!linkRoot) return content;
@@ -841,6 +870,7 @@ function processTemplate(tmplPath: string, host: Host, options: RenderOptions):
// --out-dir: repoint section-base paths to the out-dir (no-op otherwise).
if (host === 'claude') content = rewriteSectionBase(content, options.contentLinkRoot);
if (skillDir === '' || skillDir === '.') content = omitDisabledSkills(content, options.disabledSkills);
content = rewriteInstallRoot(insertRuntimePreludes(rewriteCarvedSectionRefs(content, ctx), ctx), currentHostConfig, options.installRoot);
return { outputPath, content, symlinkLoop, metadata };
@@ -921,6 +951,7 @@ export async function runGeneration(settings: GenerationOptions = {}): Promise<G
outputRoot: path.resolve(settings.outputRoot ?? ROOT),
contentLinkRoot: settings.contentLinkRoot ?? null,
installRoot: settings.installRoot ?? null,
disabledSkills: settings.disabledSkills ?? [],
model: settings.model ?? null,
catalogMode: settings.catalogMode ?? 'trim',
explainLevel: settings.explainLevel ?? 'default',
+48
View File
@@ -0,0 +1,48 @@
/**
* C8 (#3018): the root router listed every skill and ends "When in doubt,
* invoke the skill", so it routed to skills the user disabled. A render given
* the install's disabled skills drops their routing rules and names them as
* off; the canonical (committed) render is unchanged.
*/
import { afterAll, describe, expect, test } from 'bun:test';
import { spawnSync } from 'child_process';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { runGeneration } from '../scripts/gen-skill-docs';
const ROOT = path.resolve(import.meta.dir, '..');
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-router-disabled-'));
afterAll(() => fs.rmSync(tmp, { recursive: true, force: true }));
async function router(host: 'claude' | 'codex', disabledSkills?: string[]) {
const out = fs.mkdtempSync(path.join(tmp, `${host}-`));
const r = await runGeneration({ host, outputRoot: out, disabledSkills });
expect(r.exitCode).toBe(0);
return fs.readFileSync(host === 'claude' ? path.join(out, 'SKILL.md') : path.join(out, '.agents/skills/gstack/SKILL.md'), 'utf8');
}
describe('C8: disabled skills leave the rendered router', () => {
for (const host of ['claude', 'codex'] as const) {
test(`${host}: routing rules drop disabled skills and keep the other side of an "A or B" rule`, async () => {
const text = await router(host, ['retro', 'gstack-cso', '/careful', 'gstack-upgrade']);
expect(text).toContain('Disabled on this install (never invoke or suggest them): `/retro`, `/cso`, `/careful`.');
expect(text).not.toMatch(/→ invoke `\/retro`/);
expect(text).not.toMatch(/→ invoke `\/cso`/);
expect(text).toContain('careful mode → invoke `/guard`');
expect(text).toContain('→ invoke `/gstack-upgrade`');
expect(text).toContain('→ invoke `/ship`');
});
test(`${host}: no disabled skills renders the canonical router byte for byte`, async () => {
expect(await router(host, [])).toBe(await router(host));
});
}
test('the CLI flag rejects anything but skill names', () => {
const r = spawnSync(process.execPath, ['scripts/gen-skill-docs.ts', '--host', 'claude', '--dry-run', '--disabled-skills', 'retro,$(touch x)'],
{ cwd: ROOT, encoding: 'utf8', timeout: 60_000 });
expect(r.status).toBe(1);
expect(r.stderr).toContain('--disabled-skills takes skill names');
});
});