Repository navigation
fix(#4126): a module's native payloads are derived by BOTH closures and land where the loader probes - #4366
Conversation
…nd land where the loader probes A registry module bundle could CARRY a RID-specific native payload since #4239 and nothing ever wrote one to disk: carried, declared, ignored. The consumer's landing took assemblies and a wwwroot tree and nothing else, so a module that declares an engine installed cleanly, loaded, and threw DllNotFoundException at its first P/Invoke — with nothing in any log connecting the two. The other closure derivation, the container lane, could not even DECLARE such a package. Measured first, on a real SQLitePCLRaw.lib.e_sqlite3 publish rather than from the issue's description (2026-09-15): * a PORTABLE publish declares 29 `runtimeTargets` entries with `assetType: "native"` and keeps a `runtimes/` tree on disk; * `-r linux-x64` — what every MeshWeaver image is — resolves them into a ONE-entry `native` section whose KEY is still runtimes/linux-x64/native/libe_sqlite3.so while the FILE is flattened beside the app. `ContainerReferenceSet` read neither, and `Resolve` matched a package by the ASSEMBLY it contributes, so a native-only package answered Supplied=false against an image that ships the engine. LANDING. LandModule/ShelveModule take `nativeAssets`; the publish endpoint and PluginBundleClient read them off the bundle; GenerationIdOf folds them into the content address (two bundles differing only in an engine must address two generations, or the second adopts the first's); the layout is validated by ONE predicate — NuGetPackageWriter.IsModuleNativeLayout — at the derivation, the packer, the bundle reader, ModulePublish (a named 400) and the landing itself, before a byte touches disk. A payload at any other shape is refused, never landed. RE-SERVE. ModuleBundleSource.NativeAssetsOf reads the shelf's tree (an ADDITIVE call: widening CollectVersion's tuple would rewrite a signature MeshWeaver.Plugins destructures into three), ServedModule.Natives carries it from the shelf and out of a sealed publication, and the download route writes the section back into the bundle it composes. THE CONTAINER DERIVATION. ContainerReferenceSet reads both deps.json shapes through one reader (NativeContributions), Resolve treats a native-only package as SUPPLIED, PrivateClosure carries NativeRides and NAMES a declared payload neither source has, ProjectBuild lays them out and writes module-natives.txt, and the pack lane declares them with --with-native — which takes the opposite argument to --with, deliberately, because the path IS what the loader probes. The gate and publish-bake lanes now compose into /ext/modules/<Name>/: ModuleNativeAssets accepts a module folder only when its PARENT is named `modules`, so the shallower path landed natives that were never probed. Both lanes' real steps are executed by test-module-asset-landing.py, which asserts the bytes, the four segments AND the depth, with a falsification arm per half — all four driven red. Also fixed at the same call site: the publish endpoint never passed `accepted.SourceCommit` to ShelveModule, so #4158's shelf half recorded nothing — the same unwired-argument shape as #2221 and as this issue. Still open, and stated rather than quietly skipped: the pack log's two drop warnings remain warnings (arming the refusal before a clean publish wave has been measured takes the fleet's publishes down instead of the drops — the #3240 shape), the container lane carries ONE rid's engine because it resolves against ONE image, and the module-libraries shelf's native half is built but unexercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test Results (shard 0)262 tests 262 ✅ 2m 25s ⏱️ Results for commit c6e731e. ♻️ This comment has been updated with latest results. |
Test Results (shard 1)1 476 tests 1 476 ✅ 2m 44s ⏱️ Results for commit c6e731e. ♻️ This comment has been updated with latest results. |
Test Results (shard 2)915 tests 915 ✅ 3m 18s ⏱️ Results for commit c6e731e. ♻️ This comment has been updated with latest results. |
Test Results (shard 4)1 871 tests 1 679 ✅ 4m 52s ⏱️ Results for commit c6e731e. ♻️ This comment has been updated with latest results. |
Test Results (shard 5) 6 files 6 suites 5m 23s ⏱️ Results for commit c6e731e. ♻️ This comment has been updated with latest results. |
Test Results (shard 3)1 111 tests 1 109 ✅ 6m 20s ⏱️ Results for commit c6e731e. ♻️ This comment has been updated with latest results. |
Test Results 18 files 18 suites 25m 3s ⏱️ Results for commit c6e731e. ♻️ This comment has been updated with latest results. |
CI is red on a dated tolerance that expired at 00:00Z today — not reachable from this diff
That is Measured, in a full checkout, with 🚨 My first reading of this was WRONG and worth recording: I ran the harness against a minimal tree holding only the five files it names, saw the same failure, and concluded "pre-existing". That tree was missing Holding for #4369. Once it lands this PR takes main (a re-run cannot green it — Everything else on this run is green, and the change set's own evidence is in the description: 68 assemblies at 🤖 Generated with Claude Code |
There was a problem hiding this comment.
🟡 Changes recommended
Critical compatibility findings and unresolved native validation and merge-conflict issues block approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds end-to-end native runtime asset support for MeshWeaver modules, from dependency derivation through packaging, registry transport, landing, and CI validation.
Changes:
- Derives and packages RID-specific native assets.
- Preserves and lands natives at loader-probed paths.
- Extends workflows, tests, and documentation.
File summaries
| File | Reviewed change | Final review findings |
|---|---|---|
tools/MeshWeaver.PluginTester/ProjectBuild.cs |
Emits native closure manifests and provenance. | — |
tools/MeshWeaver.PluginTester/PrivateClosure.cs |
Resolves native rides and tracks missing payloads. | Moderate (1 vote): remove paths from NativesMissing when the shelf resolves them.Moderate (2 votes): avoid false managed missing warnings for native-only packages. |
tools/MeshWeaver.PluginTester/NativeContributions.cs |
Parses native deps.json contributions. |
Moderate (1 vote): preserve or report rejected declarations instead of silently dropping invalid native paths. |
tools/MeshWeaver.PluginTester/ModuleLibrariesShelf.cs |
Indexes shelf native metadata. | Moderate (3 votes): include native-only package IDs in shelf presence and resolution. |
tools/MeshWeaver.PluginTester/ContainerReferenceSet.cs |
Resolves container-supplied native packages. | Moderate (1 vote): apply the native-only supplied exception only when no managed assemblies are contributed. |
test/MeshWeaver.PluginTester.Test/NativeClosureTest.cs |
Tests native closure behavior. | — |
test/MeshWeaver.Graph.Test/ModulePackCommandTest.cs |
Tests native bundle packing. | — |
test/Memex.Portal.Shared.Test/ServedModuleBytesTest.cs |
Tests native re-serving. | Nit (2 votes): replace shared mutable static byte arrays with fresh or instance-owned data. |
test/Memex.Portal.Shared.Test/PluginBundlePublishNativesTest.cs |
Tests native publication and landing. | Nit (2 votes): replace shared mutable static byte arrays with fresh or instance-owned data. |
src/MeshWeaver.PluginCatalog/ServedModuleBytes.cs |
Re-serves shelf and sealed native payloads. | Moderate (2 votes): validate sealed archive entries against manifest native declarations. Nit (2 votes): preserve the previous Resolve CLR overload. |
src/MeshWeaver.PluginCatalog/PluginBundleClient.cs |
Adopts native assets from bundles. | Moderate (3 votes): reject declared-but-missing native ZIP entries instead of treating them as absent. |
src/MeshWeaver.PluginCatalog/ModulePublish.cs |
Validates published native assets. | Moderate (1 vote): reject .a and .lib payloads.Critical (2 votes): preserve the previous Validate CLR overload. |
src/MeshWeaver.PluginCatalog/ModuleLandingService.cs |
Validates, hashes, and lands natives. | Critical (2 votes): preserve the previous LandModule CLR overload.Moderate (1 vote): reject .a and .lib payloads.Moderate (2 votes): reject zero-byte native payloads. |
src/MeshWeaver.PluginCatalog/ModuleBundleSource.cs |
Reads native assets from the shelf. | — |
src/MeshWeaver.Plugin.Build/ModulePackCommand.cs |
Packs derived and explicit natives. | Moderate (3 votes): exclude .a and .lib in the direct --with-native path.Moderate (1 vote): reject zero-byte native payloads. |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-15-a-plugin-can-bring-its-own-engine.md |
Documents the feature. | Nit (2 votes): narrow the refusal claim to behavior actually enforced. |
src/MeshWeaver.Documentation/Data/Architecture/Modules.md |
Updates module architecture guidance. | — |
memex/Memex.Portal.Shared/Api/PluginBundleEndpoints.cs |
Wires native assets through publish and download endpoints. | Moderate (2 votes): reject incomplete manifest-declared native sections. Moderate (2 votes): preserve specific native validation errors instead of returning only the generic bundle error. |
.github/workflows/node-repo-publish-bake.yml |
Composes native assets during publish-bake. | Moderate (1 vote): detect conflicting native paths when duplicate modules are merged. |
.github/workflows/node-repo-module-pack.yml |
Declares native closure payloads. | — |
.github/workflows/node-repo-gate.yml |
Composes native assets in gate builds. | Moderate (1 vote): detect conflicting native paths when duplicate modules are merged. |
.github/workflows/dotnet-test.yml |
Runs native landing verification. | — |
.github/scripts/test-module-asset-landing.py |
Tests native landing paths and depth. | — |
Review details
Suppressed comments (9)
.github/workflows/node-repo-gate.yml:1828
- When two bundles declare the same module, this merge targets the same
$EXT/modules/$namedirectory andcp -Rsilently overwrites any same-path native from the earlier bundle. The existing one-build check only compares managed DLLs, so bundles with identical managed bytes but different native engines can pass and whichever glob order wins; compare/reject conflicting native paths before copying and add a duplicate-module native test.
if [ -d "$u/meshweaver/modulenatives" ]; then
cp -R "$u"/meshweaver/modulenatives/. "$EXT/modules/$name/"
.github/workflows/node-repo-publish-bake.yml:1459
- When two bundles declare the same module, this merge targets the same
$EXT/modules/$namedirectory andcp -Rsilently overwrites any same-path native from the earlier bundle. The existing one-build check only compares managed DLLs, so bundles with identical managed bytes but different native engines can pass and whichever glob order wins; compare/reject conflicting native paths before copying and add a duplicate-module native test.
if [ -d "$u/meshweaver/modulenatives" ]; then
cp -R "$u"/meshweaver/modulenatives/. "$EXT/modules/$name/"
src/MeshWeaver.Plugin.Build/ModulePackCommand.cs:601
File.Existsaccepts a zero-length native file, so the packer can declare and archive an unusable engine; the current landing path also does not reject it (the assembly path has an explicitLength > 0check). Reject empty native payloads here as well and name the file, so invalid bundles fail at the producer boundary instead of reaching first P/Invoke.
if (!File.Exists(nativePath))
{
src/MeshWeaver.PluginCatalog/ModuleLandingService.cs:580
- Appending
nativeAssetsas an optional parameter changes the CLR signature despite preserving source call sites. Existing compiled callers ofShelveModulewill fail withMissingMethodExceptionafter upgrading this assembly; retain the old exact overload and add a separate native-aware overload.
IReadOnlyList<(string RelativePath, byte[] Bytes)>? nativeAssets = null)
src/MeshWeaver.PluginCatalog/ModuleLandingService.cs:627
- The landing-side native validation accepts
.a/.libbecause it only checks the four-segment layout. A caller that supplies such a path can write a link-time library into the module tree even though the loader never probes it; reject these suffixes here as well (ideally by tightening the shared predicate).
if (!NuGetPackageWriter.IsModuleNativeLayout(value))
throw new ArgumentException(
$"Module '{moduleName}': '{value}' is not the layout the module loader probes "
+ "(exactly runtimes/<rid>/native/<file>). Landing it would write bytes to a path "
+ "nothing ever looks at — shipped in appearance, absent in behaviour. A native "
src/MeshWeaver.PluginCatalog/ModulePublish.cs:217
- This new registry-door check accepts
.a/.libbecause the shared four-segment predicate does not exclude link-time libraries. A bundle can therefore publish a non-loadable native payload that the derivations intentionally omit; reject those suffixes here (preferably in the shared predicate) so the bundle cannot claim to carry an engine the loader cannot use.
foreach (var native in nativeAssets ?? [])
if (!NuGetPackageWriter.IsModuleNativeLayout(native.RelativePath))
return (null,
$"'{native.RelativePath}' is not the layout the module loader probes "
+ "(exactly runtimes/<rid>/native/<file>) — a native landed anywhere else is "
+ "shipped in appearance and absent in behaviour");
tools/MeshWeaver.PluginTester/ContainerReferenceSet.cs:432
- This OR condition marks a package as supplied when any native is present, even if the same package also contributes managed assemblies that are absent from the image.
ProjectBuildthen skips the unresolved reference and can produce a module missing its managed runtime; the native-only exception must apply only whenPackageAssemblieshas no entry for the package.
var natives = NativesOf(packageId);
return new PackageResolution(packageId, version, files, !files.IsEmpty || !natives.IsEmpty)
tools/MeshWeaver.PluginTester/NativeContributions.cs:81
- The shared container/shelf parser silently discards native declarations outside the four-segment layout (and
.a/.lib), while the SDK derivation reports analogous drops. A package that also contributes a managed assembly can therefore build and produce no diagnostic even though its declared native will not ride the bundle. Preserve/report rejected declarations so this path names the drop instead of treating it as “no native.”
void Consider(string key)
{
var relative = key.Replace('\\', '/');
if (relative.EndsWith(".a", StringComparison.OrdinalIgnoreCase)
|| relative.EndsWith(".lib", StringComparison.OrdinalIgnoreCase))
return;
if (NuGetPackageWriter.IsModuleNativeLayout(relative))
found.Add(relative);
tools/MeshWeaver.PluginTester/PrivateClosure.cs:174
- If a declared path is missing in the container but is later found on the shelf, it has already been added to
NativesMissingand is never removed. The build therefore emits a false missing-native warning even though aNativeRidewas selected; remove the path when either source resolves it.
if (container.NativeFileFor(declared) is { } fromImage)
natives[declared] = new NativeRide(declared, fromImage, id, "the image");
else if (shelf?.NativeFileFor(declared) is { } fromShelf)
natives[declared] = new NativeRide(declared, fromShelf, id, "the shelf");
else
nativesMissing.Add(declared);
- Files reviewed: 23/23 changed files
- Comments generated: 14
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| IReadOnlyList<(string RelativePath, byte[] Bytes)>? staticAssets = null, | ||
| string? sourceCommit = null) | ||
| string? sourceCommit = null, | ||
| IReadOnlyList<(string RelativePath, byte[] Bytes)>? nativeAssets = null) |
| string? packagePath = null, | ||
| IReadOnlyList<BundleReader.ModuleAsset>? staticAssets = null) | ||
| IReadOnlyList<BundleReader.ModuleAsset>? staticAssets = null, | ||
| IReadOnlyList<BundleReader.ModuleAsset>? nativeAssets = null) |
| assets = BundleReader.ReadModuleAssets(bytes); | ||
| // Same guard, same reason (#4126): a declared native at a layout the loader | ||
| // never probes makes ReadModuleNativeAssets throw, and that is an unreadable | ||
| // UPLOAD — a 400 naming it, never a 500 telling the publisher the server broke. | ||
| natives = BundleReader.ReadModuleNativeAssets(bytes); |
| // Same guard, same reason (#4126): a declared native at a layout the loader | ||
| // never probes makes ReadModuleNativeAssets throw, and that is an unreadable | ||
| // UPLOAD — a 400 naming it, never a 500 telling the publisher the server broke. | ||
| natives = BundleReader.ReadModuleNativeAssets(bytes); |
| // PLATFORM-carried packages that the derivation deliberately excludes. | ||
| foreach (var native in extraNatives) | ||
| { | ||
| if (!NuGetPackageWriter.IsModuleNativeLayout(native)) |
| foreach (var declared in container.DeclaredNativesOf(id) | ||
| .Concat(shelf?.DeclaredNativesOf(id) ?? []) | ||
| .Distinct(StringComparer.Ordinal)) |
| - **A plugin that cannot deliver one is refused when it is published**, with a message naming the | ||
| problem, rather than installed and left to fail later on your machine. The one failure this could |
| string? identity, | ||
| ILogger? logger = null) | ||
| ILogger? logger = null, | ||
| IReadOnlyList<(string RelativePath, string FullPath)>? shelfNatives = null) |
| private static readonly byte[] NativeBytes = Encoding.UTF8.GetBytes("ELF-linux-x64-engine"); | ||
| private static readonly byte[] SecondNativeBytes = Encoding.UTF8.GetBytes("MACHO-osx-arm64-engine"); |
| Assert.Equal(NativeBytes, ReadAsset(Assert.Single(served.Natives))); | ||
| } | ||
|
|
||
| private static readonly byte[] NativeBytes = Encoding.UTF8.GetBytes("ELF-linux-x64-engine"); |
CI failed on something this diff cannot reach: node-repo-publish-bake.yml's LEGACY_PUBLISHER_RED_FROM tolerance expired at 2026-09-15T00:00:00Z, so test-module-set-consistency.py's legacy-publisher case read the clock and every run in the repo created after midnight failed the same step. #4369 makes that case read the BRANCH instead; taking main is what picks it up, because a re-run reads the previous attempt's artefact (#4303). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A module's native payloads are derived by BOTH closures, and they land where the loader probes
Closes #4126.
Since #4239 a registry bundle could carry a RID-specific native payload, and nothing ever
wrote one to disk: carried, declared, ignored. The consumer's landing took the assemblies and
the
wwwroottree and nothing else — so a module that declares an engine published 200, appearedin the catalog, installed, loaded, rendered, and threw
DllNotFoundExceptionat its firstP/Invoke, with nothing in any log connecting the two. The other closure derivation, the
container lane, could not even DECLARE such a package.
Root cause, measured before it was fixed
Not taken from the issue's description — re-derived from a real
SQLitePCLRaw.lib.e_sqlite3publish (2026-09-15), because the two deps.json shapes are the whole container half:
runtimeTargetsentries,assetType: "native"runtimes/<rid>/native/tree, preserved-r linux-x64(what every MeshWeaver image is)nativesection, key stillruntimes/linux-x64/native/libe_sqlite3.solibe_sqlite3.so, flattened beside the appContainerReferenceSetread neither — only theruntimesection — andResolvematched apackage by the ASSEMBLY it contributes (falling back to
<id>.dll). A package contributing onlynatives therefore answered
Supplied = falseagainst an image that demonstrably ships its engine,and
ProjectBuildreds an unresolvedPackageReference. That is whyMeshWeaver.AppleMessagescarries a conditional pin and depends on its host.
And the collection ran after the
runtimeshort-circuit in both derivations — the same bug inthe same place the SDK derivation had (#4239's note), skipping exactly the packages it exists for.
What changed
Landing.
LandModule/ShelveModuletakenativeAssets; the publish endpoint andPluginBundleClient.LandFromBundleread them off the bundle;GenerationIdOffolds them into thecontent address — two bundles differing only in an engine must address two generations, or the
second landing adopts the first's directory and serves the wrong engine under the right version.
🚨 The layout is validated by ONE predicate at every door:
NuGetPackageWriter.IsModuleNativeLayout— the derivation, the packer, the bundle reader,
ModulePublish.Validate(a named 400, so apublisher is told rather than the server blamed) and
ModuleLandingServiceitself, before a bytetouches disk. A payload at any other shape is REFUSED, never landed: it would be bytes that read
as shipped and behave as absent, which is the one outcome the section exists to prevent.
Re-serve.
ModuleBundleSource.NativeAssetsOfreads the shelf's tree — an additive call,not a fourth element on
CollectVersion's tuple, becauseMeshWeaver.Pluginsdestructures thatinto three and widening it would red a consumer's trunk for a capability it does not use.
ServedModule.Natives(aninitproperty —check-record-signatures.py) carries it from theshelf AND out of a sealed publication, and the download route writes the section back into the
bundle it composes.
The container derivation.
ContainerReferenceSetreads both deps.json shapes through onereader (
NativeContributions, shared with the shelf so the two cannot drift),Resolvetreats anative-only package as SUPPLIED,
PrivateClosurecarriesNativeRides and NAMES a declaredpayload neither source has,
ProjectBuildlays them out under the pack input and writesmodule-natives.txt, and the pack lane declares them with--with-native.🚨
--with-nativetakes the opposite argument to--with, deliberately:--withrefuses apath component because the flat closure has no place for one; this requires the path, because the
path is what the loader probes. Two flags and two builder manifests, because a lane feeding one to
the other would be wrong whichever way it was written. The pack lane refuses a native with no
builder provenance, exactly as it does a non-MeshWeaver assembly.
🚨 The gate and publish-bake lanes compose into
/ext/modules/<Name>/, one level deeper.ModuleNativeAssetsaccepts a directory as a module folder only when its PARENT is namedmodules, so at the shallower path a module's natives would land on disk and never be probed.Also fixed at a call site this change already touched: the publish endpoint never passed
accepted.SourceCommittoShelveModule, so #4158's shelf half recorded nothing — the sameunwired-argument shape as #2221 and as this issue itself.
Evidence
Every positive assertion was driven RED against the pre-fix code, by mutating the real thing:
ContainerReferenceSetreverted to reading onlyruntimeNativeClosureTestcases red, incl.ANativeOnlyPackageIsSUPPLIED…PluginBundlePublishNativesTestcases redtest-module-asset-landing.pyrevert arm)cpremoved, the in-lane check keptPlus anti-vacuity controls that pass and must: a package whose payload is DECLARED but absent is
Supplied = false(so the verdict is about the image, not the declaration); a module with noengine grows no
runtimes/tree; a bundle with no natives declares no section at all;.a/.lib,runtimes/<rid>/nativeassets/<tfm>/…andassetType: "runtime"are excluded and named.test-module-asset-landing.pynow runs 7 cases per lane and EXECUTES both lanes' real steps,asserting the bytes, the exact four segments and the depth.
dotnet build MeshWeaver.slnx -c Release -warnaserror→ 68 assemblies, 0 Warning(s), 0 Error(s)NativeClosureTest8/8 ·PluginBundlePublishNativesTest6/6 ·ModulePackCommandTest32/32 ·the module suites of
Memex.Portal.Shared.Test76/76 ·PrivateClosureTest/ContainerReferenceSetTest/ModuleLibrariesShelfTestgreen · doc link integrity greentest-module-asset-landing.pyPASS (2 lanes × 7 cases, all four falsification arms red)check-record-signatures.py✓ (16 changed C# files, no binary-breaking primary constructors) ·check-type-forwards.py0 removed, 1 added (ServedModule.Natives, an init property) ·check-interface-addition.py0 obliging members ·check-workflow-timeouts.py✓test-module-set-consistency.pyreports one failure onorigin/maintoo, byte-identical (apre-#3732-publisher case) — this diff cannot reach it.
What is deliberately NOT done
arming the refusal before a full publish wave has been measured clean takes the fleet's
publishes down instead of the drops — the Arm the registry's own 400: ModulePublish.Validate still accepts a manifest that states no framework identity #3240 shape, which was armed on two measurements
rather than on faith. Filed as A dropped native payload should REFUSE the pack, not warn — arm it on a measured publish wave (#4126 stage 5) #4367, with the measurement it needs stated.
on another rid falls back to the runtime's own probing — strictly better than the nothing it
carried before, and stated in the builder's log rather than left to be deduced.
DeclaredNativesOf/NativeFileFor)and unexercised: no curated package declares one yet.
Conserved on
Doc/Architecture/Modules→ "LANDED, RE-SERVED, and derived by BOTH lanes",replacing the stage list the previous pass wrote there, plus a What's New entry.
Pairs-with: none — nothing is removed or renamed; every public change is additive (two optional trailing parameters, one additive static, two init properties), and the sibling repo's existing call sites compile unchanged.🤖 Generated with Claude Code