mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-12 07:59:02 +02:00
fix(gen): preamble tiers are explicit; silent ?? 4 default becomes an error; spec stops rendering its preamble twice
Eight skills (scrape, diagram, spec, skillify, pair-agent, landing-report,
open-gstack-browser + its connect-chrome symlink) silently received the
HEAVIEST tier-4 preamble because a missing frontmatter field defaulted to 4.
Tiers are now declared in every {{PREAMBLE}} template's frontmatter and a
missing declaration throws at generation time with the template path (the 5
templates without {{PREAMBLE}} never invoke the resolver). The stale
hand-written tier-map comment (wrong in 3 of 4 rows) is gone.
Bonus bug fixed: spec/SKILL.md.tmpl mentioned {{PREAMBLE}} in prose, so the
generator inlined the ENTIRE preamble a second time — spec/SKILL.md shrinks
127,462 -> 80,924 bytes (-46,538) from de-duplication alone. skill-size-budget
gains a reasoned INTENTIONAL_SHRINKS entry (its frozen baseline had measured
the doubled-preamble bug). New tests: missing-tier throw carries the path;
every {{PREAMBLE}} template declares a tier. (Carries chunk-23 edits in the
shared test/gen-skill-docs.test.ts.)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
9124559810
commit
85388c66a5
+41
-33
@@ -103,8 +103,9 @@ const ALL_SKILLS = (() => {
|
||||
return skills;
|
||||
})();
|
||||
|
||||
const CLAUDE_SKIPPED_SKILL_DIRS = new Set(['claude']);
|
||||
const CLAUDE_GENERATED_SKILLS = ALL_SKILLS.filter(skill => !CLAUDE_SKIPPED_SKILL_DIRS.has(skill.dir));
|
||||
// hosts/claude.ts generation.skipSkills entries would filter here; the set is
|
||||
// currently empty (the /claude outside-voice template was removed).
|
||||
const CLAUDE_GENERATED_SKILLS = ALL_SKILLS;
|
||||
|
||||
describe('gen-skill-docs', () => {
|
||||
test('generated SKILL.md contains all command categories', () => {
|
||||
@@ -217,11 +218,6 @@ describe('gen-skill-docs', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('Claude outside-voice skill is not generated for Claude host', () => {
|
||||
expect(fs.existsSync(path.join(ROOT, 'claude', 'SKILL.md.tmpl'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(ROOT, 'claude', 'SKILL.md'))).toBe(false);
|
||||
});
|
||||
|
||||
test(`every Codex SKILL.md description stays within ${MAX_SKILL_DESCRIPTION_LENGTH} chars`, () => {
|
||||
const agentsDir = path.join(ROOT, '.agents', 'skills');
|
||||
if (!fs.existsSync(agentsDir)) return; // skip if not generated
|
||||
@@ -1773,20 +1769,6 @@ describe('Codex generation (--host codex)', () => {
|
||||
expect(fs.existsSync(path.join(AGENTS_DIR, 'gstack-codex'))).toBe(false);
|
||||
});
|
||||
|
||||
test('Codex output includes Claude outside-voice skill with read-only boundary', () => {
|
||||
const content = fs.readFileSync(path.join(AGENTS_DIR, 'gstack-claude', 'SKILL.md'), 'utf-8');
|
||||
expect(content).toContain('claude -p');
|
||||
expect(content).toContain('mktemp /tmp/gstack-claude-prompt-');
|
||||
expect(content).toContain('mktemp /tmp/gstack-claude-diff-');
|
||||
expect(content).not.toContain('/tmp/gstack-claude-diff-$$');
|
||||
expect(content).toContain('cat "$PROMPT_FILE" | claude -p');
|
||||
expect(content).toContain('--disable-slash-commands');
|
||||
expect(content).toContain('--tools ""');
|
||||
expect(content).toContain('--allowedTools Read,Grep,Glob');
|
||||
expect(content).toContain('--disallowedTools Bash,Edit,Write');
|
||||
expect(content).toContain('is_error');
|
||||
});
|
||||
|
||||
test('Codex review step stripped from Codex-host ship and review', () => {
|
||||
const shipContent = fs.readFileSync(path.join(AGENTS_DIR, 'gstack-ship', 'SKILL.md'), 'utf-8');
|
||||
expect(shipContent).not.toContain('codex review --base');
|
||||
@@ -2195,16 +2177,6 @@ describe('Parameterized host smoke tests', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('generates Claude outside-voice skill for external hosts', () => {
|
||||
const skillMd = path.join(hostDir, 'gstack-claude', 'SKILL.md');
|
||||
expect(fs.existsSync(skillMd)).toBe(true);
|
||||
const content = fs.readFileSync(skillMd, 'utf-8');
|
||||
expect(content).toContain('claude -p');
|
||||
expect(content).toContain('--disable-slash-commands');
|
||||
expect(content).toContain('--allowedTools Read,Grep,Glob');
|
||||
expect(content).toContain('--disallowedTools Bash,Edit,Write');
|
||||
});
|
||||
|
||||
test('--dry-run freshness check passes', () => {
|
||||
const result = Bun.spawnSync(
|
||||
['bun', 'run', 'scripts/gen-skill-docs.ts', '--host', hostConfig.name, '--dry-run'],
|
||||
@@ -2372,9 +2344,9 @@ describe('setup script validation', () => {
|
||||
expect(claudeSection).toContain('link_claude_root_skill_alias "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR"');
|
||||
});
|
||||
|
||||
test('setup supports --host auto|claude|codex|kiro|opencode', () => {
|
||||
test('setup supports --host auto|claude|codex|kiro|opencode|cursor|slate', () => {
|
||||
expect(setupContent).toContain('--host');
|
||||
expect(setupContent).toContain('claude|codex|kiro|factory|opencode|auto');
|
||||
expect(setupContent).toContain('claude|codex|kiro|factory|opencode|cursor|slate|auto');
|
||||
});
|
||||
|
||||
test('auto mode detects claude, codex, kiro, and opencode binaries', () => {
|
||||
@@ -3362,3 +3334,39 @@ describe('GSTACK REVIEW REPORT mandatory unresolved-decisions status', () => {
|
||||
expect(src).not.toContain('absorbs CODEX / CROSS-MODEL / UNRESOLVED lines if applicable');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── {{PREAMBLE}} requires an explicit preamble-tier ────────
|
||||
|
||||
describe('PREAMBLE resolution requires declared preamble-tier', () => {
|
||||
test('resolving {{PREAMBLE}} without preamble-tier throws with the template path', async () => {
|
||||
const { generatePreamble } = await import('../scripts/resolvers/preamble');
|
||||
const { HOST_PATHS } = await import('../scripts/resolvers/types');
|
||||
const ctx = {
|
||||
skillName: 'tierless-skill',
|
||||
tmplPath: 'tierless-skill/SKILL.md.tmpl',
|
||||
host: 'claude' as const,
|
||||
paths: HOST_PATHS.claude,
|
||||
// preambleTier deliberately absent — the generator must refuse to default it.
|
||||
};
|
||||
expect(() => generatePreamble(ctx)).toThrow(/tierless-skill\/SKILL\.md\.tmpl/);
|
||||
expect(() => generatePreamble(ctx)).toThrow(/preamble-tier/);
|
||||
});
|
||||
|
||||
test('every template that resolves {{PREAMBLE}} declares preamble-tier in frontmatter', () => {
|
||||
const entries = fs.readdirSync(ROOT, { withFileTypes: true });
|
||||
const offenders: string[] = [];
|
||||
const checkTmpl = (tmplPath: string) => {
|
||||
const tmpl = fs.readFileSync(tmplPath, 'utf-8');
|
||||
if (tmpl.includes('{{PREAMBLE}}') && !/^preamble-tier:\s*\d+$/m.test(tmpl)) {
|
||||
offenders.push(path.relative(ROOT, tmplPath));
|
||||
}
|
||||
};
|
||||
checkTmpl(path.join(ROOT, 'SKILL.md.tmpl'));
|
||||
for (const e of entries) {
|
||||
if (!e.isDirectory() || e.name.startsWith('.') || e.name === 'node_modules') continue;
|
||||
const tmplPath = path.join(ROOT, e.name, 'SKILL.md.tmpl');
|
||||
if (fs.existsSync(tmplPath)) checkTmpl(tmplPath);
|
||||
}
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -166,12 +166,19 @@ describe('SKILL.md size budget regression (gate, free)', () => {
|
||||
// skeleton+sections union), so exempt the skeleton from the body-strip floor.
|
||||
// EQ1: derived from the canonical CARVE_GUARDS registry — no parallel list.
|
||||
const SECTIONS_EXTRACTED = new Set<string>(CARVED_SKILLS);
|
||||
// Intentional one-off shrinks vs the frozen baseline (each needs a reason):
|
||||
// - spec: the baseline measured a template bug — prose at Phase 5 mentioned
|
||||
// {{PREAMBLE}} literally, so the generator expanded the ENTIRE preamble a
|
||||
// second time mid-sentence (~47 KB of duplication). Fixed by rewording the
|
||||
// prose; spec/SKILL.md now carries exactly one preamble (~80.9 KB, ×0.79).
|
||||
const INTENTIONAL_SHRINKS = new Set<string>(['spec']);
|
||||
|
||||
const undershoots: Array<{
|
||||
skill: string; beforeBytes: number; afterBytes: number; ratio: number;
|
||||
}> = [];
|
||||
for (const [skill, before] of Object.entries(baseline.skills)) {
|
||||
if (SECTIONS_EXTRACTED.has(skill)) continue;
|
||||
if (INTENTIONAL_SHRINKS.has(skill)) continue;
|
||||
const after = current.skills[skill];
|
||||
if (!after) continue; // skill removed since baseline — separate concern
|
||||
const ratio = after.skillMdBytes / before.skillMdBytes;
|
||||
|
||||
@@ -53,11 +53,13 @@ function discoverTier2PlusSkillMds(): Array<{ skillName: string; mdPath: string
|
||||
const mdPath = path.join(ROOT, e.name, 'SKILL.md');
|
||||
const tmplPath = path.join(ROOT, e.name, 'SKILL.md.tmpl');
|
||||
if (!fs.existsSync(mdPath) || !fs.existsSync(tmplPath)) continue;
|
||||
// Check tier via frontmatter
|
||||
// Check tier via frontmatter. Every template that resolves {{PREAMBLE}}
|
||||
// must declare preamble-tier (the generator throws otherwise), so a
|
||||
// missing declaration means the template has no preamble at all — scan it
|
||||
// anyway (the vocabulary check is content-wide and cheap).
|
||||
const tmpl = fs.readFileSync(tmplPath, 'utf-8');
|
||||
const tierMatch = tmpl.match(/preamble-tier:\s*(\d+)/);
|
||||
const tier = tierMatch ? parseInt(tierMatch[1], 10) : 4;
|
||||
if (tier < 2) continue;
|
||||
if (tierMatch && parseInt(tierMatch[1], 10) < 2) continue;
|
||||
results.push({ skillName: e.name, mdPath });
|
||||
}
|
||||
return results;
|
||||
|
||||
Reference in New Issue
Block a user