git worktree support for VFSForGit#1911
Conversation
tyrielv
force-pushed
the
tyrielv/gvfs-worktree-2
branch
2 times, most recently
from
2ff5914 to
ab088d8
Compare
Add TryGetWorktreeInfo() to detect git worktrees by checking for a .git file (not directory) and reading its gitdir pointer. WorktreeInfo carries the worktree name, paths, and derived pipe suffix. Add GVFSEnlistment.CreateForWorktree() factory that constructs an enlistment with worktree-specific paths: WorkingDirectoryRoot points to the worktree, DotGitRoot uses the shared .git directory, and NamedPipeName includes a worktree-specific suffix. Add WorktreeCommandParser to extract subcommands and positional args from git worktree hook arguments. Add GVFS_SUPPORTS_WORKTREES to GitCoreGVFSFlags enum. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update GetGVFSPipeName() in common.windows.cpp to detect when running inside a git worktree. If the current directory contains a .git file (not directory), read the gitdir pointer, extract the worktree name, and append a _WT_<NAME> suffix to the pipe name. This single change makes all native hooks (read-object, post-index-changed, virtual-filesystem) connect to the correct worktree-specific GVFS mount process. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MountVerb: detect worktree paths via TryGetWorktreeInfo(), create worktree-specific GVFSEnlistment, check worktree-specific pipe for already-mounted state, register worktrees by their own path (not the primary enlistment root). UnmountVerb: resolve worktree pipe name for unmount, unregister by worktree path so the primary enlistment registration is not affected. InProcessMount: bootstrap worktree metadata (.gvfs/ inside worktree gitdir), set absolute paths for core.hookspath and core.virtualfilesystem, skip hook installation for worktree mounts (hooks are shared via hookspath), set GVFS_SUPPORTS_WORKTREES bit. GitIndexProjection/FileSystemCallbacks: use worktree-specific index path instead of assuming primary .git/index. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tyrielv
force-pushed
the
tyrielv/gvfs-worktree-2
branch
3 times, most recently
from
153f071 to
07dbad0
Compare
In the managed pre/post-command hooks, intercept git worktree
subcommands to transparently manage GVFS mounts:
add: Post-command runs 'git checkout -f' to create the index,
then 'gvfs mount' to start ProjFS projection.
remove: Pre-command checks for uncommitted changes while ProjFS
is alive, writes skip-clean-check marker, unmounts.
Post-command remounts if removal failed (dir + .git exist).
move: Pre-command unmounts old path, post-command mounts new.
prune: Post-command cleans stale worktree metadata.
Add WorktreeCommandParser reference to GVFS.Hooks.csproj.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Unit tests: WorktreeInfoTests — TryGetWorktreeInfo detection, pipe suffix WorktreeEnlistmentTests — CreateForWorktree path mappings WorktreeCommandParserTests — subcommand and arg extraction Functional tests: WorktreeTests — end-to-end add/list/remove with live GVFS mount GitBlockCommandsTests — update existing test for conditional block Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tyrielv
force-pushed
the
tyrielv/gvfs-worktree-2
branch
from
07dbad0 to
e4314b3
Compare
ProjFS cannot handle nested virtualization roots. Add a pre-command check that blocks 'git worktree add' when the target path is inside the primary enlistment's working directory. Add IsPathInsideDirectory() utility to GVFSEnlistment.Shared.cs with unit tests for path matching (case-insensitive, sibling paths allowed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Before removing a worktree, probe the named pipe to verify the GVFS mount is running. If not mounted: - Without --force: error with guidance to mount or use --force - With --force: skip unmount and let git proceed Refactor UnmountWorktree to accept a pre-resolved WorktreeInfo to avoid redundant TryGetWorktreeInfo calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Normalize paths in IsPathInsideDirectory using Path.GetFullPath to prevent traversal attacks with segments like '/../'. Add GetKnownWorktreePaths to enumerate existing worktrees from the .git/worktrees directory, and block creating a worktree inside any existing worktree — not just the primary VFS working directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
KeithIsSleeping
left a comment
KeithIsSleeping
left a comment
There was a problem hiding this comment.
Risk Analysis Review
Reviewed from an OS developer perspective, focusing on GVFS + worktree integration risks, ProjFS concerns, test coverage, and deployment safety.
Assumption: GVFS is only used in the OS repo, so OS-specific layout assumptions (e.g., src/ subdirectory) are acceptable.
Summary: 3 critical issues, 4 high-risk issues, several medium concerns and test gaps. See inline comments for details.
|
|
||
| string fullPath = ResolvePath(worktreePath); | ||
| UnmountWorktree(fullPath); | ||
| } |
There was a problem hiding this comment.
CRITICAL: Race condition during index copy
File.Copy(primaryIndex, worktreeIndex) copies the primary enlistment’s index while the primary mount is live and active. If post-index-changed fires or git is updating the primary’s index concurrently, the worktree receives a corrupted/partial index. A corrupted index under ProjFS means wrong files get projected — silently.
On the OS repo this is a ~200MB index file; the window for a torn read is significant.
Recommendation: Copy to a temp file first and rename atomically, or use FileShare.Read with retry logic. Even better: have the primary mount’s named pipe serve a consistent index snapshot.
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
CRITICAL: RepoMetadata global singleton double-init
RepoMetadata.Instance is a process-global singleton with no thread safety. This code initializes it with the primary’s metadata dir, reads values, shuts down, then re-initializes with the worktree’s metadata dir. If anything throws between TryInitialize and Shutdown, the singleton is left pointing at the wrong metadata directory for all subsequent callers in this process.
While each GVFS.Mount is its own process today, this is a fragile pattern — and the TryInitialize contract explicitly throws InvalidOperationException if called with a different path while already initialized.
Recommendation: Extract the cache path reads into a separate helper that doesn’t use the singleton (e.g., directly read the key-value store file), or at minimum wrap in try/finally to guarantee Shutdown() is called.
| // Checks if the given directory is a git worktree by looking for a | ||
| // ".git" file (not directory). If found, reads it to extract the | ||
| // worktree name and returns a pipe name suffix like "_WT_NAME". | ||
| // Returns an empty string if not in a worktree. |
There was a problem hiding this comment.
HIGH: MAX_PATH buffer overflow risk in native hooks
wchar_t dotGitPath[MAX_PATH] — wcscpy_s with a MAX_PATH buffer will invoke the invalid parameter handler (which crashes the process by default) if directory exceeds 260 chars. The existing codebase already has TODO 640838: Support paths longer than MAX_PATH.
Worktree paths are more likely to be long, especially with agent-generated names. A crash in the hook process mid-git-operation can leave the repo in a corrupted state.
Additionally, char gitdirLine[MAX_PATH * 2] can truncate the .git file content, extracting the wrong worktree name and connecting to the wrong named pipe.
Recommendation: Use std::wstring with dynamic allocation, or at minimum use PATHCCH_MAX_CCH (32767) buffers with proper error handling.
| return; | ||
| } | ||
|
|
||
| string fullPath = ResolvePath(worktreePath); |
There was a problem hiding this comment.
HIGH: Hardcoded "src" path assumption
This hardcodes the OS repo’s directory layout. While we’re assuming OS-only usage, this is still fragile — if the nesting check silently uses the wrong path, a worktree could be created inside the ProjFS virtualization root, causing ProjFS to crash or corrupt.
Recommendation: Read the actual working directory root from the primary mount’s metadata or config rather than hardcoding. At minimum, add a comment explicitly documenting this OS-repo assumption so future maintainers know it’s intentional.
| string srcDir = Path.GetDirectoryName(wtInfo.SharedGitDir); | ||
| if (srcDir != null) | ||
| { | ||
| string primaryRoot = Path.GetDirectoryName(srcDir); |
There was a problem hiding this comment.
HIGH: Fragile enlistment root derivation via parent directory traversal
Path.GetDirectoryName(Path.GetDirectoryName(wtInfo.SharedGitDir)) assumes the structure is always <enlistmentRoot>/src/.git. This same pattern is repeated in MountVerb.cs, UnmountVerb.cs, and GVFSMountProcess.cs (4+ locations).
If symlinks/junctions are involved, or if SharedGitDir has trailing separators, GetDirectoryName can return unexpected results.
Recommendation: Store the primary enlistment root explicitly during worktree creation (e.g., write it to a file in .git/worktrees/<name>/gvfs-primary-root). This eliminates the path structure assumption entirely and is resilient to junctions/symlinks.
| try | ||
| { | ||
| // Kill any stuck GVFS.Mount for this worktree | ||
| foreach (Process p in Process.GetProcessesByName("GVFS.Mount")) |
There was a problem hiding this comment.
TEST GAP: ForceCleanupWorktree won’t actually kill stuck mounts
Process.StartInfo.Arguments is only populated when the process was started by the current process. For externally-launched GVFS.Mount processes, StartInfo.Arguments is always null or empty. This cleanup code will never match and never kill stuck mounts.
Recommendation: Use Process.GetProcessById with command line query via WMI (Win32_Process), or use the named pipe to send an unmount command.
| { | ||
| private const string WorktreeBranch = "worktree-test-branch"; | ||
|
|
||
| [TestCase] |
There was a problem hiding this comment.
TEST GAP: Missing concurrent worktree tests
The functional tests only exercise a single worktree at a time. For the VS Code Copilot agent scenario (the primary motivator), multiple agents creating worktrees simultaneously is the expected usage pattern. Missing tests:
- Two worktrees created/used simultaneously
- Operations in one worktree while another is mounting/unmounting
git worktree prunewith active mounts- Machine reboot /
--mount-allwith registered worktrees gvfs service --unmount-all/--mount-allcycle with worktrees
This is especially important given the shared object cache, shared git config, and shared service registration.
| /// </summary> | ||
| /// <param name="args">Full hook args array (hooktype, command, subcommand, ...)</param> | ||
| /// <param name="positionalIndex">0-based index of the positional arg after the subcommand</param> | ||
| public static string GetPositionalArg(string[] args, int positionalIndex) |
There was a problem hiding this comment.
MINOR: Missing --orphan in options-with-value set
git worktree add --orphan <branch> <path> — the --orphan flag takes a branch name as its value. Since it’s not in the optionsWithValue set, the branch name would be misidentified as a positional argument, and the actual path arg would be returned as the wrong positional index.
| } | ||
|
|
||
| public static bool WaitUntilMounted(ITracer tracer, string enlistmentRoot, bool unattended, out string errorMessage) | ||
| public static bool WaitUntilMounted(ITracer tracer, string pipeName, string enlistmentRoot, bool unattended, out string errorMessage) |
There was a problem hiding this comment.
MEDIUM: Breaking API change to WaitUntilMounted
The signature changed from (ITracer, string enlistmentRoot, bool, out string) to (ITracer, string pipeName, string enlistmentRoot, bool, out string). This is a public static method — any out-of-tree callers (OS build system tools, deployment scripts, other mount orchestrators) will break at compile time.
Recommendation: Consider adding the new overload alongside the old one (calling into shared logic) to maintain backward compatibility, or verify there are no external callers.
| if (string.IsNullOrEmpty(worktreeName)) | ||
| { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
MINOR: Bare catch swallows all exceptions silently
TryGetWorktreeInfo catches all exceptions and returns null. This means file permission errors, disk I/O errors, and even OutOfMemoryException are silently swallowed. During mount operations, this could mask the real cause of a failure — the mount would fail with a confusing "not a worktree" error instead of the actual I/O error.
Recommendation: At minimum, catch IOException and UnauthorizedAccessException specifically. Consider logging the exception even if you still return null.
3 participants
git worktreesupport for VFSForGitMotivation
VS Code Copilot Chat background agents use
git worktreeto create isolated working directories for code editing. On VFSForGit enlistments,git worktreeis blocked byBLOCK_ON_VFS_ENABLEDbecause the VFS layer had no support for multiple working trees. This PR enables worktree support by running an independent GVFS mount (ProjFS instance) per worktree.Companion PR
microsoft/git: tyrielv/gvfs-worktree (1 commit, 4 files, +82 lines)
GVFS_SUPPORTS_WORKTREES(1<<8) to thecore.gvfsbitmaskgit worktreewhen the bit is set--no-checkoutwhen VFS is active (GVFS handles checkout after mount)skip-clean-checkmarker for pre-unmount cleanliness verificationt0402-block-command-on-gvfs.shDesign
Each worktree gets its own:
GVFS_<ROOT>_WT_<NAME>) for IPC with hooks.git/worktrees/<name>/.gvfs/Worktrees share:
core.hookspath(absolute path).git/configvia git'scommondirmechanismHow it works
git worktree add <path>→ git creates the worktree structure with--no-checkout(forced by the git-side change)worktree add→ runsgit checkout -fto create the index → runsgvfs mount <path>.gitfile →gitdir:→.git/worktrees/<name>/) → creates worktree-specificGVFSEnlistment→ starts ProjFS → files appear_WT_<NAME>pipe suffixgit worktree remove→ pre-command hook unmounts ProjFS → git deletes the directoryCommits (review in order)
TryGetWorktreeInfo()detection logic,CreateForWorktree()path mappings.gitfile detection inGetGVFSPipeName(), pipe suffix derivationMountVerb/UnmountVerbworktree detection, service registration by worktree path,InProcessMountmetadata bootstrap, index path fixesskip-clean-checkflow, remount-on-failure recovery,WorktreeCommandParserarg extractionTesting done
git worktree add— auto-mounts, files visible via ProjFSgit worktree list— shows primary + worktreesgit worktree move— unmounts old, mounts newgit worktree remove— unmounts, cleans upgit worktree prune— cleans stale entriesgvfs service --unmount-all/--mount-all— worktrees survive the cyclet0402) — 18/18 passKnown limitations
worktree removepartially succeeds (ProjFS unmounted but directory deletion fails due to locked file), the worktree is left in a degraded state. The post-command hook attempts remount only if both the directory and.gitfile still exist.