From 41d68784a6e0f51c37f7dbed926116b67815e21d Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 27 Jan 2026 21:23:07 +0530 Subject: [PATCH] fix: resolve window ownership race condition with async polling - Add waitForWindowOwnership() method that polls for up to 500ms - Update sendRequest() to wait for window registration before routing - Remove hacky 1-second delay from scheduledJobRuns.ts - Improves multi-window and multi-profile routing reliability Fixes #277 --- .../background/scheduledJobRuns.ts | 7 +-- apps/server/src/browser/extension/bridge.ts | 45 ++++++++++++++++--- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/apps/agent/entrypoints/background/scheduledJobRuns.ts b/apps/agent/entrypoints/background/scheduledJobRuns.ts index 3a19740b..684db6fb 100644 --- a/apps/agent/entrypoints/background/scheduledJobRuns.ts +++ b/apps/agent/entrypoints/background/scheduledJobRuns.ts @@ -113,11 +113,8 @@ export const scheduledJobRuns = async () => { type: 'normal', }) - // FIXME: Race condition - the controller-ext extension sends a window_created - // WebSocket message to register window ownership, but our HTTP request may arrive - // at the server before that registration completes. This delay is a temporary fix. - // Proper solution: ControllerBridge should wait/poll for window ownership registration. - await new Promise((resolve) => setTimeout(resolve, 1000)) + // Note: Window ownership race condition is now handled by ControllerBridge.sendRequest() + // which polls for up to 500ms waiting for window registration before falling back. const backgroundTab = backgroundWindow?.tabs?.[0] diff --git a/apps/server/src/browser/extension/bridge.ts b/apps/server/src/browser/extension/bridge.ts index 513508e8..5c5330d7 100644 --- a/apps/server/src/browser/extension/bridge.ts +++ b/apps/server/src/browser/extension/bridge.ts @@ -127,6 +127,34 @@ export class ControllerBridge { return this.primaryClientId !== null } + /** + * Waits for window ownership registration with polling. + * This resolves the race condition where HTTP requests arrive before + * the window_created WebSocket message is processed. + * + * @param windowId - The window ID to wait for + * @param timeoutMs - Maximum time to wait (default 500ms) + * @param pollIntervalMs - Polling interval (default 50ms) + * @returns The owner clientId if found, null if timeout + */ + private async waitForWindowOwnership( + windowId: number, + timeoutMs: number = 500, + pollIntervalMs: number = 50, + ): Promise { + const startTime = Date.now() + + while (Date.now() - startTime < timeoutMs) { + const ownerClientId = this.windowOwnership.get(windowId) + if (ownerClientId && this.clients.has(ownerClientId)) { + return ownerClientId + } + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)) + } + + return null + } + async sendRequest( action: string, payload: unknown, @@ -140,14 +168,17 @@ export class ControllerBridge { const payloadObj = payload as Record | null const windowId = payloadObj?.windowId as number | undefined - // FIXME: Race condition - when a new window is created, the window_created - // WebSocket message may not be processed before requests arrive for that window. - // This causes fallback to primaryClientId. For single-profile setups this works, - // but breaks multi-profile routing. Proper fix: poll/wait for window ownership - // registration here (e.g., retry for up to 500ms before falling back). let targetClientId = this.primaryClientId if (windowId !== undefined) { - const ownerClientId = this.windowOwnership.get(windowId) + // First check if already registered + let ownerClientId = this.windowOwnership.get(windowId) + + // If not registered, wait for registration (fixes race condition) + if (!ownerClientId || !this.clients.has(ownerClientId)) { + this.logger.debug('Window not yet registered, waiting...', { windowId }) + ownerClientId = await this.waitForWindowOwnership(windowId) + } + if (ownerClientId && this.clients.has(ownerClientId)) { targetClientId = ownerClientId this.logger.debug('Routing request by windowId', { @@ -155,7 +186,7 @@ export class ControllerBridge { targetClientId, }) } else { - this.logger.warn('No owner found for windowId, using primary', { + this.logger.warn('Window ownership timeout, using primary', { windowId, primaryClientId: this.primaryClientId, })