diff --git a/mateclaw-server/src/main/java/vip/mate/agent/AgentService.java b/mateclaw-server/src/main/java/vip/mate/agent/AgentService.java index e1c3c4f8..fdada970 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/AgentService.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/AgentService.java @@ -13,6 +13,7 @@ import vip.mate.agent.context.ChatOrigin; import vip.mate.agent.context.ChatOriginHolder; import vip.mate.agent.event.AgentLifecycleEvent; import vip.mate.agent.model.AgentEntity; +import vip.mate.agent.progress.ProgressLedgerService; import vip.mate.agent.repository.AgentMapper; import vip.mate.exception.MateClawException; import vip.mate.llm.chatmodel.ThinkingLevelHolder; @@ -69,6 +70,14 @@ public class AgentService { @Autowired(required = false) private vip.mate.agent.runtime.RunningConversationRegistry runningConversationRegistry; + /** + * Optional — clears leftover auto-recorded ledger entries when a new + * user turn starts. Field-injected so existing test constructors of + * {@code AgentService} don't need to supply it. + */ + @Autowired(required = false) + private ProgressLedgerService progressLedgerService; + /** * Runtime Agent instance cache. Keyed first by agentId, then by a model * key, so a conversation that pins a non-default model gets its own graph @@ -254,6 +263,32 @@ public class AgentService { // ==================== 运行时入口 ==================== + /** + * New-user-turn housekeeping: drop auto-recorded ledger entries left + * over from the previous turn. They mark past tool calls as DONE, and + * the ledger snapshot's "已完成的步骤不要重复执行" instruction would + * otherwise stop the agent from re-running status-query tools when the + * user repeats a question that needs fresh data. + * + *

Only the fresh-turn entries ({@code chat} / {@code chatStream} / + * {@code chatStructuredStream} / {@code execute}) call this. The + * approval-replay entries ({@code chatWithReplay*}) resume the SAME + * logical turn after a tool approval and must keep the safety net for + * work already done before the pause. + */ + private void clearAutoRecordedForNewTurn(String conversationId) { + if (progressLedgerService == null || conversationId == null || conversationId.isBlank()) { + return; + } + try { + progressLedgerService.clearAutoRecorded(conversationId); + } catch (Exception e) { + // Ledger housekeeping must never block the chat itself. + log.warn("Failed to clear auto-recorded ledger entries for {}: {}", + conversationId, e.getMessage()); + } + } + public String chat(Long agentId, String message, String conversationId) { return chat(agentId, message, conversationId, ChatOrigin.EMPTY); } @@ -264,6 +299,7 @@ public class AgentService { * down to {@code @Tool} methods via Spring AI {@link org.springframework.ai.chat.model.ToolContext}. */ public String chat(Long agentId, String message, String conversationId, ChatOrigin origin) { + clearAutoRecordedForNewTurn(conversationId); memoryRecallTracker.trackRecalls(agentId, message); BaseAgent agent = getOrBuildAgentForConversation(agentId, conversationId); ChatOriginHolder.set(origin != null ? origin : ChatOrigin.EMPTY); @@ -300,6 +336,7 @@ public class AgentService { } public Flux chatStream(Long agentId, String message, String conversationId, ChatOrigin origin) { + clearAutoRecordedForNewTurn(conversationId); memoryRecallTracker.trackRecalls(agentId, message); BaseAgent agent = getOrBuildAgentForConversation(agentId, conversationId); // Capture the origin into a request-scoped holder; cleared on Flux @@ -336,6 +373,7 @@ public class AgentService { public Flux chatStructuredStream(Long agentId, String message, String conversationId, String requesterId, String thinkingLevel, ChatOrigin origin) { + clearAutoRecordedForNewTurn(conversationId); memoryRecallTracker.trackRecalls(agentId, message); BaseAgent agent = getOrBuildAgentForConversation(agentId, conversationId); @@ -382,6 +420,7 @@ public class AgentService { } public String execute(Long agentId, String goal, String conversationId, ChatOrigin origin) { + clearAutoRecordedForNewTurn(conversationId); memoryRecallTracker.trackRecalls(agentId, goal); BaseAgent agent = getOrBuildAgentForConversation(agentId, conversationId); ChatOriginHolder.set(origin != null ? origin : ChatOrigin.EMPTY); 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 886aca3a..0041e648 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 @@ -56,12 +56,28 @@ public class ActionNode implements NodeAction { /** * Tools whose results should NOT be auto-recorded into the ledger. - * Meta-tools (load_skill, enable_tool, progress_update) either have - * their own ledger side-effects or are the ledger itself. + * Two groups: + *

*/ private static final Set AUTO_RECORD_SKIP = Set.of( LOAD_SKILL_TOOL, ENABLE_TOOL, PROGRESS_UPDATE_TOOL, - "listAvailableSkills", "readSkillFile", "runSkillScript" + "listAvailableSkills", "readSkillFile", "runSkillScript", + // read-only / status-query tools + "read_file", "web_search", + "extract_document_text", "extract_pdf_text", "extract_docx_text", + "detect_file_type", + "getCurrentDateTime", "getCurrentDate", "getCurrentTime", + "listSubagents" ); private final ToolExecutionExecutor executor; @@ -256,8 +272,8 @@ public class ActionNode implements NodeAction { * avoid collisions between servers that expose tools with the same slug. * The display label uses the simplified slug for readability. */ - private void autoRecordToolCalls(String conversationId, - List responses) { + void autoRecordToolCalls(String conversationId, + List responses) { if (progressLedgerService == null || conversationId == null || conversationId.isBlank() || responses == null || responses.isEmpty()) { return; diff --git a/mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedgerService.java b/mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedgerService.java index 272027b6..5d3e9694 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedgerService.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/progress/ProgressLedgerService.java @@ -202,6 +202,39 @@ public class ProgressLedgerService { } } + /** + * Remove every auto-recorded entry ({@code auto_} key prefix) from the + * conversation's ledger. Called at the start of each new user turn. + * + *

Auto-recorded entries are a safety net against context trimming + * within one turn's tool loop. Letting them survive into the next + * user turn is harmful: the snapshot renders them as DONE alongside the + * "已完成的步骤不要重复执行" instruction, which stops the agent from + * re-running read-only / status-query tools when the user repeats a + * question that needs fresh data (e.g. "看下会议室有没有人"), and the + * frozen 120-char result note tempts it to answer from stale output. + * + *

LLM-authored regular entries and pinned skill constraints are + * untouched — multi-turn task tracking keeps working. + */ + public void clearAutoRecorded(String conversationId) { + if (conversationId == null || conversationId.isBlank()) { + return; + } + ReentrantLock lock = upsertLocks.computeIfAbsent(conversationId, k -> new ReentrantLock()); + lock.lock(); + try { + LedgerWrapper wrapper = loadWrapper(conversationId); + boolean removed = wrapper.entries.keySet().removeIf( + k -> k != null && k.startsWith(ProgressLedger.AUTO_RECORDED_PREFIX)); + if (removed) { + persistWrapper(conversationId, wrapper); + } + } finally { + lock.unlock(); + } + } + /** * Auto-record a completed tool call as a ledger entry (B5). Uses the * {@link ProgressLedger#AUTO_RECORDED_PREFIX} on the key so the renderer diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/node/ActionNodeAutoRecordSkipTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/node/ActionNodeAutoRecordSkipTest.java new file mode 100644 index 00000000..f9d6fb5b --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/node/ActionNodeAutoRecordSkipTest.java @@ -0,0 +1,117 @@ +package vip.mate.agent.graph.node; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.ai.chat.messages.ToolResponseMessage; +import vip.mate.agent.progress.ProgressLedger; +import vip.mate.agent.progress.ProgressLedgerService; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Pins the auto-record skip list in {@link ActionNode#autoRecordToolCalls}: + * read-only / status-query tools must NOT be written to the ledger as DONE + * steps. A DONE marker plus the snapshot's "已完成的步骤不要重复执行" + * instruction stops the agent from re-querying live state when the user + * repeats a question, so only mutating/task-shaped tool calls belong in + * the auto-recorded section. + */ +class ActionNodeAutoRecordSkipTest { + + /** In-memory ledger double — same pattern as the service-level tests. */ + private static final class InMemoryProgressLedgerService extends ProgressLedgerService { + private final Map store = new ConcurrentHashMap<>(); + + InMemoryProgressLedgerService() { + super(null, new ObjectMapper().registerModule(new JavaTimeModule())); + } + + @Override + protected String loadLedgerJson(String conversationId) { + return store.get(conversationId); + } + + @Override + protected void saveLedgerJson(String conversationId, String json) { + store.put(conversationId, json); + } + } + + private static ToolResponseMessage.ToolResponse resp(String name, String data) { + return new ToolResponseMessage.ToolResponse("id-" + name, name, data); + } + + private static ActionNode nodeWith(ProgressLedgerService ledgerService) { + ActionNode node = new ActionNode(null); + node.setProgressLedgerService(ledgerService); + return node; + } + + @Test + @DisplayName("read-only / status-query tools are never auto-recorded") + void readOnlyToolsSkipped() { + InMemoryProgressLedgerService ledger = new InMemoryProgressLedgerService(); + ActionNode node = nodeWith(ledger); + String conv = "conv-skip-1"; + + node.autoRecordToolCalls(conv, List.of( + resp("read_file", "{\"content\":\"...\"}"), + resp("web_search", "results..."), + resp("extract_document_text", "text..."), + resp("extract_pdf_text", "text..."), + resp("extract_docx_text", "text..."), + resp("detect_file_type", "application/pdf"), + resp("getCurrentDateTime", "2026-07-31T10:33:00"), + resp("getCurrentDate", "2026-07-31"), + resp("getCurrentTime", "10:33"), + resp("listSubagents", "[]"))); + + assertTrue(ledger.load(conv).isEmpty(), + "no read-only tool call may produce a ledger entry"); + } + + @Test + @DisplayName("mutating tools are still auto-recorded alongside skipped read-only ones") + void mutatingToolsStillRecorded() { + InMemoryProgressLedgerService ledger = new InMemoryProgressLedgerService(); + ActionNode node = nodeWith(ledger); + String conv = "conv-skip-2"; + + node.autoRecordToolCalls(conv, List.of( + resp("read_file", "{\"content\":\"...\"}"), + resp("write_file", "written"), + resp("mcp_home_check_room_a1b2c3", "0人"))); + + ProgressLedger loaded = ledger.load(conv); + Set keys = loaded.asMap().keySet(); + assertEquals(2, keys.size(), "exactly the non-read-only calls are recorded; got " + keys); + assertTrue(keys.contains(ProgressLedger.AUTO_RECORDED_PREFIX + "write_file")); + assertTrue(keys.contains(ProgressLedger.AUTO_RECORDED_PREFIX + "mcp_home_check_room_a1b2c3")); + } + + @Test + @DisplayName("meta-tools (load_skill / enable_tool / progress_update / skill helpers) stay skipped") + void metaToolsStillSkipped() { + InMemoryProgressLedgerService ledger = new InMemoryProgressLedgerService(); + ActionNode node = nodeWith(ledger); + String conv = "conv-skip-3"; + + node.autoRecordToolCalls(conv, List.of( + resp("load_skill", "loaded"), + resp("enable_tool", "enabled"), + resp("progress_update", "ok"), + resp("listAvailableSkills", "[]"), + resp("readSkillFile", "..."), + resp("runSkillScript", "..."))); + + assertTrue(ledger.load(conv).isEmpty()); + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/agent/progress/ProgressLedgerClearAutoRecordedTest.java b/mateclaw-server/src/test/java/vip/mate/agent/progress/ProgressLedgerClearAutoRecordedTest.java new file mode 100644 index 00000000..b04b31f3 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/progress/ProgressLedgerClearAutoRecordedTest.java @@ -0,0 +1,126 @@ +package vip.mate.agent.progress; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import vip.mate.agent.progress.ProgressLedgerService.AutoRecordEntry; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +/** + * Pins {@link ProgressLedgerService#clearAutoRecorded} — the new-user-turn + * housekeeping that drops auto-recorded ({@code auto_}-prefixed) entries. + * + *

Why it matters: an auto-recorded DONE entry that survives into the + * next user turn is rendered in the snapshot together with the + * "已完成的步骤不要重复执行" instruction, which stops the agent from + * re-running a status-query tool when the user repeats a question that + * needs fresh data — it answers from the frozen 120-char note instead. + * Regular (LLM-authored) entries and pinned skill constraints must survive + * the clear so multi-turn task tracking keeps working. + */ +class ProgressLedgerClearAutoRecordedTest { + + /** In-memory double — same pattern as the concurrency test. */ + private static final class InMemoryProgressLedgerService extends ProgressLedgerService { + private final Map store = new ConcurrentHashMap<>(); + final AtomicInteger saveCount = new AtomicInteger(); + + InMemoryProgressLedgerService() { + super(null, new ObjectMapper().registerModule(new JavaTimeModule())); + } + + @Override + protected String loadLedgerJson(String conversationId) { + return store.get(conversationId); + } + + @Override + protected void saveLedgerJson(String conversationId, String json) { + saveCount.incrementAndGet(); + store.put(conversationId, json); + } + } + + @Test + @DisplayName("clearAutoRecorded drops auto_ entries but keeps regular and pinned entries") + void clearsOnlyAutoEntries() { + InMemoryProgressLedgerService service = new InMemoryProgressLedgerService(); + String conv = "conv-clear-1"; + + service.upsert(conv, "step_report", "Write report", ProgressStatus.PENDING, null); + service.upsertPinned(conv, "pin_pdf_0", "Always render via template", null); + service.upsertAutoRecordedBatch(conv, List.of( + new AutoRecordEntry("mcp_home_check_room_a1b2c3", "check_room", "0人, 电池0%"), + new AutoRecordEntry("write_file", "write_file", "ok"))); + assertEquals(4, service.load(conv).size()); + + service.clearAutoRecorded(conv); + + ProgressLedger ledger = service.load(conv); + assertEquals(2, ledger.size(), "only regular + pinned should survive"); + assertTrue(ledger.asMap().containsKey("step_report")); + assertTrue(ledger.pinnedEntries().containsKey("pin_pdf_0")); + assertFalse(ledger.asMap().keySet().stream() + .anyMatch(k -> k.startsWith(ProgressLedger.AUTO_RECORDED_PREFIX)), + "no auto_ entry may survive a clear"); + } + + @Test + @DisplayName("clearAutoRecorded is a no-op (no save) when the ledger has no auto_ entries") + void noOpWithoutAutoEntries() { + InMemoryProgressLedgerService service = new InMemoryProgressLedgerService(); + String conv = "conv-clear-2"; + + service.upsert(conv, "step_x", "Step X", ProgressStatus.DONE, null); + int savesBefore = service.saveCount.get(); + + service.clearAutoRecorded(conv); + + assertEquals(savesBefore, service.saveCount.get(), + "clearing an auto-free ledger must not write to the DB"); + assertEquals(1, service.load(conv).size()); + } + + @Test + @DisplayName("clearAutoRecorded tolerates null/blank conversationId and empty ledgers") + void toleratesMissingInput() { + InMemoryProgressLedgerService service = new InMemoryProgressLedgerService(); + assertDoesNotThrow(() -> service.clearAutoRecorded(null)); + assertDoesNotThrow(() -> service.clearAutoRecorded("")); + assertDoesNotThrow(() -> service.clearAutoRecorded("conv-never-seen")); + assertNull(((InMemoryProgressLedgerService) service).store.get("conv-never-seen"), + "clearing an unknown conversation must not create a row"); + } + + @Test + @DisplayName("auto entries re-recorded after a clear behave normally (fresh note, not the old one)") + void reRecordAfterClearUsesFreshResult() { + InMemoryProgressLedgerService service = new InMemoryProgressLedgerService(); + String conv = "conv-clear-3"; + String tool = "mcp_home_check_room_a1b2c3"; + + service.upsertAutoRecordedBatch(conv, List.of( + new AutoRecordEntry(tool, "check_room", "0人, 电池0%"))); + service.clearAutoRecorded(conv); + // Next turn: the same tool runs again and must record the NEW result — + // without the clear, upsertAutoRecordedBatch skips existing keys and + // the note stays frozen at the first call's output. + service.upsertAutoRecordedBatch(conv, List.of( + new AutoRecordEntry(tool, "check_room", "2人, 电池87%"))); + + ProgressEntry entry = service.load(conv).asMap() + .get(ProgressLedger.AUTO_RECORDED_PREFIX + tool); + assertEquals("2人, 电池87%", entry.getNote()); + } +}