From 92071ecdeb563aba846adcc696e754d05b60898d Mon Sep 17 00:00:00 2001 From: DataDave-Dev <153755137+DataDave-Dev@users.noreply.github.com> Date: Thu, 2 Jul 2026 20:01:18 -0600 Subject: [PATCH] =?UTF-8?q?feat:=20becwright=20doctor=20+=20validate=20?= =?UTF-8?q?=E2=80=94=20diagnose=20the=20setup,=20lint=20the=20rules=20file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `becwright validate`: checks .bec/rules.yaml without running anything — YAML parses, no duplicate rule ids, every `becwright run ` resolves to a built-in check; warns on a files rule with no paths. Exit 0/2, consistent with the documented exit-code contract. - `becwright doctor`: one command that answers 'why isn't it working?' — rules-file findings plus hook state: becwright hook installed / foreign hook / missing; detects core.hooksPath overrides (a becwright hook in .git/hooks that git will never run), Husky (.husky/pre-commit with or without becwright) and the pre-commit framework (config with or without the becwright hook), each with the exact fix. FAIL -> exit 2, WARN -> exit 0. - git.py grows hook_state / hooks_path_override / hook_manager helpers (also the groundwork for init to stop installing a dead hook under Husky). - 20 new tests; commands documented in README(+es) and usage(+es). --- README.es.md | 2 + README.md | 2 + documentation/usage.es.md | 2 + documentation/usage.md | 2 + src/becwright/cli.py | 136 +++++++++++++++++++ src/becwright/git.py | 28 ++++ tests/test_doctor_validate.py | 242 ++++++++++++++++++++++++++++++++++ 7 files changed, 414 insertions(+) create mode 100644 tests/test_doctor_validate.py diff --git a/README.es.md b/README.es.md index 4de98bb..444b0a5 100644 --- a/README.es.md +++ b/README.es.md @@ -172,6 +172,8 @@ Referencia completa de campos: [`documentation/usage.es.md`](documentation/usage | `becwright check` | Corre las reglas sobre los archivos en staging | | `becwright check --diff ` | Corre las reglas solo sobre los archivos cambiados vs `` (para CI/PR) | | `becwright why [id]` | Muestra la intención + el por qué de las reglas — la memoria de decisiones del repo (`--json` para agentes) | +| `becwright validate` | Valida `.bec/rules.yaml` sin correr ningún check (para editores y CI) | +| `becwright doctor` | Diagnostica el setup: archivo de reglas, checks, hooks y hook managers | | `becwright search [texto]` | Lista BECs listas del catálogo incluido | | `becwright add ` | Instala una BEC del catálogo en `.bec/rules.yaml` (sin conexión) | | `becwright install` / `uninstall` | Instala / quita los hooks nativos | diff --git a/README.md b/README.md index bc7d160..32cfb32 100644 --- a/README.md +++ b/README.md @@ -168,6 +168,8 @@ Full field reference: [`documentation/usage.md`](documentation/usage.md). | `becwright check` | Runs the rules over the staged files | | `becwright check --diff ` | Runs the rules over only the files changed vs `` (for CI/PR) | | `becwright why [id]` | Shows the intent + why behind the rules — the repo's decision memory (`--json` for agents) | +| `becwright validate` | Validates `.bec/rules.yaml` without running any check (for editors and CI) | +| `becwright doctor` | Diagnoses the setup: rules file, checks, hooks and hook managers | | `becwright search [query]` | Lists ready-made BECs from the built-in catalog | | `becwright add ` | Installs a catalog BEC into `.bec/rules.yaml` (offline) | | `becwright install` / `uninstall` | Installs / removes the native hooks | diff --git a/documentation/usage.es.md b/documentation/usage.es.md index 5e2e9ae..690804f 100644 --- a/documentation/usage.es.md +++ b/documentation/usage.es.md @@ -61,6 +61,8 @@ a mano: `becwright install` más un `.bec/rules.yaml` que escribas vos.) | `becwright list` | Lista los checks incluidos | | `becwright check` | Corre las reglas sobre los archivos en staging | | `becwright check --all` | Corre las reglas sobre todo el repo (`git ls-files`) | +| `becwright validate` | Valida `.bec/rules.yaml` — YAML, ids duplicados, checks desconocidos — sin ejecutar nada | +| `becwright doctor` | Diagnostica el setup: archivo de reglas, checks, hooks y hook managers (Husky, pre-commit) | | `becwright install` | Instala el hook pre-commit | | `becwright uninstall` | Quita el hook | | `becwright export [-o archivo]` | Exporta una regla a un bundle `.bec.yaml` | diff --git a/documentation/usage.md b/documentation/usage.md index c70a39c..d83a3d1 100644 --- a/documentation/usage.md +++ b/documentation/usage.md @@ -59,6 +59,8 @@ From then on, every `git commit` runs the checks. (You can also set up by hand: | `becwright list` | List the built-in checks | | `becwright check` | Run rules over the staged files | | `becwright check --all` | Run rules over the whole repo (`git ls-files`) | +| `becwright validate` | Validate `.bec/rules.yaml` — YAML, duplicate ids, unknown checks — without running anything | +| `becwright doctor` | Diagnose the setup: rules file, checks, hooks, and hook managers (Husky, pre-commit) | | `becwright install` | Install the pre-commit hook | | `becwright uninstall` | Remove the hook | | `becwright export [-o file]` | Export a rule to a `.bec.yaml` bundle | diff --git a/src/becwright/cli.py b/src/becwright/cli.py index 359d2c3..bff85db 100644 --- a/src/becwright/cli.py +++ b/src/becwright/cli.py @@ -176,6 +176,140 @@ def _cmd_check(args: argparse.Namespace) -> int: return 0 +def _duplicate_rule_ids(rules) -> list[str]: + seen: set[str] = set() + dupes: list[str] = [] + for rule in rules: + if rule.id in seen and rule.id not in dupes: + dupes.append(rule.id) + seen.add(rule.id) + return dupes + + +def _pathless_file_rules(rules) -> list[str]: + return [r.id for r in rules if r.target == "files" and not r.paths] + + +def _cmd_validate(_: argparse.Namespace) -> int: + root = git.repo_root() + rules_path = root / ".bec" / "rules.yaml" + if not rules_path.exists(): + print(_style(f"No {rules_path}. Run `becwright init` to create one.", RED), + file=sys.stderr) + return 2 + rules = load_rules(rules_path) # RulesError propagates -> exit 2 in main() + + problems = False + for rule_id in _duplicate_rule_ids(rules): + problems = True + print(_style(f"duplicate rule id '{rule_id}' — ids must be unique.", RED), + file=sys.stderr) + unknown = _unknown_builtin_checks(rules, root) + if unknown: + problems = True + _print_unknown_checks(unknown) + if problems: + return 2 + + for rule_id in _pathless_file_rules(rules): + print(_style(f"warning: rule '{rule_id}' has no `paths` — it will never " + "match a file.", YELLOW)) + print(_style(f"OK: {len(rules)} rule(s) valid; every check resolves.", GREEN, BOLD)) + return 0 + + +# Doctor findings: (status, message). `fail` means becwright cannot enforce as +# configured (exit 2); `warn` is a gap worth fixing; `ok` is informational. +_DOCTOR_ICONS = {"ok": ("OK", GREEN), "warn": ("WARN", YELLOW), "fail": ("FAIL", RED)} + + +def _doctor_rules(root: Path) -> tuple[list, list[tuple[str, str]]]: + rules_path = root / ".bec" / "rules.yaml" + if not rules_path.exists(): + return [], [("warn", "no .bec/rules.yaml — run `becwright init` to create one.")] + try: + rules = load_rules(rules_path) + except RulesError as e: + return [], [("fail", f".bec/rules.yaml cannot be loaded: {e}")] + findings = [("ok", f".bec/rules.yaml loads: {len(rules)} rule(s).")] + for rule_id in _duplicate_rule_ids(rules): + findings.append(("fail", f"duplicate rule id '{rule_id}' — ids must be unique.")) + for rule_id, module in _unknown_builtin_checks(rules, root): + findings.append(("fail", f"rule '{rule_id}' uses '{module}', which is not a " + "built-in check (see `becwright list`).")) + for rule_id in _pathless_file_rules(rules): + findings.append(("warn", f"rule '{rule_id}' has no `paths` — it will never " + "match a file.")) + return rules, findings + + +def _doctor_precommit_hook(root: Path) -> tuple[str, str]: + manager = git.hook_manager(root) + override = git.hooks_path_override(root) + if override: + if manager == "husky": + husky_hook = root / ".husky" / "pre-commit" + if husky_hook.is_file() and "becwright" in husky_hook.read_text(encoding="utf-8"): + return "ok", "Husky runs becwright on pre-commit." + return "warn", ("Husky owns the hooks (core.hooksPath) but .husky/pre-commit " + "does not run becwright — add `npx becwright check` to it.") + return "warn", (f"core.hooksPath = {override}: git ignores .git/hooks, so a " + "becwright hook there never runs — wire `becwright check` into " + "that hook path instead.") + state = git.hook_state(root, "pre-commit") + if state == "becwright": + return "ok", "becwright pre-commit hook installed." + if state == "foreign": + if manager == "pre-commit": + config = (root / ".pre-commit-config.yaml").read_text(encoding="utf-8") + if "becwright" in config: + return "ok", "the pre-commit framework runs becwright." + return "warn", ("the pre-commit framework owns the hook but its config does " + "not include becwright — add the becwright hook to " + ".pre-commit-config.yaml.") + return "warn", ("a non-becwright pre-commit hook exists — add `becwright check` " + "to it, or let your hook manager run becwright.") + return "warn", "no pre-commit hook — run `becwright install` (or wire becwright into your hook manager)." + + +def _doctor_msg_hook(root: Path, rules) -> tuple[str, str] | None: + if not any(r.target == "commit-msg" for r in rules): + return None + if git.hooks_path_override(root): + return None # already flagged by the pre-commit finding + state = git.hook_state(root, "commit-msg") + if state == "becwright": + return "ok", "becwright commit-msg hook installed." + return "warn", ("you have commit-msg rules but no becwright commit-msg hook — " + "run `becwright install`.") + + +def _cmd_doctor(_: argparse.Namespace) -> int: + print(f"{_style('becwright doctor', BOLD)} " + f"{_style(f'— becwright {__version__}', DIM)}\n") + root = git.repo_root() # NotAGitRepo propagates -> exit 2 in main() + rules, findings = _doctor_rules(root) + findings.append(_doctor_precommit_hook(root)) + msg_finding = _doctor_msg_hook(root, rules) + if msg_finding: + findings.append(msg_finding) + + for status, message in findings: + label, color = _DOCTOR_ICONS[status] + print(f" {_style(label.ljust(4), color, BOLD)} {message}") + failed = any(status == "fail" for status, _ in findings) + warned = any(status == "warn" for status, _ in findings) + print() + if failed: + print(_style(">>> Problems found: becwright cannot enforce as configured.", RED, BOLD)) + return 2 + if warned: + print(_style(">>> Working, with gaps worth fixing (see WARN above).", YELLOW, BOLD)) + return 0 + print(_style(">>> All good.", GREEN, BOLD)) + return 0 + + def _cmd_install(_: argparse.Namespace) -> int: root = git.repo_root() for install in (git.install_hook, git.install_msg_hook): @@ -887,6 +1021,8 @@ def _build_parser() -> argparse.ArgumentParser: p_check_msg.add_argument("msgfile", help="path to the commit message file (git passes this to the hook)") p_check_msg.set_defaults(func=_cmd_check_msg) + sub.add_parser("validate", help="validate .bec/rules.yaml without running any check (for editors and CI)").set_defaults(func=_cmd_validate) + sub.add_parser("doctor", help="diagnose the setup: rules file, checks, hooks and hook managers").set_defaults(func=_cmd_doctor) sub.add_parser("demo", help="see becwright block a sample bad commit (no setup, no git needed)").set_defaults(func=_cmd_demo) sub.add_parser("list", help="list the built-in checks").set_defaults(func=_cmd_list) sub.add_parser("mcp", help="run the MCP server for AI agents (needs the 'mcp' extra)").set_defaults(func=_cmd_mcp) diff --git a/src/becwright/git.py b/src/becwright/git.py index 25e07d4..03b8dc2 100644 --- a/src/becwright/git.py +++ b/src/becwright/git.py @@ -150,6 +150,34 @@ def _uninstall_named(root: Path, name: str) -> tuple[bool, str]: return True, f"becwright {name} hook uninstalled." +def hook_state(root: Path, name: str = "pre-commit") -> str: + """'becwright' (ours), 'foreign' (someone else's), or 'missing'.""" + hook = _hook_path(root, name) + if not hook.exists(): + return "missing" + return "becwright" if _HOOK_MARK in hook.read_text(encoding="utf-8") else "foreign" + + +def hooks_path_override(root: Path) -> str | None: + """The value of `core.hooksPath` when set (e.g. `.husky/_` by Husky), else None. + When set, git ignores `.git/hooks` entirely — including a becwright hook there.""" + res = subprocess.run( + ["git", "config", "core.hooksPath"], + cwd=root, capture_output=True, text=True, + ) + value = res.stdout.strip() + return value or None + + +def hook_manager(root: Path) -> str | None: + """The hook manager this repo appears to use: 'husky', 'pre-commit', or None.""" + if (root / ".husky").is_dir(): + return "husky" + if (root / ".pre-commit-config.yaml").is_file(): + return "pre-commit" + return None + + def install_hook(root: Path) -> tuple[bool, str]: return _install_named(root, "pre-commit") diff --git a/tests/test_doctor_validate.py b/tests/test_doctor_validate.py new file mode 100644 index 0000000..a45d238 --- /dev/null +++ b/tests/test_doctor_validate.py @@ -0,0 +1,242 @@ +import subprocess + +from becwright import cli, git + + +def _git(root, *args): + subprocess.run(["git", *args], cwd=root, check=True, capture_output=True, text=True) + + +def _init_repo(path): + _git(path, "init") + _git(path, "config", "user.email", "t@t.t") + _git(path, "config", "user.name", "t") + return path + + +def _write_rules(root, body): + bec = root / ".bec" + bec.mkdir(exist_ok=True) + (bec / "rules.yaml").write_text(body, encoding="utf-8") + + +_VALID = """\ +rules: + - id: no-eval + paths: ["**/*.py"] + check: "becwright run dangerous_eval" + severity: blocking +""" + +_UNKNOWN_CHECK = """\ +rules: + - id: bad + paths: ["**/*.py"] + check: "becwright run not_a_real_check" +""" + +_DUPLICATE_IDS = """\ +rules: + - id: twice + paths: ["**/*.py"] + check: "becwright run dangerous_eval" + - id: twice + paths: ["**/*.js"] + check: "becwright run dangerous_eval" +""" + +_PATHLESS = """\ +rules: + - id: floats-free + check: "becwright run dangerous_eval" +""" + + +# --- validate --- + +def test_validate_ok(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _VALID) + monkeypatch.chdir(tmp_path) + assert cli.main(["validate"]) == 0 + assert "1 rule(s) valid" in capsys.readouterr().out + + +def test_validate_missing_rules_file(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + monkeypatch.chdir(tmp_path) + assert cli.main(["validate"]) == 2 + assert "becwright init" in capsys.readouterr().err + + +def test_validate_unknown_builtin_check(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _UNKNOWN_CHECK) + monkeypatch.chdir(tmp_path) + assert cli.main(["validate"]) == 2 + assert "not_a_real_check" in capsys.readouterr().err + + +def test_validate_duplicate_ids(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _DUPLICATE_IDS) + monkeypatch.chdir(tmp_path) + assert cli.main(["validate"]) == 2 + assert "duplicate rule id 'twice'" in capsys.readouterr().err + + +def test_validate_bad_yaml_exits_2(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, "rules: [\n") + monkeypatch.chdir(tmp_path) + assert cli.main(["validate"]) == 2 + + +def test_validate_warns_on_pathless_rule_but_passes(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _PATHLESS) + monkeypatch.chdir(tmp_path) + assert cli.main(["validate"]) == 0 + assert "never" in capsys.readouterr().out + + +# --- doctor --- + +def test_doctor_healthy_repo(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _VALID) + git.install_hook(tmp_path) + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + out = capsys.readouterr().out + assert "1 rule(s)" in out + assert "pre-commit hook installed" in out + assert "All good" in out + + +def test_doctor_missing_rules_warns(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + git.install_hook(tmp_path) + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + assert "becwright init" in capsys.readouterr().out + + +def test_doctor_unknown_check_fails(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _UNKNOWN_CHECK) + git.install_hook(tmp_path) + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 2 + assert "not_a_real_check" in capsys.readouterr().out + + +def test_doctor_missing_hook_warns(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _VALID) + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + assert "becwright install" in capsys.readouterr().out + + +def test_doctor_foreign_hook_warns(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _VALID) + hooks = tmp_path / ".git" / "hooks" + hooks.mkdir(parents=True, exist_ok=True) + (hooks / "pre-commit").write_text("#!/bin/sh\necho hi\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + assert "non-becwright" in capsys.readouterr().out + + +def test_doctor_hooks_path_override_warns(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _VALID) + _git(tmp_path, "config", "core.hooksPath", ".custom-hooks") + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + assert "core.hooksPath" in capsys.readouterr().out + + +def test_doctor_husky_with_becwright_is_ok(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _VALID) + _git(tmp_path, "config", "core.hooksPath", ".husky/_") + husky = tmp_path / ".husky" + husky.mkdir() + (husky / "pre-commit").write_text("npx becwright check\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + assert "Husky runs becwright" in capsys.readouterr().out + + +def test_doctor_husky_without_becwright_warns(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _VALID) + _git(tmp_path, "config", "core.hooksPath", ".husky/_") + husky = tmp_path / ".husky" + husky.mkdir() + (husky / "pre-commit").write_text("npm test\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + assert "npx becwright check" in capsys.readouterr().out + + +def test_doctor_precommit_framework_with_becwright_is_ok(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, _VALID) + hooks = tmp_path / ".git" / "hooks" + hooks.mkdir(parents=True, exist_ok=True) + (hooks / "pre-commit").write_text("#!/bin/sh\n# by pre-commit\n", encoding="utf-8") + (tmp_path / ".pre-commit-config.yaml").write_text( + "repos:\n - repo: https://github.com/DataDave-Dev/becwright\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + assert "pre-commit framework runs becwright" in capsys.readouterr().out + + +def test_doctor_commit_msg_rules_without_hook_warns(tmp_path, monkeypatch, capsys): + _init_repo(tmp_path) + _write_rules(tmp_path, """\ +rules: + - id: conv-commits + target: commit-msg + check: "becwright run require --pattern '^feat'" +""") + git.install_hook(tmp_path) + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 0 + assert "commit-msg" in capsys.readouterr().out + + +def test_doctor_outside_git_repo(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + assert cli.main(["doctor"]) == 2 + + +# --- git.py helpers --- + +def test_hook_state(tmp_path): + (tmp_path / ".git").mkdir() + assert git.hook_state(tmp_path) == "missing" + git.install_hook(tmp_path) + assert git.hook_state(tmp_path) == "becwright" + hook = tmp_path / ".git" / "hooks" / "pre-commit" + hook.write_text("#!/bin/sh\necho custom\n", encoding="utf-8") + assert git.hook_state(tmp_path) == "foreign" + + +def test_hooks_path_override(tmp_path): + _init_repo(tmp_path) + assert git.hooks_path_override(tmp_path) is None + _git(tmp_path, "config", "core.hooksPath", ".husky/_") + assert git.hooks_path_override(tmp_path) == ".husky/_" + + +def test_hook_manager(tmp_path): + assert git.hook_manager(tmp_path) is None + (tmp_path / ".pre-commit-config.yaml").write_text("repos: []\n", encoding="utf-8") + assert git.hook_manager(tmp_path) == "pre-commit" + (tmp_path / ".husky").mkdir() + assert git.hook_manager(tmp_path) == "husky" # husky wins when both exist