Skip to content

fix: make Windows Launcher isolation status informational - #78

Draft
ChazGo wants to merge 9 commits into
openclaw:mainfrom
ChazGo:chazgo-informational-isolation-plugin
Draft

ChazGo wants to merge 9 commits into
openclaw:mainfrom
ChazGo:chazgo-informational-isolation-plugin

Conversation

@ChazGo

@ChazGo ChazGo commented Sep 18, 2026

Copy link
Copy Markdown

What Problem This Solves

Windows Launcher presents isolation as a configurable mode even though the packaged Gateway requires an isolated session.

User Impact

Operators get one truthful Gateway Isolation: Active report and a copy-only command reference, not isolation controls. Missing, malformed, and unsupported reports show neutral Invalid status. The plugin remains opt-in.

Why This Change Was Made

  • Removed isolation enable/disable guidance. Preserved the authenticated, read-only route, captured launcher report, opaque iframe sandbox, and live theme forwarding.
  • Grouped seven useful references under Command reference, with accessible Copy controls, source-aligned descriptions, and explicit user-session versus agent-session guidance. Added a command-tree maintenance reminder.
  • Selected unreleased OpenClaw 44e9347d3342cd8b5e27fec78df40b8dff0dca34 for development theme support without changing the official stable approval. Added early signing-policy checks, architecture-matched Windows/Node validation, a native ARM64 workflow runner, and isolated inspection caches.

Dependency: Installed native-addon execution uses the separate, still-draft/unmerged #75 at a6a849b970be1d3f2a16f1514dfc08dc845b4db5. Its implementation is not included in this PR.

Evidence

  • Authored head 1b2ea924f7789b9578b59d0dea129b9ec2dc7f1a: node --test .\plugins\gateway-isolation\index.test.js passed all 52 tests; Test-GatewayIsolationPlugin.Tests.ps1 passed. Exact final heading/text browser fixtures passed four themes, missing/malformed/unsupported reports, authentication, no-reload behavior, and mouse/keyboard/manual-copy checks.
  • Installed x64 integration 9b67a25770f1945698e83e0decfcfdd6d8356772: combined authored 99597e7, canonical main a99bb66, the pinned PR75 fix, and the same development runtime. Passed 714 managed tests, 17 NativeAOT scenarios, payload qualification, MSIX composition, and test signing. The precursor integration 61df2bd also passed the static-analysis quality gate and 13 packaging/policy PowerShell suites.
  • Real isolated-session proof: launcher-provided status without an injected report; Koffi and fs-safe loaded from agent-native staging; four native dependencies loaded in both the main process and a worker; clawctl pwsh opened PowerShell 7.6.6. Authenticated Control UI passed four live themes without iframe reload, 14 exact mouse/keyboard copies, and an explicitly simulated clipboard-denial/manual-copy case. Supported cleanup was verified.
  • The start command returned its 30-second Starting timeout; authenticated readiness followed at 64.446 seconds. Test activation was explicit. The final heading-only refinement was validated separately as a component fixture, not another installed run.

The reviewed screenshots are native 2880 × 2240 captures. Open an image for full resolution.

Final UI: four-theme component proof at 1b2ea92

Direct Node fixture on runtime 44e9347 with an explicit isolation report. This verifies the final heading and text, not installed isolation.

Default dark

Final heading component, default dark

Default light

Final heading component, default light

Imported custom dark

Final heading component, imported custom dark

Imported custom light

Final heading component, imported custom light

Installed x64: four-theme isolated Gateway proof at 9b67a257

Real test-signed package 0.1.2452.40001, combined with the pinned PR75 fix and runtime 44e9347. The launcher supplied the isolation report; no report was injected. These captures precede the final heading-only refinement. Custom themes used the real Control UI importer with synthetic theme input, not a mocked Gateway.

Default dark

Installed isolated Gateway, default dark

Default light

Installed isolated Gateway, default light

Imported custom dark

Installed isolated Gateway, imported custom dark

Imported custom light

Installed isolated Gateway, imported custom light

Chaz Gordish and others added 9 commits September 17, 2026 18:38
Remove unsupported isolation commands and copy controls. Show Running and Active only with the captured enabled report, and report all other isolation signals as invalid with HTTP 503.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep isolation reports informational, replace unsupported controls with verified general command references, and cover sandbox clipboard fallback and accessible feedback.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the agent shell and Gateway chat UI distinct, preserve copy-only behavior, and fit all seven references without clipping.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the stable signing policy separate and reject incompatible official workflow inputs before building.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use a native ARM64 runner, reject incompatible inspection hosts before staging, and isolate the plugin snapshot cache for every CLI probe.

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

clawsweeper Bot commented Sep 18, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. 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. labels Sep 18, 2026
@clawsweeper

clawsweeper Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 18, 2026, 2:59 PM ET / 18:59 UTC (Revision 2).

ClawSweeper review

What this changes

The PR makes Windows Launcher isolation status informational, adds seven copy-only command references, and changes development-runtime selection, signing validation, and architecture-specific payload builds.

Merge readiness

Blocked before merge - 6 items remain

The correction remains necessary on main. The newly attached screenshots substantially improve the evidence, but an introduced test exit-code defect blocks CI, and the broader runtime/build changes still need qualification and a landing decision.

Priority: P2
Reviewed head: 1b2ea924f7789b9578b59d0dea129b9ec2dc7f1a
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful implementation and substantially improved visual proof remain limited by one concrete CI defect and incomplete qualification of the broader build changes.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: The eight inspected screenshots now prove the visible status/reference page in component fixtures and the declared installed x64 integration. They do not demonstrate exact clipboard results or denial recovery; the expanded runtime/build scope also lacks ARM64 and fresh-install/upgrade qualification. The combined dependency and final heading-only fixture are clearly disclosed. 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: The eight inspected screenshots now prove the visible status/reference page in component fixtures and the declared installed x64 integration. They do not demonstrate exact clipboard results or denial recovery; the expanded runtime/build scope also lacks ARM64 and fresh-install/upgrade qualification. The combined dependency and final heading-only fixture are clearly disclosed. 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 10 items Current main still needs the UI correction: Main still renders Enabled/Disabled and planned isolation enable/disable commands, although the launcher now supplies a constant enabled report. The latest release is v2026.9.4-msix.2 at the fetched main SHA.
Introduced negative test leaves a failing native exit code: The added runtime-failure fixture makes Node exit unsuccessfully. Its expected exception is caught, but no subsequent native success or explicit exit-code restoration occurs before the suite returns.
PowerShell step exit contract: The target workflow invokes the affected suite with shell: pwsh. GitHub's runner appends an exit using LASTEXITCODE, making the caught native failure observable as a failed workflow step.
Findings 1 actionable finding [P2] Clear the expected native failure before returning from the test
Security None None.

How this fits together

The Windows package bundles OpenClaw and an optional Launcher status plugin. The plugin turns a launch-time isolation report into an authenticated Control UI page, while the packaging workflow selects and validates the bundled runtime.

flowchart LR
  A[Workflow runtime selection] --> B[Windows payload validation]
  B --> C[Packaged Gateway]
  C --> D[Launcher isolation report]
  D --> E[Optional status plugin]
  E --> F[Active or Invalid status]
  E --> G[Copy-only command reference]
Loading

Decision needed

Question Recommendation
Should this UI correction also change the default packaged runtime and require architecture-matched payload builders? Separate runtime and builder changes: Land the informational UI after its focused fixes and proof, retaining stable defaults while qualifying the broader build changes separately.

Why: Those changes affect packaging operators and qualification scope beyond the status correction; installed evidence currently covers a combined x64 build with an unmerged dependency.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The eight inspected screenshots now prove the visible status/reference page in component fixtures and the declared installed x64 integration. They do not demonstrate exact clipboard results or denial recovery; the expanded runtime/build scope also lacks ARM64 and fresh-install/upgrade qualification. The combined dependency and final heading-only fixture are clearly disclosed. 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.
  • Clear the expected native failure before returning from the test (P2) - The new failure fixture makes the last Node invocation exit 1. Catching the expected PowerShell exception does not reset $LASTEXITCODE, and the remaining assertions only use cmdlets. GitHub's pwsh wrapper exits with that value, so this suite fails the host job even when its assertions pass, blocking downstream packaging. Restore a successful exit status after validating the expected failure without swallowing unexpected errors. The newly available CI result fails this exact step.
  • Resolve merge risk (P1) - Existing payload cross-build invocations will now stop at the architecture guard, and official signing with the new default ref will fail until the approved commit is selected explicitly.
  • Resolve merge risk (P1) - The installed x64 evidence includes an unmerged native-addon dependency; it does not establish standalone landing readiness, native ARM64 qualification, or in-place upgrade behavior for the development runtime.
  • Complete next step (P2) - Fix the test exit-code leak, resolve the runtime/build scope with maintainers, and add the remaining behavior evidence with private details redacted. Update the PR body for automatic re-review; if it does not trigger, ask a maintainer to comment @clawsweeper re-review.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P2] Clear the expected native failure before returning from the test — scripts/Test-GatewayIsolationPlugin.Tests.ps1:230-235
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth Production +109 net lines; tests +421 net lines Production includes workflow/configuration changes; the stated justification extends beyond UI correction into runtime selection and native payload qualification.
Visual evidence 8 screenshots inspected: 4 component, 4 installed x64 These establish visible layout and theme results while keeping component and combined-package provenance distinct.

Merge-risk options

Maintainer options:

  1. Keep the UI landing bounded (recommended)
    Separate the runtime default and builder requirements, then fix the test's leaked native exit code before landing the UI correction.
  2. Qualify the combined change
    Retain the expanded scope only with explicit operator-contract approval, dependency coordination, and fresh-install, upgrade, and ARM64 evidence.

Technical review

Best possible solution:

Keep the informational plugin correction focused, with the stable runtime default preserved unless maintainers explicitly adopt and qualify the development-runtime and native-build contract.

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

Yes, source provides a high-confidence path: main advertises isolation toggling despite mandatory sessions, and the introduced negative test leaves Node's failure code for the workflow wrapper. No target code was executed during this review.

Is this the best way to solve the issue?

The informational UI is a sound correction; combining it with a new development default and builder restrictions is broader than necessary and needs separate approval or qualification.

Full review comments:

  • [P2] Clear the expected native failure before returning from the test — scripts/Test-GatewayIsolationPlugin.Tests.ps1:230-235
    The new failure fixture makes the last Node invocation exit 1. Catching the expected PowerShell exception does not reset $LASTEXITCODE, and the remaining assertions only use cmdlets. GitHub's pwsh wrapper exits with that value, so this suite fails the host job even when its assertions pass, blocking downstream packaging. Restore a successful exit status after validating the expected failure without swallowing unexpected errors. The newly available CI result fails this exact step.
    Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against a99bb66d2162.

Labels

Label changes:

  • add merge-risk: 🚨 automation: The introduced expected-failure test leaves LASTEXITCODE nonzero, causing the PowerShell workflow step to fail after successful assertions.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The eight inspected screenshots now prove the visible status/reference page in component fixtures and the declared installed x64 integration. They do not demonstrate exact clipboard results or denial recovery; the expanded runtime/build scope also lacks ARM64 and fresh-install/upgrade qualification. The combined dependency and final heading-only fixture are clearly disclosed.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: This corrects an optional status page and packaging behavior without evidence of an urgent new user-facing outage.
  • merge-risk: 🚨 compatibility: The PR changes default runtime selection and rejects previously accepted payload-builder architectures.
  • merge-risk: 🚨 automation: The introduced expected-failure test leaves LASTEXITCODE nonzero, causing the PowerShell workflow step to fail after successful assertions.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp 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 eight inspected screenshots now prove the visible status/reference page in component fixtures and the declared installed x64 integration. They do not demonstrate exact clipboard results or denial recovery; the expanded runtime/build scope also lacks ARM64 and fresh-install/upgrade qualification. The combined dependency and final heading-only fixture are clearly disclosed. 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.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The eight inspected screenshots now prove the visible status/reference page in component fixtures and the declared installed x64 integration. They do not demonstrate exact clipboard results or denial recovery; the expanded runtime/build scope also lacks ARM64 and fresh-install/upgrade qualification. The combined dependency and final heading-only fixture are clearly disclosed.

Evidence

What I checked:

  • Current main still needs the UI correction: Main still renders Enabled/Disabled and planned isolation enable/disable commands, although the launcher now supplies a constant enabled report. The latest release is v2026.9.4-msix.2 at the fetched main SHA. (plugins/gateway-isolation/index.js:53, a99bb66d2162)
  • Introduced negative test leaves a failing native exit code: The added runtime-failure fixture makes Node exit unsuccessfully. Its expected exception is caught, but no subsequent native success or explicit exit-code restoration occurs before the suite returns. (scripts/Test-GatewayIsolationPlugin.Tests.ps1:234, 1b2ea924f778)
  • PowerShell step exit contract: The target workflow invokes the affected suite with shell: pwsh. GitHub's runner appends an exit using LASTEXITCODE, making the caught native failure observable as a failed workflow step. (src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs)
  • New CI evidence localizes the failure: The host job passed preceding checks and failed its final Gateway isolation plugin step at 18:46 UTC, after the previous review completed. This corroborates the source-established exit-code defect; raw job logs were inaccessible through the allowed network path. (.github/workflows/gateway-msix.yml:211, 1b2ea924f778)
  • Eight screenshots inspected: Inspected all four prepared component images and all four installed-run attachments from the supplied body snapshot, sourceRevision bafc7d294aebdb328e860f9a25900fb4b0f0acaa42b59b9b8b527fa70bfda06e. They show Active status and seven references across four themes. Component images identify the reviewed head; installed images identify integration 9b67a25770f1945698e83e0decfcfdd6d8356772. They do not display clipboard contents, denial feedback, or runtime diagnostics.
  • Installed proof has an explicit unmerged dependency: The body identifies fix: load packaged native addons from the isolated session #75 at a6a849b as part of the installed integration. GitHub confirms that dependency remains open and draft; it fixes native-addon execution rather than the informational UI. (a6a849b970be)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • RomneyDa: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

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

  • Fix the expected-failure test's exit status and verify the complete PowerShell workflow step succeeds.
  • Attach redacted clipboard-result and denial-recovery output or a recording with diagnostics; screenshots alone do not show these interactions.
  • Resolve the runtime/build scope and dependency landing choice, supplying fresh-install, upgrade, and native ARM64 evidence if that scope remains.

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 (1 earlier review cycle)
  • reviewed 2026-09-18T18:44:35.810Z sha 1b2ea92 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. 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 Sep 18, 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. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant