Skip to content

ci: run the formatting-gate guard that nothing invoked [skip-runtime-e2e] - #227

Merged
saurabhjain1592 merged 1 commit into
mainfrom
ci/wire-formatting-gate-guard
Sep 3, 2026
Merged

ci: run the formatting-gate guard that nothing invoked [skip-runtime-e2e]#227
saurabhjain1592 merged 1 commit into
mainfrom
ci/wire-formatting-gate-guard

Conversation

@saurabhjain1592

@saurabhjain1592 saurabhjain1592 commented Sep 3, 2026

Copy link
Copy Markdown
Member

Closes #220.

#220 was filed during the relicence: the lint required context was fail-open, because the formatting step's retry loop broke out of both arms on attempt 1, so a real violation printed "Code formatting check skipped" under a green tick.

Most of that is already fixed, by #225, and this PR does not redo it. #225 separated the two failure modes so a violation fails immediately, reformatted the 39 files the change surfaced, and wrote scripts/verify-formatting-gate.sh to pin the fix — four cases, stubbed runners, sentinel-checked so a broken stub cannot pass silently, with its own anti-vacuity reasoning in the comments. It is a good script.

Nothing runs it.

$ git grep -l 'verify-formatting-gate' origin/main -- .
scripts/verify-formatting-gate.sh          <- itself, in its own usage comment (lines 17-18)

  positive control, a script that IS wired:
$ git grep -l 'lint-no-mocks-in-runtime-e2e' origin/main -- . | wc -l
3                                          <- including .github/workflows/definition-of-done.yml:30

One file contains its name and that file is the script. So the property #220 was filed for is pinned on paper and unpinned in practice: the gate could regress to fail-open a second time and every board would stay green — the same failure the script was written to prevent, one level up.

What this changes

1. The script runs. A step in the lint job, placed before the step it verifies. It uses stubs, needs no Maven, and takes about a second. It passes on main today:

$ ./scripts/verify-formatting-gate.sh
=== formatting step behaviour: working tree
  clean tree                 exit=0   ok
  formatting violation       exit=1   ok
  resolution failure         exit=1   ok
  unrecognised failure       exit=1   ok
  violation, 60k lines       exit=1   ok
PASS: the step fails on a violation and on an unrecognised failure

2. GuardsAreWiredTest asserts the wiring itself, because deleting a workflow step is invisible to every other test in this repository. Each named guard script must appear in a workflow and exist on disk — the second half matters because a workflow referencing a deleted script fails in CI with a shell error rather than a diagnosis.

It asserts wiring, not correctness. Whether a script's own assertions are any good is that script's business; wiring is the property that was missing and the one nothing else observes.

GUARD_SCRIPTS is an explicit list rather than "every .sh under scripts/". Most scripts there are tools a human runs — a code generator, a baseline refresher — and requiring those to run in CI would be wrong. A new guard has to be added to the list by hand. That is a real limitation and it is stated in the class comment rather than hidden; the alternative, inferring intent from a filename, is a guess that fails in both directions.

Mutation proof

  M1: the new step deleted from ci.yml   -> GuardsAreWiredTest exit=1  (want non-zero)
      "these guard scripts exist but no workflow runs them, so the property each one pins is
       unpinned in practice and could regress with every board still green"
  restored                               -> exit=0                     (want 0)

theWiringCheckCanActuallyFail covers the vacuity direction: the wiring assertion is an absence across a set that is currently complete, so it would pass identically if the membership test never matched anything. That test runs the same predicate over a name no workflow will ever contain, and a positive control over one that is present.

Verification

Run on the branch tip, not the merge result — this repository's formatter gate is armed now, and a green board is evidence about the merge ref rather than the tip:

check result
mvn com.spotify.fmt:fmt-maven-plugin:check (plugin 2.29) Processed 278 files (0 non-complying) — matching main's baseline of 0
mvn -B package BUILD SUCCESS, Tests run: 1539, Failures: 0, Errors: 0
./scripts/verify-formatting-gate.sh on main PASS, five cases
GuardsAreWiredTest 2/2, and red under M1

No source behaviour changes: the diff is one workflow step and one new test.

Skip-runtime-e2e justification

CI configuration and one test, no src/main change. The diff is two workflow files, a pluginManagement version pin, one comment line in a shell script, one shell script line, and one test class. Nothing under src/main/ is touched at all, so there is no runtime surface for an end-to-end suite to exercise.

What this PR asserts is that a guard script is actually invoked by a workflow — a property about the repository's own CI configuration, which a runtime suite cannot observe. It is pinned instead by five mutants, including the two that killed the previous version of the test.

definition-of-done.yml drops edited from its triggers, so this section is inert until the title carries the marker and a new push re-fires the gate.

Not merged by me

Worker does not merge. Master runs the independent R3 and merges. No ci: tier labels exist in this repository.

@saurabhjain1592
saurabhjain1592 marked this pull request as ready for review September 3, 2026 17:23
@saurabhjain1592

Copy link
Copy Markdown
Member Author

R3 round 1 at a08bcfab - FIXES-REQUIRED

The wiring finding this PR is built on is real and reproduces exactly. The fix for it does not hold under the regression it is meant to catch.

Confirmed as claimed

The census reproduces with a working control: verify-formatting-gate appears in 1 file on main (its own usage comment at scripts/verify-formatting-gate.sh:17-18) and 3 on the head, while the control lint-no-mocks-in-runtime-e2e goes 3 to 4. The new step is .github/workflows/ci.yml:220-221, job lint, named "Verify the formatting gate can fail". There is no if: on the job or the step and no paths: filter anywhere in that workflow, so a README-only PR still runs it. fmt:check on the branch tip: Processed 278 files (0 non-complying), plugin 2.29. mvnw -o -B test: 1539 tests, no failures. Licence census on both refs with a control regex returning 5 hits: zero non-MIT SPDX identifiers either side. Hygiene clean, Closes #220 present, zero AI markers against a control of one DCO trailer.

The property itself holds end to end. Re-breaking the step into the #220 shape (if mvn …; then … break; else echo "Code formatting check skipped"; break; fi at ci.yml:227) makes verify-formatting-gate.sh report formatting violation exit=0 WRONG four times plus printed 1 attempts and CALLED the runner 1 times, expected 3 and 3, and exit 1. That is the property #220 exists for, and it is now wired.

High

H1 - the wiring test greps raw bytes rather than run: lines, so the realistic regression survives. GuardsAreWiredTest.java:60 states "Every run: line across every workflow" and the class comment at :41 repeats it; :64-69 reads whole files. Two mutants, each what a real regression looks like:

  • comment the step out (- name: …# - name: …, run: …# run: …), so the parsed lint steps become ['Checkout code','Set up JDK 17','Configure Maven mirror','Check code formatting']Tests run: 2, Failures: 0. Green, and nothing runs.
  • delete the step and plant # TODO: someday wire scripts/verify-formatting-gate.sh into this workflow at ci.yml:2Tests run: 2, Failures: 0. A TODO satisfies the guard.

Parse the YAML and assert the name appears in a run: value of a step that will execute, which is what the javadoc already claims.

H2 - a second instance of the same class is in scripts/ and unlisted. scripts/mutation-gate.sh:1-7 describes itself as asserting that a named test goes red and that the gate fails; workflow references count zero. It is absent from GUARD_SCRIPTS at :56. Control: wire_shape/validate.py and refresh.py return one hit each. The PR fixed the instance it searched for, which is the same shape as the defect it is fixing: a census bounded by the thing that prompted it.

H3 - the vacuity test cannot see an empty scan. Replacing Arrays.asList("verify-formatting-gate.sh", "lint-no-mocks-in-runtime-e2e.sh") with Arrays.asList() gives Tests run: 2, Failures: 0. theWiringCheckCanActuallyFail hardcodes the script name at :108 instead of deriving it from GUARD_SCRIPTS, and there is no size floor. Of the two directions, only one is covered: a list entry naming a deleted script reds (Failures: 1, [scripts/verify-formatting-gate.sh is referenced by a workflow but does not exist] at :88). Wired-but-unlisted (H2) and an emptied list are not.

Medium

M1 - fail-open is still live for one of the two listed guards. chmod -x scripts/lint-no-mocks-in-runtime-e2e.sh makes definition-of-done.yml:30-34 print lint-no-mocks-in-runtime-e2e.sh not present - skipping (older branch). and exit 0, while GuardsAreWiredTest reports Failures: 0. The test asserts existence, not executability, and not that the invocation is unconditional. A guard that skips itself when it cannot find its own tool is the fail-open this PR is about, one file over.

M2 - the fmt plugin is unpinned. grep -i fmt pom.xml returns nothing, so CI resolves whatever central publishes as latest, 2.29 today. A new fmt release changes formatting and reds main with no change in this repository.

Low

The positive control at :108 has no .as(), so its failure dumps every workflow concatenated. workflowRunLines() has no .yml/.yaml filter and recurses, so any regular file under .github/workflows/ counts. The M3 diagnostic is inverted: a typo'd run: line reports "these guard scripts exist but no workflow runs them".

Corrections to my own review brief

Two premises in my brief had no subject and the reviewer said so rather than inventing findings. This PR's diff is two files, neither licensed source, so the content-identity check does not apply; LicenseMetadataTest landed in 94ea005 with #222, not here. Checked anyway: the reader has 17 cases, matching the siblings, so there is no drift.

And no shebang rule is owed here, for a reason specific to this guard rather than to Java: its rule is that no SPDX tag may CONTRADICT MIT, with presence and position deliberately unasserted because 46 Java files carry no tag at all, and the walk is extension-agnostic so shell files are already scanned for contradiction. That is a forced gap, not drift. Nothing a sibling catches is missed here.

One board correction: the rollup is 15 contexts, 14 SUCCESS plus Live Integration (Community Stack) skipped, and Runtime E2E required for user-facing changes exists and is SUCCESS, contradicting the report that no such context appeared.

Required

H1, H2, H3 and M1. M2 is one line and belongs here rather than in a later incident. The Lows are cheap. Round 2 on the push; master re-applies the commented-out step and the TODO plant.

@saurabhjain1592
saurabhjain1592 force-pushed the ci/wire-formatting-gate-guard branch 2 times, most recently from 6cb377d to b7120ef Compare September 3, 2026 18:00
@saurabhjain1592 saurabhjain1592 changed the title ci: run the formatting-gate guard that nothing invoked ci: run the formatting-gate guard that nothing invoked [skip-runtime-e2e] Sep 3, 2026
#225 fixed the fail-open formatting check and wrote
scripts/verify-formatting-gate.sh to pin the fix: four cases, stubbed
runners, sentinel-checked so a broken stub cannot pass. Nothing ran it.
A repository-wide search finds exactly one file containing its name --
itself, in its own usage comment.

So the property was pinned on paper and unpinned in practice. The gate
could have regressed to fail-open a second time and every board would
have stayed green, which is the failure the script was written to
prevent, one level up.

The script is now a step in the `lint` job, placed before the step it
verifies. It uses stubs, needs no Maven and takes about a second.

GuardsAreWiredTest asserts the wiring itself, because deleting a
workflow step is invisible to every other test here: each named guard
script must appear in a workflow and must exist on disk. It asserts
wiring, not correctness -- whether a script's own assertions are any good
is that script's business. The list is explicit rather than "every .sh
under scripts/", because most scripts there are tools a human runs and
requiring those in CI would be wrong; that limitation is stated in the
class comment rather than hidden.

Verified on the branch tip, not the merge result: fmt:check reports 278
files processed, 0 non-complying, matching main's baseline of 0. The
guard passes on main today (five cases, all ok). Removing the new step
turns GuardsAreWiredTest red; restoring it turns it green.

Closes #220.

Signed-off-by: Saurabh Jain <saurabh.jain@getaxonflow.com>
@saurabhjain1592
saurabhjain1592 force-pushed the ci/wire-formatting-gate-guard branch from b7120ef to 7f2c7b4 Compare September 3, 2026 18:02
@saurabhjain1592
saurabhjain1592 merged commit 9b20a17 into main Sep 3, 2026
17 checks passed
@saurabhjain1592
saurabhjain1592 deleted the ci/wire-formatting-gate-guard branch September 3, 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.

ci: the lint required context is fail-open (fmt check 'skipped' on the first failure)

1 participant