Skip to content

fix(tui): deduplicate assistant messages across sessions.changed reload#96980

Merged
steipete merged 15 commits into
openclaw:mainfrom
xialonglee:fix/issue-96967-tui-duplicate-assistant
Jul 7, 2026
Merged

fix(tui): deduplicate assistant messages across sessions.changed reload#96980
steipete merged 15 commits into
openclaw:mainfrom
xialonglee:fix/issue-96967-tui-duplicate-assistant

Conversation

@xialonglee

@xialonglee xialonglee commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

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.final for 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.changed event 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

  • Blacksmith Testbox tbx_01kwxd4bqttj4qrkdzpp4x4gve: 151 focused TUI tests passed.
  • Same Testbox: 26 real PTY E2E tests passed (18 harness tests and 8 real local/Gateway backend tests).
  • Same Testbox: exact changed gate passed, including production/test types, lint, cycle, boundary, and attribution guards.
  • Direct AWS Crabbox cbx_b33918875031, run run_4490545442ba: a live tmux TUI rendered one openai/gpt-5.5 reply for an external turn, accepted /reset, then rendered one reply for the second external turn; captured evidence contained no credential value.
  • Fresh exact-branch autoreview: clean, no actionable findings (0.82 confidence).
  • Review artifacts validate with zero findings at 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.

@openclaw-barnacle openclaw-barnacle Bot added size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 26, 2026
@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 11:15 AM ET / 15:15 UTC.

Summary
The PR adds TUI runId-based surrender tracking around sessions.changed new/reset reloads, exposes ChatLog.hasStreamingRun(), and extends ChatLog/event-handler tests for duplicate assistant replies after history reloads.

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 sessions.changed new/reset, while the PR body adds real Gateway plus TUI terminal proof for the external-channel flow. I did not run a fresh live Telegram reproduction in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96967
Summary: This PR is the active candidate fix for the canonical duplicate TUI assistant message issue, with two closed unmerged sibling attempts covering the same sessions.changed/history replay root cause.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Mantis proof suggestion
A short visible TUI proof would help maintainers confirm the user-facing duplicate-reply behavior is gone after the state-machine fix. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify openclaw-tui shows one assistant reply after an external-channel sessions.changed new/reset reload races with late chat delta and final events.

Risk before merge

  • [P1] The PR changes the TUI cross-reload run-state contract: surrendered finalized runs suppress all late finals, while in-flight surrendered runs can still render displayable finals when history did not restore streaming state.
  • [P1] The contributor supplied real Gateway plus TUI terminal proof using openclaw agent as the external-channel sender; that is useful real behavior proof, but it is not a native Telegram Desktop transport recording.

Maintainer options:

  1. Accept the current surrender contract (recommended)
    Maintainers can accept the event-handler runId surrender behavior and merge after required checks because the latest head covers the finalized late-delta then late-final gap.
  2. Ask for native visual proof first
    Maintainers can pause merge for a Mantis or contributor recording that shows openclaw-tui displaying one assistant reply for the external-channel race in a native UI.
  3. Pause if finalized-run suppression is disputed
    If maintainers do not want finalized runs to suppress all late finals after a reload, the PR should pause for an explicit owner decision before more repair churn.

Next step before merge

  • No automated repair is needed; maintainer review should accept or adjust the runId surrender contract before merge.

Security
Cleared: No concrete security or supply-chain concern found; the diff is limited to TUI message/session state handling and tests, with no dependency, workflow, secret, or package-surface changes.

Review details

Best 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 sessions.changed new/reset, while the PR body adds real Gateway plus TUI terminal proof for the external-channel flow. I did not run a fresh live Telegram reproduction in this read-only review.

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 sessions.changed reloads, both of which have worse false-suppression or reset-regression risk.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 825d9a66623a.

Label changes

Label justifications:

  • P2: This is a normal-priority visible TUI duplicate-message bug with limited blast radius and no crash, data loss, or security boundary impact shown.
  • merge-risk: 🚨 session-state: The diff adds cross-reload runId tracking that changes how TUI session/run state survives sessions.changed new/reset reloads.
  • merge-risk: 🚨 message-delivery: The diff changes when late assistant deltas and finals render or suppress after history reloads, which can affect duplicate or missing visible assistant replies.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal proof from a real Gateway plus openclaw-tui external-run simulation and reset-path proof with private token redacted; unit tests cover the narrower late-delta/later-final state-machine edge.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal proof from a real Gateway plus openclaw-tui external-run simulation and reset-path proof with private token redacted; unit tests cover the narrower late-delta/later-final state-machine edge.
Evidence reviewed

PR surface:

Source +107, Tests +499. Total +606 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 110 3 +107
Tests 2 499 0 +499
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 609 3 +606

