Sfoglia il codice sorgente

fix(github): handle step-start/step-finish parts in extractResponseText (#12470)

Matt Silverlock 2 mesi fa
parent
commit
24dbc46548

+ 5 - 13
packages/opencode/src/cli/cmd/github.ts

@@ -160,25 +160,17 @@ export function parseGitHubRemote(url: string): { owner: string; repo: string }
 
 /**
  * Extracts displayable text from assistant response parts.
- * Returns null for tool-only or reasoning-only responses (signals summary needed).
- * Throws for truly unusable responses (empty, step-start only, etc.).
+ * Returns null for non-text responses (signals summary needed).
+ * Throws only for truly empty responses.
  */
 export function extractResponseText(parts: MessageV2.Part[]): string | null {
-  // Priority 1: Look for text parts
   const textPart = parts.findLast((p) => p.type === "text")
   if (textPart) return textPart.text
 
-  // Priority 2: Reasoning-only - return null to signal summary needed
-  const reasoningPart = parts.findLast((p) => p.type === "reasoning")
-  if (reasoningPart) return null
+  // Non-text parts (tools, reasoning, step-start/step-finish, etc.) - signal summary needed
+  if (parts.length > 0) return null
 
-  // Priority 3: Tool-only - return null to signal summary needed
-  const toolParts = parts.filter((p) => p.type === "tool" && p.state.status === "completed")
-  if (toolParts.length > 0) return null
-
-  // No usable parts - throw with debug info
-  const partTypes = parts.map((p) => p.type).join(", ") || "none"
-  throw new Error(`Failed to parse response. Part types found: [${partTypes}]`)
+  throw new Error("Failed to parse response: no parts returned")
 }
 
 export const GithubCommand = cmd({

+ 38 - 6
packages/opencode/test/cli/github-action.test.ts

@@ -67,6 +67,18 @@ function createStepStartPart(): MessageV2.Part {
   }
 }
 
+function createStepFinishPart(): MessageV2.Part {
+  return {
+    id: "1",
+    sessionID: "s",
+    messageID: "m",
+    type: "step-finish" as const,
+    reason: "done",
+    cost: 0,
+    tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } },
+  }
+}
+
 describe("extractResponseText", () => {
   test("returns text from text part", () => {
     const parts = [createTextPart("Hello world")]
@@ -103,18 +115,38 @@ describe("extractResponseText", () => {
     expect(extractResponseText(parts)).toBeNull()
   })
 
-  test("ignores running tool parts (throws since no completed tools)", () => {
+  test("returns null for running tool parts (signals summary needed)", () => {
     const parts = [createToolPart("bash", "", "running")]
-    expect(() => extractResponseText(parts)).toThrow("Failed to parse response")
+    expect(extractResponseText(parts)).toBeNull()
   })
 
-  test("throws with part types on empty array", () => {
-    expect(() => extractResponseText([])).toThrow("Part types found: [none]")
+  test("throws on empty array", () => {
+    expect(() => extractResponseText([])).toThrow("no parts returned")
   })
 
-  test("throws with part types on unhandled parts", () => {
+  test("returns null for step-start only", () => {
     const parts = [createStepStartPart()]
-    expect(() => extractResponseText(parts)).toThrow("Part types found: [step-start]")
+    expect(extractResponseText(parts)).toBeNull()
+  })
+
+  test("returns null for step-finish only", () => {
+    const parts = [createStepFinishPart()]
+    expect(extractResponseText(parts)).toBeNull()
+  })
+
+  test("returns null for step-start and step-finish", () => {
+    const parts = [createStepStartPart(), createStepFinishPart()]
+    expect(extractResponseText(parts)).toBeNull()
+  })
+
+  test("returns text from multi-step response", () => {
+    const parts = [
+      createStepStartPart(),
+      createToolPart("read", "src/file.ts"),
+      createTextPart("Done"),
+      createStepFinishPart(),
+    ]
+    expect(extractResponseText(parts)).toBe("Done")
   })
 
   test("prefers text over reasoning when both present", () => {