[sdhilip200]

Summary

Settings → Orchestration → Agent coverage decided whether each detected agent had the orchestration skill by matching a skill's single rootPath against a hand-maintained, renderer-side table of path segments (.claude/skills, .grok/skills, .gemini/antigravity/skills, …).

Skill discovery dedups skills by canonical file path. When one provider home is a symlink onto another agent's skills directory — npx skills add --global does exactly this — the two collapse into a single row and the absorbed roots survive only in rootPaths, which the old matcher ignored. With ~/.grok/skills symlinked onto ~/.claude/skills, discovery returns one row with rootPath = ~/.claude/skills, so Claude read Ready and Grok read Missing even though Grok loads the very same files.

• classify every entry in rootPaths, not just the root that happened to win dedup
• resolve each root through the SkillDiscoverySource the scanner already returns, accepting it when its sourceKind is not repo and its owner is either the agent's owner or null (the shared ~/.agents/skills scope)
• delete ORCHESTRATION_SKILL_LOCATIONS and ORCHESTRATION_SKILL_LOCATION_IDS_BY_AGENT — a renderer-side mirror of buildSkillDiscoverySources that had to be hand-edited for every new provider root, which is what caused this class of bug in the first place (orchestration-skill-coverage.ts: 197 → 89 lines)
• plumb sources from useInstalledAgentSkill → OrchestrationPane → OrchestrationSkillAgentCoverage

No discovery roots change here. The Grok/OpenCode/Pi/Antigravity/Cursor scan roots already landed in #8510; this PR touches no main-process production file. It fixes the coverage half that #8510 left behind.

User-visible changes

Setup	Before	After
Provider home symlinked onto another agent's skills dir	aliased agent Missing	Ready — the bug
Orchestration skill inside an enabled Claude plugin	Missing	Ready
~/.agents/skills only · ~/.claude/skills only · repo-local .agents/skills · Codex plugin cache · Gemini vs Antigravity · agents with no dedicated root	—	unchanged

The Claude plugin flip is a deliberate consequence of trusting the scanner's ownership data: Claude Code does load skills from enabled plugins, and the Codex plugin cache was already treated this way. Excluding it would mean reintroducing the allowlist this PR removes. It is only reachable in Settings on a WSL project, since the native Settings path sends no cwd and plugin roots are cwd-gated. Pinned by a test.

Fixes made during review

• Restored the required { kind: 'local' } argument to useDetectedAgents. Dropping it failed pnpm typecheck (TS2554), and at runtime an absent target means "host not yet known", pinning detectedIds to null so the widget sat on "Checking installed agents and skill paths…" forever. It also reverted the remote-host scoping from fix(agents): scope Settings agent list and quick-launch menu to the remote-server host #9790. The component test's mock ignored its arguments, so nothing caught it; it now asserts the target.
• Stopped a duplicate repo root from shadowing an owning home root. SkillDiscoverySource.path is not unique: when the scan's cwd is the home directory, ~/.claude/skills and ~/.agents/skills are each scanned as both a home root and a repo root. Indexing sources into a path-keyed Map is last-write-wins, and sources is label-sorted, so Repo … .claude always won and its repo kind then excluded the root — every agent read Missing while the panel directly above still said Installed. This was guaranteed in Settings on a WSL project, where resolveSkillDiscoveryTarget defaults cwd to the distro home. Coverage now scans for a matching owning root instead of keying by path.
• Pinned the discovery contract coverage now depends on: every deduped rootPath must key an owning source, source owners must name real agent ids (AgentType widens to string, so a typo would read Missing forever with no type error), and a home-equal workspace path must still yield both a home and a repo root.
• Carried OMP coverage in the new shape so Add OMP skill discovery source #6422's coverage-table hunks can be dropped wholesale on rebase, including the Pi/OMP distinction that PR guards.

Refs #8256

Testing

• pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/lib/orchestration-skill-coverage.test.ts src/renderer/src/components/settings/OrchestrationSkillAgentCoverage.test.tsx src/renderer/src/components/settings/OrchestrationPane.test.tsx src/main/skills/discovery.test.ts — 38 passed
• 608 test files / 5345 tests across src/main/skills, settings, hooks, native-chat, lib
• pnpm typecheck, pnpm lint
• Verified in a dev build: coverage resolves instead of spinning; reverting the useDetectedAgents fix reproduces the permanent spinner. Live discovery on a symlink-heavy home returns rootPaths spanning .agents, .claude, .grok, and .pi, each resolving to a correctly-owned source.
• Reproduced the original failure against real discoverSkills: with ~/.grok/skills symlinked onto ~/.claude/skills, main gives claude: true / grok: false; this branch covers both.

---

Original fix by @sdhilip200; review fixes pushed on top.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added global skill discovery roots for Grok, OpenCode, Pi, Antigravity, and Cursor. Extended orchestration-skill location identifiers, path matching rules, and agent mappings for these providers. Added tests covering generated discovery paths and orchestration-skill detection in each provider-specific global directory.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The code adds provider roots and coverage mappings for the affected agents, with tests verifying symlinked skill detection, matching #8256.
Out of Scope Changes check	✅ Passed	The changes stay focused on skill discovery and coverage; no unrelated behavior appears to have been added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title is concise and accurately reflects the main change: handling symlinked orchestration skill coverage.
Description check	✅ Passed	The description is detailed and on-topic, but it omits the template's Screenshots, AI Review Report, Security Audit, and Notes sections.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[brennanb2025]

Taking a look!

[brennanb2025]

Thanks for the contribution! I rebased the branch onto current main, which had since gained the provider-home discovery roots, and replaced the remaining path-pattern coverage logic with discovery source ownership metadata. Coverage now preserves every contributing root after canonical symlink deduplication, keeps repo-scoped roots from being counted as global installs, and handles shared roots plus Claude aliases without provider leakage. Validated with 49 focused tests, touched-file formatting and lint (including type-aware and React checks), full typecheck, and the max-lines ratchet.

[sdhilip200]

Rebased onto current \ and resolved the conflict in \ by keeping the newer agent-detection call shape together with the skill-source wiring from this PR. Force-pushed the updated branch. I couldn’t run the targeted Vitest/typecheck commands in this worktree because the checkout here does not have installed Node dependencies.

[brennanb2025]

Thanks — the direction here is right, and I've pushed three fixes on top.

Replacing the renderer's hardcoded path-segment table with the owner the scanner already publishes removes a second source of truth that had to be edited every time buildSkillDiscoverySources gained a root. That's the real win, and it's what makes the symlink case work.

Confirmed the bug is real. With ~/.grok/skills symlinked onto ~/.claude/skills, real discoverSkills dedups to one row (rootPath = ~/.claude/skills, rootPaths holding both), and on main grok reads Missing while claude reads Ready. This branch covers both.

Three fixes pushed:

1. useDetectedAgents() lost its argument (OrchestrationSkillAgentCoverage.tsx). The parameter is required, so pnpm typecheck failed outright; at runtime undefined means "host not yet known", which pins detectedIds to null and leaves the widget stuck on "Checking installed agents and skill paths…" forever. It also reverted the remote-host scoping from fix(agents): scope Settings agent list and quick-launch menu to the remote-server host #9790. Restored { kind: 'local' } and added an assertion for it — the existing test's useDetectedAgents mock ignored its arguments, which is why nothing caught this.

2. Regression vs main: a duplicate repo root shadowed the owning home root. Coverage keyed sources into a Map by path, but one path can carry several sources — when the scan's cwd is the home directory, ~/.claude/skills and ~/.agents/skills are each scanned as both a home root and a repo root. sources is label-sorted, so Repo … .claude always landed last and won the Map, and its repo kind then excluded the root: every agent read Missing while the panel directly above still said Installed. This is guaranteed in Settings on a WSL project, where resolveSkillDiscoveryTarget defaults cwd to the distro home. Fixed by scanning sources.some(...) for a matching owning root instead of keying by path — which also let the Map and the two-function split go.

3. Pinned the contract the renderer now depends on. Owners must name real agent ids (AgentType widens to string, so a typo would read Missing forever with no type error), a home-equal workspace path must still yield both a home and a repo root, and every deduped rootPath must key an owning source.

Behavior checked against main across ten setups — ~/.agents/skills only, ~/.claude/skills only, symlinked provider home, repo-local, both plugin caches, gemini vs antigravity, cwd == home, and agents with no dedicated root. Only two flips, both correct: the symlinked provider home (the bug), and Claude plugin roots now counting for Claude, which matches how the Codex plugin cache was already treated and is only reachable on WSL. Added a test pinning that one so it's intentional.

Please rewrite the description before this merges — it becomes the squash commit message. It currently claims to add the Grok/OpenCode/Pi/Antigravity/Cursor discovery roots, but those landed in #8510 and this diff touches no discovery roots at all. Closes #8256 is also stale; #8256 is already closed.

Validated in a dev build: coverage resolves to "All 7 detected agents have the skill", and reverting fix (1) reproduces the permanent spinner. Live discovery on a symlink-heavy home returns rootPaths spanning .agents, .claude, .grok, and .pi, each resolving to a correctly-owned source.

[greptile-apps]

Greptile Summary

This PR replaces the hand-maintained ORCHESTRATION_SKILL_LOCATIONS / ORCHESTRATION_SKILL_LOCATION_IDS_BY_AGENT tables with a source-aware lookup that classifies every path in skill.rootPaths (not just the deduplicated winner) against the SkillDiscoverySource entries the scanner already returns. This correctly handles the symlink case where npx skills add --global collapses two provider roots into one skill row.

