Skip to content

Heal durable self-typed NodeType declaration rows at startup - #2605

Merged
rbuergi merged 3 commits into
mainfrom
fix/2425-useridentitycache-type-node
Aug 28, 2026
Merged

rbuergi merged 3 commits into
mainfrom
fix/2425-useridentitycache-type-node

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #2425. Fixes #2506.

The defect that survived two fixes

UserIdentityCache logged As<User> for User: value is NodeTypeDefinition (MeshWeaver.Graph), not convertible ~600 times/hour on memex.systemorph.com (and ~70/4h on memex.meshweaver.cloud) — on image ci.6096, which already carried BOTH prior fixes:

Neither touches a row that was persisted self-typed before they landed. Verified live: search nodeType:User on prod still returns {path: "User", nodeType: "User", version: 2, lastModified: 2026-06-02} — the durable declaration fossil, answering the user-directory query beside real accounts. Reads are bad-data tolerant by design, and the row is unreachable through the serve path (the static claim shadows it — the #2534 mechanism), so "until it is next written" was never: the defect had moved from the code into the data, and no code-only guard could end it.

The fix — heal the data, once, at startup

SelfTypedDeclarationDurableRepair (hosted service, registered by AddGraph beside the validator) runs once at startup, at the storage seam where LegacyUserPartitionRepair already writes (the only seam that can reach a static-shadowed row):

  • one batched IStorageAdapter.ReadMany over the statically-registered declaration paths (the same multi-path probe the URL resolver runs constantly — absent partitions answer "absent", never fault: the PG routing adapter's reads catch 42P01 by contract);
  • rewrites exactly the rows matching the write guard's own predicate — extracted as NodeTypeDeclarationSelfTypingValidator.IsSelfTypedDeclaration (public, shared), so what the guard refuses and what the repair heals can never drift apart;
  • the rewrite is A NodeType declaration declares itself a NodeType, never the type it declares #2245's correction applied to the row: NodeType = "NodeType", version minted forward, content/identity untouched;
  • everything else is left alone — already-correct declarations, real user instances, package roots typed as an unrelated type (the UWDeepfield shape). A healthy store reads one batch and writes nothing;
  • fire-and-forget and failure-tolerant like InstalledPackageRepairService: a skipped pass heals on the next boot; concurrent replicas write the same value under the monotonic-version guard.

The adapter change feed fires on the heal, so live nodeType:User subscriptions (UserIdentityCache's directory query) converge without a restart.

Repro test — red on main for the right reason

SelfTypedDeclarationDurableRepairTest boots a mesh over a store already carrying the fossils (seeded straight into the adapter before boot — exactly how production got them):

  • FossilSelfTypedDeclarationRows_AreRetypedAtStartup — the User and VUser fossils flip to NodeType = "NodeType" (content preserved, version forward); an already-correct Partition declaration row and a real user instance are untouched.
  • TheUserDirectory_NoLongerReturnsTheFossilDeclaration — UserIdentityCache.DirectoryQuery (verbatim) returns the real account and NOT the declaration; every returned row reads as a User.

On main both fail with TimeoutException waiting for a heal that never happens (Failed: 2, Passed: 0); with the fix, Passed: 2 in <1s.

Verification

  • MeshWeaver.Graph and MeshWeaver.Hosting.Monolith.Test build clean -c Release -warnaserror.
  • Adjacent guards stay green: NodeTypeDeclarationSelfTypingValidatorTest (9 — the predicate extraction is behavior-preserving), NodeTypeDeclarationSelfTypingTest (2), WhatsNewEntryIntegrityTest.
  • What's New entry included (Category: Fix).

🤖 Generated with Claude Code

The User/VUser/Partition declarations shipped self-typed and were
PERSISTED that way on live stores. #2245 fixed the static registrations
and #2378 refuses any new write carrying the collision — but a row
already persisted with nodeType:"User" keeps answering every
nodeType:User query beside the real accounts, forever: reads are
bad-data tolerant by design, and the serve path cannot reach the row
(the static claim shadows it, #2534). On production that was ~600
UserIdentityCache "As<User> for User: value is NodeTypeDefinition"
errors per hour on an image that already carried both fixes (#2425,
#2506) — the defect had moved from the code into the data.

SelfTypedDeclarationDurableRepair (a hosted service registered by
AddGraph) retypes such rows once at startup, at the storage seam where
LegacyUserPartitionRepair already writes: one batched ReadMany over the
statically-registered declaration paths, then a rewrite (NodeType =
"NodeType", version minted forward) of exactly the rows matching the
write guard's own predicate — extracted as
NodeTypeDeclarationSelfTypingValidator.IsSelfTypedDeclaration so the
guard and the repair can never drift apart. Already-correct
declarations, real instances and package roots typed as an unrelated
type are untouched, so a healthy store reads one batch and writes
nothing. The adapter change feed fires on the heal, so live nodeType:
query subscriptions (UserIdentityCache's directory) converge without a
restart.

Repro pinned by SelfTypedDeclarationDurableRepairTest: a mesh booted
over a store already carrying the fossil rows — red on main (the rows
never heal; the user directory keeps returning the declaration), green
with the repair.

Fixes #2425. Fixes #2506.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 28, 2026 12:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a startup-time durability repair in MeshWeaver.Graph to retroactively fix persisted NodeType declaration rows that were historically self-typed (e.g., durable User declaration with nodeType:"User"), which caused nodeType:User queries (notably UserIdentityCache.DirectoryQuery) to return a declaration row alongside real user instances.

Changes:

  • Introduces SelfTypedDeclarationDurableRepair as an IHostedService to batch-read statically-registered declaration paths and rewrite only rows matching NodeTypeDeclarationSelfTypingValidator.IsSelfTypedDeclaration.
  • Extracts and publishes the self-typing predicate (IsSelfTypedDeclaration) so the write-guard and the durable repair share one definition.
  • Adds a monolith integration test that seeds “fossil” rows directly into the storage adapter and verifies both the durable retype and the corrected user directory behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/MeshWeaver.Hosting.Monolith.Test/SelfTypedDeclarationDurableRepairTest.cs Seeds fossil rows into an in-memory adapter pre-boot and asserts startup healing + user directory exclusion.
src/MeshWeaver.Graph/Security/SelfTypedDeclarationDurableRepair.cs New hosted service that repairs persisted self-typed declaration rows via storage-layer reads/writes at startup.
src/MeshWeaver.Graph/Security/NodeTypeDeclarationSelfTypingValidator.cs Refactors to use/expose a shared predicate (IsSelfTypedDeclaration) used by both guard and repair.
src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs Registers the new hosted service alongside existing graph validators.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-28-user-directory-declaration-repair.md Adds a “What’s New” entry documenting the user-directory repair behavior.

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

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

474 tests   470 ✅  4m 53s ⏱️
  6 suites    4 💤
  6 files      0 ❌

Results for commit 5ed8d08.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

957 tests   957 ✅  5m 14s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit 5ed8d08.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 582 tests   1 581 ✅  6m 58s ⏱️
    8 suites      1 💤
    8 files        0 ❌

Results for commit 5ed8d08.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

898 tests   898 ✅  5m 27s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit 5ed8d08.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

   10 files     10 suites   6m 2s ⏱️
3 755 tests 3 563 ✅ 192 💤 0 ❌
4 383 runs  4 191 ✅ 192 💤 0 ❌

Results for commit 5ed8d08.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

995 tests   995 ✅  6m 32s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit 5ed8d08.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   42 files     42 suites   35m 8s ⏱️
8 661 tests 8 464 ✅ 197 💤 0 ❌
9 289 runs  9 092 ✅ 197 💤 0 ❌

Results for commit 5ed8d08.

♻️ This comment has been updated with latest results.

…as rooting the mesh hub

MeshHubDisposalLeakTest failed on this PR's first CI run, and its ClrMD GC-root analysis named
the chain exactly:

    IHostedService[]
      -> MeshWeaver.Graph.Security.SelfTypedDeclarationDurableRepair
        -> MeshWeaver.Messaging.MessageHub  [Address=mesh/v6J4wXmBGUWYem-6gzlChw]

The generic host keeps its IHostedService[] for the whole process lifetime, so the hub this
one-shot held in a field outlived every mesh it was built for. That ACCUMULATES: a process that
builds many meshes — the test suite, a portal recycling silos — keeps all of them alive through
that array.

Both fields are now released the moment they stop being needed. The hub goes on the first line of
StartAsync via Interlocked.Exchange, so the field is null before the method returns however it
returns (including the two early exits — no storage adapter, no declarations). The sweep handle
goes when the sweep terminates: a SingleAssignmentDisposable is published to the field BEFORE it
is armed, so on a synchronously-completing store Finally nulls the field first and arming an
already-disposed gate disposes the inner subscription on the spot. Either order leaves the
instance referencing nothing. StopAsync and the Finally share one Release(), because the sweep
completing and the host stopping race by construction.

🚨 The retention is pinned LOCALLY, not only by the leak test. MeshHubDisposalLeakTest is the
outcome check but its ClrMD snapshot-attach is unavailable on macOS, where it Assert.SkipWhens as
inconclusive (#674) — so on every dev machine here the only signal is CI, three hours later on a
Linux runner. SelfTypedDeclarationRepairRetainsNothingTest asserts the MECHANISM deterministically:
the hub field is null after StartAsync, the pass is one-shot, the subscription handle is dropped
when the sweep terminates, and StopAsync is safe before any start and twice.

Red-proof: reverting the exchange to a plain field read fails two of the four, with
"...found MeshWeaver.Messaging.MessageHub" — the chain ClrMD named. All 6 tests across both
classes pass on the fix; MeshWeaver.Graph and the test project build Release -warnaserror clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi

rbuergi commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Root cause of the CI red: this hosted service was rooting the mesh hub for the process lifetime

Picked this up from the session that filed it (its triage subagent died mid-run on a 429, so there was no live hand). Handing the finding forward, because the diagnosis is the useful part.

MeshHubDisposalLeakTest.MeshHub_IsCollected_AfterMeshAndServiceProviderDisposal failed on the first run, and its ClrMD GC-root analysis named the chain outright — no inference needed:

IHostedService[]
  -> MeshWeaver.Graph.Security.SelfTypedDeclarationDurableRepair
    -> MeshWeaver.Messaging.MessageHub  [Address=mesh/v6J4wXmBGUWYem-6gzlChw]

The generic host keeps its IHostedService[] for the whole process lifetime, so a hub held in a field on a one-shot startup service outlives every mesh it was built for. That is not a one-mesh problem — it ACCUMULATES: a process that builds many meshes (this test suite; a portal recycling silos) keeps all of them, and everything they own, alive through that array.

The fix — both fields released the moment they stop being needed

  • The hub goes on the first line of StartAsync, via Interlocked.Exchange. That makes the field null before the method returns however it returns — including the two early exits (no storage adapter, no declarations) — and makes the pass one-shot for free.
  • The sweep handle goes when the sweep terminates. A SingleAssignmentDisposable is published to the field before it is armed, so on a synchronously-completing store Finally nulls the field first and arming an already-disposed gate disposes the inner subscription on the spot. Either order leaves the instance referencing nothing.
  • StopAsync and the Finally share one Release(), because the sweep completing and the host stopping race by construction.

🚨 Pinned locally, not only by the leak test

MeshHubDisposalLeakTest is the outcome check, but its ClrMD snapshot-attach is unavailable on macOS — it Assert.SkipWhens as inconclusive there (#674). So on every developer machine in this repo the only signal is CI, hours later on a Linux runner. SelfTypedDeclarationRepairRetainsNothingTest asserts the mechanism deterministically and locally:

  1. the hub field is null after StartAsync (the direct edge of the chain), with the premise asserted first so it cannot pass vacuously;
  2. a second StartAsync is an inert no-op;
  3. the subscription handle is dropped when the sweep terminates — waited on the condition, never a sleep;
  4. StopAsync is safe before any start and twice.

Red-proof: reverting the exchange to a plain field read fails two of the four, with "...but found MeshWeaver.Messaging.MessageHub". All 6 tests across both classes pass on the fix, and MeshWeaver.Graph + the test project build Release -warnaserror clean.

@rbuergi

rbuergi commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

The red is #890, not this diff — and it is the second recurrence since that issue was closed

Shard 1 failed seven tests on run 33179258773. Every one is a compile, and nothing else in the shard failed:

NodeTypeAlcSharedWithInstancesTest, three NodeTypeReleaseGateTest cases, two MeshNodeCompilationIntegrationTest cases, and CompileSourceSnapshotWedgeTest — the "one Emit poisons the process, every LATER compile NREs" shape #890 is named for.

The attribution is not mine to argue; the instrumentation makes it. The emit canary's verdict, verbatim:

canary=BELOW-ROSLYN
  shared:   THREW NullReferenceException at NamedTypeSymbol.…get_ContainingTypeDefinition
  pristine: THREW NullReferenceException at NamedTypeSymbol.…get_ContainingTypeDefinition
⇒ a trivial compilation with BRAND-NEW references and freshly parsed source cannot emit either,
  and BOTH legs died in the same frame ⇒ nothing about the reference set explains this;
  the broken state is below Roslyn (CLR heap / JIT / GC)

The pristine leg — brand-new references, freshly parsed source — died in the same frame as the shared one. That is exactly the discriminator built to separate "a PR changed the reference set" from "the process is poisoned", and it says the latter.

This PR adds a startup hosted service that retypes durable self-typed NodeType declaration rows at the storage layer. It has no path to Roslyn's emit.

Reopened #890 with this occurrence: it was closed 2026-08-18, recurred 2026-08-24 on main (32784957424, shard 4) where the note said "the canary's dump-capture + tiering-off advice stands if it recurs", and has now recurred again. Leaving it closed means every occurrence gets re-diagnosed and pinned on whichever PR was running — which is what the canary exists to prevent.

Merged main for a fresh run (AGENTS.md sanctions this for a red the diff cannot reach). Revert-checked: the diff is still the same 6 files. If the same seven compile tests fail again, that is #890 and not this change — the fix belongs there, and this PR should not be held for it.

@rbuergi
rbuergi merged commit e3721b8 into main Aug 28, 2026
23 checks passed
rbuergi added a commit that referenced this pull request Aug 30, 2026
… its instance query is pinned to (#2723)

* fix(#2641): heal the self-typed User declaration inside the partition its instance query is pinned to

The startup repair (#2605) read the declaration paths through path-routed
storage only. On memex-cloud `User`'s first segment routes to the partition
`user`, which V27 renamed to `auth`, so the probe answered "absent" while the
fossil sat in the auth schema — exactly where UserNodeType pins every
path-less `nodeType:User` query, i.e. where UserIdentityCache reads it. The
sweep wrote nothing and, having no line for "nothing found", said nothing.

The sweep now resolves each candidate declaration's own instance query
through MeshConfiguration.ResolveRoutingHints and, when it is pinned to a
partition, also reads the path inside that partition via each writable
IPartitionStorageProvider's partition-scoped adapter (CreateAdapterForTable),
retyping a fossil through the adapter that found it. Candidates are filtered
with the validator's now-public IsNodeTypeDeclarationContent, and every pass
ends with an Information summary (paths, pinned partitions, rows read /
retyped) so a miss is a count of zero rather than silence.

SelfTypedDeclarationPinnedPartitionRepairTest models the production store
(specific Auth provider whose path-routed adapter declines `User`, whose
scoped adapter exposes the fossil): times out on the old sweep, passes now.

Refs #2641

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EPCWramjFYquX5SaMdeJKW

* fix(#2641): the sweep summary fires on every terminal outcome, and a faulting lane ends only itself

Review finding: the end-of-sweep summary was emitted only on OnCompleted, so
a lane whose read faulted ended the sweep with a warning and no summary —
the exact silence the summary exists to end — and, because the lanes are
concatenated, skipped every lane after it. Each lane now isolates its own
read fault (logged, empty, next lane runs), mirroring the per-row tolerance,
and the summary is emitted on both completion and fault with the outcome
named.

Refs #2641

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EPCWramjFYquX5SaMdeJKW

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@rbuergi
rbuergi deleted the fix/2425-useridentitycache-type-node branch September 13, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants