mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-11 12:17:45 +02:00
[haik]: rename _sanitize_server_name to sanitize_mcp_server_name (mcp_config canonical, agent_manager, prompt_context, main, tests, toolLabels comment), add no-underscore-names linter check banning leading-underscore Python names in backend, extract _models.py from classes.py for shared pydantic BaseModel field detection used by both classes and vulture checks, add vulture-ignore inline comment suppression and automatic pydantic field-name whitelisting to vulture.py, remove leaked payload arg from client.py sync, update README with new check docs and inline-ignore usage
This commit is contained in:
haikdc
@@ -16,7 +16,6 @@ from backend.apps.agents.core.ws_manager import ws_manager
|
||||
from backend.apps.settings.settings import load_settings
|
||||
from backend.apps.tools_lib.tools_lib import (
|
||||
_load_all as load_all_tools,
|
||||
_sanitize_server_name,
|
||||
derive_mcp_config,
|
||||
load_builtin_permissions,
|
||||
load_trusted_sensitive_paths,
|
||||
@@ -25,6 +24,7 @@ from backend.apps.tools_lib.tools_lib import (
|
||||
refresh_hubspot_token,
|
||||
save_trusted_sensitive_paths,
|
||||
)
|
||||
from backend.apps.tools_lib.mcp_config import sanitize_mcp_server_name
|
||||
from backend.apps.agents.core.error_classify import (
|
||||
_is_auth_error,
|
||||
_is_free_trial_exhausted,
|
||||
@@ -173,7 +173,7 @@ class AgentManager:
|
||||
logger.info(f"[MCP-DEBUG] SKIPPED {tool.name}: '{tool_ref}' not in allowed_tools")
|
||||
continue
|
||||
|
||||
server_name = _sanitize_server_name(tool.name)
|
||||
server_name = sanitize_mcp_server_name(tool.name)
|
||||
if active_set is not None and server_name not in active_set:
|
||||
logger.info(f"[MCP-DEBUG] GATED {server_name}: not in session.active_mcps, model must call MCPActivate first")
|
||||
continue
|
||||
@@ -664,7 +664,7 @@ class AgentManager:
|
||||
for t in load_all_tools():
|
||||
if not t.mcp_config or not t.enabled:
|
||||
continue
|
||||
if _sanitize_server_name(t.name) == server_slug:
|
||||
if sanitize_mcp_server_name(t.name) == server_slug:
|
||||
return t.tool_permissions.get(mcp_tool_name, "ask")
|
||||
return _default_for(tool_name)
|
||||
|
||||
@@ -1039,7 +1039,7 @@ class AgentManager:
|
||||
# Emit a context_status event so the model and UI both know.
|
||||
try:
|
||||
_enabled = {
|
||||
_sanitize_server_name(t.name)
|
||||
sanitize_mcp_server_name(t.name)
|
||||
for t in load_all_tools()
|
||||
if t.mcp_config and t.enabled and t.auth_status in ("configured", "connected")
|
||||
}
|
||||
@@ -1340,7 +1340,7 @@ class AgentManager:
|
||||
|
||||
tool_def = next(
|
||||
(t for t in all_tools_list
|
||||
if t.mcp_config and t.enabled and _sanitize_server_name(t.name) == name),
|
||||
if t.mcp_config and t.enabled and sanitize_mcp_server_name(t.name) == name),
|
||||
None,
|
||||
)
|
||||
if tool_def:
|
||||
|
||||
@@ -3,8 +3,8 @@ from typing import Callable
|
||||
from backend.apps.modes.modes import load_mode
|
||||
from backend.apps.tools_lib.tools_lib import (
|
||||
_load_all as load_all_tools,
|
||||
_sanitize_server_name,
|
||||
)
|
||||
from backend.apps.tools_lib.mcp_config import sanitize_mcp_server_name
|
||||
from backend.apps.agents.manager.prompt.tool_catalog import _get_denied_tool_names, _is_fully_denied
|
||||
|
||||
|
||||
@@ -34,7 +34,7 @@ def _build_connected_tools_context(allowed_tools: list[str], get_all_tool_names:
|
||||
if _is_fully_denied(tool):
|
||||
continue
|
||||
|
||||
server_name = _sanitize_server_name(tool.name)
|
||||
server_name = sanitize_mcp_server_name(tool.name)
|
||||
denied = _get_denied_tool_names(tool)
|
||||
tool_descs = {
|
||||
k: v for k, v in tool.tool_permissions.get("_tool_descriptions", {}).items()
|
||||
@@ -263,7 +263,7 @@ def _build_mcp_registry_summary(allowed_tools: list[str], active_mcps: list[str]
|
||||
continue
|
||||
if _is_fully_denied(tool):
|
||||
continue
|
||||
server_name = _sanitize_server_name(tool.name)
|
||||
server_name = sanitize_mcp_server_name(tool.name)
|
||||
desc = (getattr(tool, "description", None) or "").strip()
|
||||
if not desc:
|
||||
# Fall back to a generic blurb keyed on the tool name so the
|
||||
@@ -373,7 +373,7 @@ def _resolve_forced_tools(forced_tools: list[str] | None) -> str:
|
||||
if not t.enabled or not t.tool_permissions:
|
||||
continue
|
||||
tool_descs = t.tool_permissions.get("_tool_descriptions", {})
|
||||
server_name = _sanitize_server_name(t.name)
|
||||
server_name = sanitize_mcp_server_name(t.name)
|
||||
for tn, td in tool_descs.items():
|
||||
desc_map[tn] = td
|
||||
tool_to_server[tn] = server_name
|
||||
|
||||
@@ -40,7 +40,7 @@ def _patched_get_credentials():
|
||||
)
|
||||
|
||||
|
||||
gauth.get_credentials = _patched_get_credentials
|
||||
gauth.get_credentials = _patched_get_credentials # vulture-ignore: get_credentials
|
||||
|
||||
|
||||
from google_workspace_mcp import __main__ as _gw_main # noqa: E402,F401
|
||||
|
||||
@@ -296,7 +296,7 @@ def sync(data: dict | None = None) -> None:
|
||||
"t": time.time(),
|
||||
"submission_id": uuid4().hex,
|
||||
}
|
||||
_log("s", payload)
|
||||
_log("s")
|
||||
if _test_sink is not None:
|
||||
try:
|
||||
_test_sink("s", body)
|
||||
|
||||
@@ -11,7 +11,7 @@ from backend.apps.tools_lib.oauth_config import OPENSWARM_OAUTH_BASE_URL
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _sanitize_server_name(name: str) -> str:
|
||||
def sanitize_mcp_server_name(name: str) -> str:
|
||||
"""Convert a tool name into a valid MCP server identifier (alphanumeric + hyphens)."""
|
||||
return re.sub(r"[^a-z0-9]+", "-", name.lower()).strip("-")
|
||||
|
||||
|
||||
+4
-3
@@ -550,7 +550,8 @@ async def mcp_meta(action: str, request: Request):
|
||||
valid options instead of activating (anti-hallucination).
|
||||
"""
|
||||
from backend.apps.agents.agent_manager import agent_manager
|
||||
from backend.apps.tools_lib.tools_lib import _load_all as load_all_tools, _sanitize_server_name
|
||||
from backend.apps.tools_lib.tools_lib import _load_all as load_all_tools
|
||||
from backend.apps.tools_lib.mcp_config import sanitize_mcp_server_name
|
||||
|
||||
body = await request.json()
|
||||
parent_session_id = body.get("parent_session_id", "")
|
||||
@@ -586,7 +587,7 @@ async def mcp_meta(action: str, request: Request):
|
||||
for t in load_all_tools():
|
||||
if not (t.mcp_config and t.enabled and t.auth_status in ("configured", "connected")):
|
||||
continue
|
||||
sanitized = _sanitize_server_name(t.name)
|
||||
sanitized = sanitize_mcp_server_name(t.name)
|
||||
# Pull tool sub-action names from tool_permissions._tool_descriptions
|
||||
# so MCPSearch can match against capability names (e.g. "send_email").
|
||||
action_names: list[str] = []
|
||||
@@ -710,7 +711,7 @@ async def mcp_meta(action: str, request: Request):
|
||||
tool_hint = ""
|
||||
try:
|
||||
for t in load_all_tools():
|
||||
if _sanitize_server_name(t.name) != server_name:
|
||||
if sanitize_mcp_server_name(t.name) != server_name:
|
||||
continue
|
||||
descs = (t.tool_permissions or {}).get("_tool_descriptions", {}) or {}
|
||||
if not descs:
|
||||
|
||||
@@ -581,9 +581,9 @@ def test_mcp_brand_covers_curated_servers():
|
||||
"google-workspace", "microsoft-365", "slack", "discord",
|
||||
"notion", "airtable", "hubspot", "reddit", "youtube",
|
||||
}
|
||||
from backend.apps.tools_lib.tools_lib import _sanitize_server_name
|
||||
from backend.apps.tools_lib.mcp_config import sanitize_mcp_server_name
|
||||
for slug in curated:
|
||||
assert _sanitize_server_name(slug) == slug, (
|
||||
assert sanitize_mcp_server_name(slug) == slug, (
|
||||
f"curated slug {slug!r} is not in sanitized form"
|
||||
)
|
||||
|
||||
@@ -599,30 +599,30 @@ def test_curated_server_aliases_in_main():
|
||||
|
||||
|
||||
def test_sanitize_server_name_idempotent():
|
||||
"""_sanitize_server_name must be idempotent (sanitize twice = sanitize once)."""
|
||||
from backend.apps.tools_lib.tools_lib import _sanitize_server_name
|
||||
"""sanitize_mcp_server_name must be idempotent (sanitize twice = sanitize once)."""
|
||||
from backend.apps.tools_lib.mcp_config import sanitize_mcp_server_name
|
||||
test_inputs = [
|
||||
"Google Workspace", "Microsoft 365", "Slack", "Discord",
|
||||
"Notion", "Airtable", "HubSpot", "Reddit", "YouTube",
|
||||
"GitHub", "GitLab", "Jira",
|
||||
]
|
||||
for raw in test_inputs:
|
||||
once = _sanitize_server_name(raw)
|
||||
twice = _sanitize_server_name(once)
|
||||
once = sanitize_mcp_server_name(raw)
|
||||
twice = sanitize_mcp_server_name(once)
|
||||
assert once == twice, f"{raw}: sanitize not idempotent ({once} != {twice})"
|
||||
|
||||
|
||||
def test_sanitize_server_name_lowercase():
|
||||
from backend.apps.tools_lib.tools_lib import _sanitize_server_name
|
||||
assert _sanitize_server_name("Gmail") == "gmail"
|
||||
assert _sanitize_server_name("UPPERCASE") == "uppercase"
|
||||
from backend.apps.tools_lib.mcp_config import sanitize_mcp_server_name
|
||||
assert sanitize_mcp_server_name("Gmail") == "gmail"
|
||||
assert sanitize_mcp_server_name("UPPERCASE") == "uppercase"
|
||||
|
||||
|
||||
def test_sanitize_server_name_strips_special_chars():
|
||||
from backend.apps.tools_lib.tools_lib import _sanitize_server_name
|
||||
assert _sanitize_server_name("Foo Bar!") == "foo-bar"
|
||||
assert _sanitize_server_name("@x/y") == "x-y"
|
||||
assert _sanitize_server_name("a__b") == "a-b"
|
||||
from backend.apps.tools_lib.mcp_config import sanitize_mcp_server_name
|
||||
assert sanitize_mcp_server_name("Foo Bar!") == "foo-bar"
|
||||
assert sanitize_mcp_server_name("@x/y") == "x-y"
|
||||
assert sanitize_mcp_server_name("a__b") == "a-b"
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
@@ -635,10 +635,10 @@ def test_mcp_activate_handler_unknown_server():
|
||||
# We test the response shape independently of the FastAPI plumbing.
|
||||
# The handler is a closure inside main.py:mcp_meta_handler, so we
|
||||
# instead exercise the contract: invalid name surfaces alternatives.
|
||||
from backend.apps.tools_lib.tools_lib import _sanitize_server_name
|
||||
from backend.apps.tools_lib.mcp_config import sanitize_mcp_server_name
|
||||
valid = {"gmail", "slack", "google-workspace"}
|
||||
requested = "Gmail" # raw, needs sanitize
|
||||
sanitized = _sanitize_server_name(requested)
|
||||
sanitized = sanitize_mcp_server_name(requested)
|
||||
if sanitized in valid:
|
||||
status = "would_activate"
|
||||
else:
|
||||
|
||||
@@ -267,7 +267,7 @@ const VARIANTS: Record<string, ToolLabel[]> = {
|
||||
],
|
||||
};
|
||||
|
||||
// Keys match backend _sanitize_server_name in tools_lib.
|
||||
// Keys match backend sanitize_mcp_server_name in tools_lib.
|
||||
const MCP_SERVER_BRAND: Record<string, string> = {
|
||||
'google-workspace': 'Google Workspace',
|
||||
'microsoft-365': 'Microsoft 365',
|
||||
|
||||
+23
-1
@@ -12,7 +12,9 @@ This folder contains the project's code quality tooling: a structural linter, de
|
||||
|
||||
**Unused Python code (Vulture)** — Flags unused functions, classes, variables, and imports in the backend. Integrated into the linter's watch loop — findings appear as warnings in the Problems panel alongside structural errors. Confidence thresholds are configurable via `vulture-min-confidence` and `vulture-error-threshold`.
|
||||
|
||||
These rules apply to `.py`, `.ts`, `.tsx`, `.js`, and `.jsx` files.
|
||||
**No leading-underscore names (`no-underscore-names`)** — Bans names that start with `_` in backend Python: functions, methods, arguments, classes, variable bindings, instance/class attribute writes (`self._x = ...`), and import aliases. The prefix is a blind spot — Pylance's `reportUnusedVariable`, ruff's `dummy-variable-rgx` (`F841`/`ARG0xx`), and vulture all treat a leading underscore as "intentionally private/unused" and stop reporting it, so dead `_name` code slips through every dead-code tool at once. Dunders (`__init__`, `__repr__`, … — required by Python) and the bare `_` throwaway (`for _ in …`) are exempt; name-mangled `__x` is **not**. The rule ships strict with no exceptions seeded — offenders are renamed by hand.
|
||||
|
||||
These rules apply to `.py`, `.ts`, `.tsx`, `.js`, and `.jsx` files (the `no-underscore-names` rule is Python-only).
|
||||
|
||||
### Orphaned endpoints
|
||||
|
||||
@@ -82,6 +84,7 @@ Or use the `knip:check` VS Code task (`Cmd+Shift+P` → "Run Task" → "knip:che
|
||||
"max-folder-items": true,
|
||||
"no-nested-imports": true,
|
||||
"vulture": true,
|
||||
"no-underscore-names": true,
|
||||
"eslint": true,
|
||||
"knip": true,
|
||||
"endpoints": true,
|
||||
@@ -113,6 +116,23 @@ Set any key in `"enabled"` to `false` to skip that check entirely. Missing keys
|
||||
|
||||
`config/vulture_whitelist.py` suppresses false positives — symbols used by frameworks, entry points, or external consumers that vulture can't detect statically. Add bare names to the file to mark them as intentionally used.
|
||||
|
||||
### Vulture inline ignores
|
||||
|
||||
For one-off false positives where the suppression reads best next to the code, drop a comment on the flagged line. Vulture points at the definition line (the `def`/`class` line, or the assignment line for an attribute), which is where the comment goes.
|
||||
|
||||
| Comment | Effect |
|
||||
|---------|--------|
|
||||
| `# vulture-ignore` | Silences any vulture finding on that line |
|
||||
| `# vulture-ignore: name1, name2` | Silences only when the finding names one of the listed symbols |
|
||||
|
||||
The scoped form is preferred — it can't accidentally swallow an unrelated future finding on the same line. Example:
|
||||
|
||||
```python
|
||||
gauth.get_credentials = _patched_get_credentials # vulture-ignore: get_credentials
|
||||
```
|
||||
|
||||
Prefer the whitelist for symbols exempt across many call sites; prefer an inline ignore when the exemption is local and benefits from sitting next to the code.
|
||||
|
||||
### ESLint
|
||||
|
||||
`frontend/eslint.config.mjs` — flat config format (ESLint v9). The key rule for unused code is `@typescript-eslint/no-unused-vars`. Prefix a variable with `_` to suppress the warning.
|
||||
@@ -155,6 +175,7 @@ linter/
|
||||
checks/ # check implementations
|
||||
__init__.py # shared filter/match utilities + .lintignore support
|
||||
structural.py # file length, folder size, nested imports
|
||||
no_underscore_names.py # bans leading-underscore Python names
|
||||
vulture.py # vulture dead-code runner
|
||||
eslint.py # eslint runner
|
||||
knip.py # knip unused-code runner
|
||||
@@ -181,6 +202,7 @@ deferred.
|
||||
| `max-file-lines` (300) | on | Our 300-line precedence. Active for new files; existing debt is grandfathered (see below). |
|
||||
| `max-folder-items` (7) | on | Grandfathered per subtree via `.lintignore-max-folder-items` markers in `backend/`, `frontend/`, `debugger/`, `electron/`, `scripts/`. |
|
||||
| `vulture` | on | Dead-code detection over `backend/`. Runs against `backend/.venv/bin/vulture`. |
|
||||
| `no-underscore-names` | on | Bans leading-underscore Python names over `backend/`. Pure-AST, no external tool. Strict with no seeded exceptions — existing offenders are renamed by hand. |
|
||||
| `no-nested-imports` | off | We deliberately use function-level / lazy imports to break import cycles (400+ sites). Flagging them all is wrong for this codebase. |
|
||||
| `eslint`, `knip` | off | Node tooling, deferred to a later pass. |
|
||||
| `endpoints` | off | Orphaned-endpoint triage deferred. |
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
"""Shared model metadata for the linter.
|
||||
|
||||
Pydantic ``BaseModel`` field names are collected here so independent checks can
|
||||
agree on which attribute names belong to a serialization schema, and are
|
||||
therefore "used" even when the only *read* happens across a language boundary
|
||||
(serialized to JSON over the wire and consumed by the frontend) which vulture's
|
||||
Python-only analysis structurally cannot see.
|
||||
|
||||
Framework detection lives in ONE place so checks/classes.py and
|
||||
checks/vulture.py never drift on what counts as a model. This module owns no
|
||||
section of its own and is not gated by config "enabled" flags, so any check that
|
||||
imports it works regardless of which sections are turned on.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from functools import lru_cache
|
||||
from pathlib import Path
|
||||
|
||||
FRAMEWORK_BASES = {"BaseModel"}
|
||||
|
||||
|
||||
def is_framework_model(cls: ast.ClassDef) -> bool:
|
||||
"""True when *cls* subclasses a known framework base (e.g. pydantic BaseModel)."""
|
||||
return any(
|
||||
(isinstance(b, ast.Name) and b.id in FRAMEWORK_BASES)
|
||||
or (isinstance(b, ast.Attribute) and b.attr in FRAMEWORK_BASES)
|
||||
for b in cls.bases
|
||||
)
|
||||
|
||||
|
||||
@lru_cache(maxsize=512)
|
||||
def _fields_in_file_cached(filepath: str, _mtime: float) -> frozenset[str]:
|
||||
"""Annotated field names on framework models in *filepath*.
|
||||
|
||||
Keyed on *(filepath, mtime)* so the long-lived watch process re-parses a file
|
||||
after it is edited instead of returning a stale set from an earlier version.
|
||||
"""
|
||||
try:
|
||||
tree = ast.parse(Path(filepath).read_text())
|
||||
except (OSError, SyntaxError):
|
||||
return frozenset()
|
||||
names: set[str] = set()
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.ClassDef) or not is_framework_model(node):
|
||||
continue
|
||||
# Direct body only: pydantic fields are annotated assignments
|
||||
# (``name: type`` / ``name: type = default``). Nested models are picked
|
||||
# up on their own ClassDef pass by ast.walk.
|
||||
for stmt in node.body:
|
||||
if isinstance(stmt, ast.AnnAssign) and isinstance(stmt.target, ast.Name):
|
||||
names.add(stmt.target.id)
|
||||
return frozenset(names)
|
||||
|
||||
|
||||
def _fields_in_file(filepath: Path) -> frozenset[str]:
|
||||
try:
|
||||
mtime = filepath.stat().st_mtime
|
||||
except OSError:
|
||||
return frozenset()
|
||||
return _fields_in_file_cached(str(filepath), mtime)
|
||||
|
||||
|
||||
def collect_pydantic_field_names(root: Path) -> set[str]:
|
||||
"""Every annotated field name on a ``BaseModel`` subclass under ``backend/``.
|
||||
|
||||
Consumed by checks/vulture.py to treat ``obj.<field> = ...`` writes as live
|
||||
even when the only read is across the Python/TS boundary, which vulture would
|
||||
otherwise report as an unused attribute.
|
||||
"""
|
||||
backend = root / "backend"
|
||||
if not backend.is_dir():
|
||||
return set()
|
||||
names: set[str] = set()
|
||||
for pyfile in backend.rglob("*.py"):
|
||||
parts = pyfile.parts
|
||||
if ".venv" in parts or "__pycache__" in parts:
|
||||
continue
|
||||
names |= _fields_in_file(pyfile)
|
||||
return names
|
||||
@@ -12,16 +12,7 @@ import ast
|
||||
from pathlib import Path
|
||||
|
||||
from . import is_excepted, is_excluded, is_lintignored
|
||||
|
||||
FRAMEWORK_BASES = {"BaseModel"}
|
||||
|
||||
|
||||
def _is_framework_model(cls: ast.ClassDef) -> bool:
|
||||
return any(
|
||||
(isinstance(b, ast.Name) and b.id in FRAMEWORK_BASES)
|
||||
or (isinstance(b, ast.Attribute) and b.attr in FRAMEWORK_BASES)
|
||||
for b in cls.bases
|
||||
)
|
||||
from ._models import is_framework_model
|
||||
|
||||
|
||||
def run_class_check(
|
||||
@@ -53,7 +44,7 @@ def run_class_check(
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.ClassDef):
|
||||
continue
|
||||
if _is_framework_model(node):
|
||||
if is_framework_model(node):
|
||||
continue
|
||||
# Tier 2 placeholder: non-framework classes are skipped until
|
||||
# cross-reference analysis is implemented.
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
"""Ban leading-underscore names that dead-code tooling silently skips.
|
||||
|
||||
Pylance's reportUnusedVariable, ruff's dummy-variable-rgx (F841/ARG0xx), and
|
||||
vulture all treat a leading underscore as "intentionally private/unused" and
|
||||
stop reporting it -- which makes ``_name`` a blind spot for dead-code detection.
|
||||
This check bans the prefix so nothing can hide behind it.
|
||||
|
||||
Exempt: dunders (``__init__`` and friends, i.e. ``__x__``) and the bare ``_``
|
||||
throwaway (``for _ in ...`` / ``a, _ = unpack()``). Everything else starting
|
||||
with ``_`` is flagged, including name-mangled ``__x``.
|
||||
|
||||
Covers function/method names, arguments (incl. lambda), class names, variable
|
||||
bindings (assignments, annotations, walrus, loop/with/except targets, tuple
|
||||
unpacking), instance/class attribute writes (``self._x = ...``), and import
|
||||
aliases. Scoped to ``backend/`` Python, like checks/classes.py.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
|
||||
from . import is_excepted, is_excluded, is_lintignored
|
||||
|
||||
RULE = "no-underscore-names"
|
||||
|
||||
Report = Callable[[str, int, int, str], None]
|
||||
|
||||
|
||||
def _is_dunder(name: str) -> bool:
|
||||
"""True for ``__x__`` style names that Python requires (``__init__`` etc.)."""
|
||||
return len(name) > 4 and name.startswith("__") and name.endswith("__")
|
||||
|
||||
|
||||
def _flagged(name: str) -> bool:
|
||||
if name == "_" or not name.startswith("_"):
|
||||
return False
|
||||
return not _is_dunder(name)
|
||||
|
||||
|
||||
def _report_targets(target: ast.AST, report: Report) -> None:
|
||||
"""Walk an assignment/loop target down to the names it binds."""
|
||||
if isinstance(target, ast.Name):
|
||||
report(target.id, target.lineno, target.col_offset, "variable")
|
||||
elif isinstance(target, ast.Attribute):
|
||||
# self._x = ... / cls._x = ...: the bound name is the attribute itself.
|
||||
report(target.attr, target.end_lineno or target.lineno, _attr_col(target), "attribute")
|
||||
elif isinstance(target, ast.Starred):
|
||||
_report_targets(target.value, report)
|
||||
elif isinstance(target, (ast.Tuple, ast.List)):
|
||||
for elt in target.elts:
|
||||
_report_targets(elt, report)
|
||||
# ast.Subscript (``d["_x"] = ...``) is not a name binding and is skipped.
|
||||
|
||||
|
||||
def _attr_col(node: ast.Attribute) -> int:
|
||||
"""Best-effort column for the attribute name (after the dot)."""
|
||||
# value.end_col_offset points just past ``value``; +1 skips the dot.
|
||||
end = getattr(node.value, "end_col_offset", None)
|
||||
return (end + 1) if end is not None else node.col_offset
|
||||
|
||||
|
||||
def _args(a: ast.arguments) -> list[ast.arg | None]:
|
||||
return [*a.posonlyargs, *a.args, *a.kwonlyargs, a.vararg, a.kwarg]
|
||||
|
||||
|
||||
def _check_tree(tree: ast.AST, rel: str) -> list[str]:
|
||||
errors: list[str] = []
|
||||
seen: set[tuple[int, int, str]] = set()
|
||||
|
||||
def report(name: str, lineno: int, col: int, kind: str) -> None:
|
||||
if not _flagged(name):
|
||||
return
|
||||
key = (lineno, col, name)
|
||||
if key in seen:
|
||||
return
|
||||
seen.add(key)
|
||||
errors.append(
|
||||
f"{rel}:{lineno}:{col + 1}: error: "
|
||||
f"[{RULE}] {kind} '{name}' has a leading underscore"
|
||||
)
|
||||
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
|
||||
report(node.name, node.lineno, node.col_offset, "function")
|
||||
for arg in _args(node.args):
|
||||
if arg is not None:
|
||||
report(arg.arg, arg.lineno, arg.col_offset, "argument")
|
||||
elif isinstance(node, ast.Lambda):
|
||||
for arg in _args(node.args):
|
||||
if arg is not None:
|
||||
report(arg.arg, arg.lineno, arg.col_offset, "argument")
|
||||
elif isinstance(node, ast.ClassDef):
|
||||
report(node.name, node.lineno, node.col_offset, "class")
|
||||
elif isinstance(node, (ast.Assign, ast.AnnAssign, ast.NamedExpr)):
|
||||
targets = node.targets if isinstance(node, ast.Assign) else [node.target]
|
||||
for t in targets:
|
||||
_report_targets(t, report)
|
||||
elif isinstance(node, (ast.For, ast.AsyncFor, ast.comprehension)):
|
||||
_report_targets(node.target, report)
|
||||
elif isinstance(node, (ast.With, ast.AsyncWith)):
|
||||
for item in node.items:
|
||||
if item.optional_vars is not None:
|
||||
_report_targets(item.optional_vars, report)
|
||||
elif isinstance(node, ast.ExceptHandler) and node.name:
|
||||
report(node.name, node.lineno, node.col_offset, "exception")
|
||||
elif isinstance(node, ast.Import):
|
||||
for alias in node.names:
|
||||
report(
|
||||
alias.asname or alias.name.split(".")[0],
|
||||
node.lineno, node.col_offset, "import",
|
||||
)
|
||||
elif isinstance(node, ast.ImportFrom):
|
||||
for alias in node.names:
|
||||
if alias.name != "*":
|
||||
report(alias.asname or alias.name, node.lineno, node.col_offset, "import")
|
||||
|
||||
return errors
|
||||
|
||||
|
||||
def run_underscore_check(
|
||||
root: Path,
|
||||
exceptions: dict[str, list[str]],
|
||||
excludes: list[str],
|
||||
ignores: dict[Path, set[str]] | None = None,
|
||||
) -> list[str]:
|
||||
"""Flag leading-underscore names in backend Python files."""
|
||||
errors: list[str] = []
|
||||
backend = root / "backend"
|
||||
if not backend.is_dir():
|
||||
return errors
|
||||
|
||||
for pyfile in sorted(backend.rglob("*.py")):
|
||||
if is_excluded(pyfile, root, excludes):
|
||||
continue
|
||||
rel = str(pyfile.relative_to(root))
|
||||
if is_excepted(rel, RULE, exceptions):
|
||||
continue
|
||||
if ignores and is_lintignored(pyfile, root, RULE, ignores):
|
||||
continue
|
||||
try:
|
||||
tree = ast.parse(pyfile.read_text(), filename=rel)
|
||||
except (OSError, SyntaxError):
|
||||
continue
|
||||
errors.extend(_check_tree(tree, rel))
|
||||
|
||||
return errors
|
||||
@@ -20,6 +20,7 @@ from functools import lru_cache
|
||||
from pathlib import Path
|
||||
|
||||
from . import CheckError, is_excepted, is_lintignored
|
||||
from ._models import collect_pydantic_field_names
|
||||
|
||||
CONFIG_DIR = Path(__file__).resolve().parent.parent / "config"
|
||||
|
||||
@@ -64,6 +65,49 @@ def _is_inside_class(filepath: str, lineno: int) -> bool:
|
||||
return any(start < lineno <= end for start, end in _class_line_ranges(filepath))
|
||||
|
||||
|
||||
# Inline suppression: `# vulture-ignore` (bare) silences any finding on the
|
||||
# line; `# vulture-ignore: name1, name2` only silences when the finding names
|
||||
# one of the listed symbols (the noqa-with-codes ergonomic).
|
||||
_INLINE_IGNORE_RE = re.compile(r"#\s*vulture-ignore\b(?::\s*(?P<names>.*))?")
|
||||
|
||||
|
||||
@lru_cache(maxsize=256)
|
||||
def _file_lines_cached(filepath: str, _mtime: float) -> tuple[str, ...]:
|
||||
"""Return the source lines of *filepath*, keyed on *(filepath, mtime)* so the
|
||||
long-lived watch process re-reads a file after it is edited."""
|
||||
try:
|
||||
return tuple(Path(filepath).read_text().splitlines())
|
||||
except OSError:
|
||||
return ()
|
||||
|
||||
|
||||
def _inline_ignored(filepath: Path, lineno: int, message: str) -> bool:
|
||||
"""True when the flagged line carries a ``# vulture-ignore`` comment.
|
||||
|
||||
Vulture points at the definition line (the ``def``/``class`` line, or the
|
||||
assignment line for an attribute), which is exactly where the comment lives.
|
||||
The scoped form only suppresses when the vulture message names a listed
|
||||
symbol, so the comment cannot accidentally swallow an unrelated future
|
||||
finding on the same line.
|
||||
"""
|
||||
try:
|
||||
mtime = filepath.stat().st_mtime
|
||||
except OSError:
|
||||
return False
|
||||
lines = _file_lines_cached(str(filepath), mtime)
|
||||
if not (1 <= lineno <= len(lines)):
|
||||
return False
|
||||
m = _INLINE_IGNORE_RE.search(lines[lineno - 1])
|
||||
if not m:
|
||||
return False
|
||||
names = m.group("names")
|
||||
if not names:
|
||||
return True
|
||||
wanted = {n.strip() for n in names.replace(",", " ").split() if n.strip()}
|
||||
flagged = re.search(r"'([^']+)'", message)
|
||||
return bool(flagged and flagged.group(1) in wanted)
|
||||
|
||||
|
||||
def run_vulture(
|
||||
root: Path, min_confidence: int, error_threshold: int,
|
||||
exceptions: dict[str, list[str]],
|
||||
@@ -107,6 +151,15 @@ def run_vulture(
|
||||
detail = result.stderr.strip()[:300] or "no output"
|
||||
raise CheckError(f"vulture exited with code {result.returncode}: {detail}")
|
||||
|
||||
# Pydantic field names assigned outside the class body (e.g.
|
||||
# ``session.memory_recalled = True``) are reported by vulture as unused
|
||||
# attributes because the only *read* is across the Python/TS boundary
|
||||
# (serialized via model_dump and consumed by the frontend). Treat any
|
||||
# attribute whose name is a known model field as live. Computed once per run
|
||||
# and independent of the "classes" section toggle, so suppression holds even
|
||||
# when that section is disabled.
|
||||
model_fields = collect_pydantic_field_names(root)
|
||||
|
||||
errors: list[str] = []
|
||||
for line in result.stdout.strip().splitlines():
|
||||
m = re.match(r"^(.+):(\d+): (.+)$", line)
|
||||
@@ -118,10 +171,15 @@ def run_vulture(
|
||||
# never gets a vote.
|
||||
if re.search(r"unused (import|variable)", message):
|
||||
continue
|
||||
attr_m = re.match(r"unused attribute '([^']+)'", message)
|
||||
if attr_m and attr_m.group(1) in model_fields:
|
||||
continue
|
||||
if is_excepted(filepath, "vulture", exceptions):
|
||||
continue
|
||||
if ignores and is_lintignored(root / filepath, root, "vulture", ignores):
|
||||
continue
|
||||
if _inline_ignored(root / filepath, int(lineno), message):
|
||||
continue
|
||||
if _is_inside_class(str(root / filepath), int(lineno)):
|
||||
continue
|
||||
conf = re.search(r"\((\d+)% confidence\)", message)
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
"no-nested-imports": false,
|
||||
"vulture": true,
|
||||
"ruff": true,
|
||||
"no-underscore-names": false,
|
||||
"eslint": false,
|
||||
"knip": false,
|
||||
"endpoints": false,
|
||||
@@ -16,6 +17,7 @@
|
||||
"eslint-knip": "Node tooling deferred to a later pass.",
|
||||
"classes": "Placeholder check, not wired up. endpoints: orphaned-endpoint triage deferred.",
|
||||
"ruff-vulture-split": "ruff owns per-file/per-scope checks (F401 unused imports, F811 redefinitions, F841 unused locals, ARG001/ARG002 unused args), which it does AST-accurately and which vulture either gets wrong (global name-set hides per-file dead imports) or rates as noisy 60% findings. vulture is narrowed in checks/vulture.py to whole-program reachability only (dead functions/methods/classes/attributes), the one thing ruff structurally cannot do since it never builds a cross-module symbol graph. F401 honors __all__ and the redundant-alias form (import x as x) for intentional re-exports; add one of those if F401 flags a deliberate re-export.",
|
||||
"no-underscore-names": "Bans leading-underscore names in backend/ Python (functions, methods, args, classes, variables, instance/class attribute writes, import aliases). The prefix is a blind spot: Pylance reportUnusedVariable, ruff dummy-variable-rgx (F841/ARG0xx), and vulture all treat it as intentionally-private/unused and stop reporting, so dead _name code hides. Exempt: dunders (__x__, required by Python) and the bare _ throwaway. Name-mangled __x IS flagged. Intentionally strict with no exceptions seeded -- offenders are renamed by hand.",
|
||||
"max-file-lines-exceptions": "Grandfather list of pre-existing >300-line files (existing debt, not new). Paths updated after the folder-tree restructure moved several of them.",
|
||||
"max-folder-items-exceptions": "Exact-path allow for folders intentionally over the cap. The rule trips at >7 (7 items is fine, the 8th tips it), so only genuinely 8+ folders are listed. backend/ and backend/apps are FastAPI feature-package registries (each child is an app mounted in main.py); agents/ aggregates agent subsystems; core/, tools_lib/, tests/ are conventionally flat. Frontend: app/pages is the page registry, AgentChat/ChatInput/Settings-sections/Onboarding are organizational parents, and shared/state (Redux slices) plus hooks/steps/mcp-cards/Views are flat peer collections. scripts/, electron/, linter/checks/ are flat tool dirs. These replaced blanket .lintignore-max-folder-items sentinels (backend, frontend, scripts, electron, linter/checks) so the rule still catches NEW unplanned bloat everywhere else. Kept as whole-subtree sentinels on purpose: debugger/ (self-contained injected sub-tool with its own Vite GUI), webapp_template (Vite scaffold payload), and vendored mcp-bundles.",
|
||||
"import-cycles": "Flags RUNTIME circular imports only (SCC>1). Skips type-only imports (import type / export type) and dynamic import() since neither runs at module init, which is why the idiomatic Redux store<->hooks type cycle is not flagged. Frontend alias resolution comes from import-cycle-aliases. Zero cycles today; the check keeps it that way."
|
||||
@@ -63,6 +65,7 @@
|
||||
"import-cycles": [],
|
||||
"vulture": [],
|
||||
"ruff": [],
|
||||
"no-underscore-names": [],
|
||||
"endpoints": [],
|
||||
"classes": []
|
||||
}
|
||||
|
||||
+5
-1
@@ -20,6 +20,7 @@ from checks.knip import run_knip
|
||||
from checks.endpoints import run_endpoint_check
|
||||
from checks.classes import run_class_check
|
||||
from checks.cycles import run_cycle_check
|
||||
from checks.no_underscore_names import run_underscore_check
|
||||
from watchfiles import watch, DefaultFilter
|
||||
|
||||
SCRIPT_DIR = Path(__file__).resolve().parent
|
||||
@@ -27,7 +28,7 @@ CONFIG_FILE = SCRIPT_DIR / "config" / "config.json"
|
||||
|
||||
# Print order for the sections; also the order they run in.
|
||||
SECTION_ORDER = [
|
||||
"structural", "vulture", "ruff", "eslint",
|
||||
"structural", "vulture", "ruff", "no-underscore-names", "eslint",
|
||||
"knip", "endpoints", "classes", "import-cycles",
|
||||
]
|
||||
|
||||
@@ -152,6 +153,9 @@ def run_checks(root: Path) -> LintResult:
|
||||
run_section("ruff", lambda: run_ruff(
|
||||
root, rules.get("ruff-select", "F401,F811,F841,ARG001,ARG002"), exceptions, ignores,
|
||||
) if enabled.get("ruff", True) else [])
|
||||
run_section("no-underscore-names", lambda: run_underscore_check(
|
||||
root, exceptions, excludes, ignores,
|
||||
) if enabled.get("no-underscore-names", True) else [])
|
||||
run_section("eslint", lambda: run_eslint(root, ignores) if enabled.get("eslint", True) else [])
|
||||
run_section("knip", lambda: run_knip(root, ignores) if enabled.get("knip", True) else [])
|
||||
run_section("endpoints", lambda: run_endpoint_check(
|
||||
|
||||
Reference in New Issue
Block a user