feat(mosaic): add secure skill registration CLI #826
Reference in New Issue
Block a user
Delete Branch "feat/824-mosaic-skill-cli"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
mosaic skill register|unregister|listfor Claude CodeVerification
Closes #824.
Independent RoR — MS-lead reviewer lane
ms-rev-826(terra), author lanems-824(sol). Author ≠ reviewer PROCESS/LANE-attested (single Gitea account).Review of Record — VERDICT: REQUEST CHANGES
Reviewed tree (TRUE PROSPECTIVE MERGE):
2fe75a43162bfe6f25c9f8621613d26667bb7dabMerge-base:
3f77229e· Target main:d3bf5289· PR heade074c7dcmerged onto current main (branch was cut pre-#804; reviewed the materialized merge, not the raw head).Independent exact-head RoR (author ≠ reviewer; terra lane). Full package suite on the merged tree: 1328/1328 green, lint/typecheck/format PASS. #804×#824
install.shintegration: PASS (no #824 path passes an argument the #804 parser rejects; reseed invocation is no-arg). Name-validation, generic auto-sync, temp-dir test hygiene: PASS.Blockers (must fix before merge)
MAJOR-1 — foreign-link deletion during auto-sync.
packages/mosaic/framework/tools/_scripts/mosaic-sync-skills:245-254removes a link whose resolved target is anywhere under$MOSAIC_HOME, rather than only links demonstrably Mosaic-owned (target under$MOSAIC_HOME/skills/*or the expected source). A foreign~/.claude/skillslink pointing at$MOSAIC_HOME/foreign-non-skill-targetisrm -f'd. The unreadable/unresolvable-link removal at245-249also deletes without proving ownership. Focused coverage atskill.spec.ts:245-266masks this (each foreign link is given a name that also exists in the canonical skills dir, so pruning is skipped). Fix: gate deletion on demonstrable Mosaic ownership; add a genuine foreign-name regression test.MAJOR-2 — unregister accepts a misdirected Mosaic-root link.
packages/mosaic/src/commands/skill.ts:230-233unlinks any target insidemosaicSkillsDirwithout requiring the expected source.mosaic skill unregister requesteddeletes~/.claude/skills/requested -> ~/.config/mosaic/skills/other. The CLI already labels thismisdirectedatskill.ts:307-312yet unregister still deletes it. Fix: requiretargetPath === resolve(directChild(mosaicSkillsDir, name))(preserving the intended dangling-link case) before unlinking; add a regression test.Non-blocking
vitest.config.tshas no coverage provider/threshold and@vitest/coverage-v8is not installed.safe.) are accepted; reject for Windows filename-normalization portability.Remediation is red-first (failing test proving the invariant, then fix). No commits/pushes/mutations were performed during review. Full RoR retained by the MS lead.
Independent RE-RoR — MS-lead reviewer lane
ms-rev-826(terra), author lanems-824(sol). Author ≠ reviewer PROCESS/LANE-attested (single Gitea account). Zero evidence transfer — prior RoR (heade074c7dc) VOID; verified fresh at this head.Review of Record — VERDICT: APPROVE
Remediation head:
2db67ed5726bc56ad21fe2da9aaa09eb5159761aReviewed TRUE PROSPECTIVE MERGE tree:
11b96982206fe386684d29c675edbd90ab3e19b0(head merged onto current maind3bf5289; merge-base3f77229e; clean auto-merge, zero conflicts)Both prior MAJOR blockers are FIXED on this exact merged tree, each proven by a fresh hermetic reproduction AND an independent red-then-green replay:
MAJOR-1 (foreign-link deletion) — FIXED.
mosaic-sync-skills:245-251now canonicalizes lexically (readlink -m), preserves any link whose ownership is unproven, and deletes only a retired link whose target is under$MOSAIC_HOME/skills/*(not merely under$MOSAIC_HOME). Fresh repro: a live foreign linkforeign-tool -> $MOSAIC_HOME/foreign-non-skill-targetand an unresolvable foreign link both SURVIVE sync while canonical skills link. Regressionskill.spec.ts:312-334uses genuine foreign names; independent RED replay one074c7dcfailed the foreign-name test; current focused run 43/43 green.MAJOR-2 (misdirected unregister) — FIXED.
skill.ts:231-242now requirestargetPath === resolve(directChild(mosaicSkillsDir, name))before unlinking; arequested -> skills/otherlink is refused (reportedmisdirected) while the exact-name dangling link remains removable. Fresh CLI repro confirms both; regressionskill.spec.ts:264-272; independent RED replay one074c7dcfailed this exact test; current 43/43 green.Regression guard — PASS. Full package suite on merged tree: 70/70 files, 1334/1334 tests green. #804×#824 install.sh integration PASS (no #824 path passes an arg #804 rejects; reseed no-arg; install.sh byte-identical to main). Name-validation, generic auto-sync (no hardcoded allowlist), temp-dir test hygiene all PASS. typecheck 42/42, lint 23/23, format PASS.
Prior non-blockers — resolved. Coverage:
@vitest/coverage-v8added + 85% thresholds configured;skill.ts= 91.05% stmts/lines, 86.27% branches, 95.23% funcs (all ≥85%). Trailing-dot names now rejected (skill.ts:91).No surviving MAJOR/MINOR findings. No commits/pushes/mutations performed during review. Cleared for coordinated-lane merge on the base-drift-at-merge-instant gate (main verified
d3bf5289at merge). Full RoR retained at reviews/pr826-824-merged-RoR.md.