Skip to content

fix(ssh): read the Windows bridge registry as UTF-8 - #3853

Open
tunatuna1002-lab wants to merge 1 commit into
devswha:mainfrom
tunatuna1002-lab:fix/windows-registry-utf8
Open

tunatuna1002-lab wants to merge 1 commit into
devswha:mainfrom
tunatuna1002-lab:fix/windows-registry-utf8

Conversation

@tunatuna1002-lab

Copy link
Copy Markdown

Fixes #3852

Change

On a native Windows PC whose user profile has non-ASCII characters (for example C:\Users\홍길동), the bridge started but the server never found it, so Connect and every bridge update ended in "Bridge did not start" with nothing in bridge.log.

The bridge writes its registry files as UTF-8 without a BOM. Windows PowerShell 5.1 reads such a file in the ANSI code page, so socket_path came back changed (on a Korean PC, code page 949, the line no longer parsed at all) and descriptors() dropped the new bridge.

  • server/remote-host.ts: both registry reads (WINDOWS_DESCRIPTORS and the clean-up in windowsHost.stop) now use Get-Content -Raw -Encoding UTF8. -Encoding UTF8 is accepted by Windows PowerShell 5.1 and PowerShell 7. The other Get-Content -Raw in this file reads the install lock owner, a sha256 hex string, so it is left as is.
  • server/windows-native.test.ts: the registry fixture's socket_path is now C:\Users\홍길동\herdr.sock, so the existing launcher/registry test covers a non-ASCII profile for descriptors(), inspect() and stop().
  • changelog.d/windows-registry-non-ascii-profile.md: a Fixed entry.

Validation

  • bun run check fast on macOS (Apple silicon, Bun 1.4.4): types, themes, typecheck and build are ok. test:unit failed in 27 files (CI build cache, browser/iOS workflow scripts, relay timing and similar). They look tied to my machine, not this change: the same 27 files on unmodified main (70e9ed7) gave the same result, 1069 pass and 97 fail, and the same failing tests apart from one timing test in the loopback relay, which failed on each side with a different case.
  • bun test scripts/changelog-fold.test.ts server/windows-native.test.ts server/machines.test.ts on macOS: 104 pass, 16 skip (the Windows-only cases), 0 fail.
  • The changed Windows test only runs on Windows, and I have no Windows checkout to run it in. Instead I ran its registry steps on a Windows 11 PC (Windows PowerShell 5.1.26100, ANSI code page 949). I wrote the same fixture as BOM-less UTF-8 into a temporary %USERPROFILE%\.config\herdr-web-ui\bridges and ran the two registry scripts through powershell -EncodedCommand with UTF-8 output, as runPowerShell does:
    • before this change: the descriptors() read did not return the fixture, and the stop() clean-up left the file;
    • with this change: the fixture came back unchanged, and stop() removed the file.
  • On my own v0.5.3 server with this change, the affected PC connected through the normal setup flow: the server started a new bridge, found its descriptor and connected.
  • Not run: bun run check full and bun run test:ssh. I expect the Windows CI runners (code page 1252) to fail the updated test without the fix, because the UTF-8 bytes of the fixture decode to other characters there; I could not check this myself.

🤖 Generated with Claude Code

The bridge writes its registry files as UTF-8 without a BOM, and
Windows PowerShell 5.1 reads such a file in the ANSI code page. On a PC
whose profile path has non-ASCII characters the descriptor's
socket_path no longer matched (or the line no longer parsed), so a
started bridge was never found and Connect ended in "Bridge did not
start". Read both registry listings with -Encoding UTF8.

Fixes devswha#3852

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

All contributors have signed the CLA. Thank you!
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Bug Fixes
    • Fixed SSH connection issues on Windows when the user-profile path contains non-ASCII characters. Bridge discovery and connection or bridge updates now work more reliably in these cases.

Windows bridge descriptor reads now specify UTF-8 in both listing and stop-time PID checks. The test fixture uses a socket path containing a non-ASCII username. A changelog entry describes the reported connection issue.

Changes: Windows bridge descriptor handling

Layer / File(s) Summary
UTF-8 descriptor reads and regression fixture
server/remote-host.ts, server/windows-native.test.ts, changelog.d/windows-registry-non-ascii-profile.md
Both registry JSON reads specify UTF-8. The fixture uses a non-ASCII socket path, and the changelog records the reported issue.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: yeongjo, devswha

Merge Risk: Minimal · up to 54c02

This change fixes bridge discovery and cleanup on Windows PCs whose user-profile path contains non-ASCII characters. The change is small, localized and easy to revert. No merge-blocking risk was found.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title "fix(ssh): read the Windows bridge registry as UTF-8" names the main change: the two Windows bridge registry reads now decode UTF-8 explicitly. It matches the changeset, which adds `-Encodin…
Description check Passed The description follows the template. It has a Change section that describes the problem (non-ASCII Windows profile paths), the cause (PowerShell 5.1 reading BOM-less UTF-8 as ANSI), and the fix with …
Linked Issues check Passed Issue #3852 requires adding -Encoding UTF8 to both bridge-registry reads in server/remote-host.ts. The reviewed head does this. WINDOWS_DESCRIPTORS now uses `Get-Content -LiteralPath $_.FullName -…
Out of Scope Changes check Passed All changes connect to #3852. The remote-host.ts edits are the two requested registry reads, and the comments document the reason for the change. The test change updates the registry fixture to a non-…
Full details: Title check

Explanation

The title "fix(ssh): read the Windows bridge registry as UTF-8" names the main change: the two Windows bridge registry reads now decode UTF-8 explicitly. It matches the changeset, which adds -Encoding UTF8 to the registry reads in server/remote-host.ts, updates the test fixture, and adds a changelog entry.

Full details: Description check

Explanation

The description follows the template. It has a Change section that describes the problem (non-ASCII Windows profile paths), the cause (PowerShell 5.1 reading BOM-less UTF-8 as ANSI), and the fix with affected files. It has a Validation section listing the checks run (bun run check fast, targeted bun test, manual Windows 11 registry tests) and the checks not run (check full, test:ssh). It links issue #3852. The template's HTTP/WS contract-test note does not apply, since no HTTP/WS changes are made.

Full details: Linked Issues check

Explanation

Issue #3852 requires adding -Encoding UTF8 to both bridge-registry reads in server/remote-host.ts. The reviewed head does this. WINDOWS_DESCRIPTORS now uses Get-Content -LiteralPath $_.FullName -Raw -Encoding UTF8, which covers the descriptor listing read (issue L264). The windowsHost.stop clean-up now uses Get-Content -LiteralPath $_.FullName -Raw -Encoding UTF8 | ConvertFrom-Json (issue L391). The issue's stated cause and fix match the diff. The install-lock owner read is unchanged, as the issue does not request a change there. server/windows-native.test.ts now uses a non-ASCII socket_path (C:\Users\홍길동\herdr.sock) in the registry fixture, which covers the descriptor read path. That test runs only on Windows. The author manually ran the registry scripts on Windows 11 with code page 949 and reports the expected results. Automated Windows test execution was not available to this assessment, so this check relies on the source diff and the author's reported manual verification.

Full details: Out of Scope Changes check

Explanation

All changes connect to #3852. The remote-host.ts edits are the two requested registry reads, and the comments document the reason for the change. The test change updates the registry fixture to a non-ASCII profile path, which is the test coverage for the fix. The changelog entry in changelog.d/windows-registry-non-ascii-profile.md documents the fix. No unrelated code paths changed in the reviewed diffs for remote-host.ts and windows-native.test.ts.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: devswha/ovis
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: 70e9ed7
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 86fdb1ad-9b3a-4852-b31f-b4b1ec3d4761
Commits

Reviewing files that changed from the base of the PR and between 70e9ed7 and 54c02ab.

Files selected for processing: 3

  • changelog.d/windows-registry-non-ascii-profile.md
  • server/remote-host.ts
  • server/windows-native.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

@tunatuna1002-lab

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Nov 14, 2026
@devswha

devswha commented Nov 14, 2026

Copy link
Copy Markdown
Owner

Thank you, @tunatuna1002-lab. The merge pass reviewed this at 54c02ab with two reviewers under the stricter bar for outside PRs; both are clear, and nothing in the supply chain changes (the existing remote PowerShell scripts gain -Encoding UTF8; no new command). It does not merge yet for one reason that is not about the code: a release freeze is on, and nothing touching server/ merges until the next Release run is done. It merges then, with its Windows checks green.

One LOW, for you to take or leave: the stop() half of the fix has no test that fails without it; the existing stop assertion checks removal by PID, which passes with the old read as long as the JSON stays parseable.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows PC with a non-ASCII profile never connects: "Bridge did not start" (bridge registry read in the ANSI code page)

2 participants