Fix bugs in repository discovery feature (#282)

This commit is contained in:
Peter Steinberger 2025-07-11 07:52:21 +02:00 committed by GitHub
parent 84fa7333f0
commit 44d5bed721
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 148 additions and 57 deletions

View file

@ -16,6 +16,11 @@ enum AppConstants {
static let preventSleepWhenRunning = "preventSleepWhenRunning"
static let enableScreencapService = "enableScreencapService"
static let repositoryBasePath = "repositoryBasePath"
// New Session keys
static let newSessionCommand = "NewSession.command"
static let newSessionWorkingDirectory = "NewSession.workingDirectory"
static let newSessionSpawnWindow = "NewSession.spawnWindow"
static let newSessionTitleMode = "NewSession.titleMode"
}
/// Default values for UserDefaults
@ -46,21 +51,22 @@ enum AppConstants {
/// Helper to get string value with proper default
static func stringValue(for key: String) -> String {
// If the key doesn't exist in UserDefaults, return our default
// First check if we have a string value
if let value = UserDefaults.standard.string(forKey: key) {
return value
}
// If the key doesn't exist at all, return our default
if UserDefaults.standard.object(forKey: key) == nil {
switch key {
case UserDefaultsKeys.repositoryBasePath:
// return last used path if it's exists
if let value = UserDefaults.standard.value(forKey: "NewSession.workingDirectory") as? String {
return value
} else {
return Defaults.repositoryBasePath
}
return Defaults.repositoryBasePath
default:
return ""
}
}
return UserDefaults.standard.string(forKey: key) ?? ""
// Key exists but contains non-string value, return empty string
return ""
}
}

View file

@ -65,24 +65,15 @@ public final class RepositoryDiscoveryService {
return
}
Task.detached { [weak self] in
// Perform discovery in background
let discoveredRepos = await self?.performDiscovery(in: expandedPath)
guard let discoveredRepos else {
return
}
await MainActor.run { [weak self] in
// Cache and update results
self?.repositoryCache[expandedPath] = discoveredRepos
self?.repositories = discoveredRepos
Logger.repositoryDiscovery.info("Discovered \(discoveredRepos.count) repositories in: \(expandedPath)")
self?.isDiscovering = false
}
}
let discoveredRepos = await self.performDiscovery(in: expandedPath)
self.isDiscovering = false
// Cache and update results
self.repositoryCache[expandedPath] = discoveredRepos
self.repositories = discoveredRepos
Logger.repositoryDiscovery.info("Discovered \(discoveredRepos.count) repositories in: \(expandedPath)")
}
/// Clear the repository cache
@ -94,32 +85,24 @@ public final class RepositoryDiscoveryService {
// MARK: - Private Methods
/// Perform the actual discovery work
private nonisolated func performDiscovery(in basePath: String) async -> [DiscoveredRepository] {
return await withTaskGroup(of: [DiscoveredRepository].self) { taskGroup in
var allRepositories: [DiscoveredRepository] = []
// Submit discovery task
taskGroup.addTask { [weak self] in
await self?.scanDirectory(basePath, depth: 0) ?? []
}
// Collect results
for await repositories in taskGroup {
allRepositories.append(contentsOf: repositories)
}
// Sort by folder name for consistent display
return allRepositories.sorted { $0.folderName < $1.folderName }
}
private func performDiscovery(in basePath: String) async -> [DiscoveredRepository] {
let allRepositories = await scanDirectory(basePath, depth: 0)
// Sort by folder name for consistent display
return allRepositories.sorted { $0.folderName < $1.folderName }
}
/// Recursively scan a directory for Git repositories
private nonisolated func scanDirectory(_ path: String, depth: Int) async -> [DiscoveredRepository] {
private func scanDirectory(_ path: String, depth: Int) async -> [DiscoveredRepository] {
guard depth < maxSearchDepth else {
Logger.repositoryDiscovery.debug("Max depth reached at: \(path)")
return []
}
guard !Task.isCancelled else {
return []
}
do {
let fileManager = FileManager.default
let url = URL(fileURLWithPath: path)
@ -170,13 +153,13 @@ public final class RepositoryDiscoveryService {
}
/// Check if a directory is a Git repository
private nonisolated func isGitRepository(at path: String) -> Bool {
private func isGitRepository(at path: String) -> Bool {
let gitPath = URL(fileURLWithPath: path).appendingPathComponent(".git").path
return FileManager.default.fileExists(atPath: gitPath)
}
/// Create a DiscoveredRepository from a path
private nonisolated func createDiscoveredRepository(at path: String) async -> DiscoveredRepository {
private func createDiscoveredRepository(at path: String) async -> DiscoveredRepository {
let url = URL(fileURLWithPath: path)
let folderName = url.lastPathComponent
@ -195,7 +178,7 @@ public final class RepositoryDiscoveryService {
}
/// Get the last modified date of a repository
nonisolated private func getLastModifiedDate(at path: String) -> Date {
private func getLastModifiedDate(at path: String) -> Date {
do {
let attributes = try FileManager.default.attributesOfItem(atPath: path)
return attributes[.modificationDate] as? Date ?? Date.distantPast

View file

@ -474,21 +474,27 @@ struct NewSessionForm: View {
// MARK: - Preferences
private func loadPreferences() {
if let savedCommand = UserDefaults.standard.string(forKey: "NewSession.command") {
if let savedCommand = UserDefaults.standard.string(forKey: AppConstants.UserDefaultsKeys.newSessionCommand) {
command = savedCommand
}
workingDirectory = AppConstants.stringValue(for: AppConstants.UserDefaultsKeys.repositoryBasePath)
// Restore last used working directory, not repository base path
if let savedDirectory = UserDefaults.standard.string(forKey: AppConstants.UserDefaultsKeys.newSessionWorkingDirectory) {
workingDirectory = savedDirectory
} else {
// Default to home directory if never set
workingDirectory = "~/"
}
// Check if spawn window preference has been explicitly set
if UserDefaults.standard.object(forKey: "NewSession.spawnWindow") != nil {
spawnWindow = UserDefaults.standard.bool(forKey: "NewSession.spawnWindow")
if UserDefaults.standard.object(forKey: AppConstants.UserDefaultsKeys.newSessionSpawnWindow) != nil {
spawnWindow = UserDefaults.standard.bool(forKey: AppConstants.UserDefaultsKeys.newSessionSpawnWindow)
} else {
// Default to true if never set
spawnWindow = true
}
if let savedMode = UserDefaults.standard.string(forKey: "NewSession.titleMode"),
if let savedMode = UserDefaults.standard.string(forKey: AppConstants.UserDefaultsKeys.newSessionTitleMode),
let mode = TitleMode(rawValue: savedMode)
{
titleMode = mode
@ -496,10 +502,10 @@ struct NewSessionForm: View {
}
private func savePreferences() {
UserDefaults.standard.set(command, forKey: "NewSession.command")
UserDefaults.standard.set(workingDirectory, forKey: "NewSession.workingDirectory")
UserDefaults.standard.set(spawnWindow, forKey: "NewSession.spawnWindow")
UserDefaults.standard.set(titleMode.rawValue, forKey: "NewSession.titleMode")
UserDefaults.standard.set(command, forKey: AppConstants.UserDefaultsKeys.newSessionCommand)
UserDefaults.standard.set(workingDirectory, forKey: AppConstants.UserDefaultsKeys.newSessionWorkingDirectory)
UserDefaults.standard.set(spawnWindow, forKey: AppConstants.UserDefaultsKeys.newSessionSpawnWindow)
UserDefaults.standard.set(titleMode.rawValue, forKey: AppConstants.UserDefaultsKeys.newSessionTitleMode)
}
}

View file

@ -0,0 +1,96 @@
import Testing
import Foundation
@testable import VibeTunnel
@Suite("RepositoryDiscoveryService Tests")
struct RepositoryDiscoveryServiceTests {
@Test("Test repository discovery initialization")
@MainActor
func testServiceInitialization() async {
let service = RepositoryDiscoveryService()
#expect(service.repositories.isEmpty)
#expect(!service.isDiscovering)
#expect(service.lastError == nil)
}
@Test("Test discovery state management")
@MainActor
func testDiscoveryStateManagement() async {
let service = RepositoryDiscoveryService()
// Start discovery
let task = Task {
await service.discoverRepositories(in: "/nonexistent/path")
}
// Give it a moment to start
try? await Task.sleep(nanoseconds: 100_000_000) // 0.1 seconds
// Should not start another discovery while one is in progress
await service.discoverRepositories(in: "/another/path")
// Wait for completion
await task.value
// Should eventually reset isDiscovering
#expect(!service.isDiscovering)
}
@Test("Test cache functionality")
@MainActor
func testCacheFunctionality() async throws {
let service = RepositoryDiscoveryService()
let testPath = NSTemporaryDirectory()
// First discovery
await service.discoverRepositories(in: testPath)
try await Task.sleep(nanoseconds: 500_000_000) // 0.5 seconds
let firstCount = service.repositories.count
// Clear cache
service.clearCache()
// Second discovery should potentially find different results
await service.discoverRepositories(in: testPath)
try await Task.sleep(nanoseconds: 500_000_000) // 0.5 seconds
// Results should be consistent for the same path
#expect(service.repositories.count == firstCount)
}
@Test("Test race condition handling")
@MainActor
func testRaceConditionHandling() async throws {
// Create a service that will be deallocated during discovery
var service: RepositoryDiscoveryService? = RepositoryDiscoveryService()
// Start discovery
Task {
await service?.discoverRepositories(in: NSTemporaryDirectory())
}
// Deallocate service while discovery might be in progress
try await Task.sleep(nanoseconds: 50_000_000) // 0.05 seconds
service = nil
// Wait a bit more to ensure the task completes
try await Task.sleep(nanoseconds: 500_000_000) // 0.5 seconds
// Test passes if no crash occurs and the flag is properly reset
#expect(true) // If we get here, the race condition was handled
}
@Test("Test tilde expansion in path")
@MainActor
func testTildeExpansion() async {
let service = RepositoryDiscoveryService()
// Test with tilde path
await service.discoverRepositories(in: "~/")
// The service should handle tilde expansion without errors
#expect(service.lastError == nil)
}
}