fix(banner): count the MCP servers the session loads

extensions/startup-banner.ts

@@ -525,6 +525,83 @@ async function countPackageExtensions(packages: unknown[]): Promise<number> {
   return count;
 }
 
+interface McpServerEntry {
+  disabled?: boolean;
+}
+
+interface McpConfigFile {
+  mcpServers?: unknown;
+  "mcp-servers"?: unknown;
+}
+
+/** MCP config layers, lowest precedence first, as `getConfigSources` in
+ *  pi-mcp-adapter orders them (verified against 2.34.0). A later layer replaces
+ *  the earlier entry for a server of the same name, which is how `/mcp disable`
+ *  turns a globally configured server off for one project.
+ *
+ *  Reading only the two Pi-owned files missed a server defined in a shared
+ *  layer entirely, and let an omitted higher-precedence `disabled` entry keep a
+ *  server in the count that the session does not load.
+ *
+ *  Four adapter sources are deliberately NOT mirrored, because none of them can
+ *  be resolved from a config path alone: exclusive-config mode, opt-in host and
+ *  ancestor discovery, and the package / agent-plugin / Claude-plugin configs.
+ *  A banner that walks those would be a second implementation of the loader
+ *  rather than a reading of it. */
+export function mcpConfigPaths(cwd: string): string[] {
+  const home = os.homedir();
+  return [
+    join(home, ".config", "mcp", "mcp.json"),
+    join(home, ".agents", "mcp.json"),
+    join(home, ".agents", "mcp", "mcp.json"),
+    join(PI_AGENT_DIR, "mcp.json"),
+    join(cwd, ".mcp.json"),
+    join(cwd, ".pi", "mcp.json"),
+  ];
+}
+
+/** The adapter's own entry test (`isRecord` in its `config.ts`): a null, a
+ *  primitive or an array is not a server definition and never reaches the
+ *  session, so it must not reach the count either. */
+function isMcpServerEntry(value: unknown): value is McpServerEntry {
+  return typeof value === "object" && value !== null && !Array.isArray(value);
+}
+
+/** How many MCP servers this session actually loads.
+ *
+ * Counting the keys of the global config file overstated it twice over: a
+ * server carrying `"disabled": true` connects to nothing and registers no
+ * tools, and a server configured only in the project layer was invisible to a
+ * global-file parse. A layer that is absent or unparseable contributes
+ * nothing and does not discard the others.
+ */
+export async function countEnabledMcpServers(
+  cwd: string,
+  read: (path: string) => Promise<string> = (path) => readFile(path, "utf8"),
+): Promise<number> {
+  const servers = new Map<string, McpServerEntry>();
+  for (const path of mcpConfigPaths(cwd)) {
+    let entries: unknown;
+    try {
+      const file = JSON.parse(await read(path)) as McpConfigFile | null;
+      // `mcp-servers` is the alias the adapter reads alongside `mcpServers`.
+      entries = file?.mcpServers ?? file?.["mcp-servers"];
+    } catch {
+      continue;
+    }
+    if (!isMcpServerEntry(entries)) continue;
+    for (const [name, entry] of Object.entries(entries)) {
+      if (!isMcpServerEntry(entry)) continue;
+      servers.set(name, entry);
+    }
+  }
+  let enabled = 0;
+  for (const entry of servers.values()) {
+    if (entry?.disabled !== true) enabled += 1;
+  }
+  return enabled;
+}
+
 export function readGitBranch(cwd: string, run: typeof execFile = execFile): Promise<string> {
   return new Promise((resolve) => {
     run("git", ["-C", cwd, "branch", "--show-current"], {
@@ -654,16 +731,7 @@ export default function (pi: ExtensionAPI) {
 
     setTimeout(() => {
       (async () => {
-        try {
-          const raw = await readFile(
-            join(os.homedir(), ".pi", "agent", "mcp.json"),
-            "utf8",
-          );
-          const cfg = JSON.parse(raw);
-          mcpServersCount = Object.keys(cfg.mcpServers || {}).length;
-        } catch {
-          mcpServersCount = 0;
-        }
+        mcpServersCount = await countEnabledMcpServers(ctx.cwd);
         refreshStats();
       })();
     }, 150);

tests/startup-banner.test.ts

@@ -2,7 +2,7 @@ import assert from "node:assert/strict";
 import test from "node:test";
 import { syncBuiltinESMExports } from "node:module";
 import fs from "node:fs/promises";
-import startup, { readGitBranch } from "../extensions/startup-banner.ts";
+import startup, { countEnabledMcpServers, mcpConfigPaths, readGitBranch } from "../extensions/startup-banner.ts";
 import type { ExtensionAPI } from "@earendil-works/pi-coding-agent";
 import { visibleWidth } from "@earendil-works/pi-tui";
 import { stripAnsi } from "../lib/terminal-theme.ts";
@@ -124,3 +124,101 @@ for (const showRose of [false, true]) for (const showTextLogo of [false, true])
 		}
 	});
 }
+
+// The banner's MCP stat counted every key in the GLOBAL config file, so a
+// server that connects to nothing still inflated it and a project-only server
+// never appeared (#979). It also read only the two Pi-owned files, while the
+// session merges six layers, so a server defined in a shared layer was missing
+// and an omitted higher-precedence `disabled` entry kept one in the count.
+const MCP_LAYER_NAMES = [
+	"sharedGlobal",
+	"agentsGlobal",
+	"agentsNested",
+	"global",
+	"projectShared",
+	"project",
+] as const;
+
+const mcpLayers = (cwd: string, layers: Partial<Record<(typeof MCP_LAYER_NAMES)[number], unknown>>) => {
+	const paths = mcpConfigPaths(cwd);
+	// Control: the layers really are distinct paths, or every case below would
+	// be measuring one file several times over.
+	assert.equal(paths.length, MCP_LAYER_NAMES.length);
+	assert.equal(new Set(paths).size, paths.length);
+	const byPath = new Map(paths.map((path, index) => [path, MCP_LAYER_NAMES[index]!]));
+	return async (path: string) => {
+		const name = byPath.get(path);
+		const body = name === undefined ? undefined : layers[name];
+		if (body === undefined) throw new Error(`ENOENT: ${path}`);
+		return typeof body === "string" ? body : JSON.stringify(body);
+	};
+};
+
+test("MCP stat counts servers the session loads, not keys in the config file", async () => {
+	const read = mcpLayers("/repo", {
+		global: { mcpServers: { one: { command: "a" }, two: { command: "b", disabled: true } } },
+	});
+	assert.equal(await countEnabledMcpServers("/repo", read), 1);
+});
+
+test("MCP stat sees a project layer, and lets it disable a globally enabled server", async () => {
+	const read = mcpLayers("/repo", {
+		global: { mcpServers: { shared: { command: "a" } } },
+		project: { mcpServers: { shared: { command: "a", disabled: true }, local: { command: "c" } } },
+	});
+	// `shared` is off for this project and `local` exists only here: one server.
+	assert.equal(await countEnabledMcpServers("/repo", read), 1);
+});
+
+test("MCP stat reads the shared layers the session merges, not only the Pi-owned two", async () => {
+	// One server per shared layer, none of them in a Pi-owned file. Reading only
+	// `~/.pi/agent/mcp.json` and `<cwd>/.pi/mcp.json` reported none of them.
+	const read = mcpLayers("/repo", {
+		sharedGlobal: { mcpServers: { a: { command: "a" } } },
+		agentsGlobal: { mcpServers: { b: { command: "b" } } },
+		agentsNested: { mcpServers: { c: { command: "c" } } },
+		projectShared: { mcpServers: { d: { command: "d" } } },
+	});
+	assert.equal(await countEnabledMcpServers("/repo", read), 4);
+});
+
+test("MCP stat lets a higher layer disable a server a lower one enabled", async () => {
+	// The direction the old two-path read could not see: `.mcp.json` sits ABOVE
+	// the Pi global file, so its `disabled` wins and the banner must not keep
+	// counting the server the global layer enabled.
+	const read = mcpLayers("/repo", {
+		sharedGlobal: { mcpServers: { shared: { command: "a" } } },
+		projectShared: { mcpServers: { shared: { command: "a", disabled: true } } },
+	});
+	assert.equal(await countEnabledMcpServers("/repo", read), 0);
+});
+
+test("MCP stat reads the mcp-servers spelling the adapter also accepts", async () => {
+	const read = mcpLayers("/repo", {
+		global: { "mcp-servers": { one: { command: "a" } } },
+	});
+	assert.equal(await countEnabledMcpServers("/repo", read), 1);
+});
+
+test("MCP stat tolerates a config whose mcpServers is not an object of entries", async () => {
+	// An entry that is not a non-null object is not a server definition: the
+	// adapter's `isRecord` drops it before the session sees it, so counting it
+	// as enabled reported a server that never loads.
+	const shapes: [unknown, number][] = [
+		[{ mcpServers: [] }, 0],
+		[{ mcpServers: null }, 0],
+		[{}, 0],
+		["null", 0],
+		[{ mcpServers: { one: null } }, 0],
+		[{ mcpServers: { one: "a" } }, 0],
+		[{ mcpServers: { one: [] } }, 0],
+		[{ mcpServers: { one: { command: "a" }, two: null } }, 1],
+	];
+	for (const [shape, expected] of shapes) {
+		assert.equal(
+			await countEnabledMcpServers("/repo", mcpLayers("/repo", { global: shape })),
+			expected,
+			`shape ${JSON.stringify(shape)}`,
+		);
+	}
+});