From 2fee3f0b9403cff23654aff99364623fbb3312f6 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 25 Jul 2026 11:53:35 +0200 Subject: [PATCH] docs(592): record that a `skipped` CI context is not red [decisions-edit] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amends ci.monitor-armed-at-pr-open (prose + Signals only; heading and key unchanged, no supersession) with the monitor-classification rules that were missing. Landing #436/#583 today, my CI monitor filtered per-context statuses on `!= "success"` and announced "NOT all green" on two fully green PRs, because `Build & push image (amd64)` reports `skipped`. Nothing was blocked — Gitea's combined /status already treats skipped as non-blocking and reported overall=success — but a false red costs a diagnosis cycle every time. Two corrections recorded: - The image job is skipped on EVERY PR (job-level `if: github.event_name != 'pull_request'`; images build only on push-to-main and tags), NOT because of the docs-only mechanism. Misattributing it to docs-only is a plausible-sounding wrong diagnosis, since docs-only gates STEPS precisely so required jobs still report success. decisions.md already stated the fact from the branch-protection angle; the monitor-authoring consequence was missing. - skipped / failure / cancelled are three distinct meanings and must not be collapsed. Prefer gating on the combined `.state`. The documented filter is verified in BOTH directions: silent on a green PR carrying a skipped build, and still dirty on a genuinely pending run. My first draft of it was itself broken — `select(.status != …)` after the pipeline had renamed `.status` to `.st`, so it compared against null and reported a green PR as nine failures. That failure is recorded in the note, per the "verify your detector" rule. fixes #592 --- docs/decisions/workflow-process.md | 38 ++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/docs/decisions/workflow-process.md b/docs/decisions/workflow-process.md index 672eb3ce3..f38fb2b74 100644 --- a/docs/decisions/workflow-process.md +++ b/docs/decisions/workflow-process.md @@ -128,13 +128,47 @@ rule did not turn it green, which is the proof that the red was never about the `key: ci.monitor-armed-at-pr-open` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none` **Rule:** Arm a CI monitor on the PR head sha the moment the PR opens, polling the commit-status endpoint — not at the end of the work. -**Signals:** arm monitor at PR open · commit status endpoint · head sha · red run sat unnoticed · MySQL-apply flake · paths: n/a · issues: #542, #216 -**Mechanics:** `GET /api/v1/repos/timothy/ersatztv/commits/{sha}/status`. +**Signals:** arm monitor at PR open · commit status endpoint · head sha · red run sat unnoticed · MySQL-apply flake · `skipped` misread as red · `Build & push image (amd64)` skipped · monitor classification · `!= "success"` filter · combined `.state` · paths: n/a · issues: #542, #216, #583 +**Mechanics:** `GET /api/v1/repos/timothy/ersatztv/commits/{sha}/status` for the combined verdict (`.state`); `…/statuses?limit=50` only when you need per-context detail. CI runs concurrently with the review and E2E work that follows a push, so the cost of a late-armed monitor is entirely wasted wall-clock. In the #216 session three PR runs sat red for roughly an hour on a CI-only flake while review and E2E ran to completion — the reds were only discovered afterwards. +**Classify per-context states correctly, or prefer the combined `.state`.** A monitor that enumerates +contexts and treats anything `!= "success"` as red is WRONG on this repo: `Build & push image (amd64)` +is `if:`-gated at the JOB level on `github.event_name != 'pull_request'`, so it reports **`skipped` on +every PR**, by design and regardless of content (images are built only on push-to-main and tags — see +`ci.docs-only-skip-steps`, which records the same fact from the branch-protection angle: "`build` is +`skipped` on every PR"). +Such a monitor cries "NOT all green" on a perfectly green PR. Note this is *not* the docs-only skip: +the docs-only mechanism deliberately gates individual STEPS so required jobs still report `success` in +seconds — misattributing the image job's skip to docs-only is a plausible-sounding wrong diagnosis +(#583 session, 2026-07-25). + +Three distinct non-`success` states, three meanings — do not collapse them: +- **`skipped`** — deliberately not applicable. Settled, and *not* red. Gitea's combined `.state` already + treats it as non-blocking (a PR with a skipped `build` reports `overall=success`), which is why the + combined endpoint is the safer thing to gate on. +- **`failure`** — a real red; diagnose it (but first check `ci.killed-job-triage` and + `ci.infra-shaped-red-under-load`). +- **`cancelled`** — no verdict at all; see `ci.cancelled-is-not-a-verdict`. + +Working filter when you do enumerate — verified silent on a green PR carrying a skipped `build`, and +verified to still report a genuinely unfinished run (i.e. proven able to go dirty, per +`process.bom-format-detection-recipe`'s "verify your detector" rule): + +```bash +curl -s -u "$ETV_GITEA_BASICAUTH" ".../commits/$SHA/statuses?limit=50" \ + | jq -r '[.[]|{c:.context,st:.status}]|group_by(.c)|map(.[0])|.[] + |select(.st!="success" and .st!="skipped")|"NOT-GREEN: \(.c) = \(.st)"' +``` + +**Mind the renamed key.** The first draft of this snippet said `select(.status != …)` after the +pipeline had already renamed `.status` to `.st`, so the comparison ran against `null`, passed +*everything*, and reported a fully green PR as nine failures. Report `failure` and `cancelled` in +separate counts. + Related context for interpreting an early red: the old MySQL host-port 3306 collision is fixed on main (`ef8915f1`), so a lone MySQL-apply red now indicates the known infra flake and warrants a rerun rather than diagnosis.