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
This commit is contained in:
gsxdsm
2026-07-12 11:44:10 -07:00
parent 23c732b2a8
commit ee7af2513f
3 changed files with 36 additions and 4 deletions

View File

@@ -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 };

View File

@@ -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";
}
/*

View File

@@ -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);
}