fix(exec): time out hung MCP connect before first inference (#1096)

* fix(exec): time out hung MCP connect before first inference

* fix(exec): keep MCP handshake abort armed and resume after connect

A rejected connectMCP batch used to disarm the 15s abort while sibling dials were still in flight, so dispose could block on allSettled. Workflow resume also ran after the 1s wait while handshake was still pending, so capability-gated steps skipped MCP tools. Connecting is abort-capped, so waiting for settle before resume cannot hang forever.

Author: TheGreatAxios

docs/ARCHITECTURE.md

@@ -118,6 +118,7 @@ In TUI chat mode there is no completion gate — the session stays open across t
   - No workflow controller (`isWorkflowActive` is always false)
   - Non-interactive permission gate by default
   - `ask_operator` is unmounted when stdin/stdout are not TTYs (no cancel stub on the wire); TTY exec still prompts on stdin
+  - MCP connect is awaited before workflow resume and first inference, abort-capped at 15s so a hung handshake cannot block the run. A rejected batch does not disarm that abort while sibling dials are still in flight. A 1s log fires if connect is still in progress; remaining dials keep running until settle or abort. The TUI still fire-and-forgets connect (no rewrite). `tool_search` does not treat an empty catalog as a definitive miss while servers are still connecting.
 - Entry: `corbits exec "prompt"` (alias `corbits run`); `loadConfig` sets `command: "exec"`
 - Streams assistant text deltas to stdout; lifecycle errors to stderr
 - Shares ChatDirector compaction continuation (`requestContinuation` → content-less deliver after compact) so long runs do not stall post-compact

docs/MCP.md

@@ -151,7 +151,17 @@ redialing. A `tool_search` miss waits up to 1 second for in-flight handshakes
 and looks again; when a reconnecting server holds tools that could match the
 query, the search takes one extra 500 ms extension for the redial to remount
 them. Servers still waiting on authorization never earn that extension — they
-settle only when authorization completes out of band.
+settle only when authorization completes out of band. An empty catalog is not
+a definitive miss while any server is still connecting: the result asks the
+model to retry shortly instead of advising different keywords.
+
+`corbits exec` waits for startup MCP connect before workflow resume and the first
+inference so capability-gated workflow steps can see MCP tools. A 1s log fires
+if the handshake is still in progress; remaining dials keep running until they
+settle or the 15s abort fires so a hung server cannot block the run. A rejected
+batch does not clear that abort while a sibling is still connecting. Each
+handshake is aborted independently, so a sibling that already connected is not
+torn down. The TUI still starts MCP in the background without that wait.
 
 ## Server Kinds
 

src/agent/tools-mcp-disconnect.test.ts

@@ -27,6 +27,8 @@ let connectFailureError = "redial refused";
 // When true the mock offers an auth URL (interactive needs-auth) before the
 // connectMode branch runs, so a redial can pend on the operator.
 let emitNeedsAuth = false;
+// Names that never settle until the connect AbortSignal fires.
+const hangNames = new Set<string>();
 // Reconnect tests repoint this to simulate a server whose tool set drifted
 // between generations; the default matches the original static payload.
 let connectedTools: MCPTool[] = [
@@ -67,7 +69,7 @@ await withMockedModule(
           error: connectFailureError,
         };
       }
-      if (connectMode === "deferred") {
+      if (connectMode === "deferred" || hangNames.has(config.name)) {
         await new Promise<void>((resolve) => {
           releaseDeferredConnect = resolve;
           const onAbort = (): void => resolve();
@@ -91,16 +93,29 @@ await withMockedModule(
           authPending: true,
         };
       }
+      let closed = false;
+      const close = async () => {
+        if (closed) return;
+        closed = true;
+        closedClients.push(config.name);
+        closedGenerations.push(generation);
+      };
+      // HTTP keeps `signal` on the live transport; abort after connect must
+      // tear the client down the way Streamable HTTP does.
+      if (options.signal !== undefined) {
+        const tearDown = (): void => {
+          void close();
+        };
+        if (options.signal.aborted) tearDown();
+        else options.signal.addEventListener("abort", tearDown, { once: true });
+      }
       return {
         ok: true as const,
         client: {
           serverName: config.name,
           tools: connectedTools,
           call: async () => "ok",
-          close: async () => {
-            closedClients.push(config.name);
-            closedGenerations.push(generation);
-          },
+          close,
         },
       };
     },
@@ -179,6 +194,7 @@ beforeEach(() => {
   failNextConnects = 0;
   connectFailureError = "redial refused";
   emitNeedsAuth = false;
+  hangNames.clear();
   connectedTools = [{ name: "list", description: "List", inputSchema: {} }];
 });
 
@@ -903,3 +919,85 @@ describe("unintentional disconnect and automatic reconnect", () => {
     }
   });
 });
+
+describe("MCP handshake bounds", () => {
+  test("a hung connect fails within the handshake abort bound", async () => {
+    connectMode = "deferred";
+    const toolset = await makeToolset();
+    const states: MCPServerState[] = [];
+    try {
+      const started = Date.now();
+      await toolset.connectMCPServer(
+        acme,
+        callbacks(states),
+        AbortSignal.timeout(50),
+      );
+      expect(Date.now() - started).toBeLessThan(500);
+      expect(states.some((s) => s.state === "failed")).toBe(true);
+    } finally {
+      releaseDeferredConnect?.();
+      await toolset.dispose();
+    }
+  });
+
+  test("a hung sibling does not abort a server that already connected", async () => {
+    hangNames.add("lin");
+    const toolset = await createAgentToolset({
+      cwd: tempCwd(),
+      permissionGate: permissionGate(),
+      onOperatorGate: async () => ({ kind: "cancel" }),
+      mcpServers: [acme, lin],
+    });
+    const states: MCPServerState[] = [];
+    try {
+      const started = Date.now();
+      await toolset.connectMCP(callbacks(states), AbortSignal.timeout(50));
+      expect(Date.now() - started).toBeLessThan(500);
+      expect(toolset.hasMCPServer("acme")).toBe(true);
+      expect(
+        toolset.dynamicRunner.currentDefinitions().map((d) => d.name),
+      ).toContain("mcp__acme__list");
+      expect(closedClients).not.toContain("acme");
+      expect(
+        states.some((s) => s.name === "acme" && s.state === "connected"),
+      ).toBe(true);
+      expect(states.some((s) => s.name === "lin" && s.state === "failed")).toBe(
+        true,
+      );
+      expect(toolset.hasMCPServer("lin")).toBe(false);
+    } finally {
+      await toolset.dispose();
+    }
+  });
+
+  test("tool_search retries while a handshake is still in flight", async () => {
+    connectMode = "deferred";
+    const toolset = await makeToolset();
+    const states: MCPServerState[] = [];
+    try {
+      const connecting = toolset.connectMCPServer(acme, callbacks(states));
+      await waitForConnectStart();
+      const remaining = await toolset.awaitPendingMcpConnections(20);
+      expect(remaining).toBe(1);
+
+      const search = toolset.dynamicRunner.run(
+        { id: "s1", name: "tool_search", arguments: { query: "acme list" } },
+        AbortSignal.timeout(5000),
+      );
+      // Production miss-wait is 1s; do not fake timers here — waitForConnectStart
+      // and the abort-bound hung-connect test use real clocks.
+      const result = await search;
+      expect(typeof result.content).toBe("string");
+      expect(result.content).toMatch(/still connecting|starting up/i);
+      expect(result.content).toMatch(/retry.*shortly/i);
+      expect(result.content).not.toMatch(/different keywords/i);
+      expect(result.content).not.toContain("mcp__acme__list");
+
+      releaseDeferredConnect?.();
+      await connecting;
+    } finally {
+      releaseDeferredConnect?.();
+      await toolset.dispose();
+    }
+  });
+});

src/agent/tools.ts

@@ -383,6 +383,33 @@ export interface AgentToolset {
   dispose: () => Promise<void>;
 }
 
