Skip to content

NuGet packages bind on every load path: the build records its #r closure - #6862

Merged
rbuergi merged 3 commits into
mainfrom
fix/nuget-load-path-probe
Oct 16, 2026
Merged

rbuergi merged 3 commits into
mainfrom
fix/nuget-load-path-probe

Conversation

@rbuergi

@rbuergi rbuergi commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Problem

#6834 fixed the compile of a NodeType declaring #r "nuget:…" (host wins). On an enterprise client portal at platform 3.1.10992 (contains cab0ae9), measured 2026-10-16, a NodeType declaring #r "nuget: Microsoft.Data.SqlClient, 6.1.1" compiled Ok, but every load of its cached build failed on both replicas, dozens of times:

Failed to extract NodeTypeConfigurations from /data/assembly-cache/<type>/v33-….dll: Could not load file or assembly 'Microsoft.Data.SqlClient, Version=6.0.0.0, …'
  ReflectionTypeLoadException at RuntimeModule.GetTypes() at MeshNodeCompilationService.CompileResultFromAssembly(…)
DynamicContentTypeRegistrationHostedService: 1 baked dynamic NodeType(s) could NOT be registered … (Faulted: …)
EnrichWithNodeType: the recorded build … still does not load in this process after 1 recompile attempt(s)

Root cause

The package probing directories reached the NodeAssemblyLoadContext only through ICompilationCacheService.RegisterProbingDirectories(nodeName, …), which only the emit called, and only in memory. Any other reader of the same bytes had an empty registry: a replica that never compiled the type, the compiling replica after a restart, an adopted CI bake, the start-up registration pass, EnrichWithNodeType, and a compile attempt that hit an existing build instead of emitting. Those readers loaded the build with nothing to probe. Load fell through to the default context, which does not ship SqlClient, so GetTypes() threw. Nothing about the build recorded which packages it needed, so no reader could recover.

Fix: the build carries its package closure

  1. NuGetBuildRecord (MeshWeaver.Compiler). Both producers, the runtime PrepareGeneratedSource and the bake's NodeSetCompiler.ResolveInputs, stamp one [assembly: System.Reflection.AssemblyMetadata("MeshWeaver.NuGetPackage", "Id, Version")] per reference into the generated skeleton, ahead of its own assembly attribute.

    • The record is in the bytes, so it travels wherever a build travels (store, blob, bundle).
    • Package paths are not recorded, because another replica does not share the compiling process's package folder.
    • The stamp is part of the generated text, so the content key covers it. A pre-stamp build of the same sources gets a different key and is rebuilt, not adopted.
    • A NodeType with no #r nuget gets a byte-identical source and keeps its key, so nothing in the fleet is rebuilt.
  2. BuildPackageProbing (Compiler.Pipeline). Before every load of an already-built assembly, it reads the record from the file's metadata (a header read; nothing is loaded). It re-resolves the record through INuGetAssemblyResolver, which is memoized and served from a warm package folder without the network. It then registers the probing directories for that build path via the new RegisterBuildProbingDirectories, which takes precedence over the node-wide registration. It is wired into:

    • the post-compile scan in CompileAndGetConfigurations (this covers the cache-hit attempt);
    • GetConfigurationsFromExistingAssembly, the funnel for start-up registration, EnrichWithNodeType, the data-model and contract areas and adopted builds. It stays eager, as callers expect, when nothing is recorded;
    • the cell surface.

    After that the load context binds exactly as on the compiling process: platform first, then host unification, then the package's runtimes/<rid>/lib asset. The In-mesh #r nuget compiles: host wins, no stub in the default context, ref-assembly compile surface #6834 host-wins rule is unchanged.

  3. If a resolution fails or times out (AssemblyLoadTimeout), a WARNING names the packages and the cause, and the load proceeds and fails with the loader's own message.

Tests

  • NuGetPackagesLoadOnAFreshReplicaTest (live feed; skips when nuget.org is unreachable, like NuGetPackageHostWinsTest). A SqlClient NodeType whose type derives from a package type is built twice: once by the runtime compile and once by the bake. Each build is copied to a store-shaped path the process never loaded and loaded through a fresh CompilationCacheService + MeshNodeCompilationService, i.e. an empty probing registry and resolver memo, the state of another replica or a restart. The configuration scan must succeed, SqlConnection must be constructed, and SqlClient must be the runtimes/ asset. Both cases were red before the fix, with the production message verbatim.
  • NuGetBuildRecordTest (offline): no package ⇒ byte-identical source; each reference round-trips through an emitted image; nothing recorded ⇒ empty.
  • Local: MeshWeaver.Compiler.Pipeline.Test 1506/1506 passed; MeshWeaver.Graph.Test NuGet filter 24/24. Touched projects and their dependents build with -c Release -warnaserror, 0 warnings.

Deploy note

After the roll, an affected NodeType's recorded build predates the stamp, so its content key no longer matches and the next compile produces a stamped build. Recycle the affected NodeType (for example via the recycle verb) if its address still serves the assembly-unavailable diagnosis.

Pairs-with: none — no public type or member removed; ICompilationCacheService is internal.

Doc: Doc/Architecture/NuGetRuntimeAssets → "Every load path: the build carries its package closure".

🤖 Generated with Claude Code

A NodeType whose sources declare `#r "nuget:…"` compiled (host wins, #6834), but its
cached/baked build could not be LOADED anywhere except the process that had just emitted it:
the package probing directories were registered in memory by the emit alone, so a replica that
never compiled the type, a restarted process, an adopted bake, the start-up registration pass,
EnrichWithNodeType and a compile attempt that hit an existing build all loaded the assembly with
nothing to probe, and GetTypes() failed with
"Could not load file or assembly 'Microsoft.Data.SqlClient, Version=6.0.0.0'".

- NuGetBuildRecord: both producers (runtime PrepareGeneratedSource, bake ResolveInputs) stamp one
  [assembly: AssemblyMetadata("MeshWeaver.NuGetPackage", "Id, Version")] per reference into the
  generated skeleton, so the record travels with the bytes. Part of the generated text, so the
  content key covers it (pre-stamp builds are rebuilt, not adopted); a NodeType with no #r nuget
  keeps a byte-identical source and key.
- BuildPackageProbing: before every load of an already-built assembly (post-compile scan,
  GetConfigurationsFromExistingAssembly, cell surface) the recorded references are re-resolved
  through INuGetAssemblyResolver (memoized) and registered per build path
  (RegisterBuildProbingDirectories), ahead of the node-wide registration. The load context then
  binds as before: platform, host unification, runtimes/<rid>/lib asset.
- NuGetPackagesLoadOnAFreshReplicaTest: runtime build and bake each loaded through a FRESH
  cache + compilation service from a path never loaded; red before with the production message.
- Doc/Architecture/NuGetRuntimeAssets: "Every load path: the build carries its package closure".

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 09:51
@rbuergi
rbuergi enabled auto-merge October 16, 2026 09:51

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.

🟡 Changes recommended

Unresolved pool cancellation, activation dependencies, offline resolution, probing precedence, and synchronous metadata reads affect loading reliability.

5 open findings
What changed in this PR

Extends the earlier NuGet compile fix so cached NodeType builds carry package references that other replicas can resolve before loading.

Changes:

  • Records package references in runtime-compiled and baked assemblies.
  • Resolves build-specific probing directories before assembly loading.
  • Adds fresh-replica tests and documents the loading behavior.
File Description
test/​MeshWeaver.Compiler.Pipeline.Test/​NuGetPackagesLoadOnAFreshReplicaTest.cs Adds replica-loading and metadata round-trip tests.
test/​MeshWeaver.Compiler.Pipeline.Test/​NuGetPackageHostWinsTest.cs Shares the NuGet availability helper.
src/​MeshWeaver.Documentation/​Data/​Architecture/​NuGetRuntimeAssets.md Documents recorded references and loading paths.
src/​MeshWeaver.Compiler/​NuGetBuildRecord.cs Writes and reads package metadata.
src/​MeshWeaver.Compiler/​NodeSetCompiler.cs Stamps references into baked builds.
src/​MeshWeaver.Compiler.Pipeline/​MeshNodeCompilationService.cs Stamps runtime builds and prepares package probing.
src/​MeshWeaver.Compiler.Pipeline/​CompilationCacheService.cs Adds build-specific probing registration.
src/​MeshWeaver.Compiler.Pipeline/​CellSurfaceAssemblyProvider.cs Prepares package probing for cell surfaces.
src/​MeshWeaver.Compiler.Pipeline/​BuildPackageProbing.cs Resolves recorded packages before loading.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/MeshWeaver.Compiler.Pipeline/BuildPackageProbing.cs Outdated
Comment thread src/MeshWeaver.Compiler.Pipeline/BuildPackageProbing.cs Outdated
Comment thread src/MeshWeaver.Compiler.Pipeline/MeshNodeCompilationService.cs Outdated
Comment thread src/MeshWeaver.Compiler.Pipeline/CompilationCacheService.cs Outdated
Comment thread src/MeshWeaver.Compiler.Pipeline/MeshNodeCompilationService.cs Outdated
…uild closure wins

Review of #6862:
- the recorded-reference read (file IO) runs on a blocking pool, never after
  DiscoverSourceVersionSnapshot on the signalling thread (also in the cell surface);
- resolution runs on the HTTP lane through the pool's cold Invoke, so the bound cancels the wait
  and frees the slot, and activation never queues behind the Compile lane a kernel script holds;
- a build's recorded closure replaces directories a context got from the node-wide fallback
  (another generation's) and is never displaced by it — BuildProbingPrecedenceTest.
Offline-first resolution from a warm package folder is #6863.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    15 files   -    11      15 suites   - 11   46m 17s ⏱️ - 10m 50s
11 228 tests  - 1 608  11 228 ✅  - 1 415  0 💤  - 193  0 ❌ ±0 
11 244 runs   - 1 608  11 244 ✅  - 1 415  0 💤  - 193  0 ❌ ±0 

Results for commit 802cb9f. ± Comparison against base commit b27d874.

This pull request removes 1649 and adds 15 tests. Note that renamed tests count towards both.

   --- End of inner exception stack trace ---
   --- End of inner exception stack trace ---)
   --- End of inner exception stack trace ---, expected: True)
   --- End of inner exception stack trace ---, isDenial: True)
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
 ---> (Inner Exception #1) MeshWeaver.Messaging.Hub.Test.InfrastructureFaultTest+ProviderException (0x80004005): Failed to connect to 10.42.18.4:5432<---
 ---> (Inner Exception #1) System.ArgumentException: Value does not fall within the expected range.<---
 ---> (Inner Exception #1) System.InvalidOperationException: boom<---
 ---> (Inner Exception #1) System.InvalidOperationException: source B is misconfigured<---
…
Memex.Portal.Shared.Test.InstanceIdRulesMatchTheRegistryTest ‑ TheSetupHostAgreesWithTheRegistry(candidate: "e4b12105-22eb-44a9-b437-60cfc83342a2")
Memex.Portal.Shared.Test.PublishStorageFaultIsNotAConflictTest ‑ AStorageFault_IsA500_NamingTheError(fault: System.AggregateException: One or more errors occurred. (Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.)
 ---> System.UnauthorizedAccessException: Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.
   --- End of inner exception stack trace ---)
Memex.Portal.Shared.Test.PublishStorageFaultIsNotAConflictTest ‑ AStorageFault_IsA500_NamingTheError(fault: System.InvalidOperationException: landing faulted
 ---> System.UnauthorizedAccessException: Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.
   --- End of inner exception stack trace ---)
Memex.Portal.Shared.Test.SessionDenialIsAnAnswerTest ‑ OnlyAVerdictReadsAsADenial(shape: "the same verdict nested, as a late denial dispatch"···, failure: System.InvalidOperationException: write failed
 ---> System.UnauthorizedAccessException: Access denied
   --- End of inner exception stack trace ---, isDenial: True)
MeshWeaver.Compiler.Pipeline.Test.BuildProbingPrecedenceTest ‑ TheBuildsRecordedClosure_ReplacesTheNodeWideFallback
MeshWeaver.Compiler.Pipeline.Test.NuGetBuildRecordTest ‑ ASourceWithoutPackages_IsUnchanged
MeshWeaver.Compiler.Pipeline.Test.NuGetBuildRecordTest ‑ EachReference_IsRecordedInTheImage
MeshWeaver.Compiler.Pipeline.Test.NuGetBuildRecordTest ‑ NothingRecorded_ReadsEmpty
MeshWeaver.Compiler.Pipeline.Test.NuGetPackagesLoadOnAFreshReplicaTest ‑ ABakedBuild_LoadsOnAReplicaThatNeverCompiledIt
MeshWeaver.Compiler.Pipeline.Test.NuGetPackagesLoadOnAFreshReplicaTest ‑ ARuntimeBuild_LoadsOnAReplicaThatNeverCompiledIt
…

♻️ This comment has been updated with latest results.

Copilot AI balanced review requested due to automatic review settings October 16, 2026 10:46
@meshweaver-cloud
meshweaver-cloud Bot disabled auto-merge October 16, 2026 10:46

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.

public void RegisterBuildProbingDirectories(string dllPath, System.Collections.Generic.IReadOnlyList<string> directories)
{
if (string.IsNullOrEmpty(dllPath) || directories.Count == 0) return;
_buildProbingDirs[dllPath] = directories.ToImmutableArray();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. The entry outlives the context it was meant for. Fixed in the stacked follow-up #6864: the registration becomes a hand-off, not a registry. The context that takes an entry removes it, in both the factory and ApplyBuildProbing, and every load path re-arms right before it loads, so a context re-created after an eviction is handed it again. That covers aliases (ApplyBuildProbing runs on the aliased context too) and a registration racing a load (whichever context takes the entry first wins; a later load re-arms). BuildProbingPrecedenceTest there pins it. I split it out because this head is fully green, including the dependent Plugins suites. The growth is one small entry per historical build path, which bounds it to the builds this process loaded, so it should not hold back the load fix.

@rbuergi
rbuergi enabled auto-merge October 16, 2026 11:41
@rbuergi
rbuergi merged commit 4771812 into main Oct 16, 2026
62 checks passed
rbuergi added a commit that referenced this pull request Oct 16, 2026
…emoves it (#6864)

* Build probing registration is a hand-off: the context that takes it removes it

Review of #6862: _buildProbingDirs kept one entry per historical build path for the mesh's
lifetime. The entry now lives only until the context for its path takes it (factory or
ApplyBuildProbing remove it); every load path re-arms right before loading, so a context
re-created after an eviction is handed it again. BuildProbingPrecedenceTest pins both.

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

* Build probing registration is read, never consumed; bounded by the builds on disk

Copilot on #6864: consuming the per-build registration on hand-off lost it in two orderings —
an eviction between arming and the queued load (the replacement context is created unarmed),
and a GetOrAdd factory that takes the entry and then loses the race to a concurrent caller's
unconfigured context. The factory and ApplyBuildProbing now READ the entry, ApplyBuildProbing
runs on whichever context GetOrAdd returns, and the map is bounded instead by pruning entries
whose build file is gone on each registration. BuildProbingPrecedenceTest forces both orderings
(both fail on the hand-off code) and pins the bound.

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

* Build probing registry: amortized stale-entry sweep on the file-system pool

Copilot on #6864: the per-registration File.Exists scan was O(N) on every package-bearing load
(N(N-1)/2 to register N builds) and ran in Arm's Do callback, outside the file-system IIoPool.
Build files are deleted mostly outside this process (the store's retention sweep on a shared
volume, from any replica), so no in-process deletion hook sees them; the bound is now an
amortized sweep on the file-system pool, run only when the map has doubled since the last one
(a re-arm never sweeps). EntriesWhoseBuildFileIsDeleted_AreSweptOnceTheMapHasGrown proves the
removal on deletion and fails without the sweep.

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

* Build probing sweep removes only the registration it checked

Copilot on #6864: the store can republish the same path, so a re-arm landing between the
sweep's File.Exists and its key-only TryRemove was dropped, and a queued load lost its probing
directories. Each registration is now its own reference-typed value, captured before the file
check and removed with TryRemove(KeyValuePair) — only that exact entry.
ARearmBetweenTheSweepsCheckAndItsRemoval_Survives forces the ordering through an internal seam
and fails with key-only removal.

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

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants