fix(skills): include plugin skill roots safely

[AaronZ345]

Supersedes #1573 with the same reviewed patch on a fresh branch after the old PR crossed the 30-day maintenance threshold. #1573 itself replaced #1415.

Summary

• add a small shared scanner that discovers plugin-provided skills root directories without registering nested SKILL.md files directly
• include Codex plugin skill roots (CODEX_HOME/plugins/**/skills) plus Codex superpowers and .claude/skills compatibility roots
• include Claude Code plugin skill roots from CLAUDE_CONFIG_DIR/plugins/**/skills and configured plugin_dir trees

This also supersedes the approach in #624. That PR became stale/dirty after #1317 and reintroduced recursive core skill registration. This patch keeps #1317's depth-1 registry behavior intact, so nested reference/template SKILL.md files stay hidden while plugin skill bundles become discoverable.

It complements #1325: plugin_dir still loads Claude plugins at runtime, while this PR makes plugin-provided skill roots visible to cc-connect's static skill/command registry.

Review disposition

• the owner review on fix(skills): include plugin skill roots safely #1573 concluded that the implementation is fundamentally sound and explicitly found no blocker
• there are no unresolved review threads, CHANGES_REQUESTED, or actionable comment candidates
• the design-confirmation questions and minor dedup note were marked non-blocking and do not require a code change for this replacement

Verification

• old and replacement trees are identical (git diff --exit-code)
• both commits are equal under git range-diff
• git diff --check
• go test ./agent/internal/skillroots ./agent/codex ./agent/claudecode ./core
• go test -tags no_web ./cmd/cc-connect reproduces the existing macOS CheckLinger redeclared failure on pure origin/main
• GOOS=linux CGO_ENABLED=0 go test -tags no_web -c ./cmd/cc-connect
• pushed with an absent-branch lease and verified local/fork parity 0 0

[chenhg5]

QA Review — PR #1713 (Plugin skill roots)

Conclusion: Approve

Overall assessment: Replacement of a previously-reviewed patch (#1573, which had owner sign-off "fundamentally sound, no blocker"). The new head a9a3a4e carries an identical tree (git diff --exit-code + git range-diff equal), so this review focuses on confirming the diff still solves the original problem without re-introducing the nested-SKILL.md regression that killed #624 and #1415. CI is 5/5 SUCCESS, merge state CLEAN.

Review scope:

• agent/internal/skillroots/skillroots.go: +68 (new shared scanner)
• agent/internal/skillroots/skillroots_test.go: +66 (3 tests incl. symlink loop guard)
• agent/claudecode/claudecode.go: +46/-40 (SkillDirs plumbing + plugin roots)
• agent/claudecode/skilldirs_test.go: +80 (3 new tests incl. symlink + nested-assets guard)
• agent/codex/codex.go: +15/-5 (Codex plugin roots)
• agent/codex/skilldirs_test.go: +80 (Codex-side coverage)
• agent/codex/session_test.go: +11 (sig updates)
• CHANGELOG.md: +1
• Focused on: scanner depth-limit, symlink-loop handling, depth-1 enforcement invariant, and per-agent wiring.

✅ What looks good:

• Shared scanner agent/internal/skillroots is the right shape. One recursive walker that returns directories named skills, with realDir symlink resolution and a seenDirs set keyed on real paths → guaranteed termination on symlink loops. Both claudecode and codex reuse it instead of duplicating logic.
• Depth-1 invariant preserved at the registry layer. The new scanner only emits "skill roots"; it does not enumerate SKILL.md files. core.SkillRegistry continues to walk each root one level deep for <root>/<skill>/SKILL.md. This is exactly the boundary that broke in #624/#1415, and the new tests (TestSkillDirs_NestedAssetsSkipped for both agents) explicitly assert that references/template/skills inside a plugin skill bundle does not appear in SkillDirs().
• Symlink handling is tested at both the scanner level (TestFindDoesNotLoopOnSymlink) and the integration level (TestSkillDirs_FollowsClaudePluginSymlink, TestSkillDirs_FollowsCodexPluginSymlink). This covers the realistic shape of ~/.claude/plugins and ~/.codex/plugins being symlinked to a config-dir override.
• Per-agent integration is minimal and additive. claudecode.go calls skillroots.Find(filepath.Join(configHome, "plugins")) plus each configured pluginDir; codex.go does the equivalent on CODEX_HOME/plugins. No existing call sites are removed or replaced.
• uniqueSkillDirs still runs at the end, so a plugin root and a project root that resolve to the same path collapse to one entry — no risk of duplicate SKILL.md registration.
• CHANGELOG entry is concise and accurate.

🟠 Should improve (non-blocking):

• skillroots.Find returns directories in filesystem walk order, not a stable sort. If the core registry's iteration order ever becomes user-visible (e.g. /skills list ordering, help text), this will produce flaky UI ordering. Consider sorting the result by filepath.Clean to make the order deterministic. Trivial change, low risk.

🔵 Optional (non-blocking):

• The //go:build windows skip in the symlink tests is correct for the test environment but means the symlink-loop guard is not exercised on Windows CI. A comment near realDir noting "the test guard skips Windows; on Windows the symlink case is covered manually or by follow-up" would help future contributors.
• The CHANGELOG entry could call out that this complements #1325 (Claude plugin_dir runtime) and supersedes #624/#1415 for any readers going via changelog-only — but the PR body already does that.

❓ Questions: None.

Testing / Risk:

• Verified evidence: CI 5/5 SUCCESS (lint, unit-test, smoke-test, regression-test, performance-test) on head a9a3a4e. Author confirms go test ./agent/internal/skillroots ./agent/codex ./agent/claudecode ./core green.
• Pre-existing root origin/main Windows compile failure (CheckLinger redeclared, agent/pi/proc_windows.go os unused) is not caused by this PR; correctly excluded from scope.
• Remaining risk:
  1. Walk-order non-determinism noted above; would only matter if the registry ever exposes ordering.
  2. Permission errors during walk silently return (no error surfaced). For a Claude/Codex plugin root under ~/.claude/plugins the scanner expects readable paths; unreadable subdirs are skipped. This matches the project's existing skill-discovery convention but means a misconfigured plugin is invisible rather than reported. Acceptable.
  3. Replacement PR churn: this is the third attempt at this fix (#624 → #1415 → #1573 → #1713). The history suggests the area is sensitive to refactors that touch SkillRegistry; the new shared skillroots package is a good structural move that should make future regressions easier to pin.

Next step: Recommend merge. The diff is a faithful re-attempt of an already-approved patch with the symlink + nested-assets guards tightened up. The core invariant (depth-1 SKILL.md registration, no nested-templates leak) is asserted by the new tests, and the shared scanner prevents future drift between claudecode and codex.