+// Connect-only abort fan-out. The MCP client keeps `signal` on the live
+// transport, so a shared parent abort would tear down HTTP that already
+// connected. Forward until this handshake settles, then detach.
+function forwardAbortUntilDisarmed(parent: AbortSignal): {
+  signal: AbortSignal;
+  disarm: () => void;
+} {
+  const controller = new AbortController();
+  if (parent.aborted) {
+    controller.abort(parent.reason);
+    return { signal: controller.signal, disarm: () => undefined };
+  }
+  const onAbort = (): void => {
+    controller.abort(parent.reason);
+  };
+  parent.addEventListener("abort", onAbort, { once: true });
+  let disarmed = false;
+  return {
+    signal: controller.signal,
+    disarm: () => {
+      if (disarmed) return;
+      disarmed = true;
+      parent.removeEventListener("abort", onAbort);
+    },
+  };
+}
+
 export async function createAgentToolset(
   args: AgentToolsetArgs,
 ): Promise<AgentToolset> {
@@ -1227,13 +1254,15 @@ export async function createAgentToolset(
       perServer = new AbortController();
       serverAborts.set(config.name, perServer);
     }
+    const forwarded =
+      signal === undefined ? undefined : forwardAbortUntilDisarmed(signal);
     const connectionSignal =
-      signal === undefined
+      forwarded === undefined
         ? AbortSignal.any([mcpAbortController.signal, perServer.signal])
         : AbortSignal.any([
             mcpAbortController.signal,
             perServer.signal,
-            signal,
+            forwarded.signal,
           ]);
 
     const staleOrDisabled = (): boolean =>
@@ -1382,7 +1411,7 @@ export async function createAgentToolset(
         tools: result.client.tools.map((t) => t.name),
       });
       callbacks.onToolsChanged(dynamicRunner.currentDefinitions());
-    })();
+    })().finally(() => forwarded?.disarm());
     inFlightConnections.set(config.name, run);
     inFlightEpochs.set(config.name, ownedEpoch);
     const clearInFlight = (): void => {

src/exec/mcp-handshake.ts

@@ -0,0 +1,80 @@
+// Log threshold while MCP connect is still in flight. Workflow resume and first
+// inference wait for connecting to settle because capability gates skip MCP
+// tools that land after resume; the runner is sequential, so this also delays
+// first infer. Hung dials cannot wait forever: the abort cap below is the bound.
+export const EXEC_MCP_CONNECT_WAIT_MS = 1_000;
+// Cap on the handshake itself. Abort only reaches in-flight dials; a live
+// sibling detaches its forward on settle so this timer cannot tear it down.
+export const EXEC_MCP_HANDSHAKE_TIMEOUT_MS = 15_000;
+
+export async function awaitExecMcpConnect(
+  connecting: Promise<void>,
+  timeoutMs: number,
+): Promise<"settled" | "timeout"> {
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  try {
+    return await Promise.race([
+      connecting.then(() => "settled" as const),
+      new Promise<"timeout">((resolve) => {
+        timer = setTimeout(() => resolve("timeout"), timeoutMs);
+      }),
+    ]);
+  } finally {
+    if (timer !== undefined) clearTimeout(timer);
+  }
+}
+
+// Connect-only abort. The MCP client ties `signal` to the transport lifecycle,
+// so AbortSignal.timeout would kill a handshake that already succeeded. The
+// toolset forwards this signal per server and detaches on settle; abort only
+// reaches handshakes still in flight. Disarm only when the batch fulfills —
+// a rejected Promise.all still leaves sibling forwards armed.
+export function armExecMcpHandshakeAbort(timeoutMs: number): {
+  signal: AbortSignal;
+  disarm: () => void;
+} {
+  const controller = new AbortController();
+  const timer = setTimeout(() => {
+    controller.abort();
+  }, timeoutMs);
+  let disarmed = false;
+  return {
+    signal: controller.signal,
+    disarm: () => {
+      if (disarmed) return;
+      disarmed = true;
+      clearTimeout(timer);
+    },
+  };
+}
+
+export function followExecMcpHandshake(
+  connecting: Promise<void>,
+  handshake: { disarm: () => void },
+): Promise<void> {
+  return connecting.then(() => {
+    handshake.disarm();
+  });
+}
+
+function whenAborted(signal: AbortSignal): Promise<void> {
+  if (signal.aborted) return Promise.resolve();
+  return new Promise((resolve) => {
+    signal.addEventListener("abort", () => resolve(), { once: true });
+  });
+}
+
+export async function awaitExecMcpThenResume(
+  connecting: Promise<void>,
+  resume: () => Promise<void>,
+  options: {
+    waitMs: number;
+    abort: AbortSignal;
+    onWaitTimeout?: () => void;
+  },
+): Promise<void> {
+  const outcome = await awaitExecMcpConnect(connecting, options.waitMs);
+  if (outcome === "timeout") options.onWaitTimeout?.();
+  await Promise.race([connecting, whenAborted(options.abort)]);
+  await resume();
+}