fix(clawsweeper): address review for automerge-openclaw-openclaw-83348 (1)

2026-05-30 04:33:36 +00:00 · 2026-05-22 03:00:53 +00:00
parent 659e5e31f9
commit 09ca7d29fd
6 changed files with 216 additions and 28 deletions
--- a/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts
+++ b/extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts
@@ -1,3 +1,4 @@
+import { createInboundDebouncer } from "openclaw/plugin-sdk/channel-inbound-debounce";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { monitorMattermostProvider } from "./monitor.js";
 import type { OpenClawConfig, RuntimeEnv } from "./runtime-api.js";
@@ -148,6 +149,14 @@ function createRuntimeCore(
    mainSessionKey?: string;
    sessionKey?: string;
  },
+  overrides: {
+    inboundDebounceMs?: number;
+    isControlCommandMessage?: (text?: string) => boolean;
+    shouldComputeCommandAuthorized?: (text?: string) => boolean;
+    shouldHandleTextCommands?: () => boolean;
+    textHasControlCommand?: (text?: string) => boolean;
+    createInboundDebouncer?: typeof createInboundDebouncer;
+  } = {},
 ) {
  const runPrepared = vi.fn(
    async (turn: {
@@ -230,21 +239,23 @@ function createRuntimeCore(
        record: vi.fn(),
      },
      commands: {
-        isControlCommandMessage: () => false,
-        shouldHandleTextCommands: () => false,
+        isControlCommandMessage: overrides.isControlCommandMessage ?? (() => false),
+        shouldComputeCommandAuthorized: overrides.shouldComputeCommandAuthorized ?? (() => false),
+        shouldHandleTextCommands: overrides.shouldHandleTextCommands ?? (() => false),
      },
      debounce: {
-        resolveInboundDebounceMs: () => 0,
-        createInboundDebouncer: <T>(params: {
-          onFlush: (entries: T[]) => Promise<void> | void;
-        }) => ({
-          enqueue: async (entry: T) => {
-            await params.onFlush([entry]);
-          },
-        }),
+        resolveInboundDebounceMs: () => overrides.inboundDebounceMs ?? 0,
+        createInboundDebouncer:
+          overrides.createInboundDebouncer ??
+          (<T>(params: { onFlush: (entries: T[]) => Promise<void> | void }) => ({
+            enqueue: async (entry: T) => {
+              await params.onFlush([entry]);
+            },
+          })),
      },
      groups: {
-        resolveRequireMention: () => false,
+        resolveRequireMention: (params: { requireMentionOverride?: boolean }) =>
+          params.requireMentionOverride ?? false,
      },
      media: {
        readRemoteMediaBuffer: vi.fn(),
@@ -317,7 +328,7 @@ function createRuntimeCore(
      text: {
        chunkMarkdownTextWithMode: (text: string) => [text],
        convertMarkdownTables: (text: string) => text,
-        hasControlCommand: () => false,
+        hasControlCommand: overrides.textHasControlCommand ?? (() => false),
        resolveChunkMode: () => "off",
        resolveMarkdownTableMode: () => "off",
        resolveTextChunkLimit: () => 4000,
@@ -544,6 +555,98 @@ describe("mattermost inbound user posts", () => {
    expect(runtimeCore.channel.session.recordInboundSession).not.toHaveBeenCalled();
  });

+  it("flushes pending group text before authorizing a bare abort without a mention", async () => {
+    const socket = new FakeWebSocket();
+    const abortController = new AbortController();
+    mockState.abortController = abortController;
+    const mentionConfig: OpenClawConfig = {
+      commands: { useAccessGroups: false },
+      messages: { inbound: { debounceMs: 60_000 } },
+      channels: {
+        mattermost: {
+          enabled: true,
+          baseUrl: "https://mattermost.example.com",
+          botToken: "bot-token",
+          chatmode: "oncall",
+          dmPolicy: "open",
+          groupPolicy: "open",
+        },
+      },
+    };
+    const isBareAbort = (text?: string) => ["abort", "stop"].includes(text?.trim() ?? "");
+    const runtimeCore = createRuntimeCore(mentionConfig, undefined, {
+      inboundDebounceMs: 60_000,
+      createInboundDebouncer,
+      isControlCommandMessage: isBareAbort,
+      shouldComputeCommandAuthorized: isBareAbort,
+      shouldHandleTextCommands: () => true,
+      textHasControlCommand: () => false,
+    });
+    mockState.runtimeCore = runtimeCore;
+
+    const monitor = monitorMattermostProvider({
+      config: mentionConfig,
+      runtime: testRuntime(),
+      abortSignal: abortController.signal,
+      webSocketFactory: () => socket,
+    });
+
+    await vi.waitFor(() => {
+      expect(socket.openListenerCount).toBeGreaterThan(0);
+    });
+    socket.emitOpen();
+
+    await socket.emitMessage({
+      event: "posted",
+      data: {
+        channel_id: "chan-1",
+        channel_name: "town-square",
+        channel_display_name: "Town Square",
+        sender_name: "alice",
+        post: JSON.stringify({
+          id: "post-pending",
+          channel_id: "chan-1",
+          user_id: "user-1",
+          message: "pending text",
+          create_at: 1_714_000_000_000,
+        }),
+      },
+      broadcast: {
+        channel_id: "chan-1",
+        user_id: "user-1",
+      },
+    });
+    expect(mockState.dispatchReplyFromConfig).not.toHaveBeenCalled();
+
+    await socket.emitMessage({
+      event: "posted",
+      data: {
+        channel_id: "chan-1",
+        channel_name: "town-square",
+        channel_display_name: "Town Square",
+        sender_name: "alice",
+        post: JSON.stringify({
+          id: "post-abort",
+          channel_id: "chan-1",
+          user_id: "user-1",
+          message: "abort",
+          create_at: 1_714_000_000_100,
+        }),
+      },
+      broadcast: {
+        channel_id: "chan-1",
+        user_id: "user-1",
+      },
+    });
+    socket.emitClose(1000);
+    await monitor;
+
+    expect(mockState.dispatchReplyFromConfig).toHaveBeenCalledTimes(1);
+    const ctx = mockState.dispatchReplyFromConfig.mock.calls.at(0)?.[0].ctx;
+    expect(ctx?.BodyForAgent).toBe("abort");
+    expect(ctx?.CommandAuthorized).toBe(true);
+  });
+
  it("pins direct-message main route updates to the configured owner", async () => {
    const socket = new FakeWebSocket();
    const abortController = new AbortController();
--- a/extensions/mattermost/src/mattermost/monitor.ts
+++ b/extensions/mattermost/src/mattermost/monitor.ts
@@ -1318,8 +1318,10 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
          cfg,
          surface: "mattermost",
        });
-        const hasControlCommand = core.channel.text.hasControlCommand(rawText, cfg);
-        const isControlCommand = allowTextCommands && hasControlCommand;
+        const isControlCommand =
+          allowTextCommands && core.channel.commands.isControlCommandMessage(rawText, cfg);
+        const shouldComputeCommandAuthorized =
+          allowTextCommands && core.channel.commands.shouldComputeCommandAuthorized(rawText, cfg);
        const accessDecision = await resolveMattermostMonitorInboundAccess({
          account,
          cfg,
@@ -1330,7 +1332,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
          groupPolicy,
          readStoreAllowFrom: pairing.readAllowFromStore,
          allowTextCommands,
-          hasControlCommand,
+          hasControlCommand: shouldComputeCommandAuthorized,
          eventKind: "message",
          mayPair: true,
        });
--- a/extensions/msteams/src/monitor-handler.test-helpers.ts
+++ b/extensions/msteams/src/monitor-handler.test-helpers.ts
@@ -15,6 +15,11 @@ type MSTeamsTestRuntimeOptions = {
  recordInboundSession?: ReturnType<typeof vi.fn>;
  resolveAgentRoute?: (params: RuntimeRoutePeer) => unknown;
  hasControlCommand?: PluginRuntime["channel"]["text"]["hasControlCommand"];
+  isControlCommandMessage?: PluginRuntime["channel"]["commands"]["isControlCommandMessage"];
+  shouldComputeCommandAuthorized?: PluginRuntime["channel"]["commands"]["shouldComputeCommandAuthorized"];
+  shouldHandleTextCommands?: PluginRuntime["channel"]["commands"]["shouldHandleTextCommands"];
+  createInboundDebouncer?: PluginRuntime["channel"]["debounce"]["createInboundDebouncer"];
+  resolveInboundDebounceMs?: PluginRuntime["channel"]["debounce"]["resolveInboundDebounceMs"];
  resolveTextChunkLimit?: () => number;
  resolveStorePath?: () => string;
 };
@@ -66,21 +71,29 @@ export function installMSTeamsTestRuntime(options: MSTeamsTestRuntimeOptions = {
    system: { enqueueSystemEvent: options.enqueueSystemEvent ?? vi.fn() },
    channel: {
      debounce: {
-        resolveInboundDebounceMs: () => 0,
-        createInboundDebouncer: <T>(params: {
-          onFlush: (entries: T[]) => Promise<void>;
-        }): { enqueue: (entry: T) => Promise<void> } => ({
-          enqueue: async (entry: T) => {
-            await params.onFlush([entry]);
-          },
-        }),
+        resolveInboundDebounceMs:
+          options.resolveInboundDebounceMs ??
+          ((() => 0) as PluginRuntime["channel"]["debounce"]["resolveInboundDebounceMs"]),
+        createInboundDebouncer:
+          options.createInboundDebouncer ??
+          (<T>(params: {
+            onFlush: (entries: T[]) => Promise<void>;
+          }): { enqueue: (entry: T) => Promise<void> } => ({
+            enqueue: async (entry: T) => {
+              await params.onFlush([entry]);
+            },
+          })),
      },
      pairing: {
        readAllowFromStore: options.readAllowFromStore ?? vi.fn(async () => []),
        upsertPairingRequest: options.upsertPairingRequest ?? vi.fn(async () => null),
      },
      commands: {
-        isControlCommandMessage: options.hasControlCommand ?? (() => false),
+        isControlCommandMessage:
+          options.isControlCommandMessage ?? options.hasControlCommand ?? (() => false),
+        shouldComputeCommandAuthorized:
+          options.shouldComputeCommandAuthorized ?? options.hasControlCommand ?? (() => false),
+        shouldHandleTextCommands: options.shouldHandleTextCommands ?? (() => true),
      },
      text: {
        hasControlCommand: options.hasControlCommand ?? (() => false),
--- a/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts
+++ b/extensions/msteams/src/monitor-handler/message-handler.authz.test.ts
@@ -1,3 +1,4 @@
+import { createInboundDebouncer } from "openclaw/plugin-sdk/channel-inbound-debounce";
 import { describe, expect, it, vi } from "vitest";
 import type { OpenClawConfig, PluginRuntime } from "../../runtime-api.js";
 import type { GraphThreadMessage } from "../graph-thread.js";
@@ -84,6 +85,11 @@ describe("msteams monitor handler authz", () => {
    cfg: OpenClawConfig,
    options: {
      hasControlCommand?: PluginRuntime["channel"]["text"]["hasControlCommand"];
+      isControlCommandMessage?: PluginRuntime["channel"]["commands"]["isControlCommandMessage"];
+      shouldComputeCommandAuthorized?: PluginRuntime["channel"]["commands"]["shouldComputeCommandAuthorized"];
+      shouldHandleTextCommands?: PluginRuntime["channel"]["commands"]["shouldHandleTextCommands"];
+      createInboundDebouncer?: PluginRuntime["channel"]["debounce"]["createInboundDebouncer"];
+      resolveInboundDebounceMs?: PluginRuntime["channel"]["debounce"]["resolveInboundDebounceMs"];
    } = {},
  ) {
    const readAllowFromStore = vi.fn(async () => ["attacker-aad"]);
@@ -100,6 +106,11 @@ describe("msteams monitor handler authz", () => {
        accountId: "default",
      })),
      hasControlCommand: options.hasControlCommand,
+      isControlCommandMessage: options.isControlCommandMessage,
+      shouldComputeCommandAuthorized: options.shouldComputeCommandAuthorized,
+      shouldHandleTextCommands: options.shouldHandleTextCommands,
+      createInboundDebouncer: options.createInboundDebouncer,
+      resolveInboundDebounceMs: options.resolveInboundDebounceMs,
    });
  }

@@ -606,6 +617,47 @@ describe("msteams monitor handler authz", () => {
    expect(runtimeApiMockState.dispatchReplyFromConfigWithSettledDispatcher).not.toHaveBeenCalled();
  });

+  it("flushes pending group text before authorizing a bare abort without a mention", async () => {
+    resetThreadMocks();
+    const isBareAbort = vi.fn((text?: string) =>
+      ["abort", "stop"].includes(text?.trim().toLowerCase() ?? ""),
+    );
+    const { deps } = createDeps(
+      {
+        commands: { useAccessGroups: false },
+        messages: { inbound: { debounceMs: 60_000 } },
+        channels: {
+          msteams: {
+            groupPolicy: "open",
+            requireMention: true,
+          },
+        },
+      } as OpenClawConfig,
+      {
+        hasControlCommand: vi.fn(() => false),
+        isControlCommandMessage: isBareAbort,
+        shouldComputeCommandAuthorized: isBareAbort,
+        shouldHandleTextCommands: vi.fn(() => true),
+        createInboundDebouncer,
+        resolveInboundDebounceMs: vi.fn(() => 60_000),
+      },
+    );
+
+    const handler = createMSTeamsMessageHandler(deps);
+    await handler(createAttackerGroupActivity({ text: "pending text" }));
+    expect(runtimeApiMockState.dispatchReplyFromConfigWithSettledDispatcher).not.toHaveBeenCalled();
+
+    await handler(createAttackerGroupActivity({ text: "abort" }));
+
+    expect(runtimeApiMockState.dispatchReplyFromConfigWithSettledDispatcher).toHaveBeenCalledTimes(
+      1,
+    );
+    const dispatched = firstSettledDispatch();
+    const ctxPayload = recordFromMockCall(dispatched.ctxPayload);
+    expect(ctxPayload.BodyForAgent).toBe("abort");
+    expect(ctxPayload.CommandAuthorized).toBe(true);
+  });
+
  it("marks skipped channel message system events as non-owner", async () => {
    resetThreadMocks();
    const { deps, enqueueSystemEvent } = createDeps({
--- a/extensions/msteams/src/monitor-handler/message-handler.test-support.ts
+++ b/extensions/msteams/src/monitor-handler/message-handler.test-support.ts
@@ -12,6 +12,11 @@ type MessageHandlerDepsOptions = {
  recordInboundSession?: ReturnType<typeof vi.fn>;
  resolveAgentRoute?: (params: { peer: { kind: string; id: string } }) => unknown;
  hasControlCommand?: PluginRuntime["channel"]["text"]["hasControlCommand"];
+  isControlCommandMessage?: PluginRuntime["channel"]["commands"]["isControlCommandMessage"];
+  shouldComputeCommandAuthorized?: PluginRuntime["channel"]["commands"]["shouldComputeCommandAuthorized"];
+  shouldHandleTextCommands?: PluginRuntime["channel"]["commands"]["shouldHandleTextCommands"];
+  createInboundDebouncer?: PluginRuntime["channel"]["debounce"]["createInboundDebouncer"];
+  resolveInboundDebounceMs?: PluginRuntime["channel"]["debounce"]["resolveInboundDebounceMs"];
 };

 export function createMessageHandlerDeps(
@@ -41,6 +46,11 @@ export function createMessageHandlerDeps(
    recordInboundSession,
    resolveAgentRoute,
    hasControlCommand: options.hasControlCommand,
+    isControlCommandMessage: options.isControlCommandMessage,
+    shouldComputeCommandAuthorized: options.shouldComputeCommandAuthorized,
+    shouldHandleTextCommands: options.shouldHandleTextCommands,
+    createInboundDebouncer: options.createInboundDebouncer,
+    resolveInboundDebounceMs: options.resolveInboundDebounceMs,
    resolveTextChunkLimit: () => 4000,
    resolveStorePath: () => "/tmp/test-store",
  });
--- a/extensions/msteams/src/monitor-handler/message-handler.ts
+++ b/extensions/msteams/src/monitor-handler/message-handler.ts
@@ -281,6 +281,14 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
      threadId,
    });

+    const allowTextCommands = core.channel.commands.shouldHandleTextCommands({
+      cfg,
+      surface: "msteams",
+    });
+    const isControlCommand =
+      allowTextCommands && core.channel.commands.isControlCommandMessage(text, cfg);
+    const shouldComputeCommandAuthorized =
+      allowTextCommands && core.channel.commands.shouldComputeCommandAuthorized(text, cfg);
    const {
      dmPolicy,
      senderId,
@@ -295,7 +303,7 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
    } = await resolveMSTeamsSenderAccess({
      cfg,
      activity,
-      hasControlCommand: core.channel.text.hasControlCommand(text, cfg),
+      hasControlCommand: shouldComputeCommandAuthorized,
    });
    const commandAuthorized = commandAccess.requested ? commandAccess.authorized : undefined;
    const effectiveDmAllowFrom = senderAccess.effectiveAllowFrom;
@@ -522,9 +530,9 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
      policy: {
        isGroup: !isDirectMessage,
        requireMention,
-        allowTextCommands: false,
-        hasControlCommand: false,
-        commandAuthorized: false,
+        allowTextCommands,
+        hasControlCommand: isControlCommand,
+        commandAuthorized: commandAuthorized === true,
      },
    });