fix(relay): let one owner hold pty.ackData instead of relying on construction order (#15079)

PtyHandler registered a no-op pty.ackData handler and SshPtyConsumerSessionAdapter
registered the real sourceCredit.acknowledge for the same method. onNotification is a
single slot, so only the adapter's survived — and only because relay.ts constructs it
second. Reversing those two lines would have made every credit-mode delivery wedge
permanently once the 256KB window emptied, with no error and no log.

Delete the dead no-op, and make onNotification throw on a duplicate registration the way
registerPtyDataPublicationAdmission already does for the admission slot. pty.ackData was
the only double-registered method in the tree, so nothing else changes behavior.

The existing test asserted only that pty.ackData appeared in the handler map, which stays
true when the real handler is shadowed. It now asserts PtyHandler does not own the method.

src/relay/dispatcher-notification-ownership.test.ts

@@ -0,0 +1,18 @@
+import { describe, expect, it, vi } from 'vitest'
+import { RelayDispatcher } from './dispatcher'
+
+describe('RelayDispatcher notification ownership', () => {
+  it('refuses a second handler for a method instead of silently shadowing the first', () => {
+    const dispatcher = new RelayDispatcher(() => {})
+    const owner = vi.fn()
+
+    dispatcher.onNotification('pty.ackData', owner)
+
+    // Why: the slot holds one handler, so a second registration used to win purely by
+    // construction order — that is how a no-op ack handler nearly disabled credit acks.
+    expect(() => dispatcher.onNotification('pty.ackData', vi.fn())).toThrow(/already registered/)
+
+    // Why: the constructor arms a keepalive interval that would outlive the test.
+    dispatcher.dispose()
+  })
+})

src/relay/dispatcher.ts

@@ -212,7 +212,14 @@ export class RelayDispatcher {
     this.requestHandlers.set(method, handler)
   }
 
+  // Why it throws: this is a single slot, so a second registration silently shadows the
+  // first and which one survives depends only on construction order. `pty.ackData` shipped
+  // that way — a no-op handler was saved from disabling credit acks purely by the adapter
+  // being constructed second (STA-4571). Fail loudly instead of encoding that ordering.
   onNotification(method: string, handler: NotificationHandler): void {
+    if (this.notificationHandlers.has(method)) {
+      throw new Error(`Notification handler for ${method} is already registered`)
+    }
     this.notificationHandlers.set(method, handler)
   }
 

src/relay/pty-handler-spawn-admission.test.ts

@@ -79,7 +79,10 @@ describe('PtyHandler', () => {
     const notifMethods = Array.from(dispatcher._notificationHandlers.keys())
     expect(notifMethods).toContain('pty.data')
     expect(notifMethods).toContain('pty.resize')
-    expect(notifMethods).toContain('pty.ackData')
+    // Why not `toContain('pty.ackData')`: PtyHandler must NOT own that method. It used to
+    // register a no-op here, which survived only because the consumer session adapter was
+    // constructed later and overwrote it (STA-4571).
+    expect(notifMethods).not.toContain('pty.ackData')
   })
 
   it('rejects strict process inspection for a missing relay PTY', async () => {

src/relay/pty-handler.ts

@@ -903,9 +903,6 @@ export class PtyHandler {
 
     this.dispatcher.onNotification('pty.data', (p) => this.writeData(p))
     this.dispatcher.onNotification('pty.resize', (p) => this.resize(p))
-    this.dispatcher.onNotification('pty.ackData', (_p) => {
-      /* flow control ack -- not yet enforced */
-    })
   }
 
   private isLikelyInteractiveRedraw(data: string): boolean {