tray: review changes in its own window; keep rows busy until the list refresh lands

rt-tray/Sources-core/Worktree/Triage.swift

@@ -236,6 +236,24 @@ public struct TriageQueryGate: Sendable {
     }
 }
 
+/// Work held until a query started after it lands. Applied rows answer every
+/// earlier waiter too, since they are at least as new; a failed query answers
+/// only its own, so nothing waits on a reply that will never come.
+public struct TriageSettleLedger<Value> {
+    private var waiting: [(ticket: Int, value: Value)] = []
+
+    public init() {}
+
+    public mutating func wait(_ ticket: Int, _ value: Value) { waiting.append((ticket, value)) }
+
+    public mutating func settle(_ ticket: Int, applied: Bool) -> [Value] {
+        let due = { (w: (ticket: Int, value: Value)) in applied ? w.ticket <= ticket : w.ticket == ticket }
+        let out = waiting.filter(due).map(\.value)
+        waiting.removeAll(where: due)
+        return out
+    }
+}
+
 /// Daemon refusal codes as the tail of a "<tree>: ..." status line. Codes
 /// may carry a ":<detail>" suffix; anything unrecognised is shown verbatim.
 public enum TriageRefusal {

rt-tray/Sources/AppDelegate.swift

@@ -1467,6 +1467,10 @@ class AppDelegate: NSObject, NSApplicationDelegate, @unchecked Sendable {
         w.center()
         w.setFrameAutosaveName("rt-worktree-panel")
         w.isReleasedWhenClosed = false
+        // The panel's hosting view outlives the window, so its onDisappear never fires on close.
+        NotificationCenter.default.addObserver(forName: NSWindow.willCloseNotification, object: w, queue: .main) { _ in
+            MainActor.assumeIsolated { WorktreeReviewWindow.shared.close() }
+        }
         worktreeWindow = w
         w.makeKeyAndOrderFront(nil)
         NSApp.activate(ignoringOtherApps: true)

rt-tray/Sources/WorktreePanelController.swift

@@ -61,6 +61,8 @@ final class WorktreePanelController: ObservableObject {
     private var statusGeneration = 0
     private var hasLoaded = false
     private var queryGate = TriageQueryGate()
+    /// A finished action keeps its row busy until a query started after it lands.
+    private var settling = TriageSettleLedger<() -> Void>()
     private let fixture: TriageData?
 
     init(fixture: TriageData? = nil) {
@@ -83,12 +85,16 @@ final class WorktreePanelController: ObservableObject {
     /// A failed background poll keeps the last rows and says nothing, so it
     /// never overwrites the footer an action just set. `force` starts a query
     /// even while a poll is in flight, so a finished action's rows are never
-    /// left to a poll that began before it.
-    func refresh(userInitiated: Bool = false, force: Bool = false) {
-        guard fixture == nil, let ticket = queryGate.begin(force: userInitiated || force) else { return }
+    /// left to a poll that began before it. `settled` runs in the same update
+    /// that applies this query's rows (or a newer query's), or once it fails.
+    func refresh(userInitiated: Bool = false, force: Bool = false, settled: (() -> Void)? = nil) {
+        guard fixture == nil, let ticket = queryGate.begin(force: userInitiated || force || settled != nil) else { settled?(); return }
+        if let settled { settling.wait(ticket, settled) }
         Task {
             let p = await client.queryTriage()
-            guard queryGate.finish(ticket, succeeded: p?.data != nil) else { return }
+            let current = queryGate.finish(ticket, succeeded: p?.data != nil)
+            defer { settling.settle(ticket, applied: current && p?.data != nil).forEach { $0() } }
+            guard current else { return }
             isLoading = false
             guard let data = p?.data else {
                 if userInitiated || !hasLoaded {
@@ -125,10 +131,14 @@ final class WorktreePanelController: ObservableObject {
         busy.insert(row.id)
         Task {
             let (outcome, trashPath) = await performReply(verb, row, payload)
-            busy.remove(row.id)
             let line = TriageStatusLine.action(tree: row.tree, outcome: outcome, done: done(trashPath))
             setStatus(line.text, isError: line.isError)
-            refresh(force: true)
+            if outcome == .done {
+                refresh(force: true) { [weak self] in self?.busy.remove(row.id) }
+            } else {
+                busy.remove(row.id)
+                refresh(force: true)
+            }
         }
     }
 
@@ -154,22 +164,29 @@ final class WorktreePanelController: ObservableObject {
     /// One at a time, so the footer and the button can report "1 of 2" and a
     /// refusal on one row doesn't hide behind the others.
     func cleanUpSafe() {
-        let safe = rows.filter { $0.group == "safe" }
+        let safe = rows.filter { $0.group == "safe" && !busy.contains($0.id) }
         guard !safe.isEmpty, bulkProgress == nil else { return }
         bulkProgress = (0, safe.count)
         Task {
             var failures: [(tree: String, outcome: TriageActionOutcome)] = []
+            var disposed: [String] = []
             for (i, row) in safe.enumerated() {
                 busy.insert(row.id)
                 bulkProgress = (i, safe.count)
                 let outcome = await perform("worktree:triage-dispose", row, ["fingerprint": row.fingerprint.jsonObject, "discard": "classified"])
-                busy.remove(row.id)
-                if outcome != .done { failures.append((tree: row.tree, outcome: outcome)) }
+                if outcome == .done {
+                    disposed.append(row.id)
+                } else {
+                    busy.remove(row.id)
+                    failures.append((tree: row.tree, outcome: outcome))
+                }
             }
-            bulkProgress = nil
             let line = TriageStatusLine.bulk(total: safe.count, failures: failures)
             setStatus(line.text, isError: line.isError)
-            refresh(force: true)
+            refresh(force: true) { [weak self] in
+                self?.busy.subtract(disposed)
+                self?.bulkProgress = nil
+            }
         }
     }
 

rt-tray/Sources/WorktreePanelView.swift

@@ -120,7 +120,6 @@ enum TriageLabels {
 
 struct WorktreePanelView: View {
     @StateObject private var controller: WorktreePanelController
-    @State private var reviewing: TriageRow?
     @State private var confirmingDisposeAnyway: TriageRow?
     @State private var keptOpen: Bool
     /// The verb each busy row is running, so its button can say what it is doing.
@@ -148,9 +147,6 @@ struct WorktreePanelView: View {
         .environment(\.triageSnapshot, isSnapshot)
         .onAppear { controller.startPolling() }
         .onDisappear { controller.stopPolling() }
-        .sheet(item: $reviewing) { row in
-            WorktreeReviewSheet(row: row, controller: controller) { inFlight[row.id] = $0 }
-        }
         .alert(confirmingDisposeAnyway.map(TriageConfirm.disposeAnywayTitle) ?? "",
                isPresented: Binding(get: { confirmingDisposeAnyway != nil },
                                     set: { if !$0 { confirmingDisposeAnyway = nil } }),
@@ -203,7 +199,9 @@ struct WorktreePanelView: View {
             TriageBulkButton(safe: progress.1, progress: progress) {}
         } else if let safe = controller.counts?.safe, safe > 0 {
             TriageBulkButton(safe: safe, progress: nil) {
-                for row in controller.rows where row.group == "safe" { inFlight[row.id] = "dispose" }
+                for row in controller.rows where row.group == "safe" && !controller.busy.contains(row.id) {
+                    inFlight[row.id] = "dispose"
+                }
                 controller.cleanUpSafe()
             }
         }
@@ -285,7 +283,7 @@ struct WorktreePanelView: View {
         switch action {
         case "dispose": inFlight[r.id] = "dispose"; controller.dispose(r)
         case "dispose-anyway": confirmingDisposeAnyway = r
-        case "review": reviewing = r
+        case "review": WorktreeReviewWindow.shared.show(r, controller: controller) { inFlight[r.id] = $0 }
         case "push-branch": inFlight[r.id] = "push-branch"; controller.pushBranch(r)
         case "keep": inFlight[r.id] = "keep"; controller.keep(r)
         case "unkeep": inFlight[r.id] = "unkeep"; controller.unkeep(r)

rt-tray/Sources/WorktreeReviewSheet.swift

@@ -5,16 +5,19 @@ struct WorktreeReviewSheet: View {
     let row: TriageRow
     @ObservedObject var controller: WorktreePanelController
     let onStart: (String) -> Void
-    @Environment(\.dismiss) private var dismiss
+    let onClose: () -> Void
     @Environment(\.triageSnapshot) private var isSnapshot
     @State private var load: DiffLoadState
+    @State private var expanded: Set<String>
 
     init(row: TriageRow, controller: WorktreePanelController, initialLoad: TriageDiffLoad? = nil,
-         onStart: @escaping (String) -> Void = { _ in }) {
+         expanded: Set<String> = [], onStart: @escaping (String) -> Void = { _ in }, onClose: @escaping () -> Void = {}) {
         self.row = row
         self.controller = controller
         self.onStart = onStart
+        self.onClose = onClose
         _load = State(initialValue: initialLoad.map(DiffLoadState.init) ?? .loading)
+        _expanded = State(initialValue: expanded)
     }
 
     private var subtitle: String {
@@ -33,6 +36,7 @@ struct WorktreeReviewSheet: View {
 
 
     private var loaded: Bool { if case .loaded = load { return true } else { return false } }
+    private var rowBusy: Bool { controller.busy.contains(row.id) || controller.bulkProgress != nil }
 
     var body: some View {
         VStack(alignment: .leading, spacing: 0) {
@@ -43,25 +47,26 @@ struct WorktreeReviewSheet: View {
             .frame(maxWidth: .infinity, alignment: .leading)
             .padding(.horizontal, 20).padding(.top, 20).padding(.bottom, 14)
             SheetRule()
-            content
+            content.frame(maxHeight: .infinity, alignment: .top)
             SheetRule()
             HStack(spacing: 8) {
                 Text("Discarded files stay in the trash for 14 days.")
                     .font(.system(size: 12.5)).foregroundStyle(WT.textTertiary)
                     .lineLimit(1).layoutPriority(-1)
                 Spacer(minLength: 8)
-                Button("Keep") { onStart("keep"); controller.keep(row); dismiss() }
+                Button("Keep") { onStart("keep"); controller.keep(row); onClose() }
                     .buttonStyle(TriageButtonStyle())
-                Button("Commit and push") { onStart("push-branch"); controller.pushBranch(row, commitDirty: true); dismiss() }
+                    .disabled(rowBusy)
+                Button("Commit and push") { onStart("push-branch"); controller.pushBranch(row, commitDirty: true); onClose() }
                     .buttonStyle(TriageButtonStyle())
-                    .disabled(!loaded)
-                Button("Discard and dispose") { onStart("dispose"); controller.dispose(row, discard: "all"); dismiss() }
+                    .disabled(!loaded || rowBusy)
+                Button("Discard and dispose") { onStart("dispose"); controller.dispose(row, discard: "all"); onClose() }
                     .buttonStyle(TriageButtonStyle(primary: true))
-                    .disabled(!loaded)
+                    .disabled(!loaded || rowBusy)
             }
             .padding(.horizontal, 20).padding(.vertical, 12)
         }
-        .frame(width: 680)
+        .frame(minWidth: 560, maxWidth: .infinity, minHeight: 360, maxHeight: .infinity, alignment: .top)
         .background(WT.card)
         .task {
             if case .loading = load { await reload() }
@@ -74,15 +79,15 @@ struct WorktreeReviewSheet: View {
             Text("Loading changes…")
                 .font(.system(size: 13)).foregroundStyle(WT.textTertiary)
                 .padding(20)
-                .frame(maxWidth: .infinity, minHeight: 280, alignment: .topLeading)
+                .frame(maxWidth: .infinity, alignment: .topLeading)
         case .failed:
             HStack(spacing: 12) {
                 Text("Couldn't load the changes.").font(.system(size: 13)).foregroundStyle(WT.textSecondary)
                 Button("Retry") { Task { await reload() } }.buttonStyle(TriageButtonStyle())
                 Spacer(minLength: 0)
             }
             .padding(20)
-            .frame(maxWidth: .infinity, minHeight: 280, alignment: .topLeading)
+            .frame(maxWidth: .infinity, alignment: .topLeading)
         case .loaded(let files, _) where files.isEmpty:
             Text("No uncommitted changes left to show.")
                 .font(.system(size: 13)).foregroundStyle(WT.textSecondary)
@@ -93,7 +98,6 @@ struct WorktreeReviewSheet: View {
                 fileList(files, truncatedFiles: truncatedFiles, lazy: false)
             } else {
                 ScrollView { fileList(files, truncatedFiles: truncatedFiles, lazy: true) }
-                    .frame(minHeight: 280, maxHeight: 520)
             }
         }
     }
@@ -107,32 +111,41 @@ struct WorktreeReviewSheet: View {
     private func fileList(_ files: [ParsedDiffFile], truncatedFiles: Bool, lazy: Bool) -> some View {
         DiffStack(lazy: lazy) {
             ForEach(files) { f in
-                HStack(spacing: 8) {
-                    Image(systemName: f.file.status == "untracked" ? "doc.badge.plus" : "doc.text")
-                        .foregroundStyle(WT.textSecondary)
-                    Text(Self.shortPath(f.file.path)).font(.system(size: 12.5, design: .monospaced)).foregroundStyle(WT.text)
-                        .lineLimit(1).truncationMode(.middle)
-                    Spacer(minLength: 12)
-                    Text(Self.stat(f.file)).font(.system(size: 12.5)).foregroundStyle(WT.textTertiary)
+                let open = expanded.contains(f.id)
+                DiffFileHeader(file: f.file, open: open) { toggle(f.id, in: files) }
+                if open {
+                    SheetRule()
+                    WT.neutralFill.frame(height: 12)
+                    ForEach(f.lines) { DiffLineRow(line: $0) }
+                    if let more = f.moreLines {
+                        Text(more).font(.system(size: 12.5)).foregroundStyle(WT.textTertiary)
+                            .padding(.horizontal, 20).padding(.top, 6)
+                            .frame(maxWidth: .infinity, alignment: .leading)
+                            .background(WT.neutralFill)
+                    }
+                    WT.neutralFill.frame(height: 12)
                 }
-                .padding(.horizontal, 20).padding(.vertical, 9)
                 SheetRule()
-                Color.clear.frame(height: 12)
-                ForEach(f.lines) { DiffLineRow(line: $0) }
-                if let more = f.moreLines {
-                    Text(more).font(.system(size: 12.5)).foregroundStyle(WT.textTertiary)
-                        .padding(.horizontal, 20).padding(.top, 6)
-                }
-                Color.clear.frame(height: 12)
             }
             if truncatedFiles {
-                SheetRule()
                 Text("More files not shown.").font(.system(size: 12.5)).foregroundStyle(WT.textTertiary)
                     .padding(.horizontal, 20).padding(.vertical, 10)
             }
         }
+        .padding(.bottom, 16)
         .frame(maxWidth: .infinity, alignment: .leading)
-        .background(WT.neutralFill)
+    }
+
+    /// Option-click opens or closes every file, as a Finder disclosure does.
+    private func toggle(_ id: String, in files: [ParsedDiffFile]) {
+        let opening = !expanded.contains(id)
+        if NSEvent.modifierFlags.contains(.option) {
+            expanded = opening ? Set(files.map(\.id)) : []
+        } else if opening {
+            expanded.insert(id)
+        } else {
+            expanded.remove(id)
+        }
     }
 
     static func shortPath(_ path: String) -> String {
@@ -242,13 +255,47 @@ struct DiffLine: Identifiable {
     }
 }
 
+private struct DiffFileHeader: View {
+    let file: TriageDiffFile
+    let open: Bool
+    let toggle: () -> Void
+    @State private var hovered = false
+
+    var body: some View {
+        Button(action: toggle) {
+            HStack(spacing: 8) {
+                Image(systemName: "chevron.right")
+                    .font(.system(size: 10, weight: .semibold))
+                    .rotationEffect(.degrees(open ? 90 : 0))
+                    .animation(.easeOut(duration: 0.15), value: open)
+                    .foregroundStyle(WT.textTertiary)
+                    .frame(width: 12)
+                Image(systemName: file.status == "untracked" ? "doc.badge.plus" : "doc.text")
+                    .foregroundStyle(WT.textSecondary)
+                Text(WorktreeReviewSheet.shortPath(file.path)).font(.system(size: 12.5, design: .monospaced)).foregroundStyle(WT.text)
+                    .lineLimit(1).truncationMode(.middle)
+                Spacer(minLength: 12)
+                Text(WorktreeReviewSheet.stat(file)).font(.system(size: 12.5)).foregroundStyle(WT.textTertiary)
+            }
+            .padding(.leading, 14).padding(.trailing, 20).padding(.vertical, 9)
+            .frame(maxWidth: .infinity, alignment: .leading)
+            .background(hovered ? WT.cardHover : WT.card)
+            .contentShape(Rectangle())
+        }
+        .buttonStyle(.plain)
+        .onHover { hovered = $0 }
+    }
+}
+
 private struct DiffLineRow: View {
     let line: DiffLine
     var body: some View {
         if line.isHunk {
             Text(line.text)
                 .font(.system(size: 12, design: .monospaced)).foregroundStyle(WT.textTertiary)
                 .padding(.leading, 20).padding(.vertical, 3)
+                .frame(maxWidth: .infinity, alignment: .leading)
+                .background(WT.neutralFill)
         } else {
             HStack(alignment: .firstTextBaseline, spacing: 0) {
                 Text(line.number.map(String.init) ?? "")
@@ -263,7 +310,8 @@ private struct DiffLineRow: View {
             }
             .font(.system(size: 12.5, design: .monospaced))
             .padding(.leading, 18).padding(.trailing, 20)
-            .frame(height: 18)
+            .frame(maxWidth: .infinity, minHeight: 18, maxHeight: 18, alignment: .leading)
+            .background(WT.neutralFill)
         }
     }
 }