fix: agent-audit-findings — fix streaming response, message type detection, and array iteration

openspec/changes/archive/2026-06-20-fix-agent-audit-findings/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-06-20

openspec/changes/archive/2026-06-20-fix-agent-audit-findings/design.md

@@ -0,0 +1,43 @@
+## Context
+
+The agent module (`src/agent/react.js`) is the core inference engine for the madz application. It handles both synchronous and streaming calls to the LangGraph React agent, manages LLM response caching, conversation compaction, and content extraction from agent results. A recent code audit identified three issues in this file: a high-severity bug in streaming response handling, a medium-severity bug in message type detection, and a low-severity performance issue.
+
+## Goals / Non-Goals
+
+**Goals:**
+- Fix streaming responses to return aggregated AI text instead of the original user query
+- Add comprehensive message type detection supporting all LangChain message types
+- Eliminate unnecessary array allocations in message iteration
+
+**Non-Goals:**
+- Changes to non-streaming `callReactAgent` return behavior
+- Changes to message handling logic beyond type detection
+- Changes to other modules or external dependencies
+- Migration or deployment procedures (this is a straightforward code fix)
+
+## Decisions
+
+### Decision 1: Return `aggregatedText || originalMessage` instead of just `aggregatedText`
+**Rationale:** Preserves existing fallback behavior for edge cases (empty streams, errors) while correctly returning the AI response on the happy path. This is a minimal, safe change that fixes the bug without altering the contract.
+
+### Decision 2: Extract message type detection into `getMessageRole()` helper
+**Rationale:** Two call sites had identical inline type detection logic. Extracting to a shared helper eliminates duplication, makes it easy to add new message types in the future, and ensures consistency. The fallback to "system" for unknown types prevents silent data loss during conversation compaction.
+
+### Decision 3: Replace `[...msgsArray].reverse().find()` with reverse for-loop
+**Rationale:** The spread + reverse + find pattern creates three O(n) operations (copy, reverse, iterate). A single reverse for-loop achieves the same result with O(n) time and O(1) extra space. This is a purely mechanical refactoring with identical semantics.
+
+## Risks / Trade-offs
+
+- **[Risk]** Streaming behavior change could affect downstream consumers expecting the original message
+  **Mitigation:** The original behavior was incorrect (returning user query instead of AI response). The fallback to `originalMessage` when no text events occurred preserves the previous behavior for edge cases.
+
+- **[Risk]** Adding ToolMessage import could fail if the LangChain version doesn't export it
+  **Mitigation:** ToolMessage has been part of `@langchain/core/messages` since early v0.x releases. The project uses `^10.3.1` of pino (latest), and LangChain is a core dependency that is kept up to date.
+
+## Migration Plan
+
+No migration needed. This is a code-only fix with no configuration changes, database migrations, or deployment steps. The fix can be deployed immediately after the PR merges.
+
+## Open Questions
+
+None. All three audit findings have clear, well-understood fixes.

openspec/changes/archive/2026-06-20-fix-agent-audit-findings/proposal.md

@@ -0,0 +1,25 @@
+## Why
+
+A code audit of `src/agent/react.js` identified three issues: a high-severity bug where `callReactAgentStreaming` returns the original user message instead of the aggregated AI response text, a medium-severity bug where message type detection only handles three message types (missing ToolMessage and chunk variants), and a low-severity performance issue with unnecessary array copies during message iteration. These bugs cause the agent to return incorrect content in streaming mode and fail to handle tool messages correctly.
+
+## What Changes
+
+- Fix `callReactAgentStreaming` to return `{ content: aggregatedText || originalMessage }` instead of `{ content: originalMessage }` on both return paths (success and end-of-stream)
+- Extract message type detection into a `getMessageRole()` helper that handles HumanMessage, AIMessage, SystemMessage, ToolMessage, and their chunk variants (HumanMessageChunk, AIMessageChunk), falling back to "system" for unknown types
+- Replace both inline type detection call sites in `callReactAgent` and `callReactAgentStreaming` with calls to `getMessageRole()`
+- Replace `[...msgsArray].reverse().find()` with a reverse for-loop in `extractContent()` to eliminate unnecessary array allocations
+
+## Capabilities
+
+### New Capabilities
+<!-- None — this is a bug fix, not a new capability -->
+
+### Modified Capabilities
+- `agent`: Streaming response now correctly returns aggregated AI text instead of the original user query; message type detection now handles ToolMessage and chunk variants
+
+## Impact
+
+- **Affected code:** `src/agent/react.js` (primary), `tests/unit/react_agent.test.js` (new tests)
+- **API changes:** None — public API (`callReactAgent`, `callReactAgentStreaming`, `createReactAgent`, `clearCache`) unchanged
+- **Behavior changes:** Streaming responses now return actual AI text (correct behavior); ToolMessage instances are now correctly mapped to "tool" role in conversation history
+- **Dependencies:** No new dependencies — ToolMessage and HumanMessageChunk are already available in `@langchain/core/messages`

openspec/changes/archive/2026-06-20-fix-agent-audit-findings/specs/react-agent/spec.md

@@ -0,0 +1,46 @@
+## ADDED Requirements
+
+### Requirement: Streaming responses return aggregated AI text
+The `callReactAgentStreaming` function SHALL return `{ content: aggregatedText }` where `aggregatedText` is the concatenated text from all `on_chat_model_stream` events containing `AIMessageChunk` instances. When no text events occur, it SHALL fall back to `{ content: originalMessage }`.
+
+#### Scenario: Successful stream returns aggregated text
+- **WHEN** `callReactAgentStreaming` processes stream events with AIMessageChunk instances containing "Hello" and " World"
+- **THEN** the function returns `{ content: "Hello World" }`
+
+#### Scenario: Empty stream falls back to original message
+- **WHEN** `callReactAgentStreaming` processes zero stream events
+- **THEN** the function returns `{ content: originalMessage }`
+
+#### Scenario: Recursion limit error returns original message immediately
+- **WHEN** the agent throws a GraphRecursionError
+- **THEN** the function returns `{ content: originalMessage }` without waiting for stream completion
+
+### Requirement: Message type detection handles all message types
+The `getMessageRole` function SHALL map LangChain message instances to their corresponding conversation role strings: HumanMessage/HumanMessageChunk → "user", AIMessage/AIMessageChunk → "assistant", ToolMessage → "tool", SystemMessage → "system". Unknown message types SHALL fall back to "system".
+
+#### Scenario: HumanMessage maps to user
+- **WHEN** `getMessageRole(new HumanMessage("hi"))` is called
+- **THEN** it returns "user"
+
+#### Scenario: AIMessage maps to assistant
+- **WHEN** `getMessageRole(new AIMessage("hello"))` is called
+- **THEN** it returns "assistant"
+
+#### Scenario: ToolMessage maps to tool
+- **WHEN** `getMessageRole(new ToolMessage({ content: "result", tool_call_id: "tc1" }))` is called
+- **THEN** it returns "tool"
+
+#### Scenario: Chunk variants are handled
+- **WHEN** `getMessageRole(new HumanMessageChunk("hi"))` or `getMessageRole(new AIMessageChunk("hello"))` is called
+- **THEN** they return "user" and "assistant" respectively
+
+#### Scenario: Unknown types fall back to system
+- **WHEN** `getMessageRole({ content: "unknown", type: "custom" })` is called
+- **THEN** it returns "system"
+
+### Requirement: Message array iteration avoids unnecessary copies
+The `extractContent` function SHALL find the last AIMessage or AIMessageChunk in the messages array using a reverse for-loop without creating intermediate array copies via spread or reverse.
+
+#### Scenario: Finds last AI message efficiently
+- **WHEN** `extractContent` is called with an array containing multiple messages including AIMessage instances
+- **THEN** it returns the content of the last AIMessage/AIMessageChunk without spreading or reversing the array

openspec/changes/archive/2026-06-20-fix-agent-audit-findings/tasks.md

@@ -0,0 +1,30 @@
+## 1. Import Updates
+
+- [x] 1.1 Add ToolMessage and HumanMessageChunk to @langchain/core/messages imports in src/agent/react.js
+
+## 2. Message Type Detection Helper
+
+- [x] 2.1 Implement getMessageRole() function that maps HumanMessage/HumanMessageChunk → "user", AIMessage/AIMessageChunk → "assistant", ToolMessage → "tool", SystemMessage → "system", with "system" fallback for unknown types
+- [x] 2.2 Replace inline type detection in callReactAgent (line ~172) with getMessageRole() call
+- [x] 2.3 Replace inline type detection in callReactAgentStreaming (line ~472) with getMessageRole() call
+
+## 3. Streaming Response Fix
+
+- [x] 3.1 Fix first return point in callReactAgentStreaming (success path, line ~436) to return { content: aggregatedText || originalMessage }
+- [x] 3.2 Fix second return point in callReactAgentStreaming (end-of-stream, line ~524) to return { content: aggregatedText || originalMessage }
+
+## 4. Performance Optimization
+
+- [x] 4.1 Replace [...msgsArray].reverse().find() with reverse for-loop in extractContent() function
+
+## 5. Unit Tests
+
+- [x] 5.1 Add tests for getMessageRole(): HumanMessage → "user", HumanMessageChunk → "user", AIMessage → "assistant", AIMessageChunk → "assistant", ToolMessage → "tool", SystemMessage → "system", unknown → "system"
+- [x] 5.2 Add test for streaming aggregation: multiple AIMessageChunk events produce concatenated text
+- [x] 5.3 Add test for streaming fallback: empty events array returns original message
+
+## 6. Verification
+
+- [x] 6.1 Run npm run test and verify all tests pass
+- [x] 6.2 Run npm run lint and verify no lint errors
+- [x] 6.3 Verify application starts without errors

