diff --git a/.github/risk.yml b/.github/risk.yml index 55c65a6..d431130 100644 --- a/.github/risk.yml +++ b/.github/risk.yml @@ -18,6 +18,8 @@ generated: [] # T2-hard: nothing lowers it. hard: - .github/risk.yml + # The R0 and R2 rules of the review: a PR must not loosen them as docs. + - RISK.md - .github/scripts/risk_tier.py - .github/scripts/general_review.py - .github/workflows/review.yml diff --git a/.github/scripts/test_risk_md.py b/.github/scripts/test_risk_md.py new file mode 100644 index 0000000..ec08e33 --- /dev/null +++ b/.github/scripts/test_risk_md.py @@ -0,0 +1,47 @@ +"""Tests of RISK.md: the R0 and R2 globs that set the review level of a PR. + + uv run --no-project --with pyyaml --with pytest -- python -m pytest -q .github/scripts + +A glob that matches no file loses its level with no error, and an R2 glob over an R0 file misleads. +An R0 file that .github/risk.yml rates as docs gets the lowest CI tier. +The globs follow the rules of risk_tier.matches. +""" + +import sys +from pathlib import Path + +import pytest +import yaml + +HERE = Path(__file__).resolve().parent +ROOT = HERE.parent.parent +sys.path.insert(0, str(HERE)) + +import risk_tier as rt # noqa: E402 + +GLOBS = yaml.safe_load((ROOT / "RISK.md").read_text(encoding="utf-8").split("---\n")[1]) +FILES = rt._git("-C", str(ROOT), "ls-files").splitlines() +RULES = rt.load_rules(str(HERE.parent / "risk.yml")) + + +def test_front_matter_lists_r0_and_r2(): + assert set(GLOBS) == {"R0", "R2"} + + +@pytest.mark.parametrize("level,glob", [(level, g) for level, globs in GLOBS.items() for g in globs]) +def test_each_glob_matches_a_file(level, glob): + assert any(rt.matches(path, [glob]) for path in FILES), \ + f"{level} glob {glob} matches no file ({{, [ and ! are not supported)" + + +def test_no_file_is_r0_and_r2(): + both = [path for path in FILES if rt.matches(path, GLOBS["R0"]) and rt.matches(path, GLOBS["R2"])] + assert not both, f"in R0 and R2: {both}" + + +def test_no_r0_file_is_docs_in_risk_yml(): + # risk.yml rates a docs file T0 unless hard or soft lists it. + low = [path for path in FILES if rt.matches(path, GLOBS["R0"]) + and rt.is_docs(rt.FileChange(path=path, status="M", added=[], removed=[]), RULES) + and not rt.matches(path, RULES["hard"] + RULES["soft"])] + assert not low, f"R0 in RISK.md, docs in .github/risk.yml: {low}" diff --git a/.github/scripts/test_risk_tier.py b/.github/scripts/test_risk_tier.py index d3ced6c..4b6652d 100644 --- a/.github/scripts/test_risk_tier.py +++ b/.github/scripts/test_risk_tier.py @@ -178,7 +178,7 @@ def test_publish_on_version(monkeypatch): @pytest.mark.parametrize("path", [".github/risk.yml", "LICENSE", ".gitleaks.toml", ".githooks/pre-commit", ".github/workflows/review.yml", ".github/workflows/risk.yml", - ".github/scripts/risk_tier.py", "CODEOWNERS"]) + ".github/scripts/risk_tier.py", "CODEOWNERS", "RISK.md"]) def test_hard_paths(path): assert tier(f(path, ["x"]))["tier"] == "T2-hard" diff --git a/RISK.md b/RISK.md new file mode 100644 index 0000000..566cf2f --- /dev/null +++ b/RISK.md @@ -0,0 +1,65 @@ +--- +R0: ["runtime/**", "tests/runner.integration.mjs", "hooks/**", "hints/**", "AGENTS.md", "CLAUDE.md", "CODING_STANDARDS.md", ".claude/**", "RISK.md", ".github/risk.yml", ".github/scripts/**", ".github/workflows/**", ".githooks/**", ".claude-plugin/**", "LICENSE"] +R2: ["docs/configuration.mdx", "docs/development.mdx", "docs/functions.mdx", "docs/hints.mdx", "docs/how-it-works.mdx", "docs/installation.mdx", "docs/troubleshooting.mdx", "docs/usage.mdx", "docs/assets/**", "README.md", "CONTRIBUTING.md", "CODE_OF_CONDUCT.md", ".github/ISSUE_TEMPLATE/**", "bench/**", "scripts/projection-report.mjs"] +--- + +The review level of a PR comes from the files that it changes: + +- R0: a changed file matches an R0 glob. Gabe reviews all of the PR. +- R2: each changed file matches an R2 glob. The PR merges without a human read. +- R1: any other PR. It gets the normal review. + +The front matter on the base branch applies, so a PR cannot change its own level. + +## Critical + +- `runtime/**`, `tests/runner.integration.mjs`: the sandbox. `runtime/runner.mjs` runs the model's program, and its + `--permission` flag, network block and `MARK` protocol are the safety layers. The integration test is the only + test that proves them. +- `hooks/register.tsx`, `hooks/paths.ts`, `hooks/functions.ts`: the permission and approval path. The approval of + nested MCP calls (`program` or `per-call`), `failClosed`, the hint-file guard (`guardedDirs`), the approval ledger in + `$.store`, and the saved functions that run code. +- The rest of `hooks/**` and `hints/**`: the text that steers the model. `hooks/hints.ts` holds `approvalKey` and the + hint loader, `hooks/protocol.ts` the runner protocol, `hooks/hooks.json` and `hooks/session-start.sh` what loads and + what the model reads at start. The bundled hints steer the model in every user's session. +- `AGENTS.md`, `CLAUDE.md`, `CODING_STANDARDS.md`, `.claude/**`: the rules for the agents that write PRs here, for + example "never weaken a sandbox layer". +- `RISK.md`, `.github/risk.yml`, `.github/scripts/**`: a PR must not loosen its own review. +- `.github/workflows/**`, `.githooks/**`: a PR must not skip a check. The `risk` workflow runs on + `pull_request_target` with write tokens. +- `.claude-plugin/**`, `LICENSE`: what users install. `plugin.json` holds the version and the public defaults + (`approval`, `projectHints`, `blockDirectMcp`), `marketplace.json` the source. + +## Decisions + +- A new dependency: an npm package or an import outside the Node built-ins. The README promises "No dependencies", + and `runtime/runner.mjs` must use Node built-ins only. +- Real data in the repo: real MCP data, company names, ticket keys, user ids or tokens in tests, hints or docs. The + repo is public. + +## Safe + +- User docs: `docs/*.mdx` and `docs/assets/`, `README.md`, `CONTRIBUTING.md`, `CODE_OF_CONDUCT.md`, issue templates. + Not `docs/permissions.mdx`: it tells users which allow rules to add, so a too-wide rule widens their permissions. + Not `SECURITY.md`: it is the path to report a vulnerability. A new `docs/` page is not R2 until it is added here. + Risk that can hide: the README install lines name the marketplace, so a changed name sends users to another plugin. +- Dev tools: `bench/**`, `scripts/projection-report.mjs`. They do not ship to users. Risk that can hide: + `bench/fake-server.mjs` is the fake server that `/verify` and `tests/smoke.e2e.mjs` drive, so a change can make the + end-to-end check pass falsely. `bench/run.mjs` makes the README cost claim. + +## Weak checks + +- `hooks/register.tsx` and `tests/*.test.ts`: CI has no type check of them. `tsconfig.json` needs + `.claude-plugin/types/`, which Claude Code writes only in a logged-in session. CI checks only the pure helpers + (`tsconfig.pure.json`). Fix: a CI job with a Claude Code token secret that starts Claude Code once with + `--plugin-dir .` to write the types, then runs `npx -p typescript tsc -p tsconfig.json`. +- The real permission path: `claude plugin test .` runs the hooks against a fake host with no processes, and + `$.mcp.call` and the permission checks are fakes (`tests/fakes.ts`). Only `tests/smoke.e2e.mjs` drives real + `claude -p`, and CI never runs it. A change to `approval`, `blockDirectMcp` or the hint-file guard can pass CI and + fail in real Claude Code. Fix: a CI job with an API key secret that runs `node tests/smoke.e2e.mjs` on PRs that + change `hooks/` or `runtime/`, and in the weekly schedule. +- Two rule sets for review risk: `.github/risk.yml` (T0, T1, T2; advisory, never fails a PR) and this file (R0, R2). + `.github/scripts/test_risk_md.py` fails when an R0 file is docs (T0) in `risk.yml`. Other drift is not checked: + `risk.yml` has `hooks/**`, `hints/**` and `AGENTS.md` only as T1, and here they are R0. Fix: extend that test to + fail when an R0 file is not in `hard` or `soft` of `risk.yml`, or make `risk_tier.py` read the front matter of + this file.