From e64752a8308ea787bcdf0ff5ed375eadccff19a8 Mon Sep 17 00:00:00 2001 From: matevip Date: Mon, 27 Apr 2026 22:25:27 +0800 Subject: [PATCH] fix(approval): unify tool-approval state machine across DB / message metadata / memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Reconcile approval status atomically: DB row, message metadata, in-memory store - Approve and deny both flip the tool-call card + timeline segment to a terminal state on the gate message — no more orange spinner stuck after a decision - Frontend hydrate matches by pendingId and reverse-converges to expired so a refresh after server-side timeout / consume clears the banner without restart - Stop sweep, GC timeout, and JVM restart all close the loop with consistent state - Remove the dead REST /approve endpoint + matching frontend client export so there is only one resolve path to maintain --- .gitignore | 3 + .../vip/mate/approval/ApprovalController.java | 87 ++----------- .../vip/mate/channel/web/ChatController.java | 105 +++++++++++---- .../conversation/ConversationService.java | 122 +++++++++++++++++- mateclaw-ui/src/api/index.ts | 2 - mateclaw-ui/src/composables/chat/useChat.ts | 48 ++++++- mateclaw-ui/src/types/index.ts | 9 +- mateclaw-ui/src/views/ChatConsole.vue | 89 ++++++++++--- 8 files changed, 337 insertions(+), 128 deletions(-) diff --git a/.gitignore b/.gitignore index 656cfe90..6c24bfe0 100644 --- a/.gitignore +++ b/.gitignore @@ -95,3 +95,6 @@ deploy/.env CLAUDE.md .claude/settings.local.json .claude/plans/ + +# Codex CLI local artifacts +.codex/ diff --git a/mateclaw-server/src/main/java/vip/mate/approval/ApprovalController.java b/mateclaw-server/src/main/java/vip/mate/approval/ApprovalController.java index 9856b189..30290509 100644 --- a/mateclaw-server/src/main/java/vip/mate/approval/ApprovalController.java +++ b/mateclaw-server/src/main/java/vip/mate/approval/ApprovalController.java @@ -2,12 +2,10 @@ package vip.mate.approval; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.tags.Tag; -import lombok.Data; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.security.core.Authentication; import org.springframework.web.bind.annotation.*; -import vip.mate.channel.web.ChatStreamTracker; import vip.mate.common.result.R; import vip.mate.workspace.conversation.ConversationService; @@ -15,10 +13,16 @@ import java.util.List; import java.util.Map; /** - * 工具执行审批接口 + * Approval read-only endpoints. *

- * 提供 approve / deny 端点，供前端在收到 tool_approval_requested SSE 事件后调用。 - * 批准后自动触发工具重放，结果通过 SSE 流推送给前端。 + * Web approve / deny actions ride the SSE {@code POST /chat/stream} path with + * {@code /approve} or {@code /deny} text commands ({@link vip.mate.channel.web.ChatController} + * intercepts), so a write-style {@code POST /approve} REST endpoint was deleted + * in RFC-067 PR 6 — it bypassed the unified workflow lifecycle and let any + * future caller silently regress to the pre-RFC ghost-approval state. + *

+ * Only {@link #getPendingApprovals} remains, used by the frontend for hydration + * after page refresh. * * @author MateClaw Team */ @@ -31,71 +35,11 @@ public class ApprovalController { private final ApprovalWorkflowService approvalService; private final ConversationService conversationService; - private final ChatStreamTracker streamTracker; /** - * 批准或拒绝工具执行 - *

- * 批准后自动触发工具重放（异步执行），结果通过已有的 SSE 连接推送给前端。 - */ - @Operation(summary = "审批工具执行") - @PostMapping("/{conversationId}/approve") - public R approve( - @PathVariable String conversationId, - @RequestBody ApprovalRequest request, - Authentication auth) { - - if (auth == null) { - return R.fail(401, "未登录，请先登录"); - } - String username = auth.getName(); - - // 校验会话归属 - if (!conversationService.isConversationOwner(conversationId, username)) { - log.warn("[Approval] Unauthorized: user={} is not owner of conversation={}", username, conversationId); - return R.fail(403, "无权操作该会话"); - } - - // 校验 pendingId - if (request.getPendingId() == null || request.getPendingId().isBlank()) { - return R.fail("pendingId 不能为空"); - } - - // 校验 decision - String decision = request.getDecision(); - if (decision == null || (!decision.equalsIgnoreCase("approved") && !decision.equalsIgnoreCase("denied"))) { - return R.fail("decision 必须为 approved 或 denied"); - } - - try { - // workflow.resolve owns DB + metadata + memory atomically (RFC-067 §4.2). - ResolveOutcome outcome = approvalService.resolve(request.getPendingId(), username, decision); - log.info("[Approval] User {} {} pending {} for conversation {} (dbSynced={}, msgRewritten={})", - username, decision, request.getPendingId(), conversationId, - outcome.dbSynced(), outcome.messagesRewritten()); - - // Web replay flows through POST /stream's /approve text-command path - // (ChatController intercepts). This endpoint only flips state; the SSE - // notify below covers the deny case where the stream is still alive. - if ("denied".equalsIgnoreCase(decision) && streamTracker.isRunning(conversationId)) { - streamTracker.broadcastObject(conversationId, "tool_approval_resolved", Map.of( - "pendingId", request.getPendingId(), - "decision", "denied", - "timestamp", System.currentTimeMillis() - )); - } - - return R.ok("操作成功"); - } catch (IllegalArgumentException e) { - log.warn("[Approval] Resolve failed: {}", e.getMessage()); - return R.fail(e.getMessage()); - } - } - - /** - * 查询指定会话下的待审批记录 - *

- * 用于页面刷新后恢复审批卡片（hydration）。 + * Hydration query for page refresh: returns every pending approval still + * waiting in the conversation. The frontend uses this to rebuild the + * approval banner after a reload. */ @Operation(summary = "查询待审批记录") @GetMapping("/{conversationId}/pending-approvals") @@ -115,11 +59,4 @@ public class ApprovalController { List> pending = approvalService.getPendingByConversation(conversationId); return R.ok(pending); } - - @Data - public static class ApprovalRequest { - private String pendingId; - /** "approved" 或 "denied" */ - private String decision; - } } diff --git a/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java b/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java index 30e92af0..53cca423 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java @@ -295,27 +295,38 @@ public class ChatController { }) .doOnComplete(() -> { if (!finalized.compareAndSet(false, true)) return; + // RFC-067 §4.6: replay can re-trigger an approval (the approved tool + // call may chain into another guarded tool). Derive status the same + // way as the normal stream so awaiting_approval doesn't get masked + // as completed. + boolean replayWasStopped = streamTracker.isStopRequested(conversationId); + ChatStreamTracker.InterruptType replayInterrupt = streamTracker.getInterruptType(conversationId); + boolean replayIsError = accumulator.getContent() != null + && accumulator.getContent().startsWith("[错误] "); + String persistStatus = derivePersistStatus( + accumulator.isAwaitingApproval(), replayIsError, + replayWasStopped, replayInterrupt); try { MessageEntity savedAssistant = null; List parts = accumulator.toAssistantParts(); String text = accumulator.getContent(); if (!text.isBlank() || !parts.isEmpty()) { savedAssistant = conversationService.saveMessage(conversationId, "assistant", text, parts, - "completed", + persistStatus, accumulator.getPromptTokens(), accumulator.getCompletionTokens(), accumulator.getRuntimeModelName(), accumulator.getRuntimeProviderId(), - accumulator.toMetadataJson()); // 包含 toolCalls 元数据 + accumulator.toMetadataJson()); // includes toolCalls metadata } broadcastEvent(conversationId, "message_complete", Map.of( - "status", "completed", + "status", persistStatus, "hasThinking", !accumulator.getThinking().isBlank(), "hasContent", !text.isBlank() )); int msgCount = conversationService.getMessageCount(conversationId); broadcastEvent(conversationId, "done", buildDonePayload( - conversationId, "completed", savedAssistant, 0, 0, true, msgCount)); + conversationId, persistStatus, savedAssistant, 0, 0, true, msgCount)); } catch (Exception e) { log.warn("SSE replay complete error: {}", e.getMessage()); } finally { @@ -477,16 +488,8 @@ public class ChatController { boolean isInterruptFollowup = interruptType == ChatStreamTracker.InterruptType.USER_INTERRUPT_WITH_FOLLOWUP; boolean isError = accumulator.getContent() != null && accumulator.getContent().startsWith("[错误] "); - String persistStatus; - if (accumulator.isAwaitingApproval()) { - persistStatus = "awaiting_approval"; - } else if (isError) { - persistStatus = "error"; - } else if (!wasStopped) { - persistStatus = "completed"; - } else { - persistStatus = isInterruptFollowup ? "interrupted" : "stopped"; - } + String persistStatus = derivePersistStatus( + accumulator.isAwaitingApproval(), isError, wasStopped, interruptType); try { MessageEntity savedAssistant = null; List assistantParts = accumulator.toAssistantParts(); @@ -1069,13 +1072,24 @@ public class ChatController { }) .doOnComplete(() -> { if (!finalized.compareAndSet(false, true)) return; + // RFC-067 §4.6: queued stream can hit a tool_approval_requested event + // mid-flight. Derive status via the shared helper so awaiting_approval + // is not silently downgraded to completed (which would prematurely fire + // expirePendingApprovals on the frontend and ghost-clear the banner). + boolean queuedWasStopped = streamTracker.isStopRequested(conversationId); + ChatStreamTracker.InterruptType queuedInterrupt = streamTracker.getInterruptType(conversationId); + boolean queuedIsError = accumulator.getContent() != null + && accumulator.getContent().startsWith("[错误] "); + String persistStatus = derivePersistStatus( + accumulator.isAwaitingApproval(), queuedIsError, + queuedWasStopped, queuedInterrupt); try { MessageEntity savedAssistant = null; List parts = accumulator.toAssistantParts(); String text = accumulator.getContent(); if (!text.isBlank() || !parts.isEmpty()) { savedAssistant = conversationService.saveMessage(conversationId, "assistant", text, parts, - "completed", + persistStatus, accumulator.getPromptTokens(), accumulator.getCompletionTokens(), accumulator.getRuntimeModelName(), @@ -1083,12 +1097,12 @@ public class ChatController { accumulator.toMetadataJson()); } broadcastEvent(conversationId, "message_complete", Map.of( - "status", "completed", + "status", persistStatus, "hasThinking", !accumulator.getThinking().isBlank(), "hasContent", !text.isBlank() )); broadcastEvent(conversationId, "done", buildDonePayload( - conversationId, "completed", savedAssistant, + conversationId, persistStatus, savedAssistant, accumulator.getPromptTokens(), accumulator.getCompletionTokens(), true, conversationService.getMessageCount(conversationId))); } catch (Exception e) { @@ -1173,6 +1187,35 @@ public class ChatController { streamTracker.broadcast(conversationId, name, payload); } + /** + * Derive the persistence status for an assistant message at stream finalization + * (RFC-067 §4.6). Five-way state machine: + *

+ * Package-private so {@link vip.mate.channel.web.ChatControllerPersistStatusTest} + * can exercise the truth table directly without spinning up the controller. + */ + static String derivePersistStatus(boolean isAwaitingApproval, + boolean isError, + boolean wasStopped, + ChatStreamTracker.InterruptType interruptType) { + if (isAwaitingApproval) return "awaiting_approval"; + if (isError) return "error"; + if (!wasStopped) return "completed"; + return interruptType == ChatStreamTracker.InterruptType.USER_INTERRUPT_WITH_FOLLOWUP + ? "interrupted" : "stopped"; + } + private Map buildDonePayload(String conversationId, String status, MessageEntity savedAssistant, int promptTokens, int completionTokens, boolean persisted, Integer messageCount) { @@ -1204,10 +1247,20 @@ public class ChatController { * Invoked from {@link ChatStreamTracker#onShutdown()} so any in-flight turn doesn't * lose its already-streamed content + tool calls when the process exits. *

+ * Status routing (RFC-067 §4.6): + *

* Idempotent w.r.t. the normal doOnComplete/doOnError save: if those paths already - * persisted the message, this writes a second row with status="interrupted_shutdown", - * which is rare in practice (race window is sub-second between dispose and save) and - * acceptable. Skipping save when nothing to save avoids empty rows. + * persisted the message, this writes a second row, which is rare in practice + * (race window is sub-second between dispose and save) and acceptable. Skipping + * save when nothing to save avoids empty rows. */ private void emergencySaveAccumulator(String conversationId, StreamAccumulator accumulator) { try { @@ -1216,17 +1269,21 @@ public class ChatController { if (text.isBlank() && parts.isEmpty()) { return; } - String savedText = text.isBlank() ? "[已中断 — 服务重启]" : text; + boolean awaitingApproval = accumulator.isAwaitingApproval(); + String status = awaitingApproval ? "awaiting_approval" : "interrupted_shutdown"; + String savedText = text.isBlank() + ? (awaitingApproval ? "[等待审批 — 服务重启]" : "[已中断 — 服务重启]") + : text; conversationService.saveMessage(conversationId, "assistant", savedText, parts, - "interrupted_shutdown", + status, accumulator.getPromptTokens(), accumulator.getCompletionTokens(), accumulator.getRuntimeModelName(), accumulator.getRuntimeProviderId(), accumulator.toMetadataJson()); log.info("[ChatController] Emergency-saved in-flight assistant message: " + - "conversationId={}, textLen={}, partsCount={}", - conversationId, text.length(), parts.size()); + "conversationId={}, status={}, textLen={}, partsCount={}", + conversationId, status, text.length(), parts.size()); } catch (Exception e) { log.error("[ChatController] Emergency save failed for {}: {}", conversationId, e.getMessage(), e); diff --git a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java index fb5a3dc3..5eb4a5eb 100644 --- a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java +++ b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/ConversationService.java @@ -561,7 +561,19 @@ public class ConversationService { if (raw == null || raw.isBlank() || !raw.contains("pendingApproval")) continue; try { - java.util.Map meta = objectMapper.readValue(raw, new TypeReference<>() {}); + // H2's JSON column returns the metadata as a JSON-encoded string + // (wrapped + escaped) when read through MyBatis. MessageVO.parseMetadataToObject + // (the read-to-frontend path) already handles this; we mirror the same + // unwrap here. Without it, readValue tokenizes the leading `"` as a + // String token and explodes with "Cannot construct LinkedHashMap from + // String value", silently turning every approve / deny / Stop sweep + // into a no-op (messagesRewritten=0). + String json = raw.trim(); + if (json.startsWith("\"") && json.endsWith("\"")) { + json = objectMapper.readValue(json, String.class); + } + java.util.Map meta = objectMapper.readValue(json, + new TypeReference>() {}); Object pa = meta.get("pendingApproval"); if (!(pa instanceof java.util.Map)) continue; @SuppressWarnings("unchecked") @@ -579,6 +591,23 @@ public class ConversationService { meta.put("currentPhase", "resolved"); } + // RFC-067 §4.10 (PR 9): flip the matching toolCall + segment entries + // inside this message's metadata. Both DENIED and APPROVED need this + // because the LLM streamed tool_call_started → segment.status='running' + // before the user's decision arrived, and replay creates a NEW assistant + // message rather than updating the original — so without this fix the + // gate message's tool card stays as an orange spinner forever. + // DENIED → success=false + result='[已拒绝]' → red ✗ + // APPROVED → success=true + result='[已批准]' → green ✓ on the gate + // row; the actual execution result still appears in the + // replayed assistant message that follows. + Object toolName = pendingApproval.get("toolName"); + Object toolArgs = pendingApproval.get("arguments"); + String tnStr = toolName == null ? null : String.valueOf(toolName); + String taStr = toolArgs == null ? null : String.valueOf(toolArgs); + flipResolvedToolCalls(meta, tnStr, taStr, decision); + flipResolvedSegments(meta, tnStr, taStr, decision); + msg.setMetadata(objectMapper.writeValueAsString(meta)); if ("awaiting_approval".equals(msg.getStatus())) { msg.setStatus(decision.messageStatus); @@ -586,8 +615,10 @@ public class ConversationService { messageMapper.updateById(msg); rewritten++; } catch (Exception e) { - log.warn("[ConversationService] Failed to rewrite pendingApproval status for message {}: {}", - msg.getId(), e.getMessage()); + String preview = raw.length() > 200 ? raw.substring(0, 200) + "..." : raw; + log.warn("[ConversationService] Failed to rewrite pendingApproval status for message {} " + + "(rawLen={}, preview={}): {}", + msg.getId(), raw.length(), preview, e.getMessage()); } } if (rewritten > 0) { @@ -598,6 +629,91 @@ public class ConversationService { return rewritten; } + /** + * Flip the gate message's tool-call entry to a terminal state matching the + * approval decision (RFC-067 §4.10). + *

+ * Driven by {@link MetadataDecision}: + *

    + *
  • {@link MetadataDecision#APPROVED} → {@code status='completed'} + + * {@code success=true} + {@code result='[已批准]'}. The actual tool + * execution result appears in the replayed assistant message that + * follows — not on this gate row.
    • + *
  • {@link MetadataDecision#DENIED} → {@code status='completed'} + + * {@code success=false} + {@code result='[已拒绝]'}. MessageBubble + * renders this as a red ✗.
    • + *
+ * Both paths flip status off {@code awaiting_approval} / {@code running} so + * MessageBubble's icon precedence (running > awaiting_approval > success + * branches) can reach the right terminal icon. Without the flip the card + * stays as an orange spinner forever — replay creates a new message + * instead of overwriting the gate row, so nothing else updates it. + * Best-effort: if metadata.toolCalls is missing or no entry matches, this + * is a silent no-op. + */ + @SuppressWarnings("unchecked") + private void flipResolvedToolCalls(java.util.Map meta, + String toolName, String toolArgs, + MetadataDecision decision) { + Object tc = meta.get("toolCalls"); + if (!(tc instanceof java.util.List)) return; + boolean approved = decision == MetadataDecision.APPROVED; + String resultText = approved ? "[已批准]" : "[已拒绝]"; + for (Object entry : (java.util.List) tc) { + if (!(entry instanceof java.util.Map)) continue; + java.util.Map call = (java.util.Map) entry; + if (!matchesNameAndArgs(call.get("name"), call.get("arguments"), toolName, toolArgs)) continue; + Object status = call.get("status"); + if ("awaiting_approval".equals(String.valueOf(status)) + || "running".equals(String.valueOf(status))) { + call.put("status", "completed"); + } + call.put("success", approved ? Boolean.TRUE : Boolean.FALSE); + call.put("result", resultText); + } + } + + /** + * Same terminal-state flip as {@link #flipResolvedToolCalls} but on the + * streaming-segments timeline. Segments use {@code toolName} / {@code toolArgs} + * + {@code toolSuccess} / {@code toolResult} field names (not + * {@code name} / {@code arguments} / {@code success} / {@code result}); the + * shape is otherwise symmetric. + */ + @SuppressWarnings("unchecked") + private void flipResolvedSegments(java.util.Map meta, + String toolName, String toolArgs, + MetadataDecision decision) { + Object segs = meta.get("segments"); + if (!(segs instanceof java.util.List)) return; + boolean approved = decision == MetadataDecision.APPROVED; + String resultText = approved ? "[已批准]" : "[已拒绝]"; + for (Object entry : (java.util.List) segs) { + if (!(entry instanceof java.util.Map)) continue; + java.util.Map seg = (java.util.Map) entry; + if (!"tool_call".equals(String.valueOf(seg.get("type")))) continue; + if (!matchesNameAndArgs(seg.get("toolName"), seg.get("toolArgs"), toolName, toolArgs)) continue; + Object status = seg.get("status"); + if ("awaiting_approval".equals(String.valueOf(status)) + || "running".equals(String.valueOf(status))) { + seg.put("status", "completed"); + } + seg.put("toolSuccess", approved ? Boolean.TRUE : Boolean.FALSE); + seg.put("toolResult", resultText); + } + } + + private static boolean matchesNameAndArgs(Object actualName, Object actualArgs, + String expectedName, String expectedArgs) { + if (expectedName == null || actualName == null) return false; + if (!expectedName.equals(String.valueOf(actualName))) return false; + // Arguments equality: pendingApproval stores them as the JSON-stringified form + // produced by the tool-call creator, identical to what's recorded on the + // toolCall / segment entry. A null comparator on either side falls through. + if (expectedArgs == null) return true; + return expectedArgs.equals(String.valueOf(actualArgs)); + } + @Transactional public void removeApprovalPlaceholders(String conversationId) { List messages = listMessages(conversationId); diff --git a/mateclaw-ui/src/api/index.ts b/mateclaw-ui/src/api/index.ts index 25e52dd1..18bc31dd 100644 --- a/mateclaw-ui/src/api/index.ts +++ b/mateclaw-ui/src/api/index.ts @@ -123,8 +123,6 @@ export const chatApi = { }, stop: (conversationId: string) => http.post<{ stopped: boolean }>(`/chat/${conversationId}/stop`), - approve: (conversationId: string, data: { pendingId: string; decision: string }) => - http.post(`/chat/${conversationId}/approve`, data), getPendingApprovals: (conversationId: string) => http.get(`/chat/${conversationId}/pending-approvals`), } diff --git a/mateclaw-ui/src/composables/chat/useChat.ts b/mateclaw-ui/src/composables/chat/useChat.ts index f6bb9654..358ec752 100644 --- a/mateclaw-ui/src/composables/chat/useChat.ts +++ b/mateclaw-ui/src/composables/chat/useChat.ts @@ -936,10 +936,47 @@ export function useChat(options: UseChatOptions): UseChatReturn { if (msg) { const metadata = parseMetadata((msg as any).metadata) if (metadata?.pendingApproval) { - const toolCalls = [...(metadata?.toolCalls || [])] - for (let i = 0; i < toolCalls.length; i++) { - if (toolCalls[i].status !== 'completed') { - toolCalls[i] = { ...toolCalls[i], status: 'completed' } + // RFC-067 §4.10: every still-pending tool call on this gate message + // must surface as a terminal state — both deny AND approve. RFC-067 + // §3 guarantees "one turn at most one pending", so any running/ + // awaiting entry IS the resolved one — skip strict (name, arguments) + // matching since JSON formatting can drift between the live SSE + // buffer and pendingApproval.arguments. + // approve → success=true + result='[已批准]' → green ✓ on the gate + // row; the actual execution result is in the replayed + // assistant message that follows. + // deny → success=false + result='[已拒绝]' → red ✗. + // Both branches must also flip metadata.segments[] entries because + // MessageBubble renders the timeline via ToolCallSegment.vue (driven + // by metadata.segments, not metadata.toolCalls). + const approved = data.decision === 'approved' + const successFlag = approved + const resultText = approved ? '[已批准]' : '[已拒绝]' + const toolCalls = (metadata?.toolCalls || []).map((tc: any) => { + const wasPending = tc.status === 'awaiting_approval' || tc.status === 'running' + if (wasPending) { + return { ...tc, status: 'completed', success: successFlag, result: resultText } + } + return tc + }) + const segments = (metadata?.segments || []).map((seg: any) => { + if (seg.type !== 'tool_call') return seg + const wasPending = seg.status === 'running' || seg.status === 'awaiting_approval' + if (wasPending) { + return { ...seg, status: 'completed', toolSuccess: successFlag, toolResult: resultText } + } + return seg + }) + // Sync currentSegments.value so the live streaming buffer agrees + // with the persisted message metadata. + for (let i = 0; i < currentSegments.value.length; i++) { + const liveSeg: any = currentSegments.value[i] + if (liveSeg.type !== 'tool_call') continue + const wasPending = liveSeg.status === 'running' || liveSeg.status === 'awaiting_approval' + if (wasPending) { + liveSeg.status = 'completed' + liveSeg.toolSuccess = successFlag + liveSeg.toolResult = resultText } } updateMessage(targetId, { @@ -949,9 +986,10 @@ export function useChat(options: UseChatOptions): UseChatReturn { ...metadata, currentPhase: 'completed', toolCalls, + segments, pendingApproval: { ...metadata.pendingApproval, - status: data.decision === 'approved' ? 'approved' : 'denied' + status: approved ? 'approved' : 'denied' } } } as any) diff --git a/mateclaw-ui/src/types/index.ts b/mateclaw-ui/src/types/index.ts index d0696286..fd1a68dc 100644 --- a/mateclaw-ui/src/types/index.ts +++ b/mateclaw-ui/src/types/index.ts @@ -124,7 +124,14 @@ export interface PendingApprovalMeta { toolName: string arguments: string reason: string - status: 'pending_approval' | 'approved' | 'denied' + /** + * pending_approval / approved / denied are server-authoritative terminal states. + * 'expired' is a frontend-only local synthesis used by hydrate reverse-convergence + * (RFC-067 §4.9): when a message's metadata still says pending_approval but the + * server's getPendingApprovals no longer lists that pendingId, the UI flips to + * 'expired' so the banner clears without waiting for a fresh stream event. + */ + status: 'pending_approval' | 'approved' | 'denied' | 'expired' // 增强字段（Phase 6: 结构化风险信息） findings?: GuardFinding[] maxSeverity?: GuardSeverity diff --git a/mateclaw-ui/src/views/ChatConsole.vue b/mateclaw-ui/src/views/ChatConsole.vue index 57dcc705..2438133f 100644 --- a/mateclaw-ui/src/views/ChatConsole.vue +++ b/mateclaw-ui/src/views/ChatConsole.vue @@ -953,33 +953,86 @@ async function selectConversation(conv: Conversation) { messages.value = extractMessages(res).messages.map((msg: Message) => normalizeMessage(msg)) } - // Hydrate pending approvals：恢复刷新后丢失的审批卡片 + // Hydrate pending approvals：恢复刷新后丢失的审批卡片（RFC-067 §4.9） + // + // Two-way reconciliation between the server's pending list and each + // message's metadata.pendingApproval: + // 1. Forward — server pending → align onto the message that already + // carries the same pendingId (so multi-pending convs don't have + // every banner overwrite the same row); fallback to last assistant + // only when no message has that id yet. + // 2. Reverse — local message metadata still says pending_approval but + // the server no longer lists that pendingId → flip to 'expired' + // locally. This closes the GC/timeout loop without requiring an + // extra server-side broadcast: the next refresh sees a clean state. try { const approvalRes: any = await chatApi.getPendingApprovals(requestedConvId) if (currentConversationId.value !== requestedConvId) return - const pendingApprovals = approvalRes.data || [] - if (pendingApprovals.length > 0) { - const assistantMessages = messages.value.filter(m => m.role === 'assistant') - const lastAssistant = assistantMessages[assistantMessages.length - 1] - if (lastAssistant) { - for (const pa of pendingApprovals) { - (lastAssistant as any).metadata = { + const pendingApprovals: any[] = approvalRes.data || [] + + // Index existing messages by their embedded pendingId (assistant only). + const indexById = new Map() + for (const m of messages.value) { + if (m.role !== 'assistant') continue + const pid = (m as any).metadata?.pendingApproval?.pendingId + if (typeof pid === 'string' && pid) indexById.set(pid, m) + } + + // Forward direction: align server-known pending onto its owning message. + for (const pa of pendingApprovals) { + const enriched = { + pendingId: pa.pendingId, + toolName: pa.toolName, + arguments: pa.toolArguments, + reason: pa.reason, + status: 'pending_approval' as const, + findings: pa.findingsJson ? JSON.parse(pa.findingsJson) : undefined, + maxSeverity: pa.maxSeverity || undefined, + summary: pa.summary || undefined, + } + const target = indexById.get(pa.pendingId) + if (target) { + (target as any).metadata = { + ...(target as any).metadata, + currentPhase: 'awaiting_approval', + pendingApproval: enriched, + } + } else { + // Fallback: no message in the loaded history claims this pendingId + // (typical when the assistant message hasn't been persisted yet — + // e.g., approval fired before doOnComplete). Append to the last + // assistant; same as pre-RFC behavior, but logged so a regression + // where multiple unmatched pendings collide is observable. + const assistantMessages = messages.value.filter(m => m.role === 'assistant') + const lastAssistant = assistantMessages[assistantMessages.length - 1] + if (lastAssistant) { + console.warn('[hydrate] pendingId %s has no owning message — falling back to last assistant', pa.pendingId) + ;(lastAssistant as any).metadata = { ...(lastAssistant as any).metadata, currentPhase: 'awaiting_approval', - pendingApproval: { - pendingId: pa.pendingId, - toolName: pa.toolName, - arguments: pa.toolArguments, - reason: pa.reason, - status: 'pending_approval', - findings: pa.findingsJson ? JSON.parse(pa.findingsJson) : undefined, - maxSeverity: pa.maxSeverity || undefined, - summary: pa.summary || undefined, - } + pendingApproval: enriched, } } } } + + // Reverse direction: any local pending_approval whose pendingId is not + // in the server's list got resolved (timeout / consume) without a UI + // event — flip to expired so MessageBubble hides the banner. + const serverIds = new Set(pendingApprovals.map((p: any) => p.pendingId)) + for (const m of messages.value) { + if (m.role !== 'assistant') continue + const meta = (m as any).metadata + const local = meta?.pendingApproval + if (local?.status === 'pending_approval' + && local.pendingId + && !serverIds.has(local.pendingId)) { + (m as any).metadata = { + ...meta, + pendingApproval: { ...local, status: 'expired' }, + } + } + } } catch { // hydration 失败不影响正常使用 }