From b9398fccd7d0680daa18fd2cffdc8228d18ebd5c Mon Sep 17 00:00:00 2001 From: withrvr Date: Mon, 14 Sep 2026 15:48:45 +0000 Subject: [PATCH] fix(ci): stop deploy.yml failing on every push, dry-run the deploy mechanics (Phase 14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deploy.yml (FastAPI Cloud) has failed on every single push to master since this repo's first one -- invisible to gh pr checks because a push-triggered workflow never appears there. Root cause: `bun run --filter frontend build`, but the workspace package has been `botpit-frontend` since the Phase 9.5 rebrand. Fixed the bug and changed the trigger to workflow_dispatch (no FASTAPI_CLOUD_TOKEN exists yet -- DECISIONS.md §1, no server), matching deploy-docker-compose.yml's own already-correct pattern. Dry-ran the compose-based deploy mechanics for real: config+build clean, and the prepare-database step against a fresh throwaway Postgres under FASTAPI_ENV=production with real secrets -- full migration chain applies cleanly, superuser seeds correctly, Settings correctly refuses dev-placeholder secrets outside development. Also adapted two template-org leftovers explicitly deferred to this phase in Phase 9.7's own write-up (deploy*.yml's/zizmor.yml's always-constant github.repository[_owner] conditionals), and fixed a stale DECISIONS.md cross-reference to a deployment doc that never existed at that path. Claude-Session: https://claude.ai/code/session_01UbiPa1rKCyqhqufgwJWkeW --- .github/workflows/deploy-docker-compose.yml | 2 - .github/workflows/deploy.yml | 18 ++--- .github/workflows/zizmor.yml | 9 ++- DECISIONS.md | 2 +- PROGRESS.md | 78 ++++++++++++++++++++- docs/deployment/production.md | 7 +- docs/deployment/staging.md | 48 ++++++++++++- 7 files changed, 145 insertions(+), 19 deletions(-) diff --git a/.github/workflows/deploy-docker-compose.yml b/.github/workflows/deploy-docker-compose.yml index 4062687..feeac9f 100644 --- a/.github/workflows/deploy-docker-compose.yml +++ b/.github/workflows/deploy-docker-compose.yml @@ -12,8 +12,6 @@ concurrency: jobs: deploy: - # Do not deploy the template repository, only projects created from it - if: github.repository != 'fastapi/full-stack-fastapi-template' runs-on: self-hosted timeout-minutes: 30 env: diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index f609307..6c4c296 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -1,11 +1,15 @@ name: Deploy +# Manual-dispatch only, not `push: branches: [master]` -- DECISIONS.md §1: +# "no staging or production server is bought or hosted until local +# development... is fully complete... deploy workflows are written and +# dry-run locally." No FastAPI Cloud account/secrets exist yet (Phase 15, +# not done), so triggering this on every push would just fail every time +# at the final "Deploy to FastAPI Cloud" step -- which it did, on every +# single push to master since this repo's very first one, invisible to PR +# checks because a `push`-triggered workflow never shows up in +# `gh pr checks` (found and fixed during Phase 14). on: - push: - branches: - - master - paths-ignore: - - release-notes.md workflow_dispatch: permissions: @@ -13,8 +17,6 @@ permissions: jobs: deploy: - # Do not deploy the template repository, only projects created from it - if: github.repository != 'fastapi/full-stack-fastapi-template' runs-on: ubuntu-latest timeout-minutes: 15 steps: @@ -30,7 +32,7 @@ jobs: - name: Install frontend dependencies run: bun ci - name: Build frontend - run: bun run --filter frontend build + run: bun run --filter botpit-frontend build env: # Override frontend/.env so the deployed frontend uses the same origin as the API VITE_API_URL: "" diff --git a/.github/workflows/zizmor.yml b/.github/workflows/zizmor.yml index 5c1fb25..0146963 100644 --- a/.github/workflows/zizmor.yml +++ b/.github/workflows/zizmor.yml @@ -26,5 +26,10 @@ jobs: - name: Run zizmor uses: zizmorcore/zizmor-action@70fb788f84895a7701f5643d103d587e460b5c99 # v0.6.3 with: - advanced-security: ${{ github.repository_owner == 'fastapi' }} - annotations: ${{ github.repository_owner != 'fastapi' }} + # `github.repository_owner` is always `withrvr`, never `fastapi`, + # for this repo -- the template's conditional always evaluated to + # the same constant here; using the constant directly instead + # (Phase 14 template-workflow adaptation, found in Phase 9.6/9.7, + # deliberately deferred then). + advanced-security: false + annotations: true diff --git a/DECISIONS.md b/DECISIONS.md index 03579a0..12310bb 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -47,7 +47,7 @@ mode) and all-human. Everything is built local-first but server-shaped: Docker Compose with Postgres and Redis runs locally, the Redis event bus and the Postgres schema are exercised locally, and the deploy workflows are written and - dry-run locally. Hosting research (`docs/deployment.md`) stands as the + dry-run locally. Hosting research (`docs/deployment/`) stands as the plan for when that day comes; nothing in the code assumes a particular host. ## 2. Configuration and capacity, RESOLVED 2026-09-12 diff --git a/PROGRESS.md b/PROGRESS.md index bdd4ec3..cd47e0b 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -42,8 +42,8 @@ branch; only the phase branch opens a PR into `master`. | 10 | Frontend: play, spectate, admin, focus view | done 2026-09-14 (PR #20) | | 11 | Chat and personas | done 2026-09-14 (PR #21) | | 12 | Leaderboard | done 2026-09-14 (PR #22) | -| 13 | Observability | done 2026-09-14 (branch phase-13-observability, PR pending) | -| 14 | CI/CD (local dry-run; no server yet) | | +| 13 | Observability | done 2026-09-14 (PR #23) | +| 14 | CI/CD (local dry-run; no server yet) | done 2026-09-14 (branch phase-14-cicd-dry-run, PR pending) | | 15 | Production deployment (deferred until local complete) | | | 16 | Scale and polish | | @@ -1226,6 +1226,80 @@ added a one-line comment on `FakeLLMClient.base_url` stating this explicitly, so a future test doesn't accidentally assert an absolute value there. +## Phase 14 — done 2026-09-14 (branch phase-14-cicd-dry-run) + +Self-planned. DECISIONS.md §1: "the deploy workflows are written and +dry-run locally" -- no formal spec, so scoped by reading what this +project's own prior phases had already said about Phase 14 rather than +guessing: `docs/deployment/production.md`/`staging.md` (written earlier, +already fully botpit-specific) both explicitly named "CI/CD automation +for the deploy itself" as "Phase 14's job," and +`docs/development/ci-cd-and-docker.md` (Phase 9.7) had already found and +deliberately deferred two template-org leftovers to this exact phase. + +- **A real, previously invisible CI failure, found by checking rather + than assuming a green `gh pr checks` meant everything was healthy**: + `.github/workflows/deploy.yml` (FastAPI Cloud deploy) triggers on + `push: branches: [master]` -- which never appears in `gh pr checks` + (that only shows a PR's own status checks, not workflows a *merge* + itself triggers). It had failed on **every single push to master since + this repo's first one** (`gh run list --workflow=deploy.yml`: ten-plus + consecutive failures). Root cause: `bun run --filter frontend build`, + but the actual workspace package has been `botpit-frontend` since the + Phase 9.5 rebrand -- a real bug, not a missing-secret failure. Even + fixed, it would still fail at the final "Deploy to FastAPI Cloud" step + (no `FASTAPI_CLOUD_TOKEN` exists -- correct, per DECISIONS §1, no server + yet). Fixed the real bug, and changed the trigger to `workflow_dispatch` + -- matching `deploy-docker-compose.yml`'s own already-correct pattern + for "this needs real infra to actually run" -- so it stops failing + loudly for a deploy nobody is asking it to attempt. +- **The actual "dry-run locally" work, done for real rather than trusted + on paper**: `docker compose -f compose.yml -f compose.deploy.yml + config` (the deploy overlay parses/interpolates cleanly with a + placeholder `DOMAIN`) and `build` (clean) against the real + `compose.deploy.yml`; then the "prepare database" step -- run directly + against a fresh throwaway Postgres with `FASTAPI_ENV=production` and + real (non-`"changethis"`) secrets, exactly matching what a first real + deploy would do -- full migration chain applies cleanly from empty, + superuser seeds correctly. `Settings._check_default_secret` correctly + *refused* to start on the first attempt (dev-placeholder secrets under + production-mode settings) -- confirmed that guard actually works, not a + bug to route around. Not dry-runnable, and not a gap: Traefik's real + HTTPS/ACME cert issuance needs a real domain with real DNS pointed at a + real reachable host, by construction -- waits for Phase 15 along with + the host itself. +- **Two template-org leftovers, explicitly named as deferred-to-Phase-14 + in Phase 9.7's own write-up, adapted**: `deploy.yml`'s and + `deploy-docker-compose.yml`'s `if: github.repository != + 'fastapi/full-stack-fastapi-template'` (always true for this repo, + permanently) and `zizmor.yml`'s `advanced-security: ${{ + github.repository_owner == 'fastapi' }}`/`annotations: ${{ ... != ... + }}` (always the same two constants) -- replaced the meaningless + conditionals with what they always evaluate to for `withrvr/botpit`, + removing dead template cruft. +- **Found, deliberately left alone**: `issue-manager.yml`, + `bump-pre-commit-hooks.yml`, `add-to-project.yml` all gate on + `github.repository_owner == 'fastapi'` too, but these are genuinely + fastapi-org-specific automation (their own bot account, their own + GitHub Project board) with no equivalent for a personal repo -- correctly + inert, not broken, matching the Phase 9.5/9.6 precedent of leaving these + three (plus `prepare-release.yml`)'s actual logic untouched. Confirmed by + checking their run history, not assumed: they show as skipped, never as + failed. +- **Docs**: fixed a stale `DECISIONS.md` §1 cross-reference + (`docs/deployment.md`, a file that never existed) to the real path, + `docs/deployment/` (already existed, already thorough -- `staging.md` + and `production.md` were written well before this phase and needed no + new file, just their own "Phase 14's job"/"not built yet" notes updated + to reflect what actually shipped). + +No code or tests changed this phase -- CI/CD workflow YAML and docs only. + +Full gate: `prek run --all-files` clean (including `zizmor`, the GitHub +Actions security linter, against every edited workflow file); the deploy +dry-run steps above verified directly, outside prek, against a real +throwaway Postgres and the real Docker build. + ## Phase 9.7 — done 2026-09-14 (branch phase-9.7-infra-optimization) Infrastructure-only pass: Docker image build, Compose, CI/CD workflows, diff --git a/docs/deployment/production.md b/docs/deployment/production.md index 9c5e6ae..68cbdcd 100644 --- a/docs/deployment/production.md +++ b/docs/deployment/production.md @@ -72,5 +72,8 @@ Genuinely undecided, not a stub — stated plainly rather than invented: - Whether staging and production ever get separate LLM tiers (e.g., staging on CPU only, production with an on-demand GPU pod) or share the same plan. -- CI/CD automation for the deploy itself (Phase 14's job — today's compose - commands above are run by hand). +- ~~CI/CD automation for the deploy itself~~ -- done as of Phase 14: + `.github/workflows/deploy-docker-compose.yml` already encodes the + compose commands above; see `docs/deployment/staging.md`'s "Phase 14 + status" section for what was actually dry-run verified. Still true that + nothing has deployed for real yet -- there is no host, so nothing can. diff --git a/docs/deployment/staging.md b/docs/deployment/staging.md index b60a338..869238c 100644 --- a/docs/deployment/staging.md +++ b/docs/deployment/staging.md @@ -113,5 +113,49 @@ the file" rule requires every autogenerated migration's downgrade to actually undo the upgrade). A bad deploy rolls back by restoring the previous image tag and, only if the new migration already ran and is incompatible, running `alembic downgrade -1` before restarting the old -image. No automated rollback tooling exists yet — that is Phase 14/15 work, -not built. +image. No automated rollback tooling exists yet — genuinely deferred to +Phase 15, when a real host makes "roll back a live deploy" a real +operation rather than a hypothetical one. + +## Phase 14 status: dry-run verified, no host to actually deploy to yet + +Steps 5-6 above (build, migrate, up) are exactly what +`.github/workflows/deploy-docker-compose.yml` already automates -- +`workflow_dispatch` + `runs-on: self-hosted` (correct, not a bug: there is +no self-hosted runner registered anywhere, because there is no host to run +one on yet; dispatching this workflow today would just queue forever with +nothing to pick it up). Phase 14 dry-ran the mechanics it encodes, for +real, rather than trusting the YAML was correct on paper: + +- `docker compose -f compose.yml -f compose.deploy.yml config` -- the + overlay parses and interpolates cleanly with a placeholder `DOMAIN`. +- `docker compose -f compose.yml -f compose.deploy.yml build` -- builds + clean. +- The "prepare database" step, run directly (not through compose, to + isolate it from Traefik/networking concerns already covered by + `config`) against a fresh throwaway Postgres with `FASTAPI_ENV=production` + and real (non-`"changethis"`) secrets, exactly matching what a first + real deploy would do: full migration chain applies cleanly from an empty + database, initial superuser seeds correctly. `Settings._check_default_secret` + correctly *refused* to start with the dev-placeholder `"changethis"` + values on the first attempt -- confirms that guard actually works, not + a bug to route around. +- **Not dry-runnable locally, and not a gap**: Traefik's real HTTPS/ACME + cert issuance genuinely needs a real domain with DNS pointed at a real + reachable host -- there is no way to exercise that part without one, by + construction. Everything else in the deploy mechanics is now verified; + this one piece waits for Phase 15 along with the host itself. + +Also found and fixed during Phase 14, unrelated to this compose-based +workflow: `.github/workflows/deploy.yml` (the *other*, FastAPI-Cloud-based +deploy path) was `push: branches: [master]`-triggered and had failed on +every single push to master since this repo's first one -- invisible to +every PR's `gh pr checks` because a `push`-triggered workflow never +appears there. Root cause was a real bug (`bun run --filter frontend +build`, but the actual workspace package is `botpit-frontend` since the +Phase 9.5 rebrand) layered under a design gap (no `FASTAPI_CLOUD_TOKEN` +exists, so even fixed it would still fail at the final step, correctly, +since there's no FastAPI Cloud account yet). Fixed the bug and changed +the trigger to `workflow_dispatch`, matching this file's own already-correct +precedent, so it stops failing loudly for a deploy nobody is asking it to +attempt yet.