Skip to content

feat(audit): PS-224 — CI runner destination must exist in the machine registry (E, day one)#438

Open
ywatanabe1989 wants to merge 2 commits into
developfrom
feat/ps224-runner-destination-registry
Open

feat(audit): PS-224 — CI runner destination must exist in the machine registry (E, day one)#438
ywatanabe1989 wants to merge 2 commits into
developfrom
feat/ps224-runner-destination-registry

Conversation

@ywatanabe1989

Copy link
Copy Markdown
Collaborator

What

PS-224 — every GitHub Actions job's runs-on must name a destination the
scitex-dev machine registry actually serves. A mismatch is an ERROR (E),
immediately (operator design, 2026-07-24 — 「食い違いがあったらすぐエラー、それで終わり」).

The registry gap this closes first

scitex_dev.hosts (backed by ~/.scitex/dev/hosts.yaml) is the fleet's single
registry of machines. Before this PR its schema was kind / ssh_alias /
scitex_roothosts only, no runner labels at all. There was no data a
runner-destination rule could validate against, so closing that gap is the
first deliverable, not an afterthought.

Each machine now records runner_labels: — a list of label SETS, one per
self-hosted runner it serves:

  spartan:
    kind: hpc-login
    runner_labels:
      - [self-hosted, Linux, X64, spartan-cpu]
      - [self-hosted, Linux, X64, spartan-cpu, scitex-ci]

Those two sets are measured, not guessed: they are what
gh api orgs/scitex-ai/actions/runners and the repo-level equivalent report for
the four online runners (2026-07-24). Recording the effective set matters —
GitHub auto-assigns self-hosted / Linux / X64 on top of the --labels
passed at registration, so recording only the --labels half would make every
workflow naming Linux/X64 look unserved.

Label sets are per-runner rather than one flat per-machine union, because a
union would green-light a combination no single runner offers.

New public surface: HostRecord.runner_labels, HostRecord.serves(),
list_runner_destinations(), find_runner_host().

The failure this kills

Three scheduled runs (scitex-io, scitex-hub, scitex-writer) sat UNDISPATCHED
since 2026-05-15
updated_at == created_at — because they requested a label
set no machine serves, while 17 runners sat online and idle. GitHub does not
reject an unmatchable job; it queues it forever, so nothing ever failed and
the workflow merely looked slow for months.

The rule is static: it fires pre-merge from the workflow YAML plus the
registry file. No Actions API call, no dispatch-time router, no capacity
dependency. Under this rule those three workflows could never have merged.

What is flagged

  1. Unserved destination — labels resolve, no registered machine advertises
    all of them. GitHub-hosted images (ubuntu-latest & co.) fall out of this
    with no special case: they are in no machine's runner_labels by construction.
  2. Unresolvable destination — a bare ${{ vars.RUNNER }} / ${{ matrix.os }}
    with no literal to read. The mandate is that every workflow names its
    destination; one that cannot be read statically names none. Without this,
    wrapping any label in a variable is a universal bypass. (Deliberately
    different from PS-169, which leaves unresolvable runners alone.)
  3. Unparseable workflow — reported, not skipped. A check that could not run
    must never report what a check that passed reports.

runs-on forms handled: bare string, list, {labels: [...]} mapping, and the
fleet idiom ${{ fromJSON(vars.CI_RUNS_ON || '["self-hosted",...]') }} — for
which the literal fallback inside the expression is validated. That fallback
is what actually runs whenever CI_RUNS_ON is unset, so it is the real
destination, not a decoration.

Registry-gap guard: if the registry records no runner destinations at all,
one finding names the registry file and the per-job noise is suppressed — an
honest "I could not check this", not a green, and not a fleet-wide red for a
reason that has nothing to do with workflows.

Known static boundary: a job with no runs-on is a uses: job delegating
to a reusable workflow, which carries its destination in its own file. A repo
can therefore inherit a destination from e.g. scitex-ai/.github that its own
YAML never mentions, and no string search of this repo can see it. Auditing the
called repo covers that; this rule does not.

Severity E, day one — no softeners

Three were considered and are explicitly rejected; they are named in the
module docstring so they are not reintroduced under another name:

  • No new-vs-old ratchet. 「昔だろうが今だろうが問題点は問題点」. It is also
    structurally broken: the baseline runs the current rules against the old
    tree, so any newly added rule flags the old tree too, every hit classifies as
    pre-existing, and the new rule is suppressed forever.
  • No severity W. W never affects the exit code
    (_audit.py: exit_code = 1 if n_errors > 0 else 0) — which is exactly why
    sibling rule PS-169 fires hundreds of times fleet-wide and has never once
    failed a build.
  • No blanket suppression flag. Individual, reasoned exemptions in
    audit.exemptions only (constitution §2) — an exemption must state WHY.

A fleet-wide red is the intended outcome. 「全て赤でいいと思います。正直に赤から
始めて、レッドスタート」. Nothing in this PR exists to reduce the initial red count.

Verification

Tests — 73 new/updated passing, real tmp_path fixtures, no mocks. Every
case builds a real hosts.yaml and a real .github/workflows/ tree and passes
the registry through the check's hosts_path= file-path seam; nothing is
patched. One assert per test (STX-TQ007), Arrange/Act/Assert markers (STX-TQ002).
Full tests/scitex_dev/_cli/audit/ + tests/scitex_dev/hosts/ suite: 1514
passed
, no regressions.

Mutation proof — the flagging path was broken (if host is not None:
if True:, i.e. the check cannot flag):

result
mutated 9 failed, 13 passed — incl. test_nonexistent_machine_label_is_flagged
restored 22 passed; git status empty, proving a byte-identical restore

Positive control — a fixture workflow naming a nonexistent machine
([self-hosted, Linux, X64, ghost-machine]) against a populated registry, run
through the real audit_project entry point:

ERRO: [E] [PS-224 §1 runner-destination-unregistered] .github/workflows/ci.yml:
  job `test` targets `[self-hosted, Linux, X64, ghost-machine]`, which NO
  registered machine serves ...
EXIT CODE: 1

Discriminating control — exit 1 alone did not prove PS-224 reached the exit
path, since 7 other PS rules also emitted E on that fixture. Re-run filtered to
rules={'PS-224'}, flipping only the destination:

arm destination PS-224-only exit code
bad [self-hosted, Linux, X64, ghost-machine] 1
good [self-hosted, Linux, X64, scitex-ci] 0

Interpreter: /opt/venv-sac/bin/python3 with PYTHONPATH=src, verified by
printing scitex_dev.__file__ inside the run — the default import resolves to
the installed copy in site-packages, which would have graded code this branch
never touched.

Operator action needed

The live ~/.scitex/dev/hosts.yaml predates this schema and carries no
runner_labels:. Until spartan's block is added there, PS-224 reports the
registry-gap finding rather than per-workflow findings. The seeded default
(for fresh installs) already contains the measured values; the existing file is
user state and this PR does not mutate it.

… registry

WIP: registry schema + check + wiring. Tests follow.

scitex-dev owns the single registry of machines (scitex_dev.hosts, backed
by ~/.scitex/dev/hosts.yaml). Until now that registry recorded hosts only
— kind / ssh_alias / scitex_root — and NO runner labels, so there was no
data a runner-destination rule could validate against. That gap is closed
first: each machine now records runner_labels:, a list of label SETS, one
per self-hosted GitHub Actions runner it serves.

PS-224 then reads every .github/workflows/*.y{a,}ml, resolves each job's
runs-on (bare string, list, labels:/group: mapping, and the fleet idiom
${{ fromJSON(vars.CI_RUNS_ON || '[...]') }} via its literal fallback),
and reports E when no registered machine serves the requested label set.

Severity E on day one — no ratchet, no bake-in W, no blanket suppression.
W cannot fail a build (exit_code = 1 if n_errors > 0), and a new-vs-
baseline ratchet is self-defeating for a NEW rule since the baseline runs
the current rules against the old tree, so every first-day hit classifies
as pre-existing. A fleet-wide red is the intended honest measurement.
…, no mocks

51 registry tests + 22 PS-224 tests. Every case builds a real hosts.yaml
and a real .github/workflows tree under tmp_path and passes the registry
through the check's hosts_path= file-path seam; nothing is patched.

One assert per test (STX-TQ007), Arrange/Act/Assert markers (STX-TQ002).

Covers: registered list/mapping/fleet-idiom destinations pass; a label set
no machine serves is flagged E; labels split across two runners are flagged
(a flat per-machine union would wrongly pass); an unresolvable expression
is flagged (otherwise a variable is a universal bypass); an unparseable
workflow is flagged rather than skipped; and an unpopulated registry
reports the registry gap ONCE instead of one finding per job.

Also updates test_to_dict_round_trips_all_fields for the widened schema.
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