design phase 2.4 connect tools - sinrimin/harness-nexus GitHub Wiki
Status: design approved, implementation in progress. PRD:
prd-phase-2.4-connect-tools.md. Builds on Phase 2.2 (McpRegistrypool,listTools(),/api/mcp-servers/status).
The operator-facing control surface for proxy-mode MCP connections. Phase 2.2
already pools proxy upstreams, auto-connects them on startup/config-change, and
exposes a read-only GET /api/mcp-servers/status. This phase adds explicit
per-server connect/disconnect and tool-list inspection on top of that pool
β no storage change, no migration, no change to /mcp aggregation.
The guiding rule: incremental, not lazy. Startup auto-pooling stays exactly
as Phase 2.2 left it. Connect/Disconnect are an operator control surface for
forcing reconnects (e.g. after fixing a broken credential) and inspecting what
an upstream actually offers β they never gate whether /mcp aggregates a server.
McpRegistry pool (Phase 2.2, unchanged at startup)
β
β + new public methods (read/act on the existing pool):
β connectServer(id) β close+reconnect one entry; returns status
β disconnectServer(id) β closeConnection but KEEP pool entry (disconnected)
β listServerTools(id) β read conn.tools (original names, no <server>__ prefix)
β refreshServerTools(id) β refreshTools(conn) then read
βΌ
GET /api/mcp-servers/status (extended: + toolCount per entry)
POST /api/mcp-servers/:id/connect
POST /api/mcp-servers/:id/disconnect
GET /api/mcp-servers/:id/tools
POST /api/mcp-servers/:id/tools/refresh
βΌ
sdk-ts: connectMcpServer / disconnectMcpServer / listMcpServerTools / refreshMcpServerTools
βΌ
McpManagement.tsx
- polling hook (setInterval 5s) keeps statuses fresh
- per-row: status badge (color = state) + toolCount badge + Connect/Disconnect
- Collapsible tool panel: name + description + inputSchema params + Refresh
McpToolInfo lives in packages/shared (NOT core). Tool inspection is a
runtime/transient shape read out of the registry pool, not a persisted domain
entity β core must stay pure (no I/O, no framework imports), and this type
crosses the serverβsdkβweb boundary via shared, which is the single source of
truth for shapes that all three validate/serialize against.
// packages/shared/src/<mcp module>.ts
export interface McpToolInfo {
name: string;
description?: string;
inputSchema: Record<string, unknown>;
}Shape-aligned with the registry's existing AggregatedTool (same fields), but
semantically distinct: AggregatedTool is the internal namespaced form the
proxy uses; McpToolInfo is the un-namespaced, externally-exposed form for a
single server's tools. Re-export from packages/shared/src/index.ts.
All in packages/server/src/mcp/registry.ts. They expose the existing pool
internals through a narrow, well-named surface. The existing private
connectOne() / closeConnection() / refreshTools() become the
implementation; new public methods add the validation + return shaping.
export interface McpServerStatus {
id: string;
name: string;
status: ConnectionStatus;
detail?: string;
toolCount: number; // NEW β conn.tools.size (0 when not connected)
}getStatuses() is updated to read c.tools.size. Existing Dashboard consumer
ignores the extra field (additive), so no web break.
Force (re)connect of one proxy upstream. The key use case: a row is stuck at
error after the operator fixed the credential; this re-dials it.
async connectServer(id: string): Promise<McpServerStatus> {
const conn = this.pool.get(id);
if (conn) {
// Idempotent reconnect: close the stale/duplicate entry first so we don't
// leak a Client. (reload()'s sameServer check can't see this path.)
await this.closeConnection(conn);
this.pool.delete(id);
}
// connectOne reads fresh config from uow β but it takes a McpServer object,
// not an id. Re-fetch the server config so a config change since startup is
// honored. Throws if the server was deleted or switched to direct.
const server = await this.uow.mcpServers.findById(id);
if (!server) throw new NotFoundError(id);
if (server.mode !== 'proxy') throw new NotProxyError(server);
await this.connectOne(server); // existing private method, non-throwing internally
return this.statusOf(id)!; // connectOne always inserts a pool entry
}Why re-fetch config: connectOne takes a McpServer object. The registry
pool may hold a stale snapshot if the operator edited config between reloads.
Re-reading from uow.mcpServers.findById(id) ensures the latest transport /
credential placeholders are honored β and naturally produces the right error if
the server was deleted or switched to direct (no proxy re-dial).
Error model: connectOne is best-effort (catches internally, sets
status='error' + detail). So connectServer resolves with the resulting
status object even on failure β the operator sees error + detail rather than
a thrown 500. The route does NOT need to translate connect failures; it returns
the status object. NotFound/NotProxy are the only throws (config-level),
translated by the route to 404/409.
Note: registry currently throws plain
Error. To attach clean status codes (404/409) without coupling the registry toAppError, the new methods throw registry-local error subclasses (RegistryNotFound,RegistryNotProxy) and the route maps them toAppError. This keeps the registry free of HTTP concerns (per AGENTS.md architecture rule #4: "Decoupled from transport"). Alternatively, throwAppErrordirectly from the registry since the existingprofileEntriesForalready throws plain Errors that aren't HTTP-mapped β but the subclass approach is cleaner. Decision: registry-local subclasses, mapped in the route.
async disconnectServer(id: string): Promise<McpServerStatus> {
const conn = this.pool.get(id);
if (!conn) throw new NotFoundError(id);
await this.closeConnection(conn); // sets status='disconnected', does NOT delete
return this.statusOf(id)!;
}Critical: do NOT this.pool.delete(id). Deleting the entry would make the
row vanish from getStatuses() (the Dashboard mesh and the table's status column
read from there), and more importantly doReload() only reconciles entries it
sees vs. configured β a deleted-but-configured entry gets re-added (re-connected)
on the next unrelated mutation's reload(), silently undoing the disconnect.
Keeping the entry with status='disconnected' is the honest state: the server is
configured and pooled, just not currently connected. doReload's
sameServer check treats it as unchanged and leaves it alone.
Read-only snapshot of the cached tool list (original names, no namespace prefix):
listServerTools(id: string): McpToolInfo[] {
const conn = this.pool.get(id);
if (!conn) throw new NotFoundError(id);
// Unconnected β empty array, NOT an error. The route returns []; the UI gates
// the tool panel on status==='connected' so this is defense-in-depth.
if (conn.status !== 'connected') return [];
return [...conn.tools.values()].map((t) => ({
name: t.name,
...(t.description ? { description: t.description } : {}),
inputSchema: t.inputSchema,
}));
}This is distinct from the existing listTools(filterServerIds?): that one
returns the namespaced aggregated form for the proxy; this returns the
un-namespaced per-server form for the UI. Both read the same underlying cache.
async refreshServerTools(id: string): Promise<McpToolInfo[]> {
const conn = this.pool.get(id);
if (!conn) throw new NotFoundError(id);
if (conn.status !== 'connected') throw new NotConnectedError(id);
await this.refreshTools(conn); // existing private: re-listTools, may set error
return this.listServerTools(id);
}refreshTools is best-effort (on failure sets status='error' + detail), so
the returned list may be empty with the connection now errored β the operator
sees the status flip. No throw on upstream failure; only config-level throws.
// registry-local, mapped by the route to HTTP β keeps registry HTTP-free
export class RegistryError extends Error {
constructor(message: string, readonly kind: 'not_found' | 'not_proxy' | 'not_connected') {
super(message);
this.name = 'RegistryError';
}
}
// internal: build one status entry (for the single-server return shape)
private statusOf(id: string): McpServerStatus | undefined { /* find in pool */ }packages/server/src/modules/mcp-servers.ts. All four reuse the existing
guard = { preHandler: [app.requireAuth] } and the ownsOrAdmin pattern. The
owner check uses the stored server record (read before the registry call),
not the pool entry β so a not-found or not-owned server returns 404 MCP_SERVER_NOT_FOUND before touching the registry (leak prevention, identical to
PATCH/DELETE).
| Method | Path | Body | Returns | Errors |
|---|---|---|---|---|
| POST | /api/mcp-servers/:id/connect |
β | { status: McpServerStatus } |
404 not-found/not-owned; 409 NOT_PROXY_MODE
|
| POST | /api/mcp-servers/:id/disconnect |
β | { status: McpServerStatus } |
404; (not-in-pool β treat as 404, no separate code) |
| GET | /api/mcp-servers/:id/tools |
β | { tools: McpToolInfo[] } |
404; (not connected β [], not an error) |
| POST | /api/mcp-servers/:id/tools/refresh |
β | { tools: McpToolInfo[] } |
404; 409 NOT_CONNECTED
|
// connect route β shape of all four (owner check first, then registry, map errors)
app.post<{ Params: { id: string } }>('/api/mcp-servers/:id/connect', guard, async (req, reply) => {
await assertProxyOwned(req); // 404 if missing/not-owned, 409 NOT_PROXY_MODE if direct
try {
const status = await app.mcpRegistry!.connectServer(req.params.id);
return { status };
} catch (e) {
throw mapRegistryError(e); // RegistryError β AppError (404/409)
}
});assertProxyOwned reads the record via uow.mcpServers.findById, applies
ownsOrAdmin (404 on miss to avoid leaking existence), and rejects mode !== 'proxy'
with 409 NOT_PROXY_MODE. A shared helper since all four routes need it (except
connect needs the proxy check; tools/refresh can rely on the pool β but routing
all four through the same owned-proxy assert keeps it consistent and avoids
leaking a direct server's existence via the tools endpoint too).
GET /api/mcp-servers/status (existing) is extended only in return shape β
each entry gains toolCount. No new route.
packages/sdk-ts/src/index.ts:
export interface McpServerStatus {
id: string;
name: string;
status: 'connecting' | 'connected' | 'error' | 'disconnected';
detail?: string;
toolCount?: number; // NEW β absent on older servers; treat 0/undefined as "unknown"
}
// re-export from shared (already the pattern for other shared types)
export type { McpToolInfo } from '@harness-nexus/shared';
// four methods, same request() pattern as existing
async connectMcpServer(id: string): Promise<{ status: McpServerStatus }> {
return this.request('POST', `/api/mcp-servers/${id}/connect`);
}
async disconnectMcpServer(id: string): Promise<{ status: McpServerStatus }> {
return this.request('POST', `/api/mcp-servers/${id}/disconnect`);
}
async listMcpServerTools(id: string): Promise<McpToolInfo[]> {
const res = await this.request('GET', `/api/mcp-servers/${id}/tools`);
return res.tools;
}
async refreshMcpServerTools(id: string): Promise<McpToolInfo[]> {
const res = await this.request('POST', `/api/mcp-servers/${id}/tools/refresh`);
return res.tools;
}// module-scope constant
const STATUS_POLL_MS = 5000;
// in McpManagementPage
const [statuses, setStatuses] = useState<Map<string, McpServerStatus>>(new Map());
// mount: load servers + statuses together
useEffect(() => {
void (async () => {
const [servers, statusRes] = await Promise.all([
withAuthGuard(() => api.listMcpServers(), logout),
withAuthGuard(() => api.listMcpServerStatuses(), logout),
]);
setItems(servers);
setStatuses(new Map(statusRes.statuses.map((s) => [s.id, s])));
})();
// poll loop β statuses only (servers change via refresh())
const timer = setInterval(() => {
api
.listMcpServerStatuses()
.then((r) => setStatuses(new Map(r.statuses.map((s) => [s.id, s]))))
.catch(() => {}); // swallow poll errors; next tick retries
}, STATUS_POLL_MS);
return () => clearInterval(timer);
}, [logout]);statuses keyed by id for O(1) row lookup. Poll errors are swallowed (a failed
poll shouldn't toast; the next interval retries). The cleanup function clears the
interval on unmount.
Color encodes connection state only (AGENTS.md "Signal"). --signal (cyan)
is reserved for liveness/links and is NOT used here. Map to the semantic palette:
| status | color token | meaning |
|---|---|---|
connected |
--ok (green) |
live |
error |
--danger (red) |
failed |
connecting |
--warn (amber) |
transient |
disconnected |
muted (neutral) | idle |
Rendered as a <Dot> + label. The Dot uses the token class (bg-ok /
bg-danger / bg-warn / bg-muted-foreground), matching the Dashboard
MeshTopology's existing dotVariantFor approach. The Badge wrapper stays a
neutral variant (secondary) β color comes from the dot, not the badge fill, so
we don't spend --signal or introduce a second decorative accent.
direct rows show a neutral Badge (outline, no dot): "dialed by the tool".
-
proxy row, not connected/connecting: Connect button (outline, small).
Clicking sets local "pending" state for that id (disables the button + shows a
spinner), calls
connectMcpServer(id), then lets the poll pick up the new status (don't optimistically flip to connected β the upstream might fail). -
proxy row, connected: Disconnect item in the existing row
DropdownMenu(alongside Delete), and the row becomes expandable. - proxy row, connecting: Connect button disabled + spinner. Poll continues.
- direct row: no connect/expand controls; neutral status hint only.
Wrap radix-ui's Collapsible in a thin shadcn-style wrapper at
apps/web/src/components/ui/collapsible.tsx (matches the existing
components/ui/dialog.tsx convention; radix-ui is already a dependency).
// apps/web/src/components/ui/collapsible.tsx
import { Collapsible as CollapsiblePrimitive } from 'radix-ui';
export const Collapsible = CollapsiblePrimitive.Root;
export const CollapsibleTrigger = CollapsiblePrimitive.Trigger;
export const CollapsibleContent = CollapsiblePrimitive.Content;Trigger = the row's Name cell (when connected): name + a ChevronRight
(rotate-90 when open). Content = ToolList (module-scope sub-component):
- On open, if no cached tools for this id, fetch
listMcpServerTools(id)(lazy). If the server has been connected for a while, tools are already cached server-side; this is one cheap GET. - Header row: "Tools (N)" + a Refresh button (
RefreshCwicon; rotates while fetching viaanimate-spin). Refresh callsrefreshMcpServerTools(id). - Each tool:
name(mono) +description(muted) + a nested expandable forinputSchemaparameters (property name, type, required flag). Reuse the same Collapsible primitive for the per-tool detail, or a simpler<details>-style toggle β keep it light. - Empty tool list: muted "No tools exposed by this server".
- Per-tool state lives in a module-scope sub-component
ToolList({ serverId })(defined at module scope, never inline β per the React perf rule).
- Sub-components (
StatusBadge,ConnectButton,ToolList,ToolRow) defined at module scope, not insideMcpManagementPage. - Static config (the statusβtoken map) hoisted to a module-scope
const. - Conditional rendering uses ternaries, not
&&. - Functional
setState(setStatuses((prev) => ...)). - The polling
setIntervalcallback doesn't capture stale state (it only overwritesstatuses, doesn't read it).
pnpm -r typecheck-
pnpm --filter @harness-nexus/server buildΒ·@harness-nexus/sdk-tsΒ·@harness-nexus/web - Extend
scripts/smoke.mjs: register admin β create proxy server βPOST /:id/connectβ assertstatus.statusin{connected,error}andtoolCountpresent βGET /:id/toolsβPOST /:id/tools/refreshβPOST /:id/disconnectβ assertdisconnected. Plus negative: create direct server βPOST /:id/connectβ 409NOT_PROXY_MODE; not-owner β 404.
(See PRD Β§Out of Scope.) Lazy connect model, tool-level authorization, tool invocation, direct-mode controls, real-time push, connection metrics.