From 2685b11e95045d12359867855c846d6626ae297c Mon Sep 17 00:00:00 2001 From: Dale Date: Sun, 1 Mar 2026 22:27:13 -0500 Subject: [PATCH 1/2] fix(signal): ignore system messages (expiration timer, group permission changes) signal-cli sends dataMessage envelopes for system events (disappearing message timer changes, group permission updates) with no user text. These fell through to the agent, which responded with a confused 'I got a media message' reply. Add isExpirationUpdate and groupV2Change fields to SignalDataMessage type, and early-return on these system-only events before dispatch. Fixes #27615, Fixes #30981 --- .../signal/src/monitor/event-handler.ts | 15 ++ .../signal/src/monitor/event-handler.types.ts | 3 + .../event-handler.system-messages.test.ts | 128 ++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 src/signal/monitor/event-handler.system-messages.test.ts diff --git a/extensions/signal/src/monitor/event-handler.ts b/extensions/signal/src/monitor/event-handler.ts index 36eb0e8d276..4e27f713917 100644 --- a/extensions/signal/src/monitor/event-handler.ts +++ b/extensions/signal/src/monitor/event-handler.ts @@ -565,6 +565,21 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { const groupName = dataMessage.groupInfo?.groupName ?? undefined; const isGroup = Boolean(groupId); + // Skip signal-cli system messages that have no user-visible content + const isTimerUpdate = + !messageText && + !quoteText && + !dataMessage.attachments?.length && + (dataMessage.isExpirationUpdate === true || + (typeof dataMessage.expiresInSeconds === "number" && dataMessage.expiresInSeconds > 0)); + const isGroupV2Change = Boolean(dataMessage.groupV2Change); + if (isTimerUpdate || isGroupV2Change) { + logVerbose( + `signal: skipping system message (isTimerUpdate=${isTimerUpdate}, isGroupV2Change=${isGroupV2Change})`, + ); + return; + } + if (!isGroup) { const allowedDirectMessage = await handleSignalDirectMessageAccess({ dmPolicy: deps.dmPolicy, diff --git a/extensions/signal/src/monitor/event-handler.types.ts b/extensions/signal/src/monitor/event-handler.types.ts index c1d0b0b3881..2980f1fe9ed 100644 --- a/extensions/signal/src/monitor/event-handler.types.ts +++ b/extensions/signal/src/monitor/event-handler.types.ts @@ -39,6 +39,9 @@ export type SignalDataMessage = { } | null; quote?: { text?: string | null } | null; reaction?: SignalReactionMessage | null; + expiresInSeconds?: number | null; + groupV2Change?: Record | null; + isExpirationUpdate?: boolean | null; }; export type SignalReactionMessage = { diff --git a/src/signal/monitor/event-handler.system-messages.test.ts b/src/signal/monitor/event-handler.system-messages.test.ts new file mode 100644 index 00000000000..34ff3b92ef9 --- /dev/null +++ b/src/signal/monitor/event-handler.system-messages.test.ts @@ -0,0 +1,128 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createSignalEventHandler } from "./event-handler.js"; +import { + createBaseSignalEventHandlerDeps, + createSignalReceiveEvent, +} from "./event-handler.test-harness.js"; + +const { sendTypingMock, sendReadReceiptMock, dispatchInboundMessageMock } = vi.hoisted(() => ({ + sendTypingMock: vi.fn(), + sendReadReceiptMock: vi.fn(), + dispatchInboundMessageMock: vi.fn(async () => ({ + queuedFinal: false, + counts: { tool: 0, block: 0, final: 0 }, + })), +})); + +vi.mock("../send.js", () => ({ + sendMessageSignal: vi.fn(), + sendTypingSignal: sendTypingMock, + sendReadReceiptSignal: sendReadReceiptMock, +})); + +vi.mock("../../auto-reply/dispatch.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + dispatchInboundMessage: dispatchInboundMessageMock, + dispatchInboundMessageWithDispatcher: dispatchInboundMessageMock, + dispatchInboundMessageWithBufferedDispatcher: dispatchInboundMessageMock, + }; +}); + +vi.mock("../../pairing/pairing-store.js", () => ({ + readChannelAllowFromStore: vi.fn().mockResolvedValue([]), + upsertChannelPairingRequest: vi.fn(), +})); + +function makeDeps() { + return createBaseSignalEventHandlerDeps({ + // oxlint-disable-next-line typescript/no-explicit-any + cfg: { messages: { inbound: { debounceMs: 0 } } } as any, + historyLimit: 0, + }); +} + +describe("signal system message filtering", () => { + beforeEach(() => { + sendTypingMock.mockReset().mockResolvedValue(true); + sendReadReceiptMock.mockReset().mockResolvedValue(true); + dispatchInboundMessageMock.mockClear(); + }); + + it("filters expiration timer update (expiresInSeconds, no text)", async () => { + const handler = createSignalEventHandler(makeDeps()); + await handler( + createSignalReceiveEvent({ + dataMessage: { + message: null, + attachments: [], + expiresInSeconds: 604800, + groupInfo: { groupId: "g1", groupName: "Test Group" }, + }, + }), + ); + expect(dispatchInboundMessageMock).not.toHaveBeenCalled(); + }); + + it("filters expiration timer update with isExpirationUpdate flag", async () => { + const handler = createSignalEventHandler(makeDeps()); + await handler( + createSignalReceiveEvent({ + dataMessage: { + message: null, + attachments: [], + isExpirationUpdate: true, + expiresInSeconds: 604800, + groupInfo: { groupId: "g1", groupName: "Test Group" }, + }, + }), + ); + expect(dispatchInboundMessageMock).not.toHaveBeenCalled(); + }); + + it("filters groupV2Change messages", async () => { + const handler = createSignalEventHandler(makeDeps()); + await handler( + createSignalReceiveEvent({ + dataMessage: { + message: null, + attachments: [], + groupV2Change: { editor: "+15550001111", changes: [] }, + groupInfo: { groupId: "g1", groupName: "Test Group" }, + }, + }), + ); + expect(dispatchInboundMessageMock).not.toHaveBeenCalled(); + }); + + it("does NOT filter normal message with expiresInSeconds=0", async () => { + const handler = createSignalEventHandler(makeDeps()); + await handler( + createSignalReceiveEvent({ + dataMessage: { + message: "hello", + attachments: [], + expiresInSeconds: 0, + groupInfo: { groupId: "g1", groupName: "Test Group" }, + }, + }), + ); + expect(dispatchInboundMessageMock).toHaveBeenCalled(); + }); + + it("does NOT filter message with text even if expiresInSeconds > 0", async () => { + const handler = createSignalEventHandler(makeDeps()); + await handler( + createSignalReceiveEvent({ + dataMessage: { + message: "hello with timer", + attachments: [], + expiresInSeconds: 604800, + groupInfo: { groupId: "g1", groupName: "Test Group" }, + }, + }), + ); + expect(dispatchInboundMessageMock).toHaveBeenCalled(); + }); +}); From f6e3d00a43dfa837552475c995cb1f5a7e928cc7 Mon Sep 17 00:00:00 2001 From: root Date: Sat, 14 Mar 2026 16:15:02 +0000 Subject: [PATCH 2/2] fix(signal): move system-messages test to extensions/signal and fix import paths Test file belongs alongside the implementation it tests in extensions/signal/src/monitor/. Update ../../ relative paths to ../../../../src/ to match the new location. --- .../src}/monitor/event-handler.system-messages.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename {src/signal => extensions/signal/src}/monitor/event-handler.system-messages.test.ts (94%) diff --git a/src/signal/monitor/event-handler.system-messages.test.ts b/extensions/signal/src/monitor/event-handler.system-messages.test.ts similarity index 94% rename from src/signal/monitor/event-handler.system-messages.test.ts rename to extensions/signal/src/monitor/event-handler.system-messages.test.ts index 34ff3b92ef9..9b055f1c75f 100644 --- a/src/signal/monitor/event-handler.system-messages.test.ts +++ b/extensions/signal/src/monitor/event-handler.system-messages.test.ts @@ -20,8 +20,8 @@ vi.mock("../send.js", () => ({ sendReadReceiptSignal: sendReadReceiptMock, })); -vi.mock("../../auto-reply/dispatch.js", async (importOriginal) => { - const actual = await importOriginal(); +vi.mock("../../../../src/auto-reply/dispatch.js", async (importOriginal) => { + const actual = await importOriginal(); return { ...actual, dispatchInboundMessage: dispatchInboundMessageMock, @@ -30,7 +30,7 @@ vi.mock("../../auto-reply/dispatch.js", async (importOriginal) => { }; }); -vi.mock("../../pairing/pairing-store.js", () => ({ +vi.mock("../../../../src/pairing/pairing-store.js", () => ({ readChannelAllowFromStore: vi.fn().mockResolvedValue([]), upsertChannelPairingRequest: vi.fn(), }));