Skip to content

refactor: add a replaceable node runtime and sidecar adapter proof - #1068

Draft
giodl73-repo wants to merge 33 commits into
openclaw:mainfrom
giodl73-repo:agent/rust-node-sidecar-seam
Draft

refactor: add a replaceable node runtime and sidecar adapter proof#1068
giodl73-repo wants to merge 33 commits into
openclaw:mainfrom
giodl73-repo:agent/rust-node-sidecar-seam

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented Jul 30, 2026

Copy link
Copy Markdown

Summary

Adds one replaceable Windows node-runtime boundary, one shared Windows capability
dispatcher, and a non-selectable C# conformance consumer of OpenClaw's proposed
authenticated Rust sidecar contract. It is independent contract proof, not a
second production runtime.

This consolidated Windows adopter PR now:

  • introduces INodeRuntimeClient and an injectable factory while keeping
    WindowsNodeClient selected by default;
  • moves NodeService and A2UI consumers behind that runtime contract;
  • extracts command indexing, bounded execution, duplicate tracking,
    cancellation, telemetry, and completion into the transport-independent
    NodeCapabilityDispatcher;
  • requires substitute runtimes to honor connection cancellation and retire
    cancelled candidates without blocking later reconnects;
  • independently implements the authenticated framing, handshake, immutable
    configuration, admission, invocation, cancellation, result, and status
    contracts proposed by OpenClaw PR3; and
  • routes admitted ordinary native invocations through the same
    NodeCapabilityDispatcher, without adding a second Windows policy or
    execution path.

The former stacked fork proof
#2 has been
fast-forwarded into this branch. The C# runtime remains the production default.

Position in the series

  1. OpenClaw #116050 — shared
    Gateway client, bounded node-host foundation, headless binary, and Linux
    Tauri convergence.
  2. OpenClaw #116450 — signing,
    lifecycle/reconnect, duplex invocation, admission, Gateway authority,
    connection manifests, and shared conformance.
  3. OpenClaw #116863
    authenticated/versioned sidecar framing, handshake, immutable configuration,
    and the bounded ordinary-command runtime bridge.
  4. This PR — the Windows runtime seam, shared capability dispatcher, and
    independent C# adopter proof.

OpenClaw and openclaw-windows-node are separate repositories, so this PR
cannot be Git-rebased onto #116863. The dependency is contractual: this branch
pins and reproduces #116863's three fixture corpora at OpenClaw head
71c1c8cb23c5647dc07fd4ee1f8663068c92a482. Production Rust selection remains
blocked until that contract is accepted and the remaining adoption gates close.

Ownership boundary

RFC #54 proposes that OpenClaw own the reusable protocol, Gateway authority
semantics, Rust runtime, and conformance corpora. Under that boundary, Windows
retains WinUI, operator behavior, MCP, approvals, native capability handlers,
process/artifact verification, protected bootstrap, concrete IPC, audit,
packaging, rollout, and rollback.

WindowsNodeClient still owns production Gateway wire parsing and response
framing. NodeCapabilityDispatcher owns Windows capability execution. The
sidecar adapter can only deliver authenticated, admitted ordinary invocations
into that dispatcher; it is not an INodeRuntimeClient, launches no process,
chooses no transport, and cannot be selected.

The generic Rust runtime still rejects the reserved system.* namespace. The
existing Windows system.run family therefore stays on C# until OpenClaw owns
an explicit authorization mechanism.

Remaining adoption gates

Before Rust can become selectable, Windows still needs verified artifact/process
launch, protected credential handoff, concrete local IPC, live Gateway pairing
and issued-token lifecycle, duplex sidecar input/progress/heartbeat transport,
process/crash supervision, health and resource proof, product audit export,
packaging, rollout, and rollback.

Validation

  • Current rebased head: focused RustSidecar/NodeCapabilityDispatcher tests — 63/63 passed
  • Current rebased head: full Shared suite — 3,701 passed, 32 environment-dependent skips
  • Current rebased head: focused NodeConnector Connection tests — 22/22 passed
  • Prior head: all Debug win-x64 projects built; 60 focused sidecar, 3,462
    Shared, 2,023 Tray, and 519 Connection tests passed
  • ./scripts/validate-mxc-e2e.ps1 on the runtime-seam head — 2/2 live Gateway MXC tests passed
  • git diff --check — passed
  • built-in Codex review plus independent correctness, architecture/API, and
    security reviews — all findings resolved; exact combined-head passes clean

The combined Windows head is
711fe095028a025fa0649c9b8e20f480644a6967. The rebase preserves the current
fail-closed pre-credential handshake authorization gate through the generic
runtime seam. The adapter review also fixed
cancellation/registration races, response ordering, admission lifetime,
handshake role/version binding, result/envelope bounds, serde-compatible typed
and untyped number handling, and bounded canonicalization across JsonElement,
JsonNode, and JsonDocument.

Real behavior proof

Behavior or issue addressed:
Windows needs one migration-safe execution boundary that preserves current
Gateway-to-native behavior while proving that the OpenClaw sidecar contract can
be consumed independently without bypassing Windows policy or handlers.

Real environment tested:
Windows 11, .NET SDK 10.0.302, combined head
711fe095028a025fa0649c9b8e20f480644a6967, OpenClaw sidecar fixtures from
71c1c8cb23c5647dc07fd4ee1f8663068c92a482, and the existing live Gateway/MXC
proof collected on runtime-seam head
c0cfa8ba66802e601de23875b07caf941209e554. The sidecar proof is an in-process
source harness; it does not launch an external Rust binary.

Exact steps or command run after this patch:

  1. Build every Windows project at the combined head.
  2. Reproduce all three Rust sidecar corpora byte-for-byte in C#.
  3. Complete authentication, immutable configuration, admission, invocation,
    cancellation, result, and status flows.
  4. Route admitted ordinary commands only through NodeCapabilityDispatcher.
  5. Exercise authentication, ordering, identity, admission, serialization,
    cancellation, and resource-bound failures.
  6. Run the focused and complete Shared/Tray suites.
  7. On the unchanged production C# selection, run the live Gateway MXC harness
    for successful contained system.run and denied tray-data writes.

Evidence after fix:
The rebased head passes 63 focused sidecar/dispatcher tests, 3,701 Shared tests,
and 22 focused Connection tests. Prior-head larger-suite evidence and the
retained live MXC transcript remain recorded; MXC reports 2/2 passed: contained
system.run returned OPENCLAW_GATEWAY_SYSTEM_RUN_MXC_OK, and a write outside
the sandbox failed with Access is denied and fileExists=False.

Observed result after fix:
The incumbent C# runtime still executes real Gateway commands through the shared
dispatcher. Independently, the non-selectable adapter accepts the exact
OpenClaw sidecar contracts, routes ordinary admitted commands into that same
dispatcher, and fails closed on authentication, ordering, identity, admission,
cancellation, serialization, and output/work-bound violations.

What was not tested:
No Rust process was launched; no concrete IPC or protected bootstrap was used;
no live Gateway session selected the Rust runtime; and no system.* sidecar
command, crash recovery, production audit, resource measurement, packaged
artifact, rollout, or rollback was exercised. Production selection remains the
existing C# WindowsNodeClient.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 30, 2026
@clawsweeper

clawsweeper Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 12, 2026, 3:39 PM ET / 19:39 UTC.

ClawSweeper review

What this changes

The branch adds a replaceable Windows node-runtime interface, moves native command execution into a shared dispatcher, and implements a non-selectable C# conformance adapter for a proposed Rust sidecar protocol.

Merge readiness

Blocked until stronger real behavior proof is added - 7 items remain

Keep this PR open for maintainer direction, but it is not ready to merge: it couples three ownership transfers and lacks current-head MCP and Gateway proof for the production dispatcher path.

