feat(setup): inject Windows-node guidance into AGENTS.md after pairing#659
feat(setup): inject Windows-node guidance into AGENTS.md after pairing#659paulcam206 wants to merge 1 commit into
Conversation
Add WindowsNodeBootstrapContextStep that runs after node pairing and injects a managed BEGIN/END block into the gateway's AGENTS.md (workspace resolved from `openclaw config get agents.defaults.workspace`, or from a `WindowsNodeContext.WorkspacePath` override). The injected block tells the running OpenClaw agent how to use the paired Windows tray node (nodes tool, exec host=node, tools.exec defaults, etc.). The file mutation is pure POSIX shell + awk + base64 — no Node.js dependency, no embedded JS, no heredocs. Bare WSL distros (and the gateway distro) do not have node installed, so the previous node-based approach failed at runtime. Scripts that need bash variable handling are piped to `bash -s` via stdin instead of being passed as argv to `bash -c`, because wsl.exe performs shell variable expansion on argv before bash sees it and silently drops user-defined $var references. New docs/WSL_EXE_ARGV_PITFALL.md documents the footgun, the empirical reproduction against fresh Ubuntu-26.04, and the ranked fixes. Notable changes: * CommandRunner.RunInWslAsync gains opt-in `inputViaStdin: true` that switches argv to `bash -s` and pipes the script over stdin * WindowsNodeBootstrapContextStep computes the absolute workspace path once via ExpandLinuxPath and threads it through both `openclaw setup --workspace` and the apply script, so `~/foo` or relative-path overrides cannot land in different directories * RunOpenclawSetupAsync and ResolveWorkspacePathAsync use the stdin path because their scripts include $PATH via WslPathPrefix * Apply script is idempotent and handles missing/symlink/malformed AGENTS.md; rollback removes the managed block in-place * Cross-references in AGENTS.md and docs/WINDOWS_NODE_TESTING.md surface the pitfall doc from likely discovery points * Tests cover apply/rollback shape, override expansion, per-call stdin-vs-argv invariants, and a tilde-override regression * Verified end-to-end against bare Ubuntu-26.04 (no openclaw, no node): apply-to-missing, apply-preserves-content, idempotency (exactly 1 BEGIN marker after second apply), rollback restores original content Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Codex review: found issues before merge. Reviewed July 1, 2026, 2:17 AM ET / 06:17 UTC. Summary Reproducibility: not applicable. This PR adds a new setup/enrichment feature rather than reporting a broken existing behavior. The relevant checks are source review, PR-head proof, and the linked setup/security discussion. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Keep the Windows-node discovery help, but deliver it as scoped, explicit, non-blocking guidance that avoids permission-relaxing defaults unless a maintainer-approved opt-in path captures user intent. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds a new setup/enrichment feature rather than reporting a broken existing behavior. The relevant checks are source review, PR-head proof, and the linked setup/security discussion. Is this the best way to solve the issue? No, not as-is. The maintainable path is narrower scoped guidance plus explicit gateway exec consent or documentation, with AGENTS enrichment treated as best-effort after pairing. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4166e0fd63f8. Label changesLabel justifications:
Evidence reviewedSecurity concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
clawsweeper
Bot
added
proof: sufficient
|
just as a heads up -- this change is a little wonky in terms of what we're doing... it works, but I wouldn't say that it's elegant. anyways, here's a completely clean install being asked to pop a notification: |
|
@paulcam206 does it only do it if the message is coming in via the Windows app? |
|
Automated Copilot review note from the wide PR triage pass: I would not treat this as safe to merge at 90% confidence yet, even aside from the current conflict state. The core concern is scope: this PR injects Windows-node guidance into the gateway workspace's global AGENTS.md, so it can affect agent behavior for requests that did not originate from the local Windows tray chat — for example Telegram-originated work, remote/gateway sessions, or future multi-node scenarios. Two things I think should be tightened before merge:
Separately, because this is post-pairing enrichment, I would prefer injection failures/timeouts/malformed existing markers to be non-terminal or self-healing where possible. Optional guidance should not block onboarding after pairing succeeds. |
|
Thanks for the proof and the implementation here. I don't think this is safe to merge as-is yet because the generated Could you please revise this so the injected guidance is clearly scoped to tasks that intentionally need this paired Windows node, remove or soften the permissive |
clawsweeper
Bot
added
proof: 📸 screenshot
2 participants
Summary
Adds a new setup step (
WindowsNodeBootstrapContextStep) that runs after node pairing and injects a managed<!-- BEGIN ... -->/<!-- END ... -->block into the gateway'sAGENTS.md. The injected block tells the OpenClaw agent how to use the paired Windows tray node (thenodestool,exec host=node,tools.execdefaults, etc.) so the agent stops trying to do Windows-only work inside the WSL gateway.Without this step, the agent has no idea a Windows companion node exists and treats the gateway as the only execution surface.
Behavior
openclaw config get agents.defaults.workspace(or aWindowsNodeContext.WorkspacePathoverride).AGENTS.mdif missing; appends the managed block; preserves any existing content above it.Implementation notes
1. Pure POSIX shell, no Node.js dependency
The previous internal iteration of this step embedded a Node.js script for the file mutation. That broke at runtime in bare WSL distros:
openclaw setupis the Rust CLI, not a Node.js app, so node is not guaranteed to be installed. This PR usesawk+base64+ standard POSIX builtins instead — no node, no embedded JS, no heredocs.2. Stdin path on
RunInWslAsync(new opt-in)While debugging this step against a fresh Ubuntu-26.04 we re-discovered a long-standing wsl.exe gotcha already half-documented at
SetupSteps.cs:937-939:So
workspace='/home/openclaw/...'; mkdir -p "$workspace"arrives at bash asmkdir -p ""and dies withcannot create directory ''. This PR adds an opt-ininputViaStdin: trueparameter toICommandRunner.RunInWslAsyncthat pipes the script tobash -sover stdin (which wsl.exe doesn't touch). The new step opts in; existing callers are unchanged.A new
docs/WSL_EXE_ARGV_PITFALL.mddocuments the pitfall with the empirical reproduction table, ranked fixes, and what does NOT work (single quotes, double quotes, braces, subshells, prefix assignment,-e VAR=valforwarding — all verified). The existingValidateWslLockdownStepcomment is expanded with a cross-reference, andAGENTS.mdplusdocs/WINDOWS_NODE_TESTING.mdget pointers so future agents discover it from the right entry points.3. Workspace-path consistency
WindowsNodeContext.WorkspacePathoverrides like~/fooor relative paths used to be passed raw toopenclaw setup --workspacewhile the apply script ran against theExpandLinuxPath-resolved version, which could land setup and injection in different directories. The override is now expanded once and threaded through both invocations.4. Helper calls that include
$PATHRunOpenclawSetupAsyncandResolveWorkspacePathAsyncbuild their command viactx.WslPathPrefix, which references$PATH. On the argv path, wsl.exe expands$PATHto the Windows$PATH— usually harmless because the Linux openclaw bin is prepended, but fragile. Both helpers now use the stdin path so the Linux$PATHis preserved.Validation
./build.ps1✅OpenClaw.Shared.Tests— 2045 passed / 29 skipped ✅OpenClaw.Tray.Tests— 934 passed ✅OpenClaw.SetupEngine.Tests— 222 passed (added tilde-override regression + per-call stdin-vs-argv assertions) ✅--mode local):autoreview clean: no accepted/actionable findings reported · patch is correct (0.82)✅Files
src/OpenClaw.SetupEngine/CommandRunner.csinputViaStdinparameter; XML docs explaining the pitfallsrc/OpenClaw.SetupEngine/SetupSteps.csWindowsNodeBootstrapContextStep(apply + rollback); expanded existing pitfall commentsrc/OpenClaw.SetupEngine/SetupPipeline.csPairNodeStepand beforeVerifyEndToEndStepsrc/OpenClaw.SetupEngine/SetupContext.csWindowsNodeContextConfig(Enabled, WorkspacePath, TimeoutSeconds)src/OpenClaw.SetupEngine/WindowsNodeContextSection.cssrc/OpenClaw.SetupEngine/default-config.jsonWindowsNodeContextsectiontests/OpenClaw.SetupEngine.Tests/*docs/WSL_EXE_ARGV_PITFALL.mddocs/WINDOWS_NODE_TESTING.md,docs/ONBOARDING_WIZARD.md,AGENTS.md