diff --git a/.changeset/nimble-lemurs-wander.md b/.changeset/nimble-lemurs-wander.md new file mode 100644 index 000000000..77c50d273 --- /dev/null +++ b/.changeset/nimble-lemurs-wander.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4691 +--- +**`gsd capability install` no longer rejects capabilities whose `requires` names a first-party or already-installed capability** — install-time validation was seeded with a candidate-only map, making any non-empty `requires` unsatisfiable; it now sees the full merged registry (first-party + committed overlays + candidate), so requires resolution, cycle and tier checks, and central config-key exclusivity all actually run at install, agreeing with load time. (#3929) diff --git a/src/capability-loader.cts b/src/capability-loader.cts index 7b469bab3..10afcfccf 100644 --- a/src/capability-loader.cts +++ b/src/capability-loader.cts @@ -461,6 +461,139 @@ function ledgerOverlayIds(ledger: LedgerModule, rootDir: string): { return { pending, committed }; } +// ─── #3929: install-time cross-capability validation seed ────────────────── + +/** First-party capabilities from the frozen registry, keyed by id. */ +function firstPartyCaps(): Record { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const base = require('./capability-registry.cjs') as { capabilities?: Record }; + return base.capabilities ?? {}; +} + +/** + * Central config-schema validKeys for the ownership-exclusivity check — the + * same source `loadRegistry`'s generator path reads, with the same + * missing-schema fallback (empty set). Memoized per process: the manifest is + * static for the lifetime of the runtime. + */ +let _centralKeysMemo: Set | null = null; +function centralConfigKeys(): Set { + if (_centralKeysMemo === null) { + // Same generator seam loadRegistry uses (including the test override), so + // install-time and load-time central keys cannot diverge when the + // generator is stubbed. + if (_generatorOverride) { + _centralKeysMemo = _generatorOverride.loadCentralConfigKeys(); + } else { + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const mod = require('../../../scripts/gen-capability-registry.cjs') as { + loadCentralConfigKeys: () => Set; + }; + _centralKeysMemo = mod.loadCentralConfigKeys(); + } catch { + _centralKeysMemo = new Set(); + } + } + } + return _centralKeysMemo; +} + +/** + * #3929: the validation seed `capability-source.stageValidated` runs the + * cross-capability suite against. Built the way `loadRegistry` builds its + * accepted map: first-party registry capabilities, then each COMMITTED overlay + * of the TARGET (global) install scope accepted INCREMENTALLY — structural + * validation, engines.gsd against the running host, then the FULL + * cross-capability suite; an overlay joins only if the suite stays clean + * after adding it (load's invariant: first-party alone is clean, so any new + * error is that overlay's fault — skip it, never fail the seed). The seed is + * therefore CLEAN BY CONSTRUCTION: pre-existing junk in the install scope + * (colliding entries, shape-invalid manifests, engines-incompatible bundles) + * is skipped exactly as load skips it and can never fail or skew a + * candidate's install decision — a repo-planted ledger cannot veto installs + * (#1459 CB-3). + * + * Scope: only the install TARGET scope is walked — `stageValidated` promotes + * into `${gsdHome}/.gsd/capabilities`, so target scope == global. + * Project-scope overlays are deliberately NOT seeded (they additionally + * require user consent to activate at load; the issue asks for overlays "in + * the target scope"). The candidate is added by the caller LAST, mirroring + * `acceptedMap.set(id, cap)`. + * + * Exported (not inlined in the installer) so the semantics this reuses — + * `overlayRoots`, `ledgerOverlayIds`, the shared bounded manifest reader, and + * the incremental-accept rules — have exactly one owner and cannot drift from + * the loader's load-time rules. + */ +export function crossValidationSeed( + cwd: string, + gsdHome: string, + hostVersion: string, + validator: ValidatorModule, + semver: SemverModule, +): { capMap: Map; centralKeys: Set } { + const fp = firstPartyCaps(); + const capMap = new Map(Object.entries(fp)); + const centralKeys = centralConfigKeys(); + // eslint-disable-next-line @typescript-eslint/no-require-imports + const ledger: LedgerModule = require('./capability-ledger.cjs') as LedgerModule; + for (const root of overlayRoots(cwd, gsdHome)) { + if (root.scope !== 'global') continue; // seed the TARGET scope only + const { committed } = ledgerOverlayIds(ledger, root.dir); + for (const id of committed) { + // First-party always wins (CONTEXT.md capability-loader entry): an + // overlay claiming a first-party id — or a reserved first-party prefix + // — is rejected at load, so it must not join the validation set. + if (Object.prototype.hasOwnProperty.call(fp, id)) continue; + if (RESERVED_ID_PREFIX.test(id)) continue; + let cap: unknown; + try { + const manifestPath = path.join(root.dir, id, 'capability.json'); + const raw = ledger.readSmallRegularFile(manifestPath, MANIFEST_MAX_BYTES); + if (raw === null) continue; // missing/non-regular/oversized — skip fail-closed + cap = JSON.parse(raw); + } catch { + continue; // unreadable overlay — skip (same rule as load) + } + // Same per-overlay pre-filters load applies before the cross suite: + // structural validity, then engines.gsd against the running host. + // validateCapability itself is not total over malformed array entries + // (#1461 finding 1) — a throw skips the overlay, as load skips it. + try { + if (validator.validateCapability(cap, id).length > 0) continue; + } catch { + continue; + } + const engines = (cap as Record)['engines']; + if (engines && typeof engines === 'object' && !Array.isArray(engines)) { + const range = (engines as Record)['gsd']; + if (typeof range === 'string' && range && !semver.semverSatisfies(hostVersion, range)) continue; + } + // Incremental accept: the overlay joins only if the FULL suite stays + // clean after adding it; any error is that overlay's fault — skip it. + capMap.set(id, cap); + let errs: string[] = []; + try { + errs = [ + ...validator.validateConsumesGlobal(capMap), + ...validator.validateCrossCapability(capMap, centralKeys), + ]; + } catch { + // The cross validators are not total over arbitrary shapes (#1461 + // finding 1 — e.g. the duplicate-producer invariant throws). A + // throwing overlay must be REMOVED here, exactly as load's + // acceptedMap.delete(id) does — retaining it would leave poisoning + // content in the seed and attribute pre-existing junk to the + // candidate. + capMap.delete(id); + } + if (errs.length > 0) capMap.delete(id); + } + } + return { capMap, centralKeys }; +} + /** Shallow-attach overlay diagnostics WITHOUT mutating the frozen registry module. */ function withOverlayMeta(reg: Registry, meta: OverlayMeta): Registry { return Object.assign({}, reg, { _overlay: meta }); @@ -902,4 +1035,4 @@ export function loadRegistry(options: LoadRegistryOptions = {}): Registry { } // readHostVersion is exported for the #1920 regression (VERSION-first host-version resolution). -module.exports = { loadRegistry, readHostVersion, _setValidatorForTest, _setGeneratorForTest }; +module.exports = { loadRegistry, readHostVersion, crossValidationSeed, _setValidatorForTest, _setGeneratorForTest }; diff --git a/src/capability-source.cts b/src/capability-source.cts index 55166fc34..8508ade9f 100644 --- a/src/capability-source.cts +++ b/src/capability-source.cts @@ -833,8 +833,39 @@ function stageValidated(opts: { } // Cross-capability validations (contract, consumes, cross-capability). - const capMap = new Map([[id, cap]]); - const centralKeys = new Set(); + // + // #3929: seed the validation set the way the loader builds its accepted + // map — frozen first-party registry, then each committed overlay of the + // TARGET (global) install scope accepted incrementally (structural + + // engines + full-suite-clean), then the candidate LAST (mirroring + // `acceptedMap.set(id, cap)`). The singleton seed + // `new Map([[id, cap]])` this replaced made every non-empty `requires` + // unsatisfiable (membership is checked against the map) and left cycles, + // tier-monotone and central config-key exclusivity vacuous at install. + // The seed builder lives in capability-loader so the overlay semantics + // have one owner and are clean by construction — pre-existing junk in the + // install scope is skipped, never attributed to the candidate. No swallow: + // if the loader cannot build the seed the install fails loudly — silently + // degrading to the singleton map would re-hide #3929. + /* eslint-disable @typescript-eslint/no-require-imports */ + const seedLoader = require('./capability-loader.cjs') as { + crossValidationSeed: ( + cwd: string, + gsdHome: string, + hostVersion: string, + validator: ValidatorModule, + semver: { semverSatisfies: (version: unknown, range: unknown) => boolean }, + ) => { capMap: Map; centralKeys: Set }; + }; + /* eslint-enable @typescript-eslint/no-require-imports */ + const { capMap, centralKeys } = seedLoader.crossValidationSeed( + process.cwd(), + gsdHome, + hostVersion, + capValidator, + semverMod, + ); + capMap.set(id, cap); const crossErrs = [ ...capValidator.validateAgainstContract(cap, id), ...capValidator.validateConsumesGlobal(capMap), diff --git a/tests/capability-cli.test.cjs b/tests/capability-cli.test.cjs index fca7b498b..a42f47079 100644 --- a/tests/capability-cli.test.cjs +++ b/tests/capability-cli.test.cjs @@ -60,7 +60,7 @@ function makeCwdWithStrict(strictValue) { * (usable directly as an install ). Declarative by default; pass `hooks` * (with materialized scripts) to make it an executable surface requiring consent. */ -function writeCapSource(id, { version = '1.0.0', hooks = [], engines, mcp } = {}) { +function writeCapSource(id, { version = '1.0.0', hooks = [], engines, mcp, requires, config, tier = 'standard' } = {}) { const src = tmpDir(`cap-cli-src-${id}-`); const cap = { id, @@ -68,13 +68,13 @@ function writeCapSource(id, { version = '1.0.0', hooks = [], engines, mcp } = {} version, title: id, description: 'test capability', - tier: 'standard', - requires: [], + tier, + requires: requires ?? [], runtimeCompat: { supported: ['*'], unsupported: [] }, skills: [], agents: [], hooks, - config: {}, + config: config ?? {}, steps: [], contributions: [], gates: [], @@ -1181,3 +1181,148 @@ describe('issue-2322: capability set --runtime materializes an installed third-p assert.ok(staged.includes('workflows/example.md'), 'the rewrite must preserve the referenced path suffix, not corrupt the body'); }); }); + +// ─── #3929 — install-time cross-capability validation sees the merged registry ── + +describe('#3929: install-time validation is seeded with first-party + installed overlays + candidate', () => { + test('requires on a first-party capability resolves at install time', () => { + const home = tmpDir('cap-cli-3929-fp-'); + // tier: 'full' — the issue's own repro manifest; tdd is a full-tier + // capability and the (now-live) tier-monotone check forbids standard + // requiring full, so the candidate must be full to require it at all. + const src = writeCapSource('needs-fp', { requires: ['tdd'], tier: 'full' }); + const r = runGsdTools(['capability', 'install', src, '--scope', 'global', '--raw'], makeCwd(), scopeEnv(home)); + assert.equal(r.success, true, `install must succeed: tdd is first-party — got: ${r.error || r.output}`); + const o = parse(r.output); + assert.equal(o.status, 'installed'); + assert.ok(readLedgerEntry(home, 'needs-fp'), 'ledger entry recorded'); + }); + + test('requires on a genuinely missing capability still refuses', () => { + const home = tmpDir('cap-cli-3929-missing-'); + const src = writeCapSource('needs-missing', { requires: ['no-such-capability-xyz'] }); + const r = runGsdTools(['capability', 'install', src, '--scope', 'global'], makeCwd(), scopeEnv(home)); + assert.equal(r.success, false, 'a genuinely missing requires id must still be refused'); + assert.match(`${r.error}\n${r.output}`, /no-such-capability-xyz/); + assert.equal(readLedgerEntry(home, 'needs-missing'), null, 'no ledger entry'); + }); + + test('requires on an installed overlay capability resolves at install time', () => { + const home = tmpDir('cap-cli-3929-overlay-'); + const srcA = writeCapSource('overlay-a'); + const first = runGsdTools(['capability', 'install', srcA, '--scope', 'global', '--raw'], makeCwd(), scopeEnv(home)); + assert.equal(first.success, true, `seed install failed: ${first.error || first.output}`); + + const srcB = writeCapSource('overlay-b', { requires: ['overlay-a'] }); + const r = runGsdTools(['capability', 'install', srcB, '--scope', 'global', '--raw'], makeCwd(), scopeEnv(home)); + assert.equal(r.success, true, `install must succeed: overlay-a is a committed installed overlay — got: ${r.error || r.output}`); + assert.ok(readLedgerEntry(home, 'overlay-b'), 'ledger entry recorded'); + }); + + test('a pending (uncommitted) overlay does not satisfy requires', () => { + const home = tmpDir('cap-cli-3929-pending-'); + const srcA = writeCapSource('pending-a'); + const first = runGsdTools(['capability', 'install', srcA, '--scope', 'global', '--raw'], makeCwd(), scopeEnv(home)); + assert.equal(first.success, true, `seed install failed: ${first.error || first.output}`); + + // Flip cap-a's committed ledger entry to an in-flight _pending intent — + // exactly the state reconciliation defers on (the loader excludes it from + // the accepted map, so install-time validation must exclude it too). + // The shape must satisfy isValidLedgerEntry's _pending rules + // (kind 'install'|'upgrade', backupName string|null, string[] sharedFiles) + // or the shared ledger reader refuses the whole file as corrupt. + const lp = ledgerPath(home); + const ledger = JSON.parse(fs.readFileSync(lp, 'utf8')); + ledger.entries['pending-a']['_pending'] = { kind: 'upgrade', backupName: null, sharedFiles: [] }; + fs.writeFileSync(lp, JSON.stringify(ledger, null, 2)); + + const srcB = writeCapSource('pending-b', { requires: ['pending-a'] }); + const r = runGsdTools(['capability', 'install', srcB, '--scope', 'global'], makeCwd(), scopeEnv(home)); + assert.equal(r.success, false, 'a pending overlay must not satisfy requires (committed-only rule)'); + assert.match(`${r.error}\n${r.output}`, /pending-a/); + }); + + test('install-time config-key exclusivity against the central schema actually runs', () => { + const home = tmpDir('cap-cli-3929-central-'); + // `mode` is a real central-schema validKey; declaring it federated must be + // refused at install once centralKeys is seeded (empty Set pre-fix). + const src = writeCapSource('claims-central', { + config: { mode: { type: 'string', default: 'standard', description: 'collides with the central schema' } }, + }); + const r = runGsdTools(['capability', 'install', src, '--scope', 'global'], makeCwd(), scopeEnv(home)); + assert.equal(r.success, false, 'declaring a central config key must refuse at install'); + assert.match(`${r.error}\n${r.output}`, /central config-schema/); + }); +}); + +describe('#3929: a throwing (duplicate-producer) overlay is skipped, never attributed to the candidate', () => { + // Hand-plant a committed overlay (manifest + structurally-valid ledger + // entry) WITHOUT the CLI — needed because post-fix installs refuse a + // duplicate-producer candidate up front, so the poisoned pair can only + // exist from pre-fix history (the exact scenario the seed must survive). + function plantOverlay(home, id, cap) { + const dir = capDir(home, id); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'capability.json'), JSON.stringify(cap, null, 2)); + const lp = ledgerPath(home); + let ledger; + try { + ledger = JSON.parse(fs.readFileSync(lp, 'utf8')); + } catch { + ledger = { schema_version: 1, updatedAt: new Date().toISOString(), entries: {} }; + } + ledger.entries = ledger.entries || {}; + ledger.entries[id] = { + id, + version: cap.version || '1.0.0', + source: 'test-plant', + integrity: '', + files: ['capability.json'], + sharedEdits: [], + }; + fs.writeFileSync(lp, JSON.stringify(ledger, null, 2)); + } + + test('pre-existing duplicate-producer overlays do not fail a later install', () => { + const home = tmpDir('cap-cli-3929-poison-'); + const cwd = makeCwd(); + + // poison-a must be seeded (and accepted) BEFORE poison-b trips the + // duplicate-producer throw during the seed's own suite run. + const srcA = writeCapSource('poison-a'); + const first = runGsdTools(['capability', 'install', srcA, '--scope', 'global', '--raw'], cwd, scopeEnv(home)); + assert.equal(first.success, true, `seed install failed: ${first.error || first.output}`); + + const poisonedStep = { + point: 'plan:pre', + ref: { skill: 'poison-skill' }, + produces: ['SHARED-ARTIFACT.md'], + consumes: [], + onError: 'skip', + }; + plantOverlay(home, 'poison-b', { + id: 'poison-b', role: 'feature', version: '1.0.0', title: 'poison-b', + description: 'duplicate producer of SHARED-ARTIFACT.md', tier: 'standard', + requires: [], skills: ['poison-skill'], agents: [], config: {}, + runtimeCompat: { supported: ['*'], unsupported: [] }, + hooks: [], steps: [poisonedStep], contributions: [], gates: [], + }); + // poison-a is re-planted with the SAME producer so the pair trips + // validateConsumesGlobal's duplicate-producer throw when both are seeded. + plantOverlay(home, 'poison-a', { + id: 'poison-a', role: 'feature', version: '1.0.0', title: 'poison-a', + description: 'duplicate producer of SHARED-ARTIFACT.md', tier: 'standard', + requires: [], skills: ['poison-skill'], agents: [], config: {}, + runtimeCompat: { supported: ['*'], unsupported: [] }, + hooks: [], steps: [poisonedStep], contributions: [], gates: [], + }); + + // The candidate only installs if the throwing overlay was DROPPED from + // the seed — if it were retained, the whole-map suite would throw during + // the candidate's own validation and this install would fail. + const srcC = writeCapSource('poison-c', { requires: ['poison-a'] }); + const r = runGsdTools(['capability', 'install', srcC, '--scope', 'global', '--raw'], cwd, scopeEnv(home)); + assert.equal(r.success, true, `install must succeed despite the poisoned overlay: ${r.error || r.output}`); + assert.ok(readLedgerEntry(home, 'poison-c'), 'candidate ledger entry recorded'); + }); +});