diff --git a/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardHandler.java b/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardHandler.java index 78476f4a..c6c85d4f 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardHandler.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardHandler.java @@ -9,7 +9,9 @@ import com.lark.oapi.event.cardcallback.model.P2CardActionTriggerData; import com.lark.oapi.event.cardcallback.model.P2CardActionTriggerResponse; import lombok.extern.slf4j.Slf4j; import vip.mate.approval.ApprovalService; +import vip.mate.approval.ApprovalWorkflowService; import vip.mate.approval.PendingApproval; +import vip.mate.approval.ResolveOutcome; import vip.mate.channel.ChannelMessage; import vip.mate.channel.feishu.FeishuChannelAdapter; import vip.mate.channel.feishu.cards.FeishuCardHandler; @@ -59,11 +61,23 @@ import java.util.Optional; public class ToolGuardCardHandler implements FeishuCardHandler { private final ApprovalService approvalService; + /** + * ISSUE #413 P2-B3: needed to resolve workflow-scoped approvals + * ({@code wf-} pendingIds) directly from the card click. Workflow + * approvals cannot go through the synthetic /approve injection + * (their conversationId is {@code workflow:run:{runId}}, which no + * IM conversation matches), so the handler resolves them inline — + * mirroring the Web / WebChat path. May be null in narrow test + * contexts (wf- approvals then fall back to the admin console). + */ + private final ApprovalWorkflowService approvalWorkflowService; private final ToolGuardButtonValue buttonValue; public ToolGuardCardHandler(ApprovalService approvalService, + ApprovalWorkflowService approvalWorkflowService, ToolGuardButtonValue buttonValue) { this.approvalService = approvalService; + this.approvalWorkflowService = approvalWorkflowService; this.buttonValue = buttonValue; } @@ -101,6 +115,19 @@ public class ToolGuardCardHandler implements FeishuCardHandler { PendingApproval pending = opt.get(); // ---- 3. Identity check (fail-closed) + // Workflow-scoped approvals (wf- prefix, ISSUE #413 P2-B3) have no + // human requester — the userId is null because the run is system- + // initiated. Their approval cards are only ever pushed to the + // channels declared in await_approval's approverChannels, so any + // member of that audience is a legitimate approver; we skip the + // requester==clicker guard and resolve inline (the synthetic /approve + // injection is a dead end for wf- ids: their conversationId is + // workflow:run:{runId}, which no IM conversation matches, so the + // router's findPendingByConversation would miss it). + if (pendingId.startsWith("wf-")) { + return handleWorkflowApproval(pendingId, decoded.toolName(), act, clickerOpenId); + } + // Agent/cron ("system") or unattributed (null) approvals have no human // requester to match the clicker against. A guarded-tool card landing in // a group chat would otherwise let ANY member click Approve and run the @@ -142,6 +169,54 @@ public class ToolGuardCardHandler implements FeishuCardHandler { return buildResolvedResponse(decoded.toolName(), act, clickerOpenId); } + // ------------------------------------------------------------------ + // Workflow-scoped approval (ISSUE #413 P2-B3) + // ------------------------------------------------------------------ + + /** + * Resolve a {@code wf-} workflow approval directly from the card click, + * bypassing the synthetic /approve injection. Workflow approvals live + * under a synthetic {@code workflow:run:{runId}} conversationId that no + * IM conversation matches, so the router path is a dead end. Instead we + * resolve inline (mirroring the Web / WebChat path); the + * {@link vip.mate.workflow.runtime.ApprovalResumeBridge} then picks up + * the {@code WorkflowApprovalResolvedEvent} published inside + * {@code ApprovalWorkflowService.resolve} and resumes the paused run. + * + *

No tool-call replay is needed — a workflow {@code await_approval} + * step is a declarative gate, not a tool invocation; resume simply + * advances to the next step. + * + *

Identity: any audience member may resolve. The card only reaches + * the channels declared in {@code await_approval.approverChannels} + * (pushed by {@code AwaitApprovalStepAdapter}'s notify step), so whoever + * can see it is a designated approver. + */ + private P2CardActionTriggerResponse handleWorkflowApproval(String pendingId, String toolName, + ToolGuardButtonValue.Action act, + String clickerOpenId) { + if (approvalWorkflowService == null) { + log.warn("[feishu-toolguard] ApprovalWorkflowService unavailable, cannot resolve wf- {} " + + "(use the admin console)", pendingId); + return buildErrorResponse("⚠️ 工作流审批需在管理端处理"); + } + String decision = act == ToolGuardButtonValue.Action.APPROVE ? "approved" : "denied"; + try { + ResolveOutcome outcome = approvalWorkflowService.resolve(pendingId, clickerOpenId, decision); + if (!outcome.dbSynced()) { + // already resolved / superseded — not an error, but tell the clicker. + log.info("[feishu-toolguard] wf- {} already resolved: {}", pendingId, outcome.decision()); + return buildExpiredResponse(toolName); + } + log.info("[feishu-toolguard] Resolved wf- {} as {} by {} (run resume delegated to bridge)", + pendingId, decision, abbrev(clickerOpenId)); + return buildResolvedResponse(toolName, act, clickerOpenId); + } catch (Exception e) { + log.error("[feishu-toolguard] Failed to resolve wf- {}: {}", pendingId, e.getMessage(), e); + return buildErrorResponse("⚠️ 工作流审批未生效，请重试或在管理端处理"); + } + } + // ------------------------------------------------------------------ // Response builders — assemble P2CardActionTriggerResponse{toast,card} // ------------------------------------------------------------------ diff --git a/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardKindFactory.java b/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardKindFactory.java index 757cc06e..435aab0c 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardKindFactory.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/feishu/cards/tool_guard/ToolGuardCardKindFactory.java @@ -3,6 +3,7 @@ package vip.mate.channel.feishu.cards.tool_guard; import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.stereotype.Component; import vip.mate.approval.ApprovalService; +import vip.mate.approval.ApprovalWorkflowService; import vip.mate.channel.feishu.cards.FeishuCardKind; /** @@ -27,21 +28,27 @@ public class ToolGuardCardKindFactory { public static final String ACTION_PREFIX = ToolGuardButtonValue.ACTION_PREFIX; private final ApprovalService approvalService; + /** ISSUE #413 P2-B3: resolves workflow-scoped (wf-) approvals from card clicks. */ + private final ApprovalWorkflowService approvalWorkflowService; private final ObjectMapper objectMapper; public ToolGuardCardKindFactory(ApprovalService approvalService, + ApprovalWorkflowService approvalWorkflowService, ObjectMapper objectMapper) { this.approvalService = approvalService; + this.approvalWorkflowService = approvalWorkflowService; this.objectMapper = objectMapper; } public FeishuCardKind create() { ToolGuardButtonValue buttonValue = new ToolGuardButtonValue(objectMapper); ToolGuardCardRenderer renderer = new ToolGuardCardRenderer(buttonValue); - // Handler no longer needs ApprovalWorkflowService — the canonical - // resolve + replay path runs via a synthetic /approve|/deny - // message injected back into the router. - ToolGuardCardHandler handler = new ToolGuardCardHandler(approvalService, buttonValue); + // ISSUE #413 P2-B3: handler needs ApprovalWorkflowService to resolve + // wf- workflow approvals inline (the synthetic /approve injection is + // a dead end for wf- ids). Regular tool approvals still go through + // the synthetic /approve | /deny router path as before. + ToolGuardCardHandler handler = new ToolGuardCardHandler( + approvalService, approvalWorkflowService, buttonValue); return new FeishuCardKind(KIND_NAME, ACTION_PREFIX, renderer, handler); } } diff --git a/mateclaw-server/src/main/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardHandler.java b/mateclaw-server/src/main/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardHandler.java index c73230b3..ac28885d 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardHandler.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardHandler.java @@ -2,7 +2,9 @@ package vip.mate.channel.wecom.cards.tool_guard; import lombok.extern.slf4j.Slf4j; import vip.mate.approval.ApprovalService; +import vip.mate.approval.ApprovalWorkflowService; import vip.mate.approval.PendingApproval; +import vip.mate.approval.ResolveOutcome; import vip.mate.channel.ChannelMessage; import vip.mate.channel.wecom.WeComChannelAdapter; import vip.mate.channel.wecom.cards.WeComCardHandler; @@ -37,10 +39,20 @@ import java.util.Optional; public class ToolGuardCardHandler implements WeComCardHandler { private final ApprovalService approvalService; + /** + * ISSUE #413 P2-B3: resolves workflow-scoped ({@code wf-}) approvals + * inline — the synthetic /approve injection is a dead end for wf- ids + * (their conversationId is {@code workflow:run:{runId}}, unmatched by + * any IM conversation). May be null in narrow test contexts. + */ + private final ApprovalWorkflowService approvalWorkflowService; private final ToolGuardButtonKey buttonKey; - public ToolGuardCardHandler(ApprovalService approvalService, ToolGuardButtonKey buttonKey) { + public ToolGuardCardHandler(ApprovalService approvalService, + ApprovalWorkflowService approvalWorkflowService, + ToolGuardButtonKey buttonKey) { this.approvalService = approvalService; + this.approvalWorkflowService = approvalWorkflowService; this.buttonKey = buttonKey; } @@ -76,6 +88,19 @@ public class ToolGuardCardHandler implements WeComCardHandler { PendingApproval pending = opt.get(); // ---- 3. Identity check (fail-closed) ---- + // Workflow-scoped approvals (wf- prefix, ISSUE #413 P2-B3) have no + // human requester (userId is null — the run is system-initiated). + // Their cards only reach the channels declared in await_approval's + // approverChannels, so any audience member is a legitimate approver. + // We resolve inline (no synthetic injection: the router path can't + // route a workflow:run:{runId} conversationId) and the + // ApprovalResumeBridge resumes the run off the resolved event. + if (pendingId.startsWith("wf-")) { + handleWorkflowApproval(adapter, eventReqId, taskId, pendingId, + decoded.toolName(), action, clickerUserId); + return; + } + // Agent/cron ("system") or unattributed (null) approvals have no human // requester to match the clicker against; a group card would let any // member resolve a guarded action. Reject here (mirrors the feishu card @@ -117,6 +142,48 @@ public class ToolGuardCardHandler implements WeComCardHandler { } } + // ------------------------------------------------------------------ + // Workflow-scoped approval (ISSUE #413 P2-B3) + // ------------------------------------------------------------------ + + /** + * Resolve a {@code wf-} workflow approval inline, then render the + * resolved card — both within the WeCom 5s callback window. The + * synthetic /approve injection is bypassed because the router cannot + * route a {@code workflow:run:{runId}} conversationId. The + * {@link vip.mate.workflow.runtime.ApprovalResumeBridge} picks up the + * {@code WorkflowApprovalResolvedEvent} published inside resolve and + * resumes the paused run asynchronously. + * + *

Identity: any audience member may resolve — the card only reaches + * the channels declared in {@code await_approval.approverChannels}. + */ + private void handleWorkflowApproval(WeComChannelAdapter adapter, String eventReqId, String taskId, + String pendingId, String toolName, + ToolGuardButtonKey.Action action, String clickerUserId) { + if (approvalWorkflowService == null) { + log.warn("[wecom-toolguard] ApprovalWorkflowService unavailable, cannot resolve wf- {} " + + "(use the admin console)", pendingId); + renderExpired(adapter, eventReqId, taskId, toolName); + return; + } + String decision = action == ToolGuardButtonKey.Action.APPROVE ? "approved" : "denied"; + try { + ResolveOutcome outcome = approvalWorkflowService.resolve(pendingId, clickerUserId, decision); + if (!outcome.dbSynced()) { + log.info("[wecom-toolguard] wf- {} already resolved: {}", pendingId, outcome.decision()); + renderExpired(adapter, eventReqId, taskId, toolName); + return; + } + log.info("[wecom-toolguard] Resolved wf- {} as {} by {} (run resume delegated to bridge)", + pendingId, decision, abbrev(clickerUserId)); + renderResolved(adapter, eventReqId, taskId, toolName, action, clickerUserId); + } catch (Exception e) { + log.error("[wecom-toolguard] Failed to resolve wf- {}: {}", pendingId, e.getMessage(), e); + renderExpired(adapter, eventReqId, taskId, toolName); + } + } + // ------------------------------------------------------------------ // Card rendering helpers // ------------------------------------------------------------------ diff --git a/mateclaw-server/src/main/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardKindFactory.java b/mateclaw-server/src/main/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardKindFactory.java index e36e5422..d37e7a49 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardKindFactory.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardKindFactory.java @@ -3,6 +3,7 @@ package vip.mate.channel.wecom.cards.tool_guard; import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.stereotype.Component; import vip.mate.approval.ApprovalService; +import vip.mate.approval.ApprovalWorkflowService; import vip.mate.channel.wecom.cards.WeComCardKind; /** @@ -34,17 +35,23 @@ public class ToolGuardCardKindFactory { public static final String MESSAGE_TYPE = "tool_guard_approval"; private final ApprovalService approvalService; + /** ISSUE #413 P2-B3: resolves workflow-scoped (wf-) approvals from card clicks. */ + private final ApprovalWorkflowService approvalWorkflowService; private final ObjectMapper objectMapper; - public ToolGuardCardKindFactory(ApprovalService approvalService, ObjectMapper objectMapper) { + public ToolGuardCardKindFactory(ApprovalService approvalService, + ApprovalWorkflowService approvalWorkflowService, + ObjectMapper objectMapper) { this.approvalService = approvalService; + this.approvalWorkflowService = approvalWorkflowService; this.objectMapper = objectMapper; } public WeComCardKind create() { ToolGuardButtonKey buttonKey = new ToolGuardButtonKey(objectMapper); ToolGuardCardRenderer renderer = new ToolGuardCardRenderer(buttonKey); - ToolGuardCardHandler handler = new ToolGuardCardHandler(approvalService, buttonKey); + ToolGuardCardHandler handler = new ToolGuardCardHandler( + approvalService, approvalWorkflowService, buttonKey); return new WeComCardKind( "tool_guard_approval", MESSAGE_TYPE, diff --git a/mateclaw-server/src/test/java/vip/mate/channel/feishu/cards/FeishuCardDispatcherTest.java b/mateclaw-server/src/test/java/vip/mate/channel/feishu/cards/FeishuCardDispatcherTest.java index c9c18b20..0d48766a 100644 --- a/mateclaw-server/src/test/java/vip/mate/channel/feishu/cards/FeishuCardDispatcherTest.java +++ b/mateclaw-server/src/test/java/vip/mate/channel/feishu/cards/FeishuCardDispatcherTest.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import vip.mate.approval.ApprovalService; +import vip.mate.approval.ApprovalWorkflowService; import vip.mate.channel.feishu.cards.tool_guard.ToolGuardButtonValue; import vip.mate.channel.feishu.cards.tool_guard.ToolGuardCardKindFactory; @@ -23,6 +24,7 @@ class FeishuCardDispatcherTest { private FeishuCardDispatcher newDispatcher() { ToolGuardCardKindFactory factory = new ToolGuardCardKindFactory( mock(ApprovalService.class), + mock(ApprovalWorkflowService.class), new ObjectMapper()); return new FeishuCardDispatcher(factory); } diff --git a/mateclaw-server/src/test/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardHandlerTest.java b/mateclaw-server/src/test/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardHandlerTest.java index 77c8b6eb..c3c8c10f 100644 --- a/mateclaw-server/src/test/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardHandlerTest.java +++ b/mateclaw-server/src/test/java/vip/mate/channel/wecom/cards/tool_guard/ToolGuardCardHandlerTest.java @@ -7,7 +7,9 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import vip.mate.approval.ApprovalService; +import vip.mate.approval.ApprovalWorkflowService; import vip.mate.approval.PendingApproval; +import vip.mate.approval.ResolveOutcome; import vip.mate.channel.ChannelMessage; import vip.mate.channel.wecom.WeComChannelAdapter; @@ -32,6 +34,7 @@ import static org.mockito.Mockito.*; class ToolGuardCardHandlerTest { private ApprovalService approvalService; + private ApprovalWorkflowService approvalWorkflowService; private WeComChannelAdapter adapter; private ToolGuardButtonKey buttonKey; private ToolGuardCardHandler handler; @@ -39,9 +42,10 @@ class ToolGuardCardHandlerTest { @BeforeEach void setUp() { approvalService = Mockito.mock(ApprovalService.class); + approvalWorkflowService = Mockito.mock(ApprovalWorkflowService.class); adapter = Mockito.mock(WeComChannelAdapter.class); buttonKey = new ToolGuardButtonKey(new ObjectMapper()); - handler = new ToolGuardCardHandler(approvalService, buttonKey); + handler = new ToolGuardCardHandler(approvalService, approvalWorkflowService, buttonKey); } @Test @@ -156,6 +160,68 @@ class ToolGuardCardHandlerTest { verify(adapter, never()).injectSyntheticMessage(any(ChannelMessage.class)); } + // ---- workflow-scoped (wf-) approval branch (ISSUE #413 P2-B3) ---- + + @Test + @DisplayName("wf- approval: card click resolves inline (no synthetic /approve injection)") + void workflowApprovalResolvesInline() { + // A workflow-scoped approval has userId=null (system-initiated) and a + // wf- pendingId. Before P2-B3 the identity check (requester==clicker) + // rejected every click — wf- approvals could only be resolved from the + // admin console. Now any audience member may resolve, and the handler + // calls resolve() directly (the synthetic injection is a dead end for + // wf- ids since their conversationId is workflow:run:{runId}). + PendingApproval wfPending = new PendingApproval( + "wf-abc123def456", "workflow:run:42", null, + "workflow:manager", "{}", "await manager approval"); + when(approvalService.getPending("wf-abc123def456")).thenReturn(Optional.of(wfPending)); + when(approvalWorkflowService.resolve("wf-abc123def456", "carol", "approved")) + .thenReturn(new ResolveOutcome( + "wf-abc123def456", "workflow:run:42", "workflow:manager", + "approved", null, true, 0)); + + Map frame = inboundFrame("evt_req_wf1", buttonKey.encode( + ToolGuardButtonKey.Action.APPROVE, "wf-abc123def456", "workflow:manager", "MEDIUM")); + handler.handle(adapter, frame, tce(frame), fromBlock("carol")); + + // resolve() was called inline — ApprovalResumeBridge resumes the run. + verify(approvalWorkflowService, times(1)).resolve("wf-abc123def456", "carol", "approved"); + // The synthetic /approve injection is NOT used for wf- approvals. + verify(adapter, never()).injectSyntheticMessage(any(ChannelMessage.class)); + // A resolved card was rendered. + verify(adapter, times(1)).updateTemplateCard(eq("evt_req_wf1"), any()); + } + + @Test + @DisplayName("wf- approval already resolved: renders 'expired' card, no resolve call") + void workflowApprovalAlreadyResolved() { + PendingApproval wfPending = new PendingApproval( + "wf-alreadydone", "workflow:run:43", null, + "workflow:manager", "{}", "await manager approval"); + when(approvalService.getPending("wf-alreadydone")).thenReturn(Optional.of(wfPending)); + // dbSynced=false means the row was already terminal (approved/denied + // via another path). The handler renders 'expired' and does not treat + // it as an error. + when(approvalWorkflowService.resolve(eq("wf-alreadydone"), anyString(), anyString())) + .thenReturn(new ResolveOutcome( + "wf-alreadydone", null, null, + "already_resolved", null, false, 0)); + + Map frame = inboundFrame("evt_req_wf2", buttonKey.encode( + ToolGuardButtonKey.Action.DENY, "wf-alreadydone", "workflow:manager", "LOW")); + handler.handle(adapter, frame, tce(frame), fromBlock("dave")); + + verify(approvalWorkflowService, times(1)).resolve(eq("wf-alreadydone"), eq("dave"), eq("denied")); + verify(adapter, never()).injectSyntheticMessage(any(ChannelMessage.class)); + // 'expired' card was rendered (title mentions 过期). + ArgumentCaptor> cardCaptor = cardArgCaptor(); + verify(adapter, times(1)).updateTemplateCard(eq("evt_req_wf2"), cardCaptor.capture()); + @SuppressWarnings("unchecked") + Map mainTitle = (Map) cardCaptor.getValue().get("main_title"); + assertTrue(((String) mainTitle.get("title")).contains("过期"), + "already-resolved wf- should show expired card; got: " + mainTitle.get("title")); + } + // ---- helpers ---- private static PendingApproval pendingFor(String pendingId, String requester, String tool) {