From 21e7f3f5c134b27d9b52708ecb1f81bebd24c820 Mon Sep 17 00:00:00 2001 From: Makonnen Date: Thu, 19 Feb 2026 05:02:23 +0000 Subject: [PATCH] fix: use parentID matching instead of ID ordering for prompt loop exit and message rendering When the client clock is ahead of the server, user message IDs (generated client-side) sort after assistant message IDs (generated server-side). This broke the prompt loop exit check and the UI message pairing logic. - Extract shouldExitLoop() into a pure function that uses parentID matching instead of relying on ID ordering - Extract findAssistantMessages() with forward+backward scan to handle messages sorted out of expected order due to clock skew - Remove debug console.log statements added during investigation - Add tests for both extracted functions Co-Authored-By: Claude Opus 4.6 --- .../session/find-assistant-messages.test.ts | 81 ++++++++++++++++++ packages/opencode/src/session/prompt.ts | 14 +-- .../test/session/prompt-loop-exit.test.ts | 85 +++++++++++++++++++ .../components/find-assistant-messages.tsx | 40 +++++++++ packages/ui/src/components/session-turn.tsx | 10 +-- 5 files changed, 217 insertions(+), 13 deletions(-) create mode 100644 packages/app/src/components/session/find-assistant-messages.test.ts create mode 100644 packages/opencode/test/session/prompt-loop-exit.test.ts create mode 100644 packages/ui/src/components/find-assistant-messages.tsx diff --git a/packages/app/src/components/session/find-assistant-messages.test.ts b/packages/app/src/components/session/find-assistant-messages.test.ts new file mode 100644 index 000000000000..ab944e2cc9f1 --- /dev/null +++ b/packages/app/src/components/session/find-assistant-messages.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, test } from "bun:test" +import type { Message } from "@opencode-ai/sdk/v2/client" +import { findAssistantMessages } from "@opencode-ai/ui/find-assistant-messages" + +function user(id: string): Message { + return { + id, + role: "user", + sessionID: "session-1", + time: { created: 1 }, + } as unknown as Message +} + +function assistant(id: string, parentID: string): Message { + return { + id, + role: "assistant", + sessionID: "session-1", + parentID, + time: { created: 1 }, + } as unknown as Message +} + +describe("findAssistantMessages", () => { + test("normal ordering: assistant after user in array → found via forward scan", () => { + const messages = [user("u1"), assistant("a1", "u1")] + const result = findAssistantMessages(messages, 0, "u1") + expect(result).toHaveLength(1) + expect(result[0].id).toBe("a1") + }) + + test("clock skew: assistant before user in array → found via backward scan", () => { + // When client clock is ahead, user ID sorts after assistant ID, + // so assistant appears earlier in the ID-sorted message array + const messages = [assistant("a1", "u1"), user("u1")] + const result = findAssistantMessages(messages, 1, "u1") + expect(result).toHaveLength(1) + expect(result[0].id).toBe("a1") + }) + + test("no assistant messages → returns empty array", () => { + const messages = [user("u1"), user("u2")] + const result = findAssistantMessages(messages, 0, "u1") + expect(result).toHaveLength(0) + }) + + test("multiple assistant messages with matching parentID → all found", () => { + const messages = [user("u1"), assistant("a1", "u1"), assistant("a2", "u1")] + const result = findAssistantMessages(messages, 0, "u1") + expect(result).toHaveLength(2) + expect(result[0].id).toBe("a1") + expect(result[1].id).toBe("a2") + }) + + test("does not return assistant messages with different parentID", () => { + const messages = [user("u1"), assistant("a1", "u1"), assistant("a2", "other")] + const result = findAssistantMessages(messages, 0, "u1") + expect(result).toHaveLength(1) + expect(result[0].id).toBe("a1") + }) + + test("stops forward scan at next user message", () => { + const messages = [user("u1"), assistant("a1", "u1"), user("u2"), assistant("a2", "u1")] + const result = findAssistantMessages(messages, 0, "u1") + expect(result).toHaveLength(1) + expect(result[0].id).toBe("a1") + }) + + test("stops backward scan at previous user message", () => { + const messages = [assistant("a0", "u1"), user("u0"), assistant("a1", "u1"), user("u1")] + const result = findAssistantMessages(messages, 3, "u1") + expect(result).toHaveLength(1) + expect(result[0].id).toBe("a1") + }) + + test("invalid index returns empty array", () => { + const messages = [user("u1")] + expect(findAssistantMessages(messages, -1, "u1")).toHaveLength(0) + expect(findAssistantMessages(messages, 5, "u1")).toHaveLength(0) + }) +}) diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 2c799b1100f7..83a9fc8fa041 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -1359,11 +1359,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the } if (!lastUser) throw new Error("No user message found in stream. This should never happen.") - if ( - lastAssistant?.finish && - !["tool-calls"].includes(lastAssistant.finish) && - lastUser.id < lastAssistant.id - ) { + if (shouldExitLoop(lastUser, lastAssistant)) { log.info("exiting loop", { sessionID }) break } @@ -1892,4 +1888,12 @@ NOTE: At any point in time through this workflow you should feel free to ask the const argsRegex = /(?:\[Image\s+\d+\]|"[^"]*"|'[^']*'|[^\s"']+)/gi const placeholderRegex = /\$(\d+)/g const quoteTrimRegex = /^["']|["']$/g + + /** @internal Exported for testing */ + export function shouldExitLoop(lastUser: MessageV2.User | undefined, lastAssistant: MessageV2.Assistant | undefined) { + if (!lastUser) return false + if (!lastAssistant?.finish) return false + if (["tool-calls", "unknown"].includes(lastAssistant.finish)) return false + return lastAssistant.parentID === lastUser.id + } } diff --git a/packages/opencode/test/session/prompt-loop-exit.test.ts b/packages/opencode/test/session/prompt-loop-exit.test.ts new file mode 100644 index 000000000000..8de582d0d7e5 --- /dev/null +++ b/packages/opencode/test/session/prompt-loop-exit.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, test } from "bun:test" +import type { MessageV2 } from "../../src/session/message-v2" +import { SessionPrompt } from "../../src/session/prompt" + +function makeUser(id: string): MessageV2.User { + return { + id, + role: "user", + sessionID: "session-1", + time: { created: Date.now() }, + agent: "default", + model: { providerID: "openai", modelID: "gpt-4" }, + } as MessageV2.User +} + +function makeAssistant( + id: string, + parentID: string, + finish?: string, +): MessageV2.Assistant { + return { + id, + role: "assistant", + sessionID: "session-1", + parentID, + mode: "default", + agent: "default", + path: { cwd: "/tmp", root: "/tmp" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + modelID: "gpt-4", + providerID: "openai", + time: { created: Date.now() }, + finish, + } as MessageV2.Assistant +} + +describe("shouldExitLoop", () => { + test("normal case: user ID < assistant ID, parentID matches, finish=end_turn → exits", () => { + const user = makeUser("01AAA") + const assistant = makeAssistant("01BBB", "01AAA", "end_turn") + expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(true) + }) + + test("clock skew: user ID > assistant ID, parentID matches, finish=stop → exits", () => { + // Simulates client clock ahead: user message ID sorts AFTER the assistant ID + const user = makeUser("01ZZZ") + const assistant = makeAssistant("01AAA", "01ZZZ", "stop") + expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(true) + }) + + test("unfinished assistant: finish=tool-calls → does NOT exit", () => { + const user = makeUser("01AAA") + const assistant = makeAssistant("01BBB", "01AAA", "tool-calls") + expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(false) + }) + + test("unfinished assistant: finish=unknown → does NOT exit", () => { + const user = makeUser("01AAA") + const assistant = makeAssistant("01BBB", "01AAA", "unknown") + expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(false) + }) + + test("no assistant yet → does NOT exit", () => { + const user = makeUser("01AAA") + expect(SessionPrompt.shouldExitLoop(user, undefined)).toBe(false) + }) + + test("assistant has no finish → does NOT exit", () => { + const user = makeUser("01AAA") + const assistant = makeAssistant("01BBB", "01AAA", undefined) + expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(false) + }) + + test("parentID mismatch → does NOT exit", () => { + const user = makeUser("01AAA") + const assistant = makeAssistant("01BBB", "01OTHER", "end_turn") + expect(SessionPrompt.shouldExitLoop(user, assistant)).toBe(false) + }) + + test("no user message → does NOT exit", () => { + const assistant = makeAssistant("01BBB", "01AAA", "end_turn") + expect(SessionPrompt.shouldExitLoop(undefined, assistant)).toBe(false) + }) +}) diff --git a/packages/ui/src/components/find-assistant-messages.tsx b/packages/ui/src/components/find-assistant-messages.tsx new file mode 100644 index 000000000000..1448e5e34712 --- /dev/null +++ b/packages/ui/src/components/find-assistant-messages.tsx @@ -0,0 +1,40 @@ +import type { AssistantMessage, Message as MessageType } from "@opencode-ai/sdk/v2/client" + +/** + * Find assistant messages that are replies to a given user message. + * + * Scans forward from the user message index first, then falls back to scanning + * backward. The backward scan handles clock skew where assistant messages + * (generated server-side) sort before the user message (generated client-side + * with an ahead clock) in the ID-sorted array. + */ +export function findAssistantMessages( + messages: MessageType[], + userIndex: number, + userID: string, +): AssistantMessage[] { + if (userIndex < 0 || userIndex >= messages.length) return [] + + const result: AssistantMessage[] = [] + + // Scan forward from user message + for (let i = userIndex + 1; i < messages.length; i++) { + const item = messages[i] + if (!item) continue + if (item.role === "user") break + if (item.role === "assistant" && item.parentID === userID) result.push(item as AssistantMessage) + } + + // Scan backward to find assistant messages that sort before the user + // message due to clock skew between client and server + if (result.length === 0) { + for (let i = userIndex - 1; i >= 0; i--) { + const item = messages[i] + if (!item) continue + if (item.role === "user") break + if (item.role === "assistant" && item.parentID === userID) result.push(item as AssistantMessage) + } + } + + return result +} diff --git a/packages/ui/src/components/session-turn.tsx b/packages/ui/src/components/session-turn.tsx index ed4c0e914914..6c9dc80f70f5 100644 --- a/packages/ui/src/components/session-turn.tsx +++ b/packages/ui/src/components/session-turn.tsx @@ -9,6 +9,7 @@ import { createEffect, createMemo, createSignal, For, on, ParentProps, Show } fr import { createStore } from "solid-js/store" import { Dynamic } from "solid-js/web" import { AssistantParts, Message, MessageDivider, PART_MAPPING, type UserActions } from "./message-part" +import { findAssistantMessages } from "./find-assistant-messages" import { Card } from "./card" import { Accordion } from "./accordion" import { StickyAccordionHeader } from "./sticky-accordion-header" @@ -268,14 +269,7 @@ export function SessionTurn( const index = messageIndex() if (index < 0) return emptyAssistant - const result: AssistantMessage[] = [] - for (let i = index + 1; i < messages.length; i++) { - const item = messages[i] - if (!item) continue - if (item.role === "user") break - if (item.role === "assistant" && item.parentID === msg.id) result.push(item as AssistantMessage) - } - return result + return findAssistantMessages(messages, index, msg.id) }, emptyAssistant, { equals: same },