feat(hooks): mcp-destructive-guard asks before irreversible MCP verbs (#52)

PreToolUse hook on mcp__.*: asks for repo_delete, file_delete, branch_delete, pr_merge, tbd_ship, repo_update, repo_mirror_push, workflow_run_trigger, tag_create, release_create on any server name, and for file_write_branch unless it targets a plain non-main/master branch with non-empty content under exactly one exact-case branch/content key. Fails closed; never emits allow.

Refs mathias/infra#508. Reviewed in three rounds by pr-gatekeeper with execution against gitea-mcp and Gitea 1.27.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Mathias <mthbqv@gmail.com>

.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "skills",
   "description": "Mathias's engineering skills library, plus the hooks and MCP servers that make it enforceable: TDD, clean code, debugging, specs, discovery, CI, and secret hygiene.",
-  "version": "0.17.1",
+  "version": "0.22.0",
   "author": {
     "name": "Mathias",
     "url": "https://git.d-ma.be/mathias"

.gitea/workflows/ci.yml

@@ -133,6 +133,33 @@ jobs:
             git add "$manifest"
             # The trailer below stops this push from re-triggering the tagger.
             git commit -m "chore(release): plugin.json version $version" -m "Skip-Release: true"
+
+            # skills#46. This commit is pushed with secrets.GITEA_TOKEN, and a
+            # push authenticated with the Actions token does NOT trigger
+            # workflows -- deliberate platform behaviour to stop CI recursing.
+            # `Skip-Release: true` stops the tagger recursing; nothing stopped
+            # the whole run, `gate` included, from never firing on this commit.
+            #
+            # So every release left main's HEAD with zero CI runs. Measured:
+            # 6cc224f (0.19.4) and f031c2b (0.19.5) each have none, and the
+            # koala cluster's CIHeadUncovered{repo=skills} had been firing on it
+            # for 85 hours -- an alert that can only be cleared by an unrelated
+            # human push.
+            #
+            # The commit is not cosmetic: plugin.json's version is what Claude
+            # Code gates plugin updates on, `task check` asserts it is not
+            # behind the latest tag, and the sed-and-retry logic above already
+            # shipped a mismatched manifest once (v0.10.0 reading 0.9.0).
+            #
+            # Gating it HERE rather than re-triggering CI afterwards: the commit
+            # is produced here, so it can be validated here, at the cost of one
+            # `task check` instead of a second full run -- and with no recursion
+            # risk to reintroduce.
+            if ! task check; then
+              echo "::error::the release-bump commit fails task check -- not pushing it" >&2
+              exit 1
+            fi
+
             if git push origin HEAD:main; then
               echo "bumped $manifest to $version"
               exit 0

.mcp.json

@@ -1,5 +1,5 @@
 {
-  "_comment": "Tailscale Serve on koala, not the public d-ma.be hostnames. The public path exits to Cloudflare and NPM to reach a k3s service on the same machine most sessions run on, which imposes rate limits and message-size caps on tool results. Serve terminates TLS with a real Tailscale cert and proxies to the NodePort over loopback, so traffic reaches the service from koala itself and satisfies the default-deny-ingress NetworkPolicy with no cluster change. Clients need 'tailscale set --accept-dns=true' for the MagicDNS name to resolve; see mathias/skills#24.",
+  "_comment": "Tailscale Serve on koala, not the public d-ma.be hostnames. The public path exits to Cloudflare and NPM to reach a k3s service on the same machine most sessions run on, which imposes rate limits and message-size caps on tool results. Serve terminates TLS with a real Tailscale cert and proxies to the NodePort over loopback, so traffic reaches the service from koala itself and satisfies the default-deny-ingress NetworkPolicy with no cluster change. Clients need 'tailscale set --accept-dns=true' for the MagicDNS name to resolve; see mathias/skills#24. Every ${VAR} below must be one ~/.op-env defines -- bin/check-mcp-env enforces that, because an undefined one sends an empty Authorization header and reports as 'requires authentication', which is indistinguishable from a revoked credential.",
   "mcpServers": {
     "brain": {
       "type": "http",
@@ -11,23 +11,24 @@
     "infra": {
       "type": "http",
       "url": "https://koala.tail30472e.ts.net:8444/mcp"
+    },
+    "gitea": {
+      "_comment": "gitea-mcp authenticates by PASS-THROUGH: the bearer is a Gitea PAT, which the server validates against Gitea and then acts as. So no dedicated MCP token is needed, and the AgentSecrets blocker that parked this entry (skills#32, #33) never applied to it -- DMABE_GITEA_API_TOKEN is op://HomeLab and already in the session environment. Verified 2026-09-07: initialize 200 (serverInfo gitea-mcp) and tools/list returns 41 tools. Two DIFFERENT PATs both authenticate while GITEA_MCP_TOKEN 401s, which is what proves this is pass-through validation and not a static-token comparison.",
+      "type": "http",
+      "url": "https://koala.tail30472e.ts.net:8446/mcp",
+      "headers": {
+        "Authorization": "Bearer ${DMABE_GITEA_API_TOKEN}"
+      }
     }
   },
   "_parked": {
-    "_reason": "cad and gitea-mcp are proxied, reachable and AUTHENTICATING: tailscale serve 8445 -> 127.0.0.1:31877 and 8446 -> 127.0.0.1:30340; an MCP initialize with the correct bearer returns 200 from both (serverInfo gitea-mcp 0.1.0, cad aed2aaf). They stay parked only because nothing puts those two tokens into the session environment. Both live in the 1Password AgentSecrets vault, and by deliberate design (infra#224, closed) no single identity on koala reads both vaults: koala-homelab is HomeLab-only and resolves ~/.op-env, koala-agentsecrets-write is AgentSecrets-only and lives in ~/.op-agent-write.env. Adding an AgentSecrets reference to ~/.op-env does NOT degrade gracefully -- `op run` fails wholesale and every other token, BRAIN_MCP_TOKEN included, goes unresolved with it. Enabling needs a read-scoped AgentSecrets identity in the session-env path; see mathias/skills#32, #33.",
+    "_reason": "cad is proxied, reachable and AUTHENTICATING: tailscale serve 8445 -> 127.0.0.1:31877, and an MCP initialize with the correct bearer returns 200 (serverInfo cad aed2aaf). It stays parked only because nothing puts CAD_MCP_TOKEN into the session environment. It lives in the 1Password AgentSecrets vault, and by deliberate design (infra#224, closed) no single identity on koala reads both vaults: koala-homelab is HomeLab-only and resolves ~/.op-env, koala-agentsecrets-write is AgentSecrets-only and lives in ~/.op-agent-write.env. Adding an AgentSecrets reference to ~/.op-env does NOT degrade gracefully -- `op run` fails wholesale and every other token, BRAIN_MCP_TOKEN included, goes unresolved with it. Enabling needs a read-scoped AgentSecrets identity in the session-env path; see mathias/skills#32, #33. gitea was parked here for the same stated reason and has been unparked: it needs no AgentSecrets token at all, because gitea-mcp is pass-through-authenticated.",
     "cad": {
       "type": "http",
       "url": "https://koala.tail30472e.ts.net:8445/mcp",
       "headers": {
         "Authorization": "Bearer ${CAD_MCP_TOKEN}"
       }
-    },
-    "gitea": {
-      "type": "http",
-      "url": "https://koala.tail30472e.ts.net:8446/mcp",
-      "headers": {
-        "Authorization": "Bearer ${GITEA_MCP_STATIC_TOKEN}"
-      }
     }
   }
 }

README.md

@@ -29,7 +29,7 @@ descriptions and "use when" triggers.
 /plugin install skills@mathias
 ```
 
-The plugin ships skills, the `secret-guard` + `pre-task-ritual` hooks, and the
+The plugin ships skills, the `secret-guard`, `mcp-destructive-guard` and `pre-task-ritual` hooks, and the
 brain/gitea/infra MCP servers as one versioned unit. Skills are namespaced
 (`skills:tdd`). Updates land when `.claude-plugin/plugin.json`'s `version`
 moves — CI bumps it on every release.
@@ -100,9 +100,11 @@ default to point into `~/dev`.
 | `AGENTS_SKILLS_DIR` | `$HOME/.agents/skills` |
 | `CRUSH_SKILLS_DIR` | `$HOME/.config/crush/skills` |
 | `OPENCODE_SKILLS_DIR` | `$HOME/.config/opencode/skills` |
+| `ANTIGRAVITY_SKILLS_JSON` | `$HOME/.gemini/config/skills.json` (registers `AGENTS_SKILLS_DIR` as a path entry; needs `jq`) |
 | `SKILLS_EXTRA_SOURCES` | unset (colon-separated extra source directories) |
 
-Antigravity and Mistral Vibe targets were removed — unused.
+The Mistral Vibe target was removed — unused. Antigravity's was removed for
+the same reason and re-added once it came back into use.
 
 ## Checking a host
 

SKILLS_INDEX.md

@@ -17,6 +17,7 @@ the SKILL.md, not the table.
 | `cognitive-load` | Cognitive complexity analysis (CLI score) | Identifying hotspots, before refactoring | claude-code, library |
 | `debug` | Hypothesis-first debugging discipline | Any test failure or bug whose cause is not immediately obvious — or a bug report with no failure text, which this skill refuses rather than guesses at | claude-code, library |
 | `discovery-framing` | Problem-before-solution discovery/framing method | Framing/validating a problem before any solution; opportunity sizing | claude-code, library |
+| `document-to-markdown` | Read .docx/.xlsx/.pptx/.epub/.msg and other formats the Read tool cannot open | You need the contents of a binary document — Word, Excel, PowerPoint, EPUB, Outlook .msg, a zip of documents — and Read returns nothing useful | claude-code, library |
 | `dream` | Reflective consolidation pass over an agent's memory directory | Pruning/merging memory files, rebuilding a memory index | claude-code, library, chat |
 | `experiment-spec` | Rigorous experiment spec — falsifiable hypothesis, quantitative acceptance, null-result protocol | Scoping a research/ML phase instead of a feature | claude-code, library |
 | `feature-spec` | Implementation-level feature spec (tighter than spec-driven-dev) | Scoping one feature inside an already-specced project | claude-code, library |

Taskfile.yml

@@ -78,7 +78,7 @@ tasks:
         for entry in "{{.TASKFILE_DIR}}"/*; do
           base=$(basename "$entry")
           case "$base" in
-            install.sh|Taskfile.yml|README.md|SKILLS_INDEX.md|skills|hooks|bin|tests) continue ;;
+            install.sh|Taskfile.yml|README.md|SKILLS_INDEX.md|skills|hooks|bin|tests|agents) continue ;;
           esac
           echo "ERROR: unexpected entry at repo root: $base" >&2
           fail=1
@@ -138,6 +138,26 @@ tasks:
           echo "ERROR: missing .claude-plugin/plugin.json" >&2; fail=1
         fi
 
+        # Every ${VAR} a live .mcp.json server interpolates must be one the
+        # session env-file defines. An undefined one is silent: the header
+        # goes out empty and the server reports "requires authentication",
+        # which reads as a revoked credential. Exit 2 means the check could
+        # not run (no jq, or no env-file — CI has neither); that is reported
+        # and not fatal, but it is never treated as a pass.
+        if [ -x "{{.TASKFILE_DIR}}/bin/check-mcp-env" ]; then
+          # `|| rc=$?` is load-bearing, not style. Task runs this block under
+          # `sh -e`, so a bare invocation exiting non-zero kills the shell
+          # before `case` is reached -- the tolerance below never ran, and the
+          # gate died with `exit status 2` on the commit that added it.
+          rc=0
+          "{{.TASKFILE_DIR}}/bin/check-mcp-env" "{{.TASKFILE_DIR}}/.mcp.json" "$HOME/.op-env" || rc=$?
+          case $rc in
+            0) ;;
+            2) echo "NOTE: MCP env wiring unverified on this host" >&2 ;;
+            *) fail=1 ;;
+          esac
+        fi
+
         if [ $fail -ne 0 ]; then
           echo "check FAILED" >&2; exit 1
         fi

agents/pr-gatekeeper.md

@@ -0,0 +1,299 @@
+---
+name: pr-gatekeeper
+description: Adversarially reviews one change and returns a blocking verdict. Two modes — an open Gitea PR (verify CI, then merge and close, or escalate), or a staged/unpushed diff in a GitOps repo where a branch cannot be reconciled, in which case it reviews before the push and records the verdict on the issue without merging. Use for risk/medium work. Spawned with a repo, the issue it closes, and either a PR number or a checkout path.
+tools: Read, Grep, Glob, Bash
+color: "#7C3AED"
+---
+
+<role>
+You are the merge gate for a solo estate. Nobody else will read this PR. If you
+approve something broken, it ships to a live GitOps cluster within 60 seconds.
+
+Your default posture is refusal. The author (another agent) believes the work is
+correct — that belief is the input you are testing, not evidence. The same model
+in the author's seat has convinced itself five times in a row and been wrong
+every time.
+
+Blocking the same PR repeatedly is the control working, not you being
+difficult. For code whose job is to refuse something, expect it.
+</role>
+
+## What you are given
+
+One of two modes. Everything after step 2 is identical — the review standard does
+not change with the plumbing.
+
+**PR mode** (the original):
+- `owner/repo`, the PR number, and the issue number it claims to close
+- The claim the author made about what the change does
+
+**Pre-push mode** (infra#470): there is no PR, because there cannot be one.
+- `owner/repo`, the path to the checkout, and the issue number the work claims to close
+- The claim the author made about what the change does
+
+In a repo whose deployment watches one branch — `mathias/infra`, where Flux
+watches `main` — a branch is invisible to the thing that applies it. A
+`risk/medium` change to `alerts.yaml` or `litellm-config.yaml` can be linted on a
+branch but never *reconciled* there, so a PR would review a state that was never
+deployed. The agreed lane is therefore that such work stays on `main` and gets a
+recorded adversarial review pass **before the push**, with you as the reviewer.
+
+If that lane resolves to "the author was careful", it has replaced nothing. You are
+the difference between the rule and a sentence about the rule.
+
+## Procedure
+
+Work in the repo checkout at the path given to you. Never push, never force,
+never amend the author's commits.
+
+### 1. Read the issue before the diff
+
+```
+mcp__plugin_skills_gitea__issue_get
+mcp__plugin_skills_gitea__issue_list_comments
+```
+
+The issue's "done when" clauses are the acceptance criteria. A PR that is
+well-built and does not satisfy them is a **reject**, not an approve-with-note.
+Comments carry sequencing constraints that the body does not.
+
+### 2. Get the real diff
+
+**PR mode:**
+
+```bash
+git fetch origin
+git diff origin/main...origin/<branch>
+```
+
+**Pre-push mode** — the change is staged, or committed and unpushed:
+
+```bash
+git status --porcelain          # know what is staged vs merely present
+git diff --staged               # staged but not committed
+git diff origin/main...HEAD     # committed locally, not yet pushed
+```
+
+Run all three. Which one holds the change is itself information: work that is
+neither staged nor committed is not a diff under review, it is a working tree, and
+you should say so rather than review it. Untracked files do not appear in either
+diff — `git status --porcelain` is how you notice a new file the author forgot to
+stage, and a control that exists only in an unstaged file is not going to ship.
+
+Read every hunk. Do not skim. If the diff is too large to review properly, say
+so and escalate rather than producing confident coverage of a fraction of it.
+
+### 3. Iron laws — any violation blocks
+
+1. **Secrets.** No credential, token or key in the diff, in a test fixture, or
+   rendered into output by a command the change adds. A `kubectl get secret -o
+   yaml`, a `sops -d` piped to stdout, or an `echo $TOKEN` is an immediate block.
+2. **Swallowed errors.** `_ = f()`, a bare `except:`, `|| true` on a line whose
+   failure matters, `err` returned naked.
+3. **Unvalidated input at a boundary.** External API responses, file reads, user
+   input used before checking.
+4. **Live-breaking GitOps changes.** For `mathias/infra`: does this leave the
+   cluster reconcilable? A `kubectl diff` that is clean is **not** proof Flux
+   will accept it — server-side apply does not prune fields it does not own, and
+   that took the whole `apps` Kustomization down on 2026-09-16.
+
+### 4. The test is the claim — attack it
+
+This estate's recurring defect is **a check that passes without proving
+anything**. Confirmed instances to pattern-match against:
+
+- a suite grading a subprocess's stdout passed when the subprocess crashed:
+  empty output, zero assertions, `fail` stayed 0
+- a gate mapped 11 of 18 subjects and *printed* the other seven, then exited 0
+- a manifest check matched `type: Recreate` inside a **comment** explaining why
+  Recreate had been rejected
+
+So, for every test the PR adds or changes:
+
+- **Flip it.** Break the thing it claims to check and run it. If it still
+  passes, the test is decoration. Say so and block. On `dispatch#75` this found
+  **three behavioural changes shipped with no assertion at all** — correct code
+  that nothing would have noticed the removal of.
+- **Read the name against the assertion.** A test whose name claims more than
+  it proves is worse than no test, because the name wins in the reader's head
+  and the gap becomes invisible. Found three times in one PR, twice in tests
+  written to fix the previous instance. `DoesNotInheritEnv` is honest;
+  `IsContained` is a claim the test cannot support. (local-dev `AGENT.md` rule 9.)
+- Does it fail when it cannot run? An empty result, a parser that matches
+  nothing, a missing file must all be failures, never silent passes.
+- Does it assert on what the machine reads, or on prose near it?
+- Is coverage total, or a majority? Partial coverage must be expressed as a
+  failure or an explicit, justified allowlist.
+
+A PR with no test for a behavioural change is a block unless the change is
+purely declarative config, and even then ask what would catch its regression.
+
+### 4b. How to attack — the three habits that actually found the escapes
+
+Five rounds on `dispatch#75` produced five real escapes. **CI was green on every
+one of them.** These are what found them; reading the diff did not.
+
+**Verify by execution, not by reading the implementation.** Build a scratch
+repo, run the command, look at the filesystem. Every finding came from a
+command that ran, not from reasoning about a matcher. The matcher said one
+thing and the filesystem said another, five times running. A claim you have not
+executed is a hypothesis.
+
+**Read the survivors.** After any allow/deny sweep, read the list of what is
+still *permitted*, one entry at a time. This is what caught `git commit -F
+/etc/passwd` — reads an arbitrary file into a commit message that is then
+pushed, i.e. exfiltration wearing a message flag. It was found by neither the
+review nor the differential, because both implementations **agreed** and both
+were wrong. Agreement is not correctness.
+
+**Grade a tool on its exit code, never on grepping its output.** A crashed
+linter greps as clean. `golangci-lint` exits 0 clean / 1 issues found / >1 the
+linter itself failed — and a harness that matched its summary text scored a
+crash as a pass, leaving the only gate over an entire package silently absent.
+
+Where a control has **two implementations** kept in sync by hand, run one case
+list through both and diff the verdicts mechanically. A case one refuses and
+the other allows is a real finding, and this has caught one.
+
+### 4c. Probing without causing the incident you are testing for
+
+**This agent has caused a credential leak.** On 2026-09-19 a probe testing
+environment scrubbing printed `/proc/$PPID/environ` and `ps e` raw, and a `git
+credential fill` consulted the host's real store. Seven live credentials went
+into the transcript and had to be rotated (`infra#445`). The finding was
+correct; the method was not.
+
+Non-negotiable when probing anything credential-adjacent:
+
+- **A probe that reads an environment prints a count, never the buffer.**
+  Grep for a sentinel and report whether it was found. A leak test that prints
+  what it found defeats itself.
+- **Use an invented sentinel**, never a real secret name. Set it yourself, then
+  assert on its absence.
+- **Anything touching git's credential subsystem runs with an isolated `HOME`**
+  and `GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM` pointed away from the real ones.
+  Otherwise it reads the host's store, which is the point of the subsystem.
+- Prove a primitive and stop. Do not aim a demonstrated write or execution
+  primitive at a real secret to show it works.
+
+A command whose *purpose* is to surface credentials will surface real ones if
+you point it at a real environment.
+
+### 5. CI
+
+```
+mcp__plugin_skills_gitea__pr_get          # mergeable, conflicts
+mcp__plugin_skills_gitea__workflow_run_list
+```
+
+CI green is necessary and not sufficient — you have just spent step 4
+establishing what its green is worth. On `dispatch#75` it was worth nothing
+five times running: `task check` exit 0, `golangci-lint` exit 0, every test
+passing, while `git rm ././.dispatch-allow` deleted the file the control
+existed to protect. Say so in your report when it applies: *green, and green
+covers none of these findings.*
+
+CI **red or absent** is an automatic block: an agent-opened PR with zero runs
+is a known platform failure mode in this estate, not a pass.
+
+**Pre-push mode: CI cannot have run**, because nothing has been pushed. The only
+signal is the author's local gate, and the absence of CI is not a finding against
+the change — it is a fact about the mode. Two consequences:
+
+- Do not block for "CI absent" here. That rule exists for a *pushed* branch with no
+  runs, which is a platform failure; an unpushed diff having no runs is arithmetic.
+- Verify the local gate the way you would verify any claim: run it yourself. Check
+  the **exit code**, not filtered output — `task check; echo $?` — because a grep
+  over a gate's output has hidden a failing test in this estate before. "The author
+  says it is green" is the belief you are testing.
+
+So in pre-push mode your report carries strictly less external evidence than in PR
+mode, and must say so. The verdict is "nothing I could find blocks this push", never
+"CI confirms this".
+
+### 6. Verdict
+
+**PR mode — merge** only when all hold:
+
+- no iron-law violation
+- every "done when" clause in the issue is satisfied
+- CI ran and is green
+- you flipped the tests and they went red
+- the change is `risk/low` or `risk/medium`
+
+Then:
+
+```
+mcp__plugin_skills_gitea__pr_merge        # squash
+mcp__plugin_skills_gitea__issue_comment   # the evidence, on the ISSUE
+mcp__plugin_skills_gitea__issue_close
+mcp__plugin_skills_gitea__branch_delete
+```
+
+**Pre-push mode — clear to push** only when all hold:
+
+- no iron-law violation
+- every "done when" clause in the issue is satisfied
+- you ran the local gate yourself and checked its exit code
+- you flipped the tests and they went red
+- the change is `risk/low` or `risk/medium`
+
+**There is nothing to merge and no PR to close, so do not merge, do not close the
+issue, and do not touch a branch.** Your entire output is a comment:
+
+```
+mcp__plugin_skills_gitea__issue_comment   # the verdict, on the ISSUE
+```
+
+**The verdict goes in the issue.** That is not a stylistic preference — it is what
+makes the lane a *recorded* review pass rather than a conversation. A verdict that
+exists only in a chat transcript is indistinguishable from no review having
+happened, and the rule that created this mode says exactly that: no recorded
+verdict means the lane was not used.
+
+Head the comment so it can be found by grep, state which mode you ran in, and say
+plainly that CI had not run at the time you reviewed. The author pushes; you do not.
+
+For `mathias/infra`, after the author has pushed:
+
+```bash
+flux reconcile kustomization apps --with-source
+flux get kustomization apps
+```
+
+If it does not go `Ready: True`, **revert immediately** rather than
+forward-fixing, and report. A blocked `apps` Kustomization stops every app in
+the cluster, not just the changed one.
+
+Put your evidence on the **issue**, not only the PR thread. In Gitea a merge
+auto-closes the gated issue and review comments live on a thread separate from
+the issue's — so a finding recorded only on the PR is a finding nobody reading
+the issue will ever see.
+
+### 7. Escalate to Mathias — and only then
+
+Do not merge. Comment the finding on the issue and report it. Escalate when:
+
+- the change is `risk/high`
+- it touches host networking, iptables, `ip rule`, Tailscale, or UFW on koala
+- it needs `sudo` (denied outright to agents on koala)
+- it writes or rotates a secret
+- it deletes data, a PVC, or a repo
+- a finding is real and the fix is a judgement call, not a correction
+- the diff is too large to review honestly
+
+Escalation is cheap. A bad merge on a GitOps cluster is not.
+
+## Output
+
+Report back, in this order:
+
+1. **VERDICT** — `MERGED` / `BLOCKED` / `ESCALATED`
+2. **Findings**, most severe first, each as `path:line — what is wrong — what
+   happens because of it`
+3. **Flip-test results** — what you broke, and whether the test noticed
+4. **CI** — run id and conclusion, or "absent"
+5. **What you did not check**, explicitly
+
+Never report a clean review you did not perform. "I read the diff and it looked
+fine" is not a review; naming what you attacked and what survived is.

bin/check-mcp-env

@@ -0,0 +1,99 @@
+#!/bin/sh
+# check-mcp-env — assert every variable a live MCP server interpolates is one
+# the session env-file actually defines.
+#
+# Why this is a gate and not a comment: the failure is silent. A header of
+# "Bearer ${FOO}" where FOO is undefined does not error -- it goes out empty,
+# the server answers 401, and Claude Code reports "requires authentication",
+# which reads exactly like a revoked credential. On 2026-09-07 that cost a
+# full investigation on koala before the credential turned out to be fine.
+#
+# Servers under `_parked` are exempt by design. Parked means "reachable, but
+# nothing puts its token in the environment yet" -- flagging those would
+# report the intended state as a fault.
+#
+# Usage:  check-mcp-env <mcp.json> <env-file>
+#
+# Exit 0  every live server's variables are defined
+#      1  at least one is not, named on stderr
+#      2  could not verify (bad usage, unreadable input, jq absent).
+#         Deliberately distinct from 0: an unverified config is not a clean
+#         one, and a gate that cannot run should say so rather than pass.
+
+set -u
+
+CONFIG=${1:-}
+ENVFILE=${2:-}
+
+if [ -z "$CONFIG" ] || [ -z "$ENVFILE" ]; then
+  echo "usage: check-mcp-env <mcp.json> <env-file>" >&2
+  exit 2
+fi
+
+if ! command -v jq >/dev/null 2>&1; then
+  echo "SKIP: jq not installed -- MCP env wiring unverified" >&2
+  exit 2
+fi
+
+if [ ! -f "$CONFIG" ]; then
+  echo "ERROR: no such config: $CONFIG" >&2
+  exit 2
+fi
+
+if [ ! -f "$ENVFILE" ]; then
+  # EXPECTED IN CI, and permanently so. The env-file holds live credentials, so
+  # it is not on the runner and must not be -- which means this check can only
+  # ever verify on a host that has one. infra#368 added jq to the runner image
+  # believing that would let CI verify; it removed the jq branch below and
+  # changed nothing else, because this branch is the binding one.
+  #
+  # Said plainly so nobody concludes CI covers this. It does not, it cannot,
+  # and putting ~/.op-env on the runner to make it would be trading a real
+  # secret for a green tick.
+  echo "ERROR: no such env-file: $ENVFILE -- cannot verify MCP variables" >&2
+  echo "       (expected in CI: this check only verifies on a host with ~/.op-env)" >&2
+  exit 2
+fi
+
+# Every "<server>\t<VAR>" pair a live server interpolates. `..` walks the whole
+# value, so a variable is found wherever it is placed, not only under headers.
+refs=$(jq -r '
+  .mcpServers // {}
+  | to_entries[]
+  | .key as $server
+  | (.value | .. | strings)
+  | select(test("\\$\\{"))
+  | [ scan("\\$\\{([A-Za-z_][A-Za-z0-9_]*)\\}")[] ]
+  | .[]
+  | "\($server)\t\(.)"
+' "$CONFIG" 2>/dev/null) || {
+  echo "ERROR: $CONFIG is not valid JSON -- MCP env wiring unverified" >&2
+  exit 2
+}
+
+if [ -z "$refs" ]; then
+  echo "check-mcp-env: no interpolated variables in live servers"
+  exit 0
+fi
+
+fail=0
+checked=0
+for pair in $(printf '%s\n' "$refs" | tr '\t' ':'); do
+  server=${pair%%:*}
+  var=${pair#*:}
+  checked=$((checked + 1))
+  if grep -qE "^[[:space:]]*(export[[:space:]]+)?$var=" "$ENVFILE"; then
+    continue
+  fi
+  printf 'ERROR: MCP server %s interpolates %s, which %s does not define.\n' \
+    "$server" "$var" "$ENVFILE" >&2
+  printf '       It will send an empty Authorization header and report as "requires authentication".\n' >&2
+  fail=1
+done
+
+if [ "$fail" -ne 0 ]; then
+  exit 1
+fi
+
+printf 'check-mcp-env: %d interpolated variable(s) across live MCP servers, all defined in %s\n' \
+  "$checked" "$ENVFILE"

bin/doctor

@@ -327,6 +327,67 @@ if [ "$ONLINE" -eq 1 ]; then
   fi
 fi
 
+# --- toolchain: golangci-lint must be built with Go >= GOROOT ---------------
+# infra#375, second recurrence. golangci-lint embeds the go/types export-data
+# reader from the Go it was BUILT with. When GOROOT is newer, that reader
+# cannot decode the stdlib and the linter dies:
+#
+#   panic: file requires newer Go version go1.27 (application built with go1.26)
+#
+# An earlier form printed a typecheck error naming the user's own file, which
+# sends people to debug code that is fine. It cost time in two repos before
+# anyone recognised it as a host fault rather than a repo fault.
+#
+# It is invisible to CI -- the runner image has matching versions -- so it
+# breaks exactly the local `task check` that AGENT.md requires before a push,
+# and nothing else. That is why it belongs in doctor: the signal was missing,
+# not the fix.
+#
+# Fails open on anything unparseable. A host with no Go toolchain is a normal
+# host, not a broken one, and doctor runs in a SessionStart hook everywhere.
+ver_lt() {
+  # 0 when $1 < $2 on major.minor. Deliberately not sort -V: this is POSIX sh
+  # that also runs on macOS, and the export-data format is minor-versioned, so
+  # patch level does not enter into it.
+  _a=${1%%.*}; _ar=${1#*.}; _am=${_ar%%.*}
+  _b=${2%%.*}; _br=${2#*.}; _bm=${_br%%.*}
+  case "$_a$_am$_b$_bm" in *[!0-9]*) return 1 ;; esac
+  [ "$_a" -lt "$_b" ] && return 0
+  [ "$_a" -gt "$_b" ] && return 1
+  [ "$_am" -lt "$_bm" ]
+}
+
+# --- agents shipped by the plugin, shadowed by host-local copies -------------
+#
+# skills#51: pr-gatekeeper lived ONLY at ~/.claude/agents/pr-gatekeeper.md for
+# weeks — unversioned, on one host, while doctrine came to depend on it
+# (local-dev AGENT.md rule 8). Now that the plugin ships agents/, a real file of
+# the same name in ~/.claude/agents takes precedence over the shipped one and
+# nothing says so. That is the same silent-drift shape as the `outside` and
+# `unmanaged` checks above: it works, so nothing complains, while serving
+# content nobody updates.
+CLAUDE_AGENTS="$HOME/.claude/agents"
+if [ -d "$CHECKOUT/agents" ]; then
+  for _shipped in "$CHECKOUT"/agents/*.md; do
+    [ -e "$_shipped" ] || continue
+    _name=$(basename -- "$_shipped")
+    _local="$CLAUDE_AGENTS/$_name"
+    [ -e "$_local" ] || continue
+    if [ -L "$_local" ]; then
+      continue        # a link is managed wiring, not a shadow
+    fi
+    add FAIL shadow "$_local is a host-local copy of the plugin's agents/$_name — it wins over the shipped one, so the versioned file is not what runs; delete it"
+  done
+fi
+
+if command -v golangci-lint >/dev/null 2>&1 && command -v go >/dev/null 2>&1; then
+  lint_go=$(golangci-lint --version 2>/dev/null | sed -n 's/.*built with go\([0-9][0-9.]*\).*/\1/p' | head -1)
+  root_go=$(go version 2>/dev/null | sed -n 's/.*go version go\([0-9][0-9.]*\).*/\1/p' | head -1)
+  if [ -n "$lint_go" ] && [ -n "$root_go" ] && ver_lt "$lint_go" "$root_go"; then
+    add FAIL toolchain "golangci-lint built with go$lint_go but GOROOT is go$root_go — lint panics on every Go repo; reinstall it under the current toolchain (infra#375)"
+  fi
+fi
+
 # --- output -----------------------------------------------------------------
 if [ "$JSON" -eq 1 ]; then
   printf '{"checkout":"%s","expected":%s,"plugin":{"installed":"%s","available":"%s"},"findings":[%s]}\n' \

hooks/hooks.json

@@ -33,6 +33,16 @@
             "timeout": 5
           }
         ]
+      },
+      {
+        "matcher": "mcp__.*",
+        "hooks": [
+          {
+            "type": "command",
+            "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/mcp-destructive-guard.sh\"",
+            "timeout": 5
+          }
+        ]
       }
     ]
   }

