diff --git a/docs-ai/019-worktree-creation-and-lifecycle/003-safe-branch-deletion-and-cleanup.md b/docs-ai/019-worktree-creation-and-lifecycle/003-safe-branch-deletion-and-cleanup.md index 63d76adc..317423cb 100644 --- a/docs-ai/019-worktree-creation-and-lifecycle/003-safe-branch-deletion-and-cleanup.md +++ b/docs-ai/019-worktree-creation-and-lifecycle/003-safe-branch-deletion-and-cleanup.md @@ -1,4 +1,4 @@ -# 019 — Amendment: Safe Branch Deletion & Cleanup Hardening (#375, #383) +# 019 — Amendment: Safe Branch Deletion & Cleanup Hardening (#375, #383; follow-up 2026-07) ## Context @@ -32,19 +32,51 @@ PR #383 (merged 2026-06-03, "Harden failed worktree cleanup"): - Only relocate existing worktree directories that actually contain `.git` metadata. - Failed-creation cleanup no longer requests branch deletion at all. +## Follow-up (2026-07): split manual preselect from automatic cleanup + +The single `deleteBranchOnDeleteWorktree` setting from #375 drove two unrelated +behaviors: preselecting the manual dialog toggle (gated on Prowl-created worktrees) +and enabling branch deletion during automatic cleanup. The Prowl-created gate on the +*manual* preselect proved too subtle — the checkbox was sometimes checked, sometimes +not, with no cue in the dialog (it confused even the author). The manual path already +has three backstops (visible toggle, `git branch -d` safe delete, force-delete +confirmation), so ownership gating added little safety there. It remains essential on +the automatic paths, which run with no dialog at all. + +Decisions: + +- **Manual dialog**: the toggle now simply remembers the last *confirmed* choice + (`@Shared(.appStorage("deleteBranchOnManualWorktreeDelete"))`, default off). No + settings toggle, no Prowl-created gating. Cancel does not update the memory. +- **Automatic cleanup** (merged-PR `.delete` action and archived auto-delete expiry): + controlled by the new explicit `deleteBranchOnAutomaticCleanup` setting (default + off), still additionally gated on `prowlCreatedWorktreeIDs` — Prowl never silently + deletes a branch it did not create. +- **Migration**: decode falls back from `deleteBranchOnAutomaticCleanup` to the legacy + `deleteBranchOnDeleteWorktree` key, because an opted-in user's expressed intent + included automatic-cleanup branch deletion (the old settings copy said the merged + `.delete` action "follows" it); letting it silently reset to off would be a worse + surprise than the rename. The manual memory intentionally starts fresh at off — the + old semantics ("preselect for Prowl-created only") don't map onto "remember last + choice". The legacy key is no longer written. + ## Refs -- PRs #375, #383; tests in `supacodeTests/GitClientRemoveWorktreeTests` and - `RepositoriesFeatureTests`. +- PRs #375, #383; follow-up PR splitting the settings (2026-07); tests in + `supacodeTests/GitClientRemoveWorktreeTests`, `RepositoriesFeatureTests`, and + `SettingsFilePersistenceTests` (legacy-key migration). ## Current state Deletion flow and `ForceDeleteBranchRequest` handling in -`supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift`; -sheet UI in `supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift`; +`supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift` +(manual preselect + remember-on-confirm live here); sheet UI in +`supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift`; `deleteLocalBranch` plus the porcelain-match/`.git`-metadata guards (`relocateWorktreeDirectory`) in `supacode/Clients/Git/GitClient.swift`. `prowlCreatedWorktreeIDs` is registered on creation in -`RepositoriesFeature+WorktreeCreation.swift` and consulted in -`RepositoriesFeature+CoreReducer.swift` / `+GithubIntegration.swift` / -`+WorktreeLifecycle.swift`. +`RepositoriesFeature+WorktreeCreation.swift` and consulted by the automatic-cleanup +paths in `RepositoriesFeature+CoreReducer.swift` / `+GithubIntegration.swift`. +`deleteBranchOnAutomaticCleanup` (with the legacy-key fallback) lives in +`supacode/Features/Settings/Models/GlobalSettings.swift`; the settings UI is in +`WorktreeSettingsView.swift`. diff --git a/docs/components/repositories-and-worktrees.md b/docs/components/repositories-and-worktrees.md index 19a108f1..8fde08ac 100644 --- a/docs/components/repositories-and-worktrees.md +++ b/docs/components/repositories-and-worktrees.md @@ -128,8 +128,8 @@ Deleting removes the worktree directory (and optionally its branch). - **Right-click** the row → "Delete Worktree", or **`⌘⇧⌫`**. - A confirmation dialog offers an **"Also delete local branch"** toggle (its - tooltip notes `git branch -d`). Default behavior comes from - `deleteBranchOnDeleteWorktree`. + tooltip notes `git branch -d`). The toggle remembers the last confirmed choice + (`deleteBranchOnManualWorktreeDelete` in UserDefaults; defaults to off). - Prowl removes the worktree (relocating + `git worktree prune` if needed), then verifies that Git no longer registers it. Cleanup failures keep the worktree visible and show Git's error. If branch deletion is rejected because the branch @@ -198,7 +198,7 @@ list. Highlights: - `defaultWorktreeBaseDirectoryPath` / per-repo `worktreeBaseDirectoryPath` - per-repo `worktreeBaseRef` (default base branch) - `copyIgnoredOnWorktreeCreate`, `copyUntrackedOnWorktreeCreate` -- `deleteBranchOnDeleteWorktree`, `mergedWorktreeAction`, `archivedAutoDeletePeriod` +- `deleteBranchOnAutomaticCleanup`, `mergedWorktreeAction`, `archivedAutoDeletePeriod` - per-repo `setupScript`, `archiveScript`, `openActionID`, `customTitle` ## Gotchas for agents diff --git a/docs/reference/settings-fields.md b/docs/reference/settings-fields.md index 277938ba..b00761fe 100644 --- a/docs/reference/settings-fields.md +++ b/docs/reference/settings-fields.md @@ -38,7 +38,7 @@ JSON is pretty-printed with sorted keys. Legacy `~/.supacode` is migrated to | `analyticsEnabled` | Bool | `true` | Send usage analytics (PostHog; off in Debug). | | `crashReportsEnabled` | Bool | `true` | Send crash reports (Sentry). | | `githubIntegrationEnabled` | Bool | `true` | Enable GitHub/PR features (via `gh`). | -| `deleteBranchOnDeleteWorktree` | Bool | `false` | Default "delete branch" when deleting a worktree. | +| `deleteBranchOnAutomaticCleanup` | Bool | `false` | Delete the local branch when automatic cleanup (merged-PR delete action, archived auto-delete) removes a Prowl-created worktree. Migrates the legacy `deleteBranchOnDeleteWorktree` key. The manual delete dialog is independent: it remembers the last confirmed choice in UserDefaults (`deleteBranchOnManualWorktreeDelete`). | | `mergedWorktreeAction` | enum? | `nil` | What to do with a merged worktree (e.g. auto-archive); `nil` = ask. | | `promptForWorktreeCreation` | Bool | `true` | Show the creation dialog vs. auto-create. | | `fetchOriginBeforeWorktreeCreation` | Bool | `true` | `git fetch` before creating a worktree. | diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift index 4e2d3c3b..21981627 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+CoreReducer.swift @@ -136,7 +136,7 @@ extension RepositoriesFeature { continue } let shouldDeleteBranch = - settingsFile.global.deleteBranchOnDeleteWorktree + settingsFile.global.deleteBranchOnAutomaticCleanup && state.prowlCreatedWorktreeIDs.contains(worktree.id) deleteEffects.append( .send( diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift index 1f26a6c0..b34cea02 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+GithubIntegration.swift @@ -290,7 +290,7 @@ extension RepositoriesFeature { return .merge( mergedWorktreeIDs.map { worktreeID in let shouldDeleteBranch = - settingsFile.global.deleteBranchOnDeleteWorktree + settingsFile.global.deleteBranchOnAutomaticCleanup && state.prowlCreatedWorktreeIDs.contains(worktreeID) return .send( .worktreeLifecycle( diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift index 5ad3d2a8..35741f0f 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeLifecycle.swift @@ -349,6 +349,7 @@ extension RepositoriesFeature { return .none } state.deleteWorktreeConfirmation = nil + state.$deleteBranchOnManualWorktreeDelete.withLock { $0 = confirmation.deleteBranch } return .merge( confirmation.targets.map { target in .send( @@ -560,12 +561,8 @@ private func makeDeleteWorktreeConfirmation( 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 + let defaultDeleteBranch = state.deleteBranchOnManualWorktreeDelete if count == 1, let target = targets.first, let worktree = state.repositories[id: target.repositoryID]?.worktrees[id: target.worktreeID] diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift index b913f591..7ebe0a28 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -352,6 +352,9 @@ struct RepositoriesFeature { var codeHostByRepositoryID: [Repository.ID: CodeHost] = [:] var sidebarSelectedWorktreeIDs: Set = [] @Shared(.appStorage("prowlCreatedWorktreeIDs")) var prowlCreatedWorktreeIDs: [Worktree.ID] = [] + /// Last "Also delete local branch" choice confirmed in the manual delete + /// dialog; preselects the toggle next time. + @Shared(.appStorage("deleteBranchOnManualWorktreeDelete")) var deleteBranchOnManualWorktreeDelete = false var nextDeleteWorktreeConfirmationID = 0 var deleteWorktreeConfirmation: DeleteWorktreeConfirmation? var removeWorkspaceConfirmation: RemoveWorkspaceConfirmation? diff --git a/supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift b/supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift index 98ac9573..2b6de67e 100644 --- a/supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift +++ b/supacode/Features/Repositories/Views/DeleteWorktreeConfirmationView.swift @@ -25,10 +25,13 @@ struct DeleteWorktreeConfirmationView: View { ) .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) + Text( + "Protected branches are kept. If safe branch deletion fails, Prowl asks before forcing it. " + + "Your choice is remembered for the next delete." + ) + .font(.footnote) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) HStack { Spacer() diff --git a/supacode/Features/Settings/Models/GlobalSettings.swift b/supacode/Features/Settings/Models/GlobalSettings.swift index d0eb264b..5627abd0 100644 --- a/supacode/Features/Settings/Models/GlobalSettings.swift +++ b/supacode/Features/Settings/Models/GlobalSettings.swift @@ -14,7 +14,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { var analyticsEnabled: Bool var crashReportsEnabled: Bool var githubIntegrationEnabled: Bool - var deleteBranchOnDeleteWorktree: Bool + var deleteBranchOnAutomaticCleanup: Bool var mergedWorktreeAction: MergedWorktreeAction? var promptForWorktreeCreation: Bool var fetchOriginBeforeWorktreeCreation: Bool @@ -59,7 +59,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { analyticsEnabled: true, crashReportsEnabled: true, githubIntegrationEnabled: true, - deleteBranchOnDeleteWorktree: false, + deleteBranchOnAutomaticCleanup: false, mergedWorktreeAction: nil, promptForWorktreeCreation: true, fetchOriginBeforeWorktreeCreation: true, @@ -103,7 +103,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { analyticsEnabled: Bool, crashReportsEnabled: Bool, githubIntegrationEnabled: Bool, - deleteBranchOnDeleteWorktree: Bool, + deleteBranchOnAutomaticCleanup: Bool, mergedWorktreeAction: MergedWorktreeAction? = nil, promptForWorktreeCreation: Bool, fetchOriginBeforeWorktreeCreation: Bool = true, @@ -145,7 +145,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { self.analyticsEnabled = analyticsEnabled self.crashReportsEnabled = crashReportsEnabled self.githubIntegrationEnabled = githubIntegrationEnabled - self.deleteBranchOnDeleteWorktree = deleteBranchOnDeleteWorktree + self.deleteBranchOnAutomaticCleanup = deleteBranchOnAutomaticCleanup self.mergedWorktreeAction = mergedWorktreeAction self.promptForWorktreeCreation = promptForWorktreeCreation self.fetchOriginBeforeWorktreeCreation = fetchOriginBeforeWorktreeCreation @@ -190,7 +190,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { try container.encode(analyticsEnabled, forKey: .analyticsEnabled) try container.encode(crashReportsEnabled, forKey: .crashReportsEnabled) try container.encode(githubIntegrationEnabled, forKey: .githubIntegrationEnabled) - try container.encode(deleteBranchOnDeleteWorktree, forKey: .deleteBranchOnDeleteWorktree) + try container.encode(deleteBranchOnAutomaticCleanup, forKey: .deleteBranchOnAutomaticCleanup) try container.encodeIfPresent(mergedWorktreeAction, forKey: .mergedWorktreeAction) try container.encode(promptForWorktreeCreation, forKey: .promptForWorktreeCreation) try container.encode(fetchOriginBeforeWorktreeCreation, forKey: .fetchOriginBeforeWorktreeCreation) @@ -236,7 +236,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { case analyticsEnabled case crashReportsEnabled case githubIntegrationEnabled - case deleteBranchOnDeleteWorktree + case deleteBranchOnAutomaticCleanup case mergedWorktreeAction case promptForWorktreeCreation case fetchOriginBeforeWorktreeCreation @@ -267,6 +267,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { // Legacy keys for migration case automaticallyArchiveMergedWorktrees case notificationSoundEnabled + case deleteBranchOnDeleteWorktree } init(from decoder: any Decoder) throws { @@ -308,9 +309,7 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { githubIntegrationEnabled = try container.decodeIfPresent(Bool.self, forKey: .githubIntegrationEnabled) ?? Self.default.githubIntegrationEnabled - deleteBranchOnDeleteWorktree = - try container.decodeIfPresent(Bool.self, forKey: .deleteBranchOnDeleteWorktree) - ?? Self.default.deleteBranchOnDeleteWorktree + deleteBranchOnAutomaticCleanup = try Self.decodeDeleteBranchOnAutomaticCleanup(from: container) mergedWorktreeAction = try Self.decodeMergedWorktreeAction(from: container) promptForWorktreeCreation = try container.decodeIfPresent(Bool.self, forKey: .promptForWorktreeCreation) @@ -431,6 +430,21 @@ nonisolated struct GlobalSettings: Codable, Equatable, Sendable { return Self.default.notificationSound } + /// Falls back to the legacy `deleteBranchOnDeleteWorktree` key: users who + /// opted into branch deletion before the setting was split keep branch + /// deletion during automatic cleanup. + private static func decodeDeleteBranchOnAutomaticCleanup( + from container: KeyedDecodingContainer + ) throws -> Bool { + if let value = try container.decodeIfPresent(Bool.self, forKey: .deleteBranchOnAutomaticCleanup) { + return value + } + if let legacy = try container.decodeIfPresent(Bool.self, forKey: .deleteBranchOnDeleteWorktree) { + return legacy + } + return Self.default.deleteBranchOnAutomaticCleanup + } + private static func decodeMergedWorktreeAction( from container: KeyedDecodingContainer ) throws -> MergedWorktreeAction? { diff --git a/supacode/Features/Settings/Reducer/SettingsFeature.swift b/supacode/Features/Settings/Reducer/SettingsFeature.swift index 5e1be01b..57e9404c 100644 --- a/supacode/Features/Settings/Reducer/SettingsFeature.swift +++ b/supacode/Features/Settings/Reducer/SettingsFeature.swift @@ -21,7 +21,7 @@ struct SettingsFeature { var analyticsEnabled: Bool var crashReportsEnabled: Bool var githubIntegrationEnabled: Bool - var deleteBranchOnDeleteWorktree: Bool + var deleteBranchOnAutomaticCleanup: Bool var mergedWorktreeAction: MergedWorktreeAction? var archivedAutoDeletePeriod: AutoDeletePeriod? var promptForWorktreeCreation: Bool @@ -79,7 +79,7 @@ struct SettingsFeature { analyticsEnabled = settings.analyticsEnabled crashReportsEnabled = settings.crashReportsEnabled githubIntegrationEnabled = settings.githubIntegrationEnabled - deleteBranchOnDeleteWorktree = settings.deleteBranchOnDeleteWorktree + deleteBranchOnAutomaticCleanup = settings.deleteBranchOnAutomaticCleanup mergedWorktreeAction = settings.mergedWorktreeAction archivedAutoDeletePeriod = settings.archivedAutoDeletePeriod promptForWorktreeCreation = settings.promptForWorktreeCreation @@ -127,7 +127,7 @@ struct SettingsFeature { analyticsEnabled: analyticsEnabled, crashReportsEnabled: crashReportsEnabled, githubIntegrationEnabled: githubIntegrationEnabled, - deleteBranchOnDeleteWorktree: deleteBranchOnDeleteWorktree, + deleteBranchOnAutomaticCleanup: deleteBranchOnAutomaticCleanup, mergedWorktreeAction: mergedWorktreeAction, promptForWorktreeCreation: promptForWorktreeCreation, fetchOriginBeforeWorktreeCreation: fetchRemoteBeforeWorktreeCreation, @@ -248,7 +248,7 @@ struct SettingsFeature { state.analyticsEnabled = normalizedSettings.analyticsEnabled state.crashReportsEnabled = normalizedSettings.crashReportsEnabled state.githubIntegrationEnabled = normalizedSettings.githubIntegrationEnabled - state.deleteBranchOnDeleteWorktree = normalizedSettings.deleteBranchOnDeleteWorktree + state.deleteBranchOnAutomaticCleanup = normalizedSettings.deleteBranchOnAutomaticCleanup state.mergedWorktreeAction = normalizedSettings.mergedWorktreeAction state.archivedAutoDeletePeriod = normalizedSettings.archivedAutoDeletePeriod state.promptForWorktreeCreation = normalizedSettings.promptForWorktreeCreation diff --git a/supacode/Features/Settings/Views/WorktreeSettingsView.swift b/supacode/Features/Settings/Views/WorktreeSettingsView.swift index b8b5ce58..7d4b8966 100644 --- a/supacode/Features/Settings/Views/WorktreeSettingsView.swift +++ b/supacode/Features/Settings/Views/WorktreeSettingsView.swift @@ -50,14 +50,15 @@ struct WorktreeSettingsView: View { Section("Cleanup") { VStack(alignment: .leading) { Toggle( - "Preselect branch deletion for Prowl-created worktrees", - isOn: $store.deleteBranchOnDeleteWorktree + "Delete local branch during automatic cleanup", + isOn: $store.deleteBranchOnAutomaticCleanup ) - .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) + .help("Delete the local branch when automatic cleanup removes a Prowl-created worktree.") + Text( + "Applies when merged or expired worktrees are cleaned up automatically, and only to branches " + + "created by Prowl. The manual delete dialog remembers your last choice instead." + ) + .foregroundStyle(.secondary) } .frame(maxWidth: .infinity, alignment: .leading) Picker(selection: $store.mergedWorktreeAction) { @@ -71,7 +72,7 @@ struct WorktreeSettingsView: View { case .archive: Text("Archives worktrees when their pull requests are merged.") case .delete: - Text("Follows the \"Also delete local branch when deleting a worktree\" option above.") + Text("Deletes worktrees when their pull requests are merged. Uncommitted changes will be lost.") case nil: EmptyView() } diff --git a/supacodeTests/RepositoriesFeatureTests.swift b/supacodeTests/RepositoriesFeatureTests.swift index 0ecfdc52..225dc7ed 100644 --- a/supacodeTests/RepositoriesFeatureTests.swift +++ b/supacodeTests/RepositoriesFeatureTests.swift @@ -4114,17 +4114,11 @@ struct RepositoriesFeatureTests { } } - @Test(.dependencies) func requestDeleteProwlCreatedWorktreeCanPreselectBranchDeletion() async { + @Test(.dependencies) func requestDeleteWorktreePreselectsRememberedBranchChoice() async { let worktree = makeWorktree(id: "/tmp/wt", name: "owl") let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) let state = makeState(repositories: [repository]) - state.$prowlCreatedWorktreeIDs.withLock { - $0 = [worktree.id] - } - @Shared(.settingsFile) var settingsFile - $settingsFile.withLock { - $0.global.deleteBranchOnDeleteWorktree = true - } + state.$deleteBranchOnManualWorktreeDelete.withLock { $0 = true } let store = TestStore(initialState: state) { RepositoriesFeature() } @@ -4144,6 +4138,70 @@ struct RepositoriesFeatureTests { } } + @Test(.dependencies) func deletePromptConfirmedRemembersBranchChoice() 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 store = TestStore(initialState: state) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.removeWorktree = { worktree, _ in worktree.workingDirectory } + $0.gitClient.deleteLocalBranch = { _, _, _ in .deleted } + $0.gitClient.worktrees = { _ in [mainWorktree] } + } + store.exhaustivity = .off + + await store.send(.worktreeLifecycle(.deleteWorktreePromptConfirmed)) { + $0.deleteWorktreeConfirmation = nil + $0.$deleteBranchOnManualWorktreeDelete.withLock { $0 = true } + } + await store.receive(\.worktreeLifecycle.worktreeDeleted) + } + + @Test(.dependencies) func deletePromptConfirmedRemembersUncheckedBranchChoice() 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: false + ) + state.$deleteBranchOnManualWorktreeDelete.withLock { $0 = true } + let store = TestStore(initialState: state) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.removeWorktree = { worktree, _ in worktree.workingDirectory } + $0.gitClient.worktrees = { _ in [mainWorktree] } + } + store.exhaustivity = .off + + await store.send(.worktreeLifecycle(.deleteWorktreePromptConfirmed)) { + $0.deleteWorktreeConfirmation = nil + $0.$deleteBranchOnManualWorktreeDelete.withLock { $0 = false } + } + await store.receive(\.worktreeLifecycle.worktreeDeleted) + } + @Test(.dependencies) func deletePromptConfirmedAsksBeforeForceDeletingBranch() async { let repoRoot = "/tmp/repo" let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot) @@ -5496,7 +5554,7 @@ struct RepositoriesFeatureTests { state.mergedWorktreeAction = .delete @Shared(.settingsFile) var settingsFile $settingsFile.withLock { - $0.global.deleteBranchOnDeleteWorktree = true + $0.global.deleteBranchOnAutomaticCleanup = true } let store = TestStore(initialState: state) { RepositoriesFeature() @@ -5523,6 +5581,51 @@ struct RepositoriesFeatureTests { await store.receive(\.worktreeLifecycle.worktreeDeleted) } + @Test(.dependencies) func repositoryPullRequestsLoadedAutoDeleteDeletesProwlCreatedBranch() async { + let repoRoot = "/tmp/repo" + let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot) + let prowlWorktree = makeWorktree( + id: "\(repoRoot)/feature", + name: "feature", + repoRoot: repoRoot + ) + let repository = makeRepository(id: repoRoot, worktrees: [mainWorktree, prowlWorktree]) + var state = makeState(repositories: [repository]) + state.mergedWorktreeAction = .delete + state.$prowlCreatedWorktreeIDs.withLock { $0 = [prowlWorktree.id] } + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { + $0.global.deleteBranchOnAutomaticCleanup = true + } + let deletedBranches = LockIsolated<[String]>([]) + let store = TestStore(initialState: state) { + RepositoriesFeature() + } withDependencies: { + $0.gitClient.removeWorktree = { worktree, _ in worktree.workingDirectory } + $0.gitClient.deleteLocalBranch = { branchName, _, force in + #expect(force == false) + deletedBranches.withValue { $0.append(branchName) } + return .deleted + } + } + store.exhaustivity = .off + let mergedPullRequest = makePullRequest(state: "MERGED", headRefName: prowlWorktree.name) + + await store.send( + .githubIntegration( + .repositoryPullRequestsLoaded( + repositoryID: repository.id, + pullRequestsByWorktreeID: [prowlWorktree.id: mergedPullRequest] + )) + ) + await store.receive(\.worktreeLifecycle.deleteWorktreeConfirmed) { + $0.deletingWorktreeIDs = [prowlWorktree.id] + } + await store.receive(\.worktreeLifecycle.worktreeDeleted) + + #expect(deletedBranches.value == [prowlWorktree.name]) + } + @Test func repositoryPullRequestsLoadedSkipsAutoDeleteForMainWorktree() async { let repoRoot = "/tmp/repo" let mainWorktree = makeWorktree(id: repoRoot, name: "main", repoRoot: repoRoot) @@ -6662,7 +6765,7 @@ struct RepositoriesFeatureTests { state.archivedAutoDeletePeriod = .oneDay @Shared(.settingsFile) var settingsFile $settingsFile.withLock { - $0.global.deleteBranchOnDeleteWorktree = true + $0.global.deleteBranchOnAutomaticCleanup = true } let store = TestStore(initialState: state) { RepositoriesFeature() diff --git a/supacodeTests/SettingsFeatureTests.swift b/supacodeTests/SettingsFeatureTests.swift index 64c20b57..99ce21e3 100644 --- a/supacodeTests/SettingsFeatureTests.swift +++ b/supacodeTests/SettingsFeatureTests.swift @@ -22,7 +22,7 @@ struct SettingsFeatureTests { analyticsEnabled: false, crashReportsEnabled: true, githubIntegrationEnabled: true, - deleteBranchOnDeleteWorktree: false, + deleteBranchOnAutomaticCleanup: false, mergedWorktreeAction: .archive, promptForWorktreeCreation: true ) @@ -46,7 +46,7 @@ struct SettingsFeatureTests { $0.analyticsEnabled = false $0.crashReportsEnabled = true $0.githubIntegrationEnabled = true - $0.deleteBranchOnDeleteWorktree = false + $0.deleteBranchOnAutomaticCleanup = false $0.mergedWorktreeAction = .archive $0.promptForWorktreeCreation = true } @@ -66,7 +66,7 @@ struct SettingsFeatureTests { analyticsEnabled: true, crashReportsEnabled: false, githubIntegrationEnabled: true, - deleteBranchOnDeleteWorktree: true, + deleteBranchOnAutomaticCleanup: true, mergedWorktreeAction: nil, promptForWorktreeCreation: false ) @@ -92,7 +92,7 @@ struct SettingsFeatureTests { analyticsEnabled: initialSettings.analyticsEnabled, crashReportsEnabled: initialSettings.crashReportsEnabled, githubIntegrationEnabled: initialSettings.githubIntegrationEnabled, - deleteBranchOnDeleteWorktree: initialSettings.deleteBranchOnDeleteWorktree, + deleteBranchOnAutomaticCleanup: initialSettings.deleteBranchOnAutomaticCleanup, mergedWorktreeAction: initialSettings.mergedWorktreeAction, promptForWorktreeCreation: initialSettings.promptForWorktreeCreation ) @@ -241,7 +241,7 @@ struct SettingsFeatureTests { analyticsEnabled: true, crashReportsEnabled: false, githubIntegrationEnabled: true, - deleteBranchOnDeleteWorktree: true, + deleteBranchOnAutomaticCleanup: true, mergedWorktreeAction: .archive, promptForWorktreeCreation: false ) @@ -258,7 +258,7 @@ struct SettingsFeatureTests { $0.analyticsEnabled = true $0.crashReportsEnabled = false $0.githubIntegrationEnabled = true - $0.deleteBranchOnDeleteWorktree = true + $0.deleteBranchOnAutomaticCleanup = true $0.mergedWorktreeAction = .archive $0.promptForWorktreeCreation = false $0.selection = selection diff --git a/supacodeTests/SettingsFilePersistenceTests.swift b/supacodeTests/SettingsFilePersistenceTests.swift index edccf530..d7581408 100644 --- a/supacodeTests/SettingsFilePersistenceTests.swift +++ b/supacodeTests/SettingsFilePersistenceTests.swift @@ -162,7 +162,7 @@ struct SettingsFilePersistenceTests { #expect(settings.global.analyticsEnabled == true) #expect(settings.global.crashReportsEnabled == true) #expect(settings.global.githubIntegrationEnabled == true) - #expect(settings.global.deleteBranchOnDeleteWorktree == false) + #expect(settings.global.deleteBranchOnAutomaticCleanup == false) #expect(settings.global.mergedWorktreeAction == nil) #expect(settings.global.promptForWorktreeCreation == true) #expect(settings.global.defaultWorktreeBaseDirectoryPath == nil) @@ -227,6 +227,44 @@ struct SettingsFilePersistenceTests { #expect(settings.global.mergedWorktreeAction == nil) } + @Test(.dependencies) func legacyDeleteBranchOnDeleteWorktreeMigratesToAutomaticCleanup() throws { + // Files written before the setting was split carry only the legacy key; + // an opted-in user keeps branch deletion during automatic cleanup. + let encoded = try JSONEncoder().encode(GlobalSettings.default) + var globalDict = try #require(try JSONSerialization.jsonObject(with: encoded) as? [String: Any]) + globalDict.removeValue(forKey: "deleteBranchOnAutomaticCleanup") + globalDict["deleteBranchOnDeleteWorktree"] = true + let data = try JSONSerialization.data(withJSONObject: ["global": globalDict, "repositories": [:]]) + let storage = MutableTestStorage(initialData: data) + + let settings: SettingsFile = withDependencies { + $0.settingsFileStorage = storage.storage + } operation: { + @Shared(.settingsFile) var settings: SettingsFile + return settings + } + + #expect(settings.global.deleteBranchOnAutomaticCleanup == true) + } + + @Test(.dependencies) func explicitDeleteBranchOnAutomaticCleanupWinsOverLegacyKey() throws { + let encoded = try JSONEncoder().encode(GlobalSettings.default) + var globalDict = try #require(try JSONSerialization.jsonObject(with: encoded) as? [String: Any]) + globalDict["deleteBranchOnAutomaticCleanup"] = false + globalDict["deleteBranchOnDeleteWorktree"] = true + let data = try JSONSerialization.data(withJSONObject: ["global": globalDict, "repositories": [:]]) + let storage = MutableTestStorage(initialData: data) + + let settings: SettingsFile = withDependencies { + $0.settingsFileStorage = storage.storage + } operation: { + @Shared(.settingsFile) var settings: SettingsFile + return settings + } + + #expect(settings.global.deleteBranchOnAutomaticCleanup == false) + } + @Test(.dependencies) func roundTripsExplicitNotificationSound() throws { let storage = SettingsTestStorage()