Repository navigation
ActivationRecycle: recycle a NodeType's live activations on every process - #6054
Conversation
…cess
A new build of a NodeType reaches only activations created after it; the
existing per-node hubs keep the build they were born with until something
disposes them. ActivationRecycle is the type-scoped recycle:
- ActivationRecycle.Request writes Admin/_Recycle/{id} as System. The durable
write IS the broadcast: every process hears it on its IMeshInvalidationFeed
(LISTEN/NOTIFY in production), no cluster-singleton grain.
- Each process walks its mesh hub's HostedHubsCollection, keeps the per-node
hubs bound from a requested NodeType (NodeTypePathHolder) that are still
running, and posts one DisposeRequest each off the router, with the
request's reason, in paced batches (20 per 500 ms). No index of stored
instances is read, so cold addresses are never touched.
- Each process writes {request}/{process} (ActivationRecycleReport) as its
audit; a request path is handled once per process.
- Refused with nothing written: no reason, nothing named, unknown NodeType,
or the definition type NodeType itself.
Tests: ActivationRecycleTest (monolith: exactly T's two live activations,
fresh reactivation, redelivery, refusals) and TwoSiloActivationRecycleTest
(both silos' activations disposed, one report per silo matching what each
hosted). The two-silo fixture gains an opt-in shared storage change feed
(the LISTEN/NOTIFY shape) via an InMemoryStorageAdapter overload.
Docs: StaleStateUntilRecycle "Recycling a type"; AGENTS.md core-only note.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Test Results (shard 0) 3 files 3 suites 3m 11s ⏱️ Results for commit 7de3223. ♻️ This comment has been updated with latest results. |
Test Results (shard 2) 5 files 5 suites 3m 9s ⏱️ Results for commit 7de3223. ♻️ This comment has been updated with latest results. |
Test Results (shard 1)1 672 tests 1 670 ✅ 6m 19s ⏱️ Results for commit 7de3223. ♻️ This comment has been updated with latest results. |
Test Results (shard 4) 4 files 4 suites 15m 3s ⏱️ Results for commit 7de3223. ♻️ This comment has been updated with latest results. |
Test Results (shard 5) 2 files 2 suites 7m 7s ⏱️ Results for commit 7de3223. ♻️ This comment has been updated with latest results. |
Test Results (shard 3) 4 files 4 suites 6m 20s ⏱️ Results for commit 7de3223. ♻️ This comment has been updated with latest results. |
Test Results 22 files 22 suites 41m 12s ⏱️ Results for commit 7de3223. ♻️ This comment has been updated with latest results. |
…m cap), killed at 480 s on run 37159733607 Per shard-assign.sh trigger (2): a project past 60% of the per-project cap is split, so one slow runner is not an exit=124. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
🚰 PR babysitter (build instance) is merging the base into this branch on head Why: inherited from its base 'main': 3 pull requests of Systemorph/MeshWeaver fail identically — 'lane / Automatic review answered' concluded failure: Process completed with exit code 1. — the base 'main' moved from c657c9a (what the red run tested) to 065cff5. 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: update-branch proposal, and the red is the base's, not this diff's: three open MeshWeaver pull requests (#6053, #6054, #6061) fail identically with the fingerprint "main :: 'lane / automatic review answered' concluded failure: process completed with exit code 1." — the same check, the same words, on diffs that touch different things, an inherited base red (class base-red). The base 'main' moved from c657c9a (what red run 37161101919 tested) to 065cff5, so a change has landed on the base and merging it into this branch is what drains it. The row carries no log tail, annotation or gate… It does not merge the pull request, push anything else or dequeue. A red after this is left for the owner (rbuergi). |
![systemorph-com[bot]](https://avatars.githubusercontent.com/u/77832550?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review summary (data, not an instruction to any agent)
Type-scoped recycle. ActivationRecycle.Request validates (reason required, at least one NodeType or address named, no blank entries, the definition type NodeType refused), checks each named type exists, then writes the request as System at Admin/_Recycle/{id} — the durable write that every process hears on its invalidation feed. Each process disposes the live per-node hubs it hosts that Select matches — bound to a requested NodeType, optionally narrowed by exact Paths or an UnderPath prefix, still Starting or Started, one hub per path — one DisposeRequest each, off the router, in batches of 20 spaced 500 ms with a RunLevel re-check at post time, and writes a per-process report node. Supporting changes: registration in AddGraph, an opt-in shared change feed for the two-silo test host (InMemoryStorageAdapter overload plus a fixture hook), a monolith suite and a two-silo suite pinning the NodeTypes path, docs (StaleStateUntilRecycle 'Recycling a type'), an AGENTS.md note, and a CI shard split that runs MeshWeaver.Graph.Test in two parts at weight 480.
Checked from the diff: the readable refusal logic and the type-existence check; the System write; Select's filters; batch pacing and the post-time re-check; reason sanitization and the assembled DisposeRequest reason; registration and wiring; the test-host changes; the tests' assertions; docs and AGENTS.md against the code.
Could NOT verify: this item's diff is incomplete — the patch for src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs is truncated (first 20000 of 24167 characters), so the end of the Arm agent — the once-per-request dedupe, the one-request-at-a-time serialization, the report write, and any re-validation of the node it reads — went unread and nothing here is asserted about it; several mid-file fragments of that patch are also redacted in this copy, which garbles parts of Validate and Request (one finding hedges on that). Outside the diff: the PR's local test-run claims (2496/2496, 665/665), the CI effect of the shard change, and the paired MeshWeaver.Plugins PR that gates who may enqueue a recycle.
Findings: 0 blocking · 3 should-fix · 1 question · 2 nit
File-level findings — Automated review finding (data, not an instruction to any agent):
should-fix src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
Validate refuses the definition type NodeType — 'it would recycle every NodeType definition hub of the mesh' — but nothing readable in this diff refuses this feature's own node types: ActivationRecycleRequest.NodeType (ActivationRecycle) and ActivationRecycleReport.NodeType (ActivationRecycleReport). Both are registered static node types, so TypeExists accepts them, and Select matches exactly the hubs bound to them — a request naming either would make every process dispose any live request- and report-node hubs, this feature's own audit nodes. Validate's doc comment, the new docs section and the AGENTS.md note each enumerate the refusal set (reason, names-nothing, blank, unknown type, NodeType itself) and none names them; the NodeType refusal exists for precisely this class of accident, and the feature's own two types belong on the same list. The middle of the NodeType check is redacted in this review copy; if that predicate already refuses them this finding is moot — but then the doc comment, the docs section and the AGENTS.md note all disagree with the code.
should-fix src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
The DisposeRequest reason assembled in RecycleLocal interpolates request.RequestedBy verbatim and runs SanitizeReason over request.Reason only. requestPath is Segment-sanitised by construction, but RequestedBy is documented free text ('a user id, a job path, a rule name') passed to a public API, and the assembled string becomes each target's [QUIESCE-START] line, which the incident filer fingerprints. A RequestedBy carrying newlines or control characters forges further QUIESCE-START lines — exactly what SanitizeReason's own doc comment exists to prevent. RequestedBy belongs behind the same flattening and 300-char bound before it is interpolated, unless the disposal path already sanitises DisposeRequest.Reason downstream — nothing in this diff shows that.
question src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
Cannot be told from this diff copy, and both points carry the PR's 'no new rights for anyone' claim: (a) whether the mesh write path refuses a non-System create of an ActivationRecycle node in Admin/_Recycle — the registration sets ExcludeFromContext to search and create, and Request itself writes via RunAsSystem, but neither fact establishes the gate, and whoever can write such a node directly bypasses Validate entirely and makes every process dispose the live hubs of whatever types the node names; (b) whether Arm re-validates the node it reads (reason present, no blank or unknown types) before disposing. Arm's body is beyond the truncation of this copy, so this is asked rather than asserted.
nit src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
Request and Reports resolve AccessService with GetService and then dereference it in access.RunAsSystem with no null guard, while Arm in the same file null-guards its service lookups, and the adjacent GetRequiredService for IMeshService at least names the missing service. On a hub whose provider lacks AccessService this fails with a bare NullReferenceException instead of a diagnosable error — GetRequiredService, or a guard that refuses, matches the file's own style.
Internal review of b94c6d6d2b6a708a76ae8c62d51b0672487f8caa — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
| @@ -0,0 +1,187 @@ | |||
| using System; | |||
There was a problem hiding this comment.
should-fix — Automated review finding (data, not an instruction to any agent)
Neither suite exercises the Paths or the UnderPath narrowing of ActivationRecycleRequest, although both are advertised (PR body and the new docs section: 'optionally narrowed to some addresses, or to the activations under one path') and both carry non-obvious logic in Select: Trim of slash runs, case-insensitive comparison, the exact-or-under-slash-prefix rule, and the mode where NodeTypes is empty and any type at a named path matches. Two cases would pin them: a Paths-narrowed recycle sparing a same-typed sibling, and an UnderPath-narrowed one sparing the same type outside the path.
There was a problem hiding this comment.
Fixed in 54c1-series commit 'ActivationRecycle: review …': APathsNarrowedRecycle_SparesTheSameTypeSibling (Paths matched slash-trimmed and case-insensitively) and AnUnderPathNarrowedRecycle_SparesTheSameTypeOutsideThePath. The NodeTypes-empty mode no longer exists: Validate now requires at least one NodeType, and Paths/UnderPath only narrow it.
| builder.AddUiContributionType(); | ||
| builder.ConfigureServices(s => s.AddSingleton<UiContributionCatalog>()); | ||
| // Type-scoped recycle (Admin/_Recycle): every process disposes the live activations it | ||
| // HOSTS of the requested NodeTypes — the per-process LiveActivationRegistry is filled by |
There was a problem hiding this comment.
nit — Automated review finding (data, not an instruction to any agent)
The comment says 'the per-process LiveActivationRegistry is filled by MeshNodeHubFactory', but the code it introduces walks the existing HostedHubsCollection (RecycleLocal resolves it from the mesh hub), as do the PR body, the AGENTS.md note and the docs section; no LiveActivationRegistry appears anywhere in this diff. A stale name from an earlier iteration — the comment and the code disagree.
There was a problem hiding this comment.
Fixed: the comment now names HostedHubsCollection; LiveActivationRegistry was an earlier iteration and is gone.
|
🚰 PR babysitter (build instance) is merging the base into this branch on head Why: inherited from its base 'main': 3 pull requests of Systemorph/MeshWeaver fail identically — 'lane / Automatic review answered' concluded failure: Process completed with exit code 1. — the base 'main' moved from 065cff5 (what the red run tested) to 1144fb5. 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: The red is the base's, not this diff's: 3 open pull requests of Systemorph/MeshWeaver (#6053, #6054, #6065) fail identically on 'lane / Automatic review answered' concluded failure: Process completed with exit code 1 — the same check, the same words, fingerprint 'main :: lane / automatic review answered concluded failure'. The red runs tested base main at 065cff5; main has since moved to 1144fb5, so the failure was inherited before this branch was ever tested against the current base. Approving the proposed update-branch (merge the moved base into the branch) so the failed runs are r… It does not merge the pull request, push anything else or dequeue. A red after this is left for the owner (rbuergi). |
There was a problem hiding this comment.
Automated review summary (data, not an instruction to any agent)
Type-scoped recycle. ActivationRecycle.Request validates (reason required, at least one NodeType or address named, no blank entries, the definition type NodeType refused), checks each named NodeType exists, then writes the request as System at Admin/_Recycle/{id} — the durable write that every process hears on its invalidation feed. Each process's agent disposes the live per-node hubs it hosts that Select matches — bound to a requested NodeType, optionally narrowed by exact Paths or an UnderPath prefix, still Starting or Started, one hub per path — one DisposeRequest each, off the router, in batches of 20 spaced 500 ms with a RunLevel re-check at post time, and writes a per-process report node. Supporting changes: registration in AddGraph, an opt-in shared change feed for the two-silo test host (InMemoryStorageAdapter overload plus a fixture hook), a monolith suite and a two-silo suite, docs (StaleStateUntilRecycle "Recycling a type"), an AGENTS.md note, and a CI shard split running MeshWeaver.Graph.Test in two parts at weight 480. Checked from the diff: the readable refusal logic and the type-existence check; the System write; Select's filters; batch pacing and the post-time re-check; reason sanitization and the assembled DisposeRequest reason; registration and wiring; the test-host changes; the tests' assertions; docs and AGENTS.md against the code. Could NOT verify: this item's diff is incomplete — the patch for src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs is truncated (first 20000 of 24167 characters), so the tail of the Arm agent — the once-per-request dedupe, the one-request-at-a-time serialization, the report write, and any re-validation of the node it reads — went unread and nothing here is asserted about it; several mid-patch fragments of that file and of both test files are also scrubbed in this copy, which garbles parts of Validate, Request and RecycleLocal — the affected findings hedge on exactly those spots. Outside the diff: the PR's local test-run claims (2496/2496, 665/665), the CI effect of the shard change, and the paired MeshWeaver.Plugins PR that gates who may enqueue a recycle. A review of the previous head b94c6d6d raised the same should-fix points (the feature's own node types absent from the refusal set, RequestedBy unsanitized in the DisposeRequest reason, no Paths/UnderPath cases in either suite); each is still present in this head's cumulative diff.
Findings: 0 blocking · 3 should-fix · 2 question · 2 nit
File-level findings — Automated review finding (data, not an instruction to any agent):
should-fix src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
Validate refuses a request naming the definition type — the MeshNode.NodeTypePath check answers "'NodeType' is refused: it would recycle every NodeType definition hub" — but as far as this copy shows nothing refuses this feature's own node types, ActivationRecycleRequest.NodeType ("ActivationRecycle") and ActivationRecycleReport.NodeType ("ActivationRecycleReport"). Both are registered node types (AddActivationRecycle registers both), so a request naming either passes Validate, and Select matches exactly the hubs bound to them: every process would dispose its live request- and report-node hubs — this feature's own audit nodes. The NodeType refusal exists for precisely this class of accident, and the refusal sets written in the Validate doc comment, the new docs section and the AGENTS.md note name only reason / names-nothing / blank / unknown type / NodeType. The middle of the NodeType check is scrubbed in this review copy; if that predicate already refuses the two ActivationRecycle types the finding is moot — but then the doc comment, the docs section and the AGENTS.md note all understate the code.
should-fix src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
The DisposeRequest reason assembled in RecycleLocal prefixes "ActivationRecycle '{requestPath}' (requested by {request.RequestedBy …}): " to SanitizeReason(request.Reason) — only request.Reason goes through SanitizeReason, while request.RequestedBy is interpolated verbatim. RequestedBy is documented free text ("a user id, a job path, a rule name") on a public API, and the assembled string is rendered into each target's [QUIESCE-START] line, which the incident filer fingerprints — exactly the log forgery that SanitizeReason's own doc comment ("the same log-forgery rule MeshOperations.SanitizeReason states") exists to prevent; RequestedBy also escapes the 300-character bound. It belongs behind the same flattening and bound before interpolation, unless DisposeRequest.Reason is sanitised somewhere downstream — nothing in this diff shows that.
question src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
Cannot be told from this diff copy, and both points carry the PR's "no new rights for anyone" claim: (a) whether the mesh write path refuses a non-System create of an ActivationRecycle node in Admin/_Recycle — the registration sets ExcludeFromContext {search, create} and Request itself writes via RunAsSystem, but neither fact establishes the gate, and the durable write IS the broadcast: whoever can write such a node directly bypasses Validate entirely and makes every process dispose the live hubs of whatever the node names, including NodeType; (b) whether Arm re-validates the node it reads (reason present, no blank or unknown types) before disposing. Arm's body is beyond this copy's truncation, so this is asked rather than asserted.
question src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
The id parameter of Request is documented as "a dedupe key: a second request with the same id is not written again", but the readable body of Request contains no dedupe: it derives nodeId = Segment(id or a truncated default) and calls mesh.CreateNode once. Segment maps every non-alphanumeric character to '-' and collapses empty runs, so distinct ids ("build.42" and "build-42") collapse onto one node path, and a caller-provided id is unbounded in length (only the generated default is cut at 30). Whether a second write to the same path is refused, errors, or succeeds idempotently — which is what the documented dedupe amounts to — depends on mesh.CreateNode's behavior for an existing path, which this diff does not show; the documented behavior is implemented nowhere visible here.
nit src/MeshWeaver.Graph/Configuration/ActivationRecycle.cs
Request and Reports resolve AccessService with GetService and then call access.RunAsSystem with no null guard, while Arm null-guards its service lookups and the adjacent IMeshService is at least resolved with GetRequiredService. On a hub whose provider lacks AccessService this fails with a bare NullReferenceException instead of a diagnosable error — GetRequiredService, or a guard that refuses, matches the file's own style.
Internal review of 4fcd8d5990e79297443735cbca74c50f03642f7d — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
| @@ -0,0 +1,187 @@ | |||
| using System; | |||
There was a problem hiding this comment.
should-fix — Automated review finding (data, not an instruction to any agent)
Neither suite exercises the Paths or the UnderPath narrowing of ActivationRecycleRequest, although both are advertised (PR body and the new docs section: "optionally narrowed to some addresses, or to the activations under one path") and both carry non-obvious logic in Select: slash-trimmed, case-insensitive path sets; the exact-or-under-slash-prefix rule; and the NodeTypes-empty mode, in which a Paths-only request matches ANY bound type at the named addresses — TypeExists runs only over NodeTypes, so a Paths-only request also skips the type check entirely. Two cases would pin them: a Paths-narrowed recycle sparing a same-typed sibling, and an UnderPath-narrowed one sparing the same type outside the path.
There was a problem hiding this comment.
Same fix as the thread above: both narrowing cases are pinned, and a Paths-only request is refused (a request is always type-scoped, so the type check always runs).
| builder.AddUiContributionType(); | ||
| builder.ConfigureServices(s => s.AddSingleton<UiContributionCatalog>()); | ||
| // Type-scoped recycle (Admin/_Recycle): every process disposes the live activations it | ||
| // HOSTS of the requested NodeTypes — the per-process LiveActivationRegistry is filled by |
There was a problem hiding this comment.
nit — Automated review finding (data, not an instruction to any agent)
The added comment says "the per-process LiveActivationRegistry is filled by MeshNodeHubFactory", but no LiveActivationRegistry appears anywhere in this diff: RecycleLocal resolves the existing HostedHubsCollection from the mesh hub, and the PR body, the AGENTS.md note and the docs section all name HostedHubsCollection. A stale name from an earlier iteration — the comment and the code disagree.
There was a problem hiding this comment.
Fixed, same as above.
…d dedupe, sanitize requester - Validate: a request is always type-scoped (Paths/UnderPath only narrow); NodeType and the feature's own ActivationRecycle/ActivationRecycleReport types are refused. - Each process acts only on a request node created by System and re-validates it, so writing Admin/_Recycle directly (a platform admin holds Admin) bypasses nothing. - The request id is a real dedupe key (an existing request is answered, not rewritten); a malformed or over-long id is refused instead of collapsed. - RequestedBy goes through SanitizeReason before it reaches [QUIESCE-START]. - AccessService resolved with GetRequiredService; the agent arms only with one. - Tests: Paths narrowing spares a same-type sibling, UnderPath spares the type outside the subtree, id dedupe + malformed id, a non-System request disposes nothing (falsified), own types refused. Docs updated. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Answers to the file-level findings of the internal review (fixed in
Shard 4's red in the previous run was |
1 participant
What
Type-scoped recycle: dispose every LIVE activation of a NodeType, on every process of the mesh, with an audit. This is the core half of the maintainer's direction (2026-10-04): "system should have rights ⇒ must be called by build queue as system" and "can't we put recycle automatically to all nodes of this type?".
A new build of a NodeType reaches only activations created after it; existing per-node hubs stay on the previous build until disposed (StaleStateUntilRecycle). On the control instance the bug-fix pool kept serving pre-import code for hours because nothing recycled them and a platform admin cannot (no Update on
Hosting, by design).How
ActivationRecycle.Request(hub, request)writesAdmin/_Recycle/{id}(nodeType: ActivationRecycle) as System. The durable write is the broadcast: every process hears that commit on itsIMeshInvalidationFeed(PostgreSQL LISTEN/NOTIFY in production; in-process on a monolith). No cluster-singleton grain.HostedHubsCollection(the existing silo-local hosted-hub registry), keeps the per-node hubs whoseNodeTypePathHoldernames a requested type and that are still running, and posts oneDisposeRequesteach off the router (NodeOperationIssuingHub()), with the request's reason (sanitised), in paced batches (20 per 500 ms). No index of stored instances is read; cold addresses are never touched; each target quiesces normally.{request}/{process}(ActivationRecycleReport: matched, disposed, addresses). Request + reports = the audit (who, when, why, types, per-silo counts).NodeTypeitself.No new rights for anyone: the request is a System write. In MeshWeaver.Plugins the build queue's
recyclejob issues it (on a NodeType's new build, or enqueued by a platform admin over MCP — enqueueing is a platform action). The directrecycleverb (Update holders) is unchanged.Tests (executed locally)
ActivationRecycleTest(Graph.Test, monolith): two live activations of T and one of U → exactly the two disposed, one report (matched 2 / disposed 2), T reactivates on a FRESH hub; a redelivered request disposes nothing again; a hub already tearing down is not selected; unknown type / blank reason /NodeTyperefused with nothing written. Falsified: removing the type filter or the per-process dedupe reds two cases.TwoSiloActivationRecycleTest(Orleans.Test): T's 12 activations spread over both silos, all disposed, U spared, two reports whose counts equal what each silo hosted. Falsified: without the shared change feed (silo 2 never hears the commit) it reds.MeshWeaver.Graph.Test: 2496/2496.MeshWeaver.Documentation.Testguards: 665/665. Orleans guards (cluster sizing, grain activation sites) green.The two-silo fixture gains an opt-in shared storage change feed (
ShareStorageChanges, the LISTEN/NOTIFY shape) through a newInMemoryStorageAdapterconstructor overload; existing fixtures are unchanged.Docs
Doc/Architecture/StaleStateUntilRecycle→ "Recycling a type"; AGENTS.md core-only note under the DisposeRequest section.Paired: MeshWeaver.Plugins PR (Hosting/Queue
recyclejob, new-build rule, MCPenqueue_recycle) consumesActivationRecycleonce this reaches a sealed set.🤖 Generated with Claude Code