Skip to content

chore: experiment with trimmed winnode publishing - #1002

Draft
shanselman wants to merge 1 commit into
mainfrom
experiment-winnode-trimming
Draft

chore: experiment with trimmed winnode publishing#1002
shanselman wants to merge 1 commit into
mainfrom
experiment-winnode-trimming

Conversation

@shanselman

Copy link
Copy Markdown
Collaborator

What Problem This Solves

This draft explores whether the standalone winnode CLI can be safely tree-trimmed before we decide to distribute it independently. Today, winnode is not included in the GitHub release installer or portable ZIP, so this experiment does not reduce current OpenClaw Companion release assets.

Why This Change Was Made

The experiment enables conservative partial trimming for winnode while copying OpenClaw.Shared.dll unchanged. It removes the CLI's reflection-based JSON pretty-print call and adds an independent x64/ARM64 CI job that publishes the trimmed CLI, verifies the Shared assembly remains byte-identical, and smoke-runs the executable.

The WinNode CI job is separate from Tray artifact publishing and is not a dependency of the release job. This PR intentionally does not trim or otherwise alter the Tray application.

User Impact

No current user-visible impact. This is an experiment for a possible future standalone winnode distribution.

Measured standalone x64 output:

Variant Folder ZIP
Self-contained, untrimmed 115.73 MiB 50.87 MiB
Self-contained, partial trim 60.24 MiB 25.56 MiB
Savings 55.49 MiB (47.9%) 25.31 MiB (49.8%)

Latest GitHub release v0.6.12 remains unchanged: x64 installer 114.12 MiB, ARM64 installer 99.90 MiB, x64 ZIP 141.64 MiB, ARM64 ZIP 133.04 MiB.

Evidence

  • x64 trimmed publish: 60.24 MiB, 112 files, winnode.exe --help exited 0.
  • ARM64 trimmed publish: 61.71 MiB, 112 files, winnode.exe --help exited 0 on an ARM64 host.
  • OpenClaw.Shared.dll SHA-256 matched its pre-link build output immediately after both publish sequences.
  • Publish completed with trim diagnostics treated as errors; no trim warnings were suppressed.
  • Final rubber-duck review found no blocking or non-blocking issues after the WinNode CI gate was separated from Tray publishing.

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

Validation

  • ./build.ps1 — passed; Shared, CLI, WinNode CLI, SetupEngine, and WinUI built successfully for win-arm64.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore — 2,863 passed, 31 skipped, 0 failed.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore — 1,717 passed, 0 failed.
  • dotnet test ./tests/OpenClaw.WinNode.Cli.Tests/OpenClaw.WinNode.Cli.Tests.csproj --no-restore — 127 passed, 0 failed.
  • dotnet publish ./src/OpenClaw.WinNode.Cli/OpenClaw.WinNode.Cli.csproj -c Release -r win-x64 --self-contained true — passed; Shared hash matched; --help passed.
  • dotnet publish ./src/OpenClaw.WinNode.Cli/OpenClaw.WinNode.Cli.csproj -c Release -r win-arm64 --self-contained true — passed; Shared hash matched; --help passed.

Real Behavior Proof

  • Environment tested: Windows 11 ARM64, .NET SDK 10.0.302; x64 publish exercised under Windows emulation and ARM64 publish exercised natively.
  • PR head or commit tested: af85963f
  • Exact steps or command run: publish each RID self-contained, compare SHA-256 of built and published OpenClaw.Shared.dll, then run the published winnode.exe --help.
  • Evidence after fix: x64 60.24 MiB; ARM64 61.71 MiB; hash comparisons true; both executables exited 0.
  • Observed result: conservative trimming removes about 48% of the standalone folder while preserving Shared byte-for-byte.
  • Screenshot or artifact links verified? N/A
  • Not verified or blocked: no live isolated-Tray MCP roundtrip was available locally. Before promotion from experiment, test the published binary on a clean second machine with no .NET SDK/runtime, run winnode --list-tools, and invoke a safe command such as system.which against an isolated Tray profile.

Security Impact

  • New permissions or capabilities? No
  • Secrets or tokens handling changed? No
  • New or changed network calls? No
  • Command or tool execution surface changed? No
  • Data access scope changed? No
  • If any answer is Yes, explain the risk and mitigation: N/A

Compatibility and Migration

  • Backward compatible? Expected yes; still experimental pending clean-machine proof.
  • Config or environment changes? No
  • Migration needed? No
  • If yes, list the exact upgrade steps: N/A

