diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index fbff381b7..0f7ac97dd 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -668,12 +668,32 @@ function applyHostRewrites(content: string, hostConfig: HostConfig): string { * 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. */ +/** + * A second {{PREAMBLE}} in one template re-expands the entire ~12K-token + * preamble mid-document (#2508/#2362 — a PROSE mention of the macro in + * spec/SKILL.md.tmpl expanded it a second time, +43KB per /spec load). + * Resolution is context-blind, so any second occurrence — code fence, prose, + * anywhere — is a generation error, never intentional. Throw at render time + * so the mistake cannot reach a generated SKILL.md again. + */ +export function assertSinglePreamble(tmplContent: string, relTmplPath: string): void { + const count = (tmplContent.match(/\{\{PREAMBLE\}\}/g) || []).length; + if (count > 1) { + throw new Error( + `${relTmplPath} contains {{PREAMBLE}} ${count} times — a template may reference it ` + + `at most once (each occurrence expands the full preamble; see #2508/#2362). ` + + `Refer to "the preamble" in prose instead of the macro.`, + ); + } +} + function resolvePlaceholders( tmplContent: string, ctx: TemplateContext, hostConfig: HostConfig, relTmplPath: string, ): string { + assertSinglePreamble(tmplContent, relTmplPath); // effectiveSuppressedResolvers() honors --respect-detection: when gbrain is // detected locally, GBRAIN_* resolvers un-suppress. Shared by SKILL.md and // section generation so both paths get the same gbrain-aware behavior. diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index e9d440e02..1c9bf1fb4 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -1,4 +1,5 @@ import { describe, test, expect, beforeAll } from 'bun:test'; +import { assertSinglePreamble } from '../scripts/gen-skill-docs'; import { COMMAND_DESCRIPTIONS } from '../browse/src/commands'; import { SNAPSHOT_FLAGS } from '../browse/src/snapshot'; import * as fs from 'fs'; @@ -1507,6 +1508,25 @@ describe('CHANGELOG_WORKFLOW resolver', () => { }); }); +// --- Duplicate {{PREAMBLE}} guard (#2508/#2362) --- + +describe('assertSinglePreamble', () => { + test('one {{PREAMBLE}} passes', () => { + expect(() => assertSinglePreamble('a\n{{PREAMBLE}}\nb', 'x/SKILL.md.tmpl')).not.toThrow(); + }); + + test('zero {{PREAMBLE}} passes (sections have none)', () => { + expect(() => assertSinglePreamble('no macro here', 'x/sections/y.md.tmpl')).not.toThrow(); + }); + + test('a second occurrence throws with the template path — even in prose', () => { + // The original #2508 bug WAS a prose mention: "emitted by {{PREAMBLE}}'s + // preamble bash". Resolution is context-blind, so the guard must be too. + const tmpl = '{{PREAMBLE}}\n\n...later: emitted by {{PREAMBLE}}\'s preamble bash'; + expect(() => assertSinglePreamble(tmpl, 'spec/SKILL.md.tmpl')).toThrow(/spec\/SKILL\.md\.tmpl.*2 times/); + }); +}); + // --- Parameterized resolver infrastructure tests --- describe('parameterized resolver support', () => {