From b36125fa67f67b5f6097e6b0a62e85e5c63f1c17 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 09:51:37 -0400 Subject: [PATCH] ci: one workflow graph, so nothing publishes on red; add govulncheck and npm audit (M462 #4984) test-go, test-web and android were separate workflows on the same push as release.yml, so the image build could not see their verdict: :dev meant "it built", never "it passed". All lanes now live in release.yml, and both publishing jobs (image-release and the new release-assets) need every lane and require `result == 'success'` from each by name, so a skipped lane blocks the publish just as a failed one does (rule 177). - New lanes: govulncheck (in golang:1.26-bookworm, the builder's image, so it checks the stdlib that ships) and `npm audit --omit=dev` in web. - Attaching the APK to a Release moved out of android-release into release-assets, behind the gate; the APK still builds in parallel. - `docker buildx build --pull`, so floating base tags can't serve a stale Go patch release from the runner's cache. - Integration wait uses `pg_isready` via docker exec: the old /dev/tcp probe never connects under dash (rule 81) and burned two minutes a run. - workflow_dispatch input force_red fails the go lane on purpose, to watch the gate refuse. - release_gate_test.go pins the gate: every job must be classified, and every publisher must need and require success from every lane. Lanes have no path filters any more; a web-only push runs the Go suite too, because "not run" must never read as "passed". Co-Authored-By: Claude Opus 5.5 --- .gitea/workflows/android.yml | 92 ------- .gitea/workflows/release.yml | 363 +++++++++++++++++++++++++-- .gitea/workflows/test-go.yml | 156 ------------ .gitea/workflows/test-web.yml | 44 ---- README.md | 3 +- ci-requirements.md | 17 +- docker-compose.yml | 2 +- internal/server/release_gate_test.go | 120 +++++++++ 8 files changed, 477 insertions(+), 320 deletions(-) delete mode 100644 .gitea/workflows/android.yml delete mode 100644 .gitea/workflows/test-go.yml delete mode 100644 .gitea/workflows/test-web.yml create mode 100644 internal/server/release_gate_test.go diff --git a/.gitea/workflows/android.yml b/.gitea/workflows/android.yml deleted file mode 100644 index 248deaae..00000000 --- a/.gitea/workflows/android.yml +++ /dev/null @@ -1,92 +0,0 @@ -name: android - -# Native Android (Kotlin/Compose/Media3) — M8 rewrite, now the only client. -# This workflow is testing only — lint + detekt + unit tests on every push -# to dev/main, plus a debug APK artifact for main. The signed-release -# build + asset attach + image-bundling lives in release.yml under a -# `needs:` chain so the docker image cannot ship without the APK. - -on: - push: - branches: [main, dev] - paths: - - 'android/**' - - '.gitea/workflows/android.yml' - -# pull_request trigger intentionally omitted — see test-web.yml for -# the rationale (single-author repo, push covers PR-merge equivalent). -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -env: - # Silences the JDK 22+ "restricted method in java.lang.System has been - # called" warning that Gradle 9.1's bundled native-platform jar trips - # at launch (System.load for native primitives). Affects the LAUNCHER - # JVM, not the daemon — that's why org.gradle.jvmargs in - # gradle.properties isn't enough. Future-compat: required opt-in once - # JDK 25 promotes the warning to an error. - JAVA_TOOL_OPTIONS: "--enable-native-access=ALL-UNNAMED" - -jobs: - build: - name: Build + lint + test - # Using flutter-ci runner label because it's the only proven-working - # label with docker that can pull our container.image. Switch to - # android-ci once the operator registers that runner label. - runs-on: flutter-ci - container: - image: git.fabledsword.com/bvandeusen/ci-android:36 - - defaults: - run: - working-directory: android - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Cache Gradle dirs - # Resolved deps + Gradle distribution + Kotlin daemon caches. - # Saves ~3 min per CI run after the first warm-up. - uses: actions/cache@v4 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - ~/.kotlin - key: gradle-${{ runner.os }}-${{ hashFiles('android/gradle/wrapper/gradle-wrapper.properties', 'android/gradle/libs.versions.toml', 'android/**/*.gradle.kts') }} - restore-keys: | - gradle-${{ runner.os }}- - - - name: Make gradlew executable - run: chmod +x ./gradlew - - - name: Gradle wrapper validation - run: ./gradlew --version - - - name: ktlint - run: ./gradlew ktlintCheck - - - name: detekt - run: ./gradlew detekt - - - name: Unit tests - run: ./gradlew testDebugUnitTest - - - name: Assemble debug - if: github.event_name == 'push' && github.ref == 'refs/heads/main' - run: ./gradlew assembleDebug - - - name: Upload debug APK - if: github.event_name == 'push' && github.ref == 'refs/heads/main' - # Stock action: it works on this forge since the runner moved to - # gitea/runner 3.x, which edits upload-artifact's client-side GHES refusal - # out of the action bundle (Scribe snippet #2271). Never @v3 — it reports - # success while Gitea serves artifacts back only through the v4 API, and - # it is what left 72 unreachable artifacts on this repo (Scribe 2270). - uses: actions/upload-artifact@v7 - with: - name: minstrel-android-debug-${{ github.sha }} - path: android/app/build/outputs/apk/debug/app-debug.apk - if-no-files-found: error diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 22aca520..d60c0ddf 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -72,9 +72,8 @@ name: release # Android APK and uploads it as a workflow artifact. The image-release # job declares `needs: android-release`, so the docker image cannot # start building until the APK is guaranteed-ready — no polling, no -# race, no silent-failure mode. Asset attachment to the gitea Release -# happens in the same android-release job, so the Release-page download -# link and the in-image bundled APK are both populated atomically. +# race, no silent-failure mode. Attaching the APK to the gitea Release is +# its own job (release-assets), behind the test gate below. # # :latest always carries an APK. Because every main push also moves # :latest (not just tags), a main build with no APK would silently strip @@ -84,8 +83,33 @@ name: release # recomputed ones — so no rebuild is needed, just a rebundle. Tag builds # keep bundling their own freshly-built APK. # -# Android testing (lint + detekt + unit tests, debug APK upload on main) -# lives in android.yml and runs independently on every push. +# THE GATE (rule 177, M462 #4984). Every verifying lane lives in this file — +# Go (vet, lint, short tests), the Postgres integration suite, the web app +# (npm audit, svelte-check, vitest), Android (ktlint, detekt, unit tests) and +# govulncheck — and every job that publishes something names each of them in +# `needs:` and requires `success` from each, by name. Nothing publishes on red. +# +# They used to be three separate workflows (test-go, test-web, android) on the +# same push trigger as this one. Separate workflows cannot see each other's +# verdict, so :dev meant "it built", never "it passed": a red test run and a +# fresh :dev could carry the same timestamp. One graph is the only place the +# edge can be written. +# +# A skipped lane is NOT a pass. The publishing conditions check +# `result == 'success'` per lane rather than `!failure()`, so a lane that +# never started blocks the publish exactly as a red one does. Lanes carry no +# path filters for the same reason: a web-only push still runs the Go suite, +# because "not run" must never read as "passed". +# +# What publishes, and is therefore gated: the image tags (image-release) and +# the APK + version sidecar attached to a tag's Release (release-assets). +# android-release only BUILDS the signed APK into a workflow artifact, which +# nobody outside this run can pull, so it runs in parallel with the lanes +# instead of after them; attaching it to the Release is the publishing half, +# and that half waits for the gate. +# +# To watch the gate refuse: dispatch this workflow with force_red=true. The go +# lane fails on purpose, and both publishing jobs must report skipped. on: push: @@ -95,6 +119,11 @@ on: - 'docs/**' - '**/*.md' workflow_dispatch: + inputs: + force_red: + description: Fail the go lane on purpose, to check that nothing publishes on red + type: boolean + default: false # A rapid re-push to main should supersede the in-flight build — the # operator explicitly wants the later commit to win. Tags no longer enter @@ -105,6 +134,265 @@ concurrency: cancel-in-progress: true jobs: + # ---------------------------------------------------------------- lanes -- + # Verifying jobs. Each one is named in the `needs:` of every publishing job + # below; add a lane here and it must be added there in the same commit. + + go: + runs-on: go-ci + container: + image: git.fabledsword.com/bvandeusen/ci-go:1.26 + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Forced failure (gate check) + if: github.event.inputs.force_red == 'true' + run: | + echo "::error::force_red dispatch: failing on purpose so the publishing jobs must skip" + exit 1 + + - name: Toolchain versions + run: | + go version + golangci-lint --version + + - name: Generated code matches queries (sqlc) + run: make verify-generate + + - name: go vet + run: go vet ./... + + - name: golangci-lint + run: golangci-lint run ./... + + - name: go test (short, race) + run: go test -short -race ./... + + # Full `go test -race` against an ephemeral Postgres. + # + # DB wiring follows the act_runner shared-daemon pattern: the runner's Docker + # daemon also runs the operator's dev compose stack, so service containers + # get NO published ports (collision) and no service-name DNS. We discover the + # service container through the mounted docker socket and reach it by bridge + # IP. The exactly-one assertion is a hard guard — pointing tests at the dev + # Postgres would truncate it (the disaster Fable #339 exists to prevent). + # + # The key stays `integration` with no `name:` (rule 80): act_runner derives + # the service container's name from the job's display name. + # + # `web/build/` has a committed placeholder index.html so go:embed succeeds + # without the SPA being built first. + integration: + runs-on: go-ci + container: + image: git.fabledsword.com/bvandeusen/ci-go:1.26 + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: minstrel + POSTGRES_PASSWORD: minstrel + POSTGRES_DB: minstrel_test + # No `ports:` — the runner shares the operator's dev compose + # Docker daemon; publishing a fixed host port collides. + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Integration suite (discover service by bridge IP, migrate, test) + run: | + set -eux + # Discover THIS job's Postgres service container via the + # mounted docker socket. act_runner attaches the job + # container and its service container(s) to a shared per-job + # network, so scope discovery to a postgres that sits on a + # network THIS job container is also on. The old + # `--filter name=integration` matched EVERY concurrent + # integration run's postgres (a dev push + the main-merge run + # overlap → 2 candidates → false "expected exactly 1" abort). + # The operator's dev compose `minstrel-postgres-*` is never on + # this job's network; skip it explicitly as belt-and-suspenders + # (a wrong target would truncate real data). + SELF=$(cat /etc/hostname) + SELF_NETS=$(docker inspect -f '{{range $k,$v := .NetworkSettings.Networks}}{{$k}} {{end}}' "$SELF") + test -n "$SELF_NETS" + echo "self ($SELF) networks: $SELF_NETS" + PG_ID="" + PG_NAME="" + for cid in $(docker ps --filter "ancestor=postgres:16-alpine" -q); do + nm=$(docker inspect -f '{{.Name}}' "$cid" | sed 's#^/##') + case "$nm" in *minstrel-postgres*|*_postgres_*) continue ;; esac + for net in $(docker inspect -f '{{range $k,$v := .NetworkSettings.Networks}}{{$k}} {{end}}' "$cid"); do + case " $SELF_NETS " in *" $net "*) PG_ID="$cid"; PG_NAME="$nm"; break 2 ;; esac + done + done + test -n "$PG_ID" || { echo "FATAL: no postgres service container on this job's network (self nets: $SELF_NETS)"; exit 1; } + echo "selected postgres: $PG_ID $PG_NAME" + PG_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$PG_ID") + test -n "$PG_IP" + export MINSTREL_TEST_DATABASE_URL="postgres://minstrel:minstrel@${PG_IP}:5432/minstrel_test?sslmode=disable" + + # Wait for Postgres to accept connections. Asked of the service + # container itself: the run: shell is dash (rule 81), where the + # old `/dev/tcp` probe never connects and the loop silently + # burned its full two minutes on every run. + ready="" + for i in $(seq 1 60); do + if docker exec "$PG_ID" pg_isready -U minstrel -d minstrel_test -q; then ready=1; break; fi + sleep 2 + done + test -n "$ready" || { echo "FATAL: postgres never became ready"; exit 1; } + + # Relax durability on the throwaway CI Postgres. Our test pattern + # is dbtest.ResetDB → TRUNCATE … RESTART IDENTITY CASCADE before + # every test, and the per-TRUNCATE commit fsync is the dominant + # cost of the integration suite. The CI DB is rebuilt every run so + # fsync / full_page_writes / synchronous_commit buy nothing. Apply + # via docker exec because: + # - The act_runner `services:` block can't override the container + # command, so `postgres -c fsync=off` at boot isn't an option. + # - ALTER SYSTEM cannot run inside a transaction; psql -c + # auto-commits each statement, which is what we need. + # - fsync / full_page_writes are sighup GUCs and + # synchronous_commit is user-context, so pg_reload_conf() picks + # all three up with no restart. + # Non-fatal: a perms surprise degrades to "slower", never red CI. + docker exec "$PG_ID" psql -U minstrel -d minstrel_test \ + -c "ALTER SYSTEM SET fsync = off" \ + -c "ALTER SYSTEM SET synchronous_commit = off" \ + -c "ALTER SYSTEM SET full_page_writes = off" \ + -c "SELECT pg_reload_conf()" \ + || echo "WARN: durability relax failed; continuing" + + # Apply embedded migrations to the fresh test DB, then run the + # full suite (no -short → integration tests execute). -p 1: + # every integration package TRUNCATEs the one shared test DB; + # concurrent package binaries → TRUNCATE deadlocks. Serialize + # package execution (the documented local invocation too). + MINSTREL_DATABASE_URL="$MINSTREL_TEST_DATABASE_URL" go run ./cmd/minstrel migrate + go test -p 1 -race ./... + + web: + runs-on: go-ci + container: + image: git.fabledsword.com/bvandeusen/ci-go:1.26 + + defaults: + run: + working-directory: web + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install deps + run: npm ci + + # What ships to browsers: `dependencies` and the runtime they pull in + # (svelte, devalue). Build and test tooling (vite, vitest, tailwind, + # kit's dev server) is left out because none of it reaches a user, and + # its open advisories need major-version upgrades tracked separately. + - name: npm audit (shipped dependencies) + run: npm audit --omit=dev --audit-level=moderate + + - name: Type-check + svelte-check + run: npm run check + + - name: Vitest + run: npm test + + android: + # Using flutter-ci runner label because it's the only proven-working + # label with docker that can pull our container.image. + runs-on: flutter-ci + container: + image: git.fabledsword.com/bvandeusen/ci-android:36 + + defaults: + run: + working-directory: android + + env: + # Silences the JDK 22+ "restricted method in java.lang.System has been + # called" warning that Gradle's bundled native-platform jar trips at + # launch. Affects the LAUNCHER JVM, not the daemon — that's why + # org.gradle.jvmargs in gradle.properties isn't enough. + JAVA_TOOL_OPTIONS: "--enable-native-access=ALL-UNNAMED" + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Cache Gradle dirs + uses: actions/cache@v4 + with: + path: | + ~/.gradle/caches + ~/.gradle/wrapper + ~/.kotlin + key: gradle-${{ runner.os }}-${{ hashFiles('android/gradle/wrapper/gradle-wrapper.properties', 'android/gradle/libs.versions.toml', 'android/**/*.gradle.kts') }} + restore-keys: | + gradle-${{ runner.os }}- + + - name: Make gradlew executable + run: chmod +x ./gradlew + + - name: Gradle wrapper validation + run: ./gradlew --version + + - name: ktlint + run: ./gradlew ktlintCheck + + - name: detekt + run: ./gradlew detekt + + - name: Unit tests + run: ./gradlew testDebugUnitTest + + - name: Assemble debug + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + run: ./gradlew assembleDebug + + - name: Upload debug APK + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + # Stock action: it works on this forge since the runner moved to + # gitea/runner 3.x, which edits upload-artifact's client-side GHES refusal + # out of the action bundle (Scribe snippet #2271). Never @v3 — it reports + # success while Gitea serves artifacts back only through the v4 API, and + # it is what left 72 unreachable artifacts on this repo (Scribe 2270). + uses: actions/upload-artifact@v7 + with: + name: minstrel-android-debug-${{ github.sha }} + path: android/app/build/outputs/apk/debug/app-debug.apk + if-no-files-found: error + + # Known vulnerabilities in the Go code and the standard library it is built + # with. Runs in the SAME image the Dockerfile's builder stage uses, so the + # standard library it checks is the one that ends up in the shipped binary; + # the ci-go image carries its own Go and would be checking a different + # toolchain. Keep this image and the Dockerfile's builder in step. + # + # govulncheck is fetched at CI time, unpinned (rule 154): the vulnerability + # database and the tool that reads it should both be current. + govulncheck: + runs-on: go-ci + container: + image: golang:1.26-bookworm + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: govulncheck + run: | + go version + go run golang.org/x/vuln/cmd/govulncheck@latest ./... + + # ------------------------------------------------------------ artifacts -- + android-release: name: Build signed APK (releases and dev) # Also builds on `dev`, which is what makes a test channel possible at @@ -245,21 +533,47 @@ jobs: # artifact existing, so an empty upload must fail here, not there. if-no-files-found: error + # Publishes the signed APK and its version sidecar on the tag's Release. + # Split out of android-release so the APK can be BUILT in parallel with the + # lanes while being PUBLISHED only once they have all passed. + release-assets: + name: Attach APK to the Release (tag releases only) + needs: [go, integration, web, android, govulncheck, android-release] + if: >- + ${{ + !cancelled() + && needs.go.result == 'success' + && needs.integration.result == 'success' + && needs.web.result == 'success' + && needs.android.result == 'success' + && needs.govulncheck.result == 'success' + && needs.android-release.result == 'success' + && startsWith(github.ref, 'refs/tags/v') }} + runs-on: go-ci + container: + image: git.fabledsword.com/bvandeusen/ci-go:1.26 + + steps: + - name: Download signed APK artifact + uses: actions/download-artifact@v8 + with: + name: minstrel-apk + path: release-apk/ + - name: Attach APK to gitea Release # Tag releases only. A dev build has no Release to hang assets on and # does not need one — the :dev image bundles the APK, and the server # serves it from /api/client/apk like any other. - if: startsWith(github.ref, 'refs/tags/v') shell: bash env: CI_TOKEN: ${{ secrets.CI_TOKEN }} - VERSION_NAME: ${{ steps.ver.outputs.name }} - VERSION_CODE: ${{ steps.ver.outputs.code }} + VERSION_NAME: ${{ needs.android-release.outputs.version_name }} + VERSION_CODE: ${{ needs.android-release.outputs.version_code }} run: | set -euxo pipefail TAG="${GITHUB_REF#refs/tags/}" REPO="${GITHUB_REPOSITORY}" - APK_PATH="app/build/outputs/apk/release/app-release.apk" + APK_PATH="release-apk/app-release.apk" ls -lh "${APK_PATH}" # Publish the version sidecar as a release asset next to the APK. @@ -313,13 +627,21 @@ jobs: image-release: name: Build + push container image - # `needs:` waits for android-release. For tag pushes android-release - # runs and must succeed before this job starts — guaranteeing the - # APK artifact is present. For main pushes android-release is - # skipped; the `if: ...` below lets this job run anyway and the - # download/copy steps gate themselves on the tag context. - needs: [android-release] - if: ${{ !failure() && !cancelled() }} + # Every lane must have SUCCEEDED, each named here (rule 177). Then the + # APK: tag and dev pushes build one and it must have succeeded; main + # pushes skip android-release and bundle the latest release's APK + # instead, so for main alone a skipped android-release is expected. + needs: [go, integration, web, android, govulncheck, android-release] + if: >- + ${{ + !cancelled() + && needs.go.result == 'success' + && needs.integration.result == 'success' + && needs.web.result == 'success' + && needs.android.result == 'success' + && needs.govulncheck.result == 'success' + && (needs.android-release.result == 'success' + || (needs.android-release.result == 'skipped' && github.ref == 'refs/heads/main')) }} runs-on: go-ci container: image: git.fabledsword.com/bvandeusen/ci-go:1.26 @@ -528,8 +850,13 @@ jobs: - name: Build and push if: steps.guard.outputs.ready == 'true' + # --pull: the Dockerfile's base images are floating tags (golang:1.26, + # debian:bookworm-slim). Without it the runner's daemon reuses + # whatever it cached, and the shipped binary can sit on a Go patch + # release govulncheck already flagged while the lane, which pulls + # fresh, reports clean. run: | - docker buildx build \ + docker buildx build --pull \ --build-arg MINSTREL_VERSION="${{ steps.tags.outputs.version }}" \ --build-arg MINSTREL_CHANNEL="${{ steps.tags.outputs.channel }}" \ --push ${{ steps.tags.outputs.args }} . @@ -554,7 +881,7 @@ jobs: # above did NOT succeed. verify-release: name: Verify release artifacts (tag releases only) - needs: [android-release, image-release] + needs: [android-release, release-assets, image-release] if: ${{ always() && startsWith(github.ref, 'refs/tags/v') }} runs-on: go-ci container: diff --git a/.gitea/workflows/test-go.yml b/.gitea/workflows/test-go.yml deleted file mode 100644 index 49cb3684..00000000 --- a/.gitea/workflows/test-go.yml +++ /dev/null @@ -1,156 +0,0 @@ -name: test-go - -# Go server: vet + golangci-lint + short race tests. Runs on push to -# dev/main and PRs to main, scoped to Go-side files only — web-only or -# Flutter-only diffs don't trigger this workflow. -# -# Two jobs: `test` (fast — vet + lint + `go test -short -race`, no DB) and -# `integration` (full `go test -race` against an ephemeral Postgres). -# -# Integration-job DB wiring follows the act_runner shared-daemon pattern: -# the runner's Docker daemon also runs the operator's dev compose stack, -# so service containers get NO published ports (collision) and no -# service-name DNS. We discover the service container by the job-scoped -# name filter via the mounted docker socket and reach it by bridge IP. -# The exactly-one assertion is a hard guard — pointing tests at the dev -# Postgres would truncate it (the disaster Fable #339 exists to prevent). -# -# `web/build/` has a committed placeholder index.html so go:embed succeeds -# without needing the SPA to be freshly built. Real builds happen in -# release.yml (container) and locally during dev. - -on: - push: - branches: [dev, main] - paths: - - '**/*.go' - - 'go.mod' - - 'go.sum' - - 'sqlc.yaml' - - 'Makefile' - - 'internal/**' - - 'cmd/**' - - '.golangci.yml' - - '.gitea/workflows/test-go.yml' - # The release lane's own trigger is `main` + tags, so nothing it - # contains is exercised until a release is already running. These two - # entries are what let internal/server/release_version_test.go guard - # the version derivation on ordinary dev pushes instead. - - 'ci/**' - - '.gitea/workflows/release.yml' - -# pull_request trigger intentionally omitted — see test-web.yml for -# the rationale (single-author repo, push covers PR-merge equivalent). -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - test: - runs-on: go-ci - container: - image: git.fabledsword.com/bvandeusen/ci-go:1.26 - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Toolchain versions - run: | - go version - golangci-lint --version - - - name: Generated code matches queries (sqlc) - run: make verify-generate - - - name: go vet - run: go vet ./... - - - name: golangci-lint - run: golangci-lint run ./... - - - name: go test (short, race) - run: go test -short -race ./... - - integration: - runs-on: go-ci - container: - image: git.fabledsword.com/bvandeusen/ci-go:1.26 - services: - postgres: - image: postgres:16-alpine - env: - POSTGRES_USER: minstrel - POSTGRES_PASSWORD: minstrel - POSTGRES_DB: minstrel_test - # No `ports:` — the runner shares the operator's dev compose - # Docker daemon; publishing a fixed host port collides. - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Integration suite (discover service by bridge IP, migrate, test) - run: | - set -eux - # Discover THIS job's Postgres service container via the - # mounted docker socket. act_runner attaches the job - # container and its service container(s) to a shared per-job - # network, so scope discovery to a postgres that sits on a - # network THIS job container is also on. The old - # `--filter name=integration` matched EVERY concurrent - # integration run's postgres (a dev push + the main-merge run - # overlap → 2 candidates → false "expected exactly 1" abort). - # The operator's dev compose `minstrel-postgres-*` is never on - # this job's network; skip it explicitly as belt-and-suspenders - # (a wrong target would truncate real data). - SELF=$(cat /etc/hostname) - SELF_NETS=$(docker inspect -f '{{range $k,$v := .NetworkSettings.Networks}}{{$k}} {{end}}' "$SELF") - test -n "$SELF_NETS" - echo "self ($SELF) networks: $SELF_NETS" - PG_ID="" - PG_NAME="" - for cid in $(docker ps --filter "ancestor=postgres:16-alpine" -q); do - nm=$(docker inspect -f '{{.Name}}' "$cid" | sed 's#^/##') - case "$nm" in *minstrel-postgres*|*_postgres_*) continue ;; esac - for net in $(docker inspect -f '{{range $k,$v := .NetworkSettings.Networks}}{{$k}} {{end}}' "$cid"); do - case " $SELF_NETS " in *" $net "*) PG_ID="$cid"; PG_NAME="$nm"; break 2 ;; esac - done - done - test -n "$PG_ID" || { echo "FATAL: no postgres service container on this job's network (self nets: $SELF_NETS)"; exit 1; } - echo "selected postgres: $PG_ID $PG_NAME" - PG_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$PG_ID") - test -n "$PG_IP" - export MINSTREL_TEST_DATABASE_URL="postgres://minstrel:minstrel@${PG_IP}:5432/minstrel_test?sslmode=disable" - - # Wait for Postgres to accept TCP (no health-check dependency). - for i in $(seq 1 60); do (echo > "/dev/tcp/${PG_IP}/5432") 2>/dev/null && break; sleep 2; done - - # Relax durability on the throwaway CI Postgres. Our test pattern - # is dbtest.ResetDB → TRUNCATE … RESTART IDENTITY CASCADE before - # every test, and the per-TRUNCATE commit fsync is the dominant - # cost of the integration suite. The CI DB is rebuilt every run so - # fsync / full_page_writes / synchronous_commit buy nothing. Apply - # via docker exec because: - # - The act_runner `services:` block can't override the container - # command, so `postgres -c fsync=off` at boot isn't an option. - # - ALTER SYSTEM cannot run inside a transaction; psql -c - # auto-commits each statement, which is what we need. - # - fsync / full_page_writes are sighup GUCs and - # synchronous_commit is user-context, so pg_reload_conf() picks - # all three up with no restart. - # Non-fatal: a perms surprise degrades to "slower", never red CI. - docker exec "$PG_ID" psql -U minstrel -d minstrel_test \ - -c "ALTER SYSTEM SET fsync = off" \ - -c "ALTER SYSTEM SET synchronous_commit = off" \ - -c "ALTER SYSTEM SET full_page_writes = off" \ - -c "SELECT pg_reload_conf()" \ - || echo "WARN: durability relax failed; continuing" - - # Apply embedded migrations to the fresh test DB, then run the - # full suite (no -short → integration tests execute). -p 1: - # every integration package TRUNCATEs the one shared test DB; - # concurrent package binaries → TRUNCATE deadlocks. Serialize - # package execution (the documented local invocation too). - MINSTREL_DATABASE_URL="$MINSTREL_TEST_DATABASE_URL" go run ./cmd/minstrel migrate - go test -p 1 -race ./... diff --git a/.gitea/workflows/test-web.yml b/.gitea/workflows/test-web.yml deleted file mode 100644 index 9893608d..00000000 --- a/.gitea/workflows/test-web.yml +++ /dev/null @@ -1,44 +0,0 @@ -name: test-web - -# Web SPA: vitest + svelte-check. Runs on push to dev/main only — -# the `pull_request` trigger is intentionally omitted because every -# branch on this repo is local-only (no fork PRs), so the dev push -# fully covers what a PR run would re-execute. Keeping both events -# doubled CI cost on every commit. - -on: - push: - branches: [dev, main] - paths: - - 'web/**' - - '.gitea/workflows/test-web.yml' - -# Cancel an earlier in-flight run for the same ref when a newer -# commit arrives. With cancel-in-progress, rapid re-pushes don't -# pile up zombie runs. -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - test: - runs-on: go-ci - container: - image: git.fabledsword.com/bvandeusen/ci-go:1.26 - - defaults: - run: - working-directory: web - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Install deps - run: npm ci - - - name: Type-check + svelte-check - run: npm run check - - - name: Vitest - run: npm test diff --git a/README.md b/README.md index 3f0ac051..f1872700 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,8 @@ Two concurrent dev processes: truncates your dev `minstrel` data (admin user, library, likes). It brings up the compose Postgres and creates the test DB if missing. - CI runs both: a fast `go test -short -race` gate plus an integration - job with its own ephemeral Postgres (`.gitea/workflows/test-go.yml`). + job with its own ephemeral Postgres (the `integration` lane in + `.gitea/workflows/release.yml`, which also gates every image publish). ### Production build diff --git a/ci-requirements.md b/ci-requirements.md index 2a579118..d933be14 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -12,8 +12,9 @@ git.fabledsword.com/bvandeusen/ci-go:1.26 git.fabledsword.com/bvandeusen/ci-android:36 ``` -- `ci-go:1.26` — Go server tests (`.gitea/workflows/test-go.yml`), web SPA tests (`.gitea/workflows/test-web.yml`), and the release container build (`release.yml`'s `image-release` job). -- `ci-android:36` — native Kotlin/Compose client: ktlint + detekt + unit tests + debug APK (`.gitea/workflows/android.yml`), and the signed release APK (`release.yml`'s `android-release` job). +- `ci-go:1.26` — the `go`, `integration` and `web` lanes, `release-assets`, and the container build (`image-release`). All CI lives in one workflow, `.gitea/workflows/release.yml`, so every publishing job can depend on every lane (rule 177). +- `golang:1.26-bookworm` (Docker Hub) — the `govulncheck` lane. Deliberately the same image as the Dockerfile's builder stage rather than `ci-go`, so the standard library it checks is the one in the shipped binary. Keep the two in step. +- `ci-android:36` — native Kotlin/Compose client: ktlint + detekt + unit tests + debug APK (the `android` lane), and the signed release APK (`android-release`). **`ci-flutter` is no longer consumed, and `ci-flutter` can now be retired.** The M8 rewrite replaced the Flutter client with the native Android app and @@ -30,9 +31,9 @@ split below. The label is a scheduling handle, not a toolchain assertion. ### From `ci-go:1.26` - **Go** (1.26 toolchain) — `go vet`, `go test -race`, `go build`, `go mod`. -- **Node + npm** — `npm ci` and `npm test` / `npm run check` in `test-web.yml`. -- **golangci-lint** — lint pass in `test-go.yml`. -- **docker CLI** — bridge-IP discovery of the per-job Postgres service container in `test-go.yml` integration job (via the runner's shared `/var/run/docker.sock`). +- **Node + npm** — `npm ci`, `npm audit`, `npm test` and `npm run check` in the `web` lane. +- **golangci-lint** — lint pass in the `go` lane. +- **docker CLI** — bridge-IP discovery of the per-job Postgres service container in the `integration` lane (via the runner's shared `/var/run/docker.sock`). - **docker buildx** — release container build + push in `release.yml`. - **curl** — release-asset polling / upload in `release.yml`. @@ -45,7 +46,7 @@ split below. The label is a scheduling handle, not a toolchain assertion. No NDK: the native client has no C/C++ sources (this is why it isn't on `ci-flutter`). - **ktlint + detekt** — `./gradlew ktlintCheck` and `./gradlew detekt` in - `android.yml`. Image pins track `android/gradle/libs.versions.toml` so local + the `android` lane. Image pins track `android/gradle/libs.versions.toml` so local and CI checks agree. - **git** — `actions/checkout@v4` baseline (and any shell git operations). - **base64 + curl** — keystore decode + release-asset upload in `release.yml`'s @@ -58,10 +59,10 @@ None. ## Notes - **Label/image split.** Workflows keep `runs-on: go-ci` / `runs-on: flutter-ci` as the scheduling label per the [`ci-runners.md`](https://…/FabledRulebook/ci-runners.md) "label = scheduling handle, image = `container.image`" pattern. The labels are intentional handles, not toolchain assertions — which is why the Android jobs still schedule on `flutter-ci` while pulling `ci-android:36`. Switch them to `android-ci` if that runner label is ever registered; nothing breaks either way. -- **Integration-job docker-socket dependency.** `test-go.yml`'s integration job uses the runner's shared docker socket (`/var/run/docker.sock`) to bridge-IP-discover the per-job Postgres service container by name + network intersection — the dev compose's `minstrel-postgres-*` containers are explicitly skipped as belt-and-suspenders. Depends on `act_runner.valid_volumes` whitelisting the socket; if that ever stops auto-mounting, integration tests fail at the `docker inspect` step. +- **Integration-job docker-socket dependency.** The `integration` lane uses the runner's shared docker socket (`/var/run/docker.sock`) to bridge-IP-discover the per-job Postgres service container by name + network intersection — the dev compose's `minstrel-postgres-*` containers are explicitly skipped as belt-and-suspenders. Depends on `act_runner.valid_volumes` whitelisting the socket; if that ever stops auto-mounting, integration tests fail at the `docker inspect` step. - **Go toolchain pin.** `go.mod` is on `go 1.25.0` because `golang.org/x/crypto v0.51.0` declares 1.25 as its minimum. `ci-go:1.26` satisfies this with headroom. Future `x/crypto` bumps that move the Go floor should be paired with an image-tag bump in this file + the workflows. - **In-app update channel — `needs:`, not polling.** `release.yml`'s `image-release` job declares `needs: [android-release]`, so on tag pushes the signed APK is guaranteed present before the image build starts — no polling window, no race. (The old cross-workflow polling against `flutter.yml` is gone with that workflow.) On non-tag `main` pushes `android-release` is skipped and `image-release` instead pulls the most recent release's APK and reconstructs its exact `versionName`, so `:latest` never ships without an update channel. It degrades to an empty `client/` — never a wrong version — if no release, asset, or tag commit-count can be resolved. -- **Cache server reachability.** `test-web.yml` does NOT use `cache: 'npm'` on `actions/setup-node` — the Gitea Actions cache server isn't reachable from this runner's container network and `setup-node` was burning ~4m41s on ETIMEDOUT before failing open. With the migration to `ci-go:1.26`, `setup-node` is removed entirely (Node is in the image). The cache concern reappears if a future change re-introduces a network-dependent action. +- **Cache server reachability.** The `web` lane does NOT use `cache: 'npm'` on `actions/setup-node` — the Gitea Actions cache server isn't reachable from this runner's container network and `setup-node` was burning ~4m41s on ETIMEDOUT before failing open. With the migration to `ci-go:1.26`, `setup-node` is removed entirely (Node is in the image). The cache concern reappears if a future change re-introduces a network-dependent action. - **Artifacts — stock `actions/upload-artifact@v7` and `actions/download-artifact@v8`; never `@v3`.** ```yaml uses: actions/upload-artifact@v7 diff --git a/docker-compose.yml b/docker-compose.yml index 140294ab..a56998cf 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -8,7 +8,7 @@ version: "3.9" # tests at it: # make test-integration # (CI runs the same suite against its own ephemeral Postgres service — -# see .gitea/workflows/test-go.yml.) +# see the `integration` job in .gitea/workflows/release.yml.) # # Full stack (server + db): # docker compose up --build diff --git a/internal/server/release_gate_test.go b/internal/server/release_gate_test.go new file mode 100644 index 00000000..345d84de --- /dev/null +++ b/internal/server/release_gate_test.go @@ -0,0 +1,120 @@ +package server + +import ( + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +// The publish gate (rule 177, M462 #4984), pinned. release.yml holds every +// verifying lane and every publishing job in one graph so that a publish can +// depend on the lanes; these tests keep that dependency from eroding. +// +// The failure each one guards against is silent. A lane added without being +// named in a publisher's needs runs, goes red, and :dev publishes anyway. A +// condition relaxed to `!failure()` lets a lane that never started count as +// a pass. Neither shows up as a broken build — only as a gate that no longer +// gates. + +// gateLanes are the verifying jobs. Every publisher must need each of them and +// require its success by name. +var gateLanes = []string{"go", "integration", "web", "android", "govulncheck"} + +// gatePublishers push something other people can pull: image tags, and the +// APK + sidecar attached to a Release. +var gatePublishers = []string{"image-release", "release-assets"} + +// gateOthers are the remaining jobs and why they sit outside the gate: +// android-release only builds a workflow artifact (nothing outside the run can +// pull it), and verify-release reports on a finished release. +var gateOthers = []string{"android-release", "verify-release"} + +type workflowJob struct { + Needs any `yaml:"needs"` + If string `yaml:"if"` +} + +func releaseJobs(t *testing.T) map[string]workflowJob { + t.Helper() + body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml")) + if err != nil { + t.Fatal(err) + } + var wf struct { + Jobs map[string]workflowJob `yaml:"jobs"` + } + if err := yaml.Unmarshal(body, &wf); err != nil { + t.Fatalf("parse release.yml: %v", err) + } + if len(wf.Jobs) == 0 { + t.Fatal("release.yml has no jobs") + } + return wf.Jobs +} + +func jobNeeds(j workflowJob) []string { + switch v := j.Needs.(type) { + case string: + return []string{v} + case []any: + out := make([]string, 0, len(v)) + for _, n := range v { + if s, ok := n.(string); ok { + out = append(out, s) + } + } + return out + } + return nil +} + +// A job nobody has classified is the case that matters: a new lane that was +// never added to the publishers' needs. +func TestReleaseGate_EveryJobIsClassified(t *testing.T) { + for name := range releaseJobs(t) { + if slices.Contains(gateLanes, name) || slices.Contains(gatePublishers, name) || slices.Contains(gateOthers, name) { + continue + } + t.Errorf("release.yml job %q is not classified. If it verifies, add it to gateLanes and to every publisher's needs and if; "+ + "if it publishes, add it to gatePublishers and gate it; otherwise add it to gateOthers with the reason", name) + } + for _, want := range append(append(slices.Clone(gateLanes), gatePublishers...), gateOthers...) { + if _, ok := releaseJobs(t)[want]; !ok { + t.Errorf("release.yml has no %q job, which this test expects", want) + } + } +} + +func TestReleaseGate_PublishersNeedEveryLaneToSucceed(t *testing.T) { + jobs := releaseJobs(t) + for _, pub := range gatePublishers { + job, ok := jobs[pub] + if !ok { + t.Errorf("no %q job", pub) + continue + } + needs := jobNeeds(job) + cond := strings.Join(strings.Fields(job.If), " ") + for _, lane := range gateLanes { + if !slices.Contains(needs, lane) { + t.Errorf("%s does not need %q: it can publish without waiting for that lane", pub, lane) + } + if !strings.Contains(cond, "needs."+lane+".result == 'success'") { + t.Errorf("%s's if does not require needs.%s.result == 'success': a skipped or failed %s would not stop it\n if: %s", + pub, lane, lane, cond) + } + } + // always() and failure() both make a job run past a red dependency; + // !cancelled() is fine only because the per-lane success checks above + // carry the gate. + for _, forbidden := range []string{"always()", "failure()"} { + if strings.Contains(cond, forbidden) { + t.Errorf("%s's if uses %s, which runs it past a failed lane\n if: %s", pub, forbidden, cond) + } + } + } +}