Skip to content

Commit 3b09b58

Browse files
committed
test: cover browser cleanup for cron and subagents (openclaw#60146) (thanks @BrianWang1990)
1 parent e697838 commit 3b09b58

3 files changed

Lines changed: 103 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ Docs: https://docs.openclaw.ai
153153
- Browser/host inspection: keep static Chrome inspection helpers out of the activated browser runtime so `openclaw doctor browser` and related checks do not eagerly load the bundled browser plugin. (#59471) Thanks @vincentkoc.
154154
- Browser/CDP: normalize trailing-dot localhost absolute-form hosts before loopback checks so remote CDP websocket URLs like `ws://localhost.:...` rewrite back to the configured remote host. (#59236) Thanks @mappel-nv.
155155
- Browser/attach-only profiles: disconnect cached Playwright CDP sessions when stopping attach-only or remote CDP profiles, while still reporting never-started local managed profiles as not stopped. (#60097) Thanks @pedh.
156+
- Browser/task cleanup: close tracked browser tabs and best-effort browser processes when cron-isolated agents and subagents finish, so background browser runs stop leaking orphaned sessions. (#60146) Thanks @BrianWang1990.
156157
- Agents/output sanitization: strip namespaced `antml:thinking` blocks from user-visible text so Anthropic-style internal monologue tags do not leak into replies. (#59550) Thanks @obviyus.
157158
- Kimi Coding/tools: normalize Anthropic tool payloads into the OpenAI-compatible function shape Kimi Coding expects so tool calls stop losing required arguments. (#59440) Thanks @obviyus.
158159
- Image tool/paths: resolve relative local media paths against the agent `workspaceDir` instead of `process.cwd()` so inputs like `inbox/receipt.png` pass the local-path allowlist reliably. (#57222) Thanks Priyansh Gupta.

src/agents/subagent-registry-lifecycle.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ const lifecycleEventMocks = vi.hoisted(() => ({
1717
emitSessionLifecycleEvent: vi.fn(),
1818
}));
1919

20+
const browserMaintenanceMocks = vi.hoisted(() => ({
21+
closeTrackedBrowserTabsForSessions: vi.fn(async () => 0),
22+
}));
23+
2024
vi.mock("../tasks/task-executor.js", () => ({
2125
completeTaskRunByRunId: taskExecutorMocks.completeTaskRunByRunId,
2226
failTaskRunByRunId: taskExecutorMocks.failTaskRunByRunId,
@@ -27,6 +31,10 @@ vi.mock("../sessions/session-lifecycle-events.js", () => ({
2731
emitSessionLifecycleEvent: lifecycleEventMocks.emitSessionLifecycleEvent,
2832
}));
2933

34+
vi.mock("../plugin-sdk/browser-maintenance.js", () => ({
35+
closeTrackedBrowserTabsForSessions: browserMaintenanceMocks.closeTrackedBrowserTabsForSessions,
36+
}));
37+
3038
vi.mock("./subagent-registry-helpers.js", async () => {
3139
const actual = await vi.importActual<typeof import("./subagent-registry-helpers.js")>(
3240
"./subagent-registry-helpers.js",
@@ -58,6 +66,7 @@ describe("subagent registry lifecycle hardening", () => {
5866
beforeEach(async () => {
5967
vi.resetModules();
6068
vi.clearAllMocks();
69+
browserMaintenanceMocks.closeTrackedBrowserTabsForSessions.mockClear();
6170
mod = await import("./subagent-registry-lifecycle.js");
6271
});
6372

@@ -164,4 +173,86 @@ describe("subagent registry lifecycle hardening", () => {
164173
expect(entry.cleanupCompletedAt).toBeTypeOf("number");
165174
expect(persist).toHaveBeenCalled();
166175
});
176+
177+
it("cleans up tracked browser sessions before subagent cleanup flow", async () => {
178+
const persist = vi.fn();
179+
const entry = createRunEntry({
180+
expectsCompletionMessage: false,
181+
});
182+
const runSubagentAnnounceFlow = vi.fn(async () => true);
183+
184+
const controller = mod.createSubagentRegistryLifecycleController({
185+
runs: new Map([[entry.runId, entry]]),
186+
resumedRuns: new Set(),
187+
subagentAnnounceTimeoutMs: 1_000,
188+
persist,
189+
clearPendingLifecycleError: vi.fn(),
190+
countPendingDescendantRuns: () => 0,
191+
suppressAnnounceForSteerRestart: () => false,
192+
shouldEmitEndedHookForRun: () => false,
193+
emitSubagentEndedHookForRun: vi.fn(async () => {}),
194+
notifyContextEngineSubagentEnded: vi.fn(async () => {}),
195+
resumeSubagentRun: vi.fn(),
196+
captureSubagentCompletionReply: vi.fn(async () => "final completion reply"),
197+
runSubagentAnnounceFlow,
198+
warn: vi.fn(),
199+
});
200+
201+
await expect(
202+
controller.completeSubagentRun({
203+
runId: entry.runId,
204+
endedAt: 4_000,
205+
outcome: { status: "ok" },
206+
reason: SUBAGENT_ENDED_REASON_COMPLETE,
207+
triggerCleanup: true,
208+
}),
209+
).resolves.toBeUndefined();
210+
211+
expect(browserMaintenanceMocks.closeTrackedBrowserTabsForSessions).toHaveBeenCalledWith({
212+
sessionKeys: [entry.childSessionKey],
213+
onWarn: expect.any(Function),
214+
});
215+
expect(runSubagentAnnounceFlow).toHaveBeenCalledWith(
216+
expect.objectContaining({
217+
childSessionKey: entry.childSessionKey,
218+
}),
219+
);
220+
});
221+
222+
it("skips browser cleanup when steer restart suppresses cleanup flow", async () => {
223+
const entry = createRunEntry({
224+
expectsCompletionMessage: false,
225+
});
226+
const runSubagentAnnounceFlow = vi.fn(async () => true);
227+
228+
const controller = mod.createSubagentRegistryLifecycleController({
229+
runs: new Map([[entry.runId, entry]]),
230+
resumedRuns: new Set(),
231+
subagentAnnounceTimeoutMs: 1_000,
232+
persist: vi.fn(),
233+
clearPendingLifecycleError: vi.fn(),
234+
countPendingDescendantRuns: () => 0,
235+
suppressAnnounceForSteerRestart: () => true,
236+
shouldEmitEndedHookForRun: () => false,
237+
emitSubagentEndedHookForRun: vi.fn(async () => {}),
238+
notifyContextEngineSubagentEnded: vi.fn(async () => {}),
239+
resumeSubagentRun: vi.fn(),
240+
captureSubagentCompletionReply: vi.fn(async () => "final completion reply"),
241+
runSubagentAnnounceFlow,
242+
warn: vi.fn(),
243+
});
244+
245+
await expect(
246+
controller.completeSubagentRun({
247+
runId: entry.runId,
248+
endedAt: 4_000,
249+
outcome: { status: "ok" },
250+
reason: SUBAGENT_ENDED_REASON_COMPLETE,
251+
triggerCleanup: true,
252+
}),
253+
).resolves.toBeUndefined();
254+
255+
expect(browserMaintenanceMocks.closeTrackedBrowserTabsForSessions).not.toHaveBeenCalled();
256+
expect(runSubagentAnnounceFlow).not.toHaveBeenCalled();
257+
});
167258
});

src/gateway/server-cron.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ const {
1212
loadConfigMock,
1313
fetchWithSsrFGuardMock,
1414
runCronIsolatedAgentTurnMock,
15+
closeTrackedBrowserTabsForSessionsMock,
1516
} = vi.hoisted(() => ({
1617
enqueueSystemEventMock: vi.fn(),
1718
requestHeartbeatNowMock: vi.fn(),
1819
loadConfigMock: vi.fn(),
1920
fetchWithSsrFGuardMock: vi.fn(),
2021
runCronIsolatedAgentTurnMock: vi.fn(async () => ({ status: "ok" as const, summary: "ok" })),
22+
closeTrackedBrowserTabsForSessionsMock: vi.fn(async () => 0),
2123
}));
2224

2325
function enqueueSystemEvent(...args: unknown[]) {
@@ -59,6 +61,10 @@ vi.mock("../cron/isolated-agent.js", () => ({
5961
runCronIsolatedAgentTurn: runCronIsolatedAgentTurnMock,
6062
}));
6163

64+
vi.mock("../plugin-sdk/browser-maintenance.js", () => ({
65+
closeTrackedBrowserTabsForSessions: closeTrackedBrowserTabsForSessionsMock,
66+
}));
67+
6268
import { buildGatewayCronService } from "./server-cron.js";
6369

6470
function createCronConfig(name: string): OpenClawConfig {
@@ -80,6 +86,7 @@ describe("buildGatewayCronService", () => {
8086
loadConfigMock.mockClear();
8187
fetchWithSsrFGuardMock.mockClear();
8288
runCronIsolatedAgentTurnMock.mockClear();
89+
closeTrackedBrowserTabsForSessionsMock.mockClear();
8390
});
8491

8592
it("routes main-target jobs to the scoped session for enqueue + wake", async () => {
@@ -200,6 +207,10 @@ describe("buildGatewayCronService", () => {
200207
sessionKey: "project-alpha-monitor",
201208
}),
202209
);
210+
expect(closeTrackedBrowserTabsForSessionsMock).toHaveBeenCalledWith({
211+
sessionKeys: ["project-alpha-monitor"],
212+
onWarn: expect.any(Function),
213+
});
203214
} finally {
204215
state.cron.stop();
205216
}

0 commit comments

Comments
 (0)