Remove MCP permission gating workarounds

packages/server/src/server/agent/providers/codex-mcp-agent.ts

@@ -319,7 +319,6 @@ class CodexMcpAgentSession implements AgentSession {
   private cachedRuntimeInfo: AgentRuntimeInfo | null = null;
   private pendingPermissions = new Map<string, AgentPermissionRequest>();
   private pendingPermissionHandlers = new Map<string, PendingPermission>();
-  private pendingToolEvents = new Map<string, Array<() => void>>();
   private resolvedPermissionRequests = new Set<string>();
   private eventQueue: Pushable<AgentStreamEvent | ProviderEvent> | null = null;
   private currentAbortController: AbortController | null = null;
@@ -613,7 +612,6 @@ class CodexMcpAgentSession implements AgentSession {
           : "denied";
     const reason = response.behavior === "deny" ? response.message : undefined;
     pending.resolve({ decision, reason });
-    this.flushQueuedToolEvents(requestId, response.behavior === "allow");
   }
 
   describePersistence(): AgentPersistenceHandle | null {
@@ -647,7 +645,6 @@ class CodexMcpAgentSession implements AgentSession {
     }
     this.pendingPermissionHandlers.clear();
     this.pendingPermissions.clear();
-    this.pendingToolEvents.clear();
     this.resolvedPermissionRequests.clear();
     this.eventQueue?.end();
     this.eventQueue = null;
@@ -811,97 +808,6 @@ class CodexMcpAgentSession implements AgentSession {
     });
   }
 
-  private getEffectiveApprovalPolicy(): string {
-    const modeId = this.currentMode ?? this.config.modeId ?? DEFAULT_CODEX_MODE_ID;
-    const preset = MODE_PRESETS[modeId] ?? MODE_PRESETS[DEFAULT_CODEX_MODE_ID];
-    return this.config.approvalPolicy ?? preset.approvalPolicy;
-  }
-
-  private shouldGatePermissions(): boolean {
-    const approvalPolicy = this.getEffectiveApprovalPolicy();
-    return approvalPolicy === "on-request" || approvalPolicy === "untrusted";
-  }
-
-  private queuePermissionGatedEvent(
-    callId: string | undefined,
-    event: Record<string, unknown>,
-    emitEvent: () => void
-  ): boolean {
-    if (!this.shouldGatePermissions()) {
-      return false;
-    }
-    if (!callId) {
-      return false;
-    }
-    const requestId = `permission-${callId}`;
-    if (this.resolvedPermissionRequests.has(requestId)) {
-      return false;
-    }
-    if (!this.pendingPermissions.has(requestId) && !this.pendingPermissionHandlers.has(requestId)) {
-      const permission = this.buildPermissionRequest(event);
-      if (!permission) {
-        return false;
-      }
-      this.pendingPermissions.set(permission.id, permission);
-      this.pendingPermissionHandlers.set(permission.id, {
-        request: permission,
-        resolve: () => {},
-        reject: () => {},
-      });
-      this.emitPermissionRequested(permission);
-    }
-    this.queueToolEvent(requestId, emitEvent);
-    return true;
-  }
-
-  private ensurePermissionRequestFromEvent(event: Record<string, unknown>): void {
-    const callId = normalizeCallId(event.call_id as string | undefined);
-    if (!callId) {
-      return;
-    }
-    const requestId = `permission-${callId}`;
-    if (this.resolvedPermissionRequests.has(requestId)) {
-      return;
-    }
-    if (this.pendingPermissions.has(requestId) || this.pendingPermissionHandlers.has(requestId)) {
-      return;
-    }
-    const permission = this.buildPermissionRequest(event);
-    if (!permission) {
-      return;
-    }
-    this.pendingPermissions.set(permission.id, permission);
-    this.pendingPermissionHandlers.set(permission.id, {
-      request: permission,
-      resolve: () => {},
-      reject: () => {},
-    });
-    this.emitPermissionRequested(permission);
-  }
-
-  private queueToolEvent(requestId: string, emitEvent: () => void): void {
-    const queued = this.pendingToolEvents.get(requestId);
-    if (queued) {
-      queued.push(emitEvent);
-      return;
-    }
-    this.pendingToolEvents.set(requestId, [emitEvent]);
-  }
-
-  private flushQueuedToolEvents(requestId: string, allow: boolean): void {
-    const queued = this.pendingToolEvents.get(requestId);
-    if (!queued) {
-      return;
-    }
-    this.pendingToolEvents.delete(requestId);
-    if (!allow) {
-      return;
-    }
-    for (const emitEvent of queued) {
-      emitEvent();
-    }
-  }
-
   private recordHistory(item: AgentTimelineItem): void {
     if (this.sessionId) {
       const history = SESSION_HISTORY.get(this.sessionId) ?? [];
@@ -1079,9 +985,6 @@ class CodexMcpAgentSession implements AgentSession {
             }),
           });
         };
-        if (this.queuePermissionGatedEvent(callId, event as Record<string, unknown>, emitEvent)) {
-          break;
-        }
         emitEvent();
         break;
       }
@@ -1141,18 +1044,9 @@ class CodexMcpAgentSession implements AgentSession {
             });
           }
         };
-        if (this.queuePermissionGatedEvent(callId, event as Record<string, unknown>, emitEvent)) {
-          break;
-        }
         emitEvent();
         break;
       }
-      case "exec_approval_request": {
-        if (this.shouldGatePermissions()) {
-          this.ensurePermissionRequestFromEvent(event as Record<string, unknown>);
-        }
-        break;
-      }
       case "patch_apply_begin": {
         const callId = normalizeCallId((event as { call_id?: string }).call_id);
         const changes = (event as { changes?: Record<string, unknown> }).changes ?? {};

plan.md

@@ -245,14 +245,51 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov
     ```
   - **Done (2025-12-24)**: Verified via debug script.
 
-- [ ] **Fix**: Update MODE_PRESETS to use `on-request` instead of `untrusted`.
+- [x] **Fix**: Update MODE_PRESETS to use `on-request` instead of `untrusted`.
 
   - Change `codex-mcp-agent.ts` MODE_PRESETS:
     - `read-only`: `approvalPolicy: "on-request"` (was `untrusted`)
     - `auto`: `approvalPolicy: "on-request"` (was `untrusted`)
   - Ensure elicitation handler returns `{ decision: "approved" | "denied" | ... }` format
   - Remove any workarounds that were compensating for missing elicitation
+  - **Done (2025-12-24 20:20)**: Removed synthetic permission gating/exec approval workarounds now that on-request elicitation is the default.
 
 - [ ] **Test (E2E)**: Rerun server vitest after fixes.
 
   - If failures: add follow-up fix tasks immediately after this item.
+
+- [ ] **Test (E2E)**: Permission flow parity - test both Codex MCP and Claude providers.
+
+  - Create/update E2E tests that verify permissions work for BOTH providers
+  - Test cases for each provider:
+    - Permission requested event fires when tool needs approval
+    - Permission granted → tool executes
+    - Permission denied → tool blocked
+    - Permission abort/interrupt → session handles gracefully
+  - Ensure test structure allows easy comparison between providers
+
+- [ ] **Audit**: Feature parity checklist for Codex MCP provider vs Claude provider.
+
+  - Document all capabilities the Claude provider supports
+  - Verify Codex MCP provider supports each one or document gaps
+  - Key areas to check:
+    - Streaming events (reasoning, text, tool calls)
+    - Session persistence/resume
+    - Abort/interrupt handling
+    - Runtime info reporting
+    - Mode switching
+
+- [ ] **Test (E2E)**: Comprehensive tool call coverage for Codex MCP provider.
+
+  - All tool call types must be tested and emit proper timeline events:
+    - **Command runs**: `shell_command` / `exec_command` → exit code, stdout, stderr
+    - **File edits**: `apply_patch` / file modifications → before/after content
+    - **File creations**: new file writes → file path, content
+    - **MCP tool calls**: external MCP server tools → tool name, input, output
+    - **Web search**: if supported → query, results
+    - **File reads**: read operations → file path, content snippet
+  - Each test should verify:
+    - Timeline item is emitted with correct `type` and `status`
+    - Tool `input` and `output` are captured
+    - `callId` is consistent across events
+    - Permission flow triggers when expected (for unsafe operations)