fix(chat): keep each tool's marker under its own card in back-to-back dispatches

When the model dispatched two Bash tools in quick succession, late ToolProgress chunks for the first tool no longer matched the current toolStreamID, so streamToolOutput fell through to the generic collapse-then-append path. The fresh live block landed at the tail of the transcript under the second tool's card, and the first tool's collapsed ⎿ marker stacked beneath the second card as well — making the two runs visually indistinguishable. The fix threads the slot beginToolRunning recorded for the first dispatch through shellTranscriptIdx. When a late progress (or late result) for that id arrives, streamToolOutput and collapseToolOutput now reuse the recorded slot instead of appending, so each tool's live block and final summary stay directly under its own card regardless of dispatch/progress arrival order. Adds TestConsecutiveToolCallsKeepMarkersUnderOwnCard to lock the behaviour in: it verifies both markers are present and that the first card's marker previews the full output, not just the chunk visible before the second tool took over.
2026-11-08 01:48:26 -10:00 · 2026-06-11 13:02:53 +08:00
1 parent 2a7d83f465
commit 8b1dcd776b
@@ -16,6 +16,9 @@ func newTestChatTUI() chatTUI {
 	ti := textarea.New()
 	configureChatTextarea(&ti)
 	ti.SetWidth(80)
+	shellIdx := map[string]int{}
+	shellOut := map[string]string{}
+	shellExp := map[string]bool{}
 	return chatTUI{
 		input:                ti,
 		width:                80,
@@ -30,6 +33,9 @@ func newTestChatTUI() chatTUI {
 		pending:              &strings.Builder{},
 		pendingCommit:        &commit,
 		renderer:             newMarkdownRenderer(80),
+		shellOutputs:         shellOut,
+		shellExpanded:        shellExp,
+		shellTranscriptIdx:   shellIdx,
 	}
 }

@@ -227,6 +233,79 @@ func TestToolWorkingLineThenClears(t *testing.T) {
 	}
 }

+// TestConsecutiveToolCallsKeepMarkersUnderOwnCard is a regression test for
+// back-to-back Bash tool calls. Before the fix, the late ToolProgress for
+// the first tool (already superseded in the controller by a second
+// ToolDispatch) appended a fresh live block at the end of the transcript
+// under the *second* tool's card. Both "⎿" markers then stacked at the
+// end, hiding which run produced which output. The fix threads the
+// transcript slot through shellTranscriptIdx so each tool's live block
+// stays directly under its own card regardless of the dispatch/progress
+// arrival order.
+func TestConsecutiveToolCallsKeepMarkersUnderOwnCard(t *testing.T) {
+	m := newTestChatTUI()
+	// First bash: dispatched and gets one progress chunk before the second
+	// bash is dispatched, mirroring the model's parallel-tool-call pattern.
+	// The "shell-" prefix ensures streamToolOutput accumulates into
+	// shellOutputs, which collapseShellSlot uses to recover the line count
+	// after the live state has been reset by the second beginToolRunning.
+	m.ingestEvent(event.Event{Kind: event.ToolDispatch, Tool: event.Tool{ID: "shell-1", Name: "bash", Args: `{"command":"git status"}`}})
+	m.ingestEvent(event.Event{Kind: event.ToolProgress, Tool: event.Tool{ID: "shell-1", Output: "On branch main-v2\n"}})
+	// Second bash dispatched before the first finishes; this switches
+	// m.toolStreamID to "shell-2" and resets the live streaming state.
+	m.ingestEvent(event.Event{Kind: event.ToolDispatch, Tool: event.Tool{ID: "shell-2", Name: "bash", Args: `{"command":"git branch -a"}`}})
+	// Late progress for the FIRST bash — the path that previously stacked
+	// its marker under the second card.
+	m.ingestEvent(event.Event{Kind: event.ToolProgress, Tool: event.Tool{ID: "shell-1", Output: "nothing to commit\n"}})
+	// Now finish both; each should collapse in place under its own card.
+	m.ingestEvent(event.Event{Kind: event.ToolResult, Tool: event.Tool{ID: "shell-1", Name: "bash", Output: "On branch main-v2\nnothing to commit\n"}})
+	m.ingestEvent(event.Event{Kind: event.ToolResult, Tool: event.Tool{ID: "shell-2", Name: "bash", Output: "* main-v2\n"}})
+
+	// Locate each tool's card. With the fix the transcript is exactly
+	// [card1, marker1, "", card2, marker2] — 5 lines, one marker per
+	// card. Without the fix the late progress overwrites the last slot
+	// in place (or appends), so the first card's slot is left holding
+	// only the first live chunk, and both markers end up at the tail.
+	transcript := m.transcript
+	idx1, idx2 := -1, -1
+	for i, ln := range transcript {
+		if idx1 == -1 && strings.Contains(ln, "git status") {
+			idx1 = i
+		}
+		if idx2 == -1 && strings.Contains(ln, "git branch -a") {
+			idx2 = i
+		}
+	}
+	if idx1 < 0 || idx2 < 0 || idx2 <= idx1 {
+		t.Fatalf("expected two bash cards in dispatch order, got idx1=%d idx2=%d\n%s", idx1, idx2, strings.Join(transcript, "\n"))
+	}
+
+	// Each card must be followed by its own ⎿-prefixed marker slot —
+	// not just "some marker somewhere after the second card".
+	for _, pair := range []struct {
+		card string
+		idx  int
+	}{
+		{card: "git status", idx: idx1},
+		{card: "git branch -a", idx: idx2},
+	} {
+		next := transcript[pair.idx+1]
+		if !strings.Contains(next, "⎿") {
+			t.Fatalf("%q's marker should be at transcript[%d] with the ⎿ connector, got %q\nfull transcript:\n%s",
+				pair.card, pair.idx+1, next, strings.Join(transcript, "\n"))
+		}
+	}
+
+	// The first card's marker must reflect the full output of the first
+	// run ("On branch main-v2" AND "nothing to commit"), not just the
+	// first chunk. The bug left only the pre-late-progress chunk in
+	// transcript[idx1+1], so the second line would be missing.
+	marker1 := transcript[idx1+1]
+	if !strings.Contains(marker1, "On branch main-v2") || !strings.Contains(marker1, "nothing to commit") {
+		t.Fatalf("first card's marker should preview the full output of shell-1, got %q", marker1)
+	}
+}
+
 func TestTodoPanelKeepsLastSuccessfulTodoWrite(t *testing.T) {
 	m := newTestChatTUI()
 	initial := `{"todos":[{"content":"Sync main-v2","status":"in_progress"},{"content":"Push origin","status":"pending"}]}`
@@ -1624,16 +1624,32 @@ func (m *chatTUI) streamToolOutput(id, chunk string) {
 		return
 	}
 	if m.toolStreamID != id {
-		m.collapseToolOutput(m.toolStreamID)
-		m.toolStreamID = id
-		m.toolTail = m.toolTail[:0]
-		m.toolPartial = ""
-		m.toolLineCount = 0
-		if m.nativeScrollback {
-			m.toolStreamIdx = -1
+		// Switching to a different id means either:
+		//   (a) the previous tool finished and a new one is starting — collapse
+		//       the current id's live block, then append a fresh slot at the
+		//       end of the transcript.
+		//   (b) late ToolProgress for an earlier (already dispatched and
+		//       possibly collapsed) tool — reuse the slot beginToolRunning
+		//       already wrote for that id, so the live block stays directly
+		//       under the earlier tool's card rather than stacking at the end.
+		if existingIdx, ok := m.shellTranscriptIdx[id]; ok && existingIdx >= 0 && existingIdx < len(m.transcript) {
+			m.toolStreamID = id
+			m.toolStreamIdx = existingIdx
+			m.toolTail = m.toolTail[:0]
+			m.toolPartial = ""
+			m.toolLineCount = 0
 		} else {
-			m.toolStreamIdx = len(m.transcript)
-			m.commitLine("")
+			m.collapseToolOutput(m.toolStreamID)
+			m.toolStreamID = id
+			m.toolTail = m.toolTail[:0]
+			m.toolPartial = ""
+			m.toolLineCount = 0
+			if m.nativeScrollback {
+				m.toolStreamIdx = -1
+			} else {
+				m.toolStreamIdx = len(m.transcript)
+				m.commitLine("")
+			}
 		}
 	}
 	// Accumulate full output for shell commands so Ctrl+B can expand it.
@@ -1723,19 +1739,63 @@ func (m *chatTUI) collapseToolOutput(id string) {
 		return
 	}
 	if m.toolStreamIdx < 0 || id == "" || m.toolStreamID != id {
+		// The slot is no longer active (e.g. another tool has since taken
+		// over via beginToolRunning, or this id was never streamed). If we
+		// still have a transcript index for it — set by beginToolRunning
+		// when the dispatch landed — collapse in place so a late
+		// ToolResult for a back-to-back tool doesn't leave the previous
+		// tool's live block as raw streamed text.
+		if idx, ok := m.shellTranscriptIdx[id]; ok && idx >= 0 && idx < len(m.transcript) {
+			m.collapseShellSlot(id, idx)
+		}
 		return
 	}
-	n := m.toolLineCount
-	if m.toolPartial != "" {
-		n++
+	m.collapseShellSlot(id, m.toolStreamIdx)
+	m.transcriptDirty = true
+	m.toolStreamIdx = -1
+	m.toolStreamID = ""
+	m.toolTail = m.toolTail[:0]
+	m.toolPartial = ""
+	m.toolLineCount = 0
+}
+
+// collapseShellSlot finalises a tool's live block at idx. Used both by the
+// active-tool path (idx == toolStreamIdx, with the streaming state intact)
+// and the late-result path (idx was recorded in shellTranscriptIdx when the
+// tool was first dispatched). The active path uses the live streaming
+// state (m.toolLineCount + m.toolPartial) for the line count; the late
+// path derives it from the accumulated shellOutputs map (only populated
+// for "shell-" prefixed ids), since toolTail/toolLineCount are reset
+// whenever a new tool takes over via beginToolRunning.
+func (m *chatTUI) collapseShellSlot(id string, idx int) {
+	n := -1
+	if id == m.toolStreamID {
+		n = m.toolLineCount
+		if m.toolPartial != "" {
+			n++
+		}
+	}
+	if n < 0 {
+		// Late-result path. shellOutputs is the most reliable record of
+		// what the tool actually emitted, but it's only populated for
+		// shell-prefixed ids; for other tools we fall back to leaving
+		// the slot blank.
+		if full, ok := m.shellOutputs[id]; ok {
+			n = len(strings.Split(strings.TrimRight(full, "\n"), "\n"))
+		}
 	}
 	if n == 0 {
-		if m.toolStreamIdx == len(m.transcript)-1 {
-			m.transcript = m.transcript[:m.toolStreamIdx]
-		} else {
-			m.transcript[m.toolStreamIdx] = ""
-		}
-	} else if full, ok := m.shellOutputs[id]; ok {
+		// The live block was a "working…" placeholder for a tool that
+		// finished with no output. Clear the rendered text so "working"
+		// disappears from scrollback, but keep the slot in place: the
+		// transcript index is still recorded in shellTranscriptIdx so a
+		// late ToolProgress for this id can land here. If no late
+		// progress ever arrives, this slot becomes a blank line — visible
+		// but harmless.
+		m.transcript[idx] = ""
+		return
+	}
+	if full, ok := m.shellOutputs[id]; ok {
 		// Shell command: show first N lines + hint.
 		lines := strings.Split(strings.TrimRight(full, "\n"), "\n")
 		total := len(lines)
@@ -1745,24 +1805,18 @@ func (m *chatTUI) collapseToolOutput(id string) {
 				preview[i] = dim(clampPlain(lines[i], m.width-len([]rune(connector))))
 			}
 			preview[shellPreviewLines] = dim(fmt.Sprintf("… %d more lines (click/Ctrl+B)", total-shellPreviewLines))
-			m.transcript[m.toolStreamIdx] = connectorBlock(preview)
+			m.transcript[idx] = connectorBlock(preview)
 		} else {
 			rendered := make([]string, total)
 			for i, ln := range lines {
 				rendered[i] = dim(clampPlain(ln, m.width-len([]rune(connector))))
 			}
-			m.transcript[m.toolStreamIdx] = connectorBlock(rendered)
+			m.transcript[idx] = connectorBlock(rendered)
 		}
-		m.shellTranscriptIdx[id] = m.toolStreamIdx
 	} else {
-		m.transcript[m.toolStreamIdx] = connectorBlock([]string{dim(fmt.Sprintf("%d lines", n))})
+		m.transcript[idx] = connectorBlock([]string{dim(fmt.Sprintf("%d lines", n))})
 	}
-	m.transcriptDirty = true
-	m.toolStreamIdx = -1
-	m.toolStreamID = ""
-	m.toolTail = m.toolTail[:0]
-	m.toolPartial = ""
-	m.toolLineCount = 0
+	m.shellTranscriptIdx[id] = idx
 }

 // toggleShellOutput expands or collapses the output of the most recent shell
@@ -1849,6 +1903,12 @@ func (m *chatTUI) beginToolRunning(id string) {
 	}
 	m.toolStreamIdx = len(m.transcript)
 	m.commitLine(connectorBlock([]string{dim(fmt.Sprintf(i18n.M.ChatToolWorkingFmt, toolWorkingFrames[0], 0))}))
+	// Remember the transcript slot for this id so a late ToolProgress for a
+	// previously dispatched (and possibly already collapsed) tool can reuse
+	// it instead of appending a fresh slot at the end of the transcript. For
+	// back-to-back tool calls this keeps each tool's live block directly
+	// under its own card.
+	m.shellTranscriptIdx[id] = m.toolStreamIdx
 }

 // tickToolRunning re-renders the working line of a tool that's dispatched but