Repository navigation
fix: an unowned AiSettings seed outlived its reader and stranded a create - #1269
Merged
Merged
Conversation
…eate
`AiSettingsNodeType.Observe` opened with a bare statement:
EnsureExists(hub, services, user);
`EnsureExists` was `void` and subscribed its own chain, so the create-on-absent
write had NO owner. Two consequences, both real:
* it fired at COMPOSITION time — building the observable was enough, even if
nobody ever subscribed to the settings stream; and
* nothing could cancel it, so it could still be in flight — or start — after
the last reader had gone away, including on a hub that was already tearing
down.
That is what reddened SkillAutocompleteTest.Autocomplete_ListsBuiltInSkills_-
FromCatalog on CI (3 occurrences on main in one morning: runs 31569022680,
31574636901, 31577766913). The test body PASSES — start 07:45:18.862, end
07:45:19.083 — and then teardown's quiesce guard throws, reporting two pending
CreateNodeRequests for Roland/_Memex/AiSettings:
cb1 ... CREATE_CHAIN_SUBSCRIBED path=Roland/_Memex/AiSettings
-> BOOTSTRAP_PERM_AWAIT partition=Roland user=system-security (+189ms)
-> BOOTSTRAP_PERM_VERDICT authorized=True (+512ms) [no reply, ever]
cb2 ... RECEIVED runLevel=Quiescing -> DEFERRED gates=[DataContextInit]
cb1 is the seed. Because the test mesh has no `Roland` partition root,
EnsurePartitionBootstrap heals it, and the heal issues cb2 — a NESTED root
create. By then the mesh is Quiescing, so cb2 parks behind `DataContextInit`,
a gate shutdown never reopens; cb1's chain therefore neither emits, faults nor
completes, and no response is ever posted. The 500 ms quiesce budget expires and
the guard fails the class. On a loaded CI runner the 512 ms permission fold alone
overruns that budget, which is why it only reproduces there.
Root fix: the seed is MERGED INTO the stream it serves, so its lifetime is the
consumer's subscription — it starts on Subscribe (not at composition) and is
disposed when the consumer unsubscribes. `EnsureExists` now returns a cold
`IObservable<Unit>` and no longer self-subscribes, matching its sibling
`NotificationSettingsNodeType.EnsureExists`. Errors stay contained exactly as
before (log + swallow) so a failed seed can never break the settings read.
This does not paper over the two deeper holes the trace also exposes — a gate
that can never reopen after shutdown, and nested creates inside
EnsurePartitionBootstrap that ride an unbounded `meshService.CreateNode` whose
advertised `MeshOperationOptions.Timeout` is dead code. Both are filed
separately; neither is reachable from here once the seed cannot outlive its
reader.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an intermittent teardown-time leak in AiSettingsNodeType.Observe where a self-subscribing create-on-absent could outlive its reader and strand CreateNodeRequest callbacks during hub shutdown, intermittently reddening SkillAutocompleteTest on CI.
Changes:
- Refactors
AiSettingsNodeType.EnsureExistsfrom a self-subscribingvoidmethod into a coldIObservable<Unit>so the caller owns the subscription lifetime. - Merges the create-on-absent (“seed”) observable into the settings read stream so it starts on subscribe and is disposed when the consumer unsubscribes.
- Preserves the prior “log + swallow” behavior so seeding failures don’t break consumers of the settings stream.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+379
to
+382
| services.GetService<ILoggerFactory>() | ||
| ?.CreateLogger(typeof(AiSettingsNodeType)) | ||
| .LogWarning(ex, "EnsureExists: AiSettings create-on-absent failed for {Path}", PathFor(user)); | ||
| return Observable.Empty<AiSettings>(); |
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This was referenced Aug 12, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
MeshWeaver.AI.Test.SkillAutocompleteTest.Autocomplete_ListsBuiltInSkills_FromCatalog— 3 failures onmainin one morning (runs31569022680,31574636901,31577766913), the most frequent single contributor to the current intermittent-red cluster.It is not an assertion failure. The test body passes (start
07:45:18.862, end07:45:19.083); teardown then throws:Mechanism
AiSettingsNodeType.Observeopened with a bare statement —EnsureExists(hub, services, user);— andEnsureExistswasvoidand subscribed its own chain. The create-on-absent write had no owner, so:cb1is that seed. The test mesh has noRolandpartition root, soEnsurePartitionBootstrapheals it, and the heal issuescb2, a nested root create. By then the mesh is Quiescing, socb2parks behindDataContextInit— a gate shutdown never reopens.cb1's chain therefore neither emits, faults, nor completes, no response is posted, the 500 ms quiesce budget expires, and the guard fails the class.On a loaded CI runner the 512 ms permission fold alone overruns the 500 ms budget — which is exactly why this reproduces there and not locally.
Fix
The seed is merged into the stream it serves, so its lifetime is the consumer's subscription: it starts on
Subscribe(not at composition) and is disposed when the consumer unsubscribes.EnsureExistsnow returns a coldIObservable<Unit>and no longer self-subscribes — matching its siblingNotificationSettingsNodeType.EnsureExists, which already had this shape.Error handling is unchanged (log + swallow), so a failed seed still cannot break the settings read that consumers actually subscribe for.
Not papered over
The trace also exposes two deeper holes, filed as #1270 — neither is reachable from here once the seed cannot outlive its reader:
DataContext.OpenInitializationGatereturns without opening the gate when the hub is shutting down, stranding anything already deferred behind it.EnsurePartitionBootstrapride an unboundedmeshService.CreateNode; the advertisedMeshOperationOptions.Timeoutis dead code (no.Timeout(anywhere inMeshService.cs).Verification
dotnet build src/MeshWeaver.AI -c Release -warnaserror→ 0 Warning(s), 0 Error(s)SkillAutocompleteTestgreen after the change (Total: 1, Failed: 0)MeshWeaver.AI.Testassembly greenNo What's New entry: the user-visible behaviour of the settings read is unchanged — this fixes a teardown-scoped leak in how the seed's lifetime is owned.
🤖 Generated with Claude Code