From 2ba1735b8349195f79869a6862632774769cb7dd Mon Sep 17 00:00:00 2001 From: Abdulkhalek Muhammad Date: Tue, 28 Jul 2026 15:37:31 +0300 Subject: [PATCH] fix(tempvoice): read hub configs from the database in the dashboard loadTempVoiceConfig() is only ever called from the bot's ready event, so the tempVoice module cache is permanently empty in the dashboard API process. The routes read it anyway: GET returned [] with rows in the database, and POST's "already a hub" pre-check always passed, so the insert hit the unique index and surfaced as a 500. Add fetchGuildConfigs / fetchConfigByHubChannel for read-through and use them in the dashboard; the sync getters stay for the bot's voiceStateUpdate hot path. Map Prisma P2002 to a 400, since check-then-insert is still racy. Two defects with the same cause go with it: - updateGuildConfig used `where: { id }` with no guildId, so a guild admin could edit another guild's config by id. Now scoped to { id, guildId }. - removeGuildConfig returned false on a cache miss without touching the database, so dashboard deletes silently no-op'd. Now deleteMany({ id, guildId }), with the cache updated only when present. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/server/features/tempvoice/routes.ts | 55 +++++++++++---- .../features/tempvoice/tempvoice.test.ts | 64 ++++++++++++++--- packages/systems/src/tempVoice/config.ts | 70 ++++++++++++++++--- 3 files changed, 157 insertions(+), 32 deletions(-) diff --git a/apps/dashboard/src/server/features/tempvoice/routes.ts b/apps/dashboard/src/server/features/tempvoice/routes.ts index bdaeca35..d247027f 100644 --- a/apps/dashboard/src/server/features/tempvoice/routes.ts +++ b/apps/dashboard/src/server/features/tempvoice/routes.ts @@ -2,16 +2,30 @@ import type { FastifyInstance } from "fastify"; import { withDocs } from "../../shared/openapi-schemas.js"; import { requireAuth, requireGuildAdmin, requirePermission } from "../../shared/middleware.js"; import { - getGuildConfigs, + fetchGuildConfigs, addGuildConfig, updateGuildConfig, removeGuildConfig, - getConfigByHubChannel, + fetchConfigByHubChannel, } from "@fluxcore/systems/tempVoice/config"; import { MAX_TEMPVOICE_CONFIGS_PER_GUILD } from "@fluxcore/systems/tempVoice/constants"; import { channelExistsInGuild } from "../../shared/discordApi.js"; import { notifyCacheInvalidation } from "@fluxcore/systems/actions/persistence"; +/** + * Prisma unique-constraint violation. The hub-already-in-use checks below read + * the database first, but two concurrent requests can still both pass them, so + * the constraint stays the last line of defence. + */ +function isUniqueViolation(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + (error as { code: unknown }).code === "P2002" + ); +} + export function registerTempVoiceRoutes(app: FastifyInstance): void { // GET all configs for a guild app.get( @@ -25,7 +39,7 @@ export function registerTempVoiceRoutes(app: FastifyInstance): void { }, async (request, reply) => { const { guildId } = request.params as { guildId: string }; - const configs = getGuildConfigs(guildId); + const configs = await fetchGuildConfigs(guildId); reply.send(configs); }, ); @@ -72,12 +86,12 @@ export function registerTempVoiceRoutes(app: FastifyInstance): void { return; } - if (getConfigByHubChannel(body.hubChannelId)) { + if (await fetchConfigByHubChannel(body.hubChannelId)) { reply.code(400).send({ error: "This channel is already a temp voice hub" }); return; } - const existing = getGuildConfigs(guildId); + const existing = await fetchGuildConfigs(guildId); if (existing.length >= MAX_TEMPVOICE_CONFIGS_PER_GUILD) { reply.code(400).send({ error: `Config limit reached (max ${MAX_TEMPVOICE_CONFIGS_PER_GUILD})`, @@ -99,11 +113,22 @@ export function registerTempVoiceRoutes(app: FastifyInstance): void { return; } - const config = await addGuildConfig(guildId, { - hubChannelId: body.hubChannelId, - categoryId: body.categoryId ?? null, - nameTemplate, - }); + let config; + try { + config = await addGuildConfig(guildId, { + hubChannelId: body.hubChannelId, + categoryId: body.categoryId ?? null, + nameTemplate, + }); + } catch (error) { + if (isUniqueViolation(error)) { + reply + .code(400) + .send({ error: "This channel is already a temp voice hub" }); + return; + } + throw error; + } await notifyCacheInvalidation(guildId, "reloadTempVoice"); reply.code(201).send(config); @@ -149,7 +174,7 @@ export function registerTempVoiceRoutes(app: FastifyInstance): void { reply.code(400).send({ error: "Invalid hub channel" }); return; } - const existingHub = getConfigByHubChannel(body.hubChannelId); + const existingHub = await fetchConfigByHubChannel(body.hubChannelId); if (existingHub && existingHub.id !== Number(configId)) { reply .code(400) @@ -185,7 +210,13 @@ export function registerTempVoiceRoutes(app: FastifyInstance): void { }); await notifyCacheInvalidation(guildId, "reloadTempVoice"); reply.send(updated); - } catch { + } catch (error) { + if (isUniqueViolation(error)) { + reply + .code(400) + .send({ error: "This channel is already a temp voice hub" }); + return; + } reply.code(404).send({ error: "Config not found" }); } }, diff --git a/apps/dashboard/tests/server/features/tempvoice/tempvoice.test.ts b/apps/dashboard/tests/server/features/tempvoice/tempvoice.test.ts index fbc91c91..965c2da0 100644 --- a/apps/dashboard/tests/server/features/tempvoice/tempvoice.test.ts +++ b/apps/dashboard/tests/server/features/tempvoice/tempvoice.test.ts @@ -38,7 +38,7 @@ vi.mock("../../../../src/server/shared/permissions.js", () => ({ createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), })); -const mockGetGuildConfigs = vi.fn().mockReturnValue([]); +const mockFetchGuildConfigs = vi.fn().mockResolvedValue([]); const mockAddGuildConfig = vi.fn().mockResolvedValue({ id: 1, hubChannelId: "ch-1", @@ -52,13 +52,13 @@ const mockUpdateGuildConfig = vi.fn().mockResolvedValue({ nameTemplate: "{user}'s Room", }); const mockRemoveGuildConfig = vi.fn().mockResolvedValue(true); -const mockGetConfigByHubChannel = vi.fn().mockReturnValue(undefined); +const mockFetchConfigByHubChannel = vi.fn().mockResolvedValue(null); vi.mock("@fluxcore/systems/tempVoice/config", () => ({ - getGuildConfigs: (...args: unknown[]) => mockGetGuildConfigs(...args), + fetchGuildConfigs: (...args: unknown[]) => mockFetchGuildConfigs(...args), addGuildConfig: (...args: unknown[]) => mockAddGuildConfig(...args), updateGuildConfig: (...args: unknown[]) => mockUpdateGuildConfig(...args), removeGuildConfig: (...args: unknown[]) => mockRemoveGuildConfig(...args), - getConfigByHubChannel: (...args: unknown[]) => mockGetConfigByHubChannel(...args), + fetchConfigByHubChannel: (...args: unknown[]) => mockFetchConfigByHubChannel(...args), })); vi.mock("@fluxcore/systems/tempVoice/constants", () => ({ @@ -94,8 +94,8 @@ describe("tempvoice routes", () => { vi.clearAllMocks(); mockGetSession.mockResolvedValue(mockSession); mockIsBotInGuild.mockResolvedValue(true); - mockGetGuildConfigs.mockReturnValue([]); - mockGetConfigByHubChannel.mockReturnValue(undefined); + mockFetchGuildConfigs.mockResolvedValue([]); + mockFetchConfigByHubChannel.mockResolvedValue(null); app = await buildApp(); }); @@ -111,7 +111,7 @@ describe("tempvoice routes", () => { }); it("returns array of configs when they exist", async () => { - mockGetGuildConfigs.mockReturnValueOnce([ + mockFetchGuildConfigs.mockResolvedValueOnce([ { id: 1, hubChannelId: "ch-1", nameTemplate: "{user}'s Room", categoryId: null }, { id: 2, hubChannelId: "ch-2", nameTemplate: "{user}'s Gaming", categoryId: "cat-1" }, ]); @@ -168,7 +168,7 @@ describe("tempvoice routes", () => { }); it("returns 400 when hub channel already configured", async () => { - mockGetConfigByHubChannel.mockReturnValueOnce({ + mockFetchConfigByHubChannel.mockResolvedValueOnce({ id: 1, hubChannelId: "ch-1", categoryId: null, @@ -185,7 +185,7 @@ describe("tempvoice routes", () => { }); it("returns 400 when config limit reached", async () => { - mockGetGuildConfigs.mockReturnValueOnce( + mockFetchGuildConfigs.mockResolvedValueOnce( Array.from({ length: 10 }, (_, i) => ({ id: i + 1, hubChannelId: `hub-${i}`, @@ -213,6 +213,34 @@ describe("tempvoice routes", () => { expect(res.statusCode).toBe(400); expect(res.json().error).toContain("too long"); }); + + it("reads the existing hub from the database, not an in-process cache", async () => { + await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/tempvoice", + cookies: { session: app.signCookie("valid") }, + payload: { hubChannelId: "ch-1" }, + }); + expect(mockFetchConfigByHubChannel).toHaveBeenCalledWith("ch-1"); + expect(mockFetchGuildConfigs).toHaveBeenCalledWith("guild-1"); + }); + + it("returns 400, not 500, when the unique constraint rejects the insert", async () => { + const conflict: Error & { code?: string } = new Error( + "Unique constraint failed", + ); + conflict.code = "P2002"; + mockAddGuildConfig.mockRejectedValueOnce(conflict); + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/tempvoice", + cookies: { session: app.signCookie("valid") }, + payload: { hubChannelId: "ch-1" }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toContain("already a temp voice hub"); + expect(mockNotifyCacheInvalidation).not.toHaveBeenCalled(); + }); }); describe("PUT /api/guilds/:guildId/tempvoice/:configId", () => { @@ -234,7 +262,7 @@ describe("tempvoice routes", () => { }); it("returns 400 when changing hub to already-used channel", async () => { - mockGetConfigByHubChannel.mockReturnValueOnce({ + mockFetchConfigByHubChannel.mockResolvedValueOnce({ id: 2, hubChannelId: "ch-other", categoryId: null, @@ -250,6 +278,22 @@ describe("tempvoice routes", () => { expect(res.json().error).toContain("already a temp voice hub"); }); + it("returns 400, not 404, when the unique constraint rejects the update", async () => { + const conflict: Error & { code?: string } = new Error( + "Unique constraint failed", + ); + conflict.code = "P2002"; + mockUpdateGuildConfig.mockRejectedValueOnce(conflict); + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/tempvoice/1", + cookies: { session: app.signCookie("valid") }, + payload: { hubChannelId: "ch-9" }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toContain("already a temp voice hub"); + }); + it("returns 404 when config not found", async () => { mockUpdateGuildConfig.mockRejectedValueOnce(new Error("Not found")); const res = await app.inject({ diff --git a/packages/systems/src/tempVoice/config.ts b/packages/systems/src/tempVoice/config.ts index 88bca9ae..fd65cd6b 100644 --- a/packages/systems/src/tempVoice/config.ts +++ b/packages/systems/src/tempVoice/config.ts @@ -8,6 +8,48 @@ let guildConfigsCache: Map = new Map(); /** hubChannelId → the config that owns that hub channel (for fast event lookups) */ let hubChannelIndex: Map = new Map(); +/** Database row → the shape the rest of the codebase passes around */ +function toConfig(row: { + id: number; + hubChannelId: string; + categoryId: string | null; + nameTemplate: string; +}): TempVoiceGuildConfig { + return { + id: row.id, + hubChannelId: row.hubChannelId, + categoryId: row.categoryId, + nameTemplate: row.nameTemplate, + }; +} + +/** + * Read a guild's configs straight from the database. + * + * The in-memory cache below is only populated by the bot process (ready.ts calls + * loadTempVoiceConfig). Any other process — the dashboard API — must read through + * to Postgres, which is the single source of truth. + */ +export async function fetchGuildConfigs( + guildId: string, +): Promise { + const rows = await getPrisma().tempVoiceGuildConfig.findMany({ + where: { guildId }, + orderBy: { id: "asc" }, + }); + return rows.map(toConfig); +} + +/** Look up the config owning a hub channel, straight from the database. */ +export async function fetchConfigByHubChannel( + hubChannelId: string, +): Promise { + const row = await getPrisma().tempVoiceGuildConfig.findUnique({ + where: { hubChannelId }, + }); + return row ? toConfig(row) : null; +} + export async function loadTempVoiceConfig(): Promise { try { const prisma = getPrisma(); @@ -88,7 +130,7 @@ export async function updateGuildConfig( ): Promise { const prisma = getPrisma(); const row = await prisma.tempVoiceGuildConfig.update({ - where: { id: configId }, + where: { id: configId, guildId }, data: { ...(updates.hubChannelId !== undefined && { hubChannelId: updates.hubChannelId, @@ -122,16 +164,24 @@ export async function removeGuildConfig( guildId: string, configId: number, ): Promise { - const configs = guildConfigsCache.get(guildId); - if (!configs) return false; - const idx = configs.findIndex((c) => c.id === configId); - if (idx === -1) return false; + // Delete against the database, not the cache: only the bot process has a + // populated cache, and the row is what actually has to go. const prisma = getPrisma(); - await prisma.tempVoiceGuildConfig.delete({ where: { id: configId } }); - hubChannelIndex.delete(configs[idx].hubChannelId); - configs.splice(idx, 1); - if (configs.length === 0) { - guildConfigsCache.delete(guildId); + const { count } = await prisma.tempVoiceGuildConfig.deleteMany({ + where: { id: configId, guildId }, + }); + if (count === 0) return false; + + const configs = guildConfigsCache.get(guildId); + if (configs) { + const idx = configs.findIndex((c) => c.id === configId); + if (idx !== -1) { + hubChannelIndex.delete(configs[idx].hubChannelId); + configs.splice(idx, 1); + if (configs.length === 0) { + guildConfigsCache.delete(guildId); + } + } } return true; }