From ee7af2513f60b7dac704aada9e599af952ba15b4 Mon Sep 17 00:00:00 2001
From: gsxdsm <gsxdsm@users.noreply.github.com>
Date: Sun, 12 Jul 2026 11:44:10 -0700
Subject: [PATCH] fix(MAIN-008): address PR review feedback (#2020)

- Label namespaced mcp__* tools as resourceType "mcp" (not "research") so approvals/audit/dedupe keys describe external MCP actions
- Guard getTask in resumeApprovalAfterUnwindIfNeeded so deferred resume cannot mask execute() finally outcomes
---
 .../engine/src/__tests__/agent-action-gate.test.ts | 13 ++++++++++++-
 packages/engine/src/agent-action-gate.ts           | 13 +++++++++++--
 packages/engine/src/executor.ts                    | 14 +++++++++++++-
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/packages/engine/src/__tests__/agent-action-gate.test.ts b/packages/engine/src/__tests__/agent-action-gate.test.ts
index 5d2c5305ca..e13cf0179e 100644
--- a/packages/engine/src/__tests__/agent-action-gate.test.ts
+++ b/packages/engine/src/__tests__/agent-action-gate.test.ts
@@ -135,10 +135,21 @@ describe("agent-action-gate", () => {
       category: "network_api",
       disposition: "require-approval",
       operation: "mcp__postiz__integrationlist",
-      resourceType: "research",
+      resourceType: "mcp",
     });
   });
 
+  it("keeps built-in research network tools as research resource type", () => {
+    const decision = evaluateAgentActionGate({
+      agentId: "agent-1",
+      toolName: "fn_research_run",
+      args: {},
+      permissionPolicy: approvalPolicy,
+    });
+    expect(decision.category).toBe("network_api");
+    expect(decision.resourceType).toBe("research");
+  });
+
   it("executes an approved namespaced MCP operation once and never executes a denied one", async () => {
     const execute = vi.fn().mockResolvedValue({ content: [{ type: "text", text: "real-shape-result" }] });
     const tool = { name: "mcp__postiz__integrationlist", label: "List integrations", description: "", parameters: {}, execute };
diff --git a/packages/engine/src/agent-action-gate.ts b/packages/engine/src/agent-action-gate.ts
index b2b787e65f..774361fe58 100644
--- a/packages/engine/src/agent-action-gate.ts
+++ b/packages/engine/src/agent-action-gate.ts
@@ -16,7 +16,11 @@ import {
 } from "./gating-classifications.js";
 import { runtimeLog } from "./logger.js";
 
-export type AgentActionGateResourceType = "file" | "git" | "task" | "agent" | "research" | "command" | "other";
+/*
+FNXC:AgentGating 2026-07-12-18:35:
+MAIN-008 review: project MCP tools must not share the "research" resource type used by built-in network research tools. Operators and approval-dedupe keys need a distinct label for external MCP side effects; "mcp" is that resource type.
+*/
+export type AgentActionGateResourceType = "file" | "git" | "task" | "agent" | "research" | "command" | "mcp" | "other";
 
 export interface AgentActionGateDecision {
   disposition: "allow" | "block" | "require-approval";
@@ -186,10 +190,15 @@ export function evaluateAgentActionGate(params: {
     the external-action approval boundary. MCP tools are dynamically named and
     therefore cannot live in the static tool registry; classify the namespace
     as network_api instead of falling through to the exempt default.
+
+    FNXC:AgentGating 2026-07-12-18:35:
+    Built-in research tools keep resourceType "research". Namespaced mcp__*
+    tools use "mcp" so approval UI/audit metadata and dedupe keys describe an
+    external MCP action rather than a research read.
     */
     category = "network_api";
     operation = params.toolName;
-    resourceType = "research";
+    resourceType = params.toolName.startsWith("mcp__") ? "mcp" : "research";
   }
 
   /*
diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts
index b5095f567c..f46f108bbf 100644
--- a/packages/engine/src/executor.ts
+++ b/packages/engine/src/executor.ts
@@ -2892,8 +2892,20 @@ export class TaskExecutor {
   }
 
   private async resumeApprovalAfterUnwindIfNeeded(taskId: string): Promise<boolean> {
+    /*
+    FNXC:ApprovalResume 2026-07-12-18:35:
+    MAIN-008 review: this runs from execute()'s outer finally. A getTask throw
+    (hard-deleted task between deferral and consume) must not escape finally and
+    mask the original execute outcome — treat unreadable tasks as no deferred resume.
+    */
     if (!this.approvalResumeAfterUnwind.delete(taskId)) return false;
-    const latestTask = await this.store.getTask(taskId);
+    let latestTask;
+    try {
+      latestTask = await this.store.getTask(taskId);
+    } catch (error) {
+      executorLog.warn(`${taskId}: failed to read latest task state for deferred approval resume: ${error instanceof Error ? error.message : String(error)}`);
+      return false;
+    }
     if (latestTask.paused || latestTask.userPaused || latestTask.column !== "in-progress") return false;
     return this.dispatchUnpauseResume(latestTask);
   }