Repository navigation
fix(cli): modules come FROM the upstream seed — identity-matched, never the latest index - #2851
Merged
Merged
Conversation
…er the latest index Round 4 (Manufacturing#37): the closest yet — the seeded upstream installed, [PASS] Manufacturing (158 nodes, 15 types), every Tests area green — and the adoption postcondition failed: 10 upstream types DECLINED with 'MeshWeaver.AI' built against mvid:699963b6…, live is mvid:813bec7e… because I composed module DLLs from the package index (LATEST: AI@1.2.8) while the sealed publication's types were built against the SEALED module mvids. The reusable gate never has this skew: with an upstream seed, compose-sealed-modules takes the modules FROM the publication. Same here now: - the framework identity is asked of the IMAGE up front (--print-framework-identity — the gate's own pre-bake step), so the seed can be fetched BEFORE stage 1; - with an upstream seed, --registry-modules resolves from the SEED's own bundles (manifest plugin/module.assemblyName, filename fallback) — identity-matched by construction; - a requested package absent from the seed is a REFUSAL naming it, never a silent skip — a module composed from anywhere else would carry the wrong mvid and every publication type built against it would be declined; - only a build with NO upstream seed falls back to the index's latest. Verified locally: build clean -warnaserror; the up-front refusal still exits 6. The compose path's real proof is the next Manufacturing run against the live registry — stated rather than simulated, because a synthetic seed exercising a private method through a scaffold would prove the scaffold. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates BuildPluginCommand in the CLI so that, when --upstream-seed is provided, external module DLLs are composed from the upstream seed’s sealed publication bundles (identity-matched) rather than from the registry index’s “latest” packages, preventing MVID mismatches that cause upstream NodeTypes to be declined at install time.
Changes:
- Move upstream seed fetching earlier and derive the framework identity from the tester image (
--print-framework-identity) before stage 1. - When
--upstream-seedis present, resolve--registry-modulesby extracting module DLLs from the seed bundles (ComposeModulesFromSeed) instead of downloading from the registry index. - Refuse (fail) if a requested module package is not present in the upstream seed, instead of silently falling back.
Suppressed comments (3)
src/MeshWeaver.Cli/BuildPluginCommand.cs:103
- Newly introduced
awaitinsrc/for error output conflicts with the repo’s ban on adding async/await undersrc/(deadlock / identity-loss risk if reused in turn-based schedulers). Prefer a non-async execution model for this command path, or push the IO behindIIoPooland keep the public surface reactive.
await error.WriteLineAsync(
$"error: could not resolve a framework identity from the image (got: '{line.Trim()}') "
+ "— cannot address an upstream publication.");
src/MeshWeaver.Cli/BuildPluginCommand.cs:106
- New
awaitadded undersrc/(writing resolved framework identity). Per repo rules this should not introduce additional async boundaries; refactor to a reactive/synchronous boundary to avoid scheduler parking and AsyncLocal identity loss when code is reused in hub-reachable contexts.
await output.WriteLineAsync($"framework identity: {frameworkIdentity} (from the image)");
src/MeshWeaver.Cli/BuildPluginCommand.cs:109
- Newly introduced
await FetchUpstreamSeed(...)undersrc/adds another async boundary. The repo’s guidance is to avoid Task/await in production code and compose reactively instead (or isolate async IO behindIIoPool).
var seeded = await FetchUpstreamSeed(
options.RegistryUrl!, options.RegistryKey!, upstreams, frameworkIdentity, seedDir, ct);
if (!seeded) return 8;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+92
to
+94
| var line = await Capture("docker", | ||
| ["run", "--rm", "--init", "--entrypoint", "/app/mw-plugin-test", pinned, | ||
| "--print-framework-identity"], ct); |
Comment on lines
+362
to
+365
| var unpack = Path.Combine(extDir, $"unpack-{plugin}"); | ||
| if (Directory.Exists(unpack)) Directory.Delete(unpack, true); | ||
| System.IO.Compression.ZipFile.ExtractToDirectory(bundle, unpack); | ||
| var modulesDir = Path.Combine(unpack, "meshweaver", "modules"); |
Comment on lines
+365
to
+372
| var modulesDir = Path.Combine(unpack, "meshweaver", "modules"); | ||
| if (name is not { Length: > 0 }) | ||
| { | ||
| var dlls = Directory.Exists(modulesDir) ? Directory.GetFiles(modulesDir, "*.dll") : []; | ||
| if (dlls.Length != 1) { Directory.Delete(unpack, true); continue; } | ||
| name = Path.GetFileNameWithoutExtension(dlls[0]); | ||
| } | ||
| if (!File.Exists(Path.Combine(modulesDir, $"{name}.dll"))) { Directory.Delete(unpack, true); continue; } |
Contributor
rbuergi
enabled auto-merge
August 30, 2026 22:31
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
rbuergi
added this pull request to the merge queue
Aug 30, 2026
rbuergi
removed this pull request from the merge queue due to a manual request
Aug 30, 2026
rbuergi
added this pull request to the merge queue
Aug 30, 2026
rbuergi
added a commit
that referenced
this pull request
Aug 31, 2026
…I split
This PR was DIRTY, so it was running no CI at all. One conflict, in
src/MeshWeaver.Cli/BuildPluginCommand.cs, and it is a genuine two-sided change
rather than a textual clash:
* this branch EXTRACTED PullImage / Exec / Capture / PullAttempts out of
BuildPluginCommand into the new src/MeshWeaver.Cli/ImageRunner.cs;
* main ADDED ComposeSealedModules + StageModuleBundle to the same file
(#2851, #2858 — modules come from the publication's SEALED MODULE SET).
Git presented both as one 159-line block with an empty "ours" side, so the
obvious resolutions are both wrong:
* "take theirs" reinstates PullImage here AND leaves it in ImageRunner —
a duplicate definition;
* "take ours" drops the whole block, silently DELETING main's
ComposeSealedModules. I did exactly that first, and the build caught it
(CS0103: ComposeSealedModules does not exist) — which is why the build ran
before the push and not after.
Resolved by splitting the block at its real seam: dropped the 39 lines of
relocated PullImage, kept the 120 lines of main's new code.
Two follow-ons the compiler then named:
* main's auto-merged code called Capture() unqualified; Capture is now a
STATIC on ImageRunner, so it is ImageRunner.Capture(...);
* splitting the block orphaned PullImage's `/// <summary>` opener, which
CS1570'd against the next member's doc comment. Removed.
🚨 Verified that neither side was lost, not just that it compiles:
ComposeSealedModules/StageModuleBundle present (5 references)
PullImage defined once — ImageRunner 1, BuildPluginCommand 0
MeshWeaver.Cli / PluginTester / PluginTester.Test -c Release -warnaserror
0 Warning(s) 0 Error(s)
MeshWeaver.PluginTester.Test 188/188
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rbuergi
added a commit
that referenced
this pull request
Aug 31, 2026
…d CLI conflict as #2850 This PR was DIRTY, so it was running no CI at all. Its branch is built on integration/core-batch (#2853), so it carries #2850's ImageRunner extraction and hit the identical conflict in src/MeshWeaver.Cli/BuildPluginCommand.cs — one 159-line block with an empty "ours" side, holding TWO unrelated things: * PullImage, which this history relocated to src/MeshWeaver.Cli/ImageRunner.cs; * main's ComposeSealedModules (#2851, #2858), which is genuinely new. "Take theirs" duplicates PullImage; "take ours" silently deletes main's method. Split at the seam instead: dropped 39 lines (relocated), kept 120 (main's new). Then the same two follow-ons the compiler names — ImageRunner.Capture is static so main's auto-merged call needed qualifying, and the split orphaned PullImage's `/// <summary>` opener (CS1570). 🚨 Verified neither side was lost, not just that it builds: ComposeSealedModules/StageModuleBundle present 5 references PullImage defined once — ImageRunner 1, BuildPluginCommand 0 MeshWeaver.Cli / PluginTester / PluginTester.Test -c Release -warnaserror, 0/0 MeshWeaver.PluginTester.Test 229/229 Note #2853 is now closed — three of its four members landed individually — so this branch's core-batch ancestry is history rather than a live dependency. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rbuergi
added a commit
that referenced
this pull request
Aug 31, 2026
…ompat red was a BASE artefact This PR was DIRTY (no CI at all) and additionally red on "Public surface (binary compatibility)". Both are addressed by merging main. ── the conflict (same as #2850/#2860) ── One 159-line block in src/MeshWeaver.Cli/BuildPluginCommand.cs holding TWO unrelated things: PullImage (relocated to ImageRunner.cs by this lane) and main's new ComposeSealedModules (#2851, #2858). Split at the seam — dropped 39 lines relocated, kept 120 of main's — then qualified ImageRunner.Capture (now static) and removed the `/// <summary>` opener the split orphaned. ── the binary-compat failure was measured against the WRONG BASE ── The gate said: Comparing against merge base ... branch integration/core-batch ✗ tools/MeshWeaver.PluginTester/ProjectFile.cs record Model — arity: primary constructor went from 21 to 22 parameter(s) 🚨 1 binary-breaking record change(s) That is a correct verdict about the wrong comparison. `ProjectFile.cs` DOES NOT EXIST ON MAIN — the Model record is introduced by #2850's lane, and the gate was comparing against integration/core-batch (#2853), where it existed at arity 21. Against main the record is NEW, so there is no prior signature to break. Not suppressed and NOT added to scripts/record-signatures.allow: an allow entry would assert a deliberate break where there is none, and would then go stale the moment #2850 lands (the shape that has blocked unrelated PRs before). Re-basing onto main is the fix; the gate re-runs against main and should now pass on its own. If it does not, the break is real and belongs in the allow file with a reason — but that is not what this data shows. MeshWeaver.PluginTester / .Test -c Release -warnaserror, 0 Warning(s) 0 Error(s) MeshWeaver.PluginTester.Test 198/198 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Round 4 of the Manufacturing adoption was the closest yet — the seeded upstream installed,
[PASS] Manufacturing (158 nodes, 15 types), every Tests area green — and the adoption postcondition caught the last skew:10 upstream types declined, because I composed module DLLs from the package index (latest, AI@1.2.8) while the sealed publication's types were built against the sealed module mvids. The reusable gate never has this skew: with an upstream seed,
compose-sealed-modules.shtakes the modules from the publication, identity-matched.The fix
--print-framework-identity— the gate's own pre-bake step), so the seed fetch moves before stage 1.--upstream-seed,--registry-modulesresolves from the seed's own bundles (manifestplugin/module.assemblyName, filename fallback) — identity-matched by construction.Verification, stated honestly
Build clean at
-warnaserror; the up-front refusal still exits 6. The compose path's real proof is the next Manufacturing run against the live registry — deliberately not simulated, because a synthetic seed exercising a private method through a scaffold would prove the scaffold, and this evening has been one long lesson in verifications that exercise the wrong thing.🤖 Generated with Claude Code