Repository navigation
test(hostlock): the conformance suite must pass on a co-tenanted host - #1903
Merged
Merged
Conversation
Policy, recorded against #1729/#1805 and now #1802: CPU scheduling and performance policy must not assume exclusive access to the machine, and a policy that wins only under exclusive quiet-host conditions is not a valid default. Two cells in my own suite were exactly that defect in test form. R2b asserted that a single-core busy loop achieves >= 0.8 of a core. That is an exclusive-host assumption wearing a test's clothes: true on a quiet box, false on the shared one this repository actually runs on. Measured, with one competitor pinned to the same cpu as the run: co-tenanted efficiency 0.503 fixed threshold 0.8 rc=6 verdict=contended <- the cell would FAIL derived threshold 0.453 rc=0 verdict=ok idle control rc=6 (unchanged under load, as it must be) So the suite would have gone red for precisely the condition it exists to detect in other people's runs, and the operator response to that -- re-run until green -- is how a real contention signal gets trained out of a team. The fix is to measure this host and derive the threshold from it, keeping the absolute assertions only where they are load-independent by construction: a sleeping command consumes no CPU whatever the neighbours are doing, and one core of work judged against two is half-efficient by arithmetic. The kernel-time cell now compares against the user-mode probe taken moments earlier on the same host rather than against 0.8, so co-tenancy scales both arms together while the ratio still collapses to ~0.23 if cstime stops being counted. Coverage is preserved, verified by mutation rather than assumed: CPU_TICKS=$((cu)) (kernel time dropped) 246/3 RED eff = constant 1.000 (measurement -> label) 244/5 RED R8.4's inertness check moves from < 3s to < 10s for the same reason -- it asserts a seam does nothing, not that the box is quiet -- and stays well under the 30s seam it is falsifying. Also states in the README that both efficiency knobs are opt-in and that `run` judges nothing by default: on a co-tenanted host, and on the edge devices this engine targets, a tool that failed by default would be asserting a dedicated machine nobody promised. Suite 247 -> 249. shellcheck clean. No crate code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
enabled auto-merge (squash)
August 23, 2026 22:50
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1903 +/- ##
==========================================
+ Coverage 80.19% 80.80% +0.60%
==========================================
Files 399 415 +16
Lines 185791 204440 +18649
Branches 185791 204440 +18649
==========================================
+ Hits 148995 165190 +16195
- Misses 31441 33674 +2233
- Partials 5355 5576 +221
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
justinchuby
added a commit
that referenced
this pull request
Aug 24, 2026
…ock-refuse-run-ttl One conflict: the pinned assertion count, 254 (this branch) vs 249 (#1903). Resolved to 256 by running the suite rather than by arithmetic; "every assertion in this file ran" passes, so neither side's assertions were lost. cleanup() did not conflict this time and retains the union from the #1885 merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 24, 2026
…at produces it Follow-up to #1903, fixing a defect its review found and I merged anyway because auto-merge fired on green CI while I was still writing the fix. #1903 replaced a fixed `--min-efficiency 0.8` with a threshold derived from a probe run on the same host, because 0.8 is an exclusive-host assumption and this suite has to pass co-tenanted (#1802). The correct fix, with a defect: a threshold derived from the measurement it judges is invariant to any CONSTANT scaling of that measurement. Review demonstrated it rather than suspecting it: eff = c / (w * n) * 0.5 halves every efficiency, every derived threshold and every ratio together, and all 249 assertions passed. The fixed 0.8 that #1903 removed is precisely what would have caught it. The two mutations I had run -- `eff` = constant 1.000, and dropping cstime -- both move the numbers RELATIVE to each other, which is the one class a self-referential threshold can still see, so my "coverage is preserved, verified by mutation" was a reassuring conclusion drawn from the wrong experiment. So pin the number to two anchors that do not come from that code path: * the child's own accounting of its own CPU -- os.times() inside the command, written to a file, compared against hostlock's cpu= field; * the arithmetic relating the three fields on the row it prints, efficiency ~= cpu / (wall * cores). Both hold under any load, because both compare the run against itself rather than against an expectation of the host, so neither reintroduces the quiet-host assumption #1903 removed. Mutation-verified: eff = c/(w*n) * 0.5 (constant scaling) 260/1 RED <- was 249/0 GREEN CPU_TICKS=$((cu)) (kernel time dropped) 258/3 RED (unchanged) Both re-measured after rebasing onto main at cdc7d93, because #1908 added nine assertions to this same file in the interim and a mutation count taken against the older total would be a number nobody measured. Two robustness fixes to #1903 in the same area, both observed rather than theorised. The derived-threshold slack goes 0.9 -> 0.5, and the kernel-time ratio 0.7 -> 0.5: the probe and the run it judges are separate 1.5s arms taken seconds apart, so a co-tenant arriving between them starves only the second, and at 0.9 that reddens the pair for a reason that has nothing to do with the gate under test. That is the same "goes red when somebody starts a build, so people re-run until green" trap #1903 exists to remove, and it was observed twice on this box: two assertions failing, then the identical file passing minutes later with nothing changed but the neighbours. 0.5 still leaves 2x headroom over the ~0.23 that dropping kernel time produces. And one cell that cannot be made vacuous by load at all: the verdict must follow the number printed on the same row. That holds at any starvation level and catches a gate that always says ok, always says contended, or has its comparison backwards. Suite 258 -> 261. shellcheck clean. No crate code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 24, 2026
…at produces it Follow-up to #1903, fixing a defect its review found and I merged anyway because auto-merge fired on green CI while I was still writing the fix. #1903 replaced a fixed `--min-efficiency 0.8` with a threshold derived from a probe run on the same host, because 0.8 is an exclusive-host assumption and this suite has to pass co-tenanted (#1802). The correct fix, with a defect: a threshold derived from the measurement it judges is invariant to any CONSTANT scaling of that measurement. Review demonstrated it rather than suspecting it: eff = c / (w * n) * 0.5 halves every efficiency, every derived threshold and every ratio together, and all 249 assertions passed. The fixed 0.8 that #1903 removed is precisely what would have caught it. The two mutations I had run -- `eff` = constant 1.000, and dropping cstime -- both move the numbers RELATIVE to each other, which is the one class a self-referential threshold can still see, so my "coverage is preserved, verified by mutation" was a reassuring conclusion drawn from the wrong experiment. So pin the number to two anchors that do not come from that code path: * the child's own accounting of its own CPU -- os.times() inside the command, written to a file, compared against hostlock's cpu= field; * the arithmetic relating the three fields on the row it prints, efficiency ~= cpu / (wall * cores). Both hold under any load, because both compare the run against itself rather than against an expectation of the host, so neither reintroduces the quiet-host assumption #1903 removed. Mutation-verified: eff = c/(w*n) * 0.5 (constant scaling) 267/1 RED <- was 249/0 GREEN CPU_TICKS=$((cu)) (kernel time dropped) 265/3 RED (unchanged) Both re-measured against the current base rather than carried over: #1908 and #1869 added sixteen assertions to this same file while this PR was open, and a mutation count taken against a superseded total is a number nobody measured. Two robustness fixes to #1903 in the same area, both observed rather than theorised. The derived-threshold slack goes 0.9 -> 0.5, and the kernel-time ratio 0.7 -> 0.5: the probe and the run it judges are separate 1.5s arms taken seconds apart, so a co-tenant arriving between them starves only the second, and at 0.9 that reddens the pair for a reason that has nothing to do with the gate under test. That is the same "goes red when somebody starts a build, so people re-run until green" trap #1903 exists to remove, and it was observed twice on this box: two assertions failing, then the identical file passing minutes later with nothing changed but the neighbours. 0.5 still leaves 2x headroom over the ~0.23 that dropping kernel time produces. And one cell that cannot be made vacuous by load at all: the verdict must follow the number printed on the same row. That holds at any starvation level and catches a gate that always says ok, always says contended, or has its comparison backwards. Suite 265 -> 268. shellcheck clean. No crate code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 24, 2026
…at produces it (#1910) ## What this fixes, and how it got merged unfixed #1903's review found a real defect and I merged the PR anyway — auto-merge fired the moment required CI went green, while I was still writing the fix. This is that fix, as its own PR. #1903 replaced a fixed `--min-efficiency 0.8` with a threshold **derived from a probe run on the same host**, because 0.8 is an exclusive-host assumption and this suite has to pass co-tenanted (#1802). That was the right correction, with a defect inside it: > **A threshold derived from the measurement it judges is invariant to any constant scaling of that measurement.** Review demonstrated that rather than suspecting it, by applying the mutation: ``` eff = c / (w * n) * 0.5 ``` Every efficiency, every derived threshold and every ratio halves **together**, and all 249 assertions passed. The fixed `0.8` that #1903 removed is exactly what would have caught it. My two mutations — `eff` = constant `1.000`, and dropping `cstime` — both move the numbers *relative to each other*, which is the one class a self-referential threshold can still see. So "coverage is preserved, verified by mutation" was a reassuring conclusion drawn from the wrong experiment. That is the seventh round in a row where the fix for the previous finding carried the next one, and the direction is always the reassuring one. ## The fix: anchor the number outside the code path that produces it Two anchors, neither of which can be reached by scaling `eff`: 1. **The child's own accounting of its own CPU.** `os.times()` *inside* the command, written to a file, compared against hostlock's `cpu=` field. 2. **The internal arithmetic of the row it prints.** `efficiency ≈ cpu / (wall × cores)`. Both hold under any load, because both compare the run against **itself** rather than against an expectation of the host — so neither reintroduces the quiet-host assumption #1903 removed. | mutation | before | now | |---|---|---| | `eff = c/(w*n) * 0.5` — constant scaling | 249 / **0 GREEN** ❌ | 267 / **1 RED** ✅ | | `CPU_TICKS=$((cu))` — kernel time dropped | 246 / 3 RED | 265 / **3 RED** | Both re-measured against the current base rather than carried over: #1908 and #1869 added sixteen assertions to this same file while this PR was open, and a mutation count taken against a superseded total is a number nobody measured. ## Two robustness fixes in the same area, both observed rather than theorised **Derived-threshold slack 0.9 → 0.5, kernel-time ratio 0.7 → 0.5.** The probe and the run it judges are separate 1.5 s arms taken seconds apart, so a co-tenant arriving in between starves only the second one. At 0.9 that reddens the pair for a reason that has nothing to do with the gate under test — the same *"goes red when somebody starts a build, so people re-run until green"* trap #1903 exists to remove. Observed twice on this box: two assertions failing, then the identical file passing minutes later with nothing changed but the neighbours. 0.5 still leaves **2× headroom** over the ~0.23 that dropping kernel time produces. **One cell that load cannot make vacuous:** the verdict must follow the number printed on the same row. That holds at *any* starvation level, and catches a gate that always says `ok`, always says `contended`, or has its comparison backwards. ## Validation - Suite **265 → 268**, green on current `main`, and green under `taskset -c 30,31` — two cores, the shape of a GitHub runner. - The new `Host lock conformance` CI job (#1912) runs the suite on this PR: **pass in 3m17s** on a GitHub-hosted runner, the first time the suite has ever run outside the box it locks. - Both decisive mutations red, restored after each. - `shellcheck` clean. No crate code, no runtime behaviour change — `scripts/hostlock_test.sh` only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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.
The policy, applied to my own work first
That correction is being applied to #1729 and #1805 under #1802. Before reviewing anyone else's code against it, I checked mine, and two cells in my own conformance suite were the same defect in test form.
Measured, not argued
R2b asserted that a single-core busy loop achieves ≥ 0.8 of a core. I re-ran those exact cells with one competitor pinned to the same cpu as the run:
0.8rc=6 verdict=contended— the cell fails0.453rc=0 verdict=oksleep 1)rc=6, unchanged under loadSo on the shared box this repository actually runs on, my suite would have gone red for precisely the condition it exists to detect in other people's runs. And the operator response to a suite that goes red under load is to re-run it until it is green — which is how a real contention signal gets trained out of a team. A quiet-host assumption in a test is not milder than one in production code; it is the same assumption, in the place that is supposed to catch it.
What changed
Measure this host, then judge against what it gave. The probe run is unjudged (
runprintsefficiency=with no threshold), the acceptance threshold is derived from it, and the cells that need an absolute number are only the ones that are load-independent by construction:rc=6;rc=6.The kernel-time cell now compares two measurements instead of a constant. It ratios the syscall-bound run against the user-mode probe taken moments earlier on the same host: co-tenancy scales both arms together, so the ratio survives a busy box, while dropping
cstimestill collapses it to ~0.23.R8.4's seam-inertness check moves 3s → 10s. It asserts that a seam does nothing; it should not also be asserting the box is quiet. Still far under the 30s seam it falsifies.
Coverage preserved — verified by mutation, not assumed
The obvious failure mode of "derive the threshold from the environment" is a tautology that passes against anything. It does not:
CPU_TICKS=$((cu))— kernel time droppedeff= constant1.000— measurement becomes a labelSuite 247 → 249, green.
shellcheckclean. No crate code.Documented, so the next person does not re-derive it
The README now states that both efficiency knobs are opt-in and that
runjudges nothing by default: on a co-tenanted host — and on the edge devices this engine targets — a tool that failed by default would be asserting a dedicated machine that nobody promised. A low efficiency is information about one measurement, never a claim that the host owes you every core.The suite header now carries the rule itself: every assertion in this file must hold on a co-tenanted host, with the 0.503 measurement as the evidence for why.
Correction, added after merge
The "Coverage preserved — verified by mutation" section above over-claims, and review had already shown why before this merged. Auto-merge fired the moment required CI went green, while I was still writing the fix, so this landed with the finding open.
A threshold derived from the measurement it judges is invariant to any constant scaling of that measurement. Review applied
eff = c / (w * n) * 0.5: every efficiency, every derived threshold and every ratio halve together, and all 249 assertions in this PR pass. The fixed0.8this PR removed is exactly what would have caught it. The two mutations I ran here both move the numbers relative to each other, which is the one class a self-referential threshold can still see.Fixed in #1910, which anchors the number to two things outside that code path — the child's own
os.times()self-report, and the internal arithmeticefficiency ≈ cpu / (wall × cores)— and takes the mutation from 249/0 GREEN to 251/1 RED. #1910 also widens the derived-threshold slack 0.9 → 0.5 and the kernel-time ratio 0.7 → 0.5, because those two arms are taken seconds apart and were observed going red on this box purely from a co-tenant arriving in between.