From d801d6c4c8a984d6a95033c49714210018d3d9a8 Mon Sep 17 00:00:00 2001 From: "jason.woltje" Date: Fri, 17 Jul 2026 23:45:35 +0000 Subject: [PATCH] feat(mosaic): add secure skill registration CLI (#826) --- README.md | 5 +- docs/SITEMAP.md | 5 + docs/guides/dev-guide.md | 36 +- docs/guides/user-guide.md | 33 ++ docs/scratchpads/824-mosaic-skill-cli.md | 112 +++++ packages/mosaic/framework/defaults/README.md | 12 +- .../tools/_scripts/mosaic-sync-skills | 22 +- .../tools/_scripts/mosaic-sync-skills.ps1 | 19 +- packages/mosaic/package.json | 1 + packages/mosaic/src/cli.ts | 19 +- packages/mosaic/src/commands/skill.spec.ts | 421 ++++++++++++++++++ packages/mosaic/src/commands/skill.ts | 419 +++++++++++++++++ .../src/runtime/update-checker.reseed.spec.ts | 45 ++ packages/mosaic/src/runtime/update-checker.ts | 29 +- .../mosaic/src/stages/finalize-skills.spec.ts | 36 +- packages/mosaic/src/stages/finalize.ts | 38 +- packages/mosaic/vitest.config.ts | 11 + pnpm-lock.yaml | 3 + 18 files changed, 1240 insertions(+), 26 deletions(-) create mode 100644 docs/scratchpads/824-mosaic-skill-cli.md create mode 100644 packages/mosaic/src/commands/skill.spec.ts create mode 100644 packages/mosaic/src/commands/skill.ts diff --git a/README.md b/README.md index 8651460..f095ee6 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,10 @@ mosaic config path # Print config file path ```bash mosaic doctor # Health audit — detect drift and missing files mosaic sync # Sync skills from canonical source -mosaic update # Check for and install CLI updates +mosaic skill list # Audit Claude skill registrations and conflicts +mosaic skill register # Register one canonical skill with Claude Code +mosaic skill unregister # Remove one Mosaic-owned Claude link +mosaic update # Update CLI/framework and auto-register canonical skills mosaic wizard # Full guided setup wizard mosaic bootstrap # Bootstrap a repo with Mosaic standards mosaic coord init # Initialize a new orchestration mission diff --git a/docs/SITEMAP.md b/docs/SITEMAP.md index 2aa6db2..f53a46e 100644 --- a/docs/SITEMAP.md +++ b/docs/SITEMAP.md @@ -1,5 +1,10 @@ # Documentation Sitemap +## CLI and skill management + +- [Skill registration user guide](guides/user-guide.md#claude-code-skill-registration) — register, unregister, list statuses, automatic install/update reconciliation, and Claude reload behavior. +- [Skill bridge developer guide](guides/dev-guide.md#claude-code-skill-bridge) — path-validation, ownership, clobber-protection, install/update wiring, tests, and Pi/Codex scope notes. + ## Fleet configuration management - [Generated environment boundary](fleet/reference/generated-env-boundary.md) — roster-derived launch projection, strict local data, legacy quarantine, and downstream interface evidence. diff --git a/docs/guides/dev-guide.md b/docs/guides/dev-guide.md index b468115..14e26f6 100644 --- a/docs/guides/dev-guide.md +++ b/docs/guides/dev-guide.md @@ -8,8 +8,9 @@ 4. [Adding New Agent Tools](#adding-new-agent-tools) 5. [Adding New MCP Tools](#adding-new-mcp-tools) 6. [Database Schema and Migrations](#database-schema-and-migrations) -7. [API Endpoint Reference](#api-endpoint-reference) -8. [Local Fleet Canary](./fleet-local-canary.md) +7. [Claude Code Skill Bridge](#claude-code-skill-bridge) +8. [API Endpoint Reference](#api-endpoint-reference) +9. [Local Fleet Canary](./fleet-local-canary.md) --- @@ -353,6 +354,37 @@ defined there. --- +## Claude Code Skill Bridge + +The framework's canonical skill root is `~/.config/mosaic/skills/`; Claude Code +requires registrations under `~/.claude/skills/`. The implementation in +`packages/mosaic/src/commands/skill.ts` owns only direct-child symlinks whose +resolved target remains inside the canonical root. + +Security invariants: + +1. Validate the user-supplied name before filesystem access against + `[A-Za-z0-9][A-Za-z0-9._-]*`. Separators, control characters, whitespace, + `..`, absolute paths, and leading `-` are invalid; filesystem-derived invalid + names are escaped before terminal output. +2. Never replace a real file, directory, foreign symlink, or live misdirected + symlink in the Claude skill directory. +3. Repair a dangling link only when its lexical target is inside the canonical + Mosaic skills root. +4. Unregister only a symlink pointing inside that root. +5. Enumerate canonical directories at runtime; never hardcode framework skill + names. + +`finalizeStage` reconciles after wizard/framework synchronization, and +`runFrameworkReseed` reconciles after the sync-only `mosaic update` path. A +foreign conflict is reported but does not prevent unrelated canonical skills +from registering. Filesystem tests use injected temporary roots in +`skill.spec.ts`, `finalize-skills.spec.ts`, and `update-checker.reseed.spec.ts`. + +M1 intentionally manages Claude Code only. Pi's Mosaic launcher can discover the +canonical root directly. Codex still relies on the existing full skill-sync +linker and needs separate parity analysis before this lifecycle API is extended. + ## API Endpoint Reference All endpoints are served by the gateway at `http://localhost:14242` by default. diff --git a/docs/guides/user-guide.md b/docs/guides/user-guide.md index 370bb8a..18746e8 100644 --- a/docs/guides/user-guide.md +++ b/docs/guides/user-guide.md @@ -309,6 +309,39 @@ mosaic quality-rails --- +### Claude Code Skill Registration + +Mosaic stores canonical skills under `~/.config/mosaic/skills/`. Claude Code scans +`~/.claude/skills/`, so Mosaic maintains one symlink per skill between those +directories. + +```bash +mosaic skill list +mosaic skill register +mosaic skill unregister +``` + +- `register` is idempotent and repairs a dangling Mosaic-owned link. Names use + the safe grammar `[A-Za-z0-9][A-Za-z0-9._-]*`; files, directories, foreign + symlinks, path traversal, absolute paths, and names beginning with `-` are + refused. +- `unregister` is idempotent when no entry exists. It removes only symlinks that + point inside `~/.config/mosaic/skills/`; foreign entries are never removed. +- `list` reports `registered`, `unregistered`, `dangling`, `foreign`, + `foreign-dangling`, or `misdirected` for each canonical or Claude entry. + +Install, wizard finalization, and `mosaic update` framework re-seeding reconcile +every canonical skill automatically. A skill directory added after initial +setup therefore receives its Claude bridge without a per-skill code change or +manual `ln -s`. If Claude Code is already running, use `/reload-skills` or start +a new session after registration so its in-process skill registry rescans. + +This command group is Claude-only in M1. Pi can consume Mosaic's canonical skill +root through its Mosaic launcher configuration and does not need this Claude +bridge. Codex has a separate link path managed by the legacy full skill-sync +script; equivalent lifecycle management remains follow-up scope and is not +changed here. + ## Sub-package Commands Each Mosaic sub-package exposes its full API surface through the `mosaic` CLI. diff --git a/docs/scratchpads/824-mosaic-skill-cli.md b/docs/scratchpads/824-mosaic-skill-cli.md new file mode 100644 index 0000000..47be56b --- /dev/null +++ b/docs/scratchpads/824-mosaic-skill-cli.md @@ -0,0 +1,112 @@ +# Issue #824 — Mosaic skill CLI and Claude bridge auto-sync + +## Objective + +Deliver `mosaic skill register|unregister|list` plus install/upgrade reconciliation of every canonical `~/.config/mosaic/skills/*` entry into `~/.claude/skills/`, without clobbering runtime-owned files or directories. + +## Scope and constraints + +- Issue: mosaicstack/stack#824 +- Branch: `feat/824-mosaic-skill-cli` +- M1 runtime: Claude Code only. +- Pi/Codex parity is documentation-only; no non-Claude bridge implementation. +- Do not author the downstream `mosaic-context-refresh` skill. +- Workers do not modify `docs/TASKS.md`, merge, close #824, or touch `main`. +- TDD is mandatory and red-first; filesystem tests use temporary directories only. +- Budget: no explicit token cap supplied; use a focused single-worker implementation with no new dependencies. + +## Requirements mapping + +1. Register creates the canonical Claude symlink and is idempotent. +2. Names are untrusted: reject empty/escaping/absolute/separator/`..`/leading-dash names before filesystem mutation, with clear CLI stderr and nonzero status. +3. Register repairs only Mosaic-owned dangling symlinks and refuses foreign files, directories, and symlinks. +4. Unregister removes only symlinks pointing inside the canonical Mosaic skills root and is idempotent when absent. +5. List reports registered, dangling, foreign, and canonical-but-unregistered skills. +6. Install and upgrade generically reconcile all canonical skills after framework sync/re-seed, continuing past foreign conflicts without clobbering them. +7. User/developer documentation describes commands, status meanings, security boundaries, and Claude-only M1 scope. + +## Plan + +1. Add co-located failing Vitest coverage for all filesystem behaviors and auto-sync. +2. Run the focused spec and record the expected RED failure. +3. Commit the red contract as `test(#824): ...`. +4. Implement the skill bridge and Commander command registration. +5. Wire reconciliation into wizard finalize and `mosaic update` re-seed, preserving non-clobber behavior. +6. Update canonical docs and sitemap if navigation changes. +7. Run focused tests, package tests, typecheck, lint, and formatting. +8. Commit implementation/docs as `feat(#824): ...`, queue-guard, push, open PR with `Closes #824.`, fire completion event, and notify the coordinator. + +## Progress + +- 2026-07-17: Loaded mission/delivery/TDD/documentation rails, issue #824, active mission state, and relevant installer/update paths. +- 2026-07-17: Confirmed `mosaic update` invokes `framework/install.sh` with `MOSAIC_SYNC_ONLY=1`; that path exits before existing post-install skill linking, leaving newly present canonical skills unregistered. +- 2026-07-17: Coordinator addendum classified the user-supplied skill name and runtime symlink target as a path-traversal/symlink-injection surface. Expanded the initial red contract to reject traversal before mutation, preserve every foreign entry, and unregister Mosaic-owned links only. +- 2026-07-17: Implemented the Commander command group and secure generic bridge; wired wizard finalize and successful framework re-seed reconciliation; updated user/developer/installed/root docs and sitemap. +- 2026-07-17: Focused, package-wide, repository baseline, temp-home situational, and independent review gates completed. Ready for scoped feature commit, queue guard, push, and PR handoff. + +## Tests and evidence + +### TDD evidence + +- RED environment attempt: `pnpm --filter @mosaicstack/mosaic exec vitest run src/commands/skill.spec.ts` initially could not locate Vitest because this fresh worktree had no dependencies. +- Dependency setup: `pnpm install --frozen-lockfile --store-dir /home/hermes/.local/share/pnpm/store` succeeded. The explicit store was required because machine pnpm config incorrectly resolves the default store under `/root`. +- RED behavior: focused Vitest failed with `Failed to load url ./skill.js ... Does the file exist?`, proving the bridge API was absent. +- RED integration: finalize/update specs failed because no Claude links or `skillSync` result existed. +- RED symlink injection: symlinked Claude/canonical root tests failed because the initial implementation followed ancestor links. +- GREEN after review remediation: `skill.spec.ts` 36/36, `finalize-skills.spec.ts` 6/6, and `update-checker.reseed.spec.ts` 30/30. + +### Baseline gates + +- `pnpm --filter '@mosaicstack/mosaic...' run build` — pass (fresh-worktree dependency outputs built). +- `pnpm --filter @mosaicstack/mosaic run typecheck` — pass. +- `pnpm --filter @mosaicstack/mosaic run lint` — pass. +- `pnpm --filter @mosaicstack/mosaic test` — pass: 69 files, 1,325 Vitest tests plus framework shell suite. +- `pnpm typecheck` — pass: 42/42 Turbo tasks. +- `pnpm lint` — pass: 23/23 Turbo tasks. +- `pnpm format:check` — pass. + +### Situational evidence + +A built-CLI temp-home smoke test (no real `~/.claude` or Mosaic config touched) proved: + +- register creates the exact link and a second run reports `already registered`; +- list reports registered and unregistered canonical skills; +- `../../etc` exits 1 with `Invalid skill name` and creates no escaped path; +- unregister removes the managed link and a second run reports `already unregistered`; +- a fake successful framework re-seed generically registered both `added-after-setup` and `second-skill` from runtime directory enumeration. + +### Review evidence + +- Initial uncommitted Codex code/security review described name validation/clobber protection as strong; its only finding was the harness-owned, unrelated `.mosaic/orchestrator/session.lock`, which is excluded from all commits and the PR. +- Exact branch review then identified two remediations: preserve successful framework re-seed status when bridge-wide reconciliation fails, and reject/escape control-character names to prevent terminal/log injection. +- Both findings were reproduced red-first and remediated. A subsequent exact review identified one finalize failure-isolation blocker; a root-wide bridge error now warns and allows wizard doctor/summary/next-steps completion, with a red-first regression. +- All remediations passed the full package and repository gates. Final exact-head review is rerun after amending the feature commit. + +### Acceptance mapping + +| Acceptance criterion | Evidence | +| --- | --- | +| register/unregister/list, idempotent | `skill.spec.ts` and built-CLI temp-home smoke | +| traversal/symlink-injection protection | invalid-name matrix, foreign file/dir/link tests, symlinked-root tests | +| list flags dangling and foreign entries | deterministic list status test | +| install and upgrade auto-sync every canonical directory | finalize + framework re-seed integration specs; two-skill built-module smoke | +| newly added skill becomes discoverable without manual link | `added-after-setup` auto-sync creates exact Claude link; Claude can rescan with `/reload-skills` or a new session | +| Pi/Codex parity captured as scope note | user guide, developer guide, installed framework README | +| documentation gate | root README, user guide, developer guide, framework README, sitemap | + +## Risks + +- Symlink replacement uses `lstat` semantics so dangling links are detectable without following them. +- Link ownership is determined lexically against the canonical skills root, and existing symlink ancestors in either managed root are rejected before mutation. +- Auto-sync continues across per-skill conflicts while never deleting real files/directories or foreign symlinks. +- Claude Code discovers filesystem skills at session launch/reload boundaries; bridge creation makes a later `/reload-skills` or new session able to discover the skill, but cannot mutate an already-cached in-process registry by itself. +- Pi does not need this Claude bridge because its Mosaic launcher can consume the canonical root. Codex lifecycle parity remains explicitly deferred. +- No deployment surface is affected. + +## PR #826 review remediation + +- 2026-07-17: Exact-head RoR requested changes for two ownership bugs: installer pruning deleted foreign-name links under `MOSAIC_HOME` outside canonical skills, and unregister deleted a same-root link targeting a different skill. It also requested trailing-dot rejection and executable coverage support. +- RED evidence: focused regression run failed 4 tests: register/unregister accepted `safe.`, misdirected unregister did not throw, and the install linker deleted the foreign-name link. +- GREEN evidence: `skill.spec.ts` passes 43/43, including live and dangling foreign-name links in a temp HOME/MOSAIC_HOME and the misdirected unregister invariant. +- Coverage: `vitest run src/commands/skill.spec.ts --coverage` passes configured 85% thresholds for `skill.ts`: 91.05% statements/lines, 86.27% branches, 95.23% functions. +- Full gates: package build passed; package tests passed 69 files / 1,332 tests plus framework shell suite; repository typecheck 42/42, lint 23/23, and format check passed. diff --git a/packages/mosaic/framework/defaults/README.md b/packages/mosaic/framework/defaults/README.md index ecffb6a..e96dfb7 100644 --- a/packages/mosaic/framework/defaults/README.md +++ b/packages/mosaic/framework/defaults/README.md @@ -181,13 +181,19 @@ The installer rejects unrecognized flags or positional arguments before making c ## Universal Skills -The installer syncs skills from `mosaic/agent-skills` into `~/.config/mosaic/skills/`, then links each skill into runtime directories. +The installer syncs skills from `mosaic/agent-skills` into `~/.config/mosaic/skills/`. Install, wizard finalization, and `mosaic update` automatically reconcile every canonical skill into Claude Code's `~/.claude/skills/` directory. ```bash -mosaic sync # Full sync (clone + link) -~/.config/mosaic/bin/mosaic-sync-skills --link-only # Re-link only +mosaic sync # Full canonical catalog sync +mosaic skill list # Show registered, missing, dangling, and foreign entries +mosaic skill register # Register or repair one canonical Claude link +mosaic skill unregister # Remove one Mosaic-owned Claude link ``` +Skill names are direct children using `[A-Za-z0-9][A-Za-z0-9._-]*`, not paths. Registration rejects traversal/control characters and never replaces foreign files, directories, or symlinks; unregister removes only links that point inside the canonical Mosaic skill root. After registering during a running Claude Code session, use `/reload-skills` or start a new session. + +M1 lifecycle management targets Claude Code. Pi can discover the canonical Mosaic root through its launcher configuration. Codex parity remains follow-up scope and continues to use the existing full skill-sync linker. + ## Health Audit ```bash diff --git a/packages/mosaic/framework/tools/_scripts/mosaic-sync-skills b/packages/mosaic/framework/tools/_scripts/mosaic-sync-skills index cf6af7c..c4c3c20 100755 --- a/packages/mosaic/framework/tools/_scripts/mosaic-sync-skills +++ b/packages/mosaic/framework/tools/_scripts/mosaic-sync-skills @@ -161,6 +161,7 @@ link_targets=( ) canonical_real="$(readlink -f "$MOSAIC_SKILLS_DIR")" +local_real="$(readlink -f "$MOSAIC_LOCAL_SKILLS_DIR")" # Build an associative array from the colon-separated whitelist for O(1) lookup. # When MOSAIC_INSTALL_SKILLS is empty, all skills are allowed. @@ -203,7 +204,14 @@ link_skill_into_target() { link_path="$target_dir/$name" if [[ -L "$link_path" ]]; then - ln -sfn "$skill_path" "$link_path" + local raw_target resolved_target + raw_target="$(readlink "$link_path")" + resolved_target="$(node -e 'const p=require("node:path"); process.stdout.write(p.resolve(p.dirname(process.argv[1]), process.argv[2]));' "$link_path" "$raw_target")" + if [[ "$resolved_target" == "$canonical_real/"* || "$resolved_target" == "$local_real/"* ]]; then + ln -sfn "$skill_path" "$link_path" + else + echo "[mosaic-skills] Preserve foreign runtime symlink: $link_path" + fi return fi @@ -234,14 +242,10 @@ prune_stale_links_in_target() { continue fi - resolved="$(readlink -f "$link_path" 2>/dev/null || true)" - if [[ -z "$resolved" ]]; then - rm -f "$link_path" - echo "[mosaic-skills] Removed stale broken skill link: $link_path" - continue - fi - - if [[ "$resolved" == "$MOSAIC_HOME/"* ]]; then + # -m resolves lexical dangling targets too. If resolution fails, ownership + # is unproven and the link must be preserved. + resolved="$(readlink -m "$link_path" 2>/dev/null || true)" + if [[ -n "$resolved" && "$resolved" == "$canonical_real/"* ]]; then rm -f "$link_path" echo "[mosaic-skills] Removed stale retired skill link: $link_path" fi diff --git a/packages/mosaic/framework/tools/_scripts/mosaic-sync-skills.ps1 b/packages/mosaic/framework/tools/_scripts/mosaic-sync-skills.ps1 index c2a8fd5..8bf30c5 100644 --- a/packages/mosaic/framework/tools/_scripts/mosaic-sync-skills.ps1 +++ b/packages/mosaic/framework/tools/_scripts/mosaic-sync-skills.ps1 @@ -79,9 +79,26 @@ function Link-SkillIntoTarget { $linkPath = Join-Path $TargetDir $name - # Already a junction/symlink — recreate + # Recreate only Mosaic-owned junctions/symlinks. Foreign reparse points are + # runtime-owned and must never be clobbered by install/upgrade auto-sync. $existing = Get-Item $linkPath -Force -ErrorAction SilentlyContinue if ($existing -and ($existing.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { + $rawTarget = @($existing.Target)[0] + $candidate = if ([System.IO.Path]::IsPathRooted($rawTarget)) { + $rawTarget + } + else { + Join-Path (Split-Path $linkPath -Parent) $rawTarget + } + $resolvedTarget = [System.IO.Path]::GetFullPath($candidate) + $canonicalRoot = [System.IO.Path]::GetFullPath($MosaicSkillsDir).TrimEnd('\') + '\' + $localRoot = [System.IO.Path]::GetFullPath($MosaicLocalSkillsDir).TrimEnd('\') + '\' + $owned = $resolvedTarget.StartsWith($canonicalRoot, [System.StringComparison]::OrdinalIgnoreCase) -or + $resolvedTarget.StartsWith($localRoot, [System.StringComparison]::OrdinalIgnoreCase) + if (-not $owned) { + Write-Host "[mosaic-skills] Preserve foreign runtime symlink: $linkPath" + return + } Remove-Item $linkPath -Force } elseif ($existing) { diff --git a/packages/mosaic/package.json b/packages/mosaic/package.json index 0de2b3d..5b045d1 100644 --- a/packages/mosaic/package.json +++ b/packages/mosaic/package.json @@ -53,6 +53,7 @@ }, "devDependencies": { "@types/node": "^22.0.0", + "@vitest/coverage-v8": "^2.0.0", "@types/react": "^18.3.0", "tsx": "^4.0.0", "typescript": "^5.8.0", diff --git a/packages/mosaic/src/cli.ts b/packages/mosaic/src/cli.ts index 73fd6a5..4bccd37 100644 --- a/packages/mosaic/src/cli.ts +++ b/packages/mosaic/src/cli.ts @@ -18,6 +18,7 @@ import { registerFleetCommand } from './commands/fleet.js'; import { registerMissionCommand } from './commands/mission.js'; import { registerUninstallCommand } from './commands/uninstall.js'; import { registerRestoreCommand } from './commands/restore.js'; +import { registerSkillCommand } from './commands/skill.js'; // prdy is registered via launch.ts import { registerLaunchCommands } from './commands/launch.js'; import { registerAuthCommand } from './commands/auth.js'; @@ -67,7 +68,7 @@ Command Groups: Runtime: tui, login, sessions Gateway: gateway - Framework: agent, bootstrap, coord, doctor, fleet, init, launch, mission, prdy, seq, sync, upgrade, wizard, yolo + Framework: agent, bootstrap, coord, doctor, fleet, init, launch, mission, prdy, seq, skill, sync, upgrade, wizard, yolo Platform: update Runtimes: claude, codex, opencode, pi `, @@ -411,6 +412,10 @@ registerUninstallCommand(program); registerRestoreCommand(program); +// ─── skill ─────────────────────────────────────────────────────────────────── + +registerSkillCommand(program); + // ─── telemetry ─────────────────────────────────────────────────────────────── registerTelemetryCommand(program); @@ -471,6 +476,18 @@ program return; } console.log('✔ Framework re-seeded.'); + if (reseed.skillSyncError) { + console.error(` ⚠ Claude skill reconciliation skipped: ${reseed.skillSyncError}`); + } + const skillConflicts = reseed.skillSync?.conflicts ?? []; + const skillChanges = + (reseed.skillSync?.registered.length ?? 0) + (reseed.skillSync?.repaired.length ?? 0); + if (skillChanges > 0) { + console.log(`✔ Registered ${skillChanges.toString()} Mosaic skill(s) with Claude Code.`); + } + for (const conflict of skillConflicts) { + console.error(` ⚠ Skill registration skipped for ${conflict.name}: ${conflict.reason}`); + } // Propagate shipped systemd unit fixes to the ACTIVE units (re-seed only // touches ~/.config/mosaic/systemd/user; systemd runs ~/.config/systemd/user). const units = refreshActiveFleetUnits(); diff --git a/packages/mosaic/src/commands/skill.spec.ts b/packages/mosaic/src/commands/skill.spec.ts new file mode 100644 index 0000000..cc768e6 --- /dev/null +++ b/packages/mosaic/src/commands/skill.spec.ts @@ -0,0 +1,421 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Command } from 'commander'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { + existsSync, + lstatSync, + mkdirSync, + mkdtempSync, + readlinkSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + listSkills, + registerSkill, + registerSkillCommand, + syncClaudeSkills, + unregisterSkill, + type SkillPaths, +} from './skill.js'; + +const LEGACY_SYNC_SCRIPT = fileURLToPath( + new URL('../../framework/tools/_scripts/mosaic-sync-skills', import.meta.url), +); + +describe('Claude skill bridge', () => { + let root: string; + let paths: SkillPaths; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'mosaic-skill-cli-')); + paths = { + mosaicSkillsDir: join(root, '.config', 'mosaic', 'skills'), + claudeSkillsDir: join(root, '.claude', 'skills'), + }; + mkdirSync(paths.mosaicSkillsDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + function createSkill(name: string): string { + const skillDir = join(paths.mosaicSkillsDir, name); + mkdirSync(skillDir, { recursive: true }); + writeFileSync(join(skillDir, 'SKILL.md'), `# ${name}\n`); + return skillDir; + } + + function expectCorrectLink(name: string): void { + const linkPath = join(paths.claudeSkillsDir, name); + expect(lstatSync(linkPath).isSymbolicLink()).toBe(true); + expect(readlinkSync(linkPath)).toBe(join(paths.mosaicSkillsDir, name)); + } + + describe('name validation', () => { + const invalidNames = [ + '../../etc', + '/abs/path', + 'a/b', + String.raw`a\b`, + '-rf', + '..', + 'safe.', + 'space name', + 'line\nbreak', + 'escape\u001B[31m', + ]; + + for (const name of invalidNames) { + it(`rejects ${JSON.stringify(name)} before register can escape its roots`, () => { + expect(() => registerSkill(name, paths)).toThrow(/invalid skill name/i); + expect(existsSync(paths.claudeSkillsDir)).toBe(false); + }); + + it(`rejects ${JSON.stringify(name)} before unregister can escape its roots`, () => { + expect(() => unregisterSkill(name, paths)).toThrow(/invalid skill name/i); + expect(existsSync(paths.claudeSkillsDir)).toBe(false); + }); + } + }); + + describe('CLI validation errors', () => { + let previousExitCode: number | string | null | undefined; + + beforeEach(() => { + previousExitCode = process.exitCode; + process.exitCode = undefined; + }); + + afterEach(() => { + process.exitCode = previousExitCode; + }); + + it.each(['register', 'unregister'])( + 'reports invalid %s names on stderr and sets a nonzero exit status', + async (subcommand) => { + const error = vi.spyOn(console, 'error').mockImplementation(() => undefined); + const program = new Command().exitOverride(); + registerSkillCommand(program, paths); + + await program.parseAsync(['node', 'mosaic', 'skill', subcommand, '../../etc']); + + expect(error).toHaveBeenCalledWith(expect.stringMatching(/invalid skill name/i)); + expect(process.exitCode).toBe(1); + expect(existsSync(paths.claudeSkillsDir)).toBe(false); + error.mockRestore(); + }, + ); + }); + + describe('CLI status output', () => { + let previousExitCode: number | string | null | undefined; + + beforeEach(() => { + previousExitCode = process.exitCode; + process.exitCode = undefined; + }); + + afterEach(() => { + process.exitCode = previousExitCode; + }); + + async function run(...args: string[]): Promise { + const program = new Command().exitOverride(); + registerSkillCommand(program, paths); + await program.parseAsync(['node', 'mosaic', 'skill', ...args]); + } + + it('reports register repair/idempotency and unregister idempotency statuses', async () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + createSkill('status-skill'); + + await run('register', 'status-skill'); + await run('register', 'status-skill'); + rmSync(join(paths.claudeSkillsDir, 'status-skill')); + symlinkSync( + join(paths.mosaicSkillsDir, 'retired'), + join(paths.claudeSkillsDir, 'status-skill'), + ); + await run('register', 'status-skill'); + await run('unregister', 'status-skill'); + await run('unregister', 'status-skill'); + + expect(log.mock.calls.flat()).toEqual([ + 'status-skill: registered', + 'status-skill: already registered', + 'status-skill: repaired dangling registration', + 'status-skill: unregistered', + 'status-skill: already unregistered', + ]); + log.mockRestore(); + }); + + it('reports empty and populated skill lists', async () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + + await run('list'); + createSkill('listed'); + await run('list'); + + expect(log).toHaveBeenCalledWith('No Mosaic or Claude Code skills found.'); + expect(log).toHaveBeenCalledWith(expect.stringMatching(/^unregistered\s+listed$/)); + log.mockRestore(); + }); + }); + + describe('registerSkill', () => { + it('creates the exact canonical symlink and is idempotent', () => { + createSkill('new-skill'); + + expect(registerSkill('new-skill', paths).status).toBe('registered'); + expectCorrectLink('new-skill'); + + expect(registerSkill('new-skill', paths).status).toBe('already-registered'); + expectCorrectLink('new-skill'); + }); + + it('repairs a dangling Mosaic-owned symlink', () => { + createSkill('new-skill'); + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + symlinkSync( + join(paths.mosaicSkillsDir, 'retired-skill'), + join(paths.claudeSkillsDir, 'new-skill'), + ); + + expect(registerSkill('new-skill', paths).status).toBe('repaired'); + expectCorrectLink('new-skill'); + }); + + it.each(['file', 'directory', 'symlink'] as const)( + 'refuses to clobber a foreign %s at the target', + (kind) => { + createSkill('protected'); + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + const target = join(paths.claudeSkillsDir, 'protected'); + const foreign = join(root, 'foreign'); + + if (kind === 'file') writeFileSync(target, 'keep me\n'); + if (kind === 'directory') mkdirSync(target); + if (kind === 'symlink') { + writeFileSync(foreign, 'keep me\n'); + symlinkSync(foreign, target); + } + + expect(() => registerSkill('protected', paths)).toThrow(/foreign|refus/i); + if (kind === 'file') expect(lstatSync(target).isFile()).toBe(true); + if (kind === 'directory') expect(lstatSync(target).isDirectory()).toBe(true); + if (kind === 'symlink') expect(readlinkSync(target)).toBe(foreign); + }, + ); + + it('refuses a symlinked Claude skills ancestor instead of writing outside the bridge root', () => { + createSkill('protected'); + const externalClaude = join(root, 'external-claude'); + mkdirSync(externalClaude); + symlinkSync(externalClaude, join(root, '.claude')); + + expect(() => registerSkill('protected', paths)).toThrow( + /symlink.*ancestor|ancestor.*symlink/i, + ); + expect(existsSync(join(externalClaude, 'skills', 'protected'))).toBe(false); + }); + + it('refuses a symlinked canonical skills root instead of registering an external source', () => { + rmSync(paths.mosaicSkillsDir, { recursive: true }); + const externalSkills = join(root, 'external-skills'); + mkdirSync(join(externalSkills, 'protected'), { recursive: true }); + symlinkSync(externalSkills, paths.mosaicSkillsDir); + + expect(() => registerSkill('protected', paths)).toThrow( + /symlink.*ancestor|ancestor.*symlink/i, + ); + expect(existsSync(paths.claudeSkillsDir)).toBe(false); + }); + + it('refuses a dangling foreign symlink rather than treating it as repairable', () => { + createSkill('protected'); + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + const foreignMissing = join(root, 'foreign-missing'); + const target = join(paths.claudeSkillsDir, 'protected'); + symlinkSync(foreignMissing, target); + + expect(() => registerSkill('protected', paths)).toThrow(/foreign|refus/i); + expect(readlinkSync(target)).toBe(foreignMissing); + }); + }); + + describe('unregisterSkill', () => { + it('removes a Mosaic-owned symlink and is idempotent when absent', () => { + createSkill('removable'); + registerSkill('removable', paths); + + expect(unregisterSkill('removable', paths).status).toBe('unregistered'); + expect(existsSync(join(paths.claudeSkillsDir, 'removable'))).toBe(false); + + expect(unregisterSkill('removable', paths).status).toBe('already-unregistered'); + }); + + it('refuses to remove a misdirected Mosaic-root symlink', () => { + createSkill('other'); + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + const requested = join(paths.claudeSkillsDir, 'requested'); + symlinkSync(join(paths.mosaicSkillsDir, 'other'), requested); + + expect(() => unregisterSkill('requested', paths)).toThrow(/misdirected/i); + expect(readlinkSync(requested)).toBe(join(paths.mosaicSkillsDir, 'other')); + }); + + it.each(['file', 'directory', 'symlink'] as const)('refuses to remove a foreign %s', (kind) => { + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + const target = join(paths.claudeSkillsDir, 'protected'); + const foreign = join(root, 'foreign'); + + if (kind === 'file') writeFileSync(target, 'keep me\n'); + if (kind === 'directory') mkdirSync(target); + if (kind === 'symlink') { + writeFileSync(foreign, 'keep me\n'); + symlinkSync(foreign, target); + } + + expect(() => unregisterSkill('protected', paths)).toThrow(/foreign|refus/i); + expect(lstatSync(target)).toBeDefined(); + if (kind === 'symlink') expect(readlinkSync(target)).toBe(foreign); + }); + }); + + describe('listSkills', () => { + it('flags registered, unregistered, Mosaic-owned dangling, and foreign entries', () => { + createSkill('registered'); + createSkill('unregistered'); + registerSkill('registered', paths); + symlinkSync(join(paths.mosaicSkillsDir, 'retired'), join(paths.claudeSkillsDir, 'dangling')); + writeFileSync(join(paths.claudeSkillsDir, 'foreign-file'), 'keep me\n'); + symlinkSync(join(root, 'missing-foreign'), join(paths.claudeSkillsDir, 'foreign-link')); + + expect(listSkills(paths)).toEqual([ + expect.objectContaining({ name: 'dangling', status: 'dangling' }), + expect.objectContaining({ name: 'foreign-file', status: 'foreign' }), + expect.objectContaining({ name: 'foreign-link', status: 'foreign-dangling' }), + expect.objectContaining({ name: 'registered', status: 'registered' }), + expect.objectContaining({ name: 'unregistered', status: 'unregistered' }), + ]); + }); + }); + + describe('install linker compatibility', () => { + it('preserves foreign-name links into Mosaic home but outside canonical skills', () => { + createSkill('missing'); + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + const mosaicHome = join(root, '.config', 'mosaic'); + const liveForeignTarget = join(mosaicHome, 'foreign-non-skill-target'); + mkdirSync(liveForeignTarget); + const liveForeignLink = join(paths.claudeSkillsDir, 'foreign-tool'); + const danglingForeignLink = join(paths.claudeSkillsDir, 'unresolvable-foreign'); + symlinkSync(liveForeignTarget, liveForeignLink); + symlinkSync(join(mosaicHome, 'foreign-missing'), danglingForeignLink); + + const result = spawnSync('bash', [LEGACY_SYNC_SCRIPT, '--link-only'], { + encoding: 'utf8', + env: { ...process.env, HOME: root, MOSAIC_HOME: mosaicHome }, + }); + + expect(result.status, result.stderr).toBe(0); + expect(readlinkSync(liveForeignLink)).toBe(liveForeignTarget); + expect(readlinkSync(danglingForeignLink)).toBe(join(mosaicHome, 'foreign-missing')); + expect(readlinkSync(join(paths.claudeSkillsDir, 'missing'))).toBe( + join(paths.mosaicSkillsDir, 'missing'), + ); + }); + + it('preserves live and dangling foreign Claude symlinks while linking missing skills', () => { + createSkill('dangling-foreign'); + createSkill('live-foreign'); + createSkill('missing'); + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + const external = join(root, 'external'); + mkdirSync(external); + const liveLink = join(paths.claudeSkillsDir, 'live-foreign'); + const danglingLink = join(paths.claudeSkillsDir, 'dangling-foreign'); + symlinkSync(external, liveLink); + symlinkSync(join(root, 'external-missing'), danglingLink); + + const result = spawnSync('bash', [LEGACY_SYNC_SCRIPT, '--link-only'], { + encoding: 'utf8', + env: { ...process.env, HOME: root, MOSAIC_HOME: join(root, '.config', 'mosaic') }, + }); + + expect(result.status, result.stderr).toBe(0); + expect(readlinkSync(liveLink)).toBe(external); + expect(readlinkSync(danglingLink)).toBe(join(root, 'external-missing')); + expect(readlinkSync(join(paths.claudeSkillsDir, 'missing'))).toBe( + join(paths.mosaicSkillsDir, 'missing'), + ); + }); + }); + + describe('syncClaudeSkills', () => { + it('generically creates every missing canonical link and repairs managed broken links', () => { + createSkill('added-after-setup'); + createSkill('another-new-skill'); + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + symlinkSync( + join(paths.mosaicSkillsDir, 'retired'), + join(paths.claudeSkillsDir, 'added-after-setup'), + ); + + const result = syncClaudeSkills(paths); + + expect(result).toEqual({ + registered: ['another-new-skill'], + repaired: ['added-after-setup'], + unchanged: [], + conflicts: [], + }); + expectCorrectLink('added-after-setup'); + expectCorrectLink('another-new-skill'); + }); + + it('escapes an invalid filesystem-derived name in conflict output', () => { + createSkill('line\nbreak'); + + const result = syncClaudeSkills(paths); + + expect(result.registered).toEqual([]); + expect(result.conflicts).toEqual([ + expect.objectContaining({ + name: '"line\\nbreak"', + reason: expect.stringMatching(/invalid/i), + }), + ]); + expect(existsSync(paths.claudeSkillsDir)).toBe(false); + }); + + it('continues syncing other skills without clobbering foreign entries', () => { + createSkill('blocked'); + createSkill('link-me'); + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + const blocked = join(paths.claudeSkillsDir, 'blocked'); + writeFileSync(blocked, 'keep me\n'); + + const result = syncClaudeSkills(paths); + + expect(result.registered).toEqual(['link-me']); + expect(result.conflicts).toEqual([ + expect.objectContaining({ + name: 'blocked', + reason: expect.stringMatching(/foreign|refus/i), + }), + ]); + expect(readlinkSync(join(paths.claudeSkillsDir, 'link-me'))).toBe( + join(paths.mosaicSkillsDir, 'link-me'), + ); + expect(lstatSync(blocked).isFile()).toBe(true); + }); + }); +}); diff --git a/packages/mosaic/src/commands/skill.ts b/packages/mosaic/src/commands/skill.ts new file mode 100644 index 0000000..d4a4efa --- /dev/null +++ b/packages/mosaic/src/commands/skill.ts @@ -0,0 +1,419 @@ +import { + existsSync, + lstatSync, + mkdirSync, + readdirSync, + readlinkSync, + symlinkSync, + unlinkSync, + type Stats, +} from 'node:fs'; +import { homedir } from 'node:os'; +import { dirname, isAbsolute, join, parse, relative, resolve, sep } from 'node:path'; +import type { Command } from 'commander'; +import { DEFAULT_MOSAIC_HOME } from '../constants.js'; + +export interface SkillPaths { + mosaicSkillsDir: string; + claudeSkillsDir: string; +} + +export type SkillRegistrationStatus = 'registered' | 'already-registered' | 'repaired'; +export type SkillUnregistrationStatus = 'unregistered' | 'already-unregistered'; +export type SkillListStatus = + | 'registered' + | 'unregistered' + | 'dangling' + | 'foreign' + | 'foreign-dangling' + | 'misdirected'; + +export interface SkillRegistrationResult { + name: string; + status: SkillRegistrationStatus; + sourcePath: string; + linkPath: string; +} + +export interface SkillUnregistrationResult { + name: string; + status: SkillUnregistrationStatus; + linkPath: string; +} + +export interface SkillListEntry { + name: string; + status: SkillListStatus; + sourcePath?: string; + linkPath: string; + targetPath?: string; +} + +export interface SkillSyncConflict { + name: string; + reason: string; +} + +export interface SkillSyncResult { + registered: string[]; + repaired: string[]; + unchanged: string[]; + conflicts: SkillSyncConflict[]; +} + +const SAFE_SKILL_NAME = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; + +export class SkillBridgeError extends Error { + public constructor(message: string) { + super(message); + this.name = 'SkillBridgeError'; + } +} + +/** Resolve the production bridge paths while keeping tests injectable. */ +export function getDefaultSkillPaths(): SkillPaths { + const mosaicHome = process.env['MOSAIC_HOME'] ?? DEFAULT_MOSAIC_HOME; + const claudeHome = process.env['CLAUDE_HOME'] ?? join(homedir(), '.claude'); + return { + mosaicSkillsDir: join(mosaicHome, 'skills'), + claudeSkillsDir: join(claudeHome, 'skills'), + }; +} + +/** + * Reject a user-supplied name before any filesystem operation. + * A skill name must identify one direct child in both managed roots. + */ +export function validateSkillName(name: string): void { + if ( + name.length === 0 || + name.startsWith('-') || + name.endsWith('.') || + name.includes('..') || + name.includes('/') || + name.includes('\\') || + isAbsolute(name) || + !SAFE_SKILL_NAME.test(name) + ) { + throw new SkillBridgeError( + `Invalid skill name ${JSON.stringify(name)}: use letters, numbers, dots, underscores, or hyphens; start with a letter or number; and do not use paths, "..", or a leading "-".`, + ); + } +} + +function displaySkillName(name: string): string { + return SAFE_SKILL_NAME.test(name) ? name : JSON.stringify(name); +} + +function lstatIfPresent(path: string): Stats | undefined { + try { + return lstatSync(path); + } catch (error: unknown) { + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') return undefined; + throw error; + } +} + +function assertNoSymlinkAncestors(path: string): void { + const absolute = resolve(path); + const pathRoot = parse(absolute).root; + let current = pathRoot; + + for (const segment of relative(pathRoot, absolute).split(sep)) { + if (segment.length === 0) continue; + current = join(current, segment); + const entry = lstatIfPresent(current); + if (!entry) break; + if (entry.isSymbolicLink()) { + throw new SkillBridgeError( + `Refusing symlink ancestor at ${current}; managed skill roots must resolve without symlink traversal.`, + ); + } + } +} + +function assertManagedRoots(paths: SkillPaths): void { + assertNoSymlinkAncestors(paths.mosaicSkillsDir); + assertNoSymlinkAncestors(paths.claudeSkillsDir); +} + +function directChild(root: string, name: string): string { + const resolvedRoot = resolve(root); + const child = resolve(resolvedRoot, name); + if (dirname(child) !== resolvedRoot) { + throw new SkillBridgeError(`Invalid skill name "${name}": resolved path escapes its root.`); + } + return child; +} + +function resolveLinkTarget(linkPath: string): string { + return resolve(dirname(linkPath), readlinkSync(linkPath)); +} + +function isInsideSkillsRoot(targetPath: string, skillsRoot: string): boolean { + const rel = relative(resolve(skillsRoot), resolve(targetPath)); + return rel.length > 0 && rel !== '..' && !rel.startsWith(`..${sep}`) && !isAbsolute(rel); +} + +function isDangling(linkPath: string): boolean { + return !existsSync(linkPath); +} + +function assertSourceSkill(name: string, paths: SkillPaths): string { + const sourcePath = directChild(paths.mosaicSkillsDir, name); + const source = lstatIfPresent(sourcePath); + if (!source?.isDirectory()) { + throw new SkillBridgeError( + `Canonical skill directory not found: ${sourcePath}. Add the skill under the Mosaic skills directory before registering it.`, + ); + } + return sourcePath; +} + +function foreignTargetError(linkPath: string): SkillBridgeError { + return new SkillBridgeError( + `Refusing to modify foreign entry at ${linkPath}; only symlinks pointing inside the Mosaic skills directory are managed.`, + ); +} + +/** Register one canonical skill with Claude Code without clobbering foreign entries. */ +export function registerSkill( + name: string, + paths: SkillPaths = getDefaultSkillPaths(), +): SkillRegistrationResult { + validateSkillName(name); + assertManagedRoots(paths); + const sourcePath = assertSourceSkill(name, paths); + const linkPath = directChild(paths.claudeSkillsDir, name); + const existing = lstatIfPresent(linkPath); + + if (!existing) { + mkdirSync(paths.claudeSkillsDir, { recursive: true }); + symlinkSync(sourcePath, linkPath); + return { name, status: 'registered', sourcePath, linkPath }; + } + + if (!existing.isSymbolicLink()) throw foreignTargetError(linkPath); + + const existingTarget = resolveLinkTarget(linkPath); + if (!isInsideSkillsRoot(existingTarget, paths.mosaicSkillsDir)) { + throw foreignTargetError(linkPath); + } + + if (existingTarget === resolve(sourcePath) && !isDangling(linkPath)) { + return { name, status: 'already-registered', sourcePath, linkPath }; + } + + if (!isDangling(linkPath)) { + throw new SkillBridgeError( + `Refusing to replace live Mosaic skill symlink at ${linkPath}; it points to ${existingTarget}, not ${sourcePath}.`, + ); + } + + unlinkSync(linkPath); + symlinkSync(sourcePath, linkPath); + return { name, status: 'repaired', sourcePath, linkPath }; +} + +/** Unregister only a symlink owned by the canonical Mosaic skills root. */ +export function unregisterSkill( + name: string, + paths: SkillPaths = getDefaultSkillPaths(), +): SkillUnregistrationResult { + validateSkillName(name); + assertManagedRoots(paths); + const linkPath = directChild(paths.claudeSkillsDir, name); + const existing = lstatIfPresent(linkPath); + + if (!existing) return { name, status: 'already-unregistered', linkPath }; + if (!existing.isSymbolicLink()) throw foreignTargetError(linkPath); + + const targetPath = resolveLinkTarget(linkPath); + if (!isInsideSkillsRoot(targetPath, paths.mosaicSkillsDir)) throw foreignTargetError(linkPath); + + const expectedTarget = resolve(directChild(paths.mosaicSkillsDir, name)); + if (targetPath !== expectedTarget) { + throw new SkillBridgeError( + `Refusing to unregister misdirected Mosaic skill symlink at ${linkPath}; it points to ${targetPath}, not ${expectedTarget}.`, + ); + } + + unlinkSync(linkPath); + return { name, status: 'unregistered', linkPath }; +} + +function canonicalSkillNames(paths: SkillPaths): string[] { + assertNoSymlinkAncestors(paths.mosaicSkillsDir); + const root = lstatIfPresent(paths.mosaicSkillsDir); + if (!root) return []; + if (!root.isDirectory()) { + throw new SkillBridgeError( + `Canonical skills path is not a directory: ${paths.mosaicSkillsDir}`, + ); + } + return readdirSync(paths.mosaicSkillsDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort(); +} + +function claudeEntryNames(paths: SkillPaths): string[] { + assertNoSymlinkAncestors(paths.claudeSkillsDir); + const root = lstatIfPresent(paths.claudeSkillsDir); + if (!root) return []; + if (!root.isDirectory()) { + throw new SkillBridgeError(`Claude skills path is not a directory: ${paths.claudeSkillsDir}`); + } + return readdirSync(paths.claudeSkillsDir) + .filter((name) => name.length > 0) + .sort(); +} + +/** Return a deterministic union of canonical skills and Claude bridge entries. */ +export function listSkills(paths: SkillPaths = getDefaultSkillPaths()): SkillListEntry[] { + const canonicalNames = new Set(canonicalSkillNames(paths)); + const names = new Set([...canonicalNames, ...claudeEntryNames(paths)]); + const entries: SkillListEntry[] = []; + + for (const name of [...names].sort()) { + const sourcePath = canonicalNames.has(name) + ? directChild(paths.mosaicSkillsDir, name) + : undefined; + const linkPath = directChild(paths.claudeSkillsDir, name); + const installed = lstatIfPresent(linkPath); + + if (!installed) { + if (sourcePath) entries.push({ name, status: 'unregistered', sourcePath, linkPath }); + continue; + } + + if (!installed.isSymbolicLink()) { + entries.push({ name, status: 'foreign', sourcePath, linkPath }); + continue; + } + + const targetPath = resolveLinkTarget(linkPath); + const owned = isInsideSkillsRoot(targetPath, paths.mosaicSkillsDir); + const dangling = isDangling(linkPath); + + if (!owned) { + entries.push({ + name, + status: dangling ? 'foreign-dangling' : 'foreign', + sourcePath, + linkPath, + targetPath, + }); + continue; + } + + if (dangling) { + entries.push({ name, status: 'dangling', sourcePath, linkPath, targetPath }); + continue; + } + + entries.push({ + name, + status: sourcePath && targetPath === resolve(sourcePath) ? 'registered' : 'misdirected', + sourcePath, + linkPath, + targetPath, + }); + } + + return entries; +} + +/** Reconcile every canonical skill directory while preserving all foreign entries. */ +export function syncClaudeSkills(paths: SkillPaths = getDefaultSkillPaths()): SkillSyncResult { + const result: SkillSyncResult = { + registered: [], + repaired: [], + unchanged: [], + conflicts: [], + }; + + for (const name of canonicalSkillNames(paths)) { + try { + const registration = registerSkill(name, paths); + if (registration.status === 'registered') result.registered.push(name); + if (registration.status === 'repaired') result.repaired.push(name); + if (registration.status === 'already-registered') result.unchanged.push(name); + } catch (error: unknown) { + result.conflicts.push({ + name: displaySkillName(name), + reason: error instanceof Error ? error.message : String(error), + }); + } + } + + return result; +} + +function reportCommandError(error: unknown): void { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; +} + +/** Register the `mosaic skill` command group. */ +export function registerSkillCommand( + program: Command, + paths: SkillPaths = getDefaultSkillPaths(), +): void { + const skill = program + .command('skill') + .description('Manage Claude Code skill registrations') + .configureHelp({ sortSubcommands: true }); + + skill + .command('register ') + .description('Register a Mosaic skill with Claude Code') + .action((name: string) => { + try { + const result = registerSkill(name, paths); + if (result.status === 'already-registered') { + console.log(`${name}: already registered`); + } else if (result.status === 'repaired') { + console.log(`${name}: repaired dangling registration`); + } else { + console.log(`${name}: registered`); + } + } catch (error: unknown) { + reportCommandError(error); + } + }); + + skill + .command('unregister ') + .description('Unregister a Mosaic skill from Claude Code') + .action((name: string) => { + try { + const result = unregisterSkill(name, paths); + console.log( + result.status === 'already-unregistered' + ? `${name}: already unregistered` + : `${name}: unregistered`, + ); + } catch (error: unknown) { + reportCommandError(error); + } + }); + + skill + .command('list') + .description('List registered, dangling, foreign, and unregistered skills') + .action(() => { + try { + const entries = listSkills(paths); + if (entries.length === 0) { + console.log('No Mosaic or Claude Code skills found.'); + return; + } + for (const entry of entries) { + console.log(`${entry.status.padEnd(17)} ${displaySkillName(entry.name)}`); + } + } catch (error: unknown) { + reportCommandError(error); + } + }); +} diff --git a/packages/mosaic/src/runtime/update-checker.reseed.spec.ts b/packages/mosaic/src/runtime/update-checker.reseed.spec.ts index ce067e5..83fe1bb 100644 --- a/packages/mosaic/src/runtime/update-checker.reseed.spec.ts +++ b/packages/mosaic/src/runtime/update-checker.reseed.spec.ts @@ -7,6 +7,7 @@ import { mkdirSync, readdirSync, readFileSync, + readlinkSync, rmSync, statSync, symlinkSync, @@ -300,6 +301,50 @@ describe('repairFleetCommsTools', () => { }); describe('runFrameworkReseed', () => { + it('auto-registers every canonical skill after a successful upgrade re-seed', () => { + const root = mkdtempSync(join(tmpdir(), 'mosaic-reseed-skills-')); + const framework = join(root, 'framework'); + const home = join(root, 'mosaic'); + const claudeSkills = join(root, '.claude', 'skills'); + mkdirSync(framework, { recursive: true }); + mkdirSync(join(home, 'skills', 'added-after-setup'), { recursive: true }); + mkdirSync(join(home, 'skills', 'another-new-skill'), { recursive: true }); + writeFileSync(join(framework, 'install.sh'), '#!/usr/bin/env bash\nexit 0\n', { mode: 0o755 }); + + const res = runFrameworkReseed(framework, home, claudeSkills); + + expect(res.ok).toBe(true); + expect(res.skillSync).toMatchObject({ + registered: ['added-after-setup', 'another-new-skill'], + conflicts: [], + }); + expect(readlinkSync(join(claudeSkills, 'added-after-setup'))).toBe( + join(home, 'skills', 'added-after-setup'), + ); + expect(readlinkSync(join(claudeSkills, 'another-new-skill'))).toBe( + join(home, 'skills', 'another-new-skill'), + ); + rmSync(root, { recursive: true, force: true }); + }); + + it('keeps a successful framework re-seed successful when bridge reconciliation fails', () => { + const root = mkdtempSync(join(tmpdir(), 'mosaic-reseed-bridge-failure-')); + const framework = join(root, 'framework'); + const home = join(root, 'mosaic'); + const claudeSkills = join(root, '.claude', 'skills'); + mkdirSync(framework, { recursive: true }); + mkdirSync(home, { recursive: true }); + writeFileSync(join(home, 'skills'), 'invalid canonical root\n'); + writeFileSync(join(framework, 'install.sh'), '#!/usr/bin/env bash\nexit 0\n', { mode: 0o755 }); + + const res = runFrameworkReseed(framework, home, claudeSkills); + + expect(res.ok).toBe(true); + expect(res.skillSync).toBeUndefined(); + expect(res.skillSyncError).toMatch(/not a directory/i); + rmSync(root, { recursive: true, force: true }); + }); + it('reports not-ok (not throw) when the installer is absent', () => { const missing = mkdtempSync(join(tmpdir(), 'mosaic-noinstaller-')); const res = runFrameworkReseed(missing, join(missing, 'home')); diff --git a/packages/mosaic/src/runtime/update-checker.ts b/packages/mosaic/src/runtime/update-checker.ts index 6b1aee6..813fe4e 100644 --- a/packages/mosaic/src/runtime/update-checker.ts +++ b/packages/mosaic/src/runtime/update-checker.ts @@ -43,6 +43,7 @@ import { ensureManagedDirectory, readRegularFileSecure, } from '../fleet/secure-file.js'; +import { getDefaultSkillPaths, syncClaudeSkills, type SkillSyncResult } from '../commands/skill.js'; // ─── Types ────────────────────────────────────────────────────────────────── @@ -871,19 +872,39 @@ export function repairFleetCommsTools( * describing what happened (so callers can message + decide on relaunch). * Best-effort: a missing installer or a non-zero exit is reported, not thrown. */ +export interface FrameworkReseedResult { + ok: boolean; + reason?: string; + skillSync?: SkillSyncResult; + skillSyncError?: string; +} + export function runFrameworkReseed( frameworkRoot = resolveBundledFrameworkRoot(), mosaicHome = join(homedir(), '.config', 'mosaic'), -): { ok: boolean; reason?: string } { + claudeSkillsDir = getDefaultSkillPaths().claudeSkillsDir, +): FrameworkReseedResult { const { installer, command, env } = buildReseedCommand(frameworkRoot, mosaicHome); if (!existsSync(installer)) { return { ok: false, reason: `installer not found: ${installer}` }; } try { execSync(command, { stdio: 'inherit', env: { ...process.env, ...env }, timeout: 120_000 }); - return { ok: true }; - } catch (err) { - return { ok: false, reason: err instanceof Error ? err.message : String(err) }; + } catch (error: unknown) { + return { ok: false, reason: error instanceof Error ? error.message : String(error) }; + } + + try { + const skillSync = syncClaudeSkills({ + mosaicSkillsDir: join(mosaicHome, 'skills'), + claudeSkillsDir, + }); + return { ok: true, skillSync }; + } catch (error: unknown) { + return { + ok: true, + skillSyncError: error instanceof Error ? error.message : String(error), + }; } } diff --git a/packages/mosaic/src/stages/finalize-skills.spec.ts b/packages/mosaic/src/stages/finalize-skills.spec.ts index 61e427c..f39cab7 100644 --- a/packages/mosaic/src/stages/finalize-skills.spec.ts +++ b/packages/mosaic/src/stages/finalize-skills.spec.ts @@ -9,7 +9,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, readlinkSync, writeFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import type { WizardState } from '../types.js'; @@ -113,6 +113,40 @@ describe('finalizeStage — skill installer', () => { ); } + it('auto-registers every canonical skill even when it was added after initial setup', async () => { + const claudeHome = join(tmp, '.claude'); + const previousClaudeHome = process.env['CLAUDE_HOME']; + process.env['CLAUDE_HOME'] = claudeHome; + mkdirSync(join(tmp, 'skills', 'added-after-setup'), { recursive: true }); + mkdirSync(join(tmp, 'skills', 'another-new-skill'), { recursive: true }); + + try { + await finalizeStage(buildPrompter(), makeState(tmp, []), makeConfigService()); + + expect(readlinkSync(join(claudeHome, 'skills', 'added-after-setup'))).toBe( + join(tmp, 'skills', 'added-after-setup'), + ); + expect(readlinkSync(join(claudeHome, 'skills', 'another-new-skill'))).toBe( + join(tmp, 'skills', 'another-new-skill'), + ); + } finally { + if (previousClaudeHome === undefined) delete process.env['CLAUDE_HOME']; + else process.env['CLAUDE_HOME'] = previousClaudeHome; + } + }); + + it('warns and completes finalization when bridge-wide reconciliation fails', async () => { + writeFileSync(join(tmp, 'skills'), 'invalid canonical root\n'); + const p = buildPrompter(); + + await finalizeStage(p, makeState(tmp, []), makeConfigService()); + + expect(p.warn).toHaveBeenCalledWith( + expect.stringMatching(/Claude skill reconciliation skipped.*not a directory/i), + ); + expect(p.outro).toHaveBeenCalledWith('Mosaic is ready.'); + }); + it('passes MOSAIC_INSTALL_SKILLS with the selected skill list', async () => { const state = makeState(tmp, ['brainstorming', 'lint', 'systematic-debugging']); const p = buildPrompter(); diff --git a/packages/mosaic/src/stages/finalize.ts b/packages/mosaic/src/stages/finalize.ts index 95dd503..3bf832b 100644 --- a/packages/mosaic/src/stages/finalize.ts +++ b/packages/mosaic/src/stages/finalize.ts @@ -7,6 +7,11 @@ import type { ConfigService } from '../config/config-service.js'; import type { WizardState } from '../types.js'; import { getShellProfilePath } from '../platform/detect.js'; import { ManifestError } from '../framework/manifest.js'; +import { + getDefaultSkillPaths, + syncClaudeSkills, + type SkillSyncResult as ClaudeSkillSyncResult, +} from '../commands/skill.js'; function linkRuntimeAssets(mosaicHome: string, skipClaudeHooks: boolean): void { const script = join(mosaicHome, 'bin', 'mosaic-link-runtime-assets'); @@ -205,7 +210,27 @@ export async function finalizeStage( skillsResult = syncSkills(state.mosaicHome, state.selectedSkills); } - // 5. Run doctor + // 5. Reconcile every canonical Mosaic skill into Claude Code. This is + // intentionally independent of the first-run selected-skill fetch above: + // framework installs/upgrades must also register skills added after setup. + spin.update('Registering Mosaic skills with Claude Code...'); + let bridgeResult: ClaudeSkillSyncResult = { + registered: [], + repaired: [], + unchanged: [], + conflicts: [], + }; + let bridgeFailure: string | undefined; + try { + bridgeResult = syncClaudeSkills({ + mosaicSkillsDir: join(state.mosaicHome, 'skills'), + claudeSkillsDir: getDefaultSkillPaths().claudeSkillsDir, + }); + } catch (error: unknown) { + bridgeFailure = error instanceof Error ? error.message : String(error); + } + + // 6. Run doctor spin.update('Running health audit...'); const doctorResult = runDoctor(state.mosaicHome); @@ -217,10 +242,15 @@ export async function finalizeStage( p.warn("Run 'mosaic sync' manually after installation to install skills."); } - // 6. PATH setup + if (bridgeFailure) p.warn(`Claude skill reconciliation skipped: ${bridgeFailure}`); + for (const conflict of bridgeResult.conflicts) { + p.warn(`Skill registration skipped for ${conflict.name}: ${conflict.reason}`); + } + + // 7. PATH setup const pathAction = setupPath(state.mosaicHome, p); - // 7. Summary + // 8. Summary const skillsSummary = skillsResult.success ? skillsResult.installedCount > 0 ? `${skillsResult.installedCount.toString()} installed` @@ -245,7 +275,7 @@ export async function finalizeStage( p.note(summary.join('\n'), 'Installation Summary'); - // 8. Next steps + // 9. Next steps const nextSteps: string[] = []; if (pathAction === 'added') { const profilePath = getShellProfilePath(); diff --git a/packages/mosaic/vitest.config.ts b/packages/mosaic/vitest.config.ts index 6d8f18f..9476528 100644 --- a/packages/mosaic/vitest.config.ts +++ b/packages/mosaic/vitest.config.ts @@ -5,5 +5,16 @@ export default defineConfig({ globals: true, environment: 'node', testTimeout: 30_000, + coverage: { + provider: 'v8', + include: ['src/commands/skill.ts'], + reporter: ['text', 'json-summary'], + thresholds: { + statements: 85, + branches: 85, + functions: 85, + lines: 85, + }, + }, }, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 71aeab5..33300ce 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -607,6 +607,9 @@ importers: '@types/react': specifier: ^18.3.0 version: 18.3.28 + '@vitest/coverage-v8': + specifier: ^2.0.0 + version: 2.1.9(vitest@2.1.9(@types/node@22.19.15)(jsdom@29.0.0(@noble/hashes@2.0.1))(lightningcss@1.31.1)) tsx: specifier: ^4.0.0 version: 4.21.0