From e5a5201dbdf2fa74a73136170b1f30d81d3cd83b Mon Sep 17 00:00:00 2001 From: Dante Date: Fri, 2 Oct 2026 10:40:47 +0800 Subject: [PATCH] fix(install): require explicit optional cross-target dependencies --- manifests/install-modules.json | 8 +++++ schemas/install-modules.schema.json | 9 ++++++ scripts/ci/validate-install-manifests.js | 7 +++++ scripts/lib/install-manifests.js | 22 +++++++------ tests/lib/install-manifests.test.js | 39 ++++++++++++++++++++---- 5 files changed, 69 insertions(+), 16 deletions(-) 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 fa366333b..85cd4448b 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) { + function resolveModule(moduleId, dependencyOf, optional = false) { const module = manifests.modulesById.get(moduleId); if (!module) { throw new Error(`Unknown install module: ${moduleId}`); @@ -657,12 +663,9 @@ function resolveInstallPlan(options = {}) { if (!supportsTarget) { if (dependencyOf) { - // A module that explicitly supports the selected target remains useful - // when one of its cross-harness dependencies does not. Treat that - // dependency as inapplicable for this target instead of dropping the - // target-capable root module with it. + // Only explicitly optional edges may be omitted for this target. skippedTargetIds.add(moduleId); - return true; + return optional; } skippedTargetIds.add(moduleId); return false; @@ -680,13 +683,12 @@ function resolveInstallPlan(options = {}) { for (const dependencyId of module.dependencies) { const dependencyResolved = resolveModule( dependencyId, - moduleId + moduleId, + (module.optionalDependencies || []).includes(dependencyId) ); if (!dependencyResolved) { visitingIds.delete(moduleId); - if (!dependencyOf) { - skippedTargetIds.add(moduleId); - } + skippedTargetIds.add(moduleId); return false; } } diff --git a/tests/lib/install-manifests.test.js b/tests/lib/install-manifests.test.js index 438c44952..97bdb6177 100644 --- a/tests/lib/install-manifests.test.js +++ b/tests/lib/install-manifests.test.js @@ -404,8 +404,8 @@ 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, @@ -698,7 +698,7 @@ function runTests() { ); })) passed++; else failed++; - if (test('keeps a requested target-capable module when a dependency 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 +735,36 @@ function runTests() { } }); - const plan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); - assert.deepStrictEqual(plan.selectedModuleIds, ['parent']); - assert.deepStrictEqual(plan.skippedModuleIds, ['child']); + 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 manifest = JSON.parse(fs.readFileSync(modulesPath, 'utf8')); + manifest.modules[0].optionalDependencies = ['child']; + writeJson(modulesPath, manifest); + const optionalPlan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); + assert.deepStrictEqual(optionalPlan.selectedModuleIds, ['parent']); + assert.deepStrictEqual(optionalPlan.skippedModuleIds, ['child']); + + manifest.modules[1].targets = ['claude']; + writeJson(modulesPath, manifest); + const supportedPlan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); + assert.deepStrictEqual(supportedPlan.selectedModuleIds, ['parent', 'child']); + + manifest.modules[1].dependencies = ['grandchild']; + manifest.modules.push({ ...manifest.modules[1], id: 'grandchild', targets: ['cursor'], dependencies: [] }); + writeJson(modulesPath, manifest); + const transitivePlan = resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }); + assert.deepStrictEqual(transitivePlan.selectedModuleIds, []); + assert.deepStrictEqual(transitivePlan.skippedModuleIds, ['parent', 'child', 'grandchild']); + + manifest.modules[0].optionalDependencies = ['missing']; + writeJson(modulesPath, manifest); + assert.throws( + () => resolveInstallPlan({ repoRoot, profileId: 'core', target: 'claude' }), + /optionalDependencies must be an array of declared dependency ids/ + ); } finally { cleanupTestRepo(repoRoot); }