fix(workspace-client): unwatchGit/unwatchFs can strand a fresh, unretained connection

Found by a review of #6856's cleanup-effect ordering, but pre-existing —
affects fs:watch too, and predates both #6848 and #6856.

getEventBus(hostUrl) unconditionally (re)creates a ConnectionState if none
exists in the shared `connections` map. unwatchGit/unwatchFs/unwatchFsFile
never called maybeCleanupConnection, unlike on()'s and retain()'s cleanups
— so a caller that only ever intends to *release* interest (never establish
it) could, if the connection's last retainer/listener already tore it down
moments earlier, silently mint a brand-new WebSocket connection just to
send an unwatch command, then leave it dangling forever: nothing else would
ever call maybeCleanupConnection for it again.

This is a real, reachable ordering hazard in a multi-effect component like
DashboardSidebarWorkspaceStatusProvider: React runs effect cleanups in
declaration order (not reversed), so a "listener registration" effect's
cleanup releasing a connection's last retain/listener, followed by a
separate "release git-watch interest" effect's cleanup (or body, on a
mid-session workspace removal) calling unwatchGit for the same host, hits
this exactly.

Fix: unwatchGit/unwatchFs/unwatchFsFile now call maybeCleanupConnection
after releasing their own interest, mirroring on()'s and retain()'s
cleanups. Verified with a real WS server (no mocks): before the fix, a
release-only call after the connection's teardown opens a second real
upgrade that never closes; after the fix, maybeCleanupConnection fires
synchronously and fast enough that the stray connection never even dials
out — confirmed via a standalone diagnostic script before writing the
regression test, so the test's "no upgrade" assertion isn't guesswork.

Claude-Session: https://claude.ai/code/session_01NmLFihnhebmL9bbbojGYCR

Author: AviPeltz

packages/workspace-client/src/lib/eventBus.test.ts

@@ -148,4 +148,39 @@ describe("eventBus", () => {
 		await Bun.sleep(1_500);
 		expect(host.clientCount()).toBe(0);
 	});
+
+	it("releasing interest via a fresh handle, after the connection's last listener already closed it, does not strand a new connection", async () => {
+		const host = makeHostServer();
+		const bus = getEventBus(host.hostUrl, () => "tok");
+		const off = bus.on("git:changed", "*", () => {});
+		bus.watchGit("ws-1");
+		cleanups.push(() => host.server.stop(true));
+		await waitFor(() => host.clientCount() === 1);
+		expect(host.upgrades.length).toBe(1);
+
+		// Simulates a sibling effect's cleanup running first and releasing
+		// this connection's only listener/retainer — a real, order-dependent
+		// scenario in a multi-effect component (see DashboardSidebar-
+		// WorkspaceStatusProvider's final-unmount and mid-session-removal
+		// cleanup ordering).
+		off();
+		await waitFor(() => host.clientCount() === 0);
+
+		// A *fresh* getEventBus() call, exactly matching the real call site
+		// (getHostEventBus(hostUrl).unwatchGit(...) — never the same cached
+		// handle). getEventBus() unconditionally (re)creates a ConnectionState
+		// if none exists; before this fix, nothing ever closed the socket it
+		// opens back down (unwatchGit/unwatchFs never called
+		// maybeCleanupConnection), so it would dial in and reconnect forever
+		// (a second real upgrade hits the server, `host.upgrades.length`
+		// reaches 2, and `clientCount()` gets stuck at 1).
+		getEventBus(host.hostUrl, () => "tok").unwatchGit("ws-1");
+
+		// With the fix, maybeCleanupConnection closes the fresh connection
+		// synchronously — fast enough that it never even dials out. No
+		// second upgrade ever reaches the server, and no client lingers.
+		await Bun.sleep(1_500);
+		expect(host.upgrades.length).toBe(1);
+		expect(host.clientCount()).toBe(0);
+	});
 });

packages/workspace-client/src/lib/eventBus.ts

@@ -480,6 +480,14 @@ export function getEventBus(
 			if (count <= 1) {
 				state.fsWatchedWorkspaces.delete(workspaceId);
 				sendCommand(state, { type: "fs:unwatch", workspaceId });
+				// getEventBus() above always creates the connection if it didn't
+				// already exist — a caller that only ever intends to release
+				// interest (a cleanup effect running after this connection's
+				// last retainer already tore it down) would otherwise mint a
+				// fresh, unretained, unlistened-to connection here and leave it
+				// dangling forever, since nothing else will ever call this again
+				// for it. Mirrors on()'s and retain()'s cleanup.
+				maybeCleanupConnection(hostUrl);
 			} else {
 				state.fsWatchedWorkspaces.set(workspaceId, count - 1);
 			}
@@ -498,6 +506,9 @@ export function getEventBus(
 			if (count <= 1) {
 				state.gitWatchedWorkspaces.delete(workspaceId);
 				sendCommand(state, { type: "git:unwatch", workspaceId });
+				// See unwatchFs's comment: a release-only call can otherwise mint
+				// and permanently strand a fresh, never-retained connection.
+				maybeCleanupConnection(hostUrl);
 			} else {
 				state.gitWatchedWorkspaces.set(workspaceId, count - 1);
 			}
@@ -526,6 +537,9 @@ export function getEventBus(
 					workspaceId,
 					absolutePath,
 				});
+				// See unwatchFs's comment: a release-only call can otherwise mint
+				// and permanently strand a fresh, never-retained connection.
+				maybeCleanupConnection(hostUrl);
 			} else {
 				state.fsWatchedFiles.set(key, count - 1);
 			}