Repository navigation
fix(#3510): the upsert's CREATE leg answers instead of hanging forever - #3603
Conversation
MEASURED on the fleet's critical path. MeshWeaver.Plugins #1463's gate shard (run 34115469651, job 101732362683): Hosting is installed 12:01:57, its bundle adopted 12:01:59-12:02:27, then CreateOrUpdateNodeRequest@portal/nodeops sits pending 30 -> 58 s "handler-side", a satellite write into Hosting/InstanceAction times out at 12:03:30, and the package never reaches its "installed" marker -- the tester gives up 8 minutes later. Per-package pace is otherwise unchanged against the green pin, so it is one package hanging, not a slowdown. Core already named the leg and did not fix it. UpsertAnswersWhenTheOwnerGoesAway says so in its own remarks: ":51 the CREATE leg, which has no disposal-NACK" and ":77 while #3510's create leg is still open". PackageInstaller.SettleRetypedRoot recycles the root it just retyped on EVERY plugin install (retypedRoot non-null iff !rootTypeIsStatic; Store/Plugin is dynamic), and that commit's own timing table lists Hosting at +7.9 s -- the fingerprint above. DispatchInnerCreate's Subscribe had two arms, onNext and onError. When the owner goes, the response never arrives at all, so NEITHER runs. This is not a source that completes empty -- it is one that never terminates, the same class as #3550. DefaultIfEmpty would be wrong for the reason the READ leg's own comment gives. The leg is now total: Take(1) BEFORE Timeout so the deadline is total rather than Rx's inter-emission one (#3550's trap), plus the third terminal arm the READ leg has carried since #2454, and a claim flag so the arms cannot double-post. The bound is STRICTLY ABOVE LatePatchResponseRegistry.WriteVerdictBound, on that property's own instruction, so the framework's OwnerUnreachable wins and names the cause. Bounding AT it is #2819 -- the refusal that explains nothing arriving first. I made that mistake in the first draft; the new test is what catches it. NOT a widened bound: the leg had no terminal guarantee at all. NOT the whole of #3510: the owner-side disposal NACK the UPDATE leg gets from RegisterOwnerDisposingNack is still missing on this leg, so #3510 stays open. Verified: Release -warnaserror clean on MeshWeaver.Mesh.Contract, MeshWeaver.Graph.Test, MeshWeaver.Hosting.Test. The existing update-leg test still passes. The new ordering test is falsified both ways -- planted at the framework bound it goes red naming both values (00:00:31 vs 00:00:31); restored, 2/2 green. Hand-written timeout literals unchanged at 206. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new timeout/completed-empty refusal text asserts the create was “NOT applied,” which is not guaranteed under a timeout/disposed Observe path, and the onCompleted arm should claim innerAnswered for consistency and future safety.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR makes the CREATE leg of the single-verb upsert (CreateOrUpdateNodeRequest) terminate deterministically when the inner CreateNodeRequest never yields a response (e.g., owner recycled mid-install), preventing the caller from hanging until its outer budget expires.
Changes:
- Add
MeshExtensions.InnerCreateVerdictBound, explicitly ordered strictly aboveLatePatchResponseRegistry.WriteVerdictBound. - Make
DispatchInnerCreatetotal by addingTake(1)+ a boundedTimeout(...)and a third terminal (onCompleted) arm, plus diagnostics/logging. - Add a unit test pinning the bound-ordering invariant, and a What’s New entry documenting the behavioral change.
File summaries
| File | Description |
|---|---|
src/MeshWeaver.Mesh.Contract/MeshExtensions.cs |
Adds the create-leg verdict bound and ensures the inner-create observe path has explicit terminal outcomes (timeout + completed-empty). |
test/MeshWeaver.Hosting.Test/InnerCreateBoundOutlivesTheFrameworkVerdictTest.cs |
Pins the strict inequality ordering against LatePatchResponseRegistry.WriteVerdictBound and constrains the margin. |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-07-an-upsert-whose-owner-goes-away-mid-create-now-answers.md |
Documents the new “answers instead of silence” behavior for the upsert create leg. |
Review details
Suppressed comments (1)
src/MeshWeaver.Mesh.Contract/MeshExtensions.cs:4801
- The onCompleted terminal arm posts a refusal but never sets innerAnswered=true, even though the surrounding comment says the terminal arms "claim" the response. Setting it improves consistency and avoids any future refactor accidentally treating this path as unanswered; this is also a good place to avoid asserting the create was definitely not applied.
// The third terminal state, for the same reason the READ leg above has one
// (#2454): a two-arm Subscribe says NOTHING when the source completes empty,
// and the handler has already returned Processed() by then.
if (innerAnswered)
return;
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| logger.LogWarning( | ||
| "[CreateOrUpdate] the inner CreateNode for {Path} produced no response " | ||
| + "within {Bound} — answering the caller with a refusal rather than " | ||
| + "leaving it to wait out its budget. The usual cause is the owner being " | ||
| + "recycled under its own install (PackageInstaller.SettleRetypedRoot " | ||
| + "retypes then recycles the package root on every plugin install), in " | ||
| + "which case the create was NOT applied and is safe to retry against " | ||
| + "the fresh activation (#3510).", | ||
| node.Path, InnerCreateVerdictBound); | ||
| PostFail( | ||
| $"The create for '{node.Path}' produced no response within " | ||
| + $"{InnerCreateVerdictBound.TotalSeconds:0}s. It was NOT applied; retry " | ||
| + "against the fresh activation.", | ||
| NodeUpsertRejectionReason.Unknown); |
WhatsNewEntryIntegrityTest.EveryEntry_CarriesTheFrontmatterTheFeedRendersFrom was the single failing test behind #3603's two red checks: the entry had only Category, missing Name/Description/Icon/Order. Verified 1/1 after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The upsert's CREATE leg answers instead of hanging forever
This is the wedge currently blocking MeshWeaver.Plugins #1463, and through it six PRs and five
issues. It is not a new regression — core already named this leg and did not fix it.
Measured, on the critical path
Plugins #1463's gate shard (run
34115469651, job101732362683), measured by meshweaver-64:Per-package pace is otherwise unchanged against the green pin (median 11.0 → 11.5 s), so this is one
package hanging, not a slowdown.
The mechanism, in core's own words
PackageInstaller.SettleRetypedRootrecycles the root it just retyped from itsSpaceplaceholder,immediately after adoption. It fires on every plugin package install (
retypedRootis non-nulliff
!rootTypeIsStatic, andStore/Pluginis dynamic).7dd16fad0's own timing table listsHosting +7.9s— the fingerprint above.test/MeshWeaver.Graph.Test/UpsertAnswersWhenTheOwnerGoesAwayTest.csalready says which leg::31HANDLER_ENTER → UPSERT_READ absent → create → HANDLER_EXIT state=Processed ⇒ (nothing):51"the CREATE leg, which has no disposal-NACK":77"while A package root is disposed while its own install is in flight (CD 7950): the creates' write-through streams lose their owner, no reply is ever sent, and the install runs out its bound — neither #3501 nor #3505 covers it #3510's create leg is still open"That commit added 15 lines of logging, a doc, and a 177-line test pinning the covered (update)
leg. The create leg was left as it was.
The defect
DispatchInnerCreate'shub.Observe(inner, …).Subscribe(…)has two arms — onNext and onError.When the owner is recycled the response never arrives at all, so neither runs. This is a source
that never terminates, not one that completes empty — the same class as #3550.
DefaultIfEmptyisthe wrong repair, for the reason the READ leg's own comment already gives.
The fix
The leg is now total:
Take(1)beforeTimeoutso the deadline is total rather than Rx'sinter-emission one (#3550's trap), the third terminal arm the READ leg has carried since #2454, and
a claim flag so the arms cannot double-post. The refusal says the create was not applied, is safe
to retry, and names the recycle as the usual cause.
🚨 The bound is strictly above
LatePatchResponseRegistry.WriteVerdictBound, on that property'sown instruction — "Anything waiting on a write must bound itself STRICTLY ABOVE this, so the
framework's terminal wins and names the cause." Bounding at it is #2819: the refusal that
explains nothing arriving one moment before the one that does. I made exactly that mistake in the
first draft, and the new test is what catches it.
What this is not
into a named refusal. It buys no headroom.
RegisterOwnerDisposingNackis still missing here, so A package root is disposed while its own install is in flight (CD 7950): the creates' write-through streams lose their owner, no reply is ever sent, and the install runs out its bound — neither #3501 nor #3505 covers it #3510 stays open — this removes thesilence, not the disposal.
literal that
TestTimeoutLiteralRatchetGuardrefuses, and the existing rig cannot stage the race(its own note: "parking the partition root's primary stream does NOT put the create behind it —
measured"). What is pinned is the part that was actually got wrong: the direction of the
inequality.
Verification
dotnet build -c Release -warnaserror, one project per invocation, exit read directly:MeshWeaver.Mesh.Contract,MeshWeaver.Graph.Test,MeshWeaver.Hosting.Test— 0/0 each.both values (
00:00:31vs00:00:31); restored, 2/2 green.Refs #3510
🤖 Generated with Claude Code