From 8f229c937ac7e1762c547022391bdabf289759ff Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 29 May 2026 21:21:21 -0700 Subject: [PATCH] feat(pipeline): section discovery + generation machinery (T9) - discover-skills.ts: discoverSectionTemplates() scans /sections/*.md.tmpl - gen-skill-docs.ts: extract resolvePlaceholders + applyHostRewrites + buildContext as shared helpers (processTemplate and the new processSectionTemplate both call them, so a sanitization/rewrite fix can't miss sections) [C1] - processSectionTemplate: body-fragment generation (no frontmatter/catalog/voice), parent-skill TemplateContext (skillName pinned to parent, not 'sections', so appliesTo gating + tier behave identically), per-host output routing - --host all now fails the build on ANY host failure, not just claude, so a stale external-host output can't slip the freshness gate [Codex outside-voice #9] Inert until a skill is carved (no sections/ dirs exist yet). Refactor is output-neutral: gen:skill-docs --dry-run --host all reports 0 STALE. 5 discovery unit tests + 389 gen-skill-docs tests green. Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/discover-skills.ts | 28 +++ scripts/gen-skill-docs.ts | 237 ++++++++++++++++++------ test/discover-section-templates.test.ts | 57 ++++++ 3 files changed, 268 insertions(+), 54 deletions(-) create mode 100644 test/discover-section-templates.test.ts diff --git a/scripts/discover-skills.ts b/scripts/discover-skills.ts index 67d9a3b6c..07e826935 100644 --- a/scripts/discover-skills.ts +++ b/scripts/discover-skills.ts @@ -26,6 +26,34 @@ export function discoverTemplates(root: string): Array<{ tmpl: string; output: s return results; } +/** + * Discover on-demand section templates: `/sections/*.md.tmpl`. + * + * Returns the relative tmpl path, its generated output path (`.tmpl` stripped), + * and the owning skill directory so the generator can build a TemplateContext + * with the PARENT skill's name (not "sections") — see processSectionTemplate. + * + * Scans one level of subdirs (same depth as discoverTemplates), looking only + * inside a `sections/` child. Skills without a sections/ dir contribute nothing, + * so this is a no-op for every skill that hasn't been carved. + */ +export function discoverSectionTemplates( + root: string, +): Array<{ tmpl: string; output: string; skillDir: string }> { + const results: Array<{ tmpl: string; output: string; skillDir: string }> = []; + for (const dir of subdirs(root)) { + const sectionsDir = path.join(root, dir, 'sections'); + if (!fs.existsSync(sectionsDir) || !fs.statSync(sectionsDir).isDirectory()) continue; + for (const entry of fs.readdirSync(sectionsDir, { withFileTypes: true })) { + if (!entry.isFile() || !entry.name.endsWith('.md.tmpl')) continue; + const rel = `${dir}/sections/${entry.name}`; + results.push({ tmpl: rel, output: rel.replace(/\.tmpl$/, ''), skillDir: dir }); + } + } + // Deterministic order so CI freshness checks don't flap on FS iteration order. + return results.sort((a, b) => a.tmpl.localeCompare(b.tmpl)); +} + export function discoverSkillFiles(root: string): string[] { const dirs = ['', ...subdirs(root)]; const results: string[] = []; diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index 30853f677..52333181d 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -11,7 +11,7 @@ import { COMMAND_DESCRIPTIONS } from '../browse/src/commands'; import { SNAPSHOT_FLAGS } from '../browse/src/snapshot'; -import { discoverTemplates } from './discover-skills'; +import { discoverTemplates, discoverSectionTemplates } from './discover-skills'; import { writeLlmsTxt } from './gen-llms-txt'; import * as fs from 'fs'; import * as path from 'path'; @@ -531,6 +531,85 @@ function extractHookSafetyProse(tmplContent: string): string | null { const GENERATED_HEADER = `\n\n`; +/** + * Apply a host's configured path + tool rewrites. Extracted so both SKILL.md + * (via processExternalHost) and section files (via processSectionTemplate) get + * identical per-host treatment — a section's cross-references must rewrite the + * same way the parent skill's do, or external hosts get wrong paths. + */ +function applyHostRewrites(content: string, hostConfig: HostConfig): string { + let result = content; + for (const rewrite of hostConfig.pathRewrites) { + result = result.replaceAll(rewrite.from, rewrite.to); + } + if (hostConfig.toolRewrites) { + for (const [from, to] of Object.entries(hostConfig.toolRewrites)) { + result = result.replaceAll(from, to); + } + } + return result; +} + +/** + * Resolve {{PLACEHOLDER}} / {{NAME:arg}} tokens against the RESOLVERS registry, + * honoring host suppression and appliesTo gating, then assert nothing is left + * unresolved. Extracted so SKILL.md and section templates resolve through the + * exact same path — a security/sanitization fix to one can't miss the other. + */ +function resolvePlaceholders( + tmplContent: string, + ctx: TemplateContext, + hostConfig: HostConfig, + relTmplPath: string, +): string { + const suppressed = new Set(hostConfig.suppressedResolvers || []); + const content = tmplContent.replace(/\{\{(\w+(?::[^}]+)?)\}\}/g, (_match, fullKey) => { + const parts = fullKey.split(':'); + const resolverName = parts[0]; + const args = parts.slice(1); + if (suppressed.has(resolverName)) return ''; + const entry = RESOLVERS[resolverName]; + if (!entry) throw new Error(`Unknown placeholder {{${resolverName}}} in ${relTmplPath}`); + const { resolve, appliesTo } = unwrapResolver(entry); + if (appliesTo && !appliesTo(ctx)) return ''; + return args.length > 0 ? resolve(ctx, args) : resolve(ctx); + }); + const remaining = content.match(/\{\{(\w+(?::[^}]+)?)\}\}/g); + if (remaining) { + throw new Error(`Unresolved placeholders in ${relTmplPath}: ${remaining.join(', ')}`); + } + return content; +} + +/** + * Build the TemplateContext from a template's frontmatter. Shared by SKILL.md + * and section generation so sections inherit the SAME context the parent skill + * resolves with (skillName, tier, benefitsFrom, interactive) — enforced by + * test/template-context-parity.test.ts. skillNameOverride lets section + * generation pin the parent skill's name instead of deriving "sections". + */ +function buildContext( + tmplContent: string, + tmplPath: string, + host: Host, + skillNameOverride?: string, +): TemplateContext { + const { name: extractedName } = extractNameAndDescription(tmplContent); + const skillName = skillNameOverride || extractedName || path.basename(path.dirname(tmplPath)); + const benefitsMatch = tmplContent.match(/^benefits-from:\s*\[([^\]]*)\]/m); + const benefitsFrom = benefitsMatch + ? benefitsMatch[1].split(',').map(s => s.trim()).filter(Boolean) + : undefined; + const tierMatch = tmplContent.match(/^preamble-tier:\s*(\d+)$/m); + const preambleTier = tierMatch ? parseInt(tierMatch[1], 10) : undefined; + const interactiveMatch = tmplContent.match(/^interactive:\s*(true|false)\s*$/m); + const interactive = interactiveMatch ? interactiveMatch[1] === 'true' : undefined; + return { + skillName, tmplPath, benefitsFrom, host, paths: HOST_PATHS[host], + preambleTier, model: MODEL_ARG_VAL, interactive, explainLevel: EXPLAIN_LEVEL, + }; +} + /** * Process external host output: routing, frontmatter, path rewrites, metadata. * Shared between Codex and Factory (and future external hosts). @@ -576,17 +655,9 @@ function processExternalHost( result = result.slice(0, bodyStart) + '\n' + safetyProse + '\n' + result.slice(bodyStart); } - // Config-driven path rewrites (order matters, replaceAll) - for (const rewrite of hostConfig.pathRewrites) { - result = result.replaceAll(rewrite.from, rewrite.to); - } - - // Config-driven tool rewrites - if (hostConfig.toolRewrites) { - for (const [from, to] of Object.entries(hostConfig.toolRewrites)) { - result = result.replaceAll(from, to); - } - } + // Config-driven path + tool rewrites (shared with processSectionTemplate so + // section cross-references get the same per-host treatment as SKILL.md). + result = applyHostRewrites(result, hostConfig); // Config-driven: generate metadata (e.g., openai.yaml for Codex) if (hostConfig.generation.generateMetadata && !symlinkLoop) { @@ -607,50 +678,18 @@ function processTemplate(tmplPath: string, host: Host = 'claude'): { outputPath: // Determine skill directory relative to ROOT const skillDir = path.relative(ROOT, path.dirname(tmplPath)); - // Extract skill name from frontmatter early — needed for both TemplateContext and external host output paths. - // When frontmatter name: differs from directory name (e.g., run-tests/ with name: test), - // the frontmatter name is used for external skill naming and setup script symlinks. + // Extract name/description: name drives external skill naming + setup symlinks + // (and TemplateContext.skillName via buildContext); description feeds external + // host metadata. When frontmatter name: differs from directory name (e.g. + // run-tests/ with name: test), the frontmatter name wins. const { name: extractedName, description: extractedDescription } = extractNameAndDescription(tmplContent); - const skillName = extractedName || path.basename(path.dirname(tmplPath)); - - // Extract benefits-from list from frontmatter (inline YAML: benefits-from: [a, b]) - const benefitsMatch = tmplContent.match(/^benefits-from:\s*\[([^\]]*)\]/m); - const benefitsFrom = benefitsMatch - ? benefitsMatch[1].split(',').map(s => s.trim()).filter(Boolean) - : undefined; - - // Extract preamble-tier from frontmatter (1-4, controls which preamble sections are included) - const tierMatch = tmplContent.match(/^preamble-tier:\s*(\d+)$/m); - const preambleTier = tierMatch ? parseInt(tierMatch[1], 10) : undefined; - - // Extract interactive flag from frontmatter (generator-only; controls plan-mode handshake inclusion) - const interactiveMatch = tmplContent.match(/^interactive:\s*(true|false)\s*$/m); - const interactive = interactiveMatch ? interactiveMatch[1] === 'true' : undefined; - - const ctx: TemplateContext = { skillName, tmplPath, benefitsFrom, host, paths: HOST_PATHS[host], preambleTier, model: MODEL_ARG_VAL, interactive, explainLevel: EXPLAIN_LEVEL }; - - // Replace placeholders (supports parameterized: {{NAME:arg1:arg2}}) - // Config-driven: suppressedResolvers return empty string for this host const currentHostConfig = getHostConfig(host); - const suppressed = new Set(currentHostConfig.suppressedResolvers || []); - let content = tmplContent.replace(/\{\{(\w+(?::[^}]+)?)\}\}/g, (match, fullKey) => { - const parts = fullKey.split(':'); - const resolverName = parts[0]; - const args = parts.slice(1); - if (suppressed.has(resolverName)) return ''; - const entry = RESOLVERS[resolverName]; - if (!entry) throw new Error(`Unknown placeholder {{${resolverName}}} in ${relTmplPath}`); - const { resolve, appliesTo } = unwrapResolver(entry); - if (appliesTo && !appliesTo(ctx)) return ''; - return args.length > 0 ? resolve(ctx, args) : resolve(ctx); - }); + const ctx = buildContext(tmplContent, tmplPath, host); + const skillName = ctx.skillName; - // Check for any remaining unresolved placeholders - const remaining = content.match(/\{\{(\w+(?::[^}]+)?)\}\}/g); - if (remaining) { - throw new Error(`Unresolved placeholders in ${relTmplPath}: ${remaining.join(', ')}`); - } + // Replace placeholders + assert none remain (shared path with section generation). + let content = resolvePlaceholders(tmplContent, ctx, currentHostConfig, relTmplPath); // Preprocess voice triggers: fold into description, strip field from frontmatter. // Must run BEFORE transformFrontmatter so all hosts see the updated description, @@ -696,6 +735,58 @@ function processTemplate(tmplPath: string, host: Host = 'claude'): { outputPath: return { outputPath, content, symlinkLoop, catalogParts }; } +/** + * Generate one on-demand section file (`/sections/.md.tmpl` → + * `.md`). Sections are BODY FRAGMENTS — no frontmatter, no catalog trim, + * no voice triggers. They resolve placeholders through the SAME path as + * SKILL.md (resolvePlaceholders) using the PARENT skill's TemplateContext + * (so appliesTo gating + tier behave identically — a section's {{PREAMBLE}}- + * style resolver renders the same content it would in the parent, not empty). + * + * Output routing mirrors SKILL.md: Claude writes in-tree at + * `/sections/.md`; external hosts write to + * `/skills//sections/.md`. External hosts get + * applyHostRewrites so cross-references resolve per host. + */ +function processSectionTemplate( + sectionTmplPath: string, + skillDir: string, + host: Host = 'claude', +): { outputPath: string; content: string } { + const tmplContent = fs.readFileSync(sectionTmplPath, 'utf-8'); + const relTmplPath = path.relative(ROOT, sectionTmplPath); + const hostConfig = getHostConfig(host); + + // Read the owning SKILL.md.tmpl so the section inherits the parent's name + + // tier + benefits-from (TemplateContext parity). Fall back to the dir name. + const parentTmplPath = path.join(ROOT, skillDir, 'SKILL.md.tmpl'); + const parentContent = fs.existsSync(parentTmplPath) ? fs.readFileSync(parentTmplPath, 'utf-8') : ''; + const parentName = (parentContent && extractNameAndDescription(parentContent).name) || skillDir; + const ctx = buildContext(parentContent || tmplContent, parentTmplPath, host, parentName); + + // Resolve placeholders against the section body (shared guard catches stragglers). + let content = resolvePlaceholders(tmplContent, ctx, hostConfig, relTmplPath); + + // External hosts: rewrite cross-reference paths/tools (no frontmatter to transform). + if (host !== 'claude') { + content = applyHostRewrites(content, hostConfig); + } + + // Plain generated header (no frontmatter to insert after). + content = GENERATED_HEADER.replace('{{SOURCE}}', path.basename(sectionTmplPath)) + content; + + const fileName = path.basename(sectionTmplPath).replace(/\.tmpl$/, ''); + let outputPath: string; + if (host === 'claude') { + outputPath = path.join(ROOT, skillDir, 'sections', fileName); + } else { + const externalName = externalSkillName(skillDir, parentName); + outputPath = path.join(ROOT, hostConfig.hostSubdir, 'skills', externalName, 'sections', fileName); + } + if (!DRY_RUN) fs.mkdirSync(path.dirname(outputPath), { recursive: true }); + return { outputPath, content }; +} + // ─── Main ─────────────────────────────────────────────────── function findTemplates(): string[] { @@ -787,6 +878,40 @@ for (const currentHost of hostsToRun) { } } + // ─── Section generation (v2 plan T9) ─────────────────────── + // On-demand sections/*.md for carved skills. No-op for any skill without a + // sections/ dir, so this is inert until a skill is carved. Mirrors the + // SKILL.md include/skip host filters (keyed on the owning skill dir) and the + // DRY_RUN freshness handling, so sections participate in the freshness gate. + for (const sec of discoverSectionTemplates(ROOT)) { + if (currentHostConfig.generation.includeSkills?.length && + !currentHostConfig.generation.includeSkills.includes(sec.skillDir)) continue; + if (currentHostConfig.generation.skipSkills?.length && + currentHostConfig.generation.skipSkills.includes(sec.skillDir)) continue; + + const { outputPath, content } = processSectionTemplate(path.join(ROOT, sec.tmpl), sec.skillDir, currentHost); + const relOutput = path.relative(ROOT, outputPath); + + if (DRY_RUN) { + const existing = fs.existsSync(outputPath) ? fs.readFileSync(outputPath, 'utf-8') : ''; + if (existing !== content) { + console.log(`STALE: ${relOutput}`); + hasChanges = true; + } else { + console.log(`FRESH: ${relOutput}`); + } + } else { + fs.writeFileSync(outputPath, content); + console.log(`GENERATED: ${relOutput}`); + } + + tokenBudget.push({ + skill: relOutput, + lines: content.split('\n').length, + tokens: Math.round(content.length / 4), + }); + } + // Generate gstack-lite and gstack-full for OpenClaw host if (currentHost === 'openclaw' && !DRY_RUN) { const openclawDir = path.join(ROOT, 'openclaw'); @@ -913,10 +1038,14 @@ The orchestrator will persist the plan link to its own memory/knowledge store. } } -// --host all: report failures. Only exit(1) if claude failed. +// --host all: any host failure fails the build. Previously only claude failures +// exited nonzero, which let a stale or broken external-host output (e.g. a +// section that failed to generate for Factory) slip through the freshness gate +// silently. With sections fanned out across every host, "all hosts regenerated +// in the same commit" is only a real gate if every host failure is fatal here. if (failures.length > 0 && HOST_ARG_VAL === 'all') { console.error(`\n${failures.length} host(s) failed: ${failures.map(f => f.host).join(', ')}`); - if (failures.some(f => f.host === 'claude')) process.exit(1); + process.exit(1); } // Single host dry-run failure already handled above diff --git a/test/discover-section-templates.test.ts b/test/discover-section-templates.test.ts new file mode 100644 index 000000000..47e32cf6b --- /dev/null +++ b/test/discover-section-templates.test.ts @@ -0,0 +1,57 @@ +/** + * Unit coverage for discoverSectionTemplates — the section-discovery half of the + * v2 plan T9 pipeline. Drives it against a temp fixture tree so it doesn't + * depend on which skills have been carved in the real repo. + */ + +import { describe, test, expect, afterAll } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { discoverSectionTemplates } from '../scripts/discover-skills'; + +const root = fs.mkdtempSync(path.join(os.tmpdir(), 'sections-disc-')); +afterAll(() => { try { fs.rmSync(root, { recursive: true, force: true }); } catch { /* noop */ } }); + +// ship/ has two section templates + a non-template file; review/ has none; +// hidden + node_modules dirs must be skipped by the shared subdirs() filter. +fs.mkdirSync(path.join(root, 'ship', 'sections'), { recursive: true }); +fs.writeFileSync(path.join(root, 'ship', 'SKILL.md.tmpl'), '---\nname: ship\n---\nbody'); +fs.writeFileSync(path.join(root, 'ship', 'sections', 'version-bump.md.tmpl'), 'bump'); +fs.writeFileSync(path.join(root, 'ship', 'sections', 'changelog.md.tmpl'), 'changelog'); +fs.writeFileSync(path.join(root, 'ship', 'sections', 'manifest.json'), '{}'); // not a .md.tmpl +fs.mkdirSync(path.join(root, 'review'), { recursive: true }); +fs.writeFileSync(path.join(root, 'review', 'SKILL.md.tmpl'), '---\nname: review\n---\nbody'); +fs.mkdirSync(path.join(root, 'node_modules', 'sections'), { recursive: true }); +fs.writeFileSync(path.join(root, 'node_modules', 'sections', 'x.md.tmpl'), 'nope'); + +describe('discoverSectionTemplates', () => { + const found = discoverSectionTemplates(root); + + test('finds only *.md.tmpl files inside /sections/', () => { + expect(found.map(f => f.tmpl)).toEqual([ + 'ship/sections/changelog.md.tmpl', + 'ship/sections/version-bump.md.tmpl', + ]); + }); + + test('strips .tmpl for the output path and records the owning skill dir', () => { + const bump = found.find(f => f.tmpl.endsWith('version-bump.md.tmpl'))!; + expect(bump.output).toBe('ship/sections/version-bump.md'); + expect(bump.skillDir).toBe('ship'); + }); + + test('ignores non-template files (manifest.json) and skipped dirs (node_modules)', () => { + expect(found.some(f => f.tmpl.includes('manifest.json'))).toBe(false); + expect(found.some(f => f.tmpl.includes('node_modules'))).toBe(false); + }); + + test('returns deterministic (sorted) order', () => { + const tmpls = found.map(f => f.tmpl); + expect([...tmpls].sort()).toEqual(tmpls); + }); + + test('skills without a sections/ dir contribute nothing', () => { + expect(found.some(f => f.skillDir === 'review')).toBe(false); + }); +});