mosaicstack/stack
1
0
Fork 0
Code Issues 79 Pull Requests Actions Packages Projects Releases 15 Wiki Activity

feat(#824): manage Claude skill registrations

Browse Source
This commit is contained in:
Hermes Agent
2026-07-17 17:43:08 -05:00
parent 1cc45b7deb
commit f751fc9e1f
13 changed files with 681 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
docs/scratchpads/824-mosaic-skill-cli.md
View File

@@ -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.