Skip to content

ci: add an image summary check and record main's required checks - #1188

Merged
mforce merged 6 commits into
mainfrom
docs/1182-required-checks
Oct 10, 2026
Merged

mforce merged 6 commits into
mainfrom
docs/1182-required-checks

Conversation

@mforce

@mforce mforce commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Closes #1182

Why

The maintainer turned on the ruleset "Main required checks" (id 24852475): required ci.yml checks, pinned to the GitHub Actions app, and "require branches to be up to date". It exists because #1162 merged green against a main that predated #1155's lint rule, and main stayed red at 3e879b0c until #1176. Two documents still said main has no required status checks, and nothing in the repository recorded the rule or what breaks it.

The ruleset first required Image build + smoke test (amd64) and (arm64). A documentation-only PR never reports those names: the image job's job-level if: skips the matrix before its names expand, so it reports one check named Image build + smoke test (${{ matrix.arch }}) (run 37552204234). The two names were removed from the ruleset on 2026-10-10, leaving six checks. This PR adds a stable-named job so image enforcement can come back as a seventh.

Scope

  • ci.yml: a new image-summary job named Image build + smoke test. It needs changes and image and runs under always(). It passes when every image leg succeeded, or when image was skipped on a documentation-only PR. Any other skip, a cancellation or a failure fails it. It is not in publish.needs. The stale comment saying main has no required checks is corrected.
  • docs/decisions/351-releases.md and the matching comment in release-please.yml: the release token's ability to approve a PR is still inert, because main requires zero approving reviews and an approval cannot satisfy a status check. Reason (b) for the App-opened release PR is now live.
  • New docs/decisions/1182-required-checks.md, its index row, and one rule paragraph in .github/AGENTS.md. They cover:
    • the seven-check end state;
    • why the per-arch names cannot be required, and that any future matrix job an if: can skip needs the same summary job;
    • that renaming a required job or adding a path filter must update the ruleset in the same PR;
    • which checks must never be required;
    • what the integration_id pin does and does not prove;
    • what strict mode costs.

After merge

Add Image build + smoke test (integration_id 15368) to ruleset 24852475. Requiring it before this merges would block every PR whose base lacks the job.

Verification

  • Ruleset, branch rules and token defaults read from the live API: main requires zero approving reviews, release/v1.0.x requires no checks, and the default workflow token permission is read. No workflow grants statuses: write.
  • node --test .github/scripts/: 102 passed. Tracked-file guards: Documentation tests 20 passed, SchemaDocsTests and SolutionTestProjectSplitTests 5 passed. actionlint is not installed here.
  • This PR's own CI reports Image build + smoke test as a single passing check.

A job-level if: skips the image matrix before its names expand, so a
docs-only PR reports no (amd64)/(arm64) check and could never satisfy a
ruleset requiring them. Image build + smoke test passes only when both
legs succeeded or the PR is documentation-only, and fails closed
otherwise. Also drop ci.yml's stale claim that main requires no checks.
@mforce mforce changed the title docs(ci): record main's required checks and correct the release-token note ci: add an image summary check and record main's required checks Oct 10, 2026
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 came from Codex (gpt-6-astra): one P1, one P2 and one P3. Head is e5c38999.

1. P1: docs-only PRs never report the two required image checks. Confirmed. The job-level if: skips the image matrix before its names expand, so run 37552204234 reported one check named Image build + smoke test (${{ matrix.arch }}). The ruleset was corrected on 2026-10-10: the two arch names were removed, leaving six required checks. a0351116 adds an image-summary job named Image build + smoke test. It needs changes and image, runs under always(), and passes only when every leg succeeded or image was skipped on a documentation-only PR. Any other skip, a cancellation or a failure fails it. It is not in publish.needs. This PR's CI reports it as one passing check run from github-actions. e5c38999 records the seven-check end state in the decision record and .github/AGENTS.md, and why the per-arch names cannot be required. It adds that any future matrix job an if: can skip needs the same summary job. The seventh check is added to the ruleset after this PR merges, because requiring it earlier would block every PR whose base lacks the job.

2. P2: the integration_id pin was described as stronger than it is. Both places now say the pin rejects statuses from other identities but does not authenticate a particular workflow, because every workflow's GITHUB_TOKEN acts as the GitHub Actions app. No workflow grants statuses: write, every workflow declares its own permissions:, and the repository's default token permission is read (checked against the API). Review keeps it that way. Fixed in e5c38999.

3. P3: ci.yml still said main has no required checks. The comment now says main requires checks from this workflow (#1182), so a path filter would wedge every docs PR. It keeps the filtered-workflow versus skipped-job distinction and points to image-summary for the skipped-matrix exception. Fixed in a0351116.

@mforce
mforce merged commit 73c434a into main Oct 10, 2026
18 checks passed
@mforce
mforce deleted the docs/1182-required-checks branch October 10, 2026 23:10
mforce added a commit that referenced this pull request Oct 10, 2026
Folds #806's OAuth audience and MCP tool-gate rules into src/AGENTS.md, and
ports the rules #1188, #1190, #1193 and #1199 added to the instruction files.
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.

ci: require PRs to be tested against the latest main before merging

1 participant