Matrix: stop one-off shared action clients

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);
   });
 });

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<void>;
 };
 
 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;

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<MatrixClient>>;
+  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,

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" }),
+    );
   });
 });