diff --git a/mateclaw-server/src/main/java/vip/mate/agent/BaseAgent.java b/mateclaw-server/src/main/java/vip/mate/agent/BaseAgent.java
index 78d87e93..d36a6810 100644
--- a/mateclaw-server/src/main/java/vip/mate/agent/BaseAgent.java
+++ b/mateclaw-server/src/main/java/vip/mate/agent/BaseAgent.java
@@ -357,10 +357,28 @@ public abstract class BaseAgent {
}
/**
- * Drop leading {@link ToolResponseMessage}s whose response ids cannot be
- * matched against any {@link AssistantMessage} tool_call id in the list.
- * Leaves any leading {@link SystemMessage}s (boundary rows, system prompts)
- * intact and continues scanning past them.
+ * Drop leading {@link ToolResponseMessage}s whose owning
+ * {@link AssistantMessage} sits before them in this list. Provider
+ * validity is order-sensitive: a tool response must follow the assistant
+ * that issued the tool_call_id; an unrelated later AssistantMessage that
+ * happens to carry the same id does not redeem an earlier orphan.
+ *
+ * Algorithm: forward scan with a {@code seenIssuedIds} set. Leading
+ * {@link SystemMessage}s (boundary rows, system prompts) pass through
+ * untouched but contribute no ids. The first {@link AssistantMessage} or
+ * {@link UserMessage} we hit stops the repair walk — by that point we're
+ * out of head-orphan territory. Every {@link ToolResponseMessage} we
+ * encounter before that stop is checked against {@code seenIssuedIds};
+ * if every response id is unseen, the message is dropped and the scan
+ * re-examines the new head. A response whose ids are all in the seen
+ * set (e.g. {@code [system, assistant(X), toolResponse(X), ...]} when
+ * the assistant fell at index 1 of the slice) is left in place.
+ *
+ *
Mixed responses (some ids matched, some not) inside a single
+ * leading {@code ToolResponseMessage} are dropped wholesale rather than
+ * surgically rewritten — the provider would reject partially-broken
+ * sequences anyway, and the mixed case implies an upstream invariant
+ * violation that surfaces in logs.
*
*
Package-private + static so unit tests can drive it without standing
* up a full BaseAgent subclass.
@@ -368,17 +386,11 @@ public abstract class BaseAgent {
static int stripHeadOrphanToolResponses(List messages, String agentName) {
if (messages.isEmpty()) return 0;
- // Collect every tool_call id issued by any AssistantMessage in scope.
- Set issuedIds = new HashSet<>();
- for (Message m : messages) {
- if (m instanceof AssistantMessage am && am.getToolCalls() != null) {
- for (AssistantMessage.ToolCall tc : am.getToolCalls()) {
- if (tc.id() != null && !tc.id().isEmpty()) {
- issuedIds.add(tc.id());
- }
- }
- }
- }
+ // Built up as we walk; only assistants we've already passed count
+ // toward "preceding". An assistant that sits behind a head orphan is
+ // irrelevant: provider order-validity asks "was this tool_call id
+ // issued BEFORE this response?", not "anywhere in the prompt".
+ Set seenIssuedIds = new HashSet<>();
int dropped = 0;
int i = 0;
@@ -390,25 +402,44 @@ public abstract class BaseAgent {
i++;
continue;
}
+ if (m instanceof AssistantMessage am) {
+ // Reached a preceding assistant — head danger is over. The
+ // tool_call ids it issued are valid for any tool responses
+ // that follow, but we stop the repair walk here either way.
+ if (am.getToolCalls() != null) {
+ for (AssistantMessage.ToolCall tc : am.getToolCalls()) {
+ if (tc.id() != null && !tc.id().isEmpty()) {
+ seenIssuedIds.add(tc.id());
+ }
+ }
+ }
+ break;
+ }
if (m instanceof ToolResponseMessage trm) {
- boolean allOrphan = trm.getResponses().stream()
+ // Every response id must have been issued by a preceding
+ // assistant we already walked through. If any single id is
+ // missing from seenIssuedIds, the message is invalid in
+ // place. Empty / null ids don't count for or against.
+ boolean anyUnmatched = trm.getResponses().stream()
.map(ToolResponseMessage.ToolResponse::id)
.filter(id -> id != null && !id.isEmpty())
- .allMatch(id -> !issuedIds.contains(id));
- if (allOrphan) {
+ .anyMatch(id -> !seenIssuedIds.contains(id));
+ if (anyUnmatched) {
messages.remove(i);
dropped++;
continue; // re-examine the new messages[i]
}
+ // All ids match a preceding assistant — keep, and stop the
+ // repair walk. Anything past here is well-formed by
+ // construction (provider validates each subsequent pair as
+ // we go).
+ break;
}
- // First non-orphan, non-system message — stop. Deeper orphans are
- // upstream bugs (every other call path keeps pairs together);
- // dropping aggressively from here on would mask those instead of
- // surfacing them in logs.
+ // UserMessage (or anything else) — past the head danger. Stop.
break;
}
if (dropped > 0) {
- log.info("[{}] Stripped {} leading orphan ToolResponseMessage(s) — owning AssistantMessage was outside the recent window",
+ log.info("[{}] Stripped {} leading orphan ToolResponseMessage(s) — no preceding AssistantMessage in scope",
agentName, dropped);
}
return dropped;
diff --git a/mateclaw-server/src/test/java/vip/mate/agent/BaseAgentHeadOrphanRepairTest.java b/mateclaw-server/src/test/java/vip/mate/agent/BaseAgentHeadOrphanRepairTest.java
index 13400023..3fad6503 100644
--- a/mateclaw-server/src/test/java/vip/mate/agent/BaseAgentHeadOrphanRepairTest.java
+++ b/mateclaw-server/src/test/java/vip/mate/agent/BaseAgentHeadOrphanRepairTest.java
@@ -110,13 +110,38 @@ class BaseAgentHeadOrphanRepairTest {
}
@Test
- void partialOrphanIsKeptToSurfaceUpstreamBug() {
- // A ToolResponseMessage with two responses — one orphan, one matched
- // in scope. Real pipelines should never produce this (each
- // ToolResponseMessage closes ONE assistant turn) but if it happens,
- // dropping the whole message would also lose the matched response.
- // Stop at the first non-orphan and let the upstream invariant
- // violation surface in logs.
+ void laterAssistantWithSameIdDoesNotRedeemHeadOrphan() {
+ // The classic order-sensitivity trap: a ToolResponseMessage sits at
+ // the head, and a LATER AssistantMessage happens to carry the same
+ // tool_call_id. The provider's contract is "tool_call must precede
+ // tool_response", not "tool_call exists somewhere in the prompt".
+ // The leading response is therefore still orphan and must be dropped.
+ List messages = new ArrayList<>(List.of(
+ new SystemMessage("[boundary]"),
+ toolResponse("call-X"),
+ new UserMessage("hi"),
+ assistantWithToolCalls("call-X"), // same id, but AFTER the response
+ toolResponse("call-X")
+ ));
+
+ int dropped = BaseAgent.stripHeadOrphanToolResponses(messages, "test");
+
+ assertEquals(1, dropped,
+ "the leading response is orphan regardless of whether a later assistant "
+ + "happens to carry the same id — provider validity is order-sensitive");
+ assertInstanceOf(SystemMessage.class, messages.get(0));
+ assertInstanceOf(UserMessage.class, messages.get(1),
+ "the orphan that sat between the boundary and the user turn is gone");
+ }
+
+ @Test
+ void partialOrphanInLeadingResponseIsDropped() {
+ // A ToolResponseMessage with two responses — one whose id has no
+ // preceding assistant, one whose id has none either (since we
+ // haven't walked any assistants yet). Provider order-validity
+ // doesn't allow partial pairs; dropping wholesale is the safer
+ // call. We lose matched-response content but never emit a request
+ // the provider would 400.
ToolResponseMessage mixed = ToolResponseMessage.builder().responses(List.of(
new ToolResponseMessage.ToolResponse("call-orphan", "tool_x", "x"),
new ToolResponseMessage.ToolResponse("call-known", "tool_y", "y")
@@ -129,10 +154,11 @@ class BaseAgentHeadOrphanRepairTest {
int dropped = BaseAgent.stripHeadOrphanToolResponses(messages, "test");
- assertEquals(0, dropped,
- "mixed orphan/matched responses in one message are NOT dropped — "
- + "the partial-match case is an upstream bug we want to see in logs");
- assertSame(mixed, messages.getFirst());
+ assertEquals(1, dropped,
+ "no preceding assistant has been walked yet, so even a partially-matched "
+ + "leading response is dropped wholesale");
+ assertInstanceOf(AssistantMessage.class, messages.getFirst(),
+ "the mixed head is gone; the assistant that would have owned call-known is now first");
}
@Test