Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions src/slack/monitor/message-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
createInboundDebouncer,
resolveInboundDebounceMs,
} from "../../auto-reply/inbound-debounce.js";
import { logVerbose } from "../../globals.js";
import type { ResolvedSlackAccount } from "../accounts.js";
import type { SlackMessageEvent } from "../types.js";
import { stripSlackMentionsForCommandDetection } from "./commands.js";
Expand All @@ -26,6 +27,10 @@ export function createSlackMessageHandler(params: {
const debounceMs = resolveInboundDebounceMs({ cfg: ctx.cfg, channel: "slack" });
const threadTsResolver = createSlackThreadTsResolver({ client: ctx.app.client });

/** Tracks muted threads. Key: "channel:threadTs". When muted, the bot ignores
* messages in that thread unless explicitly @mentioned. */
const mutedThreads = new Set<string>();

const debouncer = createInboundDebouncer<{
message: SlackMessageEvent;
opts: { source: "message" | "app_mention"; wasMentioned?: boolean };
Expand Down Expand Up @@ -117,10 +122,50 @@ export function createSlackMessageHandler(params: {
}
trackEvent?.();
// Skip messages that start with "aside" (case-insensitive, after stripping mentions).
const textForAsideCheck = stripSlackMentionsForCommandDetection(message.text ?? "");
if (/^aside\b/i.test(textForAsideCheck)) {
const strippedText = stripSlackMentionsForCommandDetection(message.text ?? "");
if (/^aside\b/i.test(strippedText)) {
return;
}

// Mute / unmute: thread-scoped commands that silence the bot unless @mentioned.
const threadTs = message.thread_ts ?? message.ts;
const muteKey = threadTs ? `${message.channel}:${threadTs}` : undefined;
Comment on lines +131 to +132
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Mute check runs before thread_ts resolution, causing mute bypass for thread replies with missing thread_ts

The mute key is computed at line 131 using message.thread_ts ?? message.ts, but threadTsResolver.resolve() is called later at line 169. When Slack omits thread_ts on a thread reply (indicated by parent_user_id being set — a known edge case handled by the thread resolver at src/slack/monitor/thread-resolution.ts:81), the mute key will be based on message.ts (the reply's own timestamp) instead of the actual thread root timestamp. This means:

  1. If a thread was correctly muted, subsequent thread replies missing thread_ts will use a different muteKey and bypass the mute check at line 165.
  2. If someone sends "mute" from such a message, the mute will be registered under the wrong key and won't apply to the actual thread.

The debouncer already accounts for this edge case with the maybe-thread key prefix at line 48-49, but the mute logic does not.

Prompt for agents
In src/slack/monitor/message-handler.ts, the mute/unmute key computation and mute-silence check (lines 131-166) should use the resolved thread_ts, not the raw message fields. Move the mute/unmute logic (lines 130-167) to AFTER the threadTsResolver.resolve() call at line 169, and compute threadTs/muteKey from resolvedMessage instead of message. Specifically:

1. Move line 169 (const resolvedMessage = await threadTsResolver.resolve(...)) to just after the aside check (after line 128)
2. Compute threadTs and muteKey from resolvedMessage.thread_ts ?? resolvedMessage.ts instead of message.thread_ts ?? message.ts
3. Use resolvedMessage.channel for the muteKey
4. Pass resolvedMessage through the mute/unmute checks
5. On line 170, enqueue resolvedMessage (already resolved) instead of re-resolving

This ensures the mute key matches the actual thread identity even when Slack omits thread_ts on thread replies.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


if (muteKey && /^mute\b/i.test(strippedText)) {
mutedThreads.add(muteKey);
logVerbose(`slack: muted thread ${muteKey}`);
ctx.app.client.chat
.postMessage({
channel: message.channel,
thread_ts: threadTs,
text: "\ud83d\udd07 Muted. Tag me to resume, or say *unmute*.",
})
.catch((err) => {
logVerbose(`slack: failed posting mute ack: ${String(err)}`);
});
return;
}

if (muteKey && /^unmute\b/i.test(strippedText)) {
mutedThreads.delete(muteKey);
logVerbose(`slack: unmuted thread ${muteKey}`);
ctx.app.client.chat
.postMessage({
channel: message.channel,
thread_ts: threadTs,
text: "\ud83d\udd0a Unmuted.",
})
.catch((err) => {
logVerbose(`slack: failed posting unmute ack: ${String(err)}`);
});
return;
}

// If thread is muted, only process messages where the bot was explicitly @mentioned.
if (muteKey && mutedThreads.has(muteKey) && opts.source !== "app_mention" && !opts.wasMentioned) {
return;
}

const resolvedMessage = await threadTsResolver.resolve({ message, source: opts.source });
await debouncer.enqueue({ message: resolvedMessage, opts });
};
Expand Down
Loading