Merge nucleic/nimble-thistle-heron-ol6t into dev

Sources/NucleicCore/Codex/CodexAppServerBackend.swift

@@ -313,15 +313,14 @@ public actor CodexAppServerBackend: AgentBackend {
                             token: token, sessionID: run.sessionID,
                             commandTracing: ContainerServiceSettings.commandTracingEnabled)
                     ) { _, new in new }
-                    // Point Codex at our MCP server: streamable HTTP over the in-guest control bridge
-                    // (same endpoint Claude uses), authenticated by the per-session bearer token read
-                    // from the env var — so the token stays out of any config file on disk.
-                    env["NUCLEIC_MCP_TOKEN"] = token
-                    mcpArgs = [
-                        "-c",
-                        "mcp_servers.nucleic.url=\"http://127.0.0.1:\(ContainerSpec.controlBridgePort)/mcp\"",
-                        "-c", "mcp_servers.nucleic.bearer_token_env_var=\"NUCLEIC_MCP_TOKEN\"",
-                    ]
+                    // Point Codex at our MCP server (streamable HTTP over the in-guest control
+                    // bridge — the same endpoint Claude uses — bearer token via env var so it stays
+                    // off disk) with the server's tools pre-approved, and strip Codex's own review
+                    // layer: Nucleic already gates every one of these tools. See CodexGateOwnership.
+                    env[CodexGateOwnership.tokenEnvVar] = token
+                    mcpArgs =
+                        CodexGateOwnership.launchArgs
+                        + CodexGateOwnership.nucleicMCPArgs(port: ContainerSpec.controlBridgePort)
                 }
                 handle = try await containerManager.exec(
                     name: name, workdir: cspec.workdir, env: env,
@@ -796,7 +795,48 @@ public actor CodexAppServerBackend: AgentBackend {
                 return true
             }
         }
-        return false
+        // Nested shapes: a guardian permission request identifies the call under
+        // `permissions[].mcpToolCall.{toolName,connectorName}` (and snake_case in the core payloads)
+        // rather than at the top level, so the checks above missed it entirely and Nucleic's own
+        // tools fell through to a generic "Grant permissions" prompt — or, with no channel able to
+        // answer, to a denial. See CodexGateOwnership for the launch-side half of this.
+        return containsNucleicMCPIdentity(p, depth: 0)
+    }
+
+    /// Keys Codex may carry the *server*/connector identity under, across its camelCase app-server
+    /// protocol and the snake_case guardian payloads. Matched only against an exact `nucleic`.
+    private static let nucleicServerKeys: Set<String> = [
+        "server", "serverName", "server_name", "mcpServer", "mcp_server",
+        "connector", "connectorName", "connector_name", "connectorId", "connector_id",
+    ]
+
+    /// Keys carrying the tool's own name. Matched only against the `mcp__nucleic__` prefix — a bare
+    /// `nucleic` is deliberately NOT accepted here, because keys this generic (`name`) collide with
+    /// unrelated payload fields, and this project is itself named "nucleic"; a bare match could
+    /// auto-allow an unrelated command approval.
+    private static let nucleicToolKeys: Set<String> = [
+        "tool", "toolName", "tool_name", "name", "toolTitle", "tool_title",
+    ]
+
+    /// Recursively look for the `nucleic` MCP server's identity anywhere in an approval payload.
+    /// Depth-bounded so a pathological payload can't spin.
+    private static func containsNucleicMCPIdentity(_ value: JSONValue, depth: Int) -> Bool {
+        guard depth <= 6 else { return false }
+        switch value {
+        case .object(let fields):
+            for (key, field) in fields {
+                if let name = field.stringValue {
+                    if nucleicServerKeys.contains(key), name == "nucleic" { return true }
+                    if nucleicToolKeys.contains(key), name.hasPrefix("mcp__nucleic__") { return true }
+                }
+                if containsNucleicMCPIdentity(field, depth: depth + 1) { return true }
+            }
+            return false
+        case .array(let items):
+            return items.contains { containsNucleicMCPIdentity($0, depth: depth + 1) }
+        default:
+            return false
+        }
     }
 
     /// Map an approval request's params onto a normalized `ApprovalRequest`. A command

Sources/NucleicCore/Codex/CodexAppServerDecoder.swift

@@ -230,12 +230,16 @@ public final class CodexAppServerDecoder {
         }
         guard phase == .completed else { return out }
         out.append(Decoded(nativeType: "item/mcpToolCall", kind: .toolCallCompleted(call)))
+        // A call Codex rejects on its own (its review layer, a disabled tool) reports `failed` with
+        // no `result` — which rendered as a bare hazard badge and nothing else. Fall back to whatever
+        // Codex did say so a failure stays legible instead of silent.
         out.append(
             Decoded(
                 nativeType: "item/mcpToolCall",
                 kind: .toolResult(
                     ToolResult(
-                        toolCallID: id, content: item["result"] ?? .null,
+                        toolCallID: id,
+                        content: item["result"] ?? item["error"] ?? .null,
                         isError: item["status"]?.stringValue == "failed"))))
         return out
     }

Sources/NucleicCore/Codex/CodexExecBackend.swift

@@ -317,15 +317,14 @@ public actor CodexExecBackend: AgentBackend {
                         Task { await self?.emit(kind, nativeType: nativeType) }
                     }
                     await platformToolRuntime.registerNucleicTools(on: server, token: token, run: run)
-                    // Point Codex at our MCP server: streamable HTTP over the in-guest control bridge,
-                    // authenticated by the per-session bearer token read from the env var — so the token
-                    // stays out of any config file on disk.
-                    env["NUCLEIC_MCP_TOKEN"] = token
-                    mcpArgs = [
-                        "-c",
-                        "mcp_servers.nucleic.url=\"http://127.0.0.1:\(ContainerSpec.controlBridgePort)/mcp\"",
-                        "-c", "mcp_servers.nucleic.bearer_token_env_var=\"NUCLEIC_MCP_TOKEN\"",
-                    ]
+                    // Point Codex at our MCP server (streamable HTTP over the in-guest control
+                    // bridge, bearer token via env var so it stays off disk) with the server's tools
+                    // pre-approved, and strip Codex's own review layer: Nucleic owns these gates, and
+                    // `codex exec` has no channel that could answer Codex's — see CodexGateOwnership.
+                    env[CodexGateOwnership.tokenEnvVar] = token
+                    mcpArgs =
+                        CodexGateOwnership.launchArgs
+                        + CodexGateOwnership.nucleicMCPArgs(port: ContainerSpec.controlBridgePort)
                 }
                 handle = try await containerManager.exec(
                     name: name, workdir: cspec.workdir, env: env,

Sources/NucleicCore/Codex/CodexExecDecoder.swift

@@ -183,12 +183,17 @@ public final class CodexExecDecoder {
         }
         guard phase == .completed else { return out }
         out.append(Decoded(nativeType: "item/mcp_tool_call", kind: .toolCallCompleted(call)))
+        // A call Codex rejects on its own (its review layer, a disabled tool) reports `failed` with
+        // no `result` — which rendered as a bare hazard badge and nothing else, giving the user no
+        // reason and the model nothing better to say than that it wasn't "authorized". Fall back to
+        // whatever Codex did say so a failure stays legible.
         out.append(
             Decoded(
                 nativeType: "item/mcp_tool_call",
                 kind: .toolResult(
                     ToolResult(
-                        toolCallID: id, content: item["result"] ?? .null,
+                        toolCallID: id,
+                        content: item["result"] ?? item["error"] ?? .null,
                         isError: item["status"]?.stringValue == "failed"))))
         return out
     }

Sources/NucleicCore/Codex/CodexGateOwnership.swift

@@ -0,0 +1,43 @@
+import Foundation
+
+/// Launch overrides that hand gate ownership to Nucleic for a *containerized* Codex run, so Codex's
+/// own approval/review layer stops shadowing gates Nucleic already owns.
+///
+/// Codex ships a "guardian" review layer (`features.guardian_approval`, **stable and ON by default**
+/// — validated against codex-cli 0.146.0) that assesses actions including `mcp_tool_call`, and can
+/// deny one outright or demand a permission grant over `permissions/requestApproval`. Nucleic's own
+/// MCP tools (`host_exec`, the VM tools, `linux_container`, …) are already gated by
+/// `ApprovalCoordinator` and Nucleic's UI, so for them that layer is pure interference: it fails
+/// closed under `codex exec --ask-for-approval never` (no channel can grant it), and the tool comes
+/// back as a bare failed `mcp_tool_call` item — no result, no reason — which the model reports to the
+/// user as not being "authorized". Turning it off makes Nucleic the single gate, matching how the
+/// Claude backend runs. No permission is bypassed: every tool still passes Nucleic's own gate.
+///
+/// Deliberately scoped to containerized runs. On a *host* run Codex's native command/file approvals
+/// are not interference — they are the seam Nucleic gates commands through (`interactiveApprovals`),
+/// and the container is anyway the isolation boundary that makes trusting our own gate reasonable.
+public enum CodexGateOwnership {
+    /// The env var carrying the per-session MCP bearer token, so it stays out of any config file.
+    public static let tokenEnvVar = "NUCLEIC_MCP_TOKEN"
+
+    /// Global `codex` args (before the `exec`/`app-server` subcommand) that strip Codex's review
+    /// layer for a containerized run. `-c features.<name>=<bool>` is Codex's documented feature
+    /// override (the same thing `--disable <FEATURE>` writes).
+    public static let launchArgs: [String] = ["-c", "features.guardian_approval=false"]
+
+    /// Global `codex` args pointing Codex at Nucleic's per-container MCP endpoint: streamable HTTP
+    /// over the in-guest control bridge, authenticated by the bearer token read from
+    /// ``tokenEnvVar``, with the server's tools pre-approved.
+    ///
+    /// `default_tools_approval_mode` (`auto | prompt | writes | approve`) defaults to `auto`, which
+    /// routes every call on this server back through the approval layer above — the second half of
+    /// the same interference, and the half that survives even when the guardian is off. `approve`
+    /// pre-approves the `nucleic` server specifically; nothing else is affected.
+    public static func nucleicMCPArgs(port: UInt16) -> [String] {
+        [
+            "-c", "mcp_servers.nucleic.url=\"http://127.0.0.1:\(port)/mcp\"",
+            "-c", "mcp_servers.nucleic.bearer_token_env_var=\"\(tokenEnvVar)\"",
+            "-c", "mcp_servers.nucleic.default_tools_approval_mode=\"approve\"",
+        ]
+    }
+}

Tests/NucleicCoreTests/CodexAppServerBackendTests.swift

@@ -48,4 +48,70 @@ import Testing
         #expect(CodexAppServerBackend.acceptedReasoningEffort(
             "unknown", model: "gpt-5.6-sol", codexPro: true) == nil)
     }
+
+    // MARK: - Nucleic-MCP auto-allow (CodexGateOwnership)
+
+    private func request(_ method: String, _ params: [String: JSONValue])
+        -> JSONRPCConnection.InboundRequest
+    {
+        JSONRPCConnection.InboundRequest(id: .string("1"), method: method, params: .object(params))
+    }
+
+    @Test func topLevelServerAndToolIdentityStillMatch() {
+        #expect(CodexAppServerBackend.concernsNucleicMCP(
+            request("item/tool/requestApproval", ["server": .string("nucleic")])))
+        #expect(CodexAppServerBackend.concernsNucleicMCP(
+            request("item/tool/requestApproval", ["toolName": .string("mcp__nucleic__mac_vm_exec")])))
+    }
+
+    /// A guardian permission request identifies the call *nested*, under the requested-permission
+    /// entry rather than at the top level — the shape a top-level-only check missed, which is how
+    /// Nucleic's own VM tools ended up prompting or failing instead of being auto-allowed.
+    @Test func nestedGuardianPermissionPayloadMatches() {
+        let nested = request(
+            "permissions/requestApproval",
+            [
+                "threadId": .string("t1"),
+                "permissions": .array([
+                    .object([
+                        "mcpToolCall": .object([
+                            "toolName": .string("mcp__nucleic__linux_container"),
+                            "connectorName": .string("nucleic"),
+                        ])
+                    ])
+                ]),
+            ])
+        #expect(CodexAppServerBackend.concernsNucleicMCP(nested))
+
+        // Same shape in the core's snake_case spelling.
+        let snake = request(
+            "permissions/requestApproval",
+            ["permissions": .array([.object(["mcp_tool_call": .object([
+                "tool_name": .string("mcp__nucleic__mac_vm_computer"),
+                "connector_name": .string("nucleic"),
+            ])])])])
+        #expect(CodexAppServerBackend.concernsNucleicMCP(snake))
+    }
+
+    /// The recursive match must not auto-allow an unrelated approval. This project is itself named
+    /// "nucleic", so a bare `name: "nucleic"` buried in a command approval's payload must NOT count
+    /// — only a server/connector key, or a `mcp__nucleic__`-qualified tool name, may.
+    @Test func unrelatedApprovalNamingTheProjectIsNotAutoAllowed() {
+        let commandApproval = request(
+            "item/commandExecution/requestApproval",
+            [
+                "itemId": .string("c1"),
+                "command": .string("rm -rf build"),
+                "workspace": .object(["name": .string("nucleic")]),
+            ])
+        #expect(!CodexAppServerBackend.concernsNucleicMCP(commandApproval))
+
+        let otherServer = request(
+            "permissions/requestApproval",
+            ["permissions": .array([.object(["mcpToolCall": .object([
+                "toolName": .string("mcp__github__create_issue"),
+                "connectorName": .string("github"),
+            ])])])])
+        #expect(!CodexAppServerBackend.concernsNucleicMCP(otherServer))
+    }
 }

Tests/NucleicCoreTests/CodexExecDecoderTests.swift

@@ -44,6 +44,35 @@ import Testing
         #expect(result.content.stringValue == "boom")
     }
 
+    /// A call Codex rejects itself (its review layer, a disabled tool) reports `failed` with no
+    /// `result`, which rendered as a hazard badge and nothing else — the user saw an empty card and
+    /// the model had nothing to report but that it wasn't "authorized". Keep the reason legible.
+    @Test func rejectedMCPToolCallSurfacesCodexsReasonNotAnEmptyError() {
+        let out = decodeLines("""
+            {"type":"item.completed","item":{"id":"m1","type":"mcp_tool_call","server":"nucleic","tool":"mac_vm_exec","status":"failed","error":"MCP tool call blocked by app configuration"}}
+            """)
+        #expect(tags(out) == ["toolCallStarted", "toolCallCompleted", "toolResult"])
+        guard case .toolResult(let result) = out[2].kind else {
+            Issue.record("not toolResult")
+            return
+        }
+        #expect(result.isError == true)
+        #expect(result.content.stringValue == "MCP tool call blocked by app configuration")
+    }
+
+    /// A successful call still prefers `result` — the error fallback must not shadow it.
+    @Test func succeededMCPToolCallKeepsItsResult() {
+        let out = decodeLines("""
+            {"type":"item.completed","item":{"id":"m2","type":"mcp_tool_call","server":"nucleic","tool":"mac_vm_exec","status":"completed","result":"ok"}}
+            """)
+        guard case .toolResult(let result) = out[2].kind else {
+            Issue.record("not toolResult")
+            return
+        }
+        #expect(result.isError == false)
+        #expect(result.content.stringValue == "ok")
+    }
+
     @Test func fileChangeUsesStringKind() {
         // The exec schema's `kind` is a plain string (not the app-server's tagged object).
         let out = decodeLines("""

Tests/NucleicCoreTests/CodexGateOwnershipTests.swift

@@ -0,0 +1,31 @@
+import Testing
+
+@testable import NucleicCore
+
+/// `CodexGateOwnership`: the launch overrides that stop Codex's own review layer from shadowing
+/// gates Nucleic already owns. Codex's guardian is stable and ON by default and assesses
+/// `mcp_tool_call`, so a containerized run's `host_exec`/VM/`linux_container` calls came back as a
+/// bare failed item — no result, no reason — which the model reported as not being "authorized".
+@Suite struct CodexGateOwnershipTests {
+    @Test func launchArgsDisableCodexReviewLayer() {
+        #expect(CodexGateOwnership.launchArgs == ["-c", "features.guardian_approval=false"])
+    }
+
+    /// The `nucleic` server must be pre-approved: `default_tools_approval_mode` defaults to `auto`,
+    /// which routes each call back through the approval layer even with the guardian off.
+    @Test func nucleicServerIsPreApprovedAndPointedAtTheControlBridge() {
+        let args = CodexGateOwnership.nucleicMCPArgs(port: 8765)
+        #expect(args.contains("mcp_servers.nucleic.url=\"http://127.0.0.1:8765/mcp\""))
+        #expect(args.contains("mcp_servers.nucleic.bearer_token_env_var=\"NUCLEIC_MCP_TOKEN\""))
+        #expect(args.contains("mcp_servers.nucleic.default_tools_approval_mode=\"approve\""))
+        // Every value is a `-c` override, so they must pair up.
+        #expect(args.count == 6)
+        #expect(args.filter { $0 == "-c" }.count == 3)
+    }
+
+    /// The token rides in an env var so it never lands in a config file on disk.
+    @Test func tokenIsNamedForEnvHandoffNotInlinedInConfig() {
+        #expect(CodexGateOwnership.tokenEnvVar == "NUCLEIC_MCP_TOKEN")
+        #expect(!CodexGateOwnership.nucleicMCPArgs(port: 1).contains { $0.contains("bearer_token=") })
+    }
+}

docs/ADAPTERS.md

@@ -291,6 +291,36 @@ The server *calls us*; we reply with the same JSON-RPC `id`. v2 methods (CONFIRM
 - The handler suspends on `ApprovalCoordinator.await` exactly like Claude's — the two backends
   are identical above the adapter.
 
+#### 2.4.1 Codex's own review layer is torn down for containerized runs ✅
+
+Codex ships a **guardian** review layer (`features.guardian_approval` — *stable and ON by default*,
+validated against codex-cli 0.146.0) that assesses actions including `mcp_tool_call` and can deny one
+or demand a grant. Nucleic already gates every tool it exposes (`ApprovalCoordinator` + its own UI),
+so for Nucleic's MCP tools that layer is pure interference — and it was **silently eating the platform
+tools**: under `codex exec --ask-for-approval never` nothing can answer it, so `host_exec` / the VM
+tools / `linux_container` came back as a bare `mcp_tool_call` with `status:"failed"` and no `result`
+— a hazard badge and nothing else in the transcript, which the model reported to the user as the tool
+not being "authorized".
+
+`CodexGateOwnership` supplies the launch overrides, added in the **containerized** branch of both
+Codex backends:
+
+| Override | Why |
+| --- | --- |
+| `-c features.guardian_approval=false` | Nucleic is the only gate; matches the `approvalsReviewer:"user"` we already send per thread. |
+| `-c mcp_servers.nucleic.default_tools_approval_mode="approve"` | Defaults to `auto`, which re-routes every call on our server through the approval layer *even with the guardian off*. Enum: `auto\|prompt\|writes\|approve`. |
+
+Deliberately **not** applied to host runs: there Codex's native command/file approvals are not
+interference, they *are* the seam Nucleic gates commands through, and the container is what makes
+trusting our own gate reasonable in the first place.
+
+`concernsNucleicMCP` is the belt-and-suspenders half — it auto-allows an approval concerning our
+`nucleic` server. It matches nested payloads too, because a guardian permission request identifies
+the call under `permissions[].mcpToolCall.{toolName,connectorName}` (snake_case in the core payloads)
+rather than at the top level. A *bare* `nucleic` is accepted only under a server/connector key: this
+project is itself named "nucleic", so matching a bare `name` could auto-allow an unrelated command
+approval.
+
 ### 2.5 Item/event mapping ✅
 
 Notifications are `{ "method": "...", "params": {...} }`; the per-item lifecycle is