feat: add Ruff code quality guardrails (replaces black + isort)

[Sergio Sisternes (sergio-sisternes-epam)]

Summary

Replace black + isort with Ruff as the sole linter and formatter for the APM codebase. Add a deterministic CI lint gate, configure 10 rule sets with empirically-derived complexity thresholds, and establish a "strangler fig" roadmap for progressively tightening standards over time.

523 files changed | 6564 tests pass | CI lint job runs in ~3s

Motivation

APM had black and isort as dev dependencies but neither was enforced in CI -- they were purely advisory. This PR introduces a proper quality gate that:

1. Blocks bad code at PR time -- deterministic, configuration-driven, zero AI in the guardrail
2. Catches real bugs -- found a Python 3.10 compatibility issue during rollout (backslash in f-string only valid in 3.12+)
3. Runs in ~3 seconds -- Ruff is Rust-based, orders of magnitude faster than black + isort + flake8 combined
4. Single tool, single config -- everything lives in pyproject.toml, no .flake8, no .isort.cfg, no .pylintrc

Why Ruff, not black + isort + pylint?

Capability	black + isort (before)	Ruff (after)
Code formatting	yes	yes (black-compatible)
Import sorting	yes	yes (I rule, isort-compatible)
PEP 8 lint	no	yes (E rules)
Unused imports	no	yes (F401)
Bug detection	no	yes (B -- bugbear)
Security checks	no	yes (S -- bandit)
Complexity metrics	no	yes (PLR, C90)
CI enforcement	no	yes
Speed	~2-5s	~0.1-0.3s

Ruff is a strict superset of what we had. The formatting output is black-compatible, the import sorting is isort-compatible. We gain 7 additional rule categories and a proper CI gate. Nothing is lost.

What changed

Configuration (pyproject.toml)

• Removed [tool.black] and [tool.isort] sections

• Removed black and isort from dev dependencies

• Added ruff>=0.11.0 to dev dependencies

• Added [tool.ruff] with 10 rule sets:

  Rule set	Code	What it catches
  pycodestyle	E	PEP 8 style violations
  pyflakes	F	Unused imports, undefined names
  isort	I	Import ordering
  pyupgrade	UP	Old Python syntax for py310+
  bugbear	B	Common bugs (mutable defaults, etc.)
  simplify	SIM	Mechanically simplifiable code
  ruff-specific	RUF	Implicit Optional, mutable class defaults
  bandit security	S	Hardcoded passwords, unsafe eval, etc.
  pylint refactor	PLR	Complexity metrics
  McCabe	C90	Cyclomatic complexity

Line length: 88 --> 100

Changed from black's default 88 to 100. This is the primary cause of the large diff (345 files reformatted). The wider line length reduces unnecessary line wrapping in a CLI codebase with many long path strings and f-string messages.

CI (ci.yml)

Added a new lint job that runs in parallel with build-and-test:

lint:
steps:
- uv run --extra dev ruff check src/ tests/
- uv run --extra dev ruff format --check src/ tests/
- YAML encoding safety (existing grep check, moved here)
- File length guardrail (awk-based, max 3000 lines)
- No raw str(relative_to) (existing grep check, moved here)

The lint job uses uv run --extra dev ruff which runs the lockfile-pinned Ruff version (currently 0.15.12) -- same trust chain as the build job, no third-party GitHub Action needed, no GitHub API calls at runtime.

Pre-commit (.pre-commit-config.yaml)

Added optional pre-commit hooks for local development. Install with uv run pre-commit install. This is optional -- CI is the authoritative gate. The pre-commit hook rev may lag behind the CI version.

Bug fix (audit_report.py)

Fixed a real Python 3.10 compatibility bug discovered by Ruff's target-version = "py310" setting. A backslash inside an f-string (f"...{x.replace('|', '\\|')}...") is only valid in Python 3.12+. Extracted to a local variable.

Complexity thresholds -- the "strangler fig" approach

Current thresholds (set in this PR)

We ran Ruff against the entire codebase with max-*=1 and extracted every function's actual metric values using JSON output. Thresholds were set just above the current worst offenders -- existing code passes, but new code cannot make things worse:

Metric	Rule	Threshold	Current worst	Worst offender
File length	CI awk	3000	2917	github_downloader.py
Statements/fn	PLR0915	250	244	commands/install.py
Complexity	C901	65	62	install/validation.py
Branches/fn	PLR0912	70	67	commands/install.py
Args/fn	PLR0913	18	16	commands/install.py
Returns/fn	PLR0911	18	16	marketplace/publisher.py

Tightening roadmap (simulation results)

We simulated progressively stricter thresholds to identify which files would break at each stage:

	Current	Stage 1	Stage 2	Stage 3	Target
File length	3000	1500	800	500	400
Statements/fn	250	150	80	50	30
Complexity	65	50	30	20	15
Branches/fn	70	50	30	20	15
Args/fn	18	12	10	8	6
Returns/fn	18	12	8	6	5
Files breaking	0	8	34	58	79

Stage 1 -- quick wins (8 files)

Only the worst outliers. See follow-up issue #1004 for details.

File	Violations
commands/install.py	stmts=244, complexity=60, branches=67, args=16
install/validation.py	complexity=62
integration/mcp_integrator.py	complexity=49, branches=52
commands/compile/cli.py	stmts=180
deps/github_downloader.py	file-length=2917
commands/audit.py	complexity=43
marketplace/publisher.py	returns=16
core/target_detection.py	returns=13

Stage 2 -- meaningful refactoring (34 files)

Hits real structural debt. Top offender clusters:

• Commands layer -- install.py, deps/cli.py, audit.py, uninstall/cli.py (large Click handlers doing too much)
• Integration layer -- mcp_integrator.py, skill_integrator.py, hook_integrator.py (god-class integrators)
• Deps layer -- github_downloader.py (2917 lines), plugin_parser.py

Stage 3 -- disciplined codebase (58 files)

Industry best practices territory (50 stmts, 20 complexity). Nearly every command and integrator needs splitting. Each file becomes genuinely readable and testable in isolation.

Target -- gold standard (79 files)

400-line files, 30-statement functions, complexity 15. This is what a mature, well-decomposed CLI looks like.

Key insight: commands/install.py is the current single worst outlier across statements, complexity, branches, and args. Refactoring it alone would unlock Stage 1 across multiple metrics.

Globally ignored rules

Permanently ignored (intentional patterns in a CLI tool)

Rule	Why
PLR2004	Magic value comparison -- too noisy, especially in tests
E501	Line too long -- ruff format handles code; remaining are strings/URLs
SIM117	Multiple with statements -- style preference, 65 violations
S110	try-except-pass -- intentional graceful degradation in CLI error handling
S602	subprocess with shell=True -- intentional in a CLI wrapping git/pip

Deferred to future PRs

Rule	Violations	Effort
RUF003 + RUF002	22	Review against ASCII convention, replace with ASCII equivalents (#1005)
SIM102	28	Auto-fixable collapsible if statements

Existing violations -- # noqa directives

908 # noqa directives across the codebase. Each names the specific rule code for searchability. Top categories:

Rule	Count	What it means
F401	388	Unused imports (many are intentional re-exports or lazy imports) (#1003)
UP035	93	Deprecated imports (e.g. typing.Optional)
RUF013	92	Implicit Optional (e.g. def f(x: str = None))
F841	80	Unused variables
B904	60	raise without from in except (needs manual from err/from None decision)
RUF059	56	Unused unpacked variables

These are all pre-existing patterns. The noqa directives make them visible and searchable -- each can be addressed in a dedicated cleanup PR by grepping for the rule code.

Follow-up issues

• fix: CodeQL alert -- clear-text storage of sensitive information in plugin_parser.py #1001 -- CodeQL alert: clear-text storage in plugin_parser.py (pre-existing)
• chore: audit F401 noqa suppressions -- genuine re-exports vs dead code #1003 -- Audit F401 noqa suppressions (re-exports vs dead code)
• refactor: Stage 1 strangler-fig -- extract mcp_integrator.py::install #1004 -- Stage 1 strangler-fig: refactor worst outliers (tagged good first issue)
• chore: enable RUF002/RUF003 (ambiguous Unicode characters) #1005 -- Enable RUF002/RUF003 (ambiguous Unicode characters)

How to reproduce the simulation

# Extract actual metric values for any rule (set threshold to 1 to capture all)
uv run ruff check --select PLR0915 --config 'lint.pylint.max-statements=1' \
--output-format json src/ | python3 -c "import json, re, sysfor d in json.loads(sys.stdin.read()): m = re.search(r'\((\d+)', d['message']) if m: print(f\"{int(m.group(1)):>4} {d['filename']}\")"| sort -rn
# File lengths
find src/ -name '*.py' -print0 | xargs -0 -I{} awk \
'END { if (NR > 100) printf "%d %s\n", NR, FILENAME }' {} | sort -rn

Testing

• ruff check src/ tests/ -- all checks passed
• ruff format --check src/ tests/ -- 546 files clean
• uv run pytest tests/unit tests/test_console.py -x -- 6564 passed, 0 failed

[github-actions]

APM Review Panel Verdict

Disposition: REQUEST_CHANGES (two required pre-merge actions: SHA-pin astral-sh/ruff-action@v3 in both CI steps; add CHANGELOG.md entry)

---

Per-persona findings

Python Architect: This is a toolchain modernization PR -- no class hierarchy is restructured, no new abstract base is introduced, and no execution-path fork is added. The one semantic change is an f-string variable extraction in audit_report.py. Diagrams reflect module-boundary context and the new CI gate path.

1. OO / class diagram

classDiagram
direction TB
class pyproject_toml {
<<Configuration>>
+line-length : 100
+target-version : py310
+select : 10 rule sets
+lint.pylint thresholds
+lint.mccabe thresholds
}
class ci_yml {
<<IOBoundary>>
+lint job
+build-and-test job
}
class ruff_action {
<<ExternalDep>>
astral-sh/ruff-action@v3
}
class AuditReporter {
<<Pure>>
+generate_report() str
-_format_cell(x str) str
}
class pre_commit_config {
<<Configuration>>
+ruff hook
+ruff-format hook
}
pyproject_toml ..> ci_yml : governs thresholds
ci_yml ..> ruff_action : invokes
pre_commit_config ..> ruff_action : mirrors
class pyproject_toml:::touched
class ci_yml:::touched
class AuditReporter:::touched
class pre_commit_config:::touched
classDef touched fill:#fff3b0,stroke:#d47600

Loading

2. Execution flow diagram

flowchart TD
PR[PR push / merge_group] --> LINT[lint job -- new]
LINT --> RL["[EXEC] astral-sh/ruff-action@v3\nruff check src/ tests/"]
LINT --> RF["[EXEC] astral-sh/ruff-action@v3\nruff format --check src/ tests/"]
LINT --> YE["[I/O] YAML encoding safety\ngrep for non-ASCII bytes"]
LINT --> FL["[I/O] File length guardrail\nawk max 2600 lines"]
LINT --> NRS["[I/O] No raw str(relative_to)\ngrep check"]
RL -->|fail -- ~3s| FAIL[CI fails fast]
RF -->|fail -- ~3s| FAIL
YE -->|fail| FAIL
FL -->|fail| FAIL
NRS -->|fail| FAIL
RL -->|pass| BT
RF -->|pass| BT
YE -->|pass| BT
FL -->|pass| BT
NRS -->|pass| BT
BT["build-and-test job\nuv run pytest tests/unit tests/test_console.py -n auto --dist worksteal\n4029 tests pass"]
BT -->|pass| DONE[Ready for merge]
style FAIL fill:#ffcccc
style DONE fill:#ccffcc
style LINT fill:#fff3b0,stroke:#d47600

Loading

Design patterns

• Used in this PR: none -- toolchain configuration and mass reformatting. The audit_report.py fix is a local-variable extraction with no pattern applicability.
• Pragmatic suggestion: the strangler-fig threshold table (max-statements=300, max-complexity=110) is the correct ratchet pattern for this scope -- each tightening stage is a dedicated PR. No additional pattern is needed here.

Thresholds set just above current worst-case maximums is architecturally sound for a first-pass gate. The Python 3.10 compatibility bug caught by target-version = "py310" (backslash in f-string, audit_report.py) validates the tooling investment independently. The 522-file diff is noise from line-length reformatting; the structurally interesting changes are confined to four files: pyproject.toml, ci.yml, .pre-commit-config.yaml, and audit_report.py.

---

CLI Logging Expert: command_logger.py is -37 lines from whitespace/line-wrap cleanup only -- no CommandLogger lifecycle method changes, no _rich_* routing changes, no STATUS_SYMBOLS altered. All 908 # noqa suppressions are named by specific rule code (never bare # noqa), which is the correct pattern: each can be grep'd for targeted cleanup in dedicated PRs. Dominant categories (F401 re-exports, UP035 legacy typing imports, RUF013 implicit Optional) are pre-existing patterns made visible, not new anti-patterns introduced. Output architecture is structurally unchanged.

---

DevX UX Expert: No user-facing CLI surface changes. The PR is developer-infrastructure only. Positive contributor DX signals: CONTRIBUTING.md now documents uv run ruff check src/ tests/ in a discoverable place; .pre-commit-config.yaml with an explicit opt-in instruction (uv run pre-commit install) follows the same mental model as npm run prepare -- CI is authoritative, local hook is convenience. The 3-second lint gate gives contributors fast feedback before the heavier build job, which is the right ergonomic choice (same as cargo fmt --check running before cargo test).

Minor concern: .pre-commit-config.yaml pins rev: v0.11.12 while pyproject.toml requires ruff>=0.11.0. These can drift silently. Document in CONTRIBUTING.md that CI uses the pyproject.toml range and pre-commit may differ -- or synchronize with a uv run ruff --version note. This is optional; CI remains the gate.

No quickstart, README, or docs/getting-started/ pages are affected.

---

Supply Chain Security Expert: Critical finding -- astral-sh/ruff-action@v3 is pinned to a mutable version tag in both CI steps (ruff check and ruff format --check). GitHub Actions tags can be force-pushed by the action maintainer or a compromised account. Per GitHub's own security hardening guide and OSSF Scorecard, all third-party actions that download and execute binaries must be pinned to the full immutable commit SHA:

# Required form:uses: astral-sh/ruff-action@f6dc84c3be43b9ba22f1d1e0bd3bd11b96fb3f25 # v3.x.x

Look up the current SHA with gh api repos/astral-sh/ruff-action/git/refs/tags/v3. This is a required action before merge.

Secondary (informational): .pre-commit-config.yaml uses rev: v0.11.12 (a mutable tag). Pre-commit is local-only and not a CI gate so the blast radius is contained, but a full SHA pin would follow best practice.

Positive findings:

• Bandit S rules now run against the full codebase -- real security scanning added
• S602 and S110 permanently ignored with documented rationale -- both are intentional CLI patterns
• Per-file ignores for tests/** are scoped and named specifically (not global noise suppression)
• No new path traversal or auth surface introduced by the reformatting
• RUF003/RUF002 (ambiguous Unicode characters) deferred but explicitly noted for a follow-up PR -- aligns with the repo ASCII convention

---

Auth Expert: Fast-path trigger fired: src/apm_cli/core/auth.py (+91/-80), src/apm_cli/core/token_manager.py (+98/-93), and src/apm_cli/core/azure_cli.py (+13/-14) are all in the changed file list.

Reviewed changes:

auth.py: from typing import Callable replaced with from collections.abc import Callable (correct Python 3.10+ canonical import); Optional[T] -> T | None (PEP 604, semantically identical); auth_scheme default reformatted across lines (no semantic change); # noqa: F401 added to is_github_hostname import (this is a legitimate re-export used by callers who import it from auth.py rather than github_host.py directly -- suppression is correct).

token_manager.py: Single-to-double quote normalization in TOKEN_PRECEDENCE and RUNTIME_ENV_VARS dicts; # noqa: RUF012 on class-level mutable dicts (correct suppression for intentional class vars); Dict[Tuple[str, Optional[int]], Optional[str]] -> dict[tuple[str, int | None], str | None] (built-in generics, semantically identical).

azure_cli.py: Optional[str]/Optional[float] -> str | None/float | None; subprocess.run argument list reflowed to fit 100-char line limit -- no behavioral change.

AuthResolver precedence chain: unchanged. Token fallback semantics: unchanged. Host classification: unchanged. Credential fill behavior: unchanged. No auth regression. No credential exposure surface added.

---

OSS Growth Hacker: Three growth-relevant signals for CEO consideration:

1. External contributor quality -- sergio-sisternes-epam (EPAM Systems) is contributing code quality infrastructure rather than a feature. This is a community health signal worth a named thank-you in the CHANGELOG entry and release notes. External contributors improving tooling infrastructure is the OSS flywheel working.

2. Ruff ecosystem association -- Astral (Ruff/uv) is the fastest-growing Python tooling brand in 2024-2025. astral-sh/ruff-action@v3 visible in the workflow file is a positive halo signal to Python OSS evaluators. "APM is a Ruff shop" reads as quality-conscious.

3. Concrete quality story -- the PR description contains publication-ready numbers: "4029 tests pass", "~3s lint gate", "388 files reformatted", "strangler-fig roadmap to Stage 1 (8 files)". The CHANGELOG entry should preserve these numbers -- they are the kind of concrete claims that appear in "cool projects" threads.

Side-channel to CEO: The strangler-fig roadmap table (Stages 1-4 with exact file counts) is an unusually contributor-friendly artifact -- it tells future contributors exactly which files to target and what the expected complexity reduction is. Worth calling out in the release note as "we published our technical debt backlog." This lowers the barrier for a first contribution.

---

CEO arbitration

All six specialists agree on the substance: this is a well-executed toolchain modernization from an external contributor that improves code quality without introducing regressions. The specialists do not disagree with each other -- the only actionable finding is the Supply Chain Security expert's SHA-pin requirement, which is correct and non-negotiable. The fix is mechanical (30 seconds to look up the SHA and substitute it in two places in ci.yml), so it should not delay the merge meaningfully.

The CHANGELOG omission is also required: per repo convention, every PR changing code, tests, docs, or dependencies gets a CHANGELOG entry. This PR replaces two dev dependencies, adds a CI job, and changes the line-length policy -- all three are developer-visible. The entry should credit the external contributor.

The strangler-fig threshold approach (thresholds at current-worst-plus-one, published tightening roadmap) is the correct strategic call for a growing OSS codebase: it stops quality regression immediately without requiring a big-bang refactor that would block this PR for months. Stage 1 (8 files, dominated by mcp_integrator.py::install which owns three metrics simultaneously) is the obvious first follow-up issue.

The target-version = "py310" call is validated by the real bug it caught. Ratified.

Final call: Two mechanical pre-merge actions (SHA-pin, CHANGELOG). Everything else merges as-is.

---

Required actions before merge

1. SHA-pin astral-sh/ruff-action@v3 in .github/workflows/ci.yml (both steps: ruff check and ruff format --check). Look up the current immutable SHA with: gh api repos/astral-sh/ruff-action/git/refs/tags/v3 --jq .object.sha. Replace astral-sh/ruff-action@v3 with astral-sh/ruff-action@<sha> # v3.x.x in both places. This prevents tag-mutation supply chain attacks.

2. Add a CHANGELOG.md entry under ## [Unreleased]. Suggested wording (per Keep a Changelog format):

   ### Changed
   - Replace `black` + `isort` with Ruff for formatting and linting; add deterministic CI lint gate (~3s), configure 10 rule sets with strangler-fig complexity thresholds, and fix a Python 3.10 f-string compatibility bug in `audit_report.py` -- by @sergio-sisternes-epam (#999)
   

---

Optional follow-ups

• Open a follow-up issue tracking Stage 1 of the strangler-fig roadmap: refactor integration/mcp_integrator.py::install (282 statements, 104 complexity, 125 branches) to unlock three metric reductions simultaneously. Tag it good-first-issue for external contributors -- the roadmap table in this PR body is already the spec.
• Synchronize the pre-commit rev: v0.11.12 pin with the pyproject.tomlruff>=0.11.0 floor, or add a note to CONTRIBUTING.md clarifying that the CI range is authoritative and the pre-commit rev may lag intentionally.
• Schedule a dedicated cleanup PR for the 388 F401 noqa suppressions in re-export positions: audit which are genuine re-exports (keep) vs dead code (remove). grep -r "# noqa: F401" src/ | wc -l is your starting point.
• Enable RUF002/RUF003 (ambiguous Unicode in comments/docstrings) in a follow-up PR after auditing the 22 violations against the repo ASCII convention.

Generated by PR Review Panel for issue #999 · ● 864.2K · ◷

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Sergio Sisternes (sergio-sisternes-epam)]

Panel feedback -- all items addressed

#	Finding	Required?	Status	Details
1	SHA-pin astral-sh/ruff-action@v3	Yes	Done	Replaced entirely with uv run --extra dev ruff -- no third-party action at all
2	Add CHANGELOG.md entry	Yes	Done	Added under [Unreleased], credits @sergio-sisternes-epam (commit 17d6aec7)
3	CodeQL clear-text storage (plugin_parser.py:534)	No (pre-existing)	Done	Follow-up issue #1001 created, review thread resolved
4	Sync pre-commit rev with uv.lock	Optional	Done	Bumped v0.11.12 -> v0.15.12, clarified CI-authoritative note in CONTRIBUTING.md
5	Stage 1 strangler-fig follow-up issue	Optional	Done	#1004 -- tagged good first issue
6	F401 re-export audit	Optional	Done	#1003
7	Enable RUF002/RUF003 (ambiguous Unicode)	Optional	Done	#1005
8	Credit external contributor	Optional	Done	Included in CHANGELOG entry