Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions docs/decisions/2026-05-19-retire-per-guild-feature-toggles.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
---
status: accepted
date: 2026-05-19
supersedes: per-guild feature toggle scope from PR #614
---

# Per-guild feature toggles are retired; only global toggles remain

PR #614 (2026-04-14) introduced per-guild feature toggles backed by the `GuildFeatureToggle` Prisma model and exposed via `setGuildFeatureToggle` / `isEnabledForGuild` on `FeatureToggleService`. PR #801 (2026-05-04, "feat(admin): add admin panel with writable global feature toggles") replaced that path with global toggles: a new admin panel writes to `GlobalFeatureToggle`, and the backend's `/api/toggles/guild/...` routes were removed. The commit body of #801 explicitly says: _"Remove per-guild toggle routes."_

The removal in #801 was deliberate, but it left orphans: the `GuildFeatureToggle` Prisma model + `guild_feature_toggles` DB table remained, and two test files (`packages/shared/src/services/FeatureToggleService.spec.ts` + `packages/backend/tests/integration/routes/toggles.test.ts`) still referenced the deleted methods. The shared test suite has been red since #801 because of the resulting TS2551 compile errors; the failure was hidden until `/adt-ship-check` surfaced it on 2026-05-19.

This ADR ratifies the removal retroactively and ships the cleanup.

## Decision

Per-guild feature toggles are **not** part of Lucky's surface area going forward. Feature toggles are **global**, sourced in order from:

1. `GlobalFeatureToggle` DB row (set by the admin panel, writes via `setGlobalFeatureToggle`)
2. Vercel Flags (`@vercel/flags-core` when `FLAGS` env var is configured)
3. Fallback defaults from `getFeatureToggleConfig()` in `@lucky/shared/config`

Cleanup performed in this commit:

- `packages/shared/src/services/FeatureToggleService.spec.ts` rewritten to test only the surviving global path; dead `describe` blocks (`getDbOverride (via isEnabledForGuild)`, `setGuildFeatureToggle`, `isEnabledForGuild with DB override`) removed.
- `packages/backend/tests/integration/routes/toggles.test.ts` mock object stripped of `mockSetGuildFeatureToggle`, `isEnabledForGuild`, `setGuildFeatureToggle` entries.
- `prisma/schema.prisma`: `model GuildFeatureToggle` removed; `Guild.featureToggles` relation deleted.
- `prisma/migrations/20260519000000_retire_per_guild_feature_toggles/migration.sql`: `DROP TABLE IF EXISTS "guild_feature_toggles";`.

## Considered options

- **A — Retire per-guild toggles (accepted).** Codifies the path #801 already took; closes the test + schema gap.
- **B — Restore per-guild toggles.** Rejected: no product driver. The admin panel introduced in #801 deliberately moved to a global-only model, with the rationale that operator-facing kill switches need cluster-wide reach and per-guild scoping was unused. Restoring would re-introduce a feature with no current consumer.
- **C — Leave the orphan in place; only fix the tests.** Rejected: the schema and DB table would continue to exist as dead state, future schema audits would re-surface this as a finding, and someone would eventually reintroduce a half-baked per-guild path believing the substrate was load-bearing.

## Consequences

**Positive:**

- Shared test suite drops from 2 failing suites to 1 (only `__tests__/utils/spotify/artistApi.test.ts` remains red and is tracked separately).
- DB schema matches application reality. Future schema introspection won't suggest a dead model.
- `CONTEXT.md` is consistent: feature toggles are global; `GuildFeatureToggle` no longer appears in the Prisma model list.

**Negative:**

- Retiring the table is destructive. The migration drops `guild_feature_toggles` with `IF EXISTS`; rows in production environments (if any) are lost. Per the deployment chain summary in [[reference-lucky-deploy-chain-architecture-2026-05-15]], the homelab Postgres is the only production DB; spot-check confirms no consumers write to this table, so data loss risk is bounded to whatever rows #614 wrote before #801 stopped writing them.
- If a future product driver wants per-guild toggle scoping, it will be a fresh design rather than a revival of this substrate — schema, service path, route, and UI would need to be rebuilt.

## Revisit when

- A product requirement arrives for per-guild feature flag scoping (e.g. per-Guild paid-tier gating, beta-cohort selection). Reopening this ADR would propose a new design rather than restoring #614's substrate.
- The Vercel Flags integration is replaced (e.g. moved to LaunchDarkly, OpenFeature, or a fully self-hosted toggle service). The "global" path is the only one that survives, so its replacement is a load-bearing decision.

## Cross-references

- PR #614 (2026-04-14) — introduced per-guild toggles + the `GuildFeatureToggle` model.
- PR #762 (2026-04-25 era) — adopted `@vercel/flags-core`.
- PR #801 (2026-05-04) — admin panel, removed per-guild routes, retained per-guild method/spec/schema by oversight.
- Diagnostic session 2026-05-19 (this session) — surfaced the orphan via `/adt-ship-check` + `/diagnose`.
- `CONTEXT.md` — the `GuildFeatureToggle` Prisma model entry should not be added (was never in glossary).
7 changes: 2 additions & 5 deletions packages/backend/tests/integration/routes/toggles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,16 @@ jest.mock('../../../src/utils/developerAccess', () => ({
isDeveloperUser: (userId?: string) => userId === '123456789',
}))

const mockSetGuildFeatureToggle = jest.fn<any>().mockResolvedValue(undefined)
const mockSetGlobalFeatureToggle = jest.fn<any>().mockResolvedValue(undefined)

// Per-guild toggles retired in PR #801. See
// docs/decisions/2026-05-19-retire-per-guild-feature-toggles.md.
jest.mock('@lucky/shared/services', () => ({
featureToggleService: {
getAllToggles: jest.fn(),
getGlobalToggleProvider: jest.fn(),
getGlobalToggleStatus: jest.fn(),
isEnabledGlobal: jest.fn(),
isEnabledForGuild: jest.fn(),
setGuildFeatureToggle: (...args: any[]) =>
mockSetGuildFeatureToggle(...args),
setGlobalFeatureToggle: (...args: any[]) =>
mockSetGlobalFeatureToggle(...args),
},
Expand Down Expand Up @@ -337,5 +335,4 @@ describe('Toggles Routes Integration', () => {
})
})
})

})
168 changes: 12 additions & 156 deletions packages/shared/src/services/FeatureToggleService.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { describe, it, expect, jest, beforeEach, afterAll } from '@jest/globals'

const mockFindUnique = jest.fn<any>()
const mockUpsert = jest.fn<any>()
// Per-guild feature toggles were retired in PR #801 (admin panel for global
// toggles). See docs/decisions/2026-05-19-retire-per-guild-feature-toggles.md.
// This spec only covers the surviving global Vercel-flag + DB-override path.

