fix: use path normalization to fix win32 issues#108
fix: use path normalization to fix win32 issues#108pschiel wants to merge 8 commits intoKilo-Org:devfrom
Conversation
|
@pschiel Can you confirm this was tested, how you tested it and how I can reproduce? |
I fixed only issues that were not working, I reproduced all of the errors. test suite ( other than these, just plenty of usage since ~2 months. backslashes are the biggest problem, relative paths and cross-drive paths. i had a testing command that checked some of these (requires some structure/files setup). when working in a windows env, it's inevitable that mixed paths come from anywhere, all of them should work: |
|
if you require more info / steps to reproduce for a certain tool or so, let me know. some others were issue always same - string matches, contains() checks etc can only work if paths are internally normalized |
|
you might want to look this is a symlink in the repo, which doesn't work in windows, hence the change into a regular file with export. (I noticed a check failing due to this file in your CI) |
|
Interesting that the windows runner tests are failing but passing locally for you |
these are just e2e tests which do a few clicks and that's it - I don't think the unit+integration tests run in a win32 runner. |
|
Since last opencode merge, |
|
e963daa fixes the cause of the test (linux) fails:
-> no providers connect -> model list is empty -> model picker has no list items -> 4 model picker tests fail added a check for these vars and skipping the 4 tests so the pipeline is green - if you fix the credentials issue they should run |
|
the other issue was that the two reason for changing was: symlinks don't work reliably on windows, and vite will fail reading them. changing into regular files with an export works |
|
all unit/integration tests pass again |
General Rebase/Conflict infoentire patch is 1 rule "normalize paths on win32 systems via Filesystem wrapper, where otherwise things will break"
|
b269132 to
70b7679
Compare
Code Review SummaryStatus: No New Issues Found | Recommendation: Merge All previously reported issues have been addressed in subsequent commits:
The Files Reviewed (33 files)
|
70b7679 to
2576353
Compare
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
packages/opencode/src/cli/cmd/tui/routes/session/permission.tsx
Outdated
Show resolved
Hide resolved
|
Windows Unit Tests (not running in pipeline): Failing tests due to 5sec timeout - bun install needs more on Windows. |
|
Hi! Thank you for taking the time to contribute to this project—we really appreciate it. 🙏 We are currently working on re-platforming the core of our VS Code and JetBrains extensions to be based on our new Kilo CLI, with a complete rebuild based on OpenCode as our new foundation, and the moment has come to promote this repository to become the main repository. To do that, we moved the code from this repository to the kilocode repository. This unfortunately means we cannot merge this branch here anymore. Please add https://github.com/Kilo-Org/kilocode.git as a remote, and push your branch there and create a new PR in https://github.com/Kilo-Org/kilocode . We unfortunately cannot do this for you as then the PR would not be in your name anymore. If you need any help, feel free to ask on our Discord in #kilo-dev-contributors Sorry for the inconvenience and thank you for contributing to Kilo! |


Fixes Kilo-Org/kilocode#6332
In reference to anomalyco/opencode#6763
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
FilesystemwrappersWhy 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-toplevelreturnsE:/x/yforward slash format → worktree/directory mismatchpath.resolve(),path.relative(),realpathSync.native(),realpath(bash) break with git bash paths (various issues)/d/xwithin C drive (/c/) results in things likeC:\d\xcontainslogic not working → issues with permission system and external_directory/tmpinside git bash is something else outside of itspawn()is used with non-existing directoryTested with fix
bun testinpackages/opencode)C:\a\b,C:/a/b,/c/a/b,/cygdrive/c/a/b) are normalized intoC:/a/bformatENOENTerrors gone, permissions, relative paths, external directory, LSP)How to reproduce/test
Notes
A few more places would require normalization - but they're unused/dead code (should be removed)
Desktop likely having more issues, especially
bun.spawn()usage