fix(#1814): the compilation fault card re-evaluates instead of latching - #1870
Merged
Conversation
Defect B of #1814 — the half that turned a ten-minute deploy window into a two-hour public outage. On 2026-08-17 every course cover on memex served "this page can't be displayed" for 1 h 24 m AFTER Store/Plugin had compiled successfully on both pods, while neither pod logged a single overlay or "did not settle" event in the preceding 30-40 minutes. Nothing was retrying. Twelve roots had to be recycled by hand. THE LATCH. Enrichment binds a per-instance hub's HubConfiguration exactly once (the `node.HubConfiguration != null` short-circuit), so an overlay applied during a bad ten seconds is served for the grain's whole lifetime unless something revokes it. ArmOverlaySelfHeal is that revocation — and both of its routes (version-advance, and the 45 s grace re-read) subscribe `meshHub.GetWorkspace().GetMeshNodeStream(nodeType)`: the SAME stream whose silence made the enrichment slow path time out in the first place. The heal signal and the fault therefore shared a single point of failure, and the grace route's re-subscribe replays the workspace's cached snapshot rather than re-reading anything. One silent stream, and the card is permanent. THE FIX. A third heal route that owes the stream nothing: * AuthoritativeTypeRead — a one-shot `path:{nodeType}` query through IMeshQueryCore, as System: the mesh's query providers (storage), not a cached stream, so a mirror that can never learn it is stale cannot suppress it. Matches the path exactly; a neighbouring hit is not an answer. * A widening ladder, not a poll — 45 s, 90 s, 3 min, 6 min, then 10 min for ever. One read per rung, each capped at 30 s and serialised with Concat so a slow read can never overlap the next. At the 2026-08-17 blast radius (12 latched instances) steady state is ~72 single-node reads an hour. * The ladder never stops. A re-evaluation budget that ran out would restore exactly the defect: a card outliving its cause because something decided to stop looking. * A faulted or empty read is not a verdict — logged, and the ladder asks again. Non-convergence is loud: the status/assembly/framework the re-read actually saw, at Warning once past the last rung, beside the existing admin notification. BOUNDING THE RECYCLE. The heal disposes the hub that owns the watcher, so the replacement hub's watcher restarts at the first rung — a pair whose re-enrichment keeps faulting would recycle every 45 s for ever, and no state inside the watcher can bound that. OverlayHealBudget (mesh-scoped singleton, keyed by instance+NodeType) is the memory that survives: the first heal is never delayed, each further heal inside 30 minutes waits out a widening spacing. It DEFERS a recycle, never cancels one. A genuinely broken type still keeps its card — the goal is a card that clears when its cause clears, never one that hides a real problem. The overlay copy's promise ("the page recovers automatically") is now true, and says how it recovers. Tests (non-vacuity proven by neutering the new route — 5 of 12 fail with main's push-only self-heal, including "the instance never recycled itself"; the 7 pre-existing tests stay green): - SilentTypeStream_ReEvaluatesFromStorage_AndHealsWithoutIntervention - StillBrokenType_KeepsItsCard_AndTheLadderStaysBounded - FaultedReRead_DoesNotEndTheLadder / EmptyReRead_IsNotAHealSignal - RepeatedlyStuckInstance_HasItsNextRecycleDeferred_NeverCancelled - NonConvergingInstance_RecyclesAreSpaced_NotOncePerLadderRung (with the un-budgeted control in the same test) - OverlayHealBudgetTest, OverlayReEvaluationReadTest (the read seam itself: query core, System identity, exact path — a wiring every shape test would pass against a read that can never answer)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #1814 defect B where the compilation fault overlay could “latch” on an instance hub indefinitely when the NodeType stream stopped emitting, by adding an authoritative storage re-read ladder and introducing cross-recycle spacing to prevent recycle storms.
Changes:
- Add a third self-heal route that periodically re-reads the NodeType via
IMeshQueryCore(storage) on a widening ladder, so the overlay can clear even when the cached stream is permanently stale. - Add
OverlayHealBudget(mesh-scoped singleton) to space repeated self-recycles across hub lifetimes for non-converging instance/type pairs. - Add focused unit + e2e-style tests and update docs/What’s New to reflect the now-true “recovers automatically” behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Graph.Test/OverlaySelfHealWatcherTest.cs | Adds regression tests covering silent type-stream re-evaluation, ladder behavior, and bounded recycle spacing. |
| test/MeshWeaver.Graph.Test/OverlayReEvaluationReadTest.cs | Pins production wiring of the authoritative re-read seam (query core, System identity, exact-path match). |
| test/MeshWeaver.Graph.Test/OverlayHealBudgetTest.cs | Verifies the spacing/forgetting contract of OverlayHealBudget. |
| src/MeshWeaver.Mesh.Contract/Services/IMeshQueryCore.cs | Grants internals visibility to Graph tests so the seam can be substituted directly. |
| src/MeshWeaver.Graph/Configuration/OverlayHealBudget.cs | Introduces cross-hub-lifetime spacing memory for overlay self-heals. |
| src/MeshWeaver.Graph/Configuration/NodeTypeEnrichmentHelpers.cs | Implements the new re-evaluation ladder + authoritative read and wires it into overlay self-heal. |
| src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs | Registers OverlayHealBudget as a mesh-scoped singleton in AddGraph(). |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-18-a-broken-page-stops-being-broken-by-itself.md | Adds a What’s New entry describing the operationally important behavioral change. |
| src/MeshWeaver.Documentation/Data/Architecture/NodeTypeCompilation.md | Documents the “fault card must not outlive its cause” invariant and the new re-evaluation/spaced-recycle design. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+58
to
+63
| private readonly ConcurrentDictionary<string, Entry> entries = | ||
| new(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| private sealed record Entry(int Heals, DateTimeOffset LastHeal); | ||
|
|
||
| private static string Key(string instancePath, string nodeType) => $"{instancePath} {nodeType}"; |
Comment on lines
+1953
to
+1956
| var request = MeshQueryRequest.FromQuery($"path:{nodeType}") with | ||
| { | ||
| UserId = WellKnownUsers.System, | ||
| }; |
Test Results (shard 4)1 758 tests +22 1 755 ✅ +24 7m 2s ⏱️ - 1m 22s Results for commit 81fc01c. ± Comparison against base commit f7cde96. This pull request removes 163 and adds 185 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
CI's ImpersonationScopeSiteRatchetGuard caught the new site: the authoritative re-read copied the enrichment probe's `Observable.Using(() => access.ImpersonateAsSystem(), _ => query)` shape, which opens the AsyncLocal scope on the SUBSCRIBING thread and disposes it when the query terminates — the owning hub's response thread — leaving the subscriber latched as system-security. Here the subscriber is an instance hub's long-lived overlay watcher, so that latch would sit on a hub, which is the worst place for it. Use ImpersonationScopeExtensions.RunAsSystem, which seals both ends inside one Subscribe, with the whole cold pipeline composed inside the work factory so emission-time behaviour is unchanged. The ratchet's inventory does not grow (NodeTypeEnrichmentHelpers.cs stays at its allowed 1 — the pre-existing probe).
Contributor
Author
|
Follow-up commit The authoritative re-read copied the enrichment probe's idiom, Replaced with CI is now green end to end: all 6 shards, |
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.
Closes defect B of #1814 — the latching fault card, and the reason a ten-minute deploy window became a two-hour public outage.
The defect, precisely
Enrichment binds a per-instance hub's
HubConfigurationexactly once — thenode.HubConfiguration != nullre-enrichment short-circuit is what keeps activation cheap — so a compilation overlay applied during a bad ten seconds is served for the grain's whole lifetime unless something revokes it.ArmOverlaySelfHealis that revocation.Both of its heal routes subscribe
meshHub.GetWorkspace().GetMeshNodeStream(nodeType)— the same stream whose silence made the enrichment slow path time out in the first place:The grace route's "re-read" is a re-subscribe to the workspace's cached stream, so it replays the same snapshot. Neither route reads anything. The heal signal and the fault shared a single point of failure: one permanently silent stream and the card is permanent.
That is exactly what was measured on 2026-08-17 (#1814):
Store/Plugincompiled successfully on both pods, and 1 h 24 m later an anonymous browser still got the card, with no overlay and no "did not settle" event logged on either pod in the preceding 30–40 minutes. The 120 s stuck-reporter fired (platform admins notified) — proving the watcher was armed and had simply never seen a usable state. Twelve roots were recycled by hand.The fix — a third route that owes the stream nothing
AuthoritativeTypeRead— a one-shotpath:{nodeType}query throughIMeshQueryCore, as System: the mesh's query providers (storage), not a cached stream, so a mirror that can never learn it is stale cannot suppress it. It matches the path exactly; a ranked/fuzzy hit for a neighbouring node is not an answer about this type.A widening ladder, not a poll.
45 s → 90 s → 3 min → 6 min, then 10 min for ever. One read per rung, each capped at 30 s and serialised withConcatso a slow read can never overlap the next rung. At the 2026-08-17 blast radius (12 latched instances) the steady state is ~72 single-node reads an hour — against the 1 h 24 m of served fault cards it replaces. The naive shape (re-probe per render) is explicitly not what this is.The ladder never stops. A re-evaluation budget that ran out would restore precisely the defect: a card outliving its cause because something decided to stop looking.
A faulted or empty read is not a verdict — it is logged and the ladder asks again. Non-convergence is loud: each re-read that still finds no usable build logs the status / assembly / framework it actually saw, at
Warningonce past the last rung, beside the existing admin notification.Bounding the recycle, which the watcher cannot do itself
The heal disposes the hub that owns the watcher. The replacement hub arms a fresh watcher whose ladder starts at the first rung — so a pair whose re-enrichment keeps faulting (a type reporting a usable build that the instance still cannot bind: #1814's deterministic cross-hub
Conflict) would recycle every 45 s for ever. No state inside the watcher can bound that, because the bound has to outlive the thing being bounded.OverlayHealBudget— mesh-scoped singleton registered inAddGraph, keyed by (instance, NodeType), instance maps only — is that memory. The first heal is never delayed (the common case is a deploy window that cleared). Each further heal inside 30 minutes waits out a widening spacing (45 s → 90 s → 3 min → 6 min → 10 min). It defers a recycle, never cancels one; a pair that heals once and stays healthy is forgotten.What deliberately did NOT change
A genuinely broken type still shows its card. The goal is "clears when the cause clears", never "never shows".
StillBrokenType_KeepsItsCard_AndTheLadderStaysBoundedpins both halves.The overlay copy's promise — "the page recovers automatically" — was false for 1 h 24 m and is now true; it also says how it recovers, so an operator knows how long to wait before reaching for Recycle.
Non-vacuity — proven by neutering, with real output
The new route removed (
_ = reEvaluated;— i.e.origin/main's push-only self-heal), same machine, same test binary:The first line is the 2026-08-17 behaviour: the instance never recycled itself. The 7 pre-existing tests stay green under the neuter, so they cover the old behaviour and the 5 new ones are non-vacuous.
With the fix restored:
All time is driven by
TestScheduler— this is a latching bug, so every wait is bounded and virtual; nothing sleeps.Tests
SilentTypeStream_ReEvaluatesFromStorage_AndHealsWithoutInterventionStillBrokenType_KeepsItsCard_AndTheLadderStaysBoundedFaultedReRead_DoesNotEndTheLadder/EmptyReRead_IsNotAHealSignalRepeatedlyStuckInstance_HasItsNextRecycleDeferred_NeverCancelledNonConvergingInstance_RecyclesAreSpaced_NotOncePerLadderRungOverlayHealBudgetTestOverlayReEvaluationReadTestWhat remains of #1814
Conflictretry (PROD memex: every course cover serves the 'type has not finished compilation' fallback — Store/Plugin build has not settled #1814's item 3) is partly addressed on main already:UpdateRemotenow includesMeshNodeErrorCode.Conflictin the re-enqueue set, and the recursion genuinely re-runs the lambda against a re-read initial state. But it re-reads throughAcquireRemoteStreamUnchecked— the same workspace mirror — so with the notify chain broken the base is stale on every attempt and the two re-enqueues cannot converge. Nothing further to change here; it is subsumed by the item below.mesh_node_notifyinstalled per database rather than per table (Install mesh_node_notify per TABLE, not once per database (V54) #1816, open),PostgreSqlChangeListenerregistered but commented out for the partitioned setup (FollowGo waits unboundedly for a GO that cannot cross processes — the PG change listener is registered but never started #1440, open — the TODO is still inPostgreSqlExtensions.cs), and notifications publishingEntity = null, whichMeshDataSourcediscards.So #1814 should stay open until the notify chain lands. What this PR changes is the severity: with the card re-evaluating, a stale mirror costs a page that recovers on its own within a minute (or ten, if it keeps failing) instead of an open-ended outage that only twelve manual recycles could end.
🤖 Generated with Claude Code