Repository navigation
fix(plugin-catalog): an unattended update binds to the repository the install came from (#6526) - #6541
Merged
Conversation
… install came from (#6526) The update watcher handed a watched repository's listing to the reconciler, which matched install records by package id alone, so a second watched repository listing the same id could update a package it never installed. - The install record carries `Origin`: the canonical identity of the SOURCE the install ran through (`InstallOrigin`), attached where the source is built from its configured repository and subdirectory (`PackageSources.FromRepo`) or by the registry client, and stamped by the one install funnel (`CatalogLayoutAreas.InstallOrUpdate`) over anything a manifest authors. - The watcher binds each pass to its repository: a candidate is taken only for a record whose origin is that repository. A record that predates the field adopts the repository when it is the only watched one listing the package; two or more are refused by name (Warning + platform bell); an unreadable watched repository adopts nothing that pass. Tests: UpdateBindsToTheInstallOriginTest (adopt, impostor ignored after adoption + control, refuse naming both, incomplete census, unbound negative control), AnInstallRecordsItsOriginTest. Doc: PluginUpdateOnGreenBuild. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…-to-install-origin
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Pinned-package handling, concurrent origin adoption, and one timing-sensitive test remain incorrect.
3 open findings
What changed in this PR
Binds plugin updates to their installation source, preventing same-ID packages from unrelated repositories from being applied.
Changes:
- Adds canonical installation-origin tracking and enforcement.
- Handles legacy records through origin adoption or localized refusal.
- Adds integration tests and architecture documentation.
| File | Description |
|---|---|
test/Memex.Portal.Shared.Test/UpdateBindsToTheInstallOriginTest.cs |
Tests origin recording, adoption, refusal, and impostor handling. |
src/MeshWeaver.PluginCatalog/RegistryPackageSource.cs |
Exposes registry source identity. |
src/MeshWeaver.PluginCatalog/PluginUpdateWatcher.cs |
Binds repository update passes and builds origin censuses. |
src/MeshWeaver.PluginCatalog/PackageUpdateReconciler.cs |
Enforces origin decisions and raises ambiguity notifications. |
src/MeshWeaver.PluginCatalog/PackageSources.cs |
Attaches origins to repository sources. |
src/MeshWeaver.PluginCatalog/PackageInstaller.cs |
Persists or carries forward installation origins. |
src/MeshWeaver.PluginCatalog/Package.cs |
Adds PackageManifest.Origin. |
src/MeshWeaver.PluginCatalog/InstallOrigin.cs |
Defines canonicalization and binding decisions. |
src/MeshWeaver.PluginCatalog/CatalogLayoutAreas.cs |
Stamps trusted source origins during installation. |
src/MeshWeaver.Messaging.Hub/Localization/strings.en.json |
Adds English refusal text. |
src/MeshWeaver.Messaging.Hub/Localization/strings.de.json |
Adds German refusal text. |
src/MeshWeaver.Documentation/Data/Architecture/PluginUpdateOnGreenBuild.md |
Documents origin-bound updates. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Test Results 20 files - 4 20 suites - 4 54m 24s ⏱️ + 1m 8s Results for commit c0f8c4b. ± Comparison against base commit 70362c4. This pull request removes 347 and adds 23 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…-to-install-origin
…an adoption decides on the record it wrote Review on #6541: - A pinned record went straight to DecideBound, whose floor gate runs before the policy switch, so a candidate from any watched repository (a foreign one included) could stamp or clear HeldUpdate and dispatch a blocking ticket for a pinned package. A bound pass now returns before it. - AdoptOrigin forwarded its pre-write snapshot. When a concurrent install had stamped another origin first, the candidate was still taken. It now decides on the content the update emitted and takes nothing unless that names this repository. - The test's Reminders helper waits for a row when one is expected, so the query's initial snapshot cannot read as none. Tests: APinnedRecord_IsNotHeldByAForeignCandidate, AnAdoptionLostToAConcurrentInstall_TakesNothing. Both fail without the fix. Refs #6526 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment on lines
+75
to
+76
| ILogger? logger, | ||
| InstallOriginBinding? binding = null) |
Comment on lines
+72
to
+73
| private ImmutableDictionary<string, PluginCatalogContent> watchedOrigins = | ||
| ImmutableDictionary<string, PluginCatalogContent>.Empty.WithComparers(StringComparer.Ordinal); |
Comment on lines
+40
to
+44
| if (PackageSources.IsUrl(repo)) | ||
| { | ||
| var scheme = repo.IndexOf("://", StringComparison.Ordinal); | ||
| repo = (scheme >= 0 ? repo[(scheme + 3)..] : repo).ToLowerInvariant(); | ||
| } |
Comment on lines
+299
to
+303
| // 🚨 The record the update EMITTED, never the pre-write snapshot: a concurrent install or | ||
| // policy edit may have won, and deciding on the snapshot would act on stale policy and | ||
| // authorization fields — or bind a candidate to an origin the record no longer names | ||
| // (review on MeshWeaver#6541). | ||
| .Select(node => node.ContentAs<PackageManifest>(hub.JsonSerializerOptions)) |
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.


What was wrong
PluginUpdateWatcherhanded a watched repository's listing toPackageUpdateReconciler.ReconcileInstalled, which matched install records by package id alone. A second watched repository listing the same id could therefore update (or remind about) a package it never installed.The change
PackageManifest.Originis the canonical identity of the source the install ran through (InstallOrigin): repository URL without scheme,.gitor trailing/, lower-cased, plus#subdir; a registry isregistry:<url>. It is attached to the source where it is built from its configured repository and subdirectory (PackageSources.FromRepo) or byRegistryPackageSource. The one install funnel,CatalogLayoutAreas.InstallOrUpdate, stamps it onto the record and overwrites anyorigina manifest authors. So the catalog click, the boot install and the unattended apply all record it.PackageInstaller.SeedOrigincarries it forward when a re-stamp's source names none. This is the same "the source's identity, never an authored field" rule as fix(plugin-catalog): the repair lane re-asserts only from a source trusted to be the recorded one #6533 (Repair lane: bind the re-assert candidate to a provenance-trusted source, not an authored one #6512) applies toSource.InstallOriginBinding). The check sits after the content-identity gate and before the floor and policy decisions. A candidate is taken only for a record whoseOriginis that repository. A candidate from any other repository is ignored: one Information line, no hold stamped, no reminder.Update refused: <name>naming the package and every candidate repository (new i18n keys, en + de);RegistryUpdateReconciler): its single configured registry is the consumer's trust root and stamps provenance itself. A package installed through a registry recordsregistry:<url>, so a watcher on the same instance never adopts it for a repository.Tests (
Memex.Portal.Shared.Test)UpdateBindsToTheInstallOriginTest:AnInstallRecordsItsOriginTest: a manual install through the funnel records the source's origin over anoriginthe manifest authors.Local:
dotnet build -c Release -warnaserroronMeshWeaver.PluginCatalog,MeshWeaver.Messaging.Hub,MeshWeaver.Documentation,Memex.Portal.Shared.Test,MeshWeaver.Messaging.Hub.Test: 0 warnings, 0 errors. After merging main (with #6533): the new tests +ARepairBindsToATrustedSourceTest,IncompleteInstallIsReassertedTest,PackagesAutoUpdateTest: 10/10. Before the merge:PackageUpdateReminderIdempotenceTest,HeldUpdatesConvergeTest,RegistryUpdateReconcilerTest,UpdateKeepsTheSyncClaimTest,MinMeshVersionHoldsTheUpdateTest: 21/21.LocalizationTestetc.: 66/66.Doc:
Doc/Architecture/PluginUpdateOnGreenBuild, new section An update comes only from the repository the install came from (#6526).Closes #6526
Pairs-with: none — no public type or member removed (one public property added:
PackageManifest.Origin).Mirror-sync:
npm run sync:i18n -- --ref <merged core sha>in MeshWeaver.Plugins after merge; two keys added (notification.packageUpdate.originAmbiguous.title/.body).🤖 Generated with Claude Code