• orchestration-skill-coverage.ts shrinks from 197 → 89 lines by deleting the path-segment allowlist and replacing it with a sources.some(...) check that respects sourceKind !== 'repo' and owner equality, fixing both the symlink-dedup bug and the class of bugs caused by the allowlist drifting from buildSkillDiscoverySources.
• useInstalledAgentSkills.ts / OrchestrationPane.tsx / OrchestrationSkillAgentCoverage.tsx plumb the sources array up from the hook through to the coverage widget, and the component test is strengthened to assert the required { kind: 'local' } argument to useDetectedAgents that prevented the widget from spinning forever when dropped.
• Three new integration tests in discovery.test.ts pin the discovery contract the renderer now depends on: every deduped rootPath has an owning source, every source owner names a real agent id, and a home-equal workspace path produces both a home and a repo source.

Confidence Score: 5/5

Safe to merge — the change is entirely renderer-side display logic with no main-process production file changes and a comprehensive test suite.

The rewrite correctly removes the hand-maintained path-segment allowlist and replaces it with a source-owner lookup that naturally handles symlink deduplication. The three new main-process discovery tests pin exactly the contract the renderer now relies on. The restored { kind: 'local' } argument to useDetectedAgents and the upgraded component test that now asserts it fix a real regression that would have caused an infinite spinner. No correctness issues found.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
src/renderer/src/lib/orchestration-skill-coverage.ts	Core logic rewritten to use sources from discovery scan; removes brittle path-segment allowlist. Logic is clean, well-commented, and covers repo-root shadowing and multi-root deduplication correctly.
src/renderer/src/lib/orchestration-skill-coverage.test.ts	Comprehensive test updates; adds cases for symlink dedup, repo-root shadowing, home+repo duplicate path, unmatched sources, plugin roots, Windows paths, and all provider pairs.
src/renderer/src/hooks/useInstalledAgentSkills.ts	Minimal additive change: exposes sources from SkillDiscoveryResult via the existing resultForRender memoization, consistent with how skills is already plumbed.
src/renderer/src/components/settings/OrchestrationPane.tsx	Destructs sources from the hook and forwards it to OrchestrationSkillAgentCoverage; no logic changes.
src/renderer/src/components/settings/OrchestrationSkillAgentCoverage.tsx	Accepts the new sources prop, restores required { kind: 'local' } to useDetectedAgents, and forwards sources to getOrchestrationSkillAgentStatuses.
src/renderer/src/components/settings/OrchestrationSkillAgentCoverage.test.tsx	Mock promoted to a named spy; test now asserts useDetectedAgents receives { kind: 'local' }, catching the dropped-argument regression that caused an infinite spinner.
src/renderer/src/components/settings/OrchestrationPane.test.tsx	Mock updated to include the new sources field; change is mechanical and correct.
src/main/skills/discovery.test.ts	Three new integration tests pin the discovery contract the renderer depends on; symlink dedup, agent-id validity, and home-equal-cwd dual-root shape are all covered.

_{Reviews (2): Last reviewed commit: "Merge main (#6422 OMP discovery) into sy..." | Re-trigger Greptile}

[brennanb2025]

⚠️ Before merging: the squash message must carry a Co-authored-by trailer

Whoever clicks merge, please add this line to the end of the commit message body:

Co-authored-by: sdhilip200 <49802211+sdhilip200@users.noreply.github.com>

Why: no commit currently on this branch is authored by @sdhilip200, so as it stands the merge lands on main with no attribution to them at all. GitHub reports the author of all six commits — including the original e12c8b55ba — as brennanb2025:

$ gh pr view 8329 --json commits --jq '[.commits[].authors[0].login] | unique'
["brennanb2025"]

On e12c8b55ba specifically, @sdhilip200 is the committer but not the author:

$ git log -1 --format='author: %an <%ae>%ncommitter: %cn <%ce>' e12c8b55ba
author:    Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
committer: Dhilip Subramanian <49802211+sdhilip200@users.noreply.github.com>

I can't tell from the history how the author field ended up that way, and it doesn't much matter — nothing here is the contributor's doing, and the fix costs one line. @sdhilip200 opened this PR, wrote the original fix and its description, and should be credited on main.

No force-push is needed. Merge commits are disabled on this repo and squash_merge_commit_message is COMMIT_MESSAGES, so the body is editable in the merge dialog — the trailer can simply be appended there. Rebase merge is also enabled; if that path is taken instead, the trailer still needs to be in the message, since rebasing preserves the existing author fields and would keep the attribution as-is. Rewriting history on the contributor's fork to re-author the commit is not worth doing for this.

This is the last point at which their name can be attached, so please don't skip it.

---

CI is green on 9e91e936b6 — all jobs success, e2e skipped by path filter.