mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(cron): close browser tabs after isolated runs (#113566)
* fix(cron): clean browser tabs by run session --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
@@ -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");
|
||||
|
||||
@@ -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,
|
||||
}));
|
||||
|
||||
@@ -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}`),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>) {
|
||||
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();
|
||||
}
|
||||
|
||||
+13
-21
@@ -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({
|
||||
|
||||
Reference in New Issue
Block a user