Skip to content

fix(windows): path handling fixes for Windows#6763

Open
pschiel wants to merge 2 commits intoanomalyco:devfrom
pschiel:windows-posix-paths
Open

fix(windows): path handling fixes for Windows#6763
pschiel wants to merge 2 commits intoanomalyco:devfrom
pschiel:windows-posix-paths

Conversation

@pschiel
Copy link
Contributor

@pschiel pschiel commented Jan 3, 2026

fixes #10360
fixes #10872
fixes #11042
fixes #11043
fixes #11044
fixes #11045
fixes #10871
fixes #8924
fixes #8567
fixes #7279
fixes #6020

fixes ALL test cases in the test suite running on win32 system

(most likely plus a series of more issues)

partly fixes #9077
possibly fixes #10798
possibly fixes #10716
possibly fixes #10642

Summary

This fix solves a whole series of bugs, resulting from filepath issues with backslashes occuring on Windows.

Solution: normalize all internal paths to forward slashes using centralized Filesystem wrappers

Why this works: all win32 shells work with forward slash paths, this is de-facto industry standard (git, vscode, cmake, ... all use this pattern)

Observed issues

git rev-parse --show-toplevel returns E:/x/y forward slash format → worktree/directory mismatch
path.resolve(), path.relative(), realpathSync.native(), realpath (bash) break with git bash paths (various issues)
/d/x within C drive (/c/) results in things like C:\d\x
❌ relative paths not working like expected using cross-drive
contains logic not working → issues with permission system and external_directory
/tmp inside git bash is something else outside of it
❌ tools break using backslash paths (some backslashes get "eaten")
❌ escaping hell for agents → requires double/quadruple escaping
❌ observed in all win32 native shells (git bash, cmd, powershell/pwsh)
❌ few issues in app/desktop (wrong filepath splits/usage)
❌ bun hard crashing with segfault when spawn() is used with non-existing directory

Tested with fix

✅ local test suite succeeds now with 100% (bun test in packages/opencode)
✅ all possible path variants (C:\a\b, C:/a/b, /c/a/b, /cygdrive/c/a/b) are normalized into C:/a/b format
✅ full functionality of all tools (ENOENT errors gone, permissions, relative paths, external directory, LSP)
✅ worktree/sandbox handling correct
✅ tested TUI and desktop
✅ bun spawns via bash/pty not segfaulting

How to reproduce/test

  • test suite
  • running prompts with various problematic/mixed filepaths (see attached md example)

Notes

A few more places would require normalization - but they're unused/dead code (should be removed):

  • packages/ui/src/context/sync.tsx line 16 absolute()
  • packages/ui/src/context/local.tsx line 392, 420, 455, 546 - file tree operations (no file tree used)

@pschiel pschiel force-pushed the windows-posix-paths branch 2 times, most recently from 032638f to 61531f5 Compare January 9, 2026 20:17
@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2026

Hey! Your PR title (feat) support MSYS forward slashes with experimental flag doesn't follow conventional commit format.

Please update it to start with one of:

  • feat: or feat(scope): new feature
  • fix: or fix(scope): bug fix
  • docs: or docs(scope): documentation changes
  • chore: or chore(scope): maintenance tasks
  • refactor: or refactor(scope): code refactoring
  • test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

@pschiel pschiel force-pushed the windows-posix-paths branch 3 times, most recently from adf2edd to 9b205c2 Compare January 11, 2026 04:37
@pschiel pschiel changed the title (feat) support MSYS forward slashes with experimental flag feat: support MSYS forward slashes with experimental flag Jan 11, 2026
@github-actions
Copy link
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@pschiel pschiel force-pushed the windows-posix-paths branch from 9b205c2 to bef6a68 Compare January 11, 2026 17:38
@Hona Hona self-requested a review January 11, 2026 20:25
@Hona
Copy link
Member

Hona commented Jan 11, 2026

This sounds perfect @pschiel

If you can get it ready and finish the cleanup you said I can test and approve.

Btw don't put it behind an experimental flag. We'll ship once it's working.

@pschiel pschiel force-pushed the windows-posix-paths branch 4 times, most recently from 767f78b to ba251fb Compare January 15, 2026 21:49
@pschiel pschiel changed the title feat: support MSYS forward slashes with experimental flag fix(windows): comprehensive path handling fixes for MSYS/Git Bash compatibility Jan 15, 2026
@pschiel pschiel changed the title fix(windows): comprehensive path handling fixes for MSYS/Git Bash compatibility fix(windows): path handling fixes for MSYS/Git Bash compatibility Jan 16, 2026
@pschiel pschiel force-pushed the windows-posix-paths branch from ba251fb to aaa0f60 Compare January 17, 2026 03:25
@@ -1 +1 @@
../../ui/src/custom-elements.d.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi this is a symlink.
on windows its disabled by default

enable it with

git config core.symlinks true

then recheckout all the files with

git checkout -- .

obviously that would reset any changes so stash/commit first.
that will fix the typecheck too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was intentional, as this requires admin permissions on windows.

thoughts: changing symlink into export statement in 2 files is worth it for that (the other ~70 symlinks are fonts/images and not affected by TS parsing)

Copy link
Contributor

@neriousy neriousy Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this would've worked keeping as a simlink

/// <reference path="../../ui/src/custom-elements.d.ts" />

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neriousy I went by this https://typescript-eslint.io/rules/triple-slash-reference/ which mentions triple slash being discouraged in modern ES6, and a rule for it.

please feel free to change, it was just so typecheck and tests run without errors, in a win32 environment

@pschiel pschiel force-pushed the windows-posix-paths branch 5 times, most recently from 62152d4 to 30a54ce Compare January 17, 2026 20:51
@pschiel pschiel marked this pull request as ready for review January 17, 2026 21:19
@pschiel pschiel requested a review from adamdotdevin as a code owner January 17, 2026 21:19
@pschiel pschiel changed the title fix(windows): path handling fixes for MSYS/Git Bash compatibility fix(windows): path handling fixes for Windows Jan 17, 2026
@Hona
Copy link
Member

Hona commented Jan 28, 2026

I'll check this out in the next week or so - sorry just fairly busy.
So far functionality wise it looks fantastic.

1 tests failed:
(fail) unicode filenames modification and restore [109.47ms]

I think there's a test failing?

@pschiel
Copy link
Contributor Author

pschiel commented Jan 28, 2026

I'll check this out in the next week or so - sorry just fairly busy. So far functionality wise it looks fantastic.

1 tests failed:
(fail) unicode filenames modification and restore [109.47ms]

I think there's a test failing?

I don't know why these e2e tests are failing. Some timeout. Can you figure that one out what the failing test is testing?

Or is that a new test? What file is that?

Test suite (bun test in packages/opencode) ran fine few hours ago (vs 44 fails on dev)

Screenshot 2026-01-28 215907

@Hona
Copy link
Member

Hona commented Jan 28, 2026

BTW the main things I'll be looking at (if you want to beat me to it)

  • Regex, do we really need it over native path methods?
  • Regex/string replace, if this exists is there a chance it modifies commands e.g. string inputs that should stay as \
  • Pit of success, how can we ensure this doesn't regress
  • Single file exe vs bun dev (no idea if the internal paths are weird)

also just confirming your section 'Observed issues' this PR fixes all those items?

@Hona
Copy link
Member

Hona commented Jan 28, 2026

I don't know why these e2e tests are failing. Some timeout. Can you figure that one out what the failing test is testing?

Ah yep - leave it with me :)

@pschiel
Copy link
Contributor Author

pschiel commented Jan 28, 2026

BTW the main things I'll be looking at (if you want to beat me to it)

* Regex, do we really need it over native path methods?

* Regex/string replace, if this exists is there a chance it modifies commands e.g. string inputs that should stay as \

* Pit of success, how can we ensure this doesn't regress

* Single file exe vs bun dev (no idea if the internal paths are weird)

also just confirming your section 'Observed issues' this PR fixes all those items?

yes, it's absolutely needed to enforce forward slashes - everything else breaks due to path mismatching.

forward slashes because they work on all Windows shells, also the reason why git itself uses this.

problem with nodejs path is that it's for output - even node core normalizes internally.

here are a new node examples that demonstrate the problem:

PS E:\src\ai\opencode\dev> git rev-parse --show-toplevel
E:/src/ai/opencode/dev

this already mismatches, used in worktree and project folder. wildcards/contains/equal checks will fail

PS E:\src\ai\opencode\dev> node -e 'console.log(require("path").relative("/c/Users/ops","e:/src"))'
..\..\..\src

mixed paths will always come up. by native windows commands, and the git bash / msys ones.
they result in completely wrong folders. path.relative doesn't work cross-drive

PS E:\src\ai\opencode\dev> node -e "console.log(require('fs').realpathSync.native('/c/Users/ops'))"
E:\c\Users\ops

realpathSync.native is also broken in this context. it will create weird folders (E:\c\ instead of C:/)

-> we can't use path when it comes to storing paths internally, or when passing them to tools. sometimes even path.join breaks things due to insert \ but the code relies on / in many places (and should be ALL places)

The drive letter conversion (/c/, /mnt/c/, /cygdrive/c/ into C:/ is also a manual thing that needs to be added.

An overview of how other cross-OS compatible apps handle this, everyone does it:

Screenshot_2026-01-20_062937

@pschiel
Copy link
Contributor Author

pschiel commented Jan 28, 2026

Can't reproduce that unicode fail. Ran tests on git bash, powershell and in a Debian container, all passed 🤔

@Hona
Copy link
Member

Hona commented Jan 28, 2026

Yup definitely get the whole need for bash compatible paths at all times, give me a sec to investigate impl details and options

@thdxr thdxr force-pushed the dev branch 3 times, most recently from f1ae801 to 08fa7f7 Compare January 30, 2026 14:37
@pschiel pschiel force-pushed the windows-posix-paths branch 3 times, most recently from 0593e41 to 2e18013 Compare January 31, 2026 23:15
@pschiel
Copy link
Contributor Author

pschiel commented Feb 1, 2026

added fixes to avoid bun hard crashing (segfaulting) when invalid cwd is passed

@pschiel
Copy link
Contributor Author

pschiel commented Feb 2, 2026

issues with added prompt test: BunProc.install() hangs in Windows and runs into the timeout -> added Mock in test
d997822

@pschiel pschiel force-pushed the windows-posix-paths branch from f0285ea to 20bd3e9 Compare February 13, 2026 16:52
@edemaine
Copy link

Just bumped into this and hope it can be finalized! I made some similar changes in #13671 and #13659 but happy to see you got all the way to 0 errors. I think most of #13659 is still relevant for Cygwin support, but I can review it once this gets merged. Anyway, let me know if I can help.

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

Comments