diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/MessageNormalizer.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/MessageNormalizer.java new file mode 100644 index 00000000..8a4182af --- /dev/null +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/MessageNormalizer.java @@ -0,0 +1,165 @@ +package vip.mate.agent.graph; + +import org.springframework.ai.chat.messages.Message; +import org.springframework.ai.chat.messages.SystemMessage; +import org.springframework.ai.chat.prompt.Prompt; + +import java.util.ArrayList; +import java.util.List; + +/** + * Pre-egress message-list normalizer. + * + *

Some OpenAI-compatible providers (notably LM Studio's built-in server, + * and certain strict-mode vLLM / SGLang deployments) enforce that exactly + * one {@link SystemMessage} must appear at index 0 of the messages array. + * Multiple consecutive SystemMessages, or any SystemMessage following a + * user / assistant / tool message, returns {@code 400 BAD_REQUEST: + * "System message must be at the beginning."}. + * + *

Permissive providers (OpenAI, DashScope, Ollama, DeepSeek, Kimi, Doubao, + * GLM) accept the relaxed shape, so the runtime historically composed + * prompts with multiple SystemMessages sprinkled through the non-history + * prefix (main system prompt + skill catalog + progress-ledger snapshot, + * each as its own SystemMessage). To stay portable across both strict and + * permissive backends, this normalizer collects every SystemMessage found + * anywhere in the input list, concatenates their text with a blank-line + * separator, and emits the result as a single SystemMessage at index 0. + * The relative order of non-system messages (user / assistant / + * tool_response) is preserved verbatim so {@code tool_call_id} pairings + * are unaffected. + * + *

Blank / whitespace-only SystemMessages are dropped from the merge. If + * every SystemMessage in the input is blank, the result is the same list + * with all SystemMessages removed (no synthetic empty SystemMessage is + * emitted). If the input contains zero SystemMessages, the input list + * reference is returned unchanged. + * + *

The transformation is semantically equivalent on permissive providers + * — the merged SystemMessage produces the same token sequence the model + * would have seen across N separate SystemMessages — and converts the + * strict-provider 400 into a success. It is also safe for non-OpenAI + * protocols: the Spring AI Anthropic and Vertex / Gemini adapters already + * extract SystemMessages out of the messages list into a top-level + * {@code system} / {@code systemInstruction} request field, so they receive + * an identical outbound payload whether handed one merged SystemMessage + * or several. + * + *

A kill switch is exposed via the JVM system property + * {@code mateclaw.llm.message-normalizer.enabled=false}, which makes + * {@link #normalize} a no-op for emergency rollback without code changes. + */ +public final class MessageNormalizer { + + /** Separator inserted between merged SystemMessage segments. */ + static final String SEPARATOR = "\n\n"; + + /** + * Kill-switch property name. Set to {@code false} (case-insensitive) on + * the JVM command line to disable normalization without a code change. + */ + public static final String ENABLED_PROPERTY = "mateclaw.llm.message-normalizer.enabled"; + + private static volatile boolean enabled = !"false".equalsIgnoreCase( + System.getProperty(ENABLED_PROPERTY, "true")); + + private MessageNormalizer() { + } + + /** Read the current kill-switch state. */ + public static boolean isEnabled() { + return enabled; + } + + /** + * Override the kill-switch at runtime (primarily for tests). Production + * code should not need to call this — set the JVM property at startup + * instead. + */ + public static void setEnabledForTesting(boolean value) { + enabled = value; + } + + /** + * Return a copy of {@code prompt} with every SystemMessage merged into a + * single SystemMessage at index 0. Returns the input prompt reference + * unchanged when no normalization is necessary (kill switch off, zero + * SystemMessages, or already a single non-blank SystemMessage at index 0). + */ + public static Prompt normalize(Prompt prompt) { + if (prompt == null || !enabled) { + return prompt; + } + List in = prompt.getInstructions(); + List out = normalize(in); + if (out == in) { + return prompt; + } + return new Prompt(out, prompt.getOptions()); + } + + /** + * List-level normalization, used by {@link #normalize(Prompt)} and by + * unit tests that want to assert on the raw message shape without + * constructing a {@link Prompt}. Returns the input list reference + * unchanged when no normalization is necessary. + */ + public static List normalize(List messages) { + if (!enabled || messages == null || messages.isEmpty()) { + return messages; + } + + int systemCount = 0; + int firstSystemIdx = -1; + for (int i = 0; i < messages.size(); i++) { + if (messages.get(i) instanceof SystemMessage) { + if (firstSystemIdx < 0) firstSystemIdx = i; + systemCount++; + } + } + + // Fast-path 1: no SystemMessages — nothing to do. + if (systemCount == 0) { + return messages; + } + // Fast-path 2: exactly one SystemMessage and it sits at index 0 with + // non-blank text. Already canonical — skip the rebuild. + if (systemCount == 1 && firstSystemIdx == 0) { + SystemMessage sm = (SystemMessage) messages.get(0); + String text = sm.getText(); + if (text != null && !text.isBlank()) { + return messages; + } + // Single blank SystemMessage at [0] — fall through to the rebuild, + // which will drop it. + } + + StringBuilder merged = new StringBuilder(); + List rest = new ArrayList<>(messages.size()); + for (Message m : messages) { + if (m instanceof SystemMessage sm) { + String text = sm.getText(); + if (text == null || text.isBlank()) { + continue; + } + if (merged.length() > 0) { + merged.append(SEPARATOR); + } + merged.append(text); + } else { + rest.add(m); + } + } + + if (merged.length() == 0) { + // Every SystemMessage in the input was blank — return just the + // non-system tail. No synthetic empty SystemMessage. + return rest; + } + + List out = new ArrayList<>(rest.size() + 1); + out.add(new SystemMessage(merged.toString())); + out.addAll(rest); + return out; + } +} diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/NodeStreamingChatHelper.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/NodeStreamingChatHelper.java index 988da757..b19a2da0 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/NodeStreamingChatHelper.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/NodeStreamingChatHelper.java @@ -716,11 +716,25 @@ public class NodeStreamingChatHelper { private StreamResult doStreamCall(ChatModel chatModel, Prompt prompt, String conversationId, String phase, boolean broadcast, int attempt) { + // Collapse every SystemMessage in the prompt into a single SystemMessage + // at index 0. Some OpenAI-compatible providers (LM Studio's built-in + // server, certain strict vLLM / SGLang deployments) reject 400 + // "System message must be at the beginning" when SystemMessages appear + // after user / assistant / tool messages — the runtime composes the + // non-history prefix from several SystemMessage segments (main prompt, + // skill catalog, progress-ledger snapshot) and some of them land mid- + // list. Permissive providers see an equivalent token sequence either + // way; non-OpenAI protocols (Anthropic, Vertex) extract the merged + // system into their top-level system field exactly as before. + // Preserves the input's options reference so downstream relay logic + // (options.user = relay token) keeps working. + Prompt outbound = MessageNormalizer.normalize(prompt); + // PR-2 L4 (RFC-049 §2.4.2): normalize as a pre-egress step (not only on retry). // Strip reasoning_content from prior-turn AssistantMessages (i <= lastUserIdx), // preserving in-turn thinking (i > lastUserIdx) so DeepSeek's contract holds. // The returned Prompt shares `options` by reference with the input prompt. - Prompt outbound = stripThinkingFromPrompt(prompt); + outbound = stripThinkingFromPrompt(outbound); // RFC-049 follow-up (2026-04-27): trim trailing AssistantMessage from the // outbound prompt. Triggered in practice by the summarizing→reasoning diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/MessageNormalizerTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/MessageNormalizerTest.java new file mode 100644 index 00000000..6af60b54 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/MessageNormalizerTest.java @@ -0,0 +1,267 @@ +package vip.mate.agent.graph; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.ai.chat.messages.AssistantMessage; +import org.springframework.ai.chat.messages.Message; +import org.springframework.ai.chat.messages.SystemMessage; +import org.springframework.ai.chat.messages.ToolResponseMessage; +import org.springframework.ai.chat.messages.UserMessage; +import org.springframework.ai.chat.prompt.Prompt; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Contract tests for {@link MessageNormalizer}. + * + *

Pins the invariants that make the normalizer safe to apply unconditionally + * before every LLM call: + *

+ */ +class MessageNormalizerTest { + + @BeforeEach + void enableNormalizer() { + MessageNormalizer.setEnabledForTesting(true); + } + + @AfterEach + void resetNormalizer() { + MessageNormalizer.setEnabledForTesting(true); + } + + // ---------- Fast paths ---------- + + @Test + @DisplayName("null and empty inputs pass through unchanged") + void nullAndEmpty_passThrough() { + assertThat(MessageNormalizer.normalize((Prompt) null)).isNull(); + assertThat(MessageNormalizer.normalize((List) null)).isNull(); + List empty = List.of(); + assertThat(MessageNormalizer.normalize(empty)).isSameAs(empty); + } + + @Test + @DisplayName("no SystemMessage at all → input list returned by reference") + void noSystem_passThrough() { + List in = List.of( + new UserMessage("hello"), + AssistantMessage.builder().content("hi").build(), + new UserMessage("follow-up") + ); + assertThat(MessageNormalizer.normalize(in)).isSameAs(in); + } + + @Test + @DisplayName("single non-blank SystemMessage at index 0 → input returned by reference") + void canonicalShape_passThrough() { + List in = List.of( + new SystemMessage("you are a helpful assistant"), + new UserMessage("hi") + ); + assertThat(MessageNormalizer.normalize(in)).isSameAs(in); + } + + // ---------- The bug case: SystemMessage after UserMessage ---------- + + @Test + @DisplayName("SystemMessage at tail (ReasoningNode ledger-snapshot pattern) → merged to head") + void systemAtTail_movedToHead() { + // Mirrors the exact shape ReasoningNode produces today: + // [system(main), system(skillCatalog), user(runtime), user(wiki), + // system(ledger snapshot), system(stale reminder), + // ...history user/assistant messages...] + List in = new ArrayList<>(List.of( + new SystemMessage("MAIN_PROMPT"), + new SystemMessage("SKILL_CATALOG"), + new UserMessage("RUNTIME_CTX"), + new UserMessage("WIKI_SNIPPET"), + new SystemMessage("LEDGER_SNAPSHOT"), + new SystemMessage("STALE_REMINDER"), + new UserMessage("user question"), + AssistantMessage.builder().content("answer").build() + )); + + List out = MessageNormalizer.normalize(in); + + // Exactly one SystemMessage, at index 0, containing all four segments + // joined by the canonical separator and in original encounter order. + assertThat(out.stream().filter(m -> m instanceof SystemMessage)).hasSize(1); + assertThat(out.get(0)).isInstanceOf(SystemMessage.class); + assertThat(out.get(0).getText()).isEqualTo( + "MAIN_PROMPT" + MessageNormalizer.SEPARATOR + + "SKILL_CATALOG" + MessageNormalizer.SEPARATOR + + "LEDGER_SNAPSHOT" + MessageNormalizer.SEPARATOR + + "STALE_REMINDER"); + + // Non-system messages preserve their original relative order. + assertThat(out.subList(1, out.size())) + .extracting(Message::getText) + .containsExactly("RUNTIME_CTX", "WIKI_SNIPPET", "user question", "answer"); + } + + // ---------- Blanks ---------- + + @Test + @DisplayName("blank SystemMessages are skipped during merge") + void blankSystemsDropped() { + List in = List.of( + new SystemMessage("MAIN"), + new SystemMessage(" "), + new SystemMessage(""), + new UserMessage("hi"), + new SystemMessage("\n\t \n") + ); + List out = MessageNormalizer.normalize(in); + + assertThat(out).hasSize(2); + assertThat(out.get(0)).isInstanceOf(SystemMessage.class); + assertThat(out.get(0).getText()).isEqualTo("MAIN"); + assertThat(out.get(1)).isInstanceOf(UserMessage.class); + } + + @Test + @DisplayName("all SystemMessages blank → SystemMessage dropped entirely") + void allBlankSystems_allDropped() { + List in = List.of( + new SystemMessage(""), + new SystemMessage(" "), + new UserMessage("hi") + ); + List out = MessageNormalizer.normalize(in); + + assertThat(out).hasSize(1); + assertThat(out.get(0)).isInstanceOf(UserMessage.class); + } + + @Test + @DisplayName("single blank SystemMessage at index 0 → dropped (not preserved by fast path)") + void singleBlankAtHead_dropped() { + // Fast-path guard: a single SystemMessage at [0] is canonical only when + // it has text. A blank one at [0] should still be dropped so we don't + // send providers an empty system slot. + List in = List.of( + new SystemMessage(" "), + new UserMessage("hi") + ); + List out = MessageNormalizer.normalize(in); + + assertThat(out).hasSize(1); + assertThat(out.get(0)).isInstanceOf(UserMessage.class); + } + + // ---------- Tool-call pairing preservation ---------- + + @Test + @DisplayName("tool_call ↔ tool_response pairing survives normalization") + void toolCallPairingPreserved() { + // Build a realistic ReAct history fragment with a system mid-stream + // (the bug case) and a tool-call/tool-response pair that must stay + // adjacent and in-order. + AssistantMessage assistantWithToolCall = AssistantMessage.builder() + .content("") + .toolCalls(List.of(new AssistantMessage.ToolCall( + "call-abc", "function", "read_file", "{\"path\":\"x\"}"))) + .build(); + ToolResponseMessage toolResponse = ToolResponseMessage.builder() + .responses(List.of(new ToolResponseMessage.ToolResponse( + "call-abc", "read_file", "file body"))) + .build(); + + List in = List.of( + new SystemMessage("MAIN"), + new UserMessage("first turn"), + new SystemMessage("LATE_SYSTEM"), + assistantWithToolCall, + toolResponse, + new UserMessage("second turn") + ); + + List out = MessageNormalizer.normalize(in); + + // System at [0] only; everything else in original order. + assertThat(out.get(0)).isInstanceOf(SystemMessage.class); + assertThat(out.get(0).getText()).isEqualTo("MAIN" + MessageNormalizer.SEPARATOR + "LATE_SYSTEM"); + assertThat(out.get(1)).isInstanceOf(UserMessage.class); + assertThat(out.get(2)).isSameAs(assistantWithToolCall); + assertThat(out.get(3)).isSameAs(toolResponse); + assertThat(out.get(4)).isInstanceOf(UserMessage.class); + + // The AssistantMessage(tool_calls) → ToolResponseMessage adjacency is + // critical: providers that strictly validate pairing (kimi-code, some + // OpenAI-compat layers) reject a 400 if these are reordered or split + // by another message. + int assistantIdx = out.indexOf(assistantWithToolCall); + int responseIdx = out.indexOf(toolResponse); + assertThat(responseIdx).isEqualTo(assistantIdx + 1); + } + + // ---------- Prompt overload ---------- + + @Test + @DisplayName("Prompt overload preserves options by reference") + void promptOverloadPreservesOptions() { + Prompt in = new Prompt(List.of( + new SystemMessage("a"), + new UserMessage("u"), + new SystemMessage("b") + )); + Prompt out = MessageNormalizer.normalize(in); + + // Different Prompt instance (because messages changed)… + assertThat(out).isNotSameAs(in); + // …but the options reference is preserved verbatim, which matters + // because doStreamCall mutates options.user via AssistantThinkingRelay + // and we cannot break that chain. + assertThat(out.getOptions()).isSameAs(in.getOptions()); + + assertThat(out.getInstructions()).hasSize(2); + assertThat(out.getInstructions().get(0).getText()) + .isEqualTo("a" + MessageNormalizer.SEPARATOR + "b"); + } + + @Test + @DisplayName("Prompt overload returns same reference on canonical input") + void promptOverloadFastPath() { + Prompt in = new Prompt(List.of( + new SystemMessage("only one"), + new UserMessage("u") + )); + assertThat(MessageNormalizer.normalize(in)).isSameAs(in); + } + + // ---------- Kill switch ---------- + + @Test + @DisplayName("kill switch off → normalize is a no-op (input returned by reference)") + void killSwitchOff_isNoOp() { + MessageNormalizer.setEnabledForTesting(false); + try { + List in = List.of( + new SystemMessage("MAIN"), + new UserMessage("u"), + new SystemMessage("LATE_SYSTEM") // would normally be merged + ); + assertThat(MessageNormalizer.normalize(in)).isSameAs(in); + + Prompt p = new Prompt(in); + assertThat(MessageNormalizer.normalize(p)).isSameAs(p); + } finally { + MessageNormalizer.setEnabledForTesting(true); + } + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/NodeStreamingChatHelperNormalizerWiringTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/NodeStreamingChatHelperNormalizerWiringTest.java new file mode 100644 index 00000000..4223fa86 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/NodeStreamingChatHelperNormalizerWiringTest.java @@ -0,0 +1,98 @@ +package vip.mate.agent.graph; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.springframework.ai.chat.messages.AssistantMessage; +import org.springframework.ai.chat.messages.Message; +import org.springframework.ai.chat.messages.SystemMessage; +import org.springframework.ai.chat.messages.UserMessage; +import org.springframework.ai.chat.metadata.ChatGenerationMetadata; +import org.springframework.ai.chat.model.ChatModel; +import org.springframework.ai.chat.model.ChatResponse; +import org.springframework.ai.chat.model.Generation; +import org.springframework.ai.chat.prompt.Prompt; +import reactor.core.publisher.Flux; +import vip.mate.channel.web.ChatStreamTracker; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Pins the wiring: {@link NodeStreamingChatHelper#streamCall} must apply + * {@link MessageNormalizer#normalize} before handing the prompt off to + * {@link ChatModel#stream}. + * + *

The pure {@code MessageNormalizer} contract is covered by + * {@code MessageNormalizerTest}. This test guards against a refactor that + * accidentally drops the call site at the top of {@code doStreamCall} — + * which is the only thing standing between MateClaw and the LM Studio + * {@code 400 "System message must be at the beginning"} regression. + */ +class NodeStreamingChatHelperNormalizerWiringTest { + + private ChatStreamTracker streamTracker; + + @BeforeEach + void setUp() { + streamTracker = mock(ChatStreamTracker.class); + when(streamTracker.isStopRequested(any())).thenReturn(false); + } + + /** Mock that emits one successful chunk with the given text. */ + private static ChatModel successModel(String text) { + ChatModel m = mock(ChatModel.class); + Generation gen = new Generation(new AssistantMessage(text), ChatGenerationMetadata.NULL); + ChatResponse resp = mock(ChatResponse.class); + when(resp.getResults()).thenReturn(List.of(gen)); + when(resp.getResult()).thenReturn(gen); + when(resp.getMetadata()).thenReturn(null); + when(m.stream(any(Prompt.class))).thenReturn(Flux.just(resp)); + return m; + } + + @Test + @DisplayName("streamCall normalizes SystemMessages before invoking ChatModel.stream") + void streamCallNormalizes() { + ChatModel chatModel = successModel("hi"); + NodeStreamingChatHelper helper = new NodeStreamingChatHelper(streamTracker); + + // Build the exact problematic shape ReasoningNode produces today: + // SystemMessages sprinkled around UserMessages — would 400 on LM Studio. + Prompt prompt = new Prompt(List.of( + new SystemMessage("MAIN_PROMPT"), + new SystemMessage("SKILL_CATALOG"), + new UserMessage("RUNTIME_CTX"), + new SystemMessage("LEDGER_SNAPSHOT"), + new UserMessage("user question") + )); + + helper.streamCall(chatModel, prompt, "conv-1", "reasoning"); + + // Capture the Prompt that actually reached ChatModel.stream. + ArgumentCaptor captor = ArgumentCaptor.forClass(Prompt.class); + verify(chatModel).stream(captor.capture()); + Prompt outbound = captor.getValue(); + List sent = outbound.getInstructions(); + + // Exactly one SystemMessage, at index 0, containing all three system + // segments in encounter order. + assertThat(sent.stream().filter(m -> m instanceof SystemMessage)).hasSize(1); + assertThat(sent.get(0)).isInstanceOf(SystemMessage.class); + assertThat(sent.get(0).getText()).isEqualTo( + "MAIN_PROMPT" + MessageNormalizer.SEPARATOR + + "SKILL_CATALOG" + MessageNormalizer.SEPARATOR + + "LEDGER_SNAPSHOT"); + + // Non-system relative order preserved. + assertThat(sent.subList(1, sent.size())) + .extracting(Message::getText) + .containsExactly("RUNTIME_CTX", "user question"); + } +}