Repository navigation
fix(#3233): a content file over the per-delivery budget travels out of band - #3286
Conversation
…f band #2885 partitions a content sync so a delivery is `<= budget + largest single file`, which fixed every Space whose AGGREGATE was the problem. A file is the atom the receiving handler writes and is never split, so a file whose packaged (base64) cost ALONE exceeds the budget travelled whole and was refused wherever the Orleans transport was in the path. Re-measured 2026-09-04 on MeshWeaver.Education@f7ae723: still 25 files across all 7 Spaces, unchanged. The budget is Orleans' memory-stream block size, hard-coded with no configuration surface, and raising it is the move OversizedDeliveryRefusal exists to forbid. Such a file now goes AROUND the message. Its bytes are written into the destination collection's reserved `_staging/` folder before the request is posted, and the delivery carries a content-addressed StagedContentFile handle; the receiver streams the blob into its destination path. Where the bytes land: the destination collection itself, reached by resolving its CONFIG from the owning node's hub (a few hundred bytes) — the mechanism MeshOperations.Upload and the /api/content route already use. The content store is already required to be reachable from a hub other than the collection's owner, so this adds no assumption, no well-known name, no deployment key and nothing to provision. Lifetime: the producer owns every blob it stages and deletes them IN the post chain, so the answer means "the staging area is clean"; only a crashed producer leaves residue, reclaimed by an age sweep (24 h) that runs before staging and therefore cannot see a live transfer. The mirror never prunes `_staging/` (excluded by name) and never prunes a staged file (its path is in the full MirrorKeepPaths keep set) — without both, the prune, which rides the FIRST delivery, would delete the blobs the following deliveries still have to read. Idempotence: the handle is the SHA-256 of the bytes. Two identical files stage once; a re-run addresses the same blob and the same destination path, so nothing is duplicated and an already-present blob is not rewritten. Failure stays honest (#3101 is not undone): - a handle the receiver cannot resolve, or whose length does not match, FAILS by name and writes nothing — never "zero files", never a truncated asset; - when staging cannot run, the file falls back to inline (a monolith carries it fine) and the refusal names BOTH halves: the #3101 budget measurement over the files that actually travelled inline, plus why the out-of-band road was unavailable; - DescribeOverBudget is now measured on the INLINE remainder, so a file that went out of band is not reported as an over-budget payload nobody built. Tests (MeshWeaver.ContentCollections.Test, real handler + real FS collection): the over-budget file arrives byte-for-byte and is on NO delivery; every delivery fits the budget outright; a re-run duplicates nothing and both runs address one handle; the mirror keeps it, prunes the stale, preserves a user upload; staging that cannot run reports both halves; an unresolvable handle fails by name and writes no empty file. Reverting the staging decision fails 3 of the 5. Design committed as Doc/Architecture/OutOfBandContentTransfer; OversizedDeliveryRefusal's residual and ContentSyncVisibility updated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…owed The reclaim's per-blob Catch continued past a failed delete — correct, so one stubborn blob cannot strand the rest or fail a sync that succeeded — but it swallowed the exception with nothing written down, which is exactly the silent-swallow shape AGENTS.md forbids. What it leaves behind is a blob on the content share that only the age sweep will now collect, so it says so. Both reclaim paths now take the builder's logger: Discard warns per blob it could not delete, and SweepStale logs what it reclaimed (Information — crash residue is rare and worth a line), warns on a blob it could not, and reports at Debug when the staging folder cannot be enumerated at all. The outer catch in Reclaiming stays as the defensive arm it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Two correctness/operability issues remain in the new staged-write and reclaim paths (unknown-size GetContentSize == -1 handling, and silent reclaim failures) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes content sync failures for single binary assets whose base64-packaged size exceeds the per-delivery payload budget by transferring those oversized files out-of-band through the destination content store’s reserved staging folder (_staging/), while keeping existing delivery chunking/mirroring semantics for the remaining inline files.
Changes:
- Add a content-addressed staged-file handle (
StagedContentFile) and plumb it throughSyncContentFilesRequestasStagedFiles. - Implement producer-side staging + receiver-side streamed write for oversized files, and ensure mirror pruning keeps staged destinations and never touches
_staging/. - Add end-to-end tests plus architecture/docs/What’s New updates describing the design, lifecycle, and failure contract.
File summaries
| File | Description |
|---|---|
| test/MeshWeaver.ContentCollections.Test/OversizedContentTravelsOutOfBandTest.cs | New integration tests covering out-of-band transfer, idempotence, mirror/prune behavior, and failure reporting. |
| src/MeshWeaver.Mesh.Contract/MeshExtensions.cs | Registers StagedContentFile in the type registry for correct typed deserialization across hubs. |
| src/MeshWeaver.Mesh.Contract/ImportDeleteRequests.cs | Introduces StagedContentFile and adds StagedFiles to SyncContentFilesRequest. |
| src/MeshWeaver.ContentCollections/ContentStaging.cs | Adds staging folder constants and producer-side staging/sweep/reclaim helpers. |
| src/MeshWeaver.ContentCollections/ContentImportExtensions.cs | Adds receiver-side staged-handle resolution + write path, updates mirroring keep-set and prune exclusions, and stages oversized files before posting deliveries. |
| src/MeshWeaver.ContentCollections/ContentDeliveryBudget.cs | Updates budget documentation and clarifies that over-budget measurement applies only to files that actually traveled inline. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-04-course-videos-sync-again.md | What’s New entry describing the user-visible fix. |
| src/MeshWeaver.Documentation/Data/Architecture/OversizedDeliveryRefusal.md | Updates prior “residual” section to point to out-of-band transfer as the closure. |
| src/MeshWeaver.Documentation/Data/Architecture/OutOfBandContentTransfer.md | New architecture page documenting the design, lifecycle, idempotence, and failure behavior. |
| src/MeshWeaver.Documentation/Data/Architecture/ContentSyncVisibility.md | Updates visibility doc to reflect the new transfer path and preserved honesty contract. |
| src/MeshWeaver.Documentation/Data/Architecture.md | Adds the new architecture page to the Reliability & wedges index. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return target.GetContentSize(blob) | ||
| .SelectMany(size => size switch | ||
| { | ||
| null => Observable.Throw<int>(new InvalidOperationException( | ||
| $"Staged content '{staged.Handle}' for '{staged.Path}' is not in collection " | ||
| + $"'{target.Collection}' staging area ('{blob}') — the out-of-band transfer " | ||
| + "did not complete, so the file was NOT written.")), | ||
| >= 0 when size != staged.Length => Observable.Throw<int>(new InvalidOperationException( | ||
| $"Staged content '{staged.Handle}' for '{staged.Path}' is {size:N0} bytes but " | ||
| + $"the handle declares {staged.Length:N0} — refusing to write a truncated asset.")), | ||
| _ => target.GetContent(blob) | ||
| .SelectMany(stream => stream is null | ||
| ? Observable.Throw<int>(new InvalidOperationException( | ||
| $"Staged content '{staged.Handle}' for '{staged.Path}' disappeared from " | ||
| + $"'{blob}' between the probe and the read, so the file was NOT written.")) | ||
| : target.SaveFile(dir, name, () => stream).Select(_ => 1)) | ||
| }); |
There was a problem hiding this comment.
Correct, and the contract was the part that was wrong rather than the branch. Refusing on -1 would reject content that is almost certainly intact on every non-seekable store, for ever — so the write still proceeds, but it is no longer silent: CopyStaged logs a warning naming the handle, the path and the declared length when a write goes out unverified, and both the StagedContentFile.Length xmldoc and Doc/Architecture/OutOfBandContentTransfer now state the limit instead of implying a guarantee. Fixed in f946f07. (Every file-system-backed collection reports a size, which is every store a content sync writes to today — the two failing shapes, a missing blob and a genuine mismatch, still throw by name.)
| internal static IObservable<Unit> Discard(ContentCollection destination, IEnumerable<string> handles) | ||
| { | ||
| var blobs = handles.Distinct(StringComparer.OrdinalIgnoreCase) | ||
| .Select(h => ContentStaging.BlobPath(h)) | ||
| .ToArray(); | ||
| return blobs.Length == 0 | ||
| ? Observable.Return(Unit.Default) | ||
| : blobs | ||
| .Select(blob => destination.DeleteFile(blob) | ||
| .Catch<Unit, Exception>(_ => Observable.Return(Unit.Default))) | ||
| .Concat() | ||
| .DefaultIfEmpty(Unit.Default) | ||
| .LastAsync(); | ||
| } |
There was a problem hiding this comment.
Agreed — that was a silent swallow, and it was fixed in 49e8f7d (pushed before this review landed). Discard and SweepStale both take the builder's logger now: a blob that will not delete is logged as a warning naming the blob and the collection, a reclaimed stale blob logs at Information, and a staging folder that cannot be enumerated logs at Debug. Continuing past a failed delete is still deliberate — one stubborn blob must not strand the rest or turn a successful sync into a failure — but what it leaves behind is now visible rather than inferred.
The receiver's contract claimed StagedContentFile.Length is "VERIFIED before the file is written", and for the two failing shapes it is: a missing blob and a size mismatch each throw by name. But GetContentSize answers -1 for a store that HAS the blob and cannot report its size, and that fell through to the write arm — so the guarantee quietly did not hold on such a store, and nothing said which case a given write was. Refusing on -1 would reject content that is almost certainly intact, on every non-seekable store, for ever — so the write still proceeds. What changes is that it is no longer silent: the copy is split out and logs a warning naming the handle, the path and the declared length when it goes out unverified. The record's xmldoc and Doc/Architecture/OutOfBandContentTransfer now state the limit instead of implying a guarantee. Every file-system-backed collection reports a size, which is every store a content sync writes to today. Found by the automatic Copilot review on #3286; its second finding (Discard swallowing per-blob delete failures) was already addressed in 49e8f7d. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Status: this change is green; the merge is blocked on fleet state it cannot reachEverything this PR is responsible for passes. On The two reds are one red.
That is the cross-repo halves shape: the satellite halves merged, and core's half is #3287 ( Not fixing it here, deliberately. Declaring What this PR needs to merge, once #3287 is on On the earlier red (
|
Closes #3233
The residual, re-measured
A GitSync content sync carries a Space's
content/**binaries inline. #3097/#2885 partitions thewrite so a delivery is
≤ budget + largest single file— which fixed every Space whose aggregatewas the problem. A file is the atom the receiving handler writes and is never split, so a file
whose packaged (base64) cost alone exceeds
ContentDeliveryBudget.BudgetBytes(= Orleans'MemoryStreamBlockBytes, 1,048,576, hard-coded with no configuration surface) travelled whole andwas refused wherever the Orleans transport was in the path.
Re-ran the issue's own cost function on
Systemorph/MeshWeaver.Education@f7ae723(2026-09-04) —unchanged, 25 files over budget across all 7 Spaces:
The axis is "has a video", not "is large" —
AdvancedBusinessRulesis one of the smallest Spacesand carries the second-largest single file.
The design
Out-of-band asset transfer. A file over the budget no longer rides the message: its bytes are
written into the destination collection's reserved
_staging/folder before the request isposted, and the delivery carries a content-addressed
StagedContentFile(Path, Handle, Length)handle. The receiver streams the blob into the destination path exactly where the inline file would
have gone.
Where the bytes land — the destination collection itself. The producer reaches it the way
MeshOperations.Uploadand the/api/contentroute already do: it resolves the collection'sconfig from the owning node's hub (a few hundred bytes) and writes through it locally. The
content store is already required to be reachable from a hub other than the collection's owner —
that is what the content route does — so this adds no assumption, and needs no well-known
collection name, no deployment key and nothing new to provision. A mesh-level staging store would
have needed all three for the same physical bytes on the same share.
Lifetime. The producer owns every blob it stages and deletes them in the post chain, after
the last delivery is answered (which is exactly when nothing can still name a handle) and before the
caller's answer — so "this sync answered" means "its staging area is clean", not a race a caller has
to poll. Only a crashed producer leaves residue; that is reclaimed by an age sweep (24 h) that runs
before staging and therefore cannot see a live transfer. The mirror never prunes
_staging/(excluded by name — it is framework state) and never prunes a staged file (its path is in the full
MirrorKeepPathskeep set, now mandatory whenever anything is staged, not only when the write issplit). Without both, the prune — which rides the first delivery — would delete the blobs the
following deliveries still have to read.
Idempotence. The handle is the SHA-256 of the bytes. Two identical files stage once; a re-run
addresses the same blob at the same key and the same file at the same destination path, so nothing is
duplicated, and a blob already present with the right length is not rewritten.
Failure behaviour stays honest — #3101 is not undone
handle and the path. Never "zero files"; never a truncated or empty asset.
today's behaviour, including today's refusal — and the failure names both halves: the A Space whose content sync is refused stops syncing SILENTLY — AgenticEngineering's assets are over the frame limit outright #3101
budget measurement and why the out-of-band road was unavailable.
ContentDeliveryBudget.DescribeOverBudgetis now measured on the files that actually travelledinline, so a file that went out of band is not reported as an over-budget payload nobody built.
StaticRepoImporteris untouched: a failed transfer still landsWarningwith its reason on theSpace's
_Activity/content-syncledger, and the fingerprint still does not stampSucceeded.What did not change
The budget is untouched, the inline partitioning of #2885 is untouched, a file is still never split
(no chunking, no reassembly, no partial-write story), and authorization is unchanged — the content
write still happens on the owning node's hub under
[SyncContentFilesPermission], and nothing landsin the collection before that check runs. A sync with no over-budget file produces a request
byte-for-byte identical to before (no config round-trip, no staging pass,
StagedFilesnull).Tests — and what they do without the implementation
test/MeshWeaver.ContentCollections.Test/OversizedContentTravelsOutOfBandTest.cs, driven through thereal
SyncContentFilesRequesthandler over a real file-system collection (no mocking, noTask.Delay):A_file_over_the_budget_travels_out_of_band_and_arrives_intact— the file arrives byte-for-byte(SHA-256 compared), its bytes are on no delivery, exactly one staged handle equals the content
hash, every delivery fits the budget outright (no "plus one file" allowance), staging is clean.
A_rerun_duplicates_nothing_and_leaves_no_residue— two runs, one folder, both runs address thesame handle, no residue.
The_mirror_keeps_the_out_of_band_file_prunes_the_stale_and_preserves_an_upload— the transfersurvives the mirror in the same operation, the retired source file is pruned, the user upload
(Silent loss of uploaded collection files after hub teardown; mesh search returns zero results on deployed portal #435) survives, one prune pass, the staged path is in
MirrorKeepPaths.Staging_that_cannot_run_falls_back_inline_and_the_failure_names_both_halves.A_handle_nothing_staged_fails_by_name_and_writes_nothing.Revert experiment (disable the staging decision, rebuild
src/and the test project):3 of 5 fail, with exactly the pre-fix evidence —
Did not expect collection {…, "videos/module1-intro.mp4", …} to contain "videos/module1-intro.mp4",Expected collection to have 2 item(s) … but found 0, and the #3101 sentence with the #3233 halfmissing. Restored: 27/27 in that project, 9/9 on the Graph content-sync tests, 250/250 on
the Documentation guard suite,
DocumentationLinkIntegrityTestgreen.Built
-c Release -warnaserror, one project per invocation,0 Error(s) / 0 Warning(s):MeshWeaver.Mesh.Contract,MeshWeaver.ContentCollections,MeshWeaver.Documentation,MeshWeaver.Graph,MeshWeaver.PluginCatalog,MeshWeaver.Mesh.Operations,MeshWeaver.Hosting.AspNetCore,MeshWeaver.Connection.Orleans, and both test projects.Docs
Doc/Architecture/OutOfBandContentTransfer— the design, the lifetime rules, theidempotence argument, and the failure contract.
Doc/Architecture/OversizedDeliveryRefusal— its own "a single file larger than the budget travelswhole" residual is now closed and points here.
Doc/Architecture/ContentSyncVisibility— records that its observability contract is what the newtransfer's fallback rules preserve.
Category: Fix): Course videos sync again.No public type or member is removed — additions only — so no
Pairs-with:counterpart is needed.🤖 Generated with Claude Code