Repository navigation
fix(gate): the Tests probe of a partition-owning NodeType is created at the top level (#4602) - #4618
Conversation
…at the top level An instance of a NodeType whose definition declares `ownsPartition` IS a partition root, so its path is just its id — and since 693feae / ae0f706 the create validation chain refuses a nested one for EVERY owning type, not only `Space`. `CreateTestsProbe` built `{typePath}/GateProbe` unconditionally, so every such type came back could not create the Tests probe: InvalidOperationException: Cannot create 'Crm/Client/GateProbe': a 'Crm/Client' owns its partition, so it must be created at the top level, where its path is just its id ('GateProbe') — not under 'Crm/Client'. The failure is deterministic, so "retry known transients" correctly declined it and the repository's `main` stayed red: `publish-bake` never ran and every merged change became unshippable. The gate now asks `PartitionOwningTypes.OwnsPartitionWithoutActivating` — the SAME resolution `OwnsPartitionProvisioningValidator` consults on a nested create — so the two cannot disagree about a type, whichever route (static registry, durable row) answered. Exempting the gate from validation was the alternative and is the wrong one: the probe exists to exercise the create a real instance gets. A top-level probe's id has to carry the disambiguation the namespace gave the constant `GateProbe` for free: one run covers every type of a repo in one in-process mesh, so two owning types would collide on one path. The id embeds the type path, sanitised to satisfy `PartitionDefinition.IsValidPartitionSegment` (it becomes the partition, hence the schema, name) and closed with a stable SHA-256 prefix past the 63-byte cap. The test ships two partition-owning types in one fixture package, so it pins both halves. Measured on this worktree: with the fix reverted it fails with exactly the error above for both types; with it, both probes are created top-level as `GateProbeEstate_Tenant` / `GateProbeEstate_Branch` and both Tests areas run green. Fixes #4602 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The probe creation retains unsafe impersonation handling and its ID sanitization permits deterministic collisions for valid type paths.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates the plugin gate so partition-owning NodeTypes receive valid top-level test probes.
Changes:
- Resolves partition ownership before creating probes.
- Adds collision regression coverage for two owning types.
- Adds a What's New fix entry.
File summaries
| File | Description |
|---|---|
tools/MeshWeaver.PluginTester/PluginGateRunner.cs |
Creates probes at the correct scope and generates IDs. |
test/MeshWeaver.PluginTester.Test/PluginGateRunnerTest.cs |
Tests multiple partition-owning types. |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-17-a-package-whose-type-owns-a-partition-can-be-published-again.md |
Documents the user-visible fix. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| .SelectMany(probe => Observable.Using( | ||
| () => access.ImpersonateAsSystem(), | ||
| _ => meshService.CreateNode(probe))) |
There was a problem hiding this comment.
Agreed, and fixed in 480c010. The site now uses the sealed access.RunAsSystem(() => meshService.CreateNode(probe)) helper, which enters the scope at Subscribe and leaves it on the way out of that same Subscribe, so the subscriber is never latched. It was this file's only such site, so its line is gone from test/ImpersonationScopeSites.allow and TotalBudget follows it 64 → 63 in the same change — the tidy the guard's STALE report asks for.
| var sanitized = string.Concat(typePath.Select(c => | ||
| char.IsAsciiLetterOrDigit(c) || c is '.' or '-' or '_' ? c : '_')); | ||
| var candidate = "GateProbe" + sanitized; | ||
| if (Encoding.UTF8.GetByteCount(candidate) <= 63) |
There was a problem hiding this comment.
Correct, and fixed in 480c010 — Estate/A/B and a type literally named Estate/A_B both sanitise to GateProbeEstate_A_B, and both are valid literal type paths, so the collision was reachable. The digest is now UNCONDITIONAL and is taken over the UNSANITISED path, so the two stay apart whatever the sanitiser does to them; the head is cut to 45 chars so GateProbe(9) + head + _ + 8 hex still fits the 63-byte partition-name cap.
TestsProbeId_IsInjective_AcrossPathsThatSanitiseAlike (PluginGateRunnerTest) asserts exactly that pair, the long-shared-head pair, and that every id it produces satisfies PartitionDefinition.IsValidPartitionSegment. It is a direct unit test rather than a gate fixture because the gate fixture cannot reach the case — its two owning types differ before the collapse.
…he sealed scope Both automatic-review findings, both valid. 1. `Observable.Using(() => access.ImpersonateAsSystem(), ...)` enters the AsyncLocal scope on the SUBSCRIBING thread and leaves it on whichever thread the create terminates on, so the subscriber stays latched as system-security (#1790). The site moves to the sealed `access.RunAsSystem(...)` helper. It was the file's only one, so its allow line goes and `TotalBudget` follows it 64 to 63 — the tidy the guard's STALE report asks for, in the change that makes it stale. 2. `TestsProbeId` was not injective. Every disallowed character collapses to `_`, so `Estate/A/B` and a type literally named `Estate/A_B` — both valid literal type paths — sanitise to the same string, and appending the digest only past the length cap left that collision open. It would surface as the SECOND owning type's probe failing already-exists: the failure this change exists to prevent, one door along. The digest is now unconditional and is taken over the UNSANITISED path, so the two stay apart whatever the sanitiser does to them; the head is cut to 45 so the 63-byte partition-name cap still holds. `TestsProbeId_IsInjective_AcrossPathsThatSanitiseAlike` asserts the collision case directly — the gate fixture cannot reach it, because its two types differ before the collapse — plus the long-path case and that every id it produces satisfies `PartitionDefinition.IsValidPartitionSegment`. Measured: MeshWeaver.PluginTester.Test 396/396, the impersonation ratchet guard 3/3, `-c Release -warnaserror` clean on both projects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…→ 62) #4618 lowered it 64 → 63 by removing PluginGateRunner's line; this branch lowered it 64 → 63 by taking InstanceAutoRegistrationService from 6 to 5. Both are real, so the merged budget is 62 — exact against the merged allow file (33 entries, sum 62). Taking either side alone would have left the constant one ABOVE the file, which is the free slot the exact seed exists to remove. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The defect
A NodeType whose definition declares
ownsPartitionhas instances that ARE partition roots, sotheir path is just their id. Since
693feae92("every partition-owning NodeType can be created atthe top level, not only Space") and
ae0f706b3(#4449 item 3) the create-validation chain refuses aNESTED instance of every owning type — but
PluginGateRunner.CreateTestsProbestill built{typePath}/GateProbeunconditionally, so the probe could no longer be created for such a type:Deterministic, so the transient-retry steward correctly declined it. Any module repo with a
partition-owning NodeType could not be green on
main,publish-bakenever ran, and merged workbecame unshippable (MeshWeaver.Crm pinned at
79c5ceb2, 2026-09-15).The fix
CreateTestsProbeasksPartitionOwningTypes.OwnsPartitionWithoutActivating— the sameresolution
OwnsPartitionProvisioningValidatorconsults on a nested create — and places the probeat the top level when the answer is yes. Gate and validator therefore cannot disagree about a type,
whichever of the two routes (static registry, durable row) produced the answer. Exempting the gate
from validation was the alternative and is the wrong one: the probe exists to exercise the same
create a real instance gets.
Addressing the caveat raised on the issue: a top-level id has to carry the disambiguation the
namespace gave the constant
GateProbefor free — one run covers every type of a repo in a singlein-process mesh, so two owning types would collide on one path.
TestsProbeIdembeds the type path,sanitised so it satisfies
PartitionDefinition.IsValidPartitionSegmentby construction (the idbecomes the partition, and so the schema, name), and closes an over-long one with a stable SHA-256
prefix rather than a Guid, so two runs of the same repo stay readable side by side.
Evidence
PartitionOwningNodeType_TestsProbeIsCreatedAtTopLevel_AndTwoOfThemDoNotCollideships a fixturepackage with TWO partition-owning types, so it pins both halves. Measured on this worktree:
GateProbeEstate_Tenant/GateProbeEstate_Branchcreated at the top level,both
Testsareas execute green,ALL GREEN, exit 0.total: 395, failed: 0(the non-owning fixtures still use the nested probe).dotnet build -c Release -warnaserrorclean onMeshWeaver.PluginTesterand its test project.Fixes #4602
Pairs-with: none — no public type or member leaves
src/; the change is insidetools/MeshWeaver.PluginTesterplus a new What's New entry.🤖 Generated with Claude Code