From 5059793dfaa7f02ad52d7e363b977e9061832241 Mon Sep 17 00:00:00 2001 From: Mateusz Urban Date: Fri, 24 Jul 2026 11:25:08 +0200 Subject: [PATCH 1/6] fix: detect in-place terminal activity reliably Use Ghostty's IO-path output heartbeat to track raw-mode and occluded work without relying on renderer activity. Preserve foreground/progress authority, require committed input evidence for ambiguous same-PID agent commands, and quiet-settle activity with a non-coalesced deadline. Expose the additive execution state through the CLI, show active-tab status without special masking, and reject GhosttyKit artifacts missing the required ABI. --- AGENTS.md | 4 +- CLI/Output.swift | 1 + Macterm/App/AppState.swift | 64 +++- Macterm/App/Notifications.swift | 4 + Macterm/Control/ControlHandler.swift | 13 +- Macterm/Control/ControlProtocol.swift | 5 + Macterm/Ghostty/GhosttyCallbacks.swift | 24 +- Macterm/Model/SplitNode.swift | 311 +++++++++++++--- Macterm/Views/Sidebar.swift | 21 +- .../Terminal/GhosttyTerminalNSView.swift | 127 ++++++- .../Terminal/TerminalCommandSubmission.swift | 66 ++++ Macterm/Views/TerminalPane.swift | 24 +- MactermTests/App/AppStateTests.swift | 37 ++ .../Control/ControlHandlerTests.swift | 17 + .../Control/ControlProtocolTests.swift | 24 ++ MactermTests/Model/PaneTests.swift | 91 ++++- .../Model/TerminalExecutionTrackerTests.swift | 347 ++++++++++++++++-- .../TerminalCommandSubmissionTests.swift | 132 +++++++ scripts/setup.sh | 19 +- website/docs/pages/80-cli.md | 2 +- 20 files changed, 1176 insertions(+), 157 deletions(-) create mode 100644 Macterm/Views/Terminal/TerminalCommandSubmission.swift create mode 100644 MactermTests/Views/Terminal/TerminalCommandSubmissionTests.swift diff --git a/AGENTS.md b/AGENTS.md index 244c3ff7..2e51e422 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,7 @@ mise run bench # Release build + window-state resource benchmark Requires macOS 14+, Swift 6.0+. Liquid glass and a few chrome refinements are macOS 26 (Tahoe) features that degrade gracefully on older systems (gated behind `WindowAppearance.glassSupported` / `#available`). GhosttyKit is a pre-built xcframework from `thdxg/ghostty` (a fork that adds CI builds); no zig toolchain needed. -`GhosttyKit.xcframework` and the `Macterm/Resources/{ghostty,terminfo,…}` contents are gitignored artifacts downloaded by `mise run setup` — **every fresh checkout, including a git worktree, must run `mise run setup`** before it can build. Don't symlink them from another checkout: `setup.sh` only re-downloads when the artifact is _absent_ (presence check, not version check), so a symlinked copy silently goes stale. To refresh a stale artifact, delete it and re-run setup. +`GhosttyKit.xcframework` and the `Macterm/Resources/{ghostty,terminfo,…}` contents are gitignored artifacts downloaded by `mise run setup` — **every fresh checkout, including a git worktree, must run `mise run setup`** before it can build. Don't symlink them from another checkout: setup normally uses a presence check rather than a version check, so a symlinked copy silently goes stale. The one capability exception is `GHOSTTY_ACTION_OUTPUT_ACTIVITY`: setup replaces a present framework that lacks this required ABI. To refresh any other stale artifact, delete it and re-run setup. ## Releasing & Updates @@ -70,7 +70,7 @@ All keybinds are configurable via `HotkeyAction` + `HotkeyRegistry`. `KeyRouter` ### Tab Naming -A tab's auto-title is, by default, the pane's live **foreground process name** (`hx`, `btop`) — falling back to the login shell name (from `getpwuid`, not `$SHELL`) when idle, overridden by a user-set `customTitle`. `ProcessInspector.runningProcessName` reads the foreground pid's kernel `comm`. `AppState` polls panes adaptively (`PollCadence` + `refreshAllForegroundProcesses`, republishing only on change): 250ms during a ~5s burst after any poll event (tab switch, keystroke, OSC title, execution transition — all post `.terminalPollEvent`) or while a command runs frontmost, 1s when active-idle, 2s when inactive with a visible window, stopped when nothing is on screen (events resume it instantly; the quit dialog re-reads names one-shot since the poll may be paused). The status indicator's quiet-settle is skipped for occluded panes — their parked renderer emits no heartbeats, so silence proves nothing — with a fresh quiet window granted on the occluded→visible edge. OSC 0/2 titles are **provenance-gated** (`Pane.receiveReportedTitle`): the raw sequence can't distinguish a program naming its session (claude, ssh) from a shell titling its prompt (nushell, Starship emit the cwd), so the title string is adopted as `Pane.programTitle` only while the foreground process is a real program — not a shell — and is pinned to that pid; the poll expires it when the pid loses the foreground (`applyForegroundRefresh`), and prompt-time titles are discarded. `displayTitle` prefers `programTitle` over the process name; the quit dialog keeps the process-derived `processTitle`. Every OSC title arrival also triggers a process refresh (command boundary). Titles aren't persisted — always derived live. +A tab's auto-title is, by default, the pane's live **foreground process name** (`hx`, `btop`) — falling back to the login shell name (from `getpwuid`, not `$SHELL`) when idle, overridden by a user-set `customTitle`. `ProcessInspector.runningProcessName` reads the foreground pid's kernel `comm`. `AppState` polls panes adaptively (`PollCadence` + `refreshAllForegroundProcesses`, republishing only on change): 250ms during a ~5s burst after any poll event (tab switch, keystroke, OSC title, execution transition — all post `.terminalPollEvent`) or while a command runs frontmost, 1s when active-idle, 2s when inactive with a visible window, stopped when nothing is on screen (events resume it instantly; the quit dialog re-reads names one-shot since the poll may be paused). A throttled `GHOSTTY_ACTION_OUTPUT_ACTIVITY` heartbeat from the pty IO path (unlike the render-path `GHOSTTY_ACTION_SCROLLBAR`, it keeps firing while occluded) keeps activity-sourced runs alive during in-place redraws and lets the indicator quiet-settle occluded panes normally once a pane proves it receives one (`Pane.hasOcclusionIndependentHeartbeat`); the old occlusion exemption survives only as a fallback before that proof. Row growth may start activity under the normal interaction guards. Non-growing output can start activity only for a recognized AI-agent foreground, as two heartbeats within 2s of an actually forwarded unmodified Return/newline submission with committed prompt content; a genuinely blank submission briefly suppresses even row-growth redraws. This covers Pi's nested `!` commands without treating Return in ordinary raw TUIs, startup output, typing, IME composition, or one prompt redraw as work. OSC 0/2 titles are **provenance-gated** (`Pane.receiveReportedTitle`): the raw sequence can't distinguish a program naming its session (claude, ssh) from a shell titling its prompt (nushell, Starship emit the cwd), so the title string is adopted as `Pane.programTitle` only while the foreground process is a real program — not a shell — and is pinned to that pid; the poll expires it when the pid loses the foreground (`applyForegroundRefresh`), and prompt-time titles are discarded. `displayTitle` prefers `programTitle` over the process name; the quit dialog keeps the process-derived `processTitle`. Every OSC title arrival also triggers a process refresh (command boundary). Titles aren't persisted — always derived live. **Remote panes** (#104) leave the local pipeline entirely — the local process table only knows the `ssh` client, so `ProcessInspector.foregroundPID(forPane:)` returns nil for them (which also makes layout `save` emit plain leaves and reconcile match them as idle). Their naming is two-tier: OSC 0/2 titles gated by the OSC 133 execution state instead of a pid (`Pane.receiveRemoteReportedTitle` — adopt while running, discard prompt churn, expire on the running→ended edge), plus `RemoteForegroundResolver`, one BatchMode ssh per host per ~3s (frontmost project only, overlapping probes dropped, failures freeze names) running the same session→leader→tpgid→comm pipeline as a portable POSIX script (`RemoteSpawn.foregroundProbeScript`). Idle fallback is the host name. diff --git a/CLI/Output.swift b/CLI/Output.swift index b7de5293..90d2ee47 100644 --- a/CLI/Output.swift +++ b/CLI/Output.swift @@ -67,6 +67,7 @@ enum Output { pane.session, pane.process ?? "-", pane.cwd ?? "-", + pane.state ?? "-", ] } printColumns(rows) diff --git a/Macterm/App/AppState.swift b/Macterm/App/AppState.swift index 4341f473..77834b1e 100644 --- a/Macterm/App/AppState.swift +++ b/Macterm/App/AppState.swift @@ -163,16 +163,21 @@ final class AppState { /// 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). + /// suppressed and (absent an occlusion-independent output heartbeat, + /// see `Pane.hasOcclusionIndependentHeartbeat`) 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. + /// Panes that were occluded on the previous poll tick (and lack an + /// occlusion-independent heartbeat), so the visible transition can + /// restart their quiet window before settling resumes. A pane that gains + /// the heartbeat is dropped from here immediately (see `settleIfVisible`) + /// so a later, unrelated occlusion edge can't replay stale bookkeeping. @ObservationIgnored private var previouslyOccludedPanes: Set = [] @@ -236,8 +241,20 @@ final class AppState { let onEvent: @Sendable (Notification) -> Void = { [weak self] _ in MainActor.assumeIsolated { self?.notePollEvent() } } + let onQuietSettleDeadline: @Sendable (Notification) -> Void = { [weak self] _ in + // Do not route through notePollEvent: if another poll ran within + // 250ms, coalescing plus a fully occluded window would pause with + // no timer and never retry this deadline. + MainActor.assumeIsolated { self?.pollNow() } + } let tokens: [(NotificationCenter, NSObjectProtocol)] = [ (center, center.addObserver(forName: .terminalPollEvent, object: nil, queue: .main, using: onEvent)), + (center, center.addObserver( + forName: .terminalQuietSettleDeadline, + object: nil, + queue: .main, + using: onQuietSettleDeadline + )), (center, center.addObserver( forName: NSApplication.didBecomeActiveNotification, object: nil, queue: .main, using: onEvent )), @@ -415,16 +432,33 @@ final class AppState { } } - /// 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. + /// Quiet-settle a pane, working around the occluded renderer's silence + /// where necessary. A pane whose GhosttyKit build delivers + /// `OUTPUT_ACTIVITY` heartbeats (`hasOcclusionIndependentHeartbeat`) + /// proved those heartbeats reach it regardless of occlusion, so its + /// silence is meaningful even while occluded — settle it exactly like a + /// visible pane, with no skip and no fresh-window grant on de-occlusion. + /// + /// Absent that proof (older GhosttyKit, or no heartbeat has arrived yet), + /// fall back to the original #123 behavior: an occluded pane's renderer + /// is parked, so it emits no activity heartbeats at all, and settling it + /// would misread that 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. /// /// 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 pane.hasOcclusionIndependentHeartbeat { + // This pane's silence is trustworthy even while occluded — drop + // any bookkeeping from before the heartbeat proved that, so a + // later occlusion edge (if the flag were ever meaningful again) + // can't replay a stale grant. + previouslyOccludedPanes.remove(pane.id) + pane.settleTerminalActivityIfQuiet() + return + } if paneIsOccluded(pane) { previouslyOccludedPanes.insert(pane.id) return @@ -1469,12 +1503,10 @@ final class AppState { projectID: UUID, saveImmediately: Bool = true ) -> Bool { - // The sidebar shows the *entire* active tab as idle (displayState masks - // `.done` for the tab the user is looking at), so every pane in that tab - // must actually be cleared — not just the focused one. Otherwise a - // non-focused split pane that finished a command stays `.done` under the - // hood, gets persisted, and reappears as a checkmark after restart even - // though the user saw an empty circle. + // Looking at the active tab acknowledges completion for the whole tab, + // not only its focused pane. Otherwise a non-focused split pane that + // finished a command stays `.done` under the hood, gets persisted, and + // reappears as a status dot after the user switches away or restarts. // Route through the injected `isAppActive` seam (not `NSApp.isActive` // directly): NSApp is nil during construction and unset in tests, and // this path is reachable from init via pollNow(). diff --git a/Macterm/App/Notifications.swift b/Macterm/App/Notifications.swift index 4c8ccc2a..7c795c12 100644 --- a/Macterm/App/Notifications.swift +++ b/Macterm/App/Notifications.swift @@ -9,6 +9,10 @@ extension Notification.Name { /// poll: tab switch, OSC title, user interaction, execution-state /// transition. Observed by `AppState.notePollEvent()`. static let terminalPollEvent = Notification.Name("MactermTerminalPollEvent") + /// A final IO heartbeat's quiet deadline. Unlike ordinary poll events, + /// this must force one poll even when coalescing and window occlusion would + /// otherwise leave the timer paused. + static let terminalQuietSettleDeadline = Notification.Name("MactermTerminalQuietSettleDeadline") /// A zmx session was created, killed, or reattached — the /// `ZmxForegroundResolver` name→leader-pid cache is stale. Observed by /// `AppState`, which invalidates its `ZmxRefreshGate` and wakes the poll. diff --git a/Macterm/Control/ControlHandler.swift b/Macterm/Control/ControlHandler.swift index bc724580..383019f6 100644 --- a/Macterm/Control/ControlHandler.swift +++ b/Macterm/Control/ControlHandler.swift @@ -769,10 +769,21 @@ final class ControlHandler { title: pane.displayTitle, process: pane.foregroundProcessName, cwd: pane.nsView?.currentPwd ?? pane.projectPath, - focused: tab.id == workspace.activeTabID && pane.id == tab.focusedPaneID + focused: tab.id == workspace.activeTabID && pane.id == tab.focusedPaneID, + state: controlState(for: pane.executionState) ) } + /// Wire representation of `TerminalExecutionState` — a plain string keeps + /// the protocol's JSON stable even if the enum's cases are renamed. + private func controlState(for state: TerminalExecutionState) -> String { + switch state { + case .idle: "idle" + case .running: "running" + case .done: "done" + } + } + private func paneIDsBySessionName() -> [String: String] { var map: [String: String] = [:] for workspace in appState.workspaces.values { diff --git a/Macterm/Control/ControlProtocol.swift b/Macterm/Control/ControlProtocol.swift index 9aa703f6..b67eb2b1 100644 --- a/Macterm/Control/ControlProtocol.swift +++ b/Macterm/Control/ControlProtocol.swift @@ -244,6 +244,11 @@ struct ControlPaneInfo: Codable, Equatable { var process: String? var cwd: String? var focused: Bool + /// The tab activity indicator's underlying state: `idle`, `running`, or + /// `done` (finished while unfocused — the indicator scripts want to poll + /// for). Optional per the additive-field convention above — nil when + /// decoded from an older server that predates this field. + var state: String? } struct ControlSessionInfo: Codable, Equatable { diff --git a/Macterm/Ghostty/GhosttyCallbacks.swift b/Macterm/Ghostty/GhosttyCallbacks.swift index 185b5d01..043693a7 100644 --- a/Macterm/Ghostty/GhosttyCallbacks.swift +++ b/Macterm/Ghostty/GhosttyCallbacks.swift @@ -68,12 +68,16 @@ final class GhosttyCallbacks: @unchecked Sendable { let s = action.action.scrollbar DispatchQueue.main.async { view.surfaceDidUpdateScrollbar(total: s.total, offset: s.offset, len: s.len) } return true - case GHOSTTY_ACTION_RENDER: - guard let view = surfaceView(from: target) else { return false } - DispatchQueue.main.async { view.surfaceDidRender() } - // Do not consume the action: keep libghostty's existing render path - // unchanged, and use this only as an activity signal. - return false + case GHOSTTY_ACTION_OUTPUT_ACTIVITY: + // Throttled heartbeat from the pty IO path — fires while the + // program produces output, even when the surface is occluded and + // the renderer (and thus the scrollbar action) is parked. Carries + // the same geometry as GHOSTTY_ACTION_SCROLLBAR so the tracker can + // tell real output growth from in-place redraws. + guard let view = surfaceView(from: target) else { return true } + let s = action.action.output_activity + DispatchQueue.main.async { view.surfaceDidOutputActivity(total: s.total, offset: s.offset, len: s.len) } + return true case GHOSTTY_ACTION_RELOAD_CONFIG: // libghostty fires this (with soft = true) when a surface's // conditional state changes — notably on set_color_scheme, which @@ -117,8 +121,14 @@ final class GhosttyCallbacks: @unchecked Sendable { // `ghostty_surface_complete_clipboard_request`'s surface parameter is // non-null on the Zig side; a request completing during surface // teardown (nil `surface`) is UB, not a graceful no-op. Guard it. - guard let surface = surface(from: ud) else { return false } + guard let ud else { return false } + let view = Unmanaged.fromOpaque(ud).takeUnretainedValue() + guard let surface = view.surface else { return false } let text = Self.readPasteboardText() ?? "" + // Record the resolved payload, not just the Command-V key code: an + // empty/whitespace clipboard must not make a later blank Return look + // like a nonempty agent submission. + DispatchQueue.main.async { view.surfaceDidPasteText(text) } text.withCString { ghostty_surface_complete_clipboard_request(surface, $0, state, false) } return true } diff --git a/Macterm/Model/SplitNode.swift b/Macterm/Model/SplitNode.swift index 907b3a22..feb39a63 100644 --- a/Macterm/Model/SplitNode.swift +++ b/Macterm/Model/SplitNode.swift @@ -71,6 +71,13 @@ private enum TerminalExecutionSource: Equatable { } struct TerminalExecutionTracker { + private enum PendingOutputStart { + case armed(Date) + case candidate(Date) + } + + private static let submissionWindow: TimeInterval = 2 + init(hasUserInteraction: Bool = false) { self.hasUserInteraction = hasUserInteraction } @@ -80,39 +87,83 @@ struct TerminalExecutionTracker { /// every poll — so a settled process doesn't flip-flop back to running just /// because it is still foreground. private var lastForeground: ForegroundProcessKey? + /// Last observed tty input mode for `lastForeground`. Mode transitions are + /// meaningful even when the pid/name key is unchanged. + private var lastTerminalInputWasRaw: Bool? /// Why the pane is currently considered running. Foreground and explicit - /// progress run until a completion/foreground transition; activity is a - /// render/output heartbeat and quiet-settles. + /// progress run until a completion/foreground transition; activity is an + /// output heartbeat and quiet-settles. private var runningSource: TerminalExecutionSource? /// After progress clears, the foreground process that owned it is /// "quiesced": its own output and re-polls are ignored until the foreground - /// moves away, so a settled program that reported progress doesn't flip back - /// to running on its own render output. `pendingProgressQuiesce` covers the - /// race where progress started and cleared before any foreground poll. + /// moves away. `pendingProgressQuiesce` covers progress that starts and + /// clears before any foreground poll. private var progressQuiesced: ForegroundProcessKey? private var pendingProgressQuiesce = false - /// Output is ignored until the user has interacted with the pane, so a - /// freshly-restored shell's startup prompt doesn't show as activity. + /// Startup output is ignored until the pane has received user input (or a + /// declarative `run:`, which seeds this at initialization). private var hasUserInteraction = false + /// Geometry baseline carried by the IO-path output heartbeat. Growth is + /// strong activity evidence; equal totals describe an in-place redraw. + private var lastOutputRows: UInt64? + /// A narrowly-armed path for work nested inside a recognized AI agent. The + /// first in-place output heartbeat is only a candidate; a second within the + /// submission window confirms sustained work. + private var pendingOutputStart: PendingOutputStart? + /// A forwarded Return with no committed prompt text can still make a TUI + /// redraw or grow rows. Suppress those start signals briefly so an empty + /// submission cannot flash the spinner. + private var blankSubmissionAt: Date? + + var isActivitySourced: Bool { + if case .activity = runningSource { return true } + return false + } mutating func recordUserInteraction() { hasUserInteraction = true + // Typing, scrolling, or any other interaction after Return means later + // redraws can no longer be attributed to that submission. + pendingOutputStart = nil + } + + mutating func recordCommandSubmission( + at date: Date, + allowInPlaceOutputStart: Bool, + hasContent: Bool + ) { + hasUserInteraction = true + guard hasContent else { + pendingOutputStart = nil + blankSubmissionAt = date + return + } + // A deliberate nonempty submission supersedes a process quiesced by an + // earlier progress report, even when a long-lived TUI retains its pid. + progressQuiesced = nil + pendingProgressQuiesce = false + blankSubmissionAt = nil + pendingOutputStart = allowInPlaceOutputStart ? .armed(date) : nil } mutating func markProgressStarted(currentState: TerminalExecutionState) -> TerminalExecutionState { + pendingOutputStart = nil + blankSubmissionAt = nil guard hasUserInteraction else { return currentState } runningSource = .progress return .running } mutating func markCommandFinished(currentState: TerminalExecutionState) -> TerminalExecutionState { - // Shell integration (OSC 133;D) fires on *every* precmd, including - // empty commands — pressing Enter, Ctrl-C, or Ctrl-L on an idle prompt - // emits COMMAND_FINISHED with no preceding command. Only treat it as a - // real completion when a command was actually running; from idle it's - // precmd noise and must not flip the pane to `.done` (which would - // persist as a spurious checkmark after restart). + // Shell integration (OSC 133;D) fires on every precmd, including an + // empty Return. Always cancel its submission candidate, but only show a + // completion when a command was genuinely running. + pendingOutputStart = nil + // OSC 133;D for an empty Return may arrive before its redraw/output. + // Keep blank suppression while idle so that later callback cannot flash + // the spinner; a genuine running completion no longer needs it. guard currentState == .running else { return currentState } + blankSubmissionAt = nil runningSource = nil progressQuiesced = nil pendingProgressQuiesce = false @@ -120,6 +171,8 @@ struct TerminalExecutionTracker { } mutating func markProgressFinished(currentState: TerminalExecutionState) -> TerminalExecutionState { + pendingOutputStart = nil + blankSubmissionAt = nil guard hasUserInteraction || runningSource == .progress else { return currentState } if let lastForeground { progressQuiesced = lastForeground @@ -134,25 +187,87 @@ struct TerminalExecutionTracker { at date: Date, currentState: TerminalExecutionState ) -> TerminalExecutionState { - // A render/output heartbeat can keep an already-running command active, - // but it must never (re)start one. From `.done` — a finished command - // whose checkmark is showing — output (e.g. a background job) must not - // flip the pane back to running; only a new foreground process or an - // explicit progress marker can. Pinned by TerminalExecutionTrackerTests - // so a refactor of the onTerminalRender closure can't silently - // reintroduce the "prompt redraw keeps spinning" bug. + // Output may start an idle, interacted-with pane or sustain an + // activity-owned run. It must not resurrect `.done`, override explicit + // progress, or demote a canonical foreground command into a run that + // quiet-settles while its process is still alive. guard currentState != .done else { return currentState } + guard !shouldSuppressOutputStart(at: date, currentState: currentState) else { return currentState } guard runningSource != .progress else { return currentState } + guard runningSource != .foreground else { return currentState } if let progressQuiesced, progressQuiesced == lastForeground { return currentState } - // Output/render only counts after user interaction (or a declarative - // `run:`, which seeds `hasUserInteraction`). Fresh/restored shells can - // emit startup banners or shell-integration redraws before the user does - // anything; those must not become persisted completion indicators. guard hasUserInteraction else { return currentState } + pendingOutputStart = nil runningSource = .activity(date) return .running } + /// Handle an occlusion-independent, throttled heartbeat from libghostty's + /// pty IO path. Scrollback growth is strong evidence and follows the normal + /// activity guards. Equal row totals only sustain an activity-owned run, + /// except for two heartbeats immediately following an explicitly armed + /// agent submission (the same-raw-pid Pi `! sleep` case). + mutating func markOutputActivity( + totalRows: UInt64, + at date: Date, + currentState: TerminalExecutionState + ) -> TerminalExecutionState { + let grew = lastOutputRows.map { totalRows > $0 } ?? false + lastOutputRows = totalRows + if grew { + return markTerminalActivity(at: date, currentState: currentState) + } + + if currentState == .running, case .activity = runningSource { + pendingOutputStart = nil + runningSource = .activity(date) + return currentState + } + + guard currentState == .idle else { + pendingOutputStart = nil + return currentState + } + guard hasUserInteraction else { return currentState } + guard runningSource != .progress else { return currentState } + if let progressQuiesced, progressQuiesced == lastForeground { return currentState } + + switch pendingOutputStart { + case let .armed(submittedAt): + guard isWithinSubmissionWindow(date, submittedAt: submittedAt) else { + pendingOutputStart = nil + return currentState + } + pendingOutputStart = .candidate(submittedAt) + return currentState + case let .candidate(submittedAt): + guard isWithinSubmissionWindow(date, submittedAt: submittedAt) else { + pendingOutputStart = nil + return currentState + } + pendingOutputStart = nil + runningSource = .activity(date) + return .running + case nil: + return currentState + } + } + + private func isWithinSubmissionWindow(_ date: Date, submittedAt: Date) -> Bool { + let elapsed = date.timeIntervalSince(submittedAt) + return elapsed >= 0 && elapsed < Self.submissionWindow + } + + private mutating func shouldSuppressOutputStart( + at date: Date, + currentState: TerminalExecutionState + ) -> Bool { + guard currentState == .idle, let blankSubmissionAt else { return false } + if isWithinSubmissionWindow(date, submittedAt: blankSubmissionAt) { return true } + self.blankSubmissionAt = nil + return false + } + mutating func settleIfQuiet( now: Date, quietInterval: TimeInterval, @@ -163,14 +278,14 @@ struct TerminalExecutionTracker { now.timeIntervalSince(lastActivityAt) >= quietInterval else { return currentState } runningSource = nil + pendingOutputStart = nil + blankSubmissionAt = nil 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`). + /// Restart the quiet window of an activity-sourced run. Used only by the + /// compatibility path for surfaces that have not proven they receive the + /// occlusion-independent heartbeat. mutating func refreshActivityWindow(now: Date) { guard case .activity = runningSource else { return } runningSource = .activity(now) @@ -181,13 +296,24 @@ struct TerminalExecutionTracker { pid: pid_t?, foregroundIsShell: Bool, terminalInputIsRaw: Bool, + at date: Date = Date(), currentState: TerminalExecutionState ) -> TerminalExecutionState { let newKey = foregroundIsShell ? nil : ForegroundProcessKey(name: name, pid: pid) + let changed = newKey != lastForeground + let returnedToCanonical = !changed + && lastTerminalInputWasRaw == true + && !terminalInputIsRaw + lastTerminalInputWasRaw = newKey == nil ? nil : terminalInputIsRaw + // An authoritative process transition supersedes output heuristics. A + // steady raw Pi pid deliberately preserves the submission candidate. + if changed { + pendingOutputStart = nil + blankSubmissionAt = nil + } - // Resolve a pending progress quiesce: the first foreground process - // after a progress race (progress cleared before any poll) is quiesced - // rather than marked running. A shell arriving first cancels it. + // Resolve the race where progress cleared before a foreground poll: the + // first process is quiesced rather than immediately restarted. if pendingProgressQuiesce { if let newKey { progressQuiesced = newKey @@ -198,45 +324,50 @@ struct TerminalExecutionTracker { pendingProgressQuiesce = false } - // Drop the quiesce once the foreground moves off the quiesced process. + // A different foreground process releases progress quiescing. if let progressQuiesced, progressQuiesced != newKey { self.progressQuiesced = nil } - let changed = newKey != lastForeground lastForeground = newKey - // Foreground returned to the shell: a foreground-running command exited. + // Returning to the shell is the authoritative completion edge for a + // foreground command, regardless of whether it was later demoted to + // activity ownership by a canonical→raw transition. if newKey == nil { guard changed, currentState == .running else { return currentState } runningSource = nil return .done } - // Explicit progress owns the state while active. + // Explicit progress owns state while active. Startup foreground noise + // remains ignored until the pane has received trusted input. if runningSource == .progress { return currentState } - - // A newly-created/restored plain shell can briefly look like a non-shell - // foreground while its startup files and shell integration settle. Do - // not turn that launch noise into a persisted checkmark. Once the user - // has interacted, foreground transitions are real user work. guard hasUserInteraction else { return currentState } if terminalInputIsRaw { - // Raw/cbreak-mode programs (editors, multiplexers, interactive CLIs) - // should not be held running by foreground alone. If a canonical - // command switched the tty raw, finish that foreground-only run; - // activity-sourced runs still quiet-settle normally. + // A TUI switching canonical→raw is still working. Demote its + // foreground-owned run so IO heartbeats keep it alive and quiet + // output can settle it, rather than marking it done immediately. if currentState == .running, runningSource == .foreground { - runningSource = nil - return .done + runningSource = .activity(date) } return currentState } - // Canonical non-shell command (a build, `sleep`, shell script, …) → - // running until it returns to the shell. Only act on a change so a - // settled idle process doesn't flip back to running on every poll. + // A same-pid TUI can return from raw to canonical mode while it keeps + // working. Restore foreground authority only while it is still + // running; a same-pid process that already quiet-settled must not be + // resurrected by a later poll. + if returnedToCanonical, currentState == .running, + case .activity = runningSource + { + runningSource = .foreground + return currentState + } + + // A canonical non-shell command is foreground-owned until its process + // changes. Re-polls of the same pid must not restart settled state. guard changed else { return currentState } runningSource = .foreground return .running @@ -357,6 +488,12 @@ final class Pane: Identifiable { @ObservationIgnored private var executionTracker = TerminalExecutionTracker() + /// The global foreground poll pauses when the app has no visible window. + /// Keep one lightweight wake scheduled from the final IO heartbeat so an + /// occluded activity-owned run can still quiet-settle. + @ObservationIgnored + private var activityQuietPollTask: Task? + private let activityQuietPollDelay: Duration /// Re-read the foreground process name from the process table and publish it /// only when it changed (so a steady poll doesn't churn `@Observable` and @@ -436,14 +573,17 @@ final class Pane: Identifiable { func markCommandRunning() { executionState = executionTracker.markProgressStarted(currentState: executionState) + cancelActivityQuietPollIfNeeded() } func markCommandFinished() { executionState = executionTracker.markCommandFinished(currentState: executionState) + cancelActivityQuietPollIfNeeded() } func markProgressFinished() { executionState = executionTracker.markProgressFinished(currentState: executionState) + cancelActivityQuietPollIfNeeded() } func markTerminalActivity(at date: Date = Date()) { @@ -459,12 +599,60 @@ final class Pane: Identifiable { quietInterval: quietInterval, currentState: executionState ) + cancelActivityQuietPollIfNeeded() } func refreshTerminalActivityWindow(now: Date = Date()) { executionTracker.refreshActivityWindow(now: now) } + /// Set on the first `OUTPUT_ACTIVITY` heartbeat delivered by libghostty + /// for this surface. Its presence proves the running GhosttyKit build + /// delivers occlusion-independent heartbeats (they fire from the pty IO + /// path, not the renderer), so `AppState`'s quiet-settle no longer needs + /// the occluded-pane exemption for this pane — silence while occluded is + /// now a meaningful signal instead of an artifact of a parked renderer. + private(set) var hasOcclusionIndependentHeartbeat = false + + /// Handle a throttled `OUTPUT_ACTIVITY` heartbeat (see + /// `TerminalExecutionTracker.markOutputActivity`). Unlike + /// `markTerminalActivity` (scrollbar-growth, visible surfaces only), this + /// also reaches occluded/background panes. + func markOutputActivity(totalRows: UInt64, now: Date = Date()) { + hasOcclusionIndependentHeartbeat = true + executionState = executionTracker.markOutputActivity(totalRows: totalRows, at: now, currentState: executionState) + scheduleActivityQuietPollIfNeeded() + } + + private func scheduleActivityQuietPollIfNeeded() { + guard executionTracker.isActivitySourced else { + cancelActivityQuietPollIfNeeded() + return + } + activityQuietPollTask?.cancel() + let delay = activityQuietPollDelay + activityQuietPollTask = Task { @MainActor [weak self] in + do { + try await Task.sleep(for: delay) + } catch { + return + } + guard let self else { return } + activityQuietPollTask = nil + // This dedicated deadline bypasses ordinary event coalescing: if + // every window is occluded there may be no timer left to retry. + // AppState still performs the settle so acknowledgement and + // persistence stay central. + NotificationCenter.default.post(name: .terminalQuietSettleDeadline, object: self) + } + } + + private func cancelActivityQuietPollIfNeeded() { + guard !executionTracker.isActivitySourced else { return } + activityQuietPollTask?.cancel() + activityQuietPollTask = nil + } + @discardableResult func acknowledgeCommandCompletion() -> Bool { guard executionState == .done else { return false } @@ -490,6 +678,21 @@ final class Pane: Identifiable { NotificationCenter.default.post(name: .terminalPollEvent, object: nil) } + func recordCommandSubmission(hasContent: Bool, at date: Date = Date()) { + // Plain Return is ambiguous in editors and menus. Only a nonempty + // submission in a recognized AI agent gets the two-heartbeat in-place + // start heuristic; ordinary programs still use process/row evidence. + let allowInPlaceOutputStart = agentIcon != nil + || AgentIcon.match(processName: foregroundProcessName) != nil + executionTracker.recordCommandSubmission( + at: date, + allowInPlaceOutputStart: allowInPlaceOutputStart, + hasContent: hasContent + ) + acknowledgeCommandCompletion() + NotificationCenter.default.post(name: .terminalPollEvent, object: nil) + } + private func applyForegroundExecutionState( name: String?, foregroundPID: pid_t?, @@ -503,6 +706,7 @@ final class Pane: Identifiable { terminalInputIsRaw: terminalInputIsRaw, currentState: executionState ) + cancelActivityQuietPollIfNeeded() } /// Handle an OSC 0/2 title reported by the surface. Always refreshes the @@ -650,13 +854,14 @@ final class Pane: Identifiable { view.onSearchSelected = nil view.onFocus = nil view.onInteraction = nil + view.onCommandSubmitted = nil view.onSplitRequest = nil view.onDesktopNotification = nil view.onCommandFinished = nil view.onProgressStarted = nil view.onProgressFinished = nil view.onTerminalActivity = nil - view.onTerminalRender = nil + view.onOutputActivity = nil view.onScrollbarUpdate = nil view.onScrollWheel = nil view.destroySurface() @@ -725,7 +930,8 @@ final class Pane: Identifiable { sessionName persistedSessionName: String? = nil, command: String? = nil, shell: String? = nil, - env: [String: String]? = nil + env: [String: String]? = nil, + activityQuietPollDelay: Duration = .seconds(3) ) { self.projectPath = projectPath self.projectID = projectID @@ -756,6 +962,7 @@ final class Pane: Identifiable { self.command = command self.shell = shell self.env = env + self.activityQuietPollDelay = activityQuietPollDelay executionTracker = TerminalExecutionTracker(hasUserInteraction: command != nil) } diff --git a/Macterm/Views/Sidebar.swift b/Macterm/Views/Sidebar.swift index 7b035b0e..8219a480 100644 --- a/Macterm/Views/Sidebar.swift +++ b/Macterm/Views/Sidebar.swift @@ -145,7 +145,7 @@ struct SidebarContent: View { set: { if $0 { expandedProjects.insert(project.id) } else { expandedProjects.remove(project.id) } } )) { ForEach(Array(tabs.enumerated()), id: \.element.id) { tabIndex, tab in - tabRow(tab: tab, index: tabIndex, activeTabID: ws?.activeTabID, project: project) + tabRow(tab: tab, index: tabIndex, project: project) } // Single drop mechanism for both cases: SwiftUI reports the // insertion `offset` within THIS project's tab list. A drop from @@ -160,11 +160,10 @@ struct SidebarContent: View { } } - private func tabRow(tab: TerminalTab, index tabIndex: Int, activeTabID: UUID?, project: Project) -> some View { + private func tabRow(tab: TerminalTab, index tabIndex: Int, project: Project) -> some View { SidebarTabRow( tab: tab, index: tabIndex + 1, - isActive: activeTabID == tab.id && appState.activeProjectID == project.id, onRename: { newName in tab.customTitle = newName.isEmpty ? nil : newName appState.saveWorkspaces() @@ -475,7 +474,6 @@ private struct SidebarProjectRow: View { private struct SidebarTabRow: View { let tab: TerminalTab let index: Int - let isActive: Bool let onRename: (String) -> Void @Environment(AppState.self) private var appState @@ -521,7 +519,7 @@ private struct SidebarTabRow: View { titleContent } icon: { if showTabStatusIndicator { - TabStatusGlyph(state: displayState, symbol: tabIconSymbol, index: index, agent: agentIcon) + TabStatusGlyph(state: tab.executionState, symbol: tabIconSymbol, index: index, agent: agentIcon) } else if let agentIcon { // "None" suppresses the user's icon, not the agent // logo — a live status signal, like the else branch. @@ -535,7 +533,7 @@ private struct SidebarTabRow: View { titleContent } icon: { if showTabStatusIndicator { - TabStatusGlyph(state: displayState, symbol: tabIconSymbol, index: index, agent: agentIcon) + TabStatusGlyph(state: tab.executionState, symbol: tabIconSymbol, index: index, agent: agentIcon) } else { SidebarRowIcon(symbol: tabIconSymbol, index: index, agent: agentIcon) .foregroundStyle(.secondary) @@ -565,17 +563,6 @@ private struct SidebarTabRow: View { appState.restoreFocusToActivePane() } - private var displayState: TerminalExecutionState { - if tab.executionState == .running { return .running } - // The tab the user is already looking at never needs an attention - // indicator; a background tab's `done` checkmark is shown until it's - // acknowledged. Visiting the tab clears all of its panes via the poll's - // `acknowledgeFinishedCommandIfActive` (which acknowledges the whole - // active tab, not just the focused pane, so the persisted state matches - // what's displayed). - return isActive ? .idle : tab.executionState - } - private func cancelRename() { isRenaming = false appState.restoreFocusToActivePane() diff --git a/Macterm/Views/Terminal/GhosttyTerminalNSView.swift b/Macterm/Views/Terminal/GhosttyTerminalNSView.swift index 2720fae8..99f34ae3 100644 --- a/Macterm/Views/Terminal/GhosttyTerminalNSView.swift +++ b/Macterm/Views/Terminal/GhosttyTerminalNSView.swift @@ -117,10 +117,6 @@ final class GhosttyTerminalNSView: NSView { } } - func surfaceDidRender() { - onTerminalRender?() - } - func surfaceDidUpdateScrollbar(total: UInt64, offset: UInt64, len: UInt64) { let snapshot = ScrollbarSnapshot(total: total, offset: offset, len: len) if let lastScrollbarSnapshot, total > lastScrollbarSnapshot.total { @@ -130,8 +126,33 @@ final class GhosttyTerminalNSView: NSView { onScrollbarUpdate?(total, offset, len) } + /// Deliver a throttled (~500ms) output heartbeat from the pty IO path + /// (`GHOSTTY_ACTION_OUTPUT_ACTIVITY`, wired separately in + /// `GhosttyCallbacks`). Unlike `surfaceDidUpdateScrollbar`, this fires + /// regardless of occlusion — the renderer doesn't need to be running — + /// so it also reaches background/occluded panes. Growth-vs-keepalive + /// decisions belong to `TerminalExecutionTracker.markOutputActivity`, not + /// here; this method forwards only the total row count that decision needs. + func surfaceDidOutputActivity(total: UInt64, offset _: UInt64, len _: UInt64) { + onOutputActivity?(total) + } + + /// Record the actual payload resolved for a libghostty clipboard request. + /// Unlike key-code inference, this distinguishes real content from an + /// empty/whitespace clipboard or a remapped Command-V binding. + func surfaceDidPasteText(_ text: String) { + recordCommandInput(text) + if TerminalCommandSubmission.textContainsNewline(text), + TerminalCommandSubmission.textContainsContent(text) + { + preserveProgrammaticCommandInput(text) + } + } + var onFocus: (() -> Void)? var onInteraction: (() -> Void)? + /// Bool is best-effort evidence that the submitted prompt contained text. + var onCommandSubmitted: ((Bool) -> Void)? var onProcessExit: (() -> Void)? var onSplitRequest: ((SplitDirection, SplitPosition) -> Void)? var onZoomRequest: (() -> Void)? @@ -145,12 +166,15 @@ final class GhosttyTerminalNSView: NSView { var onProgressStarted: (() -> Void)? var onProgressFinished: (() -> Void)? var onTerminalActivity: (() -> Void)? - var onTerminalRender: (() -> Void)? /// libghostty pushes scrollback geometry (all values in rows) whenever the /// viewport, scrollback size, or visible row count changes. /// `(total, offset, len)`: total rows including scrollback, the first /// visible row (0 = top of history), and the visible row count. var onScrollbarUpdate: ((UInt64, UInt64, UInt64) -> Void)? + /// Fires on each throttled `OUTPUT_ACTIVITY` heartbeat with the surface's + /// current total row count. Occlusion-independent — see + /// `surfaceDidOutputActivity`. + var onOutputActivity: ((UInt64) -> Void)? /// Gives the hosting `SurfaceScrollView` first chance to handle scrollback /// wheel/trackpad events with its iTerm-style line accumulator. It declines /// when there's no scrollback to move through (so alternate-screen apps @@ -161,6 +185,8 @@ final class GhosttyTerminalNSView: NSView { var currentPwd: String? private var lastScrollbarSnapshot: ScrollbarSnapshot? + private var commandSubmissionEvidence = TerminalCommandSubmission.Evidence() + private var commandSubmissionEvidenceReset: DispatchWorkItem? /// The most recent `GHOSTTY_ACTION_SCROLLBAR` values (`total`/`offset`/`len` /// rows), or nil before the first scrollbar update. Read-only introspection @@ -397,6 +423,7 @@ final class GhosttyTerminalNSView: NSView { func destroySurface() { isDestroyed = true + clearCommandSubmissionEvidence() if let surface { ghostty_surface_free(surface) } surface = nil configCStrings.forEach { free($0) } @@ -623,6 +650,38 @@ final class GhosttyTerminalNSView: NSView { // MARK: - Keyboard + private func recordCommandInput(_ text: String) { + commandSubmissionEvidenceReset?.cancel() + commandSubmissionEvidenceReset = nil + commandSubmissionEvidence.recordText(text) + } + + private func consumeCommandSubmissionEvidence() -> Bool { + commandSubmissionEvidenceReset?.cancel() + commandSubmissionEvidenceReset = nil + return commandSubmissionEvidence.consume() + } + + private func clearCommandSubmissionEvidence() { + commandSubmissionEvidenceReset?.cancel() + commandSubmissionEvidenceReset = nil + commandSubmissionEvidence.clear() + } + + /// `sendText` may contain a newline that executes directly, or it may be + /// bracketed-pasted into a raw TUI and need a following encoded Return. + /// Preserve its content briefly for the latter without leaving stale + /// evidence behind indefinitely in the former. + private func preserveProgrammaticCommandInput(_ text: String) { + commandSubmissionEvidence.recordText(text) + let reset = DispatchWorkItem { [weak self] in + self?.commandSubmissionEvidence.clear() + self?.commandSubmissionEvidenceReset = nil + } + commandSubmissionEvidenceReset = reset + DispatchQueue.main.asyncAfter(deadline: .now() + 2, execute: reset) + } + override func keyDown(with event: NSEvent) { onInteraction?() guard let surface else { super.keyDown(with: event) @@ -630,6 +689,13 @@ final class GhosttyTerminalNSView: NSView { } let action: ghostty_input_action_e = event.isARepeat ? GHOSTTY_ACTION_REPEAT : GHOSTTY_ACTION_PRESS let flags = event.modifierFlags.intersection(.deviceIndependentFlagsMask) + if TerminalCommandSubmission.clearsInputEvidence( + keyCode: event.keyCode, + hasControl: flags.contains(.control), + hasCommand: flags.contains(.command) + ) { + clearCommandSubmissionEvidence() + } if flags.contains(.control), !flags.contains(.command), !flags.contains(.option), !hasMarkedText() { if isAppShortcut(event) { return } @@ -693,6 +759,7 @@ final class GhosttyTerminalNSView: NSView { // here even though this specific text is finalized). Without this, // Korean / Japanese / Chinese input drops every committed character. // The text itself carries no composing flag since it's already final. + var forwarded = false if !keyTextAccumulator.isEmpty { var commitKE = ke commitKE.composing = false @@ -700,6 +767,10 @@ final class GhosttyTerminalNSView: NSView { text.withCString { commitKE.text = $0 _ = ghostty_surface_key(surface, commitKE) } + if TerminalCommandSubmission.shouldRecordLiteralText(hasOption: flags.contains(.option)) { + recordCommandInput(text) + } + forwarded = true } } else if !hasMarkedText() { let text = filterSpecial(event.characters ?? "") @@ -707,11 +778,27 @@ final class GhosttyTerminalNSView: NSView { text.withCString { ke.text = $0 _ = ghostty_surface_key(surface, ke) } + if TerminalCommandSubmission.shouldRecordLiteralText(hasOption: flags.contains(.option)) { + recordCommandInput(text) + } } else { ke.consumed_mods = GHOSTTY_MODS_NONE ke.text = nil _ = ghostty_surface_key(surface, ke) } + forwarded = true + } + + let userModifiers: NSEvent.ModifierFlags = [.shift, .control, .option, .command] + if forwarded, + TerminalCommandSubmission.isReturn( + keyCode: event.keyCode, + isRepeat: event.isARepeat, + hasMarkedText: hadMarkedText || hasMarkedText(), + hasUserModifiers: !flags.isDisjoint(with: userModifiers) + ) + { + onCommandSubmitted?(consumeCommandSubmissionEvidence()) } } @@ -748,6 +835,13 @@ final class GhosttyTerminalNSView: NSView { guard event.type == .keyDown, let surface else { return false } let flags = event.modifierFlags.intersection(.deviceIndependentFlagsMask) guard flags.contains(.command) || flags.contains(.control) || flags.contains(.option) else { return false } + if TerminalCommandSubmission.clearsInputEvidence( + keyCode: event.keyCode, + hasControl: flags.contains(.control), + hasCommand: flags.contains(.command) + ) { + clearCommandSubmissionEvidence() + } var ke = buildKeyEvent(from: event, action: event.isARepeat ? GHOSTTY_ACTION_REPEAT : GHOSTTY_ACTION_PRESS) ke.text = nil if ghostty_surface_key_is_binding(surface, ke, nil) { @@ -1161,12 +1255,18 @@ extension GhosttyTerminalNSView { // Same liveness signal a keystroke sends (execution tracking + poll // resume), so an injected command updates the tab title promptly. onInteraction?() + recordCommandInput(text) text.withCString { ptr in var ke = ghostty_input_key_s() ke.action = GHOSTTY_ACTION_PRESS ke.text = ptr _ = ghostty_surface_key(surface, ke) } + if TerminalCommandSubmission.textContainsNewline(text) { + let hasContent = consumeCommandSubmissionEvidence() + onCommandSubmitted?(hasContent) + if hasContent { preserveProgrammaticCommandInput(text) } + } return true } @@ -1189,6 +1289,13 @@ extension GhosttyTerminalNSView { func sendKey(keyCode: UInt16, mods flags: NSEvent.ModifierFlags) -> Bool { guard let surface else { return false } onInteraction?() + if TerminalCommandSubmission.clearsInputEvidence( + keyCode: keyCode, + hasControl: flags.contains(.control), + hasCommand: flags.contains(.command) + ) { + clearCommandSubmissionEvidence() + } var m = GHOSTTY_MODS_NONE.rawValue if flags.contains(.shift) { m |= GHOSTTY_MODS_SHIFT.rawValue } if flags.contains(.control) { m |= GHOSTTY_MODS_CTRL.rawValue } @@ -1210,6 +1317,15 @@ extension GhosttyTerminalNSView { ke.unshifted_codepoint = codepoint _ = ghostty_surface_key(surface, ke) } + let userModifiers: NSEvent.ModifierFlags = [.shift, .control, .option, .command] + if TerminalCommandSubmission.isReturn( + keyCode: keyCode, + isRepeat: false, + hasMarkedText: false, + hasUserModifiers: !flags.isDisjoint(with: userModifiers) + ) { + onCommandSubmitted?(consumeCommandSubmissionEvidence()) + } return true } @@ -1299,6 +1415,7 @@ extension GhosttyTerminalNSView: @preconcurrency NSTextInputClient { ke.text = ptr _ = ghostty_surface_key(surface, ke) } + recordCommandInput(text) } } diff --git a/Macterm/Views/Terminal/TerminalCommandSubmission.swift b/Macterm/Views/Terminal/TerminalCommandSubmission.swift new file mode 100644 index 00000000..dccbd322 --- /dev/null +++ b/Macterm/Views/Terminal/TerminalCommandSubmission.swift @@ -0,0 +1,66 @@ +import Foundation + +enum TerminalCommandSubmission { + private static let returnKeyCodes: Set = [36, 76] + + /// Best-effort evidence that the next Return submits actual prompt text. + /// Terminal protocols do not expose a TUI's editor buffer, so the view + /// records committed text it forwards and consumes that evidence on Return. + /// This rejects a genuinely blank Return without naming any specific TUI. + struct Evidence { + private var hasContent = false + + mutating func recordText(_ text: String) { + if TerminalCommandSubmission.textContainsContent(text) { + hasContent = true + } + } + + mutating func consume() -> Bool { + defer { hasContent = false } + return hasContent + } + + mutating func clear() { + hasContent = false + } + } + + static func isReturn( + keyCode: UInt16, + isRepeat: Bool, + hasMarkedText: Bool, + hasUserModifiers: Bool + ) -> Bool { + returnKeyCodes.contains(keyCode) && !isRepeat && !hasMarkedText && !hasUserModifiers + } + + static func textContainsNewline(_ text: String) -> Bool { + text.contains("\n") || text.contains("\r") + } + + static func textContainsContent(_ text: String) -> Bool { + text.unicodeScalars.contains { scalar in + !CharacterSet.whitespacesAndNewlines.contains(scalar) + && !CharacterSet.controlCharacters.contains(scalar) + } + } + + static func clearsInputEvidence( + keyCode: UInt16, + hasControl: Bool, + hasCommand: Bool + ) -> Bool { + if keyCode == 53 || keyCode == 51 || keyCode == 117 { return true } // Escape / delete + if hasControl, [4, 8, 13, 32, 40].contains(keyCode) { return true } // H/C/W/U/K + if hasCommand, [0, 7].contains(keyCode) { return true } // Select-all / cut + return false + } + + static func shouldRecordLiteralText(hasOption: Bool) -> Bool { + // With macos-option-as-alt, interpretKeyEvents yields a printable base + // character even though Ghostty sends it as Meta navigation. Prefer a + // false negative over calling that navigation committed prompt text. + !hasOption + } +} diff --git a/Macterm/Views/TerminalPane.swift b/Macterm/Views/TerminalPane.swift index 0415a7cf..568e13b0 100644 --- a/Macterm/Views/TerminalPane.swift +++ b/Macterm/Views/TerminalPane.swift @@ -131,6 +131,9 @@ private struct TerminalSurface: NSViewRepresentable { view.onInteraction = { [weak pane] in pane?.recordUserInteraction() } + view.onCommandSubmitted = { [weak pane] hasContent in + pane?.recordCommandSubmission(hasContent: hasContent) + } view.onProcessExit = onProcessExit view.onSplitRequest = onSplitRequest view.onZoomRequest = onZoomRequest @@ -210,17 +213,18 @@ private struct TerminalSurface: NSViewRepresentable { pane.refreshForegroundProcess() pane.markTerminalActivity() } - view.onTerminalRender = { [weak pane] in + view.onOutputActivity = { [weak pane] total in guard let pane, Preferences.shared.showTabStatusIndicator else { return } - // Renders also happen for prompt redraws and input echo. Use them to - // keep an already-detected command active (including in-place - // spinners), but don't let a render alone start the status spinner. - if pane.executionState != .running { - pane.refreshForegroundProcess() - } - if pane.executionState == .running { - pane.markTerminalActivity() - } + // Output heartbeats fire from the pty IO path regardless of + // occlusion, so they also reach background tabs — unlike the + // scrollbar-growth path above, which only fires while the + // surface is actually rendered. Always refresh foreground/raw + // state first: a running canonical command can switch to a raw TUI + // while normal polling is paused behind a fully occluded window. + // The heartbeat is throttled to ~2 Hz, and the tracker preserves + // foreground/progress authority until that raw transition occurs. + pane.refreshForegroundProcess() + pane.markOutputActivity(totalRows: total) } view.onCommandFinished = { [weak pane, weak view] exitCode, durationNs in guard let pane else { return } diff --git a/MactermTests/App/AppStateTests.swift b/MactermTests/App/AppStateTests.swift index 97198fde..70b052fa 100644 --- a/MactermTests/App/AppStateTests.swift +++ b/MactermTests/App/AppStateTests.swift @@ -1063,6 +1063,43 @@ struct AppStateTests { #expect(pane.executionState == .done) } + @Test + func occluded_pane_without_heartbeat_flag_keeps_pre_heartbeat_behavior() { + // A pane whose GhosttyKit build never delivered an OUTPUT_ACTIVITY + // heartbeat has no proof its silence is meaningful while occluded — + // must still skip settling and grant a fresh window on de-occlusion, + // exactly like `deoccluded_pane_gets_fresh_quiet_window_before_settling`. + let state = makeAppState() + let pane = quietRunningPane() + #expect(!pane.hasOcclusionIndependentHeartbeat) + + state.paneIsOccluded = { _ in true } + state.settleIfVisible(pane) + #expect(pane.executionState == .running) + + state.paneIsOccluded = { _ in false } + state.settleIfVisible(pane) + #expect(pane.executionState == .running) + pane.settleTerminalActivityIfQuiet(now: Date().addingTimeInterval(4)) + #expect(pane.executionState == .done) + } + + @Test + func occluded_pane_with_heartbeat_flag_settles_normally_while_occluded() { + // Once a pane has proven it receives occlusion-independent heartbeats, + // its silence is meaningful even while occluded: settle it exactly + // like a visible pane — no skip, no fresh-window grant needed. + let state = makeAppState() + let pane = quietRunningPane() + pane.markOutputActivity(totalRows: 1, now: Date().addingTimeInterval(-10)) + #expect(pane.hasOcclusionIndependentHeartbeat) + #expect(pane.executionState == .running) + + state.paneIsOccluded = { _ in true } + state.settleIfVisible(pane) + #expect(pane.executionState == .done) + } + // MARK: - zmx session lifecycle on close paths /// A ZmxClient that records every killed session name. Remote kills are diff --git a/MactermTests/Control/ControlHandlerTests.swift b/MactermTests/Control/ControlHandlerTests.swift index 258c3a6d..a8df99d1 100644 --- a/MactermTests/Control/ControlHandlerTests.swift +++ b/MactermTests/Control/ControlHandlerTests.swift @@ -198,6 +198,23 @@ struct ControlHandlerTests { #expect(panes?.last?.focused == true) #expect(panes?.allSatisfy { $0.session.hasPrefix("macterm-") } == true) #expect(panes?.allSatisfy { $0.cwd == project.path } == true) + #expect(panes?.allSatisfy { $0.state == "idle" } == true) + } + + @Test + func pane_list_reports_running_and_done_states() async throws { + let (handler, appState, projectStore) = makeHandler() + let project = seedProject(appState, projectStore) + appState.isAppActive = { false } + let pane = try #require(appState.workspaces[project.id]?.activeTab?.splitRoot.allPanes().first) + + pane.executionState = .running + var response = await handler.handle(request("pane.list")) + #expect(response.data?.panes?.first?.state == "running") + + pane.executionState = .done + response = await handler.handle(request("pane.list")) + #expect(response.data?.panes?.first?.state == "done") } @Test diff --git a/MactermTests/Control/ControlProtocolTests.swift b/MactermTests/Control/ControlProtocolTests.swift index 993c26cc..e4efdf5b 100644 --- a/MactermTests/Control/ControlProtocolTests.swift +++ b/MactermTests/Control/ControlProtocolTests.swift @@ -75,6 +75,30 @@ struct ControlProtocolTests { #expect(decoded.command == "status") } + @Test + func pane_response_without_execution_state_remains_decodable() throws { + let json = #""" + { + "v": 1, + "id": "old", + "ok": true, + "data": { + "panes": [{ + "index": 1, + "id": "p", + "session": "macterm-demo", + "tabIndex": 1, + "tabID": "t", + "title": "zsh", + "focused": true + }] + } + } + """# + let decoded = try ControlProtocol.decodeResponse(Data((json + "\n").utf8)) + #expect(decoded.data?.panes?.first?.state == nil) + } + // MARK: - New verbs (#165/#166/#167) @Test diff --git a/MactermTests/Model/PaneTests.swift b/MactermTests/Model/PaneTests.swift index 126d0b2e..7078b386 100644 --- a/MactermTests/Model/PaneTests.swift +++ b/MactermTests/Model/PaneTests.swift @@ -241,13 +241,23 @@ struct PaneTests { } @Test - func rawForegroundProcess_settlesExistingForegroundOnlyRun() { + func rawForegroundProcess_demotesExistingForegroundOnlyRun_thenSettlesWhenQuiet() { + // A TUI that spawns canonical, prints, then goes raw (claude, pi) is + // *starting* its work, not finishing — the raw switch demotes the + // foreground-owned run to the activity source instead of completing + // it outright, so it stays `.running` until output actually goes + // quiet rather than locking the pane `.done` for the whole session. let p = Pane(projectPath: "/", projectID: UUID()) p.recordUserInteraction() p.applyForegroundRefresh(name: "node", foregroundPID: 42) #expect(p.executionState == .running) p.applyForegroundRefresh(name: "node", foregroundPID: 42, terminalInputIsRaw: true) + #expect(p.executionState == .running) + + // No further output: the demoted run quiet-settles like any other + // activity-sourced run. + p.settleTerminalActivityIfQuiet(now: Date().addingTimeInterval(5), quietInterval: 3) #expect(p.executionState == .done) } @@ -287,16 +297,87 @@ struct PaneTests { } @Test - func foregroundProcessWithOutput_settlesAfterQuiet_withoutRestartingSameProcess() { + func commandSubmissionStartsOnSecondInPlaceOutputHeartbeatForAgent() { + let p = Pane(projectPath: "/", projectID: UUID()) + p.foregroundProcessName = "pi" + let submittedAt = Date(timeIntervalSince1970: 100) + p.recordCommandSubmission(hasContent: true, at: submittedAt) + p.markOutputActivity(totalRows: 10, now: submittedAt.addingTimeInterval(0.5)) + #expect(p.executionState == .idle) + p.markOutputActivity(totalRows: 10, now: submittedAt.addingTimeInterval(1)) + #expect(p.executionState == .running) + } + + @Test + func commandSubmissionDoesNotArmInPlaceOutputForOrdinaryRawProgram() { + let p = Pane(projectPath: "/", projectID: UUID()) + p.foregroundProcessName = "nvim" + let submittedAt = Date(timeIntervalSince1970: 100) + p.recordCommandSubmission(hasContent: true, at: submittedAt) + p.markOutputActivity(totalRows: 10, now: submittedAt.addingTimeInterval(0.5)) + p.markOutputActivity(totalRows: 10, now: submittedAt.addingTimeInterval(1)) + #expect(p.executionState == .idle) + } + + @Test + func blankSubmissionSuppressesImmediateGrowthAndScrollbarActivity() { + let p = Pane(projectPath: "/", projectID: UUID()) + let submittedAt = Date(timeIntervalSince1970: 100) + p.markOutputActivity(totalRows: 10, now: submittedAt.addingTimeInterval(-1)) + p.recordCommandSubmission(hasContent: false, at: submittedAt) + + p.markOutputActivity(totalRows: 20, now: submittedAt.addingTimeInterval(0.25)) + p.markTerminalActivity(at: submittedAt.addingTimeInterval(0.5)) + #expect(p.executionState == .idle) + } + + @Test + func markOutputActivity_setsOcclusionIndependentHeartbeatFlag() { + // The flag records that this surface's build actually delivers + // OUTPUT_ACTIVITY heartbeats, which `AppState.settleIfVisible` uses + // to decide whether an occluded pane's silence is trustworthy. + let p = Pane(projectPath: "/", projectID: UUID()) + #expect(!p.hasOcclusionIndependentHeartbeat) + p.markOutputActivity(totalRows: 10) + #expect(p.hasOcclusionIndependentHeartbeat) + } + + @Test + func outputActivitySchedulesQuietPollWake() async { + let p = Pane( + projectPath: "/", + projectID: UUID(), + activityQuietPollDelay: .milliseconds(20) + ) + p.recordUserInteraction() + p.markOutputActivity(totalRows: 10) + + await confirmation("quiet output wakes the paused poll") { confirm in + let token = NotificationCenter.default.addObserver( + forName: .terminalQuietSettleDeadline, + object: p, + queue: .main + ) { _ in + confirm() + } + defer { NotificationCenter.default.removeObserver(token) } + + p.markOutputActivity(totalRows: 20) + try? await Task.sleep(for: .milliseconds(100)) + } + } + + @Test + func foregroundProcessWithOutput_remainsRunningUntilItReturnsToShell() { let p = Pane(projectPath: "/", projectID: UUID()) let start = Date(timeIntervalSince1970: 100) p.recordUserInteraction() p.applyForegroundRefresh(name: "node", foregroundPID: 42) p.markTerminalActivity(at: start) + p.settleTerminalActivityIfQuiet(now: start.addingTimeInterval(30), quietInterval: 3) #expect(p.executionState == .running) - p.settleTerminalActivityIfQuiet(now: start.addingTimeInterval(3), quietInterval: 3) - #expect(p.executionState == .done) - p.applyForegroundRefresh(name: "node", foregroundPID: 42) + + p.applyForegroundRefresh(name: shellName(), foregroundPID: 43, foregroundIsShell: true) #expect(p.executionState == .done) } diff --git a/MactermTests/Model/TerminalExecutionTrackerTests.swift b/MactermTests/Model/TerminalExecutionTrackerTests.swift index 4b059458..7c4541a9 100644 --- a/MactermTests/Model/TerminalExecutionTrackerTests.swift +++ b/MactermTests/Model/TerminalExecutionTrackerTests.swift @@ -2,22 +2,12 @@ import Foundation @testable import Macterm import Testing -/// Direct unit tests for `TerminalExecutionTracker`, the state machine behind a -/// pane's tab status spinner. -/// -/// These pin the core invariant surfaced in the activity-detection fix: a -/// render/output heartbeat can keep an already-running command active (so -/// in-place spinners that repaint with carriage returns stay alive), but it can -/// never start or resurrect the spinner on its own. The "keep-alive only" rule -/// is enforced at the call site (`onTerminalRender` only calls the heartbeat -/// while already `.running`), and these tests lock the matching guarantee into -/// the tracker itself — otherwise a future refactor could silently reintroduce -/// the "prompt redraw keeps spinning" bug with nothing to catch it. +/// Pins the source-precedence rules behind the tab activity indicator. +/// Foreground/progress are authoritative; output may start or sustain only +/// under the guarded interaction and submission rules exercised below. struct TerminalExecutionTrackerTests { @Test func markTerminalActivity_fromIdleWithoutInteraction_staysIdle() { - // No prior user interaction: a fresh/restored shell's startup output - // must not register as activity. var tracker = TerminalExecutionTracker() let state = tracker.markTerminalActivity(at: Date(timeIntervalSince1970: 1), currentState: .idle) #expect(state == .idle) @@ -25,10 +15,6 @@ struct TerminalExecutionTrackerTests { @Test func markTerminalActivity_fromIdleWithInteraction_startsRun() { - // Output activity (scrollback) after interaction is a genuine signal - // that something is running, so — unlike a render heartbeat — it *may* - // start the spinner from idle. Pinned here so the "render can't start" - // guard isn't over-corrected into "no activity can ever start". var tracker = TerminalExecutionTracker() tracker.recordUserInteraction() let state = tracker.markTerminalActivity(at: Date(timeIntervalSince1970: 1), currentState: .idle) @@ -37,58 +23,339 @@ struct TerminalExecutionTrackerTests { @Test func markTerminalActivity_fromDone_doesNotReturnToRunning() { - // A finished command (`.done`, checkmark showing) must not be flipped - // back to running by a render/output heartbeat — e.g. a background job - // printing while the foreground command is already settled. var tracker = TerminalExecutionTracker() tracker.recordUserInteraction() var state = tracker.markTerminalActivity(at: Date(timeIntervalSince1970: 100), currentState: .idle) state = tracker.settleIfQuiet(now: Date(timeIntervalSince1970: 103), quietInterval: 3, currentState: state) - #expect(state == .done) - state = tracker.markTerminalActivity(at: Date(timeIntervalSince1970: 110), currentState: state) #expect(state == .done) } @Test func markTerminalActivity_fromRunning_keepsAliveAndRefreshesTimestamp() { - // While running, each heartbeat refreshes the activity timestamp so the - // quiet-settle window restarts. This is "a render can keep a spinner - // alive" — the fix for in-place spinners that repaint the same line. var tracker = TerminalExecutionTracker() tracker.recordUserInteraction() let start = Date(timeIntervalSince1970: 100) var state = tracker.markTerminalActivity(at: start, currentState: .idle) + state = tracker.markTerminalActivity(at: start.addingTimeInterval(2), currentState: state) + state = tracker.settleIfQuiet(now: start.addingTimeInterval(3), quietInterval: 3, currentState: state) #expect(state == .running) + state = tracker.settleIfQuiet(now: start.addingTimeInterval(5), quietInterval: 3, currentState: state) + #expect(state == .done) + } + + @Test + func markOutputActivity_growthStartsOnlyAfterInteraction() { + var tracker = TerminalExecutionTracker() + var state = tracker.markOutputActivity(totalRows: 10, at: Date(timeIntervalSince1970: 1), currentState: .idle) + state = tracker.markOutputActivity(totalRows: 20, at: Date(timeIntervalSince1970: 2), currentState: state) + #expect(state == .idle) + + tracker.recordUserInteraction() + state = tracker.markOutputActivity(totalRows: 30, at: Date(timeIntervalSince1970: 3), currentState: state) + #expect(state == .running) + } + + @Test + func markOutputActivity_nonGrowthOnlySustainsActivityRun() { + var tracker = TerminalExecutionTracker() + tracker.recordUserInteraction() + let start = Date(timeIntervalSince1970: 100) + var state = tracker.markOutputActivity(totalRows: 10, at: start, currentState: .idle) + state = tracker.markOutputActivity(totalRows: 10, at: start.addingTimeInterval(1), currentState: state) + #expect(state == .idle) - // A heartbeat at start+2 restarts the window; settling at start+3 (only - // 1s after the last heartbeat) must still be running. state = tracker.markTerminalActivity(at: start.addingTimeInterval(2), currentState: state) + state = tracker.markOutputActivity(totalRows: 10, at: start.addingTimeInterval(4), currentState: state) + state = tracker.settleIfQuiet(now: start.addingTimeInterval(6), quietInterval: 3, currentState: state) #expect(state == .running) - state = tracker.settleIfQuiet(now: start.addingTimeInterval(3), quietInterval: 3, currentState: state) + state = tracker.settleIfQuiet(now: start.addingTimeInterval(7), quietInterval: 3, currentState: state) + #expect(state == .done) + } + + @Test + func markOutputActivity_growthDoesNotDemoteCanonicalForegroundRun() { + var tracker = TerminalExecutionTracker() + tracker.recordUserInteraction() + var state = tracker.refreshForeground( + name: "sleep", pid: 42, foregroundIsShell: false, terminalInputIsRaw: false, currentState: .idle + ) #expect(state == .running) - // After the full quiet interval elapses past the last heartbeat, it settles. - state = tracker.settleIfQuiet(now: start.addingTimeInterval(5), quietInterval: 3, currentState: state) + state = tracker.markOutputActivity(totalRows: 10, at: Date(timeIntervalSince1970: 1), currentState: state) + state = tracker.markOutputActivity(totalRows: 20, at: Date(timeIntervalSince1970: 2), currentState: state) + state = tracker.settleIfQuiet(now: Date(timeIntervalSince1970: 30), quietInterval: 3, currentState: state) + #expect(state == .running) + + state = tracker.refreshForeground( + name: "zsh", pid: 43, foregroundIsShell: true, terminalInputIsRaw: false, currentState: state + ) #expect(state == .done) } @Test - func activitySourcedRun_settlesAfterQuietInterval() { - // An activity-sourced run is not held forever: once output goes quiet - // for the interval it decays to `.done` (foreground- and progress-sourced - // runs are not subject to this timer). + func markOutputActivity_doesNotResurrectDoneOrReplaceProgress() { + var doneTracker = TerminalExecutionTracker() + doneTracker.recordUserInteraction() + var doneState = doneTracker.markTerminalActivity(at: Date(timeIntervalSince1970: 1), currentState: .idle) + doneState = doneTracker.settleIfQuiet( + now: Date(timeIntervalSince1970: 4), quietInterval: 3, currentState: doneState + ) + _ = doneTracker.markOutputActivity(totalRows: 10, at: Date(timeIntervalSince1970: 5), currentState: doneState) + doneState = doneTracker.markOutputActivity( + totalRows: 20, at: Date(timeIntervalSince1970: 6), currentState: doneState + ) + #expect(doneState == .done) + + var progressTracker = TerminalExecutionTracker() + progressTracker.recordUserInteraction() + var progressState = progressTracker.markProgressStarted(currentState: .idle) + _ = progressTracker.markOutputActivity( + totalRows: 10, at: Date(timeIntervalSince1970: 1), currentState: progressState + ) + progressState = progressTracker.markOutputActivity( + totalRows: 20, at: Date(timeIntervalSince1970: 2), currentState: progressState + ) + progressState = progressTracker.settleIfQuiet( + now: Date(timeIntervalSince1970: 100), quietInterval: 3, currentState: progressState + ) + #expect(progressState == .running) + } + + @Test + func rawForegroundCannotStartButCanonicalRunDemotesAndSettles() { var tracker = TerminalExecutionTracker() tracker.recordUserInteraction() - let start = Date(timeIntervalSince1970: 100) - var state = tracker.markTerminalActivity(at: start, currentState: .idle) + var state = tracker.refreshForeground( + name: "pi", pid: 42, foregroundIsShell: false, terminalInputIsRaw: true, currentState: .idle + ) + #expect(state == .idle) + + state = tracker.refreshForeground( + name: "pi", pid: 42, foregroundIsShell: false, terminalInputIsRaw: false, currentState: state + ) + #expect(state == .idle) + state = tracker.refreshForeground( + name: "pi", pid: 43, foregroundIsShell: false, terminalInputIsRaw: false, currentState: state + ) #expect(state == .running) - // Just under the quiet interval: still running. - state = tracker.settleIfQuiet(now: start.addingTimeInterval(2), quietInterval: 3, currentState: state) + let rawAt = Date(timeIntervalSince1970: 100) + state = tracker.refreshForeground( + name: "pi", + pid: 43, + foregroundIsShell: false, + terminalInputIsRaw: true, + at: rawAt, + currentState: state + ) + state = tracker.markOutputActivity(totalRows: 10, at: rawAt.addingTimeInterval(2), currentState: state) + state = tracker.settleIfQuiet(now: rawAt.addingTimeInterval(4), quietInterval: 3, currentState: state) #expect(state == .running) - // At the quiet interval: settles to done. - state = tracker.settleIfQuiet(now: start.addingTimeInterval(3), quietInterval: 3, currentState: state) + state = tracker.settleIfQuiet(now: rawAt.addingTimeInterval(5), quietInterval: 3, currentState: state) + #expect(state == .done) + } + + @Test + func samePIDReturningToCanonicalRestoresForegroundAuthority() { + var tracker = TerminalExecutionTracker() + tracker.recordUserInteraction() + var state = tracker.refreshForeground( + name: "agent", pid: 42, foregroundIsShell: false, terminalInputIsRaw: false, currentState: .idle + ) + state = tracker.refreshForeground( + name: "agent", + pid: 42, + foregroundIsShell: false, + terminalInputIsRaw: true, + at: Date(timeIntervalSince1970: 100), + currentState: state + ) + state = tracker.refreshForeground( + name: "agent", pid: 42, foregroundIsShell: false, terminalInputIsRaw: false, currentState: state + ) + state = tracker.settleIfQuiet( + now: Date(timeIntervalSince1970: 1000), quietInterval: 3, currentState: state + ) + #expect(state == .running) + + state = tracker.refreshForeground( + name: "zsh", pid: 43, foregroundIsShell: true, terminalInputIsRaw: false, currentState: state + ) + #expect(state == .done) + } + + @Test + func settledSamePIDRawProcessDoesNotRestartWhenCanonical() { + var tracker = TerminalExecutionTracker() + tracker.recordUserInteraction() + var state = tracker.refreshForeground( + name: "agent", pid: 42, foregroundIsShell: false, terminalInputIsRaw: false, currentState: .idle + ) + let rawAt = Date(timeIntervalSince1970: 100) + state = tracker.refreshForeground( + name: "agent", + pid: 42, + foregroundIsShell: false, + terminalInputIsRaw: true, + at: rawAt, + currentState: state + ) + state = tracker.settleIfQuiet(now: rawAt.addingTimeInterval(3), quietInterval: 3, currentState: state) + #expect(state == .done) + + state = tracker.refreshForeground( + name: "agent", pid: 42, foregroundIsShell: false, terminalInputIsRaw: false, currentState: state + ) #expect(state == .done) } + + @Test + func submittedCommand_requiresTwoNonGrowingHeartbeatsToStartAndThenQuietSettles() { + var tracker = TerminalExecutionTracker() + let submittedAt = Date(timeIntervalSince1970: 100) + tracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: true, hasContent: true) + + var state = tracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(0.5), currentState: .idle + ) + #expect(state == .idle) + state = tracker.markOutputActivity(totalRows: 20, at: submittedAt.addingTimeInterval(1), currentState: state) + #expect(state == .running) + state = tracker.markOutputActivity(totalRows: 20, at: submittedAt.addingTimeInterval(2), currentState: state) + state = tracker.settleIfQuiet(now: submittedAt.addingTimeInterval(4.9), quietInterval: 3, currentState: state) + #expect(state == .running) + state = tracker.settleIfQuiet(now: submittedAt.addingTimeInterval(5), quietInterval: 3, currentState: state) + #expect(state == .done) + } + + @Test + func submittedCommand_candidateExpiresAtTwoSeconds() { + var tracker = TerminalExecutionTracker() + let submittedAt = Date(timeIntervalSince1970: 100) + tracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: true, hasContent: true) + var state = tracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(0.5), currentState: .idle + ) + state = tracker.markOutputActivity(totalRows: 20, at: submittedAt.addingTimeInterval(2), currentState: state) + #expect(state == .idle) + } + + @Test + func genericInteractionCancelsSubmissionCandidate() { + var tracker = TerminalExecutionTracker() + let submittedAt = Date(timeIntervalSince1970: 100) + tracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: true, hasContent: true) + var state = tracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(0.5), currentState: .idle + ) + tracker.recordUserInteraction() + state = tracker.markOutputActivity(totalRows: 20, at: submittedAt.addingTimeInterval(1), currentState: state) + #expect(state == .idle) + } + + @Test + func emptyCommandFinishPreservesBlankSuppressionForLaterGrowth() { + var tracker = TerminalExecutionTracker() + let submittedAt = Date(timeIntervalSince1970: 100) + var state = tracker.markOutputActivity( + totalRows: 10, at: submittedAt.addingTimeInterval(-1), currentState: .idle + ) + tracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: true, hasContent: false) + state = tracker.markCommandFinished(currentState: state) + state = tracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(0.7), currentState: state + ) + #expect(state == .idle) + } + + @Test + func blankSubmissionSuppressesImmediateGrowthThenExpires() { + var tracker = TerminalExecutionTracker() + let submittedAt = Date(timeIntervalSince1970: 100) + var state = tracker.markOutputActivity( + totalRows: 10, at: submittedAt.addingTimeInterval(-1), currentState: .idle + ) + tracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: true, hasContent: false) + + state = tracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(0.5), currentState: state + ) + state = tracker.markTerminalActivity(at: submittedAt.addingTimeInterval(1), currentState: state) + #expect(state == .idle) + + state = tracker.markOutputActivity( + totalRows: 30, at: submittedAt.addingTimeInterval(2), currentState: state + ) + #expect(state == .running) + } + + @Test + func progressTransitionCancelsSubmissionCandidate() { + var tracker = TerminalExecutionTracker() + let submittedAt = Date(timeIntervalSince1970: 100) + tracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: true, hasContent: true) + var state = tracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(0.2), currentState: .idle + ) + state = tracker.markProgressStarted(currentState: state) + state = tracker.markProgressFinished(currentState: state) + state = tracker.markOutputActivity(totalRows: 20, at: submittedAt.addingTimeInterval(0.7), currentState: state) + #expect(state == .done) + } + + @Test + func unchangedRawPiPreservesSubmissionButForegroundChangeCancelsIt() { + let submittedAt = Date(timeIntervalSince1970: 100) + var tracker = TerminalExecutionTracker() + tracker.recordUserInteraction() + var state = tracker.refreshForeground( + name: "pi", pid: 42, foregroundIsShell: false, terminalInputIsRaw: true, currentState: .idle + ) + tracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: true, hasContent: true) + state = tracker.refreshForeground( + name: "pi", pid: 42, foregroundIsShell: false, terminalInputIsRaw: true, currentState: state + ) + state = tracker.markOutputActivity(totalRows: 20, at: submittedAt.addingTimeInterval(0.5), currentState: state) + state = tracker.markOutputActivity(totalRows: 20, at: submittedAt.addingTimeInterval(1), currentState: state) + #expect(state == .running) + + var changedTracker = TerminalExecutionTracker() + changedTracker.recordUserInteraction() + _ = changedTracker.refreshForeground( + name: "pi", pid: 42, foregroundIsShell: false, terminalInputIsRaw: true, currentState: .idle + ) + changedTracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: true, hasContent: true) + _ = changedTracker.refreshForeground( + name: "node", pid: 43, foregroundIsShell: false, terminalInputIsRaw: true, currentState: .idle + ) + var changedState = changedTracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(0.5), currentState: .idle + ) + changedState = changedTracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(1), currentState: changedState + ) + #expect(changedState == .idle) + } + + @Test + func unrecognizedRawProgramCannotArmInPlaceOutputStart() { + var tracker = TerminalExecutionTracker() + let submittedAt = Date(timeIntervalSince1970: 100) + tracker.recordCommandSubmission(at: submittedAt, allowInPlaceOutputStart: false, hasContent: true) + var state = tracker.markOutputActivity( + totalRows: 20, at: submittedAt.addingTimeInterval(0.5), currentState: .idle + ) + state = tracker.markOutputActivity(totalRows: 20, at: submittedAt.addingTimeInterval(1), currentState: state) + #expect(state == .idle) + } + + @Test + func startupOutputCannotUseSubmissionPath() { + var tracker = TerminalExecutionTracker() + let start = Date(timeIntervalSince1970: 100) + var state = tracker.markOutputActivity(totalRows: 20, at: start, currentState: .idle) + state = tracker.markOutputActivity(totalRows: 20, at: start.addingTimeInterval(0.5), currentState: state) + #expect(state == .idle) + } } diff --git a/MactermTests/Views/Terminal/TerminalCommandSubmissionTests.swift b/MactermTests/Views/Terminal/TerminalCommandSubmissionTests.swift new file mode 100644 index 00000000..90c64aaf --- /dev/null +++ b/MactermTests/Views/Terminal/TerminalCommandSubmissionTests.swift @@ -0,0 +1,132 @@ +@testable import Macterm +import Testing + +struct TerminalCommandSubmissionTests { + @Test + func returnAndKeypadEnterAreSubmissions() { + #expect(TerminalCommandSubmission.isReturn( + keyCode: 36, isRepeat: false, hasMarkedText: false, hasUserModifiers: false + )) + #expect(TerminalCommandSubmission.isReturn( + keyCode: 76, isRepeat: false, hasMarkedText: false, hasUserModifiers: false + )) + } + + @Test(arguments: [ + (36, true, false, false), + (36, false, true, false), + (36, false, false, true), + (49, false, false, false), + ]) + func rejectsRepeatCompositionModifiersAndOtherKeys( + keyCode: Int, + isRepeat: Bool, + hasMarkedText: Bool, + hasUserModifiers: Bool + ) { + #expect(!TerminalCommandSubmission.isReturn( + keyCode: UInt16(keyCode), + isRepeat: isRepeat, + hasMarkedText: hasMarkedText, + hasUserModifiers: hasUserModifiers + )) + } + + @Test + func programmaticTextRequiresNewline() { + #expect(TerminalCommandSubmission.textContainsNewline("sleep 1\n")) + #expect(TerminalCommandSubmission.textContainsNewline("sleep 1\r")) + #expect(!TerminalCommandSubmission.textContainsNewline("sleep 1")) + } + + @Test + func contentEvidenceIgnoresBlankAndControlText() { + var evidence = TerminalCommandSubmission.Evidence() + evidence.recordText(" \t\r\n\u{3}") + let consumed = evidence.consume() + #expect(!consumed) + } + + @Test + func contentEvidenceIsConsumedOnce() { + var evidence = TerminalCommandSubmission.Evidence() + evidence.recordText("! sleep 10") + let first = evidence.consume() + let second = evidence.consume() + #expect(first) + #expect(!second) + + evidence.recordText("こんにちは") + let imeText = evidence.consume() + #expect(imeText) + } + + @Test + func imeCommitReturnPreservesContentForFollowingSubmission() { + var evidence = TerminalCommandSubmission.Evidence() + evidence.recordText("かな") + let commitIsSubmission = TerminalCommandSubmission.isReturn( + keyCode: 36, + isRepeat: false, + hasMarkedText: true, + hasUserModifiers: false + ) + #expect(!commitIsSubmission) + + let followingReturnIsSubmission = TerminalCommandSubmission.isReturn( + keyCode: 36, + isRepeat: false, + hasMarkedText: false, + hasUserModifiers: false + ) + let hasContent = evidence.consume() + #expect(followingReturnIsSubmission) + #expect(hasContent) + } + + @Test + func pasteEvidenceUsesResolvedContent() { + var evidence = TerminalCommandSubmission.Evidence() + evidence.recordText(" \n\t") + let blankPaste = evidence.consume() + #expect(!blankPaste) + + evidence.recordText("! sleep 10\n") + let commandPaste = evidence.consume() + #expect(commandPaste) + } + + @Test + func destructiveInputDiscardsEvidence() { + var evidence = TerminalCommandSubmission.Evidence() + evidence.recordText("x") + #expect(TerminalCommandSubmission.clearsInputEvidence( + keyCode: 51, hasControl: false, hasCommand: false + )) + evidence.clear() + let erased = evidence.consume() + #expect(!erased) + + #expect(TerminalCommandSubmission.clearsInputEvidence( + keyCode: 53, hasControl: false, hasCommand: false + )) + #expect(TerminalCommandSubmission.clearsInputEvidence( + keyCode: 8, hasControl: true, hasCommand: false + )) + #expect(TerminalCommandSubmission.clearsInputEvidence( + keyCode: 0, hasControl: false, hasCommand: true + )) + #expect(TerminalCommandSubmission.clearsInputEvidence( + keyCode: 7, hasControl: false, hasCommand: true + )) + #expect(!TerminalCommandSubmission.clearsInputEvidence( + keyCode: 8, hasControl: false, hasCommand: false + )) + } + + @Test + func optionAsAltTextDoesNotCountAsLiteralContent() { + #expect(!TerminalCommandSubmission.shouldRecordLiteralText(hasOption: true)) + #expect(TerminalCommandSubmission.shouldRecordLiteralText(hasOption: false)) + } +} diff --git a/scripts/setup.sh b/scripts/setup.sh index bd5e4e9c..9f39d03b 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -14,11 +14,23 @@ RESOURCES_MARKER="Macterm/Resources/terminfo" # and downloaded here, mirroring GhosttyKit — never compiled locally (zig). # Embedded into the bundle at Contents/Resources/zmx/zmx by embed-zmx.sh. ZMX_BIN="Macterm/Resources/zmx/zmx" +GHOSTTY_HEADER="$XCFRAMEWORK_DIR/macos-arm64_x86_64/Headers/ghostty.h" + +has_output_activity_action() { + [[ -f "$GHOSTTY_HEADER" ]] && grep -q 'GHOSTTY_ACTION_OUTPUT_ACTIVITY' "$GHOSTTY_HEADER" +} need_xcframework=true need_resources=true need_zmx=true -[[ -d "$XCFRAMEWORK_DIR" ]] && need_xcframework=false +if [[ -d "$XCFRAMEWORK_DIR" ]]; then + if has_output_activity_action; then + need_xcframework=false + else + echo "Existing GhosttyKit lacks GHOSTTY_ACTION_OUTPUT_ACTIVITY; refreshing it" + rm -rf "$XCFRAMEWORK_DIR" + fi +fi [[ -d "$RESOURCES_MARKER" ]] && need_resources=false [[ -x "$ZMX_BIN" ]] && need_zmx=false @@ -37,6 +49,11 @@ if $need_xcframework; then gh release download "$LATEST_TAG" --pattern "GhosttyKit.xcframework.tar.gz" --repo "$FORK_REPO" tar xzf GhosttyKit.xcframework.tar.gz rm GhosttyKit.xcframework.tar.gz + if ! has_output_activity_action; then + echo "Error: GhosttyKit from $LATEST_TAG lacks GHOSTTY_ACTION_OUTPUT_ACTIVITY" >&2 + echo "The thdxg/ghostty output-activity downstream patch must be released first." >&2 + exit 1 + fi fi # Fork-drift warning (macterm#168). The thdxg/ghostty fork ships prebuilt diff --git a/website/docs/pages/80-cli.md b/website/docs/pages/80-cli.md index ed540c58..52908e50 100644 --- a/website/docs/pages/80-cli.md +++ b/website/docs/pages/80-cli.md @@ -46,7 +46,7 @@ The grammar is `macterm [options]`. A bare noun defaults to its `l | `tab new [--project P] [--run CMD]` | New tab, becomes active. `--run` types CMD into the fresh shell. | | `tab select ` | Activate a tab (`tab:3`, index, UUID, or exact title). | | `tab close [--force]` | Close a tab, killing its panes' sessions. Refuses with `busy` when a pane runs a program, unless forced. | -| `pane list [--project P] [--tab T]` | Panes with refs, session names, cwd, foreground process, focus marker. | +| `pane list [--project P] [--tab T]` | Panes with refs, session names, cwd, foreground process, focus marker, and execution state (`idle`/`running`/`done`; live tracking requires the tab status indicator setting). | | `pane inspect [target]` | Read-only snapshot of a pane's terminal core: grid, cell/surface pixels, scrollback totals, content scale, foreground pid + argv. Needs a live surface. | | `pane dump [--scrollback] [target]` | Print a pane's terminal text — the viewport, or the full scrollback with `--scrollback`. Pipeline-friendly (text only). | | `pane split [--direction right\|down\|auto] [--run CMD] [target]` | Split a pane; the new pane inherits the source's cwd. `auto` picks the longer on-screen axis. | From 5c6ba6ac2f78c045b7757c1b95b82646b40d806f Mon Sep 17 00:00:00 2001 From: Mateusz Urban Date: Fri, 24 Jul 2026 11:54:12 +0200 Subject: [PATCH 2/6] test: exercise pane state transitions in CLI coverage --- MactermTests/Control/ControlHandlerTests.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/MactermTests/Control/ControlHandlerTests.swift b/MactermTests/Control/ControlHandlerTests.swift index a8df99d1..107a0095 100644 --- a/MactermTests/Control/ControlHandlerTests.swift +++ b/MactermTests/Control/ControlHandlerTests.swift @@ -208,11 +208,12 @@ struct ControlHandlerTests { appState.isAppActive = { false } let pane = try #require(appState.workspaces[project.id]?.activeTab?.splitRoot.allPanes().first) - pane.executionState = .running + pane.recordUserInteraction() + pane.markCommandRunning() var response = await handler.handle(request("pane.list")) #expect(response.data?.panes?.first?.state == "running") - pane.executionState = .done + pane.markCommandFinished() response = await handler.handle(request("pane.list")) #expect(response.data?.panes?.first?.state == "done") } From e91cbed2ea79fccda3c38b98a29f31d5d5d3559b Mon Sep 17 00:00:00 2001 From: Mateusz Urban Date: Sat, 25 Jul 2026 11:04:08 +0200 Subject: [PATCH 3/6] refactor: make the output heartbeat the sole activity source The OUTPUT_ACTIVITY heartbeat is now required (setup.sh rejects a GhosttyKit without the ABI) and fires from the pty IO path regardless of occlusion, so a quiet activity run's silence is meaningful on or off screen. That makes the whole pre-heartbeat occlusion workaround dead weight: drop the parked-renderer fallback (settleIfVisible, paneIsOccluded, previouslyOccludedPanes, refreshActivityWindow, hasOcclusionIndependentHeartbeat) so the poll settles occluded and visible panes identically. Also route activity solely through the heartbeat: the render-path scrollbar no longer feeds activity detection (it only drove the overlay scrollbar and was a redundant, visible-only echo of the heartbeat), which incidentally removes the double foreground re-resolve on visible panes. The TerminalExecutionTracker state machine is untouched. --- AGENTS.md | 2 +- Macterm/App/AppState.swift | 66 +---------- Macterm/Model/SplitNode.swift | 35 ++---- .../Terminal/GhosttyTerminalNSView.swift | 11 +- Macterm/Views/TerminalPane.swift | 13 +-- MactermTests/App/AppStateTests.swift | 105 ++++-------------- MactermTests/Model/PaneTests.swift | 11 -- 7 files changed, 46 insertions(+), 197 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2e51e422..4b92a9cc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,7 +70,7 @@ All keybinds are configurable via `HotkeyAction` + `HotkeyRegistry`. `KeyRouter` ### Tab Naming -A tab's auto-title is, by default, the pane's live **foreground process name** (`hx`, `btop`) — falling back to the login shell name (from `getpwuid`, not `$SHELL`) when idle, overridden by a user-set `customTitle`. `ProcessInspector.runningProcessName` reads the foreground pid's kernel `comm`. `AppState` polls panes adaptively (`PollCadence` + `refreshAllForegroundProcesses`, republishing only on change): 250ms during a ~5s burst after any poll event (tab switch, keystroke, OSC title, execution transition — all post `.terminalPollEvent`) or while a command runs frontmost, 1s when active-idle, 2s when inactive with a visible window, stopped when nothing is on screen (events resume it instantly; the quit dialog re-reads names one-shot since the poll may be paused). A throttled `GHOSTTY_ACTION_OUTPUT_ACTIVITY` heartbeat from the pty IO path (unlike the render-path `GHOSTTY_ACTION_SCROLLBAR`, it keeps firing while occluded) keeps activity-sourced runs alive during in-place redraws and lets the indicator quiet-settle occluded panes normally once a pane proves it receives one (`Pane.hasOcclusionIndependentHeartbeat`); the old occlusion exemption survives only as a fallback before that proof. Row growth may start activity under the normal interaction guards. Non-growing output can start activity only for a recognized AI-agent foreground, as two heartbeats within 2s of an actually forwarded unmodified Return/newline submission with committed prompt content; a genuinely blank submission briefly suppresses even row-growth redraws. This covers Pi's nested `!` commands without treating Return in ordinary raw TUIs, startup output, typing, IME composition, or one prompt redraw as work. OSC 0/2 titles are **provenance-gated** (`Pane.receiveReportedTitle`): the raw sequence can't distinguish a program naming its session (claude, ssh) from a shell titling its prompt (nushell, Starship emit the cwd), so the title string is adopted as `Pane.programTitle` only while the foreground process is a real program — not a shell — and is pinned to that pid; the poll expires it when the pid loses the foreground (`applyForegroundRefresh`), and prompt-time titles are discarded. `displayTitle` prefers `programTitle` over the process name; the quit dialog keeps the process-derived `processTitle`. Every OSC title arrival also triggers a process refresh (command boundary). Titles aren't persisted — always derived live. +A tab's auto-title is, by default, the pane's live **foreground process name** (`hx`, `btop`) — falling back to the login shell name (from `getpwuid`, not `$SHELL`) when idle, overridden by a user-set `customTitle`. `ProcessInspector.runningProcessName` reads the foreground pid's kernel `comm`. `AppState` polls panes adaptively (`PollCadence` + `refreshAllForegroundProcesses`, republishing only on change): 250ms during a ~5s burst after any poll event (tab switch, keystroke, OSC title, execution transition — all post `.terminalPollEvent`) or while a command runs frontmost, 1s when active-idle, 2s when inactive with a visible window, stopped when nothing is on screen (events resume it instantly; the quit dialog re-reads names one-shot since the poll may be paused). The throttled `GHOSTTY_ACTION_OUTPUT_ACTIVITY` heartbeat from the pty IO path is the **sole** activity source (the render-path `GHOSTTY_ACTION_SCROLLBAR` feeds only the overlay scrollbar): it keeps firing while occluded and `setup.sh` requires the ABI that emits it, so an activity-sourced run's silence is meaningful on or off screen — the indicator quiet-settles occluded and visible panes identically, with no occlusion exemption. The heartbeat carries the row total, keeping runs alive through in-place redraws and letting the tracker distinguish growth from a redraw. Row growth may start activity under the normal interaction guards. Non-growing output can start activity only for a recognized AI-agent foreground, as two heartbeats within 2s of an actually forwarded unmodified Return/newline submission with committed prompt content; a genuinely blank submission briefly suppresses even row-growth redraws. This covers Pi's nested `!` commands without treating Return in ordinary raw TUIs, startup output, typing, IME composition, or one prompt redraw as work. OSC 0/2 titles are **provenance-gated** (`Pane.receiveReportedTitle`): the raw sequence can't distinguish a program naming its session (claude, ssh) from a shell titling its prompt (nushell, Starship emit the cwd), so the title string is adopted as `Pane.programTitle` only while the foreground process is a real program — not a shell — and is pinned to that pid; the poll expires it when the pid loses the foreground (`applyForegroundRefresh`), and prompt-time titles are discarded. `displayTitle` prefers `programTitle` over the process name; the quit dialog keeps the process-derived `processTitle`. Every OSC title arrival also triggers a process refresh (command boundary). Titles aren't persisted — always derived live. **Remote panes** (#104) leave the local pipeline entirely — the local process table only knows the `ssh` client, so `ProcessInspector.foregroundPID(forPane:)` returns nil for them (which also makes layout `save` emit plain leaves and reconcile match them as idle). Their naming is two-tier: OSC 0/2 titles gated by the OSC 133 execution state instead of a pid (`Pane.receiveRemoteReportedTitle` — adopt while running, discard prompt churn, expire on the running→ended edge), plus `RemoteForegroundResolver`, one BatchMode ssh per host per ~3s (frontmost project only, overlapping probes dropped, failures freeze names) running the same session→leader→tpgid→comm pipeline as a portable POSIX script (`RemoteSpawn.foregroundProbeScript`). Idle fallback is the host name. diff --git a/Macterm/App/AppState.swift b/Macterm/App/AppState.swift index 77834b1e..174731ff 100644 --- a/Macterm/App/AppState.swift +++ b/Macterm/App/AppState.swift @@ -161,26 +161,6 @@ final class AppState { (NSApp?.windows ?? []).contains { $0.isVisible && $0.occlusionState.contains(.visible) } } - /// Whether a pane's surface is occluded — its renderer parked by - /// `ghostty_surface_set_occlusion`, so render/scrollbar heartbeats are - /// suppressed and (absent an occlusion-independent output heartbeat, - /// see `Pane.hasOcclusionIndependentHeartbeat`) 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 (and lack an - /// occlusion-independent heartbeat), so the visible transition can - /// restart their quiet window before settling resumes. A pane that gains - /// the heartbeat is dropped from here immediately (see `settleIfVisible`) - /// so a later, unrelated occlusion edge can't replay stale bookkeeping. - @ObservationIgnored - private var previouslyOccludedPanes: Set = [] - /// zmx session-persistence client. Injectable so tests can observe /// session kills without a real daemon. @ObservationIgnored @@ -392,13 +372,11 @@ final class AppState { // this feature. let trackExecution = Preferences.shared.showTabStatusIndicator var didAcknowledgeCompletion = false - var seenPanes: Set = [] var sawBusyPane = false var activeRemotePanes: [Pane] = [] for (projectID, ws) in workspaces { for tab in ws.tabs { for pane in tab.splitRoot.allPanes() { - seenPanes.insert(pane.id) if pane.isRemote { // The local process table only knows `ssh` here — a // local refresh would stomp the probe-derived name @@ -412,8 +390,12 @@ final class AppState { } else { pane.refreshForegroundProcess(trackExecution: trackExecution) } + // An activity-sourced run whose output has been quiet past + // the window settles to `.done`. The output heartbeat is + // occlusion-independent, so silence is meaningful whether or + // not the pane is on screen — no occlusion special-casing. if trackExecution { - settleIfVisible(pane) + pane.settleTerminalActivityIfQuiet() } if pane.executionState == .running { sawBusyPane = true } didAcknowledgeCompletion = acknowledgeFinishedCommandIfActive( @@ -424,7 +406,6 @@ final class AppState { } } } - previouslyOccludedPanes.formIntersection(seenPanes) lastPollSawBusyPane = sawBusyPane if didAcknowledgeCompletion { saveWorkspaces() } if !activeRemotePanes.isEmpty, isAnyWindowVisible() { @@ -432,43 +413,6 @@ final class AppState { } } - /// Quiet-settle a pane, working around the occluded renderer's silence - /// where necessary. A pane whose GhosttyKit build delivers - /// `OUTPUT_ACTIVITY` heartbeats (`hasOcclusionIndependentHeartbeat`) - /// proved those heartbeats reach it regardless of occlusion, so its - /// silence is meaningful even while occluded — settle it exactly like a - /// visible pane, with no skip and no fresh-window grant on de-occlusion. - /// - /// Absent that proof (older GhosttyKit, or no heartbeat has arrived yet), - /// fall back to the original #123 behavior: an occluded pane's renderer - /// is parked, so it emits no activity heartbeats at all, and settling it - /// would misread that 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. - /// - /// 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 pane.hasOcclusionIndependentHeartbeat { - // This pane's silence is trustworthy even while occluded — drop - // any bookkeeping from before the heartbeat proved that, so a - // later occlusion edge (if the flag were ever meaningful again) - // can't replay a stale grant. - previouslyOccludedPanes.remove(pane.id) - pane.settleTerminalActivityIfQuiet() - return - } - 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) Preferences.defaults.set(projectRecency.items.map(\.uuidString), forKey: recencyKey) diff --git a/Macterm/Model/SplitNode.swift b/Macterm/Model/SplitNode.swift index feb39a63..7281970d 100644 --- a/Macterm/Model/SplitNode.swift +++ b/Macterm/Model/SplitNode.swift @@ -283,14 +283,6 @@ struct TerminalExecutionTracker { return .done } - /// Restart the quiet window of an activity-sourced run. Used only by the - /// compatibility path for surfaces that have not proven they receive the - /// occlusion-independent heartbeat. - mutating func refreshActivityWindow(now: Date) { - guard case .activity = runningSource else { return } - runningSource = .activity(now) - } - mutating func refreshForeground( name: String?, pid: pid_t?, @@ -586,6 +578,9 @@ final class Pane: Identifiable { cancelActivityQuietPollIfNeeded() } + /// The row-growth activity primitive that `markOutputActivity` delegates + /// to on a growing heartbeat. Exposed directly so tests can seed an + /// activity-sourced run at a chosen instant without a growth baseline. func markTerminalActivity(at date: Date = Date()) { executionState = executionTracker.markTerminalActivity( at: date, @@ -602,24 +597,13 @@ final class Pane: Identifiable { cancelActivityQuietPollIfNeeded() } - func refreshTerminalActivityWindow(now: Date = Date()) { - executionTracker.refreshActivityWindow(now: now) - } - - /// Set on the first `OUTPUT_ACTIVITY` heartbeat delivered by libghostty - /// for this surface. Its presence proves the running GhosttyKit build - /// delivers occlusion-independent heartbeats (they fire from the pty IO - /// path, not the renderer), so `AppState`'s quiet-settle no longer needs - /// the occluded-pane exemption for this pane — silence while occluded is - /// now a meaningful signal instead of an artifact of a parked renderer. - private(set) var hasOcclusionIndependentHeartbeat = false - - /// Handle a throttled `OUTPUT_ACTIVITY` heartbeat (see - /// `TerminalExecutionTracker.markOutputActivity`). Unlike - /// `markTerminalActivity` (scrollbar-growth, visible surfaces only), this - /// also reaches occluded/background panes. + /// Handle a throttled `OUTPUT_ACTIVITY` heartbeat — the pane's sole source + /// of terminal activity. It fires from the pty IO path (not the renderer), + /// so unlike the scrollbar it also reaches occluded/background panes and + /// its silence is always meaningful. `setup.sh` requires the GhosttyKit + /// ABI that emits it, so the quiet-settle needs no occluded-pane exemption. + /// Growth-vs-keepalive is decided in `TerminalExecutionTracker`. func markOutputActivity(totalRows: UInt64, now: Date = Date()) { - hasOcclusionIndependentHeartbeat = true executionState = executionTracker.markOutputActivity(totalRows: totalRows, at: now, currentState: executionState) scheduleActivityQuietPollIfNeeded() } @@ -860,7 +844,6 @@ final class Pane: Identifiable { view.onCommandFinished = nil view.onProgressStarted = nil view.onProgressFinished = nil - view.onTerminalActivity = nil view.onOutputActivity = nil view.onScrollbarUpdate = nil view.onScrollWheel = nil diff --git a/Macterm/Views/Terminal/GhosttyTerminalNSView.swift b/Macterm/Views/Terminal/GhosttyTerminalNSView.swift index 99f34ae3..24a45aa0 100644 --- a/Macterm/Views/Terminal/GhosttyTerminalNSView.swift +++ b/Macterm/Views/Terminal/GhosttyTerminalNSView.swift @@ -118,11 +118,11 @@ final class GhosttyTerminalNSView: NSView { } func surfaceDidUpdateScrollbar(total: UInt64, offset: UInt64, len: UInt64) { - let snapshot = ScrollbarSnapshot(total: total, offset: offset, len: len) - if let lastScrollbarSnapshot, total > lastScrollbarSnapshot.total { - onTerminalActivity?() - } - lastScrollbarSnapshot = snapshot + // Renderer-driven, so it's suppressed while occluded — it feeds only the + // overlay scrollbar UI, never activity detection. Activity comes solely + // from the occlusion-independent `surfaceDidOutputActivity` heartbeat, + // which also carries row growth. + lastScrollbarSnapshot = ScrollbarSnapshot(total: total, offset: offset, len: len) onScrollbarUpdate?(total, offset, len) } @@ -165,7 +165,6 @@ final class GhosttyTerminalNSView: NSView { var onCommandFinished: ((Int16, UInt64) -> Void)? var onProgressStarted: (() -> Void)? var onProgressFinished: (() -> Void)? - var onTerminalActivity: (() -> Void)? /// libghostty pushes scrollback geometry (all values in rows) whenever the /// viewport, scrollback size, or visible row count changes. /// `(total, offset, len)`: total rows including scrollback, the first diff --git a/Macterm/Views/TerminalPane.swift b/Macterm/Views/TerminalPane.swift index 568e13b0..df7b446d 100644 --- a/Macterm/Views/TerminalPane.swift +++ b/Macterm/Views/TerminalPane.swift @@ -208,17 +208,12 @@ private struct TerminalSurface: NSViewRepresentable { pane.markProgressFinished() onCommandFinished() } - view.onTerminalActivity = { [weak pane] in - guard let pane, Preferences.shared.showTabStatusIndicator else { return } - pane.refreshForegroundProcess() - pane.markTerminalActivity() - } view.onOutputActivity = { [weak pane] total in guard let pane, Preferences.shared.showTabStatusIndicator else { return } - // Output heartbeats fire from the pty IO path regardless of - // occlusion, so they also reach background tabs — unlike the - // scrollbar-growth path above, which only fires while the - // surface is actually rendered. Always refresh foreground/raw + // The single activity source. Output heartbeats fire from the pty + // IO path regardless of occlusion, so they also reach background + // tabs, and they carry the row total so the tracker can tell + // growth from an in-place redraw. Always refresh foreground/raw // state first: a running canonical command can switch to a raw TUI // while normal polling is paused behind a fully occluded window. // The heartbeat is throttled to ~2 Hz, and the tracker preserves diff --git a/MactermTests/App/AppStateTests.swift b/MactermTests/App/AppStateTests.swift index 70b052fa..f8f3fd4b 100644 --- a/MactermTests/App/AppStateTests.swift +++ b/MactermTests/App/AppStateTests.swift @@ -999,18 +999,18 @@ struct AppStateTests { #expect(AppState.panesToWarm(in: ws).isEmpty) } - // MARK: - Occlusion-aware quiet-settle - - // 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. + // MARK: - Quiet-settle + + // The poll calls `pane.settleTerminalActivityIfQuiet()` directly (no + // occlusion special-casing): the OUTPUT_ACTIVITY heartbeat that sources + // activity is occlusion-independent, so a quiet pane settles the same + // whether or not it is on screen. These drive the settle in isolation — + // deterministic (no live surface, no `Preferences` global) and a truer + // unit than the full `refreshAllForegroundProcesses` tick, which re-reads + // each pane's real foreground process (nil under test, clearing the run + // source). + + /// A pane whose activity went quiet long ago (past the 3s settle window). private func quietRunningPane() -> Pane { let pane = Pane(projectPath: "/tmp", projectID: UUID()) pane.recordUserInteraction() @@ -1020,83 +1020,22 @@ struct AppStateTests { } @Test - func occluded_pane_does_not_quiet_settle() { - let state = makeAppState() + func quiet_activity_run_settles_to_done() { let pane = quietRunningPane() - state.paneIsOccluded = { _ in true } - - 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() { - let state = makeAppState() - let pane = quietRunningPane() - state.paneIsOccluded = { _ in false } - - state.settleIfVisible(pane) + // 10s of silence is past the window — occluded or not, it's done. + pane.settleTerminalActivityIfQuiet() #expect(pane.executionState == .done) } @Test - func deoccluded_pane_gets_fresh_quiet_window_before_settling() { - let state = makeAppState() - let pane = quietRunningPane() - - // Occluded: no settle, and the pane is marked as having been occluded. - state.paneIsOccluded = { _ in true } - state.settleIfVisible(pane) - #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 genuine quiet now elapsing from the reset window, it settles. - pane.settleTerminalActivityIfQuiet(now: Date().addingTimeInterval(4)) - #expect(pane.executionState == .done) - } - - @Test - func occluded_pane_without_heartbeat_flag_keeps_pre_heartbeat_behavior() { - // A pane whose GhosttyKit build never delivered an OUTPUT_ACTIVITY - // heartbeat has no proof its silence is meaningful while occluded — - // must still skip settling and grant a fresh window on de-occlusion, - // exactly like `deoccluded_pane_gets_fresh_quiet_window_before_settling`. - let state = makeAppState() - let pane = quietRunningPane() - #expect(!pane.hasOcclusionIndependentHeartbeat) - - state.paneIsOccluded = { _ in true } - state.settleIfVisible(pane) - #expect(pane.executionState == .running) - - state.paneIsOccluded = { _ in false } - state.settleIfVisible(pane) - #expect(pane.executionState == .running) - pane.settleTerminalActivityIfQuiet(now: Date().addingTimeInterval(4)) - #expect(pane.executionState == .done) - } - - @Test - func occluded_pane_with_heartbeat_flag_settles_normally_while_occluded() { - // Once a pane has proven it receives occlusion-independent heartbeats, - // its silence is meaningful even while occluded: settle it exactly - // like a visible pane — no skip, no fresh-window grant needed. - let state = makeAppState() - let pane = quietRunningPane() - pane.markOutputActivity(totalRows: 1, now: Date().addingTimeInterval(-10)) - #expect(pane.hasOcclusionIndependentHeartbeat) + func activity_run_holds_until_the_quiet_window_elapses() { + let pane = Pane(projectPath: "/tmp", projectID: UUID()) + pane.recordUserInteraction() + let start = Date() + pane.markTerminalActivity(at: start) + pane.settleTerminalActivityIfQuiet(now: start.addingTimeInterval(2), quietInterval: 3) #expect(pane.executionState == .running) - - state.paneIsOccluded = { _ in true } - state.settleIfVisible(pane) + pane.settleTerminalActivityIfQuiet(now: start.addingTimeInterval(3), quietInterval: 3) #expect(pane.executionState == .done) } diff --git a/MactermTests/Model/PaneTests.swift b/MactermTests/Model/PaneTests.swift index 7078b386..bd9c8d45 100644 --- a/MactermTests/Model/PaneTests.swift +++ b/MactermTests/Model/PaneTests.swift @@ -331,17 +331,6 @@ struct PaneTests { #expect(p.executionState == .idle) } - @Test - func markOutputActivity_setsOcclusionIndependentHeartbeatFlag() { - // The flag records that this surface's build actually delivers - // OUTPUT_ACTIVITY heartbeats, which `AppState.settleIfVisible` uses - // to decide whether an occluded pane's silence is trustworthy. - let p = Pane(projectPath: "/", projectID: UUID()) - #expect(!p.hasOcclusionIndependentHeartbeat) - p.markOutputActivity(totalRows: 10) - #expect(p.hasOcclusionIndependentHeartbeat) - } - @Test func outputActivitySchedulesQuietPollWake() async { let p = Pane( From 1271165fc74ef9c560171f7c0e531fdb4c8fc7dc Mon Sep 17 00:00:00 2001 From: Mateusz Urban Date: Mon, 27 Jul 2026 11:10:59 +0200 Subject: [PATCH 4/6] setup: validate a GhosttyKit replacement before discarding the working one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ABI refresh deleted the existing xcframework up front and only checked the downloaded replacement afterwards, so a release that also lacked GHOSTTY_ACTION_OUTPUT_ACTIVITY left the tree with no framework at all — a checkout that could previously build, broken, with every re-run repeating the failure. Download and validate into a scratch dir and swap only on success, so a bad release is a no-op instead of a regression. The probe also named the macos-arm64_x86_64 slice. A renamed slice would have reported "missing ABI" forever, re-downloading on every setup run with no indication why; glob the slice dirs instead. Requiring the ABI is a bisect hazard, so say so where someone hits it: the failure message now explains that pre-change checkouts need a GhosttyKit from a contemporary release, and points at AGENTS.md. --- AGENTS.md | 2 +- scripts/setup.sh | 45 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4b92a9cc..4b013207 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,7 @@ mise run bench # Release build + window-state resource benchmark Requires macOS 14+, Swift 6.0+. Liquid glass and a few chrome refinements are macOS 26 (Tahoe) features that degrade gracefully on older systems (gated behind `WindowAppearance.glassSupported` / `#available`). GhosttyKit is a pre-built xcframework from `thdxg/ghostty` (a fork that adds CI builds); no zig toolchain needed. -`GhosttyKit.xcframework` and the `Macterm/Resources/{ghostty,terminfo,…}` contents are gitignored artifacts downloaded by `mise run setup` — **every fresh checkout, including a git worktree, must run `mise run setup`** before it can build. Don't symlink them from another checkout: setup normally uses a presence check rather than a version check, so a symlinked copy silently goes stale. The one capability exception is `GHOSTTY_ACTION_OUTPUT_ACTIVITY`: setup replaces a present framework that lacks this required ABI. To refresh any other stale artifact, delete it and re-run setup. +`GhosttyKit.xcframework` and the `Macterm/Resources/{ghostty,terminfo,…}` contents are gitignored artifacts downloaded by `mise run setup` — **every fresh checkout, including a git worktree, must run `mise run setup`** before it can build. Don't symlink them from another checkout: setup normally uses a presence check rather than a version check, so a symlinked copy silently goes stale. The one capability exception is `GHOSTTY_ACTION_OUTPUT_ACTIVITY`: setup replaces a present framework that lacks this required ABI (probed by globbing the xcframework's slice dirs for the symbol in `Headers/ghostty.h`, never by naming a slice). The replacement is downloaded and validated in a scratch dir and only swapped in once it passes, so a release that also lacks the ABI leaves the existing framework alone rather than stranding the checkout with none. Requiring the ABI is also a **bisect hazard**: a checkout from before the reliable-activity-detection change doesn't need the ABI, but setup only ever fetches `latest`, so if that release ever regresses the symbol setup refuses to run at all. Bisecting across that commit means extracting a GhosttyKit from a `thdxg/ghostty` release contemporary with the older commit rather than running setup (its error message says so). To refresh any other stale artifact, delete it and re-run setup. ## Releasing & Updates diff --git a/scripts/setup.sh b/scripts/setup.sh index 9f39d03b..c597e8ff 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -14,21 +14,33 @@ RESOURCES_MARKER="Macterm/Resources/terminfo" # and downloaded here, mirroring GhosttyKit — never compiled locally (zig). # Embedded into the bundle at Contents/Resources/zmx/zmx by embed-zmx.sh. ZMX_BIN="Macterm/Resources/zmx/zmx" -GHOSTTY_HEADER="$XCFRAMEWORK_DIR/macos-arm64_x86_64/Headers/ghostty.h" - +# Does the xcframework rooted at $1 expose the output-activity ABI Macterm's +# activity indicator requires? Globs the slice directories rather than naming +# one: the macOS slice's name is chosen by the fork's build (today +# `macos-arm64_x86_64`), and hardcoding it would make a renamed slice report +# "missing ABI" forever — a silent re-download on every single setup run with +# no indication why. has_output_activity_action() { - [[ -f "$GHOSTTY_HEADER" ]] && grep -q 'GHOSTTY_ACTION_OUTPUT_ACTIVITY' "$GHOSTTY_HEADER" + local root="$1" header + for header in "$root"/*/Headers/ghostty.h; do + [[ -f "$header" ]] || continue + if grep -q 'GHOSTTY_ACTION_OUTPUT_ACTIVITY' "$header"; then return 0; fi + done + return 1 } need_xcframework=true need_resources=true need_zmx=true if [[ -d "$XCFRAMEWORK_DIR" ]]; then - if has_output_activity_action; then + if has_output_activity_action "$XCFRAMEWORK_DIR"; then need_xcframework=false else + # Deliberately NOT removed here. The replacement is downloaded and validated + # in a scratch dir below and only swapped in once it's known good, so a + # release that also lacks the ABI leaves this (older, but working) framework + # alone instead of stranding the checkout with no framework at all. echo "Existing GhosttyKit lacks GHOSTTY_ACTION_OUTPUT_ACTIVITY; refreshing it" - rm -rf "$XCFRAMEWORK_DIR" fi fi [[ -d "$RESOURCES_MARKER" ]] && need_resources=false @@ -46,14 +58,29 @@ if [[ -z "$LATEST_TAG" ]]; then fi if $need_xcframework; then - gh release download "$LATEST_TAG" --pattern "GhosttyKit.xcframework.tar.gz" --repo "$FORK_REPO" - tar xzf GhosttyKit.xcframework.tar.gz - rm GhosttyKit.xcframework.tar.gz - if ! has_output_activity_action; then + # Download and validate into a scratch dir, then swap. Validating in place + # would mean a release without the ABI leaves the tree with no framework at + # all (the pre-existing one already deleted, the new one rejected), and every + # re-run repeats the failure. Staging keeps a bad release a no-op. + staging="$(mktemp -d "${TMPDIR:-/tmp}/macterm-ghosttykit.XXXXXX")" + trap 'rm -rf "$staging"' EXIT + gh release download "$LATEST_TAG" --pattern "GhosttyKit.xcframework.tar.gz" --repo "$FORK_REPO" --dir "$staging" + tar xzf "$staging/GhosttyKit.xcframework.tar.gz" -C "$staging" + if ! has_output_activity_action "$staging/$XCFRAMEWORK_DIR"; then echo "Error: GhosttyKit from $LATEST_TAG lacks GHOSTTY_ACTION_OUTPUT_ACTIVITY" >&2 echo "The thdxg/ghostty output-activity downstream patch must be released first." >&2 + echo "" >&2 + echo "Note: Macterm has required this ABI since the reliable-activity-detection" >&2 + echo "change (see the GhosttyKit note in AGENTS.md). A checkout from BEFORE that" >&2 + echo "commit — e.g. while bisecting — does not need it: extract a GhosttyKit from" >&2 + echo "a $FORK_REPO release contemporary with that commit instead of running setup." >&2 + echo "Any existing $XCFRAMEWORK_DIR was left untouched." >&2 exit 1 fi + rm -rf "$XCFRAMEWORK_DIR" + mv "$staging/$XCFRAMEWORK_DIR" "$XCFRAMEWORK_DIR" + rm -rf "$staging" + trap - EXIT fi # Fork-drift warning (macterm#168). The thdxg/ghostty fork ships prebuilt From 5c55bb37522aba45db89e4a83b2619bc9c66f41b Mon Sep 17 00:00:00 2001 From: Mateusz Urban Date: Mon, 27 Jul 2026 11:11:09 +0200 Subject: [PATCH 5/6] refactor: name the submission key codes and keep the userdata cast in one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bare keycode literals behind a `// H/C/W/U/K` gloss are easy to transpose on edit, and nothing downstream notices when they change which keys count as a submission or wipe the evidence — the escape/delete line was already two labels for three codes. Name them, and pin them to HotkeyRegistry's shared vocabulary in a test: TerminalCommandSubmission stays isolation-free so it can't read that @MainActor map at runtime, but it can be held to it. readClipboard open-coded Unmanaged.fromOpaque at a second call site; give surfaceView an overload taking the userdata pointer so the teardown guard and the cast stay together. Also record why the paste evidence is dispatched before the synchronous clipboard completion — that ordering is load-bearing and was unremarked. --- Macterm/App/Hotkeys.swift | 9 ++++ Macterm/Ghostty/GhosttyCallbacks.swift | 21 ++++++--- .../Terminal/TerminalCommandSubmission.swift | 45 +++++++++++++++++-- .../TerminalCommandSubmissionTests.swift | 29 ++++++++++++ 4 files changed, 95 insertions(+), 9 deletions(-) diff --git a/Macterm/App/Hotkeys.swift b/Macterm/App/Hotkeys.swift index 994c2ea6..d2b7ccff 100644 --- a/Macterm/App/Hotkeys.swift +++ b/Macterm/App/Hotkeys.swift @@ -164,6 +164,15 @@ enum HotkeyRegistry { keyCodeToBaseToken[keyCode] } + /// The hardware key code for a base key token (`"c"` → 8), or nil for a + /// token we don't map — the inverse of `baseToken(forKeyCode:)`. Lets code + /// that carries its own key codes (`TerminalCommandSubmission`, which stays + /// isolation-free and so can't read this `@MainActor` map at runtime) pin + /// them to this vocabulary in a test. + static func keyCode(forToken token: String) -> UInt16? { + keyCodes[token] + } + private static let modifierOnlyCodes: Set = [54, 55, 56, 57, 58, 59, 60, 61, 62] /// Characters produced by special keys → their named token form. diff --git a/Macterm/Ghostty/GhosttyCallbacks.swift b/Macterm/Ghostty/GhosttyCallbacks.swift index 043693a7..af8150ba 100644 --- a/Macterm/Ghostty/GhosttyCallbacks.swift +++ b/Macterm/Ghostty/GhosttyCallbacks.swift @@ -121,13 +121,17 @@ final class GhosttyCallbacks: @unchecked Sendable { // `ghostty_surface_complete_clipboard_request`'s surface parameter is // non-null on the Zig side; a request completing during surface // teardown (nil `surface`) is UB, not a graceful no-op. Guard it. - guard let ud else { return false } - let view = Unmanaged.fromOpaque(ud).takeUnretainedValue() - guard let surface = view.surface else { return false } + guard let view = surfaceView(from: ud), let surface = view.surface else { return false } let text = Self.readPasteboardText() ?? "" // Record the resolved payload, not just the Command-V key code: an // empty/whitespace clipboard must not make a later blank Return look // like a nonempty agent submission. + // + // Dispatched async BEFORE the synchronous completion below, which is + // the ordering the evidence depends on: the main queue runs the record + // after this callback returns — so after the paste has reached the + // surface — but still before any Return the user types next. Moving it + // after the completion call, or making it synchronous, breaks that. DispatchQueue.main.async { view.surfaceDidPasteText(text) } text.withCString { ghostty_surface_complete_clipboard_request(surface, $0, state, false) } return true @@ -345,8 +349,15 @@ final class GhosttyCallbacks: @unchecked Sendable { return Unmanaged.fromOpaque(ud).takeUnretainedValue() } - private func surface(from ud: UnsafeMutableRawPointer?) -> ghostty_surface_t? { + /// Recover the view from a libghostty userdata pointer. Every callback that + /// needs one goes through here so the unmanaged pointer cast lives in a + /// single place. + private func surfaceView(from ud: UnsafeMutableRawPointer?) -> GhosttyTerminalNSView? { guard let ud else { return nil } - return Unmanaged.fromOpaque(ud).takeUnretainedValue().surface + return Unmanaged.fromOpaque(ud).takeUnretainedValue() + } + + private func surface(from ud: UnsafeMutableRawPointer?) -> ghostty_surface_t? { + surfaceView(from: ud)?.surface } } diff --git a/Macterm/Views/Terminal/TerminalCommandSubmission.swift b/Macterm/Views/Terminal/TerminalCommandSubmission.swift index dccbd322..b120570e 100644 --- a/Macterm/Views/Terminal/TerminalCommandSubmission.swift +++ b/Macterm/Views/Terminal/TerminalCommandSubmission.swift @@ -1,7 +1,44 @@ import Foundation enum TerminalCommandSubmission { - private static let returnKeyCodes: Set = [36, 76] + /// Hardware key codes (Carbon `kVK_*`). Named rather than written inline: + /// a transposed literal silently changes which keys count as a submission + /// or wipe the evidence, and nothing downstream would notice. These mirror + /// `HotkeyRegistry`'s vocabulary — pinned to it by + /// `TerminalCommandSubmissionTests.keyCodesMatchHotkeyRegistry` rather than + /// read from it directly, since that registry is `@MainActor` and this + /// helper is deliberately isolation-free. + private enum KeyCode { + static let selectAll: UInt16 = 0 // A + static let cut: UInt16 = 7 // X + static let backspaceChord: UInt16 = 4 // H — ^H + static let cancel: UInt16 = 8 // C — ^C + static let killWord: UInt16 = 13 // W — ^W + static let killLine: UInt16 = 32 // U — ^U + static let killToEnd: UInt16 = 40 // K — ^K + static let `return`: UInt16 = 36 + static let keypadEnter: UInt16 = 76 + static let delete: UInt16 = 51 // Backspace + static let escape: UInt16 = 53 + static let forwardDelete: UInt16 = 117 + } + + private static let returnKeyCodes: Set = [KeyCode.return, KeyCode.keypadEnter] + + /// Keys that discard the line outright, no modifier needed. + private static let discardingKeyCodes: Set = [ + KeyCode.escape, KeyCode.delete, KeyCode.forwardDelete, + ] + + /// Control chords that destroy the current line or word, after which the + /// recorded text no longer describes what is in the prompt buffer. + private static let discardingControlKeyCodes: Set = [ + KeyCode.backspaceChord, KeyCode.cancel, KeyCode.killWord, + KeyCode.killLine, KeyCode.killToEnd, + ] + + /// Command chords that replace or remove the buffer wholesale. + private static let discardingCommandKeyCodes: Set = [KeyCode.selectAll, KeyCode.cut] /// Best-effort evidence that the next Return submits actual prompt text. /// Terminal protocols do not expose a TUI's editor buffer, so the view @@ -51,9 +88,9 @@ enum TerminalCommandSubmission { hasControl: Bool, hasCommand: Bool ) -> Bool { - if keyCode == 53 || keyCode == 51 || keyCode == 117 { return true } // Escape / delete - if hasControl, [4, 8, 13, 32, 40].contains(keyCode) { return true } // H/C/W/U/K - if hasCommand, [0, 7].contains(keyCode) { return true } // Select-all / cut + if discardingKeyCodes.contains(keyCode) { return true } + if hasControl, discardingControlKeyCodes.contains(keyCode) { return true } + if hasCommand, discardingCommandKeyCodes.contains(keyCode) { return true } return false } diff --git a/MactermTests/Views/Terminal/TerminalCommandSubmissionTests.swift b/MactermTests/Views/Terminal/TerminalCommandSubmissionTests.swift index 90c64aaf..900fa379 100644 --- a/MactermTests/Views/Terminal/TerminalCommandSubmissionTests.swift +++ b/MactermTests/Views/Terminal/TerminalCommandSubmissionTests.swift @@ -124,6 +124,35 @@ struct TerminalCommandSubmissionTests { )) } + /// `TerminalCommandSubmission` carries its own key codes (it stays free of + /// actor isolation, so it can't read the `@MainActor` registry at runtime). + /// Pin them to that shared vocabulary here so the two can't drift. + @Test @MainActor + func keyCodesMatchHotkeyRegistry() throws { + func code(_ token: String) throws -> UInt16 { + try #require(HotkeyRegistry.keyCode(forToken: token)) + } + + #expect(try TerminalCommandSubmission.isReturn( + keyCode: code("return"), isRepeat: false, hasMarkedText: false, hasUserModifiers: false + )) + #expect(try TerminalCommandSubmission.clearsInputEvidence( + keyCode: code("escape"), hasControl: false, hasCommand: false + )) + // ^H / ^C / ^W / ^U / ^K all destroy the line or a word. + for token in ["h", "c", "w", "u", "k"] { + #expect(try TerminalCommandSubmission.clearsInputEvidence( + keyCode: code(token), hasControl: true, hasCommand: false + )) + } + // ⌘A (select all) and ⌘X (cut) replace the buffer wholesale. + for token in ["a", "x"] { + #expect(try TerminalCommandSubmission.clearsInputEvidence( + keyCode: code(token), hasControl: false, hasCommand: true + )) + } + } + @Test func optionAsAltTextDoesNotCountAsLiteralContent() { #expect(!TerminalCommandSubmission.shouldRecordLiteralText(hasOption: true)) From 8c355119896880fc1d0a0a57f7098462212b123d Mon Sep 17 00:00:00 2001 From: Mateusz Urban Date: Mon, 27 Jul 2026 11:11:20 +0200 Subject: [PATCH 6/6] fix: give the quiet-settle wake margin and scope the input carry-over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scheduled wake fired at 3s and the settle it triggers requires now - lastActivityAt >= 3s, so it only worked while timer jitter ran positive. An early wake would no-op the settle, and with every window occluded there is no timer left to retry — the run would sit at .running forever, which is exactly what this machinery exists to prevent. Derive both from one constant with an explicit margin so they can't drift apart or meet. preserveProgrammaticCommandInput carried a payload's content evidence for 2s, and the evidence only ORs in — so a `pane run "cmd"` followed by a genuinely blank Return inside that window reported content it didn't have. The carry is only needed where a bracketed paste can swallow the newline, which is the same agent-TUI foreground the in-place heuristic requires, so gate it on that and keep the ambiguous window out of the shell case. Also: reschedule the wake with a work item rather than cancelling and respawning a Task at ~2 Hz per live pane, matching the idiom the view already uses; state the interaction-before-submission ordering contract at both ends, since recordUserInteraction clears the arming that recordCommandSubmission sets; and wait for the wake by polling instead of a fixed 100ms sleep against a 20ms timer (the shape that flaked in #181). --- Macterm/Model/SplitNode.swift | 72 ++++++++++++++----- .../Terminal/GhosttyTerminalNSView.swift | 20 ++++++ Macterm/Views/TerminalPane.swift | 4 ++ MactermTests/Model/PaneTests.swift | 43 +++++++++-- 4 files changed, 116 insertions(+), 23 deletions(-) diff --git a/Macterm/Model/SplitNode.swift b/Macterm/Model/SplitNode.swift index 7281970d..5dca7d44 100644 --- a/Macterm/Model/SplitNode.swift +++ b/Macterm/Model/SplitNode.swift @@ -70,6 +70,27 @@ private enum TerminalExecutionSource: Equatable { case progress } +/// Single source of truth for the two activity timings that must agree: how +/// long an activity-owned run may stay silent before it settles, and when the +/// dedicated wake that performs that settle fires. They are derived from one +/// constant so an edit can't drift them apart. +enum TerminalActivityTiming { + /// Silence after which an activity-owned run settles to `.done`. + static let quietInterval: TimeInterval = 3 + + /// Slack added to `quietInterval` for the scheduled wake. The settle + /// requires `now - lastActivityAt >= quietInterval`, so a wake targeting + /// exactly the threshold only works while timer jitter runs positive — and + /// the failure is bad: with every window occluded the ordinary poll is + /// stopped, so a marginally early wake would no-op the settle with no timer + /// left to retry, stranding the run at `.running` indefinitely. The margin + /// makes the wake land strictly after the threshold instead. + static let quietPollMargin: TimeInterval = 0.25 + + /// Delay for the dedicated quiet-settle wake. + static let quietPollDelay: TimeInterval = quietInterval + quietPollMargin +} + struct TerminalExecutionTracker { private enum PendingOutputStart { case armed(Date) @@ -120,6 +141,12 @@ struct TerminalExecutionTracker { return false } + /// ORDERING CONTRACT: this CLEARS the in-place start arming, so a caller + /// that reports both an interaction and a submission for the same event + /// must call this FIRST — `recordCommandSubmission` arms, and an + /// interaction recorded after it would silently disarm the Pi path. The + /// `keyDown` / `sendText` / `sendKey` paths all fire `onInteraction` before + /// `onCommandSubmitted` for exactly this reason. mutating func recordUserInteraction() { hasUserInteraction = true // Typing, scrolling, or any other interaction after Return means later @@ -484,8 +511,8 @@ final class Pane: Identifiable { /// Keep one lightweight wake scheduled from the final IO heartbeat so an /// occluded activity-owned run can still quiet-settle. @ObservationIgnored - private var activityQuietPollTask: Task? - private let activityQuietPollDelay: Duration + private var activityQuietPollWork: DispatchWorkItem? + private let activityQuietPollDelay: TimeInterval /// Re-read the foreground process name from the process table and publish it /// only when it changed (so a steady poll doesn't churn `@Observable` and @@ -588,7 +615,10 @@ final class Pane: Identifiable { ) } - func settleTerminalActivityIfQuiet(now: Date = Date(), quietInterval: TimeInterval = 3) { + func settleTerminalActivityIfQuiet( + now: Date = Date(), + quietInterval: TimeInterval = TerminalActivityTiming.quietInterval + ) { executionState = executionTracker.settleIfQuiet( now: now, quietInterval: quietInterval, @@ -613,28 +643,28 @@ final class Pane: Identifiable { cancelActivityQuietPollIfNeeded() return } - activityQuietPollTask?.cancel() - let delay = activityQuietPollDelay - activityQuietPollTask = Task { @MainActor [weak self] in - do { - try await Task.sleep(for: delay) - } catch { - return - } + // Rescheduled on every heartbeat (~2 Hz per live pane), so this uses a + // plain work item rather than spawning and cancelling a `Task` each + // time — the same idiom the view's `commandSubmissionEvidenceReset` + // uses. + activityQuietPollWork?.cancel() + let work = DispatchWorkItem { [weak self] in guard let self else { return } - activityQuietPollTask = nil + activityQuietPollWork = nil // This dedicated deadline bypasses ordinary event coalescing: if // every window is occluded there may be no timer left to retry. // AppState still performs the settle so acknowledgement and // persistence stay central. NotificationCenter.default.post(name: .terminalQuietSettleDeadline, object: self) } + activityQuietPollWork = work + DispatchQueue.main.asyncAfter(deadline: .now() + activityQuietPollDelay, execute: work) } private func cancelActivityQuietPollIfNeeded() { guard !executionTracker.isActivitySourced else { return } - activityQuietPollTask?.cancel() - activityQuietPollTask = nil + activityQuietPollWork?.cancel() + activityQuietPollWork = nil } @discardableResult @@ -662,12 +692,20 @@ final class Pane: Identifiable { NotificationCenter.default.post(name: .terminalPollEvent, object: nil) } + /// True when a recognized AI agent holds the foreground. Two things key off + /// it: the two-heartbeat in-place start heuristic (below), and the view's + /// decision to carry a programmatic payload's content evidence forward — + /// both are only meaningful in a raw-mode agent TUI, where a bracketed + /// paste can leave the payload sitting unsubmitted in the editor buffer. + var allowsInPlaceOutputStart: Bool { + agentIcon != nil || AgentIcon.match(processName: foregroundProcessName) != nil + } + func recordCommandSubmission(hasContent: Bool, at date: Date = Date()) { // Plain Return is ambiguous in editors and menus. Only a nonempty // submission in a recognized AI agent gets the two-heartbeat in-place // start heuristic; ordinary programs still use process/row evidence. - let allowInPlaceOutputStart = agentIcon != nil - || AgentIcon.match(processName: foregroundProcessName) != nil + let allowInPlaceOutputStart = allowsInPlaceOutputStart executionTracker.recordCommandSubmission( at: date, allowInPlaceOutputStart: allowInPlaceOutputStart, @@ -914,7 +952,7 @@ final class Pane: Identifiable { command: String? = nil, shell: String? = nil, env: [String: String]? = nil, - activityQuietPollDelay: Duration = .seconds(3) + activityQuietPollDelay: TimeInterval = TerminalActivityTiming.quietPollDelay ) { self.projectPath = projectPath self.projectID = projectID diff --git a/Macterm/Views/Terminal/GhosttyTerminalNSView.swift b/Macterm/Views/Terminal/GhosttyTerminalNSView.swift index 24a45aa0..809e8c94 100644 --- a/Macterm/Views/Terminal/GhosttyTerminalNSView.swift +++ b/Macterm/Views/Terminal/GhosttyTerminalNSView.swift @@ -152,7 +152,17 @@ final class GhosttyTerminalNSView: NSView { var onFocus: (() -> Void)? var onInteraction: (() -> Void)? /// Bool is best-effort evidence that the submitted prompt contained text. + /// + /// CALL ORDER: every path that reports a submission fires `onInteraction` + /// FIRST. `Pane.recordUserInteraction` clears the tracker's in-place start + /// arming that `recordCommandSubmission` then sets, so the reverse order + /// silently disarms the agent path. Keep the two calls in this order. var onCommandSubmitted: ((Bool) -> Void)? + /// Whether a programmatic payload's content evidence may be carried past + /// the submission that consumed it — true only for a raw-mode agent + /// foreground, where a bracketed paste can leave it unsubmitted. See + /// `preserveProgrammaticCommandInput`. + var canCarryCommandInput: (() -> Bool)? var onProcessExit: (() -> Void)? var onSplitRequest: ((SplitDirection, SplitPosition) -> Void)? var onZoomRequest: (() -> Void)? @@ -671,7 +681,17 @@ final class GhosttyTerminalNSView: NSView { /// bracketed-pasted into a raw TUI and need a following encoded Return. /// Preserve its content briefly for the latter without leaving stale /// evidence behind indefinitely in the former. + /// + /// Gated on `canCarryCommandInput`, because the two cases are + /// indistinguishable from here and the evidence only ORs in — never clears + /// — so an unconditional carry makes a genuinely blank Return arriving + /// inside the window report content it doesn't have (a `pane run "…"` + /// immediately followed by a bare newline is enough). The carry is only + /// *needed* where a bracketed paste can swallow the newline, which is the + /// same agent-TUI foreground the in-place heuristic requires, so scoping it + /// there keeps the ambiguous window out of the ordinary shell case. private func preserveProgrammaticCommandInput(_ text: String) { + guard canCarryCommandInput?() ?? false else { return } commandSubmissionEvidence.recordText(text) let reset = DispatchWorkItem { [weak self] in self?.commandSubmissionEvidence.clear() diff --git a/Macterm/Views/TerminalPane.swift b/Macterm/Views/TerminalPane.swift index df7b446d..c77a47b4 100644 --- a/Macterm/Views/TerminalPane.swift +++ b/Macterm/Views/TerminalPane.swift @@ -131,9 +131,13 @@ private struct TerminalSurface: NSViewRepresentable { view.onInteraction = { [weak pane] in pane?.recordUserInteraction() } + // Order matters: `onInteraction` clears the tracker's in-place start + // arming that `onCommandSubmitted` sets, and the view calls them in + // that order. See `GhosttyTerminalNSView.onCommandSubmitted`. view.onCommandSubmitted = { [weak pane] hasContent in pane?.recordCommandSubmission(hasContent: hasContent) } + view.canCarryCommandInput = { [weak pane] in pane?.allowsInPlaceOutputStart ?? false } view.onProcessExit = onProcessExit view.onSplitRequest = onSplitRequest view.onZoomRequest = onZoomRequest diff --git a/MactermTests/Model/PaneTests.swift b/MactermTests/Model/PaneTests.swift index bd9c8d45..4718921a 100644 --- a/MactermTests/Model/PaneTests.swift +++ b/MactermTests/Model/PaneTests.swift @@ -308,6 +308,20 @@ struct PaneTests { #expect(p.executionState == .running) } + /// The view carries a programmatic payload's content evidence past the + /// submission that consumed it only when this is true — otherwise a + /// `pane run "…"` followed within the carry window by a genuinely blank + /// Return would report content it doesn't have. + @Test + func inPlaceOutputStartIsAllowedOnlyForAnAgentForeground() { + let p = Pane(projectPath: "/", projectID: UUID()) + #expect(!p.allowsInPlaceOutputStart) + p.foregroundProcessName = "zsh" + #expect(!p.allowsInPlaceOutputStart) + p.foregroundProcessName = "pi" + #expect(p.allowsInPlaceOutputStart) + } + @Test func commandSubmissionDoesNotArmInPlaceOutputForOrdinaryRawProgram() { let p = Pane(projectPath: "/", projectID: UUID()) @@ -333,29 +347,46 @@ struct PaneTests { @Test func outputActivitySchedulesQuietPollWake() async { - let p = Pane( - projectPath: "/", - projectID: UUID(), - activityQuietPollDelay: .milliseconds(20) - ) + let p = Pane(projectPath: "/", projectID: UUID(), activityQuietPollDelay: 0.02) p.recordUserInteraction() p.markOutputActivity(totalRows: 10) await confirmation("quiet output wakes the paused poll") { confirm in + var fired = false let token = NotificationCenter.default.addObserver( forName: .terminalQuietSettleDeadline, object: p, queue: .main ) { _ in + fired = true confirm() } defer { NotificationCenter.default.removeObserver(token) } p.markOutputActivity(totalRows: 20) - try? await Task.sleep(for: .milliseconds(100)) + // Poll rather than sleeping a fixed multiple of the delay: a + // co-scheduled spike on a loaded CI runner can push a 20ms timer + // well past any fixed margin (cf. #181). The generous ceiling only + // bounds a genuine failure; a healthy run exits on the first tick. + for _ in 0 ..< 200 where !fired { + try? await Task.sleep(for: .milliseconds(25)) + } } } + /// The scheduled wake must land strictly *after* the quiet threshold it + /// needs to observe as crossed — a wake at exactly the threshold only + /// settles while timer jitter runs positive, and an occluded window has no + /// other timer left to retry with. + @Test + func quietPollWakeLandsAfterTheSettleThreshold() { + #expect(TerminalActivityTiming.quietPollDelay > TerminalActivityTiming.quietInterval) + #expect( + TerminalActivityTiming.quietPollDelay + == TerminalActivityTiming.quietInterval + TerminalActivityTiming.quietPollMargin + ) + } + @Test func foregroundProcessWithOutput_remainsRunningUntilItReturnsToShell() { let p = Pane(projectPath: "/", projectID: UUID())