Fix 3 critical bugs: message privacy (#11), Windows server start (#17), status bar label (#12) (#32)

Fixes the three most critical open bugs, each via strict **test-first
TDD** (a failing test was written and confirmed to fail *because of the
bug*, then the minimal fix was applied).

## #11 — Direct messages leak to all agents (privacy/security) 🔴
The `status` MCP tool called `db.listAllMessages()` and returned
**every** message — including direct messages between *other* agents —
to any caller, with no identity filtering.

**Fix:** `status` now resolves the caller's identity (`agent_key` arg or
session) and returns only broadcasts (`*`) plus the caller's own
sent/received messages. Anonymous callers see broadcasts only. Tagged
`[MSG-PRIVACY]`.
- `packages/core/src/tools/status_tool.ts`,
`packages/core/src/server.ts`
- New test: `test/status_message_privacy_test.ts`; pre-existing tests
that asserted the leak updated to assert correct privacy.

## #17 — Windows local server fails to start; ENOENT swallowed 🔴
`spawn()` ran with no shell, so Node couldn't resolve the global npm
`.cmd` shim on Windows, and the `'error'` event was never handled — so
the real ENOENT was swallowed and users only saw the misleading `Local
server did not start within 15000ms`.

**Fix:** spawn via the shell on Windows (`buildLocalSpawnConfig`) so the
`.cmd` shim resolves, and surface spawn failures promptly via a
cancellable readiness race instead of the generic timeout.
- `src/services/connectionManager.ts`
- New test: `test/pure/connectionManagerSpawn.test.ts`

## #12 — Status bar shows "TMC: Disconnected" while connected
The mode label equated a `null` connection target with "disconnected",
but the auto-connect (restore-on-activation) path never calls
`setTarget`, so the target stayed `null` while live.

**Fix:** a connected manager with no explicit target is, by
construction, the default local server (cloud always sets a target via
the picker), so the label is now derived from connection status in a
pure, centralized `selectModeLabel` selector.
- `src/state/selectors.ts`, `src/ui/statusBar.ts`
- New test: `test/pure/statusBarModeLabel.test.ts`

## Verification
- MCP server: `npm run build`, `npm run lint`, `npm test` → **363/363
pass**
- Extension: compile, `npm run lint`, pure tests + coverage → **156/156
pass, 93.18%** (threshold 80%)
- `make lint` clean across both packages
- No test was skipped, deleted, or had assertions removed.

Closes #11
Closes #12
Closes #17

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: MelbourneDeveloper

too-many-cooks/packages/core/src/server.ts

@@ -148,8 +148,11 @@ const planZodSchema: z.ZodRawShape = {
     .describe("Agent key for authentication (optional, uses session if omitted)"),
 };
 
-/** Zod schema for status tool input (empty). */
-const statusZodSchema: z.ZodRawShape = {};
+/** Zod schema for status tool input. */
+const statusZodSchema: z.ZodRawShape = {
+  agent_key: z.string().optional()
+    .describe("Agent key for authentication (optional, uses session if omitted)"),
+};
 
 /**
  * Wrap a local ToolCallback so it satisfies the MCP SDK's typed callback
@@ -203,7 +206,7 @@ const registerTools: (
   server.registerTool(
     "status",
     { description: statusToolConfig.description, inputSchema: statusZodSchema },
-    wrapHandler(createStatusHandler(db, log)),
+    wrapHandler(createStatusHandler(db, log, getSession)),
   );
 };
 

too-many-cooks/packages/core/src/tools/status_tool.ts

@@ -12,18 +12,25 @@ import {
 } from "../types.js";
 import {
   type CallToolResult,
+  type SessionGetter,
   type ToolCallback,
   textContent,
 } from "../mcp-types.js";
-import { makeErrorResult } from "./tool_utils.js";
+import { BROADCAST_RECIPIENT } from "../notifications.js";
+import { type IdentityResult, makeErrorResult, resolveIdentity } from "./tool_utils.js";
 
-/** Input schema for status tool (no inputs required). */
+/** Input schema for status tool. */
 export const STATUS_INPUT_SCHEMA: {
   readonly type: "object";
-  readonly properties: Record<string, never>;
+  readonly properties: Record<string, unknown>;
 } = {
   type: "object",
-  properties: {},
+  properties: {
+    agent_key: {
+      type: "string",
+      description: "Agent key for authentication (optional, uses session if omitted)",
+    },
+  },
 } as const;
 
 /** Tool config for status. */
