ci(gate): hold .coderabbit.yaml to the three keys it was landed for

hk.pkl

@@ -173,6 +173,22 @@ local gate = new Mapping<String, Step> {
   ["license-table-check"] {
     check = "mise run license-table-check"
   }
+  // The mechanism CLOUD-847 shipped without (CLOUD-860). That row landed
+  // `.coderabbit.yaml` after measuring what each key buys — a formal
+  // CHANGES_REQUESTED review so a verdict exists at all, review on drafts so it
+  // exists in the phase that costs nothing, and `gitleaks` kept because
+  // `mise run ci` is `if: draft == false` and a draft would otherwise have no
+  // secret scanning. Nothing then held the file to those readings, and the
+  // failure is silent in the worst direction: reviews simply stop, which looks
+  // exactly like nobody having pushed.
+  //
+  // Globbed on the one file it reads, unlike its glob-less neighbours above: a
+  // commit touching nothing else cannot change the answer, so the whole-index
+  // scan they need would only cost time here.
+  ["coderabbit-config-check"] {
+    glob = List(".coderabbit.yaml")
+    check = "mise run coderabbit-config-check"
+  }
   // The two-tier gate ships with its mechanism (non-negotiable 2). The steps
   // below marked `profiles = List("slow")` are skipped at pre-commit and run by
   // `check`, and the dangerous direction is not the obvious one: a tier that

mise-tasks/coderabbit-config-check

@@ -0,0 +1,139 @@
+#!/usr/bin/env bash
+#MISE description="Gate: .coderabbit.yaml still carries the keys the review lifecycle depends on (pointer-only)"
+#
+# CLOUD-860, and the missing half of CLOUD-847. That row landed `.coderabbit.yaml`
+# and measured every key in it; nothing then held the file to those readings, so
+# a rule shipped without a mechanism — the shape non-negotiable rule 2 names.
+#
+# THE THREE KEYS ARE NOT A STYLE PREFERENCE, they are what the lifecycle rests on:
+#
+#   request_changes_workflow  findings arrive as a formal CHANGES_REQUESTED review,
+#                             so `reviewDecision` carries an answer. Off, the bot
+#                             only COMMENTS and the decision stays null.
+#   auto_review.drafts        the draft phase is the free phase (every job in
+#                             ci.yml is `if: draft == false`). Off, nothing reviews
+#                             it, and the review can only arrive after the ready —
+#                             which is the whole defect CLOUD-847 measured.
+#   tools.gitleaks.enabled    the ONLY secret scanning a draft gets, for the same
+#                             reason: `mise run ci` does not run on drafts. The
+#                             other linters are deliberately off because our gates
+#                             already run them; this one is deliberately kept.
+#
+# WHY THE FAILURE IS WORTH A GATE RATHER THAN A COMMENT. Flipping `drafts` back is
+# a one-line diff, and its symptom is silence: reviews stop happening, which looks
+# exactly like nobody having pushed. The gate that consumes the config would then
+# refuse every PR for want of a review the config quietly stopped producing, and
+# the visible failure would be the gate rather than the cause.
+#
+# ABSENT IS NOT PASSING for the first two, and that asymmetry is the point: a key
+# nobody wrote and a key someone deleted are the same file, and both leave the
+# default in force — which is the value this gate exists to refuse. `gitleaks` is
+# the inverse, because its default IS enabled: only an explicit `false` is a
+# violation there, so absence passes.
+#
+# Pointer-only per non-negotiable rule 4: `path:line key=value` and a count, never
+# a byte of the file. A config can carry instructions and paths, and a gate that
+# echoed them would put them in every CI log.
+#
+# Exit 0 every required key holds / 2 a key is missing or flipped. No fail-open
+# arm, unlike a gate that reads GitHub: the input is a tracked file in this
+# checkout, so "could not look" means the file is gone, which is itself the
+# violation this refuses.
+#
+# The mutation drops the empty-file guard, so a file with no keys reports zero
+# violations — the vacuous pass this gate is shaped to avoid, and only the
+# empty-fixture case can catch it.
+#MUTANT empty-file-is-a-pass|s/if \[ "\$keys" -eq 0 \]/if false/|a file with no keys must not read as compliant
+set -euo pipefail
+
+CFG="${1:-.coderabbit.yaml}"
+
+if [ ! -f "$CFG" ]; then
+	echo "::error:: coderabbit-config-check: $CFG is absent — the review lifecycle has no configuration to rest on" >&2
+	exit 2
+fi
+
+violations=0
+report() { # $1 = line (0 when the key is absent), $2 = pointer
+	echo "$CFG:$1 $2" >&2
+	violations=$((violations + 1))
+}
+
+# A key line at any indentation, reported with its line number. The file is ours
+# and its shape is reviewed; this reads the key rather than the tree, which is
+# what keeps the gate to one awk pass and no YAML dependency.
+key_line() { # $1 = key name -> "line<TAB>value", empty when absent
+	awk -v key="$1" '
+		$0 ~ "^[[:space:]]*#" { next }
+		{
+			pattern = "^[[:space:]]*" key ":[[:space:]]*"
+			if ($0 ~ pattern) {
+				value = $0
+				sub(pattern, "", value)
+				sub("[[:space:]]*#.*$", "", value)
+				sub("[[:space:]]+$", "", value)
+				print NR "\t" value
+				exit
+			}
+		}
+	' "$CFG"
+}
+
+# `enabled:` appears once per tool, so this one is scoped: find the tool, then read
+# the first `enabled:` inside its block. Reading it unscoped would answer about
+# whichever tool happened to come first in the file.
+tool_enabled() { # $1 = tool name -> "line<TAB>value", empty when the tool is absent
+	awk -v tool="$1" '
+		$0 ~ "^[[:space:]]*#" { next }
+		!seen && $0 ~ ("^[[:space:]]*" tool ":[[:space:]]*$") { seen = 1; next }
+		seen && $0 ~ "^[[:space:]]*enabled:[[:space:]]*" {
+			value = $0
+			sub("^[[:space:]]*enabled:[[:space:]]*", "", value)
+			sub("[[:space:]]*#.*$", "", value)
+			sub("[[:space:]]+$", "", value)
+			print NR "\t" value
+			exit
+		}
+		seen && $0 ~ "^[[:space:]]*[A-Za-z0-9_-]+:[[:space:]]*$" { exit }
+	' "$CFG"
+}
+
+# The vacuity guard, and the reason it is first: every check below is an assertion
+# ABOUT a key, so a file carrying none of them satisfies all of them by having
+# nothing to judge. Counting real keys turns "no violations found" back into a
+# statement about a file that was actually read.
+keys=$(grep -cE '^[[:space:]]*[A-Za-z0-9_-]+:' "$CFG" || true)
+if [ "$keys" -eq 0 ]; then
+	echo "::error:: coderabbit-config-check: $CFG carries no keys at all, so every assertion below would pass vacuously" >&2
+	exit 2
+fi
+
+require_true() { # $1 = key name
+	local hit line value
+	hit=$(key_line "$1")
+	if [ -z "$hit" ]; then
+		report 0 "$1 absent (default is in force)"
+		return
+	fi
+	line=${hit%%	*}
+	value=${hit#*	}
+	[ "$value" = "true" ] || report "$line" "$1=$value (want true)"
+}
+
+require_true "request_changes_workflow"
+require_true "drafts"
+
+# The inverse arm: gitleaks defaults to enabled, so absence is compliant and only
+# an explicit `false` is the violation.
+hit=$(tool_enabled "gitleaks")
+if [ -n "$hit" ]; then
+	line=${hit%%	*}
+	value=${hit#*	}
+	[ "$value" != "false" ] || report "$line" "tools.gitleaks.enabled=false (drafts would have no secret scanning)"
+fi
+
+if [ "$violations" -ne 0 ]; then
+	echo "::error:: coderabbit-config-check: $violations violation(s) in $CFG — see CLOUD-847 for what each key was measured to do" >&2
+	exit 2
+fi
+echo "coderabbit-config-check: $CFG holds the three keys the review lifecycle depends on"

tests/coderabbit-config-check.bats

@@ -0,0 +1,156 @@
+#!/usr/bin/env bats
+# subject: mise-tasks/coderabbit-config-check
+# The mechanism CLOUD-847 shipped without (CLOUD-860).
+#
+# That row landed `.coderabbit.yaml` and measured what each key buys; nothing then
+# held the file to those readings. These cases pin both directions per CLOUD-418 —
+# each of the three keys refused when flipped, the real file passing — plus the
+# vacuity case a key-absence check needs, where a file carrying no keys satisfies
+# every per-key assertion by having none to judge.
+
+setup() {
+	CHECK="$BATS_TEST_DIRNAME/../mise-tasks/coderabbit-config-check"
+	cd "$BATS_TEST_DIRNAME/.." || return 1
+}
+
+# The same shape as the real file, so a fixture exercises the parser rather than a
+# simplified stand-in — including the sibling tools whose `enabled:` keys are what
+# make the gitleaks arm need scoping.
+write_config() {
+	# $1 = destination, $2 = request_changes_workflow, $3 = drafts, $4 = gitleaks enabled
+	cat >"$1" <<EOF
+# a leading comment, which the parser must skip
+reviews:
+  request_changes_workflow: $2
+
+  auto_review:
+    drafts: $3
+    auto_pause_after_reviewed_commits: 50
+
+  high_level_summary: false
+
+  tools:
+    clippy:
+      enabled: false
+    shellcheck:
+      enabled: false
+    gitleaks:
+      enabled: $4
+EOF
+}
+
+@test "the repo as it stands passes" {
+	run "$CHECK"
+	[ "$status" -eq 0 ]
+}
+
+@test "the gate is wired: hk.pkl declares a step that runs this task" {
+	# Asserted on the step block, not a bare grep: the surrounding comment names
+	# the task too, and a comment is not a call site. A suite that passes while
+	# nothing invokes the task measures only itself.
+	run awk '/^  \["coderabbit-config-check"\] \{$/ { found = 1; next }
+	         found && /mise run coderabbit-config-check/ { print "wired"; exit }
+	         found && /^  \}$/ { exit }' hk.pkl
+	[ "$status" -eq 0 ]
+	[ "$output" = "wired" ]
+}
+
+@test "a compliant fixture passes" {
+	write_config "$BATS_TEST_TMPDIR/c.yaml" true true true
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 0 ]
+}
+
+@test "request_changes_workflow flipped off fails, and names the key" {
+	# Off, the bot only COMMENTS: `reviewDecision` stays null and no verdict exists.
+	write_config "$BATS_TEST_TMPDIR/c.yaml" false true true
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 2 ]
+	[[ "$output" == *"request_changes_workflow=false"* ]]
+}
+
+@test "drafts flipped off fails, and names the key" {
+	# Off, the free phase goes back to being the unreviewed phase.
+	write_config "$BATS_TEST_TMPDIR/c.yaml" true false true
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 2 ]
+	[[ "$output" == *"drafts=false"* ]]
+}
+
+@test "gitleaks disabled fails: drafts would have no secret scanning at all" {
+	write_config "$BATS_TEST_TMPDIR/c.yaml" true true false
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 2 ]
+	[[ "$output" == *"gitleaks"* ]]
+}
+
+@test "the gitleaks arm is SCOPED to gitleaks, not to the first tool in the file" {
+	# clippy and shellcheck are `enabled: false` by design. An unscoped read of
+	# `enabled:` would answer about whichever tool came first and fail a compliant
+	# file — the false refusal that would get this gate bypassed.
+	write_config "$BATS_TEST_TMPDIR/c.yaml" true true true
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 0 ]
+}
+
+@test "gitleaks absent passes: its default is enabled, so only an explicit false is a violation" {
+	cat >"$BATS_TEST_TMPDIR/c.yaml" <<'EOF'
+reviews:
+  request_changes_workflow: true
+  auto_review:
+    drafts: true
+EOF
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 0 ]
+}
+
+@test "a key deleted rather than flipped fails: absence leaves the default in force" {
+	# The two ways a key stops holding are the same file to CodeRabbit, so they
+	# must be the same verdict here.
+	cat >"$BATS_TEST_TMPDIR/c.yaml" <<'EOF'
+reviews:
+  auto_review:
+    drafts: true
+EOF
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 2 ]
+	[[ "$output" == *"request_changes_workflow absent"* ]]
+}
+
+@test "a comment-only file is a failure, not a vacuous pass" {
+	# The case the whole gate is shaped around: every check is an assertion ABOUT
+	# a key, so a file with none satisfies all of them by having nothing to judge.
+	printf '# every key deleted\n# but the file still exists\n' >"$BATS_TEST_TMPDIR/c.yaml"
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 2 ]
+	[[ "$output" == *"no keys at all"* ]]
+}
+
+@test "a commented-out key does not satisfy the assertion" {
+	# `# drafts: true` is the shape of a key someone disabled while leaving a note.
+	cat >"$BATS_TEST_TMPDIR/c.yaml" <<'EOF'
+reviews:
+  request_changes_workflow: true
+  auto_review:
+    # drafts: true
+    auto_pause_after_reviewed_commits: 50
+EOF
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[ "$status" -eq 2 ]
+	[[ "$output" == *"drafts absent"* ]]
+}
+
+@test "an absent file fails rather than passing for want of anything to read" {
+	run "$CHECK" "$BATS_TEST_TMPDIR/does-not-exist.yaml"
+	[ "$status" -eq 2 ]
+	[[ "$output" == *"absent"* ]]
+}
+
+@test "output is pointer-only: it names keys and lines, never the file's contents" {
+	# Non-negotiable rule 4. A config carries paths and instructions, and a gate
+	# that echoed them would put them in every CI log.
+	write_config "$BATS_TEST_TMPDIR/c.yaml" false true true
+	run "$CHECK" "$BATS_TEST_TMPDIR/c.yaml"
+	[[ "$output" != *"auto_pause_after_reviewed_commits"* ]]
+	[[ "$output" != *"high_level_summary"* ]]
+}