fix(agent): harden tool completion and team retries (#606)

This commit is contained in:
mateaix 2026-08-23 21:43:35 +08:00
parent 2372827762
commit dd7e561e48
21 changed files with 506 additions and 80 deletions

View File

@ -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<AssistantMessage.ToolCall> toolCalls,
String conversationId, String agentId,
boolean isReplay, String requesterId,
String workspaceBasePath,
ChatOrigin origin,
Set<String> 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<PreparedToolCall> preparedCalls = new ArrayList<>();
ApprovalBarrier barrier = null;
Set<String> 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

View File

@ -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())

View File

@ -623,7 +623,9 @@ public class PlanGenerationNode implements NodeAction {
+ "相互独立的步骤请不要标注前置,以便并行执行。\n"
+ "3. 每个步骤描述必须自包含——执行成员看不到本对话,把所需的输入与要求写进步骤里。\n"
+ "4. 若用户要求编号轮次、检查点区间或连续跟踪,必须包含一个专门的共享跟踪步骤,"
+ "明确区间、证据格式和完成条件;不要只把轮次要求埋在普通交付步骤中。"));
+ "明确区间、证据格式和完成条件;不要只把轮次要求埋在普通交付步骤中。\n"
+ "5. 不要创建专门的‘最终汇总/总结/验收’成员步骤;系统会在所有任务结束后自动汇总。"
+ "把必要的自检和验收标准写进实际产出步骤,避免为了复述结果增加串行任务。"));
} else {
List<AgentEntity> delegatable = listDelegatableAgents(chatOrigin.workspaceId(), agentId);
if (!delegatable.isEmpty()) {

View File

@ -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<AssistantMessage.ToolCall> 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<String> loadedSkills) {
if (toolCall == null || loadedSkills == null || loadedSkills.isEmpty()) {
return null;
}
Set<String> 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"}

View File

@ -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<SkillUsageStatEntity>()
.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<SkillUsageStatEntity>()
.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<String> recentLoadedSkillNames(Long agentId, int limit) {
if (agentId == null || limit <= 0) return Set.of();
try {

View File

@ -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("""

View File

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

View File

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

View File

@ -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) {

View File

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

View File

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

View File

@ -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.</li>
* <li>An {@code EVENT_TOOL_DIRECT_RESULT} event is emitted with the full text
* and {@code renderAs=assistant_message}.</li>
* <li>A payload-free {@code EVENT_TOOL_COMPLETE} closes the live tool card.</li>
* <li>Non-direct tools in the same batch keep their existing behavior.</li>
* </ol>
*/
@ -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() {

View File

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

View File

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

View File

@ -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() {

View File

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

View File

@ -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() {

View File

@ -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<TeamTaskCreateCommand> 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() {

View File

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

View File

@ -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 = {

View File

@ -66,6 +66,7 @@ export function useTeamRuns(
const loadingMore = ref(false)
const subscriptions = new Map<string, () => void>()
const inFlight = new Map<string, Promise<void>>()
const trailingRefresh = new Set<string>()
let generation = 0
let stopped = false
@ -91,11 +92,14 @@ export function useTeamRuns(
ensureSubscriptions()
}
const refreshRun = (runId: string): Promise<void> => {
const refreshRun = (runId: string, requireTrailing = false): Promise<void> => {
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)