Skip to content

fix(setup): treat the final wizard SIGTERM restart as success - #1491

Open
SebTardif wants to merge 3 commits into
openclaw:mainfrom
SebTardif:fix/setup-wizard-sigterm-success
Open

SebTardif wants to merge 3 commits into
openclaw:mainfrom
SebTardif:fix/setup-wizard-sigterm-success

Conversation

@SebTardif

@SebTardif SebTardif commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

What Problem

When the final done step restarts the gateway, the hosted wizard can return the exact terminal error Error: TUI exited from signal SIGTERM. The headless setup runner already treats that response as completion after the authoritative final step, but the WinUI wizard showed an error and skipped the remaining setup completion work, including Windows node bootstrap guidance.

Why

The gateway had finished the wizard and then terminated its hosted TUI during restart. The WinUI owner did not use the setup engine's existing terminal-error decision seam or track which step the request had just answered.

User Impact

The WinUI wizard now uses SetupWizardRunner.DecideTerminalWizardError and WizardFinalStepTracker. The exact SIGTERM response completes setup only immediately after answering the authoritative final plain acknowledgement step named done. The same SIGTERM before the final step, after a progress poll or replay, on a skipped transition, with an answerable step, or with inexact text remains an error.

Required proof pools

  • windows-wsl-gateway-e2e: wizard completion after a real gateway restart changed.
  • windows-winui-interactive: the final-step SIGTERM must visibly complete setup, while a pre-final SIGTERM must visibly remain an error.

Validation

Current head: 8c18214d82ebce3fe6073143bceb3202f218226d

  • .\build.ps1: passed. Shared, CLI, WinNode CLI, SetupEngine, and WinUI built successfully; documentation and proof-pool validation passed.
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: passed 4,115, skipped 32, failed 0, total 4,147.
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: passed 3,154, failed 0, total 3,154. OPENCLAW_TRAY_DATA_DIR used an isolated temporary directory.
  • dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore: passed 1,201, skipped 1, failed 0, total 1,202. Setup local state used an isolated temporary directory.
  • Focused terminal completion filter covering SetupWizardTerminalCompletionContractTests, WizardFinalStepTrackerTests, GatewayWizardRestartRecoveryPolicyTests, and SetupWizard_Terminal*: passed 73, failed 0.
  • An unrelated McpHttpServerTests.Dispose_DuringInFlightHandler_DoesNotSurfaceObjectDisposedException disposal-race failure appeared during concurrent suite execution. The exact test then passed 5 of 5 stress reruns, and the final sequential Shared suite passed completely.
  • Rubber-duck review: no blocking implementation issue. The review-confirmed hardening is included in this head: skipped transitions clear the final-step marker, and the WinUI contract pins the negated rejection branch and visible error return.

Real behavior proof

Exact behavior contract

Current-head automated proof verifies:

  • Exact Error: TUI exited from signal SIGTERM after the authoritative final done acknowledgement is accepted.
  • The identical SIGTERM before the final step remains fatal.
  • Progress polling and replay clear the final-step marker.
  • Skipped transitions clear the final-step marker before wizard.next.
  • Answerable/non-note steps, steps with options, non-final position metadata, non-terminal payloads, different signals, different casing, and extra text remain failures.
  • Rejected terminal errors call ShowError(error) and return. Accepted completion reaches CompleteSetupAsync, which continues the setup pipeline and Windows node context work.

Current-head WinUI evidence

Isolated Debug previews were inspected with computer-use on this head:

  • The completion page visibly showed All set!, Local gateway running, Device paired, Capability profile applied, and Node mode enabled, with the guidance: The gateway can use this PC for screen capture, camera, and system commands.
  • The wizard error preview visibly showed OpenClaw onboard hit a problem, preserved the error state, and offered Start wizard again plus More options recovery.

These previews are visual-only evidence of the two destination states. They do not execute the SIGTERM transition.

Real WSL Gateway proof

The initial run using the host's default %LOCALAPPDATA%\Temp was blocked at fresh distro creation by WSL E_ACCESSDENIED while attaching isolated ext4.vhdx / swap.vhdx. A second run used process-only TEMP and TMP set to a task-owned D:\openclaw-pr1491-proof-temp; no global ACL or environment setting was changed.

That retry successfully created disposable WSL distros and exercised real gateway behavior. The setup-connect shard reported 23 passed, 22 failed, and 1 skipped. The successful fixture provided these current-head proofs:

  • The real wizard advanced through the authoritative done step and returned a normal terminal payload. Gateway wizard completed was logged.
  • Reload mode restoration ran. The first guarded gateway restart attempt failed, the bounded ownership recheck/retry succeeded, and the health endpoint returned HTTP 200.
  • WindowsNodeBootstrapContextStep injected the managed Windows node context into the runtime workspace, and the pipeline completed successfully in 221 seconds.
  • Real Gateway to Windows node MXC proofs passed, including RealGateway_SystemRun_ExecutesThroughWindowsNodeMxcSandbox, RealGateway_SystemRun_BlocksWritesToTrayDataDirectoryInMxcSandbox, and the bound executable/tail allowlist proofs.
  • All disposable distros were unregistered, and the task-owned D:\openclaw-pr1491-proof-temp directory was removed.

The exact SIGTERM payload was not emitted by this gateway run, so the changed terminal conversion still lacks live transition proof. The second shared setup fixture failed earlier during gateway restart/finalization and never reached the wizard. Current-head CI has the same unrelated setup-connect lane blocked by restart intent/state-database contention tracked in #1507 (fix(setup): retry guarded restart intent contention); all other required jobs passed.

Landing status

Implementation, review, automated validation, visible destination-state proof, and substantial real WSL Gateway proof are complete. Merge remains blocked because required CI is red on the #1507 restart contention and the declared interactive proof pools did not produce the exact SIGTERM transition.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper clawsweeper Bot added 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. labels Sep 23, 2026
@clawsweeper

clawsweeper Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

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

ClawSweeper review

What this changes

The branch makes the Windows setup wizard accept an exact hosted-wizard SIGTERM after the final acknowledgement and adds source-contract tests for accepted and rejected paths.

Merge readiness

⛔ Blocked before merge - 3 items remain

The WinUI setup wizard still rejects this final-step SIGTERM on current main, so this PR remains useful. The implementation is narrowly gated and has no concrete code finding, but the supplied real gateway run did not emit SIGTERM and the WinUI previews did not exercise the changed transition. The required direct behavior proof remains outstanding.

Priority: P2
Reviewed head: 8c18214d82ebce3fe6073143bceb3202f218226d

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and has useful validation, but direct real-behavior proof for its changed transition remains absent.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The changed production owner is the WinUI wizard response handler. Current-head previews show completion and error destinations, and a real WSL Gateway run shows ordinary wizard completion and later setup work, but neither exercises the exact SIGTERM conversion through WinUI; capture that transition and earlier-step rejection with private endpoints and tokens redacted. No stored-data contract changes. 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 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The changed production owner is the WinUI wizard response handler. Current-head previews show completion and error destinations, and a real WSL Gateway run shows ordinary wizard completion and later setup work, but neither exercises the exact SIGTERM conversion through WinUI; capture that transition and earlier-step rejection with private endpoints and tokens redacted. No stored-data contract changes. 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 8 items Introduced WinUI behavior: The PR records whether the answer was for the authoritative final step, applies the shared terminal-error decision, and continues setup only when that decision marks completion.
Current main still lacks WinUI handling: Current main shows an error for nonempty terminal errors other than the older finalization-prompt exception; it does not call the shared SIGTERM decision or track the final answer.
Existing setup-engine contract: The previously merged headless runner accepts exact terminal SIGTERM only after its final-step tracker records the authoritative final acknowledgement.
Findings None None.
Security None None.

How this fits together

The WinUI setup wizard sends answers to the Gateway's hosted wizard and interprets its responses. A terminal response either shows an error or advances the remaining setup and Windows node guidance.

flowchart LR
  A[User answers wizard step] --> B[WinUI sends answer to Gateway]
  B --> C[Gateway wizard response]
  C --> D[Final step and exact SIGTERM?]
  D -->|Yes| E[Continue setup]
  D -->|No| F[Show wizard error]
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The changed production owner is the WinUI wizard response handler. Current-head previews show completion and error destinations, and a real WSL Gateway run shows ordinary wizard completion and later setup work, but neither exercises the exact SIGTERM conversion through WinUI; capture that transition and earlier-step rejection with private endpoints and tokens redacted. No stored-data contract changes. 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.
  • Resolve merge risk (P1) - The exact final-step SIGTERM transition has not been observed through the current-head WinUI wizard, so its visible completion and earlier-step rejection remain unverified in a real setup.
  • Complete next step (P2) - Add redacted current-head WinUI proof of exact final-step SIGTERM completion and earlier-step rejection. Updating the PR body should trigger re-review; if it does not, ask a maintainer to comment @clawsweeper re-review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Code and test growth production +45 lines, tests +91 lines Production growth is bounded to wiring the existing setup-engine policy into WinUI, while the added tests pin source-level routing.

Merge-risk options

Maintainer options:

  1. Prove the terminal transition (recommended)
    Capture a redacted current-head WinUI run showing the exact final SIGTERM complete setup and an earlier SIGTERM remain an error.

Technical review

Best possible solution:

Keep the shared final-step decision as the single policy and demonstrate both the accepted final SIGTERM and rejected earlier SIGTERM through the isolated WinUI wizard before landing.

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

Yes, at source level: current main shows the exact terminal SIGTERM as an error in WinUI, while the existing headless decision accepts it after the final acknowledgement. The supplied current-head gateway run did not reproduce the SIGTERM transition live.

Is this the best way to solve the issue?

Yes, provisionally: reusing the existing setup-engine decision and tracker is a narrow repair. Direct WinUI transition proof is still needed to confirm the integration.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This is a bounded setup-wizard failure affecting users whose Gateway restarts after the final acknowledgement.
  • merge-risk: 🚨 availability: Treating a terminal Gateway error as completion could leave setup incomplete if the final-step transition behaves differently in the live WinUI flow.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The changed production owner is the WinUI wizard response handler. Current-head previews show completion and error destinations, and a real WSL Gateway run shows ordinary wizard completion and later setup work, but neither exercises the exact SIGTERM conversion through WinUI; capture that transition and earlier-step rejection with private endpoints and tokens redacted. No stored-data contract changes. 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:

  • Scott Hanselman: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Karen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Barbara Kudiess: 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.

  • Capture redacted current-head WinUI output or a recording of final-step SIGTERM completion, Windows node guidance, and an earlier-step SIGTERM error.

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 (6 earlier review cycles)
  • reviewed 2026-09-23T02:23:58.048Z sha a311146 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-23T21:04:35.958Z sha a311146 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-23T23:17:05.767Z sha a311146 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-24T00:51:36.578Z sha a311146 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-28T18:02:21.389Z sha a311146 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-28T18:32:16.662Z sha 8c18214 :: needs real behavior proof before merge. :: none

@karkarl

karkarl commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Global triage: NEEDS_HUMAN_TEST. Take confidence 78%; recommendation confidence 90%; effort small; risk medium-low.

Reviewed exact head a311146f6856. The change is ledger-aligned and tightly gated to authoritative final done plus exact terminal SIGTERM; the prompt-bug matcher is not narrowed. Blocking proof is windows-winui-interactive and windows-wsl-gateway-e2e: show final-step SIGTERM completing setup and non-final SIGTERM still failing. Add Shared closeout results and clean Validation formatting. Non-blocking nit: WinUI discards decision.LogWarning/Result, losing headless-runner diagnostics.

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. label Sep 23, 2026
@SebTardif

Copy link
Copy Markdown
Contributor Author

Head a311146 accepts the exact final-step string Error: TUI exited from signal SIGTERM and still fails a non-final SIGTERM. Local filter SetupWizard_Terminal plus GatewayWizardRestart: 59 passed.

Shared closeout in the body: 4093 passed, 0 failed, 32 skipped. Tray: 3059 passed, 5 failed, the same five source-contract tests as 1488, 1489, and 1490.

The WinUI page was not opened, so this does not include a live click of that final-step SIGTERM.

@shanselman shanselman added the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Sep 28, 2026
Clear the final-step marker when the UI sends a skip transition and pin the rejected terminal-error branch in the WinUI source contract.

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

Copilot-Session: bd83f93d-0bd4-41ff-9c95-b0a3b02b1e9b
@shanselman shanselman removed the status: 🚢 actively landing A maintainer or agent is actively driving this item through implementation, validation, or merge. label Sep 28, 2026

This branch has not been deployed

No deployments
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. 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.

3 participants