From f751fc9e1fbf22f1abfdc1a0dfb285ad840d5558 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Fri, 17 Jul 2026 17:43:08 -0500 Subject: [PATCH] feat(#824): manage Claude skill registrations --- 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 | 56 ++- packages/mosaic/framework/defaults/README.md | 12 +- packages/mosaic/src/cli.ts | 19 +- packages/mosaic/src/commands/skill.spec.ts | 27 +- packages/mosaic/src/commands/skill.ts | 411 ++++++++++++++++++ .../src/runtime/update-checker.reseed.spec.ts | 18 + packages/mosaic/src/runtime/update-checker.ts | 29 +- .../mosaic/src/stages/finalize-skills.spec.ts | 12 + packages/mosaic/src/stages/finalize.ts | 38 +- 13 files changed, 681 insertions(+), 20 deletions(-) create mode 100644 packages/mosaic/src/commands/skill.ts diff --git a/README.md b/README.md index ffb6025..731dca1 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 e2e82e5..660b025 100644 --- a/docs/guides/user-guide.md +++ b/docs/guides/user-guide.md @@ -307,6 +307,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 index c1c617c..5bb9eca 100644 --- a/docs/scratchpads/824-mosaic-skill-cli.md +++ b/docs/scratchpads/824-mosaic-skill-cli.md @@ -41,16 +41,64 @@ Deliver `mosaic skill register|unregister|list` plus install/upgrade reconciliat - 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 verified: focused Vitest suite failed with `Failed to load url ./skill.js ... Does the file exist?`, proving the skill bridge API/implementation is absent before production code. +- 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 must use `lstat` semantics so dangling links are detectable without following them. -- Link ownership must be determined lexically/canonically against the canonical skills root without following a foreign destination through an attacker-controlled path. -- Auto-sync must continue across conflicts while never deleting real files/directories or foreign symlinks. +- 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. diff --git a/packages/mosaic/framework/defaults/README.md b/packages/mosaic/framework/defaults/README.md index 629c0bf..0a27272 100644 --- a/packages/mosaic/framework/defaults/README.md +++ b/packages/mosaic/framework/defaults/README.md @@ -179,13 +179,19 @@ bash tools/install.sh --ref v1.0 # Install from a specific git ref ## 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/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 index 77ec664..62f194a 100644 --- a/packages/mosaic/src/commands/skill.spec.ts +++ b/packages/mosaic/src/commands/skill.spec.ts @@ -52,7 +52,17 @@ describe('Claude skill bridge', () => { } describe('name validation', () => { - const invalidNames = ['../../etc', '/abs/path', 'a/b', String.raw`a\b`, '-rf', '..']; + const invalidNames = [ + '../../etc', + '/abs/path', + 'a/b', + String.raw`a\b`, + '-rf', + '..', + 'space name', + 'line\nbreak', + 'escape\u001B[31m', + ]; for (const name of invalidNames) { it(`rejects ${JSON.stringify(name)} before register can escape its roots`, () => { @@ -247,6 +257,21 @@ describe('Claude skill bridge', () => { 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'); diff --git a/packages/mosaic/src/commands/skill.ts b/packages/mosaic/src/commands/skill.ts new file mode 100644 index 0000000..fa2aa01 --- /dev/null +++ b/packages/mosaic/src/commands/skill.ts @@ -0,0 +1,411 @@ +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.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); + + 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 6f6fbba..83fe1bb 100644 --- a/packages/mosaic/src/runtime/update-checker.reseed.spec.ts +++ b/packages/mosaic/src/runtime/update-checker.reseed.spec.ts @@ -327,6 +327,24 @@ describe('runFrameworkReseed', () => { 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 c5c92e5..f39cab7 100644 --- a/packages/mosaic/src/stages/finalize-skills.spec.ts +++ b/packages/mosaic/src/stages/finalize-skills.spec.ts @@ -135,6 +135,18 @@ describe('finalizeStage — skill installer', () => { } }); + 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();