fix(process): resolve Windows commands when env aliases are blank by LZY3538 · Pull Request #111716 · openclaw/openclaw

LZY3538 · 2026-07-20T07:27:33Z

What Problem This Solves

Fixes an issue where Windows process and agent launch paths could fail to resolve an executable when an injected environment contained a blank PATH or PATHEXT entry alongside a usable differently-cased alias.

Why This Change Was Made

Windows environment keys are case-insensitive, including arbitrary mixed-case spellings. Resolution now scans all own entries whose keys case-fold to PATH or PATHEXT, chooses the first nonblank value, and preserves the existing deterministic preference for PATH/PATHEXT, then Path/Pathext, before other spellings. If all matching entries are blank, the explicit empty search configuration remains authoritative instead of falling back to the host process.

This PR is AI-assisted. I reviewed the full resolver, its callers, sibling Windows command and executable-path implementations, adjacent tests, and the official Node.js process.env Windows contract.

User Impact

Windows commands and shims remain discoverable for every environment-key casing, including injected plain objects with mixed-case aliases, without overriding callers that intentionally provide an empty search path.

Evidence

Original base: 46ceb21b895d58ca6268dee84de2fc5e7c12dd5b

Exact head: a14aa44fc01b4e023914d1f449a35f2948c2bb1f

Original regression proof:

node scripts/run-vitest.mjs src/plugin-sdk/windows-spawn.test.ts
Expected: C:\...\tool.CMD
Received: "tool"

Review regression proof against the previous PR head with pAtH and pAtHeXt:

Test Files  1 failed (1)
Tests       1 failed | 7 passed (8)
Expected: C:\...\tool.CMD
Received: "tool"

Focused tests after the fix, covering arbitrary-case PATH, arbitrary-case PATHEXT, canonical duplicate precedence, and explicit-empty behavior:

node scripts/run-vitest.mjs src/plugin-sdk/windows-spawn.test.ts
Test Files  1 passed (1)
Tests       10 passed (10)

Sibling Windows command baseline before adding the new regression:

node scripts/run-vitest.mjs src/process/windows-command.test.ts
Test Files  1 passed (1)
Tests       11 passed (11)

Real Windows production-path probe using arbitrary mixed-case aliases:

{"platform":"win32","resolved":"tool.CMD","matches":true}
cleanup=removed

Official runtime contract: Node.js documents that environment variables are case-insensitive on Windows: https://nodejs.org/api/process.html#processenv

Current-head clean-install CI proof:

Windows Node tests: passed (checks-windows-node-test).
Core executable-path regression shard: passed (checks-node-compact-small-11), including the new resolveSafeChildProcessInvocation mixed-case PATH/PATHEXT regression and the existing non-Windows Path contract.
Production and test type checks: passed (check-prod-types, check-test-types).
Plugin SDK API baseline and package contract: passed.
Lint: passed.

The overall CI gate is currently red only because test/scripts/pr-wrappers.test.ts fails in checks-node-compact-small-1; that file and its wrapper behavior are outside this PR's five-file diff. The same isolated failure reproduced on two successive PR heads, while the current main CI run is green. The contributor account cannot rerun upstream jobs.

Formatting and whitespace validation:

G:\AI??\openclaw\node_modules\.bin\oxfmt.CMD --check src/plugin-sdk/windows-spawn.ts src/plugin-sdk/windows-spawn.test.ts
All matched files use the correct format.

git diff --check
(no output)

The first revised CI head exposed TS18048 in the new entry collector. The implementation now collects narrowed [string, string] tuples explicitly. A fresh local core tsgo run reports no error in windows-spawn.ts; it stops later on two unrelated missing exports from the linked worktree's stale shared @openclaw/fs-safe/advanced dependency. GitHub CI on this exact head is the authoritative clean-install typecheck.

The linked-worktree changed-check classifier attempted to delegate its broader lane but could not start because pnpm tried to reconcile the linked node_modules and stopped with ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY. No dependency installation or purge was performed.

clawsweeper · 2026-07-20T08:48:53Z

Codex review: needs maintainer review before merge. Reviewed July 20, 2026, 9:42 PM ET / July 21, 2026, 01:42 UTC.

Summary
The PR changes Windows executable lookup to select nonblank case-insensitive PATH and PATHEXT aliases while retaining explicitly empty caller-supplied values as authoritative.

PR surface: Source +47, Tests +74. Total +121 across 5 files.

Reproducibility: yes. source-reproducible with high confidence: provide a Windows-style environment containing blank PATH or PATHEXT plus a usable differently cased alias, then resolve a command with an extension supplied by that alias. The PR body records that failure on the earlier implementation and a passing post-fix Windows probe.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/plugin-sdk/windows-spawn.test.ts, serialized state: src/process/windows-command.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

[P2] Refresh the branch against current main and validate the exact merge result before landing.

Risk before merge

[P1] The branch is behind current main and several required checks are still running, so the supplied proof does not yet establish the exact merge-head result.

Maintainer options:

