Skip to content

fix(mesh,hosting): a recycle and a bake must ask a survivor, not the thing that is dying (#5099, #5073) - #5163

Merged
meshweaver-cloud[bot] merged 6 commits into
mainfrom
worktree-agent-a2e10f11778fcd8ab
Sep 22, 2026
Merged

meshweaver-cloud[bot] merged 6 commits into
mainfrom
worktree-agent-a2e10f11778fcd8ab

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Two unrelated sev:H release blockers. They do not share a root — I looked, and the only thing they have in common is a sentence: a read was answered by something that was already gone. One is a dying DI scope, the other a dead partition's index row. Neither fix touches the other's code.

Both were, until this PR, the only two of core's nineteen open sev:H with no merged PR ever touching them.


#5099 — the recycle cascade derived its work THROUGH the hub it was tearing down

MessageHub.HandleDispose runs the RecycleCascade seam and then calls Dispose(), on the stated reasoning that the network is derived "while this hub is still whole enough to compute it". That holds for the derivation's synchronous prologue and for nothing after it: each enumeration leg is a cross-hub query composed as Concat, so the later legs subscribe on a pool thread after Dispose() completed and HostedHubsCollection.CloseScopeWhenDisposed closed that hub's Autofac scope. There is no race to lose — by construction the later legs run against a dead scope, and MeshService.StampViewer → CaptureContext() resolves AccessService from it on every Query<T>.

Production, memex.systemorph.com: Hosting/TriageItem recycled 0 addresses with one leg INCOMPLETE, and its live instance hubs kept the assembly they were born with — the exact stale-state problem the cascade exists to remove, wearing the cascade's own success at every surface except one log line.

Fix: DependencyNetwork enumerates through the mesh's read-issuing hub (MeshExtensions.ReadIssuingHub). Three independent reasons that is the right survivor: hosted by the mesh hub so the definition hub's teardown cannot touch it; off the router; and not the node-CRUD execution hub, so these reads are not queued behind every write in flight (#2901).

🚨 This is NOT the hoist that Doc/Architecture/DisposedScopeAndDyingHubs R3 prescribes, and it is worth saying why — the resolves were already hoisted, eagerly, in the prologue, exactly as R3 says. They still threw. IMeshService is scoped per hub: the hoisted instance holds the dying hub and re-resolves from it on every call, so hoisting relocates the throw into the service's own body instead of removing it. R3's rule needs its companion clause: hoist when the service outlives the scope; when the service is the scope, resolve from a survivor.

#5073 — a stale index row confirms itself, so absence was unaskable

The bake sweep's population is a live nodeType:NodeType partitions:all query, and nothing asked whether an enumerated row's partition still had a store behind it. The sweep's only absence test (TypeNodeExists) is a listing — the same index the row came from — so a stale row answers "still there" about itself. Absence was not merely unproven; it was unaskable.

Both doors are dead ends, and only one matches the issue title:

Path What happens Cost
activation-driven SubscribeRequest to a partition hub with no store, never answered a full per-type budget (5 min default) per row, ending TimedOut
batch-driven (what a readiness-gated pod uses) the compile's state write faults OwnerUnreachable a Faulted image verdict

A TimedOut is correctly non-gating — but what withholds readiness is BakePhase.Running, i.e. the sweep not being finished, not the verdict it reaches. The leniency already in the gate does not help. 31 dead rows held one pod 2/3 for 48+ minutes; under maxUnavailable: 0 the old pod could not retire, so two builds served one host (users saw dropped MCP sessions and read timeouts). On the batch path the Faulted verdict cannot be retracted, because the rescue built for exactly that shape asks the same index — so readiness is refused forever, which is the title.

Fix: PartitionExistenceProbe asks the writable storage providers — the one party that knows the store rather than the index — once per distinct pending partition, in front of the work. That ordering is the fix: asked afterwards the budget is already spent or the verdict already self-confirming. A type in a confirmed-absent partition is reported PreWarmStatus.Removed (already non-gating, already cascading as UpstreamContentBroken, already in the report — so no new enum member, no exhaustive switch to widen) and is not warmed.

Fail-open throughout, asserted case by case, because the wrong direction is worse than the defect: a false "absent" would make a pod skip real NodeTypes and serve a mesh it never baked. Absence is confirmed only by a provider that says so with none contradicting it; indeterminate, contradicted, errored, timed-out and no-providers-at-all each have their own test row and each leaves the sweep byte-for-byte as it was. PathResolutionService's own copy of the same vote now delegates to the shared probe, so the two cannot drift about what gone means.

The row is named, never deleted — a boot sweep that pruned mesh data on the strength of a probe would be a far worse failure than the one this closes.

What this deliberately does NOT fix (in the doc page too)

  • The stale rows are still there. Nothing prunes a NodeType catalog row when its partition is dropped — PartitionDropPostDeletionHandler drops the schema and Admin/Partition/{id} and stops. This makes the rows harmless to the readiness gate, not absent. That is the issue's second stated defect and is a separate change.
  • A partition dropped mid-sweep is not covered: the probe is one reading taken before the first type is warmed.
  • Other consumers of the same population must ask the question themselves; the witness is now shared and cheap.

Controls — both sides, measured, with the failing signal quoted

A control that comes back green is a claim about the test, not about the fix, so each was run against a deliberately broken implementation.

#5099 — ARecycleCascadeEnumeratesFromASurvivorTest

with the fix with var reader = hub restored
total 3 3
failed 0 2
succeeded 3 1

Both failures cite the verbatim production string: ObjectDisposedException: Instances cannot be resolved and nested lifetimes cannot be created from this LifetimeScope as it (or one of its parent scopes) has already been disposed. — one as 2 Incomplete legs (the production shape: composed while whole, subscribed after death), one thrown synchronously (called after death). The passing case on the broken build is the calibration case (hub alive), which is what proves the other two are not failing for an unrelated reason.

Two things keep those cases from going vacuous: the precondition is asserted (a resolve from the dead hub must throw — if the scope-closing order ever changes, the cases would otherwise pass having exercised nothing), and every case asserts the instance path is in the network rather than merely that the answer is "complete" — an empty network is complete too, and "complete, 0 addresses" for every input is the silent form of the very defect.

#5073 — ADeletedPartitionIsNotPumpedTest

with the fix contradiction guard + skip predicate broken
total 17 17
failed 0 5
succeeded 17 12

The 5 are exactly the fail-open control (another provider says the partition IS there), all three APartitionNobodyDenied_IsWarmedAsBefore rows, and the real-mesh inertness control. So both decisions are load-bearing, and the inertness control is the one that makes this safe to ship: on an ordinary monolith mesh nothing is confirmed absent, so the change is inert where it gets no real answer.

Also asserted directly against the gate rather than inferred from documentation: ASkippedType_DoesNotHoldReadiness — because "never becomes ready" was the release-blocking cost.

Build verification

dotnet build -c Release -warnaserror, one project per invocation, each reporting 0 Error(s) / 0 Warning(s): MeshWeaver.Graph, MeshWeaver.Hosting, and all nine direct dependents (Hosting.Monolith, Hosting.Orleans, Hosting.AspNetCore, PluginCatalog, GitSync, Mesh.Operations, Memex.Portal.Shared, plus the three test projects). Whole-solution build is blocked by policy in this worktree, so the dependent set was enumerated from the .csproj references rather than assumed.

DocumentationLinkIntegrityTest: 1/1 passed, 0 failed, on a MeshWeaver.Documentation.dll rebuilt at 02:37Z — the pages are embedded resources, so the fresh timestamp is what proves the test actually read them. All eight sibling pages referenced were confirmed present as files first.

Both suites were re-run after merging origin/main (b9d7fa70b), because #5159 landed disposal/shutdown work directly adjacent to #5099 and MeshExtensions.cs was in its diff: 3/3 and 17/17 on the merged base, with ReadIssuingHub/MeshReadHub/GetMeshHub confirmed untouched by that diff.

Gates

  • Pairs-with: — not required. The diff removes zero public or protected members (measured: git diff origin/main...HEAD -- 'src/**/*.cs' | grep -cE "^-.*(public|protected)" → 0). Everything here is additive: PartitionExistenceProbe is new, SkipForAbsentPartition is new and internal, AbsentPartitionsAmongPending widened private→internal. DependencyNetwork's signature is unchanged.
  • Mirror-sync: none — no i18n catalog key added or re-worded (0 files under strings.{en,de}.json). The added strings are operator ILogger lines, which this repo keeps in English like every line around them.
  • Implementers: not applicable — no member added to a public interface or abstract class.

Closure

🚨 Both issues are sev:H, so they close only on PRODUCTION verification, never on this merge. I am leaving both open and commenting the evidence on each; the roll is the maintainer's call.

Addresses a deploy has to exercise after the roll:

Closes nothing yet. Refs #5099, Refs #5073.

🤖 Generated with Claude Code

rbuergi and others added 3 commits September 22, 2026 04:35
…thing that is dying

Two unrelated sev:H defects, one shape each. Both close a read that is
answered by something already gone.

#5099 — the NodeType recycle cascade derived its dependency network THROUGH
the hub it was tearing down. HandleDispose runs the RecycleCascade seam and
then Dispose()s, and HostedHubsCollection.CloseScopeWhenDisposed closes that
hub's Autofac scope the moment DisposalCompleted fires. Only the derivation's
synchronous prologue runs while the hub is whole: every enumeration leg is a
cross-hub query composed as Concat, so the later legs subscribe on a pool
thread after the scope is closed and MeshService.StampViewer's per-call
AccessService resolve throws ObjectDisposedException out of
AutofacServiceProvider.GetService. Measured in production: Hosting/TriageItem
recycled 0 addresses with one leg INCOMPLETE, and its live instance hubs kept
the assembly they were born with — reported as the cascade's own success at
every surface except one log line.

DependencyNetwork now enumerates through the mesh's read-issuing hub
(MeshExtensions.ReadIssuingHub): hosted by the mesh hub so the definition
hub's teardown cannot touch it, off the router, and not the node-CRUD
execution hub, so these reads are not queued behind every write in flight.
Note this is NOT the hoist that Doc/Architecture/DisposedScopeAndDyingHubs R3
prescribes — the resolves were ALREADY hoisted. IMeshService is scoped PER
HUB, so the captured instance is itself bound to the dying scope and
re-resolves from it on every call; hoisting relocates the throw instead of
removing it.

#5073 — the pre-warm/bake sweep's population is a live
`nodeType:NodeType partitions:all` query, and nothing asked whether an
enumerated row's PARTITION still had a store behind it. The sweep's only
absence test (TypeNodeExists) asks the same index the row came from, so a
stale row confirms itself: absence is not merely unproven but unaskable.
Both doors are dead ends — the activation path spends a full per-type budget
(5 min default) on a SubscribeRequest no partition hub can answer, holding
BakePhase.Running and therefore readiness for the whole sweep (31 rows held
one pod 2/3 for 48+ minutes under maxUnavailable: 0, so two builds served one
host); the batch path faults its state write into a gating image verdict that
cannot be retracted.

PartitionExistenceProbe asks the WRITABLE STORAGE PROVIDERS — the only party
that knows the store rather than the index — once per distinct pending
partition, in front of the work, because afterwards the budget is spent or the
verdict is already self-confirming. A type in a confirmed-absent partition is
reported PreWarmStatus.Removed (already non-gating, already cascading as
UpstreamContentBroken) and not warmed. The row is NAMED, never deleted: a boot
sweep must not prune mesh data on the strength of a probe.

Fail-open throughout, asserted case by case: absence is confirmed only by a
provider that says so with none contradicting it. Indeterminate, contradicted,
errored, timed-out or no-providers-at-all all leave the sweep byte-for-byte as
it was — a false "absent" would make a pod skip real NodeTypes and serve a
mesh it never baked, which is worse than the defect. PathResolutionService's
own copy of the vote now delegates to the shared probe so the two cannot drift.

Controls, both sides, measured:
- ARecycleCascadeEnumeratesFromASurvivorTest — 3/3 pass with the fix; reverting
  the one line to `var reader = hub` gives 1 pass / 2 fail, both citing the
  verbatim production string "ObjectDisposedException: Instances cannot be
  resolved and nested lifetimes cannot be created from this LifetimeScope".
  The passing case is the calibration (hub alive), and every case asserts the
  instance path is IN the network — an empty network is "complete" too.
- ADeletedPartitionIsNotPumpedTest — 17/17 pass; breaking the contradiction
  guard and the skip predicate gives 12 pass / 5 fail, the fail-open control
  and all three warmed-as-before rows plus the real-mesh inertness control.

Both projects build clean: dotnet build -c Release -warnaserror, 0 Error(s),
0 Warning(s), one project per invocation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Doc/Architecture/EnumeratingFromASurvivor — the window HandleDispose opens
between the cascade seam and Dispose(), why "still whole enough to compute it"
covers only the synchronous prologue, and why the failure is worse than an
error because the recycle succeeds at every surface a caller can see. Its
load-bearing section is the companion clause to DisposedScopeAndDyingHubs R3:
hoisting the resolve is the correction for that family and it does NOT work
here, because IMeshService is scoped PER HUB — the hoisted instance holds the
dying hub and re-resolves from it on every call, so the hoist relocates the
throw rather than removing it. Hoist when the service outlives the scope; when
the service IS the scope, resolve from a survivor.

Doc/Architecture/AStaleIndexRowConfirmsItself — a torn-down partition leaves
index rows behind, and the platform's idiomatic existence test is a listing, so
the row's disproof is answered by the source that produced it: absence is
unaskable, not merely unproven. Carries the two-door table (a budget spent vs a
verdict that cannot be retracted), why the leniency already in the bake gate
does not help (what withholds readiness is the sweep not being FINISHED, not
the verdict it reaches), why the ordering — before the work, never after — is
what makes this a fix, the five fail-open shapes, and an explicit "what this
does NOT fix": the stale rows are still there, nothing prunes a catalog row on
partition drop, and other consumers of the same population must ask the
question themselves.

Both registered in the Architecture topic map under Reliability & wedges, next
to DisposedScopeAndDyingHubs.

Verified: DocumentationLinkIntegrityTest 1/1 passed, 0 failed, on a
MeshWeaver.Documentation.dll rebuilt at 02:37Z (the pages are embedded
resources, so the fresh timestamp is what proves the test read them). All eight
sibling pages referenced were confirmed present as files before the run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 22, 2026 02:42
@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

396 tests   - 1 194   396 ✅  - 1 194   56s ⏱️ - 2m 9s
  1 suites  -     1     0 💤 ±    0 
  1 files    -     1     0 ❌ ±    0 

Results for commit f39ee45. ± Comparison against base commit b9d7fa7.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

  1 files  ±0    1 suites  ±0   2m 15s ⏱️ -26s
306 tests ±0  306 ✅ ±0  0 💤 ±0  0 ❌ ±0 
308 runs  ±0  308 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit f39ee45. ± Comparison against base commit b9d7fa7.

♻️ This comment has been updated with latest results.

Found by re-reading my own diff, not by a failing test — so it is worth stating
plainly rather than folding into the commit above.

Putting the partition probe in FRONT of the sweep means the sweep is composed
BEHIND it with SelectMany, and that is a liveness coupling with a sharp edge:
Observable.CombineLatest never emits if ANY source completes without emitting,
and Timeout does not fire on an empty completion — it bounds silence, not
absence. So one provider that completes empty (contract-breaking, but
IPartitionStorageProvider is an extension point and the contract is prose) made
the whole probe complete silent, and the sweep behind it would then emit NO
outcomes and complete normally. DynamicTypePreWarmerHostedService marks the
gate Complete on that, readiness is granted, and the pod certifies a bake it
never performed — which is exactly the laundering WarmDynamicTypes faults an
enumeration error to avoid ("finding nothing is not passing"), reached through
the door this change had just added.

Every stage now settles an unanswerable question from the known-terminal state
rather than parking it — the same rule HostedHubsCollection.CloseScopeWhenDisposed
already applies to a hub's terminal signal, and the same operator:

  - per-provider probe  → DefaultIfEmpty(null)   = indeterminate, fails open
  - ConfirmedAbsent     → DefaultIfEmpty(false)
  - ConfirmedAbsentAmong→ DefaultIfEmpty(empty set)
  - AbsentPartitionsAmongPending → DefaultIfEmpty(empty set)

The last is the one that matters most: it is the outermost seam, so it is what
guarantees the sweep runs whatever any provider does.

The two failure modes are distinct and are pinned by different assertions, so
one case covers both by requiring an answer AND requiring it to be the
answering provider's — OneContractBreakingProvider_CannotVoidTheAnswer:

  - missing per-provider backstop → a WRONG VALUE: one empty probe suppresses a
    provider that did answer (control: 1 failed / 20, on the set comparison)
  - missing composite backstops   → NON-EMISSION: "it completed without one.
    The observable emitted nothing at all." (control: 3 failed / 20, all three
    on non-emission — the liveness hazard itself)

With all four in place: 20 / 20, 0 failed. Every control ran against a
MeshWeaver.Hosting.dll verified NEWER than the edited source by epoch mtime
(1790045285 vs 1790045268 for the final build), because minute-resolution
timestamps cannot distinguish a rebuild from a stale assembly.

dotnet build -c Release -warnaserror: MeshWeaver.Hosting and
MeshWeaver.Hosting.Test, 0 Error(s) / 0 Warning(s) each.
DocumentationLinkIntegrityTest 1/1 passed on a doc assembly 13 s newer than the
page edit.

The doc page carries the hazard as its own section, including the table of which
missing backstop produces which symptom — a test asserting only the value misses
the liveness half, and one asserting only emission misses the correctness half.

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

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

    2 files   -   1      2 suites   - 1   2m 41s ⏱️ - 3m 15s
1 576 tests  - 359  1 576 ✅  - 359  0 💤 ±0  0 ❌ ±0 
1 576 runs   - 360  1 576 ✅  - 360  0 💤 ±0  0 ❌ ±0 

Results for commit f39ee45. ± Comparison against base commit b9d7fa7.

♻️ This comment has been updated with latest results.

@rbuergi

rbuergi commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Third commit — a hazard I introduced, found by re-reading my own diff

Recording this rather than folding it into the description, because it is the kind of thing that is invisible once shipped and I would want the next reader to see that it was caught deliberately.

Putting the partition probe in front of the sweep means the sweep is now composed behind it with SelectMany. That is a liveness coupling, and two Rx properties combine badly:

  • Observable.CombineLatest never emits if any source completes without emitting;
  • Timeout does not fire on an empty completion — it bounds silence, not absence.

So one provider that completes empty — contract-breaking, but IPartitionStorageProvider is an extension point and the contract is prose — made the whole probe complete silent. The sweep behind it would then emit no outcomes and complete normally, DynamicTypePreWarmerHostedService marks the gate Complete, readiness is granted, and the pod certifies a bake it never performed. That is exactly the laundering WarmDynamicTypes faults an enumeration error to avoid — "finding nothing is not passing" — arriving through the door this PR had just added. A worse failure than #5073, and silent where #5073 is loud.

Fixed by settling an unanswerable question from the known-terminal state rather than parking it — the same rule and the same operator HostedHubsCollection.CloseScopeWhenDisposed already uses for a hub's terminal signal:

stage backstop
per-provider probe DefaultIfEmpty(null) → indeterminate, fails open
ConfirmedAbsent DefaultIfEmpty(false)
ConfirmedAbsentAmong DefaultIfEmpty(empty set)
AbsentPartitionsAmongPending DefaultIfEmpty(empty set) — the outermost seam, so it is what actually guarantees the sweep runs

The two failure modes are different, and needed separating

backstop removed symptom control result
per-provider only a wrong value — one empty probe suppresses a provider that did answer 1 failed / 20, on the set comparison
all three composites non-emission — "it completed without one. The observable emitted nothing at all." 3 failed / 20, all three on non-emission
none (fixed) — 20 / 20, 0 failed

A test asserting only the value misses the liveness half; one asserting only emission misses the correctness half. OneContractBreakingProvider_CannotVoidTheAnswer requires an answer and requires it to be the answering provider's, so it fails on either.

On the freshness of those controls

Every one ran against a MeshWeaver.Hosting.dll verified newer than the edited source by epoch mtime (final build: dll 1790045285 vs source 1790045268). Minute-resolution timestamps could not distinguish a rebuild from a stale assembly here — both read 04:45 — and a control that silently ran the old assembly would have produced a false pass and a false red in the same session.

The doc page carries this as its own section, including the symptom table, so the next person putting a question in front of a pipeline sees the edge before they hit it.

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.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate findings remain in partition probing and prewarmer handling.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

This PR fixes recycle cascades querying disposed hubs and bake sweeps warming types from deleted partitions.

Changes:

  • Routes dependency reads through a surviving mesh read hub.
  • Adds provider-backed partition existence probing and absent-partition prewarm handling.
  • Adds regression tests and architecture documentation.

Final review findings:

  • Critical (2 votes): PartitionExistenceProbe does not safely handle synchronous provider failures or empty completions.
  • Moderate (2 votes): Removed outcomes are reported as faults instead of content-broken results.
  • Moderate (1 vote): Excluding read-only providers can falsely confirm partition absence.
  • Nit (1 vote): Rename the ambiguous deleted-partition test.
File Summary
test/​MeshWeaver.Hosting.Test/​ADeletedPartitionIsNotPumpedTest.cs Partition probing, fail-open behavior, and readiness regression tests
test/​MeshWeaver.Graph.Test/​ARecycleCascadeEnumeratesFromASurvivorTest.cs Survivor-hub recycle regression tests
src/​MeshWeaver.Hosting/​PathResolutionService.cs Reuses the shared partition existence probe
src/​MeshWeaver.Hosting/​PartitionExistenceProbe.cs Provider-backed partition existence voting logic
src/​MeshWeaver.Hosting/​DynamicTypePreWarmer.cs Skips types in confirmed-absent partitions
src/​MeshWeaver.Graph/​Configuration/​NodeTypeRecycleCascade.cs Enumerates through a surviving hub
src/​MeshWeaver.Documentation/​Data/​Architecture/​EnumeratingFromASurvivor.md Survivor enumeration guidance
src/​MeshWeaver.Documentation/​Data/​Architecture/​AStaleIndexRowConfirmsItself.md Stale-index guidance
src/​MeshWeaver.Documentation/​Data/​Architecture.md Architecture documentation index updates

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +77 to +93
var probes = writableProviders
.Select(p => p.PartitionExists(partition)
.Take(1)
.Timeout(budget)
.Catch<bool?, Exception>(ex =>
{
logger?.LogDebug(ex,
"PartitionExistenceProbe: existence probe for '{Partition}' via {Provider} "
+ "failed; treating as indeterminate",
partition, p.Name);
return Observable.Return<bool?>(null);
}))
.ToList();

return Observable.CombineLatest(probes)
.Take(1)
.Select(results => results.Any(r => r == false) && !results.Any(r => r == true));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Both halves of this are right, and the fold half is the more important one — thank you.

The synchronous throw: confirmed, and it was not covered. PartitionExists is an interface method on an extension point, so it may throw instead of returning a faulted sequence, and it was being invoked while the probes list was built — so the throw escaped the per-provider Catch, the method itself, and the caller's Catch on the returned observable. I traced where it would land: AbsentPartitionsAmongPending is called in WarmPending's return, inside WarmDynamicTypes' SelectMany, so it becomes an OnError of the sweep — and WarmDynamicTypes faults deliberately on enumeration failure, which refuses the pod's readiness. An optimisation taking a pod out of rotation is worse than the defect this PR fixes. Now Observable.Defer(() => p.PartitionExists(partition)), so it arrives as an OnError the Catch normalises to indeterminate.

The fold: confirmed, and this was a real false-absent bug. You are right that one false beside a null must not confirm absence, and PartitionWriteGuardValidator had already established exactly that with the reasoning spelled out: a single false means "not in MY store", NOT "doesn't exist anywhere" — Postgres answers false for a filesystem-backed partition while the FileSystem provider that actually holds it returns null. On the bake path that pair would have skipped real NodeTypes, which is the direction my own doc page calls worse than the defect. ConfirmedAbsent is now the strict fold (Count > 0 && All(r => r == false)), citing the write guard as where the rule was set.

One thing I did NOT do, deliberately. I did not give PathResolutionService the strict fold. Its question is different — whether to synthesize a placeholder partition ROOT — and its safe direction is the opposite: a wrong "absent" there merely synthesizes a harmless placeholder, while the alternative is a 404 on a real partition. So the MEASUREMENT is now shared (PartitionExistenceProbe.Probe, one fan-out, all three not-an-answer cases normalised in one place) and the FOLD lives at each call site, each documenting which it is and why. I put a 🚨 on the loose one saying it must not be "tidied up" into the strict one, and on the strict one saying new callers want it.

On the empty-completion half: already fixed in 31b2db307f, the commit this review ran against — DefaultIfEmpty at the probe and at each composite. Flagging that only so the record is accurate; it was a live hazard and I had missed it in the first commit. It is now covered by OneContractBreakingProvider_CannotVoidTheAnswer, whose control (all backstops removed) fails 3/20 with "it completed without one. The observable emitted nothing at all."

Tests added for the two findings here:

  • OneProvidersFalse_BesideAnIndeterminate_IsNotAbsence — the exact [false, null] pair, plus the other side ([false, false] → confirmed), so it is a fold and not a blanket refusal.
  • AProviderThatThrowsSynchronously_IsIndeterminate_NotAnEscapingException — asserts the call does not throw (the half a Catch on the result cannot fix) and that the probe still answers.

Comment on lines +1150 to +1154
if (SkipForAbsentPartition(p, absentPartitions) is { } gone)
{
contentBroken.Add(p);
logger?.LogWarning(
"DynamicTypePreWarmer: {TypePath} → {Status} — its partition "

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed, and it is worse than you state — which is why I have fixed the mechanism rather than the member.

PreWarmStatus has 12 members. That switch named 9. So Retired was mis-filed too, and has been since it was introduced: both Retired and Removed are content verdicts the GATE already files as non-gating, and default was sending both to the faulted counter. So this was a pre-existing mis-count for 2 of 12 statuses, not only for the status this PR adds — my change just makes one of them reachable often enough to notice.

That also means the neighbouring test suite cannot have caught it: ARetiredNodeTypeIsNotARegressionTest asserts Retired/Removed against NodeTypeBakeGateState.ReadinessGranted and never against the consumer's counters, so the gate was right and the operator summary was wrong, with nothing comparing them. My own ASkippedType_DoesNotHoldReadiness had exactly the same blind spot — I asserted the gate and not the summary — so this finding is a fact about my tests as much as about the code.

What I changed, rather than adding two case labels. An inline switch whose default arm means "a fault" mis-files any member nobody remembered to name, silently, forever. So the mapping is now a pure DynamicTypePreWarmerHostedService.CounterFor(PreWarmStatus) returning a PreWarmCounters const string (const strings rather than a new enum, per open-vocabulary-string-constants), and the subscribe switches on its result. Retired and Removed join the content-broken arm with the reasoning in a comment.

Tests:

  • EveryOutcomeButFaulted_CountsAsSomethingOtherThanAFault — drives Enum.GetValues<PreWarmStatus>() and asserts every member except Faulted maps somewhere other than the fault counter, with Faulted asserted to map to it. So a member added later without a case fails here instead of inflating a fault count in production, which is the guard the comment now truthfully claims.
  • ASkippedType_IsCountedAsContentBroken_NotAsAFault — the counter half of the gate assertion, so the two readings of one fact are pinned together. That pairing is the actual property: an operator reads the summary and a health payload reads the gate, and them disagreeing is how "the warmer faulted N times" gets investigated instead of the partition that was torn down.

24/24 in ADeletedPartitionIsNotPumpedTest with these; MeshWeaver.Hosting and MeshWeaver.Hosting.Test build 0 Error(s) / 0 Warning(s) at -c Release -warnaserror.

rbuergi and others added 2 commits September 22, 2026 05:00
…r that agreed with the gate

Two review findings on #5163, both real, both fixed at the mechanism rather than
the instance.

1. THE FOLD WAS WRONG, in the false-absent direction.

`Any(r => r == false) && !Any(r => r == true)` accepted one provider's `false`
beside an indeterminate `null` as confirmed absence. PartitionWriteGuardValidator
had already established why that is wrong, in these words: "a single false means
'not in MY store', NOT 'doesn't exist anywhere'. A non-owning provider must never
veto a partition another provider owns — e.g. the Postgres provider returns false
for the filesystem-backed ACME partition, while the FileSystem provider (which
actually has it) can't give a definitive answer and returns null." On the bake path
that pair would have SKIPPED REAL NodeTypes — the direction this change's own doc
page calls worse than the defect it fixes.

ConfirmedAbsent is now `Count > 0 && All(r => r == false)`.

🚨 PathResolutionService deliberately KEEPS the looser fold. Its question is
different — whether to synthesize a placeholder partition ROOT — and its safe
direction is the opposite: a wrong "absent" there synthesizes a harmless
placeholder, where the alternative is a 404 on a real partition. So the
MEASUREMENT is shared (PartitionExistenceProbe.Probe — one fan-out, every
not-an-answer case normalised in one place) and the FOLD sits at each call site,
each documenting which it is; the loose one carries a marker saying it must not be
"tidied up" into the strict one.

2. A PROVIDER THAT THREW SYNCHRONOUSLY ESCAPED EVERYTHING.

PartitionExists is an interface method on an extension point, so it may throw
rather than return a faulted sequence — and it was invoked while the probe list
was BUILT, so the throw escaped the per-provider Catch, the probe method, and the
caller's Catch on the returned observable. Traced: AbsentPartitionsAmongPending is
called in WarmPending's return, inside WarmDynamicTypes' SelectMany, so it lands
as an OnError of the sweep — and WarmDynamicTypes faults deliberately on
enumeration failure, which REFUSES THE POD'S READINESS. An optimisation taking a
pod out of rotation. Now Observable.Defer, so it arrives as an OnError the Catch
normalises to indeterminate.

3. THE SUMMARY CONTRADICTED THE GATE, for 2 of 12 statuses.

DynamicTypePreWarmerHostedService's outcome switch named 9 of PreWarmStatus's 12
members and sent the rest to the FAULTED counter. So Removed — which this change
emits — AND Retired, pre-existing, were both counted as crashes while the gate
correctly filed them as non-gating content verdicts. An operator reads the summary
and a health payload reads the gate, so the two disagreeing is how "the warmer
faulted N times" gets investigated instead of the partition that was torn down.

Fixed as a mechanism, not two case labels: the mapping is a pure
`CounterFor(PreWarmStatus)` returning a `PreWarmCounters` const string (const
strings, not a new enum — policy open-vocabulary-string-constants), so
exhaustiveness is ASSERTED over the whole enum instead of left to a comment.

🚨 And this finding is a fact about the TESTS. ARetiredNodeTypeIsNotARegressionTest
asserts Retired/Removed against the GATE and never against the counters, so the
mis-count was invisible to the suite that owns those statuses — and my own
ASkippedType_DoesNotHoldReadiness had the identical blind spot. The two readings
of one fact are now pinned together.

Tests added (4): OneProvidersFalse_BesideAnIndeterminate_IsNotAbsence (with the
[false,false] control on the other side, so it is a fold and not a blanket
refusal), AProviderThatThrowsSynchronously_IsIndeterminate_NotAnEscapingException
(asserts the CALL does not throw — the half a Catch on the result cannot fix),
EveryOutcomeButFaulted_CountsAsSomethingOtherThanAFault (over Enum.GetValues, so a
future member fails here), ASkippedType_IsCountedAsContentBroken_NotAsAFault.

One existing case changed its expected VALUE under the strict fold, and its point
moved with it: OneContractBreakingProvider_CannotVoidTheAnswer now asserts
emission (the liveness hazard) plus an empty set, with an all-false control beside
it proving the empty set is the fold's answer and not a probe that measures
nothing.

Verified: ADeletedPartitionIsNotPumpedTest 24/24, 0 failed. The neighbouring
suites this replumbs — PathResolution, PreWarm, UndeterminedWitness, MeshAdmission,
ARetiredNodeType, AFirstRollout, BakeCensus, DeclaredSourcesMissing — 80/80, 0
failed. MeshWeaver.Hosting and MeshWeaver.Hosting.Test at -c Release -warnaserror:
0 Error(s) / 0 Warning(s) each, on a dll verified newer than the edited source by
epoch mtime (1790045805 vs 1790045681).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the dying hub

DisposedScopeAndDyingHubs is on main and its R3 prescribes ONE correction —
'hoist, never guard' — over a 204-site graded sweep. That prescription is
insufficient for a service registered SCOPED PER HUB, and the page did not say
so, so this corrects the page rather than leaving the refinement in a sibling
page and a triage queue.

Hoisting changes WHEN the container is asked, never WHICH container answers. For
a mesh-lifetime singleton those are the same question, which is why R3's
correction works. For a per-hub scoped service they are not: the hoisted instance
IS the short-lived thing — constructed with that hub, reaching back through it on
every later call — so the throw moves from the continuation into the service's own
method body, where no call-site hoist can reach it.

Measured on #5099: DependencyNetwork already resolved eagerly in its synchronous
prologue — R3's correction, applied before R3 was written — and still threw,
because IMeshService is AddScoped and MeshService.StampViewer -> CaptureContext()
resolves AccessService from its hub on every Query.

So the test stated for the reader is neither 'is this resolve in a continuation'
nor 'is it hoisted' but 'what is this service's lifetime keyed to, and does this
pipeline outlive it' — and where the answer is 'the thing being torn down' the
remedy is to move the WORK to a survivor (for a read, ReadIssuingHub), which is
the established recycle rule applied to reads instead of posts.

🚨 And the page now says why its own inventory cannot triage this class: the sweep
grades by CALL-SITE SHAPE, which is lexical, while service lifetime is a property
of a registration in another file. Measured on src/: 12 types are registered
AddScoped/TryAddScoped, about 142 resolve sites between them, 118 of those
IMeshService. How many fall inside the 204 graded sites is explicitly recorded as
NOT ESTABLISHED — the cross-reference is the open question — because the wrong
thing to write here is a number I did not measure. Until it is done, a site 'fixed'
by the hoist alone can pass review, pass a hoisting-shaped guard, and still throw
in production.

Cross-linked both ways with EnumeratingFromASurvivor.

Verified: DocumentationLinkIntegrityTest 1/1 passed, 0 failed, on a
MeshWeaver.Documentation.dll stamped 31 s after the page edit (epoch 1790046189 vs
1790046158) — the pages are embedded resources, so the fresh timestamp is what
proves the test read the edit rather than a cached copy.

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

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

    1 files   -     2      1 suites   - 2   5m 44s ⏱️ +11s
1 915 tests +1 195  1 915 ✅ +1 387  0 💤  - 192  0 ❌ ±0 
1 916 runs  +1 196  1 916 ✅ +1 388  0 💤  - 192  0 ❌ ±0 

Results for commit f39ee45. ± Comparison against base commit b9d7fa7.

@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

900 tests  +457   900 ✅ +457   7m 51s ⏱️ + 7m 3s
  1 suites  -   2     0 💤 ±  0 
  1 files    -   2     0 ❌ ±  0 

Results for commit f39ee45. ± Comparison against base commit b9d7fa7.

♻️ This comment has been updated with latest results.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    2 files   -     3      2 suites   - 3   7m 24s ⏱️ - 7m 25s
2 444 tests  - 1 418  2 444 ✅  - 1 416  0 💤  - 2  0 ❌ ±0 
2 448 runs   - 1 418  2 448 ✅  - 1 416  0 💤  - 2  0 ❌ ±0 

Results for commit f39ee45. ± Comparison against base commit b9d7fa7.

@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    8 files   -     9      8 suites   - 9   26m 53s ⏱️ - 6m 1s
7 537 tests  - 1 319  7 537 ✅  - 1 125  0 💤  - 194  0 ❌ ±0 
7 544 runs   - 1 319  7 544 ✅  - 1 125  0 💤  - 194  0 ❌ ±0 

Results for commit f39ee45. ± Comparison against base commit b9d7fa7.

♻️ This comment has been updated with latest results.

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