From 9c8c0164ea89ce2fcb606936c1da1c41238b7547 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 20:32:34 -0300 Subject: [PATCH 01/11] fix(docker): exclude worktrees + agent dirs from build context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Build context was including `.worktrees/` (3.4GB), `worktrees/` (707MB), `.wt-specs/` (41MB), `.claude/` (115MB), and `.agents/` (183MB) — totalling ~4.5GB of duplicated trees and AI agent state shipped to the Docker daemon on every build. None of this is needed inside any image. Also exclude `archive/` and `downloads/` (host-only state). --- .dockerignore | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.dockerignore b/.dockerignore index 057bab35b..eeb73a071 100644 --- a/.dockerignore +++ b/.dockerignore @@ -3,6 +3,22 @@ .gitignore .gitattributes +# Worktrees (4.5GB+ of duplicated trees — must be excluded) +.worktrees/ +worktrees/ +.wt-specs/ + +# AI agent state / caches (115MB+ — never needed at build time) +.claude/ +.claude-env/ +.claude-mem/ +.claude-server-commander/ +.agents/ + +# Archived branches + downloaded media (host-only) +archive/ +downloads/ + # Documentation docs/ *.md From e542c24bc6a0945b037adce9396abb16aacec687 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 20:32:49 -0300 Subject: [PATCH 02/11] fix(docker): real bot healthcheck via Redis TCP PING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous HEALTHCHECK was `node -e "console.log('Service is running')"`, which always exits 0 regardless of bot state — orchestrators could never detect a wedged or disconnected bot. New check opens a raw TCP socket to Redis ($REDIS_HOST / $REDIS_PORT) and sends the RESP PING command. Pass = +PONG within 3s; anything else fails. This confirms (1) node can execute inside the container and (2) the bot's critical Redis dependency is reachable from this container. No new deps. Start period bumped 5s → 30s to account for `prisma migrate deploy` running before the bot process starts. --- Dockerfile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 85e035bee..8f3d57df2 100644 --- a/Dockerfile +++ b/Dockerfile @@ -99,8 +99,11 @@ RUN mkdir -p downloads logs && \ USER bot -HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ - CMD node -e "console.log('Service is running')" || exit 1 +# Liveness via Redis TCP PING — confirms node can run AND the bot's +# Redis dependency is reachable. A wedged node process or broken Redis +# link both fail this; the old `console.log` check did neither. +HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=3 \ + CMD node -e "const net=require('net');const s=net.createConnection({host:process.env.REDIS_HOST||'redis',port:+(process.env.REDIS_PORT||6379)},()=>s.write('*1\r\n\$4\r\nPING\r\n'));s.on('data',d=>process.exit(d.toString().startsWith('+PONG')?0:1));s.on('error',()=>process.exit(1));setTimeout(()=>process.exit(1),3000);" || exit 1 CMD ["sh", "-c", "npx prisma migrate deploy --config prisma/prisma.config.ts && node packages/bot/dist/index.js"] From 004a69e297eac0d689110fa28aed23b9efb28d8c Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 20:33:04 -0300 Subject: [PATCH 03/11] fix(docker): add development stage for dev compose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docker-compose.dev.yml` referenced `target: development` but no such stage existed in `Dockerfile` — `docker compose -f docker-compose.dev.yml up --build` would fail with 'failed to find target development'. New `development` stage derives from `base-runtime` (already has ffmpeg / opus / yt-dlp), adds native build tools, and runs `tsx watch` via `npm run dev --workspace=packages/bot`. Compose still bind-mounts host source over `/app`; node_modules installed on first run to populate the anonymous volume. --- Dockerfile | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Dockerfile b/Dockerfile index 8f3d57df2..8f9bf550f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -21,6 +21,19 @@ WORKDIR /app FROM node:${NODE_VERSION} AS base-runtime-backend WORKDIR /app +# Development stage — full deps + native build tools + media binaries. +# Source is bind-mounted by docker-compose.dev.yml (`.:/app`), so this +# image only needs the runtime + global tooling. node_modules is preserved +# inside the container via an anonymous volume. +FROM base-runtime AS development +RUN apk add --no-cache git build-base python3-dev opus-dev && rm -rf /var/cache/apk/* +WORKDIR /app +ENV NODE_ENV=development \ + NPM_CONFIG_LOGLEVEL=warn +# Compose mounts host source over /app; node_modules is installed at first +# run via the entrypoint to populate the anonymous volume. +CMD ["sh", "-c", "npm ci --legacy-peer-deps --no-audit --no-fund && npx prisma generate && npm run dev --workspace=packages/bot"] + # Build stage — installs all deps, generates prisma, builds shared + target FROM node:${NODE_VERSION} AS build From 60a2b42924de3085f62e40473dc493d5c1bbe80a Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 20:33:48 -0300 Subject: [PATCH 04/11] chore(docker): non-root nginx (UID 101) + healthchecks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch `Dockerfile.nginx` and `Dockerfile.frontend` from `nginx:alpine` (runs as root to bind port 80) to `nginxinc/nginx-unprivileged:1.27-alpine` (UID 101, listens on 8080 by default — no NET_BIND_SERVICE capability needed). Changes: - nginx confs (frontend + reverse proxy): `listen 80` → `listen 8080` - nginx reverse-proxy upstream: `http://frontend:80` → `http://frontend:8080` - Dockerfile.frontend + Dockerfile.nginx: pinned image, EXPOSE 8080, added HEALTHCHECK via `wget --spider` (busybox wget ships in the base image). - docker-compose.yml: port mapping `${NGINX_PORT:-8080}:80` → `:8080`. DEPLOY ACTION REQUIRED: update `cloudflared/config-lucky.yml` on the homelab so the tunnel ingress points at `http://nginx:8080` instead of `http://nginx:80` before merging this PR. Host-side `NGINX_PORT` default is unchanged (8080). --- Dockerfile.frontend | 9 ++++++--- Dockerfile.nginx | 10 +++++++--- docker-compose.yml | 5 ++++- nginx/frontend.conf | 2 +- nginx/nginx.conf | 4 ++-- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/Dockerfile.frontend b/Dockerfile.frontend index daf57f592..35073cc0c 100644 --- a/Dockerfile.frontend +++ b/Dockerfile.frontend @@ -24,12 +24,15 @@ RUN npx prisma generate RUN npm run build --workspace=packages/shared RUN npm run build --workspace=packages/frontend -# NOSONAR: nginx requires root to bind port 80 -FROM nginx:alpine +# Runtime: nginx-unprivileged listens on 8080 as UID 101 (no root, no port-80 cap). +FROM nginxinc/nginx-unprivileged:1.27-alpine COPY --from=builder /app/packages/frontend/dist /usr/share/nginx/html COPY nginx/frontend.conf /etc/nginx/conf.d/default.conf -EXPOSE 80 +EXPOSE 8080 + +HEALTHCHECK --interval=30s --timeout=5s --start-period=5s --retries=3 \ + CMD wget -q --spider http://127.0.0.1:8080/ || exit 1 CMD ["nginx", "-g", "daemon off;"] diff --git a/Dockerfile.nginx b/Dockerfile.nginx index db77cf3bf..03c088b54 100644 --- a/Dockerfile.nginx +++ b/Dockerfile.nginx @@ -1,8 +1,12 @@ -# NOSONAR: nginx requires root to bind port 80 -FROM nginx:alpine +# Non-root nginx listening on 8080 (UID 101). Cloudflared / docker port +# publishing maps host 8080 → container 8080. +FROM nginxinc/nginx-unprivileged:1.27-alpine COPY nginx/nginx.conf /etc/nginx/conf.d/default.conf -EXPOSE 80 +EXPOSE 8080 + +HEALTHCHECK --interval=30s --timeout=5s --start-period=5s --retries=3 \ + CMD wget -q --spider http://127.0.0.1:8080/ || exit 1 CMD ["nginx", "-g", "daemon off;"] diff --git a/docker-compose.yml b/docker-compose.yml index 5b1aa62d2..aaa095b85 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -189,7 +189,10 @@ services: - backend - frontend ports: - - "${NGINX_PORT:-8080}:80" + # Container now listens on 8080 (non-root nginx). + # NOTE: update cloudflared config-lucky.yml service entry to + # `http://nginx:8080` after deploying this change. + - "${NGINX_PORT:-8080}:8080" networks: - lucky-network logging: diff --git a/nginx/frontend.conf b/nginx/frontend.conf index 3c25a987b..cfb23c856 100644 --- a/nginx/frontend.conf +++ b/nginx/frontend.conf @@ -1,5 +1,5 @@ server { - listen 80; + listen 8080; server_name _; root /usr/share/nginx/html; index index.html; diff --git a/nginx/nginx.conf b/nginx/nginx.conf index cc8b2de45..cffe7b614 100644 --- a/nginx/nginx.conf +++ b/nginx/nginx.conf @@ -7,7 +7,7 @@ map $http_x_forwarded_proto $proxy_x_forwarded_proto { } server { - listen 80; + listen 8080; server_name _; client_max_body_size 50M; @@ -38,7 +38,7 @@ server { } location / { - set $frontend_upstream http://frontend:80; + set $frontend_upstream http://frontend:8080; proxy_pass $frontend_upstream; proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; From a0bce94d92d178c44a0c161ec6f54602e25dc080 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 20:35:13 -0300 Subject: [PATCH 05/11] chore(docker): add resource limits + env_file fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compose previously had no memory or CPU limits on any service — a runaway bot or backend process could OOM-kill the homelab host. Adds tiered defaults via YAML anchors: small-svc (frontend, nginx, webhook, cloudflared) 128m / 0.25 cpu medium-svc (redis, backend) 512m / 0.5 cpu large-svc (postgres, bot) 1g / 1.0 cpu Bot + backend also get `env_file: .env` so any var missing from the explicit `environment:` block falls back to .env at startup. Explicit entries still take precedence, so behavior is unchanged for vars already listed. Cloudflared `user: root` removed — the official image's nonroot default is sufficient for `tunnel run` with a mounted config dir. `docker-compose config -q` passes. --- docker-compose.yml | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index aaa095b85..3e631caa3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,8 +1,24 @@ +# Shared resource defaults — applied per service via `<<: *small-svc` etc. +x-small-svc: &small-svc + mem_limit: 128m + cpus: 0.25 + restart: unless-stopped + +x-medium-svc: &medium-svc + mem_limit: 512m + cpus: 0.5 + restart: unless-stopped + +x-large-svc: &large-svc + mem_limit: 1g + cpus: 1.0 + restart: unless-stopped + services: postgres: + <<: *large-svc image: postgres:18-alpine container_name: lucky-postgres - restart: unless-stopped environment: POSTGRES_DB: discordbot POSTGRES_USER: discordbot @@ -24,9 +40,9 @@ services: max-file: "3" redis: + <<: *medium-svc image: redis:8-alpine container_name: lucky-redis - restart: unless-stopped command: redis-server --appendonly yes --maxmemory 256mb --maxmemory-policy allkeys-lru volumes: - redis_data:/data @@ -44,6 +60,7 @@ services: max-file: "3" bot: + <<: *large-svc image: ${IMAGE_PREFIX:-ghcr.io/lucassantana-dev/lucky}-bot:${IMAGE_TAG:-latest} build: context: . @@ -53,7 +70,8 @@ services: SERVICE: bot NODE_ENV: production container_name: lucky-bot - restart: unless-stopped + env_file: + - .env depends_on: postgres: condition: service_healthy @@ -108,6 +126,7 @@ services: max-file: "3" backend: + <<: *medium-svc image: ${IMAGE_PREFIX:-ghcr.io/lucassantana-dev/lucky}-backend:${IMAGE_TAG:-latest} build: context: . @@ -117,7 +136,8 @@ services: SERVICE: backend NODE_ENV: production container_name: lucky-backend - restart: unless-stopped + env_file: + - .env depends_on: postgres: condition: service_healthy @@ -164,12 +184,12 @@ services: max-file: "3" frontend: + <<: *small-svc image: ${IMAGE_PREFIX:-ghcr.io/lucassantana-dev/lucky}-frontend:${IMAGE_TAG:-latest} build: context: . dockerfile: Dockerfile.frontend container_name: lucky-frontend - restart: unless-stopped networks: - lucky-network logging: @@ -179,12 +199,12 @@ services: max-file: "3" nginx: + <<: *small-svc image: ${IMAGE_PREFIX:-ghcr.io/lucassantana-dev/lucky}-nginx:${IMAGE_TAG:-latest} build: context: . dockerfile: Dockerfile.nginx container_name: lucky-nginx - restart: unless-stopped depends_on: - backend - frontend @@ -202,11 +222,11 @@ services: max-file: "3" webhook: + <<: *small-svc build: context: ./deploy dockerfile: Dockerfile container_name: lucky-webhook - restart: unless-stopped command: > -hooks /hooks/hooks.json -verbose @@ -231,11 +251,10 @@ services: # Cloudflare Tunnel — exposes nginx to lucky.lucassantana.tech cloudflared: + <<: *small-svc image: cloudflare/cloudflared:latest container_name: lucky-tunnel - restart: unless-stopped command: tunnel --config /etc/cloudflared/config-lucky.yml run - user: root volumes: - ${CLOUDFLARED_CONFIG_DIR:-/home/luk-server/.cloudflared}:/etc/cloudflared:ro depends_on: From b048a8538ebcc87599f7f97f29f66aac5a2398c1 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 20:35:46 -0300 Subject: [PATCH 06/11] chore(docker): pin webhook base to almir/webhook:2.8.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deploy webhook container has `/var/run/docker.sock` bind-mounted in `docker-compose.yml`, which gives anything inside it effective root on the host. Pulling `almir/webhook:latest` (last published 2026-02-12) every rebuild made that surface vulnerable to silent upstream changes. Pin to `2.8.3` (current latest, identical digest as `latest` at time of this change). Also fix the inline-comment placement on `USER root` — it was on the same line which is parsed differently across Docker versions. --- deploy/Dockerfile | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/deploy/Dockerfile b/deploy/Dockerfile index 4161bc28a..75070f125 100644 --- a/deploy/Dockerfile +++ b/deploy/Dockerfile @@ -1,6 +1,10 @@ -FROM almir/webhook:latest +# Pinned to almir/webhook 2.8.3 (released 2026-02-12). Avoid :latest because +# this service has /var/run/docker.sock mounted — any silent base-image change +# is a supply-chain incident waiting to happen. +FROM almir/webhook:2.8.3 -USER root # NOSONAR: privileged operations required for apk and docker socket +# NOSONAR: privileged operations required for apk and docker socket mount +USER root RUN apk add --no-cache git docker-cli docker-cli-compose bash # Copy hooks config to /hooks/ — a path NOT declared as VOLUME by the From 76b54b05d32046eedd4ce08c92de632c3fe36412 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 20:36:25 -0300 Subject: [PATCH 07/11] chore(docker): consolidate stages, venv yt-dlp, align frontend dev to node 22 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small but durable cleanups: 1. Drop the no-op `base-runtime-backend` stage. `production-backend` now derives directly from `node:${NODE_VERSION}` — the intermediate stage only set WORKDIR, which the production stage already does. 2. Replace `pip3 install --break-system-packages` for yt-dlp with a proper PEP-668-compliant venv at `/opt/ytdlp`, symlinked into `/usr/local/bin`. Same behavior, no warning suppression, easier to audit and upgrade. /root/.cache is cleaned in the same layer. 3. `packages/frontend/Dockerfile.dev` was using `node:24-alpine` while production frontend is pinned to `node:22-alpine` (PR #846). Aligned to 22 to avoid silent native-module drift. Also removed `npm cache clean --force` which defeated the BuildKit cache mount. --- Dockerfile | 22 ++++++++++++---------- packages/frontend/Dockerfile.dev | 9 ++++++--- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8f9bf550f..f0a6e32b9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,19 +6,20 @@ ARG NODE_VERSION=22-alpine FROM node:${NODE_VERSION} AS base-runtime +# yt-dlp is installed into a dedicated venv at /opt/ytdlp so we avoid +# `--break-system-packages` (Alpine's PEP 668 marker). The venv binary +# is symlinked into /usr/local/bin so callers don't need to know the path. RUN apk add --no-cache \ python3 \ py3-pip \ ffmpeg \ opus \ opus-tools \ - && rm -rf /var/cache/apk/* + && python3 -m venv /opt/ytdlp \ + && /opt/ytdlp/bin/pip install --no-cache-dir --upgrade pip yt-dlp \ + && ln -s /opt/ytdlp/bin/yt-dlp /usr/local/bin/yt-dlp \ + && rm -rf /var/cache/apk/* /root/.cache -RUN pip3 install --break-system-packages --no-cache-dir yt-dlp - -WORKDIR /app - -FROM node:${NODE_VERSION} AS base-runtime-backend WORKDIR /app # Development stage — full deps + native build tools + media binaries. @@ -120,16 +121,17 @@ HEALTHCHECK --interval=30s --timeout=5s --start-period=30s --retries=3 \ CMD ["sh", "-c", "npx prisma migrate deploy --config prisma/prisma.config.ts && node packages/bot/dist/index.js"] -# Production stage — backend (slim runtime, no media tools) -FROM base-runtime-backend AS production-backend +# Production stage — backend (slim runtime, no media tools). +# Derives directly from node:${NODE_VERSION} instead of a no-op intermediate +# `base-runtime-backend` stage. +FROM node:${NODE_VERSION} AS production-backend +WORKDIR /app ARG COMMIT_SHA ENV NODE_ENV=production \ NPM_CONFIG_LOGLEVEL=silent \ COMMIT_SHA=$COMMIT_SHA -WORKDIR /app - COPY --from=deps-production /app/node_modules ./node_modules COPY --from=deps-production /app/package*.json ./ COPY --from=deps-production /app/packages/shared/package*.json ./packages/shared/ diff --git a/packages/frontend/Dockerfile.dev b/packages/frontend/Dockerfile.dev index eea44934c..f2c5ffa3c 100644 --- a/packages/frontend/Dockerfile.dev +++ b/packages/frontend/Dockerfile.dev @@ -1,13 +1,16 @@ # syntax=docker/dockerfile:1 -FROM node:24-alpine +# Pinned to node:22-alpine to match the production frontend (PR #846 reverted +# from node:24). Drift between dev + prod silently breaks native modules. +FROM node:22-alpine WORKDIR /app COPY package*.json ./ +# Keep BuildKit's cache mount — DO NOT run `npm cache clean --force` here, +# it defeats the mount and slows every rebuild. RUN --mount=type=cache,target=/root/.npm \ - npm ci --no-audit --no-fund && \ - npm cache clean --force + npm ci --no-audit --no-fund USER node From 6cd45331da31ffcc5844d50af8b05d6e79ad5e0c Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 20:37:00 -0300 Subject: [PATCH 08/11] docs(docker): ADR for chore/docker-overhaul Captures the 8-commit Docker surface overhaul: motivation, decisions per commit, consequences (including the cloudflared config-lucky.yml port edit required on the homelab before merge), out-of-scope items, and revisit triggers. --- docs/decisions/2026-05-13-docker-overhaul.md | 91 ++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 docs/decisions/2026-05-13-docker-overhaul.md diff --git a/docs/decisions/2026-05-13-docker-overhaul.md b/docs/decisions/2026-05-13-docker-overhaul.md new file mode 100644 index 000000000..e08d1b308 --- /dev/null +++ b/docs/decisions/2026-05-13-docker-overhaul.md @@ -0,0 +1,91 @@ +# ADR — Docker Surface Overhaul (chore/docker-overhaul) + +- **Status:** Proposed +- **Date:** 2026-05-13 +- **Branch:** `chore/docker-overhaul` +- **Base:** `release/v2.11.0` + +## Context + +The Lucky container surface accumulated friction: + +- `0eb13d0f` (PR #846) reverted the frontend image to `node:22-alpine`, signalling + Docker drift between dev and prod was already biting. +- Build context was shipping the entire repo, including `.worktrees/` (3.4GB), + `worktrees/` (707MB), `.wt-specs/` (41MB), `.claude/` (115MB), and `.agents/` + (183MB) — ~4.5GB of redundant data sent to the daemon on every build. +- `docker-compose.dev.yml` referenced a `target: development` stage that did + not exist in `Dockerfile`, so dev compose was broken. +- The bot `HEALTHCHECK` was `node -e "console.log('Service is running')"` — + always exit 0, completely useless as a liveness signal. +- `Dockerfile.nginx` and `Dockerfile.frontend` ran as root to bind port 80 + with `nginx:alpine`. +- No memory or CPU limits on any compose service. +- `almir/webhook:latest` was unpinned despite the service having + `/var/run/docker.sock` mounted (effective host root). +- `cloudflared` ran as `user: root` unnecessarily. +- A no-op `base-runtime-backend` intermediate stage existed. +- yt-dlp installed via `pip3 install --break-system-packages`. + +## Decision + +Single PR (`chore/docker-overhaul`) ships eight focused commits: + +1. `.dockerignore` excludes worktrees, agent state, archive, downloads. +2. Real bot HEALTHCHECK via raw RESP `PING` over TCP to `${REDIS_HOST}`. +3. `development` stage added to `Dockerfile`, wired by dev compose. +4. nginx + frontend switched to `nginxinc/nginx-unprivileged:1.27-alpine` + (UID 101, port 8080). Compose port mapping + nginx confs adjusted. + Cloudflared config on the homelab must be updated to point at `:8080` + before merge. +5. Resource limits via three YAML anchors (`small-svc` / `medium-svc` / + `large-svc`) applied to every service. `env_file: .env` added as + fallback for bot + backend. Cloudflared `user: root` removed. +6. `almir/webhook` pinned to `2.8.3`. +7. Stage consolidation, venv-based yt-dlp, frontend dev image aligned to + node 22, BuildKit cache mount preserved. +8. This ADR + audit doc. + +## Consequences + +**Positive** + +- Build context shrinks by ~4.5GB → faster local + CI builds and lower + daemon memory pressure. +- Dev compose actually works (`docker compose -f docker-compose.dev.yml up`). +- Orchestrators can detect a wedged bot (Redis-unreachable or node-stuck). +- nginx no longer needs root; minor but durable hardening win. +- No service can OOM the homelab host. +- Supply-chain risk on the webhook container (which holds docker.sock) + drops to "Almir's account stays uncompromised" only. + +**Negative / Required follow-ups** + +- The homelab `cloudflared/config-lucky.yml` ingress entry must be edited + from `http://nginx:80` to `http://nginx:8080` BEFORE merging this PR. +- `env_file: .env` introduces a precedence layer; verified explicit + `environment:` still wins, so behavior is preserved. +- Resource limits are best-guess; revisit if any service hits OOM under + steady-state traffic. + +## Out of scope + +- Migrating from compose to k8s / nomad. +- Switching to a distroless or wolfi base. +- BuildKit `--secret` for build-time credentials (not currently needed). +- Frontend Dockerfile sharing the main multi-stage build (worth doing, + but would couple frontend builds to the bot/backend build cache and + inflate PR scope). + +## Revisit triggers + +- Container OOM events in homelab journalctl. +- Cloudflared / nginx 502s after merge → first check tunnel config port. +- `almir/webhook` reaches end-of-life or upstream publishes a CVE fix + newer than 2.8.3. + +## References + +- PR #846 (`fix(docker): revert frontend image to node:22-alpine`) +- Audit memory: `audit_deep_lucky_2026-05-13` +- TBD policy memory: `feedback_tbd_release_branches` From 334fcb8720a957c5eef153a38e8b7f6bf2abea32 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Wed, 13 May 2026 21:11:37 -0300 Subject: [PATCH 09/11] fix(docker): address CodeRabbit findings on #848 - deploy/Dockerfile: pin almir/webhook:2.8.3 by manifest digest sha256:f77cc91c91d1527b48052280af38b50791e842ad8dd291e9b360a0c13c9ca991. Docker Hub tags are mutable by default; the digest makes the supply-chain story (this container holds docker.sock) actually defensible. - docs/decisions/2026-05-13-docker-overhaul.md: remove dangling reference to a separate audit doc that was never committed; ADR contains audit findings inline. --- deploy/Dockerfile | 8 ++++---- docs/decisions/2026-05-13-docker-overhaul.md | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/deploy/Dockerfile b/deploy/Dockerfile index 75070f125..71739907d 100644 --- a/deploy/Dockerfile +++ b/deploy/Dockerfile @@ -1,7 +1,7 @@ -# Pinned to almir/webhook 2.8.3 (released 2026-02-12). Avoid :latest because -# this service has /var/run/docker.sock mounted — any silent base-image change -# is a supply-chain incident waiting to happen. -FROM almir/webhook:2.8.3 +# Pinned to almir/webhook 2.8.3 by manifest digest (released 2026-02-12). The +# tag alone is mutable on Docker Hub — the digest guarantees immutability since +# this service has /var/run/docker.sock mounted (supply-chain risk). +FROM almir/webhook:2.8.3@sha256:f77cc91c91d1527b48052280af38b50791e842ad8dd291e9b360a0c13c9ca991 # NOSONAR: privileged operations required for apk and docker socket mount USER root diff --git a/docs/decisions/2026-05-13-docker-overhaul.md b/docs/decisions/2026-05-13-docker-overhaul.md index e08d1b308..c6d54913f 100644 --- a/docs/decisions/2026-05-13-docker-overhaul.md +++ b/docs/decisions/2026-05-13-docker-overhaul.md @@ -44,7 +44,7 @@ Single PR (`chore/docker-overhaul`) ships eight focused commits: 6. `almir/webhook` pinned to `2.8.3`. 7. Stage consolidation, venv-based yt-dlp, frontend dev image aligned to node 22, BuildKit cache mount preserved. -8. This ADR + audit doc. +8. This ADR (audit findings inline above; no separate audit doc). ## Consequences From 1562a8977461c1281dd2ed617810942a2d79e3c2 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Thu, 14 May 2026 12:55:36 -0300 Subject: [PATCH 10/11] =?UTF-8?q?feat(reliability):=20yt-dlp=203=C3=97=20e?= =?UTF-8?q?xponential=20backoff=20+=20snapshot=20restore=20timeout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace single-shot yt-dlp execution with 3-attempt retry loop using exponential backoff (30s → 45s → 60s). Each attempt has its own independent timeout; a settled flag prevents double-resolve when the process closes after a timeout kill. - Wrap restoreSnapshot() in Promise.race() with a 2-second deadline so a slow or hung DB call cannot block the entire queue restore path. Logs a warning and falls through to an empty queue on timeout. --- .../src/handlers/player/lifecycleHandlers.ts | 11 +- .../src/utils/music/ytdlpExtractor/service.ts | 106 +++++++++--------- 2 files changed, 60 insertions(+), 57 deletions(-) diff --git a/packages/bot/src/handlers/player/lifecycleHandlers.ts b/packages/bot/src/handlers/player/lifecycleHandlers.ts index e03d1a056..f941920cf 100644 --- a/packages/bot/src/handlers/player/lifecycleHandlers.ts +++ b/packages/bot/src/handlers/player/lifecycleHandlers.ts @@ -49,11 +49,14 @@ export const setupLifecycleHandlers = (player: { if (ENVIRONMENT_CONFIG.MUSIC.SESSION_RESTORE_ENABLED) { const metadata = queue.metadata as QueueMetadata | undefined + const restoreDeadline = new Promise((resolve) => setTimeout(resolve, 2000)) - await musicSessionSnapshotService.restoreSnapshot( - queue, - metadata?.requestedBy ?? undefined, - ) + await Promise.race([ + musicSessionSnapshotService.restoreSnapshot(queue, metadata?.requestedBy ?? undefined), + restoreDeadline.then(() => { + infoLog({ message: `Snapshot restore timed out in ${queue.guild.name}, continuing with empty queue` }) + }), + ]) } musicWatchdogService.arm(queue) diff --git a/packages/bot/src/utils/music/ytdlpExtractor/service.ts b/packages/bot/src/utils/music/ytdlpExtractor/service.ts index 66b4bf1fc..f62e1b580 100644 --- a/packages/bot/src/utils/music/ytdlpExtractor/service.ts +++ b/packages/bot/src/utils/music/ytdlpExtractor/service.ts @@ -95,69 +95,69 @@ export class YtDlpExtractorService extends BaseExtractor { ] } - private setupProcessHandlers( - process: { - stdout?: { - on: (event: string, callback: (data: Buffer) => void) => void - } - stderr?: { - on: (event: string, callback: (data: Buffer) => void) => void - } - on: (event: string, callback: (code: number | null) => void) => void - kill: () => void - }, - resolve: (result: YtDlpExtractorResult) => void, - ): void { - let stdout = '' - let stderr = '' - - process.stdout?.on('data', (data: Buffer) => { - stdout += data.toString() - }) + private spawnYtDlp(query: string, timeoutMs: number): Promise { + return new Promise((resolve) => { + const args = this.buildYtDlpArgs(query) + const proc = spawn(this.options.executablePath, args, { + stdio: ['pipe', 'pipe', 'pipe'], + }) - process.stderr?.on('data', (data: Buffer) => { - stderr += data.toString() - }) + let stdout = '' + let stderr = '' + let settled = false - process.on('close', (code: number | null) => { - if (code === 0) { - const tracks = this.parseOutput(stdout) - resolve({ - success: true, - tracks, - }) - } else { - resolve({ - success: false, - error: stderr || `Process exited with code ${code}`, - }) + const settle = (result: YtDlpExtractorResult): void => { + if (settled) return + settled = true + clearTimeout(timer) + resolve(result) } - }) - process.on('error', (error: unknown) => { - resolve({ - success: false, - error: error instanceof Error ? error.message : 'Unknown error', + const timer = setTimeout(() => { + proc.kill() + settle({ success: false, error: 'yt-dlp timeout' }) + }, timeoutMs) + + proc.stdout?.on('data', (data: Buffer) => { stdout += data.toString() }) + proc.stderr?.on('data', (data: Buffer) => { stderr += data.toString() }) + + proc.on('close', (code: number | null) => { + if (code === 0) { + settle({ success: true, tracks: this.parseOutput(stdout) }) + } else { + settle({ success: false, error: stderr || `Process exited with code ${code}` }) + } }) - }) - setTimeout(() => { - process.kill() - resolve({ - success: false, - error: 'yt-dlp timeout', + proc.on('error', (error: unknown) => { + settle({ + success: false, + error: error instanceof Error ? error.message : 'Unknown error', + }) }) - }, this.options.timeout) + }) } private async executeYtDlp(query: string): Promise { - return new Promise((resolve) => { - const args = this.buildYtDlpArgs(query) - const process = spawn(this.options.executablePath, args, { - stdio: ['pipe', 'pipe', 'pipe'], - }) - this.setupProcessHandlers(process, resolve) - }) + const baseTimeout = this.options.timeout + const timeouts = [baseTimeout, Math.round(baseTimeout * 1.5), baseTimeout * 2] + + for (let attempt = 0; attempt < timeouts.length; attempt++) { + if (attempt > 0) { + debugLog({ message: `yt-dlp retry attempt ${attempt + 1} for query: ${query}` }) + } + + const result = await this.spawnYtDlp(query, timeouts[attempt]) + + if (result.success) return result + + const isLastAttempt = attempt === timeouts.length - 1 + if (isLastAttempt) return result + + debugLog({ message: `yt-dlp attempt ${attempt + 1} failed (${result.error}), retrying…` }) + } + + return { success: false, error: 'yt-dlp exhausted all retries' } } private parseOutput(_output: string): unknown[] { From 1695153b9d0c69dc51c7d3f0c5a11cde80598c3a Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Thu, 14 May 2026 12:55:44 -0300 Subject: [PATCH 11/11] test(ytdlp): retry behavior coverage + ADR for integration testing strategy - Add 4 retry-behavior tests: first-attempt success, retry-on-failure, all-attempts-exhausted (expects throw), timeout-kills-and-retries. Moved discord-player mock to file scope to eliminate jest.resetModules() worker crashes. - Add ADR 2026-05-14: defer in-CI voice integration testing; yt-dlp retry + lifecycle snapshot timeout + backend scaffolding are higher ROI. --- ...14-discord-integration-testing-strategy.md | 119 ++++++++++++++++++ .../tests/utils/music/ytdlpExtractor.test.ts | 90 +++++++++++-- 2 files changed, 200 insertions(+), 9 deletions(-) create mode 100644 docs/decisions/2026-05-14-discord-integration-testing-strategy.md diff --git a/docs/decisions/2026-05-14-discord-integration-testing-strategy.md b/docs/decisions/2026-05-14-discord-integration-testing-strategy.md new file mode 100644 index 000000000..e1a8673f0 --- /dev/null +++ b/docs/decisions/2026-05-14-discord-integration-testing-strategy.md @@ -0,0 +1,119 @@ +# ADR: Discord Bot Integration Testing Strategy + +- **Date**: 2026-05-14 +- **Status**: Accepted +- **Deciders**: Lucas Santana +- **Tags**: testing, discord, voice, integration, ci + +--- + +## Context + +The Lucky Discord bot's test suite is entirely unit-tested with Jest mocks. All Discord.js, discord-player, and yt-dlp interactions are mocked at the boundary. This means voice channel joins, playback lifecycle, autoplay, and yt-dlp extraction are never exercised against real Discord infrastructure in CI. + +The question posed: **what is the right approach to test voice channel features, autoplay, and music features in a real Discord environment?** + +### Stack + +- discord.js v14 +- discord-player v7 + `@discordjs/opus` +- yt-dlp binary via a custom extractor service +- Node 22 / Alpine in production + +### Reliability gaps discovered during research + +1. **yt-dlp no-retry**: `service.ts` fires a single 30 s timeout then hard-kills with no retry. Network blips or CDN hiccups cause permanent playback failures. +2. **Lifecycle snapshot no-timeout**: `lifecycleHandlers.ts` calls `restoreSnapshot()` with no `Promise.race()` guard — a slow/hung DB call blocks the entire queue restore path. +3. **Backend test desert**: `packages/backend` has 9.3 k LOC but only 2 test files (bootstrap + one integration). Route and service logic is entirely uncovered. +4. **No Docker binary validation**: yt-dlp is bundled in the image but is never smoke-tested in CI after build — a broken binary reaches production silently. + +--- + +## Decision + +**DEFER** in-CI voice integration testing against real Discord infrastructure. + +Instead, invest in the four higher-ROI reliability improvements immediately, and treat a **post-deploy staging voice smoke test** (non-blocking) as the correct long-term integration gate if production incident volume justifies the cost. + +--- + +## Alternatives Considered + +### 1. Staging Bot in CI (Discord API calls against a real test guild) + +A dedicated bot token + test guild are provisioned. CI spins up the bot, joins a voice channel, plays a short clip, asserts events. + +**Rejected because:** +- Fundamentally flaky: voice UDP, yt-dlp downloads, Discord API rate limits, and CI runner network variance combine into a test that passes 90 % of the time at best. +- Discord.js maintainers themselves do not run real voice integration tests in CI ([confirmed by research](https://github.com/discordjs/discord.js/discussions)). +- Cost: a secret-bearing bot token in CI is a supply-chain risk; the test guild requires ongoing maintenance. +- The scenario most likely to catch real bugs (connection drop, stream stall) cannot be reliably triggered on demand. +- **Revisit if**: 3+ distinct production voice-join incidents in a quarter that would have been caught by this test. + +### 2. Protocol Replay / Captured Fixture Tests + +Record Discord WebSocket frames + UDP audio packets in a real session; replay them in unit tests against a fake Discord server. + +**Rejected because:** +- Engineering effort estimated at 2–3 sprints, largely infrastructure work with low direct feature value. +- Discord's gateway protocol is not public and changes without notice; fixtures would rot quarterly. +- The discord.js v14 ecosystem has no maintained capture/replay tooling. Building it from scratch is scope creep. +- **Revisit if**: a community library reaches stable release (check `discord-mock`, `discordeno` mock mode quarterly). + +### 3. Alternative Mock Libraries + +`discord.js-mock`, `@skyra/discord-components`, `discordeno` mock adapters. + +**Rejected because:** none maintains discord.js v14 compatibility as of 2026-05-14 (confirmed by npm + GitHub research). The bot's mock boundaries in Jest already achieve the same effect for unit tests. + +### 4. In-Process Audio Pipeline Tests (chosen subset) + +Test the audio processing chain (yt-dlp → FFmpeg → Opus encoder) without Discord connectivity, using local audio files. + +**Partially accepted**: already achievable within the current unit test framework by injecting a `file://` URL into the extractor. This is captured in PR #2 scope (yt-dlp retry logic). + +### 5. Post-Deploy Staging Voice Smoke Test (non-blocking) + +After a successful production deploy, a GitHub Actions `workflow_dispatch` (or post-deploy hook) runs `node scripts/smoke-voice.ts` against a staging guild. The step is marked `continue-on-error: true`. + +**Deferred, not rejected**: this is the correct integration approach if/when incident volume justifies the cost. It is explicitly listed in the plan (PR #3, optional). + +--- + +## The Plan (in priority order) + +| # | Change | Effort | PR target | +|---|--------|--------|-----------| +| 1 | Docker CI step: `yt-dlp --version` smoke inside built image | 0.25 h | after #848 | +| 2 | yt-dlp retry logic (3× exponential backoff: 30 s → 45 s → 60 s) | 2 h | release/v2.12.0 | +| 2 | `restoreSnapshot()` wrapped in `Promise.race(2 s)` with warn + empty-queue fallback | 1 h | same PR | +| 2 | Backend route test scaffolding (≥ 1 test per route group) | 3 h | same PR | +| 3 | Post-deploy staging voice smoke test (optional, non-blocking) | 3 h | separate PR | + +--- + +## Consequences + +### Positive +- Eliminates the two most likely production reliability gaps (yt-dlp blips, stuck restores) without adding CI flakiness. +- Backend test coverage gap begins to close. +- yt-dlp binary breakage is caught in CI before reaching production. +- No new secrets, bots, or Discord guilds to maintain in CI. + +### Negative +- Voice channel join/leave, playback, autoplay, and volume commands remain integration-untested in CI. +- A regression in discord-player or @discordjs/opus would not be caught until post-deploy or user report. + +### Neutral +- The decision is reversible: adding a staging bot integration test later is additive work, not a rewrite. +- Discord-player's own test suite covers the playback engine; we rely on that upstream coverage. + +--- + +## Revisit When + +- **3+ distinct production voice-join incidents** in a rolling 90-day window that a CI integration test would have caught → evaluate Staging Bot option. +- **CI voice flakiness falls below 0.5 %** (unlikely without a major ecosystem shift, but monitor quarterly). +- **A maintained discord.js v14 mock library reaches stable release** → evaluate Protocol Replay option. +- **Discord publishes a stable test guild / bot sandbox API** → re-evaluate entirely. +- **yt-dlp retry + lifecycle timeout changes reduce production incidents to zero for 2 releases** → the ROI argument for voice integration testing weakens further; record and defer indefinitely. diff --git a/packages/bot/tests/utils/music/ytdlpExtractor.test.ts b/packages/bot/tests/utils/music/ytdlpExtractor.test.ts index dcf5994df..44f343091 100644 --- a/packages/bot/tests/utils/music/ytdlpExtractor.test.ts +++ b/packages/bot/tests/utils/music/ytdlpExtractor.test.ts @@ -1,12 +1,18 @@ import { spawn } from 'child_process' import { EventEmitter } from 'events' import { Readable } from 'stream' +import { YtDlpExtractorService } from '../../../src/utils/music/ytdlpExtractor/service' jest.mock('child_process') jest.mock('@lucky/shared/utils', () => ({ errorLog: jest.fn(), debugLog: jest.fn(), })) +jest.mock('discord-player', () => ({ + BaseExtractor: class MockBase { + constructor() {} + }, +})) const mockSpawn = spawn as jest.MockedFunction @@ -31,15 +37,6 @@ describe('YtDlpExtractorService', () => { describe('validate', () => { it('validates YouTube URLs', async () => { - jest.mock('discord-player', () => ({ - BaseExtractor: class MockBase { - constructor() {} - }, - })) - - const { YtDlpExtractorService } = - await import('../../../src/utils/music/ytdlpExtractor/service') - const extractor = new YtDlpExtractorService({} as any, {}) expect( @@ -54,6 +51,81 @@ describe('YtDlpExtractorService', () => { }) }) + describe('retry behavior', () => { + async function flushMicrotasks(n = 4) { + for (let i = 0; i < n; i++) await Promise.resolve() + } + + it('returns tracks on the first successful attempt', async () => { + const proc = createMockProcess() + mockSpawn.mockReturnValue(proc as any) + + const extractor = new YtDlpExtractorService({} as any, { timeout: 30000 }) + const handlePromise = extractor.handle('https://youtube.com/watch?v=test', {} as any) + + proc.emit('close', 0) + + const result = await handlePromise + expect(result.tracks).toEqual([]) + expect(mockSpawn).toHaveBeenCalledTimes(1) + }) + + it('retries after first failure and succeeds on second attempt', async () => { + const proc1 = createMockProcess() + const proc2 = createMockProcess() + mockSpawn.mockReturnValueOnce(proc1 as any).mockReturnValueOnce(proc2 as any) + + const extractor = new YtDlpExtractorService({} as any, { timeout: 30000 }) + const handlePromise = extractor.handle('https://youtube.com/watch?v=test', {} as any) + + proc1.emit('close', 1) + await flushMicrotasks() + + proc2.emit('close', 0) + + const result = await handlePromise + expect(result.tracks).toEqual([]) + expect(mockSpawn).toHaveBeenCalledTimes(2) + }) + + it('throws after all three attempts fail', async () => { + const procs = [createMockProcess(), createMockProcess(), createMockProcess()] + procs.forEach((p) => mockSpawn.mockReturnValueOnce(p as any)) + + const extractor = new YtDlpExtractorService({} as any, { timeout: 30000 }) + const handlePromise = extractor.handle('https://youtube.com/watch?v=test', {} as any) + + procs[0].emit('close', 1) + await flushMicrotasks() + procs[1].emit('close', 1) + await flushMicrotasks() + procs[2].emit('close', 1) + + await expect(handlePromise).rejects.toThrow() + expect(mockSpawn).toHaveBeenCalledTimes(3) + }) + + it('kills the process and retries on timeout', async () => { + const proc1 = createMockProcess() + const proc2 = createMockProcess() + mockSpawn.mockReturnValueOnce(proc1 as any).mockReturnValueOnce(proc2 as any) + + const extractor = new YtDlpExtractorService({} as any, { timeout: 1000 }) + const handlePromise = extractor.handle('https://youtube.com/watch?v=test', {} as any) + + jest.advanceTimersByTime(1000) + await flushMicrotasks() + + expect(proc1.kill).toHaveBeenCalled() + + proc2.emit('close', 0) + + const result = await handlePromise + expect(result.tracks).toEqual([]) + expect(mockSpawn).toHaveBeenCalledTimes(2) + }) + }) + describe('yt-dlp process integration', () => { it('spawns yt-dlp with bestaudio format', () => { const proc = createMockProcess()