From 31941e24b5d45cf757a763a61c580339d1dfc355 Mon Sep 17 00:00:00 2001 From: Danila Katalshov <56929384+ADanMan@users.noreply.github.com> Date: Wed, 5 Aug 2026 10:29:48 +0300 Subject: [PATCH] ci: add a ruff (pyflakes) lint gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No static-analysis gate existed for the Python side — CI ran pytest only, nothing checked for dead imports, unused variables, undefined names, or syntax errors before review. Add ruff, scoped deliberately narrow to start (owner call, per issue #34): select = ["E9", "F"] — syntax errors + pyflakes. No style rules, no formatter, nothing to bikeshed; just the checks with essentially zero false positives that catch real latent bugs. target-version is pinned to py312 (matching what CI actually runs) rather than derived from the 3.10 requires-python floor, which would otherwise flag `ExceptionGroup` (3.11+) as an undefined name in test_mcp_connectors.py. Fixed what it found (33 violations, all pre-existing): - 28 dead imports across coworker/ and tests/. Two (SHELL_TOOL, WRITE_TOOLS in permissions.py) are an intentional back-compat re-export per the existing comment on that import block — left in place with `# noqa: F401` rather than removed. - 2 redundant local `from ..engine import ApprovalOutcome` in manager.py that shadowed the already-imported module-level name, which is what made that module-level import itself look unused. - 1 unused local variable in a test. This is the Python half of #34; the GUI's eslint/tsc half looks already covered by #419 (typecheck in gui-unit). Full suite still green: 1112 passed, 1 skipped. --- .github/workflows/ci.yml | 14 ++++++++++++++ coworker/automation/tools.py | 2 +- coworker/catalog.py | 2 +- coworker/connectors/base.py | 2 +- coworker/connectors/descriptors.py | 2 +- coworker/connectors/relay_client.py | 2 +- coworker/permissions.py | 4 ++-- coworker/personas/loading.py | 2 +- coworker/server/manager.py | 3 --- coworker/skills/store.py | 2 +- coworker/subscriptions.py | 2 +- pyproject.toml | 14 +++++++++++++- tests/test_code_tools.py | 2 -- tests/test_config.py | 2 -- tests/test_connectors.py | 2 -- tests/test_dm_routing.py | 1 - tests/test_email_tools.py | 2 -- tests/test_memory.py | 1 - tests/test_message_source.py | 2 +- tests/test_multiroot.py | 2 +- tests/test_plan_mode.py | 2 +- tests/test_risk_overrides.py | 2 +- tests/test_skills_api.py | 1 - tests/test_skills_sessions.py | 1 - tests/test_skills_store.py | 1 - tests/test_slack_relay.py | 2 +- tests/test_standing_approvals.py | 1 - tests/test_subscriptions.py | 2 -- tests/test_tools_permissions.py | 2 +- 29 files changed, 43 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90cb22d6..f9ea24a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,20 @@ name: CI on: [push, pull_request] jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Install + run: | + python -m pip install --upgrade pip + pip install -e ".[dev]" + - name: Ruff (pyflakes — undefined names, dead imports/vars, syntax errors) + run: ruff check coworker tests + pytest: runs-on: ubuntu-latest steps: diff --git a/coworker/automation/tools.py b/coworker/automation/tools.py index caf192ab..10adecfc 100644 --- a/coworker/automation/tools.py +++ b/coworker/automation/tools.py @@ -9,7 +9,7 @@ from __future__ import annotations -from typing import Any, Callable, Optional +from typing import Any, Callable import aisuite as ai diff --git a/coworker/catalog.py b/coworker/catalog.py index e2ce78a7..e77554ee 100644 --- a/coworker/catalog.py +++ b/coworker/catalog.py @@ -14,7 +14,7 @@ from __future__ import annotations -from dataclasses import dataclass, field +from dataclasses import dataclass from typing import Callable import aisuite as ai diff --git a/coworker/connectors/base.py b/coworker/connectors/base.py index e269c09e..2d2b972b 100644 --- a/coworker/connectors/base.py +++ b/coworker/connectors/base.py @@ -9,7 +9,7 @@ from __future__ import annotations from abc import ABC, abstractmethod -from dataclasses import asdict, dataclass, field +from dataclasses import asdict, dataclass from enum import Enum from typing import Any, Awaitable, Callable, Optional diff --git a/coworker/connectors/descriptors.py b/coworker/connectors/descriptors.py index 32e103c6..03a98a80 100644 --- a/coworker/connectors/descriptors.py +++ b/coworker/connectors/descriptors.py @@ -9,7 +9,7 @@ from __future__ import annotations -from dataclasses import dataclass, field +from dataclasses import dataclass from typing import Callable, Optional diff --git a/coworker/connectors/relay_client.py b/coworker/connectors/relay_client.py index 57fc38cc..0af1933a 100644 --- a/coworker/connectors/relay_client.py +++ b/coworker/connectors/relay_client.py @@ -30,7 +30,7 @@ from typing import Any, Awaitable, Callable, Optional, Protocol from .adapters import _SLACK_MENTION_RE, slack_event_to_event -from .base import BasePlatformAdapter, InteractionEvent, SendResult, SessionSource +from .base import BasePlatformAdapter, InteractionEvent, SendResult from .senders import _send_slack, _send_slack_interactive from .slack_addr import qualify diff --git a/coworker/permissions.py b/coworker/permissions.py index 82477f7b..42559114 100644 --- a/coworker/permissions.py +++ b/coworker/permissions.py @@ -25,8 +25,8 @@ def _has_shell_operators(command: str) -> bool: return any(op in command for op in _SHELL_OPERATORS) from .risk import ( # re-exported for back-compat (manager.py imports WRITE_TOOLS) - SHELL_TOOL, - WRITE_TOOLS, + SHELL_TOOL, # noqa: F401 + WRITE_TOOLS, # noqa: F401 RiskClass, RiskOverrides, classify, diff --git a/coworker/personas/loading.py b/coworker/personas/loading.py index 8c55fc9d..a5becb0d 100644 --- a/coworker/personas/loading.py +++ b/coworker/personas/loading.py @@ -11,7 +11,7 @@ import subprocess from pathlib import Path -from typing import Callable, Optional +from typing import Callable from .manifest import PersonaManifest diff --git a/coworker/server/manager.py b/coworker/server/manager.py index ad76e996..68459645 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -2656,8 +2656,6 @@ def approval_outcome(self, resolution: str, request, session_id: str): """Map an approval resolution (from any surface) to an ApprovalOutcome, handling the task-persistent "always_task" vocabulary alongside the session-scoped ones. """ - from ..engine import ApprovalOutcome - if resolution == "always_task": self.mint_task_rule( session_id, @@ -2677,7 +2675,6 @@ def approval_outcome(self, resolution: str, request, session_id: str): return ApprovalOutcome.DENY def _scheduled_approver(self, task, session_id: str): - from ..engine import ApprovalOutcome from ..permissions import WRITE_TOOLS name_allowed = task.name_allowed_tools() diff --git a/coworker/skills/store.py b/coworker/skills/store.py index 65ccc66b..7add9184 100644 --- a/coworker/skills/store.py +++ b/coworker/skills/store.py @@ -28,7 +28,7 @@ import aisuite as ai from ..secrets import state_dir -from .base import Skill, _parse_skill +from .base import _parse_skill _NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$") _MAX_NAME = 64 diff --git a/coworker/subscriptions.py b/coworker/subscriptions.py index f8b5d33a..54f9d2c3 100644 --- a/coworker/subscriptions.py +++ b/coworker/subscriptions.py @@ -21,7 +21,7 @@ import re import threading from collections import deque -from dataclasses import asdict, dataclass, field +from dataclasses import asdict, dataclass from pathlib import Path from typing import Optional diff --git a/pyproject.toml b/pyproject.toml index 687dad70..a54a61fa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,7 @@ dependencies = [ ] [project.optional-dependencies] -dev = ["pytest>=8", "pytest-asyncio", "httpx"] +dev = ["pytest>=8", "pytest-asyncio", "httpx", "ruff>=0.16"] # Inbound messaging listeners (outbound send_message needs only httpx, already a core dep). # aiohttp is slack-bolt's Socket Mode transport at runtime (and the FakeSlack test harness # drives the real handler) — declare it so CI installs it, not just transitively. @@ -62,3 +62,15 @@ coworker = ["personas/builtin/*.md"] [tool.pytest.ini_options] testpaths = ["tests"] asyncio_mode = "auto" + +[tool.ruff] +# CI actually runs 3.12 (see ci.yml); pinning here (rather than deriving from the +# 3.10-floor `requires-python`) avoids false undefined-name positives on names the +# stdlib only added later (e.g. `ExceptionGroup`, 3.11+). +target-version = "py312" + +[tool.ruff.lint] +# Deliberately narrow (owner call — see issue #34): pyflakes catches real latent bugs +# (undefined names, dead imports/variables, shadowed-unused redefinitions) with ~zero +# false positives and no style opinions to bikeshed. Widen once this is settled in. +select = ["E9", "F"] diff --git a/tests/test_code_tools.py b/tests/test_code_tools.py index 0edc83b2..f4cbfced 100644 --- a/tests/test_code_tools.py +++ b/tests/test_code_tools.py @@ -9,8 +9,6 @@ import subprocess from types import SimpleNamespace -import pytest - from coworker.tools.files import file_tools from coworker.tools.git import git_tools from coworker.tools.search import _py_grep, search_tools diff --git a/tests/test_config.py b/tests/test_config.py index 22915ba6..a1b05d87 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -2,8 +2,6 @@ from __future__ import annotations -from pathlib import Path - from coworker.config import load_config diff --git a/tests/test_connectors.py b/tests/test_connectors.py index cb155fa6..1fcb8adf 100644 --- a/tests/test_connectors.py +++ b/tests/test_connectors.py @@ -4,8 +4,6 @@ from __future__ import annotations -import asyncio - import pytest from coworker.connectors import ( diff --git a/tests/test_dm_routing.py b/tests/test_dm_routing.py index 0295ae4a..dc8bb3ab 100644 --- a/tests/test_dm_routing.py +++ b/tests/test_dm_routing.py @@ -3,7 +3,6 @@ import asyncio -import pytest from fastapi.testclient import TestClient from coworker.connectors.base import MessageEvent, SessionSource diff --git a/tests/test_email_tools.py b/tests/test_email_tools.py index 7da41534..1061eb18 100644 --- a/tests/test_email_tools.py +++ b/tests/test_email_tools.py @@ -4,8 +4,6 @@ from email.message import EmailMessage -import pytest - from coworker.connectors.email_tools import ( build_search_criteria, decode_mime_header, diff --git a/tests/test_memory.py b/tests/test_memory.py index 9e27bcec..c99ecdad 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -2,7 +2,6 @@ from __future__ import annotations -import aisuite as ai from coworker.conversations import ConversationStore from coworker.memory import Scope, SQLiteMemoryStore, format_memories, memory_tools from coworker.sessions import SessionRecord diff --git a/tests/test_message_source.py b/tests/test_message_source.py index 9b669ca7..b57b0302 100644 --- a/tests/test_message_source.py +++ b/tests/test_message_source.py @@ -9,7 +9,7 @@ from fastapi.testclient import TestClient -from coworker.connectors.base import MessageEvent, MessageSource, SessionSource +from coworker.connectors.base import MessageEvent, SessionSource from coworker.engine import TurnEngine from coworker.permissions import PermissionEngine from coworker.providers import AssistantTurn, ModelCapabilities, ProviderClient diff --git a/tests/test_multiroot.py b/tests/test_multiroot.py index d24aa0ef..fba94675 100644 --- a/tests/test_multiroot.py +++ b/tests/test_multiroot.py @@ -14,7 +14,7 @@ import aisuite as ai from coworker.engine import TurnEngine from coworker.events import EventType -from coworker.permissions import Decision, Mode, PermissionEngine +from coworker.permissions import PermissionEngine from coworker.providers import AssistantTurn, ToolCall from coworker.roots import RootDir, normalize_roots, render_context from coworker.tools import ToolRegistry diff --git a/tests/test_plan_mode.py b/tests/test_plan_mode.py index 68496bd0..f636867c 100644 --- a/tests/test_plan_mode.py +++ b/tests/test_plan_mode.py @@ -195,7 +195,7 @@ def test_discuss_mode_blocks_writes_without_plan_pressure(tmp_path): ], ) permissions.mode = Mode.DISCUSS - events = _collect(engine, "tweak x.py") + _collect(engine, "tweak x.py") assert not (tmp_path / "x.py").exists() assert any( m.get("role") == "tool" and "discuss mode is read-only" in m["content"] diff --git a/tests/test_risk_overrides.py b/tests/test_risk_overrides.py index a06066b2..021f29a1 100644 --- a/tests/test_risk_overrides.py +++ b/tests/test_risk_overrides.py @@ -5,7 +5,7 @@ from types import SimpleNamespace from coworker.overrides import RiskOverrideStore -from coworker.permissions import Mode, PermissionEngine +from coworker.permissions import PermissionEngine from coworker.risk import RiskClass, classify MCP_META = SimpleNamespace(requires_approval=True, category="mcp") diff --git a/tests/test_skills_api.py b/tests/test_skills_api.py index 88360c58..93878f60 100644 --- a/tests/test_skills_api.py +++ b/tests/test_skills_api.py @@ -11,7 +11,6 @@ import io import zipfile -import pytest from fastapi.testclient import TestClient from coworker.providers import AssistantTurn, ModelCapabilities, ProviderClient diff --git a/tests/test_skills_sessions.py b/tests/test_skills_sessions.py index d53bf4f1..9e916461 100644 --- a/tests/test_skills_sessions.py +++ b/tests/test_skills_sessions.py @@ -15,7 +15,6 @@ from coworker.skills import ( SessionSkillStore, SkillLoader, - SkillStore, effective_skills, skill_catalog_text, skill_tools, diff --git a/tests/test_skills_store.py b/tests/test_skills_store.py index ff36028e..4eb41c01 100644 --- a/tests/test_skills_store.py +++ b/tests/test_skills_store.py @@ -9,7 +9,6 @@ from __future__ import annotations import io -import json import os import zipfile from pathlib import Path diff --git a/tests/test_slack_relay.py b/tests/test_slack_relay.py index b05ce807..d902d9b7 100644 --- a/tests/test_slack_relay.py +++ b/tests/test_slack_relay.py @@ -11,7 +11,7 @@ from coworker.connectors import relay_client from coworker.connectors.adapters import make_adapter from coworker.connectors.base import InteractionEvent, MessageEvent -from coworker.connectors.config import ConnectorSettings, load_settings +from coworker.connectors.config import load_settings from coworker.connectors.relay_client import SlackRelayAdapter from coworker.connectors.slack_addr import qualify, split from coworker.connectors.tools import make_send_message_tool diff --git a/tests/test_standing_approvals.py b/tests/test_standing_approvals.py index 50a19727..f724d8f3 100644 --- a/tests/test_standing_approvals.py +++ b/tests/test_standing_approvals.py @@ -10,7 +10,6 @@ import asyncio import aisuite as ai -import pytest from coworker.automation import Schedule, ScheduledTask, Scheduler, TaskRun, TaskStore from coworker.automation.models import grant_entries, rule_entry, rule_parts diff --git a/tests/test_subscriptions.py b/tests/test_subscriptions.py index d5b4964e..b31d3150 100644 --- a/tests/test_subscriptions.py +++ b/tests/test_subscriptions.py @@ -2,8 +2,6 @@ import asyncio -import pytest - from coworker.connectors.base import MessageEvent, SessionSource from coworker.subscriptions import ( ChannelBuffer, diff --git a/tests/test_tools_permissions.py b/tests/test_tools_permissions.py index 8d7e76b9..f1481ad5 100644 --- a/tests/test_tools_permissions.py +++ b/tests/test_tools_permissions.py @@ -7,7 +7,7 @@ import pytest import aisuite as ai -from coworker.permissions import Decision, Mode, PermissionEngine +from coworker.permissions import Mode, PermissionEngine from coworker.tools import ToolRegistry