From bcb75006ab035dbcd3c8fd8bcff61a6340539008 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:50:43 +0200 Subject: [PATCH 01/15] docs(dev): add the build-gate discipline page and index it Create doc/development/build-gate-discipline.adoc as a peer of the Sonar quality-gate policy. It records the three mechanisms in this repository that mutate the working tree while the gate exits 0 -- OpenRewrite's import-group churn (and its poisoning of the change-ledger freshness stamp), the pre-commit formatter's non-idempotency, and the SimplifyTestThrows reversion reached via JUnit5BestPractices -- the three consequences a contributor acts on, the revert rule with the objection to it and the standing fixed-point debt, the recipe-scoping trap, the non-gating rewrite-report job that reports all of it, the incremental-build rule, and the recorded discard of the fully-qualified-refspec fix. Add the matching row to the contributor guide's Contents table. Co-Authored-By: Claude --- doc/development/README.adoc | 9 + doc/development/build-gate-discipline.adoc | 184 +++++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 doc/development/build-gate-discipline.adoc diff --git a/doc/development/README.adoc b/doc/development/README.adoc index 5515d556..d7938a69 100644 --- a/doc/development/README.adoc +++ b/doc/development/README.adoc @@ -30,6 +30,15 @@ to *run* the finished gateway). merging over a red or stale-green gate, PR-new-code vs post-merge project-gate auditability, and the fix-by-default / suppress-with-rationale escape hatch. +| link:build-gate-discipline.adoc[Build-Gate Discipline -- A Green Gate That Changed Your Files] +| The three mechanisms that mutate the working tree while the gate exits 0 -- OpenRewrite's + import-group churn, the pre-commit formatter's non-idempotency, and the `SimplifyTestThrows` + reversion -- the three consequences a contributor acts on (verify by surviving the gate, + attribute a dirty tree before reverting it, never hand-edit formatter-owned formatting), the + revert rule with the objection to it and the standing fixed-point debt, the recipe-scoping trap, + the non-gating `rewrite-report` job that reports all of it, the incremental-build rule, and the + recorded discard of the fully-qualified-refspec fix. + | link:release-process.adoc[Release Process] | How a release is cut -- merge a version change in `.github/project.yml`, which is the publishing act once the centrally-owned version-changed guard lets it through, with `workflow_dispatch` as the diff --git a/doc/development/build-gate-discipline.adoc b/doc/development/build-gate-discipline.adoc new file mode 100644 index 00000000..f6b9660a --- /dev/null +++ b/doc/development/build-gate-discipline.adoc @@ -0,0 +1,184 @@ += Build-Gate Discipline -- A Green Gate That Changed Your Files +:toc: +:toclevels: 2 +:sectnums: + +link:../../CLAUDE.md[`CLAUDE.md`] states the class, in its *Pre-Commit Process* section: a gate that +exits 0 can still have changed your files. This page is the detail behind that line -- which +mechanisms in this repository do it, what a contributor does about them, and the two neighbouring +rules about reading build output honestly. The class statement is not restated here; the +enforcement is named at the end. + +== The three mechanisms that exit 0 while mutating the tree + +Three distinct mechanisms in this repository mutate the working tree during a green +`verify -Ppre-commit` run. None of them is visible in the build result -- that is the whole +difficulty. `BUILD SUCCESS` is printed either way. + +=== OpenRewrite import-group churn + +The `pre-commit` profile's `activeRecipes` override in the root `pom.xml` activates +`org.openrewrite.java.OrderImports` and `org.openrewrite.java.RemoveUnusedImports`. Against a tree +that is not already at their fixed point, they rewrite import blocks across roughly *170 files* -- +almost all of them untouched by whatever branch is running the gate. + +This one has a second-order effect that is easy to miss. The plan-marshall change-ledger stamps +`worktree_sha` at *build end*, so it records the *churned* tree. Reverting the churn restores the +pre-build sha, and the freshness check then reports `stale` for a commit whose gate has just +passed. A "the worktree was never verified" refusal immediately after a green gate is this +interaction, not a lost build. + +=== The pre-commit formatter is not idempotent + +`org.openrewrite.java.format.AutoFormat` and its companions do not converge in one pass on this +tree: a run takes A to B, and the next run takes B to C rather than leaving B alone. The root +cause is that `main` is not at the formatter's fixed point, so every run has real work to do and +the work is not stable. + +The practical consequence is that "run the formatter again and see if it settles" is not a +diagnostic here. It will not settle. + +=== `SimplifyTestThrows` silently reverts an accepted narrowing + +`org.openrewrite.java.testing.junit5.JUnit5BestPractices` -- also in the `pre-commit` profile's +recipe list -- reaches `SimplifyTestThrows`, which broadens a narrowed `throws` clause on a test +method back to `throws Exception`. When a review bot asks for a narrower clause and the narrowing +is implemented and then run through the gate, the recipe reverts it, and the gate exits 0 over the +reverted form. The change is gone and every visible signal says the work landed. + +== The three operational consequences + +=== Verify a review-bot suggestion by surviving the gate, not by implementing it + +Implementing a suggestion is not evidence that the suggestion is in the tree. Run the gate, then +run `git status --porcelain`, and read the result: a file you edited that now shows as modified +against your own commit has been rewritten by the gate, and a file that shows nothing may have been +rewritten back to where it started. A suggestion is verified only by *surviving the gate*. + +=== Attribute a post-gate dirty tree before reverting it + +The reflex on a dirty tree is "the usual import churn, revert it and move on". That reflex is what +loses a `SimplifyTestThrows` reversion, because it discards the evidence unread. + +OpenRewrite prints its attribution into the build log, as a +`Changes have been made to by:` block followed by the recipe chain responsible. Read that +block before reverting anything: it distinguishes the three mechanisms above from each other and +from a genuine change of yours. + +One trap in the tooling around that log: the `build-maven` rewrite-log parser can report +`verdict: not_observed` for a build in which `rewrite:run` demonstrably ran, because it keys on a +findings marker rather than on the recipe-attribution blocks. That verdict is *not* evidence that +OpenRewrite was inactive. When it disagrees with the log, the log wins. + +=== Never hand-edit formatter-owned whitespace, wrapping or import order + +Whitespace, line wrapping and import order belong to the formatter. Editing them by hand puts you +in a loop against a mechanism that is not idempotent: two rounds of "reformat by hand, re-run the +gate, get a different result" is not bad luck, it is a proof of the non-idempotency described +above. Stop at that point and revert the cosmetic edits rather than trying to find the arrangement +the formatter will accept -- there is not one. + +== The revert rule, and the objection to it + +*Revert unrelated churn; keep the rewrite only for files the branch itself authored.* A scoped pull +request carries the files its change is about. + +The countervailing argument is real and should be stated rather than ignored: the reverted diff +*will reappear on the next run*, so reverting it is work that has to be repeated. That is true, and +it is not a reason to commit the churn. The reappearance is the standing cost of `main` not being +at the formatter's fixed point, and paying that cost once per pull request is cheaper than +polluting every scoped pull request with roughly 170 unrelated files -- which destroys the +reviewability of each one and buries the actual change. + +The durable fix is to bring `main` to the formatter's fixed point in a single dedicated pass. That +has not been done. It is recorded here as an open debt rather than left as folklore, and it is +deliberately out of scope for any scoped change, because it is a mechanical rewrite of +`api-sheriff/src/main/java/**` and belongs in a change that does nothing else. + +== The recipe-scoping trap + +A reproduction that drops the profile does not reproduce anything. `rewrite:run` *without* +`-Ppre-commit` reports `Using active recipe(s) []` and changes no file, because the recipe list +that reaches all three mechanisms is the `pre-commit` profile's `activeRecipes` override. + +The failure mode is a false negative that reads as an exoneration: a narrower, faster invocation +comes back clean, and OpenRewrite appears to be innocent of a mutation it did in fact perform. +`-Ppre-commit` is load-bearing in every reproduction of anything on this page. + +== The enforcement + +All three mechanisms are invisible in the build result by construction, so the one thing that can +be done about them mechanically is to *report* them. link:../../.github/workflows/maven.yml[`.github/workflows/maven.yml`] +carries a `rewrite-report` job, named "OpenRewrite dirty-tree report (non-gating)", which runs the +rewriting gate itself and then prints, into the job summary, the post-gate `git status --porcelain` +listing and every `Changes have been made to by:` block with its recipe chain. + +The job *deliberately never fails the build on a dirty verdict*. A blocking assertion over "the +tree is clean after the rewriting gate" would be red on every run for as long as `main` is not at +the formatter's fixed point -- so the job reports and does not judge. A genuine Maven or tooling +failure still turns the check red; only the dirty verdict is non-gating. + +== A build whose output says it did nothing has verified nothing + +This is the specific case of the general rule in link:../../CLAUDE.md[`CLAUDE.md`] -- a successful +build is not evidence that work happened -- applied to Maven's incremental compilation. + +Maven's `testCompile` compares test-source timestamps against the compiled test classes. A change +confined to `src/main/java`, or a pure signature change, leaves every test source older than its +class file, so `testCompile` skips compilation entirely and prints +`Nothing to compile - all classes are up to date`. Plain `test` inherits the same defect, because +Surefire runs whatever is already sitting in `target/test-classes`. + +*After any production-only or signature change, run `clean test-compile` -- or `clean verify`.* +That marker line in the output is the detection signal: a build that reports it has verified +nothing about the change, whatever its exit code says. + +Two instances of this repository's own history are the evidence for the single rule, not two +separate rules: a production-only change that looked green and yielded *142 compilation errors* +under `clean test-compile`, and a signature change that yielded *26*. + +== Two rules about reading a check's output + +=== An ERROR from a guard is a statement about the guard, not about its subject + +When a guard, gate or analysis step reports an error, the first reading is almost always "the thing +being checked is broken". Frequently the correct reading is "the check could not evaluate" -- a +missing input, an unresolvable path, a tool that never started. + +Establish that the check actually ran before treating its output as a finding about its subject. +Establish the same before treating a *green* as evidence: a check that silently could not run +reports success indistinguishable from a real pass, and that is the more dangerous half of this +rule. An error at least draws attention to itself. + +=== A plausible root cause is not the root cause + +A story that explains every symptom is not thereby true. An explanation that fits all the available +facts is a hypothesis with good fit, and it survives exactly until someone re-derives it against +the implementing source -- at which point a comfortable, load-bearing explanation can turn out to +describe code that does not exist. + +Before a diagnosis is acted on, read the implementation it claims to describe. + +== Recorded discard: the fully-qualified-refspec fix + +A proposal to make the release tag check use fully-qualified refspecs is recorded here as +*discarded*, so it is not re-opened from memory. + +It has nothing left to apply to. The project-local release skill's tag check now uses +`git fetch --tags --force` together with `git ls-remote --exit-code --tags`, and its only surviving +`merge-base` use is an ancestry assertion rather than a resolution against a prunable +remote-tracking ref -- which was the failure the refspec change existed to prevent. + +Its companion prescription, "run the guard checks serially", is likewise *not* landed, and +deliberately so: it was refuted by the correction that followed it, which found that serial +execution merely hid the problem rather than fixing it. + +== See also + +* link:../../CLAUDE.md[`CLAUDE.md`] -- the class statement, the general build-success rule, and the + pre-commit process this page details. +* link:../../.github/workflows/maven.yml[`.github/workflows/maven.yml`] -- the non-gating + `rewrite-report` job. +* link:sonar-quality-gate.adoc[SonarCloud Quality Gate -- Compliance Policy] -- the other blocking + gate a contributor answers to, and what a plan's declared footprint does about its findings. +* link:README.adoc[Contributor Guide] -- the index for this documentation layer. From c5253a059457ccd1663bda946a3372554d2427f1 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:51:21 +0200 Subject: [PATCH 02/15] docs(dev): state the declared-versus-asserted rules behind the matrix Add one section to the declared-limit assertion-coverage note carrying the three rules that motivate the matrix: the deletion discriminator (a parsed, schema-validated key with no production consumer is a silent security no-op, tested by asking whether deleting the key would turn anything red, with the note's own inert TLS protocol-floor row as the worked instance); that wiring an inert key is a behavioural flip for every consumer, so the first failing test is a sample of the dependant set rather than the whole of it, and a new assertion extends SingleSourceTlsContractTest instead of splitting the contract across a second file; and that an opt-in runtime feature needs a deployment-activation test, with TlsEdgeActivationWiringTest as the reference shape. The three marked blocks DescriptorInventoryWiringTest parses as its expected inventory are untouched; the integration-tests module suite is green. Co-Authored-By: Claude --- .../declared-limit-assertion-coverage.adoc | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/doc/development/declared-limit-assertion-coverage.adoc b/doc/development/declared-limit-assertion-coverage.adoc index 26c545b3..0c273be5 100644 --- a/doc/development/declared-limit-assertion-coverage.adoc +++ b/doc/development/declared-limit-assertion-coverage.adoc @@ -541,3 +541,56 @@ This table is prose, and prose drifts the moment a descriptor changes. Three pro When you add a limit-shaped key to any of the thirteen files, add its row here in the same change -- and give it a status that is honest, including `GAP`. + +== Declared Is Not Asserted: Three Rules Behind the Matrix + +The matrix answers *which declared limit has an asserting test*. These three rules are the *why* +behind that question -- what a key with no asserting test actually is, what happens the day it +acquires one, and which shape of test settles the question for an opt-in feature. Together they are +the reason this note counts gaps instead of trusting the descriptors. + +=== The deletion discriminator + +*A configuration key that parses is not a configuration key that acts.* A key that is spelled +correctly, passes schema validation and reads exactly like a security control can still have no +production consumer at all -- in which case it is a *silent security no-op*: the descriptor reads +correctly, the schema accepts it, the build is green, and the control it names applies to nothing. +Every visible signal agrees, and every one of them is about the declaration rather than the +behaviour. + +The one-line test is: *if the key were deleted entirely, would any test go red?* If the answer is +no, the control is not in effect, whatever the descriptor says. + +This note carries its own worked instance. The `tls.min_version` protocol floor and the +`tls.cipher_suites` restriction (row *G11*) were declared, valid and inert -- they had *zero* +production readers until `TlsServerCustomizer` became one, so they constrained no listener at all +while the descriptor, the schema and the build all reported health. Deleting either key at that +point would have turned nothing red. + +=== Wiring an inert key is a behavioural flip for every consumer + +The day an inert key becomes live, every consumer that reads the surface it governs changes +behaviour at once. Some test fails, and the trap is to treat that one failure as the scope of the +change. + +*The test that fails first is a sample of the dependant set, not the whole of it.* Fixing it closes +the investigation while the un-sampled dependants stay broken, and they surface later and further +from the change that caused them -- by which point the wiring commit no longer looks like a +suspect. Enumerate the dependants before declaring the wiring done, rather than letting the test +suite enumerate them for you one failure at a time. + +When the contract needs a new assertion, *extend `SingleSourceTlsContractTest`* -- or its siblings +`CipherSuiteFixtureWiringTest` and `ManagementPlainHttpActivationWiringTest` -- rather than writing +a new contract test beside them. The guards already exist, and a second one splits the contract +across two files that then drift apart. + +=== An opt-in runtime feature needs a deployment-activation test + +Unit tests prove `config -> behaviour`: given this configuration, the component does this. They do +not prove that *the deployment sets that configuration*. An opt-in feature can be fully unit-tested +and entirely off in every running instance, and no unit test can see the difference. + +The shape that closes the other half is the `*ActivationWiringTest` family this note already +indexes, with `TlsEdgeActivationWiringTest` as the reference: it asserts the *deployed descriptor +actually turns the feature on*. That is the assertion a unit test structurally cannot reach, and it +is why a feature landing without one is recorded here as a gap rather than as coverage. From 8f684c5242c512b039259c014f819aec5589759d Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:51:58 +0200 Subject: [PATCH 03/15] docs(dev): record that the plan's footprint governs in both directions Add one section to the Sonar quality-gate policy stating the two rules that answer the same question from opposite ends. A cosmetic sweep must stay out of the interior of uncovered branches, because Sonar's new-code window is the diff rather than its semantics -- a pure rename inside an uncovered catch block drags pre-existing uncovered lines into the pull request's coverage measurement. A finding inside the scanned surface but outside the declared write-boundary is dispositioned taken_into_account and reported as a named follow-up, never silently absorbed, because the widening is invisible in a gate result that only ever gets greener. Close with the page's own hygiene rule: cross-references here name documents and rules, never file:line anchors. The four-anchor deferral table that belongs here by subject is deliberately not landed -- its anchors no longer resolve and one now points at a comment recording the finding as fixed, so it is recorded as an omission with its reason rather than restored as a stale list. Co-Authored-By: Claude --- doc/development/sonar-quality-gate.adoc | 46 +++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/doc/development/sonar-quality-gate.adoc b/doc/development/sonar-quality-gate.adoc index 6548cec1..74330f40 100644 --- a/doc/development/sonar-quality-gate.adoc +++ b/doc/development/sonar-quality-gate.adoc @@ -62,6 +62,52 @@ Reaching zero by *silently marking issues won't-fix or false-positive in the Son *not acceptable* -- that decision leaves no trace in the codebase and cannot be reviewed in a pull request. Every deviation from a clean gate must be visible and justified in the source. +== The plan's footprint governs in both directions + +Two rules answer the same question from opposite ends: *what does a change do about the lines and +findings its own declared footprint does not cover?* One keeps unrelated lines out of the +measurement; the other keeps the diff from being pulled out past its boundary. + +=== Do not let a cosmetic sweep pull unrelated lines in + +Sonar's new-code window is *the diff*, not the semantics of the diff. A line touched for any reason +enters the new-code measurement, and the gate does not ask whether the change authored the +behaviour on it. + +The consequence is sharper than it sounds: a *purely cosmetic* edit inside an already-uncovered +branch drags every pre-existing uncovered line of that branch into the pull request's new-code +coverage measurement. A pure rename inside an uncovered `catch` block is enough to fail the +new-coverage condition on lines the change did not author and does not test. + +The remedy is prevention rather than repair. Keep a cosmetic sweep -- a rename, a reformat, a +comment tidy -- *out of the interior of uncovered branches*. Where the sweep has already landed and +the condition is red, the honest options are to cover the branch or to back the cosmetic edit out; +neither is cheap, which is why the rule is stated as prevention. + +=== Do not let a finding pull your diff out + +The mirror case: a finding lands *inside* the scanned surface but *outside* the plan's declared +write-boundary. + +That finding is dispositioned `taken_into_account` and reported as a *named follow-up* -- never +silently absorbed into the current change. Absorbing it widens the blast radius past the boundary +the specification declared, and the widening is *invisible in the gate result*, because a gate only +ever gets greener as more is fixed. A green gate over a change twice the size it was scoped to is +still a scope breach; the gate simply cannot report it. + +=== Cross-references here name documents, not lines + +This policy names *documents and rules*. It does not carry `file:line` anchors, and neither should +anything written into it. A line number is unowned duplicated state: it decays the moment the file +it points into is edited, and it decays *without any signal* -- nothing fails, the reference simply +starts pointing somewhere else. + +The evidence this section deliberately omits is the worked example. An earlier four-anchor deferral +table belongs here by subject and is *not* landed: its anchors no longer resolve, and one of them +now points at a comment recording that the finding was *fixed* rather than deferred -- so +re-landing the table would ship a stale deferral list that reads as current. Where a deferral list +is wanted, re-derive it from the live gate rather than restoring a decayed one. + == See also * link:../../.github/project.yml[`.github/project.yml`] -- the repository's SonarCloud declaration. From d593fb895a53e1b2fe11438ad63080720dfc1745 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:55:03 +0200 Subject: [PATCH 04/15] docs(demo-client): complete the browser-policy class and add the owed pointers The Playwright doc already carries the browser-policy rule and its generalisation; only the enumerated class was short. Add the two members it omitted -- cookie partitioning, and the navigation context a response arrives in (top-level redirect GET versus cross-site form POST) -- and state that this suite is the only gate in the repository covering that class. The navigation member is the one the page's own worked defect turned on, so the list now matches the story it sits under. Give the run-integration-tests skill the two cross-references it was missing: that a green RestAssured run here is not evidence about that class, and that a trimmed bring-up already has a working script rather than needing a Compose profile. Both are pointers -- each rule stays in the document that owns it. Co-Authored-By: Claude --- .claude/skills/run-integration-tests/SKILL.md | 5 +++++ demo-client/doc/playwright-suite.adoc | 14 +++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.claude/skills/run-integration-tests/SKILL.md b/.claude/skills/run-integration-tests/SKILL.md index c6e3cbcc..a7434ce7 100644 --- a/.claude/skills/run-integration-tests/SKILL.md +++ b/.claude/skills/run-integration-tests/SKILL.md @@ -31,6 +31,11 @@ Use a 10-minute Bash timeout. On a loaded machine the whole cycle can exceed the ``` 3. Curl the health endpoints (below) and read `docker compose logs api-sheriff`. Re-run only step 3 while iterating. +## Two pointers out of this stack + +- **A green run here is not evidence about browser-enforced controls.** These suites drive the gateway with RestAssured. `demo-client/doc/playwright-suite.adoc` states why that leaves a whole class of controls outside their reach, enumerates the class, and carries the Playwright suite that is the only gate in this repository covering it — read it before concluding anything about that class from a green run here. +- **Need a subset of this stack rather than all of it?** `demo-client/scripts/start-dev-environment.sh` already carries the working invocation for a trimmed three-container bring-up. Do not reach for a Compose profile instead: the standing prohibition, with its reasoning, is in `demo-client/doc/playwright-suite.adoc`. + ## Port & health map | Service | Container | Host | Notes | diff --git a/demo-client/doc/playwright-suite.adoc b/demo-client/doc/playwright-suite.adoc index 27f90aaf..490f622e 100644 --- a/demo-client/doc/playwright-suite.adoc +++ b/demo-client/doc/playwright-suite.adoc @@ -81,11 +81,15 @@ regardless. The suite was therefore structurally incapable of observing this def green throughout. It was not a weak test; it was a test of something else. Generalise it before adding the next control: *any control whose enforcement lives in the browser -- -`SameSite`, `Secure`, the `__Host-` prefix, CORS, `Referrer-Policy`, CSP -- cannot be proven by a -programmatic HTTP client, however faithful.* Only a real browser applies the policy. When you add or -change one, add the assertion to this Playwright suite; a green `Bff*IT` run tells you nothing about -it. That limitation is now recorded on `BffKeycloakLoginFlow` itself, so the next reader meets it at -the helper rather than in this document. +`SameSite`, `Secure`, the `__Host-` prefix, cookie partitioning, CORS, `Referrer-Policy`, CSP, and +the navigation context a response arrives in (a top-level redirect GET versus a cross-site form +POST) -- cannot be proven by a programmatic HTTP client, however faithful.* Only a real browser +applies the policy. The last member is the one the defect above turned on, and it is listed here so +the class matches the story rather than trailing it. *This Playwright suite is the only gate in this +repository that covers that class.* When you add or change one of those controls, add the assertion +here; a green `Bff*IT` run tells you nothing about it. That limitation is now recorded on +`BffKeycloakLoginFlow` itself, so the next reader meets it at the helper rather than in this +document. ==== [#_module_layout] From 77f7b2147dfb877f2d3b84a16e305fa47e56ba6a Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 14:56:48 +0200 Subject: [PATCH 05/15] docs: land three always-on statements in CLAUDE.md CLAUDE.md is the scarcest surface in the repository, so only statements that apply to most work earn a line here. Three do, and each cross-references its detail page rather than restating it. Beside the -Werror prose in the Pre-Commit Process, where a reader already has the build gate in mind: a successful build is not evidence that work happened, and a gate that exits 0 can still have changed your files -- so a review-bot suggestion is verified by surviving the gate rather than by being implemented, and unrelated churn is reverted rather than committed. The mechanisms, the operational consequences and the recipe-scoping trap stay in doc/development/build-gate-discipline.adoc. With the testing standards: a configuration key that parses is not a configuration key that acts, discriminated by asking whether deleting the key would turn any test red. The worked instance stays in doc/development/declared-limit-assertion-coverage.adoc. Co-Authored-By: Claude --- CLAUDE.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 177ef174..ac62b7f8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,6 +67,17 @@ turned on over was retired. A `@SuppressWarnings` added to get back to green hol while leaving it reporting success, which is worse than not having it — and it collides with the Pre-1.0 rule below that forbids carrying deprecated code at all. +**A successful build is not evidence that work happened.** `BUILD SUCCESS` says the build +completed — not that it compiled what you changed, ran what you wrote, or kept what you fixed. The +gate below and every specific case documented off it are instances of that one rule. + +**A gate that exits 0 can still have changed your files**, and three different mechanisms in this +repository do. So a review-bot suggestion is verified by *surviving* the gate, never by being +implemented: run the gate, then `git status --porcelain`, and attribute a dirty tree before +reverting it. Revert unrelated churn; keep the rewrite only for files the branch itself authored. +See `doc/development/build-gate-discipline.adoc` for the three mechanisms, the three operational +consequences and the recipe-scoping trap. + **Documentation-only commits skip both.** A commit whose entire footprint is prose or agent instructions cannot change build output, so a Maven run proves nothing and only burns minutes. Skip when **every** changed file is one of: @@ -119,6 +130,7 @@ one is a red `main`. - Minimum 80% coverage - CUI Test Generator for test data (`@GeneratorsSource` preferred) - **Forbidden**: Mockito, PowerMock, Hamcrest +- **A configuration key that parses is not a configuration key that acts.** Ask: *if the key were deleted entirely, would any test go red?* If not, the control it names is not in effect — see `doc/development/declared-limit-assertion-coverage.adoc` ### Javadoc From dea85c536da5330d7f14c8fb22dbae52772d4ace Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:16:33 +0200 Subject: [PATCH 06/15] ci: report the OpenRewrite dirty tree without gating on it Three mechanisms in this repository mutate the working tree while the gate exits 0, and all three are invisible in the build result by construction. Add a rewrite-report job to the Maven workflow that runs the rewriting gate and prints, into the job summary, the post-gate git status --porcelain listing and every "Changes have been made to by:" block with its recipe chain. The job is patterned on supply-chain-scan: no needs:, hardened runner, checkout without persisted credentials, and deliberately non-gating on its own verdict. It carries no continue-on-error, so a genuine Maven or tooling failure still turns the check red; only the dirty verdict is suppressed, and it is suppressed structurally -- no step asserts cleanliness, so there is no failing path to mask. shell: bash is load-bearing on both run steps: it selects pipefail, and without it the tee would swallow a Maven failure. -Ppre-commit is load-bearing too, and the leading comment says so: the recipe list that reaches all three mechanisms is the pre-commit profile's activeRecipes override, and without the profile the goal reports "Using active recipe(s) []" and exonerates OpenRewrite for free. The same comment records why this lane is deliberately not the positively-phrased fitness function ADR-0030 prescribes, and the condition under which it could become one. The job commits nothing and pushes nothing; the mutated tree lives only in the runner. Co-Authored-By: Claude --- .github/workflows/maven.yml | 87 +++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 5ad9156c..a021b714 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -156,3 +156,90 @@ jobs: trivy-filesystem.sarif retention-days: 30 if-no-files-found: error + + # OPENREWRITE DIRTY-TREE REPORT — deliberately NON-GATING. + # + # Three mechanisms in this repository mutate the working tree while the gate exits 0, and all three + # are invisible in the build result by construction. They are documented in + # doc/development/build-gate-discipline.adoc; this job is the only part of that story a machine can + # carry, so it PRINTS what the rewriting gate changed and which recipe changed it, and never judges. + # + # `-Ppre-commit` IS LOAD-BEARING. The recipe list that reaches all three mechanisms is the + # pre-commit profile's `activeRecipes` override in the root pom.xml. Without the profile, + # `rewrite:run` reports `Using active recipe(s) []`, changes nothing, and this job becomes a false + # green that appears to exonerate OpenRewrite. Do not "simplify" the profile away. + # + # No CI lane runs the rewriting gate today, so this job has to invoke it before it has a dirty tree + # to report. That is CI time this repository did not previously spend; the invocation is kept to the + # narrowest form that still produces all three mutations. + # + # DELIBERATE DIVERGENCE FROM ADR-0030, which holds that a repository invariant asserted only by an + # explanatory comment becomes a positively-phrased, machine-checked fitness function. This lane is + # deliberately NOT that shape: a gate asserting "the tree is clean after the rewriting gate" would be + # red on every run for as long as `main` is not the formatter's fixed point. Bringing it there is a + # mechanical rewrite of api-sheriff/src/main/java/** that no plan has done — and doing it is exactly + # the condition under which this lane could be upgraded to ADR-0030's shape. + # + # No `needs:` — patterned on supply-chain-scan above; the job neither delays nor is delayed by + # `build`. It commits nothing and pushes nothing: the mutated tree exists only inside the runner. + rewrite-report: + name: OpenRewrite dirty-tree report (non-gating) + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: read + + steps: + - name: Harden the runner (Audit all outbound calls) + uses: step-security/harden-runner@b09bb98e06d4d774595224525879c09bc6e98c40 # v2.20.1 + with: + egress-policy: audit + + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Set up JDK 25 + uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0 + with: + java-version: '25' + distribution: 'temurin' + cache: maven + + # `shell: bash` is required, not decorative: it selects `-eo pipefail`, whereas the default + # `bash -e` leaves pipefail OFF and the `| tee` below would swallow a Maven failure. A genuine + # gate or tooling failure must still turn this check red — only the DIRTY VERDICT is non-gating, + # which is why `continue-on-error` is deliberately not used on this job or any of its steps. + - name: Run the rewriting gate + shell: bash + run: | + ./mvnw -B -ntp -Ppre-commit rewrite:run -DskipTests 2>&1 | tee rewrite-run.log + + # Report, without a verdict. Nothing here asserts cleanliness, so there is no `exit 1` path: + # a dirty tree produces a longer summary and a green check, which is the whole point. + - name: Publish the dirty-tree report to the job summary + shell: bash + run: | + { + echo "## OpenRewrite dirty-tree report (non-gating)" + echo + echo "The findings below NEVER fail this check. They record what the rewriting gate" + echo "changed in the runner's working tree while exiting 0. See" + echo "\`doc/development/build-gate-discipline.adoc\` for what to do about them." + echo + echo "### Working tree after the gate" + echo '```' + git status --porcelain + echo '```' + echo + echo "### Recipe attribution" + echo '```' + awk ' + /Changes have been made to / { show = 1; print; next } + show && /^\[INFO\][[:space:]][[:space:]]+/ { print; next } + show && /^[[:space:]][[:space:]]+/ { print; next } + { show = 0 } + ' rewrite-run.log + echo '```' + } >> "$GITHUB_STEP_SUMMARY" From 2ad0e52ae48d57ff00f00ce9b678d5dd05c140b9 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:26:24 +0200 Subject: [PATCH 07/15] docs(dev): restate the fixed point as observed state plus drift risk The page asserted that main is not at the OpenRewrite recipes' fixed point and recorded reaching it as unpaid debt. A whole-reactor verify -Ppre-commit run drives rewrite:run with the full pre-commit recipe list in every module, produces zero 'Changes have been made to' attributions and leaves the tree clean, so the premise was false. Restate every fixed-point passage as the current observed state plus the drift risk: the fixed point is a property of the tree at a point in time, lost silently the moment a change the recipes want to rewrite lands. The three mechanisms, the three operational consequences and the revert rule are unchanged; the revert rule's reasoning is re-grounded on that drift risk rather than on standing debt. --- doc/development/build-gate-discipline.adoc | 58 ++++++++++++++-------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/doc/development/build-gate-discipline.adoc b/doc/development/build-gate-discipline.adoc index f6b9660a..99541423 100644 --- a/doc/development/build-gate-discipline.adoc +++ b/doc/development/build-gate-discipline.adoc @@ -15,6 +15,14 @@ Three distinct mechanisms in this repository mutate the working tree during a gr `verify -Ppre-commit` run. None of them is visible in the build result -- that is the whole difficulty. `BUILD SUCCESS` is printed either way. +Where the tree stands today: a whole-reactor `verify -Ppre-commit` drives `rewrite:run` with the +full `pre-commit` recipe list in every module, produces *zero* `Changes have been made to` +attributions, and leaves the working tree clean. The repository is at the active recipes' fixed +point. That is a property of the tree at a point in time, not a guarantee -- the fixed point is +lost the moment a change lands that the recipes want to rewrite, and nothing announces the loss. +The three mechanisms below are what happens on that run. They are dormant, not retired, and every +rule on this page is written for the run on which they wake up. + === OpenRewrite import-group churn The `pre-commit` profile's `activeRecipes` override in the root `pom.xml` activates @@ -30,13 +38,15 @@ interaction, not a lost build. === The pre-commit formatter is not idempotent -`org.openrewrite.java.format.AutoFormat` and its companions do not converge in one pass on this -tree: a run takes A to B, and the next run takes B to C rather than leaving B alone. The root -cause is that `main` is not at the formatter's fixed point, so every run has real work to do and -the work is not stable. +`org.openrewrite.java.format.AutoFormat` and its companions do not converge in one pass: on a tree +away from their fixed point, a run takes A to B, and the next run takes B to C rather than leaving +B alone. What was observed when this was diagnosed was a `main` that gave every run real work to +do, and the work was not stable. -The practical consequence is that "run the formatter again and see if it settles" is not a -diagnostic here. It will not settle. +`main` is at the fixed point today, so the recipes have nothing to do and the instability has +nothing to act on. It comes back with the first change the recipes want to rewrite. The practical +consequence, on any run that does have work to do, is that "run the formatter again and see if it +settles" is not a diagnostic here. It will not settle. === `SimplifyTestThrows` silently reverts an accepted narrowing @@ -83,17 +93,18 @@ the formatter will accept -- there is not one. *Revert unrelated churn; keep the rewrite only for files the branch itself authored.* A scoped pull request carries the files its change is about. -The countervailing argument is real and should be stated rather than ignored: the reverted diff -*will reappear on the next run*, so reverting it is work that has to be repeated. That is true, and -it is not a reason to commit the churn. The reappearance is the standing cost of `main` not being -at the formatter's fixed point, and paying that cost once per pull request is cheaper than -polluting every scoped pull request with roughly 170 unrelated files -- which destroys the -reviewability of each one and buries the actual change. +The countervailing argument is real and should be stated rather than ignored: on a tree away from +the fixed point the reverted diff *will reappear on the next run*, so reverting it is work that has +to be repeated. That is true, and it is not a reason to commit the churn. Paying that cost once per +pull request is cheaper than polluting every scoped pull request with roughly 170 unrelated files +-- which destroys the reviewability of each one and buries the actual change. -The durable fix is to bring `main` to the formatter's fixed point in a single dedicated pass. That -has not been done. It is recorded here as an open debt rather than left as folklore, and it is -deliberately out of scope for any scoped change, because it is a mechanical rewrite of -`api-sheriff/src/main/java/**` and belongs in a change that does nothing else. +The rule stands even though the tree is at the fixed point today, and it stands *because* of how +that fixed point is held: it is a property of the current tree rather than a guarantee about future +ones, and it is lost silently the first time a change the recipes want to rewrite lands. On that +run the churn is back and the revert-versus-commit question is live again, with the same answer. +Running `git status --porcelain` after the gate is what tells a contributor which kind of run they +are on -- there is no other signal, because the gate exits 0 either way. == The recipe-scoping trap @@ -113,10 +124,17 @@ carries a `rewrite-report` job, named "OpenRewrite dirty-tree report (non-gating rewriting gate itself and then prints, into the job summary, the post-gate `git status --porcelain` listing and every `Changes have been made to by:` block with its recipe chain. -The job *deliberately never fails the build on a dirty verdict*. A blocking assertion over "the -tree is clean after the rewriting gate" would be red on every run for as long as `main` is not at -the formatter's fixed point -- so the job reports and does not judge. A genuine Maven or tooling -failure still turns the check red; only the dirty verdict is non-gating. +The job *deliberately never fails the build on a dirty verdict*. The tree is observed clean at the +fixed point today, but that is not guaranteed to hold, and a hard fail-on-dirty gate would block +every pull request from the moment any drift lands -- including the pull requests that did not +cause it. So the job reports the dirty set and the recipe attribution and leaves the disposition to +the author. A genuine Maven or tooling failure still turns the check red; only the dirty verdict is +non-gating. + +The condition under which the ADR-0030 fitness-function shape becomes the right call is a fixed +point that is *held* rather than merely observed: once drift is caught and corrected by the change +that introduces it, a positively-phrased "the tree is clean after the rewriting gate" assertion +stops being a blanket blocker and becomes an invariant worth asserting. == A build whose output says it did nothing has verified nothing From 62b3f24d8e9d556295c2a7f3d2215c27a9a621a2 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:27:20 +0200 Subject: [PATCH 08/15] ci: correct the rewrite-report job's non-gating rationale The job stays a report, not a gate; only its leading comment changes. The old rationale claimed a fail-on-dirty assertion would be red on every run for as long as main is not the formatter's fixed point, which is false - the tree is observed clean at the active recipes' fixed point. The real reason to stay non-gating is that the observation is not a guarantee: a hard gate would block every pull request from the moment any drift lands, including the ones that did not cause it. The comment now also names the condition under which the ADR-0030 fitness-function shape becomes the right call. --- .github/workflows/maven.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index a021b714..41662d74 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -175,10 +175,14 @@ jobs: # # DELIBERATE DIVERGENCE FROM ADR-0030, which holds that a repository invariant asserted only by an # explanatory comment becomes a positively-phrased, machine-checked fitness function. This lane is - # deliberately NOT that shape: a gate asserting "the tree is clean after the rewriting gate" would be - # red on every run for as long as `main` is not the formatter's fixed point. Bringing it there is a - # mechanical rewrite of api-sheriff/src/main/java/** that no plan has done — and doing it is exactly - # the condition under which this lane could be upgraded to ADR-0030's shape. + # deliberately NOT that shape. The tree is observed clean at the active recipes' fixed point today, + # but that is an observation about the current tree and not a guarantee: a hard fail-on-dirty gate + # would block every pull request from the moment any drift lands, including the ones that did not + # cause it. So this job reports the dirty set and the OpenRewrite recipe attribution and leaves the + # disposition to the author. It could be upgraded to ADR-0030's shape once the fixed point is HELD + # rather than merely observed — i.e. once drift is caught and corrected by the change that + # introduces it, so that "the tree is clean after the rewriting gate" is an invariant rather than a + # blanket blocker. # # No `needs:` — patterned on supply-chain-scan above; the job neither delays nor is delayed by # `build`. It commits nothing and pushes nothing: the mutated tree exists only inside the runner. From bdb13463bd0a46cf703b8e86c8c663522f728c00 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 16:11:43 +0200 Subject: [PATCH 09/15] chore(simplify): collapse duplicated rationale and fix a stale count The rewrite-report job's ADR-0030 divergence comment restated, near-verbatim, the argument already carried by doc/development/build-gate-discipline.adoc while its own header comment declares that adoc as the home for the story. Collapse it to what is genuinely site-unique: the ADR-0030 summary, the divergence flag, the one-clause reason, the pointer, and the prohibition against converting the job into a hard gate. Comments only, no behaviour change. Also correct doc/development/README.adoc's "twelve-file descriptor surface" to thirteen. The page it describes states thirteen and that count is machine-checked by DescriptorInventoryWiringTest, so the README line was provably stale. Co-Authored-By: Claude --- .github/workflows/maven.yml | 12 ++++-------- doc/development/README.adoc | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 41662d74..e4570e96 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -175,14 +175,10 @@ jobs: # # DELIBERATE DIVERGENCE FROM ADR-0030, which holds that a repository invariant asserted only by an # explanatory comment becomes a positively-phrased, machine-checked fitness function. This lane is - # deliberately NOT that shape. The tree is observed clean at the active recipes' fixed point today, - # but that is an observation about the current tree and not a guarantee: a hard fail-on-dirty gate - # would block every pull request from the moment any drift lands, including the ones that did not - # cause it. So this job reports the dirty set and the OpenRewrite recipe attribution and leaves the - # disposition to the author. It could be upgraded to ADR-0030's shape once the fixed point is HELD - # rather than merely observed — i.e. once drift is caught and corrected by the change that - # introduces it, so that "the tree is clean after the rewriting gate" is an invariant rather than a - # blanket blocker. + # deliberately NOT that shape, because the fixed point is observed rather than held. The argument, + # and the condition under which ADR-0030's shape becomes the right call, are in + # doc/development/build-gate-discipline.adoc under "The enforcement" — read it before converting + # this job into a hard fail-on-dirty gate. # # No `needs:` — patterned on supply-chain-scan above; the job neither delays nor is delayed by # `build`. It commits nothing and pushes nothing: the mutated tree exists only inside the runner. diff --git a/doc/development/README.adoc b/doc/development/README.adoc index d7938a69..cdeb6265 100644 --- a/doc/development/README.adoc +++ b/doc/development/README.adoc @@ -80,7 +80,7 @@ to *run* the finished gateway). `docker-compose.yml` and the mounted `gateway.yaml`, which stay the authoritative sources. | link:declared-limit-assertion-coverage.adoc[Declared-Limit Assertion Coverage] -| Which declared gateway limit has a boundary assertion and which does not -- the twelve-file +| Which declared gateway limit has a boundary assertion and which does not -- the thirteen-file descriptor surface, the strict preset's resolved caps that appear in no yaml, one row per limit naming its enforcing component and its asserting test, the standing gap remainder, and the two report-only findings -- that the body-cap wiring guard's glob never reaches the `endpoints/` tree, From 120a2d8dd34cf923a34c21741438fc72a761f255 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 16:55:40 +0200 Subject: [PATCH 10/15] fix(ci): invoke the rewriting gate by coordinates, not by goal prefix The rewrite-report job died on its first CI run with No plugin found for prefix 'rewrite' after Maven Central answered 429 for org.openrewrite.maven/maven-metadata.xml. Resolving a goal PREFIX requires that metadata fetch even though the plugin is declared in the pre-commit profile, so the job carried a network dependency it does not otherwise need and never reached the gate at all. Invoke org.openrewrite.maven:rewrite-maven-plugin:run by full coordinates instead. The version still resolves from the profile's own plugin declaration, and no prefix metadata is fetched. Verified locally: the run reports the full 19-recipe active list (not the empty-recipe false negative the profile omission produces), exits 0, and leaves the tree clean. The job stays non-gating for the DIRTY VERDICT only; a genuine tooling failure turning it red is the designed behaviour, and this run was exactly that. Co-Authored-By: Claude --- .github/workflows/maven.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index e4570e96..4bffe991 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -211,10 +211,15 @@ jobs: # `bash -e` leaves pipefail OFF and the `| tee` below would swallow a Maven failure. A genuine # gate or tooling failure must still turn this check red — only the DIRTY VERDICT is non-gating, # which is why `continue-on-error` is deliberately not used on this job or any of its steps. + # The goal is invoked by FULL COORDINATES, not by the `rewrite:` prefix. A prefix has to be + # resolved through `org.openrewrite.maven/maven-metadata.xml` on Central, which is a network + # fetch this job does not otherwise need — and when Central answers 429 the prefix does not + # resolve, the gate never runs, and the job dies with `No plugin found for prefix 'rewrite'`. + # Full coordinates resolve the version from the pre-commit profile's own plugin declaration. - name: Run the rewriting gate shell: bash run: | - ./mvnw -B -ntp -Ppre-commit rewrite:run -DskipTests 2>&1 | tee rewrite-run.log + ./mvnw -B -ntp -Ppre-commit org.openrewrite.maven:rewrite-maven-plugin:run -DskipTests 2>&1 | tee rewrite-run.log # Report, without a verdict. Nothing here asserts cleanliness, so there is no `exit 1` path: # a dirty tree produces a longer summary and a green check, which is the whole point. From 58294abeaf433bd983cce4e1b043097abad1f53f Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:43:46 +0200 Subject: [PATCH 11/15] ci(rewrite-report): keep the gate log out of the tree report it feeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rewriting gate teed its log to `rewrite-run.log` in the checkout root. That path is not gitignored, so the next step's `git status --porcelain` printed `?? rewrite-run.log` under "Working tree after the gate" — the job whose entire purpose is to attribute tree mutations to OpenRewrite was manufacturing one mutation of its own and reporting it beside the real ones, with nothing to tell a reader the artefact from the finding. It also broke the CLAUDE.md rule that generated files live under `.plan/temp/`. The log now goes to `.plan/temp/rewrite-run.log`, which `.gitignore:20` already excludes, and the awk reads that same path. No new ignore rule. Harden the attribution parser while here. It is coupled to OpenRewrite's log wording and indentation, and that coupling cannot be removed short of reimplementing the plugin's reporting — but the SILENCE can be. When either moves, the matcher stops matching and the empty attribution block reads exactly like "no recipe changed anything": a confident report of nothing having happened. So the header and recipe-line match counts are now cross-checked against the working tree, and two drift shapes are called out as a BROKEN PARSER rather than a clean run — a dirty tree with zero matched headers (wording moved), and headers with no recipe lines beneath them (indentation moved). Both banners name the file to fix and point the reader at the raw gate log meanwhile. The job's non-gating shape is untouched: no `continue-on-error`, no `needs:`, `-Ppre-commit` and the full-coordinate plugin invocation unchanged, and the every-change trigger set deliberately kept (decisions 231a6b, 4d66ab, 671ad2). Co-Authored-By: Claude --- .github/workflows/maven.yml | 56 +++++++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 4bffe991..94ffc530 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -216,16 +216,39 @@ jobs: # fetch this job does not otherwise need — and when Central answers 429 the prefix does not # resolve, the gate never runs, and the job dies with `No plugin found for prefix 'rewrite'`. # Full coordinates resolve the version from the pre-commit profile's own plugin declaration. + # The log goes to `.plan/temp/`, which `.gitignore:20` already excludes, NOT to the checkout + # root. A log in the root is picked up by the `git status --porcelain` below as `?? rewrite-run.log` + # — the job would manufacture one tree mutation of its own and report it next to the real ones, + # leaving a reader unable to tell the artefact from the finding. - name: Run the rewriting gate shell: bash run: | - ./mvnw -B -ntp -Ppre-commit org.openrewrite.maven:rewrite-maven-plugin:run -DskipTests 2>&1 | tee rewrite-run.log + mkdir -p .plan/temp + ./mvnw -B -ntp -Ppre-commit org.openrewrite.maven:rewrite-maven-plugin:run -DskipTests 2>&1 | tee .plan/temp/rewrite-run.log # Report, without a verdict. Nothing here asserts cleanliness, so there is no `exit 1` path: # a dirty tree produces a longer summary and a green check, which is the whole point. + # + # The attribution parser is coupled to OpenRewrite's log wording and to its indentation, and + # that coupling cannot be removed short of reimplementing the plugin's reporting. What CAN be + # removed is the SILENCE. When the wording or the indentation moves, the matcher stops matching + # and an empty attribution block reads exactly like "no recipe changed anything" — a confident + # report of nothing having happened. So the match counts are cross-checked against the working + # tree: a dirty tree with zero matched headers, or headers with no recipe lines beneath them, + # is reported as a BROKEN PARSER and never as a clean run. - name: Publish the dirty-tree report to the job summary shell: bash run: | + tree_status="$(git status --porcelain)" + counts="$(awk -v out=.plan/temp/attribution.txt ' + /Changes have been made to / { block = 1; headers++; print > out; next } + block && /^\[INFO\][[:space:]][[:space:]]+/ { detail++; print > out; next } + block && /^[[:space:]][[:space:]]+/ { detail++; print > out; next } + { block = 0 } + END { print headers + 0, detail + 0 } + ' .plan/temp/rewrite-run.log)" + headers="${counts% *}" + detail="${counts#* }" { echo "## OpenRewrite dirty-tree report (non-gating)" echo @@ -235,16 +258,33 @@ jobs: echo echo "### Working tree after the gate" echo '```' - git status --porcelain + if [ -n "$tree_status" ]; then + printf '%s\n' "$tree_status" + else + echo "(clean - the gate rewrote nothing)" + fi echo '```' echo echo "### Recipe attribution" echo '```' - awk ' - /Changes have been made to / { show = 1; print; next } - show && /^\[INFO\][[:space:]][[:space:]]+/ { print; next } - show && /^[[:space:]][[:space:]]+/ { print; next } - { show = 0 } - ' rewrite-run.log + if [ "$headers" -gt 0 ]; then + cat .plan/temp/attribution.txt + else + echo "(no recipe reported a change)" + fi echo '```' + if [ "$headers" -eq 0 ] && [ -n "$tree_status" ]; then + echo + echo "> **Parser drift, not a clean run.** The tree above is dirty, yet no" + echo "> \`Changes have been made to\` line matched. Do NOT read the empty attribution as" + echo "> \"no recipe ran\" — OpenRewrite's log wording has most likely moved. Attribute the" + echo "> changes from the raw gate log in the previous step, and fix the awk matcher in" + echo "> \`.github/workflows/maven.yml\`." + elif [ "$headers" -gt 0 ] && [ "$detail" -eq 0 ]; then + echo + echo "> **Parser drift, partial match.** $headers change header(s) matched but no recipe" + echo "> line beneath them did, so the attribution above names no recipe. OpenRewrite's log" + echo "> indentation has most likely moved; fix the awk matcher in" + echo "> \`.github/workflows/maven.yml\`." + fi } >> "$GITHUB_STEP_SUMMARY" From c51f33efc6b68aa250e3b63f5b82f9457fd5fe40 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:47:51 +0200 Subject: [PATCH 12/15] docs(gate): deletion test proves coverage, not runtime effect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deletion discriminator overstated its own conclusion: "if the key were deleted entirely, would any test go red? If the answer is no, the control is not in effect, whatever the descriptor says." A green suite after deletion proves the key is not TEST-PROVEN. It says nothing about whether a production reader consumes it, and the inference fails two ways — a production consumer with no asserting test, and a default or inherited value that preserves the behaviour once the explicit key is gone. The page refuted itself three rows up. Its own matrix lists G7 (strict preset path length cap, enforced by `BasicChecksStage.validatePath`), G8 (`tls.alpn`, enforced by `TlsServerCustomizer`) and G9 (`oidc.session.ttl_seconds`, enforced by the session store) each with `none` in the asserting-test column. Applying the one-line test to those three yielded "not in effect" for three controls the same page says ARE enforced — the claim-versus-mechanism defect this document exists to catch, sitting inside the document. The discriminator now stops where its evidence stops: not test-proven, with the two green-but-live shapes named and the G7/G8/G9 rows cited as the instance, and production consumption confirmed separately by tracing the key to its reader. The G11 worked instance stays and gains the second step that actually settled it — the trace found no reader. The sibling paragraph had the same root cause: "Some test fails" assumed a test exists to fail. Where the surface has no asserting test, wiring an inert key flips behaviour with nothing going red at all. Its real point — the first failure is a SAMPLE of the dependant set, not its extent — is unchanged. CLAUDE.md:133 carried the compressed form of the same invalid inference and is restated to match. The pointer must not outrun the text it points at. The silent-security-no-op warning, the matrix rows and the GAP statuses are untouched; only the strength of the deletion test's conclusion was wrong. Co-Authored-By: Claude --- CLAUDE.md | 2 +- .../declared-limit-assertion-coverage.adoc | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ac62b7f8..faa84ff1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -130,7 +130,7 @@ one is a red `main`. - Minimum 80% coverage - CUI Test Generator for test data (`@GeneratorsSource` preferred) - **Forbidden**: Mockito, PowerMock, Hamcrest -- **A configuration key that parses is not a configuration key that acts.** Ask: *if the key were deleted entirely, would any test go red?* If not, the control it names is not in effect — see `doc/development/declared-limit-assertion-coverage.adoc` +- **A configuration key that parses is not a configuration key that acts.** Ask: *if the key were deleted entirely, would any test go red?* If not, the control it names is not test-proven — that is all a green suite settles, so trace the key to its production reader before concluding anything about whether it is in effect — see `doc/development/declared-limit-assertion-coverage.adoc` ### Javadoc diff --git a/doc/development/declared-limit-assertion-coverage.adoc b/doc/development/declared-limit-assertion-coverage.adoc index 0c273be5..3574b406 100644 --- a/doc/development/declared-limit-assertion-coverage.adoc +++ b/doc/development/declared-limit-assertion-coverage.adoc @@ -559,19 +559,27 @@ Every visible signal agrees, and every one of them is about the declaration rath behaviour. The one-line test is: *if the key were deleted entirely, would any test go red?* If the answer is -no, the control is not in effect, whatever the descriptor says. +no, the control is *not test-proven* -- and that is the whole of what the test settles. It does not +settle whether the control is in effect. Two shapes come back green and are still live: a key with a +production reader but no asserting test -- rows *G7*, *G8* and *G9* of the matrix above are exactly +that, each naming an enforcer and `none` in the asserting-test column -- and a key whose default or +inherited value carries the same behaviour once the explicit declaration is gone. So the deletion +test is the first of two steps, not the verdict. Confirm production consumption separately, by +tracing the key to the reader that consumes it. This note carries its own worked instance. The `tls.min_version` protocol floor and the `tls.cipher_suites` restriction (row *G11*) were declared, valid and inert -- they had *zero* production readers until `TlsServerCustomizer` became one, so they constrained no listener at all while the descriptor, the schema and the build all reported health. Deleting either key at that -point would have turned nothing red. +point would have turned nothing red. The deletion test is what raised the suspicion; the trace to a +production reader -- finding none -- is what settled it. === Wiring an inert key is a behavioural flip for every consumer The day an inert key becomes live, every consumer that reads the surface it governs changes -behaviour at once. Some test fails, and the trap is to treat that one failure as the scope of the -change. +behaviour at once. An existing test may fail, and the trap is to treat that one failure as the scope +of the change. Where the surface carries no asserting test at all -- rows *G7*, *G8* and *G9* again +-- the flip lands with nothing going red, and the trap has no trigger to spring. *The test that fails first is a sample of the dependant set, not the whole of it.* Fixing it closes the investigation while the un-sampled dependants stay broken, and they surface later and further From c23b92660d77971bc7dba571666a3eff94be05d0 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:50:46 +0200 Subject: [PATCH 13/15] docs(demo-client): the Playwright suite is coverage, not a gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `playwright-suite.adoc:88` called the suite "the only gate in this repository that covers that class". It gates nothing. `.github/workflows/demo-client-e2e.yml` triggers on `push: branches: [main]` and `workflow_dispatch` only — there is no `pull_request` trigger, so a pull request that breaks SameSite, Secure, the __Host- prefix, cookie partitioning, CORS, Referrer-Policy, CSP or navigation context can merge with this suite never having run. The file already said so, 338 lines later: "It is deliberately not attached to any `pull_request` trigger". Line 88 and line 426 could not both be true, and a reader who stopped at the first one came away believing a control was enforced that is not. The mechanism is right and stays. The opt-in design is deliberate and argued in the workflow's own leading comment: the suite builds a native image, starts containers and downloads a browser toolchain, and putting that on the default PR path would slow every PR for a demo-client signal. No `pull_request` trigger is added here. The claim is what changes. Line 88 now says the suite is the only COVERAGE of that class and that it gates nothing, and states the non-gating half at the point of claim rather than 338 lines downstream — so the later passage reads as elaboration instead of refutation. The sibling claim in `.claude/skills/run-integration-tests/SKILL.md:36` gets the same correction. Co-Authored-By: Claude --- .claude/skills/run-integration-tests/SKILL.md | 2 +- demo-client/doc/playwright-suite.adoc | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.claude/skills/run-integration-tests/SKILL.md b/.claude/skills/run-integration-tests/SKILL.md index a7434ce7..907947c4 100644 --- a/.claude/skills/run-integration-tests/SKILL.md +++ b/.claude/skills/run-integration-tests/SKILL.md @@ -33,7 +33,7 @@ Use a 10-minute Bash timeout. On a loaded machine the whole cycle can exceed the ## Two pointers out of this stack -- **A green run here is not evidence about browser-enforced controls.** These suites drive the gateway with RestAssured. `demo-client/doc/playwright-suite.adoc` states why that leaves a whole class of controls outside their reach, enumerates the class, and carries the Playwright suite that is the only gate in this repository covering it — read it before concluding anything about that class from a green run here. +- **A green run here is not evidence about browser-enforced controls.** These suites drive the gateway with RestAssured. `demo-client/doc/playwright-suite.adoc` states why that leaves a whole class of controls outside their reach, enumerates the class, and carries the Playwright suite that is the only coverage in this repository for it — a suite that gates nothing, since it runs on pushes to `main` and on demand but never on a pull request. Read it before concluding anything about that class from a green run here. - **Need a subset of this stack rather than all of it?** `demo-client/scripts/start-dev-environment.sh` already carries the working invocation for a trimmed three-container bring-up. Do not reach for a Compose profile instead: the standing prohibition, with its reasoning, is in `demo-client/doc/playwright-suite.adoc`. ## Port & health map diff --git a/demo-client/doc/playwright-suite.adoc b/demo-client/doc/playwright-suite.adoc index 490f622e..2dce603b 100644 --- a/demo-client/doc/playwright-suite.adoc +++ b/demo-client/doc/playwright-suite.adoc @@ -85,11 +85,14 @@ Generalise it before adding the next control: *any control whose enforcement liv the navigation context a response arrives in (a top-level redirect GET versus a cross-site form POST) -- cannot be proven by a programmatic HTTP client, however faithful.* Only a real browser applies the policy. The last member is the one the defect above turned on, and it is listed here so -the class matches the story rather than trailing it. *This Playwright suite is the only gate in this -repository that covers that class.* When you add or change one of those controls, add the assertion -here; a green `Bff*IT` run tells you nothing about it. That limitation is now recorded on -`BffKeycloakLoginFlow` itself, so the next reader meets it at the helper rather than in this -document. +the class matches the story rather than trailing it. *This Playwright suite is the only suite in +this repository that covers that class -- and it gates nothing.* It runs on pushes to `main` and on +demand, never on a pull request, so a pull request that breaks one of those controls can merge with +this suite never having run. Coverage and enforcement are separate things here, and only the first +one exists. When you add or change one of those controls, add the assertion here and run the suite +deliberately; a green `Bff*IT` run tells you nothing about it. That RestAssured limitation is now +recorded on `BffKeycloakLoginFlow` itself, so the next reader meets it at the helper rather than in +this document. ==== [#_module_layout] From dfa332aa9a8d4bb9fdb4caac0c452e9af2f79ab0 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:53:13 +0200 Subject: [PATCH 14/15] docs(sonar): scope the new-code claim to changed lines, name the disposition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two precision defects in text this branch authored. Neither changes the policy. The cosmetic-sweep section opens correctly — "Sonar's new-code window is the diff" — and then contradicts itself three lines later, claiming a cosmetic edit inside an uncovered branch "drags every pre-existing uncovered line of that branch into the pull request's new-code coverage measurement". Pull-request analysis is scoped to the CHANGED lines; unchanged siblings in the same branch do not enter the measurement. The warning underneath is real and survives intact: a rename or reformat inside an uncovered branch turns the lines it touched into uncovered new code, which is enough on its own to fail the new-coverage condition on lines the change did not author. It is now stated over the changed lines, so the section agrees with its own opening sentence. The prevention remedy is unchanged. The `taken_into_account` passage asserted a disposition and a "named follow-up" without saying where either is recorded, leaving a reader unable to check that the rule is real. It is real, but only half of it is mechanised, and the halves are now stated separately: the disposition is persisted outside this repository, by plan-marshall's triage tooling, in the plan's findings ledger via `manage-findings resolve --resolution taken_into_account`; the named follow-up has NO automated enforcement here — no workflow check, no plan-validation rule — and is labelled a process rule. No CI check is invented to back it. A section whose whole argument is that a green gate cannot report a scope breach must not itself assert an enforcement that does not exist. Co-Authored-By: Claude --- doc/development/sonar-quality-gate.adoc | 29 ++++++++++++++++++------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/doc/development/sonar-quality-gate.adoc b/doc/development/sonar-quality-gate.adoc index 74330f40..e532dd66 100644 --- a/doc/development/sonar-quality-gate.adoc +++ b/doc/development/sonar-quality-gate.adoc @@ -74,10 +74,12 @@ Sonar's new-code window is *the diff*, not the semantics of the diff. A line tou enters the new-code measurement, and the gate does not ask whether the change authored the behaviour on it. -The consequence is sharper than it sounds: a *purely cosmetic* edit inside an already-uncovered -branch drags every pre-existing uncovered line of that branch into the pull request's new-code -coverage measurement. A pure rename inside an uncovered `catch` block is enough to fail the -new-coverage condition on lines the change did not author and does not test. +The consequence is sharper than it sounds, and the sentence above also bounds it: a *purely +cosmetic* edit inside an already-uncovered branch turns *the lines it changed* into uncovered new +code. Pull-request analysis is scoped to the changed lines, so the untouched siblings in that branch +never enter the measurement -- the window really is the diff -- but the changed lines are enough on +their own. A pure rename inside an uncovered `catch` block can fail the new-coverage condition on +lines the change did not author and does not test. The remedy is prevention rather than repair. Keep a cosmetic sweep -- a rename, a reformat, a comment tidy -- *out of the interior of uncovered branches*. Where the sweep has already landed and @@ -90,10 +92,21 @@ The mirror case: a finding lands *inside* the scanned surface but *outside* the write-boundary. That finding is dispositioned `taken_into_account` and reported as a *named follow-up* -- never -silently absorbed into the current change. Absorbing it widens the blast radius past the boundary -the specification declared, and the widening is *invisible in the gate result*, because a gate only -ever gets greener as more is fixed. A green gate over a change twice the size it was scoped to is -still a scope breach; the gate simply cannot report it. +silently absorbed into the current change. + +The two halves of that rule are backed very differently, and the difference is worth stating. The +*disposition* is mechanised, though not in this repository: plan-marshall's triage tooling records it +in the plan's findings ledger via `manage-findings resolve --resolution taken_into_account`, one of +that store's resolution values, so the decision survives as an artefact rather than an intention. +The *named follow-up* is not mechanised at all. No workflow check and no plan-validation rule here +asserts that the follow-up was actually filed; it is a process rule, kept by whoever runs the +triage. + +Naming that boundary is not a caveat, it is the point. Absorbing the finding widens the blast radius +past the boundary the specification declared, and the widening is *invisible in the gate result*, +because a gate only ever gets greener as more is fixed. A green gate over a change twice the size it +was scoped to is still a scope breach; the gate simply cannot report it. A section built on that +observation must not itself claim an enforcement it does not have. === Cross-references here name documents, not lines From e03b6a614241b345fbbdee7249296530e983c2f8 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:30:41 +0200 Subject: [PATCH 15/15] chore(simplify): drop a line-number anchor from the rewrite-report comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment added in 320f09c cited `.gitignore:20` for the rule that excludes the rewrite log from the checkout. The coordinate is accurate today and decays without any signal the moment a line is inserted above it — and this same changeset lands the section in doc/development/sonar-quality-gate.adoc that condemns exactly that: cross-references here name documents, not lines. The claim is preserved; only the decaying coordinate is gone. Co-Authored-By: Claude --- .github/workflows/maven.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 94ffc530..973069b2 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -216,7 +216,7 @@ jobs: # fetch this job does not otherwise need — and when Central answers 429 the prefix does not # resolve, the gate never runs, and the job dies with `No plugin found for prefix 'rewrite'`. # Full coordinates resolve the version from the pre-commit profile's own plugin declaration. - # The log goes to `.plan/temp/`, which `.gitignore:20` already excludes, NOT to the checkout + # The log goes to `.plan/temp/`, which `.gitignore` already excludes, NOT to the checkout # root. A log in the root is picked up by the `git status --porcelain` below as `?? rewrite-run.log` # — the job would manufacture one tree mutation of its own and report it next to the real ones, # leaving a reader unable to tell the artefact from the finding.