fix(chat): stabilize long-task input recovery

This commit is contained in:
matevip 2026-08-27 05:21:59 -04:00
parent 01ed4a4fcd
commit 7eb44731e7
10 changed files with 258 additions and 21 deletions

View File

@ -12,12 +12,16 @@ import vip.mate.agent.graph.state.MateClawStateAccessor;
import vip.mate.goal.config.GoalProperties;
import vip.mate.goal.model.GoalEntity;
import vip.mate.goal.model.GoalEvaluationResult;
import vip.mate.goal.model.GoalResponse;
import vip.mate.goal.service.GoalEvaluationService;
import vip.mate.goal.service.GoalFollowupService;
import vip.mate.goal.service.GoalService;
import vip.mate.goal.service.GraphFlavor;
import vip.mate.workspace.conversation.ConversationService;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@ -205,7 +209,7 @@ public class GoalEvaluationNode implements NodeAction {
.events(List.of(goalEvent("goal_completed", Map.of(
"goalId", String.valueOf(completed.getId()),
"score", result.score(),
"goal", goalService.toResponse(completed)))))
"goal", stateSafeGoal(goalService.toResponse(completed))))))
.build();
}
@ -222,7 +226,7 @@ public class GoalEvaluationNode implements NodeAction {
"evalLlmCallsUsed", exhausted.getEvalLlmCallsUsed(),
"totalLlmCallsUsed", exhausted.totalLlmCallsUsed(),
"reason", reason,
"goal", goalService.toResponse(exhausted)))))
"goal", stateSafeGoal(goalService.toResponse(exhausted))))))
.build();
}
} catch (Throwable t) {
@ -246,7 +250,7 @@ public class GoalEvaluationNode implements NodeAction {
"score", result.score(),
"decision", result.decision(),
"gap", result.gap() == null ? "" : result.gap(),
"goal", goalService.toResponse(refreshed)))))
"goal", stateSafeGoal(goalService.toResponse(refreshed))))))
.build();
}
@ -310,7 +314,7 @@ public class GoalEvaluationNode implements NodeAction {
.events(List.of(goalEvent("goal_followup", Map.of(
"goalId", String.valueOf(refreshed.getId()),
"prompt", followup.get(),
"goal", goalService.toResponse(refreshed)))));
"goal", stateSafeGoal(goalService.toResponse(refreshed))))));
if (flavor == GraphFlavor.REACT) {
// ReAct: append the followup as a fresh user message via the
@ -364,10 +368,72 @@ public class GoalEvaluationNode implements NodeAction {
"goalId", String.valueOf(refreshed.getId()),
"score", result.score(),
"gap", result.gap() == null ? "" : result.gap(),
"goal", goalService.toResponse(refreshed)))))
"goal", stateSafeGoal(goalService.toResponse(refreshed))))))
.build();
}
/**
* Graph state may be checkpointed and restored through a generic map
* serializer. Keep event payloads limited to JSON primitives, maps and
* lists so a restored checklist cannot contain raw maps inside a typed
* {@link GoalResponse} bean and fail during SSE serialization.
*/
private static Map<String, Object> stateSafeGoal(GoalResponse goal) {
if (goal == null) {
return Map.of();
}
Map<String, Object> snapshot = new LinkedHashMap<>();
snapshot.put("id", stringId(goal.getId()));
snapshot.put("conversationId", goal.getConversationId());
snapshot.put("agentId", stringId(goal.getAgentId()));
snapshot.put("workspaceId", stringId(goal.getWorkspaceId()));
snapshot.put("createdBy", goal.getCreatedBy());
snapshot.put("title", goal.getTitle());
snapshot.put("description", goal.getDescription());
snapshot.put("exitCriteria", goal.getExitCriteria());
snapshot.put("successCheckPrompt", goal.getSuccessCheckPrompt());
snapshot.put("status", goal.getStatus() == null ? null : goal.getStatus().getValue());
snapshot.put("persistentExecution", goal.getPersistentExecution());
snapshot.put("turnBudget", goal.getTurnBudget());
snapshot.put("turnsUsed", goal.getTurnsUsed());
snapshot.put("llmCallBudget", goal.getLlmCallBudget());
snapshot.put("agentLlmCallsUsed", goal.getAgentLlmCallsUsed());
snapshot.put("evalLlmCallsUsed", goal.getEvalLlmCallsUsed());
snapshot.put("totalLlmCallsUsed", goal.getTotalLlmCallsUsed());
snapshot.put("progressSummary", goal.getProgressSummary());
snapshot.put("completionScore", goal.getCompletionScore());
snapshot.put("lastEvaluationAt", stringTime(goal.getLastEvaluationAt()));
snapshot.put("autoFollowupEnabled", goal.getAutoFollowupEnabled());
snapshot.put("followupCooldownSeconds", goal.getFollowupCooldownSeconds());
snapshot.put("lastFollowupAt", stringTime(goal.getLastFollowupAt()));
snapshot.put("version", goal.getVersion());
snapshot.put("createTime", stringTime(goal.getCreateTime()));
snapshot.put("updateTime", stringTime(goal.getUpdateTime()));
List<Map<String, Object>> criteria = new ArrayList<>();
if (goal.getCriteria() != null) {
goal.getCriteria().forEach(criterion -> {
if (criterion == null) return;
Map<String, Object> item = new LinkedHashMap<>();
item.put("id", criterion.id() == null ? "" : criterion.id());
item.put("text", criterion.text() == null ? "" : criterion.text());
item.put("passed", criterion.passed());
item.put("evidence", criterion.evidence() == null ? "" : criterion.evidence());
criteria.add(Collections.unmodifiableMap(item));
});
}
snapshot.put("criteria", List.copyOf(criteria));
return Collections.unmodifiableMap(snapshot);
}
private static String stringId(Long value) {
return value == null ? null : value.toString();
}
private static String stringTime(java.time.LocalDateTime value) {
return value == null ? null : value.toString();
}
/**
* Resolve the active goal for this run: prefer the turn-start
* {@code ACTIVE_GOAL} snapshot; if absent, fall back to a conversation

View File

@ -228,7 +228,8 @@ public class ChatController {
boolean isDenyCommand = "/deny".equals(normalizedMsg) || "deny".equals(normalizedMsg);
if (isApprovalCommand || isDenyCommand) {
PendingApproval pending = approvalService.findPendingByConversation(conversationId);
PendingApproval pending = findRequestedPendingApproval(
conversationId, request.getPendingApprovalId());
if (pending == null) {
try {
sendEvent(emitter, "error", Map.of("message", "当前没有待审批的工具调用"));
@ -302,7 +303,7 @@ public class ChatController {
conversationService.getMessageCount(conversationId)));
// deny 是正常 turn 终结用户可能在 awaiting_approval 阶段排了消息
ChatStreamTracker.CompletionResult denyCr = streamTracker.completeAndConsumeIfLast(conversationId);
if (denyCr.allDone() && hasQueuedInput(conversationId)) {
if (denyCr.allDone() && shouldDrainQueuedInput(conversationId, "completed")) {
startQueuedMessage(conversationId, emitter, approvalEmitterDone, username, requestBaseUrl);
} else {
completeEmitterQuietly(emitter, approvalEmitterDone);
@ -316,7 +317,7 @@ public class ChatController {
broadcastEvent(conversationId, "done", Map.of("status", "completed"));
// 审批记录被另一个请求消费但用户可能在等待期间排了消息
ChatStreamTracker.CompletionResult consumedNullCr = streamTracker.completeAndConsumeIfLast(conversationId);
if (consumedNullCr.allDone() && hasQueuedInput(conversationId)) {
if (consumedNullCr.allDone() && shouldDrainQueuedInput(conversationId, "completed")) {
startQueuedMessage(conversationId, emitter, approvalEmitterDone, username, requestBaseUrl);
} else {
completeEmitterQuietly(emitter, approvalEmitterDone);
@ -421,7 +422,7 @@ public class ChatController {
} finally {
ChatStreamTracker.CompletionResult cr = streamTracker.completeAndConsumeIfLast(conversationId);
if (cr.allDone()) {
if (hasQueuedInput(conversationId)) {
if (shouldDrainQueuedInput(conversationId, persistStatus)) {
startQueuedMessage(conversationId, emitter, approvalEmitterDone, username, requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
@ -522,7 +523,7 @@ public class ChatController {
streamTracker.clearInterruptState(conversationId);
ChatStreamTracker.CompletionResult cr = streamTracker.completeAndConsumeIfLast(conversationId);
if (cr.allDone()) {
if (hasQueuedInput(conversationId)) {
if (shouldDrainQueuedInput(conversationId, errStatus)) {
startQueuedMessage(conversationId, emitter, approvalEmitterDone, username, requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
@ -778,7 +779,7 @@ public class ChatController {
// run it" condition; align with them. If the user
// genuinely doesn't want continuation, no message would
// have been in messageQueue to begin with.
if (hasQueuedInput(conversationId)) {
if (shouldDrainQueuedInput(conversationId, persistStatus)) {
startQueuedMessage(conversationId, emitter, emitterDone, username, requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
@ -871,7 +872,7 @@ public class ChatController {
streamTracker.clearInterruptState(conversationId);
ChatStreamTracker.CompletionResult cr = streamTracker.completeAndConsumeIfLast(conversationId);
if (cr.allDone()) {
if (hasQueuedInput(conversationId)) {
if (shouldDrainQueuedInput(conversationId, status)) {
// 无论中断类型都消费排队消息修复 Disposable 不可用时队列被丢弃的 bug
startQueuedMessage(conversationId, emitter, emitterDone, username, requestBaseUrl);
} else {
@ -995,7 +996,7 @@ public class ChatController {
// follow-up. Whoever puts a message in messageQueue means it
// just run it. Aligns with doOnComplete and the 4 other
// queue-launch sites in this controller.
if (hasQueuedInput(conversationId)) {
if (shouldDrainQueuedInput(conversationId, status)) {
startQueuedMessage(conversationId, emitter, emitterDone, username, requestBaseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
@ -1400,6 +1401,8 @@ public class ChatController {
private String message;
private String conversationId = "default";
private List<MessageContentPart> contentParts;
/** Exact approval selected by the UI; absent for legacy FIFO clients. */
private String pendingApprovalId;
/** true 表示断线重连,不发送新消息,只附着到已有的流 */
private Boolean reconnect;
/**
@ -1588,7 +1591,7 @@ public class ChatController {
} finally {
ChatStreamTracker.CompletionResult cr = streamTracker.completeAndConsumeIfLast(conversationId);
if (cr.allDone()) {
if (hasQueuedInput(conversationId)) {
if (shouldDrainQueuedInput(conversationId, persistStatus)) {
// 链式续跑queued stream 期间又排了新消息
startQueuedMessage(conversationId, emitter, emitterDone, requesterId, baseUrl);
} else {
@ -1634,7 +1637,7 @@ public class ChatController {
}
ChatStreamTracker.CompletionResult cr = streamTracker.completeAndConsumeIfLast(conversationId);
if (cr.allDone()) {
if (hasQueuedInput(conversationId)) {
if (shouldDrainQueuedInput(conversationId, "failed")) {
startQueuedMessage(conversationId, emitter, emitterDone, requesterId, baseUrl);
} else {
conversationService.updateStreamStatus(conversationId, "idle");
@ -1722,6 +1725,31 @@ public class ChatController {
return "[本次没有输出]";
}
private boolean shouldDrainQueuedInput(String conversationId, String persistStatus) {
return shouldDrainQueuedInput(
persistStatus,
hasQueuedInput(conversationId),
approvalService.findPendingByConversation(conversationId) != null);
}
private PendingApproval findRequestedPendingApproval(String conversationId, String pendingApprovalId) {
if (pendingApprovalId == null || pendingApprovalId.isBlank()) {
return approvalService.findPendingByConversation(conversationId);
}
return approvalService.getPending(pendingApprovalId)
.filter(pending -> conversationId.equals(pending.getConversationId()))
.filter(pending -> "pending".equals(pending.getStatus()))
.orElse(null);
}
static boolean shouldDrainQueuedInput(String persistStatus,
boolean hasQueuedInput,
boolean hasPendingApproval) {
return hasQueuedInput
&& !hasPendingApproval
&& !"awaiting_approval".equals(persistStatus);
}
static boolean isAssistantPersisted(MessageEntity savedAssistant) {
return savedAssistant != null;
}

View File

@ -9,6 +9,7 @@ import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.event.EventListener;
import org.springframework.context.event.ContextClosedEvent;
import org.springframework.stereotype.Service;
import vip.mate.exception.MateClawException;
import vip.mate.tool.mcp.event.McpConnectionLostEvent;
@ -26,6 +27,7 @@ import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;
/**
@ -60,9 +62,24 @@ public class McpServerService {
return t;
});
/** Set before bean destruction so transport exit callbacks cannot heal a shutting-down app. */
private final AtomicBoolean shuttingDown = new AtomicBoolean(false);
@EventListener
public void onContextClosed(ContextClosedEvent ignored) {
beginShutdown();
}
@PreDestroy
public void shutdownConnectExecutor() {
connectExecutor.shutdownNow();
beginShutdown();
}
private void beginShutdown() {
if (shuttingDown.compareAndSet(false, true)) {
log.info("MCP reconnect service stopping; new connect/reconnect requests are disabled");
connectExecutor.shutdownNow();
}
}
/**
@ -87,6 +104,11 @@ public class McpServerService {
*/
@EventListener
public void onConnectionLost(McpConnectionLostEvent event) {
if (shuttingDown.get()) {
log.debug("Ignoring MCP connection-lost event during application shutdown: serverId={}, reason={}",
event.serverId(), event.reason());
return;
}
Long serverId = event.serverId();
if (serverId == null) {
return;
@ -407,12 +429,18 @@ public class McpServerService {
* caller's request thread returns at once.
*/
private void connectAsync(McpServerEntity server) {
if (shuttingDown.get()) {
return;
}
updateStatus(server.getId(), "connecting", null, 0);
connectExecutor.submit(() -> connectSync(server));
}
/** Async counterpart of {@link #reconnectSync}. See {@link #connectAsync}. */
private void reconnectAsync(McpServerEntity server) {
if (shuttingDown.get()) {
return;
}
updateStatus(server.getId(), "connecting", null, 0);
connectExecutor.submit(() -> reconnectSync(server));
}

View File

@ -4,12 +4,14 @@ import com.alibaba.cloud.ai.graph.OverAllState;
import org.junit.jupiter.api.Test;
import org.springframework.ai.chat.messages.Message;
import org.springframework.ai.chat.messages.UserMessage;
import vip.mate.agent.GraphEventPublisher;
import vip.mate.agent.context.ConversationWindowManager;
import vip.mate.agent.graph.state.FinishReason;
import vip.mate.agent.graph.state.MateClawStateKeys;
import vip.mate.goal.config.GoalProperties;
import vip.mate.goal.model.GoalEntity;
import vip.mate.goal.model.GoalEvaluationResult;
import vip.mate.goal.model.GoalCriterion;
import vip.mate.goal.model.GoalResponse;
import vip.mate.goal.service.GoalEvaluationService;
import vip.mate.goal.service.GoalFollowupService;
@ -188,6 +190,31 @@ class GoalEvaluationNodeContinuationTest {
verify(f.followupService,never()).maybeBuildFollowup(any(),any());
}
@Test
void goalEvaluationEventCarriesStateSafeGoalSnapshot() throws Exception {
Fixture f = new Fixture();
GoalEntity persistent = f.goalService.getById(1L);
persistent.setPersistentExecution(true);
persistent.setStatus(vip.mate.goal.model.GoalStatus.ACTIVE);
GoalResponse response = new GoalResponse();
response.setId(1L);
response.setTitle("ship the feature");
response.setCriteria(List.of(new GoalCriterion("C1", "tests pass", false, "")));
when(f.goalService.toResponse(any())).thenReturn(response);
Map<String,Object> out = f.node().apply(f.state(FinishReason.NORMAL.getValue(),0,0));
@SuppressWarnings("unchecked")
List<GraphEventPublisher.GraphEvent> events =
(List<GraphEventPublisher.GraphEvent>) out.get(MateClawStateKeys.PENDING_EVENTS);
Object goalSnapshot = events.get(0).data().get("goal");
assertInstanceOf(Map.class, goalSnapshot);
Object criteria = ((Map<?, ?>) goalSnapshot).get("criteria");
assertInstanceOf(List.class, criteria);
assertInstanceOf(Map.class, ((List<?>) criteria).get(0));
}
// ===== Test fixture =====
private static final class Fixture {

View File

@ -71,6 +71,15 @@ class ChatControllerPersistStatusTest {
.isEqualTo("interrupted");
}
@Test
@DisplayName("queued input stays durable while the current turn awaits approval")
void awaitingApprovalDoesNotDrainQueuedInput() {
assertThat(ChatController.shouldDrainQueuedInput("awaiting_approval", true, true)).isFalse();
assertThat(ChatController.shouldDrainQueuedInput("completed", true, true)).isFalse();
assertThat(ChatController.shouldDrainQueuedInput("completed", true, false)).isTrue();
assertThat(ChatController.shouldDrainQueuedInput("completed", false, false)).isFalse();
}
@Test
@DisplayName("empty completed turns persist an explicit placeholder")
void emptyCompletedTurnUsesPlaceholder() {

View File

@ -24,6 +24,8 @@ import static org.mockito.Mockito.when;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import java.util.Optional;
@ExtendWith(MockitoExtension.class)
class ChatControllerWorkerReadOnlyTest {
@ -192,6 +194,49 @@ class ChatControllerWorkerReadOnlyTest {
}
}
@Test
void approvalCommandConsumesTheRequestedPendingInsteadOfTheOldest() {
when(authentication.getName()).thenReturn("alice");
when(conversationService.isUserMessageAllowed("multi-approval")).thenReturn(true);
var requested = org.mockito.Mockito.mock(vip.mate.approval.PendingApproval.class);
when(requested.getPendingId()).thenReturn("pending-newest");
when(requested.getConversationId()).thenReturn("multi-approval");
when(requested.getStatus()).thenReturn("pending");
when(approvalService.getPending("pending-newest")).thenReturn(Optional.of(requested));
when(approvalService.resolveAndConsume("pending-newest", "alice"))
.thenReturn(vip.mate.approval.ResolveOutcome.alreadyResolved("pending-newest"));
ChatController.ChatStreamRequest request = new ChatController.ChatStreamRequest();
request.setConversationId("multi-approval");
request.setMessage("/approve");
request.setPendingApprovalId("pending-newest");
controller.chatStream(request, 1L, authentication);
verify(approvalService).getPending("pending-newest");
verify(approvalService).resolveAndConsume("pending-newest", "alice");
verify(approvalService, never()).findPendingByConversation(any());
}
@Test
void approvalCommandRejectsPendingFromAnotherConversation() {
when(authentication.getName()).thenReturn("alice");
when(conversationService.isUserMessageAllowed("owned-conversation")).thenReturn(true);
var foreign = org.mockito.Mockito.mock(vip.mate.approval.PendingApproval.class);
when(foreign.getConversationId()).thenReturn("foreign-conversation");
when(approvalService.getPending("foreign-pending")).thenReturn(Optional.of(foreign));
ChatController.ChatStreamRequest request = new ChatController.ChatStreamRequest();
request.setConversationId("owned-conversation");
request.setMessage("/deny");
request.setPendingApprovalId("foreign-pending");
controller.chatStream(request, 1L, authentication);
verify(approvalService, never()).resolve(any(), any(), any());
verify(approvalService, never()).findPendingByConversation(any());
}
@Test
void reconnectCanAttachWhileAutonomousTurnOwnsReservation() {
when(authentication.getName()).thenReturn("alice");

View File

@ -7,6 +7,8 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.event.ContextClosedEvent;
import vip.mate.exception.MateClawException;
import vip.mate.tool.mcp.model.McpServerEntity;
import vip.mate.tool.mcp.model.McpToolDescriptor;
@ -51,6 +53,9 @@ class McpServerServiceListToolsTest {
@Mock
private McpClientManager mcpClientManager;
@Mock
private ApplicationEventPublisher eventPublisher;
@InjectMocks
private McpServerService service;
@ -135,4 +140,16 @@ class McpServerServiceListToolsTest {
// the wire payload but the Java value is preserved through the mapping.
assertTrue(result.get(0).description() == null);
}
@Test
@DisplayName("application shutdown ignores MCP process-exit reconnect events")
void shutdownDoesNotReconnectExitedStdioServer() {
service.onContextClosed(org.mockito.Mockito.mock(ContextClosedEvent.class));
service.onConnectionLost(new vip.mate.tool.mcp.event.McpConnectionLostEvent(
7L, "stdio-process-exited"));
verify(mcpServerMapper, never()).selectById(7L);
verify(mcpClientManager, never()).replace(org.mockito.ArgumentMatchers.any());
}
}

View File

@ -12,4 +12,15 @@ describe('buildChatStreamRequestBody', () => {
expect(body.agentId).toBe('2079862124134313986')
expect(JSON.stringify(body)).toContain('"agentId":"2079862124134313986"')
})
it('preserves the exact approval id when several approvals coexist', () => {
const body = buildChatStreamRequestBody('/deny', {
conversationId: 'conv-multi-approval',
agentId: '1000000001',
contentParts: [],
pendingApprovalId: 'pending-newest',
})
expect(body.pendingApprovalId).toBe('pending-newest')
})
})

View File

@ -191,6 +191,8 @@ export interface SendMessageOptions {
* row is inserted and `content` is ignored server-side.
*/
regenerate?: boolean
/** Exact approval selected by the user when several pendings coexist. */
pendingApprovalId?: string
}
export function buildChatStreamRequestBody(content: string, options: SendMessageOptions): Record<string, any> {
@ -210,6 +212,9 @@ export function buildChatStreamRequestBody(content: string, options: SendMessage
if (options.regenerate) {
body.regenerate = true
}
if (options.pendingApprovalId) {
body.pendingApprovalId = options.pendingApprovalId
}
return body
}

View File

@ -2032,7 +2032,7 @@ const currentPromptTokens = computed(() => {
})
// ============ ============
async function handleSendMessage(content: string) {
async function handleSendMessage(content: string, pendingApprovalId?: string) {
//
const isApprovalCommand = /^\/(approve|deny)$/i.test(content.trim())
@ -2076,6 +2076,7 @@ async function handleSendMessage(content: string) {
conversationId: currentConversationId.value,
agentId: selectedAgentId.value,
contentParts: [],
pendingApprovalId,
})
} catch (e: any) {
console.error('Approval stream failed:', e)
@ -2215,12 +2216,12 @@ function handleToggleThinking(message: import('@/types').Message, expanded: bool
// ============ ============
async function handleApprove(pendingId: string) {
if (!currentConversationId.value) return
await handleSendMessage('/approve')
await handleSendMessage('/approve', pendingId)
}
async function handleDeny(pendingId: string) {
if (!currentConversationId.value) return
await handleSendMessage('/deny')
await handleSendMessage('/deny', pendingId)
}
// Always-approve: create the matching grant first, then send /approve as usual.
@ -2248,7 +2249,7 @@ async function handleApproveAlways(
}
if (!scopeId) {
ElMessage.error('Cannot resolve scope id for always-approve')
await handleSendMessage('/approve')
await handleSendMessage('/approve', payload.pendingId)
return
}
@ -2274,7 +2275,7 @@ async function handleApproveAlways(
} catch (e: any) {
ElMessage.error(e?.message || 'Failed to create auto-approve rule')
}
await handleSendMessage('/approve')
await handleSendMessage('/approve', payload.pendingId)
}
//