Skip to content

fix(gate-23): PDOK rule counted comments as violations; ADR-022 epoch never matched its comment - #279

Merged
rubenvdlinde merged 1 commit into
mainfrom
fix/or-abstraction-pdok-prose-and-epoch
Aug 9, 2026
Merged

fix(gate-23): PDOK rule counted comments as violations; ADR-022 epoch never matched its comment#279
rubenvdlinde merged 1 commit into
mainfrom
fix/or-abstraction-pdok-prose-and-epoch

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What this is

hydra-gates/scripts/lint-or-abstraction-anti-patterns.sh (gate-23, ADR-022) leaves WARN mode and starts hard-failing on a date this file gets wrong. I measured what would break, and found that the rule producing the most visible findings was largely measuring prose.

1. Rule 1 counted comments as violations

The rule was grep -rl api.pdok.nl. Measured across all 18 Conduction app repos at origin/development on 2026-08-09 it produced three findings — two of them the opposite of a violation:

file old verdict reality
procest src/services/pdokService.js VIOLATION Is the openconnector-routed shim. BASE_URL = generateUrl('/apps/openconnector/api/pdok'). It never contacts PDOK. Its only match was a docblock line: "Direct browser calls to api.pdok.nl are NOT permitted from this app — see Hydra umbrella shared-pdok-via-openconnector (ADR-022)". The gate flagged a file for containing a sentence citing this rule.
openregister lib/Service/Geo/PdokGeocoder.php VIOLATION Compliant. Matched a const holding the base URL that it hands to OpenConnector's CallService. It owns no HTTP client and returns null when OpenConnector is absent. The pattern ADR-022 prescribes, reported as the one it forbids.
procest lib/Service/Pdok/PdokLocatieserverService.php VIOLATION Genuine. callDirect() does fopen() + stream_context_create() on the endpoint whenever pdok_locatieserver_source is empty — its default.

