mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-12 21:03:05 -07:00
fix: isolate concurrent browser agent instances (#24794)
This commit is contained in:
@@ -0,0 +1,8 @@
|
||||
{"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}}]}
|
||||
{"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}}]}
|
||||
{"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}}]}
|
||||
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":150,"candidatesTokenCount":15,"totalTokenCount":165}}]}
|
||||
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":150,"candidatesTokenCount":15,"totalTokenCount":165}}]}
|
||||
{"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}}]}
|
||||
{"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}}]}
|
||||
{"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}}]}
|
||||
@@ -307,4 +307,48 @@ describe.skipIf(!chromeAvailable)('browser-agent', () => {
|
||||
|
||||
await run.expectText('successfully written', 15000);
|
||||
});
|
||||
|
||||
it('should handle concurrent browser agents with isolated session mode', async () => {
|
||||
rig.setup('browser-concurrent', {
|
||||
fakeResponsesPath: join(__dirname, 'browser-agent.concurrent.responses'),
|
||||
settings: {
|
||||
agents: {
|
||||
overrides: {
|
||||
browser_agent: {
|
||||
enabled: true,
|
||||
},
|
||||
},
|
||||
browser: {
|
||||
headless: true,
|
||||
// Isolated mode supports concurrent browser agents.
|
||||
// Persistent/existing modes reject concurrent calls to prevent
|
||||
// Chrome profile lock conflicts.
|
||||
sessionMode: 'isolated',
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = await rig.run({
|
||||
args: 'Launch two browser agents concurrently to check example.com',
|
||||
});
|
||||
|
||||
assertModelHasOutput(result);
|
||||
|
||||
const toolLogs = rig.readToolLogs();
|
||||
const browserCalls = toolLogs.filter(
|
||||
(t) => t.toolRequest.name === 'browser_agent',
|
||||
);
|
||||
|
||||
// Both browser_agent invocations should have been called
|
||||
expect(browserCalls.length).toBe(2);
|
||||
|
||||
// Both should complete successfully (no errors)
|
||||
for (const call of browserCalls) {
|
||||
expect(
|
||||
call.toolRequest.success,
|
||||
`browser_agent call failed: ${JSON.stringify(call.toolRequest)}`,
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -38,6 +38,8 @@ const mockBrowserManager = {
|
||||
]),
|
||||
callTool: vi.fn().mockResolvedValue({ content: [] }),
|
||||
close: vi.fn().mockResolvedValue(undefined),
|
||||
acquire: vi.fn(),
|
||||
release: vi.fn(),
|
||||
};
|
||||
|
||||
// Mock dependencies
|
||||
|
||||
@@ -81,6 +81,9 @@ export async function createBrowserAgentDefinition(
|
||||
|
||||
// Get or create browser manager singleton for this session mode/profile
|
||||
const browserManager = BrowserManager.getInstance(config);
|
||||
browserManager.acquire();
|
||||
|
||||
try {
|
||||
await browserManager.ensureConnection();
|
||||
|
||||
debugLogger.log('Browser connected with isolated MCP client.');
|
||||
@@ -268,7 +271,10 @@ export async function createBrowserAgentDefinition(
|
||||
|
||||
// Create configured definition with tools
|
||||
// BrowserAgentDefinition is a factory function - call it with config
|
||||
const baseDefinition = BrowserAgentDefinition(config, !visionDisabledReason);
|
||||
const baseDefinition = BrowserAgentDefinition(
|
||||
config,
|
||||
!visionDisabledReason,
|
||||
);
|
||||
const definition: LocalAgentDefinition<typeof BrowserTaskResultSchema> = {
|
||||
...baseDefinition,
|
||||
toolConfig: {
|
||||
@@ -282,6 +288,11 @@ export async function createBrowserAgentDefinition(
|
||||
visionEnabled: !visionDisabledReason,
|
||||
sessionMode,
|
||||
};
|
||||
} catch (error) {
|
||||
// Release the browser manager if setup fails, so concurrent tasks can try again.
|
||||
browserManager.release();
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -192,7 +192,10 @@ describe('BrowserAgentInvocation', () => {
|
||||
promptConfig: { query: '', systemPrompt: '' },
|
||||
toolConfig: { tools: ['analyze_screenshot', 'click'] },
|
||||
},
|
||||
browserManager: {} as never,
|
||||
browserManager: {
|
||||
release: vi.fn(),
|
||||
callTool: vi.fn().mockResolvedValue({ content: [] }),
|
||||
} as never,
|
||||
visionEnabled: true,
|
||||
sessionMode: 'persistent',
|
||||
});
|
||||
@@ -766,6 +769,7 @@ describe('BrowserAgentInvocation', () => {
|
||||
}
|
||||
return { isError: false };
|
||||
}),
|
||||
release: vi.fn(),
|
||||
};
|
||||
|
||||
vi.mocked(createBrowserAgentDefinition).mockResolvedValue({
|
||||
|
||||
@@ -440,6 +440,8 @@ ${output.result}`;
|
||||
}
|
||||
} catch {
|
||||
// Ignore errors for removing the overlays.
|
||||
} finally {
|
||||
browserManager.release();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -873,6 +873,122 @@ describe('BrowserManager', () => {
|
||||
|
||||
expect(instance1).not.toBe(instance2);
|
||||
});
|
||||
|
||||
it('should throw when acquired instance is requested in persistent mode', () => {
|
||||
// mockConfig defaults to persistent mode
|
||||
const instance1 = BrowserManager.getInstance(mockConfig);
|
||||
instance1.acquire();
|
||||
|
||||
expect(() => BrowserManager.getInstance(mockConfig)).toThrow(
|
||||
/Cannot launch a concurrent browser agent in "persistent" session mode/,
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw when acquired instance is requested in existing mode', () => {
|
||||
const existingConfig = makeFakeConfig({
|
||||
agents: {
|
||||
overrides: { browser_agent: { enabled: true } },
|
||||
browser: { sessionMode: 'existing' },
|
||||
},
|
||||
});
|
||||
|
||||
const instance1 = BrowserManager.getInstance(existingConfig);
|
||||
instance1.acquire();
|
||||
|
||||
expect(() => BrowserManager.getInstance(existingConfig)).toThrow(
|
||||
/Cannot launch a concurrent browser agent in "existing" session mode/,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return a different instance when the primary is acquired in isolated mode', () => {
|
||||
const isolatedConfig = makeFakeConfig({
|
||||
agents: {
|
||||
overrides: { browser_agent: { enabled: true } },
|
||||
browser: { sessionMode: 'isolated' },
|
||||
},
|
||||
});
|
||||
|
||||
const instance1 = BrowserManager.getInstance(isolatedConfig);
|
||||
instance1.acquire();
|
||||
|
||||
const instance2 = BrowserManager.getInstance(isolatedConfig);
|
||||
|
||||
expect(instance2).not.toBe(instance1);
|
||||
expect(instance1.isAcquired()).toBe(true);
|
||||
expect(instance2.isAcquired()).toBe(false);
|
||||
});
|
||||
|
||||
it('should reuse the primary when it has been released', () => {
|
||||
const instance1 = BrowserManager.getInstance(mockConfig);
|
||||
instance1.acquire();
|
||||
instance1.release();
|
||||
|
||||
const instance2 = BrowserManager.getInstance(mockConfig);
|
||||
|
||||
expect(instance2).toBe(instance1);
|
||||
expect(instance1.isAcquired()).toBe(false);
|
||||
});
|
||||
|
||||
it('should reuse a released parallel instance in isolated mode', () => {
|
||||
const isolatedConfig = makeFakeConfig({
|
||||
agents: {
|
||||
overrides: { browser_agent: { enabled: true } },
|
||||
browser: { sessionMode: 'isolated' },
|
||||
},
|
||||
});
|
||||
|
||||
const instance1 = BrowserManager.getInstance(isolatedConfig);
|
||||
instance1.acquire();
|
||||
|
||||
const instance2 = BrowserManager.getInstance(isolatedConfig);
|
||||
instance2.acquire();
|
||||
instance2.release();
|
||||
|
||||
// Primary is still acquired, parallel is released — should reuse parallel
|
||||
const instance3 = BrowserManager.getInstance(isolatedConfig);
|
||||
expect(instance3).toBe(instance2);
|
||||
});
|
||||
|
||||
it('should create multiple parallel instances in isolated mode', () => {
|
||||
const isolatedConfig = makeFakeConfig({
|
||||
agents: {
|
||||
overrides: { browser_agent: { enabled: true } },
|
||||
browser: { sessionMode: 'isolated' },
|
||||
},
|
||||
});
|
||||
|
||||
const instance1 = BrowserManager.getInstance(isolatedConfig);
|
||||
instance1.acquire();
|
||||
|
||||
const instance2 = BrowserManager.getInstance(isolatedConfig);
|
||||
instance2.acquire();
|
||||
|
||||
const instance3 = BrowserManager.getInstance(isolatedConfig);
|
||||
|
||||
expect(instance1).not.toBe(instance2);
|
||||
expect(instance2).not.toBe(instance3);
|
||||
expect(instance1).not.toBe(instance3);
|
||||
});
|
||||
|
||||
it('should throw when MAX_PARALLEL_INSTANCES is reached in isolated mode', () => {
|
||||
const isolatedConfig = makeFakeConfig({
|
||||
agents: {
|
||||
overrides: { browser_agent: { enabled: true } },
|
||||
browser: { sessionMode: 'isolated' },
|
||||
},
|
||||
});
|
||||
|
||||
// Acquire MAX_PARALLEL_INSTANCES instances
|
||||
for (let i = 0; i < BrowserManager.MAX_PARALLEL_INSTANCES; i++) {
|
||||
const instance = BrowserManager.getInstance(isolatedConfig);
|
||||
instance.acquire();
|
||||
}
|
||||
|
||||
// Next call should throw
|
||||
expect(() => BrowserManager.getInstance(isolatedConfig)).toThrow(
|
||||
/Maximum number of parallel browser instances/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resetAll', () => {
|
||||
|
||||
@@ -114,6 +114,12 @@ export class BrowserManager {
|
||||
// --- Static singleton management ---
|
||||
private static instances = new Map<string, BrowserManager>();
|
||||
|
||||
/**
|
||||
* Maximum number of parallel browser instances allowed in isolated mode.
|
||||
* Prevents unbounded resource consumption from concurrent browser_agent calls.
|
||||
*/
|
||||
static readonly MAX_PARALLEL_INSTANCES = 5;
|
||||
|
||||
/**
|
||||
* Returns the cache key for a given config.
|
||||
* Uses `sessionMode:profilePath` so different profiles get separate instances.
|
||||
@@ -128,14 +134,64 @@ export class BrowserManager {
|
||||
/**
|
||||
* Returns an existing BrowserManager for the current config's session mode
|
||||
* and profile, or creates a new one.
|
||||
*
|
||||
* Concurrency rules:
|
||||
* - **persistent / existing mode**: Only one instance is allowed at a time.
|
||||
* If the instance is already in-use, an error is thrown instructing the
|
||||
* caller to run browser tasks sequentially.
|
||||
* - **isolated mode**: Parallel instances are allowed up to
|
||||
* MAX_PARALLEL_INSTANCES. Each isolated instance gets its own temp profile.
|
||||
*/
|
||||
static getInstance(config: Config): BrowserManager {
|
||||
const key = BrowserManager.getInstanceKey(config);
|
||||
const sessionMode =
|
||||
config.getBrowserAgentConfig().customConfig.sessionMode ?? 'persistent';
|
||||
let instance = BrowserManager.instances.get(key);
|
||||
if (!instance) {
|
||||
instance = new BrowserManager(config);
|
||||
BrowserManager.instances.set(key, instance);
|
||||
debugLogger.log(`Created new BrowserManager singleton (key: ${key})`);
|
||||
} else if (instance.inUse) {
|
||||
// Persistent and existing modes share a browser profile directory.
|
||||
// Chrome prevents multiple instances from using the same profile, so
|
||||
// concurrent usage would cause "profile locked" errors.
|
||||
if (sessionMode === 'persistent' || sessionMode === 'existing') {
|
||||
throw new Error(
|
||||
`Cannot launch a concurrent browser agent in "${sessionMode}" session mode. ` +
|
||||
`The browser instance is already in use by another task. ` +
|
||||
`Please run browser tasks sequentially, or switch to "isolated" session mode for concurrent browser usage.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Isolated mode: allow parallel instances up to the limit.
|
||||
let inUseCount = 1; // primary is already in-use
|
||||
let suffix = 1;
|
||||
let parallelKey = `${key}:${suffix}`;
|
||||
let parallel = BrowserManager.instances.get(parallelKey);
|
||||
while (parallel?.inUse) {
|
||||
inUseCount++;
|
||||
if (inUseCount >= BrowserManager.MAX_PARALLEL_INSTANCES) {
|
||||
throw new Error(
|
||||
`Maximum number of parallel browser instances (${BrowserManager.MAX_PARALLEL_INSTANCES}) reached. ` +
|
||||
`Please wait for an existing browser task to complete before starting a new one.`,
|
||||
);
|
||||
}
|
||||
suffix++;
|
||||
parallelKey = `${key}:${suffix}`;
|
||||
parallel = BrowserManager.instances.get(parallelKey);
|
||||
}
|
||||
if (!parallel) {
|
||||
parallel = new BrowserManager(config);
|
||||
BrowserManager.instances.set(parallelKey, parallel);
|
||||
debugLogger.log(
|
||||
`Created parallel BrowserManager (key: ${parallelKey})`,
|
||||
);
|
||||
} else {
|
||||
debugLogger.log(
|
||||
`Reusing released parallel BrowserManager (key: ${parallelKey})`,
|
||||
);
|
||||
}
|
||||
instance = parallel;
|
||||
} else {
|
||||
debugLogger.log(
|
||||
`Reusing existing BrowserManager singleton (key: ${key})`,
|
||||
@@ -180,6 +236,36 @@ export class BrowserManager {
|
||||
private isClosing = false;
|
||||
private connectionPromise: Promise<void> | undefined;
|
||||
|
||||
/**
|
||||
* Whether this instance is currently acquired by an active invocation.
|
||||
* Used by getInstance() to avoid handing the same browser to concurrent
|
||||
* browser_agent calls.
|
||||
*/
|
||||
private inUse = false;
|
||||
|
||||
/**
|
||||
* Marks this instance as in-use. Call this when starting a browser agent
|
||||
* invocation so concurrent calls get a separate instance.
|
||||
*/
|
||||
acquire(): void {
|
||||
this.inUse = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Marks this instance as available for reuse. Call this in the finally
|
||||
* block of a browser agent invocation.
|
||||
*/
|
||||
release(): void {
|
||||
this.inUse = false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns whether this instance is currently acquired by an active invocation.
|
||||
*/
|
||||
isAcquired(): boolean {
|
||||
return this.inUse;
|
||||
}
|
||||
|
||||
/** State for action rate limiting */
|
||||
private actionCounter = 0;
|
||||
private readonly maxActionsPerTask: number;
|
||||
|
||||
Reference in New Issue
Block a user