src/agent/react.js

@@ -1,5 +1,5 @@
 import { createReactAgent as createReactAgentGraph } from "@langchain/langgraph/prebuilt";
-import { HumanMessage, SystemMessage, AIMessage, AIMessageChunk } from "@langchain/core/messages";
+import { HumanMessage, HumanMessageChunk, SystemMessage, AIMessage, AIMessageChunk, ToolMessage } from "@langchain/core/messages";
 import {
 	extractContextLength,
 	isContextLengthError,
@@ -8,6 +8,22 @@ import {
 import { createLlmCache, getCacheKey } from "../cache/llm_cache.js";
 import { loadConfig } from "../config/loader.js";
 
+/**
+ * Map a LangChain message instance to its corresponding conversation role.
+ * Handles all standard message types — HumanMessage, AIMessage, SystemMessage,
+ * ToolMessage, and their chunk variants — falling back to "system" for unknown
+ * types to avoid silent data loss during compaction.
+ * @param {import("@langchain/core/messages").BaseMessage} msg
+ * @returns {string}
+ */
+export function getMessageRole(msg) {
+	if (msg instanceof HumanMessage || msg instanceof HumanMessageChunk) return "user";
+	if (msg instanceof AIMessage || msg instanceof AIMessageChunk) return "assistant";
+	if (msg instanceof ToolMessage) return "tool";
+	if (msg instanceof SystemMessage) return "system";
+	return "system"; // fallback — shouldn't happen with well-formed conversations
+}
+
 /**
  * Lazily initialize the LLM response cache using configured lru.size and lru.ttl.
  * Falls back to defaults (100, 600000) if config is unavailable.
@@ -156,8 +172,7 @@ export async function callReactAgent(agent, message, config, systemPrompt, callb
 				const conversation = messages
 					.filter((m) => !(m instanceof SystemMessage))
 					.map((m) => ({
-						role:
-							m instanceof HumanMessage ? "user" : m instanceof AIMessage ? "assistant" : "system",
+						role: getMessageRole(m),
 						content: typeof m.content === "string" ? m.content : JSON.stringify(m.content),
 					}));
 
@@ -208,9 +223,13 @@ export async function callReactAgent(agent, message, config, systemPrompt, callb
 function extractContent(result, fallback) {
 	const msgsArray = Array.isArray(result.messages) ? result.messages : [];
 
-	const lastAI = [...msgsArray]
-		.reverse()
-		.find((msg) => msg instanceof AIMessage || msg instanceof AIMessageChunk);
+	let lastAI = null;
+	for (let i = msgsArray.length - 1; i >= 0; i--) {
+		if (msgsArray[i] instanceof AIMessage || msgsArray[i] instanceof AIMessageChunk) {
+			lastAI = msgsArray[i];
+			break;
+		}
+	}
 	if (lastAI && lastAI.content) {
 		const content =
 			typeof lastAI.content === "string" ? lastAI.content : JSON.stringify(lastAI.content);
@@ -417,7 +436,7 @@ async function callReactAgentStreaming(
 			if (compactionActive && callback) {
 				callback({ type: "compaction_end" });
 			}
-			return { content: originalMessage };
+			return { content: aggregatedText || originalMessage };
 		} catch (err) {
 			// Handle recursion limit — always return immediately
 			if (err instanceof Error && err.name === "GraphRecursionError") {
@@ -453,8 +472,7 @@ async function callReactAgentStreaming(
 				const conversation = currentMessages
 					.filter((m) => !(m instanceof SystemMessage))
 					.map((m) => ({
-						role:
-							m instanceof HumanMessage ? "user" : m instanceof AIMessage ? "assistant" : "system",
+						role: getMessageRole(m),
 						content: typeof m.content === "string" ? m.content : JSON.stringify(m.content),
 					}));
 
@@ -506,5 +524,5 @@ async function callReactAgentStreaming(
 		callback({ type: "compaction_end" });
 	}
 
-	return { content: originalMessage };
+	return { content: aggregatedText || originalMessage };
 }