From 4d76cbb4ac0f26023fbf93e7e5368982bb9f100d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 25 May 2026 20:30:36 -0700 Subject: [PATCH] =?UTF-8?q?feat(resolvers):=20T2=20=E2=80=94=20ResolverEnt?= =?UTF-8?q?ry=20+=20appliesTo=20gate=20infrastructure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the conditional-resolver-injection plumbing from the v2_PLAN A.1 step. Resolvers can now be either a bare ResolverFn (always fires, current behavior) or a ResolverEntry { resolve, appliesTo? } (gated; appliesTo returning false skips the resolver, substitutes empty string). Why infrastructure-only: the audit during T0a confirmed most resolvers don't need gating. The {{NAME}} placeholder system is already conditional at the template level — a resolver only fires for skills that reference it. The gate is for future use when a placeholder's audience needs a structural guardrail beyond social convention, or when a sub-resolver inside a larger composed resolver (e.g. preamble) needs per-skill skip. scripts/gen-skill-docs.ts:444 now uses unwrapResolver() to handle both shapes. RESOLVERS map signature widens from Record to Record. All existing resolvers stay bare functions and work unchanged. Test plan: - bun test test/resolver-entry.test.ts: 6 pass (gate plumbing + registry) - bun test test/gen-skill-docs.test.ts: 389 pass (no regression) - bun run gen:skill-docs --dry-run: all SKILL.md files FRESH (no diff) Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/gen-skill-docs.ts | 10 ++-- scripts/resolvers/index.ts | 17 +++++-- scripts/resolvers/types.ts | 33 +++++++++++++ test/resolver-entry.test.ts | 93 +++++++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 7 deletions(-) create mode 100644 test/resolver-entry.test.ts diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index b89aea8b9..bee307c7a 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -16,7 +16,7 @@ import { writeLlmsTxt } from './gen-llms-txt'; import * as fs from 'fs'; import * as path from 'path'; import type { Host, TemplateContext } from './resolvers/types'; -import { HOST_PATHS } from './resolvers/types'; +import { HOST_PATHS, unwrapResolver } from './resolvers/types'; import { RESOLVERS } from './resolvers/index'; import { externalSkillName, extractHookSafetyProse as _extractHookSafetyProse, extractNameAndDescription as _extractNameAndDescription, condenseOpenAIShortDescription as _condenseOpenAIShortDescription, generateOpenAIYaml as _generateOpenAIYaml } from './resolvers/codex-helpers'; import { generatePlanCompletionAuditShip, generatePlanCompletionAuditReview, generatePlanVerificationExec } from './resolvers/review'; @@ -441,9 +441,11 @@ function processTemplate(tmplPath: string, host: Host = 'claude'): { outputPath: const resolverName = parts[0]; const args = parts.slice(1); if (suppressed.has(resolverName)) return ''; - const resolver = RESOLVERS[resolverName]; - if (!resolver) throw new Error(`Unknown placeholder {{${resolverName}}} in ${relTmplPath}`); - return args.length > 0 ? resolver(ctx, args) : resolver(ctx); + const entry = RESOLVERS[resolverName]; + if (!entry) throw new Error(`Unknown placeholder {{${resolverName}}} in ${relTmplPath}`); + const { resolve, appliesTo } = unwrapResolver(entry); + if (appliesTo && !appliesTo(ctx)) return ''; + return args.length > 0 ? resolve(ctx, args) : resolve(ctx); }); // Check for any remaining unresolved placeholders diff --git a/scripts/resolvers/index.ts b/scripts/resolvers/index.ts index c5cbd0445..6502960f9 100644 --- a/scripts/resolvers/index.ts +++ b/scripts/resolvers/index.ts @@ -1,9 +1,20 @@ /** - * RESOLVERS record — maps {{PLACEHOLDER}} names to generator functions. + * RESOLVERS record — maps {{PLACEHOLDER}} names to generator functions + * or gated entries. + * * Each resolver takes a TemplateContext and returns the replacement string. + * Resolvers may be either a bare function (always fires) or a gated entry + * ({ resolve, appliesTo }) where appliesTo can return false to skip the + * resolver for a given skill. See ./types.ts: ResolverEntry. + * + * Most resolvers don't need a gate — the {{NAME}} placeholder system is + * already conditional at the template level (the resolver only fires for + * skills that reference it). Use a gate when you want a structural + * guardrail that says "this placeholder is meaningful only in skills X, Y, Z" + * even if someone later adds {{NAME}} to skill W. */ -import type { TemplateContext, ResolverFn } from './types'; +import type { TemplateContext, ResolverFn, ResolverValue } from './types'; // Domain modules import { generatePreamble } from './preamble'; @@ -24,7 +35,7 @@ import { generateQuestionPreferenceCheck, generateQuestionLog, generateInlineTun import { generateMakePdfSetup } from './make-pdf'; import { generateTasksSectionEmit, generateTasksSectionAggregate } from './tasks-section'; -export const RESOLVERS: Record = { +export const RESOLVERS: Record = { SLUG_EVAL: generateSlugEval, SLUG_SETUP: generateSlugSetup, COMMAND_REFERENCE: generateCommandReference, diff --git a/scripts/resolvers/types.ts b/scripts/resolvers/types.ts index c8a44425b..2e300347c 100644 --- a/scripts/resolvers/types.ts +++ b/scripts/resolvers/types.ts @@ -66,3 +66,36 @@ export interface TemplateContext { /** Resolver function signature. args is populated for parameterized placeholders like {{INVOKE_SKILL:name}}. */ export type ResolverFn = (ctx: TemplateContext, args?: string[]) => string; + +/** + * Optional gated resolver. When the gate returns false, the resolver is + * skipped (substituted with empty string) — same effect as the placeholder + * not being referenced. Use when a resolver's output is only meaningful for + * a known subset of skills, so future template authors get a structural + * guardrail instead of relying on social knowledge. + * + * Most resolvers don't need this — the {{NAME}} placeholder system is + * already conditional at the template level. Use only when a resolver + * lives inside another resolver (e.g. via preamble composition) AND must + * be conditionalized, or when a top-level resolver has a small, well-defined + * audience. + */ +export interface ResolverEntry { + resolve: ResolverFn; + appliesTo?: (ctx: TemplateContext) => boolean; +} + +/** Anything the RESOLVERS map accepts — either a bare function or a gated entry. */ +export type ResolverValue = ResolverFn | ResolverEntry; + +/** + * Type-narrowing helper for the gen-skill-docs lookup. + * Returns (resolverFn, gate) so callers can do gate?.(ctx) before invoking. + */ +export function unwrapResolver(entry: ResolverValue): { + resolve: ResolverFn; + appliesTo?: (ctx: TemplateContext) => boolean; +} { + if (typeof entry === 'function') return { resolve: entry }; + return { resolve: entry.resolve, appliesTo: entry.appliesTo }; +} diff --git a/test/resolver-entry.test.ts b/test/resolver-entry.test.ts new file mode 100644 index 000000000..6d174ea6a --- /dev/null +++ b/test/resolver-entry.test.ts @@ -0,0 +1,93 @@ +/** + * Unit tests for the ResolverEntry / unwrapResolver mechanism. + * + * Verifies the conditional-injection plumbing added in T2 (v1.45.0.0). + * Plain functions still work; gated entries skip when appliesTo returns false. + */ + +import { describe, test, expect } from 'bun:test'; +import { unwrapResolver, type ResolverFn, type ResolverEntry, type TemplateContext } from '../scripts/resolvers/types'; + +function makeCtx(overrides: Partial = {}): TemplateContext { + return { + skillName: 'test-skill', + tmplPath: '/tmp/test/SKILL.md.tmpl', + host: 'claude', + paths: { + skillRoot: '~/.claude/skills/gstack', + localSkillRoot: '.claude/skills', + binDir: '~/.claude/skills/gstack/bin', + browseDir: '~/.claude/skills/gstack/browse/dist', + designDir: '~/.claude/skills/gstack/design/dist', + makePdfDir: '~/.claude/skills/gstack/make-pdf/dist', + }, + ...overrides, + }; +} + +describe('unwrapResolver — plain function pass-through', () => { + test('returns the function as-is, no gate', () => { + const fn: ResolverFn = (ctx) => `hello-${ctx.skillName}`; + const { resolve, appliesTo } = unwrapResolver(fn); + expect(resolve(makeCtx())).toBe('hello-test-skill'); + expect(appliesTo).toBeUndefined(); + }); +}); + +describe('unwrapResolver — gated entry', () => { + test('returns resolve + gate', () => { + const entry: ResolverEntry = { + resolve: (ctx) => `gated-${ctx.skillName}`, + appliesTo: (ctx) => ['ship', 'review'].includes(ctx.skillName), + }; + const { resolve, appliesTo } = unwrapResolver(entry); + expect(resolve(makeCtx({ skillName: 'ship' }))).toBe('gated-ship'); + expect(appliesTo!(makeCtx({ skillName: 'ship' }))).toBe(true); + expect(appliesTo!(makeCtx({ skillName: 'qa' }))).toBe(false); + }); + + test('gate returning false should signal skip — gen-skill-docs substitutes empty string', () => { + // This mirrors the gen-skill-docs.ts contract: + // if (appliesTo && !appliesTo(ctx)) return ''; + const entry: ResolverEntry = { + resolve: () => 'CONTENT', + appliesTo: () => false, + }; + const { resolve, appliesTo } = unwrapResolver(entry); + const result = appliesTo && !appliesTo(makeCtx()) ? '' : resolve(makeCtx()); + expect(result).toBe(''); + }); + + test('gate returning true allows resolve to fire', () => { + const entry: ResolverEntry = { + resolve: () => 'CONTENT', + appliesTo: () => true, + }; + const { resolve, appliesTo } = unwrapResolver(entry); + const result = appliesTo && !appliesTo(makeCtx()) ? '' : resolve(makeCtx()); + expect(result).toBe('CONTENT'); + }); + + test('entry without appliesTo behaves like ungated', () => { + const entry: ResolverEntry = { resolve: () => 'ALWAYS' }; + const { resolve, appliesTo } = unwrapResolver(entry); + expect(appliesTo).toBeUndefined(); + expect(resolve(makeCtx())).toBe('ALWAYS'); + }); +}); + +describe('RESOLVERS registry still loads with mixed shapes', () => { + test('importing the live registry produces a record with expected resolvers', async () => { + const { RESOLVERS } = await import('../scripts/resolvers/index'); + // Spot-check that core resolvers are present. + expect(RESOLVERS.PREAMBLE).toBeDefined(); + expect(RESOLVERS.REVIEW_DASHBOARD).toBeDefined(); + expect(RESOLVERS.SLUG_EVAL).toBeDefined(); + // Each entry should unwrap cleanly. + for (const [name, entry] of Object.entries(RESOLVERS)) { + const { resolve } = unwrapResolver(entry); + expect(typeof resolve).toBe('function'); + expect(name.length).toBeGreaterThan(0); + } + }); +});