Priority: P2
Reviewed head: 711fe095028a025fa0649c9b8e20f480644a6967
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The branch contains substantial tested design work, but its coupled scope and incomplete current-head behavior proof keep it below merge-ready quality.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: Current-head evidence is fixture and in-process coverage; the cited live MXC result is from an earlier head and does not demonstrate the current dispatcher through MCP or the real Gateway path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Current-head evidence is fixture and in-process coverage; the cited live MXC result is from an earlier head and does not demonstrate the current dispatcher through MCP or the real Gateway path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Default production path changes: The connector still defaults to the C# runtime, while that runtime now constructs and uses the new shared dispatcher for native capability execution.
Three coupled ownership transfers: The branch marks the runtime boundary, capability dispatcher, and sidecar adaptation authoritative together; the complete diff spans 25 files and +5,688/-626 lines.
Repository policy requires a narrower landing unit: Repository guidance requires one seam per PR and says to stop and re-plan when hundreds of lines move without behavior coverage.
Findings 1 actionable finding [P2] Split the runtime, dispatcher, and sidecar transfers
Security None None.

How this fits together

The Windows node receives Gateway invocations, routes them to native Windows capability handlers, and returns results to Gateway and local MCP clients. This branch changes the runtime-to-dispatch boundary while retaining the C# node client as the default runtime.

flowchart LR
  A[Gateway events] --> B[Node runtime]
  B --> C[Capability dispatcher]
  C --> D[Windows capability handlers]
  D --> E[Gateway and MCP results]
  F[Sidecar conformance adapter] --> C
Loading

Decision needed

Question Recommendation
Should this repository adopt a non-selectable Windows conformance implementation for the proposed Rust sidecar contract before that upstream contract and its production adoption path are accepted? Sponsor staged extraction: Require separate dispatcher, runtime-boundary, and sidecar-conformance PRs, with the sidecar slice deferred until the upstream contract is accepted.

Why: The branch deliberately stops short of a selectable runtime, while adding a large local protocol implementation that depends on unresolved upstream ownership and rollout decisions.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Current-head evidence is fixture and in-process coverage; the cited live MXC result is from an earlier head and does not demonstrate the current dispatcher through MCP or the real Gateway path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Split the runtime, dispatcher, and sidecar transfers (P2) - The ledger makes three new owners authoritative together, and the diff spans 25 files with 5,688 added lines. Repository policy requires one seam per PR because this coupling prevents reviewers from isolating a production dispatch regression from the new runtime API or the speculative sidecar contract. Land the slices independently with their own preserved invariant and proof.
  • Resolve merge risk (P1) - The default C# node path now delegates command execution through a new dispatcher, but no current-head MCP or real Gateway invocation evidence is posted.
  • Resolve merge risk (P1) - Combining the runtime seam, dispatcher extraction, and sidecar protocol proof makes rollback and failure isolation difficult.
  • Resolve merge risk (P1) - The sidecar conformance contract depends on an unaccepted upstream direction and can drift before a selectable runtime exists.
  • Complete next step (P2) - A maintainer must first choose whether and when this repository should adopt the unresolved upstream sidecar contract; automated repair cannot safely split or redefine that direction.

Findings

  • [P2] Split the runtime, dispatcher, and sidecar transfers — docs/ARCHITECTURE.md:69-71
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 25 files, +5,688/-626 lines The branch combines multiple architectural transfers in one review unit.
Production versus test delta production +3,121/-610, tests +2,467/-9 The net production growth includes runtime lifecycle, dispatch, and authenticated sidecar protocol code.

Merge-risk options

Maintainer options:

  1. Split and prove the production seam (recommended)
    Separate the three ownership transfers and add current-head MCP discovery/invocation plus a real Gateway invocation for the dispatcher slice before merge.
  2. Pause the sidecar contract slice
    Defer the adapter until the upstream protocol, artifact, credential-handoff, and rollout contract have maintainer approval.

Technical review

Best possible solution:

Choose the sidecar contract upstream, then land the dispatcher extraction, runtime interface, and sidecar adapter as separate reviewable PRs with current-head MCP and Gateway proof for each production-affecting slice.

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

Not applicable as a standalone bug report. The branch has fixture and in-process coverage, but it does not provide current-head MCP or real Gateway behavior proof for the changed production route.

Is this the best way to solve the issue?

No. A staged extraction is safer than merging runtime selection, dispatcher ownership, and a future sidecar protocol proof as one branch.

