fix(gen-skill-docs): throw when a template contains {{PREAMBLE}} twice

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 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-08-16 08:38:06 -07:00
co-authored by Claude Fable 5
parent ba979dbd6f
commit 480ebe4f26
2 changed files with 40 additions and 0 deletions
+20
View File
@@ -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.
+20
View File
@@ -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', () => {