Skip to content

Report the Provider schema check on merge-queue refs - #5928

Open
Divyansh-db wants to merge 1 commit into
mainfrom
schema-check-merge-queue
Open

Report the Provider schema check on merge-queue refs#5928
Divyansh-db wants to merge 1 commit into
mainfrom
schema-check-merge-queue

Conversation

@Divyansh-db

Copy link
Copy Markdown
Contributor

Why

The status job of the Provider schema workflow is about to be made a required status check, so that breaking provider-schema changes block merging instead of just warning.

That change is not safe on its own. Required status checks are enforced against the merge-queue ref as well as the PR head, and this workflow only triggers on pull_request. Nothing would report the status context on gh-readonly-queue/*, so every queued entry would sit until the queue's status-check timeout expires and then be dequeued — blocking all merges, not just the ones with breaking schema changes.

Worth stressing that a required check which never reports is not treated as skipped or neutral; it stays pending. So the context has to report something on the queue commit.

What

Add a merge_group trigger and a merge-queue-status job that posts a synthetic status success on the queue commit. This mirrors the auto-approve job in integration-tests.yml, which exists for the same reason.

The classifier itself deliberately does not run in the queue:

  • Cost. It is a double provider build (base + head). It already ran at the tip of the branch, and the queue only squashes commits that each passed it.
  • Correctness. The bypass directive is read from github.event.pull_request.body, and the merge_group payload has no pull_request object. A PR legitimately carrying ALLOW_SCHEMA_BREAKING_CHANGE=true would pass at the PR level and then fail in the queue, with no way to unblock it.

Tightening classify's gate to an explicit event allowlist is a required part of this, not a drive-by cleanup: its previous github.event_name != 'pull_request' clause evaluates to true on merge_group, so adding the trigger alone would have rebuilt the provider on both sides inside the queue for no new signal.

The status job needed no change — it is already gated to pull_request.

Tradeoff

The schema is verified at PR head, not at the squashed queue commit, so two PRs that each pass alone but conflict semantically could merge. This is the same tradeoff already accepted for Integration Tests, and it is narrowed by the queue's strict-required-checks policy and ALLGREEN grouping. Documented in a comment on the job.

Verification

  • Workflow YAML parses; the three job conditions are mutually exclusive per event (classify/status on pull_request, classify on workflow_dispatch, merge-queue-status on merge_group only)
  • make ws passes
  • The synthetic check is named status to match the required context, which is the job id — that job declares no name:. Confirmed against live check runs.
  • The pinned github-script SHA and the runner group are both already used elsewhere in this repo

Merge order

This should land before the check is marked required. On its own it is a no-op for PR behavior — it only adds a job that runs on queue refs.

NO_CHANGELOG=true

This pull request and its description were written by Isaac.

Required status checks are enforced against the merge-queue ref as well as the
PR head. The `status` job of this workflow is about to become a required check,
but the workflow only triggers on `pull_request`, so nothing would report the
`status` context on `gh-readonly-queue/*`. Every queued entry would wait out the
queue's status-check timeout and be dequeued, blocking all merges rather than
just the ones with breaking schema changes.

Add a `merge_group` trigger and a `merge-queue-status` job that posts a synthetic
`status` success on the queue commit, mirroring the auto-approve job that
integration-tests.yml already uses for the same reason.

The classifier itself deliberately does not run in the queue:

* It is a double provider build, and it already ran at the tip of the branch.
  The queue only squashes commits that each passed it.
* The bypass directive is read from `github.event.pull_request.body`, which does
  not exist on the merge_group payload. A PR legitimately carrying
  ALLOW_SCHEMA_BREAKING_CHANGE=true would pass on the PR and then fail in the
  queue with no way to unblock it.

Tightening `classify`'s gate to an explicit event allowlist is required for this:
its previous `github.event_name != 'pull_request'` clause was true on
`merge_group`, so adding the trigger alone would have rebuilt the provider on
both sides inside the queue for no new signal.

NO_CHANGELOG=true

Co-authored-by: Isaac
@Divyansh-db
Divyansh-db requested review from a team as code owners August 5, 2026 23:38
@Divyansh-db
Divyansh-db requested review from renaudhartert-db and removed request for a team August 5, 2026 23:38
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Unit tests

If this PR is from a fork, the tests check runs offline against a pre-warmed Go module cache because fork PRs cannot authenticate to the internal Go module proxy.

If this PR changes go.mod or go.sum, the tests check will fail until a maintainer warms the cache for it:

Actions -> Warm Go Cache -> Run workflow -> pr_number = 5928

Re-run the failed check once the cache warming completes.

Integration tests

Integration tests don't run automatically for external contributors; an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 5928
  • Commit SHA: de1148936134516ca7e7f4b20c2731a439d29a24

Checks will be approved automatically on success.

@Divyansh-db
Divyansh-db removed request for a team and renaudhartert-db August 6, 2026 09:35
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.

1 participant