Skip to content

Commit 893b2bf

Browse files
committed
fix: limit parallel browser instances and reject concurrent persistent sessions
- Persistent/existing mode: throw error when instance is already in-use, instructing the model to run browser tasks sequentially. This prevents Chrome profile lock conflicts from concurrent --userDataDir usage. - Isolated mode: allow parallel instances up to MAX_PARALLEL_INSTANCES (5). Throw when the limit is exceeded to prevent resource exhaustion. - Update integration test to use isolated mode for concurrent browser agents. - Add 3 new unit tests for persistent/existing rejection and max limit.
1 parent 97da195 commit 893b2bf

3 files changed

Lines changed: 116 additions & 19 deletions

File tree

integration-tests/browser-agent.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ describe.skipIf(!chromeAvailable)('browser-agent', () => {
263263
await run.expectText('successfully written', 15000);
264264
});
265265

266-
it('should handle concurrent browser agents with persistent session mode', async () => {
266+
it('should handle concurrent browser agents with isolated session mode', async () => {
267267
rig.setup('browser-concurrent', {
268268
fakeResponsesPath: join(__dirname, 'browser-agent.concurrent.responses'),
269269
settings: {
@@ -275,9 +275,10 @@ describe.skipIf(!chromeAvailable)('browser-agent', () => {
275275
},
276276
browser: {
277277
headless: true,
278-
// Persistent mode is the default — the regression scenario.
279-
// Two concurrent calls should each get a separate browser.
280-
sessionMode: 'persistent',
278+
// Isolated mode supports concurrent browser agents.
279+
// Persistent/existing modes reject concurrent calls to prevent
280+
// Chrome profile lock conflicts.
281+
sessionMode: 'isolated',
281282
},
282283
},
283284
},

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

Lines changed: 77 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -873,11 +873,44 @@ describe('BrowserManager', () => {
873873
expect(instance1).not.toBe(instance2);
874874
});
875875

876-
it('should return a different instance when the primary is acquired', () => {
876+
it('should throw when acquired instance is requested in persistent mode', () => {
877+
// mockConfig defaults to persistent mode
877878
const instance1 = BrowserManager.getInstance(mockConfig);
878879
instance1.acquire();
879880

880-
const instance2 = BrowserManager.getInstance(mockConfig);
881+
expect(() => BrowserManager.getInstance(mockConfig)).toThrow(
882+
/Cannot launch a concurrent browser agent in "persistent" session mode/,
883+
);
884+
});
885+
886+
it('should throw when acquired instance is requested in existing mode', () => {
887+
const existingConfig = makeFakeConfig({
888+
agents: {
889+
overrides: { browser_agent: { enabled: true } },
890+
browser: { sessionMode: 'existing' },
891+
},
892+
});
893+
894+
const instance1 = BrowserManager.getInstance(existingConfig);
895+
instance1.acquire();
896+
897+
expect(() => BrowserManager.getInstance(existingConfig)).toThrow(
898+
/Cannot launch a concurrent browser agent in "existing" session mode/,
899+
);
900+
});
901+
902+
it('should return a different instance when the primary is acquired in isolated mode', () => {
903+
const isolatedConfig = makeFakeConfig({
904+
agents: {
905+
overrides: { browser_agent: { enabled: true } },
906+
browser: { sessionMode: 'isolated' },
907+
},
908+
});
909+
910+
const instance1 = BrowserManager.getInstance(isolatedConfig);
911+
instance1.acquire();
912+
913+
const instance2 = BrowserManager.getInstance(isolatedConfig);
881914

882915
expect(instance2).not.toBe(instance1);
883916
expect(instance1.isAcquired()).toBe(true);
@@ -895,32 +928,66 @@ describe('BrowserManager', () => {
895928
expect(instance1.isAcquired()).toBe(false);
896929
});
897930

898-
it('should reuse a released parallel instance', () => {
899-
const instance1 = BrowserManager.getInstance(mockConfig);
931+
it('should reuse a released parallel instance in isolated mode', () => {
932+
const isolatedConfig = makeFakeConfig({
933+
agents: {
934+
overrides: { browser_agent: { enabled: true } },
935+
browser: { sessionMode: 'isolated' },
936+
},
937+
});
938+
939+
const instance1 = BrowserManager.getInstance(isolatedConfig);
900940
instance1.acquire();
901941

902-
const instance2 = BrowserManager.getInstance(mockConfig);
942+
const instance2 = BrowserManager.getInstance(isolatedConfig);
903943
instance2.acquire();
904944
instance2.release();
905945

906946
// Primary is still acquired, parallel is released — should reuse parallel
907-
const instance3 = BrowserManager.getInstance(mockConfig);
947+
const instance3 = BrowserManager.getInstance(isolatedConfig);
908948
expect(instance3).toBe(instance2);
909949
});
910950

911-
it('should create multiple parallel instances for multiple concurrent calls', () => {
912-
const instance1 = BrowserManager.getInstance(mockConfig);
951+
it('should create multiple parallel instances in isolated mode', () => {
952+
const isolatedConfig = makeFakeConfig({
953+
agents: {
954+
overrides: { browser_agent: { enabled: true } },
955+
browser: { sessionMode: 'isolated' },
956+
},
957+
});
958+
959+
const instance1 = BrowserManager.getInstance(isolatedConfig);
913960
instance1.acquire();
914961

915-
const instance2 = BrowserManager.getInstance(mockConfig);
962+
const instance2 = BrowserManager.getInstance(isolatedConfig);
916963
instance2.acquire();
917964

918-
const instance3 = BrowserManager.getInstance(mockConfig);
965+
const instance3 = BrowserManager.getInstance(isolatedConfig);
919966

920967
expect(instance1).not.toBe(instance2);
921968
expect(instance2).not.toBe(instance3);
922969
expect(instance1).not.toBe(instance3);
923970
});
971+
972+
it('should throw when MAX_PARALLEL_INSTANCES is reached in isolated mode', () => {
973+
const isolatedConfig = makeFakeConfig({
974+
agents: {
975+
overrides: { browser_agent: { enabled: true } },
976+
browser: { sessionMode: 'isolated' },
977+
},
978+
});
979+
980+
// Acquire MAX_PARALLEL_INSTANCES instances
981+
for (let i = 0; i < BrowserManager.MAX_PARALLEL_INSTANCES; i++) {
982+
const instance = BrowserManager.getInstance(isolatedConfig);
983+
instance.acquire();
984+
}
985+
986+
// Next call should throw
987+
expect(() => BrowserManager.getInstance(isolatedConfig)).toThrow(
988+
/Maximum number of parallel browser instances/,
989+
);
990+
});
924991
});
925992

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

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,12 @@ export class BrowserManager {
114114
// --- Static singleton management ---
115115
private static instances = new Map<string, BrowserManager>();
116116

117+
/**
118+
* Maximum number of parallel browser instances allowed in isolated mode.
119+
* Prevents unbounded resource consumption from concurrent browser_agent calls.
120+
*/
121+
static readonly MAX_PARALLEL_INSTANCES = 5;
122+
117123
/**
118124
* Returns the cache key for a given config.
119125
* Uses `sessionMode:profilePath` so different profiles get separate instances.
@@ -129,24 +135,47 @@ export class BrowserManager {
129135
* Returns an existing BrowserManager for the current config's session mode
130136
* and profile, or creates a new one.
131137
*
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.
138+
* Concurrency rules:
139+
* - **persistent / existing mode**: Only one instance is allowed at a time.
140+
* If the instance is already in-use, an error is thrown instructing the
141+
* caller to run browser tasks sequentially.
142+
* - **isolated mode**: Parallel instances are allowed up to
143+
* MAX_PARALLEL_INSTANCES. Each isolated instance gets its own temp profile.
135144
*/
136145
static getInstance(config: Config): BrowserManager {
137146
const key = BrowserManager.getInstanceKey(config);
147+
const sessionMode =
148+
config.getBrowserAgentConfig().customConfig.sessionMode ?? 'persistent';
138149
let instance = BrowserManager.instances.get(key);
139150
if (!instance) {
140151
instance = new BrowserManager(config);
141152
BrowserManager.instances.set(key, instance);
142153
debugLogger.log(`Created new BrowserManager singleton (key: ${key})`);
143154
} 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.
155+
// Persistent and existing modes share a browser profile directory.
156+
// Chrome prevents multiple instances from using the same profile, so
157+
// concurrent usage would cause "profile locked" errors.
158+
if (sessionMode === 'persistent' || sessionMode === 'existing') {
159+
throw new Error(
160+
`Cannot launch a concurrent browser agent in "${sessionMode}" session mode. ` +
161+
`The browser instance is already in use by another task. ` +
162+
`Please run browser tasks sequentially, or switch to "isolated" session mode for concurrent browser usage.`,
163+
);
164+
}
165+
166+
// Isolated mode: allow parallel instances up to the limit.
167+
let inUseCount = 1; // primary is already in-use
146168
let suffix = 1;
147169
let parallelKey = `${key}:${suffix}`;
148170
let parallel = BrowserManager.instances.get(parallelKey);
149171
while (parallel?.inUse) {
172+
inUseCount++;
173+
if (inUseCount >= BrowserManager.MAX_PARALLEL_INSTANCES) {
174+
throw new Error(
175+
`Maximum number of parallel browser instances (${BrowserManager.MAX_PARALLEL_INSTANCES}) reached. ` +
176+
`Please wait for an existing browser task to complete before starting a new one.`,
177+
);
178+
}
150179
suffix++;
151180
parallelKey = `${key}:${suffix}`;
152181
parallel = BrowserManager.instances.get(parallelKey);

0 commit comments

Comments
 (0)