Skip to content

fix: keep reminders when notification navigation is not admitted - #2055

Merged
roboclaw-bot merged 4 commits into
mainfrom
bkudiess-shell-navigation-corrections
Nov 9, 2026
Merged

roboclaw-bot merged 4 commits into
mainfrom
bkudiess-shell-navigation-corrections

Conversation

@bkudiess

@bkudiess bkudiess commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

What Problem This Solves

Fixes: notification reminders disappear when their action is refused, redirected or deferred, and an older action can dismiss a reminder updated during navigation.

User Impact

User impact: inbox and Settings-banner reminders survive unsuccessful navigation. Dashboard actions dismiss only after Windows accepts the browser launch; updated or replaced reminders survive completion of an older action. Native modal-deferred requests retain reminders even if navigation later runs. Opening a reminder never approves or aborts the underlying action.

Current status: integration commit 745eea3b05e3b1a3061614a88d6366eeac26341b is published, preserving both notification admission and main’s Settings-return/focus changes. Its exact source tree passed the native Debug and unsigned non-Dev Release selections below. The PR is ready for maintainer review; current-head CI run 42558071958, attempt 1 has passed, including the UI job and required CI Gate. The only manual merge resolution retained both required imports in NotificationsPage.xaml.cs; no methods or proof assertions were changed. This is not a full-suite, actual-browser-launch or merge-readiness claim.

Why This Change Was Made

The existing notification service shares inbox/banner dismissal policy through the existing navigation owners. Review fixes await GatewayDashboardLauncher.OpenAsync for Dashboard admission, preserve About's separate section identity, and compare the captured immutable reminder under the store lock before dismissal. UI handlers use AsyncEventHandlerGuard.

The prior head 071b364c4c3eb333f388d97e8c8a242bf0e17d1c integrates main 2812fe5187f64105e297de21377f4a8daae5703d. It preserves the newer Channels removal, Settings reorganization and agent-avatar owners. No independent redesign of Pair-device, history/notification aliases, command-center startup compatibility or Activity destinations is included. Chat keys travel on the request instead of being written into ambient pending-chat state; existing downstream consumption of ambient state is unchanged.

Evidence

Current integrated-head proof

Fresh ClawSweeper review completed for 745eea3b05e3b1a3061614a88d6366eeac26341b on November 9 at 04:53 UTC: ready for maintainer review, zero actionable findings and no before-merge items. With current-head CI Gate passing, the PR was marked ready under the author’s instruction. This is not merge approval; the proof limitations below remain explicit.

Both Debug and unsigned non-Dev Release passed 3 notification cases, 3 Settings-return cases, 1 Clipboard case, 2 elapsed-time cases, 3 shared-root-dialog cases and 34 accessibility/Axe cases, with zero selected skips or failures. This is 46 executions and 45 unique cases per configuration: Clipboard is intentionally exercised both standalone and in the accessibility category. All 14 TRX inventories were checked against their retained reports.

  • Inbox and Settings-banner reminders survive browser-boundary refusal and a separate retry, then dismiss only after their own action is admitted. The native Connection action preserves the original HWND and dismisses only its admitted reminder. Browser acceptance is simulated at the existing fixture boundary, not proof of an actual browser launch or remote-page load; action activation uses UI Automation, not a new physical-pointer claim.
  • Mouse, Enter and Escape Settings-return paths each cover retained chat draft and populated/empty Notifications, preserving same-window, foreground and focus assertions. Shared-root-dialog deferral/rejection and the shared Clipboard/elapsed prerequisites also pass in both configurations.
  • Current images support the refusal dialog, own-action reminder states, Settings header/rail, retained draft and Notifications focus journey. Connection-admitted frames are transitional: Connection is selected before its body renders; some notification-row frames are faded. Native assertions establish the mounted Connection marker and selective dismissal, not settled pixels in those frames. Empty-inbox focus is assertion-backed, not a claimed visible focus ring.

Full closeout · Debug and Axe · Unsigned Release build · Release and Axe · Source/binary export · Preservation check.

Independent Astra integration source and implementation/proof-plan review found no actionable P0-P3 integration findings. This is bounded Astra review of the integration, not a new Opus or dual-model verdict. The earlier negative control and broad-hosted crash below remain bound to their original source; neither was rerun for this integration.

