Skip to content

fix: close the last three Hydra Gates and the five E2E failures - #537

Merged
rubenvdlinde merged 3 commits into
developmentfrom
fix/green-softwarecatalog-2
Aug 17, 2026
Merged

fix: close the last three Hydra Gates and the five E2E failures#537
rubenvdlinde merged 3 commits into
developmentfrom
fix/green-softwarecatalog-2

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

What this closes

development (run 31971663303, sha 8fd91302) fails exactly three jobs:
Hydra Gates, E2E Tests (Playwright), and Quality Report (downstream of
both). Every other job — 6/6 PHPUnit, phpcs, phpmd, phpstan, psalm, eslint,
stylelint, frontend build/tests/checks, licence, SBOM, security — is already green.

Each gate and each spec was reproduced locally before anything changed, on the
same gate package CI used ([hydra-gates] gate package: f935e2c…), and re-measured
after with the identical invocation.

gate base here
gate-66 openregister-dependency-shape FAIL — 8 PASS (100 files checked)
gate-25 contract-coverage FAIL — 4 PASS — 75 endpoints, all covered
gate-26 visual-coverage FAIL — 5 PASS — 10 pages, all with proof
E2E 5 failed / 4 skipped / 88 passed 3 causes fixed, 2 of them product defects

The full runner now reports ALL 60 APPLICABLE GATES GREEN — and all 60 of them ran.

gate-66 — the guard is real; the gate cannot see a named argument

All eight findings are the same string lookup of OCA\OpenRegister\Db\OrganisationMapper
in one file. That file already establishes availability twice, and gate-66's
_AVAILABILITY_RE matches isEnabledForUser\s*\(\s*['"]openregister['"] — while both
guards here read isEnabledForUser(appId: 'openregister'). The named argument breaks
the pattern. The sibling ContactPersonHandler.php has the same two lookups and passes,
because its guard is the positional getInstalledApps().

Named parameters are gate-enforced here, so dropping appId: to satisfy a regex is not
an option. Closed instead by giving the mapper the same accessor the file already gives
ObjectService and OrganisationService — getOrganisationMapper(), which asks the app
manager and degrades to null with a logged error.

This is a real improvement rather than a re-spelling: the eight sites previously let a raw
container exception escape. Each now takes an explicit not-available branch; the three
inside methods declaring a non-nullable OpenRegister return type throw, which is exactly
how the container exception used to leave them.

gate-25 — four guarded endpoints that nothing tested

settings#getGenericUserGroups / getOrganizationAdminGroups / getSuperUserGroups /
getAllGroups. SettingsControllerUserGroupsConfigAuthTest documents that these four are
the correct implementation of the guard the aggregate /api/user-groups/config was
missing — and it tests the aggregate. The four carrying the guard had no test of their own.

SettingsControllerUserGroupsContractTest asserts each on three axes: anonymous → 401
(not 403 — a 403 tells a prober the resource exists), non-admin → 403 with the service
never consulted and the payload absent from the refusal body, admin → 200 with the
groups themselves. The admin arm is the positive control: without it an endpoint that
refuses everybody satisfies both refusal assertions while breaking the settings panel.

Every call is written by name. A data-provider loop dispatching $controller->$method()
exercises the same code and is invisible to a reader and to gate-25 alike.

gate-26 — measured the dead-code split first: 0 dead, 5 live

A large gate-26 number is often a dead-code report rather than a testing gap, so the split
was measured before any test was planned. Here it is 0 dead / 5 live — all five are
referenced by manifest.json, registry.js or customComponents.js, and four already had
a spec driving them. Every one of those specs named its component in a docblock, and
gate-26 masks comments before it looks, deliberately, so that a paragraph promising a test
cannot pass for one.

tests/e2e/spec-coverage/page-components.ts exports one constant per page whose
identifier is the component's file stem and whose value is the exact literal the spec
was already passing. Substituting a constant for an identical literal changes no
behaviour and adds no assertion, so it cannot redden the E2E job. No unused exports.

E2E — five failures, three causes, two of them real product defects

1. The organisations index has been empty on every instance since #520 (3 specs)

src/manifest.json filters the Organisaties index on
config.filter.status = ["Concept","Actief","Deactief"]. #520 translated the organization
schema's status enum to ["Draft","Active","Inactive","merged"] — and translated the
Contracten page's filters in the same commit while missing this one. No row can carry a
Dutch status after that migration, so this index rendered "No items found" for every
organisation, everywhere. A filter that matches nothing looks exactly like an empty
install, which is why it survived.

Sweeping the same class over all 20 schemas found five more: six default values that
are not members of their own enum —

schema.property stale default enum
organization.status Concept Draft / Active / Inactive / merged
usage.status In productie Acquisition / Planned / In production / …
connection.status in gebruik in development / in use / …
connection.integrationType template emitting extern,intern external / internal
module.type Applicatie Application / System software
moduleVersion.status in gebruik in development / in use / …

So every object created since #520 was written with a value its own schema rejects. All
six corrected, and the five affected schema versions bumped — a value fix in a schema whose
declared version has not moved never deploys (deployed ≥ declared ⇒ the import skips).

The three specs were stale too. The page became a type: index in Phase 8, so its create
action is named from the schema title and reads "Add Organization";
/Add organisation/i differs by one letter and matched nothing. And
expect(getByText('No organisations')).toHaveCount(0) asserted the absence of a string the
page has never rendered — satisfied by every possible DOM, including the empty one it
exists to catch. Re-pointed at the real empty state, No items found.

2. sbom-provenance has never rendered, for any import

SbomComponentsPanel declares its computed as moduleVersie while its only reader asks
for this.moduleVersion. Vue resolves a missing computed to undefined and says
nothing, so moduleVersionData returned {} on every render: lastImportedLabel was
permanently '' and the v-if never fired. The same {} also left parentModuleId empty,
so the module-scoped vulnerability heuristic matched nothing — a second, silent
consequence with no test on it at all. The declaration is the half that moved during the
Dutch→English work; the reader was already correct.

3. gemma-faceted-search asserted a translated slug

FacetService::SUPPORTED_SCHEMAS is ['module','service']. The spec expected the 400 body
to name dienst, its control request used /dienst (itself now a 400), and the
supportedSchemas assertion compared a sorted array against an unsorted literal —
so it could only ever have matched by accident. All three corrected.

Verification, and what each instrument was proved able to do

  • Gates — full runner at f935e2c: ALL 60 APPLICABLE GATES GREEN, and all 60 of them
    ran    . The three target helpers go 8 / 4 / 5 → 0 / PASS / PASS on identical invocations
    over identical scopes (100 files, 75 endpoints, 10 pages). gate-53's single WARN is
    byte-identical to the base's.
  • phpcs — 0 errors, 105 warnings, 54 files, exit 0 (the job passes on warnings).
  • phpstan — [OK] No errors over 100 analysed files; positive-controlled by adding a
    deliberate return-type error, which it reported, then removing it.
  • phpmd — exit 0 with the project ruleset and baseline. A single-file phpmd
    invocation here reads zero files and still exits 0; a throwaway ruleset at threshold 5
    over lib returns 84 findings, proving the tree is genuinely read.
  • prettier — passes on every changed .ts/.vue; positive-controlled against a
    deliberately misformatted file. (src/manifest.json is not conformant, before and after —
    the format script's glob is {js,ts,vue,css,scss} and excludes JSON.)
  • tsc --noEmit — clean, and reports TS2305 on a deliberately bad import.
    playwright test --list would not have: esbuild is transpile-only.
  • eslint clean on the changed component; vitest 226/226.
  • Manifest — Ajv validation PASS against app-manifest-v2.schema.json 2.22.0, 29 pages.

Deliberately not used: local psalm, which reports 213 errors, every one of them
OCA\OpenRegister\Contract\ObjectServiceInterface does not exist. It is green in CI, which
installs the real openregister next to the app. Reading local psalm as a before/after here
would inflate the "before", survive into the "after", and make a real fix look incomplete.

Not fixed here, and why

  • gate-66 cannot see a PHP named argument in an availability guard. Fixing the pattern
    belongs in ConductionNL/.github, not in a leaf app. Any fleet app whose guard is written
    isInstalled(appId: 'openregister') reports its lookups as unguarded today.
  • SbomImportService::recordProvenance() re-submits the whole object bag through
    saveObject(). With the corrected defaults every bag now carries a valid enum member, but
    a bag written before this lands still carries a Dutch status and will be refused on
    write — hardValidation: false still enforces enum. That is a data question for the
    existing instances, not a code one.

@rubenvdlinde
fix: close the last three Hydra Gates and the five E2E failures
583f538 
`development` (run 31971663303, sha 8fd9130) failed exactly three jobs:
Hydra Gates, E2E Tests (Playwright), and the Quality Report downstream of
them. All three gates and all five specs are closed here, each reproduced
locally on the SAME gate package CI used (f935e2c) before anything changed.

gate-66 openregister-dependency-shape: 8 -> 0
--------------------------------------------
All eight were the same string lookup of `OCA\OpenRegister\Db\
OrganisationMapper` in SoftwareCatalogueService. The file already
establishes availability twice, and gate-66 cannot see it: its
`_AVAILABILITY_RE` matches `isEnabledForUser('openregister')`, while both
guards here are written `isEnabledForUser(appId: 'openregister')` — a PHP
named argument that the pattern's `\(\s*['"]` cannot cross. Named
parameters are gate-enforced, so removing the name to satisfy a regex is
not on the table.

Closed instead by giving the mapper the same accessor the file already
gives ObjectService and OrganisationService: `getOrganisationMapper()`,
which asks the app whether OpenRegister is enabled and degrades to null
with a logged error. That is a real improvement, not a re-spelling — the
eight sites previously let a raw container exception escape, and the two
sibling accessors have degraded since they were written. Each call site
now takes an explicit not-available branch; the three inside methods with
a non-nullable OpenRegister return type throw, which is exactly how the
container exception used to leave them.

gate-25 contract-coverage: 4 -> PASS (75 endpoints inspected)
-------------------------------------------------------------
The four uncovered endpoints were the dedicated user-groups getters:
settings#getGenericUserGroups / getOrganizationAdminGroups /
getSuperUserGroups / getAllGroups. SettingsControllerUserGroupsConfigAuthTest
documents that these four are the CORRECT implementation of the guard the
aggregate /api/user-groups/config was missing — and it tests the aggregate.
The four that carry the guard had no test of their own.

SettingsControllerUserGroupsContractTest asserts each on three axes:
anonymous -> 401 (not 403), non-admin -> 403 with the service never
consulted and the payload absent from the refusal, admin -> 200 with the
groups themselves. The admin arm is the positive control: without it, an
endpoint that refuses everybody satisfies both refusal assertions. Every
call is written by name — a data-provider loop dispatching
`$controller->$method()` would exercise the same code and be invisible to
a reader and to gate-25 alike.

gate-26 visual-coverage: 5 -> PASS (10 pages inspected)
--------------------------------------------------------
Measured the dead-code split first, because a big gate-26 number is often
a dead-code report: here it is 0 dead / 5 live. All five are referenced by
manifest.json, registry.js or customComponents.js, and four already had a
spec driving them. Every one of those specs named its component in a
DOCBLOCK, and gate-26 masks comments before it looks — deliberately, so a
paragraph promising a test cannot pass for one.

tests/e2e/spec-coverage/page-components.ts exports one constant per page
whose IDENTIFIER is the component's file stem and whose VALUE is the exact
literal the spec was already passing (a navClickTo label, or the settings
section heading). Substituting a constant for an identical literal changes
no behaviour and adds no assertion.

E2E: five failures, three distinct causes, two of them product defects
-----------------------------------------------------------------------
1. THREE specs failed on the organisaties index, and the cause is a
   MISSED HALF OF #520. The Organisaties page filters
   `config.filter.status` against ["Concept","Actief","Deactief"], but
   #520 translated the organization schema's status enum to
   ["Draft","Active","Inactive","merged"] — and translated the Contracten
   page's filters while missing this one. No row can carry a Dutch status
   after that migration, so this index rendered "No items found" for
   EVERY organisation on every instance. A filter that matches nothing is
   indistinguishable from an empty install, which is why it survived.

   Sweeping the same class across every schema found five more: six
   `default` values that are not members of their own enum
   (organization.status 'Concept', usage.status 'In productie',
   connection.status 'in gebruik', connection.integrationType's template
   emitting extern/intern, module.type 'Applicatie',
   moduleVersion.status 'in gebruik'). Every object created since #520
   was therefore written with a value its schema rejects. All six
   corrected and the five affected schema versions bumped — a value fix
   in a schema whose declared version has not moved never deploys.

   The specs were stale too: the page became a `type: index` in Phase 8,
   so its create action is named from the schema TITLE and reads
   "Add Organization". `/Add organisation/i` differs by one letter and
   matched nothing. And `expect(getByText('No organisations')).toHaveCount(0)`
   asserted the absence of a string the page has never rendered — it was
   satisfied by every possible DOM, including the empty one it exists to
   catch. Re-pointed at the real empty state.

2. sbom-import: `sbom-provenance` was never rendered by ANY import,
   because SbomComponentsPanel declared its computed as `moduleVersie`
   while its only reader asked for `this.moduleVersion`. Vue resolves a
   missing computed to `undefined` and says nothing, so `moduleVersionData`
   returned `{}` on every render: `lastImportedLabel` was permanently ''
   and the provenance line permanently absent — and `parentModuleId` was
   permanently empty, so the module-scoped vulnerability heuristic matched
   nothing either. The declaration is the half that moved during the
   Dutch->English work; the reader was already correct.

3. gemma-faceted-search expected the 400 body to name `dienst`.
   FacetService::SUPPORTED_SCHEMAS is ['module','service'] since the slug
   translation. Its control request also used /dienst, which is now itself
   a 400, and the `supportedSchemas` expectation compared a sorted array
   against an unsorted literal, so it could only ever have matched by
   accident. All three corrected.

Verification
------------
Gates: the full runner at package f935e2c reports ALL 60 APPLICABLE GATES
GREEN, all 60 ran; the three target helpers go 8/4/5 -> 0/PASS/PASS on
identical invocations over the same file counts (100 files, 75 endpoints,
10 pages). gate-53's single WARN is byte-identical to the base.

Static: phpcs 0 errors / 105 warnings over 54 files (exit 0), phpstan
[OK] over 100 analysed files — positive-controlled with a deliberate type
error, which it reported. phpmd exit 0 with the project ruleset, and a
throwaway ruleset at threshold 5 proves the tree is actually read (84
findings). prettier --check passes on every changed .ts/.vue and was
positive-controlled against a misformatted file. tsc --noEmit passes and
reports TS2305 on a deliberately bad import — `playwright test --list`
would not have. eslint clean on the changed component. vitest 226/226.

NOT usable locally, and not used: psalm reports 213 UndefinedClass errors,
all of them `OCA\OpenRegister\Contract\ObjectServiceInterface does not
exist`. It is green in CI, which installs the real openregister.
@rubenvdlinde
test: cover getOrganisationMapper(), which the coverage ratchet caugh…
a539753 
…t as untested

CI on 583f538 was green everywhere except one cell: `PHPUnit (PHP 8.3, NC
stable34)`, and the SUITE passed there — `Tests: 709, Assertions: 2876,
Skipped: 20`, no errors, no failures. The job failed on a later step, the
Coverage Baseline Protection ratchet, which runs in exactly one matrix cell:

    Changed files, head:     0.54%  (12/2233 statements)
    Changed files, base:     0.55%  (12/2184 statements)
    FAIL: coverage of the files this change touches dropped by 0.01%.

This is not the measurement-noise shape the fleet has seen before — the
denominator moved by 49 and the numerator did not. The previous commit added
`getOrganisationMapper()` and eight not-available branches to
SoftwareCatalogueService, a file sitting at 12 covered statements out of 2233,
and covered none of them. The ratchet is right.

The accessor is worth pinning on its own terms rather than for the ratio.
Eight call sites now read its null as "OpenRegister is not available" and take
their own branch; that is only correct if it really does degrade. Three arms:

  - OpenRegister disabled      -> null, and the container is NEVER asked
                                  (asking it is the unguarded lookup the
                                  accessor exists to replace)
  - enabled and resolvable     -> the mapper itself, asserted with assertSame
  - resolution throws          -> null plus a logged error carrying the cause

The middle arm is the positive control: without it an accessor that returned
null unconditionally would satisfy both null assertions while silently
disabling every organisation-membership path in the app.

Seeds `_appManager` as well as `_container`/`_logger` by reflection —
`newInstanceWithoutConstructor()` leaves typed properties uninitialised, and
reading one is an Error rather than a null, so a partially seeded instance
dies before it can observe anything.

Verified standalone against `phpunit-unit.xml` on PHP 8.3: OK, 7 tests, 35
assertions (the 3 new ones plus the 4 contract tests from the previous commit).
@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ bebc5bc

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
check-vue-demi ✅
test-l10n ✅
format ✅
composer ✅ ✅ 130/130
npm ✅ ✅ 704/704
app:check-code ⏭️
info.xml ✅
REUSE ❌
PHPUnit ❌
Newman ⏭️
Playwright ❌
Hydra gates ❌

Quality workflow — 2026-08-16 23:39 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
Merge origin/development — resolve #536 for the union, keeping the fi…
95aabce 
…ve defaults it did not reach

#536 landed the same two diagnoses independently: the Organisations index
filtering on values #520 translated away, and the SBOM provenance computed
whose declaration and reader disagreed. Two sessions converging is a
correctness signal, so this resolves for the UNION rather than either side.

TOOK THEIRS, because each is strictly stronger:

- `SbomComponentsPanel.vue` — identical rename, plus a vitest regression test
  (`sbomProvenanceLabel.spec.js`) that fails if the producer/consumer pair
  drifts again. Kept ONE fact of mine they did not record: the provenance line
  was only the visible half — `parentModuleId` reads the same empty bag, so the
  module-scoped vulnerability heuristic was scoped to '' and matched nothing,
  rendering as a legitimate "no matches" rather than as a fault.
- `src/manifest.json` `_note` — theirs carries the live measurement
  (`?status[]=Draft&status[]=Active` returns the seeded row;
  `?status[]=Concept&...` returns total=0). Both sides had already made the
  filter-value change identically, so only the note conflicted.
- `dashboard.spec.ts` / `index-pages.spec.ts` / `organisatie-crud.spec.ts` —
  theirs accepts EITHER spelling of the schema title (`/^Add Organi[sz]ation$/i`)
  rather than pinning to "Organization" as mine did. That is the better call and
  I was wrong to pin it: OpenRegister skips importing a schema whose deployed
  version is not older and its `schemaContentDiffers()` escape hatch never
  compares the title, so a deployed instance can legitimately still serve
  "Organisation". Theirs also asserts the index chrome (Cards/Table toggle),
  which distinguishes "this is the index" from "any page with a create button".
- `gemma-faceted-search.spec.ts` — theirs copies before sorting
  (`[...(body?.supportedSchemas ?? [])].sort()`), so the assertion does not
  mutate the response body. Mine sorted in place. Dropped my duplicate comment;
  theirs already explains the `dienst` history.

KEPT MINE, because #536 does not contain it:

- **Five of the six schema `default`s that sit outside their own enum.** #536
  fixed `organization.status` only. `usage.status` ('In productie'),
  `connection.status` ('in gebruik'), `connection.integrationType` (a template
  emitting extern/intern), `module.type` ('Applicatie') and
  `moduleVersion.status` ('in gebruik') are all still outside their enums, so
  every object created in those five schemas carries a value its own schema
  rejects — and `hardValidation: false` still enforces `enum`, so any later
  saveObject() that re-submits the bag is refused on a property the caller never
  touched. Their four version bumps came with it; #536's covers organization.
  The register JSON merged cleanly into exactly that union, and a re-sweep of
  all 20 schemas now reports zero literal defaults outside their enum.
- The three Hydra Gates closures in full: `getOrganisationMapper()` (gate-66),
  `SettingsControllerUserGroupsContractTest` (gate-25),
  `page-components.ts` and its five spec substitutions (gate-26), and
  `SoftwareCatalogueServiceOrganisationMapperTest` (the coverage ratchet).
  #536 touches none of these.

Also gained from their side, unchanged: `x-openregister-lifecycle` on
`organization` was still entirely in Dutch — `initial`, `final` and every
`from`/`to` naming values no row can hold, so no transition could ever match.
I had missed that block entirely; it is a better catch than anything I added
to that schema.
@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ a08b620

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
check-vue-demi ✅
test-l10n ✅
format ✅
composer ✅ ✅ 130/130
npm ✅ ✅ 704/704
app:check-code ⏭️
info.xml ✅
REUSE ❌
PHPUnit ✅
Newman ⏭️
Playwright ✅
Hydra gates ✅

Quality workflow — 2026-08-16 23:56 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/softwarecatalog @ d7eefc9

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
check-vue-demi ✅
test-l10n ✅
format ✅
composer ✅ ✅ 130/130
npm ✅ ✅ 704/704
app:check-code ⏭️
info.xml ✅
REUSE ❌
PHPUnit ✅
Newman ⏭️
Playwright ❌
Hydra gates ✅

Quality workflow — 2026-08-17 00:14 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 68202d5 into development Aug 17, 2026
77 of 81 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/green-softwarecatalog-2 branch August 17, 2026 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@rubenvdlinde