Skip to content

Surface accurate node mode and MCP-only states#827

Merged
shanselman merged 4 commits into
openclaw:mainfrom
bkudiess:bkudiess-node-mode-ui-enablement
Jun 26, 2026
Merged

Surface accurate node mode and MCP-only states#827
shanselman merged 4 commits into
openclaw:mainfrom
bkudiess:bkudiess-node-mode-ui-enablement

Conversation

@bkudiess

@bkudiess bkudiess commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: Node/MCP settings could present misleading state: MCP-only could look disabled, and one credentialed MCP-only startup path did not start the local MCP server after a successful operator startup connect.
  • Why it matters: Users need truthful UI for whether capabilities are being served locally. MCP-only mode must not silently start a gateway node, and it must still start local MCP when an operator connection exists.
  • What changed: Added explicit MCP-only and node-starting UI states, surfaced MCP startup errors, kept local MCP-served controls actionable while keeping gateway-only browser proxy disabled in MCP-only mode, and separated gateway-node startup gating from local NodeService/MCP initialization.
  • What did NOT change: No credential precedence changes, no MCP token model changes, no new node capability commands, and no broader exec approval policy changes.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs / instructions
  • Tests / validation
  • Security hardening
  • Chore / infra

Scope (select all touched areas)

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

Linked Issue/PR

  • Closes #
  • Related #
  • Related to a bug or regression

Validation

  • ./build.ps1
    Result: all projects built successfully.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
    Result: 2513 passed, 31 skipped, 0 failed.
  • dotnet test ./tests/OpenClaw.Connection.Tests/OpenClaw.Connection.Tests.csproj --no-restore
    Result: 373 passed, 0 failed.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~NodeModeUiStateTests|FullyQualifiedName~NodeCapabilityGatingTests|FullyQualifiedName~LocalizationValidationTests"
    Result: 49 passed, 0 failed.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore
    Result: local full-suite run currently hits unrelated AssistantBridgeServiceTests.StartListenServiceAsync_KillsTimedOutBackendCommand timing failure; that test passes when run isolated. Branch-affected Tray tests and localization validation pass.

Real behavior proof

Screenshot: Connection page MCP-only state with active operator gateway

image

Screenshot: Permissions page MCP-only state

image

Mixed operator-connected startup proof

Raw proof file: https://raw.githubusercontent.com/bkudiess/openclaw-windows-node/proof/pr-827-node-ui/mixed-startup-proof.md

Settings: EnableMcpServer=true, EnableNodeMode=false.
Profile: copied active local gateway profile with operator credential; secrets redacted.

[INFO] Connecting to last successful gateway during startup: ws://<gateway> (identity.DeviceToken)
[INFO] gateway connected, waiting for challenge...
[INFO]   role=operator, clientId=cli, mode=cli
[INFO] [HANDSHAKE] Received hello-ok!
[INFO] [MCP] HTTP server listening on http://<host>:8765/
[INFO] Started MCP-only node service without gateway connection
[INFO] [App] Skipping local NodeService auto-connect because node mode is disabled

MCP server proof artifact

Raw proof file: https://raw.githubusercontent.com/bkudiess/openclaw-windows-node/proof/pr-827-node-ui/mcp-proof.md

GET /:
OpenClaw MCP server. POST JSON-RPC to http://127.0.0.1:8765/

tools/list:
- total tools: 44
- selected tools: system.notify, system.which, canvas.present, screen.snapshot, camera.list, location.get, device.info

tools/call device.info:
{"systemName":"Windows","appVersion":"0.6.4-bkudiess-node-mode-ui-enablement.1","locale":"en-US"}

MCP server logs:
[INFO] Starting Windows Node in MCP-only mode (no gateway)
[INFO] Capabilities registered: system, canvas, screen, camera, location, device (6 caps)
[INFO] [MCP] HTTP server listening on http://<host>:8765/
[INFO] Started MCP-only node service without gateway connection
[DEBUG] [MCP] tools/call device.info

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No
  • If any Yes, explain risk + mitigation: N/A. This PR changes UI/status/gating around existing node and MCP surfaces; it does not add commands, broaden network exposure, or change credential/token handling.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No
  • If yes, exact upgrade steps: N/A.

Review Conversations

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

Addressed ClawSweeper findings:

  • Local MCP now starts after successful operator startup connects when EnableMcpServer=true and EnableNodeMode=false.
  • Browser proxy toggle remains disabled in MCP-only mode because browser proxy requires a gateway node client.
  • MCP startup errors are surfaced even when EnableNodeMode=true and the gateway node role is connected.

@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 5:52 PM ET / 21:52 UTC.

Summary
The PR separates gateway-node startup gating from local MCP-only serving, adds explicit MCP-only/starting/error tray UI states, updates localization, and adds regression/E2E test coverage.

Reproducibility: yes. Source inspection gives a high-confidence path: current docs define independent Node/MCP modes, while current main uses a combined NodeService predicate for gateway-node startup and collapses node-off/MCP-on to a generic off UI state.

Review metrics: 3 noteworthy metrics.

  • Diff scope: 15 files, +653/-83. The patch spans startup gating, WinUI state rendering, localization, shared tests, tray tests, and setup E2E behavior.
  • Real proof artifacts: 2 screenshots, 2 raw logs. The proof directly shows the changed MCP-only UI states and live MCP startup/tool-call behavior.
  • Latest check state: 4 pending, 4 pass, 2 skipped. Maintainers should wait for the latest full test and E2E jobs before merging a lifecycle-sensitive change.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Clear the required full Tray validation path or have a maintainer document the reported timing failure as unrelated.

Risk before merge

  • [P1] The diff changes node/MCP lifecycle gating; an incorrect merge could leave local MCP unavailable, accidentally connect a gateway node from MCP-only settings, or misreport startup state.
  • [P1] The PR body reports the required full Tray test run hit an unrelated timing failure locally, and live GitHub checks were still pending for the latest head during this review.

Maintainer options:

  1. Clear lifecycle validation before merge (recommended)
    Wait for the latest Build and Test jobs plus the required full Tray validation path to pass, or have a maintainer document why the reported Tray timing failure is unrelated before landing.
  2. Accept the supplied runtime proof
    Maintainers can merge based on the inspected screenshots and raw MCP/startup logs while explicitly owning the remaining validation uncertainty.
  3. Pause for an independent Windows smoke
    Keep this PR open until a maintainer repeats the Connection, Permissions, and mixed operator-connected MCP-only startup paths on a clean Windows tray setup.

Next step before merge

  • [P2] This active implementation PR has sufficient proof and no narrow automated repair finding; maintainers need to complete validation and make the lifecycle merge-risk decision.

Security
Cleared: No concrete security or supply-chain regression was found; the diff does not add dependencies, credentials handling, capability commands, or new network exposure.

Review details

Best possible solution:

Land after the required CI and full Tray validation are green, or after a maintainer explicitly accepts the reported Tray timing failure as unrelated while preserving the documented Node/MCP mode matrix.

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

Yes. Source inspection gives a high-confidence path: current docs define independent Node/MCP modes, while current main uses a combined NodeService predicate for gateway-node startup and collapses node-off/MCP-on to a generic off UI state.

Is this the best way to solve the issue?

Mostly yes. Separating gateway-node enablement from local MCP lifecycle and adding explicit MCP-only UI state matches the documented mode matrix; the remaining blocker is validation and maintainer acceptance, not a known code defect.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a broken node/MCP workflow where local capabilities can be unavailable or misrepresented after startup.
  • merge-risk: 🚨 availability: The diff changes node/MCP lifecycle gating, and an incorrect merge could leave local MCP or gateway node mode unavailable.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR body and inspected artifacts include after-fix screenshots plus raw startup and MCP logs showing the changed UI and live MCP-only behavior with private details redacted.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and inspected artifacts include after-fix screenshots plus raw startup and MCP logs showing the changed UI and live MCP-only behavior with private details redacted.
Evidence reviewed

What I checked:

  • Repository policy applied: AGENTS.md was read fully; its connection, MCP, Windows node, tray UX, and validation guidance applies because this PR changes those surfaces. (AGENTS.md:1, f51a8618ed7b)
  • Documented mode contract: The current connection architecture docs define EnableNodeMode and EnableMcpServer as independent, with node off and MCP on meaning local MCP server only with no gateway required. (docs/CONNECTION_ARCHITECTURE.md:164, f51a8618ed7b)
  • MCP-only contract: MCP_MODE.md says the tray app can run MCP-only with no gateway connection and expose local tools over the loopback MCP HTTP server. (docs/MCP_MODE.md:9, f51a8618ed7b)
  • Current-main startup mismatch: Current main passes ShouldInitializeNodeService into GatewayConnectionManager, and that predicate includes EnableMcpServer, so MCP-only can satisfy gateway-node startup gating. (src/OpenClaw.Tray.WinUI/App.xaml.cs:647, f51a8618ed7b)
  • Current-main UI mismatch: Current main maps EnableNodeMode=false directly to NodeCardState.Off, so MCP-only mode is not represented as its own Connection-page node state. (src/OpenClaw.Tray.WinUI/Pages/ConnectionPagePlan.cs:688, f51a8618ed7b)
  • PR startup split: At PR head, GatewayConnectionManager receives IsGatewayNodeEnabled, startup node-only connect checks IsGatewayNodeEnabled, and post-operator NodeService auto-connect returns when node mode is disabled. (src/OpenClaw.Tray.WinUI/App.xaml.cs:649, 7e31a450dfaf)

Likely related people:

  • codemonkeychris: The original local MCP HTTP server mode and independent EnableNodeMode/EnableMcpServer contract appear to date to the local MCP mode commit. (role: original MCP mode contributor; confidence: high; commits: a3d884f4c4ba; files: docs/MCP_MODE.md, src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Services/NodeService.cs)
  • ranjeshj: History shows connection architecture, MCP runtime-toggle, capability-count, and tray UX work in the same product area. (role: connection and tray UX area contributor; confidence: high; commits: 03e560a05ce5, d7d7661ac28c, 7d59d8d52f47; files: docs/CONNECTION_ARCHITECTURE.md, src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml.cs)
  • shanselman: History includes a prior MCP-only startup restoration, and the latest PR-head commits repair MCP-only visibility and prevent gateway joining in MCP-only mode. (role: recent MCP-only and PR-head contributor; confidence: high; commits: 49bc64708d2a, df99d37d8100, f4018fb035ae; files: src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml.cs, src/OpenClaw.Tray.WinUI/Pages/ConnectionPagePlan.cs)
  • bkudiess: Current-main history includes adjacent Connection-page recovery/status and NodeService readiness work beyond opening this PR. (role: recent adjacent contributor; confidence: medium; commits: f51a8618ed7b, ff5cafb52f52, 429be9ba9368; files: src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml.cs, src/OpenClaw.Tray.WinUI/Pages/ConnectionPagePlan.cs, src/OpenClaw.Tray.WinUI/Services/NodeService.cs)
  • indierawk2k2: Earlier connection unification work added GatewayConnectionManager ownership and EnsureNodeConnectedAsync, which this PR relies on when separating gateway-node and local MCP lifecycles. (role: connection lifecycle contributor; confidence: medium; commits: 0816eb0eefc0, d0e184bd29c8, 389f499eb618; files: src/OpenClaw.Connection/GatewayConnectionManager.cs, src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Services/NodeService.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

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.

@bkudiess bkudiess force-pushed the bkudiess-node-mode-ui-enablement branch from 0629436 to 72a351c Compare June 26, 2026 07:05
@bkudiess bkudiess changed the title Surface node mode state and add proof validation guidance Surface accurate node mode and MCP-only states Jun 26, 2026
@bkudiess bkudiess force-pushed the bkudiess-node-mode-ui-enablement branch from 72a351c to 1c3e816 Compare June 26, 2026 07:07
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. and removed status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels Jun 26, 2026
@bkudiess bkudiess force-pushed the bkudiess-node-mode-ui-enablement branch from 1c3e816 to 0eeb7f8 Compare June 26, 2026 07:17
@bkudiess

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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 Jun 26, 2026
@bkudiess

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 26, 2026
@bkudiess

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@bkudiess bkudiess force-pushed the bkudiess-node-mode-ui-enablement branch from 0eeb7f8 to fe6758e Compare June 26, 2026 16:08
@bkudiess

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@bkudiess bkudiess marked this pull request as ready for review June 26, 2026 16:12
@bkudiess

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 26, 2026
…ode gating

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Surface the local MCP-only node card even when no gateway/operator session exists, and make the reconnect-backoff test wait for server-side accept publication.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman shanselman force-pushed the bkudiess-node-mode-ui-enablement branch from fe6758e to df99d37 Compare June 26, 2026 21:12
Gate the post-operator local NodeService auto-connect on EnableNodeMode so local MCP-only serving does not create gateway node pairing requests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 26, 2026
The shared-token setup path can now remain MCP-only after operator approval, so the E2E needs to request node reconnect explicitly before waiting for a node credential.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman shanselman merged commit 9ac8cfb into openclaw:main Jun 26, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P1 Urgent regression or broken agent/channel workflow affecting real users now. 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