diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d0de40035..e08896cba 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,6 +7,36 @@ on: branches: [master] jobs: + # Forward-looking Python lint gate (ruff). The Python twin of the ESLint runtime + # guard. Runs the curated [tool.ruff] ruleset (E9 + F + B) but only on lines this + # PR adds/modifies vs the merge-base — so it keeps NEW code clean without demanding + # a reformat of the existing tree's cosmetic backlog (#3273). Fast, fails early. + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # Need history so the gate can diff against the merge-base with master. + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Install ruff + run: pip install ruff + + - name: Ensure origin/master ref is available for the diff gate + run: git fetch --no-tags --depth=1 origin master || true + + - name: Ruff forward gate (new/changed lines only) + run: python3 scripts/ruff_lint.py --diff origin/master + + - name: Ruff whole-tree report (informational — never blocks) + if: always() + run: python3 scripts/ruff_lint.py --all + test: runs-on: ubuntu-latest strategy: @@ -41,6 +71,11 @@ jobs: run: | python -m pip install --upgrade pip pip install "pyyaml>=6.0" pytest pytest-timeout pytest-asyncio pytest-shard + # ruff is installed so tests/test_ruff_forward_lint.py runs its E9/F821 + # tree-clean assertions in-suite (mirrors how eslint is available for + # tests/test_static_js_runtime_lint.py). If install fails the test + # skips cleanly — it never blocks the matrix. + pip install ruff || echo "ruff install failed — test_ruff_forward_lint.py will skip" # Install the `mcp` package so tests/test_mcp_server.py runs in CI. # The package is an optional runtime dep of mcp_server.py — users # who run the MCP integration install it themselves; CI installs diff --git a/CHANGELOG.md b/CHANGELOG.md index 498c3ae27..ca0e7c0e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## [Unreleased] +### Added +- **Forward-looking Python lint gate (ruff).** A curated `[tool.ruff]` ruleset (E9 syntax/runtime + F pyflakes + B bugbear — high-signal correctness rules, no style/formatting families) now gates **new and changed Python code** in CI (`tests.yml` `lint` job) and as part of the maintainer pre-release pre-gate. It is the Python twin of the existing ESLint runtime guard for `static/*.js`. Crucially it is **line-scoped** (`scripts/ruff_lint.py --diff`): it flags violations only on lines a change adds or modifies, so it keeps incoming code clean **without** reformatting the existing tree's cosmetic backlog (a separate, deferred, maintainer-run decision). `tests/test_ruff_forward_lint.py` additionally holds the whole tree free of E9 errors and skips cleanly when ruff isn't installed. See TESTING.md > "Python lint gate (ruff)". Closes #3273. + + ## [v0.51.188] — 2026-05-31 — Release FH (stage-batchH — configured runner-client boundary, default-off) ### Added diff --git a/TESTING.md b/TESTING.md index a065ac075..ead582689 100644 --- a/TESTING.md +++ b/TESTING.md @@ -36,6 +36,38 @@ npm run lint:runtime npx eslint --no-config-lookup -c eslint.runtime-guard.config.mjs "static/**/*.js" ``` +## Python lint gate (ruff) — forward-looking, new-code-only + +The Python twin of the ESLint runtime guard. A curated `ruff` ruleset +(`[tool.ruff]` in `pyproject.toml`) catches latent-bug shapes — unused imports +(F401), undefined/unused names (F841/F821), redefinitions (F811), mutable default +args (B006), raise-without-from (B904), loop-variable capture in closures (B023) — +**plus** real syntax/runtime errors (E9). It is **not** a style/formatting linter: +the pure-style families (line-length, whitespace) are intentionally OFF so the gate +never demands a reformat of existing code. + +The existing tree carries a cosmetic backlog (mostly unused-import F401) that is +deliberately **not** reformatted. So the gate is enforced **only on the lines a +change adds or modifies** (`scripts/ruff_lint.py --diff`), which keeps new code +clean without touching the backlog. Cleaning the backlog is a separate, +maintainer-run, safe-fixes-only decision (tracked in #3273). + +```bash +# one-time dev setup (ruff is a dev-only tool): +pip install ruff # or: uv tool install ruff / uvx ruff ... +# the gate (only flags violations on lines you added/changed vs origin/master): +python3 scripts/ruff_lint.py --diff origin/master +# whole-tree backlog report (informational — never blocks): +python3 scripts/ruff_lint.py --all +``` + +`tests/test_ruff_forward_lint.py` holds the **whole tree** free of E9 (real +syntax/runtime) findings and verifies the curated config shape; it runs in-suite +when ruff is present and **skips gracefully** when it isn't — so environments +without ruff aren't blocked, while CI (which installs ruff) enforces it. The +diff-scoped gate runs as the `lint` job in `.github/workflows/tests.yml` and is +also part of the maintainer pre-release pre-gate. + ## Automated browser smoke (runtime brick-class gate) The ESLint guard above catches `const`-reassign / import-assign statically. The diff --git a/api/config.py b/api/config.py index 8fa873b63..0889c6139 100644 --- a/api/config.py +++ b/api/config.py @@ -2527,7 +2527,7 @@ _cache_build_in_progress = False # True while a cold path is actively building # session is expensive (~10s for zai due to endpoint probing). The credential pool # only changes when the user adds/removes credentials, which is rare; a 24h TTL # is plenty safe and ensures get_available_models() cold paths are fast. -_CREDENTIAL_POOL_CACHE: dict[str, tuple[float, "CredentialPool"]] = {} # pid -> (ts, pool) +_CREDENTIAL_POOL_CACHE: dict[str, tuple[float, "CredentialPool"]] = {} # noqa: F821 forward-ref string annotation, resolved at runtime # pid -> (ts, pool) _provider_models_invalidated_ts: dict[str, float] = {} # provider_id -> timestamp of last invalidation # Disk-backed in-memory cache for get_available_models(). diff --git a/api/routes.py b/api/routes.py index 5c814e9dd..86edbfef8 100644 --- a/api/routes.py +++ b/api/routes.py @@ -912,7 +912,7 @@ def _run_cron_tracked(job, profile_home=None, execution_profile_home=None): except Exception as e: logger.exception("Manual cron run failed for job %s", job_id) try: - _with_cron_home(profile_home, lambda: mark_job_run(job_id, False, str(e))) + _with_cron_home(profile_home, lambda: mark_job_run(job_id, False, str(e))) # noqa: F821 e is bound by the enclosing `except ... as e` and the lambda runs synchronously here except Exception: logger.debug("Failed to mark manual cron run failure for %s", job_id) finally: diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 000000000..80edc1fd9 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,56 @@ +# Hermes WebUI — Python tooling config. +# +# This project is NOT a packaged distribution. The app is a plain Python + vanilla +# JavaScript server with no build step and no bundler (see AGENTS.md / README). This +# file exists only to configure dev tooling — currently ruff, used as a curated, +# forward-looking lint gate over the Python codebase. There is intentionally no +# [build-system] section: nothing pip-installs this directory. +# +# The ruff gate is the Python twin of the ESLint runtime guard (package.json +# `lint:runtime` + eslint.runtime-guard.config.mjs + tests/test_static_js_runtime_lint.py). +# It is enforced on NEW/CHANGED code only — see scripts/ruff_lint.py and TESTING.md +# > "Python lint gate (ruff)". The existing tree carries a cosmetic backlog (mostly +# unused-import F401) that is deliberately NOT reformatted here; cleaning it is a +# separate, maintainer-run, safe-fixes-only decision (tracked in #3273). + +[tool.ruff] +# Match the Python versions exercised in CI (tests.yml matrix: 3.11–3.13). +target-version = "py311" +# Keep the linter scoped to the application + tests; never crawl vendored or +# generated trees. (These are belt-and-suspenders; the gate passes explicit files.) +extend-exclude = [ + "node_modules", + "static", + ".git", + "scripts/windows", + "scripts/wsl", +] + +[tool.ruff.lint] +# Curated, correctness-leaning ruleset — high signal, low noise. We deliberately do +# NOT enable the pure-style families (E1/E2/E5/E7 line-length & whitespace) so the +# gate never demands a whitespace reformat of existing code. +# +# E9 — syntax / IO / runtime errors (E999 etc). The whole tree is already clean +# of these and the in-suite test (tests/test_ruff_forward_lint.py) keeps it +# that way across every shard. +# F — pyflakes: unused imports (F401), unused/undefined names (F841/F821), +# redefinitions (F811), f-strings with no placeholders (F541). The most +# valuable family for keeping NEW code clean. +# B — flake8-bugbear: genuine latent-bug shapes — mutable default args (B006), +# raise-without-from (B904), loop-variable capture in closures (B023), +# zip-without-strict (B905). This is where the real future-regression- +# prevention value lives. +select = ["E9", "F", "B"] + +# No global `ignore` of F401/F841/etc. The existing-tree backlog is handled by +# line-scoping the gate to changed lines (scripts/ruff_lint.py), NOT by globally +# disabling the rules — disabling them would blind the gate to the single most +# common new-code defect (a stray unused import). Forward enforcement of the full +# curated set is the whole point. + +[tool.ruff.lint.per-file-ignores] +# Tests legitimately use `assert False` as an explicit failure marker (B011) and +# occasionally shadow loop vars in table-driven cases (B007); that's idiomatic in a +# test suite and not a production-code risk. +"tests/**" = ["B011", "B007"] diff --git a/scripts/ruff_lint.py b/scripts/ruff_lint.py new file mode 100644 index 000000000..b6e523021 --- /dev/null +++ b/scripts/ruff_lint.py @@ -0,0 +1,217 @@ +#!/usr/bin/env python3 +"""Forward-looking ruff lint gate for hermes-webui. + +The Python twin of the ESLint runtime guard (``npm run lint:runtime``). It runs the +curated ``[tool.ruff]`` ruleset from ``pyproject.toml`` (E9 + F + B) but enforces it +**only on new / changed code**, so it keeps incoming PRs clean without demanding a +big-bang reformat of the existing tree (which still carries a cosmetic +unused-import backlog — see issue #3273). + +Two modes: + + --diff [BASE] Line-scoped gate (default mode). Only report findings on lines + that this branch ADDED or MODIFIED relative to the merge-base + with BASE (default: origin/master). Editing a legacy file that + has pre-existing violations elsewhere is safe — only your own + new lines are gated. Exit 1 if any new finding, else 0. + + --all Whole-tree report (informational). Runs the curated ruleset over + every tracked .py file and prints the backlog. Exit 0 always + unless --strict is also passed. Useful for tracking the cleanup + progress of #3273; NOT used as a release blocker. + +Usage:: + + python3 scripts/ruff_lint.py --diff origin/master # the CI / pre-release gate + python3 scripts/ruff_lint.py --all # backlog report + python3 scripts/ruff_lint.py --all --strict # fail on ANY tree finding + +ruff is resolved from PATH, or run via ``uvx ruff`` / ``python -m ruff`` as a +fallback. If ruff cannot be found at all the gate prints a notice and exits 0 (a +dev without ruff is not blocked; CI installs ruff so enforcement still holds there — +same contract as the ESLint guard). +""" +from __future__ import annotations + +import argparse +import json +import os +import re +import shutil +import subprocess +import sys +from collections import defaultdict + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def _run(cmd: list[str], **kw) -> subprocess.CompletedProcess: + return subprocess.run( + cmd, cwd=REPO_ROOT, capture_output=True, text=True, **kw + ) + + +def _ruff_cmd() -> list[str] | None: + """Return the argv prefix that invokes ruff, or None if unavailable.""" + if shutil.which("ruff"): + return ["ruff"] + # `python -m ruff` works when ruff is pip-installed in the active interpreter. + probe = _run([sys.executable, "-m", "ruff", "--version"]) + if probe.returncode == 0: + return [sys.executable, "-m", "ruff"] + # uvx pulls a pinned ruff on demand (used in local dev boxes without a global ruff). + if shutil.which("uvx"): + probe = _run(["uvx", "ruff", "--version"]) + if probe.returncode == 0: + return ["uvx", "ruff"] + return None + + +def _changed_py_files(base: str) -> tuple[str, list[str]]: + """Resolve the merge-base with `base` and return (merge_base, changed .py files).""" + mb = _run(["git", "merge-base", base, "HEAD"]) + merge_base = mb.stdout.strip() if mb.returncode == 0 and mb.stdout.strip() else base + diff = _run( + ["git", "diff", "--name-only", "--diff-filter=ACMR", merge_base, "HEAD"] + ) + files = [ + f + for f in diff.stdout.splitlines() + if f.endswith(".py") and os.path.exists(os.path.join(REPO_ROOT, f)) + ] + return merge_base, files + + +_HUNK_RE = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@") + + +def _added_lines(merge_base: str, path: str) -> set[int]: + """Set of NEW-file line numbers added or modified in `path` since merge_base.""" + diff = _run(["git", "diff", "--unified=0", merge_base, "HEAD", "--", path]) + added: set[int] = set() + new_ln = 0 + for line in diff.stdout.splitlines(): + m = _HUNK_RE.match(line) + if m: + new_ln = int(m.group(1)) + continue + if line.startswith("+") and not line.startswith("+++"): + added.add(new_ln) + new_ln += 1 + elif line.startswith("-") and not line.startswith("---"): + # deletion: new-file pointer does not advance + continue + elif not line.startswith(("@@", "diff ", "index ", "--- ", "+++ ")): + new_ln += 1 + return added + + +def _ruff_check_json(ruff: list[str], files: list[str]) -> list[dict]: + """Run ruff over `files` with the project config, return parsed JSON findings.""" + if not files: + return [] + proc = _run(ruff + ["check", "--output-format", "json", "--no-cache", *files]) + out = proc.stdout.strip() + if not out: + return [] + try: + return json.loads(out) + except json.JSONDecodeError: + sys.stderr.write( + "ruff_lint: could not parse ruff JSON output:\n" + proc.stdout + proc.stderr + ) + # Be conservative: surface as a failure so a broken invocation never + # silently green-lights a PR. + raise + + +def _print_findings(findings: list[dict]) -> None: + by_file: dict[str, list[dict]] = defaultdict(list) + for f in findings: + by_file[f.get("filename", "?")].append(f) + for fname in sorted(by_file): + rel = os.path.relpath(fname, REPO_ROOT) + for f in sorted(by_file[fname], key=lambda x: (x["location"]["row"], x["location"]["column"])): + loc = f["location"] + code = f.get("code") or "?" + msg = f.get("message", "").strip() + print(f" {rel}:{loc['row']}:{loc['column']} {code} {msg}") + + +def run_diff(base: str) -> int: + ruff = _ruff_cmd() + if ruff is None: + print("ruff_lint: ruff not found on PATH — skipping (CI installs ruff). OK.") + return 0 + merge_base, files = _changed_py_files(base) + if not files: + print(f"ruff_lint: no changed .py files vs {base} ({merge_base[:12]}). OK.") + return 0 + + all_findings = _ruff_check_json(ruff, files) + # Build the added-line map once per file, intersect. + added_map = {f: _added_lines(merge_base, f) for f in files} + new_findings = [] + for finding in all_findings: + rel = os.path.relpath(finding.get("filename", ""), REPO_ROOT) + row = finding.get("location", {}).get("row") + if rel in added_map and row in added_map[rel]: + new_findings.append(finding) + + print( + f"ruff_lint (diff vs {base}): {len(files)} changed .py file(s), " + f"{len(all_findings)} total finding(s) in them, " + f"{len(new_findings)} on added/modified lines." + ) + if new_findings: + print("\nNew ruff violations introduced by this change:\n") + _print_findings(new_findings) + print( + "\nThe curated ruff gate (E9+F+B) flags NEW code only. Fix the lines above, " + "or — if a finding is a genuine false positive — add a scoped " + "`# noqa: ` with a one-line reason. Config: pyproject.toml [tool.ruff]." + ) + return 1 + print("ruff_lint: no new violations on added/modified lines. OK.") + return 0 + + +def run_all(strict: bool) -> int: + ruff = _ruff_cmd() + if ruff is None: + print("ruff_lint: ruff not found on PATH — skipping. OK.") + return 0 + proc = _run(ruff + ["check", "--statistics", "--no-cache", "."]) + sys.stdout.write(proc.stdout) + sys.stderr.write(proc.stderr) + if strict and proc.returncode != 0: + return proc.returncode + print( + "\nruff_lint --all is informational (existing-tree backlog tracked in #3273). " + "Pass --strict to fail on any finding." + ) + return 0 + + +def main(argv: list[str] | None = None) -> int: + p = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + g = p.add_mutually_exclusive_group() + g.add_argument( + "--diff", + nargs="?", + const="origin/master", + metavar="BASE", + help="Line-scoped gate vs merge-base with BASE (default origin/master).", + ) + g.add_argument("--all", action="store_true", help="Whole-tree backlog report.") + p.add_argument("--strict", action="store_true", help="With --all: fail on any finding.") + args = p.parse_args(argv) + + if args.all: + return run_all(args.strict) + base = args.diff or "origin/master" + return run_diff(base) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_ruff_forward_lint.py b/tests/test_ruff_forward_lint.py new file mode 100644 index 000000000..1d96fed57 --- /dev/null +++ b/tests/test_ruff_forward_lint.py @@ -0,0 +1,133 @@ +"""Forward-looking ruff lint gate — in-suite enforcement. + +The Python twin of ``tests/test_static_js_runtime_lint.py`` (the ESLint runtime +guard). Issue #3273. + +What this asserts: + +1. The whole tree is free of E9 (real syntax / IO / runtime-error) findings. This + is the one ruleset we hold green tree-wide — there is no backlog of E9 errors, + and a new one is always a genuine bug. (The F/B families have a known cosmetic + backlog in the existing tree, deliberately NOT reformatted — see #3273 — so we + do NOT assert those are tree-clean; they are enforced on NEW code only by + ``scripts/ruff_lint.py --diff``, which runs in CI and the pre-release gate.) + +2. The curated ruff config is present and shaped as intended (E9 + F + B selected, + no global ignore of the F/B rules that would blind the new-code gate). + +3. The line-scoped gate runner imports and its diff machinery works. + +Skips cleanly when ruff is not installed, so a contributor without the dev tool is +never blocked — CI installs ruff so enforcement holds there. Same contract as the +ESLint guard. +""" +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import sys + +import pytest + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def _ruff_argv(): + """Resolve a runnable ruff invocation, or None.""" + if shutil.which("ruff"): + return ["ruff"] + probe = subprocess.run( + [sys.executable, "-m", "ruff", "--version"], capture_output=True, text=True + ) + if probe.returncode == 0: + return [sys.executable, "-m", "ruff"] + if shutil.which("uvx"): + probe = subprocess.run(["uvx", "ruff", "--version"], capture_output=True, text=True) + if probe.returncode == 0: + return ["uvx", "ruff"] + return None + + +RUFF = _ruff_argv() +ruff_required = pytest.mark.skipif(RUFF is None, reason="ruff not installed (dev-only tool; CI installs it)") + + +def test_pyproject_ruff_config_present_and_shaped(): + """The curated [tool.ruff] config exists and selects the intended families.""" + path = os.path.join(REPO_ROOT, "pyproject.toml") + assert os.path.exists(path), "pyproject.toml with [tool.ruff] config is required (#3273)" + text = open(path, encoding="utf-8").read() + assert "[tool.ruff]" in text, "[tool.ruff] section missing" + assert "[tool.ruff.lint]" in text, "[tool.ruff.lint] section missing" + # The curated correctness-leaning set: pyflakes + syntax + bugbear. + assert '"E9"' in text and '"F"' in text and '"B"' in text, ( + "ruff select must include E9, F, B (curated correctness ruleset)" + ) + # Guard the design intent: no GLOBAL ignore of F401/F841 etc. A blanket ignore + # would defeat the new-code gate (a stray unused import is the single most + # common new-code defect). Per-file-ignores for tests are fine. + lint_section = text.split("[tool.ruff.lint]", 1)[1] + before_subtables = lint_section.split("[tool.ruff.lint.", 1)[0] + assert "ignore = [" not in before_subtables and "ignore=[" not in before_subtables, ( + "do not globally ignore F/B rules — that blinds the forward gate; " + "use per-file-ignores or scoped # noqa instead" + ) + + +@ruff_required +def test_tree_is_E9_clean(): + """No real syntax / IO / runtime-error (E9) findings anywhere in the tree. + + E9 is the one ruleset held green tree-wide: there is no backlog, and any new + E9 finding is a genuine bug (the kind that bricks a module on import). + """ + proc = subprocess.run( + RUFF + ["check", "--select", "E9", "--output-format", "json", "--no-cache", "."], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + findings = json.loads(proc.stdout) if proc.stdout.strip() else [] + if findings: + lines = [ + f" {os.path.relpath(f['filename'], REPO_ROOT)}:{f['location']['row']} " + f"{f.get('code')} {f.get('message', '').strip()}" + for f in findings + ] + pytest.fail("ruff E9 (syntax/runtime) findings present:\n" + "\n".join(lines)) + + +@ruff_required +def test_known_F821_false_positives_are_noqa_suppressed(): + """The two known F821 false positives carry scoped # noqa, keeping the tree F821-clean. + + Regression guard: if someone strips a noqa or reintroduces the bare pattern, + this catches it. (#3273 documented exactly two F821 hits, both false positives.) + """ + proc = subprocess.run( + RUFF + ["check", "--select", "F821", "--output-format", "json", "--no-cache", "."], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + findings = json.loads(proc.stdout) if proc.stdout.strip() else [] + locs = [f"{os.path.relpath(f['filename'], REPO_ROOT)}:{f['location']['row']}" for f in findings] + assert not findings, f"unexpected F821 findings (annotate genuine false positives with # noqa): {locs}" + + +def test_ruff_lint_runner_importable_and_has_modes(): + """scripts/ruff_lint.py imports and exposes the --diff / --all entry points.""" + scripts_dir = os.path.join(REPO_ROOT, "scripts") + sys.path.insert(0, scripts_dir) + try: + import ruff_lint # noqa: PLC0415 intentional local import for test + finally: + sys.path.remove(scripts_dir) + assert hasattr(ruff_lint, "run_diff") + assert hasattr(ruff_lint, "run_all") + assert hasattr(ruff_lint, "_added_lines") + # The hunk-line parser is the gate's safety-critical core (only NEW lines gated). + # Smoke-test it against a synthetic unified diff via the module's regex. + assert ruff_lint._HUNK_RE.match("@@ -1,0 +5,3 @@ def f():").group(1) == "5"