Skip to content

refactor: extract OpenClaw.Connection project for independent testability#409

Merged
shanselman merged 13 commits into
openclaw:masterfrom
ranjeshj:user/ranjeshj/connectionchanges
May 15, 2026
Merged

refactor: extract OpenClaw.Connection project for independent testability#409
shanselman merged 13 commits into
openclaw:masterfrom
ranjeshj:user/ranjeshj/connectionchanges

Conversation

@ranjeshj
Copy link
Copy Markdown
Contributor

Summary

Extracts gateway connection management logic from \OpenClaw.Tray.WinUI\ into a new \OpenClaw.Connection\ project (
et10.0), enabling independent testing without WinUI dependencies.

Phase 1: Project extraction (30 files)

  • \GatewayConnectionManager, \GatewayRegistry, \ConnectionStateMachine, \CredentialResolver, \NodeConnector, \SshTunnelService/Manager, \SetupCodeDecoder, and all connection interfaces/DTOs/enums
  • New \OpenClaw.Connection.Tests\ project with 215 tests
  • Refactored \InteractiveGatewayCredentialResolver\ to drop \SettingsManager\ dependency
  • Removed \SshTunnelService.EnsureStarted(SettingsManager)\ overload

Phase 2: Architectural cleanup

  • Deleted legacy \GatewayCredentialResolver\ (zero production callers)
  • \OperatorClientChangedEventArgs\ now exposes \IOperatorGatewayClient\ (interface, not concrete)
  • Extracted \SshTunnelSnapshot\ — read-only record decouples UI from mutable service state
  • Created \ConnectionSettingsSnapshot\ — eliminates \SettingsData\ dependency from Connection layer
  • \ConnectionTrigger\ and \RetryPolicy\ made internal (implementation details)

Bug fixes

  • Approval command now uses pairing request ID (UUID) with deviceId fallback for older gateways
  • \NodePairingRequestId\ cleared when node leaves PairingRequired state (prevents stale reads)
  • Added node pairing approval requests to Connection page (was only on Nodes page)
  • Warning log added for silent \IOperatorGatewayClient\ to \OpenClawGatewayClient\ cast

Dependency graph

\
OpenClaw.Shared (net10.0) -- transport/protocol
|
OpenClaw.Connection (net10.0) -- connection lifecycle, registry, credentials
|
OpenClaw.Tray.WinUI (net10.0-windows) -- UI app
\\

Validation

  • build.ps1 pass
  • OpenClaw.Shared.Tests: 1,572 passed
  • OpenClaw.Connection.Tests: 215 passed
  • OpenClaw.Tray.Tests: 987 passed

ranjeshj and others added 11 commits May 14, 2026 21:55
…us visuals

- Remove Connection Log section (redundant with diagnostics timeline)
- Remove 'This Device' card (info available on Debug page)
- Move diagnostics button to Debug page
- Replace 3 stacked connection cards with Pivot tabs:
  Setup Code | Recent Gateways | Direct Connect
- Replace confusing state-machine pill pipeline with clean
  two-row Operator/Node status using accent mini-cards
- Add left accent stripe colored by role state
- Tint status card background for error/pairing states
- Auto-select contextual default tab based on connection state
- Wrap Pivot in card container for visual cohesion

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove PresenceCount ('X clients') from connection page detail line
- Remove PresenceCount from title bar status text
- Title bar now shows: Connected · v{version} · Op:✓ Node:✓
  with role-specific indicators (✓ connected, … connecting, ✗ error, – off)

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

- Clicking the status/version/role area in the title bar navigates
  to the Connection page
- Added a ToggleSwitch to the Node accent card so node mode can be
  toggled directly from the connection page status card
- Toggle persists to settings and raises SettingsSaved

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- ConnectionAttemptsText uses Opacity instead of Visibility to avoid
  layout shift; MinHeight reserves space even when hidden
- Debounce overall status updates: only update dot/text/tint when
  OverallState actually changes, preventing Connected→Connecting→Connected
  flicker during node reconnection

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Operator label gets Width=60 to match Node label alignment
- Subtitle simplified: 'Manage your gateway connection.'
- Pivot card bottom padding reduced from 16 to 8

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- SSH toggle and config fields extracted from Direct Connect tab
  into a standalone Expander with its own card-style container
- Expander collapsed by default, toggle in header for quick access
- Direct Connect tab is now cleaner: just URL, token, and Connect
- SSH toggle now persists setting immediately on change

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract 30 connection management files into a new OpenClaw.Connection
project (net10.0) for independent testability without WinUI dependencies.

Phase 1 - Project extraction:
- Move GatewayConnectionManager, GatewayRegistry, ConnectionStateMachine,
  CredentialResolver, NodeConnector, SshTunnelService/Manager, SetupCodeDecoder,
  and all connection interfaces/DTOs/enums
- Refactor InteractiveGatewayCredentialResolver to drop SettingsManager dependency
- Remove SshTunnelService.EnsureStarted(SettingsManager) overload
- Create OpenClaw.Connection.Tests with 215 tests

Phase 2 - Architectural cleanup:
- Delete legacy GatewayCredentialResolver (zero production callers)
- OperatorClientChangedEventArgs now exposes IOperatorGatewayClient
- Extract SshTunnelSnapshot for read-only UI consumption
- Extract ConnectionSettingsSnapshot to decouple from SettingsData
- Fix approval command to use pairing request ID instead of device ID
- Add node pairing approval requests to Connection page

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
These types are implementation details of the connection state machine
and retry logic. No external consumers need them.

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

- Approval command falls back to deviceId when gateway returns no requestId
  (fixes regression for older gateways without requestId support)
- ConnectionStateMachine.RebuildSnapshot clears NodePairingRequestId when
  node leaves PairingRequired state (mirrors operator-side behavior)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hine, and lifecycle details

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

Critical fixes:
- NodesPage: gate Approve/Reject behind operator.admin/pairing scope
- NodesPage: add error handling and list refresh for approval buttons
- ConnectionPage: remove 15s polling loop, use existing StateChanged handler
- App.OpenDashboard: use credential resolver instead of manual URL stitching

Structural improvements:
- Add ConnectWithSharedTokenAsync to IGatewayConnectionManager
- Extract DeviceIdentityStore.ClearStoredTokens (was duplicated 3x)
- Delete SshTunnelManager shim; SshTunnelService implements ISshTunnelManager
- Add ConnectionStateMachine mutator methods (replace 4 external writes)
- Add OperatorScopeHelper.CanApproveDevices (shared across 4 call sites)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@shanselman shanselman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing the connection extraction. The new OpenClaw.Connection project and moved tests validate cleanly locally on top of current master, but I found a few behavior regressions that should be fixed before merge.

Blocking:

  1. Dashboard can now put a per-device token in the browser URL. In App.xaml.cs, OpenDashboard() now uses TryResolveChatCredentials() and appends any non-bootstrap token. Before this PR, the dashboard only appended GatewayRecord.SharedGatewayToken; if the active gateway had only a paired device token, no query token was added. With this change, a per-device token can land in browser history/referrers and may not match the dashboard HTTP auth contract. Please only append token= for CredentialResolver.SourceSharedGatewayToken; otherwise route to Connection settings or open without a token intentionally.

  2. Direct Connect no longer rolls back on async connection failure/timeout. ConnectionPage.OnDirectConnect() still saves/activates the new record and clears stored tokens before connecting, but the previous wait loop for Connected/PairingRequired/Error was removed. GatewayConnectionManager.ConnectAsync() starts the actual websocket connect fire-and-forget, so auth failures/timeouts won’t throw back to the catch block. A mistyped token can replace a working active gateway and leave the user stranded. Please restore outcome-aware rollback, or expose/use a manager API that returns a definitive connect result before committing the state.

  3. Disabling SSH tunnel leaves stale tunnel error/status. The old SshTunnelService.EnsureStarted(SettingsManager) reset LastError = null and Status = NotConfigured when UseSshTunnel was false. The new inline EnsureSshTunnelStarted() path only calls Stop(), which can leave a previous failed tunnel surfaced in diagnostics/UI after the user turns SSH off. Please restore the reset semantics.

Also worth fixing in the same pass: the browser-proxy port-skip warning from the old SSH helper was dropped, and NodeConnector.NodeDeviceId changed from ShortDeviceId to FullDeviceId; if full IDs are intentional for approval fallback, the Connection page display should probably truncate the value while preserving the full approval ID.

Local current-master validation passed in D:\github\moltbot-windows-hub.pr409-review: ./build.ps1; OpenClaw.Connection.Tests 215/215; Shared 1572/1600 with 28 skipped; Tray 987/987.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@shanselman shanselman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer-side follow-up c446ffd addresses the requested blockers: dashboard query tokens are limited to shared gateway tokens, Direct Connect waits for a terminal connected/pairing/error/timeout outcome before keeping the new active record, and disabling SSH resets stale tunnel status/error. Added regression coverage for the token condition, Direct Connect wait path, and SSH reset.\n\nValidated in \D:\github\moltbot-windows-hub.pr409-fix: .\build.ps1; Connection tests 216/216; Shared tests 1572/1600 with 28 skipped; Tray tests 989/989.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman shanselman merged commit 8b699f2 into openclaw:master May 15, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants