feat: adopt v2

[runkids]

ref #135

[gemini-code-assist]

Code Review

This pull request introduces the 'Adopt CLI-bundled skills' feature, adding a new adopt command to the CLI and a corresponding page to the web dashboard. This feature allows users to claim unmanaged skills installed by external tools, migrating them to the source of truth, cleaning up orphan symlinks, and re-syncing targets. The review feedback highlights a critical concurrency issue in the server handler where a write lock is held during disk I/O, a parsing edge case in the lockfile reader when handling flat structures with a skill named 'skills', and unsafe type assertions on caught exceptions in the React frontend.

[gemini-code-assist]

Holding the write lock s.mu.Lock() during the entire handleAdoptApply handler is a significant performance bottleneck and a potential Denial of Service (DoS) vulnerability. The lock is held while decoding the request body from the network socket (which can be slow) and during the entire adopt.Apply operation, which performs heavy disk I/O (copying files, trashing files, pruning symlinks, and re-syncing targets). Since adopt.Apply does not modify any in-memory server state, it does not need to run under the write lock.

We should decode the request body first, then acquire a read lock (s.mu.RLock()) to safely snapshot the required configuration and targets, and finally release the lock before executing the adopt flow.

	start := time.Now()

	var body struct {
		Names  []string `json:"names"`
		Force  bool     `json:"force"`
		DryRun bool     `json:"dryRun"`
	}
	if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
		writeError(w, http.StatusBadRequest, "invalid request body: "+err.Error())
		return
	}

	s.mu.RLock()
	source := s.skillsSource()
	globalMode := s.cfg.Mode
	targets := s.cloneTargets()
	isProject := s.IsProjectMode()
	projectRoot := s.projectRoot
	s.mu.RUnlock()

	agentsTarget, ok := findAgentsTarget(targets)
	if !ok {
		writeError(w, http.StatusBadRequest, "universal/agents target not configured; nothing to adopt")
		return
	}
	sc := agentsTarget.SkillsConfig()

	allTargets := make(map[string]string, len(targets))
	for name, t := range targets {
		allTargets[name] = t.SkillsConfig().Path
	}

	syncMode := ssync.EffectiveMode(sc.Mode)
	if sc.Mode == "" && globalMode != "" {
		syncMode = globalMode
	}

	trashBase := trash.TrashDir()
	if isProject {
		trashBase = trash.ProjectTrashDir(projectRoot)
	}

[gemini-code-assist]

If the lockfile uses a flat structure (Shape b) but happens to contain a skill named "skills", the current implementation will attempt to parse its value as a nested map (map[string]map[string]any). This will fail with an unmarshaling error and cause the entire lockfile reading to fail, even though the lockfile is perfectly valid.

Instead of returning an error immediately when nested unmarshaling fails, we should degrade gracefully and fall back to treating the lockfile as a flat map.

	if skillsRaw, ok := top["skills"]; ok {
		var nested map[string]map[string]any
		if err := json.Unmarshal(skillsRaw, &nested); err == nil {
			for name, raw := range nested {
				entries[name] = LockEntry{
					Name:       name,
					SourceTool: sourceToolFromRaw(raw),
					Raw:        raw,
				}
			}
			return entries, nil
		}
	}

[gemini-code-assist]

Casting e as Error directly and accessing .message is unsafe because the caught exception e can be of any type (e.g., a string or a custom error object). If e is not an instance of Error, this can lead to runtime type errors or display "undefined". We should safely extract the error message by checking if e is an instance of Error.

Suggested change

	} catch (e: unknown) {
	toast((e as Error).message, 'error');
	setPhase('loaded');
	setCandidates([]);
	}
	} catch (e: unknown) {
	toast(e instanceof Error ? e.message : String(e), 'error');
	setPhase('loaded');
	setCandidates([]);
	}

[gemini-code-assist]

Casting e as Error directly and accessing .message is unsafe because the caught exception e can be of any type. We should safely extract the error message by checking if e is an instance of Error.

Suggested change

	} catch (e: unknown) {
	toast((e as Error).message, 'error');
	setPhase('loaded');
	}
	} catch (e: unknown) {
	toast(e instanceof Error ? e.message : String(e), 'error');
	setPhase('loaded');
	}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 500a98b3b7

ℹ️ 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".

[chatgpt-codex-connector]

Do not forward adopt force to copy target sync

When skillshare adopt --force is used with any copy-mode target, this re-sync passes the adopt force flag into SyncTargetCopy; in that function, force means overwriting every unmanaged local directory/non-directory that collides with any source skill, not just the skills being adopted. A user using --force to resolve an adopt source conflict can therefore unexpectedly wipe unrelated local copies in copy-mode targets, while the adopt help only advertises overwriting same-name skills in source.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Suppress sync diagnostics before emitting JSON

In --json mode this call can run the prune/re-sync path before the JSON payload is written, and those sync helpers write diagnostics to sync.DiagOutput, which defaults to stdout (for example naming-validation or collision messages from copy/merge sync). In those target/source configurations, skillshare adopt --json produces non-JSON text before the JSON object, breaking consumers that parse stdout as JSON; mirror the sync command’s behavior by discarding diagnostics while JSON mode is active.

Useful? React with 👍 / 👎.