Selaa lähdekoodia

Apply PR #14307: fix: use parentID matching instead of ID ordering for prompt loop exit and message rendering

opencode-agent[bot] 11 tuntia sitten
vanhempi
sitoutus
b9b9083d0a

+ 81 - 0
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)
+  })
+})

+ 10 - 6
packages/opencode/src/session/prompt.ts

@@ -1342,12 +1342,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the
           const hasToolCalls =
             lastAssistantMsg?.parts.some((part) => part.type === "tool" && !part.metadata?.providerExecuted) ?? false
 
-          if (
-            lastAssistant?.finish &&
-            !["tool-calls"].includes(lastAssistant.finish) &&
-            !hasToolCalls &&
-            lastUser.id < lastAssistant.id
-          ) {
+          if (!hasToolCalls && shouldExitLoop(lastUser, lastAssistant)) {
             yield* slog.info("exiting loop")
             break
           }
@@ -1835,6 +1830,15 @@ export function createStructuredOutputTool(input: {
     },
   })
 }
+
+/** @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
+}
+
 const bashRegex = /!`([^`]+)`/g
 // Match [Image N] as single token, quoted strings, or non-space sequences
 const argsRegex = /(?:\[Image\s+\d+\]|"[^"]*"|'[^']*'|[^\s"']+)/gi

+ 85 - 0
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)
+  })
+})

+ 40 - 0
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
+}

+ 2 - 8
packages/ui/src/components/session-turn.tsx

@@ -14,6 +14,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"
@@ -270,14 +271,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 },