fix(local-mcp): make the relay always-ask gate fire for codex too

The relay always-ask permission gate keyed off Claude's `_meta.claudeCode.toolName`, which codex never writes — codex populates the neutral `_meta.posthog` channel. For codex the gate silently no-op'd, so in non-asking permission modes a relayed tool could auto-run on the user's machine without a prompt. Read the server through the adapter-neutral `readMcpToolDescriptor` (with the Claude `rawInput.toolName` fallback) so the gate fires for both adapters.

Also re-append the loopback relay entries on refresh_session (Django's refresh rebuilds the list without them, whose URL and per-run bearer live only in the agent-server), and cover both fixes plus the codex `_meta.posthog` gate shape with tests.

Generated-By: PostHog Code
Task-Id: 7c87c3a4-0be6-475e-a8ec-269140ded301

Author: richardsolomou

docs/cloud-mcp-import.md

@@ -50,10 +50,10 @@ private-network URLs) need the desktop **relay** — see
    just stays desktop-only, while the reverse would ship an unreachable server
    (or leak headers) to the sandbox.
 
-3. **Show** — the cloud task composer renders `LocalMcpServersButton`
-   (`packages/ui/src/features/task-detail/components/LocalMcpServersButton.tsx`),
-   a list of the user's servers annotated "Available in cloud" / "Requires
-   your machine" / "Not available in cloud".
+3. **Show** — the MCP servers view rail renders `LocalMcpRailSection`
+   (`packages/ui/src/features/local-mcp/LocalMcpRailSection.tsx`), listing the
+   user's local servers annotated "Available in cloud" / "Relayed via your
+   machine" / "Built into cloud runs" / "Not available in cloud".
 
 4. **Send** — importable servers are included in the run-creation payload
    (`imported_mcp_servers` in `buildCloudRunRequestBody`,

docs/cloud-mcp-relay.md

@@ -95,21 +95,30 @@ Per incoming HTTP request:
 JSON-RPC *notifications* (no `id`) are relayed fire-and-forget: emit the
 event, answer 202 immediately.
 
-Liveness: relay endpoints 503 when no client can service the request,
-reusing the permission relay's `hasReachableClient` (a direct SSE viewer OR
-an active durable event stream). An earlier design used a stricter
-`desktopSeenAt` timestamp, but in the durable-ingest topology the desktop
-reads the run's stream through the agent-proxy and never connects to the
-sandbox, so that stricter signal 503s every request — `hasReachableClient`
-is the correct gate. Relay endpoints only exist when a desktop designated
+Liveness: the relay endpoints use the permission relay's `hasReachableClient`
+signal (a direct SSE viewer OR an active durable event stream) but must not
+503 during the ~2 s startup window before the first client attaches — an MCP
+client connects to each server once at session start, so a 503 there drops
+the server for the whole run. So the endpoint tracks `everReachable` and only
+503s once a client has been reachable and then went away (a genuine mid-run
+desktop disconnect). Before the first client ever attaches, the request is
+buffered — `broadcastEvent` already buffers-and-replays events until a
+controller attaches — and resolves when the client arrives or the request
+times out. (An earlier design gated purely on a `desktopSeenAt` timestamp,
+which 503'd the startup handshake in the durable-ingest topology, where the
+desktop reads the run's stream through the agent-proxy and never connects to
+the sandbox directly; that's why the gate is `everReachable`-then-lost, not
+"seen recently".) Relay endpoints only exist when a desktop designated
 servers at creation, so a non-headless run is the precondition anyway. So:
 
-- Claude gets an MCP connection error it reports cleanly, not a 60 s hang.
-- Codex-style reachability probes (which treat any HTTP response as
-  reachable but connection failures as not — see `isMcpServerReachable` in
-  `packages/workspace-server/src/services/agent/agent.ts`) must be extended
-  in the sandbox to treat 503 from loopback relay endpoints as unreachable
-  when pruning for Codex sessions.
+- Claude gets a clean MCP error on a genuine mid-run disconnect, not a 60 s
+  hang, and its session-start handshake is never dropped by a startup 503.
+- Codex reachability probes treat any HTTP response (including the buffered
+  200 or a 503) as reachable and connection failures as not — see
+  `isMcpServerReachable` in
+  `packages/workspace-server/src/services/agent/agent.ts` — so a loopback
+  relay endpoint always probes as reachable and is never pruned from a Codex
+  session.
 
 Headless-started runs (web/mobile/Slack) have no desktop: those creation
 paths never declare relayed servers, so the endpoints simply don't exist.
