From 80c00f95ecddff3bb96496fe337373c438b25186 Mon Sep 17 00:00:00 2001 From: Lior Kanfi Date: Tue, 29 Sep 2026 14:16:38 +0300 Subject: [PATCH] feat: selective-install closure via .skills-deps.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit npx/adlc-cli --skill X installs exactly X — borrowers referencing a canonical helper in another skill broke silently under selective install (no installer resolves closures, and no home is install-guaranteed). - events.mjs: fetchSkillsDeps(source) (local/URL/shorthand/bare-dir, same source handling as fetchEventsManifest) + expandSkillSelection() (BFS transitive closure, cycle-safe, fail-open to the plain selection) - commands/skills.mjs: cmdAdd + cmdUpdate --pull expand the filter and pass repeated --skill flags to npx (verified array-capable); own command-generation filter follows the expanded set - Tests: 10 cases (fetch local/missing/malformed, direct/transitive/ cyclic/unknown/missing/* /array selections) - README: --skill flag documents closure expansion --- README.md | 2 +- src/commands/skills.mjs | 32 ++++++++++++-- src/events.mjs | 67 +++++++++++++++++++++++++++++ tests/skill-deps.test.mjs | 90 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 5 deletions(-) create mode 100644 tests/skill-deps.test.mjs diff --git a/README.md b/README.md index 79ae946..60d7eea 100644 --- a/README.md +++ b/README.md @@ -170,7 +170,7 @@ adlc-cli skills add -a | `--no-events` | Skip event config generation | | `--prefix ` | Namespace command filenames (e.g., `adlc.team-setup.md`) | | `--mode ` | `inline` (embeds full skill body) or `wrapper` (references skill by name) | -| `--skill, -s ` | Install/generate for one skill only (use `'*'` for all) | +| `--skill, -s ` | Install/generate for one skill only (use `'*'` for all). Selective installs expand through the source's `.skills-deps.json` closure: borrowers auto-pull their canonical homes (e.g. `--skill architect-implement` also installs `architect-clarify`); repeated `--skill` flags pass through to `npx skills` | | `--copy` | Copy files instead of symlinking (passthrough to `npx skills`) | | `-y, --yes` | Skip confirmation prompts | diff --git a/src/commands/skills.mjs b/src/commands/skills.mjs index a150d31..529e7d9 100644 --- a/src/commands/skills.mjs +++ b/src/commands/skills.mjs @@ -12,6 +12,8 @@ import { installEvents, removeEvents, fetchEventsManifest, + fetchSkillsDeps, + expandSkillSelection, readLocalEventsManifest, resolveEvents, } from "../events.mjs"; @@ -39,6 +41,12 @@ export async function cmdAdd(args, flags) { const skillFilter = flags.skill || null; const npxYes = flags.yes || false; + // Resolve the selective-install closure once (shared across agents). + let selectedSkills = null; + if (skillFilter && skillFilter !== "*") { + selectedSkills = expandSkillSelection(await fetchSkillsDeps(source), skillFilter); + } + for (const agentKey of agents) { const agent = getAgent(agentKey); if (!agent) { @@ -51,9 +59,18 @@ export async function cmdAdd(args, flags) { const npxAgent = resolveNpxAgent(agentKey); const npxArgs = ["skills", "add", source, "-a", npxAgent]; if (isGlobal) npxArgs.push("-g"); + // Selective installs expand through .skills-deps.json closure: borrowers + // pull their canonical homes (npx accepts repeated --skill flags). + // No manifest or no match → the plain selection (fail-open). if (skillFilter) { - if (skillFilter === "*") npxArgs.push("--skill", "*"); - else npxArgs.push("--skill", skillFilter); + if (skillFilter === "*") { + npxArgs.push("--skill", "*"); + } else { + for (const name of selectedSkills) npxArgs.push("--skill", name); + if (selectedSkills.length > 1) { + console.log(`│ --skill expanded via .skills-deps.json: ${selectedSkills.join(", ")}`); + } + } } if (flags.copy) npxArgs.push("--copy"); if (npxYes) npxArgs.push("-y"); @@ -72,7 +89,7 @@ export async function cmdAdd(args, flags) { } const skills = await findInstalledSkills(skillsDir, projectRoot); - const filtered = skillFilter && skillFilter !== "*" ? skills.filter((s) => s.name === skillFilter) : skills; + const filtered = selectedSkills ? skills.filter((s) => selectedSkills.includes(s.name)) : skills; console.log(`│ Found ${filtered.length} skill(s) in ${skillsDir}`); @@ -173,10 +190,17 @@ export async function cmdUpdate(args, flags) { console.error("│ ✗ --pull requires skills-lock.json with source info (run 'add' first)"); return 1; } + // Resolve the selective-install closure once (shared across agents). + let pullSelected = null; + if (flags.skill && flags.skill !== "*") { + pullSelected = expandSkillSelection(await fetchSkillsDeps(pullSource), flags.skill); + } for (const agentKey of agents) { const npxAgent = resolveNpxAgent(agentKey); const npxArgs = ["skills", "add", pullSource, "-a", npxAgent, "--copy"]; - if (flags.skill && flags.skill !== "*") { npxArgs.push("-s", flags.skill); } + if (pullSelected) { + for (const name of pullSelected) npxArgs.push("-s", name); + } npxArgs.push("-y"); console.log(`Pulling latest skills from ${pullSource} for ${agentKey}...`); const result = spawnSync("npx", npxArgs, { stdio: "inherit", cwd: projectRoot }); diff --git a/src/events.mjs b/src/events.mjs index a8a476d..b3c88b3 100644 --- a/src/events.mjs +++ b/src/events.mjs @@ -110,6 +110,73 @@ async function fetchGitHubRaw(ownerRepo, ref, file) { } } +// ── Skill dependency closure (.skills-deps.json) ──────────────────────────── +// Selective installs (`--skill X`) install exactly X. Borrowers that reference +// a canonical helper in another skill declare it in the source repo's +// .skills-deps.json so installers can auto-include the closure: +// { "requires": { "architect-implement": ["architect-clarify"], ... } } +// Unknown skills, missing manifests, and cycles all degrade to the plain +// selection (fail-open); callers always get a usable list or null (all). + +export async function fetchSkillsDeps(source) { + const FILENAME = ".skills-deps.json"; + const parse = (raw) => { + try { + const data = JSON.parse(raw); + if (!data || typeof data !== "object" || Array.isArray(data)) return null; + return data; + } catch { + return null; + } + }; + // Mirror fetchEventsManifest source handling: local path, GitHub + // shorthand, full GitHub URL, bare local directory name. + if (source === "." || source.startsWith("./") || source.startsWith("/") || source.startsWith("../")) { + try { + return parse(readFileSync(join(source, FILENAME), "utf-8")); + } catch { + return null; + } + } + if (/^[\w.-]+\/[\w.-]+$/.test(source)) { + return await fetchGitHubRaw(source, "HEAD", FILENAME); + } + const ghMatch = source.match(/^https?:\/\/github\.com\/([\w.-]+\/[\w.-]+)/); + if (ghMatch) { + return await fetchGitHubRaw(ghMatch[1], "HEAD", FILENAME); + } + try { + if (existsSync(source)) { + return parse(readFileSync(join(source, FILENAME), "utf-8")); + } + } catch { + return null; + } + return null; +} + +export function expandSkillSelection(depsManifest, skillFilter) { + if (!skillFilter || skillFilter === "*") return null; + const selected = Array.isArray(skillFilter) ? [...skillFilter] : [skillFilter]; + const requires = (depsManifest && depsManifest.requires) || {}; + const seen = new Set(); + const out = []; + const queue = [...selected]; + while (queue.length > 0) { + const name = queue.shift(); + if (seen.has(name)) continue; + seen.add(name); + out.push(name); + const deps = requires[name]; + if (Array.isArray(deps)) { + for (const dep of deps) { + if (typeof dep === "string" && !seen.has(dep)) queue.push(dep); + } + } + } + return out; +} + // ── Event resolution ──────────────────────────────────────────────────── export function resolveEvents(manifest, agentConfig) { diff --git a/tests/skill-deps.test.mjs b/tests/skill-deps.test.mjs new file mode 100644 index 0000000..1eb91b9 --- /dev/null +++ b/tests/skill-deps.test.mjs @@ -0,0 +1,90 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +import { fetchSkillsDeps, expandSkillSelection } from "../src/events.mjs"; + +const DEPS = { + version: "1.0.0", + requires: { + "architect-implement": ["architect-clarify"], + "product-implement": ["product-clarify"], + "a": ["b"], + "b": ["c"], + "x": ["y"], + "y": ["x"], + }, +}; + +function fixtureSource(withDeps = true) { + const dir = mkdtempSync(join(tmpdir(), "adlc-deps-")); + if (withDeps) { + writeFileSync(join(dir, ".skills-deps.json"), JSON.stringify(DEPS), "utf-8"); + } + return dir; +} + +describe("Skill dependency closure (.skills-deps.json)", () => { + it("fetchSkillsDeps reads a local source dir", async () => { + const dir = fixtureSource(); + try { + const manifest = await fetchSkillsDeps(dir); + assert.ok(manifest); + assert.deepEqual(manifest.requires["architect-implement"], ["architect-clarify"]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("fetchSkillsDeps returns null when the manifest is absent", async () => { + const dir = fixtureSource(false); + try { + assert.equal(await fetchSkillsDeps(dir), null); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("fetchSkillsDeps returns null for malformed JSON", async () => { + const dir = fixtureSource(false); + try { + writeFileSync(join(dir, ".skills-deps.json"), "{not json", "utf-8"); + assert.equal(await fetchSkillsDeps(dir), null); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("expands a borrower to its canonical home, selected first", () => { + assert.deepEqual(expandSkillSelection(DEPS, "architect-implement"), + ["architect-implement", "architect-clarify"]); + }); + + it("expands transitively (a → b → c)", () => { + assert.deepEqual(expandSkillSelection(DEPS, "a"), ["a", "b", "c"]); + }); + + it("terminates on cycles (x ↔ y)", () => { + assert.deepEqual(expandSkillSelection(DEPS, "x"), ["x", "y"]); + }); + + it("unknown skill stays a single-element selection (fail-open)", () => { + assert.deepEqual(expandSkillSelection(DEPS, "nope"), ["nope"]); + }); + + it("missing manifest keeps the plain selection", () => { + assert.deepEqual(expandSkillSelection(null, "architect-implement"), ["architect-implement"]); + }); + + it("null and wildcard filters mean 'all' (no expansion)", () => { + assert.equal(expandSkillSelection(DEPS, null), null); + assert.equal(expandSkillSelection(DEPS, "*"), null); + }); + + it("array input expands each member once (BFS order)", () => { + assert.deepEqual(expandSkillSelection(DEPS, ["a", "architect-implement"]), + ["a", "architect-implement", "b", "architect-clarify", "c"]); + }); +});