Repository navigation
fix(#1979): answer the restart-as-activation question per PROCESS, and never reassuringly - #2037
Merged
Merged
Conversation
…d never reassuringly #2003 gave PendingRestart an operator surface. Reading that flag, though, answers the wrong question, and the shipped check had two holes that both fail quietly. 1. PendingRestart is ONE deployment-wide boolean that the next boot clears — and on a multi-replica deployment the pod that clears it is not the pod missing the module. Replica A lands a module and sets it; replica B restarts for unrelated reasons, applies the list and resets it; A keeps serving WITHOUT the module while every surface reads "nothing pending". 2. It named every ENABLED entry, not the pending ones — so one landing on a deployment carrying fifteen modules reported "15 module(s) are landed but not yet loaded". 3. `ModuleActivationSidecar.Read` swallows an unparseable file into the EMPTY list, and the check ignored its corruption callback. A corrupt sidecar therefore reported Healthy — "no pending activation" — cheerfully, forever. `ModuleActivationStatus` (pure) derives the honest answer: the persisted enabled entries MINUS what this process actually loaded. That is true per pod, needs no extra state, and names only what is genuinely missing. `PendingModuleActivations` is the one reader every surface consults, and its report distinguishes a KNOWN empty answer from an UNDETERMINED one — a check that cannot reach its evidence must say so, not default to the reassuring answer. The health check reports both Degraded (still never Unhealthy: the pod serves correctly with what it loaded). Registered in DI beside ModuleLandingService, rooted at the same resolved module root, so a NodeType's layout area can resolve it from hub.ServiceProvider — issue item 1, the Store install step, is the surface where the missing last step is actually met, and it now has something to ask. `IsPendingForPackage` answers per package off the entries' PackagePath back-pointer; a blank path is not a wildcard. Tests: ModuleActivationStatusTest (13, pure + over the durable sidecar) and the health check's suite grown to 7. Non-vacuity: against origin/main's health check, 4 of the 7 fail — AClearedFlagWithAnUnloadedModule_IsStillDegraded, AModuleThisProcessHasLOADED_IsNotPending, ADisabledEntry_IsNeverPending and ACorruptSidecar_IsDegraded_NeverHealthy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the “restart-as-activation” signal for landed modules to be process-accurate (per pod) and fail-closed (corrupt/undetermined state is surfaced as such), enabling both operator health reporting and a reusable reader for UI surfaces (e.g., the Store) without relying on the deployment-wide PendingRestart flag.
Changes:
- Introduces
ModuleActivationStatus(pure derivation) andPendingModuleActivations(single reader) to compute which enabled modules are landed but not loaded in the current process. - Updates the portal health check to use the new reader and to report Degraded for both pending activations and undetermined/corrupt sidecar state.
- Adds/extends unit tests to cover multi-replica semantics, corrupt sidecar handling, and per-package “restart required” matching.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.PluginCatalog.Test/ModuleActivationStatusTest.cs | Adds a new test suite covering the pure derivation, per-package matching, and durable-sidecar reader behavior. |
| test/Memex.Portal.Shared.Test/PendingModuleActivationHealthCheckTest.cs | Expands health check tests for multi-replica PendingRestart holes and corrupt-sidecar fail-closed behavior. |
| src/MeshWeaver.PluginCatalog/PluginCatalogConfigurationExtensions.cs | Registers PendingModuleActivations in DI alongside ModuleLandingService using the same resolved module root. |
| src/MeshWeaver.PluginCatalog/PendingModuleActivations.cs | Adds the shared reader (Read, IsPendingForPackage) and a report type that distinguishes “known empty” vs “undetermined”. |
| src/MeshWeaver.PluginCatalog/ModuleActivationStatus.cs | Adds the pure per-process derivation, per-package matcher, loaded-assembly enumeration, and shared description formatting. |
| memex/aspire/Memex.Portal.Distributed/PendingModuleActivationHealthCheck.cs | Switches the health check to the new reader and aligns status/reporting with the fail-closed semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1
to
+2
| using System.Collections.Immutable; | ||
| using System.Reflection; |
Contributor
Contributor
Contributor
Contributor
Contributor
Test Results (shard 2)3 218 tests +3 3 218 ✅ +3 8m 22s ⏱️ +15s Results for commit f3ff80f. ± Comparison against base commit de3f8b7. This pull request removes 1 and adds 4 tests. Note that renamed tests count towards both. |
Contributor
Contributor
Test Results 59 files ± 0 59 suites ±0 50m 3s ⏱️ + 3m 16s Results for commit f3ff80f. ± Comparison against base commit de3f8b7. This pull request removes 1 and adds 17 tests. Note that renamed tests count towards both. |
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.
Continues #1979 (#2003 shipped item 2, the operator surface).
What it was — and what it still is
PendingRestartwas written by both landing sites, consumed at boot, and read by nothing. #2003 gave it a health check. Reading that flag answers the wrong question, and the shipped check had three holes, all of which fail quietly:Enabledentry, not the pending ones. One landing on a deployment carrying fifteen modules reported "15 module(s) are landed but not yet loaded".ModuleActivationSidecar.Readswallows an unparseable file into the empty list and reports it through anonCorruptcallback the check did not pass. So "cannot parse the activation state" rendered as "no pending activation" — cheerfully, forever. Exactly the trap this cluster keeps falling into.Item 1 of the issue (the Store card) was also still open, and could not be built at all: "no API surface, so nothing external can [read it]".
What changed
ModuleActivationStatus(pure,MeshWeaver.PluginCatalog) — the honest question: which enabled entries are not among the assemblies this process loaded. True per pod, needs no extra state, names only what is genuinely missing, and immune to another replica's boot. ADisabledentry is never pending (it is the record of an uninstall; the module's absence is the outcome).PendingModuleActivations— the one reader every surface consults. ItsModuleActivationReportseparates a known empty answer from an undetermined one, andDescribe()is shared so an operator and a buyer are never told different numbers.The health check now asks that reader, and reports Degraded for both pending and undetermined — still never
Unhealthy: the pod serves correctly with what it loaded, and failing readiness would stall a rollout over work the rollout itself performs.Registered in DI beside
ModuleLandingService, rooted at the same resolved module root, so a NodeType's layout area can resolve it fromhub.ServiceProvider.IsPendingForPackage(packagePath)answers per package off the entries'PackagePathback-pointer — that is what the Store install step needs to say "restart required to finish activating this" instead of a bare "installed". A blank path matches nothing; it is not a wildcard.Note the deliberate asymmetry: an undetermined state does not put a restart prompt on every package card. It is an operator signal, reported where it is actionable — telling a buyer to restart because a file would not parse is noise they cannot act on.
Tests
test/MeshWeaver.PluginCatalog.Test/ModuleActivationStatusTest.cs(13) and the health check's suite grown to 7.Fail-closed proofs:
ACorruptSidecar_IsUNDETERMINED_NotEmpty,ACorruptSidecar_IsDegraded_NeverHealthy,AnUndeterminedState_DoesNotPutARestartPromptOnEveryCard.Non-vacuity — the health check reverted to
origin/main's implementation, same tests:For the maintainer
The remaining half of item 1 — rendering it on the package card — is a change in MeshWeaver.Plugins (
Store/Plugin/Source/PluginLayoutAreas.cs, the install CTA), not here. This PR is the platform seam it needs; it is deliberately a separate PR because the two repos deploy on different lanes.PendingRestartitself is left exactly as it is — still written, still consumed at boot. Nothing now depends on it being accurate across replicas, which is the property it never had.🤖 Generated with Claude Code