fix(tui): deduplicate assistant messages across sessions.changed reload#96980
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 11:15 AM ET / 15:15 UTC. Summary PR surface: Source +107, Tests +499. Total +606 across 4 files. Reproducibility: yes. at source level: current main and v2026.6.10 clear tracked TUI run state and replay assistant history without runIds on Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Review and land this PR as the candidate fix for #96967 once maintainers accept the runId surrender contract and required checks are green; keep the linked issue open until merge. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main and v2026.6.10 clear tracked TUI run state and replay assistant history without runIds on Is this the best way to solve the issue? Yes: the event-handler runId surrender approach is the best boundary I found because it avoids ChatLog text heuristics and preserves the shipped external new/reset reload path. The main rejected alternatives are text matching in ChatLog and blanket skipping AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 825d9a66623a. Label changesLabel justifications:
Evidence reviewedPR surface: Source +107, Tests +499. Total +606 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
5336b4d to
530c6e7
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
88760fb to
487de8c
Compare
…atLog (openclaw#96967) When sessions.changed triggers loadHistory(), assistant rows are replayed via finalizeAssistant(text) without a runId. A late live final with the same text and a runId can land immediately afterward, producing a visible duplicate assistant reply. Track the most recent history-replay assistant component in ChatLog and suppress a following live final when the text matches and no other component was appended in between. This is simpler and more targeted than the prior event-handler approach because it operates at the rendering layer where the duplicate is actually visible. Fixes openclaw#96967
…racking Drop the lastHistoryAssistant text-matching dedupe in ChatLog (posed P1 false-suppression risk). Instead, track active runIds surrendered to loadHistory() on sessions.changed 'new' — late events for surrendered runs are suppressed by exact runId match. Add ChatLog.hasStreamingRun() so the event handler can detect when loadHistory restored a surrendered run as in-flight streaming. Retain chatFinalizedRuns / chatFinalizedRunsWithDisplay from the original fix for post-reload delta/error dedup of finalized runs. Incorporates the precise surrender-tracking approach from closed PRs openclaw#96979/openclaw#96986 while keeping the scoped ChatLog API minimal.
… run When a sessions.changed 'new' surrenders an in-flight run and loadHistory does not restore it as streaming, a late displayable final must still be rendered — otherwise the user never sees the assistant reply. Only suppress non-displayable finals (empty message, no errorMessage). Add focused test for non-displayable final suppression.
…e final Covers the rank-up scenario from openclaw#96979: surrendered run restored by loadHistory as empty streaming, late displayable final passes through surrender check and renders to chatLog.
…ent history-replay duplicate
When sessions.changed 'new' fires, now surrender chatFinalizedRuns entries
too (as 'finalized'), not just sessionRuns ('in-flight'). This prevents
the finalized-before-reload history-replay duplicate: a late final for an
already-displayed run gets suppressed because the history-replay static
row covers the visible content.
In-flight surrendered runs still render displayable finals because the
user may not have seen the streaming text before the reload.
SurrenderedToHistoryRunIds is now Map<string,'in-flight'|'finalized'>
so the surrender check can branch on source. Move surrender check before
the finalizedRuns/chatFinalizedRuns guard so surrendered finalized runs
are routed correctly.
Add focused tests for history-replayed-visible (suppress) and
history-replay-missing (render) late finals.
…path Reset also calls clearTrackedRunState() + loadHistory() with the same sessionKey, so late chat events for runs from the old session window can race with the history replay the same way as 'new'. Surrender sessionRuns and chatFinalizedRuns on both reasons (openclaw#96979 P1 rank-up).
… later final For finalized surrendered runs, a late delta must not clear the surrender marker — otherwise a subsequent late final would slip through and produce a duplicate. Only in-flight surrendered runs clear the marker on delta (their final needs to render).
Co-authored-by: xialonglee <[email protected]>
487de8c to
c61eec3
Compare
|
Land-ready proof for exact head
Known proof gaps: none. |
|
Merged via squash.
|
Fixes #96967
What Problem This Solves
When an external channel completed an assistant run, the TUI could rebuild persisted history and then process a late live
chat.finalfor the same run. History rows do not carry the live run id, so the same assistant reply could appear twice.Why This Change Was Made
The fix now uses the Gateway's existing terminal
sessions.changedevent as the exact post-persistence barrier. TUI history reloads are serialized by run id; live events arriving during a rebuild are deferred; successful history owns persisted final rows; failed rebuilds retain prior display state; and an explicitly restored in-flight run remains live.This replaces the earlier text-equality, TTL, and ChatLog-specific surrender heuristics with one owner-correct state machine. Reset and session-replacement reloads preserve the distinction between finalized and still-running work, so dedupe does not hide legitimate repeated text or terminate an adopted in-flight run.
User Impact
Assistant replies triggered through Telegram or other external channel paths render once in the TUI.
/reset, session switching, delayed finals, transient history failures, and in-flight run recovery continue to work without new configuration.Evidence
tbx_01kwxd4bqttj4qrkdzpp4x4gve: 151 focused TUI tests passed.cbx_b33918875031, runrun_4490545442ba: a live tmux TUI rendered oneopenai/gpt-5.5reply for an external turn, accepted/reset, then rendered one reply for the second external turn; captured evidence contained no credential value.bc65ea0951513d2c1001a1a6bdade667dc9af03b.Release-note context: TUI external-run history now waits for persisted terminal state before rebuilding, preventing duplicate channel-triggered assistant replies while preserving reset and in-flight recovery.
Thanks @xialonglee for the original fix and reproduction work.
AI-assisted: yes, implementation and review used Codex. No transcript is attached.