Current integrated build, before and after the reminder’s own admitted action. These are interaction states from the same build, not a code-before/code-after comparison.

Before its own action

Integrated notification before its own action

After its own admission

Integrated notification after its own admission

Earlier notification proof, negative control, failures and review history

This retained history predates integration head 745eea3…. Its counts, source identities and review scopes are not promoted into fresh integration executions.

November 9 current-source native proof

  • Debug and unsigned non-Dev Release each passed 3 notification cases, 1 Clipboard case, and 2 elapsed-time cases, with no selected skips. Real inventory-bound Axe passed 34/34. Required full build, Shared 4,875 passed / 25 existing skips, and Tray 5,317 passed / no skips also passed.
  • The native Connection case invokes the uniquely scoped UIA action, verifies Connection in the original HWND, then verifies only the Dashboard reminder is removed while Slack remains. This is native invocation and same-window admission proof, not a physical pointer-hit claim.
  • A faithful guest-only negative control returned false after the native destination rendered. The case reached Connection in the original window and failed specifically waiting for Dashboard dismissal, with both reminders retained. Exact production bytes were restored and rebuilt before the final positive runs.
  • The broad hosted run aborted after 219 passed / 1 existing skip with native exit 0xC000027B; all four StartupWindow cases executed and passed. This is not a green full-suite result. A separate unmodified main a39bd70aab55bfdc0053b278756a87c8a3305bcf run reproduced the same exception/module/hash/offset. The inherited crash is investigated independently rather than attributed to this notification change. Required CI is not bypassed.
  • The two shared test prerequisites fix standalone-radio selection/traversal assertions and replace an elapsed-text fixed sleep with an existing bounded rendered-condition wait. They preserve the lifecycle assertions and are also present in merged feat: add Back to app and Esc to the Settings sidebar #2065 (feat: add Back to app and Esc to the Settings sidebar).

Required native closeout · Final Debug and Axe · Final Release · Faithful negative control · Unmodified-main baseline

Independent final implementation/proof-plan review (native OpenClaw subagent, openai/gpt-6-astra, default context, reasoning off) found no actionable P0-P3 findings in this five-file update. It independently checked all 2,174 source hashes, both new admission test files, retained TRX rows, negative control and restoration. This is bounded Astra review, not refreshed dual-model consensus. No additional simplification was recommended.

Historical November 6 validation

November 6 integrated validation: full build passed; 361 focused, 4,875 Shared passed / 25 skipped, 5,317 Tray passed, and 16 telemetry diagnostic tests passed. Native UI-test project compilation passed with zero warnings/errors; no native UI tests were executed.

The committed tree e7213df09f74ba42722b132608e69eda2b3fef19 exactly matches the source validated before the merge commit. Tests were not rerun merely for committing, pushing or editing this description.

Review history:

  • The initial shared Codex CLI P0-P3 review failed authentication (reviewer_unavailable, report_produced=false); no clean result is claimed for it.
  • An independent Copilot implementation/proof-plan assessment covered the original 17-file candidate and found no significant defect, while identifying native proof limits.
  • The requested Hanselman review used app-native claude-opus-5.5 and gpt-6-astra, both requested high reasoning, with long context requested for Astra. Both reviewed the old head 28a9346157bffcaad1102cc2f856e608ad4cdb92 against 122bda71b2211a8fdb40bb7c8589efd19beb5b2e. They found no significant candidate defect but identified Dashboard admission and About identity integration risks. Those are now fixed; the additional same-ID reminder guard addresses the disputed/pre-existing race defensively.
  • A bounded independent code-review follow-up found no verified P0-P3 bug in the integrated changes. It preceded the final event-handler guard correction, which passed the subsequent automated gates. This is not a refreshed dual-model verdict on the final head.

CI snapshot observed November 6 at approximately 21:00 Pacific: CI run 42340777734 had change-classification and Proof-pool contracts passing, fast-validation in progress, and release/package jobs skipped. Dispatch passed; C# and Actions security checks were skipped. These observations do not establish that all required CI passed.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Required proof pools

  • windows-winui-interactive: integrated notification admission, Settings return/focus, modal deferral and accessibility. All selected native cases passed in Debug and unsigned non-Dev Release. This establishes those named contracts, not complete pool coverage, actual external-browser behavior or settled Connection-content pixels.
