Skip to content

Commit 5f0cbd0

Browse files
steipetesteipete
committed
refactor(gateway): dedupe auth and discord monitor suites
1 parent ab8b8da commit 5f0cbd0

14 files changed

+432
-498
lines changed

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,31 +8,27 @@ describe("resolveDiscordDmCommandAccess", () => {
88
tag: "alice#0001",
99
};
1010

11-
it("allows open DMs and keeps command auth enabled without allowlist entries", async () => {
12-
const result = await resolveDiscordDmCommandAccess({
11+
async function resolveOpenDmAccess(configuredAllowFrom: string[]) {
12+
return await resolveDiscordDmCommandAccess({
1313
accountId: "default",
1414
dmPolicy: "open",
15-
configuredAllowFrom: [],
15+
configuredAllowFrom,
1616
sender,
1717
allowNameMatching: false,
1818
useAccessGroups: true,
1919
readStoreAllowFrom: async () => [],
2020
});
21+
}
22+
23+
it("allows open DMs and keeps command auth enabled without allowlist entries", async () => {
24+
const result = await resolveOpenDmAccess([]);
2125

2226
expect(result.decision).toBe("allow");
2327
expect(result.commandAuthorized).toBe(true);
2428
});
2529

2630
it("marks command auth true when sender is allowlisted", async () => {
27-
const result = await resolveDiscordDmCommandAccess({
28-
accountId: "default",
29-
dmPolicy: "open",
30-
configuredAllowFrom: ["discord:123"],
31-
sender,
32-
allowNameMatching: false,
33-
useAccessGroups: true,
34-
readStoreAllowFrom: async () => [],
35-
});
31+
const result = await resolveOpenDmAccess(["discord:123"]);
3632

3733
expect(result.decision).toBe("allow");
3834
expect(result.commandAuthorized).toBe(true);

‎src/discord/monitor/message-handler.process.test.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,18 @@ function getLastDispatchCtx():
168168
return params?.ctx;
169169
}
170170

171+
async function runProcessDiscordMessage(ctx: unknown): Promise<void> {
172+
// oxlint-disable-next-line typescript/no-explicit-any
173+
await processDiscordMessage(ctx as any);
174+
}
175+
176+
async function runInPartialStreamMode(): Promise<void> {
177+
const ctx = await createBaseContext({
178+
discordConfig: { streamMode: "partial" },
179+
});
180+
await runProcessDiscordMessage(ctx);
181+
}
182+
171183
describe("processDiscordMessage ack reactions", () => {
172184
it("skips ack reactions for group-mentions when mentions are not required", async () => {
173185
const ctx = await createBaseContext({
@@ -543,12 +555,7 @@ describe("processDiscordMessage draft streaming", () => {
543555
return { queuedFinal: false, counts: { final: 0, tool: 0, block: 0 } };
544556
});
545557

546-
const ctx = await createBaseContext({
547-
discordConfig: { streamMode: "partial" },
548-
});
549-
550-
// oxlint-disable-next-line typescript/no-explicit-any
551-
await processDiscordMessage(ctx as any);
558+
await runInPartialStreamMode();
552559

553560
const updates = draftStream.update.mock.calls.map((call) => call[0]);
554561
for (const text of updates) {
@@ -567,12 +574,7 @@ describe("processDiscordMessage draft streaming", () => {
567574
return { queuedFinal: false, counts: { final: 0, tool: 0, block: 0 } };
568575
});
569576

570-
const ctx = await createBaseContext({
571-
discordConfig: { streamMode: "partial" },
572-
});
573-
574-
// oxlint-disable-next-line typescript/no-explicit-any
575-
await processDiscordMessage(ctx as any);
577+
await runInPartialStreamMode();
576578

577579
expect(draftStream.update).not.toHaveBeenCalled();
578580
});

‎src/discord/monitor/native-command.model-picker.test.ts‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,24 @@ async function runSubmitButton(params: {
167167
return submitInteraction;
168168
}
169169

170+
async function runModelSelect(params: {
171+
context: ModelPickerContext;
172+
data?: PickerSelectData;
173+
userId?: string;
174+
values?: string[];
175+
}) {
176+
const select = createDiscordModelPickerFallbackSelect(params.context);
177+
const selectInteraction = createInteraction({
178+
userId: params.userId ?? "owner",
179+
values: params.values ?? ["gpt-4o"],
180+
});
181+
await select.run(
182+
selectInteraction as unknown as PickerSelectInteraction,
183+
params.data ?? createModelsViewSelectData(),
184+
);
185+
return selectInteraction;
186+
}
187+
170188
function expectDispatchedModelSelection(params: {
171189
dispatchSpy: { mock: { calls: Array<[unknown]> } };
172190
model: string;
@@ -270,15 +288,7 @@ describe("Discord model picker interactions", () => {
270288
.spyOn(dispatcherModule, "dispatchReplyWithDispatcher")
271289
.mockResolvedValue({} as never);
272290

273-
const select = createDiscordModelPickerFallbackSelect(context);
274-
const selectInteraction = createInteraction({
275-
userId: "owner",
276-
values: ["gpt-4o"],
277-
});
278-
279-
const selectData = createModelsViewSelectData();
280-
281-
await select.run(selectInteraction as unknown as PickerSelectInteraction, selectData);
291+
const selectInteraction = await runModelSelect({ context });
282292

283293
expect(selectInteraction.update).toHaveBeenCalledTimes(1);
284294
expect(dispatchSpy).not.toHaveBeenCalled();
@@ -315,15 +325,7 @@ describe("Discord model picker interactions", () => {
315325
.spyOn(timeoutModule, "withTimeout")
316326
.mockRejectedValue(new Error("timeout"));
317327

318-
const select = createDiscordModelPickerFallbackSelect(context);
319-
const selectInteraction = createInteraction({
320-
userId: "owner",
321-
values: ["gpt-4o"],
322-
});
323-
324-
const selectData = createModelsViewSelectData();
325-
326-
await select.run(selectInteraction as unknown as PickerSelectInteraction, selectData);
328+
await runModelSelect({ context });
327329

328330
const button = createDiscordModelPickerFallbackButton(context);
329331
const submitInteraction = createInteraction({ userId: "owner" });

‎src/discord/monitor/provider.lifecycle.test.ts‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,11 @@ describe("runDiscordGatewayLifecycle", () => {
143143
return { emitter, gateway };
144144
}
145145

146+
async function emitGatewayOpenAndWait(emitter: EventEmitter, delayMs = 30000): Promise<void> {
147+
emitter.emit("debug", "WebSocket connection opened");
148+
await vi.advanceTimersByTimeAsync(delayMs);
149+
}
150+
146151
it("cleans up thread bindings when exec approvals startup fails", async () => {
147152
const { runDiscordGatewayLifecycle } = await import("./provider.lifecycle.js");
148153
const { lifecycleParams, start, stop, threadStop, releaseEarlyGatewayErrorGuard } =
@@ -260,12 +265,9 @@ describe("runDiscordGatewayLifecycle", () => {
260265
});
261266
getDiscordGatewayEmitterMock.mockReturnValueOnce(emitter);
262267
waitForDiscordGatewayStopMock.mockImplementationOnce(async () => {
263-
emitter.emit("debug", "WebSocket connection opened");
264-
await vi.advanceTimersByTimeAsync(30000);
265-
emitter.emit("debug", "WebSocket connection opened");
266-
await vi.advanceTimersByTimeAsync(30000);
267-
emitter.emit("debug", "WebSocket connection opened");
268-
await vi.advanceTimersByTimeAsync(30000);
268+
await emitGatewayOpenAndWait(emitter);
269+
await emitGatewayOpenAndWait(emitter);
270+
await emitGatewayOpenAndWait(emitter);
269271
});
270272

271273
const { lifecycleParams } = createLifecycleHarness({ gateway });
@@ -299,22 +301,17 @@ describe("runDiscordGatewayLifecycle", () => {
299301
});
300302
getDiscordGatewayEmitterMock.mockReturnValueOnce(emitter);
301303
waitForDiscordGatewayStopMock.mockImplementationOnce(async () => {
302-
emitter.emit("debug", "WebSocket connection opened");
303-
await vi.advanceTimersByTimeAsync(30000);
304+
await emitGatewayOpenAndWait(emitter);
304305

305306
// Successful reconnect (READY/RESUMED sets isConnected=true), then
306307
// quick drop before the HELLO timeout window finishes.
307308
gateway.isConnected = true;
308-
emitter.emit("debug", "WebSocket connection opened");
309-
await vi.advanceTimersByTimeAsync(10);
309+
await emitGatewayOpenAndWait(emitter, 10);
310310
emitter.emit("debug", "WebSocket connection closed with code 1006");
311311
gateway.isConnected = false;
312312

313-
emitter.emit("debug", "WebSocket connection opened");
314-
await vi.advanceTimersByTimeAsync(30000);
315-
316-
emitter.emit("debug", "WebSocket connection opened");
317-
await vi.advanceTimersByTimeAsync(30000);
313+
await emitGatewayOpenAndWait(emitter);
314+
await emitGatewayOpenAndWait(emitter);
318315
});
319316

320317
const { lifecycleParams } = createLifecycleHarness({ gateway });

‎src/discord/probe.ts‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,26 +38,34 @@ async function fetchDiscordApplicationMe(
3838
timeoutMs: number,
3939
fetcher: typeof fetch,
4040
): Promise<{ id?: string; flags?: number } | undefined> {
41-
const normalized = normalizeDiscordToken(token);
42-
if (!normalized) {
43-
return undefined;
44-
}
4541
try {
46-
const res = await fetchWithTimeout(
47-
`${DISCORD_API_BASE}/oauth2/applications/@me`,
48-
{ headers: { Authorization: `Bot ${normalized}` } },
49-
timeoutMs,
50-
getResolvedFetch(fetcher),
51-
);
52-
if (!res.ok) {
42+
const appResponse = await fetchDiscordApplicationMeResponse(token, timeoutMs, fetcher);
43+
if (!appResponse || !appResponse.ok) {
5344
return undefined;
5445
}
55-
return (await res.json()) as { id?: string; flags?: number };
46+
return (await appResponse.json()) as { id?: string; flags?: number };
5647
} catch {
5748
return undefined;
5849
}
5950
}
6051

52+
async function fetchDiscordApplicationMeResponse(
53+
token: string,
54+
timeoutMs: number,
55+
fetcher: typeof fetch,
56+
): Promise<Response | undefined> {
57+
const normalized = normalizeDiscordToken(token);
58+
if (!normalized) {
59+
return undefined;
60+
}
61+
return await fetchWithTimeout(
62+
`${DISCORD_API_BASE}/oauth2/applications/@me`,
63+
{ headers: { Authorization: `Bot ${normalized}` } },
64+
timeoutMs,
65+
getResolvedFetch(fetcher),
66+
);
67+
}
68+
6169
export function resolveDiscordPrivilegedIntentsFromFlags(
6270
flags: number,
6371
): DiscordPrivilegedIntentsSummary {
@@ -198,17 +206,14 @@ export async function fetchDiscordApplicationId(
198206
timeoutMs: number,
199207
fetcher: typeof fetch = fetch,
200208
): Promise<string | undefined> {
201-
const normalized = normalizeDiscordToken(token);
202-
if (!normalized) {
209+
if (!normalizeDiscordToken(token)) {
203210
return undefined;
204211
}
205212
try {
206-
const res = await fetchWithTimeout(
207-
`${DISCORD_API_BASE}/oauth2/applications/@me`,
208-
{ headers: { Authorization: `Bot ${normalized}` } },
209-
timeoutMs,
210-
getResolvedFetch(fetcher),
211-
);
213+
const res = await fetchDiscordApplicationMeResponse(token, timeoutMs, fetcher);
214+
if (!res) {
215+
return undefined;
216+
}
212217
if (res.ok) {
213218
const json = (await res.json()) as { id?: string };
214219
if (json?.id) {

‎src/gateway/node-invoke-system-run-approval-match.test.ts‎

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,29 @@ function expectMismatch(
1919
expect(result.code).toBe(code);
2020
}
2121

22+
function expectV1BindingMatch(params: {
23+
argv: string[];
24+
requestCommand: string;
25+
commandArgv?: string[];
26+
}) {
27+
const result = evaluateSystemRunApprovalMatch({
28+
argv: params.argv,
29+
request: {
30+
host: "node",
31+
command: params.requestCommand,
32+
commandArgv: params.commandArgv,
33+
systemRunBinding: buildSystemRunApprovalBinding({
34+
argv: params.argv,
35+
cwd: null,
36+
agentId: null,
37+
sessionKey: null,
38+
}).binding,
39+
},
40+
binding: defaultBinding,
41+
});
42+
expect(result).toEqual({ ok: true });
43+
}
44+
2245
describe("evaluateSystemRunApprovalMatch", () => {
2346
test("rejects approvals that do not carry v1 binding", () => {
2447
const result = evaluateSystemRunApprovalMatch({
@@ -33,21 +56,10 @@ describe("evaluateSystemRunApprovalMatch", () => {
3356
});
3457

3558
test("enforces exact argv binding in v1 object", () => {
36-
const result = evaluateSystemRunApprovalMatch({
59+
expectV1BindingMatch({
3760
argv: ["echo", "SAFE"],
38-
request: {
39-
host: "node",
40-
command: "echo SAFE",
41-
systemRunBinding: buildSystemRunApprovalBinding({
42-
argv: ["echo", "SAFE"],
43-
cwd: null,
44-
agentId: null,
45-
sessionKey: null,
46-
}).binding,
47-
},
48-
binding: defaultBinding,
61+
requestCommand: "echo SAFE",
4962
});
50-
expect(result).toEqual({ ok: true });
5163
});
5264

5365
test("rejects argv mismatch in v1 object", () => {
@@ -124,21 +136,10 @@ describe("evaluateSystemRunApprovalMatch", () => {
124136
});
125137

126138
test("uses v1 binding even when legacy command text diverges", () => {
127-
const result = evaluateSystemRunApprovalMatch({
139+
expectV1BindingMatch({
128140
argv: ["echo", "SAFE"],
129-
request: {
130-
host: "node",
131-
command: "echo STALE",
132-
commandArgv: ["echo STALE"],
133-
systemRunBinding: buildSystemRunApprovalBinding({
134-
argv: ["echo", "SAFE"],
135-
cwd: null,
136-
agentId: null,
137-
sessionKey: null,
138-
}).binding,
139-
},
140-
binding: defaultBinding,
141+
requestCommand: "echo STALE",
142+
commandArgv: ["echo STALE"],
141143
});
142-
expect(result).toEqual({ ok: true });
143144
});
144145
});

0 commit comments

Comments
 (0)