From e889fb47a7fddcad117fedcdecd66046506bccd7 Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Sun, 26 Jul 2026 06:04:44 +0530 Subject: [PATCH] fix(cron): close browser tabs after isolated runs (#113566) * fix(cron): clean browser tabs by run session --------- Co-authored-by: Ayaan Zaidi --- .../run.session-key-isolation.test.ts | 7 ++++ src/cron/isolated-agent/run.test-harness.ts | 5 +++ src/cron/isolated-agent/run.ts | 7 ++++ src/gateway/server-cron.test.ts | 19 ----------- src/gateway/server-cron.ts | 34 +++++++------------ 5 files changed, 32 insertions(+), 40 deletions(-) diff --git a/src/cron/isolated-agent/run.session-key-isolation.test.ts b/src/cron/isolated-agent/run.session-key-isolation.test.ts index b3bba66bcbd6..861230b78210 100644 --- a/src/cron/isolated-agent/run.session-key-isolation.test.ts +++ b/src/cron/isolated-agent/run.session-key-isolation.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { makeIsolatedAgentJobFixture, makeIsolatedAgentParamsFixture } from "./job-fixtures.js"; import { setupRunCronIsolatedAgentTurnSuite } from "./run.suite-helpers.js"; import { + cleanupBrowserSessionsForLifecycleEndMock, isCliProviderMock, loadSessionEntryMock, loadRunCronIsolatedAgentTurn, @@ -76,6 +77,12 @@ describe("runCronIsolatedAgentTurn isolated session identity", () => { expect(runRequest.promptCacheKey).not.toContain("daily-monitor"); expect(runRequest.bootstrapContextMode).toBe("lightweight"); expect(runRequest.bootstrapContextRunKind).toBe("cron"); + expect(cleanupBrowserSessionsForLifecycleEndMock).toHaveBeenCalledOnce(); + expect(cleanupBrowserSessionsForLifecycleEndMock).toHaveBeenCalledWith({ + cfg: expect.any(Object), + sessionKeys: ["agent:default:cron:daily-monitor:run:isolated-run-1"], + onWarn: expect.any(Function), + }); const embeddedRunOrder = runEmbeddedAgentMock.mock.invocationCallOrder[0]; if (embeddedRunOrder === undefined) { throw new Error("Expected embedded cron execution order"); diff --git a/src/cron/isolated-agent/run.test-harness.ts b/src/cron/isolated-agent/run.test-harness.ts index 02f590ef8354..2f8cd85a00ae 100644 --- a/src/cron/isolated-agent/run.test-harness.ts +++ b/src/cron/isolated-agent/run.test-harness.ts @@ -97,6 +97,7 @@ export const resolveSessionAuthProfileOverrideMock = createMock(); export const resolveFastModeStateMock = createMock(); export const getChannelPluginMock = createMock(); export const retireSessionMcpRuntimeMock = createMock(); +export const cleanupBrowserSessionsForLifecycleEndMock = createMock(); export const callGatewayMock = createMock(); export const ensureRuntimePluginsLoadedMock = createMock(); export const hasUsableWebSearchProviderMock = createMock(); @@ -355,6 +356,10 @@ vi.mock("../../agents/agent-bundle-mcp-tools.js", () => ({ retireSessionMcpRuntime: retireSessionMcpRuntimeMock, })); +vi.mock("../../browser-lifecycle-cleanup.js", () => ({ + cleanupBrowserSessionsForLifecycleEnd: cleanupBrowserSessionsForLifecycleEndMock, +})); + vi.mock("../../gateway/call.runtime.js", () => ({ callGateway: callGatewayMock, })); diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index 35656da7e873..83de43ed882e 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -12,6 +12,7 @@ import { expandToolGroups, normalizeToolName } from "../../agents/tool-policy.js import { deriveContextPromptTokens } from "../../agents/usage.js"; import type { ThinkLevel } from "../../auto-reply/thinking.js"; import { HEARTBEAT_TOKEN, isSilentReplyPayloadText } from "../../auto-reply/tokens.js"; +import { cleanupBrowserSessionsForLifecycleEnd } from "../../browser-lifecycle-cleanup.js"; import type { CliDeps } from "../../cli/outbound-send-deps.js"; import { resolveAgentModelPrimaryValue } from "../../config/model-input.js"; import type { SessionEntry } from "../../config/sessions.js"; @@ -1953,6 +1954,12 @@ export async function runCronIsolatedAgentTurn(params: { }); } finally { prepared.context.sessionWorkAdmission.release(); + // Browser ownership follows the detached run identity, not the stable cron job key. + await cleanupBrowserSessionsForLifecycleEnd({ + cfg: prepared.context.cfgWithAgentDefaults, + sessionKeys: [prepared.context.runSessionKey], + onWarn: (message) => logWarn(`[cron:${params.job.id}] ${message}`), + }); } } } diff --git a/src/gateway/server-cron.test.ts b/src/gateway/server-cron.test.ts index 0b2b2a651a3a..059a86c55c06 100644 --- a/src/gateway/server-cron.test.ts +++ b/src/gateway/server-cron.test.ts @@ -27,7 +27,6 @@ const { fetchWithSsrFGuardMock, sendCronAnnouncePayloadStrictMock, runCronIsolatedAgentTurnMock, - cleanupBrowserSessionsForLifecycleEndMock, getGlobalHookRunnerMock, runCronChangedMock, abortAndDrainEmbeddedAgentRunMock, @@ -52,7 +51,6 @@ const { status: "ok", summary: "ok", })), - cleanupBrowserSessionsForLifecycleEndMock: vi.fn(async () => {}), runCronChangedMock: vi.fn(async (_event: unknown, _context?: unknown) => {}), getGlobalHookRunnerMock: vi.fn(() => ({ hasHooks: (hookName: string) => hookName === "cron_changed", @@ -193,10 +191,6 @@ vi.mock("../cron/isolated-agent.js", () => ({ runCronIsolatedAgentTurn: runCronIsolatedAgentTurnMock, })); -vi.mock("../browser-lifecycle-cleanup.js", () => ({ - cleanupBrowserSessionsForLifecycleEnd: cleanupBrowserSessionsForLifecycleEndMock, -})); - vi.mock("../plugins/hook-runner-global.js", () => ({ getGlobalHookRunner: getGlobalHookRunnerMock, })); @@ -321,16 +315,6 @@ function expectIsolatedRunFields(fields: Record) { return options; } -function expectCleanupForSessionKeys(sessionKeys: string[]) { - expect(cleanupBrowserSessionsForLifecycleEndMock).toHaveBeenCalledTimes(1); - const options = requireRecord( - callArg(cleanupBrowserSessionsForLifecycleEndMock, 0, 0, "cleanup options"), - "cleanup options", - ); - expect(options.sessionKeys).toEqual(sessionKeys); - expect(options.onWarn).toBeTypeOf("function"); -} - describe("buildGatewayCronService", () => { beforeEach(() => { resetActiveCronTaskRunsForTests(); @@ -342,7 +326,6 @@ describe("buildGatewayCronService", () => { fetchWithSsrFGuardMock.mockClear(); sendCronAnnouncePayloadStrictMock.mockClear(); runCronIsolatedAgentTurnMock.mockClear(); - cleanupBrowserSessionsForLifecycleEndMock.mockClear(); runCronChangedMock.mockClear(); getGlobalHookRunnerMock.mockClear(); abortAndDrainEmbeddedAgentRunMock.mockClear(); @@ -2563,7 +2546,6 @@ describe("buildGatewayCronService", () => { const options = expectIsolatedRunFields({ sessionKey }); expect(requireRecord(options.job, "isolated job").id).toBe(job.id); - expectCleanupForSessionKeys([sessionKey]); } finally { state.cron.stop(); } @@ -2604,7 +2586,6 @@ describe("buildGatewayCronService", () => { return record.sessionKey === "main"; }), ).toBe(false); - expectCleanupForSessionKeys([`cron:${job.id}`]); } finally { state.cron.stop(); } diff --git a/src/gateway/server-cron.ts b/src/gateway/server-cron.ts index 5abdeb66404e..1b8ca3bd835a 100644 --- a/src/gateway/server-cron.ts +++ b/src/gateway/server-cron.ts @@ -5,7 +5,6 @@ import { isAgentDeletionBlocked } from "../agents/agent-lifecycle-registry.js"; import { listAgentEntries, listAgentIds, resolveDefaultAgentId } from "../agents/agent-scope.js"; import { abortAndDrainEmbeddedAgentRun } from "../agents/embedded-agent.js"; import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; -import { cleanupBrowserSessionsForLifecycleEnd } from "../browser-lifecycle-cleanup.js"; import type { CliDeps } from "../cli/deps.types.js"; import { getRuntimeConfig } from "../config/io.js"; import { @@ -721,26 +720,19 @@ export function buildGatewayCronService(params: { }) => { const { agentId, cfg: runtimeConfig } = resolveCronAgent(job.agentId); const sessionKey = resolveCronSessionTargetSessionKey(job.sessionTarget) ?? `cron:${job.id}`; - try { - return await runCronIsolatedAgentTurn({ - cfg: runtimeConfig, - deps: params.deps, - job, - message, - abortSignal, - onExecutionStarted, - onExecutionPhase, - onLaneWait, - agentId, - sessionKey, - lane: "cron", - }); - } finally { - await cleanupBrowserSessionsForLifecycleEnd({ - sessionKeys: [sessionKey], - onWarn: (msg) => cronLogger.warn({ jobId: job.id }, msg), - }); - } + return await runCronIsolatedAgentTurn({ + cfg: runtimeConfig, + deps: params.deps, + job, + message, + abortSignal, + onExecutionStarted, + onExecutionPhase, + onLaneWait, + agentId, + sessionKey, + lane: "cron", + }); }, runCommandJob: async ({ job, abortSignal }) => { const result = await runCronCommandJob({