fix(desktop): honor the configured repos directory on Windows - #3547
fix(desktop): honor the configured repos directory on Windows#3547sumit-m wants to merge 2 commits into
Conversation
e3b1adf to
fe76699
Compare
Chessing234
left a comment
There was a problem hiding this comment.
Good catch on the cfg(not(unix)) stub that discarded repos_dir. Honouring the configured directory on Windows so agents and canonical_repos_roots agree closes a real footgun for self-hosters. I’d like a quick note in the PR on how junction/symlink creation is handled when the target already exists or when the process lacks symlink privilege — Windows is picky there, and a clear error beats a silent fall-through to %USERPROFILE%\.buzz\REPOS.
|
Added a Windows link creation: privilege, and an existing Short version: An existing |
ensure_repos_symlink was cfg(unix); the Windows fallback dropped the configured path and always made a real in-nest REPOS, so agents worked somewhere other than the desktop's own git commands. Link via symlink_dir, falling back to a junction so no Developer Mode or elevation is needed. Removal uses remove_dir, which is what Windows needs to unlink a directory link, and target comparison canonicalizes both sides because Windows stores verbatim paths. Un-gates the 21 repos tests that never ran on Windows. Signed-off-by: sumit-m <33051892+sumit-m@users.noreply.github.com>
expandTilde only matched a `~/` prefix, so `~\Documents\docker` fell through to the backend, which rejects any `~` path. Its trailing-separator strip also only handled `/`, doubling the separator on a Windows home. Signed-off-by: sumit-m <33051892+sumit-m@users.noreply.github.com>
fe76699 to
9c98af4
Compare
Summary
Honours the configured repos directory on Windows.
ensure_repos_symlinkwas#[cfg(unix)], and thecfg(not(unix))fallback dropped its argument entirely:So on Windows the community's Repos Directory setting silently did nothing: agents worked in
%USERPROFILE%\.buzz\REPOSwhilecanonical_repos_roots— which is cross-platform — pointed the desktop's own git commands at the configured path. Two halves of the app looking in different places, with no error.The
cfg(unix)gate is gone; one decision tree now runs everywhere, with three small platform helpers inutil.rs:create_dir_link—symlinkon Unix. On Windows it triessymlink_dirfirst, then falls back to a junction viamklink /J. The fallback is what actually carries this:symlink_dirneeds Developer Mode or elevation, and failed on every attempt on my machine. Junctions need neither.symlink_diris still tried first because junctions cannot address UNC targets.remove_dir_link—remove_fileon Unix,remove_diron Windows, which is what Windows requires to unlink a directory link. The previous code calledremove_fileunconditionally.dir_link_points_to— canonicalizes both sides, because Windows stores link targets in verbatim\\?\C:\…form which never compares equal to the caller's path.Every existing safety property is preserved: a non-empty real
REPOSis still refused rather than deleted, an empty one still converts, and re-pointing never touches either target's contents.Second commit:
expandTildeonly matched a~/prefix, so the Windows-native~\Documents\reposfell through to the backend, which rejects any~path with a message suggesting/Users/you/Development. Its trailing-separator strip also only handled/, doubling the separator on a Windows home. Both fixed, with the join logic extracted so it is testable without mockinghomeDir().The forward-slash rewrite is deliberately gated on the home path containing a backslash —
\is not a legal filename character on Windows so rewriting is lossless there, but it is legal on Unix and would corrupt a real filename. There is a test pinning that.Windows link creation: privilege, and an existing
REPOSSymlink privilege
create_dir_linktriesstd::os::windows::fs::symlink_dirfirst, and on failurefalls back to
mklink /J(a junction), which needs neither Developer Mode norelevation. Without that fallback this would fail for most users, since creating a
directory symlink on Windows requires
SeCreateSymbolicLinkPrivilege.The symlink attempt comes first on purpose: junctions only address local
volumes, so trying the junction first would silently break UNC targets.
If both fail, the error carries both causes:
symlink_reposwraps that with the link and target paths, and it propagates outof
ensure_repos_symlinkasErr(String).apply_workspaceemits it on therepos-dir-errorchannel, whichuseNestNotificationsrenders as aRepos directory not appliedtoast with the message as its description — so thefailure is visible in the UI, not just on stderr.
The silent fall-through the old code had is now gone. The deleted
#[cfg(not(unix))]stub ignoredrepos_direntirely and justcreate_dir_all'dthe in-nest
REPOS, which is exactly the "silent fall-through to%USERPROFILE%\.buzz\REPOS" worth avoiding. There is now one implementation forall platforms and no path that discards the configured value without saying so.
When
REPOSalready existsHandled as an explicit match rather than a blind create. Validation
(
validate_repos_dir) runs before any filesystem mutation, so an invalidrepos_dirreturnsErrwithREPOSuntouched.REPOSholds repositories; move or delete them before pointing repos dir elsewhere. Neverremove_dir_all— that would destroy repos the agent cloned in-nestTwo Windows-specific details that the unix code got away with ignoring:
remove_dir_linkusesremove_diron Windows, notremove_file. Windows treats both directory symlinks and junctions asdirectory entries, so
remove_filerefuses them outright;remove_dirunlinks without following. The pre-existing
fs::remove_filecall would havefailed on every re-point.
dir_link_points_tocanonicalizes both sides beforecomparing. Windows stores junction and symlink targets in verbatim
(
\\?\C:\…) form, which never compares equal to the path the caller passedin, so the old
read_link() == targetassertion would have reported "pointssomewhere else" for a link that was in fact correct — and re-created it on
every single apply.
is_symlink()is true for junctions as well as symlinkson Windows, so both link kinds take the same path.
Related issue
Part of #2388 (Windows Support).
No duplicate found: nothing open touches
managed_agents/repos.rsorsrc-tauri/src/util.rs, and the one PR touchingcommunityStorage.ts(#2580) does not touchexpandTilde. Checked by intersecting changed-file paths across all open PRs.Testing
Verified on Windows 11 (
x86_64-pc-windows-msvc):managed_agents::reposwere all#[cfg(unix)]and had never run on Windows. Un-gated, they pass there. That required swapping rawread_link()equality assertions fordir_link_points_toand one directstd::os::unix::fs::symlinkcall.REPOS [\\?\C:\Users\...\repos]underdir /AL, and a file created throughREPOS/appears at the target.communityStoragesuite passes, including three newjoinHomePathcases covering both separator styles and the Unix-backslash-filename edge.cargo clippy --libandcargo fmt --checkclean.Not verified on macOS or Linux. The Unix arms of the new helpers are the original calls unchanged, and the previously-Unix-only tests still pass here, but I have no such hardware to confirm.
Left alone deliberately:
util::create_symlinkis still a silent no-op on non-Unix (util.rs:60), used bynest.rsfor skill and binary links and bymigration.rs. Those are file links with a different failure mode and deserve their own change.