From bc5b08e9ac98e6776179467766c8536f829c9d4d Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 21:04:52 +0000 Subject: [PATCH] C4: carve ship and plan-ceo-review on every host; enforce the 160 KB byte gate The generated ship skill was 202-241 KB on all 10 non-Claude hosts (and Factory plan-ceo-review 160,039 bytes), over the 160,000-byte ceiling, because section carving ran only for Claude. Both skills now carve on every host, pointing at sections relative to the installed skill directory (literal $GSTACK_ROOT//sections refs are rewritten the same way). The ceiling is measured in UTF-8 bytes and fails generation in repo mode; under --install-root it warns so setup never leaves a user without skills. qa-lazy-sections.test.ts pinned ship as inline-on-external-hosts; migrated to the carved policy and kept the inline check on review (Claude-only carve). --- scripts/gen-skill-docs.ts | 15 ++-- scripts/resolvers/sections.ts | 22 +++++- test/external-host-carve.test.ts | 119 +++++++++++++++++++++++++++++-- test/qa-lazy-sections.test.ts | 25 ++++--- 4 files changed, 157 insertions(+), 24 deletions(-) diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index 772ddab2d..57e0e82ed 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -21,7 +21,7 @@ import * as path from 'path'; import type { Host, TemplateContext } from './resolvers/types'; import { HOST_PATHS } from './resolvers/types'; import { RESOLVERS } from './resolvers/index'; -import { usesLazySections } from './resolvers/sections'; +import { rewriteCarvedSectionRefs, SKILL_BYTE_CEILING, usesLazySections } from './resolvers/sections'; import { insertRuntimePreludes } from './resolvers/runtime-root'; import { ALL_HOST_NAMES, resolveHostArg, getHostConfig } from '../hosts/index'; import type { HostConfig } from './host-config'; @@ -840,7 +840,7 @@ function processTemplate(tmplPath: string, host: Host, options: RenderOptions): // --out-dir: repoint section-base paths to the out-dir (no-op otherwise). if (host === 'claude') content = rewriteSectionBase(content, options.contentLinkRoot); - content = rewriteInstallRoot(insertRuntimePreludes(content, ctx), currentHostConfig, options.installRoot); + content = rewriteInstallRoot(insertRuntimePreludes(rewriteCarvedSectionRefs(content, ctx), ctx), currentHostConfig, options.installRoot); return { outputPath, content, symlinkLoop, metadata }; } @@ -886,7 +886,7 @@ function processSectionTemplate( // repoint those to the out-dir too (no-op when --out-dir is unset). content = rewriteSectionBase(content, options.contentLinkRoot); } - content = rewriteInstallRoot(insertRuntimePreludes(content, ctx), hostConfig, options.installRoot); + content = rewriteInstallRoot(insertRuntimePreludes(rewriteCarvedSectionRefs(content, ctx), ctx), hostConfig, options.installRoot); // Plain generated header (no frontmatter to insert after). content = GENERATED_HEADER.replace('{{SOURCE}}', path.basename(sectionTmplPath)) + content; @@ -1005,10 +1005,11 @@ export async function runGeneration(settings: GenerationOptions = {}): Promise TOKEN_CEILING_BYTES) { - const message = `⚠️ TOKEN CEILING: ${relativePath} is ${result.content.length} bytes (~${Math.round(result.content.length / 4)} tokens), exceeds ${TOKEN_CEILING_BYTES} byte ceiling (~40K tokens)`; - diagnostics.push({ kind: 'warning', host, relativePath, message }); + const bytes = Buffer.byteLength(result.content, 'utf8'); + if (bytes > SKILL_BYTE_CEILING) { + // Setup renders per install (--install-root): an oversized skill must never leave a user with none. + const message = `${host}/${path.basename(path.dirname(result.outputPath))}/SKILL.md is ${bytes} bytes, over the ${SKILL_BYTE_CEILING.toLocaleString('en-US')}-byte limit. Fix: carve sections with usesLazySections() for this skill.`; + diagnostics.push({ kind: options.installRoot ? 'warning' : 'error', host, relativePath, message }); } } diff --git a/scripts/resolvers/sections.ts b/scripts/resolvers/sections.ts index 2c5e9c6b2..4db6a4cd9 100644 --- a/scripts/resolvers/sections.ts +++ b/scripts/resolvers/sections.ts @@ -25,8 +25,28 @@ const ROOT = path.resolve(import.meta.dir, '..', '..'); export const QA_ASSET_BLOCKER = 'If missing or unreadable, report a QA setup blocker and its affected probes as blocked; continue other safe probes (independent functional/static checks). Missing/unreadable assets block required QA.'; +/** Skills carved on every host: QA (portable assets) and those whose external + * render would exceed SKILL_BYTE_CEILING inlined (C4, #2777). */ +const CARVED_ON_EVERY_HOST = ['qa', 'qa-only', 'ship', 'plan-ceo-review']; + +/** Hosts read at most this many UTF-8 bytes of one SKILL.md (~40K tokens). */ +export const SKILL_BYTE_CEILING = 160_000; + export function usesLazySections(host: Host, skill: string): boolean { - return host === 'claude' || skill === 'qa' || skill === 'qa-only'; + return host === 'claude' || CARVED_ON_EVERY_HOST.includes(skill); +} + +/** + * External hosts: a template's literal `~/.claude/skills/gstack//sections/` + * reference arrives here as `$GSTACK_ROOT//sections/`, which external + * runtime roots do not have. For a carved skill, point it at the installed copy. + */ +export function rewriteCarvedSectionRefs(content: string, ctx: TemplateContext): string { + if (ctx.host === 'claude') return content; + return content.replace(/`\$GSTACK_ROOT\/([a-z0-9-]+)\/sections\/([A-Za-z0-9._-]+)`/g, (match, skill: string, file: string) => + usesLazySections(ctx.host, skill) && fs.existsSync(path.join(ROOT, skill, 'sections', `${file}.tmpl`)) + ? `\`sections/${file}\` relative to the installed \`gstack-${skill}\` SKILL.md directory` + : match); } interface SectionEntry { diff --git a/test/external-host-carve.test.ts b/test/external-host-carve.test.ts index a21b98df6..232d5f2f3 100644 --- a/test/external-host-carve.test.ts +++ b/test/external-host-carve.test.ts @@ -1,18 +1,56 @@ /** - * C4: carved skills on external hosts point at sections relative to the - * installed skill directory. `$GSTACK_ROOT//sections/...` does not exist - * in external runtime roots (Codex's holds bin, lib, browse, review only), so a - * root-relative pointer would 404 and the model would work from memory. + * C4 (#2777): ship and plan-ceo-review are carved on every host, and no + * generated SKILL.md exceeds 160,000 UTF-8 bytes on any host or model overlay + * setup can select. Carved sections on external hosts are pointed at relative + * to the installed skill directory: `$GSTACK_ROOT//sections/...` does + * not exist in external runtime roots (Codex's holds bin, lib, browse, review). */ -import { describe, expect, test } from 'bun:test'; +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; import { ALL_HOST_CONFIGS } from '../hosts'; -import { sectionPath } from '../scripts/resolvers/sections'; +import { ALL_MODEL_NAMES } from '../scripts/models'; +import { runGeneration } from '../scripts/gen-skill-docs'; +import { sectionPath, SKILL_BYTE_CEILING, usesLazySections } from '../scripts/resolvers/sections'; import { HOST_PATHS, type TemplateContext } from '../scripts/resolvers/types'; +import { prepareMethodology } from '../bin/gstack-autoplan-snapshot'; +const ROOT = path.resolve(import.meta.dir, '..'); +const EXTERNAL = ALL_HOST_CONFIGS.filter(c => c.name !== 'claude'); +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-external-carve-')); const context = (host: string, skillName: string): TemplateContext => ({ host, skillName, tmplPath: '', paths: HOST_PATHS[host] }); +const skillDir = (host: (typeof ALL_HOST_CONFIGS)[number], out: string, skill: string) => + host.name === 'claude' ? path.join(out, skill) : path.join(out, host.hostSubdir, 'skills', `gstack-${skill}`); + +function skillFiles(out: string): string[] { + const files: string[] = []; + const walk = (dir: string) => { + for (const e of fs.readdirSync(dir, { withFileTypes: true })) { + if (e.isDirectory()) { if (e.name !== 'node_modules') walk(path.join(dir, e.name)); } + else if (e.name === 'SKILL.md') files.push(path.join(dir, e.name)); + } + }; + walk(out); + return files; +} + +const renders: Record = {}; +beforeAll(async () => { + renders.all = path.join(tmp, 'all'); + const all = await runGeneration({ host: 'all', outputRoot: renders.all }); + expect(all.diagnostics.filter(d => d.kind === 'error' || d.kind === 'warning').map(d => d.message)).toEqual([]); + for (const model of ALL_MODEL_NAMES) { + renders[model] = path.join(tmp, `codex-${model}`); + const r = await runGeneration({ host: 'codex', model, outputRoot: renders[model] }); + expect(r.diagnostics.filter(d => d.kind === 'error').map(d => d.message)).toEqual([]); + } +}, 120_000); +afterAll(() => fs.rmSync(tmp, { recursive: true, force: true })); describe('C4: external section pointers', () => { - for (const config of ALL_HOST_CONFIGS.filter(c => c.name !== 'claude')) { + for (const config of EXTERNAL) { test(`${config.name}: ship and plan-ceo-review point relative to the installed skill directory`, () => { expect(sectionPath(context(config.name, 'ship'), 'ship', 'tests')) .toBe('`sections/tests.md` relative to the installed `gstack-ship` SKILL.md directory'); @@ -26,3 +64,70 @@ describe('C4: external section pointers', () => { expect(sectionPath(context('claude', 'ship'), 'ship', 'tests')).toBe('`~/.claude/skills/gstack/ship/sections/tests.md`'); }); }); + +describe('C4: ship and plan-ceo-review are carved on every external host', () => { + for (const config of EXTERNAL) { + test(`${config.name}: every pointer in the carved skill resolves to a generated section file`, () => { + for (const skill of ['ship', 'plan-ceo-review']) { + expect(usesLazySections(config.name, skill)).toBe(true); + const dir = skillDir(config, renders.all, skill); + const manifest = JSON.parse(fs.readFileSync(path.join(ROOT, skill, 'sections', 'manifest.json'), 'utf8')) as { sections: Array<{ file: string }> }; + const text = [fs.readFileSync(path.join(dir, 'SKILL.md'), 'utf8'), + ...manifest.sections.map(s => fs.readFileSync(path.join(dir, 'sections', s.file), 'utf8'))].join('\n'); + expect(text).not.toMatch(new RegExp(`\\$GSTACK_ROOT/${skill}/sections/`)); + const pointers = [...text.matchAll(new RegExp(`\`sections/([\\w.-]+)\` relative to the installed \`gstack-${skill}\` SKILL\\.md directory`, 'g'))].map(m => m[1]); + expect(pointers.length).toBeGreaterThan(0); + for (const file of pointers) expect(fs.existsSync(path.join(dir, 'sections', file)), `${skill}/sections/${file}`).toBe(true); + expect(fs.readFileSync(path.join(dir, 'SKILL.md'), 'utf8')).toContain('## Section index'); + } + }); + } + + test('codex: the autoplan methodology loader accepts the carved plan-ceo-review install', () => { + const codex = ALL_HOST_CONFIGS.find(c => c.name === 'codex')!; + const restore = path.join(tmp, 'restore.md'); + fs.writeFileSync(restore, '# plan\n'); + const prepared = prepareMethodology('ceo', path.join(skillDir(codex, renders.all, 'plan-ceo-review'), 'SKILL.md'), restore); + const methodology = fs.readFileSync(prepared.methodologyPath, 'utf8'); + expect(methodology).toContain('sections/review-sections.md'); + expect(methodology).toContain('## Review Sections'); + }); +}); + +describe('C4: UTF-8 byte gate (every host, every Codex model overlay)', () => { + test(`no generated SKILL.md exceeds ${SKILL_BYTE_CEILING} bytes`, () => { + const over: string[] = []; + let checked = 0; + for (const [name, out] of Object.entries(renders)) { + for (const file of skillFiles(out)) { + checked++; + const bytes = fs.statSync(file).size; + if (bytes > SKILL_BYTE_CEILING) over.push(`${name}: ${path.relative(out, file)} is ${bytes} bytes`); + } + } + expect(checked).toBeGreaterThan(500); + expect(over).toEqual([]); + }); + + test('the generator fails an oversized render in repo mode and only warns under --install-root', () => { + const fixture = path.join(tmp, 'oversized'); + fs.mkdirSync(fixture); + for (const entry of fs.readdirSync(ROOT)) { + if (['scripts', 'retro', '.git'].includes(entry)) continue; + fs.symlinkSync(path.join(ROOT, entry), path.join(fixture, entry)); + } + fs.cpSync(path.join(ROOT, 'scripts'), path.join(fixture, 'scripts'), { recursive: true }); + fs.cpSync(path.join(ROOT, 'retro'), path.join(fixture, 'retro'), { recursive: true }); + const tmpl = path.join(fixture, 'retro', 'SKILL.md.tmpl'); + fs.appendFileSync(tmpl, `\n${`${'Padding for the oversized-render fixture. '.repeat(20)}\n`.repeat(250)}`); + const gen = (...args: string[]) => spawnSync(process.execPath, ['scripts/gen-skill-docs.ts', '--host', 'codex', ...args], + { cwd: fixture, encoding: 'utf8', timeout: 60_000 }); + const repo = gen('--out-dir', path.join(fixture, 'out-repo')); + expect(repo.status).toBe(1); + expect(repo.stderr).toMatch(/codex\/gstack-retro\/SKILL\.md is \d+ bytes, over the 160,000-byte limit\. Fix: carve sections with usesLazySections\(\) for this skill\./); + const install = gen('--out-dir', path.join(fixture, 'out-install'), '--install-root', '/opt/gstack'); + expect(install.status, install.stderr).toBe(0); + expect(install.stderr).toContain('over the 160,000-byte limit'); + expect(fs.existsSync(path.join(fixture, 'out-install', '.agents', 'skills', 'gstack-retro', 'SKILL.md'))).toBe(true); + }); +}); diff --git a/test/qa-lazy-sections.test.ts b/test/qa-lazy-sections.test.ts index 4779689d6..f4e4b7042 100644 --- a/test/qa-lazy-sections.test.ts +++ b/test/qa-lazy-sections.test.ts @@ -162,18 +162,25 @@ describe('QA-only cross-host lazy rendering', () => { expect(fs.readFileSync(path.join(rendered, dir, 'SKILL.md'), 'utf8')).not.toContain(body.split('\n').slice(2).join('\n').trim()); } const outside = generated.artifacts.filter(artifact => artifact.host === host.name && artifact.kind === 'section' - && !/^(?:qa|qa-only)\//.test(artifact.relativePath) - && !/\/gstack-qa(?:-only)?\//.test(artifact.relativePath)); + && !/^(?:qa|qa-only|ship|plan-ceo-review)\//.test(artifact.relativePath) + && !/\/gstack-(?:qa(?:-only)?|ship|plan-ceo-review)\//.test(artifact.relativePath)); expect(outside.length > 0).toBe(host.name === 'claude'); + // C4: ship is carved on every host; external pointers are relative to the installed skill. const ctx = context(host.name, 'ship'); const manifest = JSON.parse(fs.readFileSync(path.join(ROOT, 'ship/sections/manifest.json'), 'utf8')); const entry = manifest.sections[0]; - expect(usesLazySections(host.name, 'ship')).toBe(host.name === 'claude'); - if (host.name === 'claude') { - expect(SECTION(ctx, [entry.id])).toBe(`> **STOP.** Before ${entry.trigger}, Read \`~/.claude/skills/gstack/ship/sections/${entry.file}\` and execute it\n> in full. Do not work from memory — that section is the source of truth for this step.`); - } else { - expect(SECTION(ctx, [entry.id])).toBe(fs.readFileSync(path.join(ROOT, 'ship/sections', `${entry.file}.tmpl`), 'utf8').trimEnd()); - expect(SECTION_INDEX(ctx)).toBe(''); + expect(usesLazySections(host.name, 'ship')).toBe(true); + const pointer = host.name === 'claude' + ? `\`~/.claude/skills/gstack/ship/sections/${entry.file}\`` + : `\`sections/${entry.file}\` relative to the installed \`gstack-ship\` SKILL.md directory`; + expect(SECTION(ctx, [entry.id])).toBe(`> **STOP.** Before ${entry.trigger}, Read ${pointer} and execute it\n> in full. Do not work from memory — that section is the source of truth for this step.`); + // Skills carved only on Claude still inline their sections elsewhere. + const reviewCtx = context(host.name, 'review'); + const reviewEntry = JSON.parse(fs.readFileSync(path.join(ROOT, 'review/sections/manifest.json'), 'utf8')).sections[0]; + expect(usesLazySections(host.name, 'review')).toBe(host.name === 'claude'); + if (host.name !== 'claude') { + expect(SECTION(reviewCtx, [reviewEntry.id])).toBe(fs.readFileSync(path.join(ROOT, 'review/sections', `${reviewEntry.file}.tmpl`), 'utf8').trimEnd()); + expect(SECTION_INDEX(reviewCtx)).toBe(''); } }); @@ -276,7 +283,7 @@ describe('QA-only cross-host lazy rendering', () => { for (const caller of ['review', 'ship']) { const dir = path.join(base, `${prefix}${caller}`); const body = fs.readFileSync(path.join(dir, 'SKILL.md'), 'utf8') - + (host.name === 'claude' && caller === 'ship' + + (caller === 'ship' // C4: ship is carved on every host ? fs.readFileSync(path.join(dir, 'sections/review-army.md'), 'utf8') : ''); expect(body).toContain(`From the installed /${caller} SKILL.md's directory`); expect(body).toContain(`Read \`../${prefix}qa/sections/exploratory.md\` in full`);