Skip to content

[RUFF] Enable SLF rule (private member access)#1046

Open
agentydragon wants to merge 1 commit intodevelfrom
claude/ruff-enable-slf
Open

[RUFF] Enable SLF rule (private member access)#1046
agentydragon wants to merge 1 commit intodevelfrom
claude/ruff-enable-slf

Conversation

@agentydragon
Copy link
Copy Markdown
Owner

Summary

  • Enable SLF (flake8-self) in ruff.toml
  • Fix 59 violations: make members public (remove underscore prefix) or add # noqa: SLF001 for unavoidable external library internals
  • Per-file-ignores: SLF001 exempted in **/test_*.py and **/conftest.py

Key renames: _req_storagereq_storage, _runningrunning, _mount_namesmount_names, _hubhub, _compositorcompositor, _openopen_db, _shutdown_taskshutdown_task, etc.

https://claude.ai/code/session_013WcexLQ8JPJG1j79uvMfCF

Copilot AI review requested due to automatic review settings March 27, 2026 18:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables Ruff’s SLF (flake8-self) rule to prevent cross-object access to private members, and updates the codebase accordingly by promoting selected internals to public APIs or adding narrow # noqa: SLF001 suppressions where unavoidable.

Changes:

  • Enable SLF in ruff.toml and add per-file ignores for tests and conftest.py.
  • Replace private-member access patterns by renaming fields/methods to public equivalents across agent server, MCP infra, auth proxy, and related utilities.
  • Add targeted # noqa: SLF001 annotations for unavoidable third-party internals usage.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
ruff.toml Enables SLF and configures per-file ignores for test code.
x/claude_linter_v2/hooks/handler.py Avoids private logging internals when mapping log levels.
x/agent_server/server/runtime.py Renames UI event application method to public API.
x/agent_server/server/mcp_routing.py Switches to public container compositor accessor.
x/agent_server/server/app.py Uses public UI handler on shutdown to flush UI events.
x/agent_server/runtime/container.py Promotes container internals (cm, compositor, session getter) to public names.
x/agent_server/persist/sqlite.py Promotes DB open helpers to public context managers.
x/agent_server/mcp_bridge/registry.py Uses public compositor field for mounting.
x/agent_server/mcp/chat/server.py Uses public persistence DB open helpers.
x/agent_server/mcp/approval_policy/engine.py Promotes approval hub and policy evaluation to public engine APIs.
wt/server/wt_server.py Renames shutdown task field; adjusts handler-import pattern.
tana/query/search/evaluator.py Promotes evaluator helper methods to public names and updates dispatch.
mcp_infra/tool_schemas.py Documents internal FastMCP access and attempts SLF suppression.
mcp_infra/compositor/server.py Promotes compositor internals (resource notifications, mount names) to public APIs.
mcp_infra/compositor/resources_server.py Uses new compositor public mount-name API.
mcp_infra/compositor/notifications_buffer.py Promotes buffer handlers and uses compositor public mount-name API.
devinfra/claude/auth_proxy/setup.py Removes private member checks and adjusts lazy import.
devinfra/claude/auth_proxy/proxy.py Promotes proxy running state to public field.
airlock/proxy_server.py Switches to public storage accessor and updates wrapped-tool handler signature.
airlock/app.py Switches to public storage accessor for API endpoints.

Comment thread mcp_infra/tool_schemas.py
Comment on lines +6 to 7
Uses FastMCP internal `_local_provider._components` because the public # noqa: SLF001
`list_tools()` API is async and callers need sync access.
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# noqa: SLF001 placed inside the docstring won’t suppress the actual SLF001 violation. The private access is on _iter_tools() (server._local_provider._components); move the # noqa: SLF001 to that code line (or refactor to a public API) and remove it from the docstring so ruff passes.

Copilot uses AI. Check for mistakes.
"""
try:
from util.bazel.runfiles import get_required_path # noqa: PLC0415
from util.bazel.runfiles import get_required_path
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local import in _resolve_rlocation() is intentional (to avoid crashing outside Bazel), but removing # noqa: PLC0415 will make ruff flag this as an import outside top-level (PLC0415 is enabled repo-wide). Please add the # noqa: PLC0415 back on this import or add a per-file ignore for this file.

Suggested change
from util.bazel.runfiles import get_required_path
from util.bazel.runfiles import get_required_path # noqa: PLC0415

Copilot uses AI. Check for mistakes.
Comment thread airlock/proxy_server.py
Comment thread wt/server/wt_server.py
Comment on lines 33 to 39
# Force import of handlers to register RPC methods
from wt.server.handlers import (
path_handler, # noqa: F401
pr_handler, # noqa: F401
status_handler, # noqa: F401
worktree_handler, # noqa: F401
path_handler,
pr_handler,
status_handler,
worktree_handler,
)
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These handler-module imports are intentionally side-effectful, but without # noqa: F401 (or another usage) ruff will flag them as unused imports (F401 is enabled). Please restore the F401 ignores or otherwise reference the imported modules to keep lint passing while preserving the registration side effects.

Copilot uses AI. Check for mistakes.
Comment on lines 88 to 92
self._upstream_url: str | None = None
self._creds_lock = threading.Lock()
self.server_socket: socket.socket | None = None
self._running = False
self.running = False
self._thread: threading.Thread | None = None
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming _running to running breaks existing call sites that still check proxy._running (e.g. devinfra/claude/test_integration.py). To avoid runtime/test failures, either update those call sites in this PR or provide a backwards-compatible alias (e.g., a property _running that proxies to running) during the transition.

Copilot uses AI. Check for mistakes.
_ui_bus: ServerBus | None = field(default=None, init=False)
# Compositor instance (AgentContainerCompositor with all agent-specific servers)
_compositor: AgentContainerCompositor | None = field(default=None, init=False)
compositor: AgentContainerCompositor | None = field(default=None, init=False)
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming the dataclass field from _compositor to compositor will break existing code that assigns/reads container._compositor (there are still references in x/agent_server/mcp_bridge/test_integration.py). Either update those references as part of this PR or add a temporary compatibility shim (getter/setter for _compositor that forwards to compositor) so tests and any downstream code don’t fail at runtime.

Suggested change
compositor: AgentContainerCompositor | None = field(default=None, init=False)
compositor: AgentContainerCompositor | None = field(default=None, init=False)
@property
def _compositor(self) -> AgentContainerCompositor | None:
"""Temporary compatibility shim for code that still accesses `container._compositor`."""
return self.compositor
@_compositor.setter
def _compositor(self, value: AgentContainerCompositor | None) -> None:
self.compositor = value

Copilot uses AI. Check for mistakes.
Comment thread x/claude_linter_v2/hooks/handler.py
@agentydragon agentydragon force-pushed the claude/ruff-enable-slf branch 17 times, most recently from f7c6e60 to a70aca1 Compare March 27, 2026 23:50
Adds `SLF` (flake8-self) to ruff.toml. Fixes 59 violations by either
making members public (removing underscore prefix) or adding # noqa: SLF001
for unavoidable access to external library internals.

Per-file-ignores: SLF001 exempted in test files and conftest.

https://claude.ai/code/session_013WcexLQ8JPJG1j79uvMfCF
@agentydragon agentydragon force-pushed the claude/ruff-enable-slf branch from a70aca1 to aa1843a Compare March 28, 2026 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants