ci: deploys are not gated on the checks workflow, so a test-breaking merge still ships both apps #85

New issue

Closed

opened 2026-09-02 19:19:46 +00:00 by yuris · 0 comments

yuris commented

2026-09-02 19:19:46 +00:00

Owner

Required by Reviewer80r2 as a condition of approving #80: the risk below is real, deliberately deferred, and after #80 lands it is recorded only in a YAML comment. This issue is that record.

The gap

.forgejo/workflows/checks.yml runs the tests, the typechecks and the lint gate on pull_request and on push: main. It exposes workflow_call so a deploy workflow can depend on it. Nothing does.

$ grep -n 'checks' .forgejo/workflows/build-publish.yml .forgejo/workflows/deploy-store.yml
(no output)

So push: main still deploys unconditionally: a merge that breaks the tests ships to production for both apps — the management container via build-publish.yml and the storefront via deploy-store.yml. The checks go red beside the deploy, not in front of it.

Why it was deferred rather than wired

Three reasons, all recorded during #80's review:

Wiring puts a hand-maintained table on the production path. The gate ratchets against a per-rule BASELINE in biome-gate.sh. That table went stale twice in one week — measured at d0b5825, then again after #75/#76/#78 merged, which fixed two format findings and added nine files. A vanished rule is a hard failure by design. Wiring the gate converts bookkeeping drift into a release outage.
The deploy path is already noisy. See #79: 56 workflow_dispatch deploys of one unchanged commit, many cancelled. Adding a gate to a path that already fires spuriously changes the failure mode from "wasted runner minutes" to "release blocked".
It is a different risk from the one #80 removes, and it deserves its own review rather than riding along in the commit that first makes tests run at all.

Why deferring costs nothing to reverse

The mechanism is proven, not assumed. tmp/ci-probe carried a probe-call.yml that did uses: ./.forgejo/workflows/checks.yml, and tasks 994–1000 carry workflow_id=probe-call.yml at run_number=120 while running the six job names that exist only in checks.yml — and the caller job probe went red alongside the callee. So Forgejo resolves a local reusable workflow and propagates the callee's failure to the caller.

Wiring it is therefore a two-line edit per deploy workflow:

jobs:
  checks:
    uses: ./.forgejo/workflows/checks.yml
  deploy:
    needs: checks
    # …existing job…

checks.yml's concurrency key is already shaped for it — keyed on the calling workflow as well as the ref, so a gate run is not cancelled by the standalone run of the same commit.

What to decide when picking this up

Gate both apps or one? The storefront is a static export and cheap to roll back; the management container is not.
Which checks are load-bearing? Blocking a release on test is uncontroversial. Blocking it on the lint gate means a stale baseline stops a deploy — see reason 1. Gating on needs: checks takes all six jobs; a narrower needs on just the test jobs is possible and probably right.
Do #79 and the baseline-staleness risk need closing first? Reviewer80r2's position, which I share: land the gate's own hardening before putting it on the release path.

#80 added the workflow and the gate, and corrected a comment that claimed these deploy-side checks jobs already existed when they never did.
#79 documents the workflow_dispatch deploy noise.
#73 asked for this decision to be recorded rather than merely made.

Required by `Reviewer80r2` as a condition of approving #80: the risk below is real, deliberately deferred, and after #80 lands it is recorded **only in a YAML comment**. This issue is that record. ## The gap `.forgejo/workflows/checks.yml` runs the tests, the typechecks and the lint gate on `pull_request` and on `push: main`. It exposes `workflow_call` so a deploy workflow can depend on it. **Nothing does.** ``` $ grep -n 'checks' .forgejo/workflows/build-publish.yml .forgejo/workflows/deploy-store.yml (no output) ``` So `push: main` still deploys unconditionally: a merge that breaks the tests ships to production for **both** apps — the management container via `build-publish.yml` and the storefront via `deploy-store.yml`. The checks go red *beside* the deploy, not in front of it. ## Why it was deferred rather than wired Three reasons, all recorded during #80's review: 1. **Wiring puts a hand-maintained table on the production path.** The gate ratchets against a per-rule `BASELINE` in `biome-gate.sh`. That table went stale twice in one week — measured at `d0b5825`, then again after `#75/#76/#78` merged, which fixed two `format` findings and added nine files. A vanished rule is a hard failure by design. Wiring the gate converts bookkeeping drift into a **release outage**. 2. **The deploy path is already noisy.** See #79: 56 `workflow_dispatch` deploys of one unchanged commit, many cancelled. Adding a gate to a path that already fires spuriously changes the failure mode from "wasted runner minutes" to "release blocked". 3. **It is a different risk from the one #80 removes**, and it deserves its own review rather than riding along in the commit that first makes tests run at all. ## Why deferring costs nothing to reverse The mechanism is **proven, not assumed**. `tmp/ci-probe` carried a `probe-call.yml` that did `uses: ./.forgejo/workflows/checks.yml`, and tasks 994–1000 carry `workflow_id=probe-call.yml` at `run_number=120` while running the six job names that exist only in `checks.yml` — and the caller job `probe` went red alongside the callee. So Forgejo resolves a local reusable workflow **and** propagates the callee's failure to the caller. Wiring it is therefore a two-line edit per deploy workflow: ```yaml jobs: checks: uses: ./.forgejo/workflows/checks.yml deploy: needs: checks # …existing job… ``` `checks.yml`'s `concurrency` key is already shaped for it — keyed on the calling workflow as well as the ref, so a gate run is not cancelled by the standalone run of the same commit. ## What to decide when picking this up - **Gate both apps or one?** The storefront is a static export and cheap to roll back; the management container is not. - **Which checks are load-bearing?** Blocking a release on `test` is uncontroversial. Blocking it on the lint gate means a stale baseline stops a deploy — see reason 1. Gating on `needs: checks` takes all six jobs; a narrower `needs` on just the test jobs is possible and probably right. - **Do #79 and the baseline-staleness risk need closing first?** Reviewer80r2's position, which I share: land the gate's own hardening before putting it on the release path. ## Related - #80 added the workflow and the gate, and corrected a comment that claimed these deploy-side `checks` jobs already existed when they never did. - #79 documents the `workflow_dispatch` deploy noise. - #73 asked for this decision to be recorded rather than merely made.

yuris referenced this issue from a commit

2026-09-28 20:56:49 +00:00

ci: gate production deploy workflows on checks (#85)

yuris referenced this issue from a pull request that will close it,

2026-09-28 20:56:53 +00:00

ci: gate production deploy workflows on checks #122

yuris closed this issue

2026-09-28 20:57:17 +00:00

yuris referenced this issue from a commit

2026-09-28 20:57:19 +00:00

ci: gate production deploy workflows on checks (#85) (#122)