diff --git a/manifests/install-modules.json b/manifests/install-modules.json index 25caf50a2..97761d2c6 100644 --- a/manifests/install-modules.json +++ b/manifests/install-modules.json @@ -266,6 +266,11 @@ "commands-core", "platform-configs" ], + "optionalDependencies": [ + "rules-core", + "agents-core", + "commands-core" + ], "defaultInstall": false, "cost": "medium", "stability": "stable" @@ -857,6 +862,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 cbdc0eb5c..617a5359d 100644 --- a/schemas/install-modules.schema.json +++ b/schemas/install-modules.schema.json @@ -74,6 +74,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 36d7c8834..abfe093cb 100644 --- a/scripts/lib/install-manifests.js +++ b/scripts/lib/install-manifests.js @@ -327,6 +327,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])); @@ -635,7 +641,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}`); @@ -659,8 +665,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; @@ -679,24 +686,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 5aff37acb..6e3dc8cbf 100644 --- a/tests/lib/install-manifests.test.js +++ b/tests/lib/install-manifests.test.js @@ -415,13 +415,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'), @@ -709,7 +711,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'), { @@ -746,9 +748,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); }