[BennyAI2]

What

Coalesces bursts of Publish() calls in OpenClawChatDataProvider into a single UI-thread dispatch per drain, instead of scheduling one Changed?.Invoke per call.

Why

Reproduced twice on a live install (2026-08-30 and 2026-09-01), correlating crash.log, the Windows Application event log, and openclaw-tray.log timestamps:

[2026-09-01 20:13:13.601] [DEBUG] Agent event received: stream=lifecycle len=404
[2026-09-01 20:13:13.741] [ERROR] CRASH UnhandledException: Microsoft.UI.Xaml.LayoutCycleException: Layout cycle detected.

[2026-08-30 21:21:02.506] UnhandledException
Microsoft.UI.Xaml.LayoutCycleException: Element is already the child of another element.

Both crashes landed right after a fast burst of session.tool / agent (stream=item, stream=command_output) events, immediately followed by a sessions.changed-triggered refresh — i.e. multiple tool-activity rows (and their Expander controls, keyed by ChatToolActivityPresentation.ActivityKey) get regrouped and re-mounted several times within milliseconds. Today's .NET Runtime event-log entry shows the fault inside Microsoft.UI.Reactor.Core.V1Protocol.Handlers.ExpanderHandler.Mount / Reconciler.Mount, which matches ToolCallCardRenderer.BuildActivityCore's Expander(...) usage.

Root cause: every Publish(snapshot) call independently does _post(() => Changed?.Invoke(...)). When several Publish() calls land faster than the WinUI3 dispatcher drains them, the Reactor visual tree gets mutated multiple times before one layout pass settles — WinUI3's layout engine detects the cycle and hard-crashes the process (unrecoverable 0xc0000005/0xc000027b native fault).

Fix

Only the latest snapshot matters for rendering — intermediate ones in a burst are immediately superseded. This PR adds a small lock-guarded "pending snapshot + scheduled flag" so a burst of N Publish() calls results in at most one _post(...) dispatch (draining to the newest snapshot at that point), instead of N queued dispatches. No behavior change for the common non-bursty case (single Publish() → single dispatch, same as before).

Verification

• Root-caused via log/event-log correlation (see stack trace above) rather than a minimal repro harness, since it's timing-dependent on tool-call event bursts.
• Reviewed the change carefully against the existing codebase's other event-coalescing patterns (matches shape used elsewhere, e.g. debounced snapshot persistence just below).
• I could not complete a local WinUI3 solution build in my sandbox environment (GitVersion.MsBuild task did not complete in a reasonable time on this box) to run the existing test suite before opening this PR. Flagging that explicitly — please treat CI as the real gate here, and let me know if dotnet test tests/OpenClaw.Tray.Tests surfaces anything I should address.
• Change is narrowly scoped: 3 new private fields + one method rewritten, no public API/behavior contract changes.

Context

Filed by an OpenClaw agent on behalf of the reporter after diagnosing a recurring tray crash during normal chat usage (not caused by gateway/model config, which was independently verified healthy at the time of both crashes).

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 1, 2026, 3:27 PM ET / 19:27 UTC.

ClawSweeper review

What this changes

The PR coalesces bursty chat snapshot notifications so the WinUI dispatcher renders the newest available snapshot rather than every intermediate one.

Merge readiness

⛔ Blocked until stronger real behavior proof is added - 4 items remain

Keep open. Current main still posts every chat snapshot independently, and this PR offers a focused mitigation, but its coalescing change leaves two existing queued-delivery tests asserting the old two-dispatch contract and the reported live result has no inspectable runtime artifact yet.

