Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ Start with these docs before changing connection, pairing, node, MCP, or tray UX
- `docs/MCP_MODE.md` - local MCP server mode and the `EnableNodeMode` / `EnableMcpServer` matrix.
- `docs/WINDOWS_NODE_TESTING.md` - Windows node capabilities, manual smokes, and gateway-dependent behavior.
- `docs/ONBOARDING_WIZARD.md` - first-run setup flow, setup-code/bootstrap pairing, and test isolation.
- `docs/WSL_EXE_ARGV_PITFALL.md` - wsl.exe argv variable-expansion pitfall; required reading before adding any multi-line WSL script through `RunInWslAsync`.

## Architecture Guardrails for Large Refactors

Expand Down
2 changes: 2 additions & 0 deletions docs/ONBOARDING_WIZARD.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ The Capabilities page applies the selected profile to both setup config and runt

### OpenClaw onboard

After OpenClaw onboard completes—or when the user explicitly skips it—local setup runs the pinned gateway CLI's non-interactive baseline initializer against the final runtime workspace, then writes fixed Windows-node guidance into a setup-owned managed section of that workspace's `AGENTS.md`. The section is replaced idempotently between markers, preserves user-authored `AGENTS.md` content and file permissions outside those markers, and does not modify OpenClaw source files. This helps the initial companion-app OpenClaw session know to use the Windows node / `nodes` tool for Windows desktop, files, screenshots, camera, notifications, browser proxy, and Windows command tasks.

Renders server-defined setup steps via RPC (`wizard.start` / `wizard.next`). The gateway controls the flow — steps can be:
- **Note** — informational messages
- **Confirm** — yes/no decisions
Expand Down
8 changes: 8 additions & 0 deletions docs/WINDOWS_NODE_TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ The Windows Node feature allows the tray app to receive commands from the OpenCl
4. Toggle "Enable Node Mode" ON
5. Click Save

## Companion-App Setup Guidance

For app-owned local WSL setup, after OpenClaw onboard completes or is explicitly skipped, setup runs the pinned gateway CLI's non-interactive baseline initializer against the final runtime workspace and then injects fixed Windows-node guidance into that workspace's `AGENTS.md`. The injected block is setup-owned and idempotently replaced between managed markers, preserving user-authored content and file permissions outside those markers and leaving OpenClaw source files unchanged.

**Note on the apply script's WSL invocation.** The `WindowsNodeBootstrapContextStep` apply and rollback scripts are piped to `bash -s` via stdin (`RunInWslAsync(..., inputViaStdin: true)`) rather than the default `bash -c` argv path. This is required because `wsl.exe` performs shell variable expansion on argv before invoking bash, which would drop user-defined `$var` references in the multi-line script (`workspace='...'` followed by `mkdir -p "$workspace"` becomes `mkdir -p ""`). See `docs/WSL_EXE_ARGV_PITFALL.md` for the full writeup.

The guidance helps the first companion-app OpenClaw session route Windows desktop, files, screenshots, camera, notifications, browser proxy, and Windows command tasks through the Windows node / `nodes` tool.

## What You Can Test Now

### Agent-driven UI and MCP validation
Expand Down
134 changes: 134 additions & 0 deletions docs/WSL_EXE_ARGV_PITFALL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# WSL.exe argv variable-expansion pitfall

## Summary

`wsl.exe -- bash -c <script>` expands shell-variable references in argv before invoking `bash`, so Bash receives an already-mutated script string; any `$var` or `${var}` not defined in the Windows process environment at `wsl.exe` invocation time is dropped to an empty string.

## Reproduction

```powershell
function Invoke-Wsl([string[]]$arr) {
$psi = New-Object System.Diagnostics.ProcessStartInfo
$psi.FileName = "wsl.exe"
$psi.RedirectStandardOutput = $true
$psi.RedirectStandardError = $true
$psi.UseShellExecute = $false
$psi.CreateNoWindow = $true
foreach ($a in $arr) { [void]$psi.ArgumentList.Add($a) }
$p = [System.Diagnostics.Process]::Start($psi)
$out = $p.StandardOutput.ReadToEnd()
$err = $p.StandardError.ReadToEnd()
$p.WaitForExit()
"EXIT=$($p.ExitCode) STDOUT=[$out] STDERR=[$err]"
}

# BROKEN: argv path — $x is dropped before bash sees it
Invoke-Wsl @("-d","Ubuntu-26.04","--","bash","-c","x=abc; echo VAL=`$x")
# → EXIT=0 STDOUT=[VAL=] ← assignment ran, but $x was already expanded to empty

