feat(adopt): claim CLI-bundled skills into skillshare management (continues #181) - #248
feat(adopt): claim CLI-bundled skills into skillshare management (continues #181)#248salmonumbrella wants to merge 2 commits into
Conversation
Some CLI tools (e.g. firecrawl/cli, googleworkspace/cli) ship skills directly into ~/.agents/skills and symlink them into the agents they detect, bypassing skillshare's source-of-truth model. `skillshare adopt` repairs this: it migrates the canonical files into the source tree, prunes only the tool links identified in the preview, trashes the originals (restorable), and re-syncs to every configured target. Supports --dry-run, --all, --force, and project mode (-p). Ownership safeguards refuse skills managed by project install replay even under --force, and the owning tool's ~/.agents/.skill-lock.json is never modified — adopt warns to release the entry from the owning tool instead. Ships with an Adopt dashboard page, an HTTP adopt endpoint, command docs, and a skill reference. Co-Authored-By: Willie <15520254+runkids@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55438de175
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| syncErr = sync.SyncTarget(name, target, req.SourcePath, false, req.ProjectRoot) | ||
| default: // merge | ||
| var mergeResult *sync.MergeResult | ||
| mergeResult, syncErr = sync.SyncTargetMerge(name, target, req.SourcePath, false, false, req.ProjectRoot) |
There was a problem hiding this comment.
Avoid syncing skipped adopt candidates
When a candidate is skipped because a same-name source skill already exists (for example adopt --all without --force), this still performs a full target sync. In merge mode, SyncTargetMerge processes every source skill, so an external/orphan symlink for the skipped candidate gets replaced with a link to the existing source copy even though that candidate was not adopted; in symlink mode this can also migrate remaining target contents. Please skip the resync when nothing was adopted, or limit the follow-up sync to successfully adopted names so skipped conflicts remain untouched.
Useful? React with 👍 / 👎.
Type
Feature — a continuation of #181 (your
adoptbranch), squashed to a single commit with the CI blocker fixed.Linked Issue
ref #135, continues #181
Summary
Picks up where #181 left off and gets it green:
TestAdoptProject_MigratesAndResyncsfailed comparing symlink targets (/varvs/private/varon macOS runners). The test now resolves both sides throughEvalSymlinksbefore comparing, and the CLI writes relative project links.adopt -prefuses to claim skills managed by project install replay, even under--force, instead of silently taking over config-managed entries. The owning tool's~/.agents/.skill-lock.jsonis never modified; adopt warns to release the entry from the owning tool.adopt. Changelog left to you at release time.Per CONTRIBUTING's note that implementation stays with the maintainer: treat this as a concrete reference and take whatever is useful — it's one commit for easy cherry-picking.
Checklist
make check) — full Go suite and 114 UI tests pass locally on macOS;fmt-checkandlintclean🤖 Generated with Claude Code