[rbuergi]

Two failure reports that sent the reader to find what the code was already holding

Closes #3712. Advances #1198.

Both were measured on live portals this week, one day apart, in subsystems that share no code.

#3712 — "find why this hub disposed"

MessageService.Dispose() files an Error (event 7301) for every delivery a hub accepted and parked behind an init gate and then threw away. The line named the message, the sender and the gates — and ended:

…Accepted work must be drained before a hub goes down; find why this hub disposed before its deferred work could run.

The hub knows. MessageHub records disposeRequestedBy / disposeReason / cascadeOwner one frame earlier, in HandleDispose — added precisely so #3510's "[QUIESCE-START] on a root should name who asked" could be satisfied. But [QUIESCE-START] is Information and the red-log pipeline files Errors, so the one line that becomes an issue was the one line without the attribution.

Measured on Admin/_LogIncident/d2249f800ffc2577: 364 occurrences, 2026-09-08 → 2026-09-14, 13 pods. Not one says which teardown discarded the work, so a reader given the incident alone cannot tell an operator recycle from a NodeType rebind from an owner's cascade — three different investigations.

MessageHub.DisposalAttribution renders WHO and WHY as one clause. Both discard sites carry it, and so does the NACK the stranded sender receives — that sender is in a different process as often as not and can see none of this hub's log.

It varies with the cause, which is what makes it an answer rather than decoration:

what happened	what the report says
routed DisposeRequest with a reason	requested by <sender>; why: <the reason>
routed DisposeRequest, no reason	requested by <sender>; why: reason not stated by the caller
no routed request (host teardown, a using, an owner cascade)	requested by a direct Dispose() (no routed DisposeRequest)

The third rules the message path out — exactly what the production incident needed and could not get.

One poster fixed with it: MeshOperations.Recycle (the operations / MCP recycle verb) was the single production poster of a DisposeRequest with no Reason — and it is the one teardown that originates OUTSIDE the framework's own lifecycle, so it is the one whose cause a log cannot reconstruct. It now states one.

#1198 — an outstanding-work field that said "none"

The shared line is [DeleteNode] timeout path=… stage=<stage> partial-deleted=<n> unanswered=<set>. unanswered= exists so an operator learns WHICH node stopped the operation; the pre-flight stage fills it. The commit stage never did, so it rendered - — which is the SAME rendering the line uses for "there is nothing outstanding".

Measured on memex-cloud 2026-09-14T08:45:38Z:

[DeleteNode] timeout path=Hosting/TriageStatus stage=commit partial-deleted=3 unanswered=-
System.TimeoutException: [DeleteNode:commit] the bottom-up delete of 'Hosting/TriageStatus'
made no progress for 30s — 3 path(s) removed from storage so far

Three removed, the drain stuck on the rest of the subtree, and the field that exists to name them asserted the opposite of the truth. Beside it, a count of what SUCCEEDED, which identifies nothing about the failure.

The plan (collected.ToDelete) and the progress (SnapshotProgress()) are both live in that closure. The difference is the answer:

[DeleteNode:commit] … made no progress for 30s — 3 of 9 planned path(s) removed from storage
so far; still owed by the plan: Hosting/TriageStatus, …

"still owed by the plan", not "remaining": a drain can remove MORE than it planned (#3392), so the set is what the plan still owes and never a claim about what else storage holds. The progress count stays — names are ADDED.

Not a downgrade, not a widened bound

Every report keeps its level, its event id and every fact it already carried. No bound moved, nothing was silenced, no retry or watchdog was added.

Deterministic repro, both directions

DeferredDeliveryNackedOnDisposeTest (+2): a routed DisposeRequest that STATES a reason, plus a control arm disposed directly so the clause is proved to vary rather than being a constant. With the two production edits reverted and the test project rebuilt, exactly those two go red on their own assertion text and the two pre-existing tests stay green:

Expected "Hub attributed/1 was disposed while GatedRequest (id=…) was still deferred;
initialization gates closed at deferral: [attributed-gate-never-opens] — the message was
never processed. The address may reactivate…" to contain "host/1"

DeleteCommitTimeoutNamesWhatIsStuckTest (new): a real IStorageAdapter over a real InMemoryStorageAdapter that serves two deletes and then goes silent. Reverted, the message is the production shape with no names:

made no progress for 3s — 0 path(s) removed from storage so far

It carries its own positive controls (it really is the commit stage; the bound really is a no-progress watchdog and not #3392's total-duration cap; the count survives) and a negative control — the two paths already removed must NOT appear in the owed set, so "the plan echoed back under a new label" fails.

Verification

suite	result
MeshWeaver.Messaging.Hub.Test	446 / 446
MeshWeaver.Graph.Test	1610 / 1610
MeshWeaver.Documentation.Test	467 / 467 (link integrity + timeout ratchet)
Memex.Portal.Shared.Test (recycle / install / dispose)	11 / 11

Every touched project built -c Release -warnaserror, one project per invocation, 0 Error(s).

Notes for the reviewer

• Pairs-with: none — the diff removes no public type and no public member; it only adds. MessageHub.DisposalAttribution is internal.
• Implementers: none — no member added to a public interface or abstract class.
• Mirror-sync: none — no i18n catalog key added or reworded. The new text is log output and framework exception text, which the existing code already documents as upstream text no catalog can carry.
• Deferred-delivery discard at hub disposal reports an empty gate list — the gate that held the message is never named #3712 will keep firing until the portals roll. Both live portals run c84c6c0550 (2026-09-12 09:30Z), which predates [DISPOSE-DISCARD] 76 Error-level lines per plugin-gate shard: $model-probe hubs dispose with a GetDataRequest still deferred behind [DataContextInit,MeshNodeInit] #4178 (3c421609e6) as well as this change — measured with git merge-base --is-ancestor. Every 2026-09-14 sample on that incident is the self-addressed $model-probe shape [DISPOSE-DISCARD] 76 Error-level lines per plugin-gate shard: $model-probe hubs dispose with a GetDataRequest still deferred behind [DataContextInit,MeshNodeInit] #4178 already demoted to Debug.

Written up as Doc/Architecture/AFailureReportAnswersItsOwnInstruction.

Review round — three findings, all valid, all fixed

Copilot found three, and each was this PR's own defect pointed the other way. Answered inline on the threads; summarised here because two of them changed the code.

1. The teardown cause was overwritable. HandleDispose wrote its attribution unconditionally, so a routed DisposeRequest whose turn ran after a direct Dispose() had begun replaced a true a direct Dispose() reading — the one that rules the message path out — with its own. Reachable, not theoretical: Dispose() sets IsShuttingDown and only then posts ShutdownRequest(Quiescing), so a request arriving in that window still sees RunLevel=Started and is admitted. Now teardownCauseClaimed + TryClaimTeardownCause() (Interlocked.CompareExchange) makes ownership atomic against NoteCascadeFrom on another thread, and the handler records only when it is the request that STARTS the teardown. The genuinely simultaneous pair is written down as the honest residue rather than papered over, and no repro is claimed for it.
2. A blank Reason rendered as nothing. null was the only value treated as "not stated", so "" or " " produced a literal why: — this page's own thesis, in a new costume. Normalised at the single capture point (two readers: DisposalReason and DisposalOriginForChildren, which propagates down a cascade). New guard ATeardownWhoseReasonIsBlank_ReportsItAsUNSTATED_NotAsNothing uses whitespace, not an empty string, because a Length > 0 check would pass the latter; negative control measured.
3. The doc table named the wrong form for a cascade. DisposalRequestedBy prefers cascadeOwner, so a cascaded child renders a cascade from its owner X — my table folded that into the direct-dispose row, exactly the conflation cascadeOwner exists to remove. Five rows now, with the self-posted form too, plus why each distinction cannot be inferred from the others.

Held correctly rather than raced: the PR had already been admitted to the merge queue when the findings landed, so it was dequeued via GraphQL dequeuePullRequest first (verified by reading repository.mergeQueue.entries — position 2, AWAITING_CHECKS, gone afterwards), then drafted, then updated.

🤖 Generated with Claude Code

[github-actions]

Test Results (shard 0)

262 tests  ±0   262 ✅ ±0   2m 18s ⏱️ +25s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 39d0944. ± Comparison against base commit 8d3c307.

♻️ This comment has been updated with latest results.

[Copilot]

🟡 Changes recommended

Unresolved findings include a disposal-ownership race, blank-reason handling, attribution documentation mismatches, and incomplete timeout-log coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Improves teardown and recursive-delete diagnostics by attributing discarded work and naming paths still owed during commit timeouts.

Changes:

• Adds disposal attribution to logs and NACKs.
• Reports outstanding delete paths during commit timeouts.
• Adds regression tests and documentation.

File summaries

File	Summary
test/MeshWeaver.Messaging.Hub.Test/DeferredDeliveryNackedOnDisposeTest.cs	Tests disposal attribution and NACK content.
test/MeshWeaver.Graph.Test/DeleteDrainCompletionTest.cs	Extends storage behavior for stalled deletes.
test/MeshWeaver.Graph.Test/DeleteCommitTimeoutNamesWhatIsStuckTest.cs	Tests commit-timeout diagnostics.
src/MeshWeaver.Messaging.Hub/MessageService.cs	Adds attribution to deferred-delivery diagnostics and NACKs.
src/MeshWeaver.Messaging.Hub/MessageHub.cs	Formats disposal source and reason.
src/MeshWeaver.Mesh.Operations/MeshOperations.cs	Supplies a reason for operator-triggered recycle.
src/MeshWeaver.Mesh.Contract/MeshExtensions.cs	Reports paths still owed by stalled deletes.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-14-a-teardown-and-a-stalled-delete-now-name-what-they-tell-you-to-find.md	Documents the user-facing fixes.
src/MeshWeaver.Documentation/Data/Architecture/AFailureReportAnswersItsOwnInstruction.md	Documents the diagnostic patterns.
src/MeshWeaver.Documentation/Data/Architecture.md	Indexes the architecture documentation.

Review details

Suppressed comments (3)

src/MeshWeaver.Messaging.Hub/MessageHub.cs:204

• The new attribution table says an owner cascade is reported as requested by a direct Dispose(), but the implementation takes the cascadeOwner branch and emits requested by a cascade from its owner <owner> instead. Please align the documented/advertised third shape with the actual cascade-specific output (or change the formatter if the direct-dispose wording is the intended contract), otherwise operators and tests will rely on a shape this code never produces.

    internal string DisposalAttribution =>
        $"requested by {DisposalRequestedBy}; why: {DisposalReason}";

test/MeshWeaver.Graph.Test/DeleteCommitTimeoutNamesWhatIsStuckTest.cs:115

• This regression test only inspects reported.Message; it never verifies the [DeleteNode] timeout ... unanswered= log line that the production change is meant to repair. Removing the new Exception.Data[DeleteUnansweredDataKey] assignment would leave the exception-text assertions green while the logger still renders unanswered=-, so capture the test logger and assert the stalled path appears in the emitted timeout line while retaining the already-deleted negative control.

        var reported = await failure.Should().Within(TestTimeouts.WriteConvergence).Emit(
            "the store went silent mid-commit, so the no-progress watchdog must end the operation");

test/MeshWeaver.Messaging.Hub.Test/DeferredDeliveryNackedOnDisposeTest.cs:261

• The control actually calls gated.Dispose() directly, while owner-cascade disposal is deliberately distinguished by cascadeOwner and is asserted by QuiesceStartNamesTheAskerTest. Describing an owner's cascade as this direct-dispose control makes the test documentation claim coverage for an output it does not exercise and contradicts the new attribution contract.

    /// taken down by a DIRECT <c>Dispose()</c> — host teardown, an owner's cascade, a <c>using</c> —

• Files reviewed: 10/10 changed files
• Comments generated: 3
• Review effort level: Lite

---

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

[rbuergi]

Valid, and fixed — this is the defect this PR is about, pointed the other way: a report that names the WRONG cause is worse than one that names none.

Confirmed reachable rather than theoretical: Dispose() sets IsShuttingDown and only then posts ShutdownRequest(Quiescing), so a DisposeRequest arriving in that window still sees RunLevel=Started, is admitted by RefusesIntake, and its turn runs after the teardown has begun. HandleDispose then overwrote a true a direct Dispose() (no routed DisposeRequest) reading — which is the reading that rules the message path out — with its own. It mattered little while the attribution appeared only on [QUIESCE-START]; it matters now that it rides the Error and the NACK.

Both halves of your suggestion are now in:

• Atomic ownership. New teardownCauseClaimed + TryClaimTeardownCause() (Interlocked.CompareExchange). NoteCascadeFrom had a first-cause-wins rule over two volatile fields — a rule, not a guarantee, since the two writers run on different threads (action block vs. the disposing owner's thread) — and it now takes the same claim. Exactly one writer ever wins.
• Ignore once disposal has begun. HandleDispose records only if (startsTheTeardown && TryClaimTeardownCause()). startsTheTeardown is read once and reused for the existing AnnounceRecycle() branch, which asks the same question; two reads of a flag another thread can flip could otherwise disagree.

What I have deliberately not claimed: a genuinely simultaneous pair — a direct Dispose() and a routed request landing within the same instant — is still a race, but one where both really happened and either attribution is true. That is written into the code comment as the honest residue rather than papered over, and it is not an overwrite of an earlier cause.

I could not build a deterministic repro for the interleaving (the reachable window needs a DisposeRequest admitted between IsShuttingDown and RunLevel=Quiescing, and the post-#3647 intake bound closes every seam a test can reach), so I am not claiming a regression test for this arm — only the guard and the reasoning. The sibling arm that is deterministic — a hub torn down with no routed request must report a direct Dispose() — is pinned by ADiscardWithNoRoutedRequest_SaysSO_RatherThanSayingNothing.

[rbuergi]

Valid, and fixed — and it is exactly the defect in a new costume, which makes it worth a guard rather than just an edit.

DisposeRequest.Reason is free text from the poster, and null was the only value any renderer treated as "not stated". A blank one produced a literal why: with nothing after it — an absent answer rendering as nothing, which reads to the next person as "there was nothing to report". That sentence is the thesis of the page this PR adds.

Normalised at the capture point, as you suggested, rather than at the read:

disposeReason = string.IsNullOrWhiteSpace(request.Message.Reason)
    ? null
    : request.Message.Reason;

Capture is the single place because there are two readers — DisposalReason and DisposalOriginForChildren (which propagates the cause down a cascade) — so fixing it at the read would have left the second one to remember, and a cascaded child would still have carried the blank.

Covered alongside the null case by a new test, ATeardownWhoseReasonIsBlank_ReportsItAsUNSTATED_NotAsNothing, which uses whitespace rather than an empty string on purpose: an empty string is the obvious case and a Length > 0 check would pass it, while " " is what a poster writing $"{maybeEmpty} " actually produces. Negative control measured — with the normalisation neutralised and the test project rebuilt, it fails on the real production string:

… The teardown was requested by host/1 (routed DisposeRequest); why:    . The address may reactivate …

and the other four tests in the class stay green. It also asserts the WHO half survives, so "the reason came out named" cannot be satisfied by the attribution having been dropped.

[rbuergi]

Valid, and the sharpest of the three: a page whose whole subject is "a report must not tell you the wrong thing" was itself telling operators to expect the wrong attribution for a supported teardown path. Fixed.

You are right on the mechanism: DisposalRequestedBy prefers cascadeOwner and renders a cascade from its owner <owner>, and DisposalReason renders the owner's own teardown — <originating cause>. My table folded that into the direct-dispose row, which is precisely the conflation cascadeOwner was added to remove — before it existed, every child in a wave reported a direct Dispose(), indistinguishable from host teardown, and reading the child told you nothing about the root (#3510's wedge one level down).

The table now has five rows, not three, and the two I had missed are both real distinctions:

• the owner's cascade, with its own WHO and its own WHY, carrying the originating cause propagated unchanged down the chain — so a leaf names the event that started it however deep the tree is, rather than only its immediate parent;
• a self-posted DisposeRequest (a rebind, a self-heal), which is a different reading again from "somebody else asked".

I added a short paragraph under the table saying why each distinction is load-bearing and cannot be inferred from the others, plus a "One cause, claimed once" section describing the atomic claim and the already-begun check from the other two threads — because the table is only true if exactly one writer ever records the cause.

[github-actions]

Test Results (shard 1)

1 472 tests  ±0   1 472 ✅ ±0   2m 48s ⏱️ -8s
    2 suites ±0       0 💤 ±0 
    2 files   ±0       0 ❌ ±0 

Results for commit 39d0944. ± Comparison against base commit 8d3c307.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 2)

913 tests  +3   913 ✅ +3   3m 26s ⏱️ +4s
  2 suites ±0     0 💤 ±0 
  2 files   ±0     0 ❌ ±0 

Results for commit 39d0944. ± Comparison against base commit 8d3c307.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 4)

1 847 tests  +1   1 655 ✅ +1   4m 54s ⏱️ +2s
    4 suites ±0     192 💤 ±0 
    4 files   ±0       0 ❌ ±0 

Results for commit 39d0944. ± Comparison against base commit 8d3c307.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 5)

    6 files  ±0      6 suites  ±0   4m 35s ⏱️ - 1m 5s
2 183 tests ±0  2 183 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 187 runs  ±0  2 187 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 39d0944. ± Comparison against base commit 8d3c307.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 3)

1 092 tests  ±0   1 090 ✅ ±0   6m 27s ⏱️ +6s
    3 suites ±0       2 💤 ±0 
    3 files   ±0       0 ❌ ±0 

Results for commit 39d0944. ± Comparison against base commit 8d3c307.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results

   18 files  ±0     18 suites  ±0   24m 31s ⏱️ -35s
7 769 tests +4  7 575 ✅ +4  194 💤 ±0  0 ❌ ±0 
7 773 runs  +4  7 579 ✅ +4  194 💤 ±0  0 ❌ ±0 

Results for commit 39d0944. ± Comparison against base commit 8d3c307.

♻️ This comment has been updated with latest results.

[meshweaver-cloud]

ℹ️ Merge-queue steward: no action — removed from the queue with reason MANUAL — the steward takes no action for this reason.