Repository navigation
fix(#1318): a registry-only consumer learns a module changed by READING the registry, not by waiting - #1467
Merged
Conversation
…registry, not by waiting `PluginUpdateWatcher` was the whole of "an installation finds out a module changed", and it works by subscribing to a `BuildCompletion` node. Across the entire tree that node is constructed in exactly ONE place — `GitHubWebhookProcessor`, handling a GitHub `workflow_run` webhook — and the watcher only opens a subscription once a catalog node names a source repo. An installation that installs from a registry over HTTP has neither: it deliberately holds no GitHub credential, receives no webhooks, and has no catalog node. The watcher is registered unconditionally, so on such an installation it was live and INERT — zero subscriptions, forever. Auto-update was therefore dead end to end: `PackageManifest.AutoUpdate` is stamped at install time and nothing ever fired it, which is why atioz's `Hosting` sat at its installed version until a human pressed Provision. Not a config gap; the absence of an input. ## What the consumer subscribes to: nothing — it reads the feed it already installs from There is nothing for it to subscribe to. The registry is a DIFFERENT deployment with a different database, so no durable row is shared, and there is no cross-process change feed either (`PostgreSqlChangeListener` is registered and never started in either partitioned-PG overload). A signal would mean a second wire protocol — subscription registry, shared secret, inbox — to keep in step with the feed that already exists. It does not need one. `GET /api/plugins` already returns every package's `ModuleVersion`, the same content identity the install records carry, so comparing the two answers "has anything changed" outright. That is the shape `BuildProtocolDriver.FollowGo` reached for the cross-cluster case (#1440/#1450): end on a fact you can read, not on a notification that cannot arrive. `RegistryUpdateReconciler` does exactly that, sequenced after `InstanceAutoRegistrationService` .Completed — the ordering idiom `ModuleDiscoveryService` already uses, and necessary because that service writes the same partitions and mints the instance key this one authenticates with. ## No poll timer It runs on an event the deployment already has — this process starting — not on a clock. #1366 removed a staleness clock for the same reason. On these deployments the restart IS the fan-out: plugin content and the framework image are published by the same CI and the portals self-update onto each new image, so a green plugin build and a pod roll already arrive together. The honest bound is "a consumer learns on its next boot", plus on demand whenever somebody opens the catalog page, which reads the same feed. An installation that never restarts also never picks up framework fixes. ## One decision, two inputs The per-module verdict moves into `PackageUpdateReconciler`, which both paths now call: content identity as the gate (equal `ModuleVersion` ⇒ silent, not one file fetched), reminder by default, unattended only for a record that opted in, and commercial re-authorization against the recorded principal on every unattended apply (#830). The watcher keeps the webhook half and loses its copy of the decision, so the two can never disagree. `BuildCompletion` stays registered and unconditional, deliberately: the watcher is how a REGISTRY learns from GitHub and this is how a CONSUMER learns from the registry. They are complements, and each is inert on the shape it does not serve. Gating either on config would hide a misconfiguration rather than fix one. An installation with no registry configured resolves no registry source and does nothing at all, so the git path stays exactly as it was. Verification: 277/277 in `MeshWeaver.PluginCatalog.Test` (5 new), 20/20 in `MeshWeaver.Documentation.Test` for the doc edit, `-c Release -warnaserror` clean with 0 warnings. The first run caught a real defect in my own test — `AutoUpdate` is seeded from the deployment default at install time, not taken from the manifest, so the reminder test had been passing against a path that was never eligible to install. Fixes #1318 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes the auto-update gap for registry-only plugin consumers by adding a boot-time reconciliation path that reads the registry feed (GET /api/plugins) and reuses the same per-module update decision logic as the existing GitHub webhook–driven watcher.
Changes:
- Add
RegistryUpdateReconciler(hosted service) to reconcile installed plugin records against configured registries at startup. - Centralize “module changed → remind vs auto-apply” into
PackageUpdateReconciler, used by bothPluginUpdateWatcher(webhook path) andRegistryUpdateReconciler(feed-read path). - Add consumer-side tests and update documentation/whats-new to describe the two-inputs/one-decision model.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.PluginCatalog.Test/RegistryUpdateReconcileTest.cs | New tests pin consumer startup reconcile behavior and “content identity” gating. |
| src/MeshWeaver.PluginCatalog/RegistryUpdateReconciler.cs | New hosted service that reads configured registries on startup and reconciles installed records. |
| src/MeshWeaver.PluginCatalog/PluginUpdateWatcher.cs | Removes duplicated decision logic; delegates to PackageUpdateReconciler. |
| src/MeshWeaver.PluginCatalog/PluginCatalogConfigurationExtensions.cs | Registers RegistryUpdateReconciler + IHostedService forward alongside the watcher. |
| src/MeshWeaver.PluginCatalog/PackageUpdateReconciler.cs | New shared reconciler implementing the per-module decision and applying/notification behavior. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-13-registry-plugin-updates.md | WhatsNew entry for registry-consumer update propagation. |
| src/MeshWeaver.Documentation/Data/Architecture/PluginUpdateOnGreenBuild.md | Doc update describing the registry-consumer startup reconcile input and shared decision. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Test Results (shard 1)1 514 tests +54 1 514 ✅ +54 7m 56s ⏱️ +26s Results for commit 607c90f. ± Comparison against base commit beb528c. This pull request removes 129 and adds 183 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 2)2 483 tests - 45 2 482 ✅ - 45 7m 6s ⏱️ -44s Results for commit 607c90f. ± Comparison against base commit beb528c. This pull request removes 183 and adds 138 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
…ot swallowed by it Copilot on #1467: `Decide` returned early when the two ModuleVersions were equal — and `null == null` is equal — so the commonest shape of the missing-identity defect (a module whose CI emits no manifest.lock, on both sides) was absorbed into 'nothing changed' and never produced the warning the failure mode promises. The operator would be left with a plugin that never updates and no line in the log saying why. The missing-identity check now runs first. New test pins the ordering. Writing it caught a second, subtler thing: the install record's hash comes from the module's manifest.lock FILE, not from the manifest handed to the installer, so a fixture that only blanks the manifest field installs a record that HAS a hash — the test would then have exercised the ordinary changed/unchanged path while claiming to exercise this one. The fixture-validity assertion is what caught it, and it stays. Copilot's second finding (CS1998 on a test marked async with no await) does not apply to this repo: `dotnet build test/MeshWeaver.PluginCatalog.Test -c Release -warnaserror` is clean with 0 warnings, and the pre-existing sibling test in BuildCompletionSubscriptionTest has the identical shape. 278/278 in MeshWeaver.PluginCatalog.Test; Release/-warnaserror clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap, stated exactly
PluginUpdateWatcherwas the whole of "an installation finds out a module changed", and it works bysubscribing to a
BuildCompletionnode. Across the entire tree that node is constructed in exactlyone place —
GitHubWebhookProcessor, handling a GitHubworkflow_runwebhook — and the watcheronly opens a subscription at all once a catalog node names a source repo.
An installation that installs from a registry over HTTP has neither: it deliberately holds no
GitHub credential, receives no webhooks, and has no catalog node. The watcher is registered
unconditionally, so on such an installation it was live and inert — zero subscriptions, forever.
Auto-update was dead end to end:
PackageManifest.AutoUpdateis stamped at install time and nothingever fired it. That is why client-i's
Hostingsat at its installed version until a human pressedProvision — not a config gap, the absence of an input.
What the consumer subscribes to: nothing. It reads the feed it already installs from.
There is nothing to subscribe to. The registry is a different deployment with a different
database, so no durable row is shared; and there is no cross-process change feed either —
PostgreSqlChangeListeneris registered and never started in either partitioned-PG overload. A pushsignal would therefore mean a second wire protocol (subscription registry, shared secret, inbox) kept
in step with the feed that already exists.
It does not need one.
GET /api/pluginsalready returns every package'sModuleVersion— thesame content identity the install records carry — so comparing the two answers "has anything changed"
outright. That is precisely the shape
BuildProtocolDriver.FollowGoarrived at for the cross-clustercase (#1440/#1450): end on a fact you can read, not on a notification that cannot reach you.
RegistryUpdateReconcilerdoes that, sequenced afterInstanceAutoRegistrationService.Completed—the ordering idiom
ModuleDiscoveryServicealready uses, and necessary because that service writesthe same partitions and mints the instance key this one authenticates with.
🚨 No poll timer
It runs on an event the deployment already has — this process starting — not on a clock. #1366
removed a staleness clock for the same reason: a timer answers "how stale am I willing to be", a
question nobody asked, and turns one misconfiguration into permanent background load.
On these deployments the restart is the fan-out: plugin content and the framework image are
published by the same CI, and the portals self-update onto each new image, so a green plugin build
and a pod roll already arrive together. The honest bound, stated plainly: a consumer learns on its
next boot — plus immediately, on demand, whenever somebody opens the catalog page, which reads the
same feed and offers the same Update. An installation that never restarts also never picks up
framework fixes, which is a louder problem with its own alarm.
One decision, two inputs
The per-module verdict moves into
PackageUpdateReconciler, which both paths now call: contentidentity as the gate (equal
ModuleVersion⇒ silent, not one file fetched), reminder by default,unattended only for a record that opted in, and commercial re-authorization against the recorded
principal on every unattended apply (#830). The watcher keeps the webhook half and loses its copy of
the decision, so the two can never disagree about what "changed" means.
Admin/_Build/{owner}.{repo}GET /api/pluginsOn
BuildCompletionbeing live-and-inertIt stays registered and unconditional, deliberately. The inertness was never the defect — the
defect was that nothing else covered the consumer. The watcher is how a REGISTRY learns from GitHub;
this is how a CONSUMER learns from the registry. They are complements, and each is inert on the shape
it does not serve, at zero cost. Gating either on config would hide a misconfiguration rather than fix
one.
An installation with no registry configured resolves no registry source and does nothing at all,
so the git path stays exactly as it was for the registry instance, which legitimately reads GitHub.
That also keeps Plugins #460's git fallback honest: nothing here changes which source content comes
from, only whether anybody ever looks.
A missing content hash is refused, loudly
A module with no
manifest.lockhas no content identity, so "has it changed" is unanswerable.Answering "yes" would act on the event rather than the content — the webhook would re-install on
every green build of the repo, and the boot reconcile on every single pod start. It is refused and
logged at Warning naming the module and which side is empty, so it cannot become a quiet "my plugin
never updates". Recorded in the doc's failure-mode table.
Verification
MeshWeaver.PluginCatalog.Test— 277/277 (5 new), full project,DOTNET_PROCESSOR_COUNT=4.MeshWeaver.Documentation.Test— 20/20 (link integrity for the doc edit).dotnet build -c Release -warnaserror— clean, 0 warnings.The first run caught a real defect in my own test:
AutoUpdateis seeded from the deploymentdefault at install time, not taken from the manifest handed to the installer
(
PackageInstaller.SeedAutoUpdate). So the opt-in test failed honestly, and the reminder test hadbeen passing for the wrong reason — asserting "nothing was installed" against a path that was never
eligible to install. Both are now driven through the real seed.
The tests pin the decision (applies on change, fetches nothing on no-change, reminds without an
opt-in, ignores a package not installed here) and the wiring (mesh-scoped singleton + the
IHostedServiceforward that actually runs it — the same pin the watcher has, because registrationalone does not start it).
Not covered by a test: the HTTP hop itself.
RegistryUpdateReconcilerconstructs itsRegistryPackageSourcefrom options, so exercising it end to end needs a live registry; the feed readis the already-proven
RegistryPackageSourcepath (measured live:GET /api/pluginswith client-i'sinstance token → 200, 20 packages).
Fixes #1318
🤖 Generated with Claude Code