From 480ebe4f261419fe504e8501c61e4f70388230d8 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 16 Aug 2026 08:38:06 -0700 Subject: [PATCH] fix(gen-skill-docs): throw when a template contains {{PREAMBLE}} twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hardens the #2508/#2362 class: a second {{PREAMBLE}} occurrence — even a prose mention, which is exactly how spec/SKILL.md.tmpl re-expanded the full ~12K-token preamble mid-document — now fails generation with the template path instead of silently shipping a doubled preamble. Pure exported guard (assertSinglePreamble) called from resolvePlaceholders, unit-tested with the original prose-mention shape. Co-Authored-By: Claude Fable 5 --- scripts/gen-skill-docs.ts | 20 ++++++++++++++++++++ test/gen-skill-docs.test.ts | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+) 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', () => {