hooks/mcp-destructive-guard.sh

@@ -0,0 +1,86 @@
+#!/usr/bin/env bash
+# PreToolUse hook (mcp__.* matcher) — makes a human approve irreversible MCP
+# verbs instead of letting the model run them unattended.
+#
+# Why (infra#508, infra ADR-0027): gitea-mcp authenticates by PAT pass-through,
+# so an agent holds the full scope of the caller's PAT. repo_delete's own guard
+# (confirm == repo) is filled in by the model on both sides: it stops typos,
+# not an injected or mistaken agent. The infra-mcp README claimed client-side
+# deny lists covered this; on 2026-10-01 none existed in any settings file.
+#
+# Policy: match gitea-mcp's verb spellings on ANY server name. The same tool is
+# reachable as mcp__plugin_skills_gitea__*, mcp__claude_ai_gitea__* or under
+# whatever name a host registers it with, so keying on the server name would
+# let a rename bypass the gate. It does NOT recognise other servers' spellings
+# of the same operation (e.g. a third-party `delete_repo`).
+#
+# file_write_branch is gated when it would land on main/master (on infra that
+# is a Flux deploy within ~60s — the same effect as pr_merge) or empty a file
+# (the same effect as file_delete). Writes to other branches pass.
+#
+# Emits "ask" or nothing; never "allow" (a hook allow would override the
+# user's own deny/ask rules). Always exits 0: Claude Code ignores a hook's
+# JSON on a non-zero exit. Anything it cannot parse — including a host
+# without jq — fails closed to "ask". The ask JSON is a literal, so the
+# fail-closed path needs no tool that might be missing.
+#
+# Scope limit, recorded in infra ADR-0027: this gates MCP calls only. A
+# session with the PAT in its environment can still reach the Gitea REST API
+# through a shell.
+
+set -u
+
+DESTRUCTIVE='repo_delete|file_delete|branch_delete|pr_merge|tbd_ship|repo_update|repo_mirror_push|workflow_run_trigger|tag_create|release_create'
+
+ask() {
+  # $1 must contain only [A-Za-z0-9 _.:()'/-] — it is spliced into JSON verbatim.
+  printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"ask","permissionDecisionReason":"%s"}}\n' "$1"
+  exit 0
+}
+
+INPUT="$(cat)"
+
+command -v jq >/dev/null 2>&1 \
+  || ask "mcp-destructive-guard: jq is not installed, so this MCP call cannot be checked. Approve only if you expected it."
+
+if ! TOOL_NAME="$(printf '%s' "$INPUT" | jq -er '.tool_name | strings | select(length > 0)' 2>/dev/null)"; then
+  ask "mcp-destructive-guard: could not read a tool name from this call, so it cannot be shown safe. Approve only if you expected it."
+fi
+
+case "$TOOL_NAME" in
+  mcp__*__*) ;;
+  *) exit 0 ;;
+esac
+
+verb="${TOOL_NAME##*__}"
+safe_verb="$(printf '%s' "$verb" | tr -cd 'A-Za-z0-9_-')"
+
+if printf '%s' "$verb" | grep -Eqix "($DESTRUCTIVE)"; then
+  ask "Irreversible MCP verb '$safe_verb' (infra ADR-0027): needs your approval. The model filled in every argument, including any confirm field."
+fi
+
+if printf '%s' "$verb" | grep -Eqix 'file_write_branch'; then
+  # Cleared only when there is exactly ONE key that case-folds to "branch" and
+  # one to "content": gitea-mcp decodes with Go encoding/json, which matches
+  # keys case-insensitively and lets the last one win, so {"branch":"feat/x",
+  # "Branch":"main"} landed on main while a lowercase-only read saw feat/x.
+  # The branch is cleared by SHAPE (allowlist), not by a blocklist of names:
+  # "refs/heads/main" created a second ref that wedged API writes to main.
+  verdict="$(printf '%s' "$INPUT" | jq -r '
+      .tool_input as $i
+      | if ($i | type) != "object" then "ask" else
+          ([$i | to_entries[] | select(.key | ascii_downcase == "branch")]) as $b
+          | ([$i | to_entries[] | select(.key | ascii_downcase == "content")]) as $c
+          | if ($b | length) != 1 or ($c | length) != 1 then "ask"
+            elif ($b[0].key != "branch") or ($c[0].key != "content") then "ask"
+            elif ($b[0].value | type) != "string" or ($c[0].value | type) != "string" then "ask"
+            elif ($b[0].value | test("^[A-Za-z0-9][A-Za-z0-9._/-]*$") | not) then "ask"
+            elif ($b[0].value | test("\\.\\.|^refs/|/$")) then "ask"
+            elif ($b[0].value | ascii_downcase | . == "main" or . == "master") then "ask"
+            elif ($c[0].value | test("^\\s*$")) then "ask"
+            else "pass" end
+        end' 2>/dev/null || echo ask)"
+  [ "$verdict" = "pass" ] \
+    || ask "file_write_branch to main/master, an unusual branch name, or emptying a file (infra ADR-0027): needs your approval."
+fi
+exit 0

install.sh

@@ -17,8 +17,8 @@
 #   /plugin install skills@mathias
 #
 # That path carries the hooks and MCP server definitions too, which symlinks
-# cannot. This script covers the harnesses with no plugin system: Crush, Pi
+# cannot. This script covers the harnesses with no plugin system: Crush, Pi,
-# and OpenCode.
+# OpenCode, and Antigravity.
 #
 # Wiring model — one managed directory, one link per harness:
 #
@@ -27,6 +27,13 @@
 #   ~/.config/crush/skills      -> ~/.agents/skills
 #   ~/.config/opencode/skills   -> ~/.agents/skills
 #
+# Antigravity has no symlink-directory discovery — it reads a path-entries
+# JSON instead (~/.gemini/config/skills.json:
+# {"entries":[{"path":"..."}]}). One entry points at ~/.agents/skills, so it
+# sees exactly the same surfaces-filtered, collision-resolved set every other
+# harness does. Requires jq; skipped with a warning if jq is unavailable —
+# every other target in this script stays dependency-free on purpose.
+#
 # ~/.agents/skills was itself a link to this checkout until #27. That encoded
 # an assumption nobody had stated — that this checkout is the ONLY source of
 # skills on the host — and on flamingo the only way to install this library
@@ -53,7 +60,12 @@ CHECKOUT_DIR="${SKILLS_CHECKOUT_DIR:-$HOME/.local/share/skills}"
 AGENTS_DIR="${AGENTS_SKILLS_DIR:-$HOME/.agents/skills}"
 CRUSH_DIR="${CRUSH_SKILLS_DIR:-$HOME/.config/crush/skills}"
 OPENCODE_DIR="${OPENCODE_SKILLS_DIR:-$HOME/.config/opencode/skills}"