@@ -41,15 +48,28 @@ export const STATUS_TOOL_CONFIG: {
   annotations: null,
 } as const;
 
+/// [MSG-PRIVACY] Issue #11: an agent may only see broadcasts plus its own
+/// (sent or received) messages. Direct messages addressed to other agents
+/// must never be returned by the status overview. A caller with no resolved
+/// identity (agentName === null) sees broadcasts only.
+const isVisibleTo: (message: Message, agentName: string | null) => boolean = (
+  message: Message,
+  agentName: string | null,
+): boolean =>
+  message.toAgent === BROADCAST_RECIPIENT ||
+  (agentName !== null && (message.toAgent === agentName || message.fromAgent === agentName));
+
 /** Create status tool handler. */
 export const createStatusHandler: (
   db: TooManyCooksDb,
   logger: Logger,
+  getSession?: SessionGetter,
 ) => ToolCallback = (
   db: TooManyCooksDb,
   logger: Logger,
+  getSession: SessionGetter = (): null => null,
 ): ToolCallback =>
-  {return async (): Promise<CallToolResult> => {
+  {return async (args: Record<string, unknown>): Promise<CallToolResult> => {
     const log: Logger = logger.child({ tool: "status" });
 
     const agentsResult: Result<readonly AgentIdentity[], DbError> = await db.listAgents();
@@ -66,7 +86,13 @@ export const createStatusHandler: (
 
     const messagesResult: Result<readonly Message[], DbError> = await db.listAllMessages();
     if (!messagesResult.ok) {return makeErrorResult(messagesResult.error);}
-    const messages: Array<Record<string, unknown>> = messagesResult.value.map(messageToJson);
+
+    // [MSG-PRIVACY] Issue #11: filter direct messages by resolved caller identity.
+    const identity: IdentityResult = await resolveIdentity(db, args, getSession);
+    const agentName: string | null = identity.isError ? null : identity.agentName;
+    const messages: Array<Record<string, unknown>> = messagesResult.value
+      .filter((message: Message): boolean => isVisibleTo(message, agentName))
+      .map(messageToJson);
 
     log.debug("Status queried");
 

too-many-cooks/packages/too-many-cooks/test/integration_test.ts

@@ -406,27 +406,39 @@ describe("Too Many Cooks MCP Server Integration", () => {
       });
     }
 
-    // Check status - MUST include messages!
+    // Check status as a known agent. [MSG-PRIVACY] Issue #11: status only
+    // returns messages visible to the caller (broadcasts + its own sent or
+    // received), so the ring of direct sends yields exactly the two that touch
+    // the viewer (one it sent, one it received).
+    const viewer = agents[0]!;
     const statusJson = JSON.parse(
-      await client.callTool("status", {}),
+      await client.callTool("status", { agent_key: viewer.key }),
     ) as Record<string, unknown>;
     assert.strictEqual((statusJson.agents as unknown[]).length, 5);
     assert.strictEqual((statusJson.locks as unknown[]).length, 5);
     assert.strictEqual((statusJson.plans as unknown[]).length, 5);
-    // CRITICAL: Status MUST return messages!
+    // CRITICAL: Status MUST include the messages field.
     assert.strictEqual(
       "messages" in statusJson,
       true,
       "Status response MUST include messages field",
     );
+    const msgs = statusJson.messages as Array<Record<string, unknown>>;
     assert.strictEqual(
-      (statusJson.messages as unknown[]).length,
-      5,
-      "Status MUST return all 5 messages sent",
+      msgs.length,
+      2,
+      "Status returns only the caller's own sent/received messages",
     );
+    // [MSG-PRIVACY] Issue #11: every returned message must involve the caller.
+    for (const m of msgs) {
+      assert.strictEqual(
+        m.to_agent === viewer.name || m.from_agent === viewer.name,
+        true,
+        "status must not leak messages unrelated to the caller",
+      );
+    }
 
     // Verify message structure
-    const msgs = statusJson.messages as Array<Record<string, unknown>>;
     const firstMsg = msgs[0]!;
     assert.ok("id" in firstMsg);
     assert.ok("from_agent" in firstMsg);

too-many-cooks/packages/too-many-cooks/test/status_handler_test.ts

@@ -76,7 +76,9 @@ describe("status handler", () => {
     await db.sendMessage(reg1.value.agentName, reg1.value.agentKey, reg2.value.agentName, "hello");
 
     const handler = createStatusHandler(db, createTestLogger());
-    const result = await handler({}, {});
+    // [MSG-PRIVACY] Issue #11: the direct message (reg1 -> reg2) is only visible
+    // to its recipient, so query status as reg2 to see it.
+    const result = await handler({ agent_key: reg2.value.agentKey }, {});
     assert.strictEqual(result.isError, false);
     const parsed = JSON.parse(result.content[0].text) as Record<string, unknown>;
     assert.strictEqual((parsed.agents as unknown[]).length, 2);

too-many-cooks/packages/too-many-cooks/test/status_message_privacy_test.ts

@@ -0,0 +1,104 @@
+/// [MSG-PRIVACY] Issue #11: the `status` tool must NOT leak direct messages
+/// addressed to other agents. An agent calling `status` may only see
+/// broadcasts (to_agent = "*") plus its own inbox/sent messages.
+
+import { describe, it, beforeEach, afterEach } from "node:test";
+import assert from "node:assert";
+import fs from "node:fs";
+import {
+  type TooManyCooksDb,
+  createDataConfig,
+  createLoggerWithContext,
+  createLoggingContext,
+  createStatusHandler,
+} from "too-many-cooks-core";
+import { createDb } from "../src/db-sqlite.js";
+
+const TEST_DB_PATH = ".test_status_message_privacy.db";
+const BROADCAST = "*";
+
+const deleteIfExists = (filePath: string): void => {
+  try {
+    if (fs.existsSync(filePath)) {
+      fs.unlinkSync(filePath);
+    }
+  } catch {
+    // Ignore
+  }
+};
+
+const createTestLogger = () =>
+  createLoggerWithContext(createLoggingContext());
+
+type StatusMessage = {
+  readonly content: string;
+};
+
+const statusMessagesFor = async (
+  db: TooManyCooksDb,
+  agentKey: string,
+): Promise<readonly StatusMessage[]> => {
+  const handler = createStatusHandler(db, createTestLogger());
+  const result = await handler({ agent_key: agentKey }, {});
+  assert.strictEqual(result.isError, false, "status must not error");
+  const parsed = JSON.parse(result.content[0].text) as { readonly messages: readonly StatusMessage[] };
+  return parsed.messages;
+};
+
+describe("status tool message privacy (#11)", () => {
+  let db: TooManyCooksDb | undefined;
+
+  beforeEach(() => {
+    deleteIfExists(TEST_DB_PATH);
+    const config = createDataConfig({ dbPath: TEST_DB_PATH });
+    const result = createDb(config);
+    assert.strictEqual(result.ok, true);
+    if (!result.ok) { throw new Error("expected ok db"); }
+    db = result.value;
+  });
+
+  afterEach(() => {
+    db?.close();
+    deleteIfExists(TEST_DB_PATH);
+  });
+
+  it("does not expose a direct message between two other agents to a third agent", async () => {
+    if (!db) { throw new Error("expected db"); }
+    const alice = await db.register("privacy-alice");
+    const bob = await db.register("privacy-bob");
+    const carol = await db.register("privacy-carol");
+    if (!alice.ok || !bob.ok || !carol.ok) { throw new Error("expected registrations ok"); }
+
+    const secret = "secret-for-bob-only";
+    const sent = await db.sendMessage(alice.value.agentName, alice.value.agentKey, bob.value.agentName, secret);
+    assert.strictEqual(sent.ok, true, "direct send must succeed");
+
+    const carolMessages = await statusMessagesFor(db, carol.value.agentKey);
+    assert.strictEqual(
+      carolMessages.some((m) => m.content === secret),
+      false,
+      "Carol must NOT see a direct message addressed to Bob",
+    );
+  });
+
+  it("still shows the recipient their own direct message and broadcasts to everyone", async () => {
+    if (!db) { throw new Error("expected db"); }
+    const alice = await db.register("privacy-alice2");
+    const bob = await db.register("privacy-bob2");
+    const carol = await db.register("privacy-carol2");
+    if (!alice.ok || !bob.ok || !carol.ok) { throw new Error("expected registrations ok"); }
+
+    const direct = "direct-to-bob";
+    const broadcast = "hello-everyone";
+    await db.sendMessage(alice.value.agentName, alice.value.agentKey, bob.value.agentName, direct);
+    await db.sendMessage(alice.value.agentName, alice.value.agentKey, BROADCAST, broadcast);
+
+    const bobMessages = await statusMessagesFor(db, bob.value.agentKey);
+    assert.strictEqual(bobMessages.some((m) => m.content === direct), true, "Bob must see his own direct message");
+    assert.strictEqual(bobMessages.some((m) => m.content === broadcast), true, "Bob must see the broadcast");
+
+    const carolMessages = await statusMessagesFor(db, carol.value.agentKey);
+    assert.strictEqual(carolMessages.some((m) => m.content === direct), false, "Carol must NOT see Bob's direct message");
+    assert.strictEqual(carolMessages.some((m) => m.content === broadcast), true, "Carol must see the broadcast");
+  });
+});

too_many_cooks_vscode_extension/src/services/connectionManager.ts

@@ -82,15 +82,32 @@ interface ManagerState {
   target: ConnectionTarget | null;
 }
 
+/** Everything needed to spawn and reach the local server. */
+interface LocalServerSpec {
+  readonly bin: string;
+  readonly port: number;
+  readonly workspaceFolder: string;
+}
+
 /** Build the local base URL for a given port. */
 function buildLocalBaseUrl(port: number): string {
   return `${LOCAL_BASE_URL_PREFIX}${String(port)}`;
 }
 
-/** Poll until the local server responds on /admin/status. */
-async function pollUntilReady(baseUrl: string, log: LogFn): Promise<void> {
+/** Platform-aware spawn configuration for the local server (Issue #17).
+ *  On Windows the global npm bin is a `.cmd` shim that Node's spawn() can only
+ *  resolve via the shell; on posix the shell is unnecessary (and lets a missing
+ *  binary surface as an ENOENT 'error' event instead of a shell exit code). */
+export function buildLocalSpawnConfig(platform: NodeJS.Platform): { readonly shell: boolean } {
+  return { shell: platform === 'win32' };
+}
+
+/** Poll until the local server responds on /admin/status. Stops early if the
+ *  signal aborts (e.g. spawn failed first), so the loser of the startup race
+ *  does not keep polling a dead port. */
+async function pollUntilReady(baseUrl: string, log: LogFn, signal: AbortSignal): Promise<void> {
   const deadline: number = Date.now() + STARTUP_TIMEOUT_MS;
-  while (Date.now() < deadline) {
+  while (Date.now() < deadline && !signal.aborted) {
     const available: boolean = await checkServerAvailable(baseUrl);
     if (available) {
       log(`[ConnectionManager] Server ready at ${baseUrl}`);
@@ -100,16 +117,18 @@ async function pollUntilReady(baseUrl: string, log: LogFn): Promise<void> {
       setTimeout(resolve, POLL_INTERVAL_MS);
     });
   }
+  if (signal.aborted) { return; }
   throw new Error(`Local server did not start within ${String(STARTUP_TIMEOUT_MS)}ms`);
 }
 
 /** Spawn the too-many-cooks CLI as a child process. */
-function spawnLocalServer(port: number, workspaceFolder: string, log: LogFn): ChildProcess {
-  log(`[ConnectionManager] Spawning local server on port ${String(port)} (workspace: ${workspaceFolder})`);
-  const child: ChildProcess = spawn(LOCAL_SERVER_BIN, [], {
-    cwd: workspaceFolder,
+function spawnLocalServer(spec: LocalServerSpec, log: LogFn): ChildProcess {
+  log(`[ConnectionManager] Spawning local server '${spec.bin}' on port ${String(spec.port)} (workspace: ${spec.workspaceFolder})`);
+  const child: ChildProcess = spawn(spec.bin, [], {
+    cwd: spec.workspaceFolder,
     detached: false,
-    env: { ...process.env, [TMC_PORT_ENV]: String(port), [TMC_WORKSPACE_ENV]: workspaceFolder },
+    env: { ...process.env, [TMC_PORT_ENV]: String(spec.port), [TMC_WORKSPACE_ENV]: spec.workspaceFolder },
+    shell: buildLocalSpawnConfig(process.platform).shell,
     stdio: ['ignore', 'pipe', 'pipe'],
   });
   child.stderr?.on('data', (data: Buffer): void => {
@@ -121,6 +140,26 @@ function spawnLocalServer(port: number, workspaceFolder: string, log: LogFn): Ch
   return child;
 }
 
+/** Resolve once the server is ready, or reject promptly if the child fails to
+ *  spawn (ENOENT 'error'). Without this the failure is swallowed and the user
+ *  only sees the misleading generic startup timeout. Issue #17. */
+async function awaitServerReady(child: ChildProcess, spec: LocalServerSpec, log: LogFn): Promise<void> {
+  const controller: AbortController = new AbortController();
+  const failure: Promise<never> = new Promise<never>(
+    (_resolve: (value: never) => void, reject: (reason: Error) => void): void => {
+      child.once('error', (err: Error): void => {
+        reject(new Error(`Local server '${spec.bin}' could not be started: ${err.message}`));
+      });
+    },
+  );
+  try {
+    await Promise.race([pollUntilReady(buildLocalBaseUrl(spec.port), log, controller.signal), failure]);
+  } finally {
+    controller.abort();
+    child.removeAllListeners('error');
+  }
+}
+
 /** Kill a child process with SIGTERM, escalating to SIGKILL after grace period. */
 function killProcess(child: ChildProcess, log: LogFn): void {
   log('[ConnectionManager] Sending SIGTERM to local server');
@@ -154,7 +193,7 @@ async function validateCloudCredentials(target: CloudTarget, log: LogFn): Promis
 }
 
 /** Create a connection manager instance. */
-export function createConnectionManager(workspaceFolder: string, log: LogFn): ConnectionManager {
+export function createConnectionManager(workspaceFolder: string, log: LogFn, serverBin: string = LOCAL_SERVER_BIN): ConnectionManager {
   const state: ManagerState = {
     localProcess: null,
     mode: 'disconnected',
@@ -173,9 +212,10 @@ export function createConnectionManager(workspaceFolder: string, log: LogFn): Co
 
   async function startLocal(port: number = DEFAULT_LOCAL_PORT): Promise<void> {
     if (state.mode !== 'disconnected') { disconnect(); }
-    const child: ChildProcess = spawnLocalServer(port, workspaceFolder, log);
+    const spec: LocalServerSpec = { bin: serverBin, port, workspaceFolder };
+    const child: ChildProcess = spawnLocalServer(spec, log);
     state.localProcess = child;
-    await pollUntilReady(buildLocalBaseUrl(port), log);
+    await awaitServerReady(child, spec, log);
     Object.assign(state, { mode: 'local' satisfies ConnectionMode, target: { mode: 'local', port, transport: LOCAL_TRANSPORT } satisfies LocalTarget });
   }
 

too_many_cooks_vscode_extension/src/state/selectors.ts

@@ -1,11 +1,28 @@
 // Derived state selectors.
 
 import type { AgentDetails, AgentIdentity, AgentPlan, AppState, ConnectionStatus, FileLock, Message } from './types';
+import type { ConnectionTarget } from '../services/connectionTypes';
+
+/** Connection mode labels shown in the status bar. */
+export const MODE_LOCAL: string = 'Local/HTTP';
+export const MODE_CLOUD_STDIO: string = 'Cloud/stdio';
+export const MODE_CLOUD_HTTP: string = 'Cloud/HTTP';
+export const MODE_DISCONNECTED: string = 'Disconnected';
 
 export function selectConnectionStatus(state: Readonly<AppState>): ConnectionStatus {
   return state.connectionStatus;
 }
 
+/** Status-bar mode label for the current connection status and target.
+ *  Issue #12: a live connection with no explicit target is, by construction,
+ *  the default local server (cloud always sets a target via the picker), so it
+ *  must read as the local mode — never "Disconnected" while connected. */
+export function selectModeLabel(status: ConnectionStatus, target: ConnectionTarget | null): string {
+  if (status !== 'connected') { return MODE_DISCONNECTED; }
+  if (target === null || target.mode === 'local') { return MODE_LOCAL; }
+  return target.transport === 'stdio' ? MODE_CLOUD_STDIO : MODE_CLOUD_HTTP;
+}
+
 export function selectAgents(state: Readonly<AppState>): readonly AgentIdentity[] {
   return state.agents;
 }