Repository navigation
fix: exports failed on a test-only API, and a half-written email was lost on connect - #1275
Merged
Merged
Conversation
…lost on connect
Three defects in "Share ⇒ as email", all user-visible.
1) SENDING WAS BROKEN IN PRODUCTION. The three export templates are .csx —
compiled at RUNTIME by the kernel, invisible to dotnet build — and they called
meshService.QueryAsync<MeshNode>(...), which exists ONLY in MeshWeaver.Fixture,
a test-only assembly whose own doc comment says production must not use it. Every
export threw CompilationErrorException CS1061 the moment a user pressed Send.
Root cause of the GAP, not just the call: KernelScriptReferences built the Roslyn
reference set from AppDomain.CurrentDomain.GetAssemblies(), and a test process
always has MeshWeaver.Fixture loaded (MonolithMeshTestBase : Fixture.TestBase).
Its extensions sit in namespace MeshWeaver.Mesh, which the kernel imports for every
script. So script compilation in tests was strictly MORE permissive than in
production, and eleven tests that genuinely execute these templates all passed.
The reference set now excludes test scaffolding, so a script compiles in a test
exactly as it does in production and those eleven tests become real gates. The five
call sites move to the reactive Query<T> Initial-snapshot shape.
2) A FILLED-IN FORM CAME BACK EMPTY FROM CONSENT. Connecting Microsoft 365 is a
server-side endpoint, so the connect button must navigate with forceLoad, which
tears down the Blazor circuit — and the compose state lived in the layout area's
/data store, keyed by a Guid minted per render. Nothing was persisted, so there was
nothing to come back to.
The state is now a mesh node and the form binds directly to it via a $meshNode
DataContext: TextField, MeshNodePicker, TextArea and RadioGroup are unchanged and
route their writes through GetMeshNodeStream(path).Update, debounced, per field.
One source of truth — no /data replica, no save subscription.
The draft is filed under its AUTHOR ({userId}/_Draft/{documentKey}), not under the
shared document. A satellite under the document CANNOT be made private: permissions
are additive along the path and the Postgres listing folds on main_node, so every
reader of the document would inherit read on everyone's drafts through four
independent paths, and BreaksInheritance is not honoured by the SQL fold at all.
A draft holds recipient addresses and a personal message, so it goes where the
framework already makes per-user state private, alongside {userId}/_Settings and
{userId}/Feedback. It routes to the ordinary mesh_nodes table, as _Settings does —
a dedicated satellite table would need a migration across every existing partition
and buys no query shape.
Lifecycle: created once per (author, document) behind the permission gate, restored
on return, reset after 7 days untouched so a stale recipient cannot resurface,
dropped on successful send and on Cancel (the explicit discard).
3) THE CONNECT STEP CAME LAST. Being sent through consent only after filling the whole
form is the expensive order. The mailbox state is probed when the dialog opens and
the connect offer goes above the fields — an offer, not a gate.
Also: recipient suggestions now render "First Last <a@b.com>", so similarly-named
contacts can be told apart and the sender sees where the mail is going. Composing now
requires a signed-in identity. AGENTS.md's data-access table and the /code skill both
still recommended the dead QueryAsync — corrected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes three user-visible defects in “Share ⇒ as email” by (1) making runtime-compiled export templates compile against the true production API surface, and (2) persisting the compose state as a per-user mesh node so a Microsoft 365 consent round-trip (full page load) can’t wipe the form, plus small UX/i18n improvements around connect/sign-in and recipient disambiguation.
Changes:
- Replace template usage of test-only
QueryAsyncwith the production reactiveIMeshService.Query<T>(...)shape, and harden kernel script reference selection to exclude test-only assemblies. - Introduce an
EmailDraftnode type and rework the send layout area to bind the compose UI directly to the draft node (survives circuit teardown; discard on cancel/send; stale reset). - Add tests for draft privacy + survival, and for script-template compilation parity; update i18n catalogs and picker rendering to show emails.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Security.Test/EmailDraftPrivacyTest.cs | Verifies drafts are private to the author even when others can read the shared document. |
| test/MeshWeaver.Hosting.Monolith.Test/SendDocumentDraftTest.cs | Acceptance tests for draft persistence across consent teardown and discard semantics. |
| test/MeshWeaver.Hosting.Monolith.Test/ExportTemplateCompilationTest.cs | Compiles templates against production references and asserts test-only assemblies are excluded. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.en.json | Adds new send-dialog strings for connect-first and sign-in messaging (EN). |
| src/MeshWeaver.Messaging.Hub/Localization/strings.de.json | Adds new send-dialog strings for connect-first and sign-in messaging (DE). |
| src/MeshWeaver.Markdown.Export/Templates/ExportPdf.csx | Switches runtime template queries to production Query<T> + “Initial” snapshot filtering. |
| src/MeshWeaver.Markdown.Export/Templates/ExportHtml.csx | Same as above for HTML export template. |
| src/MeshWeaver.Markdown.Export/Templates/ExportDocx.csx | Same as above for DOCX export template. |
| src/MeshWeaver.Markdown.Export/Layout/SendDocumentLayoutArea.cs | Moves compose state to a draft node and updates connect/sign-in flow + discard behavior. |
| src/MeshWeaver.Markdown.Export/EmailDraft.cs | New persisted draft content model for the share-by-email form. |
| src/MeshWeaver.Markdown.Export/Configuration/MarkdownExportExtensions.cs | Registers the new EmailDraft node type as part of Markdown export setup. |
| src/MeshWeaver.Markdown.Export/Configuration/EmailDraftNodeType.cs | New node type + lifecycle helpers (ensure/create/reset/discard) for per-user drafts. |
| src/MeshWeaver.Kernel.Hub/MeshScriptEnvironment.cs | Adds IsTestScaffolding helper used to keep scripts on the production surface. |
| src/MeshWeaver.Kernel.Hub/KernelScriptReferences.cs | Filters Roslyn script references to exclude test scaffolding for prod parity. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-13-a-half-written-email-is-no-longer-lost-when-you-connect.md | What’s New entry describing the user-facing fixes and new guarantees. |
| src/MeshWeaver.Blazor/Components/MeshNodePickerView.razor.css | Styles the newly displayed email address in picker rows. |
| src/MeshWeaver.Blazor/Components/MeshNodePickerView.razor | Renders email addresses alongside names in the picker dropdown/selection. |
| content/ai/Skill/code.md | Updates script guidance away from QueryAsync and documents the production-surface rule. |
| clients/react/src/i18n/strings.en.json | Mirrors new send-dialog localization keys into the React catalog (EN). |
| clients/react/src/i18n/strings.de.json | Mirrors new send-dialog localization keys into the React catalog (DE). |
| AGENTS.md | Corrects the data-access guidance to reflect the production query surface and snapshot pattern. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| IsSatelliteType = true, | ||
| // A half-written mail is not content: it must never surface in search, in the | ||
| // create menu, or as agent context. | ||
| ExcludeFromContext = new HashSet<string> { "search", "create" }, |
Contributor
Contributor
Contributor
Test Results (shard 1)1 330 tests +28 1 330 ✅ +28 6m 19s ⏱️ -30s Results for commit 2ce2764. ± Comparison against base commit e4de912. This pull request removes 134 and adds 162 tests. Note that renamed tests count towards both. |
Contributor
Contributor
Contributor
Test Results (shard 2)2 116 tests - 22 2 115 ✅ - 22 7m 9s ⏱️ - 1m 12s Results for commit 2ce2764. ± Comparison against base commit e4de912. This pull request removes 156 and adds 134 tests. Note that renamed tests count towards both. |
Contributor
rbuergi
added a commit
that referenced
this pull request
Aug 12, 2026
… is gating ExportTemplateCompilationTest (added by #1275) went red on this branch for all three templates. It is NOT an access-control failure and nothing in this PR's diff can reach it — three independent proofs: * the test uses MonolithMeshTestBase's DEFAULT ConfigureMesh, which routes through ConfigureMeshBase → AddRowLevelSecurity(), so an evaluator IS registered and the new fail-closed branch cannot fire; * the sibling test in the SAME class on the SAME mesh passed (1 passed / 3 failed), so the mesh built and the MapMeshWeaver boot gate never ran; and * the diagnostics are CS0234 "the type or namespace name 'Export' does not exist in the namespace 'MeshWeaver.Markdown'" plus a cascade of CS0246 for DocumentBuilder / ExportFormat / RenderedDocument / BrandingResolver / DocxDocumentRenderer — a missing ASSEMBLY REFERENCE, not a permission. ROOT CAUSE. The script reference set is the process-global Lazy snapshot of AppDomain assemblies UNION the per-session KernelScriptAssembly contributions (MeshScriptEnvironment.SessionAssemblies). MeshWeaver.Markdown.Export is in neither the curated core set nor, for this fixture, the contributions — AddMarkdownExport() is what registers it, and that registration says exactly why in as many words: "Without this the export template .csx files can't resolve using MeshWeaver.Markdown.Export.* when AppDomain hasn't eagerly loaded the assembly before the first script run." A default MonolithMeshTestBase mesh never calls AddMarkdownExport, so the fixture was not reproducing production — it was reproducing a portal that does not have the export feature installed, and the templates failed to resolve their OWN namespace. WHY IT EVER PASSED, AND WHY IT SURFACED HERE. The snapshot is a process-global Lazy, so the assembly was in the set iff something else in the same test process had already loaded it when the snapshot was first materialized — green or red by which class ran first in the shard. That is a latent order-dependency in the new test, not a property of any branch; this branch adds test classes to two other projects, which is enough to change what runs first. Registering the module removes the luck: SessionAssemblies is recomputed per call and never memoized. Proven, not assumed — same filter, same process, only the mesh config differs: without AddMarkdownExport → Failed: 3, Passed: 1 with AddMarkdownExport → Failed: 0, Passed: 4 Also: the assertion reported only a COUNT ("found 40 item(s)") and never the diagnostics, so the one thing the failure exists to communicate — which API a runtime-compiled script cannot resolve — could not be read from CI at all. The message now names them. No gate was softened, no permission special-cased, and the export path is not special-cased: the fixture is given the real production configuration for the feature under test. Co-Authored-By: Claude Fable 5 <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.
Three defects in Share ⇒ as email, all user-visible. Reported from the live portal.
1. 🚨 Sending was broken in production
The user hit this on send:
The three export templates are
.csx— compiled at runtime by the kernel, invisible todotnet build— and they calledmeshService.QueryAsync<MeshNode>(…), which exists only inMeshWeaver.Fixture, a test-only assembly whose own doc comment reads "Production code MUST NOT use these". Five call sites acrossExportPdf.csx,ExportHtml.csx(which the initial report missed) andExportDocx.csx. Every PDF / DOCX / email export died on the first press of Send.The CI gap — and why it is now closed at the root
Eleven tests genuinely compile and execute these templates, and all eleven were green. The reason:
KernelScriptReferencesbuilt its Roslyn reference set fromAppDomain.CurrentDomain.GetAssemblies(). A test process always hasMeshWeaver.Fixtureloaded (MonolithMeshTestBase : Fixture.TestBase), and its extensions sit in namespaceMeshWeaver.Mesh— which the kernel imports for every script. Script compilation in tests was strictly more permissive than in production, so a template could bind an API no deployed portal has and still pass.The fix is not a new test bolted on top — it is that the reference set now excludes test scaffolding (
MeshScriptEnvironment.IsTestScaffolding), making script compilation in a test identical to production. Every one of those eleven tests becomes a real gate. Two new tests pin it directly:ExportTemplates_CompileAgainstTheProductionSurface— compiles all three templates, including branches (deck query, descendants) no runtime test reaches.ScriptReferenceSet_ExcludesTestOnlyAssemblies— asserts the Fixture is absent from the reference set while provably loaded in the process, so it cannot pass by luck.Call sites move to the reactive
Query<T>shape, filtering on the emission shape (ChangeType == Initial), never a count.AGENTS.md's data-access table and the/codeskill both still recommended the dead API to script authors — corrected in the same change.2. A filled-in form came back empty from consent
Connecting Microsoft 365 is a server-side endpoint, so the connect button must navigate with
forceLoad: true(#1248) — which tears down the Blazor circuit. The compose state lived in the layout area's/datastore under aGuidminted per render. Nothing was persisted, so there was nothing to come back to; thereturnUrlreturned people to a blank form, which reads as the feature simply losing the work.The state is now a mesh node, and the form binds directly to it through a
$meshNodeDataContext. Every control is unchanged —TextField,MeshNodePicker,TextArea,RadioGroupall already route writes throughFormComponentBase → BlazorView.UpdatePointer → GetMeshNodeStream(path).Update, debounced, per field. One source of truth; no/datareplica and no save subscription (the forbidden replicate-then-save antipattern).🚨 Where the draft lives — a deliberate deviation from the brief, with evidence
The brief asked for a satellite under the document (
{doc}/_Draft/…). That placement cannot be made private, and a draft holds recipient addresses and a personal message. Access is additive along the path (PermissionEvaluatorunions every ancestor scope), the stream-cache gate probes the PATH, and the Postgres listing folds visibility onmain_node— so every colleague who can read the document would inherit read on everyone's drafts, through four independent read paths. There is no per-node owner flag and no restricting assignment;PartitionAccessPolicy.BreaksInheritance— the closest thing — is not honoured by the SQL fold at all, so search would still leak.ThreadComposerNodeType.PathForNodeis the live precedent of exactly this leak today.So the draft is anchored under its author:
{userId}/_Draft/{documentKey}— the framework's actual per-user privacy mechanism, shared with{userId}/_Settings/…,{userId}/_UserActivity/…and{userId}/Feedback/…. Privacy is structural, not asserted. It is still a satellite under a real owner (never top-level) and still names its document, in the path key and inEmailDraft.DocumentPath.Table:
_Draftis deliberately not registered inSatelliteTableMapping.Defaults, so it routes to the partition's ordinarymesh_nodes— exactly as_Settingsdoes. Right answer here: drafts are few, per-user, read only by exact path. A dedicated satellite table would require a migration across every already-provisioned partition (V26_AddNotificationsSatelliteTableshape, since provisioning is promise-cached and never re-run) and buys no query shape.Lifecycle — created once per (author, document) behind the permission gate, restored on return, reset after 7 days untouched so a stale recipient cannot resurface weeks later, and dropped on successful send and on Cancel (the explicit discard). Abandoning any other way — closing the tab, going through consent — deliberately keeps it.
3. The connect step came last
The mailbox state is probed when the dialog opens, and the connect offer is rendered above the fields. It stays an offer, not a gate: the form is fully usable and sending without connecting still goes from the shared mailbox with a reply-to. The just-in-time prompt survives for consent that lapses mid-compose — belt and braces, and now harmless either way because the draft is already on its node.
Also
First Last <a@b.com>— one place,MeshNodePickerView, off the content the query already returns; no contract change. React clients render the picker as a bare input, so nothing to mirror there.Tests
Draft_SurvivesTheConsentRoundTripReaderOfTheDocument_CannotReadAnotherUsersDraftAllon everything; it now runs with granular permissions.Author_ReachesTheirOwnDraft_WithoutAnyGrantDraftPath_IsAnchoredUnderTheAuthor_NotTheDocumentDraft_IsDiscardedAfterSending_AndDiscardIsIdempotentExportTemplates_CompileAgainstTheProductionSurface+ScriptReferenceSet_ExcludesTestOnlyAssembliesLocal: 18/18 in
MeshWeaver.Hosting.Monolith.Test(including the 7 real export tests, which now execute the templates against a Fixture-free reference set) and 3/3 inMeshWeaver.Security.Test. Touched projects build clean under-c Release -warnaserror. i18n keys added to both server catalogs and the React vendored copies (900 keys each, no missing German).🤖 Generated with Claude Code