Skip to content

ci: the build matrix changes shape between pull_request and push, so build (11) and build (21) can never report on a PR #234

Description

@saurabhjain1592

Found while deriving the required-status-check lists for getaxonflow/axonflow-enterprise#3890. axonflow-sdk-java is not in that issue's scope and nothing here needs doing today — this is filed because it is a landmine sitting directly in the path of whoever is next told to bring java's ruleset up to parity with go and python.

The mechanism

.github/workflows/ci.yml, job build, has no name:, so its status-check context is the job id plus the matrix leg — build (11), build (17), build (21). And the matrix changes shape depending on the event:

strategy:
  matrix:
    java-version: ${{ fromJson(github.event_name == 'pull_request' && '[17]' || '[11, 17, 21]') }}

So on a pull_request the only context that exists is build (17). On a push to main, three exist and build (17) is one of them.

Why that is a trap

A reviewer bringing java to parity will look at a recent push-to-main run, see build (11), build (17) and build (21), and require all three — which is the natural reading of "require the build". Two of those three can never report on a pull request, and a required context that never reports leaves the PR blocked forever with a green board.

This is the same class as the three exclusions applied in #3890 (no pull_request trigger; a job-level if: github.event_name != 'pull_request'; a paths: filter), but it is harder to see than any of them, because the job does run on pull requests — just under a different set of context names.

The symptom is also misleading: the PR is blocked on build (11), which points a reader at the Java 11 build rather than at the matrix expression that decides whether it exists.

The safe options, for whoever does the work

  1. Require only build (17) — correct today, and silently wrong the day the PR leg moves off 17.
  2. Give the job a static name: and require that, so the context is event-independent. ci.yml already has build-summary (name: Build Summary, if: always()), which is the aggregate job and the natural thing to require. Note it is already required on this repo's ruleset — so java may need nothing at all here.
  3. Require none of the matrix legs and rely on Build Summary.

Option 2/3 is what the current ruleset already does, which is why this is a hazard for a future change rather than a live defect.

Also worth knowing before touching java's ruleset

Java has two path-filtered PR checks — Validate Version Alignment (validate-version-alignment.yml) and Validate Wire Shape (wire-shape-contract.yml). Both are exactly the kind of check a reviewer insists on requiring, and both would block every PR that touches none of their filtered paths. Neither is required today. Keep it that way unless the filters are removed.

Definition of done

  • Nothing, unless someone proposes changing java's required set — at which point this issue is the checklist
  • If java's required set does change: no matrix-interpolated context name is required, and neither path-filtered check is required

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions