Conversation
Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
|
Codex review: needs real behavior proof before merge. Reviewed September 28, 2026, 2:49 PM ET / 18:49 UTC (Revision 7). ClawSweeper reviewWhat this changesThe 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 Review scores
Verification
How this fits togetherThe 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]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest 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. LabelsLabel changes: No label changes. Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (6 earlier review cycles)
|
|
Global triage: NEEDS_HUMAN_TEST. Take confidence 78%; recommendation confidence 90%; effort small; risk medium-low. Reviewed exact head |
|
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. |
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
What Problem
When the final
donestep restarts the gateway, the hosted wizard can return the exact terminal errorError: 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.DecideTerminalWizardErrorandWizardFinalStepTracker. The exact SIGTERM response completes setup only immediately after answering the authoritative final plain acknowledgement step nameddone. 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_DIRused 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.SetupWizardTerminalCompletionContractTests,WizardFinalStepTrackerTests,GatewayWizardRestartRecoveryPolicyTests, andSetupWizard_Terminal*: passed 73, failed 0.McpHttpServerTests.Dispose_DuringInFlightHandler_DoesNotSurfaceObjectDisposedExceptiondisposal-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.Real behavior proof
Exact behavior contract
Current-head automated proof verifies:
Error: TUI exited from signal SIGTERMafter the authoritative finaldoneacknowledgement is accepted.wizard.next.ShowError(error)and return. Accepted completion reachesCompleteSetupAsync, 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 gateway can use this PC for screen capture, camera, and system commands.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%\Tempwas blocked at fresh distro creation by WSLE_ACCESSDENIEDwhile attaching isolatedext4.vhdx/swap.vhdx. A second run used process-onlyTEMPandTMPset to a task-ownedD:\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:
donestep and returned a normal terminal payload.Gateway wizard completedwas logged.WindowsNodeBootstrapContextStepinjected the managed Windows node context into the runtime workspace, and the pipeline completed successfully in 221 seconds.RealGateway_SystemRun_ExecutesThroughWindowsNodeMxcSandbox,RealGateway_SystemRun_BlocksWritesToTrayDataDirectoryInMxcSandbox, and the bound executable/tail allowlist proofs.D:\openclaw-pr1491-proof-tempdirectory 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.