diff --git a/.dockerignore b/.dockerignore index c892b07..c09cffd 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,6 +1,10 @@ .git **/__pycache__ **/.venv +**/.pytest_cache +**/.ruff_cache +**/*.log +.env* backend/htmlcov frontend/blob-report frontend/dist @@ -8,3 +12,4 @@ frontend/node_modules frontend/playwright-report frontend/test-results node_modules +packages/react-email/.react-email diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index d896b03..f609307 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -26,7 +26,7 @@ jobs: - name: Set up Bun uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: - bun-version: 1.3.12 + bun-version: 1.4.0 - name: Install frontend dependencies run: bun ci - name: Build frontend diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index ba1e76d..916b820 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -16,6 +16,10 @@ permissions: contents: read pull-requests: read # dorny/paths-filter lists the PR's files +concurrency: + group: playwright-${{ github.workflow }}-${{ github.head_ref || github.ref }} + cancel-in-progress: true + jobs: changes: runs-on: ubuntu-latest @@ -36,6 +40,9 @@ jobs: changed: - backend/** - frontend/** + - pyproject.toml + - uv.lock + - bun.lock - .env - compose*.yml - .github/workflows/playwright.yml @@ -57,7 +64,7 @@ jobs: persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: - bun-version: 1.3.12 + bun-version: 1.4.0 - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version-file: .python-version @@ -75,7 +82,18 @@ jobs: - run: bun ci working-directory: frontend - run: bash scripts/generate-client.sh + - name: Set up Docker Buildx + # The default "docker" buildx driver doesn't support the `gha` cache + # exporter/importer the compose build sections below rely on -- only + # docker-container (and a couple of others) do. + uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0 + with: + driver: docker-container - run: docker compose build + env: + # Routes the build through buildx bake so the backend/playwright + # services' cache_from/cache_to: type=gha fields actually apply. + COMPOSE_BAKE: "1" - run: docker compose down -v --remove-orphans - run: docker compose run --rm backend bash scripts/prestart.sh - name: Run Playwright tests @@ -104,7 +122,7 @@ jobs: persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: - bun-version: 1.3.12 + bun-version: 1.4.0 - name: Install dependencies run: bun ci - name: Download blob reports from GitHub Actions Artifacts diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 5dcd158..f2acf52 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -4,6 +4,10 @@ on: pull_request: permissions: {} +concurrency: + group: pre-commit-${{ github.workflow }}-${{ github.head_ref || github.ref }} + cancel-in-progress: true + env: CAN_PUSH: ${{ github.event.pull_request.head.repo.full_name == github.repository && github.actor != 'dependabot[bot]' }} @@ -40,7 +44,7 @@ jobs: persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: - bun-version: 1.3.12 + bun-version: 1.4.0 - name: Set up Python uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: diff --git a/.github/workflows/test-backend.yml b/.github/workflows/test-backend.yml index eb55dfa..a94dd05 100644 --- a/.github/workflows/test-backend.yml +++ b/.github/workflows/test-backend.yml @@ -7,9 +7,39 @@ on: pull_request: permissions: contents: read + pull-requests: read # dorny/paths-filter lists the PR's files + +concurrency: + group: test-backend-${{ github.workflow }}-${{ github.head_ref || github.ref }} + cancel-in-progress: true jobs: + changes: + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + changed: ${{ steps.filter.outputs.changed }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 2 + persist-credentials: false + - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 + id: filter + with: + filters: | + changed: + - backend/** + - pyproject.toml + - uv.lock + - .env + - compose*.yml + - .github/workflows/test-backend.yml + test-backend: + needs: + - changes + if: ${{ needs.changes.outputs.changed == 'true' }} runs-on: ubuntu-latest timeout-minutes: 5 steps: diff --git a/.github/workflows/test-docker-compose.yml b/.github/workflows/test-docker-compose.yml index 4551a15..50222c9 100644 --- a/.github/workflows/test-docker-compose.yml +++ b/.github/workflows/test-docker-compose.yml @@ -7,10 +7,41 @@ on: pull_request: permissions: contents: read + pull-requests: read # dorny/paths-filter lists the PR's files + +concurrency: + group: test-docker-compose-${{ github.workflow }}-${{ github.head_ref || github.ref }} + cancel-in-progress: true jobs: + changes: + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + changed: ${{ steps.filter.outputs.changed }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 2 + persist-credentials: false + - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 + id: filter + with: + filters: | + changed: + - backend/** + - frontend/** + - pyproject.toml + - uv.lock + - bun.lock + - .env + - compose*.yml + - .github/workflows/test-docker-compose.yml test-docker-compose: + needs: + - changes + if: ${{ needs.changes.outputs.changed == 'true' }} runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -18,7 +49,18 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + - name: Set up Docker Buildx + # The default "docker" buildx driver doesn't support the `gha` cache + # exporter/importer the compose build section below relies on -- + # only docker-container (and a couple of others) do. + uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0 + with: + driver: docker-container - run: docker compose build + env: + # Routes the build through buildx bake so the backend service's + # cache_from/cache_to: type=gha field actually applies. + COMPOSE_BAKE: "1" - run: docker compose down -v --remove-orphans - run: docker compose run --rm backend bash scripts/prestart.sh - run: docker compose up -d --wait backend adminer diff --git a/PROGRESS.md b/PROGRESS.md index d96e3b3..3933978 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -37,7 +37,8 @@ branch; only the phase branch opens a PR into `master`. | 8 | First live mixed game (terminal + API) | done 2026-09-13 (PR #13) | | 9 | Persistence and replay | done 2026-09-13 (PR #14) | | 9.5 | Stabilize, document, rebrand | done 2026-09-13 (PR #15) | -| 9.6 | Upgrade to latest stable versions | done 2026-09-13 (branch phase-9.6-dependency-upgrade, PR pending) | +| 9.6 | Upgrade to latest stable versions | done 2026-09-13 (PR #17) | +| 9.7 | Docker, CI/CD, and server-start optimization | done 2026-09-14 (branch phase-9.7-infra-optimization, PR pending) | | 10 | Frontend: play, spectate, admin, focus view | | | 11 | Chat and personas | | | 12 | Leaderboard | | @@ -507,6 +508,90 @@ detail: `docs/development/dependency-audit.md`. 753 tests green, frontend `bun run build`/`bun run lint` clean, `prek run --all-files` clean. +## Phase 9.7 — done 2026-09-14 (branch phase-9.7-infra-optimization) + +Infrastructure-only pass: Docker image build, Compose, CI/CD workflows, +server startup, and test execution -- no application logic, API/websocket +behavior, or test content changed. Every Section 1 area got a documented +verdict (already fine, or changed with reasoning and, where possible, a +real before/after number). Full detail: `docs/development/ci-cd-and-docker.md`. + +- Docker image: `python:3.14` -> `python:3.14-slim` in `backend/Dockerfile`, + measured **2.13GB -> 707MB (-67%)**; added a non-root `app` user (uid/gid + 1000); worker count now configurable via `WEB_CONCURRENCY` (shell-expansion + default, not a persistent `ENV`, so it can't leak into the dev-only + `fastapi dev` command). The slim swap broke the `curl`-based healthcheck + (no `curl` in `-slim`) -- fixed via a `python3`/`urllib.request` one-liner + in `compose.yml` instead of re-adding `curl`. `.dockerignore` (root, + `backend/`) hardened with `.env*`, `.pytest_cache`, `.ruff_cache`, `*.log`, + `packages/react-email/.react-email/`. +- Compose: `compose.deploy.yml`'s `redis` service was missing + `restart: always` (present on every other service) -- added. Everything + else (named volumes, `condition: service_healthy` healthchecks, Compose + Watch) was already current best practice, confirmed and left alone. +- CI: added `concurrency` groups (cancel stale in-flight runs on a new push) + to `test-backend.yml`, `playwright.yml`, `pre-commit.yml`, + `test-docker-compose.yml` -- `deploy-docker-compose.yml` already had one. + Added `dorny/paths-filter` gating to `test-backend.yml`/ + `test-docker-compose.yml`, mirroring `playwright.yml`'s existing pattern. + Added GHA-backed Docker layer caching (`cache_from`/`cache_to: type=gha` + + `COMPOSE_BAKE=1`) for the `backend`/`playwright` builds, targeting the + 4x-redundant independent build across Playwright's 4 parallel CI shards -- + verified locally that the mechanism builds correctly; the real cross-run + cache-hit benefit needs confirming on the next CI push (no GHA cache + tokens exist locally to measure it honestly). Bumped stale + `bun-version: 1.3.12` -> `1.4.0` (matches local dev) in `playwright.yml` + (x2), `pre-commit.yml`, `deploy.yml`. Confirmed via empirical CI-log + inspection that uv's dependency cache (`setup-uv`'s `auto` default) was + already working correctly -- left alone rather than adding a redundant + `enable-cache: true`. +- Server startup: readiness (`app/main.py`'s lifespan LLM health gate, + `health.py`'s real DB/bus check) and graceful shutdown (sequential + `GameRunner`/`ModelRegistry`/bus/Redis close in the lifespan's `finally`) + were already correct -- confirmed, no changes. Deliberately deferred the + Gunicorn-vs-bare-Uvicorn production question: current guidance supports + either depending on the eventual deploy target, and Phase 15 (real + deployment) hasn't picked one yet. +- Test execution: checked `pytest-xdist` -- `tests/conftest.py`'s `db` + fixture is session-scoped against one real, shared Postgres database with + no per-worker isolation, so parallelizing would risk real cross-worker + collisions for a suite that's already fast (753 tests, ~53s). Not applied. + Checked `prestart.sh`'s lack of a wait-for-DB retry loop -- already + covered by `compose.yml`'s/CI's own `condition: service_healthy` / + `--wait` gating at the orchestration level. Not applied. +- No branch protection exists on this repo (private, free tier -- confirmed + via the GitHub API), so the new path-filter-gated jobs needed no + `alls-green`-style wrapper the way `playwright.yml` already has. +- Found, deliberately not fixed (Phase 14's job, matching the Phase 9.5/9.6 + precedent for `.github/workflows/*` template logic): `deploy.yml`'s + leftover `if: github.repository != 'fastapi/full-stack-fastapi-template'` + template-org check. +- Full gate: `ruff check`/`ruff format --check`/`mypy app`/`ty check app` + clean, backend suite 753 tests green, `prek run --all-files` clean + (including `zizmor` on every workflow edit). No application code touched + this phase, so the suite result is identical to before -- confirming + nothing broke, not that anything was fixed. +- **Review fixes (PR #18):** `code-reviewer` and `python-reviewer` each + caught one real issue before merge. `python-reviewer`: the non-root + user's blanket `RUN chown -R app:app /app` forced overlay2 to copy up + every file from every earlier layer, quietly giving back ~430MB of the + size win (1.14GB with it, re-measured 707MB without) -- fixed by + chowning each layer's own output at creation time (`COPY --chown`, + `chown` appended inside the same `RUN` as `uv sync`) instead of one + blanket pass at the end. `code-reviewer`: the GHA Docker-cache + mechanism as first written would have been silently inert in CI, since + the `gha` cache backend needs buildx's `docker-container` driver and no + workflow provisioned it (confirmed locally: `docker buildx ls` shows + the default driver is plain `docker`) -- fixed by adding + `docker/setup-buildx-action` (`driver: docker-container`) before the + relevant `docker compose build` steps. Also fixed: a path-filter gap + (missing `pyproject.toml`/`uv.lock`/`bun.lock` in all three + `changes`-job filters, which could have let a dependency-only bump + skip its own test job) and an unquoted `${WEB_CONCURRENCY:-4}` + expansion in the Dockerfile's `CMD`. Re-verified after fixes: rebuilt + image, confirmed non-root `app` user can still import `app.main` and + the backend container reports healthy, full gate re-run clean. + ## Phase 2 — done pending PR review - `botpit.games.base`: `Game[State, Move]` contract, frozen pydantic state, diff --git a/backend/.dockerignore b/backend/.dockerignore index c0de4ab..fe95a8b 100644 --- a/backend/.dockerignore +++ b/backend/.dockerignore @@ -3,6 +3,10 @@ __pycache__ app.egg-info *.pyc .mypy_cache +.pytest_cache +.ruff_cache .coverage htmlcov .venv +*.log +.env* diff --git a/backend/Dockerfile b/backend/Dockerfile index 74b7b55..f36a690 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -16,7 +16,7 @@ ARG VITE_API_URL= RUN bun run build -FROM python:3.14 +FROM python:3.14-slim ENV PYTHONUNBUFFERED=1 @@ -32,6 +32,14 @@ ENV UV_COMPILE_BYTECODE=1 # Ref: https://docs.astral.sh/uv/guides/integration/docker/#caching ENV UV_LINK_MODE=copy +# Non-root user, created early so later steps can chown their own output as +# they create it. A blanket `chown -R /app` at the end (the first draft of +# this change) forces overlay2 to copy up every file from every earlier +# layer into a new one -- undoing a chunk of the slim-base size win. Owning +# each layer's output at creation time (COPY --chown, chown within the same +# RUN that populates .venv) avoids that. +RUN groupadd --gid 1000 app && useradd --uid 1000 --gid app --no-create-home app + WORKDIR /app/ # Place executables in the environment at the front of the path @@ -43,23 +51,35 @@ ENV PATH="/app/.venv/bin:$PATH" RUN --mount=type=cache,target=/root/.cache/uv \ --mount=type=bind,source=uv.lock,target=uv.lock \ --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ - uv sync --frozen --no-install-workspace --package botpit-backend + uv sync --frozen --no-install-workspace --package botpit-backend \ + && chown -R app:app /app/.venv -COPY ./backend/scripts /app/backend/scripts +COPY --chown=app:app ./backend/scripts /app/backend/scripts -COPY ./backend/pyproject.toml ./backend/alembic.ini /app/backend/ +COPY --chown=app:app ./backend/pyproject.toml ./backend/alembic.ini /app/backend/ -COPY ./backend/app /app/backend/app +COPY --chown=app:app ./backend/app /app/backend/app -COPY --from=frontend-build /app/backend/app/frontend /app/backend/app/frontend +COPY --chown=app:app --from=frontend-build /app/backend/app/frontend /app/backend/app/frontend # Sync the project # Ref: https://docs.astral.sh/uv/guides/integration/docker/#intermediate-layers RUN --mount=type=cache,target=/root/.cache/uv \ --mount=type=bind,source=uv.lock,target=uv.lock \ --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ - uv sync --frozen --package botpit-backend + uv sync --frozen --package botpit-backend \ + && chown -R app:app /app/.venv WORKDIR /app/backend/ -CMD ["fastapi", "run", "--workers", "4"] +USER app + +# WEB_CONCURRENCY controls the worker count (default: 4, unchanged from +# before) -- overridable per deploy target instead of baked in, since the +# "right" number depends on the host's actual CPU count, which isn't known +# at build time. The default lives in the shell expansion, not a persistent +# ENV: an image-wide ENV would leak into compose.override.yml's `fastapi dev` +# too (dev's own command takes no --workers at all). `exec` replaces the +# shell so `fastapi run` receives SIGTERM directly for a clean graceful +# shutdown (see docs/development/ci-cd-and-docker.md). +CMD ["sh", "-c", "exec fastapi run --workers \"${WEB_CONCURRENCY:-4}\""] diff --git a/compose.deploy.yml b/compose.deploy.yml index 3d810f5..50935d3 100644 --- a/compose.deploy.yml +++ b/compose.deploy.yml @@ -34,6 +34,9 @@ services: db: restart: always + redis: + restart: always + adminer: restart: always labels: diff --git a/compose.override.yml b/compose.override.yml index 3cd0273..e2b0210 100644 --- a/compose.override.yml +++ b/compose.override.yml @@ -90,6 +90,12 @@ services: dockerfile: frontend/Dockerfile.playwright args: - NODE_ENV=production + # GHA-backed layer cache -- see the backend service's build block in + # compose.yml and docs/development/ci-cd-and-docker.md. + cache_from: + - type=gha,scope=playwright + cache_to: + - type=gha,scope=playwright,mode=max ipc: host depends_on: - backend diff --git a/compose.yml b/compose.yml index 5dc401f..50543f1 100644 --- a/compose.yml +++ b/compose.yml @@ -90,7 +90,16 @@ services: - "host.docker.internal:host-gateway" healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:8000/api/v1/health/"] + # python3, not curl: python:3.14-slim doesn't ship curl, and adding it + # back just for a healthcheck would undo part of the point of the slim + # base. urlopen() raises (and this exits non-zero) on anything but 2xx. + test: + [ + "CMD", + "python3", + "-c", + "import urllib.request; urllib.request.urlopen('http://localhost:8000/api/v1/health/')", + ] interval: 10s timeout: 5s retries: 5 @@ -98,6 +107,13 @@ services: build: context: . dockerfile: backend/Dockerfile + # GHA-backed layer cache (needs COMPOSE_BAKE=1 and Docker >=28 to take + # effect -- see docs/development/ci-cd-and-docker.md). Harmless outside + # CI: docker compose build ignores these fields without buildx bake. + cache_from: + - type=gha,scope=backend + cache_to: + - type=gha,scope=backend,mode=max labels: # Enable Traefik for this service - traefik.enable=true diff --git a/docs/development/ci-cd-and-docker.md b/docs/development/ci-cd-and-docker.md new file mode 100644 index 0000000..32a3e19 --- /dev/null +++ b/docs/development/ci-cd-and-docker.md @@ -0,0 +1,293 @@ +# CI/CD and Docker + +Why the Docker build, Compose setup, CI workflows, server startup, and test +execution look the way they do. Written at the end of Phase 9.7 +(infrastructure-only optimization pass, no application/test-content +changes) as the answer to "why does our infrastructure look like this," +for whoever reads it next. + +## Docker image build + +**Already current, left alone:** the multi-stage build (a discarded +`oven/bun:1` stage produces the frontend's static assets; the final stage +is Python-only), the two-phase `uv sync` pattern with BuildKit cache mounts +and bind-mounted `uv.lock`/`pyproject.toml` (deps-only sync before the app +code is copied, so a code-only change doesn't invalidate the dependency +layer; a second full sync after), and dependency-before-code layer +ordering. This is astral-sh's own documented Docker pattern for `uv`, +already implemented correctly. + +**Changed — base image, measured:** `python:3.14` → `python:3.14-slim`. +Measured on this machine: **2.13GB → 707MB (67% smaller)**. `docker +history` attributed the initial difference to `python:3.14`'s apt/Debian-base +layers (compilers, dev headers) that this image never needs at +runtime — every Python dependency here is a prebuilt wheel +(`psycopg[binary]` bundles its own `libpq`). + +This swap broke two things, both fixed: + +- `python:3.14-slim` doesn't ship `curl`, and `compose.yml`'s backend + healthcheck used `curl -f`. Re-adding `curl` via apt would partially + undo the size win, so the healthcheck now runs `python3 -c + "import urllib.request; urllib.request.urlopen(...)"` instead — + `urlopen` raises (non-zero exit) on anything but a 2xx response, the + same semantics as `curl -f`. +- The worker-count fix below (a persistent `ENV`) leaked into the + dev-only `fastapi dev` command; fixed by not using a persistent `ENV` + at all. See "Server startup." + +**Changed — non-root user:** added a `groupadd`/`useradd` (uid/gid 1000, +no home dir) + `USER app`, placed after every root-owned build step +(`uv sync`, `COPY`) but before `CMD`. The first version of this change +used a blanket `RUN chown -R app:app /app` at the very end — a +`python-reviewer` pass on the PR caught that this forces overlay2 to +copy up every file from every earlier layer into a brand-new layer, +which re-measurement confirmed: it was single-handedly responsible for +**~430MB** of the image (1.14GB with it vs. 707MB without). Fixed by +having each layer chown only the files it creates, at creation time +(`COPY --chown=app:app ...`; `chown -R app:app /app/.venv` appended +*inside* the same `RUN` that runs `uv sync`, not a separate later step) +— no cross-layer copy-up, same correct final ownership. Verified +`prestart.sh` (Alembic migrations + initial-data seed) still runs +correctly as this user against the slim base, and that the `app` user +can import `app.main` and reach a healthy `/health/` endpoint. + +**Changed — `.dockerignore` hardening** (root, `backend/`): added +`.env*`, `.pytest_cache`, `.ruff_cache`, `*.log`, and +`packages/react-email/.react-email/` (a build-cache directory discovered +in Phase 9.6, already `.gitignore`d there but not `.dockerignore`d). +`frontend/.dockerignore` was already minimal and correct — no Python +caches apply there, left alone. + +## Docker Compose for local dev and deploy + +**Already current, left alone:** `compose.yml`'s named volumes +(`app-db-data`, `app-redis-data`) for Postgres/Redis persistence across +restarts; healthchecks on `db` (`pg_isready`) and `redis` (`redis-cli +ping`); the backend's `depends_on` using the modern `condition: +service_healthy` form (with `restart: true` on the `db` dependency +specifically). `compose.override.yml`'s Compose Watch (`develop: watch:`) +for live dev sync, dev-only host-port offsets (`POSTGRES_PORT:-5433`, +`REDIS_PORT:-6380`) to avoid colliding with a natively-installed +Postgres/Redis, and the `fastapi dev` reload command. All of this already +matches current (2026) Compose guidance; nothing here needed changing. + +**Changed:** `compose.deploy.yml` gave `restart: always` to `proxy`, +`db`, `adminer`, and `backend`, but `redis` had no override block at all +— meaning a deployed Redis container wouldn't auto-restart after a +crash. Added the same `restart: always` to `redis`. + +**Changed — GHA-backed Docker layer cache:** `compose.yml`'s `backend` +build and `compose.override.yml`'s `playwright` build each get +`cache_from`/`cache_to: type=gha` (scoped per service). Combined with +`COMPOSE_BAKE=1` on the `docker compose build` steps in +`playwright.yml` and `test-docker-compose.yml`, this routes the build +through `docker buildx bake`, which understands the GHA cache backend +(`docker compose build` alone does not). Requires Docker Engine ≥28 +(this machine: 29.8.0; GitHub's `ubuntu-latest` runners ship current +Docker/buildx). This was the single highest-value, not-previously-covered +gap: Playwright's CI job runs 4 parallel shards, each independently +running `docker compose build` for the same backend/playwright images +with zero cache sharing between them — a 4x-redundant build on every +push. + +The `gha` cache exporter/importer only works on buildx's +`docker-container` (or `kubernetes`/`remote`) driver, not the default +`docker` driver every fresh Docker install (and every GitHub-hosted +runner) starts with — a `code-reviewer` pass on the PR caught that this +repo's workflows enabled `COMPOSE_BAKE=1` without ever provisioning that +driver, which would have made the whole mechanism silently inert in CI +(`docker buildx ls` on this machine confirms the default builder is +`docker`, not `docker-container` — the same starting point a fresh +runner has). Fixed by adding a `docker/setup-buildx-action` step +(`driver: docker-container`) before the `docker compose build` steps in +both `playwright.yml` and `test-docker-compose.yml`. Verified locally +with a real `docker-container` builder (`docker buildx create --driver +docker-container --use`) that the compose build still succeeds. The +actual cross-run/cross-shard cache-hit benefit can only be confirmed on +GitHub's own runners — local builds have no +`ACTIONS_CACHE_URL`/`ACTIONS_RUNTIME_TOKEN`, so there's nothing to +measure here honestly; check the next CI run's build step timing (shards +2-4 should be much faster than shard 1) to confirm. + +## CI/CD (GitHub Actions) + +**Already current, left alone — uv dependency caching:** `astral-sh/setup-uv`'s +`enable-cache` defaults to `auto`, which only *disables* caching for +`release`/tag-push/`pull_request_target`/`workflow_run` events (to avoid +cache poisoning from untrusted contexts) — not for the plain +`pull_request`/`push` events every workflow here actually uses. Verified +empirically via real CI run logs (`gh run view --log`), not just the +docs: a lockfile-hash change produces a cache miss, and a subsequent run +with the same hash produces a clean hit (`Cache hit for: +setup-uv-2-x86_64-unknown-linux-gnu-ubuntu-24.04-3.14.7-11de0f...`, 66MB +restored in 1.4s at 87.8 MB/s). Adding an explicit `enable-cache: true` +would have been a no-op at best and a slight security regression at +worst — correctly left alone. (A same-push, same-cache-key race between +two workflows saving under an identical key — "another job may be +creating this cache" — is expected, benign behavior after a lockfile +change invalidates the shared key, not a bug.) + +**Already current, left alone — action versions:** every action is +SHA-pinned with a version comment; Phase 9.6 already brought these +current and Dependabot watches the `github-actions` ecosystem +continuously. + +**Changed — concurrency groups:** `deploy-docker-compose.yml` already +had one (`group: deploy-docker-compose, cancel-in-progress: false`), +proving the pattern was already known in this repo, but `test-backend.yml`, +`playwright.yml`, `pre-commit.yml`, and `test-docker-compose.yml` had +none — a new push didn't cancel a stale in-flight run of the same +workflow on the same branch/PR. Added `concurrency: {group: ..., +cancel-in-progress: true}` to all four, keyed on +`${{ github.head_ref || github.ref }}` so pushes to the same PR/branch +cancel each other but unrelated branches don't collide. + +**Changed — path-filter gating:** `playwright.yml` already had a +`dorny/paths-filter`-based `changes` job (filtering `backend/**`, +`frontend/**`, `.env`, `compose*.yml`, the workflow file itself) so +docs-only or unrelated changes skip the expensive Playwright matrix. +`test-backend.yml` and `test-docker-compose.yml` had no such gate and ran +in full on every push/PR regardless of relevance. Added the same +`changes` job pattern to both (scoped to `backend/**` for +`test-backend.yml`; `backend/**` + `frontend/**` for +`test-docker-compose.yml`, since it also serves the built frontend). + +All three `changes` filters (including the pre-existing one in +`playwright.yml`) were also missing the root-level `pyproject.toml` and +`uv.lock` (and, for the two frontend-touching workflows, `bun.lock`) — +`code-reviewer` caught that a dependency-only bump to those files +(exactly what Dependabot/`uv lock --upgrade` produce, and exactly what +`guard-dependencies.yml` gates for org membership) touches none of +`backend/**`/`frontend/**` and so would have skipped the entire +test/Playwright job, merging an untested dependency change. Added +`pyproject.toml`/`uv.lock`/`bun.lock` to all three filters. + +No `alls-green`-style wrapper job was needed for these two, unlike +`playwright.yml`'s `alls-green-playwright`: this repo has **no branch +protection configured** (confirmed via `gh api +repos/withrvr/botpit/branches/master/protection` → 403, a private +repo on the free tier can't enable it), so no required status check +depends on a specific job reporting non-skipped. If branch protection is +ever turned on, revisit whether `test-backend`/`test-docker-compose` +need the same wrapper `playwright.yml` already uses. + +**Changed — `bun-version` consistency:** bumped the pinned +`bun-version: 1.3.12` → `1.4.0` in `playwright.yml` (both occurrences), +`pre-commit.yml`, and `deploy.yml`, matching the actual local dev Bun +version (`bun --version` → 1.4.0) confirmed throughout Phase 9.6. No +other change to `deploy.yml` — its deploy logic and the leftover +template-org `if: github.repository != 'fastapi/full-stack-fastapi-template'` +check are out of scope for this phase (see below). + +**Found, deliberately not fixed — template-org leftovers:** +`.github/workflows/zizmor.yml`'s `advanced-security: ${{ +github.repository_owner == 'fastapi' }}` (found in Phase 9.6) and +`deploy.yml`'s `if: github.repository != 'fastapi/full-stack-fastapi-template'` +(found in this phase) are both harmless no-ops for `withrvr/botpit` +(never equal to `fastapi/...`), but genuine template cruft. Both are +in scope for Phase 14's eventual template-workflow adaptation, not this +infra-optimization phase, per the Phase 9.5/9.6 precedent of leaving +`.github/workflows/{prepare-release,issue-manager,bump-pre-commit-hooks, +deploy-docker-compose,deploy,zizmor}.yml`'s actual logic untouched while +still applying narrow, safe, version-consistency fixes (like the +bun-version bump) to those same files. + +## Server startup + +**Already current, left alone:** `backend/app/main.py`'s lifespan +already blocks startup on a real LLM health check +(`require_healthy_or_raise`) before serving, and on shutdown sequentially +closes the `GameRunner` (cancelling every running game and awaiting its +completion), the LLM `ModelRegistry`, the event bus, and Redis — thorough +readiness and graceful shutdown, no changes needed. +`backend/app/api/routes/health.py`'s `GET /health/` performs a real +Postgres (`SELECT 1`) and event-bus (`bus.ping()`) check, returning 503 +on failure, and `compose.yml`'s backend healthcheck already targets +exactly this endpoint — only the healthcheck's own *command* needed +fixing for the slim base (see "Docker image build"), not this +already-correct readiness logic. + +**Changed — configurable worker count:** the `CMD` was +`["fastapi", "run", "--workers", "4"]`, a hardcoded worker count. Changed +to `["sh", "-c", "exec fastapi run --workers ${WEB_CONCURRENCY:-4}"]` — +overridable per deploy target via the `WEB_CONCURRENCY` env var (the +standard name from the tiangolo/uvicorn-gunicorn-docker image and +Heroku/Gunicorn ecosystem), still defaulting to 4 when unset. Deliberately +**not** a persistent `ENV WEB_CONCURRENCY=4` instruction: that would leak +into `compose.override.yml`'s separate `fastapi dev --host 0.0.0.0` dev +command, which takes no `--workers` flag at all and would then print a +spurious "workers flag is ignored when reloading is enabled" warning. The +default lives only in the shell parameter expansion inside the `CMD` +string, so it only applies when that exact command runs. `exec` replaces +the shell process so `fastapi run` receives `SIGTERM` directly from +Docker for a clean graceful shutdown — the standard Docker signal-handling +pattern. + +**Deliberately deferred — Gunicorn vs. bare Uvicorn workers:** current +(2026) FastAPI/Uvicorn guidance still documents Gunicorn-fronting-Uvicorn +as the "default" production setup (raw Uvicorn "doesn't handle process +signals or restarts gracefully on its own"), while acknowledging +multi-replica bare Uvicorn (`fastapi run --workers N` per pod) as "a +legitimate and increasingly common pattern in 2026" for +Kubernetes-style orchestration specifically. This project has no real +production deploy target yet (Phase 15, explicitly deferred per +`DECISIONS.md`/`PROGRESS.md`), so which pattern actually fits is +genuinely undetermined — switching process-management architecture now +would be guessing. Fixed only the concrete, actionable gap (hardcoded → +configurable worker count) and left the Gunicorn-vs-bare-Uvicorn choice +as a deliberate, reasoned deferral to whenever Phase 15 picks a real +deploy target. + +## Test environment execution + +**Already current, left alone — CI dependency/Docker caching:** covered +above (uv cache via `setup-uv`'s `auto` default; Docker layer cache now +added via GHA backend). + +**Checked, not changed — wait-for-DB retry logic:** `backend/scripts/prestart.sh` +has no `tenacity`-style retry loop (unlike the original template's +`backend_pre_start.py`), but this is already adequately handled at the +orchestration level: `compose.yml`'s `depends_on: {condition: +service_healthy}` and CI's own `docker compose up -d --wait db mailpit +redis` both already block until Postgres/Redis report healthy before +`prestart.sh` (or the app) ever runs. Adding an application-level retry +loop on top would be redundant, not a gap. + +**Checked, not changed — `pytest-xdist` parallelization:** +`backend/tests/conftest.py`'s `db` fixture is `scope="session"` — one +shared `Session` against the real Postgres database for the entire test +run, cleaned up via explicit per-model `DELETE` statements only at final +teardown, not per-test transactional rollback. Running `pytest-xdist` +workers would mean multiple OS processes concurrently reading/writing the +same live tables through that one shared-fixture pattern, with no +per-worker database or schema namespacing anywhere in the fixture setup +— a real collision risk (one worker's test could observe or clobber +another's in-progress state), not a hypothetical one. Making this safe +would need a real fixture rework (per-worker database/schema, keyed off +`PYTEST_XDIST_WORKER`) that this infra-optimization phase's scope +boundary (Section 0: no test-content changes) doesn't cover, and the +suite is already fast as-is: 753 tests in ~53s. Verdict: checked, +marginal benefit given the real isolation risk — not applied. + +## Summary of measured changes + +| Change | Before | After | +|---|---|---| +| Backend image size | 2.13GB | 707MB (-67%) | +| uv dependency cache (CI) | already working (`auto` default) | unchanged, verified | +| Docker layer cache (CI) | none (4x redundant build per Playwright run) | `type=gha` + `docker-container` buildx driver, mechanism verified locally; cross-run hit rate to confirm on next CI push | + +## Review fixes (PR #18) + +Two review passes (`code-reviewer`, `python-reviewer`) on the PR caught +two real issues before merge, both described in-place above rather than +repeated here: the non-root user's blanket `chown -R` was quietly giving +back ~430MB of the size win (fixed by chowning per-layer instead), and +the GHA Docker-cache mechanism as first written would have been silently +inert in CI because no workflow provisioned the `docker-container` buildx +driver the `gha` cache backend requires (fixed by adding +`docker/setup-buildx-action`). Also fixed: a path-filter gap (missing +`pyproject.toml`/`uv.lock`/`bun.lock`) that could have let a +dependency-only change skip its own test job, and an unquoted +`${WEB_CONCURRENCY:-4}` expansion in the Dockerfile's `CMD`.