diff --git a/.serena/memories/core.md b/.serena/memories/core.md index 08f98bed6..60c3b7c25 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -11,6 +11,14 @@ This memory is the graph root: every other memory is reached from here, and the trigger for each is stated here rather than inside it (`mem:memory_maintenance`). Read on demand, never all of them. +- `mem:evidence-hierarchy` — **before acting on anything a doc comment, a + `CLOUD-*` row, a PR body or a handoff asserts**, and before citing one as the + reason for a decision. The board audited at ~38% wrong; comments are no better. +- `mem:decision/adr-process` — recording a decision; about to add a `status:`, + `superseded-by:` or version suffix to any document. +- `mem:decision/landing-architecture` — touching `land.rs`, `lease.rs`, + `speculation.rs`, `pipeline.rs` or the landing workflows; any row about the + lease, the lap, speculation, eviction or landing throughput. - `mem:workflow/board-states` — starting or finishing a `CLOUD-*` issue; reasoning about what is in flight. - `mem:workflow/agent-fanout` — spawning a subagent, or running more than one diff --git a/.serena/memories/decision/adr-process.md b/.serena/memories/decision/adr-process.md new file mode 100644 index 000000000..f718742f5 --- /dev/null +++ b/.serena/memories/decision/adr-process.md @@ -0,0 +1,41 @@ +# Decision records: git is the amendment history + +Read when: recording an architectural decision, or about to add a `status:`, +`superseded-by:`, `-v2`, `-amended` or "see the current version" pointer to any +document in this repository. + +## The rule + +**A reader must see only the current correct state of the world.** + +- **One file per decision, always rewritten in place.** Never a chain. +- **`git log` / `git blame` is the amendment history**, reachable only by + explicitly spelunking. History exists, costs nothing to keep, and is invisible + until asked for. Git already does versioned-document-with-history better than + any convention layered on top. +- **No `status: superseded` field**, because a live file is the only kind there + is. A decision that no longer holds is a file that no longer says it. +- **A gate, or it is not a process**: no decision record may carry a supersession + marker, a version suffix in its filename, or a pointer to another record "for + the current version." + +## Why, measured + +Superseded documents, `-amended` suffixes and a folder of near-duplicates make a +doc tree **actively worse than no doc tree**: a reader cannot tell which file is +live, and an agent will confidently quote the dead one. In one 2026-09-10 session +this shape cost two wrong conclusions before the pattern was named — see +`mem:evidence-hierarchy`. + +## Home + +`.serena/memories/decision/`, beside the operational memories. + +Chosen because it needs **no change to non-negotiable rule 7** (`no-docs-tree` +fails a tracked `docs/` path and sends research to the tracker), and because that +directory is already checked in, already read on demand, and already the surface +a future session loads. Routing is `mem:core`'s existing table — the mechanism in +use, not a second one invented alongside it. + +Writes go through Serena's memory tools; `protected-mutation` denies every other +route, and `rename_memory` is the only one that rewrites `mem:` referrers. diff --git a/.serena/memories/decision/landing-architecture.md b/.serena/memories/decision/landing-architecture.md new file mode 100644 index 000000000..b22583552 --- /dev/null +++ b/.serena/memories/decision/landing-architecture.md @@ -0,0 +1,439 @@ +# The landing system: what it is for, and the decisions that follow + +Read when: touching `land.rs`, `lease.rs`, `speculation.rs`, `pipeline.rs`, the +land/lease region of `lib.rs`, the landing workflows, or any `CLOUD-*` row about +the lease, the lap, speculation, eviction or landing throughput. + +This file is a DECISION RECORD under `mem:decision/adr-process`: it is rewritten +in place to the current truth, carries no supersession markers, and `git blame` +is its amendment history. + +## The guarantee everything hangs off + +**Fast-forward-only linearized trunk. Landing PRESERVES the sha.** + +> CI goes green on head `H`; trunk fast-forwards to `H`; the runs on `H` **are** +> the runs on trunk. **CI never runs twice.** + +**The holder does NOT land by rebase.** PR #934's body and commit `3a18fb5`'s +message both say it does ("minting new ones for the same patches") and both are +wrong; a 2026-09-10 session quoted that approvingly and built a whole fencing +argument on it. A waiter that speculates on `H_A` and sees the holder land `H_A` +unchanged was **exactly right** — speculation converges, it does not churn. + +## What the lock is for + +**The lock is the LINEARIZATION mechanism, not a spend limiter.** Its purpose is +to eliminate contention and keep CI **maximally saturated with only green +matrices on the critical path.** + +The two resources are asymmetric, and this is the whole design pressure: + +- **The trunk is serial and metered.** One branch lands at a time and each + occupies a CI matrix that costs money. (Free public runners today; paid before, + when private; paid again on the bigger runners coming.) +- **Agents are parallel and free.** In-process agent work has no monetary cost, + so **agents are the parallelization lever** precisely because trunk + linearization is the bottleneck. + +So: push the maximum work into the free parallel phase, the minimum into the +metered serial one. An agent must prove — with receipts for everything CI will +run — that its tree is already green **before** admission to a matrix. The matrix +VERIFIES a claim; it does not discover. Agent local prep being ≥ half the landing +wall clock, usually much more, is the design working. + +**Speculation is the pipelining mechanism, not a workaround.** While the holder +occupies the slot with `H_A` in CI, a waiter rebases onto `H_A` — the trunk that +is about to exist — and spends its free local prep there, so the instant the +holder lands, the waiter's matrix starts against an already-proven tree. + +**k = 1, and a semaphore is not the lever.** Widening the slot means k heads +racing for one fast-forward target; every loser's matrix is spent on a head that +can no longer land. Throughput comes from pipelining the slot, not widening it. + +## The metrics that do not exist, and must + +None of the three quantities that define the objective are instrumented: + +1. **Skew** — last green matrix end → next green matrix start. The primary + metric; speculation done right drives it toward zero. +2. **Cost per run in BOTH currencies** — CI minutes (monetary) and wall clock. + They diverge and are optimised differently. +3. **Agent local prep wall clock** — free in money, dominant in time, the phase + the design deliberately loads. Unmeasured, so nobody can tell exhaustive + proving from spinning. + +CLOUD-492 measures divergence-from-linear, which is adjacent and not this. **You +cannot tune a pipeline you cannot see** — every design argument here is a guess +until these exist. + +## The poison cascade, sized correctly + +Because speculation is pipelining, a poisoned holder is not one branch's problem: +**every waiter that prepped against `H_A` spent its slow-but-free phase against a +base that will never exist.** The cost is an emptied pipeline plus N agents' prep +discarded — not a wasted matrix. Hence: detect poison with the fastest oracle, +broadcast it, and make the cooldown proportionate. + +**CI is the oracle for poison, not a waiter's local verify.** A waiter's own red +cannot distinguish "the borrowed base is bad" from "my change is bad" without a +second run. That is why the pre-2026-09-10 design needed a two-lap discrimination +dance, a suspicion surviving a process, and a promotion step — all of it working +around asking the wrong oracle. + +## Consensus: we already have it — do NOT implement Paxos or Raft + +**We are not in the message-passing model.** Each ref is a register, +`--force-with-lease` is an atomic compare-and-swap, fetch is a read. + +- **CAS has consensus number ∞** (Herlihy, _Wait-Free Synchronization_). A single + CAS register solves consensus for any number of processes, wait-free. **The + remote already IS our consensus.** Paxos on top would be a second source of + truth that can disagree with the first. +- **There is no peer transport.** Agents have no inbound addresses; every + "message" is a write to the remote another agent reads. "Gossip" here _is_ the + remote, which is already the serialization point — an eventually-consistent + overlay on a linearizable store trades away the strongest property we have. +- **Quorum members are reclaimable containers**, so quorum loss would be routine + rather than exceptional. Raft over ephemeral agents is a liveness liability. + +This is the Chubby argument (Burrows, OSDI'06): most systems want a +lock/consensus **service** their clients call, not an embedded consensus library. +The forge is our Chubby. `openraft` and TiKV's `raft-rs` were considered and +declined — the reason is the substrate, not their quality. + +## What to build instead: a replicated log on the register + +The missing primitive is not agreement but **total order**. **The log is a branch +whose commits are its entries**; appending is commit-then-CAS-the-tip, retrying on +conflict (lock-free, correct for a fleet this size). + +One construction pays for four things: + +1. **A real monotonic epoch** — the log index. Better than the trunk tip, which + only advances when something lands; the index advances on every fleet event, + so bets, evictions and cooldowns can be fenced against activity too. +2. **The queue, with agreement by construction.** Everyone reads the same + sequence, so everyone computes the same order. No votes, no round trips. +3. **Front-of-queue skips as a declared priority class.** `release` and `human` + (the manual no-agent fast-forward) sort ahead of `agent`; FIFO by log index + within a class. The release path stops being a special case that races the + fleet. **Starvation must be designed against** — strict class priority starves + `agent` under sustained release load, so this needs aging or a reserved share, + declared rather than discovered. +4. **An audit trail** — ordered, attributable entries. + +## Eviction: fencing, not voting + +**No failure detector is accurate in an asynchronous system.** FLP kills +consensus with one crash fault under message passing; the CAS register sidesteps +that for _agreement_, but nothing sidesteps _detection_. Every suspicion of a +dead holder is a heuristic, and a vote does not make a wrong suspicion right — it +makes several agents confidently wrong together. + +So the design must be **safe under a false suspicion**, which fencing gives and +voting does not: eviction is a CAS advancing the lease's fencing token. The +evicted holder is not asked and need not be reachable; its next write presents a +stale token and is refused, so **a wrongly-evicted holder cannot corrupt anything +— it loses and re-laps.** (Kleppmann's fencing argument.) + +Therefore the eviction notice is **advisory** (a peer's request the holder may +honour early and cheaply) while the eviction itself is **unilateral and fenced** +(the referee's CAS). Both halves, each doing the job it is sound for. + +## The referee is a declared ROLE, not a named platform + +Non-negotiable rule 1: batten adjudicates for six harnesses (`Harness`, +`crates/batten/src/hook.rs`) and runs on agent containers, CI runners and +developer machines. So **"a GitHub Action" must not appear in `crates/batten`.** +The core defines _an executor whose binary version is attested_ and _which +decisions require one_; a consumer's `batten.toml` names the instantiation. + +In this repository that instantiation already half-exists: five workflows run +`batten lease guard` from a binary installed **from trunk**, not from the PR. It +reads shared state and cancels unauthorised matrices; it never writes. Extending +it to write is the referee. + +**A consumer with no such executor must degrade honestly** — fall back to +epoch-and-TTL expiry, weaker but not wrong. A mandatory referee in the core is a +consumer fact smuggled into the agnostic half. + +Same discipline: the epoch is the **declared trunk's** tip, not `main`'s; the CAS +address is a ref on the declared remote; and the forge behaviours the lock leans +on (atomic ref update, fast-forward enforcement) are **declared capabilities, not +assumptions compiled in.** + +## Identity: the tuple, because a branch name is not an identity + +`host-pid-nonce` dies with the process, and **branch names get reused**. The +stable identity carried IN each entry — never derived from live state that may +have moved — is: + +``` +{account, session-id, session-name, session-url, pr-number, branch, + batten-version, epoch} +``` + +Why each: **pr-number** because a branch name alone cannot be matched to its pull +request after the fact, and an audit trail is about what was true then; +**session-url** so something clickable leads to the harness session that wrote +the entry; **session-name** so it is findable in the GUI; **account** so you know +**which human to hold responsible for a misbehaving agent cluster** — on a fleet +with a world-writable lock, the operationally important question; +**batten-version** so skew is visible rather than inferred. + +**Unresolved and must be settled before designing this in:** which commit-metadata +surface may carry it. `[attribution] identity_deny` governs committer/author and +non-negotiable rule 8 refuses a harness identity request there; `trailer_deny` +forbids `^Claude-Session:` and `claude.ai/code` outright. Neutrality is about _who +is credited_; blame attribution is a different requirement and may need its own +declared surface. Settle against `rules/commits.md` and +`no-denied-identity-prescribed`, not after a gate refuses it. + +## Transcript preservation: `refs/sessions/` + +A redacted transcript per session, from every harness, so agents can investigate +**clusters of repo misbehaviour** without touching code or the landing refs. + +**Shape: a parentless orphan per transcript, content-addressed.** No parent, no +children, created by a plain push — the same test-and-set the lock uses, except +the address IS the content hash, so there is one writer per address by +construction and no contention at all. + +**Inertness is structural, not conventional.** A parentless commit can never +_become_ an ancestor of trunk: it cannot be fast-forwarded into anything and +nothing can descend from it. Keeping it out of the lap's fetch path is then a +performance concern, not a correctness one. + +**Recanting is cheap**: delete the ref and the object is unreachable and +collectable, with nothing orphaned. So **the risk is what has already been read, +not permanence** — which is why fail-closed redaction is a strong default rather +than an absolute bar. + +**Residual risk is not uniform**, and this is the part to design to: + +- **A GitHub credential leaked through GitHub is the safest case** — GitHub scans + its own token formats and auto-revokes its own disclosed credentials. It is + self-defending. +- **Third-party credentials are the actual residual**, because nothing revokes + them. Near-zero in this environment by construction (MCP is credentialless, no + other system access) — but **batten runs across many environments**, so the + core must not assume one where the only credential self-revokes. The redactor's + patterns are a consumer fact; failing closed is the agnostic rule. + +Already built: CLOUD-651 (Done) is the collector; `ripsecrets` is provisioned; +CLOUD-59 (Done) added the `secrets` rule kind. **Durable publication is the +missing half.** + +## The open questions, stated rather than assumed + +1. Which commit-metadata surface carries the blame identity (above). +2. **Log compaction** — an append-only branch grows without bound and every agent + fetches it. Needs snapshot-and-truncate with a measured ceiling, or it becomes + the bandwidth defect the design already warns about. +3. **Starvation policy** for the priority queue — aging, or a reserved share. +4. **Whether CAS addresses must live under `refs/heads`** — untested, and the + claim traces to a misdiagnosed credential failure. See + `mem:evidence-hierarchy` and CLOUD-416. + +## The record layer: what we rent from the forge, and what we must provide + +World-writable means **shared address space**, and the honest question is which +security properties we are renting and which we only assumed. + +- **Rented**: CAS atomicity on ref update; rollback protection on protected refs; + admission control (who may write the repo at all). +- **NOT rented, and currently absent**: any binding between a **body** and the + **writer who wrote it**. The forge authenticates the PUSH, never the RECORD. Its + ACL says "can write this repo" and we have been reading it as "can write this + agent's ref." + +**"Accident, not malice" was the wrong frame** and it is worth naming as an error +rather than quietly dropping. At the record layer a stale agent writing a wrong +body and an attacker writing a wrong body are the SAME EVENT: a verifier cannot +tell them apart and does not need to. The split does no work, and it was being +used to justify less mechanism. + +### DHTs solve exactly this, and they split on mutability + +Vanilla Kademlia has no authentication and is open to storage poisoning. What +shipped in real systems is two constructions: + +1. **Immutable → self-certifying address.** `key = H(value)` (BitTorrent + info-hash, IPFS CID). Fetch from `K`, hash what came back, discard on + mismatch. Tamper-evident with **zero key material**. +2. **Mutable → the address is derived from a public key.** BEP 44 mutable items, + IPNS: `addr = H(pubkey [+ salt])`, body signed, plus a monotonic `seq`; + readers reject a bad signature or a non-increasing sequence. **No PKI, no CA, + no key distribution** — the address IS the identity. That is the whole trick. +3. S/Kademlia adds Sybil resistance (crypto puzzles on node ids, disjoint lookup + paths). **We do not need that half**: the writer population is not open, the + forge ACL already bounds it. This is the precise content of the "no Byzantine + fault" advantage — we need DHT **data authentication**, not DHT **admission + control**. + +### Mapping — git gives construction 1 for free and construction 2 nowhere + +| structure | mutability | have | gap | +| --------------------- | ---------------------- | ------------------------------------------ | ------------------------ | +| `refs/sessions/` | immutable | **self-certifying** — ref name IS the hash | none | +| the log branch | append-only | **Merkle chain** — entries name parents | rewind | +| per-agent ref | mutable, single-writer | nothing | **the whole BEP 44 gap** | +| the lease | mutable, multi-writer | CAS | attribution | + +Per-agent refs are BEP 44 verbatim: an ephemeral keypair per session, ref at +`refs/batten/agent/`, body signed over `(seq, payload)`, readers +verify-or-discard — and a discarded record folds into the membership rule already +committed (_derived from observation with expiry_), so an unverifiable agent reads +as absent. **Fail-closed on content, fail-open on availability.** It also kills a +defect class for free: two agents cannot collide on an id and none can squat +another's address. + +For the **lease**, signing changes nothing about eviction — that stays a fenced +CAS, safe under false suspicion. What it buys is that **you cannot write a body +falsely naming another session as holder**, and the fencing token becomes +attributable. + +### Where the analogy runs out: replay + +**Signatures do not stop rollback.** An old signed body is perfectly valid; +force-pushing a ref back to it passes every check. BEP 44 stops this with a `seq` +**readers remember** — DHTs can because many nodes do, which is precisely the +quorum rejected above (no peer transport, ephemeral containers). Our readers are +reclaimable containers with no memory. + +So monotonicity must anchor in **a ref with server-side rollback protection** +(branch protection forbidding force-push), and ultimately trunk. We borrow the +forge's rollback protection for monotonicity the same way we borrow its CAS for +consensus — and that dependency is a **declared forge capability**, because forge +behaviour demonstrably varies by namespace. + +CLOUD-877 already asks for this construction (_"give it a portable signed form"_). +CLOUD-591 decided not to sign COMMITS — a different surface, orthogonal, the way +rule 8 splits trailer from committer. + +## Deutsch's fallacies, as commitments to check against + +**Eight, not seven.** An earlier version merged _bandwidth is infinite_ with +_transport cost is zero_. They are different, and merging them hid the one this +project is most directly a response to: transport cost is the entire economic +premise (serial metered trunk, parallel free agents). + +| # | Fallacy | Commitment | Mechanism | +| --- | -------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | +| 1 | The network is reliable | **Name both errors and fail toward the cheaper one, per (read, decision) pair.** Never a blanket direction — see below. | partial, per call site | +| 2 | Latency is zero | No wall clock is READ inside a decision; instants are supplied. **But expiry IS a clock, and cross-agent skew is assumed and unmeasured.** | `clippy.toml` sleep ban, `tests/sleep_ban.rs`; skew: **none** | +| 3 | Bandwidth is infinite | Ref advertisement is O(total refs); 79,973 bytes already on record. Per-agent refs and transcripts must not ride it. | **none** — needs `ls-refs` `ref-prefix` scoping | +| 4 | The network is secure | Every mutable record carries a signature verifiable against its own address, plus a sequence anchored in a rollback-protected ref; a reader discards what it cannot verify. | **none yet** — CLOUD-877 | +| 5 | Topology doesn't change | Membership derived from observation, never a registry needing clean exit — **and a vanished agent's leavings are recoverable.** | `Bet::recovered`; orphan refs **CLOUD-1747, open** | +| 6 | There is one administrator | Every body carries a schema major; readers reject an unknown one and ignore unknown keys. | `Body::schema` **landed**; "readers before writers" **no gate** | +| 7 | Transport cost is zero | **The premise, not a caveat.** Trunk serial and metered, agents parallel and free; speculation is pipelining, the lock is linearization. | the speculation design; metrics **none** | +| 8 | The network is homogeneous | No host's quirk reaches `crates/batten` — **and no forge's either.** | rule 1 **enforced**; forge capabilities **prose only** | + +### 1 was actively wrong, and the wrong version would license a real bug + +It read _"every read fails open."_ The tree contradicts that where it matters. +`Live::decide` fails **closed** — _"failing open there would make a network blip +the thing that lands somebody else's work"_ — while `authorises` fails **open** — +_"a lease it cannot read stops EVERY job in the fleet, where waving one matrix +through costs one matrix."_ + +Same read, opposite directions, because the **question** differs. The direction is +a property of the (read, decision) pair, structurally the same argument +`rules/scanning.md` makes for decider-vs-floor being relational and therefore not +expressible as a column. So: at every could-not-look, state the two errors and +their costs and fail toward the cheaper — and a new call site owes that sentence. + +The old second clause — _"no fleet-binding decision rests on one failed read"_ — +**is not implemented.** `authorises` reads once. A gap, not a property. + +### 2's real gap is clocks, and it is worse than skew + +The sleep ban is real and enforced. The clock underneath is not. + +**`now_unix()` is `SystemTime::now()` — `CLOCK_REALTIME`, not monotonic.** NTP +step, VM resume and container migration all move it backwards, so a holder that +beats across a backward step re-mints an EARLIER `expires` than the one already +published. **And it answers `0` when the clock will not read**, which makes +`expired(now)` (`now >= expires`) say nothing is ever expired — a clock failure +wedges the whole fleet rather than degrading. `expires == 0` is separately the +tombstone sentinel (`released()`), so zero carries two meanings. + +**But no clock choice fixes the real defect.** `expired(now)` compares MY `now` +against YOUR `expires` — an absolute timestamp crossing the wire — and monotonic +clocks are not comparable across machines either. **Expiry must never cross the +wire.** The correct shape is observer-local staleness: B observes the lease at its +own monotonic instant with progress token `P`; if after B's own elapsed ΔT the +token is still `P`, B may judge the holder stalled. B never reads A's clock. + +**That mechanism already exists beside it.** `progress` is opaque and compared +_"for EQUALITY OVER TIME… so no clock crosses the wire"_. So `expires` is largely +redundant with its neighbour and is the half carrying every hazard. The change: +**promote the progress token to the primary liveness signal, demote expiry to a +backstop, and if a timeout is kept make it an `Instant` measured locally** — never +a timestamp shipped in a body. + +### Clocks do two jobs and only one needs real time + +- **Ordering and validity** — "is this record newer", "is my bet against the + current epoch" → **logical clock: the log index.** No real time at all. +- **Failure detection** — "is the holder dead" → **irreducibly needs a timeout.** + FLP: a crash cannot be detected without one, and no logical clock answers + liveness. This part cannot be designed away, only confined. + +### Why NOT vector clocks + +Considered and declined, so nobody reopens it cold — and recorded because the +first version of this file did not consider them at all. + +1. **They reconstruct a partial order where there is no serialization point. We + have one.** Every fleet event goes through a CAS on a single ref. Same shape as + the Paxos argument above: a vector relates events across independent replicas, + and we deliberately have one totally-ordered log. **The log index is already a + Lamport clock and the total order makes a vector strictly redundant.** +2. **O(N) entries with unbounded, churning N.** Agents are reclaimable containers; + vector-clock GC for departed processes is unsolved, so entries for dead agents + accumulate in every body forever — straight into fallacy 3. +3. **They DETECT concurrency; the lease must PREVENT it.** Learning after the fact + that two writes were concurrent gives no mutual exclusion. `--force-with-lease` + refuses the loser; CAS is strictly stronger than detection for this job. +4. **On single-writer sharded refs a vector degenerates to a scalar** — no + concurrency to relate, so a per-writer sequence is exactly sufficient, which is + what BEP 44 uses. + +**The case where a vector IS required, stated as a constraint on the multi-address +design:** N independent CAS addresses means N independent logs and no single total +order, and relating events across them genuinely needs a vector with its GC cost. +So: **one log → a scalar index suffices; multiple independent logs → vector clocks +and their costs.** An argument for one log, or for declaring separate logs +causally independent and never compared. + +The sleep ban is real and enforced. But `Body::expires` is an absolute instant and +`expired(now)` compares against one, so the liveness model **is** a wall clock — +merely supplied across the boundary rather than read inside. It assumes bounded +skew and nothing measures it: a holder with a slow clock holds past TTL while a +rival with a fast clock steals mid-matrix, which is **two landers** — the exact +failure the lease exists to prevent. + +The skew-free primitive already exists: `progress` is opaque and compared for +EQUALITY OVER TIME, so no clock crosses the wire. That is a logical clock. +**Promote it to the primary liveness signal and demote expiry to a backstop**, and +meanwhile advertise each agent's clock beside its version so skew is measured +rather than assumed. + +### 3 — and the transcript design above violates it + +"Keep it out of the lap's fetch path" is not enough: **ref advertisement is not +fetch.** The v0/v1 handshake lists every ref whatever you fetch, so transcripts — +the largest objects in the system — would bloat every lease read. The fix is +protocol v2 `ls-refs` with `ref-prefix`, scoping the advertisement server-side. + +### 5, 6, 8 each have a named unbuilt half + +A reclaimed container leaves a held lease, a readied PR, a burning matrix and +stranded `refs/batten-spec/*` (CLOUD-1747). _Readers ship before writers_ is a +process promise with no gate, which rule 2 calls half a change — `Body::writer` is +the mechanism, refusing to raise the major until the observed fleet minimum is +high enough. And rule 1 governs hosts, not **forges**: fast-forward enforcement was +measured differing by namespace on one forge, so declared forge capabilities need +to be a table. diff --git a/.serena/memories/evidence-hierarchy.md b/.serena/memories/evidence-hierarchy.md new file mode 100644 index 000000000..c6762bbe6 --- /dev/null +++ b/.serena/memories/evidence-hierarchy.md @@ -0,0 +1,68 @@ +# What counts as evidence, and what is only a claim + +Read when: about to act on anything a doc comment, a `CLOUD-*` row, a PR body, a +previous session's handoff, `AGENTS.md` or another memory asserts — and ALWAYS +before citing one as the reason for a design decision. Also read it before +writing a claim into any of those surfaces yourself. + +## The ranking + +1. **Code and tests as they execute right now**, and **the output of a command + you ran and read yourself.** +2. **Everything else is a CLAIM**: doc comments, the board, PR bodies, commit + messages, `AGENTS.md`, these memories. Comments are written by the same agents + who write the board, reviewed no harder, and go stale the same way. +3. **Even (1) goes stale.** A command's output describes the moment it ran, and + the remote is shared and mutable. + +**The operational rule: if a claim is load-bearing, re-derive it from something +executable. If nothing executable exists, THAT ABSENCE IS THE FINDING.** A +constraint worth designing around is worth a test. + +`rules/scanning.md` row five is the same rule one level down: to know what a gate +DECIDES, run it and read the exit code. Reading source to predict a verdict "is +worse than guessing, because it looks like rigour." + +## The board's measured error rate + +**5 of 13** speculation/lease rows audited against the tree on 2026-09-10 were +STALE or REFUTED — ~38%. Rows are written by agents, many running a stale binary +that only ever saw a half-implementation. + +**A cheap discriminator, worth keeping:** a row or comment that cites a **retired +artefact** — a shell variable (`spec_undo`, `spec_base`, `LAND_LOCK_HOLDER_PID`), +a line number in `mise-tasks/land.sh` or `land-lock.sh` — was wrong **4 times in +5**. A row citing live Rust paths was usually at least partly live. It is triage, +not a substitute for reading the tree. + +## Worked examples, so the shape is recognisable + +- **A doc comment that is the foundation of a design and has no test.** + `lease.rs:1310-1315` asserts the agent proxy 403s a push outside `refs/heads`, + and concludes the landing lock must therefore be a branch. No test anywhere + asserts it; `git ls-remote origin` shows `refs/notes` DOES exist on the remote; + and `mem:github-access` measures the real mechanism — proxied, `git` + authenticates with the INJECTED token, which 403s writes it is not scoped for. + Same symptom, different cause. **CLOUD-416 records that this misdiagnosis cost + the lease being implemented four times.** +- **A Done row whose landing claim is false.** CLOUD-1399 (Done) says commit + `46530200` landed the `egress-is-unproxied` `[[startup]]` row. + `git log -S'egress-is-unproxied' --all -- batten.toml` is EMPTY and `46530200` + is not a valid object. The detector (`doctor egress`) landed; the repair never + did. +- **A false premise propagating from a PR body into pushed history and then into + a plan.** PR #934's body and commit `3a18fb5`'s message both state "the holder + lands by rebase, minting new ones for the same patches." **False** — landing is + fast-forward and PRESERVES the sha (`mem:decision/landing-architecture`). A + later session quoted it approvingly and built a fencing argument on it. +- **Two Urgent rows refuted by inspection.** CLOUD-240 reasons over + `mise-tasks/land.sh:162`, a deleted file; CLOUD-1423 says `batten land` has zero + callers, but `land lap` is `mise.toml:3494`. + +## When you find one + +A row the tree refutes goes **back to Backlog with a comment saying what the tree +says instead** — never a note edited into its body, which leaves the false claim +in place above the correction. Then it is not worked until re-filed against +reality. `mem:workflow/board-states`: a state is a claim about the tree, and the +tree wins. diff --git a/.serena/memories/github-access.md b/.serena/memories/github-access.md index 471bf0e64..e8ece4779 100644 --- a/.serena/memories/github-access.md +++ b/.serena/memories/github-access.md @@ -81,10 +81,67 @@ mediated gate permitted for half a session: `no-tool-substitution`, `trailer_deny` trailer reach the remote. **At the hook boundary, refuse and permit are the same byte.** `mise run install:local` is the unblock. +**Measured again 2026-09-10, with the version numbers, because this class recurs.** +Installed binary **0.0.156**, tree at **0.0.159**. `batten.toml` declares +`command_matcher` (commit `4bc4f57`'s key, whose own footer reads +"`handler::Handler` gains a `command_matcher` field") and 0.0.156 cannot parse +it, so `batten startup` refused the **whole config** — every mediated gate +permitting, `egress-is-unproxied` never reachable, `hook-surfaces-are-battens` +never reaped. `mise run install:local` fixed it; the gates went live immediately +and began correctly denying `no-tool-substitution` and `background-redirect` on +calls that had gone unjudged minutes earlier. `contract-drift` then self-reported: +_"this session's SessionStart registration did not run … every mediated call until +it appeared failed open and said nothing."_ + +This is CLOUD-1775's shape and it is **self-referential**: the skew detector is +declared with `command_matcher`, the very key a stale binary cannot parse, so it +is unreachable in exactly the case it exists for. + +**`batten startup --repair` was ALSO needed by hand** in the same session, and +`hook-surfaces-are-battens failed not-provisioned` went to `ok` only after it — +which is what leaves the launcher's own stop hooks firing when batten is supposed +to have zeroed them in place. Needing the repair by hand is the finding, not the +fix. + A repair the host refuses is the ONE ask to put to a human (CLOUD-680's shape). Name the refusal you actually got; never assert which settings key would have granted it unless you measured that key doing something. +## The fence covers `mise` and NOT batten itself — measured 2026-09-10 + +`/root/.local/bin/mise` is a `provision-exec` wrapper whose declared env fences +`github.com` and four sibling hosts out of `NO_PROXY`, sources the token from a +first-set chain including `BATTEN_GITHUB_TOKEN`, carries +`reject_prefix = "proxy-"` to refuse the injected placeholder, and unsets +`HTTPS_PROXY` where the trust store names Anthropic. It is correct and complete. + +**It covers exactly one binary.** `/root/.local/bin/batten` is a bare stripped +ELF, not a wrapper — so the engine, the session shell and `git` all run +un-fenced. Confirmed in-container: `NO_PROXY` carries no `github.com`, while the +PAT reaches GitHub off-proxy (`rate_limit` core **5000**, not 15000). + +`fetch.rs` honouring `*_PROXY` is **deliberate and correct**, not a defect — +CLOUD-1399 argues it explicitly: _"that behaviour is correct and is what lets +batten work on a host where a proxy is mandatory. The defect is a container's +values."_ Only `get_direct` (the credential probe) refuses a proxy, because a +question about a credential has no answer on a route that substitutes its own. + +**But the repair never landed.** CLOUD-1399 is Done and claims commit `46530200` +shipped an `egress-is-unproxied` `[[startup]]` row. +`git log -S'egress-is-unproxied' --all -- batten.toml` is EMPTY and `46530200` is +not a valid object here. `batten doctor egress` correctly answers +`egress failed egress-unfenced` and **nothing repairs it** — non-negotiable rule +2's own words: _a gate that detects what it is wired to fix and waits to be asked +is sensor only._ `BATTEN_ENVIRONMENT=disposable` IS set, so the trigger is not +the problem; the row does not exist. + +CLOUD-1400 is the row for batten owning its own proxy (a decision spike, not +work). CLOUD-1475 replaces the PAT-in-environment entirely with +`repository_dispatch` as an authenticated bus and a webhook listener holding the +secrets — zero bearer in the VM. Until then the standing constraints are: a live +PAT in the environment, rotated frequently; **no access to any other system**; +MCP is credentialless. + ## Why the toolchain runs here (a per-host fence, NOT unsetting the proxy) **`mise` NEEDS BOTH HALVES, AND THE FIRST ONE IS `NO_PROXY`.** Re-measured @@ -302,6 +359,16 @@ dropped), confirm green, and land. just the old SHA dropped when you pushed a new head.) - Never echo a credential. Check presence with `${VAR:+SET}` — never a bare `$VAR` or a `${VAR:-…}` that expands the value into the transcript. +- **`${VAR:+SET}${VAR:-UNSET}` LEAKS THE VALUE, and it reads as safe.** Measured + 2026-09-10: it printed a live `ghp_` PAT into a session transcript. `:-` + substitutes only when the variable is UNSET or empty, so when it is set the + second half expands the credential. The concatenated "report either way" form + is the trap; `${VAR:+SET}` alone is the whole check. +- **Rotate a leaked PAT AFTER the session, never during.** A refreshed PAT can + only reach a session through its context, so rotating mid-session strands the + container with a dead credential and no route to the new one. Note also that a + GitHub credential disclosed _to GitHub_ is the least-bad case: GitHub scans its + own token formats and auto-revokes its own disclosed credentials. ## Transparent TLS interception — tools that carry their own CA roots diff --git a/.serena/memories/workflow/landing-loop.md b/.serena/memories/workflow/landing-loop.md index 48ada8816..7e50c5946 100644 --- a/.serena/memories/workflow/landing-loop.md +++ b/.serena/memories/workflow/landing-loop.md @@ -16,6 +16,15 @@ listed at the bottom with their bypasses so a refusal can be told from a defect. fetch → rebase → `verify` → `verified` → push → `ci-wait` ∥ `main-watch` → `/fast-forward` → read the answer → lap. +**What the lap is FOR, before any of its mechanics: landing is fast-forward and +PRESERVES the sha**, so the runs that went green on the pushed head _are_ the +runs on trunk and CI never runs twice. The lock is therefore the **linearization** +mechanism — it exists to keep CI saturated with only green matrices on the +critical path — and **speculation is pipelining**, not opportunism. The holder +does NOT land by rebase; PR #934's body and commit `3a18fb5`'s message both claim +it does and both are wrong. Full architecture, metrics and the consensus +reasoning: `mem:decision/landing-architecture`. + A refusal is **the design working**, not a failure. Each lap rebases onto a little more landed work, so conflicts arrive one small resolvable increment at a time; batching laps removes no refusal and only makes each one bigger. An agent @@ -52,10 +61,20 @@ with the branch gone (`git ls-remote` confirms). There is nothing to repair here operation, no service, no API. Four things the design was pressure-tested into, each of which cost an incident: -- **It is a BRANCH.** The agent proxy 403s a push outside `refs/heads`, and - GitHub does not enforce the fast-forward rule off `refs/heads` either — a - parentless orphan `PATCH` with `force:false` was _accepted_ on a custom - namespace. The atomicity the design rests on exists only on `refs/heads`. +- **It is a BRANCH — and this premise is UNTESTED and probably a misdiagnosis.** + The claim was: the agent proxy 403s a push outside `refs/heads`, and GitHub does + not enforce the fast-forward rule off `refs/heads` either. **No test anywhere + asserts either half**; the code only _defaults_ unqualified names to + `refs/heads/` (`lease.rs:2845`), which is a naming convenience. `git ls-remote +origin` shows `refs/notes` DOES exist on this remote. And `mem:github-access` + measures the real mechanism: **proxied, `git` authenticates with the INJECTED + token, which 403s any write it is not scoped for** — same symptom, different + cause, and the same root cause as CLOUD-1569. CLOUD-416 (Urgent, never started) + records that this misdiagnosis cost **the lease being implemented four times**, + and CLOUD-416 itself repeats the wrong cause, so its write probe must run + fenced and PAT-authenticated or it will re-measure the credential bug and bake + it in as an environment fact. Treat the namespace question as open; see + `mem:decision/landing-architecture`. - **Renewal is `--force-with-lease=:`**, a true CAS. `PATCH` with `force:false` does not give one. Create stays a plain push, so acquire is an atomic test-and-set. diff --git a/crates/batten/src/commit.rs b/crates/batten/src/commit.rs index eb8238190..e40609ca1 100644 --- a/crates/batten/src/commit.rs +++ b/crates/batten/src/commit.rs @@ -197,8 +197,40 @@ impl Commit { /// which is the whole reason the block spells every binding field out instead of /// carrying a reference. A tier that needed the store would pass locally and /// abstain in CI, which is the shape of a gate that is not there. +/// +/// # A path with a sanctioned mutation owes no block, and that is the override +/// route's own precondition read back (CLOUD-1303) +/// +/// `path write refused` declares its override route for the case where *"the +/// surface this class names cannot express the change, so writing the protected +/// path directly is the only route left"*. A path a `[[redirect]]` speaks for is +/// the negation of that sentence: the surface exists, the write goes through it, +/// nothing is refused, and so **no admission is ever issued and there is nothing +/// to articulate**. Demanding a block there leaves exactly one route — an +/// override whose precondition is false — so the honest author must either write +/// a false articulation or not commit at all. Measured: `.serena/memories/**` +/// joined `protected` and the `[[redirect]]` table together, and the first commit +/// to write a memory through `write_memory` was refused six times over. +/// +/// Articulation is therefore owed by protected paths with **no** sanctioned +/// mutation — which is the set the precondition describes. +/// +/// **ONLY THE DEMAND IS DROPPED, NEVER THE INSPECTION.** The earlier shape of +/// this fix exempted the path before looking at it, which silently lost +/// `admits-tampered` for the whole exempted set — a doctored block on a memory +/// would have gone unexamined. Here the redirect answers only the *absence* case: +/// a block that claims the path is still verified, and still reported when it +/// does not recompute. The graver finding keeps its whole subject set. +/// +/// `redirects` is the config's own table, passed in rather than resolved here for +/// [`ArmSequence`]'s reason: the predicate stays a pure function of its arguments, +/// so it decides identically on a runner that cannot reach the store OR the +/// config — which is what keeps the range half and the pending half from drifting. #[must_use] -pub fn judge_admissions(writes: &[crate::git::CommitWrite]) -> Vec { +pub fn judge_admissions( + writes: &[crate::git::CommitWrite], + redirects: &[crate::redirect::Redirect], +) -> Vec { let mut found = Vec::new(); for write in writes { let blocks = crate::admission::blocks(&write.message); @@ -207,15 +239,23 @@ pub fn judge_admissions(writes: &[crate::git::CommitWrite]) -> Vec { .iter() .filter(|block| block.binding.subject == *path) .collect(); - // A path with no block at all is the missing case. A path with blocks - // of which at least one verifies is clean — several are legitimate, - // since re-articulating the same path on one commit chains rather than - // replaces. + // A path with no block at all is the missing case — unless a + // `[[redirect]]` sanctions a mutation for it, which means the write + // had a route that refuses nothing and issues nothing. A path with + // blocks of which at least one verifies is clean — several are + // legitimate, since re-articulating the same path on one commit chains + // rather than replaces. let field = if claims.is_empty() { + if crate::redirect::resolve(redirects, path).is_some() { + continue; + } "admits" } else if claims.iter().any(|block| block.recomputes()) { continue; } else { + // REACHED FOR A REDIRECTED PATH TOO, and that is the half the + // superseded fix dropped. + //MUTANT admits-tampered-survives-the-redirect|s/^ } else if claims/ } else if crate::redirect::resolve(redirects, path).is_some() || claims/|a tampered block on a redirected path is still refused "admits-tampered" }; found.push(Finding { @@ -239,12 +279,19 @@ pub fn judge_admissions(writes: &[crate::git::CommitWrite]) -> Vec { /// explicitly not what this commit is about, and demanding a block for it would /// refuse a commit that does not touch the path at all. #[must_use] -pub fn judge_pending(message: &str, staged: &std::collections::BTreeSet) -> Vec { - judge_admissions(&[crate::git::CommitWrite { - commit: "pending".to_owned(), - message: message.to_owned(), - paths: staged.clone(), - }]) +pub fn judge_pending( + message: &str, + staged: &std::collections::BTreeSet, + redirects: &[crate::redirect::Redirect], +) -> Vec { + judge_admissions( + &[crate::git::CommitWrite { + commit: "pending".to_owned(), + message: message.to_owned(), + paths: staged.clone(), + }], + redirects, + ) } /// One commit's two conserves-ledger sets, ready to intersect (CLOUD-1402). diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 73084dd12..16aa5bb8a 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -1386,7 +1386,8 @@ pub const RETIRED_KEYS: &[(&str, &str)] = &[( /// /// [`trust::load_base`]: crate::trust::load_base pub fn parse_base(text: &str, source: &str) -> Result { - let mut table: toml::Table = toml::from_str(text).map_err(|err| config_error(source, &err))?; + let mut table: toml::Table = + toml::from_str(text).map_err(|err| config_error(source, text, &err))?; // Nothing is reported when a key is dropped: the report this feeds is a // comparison of two policies, and "the base declared a key this build no // longer has" is a fact about the build rather than about either policy. @@ -1518,7 +1519,8 @@ pub fn parse_override(text: &str, source: &str) -> Result { prune_unresolvable::(text, binary_is_behind_the_config(source, text)); let config = match pruned.config { Some(config) => config, - None => toml::from_str(&pruned.text).map_err(|err| config_error(source, &err))?, + None => toml::from_str(&pruned.text) + .map_err(|err| config_error(source, &pruned.text, &err))?, }; (config, pruned.dropped) }; @@ -1967,8 +1969,35 @@ fn names_an_unknown_key(rendered: &str) -> bool { //MUTANT-SUITE crates/batten/tests/it/config_skew.rs //MUTANT skew-reads-as-malformed|s@ if !names_an_unknown_key(&rendered) {@ if true {@|an_unknown_key_names_the_rebuild //MUTANT every-parse-error-blames-skew|s@ if !names_an_unknown_key(&rendered) {@ if false {@|a_malformed_config_does_not_mention_a_rebuild -pub(crate) fn config_error(source: &str, err: &toml::de::Error) -> anyhow::Error { +pub(crate) fn config_error(source: &str, text: &str, err: &toml::de::Error) -> anyhow::Error { let rendered = err.to_string(); + // THE SYNTAX PROBE, AND IT RUNS ONLY HERE — ON THE ERROR PATH (CLOUD-1677). + // + // `toml::de::Error` is the one type for two very different faults, and the + // rendering hides it: a missing field and an invalid type both arrive as + // "TOML parse error at line N, column C", exactly like a stray brace. Reading + // the message cannot tell them apart, and `names_an_unknown_key` answers a + // third question again — measured on the `[[fact]]`-with-no-`returns` fixture, + // which is a SCHEMA fault that renders as a parse error and was classed as + // unreadable by the message alone. + // + // A `Table` parse answers it exactly: if the bytes are well-formed TOML then + // whatever failed was the SCHEMA over them, and the file still has rows a + // build can read. This is the probe the comment in `parse_ungated` records as + // removed for costing a parse on the hot path — it is free here, because + // nothing reaches this function until a parse has already failed. + if toml::from_str::(text).is_err() { + // CLASSED, AND THE CLASS IS A DISCRIMINATOR RATHER THAN A LABEL. This is + // the one config fault with no partial function left to preserve — the + // file is not TOML, so no row is readable and none can be enforced. Every + // other fault leaves the rest of the file deciding, which is what lets an + // agent be told to repair the broken part instead of losing the gate + // surface entirely. `Native::ConfigUnreadable` carries the full argument. + return UsageError::raise_as( + crate::verdict::Native::ConfigUnreadable, + format!("invalid config {source}: {err}"), + ); + } if !names_an_unknown_key(&rendered) { return UsageError::raise(format!("invalid config {source}: {err}")); } @@ -3042,7 +3071,8 @@ fn parse_ungated(text: &str, source: &str) -> Result { let pruned = prune_unresolvable::(text, binary_is_behind_the_config(source, text)); let config = match pruned.config { Some(config) => config, - None => toml::from_str(&pruned.text).map_err(|err| config_error(source, &err))?, + None => toml::from_str(&pruned.text) + .map_err(|err| config_error(source, &pruned.text, &err))?, }; (config, pruned.dropped) }; diff --git a/crates/batten/src/exec.rs b/crates/batten/src/exec.rs index 07e9d9511..9bf4d15af 100644 --- a/crates/batten/src/exec.rs +++ b/crates/batten/src/exec.rs @@ -1742,7 +1742,15 @@ pub(crate) fn piped( // relative name cannot arise and handing a directory would only be a guess at // one. // `Drop`: both callers of this entry point parse the string it returns. - piped_through(root, None, path.to_str()?, args, stdin, Diagnostics::Drop) + piped_through( + root, + None, + path.to_str()?, + args, + stdin, + Diagnostics::Drop, + &[], + ) } /// The one spawn both piped entry points share. @@ -1777,16 +1785,38 @@ fn piped_through( args: &[String], stdin: &str, diagnostics: Diagnostics, + published: &[(String, Option)], ) -> Option<(i32, String)> { let mut child = crate::rules::spawn_resolving(resolve_root, program, |resolved, extra| { - Command::new(OsString::from(resolved)) + let mut command = Command::new(OsString::from(resolved)); + command .args(extra.iter().map(OsString::from)) .args(args) .current_dir(root) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(diagnostics.redirection()) - .spawn() + .stderr(diagnostics.redirection()); + // THE BET TRAVELS TO EVERY GATE THAT READS THE COMMIT RANGE, not only to + // `verify`'s (CLOUD-1770). `BATTEN_SPEC_BASE` is the boundary + // `claimed-keys` narrows on, and it was published into the verify child's + // environment alone — so `closing-key-check`, which runs at a later step + // and delegates to the same reader, saw no bet and counted the holder's + // borrowed keys as this branch's own. Measured: two lease acquisitions + // spent and handed back on keys the speculation adopted. + // A `None` REMOVES rather than skips, and the difference is the whole + // reason this is `Option` (measured: this file's own suite runs INSIDE a + // `land` gate, which exports the variable, so a child that merely + // inherited it read a bet that was not its own). Publishing must be a + // FUNCTION of the bet — a lap with none outstanding has to say so, or a + // stale value from an outer process narrows the inner lap's commit range + // against a base it never borrowed. + for (name, value) in published { + match value { + Some(value) => command.env(name, value), + None => command.env_remove(name), + }; + } + command.spawn() }) .ok()?; // TAKEN AND DROPPED EVEN WHEN EMPTY, because a gate that reads stdin blocks @@ -1975,13 +2005,22 @@ pub(crate) fn piped_argv( argv: &[String], stdin: &str, diagnostics: Diagnostics, + published: &[(String, Option)], ) -> Option<(i32, String)> { let (program, operands) = argv.split_first()?; // `Some(root)`, where [`piped`] passes `None`: the first word here is a NAME // the ladder resolves, so rung 3 needs a directory to read a shebang out of. // That one argument IS the difference between the two entry points, which is // why they share [`piped_through`] and not a signature. - piped_through(root, Some(root), program, operands, stdin, diagnostics) + piped_through( + root, + Some(root), + program, + operands, + stdin, + diagnostics, + published, + ) } /// This process's next dispatch number, for the live-capture key. diff --git a/crates/batten/src/land.rs b/crates/batten/src/land.rs index ee84e9c33..dbce97b2e 100644 --- a/crates/batten/src/land.rs +++ b/crates/batten/src/land.rs @@ -1428,11 +1428,43 @@ impl Step { } } +/// Whose base this lap's tree is sitting on when a step answers. +/// +/// The one fact that decides whether a refused gate is a statement about THIS +/// branch (CLOUD-1306). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Basis { + /// The branch is replayed onto trunk itself. A gate's refusal is about this + /// tree and nothing else. + Own, + /// The branch is replayed onto a lease holder's unlanded commits — "the main + /// that is about to exist". A gate's refusal is about this tree **and** that + /// borrowed base, and nothing in the exit code separates them. + Borrowed, +} + +impl Basis { + /// The basis for a lap whose bet is or is not outstanding. + /// + /// One place that turns "is a bet live" into this reading, so a caller cannot + /// spell the mapping backwards — which on this type would silently invert the + /// whole cell it qualifies. + #[must_use] + pub const fn of(speculating: bool) -> Self { + if speculating { + Self::Borrowed + } else { + Self::Own + } + } +} + #[must_use] pub const fn progress_of( step: Step, code: crate::exit::ExitCode, seen: Option, + basis: Basis, ) -> Progress { // THE REFUSAL, NOT THE STEP. Keying on `(Wait, Red)` alone reads a wait that // SUCCEEDED as a stop whenever a red reading is in hand — which the driver @@ -1443,6 +1475,39 @@ pub const fn progress_of( { return Progress::Stop; } + // A REFUSED GATE OVER A BORROWED BASE IS NOT YET THIS BRANCH'S ANSWER. + // + // `progress`'s stop cell reasons that "the replay and the gate both answer + // about THIS tree, so a refusal from either is a decision no rebase clears". + // That is exactly right when the branch sits on trunk and exactly wrong when + // it does not: a speculative lap's tree is this branch's commits replayed + // onto a holder's unlanded ones, so a red gate has two candidate causes and + // the exit code distinguishes neither. Stopping attributes the failure to the + // author on no evidence. + // + // The cost of getting it wrong is not one branch's. Under fast-forward + // linearization the waiter's local prep is the free half of the pipeline, and + // a stop here discards a prepared branch — so when the slot frees there is + // nothing proven ready to take it, and the skew that speculation exists to + // drive toward zero opens back up. It also burns the agent's remaining + // tokens, which is the opposite of what this loop is for. + // + // **AND IT MADE THE POISONED-BASE MECHANISM INERT.** That path's own comment + // says "the NEXT lap settles `Poisoned`", and behind this cell there was no + // next lap — a mechanism whose every part existed and whose entry cell voided + // it, which is the dead-gate class this module already shipped once at + // `(Ready | Verify, Internal)`. + // + // LAPPING IS THE DISCRIMINATION, NOT AN EVASION. The next replay drops the + // borrowed base, so the same gate runs again over this branch on trunk — and + // a refusal THERE arrives with `Basis::Own` and stops, carrying the evidence + // the first refusal lacked. That is one extra local gate run, which costs no + // CI matrix at all, to avoid blaming an author for a neighbour's tree. The + // driver bounds the repeat under `Bound::SpeculativeRefusals` so a base that + // keeps refusing cannot lap forever. + if let (Step::Verify, crate::exit::ExitCode::Violation, Basis::Borrowed) = (step, code, basis) { + return Progress::Lap; + } progress(step, code) } @@ -1597,7 +1662,12 @@ pub enum Readied { /// right way round: a gate that cannot run has not passed, and treating it as /// clean is exactly how a retired or renamed gate goes silently dead. #[must_use] -pub fn ready(root: &Path, gates: &[Vec], body: &str) -> Readied { +pub fn ready( + root: &Path, + gates: &[Vec], + body: &str, + published: &[(String, Option)], +) -> Readied { if body.trim().is_empty() { return Readied::Clear; } @@ -1611,8 +1681,15 @@ pub fn ready(root: &Path, gates: &[Vec], body: &str) -> Readied { // is identical across every possible finding is not a pointer (review of // #848). let gate = argv.join(" "); + // `published` CARRIES THE BET (CLOUD-1770). A body gate that reads the + // PR's commit range — `closing-key-check` does, through `claimed-keys` — + // cannot otherwise tell a commit this branch authored from one the + // speculation adopted, and counts the holder's keys as this branch's + // stranded ones. CLOUD-748 fixed that for `claim-race-check` by + // publishing the base into `verify`'s child; this is the same boundary + // reaching the same reader through its other caller. let Some((code, output)) = - crate::exec::piped_argv(root, argv, body, crate::exec::Diagnostics::Keep) + crate::exec::piped_argv(root, argv, body, crate::exec::Diagnostics::Keep, published) else { return Readied::Unrunnable { gate }; }; @@ -1838,7 +1915,7 @@ pub fn admits_the_landing(root: &Path, gates: &[Vec], pr: &str) -> Admit let gate = with_pr.join(" "); with_pr.push(pr.to_owned()); let Some((code, output)) = - crate::exec::piped_argv(root, &with_pr, "", crate::exec::Diagnostics::Keep) + crate::exec::piped_argv(root, &with_pr, "", crate::exec::Diagnostics::Keep, &[]) else { // AN ADVISORY GATE THAT WILL NOT RUN IS NOT A REFUSAL EITHER, which // is the same reading one line down rather than a separate decision: @@ -1881,6 +1958,22 @@ pub enum Bound { /// as anything wrong with the branch. The remedy is a shorter gate or a /// quieter trunk, never a re-run. GateReclaims, + /// The gate refused over a borrowed base, repeatedly (CLOUD-1306). + /// + /// Like [`Bound::GateReclaims`] this has spent NO CI — `verify` refuses + /// before the matrix — so a caller reports it as a speculation that did not + /// pay off rather than as a verdict on the branch. **And that is the whole + /// reason it is its own bound rather than a lap charge**: the refusals it + /// counts are the ones nothing has yet attributed to this tree, so spending + /// the general lap budget on them would let a neighbour's poisoned base + /// exhaust an innocent branch and then report the exhaustion against it. + /// + /// The bound is SMALL by design. One lap off the borrowed base is the + /// discrimination — the same gate re-runs on trunk and answers with + /// [`Basis::Own`] — so a second and third refusal buy progressively less. The + /// cost of too few is abandoning a branch whose base merely churned; the cost + /// of too many is re-proving somebody else's defect on this agent's clock. + SpeculativeRefusals, } /// What a charge decided. @@ -1932,6 +2025,8 @@ pub struct Ledger { pub transients: u32, /// Passes whose gate was reclaimed because the base moved under it. pub gate_reclaims: u32, + /// Passes whose gate refused over a base borrowed from a lease holder. + pub speculative_refusals: u32, } impl Ledger { @@ -2008,6 +2103,30 @@ impl Ledger { } } + /// A pass whose gate refused over a borrowed base (CLOUD-1306). + /// + /// [`Ledger::reclaimed`]'s shape, and the refund is honest for the identical + /// reason: `verify` refuses before the matrix, so the pass bought no CI. What + /// differs is what the count MEANS. A reclaimed gate is a fact about the + /// trunk's pace; this is a refusal nothing has yet attributed to anybody, and + /// the next lap — replayed onto trunk rather than onto the holder — is the + /// experiment that attributes it. + /// + /// **Refunding the lap is what stops a neighbour's defect exhausting this + /// branch.** Charged to `laps`, a poisoned base would spend an innocent + /// branch's runaway backstop and the exhaustion would then be reported + /// against the branch, which is the misattribution this whole cell exists to + /// end — one bound lower down instead of one cell higher up. + pub const fn speculative_refusal(&mut self, max: u32) -> Charge { + self.laps = self.laps.saturating_sub(1); + self.speculative_refusals = self.speculative_refusals.saturating_add(1); + if self.speculative_refusals > max { + Charge::Stop(Bound::SpeculativeRefusals) + } else { + Charge::Lap + } + } + /// A run that failed before reaching a verdict. pub const fn transient(&mut self, max: u32) -> Charge { self.laps = self.laps.saturating_sub(1); @@ -2565,7 +2684,7 @@ pub fn rerun_failed(repo: &str, run: &str) -> bool { #[cfg(test)] mod lap_tests { - use super::{Progress, Step, progress}; + use super::{Basis, Progress, Step, progress, progress_of}; use crate::exit::ExitCode::{Internal, Success, Usage, Violation}; /// **The discriminating claim: a refusal a rebase would clear laps, and one @@ -2638,6 +2757,59 @@ mod lap_tests { assert_ne!(progress(Step::Verify, Violation), Progress::Lap); } + /// **THE UNQUALIFIED TABLE IS NOT THE WHOLE ANSWER ANY MORE** (CLOUD-1306). + /// + /// [`progress`] above is the base table and still stops, which is correct for + /// a branch on trunk. [`progress_of`] is the qualified reading the driver + /// actually takes, and over a borrowed base it laps — because the tree that + /// refused is this branch's commits replayed onto a LEASE HOLDER's unlanded + /// ones, so the refusal has two candidate causes and the exit code separates + /// neither. + /// + /// This is deliberately the same shape as the `Wait`/`Red` qualifier beside + /// it: the base table carries the general answer and the qualifier reaches + /// exactly one cell. Keeping the general answer as `Stop` is what makes the + /// premise case above still meaningful. + #[test] + fn a_refused_tree_over_a_borrowed_base_laps_instead_of_blaming_this_branch() { + assert_eq!( + progress_of(Step::Verify, Violation, None, Basis::Borrowed), + Progress::Lap, + "a refusal over somebody else's base is not yet this branch's verdict" + ); + } + + /// The premise for the case above: the qualifier must not swallow the cell. + /// + /// Without this, "stop attributing failures to the author" is implementable as + /// "never stop", which is the dead-gate class — a branch with a genuinely + /// broken tree would lap until its budget ran out and then report exhaustion + /// rather than the defect it actually has. + #[test] + fn a_refused_tree_on_its_own_base_still_stops_under_the_qualifier() { + assert_eq!( + progress_of(Step::Verify, Violation, None, Basis::Own), + Progress::Stop, + "on trunk the gate answers about THIS tree, and no rebase clears it" + ); + } + + /// The qualifier reaches ONE cell, and the neighbours prove it. + /// + /// `Basis` is read on every step, so a qualifier written one match arm too + /// wide would turn a refused replay or a refused ready into a lap — and both + /// of those are statements no rebase clears, exactly as before. + #[test] + fn the_borrowed_base_qualifier_does_not_reach_the_other_stops() { + for step in [Step::Replay, Step::Ready] { + assert_eq!( + progress_of(step, Violation, None, Basis::Borrowed), + progress(step, Violation), + "{step:?} must answer the same whoever's base this is" + ); + } + } + /// **The freshness probe fails OPEN, which is the opposite of every gate in /// this module and is the whole of its correctness.** /// @@ -3041,13 +3213,13 @@ mod tests { )]]; assert_eq!( - super::ready(&root, &gate, " \n "), + super::ready(&root, &gate, " \n ", &[]), super::Readied::Clear, "a body the fetch never produced says nothing, so there is nothing to judge" ); assert_eq!( - super::ready(&root, &gate, "Closes CLOUD-1"), + super::ready(&root, &gate, "Closes CLOUD-1", &[]), super::Readied::Unrunnable { gate: String::from("batten-no-such-program-for-the-ready-phase"), }, @@ -3077,7 +3249,7 @@ mod tests { ]]; assert_eq!( - super::ready(&root, &gate, "Closes CLOUD-1"), + super::ready(&root, &gate, "Closes CLOUD-1", &[]), super::Readied::Unrunnable { gate: String::from( "batten-no-such-runner-for-the-ready-phase run closing-key-check" @@ -3087,12 +3259,67 @@ mod tests { ); } + /// **THE BET REACHES A BODY GATE, AND FOR ITS WHOLE LIFE IT DID NOT** + /// (CLOUD-1770). + /// + /// `BATTEN_SPEC_BASE` is the boundary `claimed-keys` narrows the commit range + /// on. It was published into `verify`'s child environment alone, so + /// `closing-key-check` — a LATER step delegating to that same reader — saw no + /// bet and counted the lease holder's borrowed commits as keys this branch + /// had served and stranded. Measured over one session: two lease acquisitions + /// taken, spent on a 21–28 minute gate, and handed straight back. + /// + /// The gate here is `sh -c` over the variable, so its verdict is a fact about + /// the CHILD's environment rather than about this process's — a case reading + /// `std::env::var` would pass against the defect it exists to catch. + #[test] + fn a_body_gate_is_told_which_base_the_lap_borrowed() { + let root = std::env::temp_dir(); + let gate = vec![vec![ + String::from("sh"), + String::from("-c"), + String::from("test -n \"$BATTEN_SPEC_BASE\""), + ]]; + let published = vec![( + String::from(crate::speculation::PUBLISHED_AS), + Some(String::from("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")), + )]; + // A lap with NO bet, spelled as the removal it has to be. `&[]` would + // leave whatever the parent exported in place — and this suite runs + // inside a `land` gate that exports exactly this variable, which is how + // the first version of this case failed against its own subject. + let unpublished = vec![(String::from(crate::speculation::PUBLISHED_AS), None)]; + + assert_eq!( + super::ready(&root, &gate, "Closes CLOUD-1", &published), + super::Readied::Clear, + "a speculative lap must tell its body gates which base it borrowed" + ); + + // **THE MIRROR, and without it the case above passes on any environment + // that happens to carry the variable** — a developer's shell, an outer + // `land`, or a sibling test leaking one. That is not hypothetical: this + // suite runs as a child of the gate `mise run land` drives, which + // publishes this very variable, and the first version of this case read + // that outer bet and failed. + // + // So "no bet" is an explicit REMOVAL rather than an omission, and the + // mechanism now matches the claim: publication is a function of the bet. + assert!( + matches!( + super::ready(&root, &gate, "Closes CLOUD-1", &unpublished), + super::Readied::Refused { .. } + ), + "a lap carrying no bet must publish no base" + ); + } + /// No declared gates is a clear ready, and the distinction from `Unrunnable` /// is the optional-versus-dead one the driver's own header states. #[test] fn a_consumer_declaring_no_body_gates_is_clear_rather_than_unrunnable() { assert_eq!( - super::ready(&std::env::temp_dir(), &[], "Closes CLOUD-1"), + super::ready(&std::env::temp_dir(), &[], "Closes CLOUD-1", &[]), super::Readied::Clear ); } @@ -3170,6 +3397,45 @@ mod tests { assert_eq!(ledger.lease_waits, 1, "and charged to its own bound"); } + /// A speculative refusal is refunded and charged to its own bound + /// (CLOUD-1306). + /// + /// Fails by: charging `laps` instead, which is the misattribution this whole + /// cell exists to end, arriving one bound lower down — a neighbour's poisoned + /// base would spend an innocent branch's runaway backstop and the exhaustion + /// would then be reported against that branch. + #[test] + fn a_speculative_refusal_refunds_its_lap_and_charges_its_own_bound() { + let mut ledger = Ledger::default(); + ledger.attempt(); + assert_eq!(ledger.speculative_refusal(2), Charge::Lap); + assert_eq!(ledger.laps, 0, "the attempt was refunded"); + assert_eq!( + ledger.speculative_refusals, 1, + "and charged to its own bound" + ); + assert_eq!( + ledger.paid, 0, + "verify refuses before the matrix, which is what makes the refund honest" + ); + } + + /// The bound is real, so a base that keeps refusing cannot lap forever. + /// + /// The premise for lapping at all: without a bound, "do not blame the author" + /// becomes an unbounded token burn on somebody else's defect — which is the + /// cost CLOUD-1306 is about, reintroduced by the fix for it. + #[test] + fn enough_speculative_refusals_stop_under_their_own_bound() { + let mut ledger = Ledger::default(); + assert_eq!(ledger.speculative_refusal(1), Charge::Lap); + assert_eq!( + ledger.speculative_refusal(1), + Charge::Stop(Bound::SpeculativeRefusals), + "past the bound the laps are re-proving a neighbour's defect" + ); + } + /// **A RECLAIMED GATE IS THE SAME CLASS, AND THE RACE MADE IT COMMON** /// (CLOUD-1586). /// @@ -3509,22 +3775,30 @@ mod tests { use crate::exit::ExitCode::Violation; assert_eq!( - progress_of(Step::Wait, Violation, Some(TapVerdict::Red)), + progress_of(Step::Wait, Violation, Some(TapVerdict::Red), Basis::Own), Progress::Stop, "no rebase clears a failing test" ); assert_eq!( - progress_of(Step::Wait, Violation, None), + progress_of(Step::Wait, Violation, None, Basis::Own), Progress::Lap, "the staleness arm won the race, and the next replay is the remedy" ); } - /// The qualifier reaches ONE cell and leaves the table alone otherwise — - /// without this, a validator that stopped everything would satisfy the case + /// The qualifiers reach TWO cells and leave the table alone otherwise — + /// without this, a validator that stopped everything would satisfy the cases /// above. + /// + /// **`Basis::Own` IS THE WHOLE SWEEP, AND THAT IS THE STRONGEST THING THIS + /// CASE SAYS** (CLOUD-1306). A branch on trunk reads exactly as it did before + /// the borrowed-base qualifier existed, for every step, every code and every + /// reading — so the change cannot have moved a cell for the ordinary, + /// non-speculating landing. The one borrowed-base cell is asserted by its own + /// case rather than carved out of this sweep, which is what keeps the + /// exception legible instead of hidden in a predicate here. #[test] - fn the_reading_qualifies_the_wait_row_and_nothing_else() { + fn the_reading_qualifies_two_cells_and_nothing_else() { use crate::exit::ExitCode::{Internal, Success, Usage, Violation}; for step in [ @@ -3538,19 +3812,30 @@ mod tests { for code in [Success, Usage, Violation, Internal] { for seen in [None, Some(TapVerdict::Green), Some(TapVerdict::Pending)] { assert_eq!( - progress_of(step, code, seen), + progress_of(step, code, seen, Basis::Own), progress(step, code), - "{step:?}/{code:?}/{seen:?} must read as the table does" + "{step:?}/{code:?}/{seen:?} on its own base must read as the table does" ); } // And the red reading moves only the wait's own refusal. if !(step == Step::Wait && code == Violation) { assert_eq!( - progress_of(step, code, Some(TapVerdict::Red)), + progress_of(step, code, Some(TapVerdict::Red), Basis::Own), progress(step, code), "{step:?}/{code:?} is not the wait's refusal" ); } + // The borrowed base moves the gate's refusal and NOTHING else, so + // every other cell answers identically whoever's base it is. + if !(step == Step::Verify && code == Violation) { + for seen in [None, Some(TapVerdict::Green), Some(TapVerdict::Red)] { + assert_eq!( + progress_of(step, code, seen, Basis::Borrowed), + progress_of(step, code, seen, Basis::Own), + "{step:?}/{code:?}/{seen:?} is not the gate's refusal" + ); + } + } } } } diff --git a/crates/batten/src/lease.rs b/crates/batten/src/lease.rs index 5f69088ac..1e8daea1d 100644 --- a/crates/batten/src/lease.rs +++ b/crates/batten/src/lease.rs @@ -1091,14 +1091,102 @@ pub struct Body { pub head: String, /// The one admitted successor, or empty. Advisory. pub next: String, + /// A peer's request that the holder release, naming the peer. Advisory, and + /// the most advisory field here (CLOUD-1778). + /// + /// # Why a REQUEST rather than an eviction + /// + /// No failure detector is accurate in an asynchronous system, so every + /// suspicion that a holder is wedged is a heuristic and some of them are + /// wrong. A design that let a peer TAKE the lease on suspicion would be wrong + /// under exactly those cases. So this field carries no authority at all: the + /// holder reads it, and the holder decides. + /// + /// The unilateral half — a fenced compare-and-set that advances a token the + /// evicted holder's next write then fails — is the referee's and is not built + /// here. Both halves exist because they are sound for different things: a + /// request is cheap and correct when the holder is healthy but idle, and + /// fencing is the only thing that is safe when it is not. + /// + /// # IT IS NOT A STEAL WEARING A DIFFERENT NAME + /// + /// Honouring a notice RELEASES the lease; it does not hand it to the + /// requester, who must then win the next acquire like anybody else. That is + /// the property to protect, and it is the same sentence [`reservation`] + /// carries about the holder id: a mechanism by which naming yourself gets you + /// the lease is a steal however politely it is spelled. A waiter that could + /// evict-and-inherit would make "I suspect you are stuck" the fastest route to + /// the front of the queue. + /// + /// # AND IT IS HONOURED AT A LAP BOUNDARY, NEVER MID-MATRIX + /// + /// The beat keeps renewing while a notice is pending, which looks like + /// ignoring it and is the opposite. A holder that stopped renewing here would + /// lose the lease inside its own CI matrix, a second lander would start, and + /// that is precisely the overlap [`beat`] was written to close. The notice is + /// reported to the lap driver, which honours it where releasing costs nothing + /// — between laps, with no matrix in flight. + pub stand_down: String, /// The holder's own progress token. **Opaque by design**: a rival tests it for /// EQUALITY OVER TIME and never interprets it, so no clock crosses the wire. /// Advisory. pub progress: String, + /// The body format's MAJOR, and the one field a reader may refuse over. + /// + /// **"There is one administrator" is false here by construction**: the fleet + /// runs whatever versions its agents happen to carry, and nothing coordinates + /// an upgrade. So a body needs a way to say "you do not speak this", and a + /// reader needs a way to hear it that is not "parse it loosely and act on the + /// fields I recognised" — which is what the old parser did, and which turns a + /// format change into silent disagreement across a mixed fleet. + /// + /// **ABSENT MEANS 1, AND THAT IS THE COMPATIBILITY HINGE.** Every lease + /// written before this field existed carries no `schema:` line, and those + /// bodies are exactly schema 1 — so the default is a reading rather than a + /// guess. A reader seeing a major it does not know treats the lease as + /// [`Observed::Garbage`]: held, respected until it ages out, and diagnosable. + /// That is the same safe arm an unreadable lease already takes, and the + /// opposite of ignoring the field and racing the writer. + /// + /// **READERS SHIP BEFORE WRITERS**, which is why this lands emitting 1 rather + /// than 2. An old reader ignores an unknown key, so it would parse a `schema: + /// 2` body loosely and act on it — the exact failure this field prevents. The + /// major may only be raised once a release that REFUSES an unknown one is out + /// across the fleet. + pub schema: u32, + /// The batten version that minted this body. Advisory, and never a gate. + /// + /// Advertised so that version skew is VISIBLE rather than inferred from + /// behaviour. A waiter that sees a holder older than itself can say so, and a + /// stale agent can be asked to stand down by a peer — but the asking is + /// advisory and this field decides nothing, for [`Body`]'s stated reason: an + /// identity another clone could derive is one another clone could accidentally + /// claim. Refusing to honour a lease because its writer is old would hand + /// every upgrade a fleet-wide outage. + pub writer: String, /// What makes every mint a distinct object. See [`lease_object`]. pub nonce: String, } +/// The body major this build writes and is the highest it can read. +/// +/// One constant for both directions deliberately: a build that wrote a major it +/// could not read would be announcing a format to a fleet it is not itself part +/// of. +pub const BODY_SCHEMA: u32 = 1; + +/// This build's own version, as [`Body::writer`] advertises it. +/// +/// `CARGO_PKG_VERSION` here, and `spec.rs`'s argument against it does not reach +/// this use: that one refuses it for a SPEC version, where the number is a claim +/// about a document's content and must not move just because the crate shipped. +/// This is the opposite — the claim IS "which build wrote this", so tracking the +/// crate is the whole point. +#[must_use] +pub fn writer_version() -> String { + String::from(env!("CARGO_PKG_VERSION")) +} + /// The banner every lease body opens with, so a commit that is not a lease is not /// read as one. const BANNER: &str = "land-lock"; @@ -1109,9 +1197,24 @@ impl Body { pub fn render(&self) -> String { // `nonce:` STAYS LAST. Its uniqueness argument is what makes every mint a // distinct sha, and the check half treats it as the terminal line. + // + // `schema:` IS FIRST AFTER THE BANNER, so a reader that refuses an unknown + // major has seen it before it has read a single field it might act on. + // Ordering is not what makes the refusal correct — the parser reads the + // whole body before deciding — but a format whose version is buried in the + // middle invites a streaming reader that acts before it checks. format!( - "{BANNER}\nholder: {}\nexpires: {}\nbranch: {}\nhead: {}\nnext: {}\nprogress: {}\nnonce: {}\n", - self.holder, self.expires, self.branch, self.head, self.next, self.progress, self.nonce + "{BANNER}\nschema: {}\nholder: {}\nexpires: {}\nbranch: {}\nhead: {}\nnext: {}\nstand-down: {}\nprogress: {}\nwriter: {}\nnonce: {}\n", + self.schema, + self.holder, + self.expires, + self.branch, + self.head, + self.next, + self.stand_down, + self.progress, + self.writer, + self.nonce ) } @@ -1149,6 +1252,10 @@ pub fn parse_body(object: &[u8]) -> Option { let mut body = Body::default(); let mut banner = false; let mut expires = None; + // `Option>`: absent, present-and-unreadable, present-and-a-number. + // The three are different answers and collapsing the middle into either + // neighbour is the loose parse this field exists to refuse. + let mut schema: Option> = None; for line in text.lines() { if line == BANNER { banner = true; @@ -1164,6 +1271,7 @@ pub fn parse_body(object: &[u8]) -> Option { "branch" => body.branch.clear(), "head" => body.head.clear(), "next" => body.next.clear(), + "stand-down" => body.stand_down.clear(), "progress" => body.progress.clear(), _ => {} } @@ -1177,13 +1285,31 @@ pub fn parse_body(object: &[u8]) -> Option { "head" => value.clone_into(&mut body.head), "next" => value.clone_into(&mut body.next), "progress" => value.clone_into(&mut body.progress), + "writer" => value.clone_into(&mut body.writer), + "stand-down" => value.clone_into(&mut body.stand_down), "nonce" => value.clone_into(&mut body.nonce), + // PARSED PERMISSIVELY, REFUSED BELOW. A `schema:` line this reader + // cannot turn into a number is not schema 1 — it is a body written by + // something that does not agree with us about what the field even is, + // and reading it as the oldest format would be the loose parse this + // field exists to stop. `None` here falls into the unknown-major arm. + "schema" => schema = Some(value.parse::().ok()), _ => {} } } if !banner { return None; } + // ABSENT IS 1, UNREADABLE AND TOO-NEW ARE REFUSALS. Every body written before + // this field existed is schema 1, so the absent case is a reading rather than + // a default — and a major above what this build speaks yields `None`, which + // the caller renders as `Observed::Garbage`: still held, still respected until + // it ages out, and now diagnosable instead of silently misread. + body.schema = match schema { + None => BODY_SCHEMA, + Some(Some(major)) if major <= BODY_SCHEMA => major, + Some(_) => return None, + }; body.expires = expires?; Some(body) } @@ -1307,14 +1433,56 @@ pub fn authorises(observed: Option<&Observed>, want: &str, now: i64) -> Authorit /// Where the lease lives, and the bounds the design was pressure-tested into. /// -/// **`refs/heads`, and it is an environment limitation rather than a preference.** -/// A custom namespace is the better home — invisible to a remote branch listing, -/// untouched by a push of every ref, absent from base pickers, and the CAS behaves -/// identically there — but this sandbox proxies git and its write policy refuses -/// any push outside `refs/heads`. GitHub also does not enforce the fast-forward -/// rule off `refs/heads`: a parentless orphan was ACCEPTED on a custom namespace, -/// which is the whole safety property gone. Moving is a one-line change the day -/// the proxy allows it. +/// **`refs/heads`, and the reason is the forge's fast-forward rule — NOT a +/// namespace restriction.** This paragraph used to claim both, and the first half +/// was wrong for two years. +/// +/// # The namespace claim was a misdiagnosed credential failure (CLOUD-416) +/// +/// It asserted that this sandbox's git proxy permits writes under `refs/heads` +/// and rejects every other namespace. **Deliberately paraphrased rather than +/// quoted**: a refuted sentence left sitting here verbatim is one a future reader +/// lifts out of its correction, which is how it survived this long. +/// +/// Measured 2026-09-11 as a controlled pair — same object, same ref, same push, +/// one variable: +/// +/// ```text +/// fenced, PAT via credential helper: push → refs/sessions/ exit 0 +/// unfenced, ambient injected token: push → refs/sessions/ HTTP 403 +/// ``` +/// +/// `refs/batten-probe/` behaved identically. So the 403 is the injected +/// credential being unscoped for the write, and the namespace never came into it +/// — the symptom `mem:github-access` measures and this comment attributed to a +/// proxy write policy. CLOUD-416 records the lease being implemented four times, +/// "two of those passes existed only because the environment lied and nothing said +/// so"; this is the sentence that carried the lie forward. +/// +/// **A custom namespace is therefore available**, which unblocks the per-agent +/// refs and `refs/sessions/` transcripts rather than this one struct. +/// +/// # The fast-forward claim is the one that survived, and it is the real reason +/// +/// The same run confirmed the other half: a **parentless orphan was ACCEPTED** on +/// `refs/sessions/`, so the forge does not enforce the fast-forward rule off +/// `refs/heads`. That is what keeps the lease here. +/// +/// Read precisely, because the old text conflated two mechanisms. It is not what +/// makes the CAS work — `--force-with-lease` sends the expected old value and the +/// forge swaps atomically, in any namespace, and a lease RENEWAL is a non-fast- +/// forward update by design. What the rule protects is everything else that reads +/// this ref as a branch-shaped thing, and the blast radius of a bug that writes a +/// wrong object here. +/// +/// **And off `refs/heads` that same missing rule is a FEATURE, not a loss.** An +/// append-only log branch wants fast-forward enforcement and belongs here too; a +/// content-addressed transcript wants exactly the parentless orphan the rule would +/// refuse, and is structurally inert precisely because it can never become an +/// ancestor of trunk. One property, opposite signs, decided per structure — which +/// is why the old text's sweeping conclusion — that losing the rule forfeits the +/// safety property outright — was wrong to draw once about the lease and then +/// reuse everywhere. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Terms { /// The remote the lease lives on, as a URL the transport can reach. @@ -1882,7 +2050,14 @@ pub fn claim(terms: &Terms, holder: &str, branch: &str, head: &str, now: i64) -> branch: branch.to_owned(), head: head.to_owned(), next: String::new(), + // EMPTY ON ACQUISITION, which is what keeps a notice from outliving the + // holder it was aimed at. A fresh claim inheriting a stand-down would ask + // every future holder to release on the strength of one request made of + // somebody else. + stand_down: String::new(), progress: String::new(), + schema: BODY_SCHEMA, + writer: writer_version(), nonce: nonce(), } } @@ -1936,6 +2111,14 @@ pub fn lands_by_fast_forward(branch: &str, prefixes: &[String]) -> bool { pub fn tombstone(body: &Body) -> Body { Body { expires: 0, + // STAMPED, NEVER CARRIED. Every mint here is THIS build writing a body + // now, so advertising the version it inherited would name a writer that + // did not write it — and on `reservation`, which a WAITER mints onto the + // holder's body, the inherited value is a different agent's entirely. + // Carrying forward would make the one field that exists to expose skew + // the field that hides it. + schema: BODY_SCHEMA, + writer: writer_version(), nonce: nonce(), ..body.clone() } @@ -1954,6 +2137,8 @@ pub fn renewal(terms: &Terms, body: &Body, progress: Option<&str>, now: i64) -> Body { expires: now + terms.ttl, progress: progress.map_or_else(|| body.progress.clone(), str::to_owned), + schema: BODY_SCHEMA, + writer: writer_version(), nonce: nonce(), ..body.clone() } @@ -2000,22 +2185,57 @@ pub fn renewal(terms: &Terms, body: &Body, progress: Option<&str>, now: i64) -> /// [`crate::run_land_singleton`]'s heartbeat reads it from the task registry with /// [`progress_of`] — must pass it, and only a caller with nothing to say passes /// `None`. +/// What one beat found. +/// +/// **A richer answer than the `bool` this replaced, because the beat is the only +/// thing that reads the lease every few seconds** — so it is where a peer's +/// stand-down notice is seen, and a `bool` had nowhere to put it. Collapsing the +/// notice into `false` would have been worse than dropping it: `false` means "one +/// beat did not write", which the caller is explicitly told is not news. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Beat { + /// The lease was renewed. + Renewed, + /// Nothing was written, and it is not news — no identity, no lease, a lease + /// held by somebody else, a rejected CAS, a push that did not land. See + /// [`beat`] for why one of these is survivable by construction. + Quiet, + /// Renewed, AND a peer has asked this holder to release, naming itself. + /// + /// **Renewed as well, and the conjunction is the point.** Not renewing while a + /// notice is pending would drop the lease inside this holder's own CI matrix + /// and let a second lander start — the overlap [`beat`] exists to close, + /// reintroduced by the mechanism meant to be polite. The caller honours this + /// at a lap boundary, where releasing costs nothing. + StandDown(String), +} + #[must_use] -pub fn beat(root: &Path, terms: &Terms, progress: Option<&str>, now: i64) -> bool { +pub fn beat(root: &Path, terms: &Terms, progress: Option<&str>, now: i64) -> Beat { let Ok((_, holder)) = crate::lease_identity(root) else { - return false; + return Beat::Quiet; }; let Ok(observed) = observe(terms) else { - return false; + return Beat::Quiet; }; if !holds_now(&observed, &holder, now) { - return false; + return Beat::Quiet; } let Observed::Held { body, .. } = &observed else { - return false; + return Beat::Quiet; }; + // READ BEFORE THE RE-MINT, because the re-mint carries `stand_down` forward + // like every other advisory field and the reading must be of what the peer + // actually wrote rather than of our own copy of it. + let asked = stand_down_requested(body, &holder).map(str::to_owned); let renewed = renewal(terms, body, progress, now); - matches!(cas(terms, &observed, &renewed, now), Ok(Outcome::Applied)) + if !matches!(cas(terms, &observed, &renewed, now), Ok(Outcome::Applied)) { + return Beat::Quiet; + } + // THE RENEWAL FIRST, THE NOTICE SECOND. A holder that reported the notice + // without renewing would be honouring it immediately, which is exactly the + // mid-matrix release this ordering exists to prevent. + asked.map_or(Beat::Renewed, Beat::StandDown) } /// Fill the one successor slot, re-minting every other field verbatim. @@ -2035,11 +2255,55 @@ pub fn beat(root: &Path, terms: &Terms, progress: Option<&str>, now: i64) -> boo pub fn reservation(body: &Body, want: &str) -> Body { Body { next: want.to_owned(), + // THE WAITER'S VERSION, NOT THE HOLDER'S — see `tombstone`. This is the + // one mint where the writer is a DIFFERENT agent from the holder the body + // names, so carrying `writer` forward would attribute this write to the + // holder and make the skew field actively misleading. + schema: BODY_SCHEMA, + writer: writer_version(), + nonce: nonce(), + ..body.clone() + } +} + +/// A peer's request that the holder release, naming the peer (CLOUD-1778). +/// +/// [`reservation`]'s shape exactly — one field moved, everything else re-minted +/// verbatim — and for the same reasons. A notice that recomputed the holder id +/// would be a steal; one that recomputed the expiry would hand the holder a fresh +/// TTL for the trouble of being asked to leave. +/// +/// **`next` IS NOT SET HERE, AND THAT IS THE WHOLE SAFETY PROPERTY.** The +/// requester does not become the successor by asking. It releases the holder and +/// then races the next acquire like anybody else, because a route by which naming +/// yourself gets you the lease is a steal however politely it is spelled. A caller +/// that wants both must take both, separately and visibly. +#[must_use] +pub fn notice(body: &Body, from: &str) -> Body { + Body { + stand_down: from.to_owned(), + schema: BODY_SCHEMA, + writer: writer_version(), nonce: nonce(), ..body.clone() } } +/// Has a peer asked this holder to stand down? +/// +/// A pure reading over a body already in hand, so the beat pays no extra fetch +/// for it and a caller can ask the same question of a lease it observed for some +/// other reason. +/// +/// **A notice naming the holder itself is ignored**, which is not a courtesy: the +/// holder re-mints the whole body every beat, so a self-named notice would survive +/// its own release and ask every future holder to stand down forever. +#[must_use] +pub fn stand_down_requested<'body>(body: &'body Body, holder: &str) -> Option<&'body str> { + let from = body.stand_down.trim(); + (!from.is_empty() && from != holder).then_some(from) +} + /// Sixteen hex characters of entropy, which is what keeps every mint distinct. /// /// See [`lease_object`] for why a mint that agreed with another mint on every @@ -2238,6 +2502,58 @@ pub fn turn( Turn::Wait } +/// Does a poison cooldown still hold this clone back from taking the lease? +/// +/// **The condition is two EVENTS, never a duration** (CLOUD-1778). An agent whose +/// head turned CI red takes no further turn until either the trunk moves or the +/// pool goes idle — both observable, neither a timer, so the no-wall-clock +/// invariant survives a mechanism that would obviously have been written as one. +/// +/// # Why an agent that poisoned CI must not simply re-take +/// +/// Nothing stops it today. 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 — spending the metered resource twice to learn what it already +/// knows, while every waiter that prepped behind it sits through both. +/// +/// # Why the trunk moving lapses it +/// +/// The cooldown is a claim about ONE base: that this branch, on that trunk, is +/// red. When the trunk advances the claim is about a tree that no longer exists, +/// and holding the agent back further would punish it for a state nobody is in. +/// +/// # Why an idle pool lapses it, and this is the anti-outage half +/// +/// Waiting exists so somebody ELSE gets the slot. With no lease on the ref there +/// is nobody else, and a cooldown that survived an empty pool would strand a +/// single-agent fleet completely — one red matrix and nothing can ever land +/// again. `CLOUD-1043`'s releaser clause is the same argument for its own +/// mechanism, and it is preserved here rather than rediscovered. +/// +/// A released lease counts as idle for the same reason: the tombstone says the +/// holder handed it back, so the ref exists but nobody is holding it. +#[must_use] +pub fn cooling(poisoned_at: Option<&str>, trunk_now: &str, observed: &Observed) -> bool { + let Some(at) = poisoned_at else { + return false; + }; + // THE TRUNK MOVED — the claim was about a base that is no longer trunk. + if at != trunk_now { + return false; + } + // THE POOL IS IDLE — nobody is waiting, so waiting buys nothing and would + // strand a single-agent fleet. + match observed { + Observed::Absent => false, + Observed::Held { body, .. } => !body.released(), + // A ref nobody can read is NOT evidence the pool is idle, and reading it + // as idle would let the one agent that just poisoned CI take the lease on + // the strength of a failed parse. `turn` already answers `Wait` for this + // state; the cooldown agrees rather than arguing with it. + Observed::Garbage { .. } => true, + } +} + /// What the holder's own bookkeeping says about whether it is MOVING. /// /// **Liveness answers a different question, and answers it happily for a process @@ -3905,6 +4221,66 @@ mod tests { )); } + /// The poison cooldown lapses on either event and on neither timer + /// (CLOUD-1778). + /// + /// Fails by: making it a duration, which is the obvious implementation and + /// the one the no-wall-clock invariant forbids; or by dropping the idle arm, + /// which strands a single-agent fleet after one red matrix. + #[test] + fn a_poison_cooldown_lapses_when_the_trunk_moves_or_the_pool_idles() { + let held = Observed::Held { + sha: String::from("aaaa"), + body: Body { + holder: String::from("someone-else"), + expires: 1_700_000_060, + ..Body::default() + }, + }; + assert!( + cooling(Some("trunk-1"), "trunk-1", &held), + "same trunk and somebody is holding: the cooldown holds" + ); + assert!( + !cooling(Some("trunk-1"), "trunk-2", &held), + "the trunk moved, so the claim is about a base that is no longer trunk" + ); + assert!( + !cooling(Some("trunk-1"), "trunk-1", &Observed::Absent), + "nobody is waiting, so waiting buys nothing — and a surviving cooldown \ + would strand a single-agent fleet after one red matrix" + ); + assert!( + !cooling( + Some("trunk-1"), + "trunk-1", + &Observed::Held { + sha: String::from("bbbb"), + body: Body { + expires: 0, + ..Body::default() + }, + }, + ), + "a tombstone is the holder handing it back: the ref exists, nobody holds it" + ); + assert!( + !cooling(None, "trunk-1", &held), + "and an agent that poisoned nothing is never held back" + ); + assert!( + cooling( + Some("trunk-1"), + "trunk-1", + &Observed::Garbage { + sha: String::from("cccc"), + why: String::from("not a lease"), + }, + ), + "a ref nobody can read is not evidence the pool is idle" + ); + } + #[test] fn an_expired_lease_is_not_taken_until_its_sha_has_sat_a_beat() { // THE ONE EXTRA BEAT. Expiry is an instant on the HOLDER's clock; the @@ -4064,6 +4440,9 @@ mod tests { progress: String::from("100.200"), nonce: String::from("aaaaaaaaaaaaaaaa"), next: String::new(), + stand_down: String::new(), + schema: BODY_SCHEMA, + writer: String::from("0.0.1"), }; let reserved = reservation(&body, "claude/b"); assert_eq!(reserved.next, "claude/b"); @@ -4186,13 +4565,174 @@ mod tests { branch: String::from("claude/x"), head: String::from("2222222222222222222222222222222222222222"), next: String::from("claude/y"), + stand_down: String::from("host-2-bb"), progress: String::from("1700000000.1700000030"), + schema: BODY_SCHEMA, + writer: String::from("0.0.1"), nonce: String::from("deadbeefdeadbeef"), }; let object = lease_object(&body.render(), 1_700_000_000).expect("mint"); assert_eq!(parse_body(&object.body), Some(body)); } + /// **A NOTICE IS NOT A STEAL, and this is the case that says so.** + /// + /// The requester does not become the holder and does not become the + /// successor: it releases the holder and then races the next acquire like + /// anybody else. A mechanism by which naming yourself gets you the lease is a + /// steal however politely it is spelled, and "I suspect you are stuck" would + /// be the fastest route to the front of the queue. + #[test] + fn a_stand_down_notice_moves_one_field_and_grants_the_asker_nothing() { + let held = Body { + holder: String::from("host-1-aa"), + expires: 1_700_000_060, + branch: String::from("claude/a"), + next: String::new(), + ..Body::default() + }; + let asked = notice(&held, "host-2-bb"); + assert_eq!(asked.stand_down, "host-2-bb", "the peer names itself"); + assert_eq!( + asked.holder, "host-1-aa", + "and does NOT become the holder by asking" + ); + assert!( + asked.next.is_empty(), + "nor the admitted successor — that is a separate, visible act" + ); + assert_eq!( + asked.expires, held.expires, + "asking must not hand the holder a fresh TTL for the trouble" + ); + assert_eq!(asked.branch, held.branch); + } + + /// The holder reads it; a notice naming the holder itself does not. + /// + /// Self-named is ignored because the holder re-mints the whole body every + /// beat, so such a notice would survive its own release and ask every future + /// holder to stand down forever. + #[test] + fn a_notice_is_read_by_the_holder_and_a_self_named_one_is_not() { + let asked = Body { + stand_down: String::from("host-2-bb"), + ..Body::default() + }; + assert_eq!( + stand_down_requested(&asked, "host-1-aa"), + Some("host-2-bb"), + "a peer's request reaches the holder" + ); + assert_eq!( + stand_down_requested(&asked, "host-2-bb"), + None, + "a holder does not ask itself to leave, and a self-named notice would \ + outlive every release" + ); + assert_eq!( + stand_down_requested(&Body::default(), "host-1-aa"), + None, + "and no notice is no request" + ); + } + + /// **THE COMPATIBILITY HINGE, and the case the rollout turns on.** + /// + /// Every lease written before `schema:` existed carries no such line, and + /// those bodies ARE schema 1. A reader that refused them would stop the fleet + /// on the deploy that introduced the field; one that defaulted them to + /// anything else would mislabel every lease in flight. + #[test] + fn a_body_written_before_the_schema_field_reads_as_the_first_major() { + let object = lease_object( + "land-lock\nholder: host-1-aa\nexpires: 1700000060\nnonce: bb\n", + 1_700_000_000, + ) + .expect("mint"); + let body = parse_body(&object.body).expect("a pre-schema body still parses"); + assert_eq!( + body.schema, BODY_SCHEMA, + "an absent major is a reading, not a guess: these bodies are major 1" + ); + assert_eq!(body.holder, "host-1-aa", "and the rest still parses"); + assert!( + body.writer.is_empty(), + "a pre-schema body advertises no writer, and inventing one would \ + report skew that was never measured" + ); + } + + /// A major above what this build speaks does not parse, so the caller renders + /// it as `Observed::Garbage` — held, respected until it ages out, diagnosable. + /// + /// **This is the arm that makes the field worth having.** Without it the + /// parser ignores the unknown key, acts on the fields it recognised, and a + /// format change becomes silent disagreement across a mixed fleet — which is + /// the "one administrator" fallacy with a lock under it. + #[test] + fn a_body_from_a_newer_major_is_refused_rather_than_read_loosely() { + let object = lease_object( + "land-lock\nschema: 99\nholder: host-9-zz\nexpires: 1700000060\nnonce: bb\n", + 1_700_000_000, + ) + .expect("mint"); + assert_eq!( + parse_body(&object.body), + None, + "a body this build does not speak must not be acted on" + ); + } + + /// A `schema:` this reader cannot turn into a number is refused too. + /// + /// Reading it as the oldest format would be exactly the loose parse the field + /// exists to stop: a body whose version field is unintelligible was written by + /// something that does not agree with us about what the field is. + #[test] + fn an_unreadable_major_is_refused_rather_than_treated_as_the_oldest() { + let object = lease_object( + "land-lock\nschema: tomorrow\nholder: a\nexpires: 1700000060\nnonce: bb\n", + 1_700_000_000, + ) + .expect("mint"); + assert_eq!(parse_body(&object.body), None); + } + + /// Every mint stamps THIS build, and never inherits a predecessor's. + /// + /// `reservation` is the discriminating one: a WAITER mints it onto the + /// holder's body, so a carried `writer` would attribute the write to the + /// holder — making the one field that exists to expose skew the field that + /// hides it. The holder itself must still be carried, which is what separates + /// this from a steal. + #[test] + fn every_mint_advertises_the_build_that_wrote_it() { + let held = Body { + holder: String::from("host-1-aa"), + expires: 1_700_000_060, + writer: String::from("0.0.1"), + schema: BODY_SCHEMA, + ..Body::default() + }; + for minted in [ + reservation(&held, "claude/y"), + tombstone(&held), + renewal(&Terms::default(), &held, None, 1_700_000_000), + ] { + assert_eq!( + minted.writer, + writer_version(), + "a mint advertises the build that performed it" + ); + assert_eq!(minted.schema, BODY_SCHEMA); + assert_eq!( + minted.holder, "host-1-aa", + "and stamping the writer must not become a steal of the holder" + ); + } + } + #[test] fn a_commit_that_is_not_a_lease_does_not_parse_as_an_unheld_one() { // The same refusal the advertisement makes: a foreign object parsed diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 2232a6c43..5a01abecb 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -6742,7 +6742,7 @@ fn run_land( // The verdict is the LAP's to read; a hand-driven wait reports the code // and nothing else, exactly as it did before the tap needed one. cli::LandCommand::Wait { reference } => { - run_land_wait(root, reference, &branch, out, err).map(|(code, _)| code) + run_land_wait(root, reference, &branch, out, err).map(|(code, _, _)| code) } cli::LandCommand::Push => { let Some(url) = land_remote(root, err)? else { @@ -6820,8 +6820,25 @@ fn base_moved( fn charge_the_lap( step: land::Step, code: ExitCode, + basis: land::Basis, ledger: &mut land::Ledger, ) -> Option { + // THE GATE REFUSED OVER SOMEBODY ELSE'S BASE (CLOUD-1306). Charged to its own + // bound for `reclaimed`'s reason and one of its own: the pass bought no matrix + // — `verify` refuses before the ready — and the refusal is not yet attributed + // to anybody, so spending this branch's runaway backstop on it would let a + // neighbour's poisoned base exhaust an innocent branch and report the + // exhaustion against it. + // + // The pair identifies it without a second channel, exactly as the reclaim arm + // below does: `Violation` is the gate's own refusal, and `Basis::Borrowed` is + // the only reason `progress_of` turned that refusal into a lap at all. + if step == land::Step::Verify && code == ExitCode::Violation && basis == land::Basis::Borrowed { + return match ledger.speculative_refusal(speculative_refusal_bound()) { + land::Charge::Lap => None, + land::Charge::Stop(bound) => Some(bound), + }; + } // THE RECLAIMED GATE, and it is charged to its own bound for `waited`'s // reason (CLOUD-1586). `verify_raced` aborts the gate when the base moves, // so this lap bought nothing — no matrix, no completed gate, no push — and @@ -6844,6 +6861,27 @@ fn charge_the_lap( } } +/// How many gate refusals over a borrowed base a landing absorbs before it stops. +/// +/// **DELIBERATELY SMALL, and the two neighbours above are not the precedent.** +/// `lease_wait_bound` and `gate_reclaim_bound` default to 60 because what they +/// wait out is other branches landing, and the thing they are counting carries no +/// information. This one does: the very next lap replays onto trunk instead of +/// onto the holder, so the same gate re-runs with the borrowed base gone and +/// answers with [`land::Basis::Own`] — which either stops the loop with real +/// evidence or clears it outright. +/// +/// One lap is therefore the experiment, and the default buys a second in case the +/// first also drew a holder. Past that the laps are re-proving somebody else's +/// defect on this agent's clock, which is the token burn CLOUD-1306 is about. +fn speculative_refusal_bound() -> u32 { + std::env::var("LAND_MAX_SPECULATIVE_REFUSALS") + .ok() + .and_then(|raw| raw.trim().parse::().ok()) + .filter(|refusals| *refusals > 0) + .unwrap_or(2) +} + /// How many reclaimed gates a landing absorbs before it stops. /// /// **`lease_wait_bound`'s default, on `lease_wait_bound`'s reasoning.** A lap @@ -7248,7 +7286,14 @@ fn run_land_lap( // deferred. `pipeline::unwind` already dedupes, so a step entered twice // across two laps is still compensated once. let mut entered: Vec = Vec::new(); + // Who asked this holder to stand down, latched across laps — see + // `honour_the_notice` for why it is a latch and why it is read here. + let mut stood_down: Option = None; 'laps: for lap in 1..=laps { + // Checked BEFORE `attempt`, so a lap that will not run is not charged. + if let Some(from) = stood_down.take() { + return honour_the_notice(&from, root, branch, &pipeline, &mut entered, out, err); + } ledger.attempt(); writeln!(out, "land: lap {lap} of {laps}")?; // WHAT THE WAIT SAW, or `None` where no lap took a reading. The tap @@ -7270,11 +7315,9 @@ fn run_land_lap( let step = row.step; // THE ROW'S OWN QUESTION, where the driver used to carry a // `step == Verify` exception. A pre-check runs BEFORE the primitive - // and can only lap, never land: it exists to spend nothing. - // THE BET IS SETTLED BEFORE ANYTHING IS SPENT, and before the - // replay that would otherwise build on somebody else's commits. - // THE TWO ROWS THAT CAN END A LAP BEFORE IT SPENDS, asked together - // because they share an unwind and differ only in what follows it. + // and can only lap, never land: it exists to spend nothing, and it + // settles the bet before the replay that would otherwise build on + // somebody else's commits. let asked = asks_before_the_step(*row, this_lap, &mut trunk_poll, &mut bet, out, err)?; if let Answered::Stop(code) = asked { unwind_lap(root, branch, &pipeline, &mut entered, seen, out, err)?; @@ -7285,30 +7328,15 @@ fn run_land_lap( continue 'laps; } // THE PHASE, PUSHED BEFORE THE STEP RUNS RATHER THAN AFTER IT. A step - // is what this lap is doing WHILE it blocks, and the whole reason a - // reader wants it is that a gate can hold for minutes — so announcing - // it on completion would name every phase exactly when it stopped - // being true. `land.sh` pushed at the transition for the same reason. + // is what this lap is doing WHILE it blocks, and a gate can hold for + // minutes — so announcing it on completion would name every phase + // exactly when it stopped being true. guard.phase(step.as_str(), lap); - let code = match step { - // NO RESOLUTIONS ON THE LAP, ever. A lap runs unattended, so a - // resolution it could apply would be one nobody looked at — - // `gitwrite`'s auto-resolution refusal, reached through the - // driver instead of through a flag. - land::Step::Replay => run_land_replay(root, url, reference, branch, &[], out)?, - land::Step::Verify => { - run_land_verify(root, &bet, branch, Some(reference), out, err)? - } - land::Step::Lease => run_land_lease(root, branch, out, err)?, - land::Step::Ready => run_land_ready(root, branch, &mut ledger, out, err)?, - land::Step::Push => run_land_push(root, url, branch, out)?, - land::Step::Wait => { - let (code, verdict) = run_land_wait(root, reference, branch, out, err)?; - seen = verdict; - code - } - land::Step::FastForward => run_land_fast_forward(root, branch, out, err)?, - }; + let (code, verdict, asked) = run_the_step(step, this_lap, &bet, &mut ledger, out, err)?; + // The wait is the only step with readings to carry, and both are + // latched rather than acted on here — see `honour_the_notice`. + seen = verdict.or(seen); + stood_down = stood_down.or(asked); // ENTERED ON SUCCESS, OR ON THE ATTEMPT WHERE THE UNDO SAYS SO. The // first half is the discrimination the undo rests on: a `Ready` that // REFUSED bought no matrix, so re-drafting over it would draft a pull @@ -7323,7 +7351,12 @@ fn run_land_lap( entered.push(step); } note_the_push(step, code, &mut bet); - match land::progress_of(step, code, seen) { + // READ ONCE, HERE, BEFORE ANY ARM BELOW CAN MOVE IT: `unwind_lap` can + // settle or forget the bet, so a later read would ask about a + // different world than the one that answered (CLOUD-1306). + let basis = land::Basis::of(bet.live()); + note_the_poison(step, code, basis, &mut bet); + match land::progress_of(step, code, seen, basis) { land::Progress::Proceed => {} // `None` is not merged, or nobody could say. Either way this is // a lap rather than a retirement — see `landed_for_real`. @@ -7355,7 +7388,7 @@ fn run_land_lap( // `refusal` rather than shadowing `code`: the STEP's code is // what tells a reclaimed gate from a lease wait, so the // charge needs it and a shadow would hide it (CLOUD-1586). - if let Some(refusal) = charge_or_refuse(step, code, &mut ledger, err)? { + if let Some(refusal) = charge_or_refuse(step, code, basis, &mut ledger, err)? { unwind_lap(root, branch, &pipeline, &mut entered, seen, out, err)?; return Ok(refusal); } @@ -7804,10 +7837,11 @@ fn bet_liveness(root: &Path, branch: &str, base: &str) -> speculation::Live { fn charge_or_refuse( step: land::Step, code: ExitCode, + basis: land::Basis, ledger: &mut land::Ledger, err: &mut dyn Write, ) -> Result> { - let Some(bound) = charge_the_lap(step, code, ledger) else { + let Some(bound) = charge_the_lap(step, code, basis, ledger) else { return Ok(None); }; // NO CATCH-ALL, and clippy is what insisted: the match is total inside this @@ -7826,6 +7860,13 @@ fn charge_or_refuse( land::Bound::Transients => { "CI kept failing before reaching a verdict, so the provisioning path is broken rather than flaky" } + // NAMES THE BORROWED BASE AS THE SUSPECT, and says what to do about it. + // The whole point of lapping here was to stop attributing a neighbour's + // refusal to this author; a diagnosis that said "the gate refused" would + // give the attribution back in the last sentence. + land::Bound::SpeculativeRefusals => { + "the gate kept refusing over a base borrowed from the branch ahead, and never over this branch on trunk — so the evidence points at that base rather than at this tree; re-run once the holder has landed or been evicted" + } }; writeln!( err, @@ -7834,6 +7875,86 @@ fn charge_or_refuse( Ok(Some(ExitCode::Internal)) } +/// Run one step of the lap, and hand back what only the wait produces. +/// +/// Split out of [`run_land_lap`] on this tree's own precedent — `trust.rs` and +/// `cli.rs` both record splitting rather than suppressing `too_many_lines`, and +/// the driver is the function that grows every time a step learns something new. +/// +/// The two `Option`s are the wait's alone and are `None` for every other step: +/// the tap's verdict, which the exit code cannot carry because a stale base and +/// an unanswered wait are both a lap, and a peer's stand-down notice, which is +/// latched by the caller and honoured at a lap boundary. +fn run_the_step( + step: land::Step, + asked: Asked<'_>, + bet: &speculation::Bet, + ledger: &mut land::Ledger, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result<(ExitCode, Option, Option)> { + let Asked { + root, + url, + reference, + branch, + .. + } = asked; + let code = match step { + // NO RESOLUTIONS ON THE LAP, ever. A lap runs unattended, so a resolution + // it could apply would be one nobody looked at — `gitwrite`'s + // auto-resolution refusal, reached through the driver rather than a flag. + land::Step::Replay => run_land_replay(root, url, reference, branch, &[], out)?, + land::Step::Verify => run_land_verify(root, bet, branch, Some(reference), out, err)?, + land::Step::Lease => run_land_lease(root, branch, out, err)?, + land::Step::Ready => run_land_ready(root, branch, bet, ledger, out, err)?, + land::Step::Push => run_land_push(root, url, branch, out)?, + land::Step::Wait => return run_land_wait(root, reference, branch, out, err), + land::Step::FastForward => run_land_fast_forward(root, branch, out, err)?, + }; + Ok((code, None, None)) +} + +/// Release the lease because a peer asked, and say who asked (CLOUD-1778). +/// +/// **THE LAP BOUNDARY IS WHERE A NOTICE IS HONOURED, and it is the only place it +/// can be honoured cheaply.** Between laps there is no matrix in flight and no +/// ready pull request this lap created, so releasing costs the fleet nothing and +/// costs this branch one re-run. A holder that stood down mid-wait would abandon a +/// matrix it had already paid for AND let a second lander start — the overlap the +/// heartbeat exists to close, reintroduced by the mechanism meant to be polite. +/// +/// **EXIT 3, NEVER 2.** Standing down is not a verdict about this branch: nothing +/// about the tree is wrong and the head is still landable. `3` is the same reading +/// a spent lap budget gets — the loop stopped asking — and the caller runs it +/// again. A `2` here would tell an agent to go and fix a defect that does not +/// exist. +/// +/// # The caller's latch, and why it is one +/// +/// The request is made inside a wait and honoured at the NEXT lap's boundary, so +/// the driver holds it in a binding declared outside the loop — a per-lap one +/// would drop it in the gap between the two, which is the whole distance it has +/// to travel. And a latch rather than a live re-read, because the beat re-mints +/// the body every few seconds and a later writer can overwrite the field: once +/// asked, this holder has been asked. +fn honour_the_notice( + from: &str, + root: &Path, + branch: &str, + pipeline: &pipeline::Pipeline, + entered: &mut Vec, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result { + writeln!( + out, + "land: {from} asked this holder to stand down; releasing at the lap boundary rather than mid-matrix" + )?; + unwind_lap(root, branch, pipeline, entered, None, out, err)?; + Ok(ExitCode::Internal) +} + /// Record that the speculative range reached the remote. /// /// **THE ONE WRITER OF [`speculation::Bet::pushed`], which had none** (review of @@ -7851,6 +7972,41 @@ fn note_the_push(step: land::Step, code: ExitCode, bet: &mut speculation::Bet) { } } +/// Record that the gate refused over the base this lap borrowed (CLOUD-1306). +/// +/// **THE WRITER WITHOUT WHICH THE LAP IS A SPIN.** `land::Basis::Borrowed` turns +/// a refused gate into a lap rather than a stop, on the argument that the next +/// lap re-runs the same gate with the borrowed base gone. That argument is only +/// true if the next lap declines to borrow the same base — and `unwind_lap` calls +/// `Bet::forget`, which clears `base`, and `would_rebet` compares `base`. So +/// without this line the next lap bets on the identical holder, the gate refuses +/// for the identical reason, and the loop proves nothing until its bound stops +/// it. `speculation.rs` calls that arm "CLOUD-1306's other half". +/// +/// **Written BEFORE the progress arms run, beside `note_the_push` and for its +/// reason**: the lap arm's `unwind_lap` settles or forgets the bet, so a writer +/// downstream of it would be recording against a bet that no longer names the +/// base that failed. `Bet::forget` deliberately preserves this field, exactly as +/// it preserves `conflicts`. +/// +/// Keyed on the pair the same way the charge is: `Violation` is the gate's own +/// refusal (`Internal` is `Refusal::Moved`, which is the base moving and says +/// nothing about the base's tree), and `Basis::Borrowed` is what makes the +/// refusal somebody else's candidate rather than this branch's verdict. +fn note_the_poison( + step: land::Step, + code: ExitCode, + basis: land::Basis, + bet: &mut speculation::Bet, +) { + if step == land::Step::Verify && code == ExitCode::Violation && basis == land::Basis::Borrowed { + // The BORROWED base, never the head: the head is this branch's replayed + // commits and is minted fresh every lap, so recording it would name + // something no later lap can recognise. + bet.poisoned = bet.published().map(str::to_owned); + } +} + /// Refresh the trunk's remote-tracking ref before a speculation reads it. /// /// **EVERY SPECULATION READ OF THE TRUNK WAS ONE LAP BEHIND** (CLOUD-1620's @@ -8025,6 +8181,14 @@ fn place_the_bet( // second guard's own comment reasons that "`run_land_replay` immediately // fetches a fresh trunk", which it does — one step AFTER this runs. advance_trunk(root, reference); + // THIS LANDING ALREADY DECLINED TO PUBLISH ONE (CLOUD-1681). The `Push` + // precheck unwound a bet that reached the publish, so betting again buys + // nothing before this branch lands and costs another unwind at the same row. + // Checked FIRST, because it is a fact about us rather than about the holder + // and no reading below can change it. + if bet.declined { + return Ok(()); + } let tracking = land::tracking_ref(reference); // THE HOLDER ALREADY LANDED. Their head is on the trunk, so an ordinary replay // reaches it and a bet would borrow a range that is not borrowed. @@ -9242,6 +9406,7 @@ fn trunk_watch(reference: &str, base: &str, repo: &str, interval: u64) -> main_w fn run_land_ready( root: &Path, branch: &str, + bet: &speculation::Bet, ledger: &mut land::Ledger, out: &mut dyn Write, err: &mut dyn Write, @@ -9265,12 +9430,30 @@ fn run_land_ready( // `Drop`: this string is PARSED as the body, so a client's notice // on stderr would become text the author never wrote. The gates // below take `Keep`, because their stderr IS their reason. - .and_then(|argv| exec::piped_argv(root, argv, "", exec::Diagnostics::Drop)) + // NO BET PUBLISHED TO THE FETCH, deliberately. This call reads the + // pull request's BODY; the bet is a fact about the commit RANGE, and + // handing it to a client that does not read one would be a variable + // in an environment for no reader. + .and_then(|argv| exec::piped_argv(root, argv, "", exec::Diagnostics::Drop, &[])) .filter(|(code, _)| *code == 0) .map(|(_, body)| body) .unwrap_or_default(); - match land::ready(root, &gates, &body) { + // THE SAME PUBLICATION `run_land_verify` MAKES, and for the same reason + // one step later (CLOUD-1770). `Bet::published` is `None` with no bet + // outstanding, so the variable is simply absent from a non-speculative + // lap's gates — the publication stays a function of the bet rather than a + // side effect kept in step with it. + // ONE ENTRY ALWAYS, and its VALUE is the bet. An empty list would leave + // an inherited `BATTEN_SPEC_BASE` in place — and this lap's own gates run + // as children of a `verify` that exports it, so "no bet" has to be said + // out loud rather than left unsaid (measured: the case for this asserted + // the mirror and read an outer process's bet). + let published: Vec<(String, Option)> = vec![( + speculation::PUBLISHED_AS.to_owned(), + bet.published().map(str::to_owned), + )]; + match land::ready(root, &gates, &body, &published) { land::Readied::Clear => { writeln!(out, "land: {} body gate(s) clear", gates.len())?; } @@ -9657,10 +9840,10 @@ fn run_land_wait( branch: &str, out: &mut dyn Write, err: &mut dyn Write, -) -> Result<(ExitCode, Option)> { +) -> Result<(ExitCode, Option, Option)> { let Ok(sha) = git::head_commit(root) else { writeln!(err, "::error:: land: cannot read this clone's HEAD")?; - return Ok((ExitCode::Internal, None)); + return Ok((ExitCode::Internal, None, None)); }; let required = std::env::var("CI_REQUIRED_CHECKS").unwrap_or_default(); let roster = checks_green::Roster { @@ -9679,7 +9862,7 @@ fn run_land_wait( // would be a hang whose cause is a typo. if let Err(problem) = checks_green::decide(&[], &roster) { writeln!(err, "::error:: land wait: {problem}")?; - return Ok((ExitCode::Usage, None)); + return Ok((ExitCode::Usage, None, None)); } // The base as this clone last saw it. The wait is asking whether the REMOTE @@ -9703,7 +9886,7 @@ fn run_land_wait( err, "::error:: land wait: {tracking} will not resolve, so this wait has no base to judge staleness against and would race with one arm" )?; - return Ok((ExitCode::Internal, None)); + return Ok((ExitCode::Internal, None, None)); }; let config = pr_watch::Config { @@ -9724,7 +9907,7 @@ fn run_land_wait( err, "::error:: land wait: no repository resolved, so every check-run read would 404 — set $GH_REPO, or run this in a clone whose remote names one" )?; - return Ok((ExitCode::Usage, None)); + return Ok((ExitCode::Usage, None, None)); } // A COUNT, never a deadline (CLOUD-1177). The default is generous because // the cost of too many asks is a few conditional requests the forge answers @@ -9764,8 +9947,39 @@ fn run_land_wait( land::Waited::Unanswered => (land::answers(&sha, None, None), ExitCode::Internal), }; land::record_wait(root, branch, &answers)?; + say_what_the_wait_saw(&waited, &sha, reference, asks, out, err)?; + // THE READING TRAVELS WITH THE CODE, because the exit table cannot carry it: + // a stale base and an unanswered wait are both a lap, and only one of them + // took a checks reading at all. Deriving the tap's verdict from the code in + // the driver would be a second authority over an answer this function holds. + // + // AND SO DOES A PEER'S NOTICE, for the same reason and one of its own: the + // heartbeat is scoped to THIS wait, so a request latched inside it dies here + // unless it is handed back. The driver honours it at the lap boundary, which + // is the only place releasing costs nothing. + Ok(( + code, + land::tap_verdict(&waited), + holding.was_asked_to_stand_down(), + )) +} - match &waited { +/// What the wait saw, said once, in the channel each reading belongs in. +/// +/// Split out of [`run_land_wait`] because the two do different jobs: that one +/// decides and records, this one reports. They were one function until the +/// reporting arms grew past the line ceiling, and the ceiling was right — a +/// decision and its narration are separable, and separating them is what keeps a +/// new arm from being added to the wrong one. +fn say_what_the_wait_saw( + waited: &land::Waited, + sha: &str, + reference: &str, + asks: u32, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result<()> { + match waited { land::Waited::Green { .. } => { writeln!(out, "land: {sha} is green; the loser was voided unread")?; } @@ -9807,11 +10021,7 @@ fn run_land_wait( writeln!(out, "land: no answer yet on {sha} after {asks} ask(s)")?; } } - // THE READING TRAVELS WITH THE CODE, because the exit table cannot carry it: - // a stale base and an unanswered wait are both a lap, and only one of them - // took a checks reading at all. Deriving the tap's verdict from the code in - // the driver would be a second authority over an answer this function holds. - Ok((code, land::tap_verdict(&waited))) + Ok(()) } /// The lap's landing-lease heartbeat, paced by the terms it read once. @@ -9857,6 +10067,18 @@ struct Heartbeat<'clone> { git_dir: Option, pid: u32, last: std::sync::atomic::AtomicI64, + /// Set once a peer has asked this holder to stand down, naming the peer. + /// + /// **A LATCH, not a level.** The beat re-mints the body every few seconds and + /// a peer's notice could be overwritten by a later writer, so a caller that + /// re-read the live value at the lap boundary could miss a request that was + /// made and then lost. Once asked, this holder has been asked. + /// + /// Written from inside the wait's poll and read at the lap boundary, which is + /// why it is a shared cell rather than a field on the lap: the two live in + /// different places and the whole point is that the request crosses between + /// them. + stood_down: std::sync::Mutex>, } impl<'clone> Heartbeat<'clone> { @@ -9867,9 +10089,21 @@ impl<'clone> Heartbeat<'clone> { git_dir: crate::git::git_dir(root).ok(), pid: std::process::id(), last: std::sync::atomic::AtomicI64::new(0), + stood_down: std::sync::Mutex::new(None), } } + /// Who asked this holder to stand down, if anybody has. + /// + /// A poisoned lock reads as "nobody asked", which is the fail-open this whole + /// struct takes: the notice is advisory, so losing one costs a holder that + /// keeps its lease until TTL or completion — the behaviour before the notice + /// existed. Failing closed here would let a panic anywhere in the poll release + /// a healthy lease. + fn was_asked_to_stand_down(&self) -> Option { + self.stood_down.lock().ok()?.clone() + } + /// This lap's own progress token, or `None` when the registry has nothing to /// read — which [`lease::progress_of`] documents as the honest answer for a /// lap whose bookkeeping never registered, not as evidence of a stall. @@ -9916,9 +10150,16 @@ impl<'clone> Heartbeat<'clone> { ); } let progress = self.progress(); - // Bound rather than dropped: `beat` answers a `bool`, and dropping a - // `Copy` is a lint of its own. - let _renewed = lease::beat(self.root, terms, progress.as_deref(), now); + // THE ONE READER OF A PEER'S NOTICE, because the beat is the only thing + // that looks at the lease every few seconds. Latched here and honoured at + // the lap boundary — never here, where a release would drop the lease + // inside this holder's own matrix and let a second lander start. + if let lease::Beat::StandDown(from) = + lease::beat(self.root, terms, progress.as_deref(), now) + && let Ok(mut asked) = self.stood_down.lock() + { + asked.get_or_insert(from); + } } } @@ -10010,6 +10251,34 @@ fn asks_before_the_step( )?; Ok(Answered::Stop(exit::ExitCode::Violation)) } + // A LIVE BET NEVER REACHES THE PUBLISH (CLOUD-1681). + // + // The unwind is done HERE rather than left to the lap's compensations, + // and that is the arm's correctness rather than its convenience: no + // `Compensation` drops a bet — `Nothing`, `Redraft`, `Abandon`, + // `ReleaseLease` — so answering `Lap` alone would carry the borrowed + // range straight into the next lap and back to this row. + // + // `unwind_the_bet` is the same drop `Settle::Lost` already takes, so + // this is a second caller for an existing arm rather than new machinery. + Some(pipeline::Precheck::BetLive) if bet.live() => { + writeln!( + out, + "land: a speculation is still outstanding, so this head is not publishable — unwinding and lapping onto real trunk" + )?; + if let Some(code) = unwind_the_bet(root, url, branch, bet, reference, out, err)? { + // The unwind itself refused — a tree it could not rewind is the + // one thing that stops rather than laps, because carrying on + // would push another branch's commits under this one. + return Ok(Answered::Stop(code)); + } + // AND WE DO NOT BET AGAIN THIS LANDING, which is what makes the lap + // terminate. `place_the_bet`'s own guards are about the HOLDER, so + // without this the next lap re-bets the same one, arrives back here, + // and unwinds again until the lap budget is spent. + bet.declined = true; + Ok(Answered::Lap) + } _ => Ok(Answered::Go), } } @@ -10803,7 +11072,7 @@ fn note_release(root: &Path) { let Some(argv) = land::body_gates(&declared).into_iter().next() else { return; }; - let _ = exec::piped_argv(root, &argv, "", exec::Diagnostics::Keep); + let _ = exec::piped_argv(root, &argv, "", exec::Diagnostics::Keep, &[]); } /// `lease release`: a tombstone, never a delete. @@ -11320,7 +11589,14 @@ fn commit_admissions( overrides: &Overrides, ) -> Result> { let root = Path::new("."); - let protected = resolve::resolve(root, overrides)?.protected.clone(); + // ONE resolve for both tables. `protected` answers "is this guarded" and + // `redirects` answers "what should they run instead" — deliberately + // independent sets (`redirect.rs`), and the articulation clause is the one + // reader that needs both: it demands a block from the first set minus the + // second (CLOUD-1303). + let resolved = resolve::resolve(root, overrides)?; + let protected = resolved.protected.clone(); + let redirects = resolved.redirects.clone(); match (range, message) { (Some(range), None) => { // Already validated above; re-split rather than threaded, because a @@ -11329,9 +11605,10 @@ fn commit_admissions( let Some((base, head)) = range.split_once("..") else { return Ok(Vec::new()); }; - Ok(commit::judge_admissions(&git::writes_in_range( - root, base, head, &protected, - )?)) + Ok(commit::judge_admissions( + &git::writes_in_range(root, base, head, &protected)?, + &redirects, + )) } (None, Some(message)) => { let body = std::fs::read_to_string(message).map_err(|error| { @@ -11350,7 +11627,7 @@ fn commit_admissions( .into_iter() .filter(|path| selectors.iter().any(|selector| selector.matches(path))) .collect(); - Ok(commit::judge_pending(&body, &staged)) + Ok(commit::judge_pending(&body, &staged, &redirects)) } // Both modes and neither are refused above, before this is reached. _ => Ok(Vec::new()), @@ -12113,16 +12390,10 @@ fn run_hook( out: &mut dyn Write, err: &mut dyn Write, ) -> Result { - let mut raw = String::new(); - if std::io::stdin().read_to_string(&mut raw).is_err() { - output::message(mode, Verbosity::Normal, err, UNREADABLE_STDIN)?; - return Ok(ExitCode::Success); - } - let bypass = std::env::var_os(hook::BYPASS_ENV).is_some_and(|value| !value.is_empty()); - let Some(mut envelope) = hook::decode(harness, &raw) else { - output::message(mode, Verbosity::Normal, err, UNDECODABLE_PAYLOAD)?; + let Some((raw, mut envelope)) = read_envelope(harness, mode, err)? else { return Ok(ExitCode::Success); }; + let bypass = std::env::var_os(hook::BYPASS_ENV).is_some_and(|value| !value.is_empty()); // THE WRITE TARGET IS READ AS THE REPOSITORY READS IT (CLOUD-1133), and this // is the one place that can do it: `decode` is pure and has no repository, // and the readers below — the protected gate, and any module over @@ -12140,8 +12411,7 @@ fn run_hook( // The note rides the ladder above `normal`, because on the hosts where this // is reachable it is the ordinary state rather than news. let capabilities = harness.capabilities(); - if !capabilities.emits(envelope.event) && envelope.event != hook::Event::Unrecognized { - let note = unsupported_event_note(harness, &capabilities, envelope.event); + if let Some(note) = undeclared_event_note(harness, &capabilities, envelope.event) { output::message(mode, Verbosity::Verbose, err, ¬e)?; return Ok(ExitCode::Success); } @@ -12251,18 +12521,7 @@ fn run_hook( // end-of-turn surface. The retired shell hook this replaces paid ~330-440ms // at the same boundary; `perf`'s `passthrough` and `noop` arms are pre-tool // shapes and are untouched by this clause. - let adjudicable = !envelope.command.is_empty() - || envelope.writes.is_some() - || envelope.event == hook::Event::Stop - // A FOURTH TIME, and for a mint rather than a verdict (CLOUD-856). Session - // start carries no command, no write and no tool name, so this predicate - // was false there and config was never loaded — which means the receipt - // this event exists to mint could not know which manifests were declared. - // The cost is one config load per SESSION, not per call, which is the - // same trade the `Stop` clause above makes, and it buys the whole reason - // `Fact::Document` can stay `None` on the mediated path. - || envelope.event == hook::Event::SessionStart - || (envelope.event == hook::Event::PreTool && !envelope.raw_tool.is_empty()); + let adjudicable = is_adjudicable(&envelope); // A BYPASSED CALL NOW PAYS THE CONFIG READ, and that invariant is retired // deliberately rather than eroded. // @@ -12281,8 +12540,39 @@ fn run_hook( // arms, ~0.7 ms against a 100 ms budget. `!adjudicable` keeps its old // behaviour, because an event with nothing to adjudicate has no protected // gate to run either, and that is the arm the hot path actually rides. + // A DECLARATION NOTHING COULD READ IS THE ONE FAULT THAT REFUSES, and the + // narrowness is the decision rather than caution (CLOUD-1677). + // + // Gates are registered fail-open, so a gate that fails open is INERT — it + // neither allows nor denies, it is absent. A config fault is therefore never + // a choice between refusing and allowing: it is a choice between keeping the + // enforcement surface we still have and losing it entirely. An unknown key, a + // version this build is too old for, a table whose validator refused — each + // leaves every other row readable and enforceable, and leaves an agent that + // can still be TOLD to repair the broken one. Refusing there would trade a + // working partial surface for nothing. + // + // `Native::ConfigUnreadable` is the one class with no partial function left: + // the file is not TOML, so no row is readable and none can be enforced. Then + // the refusal is the only signal available, and the hatch below is how the + // container gets back. + //MUTANT-SUITE crates/batten/tests/it/adjudicate_absent.rs + //MUTANT unloadable-config-allows|s@ Err(unreadable) if unreadable_declaration(\&unreadable) => {@ Err(unreadable) if false \&\& unreadable_declaration(\&unreadable) => {@|a_config_this_build_cannot_load_denies_rather_than_failing_open let (policy, waivers) = if adjudicable { - load_policy(overrides, harness)? + match load_policy(overrides, harness) { + Ok(loaded) => loaded, + // The declared hatch, read before the refusal so a stale binary + // meeting a newer config leaves a container recoverable, not bricked. + Err(_) if bypass => (hook::Policy::declaring_nothing(harness), Vec::new()), + Err(unreadable) if unreadable_declaration(&unreadable) => { + return deny_unadjudicable(harness, &envelope, &unreadable, mode, out, err); + } + // EVERY OTHER FAULT KEEPS ITS OLD BEHAVIOUR, deliberately. Today that + // is still a whole-file refusal at exit `1`, which is the outcome the + // argument above says is wrong — making these actually preserve the + // rows they can read is its own change, over `prune_unresolvable`. + Err(other) => return Err(other), + } } else { (hook::Policy::declaring_nothing(harness), Vec::new()) }; @@ -12524,6 +12814,237 @@ fn run_hook( render(harness, &envelope, decision, &rendering, mode, out, err) } +/// The note for an event this host does not declare, or `None` to carry on. +/// +/// **`Unrecognized` is not undeclared**, and collapsing the two is why this is a +/// named predicate rather than an inline `&&`: an event nobody could parse has +/// no capability row to be absent from, so it falls through to the ordinary +/// path instead of being reported as a host that offers less. +/// +/// Returning the note rather than a `bool` keeps [`unsupported_event_note`]'s +/// call beside the condition that earns it — a caller that tested one and +/// rendered the other could report an event the table actually declares. +fn undeclared_event_note( + harness: hook::Harness, + capabilities: &hook::Capabilities, + event: hook::Event, +) -> Option { + (!capabilities.emits(event) && event != hook::Event::Unrecognized) + .then(|| unsupported_event_note(harness, capabilities, event)) +} + +/// Whether this envelope has anything for the config to decide about. +/// +/// **The gate on whether a call pays a config read at all**, which is why the +/// hot path stays cheap: `perf`'s `passthrough` arm — a `Read` with a +/// `file_path`, no command, no write — takes the `false` branch, and its +/// below-`noop` reading comes from doing so. +/// +/// Every clause was added by a measurement rather than by symmetry, and the +/// history is the argument for keeping them enumerated here: +/// +/// * a command or a write is the original shape; +/// * `Stop` carries neither, so a `mediated_call` module registered for the end +/// of turn could not run at all — a dead gate whose own suite stayed green, +/// because a `with input as` case fabricates the shape the boundary never +/// built (CLOUD-1051); +/// * `SessionStart` likewise, and for a MINT rather than a verdict (CLOUD-856): +/// the receipt that event exists to write could not know which manifests were +/// declared. One config load per session, not per call; +/// * a `PreTool` call naming a tool is the shape a tool-keyed row exists to +/// judge, and without it such a row was loaded for no call that could match. +fn is_adjudicable(envelope: &hook::Envelope) -> bool { + !envelope.command.is_empty() + || envelope.writes.is_some() + || envelope.event == hook::Event::Stop + || envelope.event == hook::Event::SessionStart + || (envelope.event == hook::Event::PreTool && !envelope.raw_tool.is_empty()) +} + +/// The mediated call on stdin, or `None` where the call must simply proceed. +/// +/// **THE TWO FAIL-OPEN BOUNDARIES, TOGETHER BECAUSE THEY ARE ONE ANSWER.** Stdin +/// that will not read and a payload that will not decode are both "the engine +/// does not know what this call IS", and neither may block it: a guard must never +/// be the reason a session cannot proceed. That is the opposite side of +/// [`unreadable_declaration`], where the engine knows the call perfectly well and +/// has been told it cannot enforce the rules over it. +/// +/// **Loud, never silent** (CLOUD-43). A guard that cannot read its input is a gate +/// that did not run, and the silent version of that is byte-identical to a clean +/// allow — the false green this engine exists to catch, in the one place nobody +/// would think to look. +/// **The RAW bytes travel with the decoded value**, because `dispatch_handlers` +/// hands a declared handler the payload as it arrived. Re-reading stdin for it is +/// not an option — the stream is consumed — and re-serializing the envelope would +/// hand a handler a document the host never sent. +fn read_envelope( + harness: hook::Harness, + mode: Mode, + err: &mut dyn Write, +) -> Result> { + let mut raw = String::new(); + if std::io::stdin().read_to_string(&mut raw).is_err() { + output::message(mode, Verbosity::Normal, err, UNREADABLE_STDIN)?; + return Ok(None); + } + let Some(envelope) = hook::decode(harness, &raw) else { + output::message(mode, Verbosity::Normal, err, UNDECODABLE_PAYLOAD)?; + return Ok(None); + }; + Ok(Some((raw, envelope))) +} + +/// Whether this load failure is a declaration nothing could read at all. +/// +/// **Positively identified, never inferred from an absence.** The tempting +/// spelling is "carries no declared class", and it is wrong: the +/// unsupported-version and `min_batten_version` refusals carry none either, and +/// both leave every row in the file readable. Keying on absence would refuse +/// those too — and every future unclassed refusal after them, silently widening +/// what denies. +/// +/// So the loader says which one this is. `config_error` already separates a +/// syntax failure from an unknown key, and since CLOUD-1677 its syntax arm raises +/// under [`verdict::Native::ConfigUnreadable`]. +fn unreadable_declaration(err: &anyhow::Error) -> bool { + err.downcast_ref::() + .and_then(|usage| usage.verdict) + .is_some_and(|class| class == verdict::Native::ConfigUnreadable) +} + +/// Refuse a call whose rules this build could not load (CLOUD-1677). +/// +/// Lifted out of [`run_hook`] rather than left inline because that function is +/// already at its line budget, and a boundary this load-bearing should be +/// readable on its own rather than as a match arm nine levels in. +/// +/// # Certainty denies; guessing allows +/// +/// The load used to propagate with `?`, raising a [`UsageError`] — exit `1` — +/// and [`crate::exit`] makes only `2` a denial precisely so no FAILURE path can +/// block a call. So a harness read this whole class as a non-blocking hook error +/// and ran the mediated tool anyway. Measured over one 5-day session: 1,149 +/// calls proceeded unjudged through seven windows of a mid-edit `batten.toml`, +/// and ~456 more through a preset the installed build did not ship — the +/// unknown-preset arm in [`crate::policy`] raises exactly this error. +/// +/// This is the discrimination `UNREADABLE_STDIN` sits on the other side of, and +/// the one CLOUD-1572 drew one level down. Where the engine is GUESSING about +/// the call — stdin it could not read, a payload that would not decode, an event +/// the host does not declare — allowing is right, because nothing is known and +/// refusing would make Batten the reason a session cannot proceed. Here the +/// engine has READ its own authority and been told it cannot enforce it: the +/// rule set is named, and unavailable. Proceeding is not caution then, it is a +/// gate reporting a clean allow over rules it never ran. +/// +/// # The surfaces stay separate +/// +/// `doctor` still never answers `2` — a diagnosis is not a policy verdict — and +/// the CLI verbs still raise a usage error over a config they cannot read. The +/// mediated boundary is the one place where "cannot judge" must not resolve to +/// "proceed", because here the alternative is a tool call nobody looked at. +/// +/// # The hatch is read before this is reached +/// +/// [`run_hook`] takes the bypass arm first, and that is what keeps a container +/// recoverable rather than bricked: a stale binary meeting a newer config +/// refuses every call until one of them moves, so the operator's declared escape +/// has to survive exactly the state that needs it. +/// +/// **A DECISION, NOT AN ERROR, WHICH IS WHY IT RENDERS.** [`render`] owns the +/// per-harness deny channel: Claude Code answers in its JSON decision object at +/// exit `0`, where the document *is* the deny, and the neutral adapter answers +/// [`ExitCode::Violation`]. Raising a [`Denial`] here would send `2` to the one +/// host that reads the document instead of the number. +/// +/// **The rendering carries no ceiling and the general hatch**, because both live +/// on the policy that would not load. `None` reads downstream as "no declared +/// bound" rather than as a bound of zero, which is the direction that keeps a +/// refusal about an unreadable config from being truncated by a value nobody +/// could read. +fn deny_unadjudicable( + harness: hook::Harness, + envelope: &hook::Envelope, + unreadable: &dyn std::fmt::Display, + mode: Mode, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result { + // THE FIRST LINE ONLY, AND THAT IS NON-NEGOTIABLE RULE 4 RATHER THAN BREVITY. + // + // A `toml` parse error renders as a multi-line span — a header naming the + // position, then the OFFENDING SOURCE LINE with a caret under it. Interpolating + // the whole thing puts a byte of the unreadable config into a refusal that + // reaches the model, the host's log and the transcript, which is exactly the + // payload rule 4 keeps out of every report this engine writes. Measured by + // `the_declaration_that_would_not_parse_is_named_without_quoting_it`, which + // fails on the un-truncated form. + // + // The first line is the POINTER and loses nothing an operator needs: for a + // parse failure it is `TOML parse error at line L, column C`, and for the + // skew and unknown-key arms the whole message is one line already — the + // `max_age = 0` and `command_matcher` diagnostics ride through intact, which + // is what keeps the repair as findable as it was when this arm exited `1`. + let pointer = unreadable + .to_string() + .lines() + .next() + .unwrap_or_default() + .to_owned(); + let refusal = Refusal::new( + "engine-cannot-adjudicate", + format!( + "this build could not load the rules it is registered to enforce, so nothing judged \ + this call: {pointer}" + ), + // No remedy the ENGINE may declare: the repair is rebuilding or + // reinstalling the binary, or fixing the config, and each is the + // consumer's own command (non-negotiable rule 1). + Fix::None, + ); + let rendering = Rendering { + hatch: hook::BYPASS_ENV, + ceiling: None, + }; + // WHICH CHANNEL CARRIED THE REFUSAL IS `render`'S OWN ANSWER, and reading it + // here is what lets the number say could-not-look without ever spending the + // refusal to do it (CLOUD-1677's exit-code half). + // + // `render` denies through the host's protocol: a document harness gets its + // decision object and answers `Ok`, while the neutral adapter's ONLY deny + // channel is the number, so it answers `Err(Denial)` — `run_hook`'s own + // contract says as much. The two arms below are therefore not a preference + // between codes, they are the two protocols. + match render( + harness, + envelope, + hook::Decision::Deny(refusal), + &rendering, + mode, + out, + err, + ) { + // The DOCUMENT already refuses, so the number is free to be honest: §6–§7 + // reserve `3` for could-not-look, and nothing was judged about this call. + // `Internal` rather than `Violation` also keeps `exit.rs`'s guarantee + // whole — `Usage` and `Internal` are the only codes a failure of Batten's + // own may produce, *so that fail-open is structural* — and an unreadable + // declaration is such a failure. Answering `2` here would have bought the + // refusal twice and spent that guarantee for the second copy. + Ok(_) => Ok(ExitCode::Internal), + // THE NUMBER IS THIS HARNESS'S ONLY CHANNEL, so it stays the deny. Turning + // it into `3` would be honest about the cause and silent about the verdict, + // which is the fail-open this row exists to close. + // + // **The cost is stated rather than absorbed** (CLOUD-1677's Replay asks for + // the per-harness table): on this adapter the boundary cannot say BOTH + // "refused" and "nothing could be read", so the could-not-look half is + // unavailable there until that protocol grows a way to carry it. + Err(denial) => Err(denial), + } +} + /// Run the refusing row's declared repair, and say what the boundary decides now. /// /// **Every other decision passes straight through**, and most `Deny`s do too: diff --git a/crates/batten/src/lint.rs b/crates/batten/src/lint.rs index 011cf024f..9bddf29d2 100644 --- a/crates/batten/src/lint.rs +++ b/crates/batten/src/lint.rs @@ -401,7 +401,7 @@ pub fn smells( // same message it would anywhere else rather than this module's own. let config = config::parse(text, source)?; let located: Located = - toml::from_str(text).map_err(|err| config::config_error(source, &err))?; + toml::from_str(text).map_err(|err| config::config_error(source, text, &err))?; let mut found = Vec::new(); diff --git a/crates/batten/src/pipeline.rs b/crates/batten/src/pipeline.rs index dfa8b7c9f..4c5e5186a 100644 --- a/crates/batten/src/pipeline.rs +++ b/crates/batten/src/pipeline.rs @@ -51,6 +51,11 @@ //! That is raise-only in the same spirit as the deny-only Rego surface: a //! consumer may compose any pipeline, but not one that leaks spend. +// CLOUD-1681: dropping the `Push` row's precheck restores the path from a +// pending bet to a publish, which is the state measured twice. +//MUTANT-SUITE crates/batten/tests/it/land_speculation.rs +//MUTANT live-bet-reaches-the-push|s@ precheck: Some(Precheck::BetLive),@ precheck: None,@|a_live_bet_is_refused_by_the_row_that_would_publish_it + use crate::land::Step; /// A step's declared undo, run when a lap leaves without landing. @@ -243,6 +248,44 @@ pub enum Precheck { /// that reads as a narrowing: the next reader should find the gap written /// down rather than infer its absence. LeaseHeld, + /// Is a speculation still outstanding on the head we are about to publish? + /// + /// **THE INVARIANT THIS FILE ALREADY QUOTES AND DID NOT HOLD** (CLOUD-1681). + /// `mise-tasks/land.sh`, verbatim above: *"there is no path from a losing bet + /// to a push, which is what makes speculating safe rather than merely fast."* + /// [`Settle::Lost`] has no such path — [`Self::BetSettled`] unwinds it at the + /// top of the lap. [`Settle::Pending`] does, because that precheck + /// deliberately KEEPS a pending bet, and this row carried no precheck at all. + /// + /// # A PENDING BET IS EXACTLY AS UNPUBLISHABLE AS A LOST ONE + /// + /// A bet names the holder's SHAS, and the holder lands by rebase — which + /// mints new ones for the same patches. So a published pending head does not + /// merely risk going stale: the moment the holder lands it is DIVERGENT, and + /// the fast-forward is impossible by construction rather than by race. The + /// bet is unwinnable at the instant it is placed, so publishing it guarantees + /// a wasted matrix rather than risking one. + /// + /// Measured twice — four commits of another branch published under this one + /// on 2026-09-08, eight on 2026-09-10, the second costing a full matrix on a + /// head that could not merge when it was graded. + /// + /// **It answers `Lap`, never `Stop`, and that is the design.** `Stop` strands + /// the waiter behind CLOUD-1306's poisoned base indefinitely, trading a + /// wasted matrix for an unbounded stall. `Lap` drops the borrowed range and + /// re-enters: replay onto real trunk, verify the UNSPECULATED tree, publish + /// this branch's own commits alone — one extra local verify and zero CI. + /// + /// **Speculation keeps its whole value.** A holder that lands during `verify` + /// settles [`Settle::Landed`], the bet is forgotten, nothing unwinds, and the + /// lap publishes the pre-linearized head exactly as before. This fires only + /// where the bet failed to pay off — precisely where carrying the borrowed + /// range is worthless AND harmful. + /// + /// [`Settle::Lost`]: crate::speculation::Settle::Lost + /// [`Settle::Pending`]: crate::speculation::Settle::Pending + /// [`Settle::Landed`]: crate::speculation::Settle::Landed + BetLive, } /// A declared landing pipeline. @@ -450,7 +493,11 @@ impl Default for Pipeline { step: Step::Push, effectful: true, compensate: Compensation::ReleaseLease, - precheck: None, + // THE ROW THAT PUBLISHES IS THE ROW THAT ASKS (CLOUD-1681). + // `Replay` asks whether the bet is worth KEEPING; this asks + // whether it is fit to PUBLISH, and the two answers differ + // for a pending bet — which is the whole gap. + precheck: Some(Precheck::BetLive), }, StepRow { step: Step::Ready, @@ -530,6 +577,46 @@ mod tests { ); } + /// **THE PUBLISHING ROW ASKS BEFORE IT PUBLISHES** (CLOUD-1681). + /// + /// `mise-tasks/land.sh`'s invariant, quoted in this file's own header, is + /// *"there is no path from a losing bet to a push"* — and for the whole of + /// this table's life the `Push` row carried `precheck: None` while `Replay`, + /// `Ready` and the commit point each carried one. A pending bet reached the + /// publish, twice measured. + /// + /// **The mirror is the half that discriminates.** Asserting only that `Push` + /// has a precheck passes over a table where every row has one, which would be + /// its own defect — `Verify` spends the gate and must not be gated on the bet, + /// or a speculating lap could never verify the tree it speculated. + #[test] + fn the_push_row_asks_about_the_bet_and_the_verify_row_does_not() { + let shipped = Pipeline::default(); + let precheck = |step: Step| { + shipped + .steps + .iter() + .find(|row| row.step == step) + .and_then(|row| row.precheck) + }; + + assert_eq!( + precheck(Step::Push), + Some(Precheck::BetLive), + "the row that publishes must ask whether the head is publishable" + ); + assert_eq!( + precheck(Step::Verify), + None, + "gating the gate on the bet would stop a speculating lap verifying at all" + ); + assert_eq!( + precheck(Step::Replay), + Some(Precheck::BetSettled), + "and the settle stays where it was — the two ask different questions" + ); + } + /// **`Abandon` IS THE ONLY ATTEMPT-OWED UNDO, and the mirror is what makes /// this case discriminate.** Without the second half, a predicate returning /// `true` for everything would satisfy the first — and that predicate would diff --git a/crates/batten/src/repair.rs b/crates/batten/src/repair.rs index 69d1b05e6..0126c7365 100644 --- a/crates/batten/src/repair.rs +++ b/crates/batten/src/repair.rs @@ -163,7 +163,7 @@ pub fn run(root: &Path, fix: &str, key: Option<&str>, applicability: Applicabili // `Diagnostics::Drop`: the repair's own chatter is not a finding, and a // consumer's command could print anything at all. let Some((code, _output)) = - crate::exec::piped_argv(root, &words, "", crate::exec::Diagnostics::Drop) + crate::exec::piped_argv(root, &words, "", crate::exec::Diagnostics::Drop, &[]) else { // The program would not resolve. A declared repair naming something this // host does not have is a config defect, and the ordinary refusal is the diff --git a/crates/batten/src/semver.rs b/crates/batten/src/semver.rs index 7639f0750..9c64bd4d0 100644 --- a/crates/batten/src/semver.rs +++ b/crates/batten/src/semver.rs @@ -218,22 +218,36 @@ pub fn against_rev( baseline: &str, release_type: &str, ) -> Option { - let output = std::process::Command::new(ANALYSER) - .arg(format!("+{toolchain}")) - .args(["semver-checks", "check-release"]) - .args(["--package", package]) - .args(["--baseline-rev", baseline]) - .args(["--release-type", release_type]) - // Overriding whatever the caller's environment set, and load-bearing rather than - // cosmetic: the report below is PARSED, and a gate that parses colour is - // CLOUD-199's defect — an anchored pattern that can never match because - // escape sequences sit between the anchor and the word. - .env("CARGO_TERM_COLOR", "never") - .current_dir(root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .ok()?; + // THROUGH THE LADDER, never `Command::new` directly (CLOUD-1494). The adapter + // placement above sanctions the SPAWN; it says nothing about how the program + // is resolved, and resolving it here bypassed rung 0 — the pin. `rules.rs` + // measured that exact shape: *"nothing a toolchain manager provides is on + // bare `PATH` -- nor should it be"*, so a `cargo` the project pins was + // reached around every time and an unpinned one silently used instead. + // + // `extra` goes ahead of `+{toolchain}` and that is still correct. The + // ordering rule below is about cargo's OWN argv; where rung 3 fires the + // program becomes an interpreter and `extra` carries the script, so + // `+{toolchain}` remains the first argument cargo itself sees. + let output = crate::rules::spawn_resolving(Some(root), ANALYSER, |program, extra| { + std::process::Command::new(program) + .args(extra) + .arg(format!("+{toolchain}")) + .args(["semver-checks", "check-release"]) + .args(["--package", package]) + .args(["--baseline-rev", baseline]) + .args(["--release-type", release_type]) + // Overriding whatever the caller's environment set, and load-bearing rather than + // cosmetic: the report below is PARSED, and a gate that parses colour is + // CLOUD-199's defect — an anchored pattern that can never match because + // escape sequences sit between the anchor and the word. + .env("CARGO_TERM_COLOR", "never") + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + }) + .ok()?; Some(Compared { code: output.status.code(), report: merged(&output), @@ -255,32 +269,38 @@ pub fn against_rustdoc( current: Option<&Path>, release_type: &str, ) -> Option { - let mut command = std::process::Command::new(ANALYSER); - // `+toolchain` FIRST, always: cargo reads it as argv[1] and nowhere else, so - // an option pushed ahead of it silently runs the default toolchain — the - // failure that has no symptom until a version-dependent build breaks. - command - .arg(format!("+{toolchain}")) - .args(["semver-checks", "check-release"]) - .args(["--package", package]) - .arg("--baseline-rustdoc") - .arg(rustdoc); - // `--current-rustdoc` only when one was built. Absent, the tool generates the - // head side itself through the scratch resolve — which is the path CLOUD-1399 - // measured failing, so this is the arm that matters here; it stays optional - // because a caller that could not build the head side is still better served - // by the tool's own generation than by no comparison at all. - if let Some(current) = current { - command.arg("--current-rustdoc").arg(current); - } - let output = command - .args(["--release-type", release_type]) - .env("CARGO_TERM_COLOR", "never") - .current_dir(root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .ok()?; + // Through the ladder, for `against_rev`'s reason (CLOUD-1494). + let output = crate::rules::spawn_resolving(Some(root), ANALYSER, |program, extra| { + let mut command = std::process::Command::new(program); + // `+toolchain` FIRST, always: cargo reads it as argv[1] and nowhere else, so + // an option pushed ahead of it silently runs the default toolchain — the + // failure that has no symptom until a version-dependent build breaks. + // `extra` is the ladder's own prefix and precedes it, which keeps + // `+{toolchain}` first in the argv cargo itself receives. + command + .args(extra) + .arg(format!("+{toolchain}")) + .args(["semver-checks", "check-release"]) + .args(["--package", package]) + .arg("--baseline-rustdoc") + .arg(rustdoc); + // `--current-rustdoc` only when one was built. Absent, the tool generates the + // head side itself through the scratch resolve — which is the path CLOUD-1399 + // measured failing, so this is the arm that matters here; it stays optional + // because a caller that could not build the head side is still better served + // by the tool's own generation than by no comparison at all. + if let Some(current) = current { + command.arg("--current-rustdoc").arg(current); + } + command + .args(["--release-type", release_type]) + .env("CARGO_TERM_COLOR", "never") + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + }) + .ok()?; Some(Compared { code: output.status.code(), report: merged(&output), @@ -312,13 +332,21 @@ pub fn toolchain(root: &Path) -> Option { { return Some(named); } - let output = std::process::Command::new("rustc") - .arg("--version") - .current_dir(root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::null()) - .output() - .ok()?; + // Through the ladder (CLOUD-1494), and this one is the site where bypassing it + // was most clearly wrong: the doc above says the pin is READ rather than + // restated, and reading it off a `rustc` resolved on bare `PATH` reads a + // DIFFERENT compiler from the one the project pins — so the "authority" was + // whichever toolchain the ambient environment happened to expose. + let output = crate::rules::spawn_resolving(Some(root), "rustc", |program, extra| { + std::process::Command::new(program) + .args(extra) + .arg("--version") + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output() + }) + .ok()?; let text = String::from_utf8_lossy(&output.stdout).into_owned(); let version = text.split_whitespace().nth(1)?; (!version.is_empty()).then(|| version.to_owned()) @@ -383,43 +411,51 @@ pub fn baseline_rustdoc( // this whole gate exists against. crate::git::materialize_rev(root, baseline, &worktree) .map_err(|err| format!("the baseline tree could not be materialized: {err}"))?; - let built = std::process::Command::new(ANALYSER) - .arg(format!("+{toolchain}")) - .args([ - "doc", - "--locked", - "--no-deps", - "--lib", - "--package", - package, - ]) - .env("RUSTC_BOOTSTRAP", "1") - .env( - "RUSTDOCFLAGS", - "-Z unstable-options --output-format json --document-private-items", - ) - .env("CARGO_TARGET_DIR", &target) - .env("CARGO_TERM_COLOR", "never") - // THE OUTER CARGO'S ENVIRONMENT IS NOT THIS BUILD'S. When the binary - // itself is launched through `cargo run`, cargo exports its own manifest - // and toolchain into the child, and a nested `cargo doc` reads them as - // instructions about a package that is not the one in front of it. Every - // one is removed rather than overridden, because overriding requires - // knowing the whole set and removal does not. - .env_remove("CARGO") - .env_remove("CARGO_MANIFEST_DIR") - .env_remove("CARGO_MANIFEST_PATH") - .env_remove("CARGO_PKG_NAME") - .env_remove("CARGO_PKG_VERSION") - .env_remove("CARGO_MAKEFLAGS") - .env_remove("RUSTC") - .env_remove("RUSTDOC") - .env_remove("RUSTUP_TOOLCHAIN") - .current_dir(&worktree) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .map_err(|err| format!("the baseline doc build could not be run: {err}"))?; + // Through the ladder, for `against_rev`'s reason (CLOUD-1494). Resolved + // against `root` rather than the scratch worktree: the pin is the PROJECT's, + // and a materialized baseline tree carries no toolchain configuration of its + // own — asking it would resolve nothing and fall through to bare `PATH`, + // which is the bypass this change closes. + let built = crate::rules::spawn_resolving(Some(root), ANALYSER, |program, extra| { + std::process::Command::new(program) + .args(extra) + .arg(format!("+{toolchain}")) + .args([ + "doc", + "--locked", + "--no-deps", + "--lib", + "--package", + package, + ]) + .env("RUSTC_BOOTSTRAP", "1") + .env( + "RUSTDOCFLAGS", + "-Z unstable-options --output-format json --document-private-items", + ) + .env("CARGO_TARGET_DIR", &target) + .env("CARGO_TERM_COLOR", "never") + // THE OUTER CARGO'S ENVIRONMENT IS NOT THIS BUILD'S. When the binary + // itself is launched through `cargo run`, cargo exports its own manifest + // and toolchain into the child, and a nested `cargo doc` reads them as + // instructions about a package that is not the one in front of it. Every + // one is removed rather than overridden, because overriding requires + // knowing the whole set and removal does not. + .env_remove("CARGO") + .env_remove("CARGO_MANIFEST_DIR") + .env_remove("CARGO_MANIFEST_PATH") + .env_remove("CARGO_PKG_NAME") + .env_remove("CARGO_PKG_VERSION") + .env_remove("CARGO_MAKEFLAGS") + .env_remove("RUSTC") + .env_remove("RUSTDOC") + .env_remove("RUSTUP_TOOLCHAIN") + .current_dir(&worktree) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + }) + .map_err(|err| format!("the baseline doc build could not be run: {err}"))?; if !built.status.success() { return Err(format!( "the baseline doc build failed: {}", @@ -482,37 +518,41 @@ pub fn current_rustdoc( .canonicalize() .map_err(|err| format!("the current scratch directory could not be resolved: {err}"))? .join("target"); - let built = std::process::Command::new(ANALYSER) - .arg(format!("+{toolchain}")) - .args([ - "doc", - "--locked", - "--no-deps", - "--lib", - "--package", - package, - ]) - .env("RUSTC_BOOTSTRAP", "1") - .env( - "RUSTDOCFLAGS", - "-Z unstable-options --output-format json --document-private-items", - ) - .env("CARGO_TARGET_DIR", &target) - .env("CARGO_TERM_COLOR", "never") - .env_remove("CARGO") - .env_remove("CARGO_MANIFEST_DIR") - .env_remove("CARGO_MANIFEST_PATH") - .env_remove("CARGO_PKG_NAME") - .env_remove("CARGO_PKG_VERSION") - .env_remove("CARGO_MAKEFLAGS") - .env_remove("RUSTC") - .env_remove("RUSTDOC") - .env_remove("RUSTUP_TOOLCHAIN") - .current_dir(root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .map_err(|err| format!("the current doc build could not be run: {err}"))?; + // Through the ladder, for `against_rev`'s reason (CLOUD-1494). + let built = crate::rules::spawn_resolving(Some(root), ANALYSER, |program, extra| { + std::process::Command::new(program) + .args(extra) + .arg(format!("+{toolchain}")) + .args([ + "doc", + "--locked", + "--no-deps", + "--lib", + "--package", + package, + ]) + .env("RUSTC_BOOTSTRAP", "1") + .env( + "RUSTDOCFLAGS", + "-Z unstable-options --output-format json --document-private-items", + ) + .env("CARGO_TARGET_DIR", &target) + .env("CARGO_TERM_COLOR", "never") + .env_remove("CARGO") + .env_remove("CARGO_MANIFEST_DIR") + .env_remove("CARGO_MANIFEST_PATH") + .env_remove("CARGO_PKG_NAME") + .env_remove("CARGO_PKG_VERSION") + .env_remove("CARGO_MAKEFLAGS") + .env_remove("RUSTC") + .env_remove("RUSTDOC") + .env_remove("RUSTUP_TOOLCHAIN") + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + }) + .map_err(|err| format!("the current doc build could not be run: {err}"))?; if !built.status.success() { return Err(format!( "the current doc build failed: {}", @@ -666,6 +706,60 @@ mod tests { } } + /// **EVERY SPAWN HERE GOES THROUGH THE LADDER** (CLOUD-1494). + /// + /// `policy/spawn-adapters.rego` places this module, which sanctions the + /// SPAWN — it says nothing about how the program is RESOLVED, and that gap is + /// what let five sites reach `Command::new(ANALYSER)` directly and skip rung + /// 0. `rules.rs` measured the cost of exactly that: *"nothing a toolchain + /// manager provides is on bare `PATH` -- nor should it be"*, so a pinned + /// `cargo` was reached around every time and whatever the ambient environment + /// exposed was used instead. `toolchain()` was the worst of the five: its own + /// doc calls itself a READ of the pin, and it read a different compiler. + /// + /// Asserted over this file's own text because the property is syntactic — + /// the argument to `Command::new` — and no exit code reaches it. The shape is + /// `git.rs:5127`'s, which greps this crate for `Command::new("git")` and + /// fails on a hit; here the refusal is narrower, since the spawn is legitimate + /// and only the unresolved program is not. + #[test] + fn no_spawn_in_this_module_names_its_program_directly() { + // COMMENT LINES ARE DROPPED FIRST, and this file's own first draft is why. + // The prose above names the defect in its own words, so a scan over raw + // text found the call spelled in a COMMENT and reported it as a live site. + // `rules/scanning.md` row two is exactly this — a syntax question answered + // with a text scanner — and CLOUD-843 measured the same class, where + // `ci-local-parity` landed in the wrong bucket because the string appeared + // in a comment. A line filter is the cheap half of the right instrument; + // it is sound here because every real call sits in command position on a + // line of its own. + let code: String = include_str!("semver.rs") + .lines() + .filter(|line| !line.trim_start().starts_with("//")) + .collect::>() + .join("\n"); + // THE NEEDLE IS ASSEMBLED, never written whole, and `git.rs`'s own + // crate-wide scan does the same for the same reason: a scanner spelled + // literally matches ITSELF, so the first version reported its own search + // string as a defect. Stripping comments was not enough — this one is in + // executable code. + let needle = ["Command", "::new("].concat(); + for site in code.match_indices(needle.as_str()) { + let (_, tail) = code.split_at(site.0 + needle.len()); + // `unwrap_or` rather than `expect`: a call with no closing parenthesis + // is not this test's subject, and an empty argument fails the assertion + // below anyway — so the degenerate parse reports as a finding rather + // than as a panic in the checker. + let argument = tail.split(')').next().unwrap_or_default(); + assert_eq!( + argument, "program", + "a spawn here must take the program the ladder resolved, never a \ + literal or a constant — that skips rung 0 and reaches around a \ + pinned toolchain" + ); + } + } + #[test] fn a_graded_zero_run_is_never_a_pass() { let compared = report(" Checked [ 0.1s] 0 checks: 0 pass, 0 fail\n"); diff --git a/crates/batten/src/speculation.rs b/crates/batten/src/speculation.rs index 671695cea..03541580d 100644 --- a/crates/batten/src/speculation.rs +++ b/crates/batten/src/speculation.rs @@ -168,6 +168,68 @@ pub struct Bet { /// it, and reading it as "no more bets at all" would give up speculating for /// the rest of the landing over one bad candidate. pub conflicts: Option, + /// The holder's base whose tree was REFUSED by the gate (CLOUD-1306). + /// + /// **[`Bet::conflicts`]'s twin, and for the identical reason one field up.** + /// That one records a base known to conflict with this branch; this one + /// records a base known to poison it — the replay succeeded, the tree is well + /// formed, and `verify` refused it. Both are judgements about a COMMIT rather + /// than about this landing, so both survive [`Bet::forget`] and both are read + /// by [`Bet::would_rebet`]. + /// + /// **WITHOUT IT, LAPPING ON A POISONED BASE IS A SPIN RATHER THAN A + /// DISCRIMINATION.** `land::Basis::Borrowed` turns a refused gate into a lap + /// so the failure is not blamed on this author, and the whole argument for + /// that is that the NEXT lap re-runs the same gate with the borrowed base + /// gone. The lap unwinds and calls `forget`, which clears `base` — and + /// `would_rebet` compares `base`, so a forgotten bet answered `true` for the + /// same holder and the next lap borrowed the identical poisoned commits. The + /// gate would then refuse again, for the same reason, having proved nothing, + /// until the bound stopped it. `the_same_candidate_is_not_bet_on_twice` + /// records that arm as *"CLOUD-1306's other half"*, and it is the half that + /// makes the first half worth anything. + /// + /// An `Option` rather than a `bool`, for `conflicts`'s stated reason: + /// a flag cannot tell the holder that poisoned this branch from the one that + /// replaced it, so reading it as "no more bets at all" would give up + /// speculating for the rest of the landing over one bad candidate — and + /// speculation is the pipelining, so giving it up is the throughput loss this + /// mechanism exists to avoid. + /// + /// # "Poisoned" names the PAIR, and the field is right for both causes + /// + /// A gate refusing over a borrowed base has two candidate causes and this + /// clone can distinguish neither from here: the base is genuinely broken, or + /// the base and this branch are each fine and conflict semantically. The name + /// leans toward the first; the behaviour is correct under both, which is why + /// the field is a pair judgement — *this base, with these commits* — rather + /// than a verdict on the base alone. + /// + /// Under the second cause, declining to re-borrow is still right, and the + /// error does not survive: when the holder lands, the next replay puts their + /// change on trunk, the same gate refuses with [`crate::land::Basis::Own`], + /// and the loop STOPS with the conflict correctly attributed to a tree this + /// branch is now actually responsible for. So the worst case is one deferred + /// stop rather than a wrong one, and nothing here asserts a verdict about + /// somebody else's commits that this clone has not earned. + pub poisoned: Option, + /// This landing has already declined to publish a speculation (CLOUD-1681). + /// + /// **THE TERMINATION HALF, and without it the precheck is a spin.** The + /// `Push` row's precheck unwinds a live bet and laps; nothing in the lap + /// otherwise remembers that, so `place_the_bet` would bet on the same holder + /// at the top of the next lap, reach the same precheck, and unwind again — + /// bounded by the lap budget rather than by the one extra local verify the + /// design promises. + /// + /// A `bool` rather than a base, unlike [`Bet::conflicts`]: that + /// records a judgement about a COMMIT and stays true of it afterwards. This records a decision about THIS LANDING — we got as far + /// as the push with a bet outstanding, so speculating again buys nothing + /// before this branch lands. Naming a base would invite re-betting on the + /// next holder and paying the same unwind a second time. + /// + /// Survives [`Bet::forget`], because `forget` is what the unwind calls. + pub declined: bool, } impl Bet { @@ -209,6 +271,15 @@ impl Bet { if self.conflicts.as_deref() == Some(candidate) { return false; } + // AND A BASE THAT POISONED THIS BRANCH IS NOT BORROWED AGAIN + // (CLOUD-1306). Same shape as the conflict arm above and the same + // argument: the judgement is about that COMMIT, it stays true of it after + // the bet is forgotten, and re-betting would spend the next lap proving + // it a second time. Declining sends the lap onto trunk, which is the + // experiment `land::Basis` laps in order to run. + if self.poisoned.as_deref() == Some(candidate) { + return false; + } self.base.as_deref() != Some(candidate) } @@ -521,6 +592,8 @@ mod tests { recovered: true, pushed: true, conflicts: Some(String::from("feedface")), + poisoned: Some(String::from("deadbeef")), + declined: true, }; bet.forget(); assert!(bet.is_forgotten(), "a settled bet carried state forward"); @@ -717,19 +790,23 @@ mod tests { assert_eq!(settle(&adopted, Some(MOVED), false, Live::No), Settle::Lost); } - /// **CLOUD-1306, PORTED AS-IS AND PINNED SO IT CANNOT BE FIXED BY ACCIDENT.** + /// **`settle` STILL CANNOT SEE A POISONED BASE, AND THAT STAYS TRUE.** /// - /// A poisoned base — one whose tree will never pass `verify` — is - /// byte-identical here to a holder that is simply slow: the lease is held, - /// `main` has not moved, so `settle` says pending and the waiter sits. This - /// case asserts that reading rather than the one a fixed version would give, - /// because a port that quietly improved behaviour could not be shown to - /// conserve it. + /// This case was pinned as CLOUD-1306's defect, with a note that its changing + /// would be the review's cue that behaviour moved. Behaviour has moved and + /// this case has NOT changed, which is the honest outcome rather than a + /// missed update: `settle` answers "is the bet won, lost or outstanding" from + /// the holder and the trunk, and from there a base that will never pass + /// `verify` is byte-identical to a holder that is merely slow. No fourth arm + /// can be derived from these inputs, because the discriminating fact — the + /// gate's verdict on the borrowed tree — is not among them. /// - /// When CLOUD-1306 lands, this case is the one that must change, and its - /// changing is the review's cue that behaviour moved. + /// The fix lives where that fact exists: the lap records [`Bet::poisoned`] + /// when the gate refuses over a borrowed base, and [`Bet::would_rebet`] + /// declines it afterwards. So this reading is conserved and the mechanism + /// sits beside it rather than inside it. #[test] - fn a_poisoned_base_is_conserved_as_pending_because_cloud_1306_owns_the_fix() { + fn settle_cannot_tell_a_poisoned_base_from_a_slow_holder() { assert_eq!( settle(&placed(), Some(MAIN), false, Live::Yes), Settle::Pending, @@ -738,15 +815,36 @@ mod tests { ); } - /// One outstanding bet at a time. + /// One outstanding bet at a time — and never twice on a base that poisoned us. + /// + /// **THE LAST ASSERTION INVERTED, WHICH IS CLOUD-1306's OTHER HALF LANDING.** + /// It used to assert that a forgotten bet re-bets on the same holder, and + /// named that as the defect. It was load-bearing for the defect and is now + /// load-bearing for the fix: `land::Basis::Borrowed` laps a refused gate on + /// the argument that the next lap runs without the borrowed base, and a + /// forgotten bet that re-borrowed the same commits would make that argument + /// false and the lap a spin. #[test] fn the_same_candidate_is_not_bet_on_twice() { let bet = placed(); assert!(!bet.would_rebet(HOLDER), "already the outstanding bet"); assert!(bet.would_rebet(MOVED), "a different candidate is a new bet"); + + // A FORGOTTEN BET STILL DECLINES THE BASE THAT POISONED IT. `forget` + // clears `base`, so this arm is reached only because `poisoned` survives + // it — which is the whole reason that field is a judgement about the + // commit rather than about this landing. + let mut poisoned = placed(); + poisoned.poisoned = Some(String::from(HOLDER)); + poisoned.forget(); assert!( - Bet::default().would_rebet(HOLDER), - "and a forgotten bet re-bets on the same holder — CLOUD-1306's other half" + !poisoned.would_rebet(HOLDER), + "a base whose tree the gate refused is not borrowed a second time" + ); + assert!( + poisoned.would_rebet(MOVED), + "and one bad candidate does not end speculation for the whole landing, \ + because speculation is the pipelining" ); } diff --git a/crates/batten/src/verdict.rs b/crates/batten/src/verdict.rs index ff125021b..067b79ad8 100644 --- a/crates/batten/src/verdict.rs +++ b/crates/batten/src/verdict.rs @@ -1258,6 +1258,39 @@ pub enum Native { /// than in a consumer `[[verdict]]` row. No consumer can name the class, /// because the engine is what takes the plan and what compares it. PlanReadStale, + /// The declaration could not be READ AT ALL — not valid TOML. + /// + /// **APPENDED, NEVER INSERTED.** This enum carries no `repr`, so a variant + /// added in the middle shifts every later discriminant and + /// `enum_no_repr_variant_discriminant_changed` reads the whole tail as + /// broken — measured here, where placing it beside the other config classes + /// moved eighteen of them. Position is API; the reading order below is not. + /// + /// **Deliberately not in [`Native::CONFIG_FAULTS`]**, which is the per-TABLE + /// set and is censused in both directions against `config.rs`'s own list. This + /// one has no table: it is raised before any table exists, by the parse that + /// every table's validator runs after. + /// + /// # Why it needs a class when the other parse failures do not + /// + /// A class here is not for `explain` — it is the DISCRIMINATOR the mediated + /// boundary acts on (CLOUD-1677). Gates are registered fail-open, and a gate + /// that fails open is inert: it neither allows nor denies, it is absent. So a + /// config fault is never a choice between refusing and allowing, it is a + /// choice between keeping the enforcement surface we still have and losing it + /// entirely. + /// + /// An unknown key, a version this build is too old for, a row whose validator + /// refused — each leaves every OTHER row readable and enforceable, and leaves + /// an agent that can still be told to repair the one that is broken. Failing + /// the whole load there buys nothing and costs the surface that would have + /// carried the repair instruction. + /// + /// A file that is not TOML is the one case with no partial function to + /// preserve: zero rows are readable, so refusing the call is the only signal + /// left, and the declared hatch is the recovery path. That asymmetry is why + /// this class exists and why it is exactly one class wide. + ConfigUnreadable, } impl Native { @@ -1291,6 +1324,7 @@ impl Native { Native::CallFixSilent, Native::ContentRefused, Native::KeyMissing, + Native::ConfigUnreadable, Native::VerbTableRefused, Native::PatternTableRefused, Native::VerdictTableRefused, @@ -1373,6 +1407,7 @@ impl Native { Native::CallFixSilent => "call fix silent", Native::ContentRefused => "input write refused", Native::KeyMissing => "issue name missing", + Native::ConfigUnreadable => "config read refused", Native::VerbTableRefused => "verb declare refused", Native::PatternTableRefused => "pattern declare refused", Native::VerdictTableRefused => "verdict declare refused", @@ -1891,6 +1926,20 @@ it serves.", // Every route is the config itself, which is not a placeholder: a config // fault is edited in exactly one file, and a `command` route would have to // name a task that can run over a config that does not load. + VendoredVerdict { + id: "config read refused", + gloss: "the declaration is not TOML, so no rule in it could be read", + class: "Every other config fault leaves the rest of the file deciding -- an unknown key \ +costs its own row, a table whose validator refuses names that table, and a version this build \ +is too old for still says so. Each of those keeps a working gate surface and an agent that can \ +be told to repair the broken part. This one has no partial function to preserve: the bytes are \ +not TOML, so zero rows are readable and nothing is enforced. That is why it is the one class \ +the mediated boundary refuses a call under, rather than reporting and proceeding -- a gate that \ +fails open is inert, and an inert gate over an unreadable authority is the false green this \ +engine exists to catch.", + routes: &[read("config read first", "batten.toml")], + applicability: Applicability::Advice, + }, VendoredVerdict { id: "verb declare refused", gloss: "the verb table would not load", @@ -2367,6 +2416,7 @@ mod tests { | Native::VerdictTrailing | Native::RunOrphaned | Native::CeilingExceeded + | Native::ConfigUnreadable | Native::ShapeRefused | Native::CallRetryNow | Native::CallFixSilent diff --git a/crates/batten/tests/it/adjudicate_absent.rs b/crates/batten/tests/it/adjudicate_absent.rs new file mode 100644 index 000000000..300938ce4 --- /dev/null +++ b/crates/batten/tests/it/adjudicate_absent.rs @@ -0,0 +1,185 @@ +//! A call this build cannot adjudicate is DENIED, never allowed by a failure. +//! +//! The tier that proves the engine does not fail open when it has read its own +//! authority and been told it cannot enforce it. Unit cases over `adjudicate` +//! cannot host this: the defect is not in the decision, it is in what the +//! BOUNDARY does with a load that failed, and only the compiled binary answers +//! that — `mediated_admission.rs`'s header records the same lesson, where unit +//! cases passed while the binary allowed the write. +//! +//! # The mirror is not decoration +//! +//! `a_loadable_config_still_allows_an_ordinary_call` is what stops this being +//! satisfied by an adjudicator that denies everything. A fail-closed hook that +//! refuses each call is not a fix, it is an outage — CLOUD-1688's falsifier says +//! so in as many words, and that is why the pair lands together. +//! +//! # Scope +//! +//! The CONFIG half of CLOUD-1688: a `batten.toml` this build cannot load. The +//! VERB half — a registration spelling a subcommand the installed binary does +//! not have — needs `doctor` to interrogate the installed artifact rather than +//! itself, because a self-check runs in the build that mise resolves and never +//! in the one the hook does. It lands with that part and belongs in this file. + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use std::path::{Path, PathBuf}; + +use common::{Fixture, run_with_stdin}; + +/// A `batten.toml` mid-edit, which is the largest measured bucket: seven windows +/// across one 5-day session, 1,149 calls, every one of them unjudged. +const WILL_NOT_PARSE: &str = "version = 1\nthis is not toml\n"; + +/// A config that loads and declares nothing this call matches. +const LOADS: &str = "version = 1\n"; + +/// A fixture carrying `body` as its committed authority. +fn fixture(name: &str, body: &str) -> PathBuf { + Fixture::new(name) + .config(body) + .file("notes.md", "ordinary\n") + .git() + .base_commit() + .build() +} + +/// A Claude Code `PreToolUse` envelope carrying a command, so the boundary +/// treats it as adjudicable and reaches the config load at all. +fn payload() -> String { + "{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\ + \"tool_input\":{\"command\":\"echo hello\"}}" + .to_owned() +} + +/// Adjudicate on the neutral adapter, where the VERDICT IS THE NUMBER. +/// +/// `exit-code` rather than `claude-code` for the two cases that ask what the +/// code is: on Claude Code the deny is the JSON document at exit `0`, so a case +/// asserting a number there would assert the wrong channel. The document is +/// checked separately below. +fn code(dir: &Path) -> Option { + run_with_stdin(dir, &["adjudicate", "--harness", "exit-code"], &payload()) + .status + .code() +} + +#[test] +fn a_config_this_build_cannot_load_denies_rather_than_failing_open() { + // WAS `1`, WHICH A HARNESS READS AS A NON-BLOCKING HOOK ERROR, so the + // mediated tool ran with nothing judging it (CLOUD-1677). + // + // `2` HERE AND `3` ON A DOCUMENT HARNESS, and the split is the protocol + // rather than a preference: this adapter's ONLY deny channel is the number, + // so the number has to carry the refusal. Where the decision object carries + // it instead, the number is free to say could-not-look — the case below + // asserts that side. + let dir = fixture("adjudicate-unloadable", WILL_NOT_PARSE); + assert_eq!( + code(&dir), + Some(2), + "a call nothing could judge must be refused, not allowed by the failure" + ); +} + +#[test] +fn a_loadable_config_still_allows_an_ordinary_call() { + // THE MIRROR. Without it the case above is satisfied by an adjudicator that + // denies every call in the fleet, which is an outage wearing a fix's clothes. + let dir = fixture("adjudicate-loadable", LOADS); + assert_eq!( + code(&dir), + Some(0), + "a config that loads and refuses nothing must still allow" + ); +} + +#[test] +fn the_declared_hatch_still_reaches_a_clone_whose_config_will_not_load() { + // What keeps a container recoverable rather than bricked. A stale binary + // meeting a newer config refuses every call until one of them moves, so the + // operator's declared escape has to survive exactly the state that needs it. + // + // `common::batten()` scrubs every bypass variable by construction, so setting + // one here is the only way it is present — a case that inherited it from the + // developer's shell would pass without testing anything. + use std::io::Write as _; + use std::process::Stdio; + + let dir = fixture("adjudicate-hatch", WILL_NOT_PARSE); + let mut child = common::batten() + .args(["adjudicate", "--harness", "exit-code"]) + .current_dir(&dir) + .env("BATTEN_HOOK_BYPASS", "1") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("the binary runs"); + child + .stdin + .as_mut() + .expect("stdin is piped") + .write_all(payload().as_bytes()) + .expect("the payload is writable"); + let output = child.wait_with_output().expect("the binary answers"); + assert_eq!( + output.status.code(), + Some(0), + "the declared hatch must still pass a call the engine cannot judge" + ); +} + +#[test] +fn on_claude_code_the_refusal_is_the_document_rather_than_the_number() { + // THE CHANNEL IS PER-HARNESS AND THE NUMBER IS NOT (`run_hook`'s own + // contract). This host reads the JSON decision object and ignores the code, + // so a deny raised as `2` here would be a refusal nobody receives — which is + // the reason this arm renders rather than raising a `Denial`. + let dir = fixture("adjudicate-document", WILL_NOT_PARSE); + let output = run_with_stdin( + dir.as_path(), + &["adjudicate", "--harness", "claude-code"], + &payload(), + ); + let rendered = String::from_utf8_lossy(&output.stdout); + // The row's falsifier names the field rather than the word: a `contains("deny")` + // would pass on a document that merely mentioned it, including one that said + // the opposite. + assert!( + rendered.contains(r#""permissionDecision":"deny""#), + "the decision object must carry the deny: {rendered}" + ); + // AND THE NUMBER IS FREE TO BE HONEST, which is the half that needs the + // document to exist. §6-§7 reserve `3` for could-not-look, and `exit.rs` + // keeps `Usage` and `Internal` the only codes a failure of Batten's own may + // produce *so that fail-open is structural*. Answering `2` here would buy the + // refusal a second time and spend that guarantee for the copy. + assert_eq!( + output.status.code(), + Some(3), + "where the document refuses, the number says nothing was judged" + ); +} + +#[test] +fn the_declaration_that_would_not_parse_is_named_without_quoting_it() { + // Non-negotiable rule 4, and the row asks for this clause by name: the reason + // carries the parse position, never the config's contents. The fixture's body + // is `this is not toml`, so its presence in the output would be the leak. + let dir = fixture("adjudicate-pointer-only", WILL_NOT_PARSE); + let output = run_with_stdin( + dir.as_path(), + &["adjudicate", "--harness", "claude-code"], + &payload(), + ); + let rendered = String::from_utf8_lossy(&output.stdout); + assert!( + !rendered.contains("this is not toml"), + "a refusal about an unreadable config must not quote it: {rendered}" + ); +} diff --git a/crates/batten/tests/it/cli.rs b/crates/batten/tests/it/cli.rs index 1a1df93e3..ce96741e7 100644 --- a/crates/batten/tests/it/cli.rs +++ b/crates/batten/tests/it/cli.rs @@ -2680,24 +2680,56 @@ fn hook_allows_when_no_authority_is_configured() { } #[test] -fn hook_fails_open_and_loud_on_an_unloadable_authority() { - // The opposite case, and the one CLOUD-40 could not reach because `hook` - // loaded no config: an authority that EXISTS and cannot be read means the - // rules the operator wrote are not being applied. Allowing silently there - // would be the false green the engine exists to catch, so it is a usage - // error — loud on stderr, exit 1, and structurally not a deny, because §7 - // spends 2 on the verdict alone. +fn hook_refuses_and_is_loud_on_an_unloadable_authority() { + // RENAMED FROM `hook_fails_open_and_loud_…`, because the first half stopped + // being true (CLOUD-1677) — and ONLY for this fixture's fault. + // + // The reasoning it was written on is what the row acted on: an authority that + // EXISTS and cannot be read means the operator's rules are not being applied, + // and allowing silently is the false green the engine exists to catch. What + // was wrong is that "loud" was the whole remedy — exit `1` is a NON-BLOCKING + // hook error, so the tool ran anyway and the loudness reached a log nobody + // gates on. Measured: 1,149 calls proceeded through seven windows of a + // mid-edit `batten.toml`, which is itself a protected path, so the gate + // guarding the config stopped guarding it during the one operation that + // changes it. + // + // **THIS FIXTURE IS `not toml at all`, WHICH IS THE WHOLE SCOPE.** A config + // that parses and merely fails a validator, or names a key this build + // predates, still answers `1` — every other row in it remains readable and + // enforceable, and an agent can still be told to repair the broken one. + // Refusing there would trade a working partial gate surface for nothing. + // Here there is no partial surface: nothing parsed, so nothing is enforced. let dir = repo_with_config("hook-broken-authority", "this is not toml at all\n"); for harness in harnesses() { let output = run_hook_in(&dir, harness, &claude_payload("gh pr view 42"), false); let code = output.status.code(); - assert_eq!(code, Some(1), "{harness}: an unreadable authority is usage"); - assert_ne!(code, Some(2), "{harness}: must never deny"); - assert!(output.stdout.is_empty(), "{harness}: no decision document"); + let stdout = String::from_utf8_lossy(&output.stdout); + // WHAT THIS CASE PROVES IS THAT NO HARNESS FAILS OPEN, and it deliberately + // does not assert the channel. Six protocols write six different decision + // documents, so a `permissionDecision` assertion inside this loop tests + // Claude Code's spelling five times and passes it off as coverage. The + // per-protocol detail — the document on a host that reads one, the `2` on + // the neutral adapter whose only channel is the number — belongs where it + // can be stated exactly, which is `adjudicate_absent.rs`. + // + // `1` was the whole defect: non-blocking, so the call ran. `0` would be a + // clean allow over an authority nothing could read. + assert!( + matches!(code, Some(2 | 3)), + "{harness}: a call under an unreadable authority must not proceed, got {code:?}" + ); + // NEVER SILENT, AND NEVER ANONYMOUS — the half of the original case that + // was always right, asked of whichever channel actually carried the + // refusal. The original asked stderr of every harness, which was true + // while the answer was always an exit-`1` diagnostic; now a document + // harness puts the reason in the document and leaves stderr empty, so + // asserting stderr alone would fail on the hosts that refuse best. let stderr = String::from_utf8_lossy(&output.stderr); + let spoken = format!("{stderr}{stdout}"); assert!( - stderr.contains("batten.toml"), - "{harness}: the failure names the file, got: {stderr}" + spoken.contains("batten.toml"), + "{harness}: the failure names the file, got: {spoken}" ); } } diff --git a/crates/batten/tests/it/commit_admission.rs b/crates/batten/tests/it/commit_admission.rs index dcdd47842..830729475 100644 --- a/crates/batten/tests/it/commit_admission.rs +++ b/crates/batten/tests/it/commit_admission.rs @@ -41,6 +41,10 @@ use common::{Fixture, git_in, run, stdout, write}; const GUARDED: &str = "guarded.toml"; const ORDINARY: &str = "notes.md"; +/// A protected path that ALSO has a `[[redirect]]` naming its sanctioned +/// mutation — the CLOUD-1303 set. +const SURFACED: &str = "memories/note.md"; + /// A fixture protecting one path, with a base commit that predates every case. /// /// `[commit]` is present because `commit check` refuses a config without one @@ -59,6 +63,29 @@ fn fixture(name: &str) -> PathBuf { .build() } +/// The same fixture, plus a second protected path that a `[[redirect]]` speaks +/// for (CLOUD-1303). +/// +/// **BOTH paths are protected and only one is redirected**, which is what makes +/// the third case below able to fail. A fixture where the redirect covered the +/// whole protected set would pass an implementation that dropped the demand +/// entirely. +fn fixture_with_redirect(name: &str) -> PathBuf { + Fixture::new(name) + .config(&format!( + "version = 1\nprotected = [\"{GUARDED}\", \"memories/**\"]\n\n\ + [[redirect]]\nglob = \"memories/**\"\n\ + mutation = \"use the memory tools\"\n\n\ + [commit]\nsubject_pattern = \"^(feat|fix|chore)(\\\\(.+\\\\))?!?: .+\"\n" + )) + .file(GUARDED, "original = 1\n") + .file(SURFACED, "original\n") + .file(ORDINARY, "just notes\n") + .git() + .base_commit() + .build() +} + /// Commit everything staged in `dir` with `message`, and return the range that /// covers exactly that commit. /// @@ -255,6 +282,78 @@ fn a_deleted_protected_path_owes_an_articulation_too() { ); } +#[test] +fn a_protected_path_with_a_sanctioned_mutation_owes_no_block() { + // CLOUD-1303, and the whole of it. A write through the surface the class + // declares is refused by nothing, so no admission is issued and there is + // nothing to articulate — while the override route's own precondition says it + // exists for when "writing the protected path directly is the only route + // left". Demanding a block here leaves an honest author a choice between a + // false articulation and not committing. + let dir = fixture_with_redirect("commit-admits-surfaced"); + write(&dir, SURFACED, "changed\n"); + let range = commit(&dir, "chore(memory): record something"); + let (code, report) = check(&dir, &range); + assert_eq!( + code, + Some(0), + "a path with a declared mutation surface owes no articulation: {report}" + ); +} + +#[test] +fn a_tampered_block_on_a_redirected_path_is_still_refused() { + // THE HALF THE SUPERSEDED FIX DROPPED, and the reason CLOUD-1303 refuted it. + // Exempting the path before inspecting it loses `admits-tampered` for exactly + // the exempted set: a doctored block on a memory would go unexamined. The + // redirect answers only the ABSENCE case; a block that claims the path is + // still verified. + let dir = fixture_with_redirect("commit-admits-surfaced-tampered"); + write(&dir, SURFACED, "changed\n"); + let block = articulate(&dir, SURFACED); + let doctored = block.replace( + "the owning surface cannot express it", + "a reason nobody articulated", + ); + assert_ne!(doctored, block, "the case must actually change the answer"); + let range = commit( + &dir, + &format!("chore(memory): record something\n\n{doctored}"), + ); + let (code, report) = check(&dir, &range); + assert_eq!( + code, + Some(2), + "a doctored block is refused whether or not the path has a surface: {report}" + ); + assert!( + report.contains(&format!("admits-tampered {SURFACED}")), + "the tampered finding keeps its whole subject set: {report}" + ); +} + +#[test] +fn a_redirect_for_one_path_does_not_excuse_another() { + // Without this, an implementation that dropped the demand for every protected + // path the moment ANY redirect was declared would pass the case above, and the + // clause would be off. The commit writes both; only the redirected one is + // excused. + let dir = fixture_with_redirect("commit-admits-surfaced-narrow"); + write(&dir, SURFACED, "changed\n"); + write(&dir, GUARDED, "original = 2\n"); + let range = commit(&dir, "chore(config): change both"); + let (code, report) = check(&dir, &range); + assert_eq!(code, Some(2), "the unredirected path still owes: {report}"); + assert!( + report.contains(&format!("admits {GUARDED}")), + "the finding names the path with no sanctioned surface: {report}" + ); + assert!( + !report.contains(SURFACED), + "the redirected path is not reported: {report}" + ); +} + #[test] fn the_clause_needs_no_store_to_decide() { // THE PROPERTY THAT MAKES THIS A CI TIER RATHER THAN A LOCAL ONE. The block diff --git a/crates/batten/tests/it/config_forward_compatible.rs b/crates/batten/tests/it/config_forward_compatible.rs index 69a9018e3..c1a623148 100644 --- a/crates/batten/tests/it/config_forward_compatible.rs +++ b/crates/batten/tests/it/config_forward_compatible.rs @@ -658,14 +658,33 @@ fn the_report_never_tells_the_reader_to_install_an_older_release() { /// fault from a well-formed row naming a key from a newer schema, and collapsing /// the two is what produced the defect — a prune that swallowed a syntax error /// would load "no rules configured" over a broken file. +/// +/// **AND SINCE CLOUD-1677 THE REFUSAL REACHES THE CALL.** The claim above is +/// unchanged and is exactly why: a newer-schema row leaves every other row +/// readable, so the file still decides and an agent can be told to repair the one +/// key. A file that is not TOML leaves nothing — zero rows readable, nothing +/// enforced — and a gate that fails open there is inert, which is the false green +/// the engine exists to catch. So this is the one config fault that denies. +/// +/// It was `1`: the usage code, non-blocking, and the `rm /` below simply ran. #[test] fn a_file_that_is_not_toml_is_still_refused() { let dir = repo("config-forward-broken", "[[rule\nbroken\n"); - let (code, _, stderr) = adjudicate(&dir); - assert_eq!(code, Some(1), "malformed TOML is a usage error: {stderr}"); + let (code, stdout, stderr) = adjudicate(&dir); + // THE DENY IS THE JSON, as the case above states for this harness. What + // differs here is the number beside it: with the document carrying the + // refusal, the code is free to say could-not-look — §6-§7's `3` — rather than + // claiming a verdict was reached. `exit.rs` keeps `Usage` and `Internal` the + // only codes a Batten failure produces, so refusing costs that guarantee + // nothing. + assert_eq!(code, Some(3), "nothing could be judged: {stdout} {stderr}"); + assert!( + stdout.contains(r#""permissionDecision":"deny""#), + "a call under an unreadable authority is refused: {stdout}" + ); assert!( - stderr.contains("TOML parse error"), - "the refusal says the file is not TOML: {stderr}" + format!("{stdout}{stderr}").contains("TOML parse error"), + "the refusal says the file is not TOML: {stdout} {stderr}" ); } diff --git a/crates/batten/tests/it/land_lap.rs b/crates/batten/tests/it/land_lap.rs index 173e11ac4..94af42b6e 100644 --- a/crates/batten/tests/it/land_lap.rs +++ b/crates/batten/tests/it/land_lap.rs @@ -248,9 +248,11 @@ fn every_lap_ending_is_reachable_from_some_step_and_code() { Some(TapVerdict::Red), Some(TapVerdict::Pending), ] { - let progress = land::progress_of(step, code, verdict); - if !seen.contains(&progress) { - seen.push(progress); + for basis in [land::Basis::Own, land::Basis::Borrowed] { + let progress = land::progress_of(step, code, verdict, basis); + if !seen.contains(&progress) { + seen.push(progress); + } } } } diff --git a/crates/batten/tests/it/land_speculation.rs b/crates/batten/tests/it/land_speculation.rs new file mode 100644 index 000000000..82fa1511b --- /dev/null +++ b/crates/batten/tests/it/land_speculation.rs @@ -0,0 +1,138 @@ +//! A live bet never reaches the publish (CLOUD-1681). +//! +//! # What this tier is for +//! +//! `mise-tasks/land.sh`'s invariant, quoted verbatim in `pipeline.rs`: +//! +//! > there is no path from a losing bet to a push, which is what makes +//! > speculating safe rather than merely fast. +//! +//! `Settle::Lost` has no such path — `Precheck::BetSettled` unwinds it at the top +//! of the lap. `Settle::Pending` had one, because that precheck deliberately +//! KEEPS a pending bet and the `Step::Push` row carried `precheck: None`. +//! Measured twice: four commits of another branch published under this one on +//! 2026-09-08, eight on 2026-09-10 — the second costing a full CI matrix on a +//! head that could not merge when it was graded. +//! +//! # Why a pending bet is exactly as unpublishable as a lost one +//! +//! A bet names the holder's SHAS, and the holder lands by rebase, which mints new +//! ones for the same patches. So a published pending head is not merely at risk +//! of going stale — the moment the holder lands it is DIVERGENT, and the +//! fast-forward is impossible by construction rather than by race. +//! +//! # The composition is the subject, not a lap +//! +//! Every case here decides over `Pipeline::default()` and `speculation::Bet`, +//! both of which are pure. Driving a real lap would need a remote, a lease and a +//! matrix to answer a question the step table already answers — and the defect +//! was never in the lap's execution, it was in a row that declared no question. + +// Panicking on a failed assertion is how a test fails loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use batten::land::Step; +use batten::pipeline::{Pipeline, Precheck}; +use batten::speculation::Bet; + +/// The row that publishes, and the question it now asks. +fn precheck_of(step: Step) -> Option { + Pipeline::default() + .steps + .iter() + .find(|row| row.step == step) + .and_then(|row| row.precheck) +} + +/// A bet that is outstanding, as `place_the_bet` leaves one. +fn placed() -> Bet { + Bet { + base: Some(String::from("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")), + undo: Some(String::from("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")), + main_at_bet: Some(String::from("cccccccccccccccccccccccccccccccccccccccc")), + ..Bet::default() + } +} + +/// **A LAP HOLDING A LIVE BET DOES NOT REACH THE PUSH.** +/// +/// The two halves of the reading are asserted together because either alone is +/// satisfiable by the defect: the row could declare the precheck while the bet +/// answers `false`, or the bet could be live over a row that asks nothing. The +/// pair is what the lap actually evaluates. +#[test] +fn a_live_bet_is_refused_by_the_row_that_would_publish_it() { + assert_eq!( + precheck_of(Step::Push), + Some(Precheck::BetLive), + "the row that publishes must ask whether this head is publishable" + ); + assert!( + placed().live(), + "a placed bet is outstanding, which is the state the precheck refuses" + ); +} + +/// **THE MIRROR — A SETTLED BET STILL PUSHES.** +/// +/// Without this the case above is satisfied by a pipeline that never publishes +/// at all, which is an outage rather than a fix. A holder that lands during +/// `verify` settles `Landed`, `drop_the_bet` forgets it, and the lap publishes +/// the pre-linearized head exactly as before — which is the whole value of +/// speculating and the reason this refuses `live()` rather than "a bet was ever +/// placed". +#[test] +fn a_settled_bet_still_reaches_the_push() { + let mut settled = placed(); + settled.forget(); + assert!( + !settled.live(), + "a settled bet is not outstanding, so the precheck admits the push" + ); +} + +/// **AND THE GATE DOES NOT SPREAD TO THE ROW THAT SPENDS THE GATE.** +/// +/// `Verify` must stay unguarded on the bet, or a speculating lap could never +/// verify the tree it speculated — which is the point of speculating. Asserting +/// only that `Push` carries a precheck would pass over a table that gated every +/// row, so this is where the arm's boundary is written down. +#[test] +fn the_verify_row_is_not_gated_on_the_bet() { + assert_eq!( + precheck_of(Step::Verify), + None, + "gating the gate on the bet would stop a speculating lap verifying at all" + ); + assert_eq!( + precheck_of(Step::Replay), + Some(Precheck::BetSettled), + "and the settle stays on Replay — the two rows ask different questions" + ); +} + +/// **THE TERMINATION PROPERTY, which the row's own §3 does not state.** +/// +/// The precheck answers `Lap`, and no `Compensation` drops a bet — `Nothing`, +/// `Redraft`, `Abandon`, `ReleaseLease`. `place_the_bet`'s own guards are about +/// the HOLDER (already landed; trunk has passed it), so without a memory of the +/// decision the next lap re-bets the same holder, arrives back at this row, and +/// unwinds again until the lap budget is spent — rather than the one extra local +/// verify and zero CI the design promises. +/// +/// `declined` is that memory, and it survives `forget` because `forget` is what +/// the unwind calls. +#[test] +fn a_declined_landing_does_not_speculate_again() { + let mut bet = placed(); + bet.declined = true; + bet.forget(); + assert!( + bet.declined, + "the decision must outlive the unwind that made it, or the lap spins" + ); + assert!( + !bet.live(), + "and the borrowed range is gone, which is what the next lap replays without" + ); +} diff --git a/crates/batten/tests/it/lease_health.rs b/crates/batten/tests/it/lease_health.rs index 65fb69cb8..a00d2796f 100644 --- a/crates/batten/tests/it/lease_health.rs +++ b/crates/batten/tests/it/lease_health.rs @@ -274,6 +274,7 @@ fn a_stalled_holder_is_stealable_once_its_beat_has_published() { next: String::new(), progress: progress.to_owned(), nonce: String::from("n1"), + ..Body::default() }; let observed = |progress: &str| Observed::Held { sha: "f".repeat(40), diff --git a/crates/batten/tests/it/lease_lifecycle.rs b/crates/batten/tests/it/lease_lifecycle.rs index f5f98f5fe..2920345f5 100644 --- a/crates/batten/tests/it/lease_lifecycle.rs +++ b/crates/batten/tests/it/lease_lifecycle.rs @@ -157,6 +157,7 @@ fn held(branch: &str, expires: i64) -> Body { next: String::new(), progress: String::from("verify"), nonce: String::from("n1"), + ..Body::default() } } diff --git a/crates/batten/tests/it/lease_namespace_premise.rs b/crates/batten/tests/it/lease_namespace_premise.rs new file mode 100644 index 000000000..ee6837ae7 --- /dev/null +++ b/crates/batten/tests/it/lease_namespace_premise.rs @@ -0,0 +1,120 @@ +//! The lease's ref-namespace premise is measured, not remembered (CLOUD-416). +//! +//! `lease::Terms` explains why the lease lives on `refs/heads`. For two years the +//! explanation carried two claims and only one was true: +//! +//! 1. *"this sandbox proxies git and its write policy refuses any push outside +//! `refs/heads`"* — **false**. Measured 2026-09-11 as a controlled pair, one +//! variable: the same parentless orphan pushed to the same `refs/sessions/` +//! exits `0` with `github.com` fenced and the PAT supplied through a credential +//! helper, and returns `HTTP 403` unfenced with the harness-injected token. +//! `refs/batten-probe/` behaved identically. The 403 is a credential that +//! is unscoped for the write; the namespace never came into it. +//! 2. *"GitHub also does not enforce the fast-forward rule off `refs/heads`"* — +//! **true**, and the same run confirmed it: the parentless orphan was accepted. +//! +//! # Why this is a test and not a comment +//! +//! Because it was a comment, and the comment was wrong, and the tree believed it. +//! CLOUD-416 records the lease being implemented four times with "two of those +//! passes existed only because the environment lied and nothing said so". A +//! premise that expensive earns an assertion; `rules/scanning.md`'s own rule is +//! that a claim worth designing around is worth something executable, and the +//! absence of one is itself the finding. +//! +//! # WHAT THIS ASSERTS, AND WHAT IT DELIBERATELY DOES NOT +//! +//! It asserts **presence and absence in the prose**: the refuted namespace claim +//! is gone, the surviving fast-forward reason is stated, and the measurement is +//! named so the next reader can re-run it rather than re-derive it. That is the +//! same shape as `scanner_taxonomy.rs` and `spawn_census.rs` — the prose carries +//! the position and the test keeps it from evaporating. +//! +//! It does **not** re-run the probe. A test that pushed to a real remote would +//! need a credential, would write to a shared forge from `cargo test`, and would +//! fail on every host that is not this one — which is the homogeneity fallacy +//! compiled into a test target. The probe is an operator procedure; this is the +//! ratchet that stops its answer being forgotten. + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use std::fs; + +use common::at_root; + +/// The module whose doc comment carries the premise. +const LEASE: &str = "crates/batten/src/lease.rs"; + +fn lease_source() -> String { + fs::read_to_string(at_root(LEASE)).expect("the lease module is readable") +} + +#[test] +fn the_refuted_namespace_claim_is_not_stated_anywhere_in_the_lease() { + // THE PREMISE CASE. The sentence below is what sent four implementations down + // the wrong path; if it returns in any spelling, this file is the thing that + // says so. + let source = lease_source(); + for refuted in [ + "refuses any push outside", + "write policy refuses", + "the day the proxy allows it", + ] { + assert!( + !source.contains(refuted), + "{LEASE} states the refuted namespace claim again ({refuted:?}); \ + the 403 is a credential, not a namespace — see CLOUD-416" + ); + } +} + +#[test] +fn the_surviving_reason_is_the_fast_forward_rule() { + // Deleting the false half must not take the true half with it. The lease is on + // `refs/heads` because the forge does not enforce fast-forward off it — and a + // reader who finds no reason at all will move the lease and lose that. + let source = lease_source(); + assert!( + source.contains("fast-forward"), + "{LEASE} must still state the fast-forward rule as the reason the lease \ + lives on refs/heads" + ); + assert!( + source.contains("parentless orphan"), + "{LEASE} must still name the orphan acceptance that measures it" + ); +} + +#[test] +fn the_measurement_is_named_so_it_can_be_rerun() { + // A correction that says only "this was wrong" leaves the next reader with a + // bare assertion to trust, which is the failure one layer over. Both arms of + // the controlled pair have to be legible. + let source = lease_source(); + assert!( + source.contains("403"), + "{LEASE} must record the unfenced arm's status" + ); + assert!( + source.contains("CLOUD-416"), + "{LEASE} must point at the row that owns the misdiagnosis" + ); +} + +#[test] +fn the_correction_does_not_generalise_the_fast_forward_loss_into_a_ban() { + // The old text concluded "which is the whole safety property gone", and that + // conclusion — drawn once about the lease — is what would refuse the log branch + // and the transcript refs if it were reused. One property, opposite signs, + // decided per structure. + let source = lease_source(); + assert!( + !source.contains("the whole safety property gone"), + "{LEASE} must not restate the over-general conclusion: a missing \ + fast-forward rule is a loss for a branch-shaped ref and the enabling \ + property for a content-addressed one" + ); +} diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index 70c2072bb..75eab3d9d 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -42,6 +42,7 @@ mod acquisition_metric; mod acquisition_sweep; mod address_resolve; mod address_transport; +mod adjudicate_absent; mod admission; mod admission_narrowing; mod advisory_drain; @@ -158,11 +159,13 @@ mod land_entry_gates; mod land_forge_reads; mod land_hand_stepping; mod land_lap; +mod land_speculation; mod land_verify_advice; mod landed_check; mod landing_roster; mod lease_health; mod lease_lifecycle; +mod lease_namespace_premise; mod lease_precondition; mod lease_record; mod locator_index; diff --git a/mise.toml b/mise.toml index 2673c3aa2..2e1feade9 100644 --- a/mise.toml +++ b/mise.toml @@ -617,7 +617,7 @@ CI_FANIN_WORKFLOW = ".github/workflows/ci.yml" BATS_TEST_TIMEOUT = "300" REGORUS_OPA_COMPLIANCE = "1.2.0" REGORUS_OPA_COMPLIANCE_FOR = "0.11" -MUTANT_GATES = "mise,attestation-check,engine-checks-green,engine-config,engine-doctor,engine-landed,engine-perf,engine-mcp,engine-pinned,engine-ready,engine-verdict,engine-wiring,engine-surface,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,cfg-gated-test,ci-cache-declared,ci-hygiene,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land-divergence-assert,landed-check,landing-loop,landing-roster-guarded,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,weakens-declared,worktree-registration,spawn-widening,nextest-slow,engine-lease,engine-handler,engine-speculation" +MUTANT_GATES = "mise,attestation-check,engine-checks-green,engine-config,engine-doctor,engine-landed,engine-perf,engine-mcp,engine-pinned,engine-ready,engine-verdict,engine-wiring,engine-surface,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,cfg-gated-test,ci-cache-declared,ci-hygiene,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land-divergence-assert,landed-check,landing-loop,landing-roster-guarded,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,weakens-declared,worktree-registration,spawn-widening,nextest-slow,engine-lease,engine-handler,engine-speculation,engine-pipeline" # --- GitHub reachability behind an egress proxy (Claude Code web sandbox etc.) --- # mise resolves every tool's release through GitHub's *API* host, api.github.com. diff --git a/rules/scanning.md b/rules/scanning.md index 5f290bee0..42cd82b93 100644 --- a/rules/scanning.md +++ b/rules/scanning.md @@ -101,6 +101,35 @@ forge, and a command-position pass over the same files, comments stripped, gave forge bucket because the string appeared in a **comment**. Both passes were an hour apart, and the substring one was nearly published as the campaign's scoping. +## A scanner that reads the file it lives in will match itself + +Row one's instrument has one failure mode that belongs to no other row, because +it only arises when the corpus and the scanner are the same file. A presence or +absence test written with `include_str!` over its own module — the shape +`spawn_census.rs` and `scanner_taxonomy.rs` use — is scanning its own source, +and **the needle it searches for is in that source.** + +Measured 2026-09-11, three times in one session, on two different files: + +- A test asserting a refuted claim cannot return **quoted the claim verbatim** in + its own doc, so it found the sentence in its own correction and failed. +- A test asserting no spawn names its program directly **named one in its doc** as + the example of what it forbids, and found that. +- The same test, with comments stripped, then matched **its own search literal** + in executable code. + +Each was the test working exactly as written. `git.rs` already carries the fix +and has for longer than this section: it assembles its needle from parts +(`["Command", "::new(\""].concat()`) so the string never appears whole. The +idiom was undocumented, so three tests rediscovered the defect instead of the +remedy. + +**So, for a scanner over its own file:** assemble the needle rather than spell it, +strip comments when the property is about code, and **paraphrase the thing you +forbid rather than quoting it** — a forbidden or refuted literal sitting in prose +is one a reader lifts out of its context, which is the same reason the lease's +own corrected premise is paraphrased rather than quoted. + ## Row four's subject is not the tree, which is why it kept getting answered from memory The first three questions are about the tree, and the instrument for each is a