@@ -232,7 +241,8 @@ persisted, with caps and the no-secrets rule above.**
 
 | Failure | Behavior |
 | --- | --- |
-| Desktop offline at call time | Loopback endpoint 503s; agent sees "requires the desktop app" MCP error; Codex pruning treats the server as unreachable. |
+| Desktop never attaches (offline from session start) | Request buffers until it times out (60 s) → agent gets JSON-RPC `-32001`; the endpoint does **not** 503 during startup (that would drop the server for the whole run). |
+| Desktop disconnects mid-run (was reachable, now gone) | Endpoint 503s (`everReachable && !reachable`); agent sees a clean "requires the desktop app" MCP error rather than a hang. |
 | Desktop disconnects mid-call | Sandbox timeout fires (60 s), agent gets JSON-RPC `-32001`. |
 | Desktop reconnects after backlog | Replayed `mcp_request` events past `expiresAt` are dropped; unexpired ones are deduped by `requestId`. |
 | stdio process crashes | Desktop replies with `error`; next request lazily respawns. |
@@ -241,8 +251,9 @@ persisted, with caps and the no-secrets rule above.**
 ## Testing plan
 
 - Sandbox: unit tests for the correlation map — resolve, timeout, late
-  response, oversized payload, notification fire-and-forget, 503 when no
-  desktop seen (Vitest, faked clock).
+  response, oversized payload, notification fire-and-forget, startup
+  buffering before any client attaches, and 503 only after a client was
+  reachable and then went away (Vitest).
 - Desktop: unit tests for `McpRelayService` — name designation enforcement,
   requestId dedupe, expiry drop, stdio spawn failure → error reply (faked
   MCP client).

packages/agent/src/server/agent-server.test.ts

@@ -1324,6 +1324,25 @@ describe("AgentServer HTTP Mode", () => {
       };
     }
 
+    // Codex never writes `_meta.claudeCode`; it populates the neutral
+    // `_meta.posthog` channel with a structured `mcp` descriptor and sets
+    // rawInput to the tool arguments (no toolName). The gate must still fire.
+    function codexPermissionRequestFor(server: string, tool: string) {
+      return {
+        options: [{ optionId: "allow_once", kind: "allow_once" }],
+        toolCall: {
+          kind: "other",
+          _meta: {
+            posthog: {
+              toolName: `mcp__${server}__${tool}`,
+              mcp: { server, tool },
+            },
+          },
+          rawInput: { some: "arg" },
+        },
+      };
+    }
+
     const basePayload = {
       run_id: "run-1",
       task_id: "task-1",
@@ -1425,6 +1444,122 @@ describe("AgentServer HTTP Mode", () => {
         optionId: "allow_once",
       });
     });
