feat(console): load agents from a remote host (CHOO-2560) - #364
Open
abeldantas wants to merge 16 commits into
Open
feat(console): load agents from a remote host (CHOO-2560)#364abeldantas wants to merge 16 commits into
abeldantas wants to merge 16 commits into
Conversation
…the same server The Add Agent path minted a new identity and wrote `.switch/agents/<name>.json` without checking whether the slot already held credentials from a different Console install on the same Switch server. This destroyed the displaced agent's API token. Add a same-endpoint guard in `runAddAgent` that reads the existing slot and checks its `SWITCH_AGENT_ID` against the local DB — if the id is unknown locally it belongs to a colleague and the create is refused with `already-configured`. Mirror the guard in `writeNeutralAgentSettingsFs` as defence in depth. The modal shows a toast directing the user to load the existing agent instead. CHOO-2560
Restore the ability to load agents on a remote host that were created by a different Console install, host-scoped and re-runnable from the remote host page. Discovery merges two sources: server-assisted (GET /agents with repo_dir, confirmed on-disk via SFTP) and a bounded $HOME scan, deduped by (dir, name) with server-attributed entries winning to preserve owner attribution. The Load Existing Agents section appears on every remote host page and auto-expands when the host was just added (the post-add-host prompt). Each row shows name, directory, owner, provider, and endpoint-mismatch warnings. Agents without an inferred provider get an inline picker. Already-loaded agents appear disabled. A manual "scan a directory" fallback covers agents outside $HOME. Also extends RemoteAgentSummary with knownAgentOptions so the gateway client carries repo_dir through for server-assisted discovery, and removes the overly aggressive same-endpoint guard from writeNeutralAgentSettingsFs that blocked legitimate same-install agent replacements (the pre-mint check in runAddAgent is the correct guard for that case). CHOO-2560
abeldantas
requested review from
amaudruz and
christian-mcdermott
as code owners
September 3, 2026 17:29
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are a few correctness/perf/test gaps in the new remote-host loading flow (notably stale server selection via useMemo, discovery filtering, and missing regression coverage for the new “already-configured” guard).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds the ability for Switch Console (Electron renderer + main process) to discover and “load/attach” agents that already exist on a remote host (typically created by another Console install), while also hardening the “Add Agent” flow to avoid overwriting same-server credentials that belong to a colleague’s agent.
Changes:
- Extend remote agent summaries to include
known_agent_options(so discovery can use server-recordedrepo_dir). - Add a new “Load existing agents” UI section on remote host pages plus main-process discovery RPCs to merge server-assisted candidates with bounded
$HOMEscanning. - Add a same-endpoint credentials collision guard in the Add Agent path, with UI + telemetry wiring for the new failure mode.
File summaries
| File | Description |
|---|---|
| console/apps/switch-console-desktop/src/shared/core/switch-servers/switch-servers.ts | Extends RemoteAgentSummary and adds a new provision result kind for the “already configured” guard. |
| console/apps/switch-console-desktop/src/renderer/tests/browser/palette-agent-icon.test.tsx | Updates test helper objects for the new knownAgentOptions field. |
| console/apps/switch-console-desktop/src/renderer/features/remote-hosts/views/remote-hosts-view.tsx | Adds a cross-view “just added host” flag to auto-expand the new load section after adding a host. |
| console/apps/switch-console-desktop/src/renderer/features/remote-hosts/views/remote-host-view.tsx | Resolves a server id for the host page and renders the new Load Existing Agents section. |
| console/apps/switch-console-desktop/src/renderer/features/remote-hosts/load-existing-agents-section.tsx | New renderer UI for discovering/selecting/loading existing agents on a host. |
| console/apps/switch-console-desktop/src/renderer/features/locations/components/add-agent-modal/add-agent-modal.tsx | Handles the new { kind: 'already-configured' } failure with a toast. |
| console/apps/switch-console-desktop/src/main/core/telemetry/events.ts | Adds already_configured as a telemetry failure reason. |
| console/apps/switch-console-desktop/src/main/core/switch-servers/gateway-client.ts | Parses known_agent_options from the gateway GET /agents response. |
| console/apps/switch-console-desktop/src/main/core/switch-servers/backfill-agent-icons.test.ts | Updates test fixtures for the new knownAgentOptions field. |
| console/apps/switch-console-desktop/src/main/core/agents/write-switch-settings.ts | Adds parsing for same-endpoint SWITCH_AGENT_ID in an existing credentials slot and introduces a dedicated error type (plus docs). |
| console/apps/switch-console-desktop/src/main/core/agents/discover-loadable-agents.ts | New main-process discovery implementation merging server-assisted repo-dir candidates with bounded $HOME scanning and manual dir scan. |
| console/apps/switch-console-desktop/src/main/core/agents/controller.ts | Exposes new discovery functions over RPC. |
| console/apps/switch-console-desktop/src/main/core/agents/agent-credentials-slot.ts | Adds a helper to read same-endpoint agent id from the credentials slot. |
| console/apps/switch-console-desktop/src/main/core/agents/add-agent.ts | Implements the same-endpoint collision guard by checking local DB visibility of an on-disk SWITCH_AGENT_ID. |
| console/apps/switch-console-desktop/src/main/core/agents/add-agent.test.ts | Adds db/drizzle mocks needed for the new guard logic. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…on, addressing verdict 1. Wire the same-endpoint defence in depth in writeNeutralAgentSettingsFs: calls existingAgentIdInSlot and throws ExistingAgentCredentialsError when the slot holds a different agent. Callers that already verified the overwrite pass expectedAgentId to bypass the guard. runAddAgent passes slotAgentId from its pre-mint DB check. 2. Manual directory scan results are now merged into local state instead of discarded — a dir outside $HOME and the server's repo_dirs stays visible after scanning. 3. Add blockedReason field to LoadableAgent: propagated from discovery, shown in the UI as the disable reason per row. Replaces the separate alreadyAgent/endpointMismatch checks in the selectableAgents filter. 4. Show per-row addressing verdict when ownerName is known: "session access: yes · rooms: policy admits only its owner, ask <owner> to widen." 5. Test coverage for the already-configured guard: exercises the path where sameEndpointAgentId returns a non-null id and addAgent returns already-configured. Also tests the writer guard directly. CHOO-2560
…o deps Move justAddedHost exports after imports per codebase convention, hide select-all toggle when no agents are selectable, and fix useMemo deps for serverId by reading MobX observables outside the memo.
… doc fix Address remaining Copilot review findings on #364: - The bounded $HOME scan now prunes every hidden directory except .switch (plus node_modules) — dot-trees like .cargo/.npm hold hundreds of thousands of entries and can never contain a surfaceable working dir. The docstring previously claimed this pruning; now the code does it. - findSwitchAgentDirsOnHost no longer swallows exec failures silently: a failed scan is logged so it cannot masquerade as an empty host. - existingAgentIdInSlot doc no longer implies read errors map to null; they throw, and callers must let them propagate.
…e + sidebar fix in Load existing agents - fix: reload the agents store after Load so the sidebar shows the new agents immediately (agentEvents is main-process-only; same idiom as the Add Agent flow) - discovery runs on 'Scan this host', not on section expand (no unasked SSH into a colleague's box) - every row can expand to details: description, agent id, endpoint, dir, provider+source, owner, found-via - endpoint mismatches show both sides (registered against X; this server is Y) - per-row trash deletes the on-disk .switch/agents/<name>.json (confirm modal; host-file only, server registration untouched; disabled for agents already loaded here) - owner/policy line says '(you)' instead of 'ask yourself to widen' when the viewer owns the agent
…going stale A locally removed agent stayed 'Already loaded in this Console' in the Load existing agents section: agentEvents is a main-process-only bus, so no renderer store or query reacts to agent create/update/delete — each call site had to remember its own refetch, and the remove path forgot (the load path had the mirror bug, patched by hand earlier). Durable fix: agents:changed channel bridged from agentEvents at startup; a global AgentCrudEvents reactor reloads the sidebar agents store and invalidates every load-existing-agents discovery query on any agent CRUD.
…car on its host Removing a loaded agent local-only was wiping the on-disk credentials in the working directory (.switch/agents/<name>.json, the settings.local.json env block, launch profiles) and killing the sidecar — on a shared host those belong to another install, so a colleague's agent was destroyed by a remove that promised to be local. Attach guarantees it writes nothing; remove now mirrors it. DeleteAgentOptions gains a required removeProvisionedFiles flag: no caller gets a default. The remove modal offers it as an explicit opt-in checkbox naming the exact host:dir, warns that shared-host files may belong to another install, and the misleading 'the folder stays on the filesystem' copy now states plainly what a plain remove does. Server teardown keeps cleaning its own provisioned agents; removing a location forgets it without touching disk.
Removed a hardcoded line reference, process-history narration in the event bridge doc, one-liners that restate the variable they sit on, and the manual agentsStore.load() in the load mutation that the agents:changed bridge made redundant (with its now-inaccurate comment).
The agent list is gated behind scanStarted, which only the "Scan this host" button set. A manual directory scan found agents and stored them but left scanStarted false, so the ternary never reached the list branch. Flip scanStarted on manual-scan success so the results render and auto-discovery kicks in alongside them.
The previous fix flipped scanStarted on manual-scan success, which rendered the results but also kicked off the slow host-wide discovery as a side effect. Instead, widen the rendering gate to also check manualAgents — the placeholder hides once either scan path has results, and auto-discovery stays gated on the explicit "Scan this host" button.
The scanStarted gate required clicking "Scan this host" before results would render, and manual directory scans never flipped it. Remove the gate entirely: discovery runs when the section expands, results display immediately, and the Rescan button handles refresh. Both auto-discovery and manual scan results appear without an intermediate step.
The agent-crud event reactor reloaded agentsStore but not the location manager, so a loaded agent appeared in "Your agents" but its location never mounted — no sidebar row, no agent page navigation. Call getLocationManagerStore().reload() alongside agentsStore.load(). Also drop locations whose last agent was removed inside _doLoad so stale sidebar rows don't survive a delete.
The $HOME walk can be slow on large VMs. Split it from the cheap server-assisted discovery: on section expand, only check the server's registered agent directories (instant); the full home-directory walk is a hidden alternate action revealed by holding Alt, with an honest slow-scan label and tooltip. The manual directory scan stays as the precision tool between the two.
A concurrent CRUD event could trigger reload() while startAgentOnboarding had already placed an unregistered placeholder in the locations map but before the agent existed in the DB. The cleanup loop would delete it mid-onboarding. Skip locations in pendingCreationIds.
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.
Summary