diff --git a/cmd/harnesscli/tui/messagebubble_integration_test.go b/cmd/harnesscli/tui/messagebubble_integration_test.go index 4e7a29e8..fd1b411c 100644 --- a/cmd/harnesscli/tui/messagebubble_integration_test.go +++ b/cmd/harnesscli/tui/messagebubble_integration_test.go @@ -48,6 +48,62 @@ func TestRegression_MessageBubbleStreamingPreservesTranscriptEntries(t *testing. } } +// TestRegression_FinalOnlyAssistantMessagesPreserveTwoTurnConversation proves +// that terminal-only providers render and export a complete two-turn exchange +// without duplicating either final assistant response. +func TestRegression_FinalOnlyAssistantMessagesPreserveTwoTurnConversation(t *testing.T) { + m := initModel(t, 100, 30).WithCancelRun(func() {}) + + turns := []struct { + user, runID, assistant string + }{ + {"FIRST_USER_SENTINEL", "run-final-only-one", "FIRST_ASSISTANT_FINAL_SENTINEL"}, + {"SECOND_USER_SENTINEL", "run-final-only-two", "SECOND_ASSISTANT_FINAL_SENTINEL"}, + } + for _, turn := range turns { + next, _ := m.Update(inputarea.CommandSubmittedMsg{Value: turn.user}) + m = next.(tui.Model) + next, _ = m.Update(tui.RunStartedMsg{RunID: turn.runID}) + m = next.(tui.Model) + next, _ = m.Update(tui.SSEEventMsg{ + EventType: "assistant.message", + Raw: []byte(fmt.Sprintf(`{"content":%q}`, turn.assistant)), + }) + m = next.(tui.Model) + next, _ = m.Update(tui.SSEDoneMsg{EventType: "run.completed"}) + m = next.(tui.Model) + } + + wantTranscript := []struct{ role, content string }{ + {"user", "FIRST_USER_SENTINEL"}, + {"assistant", "FIRST_ASSISTANT_FINAL_SENTINEL"}, + {"user", "SECOND_USER_SENTINEL"}, + {"assistant", "SECOND_ASSISTANT_FINAL_SENTINEL"}, + } + transcript := m.Transcript() + if len(transcript) != len(wantTranscript) { + t.Fatalf("transcript = %+v, want exactly user/assistant/user/assistant", transcript) + } + for i, want := range wantTranscript { + if transcript[i].Role != want.role || transcript[i].Content != want.content { + t.Fatalf("transcript[%d] = %+v, want role=%q content=%q", i, transcript[i], want.role, want.content) + } + } + + view := m.View() + previous := -1 + for _, want := range wantTranscript { + if got := strings.Count(view, want.content); got != 1 { + t.Fatalf("viewport renders %q %d times, want once; view=%q", want.content, got, view) + } + position := strings.Index(view, want.content) + if position <= previous { + t.Fatalf("viewport order is not user/assistant/user/assistant; view=%q", view) + } + previous = position + } +} + func TestRegression_MessageBubbleStreamingKeepsViewportAtBottom(t *testing.T) { m := initModel(t, 80, 24) diff --git a/cmd/harnesscli/tui/model.go b/cmd/harnesscli/tui/model.go index 8591687a..e6108d39 100644 --- a/cmd/harnesscli/tui/model.go +++ b/cmd/harnesscli/tui/model.go @@ -189,15 +189,19 @@ type Model struct { // lastAssistantText accumulates all assistant deltas for the current run. lastAssistantText string - // responseStarted tracks whether the first assistant delta for the current - // run has been written to the viewport. On the first delta we call - // the messagebubble renderer and then replace only the active assistant tail. + // responseStarted tracks whether the assistant bubble for the current + // provider step has been written to the viewport. On the first delta we + // append it; later deltas replace only that active assistant tail. responseStarted bool // activeAssistantLineCount tracks how many viewport lines belong to the // currently streaming assistant bubble. activeAssistantLineCount int + // assistantTranscriptFinalized prevents replayed terminal events from + // recording the current run's assistant response more than once. + assistantTranscriptFinalized bool + // thinkingText accumulates reasoning deltas for the current turn. thinkingText string @@ -1681,6 +1685,11 @@ func (m *Model) handleToolStart(callID, name string, input json.RawMessage) { m.toolViews[callID] = view m.activeToolCallID = callID m.appendToolUseView(view) + // The tool card now owns the viewport tail. Close any assistant bubble from + // the preceding provider step so the next step appends a new bubble instead + // of replacing the tool card with stale tail-line ownership. + m.responseStarted = false + m.activeAssistantLineCount = 0 } func (m *Model) handleToolChunk(callID, chunk string) { @@ -3858,6 +3867,8 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case RunStartedMsg: m.RunID = msg.RunID m.runActive = true + m.lastAssistantText = "" + m.assistantTranscriptFinalized = false m.clearThinkingBar() m.spinner = spinner.New(spinnerSeed(m.config)).WithStyles(spinnerStylesFromTheme(m.theme)).Start() cmds = append(cmds, spinnerTickCmd()) @@ -4238,7 +4249,11 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { var p struct { Content string `json:"content"` } - if err := json.Unmarshal(msg.Raw, &p); err == nil && p.Content != "" { + if err := json.Unmarshal(msg.Raw, &p); err == nil && p.Content != "" && + !m.assistantTranscriptFinalized { + if !m.responseStarted { + m.lastAssistantText = "" + } // Accumulate and re-render the assistant message through the // glamour-backed message bubble. Re-rendering the full // accumulated text each delta (rather than appending raw chunks @@ -4247,6 +4262,19 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.lastAssistantText += p.Content m.renderActiveAssistantBubble() } + case "assistant.message": + var p struct { + Content string `json:"content"` + } + if err := json.Unmarshal(msg.Raw, &p); err == nil && p.Content != "" && + !m.assistantTranscriptFinalized && + (!m.responseStarted || p.Content != m.lastAssistantText) { + // assistant.message is the authoritative full response. Most + // providers precede it with deltas, but valid non-streaming + // providers may emit only this terminal message. + m.lastAssistantText = p.Content + m.renderActiveAssistantBubble() + } case "assistant.thinking.delta": var p struct { Content string `json:"content"` @@ -4476,12 +4504,13 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.cancelRun = nil } // Record completed assistant response in transcript. - if m.lastAssistantText != "" { + if m.lastAssistantText != "" && !m.assistantTranscriptFinalized { m.transcript = append(m.transcript, transcriptexport.TranscriptEntry{ Role: "assistant", Content: m.lastAssistantText, Timestamp: time.Now(), }) + m.assistantTranscriptFinalized = true } if msg.EventType == "run.failed" { for _, line := range formatRunError(msg.Error) { diff --git a/cmd/harnesscli/tui/run_control_command_test.go b/cmd/harnesscli/tui/run_control_command_test.go index 24f1b1c9..c80a868a 100644 --- a/cmd/harnesscli/tui/run_control_command_test.go +++ b/cmd/harnesscli/tui/run_control_command_test.go @@ -171,6 +171,58 @@ func TestRunControl_ResumeCommandStartsContinuationRun(t *testing.T) { } } +func TestRegression_ResumeWithoutAssistantContentDoesNotDuplicatePriorReply(t *testing.T) { + for _, terminalEvent := range []string{"run.completed", "run.failed"} { + t.Run(terminalEvent, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/v1/runs/run_prev/continue" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"run_id":"run_next","status":"queued"}`)) + })) + defer srv.Close() + + m := testRunControlModel(srv.URL).WithCancelRun(func() {}) + started, _ := m.Update(RunStartedMsg{RunID: "run_prev"}) + m = started.(Model) + assistant, _ := m.Update(SSEEventMsg{ + EventType: "assistant.message", + Raw: []byte(`{"content":"PRIOR_ASSISTANT_REPLY"}`), + }) + m = assistant.(Model) + completed, _ := m.Update(SSEDoneMsg{EventType: "run.completed"}) + m = completed.(Model) + // SSEDoneMsg consumes and clears the active run's cancel function. + // Reinstall the test seam so the continuation's RunStartedMsg does + // not open a real SSE bridge against this command-only HTTP server. + m = m.WithCancelRun(func() {}) + + cmds, quit := executeResumeCommand(&m, Command{Name: "resume", Args: []string{"run_prev", "continue", "without", "a", "reply"}}) + if quit { + t.Fatal("/resume must not quit") + } + continuationStarted := lastCmd(t, cmds)() + next, _ := m.Update(continuationStarted) + m = next.(Model) + terminal, _ := m.Update(SSEDoneMsg{EventType: terminalEvent, Error: "continuation failed"}) + m = terminal.(Model) + + transcript := m.Transcript() + if len(transcript) != 2 { + t.Fatalf("transcript = %+v, want prior assistant reply and continuation user prompt only", transcript) + } + if transcript[0].Role != "assistant" || transcript[0].Content != "PRIOR_ASSISTANT_REPLY" { + t.Fatalf("prior assistant transcript = %+v", transcript[0]) + } + if transcript[1].Role != "user" || transcript[1].Content != "continue without a reply" { + t.Fatalf("continuation user transcript = %+v", transcript[1]) + } + }) + } +} + func TestRunControl_RunsSnapshot80x24(t *testing.T) { writeRunsSnapshot(t, 80, 24, "TUI-058-runs-80x24.txt") } diff --git a/cmd/harnesscli/tui/sse_events_test.go b/cmd/harnesscli/tui/sse_events_test.go index 9eb84add..f5d272a3 100644 --- a/cmd/harnesscli/tui/sse_events_test.go +++ b/cmd/harnesscli/tui/sse_events_test.go @@ -378,6 +378,109 @@ func TestSSEDropMsg_NilSseCh_NoCmdRequired(t *testing.T) { } } +// --------------------------------------------------------------------------- +// assistant.message — terminal full-message reconciliation (issue #1056) +// --------------------------------------------------------------------------- + +func TestSSEEventMsg_AssistantMessage_FinalOnlyRendersAndFinalizesOnce(t *testing.T) { + m := initModel(t, 80, 24).WithCancelRun(func() {}) + m2, _ := m.Update(tui.RunStartedMsg{RunID: "run-final-only"}) + model := m2.(tui.Model) + + m3, _ := model.Update(tui.SSEEventMsg{EventType: "assistant.message", Raw: []byte(`{"content":"LUNA_NONSTREAM_REPLY"}`)}) + model = m3.(tui.Model) + if !model.RunActive() || model.LastAssistantText() != "LUNA_NONSTREAM_REPLY" { + t.Fatalf("terminal message did not remain active with final text: active=%v text=%q", model.RunActive(), model.LastAssistantText()) + } + if got := strings.Count(model.View(), "LUNA_NONSTREAM_REPLY"); got != 1 { + t.Fatalf("terminal-only assistant message rendered %d times, want once; view=%q", got, model.View()) + } + m4, _ := model.Update(tui.SSEDoneMsg{EventType: "run.completed"}) + model = m4.(tui.Model) + if got := model.Transcript(); len(got) != 1 || got[0].Role != "assistant" || got[0].Content != "LUNA_NONSTREAM_REPLY" { + t.Fatalf("transcript = %+v, want one terminal assistant reply", got) + } +} + +func TestSSEEventMsg_AssistantMessage_DeltaThenFinalDoesNotDuplicate(t *testing.T) { + m := initModel(t, 80, 24).WithCancelRun(func() {}) + m2, _ := m.Update(tui.RunStartedMsg{RunID: "run-streamed-final"}) + model := m2.(tui.Model) + for _, event := range []tui.SSEEventMsg{ + {EventType: "assistant.message.delta", Raw: []byte(`{"content":"streamed reply"}`)}, + {EventType: "assistant.message", Raw: []byte(`{"content":"streamed reply"}`)}, + } { + next, _ := model.Update(event) + model = next.(tui.Model) + } + if got := strings.Count(model.View(), "streamed reply"); got != 1 { + t.Fatalf("delta-plus-final reply rendered %d times, want once; view=%q", got, model.View()) + } + done, _ := model.Update(tui.SSEDoneMsg{EventType: "run.completed"}) + if got := done.(tui.Model).Transcript(); len(got) != 1 || got[0].Content != "streamed reply" { + t.Fatalf("transcript = %+v, want one reconciled reply", got) + } +} + +func TestSSEEventMsg_AssistantMessage_FinalContentIsAuthoritative(t *testing.T) { + m := initModel(t, 80, 24).WithCancelRun(func() {}) + m2, _ := m.Update(tui.RunStartedMsg{RunID: "run-final-authoritative"}) + model := m2.(tui.Model) + for _, event := range []tui.SSEEventMsg{ + {EventType: "assistant.message.delta", Raw: []byte(`{"content":"partial"}`)}, + {EventType: "assistant.message", Raw: []byte(`{"content":"partial response complete"}`)}, + } { + next, _ := model.Update(event) + model = next.(tui.Model) + } + if got := model.LastAssistantText(); got != "partial response complete" { + t.Fatalf("LastAssistantText() = %q, want authoritative final content", got) + } + if got := strings.Count(model.View(), "partial response complete"); got != 1 { + t.Fatalf("authoritative final reply rendered %d times, want once; view=%q", got, model.View()) + } +} + +func TestSSEEventMsg_AssistantMessage_MixedToolStepPreservesViewportAndReplayIdempotency(t *testing.T) { + m := initModel(t, 120, 40).WithCancelRun(func() {}) + m2, _ := m.Update(tui.RunStartedMsg{RunID: "run-mixed-terminal"}) + model := m2.(tui.Model) + events := []tui.SSEEventMsg{ + {EventType: "assistant.message.delta", Raw: []byte(`{"content":"EARLY_STREAMED_STEP"}`)}, + {EventType: "tool.call.started", Raw: []byte(`{"tool":"bash","call_id":"call-mixed","arguments":{"command":"printf tool-card"}}`)}, + {EventType: "tool.call.completed", Raw: []byte(`{"tool":"bash","call_id":"call-mixed","output":"TOOL_OUTPUT_SENTINEL","duration_ms":8}`)}, + {EventType: "assistant.message", Raw: []byte(`{"content":"FINAL_ONLY_AFTER_TOOL"}`)}, + } + for _, event := range events { + next, _ := model.Update(event) + model = next.(tui.Model) + } + // Replayed tool completion, terminal event, and completion must be harmless. + for _, event := range events[len(events)-2:] { + next, _ := model.Update(event) + model = next.(tui.Model) + } + next, _ := model.Update(tui.SSEDoneMsg{EventType: "run.completed"}) + model = next.(tui.Model) + next, _ = model.Update(events[len(events)-1]) + model = next.(tui.Model) + next, _ = model.Update(tui.SSEDoneMsg{EventType: "run.completed"}) + model = next.(tui.Model) + + view := model.View() + for _, marker := range []string{"EARLY_STREAMED_STEP", "bash(", "FINAL_ONLY_AFTER_TOOL"} { + if got := strings.Count(view, marker); got != 1 { + t.Fatalf("%q rendered %d times, want once; view=%q", marker, got, view) + } + } + if !(strings.Index(view, "EARLY_STREAMED_STEP") < strings.Index(view, "bash(") && strings.Index(view, "bash(") < strings.Index(view, "FINAL_ONLY_AFTER_TOOL")) { + t.Fatalf("mixed-step blocks out of order: view=%q", view) + } + if got := model.Transcript(); len(got) != 1 || got[0].Content != "FINAL_ONLY_AFTER_TOOL" { + t.Fatalf("transcript after replay = %+v, want one final assistant response", got) + } +} + // --------------------------------------------------------------------------- // SSEDoneMsg for run.failed — appends formatted failure output and blank line // --------------------------------------------------------------------------- diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index f1c2f947..22a1b114 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -31,6 +31,55 @@ candidate in the required non-TTY foreground host context passed, isolating the red to Keychain session access rather than the parser test. +## 2026-08-01 (Issue #1056 — TUI Terminal Assistant Message) + +- Symptom: real non-streaming runs persisted `assistant.message` followed by + `run.completed`, while the TUI displayed the user prompts but no assistant + reply and exported no assistant transcript row. +- Cause: the SSE bridge forwarded `assistant.message`, but `Model.Update` + reduced only `assistant.message.delta`; valid final-only responses therefore + left `lastAssistantText` empty. +- Current-main red: two complete final-only turns produced only the two user + transcript entries instead of exact user/assistant/user/assistant order. +- Fix: the existing reducer treats non-empty `assistant.message.content` as the + authoritative response and reuses the active bubble renderer. Tool start + closes the prior bubble's viewport-tail ownership, and a per-run finalization + bit makes terminal replay/completion consumptive while reopening on the next + `RunStartedMsg`. +- Regressions: final-only, delta plus identical/differing final, mixed + delta -> tool -> final, replay idempotency, repeated completion, and two-turn + viewport/transcript ordering. +- Focused evidence: the required two-turn red is green; the focused assistant + suite and complete TUI suite pass normal and race. The full repository gate + passes at 85.7% coverage with zero uncovered production functions. +- Real PTY evidence: an isolated exact-candidate `harnessd` and `harnesscli` + rendered `PTY_1059_USER_ONE`, `PTY_1059_REPLY_ONE`, + `PTY_1059_USER_TWO`, `PTY_1059_REPLY_TWO` once and in order. Both runs used + the same conversation id; raw SSE emitted one authoritative + `assistant.message` before `run.completed` per run; HTTP and SQLite stored + exactly four alternating rows; and a fresh `--resume` TUI replayed the four + rows once with an active composer. +- Exact-head review finding: `/resume` appended its user row but did not clear + `lastAssistantText`; `RunStartedMsg` reopened transcript finalization while + retaining the prior reply. A contentless completed or failed continuation + therefore exported that stale reply again. +- Review fix: `RunStartedMsg`, the shared boundary for initial and continuation + API starts, now clears the assistant accumulator before reopening per-run + finalization. The actual `/resume` command regression failed with a third + stale assistant row for both `run.completed` and `run.failed`, then passed + with exactly the prior assistant row and continuation user prompt. The + focused resume, new-content, replay, and two-turn matrix passes normal and + race on the rebased candidate. +- Hosted-test isolation follow-up: the first terminal event correctly consumed + the injected cancel function, so the regression's continuation start opened + a real SSE bridge against its command-only `httptest.Server`; GitHub race + exposed the unexpected `GET /v1/runs/run_next/events`. Reinstalling the + cancel seam between runs keeps the test on its intended reducer/API path. + The focused regression passes 20 normal and 10 race repetitions. The same + hosted wave also hit the unrelated workflow subscriber-close timing test; + that baseline test passed 20 normal and 10 race repetitions locally and is + being rechecked independently rather than waived. + ## 2026-07-31 (Workflow Initial Write Exit Arbitration — Issue #1076) - Symptom: hosted `test-race` run `30660042116` reported only diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index f62a16fd..776c7081 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -24,6 +24,20 @@ - Guardrails: the strict red is hosted run `30672776651`; do not invent a behavioral red, alter production code, or add a coverage exemption. +## 2026-08-01 (TUI Terminal Assistant Message — Issue #1056) + +- Command intent: restore terminal-only assistant replies in the real TUI and + preserve exact multi-turn conversation state. +- User intent: daemon success is insufficient when a client conversation looks + blank; visible and exported chat must agree with SSE and persistence. +- Success definition: final-only content renders and exports once, + delta-plus-final remains idempotent, tool-card order survives later provider + content and replay, and two complete turns remain exactly + user/assistant/user/assistant through focused, race, full, and real PTY proof. +- Guardrails: issue #1056, isolated worktree, behavior-level red before the + reducer change, no server/provider/schema expansion, and no failing-baseline + waiver. + ## 2026-07-31 (Workflow Initial Write Exit Arbitration — Issue #1076) - Command intent: repair the separate hosted race failure where the initial diff --git a/docs/logs/observational-log.md b/docs/logs/observational-log.md index 60b62fcb..79fdd4d1 100644 --- a/docs/logs/observational-log.md +++ b/docs/logs/observational-log.md @@ -12,6 +12,28 @@ Use this file for observations about system behavior without immediately prescribing code changes. +## 2026-08-01 (Issue #1056 Final-Only TUI Reproduction) + +- Raw run/conversation SSE and stored messages already carry the correct + `assistant.message` before `run.completed`; the missing reply is isolated to + the TUI reducer. +- The deterministic current-main two-turn reproduction renders and records both + user prompts but records neither final-only assistant reply. +- A streamed assistant step followed by a tool card disproves the assumption + that the assistant bubble always owns the viewport tail; later provider + content must append after the card. +- Replayed terminal messages and completion events require separate visual and + transcript idempotency; preserving `lastAssistantText` for copy cannot make it + repeatedly consumable. +- The exact-candidate real PTY confirmed the repaired reducer agrees with both + durable surfaces: two `assistant.message` SSE events became two assistant + rows in the same four-message conversation, and reconnect replay introduced + no visible duplicate. +- A continuation can terminate successfully or fail without emitting new + assistant content. Before the exact-head review fix, reopening finalization + without clearing the accumulator made either terminal path re-export the + previous run's reply; run start must reset both pieces of per-run ownership. + ## 2026-07-31 (Source-Workflow Initial Write Lifecycle) - Lifecycle observation: a successful `cmd.Start` transfers child ownership to diff --git a/docs/logs/system-log.md b/docs/logs/system-log.md index 7cda7714..f8dcad1a 100644 --- a/docs/logs/system-log.md +++ b/docs/logs/system-log.md @@ -15,6 +15,22 @@ coverage after normal and race suites completed; the repair does not change runtime behavior, persistence, or secrets handling. +## 2026-08-01 (TUI Assistant Response Reconciliation) + +- System: run SSE -> TUI bridge -> `Model.Update` -> viewport bubble -> + `SSEDoneMsg` -> transcript export. +- `assistant.message.delta` builds incremental content; + `assistant.message` is the authoritative full response and may arrive without + deltas. +- Tool start is an assistant-tail ownership boundary. A later provider response + begins a new bubble and cannot replace the intervening tool card. +- Terminal transcript finalization consumes the current run once; replayed + assistant/completion events are no-ops, and `RunStartedMsg` opens the next + run's lifecycle by clearing both the prior assistant accumulator and its + finalization state. This boundary covers initial and continuation API starts. +- Server emission, persistence, authentication, provider behavior, and other + clients remain unchanged. + ## 2026-07-31 (Source-Workflow Initial Write Lifecycle) - System/component: `internal/workflow.SourceManager.runSourceWorkflow`, the diff --git a/docs/plans/2026-07-31-issue-1056-terminal-assistant-message-impact-map.md b/docs/plans/2026-07-31-issue-1056-terminal-assistant-message-impact-map.md new file mode 100644 index 00000000..9560ee3c --- /dev/null +++ b/docs/plans/2026-07-31-issue-1056-terminal-assistant-message-impact-map.md @@ -0,0 +1,80 @@ +# Issue #1056 Terminal Assistant Message Impact Map + +## Task + +- Issue: #1056 +- PR: #1059 +- Plan: `2026-07-31-issue-1056-terminal-assistant-message-plan.md` +- Status: current-main integration candidate; unmerged. + +## Ownership and Data Flow + +- Entry: run SSE decoded by `StartSSEBridgeWithOptions`. +- Owner: `cmd/harnesscli/tui.Model.Update` owns visible and transcript + reduction; `lastAssistantText` is the existing response source of truth. +- Flow: SSE -> `SSEEventMsg` -> assistant bubble -> `SSEDoneMsg` -> transcript + export entry. +- Search evidence: `rg -n "assistant\\.message(\\.delta)?|run\\.completed" cmd internal macapp scripts`. +- Conclusion: repair the existing reducer; no parallel state owner is needed. + +## Config, API, CLI, and Tools + +- Config/env/defaults: None; searched TUI and fake-provider configuration. +- API/wire formats: unchanged; consume existing `{content}` on + `assistant.message`. +- Commands/tools: no flag, slash command, or tool change. +- Validation: malformed/empty terminal payloads remain no-ops. + +## Persistence and Compatibility + +- Schema/migration/cache: None; HTTP and SQLite transcripts already contain the + correct assistant content. +- Compatibility: streamed delta providers remain supported; final-only + providers become visible. Older clients continue to miss this event without + server coordination. + +## Lifecycle, Reliability, and Security + +- Lifecycle: preserve polling, Last-Event-ID reconnect, run-active cleanup, + tool-card offsets, later-run reset, and copy access to the last response. +- Idempotency: identical delta/final and replay are no-ops; a differing final is + authoritative; repeated completion appends one transcript row. +- Security/auth/privacy: None; only already-authorized response content is + reduced and no new logging contains prompts or credentials. + +## Product Surfaces + +- TUI: affected. +- Server, provider, web, macOS, persistence, provider/model/tool catalog: None + after symbol/caller search; they are compatibility references only. +- UX: the missing assistant bubble appears and the composer remains usable for + later turns; no keyboard/focus/motion change. + +## Deployment and Operations + +- Release: ordinary `harnesscli` binary; no migration or feature flag. +- Evidence: pane capture plus raw SSE, run JSON, HTTP/SQLite transcript, and + daemon logs. +- Rollback trigger: any duplicate, missing, or reordered streamed response or + tool card. Revert the reducer slice; no data repair. + +## Tests + +- Red-first: two final-only turns currently produce only user transcript rows. +- New behavior: exact user/assistant/user/assistant viewport and transcript, + final-only, delta/final, authoritative replacement, mixed tool lifecycle, + replay, repeated completion, and later-run reset. +- Commands: + - `go test ./cmd/harnesscli/tui -run 'TestRegression_FinalOnlyAssistantMessagesPreserveTwoTurnConversation|TestSSEEventMsg_AssistantMessage' -count=1` + - `go test ./cmd/harnesscli/tui -count=1` + - `go test -race ./cmd/harnesscli/tui -count=1` + - `go test ./cmd/harnesscli/... -count=1` + - `./scripts/test-regression.sh` +- Real path: two turns through a tmux-hosted `harnessd` and + `harnesscli -tui`, correlated with run IDs, SSE, API, and SQLite. + +## Documentation + +- Update engineering, observational, system, and long-term-thinking logs plus + plans index and active plan. +- Public docs/runbooks: None; this repairs an existing event contract. diff --git a/docs/plans/2026-07-31-issue-1056-terminal-assistant-message-plan.md b/docs/plans/2026-07-31-issue-1056-terminal-assistant-message-plan.md new file mode 100644 index 00000000..2713db3f --- /dev/null +++ b/docs/plans/2026-07-31-issue-1056-terminal-assistant-message-plan.md @@ -0,0 +1,82 @@ +# Issue #1056: Terminal Assistant Message Reconciliation + +## Context + +- Governing issue: https://github.com/dennisonbertram/go-code/issues/1056 +- Pull request: https://github.com/dennisonbertram/go-code/pull/1059 +- Problem: the TUI reduces `assistant.message.delta` but not the valid + authoritative `assistant.message`, so a non-streaming provider can complete + and persist a reply without showing or exporting it. +- User impact: a successful turn looks blank and later conversation state is + not trustworthy from the TUI. + +## Scope + +- In scope: + - reconcile non-empty terminal `assistant.message` content into the active + assistant bubble; + - keep delta-plus-final and replay delivery idempotent; + - preserve an earlier streamed bubble, intervening tool card, and a later + terminal-only response in exact viewport order; + - finalize repeated `run.completed` once while allowing a later run to record + its own reply; + - clear prior assistant ownership for every new run, including `/resume`, so + a contentless completed or failed continuation cannot export a stale reply; + - prove two complete final-only turns in one conversation. +- Out of scope: + - server/provider event changes or synthetic deltas; + - API, persistence, schema, model routing, web, or macOS changes; + - unrelated TUI rendering cleanup. + +## Test Plan + +- Required red-first behavior: + `TestRegression_FinalOnlyAssistantMessagesPreserveTwoTurnConversation` sends + user -> `run.started` -> `assistant.message` -> `run.completed` twice and + requires exact viewport/transcript order user/assistant/user/assistant. +- Adjacent coverage: + - final-only render and finalize once; + - delta plus identical final does not duplicate; + - a differing final replaces partial content; + - delta -> tool lifecycle -> final-only preserves all blocks and replay is + harmless; + - repeated completion and later-run reset are idempotent. + - `/resume` followed by completed or failed terminal delivery without new + assistant content does not duplicate the previous run's transcript row. +- Required gates: + focused TUI normal/race, complete harnesscli, repository regression, hosted + fast/race, and real PTY multi-turn evidence correlated with SSE/API/SQLite. + +## Implementation + +- Extend the existing `Model.Update(SSEEventMsg)` reducer; do not add another + transcript owner. +- Treat `assistant.message` as the authoritative full response. +- Close assistant-tail ownership when a tool card begins. +- Use a per-run finalization bit to consume transcript append exactly once and + reset it with the assistant accumulator on the next `RunStartedMsg`. + +## Checklist + +- [x] Structured issue and PR-sized scope exist. +- [x] Current ownership and cross-surface impact are recorded. +- [x] Two-turn behavior test failed on current main before implementation. +- [x] Minimal reducer fix and adjacent regressions are green. +- [x] Plan, impact map, logs, and indexes are current. +- [x] Root exact-diff review and independent rereview complete. +- [x] Review-found contentless-resume stale transcript regression is green. +- [x] Full repository regression and real PTY proof pass on the candidate. +- [ ] Hosted checks pass on the final pushed SHA. +- [ ] Production two-pass merge gate passes and closes #1056. + +## Risks and Rollback + +- Duplicate streamed responses: pin delta-plus-final and replay idempotency. +- Tool-card corruption: tool start explicitly closes stale assistant tail + ownership and the mixed-step test pins block order. +- Duplicate transcript rows: finalization is consumptive per run and resets for + the next run together with the assistant accumulator, including continuation + runs that produce no assistant content. +- Roll back the isolated reducer change if any existing streamed reply is lost, + duplicated, or reorders a tool card; no persisted repair or migration is + required. diff --git a/docs/plans/INDEX.md b/docs/plans/INDEX.md index e0fd05f2..d3b799f6 100644 --- a/docs/plans/INDEX.md +++ b/docs/plans/INDEX.md @@ -5,6 +5,8 @@ - `2026-08-01-issue-1081-keychain-parser-coverage-plan.md` — Issue #1081 portable Keychain reference parser coverage and Ubuntu gate repair plan. - `2026-08-01-issue-1081-keychain-parser-coverage-impact-map.md` — Cross-surface impact map for Issue #1081's test-only coverage repair. +- `2026-07-31-issue-1056-terminal-assistant-message-plan.md` — Issue #1056 TUI terminal assistant-message reconciliation plan. +- `2026-07-31-issue-1056-terminal-assistant-message-impact-map.md` — Cross-surface impact map for Issue #1056. - `2026-07-31-issue-1077-d0-logo-plan.md` — Issue #1077 D0 macOS app logo geometry, shared SwiftUI source, and verification status. - `2026-07-31-issue-1077-d0-logo-impact-map.md` — Cross-surface impact map for Issue #1077. - `2026-07-31-issue-1076-workflow-initial-write-exit-plan.md` — Issue #1076 plan for preserving child exit diagnostics across the initial workflow write. diff --git a/docs/plans/active-plan.md b/docs/plans/active-plan.md index 54e4304f..0adc8f24 100644 --- a/docs/plans/active-plan.md +++ b/docs/plans/active-plan.md @@ -35,6 +35,25 @@ x100, hosted-equivalent race, and the full repository gate at 85.7% coverage with zero uncovered production functions. The repair remains local and unpushed; PR #1060 remains excluded and no merge is authorized. +Current status: Issue #1056 terminal assistant-message reconciliation is +rebased onto production main `c10c085d` in an isolated worktree. The +required two-turn final-only behavior test failed before the reducer change and +now passes; focused and complete TUI normal/race suites are green. The full +repository gate passes at 85.7% with zero uncovered production functions, and +the exact-candidate real PTY/API/SQLite/reconnect matrix preserves the exact +four-message conversation without duplicates. Two independent source/diff +reviews found no P1/P2 issue. Focused TUI validation is being rerun on the +rebased candidate. An exact-head review then found contentless `/resume` +completion could duplicate the prior assistant transcript row; the actual +resume-path completed/failed red is green after resetting assistant ownership +at `RunStartedMsg`. The focused resume, new-content, replay, and two-turn matrix +passes normal and race. The isolated review fix is ready for promotion; hosted +and production-main gates remain before PR #1059 may merge. + +Current status: Issue #1067 terminal publication atomicity shipped through PR +#1070 on production main `b45b4334`; local pre/post full regressions and hosted +PR fast/race checks passed. Its issue is closed. + Current status: Issue #1076 isolates the source-workflow lifecycle race where a child exits before the initial `start` write and the early EPIPE return skips wait plus bounded stderr arbitration. Natural-exit and live-child cleanup reds @@ -78,6 +97,7 @@ Remaining work before merge is final verification and any requested review/cleanup. Current active plans: +- `2026-07-31-issue-1056-terminal-assistant-message-plan.md` - `2026-07-31-issue-1067-terminal-status-event-atomicity-plan.md` - `2026-07-31-issue-1076-workflow-initial-write-exit-plan.md` - `2026-07-31-issue-1068-dispatcher-shutdown-isolation-plan.md`