+
+    it("treats a codex relay tool call (posthog _meta channel) as always-ask", async () => {
+      const testServer = exposeCloudClient(createServer());
+      testServer.config.relayMcpServers = ["slack"];
+      testServer.session = { hasDesktopConnected: true };
+      const relaySpy = vi
+        .spyOn(testServer, "relayPermissionToClient")
+        .mockResolvedValue({
+          outcome: { outcome: "selected", optionId: "allow_once" },
+        });
+
+      const { requestPermission } = testServer.createCloudClient(basePayload);
+      await requestPermission(
+        codexPermissionRequestFor("slack", "send_message"),
+      );
+
+      expect(relaySpy).toHaveBeenCalledOnce();
+    });
+
+    it("denies a codex relay tool call when no client is reachable", async () => {
+      const testServer = exposeCloudClient(createServer());
+      testServer.config.relayMcpServers = ["slack"];
+      testServer.session = null;
+      testServer.eventStreamSender = null;
+      const relaySpy = vi.spyOn(testServer, "relayPermissionToClient");
+
+      const { requestPermission } = testServer.createCloudClient(basePayload);
+      const result = await requestPermission(
+        codexPermissionRequestFor("slack", "send_message"),
+      );
+
+      expect(relaySpy).not.toHaveBeenCalled();
+      expect(result.outcome).toEqual({ outcome: "cancelled" });
+    });
+  });
+
+  describe("refresh_session relay re-append", () => {
+    function exposeRefresh(testServer: AgentServer) {
+      return testServer as unknown as {
+        session: {
+          clientConnection: { extMethod: ReturnType<typeof vi.fn> };
+        } | null;
+        mcpRelayServer: { mcpServers: unknown[] } | null;
+        executeCommand(
+          method: string,
+          params: Record<string, unknown>,
+        ): Promise<unknown>;
+      };
+    }
+
+    it("re-appends the loopback relay entries so a refresh doesn't drop them", async () => {
+      const testServer = exposeRefresh(createServer());
+      const extMethod = vi.fn(async () => ({ refreshed: true }));
+      testServer.session = { clientConnection: { extMethod } };
+      const relayEntry = {
+        type: "http",
+        name: "slack",
+        url: "http://127.0.0.1:5555/relay/slack",
+        headers: [{ name: "Authorization", value: "Bearer secret" }],
+      };
+      testServer.mcpRelayServer = { mcpServers: [relayEntry] };
+
+      // Django's refresh list carries posthog + imported, never the relay entries.
+      await testServer.executeCommand("refresh_session", {
+        mcpServers: [
+          { type: "http", name: "posthog", url: "https://mcp", headers: [] },
+        ],
+      });
+
+      expect(extMethod).toHaveBeenCalledOnce();
+      const forwarded = (extMethod.mock.calls[0] as unknown[])[1] as {
+        mcpServers: Array<{ name: string }>;
+      };
+      expect(forwarded.mcpServers.map((s) => s.name)).toEqual([
+        "posthog",
+        "slack",
+      ]);
+
+      // Detach the fake session so afterEach's stop() short-circuits before
+      // touching the partial session/relay stubs.
+      testServer.session = null;
+    });
+
+    it("does not duplicate a relay entry already present in the refresh list", async () => {
+      const testServer = exposeRefresh(createServer());
+      const extMethod = vi.fn(async () => ({ refreshed: true }));
+      testServer.session = { clientConnection: { extMethod } };
+      testServer.mcpRelayServer = {
+        mcpServers: [
+          {
+            type: "http",
+            name: "slack",
+            url: "http://127.0.0.1:5555/relay/slack",
+            headers: [],
+          },
+        ],
+      };
+
+      await testServer.executeCommand("refresh_session", {
+        mcpServers: [
+          {
+            type: "http",
+            name: "slack",
+            url: "http://127.0.0.1:5555/relay/slack",
+            headers: [],
+          },
+        ],
+      });
+
+      const forwarded = (extMethod.mock.calls[0] as unknown[])[1] as {
+        mcpServers: Array<{ name: string }>;
+      };
+      expect(forwarded.mcpServers.map((s) => s.name)).toEqual(["slack"]);
+
+      testServer.session = null;
+    });
   });
 
   describe("GET /events", () => {

packages/agent/src/server/agent-server.ts

@@ -21,6 +21,7 @@ import {
   buildPrOutput,
   getErrorMessage,
   mergePrUrls,
+  readMcpToolDescriptor,
   readPrUrls,
 } from "@posthog/shared";
 import { unzipSync } from "fflate";
@@ -1180,13 +1181,31 @@ export class AgentServer {
           return { refreshed: true };
         }
 
+        // refresh_session replaces the session's MCP server list wholesale, and
+        // Django's refresh rebuilds it from posthog + user + imported configs —
+        // it can't include the relay loopback entries, whose URL and per-run
+        // bearer live only here. Re-append them so a mid-run refresh (token
+        // rotation, follow-up past the refresh window) doesn't silently drop
+        // every relayed server from the session.
+        const relayServers = this.mcpRelayServer?.mcpServers ?? [];
+        const refreshedMcpServers = [
+          ...mcpServers,
+          ...relayServers.filter(
+            (relay) =>
+              !mcpServers.some(
+                (s: { name?: unknown }) => s?.name === relay.name,
+              ),
+          ),
+        ];
+
         this.logger.debug("Refresh session requested", {
-          serverCount: mcpServers.length,
+          serverCount: refreshedMcpServers.length,
+          relayServerCount: relayServers.length,
         });
 
         return await this.session.clientConnection.extMethod(
           POSTHOG_METHODS.REFRESH_SESSION,
-          { mcpServers },
+          { mcpServers: refreshedMcpServers },
         );
       }
 
@@ -3638,19 +3657,21 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}
         // (docs/cloud-mcp-relay.md). Without a reachable client, deny rather
         // than auto-approve.
         {
-          const meta = params.toolCall?._meta as
-            | { claudeCode?: { toolName?: string } }
-            | undefined;
+          // Read the MCP server through the adapter-neutral `_meta.posthog`
+          // channel (codex writes `_meta.posthog.mcp`, Claude writes the legacy
+          // `_meta.claudeCode.toolName`; readMcpToolDescriptor handles both),
+          // falling back to Claude's `rawInput.toolName`. Keying off only the
+          // Claude channel would silently skip this gate for codex and let a
+          // relayed tool auto-run in non-asking modes.
           const rawInput = params.toolCall?.rawInput as
             | { toolName?: string }
             | undefined;
-          const permissionToolName =
-            meta?.claudeCode?.toolName ?? rawInput?.toolName;
           const mcpServerName =
-            typeof permissionToolName === "string" &&
-            permissionToolName.startsWith("mcp__")
-              ? permissionToolName.split("__")[1]
-              : undefined;
+            readMcpToolDescriptor(params.toolCall?._meta)?.server ??
+            (typeof rawInput?.toolName === "string" &&
+            rawInput.toolName.startsWith("mcp__")
+              ? rawInput.toolName.split("__")[1]
+              : undefined);
           if (
             mcpServerName &&
             (this.config.relayMcpServers ?? []).includes(mcpServerName)

packages/core/src/cloud-task/cloud-task.test.ts

@@ -3283,4 +3283,49 @@ describe("CloudTaskService MCP relay", () => {
       ),
     ).toBe(false);
   });
