fix(#2819): a test wait must dominate the framework's own write bound by rbuergi · Pull Request #2829 · Systemorph/MeshWeaver

rbuergi · 2026-08-30T19:17:06Z

FileSystemObservableQueryTests.ObserveQuery_ScopeExact_OnlyNotifiesExactPath ejected #2814 from the merge queue. The issue's Shape section guessed at the file-watcher or the exact-scope filter. Neither: line 301 is the UpdateNode write, not the observation, and the quoted error carries no because clause — which WaitForChanges always supplies. The write is what timed out.

The defect is relational, and total

A mesh write's outer bound:

constant	value
`LateResponseWatchBound`	30 s — the registry stops honouring a late verdict
`VerdictBoundGrace`	1 s — deliberate, so the caller's failure cannot race an admissible verdict
caller's terminal	31 s → `OwnerUnreachable`

The grace is the tell: someone thought about ordering those two and wrote a comment explaining it.

The test convention was a hand-written 30.Seconds() — the same number as LateResponseWatchBound by coincidence, one second below the terminal. So a test awaiting a write gives up before the framework can answer. Not sometimes — always. Every such failure reads

Expected the observable to emit a value within 30s … emitted nothing at all.

one second before the framework would have said OwnerUnreachable — the owner produced no terminal for this patch. The bound sat at precisely the value that destroys the most information.

Nothing here was careless: 30 s is a reasonable convergence guess, the framework's own two bounds are correctly ordered, and each number is defensible alone. The defect exists only in the pairing, and neither author could see the pair — which is why review cannot catch it.

Why it survived its own fix

#2708 (for #2700) introduced TestTimeouts and scaled CI to 90 s, incidentally clearing 31 s. So CI started reporting the cause and the laptop never did — a partial fix that repairs the loud environment and leaves the quiet one, which removes the pressure to look again.

🚨 And TestTimeouts had zero adopters. The only references were its own test. #2708 landed the mechanism; nothing consumed it, so all ~2,679 literals are still literals and #2700 is not fixed. This PR is its first real adopter.

The change

LatePatchResponseRegistry exposes VerdictBoundGrace and WriteVerdictBound; MeshNodeStreamExtensions consumes them rather than keeping a private copy the test tree could not see. A bound nobody can see gets re-authored as a literal somewhere else — that is the entire mechanism.
TestTimeouts derives LocalConvergence from WriteVerdictBound + 5 s. Additive, not a ratio: what must be covered is one terminal propagating to the assertion, and that cost does not scale with the bound — a multiplier buys the same few seconds while inflating every wedged test's failure time.
FileSystemObservableQueryTests drops its 27 literals.
Architecture/BoundsMustBeOrdered records the rule.

The guard, and its mutation proof

Asserted at every factor including local 1.0 — a guard that only runs at CI scale passes on a codebase where the laptop still reports nothing, which is exactly the green state #2708 left.

Restoring the old literal:

Failed … AConvergenceWaitDominatesTheFrameworkWriteBound(factor: null)
Failed … AConvergenceWaitDominatesTheFrameworkWriteBound(factor: "1")
   a convergence wait (30s) must exceed the framework's write verdict bound (31s) …
   Failed: 2, Passed: 12

Red on local and "1", green on "3" and "10". That asymmetry is the evidence — it shows the guard measures the ordering, not the size of the number, and it independently confirms the CI-only-repair diagnosis above.

Verification

MeshWeaver.Persistence.Test 1106/1106 green (44 s) — the whole project, not a filter: this changes a shared bound, and a filter excludes the population that would prove it.
Release -warnaserror clean: Mesh.Contract, Fixture, Graph.Test, Persistence.Test, Documentation.Test.

Fixes #2819

A mesh write's outer bound is LateResponseWatchBound (30s) + VerdictBoundGrace (1s) = 31s: the instant UpdateRemote gives up and reports OwnerUnreachable. The grace is deliberate — someone ordered those two bounds and wrote a comment saying why. The test convention was a hand-written `30.Seconds()`, the same number as LateResponseWatchBound by coincidence, one second BELOW the terminal. So a test awaiting a write gave up before the framework could answer — not sometimes, always. Every such failure read "the observable emitted nothing at all" when the framework was one second from naming OwnerUnreachable. The bound sat at exactly the value that destroys the most information. The defect is relational: neither number is wrong alone, and neither author could see the pair. #2708 built TestTimeouts and scaled CI to 90s, which incidentally cleared 31s — so CI started reporting and the laptop never did. - LatePatchResponseRegistry exposes VerdictBoundGrace and WriteVerdictBound; MeshNodeStreamExtensions consumes them instead of keeping a private copy that the test tree could not see. - TestTimeouts DERIVES LocalConvergence from WriteVerdictBound + 5s. Additive, not a ratio: what must be covered is one terminal propagating, and that cost does not scale with the bound. - TestTimeoutsTest asserts the ordering at every factor INCLUDING local 1.0 — a guard that only runs at CI scale passes on a codebase where the laptop still reports nothing. Mutation-proven: restoring the literal reds exactly the local and "1" cases and leaves "3"/"10" green, which is the asymmetry that shows the guard measures the ordering rather than the size of the number. - FileSystemObservableQueryTests (the test that flaked) drops its 27 literals. It is TestTimeouts' first real adopter: nothing consumed the type #2708 added. Doc: Architecture/BoundsMustBeOrdered. Verified: MeshWeaver.Persistence.Test 1106/1106 green, Release -warnaserror clean on Mesh.Contract, Fixture, Graph.Test, Persistence.Test, Documentation. Fixes #2819 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FhP7Jg65q8cAATnFfeaTK

Copilot

Pull request overview

This PR fixes a systemic test-timeout ordering issue where local convergence waits could expire before the framework’s own mesh-write verdict bound, suppressing the framework’s diagnostic (OwnerUnreachable) and replacing it with an uninformative “emitted nothing at all” failure. It does this by surfacing the framework write bound in one place and deriving test convergence defaults from it, plus a guard and a documentation rule to prevent regressions.

Changes:

Expose the framework write-verdict bound (WriteVerdictBound) and its grace component on LatePatchResponseRegistry, and wire MeshNodeStreamExtensions to source the grace value from there.
Derive TestTimeouts.Convergence from the framework’s write-verdict bound (additive slack) and adopt it in FileSystemObservableQueryTests to remove hand-written 30s literals.
Add a guard test asserting that test convergence waits strictly dominate the framework write-verdict bound at all scaling factors, and document the “bounds must be ordered and derived” rule.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/MeshWeaver.Persistence.Test/FileSystemObservableQueryTests.cs	Replaces hand-written `30.Seconds()` convergence waits with `TestTimeouts.Convergence` (and updates helper to default to it).
test/MeshWeaver.Graph.Test/TestTimeoutsTest.cs	Adds a guard asserting `TestTimeouts.Convergence` is strictly greater than the framework `WriteVerdictBound` across factors.
test/MeshWeaver.Fixture/TestTimeouts.cs	Derives the local convergence baseline from `LatePatchResponseRegistry.WriteVerdictBound + 5s` instead of using a literal.
src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs	Sources `VerdictBoundGrace` from `LatePatchResponseRegistry` to avoid a private restatement.
src/MeshWeaver.Mesh.Contract/LatePatchWriteWatch.cs	Publishes `VerdictBoundGrace` and `WriteVerdictBound` so tests can derive waits from the real framework bound.
src/MeshWeaver.Documentation/Data/Architecture/BoundsMustBeOrdered.md	Documents the ordering/derivation rule and the concrete failure mode that motivated it.

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

github-actions · 2026-08-30T19:26:41Z