Experiment Risks

  • The preservation hook attaches to the SDK-private _ComputeManagedAssemblyToLink target; a future SDK update could change that ordering. CI therefore verifies OpenClaw.Shared.dll byte-for-byte on both RIDs.
  • Existing unit tests execute the normal test assembly, not every path through the published executable.
  • Future winnode code that calls additional Shared functionality may require new published-binary tests.
  • The CLI still carries speech/ONNX/native assets inherited from OpenClaw.Shared; trimming does not solve that dependency-boundary problem.
  • The structured autoreview helper could not run because its secret scanner treated the existing OPENCLAW_MCP_TOKEN identifier in Program.cs as secret-like content. No secrets were added or changed.

Second-Machine Test Plan

  1. Download or build the x64/ARM64 publish-winnode output on a clean Windows machine without the .NET runtime installed.
  2. Run winnode.exe --help.
  3. Launch the Tray with isolated data and enable Local MCP Server.
  4. Set OPENCLAW_TRAY_DATA_DIR to that isolated profile.
  5. Run winnode.exe --list-tools.
  6. Run winnode.exe --command system.which --params '{"bins":["git","node","powershell"]}'.
  7. Confirm output and exit codes match an untrimmed publish.

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.

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

Copilot-Session: 8d7a4316-1b92-4ca0-970a-e9afd2d3972e
@clawsweeper clawsweeper Bot added 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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jul 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed August 4, 2026, 6:05 PM ET / 22:05 UTC.

ClawSweeper review

What this changes

The PR enables partial trimming for standalone Windows-node CLI publishes and adds x64/ARM64 CI publishing, hash, and startup checks.

Merge readiness

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

Keep open. The unchanged PR head still validates only --help, which bypasses the Local MCP transport that trimmed standalone winnode users need; it also adds a new CI job with the superseded setup-dotnet action major.

Priority: P3
Reviewed head: af85963fe755bc085d1e4acceba8ed98a8234de7

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The trim configuration is focused and has two-RID publish evidence, but the published Local MCP path remains unproven.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The PR reports real Windows publishes and help output, but does not show Local MCP discovery and invocation from the published executable. Redact tokens and endpoints in added evidence. 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) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The PR reports real Windows publishes and help output, but does not show Local MCP discovery and invocation from the published executable. Redact tokens and endpoints in added evidence. 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 7 items Help does not exercise MCP: The proposed CI smoke calls winnode.exe --help, but the CLI returns from the early usage branch before option parsing, token and endpoint resolution, JSON-RPC construction, or the HTTP POST.
Actual user path: The CLI builds tools/list or tools/call JSON and posts it to Local MCP only after the help branch, so the present executable check cannot detect trimming failures in that path.
Repository proof contract: Repository documentation requires live Local MCP tool discovery plus invocation for MCP and Windows-node work.
Findings 2 actionable findings [P2] Exercise the published Local MCP transport
[P3] Use the repository's current setup-dotnet major
Security None None.

How this fits together

The standalone Windows-node CLI connects to the locally running Tray MCP server to discover and invoke node tools. This PR trims the CLI deployment bundle while attempting to retain the shared tray/node assembly unchanged.

flowchart LR
  Source[WinNode CLI source] --> Publish[Trimmed publish]
  Publish --> Guard[Preserve Shared assembly]
  Guard --> Artifact[Standalone CLI artifact]
  Artifact --> Cli[Published winnode executable]
  Cli --> Mcp[Local MCP server]
  Mcp --> Tools[Node tools]
Loading

Before merge

  • Exercise the published Local MCP transport (P2) - --help returns at the early usage branch, before parsing options or reaching HTTP JSON-RPC. The trimming gate can therefore pass after a linker regression in --list-tools or --command; add isolated Local MCP discovery and harmless invocation proof for the published executable.
  • Use the repository's current setup-dotnet major (P3) - Current main uses actions/setup-dotnet@v6, while this new parallel job remains on v5. Align it to avoid a divergent SDK-install environment.
  • Resolve merge risk (P1) - The new trim gate can pass even if a linker regression breaks --list-tools or --command in the published executable.
  • Resolve merge risk (P1) - The preservation hook relies on an SDK-private target, so SDK changes require artifact-path proof beyond the shared-DLL hash.
  • Resolve merge risk (P1) - The added job diverges from main by using setup-dotnet v5.
  • Complete next step (P2) - The remaining merge blockers are current-head published-artifact proof and a small workflow alignment change; the missing proof is human-only evidence.

Findings

  • [P2] Exercise the published Local MCP transport — .github/workflows/ci.yml:574
  • [P3] Use the repository's current setup-dotnet major — .github/workflows/ci.yml:542
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Publish validation coverage 2 RIDs, 1 executable behavior Both RIDs publish and run help, but that behavior exits before Local MCP transport.
Production versus tests production +75/-1, tests +50 The added test verifies project XML only; it does not execute a trimmed artifact.

