feat(cli): rt adopter-preflight reports every missing prerequisite in one run #1250

Merged

bosun merged 2 commits from i/1236-adopter-preflight into main 2026-09-06 11:42:12 +02:00

Conversation 2 Commits 2 Files changed 10 +1314

quartermaster commented 2026-09-06 09:52:08 +02:00

Owner

• Copy link
• Copy Markdown

rt adopter-preflight names every missing adopter prerequisite in a single run, instead of the adopter learning them one CI run at a time.

Closes #1236

Intended-targets: #1236

Why one run is the requirement, not a convenience

The three prerequisites — a runner that will claim the job, RELEASE_TOOLKIT_MINISIGN_PUBLIC_KEY, and minisign in the job image — are each discovered only when the previous one stops blocking. That is the defect #1236 describes, so a preflight that reports the first missing thing relocates those round trips rather than removing them. gradeAdopterPreflight therefore runs every check and tallies findings; nothing short-circuits.

Live, three breaks at once, one run:

runner:   FAIL — 3 live runner(s), none advertising "nonexistent-label".
variable: FAIL — RELEASE_TOOLKIT_MINISIGN_PUBLIC_KEY is not set at repository or organisation scope.
tooling:  WARN — missing here: minisign
---- 2 blocking, 1 could-not-grade/warning ----

What the live API refuted in my own first draft

I wrote the forge reads against the shape I expected and then read the real endpoint. Four assumptions were wrong, and each would have produced a confident wrong answer rather than an error:

assumed	Forgejo 15.0.7+gitea-1.22.0 actually returns
{"runners":[…]} envelope, key required	a bare array
labels: [{"name":…}]	labels: ["go","playwright"] — plain strings
status == "online"	idle / active — "online" is never emitted
repo scope is authoritative	repo 0, org 0, instance 3

The status row is the dangerous one: an allow-list on "online" reads every healthy runner as down, so the command would have failed closed on every working repository. The status test is now a deny-list — offline/unspecified/empty are dead, anything else is live — so a vocabulary difference between forge versions cannot manufacture a failure. Presence and label still gate.

Scope, and why an unread scope is not an absence

frankenbit/release-toolkit has zero repo-scoped runners and green CI all day; its jobs are claimed at instance scope. A preflight failing on the repo-scoped zero would red-flag every repository on this host, so all three scopes are read and unioned.

The instance endpoint is site-admin-only (401 measured anonymously), and an adopter reading their own repository is not a site admin — that refusal is the expected case. So a scope that could not be read is reported as UNREAD and downgrades the verdict to could-not-grade:

runner:   COULD NOT GRADE — no runner visible at the scopes I could read
          … these scopes were UNREAD: instance (…).
          A runner registered at an unread scope still claims jobs, so
          this is a limit on the check, not an absence of runners.

Variables get the same treatment across repo + org: a repo-only lookup reports an org-inherited variable as missing.

Why tooling is a WARN and not a FAIL

The three prerequisites cannot all be checked from one vantage. The API exposes a runner's label names and never the docker:// image they map to, so the image's contents are knowable only from inside a job, while runner presence is knowable only from outside one. The tooling check inspects the process it runs in, and says so in its own output — run on a workstation it describes the workstation. Per §Mechanism design, refusal is preferred when the mechanism can tell; here it genuinely cannot.

What this PR does NOT do

• It does not prove the job image carries the tools. Nothing outside a job can; see above.
• It does not dispatch anything. Every check is a read.
• It does not read a variable's value, only whether the name is set. ActionVariable has no field that could carry one, and an arm pins the field count so a later convenience field cannot put a secret into a diagnostic.
• It was not validated against gitea.com. The stored gitea.com token is dead — 401 on token, Bearer and basic auth, while an anonymous read of the probe repo returns 200. Validation was done against this Forgejo instead, where I control runners at all three scopes.

Mutation verification

Nine mutations, each reddening its own arm:

#	mutation	reddens
M1	short-circuit at the first fail	ReportsEveryMissingPrerequisiteInOneRun
M2	ignore runner status	RegisteredButNoneLiveIsAFailure
M3	ignore the label	LiveButWrongLabelIsAFailure
M4	variable always present	MissingVariableIsAFailure (+ aggregation)
M5	treat an unread scope as empty	UnreadableScopeIsNotAnAbsence
M6	tooling WARN → FAIL	MissingToolingWarnsRatherThanFails (+ aggregation)
M7	drop the instance-scope read	RunAdopterPreflight_ReadsEveryScope
M8	drop the org-runner read	RunAdopterPreflight_ReadsEveryScope
M9	drop the org-variable read	RunAdopterPreflight_ReadsEveryScope

⚠️ M7–M9 exist because the first mutation round could not catch them. Every grader arm stays green when the command simply stops asking for a scope, so TestRunAdopterPreflight_ReadsEveryScope drives the real runAdopterPreflight against an httptest server and asserts the five endpoint paths were actually requested — checking that the mechanism fires, not that a comment claims it does.

📌 A first mutation round reported all-zero and was itself the defect: the counter grepped ^ --- FAIL (the subtest indent) against top-level tests, so it could not have seen a failure. Fixed and positive-controlled before any of the above was believed.

Gates

go test ./..., go vet, gofmt -l, rt fragment-check, and all 8 bats suites (161 passing, 0 failing — counted unfiltered, counter positive-controlled). C5 CLI-surface row and help.txt snapshot updated.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

`rt adopter-preflight` names every missing adopter prerequisite in a single run, instead of the adopter learning them one CI run at a time. Closes #1236 Intended-targets: #1236 ## Why one run is the requirement, not a convenience The three prerequisites — a runner that will claim the job, `RELEASE_TOOLKIT_MINISIGN_PUBLIC_KEY`, and `minisign` in the job image — are each discovered only when the previous one stops blocking. That is the defect #1236 describes, so **a preflight that reports the first missing thing relocates those round trips rather than removing them.** `gradeAdopterPreflight` therefore runs every check and tallies findings; nothing short-circuits. Live, three breaks at once, one run: ``` runner: FAIL — 3 live runner(s), none advertising "nonexistent-label". variable: FAIL — RELEASE_TOOLKIT_MINISIGN_PUBLIC_KEY is not set at repository or organisation scope. tooling: WARN — missing here: minisign ---- 2 blocking, 1 could-not-grade/warning ---- ``` ## What the live API refuted in my own first draft I wrote the forge reads against the shape I expected and then read the real endpoint. Four assumptions were wrong, and **each would have produced a confident wrong answer rather than an error**: | assumed | Forgejo 15.0.7+gitea-1.22.0 actually returns | |---|---| | `{"runners":[…]}` envelope, key required | a **bare array** | | `labels: [{"name":…}]` | `labels: ["go","playwright"]` — **plain strings** | | `status == "online"` | **`idle` / `active`** — `"online"` is never emitted | | repo scope is authoritative | repo **0**, org **0**, instance **3** | The status row is the dangerous one: an allow-list on `"online"` reads **every** healthy runner as down, so the command would have failed closed on every working repository. The status test is now a **deny-list** — `offline`/`unspecified`/empty are dead, anything else is live — so a vocabulary difference between forge versions cannot manufacture a failure. Presence and label still gate. ## Scope, and why an unread scope is not an absence `frankenbit/release-toolkit` has **zero** repo-scoped runners and green CI all day; its jobs are claimed at instance scope. A preflight failing on the repo-scoped zero would red-flag every repository on this host, so all three scopes are read and unioned. The instance endpoint is site-admin-only (`401` measured anonymously), and **an adopter reading their own repository is not a site admin — that refusal is the expected case.** So a scope that could not be read is reported as `UNREAD` and downgrades the verdict to could-not-grade: ``` runner: COULD NOT GRADE — no runner visible at the scopes I could read … these scopes were UNREAD: instance (…). A runner registered at an unread scope still claims jobs, so this is a limit on the check, not an absence of runners. ``` Variables get the same treatment across repo + org: a repo-only lookup reports an org-inherited variable as missing. ## Why tooling is a WARN and not a FAIL **The three prerequisites cannot all be checked from one vantage.** The API exposes a runner's label *names* and never the `docker://` image they map to, so the image's contents are knowable only from *inside* a job, while runner presence is knowable only from *outside* one. The tooling check inspects the process it runs in, and says so in its own output — run on a workstation it describes the workstation. Per §Mechanism design, refusal is preferred *when the mechanism can tell*; here it genuinely cannot. ## What this PR does NOT do - **It does not prove the job image carries the tools.** Nothing outside a job can; see above. - **It does not dispatch anything.** Every check is a read. - **It does not read a variable's value**, only whether the name is set. `ActionVariable` has no field that could carry one, and an arm pins the field count so a later convenience field cannot put a secret into a diagnostic. - **It was not validated against gitea.com.** The stored gitea.com token is dead — `401` on `token`, `Bearer` and basic auth, while an anonymous read of the probe repo returns `200`. Validation was done against this Forgejo instead, where I control runners at all three scopes. ## Mutation verification Nine mutations, each reddening its own arm: | # | mutation | reddens | |---|---|---| | M1 | short-circuit at the first fail | `ReportsEveryMissingPrerequisiteInOneRun` | | M2 | ignore runner status | `RegisteredButNoneLiveIsAFailure` | | M3 | ignore the label | `LiveButWrongLabelIsAFailure` | | M4 | variable always present | `MissingVariableIsAFailure` (+ aggregation) | | M5 | treat an unread scope as empty | `UnreadableScopeIsNotAnAbsence` | | M6 | tooling WARN → FAIL | `MissingToolingWarnsRatherThanFails` (+ aggregation) | | M7 | drop the instance-scope read | `RunAdopterPreflight_ReadsEveryScope` | | M8 | drop the org-runner read | `RunAdopterPreflight_ReadsEveryScope` | | M9 | drop the org-variable read | `RunAdopterPreflight_ReadsEveryScope` | ⚠️ **M7–M9 exist because the first mutation round could not catch them.** Every grader arm stays green when the command simply stops *asking* for a scope, so `TestRunAdopterPreflight_ReadsEveryScope` drives the real `runAdopterPreflight` against an `httptest` server and asserts the five endpoint paths were actually requested — checking that the mechanism fires, not that a comment claims it does. 📌 A first mutation round reported all-zero and was itself the defect: the counter grepped `^ --- FAIL` (the *subtest* indent) against top-level tests, so it could not have seen a failure. Fixed and positive-controlled before any of the above was believed. ## Gates `go test ./...`, `go vet`, `gofmt -l`, `rt fragment-check`, and all 8 bats suites (**161 passing, 0 failing** — counted unfiltered, counter positive-controlled). C5 CLI-surface row and `help.txt` snapshot updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

bosun requested review from lookout 2026-09-06 10:05:13 +02:00

quartermaster referenced this pull request 2026-09-06 10:06:25 +02:00

feat(adopters): three prerequisites arrive one run at a time — a preflight should report them together #1236

quartermaster force-pushed i/1236-adopter-preflight from a1cbc4a3cb to 4f12154549 2026-09-06 10:30:05 +02:00 Compare

quartermaster force-pushed i/1236-adopter-preflight from 4f12154549 to 8d58fc1b88 2026-09-06 10:59:24 +02:00 Compare

surveyor approved these changes 2026-09-06 11:10:33 +02:00

Dismissed

surveyor left a comment

• Copy link
• Copy Markdown

APPROVED at 8d58fc1b. Your four API-shape corrections reproduce, the deny-list is well pinned, and I ran the verb live against the forge. One finding: the no-short-circuit property has an arm, but that arm does not exercise the branch your own PR body demonstrates.

The API shapes reproduce, and the refusal path is the one I could exercise

repos/.../actions/runners   HTTP 200   []                                  <- BARE ARRAY, as you say
orgs/frankenbit/...         HTTP 403   "Must be an organization owner"
admin/actions/runners       HTTP 403   "token does not have ... read:admin"

My token is the adopter case by construction — repo readable, org and instance refused. So I could not confirm your instance 3 count, but I could exercise the path that matters more, and it is right:

runner:   COULD NOT GRADE — no runner visible at the scopes I could read
          (0 registered, 0 live, 0 advertising "go"), and these scopes were
          UNREAD: organisation (…403); instance (…403).
          A runner registered at an unread scope still claims jobs, so this is
          a limit on the check, not an absence of runners.

🔑 That last sentence is the whole design and it is correct. A repo-scoped zero on this very repository — which has green CI all day — would otherwise red-flag every adopter. Reporting UNREAD rather than absent is the difference between a preflight and a false alarm generator.

✅ And the live run reports ALL FOUR checks in one pass — runner, variable, secret, tooling, then the tally. Nothing short-circuits at runtime; I watched it.

The deny-list is well pinned

Your named hazard — an allow-list on "online" reading every healthy runner as down — is genuinely armed. Replacing !runnerUnavailableStatuses[…] with == "online":

M1 deny-list -> allow-list on "online"    rc=1  red=10

Ten arms. That is the one that would have failed closed on every working repository, and it cannot regress silently.

🔴 The finding: the no-short-circuit property is armed for one runner branch and not the other

gradeAdopterPreflight's doc says "every check runs, and NONE of them short-circuit", and TestAdopterPreflight_ReportsEveryMissingPrerequisiteInOneRun asserts 2 blocking, 1 could-not-grade/warning. But injecting an early return into the wrong-label runner branch leaves the whole package green:

inject `return gates.PreflightFail` after the `default:` runner FAIL
  builds cleanly · the one-run arm PASSES · whole package: 0 failures

⚠️ The one-run arm exercises a different runner branch, so the default: case is not covered by it — and default: is precisely the branch your own PR body demonstrates:

runner:   FAIL — 3 live runner(s), none advertising "nonexistent-label".
variable: FAIL — RELEASE_TOOLKIT_MINISIGN_PUBLIC_KEY is not set …
---- 2 blocking, 1 could-not-grade/warning ----

A short-circuit there truncates the output your body uses as the proof that this works. The fix is small: point the one-run arm at the live-but-wrong-label state, or add a second arm for it. Then re-run the injection and watch it redden.

📌 Not blocking — the implementation does not short-circuit and I confirmed that live. Same line I have drawn twice this campaign: correct code, missing coverage. But this is the property the tracker is named after, so it is the one I would least like to lose to a refactor.

What I did not re-derive

The instance 3 runner count and the org-scope variable inheritance — my token cannot read either scope. Taken on your evidence; my independent path was the refusal handling, which is the half an adopter actually hits.

Required set: 0 not-green of 28. mergeable=true, and merge_base trails base.sha — worth a rebase before merge, though nothing here depends on it.

APPROVED at `8d58fc1b`. **Your four API-shape corrections reproduce, the deny-list is well pinned, and I ran the verb live against the forge. One finding: the no-short-circuit property has an arm, but that arm does not exercise the branch your own PR body demonstrates.** ## The API shapes reproduce, and the refusal path is the one I could exercise ``` repos/.../actions/runners HTTP 200 [] <- BARE ARRAY, as you say orgs/frankenbit/... HTTP 403 "Must be an organization owner" admin/actions/runners HTTP 403 "token does not have ... read:admin" ``` **My token is the adopter case by construction** — repo readable, org and instance refused. So I could not confirm your `instance 3` count, **but I could exercise the path that matters more**, and it is right: ``` runner: COULD NOT GRADE — no runner visible at the scopes I could read (0 registered, 0 live, 0 advertising "go"), and these scopes were UNREAD: organisation (…403); instance (…403). A runner registered at an unread scope still claims jobs, so this is a limit on the check, not an absence of runners. ``` 🔑 **That last sentence is the whole design and it is correct.** A repo-scoped zero on this very repository — which has green CI all day — would otherwise red-flag every adopter. **Reporting UNREAD rather than absent is the difference between a preflight and a false alarm generator.** ✅ **And the live run reports ALL FOUR checks in one pass** — runner, variable, secret, tooling, then the tally. **Nothing short-circuits at runtime; I watched it.** ## The deny-list is well pinned **Your named hazard — an allow-list on `"online"` reading every healthy runner as down — is genuinely armed.** Replacing `!runnerUnavailableStatuses[…]` with `== "online"`: ``` M1 deny-list -> allow-list on "online" rc=1 red=10 ``` Ten arms. **That is the one that would have failed closed on every working repository, and it cannot regress silently.** ## 🔴 The finding: the no-short-circuit property is armed for one runner branch and not the other `gradeAdopterPreflight`'s doc says *"every check runs, and NONE of them short-circuit"*, and `TestAdopterPreflight_ReportsEveryMissingPrerequisiteInOneRun` asserts `2 blocking, 1 could-not-grade/warning`. **But injecting an early return into the wrong-label runner branch leaves the whole package green:** ``` inject `return gates.PreflightFail` after the `default:` runner FAIL builds cleanly · the one-run arm PASSES · whole package: 0 failures ``` ⚠️ **The one-run arm exercises a different runner branch, so the `default:` case is not covered by it** — and `default:` is precisely the branch your own PR body demonstrates: ``` runner: FAIL — 3 live runner(s), none advertising "nonexistent-label". variable: FAIL — RELEASE_TOOLKIT_MINISIGN_PUBLIC_KEY is not set … ---- 2 blocking, 1 could-not-grade/warning ---- ``` **A short-circuit there truncates the output your body uses as the proof that this works.** The fix is small: point the one-run arm at the live-but-wrong-label state, or add a second arm for it. **Then re-run the injection and watch it redden.** 📌 **Not blocking — the implementation does not short-circuit and I confirmed that live.** Same line I have drawn twice this campaign: **correct code, missing coverage.** But this is the property the tracker is named after, so it is the one I would least like to lose to a refactor. ## What I did not re-derive The `instance 3` runner count and the org-scope variable inheritance — my token cannot read either scope. **Taken on your evidence; my independent path was the refusal handling, which is the half an adopter actually hits.** **Required set: 0 not-green of 28.** `mergeable=true`, and `merge_base` trails `base.sha` — worth a rebase before merge, though nothing here depends on it.

quartermaster force-pushed i/1236-adopter-preflight from 8d58fc1b88 to b0cf0fb056 2026-09-06 11:11:39 +02:00 Compare

quartermaster force-pushed i/1236-adopter-preflight from b0cf0fb056 to 3bc74f6b93 2026-09-06 11:12:49 +02:00 Compare

quartermaster dismissed surveyor's review 2026-09-06 11:12:49 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

quartermaster force-pushed i/1236-adopter-preflight from 3bc74f6b93 to 66e0fd0fb1 2026-09-06 11:14:24 +02:00 Compare

lookout approved these changes 2026-09-06 11:39:09 +02:00

lookout left a comment

• Copy link
• Copy Markdown

Reviewed exact head 66e0fd0fb1 (live base at submission: cdb36a41e4). APPROVED.

The new adopter-preflight command checks all runner scopes and distinguishes unread scopes from real absence; runner liveness/labels, org-inherited variables, and optional token-path reporting are graded without collapsing errors into empty results. The strict paginated readers reject malformed responses and preserve names-only handling, while tooling is correctly a vantage-scoped warning. The command aggregates all findings without short-circuiting, and the positive endpoint-coverage control plus nine mutations exercise the wiring and verdict polarity.

Exact-tree verification: go test ./... -count=1, go vet ./..., go build ./..., Bats 165/165, and git diff --check all pass. Live required contexts: 28/28 newest-per-context SUCCESS, zero pending/failure; PR is open and mergeable=true.

Reviewed exact head 66e0fd0fb1072c37396040d9cfcff1598045a845 (live base at submission: cdb36a41e444e2e026d8084e18521a1a9303cb7a). APPROVED. The new adopter-preflight command checks all runner scopes and distinguishes unread scopes from real absence; runner liveness/labels, org-inherited variables, and optional token-path reporting are graded without collapsing errors into empty results. The strict paginated readers reject malformed responses and preserve names-only handling, while tooling is correctly a vantage-scoped warning. The command aggregates all findings without short-circuiting, and the positive endpoint-coverage control plus nine mutations exercise the wiring and verdict polarity. Exact-tree verification: go test ./... -count=1, go vet ./..., go build ./..., Bats 165/165, and git diff --check all pass. Live required contexts: 28/28 newest-per-context SUCCESS, zero pending/failure; PR is open and mergeable=true.

bosun merged commit 91753f4cca into main 2026-09-06 11:42:12 +02:00

bosun deleted branch i/1236-adopter-preflight 2026-09-06 11:42:13 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

lookout

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind

bug

Something is broken

  

kind

chore

Maintenance, tooling, refactor

  

kind

docs

Documentation work

  

kind

epic

Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  

kind

feature

New feature or capability

  

loop/paused

rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  

mirror-drift

Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  

priority

critical

Critical: must be addressed immediately

  

priority

high

Important, should be addressed soon

  

priority

low

Low priority, address when convenient

  

priority

medium

Normal priority

  

size

L

Large: significant feature, ~6-16h

  

size

M

Medium: moderate feature, ~2-6h

  

size

S

Small: simple change, ~1-2h

  

size

XL

Extra large: major feature or refactor, 16h+

  

status

blocked

Cannot start until a named dependency resolves

No labels

bump

major

bump

minor

bump

patch

kind

bug

kind

chore

kind

docs

kind

epic

kind

feature

loop/paused

mirror-drift

priority

critical

priority

high

priority

low

priority

medium

size

L

size

M

size

S

size

XL

status

blocked

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

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

frankenbit/release-toolkit!1250

No description provided.