diff --git a/.agents/skills/write-stable-tests/scripts/test-timing-lib.mjs b/.agents/skills/write-stable-tests/scripts/test-timing-lib.mjs index 5a3e79416..b35e661b3 100755 --- a/.agents/skills/write-stable-tests/scripts/test-timing-lib.mjs +++ b/.agents/skills/write-stable-tests/scripts/test-timing-lib.mjs @@ -14,6 +14,44 @@ function processGroupIsRunning(processID) { } } +function processIsRunning(processID) { + try { + process.kill(processID, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} + +function addDescendantProcessIDs(rootProcessID, processIDs) { + let rows; + try { + const listing = spawnSync("ps", ["-axo", "pid=,ppid="], { + encoding: "utf8", + timeout: 5000, + }); + if (listing.status !== 0) return; + rows = listing.stdout + .split("\n") + .map((line) => line.trim().split(/\s+/).map(Number)) + .filter(([pid, ppid]) => Number.isInteger(pid) && Number.isInteger(ppid)); + } catch { + // The direct process group remains the portable fallback when ps is unavailable. + return; + } + + processIDs.add(rootProcessID); + let changed = true; + while (changed) { + changed = false; + for (const [pid, ppid] of rows) { + if (!processIDs.has(ppid) || processIDs.has(pid)) continue; + processIDs.add(pid); + changed = true; + } + } +} + function signalProcessGroup(child, signal) { try { process.kill(-child.pid, signal); @@ -27,12 +65,30 @@ function signalProcessGroup(child, signal) { } } -async function waitForProcessGroupExit(processID, timeoutMs, pollIntervalMs) { +function signalDescendantProcesses(rootProcessID, processIDs, signal) { + // The process-group signal handles ordinary descendants. Signal every known + // non-root PID as well because swift-testing may create a new process group. + for (const processID of processIDs) { + if (processID === rootProcessID) continue; + try { + process.kill(processID, signal); + } catch { + // It either exited between the snapshot and signal or is already gone. + } + } +} + +function processTreeIsRunning(processID, processIDs) { + return processGroupIsRunning(processID) + || [...processIDs].some((candidate) => processIsRunning(candidate)); +} + +async function waitForProcessTreeExit(processID, processIDs, timeoutMs, pollIntervalMs) { const deadline = performance.now() + timeoutMs; - while (processGroupIsRunning(processID) && performance.now() < deadline) { + while (processTreeIsRunning(processID, processIDs) && performance.now() < deadline) { await delay(pollIntervalMs); } - return !processGroupIsRunning(processID); + return !processTreeIsRunning(processID, processIDs); } export async function terminateProcessTree( @@ -52,10 +108,23 @@ export async function terminateProcessTree( return true; } + const processIDs = new Set(); + addDescendantProcessIDs(child.pid, processIDs); signalProcessGroup(child, "SIGTERM"); - if (await waitForProcessGroupExit(child.pid, gracePeriodMs, pollIntervalMs)) return true; + signalDescendantProcesses(child.pid, processIDs, "SIGTERM"); + if (await waitForProcessTreeExit(child.pid, processIDs, gracePeriodMs, pollIntervalMs)) return true; + + // Refresh before forcing termination so descendants created during graceful + // shutdown cannot escape the cleanup pass. + addDescendantProcessIDs(child.pid, processIDs); signalProcessGroup(child, "SIGKILL"); - return waitForProcessGroupExit(child.pid, forcedTerminationTimeoutMs, pollIntervalMs); + signalDescendantProcesses(child.pid, processIDs, "SIGKILL"); + return waitForProcessTreeExit( + child.pid, + processIDs, + forcedTerminationTimeoutMs, + pollIntervalMs, + ); } function lineCollector(callback) { diff --git a/.agents/skills/write-stable-tests/scripts/test-verify-test-stability.mjs b/.agents/skills/write-stable-tests/scripts/test-verify-test-stability.mjs index 74715ed4d..9b7c8b574 100755 --- a/.agents/skills/write-stable-tests/scripts/test-verify-test-stability.mjs +++ b/.agents/skills/write-stable-tests/scripts/test-verify-test-stability.mjs @@ -566,10 +566,13 @@ try { if (process.platform !== "win32") { let rootPID = null; let descendantPID = null; + // swift-testing may detach its helper into a separate process group. The + // timeout owner must still discover and terminate that descendant. const descendantSource = "process.on('SIGTERM', () => {}); setInterval(() => {}, 1000);"; const rootSource = ` const { spawn } = require("node:child_process"); const descendant = spawn(process.execPath, ["-e", ${JSON.stringify(descendantSource)}], { + detached: true, stdio: "ignore", }); console.log(descendant.pid); diff --git a/macos/Sources/Lithe/Views/Git/BranchSwitcherPopover.swift b/macos/Sources/Lithe/Views/Git/BranchSwitcherPopover.swift index 5686d4dd8..6ff479118 100644 --- a/macos/Sources/Lithe/Views/Git/BranchSwitcherPopover.swift +++ b/macos/Sources/Lithe/Views/Git/BranchSwitcherPopover.swift @@ -15,6 +15,7 @@ struct BranchSwitcherPopover: View { @Binding var isPresented: Bool let onCommit: () -> Void let onPush: (GitReference) -> Void + let onDelete: (GitReference) -> Void let onNewBranch: (GitReference) -> Void let onCheckoutRevision: () -> Void let onManageBranches: () -> Void @@ -364,6 +365,9 @@ struct BranchSwitcherPopover: View { .lithePointer() } + /// A branch line. Clicking it opens the reference's action menu instead of + /// checking out directly, matching IDEA: checkout is an explicit menu entry, + /// so a stray click on the list can never switch the working tree. private func branchRow( _ reference: GitReference, indented: Bool, @@ -371,46 +375,107 @@ struct BranchSwitcherPopover: View { ) -> some View { let highlightsCurrent = presentation == .recent && reference.isCurrent - return Button { - guard !reference.isCurrent else { return } - isPresented = false - Task { await model.checkoutReference(reference) } - } label: { - HStack(spacing: 8) { - Image(systemName: referenceIcon(reference, marksCurrent: presentation == .recent)) - .font(.system(size: 11.5)) - .foregroundStyle(highlightsCurrent ? LitheTheme.warning : LitheTheme.secondaryText) - .frame(width: 17) - Text(branchDisplayName(reference, presentation: presentation)) - .font(.system(size: 12.5)) - .foregroundStyle(LitheTheme.primaryText) - .lineLimit(1) - .truncationMode(.middle) - Spacer(minLength: 10) - if let upstream = reference.upstreamShortName { - Text(upstream) + return BranchActionMenuRow( + label: { + HStack(spacing: 8) { + Image(systemName: referenceIcon(reference, marksCurrent: presentation == .recent)) .font(.system(size: 11.5)) - .foregroundStyle(LitheTheme.secondaryText) + .foregroundStyle(highlightsCurrent ? LitheTheme.warning : LitheTheme.secondaryText) + .frame(width: 17) + Text(branchDisplayName(reference, presentation: presentation)) + .font(.system(size: 12.5)) + .foregroundStyle(LitheTheme.primaryText) .lineLimit(1) .truncationMode(.middle) - } - if !reference.isCurrent { + Spacer(minLength: 10) + if let upstream = reference.upstreamShortName { + Text(upstream) + .font(.system(size: 11.5)) + .foregroundStyle(LitheTheme.secondaryText) + .lineLimit(1) + .truncationMode(.middle) + } Image(systemName: "chevron.right") .font(.system(size: 8, weight: .bold)) .foregroundStyle(LitheTheme.secondaryText) } + .padding(.leading, branchRowLeadingPadding(indented: indented, presentation: presentation)) + .padding(.trailing, 9) + .frame(maxWidth: .infinity, alignment: .leading) + .frame(height: Metrics.branchRowHeight) + .background(highlightsCurrent ? LitheTheme.subtleSelection : .clear) + .clipShape(RoundedRectangle(cornerRadius: 5)) + .contentShape(Rectangle()) + // Branch and upstream names are truncated to keep the row width + // fixed, so the untruncated pair is only reachable on hover. + .help(branchRowTooltip(reference)) + }, + menuContent: { branchActionMenu(for: reference) } + ) + .disabled(model.isPerformingBranchOperation) + } + + /// The full branch name, plus its upstream when tracked, for rows whose text + /// the fixed popup width truncates. + private func branchRowTooltip(_ reference: GitReference) -> String { + guard let upstream = reference.upstreamShortName else { return reference.shortName } + return "\(reference.shortName) → \(upstream)" + } + + /// The per-reference action list, ordered like IDEA's branch menu: creation + /// and comparison first, then checkout and integration, then destructive + /// entries last. + @ViewBuilder + private func branchActionMenu(for reference: GitReference) -> some View { + Button("New Branch from '\(reference.shortName)'…") { + dismissAndRun { onNewBranch(reference) } + } + + Button("Show Diff with Working Tree") { + dismissAndRun { Task { await model.showComparisonWithWorkingTree(for: reference) } } + } + + if let current = model.currentGitReference, current.id != reference.id { + Button("Compare with Current Branch") { + dismissAndRun { Task { await model.showComparison(from: reference, to: current) } } + } + } + + if !reference.isCurrent { + Divider() + + Button("Checkout") { + dismissAndRun { Task { await model.checkoutReference(reference) } } + } + } + + if reference.kind == .local { + Divider() + + Button("Update") { + dismissAndRun { Task { await model.updateCurrentBranch(reference) } } + } + .disabled(!reference.isCurrent) + + Button("Push…") { + dismissAndRun { onPush(reference) } + } + } + + if reference.kind == .local, !reference.isCurrent { + Divider() + + Button("Delete", role: .destructive) { + dismissAndRun { onDelete(reference) } } - .padding(.leading, branchRowLeadingPadding(indented: indented, presentation: presentation)) - .padding(.trailing, 9) - .frame(maxWidth: .infinity, alignment: .leading) - .frame(height: Metrics.branchRowHeight) - .background(highlightsCurrent ? LitheTheme.subtleSelection : .clear) - .clipShape(RoundedRectangle(cornerRadius: 5)) - .contentShape(Rectangle()) } - .buttonStyle(.plain) - .lithePointer() - .disabled(model.isPerformingBranchOperation) + } + + /// Closes the popover before running a branch action so the action's own + /// sheet or dialog is not presented behind a popover that is about to go away. + private func dismissAndRun(_ action: @escaping () -> Void) { + isPresented = false + action() } private var recentReferences: [GitReference] { @@ -577,6 +642,37 @@ struct BranchSwitcherPopover: View { } } +/// A branch row that surfaces its actions through a native pop-up menu rather +/// than a direct checkout. +/// +/// Using `SwiftUI.Menu` with `.menuStyle(.borderlessButton)` produces a native +/// NSMenu, which works correctly inside the outer popover, positions itself to +/// avoid screen edges, and provides the hover-safety path that IDEA exposes: +/// once any row's menu is open, moving the cursor to another row opens that +/// menu immediately without a click. +private struct BranchActionMenuRow: View { + @ViewBuilder let label: () -> Label + @ViewBuilder let menuContent: () -> MenuContent + + @State private var isHovering = false + + var body: some View { + SwiftUI.Menu { + menuContent() + } label: { + label() + .background(isHovering ? LitheTheme.subtleSelection : .clear) + .clipShape(RoundedRectangle(cornerRadius: 5)) + } + .menuStyle(.borderlessButton) + .menuIndicator(.hidden) + // Constrain to the list width so the menu button does not stretch. + .fixedSize(horizontal: false, vertical: true) + .lithePointer() + .onHover { isHovering = $0 } + } +} + private enum BranchRowPresentation { case recent case grouped diff --git a/macos/Sources/Lithe/Views/Workbench/WorkbenchView.swift b/macos/Sources/Lithe/Views/Workbench/WorkbenchView.swift index 2ed1e92ae..af86c5c1e 100644 --- a/macos/Sources/Lithe/Views/Workbench/WorkbenchView.swift +++ b/macos/Sources/Lithe/Views/Workbench/WorkbenchView.swift @@ -82,6 +82,7 @@ struct WorkbenchView: View { @State private var newBranchReference: GitReference? @State private var isCheckoutRevisionPresented = false @State private var pendingTopBarPushReference: GitReference? + @State private var pendingTopBarDeleteReference: GitReference? @State private var isProjectSwitcherPresented = false @State private var isPluginPanelPresented = false @State private var isNotificationCenterPresented = false @@ -278,6 +279,31 @@ struct WorkbenchView: View { } ) } + .confirmationDialog( + "Delete branch?", + isPresented: Binding( + get: { pendingTopBarDeleteReference != nil }, + set: { if !$0 { pendingTopBarDeleteReference = nil } } + ), + titleVisibility: .visible + ) { + Button("Delete", role: .destructive) { + guard let reference = pendingTopBarDeleteReference else { return } + pendingTopBarDeleteReference = nil + Task { await model.deleteBranch(reference) } + } + .disabled(model.isPerformingBranchOperation) + .lithePointer() + Button("Cancel", role: .cancel) { + pendingTopBarDeleteReference = nil + } + .lithePointer() + } message: { + Text( + "Delete the local branch \(pendingTopBarDeleteReference?.shortName ?? "")? " + + "Git will refuse if it contains unmerged work." + ) + } .overlayPreferenceValue(ProjectSwitcherButtonBoundsPreferenceKey.self) { bounds in GeometryReader { geometry in if isProjectSwitcherPresented, let bounds { @@ -678,6 +704,10 @@ struct WorkbenchView: View { updateSwitcherPresentation(branch: false) pendingTopBarPushReference = reference }, + onDelete: { reference in + updateSwitcherPresentation(branch: false) + pendingTopBarDeleteReference = reference + }, onNewBranch: { reference in updateSwitcherPresentation(branch: false) newBranchReference = reference diff --git a/macos/Tests/LitheTests/BranchSwitcherPopoverBehaviorTests.swift b/macos/Tests/LitheTests/BranchSwitcherPopoverBehaviorTests.swift new file mode 100644 index 000000000..6928b550f --- /dev/null +++ b/macos/Tests/LitheTests/BranchSwitcherPopoverBehaviorTests.swift @@ -0,0 +1,118 @@ +import Foundation +import Testing +@testable import Lithe + +/// Guards the branch popup's IDEA-aligned interaction contract. The rows are +/// SwiftUI views without a testable state surface, so these checks read the +/// source: the regression they protect against is a branch row silently going +/// back to checking out on a plain click, which switches the working tree from +/// a stray click while scanning the list. +@Suite("Branch switcher popover behavior") +struct BranchSwitcherPopoverBehaviorTests { + private static func source(at relativePath: String) throws -> String { + let repositoryRoot = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + return try String( + contentsOf: repositoryRoot.appendingPathComponent(relativePath), + encoding: .utf8 + ) + } + + private static func popoverSource() throws -> String { + try source(at: "Sources/Lithe/Views/Git/BranchSwitcherPopover.swift") + } + + private static func workbenchSource() throws -> String { + try source(at: "Sources/Lithe/Views/Workbench/WorkbenchView.swift") + } + + @Test + func branchRowsOpenAnActionMenuInsteadOfCheckingOutOnClick() throws { + let source = try Self.popoverSource() + + #expect( + source.contains("BranchActionMenuRow("), + "Branch rows must route through BranchActionMenuRow so a click opens the action menu." + ) + #expect( + source.contains("private func branchActionMenu(for reference: GitReference)"), + "The per-reference action list must exist for the menu to present." + ) + } + + @Test + func checkoutIsReachableOnlyAsAnExplicitMenuEntry() throws { + let source = try Self.popoverSource() + + // The single permitted checkout call site is the menu's Checkout entry. + let checkoutCallSites = source.components(separatedBy: "model.checkoutReference(").count - 1 + #expect( + checkoutCallSites == 1, + "Checkout must have exactly one call site, the explicit Checkout menu entry." + ) + + guard let checkoutRange = source.range(of: "model.checkoutReference(") else { + Issue.record("Expected a checkout call site in the branch popup.") + return + } + let precedingSource = source[source.startIndex..