Skip to content

Commit 97da195

Browse files
committed
fix: isolate concurrent browser agent instances
When two browser_agent calls run concurrently, they previously shared a single BrowserManager singleton, causing one agent to navigate away from the other's page. Add acquire()/release() tracking to BrowserManager so getInstance() creates a new parallel instance when the primary is already in-use. Sequential calls still reuse the same browser (persistent session). Changes: - BrowserManager: add inUse field, acquire/release/isAcquired methods - BrowserManager.getInstance: skip in-use instances, create parallel instances with suffixed keys (e.g. persistent:default:1) - browserAgentFactory: call acquire() after getInstance() - browserAgentInvocation: call release() in finally block - Add 4 unit tests for concurrent isolation behavior - Add integration test for concurrent browser agents with persistent session mode
1 parent 8ac560d commit 97da195

8 files changed

Lines changed: 166 additions & 1 deletion
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"I'll launch two browser agents concurrently to check both repositories."},{"functionCall":{"name":"browser_agent","args":{"task":"Navigate to https://example.com and get the page title"}}},{"functionCall":{"name":"browser_agent","args":{"task":"Navigate to https://example.com and get the page title"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":50,"totalTokenCount":150}}]}
2+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"navigate_page","args":{"url":"https://example.com"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":20,"totalTokenCount":120}}]}
3+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"navigate_page","args":{"url":"https://example.com"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":20,"totalTokenCount":120}}]}
4+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":150,"candidatesTokenCount":15,"totalTokenCount":165}}]}
5+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":150,"candidatesTokenCount":15,"totalTokenCount":165}}]}
6+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"complete_task","args":{"result":{"success":true,"summary":"Page title is Example Domain."}}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":200,"candidatesTokenCount":30,"totalTokenCount":230}}]}
7+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"complete_task","args":{"result":{"success":true,"summary":"Page title is Example Domain."}}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":200,"candidatesTokenCount":30,"totalTokenCount":230}}]}
8+
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"Both browser agents completed successfully. Agent 1 and Agent 2 both navigated to their respective pages and confirmed the page titles."}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":300,"candidatesTokenCount":40,"totalTokenCount":340}}]}

integration-tests/browser-agent.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,4 +262,47 @@ describe.skipIf(!chromeAvailable)('browser-agent', () => {
262262

263263
await run.expectText('successfully written', 15000);
264264
});
265+
266+
it('should handle concurrent browser agents with persistent session mode', async () => {
267+
rig.setup('browser-concurrent', {
268+
fakeResponsesPath: join(__dirname, 'browser-agent.concurrent.responses'),
269+
settings: {
270+
agents: {
271+
overrides: {
272+
browser_agent: {
273+
enabled: true,
274+
},
275+
},
276+
browser: {
277+
headless: true,
278+
// Persistent mode is the default — the regression scenario.
279+
// Two concurrent calls should each get a separate browser.
280+
sessionMode: 'persistent',
281+
},
282+
},
283+
},
284+
});
285+
286+
const result = await rig.run({
287+
args: 'Launch two browser agents concurrently to check example.com',
288+
});
289+
290+
assertModelHasOutput(result);
291+
292+
const toolLogs = rig.readToolLogs();
293+
const browserCalls = toolLogs.filter(
294+
(t) => t.toolRequest.name === 'browser_agent',
295+
);
296+
297+
// Both browser_agent invocations should have been called
298+
expect(browserCalls.length).toBe(2);
299+
300+
// Both should complete successfully (no errors)
301+
for (const call of browserCalls) {
302+
expect(
303+
call.toolRequest.success,
304+
`browser_agent call failed: ${JSON.stringify(call.toolRequest)}`,
305+
).toBe(true);
306+
}
307+
});
265308
});

packages/core/src/agents/browser/browserAgentFactory.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ const mockBrowserManager = {
3838
]),
3939
callTool: vi.fn().mockResolvedValue({ content: [] }),
4040
close: vi.fn().mockResolvedValue(undefined),
41+
acquire: vi.fn(),
42+
release: vi.fn(),
4143
};
4244

4345
// Mock dependencies

packages/core/src/agents/browser/browserAgentFactory.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export async function createBrowserAgentDefinition(
8181

8282
// Get or create browser manager singleton for this session mode/profile
8383
const browserManager = BrowserManager.getInstance(config);
84+
browserManager.acquire();
8485
await browserManager.ensureConnection();
8586

8687
debugLogger.log('Browser connected with isolated MCP client.');

packages/core/src/agents/browser/browserAgentInvocation.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,10 @@ describe('BrowserAgentInvocation', () => {
192192
promptConfig: { query: '', systemPrompt: '' },
193193
toolConfig: { tools: ['analyze_screenshot', 'click'] },
194194
},
195-
browserManager: {} as never,
195+
browserManager: {
196+
release: vi.fn(),
197+
callTool: vi.fn().mockResolvedValue({ content: [] }),
198+
} as never,
196199
visionEnabled: true,
197200
sessionMode: 'persistent',
198201
});
@@ -770,6 +773,7 @@ describe('BrowserAgentInvocation', () => {
770773
}
771774
return { isError: false };
772775
}),
776+
release: vi.fn(),
773777
};
774778

775779
vi.mocked(createBrowserAgentDefinition).mockResolvedValue({

packages/core/src/agents/browser/browserAgentInvocation.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ ${output.result}`;
445445
// Ignore errors for removing the overlays.
446446
}
447447
await cleanupBrowserAgent(browserManager, this.config, sessionMode);
448+
browserManager.release();
448449
}
449450
}
450451
}

packages/core/src/agents/browser/browserManager.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,55 @@ describe('BrowserManager', () => {
872872

873873
expect(instance1).not.toBe(instance2);
874874
});
875+
876+
it('should return a different instance when the primary is acquired', () => {
877+
const instance1 = BrowserManager.getInstance(mockConfig);
878+
instance1.acquire();
879+
880+
const instance2 = BrowserManager.getInstance(mockConfig);
881+
882+
expect(instance2).not.toBe(instance1);
883+
expect(instance1.isAcquired()).toBe(true);
884+
expect(instance2.isAcquired()).toBe(false);
885+
});
886+
887+
it('should reuse the primary when it has been released', () => {
888+
const instance1 = BrowserManager.getInstance(mockConfig);
889+
instance1.acquire();
890+
instance1.release();
891+
892+
const instance2 = BrowserManager.getInstance(mockConfig);
893+
894+
expect(instance2).toBe(instance1);
895+
expect(instance1.isAcquired()).toBe(false);
896+
});
897+
898+
it('should reuse a released parallel instance', () => {
899+
const instance1 = BrowserManager.getInstance(mockConfig);
900+
instance1.acquire();
901+
902+
const instance2 = BrowserManager.getInstance(mockConfig);
903+
instance2.acquire();
904+
instance2.release();
905+
906+
// Primary is still acquired, parallel is released — should reuse parallel
907+
const instance3 = BrowserManager.getInstance(mockConfig);
908+
expect(instance3).toBe(instance2);
909+
});
910+
911+
it('should create multiple parallel instances for multiple concurrent calls', () => {
912+
const instance1 = BrowserManager.getInstance(mockConfig);
913+
instance1.acquire();
914+
915+
const instance2 = BrowserManager.getInstance(mockConfig);
916+
instance2.acquire();
917+
918+
const instance3 = BrowserManager.getInstance(mockConfig);
919+
920+
expect(instance1).not.toBe(instance2);
921+
expect(instance2).not.toBe(instance3);
922+
expect(instance1).not.toBe(instance3);
923+
});
875924
});
876925

877926
describe('resetAll', () => {

packages/core/src/agents/browser/browserManager.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ export class BrowserManager {
128128
/**
129129
* Returns an existing BrowserManager for the current config's session mode
130130
* and profile, or creates a new one.
131+
*
132+
* When an instance is currently acquired (in-use by an active invocation),
133+
* a new instance is created with a suffixed key so concurrent browser_agent
134+
* calls each get their own browser.
131135
*/
132136
static getInstance(config: Config): BrowserManager {
133137
const key = BrowserManager.getInstanceKey(config);
@@ -136,6 +140,29 @@ export class BrowserManager {
136140
instance = new BrowserManager(config);
137141
BrowserManager.instances.set(key, instance);
138142
debugLogger.log(`Created new BrowserManager singleton (key: ${key})`);
143+
} else if (instance.inUse) {
144+
// The primary instance is in-use by another concurrent invocation.
145+
// Find or create a parallel instance with a suffixed key.
146+
let suffix = 1;
147+
let parallelKey = `${key}:${suffix}`;
148+
let parallel = BrowserManager.instances.get(parallelKey);
149+
while (parallel?.inUse) {
150+
suffix++;
151+
parallelKey = `${key}:${suffix}`;
152+
parallel = BrowserManager.instances.get(parallelKey);
153+
}
154+
if (!parallel) {
155+
parallel = new BrowserManager(config);
156+
BrowserManager.instances.set(parallelKey, parallel);
157+
debugLogger.log(
158+
`Created parallel BrowserManager (key: ${parallelKey})`,
159+
);
160+
} else {
161+
debugLogger.log(
162+
`Reusing released parallel BrowserManager (key: ${parallelKey})`,
163+
);
164+
}
165+
instance = parallel;
139166
} else {
140167
debugLogger.log(
141168
`Reusing existing BrowserManager singleton (key: ${key})`,
@@ -180,6 +207,36 @@ export class BrowserManager {
180207
private isClosing = false;
181208
private connectionPromise: Promise<void> | undefined;
182209

210+
/**
211+
* Whether this instance is currently acquired by an active invocation.
212+
* Used by getInstance() to avoid handing the same browser to concurrent
213+
* browser_agent calls.
214+
*/
215+
private inUse = false;
216+
217+
/**
218+
* Marks this instance as in-use. Call this when starting a browser agent
219+
* invocation so concurrent calls get a separate instance.
220+
*/
221+
acquire(): void {
222+
this.inUse = true;
223+
}
224+
225+
/**
226+
* Marks this instance as available for reuse. Call this in the finally
227+
* block of a browser agent invocation.
228+
*/
229+
release(): void {
230+
this.inUse = false;
231+
}
232+
233+
/**
234+
* Returns whether this instance is currently acquired by an active invocation.
235+
*/
236+
isAcquired(): boolean {
237+
return this.inUse;
238+
}
239+
183240
/** State for action rate limiting */
184241
private actionCounter = 0;
185242
private readonly maxActionsPerTask: number;

0 commit comments

Comments
 (0)