Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 81 additions & 4 deletions src/MeshWeaver.Data/DataExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1150,13 +1150,70 @@ private static void RegisterOwnerDisposingNack(
}

/// <summary>
/// Maps owner-side patch exceptions to structured <see cref="MeshNodeError"/>
/// codes. Unknown exception types fall through as
/// <see cref="MeshNodeErrorCode.Unknown"/> with the exception type prefixed
/// — visible at the consumer GUI so the gap is diagnosable, not silent.
/// 🚨 TEARDOWN IS NOT A VERDICT ABOUT THE WRITE — issue #3499, and the reason a package
/// install could not survive its own root recycling.
///
/// <para>An owner-side patch fault raised because something the answer needed had been
/// DISPOSED says exactly one thing: this activation went away. It says nothing about whether
/// the caller's patch is acceptable, and <see cref="HubDisposingException.IsDisposedContainer"/>
/// already writes the rule down — <i>"a hub whose scope has been closed cannot serve anything,
/// so a delivery that faults this way must be answered as RETRYABLE (the address reactivates)
/// rather than as a result."</i> This funnel — the ONE place every owner-side patch fault is
/// classified — did not consult it, so every such fault fell through to
/// <see cref="MeshNodeErrorCode.Unknown"/>, which is TERMINAL by the enum's own contract
/// (<see cref="MeshNodeErrorCode.OwnerDisposing"/> and <see cref="MeshNodeErrorCode.OwnerNotReady"/>
/// "are the only auto-retried NACK codes; every other code stays terminal").</para>
///
/// <para><b>Measured.</b> CD 7937 / 7941 and the MeshWeaver.Plugins gate on 2026-09-06: a
/// package root recycles mid-install, its subtree hubs die owing acks for patches whose merge
/// had ALREADY committed (<c>PATCH_MERGE_STAMPED v=2 refused=0 → PATCH_ECHO_SEEN</c>, ten of
/// ten trails), and the writer was left either with 31 s of silence (fixed by #2543) or — once
/// that silence became a NACK — with a TERMINAL <c>Unknown</c> for a write it could simply
/// have re-applied. Both read at the gate as <c>GATE FAILED — install: Chess</c>. Classifying
/// the teardown correctly hands the writer the code its OWN machinery already acts on:
/// <c>MeshNodeStreamExtensions</c> re-enqueues an <c>OwnerDisposing</c> NACK against the fresh
/// activation, capped at <c>MaxOwnerDisposingReenqueues</c>, re-diffing each time — so a merge
/// that DID commit is a no-op and one that died with its hub lands. No bound moves.</para>
///
/// <para>🚨 <b>Why every <see cref="ObjectDisposedException"/> in the graph counts</b>, not just
/// the Autofac-scope shape <see cref="HubDisposingException.IsDisposedContainer"/> matches.
/// Nothing on this path disposes anything as a way of REFUSING a patch: a disposed scope, a
/// disposed subject, a disposed CTS and the two typed teardown exceptions
/// (<see cref="HubDisposingException"/>, <c>HubDisposedBeforeResponseException</c> — both derive
/// from it) are all the same fact. And the direction of a mistake is asymmetric, exactly as
/// <c>MeshOperations.IsWriteDenial</c> argues for its own default: a false "terminal" loses a
/// write and fails an install, every time; a false "retryable" costs an idempotent re-diff,
/// capped at two.</para>
///
/// <para>🚨 <b>That asymmetry is real but the cheap side is NOT fully bounded, and this is the
/// honest statement of the cost.</b> The re-enqueue this classification feeds is
/// <c>MeshNodeStreamExtensions</c>'s <c>OwnerDisposing / OwnerNotReady / Conflict</c> arm, and
/// <b>#3477</b> is a measured, open, unreproduced case where that arm went DARK: after
/// <c>LATE_NACK_REENQUEUE … code=OwnerDisposing attempt=1</c>, nothing landed and nothing logged
/// for 45 s — no <c>LATE_ACK</c>, no second NACK, no error to the caller (1/330,
/// <c>LateNackReenqueueTest</c>, MeshWeaver.Plugins shard 3). Routing more faults here makes
/// that class MORE likely, not less, and it is silent when it happens. Traced end to end for
/// #3499: these faults take the SAME arm — and the closed-lifetime-scope shape takes its LATE
/// half preferentially, because a hosted hub's scope is closed on <c>DisposalCompleted</c>, by
/// which point <c>PostImplGeneric</c> refuses the owner's own post and <c>RoutePatchVerdict</c>
/// falls through to <c>ILatePatchVerdictSink</c> — which is exactly where #3477 was measured.
/// So the trade is <i>rarely and recoverably</i> against <i>always and fatally</i>, which is
/// still the right way round; it is not "at most two re-diffs".</para>
///
/// <para>Everything else keeps its previous mapping; unknown exception types still fall through
/// as <see cref="MeshNodeErrorCode.Unknown"/> with the exception type prefixed — visible at the
/// consumer GUI so the gap is diagnosable, not silent.</para>
/// </summary>
private static MeshNodeError ClassifyPatchException(Exception ex, string path)
{
if (IsOwnerTeardown(ex))
return new MeshNodeError(
MeshNodeErrorCode.OwnerDisposing, path,
$"the owner went away while this patch was being answered ({ex.GetType().Name}: {ex.Message}) — "
+ "the write is NOT confirmed; safe to retry against the fresh activation: a re-enqueue "
+ "re-diffs against the freshest state, so a merge that did commit is a no-op",
ex.StackTrace);

var (code, prefix) = ex switch
{
UnauthorizedAccessException => (MeshNodeErrorCode.AccessDenied, "Access denied"),
Expand All @@ -1169,6 +1226,26 @@ InvalidOperationException ioe when ioe.Message.Contains("Patch must be a JSON ob
return new MeshNodeError(code, path, $"{prefix}: {ex.Message}", ex.StackTrace);
}

/// <summary>
/// True when <paramref name="ex"/> is — or carries, at any depth and through any
/// <see cref="AggregateException"/> branch — the fact that the OWNER went away while this
/// patch was being answered. See the remarks on <see cref="ClassifyPatchException"/> for why
/// this is a retryable verdict and not a result.
///
/// <para>The graph, never <c>InnerException</c> in a loop: a reactive ack path composes, so the
/// teardown can arrive at any index of an aggregate and a chain walker would classify by
/// arrival order (<see cref="ExceptionChain"/>'s own remarks).</para>
///
/// <para><see cref="ErrorType.ShuttingDown"/> is the messaging layer's word for the same fact —
/// <c>MessageService.NackThroughParent</c> stamps it, <c>HubDisposingException</c>'s remarks
/// name it as the same contract, and <c>PackageInstaller</c> already retries on it — so a
/// delivery failure carrying it belongs here rather than in the <c>Unknown</c> bucket.</para>
/// </summary>
internal static bool IsOwnerTeardown(Exception? ex)
=> ExceptionChain.Contains<ObjectDisposedException>(ex)
|| ExceptionChain.Contains(ex, static e =>
e is DeliveryFailureException { Failure.ErrorType: ErrorType.ShuttingDown });

/// <summary>
/// 🚨 Totality for a one-shot watcher: runs <paramref name="onEmptyCompletion"/> when
/// <paramref name="source"/> COMPLETES WITHOUT EVER EMITTING — Rx's third termination, the one a
Expand Down
143 changes: 143 additions & 0 deletions src/MeshWeaver.Documentation/Data/Architecture/WriteVerdictTotality.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,149 @@ Missing both routes is then a checked fact (nobody in this mesh is armed, and th
and is logged rather than swallowed, because the remaining case — a caller in another process — is
one the registry cannot see.

## Totality is not enough — the verdict's CODE decides whether the write survives

A terminal that reaches the writer can still be the wrong terminal, and the difference is not
cosmetic: `MeshNodeErrorCode` splits every NACK into two populations, and its own doc says so.

> `OwnerDisposing` and `OwnerNotReady` "are the only auto-retried NACK codes; every other code stays
> terminal."

So a NACK carrying `Unknown` is not a weaker version of `OwnerDisposing` — it is the opposite
instruction. `MeshNodeStreamExtensions` re-enqueues the two retryable codes against the fresh
activation (capped at `MaxOwnerDisposingReenqueues`, re-diffing on each attempt, so a merge that did
commit becomes a no-op); everything else it hands straight to the caller as a failed write.

### The case: a package root recycles mid-install (#3499)

Measured on 2026-09-06 across three independent runs — core `main-cd` **#7937** (`85ab1dfbe`), core
`main-cd` **#7941** (`b8accc2b2`) and the MeshWeaver.Plugins gate on **#1422** — on two different
packages (`Chess`, `Hosting`), with a run in between that sealed cleanly. All three ended the same
way: `GATE FAILED — install: <package>` with `[install] TimeoutException`, and `main-cd` could not
produce a sealed set for the release.

The subtree hubs of a package root are hosted hubs of that root. When the root recycles — the
NodeType rebind watcher, stale-build convergence, or the installer's own retyped-root recycle, all
of which are legitimate — every hub under it goes down with it. In ten of ten `VERDICT_TIMEOUT`
trails on the gate run, the patches those hubs were answering had **already committed**:

```
HANDLER_ENTER → PATCH_MERGE_DISPATCHED → HANDLER_EXIT Processed
→ PATCH_MERGE_TURN entered → PATCH_MERGE_STAMPED v=2 refused=0 → PATCH_ECHO_SEEN ⇒ (nothing)
```

The merge landed; only the durability leg was still owed, and it faulted on a lifetime scope that
was closing. That fault reached `ClassifyPatchException` — the ONE funnel every owner-side patch
fault passes through — and fell out of its `_ =>` arm as `Unknown`. The writer therefore reported a
**lost write for a patch that had committed**, and the installer failed the package.

### 🚨 Count the base rate before naming a cause

The obvious story — *"a package root is disposed mid-install under a reconcile"* — is what the
symptom looks like, and it is **falsified by the run that sealed**. Counting the four bake jobs, all
with per-request tracing on (`handler-side fate` present in every one, so no count below is a
missing denominator):

| bake job | verdict | `[QUIESCE-TIMEOUT]` | `reconcile failed … disposed before the response arrived` | `VERDICT_TIMEOUT` |
|---|---|---:|---:|---:|
| core `main-cd` #7937 | FAILED `install: Chess` | 6 | 10 | **5** |
| core `main-cd` **#7938** | **SEALED** | **10** | **12** | **0** |
| core `main-cd` #7941 | FAILED `install: Hosting` | 5 | 8 | **0** |
| Plugins #1422 gate | FAILED `idempotence: Chess` | 3 | 7 | **10** |

**The root being recycled under an in-flight reconcile is at its MAXIMUM in the run that sealed.** It
is normal, it is answered (the requester gets a typed `HubDisposedBeforeResponseException`), and it
is not the cause. What separates a seal from a failure is the third column: a write that reached no
verdict at all.

Which also says the two failures are not one bug. #7937 and the gate run are this page's defect.
**#7941 is not**: it has zero unanswered write verdicts and instead carries two
`JsonSynchronizationStream … resubscribe failed` lines — zero in the sealed control — whose parked
stream leaves the *outer* `CreateOrUpdateNodeRequest` with no terminal at all, one layer above the
verdict watcher. That is its own defect, tracked separately, and no amount of correct classification
here would have sealed that run.

Fixing only the silence makes this *worse-looking, not better*: before the fault was caught at all
(#2543) the writer burned the full 31 s `WriteVerdictBound` and reported `OwnerUnreachable`; after,
it got a prompt `Unknown`. Same failed install, 31 seconds sooner.

### The rule

> **A disposal fault is the OWNER going away, not a verdict about the write.**

The predicate was already written down, one layer over, in
`HubDisposingException.IsDisposedContainer`:

> *"a hub whose scope has been closed cannot serve anything, so a delivery that faults this way must
> be answered as RETRYABLE (the address reactivates) rather than as a result."*

`ClassifyPatchException` now consults it first, ahead of every type-shaped arm, and answers
`OwnerDisposing` with a message that says what the writer may do. The classification walks the
exception **graph** (`ExceptionChain`), not `InnerException` in a loop, so a teardown sitting at any
index of an aggregate still decides it — a chain walker would classify by arrival order, which is a
race.

Three shapes count, and they are the same fact in three vocabularies:

| shape | where it comes from |
|---|---|
| any `ObjectDisposedException` in the graph | a closed DI scope, a disposed subject, a disposed CTS |
| `HubDisposingException` / `HubDisposedBeforeResponseException` | the two typed teardown exceptions — both **derive** from `ObjectDisposedException`, so they are covered by the line above |
| `DeliveryFailureException` with `ErrorType.ShuttingDown` | the messaging layer's word for it (`MessageService.NackThroughParent`) |

**Why every `ObjectDisposedException`, not just the Autofac-scope shape.** Nothing on this path
disposes anything as a way of *refusing* a patch, so on this path the exception has exactly one
meaning. And the direction of a mistake is asymmetric — the same argument `MeshOperations.IsWriteDenial`
makes for its own default: a false "terminal" loses a write and fails an install, every time; a false
"retryable" costs an idempotent re-diff, capped at two.

### 🚨 The cheap side of that asymmetry is not fully bounded — state it honestly

The re-enqueue this classification feeds is `MeshNodeStreamExtensions`'s
`OwnerDisposing / OwnerNotReady / Conflict` arm, and **#3477** is a measured, open, unreproduced case
where that arm went **dark**: after

```
[UpdateRemote] LATE_NACK_REENQUEUE hub=cache/… target=TestData/late-nack-node attempt=1 code=OwnerDisposing
```

nothing landed and nothing logged for 45 s — no `LATE_ACK`, no second NACK, no error to the caller
(1/330, `LateNackReenqueueTest`, MeshWeaver.Plugins shard 3). Sending more faults down that arm makes
that class **more** likely, and it is silent when it happens.

The two variants are the same mechanism reached two ways: the **direct** arm when the caller's
`Observe` callback is still pending (inside `UpdateResponseWaitBound`, 2 s), and the **late** arm when
the response arrives after that *or* when the owner's own `Post` is refused and `RoutePatchVerdict`
falls through to `ILatePatchVerdictSink`. A closed-lifetime-scope fault takes the **late** half
preferentially — `HostedHubsCollection` closes a hosted hub's scope on `DisposalCompleted`, by which
point `PostImplGeneric` refuses that hub's post — and the late half is exactly where #3477 was
measured.

So the trade is *rarely and recoverably* against *always and fatally*, which is still the right way
round, and it is not "at most two re-diffs". **A first candidate seal after this change should be
read for #3477's signature** — a `LATE_NACK_REENQUEUE` with no terminal after it — because this
change makes it more reachable and nothing yet makes it loud. #3477's own ask is the instrumentation
that would: a `LATE_NACK_REENQUEUED requestId=<new>` stage linking the re-enqueued request's trail to
the original.

### What this does not do

It does not stop the recycle, and it should not: recycling a root whose NodeType changed is the fix
for [a hub bound to the wrong configuration](../NodeTypeCompilation), and a package install that
cannot survive one is the defect. It also widens no bound — not the 2 s quiesce budget, not the 31 s
`WriteVerdictBound`, not the install timeout. The retry it enables already existed and is already
capped; all that changed is that the writer is now told which of the two things happened.

### Sibling defect, still open

A hub that has logged `[QUIESCE-OK]` still **accepts new correlated requests** — the intake gate only
refuses from `DisposeHostedHubs` onward (`MessageService.ScheduleNotify`), so a request taken on in
that window can never be answered. Measured on the same three runs: of nine `[QUIESCE-TIMEOUT]`
trails, six show `RECEIVED runLevel=Quiescing` at the issuing hub, i.e. the callback was registered
*after* teardown began. That is a different bug from this one (here the request was accepted while
the hub was `Started`), it is not fixed by this page's rule, and it is benign in every occurrence
measured so far — the requester gets `HubDisposedBeforeResponseException`, which is a real answer.

## What this is NOT

**It is not a timeout, a retry, or a watchdog.** No bound moves; nothing is re-attempted; no poller is
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
---
Name: An install survives its own package being recycled
Category: Fix
Description: While a package was installing, the platform could legitimately restart that package's own hub — and every write already in flight underneath it was then reported as failed, even though it had already been applied. The install gave up on work it could simply have redone. Those writes are now retried against the restarted package, and the install finishes.
Icon: ArrowSync
Order: -20260906
---

# An install survives its own package being recycled

Installing a package writes a lot of small things underneath it — its policy, its access entries,
the compile state of each type it brings. While that is happening the platform can decide, for
perfectly good reasons, to restart the package's own hub: its type changed, a fresh build of that
type was published, or the installer itself asked for the restart so the package comes back bound to
what it now is.

Restarting is fine. What was not fine is what happened to the writes that were in flight at that
moment. Each of them had already been *applied* — the change had landed and everything watching the
node had seen it — and only the final "and it is safely stored" step was still outstanding. When the
hub went away underneath that step, the platform told the writer the write had failed, in the one
way that means *do not try again*. So the installer stopped, and the whole package install ended in a
timeout it could have avoided entirely.

That is not what a restart means. A component going away says nothing about whether your change was
acceptable — it says the address will be back in a moment. The platform already has a word for
exactly that, and already knows what to do when it hears it: reapply the change against the restarted
component, comparing against the freshest state each time, so a change that did land costs nothing
and one that was lost is put back.

Those writes now carry that word. A package install rides out a restart of its own package instead
of failing on it, and nothing waits any longer than before: no timeout, retry budget or install
deadline changed. A write that genuinely cannot be made — one you are not allowed to do, or one the
node refuses — is still refused immediately, and still says why.

The same correction applies everywhere a node's owner answers a change, not just during installs:
saving from a page while its hub is being recycled now gets the retry it deserves instead of an
error card.
Loading
Loading