A hostname in a comment cannot make an HTTP request, and a hostname handed to the shared adapter is the fix rather than the defect. The rule now asks the two questions that separate those cases:

  1. does the host appear on a line of code (comment-only lines stripped — https:// survives, only line-opening //, *, # and /* */ blocks are removed)?
  2. does the file carry its own transport, or dispatch through OpenConnector?

Compliant and prose-only files are printed as info lines, never silently dropped — a reader must be able to tell "nothing there" from "found it and judged it compliant".

It is narrower on prose only

Still fires on: a code-line URL with no demonstrable routing; an unrecognised HTTP client (the transport list is not an allowlist); a direct fopen() in a file whose comment name-drops openconnector.

scripts/lib/test_or_abstraction_pdok.sh (new, auto-discovered by tests/run-helper-suites.sh) pins this: 6 must-fire assertions, 4 must-be-silent, plus an empty-tree control so the silences mean something.

Verified the suite can fail: mutating the suppression to a whole-file grep — the classic widening — turns it red (1 assertion(s) FAILED, exit 1). Full package run: 54 discovered, 52 passed, 2 quarantined, 0 failed.

2. BLOCK_AFTER_EPOCH never matched its own comment

The comment said "2026-05-11 + 90d" = 2026-08-09 00:00 UTC = 1786233600.
The committed value 1786636800 is 2026-08-13 16:00 UTC — four days and sixteen hours later, and not a midnight boundary, which is the tell that it was arrived at by hand. The switch-over date could be read off neither the code nor the comment.

The intent was the comment's. Taking it literally would have flipped the gate to BLOCK the morning the discrepancy was found. So it was measured before it was decided, and the measurement said not to:

  • 11 of 18 repos would have started hard-failing. procest carries an entire multi-tenant SaaS stack (26 Tenant* classes, 5 workflow-engine classes); hermiq a 6-class tenant control plane. That is an architecture programme, not a deadline.
  • On that same measurement the majority of findings were artefacts of the rules' own filename matching — e.g. docudesk lib/EventListener/ApprovalStepListener.php is flagged by the approval rule while subscribing to OpenRegister's own ApprovalStep*Events, and zaakafhandelapp lib/Controller/ZaakAuditTrailController.php is flagged by the audit rule while reading OR's audit trail and mapping it to the ZGW shape. A gate has to be believable before it is made blocking.

Moved deliberately to 2026-10-03 — the epoch CAP_BLOCK_AFTER_EPOCH in this same file already uses — so ADR-022 has one fleet-wide enforcement date instead of two unrelated cliff edges.

The deadline moved on purpose and the file says why. No rule was weakened to meet it, nothing was waived per file, and no comment was added anywhere to satisfy a matcher.

Not in this PR

Rules 2–7 are filename matchers and cannot distinguish an app-local implementation from a consumer of the OR abstraction. That is a real defect with real evidence (above), but redesigning it under a four-day clock is how gates get widened by accident. Filed separately; the new epoch gives it an 8-week runway.

… never matched its comment

Two defects in lint-or-abstraction-anti-patterns.sh, found while measuring what
would start hard-failing when the gate leaves WARN mode.

1. Rule 1 was `grep -rl api.pdok.nl`, so it matched prose.

   Measured across all 18 Conduction app repos at origin/development: three
   findings, two of them the opposite of a violation.

     procest src/services/pdokService.js  — IS the openconnector-routed shim
       (BASE_URL = generateUrl('/apps/openconnector/api/pdok')). Its only match
       was a docblock line reading "Direct browser calls to api.pdok.nl are NOT
       permitted from this app — see Hydra umbrella
       `shared-pdok-via-openconnector` (ADR-022)". The gate reported a file as
       violating the rule because it contains a sentence citing the rule.

     openregister lib/Service/Geo/PdokGeocoder.php — matched a const holding
       the Locatieserver base URL, which is the argument handed to
       OpenConnector's CallService. The class owns no HTTP client and returns
       null when OpenConnector is absent. The prescribed pattern, reported as
       the forbidden one.

   Only procest lib/Service/Pdok/PdokLocatieserverService.php was real: its
   callDirect() fopen()s the endpoint whenever `pdok_locatieserver_source` is
   empty, which is the default.

   The rule now asks the two questions that separate them — is the host on a
   line of CODE, and does the file carry its own transport or dispatch through
   OpenConnector. Compliant and prose-only files are PRINTED as info lines, so
   a reader can tell "nothing there" from "found it and judged it compliant".

   It is narrower on prose only. A code-line URL with no demonstrable routing
   still fires, an unrecognised HTTP client still fires, and a comment naming
   openconnector buys nothing. scripts/lib/test_or_abstraction_pdok.sh pins all
   of that: 6 must-fire assertions against 4 must-be-silent ones, plus an
   empty-tree control so the silences mean something. Verified it can fail —
   widening the suppression to a whole-file grep turns it red.

2. BLOCK_AFTER_EPOCH never matched its own comment.

   The comment said "2026-05-11 + 90d", i.e. 2026-08-09 00:00 UTC = 1786233600.
   The committed value 1786636800 is 2026-08-13 16:00 UTC — four days and
   sixteen hours later, and not a midnight boundary, which is the tell it was
   arrived at by hand. The switch-over date could be read off neither.

   Intent was the comment's. Taking it literally would have flipped the gate to
   BLOCK the morning the discrepancy was found, so it was measured first: ELEVEN
   of 18 repos would start hard-failing, procest on an entire 26-class tenant
   stack and hermiq on a 6-class tenant control plane — an architecture
   programme, not a deadline. And on that same measurement most findings were
   artefacts of the rules' own matching.

   Moved deliberately to 2026-10-03, which is the epoch the ADR-051 capability
   table in this same file already uses, so ADR-022 has one fleet-wide
   enforcement date instead of two. The date moved on the record and says why.
   No rule was weakened to meet it and nothing was waived per file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant