fix(windows): normalize global config path construction#7834
fix(windows): normalize global config path construction#7834sainzs wants to merge 1 commit intoKilo-Org:mainfrom
Conversation
|
Verification completed locally:
Notes:
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| import { Log } from "@/util/log" | ||
| import { Wildcard } from "@/util/wildcard" | ||
| import os from "os" | ||
| import { drainCovered } from "@/kilocode/permission/drain" // kilocode_change |
There was a problem hiding this comment.
CRITICAL: New imports target files that do not exist
Both @/kilocode/permission/drain and @/kilocode/permission/config-paths are imported here, but there is no packages/opencode/src/kilocode/permission/ directory in this branch. permission/next.ts will fail module resolution as soon as it is typechecked.
| s.approved.push(...newRules) | ||
|
|
||
| if (newRules.length > 0) { | ||
| await Config.updateGlobal({ permission: toConfig(newRules) }, { dispose: false }) |
There was a problem hiding this comment.
CRITICAL: Config.updateGlobal is called with the wrong arity
packages/opencode/src/config/config.ts:1481 defines updateGlobal(config: Info) with no overload, so this new second { dispose: false } argument produces TS2554. The identical call a few lines lower has the same problem.
|
|
||
| export function expandHomePattern(pattern: string, home = resolveHome(), pathmod: PathModule = path) { | ||
| if (pattern === "~" || pattern === "$HOME") return home | ||
| if (pattern.startsWith("~/")) return pathmod.join(home, pattern.slice(2)) |
There was a problem hiding this comment.
WARNING: Windows-style home patterns stop matching here
This helper only expands ~/... and $HOME/.... A Windows config written with backslashes (~\agent\rules.md or $HOME\agent\rules.md) now falls through unchanged, so the permission rule will never match the real path on Windows.
| return path.join(xdg, "kilo") | ||
| } | ||
|
|
||
| export class MarketplacePaths { |
There was a problem hiding this comment.
WARNING: This fix is not wired into the extension
MarketplacePaths is only defined in this new file and is not imported anywhere under packages/kilo-vscode/src. As written, the extension never uses the normalized global config root added here, so the Windows marketplace-path bug remains unchanged.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)Resolved since last review:
Issues found outside the current PR diff that cannot receive inline comments:
Files Reviewed (6 files)
Reviewed by gpt-5.4-20260305 · 1,686,119 tokens |
sainzs
force-pushed
the
fix/windows-xdg-config-paths
branch
from
8b4421a to
a50a990
Compare
1 participant
Context
Fixes
#7789.Windows users reported global config paths missing the separator before
.config, for exampleC:\Users\user1.config\kilo\.... The repo also had home-pattern expansion in permissions implemented with string concatenation, which can produce incorrect mixed-separator paths on Windows.Implementation
Added a small pure path helper in
packages/opencode/src/global/paths.tsand used it to:xdg-basedirvalues inpackages/opencode/src/global/index.ts~/...and$HOME/...paths withpath.join(...)inpackages/opencode/src/permission/next.tspackages/kilo-vscode/src/services/marketplace/paths.tsAlso added Windows-path tests using
path.win32so the behavior is validated without requiring a Windows runner.Screenshots
.configC:\Users\user1\.config\kiloin testsHow to Test
bun test --cwd packages/opencode test/global/paths.test.ts.C:\Users\user1\.config\kilo.~/agent/rules.mdand$HOME/agent/rules.mdexpand with Windows separators underpath.win32.Get in Touch
Discord: add your handle here if you want maintainer follow-up there.