Skip to content

Hold a package update whose declared requirement the loaded dependency does not meet - #6488

Merged
rbuergi merged 7 commits into
mainfrom
fix/6192-update-holds-unmet-dependency
Oct 12, 2026
Merged

rbuergi merged 7 commits into
mainfrom
fix/6192-update-holds-unmet-dependency

Conversation

@rbuergi

@rbuergi rbuergi commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What changed

  • PackageDependencyFloorGate (new, src/MeshWeaver.PluginCatalog): reads a candidate package's requires against the modules this process has LOADED (ILoadedPackageModules, the reading the GitSync import already uses). Pure decision Unmet, plus RecordHold / ClearMetHold on the install record.
  • CatalogLayoutAreas.InstallOrUpdate (the one install orchestrator: boot install, unattended apply, click): an UPDATE over an existing record whose candidate declares a requirement the loaded dependency does not meet is held. Nothing is fetched or written, the installed version and its record stay, and heldUpdate names the requirement, the module that runs and its version.
  • PackageUpdateReconciler: asks the same question before it applies or raises a reminder, so a held update is not offered either. A met requirement clears the hold.
  • ModuleSetConvergence.md: the third lane, what was measured, what is not gated.

Why

[ModuleSet] NOT proposing the landed module set: 'X' requires Dep@^a.b, but this set loads … at c.d (fingerprint 4a091bf82bb98201 and its siblings on #6192) is the proposal check working. What it reports is an install record that already states a floor the running dependency does not meet. The GitSync import declines such a module before it writes (ModuleSyncDecision.DeclineUnmetRequirements); the installer did not ask, so a package updated from the registry moved its record onto the new floor and every landing wave on the deployment was refused until the dependency's module landed.

Read on the control instance on 2026-10-11: Plugins/Governance is at 0.16.1 with requires: Essentials@^1.12.0 (installed from a commit ref by system-security), while Plugins/Essentials reads heldUpdate: … kept by its sync source … converges when the sync lands it, and the proposal named the Essentials module at 1.11.26. The same shape held for AppleWeather / AI 1.26.2 and AppleMaps / Maps 1.0.4. The refusals ran from 2026-10-10 17:26Z to 2026-10-11 05:11Z on every replica.

Governance and AppleWeather ship no module (module is absent from their index.json), so no rule on the module lane can hold them back. That includes #6468, which holds a dependent's MODULE at the accepted set's generation and refuses the wave when the dependent "lands no module here". The two changes do not overlap: this one keeps the record from taking the floor; #6468 changes what the proposal does once a record has it.

Not covered

  • A fresh install (nothing installed to keep; the installer orders dependencies first), a re-install of the recorded content, and a sync-owned partition (its import applies the same rule, and the record must adopt what that owner landed).
  • The reading is per process. While replicas run different generations of the dependency, a replica that runs a satisfying one applies the update for all of them; this is the same reading and the same limit as the GitSync import's rule. A deployment-wide verdict (the accepted module set) is not consulted here.
  • A fresh install is not held even though a dependency installed earlier in the same closure is landed and not yet loaded: refusing it would fail a new instance's first boot for every default package whose floor is above the image's own module copy. That case is still caught at the module-set proposal, as before.
  • A hold sends no blocking ticket to the control inbox. PackageHoldTicket is shaped for a platform floor; the hold is on the record (heldUpdate, heldSince) and logged once per held state at Warning.
  • Which installer lane moved the two records was not read from the logs (a Logs read for Package update: Governance|auto-updating Governance over the 12 h returned 0 lines). All installer lanes go through InstallOrUpdate.
  • Records that already carry an unmet floor are not rewritten; they converge when the dependency loads, as today.

How tested

test/Memex.Portal.Shared.Test/PackageDependencyFloorHoldTest.cs, on the platform-floor fixture (FloorHoldFixture) with the real reconciler and orchestrator; the loaded-module reading is set by the test, as in ADependencyFloorHoldsTheImportTest.

  • Auto update held while the loaded dependency is 1.11.26 against ^1.12.0: 0 fetches, v1 text and hash stay, the record's requires stays empty, heldSince is stable over two more passes; with 1.12.0 loaded the same candidate lands and the hold clears.
  • Notify update: no reminder while held, reminder once met, hold cleared first.
  • InstallOrUpdate directly (boot / click lane): InstallResult(0, 0) while held, lands once met.
  • Controls: a dependency with no loaded module is not judged and the update lands; a fresh install is not held.
  • A hold decided on a stale record is not stamped once the candidate has landed (review).
  • Pure: the range matrix, the three hold sentences told apart, and ModuleDependencyFloor.Unmet over a held record (empty) against an advanced one (the refusal).

Negative control: with both wirings reverted (the orchestrator's and the reconciler's) and the tests kept, the three hold arms fail (AnAutoUpdate_IsHeld…, ANotifyUpdate_IsNotOffered…, InstallOrUpdate_HoldsTheUpdate…) and the two controls pass: 3 of 5 red. Restored: 5 of 5 green.

Local: dotnet build test/Memex.Portal.Shared.Test -c Release -warnaserror → 0 Warning(s), 0 Error(s); the new tests plus PackageFloorHold*, HeldUpdatesConverge*, OnePartitionOneBookkeeping*, PackageUpdateReminderIdempotence* pass.

Refs #6192

🤖 Generated with Claude Code

rbuergi and others added 3 commits October 11, 2026 13:21
…aded dependency does not meet

The unattended registry update judged a candidate by its content hash and its
platform floor only. An auto-updating package therefore took a version whose
requires entry the dependency this instance runs did not meet: its install
record then declared that floor, and the module-set proposal refused every
landing wave on the deployment until the dependency's module landed.

PackageUpdateReconciler now reads the candidate's requires against the loaded
modules (ILoadedPackageModules, the reading the GitSync import uses) before it
applies or offers an update. An unmet requirement holds the update: nothing is
fetched, the installed version and its record stay, heldUpdate names the
requirement and what runs, and the first reconcile after a satisfying
dependency is loaded applies it.

Refs #6192

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…the boot install and the click go

The reconciler is one of three lanes; the boot install and a click on Update
reach the installer through CatalogLayoutAreas.InstallOrUpdate without it.
The dependency hold now sits there, for an update over an existing record.
A fresh install and a re-install of the recorded content are not held.

Refs #6192

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Fresh installs and mixed-replica races can still expose processes to incompatible dependency versions.

4 open findings
What changed in this PR

Adds dependency-floor gating so incompatible package updates are held until the required loaded module version is available.

Changes:

  • Adds dependency evaluation and hold bookkeeping.
  • Integrates gating into install, auto-update, and notification paths.
  • Adds integration tests and architecture documentation.
File Description
PackageDependencyFloorHoldTest.cs Tests hold, release, notification, and install behavior.
PackageUpdateReconciler.cs Gates automatic updates and reminders.
PackageDependencyFloorGate.cs Implements evaluation and hold state management.
CatalogLayoutAreas.cs Enforces the gate before installation.
ModuleSetConvergence.md Documents the new convergence lane.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/MeshWeaver.PluginCatalog/CatalogLayoutAreas.cs
Comment thread src/MeshWeaver.PluginCatalog/PackageDependencyFloorGate.cs Outdated
Comment thread test/Memex.Portal.Shared.Test/PackageDependencyFloorHoldTest.cs
Comment thread src/MeshWeaver.Documentation/Data/Architecture/ModuleSetConvergence.md Outdated
…e candidate has landed

Review on #6488: RecordHold wrote the hold from a record read before the write.
If another replica or a click installed the candidate in between, the hold sat
on the installed version for good, because the next reconcile ends at the
equal-hash gate before anything clears a hold. The update now re-checks the
live record and leaves it alone when it already describes the candidate.

Refs #6192

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 11:38
@meshweaver-cloud
meshweaver-cloud Bot disabled auto-merge October 11, 2026 11:38
The fresh install is left ungated because refusing it on the loaded reading
would fail a new instance's first boot, not because it is safe; and the
reading is per process. Both are now stated where the rule is documented.

Refs #6192

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@rbuergi
rbuergi enabled auto-merge (squash) October 11, 2026 11:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Storage-failure bypasses and stale concurrent hold updates can permit incompatible updates or leave incorrect hold state.

2 open findings
4 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Clear dependency holds before applying pinned policy

src/​MeshWeaver.PluginCatalog/​PackageUpdateReconciler.cs:224

The dependency gate is invoked only inside the Auto and Notify arms. If a held package is changed to Pinned, a later satisfying dependency never reaches ClearMetHold, so HeldUpdate and HeldSince continue claiming an unmet requirement forever. Evaluate and clear an existing dependency hold before the policy switch, as the platform-floor hold already does.

🧠 Review effort: Balanced

Comment thread src/MeshWeaver.PluginCatalog/CatalogLayoutAreas.cs Outdated
Comment thread src/MeshWeaver.PluginCatalog/PackageDependencyFloorGate.cs Outdated
Copilot AI balanced review requested due to automatic review settings October 11, 2026 11:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Hold transitions currently reset the continuous hold timestamp and leave related public documentation inaccurate.

2 open findings
Previously missed (2)

In code that hasn't changed since last review

Medium severity Preserve HeldSince when dependency holds replace existing holds

src/​MeshWeaver.PluginCatalog/​PackageDependencyFloorGate.cs:197

This resets the age when a dependency hold replaces a platform or sync hold, even though the package has remained continuously held. PackageManifest.HeldSince explicitly stays fixed while HeldUpdate remains set, and both other hold writers preserve current.HeldSince; resetting it here makes the fleet's hold-duration reporting understate how long the package has been blocked.

Low severity Update HeldUpdate documentation to cover dependency holds

src/​MeshWeaver.PluginCatalog/​PackageDependencyFloorGate.cs:150

This new use makes PackageManifest.HeldUpdate represent dependency holds, but that public property's XML documentation still says it is set only for MinMeshVersion/platform-floor holds. Update the property documentation in Package.cs to describe all hold reasons so API consumers do not misclassify this newly persisted value.

🧠 Review effort: Balanced

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   12 files   -    12     12 suites   - 12   35m 43s ⏱️ - 18m 58s
7 493 tests  - 4 231  7 493 ✅  - 4 038  0 💤  - 193  0 ❌ ±0 
7 505 runs   - 4 233  7 505 ✅  - 4 040  0 💤  - 193  0 ❌ ±0 

Results for commit 9fc521a. ± Comparison against base commit 977fbcd.

This pull request removes 4281 and adds 26 tests. Note that renamed tests count towards both.

   --- End of inner exception stack trace ---
   --- End of inner exception stack trace ---, expected: True)
   --- End of inner exception stack trace ---, isDenial: True)
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
 ---> (Inner Exception #1) MeshWeaver.Messaging.Hub.Test.InfrastructureFaultTest+ProviderException (0x80004005): Failed to connect to 10.42.18.4:5432<---
 ---> (Inner Exception #1) System.ArgumentException: Value does not fall within the expected range.<---
 ---> (Inner Exception #1) System.InvalidOperationException: boom<---
 ---> (Inner Exception #1) System.InvalidOperationException: source B is misconfigured<---
 ---> (Inner Exception #1) System.Net.Sockets.SocketException (0xFFFDFFFF): Name or service not known<---
…
Memex.Portal.Shared.Test.InstanceIdRulesMatchTheRegistryTest ‑ TheSetupHostAgreesWithTheRegistry(candidate: "3513f792-dd3d-471c-a124-f09aefafd076")
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ AHeldRecord_LeavesTheModuleSetProposable_WhereAnAdvancedOneRefusesTheWave
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ ARequirementIsUnmet_OnlyWhenTheLoadedDependencyIsOutsideItsRange(requirement: "AI@^1.27.0", loadedEssentials: "1.11.26", held: False)
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ ARequirementIsUnmet_OnlyWhenTheLoadedDependencyIsOutsideItsRange(requirement: "Essentials", loadedEssentials: "1.11.26", held: False)
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ ARequirementIsUnmet_OnlyWhenTheLoadedDependencyIsOutsideItsRange(requirement: "Essentials@^1.12.0", loadedEssentials: "1.11.26", held: True)
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ ARequirementIsUnmet_OnlyWhenTheLoadedDependencyIsOutsideItsRange(requirement: "Essentials@^1.12.0", loadedEssentials: "1.12.0", held: False)
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ ARequirementIsUnmet_OnlyWhenTheLoadedDependencyIsOutsideItsRange(requirement: "Essentials@^1.12.0", loadedEssentials: "1.13.4", held: False)
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ ARequirementIsUnmet_OnlyWhenTheLoadedDependencyIsOutsideItsRange(requirement: "Essentials@^1.12.0", loadedEssentials: "2.0.0", held: True)
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ ARequirementIsUnmet_OnlyWhenTheLoadedDependencyIsOutsideItsRange(requirement: "Essentials@not-a-range", loadedEssentials: "1.11.26", held: False)
Memex.Portal.Shared.Test.PackageDependencyFloorDecisionTest ‑ ARequirementIsUnmet_OnlyWhenTheLoadedDependencyIsOutsideItsRange(requirement: "essentials@^1.12.0", loadedEssentials: "1.11.26", held: True)
…

♻️ This comment has been updated with latest results.

rbuergi and others added 2 commits October 11, 2026 15:45
…ct a moved record

- HoldIfADependencyIsBelowItsFloor no longer reads a faulted install-record
  read as "no record" (a fresh install): the fault propagates, so a transient
  storage fault cannot bypass the hold and advance the record onto the unmet
  floor.
- RecordHold stamps only while the live record is still at the version the
  hold was decided on (PackageDependencyFloorGate.Moved), not merely while it
  is not at the candidate.
- ClearMetHold clears only the hold that was judged met: same version, same
  hold sentence.

Three tests, each red with its fix reverted.

Refs #6192

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 13:48
@meshweaver-cloud
meshweaver-cloud Bot disabled auto-merge October 11, 2026 13:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Read failures can bypass the gate, and conditional clearing can continue with stale state.

3 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment on lines +113 to +123
return modules.Read()
.Take(1)
.DefaultIfEmpty(ImmutableDictionary<string, LoadedPackageModule>.Empty)
.Select(loaded => Unmet(candidate, loaded))
.Catch((Exception ex) =>
{
logger?.LogWarning(ex,
"Package update: the loaded modules could not be read, so the requirements of {Id} "
+ "are NOT judged on this pass.", candidate.Id);
return Observable.Return<UnmetPackageDependency?>(null);
});
Comment on lines +264 to +266
.Take(1)
.Select(_ => cleared)
.DefaultIfEmpty(cleared)
Comment on lines +236 to +240
/// <summary>
/// A FRESH install is not held — the CURRENT behaviour, stated, not a claim that it is safe: a
/// dependency installed earlier in the same closure is landed, not loaded. Refusing it on the
/// loaded reading would fail a new instance's first boot for every default package whose floor is
/// above the image's own module copy; the module-set proposal still names such a floor.
@rbuergi
rbuergi enabled auto-merge October 12, 2026 15:20
@rbuergi
rbuergi merged commit db29146 into main Oct 12, 2026
@rbuergi
rbuergi deleted the fix/6192-update-holds-unmet-dependency branch October 24, 2026 14:13
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.

2 participants