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.
This commit is contained in:
Dante
2026-10-09 15:43:31 -04:00
committed by haelyra
parent ef648e0189
commit c29a724bcd
6 changed files with 129 additions and 16 deletions
File diff suppressed because one or more lines are too long
+8
View File
@@ -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"
+9
View File
@@ -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"
},
+7
View File
@@ -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);
+28 -9
View File
@@ -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));
+76 -6
View File
@@ -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);
}