Skip to content

Commit 68b18cd

Browse files
authored
fix(extension): coalesce daemon websocket connects (#1554)
1 parent cddc847 commit 68b18cd

3 files changed

Lines changed: 69 additions & 4 deletions

File tree

extension/dist/background.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,7 @@ let reconnectAttempts = 0;
626626
const CONTEXT_ID_KEY = "opencli_context_id_v1";
627627
let currentContextId = "default";
628628
let contextIdPromise = null;
629+
let connectInFlight = null;
629630
async function getCurrentContextId() {
630631
if (contextIdPromise) return contextIdPromise;
631632
contextIdPromise = (async () => {
@@ -698,17 +699,30 @@ console.error = (...args) => {
698699
_origError(...args);
699700
forwardLog("error", args);
700701
};
701-
async function connect() {
702-
if (ws?.readyState === WebSocket.OPEN || ws?.readyState === WebSocket.CONNECTING) return;
702+
function isDaemonSocketActive(socket = ws) {
703+
return socket?.readyState === WebSocket.OPEN || socket?.readyState === WebSocket.CONNECTING;
704+
}
705+
function connect() {
706+
if (isDaemonSocketActive()) return Promise.resolve();
707+
if (connectInFlight) return connectInFlight;
708+
connectInFlight = connectAttempt().finally(() => {
709+
connectInFlight = null;
710+
});
711+
return connectInFlight;
712+
}
713+
async function connectAttempt() {
714+
if (isDaemonSocketActive()) return;
703715
try {
704716
const res = await fetch(DAEMON_PING_URL, { signal: AbortSignal.timeout(1e3) });
705717
if (!res.ok) return;
706718
} catch {
707719
return;
708720
}
721+
if (isDaemonSocketActive()) return;
709722
let thisWs;
710723
try {
711724
const contextId = await getCurrentContextId();
725+
if (isDaemonSocketActive()) return;
712726
thisWs = new WebSocket(DAEMON_WS_URL);
713727
ws = thisWs;
714728
currentContextId = contextId;

extension/src/background.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,16 @@ class MockWebSocket {
5252
}
5353
}
5454

55+
function deferred<T>() {
56+
let resolve!: (value: T | PromiseLike<T>) => void;
57+
let reject!: (reason?: unknown) => void;
58+
const promise = new Promise<T>((res, rej) => {
59+
resolve = res;
60+
reject = rej;
61+
});
62+
return { promise, resolve, reject };
63+
}
64+
5565
function createChromeMock() {
5666
let nextTabId = 10;
5767
let nextGroupId = 100;
@@ -694,6 +704,31 @@ describe('background tab isolation', () => {
694704
});
695705
});
696706

707+
it('coalesces concurrent daemon connection attempts while the probe is in flight', async () => {
708+
const { chrome } = createChromeMock();
709+
vi.stubGlobal('chrome', chrome);
710+
const ping = deferred<{ ok: boolean }>();
711+
const fetchMock = vi.fn(() => ping.promise);
712+
vi.stubGlobal('fetch', fetchMock);
713+
714+
await import('./background');
715+
await vi.waitFor(() => {
716+
expect(fetchMock).toHaveBeenCalledTimes(1);
717+
});
718+
719+
const onAlarmListener = chrome.alarms.onAlarm.addListener.mock.calls[0][0];
720+
await onAlarmListener({ name: 'keepalive' });
721+
await onAlarmListener({ name: 'keepalive' });
722+
723+
expect(fetchMock).toHaveBeenCalledTimes(1);
724+
expect(MockWebSocket.instances).toHaveLength(0);
725+
726+
ping.resolve({ ok: true });
727+
await vi.waitFor(() => {
728+
expect(MockWebSocket.instances).toHaveLength(1);
729+
});
730+
});
731+
697732
it('ignores daemon commands delivered to a superseded WebSocket', async () => {
698733
const { chrome } = createChromeMock();
699734
vi.stubGlobal('chrome', chrome);

extension/src/background.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ let reconnectAttempts = 0;
1818
const CONTEXT_ID_KEY = 'opencli_context_id_v1';
1919
let currentContextId = 'default';
2020
let contextIdPromise: Promise<string> | null = null;
21+
let connectInFlight: Promise<void> | null = null;
2122

2223
async function getCurrentContextId(): Promise<string> {
2324
if (contextIdPromise) return contextIdPromise;
@@ -92,26 +93,41 @@ console.error = (...args: unknown[]) => { _origError(...args); forwardLog('error
9293

9394
// ─── WebSocket connection ────────────────────────────────────────────
9495

96+
function isDaemonSocketActive(socket: WebSocket | null | undefined = ws): boolean {
97+
return socket?.readyState === WebSocket.OPEN || socket?.readyState === WebSocket.CONNECTING;
98+
}
99+
95100
/**
96101
* Probe the daemon via its /ping HTTP endpoint before attempting a WebSocket
97102
* connection. fetch() failures are silently catchable; new WebSocket() is not
98103
* — Chrome logs ERR_CONNECTION_REFUSED to the extension error page before any
99104
* JS handler can intercept it. By keeping the probe inside connect() every
100105
* call site remains unchanged and the guard can never be accidentally skipped.
101106
*/
102-
async function connect(): Promise<void> {
103-
if (ws?.readyState === WebSocket.OPEN || ws?.readyState === WebSocket.CONNECTING) return;
107+
function connect(): Promise<void> {
108+
if (isDaemonSocketActive()) return Promise.resolve();
109+
if (connectInFlight) return connectInFlight;
110+
connectInFlight = connectAttempt().finally(() => {
111+
connectInFlight = null;
112+
});
113+
return connectInFlight;
114+
}
115+
116+
async function connectAttempt(): Promise<void> {
117+
if (isDaemonSocketActive()) return;
104118

105119
try {
106120
const res = await fetch(DAEMON_PING_URL, { signal: AbortSignal.timeout(1000) });
107121
if (!res.ok) return; // unexpected response — not our daemon
108122
} catch {
109123
return; // daemon not running — skip WebSocket to avoid console noise
110124
}
125+
if (isDaemonSocketActive()) return;
111126

112127
let thisWs: WebSocket;
113128
try {
114129
const contextId = await getCurrentContextId();
130+
if (isDaemonSocketActive()) return;
115131
thisWs = new WebSocket(DAEMON_WS_URL);
116132
ws = thisWs;
117133
currentContextId = contextId;

0 commit comments

Comments
 (0)