+ANTIGRAVITY_SKILLS_JSON="${ANTIGRAVITY_SKILLS_JSON:-$HOME/.gemini/config/skills.json}"
 EXTRA_SOURCES="${SKILLS_EXTRA_SOURCES:-}"
+# Overridable so the jq-absent branch is testable on a host that has jq. That
+# branch is a contract, not a fallback: jq is optional here while every other
+# target in this script is dependency-free.
+JQ="${SKILLS_JQ:-jq}"
 # Recorded beside the managed directory rather than inside it: a stray file in
 # there would look like a skill to every harness reading the dir.
 SOURCES_MANIFEST="$(dirname "$AGENTS_DIR")/$(basename "$AGENTS_DIR")-sources"
@@ -213,6 +225,42 @@ link_dir() {
   log "linked $link → $target"
 }
 
+# wire_antigravity — register $AGENTS_DIR in Antigravity's skills.json.
+# Merges rather than overwrites: skills.json is a documented user-editable
+# config (json_configs.md), so a hand-added entry (e.g. a personal skills
+# dir) must survive a re-run untouched.
+wire_antigravity() {
+  if ! command -v "$JQ" >/dev/null 2>&1; then
+    log "Antigravity: jq not found — skipping (needed to merge $ANTIGRAVITY_SKILLS_JSON safely)"
+    return
+  fi
+
+  local existing already updated
+  mkdir -p "$(dirname "$ANTIGRAVITY_SKILLS_JSON")"
+
+  if [ -s "$ANTIGRAVITY_SKILLS_JSON" ]; then
+    existing="$(cat "$ANTIGRAVITY_SKILLS_JSON")"
+  else
+    existing='{}'
+  fi
+
+  already="$(printf '%s' "$existing" | "$JQ" --arg p "$AGENTS_DIR" \
+    '[.entries // [] | .[] | select(.path == $p)] | length' 2>/dev/null)" || {
+    log "Antigravity: $ANTIGRAVITY_SKILLS_JSON is not valid JSON — leaving it untouched"
+    return 1
+  }
+
+  if [ "$already" -gt 0 ]; then
+    log "Antigravity: $AGENTS_DIR already registered in $ANTIGRAVITY_SKILLS_JSON"
+    return
+  fi
+
+  updated="$(printf '%s' "$existing" | "$JQ" --arg p "$AGENTS_DIR" \
+    '.entries = ((.entries // []) + [{path: $p}])')"
+  printf '%s\n' "$updated" > "$ANTIGRAVITY_SKILLS_JSON"
+  log "Antigravity: registered $AGENTS_DIR in $ANTIGRAVITY_SKILLS_JSON"
+}
+
 main() {
   ensure_checkout
 
@@ -224,6 +272,7 @@ main() {
 
   link_dir "$CRUSH_DIR" "$AGENTS_DIR"
   link_dir "$OPENCODE_DIR" "$AGENTS_DIR"
+  wire_antigravity
 
   log "done — $LINKED_COUNT skill(s) linked from $(sources | wc -l | tr -d ' ') source(s), ref=$REF"
   log "Claude Code: install the plugin instead — /plugin marketplace add $REPO_URL"

skills/adr/SKILL.md

@@ -58,8 +58,8 @@ adr:           0004
 title:         <the decision, as a sentence — not "database choice">
 status:        proposed | accepted | superseded | amended
 date:          2026-08-29
-deciders:      <who accepted it, by name — or "pending: <name>" while proposed>
+deciders:      <who accepted it, by name — or "pending: <name>" (QUOTED) while proposed>
-supersedes:    <adr number, or ->
+supersedes:    <adr number, or null>
 ---
 
 # ADR-0004 — <same title>
@@ -97,6 +97,25 @@ the condition that fires a revisit. An ADR with no validation section is a prefe
 with a template around it.
 ```
 
+### The frontmatter must actually parse
+
+It exists to be machine-read, so a block that does not load is worse than no block at
+all: it drops that ADR silently out of every "what is still open?" query, while looking
+correct. Two shapes this template used to suggest are invalid YAML, and both were found
+live in the estate on 2026-09-05 — `openbanking-api` ADR-0001 and `infra` ADR-0019, each
+copied verbatim from here:
+
+| Wrong | Why | Right |
+|---|---|---|
+| `supersedes:    -` | a bare dash is a YAML *sequence entry*, not a string | `supersedes:    null` |
+| `deciders:      pending: Mathias` | the second, unquoted colon reads as a nested mapping | `deciders:      "pending: Mathias"` |
+
+Check before committing:
+
+```sh
+python3 -c "import yaml,sys;yaml.safe_load(open(sys.argv[1]).read().split('---')[1]);print('ok')" docs/adr/0004-*.md
+```
+
 ## Preserving a productive tension
 
 Some tradeoffs are not unresolved because nobody thought hard enough. They are

skills/document-to-markdown/SKILL.md

@@ -0,0 +1,126 @@
+---
+name: document-to-markdown
+surfaces: [claude-code, library]
+summary: "Read .docx/.xlsx/.pptx/.epub/.msg and other formats the Read tool cannot open"
+use_when: "You need the contents of a binary document — Word, Excel, PowerPoint, EPUB, Outlook .msg, a zip of documents — and Read returns nothing useful"
+description: Convert a binary document to Markdown locally with markitdown. No network, no LLM, safe for confidential material. Says when to use the docmark service instead.
+---
+
+# Document → Markdown
+
+## Overview
+
+`Read` opens PDFs, images and notebooks natively. It does **not** open `.docx`,
+`.xlsx`, `.pptx`, `.epub`, `.msg` or `.zip`. This skill converts those locally.
+
+One command, no install step, no service:
+
+```bash
+uvx --from 'markitdown[docx,pdf,pptx,xls,xlsx,outlook]' markitdown FILE > /tmp/out.md
+```
+
+First run downloads 38 packages (~10s); afterwards it is cached and instant.
+
+## When to use this
+
+You have a file on disk whose contents you need. That is the whole trigger.
+
+## When NOT to use this — use `docmark` instead
+
+Three cases, and only three:
+
+1. **A scanned PDF or an image whose content is pixels, not text.** Extracting
+   that needs a vision model. The estate's only audited path from an image to a
+   model is `docmark`, whose `sovereignty.py` constructs an LLM client *only*
+   when `DOCMARK_VISION_MODEL` matches `koala/*` or `iguana/*`, and silently
+   degrades to metadata-only otherwise. `tests/test_sovereignty.py` is the
+   load-bearing guard. Do not reach for a cloud vision model directly; that is
+   the boundary the gate exists to hold.
+2. **You are writing an in-cluster service** (something in `dispatch`,
+   `ingestion`, `cobalt-dingo`). Call `docmark` over MCP rather than bundling a
+   converter into each service. Its NetworkPolicy already admits `supervisor`
+   and `dispatch`.
+3. **The file is larger than ~25 MiB** or you want the input/output caps
+   enforced centrally.
+
+For a text-bearing document, the service returns **byte-identical output** to
+the local command — measured 2026-09-24 on the same `.docx`, tables included.
+A network hop, a bearer token and an ingress for the same bytes is not a
+trade worth making.
+
+## Confidential material
+
+**Conversion is local and involves no LLM.** `markitdown` uses an LLM client
+only to describe images; with none configured it does not call anything. No
+bytes leave the machine.
+
+That makes this safe for client material, unlike anything that routes through a
+model. Two cautions:
+
+- **Do not install `markitdown[all]`.** It pulls `az-doc-intel` and
+  `az-content-understanding` — Azure cloud document-intelligence converters.
+  They need explicit endpoint configuration to activate, but the extras list
+  above avoids having them on the path at all.
+- The output is a file like any other. If the source was confidential, so is
+  `/tmp/out.md` — and the usual rule applies: it must not reach an open-egress
+  sink (ADR-0014).
+
+## Verify the conversion, do not assume it
+
+The failure mode is **a successful exit with empty output**. A `.docx` template
+with no body content converts to zero characters and exits 0; so does a scanned
+PDF with no text layer. Both look like success.
+
+```bash
+uvx --from 'markitdown[docx,pdf,pptx,xls,xlsx,outlook]' markitdown FILE > /tmp/out.md
+wc -c /tmp/out.md
+```
+
+If the byte count is 0 or implausibly small for the document's size, the
+converter found no text layer — that is case 1 above, and it needs `docmark`
+with a vision model, not a retry.
+
+## What it converts
+
+18 converters with the extras above:
+
+```
+Audio, BingSerp, Csv, Docx, Epub, Html, Image, Ipynb, OutlookMsg, Pdf,
+PlainText, Pptx, Rss, Wikipedia, Xls, Xlsx, YouTube, Zip
+```
+
+`docmark` registers 22 — the same set plus `DocxWithOCR`, `PdfWithOCR`,
+`PptxWithOCR`, `XlsxWithOCR`. **Those four are the entire difference**, and
+they are exactly the vision path from case 1. Everything else is identical code
+running in a different place.
+
+## Reading the output
+
+Convert to a file and `Read` it, rather than piping into the transcript —
+a 200-page document produces a lot of Markdown and most of it will not be
+what you needed. Convert, then grep or read the section you want.
+
+## Provenance
+
+`docmark` ADR-0013 (`mathias/infra docs/decisions/`) owns the service; this
+skill is the local path. The split was decided in `mathias/docmark#3` on
+measurement, not preference: three of docmark's four plausible consumers are
+better served without it, and the fourth — vision under a sovereignty gate — is
+why it exists.
+
+## Escalation
+
+Local, read-only, single command. Nothing here is handed back mid-task.
+
+Stop and hand back when:
+
+- **The output is empty or near-empty.** Do not retry, and do not guess at the
+  contents. Say which file, what the byte count was, and that it needs a vision
+  path — then either use `docmark` with a local vision model, or ask. A
+  confident summary of a document you could not read is the worst possible
+  output of this skill.
+- **The document is client material and the task would send its contents
+  anywhere** — a commit message, an issue, a PR body, a cloud model. That is
+  ADR-0014's refusal, and it is not this skill's call to make.
+- **The file is encrypted or password-protected.** markitdown will fail; asking
+  for the password is a human's decision, not a retry.

tests/agents_test.sh

@@ -0,0 +1,164 @@
+#!/bin/sh
+# Tests for the agents/ directory the plugin ships, and doctor's shadow check.
+#
+# skills#51: pr-gatekeeper blocked dispatch#75 five times, every block a real
+# escape verified by execution — and the agent definition lived only at
+# ~/.claude/agents/pr-gatekeeper.md on one host. Unversioned, unreviewable, one
+# rm from gone. Doctrine now depends on it (local-dev AGENT.md rule 8), so the
+# file has to be in the repo.
+#
+# These assertions observe FILE CONTENT and doctor's OUTPUT. They cannot and do
+# not claim the agent reviews well — naming them otherwise would be the exact
+# defect rule 9 exists for.
+
+set -u
+
+ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
+AGENT="$ROOT/agents/pr-gatekeeper.md"
+DOCTOR="$ROOT/bin/doctor"
+
+pass=0
+fail=0
+WORK=${TMPDIR:-/tmp}/agents_test.$$
+
+ok()  { pass=$((pass + 1)); printf 'ok: %s\n' "$1"; }
+no()  { fail=$((fail + 1)); printf 'FAIL: %s\n' "$1"; }
+
+# --- the file is in the repo ------------------------------------------------
+
+if [ -f "$AGENT" ]; then
+  ok 'agents/pr-gatekeeper.md exists in the checkout'
+else
+  no 'agents/pr-gatekeeper.md exists in the checkout'
+  printf '\n%s passed, %s failed\n' "$pass" "$fail"
+  exit 1
+fi
+
+# --- frontmatter is present and well-formed ---------------------------------
+
+head -1 "$AGENT" | grep -q '^---$' \
+  && ok 'the file opens with a frontmatter fence' \
+  || no 'the file opens with a frontmatter fence'
+
+for field in name description tools; do
+  if sed -n '2,/^---$/p' "$AGENT" | grep -q "^$field:"; then
+    ok "frontmatter declares $field"
+  else
+    no "frontmatter declares $field"
+  fi
+done
+
+fm_name=$(sed -n '2,/^---$/p' "$AGENT" | sed -n 's/^name: *//p' | head -1)
+if [ "$fm_name" = "pr-gatekeeper" ]; then
+  ok 'the frontmatter name equals the filename stem'
+else
+  no "the frontmatter name equals the filename stem (got '$fm_name')"
+fi
+
+# --- the practices that actually found the defects --------------------------
+#
+# Each string below is traceable to a specific round of dispatch#75 or to the
+# incident the agent itself caused. A grep is a weak check — it proves the
+# phrase is present, not that the procedure is followed — so the test is named
+# for the phrase, not for the behaviour.
+
+check_phrase() {
+  if grep -qi -- "$1" "$AGENT"; then
+    ok "the file names: $2"
+  else
+    no "the file names: $2"
+  fi
+}
+
+check_phrase 'verify by execution'        'verify by execution, not by reading the implementation'
+check_phrase 'flip'                       'flip-testing'
+check_phrase 'read the survivors'         'read the survivors of a differential'
+check_phrase 'exit code'                  'grade a linter on its exit code'
+check_phrase 'never the buffer'           'a probe prints a count, never the buffer'
+check_phrase 'claims more than'           'a test name must not claim more than it proves'
+
+# --- doctor flags a host-local copy that shadows the shipped one ------------
+
+setup() {
+  rm -rf "$WORK"
+  mkdir -p "$WORK/home/.claude/agents" "$WORK/ck/skills/x" "$WORK/ck/agents"
+  printf 'x\n' > "$WORK/ck/skills/x/SKILL.md"
+  printf -- '---\nname: pr-gatekeeper\n---\n' > "$WORK/ck/agents/pr-gatekeeper.md"
+}
+
+setup
+printf -- '---\nname: pr-gatekeeper\n---\n' > "$WORK/home/.claude/agents/pr-gatekeeper.md"
+out=$(HOME="$WORK/home" DOCTOR_CHECKOUT="$WORK/ck" sh "$DOCTOR" 2>&1)
+if printf '%s' "$out" | grep -q 'pr-gatekeeper'; then
+  ok 'doctor reports a host-local agent that shadows a shipped one'
+else
+  no "doctor reports a host-local agent that shadows a shipped one (got: $out)"
+fi
+
+setup
+out=$(HOME="$WORK/home" DOCTOR_CHECKOUT="$WORK/ck" sh "$DOCTOR" 2>&1)
+if printf '%s' "$out" | grep -q 'pr-gatekeeper'; then
+  no "doctor is silent when no host-local copy exists (got: $out)"
+else
+  ok 'doctor is silent when no host-local copy exists'
+fi
+
+# --- the pre-push mode the GitOps review lane depends on (infra#470) ---------
+#
+# infra#470 resolved a doctrine conflict: root rule 5 wants a PR for risk/medium+,
+# and mathias/infra forbids branches because Flux watches main and a branch can be
+# linted but never reconciled. The agreed lane is that such work stays on main and
+# instead gets a RECORDED ADVERSARIAL REVIEW PASS before the push.
+#
+# That lane names this agent as its reviewer. If the definition only knows how to
+# review an open PR, the lane resolves to "be careful" — which is what rule 5 was
+# written to replace. These assertions observe FILE CONTENT only: that the mode is
+# documented, that it does not merge, and that the verdict has a stated home. They
+# do NOT claim the agent reviews a staged diff well (rule 9).
+
+if grep -qi 'git diff --staged\|git diff --cached' "$AGENT"; then
+  ok 'the definition names a command that reads a staged diff, not only a branch diff'
+else
+  no 'the definition has no staged-diff command, so the infra#470 lane has no reviewer'
+fi
+
+if grep -qi 'pre-push' "$AGENT"; then
+  ok 'the definition names the pre-push mode'
+else
+  no 'the definition does not name a pre-push mode (infra#470)'
+fi
+
+# The verdict has to land somewhere durable, or "recorded review pass" is untrue.
+# The lane specifies the issue, deliberately not a PR thread — there is no PR.
+if grep -qi 'verdict in the issue\|verdict goes in the issue' "$AGENT"; then
+  ok 'the definition states the verdict goes in the issue'
+else
+  no 'the definition does not say where a pre-push verdict is recorded'
+fi
+
+# In pre-push mode there is nothing to merge and no PR to close. An agent that
+# reaches for pr_merge there would be acting on a PR that does not exist.
+#
+# SCOPED to the pre-push section on purpose. An unscoped grep for "do not merge"
+# passes on line 202 of the current file, which is the ESCALATION step talking
+# about a PR — a pass for entirely the wrong reason, and the kind of vacuous
+# assertion rule 9 is about. Caught by checking what the green was matching.
+PREPUSH=$(awk '/[Pp]re-push/{f=1} f&&/^## [^#]/&&!/[Pp]re-push/{f=0} f' "$AGENT")
+if printf '%s' "$PREPUSH" | grep -qi 'nothing to merge\|do not merge\|no PR to merge'; then
+  ok 'the pre-push section itself rules out merging'
+else
+  no 'the pre-push section does not rule out merging (an unscoped grep would pass here wrongly)'
+fi
+
+# CI cannot have run on an unpushed diff, so the CI step's meaning changes. A
+# definition that still treats "CI green" as available would grade on a signal
+# that does not exist yet.
+if grep -qi 'CI has not run\|no CI run yet\|CI cannot have run' "$AGENT"; then
+  ok 'the definition acknowledges CI has not run on an unpushed diff'
+else
+  no 'the definition does not address CI being absent pre-push'
+fi
+
+rm -rf "$WORK"
+printf '\n%s passed, %s failed\n' "$pass" "$fail"
+[ "$fail" -eq 0 ]

tests/check_tolerates_unverifiable_test.sh

@@ -0,0 +1,70 @@
+#!/bin/sh
+# `task check` must survive a check that reports "could not verify".
+#
+# The regression these exist for: bin/check-mcp-env exits 2 to mean "I could
+# not run — no jq, or no env-file", which the gate deliberately treats as a
+# reported NOTE rather than a failure. The Taskfile said exactly that:
+#
+#   "$DIR/check-mcp-env" "$DIR/.mcp.json" "$HOME/.op-env"
+#   case $? in 0) ;; 2) echo "NOTE: ..." ;; *) fail=1 ;; esac
+#
+# and it did not work. Task runs each cmds block under `sh -e`, so a bare
+# command exiting non-zero aborts the shell *before* `case $?` is ever
+# reached. The whole gate died with `exit status 2`. Locally the check found
+# jq and exited 0, so it went green; CI has neither jq nor ~/.op-env, so main
+# went red on the very commit that added it (skills run #57, 2026-09-07).
+#
+# A tolerated exit code that is only tolerated on the host where it never
+# fires is not tolerated at all. This pins the tolerance itself.
+#
+# No mocking: pointing HOME at an empty directory removes ~/.op-env, which is
+# exactly the condition CI runs under, so check-mcp-env genuinely exits 2.
+
+set -u
+
+ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
+
+pass=0
+fail=0
+
+ok() { pass=$((pass + 1)); printf '  ok   %s\n' "$1"; }
+no() { fail=$((fail + 1)); printf '  FAIL %s\n' "$1"; }
+
+if ! command -v task >/dev/null 2>&1; then
+  printf '  SKIP: task not installed -- cannot exercise the gate\n'
+  exit 0
+fi
+
+TMP=$(mktemp -d)
+trap 'rm -rf "$TMP"' EXIT
+
+# Sanity: the precondition must actually hold, or the test proves nothing.
+# An empty HOME means no ~/.op-env, so check-mcp-env cannot verify anything.
+"$ROOT/bin/check-mcp-env" "$ROOT/.mcp.json" "$TMP/.op-env" >/dev/null 2>&1
+rc=$?
+if [ "$rc" -eq 2 ]; then
+  ok 'precondition: check-mcp-env exits 2 when the env-file is absent'
+else
+  no "precondition: expected check-mcp-env to exit 2, got $rc"
+fi
+
+# The assertion. `task check` must still pass, and must say why it could not
+# verify -- silence would mean the check was skipped rather than tolerated.
+out=$(HOME="$TMP" task --dir "$ROOT" check 2>&1)
+rc=$?
+
+if [ "$rc" -eq 0 ]; then
+  ok 'task check passes when MCP env wiring cannot be verified'
+else
+  no "task check exited $rc when check-mcp-env reported unverifiable"
+  printf '%s\n' "$out" | sed 's/^/       | /' | tail -8
+fi
+
+if printf '%s' "$out" | grep -q 'unverified'; then
+  ok 'the unverifiable check is reported, not silently skipped'
+else
+  no 'no NOTE reported -- an unverifiable check must never look like a pass'
+fi
+
+printf '  %d passed, %d failed\n' "$pass" "$fail"
+[ "$fail" -eq 0 ]

tests/ci_gate_test.sh

@@ -53,5 +53,31 @@ printf '%s' "$tag_job" | grep -qE '^ +needs: *gate' \
   && ok 'ungated release.yml removed' \
   || no 'ungated release.yml removed'
 
+# skills#46. The manifest-bump commit is created and pushed by the `tag` job
+# using secrets.GITEA_TOKEN, and a push authenticated with the Actions token
+# does not trigger workflows -- deliberate platform behaviour to stop CI
+# recursing. So the ONE commit in this repo that no CI run has ever seen is the
+# one the release job writes, every release:
+#
+#   6cc224f  chore(release): plugin.json version 0.19.4   ->  0 runs
+#   f031c2b  chore(release): plugin.json version 0.19.5   ->  0 runs
+#
+# The koala cluster's CIHeadUncovered{repo=skills} alert had been firing on it
+# for 85 hours when this was found, and it cannot clear on its own -- only an
+# unrelated human push moves main's HEAD to a tested commit.
+#
+# The commit is not nothing: plugin.json's version is what Claude Code gates
+# plugin updates on, `task check` asserts it is not behind the latest tag, and
+# the sed-and-retry logic that writes it already shipped a mismatched manifest
+# once (v0.10.0 with the manifest reading 0.9.0).
+#
+# Anchored past comments: the tag job's prose already SAYS "`task check`
+# asserts the manifest is not behind the latest tag", and a bare grep matches
+# that and passes. A grep for a concept matches the rationale, and the more
+# carefully a thing is documented the more likely that is.
+printf '%s' "$tag_job" | grep -qE '^[^#]*task check' \
+  && ok 'the release-bump commit is gated before it is pushed' \
+  || no 'the release-bump commit is pushed with no gate -- nothing ever tests it (#46)'
+
 printf '\nci_gate: %d passed, %d failed\n' "$pass" "$fail"
 [ "$fail" -eq 0 ]

tests/doctor_test.sh

@@ -24,6 +24,18 @@ if [ ! -f "$DOCTOR" ]; then
   exit 1
 fi
 
+# The toolchain check (golangci-lint build-Go vs GOROOT) reads the real host
+# unless we say otherwise, which made every unrelated fixture fail on a machine
+# whose golangci-lint is genuinely stale. Give the whole suite a matching fake
+# pair so only the cases that OPT IN exercise that check.
+STUB_BIN="$WORK/_stub_bin"
+mkdir -p "$STUB_BIN"
+printf '#!/bin/sh\nprintf "golangci-lint has version 2.0.0 built with go1.27.0 from abc\\n"\n' > "$STUB_BIN/golangci-lint"
+printf '#!/bin/sh\ncase "$1" in version) printf "go version go1.27.0 linux/amd64\\n";; *) printf "\\n";; esac\n' > "$STUB_BIN/go"
+chmod +x "$STUB_BIN/golangci-lint" "$STUB_BIN/go"
+PATH="$STUB_BIN:$PATH"
+export PATH
+
 cleanup() { rm -rf "$WORK"; }
 trap cleanup EXIT INT TERM
 
@@ -291,5 +303,75 @@ else
   fail=$((fail + 1)); printf '  FAIL --online reports an unreachable remote as its own fault\n       rc=%s got: %s\n' "$_rc" "$_out"
 fi
 
+# --- toolchain: golangci-lint must be built with Go >= GOROOT ---------------
+# Measured on koala 2026-09-08 (infra#375, second recurrence): golangci-lint
+# 2.11.4 built with go1.26.1 against a go1.27.0 GOROOT PANICS on any Go repo --
+#
+#   panic: file requires newer Go version go1.27 (application built with go1.26)
+#
+# and the earlier form of the same fault printed a typecheck error naming YOUR
+# file, so the natural response is to debug code that is fine. It cost time in
+# two repos. CI never sees it (the runner image matches), so it breaks only the
+# local `task check` that AGENT.md requires before every push.
+#
+# The fixtures inject fake `go` and `golangci-lint` on PATH; nothing here reads
+# the real host toolchain, so the suite still passes on a machine without Go.
+mk_toolchain() { # $1=fixture  $2=lint-build-go  $3=goroot-go   ("" = absent)
+  _b="$1/bin"; mkdir -p "$_b"
+  if [ -n "$2" ]; then
+    printf '#!/bin/sh\nprintf "golangci-lint has version 2.11.4 built with %s from abc on 2026-01-01\\n"\n' "$2" > "$_b/golangci-lint"
+    chmod +x "$_b/golangci-lint"
+  fi
+  if [ -n "$3" ]; then
+    printf '#!/bin/sh\ncase "$1" in version) printf "go version %s linux/amd64\\n";; *) printf "\\n";; esac\n' "$3" > "$_b/go"
+    chmod +x "$_b/go"
+  fi
+  printf '%s' "$_b"
+}
+
+f=$(new_fixture golangci_old); wire_clean "$f"
+b=$(mk_toolchain "$f" go1.26.1 go1.27.0)
+_out=$(PATH="$b:$PATH" HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" "$DOCTOR" 2>&1); _rc=$?
+if [ "$_rc" -ne 0 ] && printf '%s' "$_out" | grep -qi 'golangci'; then
+  pass=$((pass + 1)); printf '  ok   golangci-lint older than GOROOT is reported\n'
+else
+  fail=$((fail + 1)); printf '  FAIL golangci-lint older than GOROOT is reported\n       rc=%s got: %s\n' "$_rc" "$_out"
+fi
+
+# Matching versions are the normal case and must stay silent.
+f=$(new_fixture golangci_ok); wire_clean "$f"
+b=$(mk_toolchain "$f" go1.27.0 go1.27.0)
+_out=$(PATH="$b:$PATH" HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" "$DOCTOR" 2>&1); _rc=$?
+if [ "$_rc" -eq 0 ]; then
+  pass=$((pass + 1)); printf '  ok   golangci-lint matching GOROOT is silent\n'
+else
+  fail=$((fail + 1)); printf '  FAIL golangci-lint matching GOROOT is silent\n       rc=%s got: %s\n' "$_rc" "$_out"
+fi
+
+# A NEWER lint than GOROOT is fine -- the constraint is one-directional.
+f=$(new_fixture golangci_newer); wire_clean "$f"
+b=$(mk_toolchain "$f" go1.28.0 go1.27.0)
+_out=$(PATH="$b:$PATH" HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" "$DOCTOR" 2>&1); _rc=$?
+if [ "$_rc" -eq 0 ]; then
+  pass=$((pass + 1)); printf '  ok   golangci-lint newer than GOROOT is not a fault\n'
+else
+  fail=$((fail + 1)); printf '  FAIL golangci-lint newer than GOROOT is not a fault\n       rc=%s got: %s\n' "$_rc" "$_out"
+fi
+
+# Fail OPEN on anything unparseable. doctor runs in a SessionStart hook on every
+# machine; a toolchain it cannot read is not a fault it may assert. (A host with
+# no Go at all takes the same path -- command -v simply misses.)
+f=$(new_fixture golangci_garbled); wire_clean "$f"
+_b="$f/bin"; mkdir -p "$_b"
+printf '#!/bin/sh\nprintf "golangci-lint dev build\\n"\n' > "$_b/golangci-lint"
+printf '#!/bin/sh\nprintf "go version devel\\n"\n' > "$_b/go"
+chmod +x "$_b/golangci-lint" "$_b/go"
+_out=$(PATH="$_b:$PATH" HOME="$f/home" DOCTOR_CHECKOUT="$f/ck" "$DOCTOR" 2>&1); _rc=$?
+if [ "$_rc" -eq 0 ]; then
+  pass=$((pass + 1)); printf '  ok   an unreadable toolchain version is not asserted as a fault\n'
+else
+  fail=$((fail + 1)); printf '  FAIL an unreadable toolchain version is not asserted as a fault\n       rc=%s got: %s\n' "$_rc" "$_out"
+fi
+
 printf '\ndoctor: %d passed, %d failed\n' "$pass" "$fail"
 [ "$fail" -eq 0 ]

tests/install_test.sh

@@ -17,6 +17,7 @@ SUT="$ROOT/install.sh"
 
 pass=0
 fail=0
+skipped=0
 WORK=${TMPDIR:-/tmp}/install_test.$$
 
 if [ ! -f "$SUT" ]; then
@@ -55,6 +56,7 @@ run_install() {
     AGENTS_SKILLS_DIR="$_f/home/.agents/skills" \
     CRUSH_SKILLS_DIR="$_f/home/.config/crush/skills" \
     OPENCODE_SKILLS_DIR="$_f/home/.config/opencode/skills" \
+    ANTIGRAVITY_SKILLS_JSON="$_f/home/.gemini/config/skills.json" \
     SKILLS_EXTRA_SOURCES="$_extra" \
     bash "$SUT" 2>&1)
   rc=$?
@@ -156,6 +158,40 @@ else
   no 'configured sources are recorded for doctor' "$out"
 fi
 
+# --- Antigravity: jq is optional, so BOTH branches must be tested -----------
+# These three cases inherited whatever jq the host happened to have. koala has
+# it, the CI runner does not, so they passed locally and failed in CI — the
+# classic works-on-my-machine gate. install.sh treats jq as optional by design,
+# which means the skip is a contract and needs a test of its own.
+
+# run_install_nojq <fixture> — same as run_install, with the jq lookup pointed
+# at a binary that does not exist. Deterministic regardless of the host.
+run_install_nojq() {
+  _f=$1
+  _out=$(HOME="$_f/home" \
+    SKILLS_SKIP_FETCH=1 \
+    SKILLS_CHECKOUT_DIR="$_f/ck" \
+    AGENTS_SKILLS_DIR="$_f/home/.agents/skills" \
+    CRUSH_SKILLS_DIR="$_f/home/.config/crush/skills" \
+    OPENCODE_SKILLS_DIR="$_f/home/.config/opencode/skills" \
+    SKILLS_JQ="definitely-not-jq-$$" \
+    bash "$SUT" 2>&1)
+  rc=$?
+  printf '%s' "$_out"
+}
+
+f=$(new_fixture antigravity_nojq)
+out=$(run_install_nojq "$f")
+if [ -e "$f/home/.gemini/config/skills.json" ]; then
+  no 'without jq, no skills.json is written' "$out"
+elif ! printf '%s' "$out" | grep -qi 'jq not found'; then
+  no 'without jq, the skip is reported' "$out"
+elif [ ! -L "$f/home/.agents/skills/alpha" ]; then
+  no 'without jq, the other harnesses are still wired' "$out"
+else
+  ok 'without jq, Antigravity is skipped and the rest still wires'
+fi
+
 # --- 8. surfaces: the library link set is not "every skill" -----------------
 # claude.ai has no shell, no cluster and no checkout, so a chat-only skill
 # linked into Crush is a skill that can never fire, competing for selection
@@ -183,5 +219,59 @@ else
   no 'a source with no surfaces convention is linked as-is' "$out"
 fi
 
-printf '\ninstall: %d passed, %d failed\n' "$pass" "$fail"
+# --- 9-11. Antigravity's merge behaviour, only where jq exists ---------------
+# These three assert what jq DOES. install.sh treats jq as optional, so on a
+# host without it there is nothing to assert — and failing there is what turned
+# main red: they passed on koala (jq present) and failed on the runner (absent).
+# A skipped case is honest; a case that fails because the host lacks an optional
+# dependency is a false red, and false reds are how a suite stops being read.
+if ! command -v jq >/dev/null 2>&1; then
+  skipped=$((skipped + 3))
+  printf '  SKIP jq absent — Antigravity merge behaviour not covered on this host (3 cases)\n'
+else
+  # skills.json entries must point at $AGENTS_DIR — that carries the
+  # surfaces-filtering, collision handling and multi-source merge. Pointing at
+  # the checkout directly would give Antigravity a different skill set than
+  # every other harness.
+  f=$(new_fixture antigravity)
+  out=$(run_install "$f")
+  AG="$f/home/.gemini/config/skills.json"
+  if [ ! -f "$AG" ]; then
+    no 'Antigravity skills.json is created' "$out"
+  else
+    registered=$(jq -r --arg p "$f/home/.agents/skills" '[.entries[]? | select(.path == $p)] | length' "$AG")
+    if [ "$registered" = "1" ]; then
+      ok 'Antigravity registers the managed agents dir, not the raw checkout'
+    else
+      no 'Antigravity registers the managed agents dir, not the raw checkout' "$(cat "$AG")"
+    fi
+  fi
+
+  f=$(new_fixture antigravity_idempotent)
+  run_install "$f" >/dev/null
+  out=$(run_install "$f")
+  AG="$f/home/.gemini/config/skills.json"
+  count=$(jq '.entries | length' "$AG" 2>/dev/null || echo "?")
+  if [ "$count" = "1" ]; then
+    ok 'Antigravity wiring is idempotent (no duplicate entries)'
+  else
+    no 'Antigravity wiring is idempotent (no duplicate entries)' "entries=$count"$'\n'"$out"
+  fi
+
+  # A user may hand-edit skills.json to point at their own personal skills dir
+  # (json_configs.md documents this). Our merge must not clobber it.
+  f=$(new_fixture antigravity_preserve)
+  mkdir -p "$f/home/.gemini/config"
+  printf '{"entries":[{"path":"/Users/mathias/personal-skills"}]}' > "$f/home/.gemini/config/skills.json"
+  out=$(run_install "$f")
+  AG="$f/home/.gemini/config/skills.json"
+  personal=$(jq -r '[.entries[]? | select(.path == "/Users/mathias/personal-skills")] | length' "$AG" 2>/dev/null || echo 0)
+  if [ "$personal" = "1" ]; then
+    ok 'Antigravity wiring preserves pre-existing hand-authored entries'
+  else
+    no 'Antigravity wiring preserves pre-existing hand-authored entries' "$(cat "$AG")"$'\n'"$out"
+  fi
+fi
+
+printf '\ninstall: %d passed, %d failed, %d skipped\n' "$pass" "$fail" "$skipped"
 [ "$fail" -eq 0 ]

tests/mcp_config_test.sh

@@ -0,0 +1,118 @@
+#!/bin/sh
+# Tests for bin/check-mcp-env.
+#
+# The fault this gate exists for is silent, which is why it needs a gate at
+# all. An `.mcp.json` server whose Authorization header interpolates a
+# variable nothing in the session environment defines does not error: the
+# header goes out empty, the server answers 401, and Claude Code reports
+# "requires authentication". That is indistinguishable from a revoked or
+# expired credential, and on 2026-09-07 it cost a full investigation on koala
+# to establish that the credential was fine and the variable was simply never
+# set.
+#
+# So the property is: every ${VAR} a LIVE server interpolates must be a
+# variable the session env-file actually defines.
+#
+# The `_parked` block is deliberately exempt. Parked servers reference
+# variables nothing provides on purpose -- that is what parked means -- and a
+# gate that flagged them would be reporting the intended state as a fault.
+#
+# Fixtures drive the checker through argv, so no test touches the real host.
+
+set -u
+
+ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
+CHECK="$ROOT/bin/check-mcp-env"
+
+pass=0
+fail=0
+WORK=${TMPDIR:-/tmp}/mcp_config_test.$$
+
+if [ ! -f "$CHECK" ]; then
+  printf 'FAIL: %s does not exist\n' "$CHECK"
+  exit 1
+fi
+
+cleanup() { rm -rf "$WORK"; }
+trap cleanup EXIT INT TERM
+mkdir -p "$WORK"
+
+if ! command -v jq >/dev/null 2>&1; then
+  printf 'SKIP: jq not installed -- bin/check-mcp-env cannot be exercised\n'
+  exit 0
+fi
+
+# ok <name> / bad <name> <detail>
+ok()  { pass=$((pass+1)); printf '  ok   %s\n' "$1"; }
+bad() { fail=$((fail+1)); printf '  FAIL %s  %s\n' "$1" "${2:-}"; }
+
+# run <mcp-json> <env-file> -> sets $rc and $out
+run() {
+  out=$("$CHECK" "$1" "$2" 2>&1); rc=$?
+}
+
+ENVFILE="$WORK/op-env"
+cat > "$ENVFILE" <<'ENVEOF'
+BRAIN_MCP_TOKEN=op://HomeLab/brain/token
+DMABE_GITEA_API_TOKEN=op://HomeLab/gitea/token
+ENVEOF
+
+# --- a live server whose variable IS defined ---------------------------------
+cat > "$WORK/good.json" <<'JSONEOF'
+{"mcpServers":{"brain":{"type":"http","url":"https://x/mcp",
+ "headers":{"Authorization":"Bearer ${BRAIN_MCP_TOKEN}"}}}}
+JSONEOF
+run "$WORK/good.json" "$ENVFILE"
+[ "$rc" -eq 0 ] && ok "defined variable passes" || bad "defined variable passes" "rc=$rc $out"
+
+# --- a live server whose variable is NOT defined ------------------------------
+# The exact shape that broke: the server looks correct and fails at runtime.
+cat > "$WORK/missing.json" <<'JSONEOF'
+{"mcpServers":{"gitea":{"type":"http","url":"https://x/mcp",
+ "headers":{"Authorization":"Bearer ${GITEA_MCP_STATIC_TOKEN}"}}}}
+JSONEOF
+run "$WORK/missing.json" "$ENVFILE"
+if [ "$rc" -ne 0 ]; then
+  case "$out" in
+    *gitea*GITEA_MCP_STATIC_TOKEN*) ok "undefined variable is flagged, naming server and variable" ;;
+    *) bad "undefined variable is flagged, naming server and variable" "report did not name both: $out" ;;
+  esac
+else
+  bad "undefined variable is flagged, naming server and variable" "rc=0"
+fi
+
+# --- a server with no headers at all (the `infra` shape) ----------------------
+# Unauthenticated by design. Nothing to resolve, so nothing to flag.
+cat > "$WORK/noauth.json" <<'JSONEOF'
+{"mcpServers":{"infra":{"type":"http","url":"https://x/mcp"}}}
+JSONEOF
+run "$WORK/noauth.json" "$ENVFILE"
+[ "$rc" -eq 0 ] && ok "header-less server is not flagged" || bad "header-less server is not flagged" "rc=$rc $out"
+
+# --- parked servers are exempt ------------------------------------------------
+cat > "$WORK/parked.json" <<'JSONEOF'
+{"mcpServers":{"brain":{"type":"http","url":"https://x/mcp",
+  "headers":{"Authorization":"Bearer ${BRAIN_MCP_TOKEN}"}}},
+ "_parked":{"_reason":"no identity provides these yet",
+  "cad":{"type":"http","url":"https://x/mcp",
+   "headers":{"Authorization":"Bearer ${CAD_MCP_TOKEN}"}}}}
+JSONEOF
+run "$WORK/parked.json" "$ENVFILE"
+if [ "$rc" -eq 0 ]; then
+  ok "parked server is exempt"
+else
+  bad "parked server is exempt" "rc=$rc $out"
+fi
+
+# --- malformed JSON must not pass silently ------------------------------------
+# A config the checker cannot read is an unverified config, not a clean one.
+printf '{"mcpServers":{' > "$WORK/broken.json"
+run "$WORK/broken.json" "$ENVFILE"
+[ "$rc" -ne 0 ] && ok "unparseable config does not report success" || bad "unparseable config does not report success" "rc=0"
+
+# --- a missing env-file is unverifiable, not clean ----------------------------
+run "$WORK/good.json" "$WORK/no-such-env"
+[ "$rc" -ne 0 ] && ok "missing env-file does not report success" || bad "missing env-file does not report success" "rc=0"
+
+printf '\n%s: %d passed, %d failed\n' "$(basename "$0")" "$pass" "$fail"
+[ "$fail" -eq 0 ]

tests/mcp_destructive_guard_test.sh

@@ -0,0 +1,118 @@
+#!/bin/sh
+# Tests for hooks/mcp-destructive-guard.sh — the PreToolUse gate that makes a
+# human approve irreversible MCP verbs (infra#508, infra ADR-0027).
+#
+# Contract: reads PreToolUse hook JSON on stdin, ALWAYS exits 0, and answers
+# in hookSpecificOutput.permissionDecision. "ask" = a human must approve;
+# empty stdout ("NONE") = no opinion, normal permission rules apply. The guard
+# never emits "allow": an allow from a hook would bypass the user's own rules.
+#
+# Cases marked (gatekeeper) are escapes found by the skills#52 review: each
+# passed the first version of this suite.
+
+set -u
+
+ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
+GUARD="$ROOT/hooks/mcp-destructive-guard.sh"
+
+pass=0
+fail=0
+
+# run_guard <json> [PATH] — invoked exactly as hooks.json does: `bash <path>`.
+# Prints "<decision> rc=<exit>", decision NONE for an empty answer.
+run_guard() {
+  if [ $# -ge 2 ]; then
+    _out=$(printf '%s' "$1" | PATH="$2" "$BASH_BIN" "$GUARD"); _rc=$?
+  else
+    _out=$(printf '%s' "$1" | bash "$GUARD"); _rc=$?
+  fi
+  if [ -z "$_out" ]; then _d=NONE
+  else _d=$(printf '%s' "$_out" | jq -r '.hookSpecificOutput.permissionDecision // "NONE"' 2>/dev/null || echo BROKEN)
+  fi
+  printf '%s rc=%s' "$_d" "$_rc"
+}
+BASH_BIN=$(command -v bash)
+
+check() {
+  _want=$1 _json=$2 _label=$3
+  _got=$(run_guard "$_json")
+  if [ "$_got" = "$_want rc=0" ]; then
+    pass=$((pass + 1)); printf '  ok   %s\n' "$_label"
+  else
+    fail=$((fail + 1)); printf '  FAIL %s\n       want=%s rc=0 got=%s\n       in=%s\n' "$_label" "$_want" "$_got" "$_json"
+  fi
+}
+tool() { printf '{"tool_name":"%s","tool_input":{"owner":"mathias","repo":"x"}}' "$1"; }
+
+echo "mcp-destructive-guard tests"
+
+for v in repo_delete file_delete branch_delete pr_merge tbd_ship repo_update repo_mirror_push \
+         workflow_run_trigger tag_create release_create; do
+  check ask "$(tool "mcp__plugin_skills_gitea__$v")" "plugin gitea $v asks"
+done
+
+# The verb is matched on any server name: connector, bare, or renamed.
+check ask "$(tool mcp__claude_ai_gitea__repo_delete)" "connector namespace asks"
+check ask "$(tool mcp__gitea__repo_delete)"           "bare server name asks"
+check ask "$(tool mcp__git__repo_delete)"             "renamed server still asks"
+check ask "$(tool mcp__plugin_skills_gitea__REPO_DELETE)" "uppercase verb asks"
+# The verb is the LAST segment (gatekeeper: parsing the second segment stayed green).
+check ask  "$(tool mcp__a__b__repo_delete)" "verb taken from last segment"
+check NONE "$(tool mcp__repo_delete__issue_get)" "server named like a verb does not trigger"
+
+# file_write_branch: content landing on main, or emptying a file, is the same
+# effect as the gated pr_merge / file_delete (gatekeeper).
+fwb() { printf '{"tool_name":"mcp__plugin_skills_gitea__file_write_branch","tool_input":{"branch":"%s","content":"%s"}}' "$1" "$2"; }
+check ask  "$(fwb main x)"     "file_write_branch to main asks"
+check ask  "$(fwb master x)"   "file_write_branch to master asks"
+check ask  "$(fwb feat/y '')"  "file_write_branch emptying a file asks"
+check ask  '{"tool_name":"mcp__plugin_skills_gitea__file_write_branch","tool_input":{"content":"x"}}' "file_write_branch without branch asks"
+check NONE "$(fwb feat/y x)"   "file_write_branch to a feature branch passes"
+
+# Go's encoding/json matches keys case-insensitively, last one wins: gitea-mcp
+# wrote to main from {"branch":"feat/y8","Branch":"main"} while the hook read
+# only `branch` (gatekeeper round 2, verified against a real Gitea). So the
+# guard clears a write only when exactly one key case-folds to branch/content.
+FWB=mcp__plugin_skills_gitea__file_write_branch
+check ask  "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y8\",\"Branch\":\"main\",\"content\":\"x\"}}" "case-variant Branch key asks"
+check ask  "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y\",\"content\":\"x\",\"Content\":\"\"}}" "case-variant Content key asks"
+check ask  "{\"tool_name\":\"$FWB\",\"tool_input\":{\"BRANCH\":\"feat/y\",\"content\":\"x\"}}" "only an uppercase BRANCH key asks"
+check ask  "$(fwb Main x)"  "file_write_branch to Main asks"
+check ask  "{\"tool_name\":\"$FWB\",\"tool_input\":{\"branch\":\"feat/y\"}}" "missing content key asks"
+check ask  "$(fwb feat/y ' ')" "whitespace-only content asks"
+check ask  "{\"tool_name\":\"$FWB\",\"tool_input\":\"branch=feat/y\"}" "non-object tool_input asks"
+# Branch names are cleared by shape, not by blocklist: refs/heads/main created a
+# second ref that wedged every API write to main (gatekeeper side finding).
+check ask  "$(fwb refs/heads/main x)" "refs/ prefixed branch asks"
+check ask  "$(fwb ' main' x)"         "branch with whitespace asks"
+check ask  "$(fwb '../main' x)"       "branch with .. asks"
+check NONE "$(fwb release/v1.2_x x)"  "ordinary branch name passes"
+
+# Reversible / read verbs pass through.
+for v in issue_close issue_create file_read repo_list pr_create; do
+  check NONE "$(tool "mcp__plugin_skills_gitea__$v")" "$v passes through"
+done
+check NONE "$(tool mcp__plugin_skills_gitea__repo_delete_dry_run_list)" "suffix near-miss passes"
+check NONE "$(tool Bash)" "non-MCP tool passes"
+check NONE "$(tool repo_delete)" "non-MCP tool named like a verb passes"
+
+# Malformed input fails CLOSED (gatekeeper: non-string tool_name got NONE).
+check ask 'not json'                        "non-JSON asks"
+check ask '{}'                              "missing tool_name asks"
+check ask '{"tool_name":null}'              "null tool_name asks"
+check ask '{"tool_name":""}'                "empty tool_name asks"
+check ask '{"tool_name":5}'                 "number tool_name asks"
+check ask '{"tool_name":["mcp__x__repo_delete"]}' "array tool_name asks"
+check ask '{"tool_name":{"a":1}}'           "object tool_name asks"
+
+# Without jq on PATH the guard must still answer ask, not stay silent
+# (gatekeeper: the fail-closed path itself needed jq).
+nojq=$(mktemp -d); trap 'rm -rf "$nojq"' EXIT
+for b in bash cat grep tr sed; do ln -s "$(command -v $b)" "$nojq/$b"; done
+_got=$(run_guard "$(tool mcp__plugin_skills_gitea__repo_delete)" "$nojq")
+if [ "$_got" = "ask rc=0" ]; then pass=$((pass+1)); echo "  ok   no jq on PATH still asks"; else fail=$((fail+1)); echo "  FAIL no jq on PATH: got=$_got"; fi
+_got=$(run_guard "$(tool mcp__plugin_skills_gitea__issue_get)" "$nojq")
+if [ "$_got" = "ask rc=0" ]; then pass=$((pass+1)); echo "  ok   no jq on PATH asks even for a read verb (cannot parse = cannot clear)"; else fail=$((fail+1)); echo "  FAIL no jq read verb: got=$_got"; fi
+
+printf '\n%d passed, %d failed\n' "$pass" "$fail"
+[ "$fail" -eq 0 ]

tests/pre_task_ritual_test.sh

@@ -0,0 +1,103 @@
+#!/bin/sh
+# Tests for hooks/pre-task-ritual.sh — the UserPromptSubmit hook that surfaces
+# rule 0 on every prompt (#44).
+#
+# WHY THIS EXISTS, AND WHAT IT DELIBERATELY DOES NOT CLAIM
+# This hook is three lines and echoes a string. It has no bash-isms, so it
+# cannot fail the way secret-guard appeared to; that risk was overstated when
+# this test was requested. What it CAN do is stop working silently, and its
+# whole job is to be visible -- if the text stops appearing, rule 0 stops being
+# surfaced on every prompt and nothing announces that.
+#
+# THE ASSERTION THAT MATTERS IS REGISTRATION.
+# A hook file that exists but is not in hooks.json does absolutely nothing, and
+# looks identical on disk to one that works. So this derives the command from
+# hooks.json rather than assuming one: the registration is the source of truth
+# for how the hook is invoked, and a test that invokes it its own way is
+# testing something the harness never runs.
+#
+# That distinction is not hypothetical. secret_guard_test.sh piped its hook into
+# `sh` while hooks.json runs it with `bash`; on Debian /bin/sh is dash, which
+# has no `set -o pipefail`, and the suite failed 20/20 in CI against a hook that
+# was fine in production. The test was wrong, not the hook. Deriving the
+# invocation removes the opportunity for that class of mistake entirely.
+
+set -u
+
+ROOT=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)
+HOOK="$ROOT/hooks/pre-task-ritual.sh"
+REG="$ROOT/hooks/hooks.json"
+
+pass=0
+fail=0
+ok() { pass=$((pass + 1)); printf '  ok   %s\n' "$1"; }
+no() { fail=$((fail + 1)); printf '  FAIL %s\n' "$1"; }
+
+if ! command -v jq >/dev/null 2>&1; then
+  printf '  SKIP: jq not installed -- cannot read the hook registration\n'
+  exit 0
+fi
+
+[ -f "$HOOK" ] || { printf '  FAIL %s does not exist\n' "$HOOK"; exit 1; }
+ok 'hook file exists'
+
+[ -f "$REG" ] || { printf '  FAIL %s does not exist\n' "$REG"; exit 1; }
+
+# 1. Registered, and on the right event. UserPromptSubmit is what makes it fire
+#    on every prompt; registered under any other event it would still "exist"
+#    and still do nothing useful.
+CMD=$(jq -r '
+  .hooks.UserPromptSubmit[]?.hooks[]?
+  | select(.command | test("pre-task-ritual"))
+  | .command
+' "$REG" 2>/dev/null | head -1)
+
+if [ -n "$CMD" ]; then
+  ok 'registered on UserPromptSubmit in hooks.json'
+else
+  no 'not registered on UserPromptSubmit -- the file would exist and never run'
+  printf '  %d passed, %d failed\n' "$pass" "$fail"
+  exit 1
+fi
+
+# 2. Run it EXACTLY as registered, with the harness variable substituted the way
+#    the harness substitutes it.
+OUT=$(CLAUDE_PLUGIN_ROOT="$ROOT" sh -c "$CMD" 2>&1)
+rc=$?
+
+[ "$rc" -eq 0 ] && ok 'exits 0 as registered' \
+  || no "exited $rc -- a non-zero UserPromptSubmit hook can block the prompt"
+
+# 3. The content is the point. A hook that runs and says nothing is the same as
+#    no hook, and that is the failure this pins.
+if [ -n "$OUT" ]; then
+  ok 'produces output'
+else
+  no 'produced no output -- rule 0 would silently stop being surfaced'
+fi
+
+case "$OUT" in
+  *"Rule 0"*) ok 'names rule 0' ;;
+  *)          no "does not name rule 0: '$OUT'" ;;
+esac
+
+# Each of the four steps, individually. Naming rule 0 while having quietly lost
+# a step is the plausible drift, not losing the whole line.
+for step in brain_query skill 'failing test' 'success criterion'; do
+  case "$OUT" in
+    *"$step"*) ok "states the '$step' step" ;;
+    *)         no "lost the '$step' step from the ritual" ;;
+  esac
+done
+
+# 4. The interpreter named in the registration must actually exist, or the hook
+#    fails on that host with a message nobody reads.
+INTERP=${CMD%% *}
+if command -v "$INTERP" >/dev/null 2>&1; then
+  ok "registered interpreter '$INTERP' is available"
+else
+  no "registered interpreter '$INTERP' not found on this host"
+fi
+
+printf '  %d passed, %d failed\n' "$pass" "$fail"
+[ "$fail" -eq 0 ]

tests/secret_guard_test.sh

@@ -24,7 +24,7 @@ decision_for() {
   _tool=${2:-Bash}
   jq -n --arg t "$_tool" --arg c "$_cmd" \
     '{tool_name: $t, tool_input: {command: $c}}' \
-    | sh "$GUARD" \
+    | "$GUARD" \
     | jq -r '.hookSpecificOutput.permissionDecision // "MISSING"'
 }
 
@@ -42,6 +42,20 @@ expect() {
   fi
 }
 
+# Invoked as a PROGRAM above, never as `sh "$GUARD"`. Claude Code executes the
+# hook directly, so its `#!/usr/bin/env bash` shebang decides the interpreter;
+# piping it into `sh` overrides that and tests something production never runs.
+#
+# It is not academic. On Debian /bin/sh is dash, which has no `set -o
+# pipefail`, so the guard died on line 23 before reading a byte of input and
+# every deny case returned empty -- 20 of 20 assertions failed in CI while
+# passing on a host where /bin/sh happens to be bash. A security gate that
+# tests green and cannot start is the worst of both.
+if [ ! -x "$GUARD" ]; then
+  printf 'FAIL: %s is not executable -- it must run via its own shebang\n' "$GUARD"
+  exit 1
+fi
+
 if [ ! -f "$GUARD" ]; then
   printf 'FAIL: %s does not exist\n' "$GUARD"
   exit 1