feat(cli): rt adopter-preflight reports every missing prerequisite in one run #1250
No reviewers
Labels
No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!1250
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/1236-adopter-preflight"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
rt adopter-preflightnames 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, andminisignin 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.gradeAdopterPreflighttherefore runs every check and tallies findings; nothing short-circuits.Live, three breaks at once, one run:
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:
{"runners":[…]}envelope, key requiredlabels: [{"name":…}]labels: ["go","playwright"]— plain stringsstatus == "online"idle/active—"online"is never emittedThe 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-toolkithas 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 (
401measured 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 asUNREADand downgrades the verdict to could-not-grade: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
ActionVariablehas 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.401ontoken,Bearerand basic auth, while an anonymous read of the probe repo returns200. Validation was done against this Forgejo instead, where I control runners at all three scopes.Mutation verification
Nine mutations, each reddening its own arm:
ReportsEveryMissingPrerequisiteInOneRunRegisteredButNoneLiveIsAFailureLiveButWrongLabelIsAFailureMissingVariableIsAFailure(+ aggregation)UnreadableScopeIsNotAnAbsenceMissingToolingWarnsRatherThanFails(+ aggregation)RunAdopterPreflight_ReadsEveryScopeRunAdopterPreflight_ReadsEveryScopeRunAdopterPreflight_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_ReadsEveryScopedrives the realrunAdopterPreflightagainst anhttptestserver 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 andhelp.txtsnapshot updated.🤖 Generated with Claude Code
https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH
An adopter learns the three prerequisites — a runner, the minisign public-key variable, and job-image tooling — one CI run at a time, because each is only discovered when the previous one stops blocking. A preflight that reports the first missing thing relocates those round trips instead of removing them, so gradeAdopterPreflight runs every check and tallies the findings together. Runners are read at repository, organisation AND instance scope. Measured on frankenbit/release-toolkit, whose CI is green continuously: repo 0, org 0, instance 3. A repo-scoped read alone answers a different question than the one being asked, and failing on its zero would red-flag every repository here. A scope the caller may not read is reported as UNREAD, never as an absence. The instance endpoint is site-admin-only and refuses an ordinary caller (401 measured anonymously), which is the adopter's normal vantage — so a partial read downgrades to could-not-grade rather than manufacturing a failure. Job-image tooling is a WARNING with its vantage stated, not a failure: the check inspects the process it runs in, and the forge cannot answer it at all. 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. Three wire-shape assumptions were refuted by reading the live API and are now pinned by fixture: the response is a bare array rather than a {"runners":[…]} envelope, `labels` is an array of plain strings rather than of objects, and `status` is idle/active — this forge never emits "online", so an allow-list on that value read every healthy runner as down. Mutation-verified: nine mutations, each reddening its own arm, including three that drop a scope read and are caught only by the wiring positive control. Refs #1236, #1229 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyHa1cbc4a3cb4f121545494f121545498d58fc1b88APPROVED 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
My token is the adopter case by construction — repo readable, org and instance refused. So I could not confirm your
instance 3count, but I could exercise the path that matters more, and it is right:🔑 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":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", andTestAdopterPreflight_ReportsEveryMissingPrerequisiteInOneRunasserts2 blocking, 1 could-not-grade/warning. But injecting an early return into the wrong-label runner branch leaves the whole package green:⚠️ The one-run arm exercises a different runner branch, so the
default:case is not covered by it — anddefault:is precisely the branch your own PR body demonstrates: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 3runner 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, andmerge_basetrailsbase.sha— worth a rebase before merge, though nothing here depends on it.8d58fc1b88b0cf0fb056b0cf0fb0563bc74f6b93New commits pushed, approval review dismissed automatically according to repository settings
3bc74f6b9366e0fd0fb1Reviewed 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.