mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-13 11:13:43 +08:00
fix(agent): stop stale ledger entries from suppressing repeated status queries
This commit is contained in:
parent
823efc0c36
commit
91c9565f1f
@ -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.
|
||||
*
|
||||
* <p>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<String> 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<StreamDelta> 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);
|
||||
|
||||
@ -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:
|
||||
* <ul>
|
||||
* <li><b>Meta-tools</b> (load_skill, enable_tool, progress_update,
|
||||
* skill helpers) — they either have their own ledger side-effects
|
||||
* or are the ledger itself.</li>
|
||||
* <li><b>Read-only / status-query tools</b> — querying live state is
|
||||
* not a task step that must not be repeated. Recording it as DONE
|
||||
* (with a frozen result excerpt in the note) pushes the model to
|
||||
* answer follow-up questions from stale output instead of
|
||||
* re-checking, because the snapshot instructs "已完成的步骤不要
|
||||
* 重复执行".</li>
|
||||
* </ul>
|
||||
*/
|
||||
private static final Set<String> 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<ToolResponseMessage.ToolResponse> responses) {
|
||||
void autoRecordToolCalls(String conversationId,
|
||||
List<ToolResponseMessage.ToolResponse> responses) {
|
||||
if (progressLedgerService == null || conversationId == null
|
||||
|| conversationId.isBlank() || responses == null || responses.isEmpty()) {
|
||||
return;
|
||||
|
||||
@ -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.
|
||||
*
|
||||
* <p>Auto-recorded entries are a safety net against context trimming
|
||||
* <b>within</b> 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.
|
||||
*
|
||||
* <p>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
|
||||
|
||||
@ -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<String, String> 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<String> 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());
|
||||
}
|
||||
}
|
||||
@ -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.
|
||||
*
|
||||
* <p>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<String, String> 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());
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user