← Back to PRs

#19567: Fix: tighten Slack multi-account event filtering via api_app_id

by TARS-Nolan open 2026-02-17 23:44 View on GitHub →
channel: slack agents size: M
#### Summary In multi-account Slack setups, all inbound messages were routing to the same agent regardless of which bot was @mentioned. The `shouldDropMismatchedSlackEvent` guard in `context.ts` only dropped events when **both** the provider and the incoming event had a non-empty `api_app_id` that differed — so events missing `api_app_id` passed silently to every provider instance. lobster-biscuit #### Repro Steps 1. Configure two Slack accounts (e.g. `og` and `makilala`) in `openclaw.config.yaml` 2. @mention the `og` bot in Slack 3. **Expected:** routed to `og` agent 4. **Actual:** routed to `makilala` agent (or whichever provider wins the race) #### Root Cause `shouldDropMismatchedSlackEvent` (`src/slack/monitor/context.ts:367`): ```ts // Before — only drops when both values are present and differ if (params.apiAppId && incomingApiAppId && incomingApiAppId !== params.apiAppId) { return true; } ``` When `incomingApiAppId` is empty/absent the condition short-circuits to `false`, so the event is accepted by every provider. #### Behavior Changes - When a provider has `apiAppId` (from `auth.test()`), events **without** `api_app_id` are now dropped (previously accepted). Slack always includes `api_app_id` in Socket Mode and Events API envelopes, so this should not affect normal operation. - Once `api_app_id` is verified as matching, the `team_id` check is skipped (short-circuit `return false`). - `auth.test()` failures are now logged as errors at startup (previously silent). - Missing `api_app_id` after a successful `auth.test()` emits a warning. - All relevant log messages now include `accountId` for easier multi-account debugging. #### Codebase and GitHub Search - Searched `shouldDropMismatchedSlackEvent` — only called in `src/slack/monitor/provider.ts` and the existing test files. - Searched for other per-account event filtering patterns — no equivalent logic in extension channels (Teams, Matrix, etc.) uses the same guard. #### Tests New file: `src/slack/monitor/context.multi-account.test.ts` — 7 unit tests: | Test | Result | |---|---| | drops events with mismatched `api_app_id` when `apiAppId` is set | ✅ | | accepts events with matching `api_app_id` | ✅ | | drops events with missing `api_app_id` when `apiAppId` is set (multi-account safety) | ✅ | | accepts events without `api_app_id` when `apiAppId` is not set (backward compat) | ✅ | | multiple accounts with different `api_app_id`s only process their own events | ✅ | | drops events with mismatched `team_id` when `apiAppId` is not available | ✅ | | accepts events with matching `team_id` when `apiAppId` is not available | ✅ | Full suite: `pnpm test` — 270 tests, 45 files, all passing. **Sign-Off** - Models used: claude-sonnet-4-6 - Submitter effort: assisted (reviewed diff, confirmed approach, approved PR) - Agent notes: root cause confirmed by reading `shouldDropMismatchedSlackEvent` logic; fix verified by unit tests covering all filtering branches; backward compat maintained via `apiAppId`-absent fallback path. <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes multi-account Slack event routing by tightening the `shouldDropMismatchedSlackEvent` guard in `context.ts`. Previously, events missing `api_app_id` slipped past the filter and were accepted by every provider instance, causing cross-account routing. Now, when a provider has a known `api_app_id`, events without it are dropped for safety. - **Core fix** (`context.ts`): Restructured `shouldDropMismatchedSlackEvent` to require `api_app_id` on incoming events when the provider has one configured, with `team_id` fallback when `api_app_id` is unavailable. - **Improved diagnostics** (`provider.ts`): `auth.test()` failures now log errors at startup, and missing `api_app_id` emits a warning. All log messages include `accountId`. - **Announce target routing** (`channel.ts`): Added `preferSessionLookupForAnnounceTarget: true` to the Slack plugin metadata so announce targets resolve the correct `accountId` from session data in multi-account setups. - **Tests**: New `context.multi-account.test.ts` with 7 tests covering all filtering branches. Additional test in `sessions-announce-target.test.ts` for Slack `accountId` hydration. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with minor style suggestions; the core logic change is correct and well-tested. - The fix correctly addresses the root cause of cross-account event routing. The new filtering logic is sound — it tightens the guard without breaking backward compatibility (when `apiAppId` is absent, behavior is unchanged). All branches are covered by new unit tests. The only findings are minor style nits (unused type cast property, redundant warning log on auth failure). - No files require special attention. `src/slack/monitor/context.ts` has a minor unused type property, and `src/slack/monitor/provider.ts` has a redundant warning log, but neither affects correctness. <sub>Last reviewed commit: 7a268f4</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs