mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-20 10:11:58 +00:00
fix(tui): MCP headline counts connected servers, not disabled ones (#48402)
The TUI banner footer used the raw `info.mcp_servers.length`, so a configured-but-disabled server (e.g. `linear`) was counted alongside connected ones. With a disabled `linear` and a connected `nous-support`, the TUI reported "2 MCP" while the classic CLI correctly reported "1 MCP" (`mcp_connected = sum(1 for s in mcp_status if s["connected"])` in hermes_cli/banner.py). The collapse toggle even labels the count "connected", which was wrong for the same reason. Count connected servers for both the toggle and the footer segment, and drop the `· N MCP` segment entirely when none are connected (matching the classic banner, which only appends it when the count is > 0). The expandable MCP section still lists every configured server, including disabled ones. Invariant test renders SessionPanel and asserts the headline equals the connected count, never the configured total.
This commit is contained in:
parent
426f321e84
commit
8abdab24c9
2 changed files with 120 additions and 3 deletions
111
ui-tui/src/__tests__/brandingMcpCount.test.ts
Normal file
111
ui-tui/src/__tests__/brandingMcpCount.test.ts
Normal file
|
|
@ -0,0 +1,111 @@
|
|||
import { PassThrough } from 'stream'
|
||||
|
||||
import { renderSync } from '@hermes/ink'
|
||||
import React from 'react'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { SessionPanel } from '../components/branding.js'
|
||||
import { DEFAULT_THEME } from '../theme.js'
|
||||
import type { McpServerStatus, SessionInfo } from '../types.js'
|
||||
|
||||
// Invariant under test: the TUI banner's MCP headline counts *connected*
|
||||
// servers, never configured-but-disabled ones. This mirrors the classic CLI
|
||||
// banner (`mcp_connected = sum(1 for s in mcp_status if s["connected"])` in
|
||||
// hermes_cli/banner.py) and the "connected" label on the MCP collapse toggle.
|
||||
//
|
||||
// Regression: branding.tsx used the raw `info.mcp_servers.length`, so a
|
||||
// disabled `linear` server alongside a connected `nous-support` server made
|
||||
// the TUI report "2 MCP" while the classic CLI correctly reported "1 MCP".
|
||||
|
||||
const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms))
|
||||
|
||||
const makeStreams = (columns = 100) => {
|
||||
const stdout = new PassThrough()
|
||||
const stdin = new PassThrough()
|
||||
const stderr = new PassThrough()
|
||||
|
||||
Object.assign(stdout, { columns, isTTY: false, rows: 40 })
|
||||
Object.assign(stdin, { isTTY: false })
|
||||
Object.assign(stderr, { isTTY: false })
|
||||
|
||||
let captured = ''
|
||||
stdout.on('data', chunk => {
|
||||
captured += chunk.toString()
|
||||
})
|
||||
|
||||
return { capture: () => captured, stderr, stdin, stdout }
|
||||
}
|
||||
|
||||
const mcp = (over: Partial<McpServerStatus> & Pick<McpServerStatus, 'name'>): McpServerStatus => ({
|
||||
connected: false,
|
||||
tools: 0,
|
||||
transport: 'http',
|
||||
...over
|
||||
})
|
||||
|
||||
const baseInfo = (mcp_servers: McpServerStatus[]): SessionInfo => ({
|
||||
mcp_servers,
|
||||
model: 'test-model',
|
||||
skills: { core: ['a', 'b'] },
|
||||
tools: { file: ['read_file', 'write_file'] }
|
||||
})
|
||||
|
||||
async function renderFooter(info: SessionInfo): Promise<string> {
|
||||
const streams = makeStreams()
|
||||
|
||||
const instance = renderSync(React.createElement(SessionPanel, { info, sid: 'test', t: DEFAULT_THEME }), {
|
||||
patchConsole: false,
|
||||
stderr: streams.stderr as NodeJS.WriteStream,
|
||||
stdin: streams.stdin as NodeJS.ReadStream,
|
||||
stdout: streams.stdout as NodeJS.WriteStream
|
||||
})
|
||||
|
||||
try {
|
||||
await delay(20)
|
||||
|
||||
// Strip ANSI so we can assert on the rendered text content.
|
||||
// eslint-disable-next-line no-control-regex
|
||||
return streams.capture().replace(/\u001b\[[0-9;]*m/g, '')
|
||||
} finally {
|
||||
instance.unmount()
|
||||
instance.cleanup()
|
||||
}
|
||||
}
|
||||
|
||||
describe('branding MCP headline count', () => {
|
||||
it('counts only connected servers, not configured-but-disabled ones', async () => {
|
||||
const frame = await renderFooter(
|
||||
baseInfo([
|
||||
mcp({ connected: true, name: 'nous-support', status: 'connected', tools: 6 }),
|
||||
mcp({ connected: false, disabled: true, name: 'linear', status: 'disabled' })
|
||||
])
|
||||
)
|
||||
|
||||
// One connected server → "1 MCP", never "2 MCP".
|
||||
expect(frame).toContain('1 MCP')
|
||||
expect(frame).not.toContain('2 MCP')
|
||||
})
|
||||
|
||||
it('drops the MCP segment entirely when no server is connected', async () => {
|
||||
const frame = await renderFooter(
|
||||
baseInfo([mcp({ connected: false, disabled: true, name: 'linear', status: 'disabled' })])
|
||||
)
|
||||
|
||||
// Matches the classic CLI, which only appends "· N MCP" when N > 0.
|
||||
expect(frame).not.toContain('MCP servers')
|
||||
expect(frame).not.toMatch(/\d MCP\b/)
|
||||
})
|
||||
|
||||
it('counts every connected server when several are connected', async () => {
|
||||
const frame = await renderFooter(
|
||||
baseInfo([
|
||||
mcp({ connected: true, name: 'alpha', status: 'connected' }),
|
||||
mcp({ connected: true, name: 'beta', status: 'connected' }),
|
||||
mcp({ connected: false, disabled: true, name: 'gamma', status: 'disabled' })
|
||||
])
|
||||
)
|
||||
|
||||
expect(frame).toContain('2 MCP')
|
||||
expect(frame).not.toContain('3 MCP')
|
||||
})
|
||||
})
|
||||
|
|
@ -223,6 +223,12 @@ export function SessionPanel({ info, maxWidth, sid, t }: SessionPanelProps) {
|
|||
const toolEntries = Object.entries(info.tools).sort()
|
||||
const toolsTotal = flat(info.tools).length
|
||||
|
||||
// MCP headline counts *connected* servers, not configured-but-disabled ones,
|
||||
// so it matches the classic CLI banner (`sum(s.connected)` in
|
||||
// hermes_cli/banner.py) and the "connected" label on the collapse toggle.
|
||||
const mcpServers = info.mcp_servers ?? []
|
||||
const mcpConnected = mcpServers.filter(s => s.connected).length
|
||||
|
||||
const toolsBody = () => {
|
||||
const shown = toolEntries.slice(0, TOOLSETS_MAX)
|
||||
const overflow = toolEntries.length - TOOLSETS_MAX
|
||||
|
|
@ -376,10 +382,10 @@ export function SessionPanel({ info, maxWidth, sid, t }: SessionPanelProps) {
|
|||
)}
|
||||
|
||||
{/* ── MCP Servers (collapsed by default) ── */}
|
||||
{info.mcp_servers && info.mcp_servers.length > 0 && (
|
||||
{mcpServers.length > 0 && (
|
||||
<Box flexDirection="column" marginTop={1}>
|
||||
<CollapseToggle
|
||||
count={info.mcp_servers.length}
|
||||
count={mcpConnected}
|
||||
onToggle={() => setMcpOpen(v => !v)}
|
||||
open={mcpOpen}
|
||||
suffix="connected"
|
||||
|
|
@ -395,7 +401,7 @@ export function SessionPanel({ info, maxWidth, sid, t }: SessionPanelProps) {
|
|||
<Text color={t.color.text}>
|
||||
{toolsTotal} tools{' · '}
|
||||
{skillsTotal} skills
|
||||
{info.mcp_servers?.length ? ` · ${info.mcp_servers.length} MCP` : ''}
|
||||
{mcpConnected ? ` · ${mcpConnected} MCP` : ''}
|
||||
{' · '}
|
||||
<Text color={t.color.muted}>/help for commands</Text>
|
||||
</Text>
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue