Address PR feedback from mjcheetham and KeithIsSleeping

TryGetWorktreeInfo:
- Walk up from subdirectories to find worktree root
- Canonicalize directory to absolute path
- Require commondir file for valid worktree (return null if missing)
- Add out string error overload; callers fail or warn on IO errors
- Add GitDirPrefix, CommonDirName, SkipCleanCheckName constants

WorktreeCommandParser:
- Handle combined short flags (e.g., -fd, -fb branch, -bfd)
- Separate long/short option handling
- Handle --git-pid/--exit_code as separate-value options
- Document assumptions and note Mono.Options as future improvement

Hooks:
- Write empty marker file instead of "1" for skip-clean-check
- Check unmount exit code; block git on failure unless --force
- Reference PhysicalFileSystem.TryCopyToTempFileAndRename in comment

Other:
- Revert whitespace-only changes in InProcessMountVerb.cs
- New unit tests for subdirectory detection, combined flags, baked-in values

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Tyrie Vella

GVFS/GVFS.Common/GVFSConstants.cs

@@ -146,6 +146,9 @@ public static class DotGit
         {
             public const string Root = ".git";
             public const string HeadName = "HEAD";
+            public const string GitDirPrefix = "gitdir: ";
+            public const string CommonDirName = "commondir";
+            public const string SkipCleanCheckName = "skip-clean-check";
             public const string IndexName = "index";
             public const string PackedRefsName = "packed-refs";
             public const string LockExtension = ".lock";

GVFS/GVFS.Common/GVFSEnlistment.Shared.cs

@@ -57,34 +57,81 @@ public static string GetWorktreePipeSuffix(string directory)
         }
 
         /// <summary>
-        /// Detects if the given directory is a git worktree. If so, returns
-        /// a WorktreeInfo with the worktree name, git dir path, and shared
-        /// git dir path. Returns null if not a worktree.
+        /// Detects if the given directory (or any ancestor) is a git worktree.
+        /// Walks up from <paramref name="directory"/> looking for a <c>.git</c>
+        /// file (not directory) containing a <c>gitdir:</c> pointer. Returns
+        /// null if not inside a worktree.
         /// </summary>
         public static WorktreeInfo TryGetWorktreeInfo(string directory)
         {
-            string dotGitPath = Path.Combine(directory, ".git");
+            return TryGetWorktreeInfo(directory, out _);
+        }
 
-            if (!File.Exists(dotGitPath) || Directory.Exists(dotGitPath))
+        /// <summary>
+        /// Detects if the given directory (or any ancestor) is a git worktree.
+        /// Walks up from <paramref name="directory"/> looking for a <c>.git</c>
+        /// file (not directory) containing a <c>gitdir:</c> pointer. Returns
+        /// null if not inside a worktree, with an error message if an I/O
+        /// error prevented detection.
+        /// </summary>
+        public static WorktreeInfo TryGetWorktreeInfo(string directory, out string error)
+        {
+            error = null;
+
+            if (string.IsNullOrEmpty(directory))
             {
                 return null;
             }
 
+            // Canonicalize to an absolute path so walk-up and Path.Combine
+            // behave consistently regardless of the caller's CWD.
+            string current = Path.GetFullPath(directory);
+            while (current != null)
+            {
+                string dotGitPath = Path.Combine(current, ".git");
+
+                if (Directory.Exists(dotGitPath))
+                {
+                    // Found a real .git directory — this is a primary worktree, not a linked worktree
+                    return null;
+                }
+
+                if (File.Exists(dotGitPath))
+                {
+                    return TryParseWorktreeGitFile(current, dotGitPath, out error);
+                }
+
+                string parent = Path.GetDirectoryName(current);
+                if (parent == current)
+                {
+                    break;
+                }
+
+                current = parent;
+            }
+
+            return null;
+        }
+
+        private static WorktreeInfo TryParseWorktreeGitFile(string worktreeRoot, string dotGitPath, out string error)
+        {
+            error = null;
+
             try
             {
                 string gitdirLine = File.ReadAllText(dotGitPath).Trim();
-                if (!gitdirLine.StartsWith("gitdir: "))
+                if (!gitdirLine.StartsWith(GVFSConstants.DotGit.GitDirPrefix))
                 {
                     return null;
                 }
 
-                string gitdirPath = gitdirLine.Substring("gitdir: ".Length).Trim();
+                string gitdirPath = gitdirLine.Substring(GVFSConstants.DotGit.GitDirPrefix.Length).Trim();
                 gitdirPath = gitdirPath.Replace('/', Path.DirectorySeparatorChar);
 
                 // Resolve relative paths against the worktree directory
                 if (!Path.IsPathRooted(gitdirPath))
                 {
-                    gitdirPath = Path.GetFullPath(Path.Combine(directory, gitdirPath));
+                    gitdirPath = Path.GetFullPath(Path.Combine(worktreeRoot, gitdirPath));
                 }
 
                 string worktreeName = Path.GetFileName(gitdirPath);
@@ -93,31 +140,34 @@ public static WorktreeInfo TryGetWorktreeInfo(string directory)
                     return null;
                 }
 
-                // Read commondir to find the shared .git/ directory
-                // commondir file contains a relative path like "../../.."
-                string commondirFile = Path.Combine(gitdirPath, "commondir");
-                string sharedGitDir = null;
-                if (File.Exists(commondirFile))
+                // Read commondir to find the shared .git/ directory.
+                // All valid worktrees must have a commondir file.
+                string commondirFile = Path.Combine(gitdirPath, GVFSConstants.DotGit.CommonDirName);
+                if (!File.Exists(commondirFile))
                 {
-                    string commondirContent = File.ReadAllText(commondirFile).Trim();
-                    sharedGitDir = Path.GetFullPath(Path.Combine(gitdirPath, commondirContent));
+                    return null;
                 }
 
+                string commondirContent = File.ReadAllText(commondirFile).Trim();
+                string sharedGitDir = Path.GetFullPath(Path.Combine(gitdirPath, commondirContent));
+
                 return new WorktreeInfo
                 {
                     Name = worktreeName,
-                    WorktreePath = directory,
+                    WorktreePath = worktreeRoot,
                     WorktreeGitDir = gitdirPath,
                     SharedGitDir = sharedGitDir,
                     PipeSuffix = "_WT_" + worktreeName.ToUpper(),
                 };
             }