Decide the mitigation before merge
Rebase or update the branch onto current main, retain the focused alias and explicit-empty regression cases, and land only after the resulting Windows and process-execution checks validate the merge head.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No mechanical defect remains from this review; a maintainer should refresh the behind branch and assess the completed checks on the exact merge result.

Security
Cleared: The diff changes local environment-key selection for executable lookup and adds tests; it introduces no dependency, workflow, permission, secret, download, or new code-execution surface.

Review details

Best possible solution:

Rebase or update the branch onto current main, retain the focused alias and explicit-empty regression cases, and land only after the resulting Windows and process-execution checks validate the merge head.

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

Yes, source-reproducible with high confidence: provide a Windows-style environment containing blank PATH or PATHEXT plus a usable differently cased alias, then resolve a command with an extension supplied by that alias. The PR body records that failure on the earlier implementation and a passing post-fix Windows probe.

Is this the best way to solve the issue?

Yes. Sharing the core Windows environment lookup removes the divergent first-match behavior from the safe child-process path, while the SDK keeps its owner-local fallback semantics without adding configuration or changing intentionally empty search paths.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a bounded Windows command-resolution defect with limited blast radius and a focused repair.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes an after-fix real Windows production-path probe resolving a command through arbitrary-cased aliases, alongside focused regression results; no sensitive data is shown.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes an after-fix real Windows production-path probe resolving a command through arbitrary-cased aliases, alongside focused regression results; no sensitive data is shown.

Evidence reviewed

PR surface:

Source +47, Tests +74. Total +121 across 5 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	67	20	+47
Tests	2	75	1	+74
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	5	142	21	+121

What I checked:

Current-main gap: The pre-PR resolver only checked the exact key and the Path spelling before falling back, so injected objects containing blank canonical entries plus usable arbitrary-cased aliases could fail to locate a command. (src/infra/executable-path.ts:11, 97130060dfdd)
Shared command-path fix: The PR centralizes Windows alias selection in resolveWindowsEnvironmentValue, preferring nonblank aliases while returning the first blank match when every matching alias is blank. (src/infra/executable-path.ts:11, 10c77befa978)
Sibling resolver coverage: The plugin-SDK Windows executable resolver applies the same configured-environment-first semantics and does not fall back to the host environment when an explicit blank matching alias exists. (src/plugin-sdk/windows-spawn.ts:96, 10c77befa978)
Core caller uses shared behavior: The Windows safe-child-process path now imports the shared resolver rather than retaining a separate first-match case-insensitive implementation. (src/process/windows-command.ts:9, 10c77befa978)
Regression coverage and runtime proof: The PR body supplies focused tests for arbitrary-case aliases, canonical precedence, and explicit-empty handling, plus a real Windows production-path probe resolving tool.CMD. (src/plugin-sdk/windows-spawn.test.ts:20, 10c77befa978)

Likely related people:

LZY3538: The only concrete ownership signal available in the reviewed item is the current-head implementation commit covering both Windows resolver boundaries; current-main feature-history attribution could not be independently established from the available read-only inspection results. (role: current implementation contributor; confidence: low; commits: 10c77befa978; files: src/infra/executable-path.ts, src/plugin-sdk/windows-spawn.ts, src/process/windows-command.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 (4 earlier review cycles)

reviewed 2026-07-20T07:31:21.571Z sha 941a94d :: needs changes before merge. :: [P2] Resolve every Windows environment-key case variant
reviewed 2026-07-20T09:10:44.504Z sha 9397107 :: needs changes before merge. :: [P2] Update the safe child-process resolver
reviewed 2026-07-20T13:02:12.780Z sha a14aa44 :: needs real behavior proof before merge. :: [P1] Normalize duplicate aliases before spawning the child
reviewed 2026-07-20T13:44:24.157Z sha a14aa44 :: needs maintainer review before merge. :: none

openclaw-barnacle Bot added the size: S label Jul 20, 2026

LZY3538 force-pushed the codex/windows-spawn-blank-env-aliases branch 2 times, most recently from e5ad373 to 9397107 Compare July 20, 2026 09:06

BradGroux mentioned this pull request Jul 20, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

LZY3538 force-pushed the codex/windows-spawn-blank-env-aliases branch 2 times, most recently from 2c081cc to a14aa44 Compare July 20, 2026 12:57

fix(process): honor nonblank Windows env aliases

10c77be

LZY3538 force-pushed the codex/windows-spawn-blank-env-aliases branch from a14aa44 to 10c77be Compare July 21, 2026 01:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(process): resolve Windows commands when env aliases are blank#111716

fix(process): resolve Windows commands when env aliases are blank#111716
LZY3538 wants to merge 1 commit into
openclaw:mainfrom
LZY3538:codex/windows-spawn-blank-env-aliases

LZY3538 commented Jul 20, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

LZY3538 commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LZY3538 commented Jul 20, 2026 •

edited

Loading

clawsweeper Bot commented Jul 20, 2026 •

edited

Loading