From 388559d726e24a080733a49024f0c4bc7e63baaf Mon Sep 17 00:00:00 2001 From: deimagjas Date: Thu, 14 May 2026 06:31:13 -0500 Subject: [PATCH] feat(cli): add ruff linter (Google style + C4) and apply-sensors target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Configure ruff in app/cli/ with Google Python Style Guide conventions plus the C4 (flake8-comprehensions) rule family. Activates check + format with line-length=100, pydocstyle Google convention, and per-file-ignores that exempt tests from docstring and unused-arg rules. Adds a new `apply-sensors` Makefile target that runs lint, unit tests, and the mutation-testing gate (without acceptance, by design). Fixes all findings: renames `vars` → `make_vars` (7 sites), adds module/package/function docstrings, sorts imports, and collapses nested `with` statements via the parenthesized context manager. Documents the Google rules that ruff cannot enforce in the root CLAUDE.md. Mutation score after the change: 98.6% (71/72). Co-Authored-By: Claude Opus 4.7 --- CLAUDE.md | 17 ++++++ app/cli/Makefile | 13 ++++ app/cli/pyproject.toml | 46 ++++++++++++++ app/cli/src/container_cli/__init__.py | 1 + .../src/container_cli/commands/__init__.py | 1 + app/cli/src/container_cli/commands/agents.py | 12 ++-- app/cli/src/container_cli/commands/build.py | 10 ++-- app/cli/src/container_cli/commands/network.py | 10 ++-- .../src/container_cli/commands/pi_agents.py | 41 +++++-------- app/cli/src/container_cli/commands/run.py | 22 +++---- app/cli/src/container_cli/main.py | 2 + app/cli/src/container_cli/utils.py | 32 ++++++++++ app/cli/tests/acceptance/conftest.py | 16 ++--- .../tests/acceptance/steps/agents_steps.py | 4 +- .../tests/acceptance/steps/common_steps.py | 28 +++------ .../tests/acceptance/steps/pi_agents_steps.py | 4 +- app/cli/tests/conftest.py | 18 +++--- app/cli/tests/test_agents.py | 30 +++++++--- app/cli/tests/test_pi_agents.py | 60 ++++++++++++++----- app/cli/tests/test_utils.py | 47 +++++++++------ 20 files changed, 286 insertions(+), 128 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c5cc89f..96508f2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -91,6 +91,23 @@ Mutation tests (run via `make mutation-ci-threshold`) act as the safety net that For skill evals (LLM-graded), see the **Skill evals** section below. +## Python code conventions (`app/cli/`) + +The Python CLI follows the [Google Python Style Guide](https://google.github.io/styleguide/pyguide.html). Ruff (configured in `app/cli/pyproject.toml`) enforces the mechanical rules: line length 100, import order, pep8-naming, pydocstyle (Google convention), pyupgrade, bugbear, comprehensions (C4), simplify, return, pathlib, unused-args, eradicate, builtins, tidy-imports. Run `make apply-sensors` to auto-fix + verify the full quality pipeline (lint, unit tests, mutation gate). + +The rules below cannot be enforced by Ruff and must be applied by hand: + +- **Docstring content (Google sections)**: the first line is a one-sentence imperative summary. When parameters, return value, raised exceptions, or yielded items deserve documentation, use `Args:`, `Returns:`, `Raises:`, `Yields:` sections indented 4 spaces under the summary. +- **`TODO` markers**: always `TODO(@username):` or `TODO(#issue-id):` — never a bare `TODO`. The identifier makes ownership explicit and lets `git grep TODO` find actionable items. +- **`TYPE_CHECKING` imports**: imports used only inside type annotations go inside `if TYPE_CHECKING:` and are referenced as forward-reference strings. Avoid runtime cost for typing-only dependencies. +- **`assert` only in tests**: production code under `src/` must not rely on `assert` for control flow — `python -O` strips them. Raise specific exceptions instead (`raise ValueError(...)`, `raise typer.Exit(1)`). +- **Specific exceptions**: never `except Exception:` bare; catch the concrete types relevant to the operation (`subprocess.CalledProcessError`, `OSError`, `typer.Exit`). +- **`typer.echo` vs `logging`**: `typer.echo` is for user-facing CLI output only. Any diagnostic or trace information goes through `logger = logging.getLogger(__name__)`. +- **Naming semantics**: variable names must not shadow built-ins (`vars`, `id`, `type`, `list`, `dict`, `input`). Prefer descriptive names: `make_vars`, `env_overrides`, `image_tag`. Function names are imperative verbs in `snake_case`. +- **No mutable default arguments**: never `def f(x: list = []):`. Use `None` and materialise inside (`x = x if x is not None else []`). +- **Absolute imports**: inside `container_cli/` always import with the full package path (`from container_cli.utils import run_make`), never relative. +- **Function size**: revisit any function past ~25 lines and consider splitting; single responsibility per function. + ## Architecture - **`config/`** — Container infrastructure: `Dockerfile.wolfi` (production, multi-stage: Rust tool compilation → runtime with Claude CLI, Node, Python), `entrypoint.sh` (credential injection + worktree creation + su-exec privilege drop), `Makefile` (orchestration) diff --git a/app/cli/Makefile b/app/cli/Makefile index 03d3b4e..b6568a9 100644 --- a/app/cli/Makefile +++ b/app/cli/Makefile @@ -44,3 +44,16 @@ eval-skills: # Pre-PR quality gate local-qa: acceptance-test eval-skills + +# Quality sensors: lint + format + unit tests + mutation gate (no acceptance) +.PHONY: lint unit-test apply-sensors + +lint: + uv run ruff check --fix . + uv run ruff format . + +unit-test: + uv run pytest tests --ignore=tests/acceptance -v + +apply-sensors: lint unit-test mutation-ci-threshold + @echo "[apply-sensors] OK — lint clean, unit tests green, mutation >= 70%" diff --git a/app/cli/pyproject.toml b/app/cli/pyproject.toml index 6a6ea74..310893f 100644 --- a/app/cli/pyproject.toml +++ b/app/cli/pyproject.toml @@ -34,3 +34,49 @@ paths_to_mutate = ["src/container_cli/"] only_covered_tests = false # CI gate threshold (enforced by `make mutation-ci-threshold`, not by mutmut itself) # threshold = 70 + +[tool.ruff] +line-length = 100 +target-version = "py313" +src = ["src", "tests"] +extend-exclude = ["mutants"] + +[tool.ruff.lint] +select = [ + "E", "W", + "F", + "I", + "N", + "D", + "UP", + "B", + "C4", + "SIM", + "RET", + "PTH", + "ARG", + "ERA", + "A", + "TID", +] +ignore = [ + "D203", + "D213", + "D406", + "D407", + "D413", +] + +[tool.ruff.lint.pydocstyle] +convention = "google" + +[tool.ruff.lint.per-file-ignores] +"tests/**/*.py" = ["D", "ARG001", "ARG002", "ARG005", "E501"] +"src/container_cli/__init__.py" = ["D104"] +"src/container_cli/commands/__init__.py" = ["D104"] + +[tool.ruff.format] +quote-style = "double" +indent-style = "space" +line-ending = "lf" +docstring-code-format = true diff --git a/app/cli/src/container_cli/__init__.py b/app/cli/src/container_cli/__init__.py index e69de29..a35bb78 100644 --- a/app/cli/src/container_cli/__init__.py +++ b/app/cli/src/container_cli/__init__.py @@ -0,0 +1 @@ +"""Container management CLI (`q`) for Claude agent containers.""" diff --git a/app/cli/src/container_cli/commands/__init__.py b/app/cli/src/container_cli/commands/__init__.py index e69de29..6e1e0db 100644 --- a/app/cli/src/container_cli/commands/__init__.py +++ b/app/cli/src/container_cli/commands/__init__.py @@ -0,0 +1 @@ +"""Typer subcommand modules exposed by the `q` CLI.""" diff --git a/app/cli/src/container_cli/commands/agents.py b/app/cli/src/container_cli/commands/agents.py index 1b5c90c..30ef3fb 100644 --- a/app/cli/src/container_cli/commands/agents.py +++ b/app/cli/src/container_cli/commands/agents.py @@ -1,3 +1,5 @@ +"""Agent lifecycle commands (spawn, list, logs, follow, stop, status, summary).""" + import json import os from pathlib import Path @@ -28,14 +30,14 @@ def spawn( ) -> None: """Spawn a detached headless agent container.""" check_token() - vars: dict[str, str] = {"BRANCH": branch, "TASK": task} + make_vars: dict[str, str] = {"BRANCH": branch, "TASK": task} if cpus is not None: - vars["CPUS"] = str(cpus) + make_vars["CPUS"] = str(cpus) if memory: - vars["MEMORY"] = memory + make_vars["MEMORY"] = memory if image: - vars["IMAGE"] = image - run_make("spawn", vars) + make_vars["IMAGE"] = image + run_make("spawn", make_vars) @app.command(name="list") diff --git a/app/cli/src/container_cli/commands/build.py b/app/cli/src/container_cli/commands/build.py index 5e11a9f..ed72b03 100644 --- a/app/cli/src/container_cli/commands/build.py +++ b/app/cli/src/container_cli/commands/build.py @@ -1,3 +1,5 @@ +"""Image build and cleanup commands.""" + from typing import Annotated import typer @@ -13,12 +15,12 @@ def build( dockerfile: Annotated[str | None, typer.Option("--dockerfile", help="Dockerfile path")] = None, ) -> None: """Build the container image (no cache).""" - vars: dict[str, str] = {} + make_vars: dict[str, str] = {} if image: - vars["IMAGE"] = image + make_vars["IMAGE"] = image if dockerfile: - vars["DOCKERFILE"] = dockerfile - run_make("build", vars) + make_vars["DOCKERFILE"] = dockerfile + run_make("build", make_vars) @app.command() diff --git a/app/cli/src/container_cli/commands/network.py b/app/cli/src/container_cli/commands/network.py index fe8bc0a..75f08dc 100644 --- a/app/cli/src/container_cli/commands/network.py +++ b/app/cli/src/container_cli/commands/network.py @@ -1,3 +1,5 @@ +"""Bridge network management commands.""" + from typing import Annotated import typer @@ -13,9 +15,9 @@ def network( network_name: Annotated[str | None, typer.Option("--network-name", help="Network name")] = None, ) -> None: """Create the isolated bridge network.""" - vars: dict[str, str] = {} + make_vars: dict[str, str] = {} if subnet: - vars["SUBNET"] = subnet + make_vars["SUBNET"] = subnet if network_name: - vars["NETWORK"] = network_name - run_make("network", vars) + make_vars["NETWORK"] = network_name + run_make("network", make_vars) diff --git a/app/cli/src/container_cli/commands/pi_agents.py b/app/cli/src/container_cli/commands/pi_agents.py index 04fef27..0241acb 100644 --- a/app/cli/src/container_cli/commands/pi_agents.py +++ b/app/cli/src/container_cli/commands/pi_agents.py @@ -32,34 +32,26 @@ def _agents_home() -> Path: @app.command() def build( - image: Annotated[ - str | None, typer.Option("--image", help="PI image tag") - ] = None, + image: Annotated[str | None, typer.Option("--image", help="PI image tag")] = None, dockerfile: Annotated[ str | None, typer.Option("--dockerfile", help="Path to PI Dockerfile") ] = None, ) -> None: """Build the PI agent image (Ubuntu 26.04 + PI SDK).""" - vars: dict[str, str] = {} + make_vars: dict[str, str] = {} if image: - vars["PI_IMAGE"] = image + make_vars["PI_IMAGE"] = image if dockerfile: - vars["PI_DOCKERFILE"] = dockerfile - run_make("build-pi", vars) + make_vars["PI_DOCKERFILE"] = dockerfile + run_make("build-pi", make_vars) @app.command() def spawn( - branch: Annotated[ - str, typer.Option("--branch", help="Git branch for the PI agent worktree") - ], - task: Annotated[ - str, typer.Option("--task", help="Task description for the PI agent") - ], + branch: Annotated[str, typer.Option("--branch", help="Git branch for the PI agent worktree")], + task: Annotated[str, typer.Option("--task", help="Task description for the PI agent")], cpus: Annotated[int | None, typer.Option("--cpus", help="CPU count")] = None, - memory: Annotated[ - str | None, typer.Option("--memory", help="Memory limit (e.g. 3G)") - ] = None, + memory: Annotated[str | None, typer.Option("--memory", help="Memory limit (e.g. 3G)")] = None, image: Annotated[str | None, typer.Option("--image", help="PI image tag")] = None, base_url: Annotated[ str | None, @@ -82,21 +74,20 @@ def spawn( uv run iac server status """ typer.echo( - "[pi] reminder: ensure mlx_lm.server is running " - "(`uv run iac server status` from /iac)" + "[pi] reminder: ensure mlx_lm.server is running (`uv run iac server status` from /iac)" ) - vars: dict[str, str] = {"BRANCH": branch, "TASK": task} + make_vars: dict[str, str] = {"BRANCH": branch, "TASK": task} if cpus is not None: - vars["CPUS"] = str(cpus) + make_vars["CPUS"] = str(cpus) if memory: - vars["MEMORY"] = memory + make_vars["MEMORY"] = memory if image: - vars["PI_IMAGE"] = image + make_vars["PI_IMAGE"] = image if base_url: - vars["PI_BASE_URL"] = base_url + make_vars["PI_BASE_URL"] = base_url if model_id: - vars["PI_MODEL_ID"] = model_id - run_make("spawn-pi", vars) + make_vars["PI_MODEL_ID"] = model_id + run_make("spawn-pi", make_vars) @app.command(name="list") diff --git a/app/cli/src/container_cli/commands/run.py b/app/cli/src/container_cli/commands/run.py index aa78d80..1f17828 100644 --- a/app/cli/src/container_cli/commands/run.py +++ b/app/cli/src/container_cli/commands/run.py @@ -1,3 +1,5 @@ +"""Interactive coordinator container (`run` and `shell` aliases).""" + from typing import Annotated import typer @@ -15,14 +17,14 @@ def run( ) -> None: """Run an interactive coordinator shell (hands off TTY).""" check_token() - vars: dict[str, str] = {} + make_vars: dict[str, str] = {} if cpus is not None: - vars["CPUS"] = str(cpus) + make_vars["CPUS"] = str(cpus) if memory: - vars["MEMORY"] = memory + make_vars["MEMORY"] = memory if name: - vars["NAME"] = name - run_make("run", vars, tty=True) + make_vars["NAME"] = name + run_make("run", make_vars, tty=True) @app.command() @@ -33,11 +35,11 @@ def shell( ) -> None: """Alias for run — open an interactive shell in the coordinator container.""" check_token() - vars: dict[str, str] = {} + make_vars: dict[str, str] = {} if cpus is not None: - vars["CPUS"] = str(cpus) + make_vars["CPUS"] = str(cpus) if memory: - vars["MEMORY"] = memory + make_vars["MEMORY"] = memory if name: - vars["NAME"] = name - run_make("shell", vars, tty=True) + make_vars["NAME"] = name + run_make("shell", make_vars, tty=True) diff --git a/app/cli/src/container_cli/main.py b/app/cli/src/container_cli/main.py index 80efbf9..dbb0ff8 100644 --- a/app/cli/src/container_cli/main.py +++ b/app/cli/src/container_cli/main.py @@ -1,3 +1,5 @@ +"""Entry point: registers top-level commands and sub-apps on the `q` Typer app.""" + import typer from container_cli.commands import agents, build, network, pi_agents, run diff --git a/app/cli/src/container_cli/utils.py b/app/cli/src/container_cli/utils.py index e4ff460..c22fdde 100644 --- a/app/cli/src/container_cli/utils.py +++ b/app/cli/src/container_cli/utils.py @@ -1,3 +1,5 @@ +"""Shared helpers: git root resolution, Makefile runner, and token validation.""" + import os import subprocess from pathlib import Path @@ -6,6 +8,15 @@ def find_git_root() -> Path: + """Return the absolute path of the repository root. + + Returns: + Path to the top-level directory reported by `git rev-parse --show-toplevel`. + + Raises: + subprocess.CalledProcessError: If the current working directory is not inside + a git repository. + """ result = subprocess.run( ["git", "rev-parse", "--show-toplevel"], capture_output=True, @@ -16,10 +27,20 @@ def find_git_root() -> Path: def makefile_dir() -> Path: + """Return the directory that contains the orchestration Makefile. + + Returns: + Path to `config/` under the git root. + """ return find_git_root() / "config" def check_token() -> None: + """Verify that the Claude container OAuth token is exported. + + Raises: + typer.Exit: With code 1 if `CLAUDE_CONTAINER_OAUTH_TOKEN` is unset or empty. + """ token = os.environ.get("CLAUDE_CONTAINER_OAUTH_TOKEN") if not token: typer.echo( @@ -30,6 +51,17 @@ def check_token() -> None: def run_make(target: str, extra_vars: dict[str, str] | None = None, *, tty: bool = False) -> None: + """Invoke a Makefile target inside the project's `config/` directory. + + Args: + target: The Make target name to execute. + extra_vars: Optional mapping of variables passed as `KEY=VALUE` to make. + tty: If True, replace the current process with `make` so it inherits the TTY + (used by interactive commands like `run` and `follow`). + + Raises: + typer.Exit: When the subprocess returns a non-zero exit code (non-TTY path). + """ vars_list = [f"{k}={v}" for k, v in (extra_vars or {}).items()] cmd = ["make", "-C", str(makefile_dir()), target, *vars_list] diff --git a/app/cli/tests/acceptance/conftest.py b/app/cli/tests/acceptance/conftest.py index b62d508..6d00b06 100644 --- a/app/cli/tests/acceptance/conftest.py +++ b/app/cli/tests/acceptance/conftest.py @@ -36,13 +36,15 @@ def invocation_context( monkeypatch.delenv("AGENTS_HOME", raising=False) - with patch("container_cli.commands.agents.run_make") as m_agents, \ - patch("container_cli.commands.build.run_make") as m_build, \ - patch("container_cli.commands.run.run_make") as m_run, \ - patch("container_cli.commands.network.run_make") as m_network, \ - patch("container_cli.commands.pi_agents.run_make") as m_pi, \ - patch("container_cli.commands.agents.find_git_root", return_value=repo), \ - patch("container_cli.commands.pi_agents.find_git_root", return_value=repo): + with ( + patch("container_cli.commands.agents.run_make") as m_agents, + patch("container_cli.commands.build.run_make") as m_build, + patch("container_cli.commands.run.run_make") as m_run, + patch("container_cli.commands.network.run_make") as m_network, + patch("container_cli.commands.pi_agents.run_make") as m_pi, + patch("container_cli.commands.agents.find_git_root", return_value=repo), + patch("container_cli.commands.pi_agents.find_git_root", return_value=repo), + ): ctx = InvocationContext( runner=CliRunner(), mocks={ diff --git a/app/cli/tests/acceptance/steps/agents_steps.py b/app/cli/tests/acceptance/steps/agents_steps.py index dac089b..000eaee 100644 --- a/app/cli/tests/acceptance/steps/agents_steps.py +++ b/app/cli/tests/acceptance/steps/agents_steps.py @@ -3,9 +3,7 @@ from tests.acceptance.steps.common_steps import * # noqa: F401, F403 -@given(parsers.parse( - 'a status file exists for branch "{branch}" with payload {payload}' -)) +@given(parsers.parse('a status file exists for branch "{branch}" with payload {payload}')) def _status_file_exists(invocation_context, branch: str, payload: str) -> None: status_dir = invocation_context.agents_home / branch / ".agent" status_dir.mkdir(parents=True, exist_ok=True) diff --git a/app/cli/tests/acceptance/steps/common_steps.py b/app/cli/tests/acceptance/steps/common_steps.py index 69c0111..6a78ab1 100644 --- a/app/cli/tests/acceptance/steps/common_steps.py +++ b/app/cli/tests/acceptance/steps/common_steps.py @@ -7,7 +7,6 @@ from container_cli.main import app - _VAR_PATTERN = re.compile(r'(\w+)="([^"]*)"') @@ -22,16 +21,12 @@ def _make_runner_ready(invocation_context) -> None: @given("the CLAUDE_CONTAINER_OAUTH_TOKEN is set") def _token_set(invocation_context) -> None: - invocation_context.monkeypatch.setenv( - "CLAUDE_CONTAINER_OAUTH_TOKEN", "test-token-123" - ) + invocation_context.monkeypatch.setenv("CLAUDE_CONTAINER_OAUTH_TOKEN", "test-token-123") @given("the CLAUDE_CONTAINER_OAUTH_TOKEN is not set") def _token_not_set(invocation_context) -> None: - invocation_context.monkeypatch.delenv( - "CLAUDE_CONTAINER_OAUTH_TOKEN", raising=False - ) + invocation_context.monkeypatch.delenv("CLAUDE_CONTAINER_OAUTH_TOKEN", raising=False) @when(parsers.parse('I run "{cmdline}"')) @@ -46,18 +41,14 @@ def _invoke_cli(invocation_context, cmdline: str) -> None: def _exits_successfully(invocation_context) -> None: result = invocation_context.result assert result is not None, "no CLI invocation recorded" - assert result.exit_code == 0, ( - f"exit_code={result.exit_code}, output={result.output!r}" - ) + assert result.exit_code == 0, f"exit_code={result.exit_code}, output={result.output!r}" @then("the command exits with an error") def _exits_with_error(invocation_context) -> None: result = invocation_context.result assert result is not None, "no CLI invocation recorded" - assert result.exit_code != 0, ( - f"expected non-zero exit, got 0; output={result.output!r}" - ) + assert result.exit_code != 0, f"expected non-zero exit, got 0; output={result.output!r}" @then(parsers.parse('the make runner was invoked with target "{target}"')) @@ -67,24 +58,19 @@ def _make_invoked_with_target(invocation_context, target: str) -> None: for call in mock.call_args_list: if call.args: targets_called.append(call.args[0]) - assert target in targets_called, ( - f"target {target!r} not in {targets_called}" - ) + assert target in targets_called, f"target {target!r} not in {targets_called}" @then(parsers.parse("the make vars include {vars_text}")) def _make_vars_include(invocation_context, vars_text: str) -> None: expected = _parse_vars(vars_text) - assert expected, f"no KEY=\"value\" pairs parsed from {vars_text!r}" + assert expected, f'no KEY="value" pairs parsed from {vars_text!r}' var_dicts: list[dict] = [] for mock in invocation_context.mocks.values(): for call in mock.call_args_list: if len(call.args) >= 2 and isinstance(call.args[1], dict): var_dicts.append(call.args[1]) - matched = any( - all(d.get(k) == v for k, v in expected.items()) - for d in var_dicts - ) + matched = any(all(d.get(k) == v for k, v in expected.items()) for d in var_dicts) assert matched, f"expected {expected} in one of {var_dicts}" diff --git a/app/cli/tests/acceptance/steps/pi_agents_steps.py b/app/cli/tests/acceptance/steps/pi_agents_steps.py index fd25c3a..3a5ddb3 100644 --- a/app/cli/tests/acceptance/steps/pi_agents_steps.py +++ b/app/cli/tests/acceptance/steps/pi_agents_steps.py @@ -3,9 +3,7 @@ from tests.acceptance.steps.common_steps import * # noqa: F401, F403 -@given(parsers.parse( - 'a PI status file exists for branch "{branch}" with payload {payload}' -)) +@given(parsers.parse('a PI status file exists for branch "{branch}" with payload {payload}')) def _pi_status_file_exists(invocation_context, branch: str, payload: str) -> None: status_dir = invocation_context.agents_home / branch / ".agent" status_dir.mkdir(parents=True, exist_ok=True) diff --git a/app/cli/tests/conftest.py b/app/cli/tests/conftest.py index 281af0e..75b3341 100644 --- a/app/cli/tests/conftest.py +++ b/app/cli/tests/conftest.py @@ -16,11 +16,13 @@ def fake_git_root(tmp_path: Path): @pytest.fixture() def mock_run_make(): """Patch run_make across all command modules.""" - with patch("container_cli.commands.agents.run_make") as m_agents, \ - patch("container_cli.commands.build.run_make") as m_build, \ - patch("container_cli.commands.run.run_make") as m_run, \ - patch("container_cli.commands.network.run_make") as m_network, \ - patch("container_cli.commands.pi_agents.run_make") as m_pi: + with ( + patch("container_cli.commands.agents.run_make") as m_agents, + patch("container_cli.commands.build.run_make") as m_build, + patch("container_cli.commands.run.run_make") as m_run, + patch("container_cli.commands.network.run_make") as m_network, + patch("container_cli.commands.pi_agents.run_make") as m_pi, + ): yield { "agents": m_agents, "build": m_build, @@ -33,8 +35,10 @@ def mock_run_make(): @pytest.fixture() def mock_check_token(): """Patch check_token across command modules that use it.""" - with patch("container_cli.commands.agents.check_token") as m_agents, \ - patch("container_cli.commands.run.check_token") as m_run: + with ( + patch("container_cli.commands.agents.check_token") as m_agents, + patch("container_cli.commands.run.check_token") as m_run, + ): yield {"agents": m_agents, "run": m_run} diff --git a/app/cli/tests/test_agents.py b/app/cli/tests/test_agents.py index 2a1de9c..c229235 100644 --- a/app/cli/tests/test_agents.py +++ b/app/cli/tests/test_agents.py @@ -1,11 +1,20 @@ from __future__ import annotations + from pathlib import Path from unittest.mock import patch import pytest import typer -from container_cli.commands.agents import _agents_home, follow, list_agents, logs, spawn, status, stop +from container_cli.commands.agents import ( + _agents_home, + follow, + list_agents, + logs, + spawn, + status, + stop, +) class TestSpawn: @@ -38,8 +47,11 @@ def test_spawn_all_optional_params(self, mock_run_make, mock_check_token): spawn(branch="b", task="t", cpus=2, memory="4G", image="img") call_vars = mock_run_make["agents"].call_args[0][1] assert call_vars == { - "BRANCH": "b", "TASK": "t", - "CPUS": "2", "MEMORY": "4G", "IMAGE": "img", + "BRANCH": "b", + "TASK": "t", + "CPUS": "2", + "MEMORY": "4G", + "IMAGE": "img", } @@ -85,9 +97,11 @@ def test_reads_status_file(self, tmp_path, monkeypatch): def test_agents_home_fallback(self, monkeypatch): monkeypatch.delenv("AGENTS_HOME", raising=False) - with patch("container_cli.commands.agents.find_git_root", return_value=Path("/fake/repo")): - with pytest.raises(typer.Exit) as exc_info: - status(branch="nonexistent-branch") + with ( + patch("container_cli.commands.agents.find_git_root", return_value=Path("/fake/repo")), + pytest.raises(typer.Exit) as exc_info, + ): + status(branch="nonexistent-branch") assert exc_info.value.exit_code == 1 @@ -98,6 +112,8 @@ def test_uses_env_var(self, tmp_path, monkeypatch): def test_fallback_path_name(self, monkeypatch): monkeypatch.delenv("AGENTS_HOME", raising=False) - with patch("container_cli.commands.agents.find_git_root", return_value=Path("/home/user/repo")): + with patch( + "container_cli.commands.agents.find_git_root", return_value=Path("/home/user/repo") + ): result = _agents_home() assert result == Path("/home/user/.worktrees") diff --git a/app/cli/tests/test_pi_agents.py b/app/cli/tests/test_pi_agents.py index 87c34e3..5193ad4 100644 --- a/app/cli/tests/test_pi_agents.py +++ b/app/cli/tests/test_pi_agents.py @@ -26,7 +26,15 @@ class TestSpawn: def test_basic_spawn(self, mock_run_make): - spawn(branch="pi/a", task="rename", cpus=None, memory=None, image=None, base_url=None, model_id=None) + spawn( + branch="pi/a", + task="rename", + cpus=None, + memory=None, + image=None, + base_url=None, + model_id=None, + ) mock_run_make["pi"].assert_called_once_with( "spawn-pi", {"BRANCH": "pi/a", "TASK": "rename"} ) @@ -37,28 +45,56 @@ def test_spawn_with_cpus(self, mock_run_make): assert call_vars["CPUS"] == "4" def test_spawn_with_memory(self, mock_run_make): - spawn(branch="b", task="t", cpus=None, memory="8G", image=None, base_url=None, model_id=None) + spawn( + branch="b", task="t", cpus=None, memory="8G", image=None, base_url=None, model_id=None + ) call_vars = mock_run_make["pi"].call_args[0][1] assert call_vars["MEMORY"] == "8G" def test_spawn_with_image(self, mock_run_make): - spawn(branch="b", task="t", cpus=None, memory=None, image="claude-pi:custom", base_url=None, model_id=None) + spawn( + branch="b", + task="t", + cpus=None, + memory=None, + image="claude-pi:custom", + base_url=None, + model_id=None, + ) call_vars = mock_run_make["pi"].call_args[0][1] assert call_vars["PI_IMAGE"] == "claude-pi:custom" def test_spawn_with_base_url(self, mock_run_make): - spawn(branch="b", task="t", cpus=None, memory=None, image=None, base_url="http://10.0.0.5:9000/v1", model_id=None) + spawn( + branch="b", + task="t", + cpus=None, + memory=None, + image=None, + base_url="http://10.0.0.5:9000/v1", + model_id=None, + ) call_vars = mock_run_make["pi"].call_args[0][1] assert call_vars["PI_BASE_URL"] == "http://10.0.0.5:9000/v1" def test_spawn_with_model_id(self, mock_run_make): - spawn(branch="b", task="t", cpus=None, memory=None, image=None, base_url=None, model_id="mlx-community/gemma-4-26b-a4b-it-4bit") + spawn( + branch="b", + task="t", + cpus=None, + memory=None, + image=None, + base_url=None, + model_id="mlx-community/gemma-4-26b-a4b-it-4bit", + ) call_vars = mock_run_make["pi"].call_args[0][1] assert call_vars["PI_MODEL_ID"] == "mlx-community/gemma-4-26b-a4b-it-4bit" def test_spawn_does_not_require_token(self, mock_run_make, monkeypatch): monkeypatch.delenv("CLAUDE_CONTAINER_OAUTH_TOKEN", raising=False) - spawn(branch="b", task="t", cpus=None, memory=None, image=None, base_url=None, model_id=None) + spawn( + branch="b", task="t", cpus=None, memory=None, image=None, base_url=None, model_id=None + ) mock_run_make["pi"].assert_called_once() @@ -84,25 +120,19 @@ def test_calls_run_make(self, mock_run_make): class TestLogs: def test_passes_branch(self, mock_run_make): logs(branch="pi/x") - mock_run_make["pi"].assert_called_once_with( - "logs-pi-agent", {"BRANCH": "pi/x"} - ) + mock_run_make["pi"].assert_called_once_with("logs-pi-agent", {"BRANCH": "pi/x"}) class TestFollow: def test_passes_branch_with_tty(self, mock_run_make): follow(branch="pi/x") - mock_run_make["pi"].assert_called_once_with( - "follow-pi-agent", {"BRANCH": "pi/x"}, tty=True - ) + mock_run_make["pi"].assert_called_once_with("follow-pi-agent", {"BRANCH": "pi/x"}, tty=True) class TestStop: def test_passes_branch(self, mock_run_make): stop(branch="pi/x") - mock_run_make["pi"].assert_called_once_with( - "stop-pi-agent", {"BRANCH": "pi/x"} - ) + mock_run_make["pi"].assert_called_once_with("stop-pi-agent", {"BRANCH": "pi/x"}) class TestStatus: diff --git a/app/cli/tests/test_utils.py b/app/cli/tests/test_utils.py index 204c27a..554fe8a 100644 --- a/app/cli/tests/test_utils.py +++ b/app/cli/tests/test_utils.py @@ -9,9 +9,9 @@ from container_cli.utils import check_token, find_git_root, makefile_dir, run_make - # ---------- find_git_root ---------- + class TestFindGitRoot: def test_returns_path_from_git(self): mock_result = MagicMock(stdout="/home/user/repo\n") @@ -31,16 +31,19 @@ def test_strips_whitespace(self): assert find_git_root() == Path("/tmp/repo") def test_raises_on_git_failure(self): - with patch( - "container_cli.utils.subprocess.run", - side_effect=subprocess.CalledProcessError(128, "git"), + with ( + patch( + "container_cli.utils.subprocess.run", + side_effect=subprocess.CalledProcessError(128, "git"), + ), + pytest.raises(subprocess.CalledProcessError), ): - with pytest.raises(subprocess.CalledProcessError): - find_git_root() + find_git_root() # ---------- makefile_dir ---------- + class TestMakefileDir: def test_returns_config_subdir(self, fake_git_root: Path): assert makefile_dir() == fake_git_root / "config" @@ -48,6 +51,7 @@ def test_returns_config_subdir(self, fake_git_root: Path): # ---------- check_token ---------- + class TestCheckToken: def test_passes_when_token_set(self, monkeypatch: pytest.MonkeyPatch): monkeypatch.setenv("CLAUDE_CONTAINER_OAUTH_TOKEN", "tok-abc") @@ -74,18 +78,21 @@ def test_exits_when_token_empty(self, monkeypatch: pytest.MonkeyPatch, capsys): # ---------- run_make ---------- + class TestRunMake: def test_basic_target(self, fake_git_root: Path): - with patch("container_cli.utils.os.execvp"), \ - patch("container_cli.utils.subprocess.run", return_value=MagicMock(returncode=0)) as m: + with ( + patch("container_cli.utils.os.execvp"), + patch("container_cli.utils.subprocess.run", return_value=MagicMock(returncode=0)) as m, + ): run_make("build") - m.assert_called_once_with( - ["make", "-C", str(fake_git_root / "config"), "build"] - ) + m.assert_called_once_with(["make", "-C", str(fake_git_root / "config"), "build"]) def test_with_extra_vars(self, fake_git_root: Path): - with patch("container_cli.utils.os.execvp"), \ - patch("container_cli.utils.subprocess.run", return_value=MagicMock(returncode=0)) as m: + with ( + patch("container_cli.utils.os.execvp"), + patch("container_cli.utils.subprocess.run", return_value=MagicMock(returncode=0)) as m, + ): run_make("spawn", {"BRANCH": "feat/x", "TASK": "do stuff"}) cmd = m.call_args[0][0] assert cmd[:4] == ["make", "-C", str(fake_git_root / "config"), "spawn"] @@ -93,8 +100,10 @@ def test_with_extra_vars(self, fake_git_root: Path): assert "TASK=do stuff" in cmd def test_raises_exit_on_failure(self, fake_git_root: Path): - with patch("container_cli.utils.os.execvp"), \ - patch("container_cli.utils.subprocess.run", return_value=MagicMock(returncode=2)): + with ( + patch("container_cli.utils.os.execvp"), + patch("container_cli.utils.subprocess.run", return_value=MagicMock(returncode=2)), + ): with pytest.raises(typer.Exit) as exc_info: run_make("build") assert exc_info.value.exit_code == 2 @@ -114,8 +123,12 @@ def test_tty_mode_passes_extra_vars(self, fake_git_root: Path): assert "NAME=test" in cmd def test_default_tty_uses_subprocess(self, fake_git_root: Path): - with patch("container_cli.utils.os.execvp") as m_exec, \ - patch("container_cli.utils.subprocess.run", return_value=MagicMock(returncode=0)) as m_sub: + with ( + patch("container_cli.utils.os.execvp") as m_exec, + patch( + "container_cli.utils.subprocess.run", return_value=MagicMock(returncode=0) + ) as m_sub, + ): run_make("build") m_sub.assert_called_once() m_exec.assert_not_called()