mirror of https://github.com/openclaw/openclaw.git
refactor(browser): share lifecycle cleanup helpers
This commit is contained in:
parent
c3f415ad6e
commit
605f48556b
|
|
@ -26,6 +26,10 @@ import {
|
|||
CDP_READY_AFTER_LAUNCH_POLL_MS,
|
||||
CDP_READY_AFTER_LAUNCH_WINDOW_MS,
|
||||
} from "./server-context.constants.js";
|
||||
import {
|
||||
closePlaywrightBrowserConnectionForProfile,
|
||||
resolveIdleProfileStopOutcome,
|
||||
} from "./server-context.lifecycle.js";
|
||||
import type {
|
||||
BrowserServerState,
|
||||
ContextOptions,
|
||||
|
|
@ -100,15 +104,6 @@ export function createProfileAvailability({
|
|||
});
|
||||
};
|
||||
|
||||
const closePlaywrightBrowserConnectionForProfile = async (cdpUrl?: string): Promise<void> => {
|
||||
try {
|
||||
const mod = await import("./pw-ai.js");
|
||||
await mod.closePlaywrightBrowserConnection(cdpUrl ? { cdpUrl } : undefined);
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
};
|
||||
|
||||
const reconcileProfileRuntime = async (): Promise<void> => {
|
||||
const profileState = getProfileState();
|
||||
const reconcile = profileState.reconcile;
|
||||
|
|
@ -277,15 +272,13 @@ export function createProfileAvailability({
|
|||
}
|
||||
const profileState = getProfileState();
|
||||
if (!profileState.running) {
|
||||
const remoteCdp = capabilities.isRemote;
|
||||
if (profile.attachOnly || remoteCdp) {
|
||||
// No process was launched for attachOnly/remote profiles, but a Playwright
|
||||
// CDP connection may still be active. Close it so emulation overrides
|
||||
// (prefers-color-scheme, etc.) are released.
|
||||
const idleStop = resolveIdleProfileStopOutcome(profile);
|
||||
if (idleStop.closePlaywright) {
|
||||
// No process was launched for attachOnly/remote profiles, but a cached
|
||||
// Playwright CDP connection may still be active and holding emulation state.
|
||||
await closePlaywrightBrowserConnectionForProfile(profile.cdpUrl);
|
||||
return { stopped: true };
|
||||
}
|
||||
return { stopped: false };
|
||||
return { stopped: idleStop.stopped };
|
||||
}
|
||||
await stopOpenClawChrome(profileState.running);
|
||||
setProfileRunning(null);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,3 @@
|
|||
import type { ChildProcessWithoutNullStreams } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import "./server-context.chrome-test-harness.js";
|
||||
import {
|
||||
|
|
@ -7,55 +5,8 @@ import {
|
|||
PROFILE_HTTP_REACHABILITY_TIMEOUT_MS,
|
||||
} from "./cdp-timeouts.js";
|
||||
import * as chromeModule from "./chrome.js";
|
||||
import type { RunningChrome } from "./chrome.js";
|
||||
import type { BrowserServerState } from "./server-context.js";
|
||||
import { createBrowserRouteContext } from "./server-context.js";
|
||||
|
||||
function makeBrowserState(): BrowserServerState {
|
||||
return {
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
server: null as any,
|
||||
port: 0,
|
||||
resolved: {
|
||||
enabled: true,
|
||||
controlPort: 18791,
|
||||
cdpProtocol: "http",
|
||||
cdpHost: "127.0.0.1",
|
||||
cdpIsLoopback: true,
|
||||
cdpPortRangeStart: 18800,
|
||||
cdpPortRangeEnd: 18810,
|
||||
evaluateEnabled: false,
|
||||
remoteCdpTimeoutMs: 1500,
|
||||
remoteCdpHandshakeTimeoutMs: 3000,
|
||||
extraArgs: [],
|
||||
color: "#FF4500",
|
||||
headless: true,
|
||||
noSandbox: false,
|
||||
attachOnly: false,
|
||||
ssrfPolicy: { allowPrivateNetwork: true },
|
||||
defaultProfile: "openclaw",
|
||||
profiles: {
|
||||
openclaw: { cdpPort: 18800, color: "#FF4500" },
|
||||
},
|
||||
},
|
||||
profiles: new Map(),
|
||||
};
|
||||
}
|
||||
|
||||
function mockLaunchedChrome(
|
||||
launchOpenClawChrome: { mockResolvedValue: (value: RunningChrome) => unknown },
|
||||
pid: number,
|
||||
) {
|
||||
const proc = new EventEmitter() as unknown as ChildProcessWithoutNullStreams;
|
||||
launchOpenClawChrome.mockResolvedValue({
|
||||
pid,
|
||||
exe: { kind: "chromium", path: "/usr/bin/chromium" },
|
||||
userDataDir: "/tmp/openclaw-test",
|
||||
cdpPort: 18800,
|
||||
startedAt: Date.now(),
|
||||
proc,
|
||||
});
|
||||
}
|
||||
import { makeBrowserServerState, mockLaunchedChrome } from "./server-context.test-harness.js";
|
||||
|
||||
function setupEnsureBrowserAvailableHarness() {
|
||||
vi.useFakeTimers();
|
||||
|
|
@ -66,7 +17,7 @@ function setupEnsureBrowserAvailableHarness() {
|
|||
const isChromeCdpReady = vi.mocked(chromeModule.isChromeCdpReady);
|
||||
isChromeReachable.mockResolvedValue(false);
|
||||
|
||||
const state = makeBrowserState();
|
||||
const state = makeBrowserServerState();
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const profile = ctx.forProfile("openclaw");
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,35 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { resolveIdleProfileStopOutcome } from "./server-context.lifecycle.js";
|
||||
import { makeBrowserProfile } from "./server-context.test-harness.js";
|
||||
|
||||
describe("resolveIdleProfileStopOutcome", () => {
|
||||
it("treats attachOnly profiles as stopped via Playwright cleanup", () => {
|
||||
expect(resolveIdleProfileStopOutcome(makeBrowserProfile({ attachOnly: true }))).toEqual({
|
||||
stopped: true,
|
||||
closePlaywright: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("treats remote CDP profiles as stopped via Playwright cleanup", () => {
|
||||
expect(
|
||||
resolveIdleProfileStopOutcome(
|
||||
makeBrowserProfile({
|
||||
cdpUrl: "http://10.0.0.5:9222",
|
||||
cdpHost: "10.0.0.5",
|
||||
cdpIsLoopback: false,
|
||||
cdpPort: 9222,
|
||||
}),
|
||||
),
|
||||
).toEqual({
|
||||
stopped: true,
|
||||
closePlaywright: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps never-started managed profiles as not stopped", () => {
|
||||
expect(resolveIdleProfileStopOutcome(makeBrowserProfile())).toEqual({
|
||||
stopped: false,
|
||||
closePlaywright: false,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
import type { ResolvedBrowserProfile } from "./config.js";
|
||||
import { getBrowserProfileCapabilities } from "./profile-capabilities.js";
|
||||
|
||||
export function resolveIdleProfileStopOutcome(profile: ResolvedBrowserProfile): {
|
||||
stopped: boolean;
|
||||
closePlaywright: boolean;
|
||||
} {
|
||||
const capabilities = getBrowserProfileCapabilities(profile);
|
||||
if (profile.attachOnly || capabilities.isRemote) {
|
||||
return {
|
||||
stopped: true,
|
||||
closePlaywright: true,
|
||||
};
|
||||
}
|
||||
return {
|
||||
stopped: false,
|
||||
closePlaywright: false,
|
||||
};
|
||||
}
|
||||
|
||||
export async function closePlaywrightBrowserConnectionForProfile(cdpUrl?: string): Promise<void> {
|
||||
try {
|
||||
const mod = await import("./pw-ai.js");
|
||||
await mod.closePlaywrightBrowserConnection(cdpUrl ? { cdpUrl } : undefined);
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
}
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createProfileAvailability } from "./server-context.availability.js";
|
||||
import type { BrowserServerState, ProfileRuntimeState } from "./server-context.types.js";
|
||||
import { createBrowserRouteContext } from "./server-context.js";
|
||||
import { makeBrowserProfile, makeBrowserServerState } from "./server-context.test-harness.js";
|
||||
|
||||
const pwAiMocks = vi.hoisted(() => ({
|
||||
closePlaywrightBrowserConnection: vi.fn(async () => {}),
|
||||
|
|
@ -13,6 +13,7 @@ vi.mock("./chrome.js", () => ({
|
|||
launchOpenClawChrome: vi.fn(async () => {
|
||||
throw new Error("unexpected launch");
|
||||
}),
|
||||
resolveOpenClawUserDataDir: vi.fn(() => "/tmp/openclaw-test"),
|
||||
stopOpenClawChrome: vi.fn(async () => {}),
|
||||
}));
|
||||
vi.mock("./chrome-mcp.js", () => ({
|
||||
|
|
@ -25,108 +26,48 @@ afterEach(() => {
|
|||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
function makeProfile(
|
||||
overrides: Partial<Parameters<typeof createProfileAvailability>[0]["profile"]> = {},
|
||||
): Parameters<typeof createProfileAvailability>[0]["profile"] {
|
||||
return {
|
||||
name: "openclaw",
|
||||
cdpUrl: "http://127.0.0.1:18800",
|
||||
cdpHost: "127.0.0.1",
|
||||
cdpIsLoopback: true,
|
||||
cdpPort: 18800,
|
||||
color: "#f60",
|
||||
driver: "openclaw",
|
||||
attachOnly: false,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeState(
|
||||
profile: Parameters<typeof createProfileAvailability>[0]["profile"],
|
||||
): BrowserServerState {
|
||||
return {
|
||||
server: null,
|
||||
port: 0,
|
||||
resolved: {
|
||||
enabled: true,
|
||||
evaluateEnabled: false,
|
||||
controlPort: 18791,
|
||||
cdpProtocol: "http",
|
||||
cdpHost: profile.cdpHost,
|
||||
cdpIsLoopback: profile.cdpIsLoopback,
|
||||
cdpPortRangeStart: 18800,
|
||||
cdpPortRangeEnd: 18810,
|
||||
remoteCdpTimeoutMs: 1500,
|
||||
remoteCdpHandshakeTimeoutMs: 3000,
|
||||
extraArgs: [],
|
||||
color: profile.color,
|
||||
headless: true,
|
||||
noSandbox: false,
|
||||
attachOnly: false,
|
||||
function createStopHarness(profile: ReturnType<typeof makeBrowserProfile>) {
|
||||
const state = makeBrowserServerState({
|
||||
profile,
|
||||
resolvedOverrides: {
|
||||
ssrfPolicy: { dangerouslyAllowPrivateNetwork: true },
|
||||
defaultProfile: profile.name,
|
||||
profiles: {
|
||||
[profile.name]: profile,
|
||||
},
|
||||
},
|
||||
profiles: new Map(),
|
||||
};
|
||||
}
|
||||
|
||||
function createStopHarness(profile: Parameters<typeof createProfileAvailability>[0]["profile"]) {
|
||||
const state = makeState(profile);
|
||||
const runtimeState: ProfileRuntimeState = {
|
||||
profile,
|
||||
running: null,
|
||||
lastTargetId: null,
|
||||
reconcile: null,
|
||||
};
|
||||
state.profiles.set(profile.name, runtimeState);
|
||||
|
||||
const ops = createProfileAvailability({
|
||||
opts: { getState: () => state },
|
||||
profile,
|
||||
state: () => state,
|
||||
getProfileState: () => runtimeState,
|
||||
setProfileRunning: (running) => {
|
||||
runtimeState.running = running;
|
||||
},
|
||||
});
|
||||
|
||||
return { ops };
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
return { profileCtx: ctx.forProfile(profile.name) };
|
||||
}
|
||||
|
||||
describe("createProfileAvailability.stopRunningBrowser", () => {
|
||||
it("disconnects attachOnly loopback profiles without an owned process", async () => {
|
||||
const profile = makeProfile({ attachOnly: true });
|
||||
const { ops } = createStopHarness(profile);
|
||||
const profile = makeBrowserProfile({ attachOnly: true });
|
||||
const { profileCtx } = createStopHarness(profile);
|
||||
|
||||
await expect(ops.stopRunningBrowser()).resolves.toEqual({ stopped: true });
|
||||
await expect(profileCtx.stopRunningBrowser()).resolves.toEqual({ stopped: true });
|
||||
expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenCalledWith({
|
||||
cdpUrl: "http://127.0.0.1:18800",
|
||||
});
|
||||
});
|
||||
|
||||
it("disconnects remote CDP profiles without an owned process", async () => {
|
||||
const profile = makeProfile({
|
||||
const profile = makeBrowserProfile({
|
||||
cdpUrl: "http://10.0.0.5:9222",
|
||||
cdpHost: "10.0.0.5",
|
||||
cdpIsLoopback: false,
|
||||
cdpPort: 9222,
|
||||
});
|
||||
const { ops } = createStopHarness(profile);
|
||||
const { profileCtx } = createStopHarness(profile);
|
||||
|
||||
await expect(ops.stopRunningBrowser()).resolves.toEqual({ stopped: true });
|
||||
await expect(profileCtx.stopRunningBrowser()).resolves.toEqual({ stopped: true });
|
||||
expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenCalledWith({
|
||||
cdpUrl: "http://10.0.0.5:9222",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps never-started local managed profiles as not stopped", async () => {
|
||||
const profile = makeProfile();
|
||||
const { ops } = createStopHarness(profile);
|
||||
const profile = makeBrowserProfile();
|
||||
const { profileCtx } = createStopHarness(profile);
|
||||
|
||||
await expect(ops.stopRunningBrowser()).resolves.toEqual({ stopped: false });
|
||||
await expect(profileCtx.stopRunningBrowser()).resolves.toEqual({ stopped: false });
|
||||
expect(pwAiMocks.closePlaywrightBrowserConnection).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,72 @@
|
|||
import type { ChildProcessWithoutNullStreams } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { RunningChrome } from "./chrome.js";
|
||||
import type { ResolvedBrowserProfile } from "./config.js";
|
||||
import type { BrowserServerState } from "./server-context.js";
|
||||
|
||||
export function makeBrowserProfile(
|
||||
overrides: Partial<ResolvedBrowserProfile> = {},
|
||||
): ResolvedBrowserProfile {
|
||||
return {
|
||||
name: "openclaw",
|
||||
cdpUrl: "http://127.0.0.1:18800",
|
||||
cdpHost: "127.0.0.1",
|
||||
cdpIsLoopback: true,
|
||||
cdpPort: 18800,
|
||||
color: "#FF4500",
|
||||
driver: "openclaw",
|
||||
attachOnly: false,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
export function makeBrowserServerState(params?: {
|
||||
profile?: ResolvedBrowserProfile;
|
||||
resolvedOverrides?: Partial<BrowserServerState["resolved"]>;
|
||||
}): BrowserServerState {
|
||||
const profile = params?.profile ?? makeBrowserProfile();
|
||||
return {
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
server: null as any,
|
||||
port: 0,
|
||||
resolved: {
|
||||
enabled: true,
|
||||
controlPort: 18791,
|
||||
cdpProtocol: "http",
|
||||
cdpHost: profile.cdpHost,
|
||||
cdpIsLoopback: profile.cdpIsLoopback,
|
||||
cdpPortRangeStart: 18800,
|
||||
cdpPortRangeEnd: 18810,
|
||||
evaluateEnabled: false,
|
||||
remoteCdpTimeoutMs: 1500,
|
||||
remoteCdpHandshakeTimeoutMs: 3000,
|
||||
extraArgs: [],
|
||||
color: profile.color,
|
||||
headless: true,
|
||||
noSandbox: false,
|
||||
attachOnly: false,
|
||||
ssrfPolicy: { allowPrivateNetwork: true },
|
||||
defaultProfile: profile.name,
|
||||
profiles: {
|
||||
[profile.name]: profile,
|
||||
},
|
||||
...params?.resolvedOverrides,
|
||||
},
|
||||
profiles: new Map(),
|
||||
};
|
||||
}
|
||||
|
||||
export function mockLaunchedChrome(
|
||||
launchOpenClawChrome: { mockResolvedValue: (value: RunningChrome) => unknown },
|
||||
pid: number,
|
||||
) {
|
||||
const proc = new EventEmitter() as unknown as ChildProcessWithoutNullStreams;
|
||||
launchOpenClawChrome.mockResolvedValue({
|
||||
pid,
|
||||
exe: { kind: "chromium", path: "/usr/bin/chromium" },
|
||||
userDataDir: "/tmp/openclaw-test",
|
||||
cdpPort: 18800,
|
||||
startedAt: Date.now(),
|
||||
proc,
|
||||
});
|
||||
}
|
||||
|
|
@ -0,0 +1,65 @@
|
|||
import { readFileSync, writeFileSync } from "node:fs";
|
||||
import { resolve } from "node:path";
|
||||
import { appendUnreleasedChangelogEntry } from "../src/infra/changelog-unreleased.js";
|
||||
|
||||
type SectionArg = "breaking" | "changes" | "fixes";
|
||||
|
||||
function parseArgs(argv: string[]): {
|
||||
changelogPath: string;
|
||||
section: "Breaking" | "Changes" | "Fixes";
|
||||
entry: string;
|
||||
} {
|
||||
let changelogPath = resolve("CHANGELOG.md");
|
||||
let section: SectionArg | undefined;
|
||||
const entryParts: string[] = [];
|
||||
|
||||
for (let index = 0; index < argv.length; index += 1) {
|
||||
const arg = argv[index];
|
||||
if (arg === "--file") {
|
||||
const next = argv[index + 1];
|
||||
if (!next) {
|
||||
throw new Error("Missing value for --file.");
|
||||
}
|
||||
changelogPath = resolve(next);
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
if (arg === "--section") {
|
||||
const next = argv[index + 1] as SectionArg | undefined;
|
||||
if (!next || !["breaking", "changes", "fixes"].includes(next)) {
|
||||
throw new Error("Missing or invalid value for --section.");
|
||||
}
|
||||
section = next;
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
entryParts.push(arg);
|
||||
}
|
||||
|
||||
if (!section) {
|
||||
throw new Error("Missing required --section <breaking|changes|fixes>.");
|
||||
}
|
||||
const entry = entryParts.join(" ").trim();
|
||||
if (!entry) {
|
||||
throw new Error("Missing changelog entry text.");
|
||||
}
|
||||
|
||||
return {
|
||||
changelogPath,
|
||||
section: section === "breaking" ? "Breaking" : section === "changes" ? "Changes" : "Fixes",
|
||||
entry,
|
||||
};
|
||||
}
|
||||
|
||||
if (import.meta.main) {
|
||||
const { changelogPath, section, entry } = parseArgs(process.argv.slice(2));
|
||||
const content = readFileSync(changelogPath, "utf8");
|
||||
const next = appendUnreleasedChangelogEntry(content, {
|
||||
section,
|
||||
entry,
|
||||
});
|
||||
if (next !== content) {
|
||||
writeFileSync(changelogPath, next);
|
||||
}
|
||||
console.log(`Updated ${changelogPath} (${section}).`);
|
||||
}
|
||||
|
|
@ -17,8 +17,8 @@ const lifecycleEventMocks = vi.hoisted(() => ({
|
|||
emitSessionLifecycleEvent: vi.fn(),
|
||||
}));
|
||||
|
||||
const browserMaintenanceMocks = vi.hoisted(() => ({
|
||||
closeTrackedBrowserTabsForSessions: vi.fn(async () => 0),
|
||||
const browserLifecycleCleanupMocks = vi.hoisted(() => ({
|
||||
cleanupBrowserSessionsForLifecycleEnd: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
vi.mock("../tasks/task-executor.js", () => ({
|
||||
|
|
@ -31,8 +31,9 @@ vi.mock("../sessions/session-lifecycle-events.js", () => ({
|
|||
emitSessionLifecycleEvent: lifecycleEventMocks.emitSessionLifecycleEvent,
|
||||
}));
|
||||
|
||||
vi.mock("../plugin-sdk/browser-maintenance.js", () => ({
|
||||
closeTrackedBrowserTabsForSessions: browserMaintenanceMocks.closeTrackedBrowserTabsForSessions,
|
||||
vi.mock("../browser-lifecycle-cleanup.js", () => ({
|
||||
cleanupBrowserSessionsForLifecycleEnd:
|
||||
browserLifecycleCleanupMocks.cleanupBrowserSessionsForLifecycleEnd,
|
||||
}));
|
||||
|
||||
vi.mock("./subagent-registry-helpers.js", async () => {
|
||||
|
|
@ -66,7 +67,7 @@ describe("subagent registry lifecycle hardening", () => {
|
|||
beforeEach(async () => {
|
||||
vi.resetModules();
|
||||
vi.clearAllMocks();
|
||||
browserMaintenanceMocks.closeTrackedBrowserTabsForSessions.mockClear();
|
||||
browserLifecycleCleanupMocks.cleanupBrowserSessionsForLifecycleEnd.mockClear();
|
||||
mod = await import("./subagent-registry-lifecycle.js");
|
||||
});
|
||||
|
||||
|
|
@ -208,10 +209,12 @@ describe("subagent registry lifecycle hardening", () => {
|
|||
}),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(browserMaintenanceMocks.closeTrackedBrowserTabsForSessions).toHaveBeenCalledWith({
|
||||
sessionKeys: [entry.childSessionKey],
|
||||
onWarn: expect.any(Function),
|
||||
});
|
||||
expect(browserLifecycleCleanupMocks.cleanupBrowserSessionsForLifecycleEnd).toHaveBeenCalledWith(
|
||||
{
|
||||
sessionKeys: [entry.childSessionKey],
|
||||
onWarn: expect.any(Function),
|
||||
},
|
||||
);
|
||||
expect(runSubagentAnnounceFlow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
childSessionKey: entry.childSessionKey,
|
||||
|
|
@ -252,7 +255,9 @@ describe("subagent registry lifecycle hardening", () => {
|
|||
}),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(browserMaintenanceMocks.closeTrackedBrowserTabsForSessions).not.toHaveBeenCalled();
|
||||
expect(
|
||||
browserLifecycleCleanupMocks.cleanupBrowserSessionsForLifecycleEnd,
|
||||
).not.toHaveBeenCalled();
|
||||
expect(runSubagentAnnounceFlow).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js";
|
||||
import { cleanupBrowserSessionsForLifecycleEnd } from "../browser-lifecycle-cleanup.js";
|
||||
import { formatErrorMessage, readErrorName } from "../infra/errors.js";
|
||||
import { closeTrackedBrowserTabsForSessions } from "../plugin-sdk/browser-maintenance.js";
|
||||
import { defaultRuntime } from "../runtime.js";
|
||||
import { emitSessionLifecycleEvent } from "../sessions/session-lifecycle-events.js";
|
||||
import {
|
||||
|
|
@ -606,17 +606,10 @@ export function createSubagentRegistryLifecycleController(params: {
|
|||
return;
|
||||
}
|
||||
|
||||
// Clean up browser tabs/processes opened by this subagent session.
|
||||
// Without this, browser processes become orphaned after the subagent
|
||||
// exits. See #60104.
|
||||
try {
|
||||
await closeTrackedBrowserTabsForSessions({
|
||||
sessionKeys: [entry.childSessionKey],
|
||||
onWarn: (msg) => params.warn(msg, { runId: entry.runId }),
|
||||
});
|
||||
} catch {
|
||||
// Best-effort cleanup.
|
||||
}
|
||||
await cleanupBrowserSessionsForLifecycleEnd({
|
||||
sessionKeys: [entry.childSessionKey],
|
||||
onWarn: (msg) => params.warn(msg, { runId: entry.runId }),
|
||||
});
|
||||
|
||||
startSubagentAnnounceCleanupFlow(completeParams.runId, entry);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -0,0 +1,48 @@
|
|||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const closeTrackedBrowserTabsForSessions = vi.hoisted(() => vi.fn(async () => 0));
|
||||
|
||||
vi.mock("./plugin-sdk/browser-maintenance.js", () => ({
|
||||
closeTrackedBrowserTabsForSessions,
|
||||
}));
|
||||
|
||||
describe("cleanupBrowserSessionsForLifecycleEnd", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("normalizes session keys before closing browser sessions", async () => {
|
||||
const { cleanupBrowserSessionsForLifecycleEnd } =
|
||||
await import("./browser-lifecycle-cleanup.js");
|
||||
const onWarn = vi.fn();
|
||||
|
||||
await expect(
|
||||
cleanupBrowserSessionsForLifecycleEnd({
|
||||
sessionKeys: ["", " session-a ", "session-a", "session-b"],
|
||||
onWarn,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(closeTrackedBrowserTabsForSessions).toHaveBeenCalledWith({
|
||||
sessionKeys: ["session-a", "session-b"],
|
||||
onWarn,
|
||||
});
|
||||
});
|
||||
|
||||
it("swallows browser cleanup failures", async () => {
|
||||
const { cleanupBrowserSessionsForLifecycleEnd } =
|
||||
await import("./browser-lifecycle-cleanup.js");
|
||||
const onError = vi.fn();
|
||||
const error = new Error("cleanup failed");
|
||||
closeTrackedBrowserTabsForSessions.mockRejectedValueOnce(error);
|
||||
|
||||
await expect(
|
||||
cleanupBrowserSessionsForLifecycleEnd({
|
||||
sessionKeys: ["session-a"],
|
||||
onError,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(onError).toHaveBeenCalledWith(error);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,33 @@
|
|||
import { runBestEffortCleanup } from "./infra/non-fatal-cleanup.js";
|
||||
import { closeTrackedBrowserTabsForSessions } from "./plugin-sdk/browser-maintenance.js";
|
||||
|
||||
function normalizeSessionKeys(sessionKeys: string[]): string[] {
|
||||
const keys = new Set<string>();
|
||||
for (const sessionKey of sessionKeys) {
|
||||
const normalized = sessionKey.trim();
|
||||
if (normalized) {
|
||||
keys.add(normalized);
|
||||
}
|
||||
}
|
||||
return [...keys];
|
||||
}
|
||||
|
||||
export async function cleanupBrowserSessionsForLifecycleEnd(params: {
|
||||
sessionKeys: string[];
|
||||
onWarn?: (message: string) => void;
|
||||
onError?: (error: unknown) => void;
|
||||
}): Promise<void> {
|
||||
const sessionKeys = normalizeSessionKeys(params.sessionKeys);
|
||||
if (sessionKeys.length === 0) {
|
||||
return;
|
||||
}
|
||||
await runBestEffortCleanup({
|
||||
cleanup: async () => {
|
||||
await closeTrackedBrowserTabsForSessions({
|
||||
sessionKeys,
|
||||
onWarn: params.onWarn,
|
||||
});
|
||||
},
|
||||
onError: params.onError,
|
||||
});
|
||||
}
|
||||
|
|
@ -12,14 +12,14 @@ const {
|
|||
loadConfigMock,
|
||||
fetchWithSsrFGuardMock,
|
||||
runCronIsolatedAgentTurnMock,
|
||||
closeTrackedBrowserTabsForSessionsMock,
|
||||
cleanupBrowserSessionsForLifecycleEndMock,
|
||||
} = vi.hoisted(() => ({
|
||||
enqueueSystemEventMock: vi.fn(),
|
||||
requestHeartbeatNowMock: vi.fn(),
|
||||
loadConfigMock: vi.fn(),
|
||||
fetchWithSsrFGuardMock: vi.fn(),
|
||||
runCronIsolatedAgentTurnMock: vi.fn(async () => ({ status: "ok" as const, summary: "ok" })),
|
||||
closeTrackedBrowserTabsForSessionsMock: vi.fn(async () => 0),
|
||||
cleanupBrowserSessionsForLifecycleEndMock: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
function enqueueSystemEvent(...args: unknown[]) {
|
||||
|
|
@ -61,8 +61,8 @@ vi.mock("../cron/isolated-agent.js", () => ({
|
|||
runCronIsolatedAgentTurn: runCronIsolatedAgentTurnMock,
|
||||
}));
|
||||
|
||||
vi.mock("../plugin-sdk/browser-maintenance.js", () => ({
|
||||
closeTrackedBrowserTabsForSessions: closeTrackedBrowserTabsForSessionsMock,
|
||||
vi.mock("../browser-lifecycle-cleanup.js", () => ({
|
||||
cleanupBrowserSessionsForLifecycleEnd: cleanupBrowserSessionsForLifecycleEndMock,
|
||||
}));
|
||||
|
||||
import { buildGatewayCronService } from "./server-cron.js";
|
||||
|
|
@ -86,7 +86,7 @@ describe("buildGatewayCronService", () => {
|
|||
loadConfigMock.mockClear();
|
||||
fetchWithSsrFGuardMock.mockClear();
|
||||
runCronIsolatedAgentTurnMock.mockClear();
|
||||
closeTrackedBrowserTabsForSessionsMock.mockClear();
|
||||
cleanupBrowserSessionsForLifecycleEndMock.mockClear();
|
||||
});
|
||||
|
||||
it("routes main-target jobs to the scoped session for enqueue + wake", async () => {
|
||||
|
|
@ -207,7 +207,7 @@ describe("buildGatewayCronService", () => {
|
|||
sessionKey: "project-alpha-monitor",
|
||||
}),
|
||||
);
|
||||
expect(closeTrackedBrowserTabsForSessionsMock).toHaveBeenCalledWith({
|
||||
expect(cleanupBrowserSessionsForLifecycleEndMock).toHaveBeenCalledWith({
|
||||
sessionKeys: ["project-alpha-monitor"],
|
||||
onWarn: expect.any(Function),
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { resolveDefaultAgentId } from "../agents/agent-scope.js";
|
||||
import { cleanupBrowserSessionsForLifecycleEnd } from "../browser-lifecycle-cleanup.js";
|
||||
import type { CliDeps } from "../cli/deps.js";
|
||||
import { createOutboundSendDeps } from "../cli/outbound-send-deps.js";
|
||||
import { loadConfig } from "../config/config.js";
|
||||
|
|
@ -11,7 +12,6 @@ import { resolveStorePath } from "../config/sessions/paths.js";
|
|||
import { resolveFailureDestination, sendFailureNotificationAnnounce } from "../cron/delivery.js";
|
||||
import { runCronIsolatedAgentTurn } from "../cron/isolated-agent.js";
|
||||
import { resolveDeliveryTarget } from "../cron/isolated-agent/delivery-target.js";
|
||||
import { closeTrackedBrowserTabsForSessions } from "../plugin-sdk/browser-maintenance.js";
|
||||
import {
|
||||
appendCronRunLog,
|
||||
resolveCronRunLogPath,
|
||||
|
|
@ -302,18 +302,10 @@ export function buildGatewayCronService(params: {
|
|||
lane: "cron",
|
||||
});
|
||||
} finally {
|
||||
// Clean up browser tabs/processes opened during this cron run.
|
||||
// Without this, browser processes become orphaned (PPID=1) after
|
||||
// the cron task completes. See #60104.
|
||||
try {
|
||||
await closeTrackedBrowserTabsForSessions({
|
||||
sessionKeys: [sessionKey],
|
||||
onWarn: (msg) => cronLogger.warn({ jobId: job.id }, msg),
|
||||
});
|
||||
} catch {
|
||||
// Best-effort cleanup — do not let browser cleanup failures
|
||||
// mask the actual cron run result.
|
||||
}
|
||||
await cleanupBrowserSessionsForLifecycleEnd({
|
||||
sessionKeys: [sessionKey],
|
||||
onWarn: (msg) => cronLogger.warn({ jobId: job.id }, msg),
|
||||
});
|
||||
}
|
||||
},
|
||||
sendCronFailureAlert: async ({ job, text, channel, to, mode, accountId }) => {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,55 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { appendUnreleasedChangelogEntry } from "./changelog-unreleased.js";
|
||||
|
||||
const baseChangelog = `# Changelog
|
||||
|
||||
## Unreleased
|
||||
|
||||
### Breaking
|
||||
|
||||
- Existing breaking entry.
|
||||
|
||||
### Changes
|
||||
|
||||
- Existing change.
|
||||
|
||||
### Fixes
|
||||
|
||||
- Existing fix.
|
||||
|
||||
## 2026.4.4
|
||||
`;
|
||||
|
||||
describe("appendUnreleasedChangelogEntry", () => {
|
||||
it("appends to the end of the requested unreleased section", () => {
|
||||
const next = appendUnreleasedChangelogEntry(baseChangelog, {
|
||||
section: "Fixes",
|
||||
entry: "New fix entry.",
|
||||
});
|
||||
|
||||
expect(next).toContain(`### Fixes
|
||||
|
||||
- Existing fix.
|
||||
- New fix entry.
|
||||
|
||||
## 2026.4.4`);
|
||||
});
|
||||
|
||||
it("avoids duplicating an existing entry", () => {
|
||||
const next = appendUnreleasedChangelogEntry(baseChangelog, {
|
||||
section: "Changes",
|
||||
entry: "- Existing change.",
|
||||
});
|
||||
|
||||
expect(next).toBe(baseChangelog);
|
||||
});
|
||||
|
||||
it("throws when the unreleased section is missing", () => {
|
||||
expect(() =>
|
||||
appendUnreleasedChangelogEntry("# Changelog\n", {
|
||||
section: "Fixes",
|
||||
entry: "New fix entry.",
|
||||
}),
|
||||
).toThrow("## Unreleased");
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,68 @@
|
|||
type UnreleasedSection = "Breaking" | "Changes" | "Fixes";
|
||||
|
||||
function findSectionRange(
|
||||
lines: string[],
|
||||
section: UnreleasedSection,
|
||||
): {
|
||||
start: number;
|
||||
insertAt: number;
|
||||
} {
|
||||
const unreleasedIndex = lines.findIndex((line) => line.trim() === "## Unreleased");
|
||||
if (unreleasedIndex === -1) {
|
||||
throw new Error("CHANGELOG.md is missing the '## Unreleased' heading.");
|
||||
}
|
||||
|
||||
const sectionHeading = `### ${section}`;
|
||||
let sectionIndex = -1;
|
||||
for (let index = unreleasedIndex + 1; index < lines.length; index += 1) {
|
||||
const line = lines[index];
|
||||
if (line.startsWith("## ")) {
|
||||
break;
|
||||
}
|
||||
if (line.trim() === sectionHeading) {
|
||||
sectionIndex = index;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (sectionIndex === -1) {
|
||||
throw new Error(`CHANGELOG.md is missing the '${sectionHeading}' section under Unreleased.`);
|
||||
}
|
||||
|
||||
let insertAt = lines.length;
|
||||
for (let index = sectionIndex + 1; index < lines.length; index += 1) {
|
||||
const line = lines[index];
|
||||
if (line.startsWith("### ") || line.startsWith("## ")) {
|
||||
insertAt = index;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
while (insertAt > sectionIndex + 1 && lines[insertAt - 1]?.trim() === "") {
|
||||
insertAt -= 1;
|
||||
}
|
||||
|
||||
return { start: sectionIndex, insertAt };
|
||||
}
|
||||
|
||||
export function appendUnreleasedChangelogEntry(
|
||||
content: string,
|
||||
params: {
|
||||
section: UnreleasedSection;
|
||||
entry: string;
|
||||
},
|
||||
): string {
|
||||
const entry = params.entry.trim();
|
||||
if (!entry) {
|
||||
throw new Error("Changelog entry must not be empty.");
|
||||
}
|
||||
|
||||
const lines = content.split("\n");
|
||||
const bullet = entry.startsWith("- ") ? entry : `- ${entry}`;
|
||||
if (lines.some((line) => line.trim() === bullet)) {
|
||||
return content;
|
||||
}
|
||||
|
||||
const { insertAt } = findSectionRange(lines, params.section);
|
||||
lines.splice(insertAt, 0, bullet, "");
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
import { describe, expect, it, vi } from "vitest";
|
||||
import { runBestEffortCleanup } from "./non-fatal-cleanup.js";
|
||||
|
||||
describe("runBestEffortCleanup", () => {
|
||||
it("returns the cleanup result when the cleanup succeeds", async () => {
|
||||
await expect(
|
||||
runBestEffortCleanup({
|
||||
cleanup: async () => 7,
|
||||
}),
|
||||
).resolves.toBe(7);
|
||||
});
|
||||
|
||||
it("swallows cleanup failures and reports them through onError", async () => {
|
||||
const onError = vi.fn();
|
||||
const error = new Error("cleanup failed");
|
||||
|
||||
await expect(
|
||||
runBestEffortCleanup({
|
||||
cleanup: async () => {
|
||||
throw error;
|
||||
},
|
||||
onError,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(onError).toHaveBeenCalledWith(error);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
export async function runBestEffortCleanup<T>(params: {
|
||||
cleanup: () => Promise<T>;
|
||||
onError?: (error: unknown) => void;
|
||||
}): Promise<T | undefined> {
|
||||
try {
|
||||
return await params.cleanup();
|
||||
} catch (error) {
|
||||
params.onError?.(error);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue