Skip to content

fix(windows): PowerShell completion install and time-format detection#39644

Open
lijinlar wants to merge 3 commits intoopenclaw:mainfrom
lijinlar:fix/windows-powershell-bugs
Open

fix(windows): PowerShell completion install and time-format detection#39644
lijinlar wants to merge 3 commits intoopenclaw:mainfrom
lijinlar:fix/windows-powershell-bugs

Conversation

@lijinlar
Copy link
Copy Markdown

@lijinlar lijinlar commented Mar 8, 2026

Summary

  • Problem: On Windows, openclaw completion --install --shell powershell silently printed "not supported" and exited without writing anything. Additionally, the time-format detection always invoked PS 5.1 (powershell.exe) even when PS7 (pwsh.exe) was installed.
  • Why it matters: PowerShell is the default shell on Windows — completion install was completely broken for the platform, and PS5.1 lacks features and is deprecated on modern Windows.
  • What changed: Added the missing powershell branch in installCompletion(), corrected the source-line syntax from bash source to PS dot-source (. ), and switched detectSystemTimeFormat() to use resolvePowerShellPath() (which already prefers PS7).
  • What did NOT change: No changes to completion script generation, shell detection logic, or any non-Windows code paths.

Change Type

  • Bug fix

Scope

  • UI / DX

Linked Issue/PR

  • Closes # (no existing issue — discovered while using OpenClaw on Windows)

User-visible / Behavior Changes

  • openclaw completion --install --shell powershell now works on Windows: creates/updates Microsoft.PowerShell_profile.ps1 with . "${cachePath}" dot-source syntax.
  • On Windows with PS7 installed, time-format detection now correctly invokes pwsh.exe instead of powershell.exe.

Security Impact

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No — resolvePowerShellPath() was already used throughout shell-utils.ts; this aligns date-time.ts with the existing pattern.
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: Windows 11 (x64), Node v24.7.0
  • Runtime/container: Native Windows (no WSL)
  • Model/provider: N/A
  • Integration/channel: N/A
  • Relevant config: N/A

Steps

  1. Install OpenClaw on Windows with PowerShell 5.1 as default shell
  2. Run openclaw completion --write-state
  3. Run openclaw completion --install --shell powershell

Expected

  • Profile at %USERPROFILE%\Documents\PowerShell\Microsoft.PowerShell_profile.ps1 is created/updated with . "${cachePath}"

Actual (before fix)

  • Prints Automated installation not supported for powershell yet. and exits with no file written

Evidence

  • Failing test/log before + passing after
# Before fix: installCompletion("powershell") hit the else-branch silently
# After fix: 7/7 tests pass

✓ detects pwsh from SHELL env
✓ detects powershell from SHELL env basename
✓ returns zsh as default when SHELL is unset
✓ cache path for powershell uses .ps1 extension
✓ installs PowerShell completion with dot-source syntax
✓ detects completion as installed after writing profile
✓ does not use bash source keyword in PowerShell profile

Test Files  1 passed (1)
Tests       7 passed (7)

Human Verification

  • Verified scenarios: TypeScript typecheck (tsc --noEmit) clean, oxlint 0 errors, oxfmt clean, all 7 new tests pass on Windows.
  • Edge cases checked: Profile already exists (idempotent update), profile directory doesn't exist (auto-created), isCompletionInstalled() correctly detects after install.
  • What I did not verify: Actual runtime execution on macOS/Linux (no code path changes for those platforms); PS7 time-format path requires a Windows machine with pwsh.exe installed.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Failure Recovery

  • How to revert: git revert the single commit; no config changes were made.
  • Known bad symptoms: None expected — the PS branch was entirely missing before, so any regression would be a no-op (same "not supported" behaviour as before).

Risks and Mitigations

  • Risk: Profile path for PowerShell on macOS/Linux (.config/powershell/Microsoft.PowerShell_profile.ps1) — untested on those platforms with pwsh installed.
    • Mitigation: Path logic was already present in getShellProfilePath() and is unchanged; this PR only wires up the call that was missing.

- completion-cli: installCompletion() now handles the 'powershell' shell
  case instead of falling through to the 'not supported' error branch.
- completion-cli: formatCompletionSourceLine() now emits PS dot-source
  syntax (`. "${path}"`) for PowerShell instead of the bash-only
  `source` keyword, which is not a valid PowerShell cmdlet.
- date-time: detectSystemTimeFormat() now calls resolvePowerShellPath()
  instead of the hardcoded `powershell` binary, so PS7 (pwsh.exe) is
  preferred over PS5.1 when available, consistent with shell-utils.ts.

Adds src/cli/completion-cli.powershell.test.ts with 7 tests covering
shell detection, dot-source line format, and end-to-end profile install.
@openclaw-barnacle openclaw-barnacle bot added cli CLI command changes agents Agent runtime and tooling size: S labels Mar 8, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 8, 2026

Greptile Summary

This PR fixes two real Windows-only bugs: PowerShell completion installation was entirely broken (the powershell branch was missing from installCompletion(), causing a silent "not supported" exit), and time-format detection always invoked powershell.exe (PS5.1) even when pwsh.exe (PS7) was available. The fixes are minimal and well-targeted — installCompletion now correctly writes a dot-sourced profile, formatCompletionSourceLine returns . "path" for PowerShell, and detectSystemTimeFormat is brought in line with the rest of the codebase by using the existing resolvePowerShellPath() helper.

Key changes:

  • src/cli/completion-cli.ts: Adds the missing powershell branch to installCompletion() (delegates to getShellProfilePath("powershell"), which already knows the correct Windows vs. POSIX profile path) and to formatCompletionSourceLine() (returns PS dot-source syntax . "${cachePath}" instead of the bash source keyword).
  • src/agents/date-time.ts: Replaces the hardcoded "powershell" string with resolvePowerShellPath() and adds -NoProfile/-NonInteractive flags, consistent with getShellConfig().
  • src/cli/completion-cli.powershell.test.ts: New test file with 7 tests covering shell detection, cache-path extension, profile creation with correct dot-source syntax, idempotent install detection, and absence of the bash source keyword.

Minor issue found: The post-install success message in installCompletion() (line 379) unconditionally prints run: source <profilePath>, which is invalid PowerShell syntax. Interactive (non---yes) PowerShell users will receive incorrect reload instructions — the correct PowerShell form is . "<profilePath>". This does not affect the actual installation but is misleading.

Confidence Score: 4/5

  • Safe to merge; changes are additive and Windows-only, with one minor UX issue in a console message.
  • The fixes are correct and well-scoped. The PowerShell installation branch delegates to the pre-existing getShellProfilePath("powershell") helper, the dot-source syntax is accurate, and the date-time.ts change aligns with the existing resolvePowerShellPath() pattern. Tests cover the new code paths. The only issue is a non-functional UX bug: the post-install message tells PowerShell users to run source <path> (bash syntax) rather than . "<path>" (PowerShell dot-source), but this does not affect the actual installation.
  • Pay close attention to src/cli/completion-cli.ts line 379 — the success message uses bash source syntax which is incorrect for PowerShell users.

Comments Outside Diff (1)

  1. src/cli/completion-cli.ts, line 379 (link)

    Incorrect reload instruction for PowerShell

    The post-install success message unconditionally tells the user to run source <profilePath>, but source is a bash keyword and is not valid in PowerShell. PowerShell users will get a The term 'source' is not recognized error if they follow this instruction.

    The correct PowerShell equivalent is dot-source (. <path>), which is exactly the syntax used in the generated source line above.

    Consider branching on the shell here, or at minimum quoting the path (paths with spaces are common on Windows):

Last reviewed commit: 412bf87

lijinlar added 2 commits March 8, 2026 13:05
Post-install success message was unconditionally printing
`source <profilePath>` which is bash syntax and not valid
in PowerShell. Now branches on the shell type so PowerShell
users see the correct dot-source form: `. "<profilePath>"`.

Addresses greptile review comment on PR openclaw#39644.
After adding the powershell case, all CompletionShell values
(zsh, bash, fish, powershell) are now handled. The else branch
is dead code and oxlint --type-aware correctly flags shell as
never there. Collapse powershell into the final else to satisfy
exhaustiveness while keeping the fallback readable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling cli CLI command changes size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant