fix(import): a content sync is many bounded deliveries, never one whole tree (#2885) by rbuergi · Pull Request #3097 · Systemorph/MeshWeaver

rbuergi · 2026-09-02T10:50:47Z

Not a duplicate of #3056 — this is the residual #3056 itself named, and it recurred

First question asked, because a duplicate remedy already cost time today: does #2885 still reproduce on current main? It does. Re-running the issue's own measurement against the live incident record rather than the recorded numbers:

Admin/_LogIncident/752d9cffb21d01c0   occurrences: 1 -> 2
  2026-08-31 05:52:32Z  memex-portal-deployment-9bf79b774-tk88b   import/xDAfkqsVUE-OMBHb0mVtSg
  2026-09-02 03:08:13Z  memex-portal-deployment-85859db964-7sqbk  import/W0k29Ro4Ok22GCYjxFBiHQ   <- NEW

A second pod, a second import hub, inside the same burst window as #3044-#3049. #3056's own PR body says so explicitly — "only the producer closes it, and that is #2885's named residual: the producer must not build the payload whole" — and #3046 was re-scoped to exactly that work and is still open with no PR. This is that work.

What was building the payload whole

The stacks name a target (import/{meshHubId}) and never a message type, which is why this stayed open across four incidents. The identification is arithmetic, not inference.

The trigger. AgenticBusiness/_GitSync was written at 2026-09-02 02:49:58Z; the burst starts five minutes later. That is a forced GitSync re-import — the event the first measurement on this issue said a quiet six-hour window could not rule out.

The message. A GitSync import mirrors a Space's git-committed content/** binaries with SyncContentFilesRequest, whose Files carry the raw bytes inline (deliberately — binary assets must never round-trip through a text API). ContentAssetMapper.ToContentSyncs emits one StaticContentSync per Space (also deliberately — the mirror is meant to be one authoritative pass), and SyncContentFilesBuilder.Post turned that one group into one message. The delivery's size was the size of the Space's entire asset tree.

The arithmetic, measured off Systemorph/MeshWeaver.Education (the repo AgenticBusiness syncs from):

Space	`content/**`	base64	UTF-16 `RawJson` string	transcode rent (≤3 B/char)
`AgenticBusiness`	28,484,421 B (10 files)	~38 MB	~76 MB	~114 MB
`AgenticEngineering`	106,070,300 B (12 videos)	~141 MB	~283 MB	~424 MB

AgenticBusiness is comfortably under MaxMessageBodySize — which is precisely why every bound in this family (#1890, #2897, #3018/#3032) let it through, and why #3046 could correctly observe "the leg is guarded and the delivery went through anyway". The rent is what failed.

AgenticEngineering is the same defect at the other end, and it is not an OOM at all: its base64 form exceeds the 100 MiB frame limit outright, so once the producer-side refusals landed that Space's assets stopped syncing entirely and silently. A bound is a diagnosis, not a remedy — and nobody had checked what the refused producer does next.

The fix — chunk the writes, keep the prune one pass, put it first

Not a bound. An OOM during serialisation means the allocation was the failure, so the producer stops making it.

SyncContentFilesBuilder.Post partitions the files against DeliveryPayloadBounds.MemoryStreamBlockBytes — the tighter of the two transport ceilings this codebase already declares, and the one a failure report about a delivery must itself survive — measured on the base64 form, because that is what the message weighs. Posted with Concat, so the pod holds one at a time. A file is never split (it is the atom the receiving handler writes), so the guarantee is delivery ≤ budget + largest single file: the delivery's size stops being a function of how much content the Space holds.
The prune does not chunk. It asks "what is under this folder that the source no longer carries", and a chunk answering that from its own slice would delete the other chunks' files. SyncContentFilesRequest gains MirrorKeepPaths — the full keep set as paths, which is all the prune ever needed and which costs nothing — and exactly one delivery mirrors.
That delivery is the FIRST, not the last. This is the safety property, not a detail: were the keep set ever not honoured by a receiver, the files it would wrongly prune are precisely the ones the following deliveries are about to write, so the operation still converges on the correct folder. Last-position pruning has no such property.
An unsplit sync is byte-for-byte what it was — one request, MirrorKeepPaths null, mirror measured against its own Files.

Both producers funnel through the one builder, so PackageInstaller.SyncPackageContent is fixed by the same change.

Verification that could have failed

Measured with only ContentImportExtensions.cs reverted (the request field kept, so the tests still compile and the assertions are on behaviour, not on a type that does not exist):

Fact	Producer reverted	Here
`A_sync_larger_than_one_delivery_is_split`	FAIL — 1 delivery, 3,200,144 packaged bytes	pass
`Exactly_one_delivery_carries_the_mirror_and_it_names_the_whole_set`	FAIL — 1 delivery	pass
`A_sync_that_fits_is_still_one_delivery` (control)	pass	pass
`A_split_mirror_writes_everything_prunes_the_stale_and_keeps_the_upload` (control, real handler + real filesystem collection)	pass	pass

The second control is deliberately one that passes both ways: it is what makes the split safe to make, and it is the assertion that fails if the chunks are ever allowed to prune one another.

Full projects, unfiltered: MeshWeaver.ContentCollections.Test 22/22, MeshWeaver.Graph.Test 1205/1205, MeshWeaver.Documentation.Test 193/193. dotnet build -c Release -warnaserror → 0 Error(s) on MeshWeaver.Mesh.Contract, MeshWeaver.ContentCollections, MeshWeaver.Graph, MeshWeaver.PluginCatalog, MeshWeaver.GitSync, MeshWeaver.Hosting, MeshWeaver.Documentation and the touched test project.

What this deliberately does NOT close — stated rather than implied

A single file larger than the budget still travels whole. The largest asset in the Education repo is a 9.9 MB video → ~13 MB base64 → ~40 MB rent. Survivable, but not bounded. A file that large belongs behind a content-store handle rather than inline; that is a different defect and is not conflated with this one.
🚨 DeliveryFailure's strip does not fire on a TYPED payload. DeliveryPayloadBounds.IsOversized returns false for anything that is not RawJson — by design, since sizing a CLR object would mean serialising it on the hot path. But a NACK is very often built on the receiving hub, where the message has already been deserialised into its CLR type, so DeliveryFailure for a failed RawJson message is lost — ReportFailure throws OutOfMemoryException while serializing the notification #3044's construction invariant silently does not apply there and the body is echoed back verbatim. On this path the producer fix makes that echo at most one chunk; as a general property of the invariant it remains. Found while tracing which inbound body at import/{id} could be large — worth knowing before relying on the strip.
Other unbounded inbound bodies at import/{id} are smaller but real and now recorded in the doc: CreateNodesResponse echoes up to 25 full MeshNodes the caller reads only Path off, CreateOrUpdateNodeResponse echoes the node, DeleteNodeResponse carries every deleted path.

Docs

Doc/Architecture/OversizedDeliveryRefusal gains "The producer that was building it whole" — the trigger, the message, the arithmetic table, the fix and the three residuals above — replacing the section that said the producer axis was untouched. Four new rules, including "a stack that names only a target address is still identifiable — by arithmetic" and "a refusal that lands on an unfixed producer converts an OOM into a silent total failure". Doc/Architecture/StaticRepoImport gains the split. What's New: Category: Fix.

Closes #2885
Closes #3046

Pairs-with: none — no public top-level type is removed; SyncContentFilesRequest.MirrorKeepPaths is an addition.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PPe8CH4mFU9E74HafWppqL

…le tree (#2885) The producer residual of #2885 — the one axis four bounds could not reach. A GitSync import mirrors a Space's git-committed content/** binaries with SyncContentFilesRequest, whose Files carry the raw bytes INLINE. ContentAssetMapper.ToContentSyncs emits one group per Space (the mirror has to be one authoritative pass) and SyncContentFilesBuilder.Post turned that group into ONE message — so the delivery's size WAS the size of the Space's asset tree: AgenticBusiness 28,484,421 B (10 files) -> ~38 MB base64 -> ~76 MB UTF-16 -> ~114 MB rent AgenticEngineering 106,070,300 B (12 videos) -> ~141 MB base64, over the 100 MiB frame limit The first is what threw OutOfMemoryException at SharedArrayPool.Rent under Utf8JsonWriter.TranscodeAndWriteRawValue on memex-cloud, twice (2026-08-31 05:52Z, 2026-09-02 03:08Z), five minutes after AgenticBusiness/_GitSync was written. It sat UNDER MaxMessageBodySize, which is exactly why #1890/#2897/#3018/#3032 let it through and #3046 could observe "the leg is guarded and it went through anyway". The second is the same defect at the other end and is not an OOM at all: once the producer-side refusals landed, that Space's assets could not sync at all — silently. An OOM DURING serialisation means the allocation was the failure, so no bound helps. The producer stops making it: * Post() partitions the files against DeliveryPayloadBounds.MemoryStreamBlockBytes — the tighter of the two transport ceilings the mesh already declares, measured on the base64 form because that is what the message weighs — and posts with Concat, so the pod holds one at a time. A file is never split (it is the atom the handler writes), so the guarantee is delivery <= budget + largest single file: the size stops being a function of how MUCH content the Space holds. * The prune does not chunk. SyncContentFilesRequest gains MirrorKeepPaths — the full keep set as PATHS, which is all the prune ever needed — and exactly one delivery mirrors. It is the FIRST, not the last: were the keep set ever not honoured, the files it would wrongly prune are precisely the ones the following deliveries are about to write, so the operation still converges. Last-position pruning has no such property. * An unsplit sync is byte-for-byte what it was: one request, MirrorKeepPaths null, the mirror measured against its own Files. Both producers go through the one builder, so PackageInstaller.SyncPackageContent is covered by the same change. Verification that could have failed — measured with only ContentImportExtensions.cs reverted (the request field kept, so the tests still compile): A_sync_larger_than_one_delivery_is_split FAIL (1 delivery, 3,200,144 B) -> pass Exactly_one_delivery_carries_the_mirror_and_it_names_the_whole_set FAIL (1 delivery) -> pass A_sync_that_fits_is_still_one_delivery (control) pass -> pass A_split_mirror_writes_everything_prunes_the_stale_and_keeps_the_upload (control) pass -> pass Full projects: MeshWeaver.ContentCollections.Test 22/22, MeshWeaver.Graph.Test 1205/1205, MeshWeaver.Documentation.Test 193/193. dotnet build -c Release -warnaserror clean (0 Error(s)) on MeshWeaver.Mesh.Contract, MeshWeaver.ContentCollections, MeshWeaver.Graph, MeshWeaver.PluginCatalog, MeshWeaver.GitSync, MeshWeaver.Hosting, MeshWeaver.Documentation and the touched test project. Docs: Doc/Architecture/OversizedDeliveryRefusal gains "The producer that was building it whole" with the arithmetic, what the fix still does not cover (a single file over budget; DeliveryFailure's strip not firing on a TYPED payload, since IsOversized only recognises RawJson and a NACK is usually built where the message is already typed), and four new rules. StaticRepoImport gains the split. What's New: Category: Fix. Closes #3046 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPe8CH4mFU9E74HafWppqL

Copilot

🟡 Changes recommended

A few newly added lines are misleading (“costs nothing”) and the new tests should use the repo-standard observable await helper for consistency and cancellation wiring.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes the remaining producer-side cause of oversized SyncContentFilesRequest deliveries by batching inline content files into multiple bounded deliveries, while keeping mirroring/pruning as a single authoritative pass (carried by the first delivery) using a new MirrorKeepPaths field.

Changes:

Split SyncContentFilesBuilder.Post() into sequential, budgeted deliveries (Concat) and fold per-delivery responses into one aggregated ImportContentResponse.
Add SyncContentFilesRequest.MirrorKeepPaths so the (single) mirror/prune pass can compare against the full keep-set even when writes are chunked.
Add regression tests plus documentation/What’s New entries describing the failure mode and the batching fix.

File summaries

File	Description
test/MeshWeaver.ContentCollections.Test/ContentSyncIsNeverBuiltWholeTest.cs	New tests pin the “never build one delivery whole” batching behavior and the single-pass mirror semantics.
src/MeshWeaver.Mesh.Contract/ImportDeleteRequests.cs	Extends `SyncContentFilesRequest` with `MirrorKeepPaths` to support split-write + single-prune behavior.
src/MeshWeaver.ContentCollections/ContentImportExtensions.cs	Implements file batching, first-delivery pruning, and keep-set handling in the content sync producer/handler.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-course-videos-and-images-sync-again.md	User-facing release note describing restored large-asset syncing via batching.
src/MeshWeaver.Documentation/Data/Architecture/StaticRepoImport.md	Architecture doc update explaining that one sync becomes many deliveries and how prune is handled.
src/MeshWeaver.Documentation/Data/Architecture/OversizedDeliveryRefusal.md	Architecture doc update capturing producer identification, arithmetic, and the chunking fix.

Review details

Suppressed comments (3)

test/MeshWeaver.ContentCollections.Test/ContentSyncIsNeverBuiltWholeTest.cs:169

In this test project the convention is to await observables via .Await(TestContext.Current.CancellationToken) so cancellation is explicit and the Rx→Task bridge stays in the approved helper. Update this await to match the surrounding tests in this repo.

            .Post()
            .Timeout(TestTimeouts.Convergence)
            .FirstAsync();

test/MeshWeaver.ContentCollections.Test/ContentSyncIsNeverBuiltWholeTest.cs:203

This observable is being awaited without the repo-standard .Await(TestContext.Current.CancellationToken) helper. Using the helper keeps cancellation behavior consistent across tests and avoids relying on implicit awaiters.

            .Post()
            .Timeout(TestTimeouts.Convergence)
            .FirstAsync();

test/MeshWeaver.ContentCollections.Test/ContentSyncIsNeverBuiltWholeTest.cs:269

This uses a direct await on the observable. Tests in this repo consistently use .Await(TestContext.Current.CancellationToken) for observable awaits so cancellation is explicit and the bridging behavior is centralized.

            .Post()
            .Timeout(TestTimeouts.Convergence)
            .FirstAsync();

Files reviewed: 6/6 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+So `SyncContentFilesRequest` gains `MirrorKeepPaths`: the full keep set as **paths**, which is all
+the prune ever needed and which costs nothing. Exactly one delivery mirrors — **the first**, not the


+    /// But the prune is not chunkable: it asks "what is under this folder that the source no longer
+    /// carries", and a chunk that answered it from its own slice would delete the other chunks'
+    /// files. So the prune stays ONE authoritative pass and is handed the whole set — as PATHS,
+    /// which is all it ever needed and which costs nothing.</para>


+            .Post()
+            .Timeout(TestTimeouts.Convergence)
+            .FirstAsync();


github-actions · 2026-09-02T10:55:16Z