# WORKING: stdin path — script bytes arrive at bash intact
$psi = New-Object System.Diagnostics.ProcessStartInfo
$psi.FileName = "wsl.exe"
$psi.RedirectStandardOutput = $true
$psi.RedirectStandardInput = $true
$psi.UseShellExecute = $false
foreach ($a in @("-d","Ubuntu-26.04","--","bash","-s")) { [void]$psi.ArgumentList.Add($a) }
$p = [System.Diagnostics.Process]::Start($psi)
$p.StandardInput.WriteLine("x=abc; echo VAL=`$x")
$p.StandardInput.Close()
$p.StandardOutput.ReadToEnd()
# → VAL=abc
```

## What bash actually receives

Empirical results from dumping `/proc/$$/cmdline` inside Bash on a fresh Ubuntu-26.04 WSL distro:

| Pattern in script string | What bash actually receives |
|---|---|
| `$PATH` (defined in Windows env at `wsl.exe` invocation time) | the full Windows `PATH` expansion |
| `$workspace` (not defined in Windows env) | **removed (empty)** |
| `${workspace}` braced (not defined in Windows env) | **removed (empty)** |
| `$$` | `wsl.exe` parent process's PID, not the Bash PID |
| `\$workspace` backslash-escaped | preserved as `$workspace`; Bash then expands it normally |
| `$(echo hi)` command substitution | preserved; Bash expands it |
| Single-quoted `'$workspace'` | still expanded; single quotes do not help because `wsl.exe` runs before Bash |
| Double-quoted `"$workspace"` | still expanded |
| Subshell `( workspace=x; echo $workspace )` | the inner `$workspace` is still dropped |
| Prefix assignment `x=abc echo $x` | `$x` is still dropped |

Concrete failure mode:

```bash
workspace='/home/openclaw/.openclaw/workspace'
mkdir -p "$workspace" # → mkdir: cannot create directory '': No such file or directory
```

The assignment runs because it has no `$var` reference, but `$workspace` on the next line is removed during `wsl.exe` argv translation.

## Why

`wsl.exe`'s argv translation layer treats argv strings as command-line text and performs shell metacharacter expansion before launching the target process. By the time `bash -c` runs, the original `$var` syntax is gone and Bash cannot recover it. This behavior is consistent across single quotes, double quotes, braces, subshells, and prefix assignment because they are all interpreted by `wsl.exe` before Bash sees the script.

## Fixes (in order of preference)

### 1. Pipe the script over stdin via `RunInWslAsync(..., inputViaStdin: true)`

`wsl.exe` does not rewrite stdin. Prefer this for any multi-line script that uses Bash variables, `${...}`, or `$$`.

```csharp
var script = """
workspace='/home/openclaw/.openclaw/workspace'
mkdir -p "$workspace"
printf 'bash pid=%s\n' "$$"
""";

await commandRunner.RunInWslAsync(
distroName,
script,
cancellationToken,
inputViaStdin: true);
```

### 2. C#-interpolate every value into the script string

Do not store values in Bash variables; bake the values into the script literally. This is the workaround used by `src/OpenClaw.SetupEngine/SetupSteps.cs:936-945` in `ValidateWslLockdownStep`. It is acceptable for short scripts with a small fixed value set and no spaces in values.

```csharp
var workspace = "/home/openclaw/.openclaw/workspace";
var script = $"mkdir -p {workspace} && test -d {workspace}";

