feat(cli): skdd link --adopt — distribute the colony into populated harness dirs#12
Conversation
…ess dirs Adds an additive alternative to the whole-directory symlink/copy mirror. For a harness dir that already holds non-colony skills (the common global ~/.claude/skills, ~/.codex/skills case), --adopt copies each colony skill INTO the dir and never touches foreign skills — so you can distribute the colony without the guardrail-violating 'link --force' that would replace (delete) the populated dir. Per skill: created if absent, unchanged if byte-identical, skipped if it diverges from the colony (add --force to overwrite a *colony* skill that diverges; foreign skills are never touched). Adopted copies aren't tracked in sync-state — one-way push, not a managed mirror. Dirs already symlinked to the colony are detected and skipped. lib/fs-link.ts: adoptSkills(). commands/link.ts: runAdopt() reached via the new --adopt flag on 'skdd link' (works with -g and project scope). +6 tests; 949 total green.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccb6137582
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (opts.force) { | ||
| rmSync(dest, { recursive: true, force: true }); | ||
| cpSync(src, dest, { recursive: true }); |
There was a problem hiding this comment.
When --adopt --force is used against a populated harness dir, this removes any existing target entry whose directory name matches a colony skill, even though adopted copies are not tracked and the target entry may be a hand-authored/foreign skill with the same name. In that collision scenario the command deletes user data despite the feature's stated safety contract that non-colony skills are never touched; force should only refresh entries known to be prior colony copies or otherwise require an explicit collision-handling path.
Useful? React with 👍 / 👎.
| harnesses = opts.global | ||
| ? (Object.keys(HARNESSES) as Harness[]).filter((h) => existsSync(dirname(globalSkillsDir(h)))) | ||
| : detectAllHarnesses(resolve(opts.cwd ?? process.cwd())); | ||
| if (harnesses.length === 0) harnesses = ["claude"]; |
There was a problem hiding this comment.
When skdd link -g --adopt is run without --harness on a machine with no global harness parent dirs, this fallback selects claude instead of following the normal global link no-op path. The subsequent adopt step creates ~/.claude/skills and copies the colony, so a global adopt can populate a harness directory for a tool that was not installed or detected; keep this as a no-op/warning unless the harness was explicit.
Useful? React with 👍 / 👎.
| continue; | ||
| } | ||
|
|
||
| const results = adoptSkills(canonicalPath, target, { force: opts.force }); |
There was a problem hiding this comment.
If one populated harness target has an invalid entry for a colony skill, such as a regular file or unreadable directory with the same name, adoptSkills() can throw while hashing/copying it, and this call is not caught per harness. In that case skdd link --adopt rejects with a stack trace and stops before processing the remaining harness dirs instead of reporting that target as skipped/failed while preserving the rest of the adoption run.
Useful? React with 👍 / 👎.
…t skills Codex review: --adopt --force could clobber a same-named target skill. A same-named skill in a harness dir may be an independent per-harness fork, not a drifted colony copy — content alone can't distinguish them, so overwriting risks destroying hand-authored work. adopt is now strictly additive: created if absent, unchanged if identical, skipped-divergent (kept, reported) if a same-named skill differs. Removed the force-overwrite path entirely; --force with --adopt warns that it does not overwrite. Divergent collisions are surfaced for manual resolution. Test updated to assert non-clobber (unit + an end-to-end --force case). 950 green.
|
Addressed the stop-review finding ( |
…non-dir entries Three follow-up review findings: - global adopt with no detected harness dirs no longer defaults to claude (would fabricate ~/.claude/skills for an uninstalled tool) — it warns and no-ops, matching runLinkGlobal - a same-named target entry that isn't a directory (regular file, device node) is treated as skipped-divergent instead of throwing during hashing - per-harness adoptSkills failures are caught and reported (that target skipped) so one bad dir doesn't abort adoption into the rest; returns 1 if any target failed (The first-round 'force clobbers' finding was already resolved by making adopt strictly additive — confirmed no rm/force path in adoptSkills.) +3 tests; 952 green.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4dcb17b83
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!existsSync(dest)) { | ||
| cpSync(src, dest, { recursive: true }); |
There was a problem hiding this comment.
When a target harness already contains a dangling symlink with the same name as a colony skill, existsSync(dest) returns false even though the path is occupied. This takes the create path and cpSync() then throws while trying to copy a directory over the symlink, causing runAdopt() to fail the whole harness and skip any remaining skills there instead of treating the entry as the documented divergent collision left untouched; use an lstat/lexists-style check before deciding the skill is absent.
Useful? React with 👍 / 👎.
| if (!quiet) logger.warn(`${label}: symlink points outside the colony — skipping`); | ||
| continue; |
There was a problem hiding this comment.
If the requested harness skills directory is a symlink to some other location, this branch only warns and then continues without incrementing failed, so skdd link --adopt --harness=<that harness> exits 0 even though no colony skills were adopted for that target; with --quiet there is no signal at all. Count this as a failed or skipped target so callers do not treat a no-op adoption as success.
Useful? React with 👍 / 👎.
| entries = readdirSync(canonicalPath, { withFileTypes: true }); | ||
| } catch { | ||
| return []; |
There was a problem hiding this comment.
If the canonical skills path exists but cannot be read, or is accidentally a file, this catch turns the readdirSync failure into an empty skill list. runAdopt() then exits successfully after adopting nothing, which hides a broken colony and leaves the harness dirs without the expected skills; only an actually readable empty directory should produce [], while read/ENOTDIR errors should fail the adopt run.
Useful? React with 👍 / 👎.
| // target must not abort adoption into the rest. | ||
| let results: ReturnType<typeof adoptSkills>; | ||
| try { | ||
| results = adoptSkills(canonicalPath, target); |
There was a problem hiding this comment.
When --adopt is used on a harness dir that is still recorded as a managed copy in .skdd-sync.json, this successful additive copy leaves that state entry behind. If the user added non-colony skills and later runs plain skdd link, the managed-copy refresh path forces a full replacement and deletes those preserved skills, contradicting the adopt contract that adopted copies are not tracked; remove or rewrite the existing mirror entry when adoption takes over a target.
Useful? React with 👍 / 👎.
1 participant
What
Adds
skdd link --adopt: an additive way to get the colony's skills into a harness dir that already holds non-colony skills (the common global~/.claude/skills,~/.codex/skillscase).Today
skdd link -grefuses those dirs (won't force-symlink over user data) and--forcewould replace (delete) them.--adoptinstead copies each colony skill into the dir, leaving every non-colony skill untouched.Per skill: created if absent · unchanged if byte-identical · skipped-divergent if it differs from the colony (add
--forceto overwrite a colony skill that diverges — a foreign skill is never touched). Dirs already symlinked to the colony are detected and skipped. Adopted copies are intentionally not tracked in.skdd-sync.json— it's a one-way push, not a managed whole-dir mirror.Why
The global colony's headline is 'one canonical place, every harness sees it', but on a real machine most harness global dirs are populated real directories (hand-authored skills that live in no colony). The symlink mirror can't be applied there without
--forcedeleting them — exactly the operational guardrail in the repo.--adoptcloses that gap safely.Files
cli/src/lib/fs-link.ts—adoptSkills()(pure, testable core)cli/src/commands/link.ts—runAdopt()reached via the new--adoptflag (works with-gand project scope)cli/src/index.ts— flag wiring;cli/README.md— docsTests
6 new in
cli/test/adopt.test.ts(create / unchanged / skipped-divergent / --force overwrite / preserves foreign skills / skips colony symlink). Full suite 949 green, typecheck + lint clean.Verified live
Ran
skdd link -g --adoptagainst this machine's global colony: 39 skills added across 6 populated harness dirs (claude/codex/opencode/goose/amp/droid), 0 divergent, foreign skills preserved, every dir still a real directory (not replaced). The 3 already-symlinked harnesses were detected and skipped.