sixfold-space/madtea

SHA256

Fork 0

ci: fourteen jobs on one runner re-do what the gate script already does in parallel, and the two lists have drifted apart #362

New issue

Closed

opened 2026-07-27 20:08:00 +00:00 by hexajon · 1 comment

hexajon commented

2026-07-27 20:08:00 +00:00

(Migrated from codeberg.org)

Noticed this while looking at why gofix is a remote runner job. It is, and the answer generalises past gofix.

Most of CI cannot fail if the local gate was green

gofix runs in two places: scripts/gate.sh:142 and .forgejo/workflows/ci.yml:175. It is a pure function of the source plus the toolchain, so a clean local gate on a commit mathematically guarantees the CI job on that same commit is green. That holds for 11 of the 14 jobs: vet, gofmt, gofix, build, tidy, staticcheck, modernize, gosec, docs, third-party-licenses, hook-tests.

The three that genuinely add signal a local run cannot: test (-race is scheduler-dependent, so a second run on other hardware is new information), mcp-conformance (drives a real stdio server against the external SDK client), and govulncheck - though see below, it is scheduled wrong.

The cost is worse than the duplication, because there is one runner

docs/contributing/ci.md:21 - CI is served by a single org-level runner. So the 14 jobs do not run in parallel, they queue, and each one pays a container start plus its own git init --object-format=sha256 and shallow fetch before doing any work.

Meanwhile gate.sh deliberately runs its 8 heavy analyzers concurrently, "roughly an order of magnitude less wall-clock". So the current shape discards the parallelism the gate already implements and pays 14 checkouts for the privilege. Fork PRs get no cache mount either, so those 14 starts are all cold.

The mirror has drifted, in both directions

ci.yml's header says a green CI run "means the same thing as a clean local checkout". It does not, either way round:


In CI, not in `gate.sh`	`third-party-licenses`
In `gate.sh`, not in CI	`adr-check`, `cross-compile`, `check-go-version`

check-go-version is the one that stings. ci.yml:53 sets GOTOOLCHAIN: local specifically so that "scripts/check-go-version.sh (the gate's toolchain-honesty step) provably compares go.mod against the toolchain that actually ran" - the precondition is set up and documented, and then no job ever runs the check it was set up for. A drifted image digest is supposed to fail that check loudly; today nothing would catch it.

This is the predictable end state of keeping two hand-maintained lists of the same checks.

`govulncheck` is scheduled wrong in the other direction

It is the only check whose answer changes without the code changing, since the vulnerability database moves under a static commit. Running it per-push means a stale answer between pushes and no answer at all during a quiet week. It belongs on the daily cron in drift.yml, which already exists.

What I want to do

Collapse ci.yml to two jobs and let the gate script be the single definition of the check set:

gate - runs ./scripts/gate.sh, which already covers everything above and parallelises the heavy half itself
mcp-conformance - stays its own job, because it needs node and the golang image has none (today it installs a pinned tarball for that job alone, and folding it in would make every run download node)
govulncheck moves to the daily cron in drift.yml
third-party-licenses folds into gate.sh so the local run stops being the weaker one

That makes "green CI == clean local gate" true by construction instead of by two lists someone has to keep in step, which is what failed here.

Needs a decision before I build it, because there are real trade-offs:

Per-check granularity in the forge UI goes away. A failure reads "gate" rather than "staticcheck", though the script prints a per-step tick/cross and tails the failing log, so the run output still names the step.
The cheap half of gate.sh is fail-fast, so a formatting error would stop the run before the analyzers instead of reporting everything broken at once. The heavy half already runs all 8 and reports each.
gate.sh would need a flag to skip conformance for the CI job.

The alternatives, if the above is too much: just fix the drift (add the 3 missing checks to CI, add third-party-licenses to the gate, keep 14 jobs), or drop the pure duplicates from CI outright and accept that an outside contributor who never ran the gate gets no formatting or vet enforcement on their PR.

Acceptance criteria

Exactly one definition of the check set; adding a check in one place cannot leave the other behind.
adr-check, cross-compile and check-go-version run in CI.
third-party-licenses runs locally.
govulncheck runs on a schedule, not only per-push.
A failing step is still identifiable by name from the run output.

Noticed this while looking at why `gofix` is a remote runner job. It is, and the answer generalises past `gofix`. ## Most of CI cannot fail if the local gate was green `gofix` runs in two places: `scripts/gate.sh:142` and `.forgejo/workflows/ci.yml:175`. It is a pure function of the source plus the toolchain, so a clean local gate on a commit mathematically guarantees the CI job on that same commit is green. That holds for 11 of the 14 jobs: `vet`, `gofmt`, `gofix`, `build`, `tidy`, `staticcheck`, `modernize`, `gosec`, `docs`, `third-party-licenses`, `hook-tests`. The three that genuinely add signal a local run cannot: `test` (`-race` is scheduler-dependent, so a second run on other hardware is new information), `mcp-conformance` (drives a real stdio server against the external SDK client), and `govulncheck` - though see below, it is scheduled wrong. ## The cost is worse than the duplication, because there is one runner `docs/contributing/ci.md:21` - CI is served by a single org-level runner. So the 14 jobs do not run in parallel, they queue, and each one pays a container start plus its own `git init --object-format=sha256` and shallow fetch before doing any work. Meanwhile `gate.sh` deliberately runs its 8 heavy analyzers concurrently, "roughly an order of magnitude less wall-clock". So the current shape discards the parallelism the gate already implements and pays 14 checkouts for the privilege. Fork PRs get no cache mount either, so those 14 starts are all cold. ## The mirror has drifted, in both directions `ci.yml`'s header says a green CI run "means the same thing as a clean local checkout". It does not, either way round: | | | |---|---| | In CI, not in `gate.sh` | `third-party-licenses` | | In `gate.sh`, not in CI | `adr-check`, `cross-compile`, `check-go-version` | `check-go-version` is the one that stings. `ci.yml:53` sets `GOTOOLCHAIN: local` specifically so that "scripts/check-go-version.sh (the gate's toolchain-honesty step) provably compares go.mod against the toolchain that actually ran" - the precondition is set up and documented, and then no job ever runs the check it was set up for. A drifted image digest is supposed to fail that check loudly; today nothing would catch it. This is the predictable end state of keeping two hand-maintained lists of the same checks. ## `govulncheck` is scheduled wrong in the other direction It is the only check whose answer changes without the code changing, since the vulnerability database moves under a static commit. Running it per-push means a stale answer between pushes and no answer at all during a quiet week. It belongs on the daily cron in `drift.yml`, which already exists. ## What I want to do Collapse `ci.yml` to two jobs and let the gate script be the single definition of the check set: - `gate` - runs `./scripts/gate.sh`, which already covers everything above and parallelises the heavy half itself - `mcp-conformance` - stays its own job, because it needs node and the golang image has none (today it installs a pinned tarball for that job alone, and folding it in would make every run download node) - `govulncheck` moves to the daily cron in `drift.yml` - `third-party-licenses` folds into `gate.sh` so the local run stops being the weaker one That makes "green CI == clean local gate" true by construction instead of by two lists someone has to keep in step, which is what failed here. Needs a decision before I build it, because there are real trade-offs: - Per-check granularity in the forge UI goes away. A failure reads "gate" rather than "staticcheck", though the script prints a per-step tick/cross and tails the failing log, so the run output still names the step. - The cheap half of `gate.sh` is fail-fast, so a formatting error would stop the run before the analyzers instead of reporting everything broken at once. The heavy half already runs all 8 and reports each. - `gate.sh` would need a flag to skip conformance for the CI job. The alternatives, if the above is too much: just fix the drift (add the 3 missing checks to CI, add `third-party-licenses` to the gate, keep 14 jobs), or drop the pure duplicates from CI outright and accept that an outside contributor who never ran the gate gets no formatting or vet enforcement on their PR. ## Acceptance criteria - Exactly one definition of the check set; adding a check in one place cannot leave the other behind. - `adr-check`, `cross-compile` and `check-go-version` run in CI. - `third-party-licenses` runs locally. - `govulncheck` runs on a schedule, not only per-push. - A failing step is still identifiable by name from the run output.

hexajon commented

2026-07-27 20:22:18 +00:00

(Migrated from codeberg.org)

Decision: collapsing to two jobs, per the plan above.

gate - runs ./scripts/gate.sh, which needs a flag to skip conformance for this job
mcp-conformance - stays separate, since it needs node and the golang image has none
govulncheck moves to the daily cron in drift.yml
third-party-licenses folds into gate.sh

Accepting the two trade-offs knowingly: a failure reads gate rather than the specific analyzer in the forge UI, and the cheap half stays fail-fast so a formatting error stops the run before the analyzers. The run output still names the failing step, and the heavy half still reports all 8 at once, so neither costs much in practice.

The point is that the check set gets one definition instead of two. check-go-version is the proof it needs one: the workflow sets GOTOOLCHAIN: local specifically so that check is meaningful, documents why, and then never runs it. That is not an oversight anyone would catch by reading either file on its own - it is only visible when you diff the two lists, which nothing does.

Unblocked; picking this up after the branch currently in flight lands.

Decision: collapsing to two jobs, per the plan above. - `gate` - runs `./scripts/gate.sh`, which needs a flag to skip conformance for this job - `mcp-conformance` - stays separate, since it needs node and the golang image has none - `govulncheck` moves to the daily cron in `drift.yml` - `third-party-licenses` folds into `gate.sh` Accepting the two trade-offs knowingly: a failure reads `gate` rather than the specific analyzer in the forge UI, and the cheap half stays fail-fast so a formatting error stops the run before the analyzers. The run output still names the failing step, and the heavy half still reports all 8 at once, so neither costs much in practice. The point is that the check set gets one definition instead of two. `check-go-version` is the proof it needs one: the workflow sets `GOTOOLCHAIN: local` specifically so that check is meaningful, documents why, and then never runs it. That is not an oversight anyone would catch by reading either file on its own - it is only visible when you diff the two lists, which nothing does. Unblocked; picking this up after the branch currently in flight lands.

hexajon referenced this issue

2026-08-17 15:52:26 +00:00

ci: Actions re-run the full local gate on every push, burning runner time #426

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea#362

No description provided.

Rows
Columns