Skip to content

Commit 4b59db6

Browse files
KrisBraunclaude
andcommitted
fix(slack): prompt reconnect when a DM grant predates im:read
The `dms` optional scope group originally requested only im:history/im:write and their mpim counterparts. im:read/mpim:read were added later, because enumerating DM conversations (conversations.list with types=im,mpim) needs the read scopes rather than the history ones. OAuth scopes are fixed when the grant is made, so a connection authorized before that change keeps the older scope set and cannot enumerate DMs. The resulting failure is silent and permanent: - registerDMWebhook only needs im:history, so the DM webhook registers and the connection looks healthy. - listDMChannels guards on im:read and returns early, so `dm_channels` is never populated. - isKnownDMChannel therefore rejects every incoming DM event, and no DM ever syncs — with no error surfaced and no reconnect prompt. That early return exists so a user who declined DM sync isn't force-flagged for reconnection by a missing_scope error. But it could not tell "declined DM sync" apart from "opted in on an older grant", so it silently stranded the latter. Both scopes come from the same group, so im:history without im:read can only mean an older grant, never a decline. Use that to disambiguate: flag re-auth so the user is prompted to reconnect and re-consent. A connection that declined DM sync has neither scope and is left alone, preserving the original guard. Reconnecting is required to recover — a scope the grant never had cannot be obtained without re-consent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b95d6ec commit 4b59db6

2 files changed

Lines changed: 65 additions & 3 deletions

File tree

‎connectors/slack/src/slack.test.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,50 @@ describe("listDMChannels", () => {
494494
expect(markNeedsReauth).not.toHaveBeenCalled();
495495
expect(store.map.has("dm_channels")).toBe(false);
496496
});
497+
498+
it("flags re-auth when the token has im:history but not im:read (opted into DMs on a pre-im:read grant)", async () => {
499+
// A token granted before im:read/mpim:read joined the `dms` scope group
500+
// carries im:history but not im:read. That user OPTED IN to DM sync, so
501+
// silently no-oping strands them: the DM webhook registers (it only needs
502+
// im:history), but listDMChannels never populates `dm_channels`, so
503+
// isKnownDMChannel rejects every incoming DM event forever — while the
504+
// connection still reports itself healthy. Both scopes come from the same
505+
// group, so im:history-without-im:read can only mean a stale grant, never
506+
// a decline; prompt a reconnect to re-consent.
507+
const store = makeStore({});
508+
const integrationsGet = vi.fn().mockResolvedValue({
509+
token: "xoxp-test",
510+
scopes: ["channels:history", "im:history", "im:write", "mpim:history"], // pre-fix grant
511+
});
512+
const markNeedsReauth = vi.fn().mockResolvedValue(undefined);
513+
const create = vi.fn(async () => ({ token: "cb" }) as never);
514+
const tools = {
515+
store,
516+
integrations: { get: integrationsGet, markNeedsReauth },
517+
network: { createWebhook: vi.fn() },
518+
files: {},
519+
callbacks: { create },
520+
tasks: { runTask: vi.fn(async () => "task-token"), scheduleRecurring: vi.fn(async () => {}) },
521+
};
522+
const slack = new Slack("twist-instance-1" as never, { getTools: () => tools } as never);
523+
524+
const fetchMock = vi.fn();
525+
vi.stubGlobal("fetch", fetchMock);
526+
try {
527+
await (slack as unknown as {
528+
listDMChannels: (c: string) => Promise<void>;
529+
}).listDMChannels("C1");
530+
} finally {
531+
vi.unstubAllGlobals();
532+
}
533+
534+
// Still no API call: the grant genuinely lacks the enumeration scope, so
535+
// conversations.list would only return missing_scope.
536+
expect(fetchMock).not.toHaveBeenCalled();
537+
// ...but unlike a decline, surface it so the user can reconnect.
538+
expect(markNeedsReauth).toHaveBeenCalledWith("C1");
539+
expect(store.map.has("dm_channels")).toBe(false);
540+
});
497541
});
498542

499543
describe("extractSlackMessageReactions (custom emoji)", () => {

‎connectors/slack/src/slack.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,11 +1683,16 @@ export class Slack extends Connector<Slack> {
16831683
* immediately if proactively registered (see `createDirectMessage`).
16841684
*
16851685
* Requires the `im:read` optional scope (part of the `dms` group) to have
1686-
* been granted — no-ops otherwise (the user declined DM sync at connect
1687-
* time). Without this guard, a token missing `im:read` would hit
1686+
* been granted. Without that guard, a token missing `im:read` would hit
16881687
* `conversations.list`, get back `missing_scope` (a permanent error in
16891688
* `SLACK_AUTH_ERRORS`), and force-flag the entire connection as needing
16901689
* reconnection even though nothing is actually broken.
1690+
*
1691+
* When `im:read` is absent, `im:history` disambiguates why: both come from
1692+
* the `dms` group, so having `im:history` alone means the user opted into DM
1693+
* sync on a grant made before `im:read` joined that group. That grant can
1694+
* never enumerate DMs, so re-auth is flagged to prompt a reconnect. A user
1695+
* who declined DM sync has neither scope and is left alone.
16911696
*/
16921697
async listDMChannels(channelId: string): Promise<void> {
16931698
const now = Date.now();
@@ -1700,7 +1705,20 @@ export class Slack extends Connector<Slack> {
17001705
console.warn("listDMChannels: Slack token unavailable");
17011706
return;
17021707
}
1703-
if (!token.scopes?.includes("im:read")) return; // optional scope declined
1708+
if (!token.scopes?.includes("im:read")) {
1709+
// im:read and im:history are granted together by the `dms` group, so a
1710+
// token carrying im:history WITHOUT im:read predates im:read joining
1711+
// that group — the user opted into DM sync but their grant can't
1712+
// enumerate DMs. Returning quietly would strand them permanently: the
1713+
// DM webhook registers (im:history is enough), but `dm_channels` stays
1714+
// empty, so isKnownDMChannel drops every incoming DM while the
1715+
// connection still reports healthy. Prompt a reconnect to re-consent.
1716+
// A user who declined DMs has neither scope and is left alone.
1717+
if (token.scopes?.includes("im:history")) {
1718+
await this.tools.integrations.markNeedsReauth(channelId);
1719+
}
1720+
return;
1721+
}
17041722

17051723
const api = new SlackApi(token.token);
17061724

0 commit comments

Comments
 (0)