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.19.4",
+  "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

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.

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

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/doctor

@@ -357,6 +357,29 @@ ver_lt() {
   [ "$_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)

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

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/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/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 ]