Repository navigation
fix(#3401): the module lane reads ONE publication, and a moved one is re-read rather than lost - #3460
Conversation
… re-read rather than lost #3409 gave the BUNDLE lane a generation and turned the republish window into 503. Two gaps were left, and both still red a satellite for a defect that is not in its diff: * the MODULE lane read UNPINNED in both consumers (compose-sealed-modules.sh and `memex build plugin`), so a reseal between the module-set index and a module fetch silently composed bytes from two publications — the mvid mismatch that DECLINES every NodeType assembly at adoption, invisible until a portal boots; * the gate DETECTED the move (412) and then died on it, so #3409 traded a silent mix for a guaranteed red on every overlapping publish. All three consumers now pin the generation their index came from, and on 412 they discard the half they hold and read the publication that NOW applies — bounded at 2 restarts, then RED naming #3401 so a pin-move PR is not blamed for it. Verification (both arms measured): * SealedModuleCompositionTest — real registry endpoints + the real CLI composition, with a publisher that reseals between the two module fetches. Fixed 2/2 pass; un-pinned (`RegistryGet(http, moduleUrl, null, ct)`) 0/2, MeshWeaver.AI comes back gen1 while MeshWeaver.Essentials is gen2. * test-sealed-module-compose.py runs the REAL compose-sealed-modules.sh and the gate's seed step EXTRACTED from node-repo-gate.yml, against a stub registry that republishes mid-read. 6/6 pass; un-pinned the gate mixes Store.zip=gen1 with Export.zip=gen2 at exit 0, and #3409's detect-and-die reds on a single overlapping publish. Memex.Portal.Shared.Test 788/788, MeshWeaver.Documentation.Test 279/279. Closes #3401 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
It introduces new async/await/Task-based control flow in src/MeshWeaver.Cli/BuildPluginCommand.cs, which is forbidden in this repo’s src-tree due to scheduler-parking and AccessContext-loss failure modes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR tightens the “sealed publication read” contract for the module composition lane so consumers (CLI + shell + gate) pin a single publication generation, survive a mid-read reseal by re-reading the new publication, and fail loudly (bounded) when the publisher reseals faster than a complete read can finish.
Changes:
- Update all three consumers (CLI composition,
compose-sealed-modules.sh, andnode-repo-gate.ymlseed) to pin generation on fetches and restart on 412 with a bounded retry + discard of partial staged data. - Add deterministic harnesses: a C# integration test driving the real CLI composition against a TestServer registry, and a Python harness that executes the real shell script and gate seed step against a republishing stub registry.
- Document the read contract and behavior in a new architecture page and a What’s New entry.
File summaries
| File | Description |
|---|---|
src/MeshWeaver.Cli/BuildPluginCommand.cs |
Adds generation-pinned sealed-module composition with bounded restart on 412 and discard of partial staging. |
.github/scripts/compose-sealed-modules.sh |
Pins module fetches with If-Match, restarts on 412 (bounded), and discards partial outputs on restart. |
.github/workflows/node-repo-gate.yml |
Updates upstream seed loop to restart on 412 (bounded) and discard partial seed files before re-reading. |
.github/scripts/test-sealed-module-compose.py |
New harness that executes the real shell script and extracts/runs the gate seed step against a republishing stub registry. |
.github/workflows/dotnet-test.yml |
Wires the new sealed-module shell/gate harness into CI. |
test/Memex.Portal.Shared.Test/SealedModuleCompositionTest.cs |
New deterministic integration tests validating “one publication or none” behavior for CLI composition. |
test/Memex.Portal.Shared.Test/Memex.Portal.Shared.Test.csproj |
Adds a reference to MeshWeaver.Cli so the tests drive the real CLI composition code. |
src/MeshWeaver.Documentation/Data/Architecture/SealedPublicationReads.md |
New architecture doc describing the sealed publication read contract (404/503/412 + generation + restart rules). |
src/MeshWeaver.Documentation/Data/Architecture/CiContentBake.md |
Links to the new “Sealed Publication Reads” doc and summarizes the read-window implications. |
src/MeshWeaver.Documentation/Data/Architecture.md |
Adds the new architecture page to the index. |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-06-a-build-takes-add-ons-from-one-publication-not-two.md |
What’s New entry describing the user-visible fix. |
Review details
Suppressed comments (15)
src/MeshWeaver.Cli/BuildPluginCommand.cs:495
- This adds an async/Task-based API in src/ ("public async Task<…>"). In this codebase, async/await/Task in src is a defect because it can park single-threaded schedulers (hub/grain/Blazor) and/or lose AccessContext across continuations; prefer an IObservable-returning composition and push HTTP/file I/O through IIoPool instead.
public async Task<List<string>?> ComposeSealedModules(
HttpClient http, string upstreamSources, string identity, string modules, string extDir,
CancellationToken ct)
src/MeshWeaver.Cli/BuildPluginCommand.cs:500
- New await in src/: async/await is forbidden here (can park hub/grain/Blazor schedulers and/or lose AccessContext across continuations). Convert this retry loop to an observable composition (e.g., SelectMany + RetryWhen/backoff) and run the HTTP work via IIoPool.
var composed = await ComposeOnce(http, upstreamSources, identity, modules, extDir, ct);
src/MeshWeaver.Cli/BuildPluginCommand.cs:505
- New await in src/: async/await is forbidden here. Keep the control flow reactive and surface errors via OnError (or log at the subscribe boundary) rather than awaiting TextWriter async APIs.
await error.WriteLineAsync(
src/MeshWeaver.Cli/BuildPluginCommand.cs:566
- New await in src/: async/await is forbidden here (can park turn-based schedulers and/or lose AccessContext across continuations). Prefer a reactive parse step (IObservable) and push stream reads through IIoPool instead of awaiting.
using var doc = await System.Text.Json.JsonDocument.ParseAsync(
src/MeshWeaver.Cli/BuildPluginCommand.cs:571
- New await in src/: async/await is forbidden here. Use reactive error propagation and keep the call chain observable-based rather than awaiting TextWriter async APIs.
await error.WriteLineAsync(
src/MeshWeaver.Cli/BuildPluginCommand.cs:588
- New await in src/: async/await is forbidden here. Keep error handling inside the observable pipeline (OnError) and avoid awaiting inside this composition logic.
await error.WriteLineAsync($"error: {url} answered 200 but not JSON: {ex.Message}");
src/MeshWeaver.Cli/BuildPluginCommand.cs:593
- New await in src/: async/await is forbidden here. Convert this logging/write-out to happen at the subscribe boundary of an observable pipeline rather than being awaited inside the composition implementation.
await output.WriteLineAsync(
$"upstream '{src}' sealed {bundles.Count} module bundle(s) for identity {identity}");
src/MeshWeaver.Cli/BuildPluginCommand.cs:620
- New await in src/: awaiting the pinned module fetch violates the no-async rule. Prefer a reactive fetch (IObservable) with the actual async HTTP I/O executed via IIoPool, so this composition doesn't park schedulers or lose AccessContext.
using var moduleResponse = await RegistryGet(http, moduleUrl, hit.Generation, ct);
src/MeshWeaver.Cli/BuildPluginCommand.cs:626
- New await in src/: async/await is forbidden here. If a publication move needs to be detected/handled, do it in an observable pipeline (e.g., materialize 412 as a signal) rather than awaiting inside the composition loop.
await output.WriteLineAsync(
$"the '{hit.Source}' publication was resealed while its module set was being "
+ $"composed (we held generation {hit.Generation}) — discarding what this "
+ "attempt staged and re-reading the publication that now applies.");
src/MeshWeaver.Cli/BuildPluginCommand.cs:636
- New await in src/: async/await is forbidden here. Prefer a reactive error path (OnError) and run any necessary I/O through IIoPool; avoid awaiting TextWriter async APIs in src.
await error.WriteLineAsync(
$"error: '{hit.Name}' is listed in the '{hit.Source}' sealed module set for "
+ $"identity {identity} but the registry will not serve it — the seal and the "
+ "store disagree, and that needs a republish. This is NOT the republish "
+ "window: that answers 503 and is waited out.");
src/MeshWeaver.Cli/BuildPluginCommand.cs:640
- New await in src/: async/await is forbidden here. Stream reads/copies should be executed via IIoPool and composed reactively, rather than awaiting inside the composition logic.
await using var body = await moduleResponse.Content.ReadAsStreamAsync(ct);
src/MeshWeaver.Cli/BuildPluginCommand.cs:641
- New
await usingin src/: async disposal implies this method remains async/Task-based, which is forbidden in src because it can park turn-based schedulers and/or lose AccessContext. Prefer a reactive composition and execute I/O off-scheduler via IIoPool.
await using var file = File.Create(zipPath);
src/MeshWeaver.Cli/BuildPluginCommand.cs:649
- New await in src/: async/await is forbidden here. Errors should flow through the observable pipeline and be handled at Subscribe, rather than awaiting TextWriter async APIs inside core composition logic.
await error.WriteLineAsync(
$"error: could not fetch sealed module {hit.Name} of '{hit.Source}' for identity "
+ $"{identity}: {ex.GetType().Name}: {ex.Message}");
return new ComposeAttempt(null, false);
src/MeshWeaver.Cli/BuildPluginCommand.cs:652
- New await in src/: async/await is forbidden here. If staging a module bundle is I/O-bound, run it through IIoPool and compose reactively rather than awaiting within this loop.
var name = await StageModuleBundle(zipPath, extDir, pkg);
if (name is null) return new ComposeAttempt(null, false);
src/MeshWeaver.Cli/BuildPluginCommand.cs:656
- New await in src/: async/await is forbidden here. Prefer logging/output as part of an observable pipeline (or at Subscribe) rather than awaiting inside the core composition code.
await output.WriteLineAsync(
$"external module composed from the SEALED publication of '{hit.Source}': {name} ({pkg}, {hit.Name})");
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| var args = new List<string>(); | ||
| using var http = new HttpClient { BaseAddress = new Uri(registryUrl.TrimEnd('/') + "/") }; | ||
| http.DefaultRequestHeaders.Authorization = new("Bearer", key); | ||
| return await ComposeSealedModules(http, upstreamSources, identity, modules, extDir, ct); |
# Conflicts: # src/MeshWeaver.Documentation/Data/Architecture.md
Closes #3401.
Where this picks up
#3409 (merged 13:38Z) fixed the reported defect: the republish window now answers 503 +
Retry-Afterinstead of 404, and the BUNDLE lane carries a generation so a consumer's N+1reads cannot silently mix two publications. The issue's last comment (14:07Z, after that merge)
changed the attribution and named what remained: the writer was MeshWeaver.Plugins' own
publish-bake, not core CD, and the exposure is per target, per publish, opened by everyrepo that publishes
bake-source: plugins— core CD'splugins-bakeand the satellite.Two gaps were left, and both still red a satellite for a defect that is not in its diff.
1. The MODULE lane read unpinned — in both of its consumers
compose-sealed-modules.shandBuildPluginCommand.ComposeSealedModulesread the module-set indexand then each module it names, sending no
If-Match. A reseal in between silently composesmodule bytes from two publications, and that is worse than a red: it is the mvid mismatch that
makes the boot seeder DECLINE every NodeType assembly built against them —
— which is invisible in CI and surfaces only when a portal boots and renders nothing. Exactly the
invisible-adoption failure
assert-bake-consumption.shexists to catch.Measured, running the real script against a registry that republishes mid-composition:
2. The gate detected the move and then gave up
#3409 added
If-Matchtonode-repo-gate.ymland made 412exit 1. That trades a silent mix fora guaranteed red on every overlapping publish — a satellite's pin-move PR going red for a defect
that is not in it, which is precisely what #3401 is about. Detecting is half a fix; the other half is
the defined recovery.
What changed
All three consumers now obey one contract:
from. Opt-in — a registry predating fix(#3401): a publication being republished answers 503, not 404 — and carries a generation #3409 publishes none, no header is sent, behaviour unchanged.
15 30 60 90backoff — unchanged).RESTARTS = 2, then RED naming Satellite gate 404s on a bundle its sealed publication names, while core CD republishes that identity #3401. The bound is what keeps this a read rather than apoll: a publisher resealing faster than a read completes is a real condition and must be
reported.
and the new one does not lingers in the seed — a bundle from a publication the consumer refused.
Nothing here retries a read the server refused, and nothing makes a gate more tolerant of a torn
publication: a mixed read is still never accepted.
Clearer failure for pin-move PRs, as the issue asked for: the exhaustion message reads "the
publisher is republishing faster than this gate can read one publication. This is MeshWeaver#3401,
NOT a fault of this repo's diff or its platform pin."
Verification — both arms, measured
SealedModuleCompositionTest(new): the REAL registry endpoints in a TestServer, the REALmemex build plugincomposition against them, and a publisher that reseals between the first modulefetch and the second — the production interleave, made deterministic rather than raced.
RegistryGet(http, moduleUrl, null, ct)— the pre-fix read, tests unchangedExpected: "gen2" / Actual: "gen1".github/scripts/test-sealed-module-compose.py(new, wired into the same job astest-cd-steps.py): runs the REALcompose-sealed-modules.shand the gate's seed step EXTRACTEDfrom
node-repo-gate.ymlby itsid:— never a copy — against a stub registry that republishesmid-read. Six cases: settled (a CONTROL that must pass), resealed once, reseal storm, a registry
publishing no generation at all, plus the gate's own seed loop resealed-once and reseal-storm. It
asserts
If-Matchwas actually sent, because a script that stopped pinning would still composeits files and pass on the outputs alone.
FAIL [settled] every bundle fetch must pin the generation…FAIL [gate-seed] the seed mixed publications: {'Store.zip': 'gen1', 'Export.zip': 'gen2'}— at exit 0exit 1on 412FAIL [gate-seed] one reseal must be survivable … not by failing the satellite's gateSuites:
Memex.Portal.Shared.Test788/788,MeshWeaver.Documentation.Test279/279 (linkintegrity + every ratchet guard).
shellcheck -S warningclean,check-workflow-shell.pyandcheck-workflow-timeouts.pyclean.Doc
New page Sealed Publication Reads —
the reader's half of the bake contract: why the window exists and cannot be closed in this layout,
the measured 2026-09-06 timeline, the three answers, what the generation folds in, the four consumer
rules, and a What is NOT closed section. Linked from
CiContentBakeand the Architecture index.What's New entry,
Category: Fix.🚨 What this does NOT close
Stated here rather than left implicit, because the issue's own last comment raised it:
Two writers on one prefix can produce a seal whose bytes are a mix, and no reader can detect it.
Core CD's
plugins-bakeand the Plugins satellite'spublish-bakeboth writeprebuilt-bundles/<identity>/plugins/. If they overlap, both unseal, both upload, and the last toseal writes a sentinel over a directory holding some of each one's bytes — one seal, one
generation, self-consistent to every consumer and wrong. Closing it needs either a single owner for
the prefix or a publish that cannot interleave (a generation directory plus an atomic pointer swap,
which is a layout migration every reader must land first). Both are scope calls, not code changes,
so they are documented and left to the maintainer rather than guessed at here.
Note for reviewers
Pairs-with: none— this adds public surface (BuildPluginCommand.ComposeSealedModules(HttpClient, …),MaxPublicationRestarts) and removes none, so the cross-repo gate has nothing to resolve. It isAGENTS.md's shape 7 (behaviour behind unchanged signatures) for satellites pinning
node-repo-gate.ymland fetchingcompose-sealed-modules.shatplatform-ref: they will startsurviving a mid-read republish instead of failing it. Strictly better for them, no satellite change
needed — flagged rather than left to be discovered.
🤖 Generated with Claude Code