Skip to content

fix(process): Windows exec/read empty output when detached spawn leaks#110198

Closed
stantheman0128 wants to merge 3 commits into
openclaw:mainfrom
stantheman0128:fix/105528-windows-empty-output
Closed

fix(process): Windows exec/read empty output when detached spawn leaks#110198
stantheman0128 wants to merge 3 commits into
openclaw:mainfrom
stantheman0128:fix/105528-windows-empty-output

Conversation

@stantheman0128

@stantheman0128 stantheman0128 commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #105528

What Problem This Solves

Fixes an issue where Windows users running OpenClaw under a headless host (Scheduled Task / service) can see exec and read tools return empty output with no error when detached: true reaches Node child_process.spawn.

Why This Change Was Made

Call sites already try to disable detached spawn on Windows, but those guards are scattered. This adds a centralized belt-and-suspenders guard in spawnWithFallback so every caller is covered, matching the historical Windows empty-stdout fix (#17806 / #18035) and the reporter's proposed upstream patch.

User Impact

Windows users should get real tool stdout/stderr from supervisor-backed exec paths even if a caller accidentally requests detached: true. POSIX detached behavior is unchanged.

AI Assistance

AI-assisted (Cursor/Grok). Human author: Stan Shih (stantheman0128).

Evidence

Labels on #105528 at publish time: bug, docs (no clawsweeper:no-new-fix-pr / needs-*). Covering open PR search for 105528 only hits meta tracker #74163.

Gold A/B (Windows 11, Node v24.15.0, tip d1aa9239cb)

Reproducible empty-before on this host with powershell.exe -NoProfile -Command Write-Output ... when detached: true is passed straight to Node spawn. The branch spawnWithFallback forces detached: false and restores stdout.

BEFORE unsanitized spawn (powershell-write):
  detached=false empty=false stdout="hello-ps-105528\r\n"
  detached=true  empty=true  stdout=""

AFTER spawnWithFallback (caller requests detached:true):
  detachedRequested=true detachedPassedToNode=false forcedDetachedFalse=true
  empty=false stdout="hello-ps-105528\r\n"

Regression test (same A/B):

pnpm exec vitest run src/process/spawn-utils.test.ts
Test Files  1 passed (1)
Tests  6 passed (6)

Note: cmd.exe /c echo and plain node -e process.stdout.write both returned stdout even with detached: true on this interactive host. The PowerShell path is the clear empty-before / non-empty-after signal.

Unit / helper (retained)

Windows forcing detached: false even when callers pass detached: true; POSIX keeping detached: true; cmd echo via spawnWithFallback.

Earlier Gateway after-fix path (still valid)

Isolated Gateway from this PR tree previously returned non-empty exec/read (hello-exec-105528, hello-read-105528-from-workspace) via Gateway-backed agent turn. That run did not show empty-before; the PowerShell A/B above is the missing before/after.

Maintainer note

createChildAdapter already passes detached: false on Windows today. This PR remains a defense-in-depth guard in spawnWithFallback for any future caller that requests detached: true (PowerShell-style capture loss is now proven on this box).

Made with Cursor

@clawsweeper clawsweeper Bot added 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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 17, 2026
@clawsweeper

clawsweeper Bot commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 20, 2026, 3:16 PM ET / 19:16 UTC.

Summary
The branch forces detached: false for Windows calls to spawnWithFallback, including fallback attempts, and adds helper-level tests for the sanitization.

PR surface: Source +18, Tests +77. Total +95 across 2 files.

Reproducibility: no. high-confidence reproduction is available yet: the report gives a Windows main-session/headless path, but the contributor states the prior failure was not reproduced and the provided successful Gateway run does not demonstrate the patch executed on that failing path.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #105528
Summary: This PR is explicitly proposed as a candidate fix for the open Windows exec/read empty-output report, but its evidence has not yet shown that the proposed helper is on the failing path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Correct the evidence date and post redacted original terminal or Gateway logs from a headless Scheduled Task/service before-and-after run.
  • [P1] Show whether detached: true reaches spawnWithFallback in the failing path; otherwise narrow the fix to the traced owner boundary.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR includes a described Windows Gateway success run, but it does not reproduce the reported headless failure, does not show this sanitizer executing on that path, and is future-dated July 21, 2026 relative to this July 20, 2026 review; provide corrected redacted terminal/log evidence from a before/after Scheduled Task or service run. 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.

Risk before merge

  • [P1] Merging a helper-level Windows override without a traced failing caller can silently change detached-process semantics for future callers while leaving the reported session-specific failure unresolved.
  • [P1] The claimed live evidence is future-dated as of July 20, 2026 and lacks a before/after headless Scheduled Task or service reproduction, so it cannot yet attribute the observed successful run to this patch.
  • [P1] The branch is behind main; refresh the merge review after rebasing, but do not treat base drift alone as evidence of a runtime regression.

Maintainer options:

  1. Trace and prove the affected path (recommended)
    Reproduce the Scheduled Task or service Gateway path with a redacted before/after trace that shows detached: true reaches this helper, then keep or relocate the fix to the actual owner boundary.
  2. Pause the global guard
    Do not merge the helper-wide override until the open Windows report has a traced runtime path; preserve detached behavior for unproven present and future callers.

Next step before merge

  • [P1] A safe repair cannot be selected until the reporter/contributor traces the session-specific Windows failure to a concrete owner boundary; the current helper guard has not been shown to affect it.

Security
Cleared: The diff is limited to a process-option sanitizer and unit tests; it adds no dependencies, workflows, secrets handling, package-resolution changes, or new code-execution source.

Review findings

  • [P1] Trace the failing Windows tool path before overriding the helper — src/process/spawn-utils.ts:109
Review details

Best possible solution:

Identify the actual Windows main-session or headless Gateway boundary that passes detached spawn into the failing exec/read path, then either place a narrow fix there with before/after proof or retain this guard only after proving it executes on that path and does not block an intentional detached caller.

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

No high-confidence reproduction is available yet: the report gives a Windows main-session/headless path, but the contributor states the prior failure was not reproduced and the provided successful Gateway run does not demonstrate the patch executed on that failing path.

Is this the best way to solve the issue?

No: forcing the shared helper is a plausible defense-in-depth measure, but it is not yet the best proven fix because the only identified production caller already sets detached: false on Windows and the session-specific failure boundary remains untraced.

Full review comments:

  • [P1] Trace the failing Windows tool path before overriding the helper — src/process/spawn-utils.ts:109
    createChildAdapter is reported to already pass detached: false on Windows, so this new shared override has not been shown to run for the reported main-session failure. The supplied success run also does not reproduce the pre-fix failure and is dated July 21, 2026—after this July 20 review. Prove the headless path reaches this line with a redacted before/after trace, or move the repair to the actual owner boundary.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against cadad3b7bd12.

Label changes

Label justifications:

  • P1: The proposed fix targets a reported Windows execution regression, but the unresolved main-session/headless path can affect core agent tool output.
  • merge-risk: 🚨 compatibility: The shared helper now overrides explicit Windows detached-spawn requests, which can alter existing or future caller process-lifetime behavior.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes a described Windows Gateway success run, but it does not reproduce the reported headless failure, does not show this sanitizer executing on that path, and is future-dated July 21, 2026 relative to this July 20, 2026 review; provide corrected redacted terminal/log evidence from a before/after Scheduled Task or service run. 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

PR surface:

Source +18, Tests +77. Total +95 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 20 2 +18
Tests 1 80 3 +77
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 100 5 +95

What I checked:

  • Changed behavior: The proposed sanitizer rewrites an explicit detached: true request to false for Windows at the shared helper boundary, including fallback options; this is a behavior change for any present or future caller that intentionally requires detached child lifetime semantics. (src/process/spawn-utils.ts:109, 5e45d16ab177)
  • Untraced failing boundary: The contributor's July 19 follow-up states that spawnWithFallback has one production caller, createChildAdapter, and that caller already passes detached: false on Windows. The new helper guard therefore has not been shown to execute on the reported failing main-session path. (src/process/supervisor/adapters/child.ts, 5e45d16ab177)
  • Prior review continuity: The latest completed ClawSweeper review at the same head SHA already required tracing the actual failing Windows tool path; the new comments still acknowledge that the before-fix headless Scheduled Task/service failure was not reproduced. (src/process/spawn-utils.ts:109, 5e45d16ab177)
  • Proof-date validity: The PR body labels the claimed Gateway run as July 21, 2026, while this review is on July 20, 2026. Until the date is corrected and the redacted original terminal/log evidence is supplied, this future-dated transcript cannot establish an observed after-fix result. (5e45d16ab177)
  • Related report remains canonical: The open report describes intermittent, session-specific Windows exec and read empty output, while this PR's contributor explicitly could not reproduce that behavior in the claimed test setup; the issue remains the canonical record for the unresolved runtime path.

Likely related people:

  • matts524: Reported the current Windows main-session symptom, including the distinction between failing main sessions and working sub-agents, which is necessary to identify the correct runtime boundary. (role: current reproduction reporter; confidence: medium; files: src/process/supervisor/adapters/child.ts, src/process/spawn-utils.ts)
  • itsnimbus: Reported the earlier Scheduled Task regression with non-PTY output capture, providing the closest concrete historical reproduction context for the detached-spawn theory. (role: historical headless-runtime reporter; confidence: low; files: src/process/supervisor/adapters/child.ts, src/process/spawn-utils.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 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.
Review history (2 earlier review cycles)
  • reviewed 2026-07-17T22:08:44.363Z sha a57d44c :: needs real behavior proof before merge. :: [P1] Trace the actual failing Windows spawn path
  • reviewed 2026-07-19T11:44:55.036Z sha 5e45d16 :: needs real behavior proof before merge. :: [P1] Trace the failing Windows tool path before changing the helper

@BradGroux BradGroux mentioned this pull request Jul 18, 2026
Draft
@stantheman0128

stantheman0128 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up (CI + review direction)

Pushed 5e45d16ab1 to the fork branch.

CI

  • Unexported sanitizeSpawnOptionsForPlatform so knip no longer flags an unused export (still applied inside spawnWithFallback).
  • Dropped direct helper tests that hit check-test-types on a readonly stdio tuple; coverage remains on spawnWithFallback (including the Windows stdout capture case).

Caller investigation

spawnWithFallback has one production caller: createChildAdapter in src/process/supervisor/adapters/child.ts. That path already sets useDetached = process.platform !== "win32" && !isServiceManagedRuntime(), so on Windows it already passes detached: false and no detach fallback. The local sanitizer does not change that established exec/supervisor path today; it is only a belt-and-suspenders guard for future callers.

Repro honesty (Windows 11, this machine)

I could not reproduce empty exec/read through a headless Gateway session here (no Scheduled Task / service Gateway setup for the reported path).

Direct Node spawn A/B on this host both returned stdout:

attached  detached=false  code=0  stdout="hello-repro\r\n"
detached  detached=true   code=0  stdout="hello-repro\r\n"

Helper-level spawnWithFallback with intentional detached: true still captures hello-105528 after the guard clears detach. That is not proof the linked Gateway exec/read failure is fixed.

Maintainer ask

Given ClawSweeper’s P1 (wrong shape / missing end-to-end path), I am not hard-selling this patch.

Options I see:

  1. Keep the internal win32 guard as defense-in-depth (current branch), knowing it does not change createChildAdapter today.
  2. Drop the utility override and treat exec/read tools silently return empty output on Windows (v2026.6.x regression) #105528 as still needing a traced Gateway/session boundary (session routing, host=gateway, read path, etc.) with a narrow caller-site fix + live headless proof.

Which do you prefer? If (2), I can close or further narrow this PR once you confirm.

Local evidence

node scripts/run-vitest.mjs src/process/spawn-utils.test.ts
Test Files  1 passed (1)
Tests       5 passed (5)

@stantheman0128

stantheman0128 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: Gateway / OpenClaw main-session after-fix proof

Updated the PR body Evidence with a stronger Windows run (branch tip 5e45d16ab1).

What was captured

  1. Isolated Gateway from this PR tree on Windows 11 / Node 24.15.
  2. Gateway-backed agent turn (runId=17396f8d-6cd7-43f3-9243-76efcdc5bfd8, no CLI embedded fallback). OpenClaw harness used exec + read (2 calls, 0 failures). Visible tool results:
hello-exec-105528
hello-read-105528-from-workspace

Gateway log also showed hello-exec-105528 for that run.
3. Same box: createOpenClawCodingTools with host=gateway also returned those non-empty exec/read payloads.

Honesty

Empty before-fix output was still not reproducible here under interactive/local Gateway. Headless Scheduled Task / service repro was not set up. Direct Node attached vs detached spawn both returned stdout on this host.

@clawsweeper re-review

@stantheman0128
stantheman0128 force-pushed the fix/105528-windows-empty-output branch from 5e45d16 to ffad8b6 Compare July 21, 2026 09:33
@stantheman0128

stantheman0128 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: reproducible Windows empty-before A/B

Rebased onto current main (tip ffad8b656f).

Gold A/B (this Win11 box, Node v24.15.0)

powershell.exe -NoProfile -Command Write-Output hello-ps-105528:

Path detached requested detached passed to Node stdout
BEFORE unsanitized spawn true true empty
AFTER spawnWithFallback true false (forced) hello-ps-105528

cmd.exe echo / plain node stdout still captured even with detached: true here; PowerShell is the clear empty-before signal.

Tests

pnpm exec vitest run src/process/spawn-utils.test.ts Test Files 1 passed (1) Tests 6 passed (6)

Added a Windows-only regression that asserts the BEFORE empty PowerShell case and AFTER restore through spawnWithFallback.

PR Evidence section updated.

@clawsweeper re-review

@stantheman0128
stantheman0128 force-pushed the fix/105528-windows-empty-output branch from ffad8b6 to d1aa923 Compare July 21, 2026 09:39
@stantheman0128

stantheman0128 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Rebased again onto current main (tip now d1aa9239cb). PowerShell empty-before / spawnWithFallback restore A/B still green on this tip:

pnpm exec vitest run src/process/spawn-utils.test.ts -t PowerShell Tests 1 passed | 5 skipped (6)

Prior @clawsweeper re-review request still stands for this tip.

@stantheman0128
stantheman0128 force-pushed the fix/105528-windows-empty-output branch from d1aa923 to 53229f0 Compare July 21, 2026 10:11
@stantheman0128

stantheman0128 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (tip now 53229f0d47). Prior Evidence was on d1aa9239cb; functional commits preserved. @clawsweeper re-review

@stantheman0128

stantheman0128 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Previous re-review run completed successfully but left the review comment stuck on the review started placeholder (lease expired; durable verdict was never written). Evidence on this PR was updated after that attempt; please publish a fresh review against current HEAD.

@clawsweeper

clawsweeper Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

stantheman0128 and others added 3 commits July 23, 2026 02:46
@stantheman0128 @cursoragent
fix(process): force detached false on Windows in spawnWithFallback
4a41eea 
On Windows, detached:true can break stdout/stderr pipe capture for
headless hosts so exec/read tools return empty output with no error.
Centralize the guard in spawnWithFallback so scattered call-site checks
cannot miss a path.

Fixes openclaw#105528

Co-authored-by: Cursor <[email protected]>
@stantheman0128 @cursoragent
fix(process): unexport Windows spawn sanitizer for knip
d6ca50c 
Keep the win32 detached:false guard internal to spawnWithFallback so
knip no longer flags an unused export, and drop direct helper tests that
tripped check-test-types on readonly stdio tuples.

Co-authored-by: Cursor <[email protected]>
@stantheman0128 @cursoragent
test(process): A/B PowerShell empty stdout when detached on Windows
d064287 
Document the reproducible before/after for openclaw#105528: unsanitized
detached:true loses powershell Write-Output stdout; spawnWithFallback
forces detached:false and restores capture.

Co-authored-by: Cursor <[email protected]>
@stantheman0128
stantheman0128 force-pushed the fix/105528-windows-empty-output branch from 53229f0 to d064287 Compare July 22, 2026 18:47
@stantheman0128 stantheman0128 mentioned this pull request Jul 23, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

exec/read tools silently return empty output on Windows (v2026.6.x regression)

1 participant

@stantheman0128