From d0e18e08be6e8636a2fba2ef1a99f80d032d6708 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 15 Oct 2026 03:38:04 -0700 Subject: [PATCH] fix(queue-health): preserve runner routing observations --- .../runner-routing-observation-20261015.md | 38 ++++++++ scripts/ci/actions_queue_health_core.py | 28 +++++- ...est_actions_queue_health_runner_routing.py | 97 +++++++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 docs/doctoring/runner-routing-observation-20261015.md create mode 100644 tests/test_actions_queue_health_runner_routing.py diff --git a/docs/doctoring/runner-routing-observation-20261015.md b/docs/doctoring/runner-routing-observation-20261015.md new file mode 100644 index 0000000000..60602f2a4d --- /dev/null +++ b/docs/doctoring/runner-routing-observation-20261015.md @@ -0,0 +1,38 @@ +# Preserve runner routing observations + +## Source defect + +The queue-health normalizer discarded optional REST job `labels`, +`runner_group_id` and `runner_group_name`. The SLO action also described every +runner-assignment incident as GitHub-hosted, including self-hosted work. + +The repair preserves these fields in collected snapshots and JSON report rows. +Missing/null data stays unknown; explicit empty labels and zero group ID remain +explicit. Malformed values fail validation. Existing assignment, exact-head +identity, queue age, conclusion and blocker decisions are unchanged. The HTML +table is unchanged; routing details are available in the JSON report. + +## Interpretation boundary + +Labels are routing metadata, not isolation attestation. Assigned group metadata +is not the requested workflow group. A null assigned group does not prove that +a requested group is absent, offline, unauthorized or full. No capacity, +repository access, physical-host separation or isolation verdict is inferred. + +The patch is diagnostic preservation only. It neither activates runner capacity +nor completes a hosted-to-self-hosted migration. Existing operational owners +must still establish actual disposable isolation and capacity before activation. +It adds no registration, credential creation, access, network or workflow change. + +## Local verification + +The new regressions failed 11 times on unchanged production source because +routing fields were missing or malformed routing was not rejected. A setup-time +missing pytest error and an initial test import error were not counted as RED. +After repair and a collector-to-JSON regression, all 98 queue-health tests passed +normally and all 98 passed with `GITHUB_ACTIONS=true`, using Python 3.12. + +This is scoped local verification, not the full repository suite, actual runner +execution, required Checks, formal approval or protected integration. No existing +PR branch was modified. Source publication must remain separate from operational +readiness and the original migration request remains incomplete. diff --git a/scripts/ci/actions_queue_health_core.py b/scripts/ci/actions_queue_health_core.py index ab600efdbc..ddf81114ec 100644 --- a/scripts/ci/actions_queue_health_core.py +++ b/scripts/ci/actions_queue_health_core.py @@ -284,6 +284,25 @@ def _normalise_job(job: dict[str, Any]) -> dict[str, Any]: runner_id = job.get("runner_id") if isinstance(runner_id, bool) or not isinstance(runner_id, int): runner_id = 0 + # REST labels are routing selectors, not evidence of isolation or capacity. + # In particular, a null assigned group does not prove a requested group + # is absent. Preserve null separately from an explicitly empty value. + labels = job.get("labels") + if labels is not None and ( + not isinstance(labels, list) + or any(not isinstance(label, str) or not label for label in labels) + ): + raise QueueHealthError("runner labels must be an array of non-empty strings or null") + runner_group_id = job.get("runner_group_id") + if runner_group_id is not None and ( + isinstance(runner_group_id, bool) + or not isinstance(runner_group_id, int) + or runner_group_id < 0 + ): + raise QueueHealthError("runner group id must be a non-negative integer or null") + runner_group_name = job.get("runner_group_name") + if runner_group_name is not None and not isinstance(runner_group_name, str): + raise QueueHealthError("runner group name must be a string or null") if "steps" in job: workflow_steps = job["steps"] if workflow_steps is not None and not isinstance(workflow_steps, list): @@ -306,6 +325,9 @@ def _normalise_job(job: dict[str, Any]) -> dict[str, Any]: "conclusion": str(job.get("conclusion") or "").upper(), "runner_id": runner_id, "runner_name": str(job.get("runner_name") or ""), + "labels": list(labels) if labels is not None else None, + "runner_group_id": runner_group_id, + "runner_group_name": runner_group_name, "created_at": str(job.get("created_at") or ""), "steps_count": steps_count, } @@ -543,6 +565,9 @@ def build_report( "execution_state": state, "is_pending": pending, "runner_assigned": assigned, + "runner_labels": job.get("labels"), + "runner_group_id": job.get("runner_group_id"), + "runner_group_name": job.get("runner_group_name"), "created_at": run.get("created_at", ""), "updated_at": run.get("updated_at", ""), "queue_age_seconds": age_seconds, @@ -577,7 +602,7 @@ def build_report( ] external_actions = sorted( { - "Inspect GitHub-hosted runner assignment, Actions billing/usage, runner-group policy, environment approval, and concurrency saturation; queued evidence remains incomplete." + "Inspect runner assignment, Actions billing/usage, runner-group policy, environment approval, and concurrency saturation; queued evidence remains incomplete." for row in rows if row["blocker"] == "external_runner_assignment_or_capacity" } @@ -611,6 +636,7 @@ def build_report( "runs": rows, "limitations": [ "The Actions REST API does not expose the evaluated concurrency group for every run; unavailable values are reported explicitly.", + "Runner labels and assigned group metadata do not attest isolation, requested group existence, repository access, or available capacity.", "This read-only slice never cancels runs or changes branch/check state.", ], } diff --git a/tests/test_actions_queue_health_runner_routing.py b/tests/test_actions_queue_health_runner_routing.py new file mode 100644 index 0000000000..7c2f675eae --- /dev/null +++ b/tests/test_actions_queue_health_runner_routing.py @@ -0,0 +1,97 @@ +"""Keep routing observations distinct from runner and isolation authority.""" + +import json + +import pytest + +from tests.test_actions_queue_health import NOW, queue_health, report_snapshot + + +def snapshot_with_routing(**routing): + """Attach raw REST routing metadata to one current-head queued job.""" + snapshot = report_snapshot() + snapshot['repositories'][0]['runs'][0]['jobs'][0].update(routing) + return snapshot + + +def test_routing_survives_normalization_and_report_roundtrip(): + """Preserve labels and assigned group without inventing capacity or safety.""" + snapshot = snapshot_with_routing(labels=['self-hosted', 'Linux', 'cwlab-ci-isolated'], + runner_group_id=13, runner_group_name='CWL CI isolated') + repo = snapshot['repositories'][0] + repo['runs'] = [queue_health._normalise_run(repo['full_name'], run, run['jobs']) + for run in repo['runs']] + report = queue_health.build_report(json.loads(json.dumps(snapshot)), now=NOW) + row = report['runs'][0] + assert row['runner_labels'] == ['self-hosted', 'Linux', 'cwlab-ci-isolated'] + assert row['runner_group_id'] == 13 + assert row['runner_group_name'] == 'CWL CI isolated' + assert row['runner_assigned'] is False + assert row['execution_state'] == 'queued_unassigned' + assert row['blocker'] == 'external_runner_assignment_or_capacity' + assert 'GitHub-hosted runner assignment' not in ' '.join(report['summary']['external_actions']) + + +@pytest.mark.parametrize('routing', [{}, {'labels': None, 'runner_group_id': None, 'runner_group_name': None}]) +def test_missing_routing_stays_unknown(routing): + """Old snapshots and unavailable REST fields do not imply a missing group.""" + row = queue_health.build_report(snapshot_with_routing(**routing), now=NOW)['runs'][0] + assert row['runner_labels'] is None + assert row['runner_group_id'] is None + assert row['runner_group_name'] is None + + +@pytest.mark.parametrize('routing', [ + {'labels': 'self-hosted'}, {'labels': [False]}, {'labels': ['']}, + {'runner_group_id': True}, {'runner_group_id': -1}, {'runner_group_id': '13'}, + {'runner_group_name': 13}, +]) +def test_malformed_routing_is_rejected(routing): + """Reject ambiguous API fields instead of converting them to routing claims.""" + with pytest.raises(queue_health.QueueHealthError, match='runner'): + queue_health.build_report(snapshot_with_routing(**routing), now=NOW) + + +def test_empty_routing_is_preserved_without_attestation(): + """Zero/unassigned group and empty labels are observations, never a pass.""" + row = queue_health.build_report(snapshot_with_routing(labels=[], runner_group_id=0, + runner_group_name=''), now=NOW)['runs'][0] + assert row['runner_labels'] == [] + assert row['runner_group_id'] == 0 + assert row['runner_group_name'] == '' + assert not row['runner_assigned'] + + +def test_collector_keeps_raw_rest_routing_through_json(): + """Exercise the public collector-to-report route with a fake read-only API.""" + from subprocess import CompletedProcess + + snapshot = snapshot_with_routing(labels=['self-hosted', 'linux'], + runner_group_id=3, runner_group_name='control') + repo = snapshot['repositories'][0] + run = repo['runs'][0] + jobs = run.pop('jobs') + responses = { + 'repos/owner/repo': {'default_branch': 'main'}, + 'repos/owner/repo/pulls?state=open&per_page=100': repo['pull_requests'], + 'repos/owner/repo/actions/runs?status=queued&per_page=50': [run], + 'repos/owner/repo/actions/runs?status=completed&head_sha=head&per_page=50': [], + 'repos/owner/repo/actions/runs?status=cancelled&event=pull_request_target&per_page=50': [], + 'repos/owner/repo/actions/runs/10/jobs?per_page=100': {'total_count': len(jobs), 'jobs': jobs}, + } + for status in ('in_progress', 'pending', 'requested', 'waiting'): + responses[f'repos/owner/repo/actions/runs?status={status}&per_page=50'] = [] + + def runner(args, **kwargs): + """Admit only the expected repository reads without network or mutation.""" + assert args[:2] == ['gh', 'api'] + return CompletedProcess(args, 0, json.dumps(responses[args[-1]]), '') + + collected = queue_health.collect_snapshot(['owner/repo'], runner=runner, + generated_at='2026-08-19T12:00:00Z') + assert not collected['collection_errors'] + row = queue_health.build_report(json.loads(json.dumps(collected)), now=NOW)['runs'][0] + assert row['runner_labels'] == ['self-hosted', 'linux'] + assert row['runner_group_id'] == 3 + assert row['runner_group_name'] == 'control' + assert not row['runner_assigned']