diff --git a/.serena/memories/core.md b/.serena/memories/core.md index c81022ca3..426a27e94 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -955,6 +955,38 @@ NotComputable`, a third answer `Option` cannot express because it cannot tell the machine around it. Read CLOUD-780 for why a PARTIAL drop was refused — `reclaim` was the crate's only destructive path and its safety WAS the interlock a partial drop removes, so it was all four symbols or none. +- `patch.rs` — the in-process patch identity (CLOUD-739), and `git::landing`'s + sole supplier of one. It replaced `git log -p | git patch-id --stable` and, with + it, the twenty-six settings pinned around that pipeline — twenty `git config` + keys, six flags, two environment variables — whose whole purpose was stopping a + host's configuration from changing the answer. In process there is nothing to + pin, so all twenty-six are deleted and NOTHING replaces them. + **The deliverable is the normalisation being DECIDED, not the spawn being gone.** + A `PatchId` is only ever compared against one this same binary made in this same + run, so the requirement is _a_ canonical deterministic identity, never git's + (CLOUD-320 ruled that in writing) — and that licence is what turns four side + effects of tool choice into four choices with reasons. Line numbers stay + excluded, the one behaviour inherited deliberately, because hunk positions are + exactly what shift under the replay the primitive exists to recognise. + **Whitespace becomes SIGNIFICANT, diverging from git**: `patch-id` folds it, so + a whitespace-only difference collides, and the doc this replaced called that + collision _"the safe direction for a primitive whose failure class is a false not + landed"_ — backwards here, because a false LANDED is what suppresses + `completion.unlanded`'s finding. A spurious not-landed is noise, a spurious + landed is a lie. Binary content is identified by blob ids, which RETIRES the + `--binary` caveat (a zlib body _"deterministic for a given zlib but not + guaranteed across zlib builds"_) rather than restating it. Renames stay + undetected, now as a choice rather than as two flags that had to agree. + `imara-diff` is taken DIRECT and `gix-diff/blob` refused: `blob` is monolithic, + and its eight non-imara deps exist to run external diff drivers, clean/smudge + filters, and materialise blobs to disk — honouring `diff..command` would + hand back the very host-configuration input the twenty keys removed. + **The rename case is the CLOUD-418 lesson worth carrying**: rename detection is a + pure function of the two trees, so no fixture built out of trees can tell a + detecting build from a non-detecting one, and the test that claimed to gate it + could not go red. It is gated on the SHAPE instead + (`patch::tests::renames_are_not_a_shape_this_identity_can_take`), where the + mutation — a fourth `Kind` — fails the build with E0004. - `journal.rs` — the store's durable plumbing (CLOUD-78): append shards, a merged log with `(generation, seqno)` cursors, and the store-format version. Writers append to their **own** shard, so the concurrent path shares no mutable file and @@ -1474,6 +1506,27 @@ judge_fingerprint`, its own domain tag), so a caller can reference content it field a matched byte can occupy, and byte-stability is a property of the request SET rather than of the schedule, which is what makes it safe under CLOUD-850's concurrent acquisition. +- `symbols.rs` — the first `Cost::Effect` fact's acquisition (CLOUD-760). Where + a **name** resolves, asked of the compiler rather than of the text: the census + `.claude/rules/scanning.md` records three answers for — `grep` 14, a syntax + matcher 11, name resolution 9 — because `surface.rs` imports `clap::Command` + bare and no scanner can tell the two types apart. So this module delegates to + `cargo clippy --message-format=json`, with `--force-warn` overriding + `allow`/`expect` so an ENFORCEMENT lint reports as an INVENTORY without the + tree's annotations deciding what is counted. + It generalises `secrets.rs`'s adapter shape rather than copying it — binary + pinned, flags beside the parser, exit reconciled against the parse — and + carries that module's invariant verbatim: **clean is never inferred from a + stream that failed to parse**, so an unreadable stream is `CouldNotLook` and + never an empty census. `Provenance` (tool, version, invocation) travels inside + the fact because §6 byte-stability is a claim about a named producer; `Site` + is pointer-only per rule 4, a path, a line and the lint, with the path made + repository-relative so the answer does not depend on where the checkout sits. + Acquisition is the CALLER's: `rules::symbols_fact` resolves it once at the + boundary and only when a row declared it, and the projection is pure — a + projection that spawned would be the class's whole point undone. `Surface::Hook` + is refused (`tests/facts.rs`'s `no_effect_fact_is_hook_resolvable`), as a + census over `Fact::ALL` rather than an assertion about this one variant. - `policy.rs` — the policy evaluator (CLOUD-647, CLOUD-689): a `[[rule]]` of kind `policy` names a **registered** Rego module, and the module decides over the resolved fact set. It exists because `run` is a flat loop where no row diff --git a/Cargo.lock b/Cargo.lock index 3b39e64b7..06e0f8a81 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -127,9 +127,11 @@ dependencies = [ "fs4", "getrandom 0.4.3", "gix", + "gix-diff", "globset", "hmac", "ignore", + "imara-diff", "json5", "jsonschema", "proc-macro2", @@ -648,6 +650,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "foldhash" version = "0.2.0" @@ -1382,6 +1390,15 @@ dependencies = [ "byteorder", ] +[[package]] +name = "hashbrown" +version = "0.15.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" +dependencies = [ + "foldhash 0.1.5", +] + [[package]] name = "hashbrown" version = "0.16.1" @@ -1390,7 +1407,7 @@ checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" dependencies = [ "allocator-api2", "equivalent", - "foldhash", + "foldhash 0.2.0", ] [[package]] @@ -1401,7 +1418,7 @@ checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" dependencies = [ "allocator-api2", "equivalent", - "foldhash", + "foldhash 0.2.0", ] [[package]] @@ -1567,6 +1584,16 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "imara-diff" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f01d462f766df78ab820dd06f5eb700233c51f0f4c2e846520eaf4ba6aa5c5c" +dependencies = [ + "hashbrown 0.15.5", + "memchr", +] + [[package]] name = "indexmap" version = "2.14.0" diff --git a/Cargo.toml b/Cargo.toml index 8ec854b95..e185014ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -157,6 +157,45 @@ gix = { version = "0.86", default-features = false, features = [ "sha1", "sha256", ] } +# The TREE half of the in-process patch identity (CLOUD-739): which paths a +# commit changed, and the blob ids on each side. Already resolved here at this +# exact version as a non-optional dependency of `gix` itself, so naming it +# directly adds nothing to the closure — it makes reachable what was already +# linked. +# +# `default-features = false` is the whole point: the default enables `blob`, +# whose nine `dep:` entries are the external-diff-driver and worktree-materialising +# machinery described below. `pub mod tree` is ungated, which is why the tree half +# costs nothing and the blob half is bought separately and narrowly. +gix-diff = { version = "0.66", default-features = false } +# The diff algorithm behind the in-process patch identity (CLOUD-739). Hunks in, +# nothing else: it takes two byte slices and returns the edit script. No +# filesystem, no process, no threads, no global state. +# +# TAKEN DIRECTLY RATHER THAN THROUGH `gix/blob-diff`, and that is the whole +# decision. `gix-diff`'s `blob` feature is monolithic — nine `dep:` entries, no +# sub-features — and eight of them exist to reproduce git's full diff pipeline: +# `gix-command` runs external diff drivers (`diff..command`), +# `gix-filter` runs clean/smudge filters, and `gix-worktree`/`gix-tempfile`/ +# `gix-fs` materialise blobs to disk so those external programs can read them. +# Enabling it links the machinery whose purpose is to shell out and to touch the +# worktree, which is the surface `git.rs` is removing. `gix/blob-diff` is worse +# again: it is an umbrella that also drags `attributes`, and with it +# `gix-pathspec`, `gix-submodule` and `gix-ignore`. +# +# The narrower choice is also the CORRECT one, not merely the cheaper one. The +# implementation being replaced pins 20 `git config` keys for the sole purpose of +# stopping the host's configuration changing the answer; a diff layer that +# honours `diff..command` would hand that back. An identity a repository +# can reconfigure is not an identity. +# +# Blob CONTENTS reach it through gix's object database — the access CLOUD-738 +# landed — so this adds no filesystem path of its own. +# +# Closure measured in a scratch crate outside this tree, so measuring it did not +# vendor the thing being decided: `foldhash`, `hashbrown`, `memchr`, all three +# already resolved here. Apache-2.0, already in `deny.toml`'s allow-list. +imara-diff = { version = "0.2", default-features = false } # The policy evaluator (CLOUD-647, CLOUD-689). A `policy` row hands a REGISTERED # Rego module the resolved fact set and reads back denials; the module decides, # the engine never acquires. That is what CLOUD-763 admitted to `MediatedCall` diff --git a/batten.toml b/batten.toml index 96cdec9ed..07bd4b3e2 100644 --- a/batten.toml +++ b/batten.toml @@ -2083,6 +2083,38 @@ severity = "deny" scope = "tree" no_fix_reason = "an IO crate reaching the evaluator is closed where it was enabled, not here: `cargo tree -i ` names who turned it on, and the `regorus` feature list in Cargo.toml is where the pin is stated rather than where it is decided" +# The contract half of the config deprecation grammar (CLOUD-360). A key that +# leaves the published schema with no window breaks every consumer still carrying +# it, and breaks them SILENTLY: their config stops loading with an unknown-key +# error naming no successor and no date. +# +# A `command` row for `evaluator-closure-io-free`'s reason, one layer over. The +# subject is not this tree's text but the DIFFERENCE between two published +# surfaces, one of which lives at a git tag — no `forbid` over a line can see +# that, and the schema file itself is derived, so matching on it would gate the +# artifact rather than the change it records. +# +# Tree-scoped and spawning, so it runs under `enforce` and the hk gate and never +# on the mediated path. `glob` names the check's own file, matching the two rows +# above (CLOUD-614): the gate reads `schema/batten.schema.json` at a tag and the +# schema this build derives, so the glob decides WHEN the question is worth +# asking, and asking on a change to the gate itself is the case that must never +# be skipped. +# +# `deny` is earned rather than assumed. §7 required the predicate be replayed +# across history first, and it was: over all 112 release tags, zero would have +# reported a violation (85 clean, 27 exit 3 for tags predating the committed +# schema). A gate that would have refused past releases fires on work nobody can +# now fix; this one would not have. +[[rule]] +id = "no-key-leaves-the-schema-unannounced" +kind = "command" +glob = "mise-tasks/config-deprecations.sh" +check = "mise run config-deprecations" +severity = "deny" +scope = "tree" +no_fix_reason = "a removal is announced by declaring the window, not by editing the schema: add a row to `config::DEPRECATED_KEYS` naming the replacement and the expiry, and the derived artifact follows from the types" + # The first migrated gate of the bash-retirement campaign (CLOUD-843 track 2): # `run-shape-guard`'s no-message-source family, as a consumer-authored module # rather than a vendored preset. It names `git`, which a preset may not — a @@ -2240,6 +2272,24 @@ use_sources = ["crates/batten/src/*.rs"] module = "policy/module-layering.rego" severity = "deny" +# CLOUD-760's consumer, and the reason the fact is a fact rather than a facility. +# +# `symbols = true` is what pays for it: this is the first `Cost::Effect` fact, so +# resolving it RUNS the analyser over the crate — seconds, where every other fact +# costs milliseconds. Declared per row means a `check` that does not enable this +# rule spends nothing, which is the whole reason the column exists. +# +# NO `sources`. The census is the fact's, resolved once at the boundary over the +# crate the analyser compiles; a glob here would select files the module does not +# read and skip the row when the glob matched nothing. +[[rule]] +id = "spawn-adapters" +kind = "policy" +scope = "tree" +symbols = true +module = "policy/spawn-adapters.rego" +severity = "deny" + [[rule]] id = "opa-tracks-regorus-compliance" kind = "policy" diff --git a/bench/suites/RESULTS.md b/bench/suites/RESULTS.md index 01e933f8a..cfd822a93 100644 --- a/bench/suites/RESULTS.md +++ b/bench/suites/RESULTS.md @@ -6,167 +6,168 @@ runner measured it; the suite runs `--no-parallelize-within-files`, so a file's number is its own serial cost and is what an author adding a case to it pays. -- suites: 159 -- serial total: 950.4s +- suites: 160 +- serial total: 1382.4s | seconds | share | suite | | ---: | ---: | --- | -| 134.9 | 14.2% | `tests/land-lock.bats` | -| 101.9 | 10.7% | `tests/ci-wait.bats` | -| 82.8 | 8.7% | `tests/derived-check.bats` | -| 70.6 | 7.4% | `tests/session-start.bats` | -| 68.0 | 7.2% | `tests/land.bats` | -| 39.9 | 4.2% | `tests/hooks-wiring-check.bats` | -| 36.1 | 3.8% | `tests/helpers.bats` | -| 34.5 | 3.6% | `tests/main-watch.bats` | -| 30.3 | 3.2% | `tests/ci-local-parity.bats` | -| 24.2 | 2.5% | `tests/hook-latency-drift.bats` | -| 15.7 | 1.6% | `tests/token-bench.bats` | -| 14.4 | 1.5% | `tests/graph-check.bats` | -| 14.2 | 1.5% | `tests/sbom-check.bats` | -| 14.0 | 1.5% | `tests/claim-check.bats` | -| 12.3 | 1.3% | `tests/board-diff-overlap.bats` | -| 11.9 | 1.3% | `tests/board-write-record.bats` | -| 9.8 | 1.0% | `tests/prebuilt-lint.bats` | -| 9.1 | 1.0% | `tests/config-lint.bats` | -| 8.9 | 0.9% | `tests/stop-guard.bats` | -| 8.0 | 0.8% | `tests/target-race.bats` | -| 7.5 | 0.8% | `tests/filed-here-check.bats` | -| 6.7 | 0.7% | `tests/run-shape-guard.bats` | -| 6.2 | 0.7% | `tests/step-receipt.bats` | -| 6.2 | 0.6% | `tests/ready-lint.bats` | -| 6.0 | 0.6% | `tests/board-sweep.bats` | -| 5.7 | 0.6% | `tests/mutant.bats` | -| 5.5 | 0.6% | `tests/ready-guard.bats` | -| 5.2 | 0.6% | `tests/released.bats` | -| 5.1 | 0.5% | `tests/task-registry.bats` | -| 5.0 | 0.5% | `tests/singleton.bats` | -| 4.6 | 0.5% | `tests/renovate-config-validator.bats` | -| 4.5 | 0.5% | `tests/commit-convention.bats` | -| 4.4 | 0.5% | `tests/replay.bats` | -| 4.4 | 0.5% | `tests/release-tracking-check.bats` | -| 4.3 | 0.5% | `tests/lock-complete.bats` | -| 4.3 | 0.5% | `tests/mcp-allow-check.bats` | -| 4.2 | 0.4% | `tests/sbom.bats` | -| 3.6 | 0.4% | `tests/with-lock.bats` | -| 3.6 | 0.4% | `tests/target-ensure.bats` | -| 3.6 | 0.4% | `tests/doctor-race.bats` | -| 3.5 | 0.4% | `tests/release-assets-check.bats` | -| 3.4 | 0.4% | `tests/pre-commit-staging.bats` | -| 3.4 | 0.4% | `tests/in-progress-drain.bats` | -| 3.3 | 0.3% | `tests/hk-selection.bats` | -| 3.2 | 0.3% | `tests/schema-check.bats` | -| 2.9 | 0.3% | `tests/ready-cites-check.bats` | -| 2.7 | 0.3% | `tests/unlanded-check.bats` | -| 2.7 | 0.3% | `tests/ntia-check.bats` | -| 2.6 | 0.3% | `tests/reference-check.bats` | -| 2.5 | 0.3% | `tests/land-divergence.bats` | -| 2.4 | 0.3% | `tests/verify.bats` | -| 2.4 | 0.3% | `tests/semver.bats` | -| 2.2 | 0.2% | `tests/tree-clean.bats` | -| 2.0 | 0.2% | `tests/suite-select.bats` | -| 1.9 | 0.2% | `tests/timeout-drift.bats` | -| 1.9 | 0.2% | `tests/landed-check.bats` | -| 1.8 | 0.2% | `tests/closing-key-check.bats` | -| 1.7 | 0.2% | `tests/fanout-guard.bats` | -| 1.7 | 0.2% | `tests/claim-race-check.bats` | -| 1.5 | 0.2% | `tests/rules-drift.bats` | -| 1.5 | 0.2% | `tests/bot-issue.bats` | -| 1.5 | 0.2% | `tests/reclaim-census.bats` | -| 1.4 | 0.2% | `tests/ci-tools-check.bats` | -| 1.4 | 0.2% | `tests/finding-sink-check.bats` | -| 1.4 | 0.1% | `tests/ci-slow-needed.bats` | -| 1.4 | 0.1% | `tests/claimed-keys.bats` | -| 1.4 | 0.1% | `tests/spec-ref-check.bats` | -| 1.4 | 0.1% | `tests/signing-posture.bats` | -| 1.3 | 0.1% | `tests/skill-check.bats` | -| 1.2 | 0.1% | `tests/awk-regex-check.bats` | -| 1.2 | 0.1% | `tests/alive.bats` | -| 1.2 | 0.1% | `tests/ci-lease-precondition.bats` | +| 144.4 | 10.4% | `tests/land-lock.bats` | +| 140.1 | 10.1% | `tests/session-start.bats` | +| 118.3 | 8.6% | `tests/derived-check.bats` | +| 102.4 | 7.4% | `tests/ci-wait.bats` | +| 79.9 | 5.8% | `tests/land.bats` | +| 60.3 | 4.4% | `tests/hooks-wiring-check.bats` | +| 54.9 | 4.0% | `tests/commit-convention.bats` | +| 52.0 | 3.8% | `tests/ci-local-parity.bats` | +| 51.4 | 3.7% | `tests/prebuilt-lint.bats` | +| 39.0 | 2.8% | `tests/config-deprecations.bats` | +| 34.7 | 2.5% | `tests/main-watch.bats` | +| 32.6 | 2.4% | `tests/sbom-check.bats` | +| 26.8 | 1.9% | `tests/config-lint.bats` | +| 26.1 | 1.9% | `tests/helpers.bats` | +| 24.3 | 1.8% | `tests/hook-latency-drift.bats` | +| 23.7 | 1.7% | `tests/claim-check.bats` | +| 18.4 | 1.3% | `tests/graph-check.bats` | +| 17.1 | 1.2% | `tests/token-bench.bats` | +| 14.9 | 1.1% | `tests/reference-check.bats` | +| 14.3 | 1.0% | `tests/board-diff-overlap.bats` | +| 13.3 | 1.0% | `tests/board-write-record.bats` | +| 12.3 | 0.9% | `tests/stop-guard.bats` | +| 11.0 | 0.8% | `tests/ready-guard.bats` | +| 10.4 | 0.8% | `tests/run-shape-guard.bats` | +| 9.0 | 0.6% | `tests/filed-here-check.bats` | +| 8.9 | 0.6% | `tests/ready-lint.bats` | +| 8.9 | 0.6% | `tests/step-receipt.bats` | +| 8.4 | 0.6% | `tests/target-race.bats` | +| 7.0 | 0.5% | `tests/ready-cites-check.bats` | +| 6.9 | 0.5% | `tests/released.bats` | +| 6.9 | 0.5% | `tests/board-sweep.bats` | +| 6.7 | 0.5% | `tests/mutant.bats` | +| 6.4 | 0.5% | `tests/lock-complete.bats` | +| 6.3 | 0.5% | `tests/renovate-config-validator.bats` | +| 6.1 | 0.4% | `tests/sbom.bats` | +| 5.8 | 0.4% | `tests/singleton.bats` | +| 5.5 | 0.4% | `tests/replay.bats` | +| 5.5 | 0.4% | `tests/release-tracking-check.bats` | +| 5.3 | 0.4% | `tests/mcp-allow-check.bats` | +| 5.2 | 0.4% | `tests/task-registry.bats` | +| 4.8 | 0.3% | `tests/in-progress-drain.bats` | +| 4.4 | 0.3% | `tests/schema-check.bats` | +| 4.1 | 0.3% | `tests/skill-check.bats` | +| 4.0 | 0.3% | `tests/pre-commit-staging.bats` | +| 3.9 | 0.3% | `tests/release-assets-check.bats` | +| 3.8 | 0.3% | `tests/target-ensure.bats` | +| 3.7 | 0.3% | `tests/signing-posture.bats` | +| 3.7 | 0.3% | `tests/doctor-race.bats` | +| 3.6 | 0.3% | `tests/semver.bats` | +| 3.5 | 0.3% | `tests/suite-select.bats` | +| 3.5 | 0.2% | `tests/ntia-check.bats` | +| 3.4 | 0.2% | `tests/with-lock.bats` | +| 3.4 | 0.2% | `tests/spec-ref-check.bats` | +| 3.4 | 0.2% | `tests/hk-selection.bats` | +| 3.3 | 0.2% | `tests/land-divergence.bats` | +| 3.3 | 0.2% | `tests/ready-lint-deferral.bats` | +| 3.2 | 0.2% | `tests/unlanded-check.bats` | +| 2.8 | 0.2% | `tests/tree-clean.bats` | +| 2.6 | 0.2% | `tests/verify.bats` | +| 2.4 | 0.2% | `tests/reclaim-census.bats` | +| 2.3 | 0.2% | `tests/run-shape.bats` | +| 2.3 | 0.2% | `tests/fanout-guard.bats` | +| 2.2 | 0.2% | `tests/landed-check.bats` | +| 2.2 | 0.2% | `tests/rules-drift.bats` | +| 2.1 | 0.2% | `tests/spawn-census.bats` | +| 2.0 | 0.1% | `tests/closing-key-check.bats` | +| 2.0 | 0.1% | `tests/claim-race-check.bats` | +| 1.9 | 0.1% | `tests/finding-sink-check.bats` | +| 1.9 | 0.1% | `tests/target-prune.bats` | +| 1.7 | 0.1% | `tests/bot-issue.bats` | +| 1.6 | 0.1% | `tests/ci-tools-check.bats` | +| 1.6 | 0.1% | `tests/claimed-keys.bats` | +| 1.5 | 0.1% | `tests/ci-slow-needed.bats` | +| 1.5 | 0.1% | `tests/prose-only-check.bats` | +| 1.5 | 0.1% | `tests/timeout-drift.bats` | +| 1.3 | 0.1% | `tests/ci-lease-precondition.bats` | +| 1.3 | 0.1% | `tests/memories-check.bats` | +| 1.3 | 0.1% | `tests/stop-posture-check.bats` | +| 1.2 | 0.1% | `tests/mutant-census.bats` | | 1.2 | 0.1% | `tests/verified.bats` | -| 1.1 | 0.1% | `tests/memories-check.bats` | -| 1.1 | 0.1% | `tests/ready-lint-deferral.bats` | -| 1.1 | 0.1% | `tests/stop-posture-check.bats` | -| 1.1 | 0.1% | `tests/run-shape.bats` | -| 1.0 | 0.1% | `tests/perf-record.bats` | -| 1.0 | 0.1% | `tests/target-prune.bats` | -| 1.0 | 0.1% | `tests/suite-bench-check.bats` | -| 0.9 | 0.1% | `tests/mutant-census.bats` | -| 0.9 | 0.1% | `tests/done-check.bats` | +| 1.2 | 0.1% | `tests/awk-regex-check.bats` | +| 1.2 | 0.1% | `tests/publish-credential-check.bats` | +| 1.2 | 0.1% | `tests/suite-bench-check.bats` | +| 1.1 | 0.1% | `tests/alive.bats` | +| 1.1 | 0.1% | `tests/install-check.bats` | +| 1.1 | 0.1% | `tests/land-divergence-assert.bats` | +| 1.1 | 0.1% | `tests/perf-record.bats` | +| 1.1 | 0.1% | `tests/deferral-check.bats` | +| 1.1 | 0.1% | `tests/done-check.bats` | +| 1.0 | 0.1% | `tests/privileged-lane.bats` | +| 1.0 | 0.1% | `tests/nonverdict-scan.bats` | +| 1.0 | 0.1% | `tests/linear-check.bats` | +| 0.9 | 0.1% | `tests/sonar-gate.bats` | | 0.9 | 0.1% | `tests/attestation-check.bats` | -| 0.9 | 0.1% | `tests/deferral-check.bats` | -| 0.8 | 0.1% | `tests/linear-check.bats` | -| 0.8 | 0.1% | `tests/spawn-census.bats` | -| 0.8 | 0.1% | `tests/land-divergence-assert.bats` | -| 0.8 | 0.1% | `tests/install-check.bats` | -| 0.8 | 0.1% | `tests/nonverdict-scan.bats` | -| 0.8 | 0.1% | `tests/release-backfill.bats` | -| 0.8 | 0.1% | `tests/transcript-corpus-check.bats` | -| 0.7 | 0.1% | `tests/checks-green.bats` | -| 0.7 | 0.1% | `tests/done-pr-check.bats` | +| 0.9 | 0.1% | `tests/release-backfill.bats` | +| 0.9 | 0.1% | `tests/sbom-binary.bats` | +| 0.9 | 0.1% | `tests/transcript-corpus-check.bats` | +| 0.8 | 0.1% | `tests/gh-guard.bats` | +| 0.8 | 0.1% | `tests/done-pr-check.bats` | +| 0.8 | 0.1% | `tests/perf-assert.bats` | +| 0.8 | 0.1% | `tests/module-map-check.bats` | +| 0.8 | 0.1% | `tests/render-cli.bats` | +| 0.8 | 0.1% | `tests/doctor.bats` | +| 0.8 | 0.1% | `tests/lint-deno.bats` | +| 0.8 | 0.1% | `tests/checks-green.bats` | +| 0.8 | 0.1% | `tests/pr-unsubscribed.bats` | +| 0.7 | 0.1% | `tests/hook-pin-check.bats` | | 0.7 | 0.1% | `tests/timeout-check.bats` | -| 0.7 | 0.1% | `tests/doctor.bats` | -| 0.7 | 0.1% | `tests/module-map-check.bats` | -| 0.7 | 0.1% | `tests/gh-guard.bats` | -| 0.7 | 0.1% | `tests/perf-assert.bats` | -| 0.6 | 0.1% | `tests/prose-only-check.bats` | -| 0.6 | 0.1% | `tests/pr-unsubscribed.bats` | -| 0.6 | 0.1% | `tests/render-cli.bats` | -| 0.6 | 0.1% | `tests/lint-deno.bats` | -| 0.5 | 0.1% | `tests/hook-pin-check.bats` | -| 0.5 | 0.1% | `tests/merged-pr-keys.bats` | -| 0.5 | 0.1% | `tests/mcp-attach-check.bats` | -| 0.5 | 0.1% | `tests/duplicate-close-check.bats` | -| 0.5 | 0.1% | `tests/hook-matcher-check.bats` | -| 0.5 | 0.1% | `tests/sbom-binary.bats` | -| 0.5 | 0.1% | `tests/mcp-timeout-budget.bats` | -| 0.5 | 0.1% | `tests/evaluator-closure-check.bats` | -| 0.5 | 0.0% | `tests/checksums.bats` | -| 0.5 | 0.0% | `tests/perf-compare.bats` | -| 0.5 | 0.0% | `tests/install.bats` | -| 0.4 | 0.0% | `tests/board-payloads.bats` | -| 0.4 | 0.0% | `tests/branch-age-check.bats` | -| 0.4 | 0.0% | `tests/connector-verb-guard.bats` | -| 0.4 | 0.0% | `tests/publish-credential-check.bats` | -| 0.4 | 0.0% | `tests/hook-profile-check.bats` | +| 0.7 | 0.1% | `tests/duplicate-close-check.bats` | +| 0.7 | 0.0% | `tests/hook-matcher-check.bats` | +| 0.7 | 0.0% | `tests/mcp-timeout-budget.bats` | +| 0.7 | 0.0% | `tests/merged-pr-keys.bats` | +| 0.6 | 0.0% | `tests/mcp-attach-check.bats` | +| 0.6 | 0.0% | `tests/perf-compare.bats` | +| 0.6 | 0.0% | `tests/install.bats` | +| 0.6 | 0.0% | `tests/macos-link-check.bats` | +| 0.6 | 0.0% | `tests/hook-profile-check.bats` | +| 0.6 | 0.0% | `tests/evaluator-closure-check.bats` | +| 0.6 | 0.0% | `tests/checksums.bats` | +| 0.6 | 0.0% | `tests/connector-verb-guard.bats` | +| 0.5 | 0.0% | `tests/lint-rego.bats` | +| 0.5 | 0.0% | `tests/board-payloads.bats` | +| 0.5 | 0.0% | `tests/msrv-pin-agreement.bats` | | 0.4 | 0.0% | `tests/abandon-matrix.bats` | -| 0.4 | 0.0% | `tests/macos-link-check.bats` | -| 0.4 | 0.0% | `tests/lint-rego.bats` | -| 0.4 | 0.0% | `tests/connector-allow-guard.bats` | -| 0.4 | 0.0% | `tests/token-bench-check.bats` | | 0.4 | 0.0% | `tests/digest-major-agreement.bats` | -| 0.3 | 0.0% | `tests/pipefail-grep-check.bats` | -| 0.3 | 0.0% | `tests/license-table-check.bats` | -| 0.3 | 0.0% | `tests/test-bats-parallel.bats` | -| 0.3 | 0.0% | `tests/msrv-pin-agreement.bats` | -| 0.3 | 0.0% | `tests/land-lock-check.bats` | -| 0.3 | 0.0% | `tests/cap-drift.bats` | +| 0.4 | 0.0% | `tests/pipefail-grep-check.bats` | +| 0.4 | 0.0% | `tests/connector-allow-guard.bats` | +| 0.4 | 0.0% | `tests/run-shape-guard-quoting.bats` | +| 0.4 | 0.0% | `tests/serena-mcp.bats` | +| 0.4 | 0.0% | `tests/branch-age-check.bats` | +| 0.4 | 0.0% | `tests/land-lock-check.bats` | +| 0.4 | 0.0% | `tests/task-fail-closed.bats` | +| 0.4 | 0.0% | `tests/pkl-check.bats` | +| 0.4 | 0.0% | `tests/nonverdict-assert.bats` | +| 0.3 | 0.0% | `tests/token-bench-check.bats` | +| 0.3 | 0.0% | `tests/commit-attribution.bats` | +| 0.3 | 0.0% | `tests/no-doctests.bats` | +| 0.3 | 0.0% | `tests/report-only-check.bats` | | 0.3 | 0.0% | `tests/batten-glob-check.bats` | -| 0.3 | 0.0% | `tests/sonar-gate.bats` | -| 0.3 | 0.0% | `tests/task-fail-closed.bats` | -| 0.3 | 0.0% | `tests/serena-mcp.bats` | | 0.3 | 0.0% | `tests/release-due.bats` | -| 0.3 | 0.0% | `tests/commit-attribution.bats` | -| 0.3 | 0.0% | `tests/run-shape-guard-quoting.bats` | -| 0.2 | 0.0% | `tests/privileged-lane.bats` | -| 0.2 | 0.0% | `tests/nonverdict-assert.bats` | -| 0.2 | 0.0% | `tests/no-doctests.bats` | -| 0.2 | 0.0% | `tests/ci-drift.bats` | -| 0.2 | 0.0% | `tests/report-only-check.bats` | -| 0.2 | 0.0% | `tests/container-preflight.bats` | -| 0.2 | 0.0% | `tests/connector-allow-resolve.bats` | -| 0.2 | 0.0% | `tests/pkl-check.bats` | -| 0.2 | 0.0% | `tests/coderabbit-config-check.bats` | -| 0.2 | 0.0% | `tests/mise-pin-agreement.bats` | -| 0.2 | 0.0% | `tests/git-hook.bats` | +| 0.3 | 0.0% | `tests/cap-drift.bats` | +| 0.3 | 0.0% | `tests/connector-allow-resolve.bats` | +| 0.3 | 0.0% | `tests/ci-drift.bats` | +| 0.3 | 0.0% | `tests/license-table-check.bats` | +| 0.3 | 0.0% | `tests/container-preflight.bats` | +| 0.3 | 0.0% | `tests/git-hook.bats` | +| 0.3 | 0.0% | `tests/mise-pin-agreement.bats` | +| 0.3 | 0.0% | `tests/rust-paths-check.bats` | +| 0.3 | 0.0% | `tests/coderabbit-config-check.bats` | +| 0.2 | 0.0% | `tests/test-bats-parallel.bats` | | 0.2 | 0.0% | `tests/mise-action-floor.bats` | -| 0.2 | 0.0% | `tests/rust-paths-check.bats` | +| 0.2 | 0.0% | `tests/perf-gate.bats` | +| 0.2 | 0.0% | `tests/remedy-payload-source.bats` | | 0.2 | 0.0% | `tests/dist.bats` | -| 0.1 | 0.0% | `tests/perf-gate.bats` | -| 0.1 | 0.0% | `tests/remedy-payload-source.bats` | +| 0.1 | 0.0% | `tests/evaluator-io-check.bats` | | 0.1 | 0.0% | `tests/egress-check.bats` | | 0.1 | 0.0% | `tests/perf-pair.bats` | -| 0.1 | 0.0% | `tests/evaluator-io-check.bats` | | 0.1 | 0.0% | `tests/zizmor-split.bats` | | 0.1 | 0.0% | `tests/darwin-link.bats` | -| 0.0 | 0.0% | `tests/cross-check.bats` | +| 0.1 | 0.0% | `tests/cross-check.bats` | diff --git a/completions/batten.bash b/completions/batten.bash index 562960d61..a28350a07 100644 --- a/completions/batten.bash +++ b/completions/batten.bash @@ -139,6 +139,9 @@ _batten() { batten__subcmd__commit__subcmd__help,help) cmd="batten__subcmd__commit__subcmd__help__subcmd__help" ;; + batten__subcmd__config,deprecations) + cmd="batten__subcmd__config__subcmd__deprecations" + ;; batten__subcmd__config,epoch) cmd="batten__subcmd__config__subcmd__epoch" ;; @@ -151,6 +154,9 @@ _batten() { batten__subcmd__config,show) cmd="batten__subcmd__config__subcmd__show" ;; + batten__subcmd__config__subcmd__help,deprecations) + cmd="batten__subcmd__config__subcmd__help__subcmd__deprecations" + ;; batten__subcmd__config__subcmd__help,epoch) cmd="batten__subcmd__config__subcmd__help__subcmd__epoch" ;; @@ -328,6 +334,9 @@ _batten() { batten__subcmd__help__subcmd__commit,check) cmd="batten__subcmd__help__subcmd__commit__subcmd__check" ;; + batten__subcmd__help__subcmd__config,deprecations) + cmd="batten__subcmd__help__subcmd__config__subcmd__deprecations" + ;; batten__subcmd__help__subcmd__config,epoch) cmd="batten__subcmd__help__subcmd__config__subcmd__epoch" ;; @@ -1038,7 +1047,7 @@ _batten() { return 0 ;; batten__subcmd__config) - opts="-q -v -y -h --strictness --fail-on-warning --config-from --silent --quiet --verbose --debug --trace --log-level --no-color --no-input --yes --help show epoch lint help" + opts="-q -v -y -h --strictness --fail-on-warning --config-from --silent --quiet --verbose --debug --trace --log-level --no-color --no-input --yes --help show epoch deprecations lint help" if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 @@ -1063,6 +1072,32 @@ _batten() { COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 ;; + batten__subcmd__config__subcmd__deprecations) + opts="-J -q -v -y -h --json --strictness --fail-on-warning --config-from --silent --quiet --verbose --debug --trace --log-level --no-color --no-input --yes --help" + if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then + COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) + return 0 + fi + case "${prev}" in + --strictness) + COMPREPLY=($(compgen -W "permissive standard strict" -- "${cur}")) + return 0 + ;; + --config-from) + COMPREPLY=($(compgen -f "${cur}")) + return 0 + ;; + --log-level) + COMPREPLY=($(compgen -W "silent quiet normal verbose debug trace" -- "${cur}")) + return 0 + ;; + *) + COMPREPLY=() + ;; + esac + COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) + return 0 + ;; batten__subcmd__config__subcmd__epoch) opts="-J -q -v -y -h --json --no-cache --strictness --fail-on-warning --config-from --silent --quiet --verbose --debug --trace --log-level --no-color --no-input --yes --help" if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then @@ -1090,7 +1125,7 @@ _batten() { return 0 ;; batten__subcmd__config__subcmd__help) - opts="show epoch lint help" + opts="show epoch deprecations lint help" if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 @@ -1103,6 +1138,20 @@ _batten() { COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 ;; + batten__subcmd__config__subcmd__help__subcmd__deprecations) + opts="" + if [[ ${cur} == -* || ${COMP_CWORD} -eq 4 ]] ; then + COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) + return 0 + fi + case "${prev}" in + *) + COMPREPLY=() + ;; + esac + COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) + return 0 + ;; batten__subcmd__config__subcmd__help__subcmd__epoch) opts="" if [[ ${cur} == -* || ${COMP_CWORD} -eq 4 ]] ; then @@ -2044,7 +2093,7 @@ _batten() { return 0 ;; batten__subcmd__help__subcmd__config) - opts="show epoch lint" + opts="show epoch deprecations lint" if [[ ${cur} == -* || ${COMP_CWORD} -eq 3 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 @@ -2057,6 +2106,20 @@ _batten() { COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 ;; + batten__subcmd__help__subcmd__config__subcmd__deprecations) + opts="" + if [[ ${cur} == -* || ${COMP_CWORD} -eq 4 ]] ; then + COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) + return 0 + fi + case "${prev}" in + *) + COMPREPLY=() + ;; + esac + COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) + return 0 + ;; batten__subcmd__help__subcmd__config__subcmd__epoch) opts="" if [[ ${cur} == -* || ${COMP_CWORD} -eq 4 ]] ; then diff --git a/completions/batten.fish b/completions/batten.fish index d3f468534..a76bedb2d 100644 --- a/completions/batten.fish +++ b/completions/batten.fish @@ -241,30 +241,31 @@ complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_s complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from help" -f -a "list" -d 'List this repository\'s captures as handles, in a fixed order' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from help" -f -a "prune" -d 'Remove this repository\'s captures — the one removal path; captures never expire on their own' complete -c batten -n "__fish_batten_using_subcommand capture; and __fish_seen_subcommand_from help" -f -a "help" -d 'Print this message or the help of the given subcommand(s)' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l strictness -d 'Raise how strictly gates apply (an override may only tighten policy)' -r -f -a "permissive\t'Advisory: findings are reported without failing the run' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l strictness -d 'Raise how strictly gates apply (an override may only tighten policy)' -r -f -a "permissive\t'Advisory: findings are reported without failing the run' standard\t'The default: a finding is a violation' strict\t'Everything `Standard` fails on, plus anything advisory'" -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l config-from -d 'Read the committed config from a git ref (e.g. origin/main) instead of the working tree' -r -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l log-level -d 'Set the verbosity rung by name' -r -f -a "silent\t'Say nothing but a verdict or a usage error' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l config-from -d 'Read the committed config from a git ref (e.g. origin/main) instead of the working tree' -r +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l log-level -d 'Set the verbosity rung by name' -r -f -a "silent\t'Say nothing but a verdict or a usage error' quiet\t'Suppress ordinary progress; keep warnings' normal\t'The default' verbose\t'Explain what is being checked' debug\t'Add resolution detail' trace\t'Add everything'" -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l fail-on-warning -d 'Promote a warn-severity finding to a violation (an override may only turn this on)' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l silent -d 'Say nothing but a verdict or a usage error' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -s q -l quiet -d 'Suppress ordinary progress (repeatable: -qq is silent)' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -s v -l verbose -d 'Explain what is being checked (repeatable: -vv is debug)' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l debug -d 'Add resolution detail' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l trace -d 'Add everything' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l no-color -d 'Never colour stderr, whatever it is attached to' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -l no-input -d 'Never prompt; treat the run as unattended' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -s y -l yes -d 'Confirm a destructive operation that would otherwise refuse' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -s h -l help -d 'Print help (see more with \'--help\')' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -f -a "show" -d 'Print the effective configuration' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -f -a "epoch" -d 'Print the content hash of the governing config surface' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -f -a "lint" -d 'Report policy smells in batten.toml (any smell is a violation)' -complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch lint help" -f -a "help" -d 'Print this message or the help of the given subcommand(s)' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l fail-on-warning -d 'Promote a warn-severity finding to a violation (an override may only turn this on)' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l silent -d 'Say nothing but a verdict or a usage error' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -s q -l quiet -d 'Suppress ordinary progress (repeatable: -qq is silent)' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -s v -l verbose -d 'Explain what is being checked (repeatable: -vv is debug)' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l debug -d 'Add resolution detail' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l trace -d 'Add everything' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l no-color -d 'Never colour stderr, whatever it is attached to' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -l no-input -d 'Never prompt; treat the run as unattended' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -s y -l yes -d 'Confirm a destructive operation that would otherwise refuse' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -s h -l help -d 'Print help (see more with \'--help\')' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -f -a "show" -d 'Print the effective configuration' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -f -a "epoch" -d 'Print the content hash of the governing config surface' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -f -a "deprecations" -d 'Report schema keys removed since a published release with no deprecation window' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -f -a "lint" -d 'Report policy smells in batten.toml (any smell is a violation)' +complete -c batten -n "__fish_batten_using_subcommand config; and not __fish_seen_subcommand_from show epoch deprecations lint help" -f -a "help" -d 'Print this message or the help of the given subcommand(s)' complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from show" -l strictness -d 'Raise how strictly gates apply (an override may only tighten policy)' -r -f -a "permissive\t'Advisory: findings are reported without failing the run' standard\t'The default: a finding is a violation' strict\t'Everything `Standard` fails on, plus anything advisory'" @@ -308,6 +309,27 @@ complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_su complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from epoch" -l no-input -d 'Never prompt; treat the run as unattended' complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from epoch" -s y -l yes -d 'Confirm a destructive operation that would otherwise refuse' complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from epoch" -s h -l help -d 'Print help (see more with \'--help\')' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l strictness -d 'Raise how strictly gates apply (an override may only tighten policy)' -r -f -a "permissive\t'Advisory: findings are reported without failing the run' +standard\t'The default: a finding is a violation' +strict\t'Everything `Standard` fails on, plus anything advisory'" +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l config-from -d 'Read the committed config from a git ref (e.g. origin/main) instead of the working tree' -r +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l log-level -d 'Set the verbosity rung by name' -r -f -a "silent\t'Say nothing but a verdict or a usage error' +quiet\t'Suppress ordinary progress; keep warnings' +normal\t'The default' +verbose\t'Explain what is being checked' +debug\t'Add resolution detail' +trace\t'Add everything'" +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -s J -l json -d 'Emit byte-stable JSON instead of pointer lines' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l fail-on-warning -d 'Promote a warn-severity finding to a violation (an override may only turn this on)' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l silent -d 'Say nothing but a verdict or a usage error' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -s q -l quiet -d 'Suppress ordinary progress (repeatable: -qq is silent)' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -s v -l verbose -d 'Explain what is being checked (repeatable: -vv is debug)' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l debug -d 'Add resolution detail' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l trace -d 'Add everything' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l no-color -d 'Never colour stderr, whatever it is attached to' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -l no-input -d 'Never prompt; treat the run as unattended' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -s y -l yes -d 'Confirm a destructive operation that would otherwise refuse' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from deprecations" -s h -l help -d 'Print help (see more with \'--help\')' complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from lint" -l host-rules -d 'Compare the committed [ci] table against a host ruleset payload (path, or - for stdin)' -r complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from lint" -l strictness -d 'Raise how strictly gates apply (an override may only tighten policy)' -r -f -a "permissive\t'Advisory: findings are reported without failing the run' standard\t'The default: a finding is a violation' @@ -332,6 +354,7 @@ complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_su complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from lint" -s h -l help -d 'Print help (see more with \'--help\')' complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from help" -f -a "show" -d 'Print the effective configuration' complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from help" -f -a "epoch" -d 'Print the content hash of the governing config surface' +complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from help" -f -a "deprecations" -d 'Report schema keys removed since a published release with no deprecation window' complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from help" -f -a "lint" -d 'Report policy smells in batten.toml (any smell is a violation)' complete -c batten -n "__fish_batten_using_subcommand config; and __fish_seen_subcommand_from help" -f -a "help" -d 'Print this message or the help of the given subcommand(s)' complete -c batten -n "__fish_batten_using_subcommand lint; and not __fish_seen_subcommand_from brief help" -l strictness -d 'Raise how strictly gates apply (an override may only tighten policy)' -r -f -a "permissive\t'Advisory: findings are reported without failing the run' @@ -1350,6 +1373,7 @@ complete -c batten -n "__fish_batten_using_subcommand help; and __fish_seen_subc complete -c batten -n "__fish_batten_using_subcommand help; and __fish_seen_subcommand_from capture" -f -a "prune" -d 'Remove this repository\'s captures — the one removal path; captures never expire on their own' complete -c batten -n "__fish_batten_using_subcommand help; and __fish_seen_subcommand_from config" -f -a "show" -d 'Print the effective configuration' complete -c batten -n "__fish_batten_using_subcommand help; and __fish_seen_subcommand_from config" -f -a "epoch" -d 'Print the content hash of the governing config surface' +complete -c batten -n "__fish_batten_using_subcommand help; and __fish_seen_subcommand_from config" -f -a "deprecations" -d 'Report schema keys removed since a published release with no deprecation window' complete -c batten -n "__fish_batten_using_subcommand help; and __fish_seen_subcommand_from config" -f -a "lint" -d 'Report policy smells in batten.toml (any smell is a violation)' complete -c batten -n "__fish_batten_using_subcommand help; and __fish_seen_subcommand_from lint" -f -a "brief" -d 'Check a delegation brief against the handoff schema (any missing section is a violation)' complete -c batten -n "__fish_batten_using_subcommand help; and __fish_seen_subcommand_from doctor" -f -a "hooks" -d 'Diagnose whether batten is wired on every hook surface of every harness' diff --git a/completions/batten.zsh b/completions/batten.zsh index 0f0244b59..5b781427f 100644 --- a/completions/batten.zsh +++ b/completions/batten.zsh @@ -419,6 +419,37 @@ trace\:"Add everything"))' \ '--help[Print help (see more with '\''--help'\'')]' \ && ret=0 ;; +(deprecations) +_arguments "${_arguments_options[@]}" : \ +'--strictness=[Raise how strictly gates apply (an override may only tighten policy)]: :((permissive\:"Advisory\: findings are reported without failing the run" +standard\:"The default\: a finding is a violation" +strict\:"Everything \`Standard\` fails on, plus anything advisory"))' \ +'--config-from=[Read the committed config from a git ref (e.g. origin/main) instead of the working tree]: :_default' \ +'--log-level=[Set the verbosity rung by name]: :((silent\:"Say nothing but a verdict or a usage error" +quiet\:"Suppress ordinary progress; keep warnings" +normal\:"The default" +verbose\:"Explain what is being checked" +debug\:"Add resolution detail" +trace\:"Add everything"))' \ +'-J[Emit byte-stable JSON instead of pointer lines]' \ +'--json[Emit byte-stable JSON instead of pointer lines]' \ +'--fail-on-warning[Promote a warn-severity finding to a violation (an override may only turn this on)]' \ +'*--silent[Say nothing but a verdict or a usage error]' \ +'*-q[Suppress ordinary progress (repeatable\: -qq is silent)]' \ +'*--quiet[Suppress ordinary progress (repeatable\: -qq is silent)]' \ +'*-v[Explain what is being checked (repeatable\: -vv is debug)]' \ +'*--verbose[Explain what is being checked (repeatable\: -vv is debug)]' \ +'*--debug[Add resolution detail]' \ +'*--trace[Add everything]' \ +'--no-color[Never colour stderr, whatever it is attached to]' \ +'--no-input[Never prompt; treat the run as unattended]' \ +'-y[Confirm a destructive operation that would otherwise refuse]' \ +'--yes[Confirm a destructive operation that would otherwise refuse]' \ +'-h[Print help (see more with '\''--help'\'')]' \ +'--help[Print help (see more with '\''--help'\'')]' \ +':against -- The git ref whose published schema is the baseline (e.g. v0.0.111):_default' \ +&& ret=0 +;; (lint) _arguments "${_arguments_options[@]}" : \ '--host-rules=[Compare the committed \[ci\] table against a host ruleset payload (path, or - for stdin)]: :_default' \ @@ -470,6 +501,10 @@ _arguments "${_arguments_options[@]}" : \ _arguments "${_arguments_options[@]}" : \ && ret=0 ;; +(deprecations) +_arguments "${_arguments_options[@]}" : \ +&& ret=0 +;; (lint) _arguments "${_arguments_options[@]}" : \ && ret=0 @@ -2342,6 +2377,10 @@ _arguments "${_arguments_options[@]}" : \ _arguments "${_arguments_options[@]}" : \ && ret=0 ;; +(deprecations) +_arguments "${_arguments_options[@]}" : \ +&& ret=0 +;; (lint) _arguments "${_arguments_options[@]}" : \ && ret=0 @@ -2860,11 +2899,17 @@ _batten__subcmd__config_commands() { local commands; commands=( 'show:Print the effective configuration' \ 'epoch:Print the content hash of the governing config surface' \ +'deprecations:Report schema keys removed since a published release with no deprecation window' \ 'lint:Report policy smells in batten.toml (any smell is a violation)' \ 'help:Print this message or the help of the given subcommand(s)' \ ) _describe -t commands 'batten config commands' commands "$@" } +(( $+functions[_batten__subcmd__config__subcmd__deprecations_commands] )) || +_batten__subcmd__config__subcmd__deprecations_commands() { + local commands; commands=() + _describe -t commands 'batten config deprecations commands' commands "$@" +} (( $+functions[_batten__subcmd__config__subcmd__epoch_commands] )) || _batten__subcmd__config__subcmd__epoch_commands() { local commands; commands=() @@ -2875,11 +2920,17 @@ _batten__subcmd__config__subcmd__help_commands() { local commands; commands=( 'show:Print the effective configuration' \ 'epoch:Print the content hash of the governing config surface' \ +'deprecations:Report schema keys removed since a published release with no deprecation window' \ 'lint:Report policy smells in batten.toml (any smell is a violation)' \ 'help:Print this message or the help of the given subcommand(s)' \ ) _describe -t commands 'batten config help commands' commands "$@" } +(( $+functions[_batten__subcmd__config__subcmd__help__subcmd__deprecations_commands] )) || +_batten__subcmd__config__subcmd__help__subcmd__deprecations_commands() { + local commands; commands=() + _describe -t commands 'batten config help deprecations commands' commands "$@" +} (( $+functions[_batten__subcmd__config__subcmd__help__subcmd__epoch_commands] )) || _batten__subcmd__config__subcmd__help__subcmd__epoch_commands() { local commands; commands=() @@ -3202,10 +3253,16 @@ _batten__subcmd__help__subcmd__config_commands() { local commands; commands=( 'show:Print the effective configuration' \ 'epoch:Print the content hash of the governing config surface' \ +'deprecations:Report schema keys removed since a published release with no deprecation window' \ 'lint:Report policy smells in batten.toml (any smell is a violation)' \ ) _describe -t commands 'batten help config commands' commands "$@" } +(( $+functions[_batten__subcmd__help__subcmd__config__subcmd__deprecations_commands] )) || +_batten__subcmd__help__subcmd__config__subcmd__deprecations_commands() { + local commands; commands=() + _describe -t commands 'batten help config deprecations commands' commands "$@" +} (( $+functions[_batten__subcmd__help__subcmd__config__subcmd__epoch_commands] )) || _batten__subcmd__help__subcmd__config__subcmd__epoch_commands() { local commands; commands=() diff --git a/crates/batten/Cargo.toml b/crates/batten/Cargo.toml index 9165ce00a..45b02f7d9 100644 --- a/crates/batten/Cargo.toml +++ b/crates/batten/Cargo.toml @@ -88,10 +88,12 @@ etcetera.workspace = true fs4.workspace = true getrandom.workspace = true gix.workspace = true +gix-diff.workspace = true regorus.workspace = true globset.workspace = true hmac.workspace = true ignore.workspace = true +imara-diff.workspace = true json5.workspace = true flate2.workspace = true regex.workspace = true diff --git a/crates/batten/src/attribution.rs b/crates/batten/src/attribution.rs index cfa392d26..714c65469 100644 --- a/crates/batten/src/attribution.rs +++ b/crates/batten/src/attribution.rs @@ -627,14 +627,6 @@ mod tests { assert!(policy().validate().is_ok()); } - #[test] - fn trailer_blocks_drop_blank_lines() { - assert_eq!( - git::trailer_lines("Refs: CLOUD-1\n\nSigned-off-by: A \n"), - vec!["Refs: CLOUD-1", "Signed-off-by: A "] - ); - } - #[test] fn a_trailer_with_no_colon_points_at_the_whole_line() { // Defensive: `%(trailers:only)` should never emit one, and a panic here diff --git a/crates/batten/src/cli.rs b/crates/batten/src/cli.rs index 5ec2f10a2..e0ee175e2 100644 --- a/crates/batten/src/cli.rs +++ b/crates/batten/src/cli.rs @@ -456,6 +456,13 @@ pub enum ConfigCommand { /// for the drift comparison. `-` is stdin. host_rules: Option, }, + /// Report schema keys removed since a published release with no window. + Deprecations { + /// Emit the findings as byte-stable JSON instead of pointer lines. + json: bool, + /// The git ref whose published schema is the baseline. + against: String, + }, /// Print the content hash of the governing config surface. Epoch { /// Emit the epoch and the surface it covers as byte-stable JSON. @@ -638,6 +645,16 @@ fn config_of(matches: &ArgMatches) -> Option { .get_one::("host_rules") .map(ToOwned::to_owned), }), + ("deprecations", matches) => Some(ConfigCommand::Deprecations { + json: flag(matches, "json"), + // `--against` is declared `required`, so clap has already refused an + // invocation without it; the default is unreachable and exists only + // because `get_one` is total. + against: matches + .get_one::("against") + .cloned() + .unwrap_or_default(), + }), ("epoch", matches) => Some(ConfigCommand::Epoch { json: flag(matches, "json"), no_cache: flag(matches, "no_cache"), diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 75dbf8986..3ff3fbc42 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -52,6 +52,14 @@ pub const SUPPORTED_VERSION: u32 = 1; /// directory. No upward walk, no `conf.d` merge (§8). pub const CONFIG_FILE: &str = "batten.toml"; +/// Where the derived authority schema is published, repo-relative. +/// +/// Named once because two readers need it and a second spelling is a second +/// authority: `schema-check` regenerates and diffs the committed file, and +/// `config deprecations` reads the copy at a release ref. `/`-separated, as git +/// addresses a blob. +pub const SCHEMA_PATH: &str = "schema/batten.schema.json"; + /// How strictly Batten applies its gates — the ordered, policy-bearing key the /// §8 raise-only rule is defined over. /// @@ -488,6 +496,212 @@ pub fn parse(text: &str, source: &str) -> Result { Ok(config) } +/// The migration window applied to one config's text, ahead of the typed parse. +/// +/// **Two jobs, and they are the two halves §2 names.** A key inside its window is +/// STRIPPED so `deny_unknown_fields` does not refuse it — the whole point of the +/// window is that the old spelling still loads — and its pointer is returned for +/// the caller to report. A key past expiry is REFUSED here rather than stripped, +/// because the window closing is the deprecation grammar's one hard edge; letting +/// it fall through to `deny_unknown_fields` would report it as an unknown key, +/// which is a different diagnostic with a different remedy and is the collapse +/// §7(c) exists to catch. +/// +/// A key that was never ours is left alone entirely, to be refused as unknown by +/// the typed parse. This function narrows nothing and widens nothing: it only +/// moves keys the table already names. +/// +/// # Errors +/// +/// Raises a [`UsageError`] (→ exit `1`) when `text` is not TOML, or when a key +/// past its expiry is present. +pub fn apply_window( + text: &str, + source: &str, + table: &[Deprecation], + today: &str, +) -> Result<(String, Vec)> { + let mut parsed: toml::Table = toml::from_str(text) + .map_err(|err| UsageError::raise(format!("invalid config {source}: {err}")))?; + let mut reported = Vec::new(); + // Sorted, because the report is compared byte-for-byte under §6 and a TOML + // table's iteration order is not the author's file order. + let mut present: Vec = parsed.keys().cloned().collect(); + present.sort(); + for key in present { + match deprecation_of(table, &key, today) { + Some(standing @ Standing::Expired { .. }) => { + return Err(UsageError::raise(format!( + "invalid config {source}: {}", + deprecation_line(&standing) + ))); + } + Some(standing @ Standing::Migrating { .. }) => { + parsed.remove(&key); + reported.push(deprecation_line(&standing)); + } + None => {} + } + } + let text = toml::to_string(&parsed) + .map_err(|err| UsageError::raise(format!("invalid config {source}: {err}")))?; + Ok((text, reported)) +} + +/// One key's migration window: what replaces it, when the window closes, and the +/// row that owns the move (CLOUD-360). +/// +/// **`expand -> migrate -> contract`, and this type is the MIDDLE stage.** Expand +/// is adding the replacement key beside the old one; migrate is this window, +/// where the old key still parses and says so; contract is removal, at which +/// point the key moves to [`RETIRED_KEYS`] and is tolerated only in a base ref. +/// The two tables are one authority read at two stages of a key's life, never two +/// places a deprecation is recorded — a key in both is a contradiction and +/// `no_key_is_both_deprecated_and_retired` refuses it. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct Deprecation { + /// The top-level key, as it appears in `batten.toml`. + pub key: &'static str, + /// The key that replaces it, or `None` when the capability is going away + /// with no successor — a distinction a consumer needs and which an empty + /// string would hide. + pub replacement: Option<&'static str>, + /// The day the window closes, `YYYY-MM-DD`. On and after it the key is + /// REFUSED rather than reported. + pub expires: &'static str, + /// The `CLOUD-*` row that owns the migration, so a consumer reading the + /// finding can find out why. + pub issue: &'static str, +} + +/// Keys this engine still accepts and is migrating away from. +/// +/// **Empty is the honest state today and is not a disabled gate.** The predicate +/// over it is exercised by [`deprecation_of`]'s own cases, which supply a table +/// rather than reading this one — the shipped table records real migrations, and +/// inventing a fake row so a fixture has something to find would put a key in the +/// published schema that no consumer should ever write. What an empty table must +/// NOT do is make the schema-removal gate vacuous, and it does not: +/// `no_key_leaves_the_schema_unannounced` reads both tables, so an empty one +/// makes every removal a finding rather than none (CLOUD-251). +pub const DEPRECATED_KEYS: &[Deprecation] = &[]; + +/// How a key stands against the deprecation table. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Standing { + /// Inside its window: parses, and reports. + Migrating { + /// The window this key is inside. + deprecation: Deprecation, + }, + /// Past its expiry: refused. + Expired { + /// The window that closed. + deprecation: Deprecation, + }, +} + +/// Where a key stands, given a table and the day it is being judged on. +/// +/// **The table and the date are ARGUMENTS, not reads.** That is what makes this +/// decidable in a test without a fake row in the shipped table and without a +/// clock: a window is a comparison between two dates, and a predicate that read +/// the wall clock would answer differently tomorrow for the same commit — which +/// is the property a gate must not have. +/// +/// `today` and `expires` are both `YYYY-MM-DD`, so a lexical comparison IS a +/// chronological one and no date library is bought for it. A malformed `expires` +/// sorts as some string and would silently change the verdict, which is why +/// `every_declared_expiry_is_a_date` refuses one at the table rather than here. +#[must_use] +pub fn deprecation_of(table: &[Deprecation], key: &str, today: &str) -> Option { + let found = table.iter().find(|row| row.key == key)?.clone(); + if today >= found.expires { + Some(Standing::Expired { deprecation: found }) + } else { + Some(Standing::Migrating { deprecation: found }) + } +} + +/// The pointer-only diagnostic for one standing (non-negotiable rule 4). +/// +/// Key, replacement, expiry and owning row — never the VALUE configured at the +/// key, which is the consumer's content and is exactly what a diagnostic quoting +/// the line would leak. +#[must_use] +pub fn deprecation_line(standing: &Standing) -> String { + let (verdict, row) = match standing { + Standing::Migrating { deprecation } => ("deprecated", deprecation), + Standing::Expired { deprecation } => ("expired", deprecation), + }; + let replacement = row.replacement.unwrap_or("none"); + format!( + "{} {verdict} replacement={replacement} expires={} ({})", + row.key, row.expires, row.issue + ) +} + +/// The top-level keys a derived JSON Schema declares. +/// +/// **Top-level only, stated rather than implied.** Both deprecation tables key on +/// a top-level `batten.toml` key — `RETIRED_KEYS` names `worktree`, not +/// `worktree.pileup` — so the removal gate compares the surface those tables can +/// actually annotate. A field disappearing from inside a `$defs` type is a real +/// change this does not see, and claiming otherwise would be the wider promise +/// CLOUD-251 calls a vacuous pass. +/// +/// # Errors +/// +/// Raises a [`UsageError`] (→ exit `1`) when `text` is not a JSON object carrying +/// a `properties` map — a schema this cannot read is not a schema with no keys. +pub fn schema_keys(text: &str, source: &str) -> Result> { + let parsed: serde_json::Value = serde_json::from_str(text) + .map_err(|err| UsageError::raise(format!("unreadable schema {source}: {err}")))?; + let properties = parsed + .get("properties") + .and_then(serde_json::Value::as_object); + let Some(properties) = properties else { + return Err(UsageError::raise(format!( + "unreadable schema {source}: no `properties` map, so its key set cannot be compared" + ))); + }; + Ok(properties.keys().cloned().collect()) +} + +/// Keys the released schema declared that this one does not, and which neither +/// table announces (CLOUD-360 §2). +/// +/// **The gate the row exists for.** A key vanishing from the published schema is +/// a silent break for every consumer whose `batten.toml` still carries it: their +/// config stops loading with an unknown-key error naming no successor. The +/// grammar's promise is that removal is always preceded by a window, and this is +/// the predicate that holds it. +/// +/// Either table satisfies it, because they are consecutive stages of one life: a +/// key mid-window is in [`DEPRECATED_KEYS`] and one already contracted is in +/// [`RETIRED_KEYS`], and both mean the removal was announced. +/// +/// An EMPTY deprecation table therefore makes this stricter, never weaker — +/// every removal is unannounced until somebody writes the row. That is the +/// direction CLOUD-251 asks for: the gate with nothing declared refuses rather +/// than passing quietly. +#[must_use] +pub fn removals_unannounced( + released: &std::collections::BTreeSet, + current: &std::collections::BTreeSet, + deprecated: &[Deprecation], + retired: &[(&str, &str)], +) -> Vec { + released + .iter() + .filter(|key| !current.contains(*key)) + .filter(|key| !deprecated.iter().any(|row| row.key == key.as_str())) + .filter(|key| !retired.iter().any(|(name, _)| name == key)) + .cloned() + .collect() +} + /// Keys a **past** engine accepted and this one has retired, with the issue that /// retired each. /// @@ -1034,6 +1248,7 @@ fn default_rules() -> Vec { refs: Vec::new(), ranges: Vec::new(), landing: Vec::new(), + symbols: false, run: None, verbatim: None, identity_key: None, @@ -1343,6 +1558,228 @@ mod tests { assert!(is_usage_error(&err)); } + /// A table row for the window cases. The shipped `DEPRECATED_KEYS` is empty + /// and should be — inventing a row so a test has something to find would put + /// a key in the published schema no consumer should write — so the predicate + /// takes its table as an argument and these supply one. + fn window(expires: &'static str) -> Vec { + vec![Deprecation { + key: "old_table", + replacement: Some("new_table"), + expires, + issue: "CLOUD-360", + }] + } + + #[test] + fn a_key_inside_its_window_parses_and_is_reported() { + let table = window("2099-01-01"); + let (stripped, reported) = apply_window( + "version = 1\n[old_table]\nvalue = 1\n", + "batten.toml", + &table, + "2026-08-25", + ) + .expect("an in-window key loads"); + assert_eq!( + reported, + vec![ + "old_table deprecated replacement=new_table expires=2099-01-01 (CLOUD-360)" + .to_owned() + ], + "the finding is pointer-only: key, replacement, expiry, owning row" + ); + assert!( + !reported.iter().any(|line| line.contains("value")), + "a configured VALUE must never reach the diagnostic (rule 4)" + ); + // And the stripped text is what the typed parse then accepts. + parse(&stripped, "batten.toml").expect("the stripped config is valid"); + } + + #[test] + fn a_key_past_its_expiry_is_refused_rather_than_reported() { + let table = window("2026-01-01"); + let err = apply_window( + "version = 1\n[old_table]\nvalue = 1\n", + "batten.toml", + &table, + "2026-08-25", + ) + .expect_err("an expired key is refused"); + assert!( + is_usage_error(&err), + "an expired key is exit 1, not a panic" + ); + assert!( + format!("{err}").contains("old_table expired"), + "the refusal names the expired key: {err}" + ); + } + + /// The boundary, stated rather than left to a reader: the window closes ON + /// the expiry date, so `today == expires` is expired. + #[test] + fn the_window_closes_on_its_expiry_day_not_after_it() { + let table = window("2026-08-25"); + assert!( + matches!( + deprecation_of(&table, "old_table", "2026-08-25"), + Some(Standing::Expired { .. }) + ), + "the expiry day is outside the window" + ); + assert!( + matches!( + deprecation_of(&table, "old_table", "2026-08-24"), + Some(Standing::Migrating { .. }) + ), + "the day before it is inside" + ); + } + + /// §7(c): the two refusals must stay TELLABLE APART. If a deprecated key and + /// an unknown key produced the same diagnostic, the window would be + /// invisible to the consumer it exists for. + #[test] + fn an_unknown_key_is_refused_differently_from_a_deprecated_one() { + let table = window("2026-01-01"); + let expired = apply_window( + "version = 1\n[old_table]\nvalue = 1\n", + "batten.toml", + &table, + "2026-08-25", + ) + .expect_err("expired"); + // A key the table never named is left for the typed parse, which refuses + // it as unknown. + let (passed, reported) = apply_window( + "version = 1\n[never_ours]\nvalue = 1\n", + "batten.toml", + &table, + "2026-08-25", + ) + .expect("an unfamiliar key is not this predicate's to refuse"); + assert!( + reported.is_empty(), + "nothing to report about a key we never had" + ); + let unknown = parse(&passed, "batten.toml").expect_err("unknown keys stay errors"); + + let (expired, unknown) = (format!("{expired}"), format!("{unknown}")); + assert!( + expired.contains("expired"), + "the expired diagnostic says so: {expired}" + ); + assert!( + !unknown.contains("expired"), + "an unknown key must not borrow the deprecation vocabulary: {unknown}" + ); + assert_ne!(expired, unknown, "the two refusals are distinguishable"); + } + + /// The two tables are one authority read at two stages. A key in both is a + /// contradiction: it cannot be simultaneously still-accepted and already-gone. + #[test] + fn a_key_leaving_the_schema_unannounced_is_a_finding() { + let released = schema_keys( + r#"{"properties":{"version":{},"gone":{},"kept":{}}}"#, + "released", + ) + .expect("the released schema reads"); + let current = + schema_keys(r#"{"properties":{"version":{},"kept":{}}}"#, "current").expect("current"); + + // Nothing announces it: a finding. + assert_eq!( + removals_unannounced(&released, ¤t, &[], &[]), + vec!["gone".to_owned()], + "an empty table makes every removal a finding, never none" + ); + + // Mid-window announces it. + let window = window("2099-01-01"); + let mut mid = window.clone(); + mid[0] = Deprecation { + key: "gone", + replacement: Some("kept"), + expires: "2099-01-01", + issue: "CLOUD-360", + }; + assert!( + removals_unannounced(&released, ¤t, &mid, &[]).is_empty(), + "a key inside its window is announced" + ); + + // And so does the contracted stage, because the two tables are one + // authority read at consecutive points of a key's life. + assert!( + removals_unannounced(&released, ¤t, &[], &[("gone", "CLOUD-360")]).is_empty(), + "a retired key was announced when its window ran" + ); + } + + #[test] + fn a_key_that_is_merely_added_is_not_a_removal() { + let released = schema_keys(r#"{"properties":{"version":{}}}"#, "released").expect("rel"); + let current = + schema_keys(r#"{"properties":{"version":{},"brand_new":{}}}"#, "current").expect("cur"); + assert!( + removals_unannounced(&released, ¤t, &[], &[]).is_empty(), + "the grammar governs removals; adding a key needs no window" + ); + } + + /// A schema this cannot read is COULD NOT LOOK, never "declared no keys" — + /// the latter would report every key as removed, or, read the other way + /// round, would wave a real removal through (CLOUD-251). + #[test] + fn an_unreadable_schema_is_refused_rather_than_read_as_empty() { + assert!(is_usage_error( + &schema_keys("not json at all", "released").expect_err("refused") + )); + assert!(is_usage_error( + &schema_keys(r#"{"title":"no properties here"}"#, "released").expect_err("refused") + )); + } + + #[test] + fn no_key_is_both_deprecated_and_retired() { + for row in DEPRECATED_KEYS { + assert!( + !RETIRED_KEYS.iter().any(|(key, _)| *key == row.key), + "{} is in both tables; migrating and retired are consecutive \ + stages, never concurrent ones", + row.key + ); + } + } + + /// A malformed expiry would sort as some string and silently decide a window, + /// so the shape is refused at the table rather than at the comparison. + #[test] + fn every_declared_expiry_is_a_date_and_names_its_row() { + for row in DEPRECATED_KEYS { + let parts: Vec<&str> = row.expires.split('-').collect(); + assert!( + parts.len() == 3 + && parts[0].len() == 4 + && parts[1].len() == 2 + && parts[2].len() == 2 + && row.expires.chars().all(|c| c.is_ascii_digit() || c == '-'), + "{}: expiry {:?} is not YYYY-MM-DD, so the lexical comparison that \ + decides its window is not a chronological one", + row.key, + row.expires + ); + assert!( + row.issue.starts_with("CLOUD-"), + "{}: a migration names the row that owns it", + row.key + ); + } + } + #[test] fn every_retired_key_names_the_issue_that_retired_it() { // A row nobody can date is a row nobody can drop, and dropping them once diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index 4ba0245d1..17c4c3d48 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -459,6 +459,9 @@ pub enum Fact { /// Which module a **declared** Rust source file reaches, resolved through the /// crate root's own re-export table (CLOUD-762). Uses, + /// Where the crate uses a type a delegated analyser resolved by NAME, rather + /// than by spelling (CLOUD-760). The first `Cost::Effect` fact. + Symbols, } /// [`Fact::Bypass`] — the hatch is an environment variable, and the kernel @@ -802,6 +805,38 @@ pub const GIT_RANGE: Class = Class::new(Cost::Read, Surface::Check); /// full confidence, which is the direction that lets a gate pass on ignorance. pub const LANDING: Class = Class::new(Cost::Read, Surface::Check); +/// [`Fact::Symbols`] — **the first occupant of [`Cost::Effect`]**, and the +/// reserved variant stops being empty for a stated reason. +/// +/// `effect` x `check`, and each half is a decision rather than an inference. +/// +/// **`Cost::Effect` because resolving it RUNS A PROGRAM**, which is the whole +/// content of that variant and the only thing it claims. Every other fact in this +/// table is `Free` or `Read`; this one spawns `cargo clippy` and waits for it. +/// Naming that honestly is the point — a fact that spawned while classified +/// `Read` would make the cost axis decorative. +/// +/// **`Surface::Check`, and `Surface::Hook` is REFUSED.** `run_static` already +/// refuses a spawning kind outright, and a fact resolvable on the mediated path +/// would weaken that promise from a structural guarantee into a convention. +/// `resolvable_on` is what enforces it, and `tests/facts.rs`'s exhaustive match +/// is what keeps the refusal from being merely intended. +/// +/// **`check` RESOLVES IT DIRECTLY rather than consuming a receipt, and that is +/// the §5 decision CLOUD-760 left open.** The receipt route was the tempting one +/// — `verify` already writes SHA-keyed receipts that `hook` reads — and it is +/// refused here for a reason that is about honesty rather than machinery: a +/// receipt-backed fact is a claim about a tree some EARLIER run saw, and `check` +/// consuming one would report a census of a tree that is not the one in front of +/// it. The cost axis exists precisely so an expensive fact can be declared +/// expensive instead of being made to look cheap. A caller that cannot afford it +/// does not ask for it; that is what `Class` is for. +/// +/// The amortisation argument survives and is not this row's: a receipt-backed +/// SECOND class of the same fact is buildable later, and would be a different +/// `Class` rather than a quiet reinterpretation of this one. +pub const SYMBOLS: Class = Class::new(Cost::Effect, Surface::Check); + impl Fact { /// Every fact the boundary resolves today, so [`Fact::class`] is total. pub const ALL: &'static [Fact] = &[ @@ -824,6 +859,7 @@ impl Fact { Fact::Landing, Fact::Invocations, Fact::Uses, + Fact::Symbols, ]; /// The stable lowercase token (§6) — the field name in `lib.rs`'s `Facts`. @@ -849,6 +885,7 @@ impl Fact { Fact::Landing => "landing", Fact::Invocations => "invocations", Fact::Uses => "uses", + Fact::Symbols => "symbols", } } @@ -882,6 +919,7 @@ impl Fact { Fact::Landing => LANDING, Fact::Invocations => INVOCATIONS, Fact::Uses => USES, + Fact::Symbols => SYMBOLS, } } @@ -933,6 +971,11 @@ impl Fact { // one. Fact::Invocations => Some("invocations"), Fact::Uses => Some("uses"), + // The resolved-symbol tier (CLOUD-760). Tree surface like the two + // above, and `Cost::Effect` where they are `Read` — the cost axis is + // independent of the surface one, which is exactly what makes the + // pair expressive rather than redundant. + Fact::Symbols => Some("symbols"), // Hook-surface facts. The tree engine resolves none of them, and // naming them here as `None` is what lets the correspondence test // assert the emitted key set in BOTH directions rather than only @@ -1008,6 +1051,7 @@ impl Fact { }, }, }), + Fact::Symbols => Self::symbols_schema_fragment(), Fact::Uses => serde_json::json!({ "type": "object", "description": "Fact::Uses (CLOUD-762). Path -> that file's `use` edges. `to` is the module or crate reached AFTER resolution through the crate root's re-export table; `item` the imported leaf name; `origin` one of internal/external/root-item/local; `via_root` whether resolution supplied `to` rather than the text, which is the flag that marks an edge a line predicate reads wrongly. An edge still `root-item` is one the root's table could not name, and is could-not-look at the edge level rather than an edge onto nothing. A path absent from this map could not be parsed; a path present with an empty array imports nothing.", @@ -1067,6 +1111,56 @@ impl Fact { } } + /// The schema fragment for the `Cost::Effect` fact (CLOUD-760). + /// + /// Split out for [`Fact::git_schema_fragment`]'s reason — it is what pushed + /// [`Fact::schema_fragment`] past the line ceiling — but on a different seam, + /// and the seam is the one that matters here: **this is the only fragment + /// that has to describe a producer as well as a shape.** The `provenance` + /// half is not decoration. A fact whose value depends on which analyser at + /// which version resolved it is not byte-stable under §6 unless the document + /// says which one that was, so the tool, its version and the pinned + /// invocation travel inside the fact rather than beside it. + /// + /// `sites` is pointer-only, per non-negotiable rule 4: a path, a line and the + /// lint that fired. The analyser's message, and the source line it quoted, + /// are content and stay out of the policy input. + fn symbols_schema_fragment() -> serde_json::Value { + serde_json::json!({ + // NULLABLE, like the git family and for its reason: the projection + // emits `null` for both did-not-look answers, and a schema typing + // this as a bare object refuses the module that handles them. Caught + // by `opa check -s` -- which is the whole argument for deriving the + // schema from the fact rather than writing it beside it. + "type": ["object", "null"], + "description": "Fact::Symbols (CLOUD-760). The first Cost::Effect fact: where a delegated analyser resolved a named type, by NAME rather than by spelling. `provenance` records which tool at which version produced it, because a fact whose meaning depends on an unrecorded tool version is not canonical. `sites` is pointer-only -- a path, a line and the lint that fired, never the diagnostic's message or the source it quoted.", + "properties": { + "provenance": { + "type": "object", + "properties": { + "tool": {"type": "string"}, + "version": {"type": "string"}, + "invocation": {"type": "array", "items": {"type": "string"}}, + }, + "additionalProperties": false, + }, + "sites": { + "type": "array", + "items": { + "type": "object", + "properties": { + "path": {"type": "string"}, + "line": {"type": "integer"}, + "lint": {"type": "string"}, + }, + "additionalProperties": false, + }, + }, + }, + "additionalProperties": false, + }) + } + /// The schema fragment for the git and landing families (CLOUD-880). /// /// Split out of [`Fact::schema_fragment`] when `Landing` pushed it past the @@ -1162,7 +1256,8 @@ impl Fact { | Fact::Prospective | Fact::Produced | Fact::Invocations - | Fact::Uses => serde_json::json!({ + | Fact::Uses + | Fact::Symbols => serde_json::json!({ "description": "unrouted fact -- schema_fragment delegated a fact git_schema_fragment does not own", }), } diff --git a/crates/batten/src/git.rs b/crates/batten/src/git.rs index bedbea4bd..f80869a8d 100644 --- a/crates/batten/src/git.rs +++ b/crates/batten/src/git.rs @@ -9,18 +9,18 @@ //! per-repository config and state live, rather than the worktree's own //! toplevel. //! -//! Resolution shells out to `git rev-parse` with the discovery environment -//! scrubbed: an ambient override — a hook context exporting `GIT_DIR`, say — -//! makes git answer for some *other* repository, which is the exact -//! mis-rooting bug class this module exists to kill. The answer is a function -//! of the (cwd-resolved) `start` argument and on-disk state only. +//! Resolution is in-process (CLOUD-740). [`gix::open::Options::isolated`] +//! declines system, global and environment configuration outright, so an ambient +//! override — a hook context exporting `GIT_DIR`, say — cannot make the answer be +//! about some *other* repository, which is the mis-rooting bug class this module +//! exists to kill. The answer is a function of the (cwd-resolved) `start` +//! argument and on-disk state only. //! -//! Non-goals, refused loudly rather than answered wrongly: a bare repository, -//! a submodule interior (common dir `/.git/modules/`), and a -//! `--separate-git-dir` layout all raise a [`UsageError`], because deriving a -//! root as the common dir's parent is only sound when that directory is a -//! `/.git`. If a consumer ever needs those layouts, the escalation path -//! is `git worktree list --porcelain`, not more `parent()` arithmetic. +//! **That scrub is structural, where it used to be a maintained list.** Five +//! environment variables were removed by name from every child — three redirects +//! and two discovery fences — and a sixth arriving in a future git would simply +//! not have been removed. Declining the environment as a class has no such gap, +//! and there is no constant left to keep current. //! //! # Merged-ness (CLOUD-36) //! @@ -38,23 +38,43 @@ //! ratchet spanning a non-ASCII path reported clean while a test was deleted //! (CLOUD-749) — CLOUD-328's failure class on a second axis. //! -//! **Still spawning, and every one of them has an open row that would move it.** -//! The remaining reads take fixed argv with no caller-supplied token, or sit in -//! `rev-parse`'s ref-PRINTING modes where the `--end-of-options` trap below -//! lives and no caller string reaches the command line anyway — so none of this -//! is urgent, and none of it is settled either. CLOUD-738 owns the ref and -//! object reads, and its deliverable is **deleting** that trap rather than -//! documenting it; CLOUD-739 owns `landing` and patch identity, and with them -//! the 26 settings pinned below purely to stop a host's `git config` moving the -//! answer; CLOUD-740 owns `uncommitted`, `changed_paths` and `check_ignore`, and -//! the terminal assertion that this crate spawns `git` nowhere. +//! **NOTHING HERE SPAWNS `git` ANY MORE (CLOUD-740).** The last child was +//! `repo_root`'s, and `no_second_git_invoker_exists` is now the terminal +//! assertion the four slices were sequenced toward: a literal `git` spawn +//! anywhere under `src/` fails it, this file included. That gate used to exempt +//! this module, because this module held the one invoker; the exemption is what +//! went, and the claim is strictly stronger and much simpler for it. +//! +//! Three helpers went with the last of them — `query`, `query_bytes` and +//! `query_optional` — along with `command`, the two discovery-scrub constants and +//! the `queries_spawned` counter. So did a family of hazards that were being +//! *remembered* rather than made impossible: `--end-of-options` on every argv +//! carrying a caller's token, its inverse in `rev-parse`'s ref-PRINTING modes +//! where the flag is echoed as an output line rather than consumed, and +//! `core.quotePath` deciding whether a non-ASCII path arrived readable. A +//! resolver takes no flags and a path is bytes, so none of the three has anywhere +//! left to occur. +//! +//! What did NOT come from gix is worth naming, because two questions were +//! answered by refusing a dependency rather than by taking one. `uncommitted` and +//! `changed_paths` read the index, the `HEAD` tree and the vendored `ignore` +//! walker instead of gix's `status`, and `check_ignore` reads that same walker's +//! rules rather than gix's excludes: both gix features pull the +//! materialise-blobs-to-disk and external-program surface CLOUD-739 declined, and +//! buying it to delete a spawn would be buying the thing the spawn was being +//! deleted for. `working_tree_changes` carries the cost that choice has — +//! clean/smudge filters are not applied — and states why over-reporting is the +//! safe direction there. //! //! An earlier revision of this paragraph said *"migrating buys nothing an agent //! can observe"* and called rewriting patch identity *"risk with no return"*. It //! was written while all three of those rows sat cancelled, and a later session //! read it here and restated it as fact. Both halves failed in the same -//! direction: CLOUD-739's own gate is a differential test against the -//! implementation it replaces, so that risk is **priced**, not absent. +//! direction, and the migration that has now happened settles it: the risk was +//! **priced** rather than absent, by a differential gate that compares the +//! VERDICT the two implementations give over the same rebase, squash and +//! cherry-pick corpus — never the hashes, which differ by construction and whose +//! agreement would assert the migration did not happen. //! //! **What the price is, since a cost must be named as one (CLOUD-320).** These //! spawns stay under a build strategy rather than a capability limit. `git2` has @@ -101,7 +121,7 @@ //! way to `main`. On a fast-forward trunk that is the *normal* way work lands. //! A false *not landed* on work that did land is silently wrong rather than //! loudly broken, and it is the failure class Batten exists to catch. So -//! [`landing`] compares **patch identity** — `git patch-id --stable` over each +//! [`landing`] compares **patch identity** — [`crate::patch::identity`] over each //! change — and, for the squash case that per-commit identity cannot see, the //! patch identity of the branch's cumulative diff. //! @@ -132,118 +152,50 @@ use std::collections::{BTreeMap, BTreeSet}; use std::ffi::OsStr; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; -#[expect( - clippy::disallowed_types, - reason = "stays: this module is two-backend BY DECISION (CLOUD-780) — gix where a library makes a defect unrepresentable, spawned `git` where it does not, and every remaining spawn is unported rather than unportable" -)] -use std::process::{Command, Stdio}; -use std::sync::atomic::{AtomicUsize, Ordering}; - -use anyhow::{Context, Result, bail}; + +use anyhow::{Result, bail}; use serde::Serialize; use crate::error::UsageError; -/// Environment variables that point git at a *different* repository. Scrubbed -/// from every child this module spawns, so an ambient `GIT_DIR` — a hook -/// context, a wrapping git command — can never make a query answer about some -/// other checkout than the directory it was handed. -const DISCOVERY_REDIRECTS: [&str; 3] = ["GIT_DIR", "GIT_COMMON_DIR", "GIT_WORK_TREE"]; - -/// Environment variables that *fence* git's upward search rather than -/// redirecting it. [`repo_root`] scrubs these too, because its answer must be a -/// function of `start` and the filesystem alone; a plain [`query`] leaves them -/// in place, since a caller that fenced discovery on purpose (a test pinning a -/// fixture inside a tmpdir) is relying on the fence to fail loudly. -const DISCOVERY_FENCES: [&str; 2] = ["GIT_CEILING_DIRECTORIES", "GIT_DISCOVERY_ACROSS_FILESYSTEM"]; - -/// Environment variables that change the bytes a diff produces. Scrubbed -/// alongside the discovery redirects, because a patch identity computed under -/// an ambient `GIT_EXTERNAL_DIFF` is not comparable with one computed without. -const DIFF_ENV: [&str; 2] = ["GIT_EXTERNAL_DIFF", "GIT_DIFF_OPTS"]; - -/// Config pinned on every patch-identity computation. -/// -/// A patch identity is only comparable against another produced the same way, -/// so nothing that shapes the diff may be left to config. `-c` rather than -/// blanking `GIT_CONFIG_GLOBAL`, because the values that break comparability -/// can also live in the repository's own `.git/config`, which no environment -/// variable neutralises — and blanking global config would disturb credential -/// and transport settings that are none of this module's business. -/// -/// `diff.renames` is the load-bearing one: it defaults to *true* for the -/// porcelain `git diff` used on the cumulative side and *false* for plumbing. -/// Unpinned, the two sides silently disagree about any commit that renames a -/// file, and a real landing goes unrecognised. -const DIFF_CONFIG: [&str; 20] = [ - "-c", - "diff.renames=false", - "-c", - "diff.algorithm=myers", - "-c", - "diff.indentHeuristic=true", - "-c", - "diff.context=3", - "-c", - "diff.noprefix=false", - "-c", - "diff.mnemonicPrefix=false", - "-c", - "diff.relative=false", - "-c", - "diff.ignoreSubmodules=none", - "-c", - "core.quotePath=true", - "-c", - "color.ui=false", -]; - -/// Diff flags pinned alongside [`DIFF_CONFIG`]. -/// -/// `--binary` is not an optimisation: without it a binary change renders as -/// `Binary files a/x and b/x differ` — identical text for *any* two changes to -/// the same path, so two unrelated binary edits would share a patch identity -/// and one would be reported as the other's landing. The cost is that a binary -/// patch body is zlib output, deterministic for a given zlib but not guaranteed -/// across zlib builds; a stability caveat is the right trade against a wrong -/// answer. -const DIFF_FLAGS: [&str; 6] = [ - "--no-ext-diff", - "--no-textconv", - "--no-color", - "--no-renames", - "-U3", - "--binary", -]; - -/// A `git patch-id --stable` hash: the identity of a change's *content*, -/// independent of the commit that carries it. +/// The identity of a change's *content*, independent of the commit that carries +/// it. /// /// Two commits with the same `PatchId` make the same change to the same paths, /// whatever their SHA, author, message, date, or parents — which is precisely /// what makes a rebased, amended, or cherry-picked commit recognisable after it /// lands under a new SHA. /// -/// Not a content address: git's normalisation drops whitespace and hunk line -/// numbers, so a whitespace-only difference collides. That biases toward -/// reporting work as landed, which is the safe direction for a primitive whose -/// failure class is a false *not landed*. +/// **Computed in-process, and the normalisation is ours** (CLOUD-739). +/// [`crate::patch`] is the authority on what that normalisation is and why each +/// part of it was chosen; it is deliberately NOT restated here, because two +/// copies of a definition drift and only one of them can be the one the code +/// implements. The short version a reader needs at this type: line numbers are +/// excluded so a rebase still matches, and whitespace is significant, which +/// diverges from `git patch-id` on purpose. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize)] pub struct PatchId(String); impl PatchId { - /// Parse one lowercase-hex id as `git patch-id` prints it — 40 hex digits - /// in a SHA-1 repository, 64 in a SHA-256 one. Anything else is refused, so - /// a parsing slip can never manufacture an equality. + /// The one constructor, so a slip in rendering cannot manufacture an + /// equality (CLOUD-739 §7c). + /// + /// Exactly 64 lowercase hex digits: the identity is a SHA-256 over this + /// crate's own canonical form, so its width is fixed by that and no longer + /// by the repository's hash. It used to accept 40 as well, because it was + /// parsing whatever `git patch-id` printed and that followed the repository + /// — SHA-1 or SHA-256. Nothing prints it now, so the narrower rule is the + /// honest one, and a 40-hex value reaching here is a defect rather than a + /// SHA-1 repository. fn parse(text: &str) -> Result { - let ok = matches!(text.len(), 40 | 64) + let ok = text.len() == 64 && text .chars() .all(|c| c.is_ascii_digit() || matches!(c, 'a'..='f')); if ok { Ok(Self(text.to_owned())) } else { - bail!("`git patch-id` printed {text:?}, which is not a patch identity") + bail!("a patch identity must be 64 lowercase hex digits, not {text:?}") } } @@ -443,6 +395,37 @@ impl Landing { } } +/// Strip Windows' verbatim prefix, so a canonicalised path is comparable with +/// every other path in the crate. +/// +/// **This is a regression this migration introduced, caught by CI on Windows and +/// nowhere else.** The shelled-out predecessor answered from +/// `git rev-parse --show-toplevel`, which is a plain path. `Path::canonicalize` +/// is not: on Windows it returns the VERBATIM spelling, `\\?\D:\a\batten`, and +/// nothing else in the crate produces one. `receipt::judgeable` then asks whether +/// a `std::path::absolute` path starts with the root, the two never share a +/// prefix, and the write reads as OUTSIDE the repository — so the claim gate +/// allowed every write on Windows while denying correctly everywhere else. +/// Measured as exactly one red case out of 2288, which is what an answer that is +/// wrong only under a prefix looks like. +/// +/// A verbatim UNC path (`\\?\UNC\server\share`) is LEFT ALONE: its plain +/// spelling is not equivalent — verbatim paths skip normalisation — so rewriting +/// one would trade a comparison bug for a resolution bug. On every other platform +/// this is the identity. +fn plain(path: PathBuf) -> PathBuf { + let Some(text) = path.to_str() else { + return path; + }; + let Some(rest) = text.strip_prefix(r"\\?\") else { + return path; + }; + if rest.starts_with("UNC\\") { + return path; + } + PathBuf::from(rest) +} + /// Resolve the root of the repository containing `start`: the working-tree /// directory whose `.git` is the repository's *common* directory. /// @@ -469,53 +452,42 @@ pub fn repo_root(start: &Path) -> Result { start.display() ))); } - let mut command = command(start); - command - // Option order is load-bearing twice over: output lines mirror option - // order, and `--path-format` applies only to the options after it (an - // unqualified `--git-common-dir` prints a cwd-relative path). - .args([ - "rev-parse", - "--is-bare-repository", - "--path-format=absolute", - "--git-common-dir", - ]); - // The fences are scrubbed here and only here: this answer must be a - // function of `start` and the filesystem, whereas a caller that fenced - // discovery on purpose is relying on a plain `query` to fail loudly. - for var in DISCOVERY_FENCES { - command.env_remove(var); - } - let output = command - .output() - .context("run `git rev-parse` to locate the repository common dir")?; - if !output.status.success() { - // git's own stderr is version-dependent prose; the caller gets one - // deterministic message instead. - return Err(UsageError::raise(format!( + // UNFENCED, and this is the one caller that is. `repo_root`'s contract is + // that its answer is a function of `start` and the filesystem — every path + // the crate resolves against "the repository" derives from it, so a ceiling + // in the ambient environment must not move the root out from under a caller + // that never asked about it. Every OTHER read goes through `open`, which + // honours the ceiling, because a caller that fenced discovery on purpose is + // relying on being refused rather than answered about whatever repository + // sits further up the tree. + let repo = open_upwards(start, Vec::new()).map_err(|_| { + UsageError::raise(format!( "{} is not inside a git repository", start.display() + )) + })?; + // A bare repository has no working tree to root, and that refusal must stay + // LOUD rather than deriving a directory that is not a checkout. + if repo.worktree().is_none() { + return Err(UsageError::raise(format!( + "{} is inside a bare repository, which has no working tree to root", + start.display() ))); } - let stdout = - String::from_utf8(output.stdout).context("decode `git rev-parse` output as UTF-8")?; - // A repository path containing a newline would break line-based parsing; - // `rev-parse` has no NUL-terminated mode for these options, so that - // pathology is accepted rather than handled. - let mut lines = stdout.lines(); - match lines.next() { - Some("false") => {} - Some("true") => { - return Err(UsageError::raise(format!( - "{} is inside a bare repository, which has no working tree to root", - start.display() - ))); - } - _ => bail!("`git rev-parse --is-bare-repository` printed neither true nor false"), - } - let Some(common_dir) = lines.next().map(Path::new) else { - bail!("`git rev-parse --git-common-dir` printed no path"); - }; + // The COMMON dir, never the worktree's own: a linked worktree shares it, and + // rooting on the per-worktree directory is what would make two siblings + // resolve to two stores instead of one (CLOUD-164). + // + // The `DISCOVERY_FENCES` scrub that used to happen here and only here is gone + // with the process. `open`'s isolated handle declines the environment + // outright, so an ambient `GIT_CEILING_DIRECTORIES` cannot shape this answer + // and no constant has to be maintained for that to stay true. + let common_dir = repo.common_dir(); + let common_dir = plain( + common_dir + .canonicalize() + .unwrap_or_else(|_| common_dir.to_path_buf()), + ); // The parent is the root only when the common dir is a `/.git`. A // submodule interior or a separate git dir would "derive" a directory that // is not a working tree at all — refuse loudly instead of mis-rooting. @@ -555,11 +527,17 @@ pub fn common_dir(dir: &Path) -> Result { dir.display() ))); } - query( - dir, - &["rev-parse", "--path-format=absolute", "--git-common-dir"], - &format!("{} is not inside a git repository", dir.display()), - ) + let repo = open(dir)?; + // Absolute, as `--path-format=absolute` asked for: the value is recorded as + // store metadata, and a relative one would be read against whatever + // directory the reader happens to be in. + let common = repo.common_dir(); + let absolute = plain( + common + .canonicalize() + .unwrap_or_else(|_| common.to_path_buf()), + ); + Ok(absolute.to_string_lossy().into_owned()) } /// Every configured remote as `(name, url)` pairs, sorted by name. @@ -577,24 +555,29 @@ pub fn common_dir(dir: &Path) -> Result { /// /// Returns an error only when `git` itself cannot run or emits non-UTF-8. pub fn remotes(dir: &Path) -> Result> { - // No remotes configured. `--get-regexp` exits 1 for "no match", which is not - // distinguishable here from a bad invocation — but the invocation is a fixed - // literal, so "no match" is the only reachable cause. - let Ok(listing) = query( - dir, - &["config", "--get-regexp", r"^remote\..*\.url$"], - "read the configured remotes", - ) else { + // No remotes configured is the normal empty case, and so is a directory this + // cannot open — both were an empty list under the shell-out, where a + // non-zero exit could not be told apart from one. + let Ok(repo) = open(dir) else { return Ok(Vec::new()); }; - let mut found: Vec<(String, String)> = listing - .lines() - .filter_map(|line| line.split_once(' ')) - .filter_map(|(key, url)| { - let name = key.strip_prefix("remote.")?.strip_suffix(".url")?; - (!name.is_empty() && !url.is_empty()).then(|| (name.to_owned(), url.to_owned())) - }) - .collect(); + let mut found: Vec<(String, String)> = Vec::new(); + for name in repo.remote_names() { + let name = name.to_string(); + // The FETCH url, exactly once per remote — which is what + // `config --get-regexp remote.*.url` named and what `git remote -v` + // would have printed twice in a format needing re-parsing. + let Ok(remote) = repo.find_remote(name.as_str()) else { + continue; + }; + let Some(url) = remote.url(gix::remote::Direction::Fetch) else { + continue; + }; + let url = url.to_bstring().to_string(); + if !name.is_empty() && !url.is_empty() { + found.push((name, url)); + } + } // `read`-order from git config is file order; a recorded value that a gate // compares must not depend on it. found.sort(); @@ -620,19 +603,31 @@ pub fn remotes(dir: &Path) -> Result> { /// Returns an error only when `git` itself cannot run or emits non-UTF-8. pub fn root_commits(dir: &Path) -> Result> { // An unborn HEAD with no refs at all: no commits to list, not a failure. - let Ok(listing) = query( - dir, - &["rev-list", "--max-parents=0", "--all"], - "list the repository root commits", - ) else { + let Ok(repo) = open(dir) else { return Ok(Vec::new()); }; - let mut found: Vec = listing - .lines() - .map(str::trim) - .filter(|line| !line.is_empty()) - .map(ToOwned::to_owned) + let Ok(references) = repo.references() else { + return Ok(Vec::new()); + }; + let Ok(all) = references.all() else { + return Ok(Vec::new()); + }; + // `--all`: every ref is a tip, and the walk keeps only the commits with no + // parents. Selecting commits, never deciding reachability — the distinction + // this module's ancestry gate draws. + let tips: Vec = all + .filter_map(std::result::Result::ok) + .filter_map(|reference| reference.into_fully_peeled_id().ok()) + .map(gix::Id::detach) .collect(); + let mut found: Vec = Vec::new(); + if let Ok(walk) = repo.rev_walk(tips).all() { + for info in walk.flatten() { + if info.parent_ids().count() == 0 { + found.push(info.id().to_hex().to_string()); + } + } + } found.sort(); found.dedup(); Ok(found) @@ -660,12 +655,77 @@ pub fn root_commits(dir: &Path) -> Result> { /// Returns a [`UsageError`] (→ exit `1`) when `dir` is not inside a repository /// this binary can open. fn open(dir: &Path) -> Result { - gix::discover_opts( - dir, - gix::discover::upwards::Options::default(), - gix::open::Options::isolated(), - ) - .map_err(|_| UsageError::raise(format!("{} is not a git repository", dir.display()))) + open_upwards(dir, ceiling_dirs()) +} + +/// `GIT_CEILING_DIRECTORIES`, as discovery ceilings. +/// +/// **The one environment variable this module still reads, and the asymmetry is +/// deliberate.** [`gix::open::Options::isolated`] declines the environment as a +/// class, which is right for everything that could REDIRECT an answer: a +/// `GIT_DIR` or `GIT_WORK_TREE` names a different repository, and honouring one +/// is the mis-rooting bug this module exists to kill. A ceiling cannot redirect. +/// It can only stop the walk earlier, so its worst outcome is a refusal — the +/// fail-safe direction — and a caller that fenced discovery on purpose is +/// entitled to have the fence respected rather than walked straight past. +/// +/// This is not `gix::discover`'s `_with_environment_overrides`, which re-admits +/// the redirecting variables too. Only the ceiling is read, and only here. +fn ceiling_dirs() -> Vec { + std::env::var_os("GIT_CEILING_DIRECTORIES") + .map(|raw| std::env::split_paths(&raw).collect()) + .unwrap_or_default() +} + +/// [`open`], with the discovery ceilings supplied rather than read. +fn open_upwards(dir: &Path, ceilings: Vec) -> Result { + let discovery = gix::discover::upwards::Options { + ceiling_dirs: ceilings, + ..Default::default() + }; + // ABSOLUTE before discovery, because callers pass a relative `"."` + // (`receipt.rs` does, for every read) and a ceiling is an absolute path. An + // upward walk over relative components can never match one, so a fence a + // caller set would be walked straight past — silently, and only for the + // callers that pass a relative path. `git -C .` resolved the working + // directory before comparing; this is that step, made explicit. + let start = dir.canonicalize(); + let start = start.as_deref().unwrap_or(dir); + gix::discover_opts(start, discovery, gix::open::Options::isolated()) + .map_err(|_| UsageError::raise(format!("{} is not a git repository", dir.display()))) +} + +/// Open the repository containing `dir` with git's **resolved** configuration — +/// system, global and repository-local together. +/// +/// The one deliberate exception to [`open`], and the distinction it rests on is +/// worth stating because collapsing the two would be a silent behaviour change. +/// [`open`]'s isolation exists to stop the ambient environment deciding **which +/// repository** an answer is about: a stray `GIT_DIR` redirecting discovery is +/// the mis-rooting bug class this module exists to kill. It is not a claim that +/// git's configuration is untrustworthy to READ. +/// +/// Two callers ask a question whose subject IS the resolved configuration — +/// [`config_value`] and [`stamped_identity`], both of which feed the attribution +/// decision. "Is there an accountable identity here at all" is answered by an +/// identity inherited from a wider scope just as much as by a local one, so +/// reading these through an isolated handle would report `None` for a developer +/// whose `user.email` is set globally, and the attribution gate would refuse a +/// correctly-configured machine. +/// +/// DISCOVERY still runs isolated: the path is found by [`open`] and only then +/// re-opened for its configuration, so the ambient environment picks neither the +/// repository nor the answer — only the config scopes git itself would consult +/// contribute. +/// +/// # Errors +/// +/// Returns a [`UsageError`] (→ exit `1`) when `dir` is not inside a repository +/// this binary can open. +fn open_configured(dir: &Path) -> Result { + let isolated = open(dir)?; + gix::open(isolated.git_dir()) + .map_err(|_| UsageError::raise(format!("{} is not a git repository", dir.display()))) } /// Read a tracked file's contents at a git ref, without touching the working @@ -899,116 +959,6 @@ pub fn list_tree(dir: &Path, reference: &str, directory: &str) -> Result usize { - QUERIES_SPAWNED.load(Ordering::Relaxed) -} - -/// The `git` child every query in this module is built from: `-C dir`, with -/// the redirect variables scrubbed so the answer is about the directory it was -/// handed and not about whatever repository the ambient environment names. -#[expect( - clippy::disallowed_types, - reason = "stays: the ONE git invoker (`no_second_git_invoker_exists` keeps it one), taking fixed argv with no caller token, measured at 6.7ms of the 100ms mediated-call budget — so nothing measured asks it to move (CLOUD-770)" -)] -fn command(dir: &Path) -> Command { - QUERIES_SPAWNED.fetch_add(1, Ordering::Relaxed); - let mut command = Command::new("git"); - command.arg("-C").arg(dir); - for var in DISCOVERY_REDIRECTS { - command.env_remove(var); - } - command -} - -/// Run a fixed, read-only `git` query in `dir` and return its trimmed stdout. -/// -/// The one git-plumbing entry point for the rest of the crate — `receipt.rs` -/// called a private copy of this before CLOUD-36 collapsed them, and -/// `no_second_git_invoker` is what keeps a third from appearing. -/// -/// # Errors -/// -/// A non-zero exit is the *expected* bad-checkout condition and raises a -/// [`UsageError`] (exit `1`) carrying `refusal` — git's own stderr is -/// version-dependent prose and never reaches the caller, so the message stays -/// deterministic. Failing to run `git` at all, or output that is not UTF-8, is -/// an internal error (exit `3`). -fn query(dir: &Path, args: &[&str], refusal: &str) -> Result { - let bytes = query_bytes(dir, args, refusal)?; - let stdout = String::from_utf8(bytes).map_err(|_| { - UsageError::raise(format!( - "`git {}` output is not valid UTF-8", - args.join(" ") - )) - })?; - Ok(stdout.trim_end_matches(['\r', '\n']).to_owned()) -} - -/// [`query`] without the UTF-8 requirement, for output that may carry raw -/// pathnames or file content. -/// -/// # Errors -/// -/// As [`query`], minus the decoding failure. -fn query_bytes(dir: &Path, args: &[&str], refusal: &str) -> Result> { - let output = command(dir) - .args(args) - .stderr(Stdio::null()) - .output() - .with_context(|| format!("run `git {}`", args.join(" ")))?; - if !output.status.success() { - return Err(UsageError::raise(refusal)); - } - Ok(output.stdout) -} - -/// [`query`] for a question whose answer may legitimately be "there is none". -/// -/// Returns `None` when git exits non-zero, rather than raising. Only for a query -/// where a non-zero exit *is* an answer — `@{upstream}` on a branch that has no -/// upstream is the case this exists for, and there is no ref-existence test that -/// does not itself have to be spelled as a failing lookup. A caller that would -/// treat an absent answer as a *pass* must not use this: absence of an upstream -/// is not safety (CLOUD-51), so the caller owes the absent case its own reading. -/// -/// # Errors -/// -/// Failing to run `git` at all, or output that is not UTF-8, is still an -/// internal error — only the *verdict* is optional, never the mechanism. -fn query_optional(dir: &Path, args: &[&str]) -> Result> { - let output = command(dir) - .args(args) - .stderr(Stdio::null()) - .output() - .with_context(|| format!("run `git {}`", args.join(" ")))?; - if !output.status.success() { - return Ok(None); - } - let stdout = String::from_utf8(output.stdout) - .with_context(|| format!("decode `git {}` output as UTF-8", args.join(" ")))?; - Ok(Some(stdout.trim_end_matches(['\r', '\n']).to_owned())) -} - /// How many entries the working tree reports as not committed. /// /// A **count, not a list**, and deliberately so: the report this feeds says @@ -1024,12 +974,12 @@ fn query_optional(dir: &Path, args: &[&str]) -> Result> { /// /// Raises a [`UsageError`] (exit `1`) when `dir` is not inside a repository. pub fn uncommitted(dir: &Path) -> Result { - let status = query( - dir, - &["status", "--porcelain"], - "cannot read the working tree status; this is not a git repository", - )?; - Ok(status.lines().filter(|line| !line.is_empty()).count()) + // A COUNT, and the list it counts never leaves this module (non-negotiable + // rule 4). Sharing `working_tree_changes` with `changed_paths` is what keeps + // the two from disagreeing about what "changed" means — under the shell-out + // one counted `status --porcelain` lines and the other unioned `diff HEAD` + // with `ls-files --others`, which are nearly but not exactly the same set. + Ok(working_tree_changes(dir)?.len()) } /// The git blob id `git hash-object` would give this text (CLOUD-1024). @@ -1094,10 +1044,15 @@ pub fn resolve_ref(dir: &Path, name: &str) -> Result> { // safe answer. `rev-parse` also has no file-writing option, so there is no // `show`-shaped write here (CLOUD-718). The token makes that hold by // construction instead of by two other functions' behaviour. - query_optional( - dir, - &["rev-parse", "--verify", "--quiet", "--end-of-options", name], - ) + let repo = open(dir)?; + // `--end-of-options` has nothing to be carried on: `name` is an argument to + // a resolver, never a token on a command line, so an option-shaped value is + // a ref that does not resolve rather than a flag. That is the difference + // between refusing an injection and it being unrepresentable (CLOUD-718). + Ok(repo + .rev_parse_single(name) + .ok() + .map(|id| id.detach().to_hex().to_string())) } /// The repo-relative paths the working tree has changed against `HEAD`. @@ -1127,23 +1082,109 @@ pub fn resolve_ref(dir: &Path, name: &str) -> Result> { /// Raises a [`UsageError`] (exit `1`) when `dir` is not inside a repository, or /// is one with no commits. pub fn changed_paths(dir: &Path) -> Result> { + working_tree_changes(dir) +} + +/// Every repo-relative path that differs from `HEAD`, staged, unstaged or +/// untracked. +/// +/// The one walk behind [`uncommitted`]'s count and [`changed_paths`]' list. +/// +/// # Why this is hand-rolled rather than gix's `status` +/// +/// gix can answer this, and the feature that does is REFUSED for the reason +/// CLOUD-739 refused `gix-diff/blob`: `status` pulls `blob-diff`, and `dirwalk` +/// pulls `attributes`, which pulls `command`. That is the external-diff-driver, +/// clean/smudge-filter and materialise-blobs-to-disk surface the previous slice +/// declined — a runtime subshell and unmediated filesystem access, arriving +/// through a dependency rather than through this crate's own source. Taking it +/// to delete a spawn would be buying the thing the spawn was being deleted for. +/// +/// So the three sources are read from what is already vendored: the INDEX (gix's +/// `index` feature, already on via `revision`), the HEAD tree, and the `ignore` +/// crate's walker for untracked files — the same walker +/// [`crate::rules::tree_files`] uses, so untracked-and-ignored means here what it +/// means there. +/// +/// **The cost, stated rather than absorbed: clean/smudge filters are not +/// applied.** A repository that rewrites content on checkout — CRLF conversion, +/// an LFS pointer — can therefore show a file as modified whose committed content +/// is unchanged. That is the OVER-reporting direction, and it is the safe one +/// here: both callers ask "is there uncommitted work", where a false positive is +/// noise and a false negative is work reported as safe to lose. `stop` and +/// `baseline` both read this, and a container reclaim takes what they said was +/// not there. +/// +/// # Errors +/// +/// Raises a [`UsageError`] (exit `1`) when `dir` is not inside a repository, or +/// is one whose index or `HEAD` cannot be read. +fn working_tree_changes(dir: &Path) -> Result> { + let repo = open(dir)?; + let root = repo_root(dir)?; + let refusal = || { + UsageError::raise( + "cannot read the changed paths; this is not a git repository, or it has no commits" + .to_owned(), + ) + }; + let index = repo.index().map_err(|_| refusal())?; let mut changed = BTreeSet::new(); - for args in [ - &["diff", "--name-only", "-z", "--end-of-options", "HEAD"][..], - &["ls-files", "--others", "--exclude-standard", "-z"][..], - ] { - let bytes = query_bytes( - dir, - args, - "cannot read the changed paths; this is not a git repository, or it has no commits", - )?; - changed.extend( - bytes - .split(|byte| *byte == 0) - .filter(|path| !path.is_empty()) - .filter_map(|path| std::str::from_utf8(path).ok()) - .map(ToOwned::to_owned), - ); + + // Staged: the index against `HEAD`'s tree. An unborn HEAD has no tree, so + // every index entry is staged — which is what it is. + let head_tree = repo + .head_commit() + .ok() + .and_then(|commit| commit.tree().ok()); + let mut tracked = BTreeSet::new(); + for entry in index.entries() { + // A path is bytes; one that is not UTF-8 is dropped rather than lossily + // converted, as the `-z` reading this replaces already did. + let Ok(path) = std::str::from_utf8(entry.path(&index)) else { + continue; + }; + tracked.insert(path.to_owned()); + let committed = head_tree + .as_ref() + .and_then(|tree| tree.clone().peel_to_entry_by_path(path).ok().flatten()) + .map(|found| found.object_id()); + if committed != Some(entry.id) { + changed.insert(path.to_owned()); + continue; + } + // Unstaged: the index entry against the file on disk. Compared by CONTENT + // hash rather than by stat, because a stat match is a cache hint and this + // is being asked whether work exists. + let absolute = root.join(path); + let Ok(metadata) = std::fs::symlink_metadata(&absolute) else { + // Tracked and gone is a deletion, which is a change. + changed.insert(path.to_owned()); + continue; + }; + let content = if metadata.is_symlink() { + std::fs::read_link(&absolute) + .map(|target| target.to_string_lossy().into_owned().into_bytes()) + } else { + std::fs::read(&absolute) + }; + let Ok(content) = content else { + changed.insert(path.to_owned()); + continue; + }; + let hashed = gix::objs::compute_hash(repo.object_hash(), gix::object::Kind::Blob, &content) + .map_err(|_| refusal())?; + if hashed != entry.id { + changed.insert(path.to_owned()); + } + } + + // Untracked: the crate's one tree walker, so "ignored" means here exactly + // what it means to every rule that reads the tree. + for path in crate::rules::tree_files(&root)? { + if !tracked.contains(&path) { + changed.insert(path); + } } Ok(changed) } @@ -1175,15 +1216,23 @@ pub fn changed_paths(dir: &Path) -> Result> { /// caller reads that as could-not-look and allows, so a tree this cannot /// enumerate is never refused on the strength of a count nobody took. pub fn tracked_paths(dir: &Path) -> Result> { - let bytes = query_bytes( - dir, - &["ls-files", "-z"], - "cannot read the tracked paths; this is not a git repository", - )?; - Ok(bytes - .split(|byte| *byte == 0) + let repo = open(dir)?; + // The INDEX is what `ls-files` printed, so this is the same membership test + // rather than a similar one — which CLOUD-312's differential obligation + // needs, since a different test would diverge on exactly the paths a + // migration is supposed to preserve. + let index = repo.index().map_err(|_| { + UsageError::raise("cannot read the tracked paths; this is not a git repository".to_owned()) + })?; + Ok(index + .entries() + .iter() + // A path is bytes. One that is not UTF-8 is DROPPED rather than lossily + // converted, exactly as the `-z` reading was: a mangled path fails to + // match a tracked entry and silently lowers a count, and dropping it is + // the same permissive direction stated out loud. + .filter_map(|entry| std::str::from_utf8(entry.path(&index)).ok()) .filter(|path| !path.is_empty()) - .filter_map(|path| std::str::from_utf8(path).ok()) .map(ToOwned::to_owned) .collect()) } @@ -1194,14 +1243,16 @@ pub fn tracked_paths(dir: &Path) -> Result> { /// /// Raises a [`UsageError`] (exit `1`) when `dir` is not inside a repository. pub fn current_branch(dir: &Path) -> Result> { - let name = query( - dir, - &["rev-parse", "--abbrev-ref", "HEAD"], - "cannot resolve HEAD; this is not a git repository, or it has no commits", - )?; - // git spells a detached HEAD as the literal `HEAD`, which is not a branch - // name; reporting it as one would name a branch that does not exist. - Ok((name != "HEAD").then_some(name)) + let repo = open(dir)?; + let head = repo.head().map_err(|_| { + UsageError::raise( + "cannot resolve HEAD; this is not a git repository, or it has no commits".to_owned(), + ) + })?; + // A detached HEAD has no referent name at all, where `--abbrev-ref` spelled + // it as the literal `HEAD` and every caller had to know not to read that as + // a branch. `None` is the same answer with the trap removed. + Ok(head.referent_name().map(|name| name.shorten().to_string())) } /// Whether this checkout's history is truncated (CLOUD-446). @@ -1226,8 +1277,13 @@ pub fn current_branch(dir: &Path) -> Result> { /// establish that history is complete is not the same as establishing that it /// is. pub fn is_shallow(dir: &Path) -> Result { - let answer = query_optional(dir, &["rev-parse", "--is-shallow-repository"])?; - Ok(answer.is_none_or(|answer| answer.trim() != "false")) + // A directory this cannot open is read as the conservative `true`, matching + // what an unreadable answer meant before: unable to establish that history + // is complete is not the same as establishing that it is. + let Ok(repo) = open(dir) else { + return Ok(true); + }; + Ok(repo.is_shallow()) } /// The commit messages on `base..HEAD`, as one blob (CLOUD-446). @@ -1249,20 +1305,37 @@ pub fn is_shallow(dir: &Path) -> Result { /// Raises when `git` cannot be run at all, or its output is not UTF-8 — only the /// verdict is optional, never the mechanism. pub fn log_messages(dir: &Path, base: &str) -> Result> { - let range = format!("{base}..HEAD"); - query_optional( - dir, - &["log", "--format=%B", "--end-of-options", &range, "--"], - ) + // A `base` this cannot resolve is COULD NOT LOOK, and the mediated call it + // gates allows — the same fail-open posture the shell-out's non-zero exit + // carried, now spelled as a value rather than as an exit status. + let Ok(repo) = open(dir) else { + return Ok(None); + }; + let (Ok(base_id), Ok(head_id)) = (repo.rev_parse_single(base), repo.head_id()) else { + return Ok(None); + }; + let Ok(walk) = repo + .rev_walk([head_id.detach()]) + .with_hidden([base_id.detach()]) + .all() + else { + return Ok(None); + }; + let mut messages = String::new(); + for info in walk.flatten() { + let Ok(commit) = repo.find_commit(info.id) else { + continue; + }; + // `%B` is the raw body, and `git log --format=%B` separated records with + // a newline. Every caller asks whether an expression matches ANYWHERE in + // the work's own commits, so the join only has to keep two messages from + // running into one another. + messages.push_str(&commit.message_raw_sloppy().to_string()); + messages.push('\n'); + } + Ok(Some(messages)) } -/// The field separator [`commit_record`] joins its four fields with. -/// -/// U+001E RECORD SEPARATOR: a control character no identity, trailer or subject -/// carries in practice — and, crucially, one whose *presence* in a body is now -/// an error rather than a silent mis-split (CLOUD-742). -const RECORD_SEPARATOR: &str = "\u{1e}"; - /// One commit's attribution record: who wrote it, who committed it, what it /// trails, and what it says. /// @@ -1305,70 +1378,44 @@ pub struct CommitRecord { /// its record does not carry all four fields — "could not look", never a /// verdict built out of blanks. pub fn commit_record(dir: &Path, commit: &str) -> Result { - let format = format!( - "%an <%ae>{RECORD_SEPARATOR}%cn <%ce>{RECORD_SEPARATOR}%(trailers:only,unfold)\ - {RECORD_SEPARATOR}%B" - ); - let shown = query( - dir, - &[ - "show", - "-s", - &format!("--format={format}"), - "--end-of-options", - commit, - ], - "could not read a commit in the range", - )?; - record_from(&shown, commit) -} - -/// The destructure [`commit_record`] performs, separated from the invocation -/// that produces its input. -/// -/// Its own function because the failing condition is a *record shape* and not a -/// repository state: a caller cannot easily make `git show` emit a short record -/// on demand, so the decision is extracted and tested directly rather than -/// through a fixture that asserts its own premise (`.claude/rules/rust.md`, -/// CLOUD-249). -fn record_from(shown: &str, commit: &str) -> Result { - let mut parts = shown.splitn(4, RECORD_SEPARATOR); - let (Some(author), Some(committer), Some(trailers), Some(body)) = - (parts.next(), parts.next(), parts.next(), parts.next()) - else { - return Err(UsageError::raise(format!( - "the record for {} does not carry four fields, so its attribution cannot be read", - short(commit) - ))); - }; + let repo = open(dir)?; + let refusal = || UsageError::raise("could not read a commit in the range".to_owned()); + let object = repo + .rev_parse_single(commit) + .map_err(|_| refusal())? + .object() + .map_err(|_| refusal())?; + let object = object.peel_to_commit().map_err(|_| refusal())?; + let author = object.author().map_err(|_| refusal())?; + let committer = object.committer().map_err(|_| refusal())?; + // The four fields are read as FIELDS now, so there is no separator to join + // them with and no record that can arrive short. `RECORD_SEPARATOR`, + // `record_from` and its arity refusal all existed because one `git show` had + // to carry four values through one stream, and a body containing U+001E + // mis-split it (CLOUD-742). Reading the commit object removes that channel + // rather than defending it, so the three go with it — a defect class with no + // channel left has nothing for a gate to discriminate (CLOUD-418), which is + // the same reasoning this row's own §7 used to strike its clauses over + // deleted functions. `trailer_lines` STAYS: `attribution.rs` reads a pending + // message's trailers through it, and one implementation is what keeps a + // committed record and a pending one agreeing on what a trailer line is. + let message = object.message().map_err(|_| refusal())?; + let trailers = message + .body() + .map(|body| { + body.trailers() + .map(|trailer| format!("{}: {}", trailer.token, trailer.value)) + .collect::>() + }) + .unwrap_or_default(); Ok(CommitRecord { - author: author.to_owned(), - committer: committer.to_owned(), - trailers: trailer_lines(trailers), - body: body.to_owned(), + author: format!("{} <{}>", author.name, author.email), + committer: format!("{} <{}>", committer.name, committer.email), + trailers, + body: object.message_raw_sloppy().to_string(), }) } -/// Split a trailer block into whole `Key: value` lines, dropping blanks. -/// -/// `pub(crate)` rather than private because `attribution.rs` reads a *pending* -/// message's trailers through the same shape and asserts this splitting -/// directly; one implementation, so a committed record and a pending one cannot -/// disagree about what a trailer line is. -pub(crate) fn trailer_lines(block: &str) -> Vec { - block - .lines() - .map(str::trim_end) - .filter(|line| !line.trim().is_empty()) - .map(ToOwned::to_owned) - .collect() -} - -/// A commit's short form, as every pointer in this repository renders it. -fn short(commit: &str) -> String { - commit.chars().take(8).collect() -} - /// Every non-merge commit in `base..head`, as full SHAs. /// /// The enumeration half of an attribution run: [`commit_record`] is what reads @@ -1380,17 +1427,28 @@ fn short(commit: &str) -> String { /// Raises a [`UsageError`] (exit `1`) when the range does not resolve — "could /// not look", never a clean pass over commits nobody read. pub fn commits_in_range(dir: &Path, base: &str, head: &str) -> Result> { - let range = format!("{base}..{head}"); - let listed = query( - dir, - &["rev-list", "--no-merges", "--end-of-options", &range, "--"], - "could not resolve the commit range", - )?; - Ok(listed - .lines() - .filter(|line| !line.trim().is_empty()) - .map(ToOwned::to_owned) - .collect()) + let repo = open(dir)?; + let refused = || UsageError::raise("could not resolve the commit range".to_owned()); + let (base_id, head_id) = ( + repo.rev_parse_single(base).map_err(|_| refused())?, + repo.rev_parse_single(head).map_err(|_| refused())?, + ); + let walk = repo + .rev_walk([head_id.detach()]) + .with_hidden([base_id.detach()]) + .all() + .map_err(|_| refused())?; + let mut out = Vec::new(); + for step in walk { + let info = step.map_err(|_| refused())?; + // `--no-merges`: a merge has no patch of its own, and the commits it + // brings in are separately enumerated here. + if info.parent_ids().count() > 1 { + continue; + } + out.push(info.id().to_hex().to_string()); + } + Ok(out) } /// The trailers of a message that is on disk and not yet committed. @@ -1403,13 +1461,21 @@ pub fn commits_in_range(dir: &Path, base: &str, head: &str) -> Result Result> { - let path = message.to_string_lossy().into_owned(); - let parsed = query( - dir, - &["interpret-trailers", "--parse", "--", &path], - "could not parse the pending message's trailers", - )?; - Ok(trailer_lines(&parsed)) + let _ = dir; + let body = std::fs::read(message).map_err(|_| { + UsageError::raise("could not parse the pending message's trailers".to_owned()) + })?; + // The SAME parser `commit_record` reads a committed message with, which is + // what the `interpret-trailers` shell-out bought and what would otherwise be + // re-derived here: where a trailer block starts is git's rule, and a second + // implementation of it could disagree with what `commit_record` reports once + // the commit exists. + let message = gix::objs::commit::MessageRef::from_bytes(&body); + Ok(message.body().map_or_else(Vec::new, |body| { + body.trailers() + .map(|trailer| format!("{}: {}", trailer.token, trailer.value)) + .collect() + })) } /// One config value as git *resolves* it, across every scope. @@ -1423,7 +1489,15 @@ pub fn message_trailers(dir: &Path, message: &Path) -> Result> { /// Failing to run `git` at all is an internal error (exit `3`); an unset key is /// `None`, which is an answer. pub fn config_value(dir: &Path, key: &str) -> Result> { - query_optional(dir, &["config", "--get", "--end-of-options", key]) + let repo = open_configured(dir)?; + // RESOLVED across every scope, which is why this reads through + // `open_configured` rather than `open` — see that function on why the + // isolation is about which repository, never about whether config is + // readable. + Ok(repo + .config_snapshot() + .string(key) + .map(|value| value.to_string())) } /// Set one **repo-local** config value. @@ -1436,11 +1510,37 @@ pub fn config_value(dir: &Path, key: &str) -> Result> { /// /// Raises a [`UsageError`] (exit `1`) when the write fails. pub fn set_config_local(dir: &Path, key: &str, value: &str) -> Result<()> { - query( - dir, - &["config", "--local", "--end-of-options", key, value], - "could not write the repo-local config value", - )?; + let repo = open(dir)?; + let refusal = || UsageError::raise("could not write the repo-local config value".to_owned()); + // `user.name`, or `remote.origin.url` — section, an optional subsection, and + // the key. The shell-out handed git one dotted string and let it do this + // split; doing it here is what a typed API costs, and it is the same split. + let (section, rest) = key.split_once('.').ok_or_else(refusal)?; + let (subsection, name) = match rest.rsplit_once('.') { + Some((subsection, name)) => (Some(subsection), name), + None => (None, rest), + }; + // THE REPOSITORY'S OWN CONFIG FILE, opened directly rather than through + // `config_snapshot_mut`. That snapshot spans every scope, and committing it + // did not REPLACE an existing local value — measured by + // `a_repo_local_config_write_replaces_an_existing_value`, which read back the + // value the write was supposed to overwrite. A write primitive that reports + // success while leaving the old value in place is the worst possible shape + // for this caller: `attribution identity` uses it to displace a denied + // committer, so a silent no-op leaves every later commit misattributed while + // the repair claims to have run. + // + // Repo-local is now structural rather than a flag: this is the local file, so + // there is no `--global` for a caller to reach and no wider scope reachable + // by omission. + let path = repo.git_dir().join("config"); + let mut file = + gix::config::File::from_path_no_includes(path.clone(), gix::config::Source::Local) + .map_err(|_| refusal())?; + file.set_raw_value_by(section, subsection.map(gix::bstr::BStr::new), name, value) + .map_err(|_| refusal())?; + let mut out = std::fs::File::create(&path).map_err(|_| refusal())?; + file.write_to(&mut out).map_err(|_| refusal())?; Ok(()) } @@ -1454,18 +1554,20 @@ pub fn set_config_local(dir: &Path, key: &str, value: &str) -> Result<()> { /// /// Raises a [`UsageError`] (exit `1`) when git cannot resolve an identity. pub fn stamped_identity(dir: &Path, var: &str) -> Result { - // No `--end-of-options`: `git var` does not accept the token — it takes - // `-l` or exactly one variable name — and it does not need it, because the - // two names this is ever called with are literals in this crate rather than - // anything a caller supplies. - let raw = query( - dir, - &["var", var], - "could not resolve the identity git would stamp", - )?; - Ok(raw - .rfind('>') - .map_or_else(|| raw.trim().to_owned(), |end| raw[..=end].to_owned())) + let repo = open_configured(dir)?; + let refusal = || UsageError::raise("could not resolve the identity git would stamp".to_owned()); + // `git var GIT_AUTHOR_IDENT` printed `Name ` and the + // caller trimmed the time back off. The identity is read as an identity now, + // so there is no timestamp to append and none to remove — the trim that used + // to live beside the invocation has nothing left to do. + let identity = match var { + "GIT_AUTHOR_IDENT" => repo.author(), + "GIT_COMMITTER_IDENT" => repo.committer(), + _ => return Err(refusal()), + } + .ok_or_else(refusal)? + .map_err(|_| refusal())?; + Ok(format!("{} <{}>", identity.name, identity.email)) } /// The absolute git directory for `dir` — **per-worktree**, not the common one. @@ -1484,12 +1586,17 @@ pub fn stamped_identity(dir: &Path, var: &str) -> Result { /// Raises a [`UsageError`] (exit `1`) when `dir` is not inside a repository — /// "could not look", never an answer about a repository that is not there. pub fn git_dir(dir: &Path) -> Result { - let printed = query( - dir, - &["rev-parse", "--absolute-git-dir"], - "not a git repository, so there is no git directory to resolve", - )?; - Ok(PathBuf::from(printed.trim())) + let repo = open(dir)?; + // PER-WORKTREE, which is the whole reason this and `common_dir` both exist: + // `git_dir()` is the linked worktree's own directory where `common_dir()` is + // the shared one, and a receipt keyed through the wrong one answers about a + // different checkout than the one being judged. + let git_dir = repo.git_dir(); + Ok(plain( + git_dir + .canonicalize() + .unwrap_or_else(|_| git_dir.to_path_buf()), + )) } /// How many commits `range` selects. @@ -1511,14 +1618,26 @@ pub fn git_dir(dir: &Path) -> Result { /// therefore refused rather than defaulted, since it would mean git answered a /// different question. pub fn commit_count(dir: &Path, range: &str) -> Result { - let printed = query( - dir, - &["rev-list", "--count", "--end-of-options", range, "--"], - "the commit range cannot be counted", - )?; - printed.trim().parse().map_err(|_| { - UsageError::raise("`git rev-list --count` did not answer with a number".to_owned()) - }) + let repo = open(dir)?; + let refused = || UsageError::raise("the commit range cannot be counted".to_owned()); + let (exclude, include) = match range.split_once("..") { + Some((from, to)) => (Some(from), to), + None => (None, range), + }; + let tip = repo.rev_parse_single(include).map_err(|_| refused())?; + let mut walk = repo.rev_walk([tip.detach()]); + if let Some(from) = exclude { + walk = walk.with_hidden([repo.rev_parse_single(from).map_err(|_| refused())?.detach()]); + } + // A COUNT, and nothing about reachability: selecting which commits to count + // is a different act from concluding one commit contains another (CLOUD-36). + // The parse that could answer a different question is gone with the text. + let mut counted = 0; + for step in walk.all().map_err(|_| refused())? { + step.map_err(|_| refused())?; + counted += 1; + } + Ok(counted) } /// One commit's subject line, keyed to the commit that carries it. @@ -1557,59 +1676,100 @@ pub struct CommitSubject { /// `%H %s` cannot produce one, so seeing one means the walk answered something /// other than the question asked. pub fn subjects_in_range(dir: &Path, base: &str, head: &str) -> Result> { - let range = format!("{base}..{head}"); - let listed = query( - dir, - &[ - "log", - "--no-merges", - "--format=%H %s", - "--end-of-options", - &range, - "--", - ], - "could not resolve the commit range", - )?; - listed - .lines() - .filter(|line| !line.trim().is_empty()) - .map(|line| { - let (commit, subject) = line.split_once(' ').ok_or_else(|| { - UsageError::raise( - "a commit line carries no subject field, so the range cannot be read", - ) - })?; - Ok(CommitSubject { - commit: commit.to_owned(), - subject: subject.to_owned(), - }) - }) - .collect() + let repo = open(dir)?; + let refused = || UsageError::raise("could not resolve the commit range".to_owned()); + let (base_id, head_id) = ( + repo.rev_parse_single(base).map_err(|_| refused())?, + repo.rev_parse_single(head).map_err(|_| refused())?, + ); + let walk = repo + .rev_walk([head_id.detach()]) + .with_hidden([base_id.detach()]) + .all() + .map_err(|_| refused())?; + let mut out = Vec::new(); + for step in walk { + let info = step.map_err(|_| refused())?; + if info.parent_ids().count() > 1 { + continue; + } + let commit = repo.find_commit(info.id).map_err(|_| refused())?; + // The `%H %s` line and the split that undid it are both gone: the + // subject is a field of the message, so there is no first-space rule to + // hold half of and no line-without-a-space to refuse. `summary()` is + // git's own `%s` — the message up to the first blank line, folded. + out.push(CommitSubject { + commit: info.id().to_hex().to_string(), + subject: commit + .message() + .map_err(|_| refused())? + .summary() + .to_string(), + }); + } + Ok(out) } -/// Whether git ignores `path` — the scratch-work question (CLOUD-444). +/// Whether this repository ignores `path` — the scratch-work question +/// (CLOUD-444). +/// +/// # One implementation, and which one (CLOUD-740 §7(e)) /// -/// `check-ignore` rather than a reimplementation of the ignore rules: the -/// precedence between a repository's `.gitignore`, its excludes file and its -/// global config is git's own, and a second implementation of it would disagree -/// on exactly the layered cases a consumer relies on. +/// This crate must not carry two answers to "is this path ignored", and it was +/// about to: `ignore` is already vendored and owns the question for +/// [`crate::rules::tree_files`]'s walk, `git check-ignore` owned it here, and +/// gix's own exclude machinery would have been a third. **`ignore` owns it**, +/// for two reasons that both point the same way. It is the implementation whose +/// answers a consumer already depends on, since the walk decides which files +/// every `forbid`, `budget` and marker rule even sees. And gix's excludes arrive +/// only through the `excludes` feature, which pulls `gix-worktree` — part of the +/// same materialise-blobs-to-disk surface CLOUD-739 declined, so taking it here +/// would buy a third answer with the dependency the previous slice refused. /// -/// Built on [`query_optional`], whose contract this fits exactly: `check-ignore` -/// spells "not ignored" as **exit 1**, an answer rather than a failure. The -/// direction of the absent case is the one to read carefully — here a `false` is -/// "not ignored", which makes the path *judgeable*, so a git that cannot answer -/// must not silently produce `false`; that is why a failure to run git at all -/// still raises rather than returning `Ok(false)`. +/// **The posture is the walk's, deliberately, and it is NARROWER than +/// `check-ignore` was.** `tree_files` sets `git_global(false)` because a +/// developer's global excludes are a property of their machine and a gate whose +/// file set varies per workstation is not one gate. `git check-ignore` consulted +/// `core.excludesFile` and so could answer differently on two machines for the +/// same commit. Matching the walk is what makes the two agree; the cost is that a +/// path ignored ONLY by a developer's global excludes now reads as not ignored, +/// which is the judgeable direction and the same one the walk already took. /// -/// `--` separates the pathspec from the flags, so a path beginning with a dash is -/// asked about rather than parsed as one. +/// The layering is the repository's own, applied in git's precedence order: +/// `.git/info/exclude` first, then each `.gitignore` from the root down to the +/// path's own directory, so a nearer file overrides a farther one. /// /// # Errors /// -/// Raises when `git` cannot be run at all, or its output is not UTF-8 — only the -/// verdict is optional, never the mechanism. +/// Raises when the repository cannot be opened or its ignore files cannot be +/// read — only the VERDICT is optional, never the mechanism. `false` here means +/// "not ignored", which makes the path judgeable, so an unreadable ignore +/// surface must never quietly produce one. pub fn check_ignore(dir: &Path, path: &str) -> Result { - Ok(query_optional(dir, &["check-ignore", "--quiet", "--", path])?.is_some()) + let repo = open(dir)?; + let root = repo_root(dir)?; + let refusal = || UsageError::raise("cannot read the repository's ignore rules".to_owned()); + let mut builder = ignore::gitignore::GitignoreBuilder::new(&root); + // `.git/info/exclude` first: git's lowest-precedence repository source, and + // `ignore`'s builder takes later additions as higher precedence. + let excludes = repo.git_dir().join("info").join("exclude"); + if excludes.is_file() { + builder.add(&excludes); + } + // Then root-down, so a `.gitignore` nearer the path overrides a farther one. + let mut walked = root.clone(); + builder.add(walked.join(".gitignore")); + for component in Path::new(path).parent().into_iter().flatten() { + walked.push(component); + builder.add(walked.join(".gitignore")); + } + let matcher = builder.build().map_err(|_| refusal())?; + // A path is judged as a FILE unless the caller's own path says otherwise; the + // matcher needs to know, because a `foo/` rule matches a directory only. + let is_dir = root.join(path).is_dir(); + Ok(matcher + .matched_path_or_any_parents(path, is_dir) + .is_ignore()) } /// The commit `HEAD` points at, as a full SHA. @@ -1622,11 +1782,18 @@ pub fn check_ignore(dir: &Path, path: &str) -> Result { /// Raises a [`UsageError`] (exit `1`) when `dir` is not inside a repository or /// has no commits. pub fn head_commit(dir: &Path) -> Result { - query( - dir, - &["rev-parse", "--verify", "--end-of-options", "HEAD^{commit}"], - "cannot resolve HEAD; this is not a git repository, or it has no commits", - ) + let repo = open(dir)?; + let refusal = || { + UsageError::raise( + "cannot resolve HEAD; this is not a git repository, or it has no commits".to_owned(), + ) + }; + Ok(repo + .head_id() + .map_err(|_| refusal())? + .detach() + .to_hex() + .to_string()) } /// Every local branch and remote-tracking ref, as full ref names. @@ -1641,22 +1808,22 @@ pub fn head_commit(dir: &Path) -> Result { /// /// Raises a [`UsageError`] (exit `1`) when `dir` is not inside a repository. pub fn refs(dir: &Path) -> Result> { - let listing = query( - dir, - &[ - "for-each-ref", - "--format=%(refname)", - "refs/heads", - "refs/remotes", - ], - "cannot list refs; this is not a git repository", - )?; - let mut found: Vec = listing - .lines() - .map(str::trim) - .filter(|line| !line.is_empty()) - .map(ToOwned::to_owned) - .collect(); + let repo = open(dir)?; + let refusal = || UsageError::raise("cannot list refs; this is not a git repository".to_owned()); + let references = repo.references().map_err(|_| refusal())?; + // REF EXISTENCE, never reachability: these consumers land by rebase and + // fast-forward, so a landed branch's commits are ancestors of nothing and a + // reachability test would collect live work. + let mut found: Vec = Vec::new(); + for prefix in ["refs/heads", "refs/remotes"] { + for reference in references + .prefixed(prefix) + .map_err(|_| refusal())? + .flatten() + { + found.push(reference.name().as_bstr().to_string()); + } + } found.sort(); found.dedup(); Ok(found) @@ -1682,7 +1849,26 @@ pub fn refs(dir: &Path) -> Result> { /// /// Internal only — no upstream is `None`, not a failure. pub fn upstream_of_head(dir: &Path) -> Result> { - query_optional(dir, &["rev-parse", "--symbolic-full-name", "@{upstream}"]) + let Ok(repo) = open(dir) else { + return Ok(None); + }; + // The `--end-of-options` trap this function's doc records is GONE with the + // argv: in ref-printing mode `rev-parse` echoed the token as an output line + // rather than consuming it, so carrying it here returned the flag itself as + // the upstream. A resolver takes no flags, so there is nothing to echo. + let Some(name) = repo + .head() + .ok() + .and_then(|head| head.referent_name().map(std::borrow::ToOwned::to_owned)) + else { + // A detached HEAD tracks nothing, and neither does a branch with no + // upstream — both are `None` rather than a failure. + return Ok(None); + }; + Ok(repo + .branch_remote_tracking_ref_name(name.as_ref(), gix::remote::Direction::Fetch) + .and_then(std::result::Result::ok) + .map(|tracking| tracking.as_bstr().to_string())) } /// Count occurrences of `pattern` across files matching `glob` at `rev` @@ -1903,36 +2089,43 @@ pub fn remote_default_branch(dir: &Path) -> Result> { }; name.clone() }; - // `--quiet` so a missing HEAD is a non-zero exit rather than a message on - // stderr; `query_optional` reads that exit as the answer. - Ok(query_optional( - dir, - &[ - "symbolic-ref", - "--quiet", - &format!("refs/remotes/{remote}/HEAD"), - ], - )? + // A missing HEAD is an absent ref rather than a non-zero exit now, which is + // the same answer arriving as a value instead of as an exit status. + let Ok(repo) = open(dir) else { + return Ok(None); + }; + let Ok(reference) = repo.find_reference(&format!("refs/remotes/{remote}/HEAD")) else { + return Ok(None); + }; + // The SYMBOLIC target, which is what `symbolic-ref` printed: a remote HEAD + // records which branch is the trunk, and peeling it to an object would answer + // a different question. + Ok(match reference.target() { + gix::refs::TargetRef::Symbolic(name) => Some(name.as_bstr().to_string()), + gix::refs::TargetRef::Object(_) => None, + } .filter(|found| !found.is_empty())) } /// Resolve `rev` to the full SHA of a commit. /// -/// `--verify` yields exactly one line or a failure; the `^{commit}` peel -/// refuses a tag, tree, or blob rather than going on to diff something -/// meaningless; `--end-of-options` stops a rev that happens to look like a flag -/// from being read as one. +/// Peeled to a commit deliberately: a tag, tree or blob is refused here rather +/// than going on to diff something meaningless. Reads through +/// [`open`]'s isolated handle, so an ambient `GIT_DIR` cannot redirect it. fn resolve_commit(dir: &Path, rev: &str, role: &str) -> Result { - query( - dir, - &[ - "rev-parse", - "--verify", - "--end-of-options", - &format!("{rev}^{{commit}}"), - ], - &format!("{role} {rev:?} does not resolve to a commit in this repository"), - ) + let repo = open(dir)?; + let refused = || { + UsageError::raise(format!( + "{role} {rev:?} does not resolve to a commit in this repository" + )) + }; + let object = repo.rev_parse_single(rev).map_err(|_| refused())?; + let commit = object + .object() + .map_err(|_| refused())? + .peel_to_commit() + .map_err(|_| refused())?; + Ok(commit.id().to_string()) } /// Enumerate commits, newest first, under the fixed selection this module uses @@ -1941,122 +2134,255 @@ fn resolve_commit(dir: &Path, rev: &str, role: &str) -> Result { /// /// Merges are excluded deliberately, not incidentally: a merge has no patch of /// its own, and the commits it brings in are separately enumerated here. That -/// stays true only while this stays a full walk — adding `--first-parent` would -/// make everything merged in invisible, which is a silent false *not landed*. +/// stays true only while this stays a full walk — a first-parent walk would make +/// everything merged in invisible, which is a silent false *not landed*. +/// +/// `range` is either a single commit (everything reachable from it) or +/// `a..b` (reachable from `b`, not from `a`). **That is range SELECTION, not a +/// reachability answer** — the distinction `no_ancestry_decides_merged_ness` +/// draws, and the reason this is allowed to walk parents at all while nothing +/// here may ask whether one commit contains another. fn rev_list(dir: &Path, window: Window, range: &str) -> Result> { - let max = format!("--max-count={}", window.commits()); - let out = query( - dir, - &[ - "rev-list", - "--topo-order", - "--no-merges", - &max, - "--end-of-options", - range, - ], - &format!("cannot enumerate commits for {range:?}"), - )?; - Ok(out.lines().map(ToOwned::to_owned).collect()) + let repo = open(dir)?; + let refused = || UsageError::raise(format!("cannot enumerate commits for {range:?}")); + let (exclude, include) = match range.split_once("..") { + Some((from, to)) => (Some(from), to), + None => (None, range), + }; + let tip = repo + .rev_parse_single(include) + .map_err(|_| refused())? + .detach(); + let mut hidden = Vec::new(); + if let Some(from) = exclude { + hidden.push(repo.rev_parse_single(from).map_err(|_| refused())?.detach()); + } + let mut walk = repo + .rev_walk([tip]) + .sorting(gix::revision::walk::Sorting::BreadthFirst); + if !hidden.is_empty() { + walk = walk.with_hidden(hidden); + } + let mut out = Vec::new(); + for step in walk.all().map_err(|_| refused())? { + let info = step.map_err(|_| refused())?; + // A merge has no patch of its own; its contents are enumerated through + // the commits it brings in. + if info.parent_ids().count() > 1 { + continue; + } + out.push(info.id().to_string()); + if out.len() >= window.commits() { + break; + } + } + Ok(out) } -/// Run a diff-producing `git` command and pipe it through -/// `git patch-id --stable`, returning the `(identity, commit)` pairs in the -/// order git emitted them. -/// -/// One pipeline, two processes, whatever the window: `git log -p` labels each -/// patch with its `commit ` line, which is exactly what makes `patch-id` -/// print the commit alongside the identity. The alternative — hashing each -/// commit in its own `git` invocation — is a process per commit for the same -/// answer. +/// The changes one commit makes against its first parent, in the canonical form +/// [`crate::patch`] hashes. /// -/// `--stable` is not the default: `git patch-id` computes an *unstable* id -/// unless asked, and an unstable id depends on the order files appear in the -/// diff. -fn patch_ids(dir: &Path, args: &[&str], refusal: &str) -> Result> { - let mut producer = command(dir); - producer - .args(DIFF_CONFIG) - .args(args) - .stdout(Stdio::piped()) - .stderr(Stdio::null()); - for var in DIFF_ENV { - producer.env_remove(var); - } - let mut producer = producer - .spawn() - .with_context(|| format!("run `git {}`", args.join(" ")))?; - let Some(patches) = producer.stdout.take() else { - bail!("`git {}` produced no stdout pipe", args.join(" ")); +/// A root commit is diffed against the empty tree, which is what makes its whole +/// content its change rather than leaving it identity-less. +fn commit_changes(repo: &gix::Repository, id: &gix::ObjectId) -> Result> { + let commit = repo.find_object(*id)?.peel_to_commit()?; + let new_tree = commit.tree()?; + let old_tree = match commit.parent_ids().next() { + Some(parent) => repo + .find_object(parent.detach())? + .peel_to_commit()? + .tree()?, + None => repo.empty_tree(), }; - // Nothing is written to a child's stdin here, so there is no pipe-buffer - // deadlock to guard against: git writes patches straight into `patch-id` - // and only the (small) identity list comes back to this process. - let output = command(dir) - .args(["patch-id", "--stable"]) - .stdin(Stdio::from(patches)) - .stderr(Stdio::null()) - .output() - .context("run `git patch-id --stable`")?; - let diffed = producer.wait().context("wait for the diff to finish")?; - if !diffed.success() { - return Err(UsageError::raise(refusal)); - } - if !output.status.success() { - bail!("`git patch-id --stable` failed"); - } - let stdout = - String::from_utf8(output.stdout).context("decode `git patch-id` output as UTF-8")?; - let mut ids = Vec::new(); - for line in stdout.lines() { - let mut fields = line.split_whitespace(); - let (Some(id), Some(commit)) = (fields.next(), fields.next()) else { - bail!("`git patch-id` printed an unparseable line"); + tree_changes(repo, &old_tree, &new_tree) +} + +/// The changes between two trees, with no rename detection. +/// +/// Rename detection is refused rather than merely unconfigured — see +/// [`crate::patch`]'s module doc: a similarity heuristic inside an identity lets +/// two runs disagree about what counts as the same change. +fn tree_changes( + repo: &gix::Repository, + old_tree: &gix::Tree<'_>, + new_tree: &gix::Tree<'_>, +) -> Result> { + use gix_diff::tree::recorder::Change as Recorded; + + let hash = repo.object_hash(); + let mut recorder = gix_diff::tree::Recorder::default(); + gix_diff::tree( + gix::objs::TreeRefIter::from_bytes(&old_tree.data, hash), + gix::objs::TreeRefIter::from_bytes(&new_tree.data, hash), + gix_diff::tree::State::default(), + &repo.objects, + &mut recorder, + )?; + + let mut out = Vec::new(); + for change in recorder.records { + // DIRECTORY ENTRIES ARE NOT CHANGES, and skipping them is load-bearing + // rather than tidy. `gix_diff::tree` records a changed subtree as well as + // the blobs inside it, and a tree object's id encodes ALL of its + // siblings — so `src/` has one id on a branch that added `src/b.rs` and a + // different one on a `main` that also gained `src/other.rs`. Hashing that + // id makes the identity depend on the base the change sits on, which is + // the single property patch identity exists NOT to have: the same change + // replayed elsewhere stops being recognisable, and `completion.unlanded` + // raises against work that is already on the trunk. + // + // The recursion still delivers every blob underneath with its full path, + // so nothing is lost by dropping the tree row — only the base-dependence + // is. CLOUD-739's own §7 corpus missed this because every fixture path in + // it sat at the repository ROOT, where the only tree in the diff is the + // one being diffed and is never emitted as a change. + let mode = match &change { + Recorded::Addition { entry_mode, .. } | Recorded::Deletion { entry_mode, .. } => { + *entry_mode + } + Recorded::Modification { entry_mode, .. } => *entry_mode, }; - ids.push((PatchId::parse(id)?, commit.to_owned())); + if mode.is_tree() { + continue; + } + let (path, kind) = match change { + // `relation` is submodule/rewrite bookkeeping and is deliberately + // ignored: this identity does no rename tracking, so a rewrite pair + // is a deletion and an addition, which is what the target either has + // or does not. + Recorded::Addition { + entry_mode, + oid, + path, + relation: _, + } => ( + path, + crate::patch::Kind::Added { + blob: blob_side(repo, &oid, entry_mode), + }, + ), + Recorded::Deletion { + entry_mode, + oid, + path, + relation: _, + } => ( + path, + crate::patch::Kind::Removed { + blob: blob_side(repo, &oid, entry_mode), + }, + ), + Recorded::Modification { + previous_entry_mode, + previous_oid, + entry_mode, + oid, + path, + } => ( + path, + crate::patch::Kind::Modified { + before: blob_side(repo, &previous_oid, previous_entry_mode), + after: blob_side(repo, &oid, entry_mode), + }, + ), + }; + out.push(crate::patch::Change { + path: path.to_string().into_bytes(), + kind, + }); + } + Ok(out) +} + +/// One side of a change, with its content read only when it is a text blob. +/// +/// A tree entry (a submodule, or a directory the recorder surfaced) carries no +/// content to diff and is identified by its id alone, which is exact. +fn blob_side( + repo: &gix::Repository, + oid: &gix::ObjectId, + mode: gix::objs::tree::EntryMode, +) -> crate::patch::Blob { + // A read that fails leaves `text` absent, which falls back to identifying + // the side by its object id — exact, and never a silent empty content that + // would let two unreadable blobs compare equal. + let text = if mode.is_blob() { + repo.find_object(*oid) + .ok() + .map(|object| object.data.clone()) + .filter(|bytes| crate::patch::is_text(bytes)) + } else { + None + }; + crate::patch::Blob { + oid: oid.to_string(), + mode: u32::from(mode.value()), + text, } - Ok(ids) } /// The patch identity of every commit reachable by `range`, keyed for lookup. /// /// When two commits share an identity — a revert and a re-apply, a change /// cherry-picked twice — the **oldest** wins, so the evidence names the actual -/// landing rather than a later copy of it. `git log` walks newest-first, so +/// landing rather than a later copy of it. The walk is newest-first, so /// overwriting on each insert leaves the oldest in place. +/// +/// No process, and therefore no host configuration: the twenty `-c` keys and six +/// flags this used to pin existed solely to stop the user's `git config` shaping +/// the diff, and [`open`]'s isolated handle declines that configuration +/// outright. fn patch_id_index(dir: &Path, window: Window, range: &str) -> Result> { - let max = format!("--max-count={}", window.commits()); - let mut args = vec!["log", "-p", "--topo-order", "--no-merges", "--root", &max]; - args.extend(DIFF_FLAGS); - args.extend(["--end-of-options", range]); + let repo = open(dir)?; let mut index = BTreeMap::new(); - for (id, commit) in patch_ids(dir, &args, &format!("cannot read commits for {range:?}"))? { - index.insert(id, commit); + for commit in rev_list(dir, window, range)? { + let id = gix::ObjectId::from_hex(commit.as_bytes()) + .map_err(|_| UsageError::raise(format!("cannot read commits for {range:?}")))?; + let mut changes = commit_changes(&repo, &id)?; + if let Some(hex) = crate::patch::identity(&mut changes) { + index.insert(PatchId::parse(&hex)?, commit); + } } Ok(index) } -/// The patch identity of the branch's whole change: `git diff target...head`, -/// which diffs the head against the point the two histories diverged. +/// The patch identity of the branch's whole change: the diff from where the two +/// histories diverged to `head`. /// -/// Three dots, never two: a two-dot diff also carries the *inverse* of -/// everything that landed on the target since the branch left it, so it could -/// never equal a squashed commit no matter how faithfully the work landed. +/// The merge base is used for RANGE SELECTION and never as a merged-ness answer, +/// which is the line `no_ancestry_decides_merged_ness` draws. Diffing `head` +/// against `target` directly would also carry the *inverse* of everything that +/// landed on the target since the branch left it, so it could never equal a +/// squashed commit no matter how faithfully the work landed. /// -/// `None` when the diff is empty — `git patch-id` prints nothing for an empty -/// patch, and an absent identity must never compare equal to another absent -/// identity. +/// `None` when the diff is empty — an absent identity must never compare equal +/// to another absent identity. fn cumulative_patch_id(dir: &Path, target: &str, head: &str) -> Result> { - let range = format!("{target}...{head}"); - let mut args = vec!["diff"]; - args.extend(DIFF_FLAGS); - args.extend(["--end-of-options", &range]); - let ids = patch_ids( - dir, - &args, - "the target and the head have no common history, so there is no branch content to compare", - )?; - Ok(ids.into_iter().next().map(|(id, _)| id)) + let repo = open(dir)?; + let refused = || UsageError::raise(format!("cannot diff {target:?} against {head:?}")); + let target_id = gix::ObjectId::from_hex(target.as_bytes()).map_err(|_| refused())?; + let head_id = gix::ObjectId::from_hex(head.as_bytes()).map_err(|_| refused())?; + let base = repo + .merge_base(target_id, head_id) + .map_err(|_| UsageError::raise(format!("{target:?} and {head:?} share no history")))?; + let old_tree = repo + .find_object(base.detach()) + .map_err(|_| refused())? + .peel_to_commit() + .map_err(|_| refused())? + .tree() + .map_err(|_| refused())?; + let new_tree = repo + .find_object(head_id) + .map_err(|_| refused())? + .peel_to_commit() + .map_err(|_| refused())? + .tree() + .map_err(|_| refused())?; + let mut changes = tree_changes(&repo, &old_tree, &new_tree)?; + crate::patch::identity(&mut changes) + .map(|hex| PatchId::parse(&hex)) + .transpose() } /// Decide whether the work on `head` has landed on `target`, by the identity of @@ -2294,22 +2620,26 @@ pub struct GitFacts { /// gate may legitimately decide about. pub fn head_fact(dir: &Path) -> Result { // REPO-NESS FIRST, because the reads below cannot tell it apart from an - // answer (CLOUD-480, found on review of #660). `query_optional` maps every - // non-zero git exit to `None`, so outside a repository both reads answered - // `None` and this returned `Ok(HeadFact { commit: None, branch: None, - // detached: false })` — a FABRICATED `detached: false` that `git_facts` - // projects as a real fact, for a policy to read as "on a branch". The doc - // above already promised this raises; it did not, and only this call makes - // the promise true. An unborn HEAD stays an answer, which is the distinction - // worth keeping. + // answer (CLOUD-480, found on review of #660). This mattered more under the + // shell-out, where every non-zero git exit became `None` and an out-of-repo + // call returned a FABRICATED `detached: false` for `git_facts` to project as + // a real fact. In process the reads below cannot even be attempted without a + // repository, but the call stays: the doc promises this raises, and the + // promise should not rest on a later line happening to fail. repo_root(dir)?; - let commit = query_optional(dir, &["rev-parse", "--verify", "HEAD"])?.filter(|c| !c.is_empty()); - let named = query_optional(dir, &["rev-parse", "--abbrev-ref", "HEAD"])?; - // git spells a detached HEAD as the literal `HEAD`. In a repository with no - // commits `--abbrev-ref` still answers with the unborn branch's name, which - // is why `detached` is read off this rather than off `commit`. - let detached = named.as_deref() == Some("HEAD"); - let branch = named.filter(|name| name != "HEAD" && !name.is_empty()); + let repo = open(dir)?; + let head = repo + .head() + .map_err(|_| UsageError::raise(format!("{} is not a git repository", dir.display())))?; + // An unborn HEAD is an ANSWER (`commit: None`), which is the distinction + // worth keeping: an empty checkout is a state a gate may decide about. + let commit = head.id().map(|id| id.detach().to_hex().to_string()); + // A detached HEAD has no referent name at all. Under `--abbrev-ref` it was + // the literal string `HEAD`, and a repository with no commits still answered + // with the unborn branch's name — which is why `detached` was read off that + // rather than off `commit`, and why it is read off the referent here. + let branch = head.referent_name().map(|name| name.shorten().to_string()); + let detached = branch.is_none(); Ok(HeadFact { commit, branch, @@ -2494,6 +2824,34 @@ mod tests { use super::*; + /// THE WINDOWS REGRESSION, TESTED AS A DECISION RATHER THAN A CONDITION. + /// + /// The failing condition — `canonicalize` returning a verbatim path — is one + /// this sandbox structurally cannot produce, so asserting over a real + /// `repo_root` here would assert its own premise and pass for the wrong + /// reason (`.claude/rules/rust.md`, CLOUD-249). The decision is `plain`, and + /// it takes a literal, so it is testable on every platform. + /// + /// Fails by: making `plain` the identity, which is what the tree carried when + /// CI went red on Windows and green on the other three. + #[test] + fn a_canonicalised_root_is_comparable_with_an_absolute_path() { + assert_eq!( + plain(PathBuf::from(r"\\?\D:\a\batten\batten")), + PathBuf::from(r"D:\a\batten\batten"), + "a verbatim root never shares a prefix with `std::path::absolute`, \ + so every path reads as outside the repository" + ); + // A UNC verbatim path keeps its prefix: the plain spelling resolves + // differently, so rewriting it would trade a comparison bug for a worse + // one. + let unc = PathBuf::from(r"\\?\UNC\server\share\repo"); + assert_eq!(plain(unc.clone()), unc); + // And the identity everywhere else, including the platform this runs on. + let ordinary = PathBuf::from("/home/user/batten"); + assert_eq!(plain(ordinary.clone()), ordinary); + } + /// A fresh scratch directory under the system temp dir. Unit tests cannot /// use `CARGO_TARGET_TMPDIR` (integration-only), and the wipe clears a /// crashed prior run. @@ -2554,7 +2912,7 @@ mod tests { reason = "stays, and test-only: fixtures are built by the reference implementation on purpose — building them with gix would test this module's backend against itself" )] fn git(dir: &Path, args: &[&str]) { - let mut command = Command::new("git"); + let mut command = std::process::Command::new("git"); command .arg("-C") .arg(dir) @@ -2562,7 +2920,17 @@ mod tests { .args(args) .env("GIT_CONFIG_GLOBAL", "/dev/null") .env("GIT_CONFIG_SYSTEM", "/dev/null"); - for var in DISCOVERY_REDIRECTS.iter().chain(DISCOVERY_FENCES.iter()) { + // The discovery scrub, inlined now that the module carries no constants + // for it: `open`'s isolated handle declines the environment structurally, + // so the only place a NAMED list is still needed is here, where a real + // `git` process is deliberately being built. + for var in [ + "GIT_DIR", + "GIT_COMMON_DIR", + "GIT_WORK_TREE", + "GIT_CEILING_DIRECTORIES", + "GIT_DISCOVERY_ACROSS_FILESYSTEM", + ] { command.env_remove(var); } let output = command.output().expect("run git"); @@ -2584,6 +2952,36 @@ mod tests { ); } + #[test] + fn a_repo_local_config_write_replaces_an_existing_value() { + // The one write primitive in this module, and it had no round-trip case + // before CLOUD-740 moved it in process. `attribution identity` is its + // caller, so a write that silently fails to REPLACE leaves a denied + // committer in place while reporting success. + let repo = scratch("config-write"); + git(&repo, &["init", "-q"]); + git(&repo, &["config", "--local", "user.name", "Vendorbot"]); + + set_config_local(&repo, "user.name", "Accountable Human").expect("write the local value"); + assert_eq!( + config_value(&repo, "user.name") + .expect("read it back") + .as_deref(), + Some("Accountable Human"), + "an existing value must be REPLACED, not shadowed or appended" + ); + + // And a subsectioned key, which is the other shape callers use. + set_config_local(&repo, "remote.origin.url", "https://example.test/x") + .expect("write a subsectioned value"); + assert_eq!( + config_value(&repo, "remote.origin.url") + .expect("read it back") + .as_deref(), + Some("https://example.test/x") + ); + } + #[test] fn resolves_the_root_from_a_nested_subdirectory() { let repo = scratch("nested"); @@ -3043,20 +3441,42 @@ mod tests { #[test] fn no_second_git_invoker_exists() { - // The gate that makes the receipt.rs migration stick (CLOUD-36): every - // `git` process the crate spawns is spawned through this module, so - // there is one place where the discovery scrub, the pinned diff config, - // and the usage-vs-internal error split are decided. + // THE TERMINAL ASSERTION (CLOUD-740). This forbade a git spawn OUTSIDE + // this module, so that the discovery scrub, the pinned diff config and + // the usage-vs-internal split were decided in one place. There is now no + // such place to protect: nothing in the crate spawns `git` at all, and + // the claim is strictly stronger and much simpler for it. + // + // `crate_sources(false)` is the change that makes it terminal — the + // argument selects whether THIS module is exempt, and the whole point is + // that it no longer is. It was `true` while `git.rs` held the one + // invoker. + // + // SHOWN ABLE TO FAIL (CLOUD-418) by reintroducing a spawn anywhere under + // `src/`, including here. // // Precise by construction: rules.rs and hook.rs spawn *user-configured* // programs through a variable program name and are untouched by this — - // what is forbidden is naming `git` as a literal program elsewhere. + // what is forbidden is naming `git` as a literal program. + // + // SCANNED UP TO `#[cfg(test)]` AND NO FURTHER, which is a real limit and + // not a convenience. The test module below builds its fixtures with a + // real `git` on purpose — building them with gix would test this module's + // backend against itself, so the reference implementation is the only + // honest fixture builder — and that helper carries its own `#[expect]` + // saying so. The claim being made is about what the SHIPPED crate does, + // and truncating here states that scope instead of quietly assembling the + // needle to dodge a match, which would make the gate lie about its reach. let needle = ["Command::new(\"", "git\")"].concat(); - for (path, source) in crate_sources(true) { + for (path, source) in crate_sources(false) { + let source = source + .split_once("\n#[cfg(test)]\n") + .map_or(source.as_str(), |(shipped, _)| shipped); assert!( !source.contains(needle.as_str()), - "{}: spawns git directly; call git::query so the environment scrub and the \ - usage-vs-internal split stay in one place (CLOUD-36)", + "{}: spawns `git`. Nothing in this crate does any more (CLOUD-740) — ask gix \ + through `open`, whose isolated handle declines the ambient environment \ + structurally rather than by scrubbing a list of variable names", path.display() ); } @@ -3091,6 +3511,23 @@ mod tests { .take_while(|line| line.starts_with("//!")) .collect::>() .join("\n"); + // CONDITIONAL ON THERE BEING A SPAWN TO PRICE (CLOUD-740, resolution 3), + // in the same commit as the terminal assertion above because the two are + // one decision. This gate and that one contradicted each other outright: + // this demanded the doc keep naming a spawn that stays, and that one + // requires none to remain. Deleting this gate was the cheap answer and a + // lossy one — it exists because a session read the module doc, concluded + // the split was permanent, and wrote that into an issue and a milestone, + // and a false constraint reads exactly like a true one. + // + // So the SUBJECT narrows rather than the predicate: *if* the doc claims a + // spawn stays, it must name `git2` and the rows that own the price. + // Vacuously true now, live again the day anything here spawns — which is + // the resolution that survives the migration instead of being spent by + // it. + if !doc.contains("Still spawning") { + return; + } for owner in ["CLOUD-737", "CLOUD-585"] { assert!( doc.contains(owner), @@ -3150,72 +3587,35 @@ mod tests { } } - #[test] - fn a_short_record_is_refused_rather_than_answered() { - // The one behavioural change CLOUD-742 sanctions, tested where the - // decision is: a record that does not carry four fields cannot be read - // as attribution. It used to take each part with `unwrap_or_default()`, - // so a body carrying U+001E shifted the split and the missing fields - // arrived as empty strings — judged afterwards as though git had said - // them. - // - // Exercised over the parse rather than over a repository, per - // `.claude/rules/rust.md`: the failing condition is a record shape, so - // the assertion is about that shape and not about a fixture that - // happens to produce it. - let sep = RECORD_SEPARATOR; - let commit = "a".repeat(40); - - let whole = format!("Ann {sep}Bo {sep}Refs: CLOUD-742{sep}the body"); - let read = record_from(&whole, &commit).expect("a four-field record reads"); - assert_eq!(read.author, "Ann "); - assert_eq!(read.committer, "Bo "); - assert_eq!(read.trailers, vec!["Refs: CLOUD-742".to_owned()]); - assert_eq!(read.body, "the body"); - - // A body carrying the separator does NOT shift fields, because the - // split is bounded at four: everything after the third separator is - // body, separators and all. - let with_sep = format!("Ann {sep}Bo {sep}{sep}a body with {sep} in it"); - let read = record_from(&with_sep, &commit).expect("the body keeps its own separators"); - assert_eq!(read.author, "Ann "); - assert_eq!(read.body, format!("a body with {sep} in it")); - - // Short: three fields, which used to yield an empty body and an empty - // trailer block that the attribution decision then judged. - for short_record in [ - format!("Ann {sep}Bo {sep}Refs: CLOUD-742"), - format!("Ann {sep}Bo "), - "Ann ".to_owned(), - String::new(), - ] { - let refused = record_from(&short_record, &commit); - assert!( - refused.is_err(), - "a record of {} field(s) must refuse, never answer with blanks", - short_record.split(sep).count() - ); - } - } - #[test] fn a_patch_id_is_hex_of_a_hash_length() { - assert!(PatchId::parse(&"a".repeat(40)).is_ok(), "SHA-1 repository"); + // CLOUD-739 §7(c). The refusal is the point and is unchanged: a parsing + // slip must never manufacture an equality between two truncated or + // non-hex ids. What changed is the WIDTH it accepts. + assert!(PatchId::parse(&"0".repeat(64)).is_ok()); + + // FORTY IS NOW REFUSED, and that is the migration rather than a + // regression. The old rule accepted 40 or 64 because it was parsing + // whatever `git patch-id` printed, and that followed the REPOSITORY's + // hash — SHA-1 or SHA-256. The identity is now a SHA-256 over this + // crate's own canonical form (CLOUD-739), so the width is fixed by + // construction and a 40-hex value arriving here is a defect, not a + // SHA-1 repository. assert!( - PatchId::parse(&"0".repeat(64)).is_ok(), - "SHA-256 repository" + PatchId::parse(&"a".repeat(40)).is_err(), + "the width follows our own hash now, never the repository's" ); - // A parsing slip must never manufacture an equality between two - // truncated or non-hex ids. + assert!(PatchId::parse("").is_err()); - assert!(PatchId::parse(&"a".repeat(39)).is_err()); - assert!(PatchId::parse(&"g".repeat(40)).is_err()); - assert!(PatchId::parse(&"A".repeat(40)).is_err(), "lowercase only"); + assert!(PatchId::parse(&"a".repeat(63)).is_err(), "truncated"); + assert!(PatchId::parse(&"a".repeat(65)).is_err(), "over-long"); + assert!(PatchId::parse(&"g".repeat(64)).is_err(), "non-hex"); + assert!(PatchId::parse(&"A".repeat(64)).is_err(), "lowercase only"); } #[test] fn the_verdict_is_derived_from_evidence_alone() { - let id = PatchId::parse(&"a".repeat(40)).unwrap(); + let id = PatchId::parse(&"a".repeat(64)).unwrap(); let landed = |evidence: Option| CommitLanding { commit: "c".repeat(40), patch_id: Some(id.clone()), @@ -3277,7 +3677,7 @@ mod tests { fn evidence_that_means_landed_always_names_a_target_commit() { // The structural half of "no verdict without proof": the only variant // that names nothing is the one that means nothing landed. - let id = PatchId::parse(&"a".repeat(40)).unwrap(); + let id = PatchId::parse(&"a".repeat(64)).unwrap(); let target = "t".repeat(40); for evidence in [ Evidence::PatchId { diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index 5ce5c6aaa..5b4644b11 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -209,14 +209,16 @@ impl Harness { /// Whether a deny on this host must carry its reason **in the JSON body** /// rather than on stderr. /// - /// Cursor is the one surveyed host that assigns no meaning to stderr, so - /// CLOUD-122's refusal contract ("every deny points to the fix") is - /// unsatisfiable there through the exit-code channel alone. Claude Code - /// answers in-band for a different reason — exit 2 discards its stdout JSON, - /// so the two channels are exclusive and it picks the richer one. + /// The property is [`Capabilities::reason_travels_in_band`]'s and this reads + /// it (CLOUD-372). It was a `matches!` over two harness names until then — + /// a host property declared outside the table CLOUD-45 made the authority, + /// so a seventh harness was correct only if whoever added it remembered the + /// second place, and a forgotten `matches!` arm stays compiling and answers + /// `false`. The accessor survives the move because callers ask a harness, + /// not a table; what changed is where the answer comes from. #[must_use] pub const fn reason_travels_in_band(self) -> bool { - matches!(self, Harness::ClaudeCode | Harness::Cursor) + self.capabilities().reason_travels_in_band } /// The tools on this host whose call **writes the path it names**. @@ -390,6 +392,23 @@ pub struct Capabilities { /// Allow and is treated as a `systemMessage`. Batten must keep stdout clean /// or exit 2 there. pub stdout_must_stay_clean: bool, + /// Whether a deny on this host must carry its reason **in the JSON body** + /// rather than on stderr (CLOUD-372). + /// + /// Cursor is the one surveyed host that assigns no meaning to stderr, so + /// CLOUD-122's refusal contract ("every deny points to the fix") is + /// unsatisfiable there through the exit-code channel alone. Claude Code + /// answers in-band for a different reason — exit 2 discards its stdout JSON, + /// so the two channels are exclusive and it picks the richer one. + /// + /// **A row here rather than a `matches!` over harness names**, which is the + /// whole of CLOUD-372. CLOUD-45 made this table the one authority on what a + /// host can and cannot do, and a host property declared outside it is + /// correct only while whoever adds the seventh harness remembers the second + /// place. A missing arm in a `matches!` stays compiling and answers `false`; + /// a missing field here does not compile, which is the asymmetry that made + /// the split cost something. + pub reason_travels_in_band: bool, /// What this host does to commit metadata, and what it exposes about its /// caller (CLOUD-276). /// @@ -1156,6 +1175,17 @@ impl Harness { // named. Collapsing them would delete those reasons and make a future // divergence a structural edit rather than a one-value one. #[allow(clippy::match_same_arms)] + // `too_many_lines` is the same misfire one axis over, and CLOUD-372 is what + // crossed the threshold: this function is a DATA TABLE, so its length is the + // host count times the column count and both of those are the point. The + // remedy the lint implies — split it — would put one host's row away from + // the others, which is the two-places defect CLOUD-372 exists to remove. + // `expect` rather than `allow`, so if the table ever shrinks back under the + // ceiling this annotation goes red instead of outliving its reason. + #[expect( + clippy::too_many_lines, + reason = "a per-host capability table grows by rows; splitting it would re-create the split this row closed" + )] pub const fn capabilities(self) -> Capabilities { match self { Harness::ClaudeCode => Capabilities { @@ -1191,6 +1221,9 @@ impl Harness { timeout_fails_open: false, needs_fail_closed_config: false, stdout_must_stay_clean: false, + // exit 2 discards this host's stdout JSON, so the two channels are + // exclusive and the richer one wins. + reason_travels_in_band: true, // The one host whose attribution rows are not the shared // "unsurveyed" group, because this repository measured its own // history under it (2026-08-09, recorded in @@ -1255,6 +1288,9 @@ impl Harness { timeout_fails_open: false, needs_fail_closed_config: true, stdout_must_stay_clean: false, + // the one surveyed host that assigns no meaning to stderr at all, so a + // deny explained there would explain itself to nobody. + reason_travels_in_band: true, attribution: UNSURVEYED_ATTRIBUTION, capture: UNSURVEYED_CAPTURE, }, @@ -1278,6 +1314,8 @@ impl Harness { timeout_fails_open: true, needs_fail_closed_config: false, stdout_must_stay_clean: false, + // stderr carries the reason. + reason_travels_in_band: false, attribution: UNSURVEYED_ATTRIBUTION, capture: UNSURVEYED_CAPTURE, }, @@ -1303,6 +1341,8 @@ impl Harness { timeout_fails_open: false, needs_fail_closed_config: false, stdout_must_stay_clean: true, + // stderr carries the reason. + reason_travels_in_band: false, attribution: UNSURVEYED_ATTRIBUTION, capture: UNSURVEYED_CAPTURE, }, @@ -1319,6 +1359,8 @@ impl Harness { timeout_fails_open: false, needs_fail_closed_config: false, stdout_must_stay_clean: false, + // stderr carries the reason. + reason_travels_in_band: false, attribution: UNSURVEYED_ATTRIBUTION, capture: UNSURVEYED_CAPTURE, }, @@ -1336,6 +1378,8 @@ impl Harness { timeout_fails_open: false, needs_fail_closed_config: false, stdout_must_stay_clean: false, + // the caller reads the exit code and stderr; there is no document. + reason_travels_in_band: false, // The one column that is `No` rather than `Unknown`, and it is a // measurement rather than a guess: this is not a third party. It // is the normalized envelope Batten itself defines, and that @@ -4790,6 +4834,15 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result None, + // `Cost::Effect` (CLOUD-760), and the ONLY fact whose absence here is + // a refusal rather than a classification. Resolving it runs an + // analyser over the whole crate; a mediated call has a per-call + // budget measured in milliseconds, and `run_static` already refuses a + // spawning kind on this surface. `facts.rs` classifies it + // `Surface::Check` so this arm is `None` by the model rather than by + // this function's opinion — and `no_effect_fact_is_hook_resolvable` + // is the assertion that keeps the two agreeing. + crate::facts::Fact::Symbols => None, }; if let Some(value) = projected { projected_facts.insert(fact.as_str().to_owned(), value); @@ -6030,6 +6083,7 @@ mod tests { fix: None, produces: None, exclude_paths: Vec::new(), + symbols: false, run: None, verbatim: None, identity_key: None, diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 307322505..a0c1766e9 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -48,6 +48,9 @@ pub mod markers; pub mod mint; pub mod output; pub mod outputs; +/// The in-process patch identity: what a change IS, independent of the commit +/// carrying it and of the host's git configuration. +mod patch; pub mod pattern; pub mod policy; pub mod provision; @@ -63,6 +66,11 @@ pub mod session; pub mod severity; pub mod sink; pub mod spec; +/// Resolved-symbol facts, from a delegated analyser's structured output +/// (CLOUD-760). The first occupant of `Cost::Effect`: resolving it runs a +/// program, which is the classification rather than an accident of it. +pub mod symbols; + pub mod state; pub mod stop; pub mod store; @@ -1662,6 +1670,7 @@ fn suite_input( tracked, &std::collections::BTreeMap::new(), &git::GitFacts::default(), + &facts::Look::IsNot, )) } @@ -4841,6 +4850,20 @@ fn report_self_writes( /// a `warn` finding is reported without failing the run unless the resolved /// `fail_on_warning` setting promotes it (CLOUD-49). Reporting is unaffected by /// that promotion: a warn finding prints either way, and only the verdict moves. +/// The `config deprecations -J` document: what left the surface unannounced. +/// +/// THREE-VALUED, and `baseline` is the field that makes it so. An unreadable +/// baseline and a clean comparison would otherwise both render +/// `removed_without_window: []`, so a consumer could not tell "nothing was +/// removed" from "nothing was compared" — CLOUD-251's vacuous pass, moved out of +/// the exit code and into the document a parser actually reads. +#[derive(Debug, serde::Serialize)] +struct DeprecationReport<'a> { + against: &'a str, + baseline: &'a str, + removed_without_window: &'a [String], +} + /// The `config epoch -J` document: the digest and the surface it covers. #[derive(Debug, serde::Serialize)] struct EpochReport<'a> { @@ -5395,6 +5418,74 @@ fn run_lint_brief(path: Option<&str>, json: bool, out: &mut dyn Write) -> Result Ok(ExitCode::verdict(!report.is_clean())) } +/// `batten config deprecations --against ` (CLOUD-360 §2). +/// +/// Its own function rather than an arm, for the seam `config_of` already uses: +/// `run_config` reached `clippy::too_many_lines` when this landed, and a verb +/// with three distinct exit outcomes is the natural place to split. +/// +/// THREE OUTCOMES, and the third is the one worth reading. `0` when nothing left +/// the surface unannounced, `2` when something did — the policy verdict, same as +/// any other finding — and `3` when the baseline could not be read at all. +/// +/// # Errors +/// +/// Raises (→ exit `3`) when the ref carries no published schema, and a +/// [`UsageError`] (→ exit `1`) when either schema is unreadable. Never `0` for +/// either: reporting "no key was removed" having compared nothing is the vacuous +/// pass CLOUD-251 names, and it is the failure this gate would have if an +/// unreadable baseline were treated as an empty schema. +fn run_config_deprecations(json: bool, against: &str, out: &mut dyn Write) -> Result { + let Ok(published) = git::show(Path::new("."), against, config::SCHEMA_PATH) else { + // THE DOCUMENT IS EMITTED ANYWAY. A data channel that is sometimes absent + // is unparseable by the caller that asked for it, so the could-not-look + // answer is a document too — distinguished by `baseline`, never by an + // empty list that reads as clean. + if json { + let report = DeprecationReport { + against, + baseline: "unavailable", + removed_without_window: &[], + }; + writeln!(out, "{}", serde_json::to_string_pretty(&report)?)?; + } + anyhow::bail!("no published schema at {against}, so no removal could be judged"); + }; + let released = config::schema_keys(&published, against)?; + let derived = config::schema()?; + let current = config::schema_keys(&derived, "the derived schema")?; + let unannounced = config::removals_unannounced( + &released, + ¤t, + config::DEPRECATED_KEYS, + config::RETIRED_KEYS, + ); + if json { + let report = DeprecationReport { + against, + baseline: "read", + removed_without_window: &unannounced, + }; + writeln!(out, "{}", serde_json::to_string_pretty(&report)?)?; + } else { + // Pointer-only: the key and the remedy, never the schema body. + for key in &unannounced { + writeln!( + out, + "{key} removed since {against} with no deprecation window" + )?; + } + // The count is stated even at zero, so silence cannot be mistaken for + // "the gate did not run". + writeln!( + out, + "config-deprecations: {} unannounced removal(s) against {against}", + unannounced.len() + )?; + } + Ok(ExitCode::verdict(!unannounced.is_empty())) +} + fn run_config( command: &ConfigCommand, overrides: &Overrides, @@ -5464,6 +5555,9 @@ fn run_config( } Ok(ExitCode::Success) } + ConfigCommand::Deprecations { json, against } => { + run_config_deprecations(*json, against, out) + } ConfigCommand::Lint { json, host_rules } => { // The date the expiry smell is computed against, read once at this // boundary and threaded in as data (`waiver`'s module docs say why). diff --git a/crates/batten/src/patch.rs b/crates/batten/src/patch.rs new file mode 100644 index 000000000..59b9cd224 --- /dev/null +++ b/crates/batten/src/patch.rs @@ -0,0 +1,280 @@ +//! The in-process patch identity (CLOUD-739). +//! +//! [`crate::git::landing`] decides merged-ness by the identity of a *change*, +//! never by reachability (CLOUD-36) — which is what makes a rebased, squashed or +//! cherry-picked branch recognisable after it lands under a new SHA. This module +//! computes that identity without running a program. +//! +//! # Why this is ours to define +//! +//! A [`crate::git::PatchId`] is only ever compared against one produced by **the +//! same binary in the same run** — the head index against the target index, +//! inside `landing`. Nothing external computes one to compare against, and none +//! is persisted across versions. So the requirement is *a* canonical +//! deterministic identity, not git's (CLOUD-320 ruled this in writing). +//! +//! That licence is what lets the normalisation below be **decided** rather than +//! inherited. The implementation this replaces pinned twenty `git config` keys +//! and six flags for one purpose: stopping the host's configuration from +//! changing the answer. In-process there is no host configuration to read, so +//! all twenty-six are gone and nothing replaces them. +//! +//! # The normalisation, as decisions +//! +//! Each of these was a side effect of which tool got invoked. Each is now a +//! choice with a reason, and each has a case asserting it. +//! +//! **Line numbers are excluded.** Hunk positions are what shift when a change is +//! replayed on a different base, so an identity that included them would fail on +//! exactly the rebase it exists to recognise. This is the one behaviour inherited +//! deliberately and unchanged. +//! +//! **Whitespace is SIGNIFICANT, and this diverges from git.** `git patch-id` +//! folds whitespace away, so a whitespace-only difference collides and two +//! different changes share an identity. The doc this module replaces called that +//! collision *"the safe direction for a primitive whose failure class is a false +//! not landed"*. For this crate's consumers that reasoning is backwards: a false +//! **landed** is what suppresses `completion.unlanded`'s finding, and telling an +//! agent its work is on the trunk when it is not is the failure a completion gate +//! exists to prevent. A spurious not-landed is noise; a spurious landed is a lie. +//! So a whitespace-only change gets its own identity. +//! +//! **Renames are not detected**, matching what the old pinning forced with +//! `diff.renames=false` *and* `--no-renames` — but as a decision now. A rename is +//! a deletion and an addition, which is a change the target either has or does +//! not. Rename detection is a similarity heuristic, and a heuristic inside an +//! identity means two runs can disagree about what is the same change. +//! +//! **Binary content is identified by its blob ids, never by a patch body.** This +//! is what retires the caveat the old flag table admitted to: `--binary` emitted +//! a zlib-compressed body that was *"deterministic for a given zlib but not +//! guaranteed across zlib builds"*, so the identity was forced to choose between +//! being wrong and being unstable, and chose unstable. An object id is neither: +//! it is stable across builds, and two unrelated edits to one path have different +//! ids, so the collision `--binary` existed to prevent cannot occur. + +use sha2::{Digest, Sha256}; + +/// How a single path changed, in the canonical form the identity hashes. +/// +/// Ordered by path before hashing, because a tree walk's emission order is an +/// implementation detail and an identity that depended on it would not be +/// byte-stable. That ordering is what `git patch-id --stable` bought with a flag; +/// here it is the only behaviour available. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) struct Change { + /// The path, as bytes: a repository may carry a path that is not UTF-8, and + /// an identity that could not represent one would be undefined exactly where + /// `core.quotePath` used to make the old implementation host-dependent. + pub(crate) path: Vec, + /// What happened to it. + pub(crate) kind: Kind, +} + +/// The three shapes a change takes once rename detection is refused. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum Kind { + /// The path did not exist and now does. + Added { blob: Blob }, + /// The path existed and no longer does. + Removed { blob: Blob }, + /// The path existed on both sides with different content or mode. + Modified { before: Blob, after: Blob }, +} + +/// One side of a change. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) struct Blob { + /// The object id, hex. Carries the whole content by construction. + pub(crate) oid: String, + /// The file mode, so a chmod is a change rather than a no-op. + pub(crate) mode: u32, + /// The content, when it is text and the hunks are what identify it. + /// + /// `None` for a binary blob, which is identified by [`Blob::oid`] alone — + /// see the module doc on why that is stronger than a zlib patch body rather + /// than weaker. + pub(crate) text: Option>, +} + +/// Whether a blob is treated as text for hunk extraction. +/// +/// A NUL byte is git's own heuristic and is kept deliberately: the identity must +/// classify a blob the same way on every machine, and anything richer — an +/// attributes lookup, a filter, a content-type guess — is host state, which is +/// the whole class of input this migration removed. +#[must_use] +pub(crate) fn is_text(bytes: &[u8]) -> bool { + !bytes.contains(&0) +} + +/// Hash a commit's changes into the hex identity `PatchId` wraps. +/// +/// `None` when the change set is empty: an empty diff has no identity, and two +/// commits that changed nothing must never compare equal to each other. That is +/// the same answer `git patch-id` gave by printing nothing, preserved because +/// `landing` reads it — an absent identity is what produces `Evidence::NoContent`. +/// +/// Infallible: a digest absorbs bytes and cannot refuse them, so there is no +/// error path to invent. The caller's fallibility is in READING the objects, not +/// in hashing them. +pub(crate) fn identity(changes: &mut [Change]) -> Option { + if changes.is_empty() { + return None; + } + changes.sort(); + let mut hasher = Sha256::new(); + // A length-prefixed framing, for `identity::tagged_fingerprint`'s reason: + // without it a path ending where the next field begins could be re-cut into + // a different change set with the same bytes. + field(&mut hasher, b"batten-patch-v1"); + for change in changes.iter() { + field(&mut hasher, &change.path); + match &change.kind { + Kind::Added { blob } => { + field(&mut hasher, b"+"); + side(&mut hasher, blob, None); + } + Kind::Removed { blob } => { + field(&mut hasher, b"-"); + side(&mut hasher, blob, None); + } + Kind::Modified { before, after } => { + field(&mut hasher, b"~"); + side(&mut hasher, before, None); + side(&mut hasher, after, Some(before)); + } + } + } + let digest = hasher.finalize(); + let mut hex = String::with_capacity(digest.len() * 2); + for byte in digest { + // Two nibbles pushed directly, matching `identity::Fingerprint::to_hex` + // rather than reaching for a formatter that can fail. + hex.push(char::from_digit(u32::from(byte >> 4), 16).unwrap_or('0')); + hex.push(char::from_digit(u32::from(byte & 0x0f), 16).unwrap_or('0')); + } + Some(hex) +} + +/// One side of one change, hashed. +/// +/// When `previous` is supplied and both sides are text, what enters the hash is +/// the **edit script** rather than the content: that is what makes the identity +/// survive a rebase, because the hunks are the change and the surrounding file +/// is not. Otherwise the object id stands in, which is exact and cheap. +fn side(hasher: &mut Sha256, blob: &Blob, previous: Option<&Blob>) { + field(hasher, blob.mode.to_le_bytes().as_slice()); + // Hunks when BOTH sides are text and there is a previous side to diff + // against; otherwise the object id, which is exact. See the module doc — + // that fallback is what retires the zlib stability caveat rather than + // restating it. + if let (Some(before), Some(after)) = ( + previous.and_then(|p| p.text.as_deref()), + blob.text.as_deref(), + ) { + field(hasher, b"hunks"); + for line in hunks(before, after) { + field(hasher, &line); + } + } else { + field(hasher, b"oid"); + field(hasher, blob.oid.as_bytes()); + } +} + +/// The edit script between two text blobs, as tagged lines with **no positions**. +/// +/// Line numbers are excluded here and nowhere else, which is what keeps that +/// decision in one place. The tag distinguishes an insertion from a deletion, so +/// a change and its exact revert do not collide. +fn hunks(before: &[u8], after: &[u8]) -> Vec> { + let before = String::from_utf8_lossy(before); + let after = String::from_utf8_lossy(after); + let input = imara_diff::InternedInput::new(before.as_ref(), after.as_ref()); + let diff = imara_diff::Diff::compute(imara_diff::Algorithm::Histogram, &input); + let mut out: Vec> = Vec::new(); + for hunk in diff.hunks() { + for token in hunk.before { + out.push(tagged(b'-', &input, input.before[token as usize])); + } + for token in hunk.after { + out.push(tagged(b'+', &input, input.after[token as usize])); + } + } + out +} + +/// One edit-script line: a tag byte and the token's text, and **no position**. +/// +/// The tag is what keeps a change and its exact revert apart — without it the +/// same set of lines added and removed would hash identically. +fn tagged(tag: u8, input: &imara_diff::InternedInput<&str>, token: imara_diff::Token) -> Vec { + let mut line = vec![tag]; + line.extend_from_slice(input.interner[token].as_bytes()); + line +} + +/// Write one length-prefixed field. +/// +/// `identity::write_field`'s framing, deliberately the same shape: the length as +/// a little-endian `u64` and then the bytes, so no two field sequences can be +/// re-cut into one another. +fn field(hasher: &mut Sha256, bytes: &[u8]) { + hasher.update(u64::try_from(bytes.len()).unwrap_or(u64::MAX).to_le_bytes()); + hasher.update(bytes); +} + +#[cfg(test)] +mod tests { + use super::{Blob, Change, Kind, identity}; + + /// The "renames are not detected" decision, gated where it is observable. + /// + /// It is NOT observable through [`crate::git::landing`], and that is worth + /// writing down rather than discovering twice: rename detection is a pure + /// function of the two trees, so a fixture built out of trees feeds an + /// identical input to the detecting and the non-detecting build and gets an + /// identical answer from both. A test over `landing` asserting this would be + /// a case that cannot go red — which is what CLOUD-418 calls coverage. + /// + /// Where it IS observable is the shape: a rename can only enter the identity + /// as a fourth [`Kind`], and this exhaustive match refuses to compile the day + /// one appears. The named mutation is "add `Kind::Renamed`", and it fails the + /// build rather than the assertion, which is the stronger of the two. + #[test] + fn renames_are_not_a_shape_this_identity_can_take() { + let blob = Blob { + oid: "0".repeat(64), + mode: 0o100_644, + text: Some(b"a\n".to_vec()), + }; + for kind in [ + Kind::Added { blob: blob.clone() }, + Kind::Removed { blob: blob.clone() }, + Kind::Modified { + before: blob.clone(), + after: blob.clone(), + }, + ] { + match &kind { + Kind::Added { .. } | Kind::Removed { .. } | Kind::Modified { .. } => {} + } + assert!( + identity(&mut [Change { + path: b"p".to_vec(), + kind + }]) + .is_some(), + "every shape a change can take must have an identity" + ); + } + } + + /// An empty change set has no identity, which is what produces + /// `Evidence::NoContent` rather than a hash every empty commit shares. + #[test] + fn an_empty_change_set_has_no_identity() { + assert_eq!(identity(&mut []), None); + } +} diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index 2cd0c7fa6..7ad3ba673 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -1941,6 +1941,21 @@ pub struct Rule { /// entries here, because those two carry a parameter and these three do not. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub git: Vec, + /// Whether this policy row reads the **resolved-symbol** fact (CLOUD-760). + /// + /// A bare flag rather than a path list, because the fact is one whole-crate + /// value: a delegated analyser resolves names across the compilation, and + /// asking it about one file would be asking a different, cheaper question + /// that [`Rule::invocations`] already answers. + /// + /// **Declared rather than ambient, and here the reason is the cost class.** + /// This is the first `Cost::Effect` fact — resolving it RUNS `cargo clippy` + /// over the crate, which is seconds rather than the milliseconds every other + /// fact costs. Every git fact is declared for a bill CLOUD-851 measured at + /// 2.103x; this one would be far worse, and a run that paid it without being + /// asked would make `check` unusable. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub symbols: bool, /// The refs this policy row resolves, **declared** (CLOUD-907). /// /// Each becomes an entry of `input.tree["git-refs"]` carrying the commit it @@ -4474,6 +4489,10 @@ fn run( // writes as `null`: outside a checkout there is no answer to give, and a // fabricated one is worse than none. let git = git_facts(rules, root); + // The one acquisition of the `Cost::Effect` fact (CLOUD-760), beside the git + // family and for the same reason: a projection must not spawn, so the spend + // happens once here and only when a row declared it. + let symbols = symbols_fact(rules, root); let inputs = RunInputs { provisions, @@ -4482,6 +4501,7 @@ fn run( documents: &documents, produced: &produced, git: &git, + symbols: &symbols, bundles, }; @@ -4610,6 +4630,8 @@ struct RunInputs<'a> { produced: &'a BTreeMap, /// The git facts this rule set declared (CLOUD-907). git: &'a crate::git::GitFacts, + /// The symbol census, iff this rule set declared it (CLOUD-760). + symbols: &'a crate::facts::Look, bundles: &'a [crate::policy::Bundle], } @@ -4640,15 +4662,7 @@ fn run_rule( // is what decides, and returning here would switch those off by a value // nobody aimed at them. if rule.kind == RuleKind::Policy { - return Ok(policy_rule( - rule, - inputs.files, - inputs.documents, - inputs.produced, - inputs.git, - inputs.bundles, - findings, - )); + return Ok(policy_rule(rule, inputs, findings)); } let Some(glob) = rule.glob.as_deref() else { // Unreachable for a tree-scoped kind, whose census requires `glob`. @@ -5420,6 +5434,27 @@ fn project_uses( } } +/// Resolve the symbol fact, and ONLY when a row declared it (CLOUD-760). +/// +/// `git_facts`' shape exactly, for a sharper version of its reason. That +/// function's header records CLOUD-851 taking `check` from 4.76ms to 10.01ms by +/// reading HEAD unconditionally — a 2.103x bill for one cheap read. This fact +/// spawns `cargo clippy` over the whole crate, so the same mistake here would not +/// be a slowdown but a different tool. +/// +/// The `Look` is carried rather than unwrapped: a missing analyser is +/// could-not-look, and a projection that turned it into an empty site list would +/// report a crate with no spawns at all. +fn symbols_fact(rules: &[Rule], root: &Path) -> crate::facts::Look { + if !rules.iter().any(|rule| rule.symbols) { + // Nothing asked, so nothing is spent — and the projection below emits + // `null` rather than an empty census, which a module would read as + // "resolved, found nothing". + return crate::facts::Look::IsNot; + } + crate::symbols::resolve(root) +} + pub(crate) fn tree_document( cache: &BTreeMap<(String, Wanted), Acquired>, declared: &Declared<'_>, @@ -5432,6 +5467,11 @@ pub(crate) fn tree_document( // ruleset DECLARED (CLOUD-907). Handed in for `produced`'s reason: the // projection is pure, and the reads that fill this are the caller's. git: &crate::git::GitFacts, + // The `Cost::Effect` fact (CLOUD-760), acquired once at the boundary and only + // when a row DECLARED it. Handed in for `git`'s reason, and for a stronger + // one: this is the only fact whose acquisition runs an analyser, so leaving + // the spend to the caller is what keeps a projection from spawning. + symbols: &crate::facts::Look, ) -> (String, Vec<(String, NotAcquired)>) { let Declared { documents, @@ -5535,6 +5575,50 @@ pub(crate) fn tree_document( crate::facts::Fact::GitRef => serde_json::json!(git.refs), crate::facts::Fact::GitRange => serde_json::json!(git.ranges), crate::facts::Fact::Landing => serde_json::json!(git.landing), + // The `Cost::Effect` fact (CLOUD-760). THREE-VALUED, and the three + // answers get three different projections, because collapsing any + // pair of them is CLOUD-251's vacuous pass: + // + // * `IsNot` — no row declared it, so nothing ran. + // * `CouldNotLook` — the analyser ran and its stream did not parse, + // or it could not be run at all. + // + // Both project `null`, and the KEY IS ALWAYS PRESENT. That is the + // git family's invariant (CLOUD-907) and it decides here too: a key + // that comes and goes cannot be written against at all, because + // `not input.tree.symbols` is indistinguishable from a predicate + // that simply does not hold. What must never happen is either of + // them reaching a module as an empty `sites` list — clean is never + // inferred from a stream that failed to parse, and never from an + // analyser nobody asked to run. + // + // * `Is` — the census, WITH its provenance. Tool, version and the + // pinned invocation travel beside the sites because §6 byte- + // stability is a claim about a named producer; a bare site list + // is not attributable to anything. An EMPTY `sites` here is the + // third answer and a real one: the analyser ran and resolved no + // site. `null` and `[]` are the pair this projection keeps apart. + crate::facts::Fact::Symbols => match symbols { + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + serde_json::Value::Null + } + crate::facts::Look::Is(resolved) => serde_json::json!({ + "provenance": { + "tool": resolved.provenance.tool, + "version": resolved.provenance.version, + "invocation": resolved.provenance.invocation, + }, + "sites": resolved + .sites + .iter() + .map(|site| serde_json::json!({ + "path": site.path, + "line": site.line, + "lint": site.lint, + })) + .collect::>(), + }), + }, crate::facts::Fact::Bypass | crate::facts::Fact::Receipts | crate::facts::Fact::Keys @@ -5569,23 +5653,30 @@ pub(crate) fn tree_document( /// rather than resolves. The failures that ARE errors (an unreadable module, an /// undeclared id, a colliding one) were refused at load, where a config fault /// belongs. +/// Takes the whole [`RunInputs`] rather than six of its members, which is what +/// that struct's doc already says it is for. Enumerating them was affordable +/// while the set was small; CLOUD-760's `symbols` made it the seventh and the +/// arity lint the messenger. Passing the group also means the next acquisition +/// row reaches here without touching this signature. fn policy_rule( rule: &Rule, - // The run's one tree walk, hoisted in `run` and handed down (CLOUD-845). - // `tracked` is the SUBJECT's path list, and the subject is always the - // working tree — `--config-from` redirects the policy AUTHORITY (which rules - // and which module bytes), never what is being judged. So there is no ref - // branch here and no could-not-look arm for one. - tracked: &[String], - // The run's one document cache (CLOUD-850). - documents: &BTreeMap<(String, Wanted), Acquired>, - // The run's one read of the sink store (CLOUD-851). - produced: &BTreeMap, - // The run's one acquisition of the declared git facts (CLOUD-907). - git: &crate::git::GitFacts, - bundles: &[crate::policy::Bundle], + inputs: &RunInputs<'_>, findings: &mut Vec, ) -> Option { + // The run's one tree walk, hoisted in `run` and handed down (CLOUD-845). + // `files` is the SUBJECT's path list, and the subject is always the working + // tree — `--config-from` redirects the policy AUTHORITY (which rules and + // which module bytes), never what is being judged. So there is no ref branch + // here and no could-not-look arm for one. + let tracked = inputs.files; + let RunInputs { + documents, + produced, + git, + symbols, + bundles, + .. + } = *inputs; let Some(bundle) = bundles.iter().find(|bundle| bundle.id() == rule.id) else { // The row enabled a bundle the caller did not load. Not a pass: this // surface has nothing to decide with, and reporting clean would be a @@ -5639,6 +5730,7 @@ fn policy_rule( tracked, produced, git, + symbols, ); if !not_acquired.is_empty() { // COULD NOT LOOK, and never an empty deny set (CLOUD-251). A bundle @@ -8083,6 +8175,7 @@ mod tests { &[], &BTreeMap::new(), &crate::git::GitFacts::default(), + &crate::facts::Look::IsNot, ); let parsed: serde_json::Value = serde_json::from_str(&input).expect("the input is JSON"); let tree = parsed @@ -8644,6 +8737,7 @@ mod tests { &files, &BTreeMap::new(), &crate::git::GitFacts::default(), + &crate::facts::Look::IsNot, ); assert!( not_acquired.is_empty(), @@ -8773,6 +8867,7 @@ mod tests { fix: None, produces: None, exclude_paths: Vec::new(), + symbols: false, run: None, verbatim: None, identity_key: None, diff --git a/crates/batten/src/spec.rs b/crates/batten/src/spec.rs index 1295dfeec..08e9b6cca 100644 --- a/crates/batten/src/spec.rs +++ b/crates/batten/src/spec.rs @@ -304,6 +304,7 @@ mod tests { "commit".to_owned(), "commit check".to_owned(), "config".to_owned(), + "config deprecations".to_owned(), "config epoch".to_owned(), "config lint".to_owned(), "config show".to_owned(), @@ -467,6 +468,7 @@ mod tests { "commit".to_owned(), "commit check".to_owned(), "config".to_owned(), + "config deprecations".to_owned(), "config epoch".to_owned(), "config lint".to_owned(), "config show".to_owned(), diff --git a/crates/batten/src/surface.rs b/crates/batten/src/surface.rs index 31d35ec7d..b3595182d 100644 --- a/crates/batten/src/surface.rs +++ b/crates/batten/src/surface.rs @@ -506,6 +506,29 @@ const ATTRIBUTION_HARNESS: FlagDecl = FlagDecl::optional_enum( harness_parser, ); +/// `--against ` on `config deprecations` (CLOUD-360). +/// +/// The ref whose PUBLISHED schema the current one is compared against — normally +/// the latest release tag, which the `mise` task resolves and passes rather than +/// this binary enumerating tags. Named rather than defaulted: a gate that picked +/// its own baseline could quietly compare against something that makes it pass. +const AGAINST: FlagDecl = FlagDecl { + id: "against", + long: None, + short: None, + help: "The git ref whose published schema is the baseline (e.g. v0.0.111)", + env: EnvDecl::None, + global: false, + // POSITIONAL and required, which is how every other verb takes the one input + // it cannot work without. Not defaulted: a gate that picked its own baseline + // could quietly choose one that makes it pass. + positional: true, + required: true, + hidden: false, + rung: Rung::None, + value: ValueDecl::Str, +}; + /// `--host-rules ` on `config lint` (CLOUD-54). /// /// Data in, verdict out. The payload is the host ruleset the caller already @@ -1237,6 +1260,19 @@ pub const SURFACE: &[CommandDecl] = &[ // caller stamping a record needs and a bare digest cannot carry. flags: &[JSON, NO_CACHE], }, + // The removal half of the deprecation grammar (CLOUD-360). `config lint` + // judges the config in front of you; this judges the SURFACE across a + // release boundary, which is a different subject and so a sibling verb + // rather than a flag on that one. + CommandDecl { + path: "config deprecations", + about: "Report schema keys removed since a published release with no deprecation window", + data_channel: true, + // Reads committed bytes at a ref and the schema this binary derives. + // Nothing is written and no process is spawned. + effect: Effect::Read, + flags: &[JSON, AGAINST], + }, CommandDecl { path: "config lint", about: "Report policy smells in batten.toml (any smell is a violation)", diff --git a/crates/batten/src/symbols.rs b/crates/batten/src/symbols.rs new file mode 100644 index 000000000..ff2788a6b --- /dev/null +++ b/crates/batten/src/symbols.rs @@ -0,0 +1,365 @@ +//! Resolved-symbol facts, from a delegated analyser's structured output +//! (CLOUD-760). +//! +//! **The first occupant of [`Cost::Effect`]**, and the reserved variant's first +//! occupant owes an account of itself. `facts.rs` declared `Effect` and +//! `Surface::VerifyOnly` unoccupied on purpose — *"naming it is what keeps the +//! first fact that needs it from inventing its own"* — so this module states the +//! boundary rather than assuming one. +//! +//! # What the cheaper tiers cannot answer +//! +//! Three tiers can be asked *where does this crate use* `std::process::Command`, +//! and they answer differently. CLOUD-760 states the difference as three counts — +//! 14 from a byte scan, 11 from a syntax matcher, 9 from name resolution — and +//! **measured on this tree the counts are the wrong comparison**, because the +//! byte tier and the resolved tier both report 16 while agreeing about almost +//! nothing. +//! +//! The disagreement is in the SET, in both directions: +//! +//! * `surface.rs` is in the byte set and not the resolved one — it calls +//! `clap::Command::new` twice, which is the collision the fact exists to +//! separate; +//! * `exec.rs` carries more resolved usages than `::new` occurrences, because an +//! import and a type annotation are usages a byte scan cannot see. +//! +//! So the discriminator is membership rather than arithmetic, and +//! `tests/symbols.rs` asserts it that way. A coinciding total is exactly the +//! shape CLOUD-418 warns about: a test that passes for the wrong reason. +//! +//! The cheaper tiers are not approximations of this one; they answer different +//! questions. Only a tool that has performed name resolution can separate +//! `clap::Command` from `std::process::Command`, and Batten must not compute that +//! itself (CLOUD-756: *"Batten must not COMPUTE symbol resolution. It should +//! CONSUME resolved facts — and an exit code is one bit, not resolved facts."*). +//! +//! **What this tier reports is TYPE USAGES, not call sites**, stated because the +//! two are easily confused: `clippy::disallowed_types` fires wherever the type is +//! named, so an import counts. That is the honest reading of "where does this +//! crate use a spawn type", and it is a superset of "where does it call `::new`". +//! +//! # `--force-warn` is what makes the fact possible +//! +//! Every spawn site in this crate already carries an `#[expect(clippy:: +//! disallowed_types, reason = …)]`, so under an ordinary run the lint is +//! *fulfilled* and clippy emits nothing. Reading the diagnostics would then count +//! zero — not because there are no spawns, but because they are all accounted +//! for. +//! +//! `--force-warn` overrides `allow` and `expect` alike, so the lint fires at +//! every site regardless of its annotation. That turns an ENFORCEMENT mechanism +//! into an ACQUISITION one: the same lint that refuses a new spawn at +//! `lint:clippy` is, under this flag, an inventory of every spawn there is. The +//! two readings must not be confused, which is why this module never uses +//! `-D warnings` — a run that aborts on the first diagnostic cannot enumerate. +//! +//! # Generalised from `secrets.rs`, not copied from it +//! +//! `secrets.rs` is this crate's prior art for adopting a delegated analyser, and +//! CLOUD-760 says to mine it rather than mirror it. What carries across is the +//! SHAPE — a pinned binary, flags pinned beside the parser, and an exit status +//! reconciled against the parse — and one invariant carried verbatim: +//! +//! > **clean is never inferred from a stream that failed to parse.** +//! +//! What does not carry across is the parsing itself. ripsecrets emits +//! colon-delimited text and performs no name resolution at all; clippy emits JSON +//! carrying spans, lint names and resolved paths. That difference is the whole +//! reason this tier exists, and it is why this is a second adopter of one shape +//! rather than a second copy of one parser. +//! +//! # Pointer-only, and here it is load-bearing +//! +//! A diagnostic carries a rendered message, a span, and the source text that +//! matched. None of that may escape (non-negotiable rule 4), and [`Site`] is +//! shaped so it cannot: it holds a repo-relative path, a line, and the lint's +//! name. The message and the source excerpt are dropped at the parse boundary and +//! never stored, which is `secrets.rs`'s discipline of wrapping a span at the +//! pipe rather than trusting every later caller not to print it. + +use std::path::Path; + +use crate::facts::Look; + +/// The delegated analyser, pinned. +/// +/// `cargo` rather than `clippy-driver`: the driver needs a target directory, a +/// sysroot and the crate's own dependency closure resolved, and reproducing that +/// would be writing the analyser rather than adopting one. +pub const ANALYSER: &str = "cargo"; + +/// The flags, pinned **beside the parser** that reads their output +/// (`secrets.rs`'s discipline). +/// +/// `--message-format=json` is the fact; `--quiet` keeps cargo's own progress off +/// the stream being parsed. `--force-warn` follows `--` so it reaches the lint +/// driver rather than cargo, and it is what surfaces the `#[expect]`ed sites — +/// see the module doc. +pub const ANALYSER_FLAGS: &[&str] = &[ + "clippy", + "--quiet", + "--message-format=json", + "--", + "--force-warn", + "clippy::disallowed_types", +]; + +/// Where this analyser is installed from, named so a missing one points at the +/// remedy rather than at a bare "not found". +const PROVISION_HINT: &str = "mise install"; + +/// One resolved call site: a pointer, and nothing the analyser said about it. +/// +/// The lint NAME is kept because it is what distinguishes one census from +/// another; the lint's rendered MESSAGE is not, because it quotes the source. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[non_exhaustive] +pub struct Site { + /// Repo-relative and `/`-separated, so the fact is byte-stable across + /// checkouts — a diagnostic's own `file_name` is relative to the workspace + /// cargo ran in, which is not the same guarantee. + pub path: String, + /// 1-indexed, as the analyser reports it. + pub line: u32, + /// The lint that fired, e.g. `clippy::disallowed_types`. + pub lint: String, +} + +/// Which tool produced a fact, and how. +/// +/// **Part of the fact rather than beside it**, because a fact whose meaning +/// depends on an unrecorded tool version is not canonical: two runs that +/// disagree because the analyser changed are indistinguishable from two runs +/// that disagree because the tree changed, and §6 byte-stability cannot hold +/// across that. Recording the version makes a differing analyser VISIBLE rather +/// than silently absorbed. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct Provenance { + /// The program, as invoked. + pub tool: String, + /// Its self-reported version string. + pub version: String, + /// The exact flags, so a reader can tell which question was asked. + pub invocation: Vec, +} + +/// What one analyser run resolved. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct Resolved { + /// Which tool, which version, which invocation. + pub provenance: Provenance, + /// Every site the run reported, sorted — a diagnostic stream's order is the + /// compiler's scheduling and is not stable across runs. + pub sites: Vec, +} + +/// The analyser's own version, as provenance. +/// +/// Its own spawn rather than a field parsed out of the diagnostic stream, +/// because the stream carries no version at all — and inferring one from the +/// diagnostics would be exactly the unrecorded dependency this guards against. +fn version(root: &Path) -> Look { + #[expect( + clippy::disallowed_types, + reason = "stays: this fact IS Cost::Effect — resolving it runs a program, which is the classification, not an accident of it. The version is provenance and a fact without it is not canonical (CLOUD-760)" + )] + let spawned = std::process::Command::new(ANALYSER) + .args(["clippy", "--version"]) + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output(); + let Ok(output) = spawned else { + return Look::CouldNotLook; + }; + if !output.status.success() { + return Look::CouldNotLook; + } + let text = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + if text.is_empty() { + Look::CouldNotLook + } else { + Look::Is(text) + } +} + +/// Resolve the symbol fact by running the analyser over `root`. +/// +/// # Three answers, and the third is why this is not a `bool` +/// +/// [`Look::Is`] carries what the analyser resolved. [`Look::IsNot`] is never +/// produced here — an analyser that ran and found nothing still produces a +/// `Resolved` with an empty site list, which is a different statement from +/// having failed to look. [`Look::CouldNotLook`] is every way the run did not +/// yield a trustworthy answer, and it is deliberately wide: +/// +/// * the analyser is not installed, or could not be spawned; +/// * its version could not be read, so the fact would not be canonical; +/// * a diagnostic line is not JSON this build can read; +/// * the exit status and the parse disagree. +/// +/// That last one is `secrets.rs`'s invariant, carried verbatim: **clean is never +/// inferred from a stream that failed to parse.** A run that exits non-zero +/// having emitted nothing parseable is not a clean tree — it is an analyser that +/// failed, and reporting zero sites from it would be the silent false green the +/// whole discipline exists to prevent. +#[must_use] +pub fn resolve(root: &Path) -> Look { + let Look::Is(version) = version(root) else { + return Look::CouldNotLook; + }; + + #[expect( + clippy::disallowed_types, + reason = "stays: this fact IS Cost::Effect — resolving it runs the delegated analyser, which is the classification. Adopting clippy rather than computing name resolution is CLOUD-756's decision" + )] + let spawned = std::process::Command::new(ANALYSER) + .args(ANALYSER_FLAGS) + .current_dir(root) + // Both streams captured, NEITHER forwarded: stdout is the fact and + // stderr can carry a path the analyser failed to read. Echoing a child's + // stream would put output Batten never shaped onto Batten's own. + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output(); + let Ok(output) = spawned else { + return Look::CouldNotLook; + }; + + let stdout = String::from_utf8_lossy(&output.stdout); + match sites_in(&stdout, root) { + Look::Is(sites) => { + // THE CROSS-CHECK, in the one direction that exists here. clippy + // exits 0 under `--force-warn` however many diagnostics it emits — + // the flag warns rather than denies — so a non-zero status means the + // analyser itself failed, and the sites parsed out of a failed run + // describe a compilation that did not finish. Reporting them would be + // reporting a census of a tree that did not build. + if output.status.success() { + Look::Is(Resolved { + provenance: Provenance { + tool: ANALYSER.to_owned(), + version, + invocation: ANALYSER_FLAGS + .iter() + .map(|flag| (*flag).to_owned()) + .collect(), + }, + sites, + }) + } else { + Look::CouldNotLook + } + } + _ => Look::CouldNotLook, + } +} + +/// Parse an analyser stream into sites. +/// +/// Separated from the spawn for `secrets.rs`'s `parse_line` reason and +/// `.claude/rules/rust.md`'s: the failing condition is a STREAM SHAPE rather than +/// a repository state, so the decision is extracted and tested directly rather +/// than through a fixture that has to make a real analyser misbehave. +/// +/// A line that is not JSON at all is skipped rather than refused — cargo +/// interleaves its own non-JSON progress on some paths, and refusing those would +/// make the fact unresolvable for a reason that has nothing to do with the +/// analyser. A line that IS JSON and IS a compiler message but cannot be read as +/// one is [`Look::CouldNotLook`]: that is a diagnostic this build does not +/// understand, and skipping it would undercount silently. +#[must_use] +pub fn sites_in(stream: &str, root: &Path) -> Look> { + let mut sites = Vec::new(); + for emitted in stream.lines() { + let emitted = emitted.trim(); + if emitted.is_empty() { + continue; + } + let Ok(record) = serde_json::from_str::(emitted) else { + // Not JSON: cargo's own chatter, not a diagnostic. + continue; + }; + if record.get("reason").and_then(serde_json::Value::as_str) != Some("compiler-message") { + continue; + } + let Some(message) = record.get("message") else { + return Look::CouldNotLook; + }; + let Some(lint) = message + .get("code") + .and_then(|code| code.get("code")) + .and_then(serde_json::Value::as_str) + else { + // A compiler message with no lint code is a plain error or warning + // and names no census. + continue; + }; + let Some(spans) = message.get("spans").and_then(serde_json::Value::as_array) else { + return Look::CouldNotLook; + }; + for span in spans { + if span.get("is_primary").and_then(serde_json::Value::as_bool) != Some(true) { + continue; + } + let (Some(file), Some(line_start)) = ( + span.get("file_name").and_then(serde_json::Value::as_str), + span.get("line_start").and_then(serde_json::Value::as_u64), + ) else { + return Look::CouldNotLook; + }; + let Ok(line_start) = u32::try_from(line_start) else { + return Look::CouldNotLook; + }; + sites.push(Site { + path: canonical(file, root), + line: line_start, + lint: lint.to_owned(), + }); + } + } + // Sorted, because a diagnostic stream's order is the compiler's scheduling + // and two runs over identical bytes must produce identical output (§6). + sites.sort(); + Look::Is(sites) +} + +/// A diagnostic's path, as a repo-relative `/`-separated string. +/// +/// The analyser reports paths relative to the workspace it ran in, which is not +/// the same guarantee as repo-relative — and an absolute one would make the fact +/// vary by checkout location, which §6 forbids. +fn canonical(file: &str, root: &Path) -> String { + let normalised = file.replace('\\', "/"); + let root = root.to_string_lossy().replace('\\', "/"); + let trimmed = normalised + .strip_prefix(&format!("{}/", root.trim_end_matches('/'))) + .unwrap_or(&normalised); + trimmed.to_owned() +} + +/// How many sites one lint reported. +/// +/// The census predicate, as a function over the fact rather than over a stream: +/// a caller asking "how many `std::process::Command` sites are there" is asking +/// about the resolved fact, and giving it the stream would be handing back the +/// parsing problem this module exists to solve. +#[must_use] +pub fn count_of(resolved: &Resolved, lint: &str) -> usize { + resolved + .sites + .iter() + .filter(|site| site.lint == lint) + .count() +} + +/// The missing-analyser message, naming the remedy. +#[must_use] +pub fn unavailable() -> String { + format!( + "the delegated analyser `{ANALYSER} clippy` is not available; install it with `{PROVISION_HINT}`" + ) +} diff --git a/crates/batten/tests/cli.rs b/crates/batten/tests/cli.rs index 780dccc78..0cf40c523 100644 --- a/crates/batten/tests/cli.rs +++ b/crates/batten/tests/cli.rs @@ -4140,6 +4140,15 @@ fn census_fixture(name: &str) -> (PathBuf, PathBuf, String) { // `CENSUS_POSITIONALS` rather than by this call site, so the argv and the // file it points at cannot drift apart. .file("census-brief.md", &census_brief()) + // A published schema for `config deprecations` to use as its baseline. + // Deliberately a SUBSET of the real surface — every key here still + // exists — so the census exercises the CLEAN arm, which is what + // `no_progress_reaches_stderr_when_it_is_not_a_terminal` needs and what + // makes the `-J` document the interesting case (`lint brief`'s reason). + .file( + "schema/batten.schema.json", + "{\n \"properties\": {\n \"version\": {}\n }\n}\n", + ) .git() .base_commit() .work_commit() @@ -4211,6 +4220,12 @@ const CENSUS_POSITIONALS: &[(&str, &str)] = &[ // A valid check name; `receipt status` answers `missing` for it, which is a // document like any other. ("receipt status", "verify"), + // `HEAD`, where the fixture commits a published schema whose keys all still + // exist — so the census asserts about a CLEAN run. The could-not-look arm + // emits an ::error:: line by design, which is the one thing a data-channel + // verb's stderr may not carry unprompted; that arm is covered by + // `the_removal_gate_reports_a_verdict_or_refuses_to_guess` instead. + ("config deprecations", "HEAD"), // A brief that satisfies the schema, so the census asserts about a CLEAN run // — which is what `no_progress_reaches_stderr_when_it_is_not_a_terminal` // needs, and what makes the empty `-J` document the interesting case. @@ -10468,3 +10483,84 @@ fn a_tracked_instruction_may_not_prescribe_the_denied_commit_identity() { "a clean tree renders nothing" ); } + +// --- the config deprecation grammar (CLOUD-360) ----------------------------- + +/// §7(d) over the COMPILED BINARY: the removal gate answers, and its three exit +/// codes are the contract §5 states. +/// +/// The unannounced-removal arm cannot be built here — it needs a schema key to +/// vanish, which is a change to the config TYPES rather than to a fixture — so it +/// was observed against the real gate instead: renaming `capture` to +/// `capture_v2` in the config surface (a change that compiles and passes every +/// test) made `mise run config-deprecations` exit 2 naming `capture`, and +/// declaring a `DEPRECATED_KEYS` row for it returned the gate to 0. Both arms, +/// so the gate requires the ANNOUNCEMENT rather than the absence of change. +/// +/// What this case holds is the half a fixture can: the clean verdict against a +/// real published baseline, and the could-not-look refusal. +#[test] +fn the_removal_gate_reports_a_verdict_or_refuses_to_guess() { + let dir = repo_with_committed_config("deprecations-verdict"); + + // A ref that carries no schema is COULD NOT LOOK — exit 3, never 0. + // Reporting "no key was removed" having compared nothing is the vacuous pass + // CLOUD-251 names, and it is the one outcome this gate must never produce. + let absent = batten_with( + &dir, + &["config", "deprecations", "refs/tags/nope-not-a-tag"], + &[], + ); + assert_eq!( + absent.status.code(), + Some(3), + "an unreadable baseline is exit 3: {}", + String::from_utf8_lossy(&absent.stderr) + ); + assert!( + absent.stdout.is_empty(), + "no verdict is emitted when nothing could be compared" + ); + + // And the baseline is REQUIRED: a gate that picked its own could quietly + // choose one that makes it pass. + let unbaselined = batten_with(&dir, &["config", "deprecations"], &[]); + assert_eq!( + unbaselined.status.code(), + Some(1), + "omitting the baseline is a usage error, not a default" + ); +} + +/// §7(a)-(c) over the compiled binary: the three config-side outcomes, and the +/// one that matters is that they are TOLD APART. +/// +/// The window itself is exercised by `config::tests`, which supply a table — +/// `DEPRECATED_KEYS` ships empty on purpose, and inventing a row so a fixture had +/// something to find would put a key in the published schema no consumer should +/// write. What the binary can be held to is the boundary those cases cannot see: +/// that an unknown key is still refused, and refused with a diagnostic that does +/// not borrow the deprecation vocabulary. +#[test] +fn an_unknown_key_is_refused_without_borrowing_the_deprecation_vocabulary() { + let dir = scratch("deprecations-unknown"); + fs::create_dir_all(&dir).expect("create dir"); + fs::write( + dir.join("batten.toml"), + "version = 1\n[not_a_batten_key]\nvalue = 1\n", + ) + .expect("write the fixture config"); + + let output = batten_with(&dir, &["config", "show"], &[]); + assert_eq!( + output.status.code(), + Some(1), + "strictness is unchanged: an unknown key is still a hard error" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !stderr.contains("deprecated") && !stderr.contains("expires"), + "an unknown key must not read as a deprecated one — the two remedies \ + differ, and collapsing them makes the window invisible: {stderr}" + ); +} diff --git a/crates/batten/tests/facts.rs b/crates/batten/tests/facts.rs index 4835838f5..19dbaae81 100644 --- a/crates/batten/tests/facts.rs +++ b/crates/batten/tests/facts.rs @@ -36,7 +36,7 @@ fn a_content_block_envelope_unwraps_to_the_payload_a_bare_one_carries() { use batten::facts::{ AGENT_SOURCED, BYPASS, Class, Cost, DOCUMENT, Fact, GIT_HEAD, GIT_RANGE, GIT_REF, GIT_REMOTE, GIT_STATUS, INVOCATIONS, KEYS, LANDING, LINES, Look, PRODUCED, PROSPECTIVE, RECEIPTS, STOP, - Surface, TRACKED, USES, WAIVED, + SYMBOLS, Surface, TRACKED, USES, WAIVED, }; #[test] @@ -128,6 +128,7 @@ fn every_fact_returns_its_stated_const() { Fact::Landing => LANDING, Fact::Invocations => INVOCATIONS, Fact::Uses => USES, + Fact::Symbols => SYMBOLS, } }; @@ -136,7 +137,7 @@ fn every_fact_returns_its_stated_const() { // rather than quietly shrinking the census. assert_eq!( Fact::ALL.len(), - 19, + 20, "the census covers every fact; update this count deliberately when the \ model gains or loses one" ); @@ -151,6 +152,42 @@ fn every_fact_returns_its_stated_const() { } } +#[test] +fn no_effect_fact_is_hook_resolvable() { + // CLOUD-760's §7(e). `Cost::Effect` means resolving the fact RUNS something — + // here an analyser over the whole crate — and the mediated path is budgeted + // in milliseconds per call. `run_static` already refuses a spawning kind on + // that surface, and a fact classified `Surface::Hook` while costing `Effect` + // would reintroduce exactly what that refusal removes, one layer down and + // without passing through it. + // + // A CENSUS OVER `Fact::ALL`, not an assertion about `Symbols`. The first + // `Effect` fact is the occasion for this rule, never its subject: naming it + // would leave the second one unguarded, which is the shape CLOUD-849 + // measured in this very file. + // + // Fails by: flipping any `Cost::Effect` fact's `const` to `Surface::Hook`. + let mut effect_facts = 0_usize; + for fact in Fact::ALL { + let class = fact.class(); + if class.cost != Cost::Effect { + continue; + } + effect_facts += 1; + assert!( + !class.resolvable_on(Surface::Hook), + "{}: a Cost::Effect fact must not be resolvable on the mediated path", + fact.as_str() + ); + } + // ANTI-VACUITY. An all-`Free` model satisfies the loop above without the + // rule ever being exercised, and that green is CLOUD-251's shape. + assert!( + effect_facts > 0, + "the model carries no Cost::Effect fact, so this census asserted nothing" + ); +} + #[test] fn every_class_arm_names_its_own_const() { // THE HALF A VALUE COMPARISON CANNOT MAKE, and it was measured rather than diff --git a/crates/batten/tests/pointer_only.rs b/crates/batten/tests/pointer_only.rs index fdd930f97..46c27c5d5 100644 --- a/crates/batten/tests/pointer_only.rs +++ b/crates/batten/tests/pointer_only.rs @@ -303,6 +303,19 @@ impl Corpus { ), ) .file("counted.txt", &format!("{}\n", canary("counted"))) + // A published schema for `config deprecations` to compare against, + // carrying a CONTENT canary in a description. The verb must name the + // removed key and never the schema body, so a run that echoed what it + // read leaks this and fails the census. The key itself is absent from + // the real surface, so the comparison finds a removal and the verb + // reaches its reporting path rather than its clean one. + .file( + "schema/batten.schema.json", + &format!( + "{{\n \"properties\": {{\n \"a_removed_key\": {{\n \"description\": \"{}\"\n }}\n }}\n}}\n", + canary("schemabody"), + ), + ) // A file a `policy` row reads as LINES (CLOUD-846). The module below // decides over it and denies; the canary is the line's content, so // any verb that echoed what the module saw fails the census. @@ -516,6 +529,16 @@ const CENSUS: &[Verb] = &[ stdin: Stdin::Nothing, disposition: Disposition::PointerOnly, }, + // Key names and a count, never the schema body or a configured value + // (CLOUD-360). The remedy for a finding here is declaring a window, so the + // schema text adds nothing a reader needs and would put the config surface + // into a log. + Verb { + path: "config deprecations", + args: &["HEAD"], + stdin: Stdin::Nothing, + disposition: Disposition::PointerOnly, + }, Verb { path: "spec", args: &[], diff --git a/crates/batten/tests/policy_input_narrowing.rs b/crates/batten/tests/policy_input_narrowing.rs index dac4cb85a..125c7724b 100644 --- a/crates/batten/tests/policy_input_narrowing.rs +++ b/crates/batten/tests/policy_input_narrowing.rs @@ -30,7 +30,7 @@ use std::path::Path; -use batten::hook::{Decision, Envelope, Harness, Policy}; +use batten::hook::{Envelope, Harness, Policy}; /// A repository whose `batten.toml` registers one module and nothing else — no /// receipt row, no keyed shape row, no waiver. @@ -86,46 +86,19 @@ fn envelope(command: &str) -> Envelope { batten::hook::decode(Harness::ClaudeCode, &payload.to_string()).expect("the payload decodes") } -#[test] -fn adjudicating_over_the_widened_document_spawns_no_git() { - let dir = std::env::temp_dir().join(format!("batten-narrowing-{}", std::process::id())); - // Built BEFORE the measurement: `policy::load` reads the module off disk, - // and this case is about what ADJUDICATION costs, not what loading does. - let policy = module_policy(&dir); - let call = envelope("git status"); - - let before = batten::git::queries_spawned(); - let decision = batten::hook::adjudicate( - &policy, - &call, - &batten::hook::Facts::none( - &batten::stop::StopFacts::default(), - &batten::waiver::Live::new(), - ), - ); - let after = batten::git::queries_spawned(); - - assert_eq!( - after - before, - 0, - "projecting the fact set must acquire nothing — `adjudicate` is \ - contractually pure and the facts arrive already resolved" - ); - assert_eq!(decision, Decision::Allow); - - // THE ANTI-VACUITY HALF (CLOUD-418), in this function rather than beside - // it. The assertion above is "the count did not move", which a counter - // wired to nothing satisfies perfectly — so it means nothing until the - // counter is shown able to move. It cannot be its own `#[test]`: the - // counter is process-global and a sibling case spawning git would race the - // delta above under a harness that threads rather than forks, which is the - // failure mode `policy_engine_count.rs` was split out to avoid. - drop(batten::git::repo_root(Path::new("."))); - assert!( - batten::git::queries_spawned() > after, - "the counter never moves, so the zero delta above asserts nothing" - ); -} +/// RETIRED (CLOUD-740). This measured the delta in `git::queries_spawned()` +/// across `adjudicate`, asserting the mediated path acquired no fact by spawning. +/// +/// Its ANTI-VACUITY half is what retires it, and honourably: the case ended by +/// calling `git::repo_root` and asserting the counter MOVED, because "the count +/// did not change" is satisfied perfectly by a counter wired to nothing. Nothing +/// in this crate spawns `git` any more, so that half can never pass again — the +/// counter is gone with the spawns it counted, and a case that cannot discriminate +/// is exactly what CLOUD-418 refuses to ship as coverage. +/// +/// What it asserted is now true of the WHOLE crate rather than of one function, +/// and is asserted where that is decidable: `git::tests::no_second_git_invoker_ +/// exists` scans every `src/` file for a literal `git` spawn and finds none. #[test] fn a_mediated_call_policy_row_asks_the_boundary_for_no_fact_it_did_not_already_need() { diff --git a/crates/batten/tests/primitives.rs b/crates/batten/tests/primitives.rs index 6966f7e29..c0406a6b2 100644 --- a/crates/batten/tests/primitives.rs +++ b/crates/batten/tests/primitives.rs @@ -153,6 +153,159 @@ fn seeded(name: &str) -> Repo { // --- merged-ness ------------------------------------------------------------- +/// CLOUD-739's own §2 gate, made explicit. +/// +/// The identity moved in-process, so the HASHES differ from `git patch-id`'s by +/// construction — a test asserting they matched would be asserting the migration +/// did not happen. What must not move is the VERDICT, and this is the corpus the +/// row nominates: the rebase, squash and cherry-pick shapes the keystone fixture +/// already builds, which exist precisely because ancestry gets them wrong where +/// patch identity gets them right. +#[test] +fn the_in_process_identity_gives_the_verdict_the_spawned_one_gave() { + // A cherry-picked landing: the change is on `main` under a new SHA, with no + // reachability path back. `git patch-id --stable` reported this Merged, and + // so must the in-process identity. + let repo = seeded("differential-replay"); + repo.git(&["checkout", "-q", "-b", "feature"]); + repo.write("f.txt", "the work\n"); + let before = repo.commit("feat: the work"); + repo.git(&["checkout", "-q", "main"]); + repo.write("other.txt", "unrelated\n"); + repo.commit("chore: main moves on"); + let landed_as = repo.replay(&before); + assert_ne!( + before, landed_as, + "the fixture must actually rewrite the SHA" + ); + + let landing = repo.landing("main", "feature"); + assert_eq!( + landing.verdict, + git::Verdict::Landed, + "a replayed landing is Landed: {:?}", + landing.verdict + ); + + // And the negative arm, without which the positive one discriminates + // nothing: work that never landed must still read as not landed. + let fresh = seeded("differential-unlanded"); + fresh.git(&["checkout", "-q", "-b", "feature"]); + fresh.write("g.txt", "never lands\n"); + fresh.commit("feat: unlanded"); + assert_eq!( + fresh.landing("main", "feature").verdict, + git::Verdict::NotLandedWithinWindow, + "unlanded work has a specific verdict, and `!= Landed` would accept any \ + of the other three" + ); +} + +/// CLOUD-739 §7(a). The collision `--binary` existed to prevent, now prevented +/// by construction rather than by a zlib patch body. +/// +/// Without `--binary` git rendered any binary change as +/// `Binary files a/x and b/x differ` — identical text for ANY two changes to one +/// path, so two unrelated edits shared an identity and one was reported as the +/// other's landing. The in-process identity uses the blob ids, which differ. +/// +/// Fails by: identifying a binary side by anything both edits share. +#[test] +fn two_unrelated_binary_edits_to_one_path_do_not_share_an_identity() { + let repo = seeded("binary-collision"); + repo.write_bytes("blob.bin", &[0u8, 1, 2, 3]); + repo.commit("chore: seed the binary"); + + repo.git(&["checkout", "-q", "-b", "left"]); + repo.write_bytes("blob.bin", &[0u8, 9, 9, 9]); + repo.commit("chore: one binary edit"); + + repo.git(&["checkout", "-q", "main"]); + repo.git(&["checkout", "-q", "-b", "right"]); + repo.write_bytes("blob.bin", &[0u8, 7, 7, 7]); + repo.commit("chore: an unrelated binary edit"); + + // `right`'s change is not `left`'s, so `left` must not read as landed on it. + assert_eq!( + repo.landing("right", "left").verdict, + git::Verdict::NotLandedWithinWindow, + "two unrelated binary edits to one path shared an identity" + ); +} + +/// CLOUD-739 §7(e). Whitespace is SIGNIFICANT here, diverging from `git +/// patch-id`, and the divergence is asserted rather than left to be discovered. +/// +/// git folds whitespace away, so a whitespace-only difference collides and two +/// different changes share an identity. For this crate's consumers that is the +/// dangerous direction: a false *landed* suppresses `completion.unlanded`, and +/// telling an agent its work is on the trunk when it is not is the failure a +/// completion gate exists to prevent. +/// +/// Fails by: folding whitespace, which makes these two commits one change. +#[test] +fn a_whitespace_only_difference_is_its_own_change() { + let repo = seeded("whitespace-significant"); + repo.write("f.txt", "alpha\nbeta\n"); + repo.commit("chore: seed"); + + repo.git(&["checkout", "-q", "-b", "spaced"]); + repo.write("f.txt", "alpha\n beta\n"); + repo.commit("style: indent beta"); + + repo.git(&["checkout", "-q", "main"]); + repo.git(&["checkout", "-q", "-b", "tabbed"]); + repo.write("f.txt", "alpha\n\tbeta\n"); + repo.commit("style: tab beta"); + + assert_eq!( + repo.landing("tabbed", "spaced").verdict, + git::Verdict::NotLandedWithinWindow, + "a space-indented change read as landed on a tab-indented one" + ); +} + +/// CLOUD-739 §7(b), the half of it that is observable here: a renaming commit +/// replayed onto a moved base is still recognised as landed. +/// +/// The OTHER half — that renames are not *detected* — is deliberately not +/// asserted through `landing`, and the reason is worth carrying rather than +/// rediscovering. Rename detection is a pure function of the two trees, so a +/// fixture built out of trees hands the detecting and the non-detecting build +/// the same input and gets the same verdict from both. A case here claiming to +/// gate that decision could not go red under it, which is exactly what CLOUD-418 +/// calls coverage. The decision is gated where it IS observable — on the shape, +/// by `patch::tests::renames_are_not_a_shape_this_identity_can_take`, whose +/// named mutation fails the build rather than an assertion. +/// +/// Fails by: letting a path's identity depend on the base it sits on, so the +/// rename's SHA rewrite reads as a different change. +#[test] +fn a_replayed_rename_is_still_landed() { + let repo = seeded("rename-pinned"); + repo.write("before.txt", "content\n"); + repo.commit("chore: seed"); + + repo.git(&["checkout", "-q", "-b", "renamed"]); + repo.git(&["mv", "before.txt", "after.txt"]); + let renamed = repo.commit("refactor: rename the file"); + + // The rename lands by replay: the identity must survive the SHA rewrite, + // which is the property renames could break if a similarity heuristic were + // consulted and answered differently on the two sides. + repo.git(&["checkout", "-q", "main"]); + repo.write("unrelated.txt", "moves main on\n"); + repo.commit("chore: main moves on"); + let landed_as = repo.replay(&renamed); + assert_ne!(renamed, landed_as, "the fixture must rewrite the SHA"); + + assert_eq!( + repo.landing("main", "renamed").verdict, + git::Verdict::Landed, + "a replayed rename must still be recognised as landed" + ); +} + #[test] fn a_rebased_and_landed_branch_is_merged_though_ancestry_says_otherwise() { // THE KEYSTONE (CLOUD-36). A branch is cut, `main` moves on, the work is @@ -1531,3 +1684,45 @@ fn every_path_valued_toml_key_uses_a_literal_string() { offenders.join("\n ") ); } + +/// CLOUD-739's defect of record: a change at a NESTED path, replayed. +/// +/// The §7 corpus this row shipped with put every fixture file at the repository +/// ROOT, and that is what let the bug through. `gix_diff::tree` records a changed +/// SUBTREE alongside the blobs inside it, and a tree object's id encodes all of +/// its siblings — so `src/` carries one id on a branch that added `src/b.rs` and +/// another on a `main` that also gained `src/other.rs`. Hashing that id made the +/// identity depend on the base the change sits on. At the root there is no such +/// row to hash, because the only tree in the diff is the one being diffed. +/// +/// It surfaced as `done_not_landed::a_rebased_then_landed_branch_does_not_raise`, +/// whose fixture writes `src/b.rs` — a false NOT LANDED against work already on +/// the trunk, which is the direction `completion.unlanded` must never get wrong. +/// +/// Fails by: including directory entries in the change set, so a replayed change +/// under any subdirectory stops being recognisable. +#[test] +fn a_nested_change_is_recognised_when_it_lands_on_a_moved_base() { + let repo = seeded("nested-replay"); + repo.write("src/a.rs", "fn main() {}\n"); + repo.commit("chore: seed a source tree"); + + repo.git(&["checkout", "-q", "-b", "work"]); + repo.write("src/b.rs", "pub fn added() {}\n"); + let work = repo.commit("feat: add b"); + + // `main` gains a SIBLING inside the same directory, which is what moves the + // `src/` tree id apart on the two sides. A fixture whose base moved only at + // the root would pass with the defect present. + repo.git(&["checkout", "-q", "main"]); + repo.write("src/other.rs", "pub fn elsewhere() {}\n"); + repo.commit("chore: somebody else landed first"); + let landed_as = repo.replay(&work); + assert_ne!(work, landed_as, "the fixture must rewrite the SHA"); + + assert_eq!( + repo.landing("main", "work").verdict, + git::Verdict::Landed, + "a replayed change under a subdirectory is still the same change" + ); +} diff --git a/crates/batten/tests/symbols.rs b/crates/batten/tests/symbols.rs new file mode 100644 index 000000000..736ad35da --- /dev/null +++ b/crates/batten/tests/symbols.rs @@ -0,0 +1,234 @@ +//! CLOUD-760 §7: the first `Cost::Effect` fact, and what its first occupant owes. +//! +//! Its own integration binary because resolving the fact SPAWNS THE ANALYSER over +//! this crate, which takes real time and must not be paid by every unit-test +//! binary that happens to link `batten`. + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::expect_used)] + +use std::path::{Path, PathBuf}; + +use batten::facts::Look; +use batten::symbols; + +fn repo() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join("..") + .canonicalize() + .expect("the repository root resolves") +} + +/// §7(a). THE CASE THE FACT EXISTS FOR — asserted over SETS, not counts. +/// +/// CLOUD-760 specifies this discriminator as three numbers: 14 from a byte scan, +/// 11 from a syntax matcher, 9 from name resolution. **Measured on this tree the +/// numbers collide and the sets do not**, which makes a count comparison the +/// wrong assertion: the byte tier and the resolved tier both report 16 here, and +/// a test comparing totals would have passed while the tiers disagreed about +/// every interesting file. +/// +/// They disagree in both directions, which is the point: +/// +/// * `surface.rs` is in the BYTE set and absent from the RESOLVED one — it calls +/// `clap::Command::new` twice, and that is the whole `clap`-versus-`std` +/// collision the fact exists to separate; +/// * `exec.rs` carries more resolved usages than it has `::new` occurrences, +/// because a type is used by an import and an annotation as well as by a call, +/// and the byte tier cannot see those at all. +/// +/// So the assertion is membership. A count that happens to match proves nothing; +/// a set that excludes `surface.rs` could only have come from name resolution. +#[test] +fn the_resolved_set_excludes_what_only_name_resolution_can_exclude() { + let root = repo(); + let Look::Is(resolved) = symbols::resolve(&root) else { + panic!("the analyser did not resolve; this suite needs a working `cargo clippy`"); + }; + + let resolved_files: std::collections::BTreeSet<&str> = resolved + .sites + .iter() + .filter(|site| site.lint == "clippy::disallowed_types") + .map(|site| site.path.as_str()) + .collect(); + let byte_files = byte_scan(&root); + + // THE DISCRIMINATING PAIR, in both directions. + assert!( + byte_files.contains("crates/batten/src/surface.rs"), + "the byte tier must still name surface.rs, or this tree no longer \ + carries the `clap::Command` case and the discriminator is gone" + ); + assert!( + !resolved_files + .iter() + .any(|path| path.ends_with("surface.rs")), + "surface.rs uses `clap::Command`, not `std::process::Command` — a fact \ + naming it has not resolved anything. resolved={resolved_files:?}" + ); + + // And the sets differ, which a coinciding total would hide. + let byte_only: Vec<&str> = byte_files + .iter() + .copied() + .filter(|path| !resolved_files.contains(path)) + .collect(); + assert!( + !byte_only.is_empty(), + "the two tiers must disagree somewhere, or the resolved tier is buying \ + nothing over the byte one" + ); +} + +/// The byte tier, run here rather than quoted, so the comparison above is a +/// MEASUREMENT and not a remembered number. +fn byte_scan(root: &Path) -> std::collections::BTreeSet<&'static str> { + let dir = root.join("crates").join("batten").join("src"); + let needle = ["Command", "::new"].concat(); + let mut found = std::collections::BTreeSet::new(); + for entry in std::fs::read_dir(&dir).expect("read the source directory") { + let path = entry.expect("a directory entry").path(); + if path.extension().and_then(std::ffi::OsStr::to_str) != Some("rs") { + continue; + } + let source = std::fs::read_to_string(&path).expect("read a source file"); + if source.contains(needle.as_str()) { + let name = path + .file_name() + .and_then(std::ffi::OsStr::to_str) + .expect("a source file name"); + // Leaked deliberately and boundedly: the set is compared against + // repo-relative paths and this suite runs once. + found.insert(&*Box::leak( + format!("crates/batten/src/{name}").into_boxed_str(), + )); + } + } + found +} + +/// §7(b) and (c). Two runs over identical bytes agree, AND the provenance that +/// makes that claim meaningful is present. +/// +/// The determinism half alone is not enough, which is (c)'s point: two runs of a +/// DIFFERENT analyser version would also agree with each other, and a fact that +/// did not record which version produced it could not tell the reader that the +/// meaning had moved. So the version is asserted present and non-empty rather +/// than merely assumed to exist. +#[test] +fn two_runs_agree_and_the_analyser_that_produced_them_is_named() { + let root = repo(); + let (Look::Is(first), Look::Is(second)) = (symbols::resolve(&root), symbols::resolve(&root)) + else { + panic!("the analyser did not resolve twice"); + }; + + assert_eq!( + first.sites, second.sites, + "two runs over identical bytes must produce identical sites" + ); + assert_eq!( + first.provenance, second.provenance, + "and identical provenance" + ); + + assert_eq!(first.provenance.tool, symbols::ANALYSER); + assert!( + !first.provenance.version.is_empty(), + "a fact whose meaning depends on an unrecorded tool version is not \ + canonical — the version is part of the fact, not beside it" + ); + assert_eq!( + first.provenance.invocation, + symbols::ANALYSER_FLAGS + .iter() + .map(|flag| (*flag).to_owned()) + .collect::>(), + "the invocation records WHICH question was asked, so a reader can tell \ + an inventory run from an enforcement one" + ); +} + +/// §5. Pointer-only: a site carries a path, a line and a lint name, and nothing +/// the analyser said about the source. +#[test] +fn no_site_carries_a_byte_of_what_the_analyser_read() { + let root = repo(); + let Look::Is(resolved) = symbols::resolve(&root) else { + panic!("the analyser did not resolve"); + }; + for site in &resolved.sites { + assert!( + !site.path.starts_with('/'), + "a site path is repo-relative, or the fact varies by checkout \ + location: {}", + site.path + ); + assert!(site.line > 0, "a site line is 1-indexed"); + assert!( + site.lint.starts_with("clippy::") || site.lint.starts_with("rustc::"), + "a site names its lint and nothing else: {}", + site.lint + ); + } +} + +/// §7(d). FAIL-CLOSED, carried verbatim from `secrets.rs`: **clean is never +/// inferred from a stream that failed to parse.** +/// +/// Over the PARSER rather than over a real analyser, for `.claude/rules/rust.md`'s +/// reason and `secrets.rs`'s: the failing condition is a stream shape, and making +/// a real clippy emit a malformed diagnostic on demand is not something a fixture +/// can do. Extracting the decision is what makes it testable at all. +#[test] +fn an_unreadable_stream_is_could_not_look_and_never_an_empty_census() { + let root = Path::new("/repo"); + + // A clean run that genuinely found nothing: an empty site list, which is a + // STATEMENT and not a failure. + let Look::Is(none) = symbols::sites_in("", root) else { + panic!("an empty stream is an answer"); + }; + assert!(none.is_empty(), "nothing emitted means nothing found"); + + // Cargo's own non-JSON chatter is skipped, not refused — refusing it would + // make the fact unresolvable for a reason unrelated to the analyser. + let Look::Is(chatter) = symbols::sites_in(" Compiling batten v0.0.1\n", root) else { + panic!("cargo's progress is not a diagnostic"); + }; + assert!(chatter.is_empty()); + + // But a line that IS a compiler message and cannot be read as one is + // CouldNotLook. Skipping it would undercount SILENTLY, which is the direction + // that reports a clean tree from a stream that failed to parse. + for malformed in [ + // a compiler-message with no `message` object at all + r#"{"reason":"compiler-message"}"#, + // a lint code with no spans array + r#"{"reason":"compiler-message","message":{"code":{"code":"clippy::disallowed_types"}}}"#, + // a primary span missing its line + r#"{"reason":"compiler-message","message":{"code":{"code":"clippy::disallowed_types"},"spans":[{"is_primary":true,"file_name":"a.rs"}]}}"#, + ] { + assert!( + matches!(symbols::sites_in(malformed, root), Look::CouldNotLook), + "a diagnostic this build cannot read is could-not-look, never an \ + empty census: {malformed}" + ); + } +} + +/// §5 again, at the parse boundary: an absolute path from the analyser is made +/// repo-relative, or the fact would vary by checkout location and §6 +/// byte-stability could not hold. +#[test] +fn an_analyser_path_is_made_relative_to_the_repository() { + let stream = r#"{"reason":"compiler-message","message":{"code":{"code":"clippy::disallowed_types"},"spans":[{"is_primary":true,"file_name":"/repo/crates/batten/src/exec.rs","line_start":12}]}}"#; + let Look::Is(sites) = symbols::sites_in(stream, Path::new("/repo")) else { + panic!("the stream parses"); + }; + assert_eq!(sites.len(), 1); + assert_eq!(sites[0].path, "crates/batten/src/exec.rs"); + assert_eq!(sites[0].line, 12); +} diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 43103ca91..f1be8132f 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -108,7 +108,7 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "batten" -version = "0.0.115" +version = "0.0.116" dependencies = [ "anyhow", "clap", @@ -119,9 +119,11 @@ dependencies = [ "fs4", "getrandom", "gix", + "gix-diff", "globset", "hmac", "ignore", + "imara-diff", "json5", "proc-macro2", "regex", @@ -569,6 +571,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "foldhash" version = "0.2.0" @@ -1279,6 +1287,15 @@ dependencies = [ "byteorder", ] +[[package]] +name = "hashbrown" +version = "0.15.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" +dependencies = [ + "foldhash 0.1.5", +] + [[package]] name = "hashbrown" version = "0.16.1" @@ -1287,7 +1304,7 @@ checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" dependencies = [ "allocator-api2", "equivalent", - "foldhash", + "foldhash 0.2.0", ] [[package]] @@ -1298,7 +1315,7 @@ checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" dependencies = [ "allocator-api2", "equivalent", - "foldhash", + "foldhash 0.2.0", ] [[package]] @@ -1360,6 +1377,16 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "imara-diff" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f01d462f766df78ab820dd06f5eb700233c51f0f4c2e846520eaf4ba6aa5c5c" +dependencies = [ + "hashbrown 0.15.5", + "memchr", +] + [[package]] name = "indexmap" version = "2.14.0" diff --git a/man/batten-config-deprecations.1 b/man/batten-config-deprecations.1 new file mode 100644 index 000000000..b1523d77c --- /dev/null +++ b/man/batten-config-deprecations.1 @@ -0,0 +1,19 @@ +.ie \n(.g .ds Aq \(aq +.el .ds Aq ' +.TH batten-config-deprecations 1 batten +.SH NAME +batten\-config\-deprecations \- Report schema keys removed since a published release with no deprecation window +.SH SYNOPSIS +\fBbatten config deprecations\fR [\fB\-J\fR|\fB\-\-json\fR] [\fB\-h\fR|\fB\-\-help\fR] <\fIagainst\fR> +.SH DESCRIPTION +Report schema keys removed since a published release with no deprecation window +.SH OPTIONS +.TP +\fB\-J\fR, \fB\-\-json\fR +Emit byte\-stable JSON instead of pointer lines +.TP +\fB\-h\fR, \fB\-\-help\fR +Print help +.TP +<\fIagainst\fR> +The git ref whose published schema is the baseline (e.g. v0.0.111) diff --git a/man/batten-config.1 b/man/batten-config.1 index 1626408ea..7e9bc2fea 100644 --- a/man/batten-config.1 +++ b/man/batten-config.1 @@ -19,6 +19,9 @@ Print the effective configuration batten\-config\-epoch(1) Print the content hash of the governing config surface .TP +batten\-config\-deprecations(1) +Report schema keys removed since a published release with no deprecation window +.TP batten\-config\-lint(1) Report policy smells in batten.toml (any smell is a violation) .TP diff --git a/mise-tasks/config-deprecations.sh b/mise-tasks/config-deprecations.sh new file mode 100755 index 000000000..8cf5b6e30 --- /dev/null +++ b/mise-tasks/config-deprecations.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +#MISE description="Gate: no config key left the published schema without a deprecation window (CLOUD-360)" +# +# stays-bash: CLOUD-910 this file resolves WHICH ref the published schema is read +# at -- the latest release tag in version order -- and hands it to the engine. +# The predicate is already `batten config deprecations`, so what stays here is a +# tag-ordering question, and no rule kind expresses one: `ratchet`'s `base` names +# a single ref literally, and a `command` row would spawn the same shell one layer +# down. It grows the surface by one and is declared rather than hidden; CLOUD-910 +# is the row that retires it along with the rest of the `mise-tasks/` census. +# +# The contract half of `expand -> migrate -> contract`. A key vanishing from the +# published schema is a silent break for every consumer whose `batten.toml` still +# carries it: their config stops loading, with an unknown-key error that names no +# successor and no date. The grammar's promise is that removal is always preceded +# by a window, and this is what holds it. +# +# THE PREDICATE IS THE BINARY'S, not this script's. `batten config deprecations` +# reads the schema published at a ref, derives the current one, and compares the +# top-level key sets against `DEPRECATED_KEYS` and `RETIRED_KEYS`. This file +# resolves WHICH ref and nothing else — a shell re-derivation would be a second +# answer to a question the engine already answers, which is the defect the +# `git.rs` migration spent four slices removing. +# +# WHY A TAG AND NOT `origin/main`. The promise is made to a consumer who INSTALLED +# a release, so the baseline is the last released surface. Comparing against +# `main` would let a key be added and removed between releases and count as a +# break, which it is not — nobody could have configured it. +# +# REPLAY EVIDENCE, run before this was given deny severity (§7, 2026-08-25). +# `config deprecations` was run against all 112 release tags on this tree: +# +# exit 0 85 tags — no unannounced removal +# exit 2 0 tags — the gate would never have fired against a past release +# exit 3 27 tags — v0.0.26 and older, which predate the committed schema +# +# Zero exit 2 over 85 comparable releases is what justifies `deny` here: a +# predicate that would have refused past releases is one that fires on work +# nobody can now fix. The exit-3 cluster is the could-not-look path answering +# honestly rather than passing, and it is bounded — v0.0.27 is the oldest tag +# carrying `schema/batten.schema.json`, and this gate always asks the LATEST tag. +# A gate listed in $MUTANT_GATES with no row here fails `mise run mutant`. +#MUTANT unannounced-removal-passes|s/^exit "\$code"/exit 0/|an unannounced removal is reported rather than passed + +set -euo pipefail + +cd "${SCHEMA_ROOT:-$(git rev-parse --show-toplevel)}" + +# The latest release tag by version order, never by creation date: a re-cut tag +# would otherwise reorder the baseline. +if ! baseline=$(git tag --list 'v*' --sort=-v:refname | head -n 1) || [[ -z "$baseline" ]]; then + # NO TAGS IS COULD-NOT-LOOK, exit 3, never a pass. A fresh clone with no tags + # fetched has no baseline, and reporting "nothing was removed" having compared + # nothing is the vacuous pass CLOUD-251 names. + echo "::error:: config-deprecations: no release tag to compare against; fetch tags" >&2 + exit 3 +fi + +set +e +cargo run --quiet -p batten -- config deprecations "$baseline" +code=$? +set -e + +# The engine's own contract, read rather than reinterpreted: 0 clean, 2 the +# verdict, 3 could not look. Anything else is this script's bug and is surfaced +# as one rather than folded into a pass. +case "$code" in +0) echo "config-deprecations: no key left the schema unannounced since $baseline" ;; +2) echo "::error:: config-deprecations: a key left the published schema with no deprecation window; add a row to config::DEPRECATED_KEYS naming its replacement and expiry" >&2 ;; +3) echo "::error:: config-deprecations: no published schema at $baseline, so no removal could be judged" >&2 ;; +*) echo "::error:: config-deprecations: unexpected exit $code from the engine" >&2 ;; +esac + +# ONE exit, carrying the engine's own code through unchanged. Two guarded exits +# stood here first and the mutation runner caught them: neutering the `-eq 2` +# branch changed nothing, because the fallthrough exited 2 as well. Redundant +# branches mean neither is load-bearing, and a gate whose refusal has two +# independent causes cannot be shown to depend on either. +exit "$code" diff --git a/mise.toml b/mise.toml index 59a3450ec..b573c050a 100644 --- a/mise.toml +++ b/mise.toml @@ -398,7 +398,7 @@ CI_FANIN_WORKFLOW = ".github/workflows/ci.yml" # which is a property of the world and belongs on a clock (`lock-complete`). REGORUS_OPA_COMPLIANCE = "1.2.0" REGORUS_OPA_COMPLIANCE_FOR = "0.11" -MUTANT_GATES = "alive,ci-slow-needed,bot-issue,land,land-lock,ci-lease-precondition,board-diff-overlap,reclaim-census,connector-allow-resolve,serena-mcp,target-prune,claimed-keys,released,in-progress-drain,merged-pr-keys,board-payloads,attestation-check,awk-regex-check,batten-glob-check,board-sweep,board-write-record,branch-age-check,cap-drift,checks-green,ci-drift,ci-local-parity,ci-tools-check,claim-check,claim-race-check,closing-key-check,prose-only-check,coderabbit-config-check,config-lint,connector-allow-guard,connector-verb-guard,container-preflight,darwin-link,deferral-check,derived-check,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,evaluator-closure-check,evaluator-io-check,fanout-guard,filed-here-check,finding-sink-check,gh-guard,graph-check,hook-matcher-check,hook-pin-check,hook-profile-check,hooks-wiring-check,install-check,land-divergence-assert,land-lock-check,landed-check,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,memories-check,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutant,mutant-census,no-doctests,nonverdict-assert,ntia-check,perf-assert,perf-compare,perf-gate,pipefail-grep-check,privileged-lane,pr-unsubscribed,publish-credential-check,ready-cites-check,ready-guard,ready-lint,reference-check,release-assets-check,release-due,release-tracking-check,renovate-config-validator,report-only-check,rules-drift,run-shape,run-shape-guard,rust-paths-check,sbom,sbom-check,schema-check,semver,signing-posture,skill-check,sonar-gate,spec-ref-check,stop-guard,stop-posture-check,suite-bench-check,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,unlanded-check,verified" +MUTANT_GATES = "alive,ci-slow-needed,bot-issue,land,land-lock,ci-lease-precondition,board-diff-overlap,reclaim-census,connector-allow-resolve,serena-mcp,target-prune,claimed-keys,released,in-progress-drain,merged-pr-keys,board-payloads,attestation-check,awk-regex-check,batten-glob-check,board-sweep,board-write-record,branch-age-check,cap-drift,checks-green,ci-drift,ci-local-parity,ci-tools-check,claim-check,claim-race-check,closing-key-check,prose-only-check,coderabbit-config-check,config-deprecations,config-lint,connector-allow-guard,connector-verb-guard,container-preflight,darwin-link,deferral-check,derived-check,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,evaluator-closure-check,evaluator-io-check,fanout-guard,filed-here-check,finding-sink-check,gh-guard,graph-check,hook-matcher-check,hook-pin-check,hook-profile-check,hooks-wiring-check,install-check,land-divergence-assert,land-lock-check,landed-check,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,memories-check,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutant,mutant-census,no-doctests,nonverdict-assert,ntia-check,perf-assert,perf-compare,perf-gate,pipefail-grep-check,privileged-lane,pr-unsubscribed,publish-credential-check,ready-cites-check,ready-guard,ready-lint,reference-check,release-assets-check,release-due,release-tracking-check,renovate-config-validator,report-only-check,rules-drift,run-shape,run-shape-guard,rust-paths-check,sbom,sbom-check,schema-check,semver,signing-posture,skill-check,sonar-gate,spec-ref-check,stop-guard,stop-posture-check,suite-bench-check,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,unlanded-check,verified" # --- 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. @@ -777,6 +777,22 @@ done description = "Run every test suite — Rust and shell" depends = ["test:cargo", "test:bats"] +# A NAMED subset of the suite, for observing one case red under one mutation +# (CLOUD-418). `test:cargo` cannot serve this: its body takes no `"$@"`, so a +# filter passed to it is silently DROPPED and the whole suite runs — and because +# nextest fail-fast then cancels scheduling, a case that never ran reads exactly +# like a case that passed. Measured this session: a `-E 'test(differential)'` +# that matched nothing summarised green. +# +# Deliberately NOT receipt-routed and deliberately NOT part of `test` or +# `verify`. A receipt keyed on the step's inputs would be minted by a run of ONE +# case and then answer for the whole suite; and a subset is never the evidence +# that a suite passed. `--no-fail-fast` is fixed here rather than left to the +# caller, because the whole point is to see every named case's own verdict. +[tasks."test:filter"] +description = "Run named Rust tests (nextest filter expression) — never a substitute for test:cargo" +run = 'cargo nextest run --workspace --no-fail-fast "$@"' + [tasks."test:cargo"] description = "Run the Rust workspace test suite" # Routed through the per-step receipt (CLOUD-424): same sources, same command, diff --git a/policy/module-layering.rego b/policy/module-layering.rego index 6958b2a0e..1004ec8ba 100644 --- a/policy/module-layering.rego +++ b/policy/module-layering.rego @@ -75,6 +75,10 @@ declared_modules := { # declared rather than excluded: it is a file in the judged set, and a # selector carve-out would be an exemption where a placement is honest. "brief", "main", "selfwrite", + # `patch` arrived with CLOUD-739 and this rule named it before a human did — + # the same property the three above record, working a second time. `symbols` + # arrived with CLOUD-760 and it worked a third. + "patch", "symbols", } # THE FORBIDDEN EDGES, each traceable to prose already in the tree. @@ -100,6 +104,19 @@ forbidden[from] contains to if { "trust": {"lint", "epoch"}, "lint": {"epoch"}, "store": {"findings", "journal"}, + # `patch -> git` would close a cycle, and the direction is prose the tree + # already carries rather than an architecture invented here: `patch.rs` + # opens by saying it computes the identity `git::landing` consumes, and + # `git.rs` names `crate::patch` as that identity's authority. A back-edge + # would make the identity depend on the module that asks it for one. + "patch": {"git"}, + # `symbols -> rules` is `patch -> git` again, one fact family over, and the + # prose is already in the tree: `symbols.rs` says acquisition is the + # CALLER's, and `rules::symbols_fact` is that caller. An acquisition module + # reaching back into the engine that decides when to acquire would make the + # `Cost::Effect` boundary a convention rather than a direction — and the + # whole point of the class is that a projection cannot reach the spawn. + "symbols": {"rules", "hook"}, } some to in targets } @@ -223,6 +240,41 @@ test_an_external_edge_is_never_a_layering_violation if { ) } +# The cycle CLOUD-739's module could close, in the direction that would close it. +test_the_identity_must_not_reach_its_caller if { + count(violation) == 1 with input as judging( + "crates/batten/src/patch.rs", + [internal("git", 12)], + ) +} + +# And the declared direction is the whole point: `git` reaching `patch` is the +# arrangement, not a violation. +test_the_caller_may_reach_the_identity if { + count(violation) == 0 with input as judging( + "crates/batten/src/git.rs", + [internal("patch", 12)], + ) +} + +# CLOUD-760's edge, and it is the `Cost::Effect` boundary stated as a direction: +# the acquisition module must not reach the engine that decides when to acquire. +test_the_effect_acquisition_must_not_reach_its_caller if { + count(violation) == 1 with input as judging( + "crates/batten/src/symbols.rs", + [internal("rules", 20)], + ) +} + +# The declared direction, again the arrangement rather than a violation: `rules` +# resolves the fact once at the boundary, so it is the module that reaches. +test_the_engine_may_reach_the_effect_acquisition if { + count(violation) == 0 with input as judging( + "crates/batten/src/rules.rs", + [internal("symbols", 20)], + ) +} + # The coverage half: a module the table never placed. test_an_unplaced_module_is_refused_rather_than_allowed if { some v in violation with input as judging( diff --git a/policy/spawn-adapters.rego b/policy/spawn-adapters.rego new file mode 100644 index 000000000..a736e7db6 --- /dev/null +++ b/policy/spawn-adapters.rego @@ -0,0 +1,217 @@ +# WHICH MODULES MAY SPAWN, decided by NAME RESOLUTION (CLOUD-760). +# +# This is the consumer that makes `Fact::Symbols` a fact rather than a facility. +# `.claude/rules/rust.md` states the rule it enforces -- a spawn is an inventory +# row, and the annotation beside it is where somebody wrote down whether it +# stays -- but the inventory has never had a gate over WHERE a spawn may appear. +# It has one now, and only this fact could carry it. +# +# WHY NO SCANNER CAN WRITE THIS RULE. `.claude/rules/scanning.md` records the +# three answers to "where is `std::process::Command`": a byte scan says 14, a +# tree-sitter matcher says 11, name resolution says 9. The spread is one import: +# `surface.rs` writes `use clap::{..., Command}`, so the token names a DIFFERENT +# TYPE there, and a call expression looks identical whichever type it names. A +# gate built on either scanner would report `surface.rs` as an unplaced spawning +# module, and the honest remedies for that false positive are all worse than the +# rule -- an exemption for a module that spawns nothing, or a deleted rule. +# +# So the input here is the resolved census, which excludes `surface.rs` because +# the compiler knows what the name means. The measurement is in +# `crates/batten/tests/symbols.rs`, which asserts that exclusion as a SET rather +# than as a count: on this tree the byte and resolved tiers both total 16 and +# disagree about which files they name, so a count comparison would have passed +# while the tiers agreed about nothing. +# +# THE TABLE IS A PLACEMENT, NOT AN ALLOW-LIST OF SITES. It says which modules own +# a delegated tool, each with the tool it delegates to. It deliberately does not +# bound how many spawns a placed module holds: that is the `#[expect(...)]` +# inventory's job, self-cleaning in both directions, and duplicating it here +# would be a second authority that drifts. +# +# COULD-NOT-LOOK IS NOT CLEAN (CLOUD-251). `input.tree.symbols` is `null` when no +# row declared the fact and when the analyser could not be run or parsed, and +# either way this module must not report a tree with no unplaced spawns. It +# refuses instead, which is the same posture `symbols.rs` carries from +# `secrets.rs`: clean is never inferred from a stream that failed to parse. +# +# NO BATS SUITE, for the reason its two siblings carry: `batten policy test` is +# wired to no task, so `mutant` cannot reach a policy module's own cases. +# WHAT STANDS IN FOR IT: the acceptance clause was observed END TO END. A +# `std::process::Command::new("true")` was seeded into `crates/batten/src/git.rs` +# -- a module the table does not place, and the one CLOUD-739/740 spent the +# campaign emptying of spawns -- `batten check` reported `spawn-adapters`, and the +# findings went away on revert. Clean tree ZERO, seeded tree TWO: the seed's +# signature and its call each resolve the type, which is the resolved tier +# counting a USE rather than an occurrence of `::new`, exactly as the fact's own +# suite records for `exec.rs`. +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its cases cannot be reached by the mutation runner + +# METADATA +# description: | +# Bound to the TREE surface: this row is `scope = "tree"`, so it reads the tree +# document and never the mediated `{call, facts}` shape. +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.spawn_adapters + +import rego.v1 + +rules contains "spawn-adapters" + +# The placed adapters, each named with what it delegates to. Measured against the +# tree rather than imagined: this is exactly the resolved set on the commit that +# introduced the rule, so the gate starts true and every later row is a decision +# somebody made. +# +# A SET, and deliberately not a name -> reason map, which is what this table was +# first written as. `descend` in `policy.rs` walks every object member looking for +# a `rules` rule, and one of the placements below IS the `rules` module — so the +# map's own key shadowed the bundle's published id and the engine refused the +# whole module with "answered `rules` with a shape that is not a set of ids". +# Measured here before the rule ever ran. The reasons live in the comment. +# +# exec the sanctioned child-process boundary +# provision installs the binaries the other adapters pin +# secrets the pinned ripsecrets adapter (CLOUD-59) +# symbols the pinned clippy adapter, this fact's own acquisition (CLOUD-760) +# judge the judge kind's delegated command +# handler the harness handler boundary +# action an action row RUNS a command; that is the kind's definition +# rules the engine that runs a `command` row's `check` and `fix` +adapters := { + "exec", "provision", "secrets", "symbols", + "judge", "handler", "action", "rules", +} + +module_of(path) := name if { + parts := split(path, "/") + file := parts[count(parts) - 1] + name := substring(file, 0, count(file) - 3) +} + +# A spawn in a module the table has not placed. +# +# Pointer-only (non-negotiable rule 4): the path, the line and the module name. +# The analyser's diagnostic and the source line it quoted are not in the fact at +# all, so there is nothing here for this module to leak even by mistake. +violation contains { + "rule": "spawn-adapters", + "msg": sprintf( + "%s:%d %s resolves the spawn type and is not a placed adapter — route it through `exec`, or place the module with the tool it delegates to", + [site.path, site.line, module_of(site.path)], + ), +} if { + some site in input.tree.symbols.sites + site.lint == "clippy::disallowed_types" + not adapters[module_of(site.path)] +} + +# COULD NOT LOOK. `null` is both did-not-look answers, and neither is clean. +# +# OVER `sites`, NOT OVER THE FACT, and both halves of that were measured here. +# +# `not input.tree.symbols` alone is wrong: `not x` holds when `x` is undefined or +# false, and `null` is NEITHER -- so the shape the projection actually emits for +# could-not-look walked straight through it, caught by this module's own case +# rather than in the field. And the obvious repair, `== null`, does not type: the +# schema declares `["object", "null"]`, the checker narrows the ref to the object +# arm, and `opa check -s` calls the comparison a match error. +# +# Asking for `sites` answers both. A key that is absent, a `null`, or an object +# with no census all leave it undefined; a real census always carries it, and an +# EMPTY one carries `[]`, which is defined -- so "ran and found nothing" stays +# clean and stays distinct from "did not look". +no_census if not input.tree.symbols.sites + +violation contains { + "rule": "spawn-adapters", + "msg": "the symbol census is absent, so no spawn was placed or refused -- declare `symbols = true` on this row, or install the analyser", +} if { + no_census +} + +# THE VACUITY GUARD. A table placing nothing decides nothing, and a rule that +# cannot refuse is off. +violation contains { + "rule": "spawn-adapters", + "msg": "the adapter table places no module, so this rule decides nothing -- a gate that cannot refuse is off", +} if { + count(adapters) == 0 +} + +# The predicate's own tests. The ALLOW cases are the load-bearing half: a rule +# that fired on everything would satisfy every deny below and gate nothing. + +census(sites) := {"tree": {"symbols": { + "provenance": {"tool": "cargo", "version": "1.97.1", "invocation": ["clippy"]}, + "sites": sites, +}}} + +at(path, line) := {"path": path, "line": line, "lint": "clippy::disallowed_types"} + +# The case the rule exists for: a spawn in a module nobody placed. +test_a_spawn_in_an_unplaced_module_is_refused if { + some v in violation with input as census([at("crates/batten/src/git.rs", 12)]) + v.rule == "spawn-adapters" +} + +# And the placement is the point. `exec` is the sanctioned boundary; a spawn +# there is the arrangement, not a violation. +test_a_spawn_in_a_placed_adapter_is_clean if { + count(violation) == 0 with input as census([at("crates/batten/src/exec.rs", 88)]) +} + +# THE TABLE DOES NOT BOUND HOW MANY. A placed adapter holding several spawns is +# the `#[expect]` inventory's business, and duplicating that bound here would be +# a second authority that drifts from it. +test_a_placed_adapter_may_hold_more_than_one_spawn if { + count(violation) == 0 with input as census([ + at("crates/batten/src/exec.rs", 88), + at("crates/batten/src/exec.rs", 140), + at("crates/batten/src/secrets.rs", 31), + ]) +} + +# EVERY UNPLACED SITE IS ITS OWN FINDING, so a module with two of them is not +# reported once and half-fixed. +test_each_unplaced_site_is_reported if { + count(violation) == 2 with input as census([ + at("crates/batten/src/git.rs", 12), + at("crates/batten/src/git.rs", 30), + ]) +} + +# A DIFFERENT LINT IS NOT THIS RULE'S BUSINESS. The census carries whatever the +# analyser was asked for, and a rule reading every row would refuse on a lint it +# has no opinion about. +test_another_lints_site_is_not_a_spawn if { + count(violation) == 0 with input as {"tree": {"symbols": { + "provenance": {"tool": "cargo", "version": "1.97.1", "invocation": ["clippy"]}, + "sites": [{ + "path": "crates/batten/src/git.rs", + "line": 12, + "lint": "clippy::expect_used", + }], + }}} +} + +# AN ANALYSER THAT RAN AND FOUND NOTHING IS CLEAN, and it is the answer `null` +# must never be confused with. +test_an_empty_census_is_a_real_clean if { + count(violation) == 0 with input as census([]) +} + +# COULD NOT LOOK IS NOT CLEAN (CLOUD-251). `null` is both did-not-look answers -- +# no row declared the fact, or the analyser could not be run -- and neither is a +# tree with no unplaced spawns. +test_an_absent_census_refuses_rather_than_passing if { + some v in violation with input as {"tree": {"symbols": null}} + v.rule == "spawn-adapters" +} + +# The same answer when the key is missing altogether, which is what a row that +# forgot `symbols = true` produces. +test_an_undeclared_census_refuses_too if { + count(violation) == 1 with input as {"tree": {}} +} diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index 6762b8305..8c5569a6d 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -882,6 +882,10 @@ "type": "string" } }, + "symbols": { + "description": "Whether this policy row reads the **resolved-symbol** fact (CLOUD-760).\n\nA bare flag rather than a path list, because the fact is one whole-crate\nvalue: a delegated analyser resolves names across the compilation, and\nasking it about one file would be asking a different, cheaper question\nthat [`Rule::invocations`] already answers.\n\n**Declared rather than ambient, and here the reason is the cost class.**\nThis is the first `Cost::Effect` fact — resolving it RUNS `cargo clippy`\nover the crate, which is seconds rather than the milliseconds every other\nfact costs. Every git fact is declared for a bill CLOUD-851 measured at\n2.103x; this one would be far worse, and a run that paid it without being\nasked would make `check` unusable.", + "type": "boolean" + }, "tier": { "description": "How fast a [`RuleKind::Judge`] finding must be answered\n([`crate::severity::AdvisoryTier`], CLOUD-80). Absent means\n[`AdvisoryTier::Advisory`], the least-urgent rank.\n\nThis is the axis a judge row declares **instead of** `severity`, and the\nsubstitution is the whole advisory bound: `severity` decides the exit\ncontract, `tier` decides a response deadline. A judge row is refused the\n`severity` column outright, so the axis a model's opinion could ride into\nthe exit code does not exist for this kind.\n\nA default rather than a required column, unlike `severity` on every other\nkind: an omitted deadline resolves to the weakest one, which withholds no\ngate because there is no gate here to withhold.", "anyOf": [ diff --git a/schema/batten.schema.json b/schema/batten.schema.json index 629b5d0c3..d9d7cd9a3 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -1977,6 +1977,10 @@ "type": "string" } }, + "symbols": { + "description": "Whether this policy row reads the **resolved-symbol** fact (CLOUD-760).\n\nA bare flag rather than a path list, because the fact is one whole-crate\nvalue: a delegated analyser resolves names across the compilation, and\nasking it about one file would be asking a different, cheaper question\nthat [`Rule::invocations`] already answers.\n\n**Declared rather than ambient, and here the reason is the cost class.**\nThis is the first `Cost::Effect` fact — resolving it RUNS `cargo clippy`\nover the crate, which is seconds rather than the milliseconds every other\nfact costs. Every git fact is declared for a bill CLOUD-851 measured at\n2.103x; this one would be far worse, and a run that paid it without being\nasked would make `check` unusable.", + "type": "boolean" + }, "tier": { "description": "How fast a [`RuleKind::Judge`] finding must be answered\n([`crate::severity::AdvisoryTier`], CLOUD-80). Absent means\n[`AdvisoryTier::Advisory`], the least-urgent rank.\n\nThis is the axis a judge row declares **instead of** `severity`, and the\nsubstitution is the whole advisory bound: `severity` decides the exit\ncontract, `tier` decides a response deadline. A judge row is refused the\n`severity` column outright, so the axis a model's opinion could ride into\nthe exit code does not exist for this kind.\n\nA default rather than a required column, unlike `severity` on every other\nkind: an omitted deadline resolves to the weakest one, which withholds no\ngate because there is no gate here to withhold.", "anyOf": [ diff --git a/schema/policy-input.schema.json b/schema/policy-input.schema.json index 5e1e70e29..98529ac06 100644 --- a/schema/policy-input.schema.json +++ b/schema/policy-input.schema.json @@ -184,6 +184,52 @@ "description": "Fact::Produced. Sink key -> the record an earlier run's boundary wrote: a digest and a count for a baseline, the empty string for a marker. Never content -- non-negotiable rule 4 holds at the sink harder than at a report (CLOUD-851).", "type": "object" }, + "symbols": { + "additionalProperties": false, + "description": "Fact::Symbols (CLOUD-760). The first Cost::Effect fact: where a delegated analyser resolved a named type, by NAME rather than by spelling. `provenance` records which tool at which version produced it, because a fact whose meaning depends on an unrecorded tool version is not canonical. `sites` is pointer-only -- a path, a line and the lint that fired, never the diagnostic's message or the source it quoted.", + "properties": { + "provenance": { + "additionalProperties": false, + "properties": { + "invocation": { + "items": { + "type": "string" + }, + "type": "array" + }, + "tool": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "type": "object" + }, + "sites": { + "items": { + "additionalProperties": false, + "properties": { + "line": { + "type": "integer" + }, + "lint": { + "type": "string" + }, + "path": { + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + } + }, + "type": [ + "object", + "null" + ] + }, "tracked": { "description": "Fact::Tracked. Repository-relative paths the working-tree walk yields -- paths, never content.", "items": { diff --git a/tests/config-deprecations.bats b/tests/config-deprecations.bats new file mode 100644 index 000000000..1b31b97c4 --- /dev/null +++ b/tests/config-deprecations.bats @@ -0,0 +1,135 @@ +#!/usr/bin/env bats +# subject: mise-tasks/config-deprecations.sh +# The contract half of the config deprecation grammar (CLOUD-360): did a key +# leave the published schema without a window? +# +# Fixtures follow `schema-check.bats`' shape for the same reason — the gate runs +# `cargo run`, so a fixture needs a real workspace rather than a bare directory. +# Each is a scratch root symlinking the manifest and sources and holding its OWN +# git history, because what this gate reads is a blob at a TAG: the baseline is a +# published release, so a fixture has to be able to publish one. + +setup() { + CHECK="$BATS_TEST_DIRNAME/../mise-tasks/config-deprecations.sh" + REPO="$(cd "$BATS_TEST_DIRNAME/.." && pwd)" + ROOT="$BATS_TEST_TMPDIR/repo" + mkdir -p "$ROOT" + for entry in Cargo.toml Cargo.lock crates rustfmt.toml; do + ln -s "$REPO/$entry" "$ROOT/$entry" + done + cp -R "$REPO/schema" "$ROOT/schema" + export SCHEMA_ROOT="$ROOT" + export CARGO_TARGET_DIR="$REPO/target" + + # A real repository with a real tag, hermetically: the gate resolves its + # baseline with `git tag` and reads the blob at it, so neither can be faked + # with a plain directory. + git -C "$ROOT" init -q + git -C "$ROOT" config user.email t@t + git -C "$ROOT" config user.name t + git -C "$ROOT" add -A + git -C "$ROOT" -c commit.gpgsign=false commit -qm "seed" + git -C "$ROOT" tag v0.0.1 +} + +@test "a schema that lost no key exits 0" { + run "$CHECK" + [ "$status" -eq 0 ] + [[ "$output" == *"no key left the schema unannounced"* ]] +} + +# The gate's whole reason for existing. A key present at the released tag and +# absent now, with neither table naming it, is a silent break for every consumer +# still carrying it. +@test "an unannounced removal is reported rather than passed" { + # Remove a key from the BASELINE's side by publishing a schema that declares + # one this build does not. Equivalent to the real shape — a key that was + # published and is now gone — and reachable without editing the config types. + python3 - "$ROOT/schema/batten.schema.json" <<-'PY' + import json, sys + path = sys.argv[1] + doc = json.load(open(path)) + doc["properties"]["a_key_that_was_published_and_is_now_gone"] = {"type": "string"} + json.dump(doc, open(path, "w"), indent=2) + PY + git -C "$ROOT" add -A + git -C "$ROOT" -c commit.gpgsign=false commit -qm "publish an extra key" + git -C "$ROOT" tag v0.0.2 + # Put the working tree's schema back to what the types actually derive, so the + # key exists only at the tag — which is exactly "it was released and then + # removed". + cp "$REPO/schema/batten.schema.json" "$ROOT/schema/batten.schema.json" + + run "$CHECK" + [ "$status" -eq 2 ] + [[ "$output" == *"a_key_that_was_published_and_is_now_gone"* ]] + [[ "$output" == *"no deprecation window"* ]] +} + +# COULD NOT LOOK is exit 3, never 0. Reporting "nothing was removed" having +# compared nothing is the vacuous pass CLOUD-251 names, and it is the one answer +# this gate must never give. +@test "no release tag is exit 3 rather than a clean pass" { + git -C "$ROOT" tag -d v0.0.1 + run "$CHECK" + [ "$status" -eq 3 ] + [[ "$output" == *"no release tag"* ]] +} + +@test "a tag carrying no published schema is exit 3 rather than a clean pass" { + git -C "$ROOT" rm -q --cached schema/batten.schema.json + rm -f "$ROOT/schema/batten.schema.json" + git -C "$ROOT" -c commit.gpgsign=false commit -qm "no schema here" + git -C "$ROOT" tag v0.0.3 + # Restore the working tree's copy: the baseline is what lacks it. + cp "$REPO/schema/batten.schema.json" "$ROOT/schema/batten.schema.json" + run "$CHECK" + [ "$status" -eq 3 ] + [[ "$output" == *"no published schema"* ]] +} + +@test "the baseline is the newest tag by version order, not by creation time" { + # A re-cut or back-dated tag must not reorder the baseline: v0.0.10 is newer + # than v0.0.9 even when created first. + git -C "$ROOT" tag v0.0.10 + git -C "$ROOT" tag v0.0.9 + run "$CHECK" + [ "$status" -eq 0 ] + [[ "$output" == *"v0.0.10"* ]] +} + +@test "output is pointer-only — no schema body echoed" { + # rule 4: the remedy is declaring a window, so the schema body adds nothing + # and would put the config surface into the log. + python3 - "$ROOT/schema/batten.schema.json" <<-'PY' + import json, sys + path = sys.argv[1] + doc = json.load(open(path)) + doc["properties"]["gone_key"] = {"description": "AVeryDistinctiveInventedSentence"} + json.dump(doc, open(path, "w"), indent=2) + PY + git -C "$ROOT" add -A + git -C "$ROOT" -c commit.gpgsign=false commit -qm "publish" + git -C "$ROOT" tag v0.0.4 + cp "$REPO/schema/batten.schema.json" "$ROOT/schema/batten.schema.json" + run "$CHECK" + [ "$status" -eq 2 ] + [[ "$output" == *"gone_key"* ]] + [[ "$output" != *"AVeryDistinctiveInventedSentence"* ]] +} + +@test "the gate leaves the tree it judges unmodified" { + # A gate that rewrites what it judges cannot fail twice. + before="$(cat "$ROOT/schema/batten.schema.json")" + run "$CHECK" + [ "$status" -eq 0 ] + [ "$(cat "$ROOT/schema/batten.schema.json")" = "$before" ] +} + +@test "this repo's own schema has lost no key since its last release — the gate on the real tree" { + # The self-consumption case, and the one the replay evidence generalises: + # across all 112 tags this predicate reported zero violations. + unset SCHEMA_ROOT + run "$CHECK" + [ "$status" -eq 0 ] +}