Skip to content

fix: an update validator sees the existing node typed like the proposed one (#3056 fallout) - #3085

Merged
meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/update-validators-see-the-existing-node
Sep 2, 2026
Merged

meshweaver-cloud[bot] merged 1 commit into
mainfrom
fix/update-validators-see-the-existing-node

Conversation

@rbuergi

@rbuergi rbuergi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

An update validator sees the existing node typed like the proposed one

Symptom. MeshWeaver.Plugins PR #1171 (run 33606589197, Portal hosts (shard 0)) failed
NodeOperationsWithUpdateValidatorTest.UpdateNode_VersionDowngrade_ShouldFail: Expected a
UnauthorizedAccessException, but no exception was thrown.
A validator-refused write went through.
The same shard passed on #1165 at 05:45Z; #1171 touches nothing that test reaches.

Bisect (core main, first-parent, the Plugins test run against each checkout):
470cdd58e pass · 02e9007c1 pass · 3ac34af52 pass · cb1dd3600 pass · 554f07e94 pass ·
804b9beca (#3059) pass · 3d6faa284 (#3056) FAIL · 9b3b00201 fail · 19035c1ea fail.

Mechanism. The failing run's log carries
MeshNodeStreamCache.GetStream: Content for update/validation/HighVersionNode stayed an untyped JsonElement after deserialization (TypeRegistry lacks the $type discriminator) … Raw: {"$type":"UpdatableContent",…}
— absent in every passing run. NodeUpdatePipeline reads the existing node through the stream cache,
which re-types the raw JSON with the running hub's registry. UpdatableContent is registered on no
hub in that fixture. Until #3056 that did not matter: MessageService.Post rendered every delivery
through the logging options EAGERLY (built as the LogDebug argument), and
ObjectPolymorphicConverter.Write calls typeRegistry.GetOrAddType for every value it serialises —
so a hub that had merely handed a typed instance to CreateNode had registered the type as a side
effect and could re-type the node when the pipeline read it back. #3056 removed the render (it was
the production OOM) and, with it, the accidental registration. ExistingNode.Content reached the
validator as a JsonElement while Node.Content was the caller's CLR instance; its
ExistingNode.Content is UpdatableContent && Node.Content is UpdatableContent was false, the if
was skipped, it answered Valid.

Fix (root, in the pipeline). Before building the NodeValidationContext,
NodeUpdatePipeline types the existing content like the proposal: the proposed node carries a
live CLR instance of exactly the type the existing content must have (same node, same NodeType), so
the degraded snapshot is deserialised as node.Content.GetType() on the hub that runs the
validators — a concrete-type deserialisation, which needs no registry entry. This is ContentAs<T>
done once for every validator; a new runtime-Type overload ObjectAsExtensions.As(object, Type, …)
carries it. A snapshot that will not deserialise as the proposal's type is left as it was and logged
at Error — hiding a shape mismatch would be a second silent pass. No registry is mutated on the read
path.

Test. UpdateValidatorSeesTypedExistingContentTest (MeshWeaver.Graph.Test): content type
registered on no hub, a fleet-shaped downgrade validator, the CLR type it saw recorded. Fails before
(seen == JsonElement, no exception), passes after. It lives in core because the test that found the
hole runs only in Plugins — the cross-repo blind spot that let #3056 merge green.

Docs. Doc/Architecture/UpdateValidatorsSeeTypedContent records the finding; What's New:
A rejected update is refused again (Fix).

Plugins follow-up (not in this PR). The fixture in NodeOperationsTest.cs should register
UpdatableContent on the hub (WithType) and NoVersionDowngradeValidator should read both sides
through .ContentAs<UpdatableContent>(hub.JsonSerializerOptions) instead of is — the pipeline
guarantee makes the existing validators safe; it does not make the cast idiom right.

Verification. MeshWeaver.Mesh.Contract, MeshWeaver.Hosting, MeshWeaver.Graph.Test build
-c Release -warnaserror clean; MeshWeaver.Graph.Test run whole; the Plugins
NodeOperationsWithUpdateValidatorTest filter re-run against this branch passes 4/4.

🤖 Generated with Claude Code
Claude-Session: https://claude.ai/code/session_01G4xPnGYEbdu8AVvR5jytyj

…ed one

Plugins' UpdateNode_VersionDowngrade_ShouldFail went red against core main
at 3d6faa2 (#3056): the validator saw ExistingNode.Content as a
JsonElement while Node.Content was the caller's CLR instance, skipped its
typed comparison and answered Valid — a refused write landed, unlogged.

Until #3056, MessageService.Post rendered every delivery through the
logging options eagerly and ObjectPolymorphicConverter.Write registered
each value's type on the hub as a side effect, so a hub that had merely
handed a typed instance to CreateNode could re-type the node when the
update pipeline read it back. #3056 removed that render (the OOM) and with
it the accidental registration.

NodeUpdatePipeline now types the existing content like the proposal —
node.Content.GetType() is the type authority for this update, and a
concrete-type deserialisation needs no registry entry — through a new
runtime-Type overload ObjectAsExtensions.As(object, Type, options, …).
A snapshot that will not deserialise as the proposal's type is left as it
was and logged at Error; no registry is mutated on the read path.

UpdateValidatorSeesTypedExistingContentTest pins the contract in core
(content type registered on no hub; fails before: seen == JsonElement).
Doc: Architecture/UpdateValidatorsSeeTypedContent. What's New: Fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G4xPnGYEbdu8AVvR5jytyj
Copilot AI lite review requested due to automatic review settings September 2, 2026 09:48
@meshweaver-cloud
meshweaver-cloud Bot enabled auto-merge September 2, 2026 09:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is targeted, adds a clear regression test, and the pipeline behavior aligns with the documented validator contract with only minor non-blocking nits noted.

Pull request overview

This PR restores and codifies the contract that IMeshService.UpdateNode update validators must see the existing node’s Content typed the same way as the proposed node’s Content, preventing validators from silently skipping comparisons when the existing content degrades to JsonElement (notably after #3056 removed the logging side-effect that previously registered types incidentally).

Changes:

  • Type the existing node content in NodeUpdatePipeline based on the proposal’s runtime CLR type before building NodeValidationContext.
  • Add a runtime-Type overload of ObjectAsExtensions.As(...) to support “type like this live instance” conversions without relying on hub registry entries.
  • Add a regression test plus documentation/What’s New entries capturing the guarantee and its historical failure mode.
File summaries
File Description
test/MeshWeaver.Graph.Test/UpdateValidatorSeesTypedExistingContentTest.cs Adds regression coverage ensuring validators observe typed existing content and that downgrades are refused.
src/MeshWeaver.Mesh.Contract/ObjectAsExtensions.cs Introduces As(object, Type, ...) for runtime-type conversion from degraded JSON/foreign types.
src/MeshWeaver.Hosting/NodeUpdatePipeline.cs Ensures ExistingNode.Content is re-typed like the proposal before validator execution.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-a-rejected-update-is-refused-again.md User-facing release note describing the restored refusal behavior.
src/MeshWeaver.Documentation/Data/Architecture/UpdateValidatorsSeeTypedContent.md Architecture note documenting the guarantee, failure mechanism, and current fix location.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

/// </summary>
public sealed class NoVersionDowngradeValidator : INodeValidator
{
private readonly ReplaySubject<Type?> observed = new();
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

234 tests   234 ✅  2m 22s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit e0518b8.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

430 tests  ±0   430 ✅ ±0   50s ⏱️ -8s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit e0518b8. ± Comparison against base commit fce1452.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

612 tests  ±0   612 ✅ ±0   49s ⏱️ -1s
  2 suites ±0     0 💤 ±0 
  2 files   ±0     0 ❌ ±0 

Results for commit e0518b8. ± Comparison against base commit fce1452.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    3 files  ±0      3 suites  ±0   1m 9s ⏱️ -7s
1 227 tests ±0  1 035 ✅ ±0  192 💤 ±0  0 ❌ ±0 
1 228 runs  ±0  1 036 ✅ ±0  192 💤 ±0  0 ❌ ±0 

Results for commit e0518b8. ± Comparison against base commit fce1452.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 798 tests  +2   1 798 ✅ +2   1m 44s ⏱️ -1s
    3 suites ±0       0 💤 ±0 
    3 files   ±0       0 ❌ ±0 

Results for commit e0518b8. ± Comparison against base commit fce1452.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

561 tests  ±0   559 ✅ ±0   2m 45s ⏱️ -3s
  4 suites ±0     2 💤 ±0 
  4 files   ±0     0 ❌ ±0 

Results for commit e0518b8. ± Comparison against base commit fce1452.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Test Results

   14 files  ±0     14 suites  ±0   9m 42s ⏱️ +27s
4 862 tests +2  4 668 ✅ +2  194 💤 ±0  0 ❌ ±0 
4 863 runs  +2  4 669 ✅ +2  194 💤 ±0  0 ❌ ±0 

Results for commit e0518b8. ± Comparison against base commit fce1452.

@meshweaver-cloud
meshweaver-cloud Bot merged commit 1fa88ec into main Sep 2, 2026
27 checks passed
@rbuergi
rbuergi deleted the fix/update-validators-see-the-existing-node branch October 10, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants