Skip to content

fix(chat): restore the tail when switching conversations - #1438

Draft
shanselman wants to merge 2 commits into
mainfrom
shanselman-chat-session-tail-restoration
Draft

shanselman wants to merge 2 commits into
mainfrom
shanselman-chat-session-tail-restoration

Conversation

@shanselman

@shanselman shanselman commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Related: #1437. Follow-up context: #1424.

Status: rejected candidate, investigation continues

Do not merge. The one-key candidate at 4101ee5d21e0cfbe031e4956fe4ca10b27598003 performed worse in the controlled, guarded real-application comparison. It is not a verified fix. Earlier mounted-host passes and clean source review are insufficient and do not override these results.

What Problem This Solves

The target bug is returning to a populated conversation and seeing its first messages instead of the previously visible tail. The bug predates this patch; no known-good version or introducing commit has been established, so it is not yet a confirmed regression window.

Latest real behavior proof

Predeclared budget: five sequential non-Dev Release runs per arm, alternating A/B. Both used the same unchanged GatewayFixtureUiTests.SessionPickerSwitchesRealHistoriesAndShowsMessage240AtBothWidths test, scenario, natural message-240 assertion and timeout. There was no manual scrolling before the natural-tail gate and no retry-until-green.

Arm Source Passed Natural-tail assertion failures Hangs Loaded XAML verified
A Exact published harness afc3e9f7e616bde264ed78490e6738d0c1bbbd44 4 1 0 5/5
B Same harness plus only the two-line session/generation key and comment from 4101ee5d 0 5 0 5/5

All five B failures followed the first A selection and subsequent B history request. The unchanged test only selects B after the first A natural-tail assertion passes. The failure is therefore on return to the cached long conversation, not its first selection.

The comparison used separate short-path owned worktrees and normal self-contained Windows App SDK app outputs. Each run's actually loaded Microsoft.UI.Xaml.dll and Microsoft.UI.Xaml.Controls.dll paths and SHA-256 hashes matched that arm's output. Both used the same native binaries:

  • XAML: B241F72BFC9460E823D8A122FBDBC2E2D7AC6BA60B9E5BF981E9B5054D4555B5
  • Controls: BFB7EE4E86F00B9365FA50DECA8D30BF32C71ACAA536779D20DCF17AE32EC725

Both runtime configurations retained OpenClaw.GatewayFixtureIsolationVersion=1 and System.Diagnostics.Debugger.IsSupported=false. This is explicitly combined harness+candidate proof, not an untouched fix-PR-head full-app run. Neither PR base was changed and the combined overlay was not pushed.

Every outcome, screenshot, Gateway trace, binary/runtime hash and exact command is retained under the coordinating session's fixture-comparison artifacts (contract.json, provenance.json, results.json, and a1 through b5). These identifiers refer to retained local artifacts, not public download links. Earlier MAX_PATH build failures are separate setup evidence and did not consume or alter the five UI runs per arm.

Why the original candidate is being withdrawn

The candidate keyed the outer ItemsView/scroll-controller decorator by session and timeline generation. The controlled real-app results reject the apparent improvement from the exploratory mounted tests. Next work is bounded to comparable Request/Update/mount/disposal tracing in the actual guarded application before proposing another product change.

The exploratory VSTest host also exposed independent initialization and runtime mismatches, including loading installed 2.5.1 native binaries instead of the app's bundled 2.4.0 payload. Those experiments and their failures are preserved separately. They are not evidence that the product bug is fixed, and speculative controller/test-host/runtime workarounds are not being folded into the landing patch.

Required proof pools

  • windows-winui-interactive: populated-session return, actual final-message visibility, native lifetime, streaming and scroll-away behavior.

Validation

Historical validation of head 4101ee5d, before the decisive comparison:

  • .\build.ps1: passed, all five projects and documentation gate.
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: 3,984 passed, 32 skipped.
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: 2,978 passed.
  • Mounted native Debug: 6 passed, 1 initial-tail failure.
  • Mounted native non-Dev Release: 7 passed.
  • Structured Codex branch review: no actionable source findings.

These results do not make the current candidate merge-ready. The unchanged real-app natural-tail gate is the decisive failed check.

Security and scope

The fixture guards remained enabled. No installed pairings, credentials, autostart, toast registration, WSL Gateway or default MCP port were touched. No parent-checkout edits and no merge were performed. The separately published harness is #1439 (feat: explore and test the app without a live Gateway); its assertion is unchanged.

Fixes: #1437

Scope the native ItemsView and scroll-controller decorator to session and timeline generation. Keep same-session scrolling, virtualization, and row-targeted navigation unchanged. Add mounted native regression and lifecycle proof with isolated XAML resource setup.

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

