fix(layout): an area whose store is unreachable says so; the fold's anchoring stays rejected - #3031
Merged
Merged
Conversation
…eaking the driver's error Fixes #2876, fixes #2640. #2876: the retry was never missing. MeshQuery.MergeProviderObservables already wraps every provider observable in TransientStorageFaults.RetryTransientConnect (#2521, merged three days before the 08-31 capture) — 250/500/1000 ms, then the last error surfaces. A database unreachable for 21 s outlives 1.75 s of budget, so the fault reached the render exactly as designed. What was missing is what the area SHOWS when that budget is honestly spent: the generic panel carried the driver's own text and the database host the pod could not reach to an end user, under a log line naming the AREA as the thing that failed. A fifth area frame (AreaFrameClassifier.StorageUnavailableId) now says the content is temporarily unavailable and worth reloading, localized en + de. Deliberately: no retry on the render path (the fan-in's is spent; a second one would be an unbounded resubscribe aimed at the resource that is already the bottleneck), no log downgrade (an availability failure stays at Error where an operator sees it, #974 — only the wording changes, to name the store rather than the area), and NOT part of IsTransientFrame (nothing fires when a database comes back, so a waiter told this was transient would wait forever). The classification moves to StorageFaults.IsTransientConnectFault in MeshWeaver.Data.Contract — the one assembly both the query fan-in and MeshWeaver.Layout can see — typed on the BCL DbException surface since the driver lives in the plugins repo. TransientStorageFaults forwards to it; a corpus test asserts the two surfaces cannot drift. #2640: the anchoring its body proposes was considered and REJECTED. A GroupMembership lives under the GROUP node, which may sit in a different partition than the grant that names it, so pinning the fold's reads to the viewer's partition IS truncation: a group-derived permission vanishes in one direction and a group-scoped DENY fails open in the other, with nothing logged and nothing failing. Paging is out for the same reason. The reasoning, the per-shape census with a reason each, the per-lever verdicts (including that ObserveSharedTargets and the GatedNodes collapse need decisions, not patches, and that QueryRoutingHints is inert on Postgres so several in-repo docstrings describe their query as pinned on the strength of a rule that does not run) are conserved as Doc/Architecture/UnanchoredSecurityReads. SecurityQueries.AllShapes documented itself as existing "for the completeness test that pins them" — and no source file in this repo or any satellite referenced it. SecurityQueryShapesTest is that test: it pins the completeness stamp (#2011/#2048) AND parses every shape with the real QueryParser to assert the unanchored population is exactly the declared one, with a positive control on the anchored per-scope legs so it cannot pass vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SD7aiLSo59xng2TzU32xEa
…liminated The fan-out elimination page's plan 2 is the security fold, and its own "read this first" was missing: anchoring those reads is truncation, which makes a group-derived permission vanish AND a group-scoped deny fail open, with nothing logged and nothing failing. A reader arriving at plan 2 now hits that warning before the plan, rather than after implementing it. Also names #2876 in the Related line — a transient connect timeout took a whole area render down inside GetSchemasWithTableAsync, the call that enumerates the schemas a fan-out is about to UNION, so it is this defect seen from the render side rather than a separate one. Fixes #2640, fixes #2876. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SD7aiLSo59xng2TzU32xEa
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, uses a single shared classifier to prevent drift, and is backed by targeted tests and documentation that pin both the new UI behavior and the security-query shape contract.
Pull request overview
Improves layout-area failure handling so transient storage-connectivity outages render a dedicated, localized “storage unavailable” frame (instead of leaking raw driver errors to viewers), and adds an executable ratchet that pins the intended “global vs anchored” shapes of security-fold queries while explicitly rejecting the previously-proposed anchoring approach for global security reads.
Changes:
- Add a shared
StorageFaults.IsTransientConnectFaultclassifier (inMeshWeaver.Data.Contract) and route both the query-fan-in retry and layout render-failure UI classification through it. - Introduce a new area-frame state (
AreaFrameClassifier.StorageUnavailableId) + localized viewer copy, and adjustLayoutAreaHostlogging to name the store outage rather than blaming the view. - Add tests guarding both the new render behavior and the security-query shape census/anchoring ratchet; conserve the rejected-anchoring reasoning + triage findings in docs and a What’s New entry.
File summaries
| File | Description |
|---|---|
| test/MeshWeaver.Layout.Test/StorageUnavailableRenderTest.cs | New end-to-end test proving storage-connect faults render the named frame, don’t leak driver text, and still log at Error. |
| test/MeshWeaver.Layout.Test/AreaFrameClassifierTest.cs | Adds coverage for the new StorageUnavailableId frame state and ensures it’s not treated as transient. |
| test/MeshWeaver.Hosting.Test/SecurityQueryShapesTest.cs | New ratchet test that pins security query completeness and the declared “deliberately global” (unanchored) population with positive controls. |
| test/MeshWeaver.Hosting.Test/MeshQueryTransientRetryTest.cs | Ensures retry-layer and render-layer classify the same transient-connect corpus via the shared rule. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.en.json | Adds error.storageUnavailable* localization keys (English). |
| src/MeshWeaver.Messaging.Hub/Localization/strings.de.json | Adds error.storageUnavailable* localization keys (German). |
| src/MeshWeaver.Layout/Composition/LayoutAreaHost.cs | Uses new storage-unavailable classification to adjust logging + render a dedicated frame with a well-known id. |
| src/MeshWeaver.Layout/AreaFrameClassifier.cs | Introduces StorageUnavailableId and IsStorageUnavailable classifier on rendered frames. |
| src/MeshWeaver.Layout/AreaErrorClassifier.cs | Adds IsStorageUnavailable(Exception?), delegating to shared StorageFaults. |
| src/MeshWeaver.Hosting/Persistence/Query/TransientStorageFaults.cs | Removes duplicated classification logic; forwards IsTransientConnectFault to StorageFaults. |
| src/MeshWeaver.Data.Contract/StorageFaults.cs | New shared, driver-agnostic transient-connect fault classifier (typed on DbException). |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-02-a-view-whose-database-was-unreachable-now-says-so.md | What’s New entry describing the user-visible improvement. |
| src/MeshWeaver.Documentation/Data/Architecture/UnanchoredSecurityReads.md | New architecture page conserving the rejected anchoring reasoning + per-lever verdicts for #2640. |
| src/MeshWeaver.Documentation/Data/Architecture/CrossSchemaFanOutElimination.md | Links to the new “Unanchored Security Reads” companion page to prevent repeating the rejected approach. |
| src/MeshWeaver.Documentation/Data/Architecture.md | Adds the new security architecture page to the index. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This was referenced Sep 2, 2026
Closed
rbuergi
added a commit
that referenced
this pull request
Sep 2, 2026
…ot Unknown (#3057) * fix(mesh): a create whose store is unreachable answers Unavailable, not Unknown A transient database connect timeout reaching HandleCreateNodeRequest's terminal error arm fell into the catch-all and was reported twice over as something it was not: to the operator as "Unexpected error during node creation" (naming the create, which was fine, rather than the store), and to the caller as NodeCreationRejectionReason.Unknown -- indistinguishable from a verdict. That distinction has teeth in one direction. A create that reads as REFUSED tells a caller to stop; a caller that stops using the id it was retrying and mints a fresh one on its next attempt writes a DUPLICATE. That is #2229's shape arriving through the reporting layer instead of through a stale query. Both ends of the wire already existed. NodeCreationRejectionReason.Unavailable was added for a different unreachable dependency (#1446) and means exactly this -- "the create was NOT evaluated ... an availability failure, not a verdict". And StorageFaults.IsTransientConnectFault is the ONE classification rule, extracted by #3031 into the assembly every consumer can see. This adds the branch between them. Deliberately NOT a retry. The bounded retry already ran upstream (TransientStorageFaults.RetryTransientConnect, #2521: 250 -> 500 -> 1000 ms, then the last error surfaces), so a fault reaching this arm is one whose budget is honestly spent; retrying here would aim a second, unbounded-in-aggregate retry at the resource that is already the bottleneck. Log level stays at Error -- only the wording changes, to name the store. The branch is judged by the CONDITION and so sits ABOVE the `ex is InvalidOperationException` test: an IOE wrapping a driver connect fault is still the store being unreachable, and answering it ValidationFailed would tell the caller their request was invalid because a database was down. Same argument CancellationClassifier makes about the timeout impostor. Both create verbs carry it -- a guard on one create verb and not the other is a guard on neither, and the bulk verb is what every installer and static-repo import travels. On the bulk side it sits BELOW the partial-landing branch: "nothing was written" is false once a batch has committed a window. Test: CreateWhenTheStoreIsUnreachableTest drives the real create pipeline through a storage adapter that faults on marked paths only. On the pre-fix code 3 of its 5 cases fail and 2 pass -- the falsifying case (42P01 must still answer Unknown) and the positive control (a healthy path still lands) pass before AND after, so the change is attributable rather than merely correlated. Docs conserve two findings from the same night: - StoreUnreachableIsNotARefusal -- the one rule and its three consumers, the retry-once-then-answer ladder, the falsification boundary, the known edge left open (the in-process exception mapping still collapses Unavailable and Unknown), and the measurement showing 89.4% of that pod's Npgsql exceptions landed within 100 ms of a GC-stall report, in windows covering 1.77% of the log -- so the incidents' "the database was unreachable" reading is not supported. - ReadingASiloEviction -- why a heartbeat newer than the suspect votes does not prove a silo was healthy, and the probe-target control arm that separates a correct eviction from a false positive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SD7aiLSo59xng2TzU32xEa * fix(#3051): do not claim "nothing was written" when the write had started From the automatic review on #3057, and it is a correctness bug rather than wording. The bulk create answered DescribeNotAttempted unconditionally on a store-unreachable fault — but attemptedPaths is set once the batch is stamped and handed to the store, so a fault after that point may have landed part of it. "Nothing was written" is a claim ABOUT THE DATA. Said after a partial landing it tells the caller the store is in a state it is not, and the caller's natural response — retry the whole batch — then double-writes whatever did land. That is the #2229 shape reached from the opposite direction. So the verdict is now picked on evidence: DescribeMayHavePartiallyLanded when the write had started, DescribeNotAttempted when it provably had not. The log line asserted the same thing unconditionally and now reports which case it is. Both sentences keep the Unavailable/not-refused distinction the PR is about; the new one adds "read the current state before retrying" because re-sending is only safe in the not-attempted case. Full MeshWeaver.Graph.Test: 1163/1163. Release -warnaserror clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SD7aiLSo59xng2TzU32xEa --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Roland Bürgi <rbuergi@icloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2876, fixes #2640.
🚨 Read this first: the anchoring #2640 proposes was considered and REJECTED
#2640's body proposes, as its first candidate:
The
accessbucket is the biggest of the four (465 of 1 030 measured fan-outs, 45%), so it is the natural place to start. It is the one that must not be done that way, and the reasoning is the most valuable thing in this PR — it is now conserved as a doc page rather than as a comment on an issue thread, because the next person will otherwise reach for it.SecurityQueries— "the ONE place the permission-deciding mesh queries are written" — says why several of its reads carry nopath:and nonamespace::Anchoring the membership read to the viewer's partition IS truncation. A membership record living in the group's partition is not in the viewer's, so it is not returned. Two failures follow, pointing in opposite directions, and nothing goes red either way:
Denied = trueassignment is applied only to the viewers the membership read says are in the group — so a revocation FAILS OPEN and the viewer keeps reading content the deny was written to take awayPaging is off the table for the same reason:
SecurityQueries.Enumerationdeliberately overwrites alimit:rather than honouring it, because in this fold a page IS the bug. And the trigger is growth, not a change — it fires the moment a mesh'sRole/GroupMembershipset outgrows whatever bound was introduced, so it appears on the largest install first and nobody will have touched anything.What this PR actually does
1. #2876 — an area whose store was unreachable now says so
The retry was not missing.
MeshQuery.MergeProviderObservablesalready wraps every provider observable inTransientStorageFaults.RetryTransientConnect(#2521, merged 2026-08-28 — three days before the 08-31 capture on #2876): 250 → 500 → 1000 ms, then the last error surfaces. A database unreachable for 21 s outlives 1.75 s of budget, so the fault reached the render exactly as designed.What was missing is the answer to "what does the area SHOW when the bounded retry is honestly spent". Before:
…i.e. the driver's own text plus the database host the pod could not reach, rendered to an end user, under a log line (
Rendering failed for area Catalog) that names the area as the thing that failed — sending every reader hunting for a bug in a view that was fine.After: a fifth area frame,
AreaFrameClassifier.StorageUnavailableId, carrying localized copy (en + de) that says the content is temporarily unavailable and worth reloading.Four deliberate non-choices, each stated in the code:
Errorwhere an operator sees it (the same argument Three more sites report an availability failure as a definitive negative (permission-fold fault → "Access denied", read timeout → "Not found", cold cache → "user unknown") #974 makes). What changes is the wording — it now names the store rather than the area.IsTransientFrame. That predicate promises "this WILL be replaced without anyone acting". Nothing fires when a database becomes reachable again, so a waiter told this frame was transient would wait forever — while a waiter told it was a verdict would give up on an area that is perfectly fine.StorageFaults.IsTransientConnectFaultinMeshWeaver.Data.Contract(the one assembly both the query fan-in andMeshWeaver.Layoutcan see), typed on the BCLDbExceptionsurface since the driver lives in the plugins repo.TransientStorageFaultsnow forwards to it. Two copies of that rule would drift silently in either direction — a fault the fan-in retries but the renderer reports as a defect, or an outage the renderer excuses that the fan-in never retried — soMeshQueryTransientRetryTestasserts both surfaces agree on a corpus including the boundaries (42P01and40P01are NOT the connect class; a bareTimeoutExceptionis a hub timeout with its own policy).2. #2640 — triage, the executable census, and the missing guard
SecurityQueries.AllShapesdocuments itself as "every query shape this class produces, for the completeness test that pins them". That test did not exist — no source file in this repo or in any satellite referencedAllShapesat all. A census nothing reads is a list, not a guard.SecurityQueryShapesTestis that test, and it pins two independent properties:limit:all,EnumerationREPLACES an existing limit rather than honouring it, and thecontent.limit:3boundary the regex is written for survives untouched.QueryParserand classified — does it pin a concrete partition, or does itUNION ALLevery schema? The unanchored population is declared with a reason each, so a new unanchored security read cannot be added silently, and a declared entry that has since been anchored must be struck rather than left as a stale justification. It carries a positive control (the anchored per-scope legs — the queries the fold issues most often — must come back pinned torbuergi/acme/Admin/Doc), so "everything is unanchored" cannot pass vacuously.No fan-out was eliminated in this PR, and that is deliberate. What the sweep found, and where it landed:
Enumerationoverwriteslimit:)notifications/threadsNotificationCenter{,Panel}.razor) and the thread list live in MeshWeaver.Plugins. Core'sNotificationServicesettings reads are already anchoredAdmin/Menu/{X}route misses83b1892be, an anchored existenceGetQuery, 50 minutes after #2640's measurement window closedUserActivityLayoutAreas.ObserveSharedTargetsaccessbucket. It cannot be anchored (a share grant lives in the GRANTING partition; pinning it to{user}/…makes everything shared with you disappear).GetQuerywould move it from source-side RLS to consumer-sidePermissionEvaluator— two implementationsAccessControl.mdsays must agree; a per-viewer cache needs anIIoPoolpromise slot and an invalidation contract (a bareConcurrentDictionary<key, IObservable<T>>latches a transientOnErrorforever, #1369). Both are designs, not patchesGatedNodes(type)intonodeType:A|B|CParsedQuery.ExtractNodeTypereturnsnullfor an alternation, and that value drives satellite-TABLE routing — collapsing silently changes which table a gated satellite type reads. Prerequisite: an alternation-awareExtractNodeTypeswhose consumers route only when every value agreesQueryRoutingHintsMeshConfiguration.ResolveRoutingHintsregisters rules pinningnodeType:Role/Partition/GlobalSettingstoAdmin— andInvitationNodeTypestates in-repo that "the PostgreSQL query router routes purely by the path's first segment and does NOT consume these QueryRoutingHints yet, so this rule is currently inert". Several docstrings in this repo describe their query as pinned on the strength of a rule that does not run. Honouring the hints would remove real fan-outs — and would also silently truncate anyRoleauthored outsideAdmin, which is the exact failure this PR is about3. Conserved
Doc/Architecture/UnanchoredSecurityReads— the rejected-anchoring reasoning, the per-shape census with reasons, the per-lever verdicts above, theQueryRoutingHints-is-inert finding, and why Every page render fires unanchored cross-schema queries (access, notifications, threads, mesh_nodes) — ~2 s each over 186 partition schemas #2640 and Catalog area render fails entirely on transient PostgreSQL connection timeout in PostgreSqlCrossSchemaQueryProvider.GetSchemasWithTableAsync #2876 are one story (theCatalogrender died insideGetSchemasWithTableAsync, the call that enumerates the schemas a fan-out is about to UNION — an anchored query never makes it). Linked from the Architecture index and fromCrossSchemaFanOutElimination's sibling row.Category: Fix) for the user-visible half.🚨 If you want #2640 to stay open until the fan-outs are actually eliminated, reopen it. It is closed here on the strength of the triage + the ratchet + the conserved reasoning, not on an elimination; the elimination plan itself lives on
Doc/Architecture/CrossSchemaFanOutEliminationand is provider-side (MeshWeaver.Plugins) for the two biggest buckets.Verification
Full projects, no
--filter, all built first (0 Error(s),0 Warning(s)) with-c Release -warnaserrorper touched project:MeshWeaver.Layout.TestMeshWeaver.Hosting.TestMeshWeaver.Documentation.TestMeshWeaver.Messaging.Hub.TestLocalizationTest)Mutation-verified. Disabling the new arm in
CreateRenderErrorControlfailsStorageUnavailableRenderTest.AStoreThatCouldNotBeReached_ServesTheNamedFrame_AndStillPagesTheOperator— the test would have failed onmain. Restored and the full project re-run green.scripts/check-type-forwards.py --base origin/main→ "OK — no unguarded public-type move".Follow-up in another repo
The two new localization keys (
error.storageUnavailable,error.storageUnavailableHint) have a second home inMeshWeaver.Plugins/clients/react/src/i18n/. Core is the source of truth and merges first; the mirror PR follows.🤖 Generated with Claude Code
https://claude.ai/code/session_01SD7aiLSo59xng2TzU32xEa