P0: async void event handlers are a process-crash hazard across the WinUI codebase

[QQSHI13]

Problem

There are ~30+ async void methods in the WinUI layer (code-behind event handlers, node status callbacks, etc.). An unhandled exception in any of them bypasses WinUI's UnhandledException handler and terminates the process.

Notable locations:

• src/OpenClaw.Tray.WinUI/App.xaml.cs — OnLaunched, ShowTrayMenuPopup, ToggleChannel, OnSshTunnelExited
• src/OpenClaw.Tray.WinUI/Pages/*Page.xaml.cs — ConnectionPage, VoiceSettingsPage, PermissionsPage, ConfigPage, etc.
• src/OpenClaw.Tray.WinUI/Windows/*Window.xaml.cs — CanvasWindow.InitializeWebViewAsync, SetupWizardWindow, VoiceOverlayWindow
• src/OpenClaw.Connection/GatewayConnectionManager.cs — OnNodeStatusChanged, OnNodePairingStatusChanged
• src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs — AckAction, ReadAloud

PermissionsPage.EnsureWhisperModelDownloadedAsync already contains a giant try/catch with an explicit comment explaining this exact risk — which proves the team has been bitten by it before.

Impact

• Editing any screen risks introducing a new crash path.
• There is no centralized guard; each author must remember to wrap every statement.
• Production crashes are hard to diagnose because the stack trace originates from the dispatcher, not user code.

Suggested fix

1. Add a centralized FireAndForget(Func<Task> work, Action<Exception>? onError = null) helper.
2. Refactor handlers to call the real work via InnerFooAsync().FireAndForget(_logger).
3. For XAML event handlers that must keep the async void signature, enforce a one-line body that delegates to the guarded helper.

Files

• src/OpenClaw.Tray.WinUI/App.xaml.cs
• src/OpenClaw.Tray.WinUI/Pages/*.xaml.cs
• src/OpenClaw.Tray.WinUI/Windows/*.xaml.cs
• src/OpenClaw.Connection/GatewayConnectionManager.cs
• src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed May 27, 2026, 9:55 AM ET / 13:55 UTC.

Summary
This issue is still necessary: current main still contains the reported unguarded async void connection handlers and a broad remaining WinUI surface, while the linked automation PR only covers two connection-layer handlers.

Reproducibility: yes. from source inspection: current main awaits inside async void connection handlers before any catch block, and several WinUI/chat handlers remain unguarded. I did not run a live Windows crash reproduction because this is a read-only review.

Next step
Maintainers should first decide sequencing with the open partial connection-layer PR, then promote a broader WinUI guard pass if that PR does not expand scope.

Review details

Best possible solution:

Keep this issue open as the umbrella, review or land the connection-layer guard from the linked draft PR or equivalent, then add a shared guarded fire-and-forget helper and refactor the remaining WinUI/page/window/chat handlers with focused regression coverage and Windows smoke validation.

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

Yes from source inspection: current main awaits inside async void connection handlers before any catch block, and several WinUI/chat handlers remain unguarded. I did not run a live Windows crash reproduction because this is a read-only review.

Is this the best way to solve the issue?

Yes in direction: a centralized guarded fire-and-forget helper with one-line event-handler delegation is the narrow maintainable fix. The best sequencing is to handle the open connection-layer partial PR first, then do the broader WinUI pass.

AGENTS.md: found and applied where relevant.

Remaining risk / open question:

• The linked draft PR only addresses two connection-layer handlers, so closing this issue now would lose the broader WinUI crash-hardening work.
• A full pass touches many UI event handlers and should be sequenced with Windows UI validation rather than treated as a tiny mechanical cleanup.

Codex review notes: model gpt-5.5, reasoning high; reviewed against bf21e7b216aa.

Label changes

Label changes:

• add P1: Current main has source-confirmed process-crash paths in async-void connection/UI handlers, but no confirmed active crash loop or unusable runtime state.
• add impact:crash-loop: The issue is about unhandled exceptions escaping event handlers and terminating the tray process.
• add issue-rating: 🦞 diamond lobster: Current issue advisory state selects this label.
• add clawsweeper:source-repro: Current issue advisory state selects this label.
• add clawsweeper:no-new-fix-pr: Current issue advisory state selects this label.
• add clawsweeper:fix-shape-clear: Current issue advisory state selects this label.
• add clawsweeper:needs-maintainer-review: Current issue advisory state selects this label.

Label justifications:

• P1: Current main has source-confirmed process-crash paths in async-void connection/UI handlers, but no confirmed active crash loop or unusable runtime state.
• impact:crash-loop: The issue is about unhandled exceptions escaping event handlers and terminating the tray process.

Evidence reviewed

Acceptance criteria:

• ./build.ps1
• dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
• dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore
• Windows UI smoke for tray startup, connection settings actions, chat timeline actions, canvas initialization, and setup wizard actions

What I checked:

• Repository policy read: AGENTS.md was read fully; it makes GatewayConnectionManager the owner of connection state and requires full build/shared/tray validation after code changes, but this review made no repository changes. (AGENTS.md:1, bf21e7b216aa)
• Current main still has unguarded connection handlers: OnNodeStatusChanged and OnNodePairingStatusChanged are async void; both await _transitionSemaphore.WaitAsync() before any catch block, so an exception there would escape the handler. (src/OpenClaw.Connection/GatewayConnectionManager.cs:770, bf21e7b216aa)
• Existing source comment confirms the crash class: Dispose() already comments that node event unsubscription is needed before disposing the semaphore to prevent async-void handlers from crashing via ObjectDisposedException. (src/OpenClaw.Connection/GatewayConnectionManager.cs:959, bf21e7b216aa)
• WinUI surface remains broad: rg found async-void handlers across the connection layer, tray pages, windows, chat timeline, app startup/tray code, and SetupEngine UI. (bf21e7b216aa)
• Existing local guard pattern is not centralized: OpenClawChatRoot has a private RunFireAndForget helper, but it is local to that component and does not protect the reported page/window/app handlers. (src/OpenClaw.Tray.WinUI/Chat/OpenClawChatRoot.cs:884, bf21e7b216aa)
• Permissions page documents and guards the same risk: EnsureWhisperModelDownloadedAsync explicitly says uncaught async-void exceptions bypass WinUI handling and wraps the full body in a last-resort catch. (src/OpenClaw.Tray.WinUI/Pages/PermissionsPage.xaml.cs:228, bf21e7b216aa)

Likely related people:

• Ranjesh: Blame and file history show the current GatewayConnectionManager handlers, App.xaml.cs async-void surface, and PermissionsPage guard were introduced in Fix tray disconnected behavior. (role: introduced current affected implementation; confidence: high; commits: 6d8541e57008, 7747e6075052, cefce3952ab1; files: src/OpenClaw.Connection/GatewayConnectionManager.cs, src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Pages/PermissionsPage.xaml.cs)
• kenehong: Shortlog and history show many recent commits on the chat UI and ChatTimeline area, including the local fire-and-forget helper history. (role: heavy chat UI contributor; confidence: medium; commits: 8add87d10c54, 4231feab9c0e, 77375498451c; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatRoot.cs, src/OpenClaw.Tray.WinUI/App.xaml.cs)
• Régis Brid: The current HEAD and adjacent markdown-rendering commit both changed OpenClawChatTimeline, which still contains async-void local handlers. (role: recent chat timeline contributor; confidence: medium; commits: bf21e7b216aa, 0a164fc111b2; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs)
• Scott Hanselman: Recent connection history includes a focused GatewayConnectionManager change removing operator-side node pair auto-approve, so this is a relevant routing candidate for connection-layer follow-up. (role: recent connection contributor; confidence: medium; commits: 45dccec60c51, 51c4b4fd4704; files: src/OpenClaw.Connection/GatewayConnectionManager.cs, src/OpenClaw.Tray.WinUI/App.xaml.cs)

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.

[github-actions]

🤖 This is an automated response from Repo Assist.

The issue is well-scoped. I looked at the two highest-risk locations: GatewayConnectionManager.OnNodeStatusChanged and OnNodePairingStatusChanged — both are async void and both have an await _transitionSemaphore.WaitAsync() before any catch block. If that semaphore or anything before the try throws, the exception is entirely uncontained and will terminate the process via the thread pool's unhandled exception path (not WinUI's UnhandledException, which only covers UI thread exceptions).

I've opened a PR that adds a top-level try/catch(Exception ex) around the full body of both handlers, logging via _logger.Warn and _diagnostics.Record. This is the narrowest safe fix for the connection layer and can be validated with the existing test suite.

For the broader WinUI surface (App.xaml.cs, Page code-behind, etc.), the RunFireAndForget pattern already in OpenClawChatRoot is the right model. Those handlers will need a separate pass — the change is safe but spans many files and needs Windows UI testing.

Generated by 🌈 Repo Assist, see workflow run.

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

[shanselman]

Addressed by merged PR #564. The source async-void handler surface was converted to guarded Task delegation, with a regression test preventing new source async-void handlers.