fix(retention): a declaration-less static NodeType is not an unreadable adoption record

[rbuergi]

What was swallowed

DeploymentReportService.ReadAdoptedFrameworks threw NodeType adoption record ControlLaneRecord could not be read for the static, content-less ControlLaneRecord NodeType (registered by AddControlLane on every portal), caught it at Warning, and reported AdoptedFrameworkInventoryComplete = false. DeploymentPinnedReferences.ReportedBuildsOf then refuses every such report, so fleet artifact retention never had a complete inventory.

Evidence (control instance Logs actions, last 6 h, 2026-10-06): Ops/Actions/swallow-audit-2050-control-exc2 (control: ~22 occurrences), Ops/Actions/swallow-audit-2100-memex-exc4 (memex), Ops/Actions/swallow-audit-2100-cloud-exc4 (memex-cloud). The header line before each one is warn: MeshWeaver.PluginCatalog.DeploymentReportService (Ops/Actions/swallow-ctx-control-n5fnk-1955).

The retention pass (PrebuiltBundleRetentionHostedService.StampedIdentities) folded the other way: content it could not read became "no stamp" and silently left the reference set, so that bundle could be deleted.

Fix

NodeTypeAdoptionStamp.CompiledFrameworkVersionOf is now the one reading, used by both:

• no content means a static declaration-less NodeType. It adopts nothing, so it returns null.
• a NodeTypeDefinition returns its stamp.
• content that is present but unreadable throws. The report stays incomplete for that and retention aborts.

Test

AdoptedInventoryReadsDeclarationLessNodeTypesTest: a mesh with AddControlLane() reports a complete inventory, and the stamp reading tells the three states apart. Negative control: with DeploymentReportService.cs reverted, the first test fails with found False. Also green: AnInstanceReportsWhatItRunsTest, DeploymentPinnedReferencesTest, PrebuiltBundleRetentionTest, PrebuiltBundleRetentionArmingGuard, SealedPublicationLineageTest, PrebuiltBundleRetentionIsRegisteredTest. All touched projects build with -c Release -warnaserror.

Doc: ArtifactRetentionInterlock.md gets a short section with the three-state table.

Recycle after deploy: none needed. The report reruns each hour and the retention pass runs on its own schedule.

Pairs-with: none — adds a public type and removes no public surface.

Opened as a draft on purpose: the audit task said to open PRs without merging them, and auto-arm merges any non-draft PR.

🤖 Generated with Claude Code

[github-actions]

Test Results

   13 files     13 suites   35m 43s ⏱️
9 406 tests 9 406 ✅ 0 💤 0 ❌
9 419 runs  9 419 ✅ 0 💤 0 ❌

Results for commit 70391cb.

♻️ This comment has been updated with latest results.

[systemorph-com]

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

Adds the public helper NodeTypeAdoptionStamp.CompiledFrameworkVersionOf as the single three-state reading of a NodeType record's adoption stamp — no content (a declaration-less static NodeType) → null; a readable NodeTypeDefinition → its CompiledFrameworkVersion, or null when blank; present-but-unreadable content → throw — and rewires both readers onto it: the deployment report no longer marks the adopted-framework inventory incomplete for the content-less ControlLaneRecord record every portal registers, and the prebuilt-bundle retention pass aborts on an unreadable record instead of silently dropping it from the reference set. Checked the helper's state machine, both call sites' before/after semantics (the report still degrades to Complete:false through its existing .Catch; the retention pipeline's Where/v! tail is unchanged context), the thrown message against the new test's WithMessage pattern, the doc table against the code, and that the tests execute real paths. Not verifiable from the diff: where the new throw surfaces in PrebuiltBundleRetentionHostedService (no catch visible — question finding), the claimed builds and green tests, the real adopted-artifact-incomplete warning text the test asserts on, and any project-level convention for CS1591. Blocking: the added test file silences warnings with #pragma warning disable CS1591 and outcome.Report!.

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

---

Internal review of 439b2cc1bda5dd6e99003b4c4596ae6a2c4bdb59 — 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)

The added test file silences compiler warnings in two places, which the repository's rules forbid (no `!` or `#pragma` to silence a warning). Line 1 opens with `#pragma warning disable CS1591` — the class itself carries a `<summary>`, so the flagged members are the two `[Fact]` methods, which can be documented instead. Line 59 reads `var report = outcome.Report!;`, suppressing the nullable warning instead of null-checking `outcome.Report`. If CS1591 does not fire in this test project at all, the pragma is dead weight and can simply be removed.

[systemorph-com]

Correct on both counts - the repository rules forbid silencing a warning with #pragma or !, and the class already carries a <summary>, so the members CS1591 would name can simply be documented.

Not changed yet on 439b2cc: #6210 is a draft its author is holding (see the PR body), and the owning thread has no push path to a non-bugfix/ branch, so this waits for the author's push. The change, confined to test/Memex.Portal.Shared.Test/AdoptedInventoryReadsDeclarationLessNodeTypesTest.cs:

1. Delete line 1, #pragma warning disable CS1591.
2. Document instead: /// <param name="output">The xUnit output sink.</param> in the class doc, /// <inheritdoc /> on ConfigureMesh, and a /// <summary> on each [Fact].
3. Replace var report = outcome.Report!; with
   var report = outcome.Report ?? throw new InvalidOperationException($"the report carried no DeploymentReport: {outcome.Delivery} - {outcome.Detail}");

No assertion changes, and the negative control (revert DeploymentReportService.cs, the first test fails with found False) is unaffected.

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[systemorph-com]

Correction to my earlier reply on this thread: the reason I gave was wrong. This PR was not a draft held by its author. It was out of draft when it was armed (21:25Z) and reviewed (21:26Z). The fix is not pushed because the owning thread has no push path to this PR's branch: its code writes exist only on a bug item's own bugfix/ branch. The fix waits for the pr-fixer's commit or the author's push. The substance of the earlier reply stands unchanged: remove the #pragma warning disable CS1591, document the two [Fact] methods, and replace outcome.Report! with a null-check.

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[rbuergi]

Valid — fixed in 51acfee, both halves:

1. #pragma warning disable CS1591 is gone. Instead: <param name="output"> on the class doc, <inheritdoc /> on ConfigureMesh, and a <summary> on each [Fact].
2. outcome.Report! is replaced by a null-check that names what came back: outcome.Report ?? throw new InvalidOperationException($"the report carried no DeploymentReport: {outcome.Delivery} - {outcome.Detail}").

No assertion changed. MeshWeaver.Hosting and Memex.Portal.Shared.Test build clean with -c Release -warnaserror (so CS1591 is satisfied by documentation, not suppressed), and AdoptedInventoryReadsDeclarationLessNodeTypesTest passes (2/2).

[systemorph-com]

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

The new throw is documented as aborting 'the pass', but this diff shows no handler on that pipeline: the report path wraps its equivalent Select in `.Catch` and degrades to `Complete: false`, while `StampedIdentities()` here is Query → Take(1) → Timeout → Select with no Catch in view. Where the InvalidOperationException surfaces is not visible — if `StampedIdentities()` feeds a long-lived subscription rather than a fresh per-pass subscribe, one unreadable record would stop retention entirely until restart instead of aborting a single pass. The subscription site (or a catch that bounds the failure to one pass) would settle it.

[systemorph-com]

It aborts that one pass, never the schedule - no change needed.

StampedIdentities() is not held by a long-lived subscription: RunPass builds it afresh on every tick (StampedIdentities().Zip(PinnedReferences(), ...)), and KickSchedule wraps each pass in its own catch before the Concat:

ticks.Select(_ => RunPass(root, pool)
        .Do(status.Record)
        .Select(_ => Unit.Default)
        .Catch<Unit, Exception>(ex => { status.RecordFault(ex); logger.LogWarning(ex, ...); return Observable.Return(Unit.Default); }))
    .Concat()

An exception thrown inside the Select selector reaches that pass's observable as OnError, so it lands in this per-pass Catch: the fault is recorded on PrebuiltBundleRetentionStatus.LastFault, logged at Warning ("the pass over {Root} faulted - nothing was collected; the next pass plans it again"), and the next tick runs. That is the contract the class summary states ("A faulted pass is logged, recorded ... and the schedule continues"). The sweep sits behind the Zip, so a faulted pass collects nothing - an unreadable record keeps every bundle, the safe direction.

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[rbuergi]

It aborts that one pass, never the schedule — no change needed. Checked against PrebuiltBundleRetentionHostedService on this head:

StampedIdentities() is not held by a long-lived subscription. RunPass builds it afresh on every tick (StampedIdentities().Zip(PinnedReferences(), …)), and the schedule wraps EACH pass in its own Catch before the Concat:

schedule = ticks
    .Select(_ => RunPass(root, pool)
        .Do(status.Record)
        .Select(_ => Unit.Default)
        .Catch<Unit, Exception>(ex => { status.RecordFault(ex); logger.LogWarning(ex, "… the pass over {Root} faulted — nothing was collected; the next pass plans it again", root); return Observable.Return(Unit.Default); }))
    .Concat()

A throw inside the Select selector reaches that pass's observable as OnError, lands in the per-pass Catch (recorded on the retention status, logged at Warning), and the next tick runs. Because the sweep sits behind the Zip, a faulted pass collects nothing — an unreadable record keeps every bundle, which is the safe direction.

[systemorph-com]

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

The unreadable-record message changes from `NodeType adoption record … could not be read` to `carries content of type … that cannot be read as a NodeTypeDefinition`. The PR's own evidence was located by the old wording, and genuinely unreadable records remain possible after this fix, so log searches or alerts keyed on the old fingerprint stop matching; keeping the old substring in the new message (or updating the audit pattern) preserves the correlation.

[systemorph-com]

Agreed - the swallow audit found the incident by NodeType adoption record ... could not be read, and a genuinely unreadable record should still match that fingerprint.

Not changed yet on 439b2cc, for the same reason as the blocking thread (a draft held by its author; no push path from the owning thread). The change in NodeTypeAdoptionStamp.CompiledFrameworkVersionOf keeps the old prefix and appends the detail:

$"NodeType adoption record {node.Path} could not be read: it carries content of type "
+ $"{DescribeContent(node.Content)} that cannot be read as a NodeTypeDefinition - its adoption stamp is unknown"

The test's WithMessage("*Unreadable*String*") still matches.

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[systemorph-com]

Correction to my earlier reply on this thread: the reason I gave was wrong. This PR was not a draft held by its author. It was out of draft when it was armed (21:25Z) and reviewed (21:26Z). The change is not pushed because the owning thread has no push path to this PR's branch: its code writes exist only on a bug item's own bugfix/ branch. The change waits for the pr-fixer's commit or the author's push. The substance of the earlier reply stands unchanged: the new message keeps the old prefix NodeType adoption record {path} could not be read, so log searches and alerts keyed on the old fingerprint still match, and the test's *Unreadable*String* pattern still matches.

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[rbuergi]

Agreed — fixed in 51acfee. The message keeps the established fingerprint as its prefix and appends the detail:

NodeType adoption record {path} could not be read: it carries content of type {type} that cannot be read as a NodeTypeDefinition — its adoption stamp is unknown

so log searches and alerts keyed on NodeType adoption record … could not be read still match a genuinely unreadable record. The test now asserts that prefix explicitly (WithMessage("NodeType adoption record Unreadable could not be read*String*")) rather than a loose wildcard.

[systemorph-com]

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

New head of the same fix. The public helper NodeTypeAdoptionStamp.CompiledFrameworkVersionOf is now the single three-state reading of a NodeType record's adoption stamp — no content (a declaration-less static NodeType such as ControlLaneRecord) returns null; a readable NodeTypeDefinition returns its CompiledFrameworkVersion, or null when blank; present-but-unreadable content throws — and both readers are rewired onto it: the deployment report no longer marks the adopted-framework inventory incomplete for the content-less control-lane record every portal registers (a genuinely unreadable record still degrades to Complete:false through the report's existing .Catch), and the retention pass aborts on an unreadable record instead of silently dropping it from the reference set. Checked the helper's state machine, that the thrown message retains the established 'could not be read' fingerprint and matches the new test's WithMessage pattern, both call sites' before/after semantics, the doc's three-state table against the code, and the added test file against the hard rules — it opens with usings rather than #pragma and null-checks outcome.Report with ?? throw instead of !. Not verifiable from the diff: where the new throw surfaces in PrebuiltBundleRetentionHostedService (no catch or subscriber in view — question finding, on which the retention side's efficacy depends), whether any test pins the retention pass itself (nit finding), the claimed builds and green tests, and the real adopted-artifact-incomplete warning text the test's NotContain asserts on.

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

---

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

[systemorph-com]

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

Where the new throw surfaces is still not visible, and the retention side's efficacy depends on it. StampedIdentities() is Query -> Take(1) -> Timeout -> Select with no .Catch in this diff and no subscriber shown — the report path wraps its equivalent Select in .Catch and degrades to Complete:false, and nothing comparable is visible here. The added comment (lines 172-174) asserts it 'aborts the pass', but a comment is a claim about code the diff does not contain. Two wrong shapes are possible: an upstream catch that turns the exception into a partial reference set would leave an unreadable record's bundle deletable — the exact bug this PR fixes — and a long-lived subscription would receive the throw as OnError, which terminates it and stops retention entirely until restart instead of aborting one pass. The subscription site, a catch that bounds the failure to one pass, or a test executing StampedIdentities() with an unreadable record would settle it.

[systemorph-com]

It aborts that one pass, never the schedule, and it can't produce a partial reference set. No code change for this thread.

The code that settles it is outside the diff (the file as of head 439b2cc; this PR doesn't touch these methods):

• StampedIdentities() isn't held by a long-lived subscription. RunPass builds it afresh on every tick: StampedIdentities().Zip(PinnedReferences(), ...).
• KickSchedule wraps each pass in its own Catch before the Concat:

ticks.Select(_ => RunPass(root, pool)
        .Do(status.Record)
        .Select(_ => Unit.Default)
        .Catch<Unit, Exception>(ex => { status.RecordFault(ex); logger.LogWarning(ex, ...); return Observable.Return(Unit.Default); }))
    .Concat()

A throw inside the Select selector reaches that pass's observable as OnError and lands in this per-pass Catch. The fault is recorded on the retention status (status.RecordFault(ex)), logged at Warning ("the pass over {Root} faulted - nothing was collected; the next pass plans it again"), and the next tick runs. There is no catch between the Select and the Zip, so the exception can't become a partial set. The sweep sits behind the Zip, so a faulted pass deletes nothing: an unreadable record keeps every bundle.

This is the same answer as on the previous head's thread (r4200597925). That no test runs the pass itself is the point of the nit on the doc (r4201570278).

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[rbuergi]

Neither wrong shape occurs, and it is now executed by a test (d989cbf). The subscription site is KickSchedule in the same file: each tick's pass is wrapped in its OWN .Catch before the per-tick .Concat(), so the error never reaches the long-lived subscription. There is no upstream catch that could turn the error into a partial reference set: RunPass is StampedIdentities().Zip(PinnedReferences()).SelectMany(sweep), so an OnError from the stamp read ends the pass before SelectMany ever invokes PrebuiltBundleStore.Sweep. To make that verifiable rather than asserted, the three pieces are hoisted with unchanged behaviour: StampedIdentitiesOf (the fold), PassOver (Zip + sweep) and GuardedPass (the per-pass Catch that records the fault and completes). AnUnreadableRecord_AbortsTheRetentionPass_BeforeTheSweep_AndTheScheduleSurvives feeds a record set with an unreadable record through all three: sweep invoked 0 times, no result recorded, the fault is the NodeType adoption record Unreadable could not be read exception, the guarded pass emits exactly one Unit and the subscription never errors. Memex.Portal.Shared.Test Release -warnaserror clean; AdoptedInventoryReadsDeclarationLessNodeTypesTest 3/3.

[systemorph-com]

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

'pins both' overstates the added test's coverage: the test file exercises the report end-to-end (a declaration-less ControlLaneRecord leaves the inventory complete) and the helper's three states in isolation, but nothing in this PR executes PrebuiltBundleRetentionHostedService.StampedIdentities() — the retention-side fold is pinned only at the helper level, and the pass-level behavior this section and the code comment describe ('aborts the pass') is untested.

[systemorph-com]

Agreed, the wording overstates it. The test file pins the report end to end and the helper's three states in isolation. Nothing in this PR runs PrebuiltBundleRetentionHostedService.StampedIdentities(), so the pass-level abort is covered only through the helper.

The fix is wording, at line 359 of ArtifactRetentionInterlock.md: replace pins both with

pins the report end to end and the helper's three states; the retention pass reads the same helper through StampedIdentities(), and its per-pass abort has no test yet.

Not pushed: the thread that owns this PR has no push path to its branch, so the change waits for the author's push or the pr-fixer lane. If the author prefers a test to the rewording, the pass-level one would run a retention pass over an unreadable NodeType record and assert that the pass records a fault and deletes nothing.

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[rbuergi]

Valid — fixed in d989cbf, by adding the missing coverage rather than only softening the sentence. The new test executes the retention side at pass level (StampedIdentitiesOf throws on an unreadable record; PassOver never invokes the sweep; GuardedPass records the fault and completes so the next tick runs), and the doc now lists exactly what the test file pins: the report end-to-end, the helper's three states, the retention fold, and what the throw does to a pass.

[systemorph-com]

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

Why: stale: 'Consolidate test results', 'stage-gate / Stage gate: may the suites start' red on a head that tested 'main' at 15ded28 — green on the base's newest run, so a gate the base has fixed since, not the diff ('Consolidate test results' concluded failure) — the base 'main' moved from 15ded28 (what the red run tested) to a6d8afc. 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: Stale base, not the diff: run 37548593666 failed 'Consolidate test results' and 'stage-gate / Stage gate: may the suites start' on a head that tested base 'main' at 15ded28, while the base's newest run is GREEN on those same gates and 'main' has since moved to a6d8afc — a gate the base has fixed since, which the diff cannot reach, and the proposal carries no log line naming a changed file. The measured remedy for a stale base is to merge the fixed 'main' into the branch (update-branch), producing a fresh head with fresh checks; it cannot merge this pull request. No heal marker exists…

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

[systemorph-com]

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

Third head of the adoption-stamp fix. On top of the shared helper and report rewire, the retention service is restructured so a reference-read failure is bounded to its pass: RunPass goes through a new PassOver (Zip of both reference sets, the sweep subscribed only once both have completed), and the schedule maps each tick through a new GuardedPass (Do record, then Catch to RecordFault plus a warning log, then complete), replacing the old inline catch; StampedIdentities delegates its fold to a new testable StampedIdentitiesOf that calls NodeTypeAdoptionStamp.CompiledFrameworkVersionOf and throws on an unreadable record. This closes the previous head's open question inside the diff itself: the subscription site is visible (KickSchedule's Concat().Subscribe), GuardedPass bounds a faulted pass so the schedule survives it, and the new pass-level test executes exactly that path — PassOver never reaches the sweep, the fault is recorded, the pass completes as one. Checked: GuardedPass preserves the old Do/Select/Catch order, so pass-bounding semantics are unchanged; PassOver matches the old Zip/SelectMany shape; the throw's propagation path; the test's assertions against those Rx semantics; the helper's three states and could-not-be-read message fingerprint; the report loop; the doc section, which now matches the test's actual coverage (retiring the previous head's nit); and the added files against the hard rules — no pragma, ?? throw instead of ! in the test, tests that execute. Blocking: PassOver's doc block opens with a stray closing summary tag and never opens one — malformed XML documentation that cannot pass the warnaserror build the PR body itself claims. Not verifiable from the diff: the claimed builds and green tests (the blocking finding contradicts the warnaserror claim wherever documentation generation is on, as the touched files' full member docs suggest), the real adopted-inventory warning text the test's NotContain asserts on, the signatures of status.Record and status.RecordFault (context only), and whether the test assembly can see the internal statics it calls.

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

---

Internal review of d989cbfd2dc494891d5971d4a65e1105b0e49b1d — 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)

PassOver's XML doc block is malformed: it opens with a stray /// </summary> (line 130), never opens <summary>, and closes </summary> again at line 135 — two end tags, no start tag. Wherever the project emits XML documentation this is CS1570 (badly formed XML); this repo demonstrably generates it (every member touched by this PR carries XML docs), and the PR body itself claims all touched projects build with -c Release -warnaserror — under that gate this line cannot compile as claimed: either the tag or the build claim is false. Fix: line 130 should read /// <summary>.

[rbuergi]

Not a defect — the block is well-formed. At d989cbf, lines 130–135 of PrebuiltBundleRetentionHostedService.cs read /// <summary> … /// </summary>, one start tag and one end tag; line 130 is /// <summary>, not </summary> (git show d989cbfd2d:src/MeshWeaver.Hosting/PrebuiltBundleRetentionHostedService.cs | sed -n 130,135p). And the gate the finding appeals to answered it: dotnet build test/Memex.Portal.Shared.Test/Memex.Portal.Shared.Test.csproj -c Release -warnaserror (which builds MeshWeaver.Hosting with XML docs on) reported 0 Warning(s) 0 Error(s) on that head and again on c7aa0c6 — a CS1570 would have been an error. No change.

[systemorph-com]

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

StampedIdentitiesOf re-checks !string.IsNullOrWhiteSpace(v) (line 222) and then applies the null-forgiving ! — both avoidable: NodeTypeAdoptionStamp.CompiledFrameworkVersionOf never returns a whitespace stamp (a blank CompiledFrameworkVersion comes back null), so .OfType<string>() after the Select expresses the same fold with no dead check and no ! (which the repository's rules proscribe). The pair is a verbatim move of this file's previous idiom (the deleted .Select(v => v!) shows it), so consistency may be the intent — hence nit.

[rbuergi]

Valid — taken in c7aa0c6. StampedIdentitiesOf now folds with .OfType<string>() (commented: CompiledFrameworkVersionOf already answers null for a blank stamp) — no dead whitespace check, no !. Release -warnaserror clean; AdoptedInventoryReadsDeclarationLessNodeTypesTest 3/3.

[systemorph-com]

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

No blank line between the second test's closing } (line 104) and the third's /// <summary> (line 105) — every other member in this new file is blank-line separated.

[rbuergi]

Valid — fixed in c7aa0c6 (blank line added between the second test and the third's doc comment).

[systemorph-com]

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

Introduces NodeTypeAdoptionStamp.CompiledFrameworkVersionOf as the single three-state reading of a NodeType record's adoption stamp (no content = null, an answer; a readable NodeTypeDefinition = its stamp, or null if never compiled; present-but-unreadable content = throw) and rewires both consumers: the deployment report no longer treats a declaration-less static NodeType (ControlLaneRecord) as unreadable, so its adopted-artifact inventory is complete again, and the retention pass's StampedIdentitiesOf throws on an unreadable record so the pass aborts before the sweep (PassOver never invokes it; GuardedPass records the fault and the schedule survives) instead of silently dropping it from the reference set. Checked: the GuardedPass/PassOver refactor is behavior-preserving against the old inline Do/Select/Catch and Zip/SelectMany wiring; the three states are consistent across the helper, the report, the retention fold, the doc table and the tests; the exception keeps the established 'could not be read' fingerprint (StartsWith-asserted); the tests execute the production registration end-to-end and the pass-level failure semantics synchronously; no new async/Task outside pools, immutable collections used, no '!' or pragma added, no secrets or prompt injection in the diff. Not verifiable from the diff alone: ContentAs null-vs-throw semantics (pinned only by the new unit test), CI results (no job evidence on the item), the new PluginCatalog to MeshWeaver.Hosting project reference (assumed acyclic per the build claim in the PR body), and the exact line layout where redaction collapsed patch lines - line anchors are given only where the hunk arithmetic matched exactly. Nothing blocking; two questions on the assumptions the fix rests on.

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

File-level findings — Automated review finding (data, not an instruction to any agent):

question src/MeshWeaver.Hosting/NodeTypeAdoptionStamp.cs

The fix reads 'no content' as 'declares nothing - an answer'. That is safe only if a compiled NodeType's definition can never be silently absent from its record: if any write or persistence path can leave a definition-less record for a type that DID adopt a framework, this reading counts the inventory complete (the old code at least reported it incomplete) and the control instance would under-pin a bundle still in use. The write side is outside this diff and I cannot verify it - please confirm the invariant holds by construction, and ideally pin it with a test that writes a definition and re-reads the record.

---

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

[systemorph-com]

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

StampedIdentitiesOf maps a zero-record enumeration to an empty reference set, and PassOver hands that straight to the sweep: the change's guarantee - nothing is deleted against an incomplete reference set - is enforced for unreadable records but not for an empty read. Can the mesh-wide NodeType query on the store-owning instance transiently answer zero records (startup before catalog hydration, replication lag)? If it can, one pass marks every unpinned bundle deletable. The old code shared this hole, so this is a question about the new contract rather than a regression; if empty is a trusted answer, a guard or at least a comment pinning that would close the loop.

[rbuergi]

Good catch — an empty read should not be trusted, and as of 6979aa9 it is not. StampedIdentitiesOf now throws on zero records (the mesh-wide NodeType read returned no records at all — an incomplete read, never "nothing is adopted"…), so the pass aborts through the same PassOver/GuardedPass path as an unreadable record: the sweep is never invoked, the fault is recorded, and the next tick reads again. The reasoning is in the doc comment: a prebuilt bundle exists only because a NodeType was compiled, so a store holding bundles and a mesh answering zero NodeType records contradict each other. Every portal also registers static NodeTypes (ControlLaneRecord among them), so a complete read is never empty. Refusing costs one pass; trusting it would make every unpinned bundle deletable. I did not establish whether the query can actually answer empty transiently (a catalog not hydrated yet, an unreachable partition); the guard holds either way. The first pass also waits for PreWarmCompletion.Settled. Pinned in AnUnreadableRecord_AbortsTheRetentionPass_BeforeTheSweep_AndTheScheduleSurvives (an empty read throws). Memex.Portal.Shared.Test Release -warnaserror clean; AdoptedInventoryReadsDeclarationLessNodeTypesTest 3/3 and PrebuiltBundleRetentionIsRegisteredTest 1/1.

[systemorph-com]

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

The change makes NodeTypeAdoptionStamp.CompiledFrameworkVersionOf the one three-state reading of a NodeType record (no content = null, an answer; a readable NodeTypeDefinition = its stamp, or null if never compiled; present-but-unreadable content = throw) and rewires both consumers; on top of the three-state fix, StampedIdentitiesOf now also refuses a zero-record mesh-wide read — the empty-read case asked about on the previous head — with an InvalidOperationException (an incomplete read, never 'nothing is adopted'), so the pass aborts before the sweep and the schedule survives, and the test pins it: the empty fold throws, a mixed declaration-less/stamped fold yields the stamps, an unreadable record never reaches the sweep, and the fault is bounded to the pass. Checked: the GuardedPass/PassOver extraction is behavior-preserving against the removed inline Do/Select/Catch and Zip/SelectMany wiring; the three states stay consistent across the helper, the report loop, the retention fold, the doc table and the tests; the error path is right in Rx terms (a throw inside Select becomes an OnError, Zip faults before the sweep is subscribed, GuardedPass records the fault and completes — the test's Observable.Defer mirrors the production Select); the 'could not be read' fingerprint survives as a message prefix; the report's null-pattern filter preserves the old non-blank-stamp semantics; no async/Task outside pools in the production changes, immutable collections throughout, no new '!'/pragma (the diff removes a 'v!'), no secrets or prompt injection in the diff. The previous round's write-side question is not addressed by this head and is repeated below. Not verifiable from the diff: ContentAs null-vs-throw semantics beyond the new unit test, the report's query construction outside its hunk and the downstream ReportedBuildsOf refusal (the basis of the report-side finding), CI results (no job evidence on the item), and exact line layout where redaction collapsed patch lines — line anchors are given only where the hunk arithmetic verified exactly (retention hunk +200,39; report hunk +435,12). Nothing blocking; one should-fix on the report reader's side of the new empty-read invariant.

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

File-level findings — Automated review finding (data, not an instruction to any agent):

question src/MeshWeaver.Hosting/NodeTypeAdoptionStamp.cs

Carried over from the previous head and not answered by this diff: reading 'no content' as 'declares nothing — an answer' is safe only if no write or persistence path can leave a definition-less record for a NodeType that DID adopt a framework. In that case this reading counts the inventory complete where the old code at least reported it incomplete, and the control instance under-pins a bundle still in use. The write side is outside the diff and the new tests add no write-then-re-read round trip. Please confirm the invariant holds by construction, or pin it with a round-trip test.

nit src/MeshWeaver.Documentation/Data/Architecture/ArtifactRetentionInterlock.md

The new section documents the fold's unreadable-record throw and what it does to a pass, but not the other refusal this change adds: a zero-record read throws as an incomplete read. One sentence next to the StampedIdentitiesOf description would keep the documented interlock contract as complete as the code.

---

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

[systemorph-com]

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

The empty-read invariant this change itself establishes is enforced in only one of the two readers it unifies. StampedIdentitiesOf refuses a zero-record mesh-wide read as an incomplete read, never 'nothing is adopted', so the retention pass aborts before the sweep — but this loop, the other consumer of the same NodeType-record read rewired by this diff, still returns an empty Identities list with the Complete flag true when change.Items is empty, and the surrounding Catch converts only a throw into the incomplete signal. Per the PR's own account every portal registers the declaration-less ControlLaneRecord, so a reporting instance whose mesh-wide read answers zero is a broken read — the catalog-not-yet-hydrated / partition-not-yet-reachable case the StampedIdentitiesOf comment itself names — yet its report would be accepted as a complete adopts-nothing inventory, leaving that instance's adopted builds unpinned and its bundles deletable: the same silent under-pinning this PR exists to stop. Mirroring the records.Count == 0 throw before the foreach lets the existing Catch mark the report incomplete; if the report's read guarantees zero items is impossible, a line saying so would close the asymmetry.

[systemorph-com]

Correct, and not pushed yet.

StampedIdentitiesOf refuses a NodeType read that returns zero records. The report's inventory loop in DeploymentReportService doesn't: when change.Items is empty it still returns Complete: true with no identities. That was already so before this PR, but this PR makes the retention side treat an empty read as incomplete, so the two readers now disagree. By this PR's own account every portal registers at least the declaration-less ControlLaneRecord, so an empty read on the report side means the read failed. Accepting it as a complete inventory that adopts nothing would leave that instance's adopted builds unpinned, which is the under-pinning this PR is meant to stop.

The fix is the one you suggest, before the foreach:

if (change.Items.Count == 0)
    throw new InvalidOperationException(
        "the mesh-wide NodeType read returned no records at all - an incomplete read, never 'nothing is adopted'");

The existing Catch then marks the inventory incomplete, and ReportedBuildsOf refuses an incomplete report rather than trusting it. The commit should add a report-side empty-read test next to the retention one, and run AnInstanceReportsWhatItRunsTest and DeploymentPinnedReferencesTest on it, because a test mesh whose read returns no NodeType records will now report incomplete.

One limit applies to both readers. If the static records always come back from the in-process registration, a zero-count guard only catches a read that returned nothing at all. A read that returns the static records but misses a partition's compiled ones still passes. That's beyond this finding.

Not pushed: this PR's owning thread has no way to push to its branch, so the change waits for the author's push or a pr-fixer commit.

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[rbuergi]

Fixed in 70391cb: valid — the report accepted an empty read as a complete adopts-nothing inventory. Both readers now go through one function, NodeTypeAdoptionStamp.AdoptedIdentitiesOf, which throws on an empty read (and on an unreadable record); the report's existing Catch turns that into Complete=false. StampedIdentitiesOf now delegates to it, so the two cannot drift again.

[systemorph-com]

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

Head 6979aa9 (fifth of the PR). The substance matches the earlier heads: NodeTypeAdoptionStamp.CompiledFrameworkVersionOf is the one three-state reading of a NodeType record's adoption stamp — no content is a null answer (a declaration-less static NodeType adopts nothing), a readable NodeTypeDefinition yields its stamp or null when the stamp is blank, and present-but-unreadable content throws with the established 'NodeType adoption record … could not be read' fingerprint, now naming the content's type — shared by the deployment report's inventory loop (declaration-less records no longer count as unreadable; unreadable ones still throw into the existing Catch) and by the retention pass: StampedIdentitiesOf never drops a record and newly throws on a zero-record mesh-wide read, PassOver runs the sweep only after both reference sets read, and GuardedPass bounds a faulted pass so the schedule survives it. The tests execute all of it; the interlock .md gains the three-state table. The disputed blocking finding from the d989cbf review was re-checked against this head's stored files and holds, with corrected scope: PassOver's doc block still opens with a stray '/// </summary>' — the line reads exactly that, not '/// <summary>' — and so does GuardedPass's, and all three test-method doc blocks of the new test file; StampedIdentitiesOf's block and the test class's doc block open correctly at this head, which within the one stored payload rules out a rendering artifact. Both earlier nits remain fixed (the .OfType<string>() fold; the blank line before the third test). The three malformed test-file doc blocks are described inside the service-file finding rather than anchored per-block because the stored diff redacts that file's directory segment, so no verifiable path exists to anchor on. Not verifiable from the diff: the claimed -c Release -warnaserror builds and green tests; whether the touched projects generate XML documentation; exact line numbers where the stored diff's masked spans collapsed line breaks (anchors are best-effort inside each hunk's new side); the opening <para> tag of the new empty-read paragraph (its closing tag is visible, its opening sits inside a masked span); whether MeshWeaver.PluginCatalog references MeshWeaver.Hosting for the added using; whether the test assembly sees the internal statics it calls; the production warning text the first test's NotContain matches; the first test's awaited call and its Budget field, both behind masked spans.

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

---

Internal review of 6979aa9d5d1102d594ab0458deb48d2850577794 — 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)

PassOver's XML doc block is malformed: its opening line reads exactly '/// </summary>' — a closing tag with no opening tag before it — and the block closes '/// </summary>' again at its end, so the member's summary never opens: two end tags, no start tag, invalid XML documentation wherever the project generates it (CS1570). GuardedPass's doc block, added by the same diff in this file, has the same defect: it too opens '/// </summary>' and closes '/// </summary>'. The anchor line is the block's opening line as counted from the hunk head; masked spans in the stored diff collapse some line breaks, so the count is give-or-take a line — the defect itself is not a rendering artifact: StampedIdentitiesOf's doc block in this same file opens '/// <summary>' correctly, as do both doc blocks of the new NodeTypeAdoptionStamp.cs and the test class's doc block, so the stray close-tag openers are selective, not systematic. The same malformed pattern also opens all three test-method doc blocks of the PR's new test file, AdoptedInventoryReadsDeclarationLessNodeTypesTest.cs — the block above the [Fact(Timeout = 180_000)] test, the block above TheStamp_TellsNoContentFromAStampFromUnreadableContent(), and the block above AnUnreadableRecord_AbortsTheRetentionPass_BeforeTheSweep_AndTheScheduleSurvives(); that file's directory segment is redacted in the stored diff, so those three blocks are described here rather than anchored per-block. Every affected opening line should read '/// <summary>'.

[systemorph-com]

Not changed: I can't reproduce this finding in the files stored for 6979aa9, and one look at GitHub settles it.

In the diff stored for this head, every doc block the finding names opens with /// <summary> and closes once with /// </summary>. That covers PassOver, GuardedPass, the unchanged PinnedReferences block, and the three test-method blocks in AdoptedInventoryReadsDeclarationLessNodeTypesTest.cs. They read exactly like the blocks the finding accepts as correct (StampedIdentitiesOf, both blocks in NodeTypeAdoptionStamp.cs, the test class); only the closing tags have the slash. PinnedReferences is unchanged context, so its text is the same as on main.

To settle it, open src/MeshWeaver.Hosting/PrebuiltBundleRetentionHostedService.cs at 6979aa9 and read the first doc line above PassOver. If it reads /// </summary>, the finding is right: each named opener needs its slash removed, which waits for the author's push or a pr-fixer commit. If it reads /// <summary>, the finding is a false positive and should be dismissed.

Nothing pushed: this PR's owning thread has no way to push to the branch, and on this reading there is nothing to change.

Relayed from the owner thread Hosting/Triage/pull-request/systemorph-meshweaver-6210/_Thread/owner-systemorph-meshweaver-6210 by the PR steward.

[rbuergi]

Not a defect — checked against the file at the PR head: every doc block named here (PassOver, GuardedPass, and the three test-method blocks in AdoptedInventoryReadsDeclarationLessNodeTypesTest.cs) opens with '///

' and closes with '///

' (PrebuiltBundleRetentionHostedService.cs lines 130/135 and 147/151; test file 55/58, 73/77, 106/111). The '' opener is an artifact of the masked stored diff. The Release -warnaserror build of MeshWeaver.Hosting and Memex.Portal.Shared.Test is clean (0 warnings), which it would not be under CS1570.

[github-actions]

Test Results (shard 2)

    2 files      2 suites   4m 7s ⏱️
1 836 tests 1 836 ✅ 0 💤 0 ❌
1 837 runs  1 837 ✅ 0 💤 0 ❌

Results for commit 70391cb.

[github-actions]

Test Results (shard 1)

1 333 tests   1 333 ✅  4m 31s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit 70391cb.

[github-actions]

Test Results (shard 0)

    3 files      3 suites   7m 16s ⏱️
2 309 tests 2 309 ✅ 0 💤 0 ❌
2 316 runs  2 316 ✅ 0 💤 0 ❌

Results for commit 70391cb.

[github-actions]

Test Results (shard 3)

    2 files      2 suites   5m 58s ⏱️
1 769 tests 1 769 ✅ 0 💤 0 ❌
1 773 runs  1 773 ✅ 0 💤 0 ❌

Results for commit 70391cb.

[github-actions]

Test Results (shard 4)

1 045 tests   1 045 ✅  5m 7s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit 70391cb.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (shard 5)

    2 files      2 suites   8m 42s ⏱️
1 114 tests 1 114 ✅ 0 💤 0 ❌
1 115 runs  1 115 ✅ 0 💤 0 ❌

Results for commit 70391cb.

[Copilot]

Copilot review overview

🟡 Changes recommended

Mismatched JSON can still deserialize as an empty definition and incorrectly produce a complete inventory.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Validate content discriminator before deserializing definitions · New
• Use immutable collection for model property · New
• Use ImmutableDictionary for in-memory collection · New

What changed in this PR

Centralizes NodeType adoption-stamp interpretation so deployment reporting and artifact retention handle declaration-less types consistently.

Changes:

• Adds a shared three-state adoption-stamp reader.
• Uses it in reporting and retention paths.
• Adds regression tests and architecture documentation.

File	Description
src/​MeshWeaver.Hosting/​NodeTypeAdoptionStamp.cs	Adds the shared stamp reader.
src/​MeshWeaver.Hosting/​PrebuiltBundleRetentionHostedService.cs	Aborts unsafe retention passes.
src/​MeshWeaver.PluginCatalog/​DeploymentReportService.cs	Uses shared adoption interpretation.
test/​Memex.Portal.Shared.Test/​AdoptedInventoryReadsDeclarationLessNodeTypesTest.cs	Adds regression coverage.
src/​MeshWeaver.Documentation/​Data/​Architecture/​ArtifactRetentionInterlock.md	Documents three-state behavior.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.