Repository navigation
The AI engine leaves the composition root — Memex.Portal.Shared no longer references it (#2276) - #2315
Merged
Conversation
…nger references it `Memex.Portal.Shared` compiled against `MeshWeaver.AI` in order to CONFIGURE it: the composition root registered the chat services, the CLI Connect surface, the model-provider service, the AI content sources, the credential seed, the menu entries, the settings tab and the portal's AI type registrations. That reference is the one a Store-delivered module cannot coexist with — a closure DLL and a registry module of the same name are mutually exclusive, and the landing service refuses with a 409 on every instance. So it had to go before the engine could ship as one Store entry (#2276). It moves as ONE change and could not be split: if the module registered the AI content sources from configuration while the portal still called `AddAI(...)` compiled-in, `AddBuiltInAiContentSources()` would run twice and the importer would import every AI source twice. **`AiMeshModuleAttribute` is now the whole composition.** It reads `MeshBuilder.Configuration` (#2300, added for exactly this) and does what MemexConfiguration did: `AddAI` with the serve-from-DB set, the AI menu seeds, `ModelProviderService`, the Connect token sink + session manager, the gated Claude strategy with its options, the skill→file sync, the AI content sources, and the provider-credential seed. Reading configuration at BUILDER time is not a preference: the serve-from-DB decision sets `MeshNode.IsDefinitionOnly`, an `init` property, and getting it wrong makes a partition root permanently unrecoverable (#902). **Five files move into the engine** — AiMenuContributions, ModelProviderService, ConnectTokenSink, TokenUsageSettingsTab, AgentSkillSyncService — and take their tests with them. **Three seams the platform keeps**, each because the HOST implements it: - `IMcpBackConnection` + `McpConnectionInfo` → `MeshWeaver.Mesh.Services`. `McpBackConnectionService` needs ApiTokenService and ASP.NET, so the implementation stays host-side and only the contract is platform — the `ISpeechTranscriber` shape. - `IPortalHubAccessor` → `MeshWeaver.Mesh.Services`. The skill sync needs the PORTAL's hub, and an ambient `IMessageHub` is the router. Nullable by design: a headless host has no portal, and the consumer degrades rather than failing a boot over an absent UI. - `IStaticRepoSource.AssertIndexedAfterImport`. The portal used to hand `ImportAll` the AI partition NAMES for the #354 index assertion — a module cannot be delisted from a list the host hard-codes. A source now declares its own requirement, the same shape as `SyncMode`, and `ImportAll` derives the set. Same partitions, declared by their owner. The AI catalogs and the token-usage settings tab register through the platform's existing per-node-hub and `AddSettingsMenuItems` seams; the portal's AI types and thread area are contributed via `PortalConfigurationRegistry`, which exists precisely so a plugin can configure portals it does not compile against. `ProviderCredentialSeedWiringTest` follows the seed into the module and now installs the assembly attribute the way `Modules:Assemblies` does — WHO registers it changed, whether it is registered did not, and that is what the test still asserts. Verified: full solution build under CI's flags; MeshWeaver.AI.Test 1306, MeshWeaver.Threading.Test 139, Memex.Portal.Shared.Test 428 — all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the last compiled MeshWeaver.AI dependency from Memex.Portal.Shared by moving AI composition/wiring into an assembly-installed module (AiMeshModuleAttribute) that reads deployment configuration at MeshBuilder time, enabling Store-delivered AI modules without closure/module identity conflicts.
Changes:
- Introduces
AiMeshModuleAttributeas the AI engine’s composition root (config-drivenAddAI(...), AI menu seeding, Connect wiring, skill-sync, and DB-served catalog source/seed wiring). - Moves/rewires portal-facing AI services and settings integration into
MeshWeaver.AI.Portaland registers them via existing platform seams (settings menu items, portal configuration registry). - Shifts static-repo index-assertion responsibility from host-provided partition lists to per-source opt-in (
IStaticRepoSource.AssertIndexedAfterImport), and updates import wiring accordingly.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.AI.Test/UseWithoutSeeResolverTest.cs | Updates test imports to the new AI-portal namespace. |
| test/MeshWeaver.AI.Test/ProviderModelListerTest.cs | Updates test imports to the new AI-portal namespace. |
| test/MeshWeaver.AI.Test/ProviderKeyEncryptionTest.cs | Updates test imports to the new AI-portal namespace. |
| test/MeshWeaver.AI.Test/ProviderCredentialSeedWiringTest.cs | Reworks the wiring test to install the AI module via its assembly attribute/config-driven builder contributions. |
| test/MeshWeaver.AI.Test/ModelProviderServiceTest.cs | Updates test imports to the new AI-portal namespace. |
| test/MeshWeaver.AI.Test/ModelProviderSelectionTest.cs | Updates test imports to the new AI-portal namespace. |
| test/MeshWeaver.AI.Test/ConnectStrategyTest.cs | Updates test imports to the new AI-portal namespace. |
| test/MeshWeaver.AI.Test/AiResolutionSeamAuditTest.cs | Extends seam audit coverage to include the moved portal sync service. |
| test/MeshWeaver.AI.Test/AgentSkillSyncServiceTest.cs | Updates unit test namespace/imports for the moved skill-sync instructions logic. |
| test/Memex.Portal.Shared.Test/PlatformSettingsTabSeedTest.cs | Removes Token Usage tab from portal-side settings-tab seed assertions now that it’s contributed by the AI module. |
| test/Memex.Portal.Shared.Test/Memex.Portal.Shared.Test.csproj | Allows the test suite to reference MeshWeaver.AI to assert portal shell integration while keeping the platform closure AI-free. |
| test/Memex.Portal.Shared.Test/AiMenuNewThreadTest.cs | Updates test imports to reference AI menu contribution types from the AI module. |
| src/MeshWeaver.Mesh.Contract/Services/IPortalHubAccessor.cs | Adds a new seam for exposing the portal hub without referencing portal assemblies. |
| src/MeshWeaver.Mesh.Contract/Services/IMcpBackConnection.cs | Moves the MCP back-connection contract out of the AI namespace into platform services. |
| src/MeshWeaver.Mesh.Contract/IStaticRepoSource.cs | Adds a per-source opt-in flag for post-import index assertions. |
| src/MeshWeaver.Graph/StaticRepoImporter.cs | Derives the “assert indexed after import” set from sources when the host does not supply it. |
| src/MeshWeaver.AI/SkillStaticRepoSource.cs | Opts the Skill catalog source into post-import index assertions. |
| src/MeshWeaver.AI/ModelStaticRepoSource.cs | Opts the Provider/Model catalog source into post-import index assertions. |
| src/MeshWeaver.AI/HarnessStaticRepoSource.cs | Opts the Harness catalog source into post-import index assertions. |
| src/MeshWeaver.AI/ProviderCredentialSeedHostedService.cs | Moves the provider-credential seed boot hook into the AI engine. |
| src/MeshWeaver.AI/Portal/TokenUsageSettingsTab.cs | Moves the Token Usage settings tab into the AI module and registers it via the settings seam. |
| src/MeshWeaver.AI/Portal/ModelProviderService.cs | Moves the model-provider credential management service into the AI module namespace. |
| src/MeshWeaver.AI/Portal/ConnectTokenSink.cs | Moves the portal-side connect token sink into the AI module namespace. |
| src/MeshWeaver.AI/Portal/AiMenuContributions.cs | Moves AI menu seed definitions into the AI module namespace. |
| src/MeshWeaver.AI/Portal/AgentSkillSyncService.cs | Moves the hosted service for writing AGENTS.md + skill catalog sync into the AI module and rewires portal hub access via the new seam. |
| src/MeshWeaver.AI/InternalsVisibleTo.cs | Grants internals access to Memex.Portal.Shared.Test for cross-assembly integration assertions. |
| src/MeshWeaver.AI/Application/AgentsApplicationExtensions.cs | Contributes AI portal wiring (types + thread area) via portal configuration registry instead of a host composition root. |
| src/MeshWeaver.AI/AiMeshModuleAttribute.cs | Adds the assembly-installed AI module attribute that reads deployment config and performs all AI composition. |
| src/MeshWeaver.AI/AIExtensions.cs | Ensures AI layout areas/settings contributions are registered as part of AddAI(...) without portal composition-root involvement. |
| memex/Memex.Portal.Shared/StaticRepoSyncExtensions.cs | Removes AI-specific import wiring from the portal host and relies on module/source self-declaration. |
| memex/Memex.Portal.Shared/Settings/PlatformSettingsTabAreas.cs | Removes Token Usage area/seeded UiContribution now that it’s contributed through settings menu items by the AI module. |
| memex/Memex.Portal.Shared/MemexConfiguration.cs | Drops compiled AI composition (skill-sync, connect wiring, AddAI, menu contributions) and keeps only the host-owned seams. |
| memex/Memex.Portal.Shared/Memex.Portal.Shared.csproj | Removes the MeshWeaver.AI project reference and documents the Store/module rationale. |
| memex/Memex.Portal.Shared/Authentication/McpBackConnectionService.cs | Updates the service to implement the moved IMcpBackConnection contract. |
Suppressed comments (2)
src/MeshWeaver.AI/Portal/AgentSkillSyncService.cs:103
- AgentSkillSyncService now resolves the portal hub via IPortalHubAccessor, but there is no implementation or DI registration of IPortalHubAccessor anywhere in the repo (no
: IPortalHubAccessorand noAdd*<IPortalHubAccessor>). As a result,hubwill always be null and the service will only write the base AGENTS.md instructions, never the live skill catalog sync.
src/MeshWeaver.AI/Portal/TokenUsageSettingsTab.cs:31 - The class-level docs still describe Token Usage as being exposed via seeded UiContribution nodes and a SettingsTokenUsage layout area, but this PR moves the tab to the declarative SettingsMenuItemDefinition seam (AddTokenUsageSettingsTab) and removes it from PlatformSettingsTabAreas. This documentation is now misleading about how the tab is registered and gated.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Test Results (shard 3) 9 files ± 0 9 suites ±0 5m 40s ⏱️ +22s Results for commit faacf8b. ± Comparison against base commit 187f910. This pull request removes 473 and adds 415 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
Contributor
Test Results (shard 2)3 436 tests - 6 3 436 ✅ - 6 9m 34s ⏱️ -3s Results for commit faacf8b. ± Comparison against base commit 187f910. This pull request removes 6 tests.♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 4)2 296 tests +64 1 999 ✅ +65 11m 28s ⏱️ - 2m 22s Results for commit faacf8b. ± Comparison against base commit 187f910. This pull request removes 412 and adds 476 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Test Results 55 files ±0 55 suites ±0 59m 28s ⏱️ - 3m 0s Results for commit faacf8b. ± Comparison against base commit 187f910. This pull request removes 6 and adds 6 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…e-composition-root
`ImpersonationScopeSiteRatchetGuard` went red on `ModelProviderService`: the file moved into `src/MeshWeaver.AI/Portal/`, and its PRE-EXISTING `Observable.Using(() => …SwitchAccessContext(ctx), …)` therefore reads as a NEW site rather than as accepted debt at its old path. The guard is right, and it says so in the failure: do NOT add a line to the allow file. That shape opens the AsyncLocal scope on the SUBSCRIBING thread and disposes it on whichever thread the work terminates, leaving the subscriber running as the impersonated identity (#1790). Replaced with `accessService.RunAs(ctx, () => meshService.CreateNode(modelNode))` — the sanctioned scope, and a smaller expression than what it replaces: `RunAs` already defers when access or identity is null, which is exactly what the ternary's other branch did by hand. So the move did not merely relocate the debt past a guard keyed on paths; it paid it. Also merges origin/main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Four reds, triaged — two were mine and are fixed, two are not. 1.
|
…e-composition-root # Conflicts: # memex/Memex.Portal.Shared/MemexConfiguration.cs # src/MeshWeaver.AI/Portal/AgentSkillSyncService.cs
Contributor
Author
|
Conflicts resolved — main moved underneath this branch when #2285 landed, relocating Two conflicts, both in files this PR touches from the opposite end:
Verified rather than assumed, per the rule that a branch-favoured hunk silently undoing merged work is the real failure mode:
|
rbuergi
added a commit
that referenced
this pull request
Aug 26, 2026
`BlockingBridgeInTestRatchetGuard` went red for the same reason the impersonation ratchet did on #2315: `ThreadMessageToolTest` moved, so its PRE-EXISTING `InvokeAsync(...).AsTask().GetAwaiter().GetResult()` stopped being accepted debt at its old path and read as a NEW site. Both guards say the same thing in their failure text — do NOT add a line to the allow file. So the helper becomes `async Task<string> InvokeAsync(...)` with a real `await`, and its three callers become `async Task` tests. That is what the guard asks for, and it is what the suite's own convention already is everywhere else: a blocking bridge parks the calling thread on a continuation that may need that thread, which is the deadlock shape the ratchet exists to keep out. Two moves, two path-keyed ratchets, two pieces of debt PAID rather than relocated. Worth noting the pattern: a file move is the moment a path-keyed allow-list stops protecting a site, which makes it the moment to fix it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-composition-root
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.
Memex.Portal.Sharedcompiled againstMeshWeaver.AIin order to configure it. That is the reference a Store-delivered module cannot coexist with: a closure DLL and a registry module of the same name are mutually exclusive, and the landing service refuses with a 409 on every instance.It could not be split
If the module registered the AI content sources from configuration while the portal still called
AddAI(...)compiled-in,AddBuiltInAiContentSources()would run twice — duplicateIStaticRepoSourceregistrations, and the importer imports every AI source twice. So the module attribute, the removal of the compiled call, and the ~13 types of wiring land together.AiMeshModuleAttributeis now the whole compositionIt reads
MeshBuilder.Configuration(#2300, added for exactly this) and does whatMemexConfigurationdid:AddAIwith the serve-from-DB set, the AI menu seeds,ModelProviderService, the Connect token sink + session manager, the gated Claude strategy and its options, the skill→file sync, the AI content sources, and the provider-credential seed.Reading configuration at builder time is not a preference: the serve-from-DB decision sets
MeshNode.IsDefinitionOnly, aninitproperty, and getting it wrong makes a partition root permanently unrecoverable (#902).Three seams the platform keeps, each because the HOST implements it
IMcpBackConnection+McpConnectionInfo→MeshWeaver.Mesh.ServicesMcpBackConnectionServiceneedsApiTokenService+ ASP.NET; only the contract is platform (theISpeechTranscribershape)IPortalHubAccessor→MeshWeaver.Mesh.ServicesIMessageHubis the router. Nullable by design — a headless host has no portal, and the consumer degradesIStaticRepoSource.AssertIndexedAfterImportImportAllthe AI partition names for the #354 index assertion. A module cannot be delisted from a list the host hard-codes, so a source declares its own requirement — same shape asSyncMode— andImportAllderives the setEverything else uses seams that already existed: the AI catalogs and the token-usage tab register through the per-node-hub chain and
AddSettingsMenuItems; the portal's AI types and thread area are contributed throughPortalConfigurationRegistry, which exists precisely so a plugin can configure portals it does not compile against.Tests follow their code
Five files move into the engine with their suites.
ProviderCredentialSeedWiringTestnow installs the assembly attribute the wayModules:Assembliesdoes — who registers the seed changed, whether it is registered did not, and that is what it still asserts. It failed first, which is how I knew the move was real.Verification
0 Error(s).MeshWeaver.AI.Test1306,MeshWeaver.Threading.Test139,Memex.Portal.Shared.Test428 — all green.What this does and does not finish
Memex.Portal.Sharedis off the list. The engine still reaches the app closure throughsrc/MeshWeaver.Blazor.Portal(a view pack, deleted from core by #2293) andtools/MeshWeaver.PluginTester(a tool, not the platform). The 409 clears when #2293 lands, and then Plugins#687 can flip topreInstalled.Refs #2276
🤖 Generated with Claude Code