const mockGlobalFindUnique = jest.fn<any>()
const mockGlobalUpsert = jest.fn<any>()
const mockPrismaClient = {
guildFeatureToggle: {
findUnique: (...args: unknown[]) => mockFindUnique(...args),
upsert: (...args: unknown[]) => mockUpsert(...args),
globalFeatureToggle: {
findUnique: (...args: unknown[]) => mockGlobalFindUnique(...args),
upsert: (...args: unknown[]) => mockGlobalUpsert(...args),
},
}
const mockEvaluate = jest.fn<any>()
Expand Down Expand Up @@ -53,10 +57,11 @@ describe('FeatureToggleService', () => {
beforeEach(async () => {
jest.resetModules()
delete process.env.FLAGS
mockFindUnique.mockReset()
mockUpsert.mockReset()
mockGlobalFindUnique.mockReset()
mockGlobalUpsert.mockReset()
mockEvaluate.mockReset()
mockCreateClient.mockClear()
mockGlobalFindUnique.mockResolvedValue(null)

const mod = await import('./FeatureToggleService')
service = mod.featureToggleService as unknown as typeof service
Expand All @@ -70,111 +75,6 @@ describe('FeatureToggleService', () => {
}
})

describe('getDbOverride (via isEnabledForGuild)', () => {
it('returns db value when row exists (enabled=true)', async () => {
mockFindUnique.mockResolvedValue({ enabled: true })
const result = await (
service as unknown as {
getDbOverride(
guildId: string,
name: string,
): Promise<boolean | null>
}
).getDbOverride('guild-1', 'DOWNLOAD_AUDIO')
expect(result).toBe(true)
})

it('returns db value when row exists (enabled=false)', async () => {
mockFindUnique.mockResolvedValue({ enabled: false })
const result = await (
service as unknown as {
getDbOverride(
guildId: string,
name: string,
): Promise<boolean | null>
}
).getDbOverride('guild-1', 'DOWNLOAD_AUDIO')
expect(result).toBe(false)
})

it('returns null when no row found', async () => {
mockFindUnique.mockResolvedValue(null)
const result = await (
service as unknown as {
getDbOverride(
guildId: string,
name: string,
): Promise<boolean | null>
}
).getDbOverride('guild-1', 'DOWNLOAD_AUDIO')
expect(result).toBeNull()
})

it('returns null when db throws', async () => {
mockFindUnique.mockRejectedValue(new Error('DB connection failed'))
const result = await (
service as unknown as {
getDbOverride(
guildId: string,
name: string,
): Promise<boolean | null>
}
).getDbOverride('guild-1', 'DOWNLOAD_AUDIO')
expect(result).toBeNull()
})
})

describe('setGuildFeatureToggle', () => {
it('upserts the toggle with correct args', async () => {
mockUpsert.mockResolvedValue({})
await service.setGuildFeatureToggle(
'guild-1',
'DOWNLOAD_VIDEO',
true,
)
expect(mockUpsert).toHaveBeenCalledWith({
where: {
guildId_name: {
guildId: 'guild-1',
name: 'DOWNLOAD_VIDEO',
},
},
update: { enabled: true },
create: {
guildId: 'guild-1',
name: 'DOWNLOAD_VIDEO',
enabled: true,
},
})
})

it('upserts with enabled=false', async () => {
mockUpsert.mockResolvedValue({})
await service.setGuildFeatureToggle(
'guild-1',
'DOWNLOAD_AUDIO',
false,
)
expect(mockUpsert).toHaveBeenCalledWith(
expect.objectContaining({
update: { enabled: false },
create: expect.objectContaining({ enabled: false }),
}),
)
})

it('propagates db errors', async () => {
mockUpsert.mockRejectedValue(new Error('DB write failed'))
await expect(
service.setGuildFeatureToggle(
'guild-1',
'DOWNLOAD_VIDEO',
true,
),
).rejects.toThrow('DB write failed')
})
})

describe('global Vercel flags', () => {
it('uses fallback toggles when FLAGS is not configured', async () => {
const enabled = await service.isEnabledGlobal('DOWNLOAD_VIDEO')
Expand Down Expand Up @@ -272,48 +172,4 @@ describe('FeatureToggleService', () => {
)
})
})

describe('isEnabledForGuild with DB override', () => {
it('returns db override (true) without checking Vercel', async () => {
mockFindUnique.mockResolvedValue({ enabled: true })
const result = await service.isEnabledForGuild(
'DOWNLOAD_AUDIO',
'guild-1',
)
expect(result).toBe(true)
})

it('returns db override (false) without checking Vercel', async () => {
process.env.FLAGS = 'vf_test'
mockEvaluate.mockResolvedValue({
value: true,
reason: 'fallthrough',
})
mockFindUnique.mockResolvedValue({ enabled: false })
const result = await service.isEnabledForGuild(
'DOWNLOAD_VIDEO',
'guild-1',
)
expect(result).toBe(false)
expect(mockEvaluate).not.toHaveBeenCalled()
})

it('falls back to fallback toggle when no db override', async () => {
mockFindUnique.mockResolvedValue(null)
const result = await service.isEnabledForGuild(
'DOWNLOAD_VIDEO',
'guild-1',
)
expect(result).toBe(true)
})

it('falls back to fallback toggle when db throws', async () => {
mockFindUnique.mockRejectedValue(new Error('DB error'))
const result = await service.isEnabledForGuild(
'DOWNLOAD_VIDEO',
'guild-1',
)
expect(result).toBe(true)
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-- Retire per-guild feature toggles.
--
-- The application path for per-guild feature toggles was removed in PR #801
-- (admin panel for writable global toggles). This migration cleans up the
-- orphan table left behind.
--
-- See docs/decisions/2026-05-19-retire-per-guild-feature-toggles.md.

DROP TABLE IF EXISTS "guild_feature_toggles";
14 changes: 2 additions & 12 deletions prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ model Guild {
trackHistory TrackHistory[]
commandUsage CommandUsage[]
twitchNotifications TwitchNotification[]
featureToggles GuildFeatureToggle[]

@@map("guilds")
}
Expand All @@ -92,17 +91,8 @@ model GuildMembershipEvent {
@@map("guild_membership_events")
}

model GuildFeatureToggle {
id String @id @default(cuid())
guildId String
guild Guild @relation(fields: [guildId], references: [id], onDelete: Cascade)
name String
enabled Boolean

@@unique([guildId, name])
@@index([guildId])
@@map("guild_feature_toggles")
}
// GuildFeatureToggle was retired in PR #801 (admin panel for global toggles).
// See docs/decisions/2026-05-19-retire-per-guild-feature-toggles.md for context.

model GlobalFeatureToggle {
id String @id @default(cuid())
Expand Down
Loading