diff --git a/decisions/2026-06-24-deploy-frontend-health-gate.md b/decisions/2026-06-24-deploy-frontend-health-gate.md new file mode 100644 index 000000000..1e634137b --- /dev/null +++ b/decisions/2026-06-24-deploy-frontend-health-gate.md @@ -0,0 +1,61 @@ +# Deploy: gate on lucky-frontend + verify the served dashboard SPA + +- Status: accepted +- Date: 2026-06-24 +- Method: /deep-research (deploy root-cause investigation) + +## Context + +The v2.23.0 release deployed and every signal was green — the "Deploy to Homelab" +workflow's `Validate deployed version` (backend SHA via `/api/health/version`), +auth-config + OAuth-redirect smoke checks all passed, and the bot's `/version` +reported v2.23.0 — yet the deploy was observably "not done" (the dashboard surface). + +Root cause: `scripts/deploy.sh` **pulls and starts** `lucky-frontend` +(`docker compose pull/up … frontend …`) but **never verifies it**: + +- `require_running_containers()` listed `lucky-backend lucky-nginx lucky-postgres +lucky-redis lucky-bot` — **`lucky-frontend` was absent**. +- `run_health_checks()` only probed backend endpoints (`/api/health`, + `/api/health/auth-config`) — **nothing checked the dashboard**. + +`lucky-frontend` is a long-running nginx (`Dockerfile` target `production-frontend`: +`CMD ["nginx","-g","daemon off;"]`, `restart: unless-stopped`) that serves the built +SPA from its image; the main `lucky-nginx` reverse-proxies `/` to it. So a frontend +that was down, crashed, or pulled a stale image would leave the dashboard broken +(502 / old assets) while the deploy reported full success — and the deploy verifies +only the bot + backend, neither of which reflects the frontend. + +This is a **recurrence of "Gap E"** — the same "service in the prod compose but +missing from `require_running_containers`" defect that +`decisions/2026-05-24-deploy-lock-contention-signal.md` fixed for `lucky-bot`. + +## Decision + +In `scripts/deploy.sh`: + +1. Add `lucky-frontend` to `require_running_containers()` (safe: it's long-running + with `restart: unless-stopped`, so requiring `State.Running` cannot false-fail a + healthy deploy). +2. Add an HTTP readiness check in `run_health_checks()` that fetches `http://nginx:8080/` + (the dashboard, via the reverse proxy) and matches the React mount point + `id="root"` — confirming the SPA actually renders end-to-end (main nginx → + lucky-frontend → index.html), not just that a container is up. + +Both run before the success signal, so a broken frontend now fails health checks and +triggers the existing auto-rollback path. + +## Consequences + +- A down/crashed/unreachable frontend now **fails the deploy** instead of passing. +- **Not covered:** a frontend that is _running but serving a stale image_ (old build + still has `id="root"`). Detecting staleness needs a build-version marker in the + served HTML/assets compared against the expected release — tracked as follow-up. + +## Prevention rule + +Any new **long-running** service added to the production `docker-compose.yml` MUST be +added to `require_running_containers()` (and HTTP-health-checked if user-facing). To +stop this class recurring a 3rd time, prefer **deriving** the required list from +`docker compose config --services` (minus known one-shots) rather than a hand-maintained +array. Tracked in the follow-up issue. diff --git a/nginx/frontend.conf b/nginx/frontend.conf index cfb23c856..17a1c21fb 100644 --- a/nginx/frontend.conf +++ b/nginx/frontend.conf @@ -4,7 +4,18 @@ server { root /usr/share/nginx/html; index index.html; + # Content-addressed build assets (Vite hashes the filename) are safe to + # cache forever — a new build emits new filenames. + location /assets/ { + add_header Cache-Control "public, max-age=31536000, immutable"; + try_files $uri =404; + } + + # index.html (and the SPA fallback) must NEVER be cached: a stale index.html + # references old asset hashes, so clients keep loading the previous build + # after a deploy. Force revalidation so a deploy is immediately visible. location / { + add_header Cache-Control "no-cache, must-revalidate"; try_files $uri $uri/ /index.html; } } diff --git a/packages/bot/src/functions/music/commands/play/handlers/resolveProvider.spec.ts b/packages/bot/src/functions/music/commands/play/handlers/resolveProvider.spec.ts index eb6985857..0de13defb 100644 --- a/packages/bot/src/functions/music/commands/play/handlers/resolveProvider.spec.ts +++ b/packages/bot/src/functions/music/commands/play/handlers/resolveProvider.spec.ts @@ -89,6 +89,34 @@ describe('resolveQueryWithFallbacks', () => { ) }) + it('should block SpotifyExtractor in YouTube fallback so it cannot intercept text queries', async () => { + const primaryError = new Error( + 'No results found (Spotify extractor)', + ) + const mockTrack = { title: 'Test Song' } + + mockPlayer.play + .mockRejectedValueOnce(primaryError) + .mockResolvedValueOnce(mockTrack) + + await resolveQueryWithFallbacks( + mockPlayer, + mockVoiceChannel, + 'pink floyd wish you were here', + 'spotify', + QueryType.SPOTIFY_SEARCH, + mockPlayOptions, + ) + + const youtubeFallbackCall = mockPlayer.play.mock.calls[1] + expect(youtubeFallbackCall[2]).toMatchObject({ + searchEngine: QueryType.YOUTUBE_SEARCH, + blockExtractors: expect.arrayContaining([ + 'com.discord-player.itsmaat.spotifyextractor', + ]), + }) + }) + it('should fallback to SoundCloud when YouTube also fails', async () => { const primaryError = new Error('Primary failed') const youtubeError = new Error('YouTube failed') @@ -112,6 +140,36 @@ describe('resolveQueryWithFallbacks', () => { expect(telemetry.resolvedVia).toBe('soundcloud-fallback') expect(warnLogMock).toHaveBeenCalledTimes(2) }) + + it('should block SpotifyExtractor in SoundCloud fallback so it cannot intercept text queries', async () => { + const primaryError = new Error( + 'No results found (Spotify extractor)', + ) + const youtubeError = new Error('YouTube failed') + const mockTrack = { title: 'Test Song' } + + mockPlayer.play + .mockRejectedValueOnce(primaryError) + .mockRejectedValueOnce(youtubeError) + .mockResolvedValueOnce(mockTrack) + + await resolveQueryWithFallbacks( + mockPlayer, + mockVoiceChannel, + 'pink floyd wish you were here', + 'spotify', + QueryType.SPOTIFY_SEARCH, + mockPlayOptions, + ) + + const soundcloudFallbackCall = mockPlayer.play.mock.calls[2] + expect(soundcloudFallbackCall[2]).toMatchObject({ + searchEngine: QueryType.SOUNDCLOUD_SEARCH, + blockExtractors: expect.arrayContaining([ + 'com.discord-player.itsmaat.spotifyextractor', + ]), + }) + }) }) describe('failure handling', () => { diff --git a/packages/bot/src/functions/music/commands/play/handlers/resolveProvider.ts b/packages/bot/src/functions/music/commands/play/handlers/resolveProvider.ts index 41645d1bf..e7c6f1f37 100644 --- a/packages/bot/src/functions/music/commands/play/handlers/resolveProvider.ts +++ b/packages/bot/src/functions/music/commands/play/handlers/resolveProvider.ts @@ -8,6 +8,8 @@ import type { VoiceBasedChannel } from 'discord.js' import { warnLog } from '@lucky/shared/utils' import { addBreadcrumb } from '@lucky/shared/utils/monitoring' +const SPOTIFY_EXTRACTOR_ID = 'com.discord-player.itsmaat.spotifyextractor' + export type PlayResolutionArm = | 'primary' | 'youtube-fallback' @@ -62,10 +64,13 @@ export async function resolveQueryWithFallbacks( }) try { - // Attempt YouTube fallback + // Attempt YouTube fallback — block SpotifyExtractor so it + // cannot intercept text queries (its validate() ignores queryType + // and would capture YOUTUBE_SEARCH/SOUNDCLOUD_SEARCH too). const result = await player.play(voiceChannel, query, { ...playOptions, searchEngine: QueryType.YOUTUBE_SEARCH, + blockExtractors: [SPOTIFY_EXTRACTOR_ID], }) telemetry.latencyMs = Date.now() - startTime telemetry.resolvedVia = 'youtube-fallback' @@ -74,14 +79,15 @@ export async function resolveQueryWithFallbacks( warnLog({ message: 'YouTube search failed, falling back to SoundCloud', - data: { query }, + data: { query, error: String(_youtubeError) }, }) try { - // Attempt SoundCloud fallback + // Attempt SoundCloud fallback — same block reason as above const result = await player.play(voiceChannel, query, { ...playOptions, searchEngine: QueryType.SOUNDCLOUD_SEARCH, + blockExtractors: [SPOTIFY_EXTRACTOR_ID], }) telemetry.latencyMs = Date.now() - startTime telemetry.resolvedVia = 'soundcloud-fallback' diff --git a/packages/bot/src/functions/music/commands/play/queryUtils.ts b/packages/bot/src/functions/music/commands/play/queryUtils.ts index aa89ede1e..cf5648d70 100644 --- a/packages/bot/src/functions/music/commands/play/queryUtils.ts +++ b/packages/bot/src/functions/music/commands/play/queryUtils.ts @@ -1,5 +1,7 @@ import { QueryType } from 'discord-player' import type { ChatInputCommandInteraction, GuildMember } from 'discord.js' + +const SPOTIFY_EXTRACTOR_ID = 'com.discord-player.itsmaat.spotifyextractor' import type { CustomClient } from '../../../../types' import { requireVoiceChannel, @@ -177,7 +179,10 @@ export async function executePlayAtTop({ if (!(await requireVoiceChannel(interaction))) return if (!(await requireDJRole(interaction, interaction.guildId))) return - const voiceChannel = assertDefined(member.voice.channel, 'voice channel present after requireVoiceChannel guard') + const voiceChannel = assertDefined( + member.voice.channel, + 'voice channel present after requireVoiceChannel guard', + ) try { await interaction.deferReply() @@ -208,6 +213,7 @@ export async function executePlayAtTop({ try { result = await client.player.play(voiceChannel, query, { searchEngine: QueryType.YOUTUBE_SEARCH, + blockExtractors: [SPOTIFY_EXTRACTOR_ID], }) } catch (youtubeError) { warnLog({ @@ -217,6 +223,7 @@ export async function executePlayAtTop({ }) result = await client.player.play(voiceChannel, query, { searchEngine: QueryType.SOUNDCLOUD_SEARCH, + blockExtractors: [SPOTIFY_EXTRACTOR_ID], }) } } else { diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 125598468..8389cd621 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -108,8 +108,8 @@ notify() { } print_targeted_logs() { - log "Collecting backend/nginx/postgres/redis logs..." - docker_compose logs --tail=80 --no-color backend nginx postgres redis || true + log "Collecting backend/frontend/bot/nginx/postgres/redis logs..." + docker_compose logs --tail=80 --no-color backend frontend bot nginx postgres redis || true } verify_cloudflared_config() { @@ -146,7 +146,7 @@ verify_cloudflared_config() { require_running_containers() { local required - required=(lucky-backend lucky-nginx lucky-postgres lucky-redis lucky-bot) + required=(lucky-backend lucky-frontend lucky-nginx lucky-postgres lucky-redis lucky-bot) local missing=() local not_running=() local container running @@ -394,6 +394,20 @@ run_health_checks() { return 1 fi + # Frontend dashboard: the SPA is served by the long-running lucky-frontend + # container (nginx) and reverse-proxied at "/". Previously the deploy started + # lucky-frontend but never verified it, so a down/broken frontend (the surface + # hosting the dashboard features) passed as a successful deploy. Verify the + # SPA actually renders by matching the React mount point in the served HTML. + if ! wait_for_http_ready \ + "Frontend dashboard" \ + "http://nginx:8080/" \ + 'id="root"'; then + print_targeted_logs + log "HEALTH: frontend dashboard did not serve the SPA (lucky-frontend down or stale image?)" + return 1 + fi + log "Waiting for bot gateway health (start-period: 45s, timeout: 90s)..." local bot_deadline bot_health bot_deadline=$(( SECONDS + 90 ))