diff --git a/supacode/Clients/Git/GitClientShellHelpers.swift b/supacode/Clients/Git/GitClientShellHelpers.swift index 8b7ae55e..5bd77939 100644 --- a/supacode/Clients/Git/GitClientShellHelpers.swift +++ b/supacode/Clients/Git/GitClientShellHelpers.swift @@ -7,11 +7,13 @@ nonisolated func shouldFallbackToLoginShell(_ error: Error) -> Bool { guard let shellError = error as? ShellClientError else { return false } - if shellError.exitCode == 127 { - return true - } let output = "\(shellError.stderr)\n\(shellError.stdout)".lowercased() - return output.contains("command not found") + // When git itself ran fine but confirmed this isn't a repo, retrying + // under a login shell won't change the answer. + if output.contains("not a git repository") { + return false + } + return true } nonisolated func wrapShellError( diff --git a/supacodeTests/GitClientShellFallbackTests.swift b/supacodeTests/GitClientShellFallbackTests.swift new file mode 100644 index 00000000..3e838e35 --- /dev/null +++ b/supacodeTests/GitClientShellFallbackTests.swift @@ -0,0 +1,68 @@ +import Foundation +import Testing + +@testable import supacode + +struct GitClientShellFallbackTests { + private func shellError( + stderr: String = "", + stdout: String = "", + exitCode: Int32 = 1 + ) -> ShellClientError { + ShellClientError(command: "wt root", stdout: stdout, stderr: stderr, exitCode: exitCode) + } + + // MARK: - Should fallback + + @Test func fallsBackWhenExecutableNotFound() { + #expect(shouldFallbackToLoginShell(shellError(exitCode: 127))) + } + + @Test func fallsBackOnCommandNotFoundMessage() { + #expect(shouldFallbackToLoginShell(shellError(stderr: "env: git: command not found"))) + } + + @Test func fallsBackWhenXcodeLicenseUnaccepted() { + let stderr = """ + You have not agreed to the Xcode license agreements. Please run 'sudo xcodebuild -license' \ + from within a Terminal window to review and agree to the Xcode and Apple SDKs license. + """ + #expect(shouldFallbackToLoginShell(shellError(stderr: stderr, exitCode: 69))) + } + + @Test func fallsBackOnInvalidActiveDeveloperPath() { + let stderr = "xcode-select: error: invalid active developer path (/Library/Developer/CommandLineTools)" + #expect(shouldFallbackToLoginShell(shellError(stderr: stderr, exitCode: 1))) + } + + @Test func fallsBackOnUnknownShellError() { + #expect(shouldFallbackToLoginShell(shellError(stderr: "something unexpected", exitCode: 42))) + } + + @Test func fallsBackOnEmptyErrorOutput() { + #expect(shouldFallbackToLoginShell(shellError(exitCode: 1))) + } + + // MARK: - Should NOT fallback + + @Test func doesNotFallBackForGenuineNonGitDirectory() { + #expect( + shouldFallbackToLoginShell( + shellError(stderr: "fatal: not a git repository (or any of the parent directories)", exitCode: 128) + ) == false + ) + } + + @Test func doesNotFallBackWhenWtReportsNotGitRepo() { + #expect( + shouldFallbackToLoginShell( + shellError(stderr: "wt: not a git repository", exitCode: 1) + ) == false + ) + } + + @Test func doesNotFallBackForNonShellError() { + struct OtherError: Error {} + #expect(shouldFallbackToLoginShell(OtherError()) == false) + } +} -- 2.51.2 From bb871a6ef9f742f5aa05beaf1d9f10179eb3f9a0 Mon Sep 17 00:00:00 2001 From: onevcat Date: Tue, 23 Jun 2026 00:48:55 +0900 Subject: [PATCH 2/2] Update worktree discovery tests for inverted fallback logic Replace the "no fallback for regular failures" test with two tests that match the new behavior: non-git directories skip the fallback (pointless to retry), while environment errors (e.g. permission denied) do trigger it and recover via the login shell. --- .../GitClientWorktreeDiscoveryTests.swift | 50 +++++++++++++++++-- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/supacodeTests/GitClientWorktreeDiscoveryTests.swift b/supacodeTests/GitClientWorktreeDiscoveryTests.swift index a7202de5..dffea2bc 100644 --- a/supacodeTests/GitClientWorktreeDiscoveryTests.swift +++ b/supacodeTests/GitClientWorktreeDiscoveryTests.swift @@ -209,7 +209,7 @@ struct GitClientWorktreeDiscoveryTests { } } - @Test func worktreesDoNotFallbackToLoginShellForRegularFailures() async { + @Test func worktreesDoNotFallbackToLoginShellForNonGitDirectory() async { let recorder = GitWorktreeDiscoveryRecorder() let shell = ShellClient( run: { executableURL, arguments, currentDirectoryURL in @@ -221,8 +221,8 @@ struct GitClientWorktreeDiscoveryTests { throw ShellClientError( command: "wt ls --json", stdout: "", - stderr: "permission denied", - exitCode: 1 + stderr: "fatal: not a git repository (or any of the parent directories): .git", + exitCode: 128 ) }, runLoginImpl: { executableURL, arguments, currentDirectoryURL, _ in @@ -231,17 +231,57 @@ struct GitClientWorktreeDiscoveryTests { arguments: arguments, currentDirectoryURL: currentDirectoryURL ) - Issue.record("worktrees should not fallback to runLogin for regular command failures") + Issue.record("worktrees should not fallback to runLogin for non-git directories") return ShellOutput(stdout: "", stderr: "", exitCode: 0) } ) let client = GitClient(shell: shell) await #expect(throws: GitClientError.self) { - _ = try await client.worktrees(for: URL(fileURLWithPath: "/tmp/repo")) + _ = try await client.worktrees(for: URL(fileURLWithPath: "/tmp/not-a-repo")) } #expect(recorder.runInvocations().count == 1) #expect(recorder.loginInvocations().isEmpty) } + + @Test func worktreesFallbackToLoginShellForEnvironmentErrors() async throws { + let recorder = GitWorktreeDiscoveryRecorder() + let shell = ShellClient( + run: { executableURL, arguments, currentDirectoryURL in + recorder.recordRun( + executableURL: executableURL, + arguments: arguments, + currentDirectoryURL: currentDirectoryURL + ) + throw ShellClientError( + command: "wt ls --json", + stdout: "", + stderr: "permission denied", + exitCode: 1 + ) + }, + runLoginImpl: { executableURL, arguments, currentDirectoryURL, _ in + recorder.recordLogin( + executableURL: executableURL, + arguments: arguments, + currentDirectoryURL: currentDirectoryURL + ) + return ShellOutput( + stdout: """ + [{"branch":"main","path":"/tmp/repo","head":"abc","is_bare":false}] + """, + stderr: "", + exitCode: 0 + ) + } + ) + let client = GitClient(shell: shell) + + let worktrees = try await client.worktrees(for: URL(fileURLWithPath: "/tmp/repo")) + + #expect(worktrees.count == 1) + #expect(recorder.runInvocations().count == 1) + #expect(recorder.loginInvocations().count == 1) + } }