runkids/skillshare/proposals

Feature Proposal: Move an installed skill into another folder

Issue: #510. Related: #247 (second half asks for the same), #108 (folder view; move was a non-goal there). Depends on: PR #513 (fix/install-missing-moved-skill), assumed merged first.

Problem

Install records live in one .metadata.json keyed by source-relative path, so moving an installed skill with mv strands its record at the old key (doctor: "N entries ... missing on disk"; dashboard: "Install missing"). website/docs/how-to/daily-tasks/organizing-skills.md now tells users to uninstall + install --into, which re-downloads, drops audit acceptances, and in project mode rewrites the lockfile pin. Dashboard "into folder" in InstallDialog is free text, so users mistype or cannot see existing folders.

PR #513 makes reconcile follow a plain mv by hash matching (MetadataStore.MovedEntryKey, MoveEntry). Its own "Not covered" list is this feature's reason to exist: records without file_hashes, skills edited after the move, and no way to do it from the dashboard. Hash inference is a recovery path; an explicit move should not depend on it.

Proposed Solution

One core operation, exposed as skillshare move and POST /api/resources/batch/move, plus two dashboard entry points. A move takes skills or whole folders; the verb is move because every existing command is a full word (mv, rename would be the odd ones out).

Core: internal/skillmove (new, mirrors internal/uninstall)

Plan(discovered, names, dest, Options) []Planned resolves and refuses without touching disk. Run([]Planned, Options) Outcome applies, with per-item results like uninstall.Run.

A name that resolves to a skill moves that skill, plus any skills nested below its directory (discovery keeps walking after a SKILL.md, so foo/SKILL.md and foo/sub/SKILL.md are both discovered; the rename carries both, so both need their records moved). A name that is a folder under the source moves the folder with every skill under it, keeping its base name: move frontend archive gives archive/frontend/.... Plan expands either into one Planned per discovered skill below it and refuses every one of them before any rename; a single refusal aborts the whole folder, so a folder never ends half-moved. Because discovery only sees skills, Plan also walks the whole directory tree of a skill or folder for _-prefixed tracked checkouts (even one with no discoverable skill) and followed links, and refuses on the first. The folder is then renamed once (Root.Rename), so empty subfolders and non-skill files move along; the per-skill steps below (store, lock, .skillignore, .gitignore) run for each skill, and the rename happens once per requested name.

Per skill, in this order:

1. sourcefs.MkdirAllIn(source, dest, follow) (as handler_install.go does for --into), then Root.Rename(from, to) (internal/sourcefs/sourcefs.go), with from the skill or the folder. Rename already refuses a link component and a rename across a followed link.
2. If the skill has a record: store.MoveEntry(oldKey, to) from PR #513. MoveEntry re-keys the entry and its AuditAccepted list but leaves Group to the caller. Do not copy reconcile's group snippet: save the store, then run the existing reconcile (config.ReconcileGlobalSkills / ReconcileProjectSkills), which sets Group, prunes, and in project mode rewrites config.yaml skills and calls WriteProjectLock.
3. Project mode, before that reconcile: carry the lock pin. writeProjectLock never moves a pin, so without this the pin would be re-created from the installed commit, losing a deliberately older pin. New Lock.MovePin(old, new) in internal/install/lock.go, saved with install.LoadLock(projectdir.Resolve(root)); pruneProjectLock in cmd/skillshare/uninstall_project.go is the precedent.
4. .skillignore: if a literal line equals the old relPath, skillignore.RemovePattern + AddPattern (internal/skillignore/write.go) with the new one. Glob rules are left alone; re-run sync.DiscoverSourceSkillsAll and warn when Disabled flipped (as toggleOverrideError does).
5. Project .gitignore: swap the old entry for the new one via config.ProjectGitignoreTarget + install.RemoveFromGitIgnoreBatch (reconcile's onFound only adds). Whether project uninstall removes skill lines today is unverified; check before coding.

The reconcile walk stops at an installed parent (reconcile_core.go: existing.Source != "" returns SkipDir) and pruneStaleEntries then drops every record absent from that walk, so a re-keyed nested record under an installed parent would be pruned with its audit acceptances. Step 2 therefore also changes reconcile to keep records whose key lies below a live installed record (own test in internal/config), rather than bypassing the prune.

Order is rename first, store second: a failed rename changes nothing; a failed store save after a rename is healed by #513's reconcile when hashes exist, and reported otherwise.

MetadataStore has no exported "key for relPath" (GetByPath returns the entry only), so add KeyForPath(relPath) string matching KeyToRelPath(key, entry); needed for legacy basename keys that carry a Group.

Refusals (stable Code, shared by CLI and API):

Empty folders and non-skill files inside a moved folder are not refusal reasons; they move along with it.

Extract the tracked-repo ancestor loop from uninstall.checkMoveOut into an exported helper in internal/install (next to IsTrackedCheckout) so both packages share it.

Name collisions: target names are FlatName (grp__demo) or SKILL.md names, depending on target_naming. For each target run sync.ResolveTargetSkillsForTarget on the discovered list with the moved skills rewritten, and report Collisions that include a moved path. Identical destination paths are duplicate_dest, checked first, and never reach this step. The flat branch of that function returns before collision detection (verified, internal/sync/target_naming.go), so also compare FlatName duplicates directly. A collision does not lose data (sync skips both entries), so --force accepts it. That is the only thing --force does.

Target filters are not rewritten. include/exclude match flat names (internal/sync/filter.go), so a rule naming demo stops matching grp__demo. Preflight evaluates each target's effective filter (ShouldSyncFlatName) for the old and the new flat name and warns whenever the result differs in either direction (a rule naming demo stops matching; an exclude: archive__* newly drops it; an include newly adds it); sync already warns on unmatched includes (UnmatchedInclude.Warning).

CLI: skillshare move <skill|folder>... <dest-folder>

rg '"mv"|"move"|"rename"' cmd/ finds only TUI key hints ("↑↓ move"), no command. move is free in the commands map in cmd/skillshare/main.go. The existing verb pair link/unlink, enable/disable has no conflict.

• Last positional is the destination, relative to the skills source; . moves to the root. At least two positionals. A folder argument moves under the destination (frontend archive → archive/frontend); a trailing / is accepted.
• Flags: --dry-run/-n, --force/-f (collision only), --json, -p/-g via parseModeArgs, --help. No --kind: agents are out of scope (see below).
• Handler split: move.go (parse, mode), move_handlers.go (resolve, render), project differences only where config.LoadProject paths differ, as uninstall_project.go.
• Output with ui.Row(ui.MarkOK, name, "→ grp/demo"), ui.Done, then ui.Next("skillshare sync", "rename the links in your targets"). --dry-run uses ui.DryRun() and prints the same rows.
• --json: {"moved":[{"name","from","to","record":bool,"skills":n}],"failed":[{"name","code","error"}],"skipped":0,"warnings":[],"dry_run":false,"duration":"..."}, shaped like uninstallJSONOutput; skills is 1 for a skill and the count below a folder. Non-zero exit when failed is non-empty.
• oplog: oplog.NewEntry("move", status, d), args names, dest, force, mode; written with oplog.Write(cfgPath, oplog.OpsFile, e) as enable.go does. No command allow-list was found in cmd/skillshare/log*.go (unverified for the UI log filters).
• Completions: all five completion_{bash,zsh,fish,powershell,nushell}.go plus a literal in tests/integration/completion_test.go.

Sync is not automatic. install, uninstall, enable, disable and link all print ui.Next("skillshare sync") (cmd/skillshare/install_handlers.go, enable.go, link.go); sync touches every target and can refuse (symlink conflicts need --force, a different decision from move's). Until sync runs, the old target link is dangling, as after uninstall; the dashboard result view offers Sync now.

Web API

POST /api/resources/batch/move, registered next to batch/targets and batch/toggle in internal/server/server.go; handler in new internal/server/handler_move.go (split follows handler_toggle.go, handler_skills_batch.go).

// request
{ "names": ["demo", "frontend/other", "archive-me"], "dest": "grp", "force": false, "dryRun": false }
// 200 response (partial failure is still 200, like /api/uninstall/batch)
{ "results": [{ "name": "demo", "success": true, "from": "demo", "to": "grp/demo",
                "flatName": "grp__demo", "record": true },
              { "name": "x", "success": false, "error": "...", "error_code": "dest_exists" }],
  "summary": { "succeeded": 1, "failed": 1 }, "warnings": [], "dryRun": false }