Skip to content

fix: the install no longer waits 30 s for a cover grant it documents as optional - #2849

Merged
rbuergi merged 1 commit into
mainfrom
fix/gating-settle-stall
Aug 31, 2026
Merged

rbuergi merged 1 commit into
mainfrom
fix/gating-settle-stall

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

The defect

PackageInstaller waited up to 30 seconds for a partition's cover grant while the wait's own doc comment called that grant optional — "a partition whose node type does not gate never writes it" — and the catch's own message called the missing grant "normal — a course copy or a plain partition".

Both statements were true at once, and together they made a deadlock that never went red. On the shape the code itself called normal the awaited node is never written, so the query can never emit: every install burned the entire budget and then proceeded green, leaving one Information line worded to cover both outcomes — so the healthy install and the wedged one printed the same sentence.

Measured in MeshWeaver.PluginCatalog.Test on origin/main:

Test Installs Duration
SelfTypedRootInstallTest.SelfTypedRoot_ReinstallImmediately_WritesNothing 2 60.28 s
StaleStampRootBindingTest.StaleStampSelfTypedRoot_RootServesItsTypesArea 1 30.33 s
StaleStampRootBindingTest.StaleStampSelfTypedRootShippingContent_RootServesItsTypesArea 1 30.29 s
SelfTypedRootInstallTest.RootTypedByAnInPackageNodeType_Installs 1 30.25 s
InstallReleaseOrderingTest.DeferredNodeTypeReleases_AreRequestedAfterTheRootRecycle 1 30.24 s

One install, one 30 s stall — 181 s of a 336 s suite. PackageInstaller.Install is the production install path, so a live mesh paid the same 30 seconds for every package that does not gate.

The discriminator core actually has

Core cannot ask a node type whether it gates — Store/Plugin, PluginContent and AddPluginGating live in MeshWeaver.Plugins and compile live on the mesh, which is exactly why the grant is addressed as a well-known PATH. It does not have to ask.

EnsureDeclaredAccess is the ONE access-establishment step of an install, and it already states, per manifest, whether it published the partition or left it gated. DeclaredAccessMarker names the node it wrote and is null exactly when it wrote nothing — the commercial branch (any non-zero price, or a contact-sales address):

Manifest shape What core wrote Is a cover grant owed?
pre-installed {partition}/_Policy · PublicRead = true no — readable through the policy
free, no publicSegments the same fully-public policy no
free with publicSegments the root Public Viewer grant — which IS the cover grant no — waiting on core's own write, already verified by VerifyDeclaredAccess
commercial nothing, on purpose yes — the gating pass's grant is the only observable proof anything can be read, including the Subscribe cover that would sell it

PackageInstaller.CoverGrantExpected is that and nothing more, restricted to the partition the manifest actually speaks for (a root the package wrote into but does not target is one core has made no access statement about, so it stays silent rather than inventing a verdict).

What changed

  1. Do not wait for what is not owed. CoverGrantExpected gates the watch, so the case that can never succeed is not watched at all.
  2. GatingSettleTimeout (30 s) → GatingDetectorBudget (5 s), renamed for what it now is: a deadlock-detector budget, not a settle barrier. Justified by measurement, not taste — when a grant IS owed it is observable in 44–55 ms over repeated runs (GatingDetectorTest.Detector_sees_a_cover_grant_that_lands measures the real write→observe latency on a live mesh), and phase 1 has already paid the hub activation, so the only outstanding work is one access-table write.
  3. The two outcomes stop looking identical. The expiry is now a WARNING naming the missing path and the consequence — the partition denies every viewer, including on its own Subscribe cover. "Nothing owed" is Debug. Warning and not Error deliberately: the expiry proves the grant is not there yet, not that it never will be, and VerifyDeclaredAccess already owns the Error-level verdict for access core itself promised to write.
  4. Still never fatal — the caller discards the outcome, exactly as before (the call site's own comment already said this step "cannot change what is installed or who can read it").
  5. The query is untouched. Still path:{grant} exact — empty-on-absent, never a GetMeshNodeStream point read (Write lane degraded on memex-cloud: create chain dies on a NotFound stream, a finished Store/Provision storms compile-state, plus a permanent-silent import order bug and a 2/s identity-cache loop #2229 item A, where the routing NotFound opened MeshNodeStreamCache's storm-breaker and suppressed the very write the wait was waiting for) and never a scope:children listing that can page past a grant that exists.

The trade is named in the code: a gating pass slower than 5 s is now reported rather than waited out.

The detector is proven to fire

GatingDetectorTest (new, 6 cases) pins all three outcomes — nothing owed (0.27 s), the grant lands (44–55 ms), the grant never comes (bounded and loud, asserting the level, the exact missing path and the named consequence). It deliberately mirrors the budget rather than importing the production constant, so it cannot pass at 30 s the way the old code did.

Mutation-proven — each of these turns it red:

Mutation Result
LogWarning → LogInformation (the old, indistinguishable line) ❌ "Expected collection to contain a single item because a wedged gating pass is reported exactly once, loudly, but found 0"
CoverGrantExpected → always false (detector never engages) ❌ 3 tests: "Expected value to be Stalled …, but found NotExpected"
budget back to 30 s ❌ "Expected 00:00:30.007 to be less than 00:00:15 because the detector budget is 5 s"

Verification

dotnet test test/MeshWeaver.PluginCatalog.Test -c Release -warnaserror, each run alone on the same machine:

before after
result 701 passed, 0 failed 707 passed, 0 failed
wall clock 5 m 36 s 2 m 49 s
sum of per-test durations 336.6 s 168.8 s

Per test:

Test before after
SelfTypedRoot_ReinstallImmediately_WritesNothing 60.28 s 0.53 s
StaleStampSelfTypedRoot_RootServesItsTypesArea 30.33 s 0.38 s
StaleStampSelfTypedRootShippingContent_RootServesItsTypesArea 30.29 s 0.29 s
RootTypedByAnInPackageNodeType_Installs 30.25 s 1.86 s
DeferredNodeTypeReleases_AreRequestedAfterTheRootRecycle 30.24 s 2.22 s
RealPluginsCheckout_ListsTheStore (control — unrelated) 25.17 s 26.02 s

Also green: test/MeshWeaver.Documentation.Test (129/129, including the guard that rejected an early .ToTask() in the new test — it is fully reactive now) and test/Memex.Portal.Shared.Test (490/490; one run showed the known 31 s contention flake in PluginBundlePlanTest.PromotingTheInstance_WidensTheNextRequest, which passes in isolation in 1 s and passed on two subsequent full runs of that suite).

Work products committed

Per the conserve-work-products rule: Doc/Architecture/InstallReadability (the two doors an install can open, the detector and its budget, the "never block on a node you documented as optional" rule, and the measured arithmetic), linked from Doc/Architecture and Doc/Architecture/Plugins, plus a What's New entry.

🤖 Generated with Claude Code

…as optional

`PackageInstaller` waited up to 30 s for a partition's cover grant while the
wait's own doc comment called that grant optional — "a partition whose node
type does not gate never writes it". On the shape the code itself called
*normal* the query can never emit, so every install burned the entire budget
and then reported success, leaving one Information line worded to cover both
outcomes. Measured in `MeshWeaver.PluginCatalog.Test`: 30.25 s / 30.29 s /
30.33 s / 30.24 s per install and 60.28 s for the test that installs twice —
181 s of a 336 s suite. `PackageInstaller.Install` is the production install
path, so a live mesh paid the same 30 s per non-gating package.

Core cannot ask a node type whether it gates (`Store/Plugin`, `PluginContent`
and `AddPluginGating` live in MeshWeaver.Plugins), which is why the grant is a
well-known PATH. It does not have to: `EnsureDeclaredAccess` is the ONE
access-establishment step of an install and already states, per manifest,
whether it published the partition or left it gated. `DeclaredAccessMarker` is
null exactly when it wrote nothing — the commercial branch — and that is the
only shape a gating pass owes a cover grant. `CoverGrantExpected` is that, and
the watch now runs only there.

- `GatingSettleTimeout` (30 s) → `GatingDetectorBudget` (5 s), renamed for what
  it now is. Justified by measurement: when a grant IS owed it is observable in
  44–55 ms (`GatingDetectorTest` measures the real write→observe latency on a
  live mesh), and phase 1 has already paid the activation, so the only
  outstanding work is one access-table write.
- The two outcomes stop looking identical: the expiry is a WARNING naming the
  missing path and the consequence (the partition denies every viewer,
  including on the Subscribe cover that would sell it); "nothing owed" is Debug.
  Warning, not Error — the expiry proves the grant is not there yet, not that
  it never will be, and `VerifyDeclaredAccess` owns the Error verdict for
  access core itself promised to write.
- Still never fatal, and the query is unchanged: `path:{grant}` exact,
  empty-on-absent, never a stream read (#2229) and never a children listing.

`GatingDetectorTest` pins all three outcomes and is mutation-proven: downgrading
the warning, disabling `CoverGrantExpected`, or restoring the 30 s budget each
turns it red.

Suite: 707/707 green, 5 m 36 s → 2 m 49 s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 30, 2026 22:17

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.

Pull request overview

This PR fixes a performance/diagnostic defect in the plugin install path where PackageInstaller always waited up to 30s for a “cover grant” it also documented as optional, by only running a bounded “deadlock detector” when a cover grant is actually owed (commercial packages) and making the “stalled” outcome explicitly visible in logs.

Changes:

  • Introduces CoverGrantExpected and replaces the unconditional 30s wait with a 5s bounded detector (DetectGatingStall) that runs only for installs that deliberately leave the target partition gated.
  • Refines install warming to filter detector fan-out early and distinguishes outcomes (not expected / landed / stalled) with clearer logging.
  • Adds a dedicated GatingDetectorTest suite plus new/updated documentation (including a What’s New entry and an architecture page) explaining the install readability model.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/MeshWeaver.PluginCatalog.Test/InstallGatingHandshakeTest.cs Updates the handshake contract commentary to reflect “watching” only for deliberately gated partitions.
test/MeshWeaver.PluginCatalog.Test/GatingDetectorTest.cs Adds live-mesh tests pinning the detector’s three outcomes and the budget expectations.
src/MeshWeaver.PluginCatalog/PackageInstaller.cs Implements CoverGrantExpected, DetectGatingStall, renames/adjusts the budget, and wires the detector into root warming.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-31-installs-no-longer-wait-30-seconds-for-a-node-that-is-optional.md Adds a What’s New entry describing the behavioral and performance change.
src/MeshWeaver.Documentation/Data/Architecture/Plugins.md Documents why the cover grant is addressed by path and links to the new install-readability model.
src/MeshWeaver.Documentation/Data/Architecture/InstallReadability.md New architecture page describing the two “readability doors” and the detector/budget rationale.
src/MeshWeaver.Documentation/Data/Architecture.md Adds “Install Readability” to the architecture index under plugins/content delivery.

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

Comment on lines +622 to +626
.Catch<GatingOutcome, Exception>(_ =>
{
logger?.LogInformation(
"[PackageInstaller] root {Root} has no cover grant after {Seconds}s. Either its "
+ "node type does not gate (normal — a course copy or a plain partition), or the "
+ "gating pass has not settled and the partition will DENY viewers until it "
+ "does. The install itself is complete either way.",
root, GatingSettleTimeout.TotalSeconds);
return Observable.Return(Unit.Default);
logger?.LogWarning(
"[PackageInstaller] {Id} installed into {Root} GATED — it is commercial, so the "
+ "installer published no access shape on purpose — but its cover grant {Grant} "
Comment on lines 1533 to 1534
var workspace = hub.GetWorkspace();
var accessService = hub.ServiceProvider.GetService<AccessService>();
Comment on lines +207 to +212
private IObservable<PackageInstaller.GatingOutcome> Detect(
PackageManifest manifest, string root, ILogger logger) =>
Mesh.ServiceProvider.GetService<AccessService>()
.RunAsSystem(() => PackageInstaller.DetectGatingStall(Mesh, manifest, root, logger))
.FirstAsync()
.Timeout(TimeSpan.FromSeconds(60));
Comment on lines +272 to +273
public IReadOnlyList<(LogLevel Level, string Message)> Records => records;

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

792 tests  ±0   788 ✅ ±0   4m 34s ⏱️ +9s
  7 suites ±0     4 💤 ±0 
  7 files   ±0     0 ❌ ±0 

Results for commit 503d726. ± Comparison against base commit 26690fc.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    8 files  ± 0      8 suites  ±0   4m 13s ⏱️ - 1m 54s
1 481 tests  - 52  1 480 ✅  - 52  1 💤 ±0  0 ❌ ±0 
2 174 runs   - 52  2 173 ✅  - 52  1 💤 ±0  0 ❌ ±0 

Results for commit 503d726. ± Comparison against base commit 26690fc.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 145 tests  +60   1 145 ✅ +60   4m 34s ⏱️ - 1m 5s
    7 suites ± 0       0 💤 ± 0 
    7 files   ± 0       0 ❌ ± 0 

Results for commit 503d726. ± Comparison against base commit 26690fc.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

951 tests  ±0   951 ✅ ±0   4m 33s ⏱️ -38s
  7 suites ±0     0 💤 ±0 
  7 files   ±0     0 ❌ ±0 

Results for commit 503d726. ± Comparison against base commit 26690fc.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

3 025 tests  ±0   2 833 ✅ ±0   6m 5s ⏱️ +21s
    8 suites ±0     192 💤 ±0 
    8 files   ±0       0 ❌ ±0 

Results for commit 503d726. ± Comparison against base commit 26690fc.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

1 181 tests  ±0   1 181 ✅ ±0   8m 8s ⏱️ +20s
    7 suites ±0       0 💤 ±0 
    7 files   ±0       0 ❌ ±0 

Results for commit 503d726. ± Comparison against base commit 26690fc.

@rbuergi
rbuergi enabled auto-merge August 30, 2026 22:31
@github-actions

Copy link
Copy Markdown
Contributor

Test Results

   44 files  ±0     44 suites  ±0   32m 10s ⏱️ - 2m 46s
8 575 tests +8  8 378 ✅ +8  197 💤 ±0  0 ❌ ±0 
9 268 runs  +8  9 071 ✅ +8  197 💤 ±0  0 ❌ ±0 

Results for commit 503d726. ± Comparison against base commit 26690fc.

@rbuergi
rbuergi added this pull request to the merge queue Aug 30, 2026
@rbuergi
rbuergi removed this pull request from the merge queue due to a manual request Aug 30, 2026
@rbuergi
rbuergi added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit 2885da3 Aug 31, 2026
26 checks passed
rbuergi added a commit that referenced this pull request Aug 31, 2026
rbuergi added a commit that referenced this pull request Aug 31, 2026
…tifiers

CI reddened my own guard within the hour, and for a reason that had nothing to
do with what it guards:

    Set:       ["GatingDetectorBudget", "DeclaredAccessVerifyBudget", ...]
    Not found: "GatingSettleTimeout"

#2849 renamed GatingSettleTimeout -> GatingDetectorBudget while this branch was
being written, and the self-test listed three bound names verbatim. So a guard
about SILENCE ON EXPIRY failed because a symbol moved.

That is the brittleness I flagged to myself while writing it and introduced
anyway. A guard naming a symbol inherits every rename of that symbol as a false
failure — and a guard that cries wolf gets deleted, taking the real check with
it.

The self-test's job is only to prove the scanner is not vacuous. It now asserts:

  * a COUNT (>= 6 bounded waits found) — a regex matching nothing would make the
    real assertion pass on an empty set;
  * a SHAPE — each matched bound ends in Timeout / Budget / Bound, so the scanner
    is provably matching budgets rather than incidental tokens, and this survives
    any rename.

No hard-coded identifiers remain.

Also merges main, which is where the rename came from.

Re-proved after the change: restoring the silent swallow still fails the primary
assertion, naming file, line and bound. Documentation.Test 134/134;
MeshWeaver.PluginCatalog and MeshWeaver.Documentation.Test both
-c Release -warnaserror at 0 Warning(s) 0 Error(s).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi
rbuergi deleted the fix/gating-settle-stall branch September 13, 2026 10:31
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