Skip to content

Commit b37307c

Browse files
bkudiessCopilotkarkarl
authored
refactor(setup): split setup steps and own keepalive processes (#1093)
* refactor(setup): split setup steps and own keepalive processes Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs(setup): comply with prose punctuation policy Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 61934333-9a0d-4743-b5e1-cc62b0f65570 --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Karen Lai <7976322+karkarl@users.noreply.github.com> Copilot-Session: 61934333-9a0d-4743-b5e1-cc62b0f65570
1 parent 99730e7 commit b37307c

31 files changed

Lines changed: 5027 additions & 4101 deletions

docs/ARCHITECTURE.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ These are the canonical homes. Do not reintroduce private copies elsewhere.
120120
| `src/OpenClaw.Shared/OpenClawGatewayClient.cs` | `GatewayMessageRouter`, per-domain API facades |
121121
| `src/OpenClaw.Shared/Models.cs` | per-domain model files + `*Mapper` classes |
122122
| `src/OpenClaw.Shared/Capabilities/SystemCapability.cs` | `ExecApprovalService` |
123-
| `src/OpenClaw.SetupEngine/SetupSteps.cs` | one file per step; `WslShellClient`, `GatewayConfigScriptBuilder`, `KeepaliveProcessManager`. WSL/POSIX quoting is done - use `WslShellQuoting`, never a local `ShellEscape`. |
123+
| `src/OpenClaw.SetupEngine/SetupSteps.cs` | one file per step (done for the steps still referencing this file); `WslShellClient` and `GatewayConfigScriptBuilder` remain pending. WSL/POSIX quoting is done - use `WslShellQuoting`, never a local `ShellEscape`. Setup-time keepalive process ownership is authoritative in `KeepaliveProcessManager` (see `setup-keepalive-process-manager`). |
124124
| `src/OpenClaw.Connection/GatewayConnectionManager.cs` | The three connection-domain owners are authoritative. Keep only the public lifecycle facade, manager-owned operator/state/tunnel orchestration, typed source/sink/security ports, and event forwarding. |
125125
| Any test hand-rolling a temp dir / env save-restore / CLI capture | `OpenClaw.TestSupport` fixtures |
126126

@@ -156,7 +156,8 @@ leading and trailing pipe. Columns, in order:
156156
| test-settings-builder | authoritative | scattered test files | ad hoc SettingsData construction in migrated tests | OpenClaw.TestSupport.SettingsDataBuilder | pre-existing un-migrated tests until adopted | settings test data starts from production defaults | TestSupportFixtureTests.SettingsDataBuilder_StartsFromDefaults | behavioral | when settings tests adopt the builder |
157157
| json-read-helpers | authoritative | OpenClaw.Shared (multiple files) | duplicate non-nullable fallback-returning JsonElement getters | JsonReadHelpers | null-sentinel / non-negative / whitespace-absent / trimming variants stay separate | canonical non-nullable fallback JSON coercion; divergent-contract helpers are not blindly routed here | JsonReadHelpersTests.GetString_ReturnsNull_WhenPropertyMissing | behavioral | when the non-nullable fallback getters are all routed here |
158158
| wsl-posix-quoting | authoritative | OpenClaw.SetupEngine/SetupSteps.cs | ad hoc ShellEscape with divergent wrap semantics | WslShellQuoting | - | WSL command lines use POSIX single-quote quoting via WslShellQuoting not cmd/PowerShell quoting | WslShellQuotingTests.QuotePosixSingleQuote_WrapsAndEscapesEmbeddedQuote | behavioral | when no code builds WSL command lines outside WslShellQuoting |
159-
| setup-shellescape-closed | closed | src/OpenClaw.SetupEngine/SetupSteps.cs | private ShellEscape helpers with divergent wrap semantics | WslShellQuoting | - | SetupSteps builds WSL command lines only via WslShellQuoting; no local ShellEscape helper | SetupStepsShellEscapeClosureTests.SetupSteps_DoesNotReintroduce_PrivateShellEscape | source-shape | when SetupSteps.cs no longer builds any WSL command strings |
159+
| setup-shellescape-closed | closed | src/OpenClaw.SetupEngine/SetupSteps.cs | private ShellEscape helpers with divergent wrap semantics | WslShellQuoting | - | OpenClaw.SetupEngine builds WSL command lines only via WslShellQuoting; no local ShellEscape helper anywhere in the project | SetupStepsShellEscapeClosureTests.SetupEngine_DoesNotReintroduce_PrivateShellEscape | source-shape | when no file under src/OpenClaw.SetupEngine builds any WSL command strings |
160+
| setup-keepalive-process-manager | authoritative | src/OpenClaw.SetupEngine/SetupSteps.cs (StartKeepaliveStep) | setup-time WSL keepalive process discovery, start, marker read/write, command-line identity, and rollback cleanup | KeepaliveProcessManager (raw OS calls delegated to internal IKeepaliveProcessRuntime seam; StartKeepaliveStep is the only caller that reads SetupContext) | StartKeepaliveStep keeps Id/DisplayName and thin ExecuteAsync/RollbackAsync orchestration only | setup-time keepalive never hard-fails the pipeline on start failure (null PID or thrown exception both soft-fail identically); its marker path/JSON are the intentional handoff consumed by the tray keepalive service; rollback kills only wsl/wsl.exe processes whose command line matches this distro via WslCommandLineMatcher, leaves wrong-distro/unmatched command lines untouched, and deletes only its own marker/empty directory | KeepaliveProcessManagerTests.RollbackAsync_KillsOnlyMatchingDistroProcesses_LeavesOthersUntouched | behavioral | when StartKeepaliveStep contains no process/marker logic of its own |
160161
| wsl-distro-install-path | authoritative | OpenClaw.SetupEngine/SetupSteps.cs | inline Path.Combine wsl distro install-path derivation | DistroInstallPathPolicy | - | new installs use the strict supported name grammar; teardown accepts only unambiguous single-segment names whose canonical path is an immediate child of LocalDataDir\wsl with no aliases, case or Unicode collisions, or reparse points at the root or child | SetupStepsTests.DistroInstallPathPolicy_ResolvesImmediateChild | behavioral | - |
161162
| managed-local-provenance | authoritative | scattered connection, setup, browser, and reconnect call sites | implicit loopback trust and duplicated strong-credential listener checks | ManagedLocalGatewayPortProvenanceService | callers request inspection, authorization, or conflict repair only | unknown, incomplete, conflicting, or changed Windows listener ownership never receives strong credentials or destructive remediation; relayless ownership requires a complete empty Windows snapshot, expected-distro systemd MainPID proof, and immediate complete empty revalidation | ManagedLocalGatewayPortProvenanceServiceTests.InteractiveCredentialGate_ExpectedCacheThenOwnerChanges_FailsClosed | behavioral | - |
162163
| gateway-wizard-restart-recovery | authoritative | WizardPage + SetupWizardRunner reconnect call sites | duplicated exact-version terminal-restart classification and bounded provenance retry orchestration | GatewayWizardRestartRecoveryPolicy | WizardPage and SetupWizardRunner apply hosted and headless lifecycle and consume provenance inspection results | only managed-local restart-like disconnects may retry NoListener or the typed snapshot-changed race; other unknown or conflicting ownership fails immediately, retryable startup close 1013 stays inside the existing reconnect bound, and exact Gateway 2026.7.1 final model-check close 1012 completes only after a fresh hello-ok, and a terminal hosted-wizard payload completes on the exact TUI SIGTERM termination only when the request just sent answered the authoritative final done acknowledgement step | GatewayWizardRestartRecoveryPolicyTests.Exact2026_7_1TerminalModelCheckServiceRestart_IsExpected | behavioral | when the 2026.7.1 terminal-restart compatibility path is removed |

docs/SETUP_ENGINE_REDESIGN.md

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,13 @@ src/OpenClaw.SetupEngine/
3030
├── Program.cs # callable entry: --config, --headless, --dry-run, --rollback-on-failure
3131
├── SetupPipeline.cs # Sequential step orchestrator (132 lines)
3232
├── SetupContext.cs # Config model + shared state bag (217 lines)
33-
├── SetupSteps.cs # All setup step implementations
33+
├── SetupSteps.cs # Shared setup-engine helpers (WslConstants, WslInstallSupport,
34+
│ # SetupOpenClawLogger, SetupPairingCredentialPolicy,
35+
│ # WindowsGatewayReachability); one file per step class lives
36+
│ # alongside it (e.g. CreateWslInstanceStep.cs,
37+
│ # ConfigureGatewayStep.cs, StartKeepaliveStep.cs, ...)
38+
├── KeepaliveProcessManager.cs # Setup-time WSL keepalive process/marker/rollback owner
39+
├── TailscaleSetupSteps.cs # The 4 Tailscale setup steps, grouped
3440
├── TransactionJournal.cs # Append-only JSONL journal (77 lines)
3541
├── SetupLogger.cs # Structured JSONL logger (112 lines)
3642
├── CommandRunner.cs # Concrete WSL/process command runner
@@ -50,7 +56,9 @@ src/OpenClaw.SetupEngine.UI/
5056
└── CompletePage.xaml / .cs # Mascot status badge, summary, startup toggle
5157
```
5258

53-
**Total engine code: ~1,882 lines across 8 files.** UI adds ~10 more files.
59+
The pipeline runs 24 steps (see `SetupStepFactory.BuildDefaultSteps()` in `SetupPipeline.cs` for
60+
the authoritative order. This doc's step table below predates the 4 Tailscale steps and is not
61+
fully current). UI adds ~10 more files.
5462

5563
---
5664

@@ -158,7 +166,12 @@ rerun setup with a supported new name.
158166

159167
## Pipeline Steps (24 total)
160168

161-
Executed sequentially. Each step is a small class (30–120 lines) in `SetupSteps.cs`.
169+
> Note: this table predates the 4 Tailscale setup steps; the current pipeline runs 24 steps
170+
> total. See `SetupStepFactory.BuildDefaultSteps()` in `SetupPipeline.cs` for the authoritative,
171+
> current order. Fixing this table fully is out of scope for the E0 file-split PR.
172+
173+
Executed sequentially. Each step is a small class (30–120 lines) in its own file under
174+
`src/OpenClaw.SetupEngine/` (e.g. `PreflightOsStep.cs`).
162175

163176
| # | Step Class | What It Does |
164177
|---|-----------|-------------|

docs/WSL_EXE_ARGV_PITFALL.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ All of these failed workarounds were verified empirically:
126126
## Where this matters in the codebase
127127

128128
- `src/OpenClaw.SetupEngine/CommandRunner.cs` - `RunInWslAsync` exposes the opt-in `inputViaStdin` parameter.
129-
- `src/OpenClaw.SetupEngine/SetupSteps.cs:936-945` - `ValidateWslLockdownStep` uses workaround #2, C# interpolation.
130-
- `src/OpenClaw.SetupEngine/SetupSteps.cs` `WindowsNodeBootstrapContextStep` - uses workaround #1, stdin.
129+
- `src/OpenClaw.SetupEngine/ValidateWslLockdownStep.cs` - `ValidateWslLockdownStep` uses workaround #2, C# interpolation.
130+
- `src/OpenClaw.SetupEngine/WindowsNodeBootstrapContextStep.cs` - `WindowsNodeBootstrapContextStep` uses workaround #1, stdin.
131131

132132
## Related
133133

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
using System.Diagnostics;
2+
using System.Net;
3+
using System.Net.Http;
4+
using System.Net.Sockets;
5+
using System.Runtime.InteropServices;
6+
using System.Security.Cryptography;
7+
using System.Text.Json;
8+
using OpenClaw.Connection;
9+
using OpenClaw.Shared;
10+
11+
namespace OpenClaw.SetupEngine;
12+
13+
public sealed class CleanupStaleDistroStep : SetupStep
14+
{
15+
public override string Id => "cleanup-distro";
16+
public override string DisplayName => "Clean up stale WSL distro";
17+
public override bool CanRetry => false;
18+
19+
public override bool CanSkip(SetupContext ctx) => !ctx.Config.CleanBeforeRun;
20+
21+
public override async Task<StepResult> ExecuteAsync(SetupContext ctx, CancellationToken ct)
22+
{
23+
var distro = ctx.DistroName!;
24+
if (!DistroInstallPathPolicy.TryGetManagedInstallPath(ctx.LocalDataDir, distro, out var wslDir, out var pathError))
25+
return StepResult.Terminal(pathError);
26+
27+
var list = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--list", "--quiet"], TimeSpan.FromSeconds(15), ct: ct);
28+
if (list.ExitCode != 0)
29+
return StepResult.Ok("WSL not available or no distros - nothing to clean");
30+
31+
var distros = WslInstallSupport.ParseQuietDistroList(list.Stdout);
32+
33+
ctx.Logger.Debug($"Found WSL distros: [{string.Join(", ", distros)}]");
34+
35+
if (!distros.Any(d => d.Equals(distro, StringComparison.OrdinalIgnoreCase)))
36+
{
37+
// Distro not registered, but disk directory may still exist from prior crash
38+
if (Directory.Exists(wslDir))
39+
{
40+
ctx.Logger.Info($"Removing orphaned WSL directory: {wslDir}");
41+
var delete = await DeleteDistroDirectoryWithRetries(ctx, distro, wslDir, ct);
42+
if (!delete.IsSuccess)
43+
return delete;
44+
}
45+
ctx.Logger.Decision("No stale distro found", "skip cleanup");
46+
return StepResult.Ok("No stale distro to clean");
47+
}
48+
49+
ctx.Logger.Decision($"Found existing distro '{distro}'", "terminating and unregistering");
50+
51+
// Stop only the app-owned distro. Global WSL shutdown would disrupt unrelated distros.
52+
await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--terminate", distro], TimeSpan.FromSeconds(30), ct: ct);
53+
await Task.Delay(2000, ct); // Let port release
54+
55+
var unregister = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--unregister", distro], TimeSpan.FromSeconds(60), ct: ct);
56+
if (unregister.ExitCode != 0)
57+
{
58+
ctx.Logger.Warn($"First unregister attempt failed (exit {unregister.ExitCode}); retrying targeted termination");
59+
await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--terminate", distro], TimeSpan.FromSeconds(30), ct: ct);
60+
await Task.Delay(3000, ct);
61+
unregister = await ctx.Commands.RunAsync(WslConstants.WslExePath, ["--unregister", distro], TimeSpan.FromSeconds(60), ct: ct);
62+
}
63+
64+
if (unregister.ExitCode == 0)
65+
{
66+
// Also remove the on-disk WSL vhdx directory (--import fails if it exists)
67+
var delete = await DeleteDistroDirectoryWithRetries(ctx, distro, wslDir, ct);
68+
if (!delete.IsSuccess)
69+
return delete;
70+
71+
// Wait for port to be released
72+
ctx.Logger.Info("Waiting for port release after distro termination...");
73+
await PreflightPortStep.WaitForPortFreeAsync(ctx.Config.GatewayPort, ctx.Config.Gateway.Bind, ctx.Logger, ct);
74+
return StepResult.Ok($"Unregistered stale distro '{distro}'");
75+
}
76+
77+
return StepResult.Fail($"Failed to unregister distro: {unregister.Stderr}");
78+
}
79+
80+
internal static async Task<StepResult> DeleteDistroDirectoryWithRetries(
81+
SetupContext ctx,
82+
string distroName,
83+
string wslDir,
84+
CancellationToken ct)
85+
{
86+
var deletePath = wslDir;
87+
Exception? lastError = null;
88+
89+
for (var attempt = 0; attempt < 4; attempt++)
90+
{
91+
if (!DistroInstallPathPolicy.TryValidateDeleteTarget(
92+
ctx.LocalDataDir,
93+
distroName,
94+
wslDir,
95+
out deletePath,
96+
out var pathError))
97+
{
98+
return StepResult.Terminal(pathError);
99+
}
100+
101+
try
102+
{
103+
if (File.Exists(deletePath))
104+
{
105+
if (File.GetAttributes(deletePath).HasFlag(FileAttributes.ReparsePoint))
106+
return StepResult.Fail($"App-owned WSL path '{deletePath}' is a reparse point; remove it manually and retry setup.");
107+
108+
ctx.Logger.Info($"Removing app-owned WSL file at install path: {deletePath}");
109+
File.Delete(deletePath);
110+
}
111+
else if (Directory.Exists(deletePath))
112+
{
113+
if (new DirectoryInfo(deletePath).Attributes.HasFlag(FileAttributes.ReparsePoint))
114+
return StepResult.Fail($"App-owned WSL directory '{deletePath}' is a reparse point; remove it manually and retry setup.");
115+
116+
ctx.Logger.Info($"Removing app-owned WSL directory: {deletePath}");
117+
Directory.Delete(deletePath, recursive: true);
118+
}
119+
120+
var parent = Path.GetDirectoryName(deletePath);
121+
if (!string.IsNullOrWhiteSpace(parent) &&
122+
Directory.Exists(parent) &&
123+
!new DirectoryInfo(parent).Attributes.HasFlag(FileAttributes.ReparsePoint) &&
124+
!Directory.EnumerateFileSystemEntries(parent).Any())
125+
{
126+
Directory.Delete(parent);
127+
ctx.Logger.Info("Deleted empty wsl\\ parent directory");
128+
}
129+
130+
return StepResult.Ok("WSL directory removed");
131+
}
132+
catch (DirectoryNotFoundException)
133+
{
134+
return StepResult.Ok("WSL directory already absent");
135+
}
136+
catch (IOException ex)
137+
{
138+
lastError = ex;
139+
if (attempt >= 3)
140+
break;
141+
142+
ctx.Logger.Warn($"VHD directory still locked, retrying in {(attempt + 1) * 2}s...");
143+
await Task.Delay(TimeSpan.FromSeconds((attempt + 1) * 2), ct);
144+
}
145+
catch (UnauthorizedAccessException ex)
146+
{
147+
lastError = ex;
148+
if (attempt >= 3)
149+
break;
150+
151+
ctx.Logger.Warn($"VHD directory access denied, retrying in {(attempt + 1) * 2}s...");
152+
await Task.Delay(TimeSpan.FromSeconds((attempt + 1) * 2), ct);
153+
}
154+
}
155+
156+
return StepResult.Fail(
157+
$"Failed to remove app-owned WSL directory '{deletePath}'. Close any process using the OpenClaw WSL distro and retry setup."
158+
+ (lastError is null ? "" : $" Last error: {lastError.Message}"));
159+
}
160+
}

0 commit comments

Comments
 (0)