diff --git a/.github/workflows/dead-code-hook.yml b/.github/workflows/dead-code-hook.yml deleted file mode 100644 index 8befa24..0000000 --- a/.github/workflows/dead-code-hook.yml +++ /dev/null @@ -1,66 +0,0 @@ -name: dead-code hook - -on: - push: - branches: [main] - pull_request: - workflow_dispatch: - -jobs: - test: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-python@v5 - with: - python-version: "3.12" - - # Matches the pin every consumer uses. - - run: pip install vulture==2.14 - - - name: Test check-dead-code.sh - env: - REQUIRE_VULTURE: 1 - run: bash tests/test-check-dead-code.sh - - # tests/test-check-dead-code.sh invokes the script directly and never - # goes through pre-commit, so it can't catch a broken entry:/language: - # in .pre-commit-hooks.yaml — that would only surface in a consumer - # repo, after the tag is published. try-repo runs the hook exactly as - # a consumer's pre-commit would, without needing a published tag. - # - # `ops` itself has zero .py files, so pointing try-repo at it proves - # nothing — it can't tell "ran and found nothing" from "no-opped". Both - # steps below build a throwaway git repo (try-repo needs a real repo - # with a resolvable rev) with planted content and check the hook's - # actual verdict on it. - - run: pip install pre-commit==4.6.2 - - - name: Exercise the hook manifest through pre-commit (positive control) - run: | - set -euo pipefail - tmp="$(mktemp -d)" - cd "$tmp" - git init -q - git config user.email test@example.com - git config user.name test - printf 'def used():\n return 1\n\n\ndef planted_orphan():\n return 2\n\n\nprint(used())\n' > main.py - git add -A - if pre-commit try-repo "$GITHUB_WORKSPACE" dead-code --all-files 2>&1 | tee out.txt; then - echo "FAIL: hook did not catch planted dead code" >&2 - exit 1 - fi - grep -q planted_orphan out.txt || { echo "FAIL: hook ran but did not name the finding" >&2; exit 1; } - - - name: Exercise the hook manifest through pre-commit (negative control) - run: | - set -euo pipefail - tmp="$(mktemp -d)" - cd "$tmp" - git init -q - git config user.email test@example.com - git config user.name test - printf 'def used():\n return 1\n\n\nprint(used())\n' > main.py - git add -A - pre-commit try-repo "$GITHUB_WORKSPACE" dead-code --all-files diff --git a/.github/workflows/hooks.yml b/.github/workflows/hooks.yml new file mode 100644 index 0000000..a349adc --- /dev/null +++ b/.github/workflows/hooks.yml @@ -0,0 +1,70 @@ +name: hooks + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + # Matches the pin every consumer uses. + - run: pip install vulture==2.14 pre-commit==4.6.2 + + - name: Test check-dead-code.sh + env: + REQUIRE_VULTURE: 1 + run: bash tests/test-check-dead-code.sh + + - name: Test check-ruff-config.py + run: bash tests/test-check-ruff-config.sh + + # A try-repo run that can only ever pass proves nothing. Plant a finding, + # require the hook to catch it, then require it to pass once removed. + - name: Exercise the dead-code hook through pre-commit + run: | + set -euo pipefail + tmp="$(mktemp -d)"; cd "$tmp"; git init -q + printf 'def used():\n return 1\n\n\ndef planted_orphan():\n return 2\n\n\nprint(used())\n' > main.py + git add -A + if pre-commit try-repo "$GITHUB_WORKSPACE" dead-code --all-files 2>&1 | tee out.txt; then + echo "FAIL: hook did not catch planted dead code" >&2; exit 1 + fi + grep -q planted_orphan out.txt || { echo "FAIL: hook ran but did not name the finding" >&2; exit 1; } + printf 'def used():\n return 1\n\n\nprint(used())\n' > main.py + git add -A + pre-commit try-repo "$GITHUB_WORKSPACE" dead-code --all-files + + - name: Exercise the ruff-config hook through pre-commit + run: | + set -euo pipefail + tmp="$(mktemp -d)"; cd "$tmp"; git init -q + # A deliberately drifted config: the canonical select list minus SIM. + cat > pyproject.toml <<'TOML' + [project] + name = "drifted" + version = "0.1.0" + + [tool.ruff] + line-length = 120 + + [tool.ruff.lint] + select = ["E", "W", "F", "I", "B", "UP", "S"] + TOML + git add -A + if pre-commit try-repo "$GITHUB_WORKSPACE" ruff-config --all-files 2>&1 | tee out.txt; then + echo "FAIL: hook did not catch the drifted config" >&2; exit 1 + fi + grep -q "SIM" out.txt || { echo "FAIL: hook ran but did not name the missing rule" >&2; exit 1; } + # Now the compliant version: canonical keys verbatim plus a target-version. + { printf '[project]\nname = "compliant"\nversion = "0.1.0"\n\n'; cat "$GITHUB_WORKSPACE/ruff-shared.toml"; } > pyproject.toml + git add -A + pre-commit try-repo "$GITHUB_WORKSPACE" ruff-config --all-files diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 7133b11..f44146d 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -5,3 +5,10 @@ language: script pass_filenames: false always_run: true +- id: ruff-config + name: ruff config matches the shared standard + description: Fail if this repo's shared ruff keys have drifted from ops/ruff-shared.toml. + entry: check-ruff-config.py + language: script + pass_filenames: false + always_run: true diff --git a/README.md b/README.md index 7f24a72..e8f58ef 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ Small operational scripts and scheduled chores for Offworld Labs. | --- | --- | --- | | [`standup-nudge/`](standup-nudge/) | Posts a fixed standup prompt to the "Offworld Labs" ClickUp chat channel | 09:00 Europe/London, Mon–Fri | | [`check-dead-code.sh`](check-dead-code.sh) | Dead-code gate (vulture) consumed by the Python repos as a pre-commit hook | On every commit / CI run in consumer repos | +| [`ruff-shared.toml`](ruff-shared.toml) | Canonical ruff configuration, enforced in consumer repos by the `ruff-config` hook | On every commit / CI run in consumer repos | ## Shared pre-commit hooks @@ -24,21 +25,32 @@ and vendored drift cannot happen: repos: - repo: https://github.com/offworldlabs/ops - rev: dead-code-v1.1 + rev: hooks-v1.0 hooks: - - id: dead-code + - id: dead-code # requires vulture==2.14 on PATH + - id: ruff-config + +If a consumer's Python lives in a subdirectory, add `args: []` to both +hooks — Tower-Finder, whose backend is not at repo root, needs exactly that. | Hook | Script | Requires | | --- | --- | --- | | `dead-code` | [`check-dead-code.sh`](check-dead-code.sh) | `vulture==2.14` on `PATH` | +| `ruff-config` | [`check-ruff-config.py`](check-ruff-config.py) | Python 3.11+ (`tomllib`) | + +Hook versions are published as repo-level tags named `hooks-v.`. +**The dot is required, not cosmetic.** pre-commit warns "appears to be a mutable +reference" for any `rev` containing neither a `.` nor pure hex +(`clientlib.py`, `WarnMutableRev`), so `hooks-v1` would make every consumer +print a spurious warning on every run. + +Tags are repo-level rather than per-hook because pre-commit pins the whole +repository at one `rev` — two hooks cannot be versioned independently from a +single repo. The older `dead-code-v1.0` and `dead-code-v1.1` tags remain valid +for consumers that have not moved. -Hook versions are published as tags named `-v.`. **The dot is -required, not cosmetic.** pre-commit warns "appears to be a mutable reference" -for any `rev` containing neither a `.` nor pure hex, so a tag like -`dead-code-v1` makes every consumer print a spurious warning on every run -(`clientlib.py`, `WarnMutableRev`). To change a hook: edit -it here, run `bash tests/test-check-dead-code.sh`, merge, tag, then bump `rev` -in the consumers (`pre-commit autoupdate` does the bump for you). +To change a hook: edit it here, run both suites in `tests/`, merge, tag, then +bump `rev` in the consumers (`pre-commit autoupdate` does the bump for you). The "Adding a script" conventions below are about scheduled chores on the VPS and do not apply to hooks — a hook takes no env config and nothing schedules it. diff --git a/check-dead-code.sh b/check-dead-code.sh index a2a71a6..789e5cb 100755 --- a/check-dead-code.sh +++ b/check-dead-code.sh @@ -5,7 +5,7 @@ # Consumed by other repos as a pre-commit hook, pinned by rev: # # - repo: https://github.com/offworldlabs/ops -# rev: +# rev: # hooks: # - id: dead-code # @@ -13,7 +13,7 @@ # a consumer pinned, so any version written here is guaranteed wrong for every # release after it. The README on main is the current reference. # -# Do not vendor this file. Change it here, publish a dead-code-v. +# Do not vendor this file. Change it here, publish a hooks-v. # tag (the dot matters — pre-commit warns "mutable reference" without one), then # bump rev in the consumers (`pre-commit autoupdate` does that for you). # diff --git a/check-ruff-config.py b/check-ruff-config.py new file mode 100755 index 0000000..03bf8f5 --- /dev/null +++ b/check-ruff-config.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python3 +"""Fail if a repo's shared ruff keys have drifted from the canonical set. + +The canonical values live in ruff-shared.toml beside this script. pre-commit +clones this hook repo locally before running it, so that file is always a local +path at run time — which is what makes sharing possible at all: ruff's `extend` +accepts only local paths, and there is no rev:-style sharing for pyproject.toml. + +Comparison is SEMANTIC, not textual. `select`, `ignore` and the values inside +`per-file-ignores` are compared as sets, so comment whitespace and ordering are +irrelevant. This matters concretely: Tower-Finder's config is semantically +identical to the baseline but pads its comments differently, and a text diff +would report false drift. + +`target-version` is never compared. It tracks each package's requires-python and +is legitimately per-repo. + +Permissive by design: the canonical keys must be present and equal, but a repo +may add extra ignores or per-file-ignores entries. The accepted cost is that a +repo could ignore a rule from the shared select list without being flagged. + + check-ruff-config.py # check ./pyproject.toml + check-ruff-config.py backend # check backend/pyproject.toml + +Exit codes: 0 compliant, 1 drift, 2 usage error or missing pyproject.toml. +""" + +import sys +import tomllib +from pathlib import Path + +HERE = Path(__file__).resolve().parent +CANONICAL = HERE / "ruff-shared.toml" + + +def load(path: Path) -> dict: + with open(path, "rb") as handle: + return tomllib.load(handle) + + +def ruff_of(doc: dict) -> dict: + return doc.get("tool", {}).get("ruff", {}) + + +def main(argv: list[str]) -> int: + if len(argv) > 2: + print(f"check-ruff-config: expected at most one target directory, got {len(argv) - 1}", file=sys.stderr) + return 2 + + target = Path(argv[1]) if len(argv) == 2 else Path(".") + pyproject = target if target.is_file() else target / "pyproject.toml" + + if not pyproject.is_file(): + print(f"check-ruff-config: no pyproject.toml at {pyproject}", file=sys.stderr) + return 2 + if not CANONICAL.is_file(): + print(f"check-ruff-config: canonical config missing at {CANONICAL}", file=sys.stderr) + return 2 + + canon = ruff_of(load(CANONICAL)) + repo = ruff_of(load(pyproject)) + + problems: list[str] = [] + + if not repo: + problems.append("no [tool.ruff] section at all") + + if repo.get("line-length") != canon.get("line-length"): + problems.append( + f"line-length is {repo.get('line-length')!r}, canonical is {canon.get('line-length')!r}" + ) + + canon_lint = canon.get("lint", {}) + repo_lint = repo.get("lint", {}) + + for key in ("select", "ignore"): + missing = sorted(set(canon_lint.get(key, [])) - set(repo_lint.get(key, []))) + if missing: + problems.append(f"lint.{key} is missing {missing}") + + canon_pfi = canon_lint.get("per-file-ignores", {}) + repo_pfi = repo_lint.get("per-file-ignores", {}) + for pattern, rules in canon_pfi.items(): + if pattern not in repo_pfi: + problems.append(f"lint.per-file-ignores is missing {pattern!r}") + elif set(repo_pfi[pattern]) != set(rules): + problems.append( + f"lint.per-file-ignores[{pattern!r}] is {sorted(repo_pfi[pattern])}, " + f"canonical is {sorted(rules)}" + ) + + if problems: + print(f"check-ruff-config: {pyproject} has drifted from the shared ruff standard", file=sys.stderr) + for problem in problems: + print(f" - {problem}", file=sys.stderr) + print(file=sys.stderr) + print("Copy the entries above into this repo's pyproject.toml to match the", file=sys.stderr) + print("shared standard. If the standard itself should change instead, edit", file=sys.stderr) + print("offworldlabs/ops:ruff-shared.toml and publish a new hooks-v*.* tag.", file=sys.stderr) + return 1 + + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/ruff-shared.toml b/ruff-shared.toml new file mode 100644 index 0000000..cd0491d --- /dev/null +++ b/ruff-shared.toml @@ -0,0 +1,51 @@ +# Canonical shared ruff configuration for offworldlabs Python repos. +# +# Consumers copy these keys into their own pyproject.toml and the `ruff-config` +# pre-commit hook checks they have not drifted. Copying rather than referencing +# is forced: ruff's `extend` accepts only a local path, so there is no way for +# one repo to point at another's config. +# +# target-version is deliberately absent. It tracks each package's +# requires-python and is legitimately per-repo, so the checker ignores it. + +[tool.ruff] +line-length = 120 + +[tool.ruff.lint] +select = [ + "E", # pycodestyle errors + "W", # pycodestyle warnings + "F", # pyflakes + "I", # isort + "B", # flake8-bugbear + "UP", # pyupgrade + "S", # flake8-bandit (security) + "SIM", # flake8-simplify +] +ignore = [ + "E501", # line too long — handled by formatter + "E402", # module-level import not at top — env setup before imports is intentional + "S101", # assert in tests is fine + "S104", # binding to 0.0.0.0 is intentional (Docker) + "S105", # hardcoded password false positives on dev defaults + "S106", # hardcoded password false positives + "S110", # try-except-pass is used intentionally + "S112", # try-except-continue is intentional in iteration + "S310", # URL open audit — URLs are constructed internally + "S311", # pseudo-random is fine for non-crypto uses + "S501", # requests without verify — internal calls + "S603", # subprocess calls are in controlled scripts + "S607", # partial executable path is fine for scripts + "B008", # function call in default arg — Depends() is FastAPI pattern + "B905", # zip strict — not needed everywhere + "SIM102", # nested if — readability preference + "SIM105", # contextlib.suppress — try/except is more explicit + "SIM108", # ternary operator — readability preference + "SIM117", # combine with statements — readability preference + "UP017", # datetime.UTC — cosmetic, timezone.utc is fine + "UP028", # yield from — explicit loop is clearer +] + +[tool.ruff.lint.per-file-ignores] +"tests/*" = ["S", "B"] +"scripts/*" = ["S", "E"] diff --git a/tests/test-check-ruff-config.sh b/tests/test-check-ruff-config.sh new file mode 100644 index 0000000..a6483d0 --- /dev/null +++ b/tests/test-check-ruff-config.sh @@ -0,0 +1,198 @@ +#!/usr/bin/env bash +# +# Tests for check-ruff-config.py. Plain bash asserts, matching the style of +# tests/test-check-dead-code.sh. +# +# bash tests/test-check-ruff-config.sh + +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPT="$HERE/../check-ruff-config.py" +CANON="$HERE/../ruff-shared.toml" + +passed=0 +failed=0 +ok() { printf ' ok %s\n' "$1"; passed=$((passed + 1)); } +bad() { printf ' FAIL %s\n %s\n' "$1" "$2"; failed=$((failed + 1)); } + +# A pyproject carrying exactly the canonical keys, plus a per-repo +# target-version, which is what a compliant consumer looks like. +mkgood() { + local dir; dir="$(mktemp -d)" + { + printf '[project]\nname = "x"\nversion = "0.1.0"\n\n' + # reuse the canonical file verbatim, then add target-version + cat "$CANON" + printf '\n' + } >"$dir/pyproject.toml" + python3 - "$dir/pyproject.toml" <<'PY' +import sys, pathlib +p = pathlib.Path(sys.argv[1]); s = p.read_text() +s = s.replace("[tool.ruff]\nline-length = 120", + '[tool.ruff]\nline-length = 120\ntarget-version = "py310"') +p.write_text(s) +PY + printf '%s' "$dir" +} + +t_matching_config_passes() { + local dir out rc + dir="$(mkgood)" + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + if [ "$rc" -eq 0 ]; then ok "canonical config passes" + else bad "canonical config passes" "rc=$rc out=$out"; fi + rm -rf "$dir" +} + +t_missing_select_fails() { + local dir out rc + dir="$(mkgood)" + python3 - "$dir/pyproject.toml" <<'PY' +import sys, pathlib +p = pathlib.Path(sys.argv[1]); s = p.read_text() +s = s.replace(' "SIM", # flake8-simplify\n', '') +p.write_text(s) +PY + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + if [ "$rc" -eq 1 ] && [[ "$out" == *"SIM"* ]]; then + ok "a missing select entry fails and names it" + else bad "missing select fails" "rc=$rc out=$out"; fi + rm -rf "$dir" +} + +t_missing_ignore_fails() { + local dir out rc + dir="$(mkgood)" + python3 - "$dir/pyproject.toml" <<'PY' +import sys, pathlib +p = pathlib.Path(sys.argv[1]); s = p.read_text() +s = s.replace(' "B905", # zip strict — not needed everywhere\n', '') +p.write_text(s) +PY + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + if [ "$rc" -eq 1 ] && [[ "$out" == *"B905"* ]]; then + ok "a missing ignore entry fails and names it" + else bad "missing ignore fails" "rc=$rc out=$out"; fi + rm -rf "$dir" +} + +# The negative control. A check that fires on legitimate variation gets +# disabled within a month, so this matters as much as detecting real drift. +t_target_version_is_ignored() { + local dir out rc + dir="$(mkgood)" + python3 - "$dir/pyproject.toml" <<'PY' +import sys, pathlib +p = pathlib.Path(sys.argv[1]); s = p.read_text() +p.write_text(s.replace('target-version = "py310"', 'target-version = "py312"')) +PY + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + if [ "$rc" -eq 0 ]; then ok "a differing target-version still passes" + else bad "target-version ignored" "rc=$rc out=$out"; fi + rm -rf "$dir" +} + +# Permissive by design: extra local rules are additions, not drift. +t_extra_entries_permitted() { + local dir out rc + dir="$(mkgood)" + python3 - "$dir/pyproject.toml" <<'PY' +import sys, pathlib +p = pathlib.Path(sys.argv[1]); s = p.read_text() +s = s.replace(' "UP028", # yield from — explicit loop is clearer\n', + ' "UP028", # yield from — explicit loop is clearer\n "C901", # local addition\n') +s = s.replace('"scripts/*" = ["S", "E"]', '"scripts/*" = ["S", "E"]\n"simulation/*" = ["S"]') +p.write_text(s) +PY + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + if [ "$rc" -eq 0 ]; then ok "extra ignores and per-file-ignores are permitted" + else bad "extra entries permitted" "rc=$rc out=$out"; fi + rm -rf "$dir" +} + +t_missing_per_file_ignore_fails() { + local dir out rc + dir="$(mkgood)" + python3 - "$dir/pyproject.toml" <<'PY' +import sys, pathlib +p = pathlib.Path(sys.argv[1]); s = p.read_text() +p.write_text(s.replace('"scripts/*" = ["S", "E"]\n', '')) +PY + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + if [ "$rc" -eq 1 ] && [[ "$out" == *"scripts/*"* ]]; then + ok "a missing per-file-ignores entry fails and names it" + else bad "missing per-file-ignores fails" "rc=$rc out=$out"; fi + rm -rf "$dir" +} + +# Whitespace and ordering must not matter — Tower-Finder is semantically +# identical to the baseline but formats its comments differently. +t_whitespace_and_order_ignored() { + local dir out rc + dir="$(mkgood)" + python3 - "$dir/pyproject.toml" <<'PY' +import sys, pathlib, re +p = pathlib.Path(sys.argv[1]); s = p.read_text() +s = re.sub(r'",\s+#', '", #', s) # collapse comment padding +s = s.replace(' "E", # pycodestyle errors\n', '') +s = s.replace('select = [\n', 'select = [\n "E", # moved to the end later\n') +p.write_text(s) +PY + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + if [ "$rc" -eq 0 ]; then ok "comment whitespace and ordering are ignored" + else bad "whitespace/order ignored" "rc=$rc out=$out"; fi + rm -rf "$dir" +} + +# Tower-Finder's shape: config lives in a subdirectory. +t_subdirectory_argument() { + local root dir out rc + root="$(mktemp -d)"; mkdir -p "$root/backend" + dir="$(mkgood)" + mv "$dir/pyproject.toml" "$root/backend/pyproject.toml"; rmdir "$dir" + out="$(cd "$root" && python3 "$SCRIPT" backend 2>&1)"; rc=$? + if [ "$rc" -eq 0 ]; then ok "positional target dir finds backend/pyproject.toml" + else bad "subdirectory argument" "rc=$rc out=$out"; fi + rm -rf "$root" +} + +t_missing_pyproject_exits_2() { + local dir out rc + dir="$(mktemp -d)" + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + # Assert the message, not just the code: python itself exits 2 when it + # cannot open the script, so a bare rc check passes even when + # check-ruff-config.py does not exist. + if [ "$rc" -eq 2 ] && [[ "$out" == *"no pyproject.toml at"* ]]; then + ok "no pyproject.toml exits 2 and says so" + else + bad "no pyproject exits 2 and says so" "rc=$rc out=$out" + fi + rm -rf "$dir" +} + +t_no_ruff_section_fails() { + local dir out rc + dir="$(mktemp -d)" + printf '[project]\nname = "x"\nversion = "0.1.0"\n' >"$dir/pyproject.toml" + out="$(python3 "$SCRIPT" "$dir" 2>&1)"; rc=$? + if [ "$rc" -eq 1 ]; then ok "a pyproject with no [tool.ruff] fails" + else bad "no ruff section fails" "rc=$rc out=$out"; fi + rm -rf "$dir" +} + +echo "checker:" +t_matching_config_passes +t_missing_select_fails +t_missing_ignore_fails +t_target_version_is_ignored +t_extra_entries_permitted +t_missing_per_file_ignore_fails +t_whitespace_and_order_ignored +t_subdirectory_argument +t_missing_pyproject_exits_2 +t_no_ruff_section_fails + +printf '\n%d passed, %d failed\n' "$passed" "$failed" +[ "$failed" -eq 0 ]