DeleteNode drain reports "did not drain within 30s" after removing 162 of 161 planned paths — and aborts the rest of an approved operation #3392

Activity

1. rbuergi commented on Sep 6, 2026

   

   rbuergi

   on Sep 6, 2026

   ContributorAuthor

   More actions

   Confirmed: the failed delete had in fact completed

   The operation was re-run after a fresh preview. Step 53 — the exact step that threw — reported:

   ##STEP## 53/68 delete-space AgenticOffice: absent — nothing to delete
   

   So AgenticOffice was already fully gone when the first run declared it "did not drain within 30s". That closes the question the original report left open: this is a false negative in the completion check, not a partial delete. The run then completed ##DONE## 68 step(s), status: Succeeded.

   A second, quieter symptom in the same evidence

   On the re-run, seven Spaces the first run had logged as deleted were found with one descendant still present:

   Space	first run	re-run found
   ReinsurancePractice	deleted (208 drained)	1 descendant(s) → deleted
   Reinsurance	deleted (328 drained)	1 descendant(s) → deleted
   Underwriting	deleted (1394 in 36 chunks)	1 descendant(s) → deleted
   RiskTransfer	deleted (106)	1 descendant(s) → deleted
   ClaimsDeepfield	deleted (82)	1 descendant(s) → deleted
   AdvancedBusinessRules	deleted (144)	1 descendant(s) → deleted
   AgenticEngineering	deleted (362)	1 descendant(s) → deleted

   while ReinsuranceDemo, Ifrs17, SST, SwissSolvencyTest, Planning, AgenticBusiness were genuinely absent — nothing to delete.

   So delete-space reported success while leaving one node behind, roughly half the time — the mirror image of the reported defect: the same completion check that refuses to admit a finished delete also admits an unfinished one. I have not established what that residual node is (root? a satellite created during the drain?), and I am not claiming the two share a cause — but they are the same predicate and the evidence for both came from one run pair.

   That also means a single run of this operation would have left seven Spaces partially deleted while reporting success, and nothing would have said so.

   Method note

   The first run's failure was nearly missed: the request node's activityPath still pointed at the PREVIOUS (preview) activity, which reads status: Succeeded. The failing run was a different activity that had to be found by listing _Activity children. A caller reading activityPath gets a stale verdict about a run that failed.

   Evidence: activity 0fefe74fd49b485da5077e3526d80917 (Succeeded, 84 messages, 2026-09-06T10:55:23→10:55:50Z) vs 27992cd1d4e244619f94e61050999ea0 (Failed, 79 messages, 10:34:31→10:37:40Z).
2. rbuergi commented on Sep 6, 2026

   

   rbuergi

   on Sep 6, 2026

   ContributorAuthor

   More actions

   The completion check is not the defect — the timeout is. Read before fixing.

   Chased this into the code, because the write-up above would send a fixer at the wrong line.

   The completion predicate is already correct. RunDeletePass does not compare counts at all:

   var survivorSet = survivors.Where(p => !string.IsNullOrEmpty(p))
       .ToImmutableHashSet(StringComparer.OrdinalIgnoreCase).Remove(rootPath);
   if (survivorSet.IsEmpty)
       return Observable.Return((IReadOnlyList<string>)acc);   // ← drained, done

   So "a completion check satisfied only by removed == planned" is not what is implemented, and
   162 of 161 is not a predicate mis-firing. It is the timeout's progress message, rendered on the
   way out — {done.Count} of {collected.ToDelete.Count} planned path(s). Confusing, but a symptom.

   The actual defect is at MeshExtensions.cs:2752:

   return DeleteSubtreeUntilDrained(...)
       .TimeoutAtStage(budget, () => …)          // TimeoutAtStage == source.Timeout(timeout, …)

   Observable.Timeout is an inter-emission timeout. DeleteSubtreeUntilDrained emits exactly
   once, at the very end of the whole multi-pass drain — so on a single-emission source that
   degenerates into a total-duration cap. A drain that is removing nodes steadily is killed at 30 s
   purely for taking 30 s.

   That also explains the asymmetry the report found puzzling, and it is not about size:

   • each stage gets its own budget (six separate TimeoutAtStage calls — they share the value,
     not a running clock), and
   • the chunked path is chunked by the caller, so each chunk is a separate DeleteNode call with a
     fresh 30 s.

   So Underwriting (1394 descendants) survived 59 s as 36 calls × 30 s each, while AgenticOffice
   (106) died as one call needing several passes. Whether an operation fits is decided by how the
   caller chunked it, not by how much work there is.

   The fix shape — and explicitly NOT raising the bound

   Make the drain emit once per pass instead of once at the end. Then the existing
   Observable.Timeout becomes what it should have been all along: a no-progress watchdog, firing
   only when a pass produces nothing for the budget, never while the drain advances.

   // RunDeletePass, survivors branch — surface progress before recursing
   return Observable.Return((IReadOnlyList<string>)acc)
       .Concat(RunDeletePass(…, pass + 1, acc));

   // call site: the stage now emits per pass, so collapse to the final value
   .TimeoutAtStage(budget, …)
   .TakeLast(1)          // NOT .Last() — that one blocks

   Total time stays bounded, because MaxDeleteDrainPasses = 5 already caps the passes: worst case
   5 × budget, and a genuinely stuck subtree still fails loudly with the existing
   "could not drain the subtree after N pass(es)". The message at
   MeshExtensions.cs:2757 should then read "made no progress for {budget}s" and drop the
   N of M planned ratio that produced 162 of 161.

   Why I am not landing it

   This is the recursive-delete path, and the surrounding code carries a dense incident history —
   #1198 (stage-tagged budgets), #1128 (caller authorization evaporating mid-commit), #2477 (two hubs,
   two duties), plus the tombstone resurrect-race. Changing when it gives up deserves a test that
   drives a multi-pass drain, and the seam for that does not exist yet: DeleteSubtreeUntilDrained is
   private static and MeshWeaver.Mesh.Contract grants InternalsVisibleTo only to
   MeshWeaver.Graph.Test. Landing a change to delete semantics with no test that could fail is the
   pattern I would flag in review, so I am not doing it here.

   Whoever picks it up: the change is small, the bound is already there, and the correction that matters
   is that the predicate is fine and the timeout is measuring the wrong quantity.

   🤖 Filed from Claude Code.
3. rbuergi

   closed this as completedin #3418on Sep 6, 2026