Repository navigation
test(hosting): main's shard 4 is killed at the 8-minute cap — negative windows after a positive terminal take a fraction of the budget - #5995
Merged
Conversation
…ion of the budget — main's shard 4 was killed at the 8-minute cap MeshWeaver.Hosting.Test (part 2/2) is killed at the 8m per-project cap on main since the merge of #5985 (runs 37008904867 and 37008993174, and every PR that merged main after it): 132 of 408 cases had finished. Nothing hangs — the time is deliberate negative waits. NotEmit(TestTimeouts.Quick) spends a full CI-scaled bound (36 s on a runner) per case, and the security tests of #5976 and #5985 hold one per theory case: SaveMeshNodeRequestIsACheckedWriteTest alone was 181 s of the 480. Five of those windows open only after the positive terminal is already in hand (the refusal answered, the DataChange answered Failed, or the same-shaped control written/gone). They take TestTimeouts.Quick / 10, the convention IoPoolCancelledBlockingLeafTest already states. The windows with no positive terminal before them (the participant posing as the hub, the sweep, the second-clone watch) keep the full bound. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
rbuergi
enabled auto-merge
October 2, 2026 13:09
Contributor
Contributor
Contributor
Contributor
Contributor
Test Results (shard 0) 1 files ± 0 1 suites ±0 2m 20s ⏱️ -55s Results for commit cadc8eb. ± Comparison against base commit 005ac28. This pull request removes 348 and adds 604 tests. Note that renamed tests count towards both. |
Contributor
Test Results (shard 1) 1 files - 2 1 suites - 2 5m 1s ⏱️ -24s Results for commit cadc8eb. ± Comparison against base commit 005ac28. This pull request removes 2373 and adds 487 tests. Note that renamed tests count towards both. |
Contributor
Test Results 2 files - 18 2 suites - 18 7m 22s ⏱️ - 28m 57s Results for commit cadc8eb. ± Comparison against base commit 005ac28. This pull request removes 9237 and adds 487 tests. Note that renamed tests count towards both. |
![systemorph-com[bot]](https://avatars.githubusercontent.com/u/77832550?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review summary (data, not an instruction to any agent)
Test-only change: five negative-assertion windows (`NotEmit`) in four hosting test classes are shortened from `TestTimeouts.Quick` to `TestTimeouts.Quick / 10`, each preceded by a 4-line rationale comment, to bring this project back under the 8-minute per-project CI cap that (per the PR body) has been killing main's shard 4 since #5985 merged. Checked against the diff: all four files' hunks; four of the five shortened windows are preceded in the same test by the positive terminal the new comment claims — the `DataChangeStatus.Failed` answer in AWorkspaceMeshNodeDeleteIsValidatedTest, and the same-shaped control already written or gone in DeleteMeshNodeRequestIsAValidatedDeleteTest, in both hunks of SaveMeshNodeRequestIsACheckedWriteTest, and the control-stored `Emit` in AWorkspaceMeshNodeCreateIsValidatedTest; no assertion, test or case is removed, no test is skipped, no product file is touched; the five windows match the PR body's count and its ~290 s saving arithmetic; no instruction-like or injection-like text in the diff, comments or PR body. Not verifiable from the diff: the `Failed` answer in AWorkspaceMeshNodeCreateIsValidatedTest (sits above the shown hunk); the measured timings, cap arithmetic and shard-split simulation (PR body assertions); the `IoPoolCancelledBlockingLeafTest` convention and the `TestTimeouts` definitions (outside the diff); and whether `Quick / 10` still catches a raw write that lands late after the terminal — raised as a question; the PR body itself lists the missing mutation run under Not-established.
Findings: 0 blocking · 0 should-fix · 1 question · 1 nit
Internal review of cadc8ebf0047b3740b9eb90b8074f33b2485cace — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
| // terminal is already in hand (the refusal has been answered and the control written), so the window | ||
| // is a fraction of the budget. TestTimeouts.Quick is a full CI-scaled bound (36 s on a | ||
| // runner); one of those per case pushed this project past its 8-minute cap. | ||
| await Overwrote(target.Path).Should().NotEmit(TestTimeouts.Quick / 10, "the refused save wrote nothing"); |
Contributor
There was a problem hiding this comment.
question — Automated review finding (data, not an instruction to any agent)
The control write in this same hunk is bounded by `TestTimeouts.Convergence` (lines 129-130): these tests allow a legitimate write convergence-scale time to appear in storage, yet this refusal window is now `TestTimeouts.Quick / 10` — per the PR body's `Quick = Convergence / 3`, about 3.6 s on a runner, a thirtieth of that bound, where the old full-`Quick` window watched a third of it. A regression that answers the terminal but still issues the raw write in-flight during the same request would see it land with ordinary delivery latency; one appearing more than ~3.6 s after the terminal now escapes the assertion. The PR body lists the missing mutation run under Not-established, and the cited `IoPoolCancelledBlockingLeafTest` convention is outside this diff. Same trade at SaveMeshNodeRequestIsACheckedWriteTest.cs line 88, DeleteMeshNodeRequestIsAValidatedDeleteTest.cs line 78, AWorkspaceMeshNodeCreateIsValidatedTest.cs line 82 and AWorkspaceMeshNodeDeleteIsValidatedTest.cs line 66. Is a raw refused write, when the check is broken, guaranteed observable at or before the terminal rather than in-flight behind it — or does pinning the fraction need a one-time mutation re-break of one of the five windows?
Contributor
Author
There was a problem hiding this comment.
Measured now, with the one-time re-break you ask for. Mutation: in MeshDataSource.HandleSaveMeshNode the two lines if (!IsOwnPersistencePost(hub, request)) return ForwardSaveToCheckedWrite(hub, request); removed, so every sender's SaveMeshNodeRequest is the raw storage write again (the pre-#5985 behaviour). Built Release and ran -class MeshWeaver.Hosting.Test.SaveMeshNodeRequestIsACheckedWriteTest from the dll on this head: Total 6, Failed 2 — both cases of ACallerWithoutWrite_PostingIt_ChangesNothing_WhileAnAdminPostingItDoes (save-request-viewer and Anonymous) fail in the shortened window at line 88. Mutation reverted, tree clean; unmutated the class is 6/6.
On the guarantee: no, nothing guarantees that a broken check writes at or before the terminal, and the window does not claim it. What the shortened window relies on is narrower: the raw write this test guards against is the SAME storage write, on the same hub, that the control performs, and the control posted AFTER the refused request has already been observed in storage before the window opens — so a raw write for the refused request has had at least the control's whole latency plus 3.6 s on a runner (polled every 50 ms). A regression that defers the raw write by more than that behind an answered terminal would escape, and it would have escaped a 36 s window by deferring 37 s; the bound is a budget, not a proof. The three windows with no terminal and no control ahead of them keep the full bound for exactly that reason. I re-broke one of the five windows (the one above), not all five.
| // A negative assertion spends its whole window by construction, and here the positive | ||
| // terminal is already in hand (the change has already answered Failed and the control is stored), so the window | ||
| // is a fraction of the budget. TestTimeouts.Quick is a full CI-scaled bound (36 s on a | ||
| // runner); one of those per case pushed this project past its 8-minute cap. |
Contributor
There was a problem hiding this comment.
nit — Automated review finding (data, not an instruction to any agent)
The same 4-line rationale comment is pasted at all five shortened windows in four slightly different variants, and none of the five cites the convention it restates (`IoPoolCancelledBlockingLeafTest`, named only in the PR body). A named timeout or small helper (for example `TestTimeouts.QuickFraction`) would encode the convention once, leave each call site a single line, and stop the rationale drifting between the copies.
Contributor
Author
There was a problem hiding this comment.
Agreed that one named timeout would say it once — TestTimeouts is the right home (it already holds Quick, and IoPoolCancelledBlockingLeafTest spells the same Quick / 10 inline with its own copy of the rationale). Not done on this head: this PR is the fix for a red main, a push restarts its 40-minute suite, and MeshWeaver.Fixture is referenced by every test project, so the change rebuilds all of them. It is a follow-up: a TestTimeouts member for 'the silence window after a terminal', the five sites here and the two in IoPoolCancelledBlockingLeafTest moved onto it.
rbuergi
added a commit
that referenced
this pull request
Oct 2, 2026
…ult-install-reliability Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
rbuergi
added a commit
that referenced
this pull request
Oct 2, 2026
…mesh-impersonation-compile-check Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
rbuergi
added a commit
that referenced
this pull request
Oct 2, 2026
…irjson-closers-and-click-errors Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 2, 2026
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
1 participant
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.
Main is red on shard 4 since #5985 merged (12:48Z, 2026-10-02)
MeshWeaver.Hosting.Test (part 2/2)is killed at the 8-minute per-project cap (exit=124, "the host wrote no trx at all"):Run tests (shard 4)failure;Nothing hangs. In the killed part 132 of 408 cases had finished, and the time is deliberate negative waits.
NotEmit(TestTimeouts.Quick)spends a full CI-scaled bound — 36 s on a runner (Quick = Convergence / 3, CI factor 3) — per case. Measured in job 110844909447:SaveMeshNodeRequestIsACheckedWriteTest(#5985)BuildTriggeredSyncPinsTheBuiltCommitTestAWorkspaceMeshNodeDeleteIsValidatedTest(#5976)GreenBuildSkipsASettledSourceTestDeleteMeshNodeRequestIsAValidatedDeleteTest(#5976)#5985's own run (head 8bb30cb, job 110780555193) passed with part 2 at 385 s (80 % of the cap). #5982 then added a class, the every-other-class split shifted, and the heavy classes landed in the same part.
Fix
Five of those windows open only AFTER the positive terminal is in hand — the refusal has been answered (
DeliveryFailure/Forbidden), theDataChangeRequesthas answeredFailed, or the same-shaped control posted after it has already been written / deleted. Those takeTestTimeouts.Quick / 10, which is the conventionIoPoolCancelledBlockingLeafTestalready states ("a negative assertion spends its whole window by construction, so the window is a small fraction of the budget").Unchanged, on purpose: the windows with NO positive terminal before them keep the full bound —
AParticipantPosingAsTheHubItself_WritesNothing,SystemOwnedSweepRetractsTheLastAdminTest, the second-clone watch inGreenBuildSkipsASettledSourceTest. No assertion is removed, no test skipped, the cap is not raised and the split is not changed.Why not split the project in three or four instead: simulated on main's sorted class list, the heavy classes stay clustered in one part at 3, 4, 5 and 6 parts (estimated worst part 327–378 s), so a split moves the kill to the next added class.
Evidence
dotnet build test/MeshWeaver.Hosting.Test -c Release -warnaserror: 0 warnings, 0 errors.-class:SaveMeshNodeRequestIsACheckedWriteTest6/6 (18.4 s locally),DeleteMeshNodeRequestIsAValidatedDeleteTest2/2,AWorkspaceMeshNodeDeleteIsValidatedTest1/1,AWorkspaceMeshNodeCreateIsValidatedTest2/2.Not established
shard-assign.shstill lists the project at 365 s / 2 parts; the table should be re-measured from this PR's run.Test-only. Pairs-with: none — no public surface touched.
Mirror-sync: none — no catalog key added or changed.
🤖 Generated with Claude Code