diff --git a/supacode/Clients/Git/GitClient.swift b/supacode/Clients/Git/GitClient.swift index 48b5d61e..61f99ea5 100644 --- a/supacode/Clients/Git/GitClient.swift +++ b/supacode/Clients/Git/GitClient.swift @@ -883,13 +883,13 @@ struct GitClient { guard hostParts.count == 2 else { return nil } - return parseRepositoryWebInfo(host: String(hostParts[0]), path: String(hostParts[1])) + return parseRepositoryWebInfo(host: String(hostParts[0]), port: nil, path: String(hostParts[1])) } guard let url = URL(string: trimmed), let host = url.host else { return nil } let path = url.path.trimmingCharacters(in: CharacterSet(charactersIn: "/")) - return parseRepositoryWebInfo(host: host, path: path) + return parseRepositoryWebInfo(host: host, port: url.port, path: path) } nonisolated static func parseGithubRemoteInfo(_ remoteURL: String) -> GithubRemoteInfo? { @@ -899,7 +899,11 @@ struct GitClient { return parseGithubRemoteInfo(remoteWebInfo) } - nonisolated private static func parseRepositoryWebInfo(host: String, path: String) -> GitRemoteWebInfo? { + nonisolated private static func parseRepositoryWebInfo( + host: String, + port: Int?, + path: String + ) -> GitRemoteWebInfo? { let components = path.split(separator: "/", omittingEmptySubsequences: true) guard components.count >= 2 else { return nil @@ -911,7 +915,7 @@ struct GitClient { guard !repositoryPath.isEmpty else { return nil } - return GitRemoteWebInfo(host: host, repositoryPath: repositoryPath) + return GitRemoteWebInfo(host: host, repositoryPath: repositoryPath, port: port) } nonisolated private static func parseGithubRemoteInfo(_ remoteWebInfo: GitRemoteWebInfo) -> GithubRemoteInfo? { diff --git a/supacode/Clients/Git/GitRemoteWebInfo.swift b/supacode/Clients/Git/GitRemoteWebInfo.swift index c437e112..cdce1aa1 100644 --- a/supacode/Clients/Git/GitRemoteWebInfo.swift +++ b/supacode/Clients/Git/GitRemoteWebInfo.swift @@ -3,8 +3,20 @@ import Foundation struct GitRemoteWebInfo: Equatable, Sendable { let host: String let repositoryPath: String + let port: Int? - var repositoryURL: URL? { - URL(string: "https://\(host)/\(repositoryPath)") + nonisolated init(host: String, repositoryPath: String, port: Int? = nil) { + self.host = host + self.repositoryPath = repositoryPath + self.port = port + } + + nonisolated var repositoryURL: URL? { + var components = URLComponents() + components.scheme = "https" + components.host = host + components.port = port + components.path = "/\(repositoryPath)" + return components.url } } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index f4a9bab1..64965a79 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -258,7 +258,7 @@ extension RepositoriesFeature { case .openOnCodeHost: let gitClient = gitClient let openURLClient = openURLClient - let pullRequestURL = pullRequest.flatMap { URL(string: $0.url) } + let pullRequestURL = pullRequest.flatMap { Self.validWebURL($0.url) } return .run { send in if let pullRequestURL { await openURLClient.open(pullRequestURL) @@ -573,6 +573,17 @@ extension RepositoriesFeature { } } + nonisolated private static func validWebURL(_ raw: String) -> URL? { + guard let url = URL(string: raw), + let scheme = url.scheme?.lowercased(), + ["http", "https"].contains(scheme), + url.host != nil + else { + return nil + } + return url + } + var githubIntegrationReducer: some ReducerOf { Reduce { state, action in guard case .githubIntegration(let action) = action else { diff --git a/supacodeTests/GitRemoteInfoTests.swift b/supacodeTests/GitRemoteInfoTests.swift index 3728f35c..b592e504 100644 --- a/supacodeTests/GitRemoteInfoTests.swift +++ b/supacodeTests/GitRemoteInfoTests.swift @@ -16,6 +16,17 @@ struct GitRemoteInfoTests { #expect(info?.repositoryURL == URL(string: "https://gitlab.com/group/subgroup/repo")) } + @Test func parseRepositoryWebInfoPreservesCustomPortAndPathPrefix() { + let info = GitClient.parseRepositoryWebInfo("ssh://git@git.example.com:8443/scm/platform/repo.git") + #expect(info == GitRemoteWebInfo(host: "git.example.com", repositoryPath: "scm/platform/repo", port: 8443)) + #expect(info?.repositoryURL == URL(string: "https://git.example.com:8443/scm/platform/repo")) + } + + @Test func parseRepositoryWebInfoRejectsUnparseableRemote() { + let info = GitClient.parseRepositoryWebInfo("/tmp/local-only/repo.git") + #expect(info == nil) + } + @Test func parseSSHRemote() { let info = GitClient.parseGithubRemoteInfo("git@github.com:octo/repo.git") #expect(info == GithubRemoteInfo(host: "github.com", owner: "octo", repo: "repo")) diff --git a/supacodeTests/GitRepositoryWebURLIntegrationTests.swift b/supacodeTests/GitRepositoryWebURLIntegrationTests.swift new file mode 100644 index 00000000..35f99499 --- /dev/null +++ b/supacodeTests/GitRepositoryWebURLIntegrationTests.swift @@ -0,0 +1,78 @@ +import Foundation +import Testing + +@testable import supacode + +struct GitRepositoryWebURLIntegrationTests { + @Test func returnsNilWhenRepositoryHasNoRemote() async throws { + let repoURL = try makeTemporaryRepo(namePrefix: "supacode-weburl-no-remote") + defer { try? FileManager.default.removeItem(at: repoURL) } + + let url = await GitClient().repositoryWebURL(for: repoURL) + + #expect(url == nil) + } + + @Test func returnsNilWhenRemoteCannotBeParsed() async throws { + let repoURL = try makeTemporaryRepo(namePrefix: "supacode-weburl-unparseable") + defer { try? FileManager.default.removeItem(at: repoURL) } + + try runGit([ + "-C", repoURL.path(percentEncoded: false), + "remote", "add", "origin", "/tmp/local-only/repo.git", + ]) + + let url = await GitClient().repositoryWebURL(for: repoURL) + + #expect(url == nil) + } + + @Test func preservesCustomPortAndPathPrefixFromRemote() async throws { + let repoURL = try makeTemporaryRepo(namePrefix: "supacode-weburl-port-prefix") + defer { try? FileManager.default.removeItem(at: repoURL) } + + try runGit([ + "-C", repoURL.path(percentEncoded: false), + "remote", "add", "origin", "ssh://git@git.example.com:8443/scm/platform/repo.git", + ]) + + let url = await GitClient().repositoryWebURL(for: repoURL) + + #expect(url == URL(string: "https://git.example.com:8443/scm/platform/repo")) + } +} + +private struct GitCommandError: Error { + let output: String +} + +private func makeTemporaryRepo(namePrefix: String) throws -> URL { + let tempRoot = URL(filePath: "/tmp", directoryHint: .isDirectory) + let repoURL = tempRoot.appending( + path: "\(namePrefix)-\(UUID().uuidString)", + directoryHint: URL.DirectoryHint.isDirectory + ) + try runGit(["init", repoURL.path(percentEncoded: false)]) + return repoURL +} + +@discardableResult +private func runGit(_ arguments: [String]) throws -> String { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/git") + process.arguments = arguments + var environment = ProcessInfo.processInfo.environment + environment["GIT_CONFIG_GLOBAL"] = "/dev/null" + process.environment = environment + let pipe = Pipe() + process.standardOutput = pipe + process.standardError = pipe + try process.run() + process.waitUntilExit() + let data = pipe.fileHandleForReading.readDataToEndOfFile() + let output = String(data: data, encoding: .utf8) ?? "" + if process.terminationStatus != 0 { + throw GitCommandError(output: output) + } + return output +} diff --git a/supacodeTests/RepositoriesFeatureTests.swift b/supacodeTests/RepositoriesFeatureTests.swift index f95613fd..4289e6d1 100644 --- a/supacodeTests/RepositoriesFeatureTests.swift +++ b/supacodeTests/RepositoriesFeatureTests.swift @@ -3172,6 +3172,75 @@ struct RepositoriesFeatureTests { #expect(openedURLs.value == [repositoryURL]) } + @Test func pullRequestActionOpenOnCodeHostFallsBackWhenPullRequestURLIsInvalid() async { + let repoRoot = "/tmp/repo" + let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot) + let featureWorktree = makeWorktree( + id: "\(repoRoot)/feature", + name: "feature", + repoRoot: repoRoot + ) + let repository = makeRepository(id: repoRoot, worktrees: [mainWorktree, featureWorktree]) + let pullRequest = makePullRequest( + state: "OPEN", + headRefName: featureWorktree.name, + number: 12, + url: "/octo/repo/pull/12" + ) + var state = makeState(repositories: [repository]) + state.worktreeInfoByID[featureWorktree.id] = WorktreeInfoEntry( + addedLines: nil, + removedLines: nil, + pullRequest: pullRequest + ) + let repositoryURL = URL(string: "https://git.example.com/scm/repo")! + let openedURLs = LockIsolated<[URL]>([]) + let store = TestStore(initialState: state) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.repositoryWebURL = { _ in + repositoryURL + } + $0.openURLClient.open = { url in + openedURLs.withValue { $0.append(url) } + } + } + + await store.send(.githubIntegration(.pullRequestAction(featureWorktree.id, .openOnCodeHost))) + await store.finish() + + #expect(openedURLs.value == [repositoryURL]) + } + + @Test func pullRequestActionOpenOnCodeHostShowsAlertWhenRepositoryURLUnavailable() async { + let repoRoot = "/tmp/repo" + let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot) + let featureWorktree = makeWorktree( + id: "\(repoRoot)/feature", + name: "feature", + repoRoot: repoRoot + ) + let repository = makeRepository(id: repoRoot, worktrees: [mainWorktree, featureWorktree]) + let store = TestStore(initialState: makeState(repositories: [repository])) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.repositoryWebURL = { _ in nil } + } + + await store.send(.githubIntegration(.pullRequestAction(featureWorktree.id, .openOnCodeHost))) + await store.receive(\.presentAlert) { + $0.alert = AlertState { + TextState("Repository URL not available") + } actions: { + ButtonState(role: .cancel) { + TextState("OK") + } + } message: { + TextState("Prowl could not determine a code host URL for this repository.") + } + } + } + @Test func worktreeInfoEventRepositoryPullRequestRefreshMarksInFlightThenCompletes() async { let repoRoot = "/tmp/repo" let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot)