Skip to content

Conversation

@Suhaib3100
Copy link

Fixes #277

Resolves a critical race condition where HTTP requests arrive before window_created WebSocket message is processed.

Changes

  1. Added waitForWindowOwnership() method - polls for up to 500ms for window registration
  2. Updated sendRequest() to wait for window registration before routing
  3. Removed hacky 1-second delay from scheduledJobRuns.ts

Benefits

  • Proper multi-window support
  • Multi-profile routing works correctly
  • Better UX (no arbitrary delays)
  • More reliable scheduled job execution

- 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 browseros-ai#277
@github-actions
Copy link
Contributor

Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement.

To sign the CLA, please add a comment to this PR with the following text:

I have read the CLA Document and I hereby sign the CLA

You only need to sign once. After signing, this check will pass automatically.


Troubleshooting
  • Already signed but still failing? Comment recheck to trigger a re-verification.
  • Signed with a different email? Make sure your commit email matches your GitHub account email, or add your commit email to your GitHub account.
- - - I have read the CLA Document and I hereby sign the CLA - - - You can retrigger this bot by commenting **recheck** in this Pull Request. Posted by the **CLA Assistant Lite bot**.

@Suhaib3100
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 27, 2026

Greptile Overview

Greptile Summary

This PR resolves a critical race condition in multi-window/multi-profile routing by implementing async polling in ControllerBridge.sendRequest().

Key Changes:

  • Added waitForWindowOwnership() method that polls for up to 500ms waiting for window registration
  • Updated sendRequest() to wait for window ownership before routing requests (fixes race where HTTP requests arrive before window_created WebSocket message)
  • Removed 1-second arbitrary delay from scheduledJobRuns.ts (no longer needed with proper polling)

Impact:

  • Fixes multi-profile routing reliability
  • Improves UX by eliminating arbitrary 1-second delays
  • Maintains backward compatibility with fallback to primaryClientId on timeout

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk - it fixes a real race condition with a proper solution
  • The implementation is sound: polling with timeout (500ms) is the appropriate solution for this race condition, the fallback to primaryClientId preserves existing behavior, and the removal of the 1-second delay improves UX. No security issues, type errors, or logic bugs detected.
  • No files require special attention

Important Files Changed

Filename Overview
apps/server/src/browser/extension/bridge.ts Added waitForWindowOwnership() polling mechanism to resolve race condition; updated sendRequest() to wait for window registration before routing requests
apps/agent/entrypoints/background/scheduledJobRuns.ts Removed 1-second delay workaround; now relies on ControllerBridge polling for proper window registration

Sequence Diagram

sequenceDiagram
    participant Agent as Agent Extension
    participant Bridge as ControllerBridge
    participant Ext as Controller Extension
    
    Note over Agent,Ext: Race Condition Scenario
    
    Agent->>Agent: chrome.windows.create()
    Agent->>Bridge: HTTP Request with windowId
    
    Note over Ext: Window creation event triggered
    Ext->>Bridge: WebSocket: window_created message
    
    Note over Bridge: BEFORE FIX: Request arrives before window_created<br/>Result: Falls back to primaryClientId (wrong routing)
    
    Note over Agent,Ext: With Fix Applied
    
    Agent->>Agent: chrome.windows.create()
    Agent->>Bridge: HTTP Request with windowId
    
    Bridge->>Bridge: Check windowOwnership.get(windowId)
    Bridge->>Bridge: Not registered yet, call waitForWindowOwnership()
    
    loop Poll every 50ms (max 500ms)
        Bridge->>Bridge: Check windowOwnership.get(windowId)
        Note over Ext: Window creation event triggered
        Ext-->>Bridge: WebSocket: window_created message
        Bridge->>Bridge: handleWindowCreated() registers window
        Bridge->>Bridge: ownerClientId found!
    end
    
    Bridge->>Ext: Route request to correct client
    Ext->>Bridge: Response
    Bridge->>Agent: Response
Loading

@Suhaib3100
Copy link
Author

Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement.

To sign the CLA, please add a comment to this PR with the following text:

I have read the CLA Document and I hereby sign the CLA

You only need to sign once. After signing, this check will pass automatically.

Troubleshooting

  • Already signed but still failing? Comment recheck to trigger a re-verification.
  • Signed with a different email? Make sure your commit email matches your GitHub account email, or add your commit email to your GitHub account.
      • I have read the CLA Document and I hereby sign the CLA - - - You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

I have read the CLA Document and I hereby sign the CLA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: Race condition in window ownership registration causes multi-window routing failures

1 participant