From dc7a95a4b482a1dc99ca3f4e21699bdb4385b234 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:42:40 +0000 Subject: [PATCH] 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. --- scripts/gen-skill-docs.ts | 31 +++++++++++++++++++ test/router-disabled-skills.test.ts | 48 +++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 test/router-disabled-skills.test.ts diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index 6a9fa3c4c..68fe320a7 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -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 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'); + }); +});