Fix: Windows terminal encoding set to UTF-8#46992
Fix: Windows terminal encoding set to UTF-8#46992slc03 wants to merge 3 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes inconsistent terminal encoding on Windows by prepending a short PowerShell UTF-8 initialization block ( Key observations:
Confidence Score: 4/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/shell-utils.test.ts
Line: 217-231
Comment:
**Test only covers wrapping logic on Windows**
On non-Windows CI, this test passes `"pwsh"` as the shell, but `isWindowsPowerShellShell` returns `false` immediately when `process.platform !== "win32"`. The function returns the command unchanged, and the test just asserts `wrapped === "Write-Output 'ok'"`. The actual UTF-8 initialization path (all five lines of setup code) is never exercised outside a Windows runner.
Consider splitting this into a platform-agnostic test that verifies the *output shape* of the wrapper directly (bypassing the `process.platform` guard), rather than relying on the test runner being on Windows to exercise that branch. For example, extracting a pure helper that builds the wrapped string and unit-testing it independently would let non-Windows CI catch regressions in the wrapper content.
As it stands, the test named *"wraps command with UTF-8 initialization on Windows PowerShell"* only truly tests that description on Windows; on all other platforms it silently tests the no-op path.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/agents/shell-utils.ts
Line: 90-97
Comment:
**Trailing `#`-comment in user command could silently drop setup lines**
PowerShell treats `#` as a line comment — everything from `#` to the next newline is ignored. When all setup statements and the user command are joined with `"; "` into a single `-Command` string, there are no embedded newlines, so a user command that ends with a `#` comment (e.g. `Write-Output 'hello' # greet`) will cause the `#` to span to the *end of the entire joined string*. Because the user command is the last element in the join this is safe today, but it is a subtle invariant that must be maintained if the order of elements ever changes.
A more defensive approach would be to append the user command on its own line (using `\n` as the separator before it) rather than a semicolon, which would contain any trailing comment to the user's line:
```ts
return [
"$__openclawUtf8 = [System.Text.UTF8Encoding]::new($false)",
"[Console]::InputEncoding = $__openclawUtf8",
"[Console]::OutputEncoding = $__openclawUtf8",
"$OutputEncoding = $__openclawUtf8",
"try { chcp 65001 > $null } catch {}",
].join("; ") + "\n" + command;
```
This makes the comment-safety property explicit and structural rather than reliant on ordering.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: 97ab01e |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("wraps command with UTF-8 initialization on Windows PowerShell", () => { | ||
| const shell = process.platform === "win32" ? "powershell.exe" : "pwsh"; | ||
| const wrapped = wrapCommandWithWindowsPowerShellUtf8("Write-Output 'ok'", shell); | ||
| const shouldWrap = process.platform === "win32"; | ||
|
|
||
| if (shouldWrap) { | ||
| expect(wrapped).toContain("[Console]::OutputEncoding = $__openclawUtf8"); | ||
| expect(wrapped).toContain("$OutputEncoding = $__openclawUtf8"); | ||
| expect(wrapped).toContain("chcp 65001 > $null"); | ||
| expect(wrapped).toContain("Write-Output 'ok'"); | ||
| return; | ||
| } | ||
|
|
||
| expect(wrapped).toBe("Write-Output 'ok'"); | ||
| }); |
There was a problem hiding this comment.
Test only covers wrapping logic on Windows
On non-Windows CI, this test passes "pwsh" as the shell, but isWindowsPowerShellShell returns false immediately when process.platform !== "win32". The function returns the command unchanged, and the test just asserts wrapped === "Write-Output 'ok'". The actual UTF-8 initialization path (all five lines of setup code) is never exercised outside a Windows runner.
Consider splitting this into a platform-agnostic test that verifies the output shape of the wrapper directly (bypassing the process.platform guard), rather than relying on the test runner being on Windows to exercise that branch. For example, extracting a pure helper that builds the wrapped string and unit-testing it independently would let non-Windows CI catch regressions in the wrapper content.
As it stands, the test named "wraps command with UTF-8 initialization on Windows PowerShell" only truly tests that description on Windows; on all other platforms it silently tests the no-op path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/shell-utils.test.ts
Line: 217-231
Comment:
**Test only covers wrapping logic on Windows**
On non-Windows CI, this test passes `"pwsh"` as the shell, but `isWindowsPowerShellShell` returns `false` immediately when `process.platform !== "win32"`. The function returns the command unchanged, and the test just asserts `wrapped === "Write-Output 'ok'"`. The actual UTF-8 initialization path (all five lines of setup code) is never exercised outside a Windows runner.
Consider splitting this into a platform-agnostic test that verifies the *output shape* of the wrapper directly (bypassing the `process.platform` guard), rather than relying on the test runner being on Windows to exercise that branch. For example, extracting a pure helper that builds the wrapped string and unit-testing it independently would let non-Windows CI catch regressions in the wrapper content.
As it stands, the test named *"wraps command with UTF-8 initialization on Windows PowerShell"* only truly tests that description on Windows; on all other platforms it silently tests the no-op path.
How can I resolve this? If you propose a fix, please make it concise.| return [ | ||
| "$__openclawUtf8 = [System.Text.UTF8Encoding]::new($false)", | ||
| "[Console]::InputEncoding = $__openclawUtf8", | ||
| "[Console]::OutputEncoding = $__openclawUtf8", | ||
| "$OutputEncoding = $__openclawUtf8", | ||
| "try { chcp 65001 > $null } catch {}", | ||
| command, | ||
| ].join("; "); |
There was a problem hiding this comment.
Trailing #-comment in user command could silently drop setup lines
PowerShell treats # as a line comment — everything from # to the next newline is ignored. When all setup statements and the user command are joined with "; " into a single -Command string, there are no embedded newlines, so a user command that ends with a # comment (e.g. Write-Output 'hello' # greet) will cause the # to span to the end of the entire joined string. Because the user command is the last element in the join this is safe today, but it is a subtle invariant that must be maintained if the order of elements ever changes.
A more defensive approach would be to append the user command on its own line (using \n as the separator before it) rather than a semicolon, which would contain any trailing comment to the user's line:
return [
"$__openclawUtf8 = [System.Text.UTF8Encoding]::new($false)",
"[Console]::InputEncoding = $__openclawUtf8",
"[Console]::OutputEncoding = $__openclawUtf8",
"$OutputEncoding = $__openclawUtf8",
"try { chcp 65001 > $null } catch {}",
].join("; ") + "\n" + command;This makes the comment-safety property explicit and structural rather than reliant on ordering.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/shell-utils.ts
Line: 90-97
Comment:
**Trailing `#`-comment in user command could silently drop setup lines**
PowerShell treats `#` as a line comment — everything from `#` to the next newline is ignored. When all setup statements and the user command are joined with `"; "` into a single `-Command` string, there are no embedded newlines, so a user command that ends with a `#` comment (e.g. `Write-Output 'hello' # greet`) will cause the `#` to span to the *end of the entire joined string*. Because the user command is the last element in the join this is safe today, but it is a subtle invariant that must be maintained if the order of elements ever changes.
A more defensive approach would be to append the user command on its own line (using `\n` as the separator before it) rather than a semicolon, which would contain any trailing comment to the user's line:
```ts
return [
"$__openclawUtf8 = [System.Text.UTF8Encoding]::new($false)",
"[Console]::InputEncoding = $__openclawUtf8",
"[Console]::OutputEncoding = $__openclawUtf8",
"$OutputEncoding = $__openclawUtf8",
"try { chcp 65001 > $null } catch {}",
].join("; ") + "\n" + command;
```
This makes the comment-safety property explicit and structural rather than reliant on ordering.
How can I resolve this? If you propose a fix, please make it concise.…claw#46994) fix: Windows terminal encoding set to UTF-8 (openclaw#46992)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa954184e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "try { chcp 65001 > $null } catch {}", | ||
| command, | ||
| ].join("; "); |
There was a problem hiding this comment.
Prepending the UTF-8 setup statements changes parsing rules for commands that must begin at the first statement in a PowerShell script context (for example param(...) blocks or #requires directives). With this wrapper, those commands are no longer first and can fail on Windows even though they worked before, so exec behavior is no longer command-semantic-preserving for a real class of scripts.
Useful? React with 👍 / 👎.
|
I have an alternate solution: #52154 |
2 participants
Title
Fix: Windows terminal encoding set to UTF-8
Summary
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
Security Impact (required)
Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Failure Recovery (if this breaks)
Risks and Mitigations
Implementation note: This change was implemented with GitHub Copilot (GPT-5.3-Codex).