Skip to content

Clear chat history after session reset#740

Merged
ranjeshj merged 3 commits into
openclaw:mainfrom
RBrid:user/rbrid/ChatHistoryClearing
Jun 10, 2026
Merged

Clear chat history after session reset#740
ranjeshj merged 3 commits into
openclaw:mainfrom
RBrid:user/rbrid/ChatHistoryClearing

Conversation

@RBrid

@RBrid RBrid commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Clear the native Chat page timeline when a Sessions page reset completes.
  • Fence stale pre-reset history, live chat/agent events, remote backfill, abort persistence, local echo state, send completions, and tool metadata/session-id reseeding.
  • Add regression coverage for reset races and timestamp/session-id edge cases.

Validation

  • ./build.ps1
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore — 2200 passed, 29 skipped
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore — 1005 passed

Review

  • Dual-model review ran repeatedly until both final reviewers reported no actionable findings.

Proof Screenshots

Before the session reset:
image

After the session reset:
image

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 10, 2026, 1:38 PM ET / 17:38 UTC.

Summary
The PR clears the native Chat timeline after a successful session reset and adds generation, timestamp, run, send, abort, history, metadata, and session-ID fencing against stale pre-reset activity.

Reproducibility: yes. Current main has no Chat-provider reset-completion handling, and the inspected screenshots plus source path show the timeline remaining populated until this branch clears it.

Review metrics: 3 noteworthy metrics.

  • Patch surface: 6 files; +1,324/-74. The change introduces a substantial asynchronous reset state machine in the native message-delivery path.
  • Focused reset coverage: 11 reset-specific tests. Coverage now includes the prior timestamp-less remote first-turn defect plus send, lifecycle, history, and reset-race cases.
  • Behavior proof: 2 screenshots inspected. The images directly demonstrate the intended visible timeline-clearing result.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
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.

Risk before merge

  • [P1] The patch deliberately suppresses multiple event classes during a reset boundary; despite strong focused coverage, maintainers should treat first-turn delivery and session-state upgrade behavior as merge-sensitive.
  • [P1] Validation results are contributor-reported from the required Windows commands; this read-only Linux review did not rerun the Windows build or tests.

Maintainer options:

  1. Merge with the focused reset suite (recommended)
    Accept the intentional event fencing after confirming the reported Windows build and both test suites remain green on the exact head.
  2. Request one live first-turn recording
    If maintainers want stronger upgrade confidence, ask for a short recording showing reset followed by both a local send and a remote-client turn before merging.

Next step before merge

  • Review the exact head for ordinary merge approval and confirm the contributor-reported Windows build and test results; no automated repair is indicated.

Security
Cleared: The diff changes local chat/session state and tests without adding dependencies, workflow execution, permissions, secret access, downloads, or other supply-chain exposure.

Review details

Best possible solution:

Land the provider-owned reset handling with its generation-safe first-turn gates and retain the focused race tests as the compatibility contract for local and remote post-reset delivery.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main has no Chat-provider reset-completion handling, and the inspected screenshots plus source path show the timeline remaining populated until this branch clears it.

Is this the best way to solve the issue?

Yes. Handling successful reset completion in the provider that owns the native timeline is the narrowest maintainable boundary, and the follow-up closes the identified timestamp-less remote-turn gap without introducing a parallel reset mechanism.

AGENTS.md: found and applied where relevant.

Codex review notes: reasoning high; reviewed against 0e61fa287afb.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The inspected before/after screenshots directly show native Chat history present before reset and cleared to the welcome state afterward.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P1: The PR fixes a core Chat session-reset defect, while any regression in its gate could suppress valid user or assistant messages.
  • merge-risk: 🚨 message-delivery: The new reset gate intentionally drops chat and lifecycle events until post-reset evidence is accepted.
  • merge-risk: 🚨 session-state: The patch clears and reseeds timeline, history, run, session-ID, abort, echo, and metadata state across asynchronous reset races.
  • 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 (screenshot): The inspected before/after screenshots directly show native Chat history present before reset and cleared to the welcome state afterward.
  • proof: sufficient: Contributor real behavior proof is sufficient. The inspected before/after screenshots directly show native Chat history present before reset and cleared to the welcome state afterward.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The inspected before/after screenshots directly show native Chat history present before reset and cleared to the welcome state afterward.
Evidence reviewed

Acceptance criteria:

  • [P1] ./build.ps1.
  • [P1] dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore.
  • [P1] dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore.

What I checked:

Likely related people:

  • Scott Hanselman: Has the highest commit count in current history for the central Chat provider and carried recent adjacent hardening and model-filter work. (role: frequent recent area contributor; confidence: medium; commits: d23f8ca, d1b1363; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs)
  • Ranjesh: Recent merged attachment-metadata and gateway chat work intersects the history, metadata, and event paths changed here. (role: recent feature contributor; confidence: high; commits: 9a3a7a6, 3d26594; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs, src/OpenClaw.Shared/OpenClawGatewayClient.cs)
  • Christine Yan: Git blame and follow-history tie the current provider live-message handling foundation to this contributor’s localization-era implementation commit. (role: original chat handler contributor; confidence: high; commits: 85445c7; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs)
  • RBrid: Has multiple prior current-main commits in the central provider, including the latest logging cleanup, beyond authorship of this proposal. (role: recent area contributor; confidence: medium; commits: 753828f; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs)
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 proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. 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. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. labels Jun 10, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@RBrid

RBrid commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown

🦞🧹
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.

Re-review progress:

@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 10, 2026
@RBrid RBrid marked this pull request as ready for review June 10, 2026 17:43
@ranjeshj

Copy link
Copy Markdown
Collaborator

Follow-up on the stale post-reset agent-event risk: I have a local surgical patch ready in this worktree.

Root cause: ShouldDropAgentEventAfterResetLocked stopped applying the reset timestamp cutoff once the reset gate opened, so a late pre-reset agent.event with an old timestamp and no/untracked runId could render into the cleared timeline. The chat-message path already kept that stale timestamp check active after the gate opened.

Local fix:

  • Keep IsPreResetTimestampLocked(...) active for agent events even after _resetAwaitingUserMessage is cleared.
  • Preserve ignored-run cleanup behavior by leaving IsResetIgnoredRunLocked(...) first.
  • Add SessionResetCompletion_DropsLatePreResetAgentEventAfterGateOpens to prove a fresh post-reset turn still renders while a late pre-reset agent delta is dropped.

Local validation:

  • ./build.ps1 passed.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore passed: 2200 passed, 29 skipped.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore passed: 1007 passed.
  • Hanselman-style review: no actionable findings.

Bundled autoreview note: the default Codex engine is not installed in this environment, and the available Copilot helper path requires an explicit unsandboxed-tools opt-in, so I did not override that safety gate here.

Continue applying the reset timestamp cutoff to agent events even after the post-reset gate has reopened. This matches the chat-message path and prevents late pre-reset agent frames without tracked run IDs from rendering into a cleared timeline.

Add regression coverage for a late pre-reset assistant delta arriving after a fresh post-reset turn has opened the gate.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranjeshj ranjeshj merged commit 3fdfbfa into openclaw:main Jun 10, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants