Skip to content

fix(config): use Start-Process -FilePath for Windows config opener#91536

Closed
harjothkhara wants to merge 1 commit into
openclaw:mainfrom
harjothkhara:fix/windows-config-open-filepath
Closed

fix(config): use Start-Process -FilePath for Windows config opener#91536
harjothkhara wants to merge 1 commit into
openclaw:mainfrom
harjothkhara:fix/windows-config-open-filepath

Conversation

@harjothkhara

@harjothkhara harjothkhara commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the Dashboard Open config action on Windows.

The gateway builds the Windows opener command with Start-Process -LiteralPath '<path>'. Start-Process has no -LiteralPath parameter (it exposes -FilePath), so on both Windows PowerShell 5.1 and pwsh 7+ the command fails with a parameter-binding error and the config file never opens.

This changes only the Windows branch of resolveConfigOpenCommand to Start-Process -FilePath '<path>', preserving the existing single-quoted PowerShell escaping (escapePowerShellSingleQuotedString) so the path stays data, not executable shell text. macOS (open) and Linux (xdg-open) branches are untouched.

Fixes #90157

Why this shape

  • Smallest correct fix: 1 prod line + 1 test line; only the Windows token changes.
  • Matches shipped in-repo precedent: core already ships this exact idiom on Windows at src/cli/update-cli/restart-helper.ts:319 (Start-Process -FilePath $launcherPath ...). This patch conforms a broken call site to an idiom the project already relies on in production, rather than introducing a new one.
  • Escaping unchanged: the path remains inside a single-quoted PowerShell literal, so &, $(...), and spaces stay inert. The existing String.raw regression case (C:\tmp\o'hai & calc.json) still exercises quote-doubling + metacharacters.

Relationship to existing PRs

Verification

node scripts/run-vitest.mjs src/gateway/server-methods/config.test.ts
node scripts/run-vitest.mjs ui/src/ui/controllers/config.test.ts
git diff --check

Both test files pass (gateway: 18 tests; UI controller suite green). Local Codex autoreview: clean, no accepted/actionable findings.

Real behavior proof

Behavior addressed: Dashboard "Open config" on Windows calls gateway config.openFile, which built Start-Process -LiteralPath '<openclaw.json>'. Start-Process supports -FilePath for files/documents, not -LiteralPath, so Windows users saw the request fail instead of opening openclaw.json. After this patch the command is Start-Process -FilePath '<escaped path>'.

Real environment tested: Local OpenClaw checkout on macOS (darwin), commit 5b76436c45. Proof exercised the actual resolveConfigOpenCommand() gateway builder, the config.openFile gateway test, and the UI controller test that issues config.openFile. Windows command behavior cross-checked against Microsoft Learn Start-Process docs (PowerShell 7.x and 5.1), which list [-FilePath] <string> and no -LiteralPath parameter, and against shipped in-repo Windows usage at src/cli/update-cli/restart-helper.ts:319.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs src/gateway/server-methods/config.test.ts
node scripts/run-vitest.mjs ui/src/ui/controllers/config.test.ts
git diff --check

Evidence after fix:

resolveConfigOpenCommand(String.raw`C:\tmp\o'hai & calc.json`, "win32") =>
{
  command: "powershell.exe",
  args: [
    "-NoProfile",
    "-NonInteractive",
    "-Command",
    "Start-Process -FilePath 'C:\\tmp\\o''hai & calc.json'"
  ]
}

Gateway config tests:  18 passed (18)
UI controller tests:   green

Observed result after fix: The Windows opener emits powershell.exe -NoProfile -NonInteractive -Command "Start-Process -FilePath '<escaped config path>'", preserving quote escaping while using the supported Start-Process parameter.

What was not tested: I did not run a live Windows Dashboard "Open config" click-through. This is a macOS-only environment with no Windows machine, no Parallels, and no access to a Crabbox/Testbox Windows broker, so live PowerShell execution on Windows is not available to me. The unproven residual is solely the runtime launch on Windows; the emitted command contract is verified by unit tests, Microsoft Start-Process documentation, and shipped in-repo Windows usage of Start-Process -FilePath. Requesting a maintainer with a Windows host (or Testbox) to confirm with the throwaway-file check below before landing:

$f = Join-Path $env:TEMP 'openclaw-proof.json'; 'x' | Out-File $f
try { Start-Process -LiteralPath $f; 'OLD-OK' } catch { 'OLD-ERR: ' + $_.Exception.Message }  # expect parameter-binding error (reproduces #90157)
try { Start-Process -FilePath  $f; 'NEW-OK' } catch { 'NEW-ERR: ' + $_.Exception.Message }     # expect the parameter to bind (fix)

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 9, 2026
@clawsweeper

clawsweeper Bot commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 8, 2026, 9:27 PM ET / 01:27 UTC.

Summary
The PR changes the Windows config opener command from Start-Process -LiteralPath to Start-Process -FilePath and updates the matching gateway regression test.

PR surface: Source 0, Tests 0. Total 0 across 2 files.

Reproducibility: yes. source inspection gives a high-confidence reproduction path: current main routes Dashboard Open config to config.openFile, which emits Start-Process -LiteralPath on Windows, while the PowerShell contract supports -FilePath. I did not run a live Windows click-through in this review.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
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:

  • [P1] Add redacted Windows PowerShell or Dashboard proof showing the old -LiteralPath failure and the patched -FilePath path binding or opening openclaw.json.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR supplies tests, command-shape output, docs, and an explicit note that no live Windows Dashboard or PowerShell run was performed, so it still needs redacted real Windows proof before merge. 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.

Mantis proof suggestion
A short Windows desktop proof would directly settle the remaining live behavior gap for the Dashboard Open config flow. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: on a Windows OpenClaw dashboard, click Open config and verify openclaw.json opens after the FilePath change; redact user paths if captured.

Risk before merge

  • [P1] No live Windows PowerShell or Dashboard click-through proof is attached yet, so the remaining merge blocker is real behavior proof rather than a code defect.

Maintainer options:

  1. Decide the mitigation before merge
    Land this narrow gateway fix after a contributor or maintainer adds redacted Windows proof, then let the linked issue at Dashboard "Open config" fails on Windows: Start-Process -LiteralPath is invalid in all PowerShell versions #90157 close through the merged fix.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The code repair appears complete, but automation cannot supply the contributor's required real Windows behavior proof, so a human or Mantis/Testbox proof path is needed before merge.

Security
Cleared: The diff preserves the existing execFile argument-array invocation and single-quoted PowerShell escaping, and it does not touch dependencies, workflows, permissions, credentials, or package metadata.

Review details

Best possible solution:

Land this narrow gateway fix after a contributor or maintainer adds redacted Windows proof, then let the linked issue at #90157 close through the merged fix.

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

Yes, source inspection gives a high-confidence reproduction path: current main routes Dashboard Open config to config.openFile, which emits Start-Process -LiteralPath on Windows, while the PowerShell contract supports -FilePath. I did not run a live Windows click-through in this review.

Is this the best way to solve the issue?

Yes, the proposed code shape is the best narrow fix: it changes only the unsupported PowerShell parameter, preserves the existing escaping and execFile boundary, and leaves macOS/Linux behavior unchanged. The safer alternative would be a broader opener abstraction, but this bug does not need that added surface.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against c4a0ca0b7a41.

Label changes

Label changes:

  • add P2: The PR fixes a real Windows Dashboard config-opening failure with limited blast radius and a narrow gateway change.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

  • P2: The PR fixes a real Windows Dashboard config-opening failure with limited blast radius and a narrow gateway change.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • 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 supplies tests, command-shape output, docs, and an explicit note that no live Windows Dashboard or PowerShell run was performed, so it still needs redacted real Windows proof before merge. 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 0, Tests 0. Total 0 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 1 1 0
Tests 1 2 2 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 3 3 0

What I checked:

  • Current main still emits the unsupported parameter: resolveConfigOpenCommand() on current main builds the Windows command with Start-Process -LiteralPath, and config.openFile calls that resolver before invoking execFile. (src/gateway/server-methods/config.ts:186, c4a0ca0b7a41)
  • Adjacent tests cover the exact command shape: The current gateway test asserts the Windows command string and would need the expected parameter changed from -LiteralPath to -FilePath, matching the PR diff. (src/gateway/server-methods/config.test.ts:77, c4a0ca0b7a41)
  • UI caller reaches the affected gateway method: The Control UI openConfigFile() controller sends config.openFile, so the Dashboard Open config action depends on the gateway command builder fixed by this PR. (ui/src/ui/controllers/config.ts:524, c4a0ca0b7a41)
  • In-repo Windows precedent uses FilePath: Another shipped Windows PowerShell launcher path in the update restart helper uses Start-Process -FilePath, which supports the PR's command shape. (src/cli/update-cli/restart-helper.ts:319, c4a0ca0b7a41)
  • PowerShell contract supports FilePath for documents: Microsoft Learn's Start-Process page lists [-FilePath] <string> in the syntax and describes opening executable files, scripts, or associated document files; the inspected page did not show -LiteralPath as a Start-Process parameter.
  • Current line provenance: git blame ties the current -LiteralPath command and matching test assertion to commit a54f50a41ba01511a888a9ed5defd4586c4c0656 in this checkout's history. (src/gateway/server-methods/config.ts:186, a54f50a41ba0)

Likely related people:

  • Kevin Lin: git blame for the resolver, test assertion, UI caller, and gateway descriptor points to commit a54f50a41ba01511a888a9ed5defd4586c4c0656 in this checkout, so this is the clearest current-source routing signal. (role: current line provenance; confidence: medium; commits: a54f50a41ba0; files: src/gateway/server-methods/config.ts, src/gateway/server-methods/config.test.ts, ui/src/ui/controllers/config.ts)
  • Peter Steinberger: Recent history shows commit d6e89f96d66c5c84ba968749ffa6f7f3d2965670 refactored the same gateway config handler and test files. (role: recent area contributor; confidence: medium; commits: d6e89f96d66c; files: src/gateway/server-methods/config.ts, src/gateway/server-methods/config.test.ts)
  • Glucksberg: Commit 60661441b1c231115d090607dd18fedb87d9f8ac added gateway config write behavior in the same server-methods module, making this a plausible adjacent routing owner. (role: gateway config feature contributor; confidence: low; commits: 60661441b1c2; files: src/gateway/server-methods/config.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.

@harjothkhara harjothkhara mentioned this pull request Jun 9, 2026
Closed
@harjothkhara harjothkhara marked this pull request as ready for review June 9, 2026 01:18
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. P2 Normal backlog priority with limited blast radius. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 9, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 9, 2026
Open
@vincentkoc vincentkoc self-assigned this Jun 9, 2026
@vincentkoc

vincentkoc commented Jun 9, 2026

Copy link
Copy Markdown
Member

Landed this fix on main by cherry-picking the focused commit, preserving contributor credit:
82afc46

Thanks @harjothkhara. I used the same narrow fix shape from this PR and verified it with:

  • node scripts/run-vitest.mjs src/gateway/server-methods/config.test.ts ui/src/ui/controllers/config.test.ts
  • node scripts/run-oxlint.mjs src/gateway/server-methods/config.ts src/gateway/server-methods/config.test.ts
  • autoreview clean on HEAD
  • Crabbox/Testbox check:changed: blacksmith-testbox tbx_01ktn2rt7t8g6vdsxks1ps6g6r, https://github.com/openclaw/openclaw/actions/runs/27179697098, exit 0

Closing this PR as landed via main.

@vincentkoc vincentkoc closed this Jun 9, 2026
@github-actions github-actions Bot mentioned this pull request Jun 9, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

gateway Gateway runtime P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS 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.

Dashboard "Open config" fails on Windows: Start-Process -LiteralPath is invalid in all PowerShell versions

2 participants

@harjothkhara @vincentkoc