Skip to content

fleet-ops: run the required-context preflight in the read-only check - #294

Merged
cpitzi merged 1 commit into
mainfrom
read-only-preflight-293
Oct 31, 2026
Merged

cpitzi merged 1 commit into
mainfrom
read-only-preflight-293

Conversation

@cpitzi

@cpitzi cpitzi commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Closes #293

Summary

fleet-ops/README.md describes the read-only fleet-apply.sh check as the home of the required-context preflight: the proof that a context has reported on a recent PR before anything requires it. The script did not match that. seen_contexts ran only inside the mutating --require-checks path. Since terraform/ took over required checks, the README itself describes that path as racing the Terraform state, so the fleet had no supported way to run the preflight without writing to an enforced ruleset. For #291 the preflight had to be run by extracting the function by hand.

This PR makes the read-only check run the preflight itself. For each repo where required-checks.json lists contexts the live ruleset does not require yet (⧗ req-checks missing=[…]), it calls the existing seen_contexts and appends ✗ req-checks never-seen-on-a-PR=[…] for any context that has not reported on the repo's five most recent PRs. That is exactly the state of a branch that adds a context, before its Terraform apply. With the change, the README's existing claim is accurate. One sentence is added to the required-checks.json section documenting the new marker.

Fix the code or fix the docs

The alternative was to correct the README to say only --require-checks runs the preflight. That would have documented a preflight you can only run by mutating live state on a Terraform-enforced surface, which is what the README warns against. The code change is 9 added lines plus one widened declare, not a one-liner. It reuses seen_contexts unchanged and only adds API calls for repos with missing contexts, so a fleet-wide sweep with no rollout pending costs the same as before.

Testing

I ran a scratch copy of the script against a scratch required-checks.json that adds two contexts to epigaea: YAML Lint, which reports on epigaea PRs but is not required, and Batz, which is bogus.

  • epigaea                     ⧗ req-checks missing=[YAML Lint Batz] ✗ req-checks never-seen-on-a-PR=[Batz]

Against the real config, where epigaea has no missing contexts and the preflight is not run:

  ✓ epigaea                    settings ok (branch rulesets: 1)

shellcheck fleet-ops/fleet-apply.sh is clean and python3 ci/validate.py passes.

Scope

Nothing else in fleet-apply.sh changes. That includes the --require-checks path, its own preflight, and the end-of-run summary hints.

🤖 Generated with Claude Code

…293)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Review (haiku, claude-review.yml)

Review of #294: fleet-ops: run the required-context preflight in the read-only check

I read the diff and the full fleet-apply.sh. I could not run shellcheck, bash -n, or fetch issue #293 here because those shell commands were refused, so the shellcheck-clean claim is unverified by me.

  1. minor: fleet-ops/fleet-apply.sh:396-397. The end-of-run hint still tells the operator to run --require-checks once the producing workflows are merged. For a context that never reported on a PR, which the new ✗ never-seen-on-a-PR marker now flags at lines 286-288, that command refuses (require_checks_apply, lines 138-143). The summary's advice and the new marker now disagree. The PR body says the hints were left out of scope, but this contradiction is created by the change.

  2. minor: fleet-ops/fleet-apply.sh:280-288. The new preflight path has no automated coverage. CI runs shellcheck and ci/validate.py, and neither exercises the seen/unseen comparison. A regression in that logic would leave CI green. The only evidence is the scratch run described in the PR body. The bar in CLAUDE.md is to prove a check can fail, so this is worth a reproducible test or a note on the limitation.

The PR-workflow items pass. The body has Closes #293, the title matches the change, the summary covers what, why and constraints in neutral prose, and no README or CHANGELOG at the repo root changed. No Terraform, ruleset, label, generated-artifact, or secret-exposure issues are present in this diff.

@cpitzi

cpitzi commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Review triage

Finding 1 (summary hint says to run --require-checks): declined here, filed as #295. This PR does not create the disagreement. The hint was already conditional ("After each producing workflow is merged to main & observed green on a PR"), and ✗ req-checks never-seen-on-a-PR is the check for that condition, so the two read together. A flagged context hasn't met the condition yet. The underlying problem is real and pre-existing, though. Since terraform/ took over required checks, the hint points at the path the README says races the Terraform state, and the live-not-in-file hint has the same framing. #291's follow-up kept every other fleet-apply.sh behavior out of scope, so I filed it as #295 with the Terraform-era wording to use.

Finding 2 (no automated coverage of the seen/unseen comparison): declined. fleet-apply.sh has no test harness for any of its behavior, including the identical comparison in --require-checks's own preflight (require_checks_apply), which this change reuses as is (seen_contexts, then grep -qxF against the wanted list). A test needs a gh mock to build, and that is its own work item. The failure path was shown to fail before this PR opened: in the scratch run, the bogus Batz was flagged, the seen-but-unrequired YAML Lint was not, and the real config stayed clean. That output is in the PR body. Shellcheck now passes on this PR.

Both findings are triaged, so I'm arming auto-merge.

@cpitzi
cpitzi merged commit d70f6fa into main Oct 31, 2026
12 checks passed
@cpitzi
cpitzi deleted the read-only-preflight-293 branch October 31, 2026 18:13
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.

fleet-ops: the read-only check does not run the required-context preflight the README describes

1 participant