Repository navigation
fix(#3404): an UNDETERMINED witness is not a witness that said "no GO" - #3457
Merged
Merged
Conversation
#3410 gave the pre-warmer a second door — read the durable build root when the SubscribeRequest to Admin/Build cannot be answered. That door had TWO branches, while ReadBuildGo folds THREE outcomes into its null: no GO for this fingerprint, no durable store, and THE READ FAILED. So a read that never completed was acted on, and logged, as a definitive negative ("the durable witness carries no GO for framework X"). That is not a corner case: the second door is NOT in a different failure domain from the first. AddPartitionStorageHubs replaces IStorageAdapter with RoutingProxyAdapter, whose Read is hub.Observe<ReadNodeResponse>(new ReadNodeRequest(path, options), WithTarget(addr)) — the same hub transport under the same 60 s budget as the SubscribeRequest. Of the five candidates on record for #3404's silence, the routing loss, the deferred-queue ordering defect (#3408) and the non-emitting root take BOTH doors down together, and the durable read then fails with the same TimeoutException. - ReadBuildGoReading returns a three-state BuildGoReading (Go / NoGo / Undetermined) that always carries a Detail and, on a fault, its Error. ReadBuildGo keeps its fold and its signature — it is still right for a caller whose negative branch is BAKE into a content-addressed store — and its contract now says a caller that REFUSES on the negative must use the three-state read. - Go grants as before. NoGo — the witness ANSWERED — still refuses, fail-closed, and now says the witness answered. Undetermined no longer guesses in either direction: it measures on the one witness the broken transport cannot touch, this pod's own IAssemblyStore, and grants only when NodeTypeBakeReport.GateRelevant is empty. - That grant is STRICTER than the GO branch, not laxer: a still-pending type after a GO is non-gating (PreWarmStatus.TimedOut), so a half-baked share passes WITH a GO and is refused WITHOUT one. "Finding nothing is not passing" is preserved. No timeout widened, no retry, no watchdog, no poller, no catch{}. The transport fault is still logged in full on every branch — it remains the only signal that the pod-to-Admin/Build path is broken, and this changes the readiness VERDICT and the REPORTING, never the visibility of the fault. Verification (falsify both ways, tests unchanged): - with the fix: 9/9 pass (4 new + 5 in PreWarmerReadsTheDurableGoTest); 16/16 including BuildCoordinationRetryTest. - behavioural revert (Undetermined folded back into NoGo): 2 failed / 7 passed — the grant and the reporting arms, exactly the two the change is about. - full src/ revert: does not compile (BuildGoWitness, ReadBuildGoReading absent). Docs: Doc/Architecture/UndeterminedIsNotNo (the rule and the argument), BuildCoordination updated, What's New entry (Fix). Closes #3404 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ate disarmed All three logged occurrences in #3404 carry the pre-warmer's NOT-ARMED branch ("Nothing gates on this state here") and NONE carries "REFUSING READINESS", so on those pods the bake gate was off and no rollout was held. That matches the 2026-09-06 measurement of memex-cloud: ConfigMap, values file and rendered manifest all say PreWarm__GateReadiness=true while the pods run false from an inline env: entry set on the Deployment at 2026-09-03T17:32:45Z — an inline env: overrides envFrom, and the effective value is only readable with printenv inside the pod. This is why the third state has to be modelled BEFORE the gate is armed: arming a gate whose "could not determine" branch reads a witness inside the failing transport's own domain turns every blip into a held rollout, reported as a build nobody approved. Read-only finding; no cluster was touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The tri-state durable witness modeling, pre-warmer decision logic, tests, and documentation updates are coherent and correctly preserve fail-closed behavior for real negatives while preventing misclassification/reporting of unreadable reads.
Pull request overview
This PR refines the build-coordination “second door” logic in MeshWeaver.Hosting so that an unreadable durable GO witness is treated as a distinct Undetermined state (not as “no GO”), preventing readiness refusals (and misleading logs) when the durable read fails in the same failure domain as the unreachable subscription.
Changes:
- Introduces a tri-state durable witness reading (
BuildGoReading/BuildGoWitness) and a newReadBuildGoReadingAPI while preservingReadBuildGo’s existing folded/null contract for bake-on-null callers. - Updates
BuildProtocolDriver.WhenTheSubscriptionDoorIsShutto branch onGo/NoGo/Undetermined, and onUndetermineddecide readiness by probingIAssemblyStore(strict grant rule based onNodeTypeBakeReport.GateRelevant). - Adds targeted tests that stage the “durable read fails like the transport did” shape and pin both the decision and the reporting behavior; adds/updates documentation and a What’s New entry.
File summaries
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Test/UndeterminedWitnessIsNotNoGoTest.cs | New coverage for the “failed read ⇒ Undetermined” classification, probe-based grant/refuse behavior, and non-misleading logging. |
| test/MeshWeaver.Hosting.Test/PreWarmerReadsTheDurableGoTest.cs | Adds the complementary “readable real NoGo still refuses even if share is baked” arm and generalizes the driver helper. |
| src/MeshWeaver.Hosting/BuildProtocolDriver.cs | Switches the second-door behavior to tri-state and adds the probe-based decision path for Undetermined. |
| src/MeshWeaver.Graph/BuildCoordinationExtensions.cs | Preserves ReadBuildGo signature while adding ReadBuildGoReading to expose the third state and diagnostic detail. |
| src/MeshWeaver.Graph.Contract/BuildState.cs | Adds BuildGoWitness and BuildGoReading types to represent the tri-state durable witness outcome. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-06-a-check-that-could-not-run-no-longer-reports-a-failure.md | What’s New entry describing the corrected readiness/reporting behavior. |
| src/MeshWeaver.Documentation/Data/Architecture/UndeterminedIsNotNo.md | New architecture page documenting the “undetermined is not no” rule and the decision pattern. |
| src/MeshWeaver.Documentation/Data/Architecture/BuildCoordination.md | Updates build coordination docs to describe the shared failure domain and the tri-state read/decision rule. |
| src/MeshWeaver.Documentation/Data/Architecture.md | Adds the new architecture page to the reliability & wedges index. |
Review details
- Files reviewed: 9/9 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 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.
The half #3410 left open
#3404's shape is "a failed read rendered as a definitive negative". #3410 fixed that one level up
— the pre-warmer got a second door, so an unanswerable
SubscribeRequesttoAdmin/Buildno longerrefuses a build the durable root already approved. The same defect was still live inside that
door.
ReadBuildGodeliberately folds three outcomes into onenull:The new door had two branches. So an unread witness was acted on as a negative and narrated as
one:
"the durable witness carries no GO for framework {X}"— about a read that had timed out.🚨 The decisive fact: the second door was in the FIRST door's failure domain
This turns a tidiness argument into a defect. In the fleet's portal wiring
AddPartitionStorageHubsreplacesIStorageAdapterwithRoutingProxyAdapter(
src/MeshWeaver.Hosting/Persistence/PartitionStorage/), whoseReadis:— a hub request on the same transport, under the same 60 s budget, as the
SubscribeRequestthat just went unanswered. Of the five candidates on record for #3404's silence,the routing loss, the deferred-queue ordering defect (#3408) and the non-emitting root take both
doors down together, and the durable read then fails with the same
TimeoutException.So on the very fault the second door exists for, the expected reading is Undetermined — and that
was exactly the reading being laundered into "no GO".
A recovery path that shares a failure domain with the thing it recovers from is not a recovery
path.
🚨 Read-only finding: on
memex-cloudthe hold is LATENT, and an inlineenv:is whyThe refusals in #3404 are real; the rollout hold in its title did not happen on 2026-09-06. The
pre-warmer's fault handler logs one of two lines, and which one it picks IS the evidence of whether
the gate was armed:
REFUSING READINESS — the warm-up stream FAULTED…(Critical)gate is { GatesReadiness: true, Phase: BakePhase.Faulted }…**Nothing gates on this state here**; the bake is recorded as unproven rather than complete.(Error)Counted in the issue body: 3 occurrences of the NOT-armed line, 0 of
REFUSING READINESS. Thosepods went Ready having verified nothing.
That matches the deployment as measured on 2026-09-06:
memex-cloud's ConfigMap, values file andrendered manifest all say
PreWarm__GateReadiness: "true", while the pods runfalsefrom aninline
env:entry set on the Deployment at 2026-09-03T17:32:45Z — an inlineenv:overridesenvFrom, and the effective value is readable only withprintenvinside the pod, never from theConfigMap, the values file or
helm get manifest. No cluster was touched by this PR; this is readoff the issue's own log text plus that recorded measurement.
The consequence is the point: arming
memex-cloud's gate is what is waiting on #3404, andarming a gate whose "could not determine" branch reads a witness inside the failing transport's own
domain would turn every transport blip into a held rollout, reported as a build nobody approved. The
third state has to be modelled before the gate is armed, not after the first stalled deploy.
The third state
BuildGoReading—Go/NoGo/Undetermined, always with aDetail, and with theErrorwhenthere was a fault — read through the new
ReadBuildGoReading.ReadBuildGokeeps its fold and its exact signature. It is still correct for a caller whosenegative branch is bake into a content-addressed store (
FollowGo,OnGranted): that costs oneredundant compile and can never corrupt. Its contract now states that a caller whose negative branch
refuses must use the three-state read instead — the fold is licensed by its callers, and #3410
added one the licence did not cover.
Fail-open vs fail-closed — the choice, and what decided it
Both are defensible, and both are guesses:
broken, and deletes the gate on exactly the days the cluster is unhealthy.
rollout on a transient.
Neither. Do not guess — MEASURE, on a witness in a different failure domain. The pod already
holds one: its
IAssemblyStore(a blob container in production, a mounted volume in monolith/dev/test) — never a hub message.
NodeTypeBakeStatus.Probeasks it, per NodeType, whether bytesexist for the live framework identity.
GoNoGoUndeterminedNodeTypeBakeReport.GateRelevantis empty; refuse otherwise, saying the witness was UNREADABLE🚨 The grant is STRICTER than the branch it substitutes for, not laxer. A pod that receives a GO
grants and then reports still-pending types as
PreWarmStatus.TimedOut, whichIsGatingFailuredeliberately does not gate on — so a half-baked share passes WITH a GO. The undetermined branch
grants only when every previously-healthy NodeType is
Bakedfor this framework identity. A sharethat passes with a GO can be refused without one. That asymmetry is the point: a GO is somebody
else's certification, and this branch has none, so it may only grant on evidence it measured itself.
What I deliberately did NOT do
catch {}, noTask.Delay.CoordinationAttempts(3) and the hub's 60 s budget are untouched.logged at its own severity with its own diagnostic detail on every branch — it remains the only
signal the pod↔
Admin/Buildpath is broken. This changes the readiness verdict and thereporting, never the visibility of the fault.
NoGoverdict. Letting a pod's own volume overturn a coordination answerwould be a scope call nobody made.
Pairs-with:—ReadBuildGokeeps its signature and everything elseis additive (
git diff origin/main...HEAD -- src | grep '^-' | grep -E 'public |internal 'isempty).
Verification — falsified both ways, tests unchanged
PreWarmerReadsTheDurableGoTest); 16 / 16 includingBuildCoordinationRetryTestUndeterminedfolded back intoNoGo, tri-state type kept so it compilesAnUnreadableWitness_WithTheShareAlreadyBaked_GrantsReadiness(threw the refusal) andAnUnreadableWitness_IsNeverReportedAsAWitnessThatSaidNoGo(log said "carries no GO"). Exactly the two arms the change is aboutsrc/revertBuildGoWitnessandReadBuildGoReadingabsent (2 × CS)The repro is a real mesh (
MonolithMeshTestBase) with a real assembly store — the baked armsPutbytes and the probe finds them by the production key. Exactly one thing is injected: aTimeoutExceptionon the durable read ofAdmin/Build, verbatim in the incident's shape, wrappedaround the real adapter — the same fault-injection discipline
PreWarmerReadsTheDurableGoTestandBuildCoordinationRetryTestalready use. The decorator asserts it was actually served, so awiring change that shadowed it fails the tests instead of passing them silently.
The non-vacuity arm lives in
PreWarmerReadsTheDurableGoTest(
NoDurableGo_StillRefuses_EvenWhenTheShareIsFullyBaked), because it needs a readable witness: animplementation that simply granted whenever the share looked complete passes every case in the new
file and fails that one.
DocumentationLinkIntegrityTest: 1 / 1 pass — and proven non-vacuous against the new page by anegative control (a deliberately broken sibling link made it fail naming
Doc/Architecture/UndeterminedIsNotNo, then removed).Builds:
MeshWeaver.Graph.Contract,MeshWeaver.Graph,MeshWeaver.Hosting,MeshWeaver.Hosting.Test,MeshWeaver.Documentation.Test— each-c Release -warnaserror, one perinvocation,
0 Error(s).Docs
Doc/Architecture/UndeterminedIsNotNo— the rule, the failure-domain finding, thefail-open/fail-closed argument, and the cross-link to the write-side sibling
An Unreachable Store Is Not a Refusal.BuildCoordination(a new subsection on the shared failure domain and the three-statereading), the Architecture index, and a What's New entry (
Category: Fix).Closes #3404