Merge-risk options

Maintainer options:

  1. Prove the published MCP path (recommended)
    Add isolated Tray Local MCP proof that runs the published executable through tool discovery and a harmless command on both supported RIDs before merge.
  2. Keep the experiment in draft
    Leave the experiment unmerged until a clean-machine or isolated-profile run demonstrates the actual published CLI path.

Technical review

Best possible solution:

Retain the focused partial-trim experiment, but gate it with a published-binary Local MCP discovery and harmless invocation against an isolated Tray profile, then align its SDK setup action with current CI.

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

Yes. Source proves that --help returns before Local MCP setup and HTTP JSON-RPC, so the existing publish job cannot exercise the path it is intended to protect.

Is this the best way to solve the issue?

No. The hash and help checks are useful, but repository policy requires Local MCP discovery and invocation proof for this Windows-node CLI surface.

Full review comments:

  • [P2] Exercise the published Local MCP transport — .github/workflows/ci.yml:574
    --help returns at the early usage branch, before parsing options or reaching HTTP JSON-RPC. The trimming gate can therefore pass after a linker regression in --list-tools or --command; add isolated Local MCP discovery and harmless invocation proof for the published executable.
    Confidence: 0.99
  • [P3] Use the repository's current setup-dotnet major — .github/workflows/ci.yml:542
    Current main uses actions/setup-dotnet@v6, while this new parallel job remains on v5. Align it to avoid a divergent SDK-install environment.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P3: This is an experimental publishing and CI change with no current release-asset impact.
  • merge-risk: 🚨 compatibility: Trimming can remove code required by the established Local MCP CLI path while help continues to pass.
  • merge-risk: 🚨 automation: The new CI gate is intended to catch trimmed-artifact regressions but currently checks only the help branch.
  • 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: The PR reports real Windows publishes and help output, but does not show Local MCP discovery and invocation from the published executable. Redact tokens and endpoints in added evidence. 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:

Likely related people:

  • shanselman: Current-main blame assigns the CLI help branch and Local MCP request path to Scott Hanselman, and recent CLI history includes his tool-discovery work. (role: recent Windows-node CLI contributor; confidence: high; commits: 277936553135, 00dda4521698; files: src/OpenClaw.WinNode.Cli/Program.cs)
  • Chris Anderson: Feature history identifies Chris Anderson as the author of the initial local-MCP Windows-node CLI. (role: original Windows-node CLI contributor; confidence: medium; commits: 3b8793db3702; files: src/OpenClaw.WinNode.Cli/Program.cs, src/OpenClaw.WinNode.Cli/OpenClaw.WinNode.Cli.csproj)

Rank-up moves

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

  • Run the published executable against an isolated Tray Local MCP server with --list-tools and a safe --command, then add redacted output to the PR body.
  • Update the new workflow job to actions/setup-dotnet@v6.

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 (19 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-02T16:41:50.511Z sha af85963 :: found issues before merge. :: [P2] Exercise Local MCP from the published executable | [P3] Use the repository's current setup-dotnet major
  • reviewed 2026-08-02T17:58:24.297Z sha af85963 :: found issues before merge. :: [P2] Exercise Local MCP from the published executable | [P3] Use the repository’s current setup-dotnet major
  • reviewed 2026-08-02T21:29:39.173Z sha af85963 :: found issues before merge. :: [P2] Exercise Local MCP from the published executable | [P3] Use the repository's current setup-dotnet major
  • reviewed 2026-08-02T23:03:33.269Z sha af85963 :: needs changes before merge. :: [P2] Exercise Local MCP from the published executable | [P3] Match the repository's setup-dotnet major
  • reviewed 2026-08-03T08:00:58.921Z sha af85963 :: found issues before merge. :: [P2] Exercise Local MCP from the published executable | [P3] Use the repository's current setup-dotnet major
  • reviewed 2026-08-03T12:51:54.046Z sha af85963 :: found issues before merge. :: [P2] Exercise the published Local MCP transport path | [P3] Use the workflow's current setup-dotnet major
  • reviewed 2026-08-03T14:56:59.193Z sha af85963 :: found issues before merge. :: [P2] Exercise the published Local MCP transport path | [P3] Use the repository's current setup-dotnet major
  • reviewed 2026-08-03T20:16:18.559Z sha af85963 :: found issues before merge. :: [P2] Exercise the published Local MCP transport | [P3] Use the repository's current setup-dotnet major

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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