Full review comments:

  • [P2] Split the runtime, dispatcher, and sidecar transfers — docs/ARCHITECTURE.md:69-71
    The ledger makes three new owners authoritative together, and the diff spans 25 files with 5,688 added lines. Repository policy requires one seam per PR because this coupling prevents reviewers from isolating a production dispatch regression from the new runtime API or the speculative sidecar contract. Land the slices independently with their own preserved invariant and proof.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P2: This is a substantial but non-emergency refactor of Windows node runtime and native command execution.
  • merge-risk: 🚨 compatibility: Existing Gateway node behavior now crosses a new runtime and dispatch boundary while preserving stored pairing and capability semantics.
  • merge-risk: 🚨 availability: A failure in the new dispatcher can prevent Windows node commands from completing after merge.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Current-head evidence is fixture and in-process coverage; the cited live MXC result is from an earlier head and does not demonstrate the current dispatcher through MCP or the real Gateway path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Default production path changes: The connector still defaults to the C# runtime, while that runtime now constructs and uses the new shared dispatcher for native capability execution. (src/OpenClaw.Connection/NodeConnector.cs:37, 711fe095028a)
  • Three coupled ownership transfers: The branch marks the runtime boundary, capability dispatcher, and sidecar adaptation authoritative together; the complete diff spans 25 files and +5,688/-626 lines. (docs/ARCHITECTURE.md:69, 711fe095028a)
  • Repository policy requires a narrower landing unit: Repository guidance requires one seam per PR and says to stop and re-plan when hundreds of lines move without behavior coverage. (AGENTS.md:115, 4206611f2b2e)
  • Current-head proof is incomplete: The PR body describes sidecar coverage as an in-process harness and identifies the live MXC run as an earlier runtime-seam head; repository policy requires current-head MCP discovery/invocation and a real Gateway path when available. (AGENTS.md:70, 4206611f2b2e)
  • The proposed sidecar contract is not yet a selectable runtime: The branch’s own migration document states the adapter does not implement the runtime interface or runtime selection and still needs artifact, IPC, credential-handoff, supervision, audit, and rollback work. (docs/RUST_NODE_RUNTIME_MIGRATION.md:39, 711fe095028a)
  • Not implemented on current main or release: The PR head is not an ancestor of current main. Current main is tagged v2026.7.1. (4206611f2b2e)

Likely related people:

  • shanselman: Current-main blame for the central NodeConnector and WindowsNodeClient surfaces resolves to Scott Hanselman; available history is shallow, so older ownership cannot be established confidently. (role: recent node-runtime area contributor; confidence: medium; commits: 4206611f2b2e; files: src/OpenClaw.Connection/NodeConnector.cs, src/OpenClaw.Shared/WindowsNodeClient.cs)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Split the three ownership transfers into independently reviewable PRs.
  • Post redacted current-head winnode discovery/invocation output and a real Gateway invocation, or explicitly document the Gateway blocker.
  • Obtain maintainer direction on adopting the upstream sidecar contract before resubmitting that slice.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (37 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-02T13:17:12.441Z sha 3ca913a :: needs real behavior proof before merge. :: [P1] Preserve node-client event access through the runtime seam
  • reviewed 2026-08-02T14:31:00.983Z sha 3ca913a :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-02T16:41:48.116Z sha 3ca913a :: needs real behavior proof before merge. :: [P1] Preserve the public node-client type contract
  • reviewed 2026-08-02T17:57:57.698Z sha 3ca913a :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-02T21:29:51.898Z sha 3ca913a :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-02T23:03:30.346Z sha 3ca913a :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-03T08:00:31.629Z sha 3ca913a :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-12T19:29:54.378Z sha 711fe09 :: needs real behavior proof before merge. :: [P2] Split the three ownership transfers before merge

@giodl73-repo giodl73-repo changed the title refactor: add a replaceable node runtime boundary refactor: add a replaceable node runtime and shared dispatcher Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 30, 2026
@giodl73-repo
giodl73-repo force-pushed the agent/rust-node-sidecar-seam branch from 69fe021 to 194928f Compare July 30, 2026 09:40
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. label Jul 30, 2026
@giodl73-repo

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 30, 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: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added 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. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 30, 2026
@giodl73-repo
giodl73-repo force-pushed the agent/rust-node-sidecar-seam branch from 3ca913a to 711fe09 Compare August 12, 2026 19:20
@giodl73-repo

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 12, 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 exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. label Aug 12, 2026
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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant