From 1fa6fac17c11dd3637823ee9580e321e5ac09952 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Thu, 17 Sep 2026 23:21:39 -0700
Subject: [PATCH] fix(daemon): answer the per-pty snapshot predicate for the
 pty it was asked about (#21381)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

canProvideAuthoritativeBufferSnapshot is contracted as "whether this exact PTY can
return a sequence-safe provider snapshot" (pty-provider-contract.ts), and two of the
three layers already route it per id: DaemonPtyRouter forwards to adapterFor(id), and
DegradedDaemonPtyProvider forwards to the provider that owns the session. The daemon
adapter was the leaf that discarded the id and returned supportsAuthoritativeBufferSnapshots
— a negotiated protocol version, which is a fact about the connection, not about a pty.

That is reachable, not theoretical. getProviderForPty falls back to the local provider
for any id it cannot place, so a remote-runtime id (whose pty lives on another machine)
resolves to the local daemon adapter, and pty:getAuthoritativeBufferSnapshotCapabilities
answered `true` for a session this daemon has never owned. The renderer caches that as a
definitive per-pty verdict, and because the leaf discarded the id it could not tell it had
been asked about something it does not own.

Today the wrong answer is masked: allowOrdinaryParkRestore short-circuits remote and SSH
ptys before the cached verdict is read, so nothing consults it. This closes the gap before
something relies on it — a caller reaching for a per-pty answer should not be handed a
confident one that is wrong.

Not touching that short-circuit. It is deliberate: SSH bytes transit the client's own main
process into its headless mirror, so those panes have a local copy the predicate says
nothing about, and the direct-SSH lane was confirmed to repaint from a daemon-backed
restore with the park capture disabled entirely. Routing SSH around a daemon-snapshot
predicate is correct, and removing the short-circuit would disable SSH parking for no
correctness gain.

The existing protocol-compatibility test asserted `true` for a made-up session id, which
encoded the bug. It now spawns a real session, so it still proves the protocol-version
gate without depending on an unowned id reading as supported.
---
 ...pty-adapter-protocol-compatibility.test.ts | 19 ++++++++++++++++---
 src/main/daemon/daemon-pty-runtime-state.ts   |  8 ++++++--
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/main/daemon/daemon-pty-adapter-protocol-compatibility.test.ts b/src/main/daemon/daemon-pty-adapter-protocol-compatibility.test.ts
index 8132f2de4e2..3fc255b60ec 100644
--- a/src/main/daemon/daemon-pty-adapter-protocol-compatibility.test.ts
+++ b/src/main/daemon/daemon-pty-adapter-protocol-compatibility.test.ts
@@ -243,16 +243,29 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
   })
 
   describe('background stream thinning compatibility', () => {
-    it('reports authoritative snapshot support only for the corrected serializer protocol', () => {
+    it('reports authoritative snapshot support only for the corrected serializer protocol', async () => {
+      const { id } = await adapter.spawn({ cols: 80, rows: 24 })
       const legacy = new DaemonPtyAdapter({ socketPath, tokenPath, protocolVersion: 31 })
       try {
-        expect(legacy.canProvideAuthoritativeBufferSnapshot('legacy-session')).toBe(false)
-        expect(adapter.canProvideAuthoritativeBufferSnapshot('current-session')).toBe(true)
+        expect(legacy.canProvideAuthoritativeBufferSnapshot(id)).toBe(false)
+        expect(adapter.canProvideAuthoritativeBufferSnapshot(id)).toBe(true)
       } finally {
         legacy.dispose()
       }
     })
 
+    // Why this matters beyond tidiness: getProviderForPty falls back to the local provider for
+    // any id it cannot place, so these ids reach this adapter for real. Answered from the
+    // protocol flag alone they came back `true`, which the renderer caches as a definitive
+    // per-pty licence to unmount a pane whose bytes this daemon never held.
+    it('refuses an authoritative snapshot claim for a session it does not own', async () => {
+      const { id } = await adapter.spawn({ cols: 80, rows: 24 })
+
+      expect(adapter.canProvideAuthoritativeBufferSnapshot(id)).toBe(true)
+      expect(adapter.canProvideAuthoritativeBufferSnapshot('remote:env-1:pty-1')).toBe(false)
+      expect(adapter.canProvideAuthoritativeBufferSnapshot('never-spawned-session')).toBe(false)
+    })
+
     it('reports background state on the authoritative-snapshot protocol', () => {
       const notifySpy = vi.spyOn(DaemonClient.prototype, 'notify')
       try {
diff --git a/src/main/daemon/daemon-pty-runtime-state.ts b/src/main/daemon/daemon-pty-runtime-state.ts
index e471f698748..38480af1489 100644
--- a/src/main/daemon/daemon-pty-runtime-state.ts
+++ b/src/main/daemon/daemon-pty-runtime-state.ts
@@ -237,8 +237,12 @@ export abstract class DaemonPtyRuntimeState {
     return this.protocolVersion >= GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION
   }
 
-  canProvideAuthoritativeBufferSnapshot(_id: string): boolean {
-    return this.supportsAuthoritativeBufferSnapshots
+  // Why the id is read rather than ignored: the contract promises a fact about THIS pty, and
+  // getProviderForPty falls back to the local provider for any id it cannot place. A
+  // remote-runtime id therefore reaches this adapter, and answering from the protocol flag
+  // alone returned `true` for a session this daemon has never owned.
+  canProvideAuthoritativeBufferSnapshot(id: string): boolean {
+    return this.supportsAuthoritativeBufferSnapshots && this.activeSessionIds.has(id)
   }
 
   protected get canDelegateBackgroundToDaemon(): boolean {