From bef620babe088234ad5fd0fe53827624b4e9deb4 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Fri, 13 Mar 2026 10:34:46 +0000 Subject: [PATCH] Matrix: stop one-off shared action clients --- .../matrix/src/matrix/actions/client.test.ts | 32 +++++++++++++------ .../matrix/src/matrix/client-bootstrap.ts | 19 ++++++++++- .../matrix/client-resolver.test-helpers.ts | 4 +++ .../matrix/src/matrix/send/client.test.ts | 19 ++++++++--- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/extensions/matrix/src/matrix/actions/client.test.ts b/extensions/matrix/src/matrix/actions/client.test.ts index 52f13787b42..20d9d03e816 100644 --- a/extensions/matrix/src/matrix/actions/client.test.ts +++ b/extensions/matrix/src/matrix/actions/client.test.ts @@ -12,6 +12,7 @@ const { getMatrixRuntimeMock, getActiveMatrixClientMock, resolveSharedMatrixClientMock, + stopSharedClientForAccountMock, isBunRuntimeMock, resolveMatrixAuthContextMock, } = matrixClientResolverMocks; @@ -30,6 +31,10 @@ vi.mock("../client.js", () => ({ resolveMatrixAuthContext: resolveMatrixAuthContextMock, })); +vi.mock("../client/shared.js", () => ({ + stopSharedClientForAccount: (...args: unknown[]) => stopSharedClientForAccountMock(...args), +})); + vi.mock("../send.js", () => ({ resolveMatrixRoomId: (...args: unknown[]) => resolveMatrixRoomIdMock(...args), })); @@ -54,7 +59,7 @@ describe("action client helpers", () => { vi.unstubAllEnvs(); }); - it("reuses the shared client pool when no active monitor client is registered", async () => { + it("stops one-off shared clients when no active monitor client is registered", async () => { vi.stubEnv("OPENCLAW_GATEWAY_PORT", "18799"); const result = await withResolvedActionClient({ accountId: "default" }, async () => "ok"); @@ -68,7 +73,10 @@ describe("action client helpers", () => { }); const sharedClient = await resolveSharedMatrixClientMock.mock.results[0]?.value; expect(sharedClient.prepareForOneOff).toHaveBeenCalledTimes(1); - expect(sharedClient.stop).not.toHaveBeenCalled(); + expect(sharedClient.stop).toHaveBeenCalledTimes(1); + expect(stopSharedClientForAccountMock).toHaveBeenCalledWith( + expect.objectContaining({ userId: "@bot:example.org" }), + ); expect(result).toBe("ok"); }); @@ -78,7 +86,7 @@ describe("action client helpers", () => { const sharedClient = await resolveSharedMatrixClientMock.mock.results[0]?.value; expect(sharedClient.prepareForOneOff).not.toHaveBeenCalled(); expect(sharedClient.start).not.toHaveBeenCalled(); - expect(sharedClient.stop).not.toHaveBeenCalled(); + expect(sharedClient.stop).toHaveBeenCalledTimes(1); }); it("starts one-off clients when started readiness is required", async () => { @@ -88,7 +96,10 @@ describe("action client helpers", () => { expect(sharedClient.start).toHaveBeenCalledTimes(1); expect(sharedClient.prepareForOneOff).not.toHaveBeenCalled(); expect(sharedClient.stop).not.toHaveBeenCalled(); - expect(sharedClient.stopAndPersist).not.toHaveBeenCalled(); + expect(sharedClient.stopAndPersist).toHaveBeenCalledTimes(1); + expect(stopSharedClientForAccountMock).toHaveBeenCalledWith( + expect.objectContaining({ userId: "@bot:example.org" }), + ); }); it("reuses active monitor client when available", async () => { @@ -178,7 +189,7 @@ describe("action client helpers", () => { }); }); - it("does not stop shared action clients after wrapped calls succeed", async () => { + it("stops shared action clients after wrapped calls succeed", async () => { const sharedClient = createMockMatrixClient(); resolveSharedMatrixClientMock.mockResolvedValue(sharedClient); @@ -188,11 +199,14 @@ describe("action client helpers", () => { }); expect(result).toBe("ok"); - expect(sharedClient.stop).not.toHaveBeenCalled(); + expect(sharedClient.stop).toHaveBeenCalledTimes(1); expect(sharedClient.stopAndPersist).not.toHaveBeenCalled(); + expect(stopSharedClientForAccountMock).toHaveBeenCalledWith( + expect.objectContaining({ userId: "@bot:example.org" }), + ); }); - it("keeps shared action clients alive when the wrapped call throws", async () => { + it("stops shared action clients when the wrapped call throws", async () => { const sharedClient = createMockMatrixClient(); resolveSharedMatrixClientMock.mockResolvedValue(sharedClient); @@ -202,7 +216,7 @@ describe("action client helpers", () => { }), ).rejects.toThrow("boom"); - expect(sharedClient.stop).not.toHaveBeenCalled(); + expect(sharedClient.stop).toHaveBeenCalledTimes(1); expect(sharedClient.stopAndPersist).not.toHaveBeenCalled(); }); @@ -222,6 +236,6 @@ describe("action client helpers", () => { expect(resolveMatrixRoomIdMock).toHaveBeenCalledWith(sharedClient, "room:#ops:example.org"); expect(result).toBe("!room:example.org"); - expect(sharedClient.stop).not.toHaveBeenCalled(); + expect(sharedClient.stop).toHaveBeenCalledTimes(1); }); }); diff --git a/extensions/matrix/src/matrix/client-bootstrap.ts b/extensions/matrix/src/matrix/client-bootstrap.ts index 8a435e94ecc..6a89239ea81 100644 --- a/extensions/matrix/src/matrix/client-bootstrap.ts +++ b/extensions/matrix/src/matrix/client-bootstrap.ts @@ -2,11 +2,13 @@ import { getMatrixRuntime } from "../runtime.js"; import type { CoreConfig } from "../types.js"; import { getActiveMatrixClient } from "./active-client.js"; import { isBunRuntime, resolveMatrixAuthContext, resolveSharedMatrixClient } from "./client.js"; +import { stopSharedClientForAccount } from "./client/shared.js"; import type { MatrixClient } from "./sdk.js"; type ResolvedRuntimeMatrixClient = { client: MatrixClient; stopOnDone: boolean; + cleanup?: (mode: ResolvedRuntimeMatrixClientStopMode) => Promise; }; type MatrixRuntimeClientReadiness = "none" | "prepared" | "started"; @@ -67,7 +69,18 @@ async function resolveRuntimeMatrixClient(opts: { accountId: authContext.accountId, }); await opts.onResolved?.(client, { preparedByDefault: true }); - return { client, stopOnDone: false }; + return { + client, + stopOnDone: true, + cleanup: async (mode) => { + if (mode === "persist") { + await client.stopAndPersist(); + } else { + client.stop(); + } + stopSharedClientForAccount(authContext.resolved); + }, + }; } export async function resolveRuntimeMatrixClientWithReadiness(opts: { @@ -99,6 +112,10 @@ export async function stopResolvedRuntimeMatrixClient( if (!resolved.stopOnDone) { return; } + if (resolved.cleanup) { + await resolved.cleanup(mode); + return; + } if (mode === "persist") { await resolved.client.stopAndPersist(); return; diff --git a/extensions/matrix/src/matrix/client-resolver.test-helpers.ts b/extensions/matrix/src/matrix/client-resolver.test-helpers.ts index 9b4256131e2..b4247cdfe99 100644 --- a/extensions/matrix/src/matrix/client-resolver.test-helpers.ts +++ b/extensions/matrix/src/matrix/client-resolver.test-helpers.ts @@ -6,6 +6,7 @@ type MatrixClientResolverMocks = { getMatrixRuntimeMock: Mock<() => unknown>; getActiveMatrixClientMock: Mock<(...args: unknown[]) => MatrixClient | null>; resolveSharedMatrixClientMock: Mock<(...args: unknown[]) => Promise>; + stopSharedClientForAccountMock: Mock<(...args: unknown[]) => void>; isBunRuntimeMock: Mock<() => boolean>; resolveMatrixAuthContextMock: Mock< (params: { cfg: unknown; accountId?: string | null }) => unknown @@ -17,6 +18,7 @@ export const matrixClientResolverMocks: MatrixClientResolverMocks = { getMatrixRuntimeMock: vi.fn(), getActiveMatrixClientMock: vi.fn(), resolveSharedMatrixClientMock: vi.fn(), + stopSharedClientForAccountMock: vi.fn(), isBunRuntimeMock: vi.fn(() => false), resolveMatrixAuthContextMock: vi.fn(), }; @@ -42,6 +44,7 @@ export function primeMatrixClientResolverMocks(params?: { getMatrixRuntimeMock, getActiveMatrixClientMock, resolveSharedMatrixClientMock, + stopSharedClientForAccountMock, isBunRuntimeMock, resolveMatrixAuthContextMock, } = matrixClientResolverMocks; @@ -67,6 +70,7 @@ export function primeMatrixClientResolverMocks(params?: { }); getActiveMatrixClientMock.mockReturnValue(null); isBunRuntimeMock.mockReturnValue(false); + stopSharedClientForAccountMock.mockReset(); resolveMatrixAuthContextMock.mockImplementation( ({ cfg: explicitCfg, diff --git a/extensions/matrix/src/matrix/send/client.test.ts b/extensions/matrix/src/matrix/send/client.test.ts index ce3b13263f1..61bcd897618 100644 --- a/extensions/matrix/src/matrix/send/client.test.ts +++ b/extensions/matrix/src/matrix/send/client.test.ts @@ -9,6 +9,7 @@ const { getMatrixRuntimeMock, getActiveMatrixClientMock, resolveSharedMatrixClientMock, + stopSharedClientForAccountMock, isBunRuntimeMock, resolveMatrixAuthContextMock, } = matrixClientResolverMocks; @@ -23,6 +24,10 @@ vi.mock("../client.js", () => ({ resolveMatrixAuthContext: resolveMatrixAuthContextMock, })); +vi.mock("../client/shared.js", () => ({ + stopSharedClientForAccount: (...args: unknown[]) => stopSharedClientForAccountMock(...args), +})); + vi.mock("../../runtime.js", () => ({ getMatrixRuntime: () => getMatrixRuntimeMock(), })); @@ -43,7 +48,7 @@ describe("withResolvedMatrixClient", () => { vi.unstubAllEnvs(); }); - it("reuses the shared client pool when no active monitor client is registered", async () => { + it("stops one-off shared clients when no active monitor client is registered", async () => { vi.stubEnv("OPENCLAW_GATEWAY_PORT", "18799"); const result = await withResolvedMatrixClient({ accountId: "default" }, async () => "ok"); @@ -57,7 +62,10 @@ describe("withResolvedMatrixClient", () => { }); const sharedClient = await resolveSharedMatrixClientMock.mock.results[0]?.value; expect(sharedClient.prepareForOneOff).toHaveBeenCalledTimes(1); - expect(sharedClient.stop).not.toHaveBeenCalled(); + expect(sharedClient.stop).toHaveBeenCalledTimes(1); + expect(stopSharedClientForAccountMock).toHaveBeenCalledWith( + expect.objectContaining({ userId: "@bot:example.org" }), + ); expect(result).toBe("ok"); }); @@ -115,7 +123,7 @@ describe("withResolvedMatrixClient", () => { }); }); - it("keeps shared matrix clients alive when wrapped sends fail", async () => { + it("stops shared matrix clients when wrapped sends fail", async () => { const sharedClient = createMockMatrixClient(); resolveSharedMatrixClientMock.mockResolvedValue(sharedClient); @@ -125,6 +133,9 @@ describe("withResolvedMatrixClient", () => { }), ).rejects.toThrow("boom"); - expect(sharedClient.stop).not.toHaveBeenCalled(); + expect(sharedClient.stop).toHaveBeenCalledTimes(1); + expect(stopSharedClientForAccountMock).toHaveBeenCalledWith( + expect.objectContaining({ userId: "@bot:example.org" }), + ); }); });