← Back to PRs

#10643: fix(slack): classify D-prefix DMs correctly when channel_type disagrees

by mcaxtr open 2026-02-06 20:05 View on GitHub →
channel: slack size: S trusted-contributor experienced-contributor
Fixes #8421 ## Problem Slack sometimes sends events where the `channel_type` field contradicts the channel ID prefix. For example, a DM channel with ID `D0ABC123` may arrive with `channel_type: "channel"` instead of `"im"`. This causes the bot to misclassify DMs as public channels, leading to: - Wrong session key: messages route to `agent:main:slack:channel:D0ABC123` instead of `agent:main:main` (when `dmScope` is `"main"`) - Wrong `ChatType`: set to `"channel"` instead of `"direct"` - Policy bypass: DM policy checks are skipped because the message is treated as a channel message - Broken context: conversation history is isolated per-channel instead of unified in the DM session ## Root Cause `normalizeSlackChannelType()` in `context.ts` trusts the event's `channel_type` field when it's a recognized value (`"im"`, `"mpim"`, `"channel"`, `"group"`), even when the channel ID prefix contradicts it. Since Slack channel IDs with `D` prefix are **always** DMs by Slack's own convention, the `channel_type` field should be overridden when it disagrees. ## Fix Added a cross-validation check in `normalizeSlackChannelType()`: when the channel ID starts with `D` (inferred as `"im"`), override any contradicting `channel_type` value. This is the canonical normalization function used across the entire Slack integration — all downstream callers (`prepareSlackMessage`, `isChannelAllowed`, `resolveSlackSystemEventSessionKey`, member events) are automatically protected. Only `D`-prefix is overridden because it unambiguously identifies DMs. `G`-prefix is intentionally left alone since it's ambiguous (can be either a private channel or a group DM — Slack's API provides the correct `channel_type` in that case). ## Test Plan TDD approach — all 8 new tests were written first and confirmed failing before the fix: - [x] `normalizeSlackChannelType` unit tests (3 cases): - Overrides wrong `channel_type` (`"channel"`, `"group"`, `"mpim"`) for D-prefix channels → returns `"im"` - Preserves correct `channel_type` (`"im"`) for D-prefix channels - Does not override G-prefix channels (ambiguous prefix) - [x] `prepareSlackMessage` integration tests (2 cases): - D-prefix + wrong `channel_type: "channel"` → `isDirectMessage: true`, session routes to `agent:main:main`, `ChatType: "direct"` - D-prefix + missing `channel_type` → correctly inferred as DM - [x] All 5337 existing tests pass (no regressions) - [x] Full CI gate: `pnpm build && pnpm check && pnpm test` <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> - Updates `normalizeSlackChannelType()` to cross-check the channel ID prefix and override contradictory `channel_type` values for `D*` channels (treat as `im`). - Adds unit tests covering D-prefix overrides, preserving correct values, and leaving ambiguous `G*` channels unchanged. - Adds inbound `prepareSlackMessage` contract tests to ensure D-prefix DMs route to the main DM session and produce `ChatType: direct` even when Slack mislabels `channel_type`. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk. - Change is narrowly scoped to normalization logic, has clear test coverage for the regression scenario (D-prefix DM misclassified), and doesn’t affect ambiguous prefixes; no additional behavioral regressions were found in call sites using this normalization. - No files require special attention <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs