fix(signal): allow defaultTo approval reactions

kevinslin · kevinslin · commit 2031784dbb23 · 2026-05-25T12:24:41.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,7 +13,6 @@ Docs: https://docs.openclaw.ai
 - Migrate: import supported Hermes, OpenCode, and Codex auth credentials into OpenClaw auth profiles when credential migration is selected, with explicit opt-out and non-interactive controls. (#85667) Thanks @fuller-stack-dev.
 - iOS: improve Talk mode with direct realtime voice sessions, compact toolbar status, and responsive voice waveform feedback. (#86355) Thanks @ngutman.
 - Media: replace the Sharp image backend with Photon for metadata, resizing, EXIF orientation, and PNG alpha-preserving optimization so OpenClaw no longer installs Sharp or the WhatsApp Jimp fallback for image processing. (#86437)
-- Signal: support native exec and plugin approval prompts with 👍 allow-once and 👎 deny reactions, while keeping `allow-always` on the `/approve` command path.
 
 ### Fixes
 
@@ -87,7 +86,6 @@ Docs: https://docs.openclaw.ai
 - Gateway/perf: fail startup benchmark samples when the Gateway process exits before benchmark teardown, including signal deaths after readiness probes.
 - Gateway/perf: fail restart benchmark samples when the Gateway exits before benchmark teardown, including clean exits and signal deaths after successful restart probes.
 - Agents/tests: keep model catalog visibility on static selection helpers so catalog visibility checks avoid the broad model-selection barrel import.
-- Signal: suppress duplicate direct exec approval fallback messages after native approval delivery succeeds.
 - Agents/commitments: serialize commitment store load-modify-save writes so concurrent heartbeat and CLI updates no longer lose dismissal, sent, or attempt state. (#81153) Thanks @ai-hpc.
 - xAI/LM Studio: promote plain-text tool-call fallbacks into structured tool calls and strip leaked internal tool syntax before user-facing delivery. (#86222) Thanks @fuller-stack-dev.
 - CLI: suppress benign self-update version-skew warnings during package post-update finalization.
diff --git a/extensions/signal/src/approval-reactions.test.ts b/extensions/signal/src/approval-reactions.test.ts
@@ -287,6 +287,64 @@ describe("Signal approval reactions", () => {
     });
   });
 
+  it("authorizes reactions using Signal defaultTo approvers", async () => {
+    registerSignalApprovalReactionTarget({
+      accountId: "default",
+      conversationKey: "+15551230000",
+      messageId: "1700000000008",
+      approvalId: "exec-default-to",
+      allowedDecisions: ["allow-once"],
+      targetAuthorKeys: ["+15550009999"],
+      route: approvalRoute,
+      routeAllowed: true,
+    });
+
+    const handled = await maybeResolveSignalApprovalReaction({
+      cfg: {
+        channels: {
+          signal: {
+            allowFrom: [],
+            defaultTo: "+15551230000",
+          },
+        },
+        approvals: {
+          exec: {
+            enabled: true,
+            mode: "session",
+          },
+        },
+      },
+      accountId: "default",
+      conversationKey: "+15551230000",
+      messageId: "1700000000008",
+      reactionKey: "👍",
+      actorId: "+15551230000",
+      targetAuthor: "+15550009999",
+    });
+
+    expect(handled).toBe(true);
+    expect(resolverMocks.resolveSignalApproval).toHaveBeenCalledWith({
+      cfg: {
+        channels: {
+          signal: {
+            allowFrom: [],
+            defaultTo: "+15551230000",
+          },
+        },
+        approvals: {
+          exec: {
+            enabled: true,
+            mode: "session",
+          },
+        },
+      },
+      approvalId: "exec-default-to",
+      decision: "allow-once",
+      senderId: "+15551230000",
+      gatewayUrl: undefined,
+    });
+  });
+
   it("requires explicit approvers for approval reactions", async () => {
     registerSignalApprovalReactionTarget({
       accountId: "default",
diff --git a/extensions/signal/src/monitor/event-handler.inbound-context.test.ts b/extensions/signal/src/monitor/event-handler.inbound-context.test.ts
@@ -36,6 +36,10 @@ const {
   };
 });
 
+const approvalReactionMocks = vi.hoisted(() => ({
+  maybeResolveSignalApprovalReaction: vi.fn(async () => false),
+}));
+
 vi.mock("../send.js", () => ({
   sendMessageSignal: vi.fn(),
   sendTypingSignal: sendTypingMock,
@@ -76,6 +80,16 @@ vi.mock("openclaw/plugin-sdk/system-event-runtime", async () => {
   };
 });
 
+vi.mock("../approval-reactions.js", async () => {
+  const actual = await vi.importActual<typeof import("../approval-reactions.js")>(
+    "../approval-reactions.js",
+  );
+  return {
+    ...actual,
+    maybeResolveSignalApprovalReaction: approvalReactionMocks.maybeResolveSignalApprovalReaction,
+  };
+});
+
 function requireCapturedContext(): MsgContext {
   if (!capture.ctx) {
     throw new Error("expected inbound MsgContext");
@@ -91,6 +105,7 @@ describe("signal createSignalEventHandler inbound context", () => {
     enqueueSystemEventMock.mockReset();
     recordInboundSessionMock.mockReset().mockResolvedValue(undefined);
     dispatchInboundMessageMock.mockClear();
+    approvalReactionMocks.maybeResolveSignalApprovalReaction.mockReset().mockResolvedValue(false);
   });
 
   it("passes a finalized MsgContext to dispatchInboundMessage", async () => {
@@ -521,6 +536,59 @@ describe("signal createSignalEventHandler inbound context", () => {
     });
   });
 
+  it("checks approval reactions before dropping defaultTo-only senders at the generic access gate", async () => {
+    approvalReactionMocks.maybeResolveSignalApprovalReaction.mockResolvedValueOnce(true);
+    const cfg = {
+      messages: { inbound: { debounceMs: 0 } },
+      channels: {
+        signal: {
+          dmPolicy: "allowlist",
+          allowFrom: [],
+          defaultTo: "+15550001111",
+        },
+      },
+    };
+    const handler = createSignalEventHandler(
+      createBaseSignalEventHandlerDeps({
+        cfg: cfg as any,
+        dmPolicy: "allowlist",
+        allowFrom: [],
+        reactionMode: "all",
+        isSignalReactionMessage: (reaction): reaction is SignalReactionMessage => Boolean(reaction),
+        shouldEmitSignalReactionNotification: () => true,
+        resolveSignalReactionTargets: () => [
+          { kind: "phone", id: "+15550001111", display: "+15550001111" },
+        ],
+        buildSignalReactionSystemEventText: () => "reaction added",
+        historyLimit: 0,
+      }),
+    );
+
+    await handler(
+      createSignalReceiveEvent({
+        reactionMessage: {
+          emoji: "👍",
+          targetAuthor: "+15550009999",
+          targetSentTimestamp: 1700000000000,
+        },
+      }),
+    );
+
+    expect(approvalReactionMocks.maybeResolveSignalApprovalReaction).toHaveBeenCalledWith(
+      expect.objectContaining({
+        cfg,
+        accountId: "default",
+        conversationKey: "+15550001111",
+        messageId: "1700000000000",
+        reactionKey: "👍",
+        actorId: "+15550001111",
+        targetAuthor: "+15550009999",
+      }),
+    );
+    expect(dispatchInboundMessageMock).not.toHaveBeenCalled();
+    expect(enqueueSystemEventMock).not.toHaveBeenCalled();
+  });
+
   it("drops quote-only group context from non-allowlisted quoted senders in allowlist mode", async () => {
     const handler = createSignalEventHandler(
       createBaseSignalEventHandlerDeps({
diff --git a/extensions/signal/src/monitor/event-handler.ts b/extensions/signal/src/monitor/event-handler.ts
@@ -442,12 +442,6 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
     const groupId = params.reaction.groupInfo?.groupId ?? undefined;
     const groupName = params.reaction.groupInfo?.groupName ?? undefined;
     const isGroup = Boolean(groupId);
-    if (params.accessDecision.decision !== "allow") {
-      logVerbose(
-        `Blocked signal reaction sender ${params.senderDisplay} (${params.accessDecision.reasonCode})`,
-      );
-      return true;
-    }
     const messageId = params.reaction.targetSentTimestamp
       ? String(params.reaction.targetSentTimestamp)
       : "unknown";
@@ -470,6 +464,12 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
     ) {
       return true;
     }
+    if (params.accessDecision.decision !== "allow") {
+      logVerbose(
+        `Blocked signal reaction sender ${params.senderDisplay} (${params.accessDecision.reasonCode})`,
+      );
+      return true;
+    }
     const targets = deps.resolveSignalReactionTargets(params.reaction);
     const shouldNotify = deps.shouldEmitSignalReactionNotification({
       mode: deps.reactionMode,
