diff --git a/supacode/App/ContentView.swift b/supacode/App/ContentView.swift index 208b37e0..4c03a34f 100644 --- a/supacode/App/ContentView.swift +++ b/supacode/App/ContentView.swift @@ -31,6 +31,14 @@ struct ContentView: View { get: { store.runScriptDraft }, set: { store.send(.runScriptDraftChanged($0)) } ) + let deleteWorktreeConfirmationPresented = Binding( + get: { repositoriesStore.deleteWorktreeConfirmation != nil }, + set: { isPresented in + if !isPresented { + repositoriesStore.send(.worktreeLifecycle(.deleteWorktreePromptDismissed)) + } + } + ) Group { if store.repositories.isInitialLoadComplete { mainSplitView @@ -72,6 +80,22 @@ struct ContentView: View { promptStore in WorktreeCreationPromptView(store: promptStore) } + .sheet(isPresented: deleteWorktreeConfirmationPresented) { + if let confirmation = repositoriesStore.deleteWorktreeConfirmation { + DeleteWorktreeConfirmationView( + confirmation: confirmation, + onDeleteBranchChanged: { + repositoriesStore.send(.worktreeLifecycle(.deleteWorktreePromptDeleteBranchChanged($0))) + }, + onCancel: { + repositoriesStore.send(.worktreeLifecycle(.deleteWorktreePromptDismissed)) + }, + onDelete: { + repositoriesStore.send(.worktreeLifecycle(.deleteWorktreePromptConfirmed)) + } + ) + } + } .sheet(isPresented: isRunScriptPromptPresented) { RunScriptPromptView( script: runScriptDraft, diff --git a/supacode/Clients/Git/GitClient.swift b/supacode/Clients/Git/GitClient.swift index 9ca5e5ff..7af1e6ac 100644 --- a/supacode/Clients/Git/GitClient.swift +++ b/supacode/Clients/Git/GitClient.swift @@ -45,6 +45,13 @@ enum GitWorktreeCreateEvent: Equatable, Sendable { case finished(Worktree) } +enum LocalBranchDeletionOutcome: Equatable, Sendable { + case deleted + case notFound + case protected + case notRequested +} + nonisolated enum GitRemoteMatcher { static func matchingRemote(for ref: String, from remotes: [String]) -> String? { remotes @@ -590,14 +597,12 @@ struct GitClient { } catch { await runGitWorktreeRemove(rootPath: rootPath, worktreePath: worktreePath) } - if deleteBranch, !worktree.name.isEmpty { - let names = try await localBranchNames(for: worktree.repositoryRootURL) - if names.contains(worktree.name.lowercased()) { - _ = try? await runGit( - operation: .branchDelete, - arguments: ["-C", rootPath, "branch", "-D", worktree.name] - ) - } + if deleteBranch { + _ = try? await deleteLocalBranch( + named: worktree.name, + for: worktree.repositoryRootURL, + force: false + ) } Task.detached { try? FileManager.default.removeItem(at: relocatedURL) @@ -605,18 +610,55 @@ struct GitClient { return worktree.workingDirectory } await runGitWorktreeRemove(rootPath: rootPath, worktreePath: worktreePath) - if deleteBranch, !worktree.name.isEmpty { - let names = try await localBranchNames(for: worktree.repositoryRootURL) - if names.contains(worktree.name.lowercased()) { - _ = try? await runGit( - operation: .branchDelete, - arguments: ["-C", rootPath, "branch", "-D", worktree.name] - ) - } + if deleteBranch { + _ = try? await deleteLocalBranch( + named: worktree.name, + for: worktree.repositoryRootURL, + force: false + ) } return worktree.workingDirectory } + nonisolated func deleteLocalBranch( + named branchName: String, + for repoRoot: URL, + force: Bool + ) async throws -> LocalBranchDeletionOutcome { + guard !branchName.isEmpty else { return .notRequested } + let rootPath = repoRoot.path(percentEncoded: false) + let normalizedName = branchName.lowercased() + let names = try await localBranchNames(for: repoRoot) + guard names.contains(normalizedName) else { return .notFound } + let protectedNames = await protectedLocalBranchNames(for: repoRoot) + guard !protectedNames.contains(normalizedName) else { return .protected } + _ = try await runGit( + operation: .branchDelete, + arguments: ["-C", rootPath, "branch", force ? "-D" : "-d", branchName] + ) + return .deleted + } + + nonisolated private func protectedLocalBranchNames(for repoRoot: URL) async -> Set { + var names: Set = ["main", "master"] + if let defaultRef = try? await defaultRemoteBranchRef(for: repoRoot), + let defaultBranchName = Self.localBranchName(fromRef: defaultRef) + { + names.insert(defaultBranchName.lowercased()) + } + return names + } + + nonisolated private static func localBranchName(fromRef ref: String) -> String? { + let trimmed = ref.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return nil } + guard let slashIndex = trimmed.firstIndex(of: "/") else { + return trimmed + } + let name = trimmed[trimmed.index(after: slashIndex)...] + return name.isEmpty ? nil : String(name) + } + nonisolated private func parseShortstat(_ output: String) -> (added: Int, removed: Int) { let trimmed = output.trimmingCharacters(in: .whitespacesAndNewlines) guard !trimmed.isEmpty else { diff --git a/supacode/Clients/Repositories/GitClientDependency.swift b/supacode/Clients/Repositories/GitClientDependency.swift index c0561b2d..26ee0f36 100644 --- a/supacode/Clients/Repositories/GitClientDependency.swift +++ b/supacode/Clients/Repositories/GitClientDependency.swift @@ -32,6 +32,9 @@ struct GitClientDependency: Sendable { _ baseRef: String ) -> AsyncThrowingStream var removeWorktree: @Sendable (_ worktree: Worktree, _ deleteBranch: Bool) async throws -> URL + var deleteLocalBranch: + @Sendable (_ branchName: String, _ repoRoot: URL, _ force: Bool) async throws + -> LocalBranchDeletionOutcome var isBareRepository: @Sendable (_ repoRoot: URL) async throws -> Bool var branchName: @Sendable (URL) async -> String? var lineChanges: @Sendable (URL) async -> (added: Int, removed: Int)? @@ -77,6 +80,9 @@ extension GitClientDependency: DependencyKey { removeWorktree: { worktree, deleteBranch in try await GitClient().removeWorktree(worktree, deleteBranch: deleteBranch) }, + deleteLocalBranch: { branchName, repoRoot, force in + try await GitClient().deleteLocalBranch(named: branchName, for: repoRoot, force: force) + }, isBareRepository: { repoRoot in try await GitClient().isBareRepository(for: repoRoot) }, diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index d91cc7af..0310bfb5 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -237,9 +237,17 @@ extension RepositoriesFeature { } ) case .delete: + @Shared(.settingsFile) var settingsFile return .merge( mergedWorktreeIDs.map { worktreeID in - .send(.worktreeLifecycle(.deleteWorktreeConfirmed(worktreeID, repositoryID))) + .send( + .worktreeLifecycle( + .deleteWorktreeConfirmed( + worktreeID, + repositoryID, + deleteBranch: settingsFile.global.deleteBranchOnDeleteWorktree + )) + ) } ) case nil: diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeCreation.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeCreation.swift index d0c3fffb..227feb5e 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeCreation.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeCreation.swift @@ -556,6 +556,11 @@ extension RepositoriesFeature { analyticsClient.capture("worktree_created", [String: Any]?.none) state.pendingSetupScriptWorktreeIDs.insert(worktree.id) state.pendingTerminalFocusWorktreeIDs.insert(worktree.id) + state.$prowlCreatedWorktreeIDs.withLock { + if !$0.contains(worktree.id) { + $0.append(worktree.id) + } + } removePendingWorktree(pendingID, state: &state) if state.selection == .worktree(pendingID) { setSingleWorktreeSelection(worktree.id, state: &state, recordHistory: false) diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift index 1fa2815b..d685a7d8 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift @@ -295,24 +295,12 @@ extension RepositoriesFeature { if state.deletingWorktreeIDs.contains(worktree.id) { return .none } - @Shared(.settingsFile) var settingsFile - let deleteBranchOnDeleteWorktree = settingsFile.global.deleteBranchOnDeleteWorktree - let removalMessage = - deleteBranchOnDeleteWorktree - ? "This deletes the worktree directory and its local branch." - : "This deletes the worktree directory and keeps the local branch." - state.alert = AlertState { - TextState("🚨 Delete worktree?") - } actions: { - ButtonState(role: .destructive, action: .confirmDeleteWorktree(worktree.id, repository.id)) { - TextState("Delete (⌘↩)") - } - ButtonState(role: .cancel) { - TextState("Cancel") - } - } message: { - TextState("Delete \(worktree.name)? " + removalMessage) - } + state.deleteWorktreeConfirmation = makeDeleteWorktreeConfirmation( + id: state.nextDeleteWorktreeConfirmationID, + targets: [DeleteWorktreeTarget(worktreeID: worktree.id, repositoryID: repository.id)], + state: state + ) + state.nextDeleteWorktreeConfirmationID += 1 return .none case .requestDeleteWorktrees(let targets): @@ -339,28 +327,41 @@ extension RepositoriesFeature { guard !validTargets.isEmpty else { return .none } - @Shared(.settingsFile) var settingsFile - let deleteBranchOnDeleteWorktree = settingsFile.global.deleteBranchOnDeleteWorktree - let removalMessage = - deleteBranchOnDeleteWorktree - ? "This deletes the worktree directories and their local branches." - : "This deletes the worktree directories and keeps their local branches." - let count = validTargets.count - state.alert = AlertState { - TextState("🚨 Delete \(count) worktrees?") - } actions: { - ButtonState(role: .destructive, action: .confirmDeleteWorktrees(validTargets)) { - TextState("Delete \(count) (⌘↩)") - } - ButtonState(role: .cancel) { - TextState("Cancel") - } - } message: { - TextState("Delete \(count) worktrees? " + removalMessage) - } + state.deleteWorktreeConfirmation = makeDeleteWorktreeConfirmation( + id: state.nextDeleteWorktreeConfirmationID, + targets: validTargets, + state: state + ) + state.nextDeleteWorktreeConfirmationID += 1 + return .none + + case .deleteWorktreePromptDeleteBranchChanged(let deleteBranch): + state.deleteWorktreeConfirmation?.deleteBranch = deleteBranch return .none - case .deleteWorktreeConfirmed(let worktreeID, let repositoryID): + case .deleteWorktreePromptDismissed: + state.deleteWorktreeConfirmation = nil + return .none + + case .deleteWorktreePromptConfirmed: + guard let confirmation = state.deleteWorktreeConfirmation else { + return .none + } + state.deleteWorktreeConfirmation = nil + return .merge( + confirmation.targets.map { target in + .send( + .worktreeLifecycle( + .deleteWorktreeConfirmed( + target.worktreeID, + target.repositoryID, + deleteBranch: confirmation.deleteBranch + )) + ) + } + ) + + case .deleteWorktreeConfirmed(let worktreeID, let repositoryID, let deleteBranch): guard let repository = state.repositories[id: repositoryID], let worktree = repository.worktrees[id: worktreeID] else { @@ -379,21 +380,35 @@ extension RepositoriesFeature { selectionWasRemoved ? nextWorktreeID(afterRemoving: worktree, in: repository, state: state) : nil - @Shared(.settingsFile) var settingsFile - let deleteBranchOnDeleteWorktree = settingsFile.global.deleteBranchOnDeleteWorktree return .run { send in do { _ = try await gitClient.removeWorktree( worktree, - deleteBranchOnDeleteWorktree + false ) + let forceDeleteBranchRequest: ForceDeleteBranchRequest? + if deleteBranch { + do { + _ = try await gitClient.deleteLocalBranch(worktree.name, worktree.repositoryRootURL, false) + forceDeleteBranchRequest = nil + } catch { + forceDeleteBranchRequest = ForceDeleteBranchRequest( + branchName: worktree.name, + repositoryRootURL: worktree.repositoryRootURL, + errorMessage: error.localizedDescription + ) + } + } else { + forceDeleteBranchRequest = nil + } await send( .worktreeLifecycle( .worktreeDeleted( worktree.id, repositoryID: repository.id, selectionWasRemoved: selectionWasRemoved, - nextSelection: nextSelection + nextSelection: nextSelection, + forceDeleteBranchRequest: forceDeleteBranchRequest ) ) ) @@ -406,7 +421,8 @@ extension RepositoriesFeature { let worktreeID, let repositoryID, _, - let nextSelection + let nextSelection, + let forceDeleteBranchRequest ): analyticsClient.capture("worktree_deleted", [String: Any]?.none) let previousSelection = state.selectedWorktreeID @@ -424,6 +440,9 @@ extension RepositoriesFeature { state.worktreeInfoByID.removeValue(forKey: worktreeID) state.pinnedWorktreeIDs.removeAll { $0 == worktreeID } state.archivedWorktrees.removeAll { $0.id == worktreeID } + state.$prowlCreatedWorktreeIDs.withLock { + $0.removeAll { $0 == worktreeID } + } if var order = state.worktreeOrderByRepository[repositoryID] { order.removeAll { $0 == worktreeID } if order.isEmpty { @@ -482,6 +501,9 @@ extension RepositoriesFeature { } ) } + if let forceDeleteBranchRequest { + state.alert = forceDeleteBranchAlert(forceDeleteBranchRequest) + } return .concatenate( .merge(immediateEffects), .merge(followupEffects) @@ -491,6 +513,20 @@ extension RepositoriesFeature { state.deletingWorktreeIDs.remove(worktreeID) state.alert = messageAlert(title: "Unable to delete worktree", message: message) return .none + + case .forceDeleteBranchConfirmed(let request): + state.alert = nil + return .run { send in + do { + _ = try await gitClient.deleteLocalBranch(request.branchName, request.repositoryRootURL, true) + } catch { + await send(.worktreeLifecycle(.forceDeleteBranchFailed(error.localizedDescription))) + } + } + + case .forceDeleteBranchFailed(let message): + state.alert = messageAlert(title: "Unable to delete branch", message: message) + return .none } } @@ -513,3 +549,56 @@ private func archiveWorktreesAlertMessage() -> String { let shortcut = AppShortcuts.archivedWorktrees.display return "Find them later in Menu Bar > Worktrees > Archived Worktrees (\(shortcut))." } + +private func makeDeleteWorktreeConfirmation( + id: Int, + targets: [RepositoriesFeature.DeleteWorktreeTarget], + state: RepositoriesFeature.State +) -> DeleteWorktreeConfirmation { + @Shared(.settingsFile) var settingsFile + let count = targets.count + let allProwlCreated = targets.allSatisfy { target in + state.prowlCreatedWorktreeIDs.contains(target.worktreeID) + } + let defaultDeleteBranch = settingsFile.global.deleteBranchOnDeleteWorktree && allProwlCreated + if count == 1, + let target = targets.first, + let worktree = state.repositories[id: target.repositoryID]?.worktrees[id: target.worktreeID] + { + return DeleteWorktreeConfirmation( + id: id, + title: "Delete worktree?", + message: "Delete \(worktree.name)? The worktree directory will be removed.", + targets: targets, + deleteBranch: defaultDeleteBranch + ) + } + return DeleteWorktreeConfirmation( + id: id, + title: "Delete \(count) worktrees?", + message: "Delete \(count) worktrees? Their worktree directories will be removed.", + targets: targets, + deleteBranch: defaultDeleteBranch + ) +} + +private func forceDeleteBranchAlert(_ request: ForceDeleteBranchRequest) -> AlertState { + AlertState { + TextState("Force delete branch?") + } actions: { + ButtonState(role: .destructive, action: .confirmForceDeleteBranch(request)) { + TextState("Force Delete") + } + ButtonState(role: .cancel) { + TextState("Keep Branch") + } + } message: { + TextState( + """ + The worktree was deleted, but \(request.branchName) could not be deleted safely. + + \(request.errorMessage) + """ + ) + } +} diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift index 7a0ca063..d37f9885 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -55,6 +55,20 @@ struct PendingRenameBranchRequest: Equatable, Sendable { let worktreeID: Worktree.ID } +struct DeleteWorktreeConfirmation: Equatable, Identifiable { + let id: Int + let title: String + let message: String + let targets: [RepositoriesFeature.DeleteWorktreeTarget] + var deleteBranch: Bool +} + +struct ForceDeleteBranchRequest: Equatable { + let branchName: String + let repositoryRootURL: URL + let errorMessage: String +} + @Reducer struct RepositoriesFeature { enum CancelID { @@ -133,14 +147,20 @@ struct RepositoriesFeature { case unarchiveWorktree(Worktree.ID) case requestDeleteWorktree(Worktree.ID, Repository.ID) case requestDeleteWorktrees([DeleteWorktreeTarget]) - case deleteWorktreeConfirmed(Worktree.ID, Repository.ID) + case deleteWorktreePromptDeleteBranchChanged(Bool) + case deleteWorktreePromptConfirmed + case deleteWorktreePromptDismissed + case deleteWorktreeConfirmed(Worktree.ID, Repository.ID, deleteBranch: Bool) case worktreeDeleted( Worktree.ID, repositoryID: Repository.ID, selectionWasRemoved: Bool, - nextSelection: Worktree.ID? + nextSelection: Worktree.ID?, + forceDeleteBranchRequest: ForceDeleteBranchRequest? ) case deleteWorktreeFailed(String, worktreeID: Worktree.ID) + case forceDeleteBranchConfirmed(ForceDeleteBranchRequest) + case forceDeleteBranchFailed(String) } @CasePathable @@ -244,6 +264,9 @@ struct RepositoriesFeature { var remoteInfoByRepositoryID: [Repository.ID: GithubRemoteInfo] = [:] var codeHostByRepositoryID: [Repository.ID: CodeHost] = [:] var sidebarSelectedWorktreeIDs: Set = [] + @Shared(.appStorage("prowlCreatedWorktreeIDs")) var prowlCreatedWorktreeIDs: [Worktree.ID] = [] + var nextDeleteWorktreeConfirmationID = 0 + var deleteWorktreeConfirmation: DeleteWorktreeConfirmation? var nextPendingSidebarRevealID = 0 var pendingSidebarReveal: PendingSidebarReveal? var nextPendingRenameBranchRequestID = 0 @@ -377,8 +400,7 @@ struct RepositoriesFeature { enum Alert: Equatable { case confirmArchiveWorktree(Worktree.ID, Repository.ID) case confirmArchiveWorktrees([ArchiveWorktreeTarget]) - case confirmDeleteWorktree(Worktree.ID, Repository.ID) - case confirmDeleteWorktrees([DeleteWorktreeTarget]) + case confirmForceDeleteBranch(ForceDeleteBranchRequest) case confirmRemoveRepository(Repository.ID) } @@ -545,6 +567,7 @@ struct RepositoriesFeature { guard !expiredEntries.isEmpty else { return .none } + @Shared(.settingsFile) var settingsFile var deleteEffects: [Effect] = [] for entry in expiredEntries { guard @@ -558,7 +581,14 @@ struct RepositoriesFeature { continue } deleteEffects.append( - .send(.worktreeLifecycle(.deleteWorktreeConfirmed(worktree.id, repository.id))) + .send( + .worktreeLifecycle( + .deleteWorktreeConfirmed( + worktree.id, + repository.id, + deleteBranch: settingsFile.global.deleteBranchOnDeleteWorktree + )) + ) ) } guard !deleteEffects.isEmpty else { @@ -1098,15 +1128,8 @@ struct RepositoriesFeature { } ) - case .alert(.presented(.confirmDeleteWorktree(let worktreeID, let repositoryID))): - return .send(.worktreeLifecycle(.deleteWorktreeConfirmed(worktreeID, repositoryID))) - - case .alert(.presented(.confirmDeleteWorktrees(let targets))): - return .merge( - targets.map { target in - .send(.worktreeLifecycle(.deleteWorktreeConfirmed(target.worktreeID, target.repositoryID))) - } - ) + case .alert(.presented(.confirmForceDeleteBranch(let request))): + return .send(.worktreeLifecycle(.forceDeleteBranchConfirmed(request))) case .alert(.presented(.confirmRemoveRepository(let repositoryID))): guard let repository = state.repositories[id: repositoryID] else { @@ -1497,6 +1520,9 @@ struct RepositoriesFeature { let filteredWorktreeInfo = state.worktreeInfoByID.filter { availableWorktreeIDs.contains($0.key) } + state.$prowlCreatedWorktreeIDs.withLock { + $0.removeAll { !availableWorktreeIDs.contains($0) } + } let identifiedRepositories = IdentifiedArray(uniqueElements: repositories) if animated { withAnimation { @@ -2016,12 +2042,6 @@ extension RepositoriesFeature.State { if case .confirmArchiveWorktrees(let targets)? = button.action.action { return .confirmArchiveWorktrees(targets) } - if case .confirmDeleteWorktree(let worktreeID, let repositoryID)? = button.action.action { - return .confirmDeleteWorktree(worktreeID, repositoryID) - } - if case .confirmDeleteWorktrees(let targets)? = button.action.action { - return .confirmDeleteWorktrees(targets) - } } return nil } diff --git a/supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift b/supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift new file mode 100644 index 00000000..70c8250c --- /dev/null +++ b/supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift @@ -0,0 +1,51 @@ +import SwiftUI + +struct DeleteWorktreeConfirmationView: View { + let confirmation: DeleteWorktreeConfirmation + let onDeleteBranchChanged: (Bool) -> Void + let onCancel: () -> Void + let onDelete: () -> Void + + var body: some View { + VStack(alignment: .leading, spacing: 16) { + VStack(alignment: .leading, spacing: 6) { + Text(confirmation.title) + .font(.headline) + Text(confirmation.message) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + } + + Toggle( + "Also delete local branch", + isOn: Binding( + get: { confirmation.deleteBranch }, + set: onDeleteBranchChanged + ) + ) + .help("Try to delete the local branch with git branch -d after removing the worktree.") + + Text("Protected branches are kept. If safe branch deletion fails, Prowl asks before forcing it.") + .font(.footnote) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + + HStack { + Spacer() + Button("Cancel", role: .cancel) { + onCancel() + } + .keyboardShortcut(.cancelAction) + .help("Cancel") + + Button("Delete", role: .destructive) { + onDelete() + } + .keyboardShortcut(.defaultAction) + .help("Delete worktree") + } + } + .padding(24) + .frame(width: 420) + } +} diff --git a/supacode/Features/Settings/Models/GlobalSettings.swift b/supacode/Features/Settings/Models/GlobalSettings.swift index 53861d4c..ffbbf019 100644 --- a/supacode/Features/Settings/Models/GlobalSettings.swift +++ b/supacode/Features/Settings/Models/GlobalSettings.swift @@ -54,7 +54,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { analyticsEnabled: true, crashReportsEnabled: true, githubIntegrationEnabled: true, - deleteBranchOnDeleteWorktree: true, + deleteBranchOnDeleteWorktree: false, mergedWorktreeAction: nil, promptForWorktreeCreation: true, fetchOriginBeforeWorktreeCreation: true, diff --git a/supacode/Features/Settings/Views/WorktreeSettingsView.swift b/supacode/Features/Settings/Views/WorktreeSettingsView.swift index 73b25a63..b8b5ce58 100644 --- a/supacode/Features/Settings/Views/WorktreeSettingsView.swift +++ b/supacode/Features/Settings/Views/WorktreeSettingsView.swift @@ -50,11 +50,11 @@ struct WorktreeSettingsView: View { Section("Cleanup") { VStack(alignment: .leading) { Toggle( - "Also delete local branch when deleting a worktree", + "Preselect branch deletion for Prowl-created worktrees", isOn: $store.deleteBranchOnDeleteWorktree ) - .help("Delete the local branch when deleting a worktree") - Text("Removes the local branch along with the worktree. Remote branches must be deleted on GitHub.") + .help("Preselect local branch deletion for worktrees created by Prowl.") + Text("External worktrees stay unchecked by default. Remote branches must be deleted on GitHub.") .foregroundStyle(.secondary) Text("Uncommitted changes will be lost.") .foregroundStyle(.red) diff --git a/supacodeTests/AppFeatureCommandPaletteTests.swift b/supacodeTests/AppFeatureCommandPaletteTests.swift index deccdd61..ce8363e1 100644 --- a/supacodeTests/AppFeatureCommandPaletteTests.swift +++ b/supacodeTests/AppFeatureCommandPaletteTests.swift @@ -642,22 +642,16 @@ struct AppFeatureCommandPaletteTests { AppFeature() } - let expectedAlert = AlertState { - TextState("🚨 Delete worktree?") - } actions: { - ButtonState(role: .destructive, action: .confirmDeleteWorktree(worktree.id, repository.id)) { - TextState("Delete (⌘↩)") - } - ButtonState(role: .cancel) { - TextState("Cancel") - } - } message: { - TextState("Delete \(worktree.name)? This deletes the worktree directory and its local branch.") - } - await store.send(.commandPalette(.delegate(.deleteWorktree(worktree.id, repository.id)))) await store.receive(\.repositories.worktreeLifecycle.requestDeleteWorktree) { - $0.repositories.alert = expectedAlert + $0.repositories.deleteWorktreeConfirmation = DeleteWorktreeConfirmation( + id: 0, + title: "Delete worktree?", + message: "Delete \(worktree.name)? The worktree directory will be removed.", + targets: [RepositoriesFeature.DeleteWorktreeTarget(worktreeID: worktree.id, repositoryID: repository.id)], + deleteBranch: false + ) + $0.repositories.nextDeleteWorktreeConfirmationID = 1 } } diff --git a/supacodeTests/GitClientRemoveWorktreeTests.swift b/supacodeTests/GitClientRemoveWorktreeTests.swift new file mode 100644 index 00000000..ab92e416 --- /dev/null +++ b/supacodeTests/GitClientRemoveWorktreeTests.swift @@ -0,0 +1,109 @@ +import Foundation +import Testing + +@testable import supacode + +struct GitClientRemoveWorktreeTests { + @Test func removeWorktreeDoesNotDeleteMainBranch() async throws { + let store = ShellCallStore() + let shell = ShellClient( + run: { _, arguments, _ in + await store.record(arguments) + if arguments.contains("for-each-ref") { + return ShellOutput(stdout: "main\nfeature\n", stderr: "", exitCode: 0) + } + return ShellOutput(stdout: "", stderr: "", exitCode: 0) + }, + runLoginImpl: { _, _, _, _ in ShellOutput(stdout: "", stderr: "", exitCode: 0) } + ) + let client = GitClient(shell: shell) + let worktree = Worktree( + id: "/tmp/repo-main-copy", + name: "main", + detail: "../repo-main-copy", + workingDirectory: URL(fileURLWithPath: "/tmp/repo-main-copy"), + repositoryRootURL: URL(fileURLWithPath: "/tmp/repo") + ) + + _ = try await client.removeWorktree(worktree, deleteBranch: true) + + let calls = await store.calls + #expect(calls.contains { $0.contains("worktree") && $0.contains("remove") }) + #expect(calls.contains { $0.contains("for-each-ref") }) + #expect(!calls.contains { $0.suffix(3) == ["branch", "-d", "main"] }) + #expect(!calls.contains { $0.suffix(3) == ["branch", "-D", "main"] }) + } + + @Test func removeWorktreeDeletesNonProtectedLocalBranchWhenRequested() async throws { + let store = ShellCallStore() + let shell = ShellClient( + run: { _, arguments, _ in + await store.record(arguments) + if arguments.contains("for-each-ref") { + return ShellOutput(stdout: "main\nfeature\n", stderr: "", exitCode: 0) + } + return ShellOutput(stdout: "", stderr: "", exitCode: 0) + }, + runLoginImpl: { _, _, _, _ in ShellOutput(stdout: "", stderr: "", exitCode: 0) } + ) + let client = GitClient(shell: shell) + let worktree = Worktree( + id: "/tmp/repo-feature", + name: "feature", + detail: "../repo-feature", + workingDirectory: URL(fileURLWithPath: "/tmp/repo-feature"), + repositoryRootURL: URL(fileURLWithPath: "/tmp/repo") + ) + + _ = try await client.removeWorktree(worktree, deleteBranch: true) + + let calls = await store.calls + #expect(calls.contains { $0.suffix(3) == ["branch", "-d", "feature"] }) + } + + @Test func forceDeleteLocalBranchUsesForceFlag() async throws { + let store = ShellCallStore() + let shell = ShellClient( + run: { _, arguments, _ in + await store.record(arguments) + if arguments.contains("for-each-ref") { + return ShellOutput(stdout: "feature\n", stderr: "", exitCode: 0) + } + return ShellOutput(stdout: "", stderr: "", exitCode: 0) + }, + runLoginImpl: { _, _, _, _ in ShellOutput(stdout: "", stderr: "", exitCode: 0) } + ) + let client = GitClient(shell: shell) + + let outcome = try await client.deleteLocalBranch( + named: "feature", + for: URL(fileURLWithPath: "/tmp/repo"), + force: true + ) + + #expect(outcome == .deleted) + let calls = await store.calls + #expect(calls.contains { $0.suffix(3) == ["branch", "-D", "feature"] }) + } + + @Test func safeDeleteLocalBranchPropagatesGitFailure() async { + let shell = ShellClient( + run: { _, arguments, _ in + if arguments.contains("for-each-ref") { + return ShellOutput(stdout: "feature\n", stderr: "", exitCode: 0) + } + throw ShellClientError(command: "git branch -d feature", stdout: "", stderr: "not fully merged", exitCode: 1) + }, + runLoginImpl: { _, _, _, _ in ShellOutput(stdout: "", stderr: "", exitCode: 0) } + ) + let client = GitClient(shell: shell) + + await #expect(throws: GitClientError.self) { + _ = try await client.deleteLocalBranch( + named: "feature", + for: URL(fileURLWithPath: "/tmp/repo"), + force: false + ) + } + } +} diff --git a/supacodeTests/RepositoriesFeatureTests.swift b/supacodeTests/RepositoriesFeatureTests.swift index db54f2a4..4fb38c9e 100644 --- a/supacodeTests/RepositoriesFeatureTests.swift +++ b/supacodeTests/RepositoriesFeatureTests.swift @@ -2262,22 +2262,99 @@ struct RepositoriesFeatureTests { RepositoriesFeature() } - let expectedAlert = AlertState { - TextState("🚨 Delete worktree?") - } actions: { - ButtonState(role: .destructive, action: .confirmDeleteWorktree(worktree.id, repository.id)) { - TextState("Delete (⌘↩)") + await store.send(.worktreeLifecycle(.requestDeleteWorktree(worktree.id, repository.id))) { + $0.deleteWorktreeConfirmation = DeleteWorktreeConfirmation( + id: 0, + title: "Delete worktree?", + message: "Delete \(worktree.name)? The worktree directory will be removed.", + targets: [RepositoriesFeature.DeleteWorktreeTarget(worktreeID: worktree.id, repositoryID: repository.id)], + deleteBranch: false + ) + $0.nextDeleteWorktreeConfirmationID = 1 + } + } + + @Test(.dependencies) func requestDeleteProwlCreatedWorktreeCanPreselectBranchDeletion() async { + let worktree = makeWorktree(id: "/tmp/wt", name: "owl") + let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) + var state = makeState(repositories: [repository]) + state.$prowlCreatedWorktreeIDs.withLock { + $0 = [worktree.id] + } + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { + $0.global.deleteBranchOnDeleteWorktree = true + } + let store = TestStore(initialState: state) { + RepositoriesFeature() + } + + await store.send(.worktreeLifecycle(.requestDeleteWorktree(worktree.id, repository.id))) { + $0.deleteWorktreeConfirmation = DeleteWorktreeConfirmation( + id: 0, + title: "Delete worktree?", + message: "Delete \(worktree.name)? The worktree directory will be removed.", + targets: [RepositoriesFeature.DeleteWorktreeTarget(worktreeID: worktree.id, repositoryID: repository.id)], + deleteBranch: true + ) + $0.nextDeleteWorktreeConfirmationID = 1 + } + } + + @Test(.dependencies) func deletePromptConfirmedAsksBeforeForceDeletingBranch() async { + let repoRoot = "/tmp/repo" + let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot) + let worktree = makeWorktree(id: "\(repoRoot)/feature", name: "feature", repoRoot: repoRoot) + let repository = makeRepository(id: repoRoot, worktrees: [mainWorktree, worktree]) + var state = makeState(repositories: [repository]) + state.deleteWorktreeConfirmation = DeleteWorktreeConfirmation( + id: 0, + title: "Delete worktree?", + message: "Delete feature? The worktree directory will be removed.", + targets: [RepositoriesFeature.DeleteWorktreeTarget(worktreeID: worktree.id, repositoryID: repository.id)], + deleteBranch: true + ) + let forceDeleteAttempts = LockIsolated<[Bool]>([]) + let store = TestStore(initialState: state) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.removeWorktree = { worktree, deleteBranch in + #expect(deleteBranch == false) + return worktree.workingDirectory } - ButtonState(role: .cancel) { - TextState("Cancel") + $0.gitClient.deleteLocalBranch = { _, _, force in + forceDeleteAttempts.withValue { $0.append(force) } + if force { + return .deleted + } + throw GitClientError.commandFailed(command: "git branch -d feature", message: "not fully merged") } - } message: { - TextState("Delete \(worktree.name)? This deletes the worktree directory and its local branch.") + $0.gitClient.worktrees = { _ in [mainWorktree] } } + store.exhaustivity = .off - await store.send(.worktreeLifecycle(.requestDeleteWorktree(worktree.id, repository.id))) { - $0.alert = expectedAlert + await store.send(.worktreeLifecycle(.deleteWorktreePromptConfirmed)) { + $0.deleteWorktreeConfirmation = nil + } + await store.receive(\.worktreeLifecycle.deleteWorktreeConfirmed) { + $0.deletingWorktreeIDs = [worktree.id] + } + await store.receive(\.worktreeLifecycle.worktreeDeleted) { + $0.deletingWorktreeIDs = [] + $0.repositories = [makeRepository(id: repoRoot, worktrees: [mainWorktree])] + #expect($0.alert != nil) } + await store.send( + .alert( + .presented( + .confirmForceDeleteBranch( + ForceDeleteBranchRequest( + branchName: "feature", + repositoryRootURL: URL(fileURLWithPath: repoRoot), + errorMessage: "Git command failed: git branch -d feature\nnot fully merged" + ))))) + + #expect(forceDeleteAttempts.value == [false, true]) } @Test func requestDeleteMainWorktreeShowsNotAllowedAlert() async { @@ -2314,21 +2391,15 @@ struct RepositoriesFeatureTests { RepositoriesFeature() } - let expectedAlert = AlertState { - TextState("🚨 Delete 2 worktrees?") - } actions: { - ButtonState(role: .destructive, action: .confirmDeleteWorktrees(targets)) { - TextState("Delete 2 (⌘↩)") - } - ButtonState(role: .cancel) { - TextState("Cancel") - } - } message: { - TextState("Delete 2 worktrees? This deletes the worktree directories and their local branches.") - } - await store.send(.worktreeLifecycle(.requestDeleteWorktrees(targets))) { - $0.alert = expectedAlert + $0.deleteWorktreeConfirmation = DeleteWorktreeConfirmation( + id: 0, + title: "Delete 2 worktrees?", + message: "Delete 2 worktrees? Their worktree directories will be removed.", + targets: targets, + deleteBranch: false + ) + $0.nextDeleteWorktreeConfirmationID = 1 } } @@ -3228,7 +3299,8 @@ struct RepositoriesFeatureTests { removedWorktree.id, repositoryID: repository.id, selectionWasRemoved: false, - nextSelection: nil + nextSelection: nil, + forceDeleteBranchRequest: nil )) ) { $0.deletingWorktreeIDs = [] @@ -3268,7 +3340,8 @@ struct RepositoriesFeatureTests { removedWorktree.id, repositoryID: repository.id, selectionWasRemoved: false, - nextSelection: nil + nextSelection: nil, + forceDeleteBranchRequest: nil )) ) { $0.deletingWorktreeIDs = [] @@ -3320,6 +3393,9 @@ struct RepositoriesFeatureTests { $0.pendingWorktrees = [] $0.selection = .worktree(newWorktree.id) $0.sidebarSelectedWorktreeIDs = [newWorktree.id] + $0.$prowlCreatedWorktreeIDs.withLock { + $0.append(newWorktree.id) + } $0.repositories = [updatedRepository] } diff --git a/supacodeTests/SettingsFilePersistenceTests.swift b/supacodeTests/SettingsFilePersistenceTests.swift index cf2ddbea..378313b6 100644 --- a/supacodeTests/SettingsFilePersistenceTests.swift +++ b/supacodeTests/SettingsFilePersistenceTests.swift @@ -159,7 +159,7 @@ struct SettingsFilePersistenceTests { #expect(settings.global.analyticsEnabled == true) #expect(settings.global.crashReportsEnabled == true) #expect(settings.global.githubIntegrationEnabled == true) - #expect(settings.global.deleteBranchOnDeleteWorktree == true) + #expect(settings.global.deleteBranchOnDeleteWorktree == false) #expect(settings.global.mergedWorktreeAction == nil) #expect(settings.global.promptForWorktreeCreation == true) #expect(settings.global.defaultWorktreeBaseDirectoryPath == nil)