Repository navigation
fix(agents): scope Settings agent list and quick-launch menu to the remote-server host - #9790
Merged
Merged
Conversation
…emote-server host With a paired Remote Server as the Active Server, Settings → Agents and the tab-bar + quick-launch items always ran agent detection on the local client's PATH, so a Windows client showed its own agents while worktree-create correctly listed the server's. - Extract TabBar's ssh/runtime/local owner resolution into a shared useAgentDetectionTargetForWorktree hook and use it in QuickLaunch, which previously resolved only SSH connections and fell back to local for paired-runtime worktrees. - Scope AgentsPane detection (and its Refresh button) to the Active Server, with an "on <server>" badge showing which host the list came from. Enable/disable/default toggles remain client-side settings. - Split runtime detection into store/slices/runtime-detected-agents.ts and add refreshRuntimeDetectedAgents: preflight.refreshAgents over the relay (login-shell PATH re-read), falling back to preflight.detectAgents on servers that predate the refresh RPC, keeping the last known list when the runtime is unreachable.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAgent detection now supports local, SSH, and runtime targets through a shared worktree resolver. Runtime agent state moved into a dedicated Zustand slice with cached results, refresh and detection flags, in-flight deduplication, cleanup, and fallback detection. Settings and tab-bar components pass explicit targets to 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/renderer/src/store/slices/runtime-detected-agents.ts (1)
48-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the repeated "commit-if-still-current" guard.
The
if (runtime{Detect,Refresh}Promises.get(environmentId) === pending) { set(...) }pattern is duplicated four times (two inensureRuntimeDetectedAgents, two inrefreshRuntimeDetectedAgents). A small shared helper (e.g.commitIfCurrent(map, id, pending, updater)) would reduce the risk of the guard being dropped or diverging in a future edit.src/renderer/src/store/slices/detected-agents.test.ts (1)
565-676: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding coverage for
ensureRuntimeDetectedAgents,clearRuntimeDetectedAgents, andretainRuntimeDetectedAgents.This file only exercises
refreshRuntimeDetectedAgents. The other three exported actions contain the empty-result re-detect logic, the mid-flight-removal guards, and the pruning behavior for removed environments — all non-trivial and worth a dedicated test each (e.g., detect dedup, clear during in-flight detect, retain pruning both cache and in-flight maps).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e8313b5a-3cba-423f-bbfa-84e11d66a887
📒 Files selected for processing (19)
src/renderer/src/components/settings/AgentsPane.test.tsxsrc/renderer/src/components/settings/AgentsPane.tsxsrc/renderer/src/components/tab-bar/QuickLaunchButton.test.tssrc/renderer/src/components/tab-bar/QuickLaunchButton.tsxsrc/renderer/src/components/tab-bar/TabBar.tsxsrc/renderer/src/hooks/useAgentDetectionTarget.tssrc/renderer/src/hooks/useDetectedAgents.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/store/index.tssrc/renderer/src/store/slices/detected-agents.test.tssrc/renderer/src/store/slices/detected-agents.tssrc/renderer/src/store/slices/diffComments.test.tssrc/renderer/src/store/slices/runtime-detected-agents.tssrc/renderer/src/store/slices/store-test-helpers.tssrc/renderer/src/store/types.ts
💤 Files with no reviewable changes (1)
- src/renderer/src/store/slices/detected-agents.ts
AmethystLiang
added
the
orca-remote-server
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/hooks/useDetectedAgents.ts (1)
106-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
refresh()silently targets local when target is still unresolved (isUnknown).
isUnknown(targetundefined, i.e. "not yet hydrated") is checked fordetectedIds/isLoading, butrefresh()has no such guard — it falls through the runtime/ssh checks straight torefreshLocal(). If a caller invokesrefresh()during that hydration window, it refreshes the wrong host, exactly the "silent no-op" the inlineWhycomment above warns about for known remote targets.🩹 Proposed fix
const refresh = useCallback((): Promise<TuiAgent[]> => { + if (isUnknown) { + return Promise.resolve([]) + } if (targetKind === 'runtime' && targetId) { return refreshRuntime(targetId) } if (targetKind === 'ssh' && targetId) { return refreshRemote(targetId) } return refreshLocal() - }, [targetKind, targetId, refreshRuntime, refreshRemote, refreshLocal]) + }, [isUnknown, targetKind, targetId, refreshRuntime, refreshRemote, refreshLocal])
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5dd9c0d9-8b83-4c34-814e-407bce5cac48
📒 Files selected for processing (9)
src/renderer/src/components/settings/AgentsPane.test.tsxsrc/renderer/src/components/settings/AgentsPane.tsxsrc/renderer/src/components/tab-bar/QuickLaunchButton.test.tssrc/renderer/src/hooks/useAgentDetectionTarget.tssrc/renderer/src/hooks/useDetectedAgents.test.tsxsrc/renderer/src/hooks/useDetectedAgents.tssrc/renderer/src/store/slices/detected-agents.test.tssrc/renderer/src/store/slices/detected-agents.tssrc/renderer/src/store/slices/runtime-detected-agents.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/renderer/src/hooks/useAgentDetectionTarget.ts
- src/renderer/src/components/tab-bar/QuickLaunchButton.test.ts
- src/renderer/src/store/slices/runtime-detected-agents.ts
- src/renderer/src/store/slices/detected-agents.ts
- src/renderer/src/components/settings/AgentsPane.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f6fca619-f751-4426-8e6a-065372050539
📒 Files selected for processing (5)
src/renderer/src/components/settings/OrchestrationSkillAgentCoverage.tsxsrc/renderer/src/hooks/useAgentDetectionTarget.test.tssrc/renderer/src/hooks/useAgentDetectionTarget.tssrc/renderer/src/hooks/useDetectedAgents.test.tsxsrc/renderer/src/hooks/useDetectedAgents.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/renderer/src/hooks/useDetectedAgents.ts
- src/renderer/src/hooks/useDetectedAgents.test.tsx
- src/renderer/src/hooks/useAgentDetectionTarget.ts
This was referenced Jul 27, 2026
brennanb2025
added a commit
to sdhilip200/orca
that referenced
this pull request
Jul 29, 2026
The coverage widget dropped useDetectedAgents' detection target, which reads as "host not yet known": detectedIds pins to null, so loading never clears and the widget spins forever. It also reverted the remote-host scoping from stablyai#9790. - Restore the { kind: 'local' } target and assert it in the component test, whose useDetectedAgents mock previously ignored its arguments. - Cover the discovery contract renderer coverage consumes: every deduped rootPath must key an owning source, or that agent silently reads uncovered.
5 tasks done
brennanb2025
pushed a commit
that referenced
this pull request
Jul 29, 2026
Settings → Orchestration → Agent coverage decided whether each detected agent had the orchestration skill by matching a skill's single `rootPath` against a hand-maintained table of path segments. Skill discovery dedups by canonical file path, so when one provider home is a symlink onto another agent's skills directory — which `npx skills add --global` does — the two collapse into one row and the absorbed roots survive only in `rootPaths`, which the old matcher ignored. With `~/.grok/skills` symlinked onto `~/.claude/skills`, Claude read Ready and Grok read Missing even though Grok loads the same files. Now classifies every entry in `rootPaths` and resolves each root through the `SkillDiscoverySource` the scanner already returns, accepting it when `sourceKind` is not `repo` and `owner` is either the agent's owner or `null`. Deletes `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. No discovery roots change here; those landed in #8510. Fixed during review: - Restored the required `{ kind: 'local' }` argument to `useDetectedAgents`. Dropping it failed typecheck (TS2554) and pinned `detectedIds` to null, leaving the widget on "Checking installed agents and skill paths…" forever. It also reverted the remote-host scoping from #9790. - Stopped a duplicate repo root from shadowing an owning home root. `SkillDiscoverySource.path` is not unique: when the scan cwd is the home directory, `~/.claude/skills` is scanned as both a home root and a repo root, and a path-keyed Map is last-write-wins — so every agent read Missing while the panel above said Installed. Guaranteed in Settings on a WSL project. - Carried OMP coverage in the owner-based shape so #6422's coverage-table hunks could be dropped on merge. Behavior note: an orchestration skill inside an enabled Claude plugin now counts for Claude. That is a deliberate consequence of trusting the scanner's ownership data, matches how the Codex plugin cache was already treated, and is pinned by a test. Refs #8256 Co-authored-by: sdhilip200 <49802211+sdhilip200@users.noreply.github.com>
6 tasks
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
…emote-server host (stablyai#9790) * fix(agents): scope Settings agent list and quick-launch menu to the remote-server host With a paired Remote Server as the Active Server, Settings → Agents and the tab-bar + quick-launch items always ran agent detection on the local client's PATH, so a Windows client showed its own agents while worktree-create correctly listed the server's. - Extract TabBar's ssh/runtime/local owner resolution into a shared useAgentDetectionTargetForWorktree hook and use it in QuickLaunch, which previously resolved only SSH connections and fell back to local for paired-runtime worktrees. - Scope AgentsPane detection (and its Refresh button) to the Active Server, with an "on <server>" badge showing which host the list came from. Enable/disable/default toggles remain client-side settings. - Split runtime detection into store/slices/runtime-detected-agents.ts and add refreshRuntimeDetectedAgents: preflight.refreshAgents over the relay (login-shell PATH re-read), falling back to preflight.detectAgents on servers that predate the refresh RPC, keeping the last known list when the runtime is unreachable. * fix(agents): avoid redundant runtime refresh fallback * fix(agents): dedupe SSH agent refreshes * fix(agents): preserve remote host boundaries * fix(agents): prevent remote detection refresh races * fix(agents): harden remote detection failures * fix(agents): keep unresolved detection off local host * fix(agents): keep cold remote ownership unresolved
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
Settings → Orchestration → Agent coverage decided whether each detected agent had the orchestration skill by matching a skill's single `rootPath` against a hand-maintained table of path segments. Skill discovery dedups by canonical file path, so when one provider home is a symlink onto another agent's skills directory — which `npx skills add --global` does — the two collapse into one row and the absorbed roots survive only in `rootPaths`, which the old matcher ignored. With `~/.grok/skills` symlinked onto `~/.claude/skills`, Claude read Ready and Grok read Missing even though Grok loads the same files. Now classifies every entry in `rootPaths` and resolves each root through the `SkillDiscoverySource` the scanner already returns, accepting it when `sourceKind` is not `repo` and `owner` is either the agent's owner or `null`. Deletes `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. No discovery roots change here; those landed in stablyai#8510. Fixed during review: - Restored the required `{ kind: 'local' }` argument to `useDetectedAgents`. Dropping it failed typecheck (TS2554) and pinned `detectedIds` to null, leaving the widget on "Checking installed agents and skill paths…" forever. It also reverted the remote-host scoping from stablyai#9790. - Stopped a duplicate repo root from shadowing an owning home root. `SkillDiscoverySource.path` is not unique: when the scan cwd is the home directory, `~/.claude/skills` is scanned as both a home root and a repo root, and a path-keyed Map is last-write-wins — so every agent read Missing while the panel above said Installed. Guaranteed in Settings on a WSL project. - Carried OMP coverage in the owner-based shape so stablyai#6422's coverage-table hunks could be dropped on merge. Behavior note: an orchestration skill inside an enabled Claude plugin now counts for Claude. That is a deliberate consequence of trusting the scanner's ownership data, matches how the Codex plugin cache was already treated, and is pinned by a test. Refs stablyai#8256 Co-authored-by: sdhilip200 <49802211+sdhilip200@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
With a paired Remote Server set as the Active Server (
orca serveon a headless Linux box, client on Windows/macOS), two surfaces list the client's agents instead of the server's:AgentsPanecalleduseDetectedAgents()with no target, which always runspreflight.detectAgentson the local main process. It never readsettings.activeRuntimeEnvironmentId.+menu quick-launch items —QuickLaunchButtonresolved the host via the SSH-onlygetConnectionIdFromState, which returnsnullfor paired-runtime worktrees, falling back to local detection. (The SSH path came from feat(ssh): detect remote-host CLI agents in quick-launch menu #1027 and was never extended when paired runtimes arrived.)Worktree-create already routed correctly (
useComposerStatebuilds a runtime target and dispatches detection over the relay), which is exactly how the reporting user narrowed it down: server agents in worktree-create, local agents everywhere else. Reported on Discord (Windows 1.4.137/1.4.146 client + Linux headless server, both surfaces confirmed by screenshots).Fix
hooks/useAgentDetectionTarget.ts— extracts TabBar's inline ssh/runtime/local owner resolution into a shared hook. TabBar behavior is unchanged; QuickLaunch now uses the same resolution, so the+menu probes the worktree's owning runtime host.AgentsPanescopes detection to the Active Server when one is set (matching the settings-page contract that the Active Server routes provider checks), shows a mutedon <server>badge next to "N detected", and swaps the Refresh tooltip. Enable/disable/default toggles remain client-side settings.store/slices/runtime-detected-agents.ts— runtime detection split out ofdetected-agents.ts(the addition crossed the 300-line max-lines cap; no ratchet bump). AddsrefreshRuntimeDetectedAgents:preflight.refreshAgentsover the relay for a real login-shell PATH re-read, falling back topreflight.detectAgentson servers that predate the refresh RPC, and keeping the last known list when the runtime is unreachable.useDetectedAgents().refresh/isRefreshingare now target-aware for local, SSH, and runtime hosts.Testing
null, AgentsPane passed nothing) and pass now.preflight.refreshAgents, fallback topreflight.detectAgentsfor older servers, and no cache wipe on total failure; retain/clear prune the new refresh state.Notes
verify:localization-coverageis currently red onmainitself (two unlocalized ai-vault strings:AiVaultSessionDropLayer.tsx:260,ai-vault-session-launch-actions.ts:150) — unrelated to this change, left untouched.OrchestrationSkillAgentCoverage.tsxstill callsuseDetectedAgents()local-only.