Skip to content

Commit 10ea45d

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 40c5709 commit 10ea45d

File tree

3 files changed

+116
-19
lines changed

3 files changed

+116
-19
lines changed

integration-tests/browser-agent.test.ts

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

311-
it('should handle concurrent browser agents with persistent session mode', async () => {
311+
it('should handle concurrent browser agents with isolated session mode', async () => {
312312
rig.setup('browser-concurrent', {
313313
fakeResponsesPath: join(__dirname, 'browser-agent.concurrent.responses'),
314314
settings: {
@@ -320,9 +320,10 @@ describe.skipIf(!chromeAvailable)('browser-agent', () => {
320320
},
321321
browser: {
322322
headless: true,
323-
// Persistent mode is the default — the regression scenario.
324-
// Two concurrent calls should each get a separate browser.
325-
sessionMode: 'persistent',
323+
// Isolated mode supports concurrent browser agents.
324+
// Persistent/existing modes reject concurrent calls to prevent
325+
// Chrome profile lock conflicts.
326+
sessionMode: 'isolated',
326327
},
327328
},
328329
},

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

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

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

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

883916
expect(instance2).not.toBe(instance1);
884917
expect(instance1.isAcquired()).toBe(true);
@@ -896,32 +929,66 @@ describe('BrowserManager', () => {
896929
expect(instance1.isAcquired()).toBe(false);
897930
});
898931

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

903-
const instance2 = BrowserManager.getInstance(mockConfig);
943+
const instance2 = BrowserManager.getInstance(isolatedConfig);
904944
instance2.acquire();
905945
instance2.release();
906946

907947
// Primary is still acquired, parallel is released — should reuse parallel
908-
const instance3 = BrowserManager.getInstance(mockConfig);
948+
const instance3 = BrowserManager.getInstance(isolatedConfig);
909949
expect(instance3).toBe(instance2);
910950
});
911951

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

916-
const instance2 = BrowserManager.getInstance(mockConfig);
963+
const instance2 = BrowserManager.getInstance(isolatedConfig);
917964
instance2.acquire();
918965

919-
const instance3 = BrowserManager.getInstance(mockConfig);
966+
const instance3 = BrowserManager.getInstance(isolatedConfig);
920967

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

927994
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)