Prior declaration and scope
  • windows-winui-interactive: mounted inbox/banner browser-refusal, separate retry, own-action admission, same-window Connection admission and selective reminder dismissal. Three focused cases pass in both configurations, with 34 real Axe cases. This does not claim the entire pool or every setup/modal/focus/draft scenario was executed.

Validation

The published merge commit 745eea3b05e3b1a3061614a88d6366eeac26341b has parents 86615d8a2ea0aa2eff56d55fc827dc35264b2922 and a39bd70aab55bfdc0053b278756a87c8a3305bcf. Its tree ebeaf64f299e6573a7c1b036f395c195ea36d4f4 exactly matches the tree validated before committing. The 2,175-file manifest SHA256 is 0cc0cd341b10de1c5c51add1a7592b34745a454257a72d8fe9293ac42336a65b. Committing and pushing did not create another test execution.

Required command / build Actual integrated result
./build.ps1 Passed
Explicit Shared and Tray project builds Passed
dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore 4,875 passed, 25 existing opt-in skips, 0 failed
dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore 5,328 passed, 0 skipped/failed
dotnet build ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -c Debug -r win-x64 Passed, 0 warnings/errors
./scripts/validate-docs.ps1 Passed: 72 Markdown files, 7 proof pools
Unsigned non-Dev Release app/UI builds Passed, 0 UI warnings/errors; local behavior proof, not signing/distribution proof
Native selection Debug Release
Browser refusal/separate retry: inbox and Settings banner 2/2 2/2
Same-HWND Connection admission/selective dismissal 1/1 1/1
Settings return: mouse, Enter, Escape; chat and both inbox states 3/3 3/3
Clipboard keyboard selection/collapse 1/1 1/1
Elapsed after assistant preamble 2/2 2/2
Shared-root-dialog deferral/native-binding rejection 3/3 3/3
Accessibility/Axe 34/34 34/34

Every selected row passed; 46 executions / 45 unique cases per configuration, not 46 distinct cases. Ordinary Shared skips remain skips, not runtime/MXC proof. Current-head CI run 42558071958, attempt 1 passed, including the UI job and required CI Gate. No broad-hosted replay or crash experiment was included in this integration run.

Exact current focused commands, filters and binary bindings

For each retained Debug/Release configuration and filter below, the native owner ran:

dotnet test ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -c $Configuration -r win-x64 --no-build --no-restore --filter $filter --logger "trx;LogFileName=$name.trx" --results-directory "$proof/$name" --diag "$proof/$name/vstest.log" --blame --blame-hang --blame-hang-timeout 300s --blame-hang-dump-type none
  • Notification, 3: FullyQualifiedName~NotificationAction_BrowserRefusalAndSeparateRetryKeepReminderUntilItsOwnActionSucceeds|FullyQualifiedName~NotificationAction_NativeConnectionDismissesOnlyItsReminderInTheSameWindow
  • Settings return, 3: FullyQualifiedName~SettingsBackToApp_ReturnsToAppDestinationInSameWindow
  • Clipboard, 1: FullyQualifiedName~ClipboardExpander_SupportsKeyboardSelectionAndCollapse
  • Elapsed, 2: FullyQualifiedName~ActiveRun_AfterAssistantPreamble_KeepsClawAndElapsedTimeVisible
  • Shared-root dialog, 3: FullyQualifiedName~SharedRootDialog_DefersNavigationAndRejectsNativeBinding
  • Additional Release accessibility, 34: Category=Accessibility

Debug accessibility used the inventory-owned wrapper:

pwsh -NoProfile -File ./scripts/run-proof-tests.ps1 -Project tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -Filter Category=Accessibility -ResultName winui-accessibility -RuntimeIdentifier win-x64 -ResultsDirectory ./TestResults/ProofPools/pr2055-integration-accessibility

The wrapper has no Configuration parameter. Release accessibility used the explicit selected command above with -c Release, --no-build and --no-restore; its nonzero inventory and all-selected-pass outcome were checked independently.

Unsigned local Release was built with process-scoped RequireSignedCleanupHost=false, ./build.ps1 -Project WinUI -Configuration Release, and dotnet build ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -c Release -r win-x64 -p:DevBuild=false -p:RequireSignedCleanupHost=false. The override was restored; this was not a machine setting or shipping-signing waiver. Both configurations used isolated profiles, explicit fixture/keyboard opt-ins and their own freshly built app path.

