Skip to content

Commit e0e7bae

Browse files
authored
fix(discord): handle PluralKit DM pairing ids
Fix Discord DM pairing for PluralKit senders by storing the pairing identity with the same `pk:<member-id>` form used at inbound lookup time. Also recognizes both canonical direct DM session keys and account-scoped direct DM session keys as DM approval sessions. Focused proof: `node scripts/run-vitest.mjs extensions/discord/src/approval-native.test.ts extensions/discord/src/monitor/dm-command-auth.test.ts extensions/discord/src/monitor/dm-command-decision.test.ts extensions/discord/src/monitor/message-handler.preflight.test.ts` passed with 4 files and 82 tests. Closes #86332 Co-authored-by: Sanjays2402 <51058514+Sanjays2402@users.noreply.github.com>
1 parent b9dc3c3 commit e0e7bae

5 files changed

Lines changed: 148 additions & 5 deletions

File tree

extensions/discord/src/approval-native.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,56 @@ describe("createDiscordNativeApprovalAdapter", () => {
170170
expect(target).toBeNull();
171171
});
172172

173+
it("falls back to approver DMs for canonical Discord direct sessions", async () => {
174+
const adapter = createDiscordNativeApprovalAdapter();
175+
176+
const target = await adapter.native?.resolveOriginTarget?.({
177+
cfg: NATIVE_DELIVERY_CFG as never,
178+
accountId: "main",
179+
approvalKind: "plugin",
180+
request: {
181+
id: "abc",
182+
request: {
183+
title: "Plugin approval",
184+
description: "Let plugin proceed",
185+
sessionKey: "agent:main:discord:direct:123456789",
186+
turnSourceChannel: "discord",
187+
turnSourceTo: "123456789",
188+
turnSourceAccountId: "main",
189+
},
190+
createdAtMs: 1,
191+
expiresAtMs: 2,
192+
},
193+
});
194+
195+
expect(target).toBeNull();
196+
});
197+
198+
it("falls back to approver DMs for account-scoped Discord direct sessions", async () => {
199+
const adapter = createDiscordNativeApprovalAdapter();
200+
201+
const target = await adapter.native?.resolveOriginTarget?.({
202+
cfg: NATIVE_DELIVERY_CFG as never,
203+
accountId: "main",
204+
approvalKind: "plugin",
205+
request: {
206+
id: "abc",
207+
request: {
208+
title: "Plugin approval",
209+
description: "Let plugin proceed",
210+
sessionKey: "agent:main:discord:default:direct:123456789",
211+
turnSourceChannel: "discord",
212+
turnSourceTo: "123456789",
213+
turnSourceAccountId: "main",
214+
},
215+
createdAtMs: 1,
216+
expiresAtMs: 2,
217+
},
218+
});
219+
220+
expect(target).toBeNull();
221+
});
222+
173223
it("ignores session-store turn targets for Discord DM sessions", async () => {
174224
writeStore({
175225
"agent:main:discord:dm:123456789": {

extensions/discord/src/approval-native.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,18 @@ function extractDiscordSessionKind(sessionKey?: string | null): "channel" | "gro
3636
if (!sessionKey) {
3737
return null;
3838
}
39-
const match = sessionKey.match(/discord:(channel|group|dm):/);
39+
// DM session keys use the `direct` peer kind in the normalized form
40+
// (`agent:<id>:discord[:account]:direct:<userId>`); legacy keys may still use
41+
// `dm`. Treat both as the same logical kind for downstream comparisons.
42+
const match = sessionKey.match(/discord:(?:[^:]+:)?(channel|group|dm|direct):/);
4043
if (!match) {
4144
return null;
4245
}
43-
return match[1] as "channel" | "group" | "dm";
46+
const raw = match[1];
47+
if (raw === "direct") {
48+
return "dm";
49+
}
50+
return raw as "channel" | "group" | "dm";
4451
}
4552

4653
function normalizeDiscordOriginChannelId(value?: string | null): string | null {

extensions/discord/src/monitor/dm-command-auth.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,24 @@ describe("resolveDiscordDmCommandAccess", () => {
167167
expect(dmCommandAuthorized(result)).toBe(true);
168168
});
169169

170+
it("authorizes PluralKit senders from prefixed pairing-store allowlist entries", async () => {
171+
const result = await resolveDiscordDmCommandAccess({
172+
accountId: "default",
173+
dmPolicy: "pairing",
174+
configuredAllowFrom: [],
175+
sender: {
176+
id: "pk-member-1",
177+
name: "Echo",
178+
tag: "Echo",
179+
},
180+
allowNameMatching: false,
181+
readStoreAllowFrom: async () => ["pk:pk-member-1"],
182+
});
183+
184+
expect(result.senderAccess.decision).toBe("allow");
185+
expect(dmCommandAuthorized(result)).toBe(true);
186+
});
187+
170188
it("authorizes allowlist DMs from a Discord channel audience access group", async () => {
171189
canViewDiscordGuildChannelMock.mockResolvedValueOnce(true);
172190

extensions/discord/src/monitor/message-handler.dm-preflight.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ async function loadDiscordSendRuntime() {
2424
return await discordSendRuntimePromise;
2525
}
2626

27+
function resolveDiscordDmPairingSenderId(sender: DiscordSenderIdentity): string {
28+
return sender.isPluralKit ? `pk:${sender.id}` : sender.id;
29+
}
30+
2731
export async function resolveDiscordDmPreflightAccess(params: {
2832
preflight: DiscordMessagePreflightParams;
2933
author: User;
@@ -79,10 +83,15 @@ export async function resolveDiscordDmPreflightAccess(params: {
7983
await handleDiscordDmCommandDecision({
8084
senderAccess: dmAccess.senderAccess,
8185
accountId: params.resolvedAccountId,
86+
// Use the resolved sender identity (e.g. PluralKit member UUID) here so
87+
// the pairing record is keyed under the same stableId that
88+
// resolveDiscordDmCommandAccess / createDiscordDmIngressSubject use on
89+
// subsequent inbound messages. Previously this used the raw gateway
90+
// author id, which only matched non-PK users.
8291
sender: {
83-
id: params.author.id,
84-
tag: formatDiscordUserTag(params.author),
85-
name: params.author.username ?? undefined,
92+
id: resolveDiscordDmPairingSenderId(params.sender),
93+
tag: params.sender.tag ?? formatDiscordUserTag(params.author),
94+
name: params.sender.name ?? params.author.username ?? undefined,
8695
},
8796
onPairingCreated: async (code) => {
8897
logVerbose(

extensions/discord/src/monitor/message-handler.preflight.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,65 @@ describe("preflightDiscordMessage", () => {
961961
expect(preflight.canonicalMessageId).toBe("orig-123");
962962
});
963963

964+
it("uses the resolved PluralKit member id when creating DM pairing requests", async () => {
965+
fetchPluralKitMessageInfoMock.mockResolvedValue({
966+
id: "proxy-dm-1",
967+
original: "orig-dm-1",
968+
member: { id: "pk-member-1", name: "Echo" },
969+
system: { id: "system-1", name: "System" },
970+
});
971+
resolveDiscordDmCommandAccessMock.mockResolvedValue({
972+
senderAccess: {
973+
allowed: false,
974+
decision: "pairing",
975+
reasonCode: "dm_policy_pairing_required",
976+
},
977+
commandAccess: {
978+
authorized: false,
979+
},
980+
});
981+
982+
const result = await runDmPreflight({
983+
channelId: "dm-channel-pk-1",
984+
message: createDiscordMessage({
985+
id: "proxy-dm-1",
986+
channelId: "dm-channel-pk-1",
987+
content: "hello",
988+
webhookId: "pluralkit-webhook-1",
989+
author: {
990+
id: "webhook-author",
991+
bot: true,
992+
username: "PluralKit",
993+
},
994+
}),
995+
discordConfig: {
996+
allowBots: true,
997+
dmPolicy: "pairing",
998+
pluralkit: { enabled: true },
999+
} as DiscordConfig,
1000+
});
1001+
1002+
expect(result).toBeNull();
1003+
expect(resolveDiscordDmCommandAccessMock).toHaveBeenCalledWith(
1004+
expect.objectContaining({
1005+
sender: {
1006+
id: "pk-member-1",
1007+
name: "Echo",
1008+
tag: "Echo",
1009+
},
1010+
}),
1011+
);
1012+
expect(handleDiscordDmCommandDecisionMock).toHaveBeenCalledWith(
1013+
expect.objectContaining({
1014+
sender: {
1015+
id: "pk:pk-member-1",
1016+
tag: "Echo",
1017+
name: "Echo",
1018+
},
1019+
}),
1020+
);
1021+
});
1022+
9641023
it("skips PluralKit lookup for bound-thread webhook echoes", async () => {
9651024
const threadBinding = createThreadBinding({
9661025
targetKind: "session",

0 commit comments

Comments
 (0)