Skip to content

fix(settings): register-import-reliability — schema changes now actually reach installed instances (#391) - #396

Merged
rubenvdlinde merged 3 commits into
developmentfrom
wip/register-import-reliability
Jul 24, 2026
Merged

fix(settings): register-import-reliability — schema changes now actually reach installed instances (#391)#396
rubenvdlinde merged 3 commits into
developmentfrom
wip/register-import-reliability

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Closes #391 — the Tier 0 blocker. Without this, every OpenSpec change that touches the register ships dead to existing installs while CI stays green (which is exactly what happened to all 8 market-gap changes).

Two compounding root causes — the second found during implementation

  1. Monolith content was not part of the import version. computeConfigVersion() derived it from info.version + a hash of register.d/*.json fragments only. A direct monolith edit without a manual info.version bump produced a byte-identical version, so OpenRegister's version-gated importFromApp() silently no-op'd.
  2. shouldLoadSettings() compared the wrong two things — the app's own semver against the register content version stored on the same slot: version_compare("0.2.17", "2.3.1+frag...", ">")false (verified). This could permanently block loadSettings() from running again, and explains the observed "versions differed yet no import log line appeared".

Fix

  • Fold +base.<md5-8> of the monolith's own content into the computed version, so any register change re-imports — without degrading into always-import (kept content-derived and cheap).
  • Remove the broken semver-vs-content-version gate.
  • Make silence impossible: verifyRegisterAgainstEffectiveConfig() checks the live schema set against the shipped register after import, logs a WARNING on mismatch and surfaces it in the settings status payload.

Regression test

testMonolithContentChangeAloneChangesComputedVersion() — asserts the computed version changes when only monolith content changes (info.version and fragments untouched). That is precisely the input the old signature missed.

Duplicate configuration rows

Not caused by this app (its single importFromApp() call site always passes the same constant app id). OpenRegister's findByApp()/findBySourceUrl() organisation-scope their lookup (applyOrganisationFilter, allowNullOrg: false), so a caller in a different active-organisation context can fail to find an existing row and create a duplicate. Filed ConductionNL/openregister#2072 and referenced in code/docs rather than hacked around here.

Tests: 439 / 1421 assertions, zero new failures (1 pre-existing unrelated: #393 CSV HeaderUtils). openspec validate --specs --strict 52/52. Archived. Docs: new "How Register and Schema Changes Reach an Installed Instance" section.

Follow-up filed separately: getVersionInfo() has the same bad comparison in a second place, mislabelling "Up to date" in the Settings UI.

🤖 Generated with Claude Code

… re-import gate

SettingsService::loadSettings() computed the OpenRegister import version
from the register JSON's own info.version plus a hash of ADR-037
fragment files only. A change that edited the monolith
softwarecatalogus_register.json directly without also bumping
info.version by hand produced a byte-identical version, and
OpenRegister's version-gated importFromApp() silently skipped the
import — eight merged market-gap changes went dead on an upgraded
instance while CI and `occ upgrade` both reported success.

Fold an md5 of the monolith file's own content into the computed
version (+base.<md5-8>, alongside the existing +frag.<md5-8>) so ANY
register edit forces a re-import, without weakening the gate into
always-import.

Also fix a second, independently sufficient defect found while reading
this code path: SettingsService::shouldLoadSettings() compared this
app's own semantic version against
ConfigurationService::getConfiguredAppVersion() — but that value is the
register-content version this same service passes to importFromApp(),
not an app semver. version_compare("0.2.17", "2.3.1+frag.9003c029", ">")
is false (verified), so once any import ever stored such a value,
loadSettings() could never run again for any future app version bump.
This is the confirmed mechanism behind the live evidence's "versions
DID differ ... yet nothing imported and no import log line appeared".
shouldLoadSettings() now always returns true; the actual (potentially
expensive) import remains gated by importFromApp()'s own comparison of
two like-for-like content-derived versions.

Add post-import verification: after importFromApp() runs, confirm every
schema slug in the effective (monolith + fragments) register resolves
in OpenRegister, and that the schema ids this app tracks for its own
object types are non-null. Mismatches are logged as warnings and
persisted so getConfigurationStatus() surfaces a translated warning
instead of a no-op import looking identical to full success.

Investigated the three duplicate "Software Catalog Register"
configuration rows: this app's only importFromApp() call site always
passes the same constant appId, so the duplication is not attributable
to this app. Code review of OpenRegister's ImportHandler/
ConfigurationMapper found findByApp()/findBySourceUrl() organisation-
scope their lookup, so a caller whose active-organisation context
differs from an existing row's can fail to find it and create a
duplicate. Filed ConductionNL/openregister#2072 with the full
mechanism; referenced from a code comment rather than hacked around.
…on warning

Add a CONFIGURATION.md section explaining how register/schema changes
reach an installed instance: the ADR-037 fragment-file preference, and
the content-hash mechanism that now forces a re-import on any monolith
edit. Includes guidance for diagnosing a no-op import via the new
registerVerification status field.

Add Dutch and English l10n entries for the new user-facing
register-verification warning message surfaced through
getConfigurationStatus().
Merges the settings-service spec deltas (content-derived version
signature, register-verification status surface, and the
call-site-determinism requirement for the OpenRegister configuration
row) into the canonical spec and archives the change artifacts.
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