diff --git a/.bully.yml b/.bully.yml index 5d11040..776e58c 100644 --- a/.bully.yml +++ b/.bully.yml @@ -97,12 +97,14 @@ rules: ruff-clean: description: > - Python files must pass `ruff check`. Requires ruff on PATH (see - the `dev` extras in pyproject.toml). When ruff isn't installed the - rule passes with a stderr hint rather than blocking edits. - --force-exclude makes ruff respect pyproject's extend-exclude so - test fixtures under bench/fixtures/ are skipped. + Python files must pass `ruff check`. Looks for ruff on PATH first, + then falls back to `.venv/bin/ruff` for repo-local venv installs + (the common case when the hook runs from Claude Code without the + venv activated). When neither is present the rule passes with a + stderr hint rather than blocking edits. --force-exclude makes + ruff respect pyproject's extend-exclude so test fixtures under + bench/fixtures/ are skipped. engine: script scope: "*.py" severity: error - script: "command -v ruff >/dev/null 2>&1 || { echo 'bully: ruff not on PATH; pip install -e .[dev]' >&2; exit 0; }; ruff check --force-exclude {file}" + script: "if command -v ruff >/dev/null 2>&1; then ruff check --force-exclude {file}; elif [ -x .venv/bin/ruff ]; then .venv/bin/ruff check --force-exclude {file}; else echo 'bully: ruff not found; pip install -e .[dev]' >&2; exit 0; fi" diff --git a/bully b/bully new file mode 100755 index 0000000..587e13a --- /dev/null +++ b/bully @@ -0,0 +1,4 @@ +#!/usr/bin/env bash +set -euo pipefail +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +exec python3 "$HERE/pipeline/pipeline.py" "$@" diff --git a/docs/rule-authoring.md b/docs/rule-authoring.md index 17f7565..61098b3 100644 --- a/docs/rule-authoring.md +++ b/docs/rule-authoring.md @@ -41,6 +41,14 @@ rule-id: - Stdout on failure should list violations. The pipeline parses common formats (see [Output formats](#output-formats)). - Each rule has a 30-second timeout. +### YAML string escapes + +Double-quoted scalars process YAML escapes: `\\` becomes `\`, `\"` becomes `"`, `\n` becomes newline, `\t` becomes tab. Single-quoted scalars only process `''` as `'` (YAML spec); backslashes pass through literally. + +The practical upshot for regex in `script:` values: write `"grep -nE 'console\\.log\\(' {file}"` — YAML turns each `\\` into `\`, so grep sees `console\.log\(` and matches the intended pattern. Under-escaping (`"grep -nE 'console\.log\('"`) leaves grep with `console.log(`, where `.` matches any character and the parens become a group. + +For a literal backslash in the script, use single-quoted YAML (`'grep "\\"'`) or double up explicitly in double quotes. + ### Minimal grep pattern ```yaml @@ -221,6 +229,8 @@ Scope is right-anchored glob matching via `PurePath.match`. | `*` | everything | | `["*.php", "*.blade.php"]` | any file matching either glob | +`**` works on all supported Python versions because bully's matcher implements recursive globbing explicitly, not via `PurePath.match` (whose recursive-`**` support only landed in 3.13). + Scope narrowly. Broad scopes (like `"*"`) run more rules per edit and inflate the noise floor. Save `"*"` for truly cross-language rules like orchestration-label bans. ## Output formats @@ -328,6 +338,11 @@ bully lint src/foo.php --print-prompt bully lint src/foo.php --diff "$(git diff src/foo.php)" ``` +Two more flags worth knowing about: + +- **`bully --explain --file `** — prints one line per rule in scope with its verdict: `fire`, `pass`, `skipped `, or `dispatched`. Reach for this when a rule *should* match a file but isn't firing — the output tells you whether the scope filter dropped it or the rule ran and passed. Already present; surfaced here because it's easy to miss. +- **`bully validate --execute-dry-run`** — runs every script rule against empty input and flags rules that error out at the shell or regex level. Catches the "grep: parentheses not balanced" class of bugs at config time instead of at hook time. Run it before committing a new regex rule. + Exit codes: - `0` — pass or evaluate. Stdout is JSON. diff --git a/pipeline/pipeline.py b/pipeline/pipeline.py index e1c8072..5a2ff9c 100644 --- a/pipeline/pipeline.py +++ b/pipeline/pipeline.py @@ -152,13 +152,67 @@ def _strip_inline_comment(raw: str) -> str: def _parse_scalar(raw: str) -> str: - """Normalize a scalar value: strip inline comment, then matched outer quotes only.""" + """Normalize a scalar value: strip inline comment, then process YAML quote escapes. + + Double-quoted scalars process standard YAML escapes (`\\\\`, `\\"`, `\\n`, `\\t`, + `\\r`, `\\/`, `\\0`); unknown escapes are kept verbatim (the backslash is + preserved) to avoid silently eating author intent. Single-quoted scalars only + process `''` (doubled single quote) as a literal `'`. Plain unquoted scalars + pass through unchanged -- backslashes have no special meaning outside quotes. + """ raw = _strip_inline_comment(raw).strip() - if len(raw) >= 2 and ((raw[0] == '"' and raw[-1] == '"') or (raw[0] == "'" and raw[-1] == "'")): - return raw[1:-1] + if len(raw) >= 2 and raw[0] == '"' and raw[-1] == '"': + return _unescape_double_quoted(raw[1:-1]) + if len(raw) >= 2 and raw[0] == "'" and raw[-1] == "'": + return raw[1:-1].replace("''", "'") return raw +# YAML double-quoted escape table (subset per the YAML 1.2 spec plus the few +# C-style escapes we actually see in bully configs). Unknown escapes fall +# through as `\x` (backslash preserved) so unusual regex patterns survive. +_DOUBLE_QUOTED_ESCAPES: dict[str, str] = { + "\\": "\\", + '"': '"', + "n": "\n", + "t": "\t", + "r": "\r", + "/": "/", + "0": "\x00", +} + + +def _unescape_double_quoted(inner: str) -> str: + """Apply YAML double-quoted escape processing to the inside of a scalar. + + Only the subset listed in `_DOUBLE_QUOTED_ESCAPES` is collapsed. Unknown + sequences (e.g. `\\z`) are kept literally -- we preserve the backslash and + the following character rather than raising, so rule authors can use + backslash-heavy regex patterns without the parser throwing at config load. + A trailing lone backslash is also kept literally. + """ + if "\\" not in inner: + return inner + out: list[str] = [] + i = 0 + n = len(inner) + while i < n: + ch = inner[i] + if ch == "\\" and i + 1 < n: + nxt = inner[i + 1] + mapped = _DOUBLE_QUOTED_ESCAPES.get(nxt) + if mapped is not None: + out.append(mapped) + else: + out.append(ch) + out.append(nxt) + i += 2 + continue + out.append(ch) + i += 1 + return "".join(out) + + def _parse_inline_list(raw: str) -> list[str] | None: """Parse `[a, b, "c"]` into a list of scalars, or return None if not a list.""" raw = _strip_inline_comment(raw).strip() @@ -659,10 +713,120 @@ def effective_skip_patterns( return (*SKIP_PATTERNS, *user_global, *project) +def _scope_glob_matches(pattern: str, file_path: str) -> bool: + """Match a scope glob against a file path, with recursive `**` support. + + `PurePath.match` only grew zero-or-more-segment `**` semantics in Python + 3.13; bully supports 3.10+. We split the pattern on `**` and require each + segment to match contiguously against the path, with `**` absorbing zero + or more intermediate path segments. Single `*` still only matches within + one segment (via fnmatch). + + Simple patterns without `**` (the common case) fall back to + `PurePath.match`, which handles right-anchored suffix matches like + `*.ts` matching `src/foo.ts`. + """ + if "**" not in pattern: + try: + return PurePath(file_path).match(pattern) + except ValueError: + return False + + path_parts = PurePath(file_path).parts + # Split on `**` but keep empty strings at the boundaries so a leading or + # trailing `**` is explicit in the segment list. + raw_segments = pattern.split("**") + # Each non-`**` segment can contain `/`; split further into path-segment + # globs. An empty segment (between two consecutive `**`, or at the edges + # of the pattern) yields []. + segments: list[list[str]] = [] + for raw in raw_segments: + trimmed = raw.strip("/") + segments.append(trimmed.split("/") if trimmed else []) + + # Anchored prefix: the first segment must match starting at path_parts[0] + # UNLESS the pattern starts with `**/` (i.e. segments[0] is empty), in + # which case `**` can absorb zero-or-more leading path parts. + return _match_glob_segments(segments, 0, path_parts, 0) + + +def _segment_matches(globs: list[str], parts: tuple[str, ...], start: int) -> bool: + """True iff every glob in `globs` matches `parts[start:start+len(globs)]`.""" + if start + len(globs) > len(parts): + return False + return all(fnmatch.fnmatchcase(parts[start + i], g) for i, g in enumerate(globs)) + + +def _match_glob_segments( + segments: list[list[str]], + seg_idx: int, + parts: tuple[str, ...], + part_idx: int, +) -> bool: + """Recursively match `**`-delimited glob segments against path parts. + + `segments[0]` is anchored (must match at `part_idx`). Each subsequent + `segments[i]` is preceded by a `**` and may float -- it can start at any + position at or after `part_idx`. An empty segment between two `**` + markers is a no-op. After matching the last segment the remaining path + parts must be fully consumed (len(parts) == part_idx + len(globs)) unless + the pattern ended with `**`, in which case they're absorbed. + """ + if seg_idx >= len(segments): + # Consumed all segments; remaining path parts must be empty. + return part_idx == len(parts) + + globs = segments[seg_idx] + is_last = seg_idx == len(segments) - 1 + # Pattern ended with `**` (trailing empty segment) means the final + # segment list is empty and `**` can absorb all remaining parts. + trailing_double_star = is_last and not globs + + if seg_idx == 0: + # Anchored at part_idx (which is 0 on the initial call). An empty + # first segment means the pattern starts with `**`, so the next + # segment floats. + if not globs: + return _match_glob_segments(segments, seg_idx + 1, parts, part_idx) + if not _segment_matches(globs, parts, part_idx): + return False + new_idx = part_idx + len(globs) + if is_last: + return new_idx == len(parts) + return _match_glob_segments(segments, seg_idx + 1, parts, new_idx) + + # Floating segment (preceded by `**`). Try every possible start position + # at or after part_idx. `**` can absorb zero or more path parts. + if trailing_double_star: + # Trailing `**` matches anything from part_idx onward, so any path + # parts remaining (zero or more) are absorbed. + return True + + if not globs: + # Empty floating segment (consecutive `**`s). Collapse and continue. + return _match_glob_segments(segments, seg_idx + 1, parts, part_idx) + + end_limit = len(parts) - len(globs) + if is_last: + # Last segment must consume exactly to the end of parts. + return _segment_matches(globs, parts, end_limit) if end_limit >= part_idx else False + for try_at in range(part_idx, end_limit + 1): + if _segment_matches(globs, parts, try_at) and _match_glob_segments( + segments, seg_idx + 1, parts, try_at + len(globs) + ): + return True + return False + + def filter_rules(rules: list[Rule], file_path: str) -> list[Rule]: - """Return rules whose scope glob(s) match the given file path.""" - path = PurePath(file_path) - return [r for r in rules if any(path.match(g) for g in r.scope)] + """Return rules whose scope glob(s) match the given file path. + + Scope matching supports recursive `**` (zero-or-more path segments) in + addition to the standard `*` (single-segment) and `?` wildcards. This is + implemented in-house because `PurePath.match` only gained recursive `**` + semantics in Python 3.13 and bully supports 3.10+. + """ + return [r for r in rules if any(_scope_glob_matches(g, file_path) for g in r.scope)] # --------------------------------------------------------------------------- @@ -1930,6 +2094,13 @@ def _parse_args(argv: list[str]) -> argparse.Namespace: help="Print per-rule verdict (fire/pass/skipped /dispatched) for " "every rule in scope, instead of the JSON pipeline result.", ) + parser.add_argument( + "--execute-dry-run", + dest="execute_dry_run", + action="store_true", + help="With --validate: run each script rule against empty input to catch " + "shell/regex-level errors (unbalanced parens, missing commands) at config time.", + ) args = parser.parse_args(argv) # Back-compat: accept positional args (used by hook) if args.positional and not args.config: @@ -1942,7 +2113,7 @@ def _parse_args(argv: list[str]) -> argparse.Namespace: # ---- subcommand handlers ---- -def _cmd_validate(config_path: str | None) -> int: +def _cmd_validate(config_path: str | None, *, execute_dry_run: bool = False) -> int: path = config_path or ".bully.yml" if not os.path.exists(path): print(f"[FAIL] config not found: {path}", file=sys.stderr) @@ -1962,9 +2133,79 @@ def _cmd_validate(config_path: str | None) -> int: f"ast-grep not on PATH. {_AST_GREP_INSTALL_HINT}", file=sys.stderr, ) + if execute_dry_run: + return _run_execute_dry_run(rules) return 0 +def _run_execute_dry_run(rules: list[Rule]) -> int: + """Execute every script rule against `/dev/null`, report broken scripts. + + Catches shell/regex-level errors at config time: unbalanced parens in a + `grep -E` pattern, typos in command names, non-executable scripts, etc. + A rule is flagged as broken when either: + + - The exit code is not in {0, 1} (2 = grep syntax error, 126 = not + executable, 127 = command-not-found, etc.), OR + - stderr carries a known tool-error signature even when exit is 0/1. + This matters because shells often mask inner errors: `grep ... && + exit 1 || exit 0` swallows grep's exit-2 and reports 0, leaving the + regex diagnostic only in stderr. + + Returns 0 if all script rules are healthy, 1 if any were flagged. + """ + # Prefixes that indicate a tool surfaced an error, not just incidental + # stderr chatter. Keep narrow to avoid false positives from tools that + # write progress to stderr. + error_signatures = ( + "grep:", + "sed:", + "awk:", + "bash:", + "sh:", + "command not found", + "syntax error", + "not recognized as an internal", + ) + + script_rules = [r for r in rules if r.engine == "script" and r.script] + if not script_rules: + print("[OK] no script rules to dry-run") + return 0 + + failures = 0 + for rule in script_rules: + cmd = rule.script.replace("{file}", "/dev/null") + try: + result = subprocess.run( # bully-disable: no-shell-true-subprocess dry-run probe of user-configured script; mirrors real execute_script_rule path + cmd, + shell=True, + timeout=5, + capture_output=True, + text=True, + ) + except subprocess.TimeoutExpired: + print(f"[WARN] {rule.id}: dry-run exit=timeout stderr: script timed out") + failures += 1 + continue + + rc = result.returncode + stderr = result.stderr.strip() + stderr_first = stderr.splitlines()[0] if stderr else "" + # A stderr line that matches a known error signature means the tool + # reported an error even if the outer shell construct normalized + # the exit code back to 0/1. + stderr_looks_broken = any(sig in stderr.lower() for sig in error_signatures) + + if rc in (0, 1) and not stderr_looks_broken: + print(f"[OK] {rule.id}: dry-run clean (exit {rc})") + continue + failures += 1 + print(f"[WARN] {rule.id}: dry-run exit={rc} stderr: {stderr_first}") + + return 0 if failures == 0 else 1 + + def _cmd_show_resolved(config_path: str | None) -> int: path = config_path or ".bully.yml" try: @@ -1980,11 +2221,48 @@ def _cmd_show_resolved(config_path: str | None) -> int: return 0 +_MIN_PYTHON = (3, 10) + + +def _check_python_version(version_info: tuple[int, int] = sys.version_info[:2]) -> tuple[bool, str]: + """Return (ok, message) for the Python version check. + + Split out of `_cmd_doctor` so tests can feed synthetic version tuples + without spawning a different interpreter. + """ + major, minor = version_info[:2] + if (major, minor) >= _MIN_PYTHON: + return True, f"[OK] Python {major}.{minor}" + need = f"{_MIN_PYTHON[0]}.{_MIN_PYTHON[1]}" + return False, f"[FAIL] Python {major}.{minor} < {need} -- upgrade required" + + +def _plugin_cache_candidates(resource_kind: str, name: str) -> list[Path]: + """Return plausible `~/.claude/plugins/cache/*/bully/*/{skills,agents}//...` paths. + + resource_kind is "skills" or "agents". For skills, the file is `/SKILL.md`; + for agents, the file is `.md` directly under `agents/`. + """ + root = Path.home() / ".claude" / "plugins" / "cache" + if not root.is_dir(): + return [] + pattern = f"*/bully/*/{resource_kind}/" + out: list[Path] = [] + for base in root.glob(pattern): + candidate = base / name / "SKILL.md" if resource_kind == "skills" else base / f"{name}.md" + if candidate.is_file(): + out.append(candidate) + return out + + def _cmd_doctor() -> int: ok = True # Python version - print(f"[OK] Python {sys.version_info.major}.{sys.version_info.minor} >= 3.10") + py_ok, py_msg = _check_python_version() + print(py_msg) + if not py_ok: + ok = False # Config present cfg = Path.cwd() / ".bully.yml" @@ -2060,16 +2338,22 @@ def _cmd_doctor() -> int: print("[FAIL] no PostToolUse hook invoking hook.sh found in .claude/settings.json") ok = False - # Evaluator subagent definition + # Evaluator subagent definition -- legacy path OR plugin cache path claude_home = Path(os.environ.get("CLAUDE_HOME", str(Path.home() / ".claude"))) agent_file = claude_home / "agents" / "bully-evaluator.md" + plugin_agents = _plugin_cache_candidates("agents", "bully-evaluator") if agent_file.is_file(): print(f"[OK] evaluator agent at {agent_file}") + elif plugin_agents: + print(f"[OK] evaluator agent at {plugin_agents[0]} (plugin install)") else: - print(f"[FAIL] evaluator agent missing at {agent_file}") + print( + f"[FAIL] evaluator agent missing -- expected at {agent_file} " + f"or under ~/.claude/plugins/cache/*/bully/*/agents/bully-evaluator.md" + ) ok = False - # Skills + # Skills -- legacy path OR plugin cache path for suffix in ( "bully", "bully-init", @@ -2077,10 +2361,16 @@ def _cmd_doctor() -> int: "bully-review", ): skill_md = Path.home() / ".claude" / "skills" / suffix / "SKILL.md" + plugin_skill = _plugin_cache_candidates("skills", suffix) if skill_md.is_file(): print(f"[OK] skill {suffix} present") + elif plugin_skill: + print(f"[OK] skill {suffix} present at {plugin_skill[0]} (plugin install)") else: - print(f"[FAIL] skill {suffix} missing (expected at {skill_md})") + print( + f"[FAIL] skill {suffix} missing -- expected at {skill_md} " + f"or under ~/.claude/plugins/cache/*/bully/*/skills/{suffix}/SKILL.md" + ) ok = False return 0 if ok else 1 @@ -2253,7 +2543,7 @@ def main() -> None: if args.trust: sys.exit(_cmd_trust(args.config, refresh=args.refresh)) if args.validate: - sys.exit(_cmd_validate(args.config)) + sys.exit(_cmd_validate(args.config, execute_dry_run=args.execute_dry_run)) if args.doctor: sys.exit(_cmd_doctor()) if args.show_resolved_config: diff --git a/pipeline/tests/test_doctor.py b/pipeline/tests/test_doctor.py index 3873621..fc54b92 100644 --- a/pipeline/tests/test_doctor.py +++ b/pipeline/tests/test_doctor.py @@ -5,9 +5,91 @@ import sys from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +from pipeline import _check_python_version # noqa: E402 + PIPELINE = Path(__file__).resolve().parent.parent / "pipeline.py" +def test_check_python_version_passes_on_310_and_above(): + ok, msg = _check_python_version((3, 10)) + assert ok is True + assert msg == "[OK] Python 3.10" + + ok, msg = _check_python_version((3, 12)) + assert ok is True + assert msg == "[OK] Python 3.12" + + ok, msg = _check_python_version((4, 0)) + assert ok is True + assert msg == "[OK] Python 4.0" + + +def test_check_python_version_fails_below_310(): + ok, msg = _check_python_version((3, 9)) + assert ok is False + assert msg.startswith("[FAIL] Python 3.9 < 3.10") + + ok, msg = _check_python_version((3, 8)) + assert ok is False + assert "3.8" in msg + + ok, msg = _check_python_version((2, 7)) + assert ok is False + assert "2.7" in msg + + +def test_doctor_finds_skills_and_agent_in_plugin_cache(tmp_path): + """Plugin-installed skills and the evaluator agent live under + ~/.claude/plugins/cache//bully//{skills,agents}/... + Doctor must accept either the legacy or the plugin path. + """ + project = tmp_path / "project" + project.mkdir() + (project / ".bully.yml").write_text( + "rules:\n" + " r1:\n" + ' description: "d"\n' + " engine: script\n" + ' scope: "*"\n' + " severity: error\n" + ' script: "exit 0"\n' + ) + (project / ".claude").mkdir() + (project / ".claude" / "settings.json").write_text( + json.dumps({"hooks": {"PostToolUse": [{"hooks": [{"command": "hook.sh"}]}]}}) + ) + + # Plugin-only layout (no legacy ~/.claude/skills or ~/.claude/agents). + home = tmp_path / "home" + plugin_root = home / ".claude" / "plugins" / "cache" / "bully-marketplace" / "bully" / "0.2.0" + skills_root = plugin_root / "skills" + for name in ("bully", "bully-init", "bully-author", "bully-review"): + (skills_root / name).mkdir(parents=True) + (skills_root / name / "SKILL.md").write_text("# skill\n") + agents_root = plugin_root / "agents" + agents_root.mkdir(parents=True) + (agents_root / "bully-evaluator.md").write_text("# eval\n") + + import os + + env = os.environ.copy() + env.update({"HOME": str(home), "CLAUDE_HOME": str(home / ".claude")}) + r = subprocess.run( + [sys.executable, str(PIPELINE), "--doctor"], + capture_output=True, + text=True, + timeout=10, + cwd=str(project), + env=env, + ) + assert r.returncode == 0, f"stdout={r.stdout}\nstderr={r.stderr}" + assert "[OK] evaluator agent at" in r.stdout + assert "(plugin install)" in r.stdout + for name in ("bully", "bully-init", "bully-author", "bully-review"): + assert f"[OK] skill {name} present" in r.stdout + + def _run_doctor(cwd: Path, env_extra: dict[str, str] | None = None) -> subprocess.CompletedProcess: import os diff --git a/pipeline/tests/test_parser_hardening.py b/pipeline/tests/test_parser_hardening.py index 366e275..ce03d34 100644 --- a/pipeline/tests/test_parser_hardening.py +++ b/pipeline/tests/test_parser_hardening.py @@ -64,3 +64,161 @@ def test_mismatched_outer_quotes_not_stripped(): assert _parse_scalar('foo"') == 'foo"' assert _parse_scalar('"foo"') == "foo" assert _parse_scalar("'foo'") == "foo" + + +# --------------------------------------------------------------------------- +# Regression tests: YAML escape sequence handling in double-quoted scalars. +# The silent miscompile case: `"\\."` must yield `\.` (regex-correct), not `\\.`. +# --------------------------------------------------------------------------- + + +def test_double_quoted_backslash_escape_collapses_to_single_backslash(): + """`"\\\\."` in YAML must produce `\\.` -- the escape grep regex needs.""" + from pipeline import _parse_scalar + + # Raw input from YAML parser (outer quotes + inner double-backslash + dot). + assert _parse_scalar(r'"\\."') == r"\." + + +def test_double_quoted_escapes_for_grep_console_log_pattern(): + r"""The real-world console.log regex pattern must round-trip correctly. + + YAML-quoted: "(^|[^a-zA-Z_.])console\\.log\\(" + Should become: (^|[^a-zA-Z_.])console\.log\( (what grep -E expects) + """ + from pipeline import _parse_scalar + + raw = r'"(^|[^a-zA-Z_.])console\\.log\\("' + assert _parse_scalar(raw) == r"(^|[^a-zA-Z_.])console\.log\(" + + +def test_double_quoted_escape_embedded_double_quote(): + """`"foo\\"bar"` should yield `foo"bar`.""" + from pipeline import _parse_scalar + + # raw bytes in the YAML file: "foo\"bar" + assert _parse_scalar(r'"foo\"bar"') == 'foo"bar' + + +def test_double_quoted_escape_newline_tab_return(): + """Standard C-style escapes are supported in double-quoted scalars.""" + from pipeline import _parse_scalar + + assert _parse_scalar(r'"a\nb"') == "a\nb" + assert _parse_scalar(r'"a\tb"') == "a\tb" + assert _parse_scalar(r'"a\rb"') == "a\rb" + assert _parse_scalar(r'"a\0b"') == "a\x00b" + assert _parse_scalar(r'"a\/b"') == "a/b" + + +def test_double_quoted_unknown_escape_kept_literally(): + """Unknown escape sequences are kept verbatim (graceful, no raise).""" + from pipeline import _parse_scalar + + # `\z` isn't a standard escape -- keep as `\z` rather than erroring. + assert _parse_scalar(r'"a\zb"') == r"a\zb" + + +def test_single_quoted_no_backslash_processing(): + """Single-quoted scalars keep backslashes literally; only `''` is an escape.""" + from pipeline import _parse_scalar + + assert _parse_scalar(r"'\\.'") == r"\\." + assert _parse_scalar(r"'a\nb'") == r"a\nb" + + +def test_single_quoted_double_single_quote_escape(): + """In single-quoted YAML scalars, `''` -> `'`.""" + from pipeline import _parse_scalar + + # YAML spec: 'can''t' -> can't + assert _parse_scalar("'can''t'") == "can't" + + +def test_plain_unquoted_scalar_not_touched(): + """Unquoted scalars must not get escape processing.""" + from pipeline import _parse_scalar + + assert _parse_scalar(r"a\nb") == r"a\nb" + assert _parse_scalar(r"foo\\bar") == r"foo\\bar" + + +# --------------------------------------------------------------------------- +# Recursive `**` glob matching in scope filters. +# Python's `PurePath.match` only grew recursive-`**` support in 3.13; bully +# supports 3.10+, so we implement the matcher ourselves. +# --------------------------------------------------------------------------- + + +def _scope_rule(scope): + from pipeline import Rule + + return Rule(id="r", description="d", engine="semantic", scope=scope, severity="error") + + +def test_double_star_matches_zero_directories(): + """`src/**/*.ts` must match `src/foo.ts` (zero intermediate dirs).""" + from pipeline import filter_rules + + rules = [_scope_rule(("src/**/*.ts",))] + assert len(filter_rules(rules, "src/foo.ts")) == 1 + + +def test_double_star_matches_one_directory(): + """`src/**/*.ts` must match `src/sub/foo.ts`.""" + from pipeline import filter_rules + + rules = [_scope_rule(("src/**/*.ts",))] + assert len(filter_rules(rules, "src/sub/foo.ts")) == 1 + + +def test_double_star_matches_many_directories(): + """`src/**/*.ts` must match arbitrarily nested paths.""" + from pipeline import filter_rules + + rules = [_scope_rule(("src/**/*.ts",))] + assert len(filter_rules(rules, "src/a/b/c/d/foo.ts")) == 1 + + +def test_double_star_does_not_match_sibling_trees(): + """`src/**/*.ts` must not match `lib/foo.ts` -- prefix still anchors.""" + from pipeline import filter_rules + + rules = [_scope_rule(("src/**/*.ts",))] + assert len(filter_rules(rules, "lib/foo.ts")) == 0 + + +def test_leading_double_star_matches_root_file(): + """`**/*.ts` must match a top-level `foo.ts` (zero dirs above).""" + from pipeline import filter_rules + + rules = [_scope_rule(("**/*.ts",))] + assert len(filter_rules(rules, "foo.ts")) == 1 + + +def test_leading_double_star_matches_nested_file(): + """`**/*.ts` must match deeply nested `a/b/foo.ts`.""" + from pipeline import filter_rules + + rules = [_scope_rule(("**/*.ts",))] + assert len(filter_rules(rules, "a/b/c/foo.ts")) == 1 + + +def test_plain_glob_still_matches_basename(): + """Simple `*.ts` must still work (basename match, the common case).""" + from pipeline import filter_rules + + rules = [_scope_rule(("*.ts",))] + assert len(filter_rules(rules, "anywhere/foo.ts")) == 1 + assert len(filter_rules(rules, "foo.ts")) == 1 + assert len(filter_rules(rules, "foo.js")) == 0 + + +def test_trailing_double_star_matches_all_descendants(): + """`tests/**` must match everything below `tests/`.""" + from pipeline import filter_rules + + rules = [_scope_rule(("tests/**",))] + assert len(filter_rules(rules, "tests/foo.py")) == 1 + assert len(filter_rules(rules, "tests/a/b/c.py")) == 1 + assert len(filter_rules(rules, "src/foo.py")) == 0 diff --git a/pipeline/tests/test_validate_cli.py b/pipeline/tests/test_validate_cli.py index 3107cc3..244eb1d 100644 --- a/pipeline/tests/test_validate_cli.py +++ b/pipeline/tests/test_validate_cli.py @@ -86,3 +86,106 @@ def test_validate_invalid_severity_fails(tmp_path): r = _run(["--validate", "--config", str(bad)]) assert r.returncode == 1 assert "severity" in r.stderr.lower() + + +# --------------------------------------------------------------------------- +# --execute-dry-run: verify each script rule runs against empty input without +# shell/regex-level errors. Catches bad grep patterns at config time. +# --------------------------------------------------------------------------- + + +def test_execute_dry_run_clean_script_rule_reports_ok(tmp_path): + cfg = tmp_path / ".bully.yml" + cfg.write_text( + "rules:\n" + " clean:\n" + ' description: "no foobar"\n' + " engine: script\n" + ' scope: "*"\n' + " severity: error\n" + ' script: "grep -nE foobar {file} && exit 1 || exit 0"\n' + ) + r = _run(["--validate", "--config", str(cfg), "--execute-dry-run"]) + assert r.returncode == 0, f"stderr={r.stderr}" + assert "clean" in r.stdout + assert "[OK]" in r.stdout + + +def test_execute_dry_run_flags_broken_grep_regex(tmp_path): + """A grep -E with unbalanced parens must surface as a dry-run failure.""" + cfg = tmp_path / ".bully.yml" + cfg.write_text( + "rules:\n" + " broken-regex:\n" + ' description: "bad pattern"\n' + " engine: script\n" + ' scope: "*"\n' + " severity: error\n" + " script: \"grep -nE '(unclosed' {file} && exit 1 || exit 0\"\n" + ) + r = _run(["--validate", "--config", str(cfg), "--execute-dry-run"]) + assert r.returncode == 1, f"stdout={r.stdout} stderr={r.stderr}" + assert "broken-regex" in r.stdout or "broken-regex" in r.stderr + + +def test_execute_dry_run_flags_nonexistent_command(tmp_path): + """A script referencing a command that doesn't exist should fail the dry run.""" + cfg = tmp_path / ".bully.yml" + cfg.write_text( + "rules:\n" + " missing-cmd:\n" + ' description: "missing binary"\n' + " engine: script\n" + ' scope: "*"\n' + " severity: error\n" + ' script: "thiscommanddoesnotexist123 {file}"\n' + ) + r = _run(["--validate", "--config", str(cfg), "--execute-dry-run"]) + assert r.returncode == 1 + assert "missing-cmd" in r.stdout or "missing-cmd" in r.stderr + + +def test_execute_dry_run_skips_non_script_rules(tmp_path): + """Semantic and ast rules aren't shell-executed, so dry run skips them.""" + cfg = tmp_path / ".bully.yml" + cfg.write_text( + "rules:\n" + " sem:\n" + ' description: "semantic only"\n' + " engine: semantic\n" + ' scope: "*"\n' + " severity: warning\n" + ) + r = _run(["--validate", "--config", str(cfg), "--execute-dry-run"]) + assert r.returncode == 0, f"stderr={r.stderr}" + # Non-script rule is parsed but not dry-run executed. + assert "sem" in r.stdout + + +def test_execute_dry_run_requires_validate(tmp_path): + """--execute-dry-run is only meaningful alongside --validate.""" + r = _run(["--execute-dry-run"]) + assert r.returncode != 0 + + +def test_execute_dry_run_multiple_rules_all_clean(tmp_path): + cfg = tmp_path / ".bully.yml" + cfg.write_text( + "rules:\n" + " a:\n" + ' description: "a"\n' + " engine: script\n" + ' scope: "*"\n' + " severity: error\n" + ' script: "exit 0"\n' + " b:\n" + ' description: "b"\n' + " engine: script\n" + ' scope: "*"\n' + " severity: error\n" + ' script: "grep -nE ok {file} && exit 1 || exit 0"\n' + ) + r = _run(["--validate", "--config", str(cfg), "--execute-dry-run"]) + assert r.returncode == 0, f"stderr={r.stderr}" + assert "a" in r.stdout + assert "b" in r.stdout diff --git a/skills/bully-init/SKILL.md b/skills/bully-init/SKILL.md index 5cc4d5c..41cafa1 100644 --- a/skills/bully-init/SKILL.md +++ b/skills/bully-init/SKILL.md @@ -67,9 +67,20 @@ rules: For multi-line descriptions use a folded scalar (`description: >`). Quote all script values with double quotes. Use `{file}` as the target file placeholder. Formatters use `severity: warning`; correctness rules use `severity: error`. -## Step 6: Summarize and hand off +**Binary note.** Bully ships a `bully` wrapper at its repo root that `exec`s `python3 pipeline/pipeline.py`. Plugin installs don't run `pip install`, so `bully` is not on `PATH` by default -- the wrapper lives at `~/.claude/plugins/cache/bully-marketplace/bully//bully`. If subsequent steps fail with "command not found", tell the user to either alias it (`alias bully='~/.claude/plugins/cache/bully-marketplace/bully//bully'`) or symlink it into `~/.local/bin`. Until that's done, fall back to `python3 /pipeline/pipeline.py ...` with the equivalent flags. -After writing, print: +## Step 6: Verify and enable + +Before handing off, bring the config into a runnable state: + +1. **Trust the config** so script/ast rules can execute: `bully trust` (fallback: `python3 /pipeline/pipeline.py --trust --config .bully.yml`). +2. **Run `bully doctor`** and surface any `[FAIL]` lines. Plugin installs may emit false positives for skill/agent checks -- if doctor reports a missing skill or agent file, note it but do not block. +3. **Telemetry directory** (only if the user answered yes in Step 3): `mkdir -p .bully/` and ensure `.gitignore` contains `.bully/` (append it if missing). +4. **Smoke-test script rules.** For each rule with a concrete `script:`, pick the first in-scope file (e.g. `git ls-files | grep -E '\.(ts|tsx)$' | head -1` against the rule's scope) and run `bully lint --rule `. Report each verdict. If a rule that is *meant* to fire on a known pattern returns pass, flag it as a likely miscompile -- surface it now, not after 40 edits. + +## Step 7: Summarize and hand off + +After the verification pass, print: ``` .bully.yml generated. @@ -79,6 +90,9 @@ Extends: Migrated: rules from Overrides: Excluded globs: +Trust: +Doctor: +Smoke test: ``` Tell the user: "To add project-specific rules, use `/bully-author`. To audit rule health later, use `/bully-review`."