fix: keep what Kelpie knows across a subscription rebuild

CLAUDE.md

@@ -130,6 +130,24 @@ herdr.
   `SubscriptionPlan` in `KelpieCore`. Note the delivered `event` field for
   per-pane subscription events is the **dotted** form
   (`pane.agent_status_changed`), unlike the underscored global events.
+- **`session.snapshot`'s `agents[]` carries only panes with a detected agent,
+  and an agent appearing in a pane produces no event Kelpie subscribes to.** A
+  pane split off with no agent in it never reaches `agents[]`, so it cannot
+  change the pane set or force a subscription rebuild. When an agent is then
+  detected there, no `pane.created` or `pane.updated` follows: Kelpie learns of
+  it only on the next refresh some other pane's status change triggers, or on
+  the 300 s resync. A newly started agent can therefore be missing from the
+  menu bar for minutes.
+- **A `session.snapshot` request in flight fails on essentially every
+  subscription rebuild.** Measured against 0.8.2: within about ten milliseconds
+  of the event connection closing, an outstanding request errors and then
+  succeeds on the first retry. This is why `SnapshotRetry` exists; the failure
+  was originally believed to be a one-off seen during a replay burst.
+- **A second `events.subscribe` on an open subscription connection makes herdr
+  close it.** The first is answered with `subscription_started`; the second
+  draws no error, just a disconnect. Subscriptions are fixed for a connection's
+  lifetime, which is why a changed pane set costs a whole new connection.
+
 - **Walking the process tree to find the hosting terminal must read
   `PROC_PIDT_SHORTBSDINFO`, not the full `PROC_PIDTBSDINFO`.** The full struct
   returns `EPERM` for the setuid-root `/usr/bin/login` that sits in a TUI

ROADMAP.md

@@ -36,10 +36,9 @@ left out rather than an oversight.
   "Start at login" toggle already in the popover footer. Adding a window for
   one boolean would be more surface than the feature deserves.
 
-- **Notifications suppressed across a subscription rebuild.** When the pane
-  set changes, the event connection is rebuilt through the reconnect path,
-  which resets `SessionState` and applies the next snapshot as `.bootstrap` —
-  so a pane that turned `blocked` inside that ~1 s window never notifies.
-  Accepted for now because pane lifecycle changes are rare and the window is
-  short; fixing it means carrying state across the rebuild, which reopens the
-  stale-notification questions the reset exists to avoid.
+- **The popover footer flashes "connecting" during a subscription rebuild.**
+  `runConnectionLoop` sets `.connecting` at the top of every iteration,
+  including the ~400 ms one that only replaces the event connection over a
+  changed pane set. The menu bar contents stay up, so only the footer moves.
+  Cosmetic, and the rebuild is now otherwise invisible — see
+  `ConnectionRestart`.

Sources/Kelpie/AppCoordinator.swift

@@ -89,10 +89,11 @@ final class AppCoordinator {
     // MARK: - Connection
 
     private func runConnectionLoop() async {
+        var restart = ConnectionRestart.fresh
         while !Task.isCancelled {
             model.connection = .connecting
             do {
-                try await connectOnce()
+                try await connectOnce(restart: restart)
                 backoff.reset()
                 Self.log.debug("connected; pumping events")
                 await pumpEvents()
@@ -102,11 +103,14 @@ final class AppCoordinator {
                 // failure — it just schedules the next attempt.
                 Self.log.debug("connect failed: \(String(describing: error), privacy: .public)")
             }
-            await teardown()
+            // The stream ending because Kelpie closed it over a pane set change
+            // is not a disconnection: herdr never went away, so the next
+            // connection continues this one rather than starting over.
+            restart = rebuildRequested ? .continuation : .fresh
+            await teardown(restart)
             if rebuildRequested {
-                // The stream ended because Kelpie closed it over a pane set
-                // change, not because herdr went away: reconnect immediately
-                // and leave the current menu bar contents up meanwhile.
+                // Reconnect immediately and leave the current menu bar
+                // contents up meanwhile.
                 rebuildRequested = false
                 continue
             }
@@ -141,7 +145,7 @@ final class AppCoordinator {
         }
     }
 
-    private func connectOnce() async throws {
+    private func connectOnce(restart: ConnectionRestart) async throws {
         let pong = try await request { try await $0.ping() }
 
         // Status changes only arrive through per-pane subscriptions (see
@@ -175,10 +179,13 @@ final class AppCoordinator {
         model.connection = pong.protocolVersion == Self.knownProtocol
             ? .connected
             : .protocolMismatch(pong.protocolVersion)
-        // The first snapshot of a connection describes state that already
-        // existed, so it is `.bootstrap` and must not notify. This is the only
-        // place that phase is produced; everything after it is `.live`.
-        applySnapshot(snapshot, phase: .bootstrap)
+        // A first sighting of herdr describes state that already existed, so
+        // it is `.bootstrap` and must not notify. A connection that merely
+        // replaces one Kelpie closed itself is `.live`: the pane set changed
+        // under a herdr that never went away, and a pane that turned `blocked`
+        // in between deserves its banner. `ConnectionRestart` states both
+        // rules; this is the only place either phase is produced.
+        applySnapshot(snapshot, phase: restart.phase)
         startResyncLoop()
     }
 
@@ -195,6 +202,12 @@ final class AppCoordinator {
             Task { [events] in await events?.close() }
         }
         let notifiable = NotificationPolicy.notifiable(transitions, phase: phase)
+        if !notifiable.isEmpty {
+            // The counterpart to `fireDueReminders`' breadcrumb. Without it
+            // there is no way to tell a build that decided to stay silent from
+            // one that tried to notify and was not allowed to.
+            Self.log.debug("notifying \(notifiable.count) pane(s) entering blocked")
+        }
         for transition in notifiable {
             NotificationManager.shared.postBlocked(
                 workspace: state.label(for: transition.workspaceID),
@@ -318,17 +331,23 @@ final class AppCoordinator {
         }
     }
 
-    private func teardown() async {
+    /// The timers are always torn down — the next snapshot reschedules every
+    /// one of them — but what Kelpie *knows* survives a continuation. Clearing
+    /// it there would leave the following snapshot with nothing to diff
+    /// against, which is what used to swallow a notification, and would drop
+    /// the pending reminders for panes that were blocked all along.
+    private func teardown(_ restart: ConnectionRestart) async {
         resyncTask?.cancel(); resyncTask = nil
         refreshTask?.cancel(); refreshTask = nil
         reminderTask?.cancel(); reminderTask = nil
         emphasisTask?.cancel(); emphasisTask = nil
-        reminder = BlockedReminder()
-        emphasis = BlockedEmphasis()
         coalescer = RefreshCoalescer()
         eventStream = nil
         await events?.close(); events = nil
         subscribedPaneIDs = []
+        guard !restart.carriesStateForward else { return }
+        reminder = BlockedReminder()
+        emphasis = BlockedEmphasis()
         state = SessionState()
     }
 

Sources/KelpieCore/AgentRecord.swift

@@ -40,7 +40,13 @@ public struct AgentRecord: Decodable, Equatable, Sendable {
         self.agentKind = agentKind
     }
 
-    /// Panes running a plain shell are reported too; Kelpie only shows panes
-    /// where herdr has detected an agent.
+    /// Kelpie only shows panes where herdr has detected an agent.
+    ///
+    /// Measured against 0.8.2, `session.snapshot`'s `agents[]` already
+    /// excludes plain shells — a pane split off with no agent in it never
+    /// appears — so this filter is currently redundant. It is kept because
+    /// `agent` is declared optional by herdr's own schema, which is the only
+    /// promise Kelpie has; a pane arriving with a null `agent` would otherwise
+    /// be tallied and, if it reported `blocked`, notified for.
     public var isAgentPane: Bool { agentKind != nil }
 }

Sources/KelpieCore/ConnectionRestart.swift

@@ -0,0 +1,41 @@
+import Foundation
+
+/// Why Kelpie is opening an event connection, and what that implies for the
+/// snapshot that follows.
+///
+/// The two cases look identical from the socket's side — connect, subscribe,
+/// snapshot — but they mean opposite things. A `.fresh` connection is a first
+/// sighting: herdr was not there, or has only just come back, so the state it
+/// reports already existed and notifying for it would fire every blocked agent
+/// at once. A `.continuation` is the same session seen through a new socket:
+/// Kelpie itself closed the previous one to change its subscriptions, herdr
+/// never went away, and the couple of hundred milliseconds in between are an
+/// implementation detail the user should not be able to detect.
+///
+/// Treating a continuation as a first sighting is what made a pane that turned
+/// `blocked` inside that window notify never — and, because the reminder state
+/// went with it, silenced the reminders for panes that were already blocked.
+public enum ConnectionRestart: Sendable, Equatable {
+    /// herdr was unreachable, or this is the first connection of the session.
+    case fresh
+    /// Kelpie closed the previous connection on purpose, to subscribe to a
+    /// changed set of panes.
+    case continuation
+
+    /// How the first snapshot of the new connection is applied.
+    public var phase: ApplyPhase {
+        switch self {
+        case .fresh: .bootstrap
+        case .continuation: .live
+        }
+    }
+
+    /// Whether `SessionState` — and with it the blocked reminders and the
+    /// menu bar's blocked emphasis — survives into the new connection. It has
+    /// to for a continuation: the transitions that notify are the difference
+    /// between what Kelpie knew and what the new snapshot says, and a cleared
+    /// state has no difference to offer.
+    public var carriesStateForward: Bool {
+        self == .continuation
+    }
+}

Sources/KelpieCore/SessionState.swift

@@ -20,10 +20,11 @@ public struct SessionState: Equatable, Sendable {
             uniqueKeysWithValues: snapshot.workspaces.map { ($0.workspaceID, $0.label) }
         )
         return snapshot.agents.compactMap { record in
-            // Only panes with a detected agent, matching `agentPanes`. herdr
-            // reports plain shells too, and one of those reporting `blocked`
-            // would otherwise post a notification for a row that appears
-            // nowhere in the counts or the popover.
+            // Only panes with a detected agent, matching `agentPanes`, so a
+            // pane reporting `blocked` without one cannot post a notification
+            // for a row that appears nowhere in the counts or the popover.
+            // See `AgentRecord.isAgentPane` for why this guard is kept even
+            // though 0.8.2 does not put such panes in `agents[]`.
             guard record.isAgentPane else { return nil }
             return transition(from: previous[record.paneID], to: record)
         }

Sources/KelpieCore/SnapshotRetry.swift

@@ -4,10 +4,12 @@ import Foundation
 ///
 /// Events say only *that* something changed, so a failed `session.snapshot`
 /// leaves the UI showing the previous state until the next event or the 300 s
-/// resync — observed once, transiently, while herdr was mid-replay. This hands
-/// out a short run of delays to retry over, then gives up: the failure seen in
-/// practice clears in well under a second, and the resync is the backstop for
-/// anything longer. Retrying forever would only compete with it.
+/// resync. That failure is not the rarity it first looked like: measured
+/// against a live 0.8.2, a snapshot request in flight fails within about ten
+/// milliseconds of *every* subscription rebuild, and recovers on the first
+/// retry. So this hands out a short run of delays, then gives up — the failure
+/// clears in well under a second, and the resync is the backstop for anything
+/// longer. Retrying forever would only compete with it.
 public struct SnapshotRetry: Sendable {
     private var remaining: [Duration]
 

Tests/KelpieCoreTests/ConnectionRestartTests.swift

@@ -0,0 +1,27 @@
+import Testing
+import Foundation
+@testable import KelpieCore
+
+@Suite("ConnectionRestart")
+struct ConnectionRestartTests {
+
+    @Test("A fresh connection bootstraps, so a launch with agents already blocked is silent")
+    func freshBootstraps() {
+        #expect(ConnectionRestart.fresh.phase == .bootstrap)
+    }
+
+    @Test("A continuation is live, so a pane that blocked during the rebuild still notifies")
+    func continuationIsLive() {
+        #expect(ConnectionRestart.continuation.phase == .live)
+    }
+
+    @Test("A fresh connection starts from nothing")
+    func freshDiscardsState() {
+        #expect(ConnectionRestart.fresh.carriesStateForward == false)
+    }
+
+    @Test("A continuation keeps what the previous connection knew")
+    func continuationKeepsState() {
+        #expect(ConnectionRestart.continuation.carriesStateForward)
+    }
+}