await commandRunner.RunInWslAsync(distroName, script, cancellationToken);
```

### 3. Backslash-escape `\$var`

Escaping the dollar sign preserves it through `wsl.exe` and lets Bash expand it later.

```powershell
wsl.exe -d Ubuntu-26.04 -- bash -c "x=abc; echo VAL=\`$x"
```

This works for single isolated references, but it is fragile and easy to miss in any non-trivial script. Treat it as a last resort.

## What does NOT work

All of these failed workarounds were verified empirically:

- **Single quotes** — `'$workspace'` is still rewritten before Bash sees the quotes.
- **Double quotes** — `"$workspace"` is also rewritten before Bash sees the quotes.
- **Braces** — `${workspace}` is removed just like `$workspace`.
- **Subshells** — `( workspace=x; echo $workspace )` still loses the inner `$workspace`.
- **Prefix assignment** — `x=abc echo $x` still expands `$x` before the assignment can matter.
- **`-e VAR=val` flag forwarding** — forwarded environment values do not prevent argv rewriting before Bash receives the script.
- **Switching to `/bin/sh`** — the mutation happens in `wsl.exe`, before any shell starts.

## Where this matters in the codebase

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

## Related

- `docs/XAML_COMPILER_BUG.md` — sibling footgun doc for XAML compiler crashes.
1 change: 1 addition & 0 deletions src/OpenClaw.SetupEngine.UI/Pages/ProgressPage.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ private void Onboard_Click(object sender, RoutedEventArgs e)
private static List<SetupStep> BuildSteps(SetupConfig config)
=> SetupStepFactory.BuildDefaultSteps()
.Where(step => step is not RunGatewayWizardStep)
.Where(step => config.SkipWizard || step is not WindowsNodeBootstrapContextStep)
.ToList();
}

Expand Down
53 changes: 47 additions & 6 deletions src/OpenClaw.SetupEngine.UI/Pages/WizardPage.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public sealed partial class WizardPage : Page
private WizardStepCategory _stepCategory = WizardStepCategory.Acknowledge;
private bool _sensitive;
private bool _errorState;
private bool _finalizationErrorState;
private int _operationGeneration;
private int _wizardStepCount;
private int _progressPolls;
Expand Down Expand Up @@ -104,6 +105,7 @@ private async Task StartWizardAsync(bool clearTranscript = true)
try
{
_errorState = false;
_finalizationErrorState = false;
HideRecoveryActions();
// Cancel any in-progress server-side wizard session before starting a
// fresh one, so the gateway doesn't reject wizard.start with "wizard
Expand Down Expand Up @@ -236,8 +238,7 @@ private async Task ApplyPayloadAsync(JsonElement payload)
if (generation != _operationGeneration || _errorState)
return;

// Permissions are collected before install, so the wizard completes straight to summary.
SetupWindow.Active?.NavigateToComplete(true, TimeSpan.Zero, _config!.LogPath);
await CompleteSetupAsync(generation);
return;
}

Expand Down Expand Up @@ -514,6 +515,14 @@ private async Task PrimaryClickAsync()
{
if (_errorState)
{
if (_finalizationErrorState)
{
_errorState = false;
_finalizationErrorState = false;
await CompleteSetupAsync(_operationGeneration);
return;
}

await StartWizardAsync();
return;
}
Expand Down Expand Up @@ -1008,6 +1017,7 @@ private void SetBusy(string status)
private void ShowError(string message)
{
_errorState = true;
_finalizationErrorState = false;
BusyRing.Visibility = Visibility.Collapsed;
BusyRing.IsActive = false;
StatusText.Text = "Wizard needs attention";
Expand All @@ -1021,6 +1031,14 @@ private void ShowError(string message)
MaybeShowGatewayRecovery();
}

private void ShowFinalizationError(string message)
{
ShowError(message);
_finalizationErrorState = true;
StatusText.Text = "Windows integration needs attention";
PrimaryButton.Content = "Retry Windows integration";
}

private async Task EnterWizardErrorAsync(string detail)
{
if (_errorState)
Expand Down Expand Up @@ -1140,13 +1158,36 @@ private async Task RestartGatewayAsync()

private async Task SkipWizardAsync()
{
AdvanceOperationGeneration();
var generation = AdvanceOperationGeneration();
_errorState = false;
HideRecoveryActions();
SetBusy("Skipping wizard...");
await CancelCurrentSessionAsync();
// Permissions were already collected before install, so skipping OpenClaw
// onboard completes straight to the summary.
SetupWindow.Active?.NavigateToComplete(true, TimeSpan.Zero, _config!.LogPath);
await CompleteSetupAsync(generation);
}

private async Task CompleteSetupAsync(int generation)
{
if (generation != _operationGeneration || _errorState)
return;

var setupWindow = SetupWindow.Active;
if (setupWindow is null or { IsClosed: true })
return;

SetBusy("Finishing Windows integration...");
var contextResult = await setupWindow.ApplyWindowsNodeContextAsync();
if (generation != _operationGeneration || setupWindow.IsClosed)
return;

if (!contextResult.IsSuccess)
{
ShowFinalizationError($"OpenClaw onboard finished, but Windows node guidance could not be installed: {contextResult.Message}");
return;
}

// Permissions were collected before install, so completion goes straight to summary.
setupWindow.NavigateToComplete(true, TimeSpan.Zero, _config!.LogPath);
}

private async Task CancelCurrentSessionAsync()
Expand Down
79 changes: 74 additions & 5 deletions src/OpenClaw.SetupEngine.UI/SetupWindow.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ public sealed partial class SetupWindow : Window
{
private SetupConfig _config = null!;
private SetupRunLock? _setupLock;
private readonly CancellationTokenSource _lifetimeCts = new();
private Task<StepResult>? _contextApplyTask;
private readonly TaskCompletionSource<bool> _initialContentReady =
new(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly TaskCompletionSource<bool> _cleanupCompleted =
new(TaskCreationOptions.RunContinuationsAsynchronously);
private bool _isClosed;
private bool _persistStartupPreferenceOnComplete = true;
private bool _showStartupPreferenceOnComplete = true;
Expand All @@ -26,6 +30,7 @@ public sealed partial class SetupWindow : Window
public event EventHandler? AdvancedSetupRequested;
public event EventHandler<SetupCompletedEventArgs>? SetupCompleted;
public bool IsClosed => _isClosed;
public Task CleanupCompleted => _cleanupCompleted.Task;
internal string DataDir => _dataDir;
internal string LocalDataDir => _localDataDir;
public bool CanNavigateToWizard =>
Expand Down Expand Up @@ -109,14 +114,32 @@ public SetupWindow(
_showStartupPreferenceOnComplete = false;
}

Closed += (_, _) =>
Closed += async (_, _) =>
{
_isClosed = true;
_initialContentReady.TrySetResult(true);
_setupLock?.Dispose();
_setupLock = null;
if (ReferenceEquals(Active, this))
Active = null;
try
{
_lifetimeCts.Cancel();
if (_contextApplyTask is { } contextApplyTask)
await contextApplyTask;
}
catch (OperationCanceledException)
{
// Window teardown owns this cancellation; cleanup still must finish.
}
catch (Exception ex)
{
System.Diagnostics.Debug.WriteLine($"Setup cleanup failed: {ex}");
}
finally
{
_setupLock?.Dispose();
_setupLock = null;
if (ReferenceEquals(Active, this))
Active = null;
_cleanupCompleted.TrySetResult(true);
}
};

var previewPage = SetupPreview.RequestedPage;
Expand Down Expand Up @@ -168,6 +191,52 @@ public bool TryNavigateToWizard(bool back = false)
return true;
}

internal async Task<StepResult> ApplyWindowsNodeContextAsync()
{
if (_contextApplyTask is { } existingTask)
return await existingTask;

_contextApplyTask = ApplyWindowsNodeContextCoreAsync();
try
{
return await _contextApplyTask;
}
finally
{
_contextApplyTask = null;
}
}

private async Task<StepResult> ApplyWindowsNodeContextCoreAsync()
{
if (!_config.WindowsNodeContext.Enabled)
return StepResult.Skip("Windows node context injection disabled");

var ct = _lifetimeCts.Token;
using var logger = new SetupLogger(filePath: null);
using var journal = new TransactionJournal(filePath: null, logger);
var context = new SetupContext(
_config,
logger,
journal,
new CommandRunner(logger),
ct,
_dataDir,
_localDataDir);
// This is an idempotent refresh after onboarding, not a transactional
// install. A failed refresh must not remove a valid block from an earlier run.
var pipeline = new SetupPipeline(
[new WindowsNodeBootstrapContextStep()],
rollbackOnFailureOverride: false);
var result = await pipeline.RunAsync(context);
return result.Outcome switch
{
PipelineOutcome.Success => StepResult.Ok("Windows node context injected"),
PipelineOutcome.Cancelled => StepResult.Fail("Windows node context injection was cancelled"),
_ => StepResult.Fail(result.Message ?? "Windows node context injection failed")
};
}

public void NavigateToComplete(bool success, TimeSpan elapsed, string? logPath, string? errorMessage = null)
=> NavigateTo(
typeof(CompletePage),
Expand Down
Loading