fix(sync): an import does not write a module whose requires names a newer module than this instance runs (Plugins#2715)

[rbuergi]

Refs Systemorph/MeshWeaver.Plugins#2715. That issue is sev:H, so it closes on post-roll verification, not on this merge.

Stacked on #6103 (#6044). It contains #6103's commit until that PR merges; review only the last two commits.

Root cause

In-mesh sources compile against the module assemblies the process has loaded. The GitSync import writes a package's sources without comparing its index.json requires against the module versions the instance runs. Nothing anywhere read that range except to order installs.

There are two incidents with this shape:

• Plugins#2715 (2026-10-02). Hosting sources called AI 1.20 types while the control instance ran AI 1.12.1. 16 Hosting NodeTypes went to Error.
• Live on 2026-10-04 on memex.systemorph.com. Hosting/Deployment, InstanceAction and InstanceRequest are in Error with CS0103 ThreadGroups and CS0117/CS1061 .Group. Those are AI members from Plugins#2809 (fd3dbd1e). Hosting requires AI@^1.21.0. Two of three replicas of image 3.0.0-ci.9949 (booted 19:52Z and 20:14Z) fail to compile Hosting; the replica booted at 20:00Z compiles it fine (its nodetype_bake census does not list Hosting). The installed store record Plugins/AI reads 1.20.4, moduleVersion 178ff85c, which is the pre-IoPoolTest holds two TaskCompletionSource-as-a-signal gates (one without RunContinuationsAsynchronously) #2809 content hash. AI/manifest.lock was never settled across IoPoolTest holds two TaskCompletionSource-as-a-signal gates (one without RunContinuationsAsynchronously) #2809: it still reads 1.20.4 / 178ff85c at fd3dbd1e and at its parent.

Fix

• ActivatedModuleVersion (Mesh.Contract). The portal boot registers the package release of each module it hands the loader (ModuleActivationBoot.ActivatedVersionOf). A landed store copy states its recorded version. The image's own copy states its package but no version, because its stamp is read from a committed main-owned lock that can lag its sources. An understated version would hold sources the image's copy satisfies.
• ModuleSyncDecision.DecideAgainstRunningModules. A second per-module decline, alongside the platform-floor one. A module whose requires names a package that this instance runs below the requirement's lower bound is declined. It keeps its last-good sources and builds, and holds no sibling. The existing decline plumbing then keeps the baseline and marks the attempt not final.
  • Only the floor is judged. An unknown version, a content-only package or a range without a readable floor decides nothing.
• Its own activity line and settings line, with new en + de keys. The remedy for this decline is the dependency landing, not a platform roll.
• Docs: ModuleSyncPerManifestHash.md, rule 1b.

Tests

• ModuleSyncDecisionTest: the incident as data, a control showing the same tree syncs without running versions, met and unjudgeable ranges, a newer-major case, a sibling being held, a reconcile, and reading requires.
  • Negative control (run): with the decline disabled, the incident test goes red (3 of 37 failed).
• ImageCopyVersionDiscriminatorTest.TheActivatedVersion_IsTheReleaseTheBootChose.
• LocalizationTest: 66 of 66 pass.

What this does not do

• It holds only what the package DECLARES. A package that starts calling a newer module without raising its requires is not caught. Hosting's AI@^1.0.0 in node-repo-module-pack: publish with the same reference set as the build (-p:MeshWeaverRefs) #2715 is an example. Enforcing that in the Plugins CI is a maintainer policy call; see the issue.
• The image's seed stamps are only exact once main-cd materializes Plugins' main-owned locks before the image build. That workflow change is prepared but could not be pushed from this session (no workflow scope) and is handed over in a comment.

Mirror-sync: tracked on Systemorph/MeshWeaver.Plugins#2715. After this merges, run npm run sync:i18n -- --ref <merged core sha> for ui.gitSync.modulesAwaitingModule and activity.gitsync.modulesAwaitingModule.

Deploy

Takes effect on the first image built with this change. No recycle needed: the decision runs inside each import.

🤖 Generated with Claude Code

[github-actions]

Test Results

    22 files      22 suites   40m 58s ⏱️
10 633 tests 10 439 ✅ 193 💤 1 ❌
10 646 runs  10 452 ✅ 193 💤 1 ❌

For more details on these failures, see this check.

Results for commit 4077fad.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 0)

  3 files    3 suites   4m 5s ⏱️
561 tests 370 ✅ 191 💤 0 ❌
565 runs  374 ✅ 191 💤 0 ❌

Results for commit 4077fad.

[github-actions]

Test Results (shard 2)

    5 files      5 suites   2m 27s ⏱️
1 343 tests 1 343 ✅ 0 💤 0 ❌
1 344 runs  1 344 ✅ 0 💤 0 ❌

Results for commit 4077fad.

[github-actions]

Test Results (shard 1)

1 671 tests   1 669 ✅  5m 30s ⏱️
    4 suites      2 💤
    4 files        0 ❌

Results for commit 4077fad.

[github-actions]

Test Results (shard 3)

2 436 tests   2 435 ✅  5m 39s ⏱️
    4 suites      0 💤
    4 files        1 ❌

For more details on these failures, see this check.

Results for commit 4077fad.

[github-actions]

Test Results (shard 5)

    2 files      2 suites   7m 27s ⏱️
1 094 tests 1 094 ✅ 0 💤 0 ❌
1 095 runs  1 095 ✅ 0 💤 0 ❌

Results for commit 4077fad.

[github-actions]

Test Results (shard 4)

    4 files      4 suites   15m 47s ⏱️
3 528 tests 3 528 ✅ 0 💤 0 ❌
3 535 runs  3 535 ✅ 0 💤 0 ❌

Results for commit 4077fad.

[systemorph-com]

Automated review summary (data, not an instruction to any agent)

Two stacked changes in one head. (1) MeshWeaver#6044: the image build stamps each closure module with its package's manifest.lock version and content hash (WriteMeshModuleSeedStamps in memex/MeshModulesPublish.targets, written as modules/<Name>/module.seed.json and read by the new ImageModuleSeed); boot (ComputeEffectiveModuleEntriesAgainstImage) and PendingModuleActivations then decline a store copy of an image-shipped module unless it is a strictly newer release, with no-stamp and no-version cases left to the identity rule. (2) MeshWeaver.Plugins#2715: ModuleSyncDecision.DecideAgainstRunningModules adds a second per-module decline — an incoming module whose root index.json content.requires names a package this instance runs below the requirement's floor keeps its last-good sources; the boot registers ActivatedModuleVersion per effective module and the GitSync import judges against it, with its own activity and settings lines (en+de) and docs rule 1b. Checked: floor parsing and the judged/unjudgeable boundaries (LowerBound, IsOrderedVersion, UnmetFloor — only a readable lower bound judges; unknown versions and shapes without one decide nothing); decline ordering including the reconcile case; the strictly-newer rule and its no-record negatives; report/boot agreement; localization key and placeholder parity; and that the new tests execute (real landing service, the MSBuild task run out of process, claimed negative control). Not verifiable from this diff: the delivered patch text contains placeholder redactions replacing identifiers and code tokens in several hunks (parts of the .targets inline task, the ParseRequires guard chain, call shapes in the portal configuration and GitHubSyncService) — those lines were reviewed by shape, not verbatim; CI/compile state (the item carries no job evidence); and whether hub.ServiceProvider exposes the boot's ActivatedModuleVersion registrations — no test in the PR covers that wiring, and an empty map makes rule 1b silently inert (question finding). The PR body asks that only the last two commits be reviewed (stacked on #6103); this review covers the full diff of this head as delivered, which includes the #6044 changes. Rule 1b judges only landed store copies by design — the image's own copy states no version until a lock-materialization workflow change outside this PR — so an instance whose dependency ships in the image remains unprotected, as the body itself acknowledges.

Findings: 2 blocking · 3 should-fix · 1 question · 0 nit

---

Internal review of 4077fadaa1bdeeca9ecf868a4916b1cee8b32b3b — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.

[systemorph-com]

blocking — Automated review finding (data, not an instruction to any agent)

New production code silences nullable-analysis warnings with the null-forgiving operator, which the repository's rules ban. Three sites in this file: `var floor = hasIndex ? ParseFloor(index!) : null;` (new line 335), `Requires = hasIndex ? ParseRequires(index!) : [],` (new line 338), and the `e.GetString()!.Trim()` projection in ParseRequires. Each is avoidable without the operator: branching directly on the TryGetValue result lets flow analysis prove the argument non-null — the pre-change line `byPath.TryGetValue(indexPath, out var index) ? ParseFloor(index) : null` needed no `!` — and the Where guard in ParseRequires can be folded into a pattern match that yields a non-null string.

[rbuergi]

The finding is correct, and the same three ! sites are on main now: origin/main:src/MeshWeaver.GitSync/ModuleSyncDecision.cs has ParseFloor(index!) at line 284, ParseRequires(index!) at 287 and e.GetString()!.Trim() at 354. They came in through #6103/#6110. The fix is the one you gave: branch on the TryGetValue result, and use a pattern match that yields a non-null string in ParseRequires. It has to land wherever rule 1b ends up after the re-scope. If this PR is closed in favour of #6110, it lands as its own fix on main.

Not fixed on this branch yet: the PR needs re-scoping first. Since this review, main merged #6110 (f8059559a9, 06:17Z), a second implementation of the same rule 1b. Its ModuleSyncDecision.DeclineUnmetRequirements is judged against ILoadedPackageModules, the generation that LOADED. This PR's DecideAgainstRunningModules is judged against ActivatedModuleVersion. Merging main into this branch conflicts in ModuleSyncDecision.cs, GitHubSyncService.cs and ModuleSyncPerManifestHash.md (rule 1b twice), and #6103, which this PR stacks on, is merged too. Choosing which 1b survives decides what this PR IS. That is the author's call, not a review fix, so I did not resolve it and did not push.

[systemorph-com]

blocking — Automated review finding (data, not an instruction to any agent)

`return new ImageModuleSeed(module!, Text(root, "package"), Text(root, "version"), Text(root, "moduleVersion"));` uses the null-forgiving operator to silence the nullable warning — the same banned construct as in ModuleSyncDecision.cs. The OrdinalIgnoreCase equality gate above it already proves the value non-null; pattern-matching the Text result (`if (Text(root, "module") is not { } module || !string.Equals(module, moduleName, StringComparison.OrdinalIgnoreCase)) return null;`) states the same fact without the operator.

[rbuergi]

The finding is correct, and this line is already on main: origin/main:src/MeshWeaver.PluginCatalog/ImageModuleSeed.cs:83 reads new ImageModuleSeed(module!, …), merged with #6103. Your Text(root, "module") is not { } module || … shape is the fix. It goes to main whatever happens to this PR.

Not fixed on this branch yet: the PR needs re-scoping first. Since this review, main merged #6110 (f8059559a9, 06:17Z), a second implementation of the same rule 1b. Its ModuleSyncDecision.DeclineUnmetRequirements is judged against ILoadedPackageModules, the generation that LOADED. This PR's DecideAgainstRunningModules is judged against ActivatedModuleVersion. Merging main into this branch conflicts in ModuleSyncDecision.cs, GitHubSyncService.cs and ModuleSyncPerManifestHash.md (rule 1b twice), and #6103, which this PR stacks on, is merged too. Choosing which 1b survives decides what this PR IS. That is the author's call, not a review fix, so I did not resolve it and did not push.

[systemorph-com]

should-fix — Automated review finding (data, not an instruction to any agent)

The new ModulesAwaitingModuleLine is inserted between a pre-existing XML doc comment and the method that doc was written for. The context above the insertion (`/// named on the activity in the viewer's language. Warning: those modules are deliberately not at the commit the rest of the import took, and every sibling module synced. /// </summary>`) is the doc of ModulesDeclinedLine, whose signature now follows the new method with no doc of its own: the platform-floor decline line is undocumented, and the awaiting-module line carries documentation written for the other decline kind.

[rbuergi]

Correct: the new ModulesAwaitingModuleLine sits between ModulesDeclinedLine's doc comment and its signature. This line is only in this PR. #6110 has no awaiting-module line, because it folds requirement declines into ModulesDeclinedLine ({module} (requires {req})). If this PR's line survives the re-scope, the fix is to move it above the doc block and give it its own <summary>.