Priority: P2
Reviewed head: 482dd30f33ee2b04860c15a653237c9e6be91e2c

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch is focused and plausible, but its test contract is currently broken and the reported live proof is not auditable.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: The changed production owner is Publish, which posts snapshots to the WinUI dispatcher for Reactor rendering. The contributor describes a current-head patched-app 20-event burst and continued processing, but no inspectable redacted terminal output, log excerpt, screenshot, recording, or artifact shows the asserted after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: The changed production owner is Publish, which posts snapshots to the WinUI dispatcher for Reactor rendering. The contributor describes a current-head patched-app 20-event burst and continued processing, but no inspectable redacted terminal output, log excerpt, screenshot, recording, or artifact shows the asserted after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	7 items	Introduced coalescing behavior: The introduced branch stores the latest pending snapshot and suppresses a second post while one callback is scheduled. Queued history test now contradicts the branch: This test expects two queued callbacks after the history snapshot and disconnect snapshot. With the introduced scheduled flag, the second publish updates the pending snapshot without adding a second callback. Queued reset test has the same obsolete count: The reset-generation test likewise requires two manually queued deliveries before it can assert the final snapshot, so it will fail under the intended coalesced-delivery contract.
Findings	1 actionable finding	[P2] Update queued-delivery tests for the coalesced dispatch contract
Security	None	None.

How this fits together

The tray chat provider turns gateway chat events into immutable snapshots and posts them to the WinUI thread. Those notifications cause Reactor to render streaming messages and tool activity.

flowchart LR
A[Gateway chat events] --> B[Chat snapshot provider]
B --> C{UI dispatcher available}
C -->|No| D[Notify subscribers]
C -->|Yes| E[Keep newest pending snapshot]
E --> F[Dispatcher callback]
F --> G[Reactor chat rendering]

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: The changed production owner is Publish, which posts snapshots to the WinUI dispatcher for Reactor rendering. The contributor describes a current-head patched-app 20-event burst and continued processing, but no inspectable redacted terminal output, log excerpt, screenshot, recording, or artifact shows the asserted after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Update queued-delivery tests for the coalesced dispatch contract (P2) - The new scheduled flag intentionally leaves only one action in a manual post queue, but Disconnect_DropsHistoryDeliveryQueuedAfterNewerStatusSnapshot and LoadHistoryAsync_QueuedDeliveryDropsAfterResetGenerationAdvances still assert two. Those tests fail on this branch before they can verify that the single callback delivers the newest snapshot.
• Resolve merge risk (P1) - The changed notification contract is not yet protected by updated queued-delivery tests, so a future change could reintroduce stale or stranded chat rendering without a focused regression signal.
• Complete next step (P2) - A narrow mechanical repair can update the obsolete queue-count assertions and add the missing coalescing regression, while the contributor separately supplies auditable live proof.

Findings

• [P2] Update queued-delivery tests for the coalesced dispatch contract — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:1839-1841

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Introduced scope	1 production file, +43/-3 lines, 0 test files	A behavior-changing dispatcher contract was added without corresponding test updates.

Merge-risk options

Maintainer options:

1. Codify the coalesced delivery contract (recommended)
   Update the stale-history and reset-generation queue tests, and add a burst assertion that one pending callback delivers the latest snapshot.
2. Pause pending auditable runtime proof
   Wait for a redacted current-head build and burst-run trace if the contributor cannot provide it with the revised validation results.

Technical review

Best possible solution:

Keep latest-snapshot coalescing, update the manual-post tests to assert one queued callback and newest-snapshot delivery, then attach redacted current-head runtime evidence for the burst scenario.

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

No high-confidence reviewer reproduction is available from the checked-out source. The contributor reports a before-and-after live burst run, but no concrete after-fix output is available to audit.

Is this the best way to solve the issue?

No, not as submitted. Latest-snapshot coalescing is a narrow maintainable approach, but the existing queued-delivery tests must be updated to define and protect its contract.

Full review comments:

• [P2] Update queued-delivery tests for the coalesced dispatch contract — src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:1839-1841
  The new scheduled flag intentionally leaves only one action in a manual post queue, but Disconnect_DropsHistoryDeliveryQueuedAfterNewerStatusSnapshot and LoadHistoryAsync_QueuedDeliveryDropsAfterResetGenerationAdvances still assert two. Those tests fail on this branch before they can verify that the single callback delivers the newest snapshot.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0466d2db1bd1.

Labels

Label changes:

• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

• P2: The reported WinUI crash affects normal chat use, while the submitted repair is narrow and localized.
• merge-risk: 🚨 availability: The modified dispatcher notification path controls whether the tray chat surface continues to refresh under bursty updates.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The changed production owner is Publish, which posts snapshots to the WinUI dispatcher for Reactor rendering. The contributor describes a current-head patched-app 20-event burst and continued processing, but no inspectable redacted terminal output, log excerpt, screenshot, recording, or artifact shows the asserted after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

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:

• Introduced coalescing behavior: The introduced branch stores the latest pending snapshot and suppresses a second post while one callback is scheduled. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:1837, 482dd30f33ee)
• Queued history test now contradicts the branch: This test expects two queued callbacks after the history snapshot and disconnect snapshot. With the introduced scheduled flag, the second publish updates the pending snapshot without adding a second callback. (tests/OpenClaw.Tray.Tests/OpenClawChatDataProviderTests.cs:9202, 482dd30f33ee)
• Queued reset test has the same obsolete count: The reset-generation test likewise requires two manually queued deliveries before it can assert the final snapshot, so it will fail under the intended coalesced-delivery contract. (tests/OpenClaw.Tray.Tests/OpenClawChatDataProviderTests.cs:13151, 482dd30f33ee)
• Current main does not already contain the fix: Fetched current main retains the original one-post-per-Publish implementation, and the PR head is not contained in it. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs:1819, 0466d2db1bd1)
• Repository UI-proof policy: The applicable policy requires full build and shared/tray tests, plus current-head UI proof and the PR proof sections for chat changes. (AGENTS.md:39, 482dd30f33ee)
• Reported live exercise: The contributor reports a Release build and a 20-event gateway burst against the patched app, with no crash or PID change, but the comment provides no copied redacted output, log excerpt, screenshot, or linked artifact to audit that after-fix result.

Likely related people:

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

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Update the two queued-delivery tests and add a burst test that proves one queued callback publishes the latest snapshot.
• Add redacted current-head build and burst-run evidence under ## Real behavior proof, then update the PR body to trigger a fresh review.
• Report the required full build, shared-test, and tray-test results under ## Validation.

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 (1 earlier review cycle)

• reviewed 2026-09-01T18:48:00.360Z sha 482dd30 :: needs real behavior proof before merge. :: [P2] Update queued-delivery tests for the coalesced dispatch contract

[BennyAI2]

Update: build + live crash-reproduction verified

Completed a local build (needed -p:DisableGitVersionTask=true to work around an unrelated GitVersion.MsBuild issue in my sandbox). Cold WinUI3 build succeeded: 0 errors, 1 pre-existing unrelated warning, ~5 min elapsed.

Verification steps:

1. Built this branch's OpenClaw.Tray.WinUI.csproj (Release, x64) successfully.
2. Launched the patched exe from the build output directory, side-by-side with the stock installed app.
3. Confirmed clean startup and normal gateway connection via openclaw-tray.log.
4. Stress-tested the exact failure condition: fired 20 rapid tool-call events through the connected gateway in a tight burst, matching the timing pattern of both prior field crashes (which fired within ~150ms of a fast tool-call burst + a sessions.changed refresh).
5. The patched app survived the burst -- same PID throughout, no new crash.log entry, openclaw-tray.log showed it actively processing the burst normally.

This moves the fix from "root-caused via log correlation, reasoned patch" to "reproduced and confirmed fixed via live testing." Happy to add an automated regression test around Publish() burst-coalescing if there's a preferred harness/pattern in tests/OpenClaw.Tray.Tests for this kind of dispatcher-burst scenario.

Marking ready for review.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-01T19:23:08.354334Z	482dd30	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[shanselman]

Superseded by maintainer-owned replacement #1301 at 94e847e3e5955fe4bd8397de18f25c7caea597fe. The replacement includes the current-main transplant, deterministic burst/concurrency/lifetime coverage, required validation, current-head WinUI proof, and final clean review. Closing this stale contributor head without merging it.