fix(codex): require final approval decisions (#70751)

Require the Codex app-server bridge to wait for the final two-phase approval decision, while preserving the explicit no-route sentinel behavior.

Local gate on rebased branch: pnpm check:changed (20 files, 157 tests).

Thanks @Lucenx9.

Co-authored-by: Lucenx9 <185146821+Lucenx9@users.noreply.github.com>

extensions/codex/src/app-server/approval-bridge.test.ts

@@ -81,10 +81,9 @@ describe("Codex app-server approval bridge", () => {
 
   it("describes command approvals from parsed command actions when available", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-actions",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-actions", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-actions", decision: "allow-once" });
 
     await handleCodexAppServerApprovalRequest({
       method: "item/commandExecution/requestApproval",
@@ -121,10 +120,9 @@ describe("Codex app-server approval bridge", () => {
 
   it("sanitizes command previews before forwarding approval text and events", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-sanitized-command",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-sanitized-command", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-sanitized-command", decision: "allow-once" });
 
     await handleCodexAppServerApprovalRequest({
       method: "item/commandExecution/requestApproval",
@@ -159,10 +157,9 @@ describe("Codex app-server approval bridge", () => {
 
   it("preserves visible OSC-8 link labels in command previews", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-osc",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-osc", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-osc", decision: "allow-once" });
     const esc = "\u001b";
 
     await handleCodexAppServerApprovalRequest({
@@ -194,10 +191,9 @@ describe("Codex app-server approval bridge", () => {
 
   it("strips bidi and invisible formatting controls from command previews", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-bidi",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-bidi", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-bidi", decision: "allow-once" });
 
     await handleCodexAppServerApprovalRequest({
       method: "item/commandExecution/requestApproval",
@@ -228,10 +224,9 @@ describe("Codex app-server approval bridge", () => {
 
   it("marks oversized unsafe command previews as omitted", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-omitted-command",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-omitted-command", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-omitted-command", decision: "allow-once" });
     const esc = "\u001b";
     const oversizedPrefix = `${esc}]8;;https://example.com${esc}\\`.repeat(300);
 
@@ -267,10 +262,9 @@ describe("Codex app-server approval bridge", () => {
 
   it("marks clipped command previews even when a safe prefix remains", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-clipped-command",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-clipped-command", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-clipped-command", decision: "allow-once" });
 
     await handleCodexAppServerApprovalRequest({
       method: "item/commandExecution/requestApproval",
@@ -298,6 +292,137 @@ describe("Codex app-server approval bridge", () => {
     );
   });
 
+  it("does not trust request-time decisions for two-phase command approvals", async () => {
+    const params = createParams();
+    mockCallGatewayTool
+      .mockResolvedValueOnce({
+        id: "plugin:approval-untrusted",
+        status: "accepted",
+        decision: "allow-always",
+      })
+      .mockResolvedValueOnce({ id: "plugin:approval-untrusted", decision: "deny" });
+
+    const result = await handleCodexAppServerApprovalRequest({
+      method: "item/commandExecution/requestApproval",
+      requestParams: {
+        threadId: "thread-1",
+        turnId: "turn-1",
+        itemId: "cmd-untrusted",
+        command: "pnpm test",
+      },
+      paramsForRun: params,
+      threadId: "thread-1",
+      turnId: "turn-1",
+    });
+
+    expect(result).toEqual({ decision: "decline" });
+    expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
+      "plugin.approval.request",
+      "plugin.approval.waitDecision",
+    ]);
+    expect(params.onAgentEvent).toHaveBeenCalledWith(
+      expect.objectContaining({
+        stream: "approval",
+        data: expect.objectContaining({
+          status: "denied",
+          approvalId: "plugin:approval-untrusted",
+        }),
+      }),
+    );
+  });
+
+  it("only treats own null data-property request decisions as no-route", async () => {
+    const params = createParams();
+    const inheritedDecisionResult = Object.assign(Object.create({ decision: null }), {
+      id: "plugin:approval-inherited",
+      status: "accepted",
+    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce(inheritedDecisionResult)
+      .mockResolvedValueOnce({ id: "plugin:approval-inherited", decision: "allow-once" });
+
+    const result = await handleCodexAppServerApprovalRequest({
+      method: "item/commandExecution/requestApproval",
+      requestParams: {
+        threadId: "thread-1",
+        turnId: "turn-1",
+        itemId: "cmd-inherited",
+        command: "pnpm test",
+      },
+      paramsForRun: params,
+      threadId: "thread-1",
+      turnId: "turn-1",
+    });
+
+    expect(result).toEqual({ decision: "accept" });
+    expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
+      "plugin.approval.request",
+      "plugin.approval.waitDecision",
+    ]);
+  });
+
+  it("does not invoke request-time decision accessors", async () => {
+    const params = createParams();
+    const requestResult = {
+      id: "plugin:approval-accessor",
+      status: "accepted",
+      get decision() {
+        throw new Error("decision getter must not run");
+      },
+    };
+    mockCallGatewayTool
+      .mockResolvedValueOnce(requestResult)
+      .mockResolvedValueOnce({ id: "plugin:approval-accessor", decision: "allow-once" });
+
+    const result = await handleCodexAppServerApprovalRequest({
+      method: "item/commandExecution/requestApproval",
+      requestParams: {
+        threadId: "thread-1",
+        turnId: "turn-1",
+        itemId: "cmd-accessor",
+        command: "pnpm test",
+      },
+      paramsForRun: params,
+      threadId: "thread-1",
+      turnId: "turn-1",
+    });
+
+    expect(result).toEqual({ decision: "accept" });
+  });
+
+  it("does not fail when request-time decision descriptors throw", async () => {
+    const params = createParams();
+    const requestResult = new Proxy(
+      { id: "plugin:approval-proxy", status: "accepted" },
+      {
+        getOwnPropertyDescriptor(target, property) {
+          if (property === "decision") {
+            throw new Error("descriptor trap must not fail approval");
+          }
+          return Reflect.getOwnPropertyDescriptor(target, property);
+        },
+      },
+    );
+    mockCallGatewayTool
+      .mockResolvedValueOnce(requestResult)
+      .mockResolvedValueOnce({ id: "plugin:approval-proxy", decision: "allow-once" });
+
+    const result = await handleCodexAppServerApprovalRequest({
+      method: "item/commandExecution/requestApproval",
+      requestParams: {
+        threadId: "thread-1",
+        turnId: "turn-1",
+        itemId: "cmd-proxy",
+        command: "pnpm test",
+      },
+      paramsForRun: params,
+      threadId: "thread-1",
+      turnId: "turn-1",
+    });
+
+    expect(result).toEqual({ decision: "accept" });
+  });
+
   it("fails closed when no approval route is available", async () => {
     const params = createParams();
     mockCallGatewayTool.mockResolvedValueOnce({
@@ -389,10 +514,9 @@ describe("Codex app-server approval bridge", () => {
   });
   it("labels permission approvals explicitly with sanitized permission detail", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-3",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-3", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-3", decision: "allow-once" });
 
     const result = await handleCodexAppServerApprovalRequest({
       method: "item/permissions/requestApproval",
@@ -443,10 +567,9 @@ describe("Codex app-server approval bridge", () => {
 
   it("keeps permission detail bounded with truncated and redacted target samples", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-4",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-4", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-4", decision: "allow-once" });
 
     await handleCodexAppServerApprovalRequest({
       method: "item/permissions/requestApproval",
@@ -495,10 +618,9 @@ describe("Codex app-server approval bridge", () => {
 
   it("strips terminal and invisible controls from permission descriptions", async () => {
     const params = createParams();
-    mockCallGatewayTool.mockResolvedValueOnce({
-      id: "plugin:approval-permission-controls",
-      decision: "allow-once",
-    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-permission-controls", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-permission-controls", decision: "allow-once" });
 
     await handleCodexAppServerApprovalRequest({
       method: "item/permissions/requestApproval",

extensions/codex/src/app-server/approval-bridge.ts

@@ -3,6 +3,7 @@ import {
   type EmbeddedRunAttemptParams,
 } from "openclaw/plugin-sdk/agent-harness-runtime";
 import {
+  approvalRequestExplicitlyUnavailable,
   mapExecDecisionToOutcome,
   requestPluginApproval,
   type AppServerApprovalOutcome,
@@ -99,8 +100,8 @@ export async function handleCodexAppServerApprovalRequest(params: {
       message: "Codex app-server approval requested.",
     });
 
-    const decision = Object.prototype.hasOwnProperty.call(requestResult, "decision")
-      ? requestResult.decision
+    const decision = approvalRequestExplicitlyUnavailable(requestResult)
+      ? null
       : await waitForPluginApprovalDecision({ approvalId, signal: params.signal });
     const outcome = mapExecDecisionToOutcome(decision);
 

extensions/codex/src/app-server/elicitation-bridge.test.ts

@@ -104,6 +104,58 @@ describe("Codex app-server elicitation bridge", () => {
     ]);
   });
 
+  it("does not trust request-time decisions for two-phase MCP approvals", async () => {
+    mockCallGatewayTool
+      .mockResolvedValueOnce({
+        id: "plugin:approval-untrusted",
+        status: "accepted",
+        decision: "allow-always",
+      })
+      .mockResolvedValueOnce({ id: "plugin:approval-untrusted", decision: "deny" });
+
+    const result = await handleCodexAppServerElicitationRequest({
+      requestParams: buildApprovalElicitation(),
+      paramsForRun: createParams(),
+      threadId: "thread-1",
+      turnId: "turn-1",
+    });
+
+    expect(result).toEqual({ action: "decline", content: null, _meta: null });
+    expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
+      "plugin.approval.request",
+      "plugin.approval.waitDecision",
+    ]);
+  });
+
+  it("does not treat inherited request-time MCP decisions as final", async () => {
+    const inheritedDecisionResult = Object.assign(Object.create({ decision: null }), {
+      id: "plugin:approval-inherited",
+      status: "accepted",
+    });
+    mockCallGatewayTool
+      .mockResolvedValueOnce(inheritedDecisionResult)
+      .mockResolvedValueOnce({ id: "plugin:approval-inherited", decision: "allow-once" });
+
+    const result = await handleCodexAppServerElicitationRequest({
+      requestParams: buildApprovalElicitation(),
+      paramsForRun: createParams(),
+      threadId: "thread-1",
+      turnId: "turn-1",
+    });
+
+    expect(result).toEqual({
+      action: "accept",
+      content: {
+        approve: true,
+      },
+      _meta: null,
+    });
+    expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
+      "plugin.approval.request",
+      "plugin.approval.waitDecision",
+    ]);
+  });
+
   it("accepts current Codex MCP approval elicitations with an empty form schema", async () => {
     mockCallGatewayTool
       .mockResolvedValueOnce({ id: "plugin:approval-current", status: "accepted" })

extensions/codex/src/app-server/elicitation-bridge.ts

@@ -3,6 +3,7 @@ import {
   type EmbeddedRunAttemptParams,
 } from "openclaw/plugin-sdk/agent-harness-runtime";
 import {
+  approvalRequestExplicitlyUnavailable,
   mapExecDecisionToOutcome,
   requestPluginApproval,
   type AppServerApprovalOutcome,
@@ -198,8 +199,8 @@ async function requestPluginApprovalOutcome(params: {
       return "unavailable";
     }
 
-    const decision = Object.prototype.hasOwnProperty.call(requestResult, "decision")
-      ? requestResult.decision
+    const decision = approvalRequestExplicitlyUnavailable(requestResult)
+      ? null
       : await waitForPluginApprovalDecision({ approvalId, signal: params.signal });
     return mapExecDecisionToOutcome(decision);
   } catch {

extensions/codex/src/app-server/plugin-approval-roundtrip.ts

@@ -58,6 +58,19 @@ export async function requestPluginApproval(params: {
   ) as Promise<ApprovalRequestResult | undefined>;
 }
 
+export function approvalRequestExplicitlyUnavailable(result: unknown): boolean {
+  if (result === null || result === undefined || typeof result !== "object") {
+    return false;
+  }
+  let descriptor: PropertyDescriptor | undefined;
+  try {
+    descriptor = Object.getOwnPropertyDescriptor(result, "decision");
+  } catch {
+    return false;
+  }
+  return descriptor !== undefined && "value" in descriptor && descriptor.value === null;
+}
+
 export async function waitForPluginApprovalDecision(params: {
   approvalId: string;
   signal?: AbortSignal;