From dd7e561e484575da98dc2bcafc06b5b58cbbfdec Mon Sep 17 00:00:00 2001 From: mateaix <7333791@qq.com> Date: Sun, 23 Aug 2026 21:43:35 +0800 Subject: [PATCH] fix(agent): harden tool completion and team retries (#606) --- .../graph/executor/ToolExecutionExecutor.java | 105 +++++++++++++++++- .../vip/mate/agent/graph/node/ActionNode.java | 3 +- .../graph/plan/node/PlanGenerationNode.java | 4 +- .../graph/plan/node/StepExecutionNode.java | 37 +----- .../mate/skill/usage/SkillUsageService.java | 42 ++++--- .../team/service/TeamDispatchService.java | 36 +++++- .../mate/team/service/TeamRunViewFactory.java | 8 +- .../mate/team/service/TeamTaskService.java | 7 ++ .../vip/mate/team/tool/TeamTasksTool.java | 17 +++ .../tool/ToolInputValidationException.java | 19 ++++ .../vip/mate/tool/builtin/DocxRenderTool.java | 15 ++- ...ToolExecutionExecutorReturnDirectTest.java | 75 ++++++++++++- ...gentStreamAccumulatorReturnDirectTest.java | 57 ++++++++++ .../SkillUsageServiceActivityBubbleTest.java | 17 ++- .../team/service/TeamDispatchServiceTest.java | 34 ++++++ .../team/service/TeamRunViewFactoryTest.java | 12 ++ .../team/service/TeamTaskServiceTest.java | 16 +++ .../vip/mate/team/tool/TeamTasksToolTest.java | 18 +++ .../DocxRenderToolReturnDirectTest.java | 12 ++ .../chat/__tests__/useTeamRuns.test.ts | 30 +++++ .../src/composables/chat/useTeamRuns.ts | 22 +++- 21 files changed, 506 insertions(+), 80 deletions(-) create mode 100644 mateclaw-server/src/main/java/vip/mate/tool/ToolInputValidationException.java create mode 100644 mateclaw-server/src/test/java/vip/mate/channel/web/AgentStreamAccumulatorReturnDirectTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java index 4500618c..86c02be2 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java @@ -23,6 +23,7 @@ import vip.mate.approval.grant.AutoApproveResult; import vip.mate.approval.grant.WorkspaceLookupCache; import vip.mate.approval.grant.service.ApprovalGrantResolver; import vip.mate.channel.web.ChatStreamTracker; +import vip.mate.tool.ToolInputValidationException; import vip.mate.tool.guard.ToolExecutionGuardHelper; import vip.mate.tool.guard.ToolGuard; import vip.mate.tool.guard.ToolGuardResult; @@ -465,6 +466,22 @@ public class ToolExecutionExecutor { boolean isReplay, String requesterId, String workspaceBasePath, ChatOrigin origin) { + return execute(toolCalls, conversationId, agentId, isReplay, requesterId, + workspaceBasePath, origin, Set.of()); + } + + /** + * Preferred graph overload. {@code loadedSkills} is the conversation/run + * state captured before this batch, allowing the shared executor to reject + * both cross-iteration and same-batch duplicate {@code load_skill} calls + * before parallel execution starts. + */ + public ToolExecutionResult execute(List toolCalls, + String conversationId, String agentId, + boolean isReplay, String requesterId, + String workspaceBasePath, + ChatOrigin origin, + Set loadedSkills) { ChatOrigin safeOrigin = origin != null ? origin : ChatOrigin.EMPTY; // Reset per-turn audit dedupe state. A retried denied tool inside the // same turn writes a single audit row; the set is repopulated by the @@ -511,6 +528,15 @@ public class ToolExecutionExecutor { // ═══ Phase 1: 顺序 Guard + 分段 ═══ List preparedCalls = new ArrayList<>(); ApprovalBarrier barrier = null; + Set seenSkillLoads = new LinkedHashSet<>(); + if (loadedSkills != null) { + loadedSkills.stream() + .filter(Objects::nonNull) + .map(String::trim) + .filter(name -> !name.isEmpty()) + .map(name -> name.toLowerCase(Locale.ROOT)) + .forEach(seenSkillLoads::add); + } for (int i = 0; i < effectiveCalls.size(); i++) { AssistantMessage.ToolCall toolCall = effectiveCalls.get(i); @@ -590,6 +616,26 @@ public class ToolExecutionExecutor { } } + // load_skill is retrieval-only and concurrency-safe, so identical + // calls in one model response would otherwise race through the + // parallel phase and read/record the same skill twice. Keep this in + // the shared executor so both ActionNode and plan execution receive + // identical protection while preserving one response per call id. + if ("load_skill".equals(toolName)) { + String requestedSkill = requestedSkillName(arguments); + if (requestedSkill != null + && !seenSkillLoads.add(requestedSkill.toLowerCase(Locale.ROOT))) { + String message = "Skill '" + requestedSkill + "' was already loaded earlier in this run. " + + "Reuse the SKILL.md content already present in the conversation; " + + "do not call load_skill for this skill again."; + log.debug("[ToolExecutor] Skipping duplicate load_skill({})", requestedSkill); + events.add(GraphEventPublisher.toolComplete(toolCall.id(), toolName, message, true)); + allResponses.add(new ToolResponseMessage.ToolResponse( + toolCall.id(), responseName, message)); + continue; + } + } + // 2. ToolGuard 安全检查（replay 模式跳过） if (!isReplay) { GuardDecision decision = evaluateGuard(toolCall, toolName, arguments, @@ -678,6 +724,23 @@ public class ToolExecutionExecutor { rawEvidenceRef.get()); } + private static String requestedSkillName(String arguments) { + if (arguments == null || arguments.isBlank()) { + return null; + } + try { + var node = OBJECT_MAPPER.readTree(arguments); + var value = node.get("skillName"); + if (value == null || value.isNull() || value.asText().isBlank()) { + value = node.get("name"); + } + return value == null || value.isNull() || value.asText().isBlank() + ? null : value.asText().trim(); + } catch (Exception ignored) { + return null; + } + } + /** * Execute a pre-approved tool call (used by StepExecutionNode's replay path * after a user approves a previously-blocked invocation). @@ -767,6 +830,13 @@ public class ToolExecutionExecutor { } events.add(GraphEventPublisher.toolDirectResult( toolCall.id(), toolName, fullResult)); + // A direct result replaces the tool card's body, but the + // started event still needs a terminal pair so live clients + // do not leave the card spinning forever. The placeholder is + // deliberately used here: the full result remains confined to + // tool_direct_result / DIRECT_TOOL_OUTPUTS. + events.add(GraphEventPublisher.toolComplete( + toolCall.id(), toolName, DIRECT_TOOL_PLACEHOLDER, true)); return new ToolResponseMessage.ToolResponse( toolCall.id(), toolName, DIRECT_TOOL_PLACEHOLDER); } @@ -789,9 +859,12 @@ public class ToolExecutionExecutor { throw e; } catch (Exception e) { log.error("[ToolExecutor] Pre-approved tool {} failed: {}", toolName, e.getMessage()); - String safeError = isReturnDirect(callback) - ? "Tool execution failed (details withheld per returnDirect policy)" - : "Tool execution failed: " + e.getMessage(); + String validationError = safeInputValidationMessage(e); + String safeError = validationError != null + ? validationError + : isReturnDirect(callback) + ? "Tool execution failed (details withheld per returnDirect policy)" + : "Tool execution failed: " + e.getMessage(); events.add(GraphEventPublisher.toolComplete(toolCall.id(), toolName, safeError, false)); return new ToolResponseMessage.ToolResponse(toolCall.id(), toolName, safeError); } finally { @@ -1019,8 +1092,14 @@ public class ToolExecutionExecutor { if (streamTracker != null) { streamTracker.broadcastObject(pc.conversationId, GraphEventPublisher.EVENT_TOOL_DIRECT_RESULT, directEvent.data()); + streamTracker.broadcastObject(pc.conversationId, + GraphEventPublisher.EVENT_TOOL_COMPLETE, + GraphEventPublisher.toolComplete(pc.toolCall.id(), toolName, + DIRECT_TOOL_PLACEHOLDER, true).data()); streamTracker.updateRunningTool(pc.conversationId, null); } + events.add(GraphEventPublisher.toolComplete( + pc.toolCall.id(), toolName, DIRECT_TOOL_PLACEHOLDER, true)); // Placeholder keeps the tool_call_id ↔ tool_response pairing valid // for OpenAI-compatible providers, while withholding the data from // any subsequent LLM round (the graph won't take a next round — @@ -1080,9 +1159,12 @@ public class ToolExecutionExecutor { // or other sensitive substrings that should not enter LLM context. // Emit a generic placeholder instead. Full error still goes to logs // for operator diagnosis. - String reportedError = isReturnDirect(pc.callback) - ? "Tool execution failed (details withheld per returnDirect policy)" - : normalizeToolExecutionError(e); + String validationError = safeInputValidationMessage(e); + String reportedError = validationError != null + ? validationError + : isReturnDirect(pc.callback) + ? "Tool execution failed (details withheld per returnDirect policy)" + : normalizeToolExecutionError(e); events.add(GraphEventPublisher.toolComplete(pc.toolCall.id(), toolName, reportedError, false)); if (streamTracker != null) { streamTracker.broadcastObject(pc.conversationId, GraphEventPublisher.EVENT_TOOL_COMPLETE, @@ -1304,6 +1386,17 @@ public class ToolExecutionExecutor { return "Tool execution failed: " + message; } + private String safeInputValidationMessage(Throwable error) { + Throwable current = error; + while (current != null) { + if (current instanceof ToolInputValidationException validation) { + return "Tool input validation failed: " + validation.getMessage(); + } + current = current.getCause(); + } + return null; + } + /** * Issue #46 — when a tool callback miss happens, check whether the * unrecognized name actually matches an active skill. If it does, return diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/node/ActionNode.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/node/ActionNode.java index 424ac63f..8423ae04 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/node/ActionNode.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/node/ActionNode.java @@ -144,7 +144,8 @@ public class ActionNode implements NodeAction { // 委托 ToolExecutionExecutor 执行（两阶段：顺序 Guard + 分段并发执行） ToolExecutionExecutor.ToolExecutionResult result = executor.execute( - toolCalls, conversationId, agentId, isReplay, requesterId, workspaceBasePath, origin); + toolCalls, conversationId, agentId, isReplay, requesterId, workspaceBasePath, origin, + accessor.loadedSkills()); ToolResponseMessage toolResponseMessage = ToolResponseMessage.builder() .responses(result.responses()) diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/plan/node/PlanGenerationNode.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/plan/node/PlanGenerationNode.java index 44bd5a3d..26b9f1d3 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/plan/node/PlanGenerationNode.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/plan/node/PlanGenerationNode.java @@ -623,7 +623,9 @@ public class PlanGenerationNode implements NodeAction { + "相互独立的步骤请不要标注前置，以便并行执行。\n" + "3. 每个步骤描述必须自包含——执行成员看不到本对话，把所需的输入与要求写进步骤里。\n" + "4. 若用户要求编号轮次、检查点区间或连续跟踪，必须包含一个专门的共享跟踪步骤，" - + "明确区间、证据格式和完成条件；不要只把轮次要求埋在普通交付步骤中。")); + + "明确区间、证据格式和完成条件；不要只把轮次要求埋在普通交付步骤中。\n" + + "5. 不要创建专门的‘最终汇总/总结/验收’成员步骤；系统会在所有任务结束后自动汇总。" + + "把必要的自检和验收标准写进实际产出步骤，避免为了复述结果增加串行任务。")); } else { List delegatable = listDelegatableAgents(chatOrigin.workspaceId(), agentId); if (!delegatable.isEmpty()) { diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/plan/node/StepExecutionNode.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/plan/node/StepExecutionNode.java index 6cbfba30..2fc267b4 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/plan/node/StepExecutionNode.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/plan/node/StepExecutionNode.java @@ -368,7 +368,8 @@ public class StepExecutionNode implements NodeAction { } else { // 非预批准工具走正常执行器 ToolExecutionExecutor.ToolExecutionResult execResult = executor.execute( - List.of(toolCall), conversationId, agentId, false, "", workspaceBasePath, chatOrigin); + List.of(toolCall), conversationId, agentId, false, "", workspaceBasePath, + chatOrigin, loadedSkills); toolResponses.addAll(execResult.responses()); events.addAll(execResult.events()); if (execResult.hasDirectOutputs()) { @@ -383,18 +384,10 @@ public class StepExecutionNode implements NodeAction { } } else { // 正常路径：委托 ToolExecutionExecutor（支持并发执行 + 审批 barrier） - List executableToolCalls = new ArrayList<>(); - for (AssistantMessage.ToolCall toolCall : allToolCalls) { - String alreadyLoadedSkill = alreadyLoadedSkillName(toolCall, loadedSkills); - if (alreadyLoadedSkill != null) { - toolResponses.add(alreadyLoadedSkillResponse(toolCall, alreadyLoadedSkill)); - } else { - executableToolCalls.add(toolCall); - } - } - if (!executableToolCalls.isEmpty()) { + if (!allToolCalls.isEmpty()) { ToolExecutionExecutor.ToolExecutionResult execResult = executor.execute( - executableToolCalls, conversationId, agentId, false, "", workspaceBasePath, chatOrigin); + allToolCalls, conversationId, agentId, false, "", workspaceBasePath, + chatOrigin, loadedSkills); toolResponses.addAll(execResult.responses()); events.addAll(execResult.events()); if (execResult.hasDirectOutputs()) { @@ -891,26 +884,6 @@ public class StepExecutionNode implements NodeAction { return String.format("步骤%d结果：%s", stepIndex + 1, result); } - private static String alreadyLoadedSkillName(AssistantMessage.ToolCall toolCall, Set loadedSkills) { - if (toolCall == null || loadedSkills == null || loadedSkills.isEmpty()) { - return null; - } - Set requested = ActionNode.extractLoadedSkillNames(List.of(toolCall)); - if (requested.isEmpty()) { - return null; - } - String skillName = requested.iterator().next(); - return loadedSkills.contains(skillName) ? skillName : null; - } - - private static ToolResponseMessage.ToolResponse alreadyLoadedSkillResponse( - AssistantMessage.ToolCall toolCall, String skillName) { - String message = "Skill '" + skillName + "' was already loaded earlier in this run. " - + "Reuse the SKILL.md content already present in the conversation; " - + "do not call load_skill for this skill again."; - return new ToolResponseMessage.ToolResponse(toolCall.id(), toolCall.name(), message); - } - /** * 判断当前工具调用是否与预批准 payload 中的工具名匹配。 * payload 格式: {"name":"toolName","arguments":"...","status":"running"} diff --git a/mateclaw-server/src/main/java/vip/mate/skill/usage/SkillUsageService.java b/mateclaw-server/src/main/java/vip/mate/skill/usage/SkillUsageService.java index 27438823..3b3020a9 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/usage/SkillUsageService.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/usage/SkillUsageService.java @@ -1,8 +1,10 @@ package vip.mate.skill.usage; import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper; +import com.baomidou.mybatisplus.core.conditions.update.LambdaUpdateWrapper; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.springframework.dao.DuplicateKeyException; import org.springframework.stereotype.Service; import vip.mate.skill.lifecycle.SkillLifecycleService; import vip.mate.skill.repository.SkillUsageStatMapper; @@ -28,14 +30,11 @@ public class SkillUsageService { try { Long scopedAgentId = agentId != null ? agentId : 0L; String scopedConversationId = blankToEmpty(conversationId); - SkillUsageStatEntity row = mapper.selectOne(new LambdaQueryWrapper() - .eq(SkillUsageStatEntity::getSkillName, skill.getName()) - .eq(SkillUsageStatEntity::getAgentId, scopedAgentId) - .eq(SkillUsageStatEntity::getConversationId, scopedConversationId) - .last("LIMIT 1")); LocalDateTime now = LocalDateTime.now(); - if (row == null) { - row = new SkillUsageStatEntity(); + int updated = incrementExisting(skill, scopedAgentId, scopedConversationId, + filePath, tokenEstimate, now); + if (updated == 0) { + SkillUsageStatEntity row = new SkillUsageStatEntity(); row.setSkillName(skill.getName()); row.setSkillId(skill.getId()); row.setAgentId(scopedAgentId); @@ -45,14 +44,14 @@ public class SkillUsageService { row.setLastFilePath(filePath); row.setLastTokenEstimate(tokenEstimate); row.setDeleted(0); - mapper.insert(row); - } else { - row.setSkillId(skill.getId()); - row.setLoadCount((row.getLoadCount() == null ? 0L : row.getLoadCount()) + 1); - row.setLastLoadedAt(now); - row.setLastFilePath(filePath); - row.setLastTokenEstimate(tokenEstimate); - mapper.updateById(row); + try { + mapper.insert(row); + } catch (DuplicateKeyException race) { + // Another parallel invocation inserted the same scoped row + // after our update missed it. Retry as one atomic update. + incrementExisting(skill, scopedAgentId, scopedConversationId, + filePath, tokenEstimate, now); + } } // Mirror the activity anchor onto mate_skill so the lifecycle // curator's daily scan stays a single indexed select. @@ -62,6 +61,19 @@ public class SkillUsageService { } } + private int incrementExisting(ResolvedSkill skill, Long agentId, String conversationId, + String filePath, int tokenEstimate, LocalDateTime now) { + return mapper.update(null, new LambdaUpdateWrapper() + .set(SkillUsageStatEntity::getSkillId, skill.getId()) + .set(SkillUsageStatEntity::getLastLoadedAt, now) + .set(SkillUsageStatEntity::getLastFilePath, filePath) + .set(SkillUsageStatEntity::getLastTokenEstimate, tokenEstimate) + .setSql("load_count = COALESCE(load_count, 0) + 1") + .eq(SkillUsageStatEntity::getSkillName, skill.getName()) + .eq(SkillUsageStatEntity::getAgentId, agentId) + .eq(SkillUsageStatEntity::getConversationId, conversationId)); + } + public Set recentLoadedSkillNames(Long agentId, int limit) { if (agentId == null || limit <= 0) return Set.of(); try { diff --git a/mateclaw-server/src/main/java/vip/mate/team/service/TeamDispatchService.java b/mateclaw-server/src/main/java/vip/mate/team/service/TeamDispatchService.java index e371bcdf..92da9640 100644 --- a/mateclaw-server/src/main/java/vip/mate/team/service/TeamDispatchService.java +++ b/mateclaw-server/src/main/java/vip/mate/team/service/TeamDispatchService.java @@ -55,6 +55,9 @@ public class TeamDispatchService { /** Result summaries are capped before persisting to keep the board readable. */ static final int MAX_RESULT_CHARS = 8000; + /** Empty/fallback member runs get one recovery attempt, not three long identical runs. */ + static final int MAX_RESPONSE_FAILURE_DISPATCHES = 2; + private static final Pattern GENERATED_FILE_MARKDOWN_LINK = Pattern.compile( "\\[([^\\]\\r\\n]{1,200})]\\(((?:https?://[^/\\s)\\]]+)?/api/v1/files/generated/[A-Za-z0-9-]+)\\)"); @@ -178,6 +181,13 @@ public class TeamDispatchService { } dispatchedThisRound.add(assignee); TeamTaskEntity assigned = taskService.getTask(task.getId()); + // assignTask clears the persisted reason for a clean running state, + // but the worker needs the previous failure feedback or an + // automatic retry is just the same prompt sent to the same agent. + if (assigned != null && (assigned.getReason() == null || assigned.getReason().isBlank()) + && task.getReason() != null && !task.getReason().isBlank()) { + assigned.setReason(task.getReason()); + } DISPATCH_EXECUTOR.submit(() -> runTask(teamId, assigned)); } } @@ -279,10 +289,13 @@ public class TeamDispatchService { String invalidReason = invalidResultReason(current, reply, attachedGeneratedFile); if (invalidReason != null) { int attempts = current.getDispatchCount() == null ? 0 : current.getDispatchCount(); - if (attempts < TeamTaskService.MAX_DISPATCHES + int maxAttempts = isResponseGenerationFailure(invalidReason) + ? MAX_RESPONSE_FAILURE_DISPATCHES + : TeamTaskService.MAX_DISPATCHES; + if (attempts < maxAttempts && taskService.requeueUnusableResult(task.getId(), invalidReason)) { log.warn("Team task #{} produced an unusable result on attempt {}/{}; requeued: {}", - task.getTaskNumber(), attempts, TeamTaskService.MAX_DISPATCHES, + task.getTaskNumber(), attempts, maxAttempts, invalidReason); broadcast(task, "team_task_retrying", Map.of("reason", invalidReason)); return; @@ -364,6 +377,11 @@ public class TeamDispatchService { return null; } + private boolean isResponseGenerationFailure(String reason) { + return "member produced no result".equals(reason) + || "member response generation failed".equals(reason); + } + private boolean looksLikeClarificationQuestion(String reply) { if (reply == null) { return false; @@ -390,7 +408,10 @@ public class TeamDispatchService { } private boolean attachGeneratedFileDeliverable(TeamTaskEntity task, String reply) { - if (!requiresDeliverable(task) || reply == null || reply.isBlank()) { + // A render link is a useful task artifact regardless of how the task + // was created. The metadata flag controls validation/retry semantics, + // not whether an otherwise valid generated file is discoverable in UI. + if (reply == null || reply.isBlank()) { return false; } Matcher link = GENERATED_FILE_MARKDOWN_LINK.matcher(reply); @@ -430,7 +451,7 @@ public class TeamDispatchService { static final int MAX_LEAD_ATTACHMENT_SECTION_CHARS = 6000; /** The full instruction envelope the member receives. */ - private String buildDispatchContent(TeamTaskEntity task) { + String buildDispatchContent(TeamTaskEntity task) { StringBuilder sb = new StringBuilder(1024); sb.append("[Assigned team task #").append(task.getTaskNumber()) .append(" (taskId: ").append(task.getId()).append(")]\n") @@ -438,6 +459,13 @@ public class TeamDispatchService { if (task.getDescription() != null && !task.getDescription().isBlank()) { sb.append("\n").append(task.getDescription()).append('\n'); } + if (task.getDispatchCount() != null && task.getDispatchCount() > 1 + && task.getReason() != null && !task.getReason().isBlank()) { + sb.append("\n[Retry feedback]\n") + .append("The previous attempt was rejected: ") + .append(truncate(task.getReason().strip(), 500)) + .append(". Correct that failure in this attempt; do not repeat the same empty or fallback response.\n"); + } appendLeadAttachmentContext(sb, task); appendPrerequisiteResults(sb, task); sb.append(""" diff --git a/mateclaw-server/src/main/java/vip/mate/team/service/TeamRunViewFactory.java b/mateclaw-server/src/main/java/vip/mate/team/service/TeamRunViewFactory.java index f4a9fdae..720fedb9 100644 --- a/mateclaw-server/src/main/java/vip/mate/team/service/TeamRunViewFactory.java +++ b/mateclaw-server/src/main/java/vip/mate/team/service/TeamRunViewFactory.java @@ -191,11 +191,9 @@ final class TeamRunViewFactory { task.getId(), message == null ? task.getSubject() : message, task.getUpdateTime())); } } - String quality = outcomeQuality(run, tasks); - if ("fallback".equals(quality) || "partial".equals(quality)) { - items.add(new TeamRunView.AttentionItem("run:" + run.getId() + ":synthesis", "synthesis", - "warning", 10, null, "Final synthesis used a degraded outcome", run.getUpdateTime())); - } + // Synthesis quality is already exposed as outcomeQuality. A fallback + // summary is informative but has no user action, so it must not inflate + // the board's "needs attention" count. if (text(run.getStopReason()) != null) { items.add(new TeamRunView.AttentionItem("run:" + run.getId() + ":stopped", "stopped", "warning", 10, null, run.getStopReason(), run.getUpdateTime())); diff --git a/mateclaw-server/src/main/java/vip/mate/team/service/TeamTaskService.java b/mateclaw-server/src/main/java/vip/mate/team/service/TeamTaskService.java index 5397d1f0..e18846c9 100644 --- a/mateclaw-server/src/main/java/vip/mate/team/service/TeamTaskService.java +++ b/mateclaw-server/src/main/java/vip/mate/team/service/TeamTaskService.java @@ -559,6 +559,13 @@ public class TeamTaskService { if (deliverables == null) { deliverables = new JSONArray(); } + for (Object item : deliverables) { + if (item instanceof JSONObject existing + && trimmedUrl.equals(existing.getStr("url"))) { + log.debug("Team task {} deliverable already attached: {}", taskId, trimmedUrl); + return; + } + } if (deliverables.size() >= MAX_DELIVERABLES) { throw new IllegalStateException("task #" + task.getTaskNumber() + " already has " + MAX_DELIVERABLES + " deliverables; consolidate outputs instead of adding more"); diff --git a/mateclaw-server/src/main/java/vip/mate/team/tool/TeamTasksTool.java b/mateclaw-server/src/main/java/vip/mate/team/tool/TeamTasksTool.java index 2d08a920..83a2cdeb 100644 --- a/mateclaw-server/src/main/java/vip/mate/team/tool/TeamTasksTool.java +++ b/mateclaw-server/src/main/java/vip/mate/team/tool/TeamTasksTool.java @@ -31,6 +31,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.regex.Pattern; /** * Shared team task board exposed to the LLM. One multi-action tool (rather @@ -48,6 +49,10 @@ import java.util.Optional; @Slf4j public class TeamTasksTool { + private static final Pattern DELIVERABLE_REQUEST = Pattern.compile( + "(?i)(交付物|生成.{0,8}(文件|文档)|文档成稿|报告成稿|" + + "docx|xlsx|pptx|pdf|deliverable|document|spreadsheet|presentation)"); + private final TeamService teamService; private final TeamTaskService taskService; private final TeamRunService runService; @@ -201,6 +206,7 @@ public class TeamTasksTool { .blockedBy(parseIdList(blockedBy)) .requireApproval(Boolean.TRUE.equals(requireApproval)) .leadConversationId(conversationId) + .metadata(deliverableMetadata(subject, description)) .build()); eventChannel.publishTaskEvent(task, "team_task_created", Map.of()); return "✓ Created task #" + task.getTaskNumber() + " (id: " + task.getId() @@ -211,6 +217,17 @@ public class TeamTasksTool { + " Seal the run after all tasks are staged."; } + private String deliverableMetadata(String subject, String description) { + String taskText = (subject == null ? "" : subject) + "\n" + + (description == null ? "" : description); + if (!DELIVERABLE_REQUEST.matcher(taskText).find()) { + return null; + } + return new cn.hutool.json.JSONObject() + .set("deliverableRequired", true) + .toString(); + } + private String sealRun(AgentTeamEntity team, boolean isLead, Long workspaceId, String conversationId, String runId) { if (!isLead) { diff --git a/mateclaw-server/src/main/java/vip/mate/tool/ToolInputValidationException.java b/mateclaw-server/src/main/java/vip/mate/tool/ToolInputValidationException.java new file mode 100644 index 00000000..84feb8b2 --- /dev/null +++ b/mateclaw-server/src/main/java/vip/mate/tool/ToolInputValidationException.java @@ -0,0 +1,19 @@ +package vip.mate.tool; + +/** + * Signals an LLM-correctable tool argument error whose message is safe to + * return to the model, including for {@code returnDirect} tools. + * + *

Ordinary exceptions from direct tools remain redacted because they can + * contain credentials, connection strings, or other sensitive internals. + */ +public class ToolInputValidationException extends RuntimeException { + + public ToolInputValidationException(String message) { + super(message); + } + + public ToolInputValidationException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/DocxRenderTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/DocxRenderTool.java index 73086630..3174ea7c 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/DocxRenderTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/DocxRenderTool.java @@ -7,6 +7,7 @@ import org.springframework.ai.tool.annotation.ToolParam; import org.springframework.ai.chat.model.ToolContext; import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; +import vip.mate.tool.ToolInputValidationException; import vip.mate.tool.document.FilenameSanitizer; import vip.mate.tool.document.GeneratedFileCache; import vip.mate.tool.document.GeneratedFileLink; @@ -76,7 +77,9 @@ public class DocxRenderTool { @Nullable ToolContext ctx) { if (markdown == null || markdown.isBlank()) { - return "错误：markdown 参数为空，无法生成文档。"; + throw new ToolInputValidationException( + "markdown must not be blank; provide the document content, " + + "or write it to a .md file and call renderDocxFromFile"); } String displayName = FilenameSanitizer.sanitize(filename, "document", ".docx") + ".docx"; @@ -90,7 +93,7 @@ public class DocxRenderTool { return GeneratedFileLink.resultZh(bytes, displayName, DOCX_MIME, cache, "文档", ctx); } catch (Exception e) { log.error("[DocxRender] render failed for {}: {}", displayName, e.getMessage(), e); - return "渲染失败：" + e.getMessage(); + throw new IllegalStateException("DOCX rendering failed", e); } } @@ -142,7 +145,7 @@ public class DocxRenderTool { try { input = MarkdownInputResolver.readSingle(filePath); } catch (ResolveException e) { - return "Error: " + e.getMessage(); + throw new ToolInputValidationException(e.getMessage(), e); } String displayName = FilenameSanitizer.sanitize(filename, "document", ".docx") + ".docx"; @@ -157,7 +160,7 @@ public class DocxRenderTool { } catch (Exception e) { log.error("[DocxRender] render failed for {} (source: {}): {}", displayName, input.sources().get(0), e.getMessage(), e); - return "Render failed: " + e.getMessage(); + throw new IllegalStateException("DOCX rendering failed", e); } } @@ -202,7 +205,7 @@ public class DocxRenderTool { try { input = MarkdownInputResolver.readManyJoined(filePaths); } catch (ResolveException e) { - return "Error: " + e.getMessage(); + throw new ToolInputValidationException(e.getMessage(), e); } String displayName = FilenameSanitizer.sanitize(filename, "document", ".docx") + ".docx"; @@ -219,7 +222,7 @@ public class DocxRenderTool { } catch (Exception e) { log.error("[DocxRender] render failed for {} (sources: {}): {}", displayName, input.sources(), e.getMessage(), e); - return "Render failed: " + e.getMessage(); + throw new IllegalStateException("DOCX rendering failed", e); } } diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/ToolExecutionExecutorReturnDirectTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/ToolExecutionExecutorReturnDirectTest.java index 18f45ed1..d900f65e 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/ToolExecutionExecutorReturnDirectTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/ToolExecutionExecutorReturnDirectTest.java @@ -10,11 +10,15 @@ import org.springframework.ai.tool.definition.ToolDefinition; import org.springframework.ai.tool.metadata.ToolMetadata; import vip.mate.agent.AgentToolSet; import vip.mate.agent.GraphEventPublisher; +import vip.mate.agent.context.ChatOrigin; import vip.mate.agent.graph.state.DirectToolOutput; +import vip.mate.tool.ToolInputValidationException; import vip.mate.tool.guard.ToolGuard; import vip.mate.tool.guard.ToolGuardResult; import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import static org.junit.jupiter.api.Assertions.*; @@ -29,6 +33,7 @@ import static org.junit.jupiter.api.Assertions.*; * fixed placeholder, not the sensitive content. *

  • An {@code EVENT_TOOL_DIRECT_RESULT} event is emitted with the full text * and {@code renderAs=assistant_message}.
    • + *
  • A payload-free {@code EVENT_TOOL_COMPLETE} closes the live tool card.
    • *
  • Non-direct tools in the same batch keep their existing behavior.
    • * */ @@ -81,11 +86,17 @@ class ToolExecutionExecutorReturnDirectTest { assertEquals(SECRET, data.get("result")); assertEquals("assistant_message", data.get("renderAs")); - // (4) no tool_call_completed event for the direct tool — direct path replaces it - boolean hasCompleted = result.events().stream() - .anyMatch(e -> GraphEventPublisher.EVENT_TOOL_COMPLETE.equals(e.type())); - assertFalse(hasCompleted, "direct path replaces tool_call_completed; double-emit would " + - "leak the placeholder into UI as a tool result card"); + // (4) the started tool card receives a terminal pair, without leaking + // the direct payload into its ordinary result field. + var completed = result.events().stream() + .filter(e -> GraphEventPublisher.EVENT_TOOL_COMPLETE.equals(e.type())) + .toList(); + assertEquals(1, completed.size()); + assertEquals(Boolean.TRUE, completed.get(0).data().get("success")); + assertEquals(ToolExecutionExecutor.DIRECT_TOOL_PLACEHOLDER, + completed.get(0).data().get("result")); + assertFalse(String.valueOf(completed.get(0).data().get("result")) + .contains("EMPLOYEE-SALARY")); } @Test @@ -110,6 +121,39 @@ class ToolExecutionExecutorReturnDirectTest { assertFalse(hasDirect); } + @Test + @DisplayName("duplicate load_skill calls execute once across batch and loaded state") + void duplicateSkillLoadsAreShortCircuitedBeforeParallelExecution() { + AtomicInteger executions = new AtomicInteger(); + ToolCallback loadSkill = stubCallback("load_skill", false, args -> { + executions.incrementAndGet(); + return "skill instructions"; + }); + ToolExecutionExecutor executor = newExecutor(loadSkill); + AssistantMessage.ToolCall first = new AssistantMessage.ToolCall( + "skill_1", "function", "load_skill", "{\"skillName\":\"docx\"}"); + AssistantMessage.ToolCall duplicate = new AssistantMessage.ToolCall( + "skill_2", "function", "load_skill", "{\"skillName\":\"DOCX\"}"); + + ToolExecutionExecutor.ToolExecutionResult batch = executor.execute( + List.of(first, duplicate), "conv", "agent", false, "", null, + ChatOrigin.EMPTY, Set.of()); + + assertEquals(1, executions.get()); + assertEquals(2, batch.responses().size()); + assertTrue(batch.responses().get(1).responseData().contains("already loaded")); + assertTrue(batch.events().stream().anyMatch(event -> + GraphEventPublisher.EVENT_TOOL_COMPLETE.equals(event.type()) + && "skill_2".equals(event.data().get("toolCallId")) + && Boolean.TRUE.equals(event.data().get("success")))); + + ToolExecutionExecutor.ToolExecutionResult laterTurn = executor.execute( + List.of(first), "conv", "agent", false, "", null, + ChatOrigin.EMPTY, Set.of("docx")); + assertEquals(1, executions.get()); + assertTrue(laterTurn.responses().getFirst().responseData().contains("already loaded")); + } + @Test @DisplayName("RFC-052: returnDirect tool throwing yields generic message (no exception details leak)") void directTool_throwing_genericErrorMessage() { @@ -133,6 +177,27 @@ class ToolExecutionExecutorReturnDirectTest { assertFalse(content.contains("OracleDriver"), "Stack/connection details must not leak"); } + @Test + @DisplayName("RFC-052: safe input validation errors return to the model for correction") + void directTool_validationErrorIsActionableAndDoesNotShortCircuit() { + ToolCallback invalidDirect = stubCallback("renderDocx", true, args -> { + throw new ToolInputValidationException("markdown must not be blank"); + }); + ToolExecutionExecutor executor = newExecutor(invalidDirect); + + AssistantMessage.ToolCall call = new AssistantMessage.ToolCall( + "call_v", "function", "renderDocx", "{\"markdown\":\"\"}"); + ToolExecutionExecutor.ToolExecutionResult result = + executor.execute(List.of(call), "conv_v", "agent_v", false, "user_v", null); + + assertFalse(result.hasDirectOutputs(), "invalid input must not trigger returnDirect"); + assertEquals("Tool input validation failed: markdown must not be blank", + result.responses().get(0).responseData()); + assertTrue(result.events().stream() + .anyMatch(e -> GraphEventPublisher.EVENT_TOOL_COMPLETE.equals(e.type()) + && Boolean.FALSE.equals(e.data().get("success")))); + } + @Test @DisplayName("RFC-052: pre-approved direct tool replays through direct path") void executePreApproved_directTool_takesDirectPath() { diff --git a/mateclaw-server/src/test/java/vip/mate/channel/web/AgentStreamAccumulatorReturnDirectTest.java b/mateclaw-server/src/test/java/vip/mate/channel/web/AgentStreamAccumulatorReturnDirectTest.java new file mode 100644 index 00000000..e7f613c6 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/channel/web/AgentStreamAccumulatorReturnDirectTest.java @@ -0,0 +1,57 @@ +package vip.mate.channel.web; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import vip.mate.agent.AgentService.StreamDelta; + +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +class AgentStreamAccumulatorReturnDirectTest { + + private static final String DIRECT_PLACEHOLDER = + "[Tool result returned directly to user. " + + "Content withheld from model context per tool policy.]"; + + private static final AgentStreamAccumulator.Sink NOOP_SINK = + new AgentStreamAccumulator.Sink() { + @Override + public void broadcast(String conversationId, String eventName, Object payload) { } + + @Override + public void updatePhase(String conversationId, String phase) { } + }; + + @Test + @DisplayName("returnDirect result terminates its tool card without persisting the direct payload there") + void directResultHasCompletedToolPair() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + AgentStreamAccumulator accumulator = new AgentStreamAccumulator(mapper, NOOP_SINK); + String conversationId = "conv-direct"; + + accumulator.accept(StreamDelta.event("tool_call_started", Map.of( + "toolCallId", "call-1", "toolName", "renderDocx", "arguments", "{}")), + conversationId); + accumulator.accept(StreamDelta.event("tool_direct_result", Map.of( + "toolCallId", "call-1", "toolName", "renderDocx", + "result", "SECRET-DIRECT-PAYLOAD")), conversationId); + accumulator.accept(StreamDelta.event("tool_call_completed", Map.of( + "toolCallId", "call-1", "toolName", "renderDocx", + "result", DIRECT_PLACEHOLDER, "success", true)), + conversationId); + + JsonNode metadata = mapper.readTree(accumulator.toMetadataJson()); + JsonNode call = metadata.path("toolCalls").get(0); + JsonNode segment = metadata.path("segments").get(0); + + assertEquals("completed", call.path("status").asText()); + assertEquals("completed", segment.path("status").asText()); + assertEquals(DIRECT_PLACEHOLDER, segment.path("toolResult").asText()); + assertFalse(metadata.toString().contains("SECRET-DIRECT-PAYLOAD")); + assertEquals("renderDocx", metadata.path("directToolNames").get(0).asText()); + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/skill/usage/SkillUsageServiceActivityBubbleTest.java b/mateclaw-server/src/test/java/vip/mate/skill/usage/SkillUsageServiceActivityBubbleTest.java index 0e94e902..e6b97a32 100644 --- a/mateclaw-server/src/test/java/vip/mate/skill/usage/SkillUsageServiceActivityBubbleTest.java +++ b/mateclaw-server/src/test/java/vip/mate/skill/usage/SkillUsageServiceActivityBubbleTest.java @@ -9,12 +9,15 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.dao.DuplicateKeyException; import vip.mate.skill.lifecycle.SkillLifecycleService; import vip.mate.skill.repository.SkillUsageStatMapper; import vip.mate.skill.runtime.model.ResolvedSkill; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -48,7 +51,6 @@ class SkillUsageServiceActivityBubbleTest { @Test void recordLoadedBubblesActivityToLifecycle() { ResolvedSkill skill = ResolvedSkill.builder().id(7L).name("demo").build(); - when(mapper.selectOne(any())).thenReturn(null); service.recordLoaded(skill, 1L, "conv-1", "SKILL.md", 100); @@ -60,4 +62,17 @@ class SkillUsageServiceActivityBubbleTest { service.recordLoaded(null, 1L, "conv-1", "SKILL.md", 100); verify(lifecycleService, never()).bumpActivity(any()); } + + @Test + void recordLoadedRetriesAtomicUpdateWhenParallelInsertWins() { + ResolvedSkill skill = ResolvedSkill.builder().id(7L).name("demo").build(); + when(mapper.update(any(), any())).thenReturn(0, 1); + doThrow(new DuplicateKeyException("parallel insert")) + .when(mapper).insert(any(SkillUsageStatEntity.class)); + + service.recordLoaded(skill, 1L, "conv-1", "SKILL.md", 100); + + verify(mapper, times(2)).update(any(), any()); + verify(lifecycleService).bumpActivity(7L); + } } diff --git a/mateclaw-server/src/test/java/vip/mate/team/service/TeamDispatchServiceTest.java b/mateclaw-server/src/test/java/vip/mate/team/service/TeamDispatchServiceTest.java index de5d6ade..35c71f2e 100644 --- a/mateclaw-server/src/test/java/vip/mate/team/service/TeamDispatchServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/team/service/TeamDispatchServiceTest.java @@ -215,6 +215,40 @@ class TeamDispatchServiceTest { verify(announceService).announceTaskSettled(failed); } + @Test + @DisplayName("a second empty member result fails without starting a third long run") + void settleEmptyResultFailsAfterSingleRecoveryAttempt() { + TeamTaskEntity running = task(1L, MEMBER_A); + running.setStatus(TeamTaskStatus.IN_PROGRESS); + running.setDispatchCount(TeamDispatchService.MAX_RESPONSE_FAILURE_DISPATCHES); + TeamTaskEntity failed = task(1L, MEMBER_A); + failed.setStatus(TeamTaskStatus.FAILED); + failed.setReason("member produced no result"); + when(taskService.getTask(1L)).thenReturn(running, failed); + when(taskService.failTask(1L, "member produced no result")).thenReturn(true); + + service.settleOutcome(running, ""); + + verify(taskService, never()).requeueUnusableResult(any(), anyString()); + verify(taskService).failTask(1L, "member produced no result"); + verify(eventChannel).publishTaskEvent(any(), eq("team_task_failed"), any()); + verify(announceService).announceTaskSettled(failed); + } + + @Test + @DisplayName("automatic retry tells the worker why its previous attempt was rejected") + void retryDispatchContentIncludesPreviousFailure() { + TeamTaskEntity retry = task(1L, MEMBER_A); + retry.setDispatchCount(2); + retry.setReason("member produced no result"); + + String content = service.buildDispatchContent(retry); + + assertTrue(content.contains("[Retry feedback]")); + assertTrue(content.contains("member produced no result")); + assertTrue(content.contains("do not repeat the same empty or fallback response")); + } + @Test @DisplayName("a declared deliverable task without an attachment is requeued") void settleMissingDeliverableRequeues() { diff --git a/mateclaw-server/src/test/java/vip/mate/team/service/TeamRunViewFactoryTest.java b/mateclaw-server/src/test/java/vip/mate/team/service/TeamRunViewFactoryTest.java index b8600c0a..7f6ed5df 100644 --- a/mateclaw-server/src/test/java/vip/mate/team/service/TeamRunViewFactoryTest.java +++ b/mateclaw-server/src/test/java/vip/mate/team/service/TeamRunViewFactoryTest.java @@ -69,6 +69,18 @@ class TeamRunViewFactoryTest { assertEquals(full.outcomeQuality(), summary.outcomeQuality()); } + @Test + void fallbackSummaryDoesNotCreateUnactionableAttentionItem() { + TeamRunEntity fallbackRun = run("{\"summaryQuality\":\"fallback\"}"); + TeamTaskEntity completed = task(101L, 201L, null); + + TeamRunView view = TeamRunViewFactory.create(fallbackRun, TeamRunStatus.COMPLETED, + new TeamRunView.Progress(1, 1, 0, 0, 100), List.of(completed), true); + + assertEquals("fallback", view.outcomeQuality()); + assertEquals(List.of(), view.attentionItems()); + } + @Test void aggregatesRunOnlyDeliverables() { TeamRunEntity run = run("{\"deliverables\":[{\"name\":\"report.pdf\"," diff --git a/mateclaw-server/src/test/java/vip/mate/team/service/TeamTaskServiceTest.java b/mateclaw-server/src/test/java/vip/mate/team/service/TeamTaskServiceTest.java index 5eb17afe..a8839b16 100644 --- a/mateclaw-server/src/test/java/vip/mate/team/service/TeamTaskServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/team/service/TeamTaskServiceTest.java @@ -542,6 +542,22 @@ class TeamTaskServiceTest { assertEquals("/api/v1/files/generated/abc", files.get(0).url()); } + @Test + @DisplayName("addDeliverable is idempotent for the same generated URL") + void addDeliverableIgnoresDuplicateUrl() { + TeamTaskEntity running = task(5L, TeamTaskStatus.IN_PROGRESS); + running.setOwnerAgentId(MEMBER_ID); + running.setMetadata("{\"deliverables\":[{\"name\":\"report.docx\"," + + "\"url\":\"/api/v1/files/generated/abc\"}]}"); + when(taskMapper.selectById(5L)).thenReturn(running); + + service.addDeliverable(5L, MEMBER_ID, "report-again.docx", + "/api/v1/files/generated/abc"); + + verify(taskMapper, never()).update(isNull(), any()); + verify(eventMapper, never()).insert(any(TeamTaskEventEntity.class)); + } + @Test @DisplayName("deliverable guards: external URL, non-owner, terminal task and overflow are rejected") void addDeliverableGuards() { diff --git a/mateclaw-server/src/test/java/vip/mate/team/tool/TeamTasksToolTest.java b/mateclaw-server/src/test/java/vip/mate/team/tool/TeamTasksToolTest.java index ff6395f4..30d8d1eb 100644 --- a/mateclaw-server/src/test/java/vip/mate/team/tool/TeamTasksToolTest.java +++ b/mateclaw-server/src/test/java/vip/mate/team/tool/TeamTasksToolTest.java @@ -295,6 +295,24 @@ class TeamTasksToolTest { assertTrue(captor.getValue().isRequireApproval()); } + @Test + @DisplayName("file-producing create task records a required deliverable contract") + void createInfersRequiredDeliverableFromDescription() { + callerIs(LEAD_ID); + when(runService.requireRun(RUN_ID, WORKSPACE_ID)) + .thenReturn(run(TEAM_ID, CONV, TeamRunStatus.PLANNING)); + when(taskService.createTask(any())).thenReturn(task(53L, TeamTaskStatus.PENDING)); + + tool.team_tasks("create", null, String.valueOf(RUN_ID), null, null, + "生成报告", "请生成最终 DOCX 文件", String.valueOf(MEMBER_ID), null, null, + null, null, null, null, null, null, null, null, null); + + ArgumentCaptor captor = + ArgumentCaptor.forClass(TeamTaskCreateCommand.class); + verify(taskService).createTask(captor.capture()); + assertTrue(captor.getValue().getMetadata().contains("\"deliverableRequired\":true")); + } + @Test @DisplayName("create requires an explicit run id") void createRequiresRunId() { diff --git a/mateclaw-server/src/test/java/vip/mate/tool/builtin/DocxRenderToolReturnDirectTest.java b/mateclaw-server/src/test/java/vip/mate/tool/builtin/DocxRenderToolReturnDirectTest.java index 31d0aecf..b60e44e9 100644 --- a/mateclaw-server/src/test/java/vip/mate/tool/builtin/DocxRenderToolReturnDirectTest.java +++ b/mateclaw-server/src/test/java/vip/mate/tool/builtin/DocxRenderToolReturnDirectTest.java @@ -3,7 +3,9 @@ package vip.mate.tool.builtin; import org.junit.jupiter.api.Test; import org.springframework.ai.tool.annotation.Tool; import org.springframework.ai.chat.model.ToolContext; +import vip.mate.tool.ToolInputValidationException; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class DocxRenderToolReturnDirectTest { @@ -15,6 +17,16 @@ class DocxRenderToolReturnDirectTest { assertReturnDirect("renderDocxFromFiles", java.util.List.class, String.class, String.class, ToolContext.class); } + @Test + void blankMarkdownIsARecoverableInputFailureNotADirectSuccess() { + DocxRenderTool tool = new DocxRenderTool(null, null); + + ToolInputValidationException error = assertThrows(ToolInputValidationException.class, + () -> tool.renderDocx(" ", "report", "A4", null)); + + assertTrue(error.getMessage().contains("markdown must not be blank")); + } + private static void assertReturnDirect(String methodName, Class... parameterTypes) throws Exception { Tool tool = DocxRenderTool.class .getMethod(methodName, parameterTypes) diff --git a/mateclaw-ui/src/composables/chat/__tests__/useTeamRuns.test.ts b/mateclaw-ui/src/composables/chat/__tests__/useTeamRuns.test.ts index dc098296..59b28bc8 100644 --- a/mateclaw-ui/src/composables/chat/__tests__/useTeamRuns.test.ts +++ b/mateclaw-ui/src/composables/chat/__tests__/useTeamRuns.test.ts @@ -184,6 +184,36 @@ describe('useTeamRuns', () => { scope.stop() }) + it('performs a trailing detail refresh when a terminal event arrives in flight', async () => { + let onEvent: ((event: TeamBoardEvent) => void) | undefined + let resolveStale!: (value: { data: TeamRun }) => void + let resolveTerminal!: (value: { data: TeamRun }) => void + const staleDetail = new Promise<{ data: TeamRun }>(resolve => { resolveStale = resolve }) + const terminalDetail = new Promise<{ data: TeamRun }>(resolve => { resolveTerminal = resolve }) + const dependencies: TeamRunsDependencies = { + listByConversation: vi.fn().mockResolvedValue({ data: [run('10')] }), + getRun: vi.fn() + .mockReturnValueOnce(staleDetail) + .mockReturnValueOnce(terminalDetail), + subscribe: vi.fn((_teamId, callback) => { onEvent = callback; return vi.fn() }), + } + const scope = effectScope() + const state = scope.run(() => useTeamRuns(ref('lead'), { dependencies }))! + await flush() + + onEvent!({ event: 'team_run_progress', data: { runId: '10' } }) + onEvent!({ event: 'team_run_completed', data: { runId: '10', status: 'completed' } }) + expect(dependencies.getRun).toHaveBeenCalledTimes(1) + + resolveStale({ data: run('10') }) + await flush() + expect(dependencies.getRun).toHaveBeenCalledTimes(2) + resolveTerminal({ data: run('10', 'team-1', 'completed') }) + await flush() + expect(state.runs.value[0].status).toBe('completed') + scope.stop() + }) + it('ignores team events belonging to another lead conversation', async () => { let onEvent: ((event: TeamBoardEvent) => void) | undefined const dependencies: TeamRunsDependencies = { diff --git a/mateclaw-ui/src/composables/chat/useTeamRuns.ts b/mateclaw-ui/src/composables/chat/useTeamRuns.ts index 5a4ee7cf..edbcd5e3 100644 --- a/mateclaw-ui/src/composables/chat/useTeamRuns.ts +++ b/mateclaw-ui/src/composables/chat/useTeamRuns.ts @@ -66,6 +66,7 @@ export function useTeamRuns( const loadingMore = ref(false) const subscriptions = new Map void>() const inFlight = new Map>() + const trailingRefresh = new Set() let generation = 0 let stopped = false @@ -91,11 +92,14 @@ export function useTeamRuns( ensureSubscriptions() } - const refreshRun = (runId: string): Promise => { + const refreshRun = (runId: string, requireTrailing = false): Promise => { const activeGeneration = generation const requestKey = `${activeGeneration}:${runId}` const existing = inFlight.get(requestKey) - if (existing) return existing + if (existing) { + if (requireTrailing) trailingRefresh.add(requestKey) + return existing + } const request = dependencies.getRun(runId) .then((result) => { if (!stopped && activeGeneration === generation) replaceRun(dataOf(result)) @@ -103,7 +107,12 @@ export function useTeamRuns( .catch((cause) => { if (!stopped && activeGeneration === generation) error.value = cause }) - .finally(() => { inFlight.delete(requestKey) }) + .finally(() => { + inFlight.delete(requestKey) + if (!stopped && activeGeneration === generation && trailingRefresh.delete(requestKey)) { + void refreshRun(runId) + } + }) inFlight.set(requestKey, request) return request } @@ -127,7 +136,10 @@ export function useTeamRuns( ? { ...current, status, progress } : run) } - void refreshRun(runId) + const terminalEvent = ['completed', 'failed', 'cancelled', 'stopped'].includes( + typeof event.data.status === 'string' ? event.data.status : '', + ) || ['team_run_completed', 'team_run_failed', 'team_run_cancelled', 'team_run_stopped'].includes(event.event) + void refreshRun(runId, terminalEvent) } const refresh = async () => { @@ -136,6 +148,7 @@ export function useTeamRuns( loadingMore.value = false cleanupSubscriptions() inFlight.clear() + trailingRefresh.clear() loading.value = true error.value = null try { @@ -194,6 +207,7 @@ export function useTeamRuns( generation += 1 stopWatch() cleanupSubscriptions() + trailingRefresh.clear() } if (getCurrentScope()) onScopeDispose(stop)