From 68c968f6f3fcc117b60b89f28b17da8729c71f45 Mon Sep 17 00:00:00 2001 From: qin-chenghan Date: Wed, 1 Jul 2026 11:12:20 +0800 Subject: [PATCH] fix(frontend): keep orphan tool messages visible (#3880) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(frontend): keep orphan tool messages visible LangGraph `messages-tuple` stream mode can emit tool-result events out of order or replay them from subagent state (e.g. the bash subagent under LocalSandboxProvider with allow_host_bash: true). When that happens, the tool message arrives after a terminal assistant/human group, so getMessageGroups' lastOpenGroup() returns null. The previous behaviour was console.error + drop, which silently hid the tool result from the UI - the user could not see the tool output or tool-call records even though the agent executed the action correctly (backend + Langfuse trace were both fine). Fallback now attaches the orphan tool message to the most recent group so the UI shows it. Adds a unit test that covers the orphan-tool path and a duplicate-stream regression test. * test(frontend): make orphan-tool replay test actually reach the fallback The previous fixture was `human → ai(tool_calls) → tool → tool(replay)` with no terminal group in between, so both tool messages hit the unchanged happy path (`open.messages.push(...)`). Neither message was an orphan, the new `else if (groups.length > 0)` fallback was never exercised, and the `>= 1` length assertion was satisfied trivially by t-1a alone. Interleave a terminal assistant message between the original tool result and the replayed one, so t-1b arrives when lastOpenGroup() returns null. Now the strict assertion that t-1b is reachable can only pass via the new fallback branch. Review feedback from @willem-bd on PR #3880. * fix(frontend): satisfy noUncheckedIndexedAccess in orphan-tool fallback The fallback branch pushed into `groups[groups.length - 1].messages` directly, which trips `noUncheckedIndexedAccess` under the project's strict TS config and breaks lint-frontend, e2e-tests, and the full-stack render CI layer. Take `groups[groups.length - 1]` into a local `lastGroup` and check for `undefined` explicitly. The `else if (groups.length > 0)` guard becomes redundant once `lastGroup` is checked, so the branches are folded into the outer `else` to keep the structure flat. Behavior is unchanged: orphan tools still attach to the most recent group, and the empty-groups diagnostic still logs at ERROR level. Review feedback from @willem-bd on PR #3880. --------- Co-authored-by: Willem Jiang --- frontend/src/core/messages/utils.ts | 24 ++++- .../tests/unit/core/messages/utils.test.ts | 98 +++++++++++++++++++ 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/frontend/src/core/messages/utils.ts b/frontend/src/core/messages/utils.ts index 3ced9d3f4..f2ea7f3eb 100644 --- a/frontend/src/core/messages/utils.ts +++ b/frontend/src/core/messages/utils.ts @@ -80,10 +80,26 @@ export function getMessageGroups(messages: Message[]): MessageGroup[] { if (open) { open.messages.push(message); } else { - console.error( - "Unexpected tool message outside a processing group", - message, - ); + // Fallback for orphan tool messages — LangGraph `messages-tuple` can + // emit tool-result events out of order or replay them from subagent + // state (e.g. bash subagent under LocalSandboxProvider with + // allow_host_bash). When that happens, the tool message arrives after + // a terminal group and lastOpenGroup() returns null. Previously we + // dropped the message with console.error, silently hiding the tool + // result from the UI. Attach to the most recent group instead so the + // user can still see what the agent did. + const lastGroup = groups[groups.length - 1]; + if (lastGroup) { + lastGroup.messages.push(message); + } else { + // groups is empty (shouldn't happen — the outer for loop is guarded + // by `messages.length === 0 -> return []`), but keep the diagnostic + // just in case. + console.error( + "Unexpected tool message with no preceding group", + message, + ); + } } } continue; diff --git a/frontend/tests/unit/core/messages/utils.test.ts b/frontend/tests/unit/core/messages/utils.test.ts index 999b2d4b8..dde79e7ca 100644 --- a/frontend/tests/unit/core/messages/utils.test.ts +++ b/frontend/tests/unit/core/messages/utils.test.ts @@ -541,3 +541,101 @@ describe("multi-part content with bare-string continuations", () => { ); }); }); + +describe("orphan tool messages", () => { + // LangGraph stream-mode "messages-tuple" can emit tool-result events out of order or + // replayed from subagent state (e.g. bash subagent under LocalSandboxProvider with + // allow_host_bash). When that happens, the tool message arrives after a terminal + // assistant/human group, so getMessageGroups' lastOpenGroup() returns null. + // + // The previous behaviour was console.error + drop, which silently hid the tool + // result from the UI. The fix falls back to attaching the orphan tool to the most + // recent group so the user can still see what the agent did. + + test("attaches orphan tool message to the most recent group instead of dropping it", () => { + const messages = [ + { id: "h-1", type: "human", content: "Run something" }, + { + id: "ai-1", + type: "ai", + content: "ok", + tool_calls: [{ id: "call-1", name: "bash", args: {} }], + }, + { + id: "t-1", + type: "tool", + name: "bash", + tool_call_id: "call-1", + content: "output-1", + }, + { id: "ai-2", type: "ai", content: "Done." }, // terminal assistant group + // Orphan tool: arrives after a terminal group, no preceding processing group + { + id: "t-2", + type: "tool", + name: "bash", + tool_call_id: "call-2", + content: "output-2", + }, + ] as Message[]; + + const groups = getMessageGroups(messages); + + // Expect groups: human, assistant:processing (ai-1 + t-1), assistant (ai-2), and + // t-2 should be attached to the last group (assistant), not dropped. + const types = groups.map((g) => g.type); + expect(types).toEqual(["human", "assistant:processing", "assistant"]); + + // t-2 must be retrievable from one of the groups — must NOT be silently dropped + const allMessages = groups.flatMap((g) => g.messages); + const t2 = allMessages.find((m) => m.id === "t-2"); + expect(t2).toBeDefined(); + expect(t2?.type).toBe("tool"); + }); + + test("replayed tool with same tool_call_id is not lost (duplicate stream events)", () => { + // LangGraph subagent state restoration can replay tool-result events. The + // frontend log shows the same tool_call_id arriving twice. Both occurrences + // should be visible in the UI, not just the first. + const messages = [ + { id: "h-1", type: "human", content: "q" }, + { + id: "ai-1", + type: "ai", + content: "", + tool_calls: [{ id: "call-x", name: "bash", args: {} }], + }, + { + id: "t-1a", + type: "tool", + name: "bash", + tool_call_id: "call-x", + content: "first delivery", + }, + // Terminal assistant group ends the turn and closes the processing group. + // Without this interleave the replayed t-1b would still take the + // unchanged happy path; with it, t-1b arrives when lastOpenGroup() + // returns null and must take the new fallback branch to be visible. + { id: "ai-2", type: "ai", content: "Done." }, + // Replayed tool-result for the original tool_call — must reach the new + // else-if (groups.length > 0) branch instead of being dropped. + { + id: "t-1b", + type: "tool", + name: "bash", + tool_call_id: "call-x", + content: "first delivery", + }, + ] as Message[]; + + const groups = getMessageGroups(messages); + const allMessages = groups.flatMap((g) => g.messages); + + // Strict assertion: the replayed tool message must be reachable from a + // group (i.e. attached via the new fallback). Before the fix this was + // silently dropped by console.error. + const t1b = allMessages.find((m) => m.id === "t-1b"); + expect(t1b).toBeDefined(); + expect(t1b?.type).toBe("tool"); + }); +});