-            catch (IOException)
+            catch (IOException e)
             {
+                error = e.Message;
                 return null;
             }
-            catch (UnauthorizedAccessException)
+            catch (UnauthorizedAccessException e)
             {
+                error = e.Message;
                 return null;
             }
         }

GVFS/GVFS.Common/GVFSEnlistment.cs

@@ -138,7 +138,13 @@ public static GVFSEnlistment CreateFromDirectory(
                 // Always check for worktree first. A worktree directory may
                 // be under the enlistment tree, so TryGetGVFSEnlistmentRoot
                 // can succeed by walking up — but we need a worktree enlistment.
-                WorktreeInfo wtInfo = TryGetWorktreeInfo(directory);
+                string worktreeError;
+                WorktreeInfo wtInfo = TryGetWorktreeInfo(directory, out worktreeError);
+                if (worktreeError != null)
+                {
+                    throw new InvalidRepoException($"Failed to check worktree status for '{directory}': {worktreeError}");
+                }
+
                 if (wtInfo?.SharedGitDir != null)
                 {
                     string primaryRoot = wtInfo.GetEnlistmentRoot();

GVFS/GVFS.Common/WorktreeCommandParser.cs

@@ -6,9 +6,20 @@ namespace GVFS.Common
     /// <summary>
     /// Parses git worktree command arguments from hook args arrays.
     /// Hook args format: [hooktype, "worktree", subcommand, options..., positional args..., --git-pid=N, --exit_code=N]
+    ///
+    /// Assumptions:
+    /// - Args are passed by git exactly as the user typed them (no normalization).
+    /// - --git-pid and --exit_code are always appended by git in =value form.
+    /// - Single-letter flags may be combined (e.g., -fd for --force --detach).
+    /// - -b/-B always consume the next arg as a branch name, even when combined (e.g., -fb branch).
+    ///
+    /// Future improvement: consider replacing with a POSIX-compatible arg parser
+    /// library (e.g., Mono.Options, MIT license) to handle edge cases more robustly.
     /// </summary>
     public static class WorktreeCommandParser
     {
+        private static readonly HashSet<char> ShortOptionsWithValue = new HashSet<char> { 'b', 'B' };
+
         /// <summary>
         /// Gets the worktree subcommand (add, remove, move, list, etc.) from hook args.
         /// </summary>
@@ -17,7 +28,7 @@ public static string GetSubcommand(string[] args)
             // args[0] = hook type, args[1] = "worktree", args[2+] = subcommand and its args
             for (int i = 2; i < args.Length; i++)
             {
-                if (!args[i].StartsWith("--"))
+                if (!args[i].StartsWith("-"))
                 {
                     return args[i].ToLowerInvariant();
                 }
@@ -36,18 +47,24 @@ public static string GetSubcommand(string[] args)
         /// <param name="positionalIndex">0-based index of the positional arg after the subcommand</param>
         public static string GetPositionalArg(string[] args, int positionalIndex)
         {
-            var optionsWithValue = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
+            var longOptionsWithValue = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
             {
-                "-b", "-B", "--reason"
+                "--reason"
             };
 
             int found = -1;
             bool pastSubcommand = false;
             bool pastSeparator = false;
             for (int i = 2; i < args.Length; i++)
             {
-                if (args[i].StartsWith("--git-pid=") || args[i].StartsWith("--exit_code="))
+                if (args[i].StartsWith("--git-pid") || args[i].StartsWith("--exit_code"))
                 {
+                    // Always =value form, but skip either way
+                    if (!args[i].Contains("=") && i + 1 < args.Length)
+                    {
+                        i++;
+                    }
+
                     continue;
                 }
 
@@ -57,9 +74,40 @@ public static string GetPositionalArg(string[] args, int positionalIndex)
                     continue;
                 }
 
-                if (!pastSeparator && args[i].StartsWith("-"))
+                if (!pastSeparator && args[i].StartsWith("--"))
+                {
+                    // Long option — check if it takes a separate value
+                    if (longOptionsWithValue.Contains(args[i]) && i + 1 < args.Length)
+                    {
+                        i++;
+                    }
+
+                    continue;
+                }
+
+                if (!pastSeparator && args[i].StartsWith("-") && args[i].Length > 1)
                 {
-                    if (optionsWithValue.Contains(args[i]) && i + 1 < args.Length)
+                    // Short option(s), possibly combined (e.g., -fd, -fb branch).
+                    // A value-taking letter consumes the rest of the arg as its value.
+                    // Only consume the next arg if the first value-taking letter is
+                    // the last character (no baked-in value).
+                    // e.g., -bfd → b="fd" (baked), -fdb val → f,d booleans, b="val"
+                    //        -Bb → B="b" (baked), -fBb → f boolean, B="b" (baked)
+                    string flags = args[i].Substring(1);
+                    bool consumesNextArg = false;
+                    for (int j = 0; j < flags.Length; j++)
+                    {
+                        if (ShortOptionsWithValue.Contains(flags[j]))
+                        {
+                            // This letter takes a value. If it's the last letter,
+                            // the value is the next arg. Otherwise the value is the
+                            // remaining characters (baked in) and we're done.
+                            consumesNextArg = (j == flags.Length - 1);
+                            break;
+                        }
+                    }
+
+                    if (consumesNextArg && i + 1 < args.Length)
                     {
                         i++;
                     }

GVFS/GVFS.Hooks/Program.Worktree.cs

@@ -76,7 +76,12 @@ private static void UnmountWorktreeByArg(string[] args)
             }
 
             string fullPath = ResolvePath(worktreePath);
-            UnmountWorktree(fullPath);
+            if (!UnmountWorktree(fullPath))
+            {
+                Console.Error.WriteLine(
+                    $"error: failed to unmount worktree '{fullPath}'. Cannot proceed with move.");
+                Environment.Exit(1);
+            }
         }
 
         /// <summary>
@@ -118,7 +123,7 @@ private static void CleanupSkipCleanCheckMarker(string[] args)
             GVFSEnlistment.WorktreeInfo wtInfo = GVFSEnlistment.TryGetWorktreeInfo(fullPath);
             if (wtInfo != null)
             {
-                string markerPath = Path.Combine(wtInfo.WorktreeGitDir, "skip-clean-check");
+                string markerPath = Path.Combine(wtInfo.WorktreeGitDir, GVFSConstants.DotGit.SkipCleanCheckName);
                 if (File.Exists(markerPath))
                 {
                     File.Delete(markerPath);
@@ -222,32 +227,40 @@ private static void HandleWorktreeRemove(string[] args)
             // Write a marker in the worktree gitdir that tells git.exe
             // to skip the cleanliness check during worktree remove.
             // We already did our own check above while ProjFS was alive.
-            string skipCleanCheck = Path.Combine(wtInfo.WorktreeGitDir, "skip-clean-check");
-            File.WriteAllText(skipCleanCheck, "1");
+            string skipCleanCheck = Path.Combine(wtInfo.WorktreeGitDir, GVFSConstants.DotGit.SkipCleanCheckName);
+            File.WriteAllText(skipCleanCheck, string.Empty);
 
             // Unmount ProjFS before git deletes the worktree directory.
-            UnmountWorktree(fullPath, wtInfo);
+            if (!UnmountWorktree(fullPath, wtInfo) && !hasForce)
+            {
+                Console.Error.WriteLine(
+                    $"error: failed to unmount worktree '{fullPath}'.\n" +
+                    $"Use 'git worktree remove --force' to attempt removal anyway.");
+                Environment.Exit(1);
+            }
         }
 
-        private static void UnmountWorktree(string fullPath)
+        private static bool UnmountWorktree(string fullPath)
         {
             GVFSEnlistment.WorktreeInfo wtInfo = GVFSEnlistment.TryGetWorktreeInfo(fullPath);
             if (wtInfo == null)
             {
-                return;
+                return false;
             }
 
-            UnmountWorktree(fullPath, wtInfo);
+            return UnmountWorktree(fullPath, wtInfo);
         }
 
-        private static void UnmountWorktree(string fullPath, GVFSEnlistment.WorktreeInfo wtInfo)
+        private static bool UnmountWorktree(string fullPath, GVFSEnlistment.WorktreeInfo wtInfo)
         {
-            ProcessHelper.Run("gvfs", $"unmount \"{fullPath}\"", redirectOutput: false);
+            ProcessResult result = ProcessHelper.Run("gvfs", $"unmount \"{fullPath}\"", redirectOutput: false);
 
             // After gvfs unmount exits, ProjFS handles may still be closing.
             // Wait briefly to allow the OS to release all handles before git
             // attempts to delete the worktree directory.
             System.Threading.Thread.Sleep(200);
+
+            return result.ExitCode == 0;
         }
 
         private static void MountNewWorktree(string[] args)
@@ -264,7 +277,12 @@ private static void MountNewWorktree(string[] args)
             string dotGitFile = Path.Combine(fullPath, ".git");
             if (File.Exists(dotGitFile))
             {
-                GVFSEnlistment.WorktreeInfo wtInfo = GVFSEnlistment.TryGetWorktreeInfo(fullPath);
+                string worktreeError;
+                GVFSEnlistment.WorktreeInfo wtInfo = GVFSEnlistment.TryGetWorktreeInfo(fullPath, out worktreeError);
+                if (worktreeError != null)
+                {
+                    Console.Error.WriteLine($"warning: failed to read worktree info for '{fullPath}': {worktreeError}");
+                }
 
                 // Store the primary enlistment root so mount/unmount can find
                 // it without deriving from path structure assumptions.
@@ -291,6 +309,9 @@ private static void MountNewWorktree(string[] args)
                         // The primary index may be updated concurrently by the
                         // running mount; a direct copy risks a torn read on
                         // large indexes (200MB+ in some large repos).
+                        // Note: mirrors PhysicalFileSystem.TryCopyToTempFileAndRename
+                        // but that method requires GVFSPlatform which is not
+                        // available in the hooks process.
                         string tempIndex = worktreeIndex + ".tmp";
                         try
                         {

GVFS/GVFS.Mount/InProcessMountVerb.cs

@@ -57,11 +57,11 @@ public InProcessMountVerb()
             HelpText = "Service initiated mount.")]
         public string StartedByService { get; set; }
 
-        [Option(
-            'b',
-            GVFSConstants.VerbParameters.Mount.StartedByVerb,
-            Default = false,
-            Required = false,
+        [Option(
+            'b',
+            GVFSConstants.VerbParameters.Mount.StartedByVerb,
+            Default = false,
+            Required = false,
             HelpText = "Verb initiated mount.")]
         public bool StartedByVerb { get; set; }
 

GVFS/GVFS.Service/GVFSMountProcess.cs

@@ -36,7 +36,14 @@ public bool MountRepository(string repoRoot, int sessionId)
 
                 string errorMessage;
                 string pipeName = GVFSPlatform.Instance.GetNamedPipeName(repoRoot);
-                GVFSEnlistment.WorktreeInfo wtInfo = GVFSEnlistment.TryGetWorktreeInfo(repoRoot);
+                string worktreeError;
+                GVFSEnlistment.WorktreeInfo wtInfo = GVFSEnlistment.TryGetWorktreeInfo(repoRoot, out worktreeError);
+                if (worktreeError != null)
+                {
+                    this.tracer.RelatedError($"Failed to check worktree status for '{repoRoot}': {worktreeError}");
+                    return false;
+                }
+
                 if (wtInfo?.SharedGitDir != null)
                 {
                     string enlistmentRoot = wtInfo.GetEnlistmentRoot();