refactor(macapp): workroom-session becomes attach-only, so an update cannot strand a live terminal

TODOS.md

@@ -9,8 +9,49 @@
 
 ## P2 — perf, correctness, and the next VCS phase
 
+### Retire the attach-only `workroom-session` shim (macapp) — issue #154 follow-up
+
+**What:** Delete the remaining `macapp/WorkroomSession/` client, its pinned v2.0.0 binary fixture
+(`macapp/WorkroomAppTests/Fixtures/workroom-session-v2.0.0`, 464KB), `SessionShimCompatibilityTests`,
+and the freeze on `WorkroomSessionProtocol` — once no user can still be running a v2.0.0 daemon.
+
+**Why:** The shim is deliberately temporary. It exists only to reattach terminals held by a daemon
+process started by an app version older than the agent. That population shrinks with every reboot,
+every logout, and every closed pane, and eventually reaches zero — at which point the client, its
+fixture and its tests are pure carrying cost.
+
+**Pros:** Removes the last of two pty client implementations, drops a binary from git, and frees
+`WorkroomSessionProtocol` to change again (it is frozen today because its only real peer — a shipped
+v2.0.0 binary — can never be recompiled to match).
+
+**Cons:** Nothing signals when the population hits zero. It is a judgement call on elapsed time and
+release adoption, so this will sit open for a while. Deleting it early strands exactly the users it
+was added for.
+
+**How to start:** Check whether any reattach-from-old-daemon has been reported since the shim landed.
+Then delete the directory, the fixture, the compatibility tests, and the freeze comment together —
+they are one unit, and leaving the fixture behind without its tests is worse than leaving all of it.
+
+**Depends on / blocked by:** The shim landing first, then elapsed time plus release adoption. No hard
+gate.
+
+**Priority:** P2, not before 2027.
+
 ### `workroom-session` daemon: the findings the `/review` pass didn't fix (macapp) — persist-sessions follow-up
 
+> **RETIRED.** The daemon this section is about no longer exists in the tree: `SessionDaemon.swift`
+> and `SessionPTY.swift` were deleted when `workroom-session` became an attach-only client (issue
+> #154). The `SessionDaemonEndToEndTests.test*` methods cited below as red/green evidence went with
+> it, so those citations now point at git history rather than at runnable tests — read them as a
+> record of what was fixed, not as coverage that still runs. `git log -- macapp/WorkroomSession/`
+> has the originals.
+>
+> **The P0 architectural note below is NOT closed by that removal, and it would be wrong to read it
+> that way.** Deleting the daemon's source from this build does not stop the daemons already running
+> on users' machines: those were started by an app at or before v2.0.0, they keep their original
+> `LOCAL_PEERCRED`-only trust boundary, and reaching them is precisely what the retained attach
+> client is for. The note stands, unchanged, until the last such process is gone.
+
 **What:** The `workroom-session` daemon (persisted ordinary terminals across quit, `b28e9134`) went
 through a two-round `/review` (checklist + specialist dispatch, then a full Claude+Codex adversarial
 pass). The three highest-value findings from round two were fixed directly (fallback-socket path

macapp/WorkroomApp/Core/GhosttySurfaceView.swift

@@ -1,6 +1,7 @@
 import AppKit
 import Defaults
 import GhosttyKit
+import WorkroomSessionProtocol
 import os
 
 /// One terminal surface: an `NSView` that hosts a `ghostty_surface_t` (Metal-rendered by libghostty
@@ -70,6 +71,11 @@ final class GhosttySurfaceView: NSView {
   /// Daemon session this surface attaches to. When set and the helper is available, `createSurface`
   /// launches `workroom-session attach` instead of a login shell.
   var persistentSessionID: UUID?
+  /// Whether `persistentSessionID` was carried over from a previous launch rather than minted for
+  /// this pane. Only a restored id can name a session that has since died, and only a restored id
+  /// is re-checked before attaching — asking about a fresh one would refuse every new pane, since
+  /// no helper holds an id that has never existed.
+  var persistentSessionIsRestored = false
 
   /// Project / workroom / tab / title metadata sent on attach.
   var sessionMetadata: [(key: String, value: String)] = []
@@ -402,28 +408,66 @@ final class GhosttySurfaceView: NSView {
     envVarCount = 0
   }
 
-  /// Configure libghostty to spawn `workroom-session attach` for a background session.
+  /// Configure libghostty to spawn a session helper for a background session.
+  ///
+  /// Returns whether it set `config.command` — not strictly "a session was attached". The two
+  /// differ in one case: a session the daemon has already lost, where this sets a command that
+  /// tells the user so and then becomes their shell. The caller only uses the result to decide
+  /// whether to install a run command, and a pane with a persistent session is never a run-command
+  /// pane (`TerminalPersistentSessionPolicy` excludes them), so the distinction costs nothing.
   @discardableResult
   private func applyPersistentSession(
     to config: inout ghostty_surface_config_s,
     environment: inout [(String, String)]
   ) -> Bool {
     // No session was requested for this pane — not a failure, nothing to log.
     guard let persistentSessionID else { return false }
+
+    // Ask the daemon once more, before committing to an attach, whether it still holds this.
+    // The shipped daemon CREATES a session on attach for an id it does not hold, so a cached
+    // ownership answer that has outlived its session turns a reattach into a brand new shell with
+    // nothing to mark it as one. See `confirmBeforeAttach`.
+    if PersistentSessionService.shared.confirmBeforeAttach(
+      sessionID: persistentSessionID, wasRestored: persistentSessionIsRestored) == .gone,
+      let noticePointer = strdup(Self.lostSessionCommand())
+    {
+      Self.sessionLogger.error(
+        """
+        persistent session \(persistentSessionID.uuidString, privacy: .public) is gone; \
+        opening a shell with a notice rather than letting the daemon create a new session
+        """)
+      surfaceCStrings.append(noticePointer)
+      config.command = UnsafePointer(noticePointer)
+      config.wait_after_command = false
+      // This branch returns before `launchEnvironment`, so without this the notice pane would be a
+      // second-class terminal: no `GHOSTTY_RESOURCES_DIR` means no shell integration, which means
+      // no OSC 133 and no OSC 7 — the title latches to whatever the prompt last set and the footer
+      // never learns the directory. The agent's own fallback keeps them by routing through
+      // `shell::invocation`; losing a session should not also cost you a working title bar.
+      if let resources = GhosttyResources.bundledURL?.path {
+        environment.append(("GHOSTTY_RESOURCES_DIR", resources))
+      }
+      return true
+    }
+    // Deliberately NOT gated on `PersistentSessionService.isAvailable`, and
+    // `PersistentSessionAttachGateTests` fails if that is ever reintroduced. `isAvailable` answers
+    // for the backend a NEW session would go to, so it goes false whenever the agent is unhealthy
+    // — which is precisely when a session the retired Swift daemon still holds most needs reaching.
+    // Gating here rejected those panes before `attachCommand` could route them, defeating the
+    // attach-only client in its own use case. `attachCommand(forSession:)` already answers nil when
+    // the owning backend has no binary, so the global check bought nothing on this path.
     guard
-      PersistentSessionService.shared.isAvailable,
       let attach = PersistentSessionService.shared.attachCommand(
         forSession: persistentSessionID),
       let attachPointer = strdup(attach)
     else {
       // A pane that expected a persisted session fell back to a plain login shell. Whatever the
-      // daemon-side session was doing is now orphaned/unreachable from this pane — worth a log
+      // helper-side session was doing is now orphaned/unreachable from this pane — worth a log
       // line since the resulting terminal otherwise looks identical to a normal fresh shell.
       Self.sessionLogger.error(
         """
         persistent session \(persistentSessionID.uuidString, privacy: .public) unavailable \
-        (helper isAvailable=\(PersistentSessionService.shared.isAvailable, privacy: .public)); \
-        falling back to a plain shell
+        (no owning helper resolved); falling back to a plain shell
         """)
       return false
     }
@@ -438,6 +482,43 @@ final class GhosttySurfaceView: NSView {
     return true
   }
 
+  /// A command that says the previous session is gone, then becomes the user's shell.
+  ///
+  /// **Why a printed line and not silence.** Refusing the attach already prevents the wrong
+  /// outcome: the daemon does not get asked, so it cannot fork a replacement shell and pass it off
+  /// as the user's. But the pane still comes back showing a fresh prompt, which is exactly what a
+  /// successfully restored session looks like — and the thing that was running there is gone. A
+  /// terminal that quietly swaps a finished build for an empty prompt is the failure worth paying
+  /// a line of text to avoid.
+  ///
+  /// `exec` so the shell IS the pane's process: its exit status, title and signals behave the way
+  /// they would in any other terminal, rather than being wrapped by something the user did not ask
+  /// for. The agent's `fall_back_to_shell` shares that idea and little else — it builds its argv
+  /// through `shell::invocation`, strips `WORKROOM_SESSION_*` and sets a marker, none of which
+  /// apply here: this branch returns before `launchEnvironment`, so the pane never had those
+  /// variables to strip.
+  /// `SHELL`, or the default when it is unset OR EMPTY.
+  ///
+  /// `??` only defaults on absent, so `SHELL=""` produced `exec '' -l` — exit 127, and with
+  /// `wait_after_command` false that is the dead pane this whole function exists to prevent. The
+  /// Rust side has always filtered empties (`serve.rs`'s `from_vars`); these two are now in step.
+  static func resolvedShell(
+    environment: [String: String] = ProcessInfo.processInfo.environment
+  ) -> String {
+    guard let shell = environment["SHELL"], !shell.isEmpty else {
+      return SessionShellIntegration.defaultShell
+    }
+    return shell
+  }
+
+  static func lostSessionCommand(
+    shell: String = GhosttySurfaceView.resolvedShell()
+  ) -> String {
+    let notice = "The terminal that was running here has ended, so this is a new shell."
+    let script = "printf '%s\\n' \(shellQuoted(notice)); exec \(shellQuoted(shell)) -l"
+    return "/bin/sh -c \(shellQuoted(script))"
+  }
+
   func reattachPersistentSession() {
     guard persistentSessionID != nil, !isTornDown else { return }
     destroySurface()
@@ -1741,7 +1822,7 @@ extension GhosttySurfaceView {
 
   /// POSIX single-quote a path so spaces and shell metacharacters are taken literally by the shell
   /// that receives the inserted text.
-  private static func shellQuoted(_ path: String) -> String {
+  static func shellQuoted(_ path: String) -> String {
     "'" + path.replacingOccurrences(of: "'", with: "'\\''") + "'"
   }
 }

macapp/WorkroomApp/Core/Session/AgentControlClient.swift

@@ -21,6 +21,14 @@ import WorkroomSessionProtocol
 protocol SessionControlPlane {
   func list() -> [SessionDescriptor]
   func info(identifier: SessionIdentifier) -> SessionDescriptor?
+  /// Whether this helper holds the session, **distinguishing "it said no" from "it never
+  /// answered"** — which `info` cannot, because it returns nil for both.
+  ///
+  /// On the protocol rather than on one client because BOTH helpers create-on-attach: the daemon's
+  /// `handleAttach` ends in `create(request:connection:)` and the agent's is guarded only by
+  /// `sessions.contains(id)` (`serve.rs`). Asking only the daemon closed the substitution hole on
+  /// the backend being retired and left it open on the one that owns every new session.
+  func ownership(identifier: SessionIdentifier) -> SessionOwnership
   func kill(identifier: SessionIdentifier) -> Bool
   func killAll() -> Bool
 }
@@ -53,6 +61,19 @@ struct AgentControlClient: SessionControlPlane {
     list().first { $0.identifier == identifier }
   }
 
+  /// `list()` collapses "no reply" into an empty array, which is why this cannot be written on top
+  /// of it: an agent that is wedged and an agent holding nothing would be indistinguishable, and
+  /// answering `.notOwned` for the first would throw away a live session. Going through `transact`
+  /// directly keeps nil meaning "nothing readable came back".
+  func ownership(identifier: SessionIdentifier) -> SessionOwnership {
+    let sessions = transact(SessionFrame(kind: .list)) { frame -> [SessionDescriptor]? in
+      guard frame.kind == .sessions else { return nil }
+      return try? SessionDescriptor.decodeList(frame.payload)
+    }
+    guard let sessions else { return .unreachable }
+    return sessions.contains { $0.identifier == identifier } ? .owned : .notOwned
+  }
+
   func kill(identifier: SessionIdentifier) -> Bool {
     transact(
       SessionFrame(kind: .kill, payload: SessionIdentifierPayload.encode(identifier))