From c29a724bcd773cdb58e42bdb60cac6d6636189bf Mon Sep 17 00:00:00 2001 From: Dante Date: Fri, 9 Oct 2026 15:43:31 -0400 Subject: [PATCH] fix: declare optional cross-target install dependencies Carry forward the final scoped delta of PR #3119 at 60e82f95. Keep required dependency closure and select only modules reachable from successful requests; refresh the unchanged 293-skill registry binding. --- manifests/context-packs/skill-triggers@1.json | 2 +- manifests/install-modules.json | 8 ++ schemas/install-modules.schema.json | 9 ++ scripts/ci/validate-install-manifests.js | 7 ++ scripts/lib/install-manifests.js | 37 +++++++-- tests/lib/install-manifests.test.js | 82 +++++++++++++++++-- 6 files changed, 129 insertions(+), 16 deletions(-) diff --git a/manifests/context-packs/skill-triggers@1.json b/manifests/context-packs/skill-triggers@1.json index b478a347e..78681aa7a 100644 --- a/manifests/context-packs/skill-triggers@1.json +++ b/manifests/context-packs/skill-triggers@1.json @@ -1 +1 @@ -{"coverage":{"skills":293,"withTriggers":32},"generatedAt":"2026-09-24T23:51:22.784Z","id":"skill-triggers@1","model":{"effort":null,"id":"hand-seeded","source":"manual-curation-pending-regeneration"},"registryDigest":"a4abb961bbb5032e40d14e9aeef7e8c652d3517458b7ab72086b768c2e8b5a8b","schemaVersion":1,"triggers":{"skill:api-connector-builder":["add api integration","new provider connector","match existing integration pattern"],"skill:api-design":["rest endpoint design","pagination api","status codes","api versioning","rate limiting api","resource naming","filtering api","api error responses","offset pagination","limit query parameter","pagination defaults"],"skill:backend-patterns":["express api","node backend architecture","nextjs api routes","server side patterns","data access layer","static file server","url path handling","file server"],"skill:browser-qa":["deployed feature test","visual regression screenshots","core web vitals check","axe accessibility audit","ship do not ship","staging verification"],"skill:canary-watch":["post deploy monitoring","smoke test url","production url check","console errors production","sse stream check","after deploy verification"],"skill:code-tour":["onboarding walkthrough","explain subsystem","architecture tour","pr walkthrough","rca tour"],"skill:coding-standards":["code review standards","naming conventions","readability review","immutability conventions","fix naming typo","export naming","consistent exports"],"skill:content-hash-cache-pattern":["cache file processing","content addressed cache","sha256 hash cache"],"skill:database-migrations":["zero downtime migration","schema change production","add column large table","backfill data","expand contract","concurrent index","migration rollback","prisma migration","django migration"],"skill:deployment-patterns":["ci cd setup","dockerize app","health checks","rollback strategy","production readiness","deploy pipeline","containerize application"],"skill:design-system":["design tokens","visual consistency audit","css custom properties","ui audit","design system bootstrap"],"skill:django-patterns":["django orm","drf api","django rest framework","django caching","django signals","django middleware"],"skill:django-security":["django authentication","csrf protection","sql injection prevention","xss prevention","django deployment security","role based access control","authorization middleware","permissions checks"],"skill:docker-patterns":["dockerfile review","docker compose setup","container security","multi service orchestration"],"skill:error-handling":["error types","retry logic","circuit breaker","user facing errors","exception handling patterns","typed errors","error boundaries","go error handling","custom error class","error codes","config validation"],"skill:evm-token-decimals":["token decimals","wei conversion","erc20 balance off","bridge token precision"],"skill:frontend-a11y":["aria attributes","screen reader support","focus management","semantic html","form labeling","keyboard navigation react","a11y lint errors"],"skill:git-workflow":["merge vs rebase","commit conventions","resolve merge conflict","branching strategy","clean up commits","pull request cleanup","git history tidy"],"skill:hexagonal-architecture":["ports and adapters","dependency injection boundaries","decouple domain from io"],"skill:kubernetes-patterns":["kubernetes manifests","kubectl debugging","pod probes","k8s rbac","autoscaling config","configmap secrets"],"skill:orch-fix-defect":["fix a bug","broken behavior","regression fix","reproduce bug","defect repair"],"skill:postgres-patterns":["slow postgres query","query optimization","index design","rls policies","supabase schema","postgres indexing","database performance","schema design postgres","postgres driver","node postgres","query planner"],"skill:python-patterns":["pythonic code","pep 8","type hints python","python code review","idiomatic python"],"skill:python-testing":["pytest fixtures","mocking python","parametrized tests","coverage python","tdd python"],"skill:redis-patterns":["cache aside pattern","distributed lock","redis rate limiting","cache invalidation"],"skill:regex-vs-llm-structured-text":["parse invoice","extract receipt data","text extraction pipeline","parse form fields","cheap document parser","extract table data","parse log lines","parse access logs","common log format","log line parsing"],"skill:rust-patterns":["rust ownership","borrow checker","rust error handling","traits rust","rust concurrency","idiomatic rust"],"skill:search-first":["find existing library","npm package research","before writing custom code","evaluate existing tools","add dependency research"],"skill:security-review":["security audit","authentication review","sanitize user input","secrets handling","payment security checklist","prevent injection attacks","secure api endpoints","authn authz review","vulnerability checklist","input validation security","parameterized queries","sql injection"],"skill:security-scan":["audit claude config","claudemd security","mcp server audit","agentshield scan","hook configuration audit","settings json security"],"skill:tdd-workflow":["write test first","failing test","red green refactor","test driven development","regression test first","write a regression test"],"skill:verification-loop":["pre pr checks","verification report","quality gates","build lint test coverage","before creating a pr"]},"triggersDigest":"25b97a9e06fc336c7cf95ab854ed1a41033a54bcd6e1fb1cf69dc906332462aa"} +{"coverage":{"skills":293,"withTriggers":32},"generatedAt":"2026-09-24T23:51:22.784Z","id":"skill-triggers@1","model":{"effort":null,"id":"hand-seeded","source":"manual-curation-pending-regeneration"},"registryDigest":"1af7cbf1c0d0460643816c368f812f0be54dacdc7c2cc85a15f3a57c38f72400","schemaVersion":1,"triggers":{"skill:api-connector-builder":["add api integration","new provider connector","match existing integration pattern"],"skill:api-design":["rest endpoint design","pagination api","status codes","api versioning","rate limiting api","resource naming","filtering api","api error responses","offset pagination","limit query parameter","pagination defaults"],"skill:backend-patterns":["express api","node backend architecture","nextjs api routes","server side patterns","data access layer","static file server","url path handling","file server"],"skill:browser-qa":["deployed feature test","visual regression screenshots","core web vitals check","axe accessibility audit","ship do not ship","staging verification"],"skill:canary-watch":["post deploy monitoring","smoke test url","production url check","console errors production","sse stream check","after deploy verification"],"skill:code-tour":["onboarding walkthrough","explain subsystem","architecture tour","pr walkthrough","rca tour"],"skill:coding-standards":["code review standards","naming conventions","readability review","immutability conventions","fix naming typo","export naming","consistent exports"],"skill:content-hash-cache-pattern":["cache file processing","content addressed cache","sha256 hash cache"],"skill:database-migrations":["zero downtime migration","schema change production","add column large table","backfill data","expand contract","concurrent index","migration rollback","prisma migration","django migration"],"skill:deployment-patterns":["ci cd setup","dockerize app","health checks","rollback strategy","production readiness","deploy pipeline","containerize application"],"skill:design-system":["design tokens","visual consistency audit","css custom properties","ui audit","design system bootstrap"],"skill:django-patterns":["django orm","drf api","django rest framework","django caching","django signals","django middleware"],"skill:django-security":["django authentication","csrf protection","sql injection prevention","xss prevention","django deployment security","role based access control","authorization middleware","permissions checks"],"skill:docker-patterns":["dockerfile review","docker compose setup","container security","multi service orchestration"],"skill:error-handling":["error types","retry logic","circuit breaker","user facing errors","exception handling patterns","typed errors","error boundaries","go error handling","custom error class","error codes","config validation"],"skill:evm-token-decimals":["token decimals","wei conversion","erc20 balance off","bridge token precision"],"skill:frontend-a11y":["aria attributes","screen reader support","focus management","semantic html","form labeling","keyboard navigation react","a11y lint errors"],"skill:git-workflow":["merge vs rebase","commit conventions","resolve merge conflict","branching strategy","clean up commits","pull request cleanup","git history tidy"],"skill:hexagonal-architecture":["ports and adapters","dependency injection boundaries","decouple domain from io"],"skill:kubernetes-patterns":["kubernetes manifests","kubectl debugging","pod probes","k8s rbac","autoscaling config","configmap secrets"],"skill:orch-fix-defect":["fix a bug","broken behavior","regression fix","reproduce bug","defect repair"],"skill:postgres-patterns":["slow postgres query","query optimization","index design","rls policies","supabase schema","postgres indexing","database performance","schema design postgres","postgres driver","node postgres","query planner"],"skill:python-patterns":["pythonic code","pep 8","type hints python","python code review","idiomatic python"],"skill:python-testing":["pytest fixtures","mocking python","parametrized tests","coverage python","tdd python"],"skill:redis-patterns":["cache aside pattern","distributed lock","redis rate limiting","cache invalidation"],"skill:regex-vs-llm-structured-text":["parse invoice","extract receipt data","text extraction pipeline","parse form fields","cheap document parser","extract table data","parse log lines","parse access logs","common log format","log line parsing"],"skill:rust-patterns":["rust ownership","borrow checker","rust error handling","traits rust","rust concurrency","idiomatic rust"],"skill:search-first":["find existing library","npm package research","before writing custom code","evaluate existing tools","add dependency research"],"skill:security-review":["security audit","authentication review","sanitize user input","secrets handling","payment security checklist","prevent injection attacks","secure api endpoints","authn authz review","vulnerability checklist","input validation security","parameterized queries","sql injection"],"skill:security-scan":["audit claude config","claudemd security","mcp server audit","agentshield scan","hook configuration audit","settings json security"],"skill:tdd-workflow":["write test first","failing test","red green refactor","test driven development","regression test first","write a regression test"],"skill:verification-loop":["pre pr checks","verification report","quality gates","build lint test coverage","before creating a pr"]},"triggersDigest":"25b97a9e06fc336c7cf95ab854ed1a41033a54bcd6e1fb1cf69dc906332462aa"} diff --git a/manifests/install-modules.json b/manifests/install-modules.json index b61d4e20f..9411dc99f 100644 --- a/manifests/install-modules.json +++ b/manifests/install-modules.json @@ -243,6 +243,11 @@ "commands-core", "platform-configs" ], + "optionalDependencies": [ + "rules-core", + "agents-core", + "commands-core" + ], "defaultInstall": false, "cost": "medium", "stability": "stable" @@ -787,6 +792,9 @@ "commands-core", "platform-configs" ], + "optionalDependencies": [ + "commands-core" + ], "defaultInstall": false, "cost": "medium", "stability": "beta" diff --git a/schemas/install-modules.schema.json b/schemas/install-modules.schema.json index 620f616dc..4cb2897be 100644 --- a/schemas/install-modules.schema.json +++ b/schemas/install-modules.schema.json @@ -73,6 +73,15 @@ "pattern": "^[a-z0-9-]+$" } }, + "optionalDependencies": { + "description": "Subset of dependencies that may be omitted when unsupported by the selected target. Supported dependencies are still installed.", + "type": "array", + "uniqueItems": true, + "items": { + "type": "string", + "pattern": "^[a-z0-9-]+$" + } + }, "defaultInstall": { "type": "boolean" }, diff --git a/scripts/ci/validate-install-manifests.js b/scripts/ci/validate-install-manifests.js index aa2a60148..3eac887fb 100644 --- a/scripts/ci/validate-install-manifests.js +++ b/scripts/ci/validate-install-manifests.js @@ -123,6 +123,13 @@ function validateInstallManifests() { } } + for (const dependency of module.optionalDependencies || []) { + if (!module.dependencies.includes(dependency)) { + console.error(`ERROR: Module ${module.id} optional dependency ${dependency} is not a declared dependency`); + hasErrors = true; + } + } + for (const relativePath of module.paths) { const normalizedPath = normalizeRelativePath(relativePath); const absolutePath = path.join(REPO_ROOT, normalizedPath); diff --git a/scripts/lib/install-manifests.js b/scripts/lib/install-manifests.js index 6d0fac5cf..ddef462a1 100644 --- a/scripts/lib/install-manifests.js +++ b/scripts/lib/install-manifests.js @@ -325,6 +325,12 @@ function loadInstallManifests(options = {}) { for (const module of modules) { readModuleTargetsOrThrow(module); + if (module.optionalDependencies !== undefined && ( + !Array.isArray(module.optionalDependencies) + || module.optionalDependencies.some(id => !module.dependencies.includes(id)) + )) { + throw new Error(`Install module ${module.id} optionalDependencies must be an array of declared dependency ids`); + } } const modulesById = new Map(modules.map(module => [module.id, module])); @@ -633,7 +639,7 @@ function resolveInstallPlan(options = {}) { const visitingIds = new Set(); const resolvedIds = new Set(); - function resolveModule(moduleId, dependencyOf, rootRequesterId) { + function resolveModule(moduleId, dependencyOf, optional = false) { const module = manifests.modulesById.get(moduleId); if (!module) { throw new Error(`Unknown install module: ${moduleId}`); @@ -657,8 +663,9 @@ function resolveInstallPlan(options = {}) { if (!supportsTarget) { if (dependencyOf) { - skippedTargetIds.add(rootRequesterId || dependencyOf); - return false; + // Only explicitly optional edges may be omitted for this target. + skippedTargetIds.add(moduleId); + return optional; } skippedTargetIds.add(moduleId); return false; @@ -677,24 +684,36 @@ function resolveInstallPlan(options = {}) { const dependencyResolved = resolveModule( dependencyId, moduleId, - rootRequesterId || moduleId + (module.optionalDependencies || []).includes(dependencyId) ); if (!dependencyResolved) { visitingIds.delete(moduleId); - if (!dependencyOf) { - skippedTargetIds.add(moduleId); - } + skippedTargetIds.add(moduleId); return false; } } visitingIds.delete(moduleId); resolvedIds.add(moduleId); - selectedIds.add(moduleId); return true; } + // Resolution may visit supported dependencies before a later required edge fails. + // Select only the closure of successful requests, so failed requests leave no + // orphan modules while shared dependencies remain available to successful ones. + function selectResolvedModule(moduleId) { + if (!resolvedIds.has(moduleId) || selectedIds.has(moduleId)) { + return; + } + selectedIds.add(moduleId); + for (const dependencyId of manifests.modulesById.get(moduleId).dependencies) { + selectResolvedModule(dependencyId); + } + } + for (const moduleId of effectiveRequestedIds) { - resolveModule(moduleId, null, moduleId); + if (resolveModule(moduleId, null)) { + selectResolvedModule(moduleId); + } } const selectedModules = manifests.modules.filter(module => selectedIds.has(module.id)); diff --git a/tests/lib/install-manifests.test.js b/tests/lib/install-manifests.test.js index cd91e6e3a..b6b92df2a 100644 --- a/tests/lib/install-manifests.test.js +++ b/tests/lib/install-manifests.test.js @@ -404,13 +404,15 @@ function runTests() { )), 'Should install the MLE workflow skill'); })) passed++; else failed++; - if (test('resolves machine-learning component on JoyCode and Qwen targets', () => { - for (const target of ['joycode', 'qwen']) { + if (test('resolves machine-learning component across native and shared harness targets', () => { + for (const target of ['joycode', 'qwen', 'codex', 'opencode']) { const plan = resolveInstallPlan({ includeComponentIds: ['capability:machine-learning'], target, projectRoot: '/workspace/ml-app', homeDir: '/Users/example', + // This checks dependency planning independently of optional build artefacts. + exemptValidationCodes: ['opencode-plugin-not-built'], }); assert.ok(plan.selectedModuleIds.includes('machine-learning'), @@ -698,7 +700,7 @@ function runTests() { ); })) passed++; else failed++; - if (test('skips a requested module when its dependency chain does not support the target', () => { + if (test('skips required unsupported dependencies but permits explicitly optional ones', () => { const repoRoot = createTestRepo(); try { writeJson(path.join(repoRoot, 'manifests', 'install-modules.json'), { @@ -735,9 +737,77 @@ function runTests() { } }); - const plan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); - assert.deepStrictEqual(plan.selectedModuleIds, []); - assert.deepStrictEqual(plan.skippedModuleIds, ['parent']); + const requiredPlan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); + assert.deepStrictEqual(requiredPlan.selectedModuleIds, []); + assert.deepStrictEqual(requiredPlan.skippedModuleIds, ['parent', 'child']); + + const modulesPath = path.join(repoRoot, 'manifests', 'install-modules.json'); + const baseManifest = JSON.parse(fs.readFileSync(modulesPath, 'utf8')); + const optionalModules = baseManifest.modules.map((module, index) => ( + index === 0 ? { ...module, optionalDependencies: ['child'] } : { ...module } + )); + writeJson(modulesPath, { ...baseManifest, modules: optionalModules }); + const optionalPlan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); + assert.deepStrictEqual(optionalPlan.selectedModuleIds, ['parent']); + assert.deepStrictEqual(optionalPlan.skippedModuleIds, ['child']); + + const supportedModules = optionalModules.map((module, index) => ( + index === 1 ? { ...module, targets: ['claude'] } : { ...module } + )); + writeJson(modulesPath, { ...baseManifest, modules: supportedModules }); + const supportedPlan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); + assert.deepStrictEqual(supportedPlan.selectedModuleIds, ['parent', 'child']); + + const transitiveModules = [ + ...supportedModules.map((module, index) => ( + index === 1 ? { ...module, dependencies: ['grandchild'] } : { ...module } + )), + { ...supportedModules[1], id: 'grandchild', targets: ['cursor'], dependencies: [] } + ]; + writeJson(modulesPath, { ...baseManifest, modules: transitiveModules }); + const transitivePlan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); + assert.deepStrictEqual(transitivePlan.selectedModuleIds, []); + assert.deepStrictEqual(transitivePlan.skippedModuleIds, ['parent', 'child', 'grandchild']); + + const invalidModules = baseManifest.modules.map((module, index) => ( + index === 0 ? { ...module, optionalDependencies: ['missing'] } : { ...module } + )); + writeJson(modulesPath, { ...baseManifest, modules: invalidModules }); + assert.throws( + () => resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }), + /optionalDependencies must be an array of declared dependency ids/ + ); + } finally { + cleanupTestRepo(repoRoot); + } + })) passed++; else failed++; + + if (test('retains dependencies only when reachable from a successful request', () => { + const repoRoot = createTestRepo(); + try { + const module = (id, dependencies = [], targets = ['claude']) => ({ + id, kind: 'skills', description: id, paths: [id], targets, + dependencies, defaultInstall: false, cost: 'light', stability: 'stable' + }); + writeJson(path.join(repoRoot, 'manifests', 'install-modules.json'), { + version: 1, + modules: [ + module('parent', ['child', 'unsupported']), + module('child', ['leaf']), + module('leaf'), + module('unsupported', [], ['cursor']), + module('successful', ['child']) + ] + }); + writeJson(path.join(repoRoot, 'manifests', 'install-profiles.json'), { + version: 1, profiles: { core: { description: 'Core', modules: ['parent'] } } + }); + const plan = modules => resolveInstallPlan({ repoRoot, moduleIds: modules, target: 'claude' }); + assert.deepStrictEqual(plan(['parent']).selectedModuleIds, []); + assert.deepStrictEqual(plan(['parent']).skippedModuleIds, ['parent', 'unsupported']); + assert.deepStrictEqual(plan(['parent', 'successful']).selectedModuleIds, ['child', 'leaf', 'successful']); + assert.deepStrictEqual(plan(['successful', 'parent']).selectedModuleIds, ['child', 'leaf', 'successful']); + assert.deepStrictEqual(plan(['parent', 'child']).selectedModuleIds, ['child', 'leaf']); } finally { cleanupTestRepo(repoRoot); }