Fix several small bugs that an AI agent detected

src/osx/Installer.Mac/notarize.sh

@@ -1,4 +1,8 @@
 #!/bin/bash
+die () {
+    echo "$*" >&2
+    exit 1
+}
 
 for i in "$@"
 do

src/shared/Core.Tests/Commands/DiagnoseCommandTests.cs

@@ -2,6 +2,7 @@
 using System.Net.Http;
 using System.Security.AccessControl;
 using System.Text;
+using System.Threading.Tasks;
 using GitCredentialManager.Diagnostics;
 using GitCredentialManager.Tests.Objects;
 using Xunit;
@@ -11,7 +12,7 @@ namespace Core.Tests.Commands;
 public class DiagnoseCommandTests
 {
     [Fact]
-    public void NetworkingDiagnostic_SendHttpRequest_Primary_OK()
+    public async Task NetworkingDiagnostic_SendHttpRequest_Primary_OK()
     {
         var primaryUriString = "http://example.com";
         var sb = new StringBuilder();
@@ -24,14 +25,14 @@ public void NetworkingDiagnostic_SendHttpRequest_Primary_OK()
 
         httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse);
 
-        networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler));
+        await networkingDiagnostic.SendHttpRequestAsync(sb, new HttpClient(httpHandler));
 
         httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1);
         Assert.Contains(expected, sb.ToString());
     }
 
     [Fact]
-    public void NetworkingDiagnostic_SendHttpRequest_Backup_OK()
+    public async Task NetworkingDiagnostic_SendHttpRequest_Backup_OK()
     {
         var primaryUriString = "http://example.com";
         var backupUriString = "http://httpforever.com";
@@ -48,15 +49,15 @@ public void NetworkingDiagnostic_SendHttpRequest_Backup_OK()
         httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse);
         httpHandler.Setup(HttpMethod.Head, backupUri, httpResponse);
 
-        networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler));
+        await networkingDiagnostic.SendHttpRequestAsync(sb, new HttpClient(httpHandler));
 
         httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1);
         httpHandler.AssertRequest(HttpMethod.Head, backupUri, expectedNumberOfCalls: 1);
         Assert.Contains(expected, sb.ToString());
     }
 
     [Fact]
-    public void NetworkingDiagnostic_SendHttpRequest_No_Network()
+    public async Task NetworkingDiagnostic_SendHttpRequest_No_Network()
     {
         var primaryUriString = "http://example.com";
         var backupUriString = "http://httpforever.com";
@@ -73,7 +74,7 @@ public void NetworkingDiagnostic_SendHttpRequest_No_Network()
         httpHandler.Setup(HttpMethod.Head, primaryUri, httpResponse);
         httpHandler.Setup(HttpMethod.Head, backupUri, httpResponse);
 
-        networkingDiagnostic.SendHttpRequest(sb, new HttpClient(httpHandler));
+        await networkingDiagnostic.SendHttpRequestAsync(sb, new HttpClient(httpHandler));
 
         httpHandler.AssertRequest(HttpMethod.Head, primaryUri, expectedNumberOfCalls: 1);
         httpHandler.AssertRequest(HttpMethod.Head, backupUri, expectedNumberOfCalls: 1);

src/shared/Core.Tests/EnvironmentTests.cs

@@ -94,6 +94,7 @@ public void PosixEnvironment_TryLocateExecutable_Exists_ReturnTrueAndPath()
                     [expectedPath] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 
@@ -116,6 +117,32 @@ public void PosixEnvironment_TryLocateExecutable_ExistsMultiple_ReturnTrueAndFir
                     ["/bin/foo"] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(expectedPath);
+            fs.SetExecutable("/usr/local/bin/foo");
+            fs.SetExecutable("/bin/foo");
+            var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
+            var env = new PosixEnvironment(fs, envars);
+
+            bool actualResult = env.TryLocateExecutable(PosixExecName, out string actualPath);
+
+            Assert.True(actualResult);
+            Assert.Equal(expectedPath, actualPath);
+        }
+
+        [PosixFact]
+        public void PosixEnvironment_TryLocateExecutable_NotExecutable_SkipsToNextMatch()
+        {
+            string nonExecPath = "/home/john.doe/bin/foo";
+            string expectedPath = "/usr/local/bin/foo";
+            var fs = new TestFileSystem
+            {
+                Files = new Dictionary<string, byte[]>
+                {
+                    [nonExecPath] = Array.Empty<byte>(),
+                    [expectedPath] = Array.Empty<byte>(),
+                }
+            };
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 
@@ -142,6 +169,8 @@ public void MacOSEnvironment_TryLocateExecutable_Paths_Are_Ignored()
                     [expectedPath] = Array.Empty<byte>(),
                 }
             };
+            fs.SetExecutable(pathsToIgnore.FirstOrDefault());
+            fs.SetExecutable(expectedPath);
             var envars = new Dictionary<string, string> {["PATH"] = PosixPathVar};
             var env = new PosixEnvironment(fs, envars);
 

src/shared/Core.Tests/StreamExtensionsTests.cs

@@ -381,12 +381,13 @@ public void StreamExtensions_WriteDictionary_MultiEntriesWithEmpty_WritesKVPList
             {
                 ["a"] = new[] {"1", "2", "", "3", "4"},
                 ["b"] = new[] {"5"},
-                ["c"] = new[] {"6", "7", ""}
+                ["c"] = new[] {"6", "7", ""},
+                ["d"] = new[] {"8", "", "9"}
             };
 
             string output = WriteStringStream(input, StreamExtensions.WriteDictionary, newLine: LF);
 
-            Assert.Equal("a[]=3\na[]=4\nb=5\n\n", output);
+            Assert.Equal("a[]=3\na[]=4\nb=5\nd=9\n\n", output);
         }
 
         #endregion

src/shared/Core.Tests/Trace2MessageTests.cs

@@ -12,6 +12,8 @@ public class Trace2MessageTests
     [InlineData(26.316083,    " 26.316083 ")]
     [InlineData(100.316083,   "100.316083 ")]
     [InlineData(1000.316083,  "1000.316083")]
+    [InlineData(10000.316083, "10000.316083")]
+    [InlineData(100000.31608, "100000.316080")]
     public void BuildTimeSpan_Match_Returns_Expected_String(double input, string expected)
     {
         var actual = Trace2Message.BuildTimeSpan(input);

src/shared/Core/Authentication/OAuthAuthentication.cs

@@ -247,7 +247,7 @@ private Task ShowDeviceCodeViaUiAsync(OAuth2DeviceCodeResult dcr, CancellationTo
                 VerificationUrl = dcr.VerificationUri.ToString(),
             };
 
-            return AvaloniaUi.ShowViewAsync<DeviceCodeView>(viewModel, GetParentWindowHandle(), CancellationToken.None);
+            return AvaloniaUi.ShowViewAsync<DeviceCodeView>(viewModel, GetParentWindowHandle(), ct);
         }
 
         private Task ShowDeviceCodeViaHelperAsync(

src/shared/Core/Diagnostics/NetworkingDiagnostic.cs

@@ -29,7 +29,7 @@ protected override async Task<bool> RunInternalAsync(StringBuilder log, IList<st
             bool hasNetwork = NetworkInterface.GetIsNetworkAvailable();
             log.AppendLine($"IsNetworkAvailable: {hasNetwork}");
 
-            SendHttpRequest(log, httpClient);
+            await SendHttpRequestAsync(log, httpClient);
 
             log.Append($"Sending HEAD request to {TestHttpsUri}...");
             using var httpsResponse = await httpClient.HeadAsync(TestHttpsUri);
@@ -98,7 +98,7 @@ protected override async Task<bool> RunInternalAsync(StringBuilder log, IList<st
             return true;
         }
 
-        internal /* For testing purposes */ async void SendHttpRequest(StringBuilder log, HttpClient httpClient)
+        internal /* For testing purposes */ async Task SendHttpRequestAsync(StringBuilder log, HttpClient httpClient)
         {
             foreach (var uri in new List<string> { TestHttpUri, TestHttpUriFallback })
             {

src/shared/Core/EnvironmentBase.cs

@@ -138,7 +138,8 @@ internal virtual bool TryLocateExecutable(string program, ICollection<string> pa
                 {
                     string candidatePath = Path.Combine(basePath, program);
                     if (FileSystem.FileExists(candidatePath) && (pathsToIgnore is null ||
-                        !pathsToIgnore.Contains(candidatePath, StringComparer.OrdinalIgnoreCase)))
+                        !pathsToIgnore.Contains(candidatePath, StringComparer.OrdinalIgnoreCase))
+                        && FileSystem.FileIsExecutable(candidatePath))
                     {
                         path = candidatePath;
                         return true;

src/shared/Core/FileSystem.cs

@@ -34,6 +34,14 @@ public interface IFileSystem
         /// <returns>True if a file exists, false otherwise.</returns>
         bool FileExists(string path);
 
+        /// <summary>
+        /// Check if a file has execute permissions.
+        /// On Windows this always returns true. On POSIX it checks for any execute bit.
+        /// </summary>
+        /// <param name="path">Full path to file to test.</param>
+        /// <returns>True if the file is executable, false otherwise.</returns>
+        bool FileIsExecutable(string path);
+
         /// <summary>
         /// Check if a directory exists at the specified path.
         /// </summary>
@@ -122,6 +130,23 @@ public abstract class FileSystem : IFileSystem
 
         public bool FileExists(string path) => File.Exists(path);
 
+#if NETFRAMEWORK
+        public bool FileIsExecutable(string path) => true;
+#else
+        public bool FileIsExecutable(string path)
+        {
+            if (!PlatformUtils.IsPosix())
+                return true;
+
+#pragma warning disable CA1416 // Platform guard via PlatformUtils.IsPosix()
+            var mode = File.GetUnixFileMode(path);
+            return (mode & (UnixFileMode.UserExecute |
+                            UnixFileMode.GroupExecute |
+                            UnixFileMode.OtherExecute)) != 0;
+#pragma warning restore CA1416
+        }
+#endif
+
         public bool DirectoryExists(string path) => Directory.Exists(path);
 
         public string GetCurrentDirectory() => Directory.GetCurrentDirectory();

src/shared/Core/Git.cs

@@ -147,6 +147,13 @@ private string GetCurrentRepositoryInternal(bool suppressStreams)
 
                 git.Start(Trace2ProcessClass.Git);
                 string data = git.StandardOutput.ReadToEnd();
+
+                // Read all of standard error to prevent the process from hanging if it writes enough data to fill the buffer
+                if (suppressStreams)
+                {
+                    git.StandardError.ReadToEnd();
+                }
+
                 git.WaitForExit();
 
                 switch (git.ExitCode)
@@ -167,6 +174,8 @@ public IEnumerable<GitRemote> GetRemotes()
         {
             using (var git = CreateProcess("remote -v show"))
             {
+                // Redirect stderr so we can check for 'not a git repository' errors
+                git.StartInfo.RedirectStandardError = true;
                 git.Start(Trace2ProcessClass.Git);
                 // To avoid deadlocks, always read the output stream first and then wait
                 // TODO: don't read in all the data at once; stream it
@@ -267,7 +276,9 @@ public async Task<IDictionary<string, string>> InvokeHelperAsync(string args, ID
 
         public static GitException CreateGitException(ChildProcess git, string message, ITrace2 trace2 = null)
         {
-            var gitMessage = git.StandardError.ReadToEnd();
+            var gitMessage = git.StartInfo.RedirectStandardError
+                ? git.StandardError.ReadToEnd()
+                : null;
 
             if (trace2 != null)
                 throw new Trace2GitException(trace2, message, git.ExitCode, gitMessage);

src/shared/Core/Settings.cs

@@ -659,7 +659,7 @@ public bool IsCertificateVerificationEnabled
         }
 
         public bool AutomaticallyUseClientCertificates =>
-            TryGetSetting(null, KnownGitCfg.Credential.SectionName, KnownGitCfg.Http.SslAutoClientCert, out string value) && value.ToBooleanyOrDefault(false);
+            TryGetSetting(null, KnownGitCfg.Http.SectionName, KnownGitCfg.Http.SslAutoClientCert, out string value) && value.ToBooleanyOrDefault(false);
 
         public string CustomCertificateBundlePath =>
             TryGetPathSetting(KnownEnvars.GitSslCaInfo, KnownGitCfg.Http.SectionName, KnownGitCfg.Http.SslCaInfo, out string value) ? value : null;

src/shared/Core/StreamExtensions.cs

@@ -179,7 +179,7 @@ public static void WriteDictionary(this TextWriter writer, IDictionary<string, I
                         break;
 
                     case 1:
-                        writer.WriteLine($"{kvp.Key}={kvp.Value[0]}");
+                        writer.WriteLine($"{kvp.Key}={values[0]}");
                         break;
 
                     default:

src/shared/Core/Trace2.cs

@@ -460,11 +460,11 @@ protected override void ReleaseManagedResources()
         {
             try
             {
-                for (int i = 0; i < _writers.Count; i += 1)
+                for (int i = _writers.Count - 1; i >= 0; i--)
                 {
-                    using (var writer = _writers[i])
+                    using (_writers[i])
                     {
-                        _writers.Remove(writer);
+                        _writers.RemoveAt(i);
                     }
                 }
             }
@@ -640,7 +640,7 @@ private void WriteMessage(Trace2Message message)
     private static string BuildThreadName()
     {
         // If this is the entry thread, call it "main", per Trace2 convention
-        if (Thread.CurrentThread.ManagedThreadId == 0)
+        if (Thread.CurrentThread.ManagedThreadId == 1)
         {
             return "main";
         }

src/shared/Core/Trace2Message.cs

@@ -151,7 +151,7 @@ private static string BuildSpan(PerformanceFormatSpan component, string data)
             if (double.TryParse(data, out _))
             {
                 // Remove all padding for values that take up the entire span
-                if (Math.Abs(sizeDifference) == paddingTotal)
+                if (Math.Abs(sizeDifference) >= paddingTotal)
                 {
                     component.BeginPadding = 0;
                     component.EndPadding = 0;

src/shared/GitHub/GitHubAuthChallenge.cs

@@ -107,7 +107,15 @@ public override bool Equals(object obj)
 
     public override int GetHashCode()
     {
-        return Domain.GetHashCode() * 1019 ^
-               Enterprise.GetHashCode() * 337;
+        int domainHash = Domain is null
+            ? 0
+            : StringComparer.OrdinalIgnoreCase.GetHashCode(Domain);
+
+        int enterpriseHash = Enterprise is null
+            ? 0
+            : StringComparer.OrdinalIgnoreCase.GetHashCode(Enterprise);
+
+        return (domainHash * 1019) ^
+               (enterpriseHash * 337);
     }
 }

src/shared/GitHub/GitHubHostProvider.cs

@@ -198,6 +198,7 @@ private bool FilterAccounts(Uri remoteUri, IEnumerable<string> wwwAuth, ref ILis
             if (!IsGitHubDotCom(remoteUri))
             {
                 _context.Trace.WriteLine("No account filtering outside of GitHub.com.");
+                return false;
             }
 
             // Allow the user to disable account filtering until this feature stabilises.

src/shared/GitHub/UI/Commands/CredentialsCommand.cs

@@ -38,7 +38,7 @@ protected CredentialsCommand(ICommandContext context)
             this.SetHandler(ExecuteAsync, url, userName, basic, browser, device, pat, all);
         }
 
-        private async Task<int> ExecuteAsync(string userName, string enterpriseUrl,
+        private async Task<int> ExecuteAsync(string enterpriseUrl, string userName,
             bool basic, bool browser, bool device, bool pat, bool all)
         {
             var viewModel = new CredentialsViewModel(Context.SessionManager, Context.ProcessManager)

src/shared/GitLab/UI/Commands/CredentialsCommand.cs

@@ -35,7 +35,7 @@ protected CredentialsCommand(ICommandContext context)
             this.SetHandler(ExecuteAsync, url, userName, basic, browser, pat, all);
         }
 
-        private async Task<int> ExecuteAsync(string userName, string url, bool basic, bool browser, bool pat, bool all)
+        private async Task<int> ExecuteAsync(string url, string userName, bool basic, bool browser, bool pat, bool all)
         {
             var viewModel = new CredentialsViewModel(Context.SessionManager)
             {

src/shared/TestInfrastructure/Objects/TestFileSystem.cs

@@ -11,6 +11,7 @@ public class TestFileSystem : IFileSystem
         public string UserHomePath { get; set; }
         public string UserDataDirectoryPath { get; set; }
         public IDictionary<string, byte[]> Files { get; set; } = new Dictionary<string, byte[]>();
+        public ISet<string> ExecutableFiles { get; } = new HashSet<string>();
         public ISet<string> Directories { get; set; } = new HashSet<string>();
         public string CurrentDirectory { get; set; } = Path.GetTempPath();
         public bool IsCaseSensitive { get; set; } = false;
@@ -36,6 +37,18 @@ bool IFileSystem.FileExists(string path)
             return Files.ContainsKey(path);
         }
 
+        bool IFileSystem.FileIsExecutable(string path)
+        {
+            if (!Files.ContainsKey(path))
+                throw new FileNotFoundException("File not found", path);
+
+            // On Windows, all files are considered executable.
+            if (!PlatformUtils.IsPosix())
+                return true;
+
+            return ExecutableFiles.Contains(path);
+        }
+
         bool IFileSystem.DirectoryExists(string path)
         {
             return Directories.Contains(TrimSlash(path));
@@ -130,6 +143,20 @@ string[] IFileSystem.ReadAllLines(string path)
 
         #endregion
 
+        /// <summary>
+        /// Mark a test file as executable. File must exist in <see cref="Files"/> already.
+        /// </summary>
+        public void SetExecutable(string path, bool isExecutable = true)
+        {
+            if (!Files.ContainsKey(path))
+                throw new FileNotFoundException("File not found", path);
+
+            if (isExecutable)
+                ExecutableFiles.Add(path);
+            else
+                ExecutableFiles.Remove(path);
+        }
+
         /// <summary>
         /// Trim trailing slashes from a path.
         /// </summary>