Keep a user's submodule and signing config out of worktree prep

src/Capacitor.Cli.Daemon/Services/WorktreeManager.cs

@@ -279,7 +279,7 @@ await WithWorktreeMetadataGate(repoPath, time, () =>
                 // --no-auto-maintenance: that flag needs a newer git than the override transport does.
                 await RunGit(repoPath, FetchTimeout,
                     time,
-                    [new("maintenance.auto", "false"), new("gc.auto", "0")],
+                    [new("maintenance.auto", "false"), new("gc.auto", "0"), .. NoSubmoduleRecursion()],
                     "fetch", "origin", $"{baseRef}:{fetchedRef}");
                 await WithWorktreeMetadataGate(repoPath, time, () =>
                     RunGit(repoPath, GitTimeout, time, noHooks,
@@ -435,29 +435,35 @@ async Task<WorktreeInfo> BuildStandaloneSnapshotAsync(string repoPath, string wo
         StripWorkspaceMcpConfig(worktreePath);
         var noHooks = NoBranchHooks();
         await RunGit(worktreePath, GitTimeout, time, noHooks, "init");
+        var userAttributes = await UserAttributesFileAsync(worktreePath, time);
+        var verbatim       = VerbatimLineEndings(CreateSnapshotAttributesFile(worktreePath, userAttributes));
+        // Kept in the snapshot's own config too, so the agent's later diffs compare the bytes that were copied.
+        foreach (var setting in verbatim)
+            await RunGit(worktreePath, GitTimeout, time, noHooks, "config", setting.Key, setting.Value);
         // Inventoried here: `add -A` is where this snapshot's filters resolve.
         var standaloneOverrides = await FilterOverridesForAsync(worktreePath);
-        await RunGit(worktreePath, GitTimeout, time, [.. noHooks, .. standaloneOverrides], "add", "-A");
+        await RunGit(worktreePath, GitTimeout, time, sourceReadOnly: false, [.. noHooks, .. verbatim, .. standaloneOverrides],
+            NoSystemAttributes, "add", "-A");
         // Exit 0 = empty index (greenfield: nothing to copy). Exit 1 = staged content.
         // Any other code is a real git failure — without this check, commit --allow-empty would
         // hide it behind a successful empty tree.
         var staged = await RunGitCaptureResult(
             worktreePath, GitTimeout, time, sourceReadOnly: false,
-            [.. noHooks, .. standaloneOverrides],
+            [.. noHooks, .. verbatim, .. standaloneOverrides], NoSystemAttributes,
             "diff", "--cached", "--quiet");
         if (staged.ExitCode is not (0 or 1))
             throw new InvalidOperationException(
                 $"git diff --cached --quiet failed: {GitFailureDetail(staged.Stderr, staged.Stdout)}");
-        // Bookkeeping commit. Identity is explicit: a host with no user.email fails with
-        // "Author identity unknown". An empty folder is a valid start — commit an empty tree
-        // so the agent has a real HEAD to build on.
+        // Bookkeeping commit, with its own identity and unsigned: an unattended daemon cannot drive the
+        // user's signing program. An empty folder is a valid start — commit an empty tree so the agent
+        // has a real HEAD to build on.
         var commitArgs = staged.ExitCode == 0
             ? new[] { "commit", "--allow-empty", "-m", "Initial snapshot" }
             : new[] { "commit", "-m", "Initial snapshot" };
-        await RunGit(worktreePath, GitTimeout,
-            time,
-            [.. noHooks, new("user.email", "daemon@kcap.local"), new("user.name", "kcap")],
-            commitArgs);
+        await RunGit(worktreePath, GitTimeout, time, sourceReadOnly: false,
+            [.. noHooks, .. verbatim,
+             new("user.email", "daemon@kcap.local"), new("user.name", "kcap"), new("commit.gpgsign", "false")],
+            NoSystemAttributes, commitArgs);
 
         return new WorktreeInfo(worktreePath, "", repoPath, IsStandalone: true);
     }
@@ -501,6 +507,74 @@ await RunGit(worktreePath, GitTimeout,
     /// (see <c>ProveConfigTransportIfCarryingAsync</c>), so nothing here has to remember to.</summary>
     static GitConfigOverride[] NoBranchHooks() => [new("core.hooksPath", NoHooksPath)];
 
+    /// <summary>A standalone snapshot records the bytes it copied: a user's line-ending conversion would rewrite
+    /// them, and its strict form refuses the commit outright. A user's own attributes file is replaced by the
+    /// snapshot's, which keeps its rules but resets their line endings, so only the project's <c>.gitattributes</c>
+    /// decides a file's conversion.</summary>
+    static GitConfigOverride[] VerbatimLineEndings(string attributesFile) =>
+        [new("core.autocrlf", "false"), new("core.safecrlf", "false"), new("core.attributesFile", attributesFile)];
+
+    /// <summary>The machine's attributes file is not the project's either, and no config replaces it, so the snapshot's
+    /// add, diff and commit skip it.</summary>
+    static readonly KeyValuePair<string, string>[] NoSystemAttributes = [new("GIT_ATTR_NOSYSTEM", "1")];
+
+    /// <summary>The snapshot's attributes file, inside its git dir, as a path git reads the same on every platform: a
+    /// config value read from the environment is not unescaped, so it carries no backslash. Its <c>!text !eol</c>
+    /// resets both to unspecified for every path; git resolves each attribute on its own, so both are named.
+    /// <c>-text</c> would mark every path binary and void a project rule that sets only <c>eol</c>. Git ranks the file
+    /// above the machine's attributes and below the project's <c>.gitattributes</c>, so the agent's later commands,
+    /// which skip neither, still convert only what the project asks for. <c>info/attributes</c> would not do: it
+    /// outranks the project's own file.</summary>
+    static string CreateSnapshotAttributesFile(string worktreePath, string? userAttributesFile) {
+        var info = Path.Combine(worktreePath, ".git", "info");
+        Directory.CreateDirectory(info);
+        var path = Path.Combine(info, "kcap-snapshot-attributes");
+        // The user's own rules come first, so the agent's later commands keep them (an LFS filter, a diff driver), and
+        // the reset after them wins for text and eol, a later line in one file outranking an earlier one.
+        var user = userAttributesFile is not null && File.Exists(userAttributesFile) ? File.ReadAllText(userAttributesFile) : "";
+        File.WriteAllText(path, (user.Length > 0 && !user.EndsWith('\n') ? user + "\n" : user) + "* !text !eol\n");
+
+        return Path.GetFullPath(path).Replace('\\', '/');
+    }
+
+    /// <summary>The attributes file the user's git config names, relative to the snapshot as git reads it, else git's
+    /// default. Git 2.42 and later report that default; for an older git it is rebuilt from the same environment git
+    /// reads, which is why the home here is git's rather than the daemon's resolved one</summary>
+    static async Task<string?> UserAttributesFileAsync(string worktreePath, TimeProvider time) {
+        var configured = await RunGitCaptureResult(worktreePath, GitTimeout, time, sourceReadOnly: false, [],
+            "config", "--path", "--get", "core.attributesFile");
+        // An explicitly empty value turns the user's attributes off, so the default is not consulted.
+        if (configured.ExitCode == 0)
+            return OutputLine(configured.Stdout) is { Length: > 0 } named ? Path.Combine(worktreePath, named) : null;
+
+        var reported = await RunGitCaptureResult(worktreePath, GitTimeout, time, sourceReadOnly: false, [], "var", "GIT_ATTR_GLOBAL");
+        if (reported.ExitCode == 0 && OutputLine(reported.Stdout) is { Length: > 0 } path) return path;
+
+        if (Environment.GetEnvironmentVariable("XDG_CONFIG_HOME") is { Length: > 0 } xdg) return Path.Combine(xdg, "git", "attributes");
+
+        return GitHome() is { } home ? Path.Combine(home, ".config", "git", "attributes") : null;
+    }
+
+    /// <summary>Git's output without the LF it ends with, on every platform; a path's own trailing characters stay</summary>
+    static string OutputLine(string stdout) => stdout.EndsWith('\n') ? stdout[..^1] : stdout;
+
+    /// <summary>The home git uses: <c>HOME</c>, else on Windows <c>HOMEDRIVE</c> plus <c>HOMEPATH</c>, else
+    /// <c>USERPROFILE</c></summary>
+    static string? GitHome() {
+        if (Environment.GetEnvironmentVariable("HOME") is { Length: > 0 } home) return home;
+        if (!OperatingSystem.IsWindows()) return null;
+        if (Environment.GetEnvironmentVariable("HOMEDRIVE") is { Length: > 0 } drive
+         && Environment.GetEnvironmentVariable("HOMEPATH") is { Length: > 0 } homePath) return drive + homePath;
+
+        return Environment.GetEnvironmentVariable("USERPROFILE") is { Length: > 0 } profile ? profile : null;
+    }
+
+    /// <summary>A linked worktree's submodule has no gitdir of its own, so a checkout that recursed into it
+    /// fails outright; and a fetch that recursed would reach each submodule's own remote. Neither may
+    /// depend on the user's git config.</summary>
+    static GitConfigOverride[] NoSubmoduleRecursion() =>
+        [new("submodule.recurse", "false"), new("fetch.recurseSubmodules", "false")];
+
     /// <summary>
     /// Neutralizes the tree, and UNDOES the creation if that fails.
     ///
@@ -545,7 +619,8 @@ async Task CheckoutInTargetContextAsync(string worktreePath) {
         // pathspec matches nothing. This is the step that materialises the tree, and therefore the step
         // the overrides have to guard.
         await RunGit(worktreePath, GitTimeout, time,
-            [.. noHooks, .. overrides, .. ParallelCheckoutFor(OperatingSystem.IsWindows())], "reset", "--hard", "HEAD");
+            [.. noHooks, .. NoSubmoduleRecursion(), .. overrides, .. ParallelCheckoutFor(OperatingSystem.IsWindows())],
+            "reset", "--hard", "HEAD");
     }
 
     /// Windows only: its per-file write cost makes a serial checkout most of a launch's wait (7.5k files,
@@ -1019,12 +1094,10 @@ await SyncFromSourceCoreAsync(
                 throw new InvalidOperationException("borrowed_snapshot_sparse_checkout_unsupported");
 
             await RunGit(parent, GitTimeout, time, "clone", "--no-hardlinks", "--no-checkout", "--", bundle, destination);
-            // Same guard as `worktree add`: this checkout materialises branch content, so a relative
-            // core.hooksPath would run the branch's post-checkout here too. Missed when the guard was
-            // added — it went on the worktree paths only.
+            // Same guards as the worktree checkout: this one materialises branch content too.
             await RunGit(destination, GitTimeout,
                 time,
-                [.. NoBranchHooks(), .. await FilterOverridesForAsync(destination)],
+                [.. NoBranchHooks(), .. NoSubmoduleRecursion(), .. await FilterOverridesForAsync(destination)],
                 "checkout", "--detach", "HEAD");
             var clonedHead = (await RunGitCapture(destination, GitTimeout, time, false, "rev-parse", "HEAD")).Trim();
             if (!string.Equals(sourceHead, clonedHead, StringComparison.Ordinal)) throw new SourceChangedException();
@@ -1729,8 +1802,8 @@ static bool IsActive(string candidate, string[] activePaths) {
     static readonly TimeSpan FetchTimeout = TimeSpan.FromMinutes(2);
 
     /// <summary>How long to wait for a timed-out git process to actually die after being killed,
-    /// before giving up and unwinding anyway. See the timeout path in
-    /// <see cref="RunGitCaptureResult"/> for why the wait matters.</summary>
+    /// before giving up and unwinding anyway. The wait is why the capture path's timeout cannot let a
+    /// killed git outlive the metadata gate its caller holds.</summary>
     static readonly TimeSpan KillGrace = TimeSpan.FromSeconds(5);
 
     static async Task<bool> IsGitRepoWithCommits(string path, TimeProvider time) {
@@ -1791,10 +1864,15 @@ static Task RunGit(
             params string[] args) =>
         RunGit(cwd, timeout, time, sourceReadOnly: false, config, args);
 
+    static Task RunGit(
+            string cwd, TimeSpan timeout, TimeProvider time, bool sourceReadOnly, GitConfigOverride[] config,
+            params string[] args) =>
+        RunGit(cwd, timeout, time, sourceReadOnly, config, [], args);
+
     static async Task RunGit(
             string cwd, TimeSpan timeout, TimeProvider time, bool sourceReadOnly, GitConfigOverride[] config,
-            params string[] args) {
-        var result = await RunGitCaptureResult(cwd, timeout, time, sourceReadOnly, config, args);
+            KeyValuePair<string, string>[] environment, params string[] args) {
+        var result = await RunGitCaptureResult(cwd, timeout, time, sourceReadOnly, config, environment, args);
         if (result.ExitCode != 0)
             throw new InvalidOperationException(
                 $"git {string.Join(' ', args)} failed: {GitFailureDetail(result.Stderr, result.Stdout)}");
@@ -1827,12 +1905,17 @@ static async Task<bool> GitConfigBoolAsync(string cwd, string key, TimeProvider
         return string.Equals(result.Stdout.Trim(), "true", StringComparison.OrdinalIgnoreCase);
     }
 
-    internal static async Task<(int ExitCode, string Stdout, string Stderr)> RunGitCaptureResult(
+    internal static Task<(int ExitCode, string Stdout, string Stderr)> RunGitCaptureResult(
             string cwd, TimeSpan timeout, TimeProvider time, bool sourceReadOnly, GitConfigOverride[] config,
-            params string[] args) {
+            params string[] args) =>
+        RunGitCaptureResult(cwd, timeout, time, sourceReadOnly, config, [], args);
+
+    static async Task<(int ExitCode, string Stdout, string Stderr)> RunGitCaptureResult(
+            string cwd, TimeSpan timeout, TimeProvider time, bool sourceReadOnly, GitConfigOverride[] config,
+            KeyValuePair<string, string>[] environment, params string[] args) {
         await ProveConfigTransportIfCarryingAsync(cwd, time, sourceReadOnly, config);
 
-        return await RunGitCaptureResultUnproven(cwd, timeout, time, sourceReadOnly, config, args);
+        return await RunGitCaptureResultUnproven(cwd, timeout, time, sourceReadOnly, config, environment, args);
     }
 
     /// <summary>
@@ -1861,10 +1944,16 @@ static Task ProveConfigTransportIfCarryingAsync(
     /// <summary>Runs git WITHOUT proving the transport first. Only the probe itself may use this — it is
     /// what the gate above is measuring, so routing it through the gate would deadlock on the
     /// non-reentrant semaphore.</summary>
+    static Task<(int ExitCode, string Stdout, string Stderr)> RunGitCaptureResultUnproven(
+            string cwd, TimeSpan timeout, TimeProvider time, bool sourceReadOnly, GitConfigOverride[] config,
+            params string[] args) =>
+        RunGitCaptureResultUnproven(cwd, timeout, time, sourceReadOnly, config, [], args);
+
     static async Task<(int ExitCode, string Stdout, string Stderr)> RunGitCaptureResultUnproven(
             string cwd, TimeSpan timeout, TimeProvider time, bool sourceReadOnly, GitConfigOverride[] config,
-            params string[] args) {
+            KeyValuePair<string, string>[] environment, params string[] args) {
         var psi = NewGitPsi(cwd, args, sourceReadOnly, config);
+        foreach (var (key, value) in environment) psi.Environment[key] = value;
         using var proc = Process.Start(psi)!;
         using var cts = new CancellationTokenSource(timeout, time);
 

test/Capacitor.Cli.Daemon.Tests.Unit/Services/WorktreeManagerTests.cs

@@ -68,6 +68,49 @@ public async Task CreateAsync_WithBaseRef_WorktreeHeadMatchesFetchedCommit() {
         }
     }
 
+    /// <summary>
+    /// Fetching a head that moves a submodule makes git fetch the submodule too, from the submodule's own
+    /// remote. That remote is the user's business, not the review's: unreachable here, it must not fail
+    /// the launch.
+    /// </summary>
+    [Test]
+    public async Task CreateAsync_fetches_a_head_that_moves_a_submodule_whose_remote_is_unreachable() {
+        using var sub = GitRepo.CreateWithCommit("sub");
+        using var up = GitRepo.CreateWithCommit("upstream");
+        up.Do("-c", "protocol.file.allow=always", "submodule", "add", sub, "vendored");
+        up.Commit("add submodule");
+        up.Config("uploadpack.allowAnySHA1InWant", "true");
+
+        using var cloneDir = new TempDir("clone");
+        var clone = up.Clone(cloneDir.PathTo("repo"));
+        clone.Do("-c", "protocol.file.allow=always", "submodule", "update", "--init");
+
+        sub.CreateFile("moved.txt", "moved");
+        sub.CommitAll("moved");
+        var vendored = GitRepo.At(up.PathTo("vendored"));
+        vendored.Do("fetch", "-q", "origin");
+        vendored.Do("checkout", "-q", sub.Head);
+        up.Add("vendored");
+        up.Commit("move the submodule");
+        up.Do("update-ref", "refs/pull/7/head", up.Head);
+        var head = up.Head;
+        // The clone's submodule remote must not exist: a recursing fetch that could reach the moved commit would make
+        // the HEAD assertion prove nothing. Repointed rather than deleted, since Windows keeps git's read-only files.
+        var missing = cloneDir.PathTo("no-such-remote");
+        GitRepo.At(clone.PathTo("vendored")).Do("remote", "set-url", "origin", missing);
+        clone.Do("config", "submodule.vendored.url", missing);
+        await Assert.That(Directory.Exists(missing)).IsFalse();
+
+        var manager  = new WorktreeManager(new DaemonConfig(), NullLogger<WorktreeManager>.Instance, NoSnapshotBarrier.Instance, TimeProvider.System);
+        var worktree = await manager.CreateAsync(clone, name: "review-pr-7", baseRef: "refs/pull/7/head");
+
+        try {
+            await Assert.That(GitRepo.At(worktree.Path).Head).IsEqualTo(head);
+        } finally {
+            await WorktreeManager.RemoveAsync(worktree, TimeProvider.System);
+        }
+    }
+
     [Test]
     public async Task CreateAsync_with_a_local_only_sha_checks_it_out_without_a_remote() {
         using var repo = GitRepo.Create("local-only");