Require every blocking gate to prove it can fail (check-liveness ratchet)

cli/internal/gates/checks/negative_witness_test.go

@@ -0,0 +1,215 @@
+package checks
+
+import (
+	"bufio"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/boshu2/agentops/cli/internal/gates"
+)
+
+// Check-liveness closure: every BLOCKING script-backed gate must have a test
+// that demonstrates it FAILING on the thing it claims to detect.
+//
+// WHY: the 2026-07-25 review found two load-bearing rules that existed only as
+// text — ADR-0016's "Python never ships in skills" with no gate, and an
+// exact-identity digest contract whose test mocked Validate with the code under
+// test's own digest function. Both were green while broken. Those were
+// instances of one class: a rule nobody executes, and a check nobody proved
+// executes. A gate verified only on a green tree is indistinguishable from
+// `exit 0` — it would still pass with its detector deleted. This test is the
+// standing requirement the review asked for.
+//
+// SHAPE: a shrink-only ratchet, the same mechanism the repo already uses for
+// preamble adoption and shipped Python. The gates lacking a witness at the
+// cutoff are pinned in scripts/.gate-negative-witness-grandfather and exempt;
+// a NEW blocking gate must ship with a witness; a pinned gate that gains one
+// must be pruned. The list cannot grow — the growth guard below rejects any
+// entry not present at HEAD, so a change cannot exempt its own new gate.
+//
+// WHAT COUNTS: any file under tests/ that names the backing script AND asserts
+// a non-zero outcome from it. Deliberately generous about form — bats
+// (`[ "$status" -eq 1 ]`), shell integration tests, and Python
+// (`returncode != 0`) all qualify. The bar is "somebody proved it can fail",
+// not a particular framework.
+const negativeWitnessGrandfather = "scripts/.gate-negative-witness-grandfather"
+
+// negativeAssertion matches the ways this repo's tests assert a failing run.
+// Kept broad on purpose: a false NEGATIVE here (missing a real witness) would
+// wrongly pin a well-tested gate, which is merely noisy; the ratchet's teeth
+// come from the growth guard, not from this regex being exhaustive.
+var negativeAssertion = regexp.MustCompile(
+	`\$status" -eq [1-9]|\$status" -ne 0|assert_failure|refute_success|returncode, 1|returncode != 0|-ne 0 \]`,
+)
+
+// scrubbedGitEnv is the package-local twin of gitDiscoveryEnv
+// (cli/cmd/ao/git_read.go): the process environment minus the four git
+// discovery overrides. A hook-launched process inherits GIT_DIR/GIT_WORK_TREE,
+// and under a leaked GIT_DIR even a `-C <dir>`-scoped git call resolves against
+// the LEAKED repository — which is how a fixture `git init` once rewrote a
+// shared .git/config and bricked every worktree
+// (age-gate-scripts-worktree-gitdir-p62wo, ek8v).
+func scrubbedGitEnv() []string {
+	const prefixes = "GIT_DIR=|GIT_WORK_TREE=|GIT_COMMON_DIR=|GIT_INDEX_FILE="
+	env := make([]string, 0, len(os.Environ()))
+	for _, entry := range os.Environ() {
+		drop := false
+		for _, p := range strings.Split(prefixes, "|") {
+			if strings.HasPrefix(entry, p) {
+				drop = true
+				break
+			}
+		}
+		if !drop {
+			env = append(env, entry)
+		}
+	}
+	return env
+}
+
+func loadPinnedGates(t *testing.T, root string) map[string]bool {
+	t.Helper()
+	pinned := map[string]bool{}
+	f, err := os.Open(filepath.Join(root, negativeWitnessGrandfather))
+	if err != nil {
+		t.Fatalf("read %s: %v", negativeWitnessGrandfather, err)
+	}
+	defer f.Close()
+	scanner := bufio.NewScanner(f)
+	for scanner.Scan() {
+		line := strings.TrimSpace(scanner.Text())
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+		pinned[line] = true
+	}
+	if err := scanner.Err(); err != nil {
+		t.Fatalf("scan %s: %v", negativeWitnessGrandfather, err)
+	}
+	return pinned
+}
+
+// collectTestBodies reads every file under tests/ once.
+func collectTestBodies(t *testing.T, root string) map[string]string {
+	t.Helper()
+	bodies := map[string]string{}
+	testsDir := filepath.Join(root, "tests")
+	err := filepath.WalkDir(testsDir, func(path string, d os.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() {
+			return nil
+		}
+		body, readErr := os.ReadFile(path)
+		if readErr != nil {
+			return nil // unreadable fixture is not this test's business
+		}
+		bodies[path] = string(body)
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("walk tests/: %v", err)
+	}
+	return bodies
+}
+
+// hasNegativeWitness reports whether some test names the backing script and
+// asserts a non-zero outcome.
+func hasNegativeWitness(bodies map[string]string, backing string) bool {
+	base := filepath.Base(backing)
+	for _, body := range bodies {
+		if strings.Contains(body, base) && negativeAssertion.MatchString(body) {
+			return true
+		}
+	}
+	return false
+}
+
+func TestBlockingGatesHaveProvenNegativeWitness(t *testing.T) {
+	root := repoRootFromTest(t)
+	pinned := loadPinnedGates(t, root)
+	bodies := collectTestBodies(t, root)
+
+	proven := map[string]bool{}
+	for _, c := range gates.Default.All() {
+		if c.Backing == "" || !c.Blocking {
+			continue
+		}
+		if hasNegativeWitness(bodies, c.Backing) {
+			proven[c.ID] = true
+			continue
+		}
+		if pinned[c.ID] {
+			continue // grandfathered: inert, but visibly and countably so
+		}
+		t.Errorf("blocking gate %q (backing %s) has no test proving it FAILS on what it detects.\n"+
+			"    Add a test under tests/ that names %s and asserts a non-zero outcome.\n"+
+			"    Pinning it in %s is NOT a repair — the growth guard rejects new entries.",
+			c.ID, c.Backing, filepath.Base(c.Backing), negativeWitnessGrandfather)
+	}
+
+	// Shrink direction: a pinned gate that now has a witness must be pruned, so
+	// the count can only go down.
+	for id := range pinned {
+		if proven[id] {
+			t.Errorf("gate %q now has a negative witness but is still pinned in %s — remove that line (the allowlist only shrinks)",
+				id, negativeWitnessGrandfather)
+		}
+	}
+
+	// Stale direction: a pinned gate that no longer exists in the registry is
+	// dead weight hiding the real count.
+	live := map[string]bool{}
+	for _, c := range gates.Default.All() {
+		live[c.ID] = true
+	}
+	for id := range pinned {
+		if !live[id] {
+			t.Errorf("gate %q is pinned in %s but is not in the registry — remove that line", id, negativeWitnessGrandfather)
+		}
+	}
+
+	t.Logf("check-liveness: %d blocking gate(s) still lack a proven negative witness (shrink-only)", len(pinned))
+}
+
+// TestNegativeWitnessAllowlistOnlyShrinks is the growth guard. Without it the
+// ratchet has no teeth: a change could add a new blocking gate and exempt it in
+// the same diff, which is exactly the "rule nobody executes" pattern this whole
+// mechanism exists to stop. HEAD is the authority — a same-diff self-allowlist
+// grants no protection.
+func TestNegativeWitnessAllowlistOnlyShrinks(t *testing.T) {
+	root := repoRootFromTest(t)
+	// cmd.Dir pins the repository and cmd.Env strips the git discovery overrides.
+	// TestMain's process-level ScrubGitDiscoveryEnv already covers this package,
+	// but the per-command scrub is the repo's stated contract and does not rely
+	// on a TestMain elsewhere in the package staying correct
+	// (age-gate-scripts-worktree-gitdir-p62wo; gitDiscoveryEnv in
+	// cli/cmd/ao/git_read.go). A hook-leaked GIT_DIR would otherwise resolve this
+	// read against a different repository entirely.
+	cmd := exec.Command("git", "show", "HEAD:"+negativeWitnessGrandfather)
+	cmd.Dir = root
+	cmd.Env = scrubbedGitEnv()
+	out, err := cmd.Output()
+	if err != nil {
+		t.Skip("no HEAD version of the allowlist (initial snapshot)")
+	}
+	base := map[string]bool{}
+	for _, line := range strings.Split(string(out), "\n") {
+		line = strings.TrimSpace(line)
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+		base[line] = true
+	}
+	for id := range loadPinnedGates(t, root) {
+		if !base[id] {
+			t.Errorf("%s gained entry %q — the allowlist only SHRINKS. A new blocking gate ships with a negative witness or it does not ship.",
+				negativeWitnessGrandfather, id)
+		}
+	}
+}

scripts/.gate-negative-witness-grandfather

@@ -0,0 +1,50 @@
+# scripts/.gate-negative-witness-grandfather — check-liveness ratchet allowlist.
+#
+# Every BLOCKING script-backed gate in the Go registry that, at the ratchet
+# cutoff, has no test anywhere under tests/ demonstrating it FAILS on the thing
+# it claims to detect. A gate proven only on a green tree is indistinguishable
+# from `exit 0`: it would still pass with its detector removed.
+#
+# This is the generalization of the two inert rules the 2026-07-25 review found
+# (ADR-0016 with no gate; a digest contract whose test mocked the code under
+# test). Those were instances; this is the class.
+#
+# SHRINK-ONLY: the list only shrinks. When a gate gains a negative witness, its
+# line MUST be pruned (the closure test FAILS until it is). A NEW blocking gate
+# cannot be added here — it ships with a negative witness or it does not ship.
+#
+# A "negative witness" is any test under tests/ that names the backing script
+# AND asserts a non-zero outcome from it.
+#
+# Enforced by: cli/internal/gates/checks/negative_witness_test.go
+adapter.gc-executor
+always.file-manifest-overlap
+always.git-config-hygiene
+always.mutation-route
+always.quarantine-empty
+always.regen-all
+always.retrieval-manifest-paths
+ci.policy-parity
+contract.cathedral-cut
+contract.compatibility
+contract.finding-registry
+contract.skill-mesh
+contract.verdict-corpus
+derived.changed-scope
+go.cli-contract
+go.cli-reference
+go.cli-surface-counts
+go.command-test-pair
+go.complexity
+go.home-isolation
+go.test-home-isolation
+go.test-isolation
+skill.cli-snippets
+skill.codex-override-coverage
+skill.codex-parity-drift
+skill.codex-runtime-sections
+skill.heal-strict
+skill.runtime-formats
+skill.runtime-parity
+skill.triggers
+workflow.install-drift

tests/scripts/validate-skill-schema.bats

@@ -0,0 +1,89 @@
+#!/usr/bin/env bats
+# Tests for scripts/validate-skill-schema.sh — the skill.schema blocking gate.
+#
+# WHY THIS EXISTS: skill.schema gates every SKILL.md in the corpus and, until
+# now, had no test at all. Nothing had ever demonstrated it can FAIL. A gate
+# proven only against a green tree is indistinguishable from `exit 0` — replace
+# its validator with a no-op and CI stays green. This file is the negative
+# witness, and it prunes skill.schema from
+# scripts/.gate-negative-witness-grandfather (the check-liveness ratchet in
+# cli/internal/gates/checks/negative_witness_test.go).
+#
+# The script anchors REPO_ROOT to `dirname $0/..`, so a fixture tree with its
+# own scripts/ + schemas/ + skills/ exercises the real validator against
+# controlled input rather than against the live corpus.
+
+setup() {
+    REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../.." && pwd)"
+    TMP_DIR="$(mktemp -d)"
+    mkdir -p "$TMP_DIR/scripts" "$TMP_DIR/schemas" "$TMP_DIR/skills"
+    cp "$REPO_ROOT/scripts/validate-skill-schema.sh" "$TMP_DIR/scripts/"
+    cp "$REPO_ROOT/schemas/skill-frontmatter.v1.schema.json" "$TMP_DIR/schemas/"
+    chmod +x "$TMP_DIR/scripts/validate-skill-schema.sh"
+}
+
+teardown() {
+    rm -rf "$TMP_DIR"
+}
+
+write_skill() {  # write_skill <slug> <frontmatter-body>
+    mkdir -p "$TMP_DIR/skills/$1"
+    { printf -- '---\n'; printf '%s\n' "$2"; printf -- '---\n\n# %s\n' "$1"; } \
+        > "$TMP_DIR/skills/$1/SKILL.md"
+}
+
+run_gate() {
+    ( cd "$TMP_DIR" && bash scripts/validate-skill-schema.sh )
+}
+
+# Descriptions are single-quoted: the corpus convention embeds `Triggers: "x"`
+# inside the value, and unquoted that colon makes YAML read a nested mapping —
+# which fails the gate for a PARSE error rather than the schema violation each
+# negative below is meant to witness.
+valid_frontmatter() {
+    printf "name: alpha\ndescription: 'A valid fixture skill. Triggers: \"alpha\".'\nskill_api_version: 1"
+}
+
+@test "a schema-valid skill passes" {
+    write_skill alpha "$(valid_frontmatter)"
+    run run_gate
+    [ "$status" -eq 0 ]
+}
+
+# --- negative witnesses: the gate must be shown to FAIL ----------------------
+
+@test "NEGATIVE: a skill missing the required name field fails" {
+    write_skill alpha "$(valid_frontmatter)"
+    write_skill broken "description: 'Missing its name. Triggers: \"broken\".'
+skill_api_version: 1"
+    run run_gate
+    [ "$status" -eq 1 ]
+    [[ "$output" == *broken* ]]
+}
+
+@test "NEGATIVE: a skill missing the required description field fails" {
+    write_skill alpha "$(valid_frontmatter)"
+    write_skill broken 'name: broken
+skill_api_version: 1'
+    run run_gate
+    [ "$status" -eq 1 ]
+    [[ "$output" == *broken* ]]
+}
+
+@test "NEGATIVE: a skill missing skill_api_version fails" {
+    write_skill alpha "$(valid_frontmatter)"
+    write_skill broken "name: broken
+description: 'Has no api version. Triggers: \"broken\".'"
+    run run_gate
+    [ "$status" -eq 1 ]
+    [[ "$output" == *broken* ]]
+}
+
+@test "NEGATIVE: one bad skill fails the run even when others are valid" {
+    write_skill alpha "$(valid_frontmatter)"
+    write_skill beta "$(printf "name: beta\ndescription: 'Another valid one. Triggers: \"beta\".'\nskill_api_version: 1")"
+    write_skill broken "description: 'no name here'
+skill_api_version: 1"
+    run run_gate
+    [ "$status" -eq 1 ]
+}