Actual binary Debug SHA256 Release SHA256
App DLL 02dd86fc516615115d4a68b59038915198bc998be2c5e34afa5e6e8e49b87252 3cc65b12574e41c0268231b268d95208e6858b47442b822e07bd2ce047e00127
UI test DLL 426b14dd4ff3936860bc03d607b3e2678446a3e7cedb8df2c5c66df4800057fc 0c0eaa37b08368535bfd1f8bce8434e66a4148390be1849a672de2ac2b0f109e

Both app identities report release (non-Dev), independently of compilation configuration. Final receipts verify these same files after the runs.

Pre-integration validation and retained failure history

The following original results belong to earlier source identities. They are preserved, not counted as integration reruns.

November 9 validated update

The published update is byte-bound to the native-tested 2,174-file source manifest 545b704d85e4d2636af5d019feb47f7fb5aa6e51e68931e4f5ad390005b4e748. Source equality is not a new execution after commit.

Command Result
./build.ps1 Passed after exact negative-control restoration
dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore 4,875 passed, 25 existing skips, 0 failed
dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore 5,317 passed, 0 skipped/failed
dotnet build ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -c Debug -r win-x64 Passed; corresponding unsigned non-Dev Release app/UI builds passed
pwsh -NoProfile -File ./scripts/run-proof-tests.ps1 -Project tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -Filter Category=Accessibility -ResultName winui-accessibility -RuntimeIdentifier win-x64 -ResultsDirectory ./TestResults/ProofPools/pr2055-current-accessibility 34 passed, 0 skipped/failed

For each $Configuration in Debug and Release, the actual focused command was:

dotnet test ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -c $Configuration -r win-x64 --no-build --no-restore --filter $filter --logger "trx;LogFileName=$name.trx" --results-directory "$proof/$name" --diag "$proof/$name/vstest.log" --blame --blame-hang --blame-hang-timeout 300s --blame-hang-dump-type none

Filters and actual counts per configuration:

  • FullyQualifiedName~NotificationAction_BrowserRefusalAndSeparateRetryKeepReminderUntilItsOwnActionSucceeds|FullyQualifiedName~NotificationAction_NativeConnectionDismissesOnlyItsReminderInTheSameWindow: 3/3.
  • FullyQualifiedName~ClipboardExpander_SupportsKeyboardSelectionAndCollapse: 1/1.
  • FullyQualifiedName~ActiveRun_AfterAssistantPreamble_KeepsClawAndElapsedTimeVisible: 2/2.

Both fixture and keyboard opt-ins were explicitly set; profiles were isolated and exact app/test binaries bound. Release is unsigned local behavior proof, not signing/distribution proof. The broad hosted selector Category!=Accessibility&Category!=NativeOnboardingProof&Category!=GatewayFixture aborted at 219 passed / 1 skip. Original build/text-assertion failures, negative-control failure, and crash remain retained.

Historical validation

Fresh integrated runs on November 6, 2026, superseding the original pre-integration counts:

Exact command Result
.\build.ps1 Passed all five projects, Debug win-x64
dotnet build .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --verbosity quiet Passed, zero warnings/errors
dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore --logger 'trx;LogFileName=shared.trx' --results-directory $results --verbosity quiet 4,875 passed, 0 failed, 25 skipped
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --logger 'trx;LogFileName=tray.trx' --results-directory $results --verbosity quiet 5,317 passed, 0 failed, 0 skipped
dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore --filter 'FullyQualifiedName~McpHttpServerTelemetryTests' --logger 'trx;LogFileName=telemetry-diagnostic.trx' --results-directory $results --verbosity quiet 16 passed, 0 failed, 0 skipped
dotnet build .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -r win-x64 --verbosity quiet Passed, zero warnings/errors; compile only
.\scripts\validate-docs.ps1 Passed: 72 Markdown files, 7 proof pools
git diff --cached --check 2812fe5187f64105e297de21377f4a8daae5703d Passed for the staged feature delta before commit

Focused command: 361 passed, 0 failed, 0 skipped.

dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --filter 'FullyQualifiedName~AppNotificationServiceTests|FullyQualifiedName~HubPresentationContractTests|FullyQualifiedName~WindowManagerTests|FullyQualifiedName~WorkspaceNavigationTests|FullyQualifiedName~AppSurfaceCharacterizationTests|FullyQualifiedName~WorkspaceRoutingBoundaryTests|FullyQualifiedName~NativeCompletionPresentationTests|FullyQualifiedName~SetupDashboardHandoffTests|FullyQualifiedName~DashboardBrowserTests|FullyQualifiedName~HubPageRegistryTests|FullyQualifiedName~TrayMenuWindowMarkupTests.Source_DoesNotDeclareAsyncVoidHandlers|FullyQualifiedName~AppRefactorContractTests.Dashboard_SurfacesSshTunnelConfigurationFailure' --logger 'trx;LogFileName=focused.trx' --results-directory $results --verbosity quiet

$results was a session-artifact output directory. Runs used isolated tray data, roaming and local roots, OPENCLAW_REPO_ROOT, unset runtime opt-ins and OPENCLAW_STATE_DIR, and a bounded process-only PATH.

Preserved failures: an unreadable task-owned FunctionalUI reference assembly was regenerated with dotnet build .\src\OpenClawTray.FunctionalUI\OpenClawTray.FunctionalUI.csproj -r win-x64 -t:Rebuild --verbosity quiet (passed). Handler/source-guard failures were corrected before final closeout. An unchanged Shared test failed in FreePort before product code because Windows lacked socket buffer space; its 16-test batch and full required build/Shared/Tray sequence then passed without source edits or machine changes. The original candidate's IPC timeout is also retained in historical evidence. No timeout or assertion was weakened.

Real Behavior Proof

  • Environment tested: qualified disposable Azure Windows client, x64 interactive 1600×1200 desktop, isolated task profiles. Debug and unsigned non-Dev Release each used its explicitly selected current app and corresponding test binaries.
  • PR head or commit tested: 745eea3b05e3b1a3061614a88d6366eeac26341b. Its tree ebeaf64f299e6573a7c1b036f395c195ea36d4f4 is the exact 2,175-file tree exercised before the merge commit, not a post-push rerun. Both parent identities, manifest and binary hashes are recorded above.
  • Exact steps or command run: the current configuration/filter commands above. Notification actions exercise refusal, separate retry, own-action admission and selective same-HWND Connection dismissal; Settings return exercises mouse/Enter/Escape with retained draft and populated/empty Notifications. Clipboard, elapsed, shared-root-dialog and real accessibility selections cover the integration boundaries.
  • Evidence after fix: all 14 TRX reports checked; each configuration has 46 successful UI executions, 45 unique cases, zero selected skips/failures. Full build, Shared, Tray and docs passed on the integrated tree.
  • Observed result: the selected integrated contracts pass. The existing fixture browser boundary is not an actual external-browser launch. UIA establishes native destination admission and selective dismissal; the transitional Connection frames are not settled-content visual proof. The previous negative control and inherited broad-hosted crash were not rerun on this head.
  • Screenshot or artifact links verified? (Yes/No/N/A) Native run URLs match retained receipts and local artifact hashes. Integrated archive SHA256: 1e690623b0bae77a956ac0d9d4152beddef138eaad99bfda96f3b88b3f15080d. Two current native interaction frames are embedded under Evidence; they depict before/after the reminder’s own action, not before/after code versions. Current images have the explicit transition/focus limitations above.
  • Not verified or blocked: actual browser/remote-page loading, ambient-chat-intent end-to-end preservation, unselected setup/modal scenarios, complete proof-pool acceptance and refreshed dual-model review remain unverified. No broad-hosted crash cure, signing, installation, SAC, Store or native ARM64 claim. No diagnostic instrumentation or pointer helper is included.
Previous head’s native proof and source-bound limits
  • Environment tested: disposable Azure Windows 11 Pro 25H2 client, 16 CPUs / 64 GiB, 1600×1200 work area, Default interactive desktop, 96 DPI; .NET SDK 10.0.403 and Windows App Runtime 2.6.5.0 (source SDK pin 2.6.4). Isolated task profiles only.
  • PR head or commit tested: 86615d8a2ea0aa2eff56d55fc827dc35264b2922. All 2,174 committed file bytes match the source tested before committing, based on parent 071b364c4c3eb333f388d97e8c8a242bf0e17d1c plus the five-file test update.
  • Exact steps or command run: final Debug/Release focused commands and inventory-bound Axe above.
  • Evidence after fix: 3 native notification cases per configuration, browser-boundary refusal and separate-retry retention, same-HWND Connection admission, selective dismissal, plus the faithful post-render false-admission negative control.
  • Observed result: focused contracts and required local closeout pass. The broader hosted crash also reproduces on unmodified main; its original inner HRESULT and product-versus-fixture origin remain unresolved.
  • Screenshot or artifact links verified? (Yes/No/N/A) Yes for retained native run receipts and local artifact hashes. Archive SHA256: b87de0fdf1b09ccc7448860dd4422fb316c2f98090136910a17c58430184fa17. Connection captures are transitional, not settled pixel proof; no polished screenshot comparison is claimed.
  • Not verified or blocked: real external-browser/remote-page loading, every setup/modal/focus/draft scenario, ambient-chat-intent end-to-end preservation, refreshed dual-model review, complete custom proof-pool coverage, and new-head CI/mergeability. Original failed Codex review remains unavailable evidence. No diagnostic instrumentation or pointer helper is included in this PR.

Security Impact

  • New permissions or capabilities? (Yes/No) No
  • Secrets or tokens handling changed? (Yes/No) No
  • New or changed network calls? (Yes/No) No
  • Command or tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any answer is Yes, explain the risk and mitigation: N/A. The existing credential/browser owner is reused; navigation failures log exception type, not routes, keys or messages.

Compatibility and Migration

  • Backward compatible? (Yes/No) Yes. Newer main routes and owners are preserved; reminder-retention semantics change intentionally.
  • Config or environment changes? (Yes/No) No product configuration changes.
  • Migration needed? (Yes/No) No
  • If yes, list the exact upgrade steps: N/A

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

Preserve reminders on refused, redirected, throwing and deferred navigation. Pass chat session keys directly through the existing navigation owners.

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

clawsweeper Bot commented Nov 7, 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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Nov 7, 2026
@clawsweeper

clawsweeper Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge.

What this changes

The PR dismisses notification reminders only after navigation is admitted and preserves reminders updated during navigation.

Example: Click Open Channels on the Slack channel needs attention reminder when browser launch is refused.

  • Before: The reminder disappears despite the failed launch.
  • After: The reminder survives refusal and a separate dialog retry, then disappears after its own action succeeds.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused patch has no identified defects and current native application evidence, bounded by simulated browser acceptance.
Proof confidence 🐚 platinum hermit (4/6) ✨ media proof bonus Sufficient (screenshot): Current-head native application captures and reported Debug/Release UI observations exercise page/banner refusal, separate retry, own-action dismissal and same-window selective Connection admission on isolated Azure Windows profiles. Browser acceptance is simulated at the existing fixture boundary, so this proves application admission handling rather than an actual external-browser launch; the retained negative control belongs to the earlier head.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Users lose actionable reminders when navigation fails, is redirected or deferred, or completes after the reminder has changed.
Reason: The maintainer-authored change repairs reminder lifetime while preserving existing navigation, modal and approval ownership.

Merge readiness

✅ Ready for maintainer review

Keep this maintainer-authored draft open: it fixes reminder loss still present on main, resolves the earlier integration concerns, and has no identified actionable patch defects.

Priority: P2
Reviewed head: 745eea3b05e3b1a3061614a88d6366eeac26341b

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: An unmodified-base execution of the new notification cases is not shown; the supplied native negative control predates the integration head.
Agent review details

How this fits together

NotificationsPage and the Settings banner send reminder actions through AppNotificationService to the existing Workspace and Dashboard navigation owners; admission results control dismissal in the in-memory notification store.

flowchart TD
  A[Notifications page] --> C[Notification service]
  B[Settings banner] --> C
  C --> D[Window manager and route policy]
  D --> E[Native Workspace destination]
  D --> F[Dashboard browser launcher]
  E --> G[Admission result]
  F --> G
  G --> C
Loading

Technical review

Best possible solution:

Land the focused reminder-lifetime fix when the author marks the draft ready.

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

Fetched main's page and banner handlers dismiss immediately after void navigation, establishing the failure path from source; no target code, builds or tests were executed during this read-only review.

Is this the best way to solve the issue?

The patch fixes the cause by returning admission through existing navigation owners and protecting replacement reminders with an atomic identity check, without adding a competing state owner.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 1c7901f6cb1e.

Provenance checked

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

  • Current main still loses reminders: NotificationsPage invokes navigation and immediately dismisses the reminder without receiving an admission result. (src/OpenClaw.Tray.WinUI/Pages/NotificationsPage.xaml.cs:111, 1c7901f6cb1e)
  • Shared dismissal policy and stale-completion protection: NavigateActionAsync awaits admission, retains reminders on refusal or exceptions, and compares the captured immutable notification under the store lock before removal. (src/OpenClaw.Tray.WinUI/Services/AppNotificationService.cs:197, 745eea3b05e3)
  • Existing navigation boundaries preserved: Native admission rejects closed, unmounted and modal-deferred destinations; Dashboard dispatch awaits the existing launcher, and Settings admission checks the requested section identity. (src/OpenClaw.Tray.WinUI/Windows/WorkspaceWindow.xaml.cs:155, 745eea3b05e3)
  • Current native visual evidence inspected: The current-head before-action image shows the actionable Slack reminder; the after-admission image shows it removed. The PR reports native Debug and Release refusal, separate-retry and selective-dismissal observations, explicitly using simulated browser acceptance. (745eea3b05e3)
  • Validation and current workflow state: Current-head CI completed successfully, including core, Tray, UI/accessibility and CI Gate jobs. GitHub reports a clean merge and draft status. Raw job-log retrieval was blocked by the artifact-host allowlist; job results were readable. (745eea3b05e3)
  • Discussion and related work: The re-review request follows repairs to Dashboard admission and About identity. The same author's Inbox redesign changes notification presentation and remains adjacent work rather than an implemented replacement for this fix. The fully retrieved PR body matches captured SHA256 460ac137510d44de2da846154a11654101bc40bbe3a1e65d90e09ed56ad1918c. (745eea3b05e3)

Likely related people:

  • Régis Brid: Raw commit 06ed71f adds src/OpenClaw.Tray.WinUI/Services/AppNotificationService.cs:93 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 06ed71f3c01f; files: src/OpenClaw.Tray.WinUI/Services/AppNotificationService.cs)
  • Karen: Raw commit 3a58bf3 adds src/OpenClaw.Tray.WinUI/Presentation/WorkspaceNavigation.cs:36 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 3a58bf34902f; files: src/OpenClaw.Tray.WinUI/Presentation/WorkspaceNavigation.cs)

Preflight source

Source unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove merge-risk: 🚨 compatibility: Current PR review selected no merge-risk labels.

Label justifications:

  • P2: Unsuccessful notification actions can lose actionable reminders, with a limited notification-navigation blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • proof: sufficient: Contributor real behavior proof is sufficient.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-11-07T01:42:05.782Z sha 28a9346 :: blocked before merge. :: none

Reviewed November 8, 2026, 11:53 PM ET / November 9, 2026, 04:53 UTC (Revision 2).

bkudiess and others added 3 commits November 6, 2026 20:59
…utes

Integrate main while preserving Dashboard dispatch and the separate About page. Await confirmed browser launch before dismissing reminders, and retain reminders replaced or updated during navigation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add native same-window and browser-refusal admission coverage. Preserve the validated Clipboard lifecycle and elapsed-render prerequisites.

Co-authored-by: bkudiess <7658216+bkudiess@users.noreply.github.com>
Co-authored-by: bkudiess <7658216+bkudiess@users.noreply.github.com>
@roboclaw-bot

Copy link
Copy Markdown
Contributor

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. 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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Nov 9, 2026
@roboclaw-bot
roboclaw-bot marked this pull request as ready for review November 9, 2026 04:57
@roboclaw-bot roboclaw-bot added the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Nov 9, 2026
@roboclaw-bot
roboclaw-bot merged commit 7bcd9f5 into main Nov 9, 2026
37 checks passed
@roboclaw-bot
roboclaw-bot deleted the bkudiess-shell-navigation-corrections branch November 9, 2026 05:23
@roboclaw-bot roboclaw-bot removed the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. 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