diff --git a/batten.toml b/batten.toml index 07bd4b3e2..496ee4e1e 100644 --- a/batten.toml +++ b/batten.toml @@ -430,6 +430,123 @@ retry — or just `mise run land`, which drives the whole loop. A red run on a \ freshly-readied PR means that step was skipped, and the CI minutes are already \ spent by then.""" +# CLOUD-859. The read-the-review gate, and this repository's FIRST `[[fact]]` +# row — the agent-sourced channel has had no consumer since CLOUD-776 landed it. +# +# THE COMMAND IS THE PREDICATE'S SELECTION, not merely its source. `Sourced` +# stores `{command, seen_at, rows}` and no byte of the buffer (non-negotiable +# rule 4), so a module can ask how many rows came back and nothing else. The +# `--jq` projection therefore emits ONE ELEMENT PER BLOCKING CONDITION — each +# unresolved review thread's id, plus the PR author's login when nothing but the +# author has reviewed — which is what makes `rows == 0` mean "reviewed and +# addressed" rather than "nobody looked". +# +# WHY IT PROJECTS TO AN ARRAY, on grounds that outlived the parsing reason. +# CLOUD-992 taught `rows_in` to read a text buffer, so a bare object no longer +# fails to parse — it normalises to `Is(1)`, always one row whatever the query +# found. The `[...]` is semantic now: without it the count means nothing. Do not +# "simplify" it away on the grounds that the parse exists. +# +# WHY `node(id:)` RATHER THAN `repository(owner:,name:)`. `Declared.command` is +# compared byte-for-byte against what the agent ran — that byte-equality IS the +# forgery control — so the string is a CONSTANT and cannot be parameterised per +# pull request. `gh pr view --json id` derives the PR's node id from the current +# branch, so the whole query is constant and still per-PR. It also keeps the +# owner and repository name out of the string, which `repository(owner:...)` +# would have hard-coded. +# +# WHY NOT `gh pr view` ALONE: checked against `gh` 2.97.0, its field list carries +# `reviews`, `latestReviews` and `reviewDecision` and has NO `reviewThreads`. +# Threads are the half this decides on, so `gh pr view` cannot answer it. +# +# WHY NOT A `mise run` PRODUCER, which is shorter and would have been nicer: +# measured, and it is unusable. `capture::decode_response` concatenates `stdout` +# then `stderr`, so mise's `[task] $ …` prefix lands in the buffer either way, +# the text stops parsing as JSON, and the count is one opaque row forever — a +# gate refusing every `gh pr ready` in the repository and unsatisfiable by the +# command its own deny prints. Probed side by side: the bare command recorded +# `rows 3` where `mise run envelope-probe` over the identical body recorded +# `rows 1`. The residue is broader than mise and is CLOUD-993's inventory: any +# declared command routed through something that annotates its own output is +# disqualified by the same mechanism. +# +# `returns = "json-array"` is what keeps that from being silent (CLOUD-993). A +# command that stops emitting an array — an auth error, a changed default format, +# a wrapper that annotates — records NOTHING and denies, instead of recording a +# plausible `rows 1` that no `rows == 0` predicate can ever clear. +# +# AN UNREAD PAGE IS A BLOCKING CONDITION, NOT A ZERO. GitHub caps a connection +# page at 100, so a PR with more than 100 review threads would have the surplus +# fall outside the query — and an unresolved thread out there would leave +# `rows == 0`, which is a FALSE GREEN in the one direction this gate exists to +# prevent. Both connections therefore carry `pageInfo{hasNextPage}` and the +# projection emits an extra element when either is true, so "I could not see all +# of it" refuses instead of passing. +# +# Chosen over paginating, deliberately: `Declared.command` is byte-compared, so +# it is one string with no loop in it, and `--paginate` emits one JSON document +# per page — which stops being a single array and lands back in the +# could-not-look arm. Counting a truncation as blocking gets the safe verdict +# with no iteration at all. `true` rather than a quoted marker keeps the whole +# command free of double quotes, which is what lets it be a plain TOML string +# with no escaping — one fewer way to retype a CLOUD-990-shaped remedy wrong. +# +# WHAT THIS FACT DOES NOT BIND, stated because the gate must not over-claim: +# **the record is not tied to the head it was taken on.** `facts::sourced_path` +# keys an agent-sourced record by the fact's NAME only, and its own doc says why +# — a `claimed-key` answer is about an issue row and is deliberately shared +# across branches. So `key = "head"` below is accepted and unread on this path, +# and it is measured rather than inferred: a record minted clear on one head +# still allowed `gh pr ready` on the next commit with no new record. The gate +# therefore binds once per branch rather than once per head, which is strictly +# better than today's nothing and weaker than the row's Acceptance. Fixing it +# changes the channel for every fact and contradicts a documented decision, so +# it is CLOUD-1064's rather than smuggled in here. +[[fact]] +name = "review-answered" +returns = "json-array" +command = "gh api graphql -F id=$(gh pr view --json id --jq .id) -f query='query($id:ID!){node(id:$id){... on PullRequest{author{login} reviews(first:100){pageInfo{hasNextPage} nodes{author{login}}} reviewThreads(first:100){pageInfo{hasNextPage} nodes{id isResolved}}}}}' --jq '.data.node as $p | [($p.reviewThreads.nodes[] | select(.isResolved == false) | .id), (if ([$p.reviews.nodes[] | select(.author.login != $p.author.login)] | length) == 0 then $p.author.login else empty end), (if $p.reviewThreads.pageInfo.hasNextPage then true else empty end), (if $p.reviews.pageInfo.hasNextPage then true else empty end)]'" + +# The DID-YOU-LOOK half. A `receipt` row is also what makes the fact reach Rego +# at all: `required_checks_for` reads the receipt rows, and `agent_records` +# populates `input.facts["agent-sourced"]` only for the checks they name — so +# without this row the policy module below reads `null` on every call and decides +# nothing. That coupling is not obvious and is why the two rows ship together. +# +# It carries the never-ran and command-mismatch refusals, and only it can: the +# boundary hands it the declaration, so its deny is built from the declared +# command rather than from prose. The module decides the COUNT and stays silent +# about absence, so one call never draws two refusals. +# +# `key = "head"` because an agent-sourced record is resolved from its own file +# rather than from the receipt store, so the keying is unread on this path — the +# column is required by `rules::validate` and `head` is the honest answer for a +# fact about a specific head. +[[rule]] +id = "ready-needs-an-answered-review" +kind = "receipt" +scope = "mediated_call" +severity = "deny" +pattern = "gh pr ready" +checks = ["review-answered"] +key = "head" +reason = """ +Read the review before buying a matrix. Run the declared command above — it \ +prints one line per blocking condition and nothing when the head is clear — \ +then retry. If no review exists yet, force one with `@coderabbitai full review` \ +(~3 minutes measured, and free: the draft phase runs no CI), answer what it \ +says, and run the command again. Replayed over the last 100 merges this fires \ +on 89, so the usual outcome is that there is something to read.""" + +# The DID-YOU-ANSWER-IT half. The module's own header carries the measurement, +# the count reformulation, and why the thread ids cannot be in the message. +[[rule]] +id = "review-answered" +kind = "policy" +scope = "mediated_call" +module = "policy/review-answered.rego" +severity = "deny" + # `claim-guard`, as config (CLOUD-444) — the other half of the pair # `ready-needs-receipts` starts, and the one the receipt kind could not express # until this change. diff --git a/bench/suites/RESULTS.md b/bench/suites/RESULTS.md index cfd822a93..d71c8e9b4 100644 --- a/bench/suites/RESULTS.md +++ b/bench/suites/RESULTS.md @@ -6,168 +6,169 @@ 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: 160 -- serial total: 1382.4s +- suites: 161 +- serial total: 1498.2s | seconds | share | suite | | ---: | ---: | --- | -| 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` | +| 141.9 | 9.5% | `tests/land-lock.bats` | +| 128.3 | 8.6% | `tests/derived-check.bats` | +| 118.0 | 7.9% | `tests/step-receipt.bats` | +| 116.1 | 7.7% | `tests/stop-guard.bats` | +| 102.3 | 6.8% | `tests/spec-ref-check.bats` | +| 102.3 | 6.8% | `tests/ci-wait.bats` | +| 95.5 | 6.4% | `tests/semver.bats` | +| 81.0 | 5.4% | `tests/land.bats` | +| 63.0 | 4.2% | `tests/hooks-wiring-check.bats` | +| 40.6 | 2.7% | `tests/ci-local-parity.bats` | +| 34.5 | 2.3% | `tests/main-watch.bats` | +| 28.3 | 1.9% | `tests/suite-bench-check.bats` | +| 27.9 | 1.9% | `tests/graph-check.bats` | +| 24.3 | 1.6% | `tests/hook-latency-drift.bats` | +| 18.3 | 1.2% | `tests/sbom-check.bats` | +| 18.1 | 1.2% | `tests/prebuilt-lint.bats` | +| 17.0 | 1.1% | `tests/claim-check.bats` | +| 16.2 | 1.1% | `tests/helpers.bats` | +| 14.5 | 1.0% | `tests/board-diff-overlap.bats` | +| 14.2 | 0.9% | `tests/board-write-record.bats` | +| 13.6 | 0.9% | `tests/filed-here-check.bats` | +| 11.5 | 0.8% | `tests/config-lint.bats` | +| 9.9 | 0.7% | `tests/suite-select.bats` | +| 9.7 | 0.6% | `tests/ready-lint.bats` | +| 8.5 | 0.6% | `tests/replay.bats` | +| 8.5 | 0.6% | `tests/run-shape-guard.bats` | +| 8.0 | 0.5% | `tests/ready-guard.bats` | +| 7.5 | 0.5% | `tests/lock-complete.bats` | +| 7.4 | 0.5% | `tests/target-race.bats` | +| 7.4 | 0.5% | `tests/token-bench.bats` | +| 7.3 | 0.5% | `tests/released.bats` | +| 7.1 | 0.5% | `tests/board-sweep.bats` | +| 7.1 | 0.5% | `tests/mutant.bats` | +| 6.3 | 0.4% | `tests/renovate-config-validator.bats` | +| 6.0 | 0.4% | `tests/release-tracking-check.bats` | +| 5.5 | 0.4% | `tests/release-assets-check.bats` | +| 5.4 | 0.4% | `tests/mcp-allow-check.bats` | +| 5.3 | 0.4% | `tests/session-start.bats` | +| 5.0 | 0.3% | `tests/sbom.bats` | +| 5.0 | 0.3% | `tests/singleton.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.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` | +| 4.1 | 0.3% | `tests/task-registry.bats` | +| 4.0 | 0.3% | `tests/schema-check.bats` | +| 3.8 | 0.3% | `tests/pre-commit-staging.bats` | +| 3.8 | 0.3% | `tests/reference-check.bats` | +| 3.8 | 0.3% | `tests/ntia-check.bats` | +| 3.8 | 0.3% | `tests/ready-cites-check.bats` | +| 3.6 | 0.2% | `tests/doctor-race.bats` | +| 3.6 | 0.2% | `tests/land-divergence.bats` | +| 3.6 | 0.2% | `tests/tree-clean.bats` | +| 3.6 | 0.2% | `tests/unlanded-check.bats` | +| 3.5 | 0.2% | `tests/fanout-guard.bats` | +| 3.4 | 0.2% | `tests/finding-sink-check.bats` | +| 3.2 | 0.2% | `tests/hk-selection.bats` | +| 2.8 | 0.2% | `tests/with-lock.bats` | +| 2.6 | 0.2% | `tests/target-ensure.bats` | +| 2.5 | 0.2% | `tests/landed-check.bats` | +| 2.1 | 0.1% | `tests/claim-race-check.bats` | +| 2.1 | 0.1% | `tests/config-deprecations.bats` | +| 2.1 | 0.1% | `tests/closing-key-check.bats` | +| 2.1 | 0.1% | `tests/rules-drift.bats` | +| 2.0 | 0.1% | `tests/done-check.bats` | +| 1.9 | 0.1% | `tests/ready-lint-deferral.bats` | +| 1.8 | 0.1% | `tests/alive.bats` | +| 1.8 | 0.1% | `tests/skill-check.bats` | +| 1.7 | 0.1% | `tests/ci-slow-needed.bats` | +| 1.7 | 0.1% | `tests/reclaim-census.bats` | +| 1.7 | 0.1% | `tests/signing-posture.bats` | +| 1.7 | 0.1% | `tests/memories-check.bats` | +| 1.7 | 0.1% | `tests/ci-tools-check.bats` | +| 1.6 | 0.1% | `tests/bot-issue.bats` | +| 1.5 | 0.1% | `tests/done-pr-check.bats` | +| 1.5 | 0.1% | `tests/claimed-keys.bats` | +| 1.5 | 0.1% | `tests/doctor.bats` | +| 1.5 | 0.1% | `tests/ci-lease-precondition.bats` | +| 1.4 | 0.1% | `tests/gh-guard.bats` | +| 1.4 | 0.1% | `tests/mutant-census.bats` | +| 1.3 | 0.1% | `tests/perf-record.bats` | +| 1.3 | 0.1% | `tests/awk-regex-check.bats` | +| 1.2 | 0.1% | `tests/duplicate-close-check.bats` | +| 1.2 | 0.1% | `tests/install-check.bats` | +| 1.2 | 0.1% | `tests/review-answered.bats` | +| 1.2 | 0.1% | `tests/verify.bats` | +| 1.2 | 0.1% | `tests/release-backfill.bats` | +| 1.2 | 0.1% | `tests/land-divergence-assert.bats` | +| 1.2 | 0.1% | `tests/deferral-check.bats` | +| 1.2 | 0.1% | `tests/run-shape.bats` | +| 1.2 | 0.1% | `tests/perf-assert.bats` | +| 1.1 | 0.1% | `tests/module-map-check.bats` | +| 1.0 | 0.1% | `tests/attestation-check.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/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` | +| 1.0 | 0.1% | `tests/spawn-census.bats` | +| 1.0 | 0.1% | `tests/hook-profile-check.bats` | +| 1.0 | 0.1% | `tests/nonverdict-scan.bats` | +| 0.9 | 0.1% | `tests/target-prune.bats` | +| 0.9 | 0.1% | `tests/evaluator-closure-check.bats` | +| 0.9 | 0.1% | `tests/checks-green.bats` | +| 0.9 | 0.1% | `tests/pr-unsubscribed.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/duplicate-close-check.bats` | -| 0.7 | 0.0% | `tests/hook-matcher-check.bats` | -| 0.7 | 0.0% | `tests/mcp-timeout-budget.bats` | +| 0.8 | 0.1% | `tests/install.bats` | +| 0.8 | 0.1% | `tests/sbom-binary.bats` | +| 0.8 | 0.1% | `tests/hook-pin-check.bats` | +| 0.8 | 0.1% | `tests/timeout-drift.bats` | +| 0.7 | 0.0% | `tests/prose-only-check.bats` | +| 0.7 | 0.0% | `tests/render-cli.bats` | +| 0.7 | 0.0% | `tests/perf-compare.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.7 | 0.0% | `tests/lint-rego.bats` | +| 0.6 | 0.0% | `tests/hook-matcher-check.bats` | +| 0.6 | 0.0% | `tests/mcp-timeout-budget.bats` | | 0.6 | 0.0% | `tests/checksums.bats` | +| 0.6 | 0.0% | `tests/mcp-attach-check.bats` | | 0.6 | 0.0% | `tests/connector-verb-guard.bats` | -| 0.5 | 0.0% | `tests/lint-rego.bats` | +| 0.5 | 0.0% | `tests/publish-credential-check.bats` | +| 0.5 | 0.0% | `tests/macos-link-check.bats` | +| 0.5 | 0.0% | `tests/pipefail-grep-check.bats` | +| 0.5 | 0.0% | `tests/branch-age-check.bats` | +| 0.5 | 0.0% | `tests/verified.bats` | | 0.5 | 0.0% | `tests/board-payloads.bats` | +| 0.5 | 0.0% | `tests/abandon-matrix.bats` | | 0.5 | 0.0% | `tests/msrv-pin-agreement.bats` | -| 0.4 | 0.0% | `tests/abandon-matrix.bats` | +| 0.4 | 0.0% | `tests/git-hook.bats` | | 0.4 | 0.0% | `tests/digest-major-agreement.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/release-due.bats` | +| 0.4 | 0.0% | `tests/stop-posture-check.bats` | +| 0.4 | 0.0% | `tests/release-due.bats` | +| 0.4 | 0.0% | `tests/timeout-check.bats` | +| 0.4 | 0.0% | `tests/report-only-check.bats` | +| 0.4 | 0.0% | `tests/sonar-gate.bats` | +| 0.4 | 0.0% | `tests/no-doctests.bats` | +| 0.4 | 0.0% | `tests/transcript-corpus-check.bats` | +| 0.4 | 0.0% | `tests/serena-mcp.bats` | +| 0.4 | 0.0% | `tests/run-shape-guard-quoting.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/nonverdict-assert.bats` | +| 0.3 | 0.0% | `tests/commit-attribution.bats` | | 0.3 | 0.0% | `tests/license-table-check.bats` | +| 0.3 | 0.0% | `tests/privileged-lane.bats` | +| 0.3 | 0.0% | `tests/batten-glob-check.bats` | | 0.3 | 0.0% | `tests/container-preflight.bats` | -| 0.3 | 0.0% | `tests/git-hook.bats` | +| 0.3 | 0.0% | `tests/commit-convention.bats` | | 0.3 | 0.0% | `tests/mise-pin-agreement.bats` | -| 0.3 | 0.0% | `tests/rust-paths-check.bats` | +| 0.3 | 0.0% | `tests/ci-drift.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/egress-check.bats` | +| 0.2 | 0.0% | `tests/rust-paths-check.bats` | | 0.2 | 0.0% | `tests/perf-gate.bats` | +| 0.2 | 0.0% | `tests/evaluator-io-check.bats` | +| 0.2 | 0.0% | `tests/token-bench-check.bats` | | 0.2 | 0.0% | `tests/remedy-payload-source.bats` | -| 0.2 | 0.0% | `tests/dist.bats` | -| 0.1 | 0.0% | `tests/evaluator-io-check.bats` | -| 0.1 | 0.0% | `tests/egress-check.bats` | +| 0.2 | 0.0% | `tests/test-bats-parallel.bats` | +| 0.1 | 0.0% | `tests/task-fail-closed.bats` | +| 0.1 | 0.0% | `tests/dist.bats` | | 0.1 | 0.0% | `tests/perf-pair.bats` | -| 0.1 | 0.0% | `tests/zizmor-split.bats` | -| 0.1 | 0.0% | `tests/darwin-link.bats` | | 0.1 | 0.0% | `tests/cross-check.bats` | +| 0.1 | 0.0% | `tests/darwin-link.bats` | +| 0.0 | 0.0% | `tests/zizmor-split.bats` | diff --git a/mise.toml b/mise.toml index b573c050a..f01786421 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-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" +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,review-answered,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. diff --git a/policy/review-answered.rego b/policy/review-answered.rego new file mode 100644 index 000000000..5ab13c40a --- /dev/null +++ b/policy/review-answered.rego @@ -0,0 +1,245 @@ +# The read-the-review gate (CLOUD-859), as a COUNT predicate over the one +# agent-sourced fact this repository declares. +# +# WHY IT IS WORTH REFUSING, measured rather than argued. Replayed over the 100 +# most recently merged pull requests (2026-08-19 to 2026-08-22) this predicate +# fires on 89 — 21 carrying no review from anyone but their own author, 68 +# carrying unresolved threads, 163 open threads across the fired set. Readying is +# the event that starts CI, and nothing in `land`'s pre-ready sequence asks about +# review, so an unreviewed head — or one carrying findings nobody answered — +# spends the matrix exactly as readily as a reviewed one. The worst instance is +# #617: reviewed at 15:39:44, merged twelve minutes later carrying three threads +# that are open to this day. Not an unreviewed merge, an unanswered one. +# +# THE COUNT IS THE PREDICATE, and that is a constraint rather than a shortcut. +# `facts::Sourced` stores `{command, seen_at, rows}` and no byte of the buffer — +# non-negotiable rule 4, structural rather than careful — so `reviews`, +# `latestReviews` and `reviewThreads` never reach Rego and a predicate over them +# is unwritable here. What reaches this module is HOW MANY rows the declared +# command's stdout carried. So the selection lives in the declared command's own +# `--jq` projection, one element per blocking condition, and `rows == 0` is +# exactly "reviewed and addressed". CLOUD-859's §2 specifies a predicate over the +# payload; it is not implementable on this channel, and the row records the +# reformulation rather than this file re-arguing it. +# +# WHICH IS ALSO WHY THE THREAD IDS ARE NOT IN THE MESSAGE. §5 asks for them. They +# are not in the engine, so the finding names the count and points at the command +# that produces the ids. A `msg` claiming to name them would be prose asserting a +# payload this channel refuses to carry, which is worse than the honest count. +# +# THE ABSENT RECORD IS NOT THIS MODULE'S, and the split is deliberate. +# `ready-needs-an-answered-review` is a `receipt` row over the same fact: a +# never-ran record, and one whose command does not match the declaration +# byte-for-byte, are `Validity::Missing` there — the deny that carries the +# `Fix::Run` asking for the command. Deciding it here as well would be two rows +# refusing one call with two messages, and only the receipt row's can name the +# command, because only it is handed the declaration. +# +# `--undo` IS NOT A READY. `land` re-drafts a PR on a red run, and that is the +# one thing that stops the next push buying another matrix. A predicate anchored +# on `gh pr ready` alone refuses the re-draft, which would leave the tap open on +# exactly the head this gate is trying to keep out of CI. +# METADATA +# description: | +# Bound to the mediated-call surface: this module is `scope = "mediated_call"`, +# so it reads `{call, facts}` and NOT the tree document. Binding it to the tree +# schema would type check it against a shape the engine never hands it, which is +# CLOUD-845's defect introduced on purpose rather than caught. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`. +# schemas: +# - input: schema["policy-call.schema"] +package batten.review_answered + +import rego.v1 + +rules contains "review-unanswered" + +# THE MUTATIONS, and each corrupts a conjunct no other conjunct already excludes +# — the discrimination `.claude/rules/policy-modules.md` records a survivor for, +# and the first draft of this block earned that warning twice. Field 3 is a bats +# `--filter` and must match a real case name: three descriptions that matched +# nothing all reported `case-already-red`, which is indistinguishable from a +# mutation nobody caught. `@` delimits each sed script because the rows are +# `|`-separated. +# +# THE THIRD ROW TOOK THREE ATTEMPTS AND THE FIRST TWO ARE THE LESSON. It began as +# `command-not-anchored`, naming a case whose command carries no `ready` at all, +# so the cheap conjunct excluded the input and the mutation could not be seen. It +# was renamed to `prose-judged` — correctly named, and it SURVIVED anyway, because +# the engine never hands this module a prose command with a record beside it, so +# no bats case can discriminate that conjunct at all. Both survivals were true +# statements about the module rather than bad filters. +# +# What finally made it discriminating was deleting the conjunct. `compound-not-judged` +# reinstates the anchor those rows were defending, and the anchor is exactly what +# opened the bypass below — so the mutation now corrupts something a case CAN see: +# `cd /repo && gh pr ready` goes from refused to allowed. A mutation that cannot +# be caught is a coverage claim that is not true; a mutation that reintroduces a +# measured defect is the opposite. +#MUTANT compound-not-judged|s@\tcontains(input.call.command, "gh pr ready")@\tstartswith(trim_space(input.call.command), "gh pr ready")@|a compound command is still a ready +#MUTANT count-not-decided|s@\trecord.rows > 0@\trecord.rows > 99999@|a head carrying unresolved threads is refused +#MUTANT redraft-judged|s@\tnot contains(input.call.command, "--undo")@\ttrue@|a re-draft is not a ready + +violation contains { + "rule": "review-unanswered", + "msg": sprintf( + "readying this PR would buy a CI matrix on a head carrying %d blocking review condition(s): an unresolved review thread, or no review from anyone but the author. Answer them first — resolve each thread, or force a review with `@coderabbitai full review`, which returns in ~3 minutes and costs no CI because the draft phase is the free phase — then re-run the declared command and retry. Replayed over the last 100 merges this fires on 89, so the common case is that there is something here to read. The thread ids are deliberately absent: the fact channel stores a COUNT and no byte of the buffer (non-negotiable rule 4), so the command that produced this number is also the only thing that can name them", + [record.rows], + ), +} if { + readying + record := input.facts["agent-sourced"]["review-answered"] + record.rows > 0 +} + +# The cheap term first, for `run-shape.rego`'s measured reason: everything else +# here is computed only if a `ready` appears in the command at all. +# +# `contains` RATHER THAN AN ANCHOR, and this reversed twice before it was right. +# +# The draft before this one used `startswith(trim_space(...))`, on the reasoning +# that an anchor keeps PROSE out — a commit message naming `gh pr ready`, which +# this repository writes constantly, and which `run-shape.rego`'s header records +# as a real failure for its own predicate. That anchor was wrong twice over, both +# measured against the real binary rather than argued: +# +# 1. **It bought almost nothing, and the "almost" is measured too.** With only +# `ready-needs-an-answered-review` declared, +# `git commit -m "run gh pr ready once ... answered"` is ALLOWED even under +# `contains`: a quoted span is scrubbed before the engine's `pattern` matcher +# sees it, so the receipt row does not select that commit, no record is +# projected, and this module decides nothing about it. +# +# A HEREDOC IS NOT SCRUBBED, and that is the residue. `git commit -F - <"$REPO/batten.toml" + + # No global or system config: a contributor's own git settings must not be + # able to change a verdict here (CLOUD-282). + GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null git init -q -b main "$REPO" +} + +# Mint the record the way a session does: a PostToolUse envelope carrying the +# declared command and the buffer the host handed back. +record() { # record + local envelope + envelope=$(python3 -c 'import json,sys; print(json.dumps({"hook_event_name":"PostToolUse","session_id":"sess-review","cwd":"/repo","tool_name":"Bash","tool_input":{"command":sys.argv[1]},"tool_response":{"stdout":sys.argv[2],"stderr":""}}))' "$DECLARED" "$1") + (cd "$REPO" && printf '%s' "$envelope" | "$BIN" hook --harness claude-code) +} + +# Read it: the call the gate exists to judge. +ready() { # ready [] + local envelope + envelope=$(python3 -c 'import json,sys; print(json.dumps({"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":sys.argv[1]}}))' "gh pr ready 702${1:+ $1}") + (cd "$REPO" && printf '%s' "$envelope" | "$BIN" hook --harness claude-code) +} + +# BOTH HELPERS ASSERT THE EXIT STATUS, for `tests/run-shape.bats`' measured +# reason: `batten hook` prints nothing on an allow and exits 0 either way, so a +# substring check over an empty string is true — including the empty output of a +# binary that died before it judged anything. +denied() { [ "$status" -eq 0 ] && [[ "$1" == *'"permissionDecision":"deny"'* ]]; } +allowed() { [ "$status" -eq 0 ] && [[ "$1" != *'"deny"'* ]]; } + +# --- the two refusals, and which row owns each ------------------------------ + +@test "a ready with no record at all is refused, and the receipt row names the command" { + # The did-you-look half. The deny is built from the DECLARED command rather + # than from prose, which is the property that makes the remedy runnable. + run ready + denied "$output" + [[ "$output" == *"gh api graphql"* ]] + [[ "$output" == *"reviewThreads"* ]] +} + +@test "THE MEASURED SHAPE: a head carrying unresolved threads is refused, naming the count" { + # #623's four open threads, as the projection emits them: one element per + # thread id. + run record '["PRRT_a","PRRT_b","PRRT_c","PRRT_d"]' + [ "$status" -eq 0 ] + run ready + denied "$output" + [[ "$output" == *"review-unanswered"* ]] + [[ "$output" == *"4 blocking"* ]] + # Pointer-only (non-negotiable rule 4): the ids are not in the engine, so a + # message naming one would be a payload this channel refuses to carry. + [[ "$output" != *"PRRT_a"* ]] +} + +@test "a head whose threads are all answered is allowed" { + # THE LOAD-BEARING HALF. A predicate that only ever denied would satisfy every + # case above and gate nothing (CLOUD-418). `[]` is the genuine zero: the + # command looked and found none. + run record '[]' + [ "$status" -eq 0 ] + run ready + allowed "$output" +} + +# --- the vacuity cases the row enumerates ----------------------------------- + +@test "VACUITY: zero threads and no review reads as unreviewed, not as all-addressed" { + # #618 carries no threads and no review. The projection emits the PR author's + # login when nothing but the author reviewed, so the honest count is one — and + # a thread-only predicate would have read this as zero and passed it. + run record '["wenzowski"]' + [ "$status" -eq 0 ] + run ready + denied "$output" + [[ "$output" == *"1 blocking"* ]] +} + +@test "VACUITY: a buffer that is not the declared shape records nothing rather than one row" { + # `returns = "json-array"` (CLOUD-993). A `gh` that printed an auth error, or a + # wrapper that annotated its own output, must not become `rows 1` — which would + # be a refusal nobody can clear — nor `rows 0`, which would be a pass over a + # command that never answered. It records NOTHING, so the receipt row's + # did-you-look refusal stands. + run record 'gh: could not determine the current repository' + [ "$status" -eq 0 ] + run ready + denied "$output" + [[ "$output" == *"gh api graphql"* ]] +} + +@test "VACUITY: an empty buffer is not zero rows" { + # A command that printed nothing is could-not-look, not "there are none". + # Recording a zero here would turn silence into a pass. + run record '' + [ "$status" -eq 0 ] + run ready + denied "$output" +} + +@test "a buffer from a command nobody asked for never becomes the record" { + # The forgery control, over this fact: the agent chooses WHICH command runs and + # does not author what it prints, so byte-equality against the declaration is + # what stands between the two. + local envelope + envelope=$(python3 -c 'import json; print(json.dumps({"hook_event_name":"PostToolUse","session_id":"s","cwd":"/repo","tool_name":"Bash","tool_input":{"command":"echo []"},"tool_response":{"stdout":"[]","stderr":""}}))') + run bash -c "cd '$REPO' && printf '%s' '$envelope' | '$BIN' hook --harness claude-code" + [ "$status" -eq 0 ] + run ready + denied "$output" +} + +# --- what must NOT be refused ---------------------------------------------- + +@test "a re-draft is not a ready, even on a head carrying findings" { + # `land` re-drafts on a red run, and that is the one thing that stops the next + # push buying another matrix (CLOUD-240). Refusing it would leave the tap open + # on exactly the head this gate is keeping out of CI. + run record '["PRRT_a","PRRT_b"]' + [ "$status" -eq 0 ] + run ready '--undo' + allowed "$output" +} + +@test "VACUITY: a page the command could not read refuses rather than passing" { + # GitHub caps a connection page at 100, so a PR with more threads than that + # would have the surplus fall outside the query — and an unresolved thread out + # there would leave `rows == 0`, a FALSE GREEN in the one direction this gate + # exists to prevent. The projection emits an extra element per truncated + # connection, so the buffer a clear-but-truncated head produces is `[true]` + # rather than `[]`. + # + # THE DISCRIMINATING PAIR is this case beside "all answered": both are a head + # with zero unresolved threads, and only the truncated one refuses. Without the + # `pageInfo` clauses they would be the same buffer. + run record '[true]' + [ "$status" -eq 0 ] + run ready + denied "$output" + [[ "$output" == *"1 blocking"* ]] +} + +@test "THE BYPASS: a compound command is still a ready" { + # The case an earlier draft did not have, and the reason it did not: this module + # anchored on `startswith`, so `cd /repo && gh pr ready 702` went unjudged. The + # receipt row DOES select it, so an existing record satisfies the did-you-look + # half — and with the count half silent the call was allowed carrying two + # unresolved threads. Measured exactly that before the anchor came out. + # + # End to end rather than only in the module's `test_` rules, because what was + # wrong was the interaction between two rows: the receipt row's selection and + # this module's narrowing disagreeing about the same command. + run record '["PRRT_a","PRRT_b"]' + [ "$status" -eq 0 ] + + local envelope + envelope=$(python3 -c 'import json,sys; print(json.dumps({"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":sys.argv[1]}}))' 'cd /repo && gh pr ready 702') + run bash -c "cd '$REPO' && printf '%s' \"\$1\" | '$BIN' hook --harness claude-code" _ "$envelope" + denied "$output" + [[ "$output" == *"2 blocking"* ]] +} + +@test "a commit message naming the command is prose, not a ready" { + # THE ANCHOR'S DISCRIMINATING CASE. This repository writes `gh pr ready` down + # constantly — in commit messages, in issue bodies, in the module itself — so a + # `contains` over the raw command refuses its own documentation, which is the + # hazard `run-shape.rego`'s header records for the identical predicate. + # + # Over the binary rather than only in the module's own `test_` rules, because + # what is at risk is the engine handing the whole command string through: a + # `with input as` case fabricates that string and cannot show it arrives raw. + run record '["PRRT_a","PRRT_b"]' + [ "$status" -eq 0 ] + + local envelope + envelope=$(python3 -c 'import json,sys; print(json.dumps({"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":sys.argv[1]}}))' \ + 'git commit -m "run gh pr ready once the review is answered"') + run bash -c "cd '$REPO' && printf '%s' \"\$1\" | '$BIN' hook --harness claude-code" _ "$envelope" + allowed "$output" +} + +@test "reading the review is never refused, so the remedy is reachable" { + # A gate whose own remedy it blocks is unsatisfiable. Both `gh pr view` and the + # declared `gh api graphql` must pass on a head with findings recorded. + run record '["PRRT_a"]' + [ "$status" -eq 0 ] + local envelope + for c in 'gh pr view 702 --json reviewDecision' "$DECLARED"; do + envelope=$(python3 -c 'import json,sys; print(json.dumps({"hook_event_name":"PreToolUse","tool_name":"Bash","tool_input":{"command":sys.argv[1]}}))' "$c") + run bash -c "cd '$REPO' && printf '%s' \"\$1\" | '$BIN' hook --harness claude-code" _ "$envelope" + allowed "$output" + done +}