What I checked:

  • Repository policy read: Root AGENTS.md, scoped src/tui/AGENTS.md, and the Telegram maintainer note were read; the TUI guide limited fake-backend PTY proof claims, and the Telegram note informed the proof-risk assessment. (src/tui/AGENTS.md:1, 825d9a66623a)
  • Current main still has the vulnerable reload shape: On current main, matching sessions.changed new/reset events clear tracked TUI run state and call loadHistory() for the current session. (src/tui/tui-event-handlers.ts:750, 825d9a66623a)
  • History replay lacks run IDs: loadHistory() clears the chat log and replays assistant rows through chatLog.finalizeAssistant(text) without a run ID, so replayed rows cannot match later live finals by runId on main. (src/tui/tui-session-actions.ts:472, 825d9a66623a)
  • PR head fixes at the event-handler boundary: The PR checks surrendered run IDs before the finalized-run guard, preserves finalized surrender markers through late deltas, and renders displayable finals only for in-flight surrendered runs. (src/tui/tui-event-handlers.ts:621, 88760fbc5d7e)
  • PR head surrenders runs before reset/new reloads: The PR captures in-flight and finalized run IDs before clearTrackedRunState() on sessions.changed new/reset, which is the central race in the report. (src/tui/tui-event-handlers.ts:835, 88760fbc5d7e)
  • Regression coverage includes the late-delta then late-final edge: The PR adds a focused test where a surrendered finalized run receives a late delta and then a late displayable final, asserting both remain suppressed. (src/tui/tui-event-handlers.test.ts:2031, 88760fbc5d7e)

Likely related people:

  • vincentkoc: Authored and merged the active-session sessions.changed refresh path in merged PR fix(tui): refresh after external session reset #93562, which is the central behavior this PR adjusts. (role: recent TUI session-change contributor and merger; confidence: high; commits: 9b49387ad8c2; files: src/tui/tui-event-handlers.ts, src/tui/tui.ts, src/tui/gateway-chat.ts)
  • SebTardif: Authored merged PR fix(tui): skip history reload when final event has displayable output #88004, the closest existing fix for displayable final/history reload races outside the direct sessions.changed handler. (role: adjacent final-reload fix author; confidence: medium; commits: 9a1b95c1e63a; files: src/tui/tui-event-handlers.ts, src/tui/tui-event-handlers.test.ts)
  • Dallin Romney: Recent history shows work stabilizing optimistic user messages across history reloads and runId reassignment, adjacent to the replay/reconcile behavior this PR must preserve. (role: adjacent TUI history/reload contributor; confidence: medium; commits: ac9a219692aa; files: src/tui/tui-event-handlers.ts, src/tui/tui-session-actions.ts, src/tui/components/chat-log.ts)
  • Josh Avant: Current-main blame on the newly staged TUI modules points to a broad recent code move, useful for routing current-file context but not strong evidence of semantic ownership for this bug. (role: recent current-source mover; confidence: low; commits: 7c47904bb42d; files: src/tui/tui-event-handlers.ts, src/tui/tui-session-actions.ts, src/tui/components/chat-log.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 26, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 26, 2026
@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 26, 2026
@xialonglee

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 26, 2026
@xialonglee

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 26, 2026
@xialonglee
xialonglee force-pushed the fix/issue-96967-tui-duplicate-assistant branch from 5336b4d to 530c6e7 Compare June 26, 2026 12:14
@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 26, 2026
@xialonglee

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 26, 2026
@clawsweeper clawsweeper Bot added the status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. label Jun 29, 2026
@xialonglee

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 29, 2026
@steipete steipete self-assigned this Jul 7, 2026
@steipete
steipete force-pushed the fix/issue-96967-tui-duplicate-assistant branch from 88760fb to 487de8c Compare July 7, 2026 04:37
xialonglee and others added 11 commits July 7, 2026 00:39
…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).
@steipete
steipete force-pushed the fix/issue-96967-tui-duplicate-assistant branch from 487de8c to c61eec3 Compare July 7, 2026 04:39
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Land-ready proof for exact head bc65ea0951513d2c1001a1a6bdade667dc9af03b:

  • Review artifacts are current and valid with zero findings. Fresh autoreview completed cleanly at 0.82 confidence after the final refactor.
  • The final design removes text/TTL heuristics. sessions.changed now waits for the exact terminal persistence barrier, serializes history reloads by run ID, and preserves authoritative history or prior display state across reload success, failure, reset, and delayed events.
  • Blacksmith Testbox tbx_01kwxd4bqttj4qrkdzpp4x4gve: 151 focused TUI tests passed; 26 PTY E2E tests passed (18 harness plus 8 real local/Gateway cases); exact changed gate passed, including production/test types, lint, cycles, boundary, and attribution guards.
  • Direct AWS Crabbox cbx_b33918875031, run run_4490545442ba: clean build, tmux Gateway/TUI, and real openai/gpt-5.5 turns. The external-turn marker rendered exactly once before /reset and exactly once after /reset; credential-output scan was clean.
  • Exact-head hosted CI run 28842558421 passed with no failed jobs: https://github.com/openclaw/openclaw/actions/runs/28842558421
  • Required native prepare completed with OPENCLAW_TESTBOX=1; exact head and remote PR head match. Mainline drift touched only release-owned CHANGELOG.md, outside this PR's changed surface.

Known proof gaps: none.

@steipete
steipete merged commit 6192b03 into openclaw:main Jul 7, 2026
107 checks passed
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: TUI displays assistant reply twice when triggered via Telegram channel

2 participants