From 58192039056f921d3438ac0d9de298ef620dc765 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 12 Aug 2026 10:57:27 -0700 Subject: [PATCH] feat(evals): register shipped skills for hermetic PTY children MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hermetic children get a config dir that deliberately seeds no skills — right for children that install their own, fatal for the PTY family that TYPES /office-hours or /plan-ceo-review: claude rejects the command as Unknown before any model turn, so the plan-family gate smokes measure nothing. hermeticSkillsConfigDir() is a second, opt-in config dir under the same runRoot that mirrors ./setup's registration exactly (real dir per registry name, SKILL.md + sections/ symlinks, frontmatter-name resolution, _gstack-command root alias), driven by the shared skill-census so connect-chrome's dir symlink collapses the same way setup's idempotent overwrite does. Ported from fork commit 03c4eca2, tree walk rewritten for the upstream layout (top-level /SKILL.md dirs, no skills/ tree). Unit tests are new: seed shape, census parity, symlink resolution, connect-chrome collapse, idempotence, no-API-key seed. Co-Authored-By: Claude Fable 5 (cherry picked from commit 93dae6107b30ce453a07c2d342b60262bba6ce0b) --- test/helpers/hermetic-env.ts | 70 ++++++++++++++++++++++- test/hermetic-skills-seeding.test.ts | 84 ++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 test/hermetic-skills-seeding.test.ts diff --git a/test/helpers/hermetic-env.ts b/test/helpers/hermetic-env.ts index def6dd9bc..4fb17e734 100644 --- a/test/helpers/hermetic-env.ts +++ b/test/helpers/hermetic-env.ts @@ -36,7 +36,8 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { promotedEnv } from '../../lib/conductor-env-shim'; -import { isProcessAlive } from '../../browse/src/error-handling'; +import { isProcessAlive, safeUnlink } from '../../browse/src/error-handling'; +import { skillCensus, frontmatterName } from './skill-census'; /** Exact env names a hermetic child keeps. Everything not listed (or matched * by a prefix rule below) is dropped. */ @@ -225,6 +226,73 @@ export function getHermeticDirs(): HermeticDirs { return cachedDirs; } +let cachedSkillsConfigDir: string | null = null; + +/** + * A hermetic CLAUDE_CONFIG_DIR with the repo's shipped skills REGISTERED in + * user scope, mirroring ./setup's registration exactly: each discovered skill + * gets a REAL directory `/skills//` containing a + * SYMLINK to that skill's SKILL.md (absolute path), plus a `sections/` + * symlink when the skill has one. registryName is the frontmatter `name:` + * (dir-name fallback), NO gstack- prefix; the root SKILL.md router registers + * as `_gstack-command`. skillCensus().registryEntries is the authoritative + * set of what must appear here. + * + * The default hermetic dir deliberately seeds no skills — correct for + * children that install their own or probe setup behavior — but a PTY test + * that TYPES `/office-hours` needs the slash command to exist, or claude + * rejects it as Unknown command before any model turn and the gate measures + * nothing. Separate dir, same runRoot: opt-in per session, never contaminates + * the default-config children, and the existing exit teardown + pid-aware GC + * cover it. Ends in `/.claude` for the same plan-path anchoring reason as + * HermeticDirs.configDir. + * + * Two intentional non-hermetic edges: + * - Seeding reads the LIVE repo tree BY DESIGN — the skills ARE the subject + * under test; a snapshot would measure stale copies. + * - HOME is not hermeticized, so the ~64 absolute + * `~/.claude/skills/gstack/...` preamble references inside each SKILL.md + * still resolve to the operator install (same limitation as CI). + */ +export function hermeticSkillsConfigDir(): string { + if (cachedSkillsConfigDir) return cachedSkillsConfigDir; + const { runRoot } = getHermeticDirs(); + const configDir = path.join(runRoot, 'with-skills', '.claude'); + const skillsDir = path.join(configDir, 'skills'); + fs.mkdirSync(skillsDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, '.claude.json'), + JSON.stringify(buildSeedConfig({ + apiKey: process.env.ANTHROPIC_API_KEY ?? process.env.GSTACK_ANTHROPIC_API_KEY, + trustedDirs: [repoRoot()], + }), null, 2), + ); + const root = repoRoot(); + for (const rel of skillCensus(root).physicalSkillFiles) { + const skillMd = path.join(root, rel); + const skillDir = path.dirname(rel); + const registryName = rel === 'SKILL.md' + ? '_gstack-command' + : frontmatterName(skillMd) || skillDir; + const target = path.join(skillsDir, registryName); + // Idempotent overwrite mirrors setup's re-link: connect-chrome (a dir + // symlink to open-gstack-browser) shares its target's frontmatter name, + // so the two walk entries collapse to one registry dir. + fs.mkdirSync(target, { recursive: true }); + safeUnlink(path.join(target, 'SKILL.md')); + fs.symlinkSync(skillMd, path.join(target, 'SKILL.md')); + if (rel !== 'SKILL.md') { + const sections = path.join(root, skillDir, 'sections'); + if (fs.existsSync(sections)) { + safeUnlink(path.join(target, 'sections')); + fs.symlinkSync(sections, path.join(target, 'sections')); + } + } + } + cachedSkillsConfigDir = configDir; + return configDir; +} + /** A dir younger than this is never GC'd even if its pid looks dead — guards * against PID reuse deleting a freshly-created dir whose original pid exited * and was recycled to an unrelated live process between create and GC. */ diff --git a/test/hermetic-skills-seeding.test.ts b/test/hermetic-skills-seeding.test.ts new file mode 100644 index 000000000..43b294db9 --- /dev/null +++ b/test/hermetic-skills-seeding.test.ts @@ -0,0 +1,84 @@ +/** + * Unit tests for hermeticSkillsConfigDir() — the opt-in hermetic config dir + * that registers the repo's shipped skills for PTY slash-command children. + * Free tier — no API calls; exercises the real seeder against the live repo + * tree (that's the seeder's contract: the skills ARE the subject under test). + * + * Pins four contracts: + * 1. The seeded dir is a valid CLAUDE_CONFIG_DIR (.claude.json present, + * /.claude suffix, under the hermetic runRoot). + * 2. Registration mirrors ./setup exactly: one entry per + * skillCensus().registryEntries, each a REAL dir with a SKILL.md symlink + * resolving to a real file (plus sections/ when the skill has one). + * 3. connect-chrome (dir symlink) collapses into open-gstack-browser — no + * duplicate, no connect-chrome entry. + * 4. Per-process idempotence: the second call returns the cached dir. + */ + +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; +import { + hermeticSkillsConfigDir, + getHermeticDirs, + buildSeedConfig, +} from './helpers/hermetic-env'; +import { skillCensus } from './helpers/skill-census'; + +const ROOT = path.resolve(__dirname, '..'); +const configDir = hermeticSkillsConfigDir(); +const skillsDir = path.join(configDir, 'skills'); + +describe('hermeticSkillsConfigDir', () => { + test('seeded dir contains .claude.json and ends in /.claude under runRoot', () => { + expect(fs.existsSync(path.join(configDir, '.claude.json'))).toBe(true); + expect(path.basename(configDir)).toBe('.claude'); + expect(configDir.startsWith(getHermeticDirs().runRoot + path.sep)).toBe(true); + }); + + test('one registry entry per skillCensus registryEntries, nothing extra', () => { + const seeded = fs.readdirSync(skillsDir).sort(); + expect(seeded).toEqual(skillCensus(ROOT).registryEntries); + }); + + test('every SKILL.md is a symlink resolving to a real file', () => { + for (const entry of fs.readdirSync(skillsDir)) { + const link = path.join(skillsDir, entry, 'SKILL.md'); + expect(fs.lstatSync(link).isSymbolicLink()).toBe(true); + expect(fs.statSync(link).isFile()).toBe(true); // follows the link + } + }); + + test('sections/ symlink registered for skills that ship one', () => { + // ship/ is a carved skill with a sections/ dir — the registered entry + // must expose it or runtime "Read sections/.md" 404s. + expect(fs.existsSync(path.join(ROOT, 'ship', 'sections'))).toBe(true); + const link = path.join(skillsDir, 'ship', 'sections'); + expect(fs.lstatSync(link).isSymbolicLink()).toBe(true); + expect(fs.statSync(link).isDirectory()).toBe(true); + }); + + test('connect-chrome collapses into a single open-gstack-browser entry', () => { + const seeded = fs.readdirSync(skillsDir); + expect(seeded.filter((n) => n === 'open-gstack-browser')).toHaveLength(1); + expect(seeded).not.toContain('connect-chrome'); + }); + + test('root router registered as _gstack-command pointing at the root SKILL.md', () => { + const link = path.join(skillsDir, '_gstack-command', 'SKILL.md'); + expect(fs.realpathSync(link)).toBe(fs.realpathSync(path.join(ROOT, 'SKILL.md'))); + }); + + test('second call returns the cached dir', () => { + expect(hermeticSkillsConfigDir()).toBe(configDir); + }); + + test('buildSeedConfig with undefined apiKey omits customApiKeyResponses', () => { + // The seeder passes process.env keys straight through; when the operator + // has no key exported the seed must stay valid (child fails auth later, + // not here). + const seed = buildSeedConfig({ apiKey: undefined, trustedDirs: [ROOT] }); + expect(seed).not.toHaveProperty('customApiKeyResponses'); + expect(seed.hasCompletedOnboarding).toBe(true); + }); +});