diff --git a/commands/__tests__/skills-surface.test.ts b/commands/__tests__/skills-surface.test.ts index 7571e479..ce134aa2 100644 --- a/commands/__tests__/skills-surface.test.ts +++ b/commands/__tests__/skills-surface.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { execFileSync } from "child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, writeFileSync } from "fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, symlinkSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import { dirname, join } from "path"; import { computeRows, decidePaletteAction, skillsSurface } from "../skills.ts"; @@ -436,3 +436,150 @@ describe("computeRows -- previously-public names absent from skills/, attachment } }); }); + +describe("grouped packs and pack selection", () => { + test("list reads grouped layouts (skills//) and reports the root surface.jsonc", async () => { + const packDir = makePackDir(); + writeFile(join(packDir, "surface.jsonc"), `{ "public": ["subagent-review-loop"] }\n`); + writeFile(join(packDir, "skills", "review", "subagent-review-loop", "SKILL.md"), "---\nname: subagent-review-loop\n---\nbody\n"); + writeFile(join(packDir, "attachments", "forge", "checkout", "SKILL.md"), "---\nname: checkout\n---\nbody\n"); + + await skillsSurface(["list", "--pack", "mattstack", "--pack-dir", packDir]); + + const joined = logs.join("\n"); + expect(joined).toContain("rt skills surface -- pack mattstack"); + expect(joined).toContain("source: surface.jsonc"); + expect(joined).toMatch(/public {3}hand-authored {2}subagent-review-loop/); + expect(joined).toMatch(/internal hand-authored {2}checkout/); + }); + + test("set --public on a grouped internal skill moves it keeping its group, and writes the root surface.jsonc", async () => { + const packDir = makePackDir(); + const { mattstackDir, manifestPath } = makeEngineFixture(); + writeFile(join(packDir, "surface.jsonc"), `{ "public": [] }\n`); + writeFile(join(packDir, "attachments", "forge", "checkout", "SKILL.md"), "---\nname: checkout\n---\nbody\n"); + + await skillsSurface([ + "set", "checkout", "--public", + "--pack", "mattstack", "--pack-dir", packDir, "--mattstack-dir", mattstackDir, "--manifest", manifestPath, + ]); + + expect(existsSync(join(packDir, "skills", "forge", "checkout", "SKILL.md"))).toBe(true); + expect(existsSync(join(packDir, "attachments", "forge", "checkout"))).toBe(false); + expect(existsSync(join(packDir, "pack", "surface.jsonc"))).toBe(false); + expect(readFileSync(join(packDir, "surface.jsonc"), "utf8")).toContain('"checkout"'); + expect(logs.join("\n")).toContain("moved checkout: attachments/forge/ -> skills/forge/"); + }); + + test("no pack named and no tty: clean error that names the flag instead of guessing", async () => { + const { mattstackDir } = makeEngineFixture(); + const { exitCode, errors } = await runExpectingCleanExit(() => skillsSurface(["list", "--mattstack-dir", mattstackDir])); + expect(exitCode).toBe(1); + expect(errors.join("\n")).toContain("--pack"); + }); +}); + +describe("registered roots and name uniqueness", () => { + test("plugin.json skills roots are honored: a skill under a second root counts as registered", async () => { + const packDir = makePackDir(); + writeFile(join(packDir, ".claude-plugin", "plugin.json"), JSON.stringify({ version: "1.0.0", skills: ["./skills/review", "./plugin/skills"] })); + writeFile(join(packDir, "surface.jsonc"), `{ "public": ["editing-skills", "subagent-review-loop"] }\n`); + writeFile(join(packDir, "skills", "review", "subagent-review-loop", "SKILL.md"), "---\nname: subagent-review-loop\n---\nbody\n"); + writeFile(join(packDir, "plugin", "skills", "editing-skills", "SKILL.md"), "---\nname: editing-skills\n---\nbody\n"); + + await skillsSurface(["list", "--pack", "mattstack", "--pack-dir", packDir]); + + const joined = logs.join("\n"); + expect(joined).toMatch(/public {3}hand-authored {2}editing-skills/); + expect(joined).not.toContain("(no files on disk)editing-skills"); + }); + + test("duplicate leaf names across groups are rejected with a clean error naming both dirs", async () => { + const packDir = makePackDir(); + writeFile(join(packDir, "surface.jsonc"), `{ "public": [] }\n`); + writeFile(join(packDir, "attachments", "forge", "sync", "SKILL.md"), "---\nname: sync\n---\nbody\n"); + writeFile(join(packDir, "attachments", "pipeline", "sync", "SKILL.md"), "---\nname: sync\n---\nbody\n"); + + const { exitCode, errors } = await runExpectingCleanExit(() => skillsSurface(["list", "--pack", "mattstack", "--pack-dir", packDir])); + expect(exitCode).toBe(1); + expect(errors.join("\n")).toContain('skill name "sync" appears twice'); + expect(errors.join("\n")).toContain("forge/sync"); + expect(errors.join("\n")).toContain("pipeline/sync"); + }); +}); + +describe("registered roots stay inside the pack", () => { + test("plugin.json skills entries that escape the pack (../ or absolute) are ignored; in-pack roots are kept", async () => { + const packDir = makePackDir(); + const outside = makePackDir(); + writeFile(join(outside, "stray", "SKILL.md"), "---\nname: stray\n---\nbody\n"); + writeFile( + join(packDir, ".claude-plugin", "plugin.json"), + JSON.stringify({ version: "1.0.0", skills: ["./skills", "../" + outside.split("/").pop()!, outside] }), + ); + writeFile(join(packDir, "surface.jsonc"), `{ "public": ["inside"] }\n`); + writeFile(join(packDir, "skills", "inside", "SKILL.md"), "---\nname: inside\n---\nbody\n"); + + await skillsSurface(["list", "--pack", "p", "--pack-dir", packDir]); + + const joined = logs.join("\n"); + expect(joined).toMatch(/public {3}hand-authored {2}inside/); + expect(joined).not.toContain("stray"); + }); +}); + +describe("registered roots are canonicalized", () => { + test("a symlinked root inside the pack that points outside it is ignored", async () => { + const packDir = makePackDir(); + const outside = makePackDir(); + writeFile(join(outside, "stray", "SKILL.md"), "---\nname: stray\n---\nbody\n"); + mkdirSync(join(packDir, "skills"), { recursive: true }); + symlinkSync(outside, join(packDir, "linked-root")); + writeFile(join(packDir, ".claude-plugin", "plugin.json"), JSON.stringify({ version: "1.0.0", skills: ["./skills", "./linked-root"] })); + writeFile(join(packDir, "surface.jsonc"), `{ "public": ["inside"] }\n`); + writeFile(join(packDir, "skills", "inside", "SKILL.md"), "---\nname: inside\n---\nbody\n"); + + await skillsSurface(["list", "--pack", "p", "--pack-dir", packDir]); + + const joined = logs.join("\n"); + expect(joined).toMatch(/public {3}hand-authored {2}inside/); + expect(joined).not.toContain("stray"); + }); +}); + +describe("registered roots may start with dots without escaping", () => { + test("a directory literally named ..skills inside the pack is accepted as a root", async () => { + const packDir = makePackDir(); + writeFile( + join(packDir, ".claude-plugin", "plugin.json"), + JSON.stringify({ version: "1.0.0", skills: ["./skills", "./..skills"] }), + ); + writeFile(join(packDir, "surface.jsonc"), `{ "public": ["inside", "dotty"] }\n`); + writeFile(join(packDir, "skills", "inside", "SKILL.md"), "---\nname: inside\n---\nbody\n"); + writeFile(join(packDir, "..skills", "dotty", "SKILL.md"), "---\nname: dotty\n---\nbody\n"); + + await skillsSurface(["list", "--pack", "p", "--pack-dir", packDir]); + + const joined = logs.join("\n"); + expect(joined).toMatch(/public {3}hand-authored {2}inside/); + expect(joined).toContain("dotty"); + }); +}); + +describe("registered roots must be directories", () => { + test("a plugin.json skills entry pointing at a regular file is ignored", async () => { + const packDir = makePackDir(); + writeFile(join(packDir, "notes.md"), "not a skills root\n"); + writeFile( + join(packDir, ".claude-plugin", "plugin.json"), + JSON.stringify({ version: "1.0.0", skills: ["./skills", "./notes.md", "./missing"] }), + ); + writeFile(join(packDir, "surface.jsonc"), `{ "public": ["inside"] }\n`); + writeFile(join(packDir, "skills", "inside", "SKILL.md"), "---\nname: inside\n---\nbody\n"); + + await skillsSurface(["list", "--pack", "p", "--pack-dir", packDir]); + + const joined = logs.join("\n"); + expect(joined).toMatch(/public {3}hand-authored {2}inside/); + }); +}); diff --git a/commands/skills.ts b/commands/skills.ts index 27dc95d8..16e6f4f8 100644 --- a/commands/skills.ts +++ b/commands/skills.ts @@ -17,12 +17,13 @@ */ import { execFileSync, spawnSync } from "child_process"; -import { chmodSync, copyFileSync, existsSync, mkdirSync, readdirSync, readFileSync, renameSync, rmSync, statSync, writeFileSync } from "fs"; +import { chmodSync, copyFileSync, existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, renameSync, rmSync, statSync, writeFileSync } from "fs"; import { createInterface } from "node:readline"; -import { dirname, join } from "path"; +import { dirname, isAbsolute as isAbsolutePath, join, relative as relativePath, resolve as resolvePath, sep } from "path"; import { resolveFzf } from "../lib/fzf.ts"; import { mattstackHome } from "../lib/rt-paths.ts"; import { compileSkill, HEADER_COMMENT } from "../lib/skills/compile.ts"; +import { discoverPacks, surfaceFileFor, type PackInfo } from "../lib/skills/packs.ts"; import { invocableRoster, loadAttachment, @@ -58,7 +59,7 @@ async function withCleanErrors(fn: () => Promise): Promise { } type Flags = { - team: string; + team: string | null; verbs: string[] | null; manifest: string | null; dryRun: boolean; @@ -68,7 +69,7 @@ type Flags = { function parseFlags(args: string[]): Flags { const verbs: string[] = []; - let team = "claimview"; + let team: string | null = null; let manifest: string | null = null; let dryRun = false; let packDir: string | null = null; @@ -77,6 +78,7 @@ function parseFlags(args: string[]): Flags { for (let i = 0; i < args.length; i++) { const a = args[i]!; switch (a) { + case "--pack": case "--team": team = args[++i] ?? team; break; case "--verb": { const v = args[++i]; if (v) verbs.push(v); break; } case "--manifest": manifest = args[++i] ?? null; break; @@ -95,6 +97,51 @@ function packRootDir(mattstackRoot: string, team: string): string { return join(mattstackRoot, "teams", team, "mattstack", "packs", team); } +type PackTarget = { team: string; packDir: string }; + +/** + * Which pack a command acts on. Explicit --pack-dir wins (tests); a named + * pack resolves through marketplace discovery, falling back to the teams-zone + * path for packs installed by hand; no name at all follows the rt convention + * -- offer a picker over what is actually installed, auto-selecting when only + * one pack exists, and name the choices instead of guessing when there is no tty. + */ +async function resolvePack(flags: { team: string | null; packDir: string | null; mattstackDir: string | null }): Promise { + const mattstackRoot = flags.mattstackDir ?? mattstackHome(); + if (flags.packDir) return { team: flags.team ?? "claimview", packDir: flags.packDir }; + + const packs = flags.mattstackDir ? [] : discoverPacks(); + if (flags.team) { + const pack = packs.find((p) => p.name === flags.team); + if (pack) return { team: pack.name, packDir: pack.dir }; + const legacy = packRootDir(mattstackRoot, flags.team); + if (existsSync(legacy)) return { team: flags.team, packDir: legacy }; + throw new SkillsUsageError( + `no pack named "${flags.team}" (discovered: ${packs.map((p) => p.name).join(", ") || "none"}; checked ${legacy})`, + ); + } + + if (packs.length === 1) return { team: packs[0]!.name, packDir: packs[0]!.dir }; + if (packs.length === 0) throw new SkillsUsageError("no packs discovered (no directory marketplace plugin carries a surface.jsonc); pass --pack "); + + if (!process.stdin.isTTY) { + throw new SkillsUsageError(`which pack? pass --pack (discovered: ${packs.map((p) => p.name).join(", ")})`); + } + const picked = await pickPack(packs); + if (!picked) process.exit(0); + return { team: picked.name, packDir: picked.dir }; +} + +async function pickPack(packs: PackInfo[]): Promise { + const { filterableSelect } = await import("../lib/rt-render.tsx"); + const value = await filterableSelect({ + message: "which pack?", + options: packs.map((p) => ({ value: p.name, label: p.name, hint: `${p.layout} ${p.dir}` })), + stderr: true, + }); + return packs.find((p) => p.name === value) ?? null; +} + function leadingCommentBlock(raw: string): string { const lines: string[] = []; for (const line of raw.split("\n")) { @@ -115,6 +162,93 @@ function listSubdirs(dir: string): string[] { .sort(); } +type SkillEntry = { name: string; group: string | null; dir: string }; + +/** + * Skill dirs live either flat (skills/) or one group deep + * (skills//, the mattstack plugin's layout); a depth-1 dir + * without its own SKILL.md is a group, and its leaves are the skills. + */ +function enumerateSkillEntries(root: string, into: Map = new Map()): Map { + // surface.jsonc is keyed by bare skill name, so two groups carrying the same + // leaf would be indistinguishable to every surface operation -- refuse rather + // than let one silently shadow the other. + const add = (entry: SkillEntry) => { + const clash = into.get(entry.name); + if (clash && clash.dir !== entry.dir) { + throw new SkillsUsageError( + `skill name "${entry.name}" appears twice (${clash.dir} and ${entry.dir}); skill names must be unique within a pack`, + ); + } + into.set(entry.name, entry); + }; + for (const top of listSubdirs(root)) { + const topDir = join(root, top); + if (existsSync(join(topDir, "SKILL.md"))) { + add({ name: top, group: null, dir: topDir }); + continue; + } + let sawLeaf = false; + for (const leaf of listSubdirs(topDir)) { + const leafDir = join(topDir, leaf); + if (!existsSync(join(leafDir, "SKILL.md"))) continue; + sawLeaf = true; + add({ name: leaf, group: top, dir: leafDir }); + } + if (!sawLeaf) add({ name: top, group: null, dir: topDir }); + } + return into; +} + +/** + * A plugin may register more than one skills root (plugin.json `skills`, e.g. + * ["./skills/review", "./plugin/skills"]); the registered surface is the union + * of those roots, defaulting to skills/ when the manifest lists none. + */ +function registeredSkillRoots(packDir: string): string[] { + const manifestPath = join(packDir, ".claude-plugin", "plugin.json"); + if (existsSync(manifestPath)) { + try { + const parsed = JSON.parse(readFileSync(manifestPath, "utf8")) as { skills?: unknown }; + if (Array.isArray(parsed.skills) && parsed.skills.length > 0) { + // A root is only honored inside the pack: a manifest value like "../x" or an + // absolute path would otherwise let the surface verbs enumerate (and move) dirs + // that belong to some other tree. + const canonical = (p: string) => { + try { + return realpathSync(p); + } catch { + return resolvePath(p); + } + }; + const packRoot = canonical(packDir); + const roots = parsed.skills + .filter((s): s is string => typeof s === "string") + .map((s) => canonical(resolvePath(packDir, s))) + .filter((root) => { + const rel = relativePath(packRoot, root); + if (rel === ".." || rel.startsWith(`..${sep}`) || isAbsolutePath(rel)) return false; + try { + return statSync(root).isDirectory(); + } catch { + return false; + } + }); + if (roots.length > 0) return roots; + } + } catch { + // an unreadable manifest falls back to the conventional root below + } + } + return [join(packDir, "skills")]; +} + +function enumerateRegistered(packDir: string): Map { + const entries = new Map(); + for (const root of registeredSkillRoots(packDir)) enumerateSkillEntries(root, entries); + return entries; +} + function listFilesRecursive(dir: string, prefix = ""): string[] { if (!existsSync(dir)) return []; const files: string[] = []; @@ -221,10 +355,10 @@ function computeInternalRoster( const internal = new Set(); if (!surface) return internal; const publicSet = new Set(surface.public); - for (const name of listSubdirs(join(packDir, "skills"))) { + for (const name of enumerateRegistered(packDir).keys()) { if (!publicSet.has(name)) internal.add(`${team}:${name}`); } - for (const name of listSubdirs(join(packDir, "attachments"))) { + for (const name of enumerateSkillEntries(join(packDir, "attachments")).keys()) { if (!publicSet.has(name)) internal.add(`${team}:${name}`); } for (const verb of fullRoster) { @@ -233,18 +367,26 @@ function computeInternalRoster( return internal; } -function resolve(flags: Flags): Resolved { +async function resolve(flags: Flags): Promise { const mattstackRoot = flags.mattstackDir ?? mattstackHome(); - const packDir = flags.packDir ?? packRootDir(mattstackRoot, flags.team); - const manifestPath = flags.manifest ?? findDefaultManifest(mattstackRoot, flags.team); + const { team, packDir } = await resolvePack(flags); const fullRoster = readVerbRoster(packDir); const roster = selectVerbs(fullRoster, flags.verbs); - const bindings = readManifestBindings(manifestPath); - const pluginRoots = flags.mattstackDir ? resolvePluginRootsFromDir(mattstackRoot) : resolvePluginRoots(); - const invocable = invocableRoster(pluginRoots); + // A pack with no verb roster needs no manifest: bindings only feed compile targets. + const manifestPath = fullRoster.length === 0 ? null : (flags.manifest ?? findDefaultManifest(mattstackRoot, team)); + const bindings = manifestPath ? readManifestBindings(manifestPath) : {}; + // No compile targets means nothing needs plugin roots or the invocable roster; + // skipping the `claude plugin list` subprocess keeps rosterless packs usable + // even where the Claude CLI is absent. + const pluginRoots: PluginRoots = fullRoster.length === 0 + ? { byName: {} } + : flags.mattstackDir + ? resolvePluginRootsFromDir(mattstackRoot) + : resolvePluginRoots(); + const invocable = fullRoster.length === 0 ? new Set() : invocableRoster(pluginRoots); const surface = readSurface(packDir); - const internalRoster = computeInternalRoster(flags.team, packDir, surface, fullRoster); + const internalRoster = computeInternalRoster(team, packDir, surface, fullRoster); return { packDir, roster, bindings, pluginRoots, invocable, surface, internalRoster }; } @@ -299,7 +441,7 @@ function writeCompiledVerb(outDir: string, result: CompileResult): void { export async function skillsCompile(args: string[]): Promise { await withCleanErrors(async () => { const flags = parseFlags(args); - const resolved = resolve(flags); + const resolved = await resolve(flags); const publicSet = resolved.surface ? new Set(resolved.surface.public) : null; for (const verb of resolved.roster) { @@ -330,7 +472,7 @@ export async function skillsCompile(args: string[]): Promise { } if (publicSet) { - for (const name of listSubdirs(join(resolved.packDir, "skills"))) { + for (const name of enumerateRegistered(resolved.packDir).keys()) { if (!publicSet.has(name)) { console.log(`misplaced: ${name} (run rt skills surface apply, or move it)`); process.exitCode = 1; @@ -343,7 +485,7 @@ export async function skillsCompile(args: string[]): Promise { export async function skillsCheck(args: string[]): Promise { await withCleanErrors(async () => { const flags = parseFlags(args); - const resolved = resolve(flags); + const resolved = await resolve(flags); // No surface.jsonc means the pack has no public/internal split yet -- every roster verb is public. const publicSet = resolved.surface ? new Set(resolved.surface.public) : null; @@ -394,7 +536,7 @@ export async function skillsCheck(args: string[]): Promise { // ─── rt skills surface -- list / set / apply / fzf palette ──────────────── type SurfaceFlags = { - team: string; + team: string | null; dryRun: boolean; packDir: string | null; mattstackDir: string | null; @@ -408,7 +550,7 @@ function kindLabel(kind: SurfaceRow["kind"]): string { } function parseSurfaceFlags(args: string[]): { flags: SurfaceFlags; rest: string[] } { - let team = "claimview"; + let team: string | null = null; let dryRun = false; let packDir: string | null = null; let mattstackDir: string | null = null; @@ -418,6 +560,7 @@ function parseSurfaceFlags(args: string[]): { flags: SurfaceFlags; rest: string[ for (let i = 0; i < args.length; i++) { const a = args[i]!; switch (a) { + case "--pack": case "--team": team = args[++i] ?? team; break; case "--dry-run": dryRun = true; break; case "--pack-dir": packDir = args[++i] ?? null; break; @@ -430,10 +573,12 @@ function parseSurfaceFlags(args: string[]): { flags: SurfaceFlags; rest: string[ return { flags: { team, dryRun, packDir, mattstackDir, manifest }, rest }; } -function resolveSurfacePaths(flags: SurfaceFlags): { packDir: string } { - const mattstackRoot = flags.mattstackDir ?? mattstackHome(); - const packDir = flags.packDir ?? packRootDir(mattstackRoot, flags.team); - return { packDir }; +/** Pins the pack on the flags so the compile delegation and the printed header name the same pack the user picked. */ +async function resolveSurfacePaths(flags: SurfaceFlags): Promise<{ packDir: string }> { + const target = await resolvePack(flags); + flags.team = target.team; + flags.packDir = target.packDir; + return { packDir: target.packDir }; } function isCompiledDir(dir: string): boolean { @@ -451,10 +596,12 @@ function classify(name: string, dir: string | null, verbNames: Set): "co } function collectRegistry(packDir: string, verbNames: Set) { - const skillsNames = new Set(listSubdirs(join(packDir, "skills"))); - const attachmentNames = new Set(listSubdirs(join(packDir, "attachments"))); + const skillEntries = enumerateRegistered(packDir); + const attachmentEntries = enumerateSkillEntries(join(packDir, "attachments")); + const skillsNames = new Set(skillEntries.keys()); + const attachmentNames = new Set(attachmentEntries.keys()); const allNames = new Set([...skillsNames, ...attachmentNames, ...verbNames]); - return { skillsNames, attachmentNames, allNames }; + return { skillsNames, attachmentNames, allNames, skillEntries, attachmentEntries }; } /** The set `set`'s first use bootstraps surface.jsonc from -- so the first edit is a delta from reality, not a cliff. */ @@ -467,10 +614,11 @@ export function computeRows( verbNames: Set, surface: SurfaceConfig | null, ): { source: string; rows: SurfaceRow[] } { - const { skillsNames, attachmentNames, allNames } = collectRegistry(packDir, verbNames); + const { skillsNames, attachmentNames, allNames, skillEntries, attachmentEntries } = collectRegistry(packDir, verbNames); const publicSet = surface ? new Set(surface.public) : defaultPublicSet(skillsNames, verbNames); - const source = surface - ? "pack/surface.jsonc" + const surfacePath = surfaceFileFor(packDir); + const source = surface && surfacePath + ? surfacePath.slice(packDir.length + 1) : "(no surface.jsonc yet -- inferred from current skills/ + stubs.jsonc placement)"; // A name in surface.jsonc's public list but absent from skills/, attachments/, and @@ -480,11 +628,7 @@ export function computeRows( for (const name of publicSet) names.add(name); const rows = [...names].sort().map((name) => { - const dir = skillsNames.has(name) - ? join(packDir, "skills", name) - : attachmentNames.has(name) - ? join(packDir, "attachments", name) - : null; + const dir = skillEntries.get(name)?.dir ?? attachmentEntries.get(name)?.dir ?? null; return { name, kind: allNames.has(name) ? classify(name, dir, verbNames) : ("missing" as const), @@ -496,14 +640,16 @@ export function computeRows( } function writeSurfaceConfig(packDir: string, publicList: string[]): void { - const path = join(packDir, "pack", "surface.jsonc"); + // Write back wherever the pack already keeps its surface (pack/ for team packs, + // the plugin root for packs without a pack/ dir); new packs get pack/. + const path = surfaceFileFor(packDir) ?? join(packDir, "pack", "surface.jsonc"); mkdirSync(dirname(path), { recursive: true }); const json = JSON.stringify({ public: publicList }, null, 2); writeFileSync(path, `// surface.jsonc -- names this pack's public skills/ directories.\n${json}\n`); } function compileArgs(flags: SurfaceFlags, packDir: string): string[] { - const args = ["--team", flags.team, "--pack-dir", packDir]; + const args = ["--pack", flags.team ?? "claimview", "--pack-dir", packDir]; if (flags.mattstackDir) args.push("--mattstack-dir", flags.mattstackDir); if (flags.manifest) args.push("--manifest", flags.manifest); if (flags.dryRun) args.push("--dry-run"); @@ -519,11 +665,11 @@ function isInsideGitWorkTree(dir: string): boolean { } } -/** git mv keeps rename history for the common case; fixtures (and any non-git pack dir) fall back to a plain rename. */ -function moveHandAuthoredDir(packDir: string, name: string, from: "skills" | "attachments", to: "skills" | "attachments"): string | null { - const fromRel = join(from, name); - const toRel = join(to, name); - mkdirSync(join(packDir, to), { recursive: true }); +/** git mv keeps rename history for the common case; fixtures (and any non-git pack dir) fall back to a plain rename. A grouped skill keeps its group on the other side. */ +function moveHandAuthoredDir(packDir: string, name: string, from: "skills" | "attachments", to: "skills" | "attachments", group: string | null): string | null { + const fromRel = group ? join(from, group, name) : join(from, name); + const toRel = group ? join(to, group, name) : join(to, name); + mkdirSync(dirname(join(packDir, toRel)), { recursive: true }); if (isInsideGitWorkTree(packDir)) { execFileSync("git", ["mv", fromRel, toRel], { cwd: packDir, stdio: "pipe" }); @@ -535,7 +681,7 @@ function moveHandAuthoredDir(packDir: string, name: string, from: "skills" | "at } function printSurfaceRows(flags: SurfaceFlags, source: string, rows: SurfaceRow[]): void { - console.log(`rt skills surface -- team ${flags.team}`); + console.log(`rt skills surface -- pack ${flags.team}`); console.log(`source: ${source}`); for (const row of rows) { console.log(` ${row.status.padEnd(9)}${kindLabel(row.kind).padEnd(15)}${row.name}`); @@ -543,7 +689,7 @@ function printSurfaceRows(flags: SurfaceFlags, source: string, rows: SurfaceRow[ } async function runList(flags: SurfaceFlags): Promise { - const { packDir } = resolveSurfacePaths(flags); + const { packDir } = await resolveSurfacePaths(flags); const verbNames = new Set(readVerbRoster(packDir).map((v) => v.name)); const surface = readSurface(packDir); const { source, rows } = computeRows(packDir, verbNames, surface); @@ -553,10 +699,10 @@ async function runList(flags: SurfaceFlags): Promise { } async function runApply(flags: SurfaceFlags): Promise { - const { packDir } = resolveSurfacePaths(flags); + const { packDir } = await resolveSurfacePaths(flags); const verbNames = new Set(readVerbRoster(packDir).map((v) => v.name)); const surface = readSurface(packDir); - const { skillsNames, attachmentNames } = collectRegistry(packDir, verbNames); + const { skillsNames, attachmentNames, skillEntries, attachmentEntries } = collectRegistry(packDir, verbNames); const publicSet = surface ? new Set(surface.public) : defaultPublicSet(skillsNames, verbNames); const candidates = [...new Set([...skillsNames, ...attachmentNames])].sort(); @@ -564,7 +710,8 @@ async function runApply(flags: SurfaceFlags): Promise { for (const name of candidates) { const currentlyUnderSkills = skillsNames.has(name); - const dir = join(packDir, currentlyUnderSkills ? "skills" : "attachments", name); + const entry = (currentlyUnderSkills ? skillEntries : attachmentEntries).get(name)!; + const dir = entry.dir; if (classify(name, dir, verbNames) === "compiled") continue; // regenerated/removed by the compile step below, never git-mv'd const wantPublic = publicSet.has(name); @@ -579,8 +726,9 @@ async function runApply(flags: SurfaceFlags): Promise { continue; } - const note = moveHandAuthoredDir(packDir, name, from, to); - console.log(`moved ${name}: ${from}/ -> ${to}/${note ? ` (${note})` : ""}`); + const note = moveHandAuthoredDir(packDir, name, from, to, entry.group); + const where = entry.group ? `${entry.group}/` : ""; + console.log(`moved ${name}: ${from}/${where} -> ${to}/${where}${note ? ` (${note})` : ""}`); } if (moved === 0) console.log("no moves needed"); @@ -589,7 +737,7 @@ async function runApply(flags: SurfaceFlags): Promise { } async function runSet(name: string, want: "public" | "internal", flags: SurfaceFlags): Promise { - const { packDir } = resolveSurfacePaths(flags); + const { packDir } = await resolveSurfacePaths(flags); const verbNames = new Set(readVerbRoster(packDir).map((v) => v.name)); const { skillsNames, allNames } = collectRegistry(packDir, verbNames); @@ -665,7 +813,7 @@ function confirmYesNo(promptText: string): Promise { } async function runPalette(flags: SurfaceFlags): Promise { - const { packDir } = resolveSurfacePaths(flags); + const { packDir } = await resolveSurfacePaths(flags); const verbNames = new Set(readVerbRoster(packDir).map((v) => v.name)); const surface = readSurface(packDir); const { skillsNames } = collectRegistry(packDir, verbNames); diff --git a/lib/command-tree-def.ts b/lib/command-tree-def.ts index a2127bfc..9109dae3 100644 --- a/lib/command-tree-def.ts +++ b/lib/command-tree-def.ts @@ -758,9 +758,9 @@ export const TREE: Record = { module: "./commands/skills.ts", fn: "skillsCompile", args: [ - { name: "Team", flag: "--team", type: "text", placeholder: "claimview", hint: "Pack team; default claimview" }, + { name: "Pack", flag: "--pack", type: "text", placeholder: "claimview", hint: "Pack name (--team still accepted); omit to pick from the discovered packs" }, { name: "Verb", flag: "--verb", type: "text", placeholder: "watch-ci", hint: "Compile only this verb (repeatable); omit for every verb in the roster" }, - { name: "Manifest", flag: "--manifest", type: "text", placeholder: "/path/to/skills.jsonc", hint: "Manifest path; omit to auto-find the newest ~/.mattstack/repos/*/skills.jsonc naming this team" }, + { name: "Manifest", flag: "--manifest", type: "text", placeholder: "/path/to/skills.jsonc", hint: "Manifest path; omit to auto-find the newest ~/.mattstack/repos/*/skills.jsonc naming this pack" }, { name: "Dry run", flag: "--dry-run", type: "boolean", default: false, hint: "Print what would be written without touching disk" }, ], }, @@ -769,9 +769,9 @@ export const TREE: Record = { module: "./commands/skills.ts", fn: "skillsCheck", args: [ - { name: "Team", flag: "--team", type: "text", placeholder: "claimview", hint: "Pack team; default claimview" }, + { name: "Pack", flag: "--pack", type: "text", placeholder: "claimview", hint: "Pack name (--team still accepted); omit to pick from the discovered packs" }, { name: "Verb", flag: "--verb", type: "text", placeholder: "watch-ci", hint: "Check only this verb (repeatable); omit for every compiled verb" }, - { name: "Manifest", flag: "--manifest", type: "text", placeholder: "/path/to/skills.jsonc", hint: "Manifest path; omit to auto-find the newest ~/.mattstack/repos/*/skills.jsonc naming this team" }, + { name: "Manifest", flag: "--manifest", type: "text", placeholder: "/path/to/skills.jsonc", hint: "Manifest path; omit to auto-find the newest ~/.mattstack/repos/*/skills.jsonc naming this pack" }, ], }, surface: { @@ -780,7 +780,7 @@ export const TREE: Record = { fn: "skillsSurface", args: [ { name: "Mode", type: "text", placeholder: "list", hint: "list | set --public|--internal | apply; omit for the fzf palette" }, - { name: "Team", flag: "--team", type: "text", placeholder: "claimview", hint: "Pack team; default claimview" }, + { name: "Pack", flag: "--pack", type: "text", placeholder: "claimview", hint: "Pack name (--team still accepted); omit to pick from the discovered packs" }, { name: "Dry run", flag: "--dry-run", type: "boolean", default: false, hint: "apply only: print planned moves without touching disk" }, ], }, diff --git a/lib/skills/__tests__/packs.test.ts b/lib/skills/__tests__/packs.test.ts new file mode 100644 index 00000000..f15ff36d --- /dev/null +++ b/lib/skills/__tests__/packs.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, realpathSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; +import { dirname, join } from "path"; +import { detectLayout, discoverPacks } from "../packs.ts"; + +function writeFile(path: string, content: string): void { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, content); +} + +function tmp(prefix: string): string { + return realpathSync(mkdtempSync(join(tmpdir(), prefix))); +} + +/** A directory marketplace serving two plugins: a flat team pack and a grouped one. */ +function makeMarketplaceFixture() { + const market = tmp("rt-packs-market-"); + const teamPack = join(market, "plugins", "claimview"); + const groupedPack = join(market, "plugins", "mattstack"); + const noSurface = join(market, "plugins", "current-time"); + + writeFile(join(teamPack, "pack", "surface.jsonc"), `// flat pack\n{ "public": ["work"] }\n`); + writeFile(join(teamPack, "skills", "work", "SKILL.md"), "---\nname: work\n---\nbody\n"); + writeFile(join(teamPack, "attachments", "cvi-gates", "SKILL.md"), "---\nname: cvi-gates\n---\nbody\n"); + + writeFile(join(groupedPack, "surface.jsonc"), `{ "public": ["subagent-review-loop"] }\n`); + writeFile(join(groupedPack, "skills", "review", "subagent-review-loop", "SKILL.md"), "---\nname: subagent-review-loop\n---\nbody\n"); + writeFile(join(groupedPack, "attachments", "pipeline", "work", "SKILL.md"), "---\nname: work\n---\nbody\n"); + + writeFile(join(noSurface, "skills", "getting-current-time", "SKILL.md"), "---\nname: getting-current-time\n---\nbody\n"); + + writeFile( + join(market, ".claude-plugin", "marketplace.json"), + JSON.stringify({ + plugins: [ + { name: "claimview", source: "./plugins/claimview" }, + { name: "mattstack", source: "./plugins/mattstack" }, + { name: "current-time", source: "./plugins/current-time" }, + ], + }), + ); + + const settingsDir = tmp("rt-packs-settings-"); + const settingsPath = join(settingsDir, "settings.json"); + writeFile( + settingsPath, + JSON.stringify({ + extraKnownMarketplaces: { + local: { source: { source: "directory", path: market } }, + remote: { source: { source: "github", repo: "someone/marketplace" } }, + }, + }), + ); + + return { market, settingsPath, teamPack, groupedPack }; +} + +describe("discoverPacks", () => { + test("finds directory-marketplace plugins that carry a surface.jsonc, with layout and surface path", () => { + const { settingsPath, teamPack, groupedPack } = makeMarketplaceFixture(); + const packs = discoverPacks({ settingsPath }); + expect(packs.map((p) => p.name)).toEqual(["claimview", "mattstack"]); + + const claimview = packs.find((p) => p.name === "claimview")!; + expect(claimview.dir).toBe(teamPack); + expect(claimview.layout).toBe("flat"); + expect(claimview.surfacePath).toBe(join(teamPack, "pack", "surface.jsonc")); + + const mattstack = packs.find((p) => p.name === "mattstack")!; + expect(mattstack.dir).toBe(groupedPack); + expect(mattstack.layout).toBe("grouped"); + expect(mattstack.surfacePath).toBe(join(groupedPack, "surface.jsonc")); + }); + + test("a plugin without surface.jsonc is not a pack; non-directory marketplaces are ignored", () => { + const { settingsPath } = makeMarketplaceFixture(); + const names = discoverPacks({ settingsPath }).map((p) => p.name); + expect(names).not.toContain("current-time"); + }); + + test("missing settings file yields no packs rather than throwing; extraPackDirs still count", () => { + const extra = tmp("rt-packs-extra-"); + writeFile(join(extra, "pack", "surface.jsonc"), `{ "public": [] }\n`); + const packs = discoverPacks({ settingsPath: join(extra, "nope.json"), extraPackDirs: [{ name: "solo", dir: extra }] }); + expect(packs.map((p) => p.name)).toEqual(["solo"]); + }); +}); + +describe("detectLayout", () => { + test("flat when every skill dir carries SKILL.md at depth one", () => { + const dir = tmp("rt-packs-flat-"); + writeFile(join(dir, "skills", "a", "SKILL.md"), "x"); + writeFile(join(dir, "attachments", "b", "SKILL.md"), "x"); + expect(detectLayout(dir)).toBe("flat"); + }); + + test("grouped when a depth-one dir holds SKILL.md-bearing children", () => { + const dir = tmp("rt-packs-grouped-"); + writeFile(join(dir, "attachments", "forge", "checkout", "SKILL.md"), "x"); + expect(detectLayout(dir)).toBe("grouped"); + }); +}); diff --git a/lib/skills/packs.ts b/lib/skills/packs.ts new file mode 100644 index 00000000..078ccbec --- /dev/null +++ b/lib/skills/packs.ts @@ -0,0 +1,121 @@ +import { existsSync, readFileSync, readdirSync, realpathSync } from "fs"; +import { homedir } from "os"; +import { dirname, isAbsolute, join, resolve } from "path"; +import { stripJsonc } from "./sources.ts"; + +export type PackLayout = "flat" | "grouped"; + +export type PackInfo = { + name: string; + dir: string; + layout: PackLayout; + surfacePath: string; +}; + +export type DiscoverOpts = { + settingsPath?: string; + extraPackDirs?: { name: string; dir: string }[]; +}; + +function claudeSettingsPath(): string { + const configDir = process.env.CLAUDE_CONFIG_DIR ?? join(homedir(), ".claude"); + return join(configDir, "settings.json"); +} + +function readJsonc(path: string): unknown { + return JSON.parse(stripJsonc(readFileSync(path, "utf8"))); +} + +export function surfaceFileFor(dir: string): string | null { + const candidates = [join(dir, "pack", "surface.jsonc"), join(dir, "surface.jsonc")]; + return candidates.find((candidate) => existsSync(candidate)) ?? null; +} + +function hasNestedSkill(root: string): boolean { + if (!existsSync(root)) return false; + for (const group of readdirSync(root, { withFileTypes: true })) { + if (!group.isDirectory()) continue; + const groupDir = join(root, group.name); + if (existsSync(join(groupDir, "SKILL.md"))) continue; + for (const leaf of readdirSync(groupDir, { withFileTypes: true })) { + if (leaf.isDirectory() && existsSync(join(groupDir, leaf.name, "SKILL.md"))) return true; + } + } + return false; +} + +export function detectLayout(dir: string): PackLayout { + return hasNestedSkill(join(dir, "skills")) || hasNestedSkill(join(dir, "attachments")) + ? "grouped" + : "flat"; +} + +function packFromDir(name: string, dir: string): PackInfo | null { + let real: string; + try { + real = realpathSync(dir); + } catch { + return null; + } + const surfacePath = surfaceFileFor(real); + if (!surfacePath) return null; + return { name, dir: real, layout: detectLayout(real), surfacePath }; +} + +type MarketplaceEntry = { name?: string; source?: string | { source?: string; path?: string } }; + +/** + * A pack is any plugin served from a directory marketplace that carries a + * surface.jsonc -- discovery reads what is actually installed instead of a + * hardcoded pack list, so a new team pack appears the moment its marketplace + * is registered. + */ +export function discoverPacks(opts: DiscoverOpts = {}): PackInfo[] { + const found = new Map(); + const settingsPath = opts.settingsPath ?? claudeSettingsPath(); + + if (existsSync(settingsPath)) { + let settings: { extraKnownMarketplaces?: Record } = {}; + try { + settings = readJsonc(settingsPath) as typeof settings; + } catch { + settings = {}; + } + for (const marketplace of Object.values(settings.extraKnownMarketplaces ?? {})) { + const src = marketplace.source; + if (!src || src.source !== "directory" || !src.path) continue; + const marketDir = src.path.startsWith("~") ? join(homedir(), src.path.slice(1)) : src.path; + const manifest = join(marketDir, ".claude-plugin", "marketplace.json"); + if (!existsSync(manifest)) continue; + let entries: MarketplaceEntry[] = []; + try { + entries = ((readJsonc(manifest) as { plugins?: MarketplaceEntry[] }).plugins) ?? []; + } catch { + continue; + } + for (const entry of entries) { + if (!entry.name) continue; + const rel = typeof entry.source === "string" ? entry.source : entry.source?.path; + if (!rel) continue; + const pluginDir = isAbsolute(rel) ? rel : resolve(marketDir, rel); + const pack = packFromDir(entry.name, pluginDir); + if (pack && !found.has(pack.name)) found.set(pack.name, pack); + } + } + } + + for (const extra of opts.extraPackDirs ?? []) { + const pack = packFromDir(extra.name, extra.dir); + if (pack && !found.has(pack.name)) found.set(pack.name, pack); + } + + return [...found.values()].sort((a, b) => a.name.localeCompare(b.name)); +} + +export function packDirOf(pack: PackInfo): string { + return pack.dir; +} + +export function surfaceConfigDir(pack: PackInfo): string { + return dirname(pack.surfacePath); +} diff --git a/lib/skills/sources.ts b/lib/skills/sources.ts index e238ff7f..66c4280d 100644 --- a/lib/skills/sources.ts +++ b/lib/skills/sources.ts @@ -302,6 +302,9 @@ function assertSafeVerbName(name: string, stubsPath: string): void { export function readVerbRoster(packDir: string): VerbDef[] { const stubsPath = join(packDir, "pack", "stubs.jsonc"); + // A pack with no verb roster (the mattstack plugin) has no compile targets; + // its surface is still manageable, so absence is an empty roster, not an error. + if (!existsSync(stubsPath)) return []; const parsed = JSON.parse(stripJsonc(readFileSync(stubsPath, "utf8"))) as { verbs?: Record; };