fix: cleanup protocol listeners (#33391)

Author: mschile

cli/CHANGELOG.md

@@ -13,6 +13,7 @@ _Released 02/17/2026 (PENDING)_
 
 - Fixed an issue where a cancelled or incomplete login attempt would not properly open a browser window or tab, and required a restart of Cypress to enable a new login attempt. Fixed in [#33366](https://github.com/cypress-io/cypress/pull/33366). Fixes [#33350](https://github.com/cypress-io/cypress/issues/33350).
 - Fixed an issue on Windows where extracting the Studio or Prompt bundle could fail with `EPERM: operation not permitted` when renaming extracted files. The extract step now retries on EPERM/EACCES with a short delay to handle transient file locks. Addressed in [#33330](https://github.com/cypress-io/cypress/pull/33330).
+- The capture protocol is now properly cleaned up when the protocol is re-initialized or when the run closes, ensuring CDP client listeners and resources are removed. Addressed in [#33391](https://github.com/cypress-io/cypress/pull/33391).
 
 **Misc:**
 

packages/server/lib/cloud/protocol.ts

@@ -102,6 +102,9 @@ export class ProtocolManager implements ProtocolManagerShape {
     debug('setting up protocol')
 
     try {
+      // Cleanup the previous protocol
+      this.cleanup()
+
       if (!this.AppCaptureProtocol || !this.options) {
         throw new Error('Cannot setup protocol without a prepared protocol')
       }
@@ -126,11 +129,15 @@ export class ProtocolManager implements ProtocolManagerShape {
   }
 
   async connectToBrowser (cdpClient: CDPClient) {
-    // Wrap the cdp client listeners so that we can be notified of any errors that may occur
+    // Keyed by event name then by original listener so that the same function
+    // registered for multiple events doesn't collide and leak wrappers.
+    const listenerMap = new Map<string, Map<Function, Function>>()
+
     const newCdpClient: CDPClient = {
       ...cdpClient,
       on: (event, listener) => {
-        cdpClient.on(event, async (message) => {
+        // Wrap the cdp client listeners so that we can be notified of any errors that may occur
+        const wrapper = async (message) => {
           try {
             await listener(message)
           } catch (error) {
@@ -141,7 +148,26 @@ export class ProtocolManager implements ProtocolManagerShape {
               throw error
             }
           }
-        })
+        }
+
+        if (!listenerMap.has(event)) {
+          listenerMap.set(event, new Map())
+        }
+
+        listenerMap.get(event)!.set(listener, wrapper)
+        cdpClient.on(event, wrapper)
+      },
+      off: (event, listener) => {
+        const eventListeners = listenerMap.get(event)
+        const wrapper = eventListeners?.get(listener)
+
+        if (wrapper) {
+          cdpClient.off(event, wrapper as any)
+          eventListeners!.delete(listener)
+          if (eventListeners!.size === 0) {
+            listenerMap.delete(event)
+          }
+        }
       },
     }
 
@@ -166,7 +192,7 @@ export class ProtocolManager implements ProtocolManagerShape {
       this._beforeSpec(spec)
     } catch (error) {
       // Clear out protocol since we will not have a valid state when spec has failed
-      this._protocol = undefined
+      this.cleanup()
 
       if (CAPTURE_ERRORS) {
         this.captureError({ captureMethod: 'beforeSpec', fatal: true, error, args: [spec], runnableId: this._runnableId })
@@ -491,6 +517,11 @@ export class ProtocolManager implements ProtocolManagerShape {
     this._specName = undefined
     this._runId = undefined
     this._errors = []
+    this.cleanup()
+  }
+
+  cleanup (): void {
+    this.invokeSync('cleanup', { isEssential: false })
     this._protocol = undefined
   }
 

packages/server/test/support/fixtures/cloud/protocol/test-protocol.ts

@@ -42,4 +42,5 @@ export class AppCaptureProtocol implements AppCaptureProtocolInterface {
 
   pageLoading (input: any): void {}
   resetTest (testId: string): void {}
+  cleanup (): void {}
 }

packages/server/test/unit/cloud/protocol_spec.ts

@@ -9,7 +9,7 @@ import fs from 'fs-extra'
 import type { SinonStub } from 'sinon'
 
 class TestClient extends EventEmitter {
-  send: sinon.SinonStub = sinon.stub()
+  send: SinonStub = sinon.stub()
 }
 
 const mockDb = sinon.stub()
@@ -93,6 +93,86 @@ describe('lib/cloud/protocol', () => {
     ])
   })
 
+  it('should unregister listener when off() is called on wrapped CDP client', async () => {
+    const mockCdpClient = new TestClient()
+
+    sinon.stub(protocol, 'connectToBrowser').resolves()
+
+    await protocolManager.connectToBrowser(mockCdpClient as any)
+
+    const newCdpClient = (protocol.connectToBrowser as SinonStub).getCall(0).args[0]
+    const listener = sinon.stub()
+
+    newCdpClient.on('Page.loadEventFired', listener)
+    mockCdpClient.emit('Page.loadEventFired')
+    expect(listener).to.have.been.calledOnce
+
+    newCdpClient.off('Page.loadEventFired', listener)
+    mockCdpClient.emit('Page.loadEventFired')
+    expect(listener).to.have.been.calledOnce
+  })
+
+  it('uses event+listener composite key so same listener on multiple events does not leak wrappers', async () => {
+    const mockCdpClient = new TestClient()
+    const onCalls: Array<{ event: string, listener: Function }> = []
+    const offCalls: Array<{ event: string, listener: Function }> = []
+
+    const originalOn = mockCdpClient.on.bind(mockCdpClient)
+    const originalOff = mockCdpClient.off.bind(mockCdpClient)
+
+    mockCdpClient.on = function (event: string, listener: Function) {
+      onCalls.push({ event, listener })
+
+      return originalOn(event, listener)
+    }
+
+    mockCdpClient.off = function (event: string, listener: Function) {
+      offCalls.push({ event, listener })
+
+      return originalOff(event, listener)
+    }
+
+    let capturedWrappedClient: any
+
+    sinon.stub(protocolManager as any, 'invokeAsync').callsFake(async (_method: string, _opts: any, cdpClient: any) => {
+      capturedWrappedClient = cdpClient
+    })
+
+    await protocolManager.connectToBrowser(mockCdpClient as any)
+
+    expect(capturedWrappedClient).to.exist
+
+    const sharedListener = sinon.stub()
+
+    capturedWrappedClient.on('Page.frameAttached', sharedListener)
+    capturedWrappedClient.on('Page.frameDetached', sharedListener)
+
+    expect(onCalls).to.have.length(2)
+
+    const wrapperForAttached = onCalls.find((c) => c.event === 'Page.frameAttached')!.listener
+    const wrapperForDetached = onCalls.find((c) => c.event === 'Page.frameDetached')!.listener
+
+    expect(wrapperForAttached).to.not.equal(wrapperForDetached)
+
+    capturedWrappedClient.off('Page.frameAttached', sharedListener)
+    expect(offCalls).to.have.length(1)
+    expect(offCalls[0].event).to.equal('Page.frameAttached')
+    expect(offCalls[0].listener).to.equal(wrapperForAttached)
+
+    capturedWrappedClient.off('Page.frameDetached', sharedListener)
+    expect(offCalls).to.have.length(2)
+    expect(offCalls[1].event).to.equal('Page.frameDetached')
+    expect(offCalls[1].listener).to.equal(wrapperForDetached)
+  })
+
+  it('should call cleanup on existing protocol when setupProtocol is called again', () => {
+    const cleanupStub = sinon.stub(protocol, 'cleanup')
+
+    protocolManager.setupProtocol()
+
+    expect(cleanupStub).to.have.been.calledOnce
+  })
+
   it('should be able to initialize a new spec', () => {
     sinon.stub(protocol, 'beforeSpec')
 
@@ -364,6 +444,20 @@ describe('lib/cloud/protocol', () => {
       expect(protocolManager['_errors']).to.be.empty
       expect(protocolManager['_protocol']).to.be.undefined
     })
+
+    it('calls cleanup on protocol before clearing it', () => {
+      const cleanupStub = sinon.stub(protocol, 'cleanup')
+
+      protocolManager['_db'] = { close: sinon.stub() }
+      protocolManager['_dbPath'] = '/path/to/db'
+      protocolManager['_archivePath'] = '/path/to/archive'
+      sinon.stub(fs, 'unlink').resolves()
+
+      protocolManager.close()
+
+      expect(cleanupStub).to.have.been.calledOnce
+      expect(protocolManager['_protocol']).to.be.undefined
+    })
   })
 
   describe('.dbPath', () => {

packages/types/src/protocol.ts

@@ -17,8 +17,6 @@ export interface CDPClient {
   off (eventName: string, cb: (event: any) => void): void
 }
 
-// TODO(protocol): This is basic for now but will evolve as we progress with the protocol work
-
 export interface AppCaptureProtocolCommon {
   cdpReconnect (): Promise<void>
   addRunnables (runnables: any): void
@@ -42,6 +40,7 @@ export interface AppCaptureProtocolInterface extends AppCaptureProtocolCommon {
   beforeSpec ({ spec, workingDirectory, archivePath, dbPath, db }: { spec: FoundSpec & { instanceId: string }, workingDirectory: string, archivePath: string, dbPath: string, db: Database.Database }): void
   uploadStallSamplingInterval: () => number
   connectToBrowser (cdpClient: CDPClient): Promise<void>
+  cleanup (): void
 }
 
 export type ProtocolCaptureMethod = keyof AppCaptureProtocolInterface | 'setupProtocol' | 'prepareProtocol' | 'uploadCaptureArtifact' | 'getCaptureProtocolScript' | 'cdpClient.on' | 'getZippedDb' | 'UNKNOWN' | 'createProtocolArtifact' | 'protocolUploadUrl'