From 7ca11e0137fac9bf36d8ee0597db69654a0bbc6f Mon Sep 17 00:00:00 2001 From: Ethan Lee Date: Sat, 4 Jul 2026 15:20:43 +0900 Subject: [PATCH 1/2] Skip quiet-settle for occluded panes so parked renderers can't fake completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #115 parks an occluded surface's renderer, hidden panes emit no render/scrollbar heartbeats. The status indicator's quiet-settle read that silence as "command finished" and flipped a still-running TUI in a hidden tab to a done checkmark — which sticks, because activity events can never revive a done pane. Settle now runs only while the pane's window is visible. On the occluded-to-visible edge the quiet window restarts, so a still-running program gets its heartbeats back before the 3s settle can fire. The occlusion probe is an injected closure (no window = occluded, covering incubator-hosted panes) so the behavior is unit-tested. --- Macterm/App/AppState.swift | 36 +++++++++++++- Macterm/Model/SplitNode.swift | 14 ++++++ MactermTests/App/AppStateTests.swift | 72 ++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/Macterm/App/AppState.swift b/Macterm/App/AppState.swift index 91ba437..ce67b9a 100644 --- a/Macterm/App/AppState.swift +++ b/Macterm/App/AppState.swift @@ -88,6 +88,21 @@ final class AppState { @ObservationIgnored private var processNameTimer: Timer? + /// Whether a pane's surface is occluded — its renderer parked by + /// `ghostty_surface_set_occlusion`, so render/scrollbar heartbeats are + /// suppressed and silence says nothing about completion. Injectable for + /// tests. "No window" counts as occluded, which also covers panes + /// incubated off-screen (the incubator window is never visible). + @ObservationIgnored + var paneIsOccluded: (Pane) -> Bool = { pane in + !(pane.nsView?.window?.occlusionState.contains(.visible) ?? false) + } + + /// Panes that were occluded on the previous poll tick, so the visible + /// transition can restart their quiet window before settling resumes. + @ObservationIgnored + private var previouslyOccludedPanes: Set = [] + init(workspaceStore: WorkspaceStore = WorkspaceStore()) { self.workspaceStore = workspaceStore autoTileObserver = NotificationCenter.default.addObserver( @@ -118,12 +133,14 @@ final class AppState { // this feature. let trackExecution = Preferences.shared.showTabStatusIndicator var didAcknowledgeCompletion = false + var seenPanes: Set = [] for (projectID, ws) in workspaces { for tab in ws.tabs { for pane in tab.splitRoot.allPanes() { + seenPanes.insert(pane.id) pane.refreshForegroundProcess(trackExecution: trackExecution) if trackExecution { - pane.settleTerminalActivityIfQuiet() + settleIfVisible(pane) } didAcknowledgeCompletion = acknowledgeFinishedCommandIfActive( paneID: pane.id, @@ -133,9 +150,26 @@ final class AppState { } } } + previouslyOccludedPanes.formIntersection(seenPanes) if didAcknowledgeCompletion { saveWorkspaces() } } + /// Quiet-settle only while the surface actually renders: an occluded pane + /// emits no activity heartbeats (its renderer is parked), so settling it + /// would misread suppressed output as completion. On the occluded→visible + /// edge the quiet window restarts, giving a still-running program time to + /// deliver heartbeats again before the settle can fire. + private func settleIfVisible(_ pane: Pane) { + if paneIsOccluded(pane) { + previouslyOccludedPanes.insert(pane.id) + return + } + if previouslyOccludedPanes.remove(pane.id) != nil { + pane.refreshTerminalActivityWindow() + } + pane.settleTerminalActivityIfQuiet() + } + private func recordProjectVisit(_ projectID: UUID) { projectRecency.push(projectID) UserDefaults.standard.set(projectRecency.items.map(\.uuidString), forKey: recencyKey) diff --git a/Macterm/Model/SplitNode.swift b/Macterm/Model/SplitNode.swift index b3432b0..fec5aee 100644 --- a/Macterm/Model/SplitNode.swift +++ b/Macterm/Model/SplitNode.swift @@ -166,6 +166,16 @@ struct TerminalExecutionTracker { return .done } + /// Restart the quiet window of an activity-sourced run. Used on the + /// occluded→visible edge: a parked renderer emits no heartbeats, so the + /// elapsed silence says nothing about completion — and a false `.done` + /// would stick, because activity can never revive `.done` (see + /// `markTerminalActivity`). + mutating func refreshActivityWindow(now: Date) { + guard case .activity = runningSource else { return } + runningSource = .activity(now) + } + mutating func refreshForeground( name: String?, pid: pid_t?, @@ -354,6 +364,10 @@ final class Pane: Identifiable { ) } + func refreshTerminalActivityWindow(now: Date = Date()) { + executionTracker.refreshActivityWindow(now: now) + } + @discardableResult func acknowledgeCommandCompletion() -> Bool { guard executionState == .done else { return false } diff --git a/MactermTests/App/AppStateTests.swift b/MactermTests/App/AppStateTests.swift index 7a284a9..2c139b6 100644 --- a/MactermTests/App/AppStateTests.swift +++ b/MactermTests/App/AppStateTests.swift @@ -652,4 +652,76 @@ struct AppStateTests { let ws = Workspace(projectID: pid, tabs: [tab], activeTabID: tab.id) #expect(AppState.panesToWarm(in: ws).isEmpty) } + + // MARK: - Occlusion-aware quiet-settle + + /// Seed a project whose single pane is running from terminal activity + /// that went quiet long ago (well past the 3s settle threshold). + private func seedQuietRunningPane( + _ state: AppState + ) throws -> Pane { + let p = seedProject(state) + let pane = try #require(state.workspaces[p.id]?.activeTab?.splitRoot.allPanes().first) + pane.recordUserInteraction() + pane.markTerminalActivity(at: Date().addingTimeInterval(-10)) + #expect(pane.executionState == .running) + return pane + } + + private func withStatusIndicator(_ body: () throws -> Void) rethrows { + let prior = Preferences.shared.showTabStatusIndicator + Preferences.shared.showTabStatusIndicator = true + defer { Preferences.shared.showTabStatusIndicator = prior } + try body() + } + + @Test + func occluded_pane_does_not_quiet_settle() throws { + try withStatusIndicator { + let state = makeAppState() + let pane = try seedQuietRunningPane(state) + state.paneIsOccluded = { _ in true } + + state.refreshAllForegroundProcesses() + // 10s of silence, but the renderer was parked — silence proves + // nothing, the pane must stay running. + #expect(pane.executionState == .running) + } + } + + @Test + func visible_pane_still_quiet_settles() throws { + try withStatusIndicator { + let state = makeAppState() + let pane = try seedQuietRunningPane(state) + state.paneIsOccluded = { _ in false } + + state.refreshAllForegroundProcesses() + #expect(pane.executionState == .done) + } + } + + @Test + func deoccluded_pane_gets_fresh_quiet_window_before_settling() throws { + try withStatusIndicator { + let state = makeAppState() + let pane = try seedQuietRunningPane(state) + + // Tick 1: occluded — no settle. + state.paneIsOccluded = { _ in true } + state.refreshAllForegroundProcesses() + #expect(pane.executionState == .running) + + // Tick 2: now visible. The stale 10s-old activity timestamp must + // not settle it instantly — `.done` would stick, since activity + // can never revive a done pane. The window restarts instead. + state.paneIsOccluded = { _ in false } + state.refreshAllForegroundProcesses() + #expect(pane.executionState == .running) + + // With heartbeats back and genuine quiet elapsing, it settles. + pane.settleTerminalActivityIfQuiet(now: Date().addingTimeInterval(4)) + #expect(pane.executionState == .done) + } + } } From 4badb4f379b085d3720813feb2cbb2ced797c7d7 Mon Sep 17 00:00:00 2001 From: Ethan Lee Date: Sat, 4 Jul 2026 15:52:13 +0900 Subject: [PATCH 2/2] Test the occlusion guard directly, not through the full poll tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tick-based tests raced the parallel runner: they toggled the shared Preferences.showTabStatusIndicator singleton and drove the whole refreshAllForegroundProcesses path (which also re-reads a live surface's foreground pid — nil under test — clearing the run source). CI's clean UserDefaults + parallel scheduling surfaced the flake the local run hid. settleIfVisible is now internal and tested directly with an injected paneIsOccluded closure: no global mutation, no surface dependency, and a truer unit of the guard this PR adds. --- Macterm/App/AppState.swift | 6 +- MactermTests/App/AppStateTests.swift | 92 +++++++++++++--------------- 2 files changed, 47 insertions(+), 51 deletions(-) diff --git a/Macterm/App/AppState.swift b/Macterm/App/AppState.swift index ce67b9a..dd94c82 100644 --- a/Macterm/App/AppState.swift +++ b/Macterm/App/AppState.swift @@ -159,7 +159,11 @@ final class AppState { /// would misread suppressed output as completion. On the occluded→visible /// edge the quiet window restarts, giving a still-running program time to /// deliver heartbeats again before the settle can fire. - private func settleIfVisible(_ pane: Pane) { + /// + /// Not private so tests can drive the guard directly (`paneIsOccluded` is + /// injectable) without a live surface or mutating the `Preferences` + /// singleton the poll reads. + func settleIfVisible(_ pane: Pane) { if paneIsOccluded(pane) { previouslyOccludedPanes.insert(pane.id) return diff --git a/MactermTests/App/AppStateTests.swift b/MactermTests/App/AppStateTests.swift index 2c139b6..7dcf648 100644 --- a/MactermTests/App/AppStateTests.swift +++ b/MactermTests/App/AppStateTests.swift @@ -655,73 +655,65 @@ struct AppStateTests { // MARK: - Occlusion-aware quiet-settle - /// Seed a project whose single pane is running from terminal activity - /// that went quiet long ago (well past the 3s settle threshold). - private func seedQuietRunningPane( - _ state: AppState - ) throws -> Pane { - let p = seedProject(state) - let pane = try #require(state.workspaces[p.id]?.activeTab?.splitRoot.allPanes().first) + // These drive `AppState.settleIfVisible` directly with an injected + // occlusion closure, rather than the full `refreshAllForegroundProcesses` + // tick. The tick also re-reads each pane's real foreground process (nil in + // a unit test with no live surface, which clears the run source) and is + // gated on the `Preferences.shared.showTabStatusIndicator` singleton — + // mutating that global races the parallel test runner. Testing the guard + // in isolation is both deterministic and a truer unit of what PR adds. + + /// A pane whose activity went quiet long ago (past the 3s settle window), + /// so a visible settle resolves it to `.done` and an occluded one holds. + private func quietRunningPane() -> Pane { + let pane = Pane(projectPath: "/tmp", projectID: UUID()) pane.recordUserInteraction() pane.markTerminalActivity(at: Date().addingTimeInterval(-10)) #expect(pane.executionState == .running) return pane } - private func withStatusIndicator(_ body: () throws -> Void) rethrows { - let prior = Preferences.shared.showTabStatusIndicator - Preferences.shared.showTabStatusIndicator = true - defer { Preferences.shared.showTabStatusIndicator = prior } - try body() - } - @Test - func occluded_pane_does_not_quiet_settle() throws { - try withStatusIndicator { - let state = makeAppState() - let pane = try seedQuietRunningPane(state) - state.paneIsOccluded = { _ in true } + func occluded_pane_does_not_quiet_settle() { + let state = makeAppState() + let pane = quietRunningPane() + state.paneIsOccluded = { _ in true } - state.refreshAllForegroundProcesses() - // 10s of silence, but the renderer was parked — silence proves - // nothing, the pane must stay running. - #expect(pane.executionState == .running) - } + state.settleIfVisible(pane) + // 10s of silence, but the renderer was parked — silence proves + // nothing, so the pane must stay running. + #expect(pane.executionState == .running) } @Test - func visible_pane_still_quiet_settles() throws { - try withStatusIndicator { - let state = makeAppState() - let pane = try seedQuietRunningPane(state) - state.paneIsOccluded = { _ in false } + func visible_pane_still_quiet_settles() { + let state = makeAppState() + let pane = quietRunningPane() + state.paneIsOccluded = { _ in false } - state.refreshAllForegroundProcesses() - #expect(pane.executionState == .done) - } + state.settleIfVisible(pane) + #expect(pane.executionState == .done) } @Test - func deoccluded_pane_gets_fresh_quiet_window_before_settling() throws { - try withStatusIndicator { - let state = makeAppState() - let pane = try seedQuietRunningPane(state) + func deoccluded_pane_gets_fresh_quiet_window_before_settling() { + let state = makeAppState() + let pane = quietRunningPane() - // Tick 1: occluded — no settle. - state.paneIsOccluded = { _ in true } - state.refreshAllForegroundProcesses() - #expect(pane.executionState == .running) + // Occluded: no settle, and the pane is marked as having been occluded. + state.paneIsOccluded = { _ in true } + state.settleIfVisible(pane) + #expect(pane.executionState == .running) - // Tick 2: now visible. The stale 10s-old activity timestamp must - // not settle it instantly — `.done` would stick, since activity - // can never revive a done pane. The window restarts instead. - state.paneIsOccluded = { _ in false } - state.refreshAllForegroundProcesses() - #expect(pane.executionState == .running) + // Now visible. The stale 10s-old activity timestamp must not settle it + // instantly — a false `.done` would stick, since activity can never + // revive a done pane. The window restarts instead. + state.paneIsOccluded = { _ in false } + state.settleIfVisible(pane) + #expect(pane.executionState == .running) - // With heartbeats back and genuine quiet elapsing, it settles. - pane.settleTerminalActivityIfQuiet(now: Date().addingTimeInterval(4)) - #expect(pane.executionState == .done) - } + // With genuine quiet now elapsing from the reset window, it settles. + pane.settleTerminalActivityIfQuiet(now: Date().addingTimeInterval(4)) + #expect(pane.executionState == .done) } }