A gate for the weekly refresh, and the lever that makes it testable #248

Merged
bvandeusen merged 3 commits from dev into main 2026-09-02 15:51:50 -04:00
Owner

Milestone 362 step 3, plus the fix for a bug the lever had.

2c88ad3 + 24a2b70 — the refresh lever silently did nothing

Runs 5265 and 5270 dispatched with refresh=true both went green with every step skipped. That is the dangerous shape: a refresh whose trigger evaluates false is behaviourally identical to an ordinary push, so success is exactly what it looks like.

2c88ad3 added a diagnostic to the step that already exists to say what a run derived. It gave the answer immediately:

expression '(… || github.event.inputs.refresh == 'true') && 'true' || 'false''
  evaluated to '%!t(string=false)'
trigger: raw inputs refresh='true'

The input arrived as true and the comparison still said false. type: boolean delivers a real boolean, and GitHub expression semantics cast mismatched operands to numbers — true == 'true' compares 1 against NaN. My comment on the previous commit asserted the opposite ("Forgejo delivers inputs as strings") and asserted it without checking.

force_build was never bitten because it compares in the shell, where everything is a string already. 24a2b70 normalises through format(), which is correct whichever representation the forge uses — guessing the representation is what caused this. Filed as #3414.

Verified on run 5277: guard fired, BUILD_REF resolved to main, the build published the candidate, and the promote pointed :latest at it. Afterwards the registry shows mediaType: …image.manifest.v1+json (a plain manifest, not an index) and fc.revision still readable — #3183 avoided, demonstrated rather than argued — with created holding the commit time, not the build time.

bfa9fd6 — the smoke suite

ci.yml cannot gate a base refresh: its lanes run on ci-python:3.14 and install requirements.txt, and a refresh changes neither. What it re-resolves is the Dockerfile's apt layer — ffmpeg unar libpq5 postgresql-client zstd megatools libjpeg62-turbo libwebp7 libpng16-16 — unpinned, every build, and nothing else in this repo looks at it.

So smoke-web runs the candidate image against real pgvector and redis:

  1. alembic upgrade head on an empty DB — the image's own libpq and psycopg, and the same call entrypoint.sh makes before serving.
  2. The apt binaries, then the app's own Thumbnailer — JPEG, PNG-with-alpha, WebP, and a video frame through ffmpeg. It needs no DB and no app context, so this is real product code against the refreshed libraries. ffmpeg -version exiting 0 would pass while a codec removal broke every thumbnail in the library.
  3. The web role boots and answers /api/health.

Every failure names the package it implicates.

Why this needs to be on main to be tested at all

smoke-web checks out BUILD_REF (= main), so scripts/smoke_image.py has to be there. That ref is correct and should not be changed to dodge this: the script imports Thumbnailer from inside the image, so it must match the image's API, and the image is built from BUILD_REF. Pointing it at the workflow's own branch would let a dev-side script reference an API main's image lacks — a false failure in the one job nobody is watching.

Same constraint that gated the previous PR, recurring cleanly, which is evidence the design is consistent rather than accidentally awkward.

Not yet done

The gate reports a verdict; it does not gate the promote. That is step 4, deliberately separate — landing the gate and the thing it gates together would mean the first time anyone saw this job run would also be the first time it could stop a publish. It also still has to be shown FAILING before it can be believed.

Verification

CI green on bfa9fd6, all five lanes. ruff covers scripts/, so the new script is linted there.

Prod runs adab336 and is unaffected until the next deploy.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TTjbZZ6JirCMSaJzQV1RhA

Milestone 362 step 3, plus the fix for a bug the lever had. ## `2c88ad3` + `24a2b70` — the refresh lever silently did nothing Runs 5265 and 5270 dispatched with `refresh=true` both went **green with every step skipped**. That is the dangerous shape: a refresh whose trigger evaluates false is behaviourally identical to an ordinary push, so success is exactly what it looks like. `2c88ad3` added a diagnostic to the step that already exists to say what a run derived. It gave the answer immediately: ``` expression '(… || github.event.inputs.refresh == 'true') && 'true' || 'false'' evaluated to '%!t(string=false)' trigger: raw inputs refresh='true' ``` The input arrived as `true` and the comparison still said false. `type: boolean` delivers a real boolean, and GitHub expression semantics cast mismatched operands to numbers — `true == 'true'` compares 1 against NaN. My comment on the previous commit asserted the opposite ("Forgejo delivers inputs as strings") and asserted it without checking. `force_build` was never bitten because it compares in the *shell*, where everything is a string already. `24a2b70` normalises through `format()`, which is correct whichever representation the forge uses — guessing the representation is what caused this. Filed as #3414. Verified on run 5277: guard fired, `BUILD_REF` resolved to `main`, the build published the candidate, and the promote pointed `:latest` at it. Afterwards the registry shows `mediaType: …image.manifest.v1+json` (a plain manifest, **not** an index) and `fc.revision` still readable — #3183 avoided, demonstrated rather than argued — with `created` holding the *commit* time, not the build time. ## `bfa9fd6` — the smoke suite `ci.yml` cannot gate a base refresh: its lanes run on `ci-python:3.14` and install `requirements.txt`, and a refresh changes neither. What it re-resolves is the Dockerfile's apt layer — `ffmpeg unar libpq5 postgresql-client zstd megatools libjpeg62-turbo libwebp7 libpng16-16` — unpinned, every build, and nothing else in this repo looks at it. So `smoke-web` runs the **candidate image** against real pgvector and redis: 1. `alembic upgrade head` on an empty DB — the image's own libpq and psycopg, and the same call `entrypoint.sh` makes before serving. 2. The apt binaries, then the app's own `Thumbnailer` — JPEG, PNG-with-alpha, WebP, and a video frame through ffmpeg. It needs no DB and no app context, so this is real product code against the refreshed libraries. `ffmpeg -version` exiting 0 would pass while a codec removal broke every thumbnail in the library. 3. The web role boots and answers `/api/health`. Every failure names the package it implicates. ## Why this needs to be on `main` to be tested at all `smoke-web` checks out `BUILD_REF` (= `main`), so `scripts/smoke_image.py` has to be there. That ref is correct and should not be changed to dodge this: the script imports `Thumbnailer` from *inside the image*, so it must match the image's API, and the image is built from `BUILD_REF`. Pointing it at the workflow's own branch would let a dev-side script reference an API main's image lacks — a false failure in the one job nobody is watching. Same constraint that gated the previous PR, recurring cleanly, which is evidence the design is consistent rather than accidentally awkward. ## Not yet done The gate **reports** a verdict; it does not gate the promote. That is step 4, deliberately separate — landing the gate and the thing it gates together would mean the first time anyone saw this job run would also be the first time it could stop a publish. It also still has to be shown FAILING before it can be believed. ## Verification CI green on `bfa9fd6`, all five lanes. `ruff` covers `scripts/`, so the new script is linted there. Prod runs `adab336` and is unaffected until the next deploy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01TTjbZZ6JirCMSaJzQV1RhA
bvandeusen added 3 commits 2026-09-02 15:51:44 -04:00
ci: report the raw and normalised trigger values
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 2s
Build images / build-ml (push) Successful in 7s
Build images / build-agent (push) Successful in 7s
CI / backend-lint-and-test (push) Successful in 31s
Build images / build-web (push) Successful in 9s
extension / lint (push) Successful in 16s
CI / frontend-build (push) Successful in 22s
CI / integration (push) Successful in 2m39s
2c88ad3efb
The refresh dispatch on run 5265 went green with every step skipped: the
main-only guard did not fire, checkout took dev, and the reuse step read
IS_REFRESH as false. So both workflow-level expressions evaluated false while
the identical accessor works for force_build, which compares its value in the
shell rather than in an expression.

That is a guess until it is measured, and the failure is silent by
construction — a refresh that evaluates false behaves exactly like an ordinary
push and reports success. This prints the raw input beside the normalised
value in the step that already exists to say what a run derived, so the two
disagreeing is visible rather than inferred.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTjbZZ6JirCMSaJzQV1RhA
ci: a boolean input never equals the string 'true'
Build images / sign-extension (push) Successful in 4s
Build images / build-ml (push) Successful in 6s
Build images / build-agent (push) Successful in 7s
CI / frontend-build (push) Successful in 23s
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 2s
CI / backend-lint-and-test (push) Successful in 35s
Build images / build-web (push) Successful in 8s
CI / integration (push) Successful in 1m48s
24a2b70a5a
The refresh lever did not work, and the way it did not work is the point.

Run 5270 dispatched with refresh=true. Its log:

  expression '(github.event_name == 'schedule'
               || github.event.inputs.refresh == 'true') && 'true' || 'false''
    evaluated to '%!t(string=false)'
  trigger: event=workflow_dispatch IS_REFRESH='false' BUILD_REF='refs/heads/dev'
  trigger: raw inputs refresh='true' force_build='false'

The input arrived as true and the comparison still said false. `type: boolean`
delivers a real boolean, and GitHub expression semantics cast operands to
numbers when their types differ — so `true == 'true'` compares 1 against NaN.
My comment on the previous commit asserted the opposite, that Forgejo delivers
inputs as strings, and asserted it without checking.

The run went GREEN with every step skipped, because a refresh that evaluates
false is indistinguishable from an ordinary push. A lever that silently does
nothing is worse than no lever: it would have been trusted.

Normalised through format(), which is representation-independent — a boolean
true and a string 'true' both render 'true'. That is also why force_build was
never bitten: it passes its raw value into an env var and compares in the
shell, where everything is a string already. format() buys the same thing at
expression level, which is where a step `if:` needs the answer.

The diagnostic from the previous commit stays. It is what turned this from a
guess into a measurement, and it is the only thing that would catch the same
class of failure next time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTjbZZ6JirCMSaJzQV1RhA
ci: smoke the refreshed image against real Postgres and Redis
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 3s
CI / extension-version (push) Successful in 4s
Build images / build-ml (push) Successful in 7s
Build images / build-agent (push) Successful in 9s
Build images / build-web (push) Successful in 7s
Build images / smoke-web (push) Skipped
extension / lint (push) Successful in 20s
CI / frontend-build (push) Successful in 23s
CI / backend-lint-and-test (push) Successful in 32s
CI / integration (push) Successful in 1m48s
extension / lint (pull_request) Successful in 20s
bfa9fd678b
Milestone 362 step 3. This is the gate the weekly base refresh never had.

`ci.yml` cannot be that gate, and the reason matters more than the fix. Its
lanes run on ci-python:3.14 and install requirements.txt — a base refresh
changes neither, so all five stay green through a bump that breaks the product.
What a refresh re-resolves is the Dockerfile's apt layer:

    ffmpeg unar libpq5 postgresql-client zstd megatools
    libjpeg62-turbo libwebp7 libpng16-16 ca-certificates

Unpinned, every build, and nothing else in this repo looks at it. That line is
the dependency creep; it is also precisely what the test suite structurally
cannot observe, since the suite never runs inside the image and the image
carries no tests and no pytest.

So `smoke-web` runs the CANDIDATE IMAGE against real service containers:

  1. `alembic upgrade head` on an empty database — the image's own libpq and
     psycopg, and the same call entrypoint.sh makes before it serves anything,
     so a failure here is a failure to boot.
  2. The apt binaries, then the application's own `Thumbnailer` — JPEG, PNG
     with alpha, WebP, and a video frame through ffmpeg. `Thumbnailer` needs no
     database and no app context, so the check exercises real product code
     rather than a proxy for it. `ffmpeg -version` exiting 0 would pass while a
     codec removal broke every thumbnail in the library.
  3. The web role boots and answers /api/health.

Every failure names the package it implicates. This fires on a Sunday,
unattended, about a change nobody made deliberately — "assertion failed" a week
later teaches nobody anything.

The script is piped over stdin rather than bind-mounted: the workspace is a
docker volume belonging to the job's own container, so a host bind of $PWD does
not resolve for a sibling. Container logs are dumped only on failure, and the
trap re-exits with the real status rather than the status of `docker rm`.

Deliberately NOT gating the promote yet — that is step 4. Landing the gate and
the thing it gates together would mean the first time anyone saw this job run
would also be the first time it could stop a publish.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TTjbZZ6JirCMSaJzQV1RhA
bvandeusen merged commit d01f33dea6 into main 2026-09-02 15:51:50 -04:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bvandeusen/FabledCurator#248