Greptile Summary

This PR fixes a silent message-drop bug in the Slack integration where app_mention events were incorrectly rejected by the mention gate in prepareSlackMessage when requireMention: true was configured. The fix adds a single guard (opts.source !== "app_mention") so that Slack's own platform-level confirmation of a mention is trusted rather than re-checked with text-based detection that can produce false negatives.

Key points:

• The change is minimal and targeted — one condition added to an existing if-guard at prepare.ts:496.
• The rationale is sound: Slack only dispatches app_mention events to the explicitly mentioned bot's app, so text-based re-validation is redundant and can fail due to race conditions or batching.
• Four tests in message-handler.app-mention-race.test.ts cover the outer handler's dedup/retry mechanism, but there is no unit test in prepare.test.ts that exercises the new opts.source === "app_mention" path directly — meaning the specific guard change itself is untested.
• effectiveWasMentioned (used downstream for ack-reactions and WasMentioned payload) remains correctly computed because opts.wasMentioned: true is always set for app_mention events in messages.ts, so bypassing the skip-gate does not affect those consumers.

Confidence Score: 4/5

• Safe to merge — the fix is logically correct and the change surface is minimal (one added condition).
• The logic is sound (Slack's app_mention is a platform-level mention confirmation), the change is a one-liner on an existing condition, and the outer retry/dedup behaviour is well-tested by four new scenarios in message-handler.app-mention-race.test.ts. Score is 4 rather than 5 because the prepare.ts code path altered by this change has no direct unit test — a future refactor that removes wasMentioned: true from the app_mention event handler could silently re-introduce the bug without any test catching it at the prepare layer.
• No files require special attention beyond the suggested unit test addition in prepare.test.ts.

This is a comment left during a code review.
Path: extensions/slack/src/monitor/message-handler/prepare.ts
Line: 496

Comment:
**Missing unit test for the prepare.ts change**

The fix bypasses the mention gate when `opts.source === "app_mention"`, but `prepare.test.ts` has no test for `requireMention: true` scenarios at all, and no test exercising the new `opts.source === "app_mention"` branch.

This matters because `opts.wasMentioned: true` is always set in `messages.ts` for `app_mention` events, which means `effectiveWasMentioned` is already `true` and `mentionGate.shouldSkip` would already be `false` in the typical flow — making it hard to verify from tests alone when the gate was previously wrong.

A targeted unit test in `prepare.test.ts` along these lines would lock in the intended behavior:

```ts
it("skips mention gate when source is app_mention even if text-based detection fails", async () => {
  // ctx.botUserId set, mention text absent, requireMention: true, source: "app_mention"
  const prepared = await prepareSlackMessage({
    ctx: buildCtx({ requireMention: true, botUserId: "U_BOT" }),
    account,
    message: buildMessage({ channel: "C1", text: "hello" }), // no <@U_BOT>
    opts: { source: "app_mention", wasMentioned: false },     // simulate text-detection failure
  });
  expect(prepared).not.toBeNull();
});
```

Without this, the fix is hard to verify in isolation and a future refactor of `opts.wasMentioned` propagation could silently reintroduce the bug without a test catching it.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(slack): bypass mention gate for app_..." | Re-trigger Greptile}

I don't think this fixes the reported bug.

app_mention already reaches prepareSlackMessage with wasMentioned: true from events/messages.ts, and the debounce flush preserves that via combinedMentioned || last.opts.wasMentioned in message-handler.ts. prepareSlackMessage then prefers opts.wasMentioned over text detection before mention gating.

So by the time execution reaches this guard in prepare.ts, mentionGate.shouldSkip should already be false for reachable app_mention events. That makes the new opts.source !== "app_mention" condition look behaviorally inert.

If the symptom in #34833 is real, I suspect the bug is earlier in the dedup/race path in message-handler.ts, not the prepare-layer mention gate.

Could you add a failing test that reproduces the actual drop? I think the right place is probably message-handler.app-mention-race.test.ts rather than prepare.ts.