fix(modules): skill exclusions honor pinned sections and section references; own-property loadout lookups; llms.txt follows the selection; ownership rechecked before swap

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

.gstack/pregate.json

@@ -20,5 +20,5 @@
     "windows-free-tests.yml/windows-free-shard": "bun run scripts/test-free-shards.ts --windows-only --list"
   },
   "warn": [],
-  "strays": { "allow": ["modules/**"] }
+  "strays": { "allow": ["modules/loadouts.json", "modules/loadouts.schema.json"] }
 }

docs/loadouts.md

@@ -8,7 +8,7 @@ Install-time model detection, which picks a loadout for the model you run, ships
 
 | Term | Meaning |
 |---|---|
-| Module | One of four kinds. `skill:<dir>` is a whole skill. `section:<skill>/<id>` is one entry of `<skill>/sections/manifest.json`. `overlay:<name>` is one `model-overlays/<name>.md`; excluding a base overlay also drops the text that inherits it. `preamble:<id>` is one piece of the shared preamble (`PREAMBLE_PIECES` in `scripts/resolvers/preamble.ts`). |
+| Module | One of four kinds. `skill:<dir>` is a whole skill. `section:<skill>/<id>` is one entry of `<skill>/sections/manifest.json`. `overlay:<name>` is one `model-overlays/<name>.md`; excluding a base overlay also removes the base's text from every overlay that inherits it; the inheriting overlay's own text stays. `preamble:<id>` is one piece of the shared preamble (`PREAMBLE_PIECES` in `scripts/resolvers/preamble.ts`). |
 | Pinned | A module no loadout can remove where `full` renders it: the start command, plan-mode rules, the question format, completion status, the unattended profile, /ship's review army and adversarial pass, the router and `/gstack-upgrade`. A skill's `preamble-tier` still applies, so a tier-1 skill has no question-format section in any loadout. |
 | Rung | How much scaffolding a model gets: `frontier`, `strong`, `mid`, `light`, or `unknown`. Model ids are matched after lowercasing and stripping a `provider/` prefix (`anthropic/claude-haiku-5-5` is `mid`). Rungs are separate from the frontier/smart tiers in `lib/model-catalog.ts`, which choose which model runs a role. |
 | Loadout | `{ base, exclude, include, preamble_tier_max, note }`. `full` (include `*`) is always present. An unknown model gets `default.unknown`, which is `full`. |

scripts/gen-llms-txt.ts

@@ -125,6 +125,8 @@ interface GenerateOptions {
   root?: string;
   /** When true, missing skill description should fail the build. */
   strict?: boolean;
+  /** Skill directories a module loadout leaves out (scripts/modules.ts). */
+  excludedSkills?: string[];
 }
 
 export interface GenerateResult {
@@ -143,6 +145,7 @@ export async function generateLlmsTxt(opts: GenerateOptions = {}): Promise<Gener
   const skills: SkillEntry[] = [];
   for (const t of templates) {
     if (!includesSkill(claudeHost, path.dirname(t.tmpl))) continue;
+    if (opts.excludedSkills?.includes(path.dirname(t.tmpl))) continue;
     const filePath = path.join(root, t.tmpl);
     const entry = parseSkillFrontmatter(filePath);
     if (!entry) {

scripts/gen-skill-docs.ts

@@ -150,8 +150,8 @@ function parseGenerationArgs(args: string[]): GenerationOptions {
   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 modules = selectModules({ loadout: value('--loadout'), modules: value('--modules') });
-  if (modules && outDir === undefined) {
-    throw new ModuleError('MODULE_SPEC_INVALID', `--${value('--modules') === undefined ? 'loadout' : 'modules'} with exclusions needs --out-dir; the committed tree is always the full render`, 'bin/gstack-modules render --host <host> --modules <spec> --out-dir <dir>');
+  if (modules && (outDir === undefined || path.resolve(outDir) === ROOT)) {
+    throw new ModuleError('MODULE_SPEC_INVALID', `--${value('--modules') === undefined ? 'loadout' : 'modules'} with exclusions needs an --out-dir outside the checkout root; the committed tree is always the full render`, 'bin/gstack-modules render --host <host> --modules <spec> --out-dir <dir>');
   }
   const installRoot = value('--install-root');
   if (installRoot !== undefined && !INSTALL_ROOT_PATTERN.test(installRoot)) {
@@ -1151,7 +1151,7 @@ export async function runGeneration(settings: GenerationOptions = {}): Promise<G
   // Shared artifacts are awaited in both modes. A failure must reach the CLI
   // exit code, never disappear in a fire-and-forget auxiliary writer.
   try {
-    const index = await generateLlmsTxt();
+    const index = await generateLlmsTxt({ excludedSkills: excludedSkillDirs(options.modules) });
     emit(path.join(options.outputRoot, 'gstack', 'llms.txt'), index.content, 'index');
     for (const warning of index.warnings) diagnostics.push({ kind: 'warning', message: `[gen-llms-txt] ${warning}` });
   } catch (error) {
@@ -1196,6 +1196,10 @@ export async function main(args = process.argv.slice(2)): Promise<number> {
     }
     return result.exitCode;
   } catch (error) {
+    if (error instanceof ModuleError) {
+      console.error(`${error.code}: ${error.message}`);
+      return 2;
+    }
     console.error(`ERROR: ${(error as Error).message}`);
     return 1;
   }

scripts/modules-render.ts

@@ -59,7 +59,8 @@ function walk(dir: string, base = dir): string[] {
  * file, so resolver-inlined cross-skill pointers count), an installed-relative
  * pointer naming its skill, or, inside its own skill's directory, a bare
  * `sections/<file>` or the inline-host "section inlined" phrase. An excluded
- * skill counts as referenced by another skill naming its SKILL.md.
+ * skill counts as referenced by another skill naming its SKILL.md or one of
+ * its section files.
  */
 export function danglingReferences(stage: string, selection: ModuleSelection): string[] {
   const checks: Array<{ id: string; scope: string | null; test: (line: string) => boolean }> = [];
@@ -77,6 +78,15 @@ export function danglingReferences(stage: string, selection: ModuleSelection): s
     } else if (id.startsWith('skill:')) {
       const skill = id.slice('skill:'.length);
       checks.push({ id, scope: null, test: line => line.includes(`/${skill}/SKILL.md`) || line.includes(`gstack-${skill}/SKILL.md`) });
+      const manifestPath = path.join(ROOT, skill, 'sections', 'manifest.json');
+      if (fs.existsSync(manifestPath)) {
+        for (const { file } of JSON.parse(fs.readFileSync(manifestPath, 'utf-8')).sections as Array<{ file: string }>) {
+          checks.push(
+            { id, scope: null, test: line => line.includes(`/${skill}/sections/${file}`) },
+            { id, scope: null, test: line => line.includes(`sections/${file}\` relative to the installed`) && line.includes(`${skill}\``) },
+          );
+        }
+      }
     }
   }
   if (!checks.length) return [];
@@ -148,6 +158,9 @@ export async function renderLoadout(request: RenderRequest): Promise<LoadoutLock
       files,
     };
     fs.writeFileSync(path.join(stage, LOCK_FILE), JSON.stringify(lock, null, 2) + '\n');
+    if (!owned(final)) {
+      throw new ModuleError('RENDER_DEST_NOT_OWNED', `${final} changed during the render and now holds no ${LOCK_FILE}`, 'pass an empty or new --out-dir, or one an earlier gstack-modules render wrote');
+    }
     if (fs.existsSync(final)) fs.renameSync(final, retired);
     try {
       fs.renameSync(stage, final);

scripts/modules.ts

@@ -111,7 +111,7 @@ const matches = (pattern: string, id: string) => (pattern.endsWith('*') ? id.sta
 /** The rung a model id belongs to: aliases first, then the first rung whose pattern matches. */
 export function rungFor(manifest: LoadoutsManifest, raw: string): Rung {
   const id = normalizeModelId(raw);
-  if (manifest.aliases[id]) return manifest.aliases[id];
+  if (Object.hasOwn(manifest.aliases, id)) return manifest.aliases[id];
   for (const rung of RUNGS) if (manifest.rungs[rung].some(p => matches(p, id))) return rung;
   return 'unknown';
 }
@@ -128,23 +128,28 @@ export function validateLoadouts(manifest: LoadoutsManifest, catalog: ModuleEntr
   }
   for (const id of manifest.pinned ?? []) if (!ids.has(id)) problems.push(`pinned id ${clamp(id)} is not a module`);
   const loadouts = manifest.loadouts ?? {};
-  if (!loadouts[FULL] || loadouts[FULL].include !== '*') problems.push('loadouts.full must exist with include "*"');
+  const has = (name: string | undefined) => name !== undefined && Object.hasOwn(loadouts, name);
+  if (!has(FULL) || loadouts[FULL].include !== '*') problems.push('loadouts.full must exist with include "*"');
   for (const [name, def] of Object.entries(loadouts)) {
     if (!NAME_RE.test(name)) problems.push(`loadout name ${clamp(name)} must match ^[a-z0-9-]+$`);
-    if (def.base !== undefined && !loadouts[def.base]) problems.push(`loadouts.${name}.base names no loadout (${clamp(def.base)})`);
+    if (def.base !== undefined && !has(def.base)) problems.push(`loadouts.${name}.base names no loadout (${clamp(def.base)})`);
     for (const id of [...(def.exclude ?? []), ...(Array.isArray(def.include) ? def.include : [])]) {
       if (!ids.has(id)) problems.push(`loadouts.${name} names ${clamp(id)}, which is not a module`);
     }
-    for (const id of def.exclude ?? []) if (pinned.has(id)) problems.push(`loadouts.${name} excludes pinned ${id}`);
+    for (const id of def.exclude ?? []) {
+      if (pinned.has(id)) problems.push(`loadouts.${name} excludes pinned ${id}`);
+      const child = id.startsWith('skill:') ? [...pinned].find(p => p.startsWith(`section:${id.slice('skill:'.length)}/`)) : undefined;
+      if (child) problems.push(`loadouts.${name} excludes ${id}, which holds pinned ${child}`);
+    }
     if (def.preamble_tier_max !== undefined && ![1, 2, 3, 4].includes(def.preamble_tier_max)) problems.push(`loadouts.${name}.preamble_tier_max must be 1-4`);
     const seen = new Set<string>();
-    for (let at: string | undefined = name; at && loadouts[at]; at = loadouts[at].base) {
+    for (let at: string | undefined = name; at && has(at); at = loadouts[at].base) {
       if (seen.has(at)) { problems.push(`loadouts.${name} has a base cycle through ${at}`); break; }
       seen.add(at);
     }
   }
   for (const rung of [...RUNGS, 'unknown'] as const) {
-    if (!loadouts[manifest.default?.[rung]]) problems.push(`default.${rung} names no loadout (${clamp(String(manifest.default?.[rung]))})`);
+    if (!has(manifest.default?.[rung])) problems.push(`default.${rung} names no loadout (${clamp(String(manifest.default?.[rung]))})`);
   }
   for (const raw of corpus) {
     const id = normalizeModelId(raw);
@@ -188,7 +193,7 @@ export function selectModules(input: { loadout?: string; modules?: string }, roo
   assertValid(manifest, catalog);
   const [base, ...edits] = text.split(',').map(t => t.trim());
   const names = Object.keys(manifest.loadouts).sort().join(', ');
-  if (!manifest.loadouts[base]) {
+  if (!Object.hasOwn(manifest.loadouts, base)) {
     throw new ModuleError('LOADOUT_UNKNOWN', `${clamp(base)} is not a loadout (valid: ${names})`, 'gstack-modules list');
   }
   if (input.loadout !== undefined && edits.length) {
@@ -207,6 +212,10 @@ export function selectModules(input: { loadout?: string; modules?: string }, roo
     if (op === '-' && entry.pinned) {
       throw new ModuleError('MODULE_PINNED', `${id} is pinned and can never be excluded`, `drop -${id} from the spec (pinned ids: gstack-modules list --pinned)`);
     }
+    const pinnedChild = op === '-' && id.startsWith('skill:') ? catalog.find(e => e.pinned && e.id.startsWith(`section:${id.slice('skill:'.length)}/`)) : undefined;
+    if (pinnedChild) {
+      throw new ModuleError('MODULE_PINNED', `${id} holds pinned ${pinnedChild.id} and can never be excluded`, `drop -${id} from the spec (pinned ids: gstack-modules list --pinned)`);
+    }
     if (op === '-') excluded.add(id); else excluded.delete(id);
   }
   if (excluded.size === 0 && tierMax === undefined && base === FULL) return null;

scripts/resolvers/sections.ts

@@ -19,7 +19,7 @@
 
 import * as fs from 'fs';
 import * as path from 'path';
-import { moduleExcluded, type Host, type ResolverFn, type TemplateContext } from './types';
+import type { Host, ResolverFn, TemplateContext } from './types';
 
 const ROOT = path.resolve(import.meta.dir, '..', '..');
 
@@ -108,7 +108,7 @@ export const SECTION: ResolverFn = (ctx: TemplateContext, args?: string[]): stri
   const id = args?.[0];
   if (!id) throw new Error('{{SECTION:id}} requires a section id');
   const entry = findSection(ctx.skillName, id);
-  if (moduleExcluded(ctx, `section:${ctx.skillName}/${id}`)) {
+  if (ctx.modules?.excluded.has(`section:${ctx.skillName}/${id}`)) {
     return `> Not loaded in the ${ctx.modules!.name} loadout: skip ${entry.trigger}. Do not offer it, and treat it as not run.`;
   }
 
@@ -143,7 +143,7 @@ const SECTION_GUARD_RE = /\{\{IF_SECTION:([a-z0-9-]+)\}\}([\s\S]*?)\{\{\/IF_SECT
 export function applySectionGuards(content: string, ctx: TemplateContext): string {
   return content.replace(SECTION_GUARD_RE, (_match, id: string, inner: string) => {
     findSection(ctx.skillName, id);
-    return moduleExcluded(ctx, `section:${ctx.skillName}/${id}`) ? '' : inner;
+    return ctx.modules?.excluded.has(`section:${ctx.skillName}/${id}`) ? '' : inner;
   });
 }
 
@@ -167,7 +167,7 @@ export const SECTION_INDEX: ResolverFn = (ctx: TemplateContext, args?: string[])
     '|------|-------------------|',
   ];
   for (const s of manifest.sections) {
-    if (s.runner_only || moduleExcluded(ctx, `section:${skill}/${s.id}`)) continue;
+    if (s.runner_only || ctx.modules?.excluded.has(`section:${skill}/${s.id}`)) continue;
     const reference = skill === 'qa' || skill === 'qa-only' ? sectionPath(ctx, skill, s.id) : `\`sections/${s.file}\``;
     if (skill === 'review' && s.id === 'review-army') {
       lines.push(`| Select surfaces and read QA methods | Inline in [Step 4](#step-4-critical-pass-core-review); setup and probes run in Step 4.7 |`);

scripts/resolvers/types.ts

@@ -142,10 +142,6 @@ export interface ModuleFilter {
   preambleTierMax?: number;
 }
 
-export function moduleExcluded(ctx: TemplateContext, id: string): boolean {
-  return ctx.modules?.excluded.has(id) ?? false;
-}
-
 /** Resolver function signature. args is populated for parameterized placeholders like {{INVOKE_SKILL:name}}. */
 export type ResolverFn = (ctx: TemplateContext, args?: string[]) => string;
 

test/gen-skill-docs-prune-stale.test.ts

@@ -39,7 +39,7 @@ describe('gen-skill-docs stale-render prune', () => {
     const source = path.join(out, 'source');
     fs.mkdirSync(path.join(source, 'scripts'), { recursive: true });
     fs.copyFileSync(path.join(ROOT, 'scripts', 'gen-skill-docs.ts'), path.join(source, 'scripts', 'gen-skill-docs.ts'));
-    for (const file of ['discover-skills.ts', 'external-skill-names.ts', 'gen-llms-txt.ts', 'gen-agents-digest.ts', 'gen-doctor-components.ts', 'models.ts']) {
+    for (const file of ['discover-skills.ts', 'external-skill-names.ts', 'gen-llms-txt.ts', 'gen-agents-digest.ts', 'gen-doctor-components.ts', 'models.ts', 'modules.ts']) {
       fs.symlinkSync(path.join(ROOT, 'scripts', file), path.join(source, 'scripts', file), 'file');
     }
     fs.symlinkSync(path.join(ROOT, 'scripts', 'resolvers'), path.join(source, 'scripts', 'resolvers'), 'dir');

test/module-loadouts.test.ts

@@ -65,6 +65,7 @@ describe('modules/loadouts.json', () => {
     expect(rungFor(manifest, 'zai/glm-5.3')).toBe('mid');
     expect(rungFor(manifest, 'haiku')).toBe('mid');
     expect(rungFor(manifest, 'some-new-model')).toBe('unknown');
+    expect(rungFor(manifest, 'constructor')).toBe('unknown');
     for (const model of MODEL_CATALOG) expect(rungFor(manifest, model.model)).not.toBe('unknown');
   });
 
@@ -72,12 +73,17 @@ describe('modules/loadouts.json', () => {
     const bad = structuredClone(manifest);
     bad.loadouts.lean = { base: 'missing', exclude: ['preamble:bash', 'section:nope/none'] };
     bad.default.mid = 'ghost';
+    bad.default.light = 'toString';
+    bad.loadouts.noship = { base: 'constructor', exclude: ['skill:ship'] };
     bad.rungs.light.push('claude-opus-5*');
     const problems = validateLoadouts(bad, catalog, CORPUS).join('\n');
     expect(problems).toContain('loadouts.lean.base names no loadout');
     expect(problems).toContain('excludes pinned preamble:bash');
     expect(problems).toContain('"section:nope/none", which is not a module');
     expect(problems).toContain('default.mid names no loadout');
+    expect(problems).toContain('default.light names no loadout');
+    expect(problems).toContain('loadouts.noship.base names no loadout');
+    expect(problems).toContain('excludes skill:ship, which holds pinned section:ship/');
     expect(problems).toContain('matches more than one rung');
   });
 });
@@ -105,6 +111,9 @@ describe('selectModules', () => {
     expect(codeOf(() => selectModules({ modules: 'full,-skill:nope' }))).toBe('MODULE_UNKNOWN');
     expect(codeOf(() => selectModules({ modules: 'full,-preamble:bash' }))).toBe('MODULE_PINNED');
     expect(codeOf(() => selectModules({ modules: 'full,-section:ship/adversarial' }))).toBe('MODULE_PINNED');
+    expect(codeOf(() => selectModules({ modules: 'full,-skill:ship' }))).toBe('MODULE_PINNED');
+    expect(codeOf(() => selectModules({ modules: 'full,-skill:autoplan' }))).toBe('MODULE_PINNED');
+    expect(codeOf(() => selectModules({ loadout: 'constructor' }))).toBe('LOADOUT_UNKNOWN');
   });
 
   test('every error code has a troubleshooting section', () => {
@@ -144,7 +153,7 @@ describe('render filter units', () => {
     expect(preambleSections({ ...tier1, modules: filter([], 1) })).toEqual(preambleSections(tier1));
   });
 
-  test('excluding a base overlay drops the text that inherits it', () => {
+  test('excluding a base overlay drops its text from the overlay that inherits it', () => {
     const astra = ctx({ model: 'gpt-6-astra' as TemplateContext['model'] });
     const withBase = preambleSections(astra).join('\n');
     const withoutBase = preambleSections({ ...astra, modules: filter(['overlay:gpt']) }).join('\n');
@@ -175,6 +184,7 @@ describe('gstack-modules render', () => {
     expect(preamble).not.toContain('Model-Specific Behavioral Patch');
     expect(preamble).toContain('## AskUserQuestion Format');
     expect(fs.existsSync(path.join(out, 'design-shotgun'))).toBe(false);
+    expect(fs.readFileSync(path.join(out, 'gstack/llms.txt'), 'utf-8')).not.toContain('design-shotgun/SKILL.md');
     expect(fs.readFileSync(path.join(out, 'SKILL.md'), 'utf-8')).toContain('Disabled on this install (never invoke or suggest them): `/design-shotgun`');
     expect(lock).toMatchObject({ loadout: 'adhoc', spec, host: 'claude', model: 'claude' });
     expect(lock.excluded).toEqual(spec.split(',').slice(1).map(t => t.slice(1)).sort());
@@ -200,6 +210,7 @@ describe('gstack-modules render', () => {
     const unsafe = renderLoadout({ host: 'claude', selection: selectModules({ modules: 'full,-section:plan-design-review/review-sections' }), outDir: out });
     await expect(unsafe).rejects.toMatchObject({ code: 'MODULE_EXCLUSION_UNSAFE' });
     await expect(unsafe).rejects.toThrow('referenced by plan-design-review/sections/checklist.md');
+    await expect(renderLoadout({ host: 'claude', selection: selectModules({ modules: 'full,-skill:qa' }), outDir: out })).rejects.toThrow(/skill:qa: referenced by qa-only\//);
     expect(readLock(out)).toEqual(before);
     expect(fs.readdirSync(tmp).filter(n => n.includes('.stage-'))).toEqual([]);
   });
@@ -272,9 +283,11 @@ describe('bin/gstack-modules', () => {
 
   test('gen:skill-docs refuses an exclusion render into the committed tree', () => {
     const before = spawnSync('git', ['status', '--porcelain'], { cwd: ROOT, encoding: 'utf-8', timeout: 30_000 }).stdout;
-    const result = spawnSync(process.execPath, [...bunIsolationArgs(), path.join(ROOT, 'scripts/gen-skill-docs.ts'), '--host', 'claude', '--modules', 'full,-skill:design-shotgun'], { cwd: ROOT, encoding: 'utf-8', timeout: 120_000 });
-    expect(result.status).not.toBe(0);
-    expect(result.stderr + result.stdout).toContain('MODULE_SPEC_INVALID');
+    for (const extra of [[], ['--out-dir', '.']]) {
+      const result = spawnSync(process.execPath, [...bunIsolationArgs(), path.join(ROOT, 'scripts/gen-skill-docs.ts'), '--host', 'claude', '--modules', 'full,-skill:design-shotgun', ...extra], { cwd: ROOT, encoding: 'utf-8', timeout: 120_000 });
+      expect(result.status).toBe(2);
+      expect(result.stderr).toContain('MODULE_SPEC_INVALID');
+    }
     expect(spawnSync('git', ['status', '--porcelain'], { cwd: ROOT, encoding: 'utf-8', timeout: 30_000 }).stdout).toBe(before);
   });