Repository navigation
feat(approvals): Approvals rides the module lane — and its views drop hand-built HTML - #1654
Merged
Merged
Conversation
Approval workflows move out of MeshWeaver.Graph into a new module project (the OgCard/MarkdownExport precedent): an assembly-level ApprovalsModuleAttribute whose single BuilderConfigurations hook invokes the same public AddApprovals(MeshBuilder) a fixture calls, so the Modules:Assemblies lane and the compiled-in lane share one code path. Registration is now mesh-wide (ConfigureDefaultNodeHub) instead of Markdown-only — approvals can be requested on any node type — and both portals list MeshWeaver.Approvals.dll. Stays platform-level: the Approval record + type-registry seeding + the _Approval → annotations satellite mapping (stored approvals keep deserializing and routing while the module is delisted), and a 3-line ApprovalsEnabled marker in Graph so MarkdownOverviewLayoutArea's embedded section self-suppresses when the module is absent. Fixed during the move: every hand-built Controls.Html string in the approval views is replaced with typed controls (H2/H3/Body/Badge/NavLink/Stack rows; the private EscapeHtml is deleted), Content-is casts become ContentAs<Approval>, and the previously hard-coded English strings are localized (19 approval.* keys, mirrored byte-identically into clients/react/src/i18n). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extracts Approvals from MeshWeaver.Graph into a new MeshWeaver.Approvals module that can be activated via Modules:Assemblies, and updates the approval UI to use typed controls instead of hand-built HTML while expanding approvals registration to all per-node hubs.
Changes:
- Introduces
src/MeshWeaver.Approvals/as a module project with an assembly-levelMeshNodeProviderAttributeregistration hook. - Moves/rewires approvals node type + views + hub registration into the module, leaving only a small marker/guard surface in
MeshWeaver.Graph. - Adds
approval.*localization keys on server and mirrors them into the React i18n catalogs; updates portal appsettings to load the module.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Graph.Test/MeshWeaver.Graph.Test.csproj | Adds module project reference for new module-surface tests. |
| test/MeshWeaver.Graph.Test/ApprovalModuleTest.cs | New tests pin module registration surfaces and marker behavior. |
| test/MeshWeaver.Graph.Test/ApprovalAndNotificationTest.cs | Narrows scope to platform-level models; moves module assertions to new test class. |
| test/Memex.Portal.Shared.Test/ApprovalsModuleTest.cs | Pins module-lane activation via assembly attribute + builder registration. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.en.json | Adds English approval.* localization keys. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.de.json | Adds German approval.* localization keys. |
| src/MeshWeaver.Graph/MeshWeaver.Graph.csproj | Grants InternalsVisibleTo for module to reuse internal layout plumbing. |
| src/MeshWeaver.Graph/Configuration/MarkdownNodeType.cs | Removes Markdown-only approvals wiring (now module-provided). |
| src/MeshWeaver.Graph/Configuration/GraphConfigurationExtensions.cs | Removes graph-level approval type registration (now module-provided). |
| src/MeshWeaver.Graph/ApprovalsMarker.cs | Adds a Graph-level marker/guard API to let embedders self-suppress when module is delisted. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-16-approvals-module.md | Adds “What’s New” entry documenting module activation and scope changes. |
| src/MeshWeaver.Approvals/README.md | Documents module purpose, activation, and data storage behavior. |
| src/MeshWeaver.Approvals/MeshWeaver.Approvals.csproj | New module project definition referencing Graph. |
| src/MeshWeaver.Approvals/ApprovalsView.cs | Updates request/inline views to typed controls and localized labels; removes hand-built HTML. |
| src/MeshWeaver.Approvals/ApprovalsModuleAttribute.cs | Adds assembly-level module registration attribute. |
| src/MeshWeaver.Approvals/ApprovalNodeType.cs | Moves approval node type definition into module. |
| src/MeshWeaver.Approvals/ApprovalLayoutAreas.cs | Moves approval overview/thumbnail areas into module and replaces HTML with typed controls. |
| src/MeshWeaver.Approvals/ApprovalExtensions.cs | Centralizes mesh + per-node-hub registration under AddApprovals/module hook. |
| MeshWeaver.slnx | Adds the new module project to the solution. |
| memex/Memex.Portal.Shared/Memex.Portal.Shared.csproj | References module so it rides publish output. |
| memex/Memex.Portal.Monolith/appsettings.json | Adds MeshWeaver.Approvals.dll to module assemblies list. |
| memex/aspire/Memex.Portal.Distributed/appsettings.json | Adds MeshWeaver.Approvals.dll to module assemblies list. |
| clients/react/src/i18n/strings.en.json | Mirrors new English approval keys for React drift guard. |
| clients/react/src/i18n/strings.de.json | Mirrors new German approval keys for React drift guard. |
Suppressed comments (1)
src/MeshWeaver.Approvals/ApprovalsView.cs:82
- The due-date placeholder is a specific calendar date, which can become misleading over time and doesn’t match the localized “YYYY-MM-DD” format hint. Prefer a format placeholder so the UI stays correct regardless of current date.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Test Results (shard 0)914 tests 913 ✅ 10m 26s ⏱️ Results for commit 1f7ae0d. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 4)1 542 tests 1 536 ✅ 7m 20s ⏱️ Results for commit 1f7ae0d. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 5)1 367 tests 1 366 ✅ 6m 11s ⏱️ Results for commit 1f7ae0d. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 3) 11 files 11 suites 6m 25s ⏱️ Results for commit 1f7ae0d. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 1)2 178 tests 2 075 ✅ 7m 30s ⏱️ Results for commit 1f7ae0d. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 2)2 367 tests 2 363 ✅ 7m 46s ⏱️ Results for commit 1f7ae0d. ♻️ This comment has been updated with latest results. |
Contributor
Test Results 65 files 65 suites 45m 42s ⏱️ Results for commit 1f7ae0d. ♻️ This comment has been updated with latest results. |
…e into the module
Both PR reds shared one root cause: CreateNode's NodeType-existence check
(MeshExtensions step 3) refuses NodeType 'Approval' in any mesh that does not
activate the Approvals module, now that AddGraph no longer registers it.
- Shard 5: ThreadUrlResolutionTest's _Approval theory case creates an Approval
satellite through the guarded CreateNode — the fixture now opts in via the
module's builder extension (.AddApprovals()), like the portals do via
Modules:Assemblies. (MeshWeaver.Approvals arrives transitively through the
project's existing Memex.Portal.Shared reference.)
- Doc content gate: the SAMPLES stage failed on 'install: FutuRe' — the tree
ships two Approval satellites (…/TransactionMapping/MappingRules/_Approval/a1)
whose install the tester's mesh refused for the same reason. The tester's
portal-shaped composition now calls .AddApprovals(), the exact precedent of
its .AddAI(...) line ('a portal always registers these; without them those
installs are refused'). The gate is untouched — no allow-list entry added.
(The reported 'digest-mismatch: error' was a red herring: it is the
download-artifact step echoing its own config option, and the digest matched.)
Verified: ThreadUrlResolutionTest 20/20 passed (incl. the exact failing case);
samples stage GREEN locally (FutuRe installs 145 files; only the 8 pre-existing
known-debt entries stay red); doc stage ALL GREEN; both touched projects build
clean with -c Release -warnaserror.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…chrome areas Shard-4 red on PR #1654: GoodPackage_CompilesRendersAndExecutesTestsGreen saw TestsDetail = 'all rendered cases green' instead of '1/1 passed'. Root cause: RenderMenus is a GLOBAL predicate renderer that writes the node menu into EVERY layout-area store under $Menu:{context}, and the Approvals module's menu entry carries the ✅ emoji as its Icon — AreaProbe.ClassifyTestsFrame walked ALL strings in the frame, so an early menu-bearing frame (permissions resolve fast; the Tests markdown renders later) matched the '✅ without a summary' rule and Take(1) latched the premature verdict. A ❌-shaped icon would have failed a green run the same way. A race, hence CI-only. Fix at the defect: CollectStrings skips the reserved '$'-prefixed chrome areas ($Menu, $Menu:{context}, $Dialog, $NavMenu) — chrome strings are labels and icons, never test output — covering both the wire key encoding ('"$Menu"') and the synthetic-seam plain keys. The Approvals icon stays ✅ (deliberate, reads as sign-off in any locale); no assertion weakened. Deterministic repro pinned: AreaProbeTest.MenuChromeFrame_ThenGreenTable replays the exact CI frame order (chrome first, content second) and demands the real 'N/M passed' detail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 16, 2026
Merged
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.
What moved
src/MeshWeaver.Approvals/is a new module project (OgCard/MarkdownExport precedent), taking the four approval sources out ofMeshWeaver.Graph:ApprovalsView.cssrc/MeshWeaver.Graph/ApprovalsView.csApprovalLayoutAreas.cssrc/MeshWeaver.Graph/ApprovalLayoutAreas.csApprovalExtensions.cssrc/MeshWeaver.Graph/ApprovalExtensions.csApprovalNodeType.cssrc/MeshWeaver.Graph/Configuration/ApprovalNodeType.csActivation is the module lane: an assembly-level
ApprovalsModuleAttribute : MeshNodeProviderAttributewhose singleBuilderConfigurationshook invokes the same publicAddApprovals(this MeshBuilder)a test fixture calls — one code path for both lanes, so they cannot drift (theMarkdownExportProviderAttributeshape; the brief's decomposedNodes/DefaultNodeHubConfigurationssplit would have re-opened exactly the drift the shared-path rule forbids, andMeshNodeProviderAttribute.BuilderConfigurations' own doc names this case).MeshWeaver.Approvals.dllis listed inModules:Assembliesof both portals (Monolith + Distributed) and referenced fromMemex.Portal.Sharedso the DLL rides the publish output.What stayed platform-level
Approval/ApprovalStatusrecords (MeshWeaver.Mesh.Contract/Approval.cs) — the type-registry seeding (MeshNodeExtensions.WithGraphTypes) andSatelliteTableMapping.Defaults("_Approval","annotations","Approval")depend on them, so stored approvals keep deserializing and routing with the module delisted."Approval" => "annotations"query-routing rule inGraphConfigurationExtensions(wire literal, storage routing).ApprovalsMarker.cs:ApprovalsEnabled+HasApprovals()): the module's per-node-hub registration sets the marker;MarkdownOverviewLayoutAreakeeps guarding its embedded Approvals section with it, so the section self-suppresses when the module is delisted.Mesh-wide instead of Markdown-only — deliberate
AddApprovalsused to hang off the Markdown NodeType's HubConfiguration only. The module registers viaConfigureDefaultNodeHubon EVERY per-node hub, so approvals can be requested on any node type. The registrations removed from Graph:GraphConfigurationExtensions.AddGraph()'s.AddApprovalType(),MarkdownNodeType's.AddApprovals(), andAddApprovalViews()moved with the node type.The Controls.Html violations are fixed in the move
All hand-built HTML strings in the approval views are gone (the banned
Controls.Html($"<div><strong>…")shape), replaced with typed controls rendering the same information in the same order:Controls.H2/Controls.H3; form labels →Controls.Body(bold).Status/Requester/Approver/Purpose/Due/Decision Date/Created) → horizontalControls.Stacklabel+value rows; status →Controls.Badgewith the original status colors.<a>→Controls.NavLink.Stack+Badge+Body; the privateEscapeHtmlis deleted (typed controls escape text by nature). One rendering nuance: the connector in "Approved by: X on date" became a language-neutral "·" separator (same fields, untranslatable English word dropped).Content is Approvalcasts →ContentAs<Approval>()(the trap-door rule), so a degradedJsonElementpayload still renders instead of silently dropping.While rewriting those lines, the previously hard-coded English strings were localized: 19 new
approval.*keys instrings.{en,de}.json, mirrored byte-identically intoclients/react/src/i18n/(drift guard:localize.test.ts);menu.requestApproval/ui.approve/ui.reject/ui.goToDocumentreused.Fallback story
Delisting
MeshWeaver.Approvals.dllremoves the Approvals UI mesh-wide (menu entry, form, inline section, approval Overview/Thumbnail); the markdown overview's embed self-suppresses via the marker; approval data, its typing, and its satellite routing stay intact and come back when the module is re-listed. Swept repo (content/,samples/*/Data,clients/,memex/) and the live mesh (search/search_chunks) — zero external consumers of the moved symbols or area names.Tests
test/MeshWeaver.Graph.Test/ApprovalModuleTest.cs— the module-surface tests split out ofApprovalAndNotificationTest.cs(which keeps the platform-level data-model tests), plus a pin thatConfigureHubsets theApprovalsEnabledmarker the overview guard reads.test/Memex.Portal.Shared.Test/ApprovalsModuleTest.cs— module-shape pin (assembly carriesMeshNodeProviderAttributewith a builder registration), mirroringOgCardModuleTest.Verification (all
-c Release -warnaserror, one project per invocation)MeshWeaver.Approvals,MeshWeaver.Graph(--no-incremental),Memex.Portal.Shared,Memex.Portal.Monolith,MeshWeaver.Graph.Test,Memex.Portal.Shared.Test,MeshWeaver.Hosting.Monolith.Test,MeshWeaver.Documentation.Test.MeshWeaver.Graph.Test: Passed 1161/1161 (fresh .trx; the 5 new module tests executed).Memex.Portal.Shared.Test: Passed 362/362 (pin test executed).MeshWeaver.Hosting.Monolith.Test --filter "FullyQualifiedName~DocumentExport|FullyQualifiedName~Markdown": Passed 19/19 (the overview-embed path with the module absent from fixtures — the marker guard suppresses the section, matching delisted behavior).MeshWeaver.Documentation.TestDocumentationLinkIntegrityTest: Passed 1/1 (new What's New page).MeshWeaver.Messaging.Hub.Test --filter "FullyQualifiedName~LocalizationTest": Passed 56/56 (catalog completeness with the 19 new keys).localize.test.tsdrift guard asserts exactly this equality).MeshWeaver.Approvals.dllconfirmed present in the Monolith portal's publish output (ridesMemex.Portal.Shared).2026-08-16-approvals-module.md(Category: Feature).🤖 Generated with Claude Code