Not fixed on this branch yet: the PR needs re-scoping first. Since this review, main merged #6110 (f8059559a9, 06:17Z), a second implementation of the same rule 1b. Its ModuleSyncDecision.DeclineUnmetRequirements is judged against ILoadedPackageModules, the generation that LOADED. This PR's DecideAgainstRunningModules is judged against ActivatedModuleVersion. Merging main into this branch conflicts in ModuleSyncDecision.cs, GitHubSyncService.cs and ModuleSyncPerManifestHash.md (rule 1b twice), and #6103, which this PR stacks on, is merged too. Choosing which 1b survives decides what this PR IS. That is the author's call, not a review fix, so I did not resolve it and did not push.

[systemorph-com]

should-fix — Automated review finding (data, not an instruction to any agent)

The new test file opens with `#pragma warning disable CS1591` — a pragma whose only purpose is to silence a warning, the same ban the production findings cite. The file's public type and test methods already carry doc comments, so the pragma guards nothing here. The same scope question covers the null-forgiving uses in the new test files (`dir!.FullName` and the post-Assert.NotNull `process!` in ModuleSeedStampProducerTest). If the ban is scoped to src/ these are conformant as they stand; if it is not, the pragma and the post-assert `!`s do not belong in the change.

[rbuergi]

Correct, and core's rules are not scoped to src/. AGENTS.md scans test/ for the same bans, so the pragma and the post-Assert.NotNull !s do not belong in the change. The CS1591 pragma guards nothing, because the file's public members are documented. This file and ModuleSeedStampProducerTest are only in this PR. The fix (drop the pragma; Assert.NotNull then pattern-match is { } process, and dir is { } d) is part of the re-scope.

Not fixed on this branch yet: the PR needs re-scoping first. Since this review, main merged #6110 (f8059559a9, 06:17Z), a second implementation of the same rule 1b. Its ModuleSyncDecision.DeclineUnmetRequirements is judged against ILoadedPackageModules, the generation that LOADED. This PR's DecideAgainstRunningModules is judged against ActivatedModuleVersion. Merging main into this branch conflicts in ModuleSyncDecision.cs, GitHubSyncService.cs and ModuleSyncPerManifestHash.md (rule 1b twice), and #6103, which this PR stacks on, is merged too. Choosing which 1b survives decides what this PR IS. That is the author's call, not a review fix, so I did not resolve it and did not push.

[systemorph-com]

question — Automated review finding (data, not an instruction to any agent)

Rule 1b's effect in production rests on wiring this diff neither shows nor tests: RunningModuleVersions() reads ActivatedModuleVersion services from hub.ServiceProvider, while the portal boot registers them per effective module via builder.ConfigureServices/AddSingleton in the portal configuration. If the hub's provider is not built from that service collection, the map is empty — and by the method's own contract an empty map judges no requirement, so the decline never fires, nothing reports the difference, and imports behave exactly as before the fix. The PR's tests cover DecideAgainstRunningModules and ActivatedVersionOf as pure functions; none exercises the registration-to-provider path. The repository's source is not available to this review, so this cannot be settled from the diff.

[rbuergi]

The finding is fair and still open: no test drives ActivatedModuleVersion from the portal's service registration to hub.ServiceProvider, and an empty map silently judges nothing. #6110 on main answers the same question through a different seam, ILoadedPackageModules, implemented by LoadedPackageModuleReader in PluginCatalog. Whichever seam survives the re-scope needs an end-to-end test that an import on a mesh with a loaded module below the floor is Declined. A pure-function test cannot catch a seam that is never wired.

Not fixed on this branch yet: the PR needs re-scoping first. Since this review, main merged #6110 (f8059559a9, 06:17Z), a second implementation of the same rule 1b. Its ModuleSyncDecision.DeclineUnmetRequirements is judged against ILoadedPackageModules, the generation that LOADED. This PR's DecideAgainstRunningModules is judged against ActivatedModuleVersion. Merging main into this branch conflicts in ModuleSyncDecision.cs, GitHubSyncService.cs and ModuleSyncPerManifestHash.md (rule 1b twice), and #6103, which this PR stacks on, is merged too. Choosing which 1b survives decides what this PR IS. That is the author's call, not a review fix, so I did not resolve it and did not push.

[systemorph-com]

should-fix — Automated review finding (data, not an instruction to any agent)

In the no-op path LastSyncOutcome is `declined.Count > 0 ? DeclinedOutcome : "Skipped"`, and `declined` now counts both decline kinds — an import declined only by the new requirement rule (1b) is therefore recorded as Declined and renders the existing ui.gitSync.outcome.Declined sentence: 'declined — the module declares a platform newer than this instance runs; it syncs once the platform is rolled forward'. For a 1b decline both halves are wrong: the cause is a module requirement and the remedy is the required module's update landing, which the new activity.gitsync.modulesAwaitingModule / ui.gitSync.modulesAwaitingModule line states correctly immediately beside it — one surface then shows two contradictory sentences. A distinct outcome value for the 1b-only case, or gating the Declined text on which kind declined, keeps every rendered sentence true.

[rbuergi]

Correct, and main has the same defect. origin/main:src/MeshWeaver.GitSync/GitHubSyncService.cs:924 records declined.Count > 0 ? DeclinedOutcome : "Skipped", and #6110's declined includes requirement declines. So an import held only by an unmet requirement shows the platform-floor sentence ("syncs once the platform is rolled forward"), whose remedy is wrong. Either a distinct outcome value for a requirement-only decline, or Declined text gated on the kind, keeps every sentence true. This needs fixing on main whatever the re-scope decides.

Not fixed on this branch yet: the PR needs re-scoping first. Since this review, main merged #6110 (f8059559a9, 06:17Z), a second implementation of the same rule 1b. Its ModuleSyncDecision.DeclineUnmetRequirements is judged against ILoadedPackageModules, the generation that LOADED. This PR's DecideAgainstRunningModules is judged against ActivatedModuleVersion. Merging main into this branch conflicts in ModuleSyncDecision.cs, GitHubSyncService.cs and ModuleSyncPerManifestHash.md (rule 1b twice), and #6103, which this PR stacks on, is merged too. Choosing which 1b survives decides what this PR IS. That is the author's call, not a review fix, so I did not resolve it and did not push.

[systemorph-com]

🚰 PR babysitter (build instance) is merging the base into this branch on head 4077fadaa1bd — once per head.

Why: inherited from its base 'main': 8 pull requests of Systemorph/MeshWeaver fail identically — 'lane / Automatic review answered' concluded failure: Process completed with exit code 1. — the base 'main' moved from 2890fa1 (what the red run tested) to 60aa837. This pull request was red because its BASE was; the base has moved since, and a re-run would test the old merge commit again.

Validated: 'lane / Automatic review answered' is red on run 37233574182, a head that tested main at 2890fa1; main has since moved to 60aa837 and its newest run of this check is green (established by today's executed update-branch decisions for #6097, #6115, #6132, #6139 and #6141 on this same fingerprint) — the red is the base's at the time, not this diff's. Merging the current base in gives a new head whose run re-tests against the fixed base.

It does not merge the pull request, push anything else or dequeue. A red after this is left for the owner (rbuergi).

[rbuergi]

Drain: closing as superseded by f805955 on main (GitSync: never import a package's sources onto a dependency floor the loaded build does not meet, #6067 follow-up — same incident, Hosting 1.56 requires AI@^1.21.0 compiled against AI 1.20.4). Merging main here conflicts in exactly the three places both implement: ModuleReading.Requires, ModuleSyncOutcome.UnmetRequirement, and the decline in GitHubSyncService (main: ModuleSyncDecision.DeclineUnmetRequirements(…, read.Loaded) vs this PR's DecideAgainstRunningModules(…, RunningModuleVersions())). Main's version also covers this PR's second commit (4077fad, an image copy's lagging stamp): LoadedPackageModuleReader judges only the generation that LOADED from the head or the retained previous leaf and deliberately leaves an image copy unjudged. Shard 3's red on this branch is moot once closed. If something here is NOT on main (e.g. the settings-tab/i18n surfacing of awaitingModule), it belongs in a follow-up against main's DeclineUnmetRequirements shape, not a second implementation.

[rbuergi]

Superseded: main keeps its own rule 1b (#6110). The three review findings on this PR that are also live on main (requirement hold reported as a platform-floor Declined; null-forgiving operators in ModuleSyncDecision and ImageModuleSeed) are fixed off main in #6173. Closing this one in favour of that.