feat(im): resolve workflow approvals via feishu/wecom card clicks (ISSUE #413 P2-B3) (#416)

Before this, a workflow await_approval step whose approverChannels
pointed at feishu/wecom was effectively dead for IM interaction. Even
after PR #414 (B1) pushed the notice to the IM group, clicking the
card's Approve/Deny buttons did nothing useful:

- Identity check (requester==clicker) failed-closed: wf- approvals
  have userId=null (system-initiated), so every click was rejected.
- Even if it passed, the synthetic /approve injection was a dead end:
  the router routes by conversationId, but wf- ids use a synthetic
  workflow:run:{runId} key that no IM conversation matches, so
  findPendingByConversation returned null and the /approve was fed
  to the LLM as plain text.

B3 fix: both ToolGuardCardHandlers now detect the wf- prefix and
resolve inline (approvalService.resolve), bypassing the synthetic
injection entirely. The WorkflowApprovalResolvedEvent published
inside resolve is picked up by ApprovalResumeBridge (activated in
PR #414 B2), which resumes the paused run. This mirrors the Web /
WebChat resolve path (PR #415).

Identity policy: any audience member may resolve a wf- approval.
The card only reaches channels declared in await_approval's
approverChannels, so whoever sees it is a designated approver.
Regular tool approvals keep the strict requester==clicker guard.

Tests:
- wecom ToolGuardCardHandlerTest: +2 wf- cases (inline resolve, no
  synthetic injection; already-resolved renders expired). Existing 6
  cases updated for the new 3-arg constructor.
- feishu FeishuCardDispatcherTest: updated for the new factory
  constructor signature.

Regression: ApprovalWorkflowServiceResolveTest (13), GcTest (7),
RecoveryTest (7), feishu dispatcher (4), button value (7),
renderer (3+3) — all green.
This commit is contained in:
倪程伟 2026-06-25 09:56:36 +08:00 committed by GitHub
parent 20014c72ff
commit b478eef78c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 232 additions and 8 deletions

View File

@ -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.
*
* <p>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.
*
* <p>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}
// ------------------------------------------------------------------

View File

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

View File

@ -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.
*
* <p>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
// ------------------------------------------------------------------

View File

@ -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,

View File

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

View File

@ -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<String, Object> 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<String, Object> 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<Map<String, Object>> cardCaptor = cardArgCaptor();
verify(adapter, times(1)).updateTemplateCard(eq("evt_req_wf2"), cardCaptor.capture());
@SuppressWarnings("unchecked")
Map<String, Object> mainTitle = (Map<String, Object>) 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) {