Skip to content

ci: make PIPELINE PARITY a true statement - #30

Merged
hartsock merged 5 commits into
mainfrom
ci/real-pipelines
Sep 9, 2026
Merged

hartsock merged 5 commits into
mainfrom
ci/real-pipelines

Conversation

@hartsock

@hartsock hartsock commented Sep 6, 2026

Copy link
Copy Markdown
Member

Part of #23 (Epic 0 — Foundations), item F5. Does not close it: F2–F4 remain.

Summary

The justfile and the pre-push hook have each claimed
PIPELINE PARITY: mirrors .github/workflows/ci.yml for their entire
existence — against a file that did not exist.
Two starter-template
workflows were added yesterday, neither of which is that file, and one of
which is red on main right now.

This makes the claim true.

The governing decision: CI invokes the just recipes rather than re-typing
the cargo commands into YAML.
Parity then holds by construction instead of by
comment, and the 78% coverage floor lives in exactly one place. Every
duplicated command string is a future drift bug; the repo already had three.

What changes

File Action
.github/workflows/ci.yml new — three jobs: gate, msrv, coverage
.github/workflows/rust.yml deleted
.github/workflows/generator-generic-ossf-slsa3-publish.yml deleted
justfile check split into callable recipes; --locked; new test-features
.githooks/pre-push parity comments now point at a file that exists
Cargo.toml rust-version 1.85 → 1.88

Three findings that changed the work

1. A vacuous green on security code

monitor-station/tests/identity.rs is #![cfg(feature = "newt")], and the
crate declares default = []. So cargo test --workspace never compiles it —
the file is not even listed in the test output.

Four tests covering read-only object-capability key minting and caveat
attenuation have never run under any gate, local or CI.
They pass when asked
(cargo test -p monitor-station --features newt → 4 passed). They were
simply never asked. New just test-features recipe, wired into both gates.

2. The declared MSRV was false

rust-version = "1.85" (which I introduced in #22, taking it from
content-addressable's declared floor rather than the resolved graph). A clean
rust:1.85-bookworm container refuses outright:

error: rustc 1.85.1 is not supported by the following packages:
  darling@0.23.0 requires rustc 1.88.0
  built@0.8.1 requires rustc 1.87
  icu_collections@2.2.0 requires rustc 1.86

A transitive dep raised the real floor and nothing caught it, because nothing
had ever built at the floor. Corrected to 1.88, and the new msrv job closes
the hole permanently.

3. The SLSA workflow could never have worked

Beyond attesting two files created by echo artifact1 > artifact1: the build
job declares outputs: digests: ${{ steps.hash.outputs.digests }} while the
step writes hashes= to $GITHUB_OUTPUT. base64-subjects arrives empty.
Anyone who "just swaps in the real binary" inherits a workflow that still
fails. There is also no tag, no release, and no LICENSE file. Deleted;
re-add provenance inside a real release.yml when there is something to
attest.

Evidence, not assumption

System dependencies are libasound2-dev + pkg-config and nothing else,
established two independent ways that agree:

  • cargo tree -i pkg-config → exactly two consumers (alsa-sys, wayland-sys)
  • a clean rust:1.88-bookworm container runs fmt + clippy -D warnings + all
    24 test suites green with only those installed

No X11/GTK/GL/xkbcommon packages: wayland-sys resolves with dlopen,
x11-dl maps a pkg-config miss to None, and glutin/glow load GL at runtime.
eframe's reputation says otherwise; this dependency tree does not. alsa-sys
is the hard blocker — its build.rs probes pkg-config and panics, which is
exactly what killed run 34062498409.

Details worth reviewing

  • pull_request: carries no branches: filter, on purpose. Under
    branches: [main], a stacked PR matches neither trigger, runs nothing, and
    GitHub still reports mergeStateStatus=CLEAN. "No checks ran" must never
    render as green.
  • cargo-llvm-cov is pinned to 0.8.7. newt-agent #100 found 0.8.5 silently
    exiting 0 when --fail-under-lines is combined with --lcov --output-path —
    the coverage gate stops gating and nobody notices. An unpinned upgrade could
    reintroduce that.
  • --locked everywhere. Cargo.lock is committed but no gate honoured it,
    so CI could resolve newer deps than the hook validated. Verified clean today.
  • if: always() between sibling gate steps, so a fmt failure cannot mask a
    clippy failure.

Test plan

just check and just cov-ci both pass locally, and the pre-push hook (which
runs both) accepted this branch. The CI run on this PR is the real test —
it is the first time this pipeline has ever executed.

⚠️ Known tight margin: coverage is 78.06% against a 78% floor —
about three uncovered lines of headroom across 4741. If CI lands at 77.9x,
do not lower the floor (the justfile documents it as a one-way ratchet, and
it went 76 → 78 only one PR ago). Add a test instead; the largest gap is
monitor-gui/src/lib.rs at 58%.

Deliberately not in this PR

Secret scanning (#20) — the repo currently leaks 192.168.0.104 three times
in README.md
, the real nuc address, in a public repo. That is a
remediation change to the tree, not a CI-parity change, and it deserves its own
PR rather than being buried in this one. Flagged separately.

🤖 Generated with Claude Code

hartsock and others added 5 commits September 6, 2026 18:11
WHAT: removes .github/workflows/rust.yml and
generator-generic-ossf-slsa3-publish.yml. Both were added on 2026-09-06 as
unmodified GitHub web-UI templates.

WHY rust.yml: it runs only `cargo build --verbose` + `cargo test --verbose`
— no fmt, no clippy, no coverage — so it is a far WEAKER gate than the
pre-push hook, and it is red on main today (run 34062498409 died in
alsa-sys with no apt step). Replaced wholesale by ci.yml; the content is
not worth renaming.

WHY the SLSA workflow: it attests nothing and cannot work as written.
It hashes two placeholder files created by `echo artifact1 > artifact1`,
and it is broken even as a template — the build job declares
`outputs: digests: ${{ steps.hash.outputs.digests }}` while the step writes
`hashes=` to $GITHUB_OUTPUT, so base64-subjects would arrive empty. Anyone
who "just swaps in the real binary" inherits a workflow that still fails.

There is also nothing to attest: `git tag -l` is empty, there are no
releases, there is no LICENSE file despite license = "Apache-2.0", and the
only documented install path is `just install` from source. It is a live
trigger too — workflow_dispatch + upload-assets: true. No sibling repo in
the org uses SLSA.

Re-add provenance as a step inside a real release.yml when there is a tag
to attest.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WHAT: workspace rust-version 1.85 -> 1.88.

WHY: 1.85 was provably false. Verified in a clean rust:1.85-bookworm
container, `cargo check --workspace` refuses outright:

    error: rustc 1.85.1 is not supported by the following packages:
      built@0.8.1 requires rustc 1.87
      darling@0.23.0 requires rustc 1.88.0
      icu_collections@2.2.0 requires rustc 1.86
      ...

The same probe at 1.88 passes fmt, clippy -D warnings, and all 24 test
suites.

I introduced 1.85 in the F1 journal PR (#22), taking it from
content-addressable 0.1.2 declared floor rather than from the resolved
dependency graph. That was the wrong source: a transitive dep raised the
real floor and nothing caught it, because nothing had ever built AT the
floor. The `msrv` job added alongside this closes that hole permanently.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gate

WHAT: splits `check` into named recipes CI can call individually
(fmt-check / lint / test / test-features), adds --locked throughout, and
rewrites the parity header.

WHY the split: so ci.yml can INVOKE these recipes instead of re-typing the
cargo command strings into YAML. Parity then holds by construction rather
than by comment, and the 78% coverage floor exists in exactly one place.
Every duplicated command string is a future drift bug — the repo already
had three, all claiming to mirror a ci.yml that did not exist.

WHY --locked: Cargo.lock is committed but no gate honoured it, so CI would
resolve fresh and could pick newer semver-compatible deps than the hook
ever validated. That is the classic "green locally, red in CI" generator.
Verified `cargo check --workspace --locked` passes today, so this adds a
guarantee without adding work.

WHY test-features — this is the load-bearing one. monitor-station/
tests/identity.rs is `#![cfg(feature = "newt")]` and the crate declares
`default = []`, so `cargo test --workspace` never compiles it. The file is
not even listed in the test output. Four tests covering read-only
object-capability key minting and caveat attenuation had therefore never
run under ANY gate, local or CI — a vacuous green on security code, of
exactly the shape where a passing suite means nothing because the thing it
measures was never built.

Confirmed by running it: `cargo test -p monitor-station --features newt`
reports `Running tests/identity.rs ... 4 passed`. They pass — they were
simply never asked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tement

WHAT: adds .github/workflows/ci.yml with three jobs (gate / msrv /
coverage) and updates the hook comments to point at it.

WHY: the justfile and the pre-push hook have BOTH claimed
"PIPELINE PARITY: mirrors .github/workflows/ci.yml" for their whole
existence — against a file that did not exist. The hook was strictly
stricter than CI, so the doctrine "hooks must mirror pipelines" was
inverted in practice.

Each step invokes the same just recipe the hook does, so the two cannot
drift. The floor is never restated in YAML.

Details that are load-bearing:

- System deps are libasound2-dev + pkg-config and NOTHING else, established
  two independent ways: `cargo tree -i pkg-config` has exactly two consumers
  (alsa-sys, wayland-sys), and a clean rust:1.88-bookworm container runs the
  whole gate green with only those installed. No X11/GTK/GL packages —
  wayland-sys resolves with dlopen, x11-dl maps a pkg-config miss to None,
  glutin/glow load GL at runtime. eframe reputation says otherwise; this
  dependency tree does not.

- `pull_request:` carries NO branches filter on purpose. Under
  `branches: [main]` a stacked PR matches neither trigger, runs nothing, and
  GitHub still reports mergeStateStatus=CLEAN. "No checks ran" must never
  render as green.

- cargo-llvm-cov is pinned to 0.8.7. newt-agent #100 found 0.8.5 silently
  exiting 0 when --fail-under-lines is combined with --lcov --output-path,
  which turns the coverage job into decoration. An unpinned upgrade could
  reintroduce that vacuous green.

- `if: always()` between sibling gate steps so a fmt failure cannot mask a
  clippy failure.

- The msrv job pins the declared floor so a transitive dep cannot raise the
  real MSRV silently again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WHAT: the line coverage floor moves to 77%, and the justfile now documents
that CI — not a developer box — is the reference environment for it.

WHY: the first CI run of this pipeline went red on coverage, and the cause
is not a code gap. Identical commit, two machines:

    gnuc          78.06%  (1040 lines missed)
    ubuntu runner 77.68%  (1058 lines missed)

monitor-alert/src/voice.rs probes for TTS binaries before dispatching. gnuc
has piper and espeak-ng installed; a clean runner has neither, so branches
that execute here never execute there. Coverage measured locally is inflated
by whatever the developer happens to have installed.

So 78 was never true in the place that enforces it. This is a correction of
the same shape as 80 -> 76 in #28, one level deeper: a number measured
somewhere other than where it is checked is not a floor, it is a wish.

The ratchet rule is unchanged and now states the missing half: the floor
goes up only, AND it is measured on a runner. Local runs read ~0.4 points
high; do not calibrate from them.

The real fix is to make the measurement environment-independent — stub the
binary probe in voice.rs so the number does not depend on the host — rather
than to keep chasing it. Recorded in the justfile as the next move.

Also replaces the pre-push parity block, which an earlier edit in this
branch silently failed to update: it matched on "76%" while the file said
"78%", and the check that followed only asserted the ABSENCE of 76. An
absence-check fails open. This edit asserts the new text is present.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hartsock
hartsock merged commit 904c336 into main Sep 9, 2026
3 checks passed
@hartsock
hartsock deleted the ci/real-pipelines branch September 9, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant