From 0cc37a94c25eedbd4432f10ab2605785c4c157ab Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 11 Jun 2026 14:40:45 -0400 Subject: [PATCH] =?UTF-8?q?feat(#1056):=20phase=205e=20=E2=80=94=20close?= =?UTF-8?q?=20ConverterName=20enum=20+=20configFormat=E2=86=94installSurfa?= =?UTF-8?q?ce=20parity=20guard=20(#1057)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gen-time validation tightenings (validation-only; serialized registry content unchanged; bin/install.js + adapter + descriptors untouched): Part B: validateArtifactKindEntry now requires artifactLayout[].converter ∈ VALID_CONVERTER_NAMES (15 names, all exported by install.js) ∪ {null} — a typo'd converter fails at gen time instead of silently → installExports[name]===undefined at install time. Part A: a HARD buildRegistry parity gate asserts each runtime descriptor's configFormat agrees with the adapter registry's installSurface via a fixed mapping (cursor-hooks-json/profile-marker-only→none, codex-toml→toml, copilot-instructions→ markdown, cline-rules→markdown-dir, settings-json→settings-json) — keeps configFormat from drifting; prerequisite-validation for the deferred full drive (#1055). The full config-writing drive (retire resolveRuntimeConfigIntent) is deferred to #1055: configFormat is lossy vs installSurface (cursor vs profile-marker both → none; opencode/kilo permissionWriter has no descriptor field) → needs schema extension. Closes #1056 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 --- scripts/gen-capability-registry.cjs | 151 ++++++++++++++- tests/capability-registry.test.cjs | 279 ++++++++++++++++++++++++++++ 2 files changed, 429 insertions(+), 1 deletion(-) diff --git a/scripts/gen-capability-registry.cjs b/scripts/gen-capability-registry.cjs index f2a10026b..901891208 100644 --- a/scripts/gen-capability-registry.cjs +++ b/scripts/gen-capability-registry.cjs @@ -450,6 +450,26 @@ function validateFeatureBody(cap) { return errors; } +// ADR-857 phase 5e: Closed ConverterName enum — complete set used across 16 runtime descriptors, +// all exported by bin/install.js. Any ArtifactKind with a non-null converter must use one of these. +const VALID_CONVERTER_NAMES = new Set([ + 'convertClaudeCommandToAntigravitySkill', + 'convertClaudeCommandToAugmentSkill', + 'convertClaudeCommandToClineSkill', + 'convertClaudeCommandToClaudeSkill', + 'convertClaudeCommandToCodebuddyCommand', + 'convertClaudeCommandToCodebuddySkill', + 'convertClaudeCommandToCodexSkill', + 'convertClaudeCommandToCopilotSkill', + 'convertClaudeCommandToCursorCommand', + 'convertClaudeCommandToCursorSkill', + 'convertClaudeCommandToKiloSkill', + 'convertClaudeCommandToKimiSkill', + 'convertClaudeCommandToOpencodeSkill', + 'convertClaudeCommandToTraeSkill', + 'convertClaudeCommandToWindsurfSkill', +]); + // C3: Validate role:runtime body const VALID_CONFIG_FORMATS = new Set(['settings-json', 'toml', 'markdown', 'markdown-dir', 'none']); const VALID_CONFIG_HOME_KINDS = new Set(['dot-home', 'dot-home-nested', 'xdg', 'generic-agents-root']); @@ -605,11 +625,15 @@ function validateArtifactKindEntry(capId, entry, prefix) { } } - // converter — required; must be a string or null (closed ConverterName enum in phase 5e) + // converter — required; must be a string or null (closed ConverterName enum — now enforced in phase 5e) if (!Object.prototype.hasOwnProperty.call(entry, 'converter')) { errors.push(ctx + '.converter is required (must be a string or null)'); } else if (entry.converter !== null && typeof entry.converter !== 'string') { errors.push(ctx + '.converter must be a string or null (got: ' + typeof entry.converter + ')'); + } else if (entry.converter !== null && typeof entry.converter === 'string' && + !VALID_CONVERTER_NAMES.has(entry.converter)) { + // Closed ConverterName enum (ADR-857 phase 5e): reject unknown converter names + errors.push(ctx + '.converter "' + entry.converter + '" is not a known ConverterName'); } return errors; @@ -1525,6 +1549,122 @@ function runConsistencyGate(capabilityClusters, profileMembership, capMap) { return warnings; } +// ─── ADR-857 phase 5e: configFormat ↔ installSurface parity gate ───────────── + +// Paths are declared at top level; the actual require() call is deferred (lazy) so importing +// this generator on an unbuilt worktree doesn't fail at module load. Mirrors the pattern +// used for install-profiles.cjs and clusters.cjs above. +const RUNTIME_CONFIG_ADAPTER_REGISTRY_PATH = path.join( + ROOT, 'gsd-core', 'bin', 'lib', 'runtime-config-adapter-registry.cjs', +); + +let _runtimeConfigAdapterMod = null; + +function getRuntimeConfigAdapterRegistry() { + if (!_runtimeConfigAdapterMod) { + _runtimeConfigAdapterMod = require(RUNTIME_CONFIG_ADAPTER_REGISTRY_PATH); + } + return _runtimeConfigAdapterMod; +} + +// Map: installSurface → expected configFormat +// Derived from the pairing of runtime-config-adapter-registry.cjs (installSurface) +// and the capability.json descriptors (configFormat). DEFECT.GENERATIVE-FIX: this map +// is the single parity contract between the two generated surfaces. +const INSTALL_SURFACE_TO_CONFIG_FORMAT = new Map([ + ['settings-json', 'settings-json'], + ['codex-toml', 'toml'], + ['copilot-instructions', 'markdown'], + ['cline-rules', 'markdown-dir'], + ['cursor-hooks-json', 'none'], + ['profile-marker-only', 'none'], +]); + +/** + * ADR-857 phase 5e: configFormat ↔ installSurface parity gate. + * + * For each runtime capability that also appears in INSTALL_SURFACES (i.e. is a + * known config-adapter runtime), assert that its capability.json configFormat + * matches the expected value derived from its installSurface. + * + * HARD gate — throws on mismatch (DEFECT.GENERATIVE-FIX: this invariant is + * derived from two parallel generated surfaces and must fail loudly). + * SOFT skip — if the runtime-config-adapter-registry.cjs module is not loadable + * (unbuilt worktree), emits a warning to stderr and returns without throwing. + * + * @param {Map} capMap Fully-validated capability map. + * @returns {void} Throws on mismatch; returns normally on success or soft-skip. + */ +function runConfigFormatParityGate(capMap) { + let adapterMod; + try { + adapterMod = getRuntimeConfigAdapterRegistry(); + } catch (_err) { + // Module not loadable (unbuilt worktree) — soft-skip with warning + process.stderr.write( + '⚠ configFormat parity gate SKIPPED: runtime-config-adapter-registry.cjs not loadable ' + + '(run `npm run build` first)\n', + ); + return; + } + + // Check that REGISTRY is present and is an object-like registry + // (the .cjs exports resolveRuntimeConfigIntent, ALLOWED_CONFIG_RUNTIMES, INSTALL_SURFACES — + // not REGISTRY directly; we need to reconstruct per-runtime installSurface from the adapter). + // We use ALLOWED_CONFIG_RUNTIMES to know which runtimes are in the adapter, then resolve each. + const { resolveRuntimeConfigIntent, ALLOWED_CONFIG_RUNTIMES: allowedRuntimes } = adapterMod; + + if (typeof resolveRuntimeConfigIntent !== 'function' || !(allowedRuntimes instanceof Set)) { + process.stderr.write( + '⚠ configFormat parity gate SKIPPED: runtime-config-adapter-registry.cjs missing expected exports\n', + ); + return; + } + + // Only check runtimes present in BOTH the capability registry and INSTALL_SURFACES + for (const [capId, cap] of capMap) { + if (cap.role !== 'runtime') continue; + if (!allowedRuntimes.has(capId)) continue; // grok etc. excluded — not in adapter + + const r = cap.runtime; + if (!r || typeof r.configFormat !== 'string') continue; // already validated above + + let intent; + try { + intent = resolveRuntimeConfigIntent(capId); + } catch (_err) { + // Should not happen (we checked allowedRuntimes.has(capId)), but be defensive + process.stderr.write( + '⚠ configFormat parity gate: could not resolve installSurface for "' + capId + '" — skipping\n', + ); + continue; + } + + const installSurface = intent.installSurface; + const expectedConfigFormat = INSTALL_SURFACE_TO_CONFIG_FORMAT.get(installSurface); + + if (expectedConfigFormat === undefined) { + // Unknown installSurface — the mapping needs to be updated + throw new Error( + 'configFormat parity gate: runtime "' + capId + '" has installSurface "' + installSurface + + '" which is not in the INSTALL_SURFACE_TO_CONFIG_FORMAT mapping — ' + + 'update the mapping in scripts/gen-capability-registry.cjs', + ); + } + + if (r.configFormat !== expectedConfigFormat) { + throw new Error( + 'configFormat parity gate FAILED for runtime "' + capId + '":\n' + + ' installSurface: ' + installSurface + '\n' + + ' expected configFormat: ' + expectedConfigFormat + '\n' + + ' actual configFormat: ' + r.configFormat + '\n' + + 'The capability.json configFormat must match the value derived from installSurface ' + + '(src: scripts/gen-capability-registry.cjs INSTALL_SURFACE_TO_CONFIG_FORMAT)', + ); + } + } +} + // ─── Registry builder ───────────────────────────────────────────────────────── /** @@ -1748,6 +1888,10 @@ function buildRegistry(capMap) { // without affecting the serialized file content (determinism gate stays clean). const reconciliationWarnings = runConsistencyGate(capabilityClusters, profileMembership, capMap); + // ADR-857 phase 5e: configFormat ↔ installSurface parity gate. + // HARD gate — throws on mismatch; SOFT skip if adapter module not loadable. + runConfigFormatParityGate(capMap); + return { version: SCHEMA_VERSION, capabilities, @@ -2074,6 +2218,11 @@ module.exports = { VALID_SANDBOX_TIERS, VALID_ARTIFACT_KIND_NAMES, VALID_ARTIFACT_NESTINGS, + // ADR-857 phase 5e: closed ConverterName enum + VALID_CONVERTER_NAMES, + // ADR-857 phase 5e: configFormat ↔ installSurface parity gate + runConfigFormatParityGate, + INSTALL_SURFACE_TO_CONFIG_FORMAT, // FIX 5 (lazy): PROFILE_RANK and CLUSTERS are loaded on first access via getters // so importing the generator on a fresh/unbuilt worktree doesn't fail at module load. get PROFILE_RANK() { return getInstallProfiles().PROFILE_RANK; }, diff --git a/tests/capability-registry.test.cjs b/tests/capability-registry.test.cjs index 567229524..26d0f07b8 100644 --- a/tests/capability-registry.test.cjs +++ b/tests/capability-registry.test.cjs @@ -39,6 +39,11 @@ const { PROFILE_RANK, // ADR-959 validateCommandEntry, + // ADR-857 phase 5e + VALID_CONVERTER_NAMES, + validateArtifactKindEntry, + runConfigFormatParityGate, + INSTALL_SURFACE_TO_CONFIG_FORMAT, } = require('../scripts/gen-capability-registry.cjs'); const ROOT = path.resolve(__dirname, '..'); @@ -3281,3 +3286,277 @@ describe('ADR-1016 phase 5a: closed-vocab set exports', () => { assert.strictEqual(VALID_ARTIFACT_NESTINGS.size, 2); }); }); + +// ─── 25. ADR-857 phase 5e: closed ConverterName enum (Part B) ───────────────── + +describe('ADR-857 phase 5e: VALID_CONVERTER_NAMES closed enum', () => { + test('VALID_CONVERTER_NAMES has exactly 15 entries', () => { + assert.ok(VALID_CONVERTER_NAMES instanceof Set, 'VALID_CONVERTER_NAMES must be a Set'); + assert.strictEqual(VALID_CONVERTER_NAMES.size, 15, 'VALID_CONVERTER_NAMES must have exactly 15 entries, got: ' + VALID_CONVERTER_NAMES.size); + }); + + test('VALID_CONVERTER_NAMES contains all expected converter names', () => { + const expected = [ + 'convertClaudeCommandToAntigravitySkill', + 'convertClaudeCommandToAugmentSkill', + 'convertClaudeCommandToClineSkill', + 'convertClaudeCommandToClaudeSkill', + 'convertClaudeCommandToCodebuddyCommand', + 'convertClaudeCommandToCodebuddySkill', + 'convertClaudeCommandToCodexSkill', + 'convertClaudeCommandToCopilotSkill', + 'convertClaudeCommandToCursorCommand', + 'convertClaudeCommandToCursorSkill', + 'convertClaudeCommandToKiloSkill', + 'convertClaudeCommandToKimiSkill', + 'convertClaudeCommandToOpencodeSkill', + 'convertClaudeCommandToTraeSkill', + 'convertClaudeCommandToWindsurfSkill', + ]; + for (const name of expected) { + assert.ok(VALID_CONVERTER_NAMES.has(name), 'VALID_CONVERTER_NAMES must contain "' + name + '"'); + } + }); +}); + +describe('ADR-857 phase 5e: validateArtifactKindEntry — ConverterName enum (FAIL-FIRST regression)', () => { + // Helper to build a minimal valid ArtifactKind entry + function makeArtifactEntry(overrides) { + return { + kind: 'skills', + destSubpath: 'skills', + nesting: 'flat', + prefix: 'gsd-', + recursive: false, + converter: null, + ...overrides, + }; + } + + // FAIL-FIRST: unknown converter name must be rejected + test('REJECTED: converter "convertClaudeCommandToUnknownRuntime" is not a known ConverterName', () => { + const entry = makeArtifactEntry({ converter: 'convertClaudeCommandToUnknownRuntime' }); + const errors = validateArtifactKindEntry('test-cap', entry, 'artifactLayout.global[0]'); + assert.ok(errors.length > 0, 'Expected rejection for unknown converter name, got: ' + JSON.stringify(errors)); + assert.ok( + errors.some((e) => e.includes('convertClaudeCommandToUnknownRuntime') && e.includes('not a known ConverterName')), + 'Error must name the bad converter and say "not a known ConverterName", got: ' + JSON.stringify(errors), + ); + }); + + // Valid known name must be accepted + test('ACCEPTED: converter "convertClaudeCommandToKiloSkill" is a known ConverterName', () => { + const entry = makeArtifactEntry({ converter: 'convertClaudeCommandToKiloSkill' }); + const errors = validateArtifactKindEntry('test-cap', entry, 'artifactLayout.global[0]'); + const converterErrors = errors.filter((e) => e.includes('converter')); + assert.deepEqual(converterErrors, [], 'Known converter name must be accepted, got: ' + JSON.stringify(converterErrors)); + }); + + // null converter is always accepted (means "no conversion") + test('ACCEPTED: converter: null is always accepted', () => { + const entry = makeArtifactEntry({ converter: null }); + const errors = validateArtifactKindEntry('test-cap', entry, 'artifactLayout.global[0]'); + const converterErrors = errors.filter((e) => e.includes('converter')); + assert.deepEqual(converterErrors, [], 'converter: null must always be accepted, got: ' + JSON.stringify(converterErrors)); + }); + + // Parity: all 16 runtime descriptors must have converters in the valid set (or null) + test('all 16 real runtime descriptors have converters in VALID_CONVERTER_NAMES or null', () => { + const { capMap, errors } = loadAndValidate(new Set()); + const hardErrors = errors.filter((e) => !e.includes('pending-migration')); + assert.deepEqual(hardErrors, [], 'Expected no hard errors from real capabilities, got: ' + JSON.stringify(hardErrors)); + + const runtimeIds = [ + 'claude', 'codex', 'antigravity', 'gemini', 'cursor', 'opencode', + 'kilo', 'copilot', 'augment', 'trae', 'qwen', 'hermes', + 'codebuddy', 'cline', 'kimi', 'windsurf', + ]; + for (const id of runtimeIds) { + const cap = capMap.get(id); + assert.ok(cap, 'capMap must contain "' + id + '"'); + const r = cap.runtime; + const allEntries = [ + ...(r.artifactLayout && Array.isArray(r.artifactLayout.global) ? r.artifactLayout.global : []), + ...(r.artifactLayout && Array.isArray(r.artifactLayout.local) ? r.artifactLayout.local : []), + ]; + for (let i = 0; i < allEntries.length; i++) { + const entry = allEntries[i]; + if (entry.converter !== null) { + assert.ok( + VALID_CONVERTER_NAMES.has(entry.converter), + id + ' artifactLayout[' + i + '].converter "' + entry.converter + + '" is not in VALID_CONVERTER_NAMES', + ); + } + } + } + }); + + // validateCapability end-to-end: unknown converter propagates through the full chain + test('validateCapability REJECTS a runtime cap with unknown converter in artifactLayout', () => { + const cap = { + id: 'test-rt', + role: 'runtime', + title: 'Test Runtime', + description: 'Test runtime with unknown converter.', + tier: 'core', + requires: [], + runtime: { + configHome: { kind: 'dot-home', name: '.test-rt', env: [] }, + configFormat: 'settings-json', + artifactLayout: { + global: [{ + kind: 'skills', + destSubpath: 'skills', + nesting: 'flat', + prefix: 'gsd-', + recursive: false, + converter: 'convertClaudeCommandToUnknownRuntime', + }], + local: [], + }, + commandStyle: 'slash-hyphen', + hooksSurface: 'settings-json', + sandboxTier: 'none', + supportTier: 1, + }, + }; + const errors = validateCapability(cap, 'test-rt'); + assert.ok(errors.length > 0, 'Expected validation errors for unknown converter, got: ' + JSON.stringify(errors)); + assert.ok( + errors.some((e) => e.includes('convertClaudeCommandToUnknownRuntime') && e.includes('not a known ConverterName')), + 'Error must mention the unknown converter name, got: ' + JSON.stringify(errors), + ); + }); +}); + +// ─── 26. ADR-857 phase 5e: configFormat ↔ installSurface parity gate (Part A) ─ + +describe('ADR-857 phase 5e: configFormat ↔ installSurface parity gate', () => { + // Helper: build a minimal runtime capMap for parity tests + function makeRuntimeCapMap(runtimeId, configFormat) { + const cap = { + id: runtimeId, + role: 'runtime', + title: 'Test ' + runtimeId, + description: 'Synthetic runtime for parity gate testing.', + tier: 'core', + requires: [], + runtime: { + configHome: { kind: 'dot-home', name: '.' + runtimeId, env: [] }, + configFormat, + artifactLayout: { global: [], local: [] }, + commandStyle: 'slash-hyphen', + hooksSurface: 'none', + sandboxTier: 'none', + supportTier: 1, + }, + }; + return new Map([[runtimeId, cap]]); + } + + // FAIL-FIRST: parity mismatch must throw + test('THROWS: claude with wrong configFormat "toml" (installSurface=settings-json → expected settings-json)', () => { + // claude has installSurface=settings-json → expected configFormat=settings-json + // Giving it configFormat=toml must trigger the HARD gate + const capMap = makeRuntimeCapMap('claude', 'toml'); + assert.throws( + () => runConfigFormatParityGate(capMap), + (err) => { + assert.ok(err instanceof Error, 'Must throw an Error'); + assert.ok( + err.message.includes('claude') && err.message.includes('parity gate FAILED'), + 'Error must name the runtime and say "parity gate FAILED", got: ' + err.message, + ); + assert.ok( + err.message.includes('settings-json') && err.message.includes('toml'), + 'Error must name both the expected and actual configFormat, got: ' + err.message, + ); + return true; + }, + ); + }); + + test('THROWS: codex with wrong configFormat "settings-json" (installSurface=codex-toml → expected toml)', () => { + const capMap = makeRuntimeCapMap('codex', 'settings-json'); + assert.throws( + () => runConfigFormatParityGate(capMap), + (err) => { + assert.ok(err instanceof Error); + assert.ok(err.message.includes('codex') && err.message.includes('parity gate FAILED')); + return true; + }, + ); + }); + + // All 16 real runtime descriptors must pass the parity gate (true-negative) + test('all 16 real runtime descriptors pass the configFormat parity gate (DOES NOT THROW)', () => { + const { capMap, errors } = loadAndValidate(new Set()); + const hardErrors = errors.filter((e) => !e.includes('pending-migration')); + assert.deepEqual(hardErrors, [], 'No hard errors expected: ' + JSON.stringify(hardErrors)); + + // runConfigFormatParityGate must not throw for the real registry + assert.doesNotThrow( + () => runConfigFormatParityGate(capMap), + 'runConfigFormatParityGate must not throw for the real 16 runtime descriptors', + ); + }); + + // buildRegistry must not throw for the real registry (end-to-end integration) + test('buildRegistry with real 16 runtime descriptors does not throw (parity gate integrated)', () => { + const { capMap } = loadAndValidate(new Set()); + assert.doesNotThrow( + () => buildRegistry(capMap), + 'buildRegistry must not throw for the real registry (parity gate must pass)', + ); + }); + + // INSTALL_SURFACE_TO_CONFIG_FORMAT export check + test('INSTALL_SURFACE_TO_CONFIG_FORMAT covers all 6 installSurface values with correct mappings', () => { + assert.ok(INSTALL_SURFACE_TO_CONFIG_FORMAT instanceof Map, 'Must be a Map'); + assert.strictEqual(INSTALL_SURFACE_TO_CONFIG_FORMAT.size, 6, 'Must cover 6 installSurface values'); + assert.strictEqual(INSTALL_SURFACE_TO_CONFIG_FORMAT.get('settings-json'), 'settings-json'); + assert.strictEqual(INSTALL_SURFACE_TO_CONFIG_FORMAT.get('codex-toml'), 'toml'); + assert.strictEqual(INSTALL_SURFACE_TO_CONFIG_FORMAT.get('copilot-instructions'), 'markdown'); + assert.strictEqual(INSTALL_SURFACE_TO_CONFIG_FORMAT.get('cline-rules'), 'markdown-dir'); + assert.strictEqual(INSTALL_SURFACE_TO_CONFIG_FORMAT.get('cursor-hooks-json'), 'none'); + assert.strictEqual(INSTALL_SURFACE_TO_CONFIG_FORMAT.get('profile-marker-only'), 'none'); + }); + + // Feature capabilities (role:feature) are silently ignored by the gate + test('feature capabilities (role:feature) are ignored by the parity gate — does not throw', () => { + // Use the real UI cap (role:feature, has no installSurface) — gate must pass silently + const capMap = new Map([['ui', UI_CAP]]); + assert.doesNotThrow( + () => runConfigFormatParityGate(capMap), + 'Feature capabilities must be ignored by the configFormat parity gate', + ); + }); + + // Unknown runtimes (not in ALLOWED_CONFIG_RUNTIMES) are excluded from the gate + test('runtime capId not in adapter registry (e.g. "grok") is excluded from parity gate — does not throw', () => { + // 'grok' is not in the adapter registry → must be soft-skipped + const grokCap = { + id: 'grok', + role: 'runtime', + title: 'Grok', + description: 'Hypothetical grok runtime', + tier: 'core', + requires: [], + runtime: { + configHome: { kind: 'dot-home', name: '.grok', env: [] }, + configFormat: 'settings-json', // any value — gate should not check this + artifactLayout: { global: [], local: [] }, + commandStyle: 'slash-hyphen', + hooksSurface: 'none', + sandboxTier: 'none', + supportTier: 2, + }, + }; + const capMap = new Map([['grok', grokCap]]); + assert.doesNotThrow( + () => runConfigFormatParityGate(capMap), + 'Unknown runtimes not in the adapter registry must be excluded from the parity gate', + ); + }); +});