Repository navigation
feat(build-project): embedded resources + assembly identity — the batch that unblocks memex-as-compiler - #2892
Merged
Merged
Conversation
`<EmbeddedResource>` items are EMBEDDED now, each under the manifest name the SDK's own `CreateCSharpManifestResourceName` would have produced — the single biggest coverage gap in the SDK-free builder, blocking 19 of the 54 non-test projects in MeshWeaver.Plugins/src on nothing but `<EmbeddedResource Include="Data\**\*.md">`. manifest-resource name is exactly the silent difference that design exists to prevent. The assembly compiles, ships, loads, and GetManifestResourceStream returns null at run time in another process — no error, no failing test, nothing a review can see. Core's own MeshWeaver.Messaging.Hub.csproj already carries a comment describing that outcome verbatim. So no naming rule here was recalled. Each was established by building a probe project with the real .NET SDK and reading the manifest-resource table back out of the emitted PE — including the asymmetry that makes the whole thing treacherous: the DIRECTORY is mangled into identifiers and the FILE NAME is not. Supported, measured: the default $(RootNamespace).<mangled dir>.<file name>; %(TargetPath) beating %(Link) beating the item spec; a file outside the project losing its directory entirely; LogicalName as attribute or child element; Include/Exclude/Remove/Update; the SDK's default **/*.resx glob; PUBLIC resources; CS1566 for a missing literal include; and $(AssemblyName) / $(RootNamespace) defaulting to the PROJECT name, which fixes an $(AssemblyName)-expands-to-empty bug this change exposed. Refused BY NAME, each with its own --accept token: .resx/.restext (the name is reproducible, the content needs resgen), a culture in the file name (the SDK routes it to a SATELLITE assembly and an explicit LogicalName does NOT rescue it), DependentUpon (the name becomes a class name extracted by MSBuild's own tokenizer), ManifestResourceName (the SDK's handling is a quirk), and embedding the build's own output. EmitPipeline gains an OVERLOAD rather than an optional parameter: adding a parameter replaces a method's signature, and the four-argument entry point stays byte-identical for MeshNodeCompilationService and NodeSetCompiler. CreateCompilationOptions and OptionsFingerprint are untouched. Verified by running: 226 tests green (32 new), including a PARITY test that builds one probe project with the real SDK and with build-project and asserts the manifest name sets are identical, plus three mutation proofs. Sweep over all 54 non-test projects in MeshWeaver.Plugins/src against the pinned portal image. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 62910be)
… obvious thing with All three found by continuing to MEASURE rather than by reading the code back, and each would have been silent. 1. An EXPLICIT %(Culture) beats %(WithCulture)='false' — the opposite of what the two names suggest. `Include="A.md" Culture="fr" WithCulture="false"` still emits a fr/ SATELLITE and leaves the main assembly without the resource, while `Include="C.de.md" WithCulture="false"` keeps it. The check order was the other way round, which would have embedded, under a name the SDK never produces, a resource the SDK does not put in this assembly at all. 2. A glob reaching OUTSIDE the project expanded to NOTHING — this evaluator enumerates from the project directory down — and an empty glob is legal, so the resources were simply absent with no line of output saying so. The real SDK expands `..\lb\**\*.md` and embeds what it finds. Now a named refusal. 3. %(LinkBase) synthesizes a %(Link) of `<LinkBase>\%(RecursiveDir)%(Filename) %(Extension)` for a file OUTSIDE the project cone and is IGNORED for one inside it — measured both ways. Only the outside case changes a name, so only the outside case is refused; refusing the inside one would be a false refusal on a no-op. Also: the measured sweep table, and the WhatsNew entry. The sweep number is 9 → 10 green of 54, not the 9 → ~24 the gap size suggested, and the reason is worth writing down: a refusal STOPS THE LOAD, so it masks every gap behind it. The 19 projects failing on the resource refusal were not 19 projects away from green — they were 19 projects about which nothing further was known. Ten of them want SDK source generators (the previous sweep counted 3), three want Razor, three want an additional library, one wants protoc. No project regressed, and none fails on an embedded resource any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit a8f38af)
…ng identity `GenerateAssemblyInfo` is a TARGET, and `build-project` runs no targets — so every assembly it emitted carried Roslyn's default identity, `AssemblyVersion=0.0.0.0`, while the whole fleet binds `3.0.0.0`. Nothing went red. Measured first-hand: building `MeshWeaver.Plugins/src/MeshWeaver.Speech.Contract` through a tester image from `src/Directory.Build.props` pins `3.0.0.0` precisely because #143 shipped 1.0.0.0 assemblies into a 3.0.0.0 process and CrashLoopBackOff'd a migration. The failure would have surfaced in another repo, at runtime, as `FileNotFoundException: … Version=3.0.0.0`. `ProjectFile` now evaluates the identity and `ProjectBuild` synthesizes it into the compilation as one more source document, exactly as the SDK's generated `<Project>.AssemblyInfo.cs` is one more Compile item. Every rule was MEASURED against SDK 10.0.400 rather than recalled — `Version=1.2.3-beta.4` → `1.2.3.0`, `1` → `1.0.0.0`, `01.2.3` → `1.2.3.0`, `FileVersion` follows `AssemblyVersion` and never `$(Version)`. Also promoted out of this evaluator's "metadata that changes nothing" list, which was true of the COMPILE and false of the ASSEMBLY: `InternalsVisibleTo` (sixty-odd projects across these repos), `AssemblyMetadata` and `AssemblyAttribute` items. `GenerateAssemblyInfo=false` synthesizes nothing (a second set is CS0579); each `Generate…Attribute` switch is honoured. A property that cannot be derived is a NAMED failure, never a plausible substitute. Two divergences, both said out loud rather than hidden: `$(SourceRevisionId)` (the SDK reads the commit from git; every build here reports the absent `+<sha>` suffix by name, and the new `-p:Name=Value` gives exact parity) and `TargetFrameworkAttribute` (a different target, with platform companions this evaluator cannot compute). `-p:` also fixes a global property that was documented to win and did not: a `<PropertyGroup>` could overwrite one, so the flag would have meant nothing for exactly the properties a caller passes it for. 🚨 `CreateCompilationOptions` and `OptionsFingerprint` are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 2e353f2)
… a 22nd parameter
The binary-compat gate refused this batch — correctly, this time:
record Model — arity: primary constructor went from 21 to 22 parameter(s)
#2855's original commit passed AssemblyInfo positionally. Its green attested to
a stale merge base (integration/core-batch) where the arity comparison ran
against a tree that already carried the change — the wrong-base shape its own
PR thread documented for a DIFFERENT record the same evening. Against main the
break is real, and the record's own doc block states the house rule the commit
violated:
"These four are init PROPERTIES, not primary-constructor parameters. Adding
a parameter — even with a default — REPLACES the record's constructor
signature, and scripts/check-record-signatures.py refuses that for exactly
the reason it should."
So AssemblyInfo now follows RazorItems/EmbeddedResources onto the initializer:
* Model stays at arity 21;
* `public AssemblyInfo AssemblyInfo { get; init; } = AssemblyInfo.None;`
* AssemblyInfo.None — the "nothing was stamped" identity — keeps the property
non-nullable with a valid default;
* ToModel assigns it in the object initializer beside the other features.
No allow-file entry: the gate found a real break and the fix is to not break,
not to document breaking.
MeshWeaver.PluginTester / .Test -c Release -warnaserror 0/0
MeshWeaver.PluginTester.Test 265/265
scripts/check-record-signatures.py ✓ no binary-breaking changes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Extends the SDK-free build-project pipeline to more faithfully reproduce dotnet build output by embedding managed resources under SDK-equivalent manifest names and synthesizing SDK-style assembly identity (AssemblyVersion/FileVersion/etc.) without running MSBuild targets—supporting the “memex-as-compiler” lane.
Changes:
- Add
<EmbeddedResource>evaluation + manifest-name computation (SDK parity) and pass resources into the verified emit pipeline. - Synthesize SDK
GenerateAssemblyInfooutputs into the compilation (including additional item-derived attributes) and add MSBuild-style global properties (-p:Name=Value/--property). - Add extensive tests and documentation/What’s New updates covering resource naming + assembly identity parity.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/MeshWeaver.PluginTester/README.md | Updates build-project documentation (usage, capabilities, limitations). |
| tools/MeshWeaver.PluginTester/ProjectFile.cs | Adds embedded-resource + assembly-identity evaluation, global property immutability, and acceptance/refusal rules. |
| tools/MeshWeaver.PluginTester/ProjectBuild.cs | Logs resource names, synthesizes assembly-info source, and emits managed resources via EmitPipeline. |
| tools/MeshWeaver.PluginTester/Program.cs | Adds -p:/--property parsing for MSBuild-style global properties. |
| tools/MeshWeaver.PluginTester/ManifestResourceNames.cs | Implements SDK-measured manifest-resource naming rules (TargetPath/Link, Everett mangling, culture probing). |
| test/MeshWeaver.PluginTester.Test/ProjectFileTest.cs | Updates/extends evaluator tests for embedded-resource behavior and blanket accept. |
| test/MeshWeaver.PluginTester.Test/ProjectAssemblyIdentityTest.cs | New test suite validating emitted assembly identity via PE metadata (SDK parity + overrides). |
| test/MeshWeaver.PluginTester.Test/ManifestResourceNamesTest.cs | New unit tests pinning measured naming rules for manifest resource names. |
| test/MeshWeaver.PluginTester.Test/EmbeddedResourceTest.cs | New end-to-end + parity tests comparing emitted manifest resources against real dotnet build. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-31-the-sdk-free-builder-embeds-resources-under-the-sdks-own-names.md | Adds a user-facing What’s New entry for embedded resource support. |
| src/MeshWeaver.Documentation/Data/Architecture/InMeshBuildAndTest.md | Updates architectural docs with embedded resource support + updated sweep reporting. |
| src/MeshWeaver.Compiler/EmitPipeline.cs | Adds an overload to embed managed resources during verified emit (preserving existing entry point). |
| src/MeshWeaver.Cli/README.md | Updates CLI docs around evaluator refusals and accepted constructs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
141
to
+153
| **Still not supported, and each says so:** SDK source generators (they live in the SDK, not the | ||
| runtime; `--generators` supplies them), embedded resources, `<Protobuf>`, MSBuild `<Target>`s and | ||
| `<Sdk>` elements. Full measurements: | ||
| **Not supported, and each says so:** Razor/Blazor compilation (needs | ||
| `Microsoft.CodeAnalysis.Razor.Compiler`, `Microsoft.AspNetCore.Razor.Utilities.Shared` and | ||
| `Microsoft.Extensions.ObjectPool` in the image — it ships none), SDK source generators (they live in | ||
| the SDK, not the runtime; `--generators` supplies them), `<Protobuf>`, MSBuild `<Target>`s and | ||
| `<Sdk>` elements. **Embedded resources ARE supported** — under the SDK's own manifest names, every | ||
| rule established by building probe projects with the real SDK and reading the names back out of the | ||
| emitted PE; `.resx`, a culture in a file name (which the SDK routes to a SATELLITE assembly), | ||
| `DependentUpon` and `ManifestResourceName` are each refused BY NAME rather than guessed, because a | ||
| plausible-looking wrong resource name is the one defect nothing downstream can see. Full | ||
| measurements: |
Comment on lines
78
to
+79
| | `--accept <construct>` | acknowledge one construct the evaluator cannot reproduce (`target:<Name>`, `embedded-resource`, `conditions`, `razor-css-scope`, `razor-not-compiled`). Repeatable | | ||
| | `--accept <construct>` | acknowledge one construct the evaluator cannot reproduce (`target:<Name>`, `conditions`, `embedded-resource`, `embedded-resource:{resx,culture,dependent-upon,manifest-resource-name,build-output}`). Repeatable | |
Comment on lines
+435
to
439
| Over every non-test project in `MeshWeaver.Plugins/src` (54 of them), against the portal image | ||
| `memex-portal-ai@sha256:6f38db08…` (the pin `MeshWeaver.Plugins/.github/workflows/ci.yml` carries), | ||
| with `--accept targets` and no `--extra-refs`. Both columns come from the SAME image, so the delta is | ||
| the change and nothing else. | ||
|
|
Contributor
Test Results (shard 0)792 tests 788 ✅ 5m 2s ⏱️ Results for commit bf7c7ad. |
Contributor
Test Results (shard 1)1 119 tests 1 119 ✅ 3m 56s ⏱️ Results for commit bf7c7ad. |
Contributor
Test Results (shard 5) 8 files 8 suites 4m 18s ⏱️ Results for commit bf7c7ad. |
Contributor
Test Results (shard 4)1 165 tests 1 165 ✅ 4m 20s ⏱️ Results for commit bf7c7ad. |
Contributor
Test Results (shard 2)3 057 tests 2 865 ✅ 6m 31s ⏱️ Results for commit bf7c7ad. |
Contributor
Test Results (shard 3)1 145 tests 1 145 ✅ 7m 46s ⏱️ Results for commit bf7c7ad. |
Contributor
Test Results 44 files 44 suites 31m 56s ⏱️ Results for commit bf7c7ad. |
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.
Replaces #2855 and #2860, which cannot pass the merge queue as they stand: their branches descend from
integration/core-batchand carry the originals of commits main has as squashes, so they readCLEANon their pages and come backUNMERGEABLEfrom the queue, every time. This branch is current main + exactly their three unique commits, cherry-picked and union-resolved:The resolution is a UNION, and the naive alternatives both lose a feature
Main gained Razor support (#2856, via #2850) in the same files these commits extend. Taking either side wholesale is destructive — checking the branch's files onto main would have deleted
RazorGenerators.cs(−494 lines) andRazorBuildTest(−387), measured before resolving, which is why every conflicted region was merged by hand instead:ProjectFile.cs— Razor's record/items/globs and the EmbeddedResource ordered log and the AssemblyInfo records now coexist; main's stricter exact-suffix glob helper kept; main'scase "None"grouping (a.razorcan arrive as aNoneitem) kept over the branch's.ProjectBuild.cs—RazorCountandResourceCountonProjectResult; the OK-log line reports sources + Razor files + resources.Program.cs/ READMEs — usage and help are the union of--razor-generatorsand-p:Name=Value.Proof neither feature was lost:
MeshWeaver.PluginTester.Testruns 265/265 — more than either source branch alone (#2860: 229, #2855: 198), because it is both suites together.Why this is priority: it un-blocks the memex-as-compiler lane, measured
Swept all 49 non-test
MeshWeaver.Plugins/srcprojects throughbuild-projectlocally (native, fresh portal reference set built from current main):MeshWeaver.AIitself: 169 sources, 0 errors, GREEN--generatorsThree separate blockers fell in one afternoon: the
<EmbeddedResource>refusal (this batch), the stale-reference CS0436s (fresh app), and CS8795 ([GeneratedRegex]) — closed by handing the builder the SDK's ownSystem.Text.RegularExpressions.Generator.dll, which answers #2854's "source generators are the honest unknown" with a measured yes, they run.Full final numbers land on #2854 when the sweep completes; the per-module opt-in list there grows from measurement, exactly as that draft prescribes.
MeshWeaver.PluginTester/.Test/MeshWeaver.Cli/MeshWeaver.Compiler:-c Release -warnaserror,0 Warning(s) 0 Error(s).(#2855 and #2860 will be closed in favour of this once it lands — their content is byte-identical here, their history is what could not merge.)
🤖 Generated with Claude Code