fix(browser): remove premature browser cleanup after subagent invocation (#24753)

This commit is contained in:
Gaurav
2026-04-07 05:17:31 +08:00
committed by GitHub
parent ad98294352
commit e116aa34f4
4 changed files with 56 additions and 11 deletions

View File

@@ -0,0 +1,8 @@
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"I'll browse to example.com twice to verify the content. Let me first check the page title, then check the links on the page."},{"functionCall":{"name":"browser_agent","args":{"task":"Navigate to https://example.com and tell me the page title using the accessibility tree"}}}],"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":30,"totalTokenCount":130}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":150,"candidatesTokenCount":20,"totalTokenCount":170}}]}
{"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":40,"totalTokenCount":240}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"The page title is 'Example Domain'. Now let me check the links on the page."},{"functionCall":{"name":"browser_agent","args":{"task":"Take a snapshot of the accessibility tree on the currently open page and tell me about any links"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":200,"candidatesTokenCount":50,"totalTokenCount":250}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":150,"candidatesTokenCount":20,"totalTokenCount":170}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"complete_task","args":{"result":{"success":true,"summary":"Found a link 'More information...' pointing to iana.org."}}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":200,"candidatesTokenCount":40,"totalTokenCount":240}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"I browsed example.com twice using persistent browser sessions:\n\n1. **First visit**: Page title is 'Example Domain'\n2. **Second visit**: Found a link 'More information...' pointing to iana.org\n\nThe browser stayed open between both visits, confirming persistent session management works correctly."}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":300,"candidatesTokenCount":60,"totalTokenCount":360}}]}

View File

@@ -229,6 +229,51 @@ describe.skipIf(!chromeAvailable)('browser-agent', () => {
assertModelHasOutput(result);
});
it('should keep browser open across multiple browser_agent invocations', async () => {
rig.setup('browser-persistent-session', {
fakeResponsesPath: join(
__dirname,
'browser-agent.persistent-session.responses',
),
settings: {
agents: {
overrides: {
browser_agent: {
enabled: true,
},
},
browser: {
headless: true,
sessionMode: 'isolated',
},
},
},
});
const result = await rig.run({
args: 'Browse to example.com twice: first get the page title, then check for links.',
});
const toolLogs = rig.readToolLogs();
const browserCalls = toolLogs.filter(
(t) => t.toolRequest.name === 'browser_agent',
);
// Both browser_agent invocations must succeed — if the browser was
// incorrectly closed after the first call (regression #24210),
// the second call would fail.
expect(
browserCalls.length,
'Expected browser_agent to be called twice',
).toBe(2);
expect(
browserCalls.every((c) => c.toolRequest.success),
'Both browser_agent calls should succeed',
).toBe(true);
assertModelHasOutput(result);
});
it('should handle tool confirmation for write_file without crashing', async () => {
rig.setup('tool-confirmation', {
fakeResponsesPath: join(

View File

@@ -742,7 +742,7 @@ describe('BrowserAgentInvocation', () => {
);
});
it('should call cleanupBrowserAgent with correct params', async () => {
it('should not call cleanupBrowserAgent (cleanup is handled by BrowserManager.resetAll)', async () => {
const invocation = new BrowserAgentInvocation(
mockConfig,
mockParams,
@@ -750,11 +750,7 @@ describe('BrowserAgentInvocation', () => {
);
await invocation.execute(new AbortController().signal, vi.fn());
expect(cleanupBrowserAgent).toHaveBeenCalledWith(
expect.anything(),
mockConfig,
'persistent',
);
expect(cleanupBrowserAgent).not.toHaveBeenCalled();
});
});

View File

@@ -34,10 +34,7 @@ import {
isToolActivityError,
} from '../types.js';
import type { MessageBus } from '../../confirmation-bus/message-bus.js';
import {
createBrowserAgentDefinition,
cleanupBrowserAgent,
} from './browserAgentFactory.js';
import { createBrowserAgentDefinition } from './browserAgentFactory.js';
import { removeInputBlocker } from './inputBlocker.js';
import { recordBrowserAgentTaskOutcome } from '../../telemetry/metrics.js';
import {
@@ -444,7 +441,6 @@ ${output.result}`;
} catch {
// Ignore errors for removing the overlays.
}
await cleanupBrowserAgent(browserManager, this.config, sessionMode);
}
}
}