Copilot-Session: 8346e696-3092-49d5-98be-80b5cae59048
Use the ScrollView operation correlation for manual reader scrolling after all natural-tail assertions. Keep token/revision checks strictly about reconciled native identity and supersede their pending navigation before checking a new session.

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

Copilot-Session: 8346e696-3092-49d5-98be-80b5cae59048
@clawsweeper

clawsweeper Bot commented Sep 17, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. 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: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 17, 2026
@clawsweeper

clawsweeper Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 17, 2026, 4:27 PM ET / 20:27 UTC (Revision 4).

ClawSweeper review

What this changes

The branch recreates the native chat list when the conversation or timeline generation changes and adds mounted Windows UI tests and testing guidance.

Regression provenance

Possible regression — probable (reviewed change; failure trace). No predecessor PR is attributed.

Merge readiness

Blocked before merge - 5 items remain

Keep open and blocked: the prior finding remains unresolved, and the author’s controlled real-app comparison rejects this candidate. Current main does not establish a replacement fix; collaborator-authored work also requires explicit maintainer disposition.

Priority: P2
Reviewed head: 4101ee5d21e0cfbe031e4956fe4ca10b27598003

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) PR readiness rating was derived from proof quality, review findings, security review, and reviewer confidence.
Proof confidence 🌊 off-meta tidepool Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🦪 silver shellfish (2/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Evidence reviewed 8 items Introduced lifetime change remains: The pinned introduction adds the session/generation key to the outer scroll-controller decorator. The production controller creates its positioner on Mount and disposes it on Unmount. Comparison with the previously reviewed head is empty, so the existing blocker is unchanged.
Controlled comparison rejects the candidate: The complete supplied PR body, captured under sourceRevision bed2cbb69e8cb94992c1248e57e4c11758ad02a5d971ad5f676b79ec0e2dac06, reports five alternating Release runs per arm: baseline passed 4/5 and baseline plus only the two production lines passed 0/5. The same natural-tail assertion failed on cached conversation return after initial selection succeeded. Native binary hashes matched across arms. This is reported combined harness-plus-candidate evidence, not an untouched PR-head run; retained artifact identifiers are not public links. The author explicitly says not to merge.
Current-main and release comparison: Current main retains the unkeyed decorator and existing row-targeted scrolling. The chat directory has no changes between the supplied latest release v2026.9.4 and fetched main. No merged fixing PR for cached-session tail restoration is established.
Findings 1 actionable finding [P2] Replace the rejected session-key lifetime change
Security None None.

How this fits together

The Windows chat timeline receives the selected conversation’s cached or loaded messages. Its native list and scroll controller determine which messages are visible and whether new content follows the tail.

flowchart TD
  A[Conversation picker] --> B[Selected conversation and history]
  B --> C[Chat timeline]
  C --> D{Session or generation changed}
  D -->|Yes| E[Replace native list and positioner]
  D -->|No| F[Update existing list]
  E --> G[Visible messages and tail following]
  F --> G
Loading

Before merge

  • Replace the rejected session-key lifetime change (P2) - This key replaces the native ItemsView and its positioner whenever the conversation changes. The supplied controlled Release comparison isolates these two production lines: the unchanged cached-return assertion passed 4/5 baseline runs and 0/5 candidate runs after initial long-session selection succeeded. The author consequently withdrew this approach. Remove or replace this lifetime change before landing and align its identity assertions and documentation with the replacement. The precise native lifecycle cause remains unproven.
  • Resolve merge risk (P1) - Merging the session key could worsen existing conversation-return navigation, leaving users at the beginning instead of the tail; the controlled comparison reports this outcome.
  • Resolve merge risk (P1) - The precise native lifecycle cause remains unresolved, so removing the key alone would restore baseline behavior without fixing the original intermittent failure.
  • Complete next step (P2) - Keep this candidate unmerged until the rejected lifetime change is replaced and the unchanged guarded Release scenario demonstrates recovery.
  • Improve patch quality - Address the highest-priority review finding and re-run the changed-surface validation.

Findings

  • [P2] Replace the rejected session-key lifetime change — src/OpenClaw.Tray.WinUI/Chat/ReactorChatTimeline.cs:170-171
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Controlled native comparison Baseline 4/5 passed; candidate 0/5 passed The reported identical natural-tail gate rejects the candidate despite mounted-test successes.
Production versus test growth Production +2; tests +297/-1; docs +19 The small lifetime change has substantial focused coverage, but that coverage does not establish application-level recovery.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #1437
Summary: This is the explicitly linked but rejected candidate for the canonical tail-return issue; the relationship does not establish a successful fix.

Members:

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

Merge-risk options

Maintainer options:

  1. Keep the rejected candidate paused (recommended)
    Preserve the failed comparison and continue the bounded lifecycle investigation before selecting a replacement fix.
  2. Replace the lifetime change
    Land only a replacement supported by the unchanged guarded application assertion and aligned tests and documentation.

Technical review

Best possible solution:

Restore reliable cached-conversation tail visibility through a demonstrated lifecycle repair while preserving virtualization, row-targeted navigation, streaming follow and reader scroll-away position.

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

Yes, the supplied guarded Release A/B/A scenario provides a concrete reproduction path and repeated candidate failures; this read-only review did not execute it on current main.

Is this the best way to solve the issue?

No. The isolated session-key change performed worse in the reported controlled application comparison, so mounted identity checks cannot justify this approach.

Full review comments:

  • [P2] Replace the rejected session-key lifetime change — src/OpenClaw.Tray.WinUI/Chat/ReactorChatTimeline.cs:170-171
    This key replaces the native ItemsView and its positioner whenever the conversation changes. The supplied controlled Release comparison isolates these two production lines: the unchanged cached-return assertion passed 4/5 baseline runs and 0/5 candidate runs after initial long-session selection succeeded. The author consequently withdrew this approach. Remove or replace this lifetime change before landing and align its identity assertions and documentation with the replacement. The precise native lifecycle cause remains unproven.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 07830aa5a9be.

Labels

Label justifications:

  • P2: This is a bounded native chat navigation defect, with no demonstrated data loss or blocked first-run workflow.
  • merge-risk: 🚨 compatibility: Replacing native list lifetime worsened existing cached-conversation navigation in the reported controlled comparison.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.

Evidence

What I checked:

  • Introduced lifetime change remains: The pinned introduction adds the session/generation key to the outer scroll-controller decorator. The production controller creates its positioner on Mount and disposes it on Unmount. Comparison with the previously reviewed head is empty, so the existing blocker is unchanged. (src/OpenClaw.Tray.WinUI/Chat/ReactorChatTimeline.cs:171, 4101ee5d21e0)
  • Controlled comparison rejects the candidate: The complete supplied PR body, captured under sourceRevision bed2cbb69e8cb94992c1248e57e4c11758ad02a5d971ad5f676b79ec0e2dac06, reports five alternating Release runs per arm: baseline passed 4/5 and baseline plus only the two production lines passed 0/5. The same natural-tail assertion failed on cached conversation return after initial selection succeeded. Native binary hashes matched across arms. This is reported combined harness-plus-candidate evidence, not an untouched PR-head run; retained artifact identifiers are not public links. The author explicitly says not to merge. (4101ee5d21e0)
  • Current-main and release comparison: Current main retains the unkeyed decorator and existing row-targeted scrolling. The chat directory has no changes between the supplied latest release v2026.9.4 and fetched main. No merged fixing PR for cached-session tail restoration is established. (src/OpenClaw.Tray.WinUI/Chat/ReactorChatTimeline.cs:162, 07830aa5a9be)
  • Mounted tests have a narrower boundary: The new tests reconcile the production timeline directly at 360 and 900 DIPs, assert final-text viewport bounds, and check native identity, appended content and reader position. They bypass the application composition root and actual picker/history path, consistent with the documentation’s explicit limitation. (tests/OpenClaw.Tray.UITests/ReactorTimelineTailProofTests.cs:61, 4101ee5d21e0)
  • Native dependency boundary: The patch directly uses Reactor key reconciliation and WinUI ItemsView lifetime. Both Reactor packages remain pinned to preview.12; the testing guide requires actual final-message visibility and preserving row-targeted navigation. The patch adds no dependency upgrade or separate Codex runtime dependency. (docs/WINDOWS_NODE_TESTING.md:67, 4101ee5d21e0)
  • Related work has distinct ownership: Chat can return to the first message instead of the tail when switching back to a populated session #1437 remains the canonical reported failure. feat: explore and test the app without a live Gateway #1439 (feat: explore and test the app without a live Gateway) supplies the guarded harness and expressly does not fix scrolling. Track WinUI ItemsView fix and retire Reactor preview.12 Markdown workaround #1424 owns dependency-workaround retirement, not a verified fix for this return-to-tail failure.

Likely related people:

  • karkarl: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • shanselman: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (3 earlier review cycles)
  • reviewed 2026-09-17T17:23:51.505Z sha 4101ee5 :: blocked before merge. :: none
  • reviewed 2026-09-17T17:30:42.059Z sha 4101ee5 :: blocked before merge. :: none
  • reviewed 2026-09-17T19:03:50.309Z sha 4101ee5 :: blocked before merge. :: [P2] Replace the rejected session-key lifetime change

@shanselman shanselman added the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Sep 17, 2026
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Sep 17, 2026
@shanselman shanselman removed the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chat can return to the first message instead of the tail when switching back to a populated session

1 participant