Repository navigation
Forward the six #2276 types that left MeshWeaver.AI without one (#2398) - #2428
Conversation
#2370 was one instance of a class, not a one-off: a public type that changes assembly (or namespace) breaks every module compiled earlier, because a module's IL binds `Namespace.Type` scoped to an AssemblyRef and the module lane's gate is a semver floor that cannot see a type at all. No source build in either repo can observe it — building the plugin's source against the branch answers SOURCE compatibility, a different question. Replaying #2370's gate across v3.0.0-rc7 -> main found 17 unguarded moves. Four were fixed by #2370. This restores six more that #2276 made when it moved the credential-protection and MCP-back-connection contracts into MeshWeaver.Mesh.Contract: MeshWeaver.AI.IMasterKeyProvider MeshWeaver.AI.IProviderKeyProtector MeshWeaver.AI.ConfigMasterKeyProvider MeshWeaver.AI.ProviderKeyProtector MeshWeaver.AI.Connect.IMcpBackConnection MeshWeaver.AI.Connect.McpConnectionInfo Three have a PROVEN module consumer on Systemorph/MeshWeaver.Plugins@main — IMcpBackConnection and McpConnectionInfo in MeshWeaver.AI.{ClaudeCode,Copilot}, IProviderKeyProtector in MeshWeaver.Blazor.Portal and Memex.Portal.Gui. Their source was updated in the same wave, so landed-modules-gate is green and a REBUILT bundle is correct; a deployment still running a bundle published BEFORE the wave holds the old TypeRef and dies exactly like #2370 the moment its platform rolls. A forwarder makes that ordering irrelevant. A forwarder cannot rename, so each type keeps its ORIGINAL full name in its new home: five files under MeshWeaver.Mesh.Contract now declare `namespace MeshWeaver.AI` / `MeshWeaver.AI.Connect`. That mismatch is the contract, and each carries a comment saying so — tidying exactly this is what broke main today. Consumers gain a `using`; no shim types (a shim mints a SECOND identity and reintroduces the is/as trap-door). Proven both ways, not asserted: * gate red without the forwarders (13 findings, exactly the issue's list), green for these six with them: `check-type-forwards.py --base v3.0.0-rc7` now reports only the 7 cycle-blocked ones. * MovedTypeBinaryContractTest extended to 10 frozen names; it fails 7/12 with the forwarder lines deleted and passes 12/12 with them. The frozen list is now name -> hosting assembly, because the two waves landed in different assemblies. Also fixed, both found while doing the above: * scripts/check-type-forwards.py counted a COMMENTED-OUT forwarder. That is the gate's own false-PASS shape and the likeliest way a forwarder gets disabled (comment first, delete later) — it is what made the first negative control here look green. parse_forwards now truncates each line at `//`; three self-test rows pin it, including that a REAL forwarder with a trailing comment still counts. 23 self-tests pass. * check-record-signatures.py's workflow step anchored on the base branch TIP while the script's own docstring says merge base. The half that was wrong is not the file list (three-dot diff already resolves it) but the BEFORE CONTENT: `git show {base}:{file}` reads the literal ref, so a record whose primary constructor changes on main mid-PR reads as this PR reverting it. Now passes the merge-base SHA, matching the type-forwards step. NOT fixed, and deliberately left for a maintainer decision (#2398 stays open, narrowed to exactly these): MeshWeaver.GitSync -> MeshWeaver.AI (6 types) and MeshWeaver.Hosting -> MeshWeaver.AI (AgentFileParser) cannot be forwarded at all — the forwarder must live in the assembly being left, and MeshWeaver.AI already references both, so the reference would be a cycle. Confirmed rather than assumed: zero consumers of those 7 names in MeshWeaver.Plugins, .Reinsurance or .SocialMedia. The two honest options are to move the type back or to declare the atomic republish; inventing a shim to dodge the cycle is the one thing that must not happen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Restores binary compatibility for previously moved public contracts by adding type forwarders and keeping the original namespaces stable across assemblies, preventing TypeLoadException failures in already-published modules after platform updates.
Changes:
- Added
[assembly: TypeForwardedTo]entries inMeshWeaver.AIfor six types that moved toMeshWeaver.Mesh.Contract, with corresponding namespace “pins” in the destination files. - Hardened the type-forward gate script to ignore commented-out forwarders; adjusted CI to compare record signatures against the merge base rather than the base-branch tip.
- Updated tests, docs, and call sites to reference the preserved original namespaces (
MeshWeaver.AI,MeshWeaver.AI.Connect) while hosting types in the new assemblies.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Graph.Test/ProviderKeyProtectorRegistrationTest.cs | Imports updated to reference preserved MeshWeaver.AI namespace for credential protection contracts. |
| test/MeshWeaver.GitSync.Test/GitHubSyncTestBase.cs | Imports updated to reference preserved MeshWeaver.AI namespace for credential protection contracts. |
| test/MeshWeaver.AI.Test/MovedTypeBinaryContractTest.cs | Extends binary-contract test to include the six newly-forwarded names and their hosting assemblies. |
| src/MeshWeaver.PluginCatalog/SyncTokenSigningKeyService.cs | Imports updated to reference preserved MeshWeaver.AI namespace types. |
| src/MeshWeaver.PluginCatalog/InstanceAutoRegistrationService.cs | Imports updated to reference preserved MeshWeaver.AI namespace types. |
| src/MeshWeaver.Mesh.Contract/Services/IMcpBackConnection.cs | Pins the original MeshWeaver.AI.Connect namespace for the moved MCP back-connection contracts. |
| src/MeshWeaver.Mesh.Contract/Security/ProviderKeyProtector.cs | Pins the original MeshWeaver.AI namespace for the moved provider-key protector implementation. |
| src/MeshWeaver.Mesh.Contract/Security/IProviderKeyProtector.cs | Pins the original MeshWeaver.AI namespace for the moved provider-key protector interface. |
| src/MeshWeaver.Mesh.Contract/Security/IMasterKeyProvider.cs | Pins the original MeshWeaver.AI namespace for the moved master-key provider interface. |
| src/MeshWeaver.Mesh.Contract/Security/ConfigMasterKeyProvider.cs | Pins the original MeshWeaver.AI namespace for the moved config-backed master-key provider. |
| src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs | Imports updated to reference preserved MeshWeaver.AI namespace types. |
| src/MeshWeaver.GitSync/GitHubCredentialService.cs | Imports updated to reference preserved MeshWeaver.AI namespace types. |
| src/MeshWeaver.GitSync/GitHubCredential.cs | Documentation updated to reference the preserved MeshWeaver.AI.IProviderKeyProtector contract. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-26-installed-tools-survive-a-platform-update.md | Adds a “What’s New” entry explaining the user-visible impact of keeping installed tools working across updates. |
| src/MeshWeaver.Documentation/Data/Architecture/Modules.md | Documents the repeated type-move failure mode and why namespace/forwarder mismatches are intentional contracts. |
| src/MeshWeaver.AI/TypeForwards.cs | Adds six new type forwarders for the moved security and MCP back-connection contracts. |
| scripts/check-type-forwards.py | Fixes false-pass behavior by ignoring //-commented-out forwarders; adds self-tests for the new behavior. |
| memex/Memex.Portal.Shared/Authentication/McpBackConnectionService.cs | Imports updated to reference preserved MeshWeaver.AI.Connect namespace for MCP back-connection contracts. |
| memex/Memex.Portal.Shared/Authentication/EaGraphAuth.cs | Imports updated to reference preserved MeshWeaver.AI namespace for credential protection contracts. |
| .github/workflows/dotnet-test.yml | Makes record-signature checking compare against the PR merge base to avoid false failures from unrelated base-branch tip changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
🔗 Cross-repo half: Systemorph/MeshWeaver.Plugins#718 must merge FIRST.
// src/Memex.Portal.Gui/MemexPortalComposition.cs:291
services.AddSingleton<MeshWeaver.Mesh.Services.IMcpBackConnection, McpBackConnectionService>();So this PR will go red on that gate until #718 lands. It was the only such reference — swept every #718 is tolerant of both platforms, so plugins-first is the safer order here rather than the usual platform-first: One more datapoint from the sweep, in the reassuring direction: the private |
Test Results (shard 0)624 tests 620 ✅ 13m 16s ⏱️ Results for commit f1a47f1. |
Test Results (shard 5) 9 files 9 suites 7m 15s ⏱️ Results for commit f1a47f1. |
Test Results (shard 3) 9 files 9 suites 5m 16s ⏱️ Results for commit f1a47f1. |
Test Results (shard 4)2 195 tests 2 193 ✅ 14m 47s ⏱️ Results for commit f1a47f1. |
Test Results (shard 1)2 036 tests 1 843 ✅ 9m 38s ⏱️ Results for commit f1a47f1. |
Test Results (shard 2)2 878 tests 2 775 ✅ 8m 10s ⏱️ Results for commit f1a47f1. |
Fixes the forwardable part of #2398. #2398 stays open, narrowed to the two groups that cannot be forwarded at all — those need a maintainer decision, not a clever workaround.
What this is
#2370 was one instance of a class, not a one-off. A module's IL binds
Namespace.Typescoped to an AssemblyRef, and the module lane's gate is a semver floor that cannot see a type at all — so a public type that changes assembly or namespace breaks every module compiled earlier, at the next platform roll, with no warning anywhere. No source build in either repo can observe it: building the plugin's source against the branch answers source compatibility, which is a different question. That is precisely why #2283 shipped aTypeLoadExceptionon every MCP tool call having been verified exactly that way.Replaying #2370's gate across
v3.0.0-rc7 → mainfound 17 unguarded moves. #2370 fixed four. This fixes six more — the ones #2276 made when it moved the credential-protection and MCP-back-connection contracts intoMeshWeaver.Mesh.Contract:MeshWeaver.AI.IMasterKeyProviderMeshWeaver.AIMeshWeaver.Mesh.ContractMeshWeaver.AI.ConfigMasterKeyProviderMeshWeaver.AIMeshWeaver.Mesh.ContractMeshWeaver.AI.IProviderKeyProtectorMeshWeaver.AIMeshWeaver.Mesh.ContractMeshWeaver.AI.ProviderKeyProtectorMeshWeaver.AIMeshWeaver.Mesh.ContractMeshWeaver.AI.Connect.IMcpBackConnectionMeshWeaver.AIMeshWeaver.Mesh.ContractMeshWeaver.AI.Connect.McpConnectionInfoMeshWeaver.AIMeshWeaver.Mesh.ContractThree have a proven module consumer on
Systemorph/MeshWeaver.Plugins@main— confirmed by grepping the repo, not assumed:IMcpBackConnection+McpConnectionInfoinMeshWeaver.AI.ClaudeCode/MeshWeaver.AI.Copilot,IProviderKeyProtectorinMeshWeaver.Blazor.Portal/Chat/ThreadChatView.razor.csandMemex.Portal.Gui/MemexPortalComposition.cs.Their source was updated in the same wave, so
landed-modules-gateis green and a rebuilt bundle is correct. What that says nothing about is a deployment still running a bundle published before the wave: it holds the old TypeRef and dies exactly like #2370 the moment its platform rolls. A forwarder makes that ordering irrelevant — which is the whole point, since the ordering is not something we control.The two non-obvious constraints, both already load-bearing here
A forwarder cannot rename. So each type keeps its original full name in its new home: five files under
src/MeshWeaver.Mesh.Contractnow declarenamespace MeshWeaver.AI/MeshWeaver.AI.Connect. That mismatch is the contract, not a leftover — the same shapeMeshOperationsalready has inMeshWeaver.Mesh.Operations. Each file carries a comment saying so, because a "tidy-up" of exactly that brokemaintoday. Consumers gain ausing.No shim types. A forwarder keeps one type identity via the CLR's ExportedType table. A shim class left behind in
MeshWeaver.AIwould satisfy a naive resolution check while minting a second identity — reintroducing theis/astrap-door that reads as a silent null.MovedTypeBinaryContractTest.TheForwardersAreForwarders_NotDuplicatedTypesreads the ExportedType table specifically to refuse that.Proven, not asserted
python3 scripts/check-type-forwards.py --base v3.0.0-rc7reports 13 findings without the forwarders — exactly the list in 7 public types that left GitSync/Hosting for MeshWeaver.AI CANNOT be forwarded — move back, or republish atomically #2398 — and 7 with them, all of which are the cycle-blocked group below. Againstorigin/main(what CI runs) it is green.MovedTypeBinaryContractTestextended from 4 frozen names to 10. With the six forwarder lines deleted it fails 7/12; with them it passes 12/12. The frozen list became aname → hosting assemblymap, because the two waves landed in different assemblies.-c Release -warnaserror, one project per invocation):Mesh.Contract,AI,PluginCatalog,Memex.Portal.Shared,AI.Test,GitSync.Test,Graph.Test,Documentation.Test. Tests run: 40 inAI.Test(forwarder + provider-key + credential-seed),ProviderKeyProtectorRegistrationTest,WhatsNewEntryIntegrityTest,DocumentationLinkIntegrityTest— all green.Two gate defects found while doing it
A commented-out forwarder counted.
check-type-forwards.py'sFORWARD_REran over raw file text, so// [assembly: TypeForwardedTo(typeof(X))]satisfied the gate. That is the gate's own false-PASS shape, and the likeliest way a forwarder gets disabled in a refactor (comment first, delete later) — it is literally what made the first negative control here look green.parse_forwardsnow truncates each line at//. Three self-test rows pin it, including that a real forwarder with a trailing comment still counts (without that row, the fix would look correct while silencing live forwarders). 20 → 23 self-tests, all passing. Line comments only; a/* … */-buried forwarder would still count, and the docstring says so rather than implying coverage that is not there.check-record-signatures.py's step anchored on the base branch TIP while the script's own docstring says merge base (recorded on #2398). The half that was wrong is not the file list — the three-dotgit diff --name-only {base}...HEADalready resolves the merge base. It is the before content:git show {base}:{file}reads the literal ref, so a record whose primary constructor changes on main while a PR is open reads as that PR reverting it — a red on somebody else's commit. Now passes the merge-base SHA, matching what the type-forwards step already did.What is NOT fixed, and why it is a decision rather than a task
Two groups cannot be forwarded at all. The forwarder must live in the assembly being left, so that assembly must reference the type's new home — and
MeshWeaver.AIalready references both of these, so the forwarder would be a reference cycle:MeshWeaver.GitSync → MeshWeaver.AI:AiContentDiskWriter,AiContentSyncArea,AiContentSyncMenuProvider,AiContentSyncResult,HubAiContentSyncExtensions,PullRequestDraftServiceMeshWeaver.Hosting → MeshWeaver.AI:MeshWeaver.Hosting.Persistence.Parsers.AgentFileParserExposure, confirmed rather than assumed: zero consumers of those seven names in
MeshWeaver.Plugins,MeshWeaver.ReinsuranceorMeshWeaver.SocialMedia. So it does look core-only — but "no consumer in the repos I can see" is not the same as "no published bundle holds the TypeRef", which is the question that actually matters and the one #2370 answered the expensive way.The two honest options, both of them a maintainer call: move the types back, or accept the break and do the atomic republish (#2234) — rebuild and republish every affected bundle, then roll the image, so no deployment is ever running an old bundle against a new platform — recording it in
scripts/type-forwards.allow. Full detail on #2398.Also worth a separate decision, unchanged here on purpose: the Public surface (binary compatibility) job is not inSuperseded while this PR was open — #2407 (collect-results'needs.5ce64c984) landed onmainand wiredrecord-signaturesintocollect-results'needsplus the explicit fail step, so both binary-compat gates can now block a merge. Nothing to do here; this branch predates it and the two edits touch different hunks ofdotnet-test.yml, so they merge cleanly (mergeStateStatusisBLOCKEDon checks, notDIRTY).🤖 Generated with Claude Code