ConnectionPage polish + per-page "Back to Connection" link#419
Merged
shanselman merged 5 commits intoMay 17, 2026
Conversation
… badge
User-visible bugs:
- "Copy command" and "Re-pair" red strip CTAs duplicated inline actions
(RecoveryApproveCmdBlock copy button, RecoveryAuthPasteBlock paste+Apply,
ConnectionToggle) so they were noisy on top of caution/critical statuses.
- Pairing approve command emitted verb-first ("openclaw approve node <id>")
which silently failed when pasted on the gateway host. The CLI is
noun-first: "openclaw nodes approve <id>" / "openclaw devices approve <id>".
- Glance chip showed "0/1 channels" yellow when one channel was healthy
because the chip required IsLinked AND a narrower healthy whitelist than
ChannelHealth.IsHealthyStatus (status="running"/"active" weren't counted).
- Saved gateway row badge said "Connected" while the strip up top said
"Connecting..." / "Awaiting approval" — BuildSavedGatewayRowControl
collapsed Connecting and PairingRequired into the Connected branch.
Fixes:
- ConnectionPagePlan.cs: BuildNodeApproveCommand /
BuildDevicePairingApproveCommand use noun-first; missing requestId
emits a discovery hint ("openclaw nodes pending # then: ...") instead
of a bare approve the CLI rejects.
- ConnectionPagePlan.cs: drop StripPrimaryLabel/Action for
RecoveryCategory.Pairing and Auth. Retire CopyApproveCommand and Rep
enum values; matching arms removed from OnStripPrimaryClicked.
- ConnectionPage.xaml: drop AccentButtonStyle from StripPrimaryButton
and from RecoveryAuthPasteBlock Apply (matches AddSaveButton parity).
- ConnectionPage.xaml.cs: per-row Approve/Deny in device + node pairing
cards use plain Button + leading Check / Exit glyphs (success /
critical brushes) via new BuildPairingDecisionButton helper —
complies with Fluent "one accent per view" when multiple pending
requests stack. Approve/Deny disabled-state behavior preserved.
- ConnectionPage.xaml: add ToolTip + AutomationProperties.Name to the
inline Copy buttons in NodeApproveCmdBox and RecoveryApproveCmdBlock.
- ConnectionPageChannelMetrics.cs: new pure helper
CountHealthyChannels using ChannelHealth.IsHealthyStatus and not
requiring IsLinked. ApplyGlanceChips delegates to it.
- ConnectionPage.xaml.cs: BuildActiveRowBadge maps each
OverallConnectionState to the correct (glyph, brush, label) — green
Connected only for Connected/Ready/Degraded; caution Connecting... /
Awaiting approval for transient states; neutral Disconnecting...;
null (-> [Connect] button) otherwise. The new hasLiveAffordance
flag drives the overflow menu's Disconnect/Edit vs Edit/Remove split.
Tests:
- ConnectionPageApproveCommandTests: pin noun-first commands and the
missing-requestId discovery-hint fallback.
- ConnectionPageChannelMetricsTests: cover each healthy /
non-healthy ChannelHealth status, IsLinked=false healthy counts,
case-insensitivity, and mixed sets.
Validation:
- ./build.ps1: all projects green.
- dotnet test OpenClaw.Shared.Tests: 1620 passed (28 skipped) — unchanged.
- dotnet test OpenClaw.Tray.Tests: 976 passed (was 966; +10 new).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both reviewers (Claude Opus 4.6 + GPT-5.2-codex) ran against 6544d03. This commit fixes the 5 HIGH-consensus findings plus the 2 LOW-consensus ones (Error-state badge UX call + test repo-root resilience). 1. Active-row badge accessibility (HIGH consensus) BuildActiveRowBadge now sets AutomationProperties.Name and LiveSetting=Polite on the badge container so screen readers re-announce when the row transitions Connecting -> Awaiting approval -> Connected. 2. Disconnect during Disconnecting (HIGH consensus) New ConnectionPageRowState.CanDisconnectFromBadge pure predicate excludes Disconnecting from the overflow's Disconnect item, so the user can't re-enter a teardown that's already in flight. Predicate lives in its own file so the test project (pure net10.0) can pin it via behavior tests. 3. Stuck Approve/Deny on success (HIGH consensus, codex-caught asymmetry) Node-pair Approve/Deny now call RequestNodePairListAsync on success, matching device-pair behavior. Both flows go through a shared RunPairingDecisionAsync helper that: - disables both buttons during the call - re-enables on failure / missing client / exception - on success, arms an 8 s DispatcherTimer watchdog that re-enables if the gateway never pushes a list-updated event (a dropped websocket frame used to leave the row permanently inert). 4. cmd.exe-hostile discovery hint (HIGH consensus, severity disputed) Missing-id fallback no longer embeds "# then: ... <requestId>" in the clipboard text. The # was parsed as a literal arg by cmd.exe and <requestId> as input redirection, breaking the paste-and-run flow for Windows-cmd users. Clipboard now contains only the shell-safe discovery command (e.g. "openclaw nodes pending"); the UI prose around the box already implies the follow-up. 5. Brittle approve-cmd source-parse tests (HIGH consensus) ConnectionPageApproveCommandTests now assert on the command prefix ("openclaw nodes approve ") instead of the full interpolated literal, and use a custom AssertContainsCli helper that surfaces a contract-explaining message on failure. A new AssertNoShellHostileChars guard scans the emission method bodies (not the whole file) for # / < / > inside string literals, so the cmd.exe regression is caught structurally instead of by substring. 6. Active row in Error indistinguishable from inactive (LOW consensus, codex-only — accepted as UX improvement) BuildActiveRowBadge now maps OverallConnectionState.Error to a critical-colored "Error" badge instead of falling through to [Connect]. CanDisconnectFromBadge includes Error so the overflow still offers Disconnect / teardown. 7. Test repo-root walk failing in CI artifact copies (LOW consensus, codex-only — accepted defensively) GetRepositoryRoot now has a third fallback: probe upward from the [CallerFilePath]-captured source location of the test file before throwing. OPENCLAW_REPO_ROOT env var remains the primary escape hatch. New tests: - ConnectionPageRowStateTests: CanDisconnectFromBadge true for live / pending / Error, false for Disconnecting / Idle; HasActiveRowBadge true for every non-Idle state. - Extended ConnectionPageApproveCommandTests with the shell-safety scan and a more refactor-tolerant prefix assertion. Validation: - ./build.ps1: all projects green. - dotnet test OpenClaw.Shared.Tests: 1620 passed (28 skipped) - unchanged. - dotnet test OpenClaw.Tray.Tests: 992 passed (was 976; +16 new). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…retry User preferred the original Codex-flagged behavior: in an Error state the active gateway row should render [Connect] (so the user can explicitly retry, with the URL visible right next to it) instead of an "Error" badge that only mirrors what the status strip + Recovery card up top already say. The strip already shouts "Can't reach gateway / <category>", the Recovery card right under it offers Disconnect — duplicating either signal on the row removes the one thing the row uniquely offers: a per-gateway action surface. The status strip currently does not even show the gateway URL in Recovery, so the row's URL + [Connect] is in fact the clearest way for the user to confirm what they're retrying. Changes: - BuildActiveRowBadge: drop the Error arm, fall back to null (row renders [Connect]). - ConnectionPageRowState.CanDisconnectFromBadge: remove Error (no badge → no Disconnect overflow; the Recovery card carries it). - ConnectionPageRowState.HasActiveRowBadge: explicit set of live / transient states, no longer "not Idle". - ConnectionPageRowStateTests: update theory data, add explicit Error-state assertions documenting the design choice. Validation: - ./build.ps1: green. - dotnet test (ConnectionPage filter): 40 passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkudiess
changed the title
Cross-page links on Connection (Open Sessions / Open Instances / Open Permissions) were one-way trips with no return path. Add an inline "← Back to Connection" HyperlinkButton at the top of each destination page that's only visible when the user actually arrived via one of those links — staying hidden when the user navigated via the rail keeps the page chrome quiet for direct navigation. Implementation: - HubWindow.NavigateTo(string tag, string? originTag) overload stashes the origin in _pendingNavigationOrigin; consumed once by a new Frame.Navigated handler and surfaced as HubWindow.LastNavigationOrigin for the destination page's Initialize() to read. - Navigation funnel refactored into NavigateTo -> NormalizeNavTag -> NavigateInternal -> Frame.Navigate(pageType, tag) so navigation identity is (PageType, normalized tag), not just PageType. Needed because agent-scoped pages (agent:main:sessions) and top-level (sessions) tags both map to SessionsPage. - OnContentFrameNavigated becomes the single post-nav dispatcher: updates _currentNavTag / _currentAgentId / LastNavigationOrigin and re-runs InitializeCurrentPage() exactly once. - ConnectionPage's three cross-page link handlers now pass "connection" as the origin tag. - Inline link added to Sessions/Instances/Permissions; Initialize toggles visibility based on hub.LastNavigationOrigin == "connection". Reviewed via Hanselman dual-model adversarial review (Opus + Codex). Both models flagged HIGH-severity origin leak: _pendingNavigationOrigin wasn't cleared on NavigateInternal early-return paths (dedupe or unknown tag), so a later unrelated navigation could inherit a stale origin and surface a wrong back link. Fixed by clearing it in both early-return branches. Validated: ./build.ps1 green, Shared.Tests 1620 passed / 28 skipped, Tray.Tests 1001 passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkudiess
changed the title
Contributor
|
🤖 This is an automated response from Repo Assist. Nice set of fixes and a very thorough PR description — the Hanselman dual-model review and the documented design decision on the Error-row badge are a great pattern to follow. A few observations from the diff:
Overall the PR looks solid. +26 tests with behaviour-level coverage on the pure helpers is exactly the right approach.
|
23 tasks
Resolve conflicts with the AppState and gateway connection manager changes on current master. Preserve the Connection page cross-links, pairing refresh behavior, and origin-aware inline back links while keeping the newer service architecture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Polish pass over the rebuilt
ConnectionPage(#418) — four user-visible bugs + Fluent guideline cleanup.Bugs fixed
openclaw approve node <id>which the CLI silently rejects. Now noun-first:openclaw nodes approve <id>/openclaw devices approve <id>. Missing-id fallback emits a single shell-safe discovery command.0/1yellow when a single channel was healthy. Chip requiredIsLinkedAND a narrower healthy whitelist thanChannelHealth.IsHealthyStatus;running/activeweren't counted. Now uses the canonical predicate, drops theIsLinkedgate.BuildActiveRowBadgemaps eachOverallConnectionStateto the correct (glyph, brush, label).Errordeliberately falls back to[Connect]so the user can explicitly retry with the URL visible — the strip + Recovery card already carry the failure status.RecoveryApproveCmdBlockCopy button and theRecoveryAuthPasteBlockpaste-and-Apply affordance. Both removed;StripPrimaryButtondropsAccentButtonStyleso the survivingRestartTunnel/Retrycases stop reading red.Fluent guideline cleanup
Approve/Denyin the pending-pair lists no longer useAccentButtonStyle(violated "one accent per view" when multiple requests stacked). PlainButton+ leadingCheck/Exitglyphs in success / critical brushes via newBuildPairingDecisionButtonhelper. The outerPendingApprovalsBanner(caution-yellow + bold count +LiveSetting=Polite) already carries page-level attention.RecoveryAuthPasteBlockApplydropsAccentButtonStylefor parity withAddSaveButtonSave & connect.AutomationProperties.Name+LiveSetting=Politeso screen readers re-announce on state transitions.Robustness
Disconnectingno longer offersDisconnectin the overflow (was re-entering teardown).Approve/Denynow callRequestNodePairListAsyncon success (symmetric with device-pair). Both flows go through a sharedRunPairingDecisionAsyncwith an 8sDispatcherTimerwatchdog so a dropped websocket frame can't leave the buttons permanently disabled.ToolTipService.ToolTip+AutomationProperties.Name.Tests
New:
ConnectionPageChannelMetricsTests,ConnectionPageApproveCommandTests(with a shell-safety scan that fails on#/</>in emission method bodies),ConnectionPageRowStateTests. +26 new tests, all behavior-level where possible (pure helpers extracted toConnectionPageChannelMetrics.csandConnectionPageRowState.csso the net10.0 test project can link them).Validation
./build.ps1— all projects green.dotnet test OpenClaw.Shared.Tests— 1620 passed, 28 skipped (unchanged baseline).dotnet test OpenClaw.Tray.Tests— 992 passed (was 966; +26 new).Review
Cross-referenced with the Hanselman dual-model review (Claude Opus 4.6 + GPT-5.2-codex). All 5 HIGH-consensus findings addressed; one LOW-consensus design-call (Error row badge vs
[Connect]retry button) was deliberately reverted after user UX feedback — the strip + Recovery card already carry the failure status, and[Connect]is more actionable.Follow-up: "Back to Connection" inline link (9c9f705)
Connection's cross-page links (Open Sessions / Open Instances / Open Permissions) were one-way trips. Adds a small "← Back to Connection"
HyperlinkButtonat the top of each destination page, only visible when the user actually arrived via one of those links — staying hidden when the user navigates via the rail keeps the chrome quiet for direct navigation. Chosen over a chromeNavigationView.IsBackButtonVisibleback arrow (which felt visually noisy and redundant with the rail) per the WinUI backwards-navigation guidance trade-offs.Plumbing. New
HubWindow.NavigateTo(string tag, string? originTag)overload stashes the origin in_pendingNavigationOrigin, consumed once by a newFrame.Navigatedhandler and surfaced asHubWindow.LastNavigationOriginfor the destination'sInitialize(). The navigation funnel is also refactored:NavigateTo→NormalizeNavTag→NavigateInternal→Frame.Navigate(pageType, tag), so navigation identity is(PageType, normalized tag)rather than justPageType— necessary because agent-scoped pages (e.g.agent:main:sessions) and top-level (sessions) tags both map toSessionsPage.OnContentFrameNavigatedbecomes the single post-nav dispatcher (updates_currentNavTag/_currentAgentId/LastNavigationOriginand re-runsInitializeCurrentPage()exactly once).Hanselman dual-model review (Opus + Codex). Both models flagged a HIGH-severity origin leak:
_pendingNavigationOriginwasn't cleared onNavigateInternalearly-return paths (dedupe or unknown tag), so a later unrelated navigation could inherit a stale origin and surface a wrong back link. Fixed by clearing the field in both early-return branches.Validation.
./build.ps1✅ ·Shared.Tests1620 passed / 28 skipped ·Tray.Tests1001 passed.