fix(lease): wire the poison cooldown and the stand-down notice to their callers - #950
Merged
Merged
Conversation
|
Warning Review limit reachedNext included review available in 58 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ened CI takes no further turn `lease::cooling` landed pure, documented at length and unit-tested, with nothing calling it. Its first parameter is `poisoned_at: Option<&str>` and no code path in the tree ever produced a `Some`, so every real call would have passed `None` and returned `false` on the first line. The behaviour its own doc comment promises — "a holder whose matrix goes red releases and may win the very next acquire, replay onto the same trunk and buy another matrix on the same defect" — described `main`, not the state before some earlier fix. This wires the two ends it was missing. THE WRITER. `land::poisons_this_clone` is the cell, pure and free-standing for the reason `progress_of` is: a decision reachable only through a git resolution and a filesystem write is a decision nothing tests. It is `(Wait, Violation, Red, Own)` — the wait's own refusal, never a stale red in `seen`; `Red` and not `Pending` or `None`, because a could-not-look is not a poisoning; and `Own` and not `Borrowed`, because on a borrowed base the red may be the speculated base's fault and charging it here would let a neighbour's bad tree evict an innocent agent. `note_the_cooldown` records the trunk position. THE READER. `still_cooling` consults `cooling` in `run_lease_acquire`, after `turn` has said whose turn it is and before the compare-and-swap. The order keeps the two separable: folding it into `turn` would put a local file inside a decision every other clone must be able to reproduce from the ref alone. THE RECORD NAMES ITS OWN REF. It stores `<tracking-ref>\t<sha>` so the reader resolves what the writer used. The first spelling wrote a bare sha, which made the reader assume `origin/main`: on any other base it would compare against an unrelated ref, never match, and answer "not cooling" — a fail-open arm inside a mechanism that exists to fail closed, which is the shape CLOUD-1792 was filed over one module away. Naming the ref costs one field and removes the assumption. A record missing its tab is malformed rather than read loosely, because reading it loosely would resurrect exactly that assumption. An unreadable record reads as UNPOISONED, the opposite of `Local::holder`'s rule and for the opposite reason: a defaulted holder id lets two clones claim one lease, while a cooldown defaulting to ON strands a clone over a file it cannot parse. The anchor is a trunk POSITION throughout, never an instant. The cooldown lapses on two observable events — the trunk moving, or the pool going idle — so no wall clock enters a mechanism that would obviously have been written as a timer (CLOUD-1784's decomposition). `//MUTANT poison-never-recorded` inverts the cell. Measured: the declared case dies under it and the two record twins stay green, so the cases discriminate the decision rather than the plumbing around it. Closes CLOUD-1797
…can be asked `lease::notice` mints a body carrying a stand-down request. The whole receiving half landed wired — `stand_down_requested`, `Beat::StandDown`, `honour_the_notice`, latched across laps — and `notice` itself had one caller in the tree, its own unit test. So `stand_down` was empty on every lease in the fleet and the receiving half was unreachable in production. That is what made `Body::writer` unable to earn its place. The field exists so a waiter can SEE that the holder runs an older build; seeing it and having no way to act left exactly one remedy — wait out the TTL — which is the outcome the field was added to avoid. THE SENDER is one arm of `run_lease_acquire`'s `Turn::Wait`, and it changes nothing else about the wait: the waiter still answers `Violation`, still does not take the lease, still does not retry. Asking is a REQUEST the holder may decline, which is what makes it the eviction path that stays safe under a false suspicion — `turn`'s steal arms are the unsafe ones and they are untouched. THE DECISION is `lease::worth_asking`, pure, so the three guards are decidable without a ref, a CAS or a wire. Each is a distinct way to get this wrong: - STRICTLY NEWER (`lease::outranks`). An equal build is not out-ranked — two clones on one version asking each other to stand down is a request loop with no newer party to win it. A newer holder must never be asked: that inversion lets an old clone evict the fleet, which is what `Body::writer`'s own doc refuses when it says the field decides nothing by itself. - IDEMPOTENT. A body already carrying a request is left alone. Without it every lap of every waiter rewrites the ref and the holder's heartbeat CAS fails against a stream of cosmetic updates — eviction by contention rather than by asking, the steal this path exists to avoid. A blank `stand-down:` is an absence, not a request, because a rendered body round-trips to that spelling. - NEVER SELF-ADDRESSED, the mirror of `stand_down_requested`'s guard one step earlier: that one stops a holder honouring a notice it minted onto itself, this one stops it being written. `outranks` answers false on every uncertain reading — an unparseable version on either side, and a fourth segment, which would otherwise rank two distinct builds equal. A body written before `writer` existed carries an empty one, and that is the shape that actually occurs on upgrade. A lost CAS is silent: the holder beat, released, or another waiter asked first, and all three make the request moot or already-made. `//MUTANT notice-ignores-the-writer` drops the version comparison. Measured: the declared case dies, the unparseable-writer case dies with it, and the idempotence twin stays green — so the clauses are pinned independently rather than by one case that happens to cover them all. Closes CLOUD-1798
…on row The census holds a module that DECLARES rows to the enforced set: rows nobody sweeps are the coverage-shaped nothing the verb exists to refuse. Adding `//MUTANT poison-never-recorded` to `land.rs` therefore made it a subject, and a subject absent from `$MUTANT_GATES` reads `uncovered`. Caught by `mutate::this_repositorys_own_census_is_closed`, which is the case that makes every census fixture evidence about this repository rather than about a toy tree. Worth recording because the near-miss was mine: `engine-landed` (for `landed.rs`) was already in the set, a hand check for the substring `engine-land` matched inside it, and the set read as already correct. The name is a whole element, not a prefix. Refs: CLOUD-1797
…to decide `perf-gate` refused #950 at `passthrough` 1.707x. The branch touches no budgeted path. `perf pair --null` — which measures the identical binary as both arms, so every ratio it produces is 1.0 plus pure noise — says why. `REGRESSION_RATIO`'s comment records the original null (2026-08-11, 100 runs after 10 warmups, spread 0.966 to 1.102) and closes with "Re-measure with `perf pair --null`". Re-measured on a busier container at those same defaults: check 4.99 -> 9.37 1.88x noop 7.87 -> 3.55 0.45x posttool 16.26 -> 18.27 1.12x passthrough 3.90 -> 4.21 1.08x wired 106.19 -> 111.40 1.05x hook 7.82 -> 7.80 1.00x A control at 1.88x is refused by a 1.30x threshold, and 0.45x is the same noise with the sign reversed — a binary measured 2.2x faster than itself. Three real pair runs on one unchanged branch each named a different path as the regression, and the `passthrough` base arm alone read 4.43, 7.91 and 8.69 ms. At 300 runs after 30 warmups, same container, same binary: 0.95 to 1.145 — back inside the band the original experiment produced, and clear of the threshold. SO THE CONSTANT MOVES AND THE THRESHOLD DOES NOT. This module's differencing premise — both arms on one machine within seconds, so the noise divides out — holds under steady load and fails when a preemption burst lands inside one arm's window and not the other's. A larger sample makes the burst a smaller fraction of each window. It is the remedy that adds no machinery: interleaving the arms, re-measuring on a failure, and self-calibrating the threshold per run were each considered and are each a way of working around a sample that simply needs to be bigger. Raising `REGRESSION_RATIO` instead was rejected outright. It would have to clear 1.88x, which is above the 1.462x real regression CLOUD-875 measured and caught — trading a false positive for a false negative on the gate whose job is catching the second. `DEFAULT_RUNS` and `DEFAULT_WARMUP` were bare constants carrying no measurement of their own, unlike the threshold below them, so this overturns no recorded experiment. It supplies the one that was never taken, and records both nulls where the threshold records its own. The cost is three times the runs, paid only by a commit touching crate source, a manifest or the lockfile — the gate skips clean otherwise. The case is a FLOOR, not an equality: `== "300"` would pass a later edit back to a sample this comparison is measured to fail at, so long as whoever made it moved the case too. `//MUTANT sample-too-small-to-decide` drops it back to 100 and the case dies. Closes CLOUD-1803
wenzowski
force-pushed
the
claude/batten-pr-934-takeover-mtdtnq
branch
from
September 12, 2026 04:22
0424f0e to
d27f4ba
Compare
wenzowski
marked this pull request as ready for review
September 12, 2026 04:22
Contributor
Author
|
/fast-forward |
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.
Two mechanisms that landed with no caller, wired. Both were found auditing
#934 against what it claimed to build, and both were sitting in a PR body and
in chat rather than on the board — which is why they are rows now.
lease::coolinghad no writer (CLOUD-1797)The poison cooldown landed pure, documented at length and unit-tested, with
nothing calling it. Its first parameter is
poisoned_at: Option<&str>and nocode path in the tree produced a
Some, so every real call passedNoneandreturned
falseon the first line. Its own doc comment describes the defect inthe present tense — "a holder whose matrix goes red releases and may win the
very next acquire … spending the metered resource twice to learn what it
already knows" — and that described
main.land::poisons_this_clone:(Wait, Violation, Red, Own).Pure and free-standing for the reason
progress_ofis — a decision reachableonly through a git resolution and a filesystem write is a decision nothing
tests.
Ownrather thanBorrowedis what stops a neighbour's poisoned baseevicting an innocent agent.
note_the_cooldown, recording the trunk position.still_cooling, consulted inrun_lease_acquireafterturnsays whose turn it is and before the CAS. Folding it intoturnwouldput a local file inside a decision every other clone must be able to
reproduce from the ref alone.
The record names its own ref. The first spelling wrote a bare sha, which
made the reader assume
origin/main: on any other base it would compareagainst an unrelated ref, never match, and answer "not cooling" — a fail-open
arm inside a mechanism that exists to fail closed, which is the shape
CLOUD-1792 was filed over one module away, last night. It now stores
<tracking-ref>\t<sha>.No clock enters any of it. The cooldown lapses on two observable events — the
trunk moving, or the pool going idle (CLOUD-1784's decomposition).
lease::noticehad no sender (CLOUD-1798)The whole receiving half was wired —
stand_down_requested,Beat::StandDown,honour_the_notice, latched across laps — andnoticeitself had exactly onecaller in the tree: its own unit test.
stand_downwas empty on every lease inthe fleet, so
Body::writercould show a waiter that the holder was behind andleave it no remedy but the TTL, which is the outcome that field was added to
avoid.
The sender is one arm of
run_lease_acquire'sTurn::Waitand changes nothingelse about the wait: still
Violation, still no take, still no retry. Askingis a request the holder may decline, which is what makes it the eviction path
that stays safe under a false suspicion —
turn's steal arms are the unsafeones and are untouched.
lease::worth_askingis the decision, pure, three clauses each of which is adistinct way to get this wrong: strictly newer (an equal build is a request
loop with no newer party to win it; a newer one inverts the design and lets an
old clone evict the fleet), idempotent (or every lap of every waiter rewrites
the ref and the holder's heartbeat CAS fails against cosmetic updates —
eviction by contention), and never self-addressed.
outranksanswers false on every uncertain reading, including the emptywritera body written before the field carries — the shape that actuallyoccurs on upgrade.
Evidence
Both mutations reverted and measured, not asserted:
poison-never-recordednotice-ignores-the-writerFull suite green (
mise run test, exit 0).mutate censusgreen: 140 gates,every one enforced or exempt by a filed row.
The third commit is why: declaring a
//MUTANTrow inland.rsmakes themodule a census subject, and a subject absent from
$MUTANT_GATESreadsuncovered. A hand check forengine-landmatched inside the existingengine-landedand read as already correct — the name is a whole element, nota prefix.
mutate::this_repositorys_own_census_is_closedcaught it.Also filed, not built
CLOUD-1799 —
batten landed checkexists and nothing calls it. It is theboth-directions board sweep CLOUD-1127 shipped;
mise.tomldeclares no taskthat runs it,
hk.pklregisters no step, and run bare it asks for--merged-prsevidence no caller supplies. It is the detector that should havecaught #948 over-advancing CLOUD-1426, which was instead found by hand three
hours later. That row has been moved back to Backlog, its wrong PR attachment
removed, and its comment records both the mechanics and fresh evidence for its
own open question.
Closes CLOUD-1797
Closes CLOUD-1798
https://claude.ai/code/session_014zmrMLGEsPxiTyFRq28uXX