+
+  it("posts a -32000 mcp_response when the executor throws", async () => {
+    mcpRelayExecutor.execute.mockRejectedValueOnce(new Error("spawn failed"));
+    mockStreamFetch.mockResolvedValueOnce(
+      createOpenSseResponse(
+        mcpRequestSseLine({ requestId: "req-1", server: "slack" }),
+      ),
+    );
+    mockNetFetch.mockResolvedValueOnce(createJsonResponse({ result: {} }));
+    relayService.designateRelayedMcpServers("run-1", ["slack"]);
+    watchRun("run-1");
+
+    await waitFor(() =>
+      mockNetFetch.mock.calls.some(([url]) =>
+        (url as string).includes("/command/"),
+      ),
+    );
+    const commandCall = mockNetFetch.mock.calls.find(([url]) =>
+      (url as string).includes("/command/"),
+    );
+    const body = JSON.parse((commandCall?.[1] as RequestInit).body as string);
+    // A thrown executor error must still answer the sandbox — otherwise it hangs to timeout.
+    expect(body).toEqual(
+      expect.objectContaining({
+        method: "mcp_response",
+        params: {
+          requestId: "req-1",
+          server: "slack",
+          error: { code: -32000, message: "spawn failed" },
+        },
+      }),
+    );
+  });
+
+  it("closes the run's relay connections when the run is unwatched", async () => {
+    mockStreamFetch.mockResolvedValue(createOpenSseResponse(""));
+    relayService.designateRelayedMcpServers("run-1", ["slack"]);
+    watchRun("run-1");
+    await waitFor(() => mockStreamFetch.mock.calls.length > 0);
+
+    relayService.unwatch("task-1", "run-1");
+    await waitFor(() => mcpRelayExecutor.closeRun.mock.calls.length > 0);
+
+    expect(mcpRelayExecutor.closeRun).toHaveBeenCalledWith("run-1");
+  });
 });