Type checking: mypy in lint and CI, 121 findings fixed

.github/workflows/ci.yml

@@ -14,4 +14,5 @@ jobs:
         with:
           cache: true
       - run: pixi run lint
+      - run: pixi run typecheck
       - run: pixi run test

README.md

@@ -75,4 +75,4 @@ Agents are untrusted code driven by text from issues, so they never run on the h
 | `docs/` | setup guides and the [mia-data workflow overview](docs/mia-data-workflow.md) |
 | `notes/` | design notes, runbook, specs |
 | `scripts/` | `autodev-up.sh`, `mia-data-up.sh` |
-| `tests/` | unit tests (`pixi run test`); no GitHub, LSF or Temporal server needed. `pixi run lint` runs ruff |
+| `tests/` | unit tests (`pixi run test`); no GitHub, LSF or Temporal server needed. `pixi run lint` runs ruff, `pixi run typecheck` runs mypy |

autodev/activities.py

@@ -13,7 +13,7 @@
 import re
 from collections.abc import Awaitable, Callable
 from pathlib import Path
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, cast
 
 from temporalio import activity
 from temporalio.common import WorkflowIDConflictPolicy
@@ -90,7 +90,7 @@ async def wrapper(*args, **kwargs):
                 raise ApplicationError(str(error), non_retryable=True) from error
             raise
 
-    return wrapper
+    return cast(Fn, wrapper)
 
 
 # --- comment templates ------------------------------------------------------------------------
@@ -150,6 +150,13 @@ def __init__(
     ):
         self.cfg, self.gh, self.mirror, self.jobs, self.bot, self.client = cfg, gh, mirror, jobs, bot, client
 
+    @property
+    def temporal(self) -> Client:
+        """The Temporal client; only the worker's activities (poll_issues) have one."""
+        if self.client is None:
+            raise RuntimeError("no Temporal client: this activity runs only inside the worker")
+        return self.client
+
     def all(self) -> list:
         """The activities registered on the main task queue (await_agent runs on the -await queue)."""
         return [
@@ -222,7 +229,7 @@ async def prepare_round(self, request: RoundInput) -> Workspace:
         base_sha = await asyncio.to_thread(self.mirror.workspace, round_dir, start_branch, branch, self.cfg.git_url)
         issue = await self.gh.get_issue(request.issue)
         comments = await self.gh.issue_comments(request.issue)
-        logins = {(comment.get("user") or {}).get("login") for comment in comments} - {None}
+        logins = {login for comment in comments if (login := (comment.get("user") or {}).get("login"))}
         writers = {login for login in logins if await self.gh.permission(login) in WRITE_PERMISSIONS}
         comments = [comment for comment in comments if (comment.get("user") or {}).get("login") in writers]
         (round_dir / "prompt.md").write_text(
@@ -298,7 +305,7 @@ async def observe_pr(self, request: ObserveInput) -> PrSnapshot:
         reviews = await self.gh.pr_reviews(request.pr)
         inline_comments = await self.gh.pr_review_comments(request.pr)
         comments = await self.gh.issue_comments(request.pr)
-        logins = {(item.get("user") or {}).get("login") for item in [*reviews, *comments]} - {None}
+        logins = {login for item in [*reviews, *comments] if (login := (item.get("user") or {}).get("login"))}
         # ponytail: one permission lookup per participant per poll; cache if the rate limit ever matters.
         permissions = {login: await self.gh.permission(login) for login in logins}
         marker_prefix = self.cfg.marker_prefix
@@ -372,7 +379,7 @@ async def poll_issues(self) -> list[int]:
                         marker,
                     )
                     continue
-                await self.client.start_workflow(
+                await self.temporal.start_workflow(
                     IssueWorkflow.execute,
                     IssueInput(number, self.cfg.agent.max_revisions, self.cfg.temporal.poll_minutes),
                     id=self.cfg.workflow_id(number),

autodev/auth.py

@@ -19,10 +19,10 @@
 
 import httpx
 from cryptography.hazmat.primitives import hashes, serialization
-from cryptography.hazmat.primitives.asymmetric import padding
+from cryptography.hazmat.primitives.asymmetric import padding, rsa
 
 if TYPE_CHECKING:
-    from .config import Config
+    from .config import Deployment, GitHubAuth
 
 _HEADERS = {"Accept": "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28"}
 
@@ -38,6 +38,8 @@ def app_jwt(app_id: int, pem: bytes, now: float | None = None) -> str:
     header = _b64url(json.dumps({"alg": "RS256", "typ": "JWT"}).encode())
     payload = _b64url(json.dumps({"iat": now - 60, "exp": now + 540, "iss": str(app_id)}).encode())
     key = serialization.load_pem_private_key(pem, password=None)
+    if not isinstance(key, rsa.RSAPrivateKey):
+        raise ValueError("the GitHub App private key must be an RSA key")
     signature = key.sign(f"{header}.{payload}".encode(), padding.PKCS1v15(), hashes.SHA256())
     return f"{header}.{payload}.{_b64url(signature)}"
 
@@ -83,11 +85,18 @@ def __call__(self) -> str:
             return self._token
 
 
-def token_fn(cfg: Config) -> Callable[[], str]:
+def _app_credentials(github: GitHubAuth) -> tuple[int, int, Path]:
+    """(app id, installation id, private key path); config validation guarantees them when auth is "app"."""
+    if github.app_id is None or github.installation_id is None or github.private_key_path is None:
+        raise ValueError("github.auth=app needs github.app_id, installation_id and private_key_path")
+    return github.app_id, github.installation_id, github.private_key_path
+
+
+def token_fn(cfg: Deployment) -> Callable[[], str]:
     """A zero-argument callable that returns the current GitHub token for this config's auth mode."""
     github = cfg.github
     if github.auth == "app":
-        return AppTokens(github.app_id, github.installation_id, github.private_key_path, github.api_url)
+        return AppTokens(*_app_credentials(github), github.api_url)
 
     def from_env_or_file() -> str:
         if token := os.environ.get(github.token_env):
@@ -99,21 +108,22 @@ def from_env_or_file() -> str:
     return from_env_or_file
 
 
-async def bot_login(cfg: Config) -> str:
+async def bot_login(cfg: Deployment) -> str:
     """The login autodev's own comments, commits and PRs appear under ("<app slug>[bot]" for an app)."""
     github = cfg.github
     async with httpx.AsyncClient(base_url=github.api_url, timeout=30, headers=_HEADERS) as http:
         if github.auth == "token":
             response = await http.get("/user", headers={"Authorization": f"Bearer {token_fn(cfg)()}"})
             response.raise_for_status()
             return response.json()["login"]
-        jwt = app_jwt(github.app_id, github.private_key_path.read_bytes())
+        app_id, _, key_path = _app_credentials(github)
+        jwt = app_jwt(app_id, key_path.read_bytes())
         response = await http.get("/app", headers={"Authorization": f"Bearer {jwt}"})
         response.raise_for_status()
         return f"{response.json()['slug']}[bot]"
 
 
-async def noreply_email(cfg: Config, login: str) -> str:
+async def noreply_email(cfg: Deployment, login: str) -> str:
     """The address that links a commit to `login` on GitHub (for an app, the id of its [bot] user, not the app id)."""
     async with httpx.AsyncClient(base_url=cfg.github.api_url, timeout=30, headers=_HEADERS) as http:
         response = await http.get(f"/users/{quote(login)}")

autodev/cli.py

@@ -11,7 +11,7 @@
 import os
 import signal
 import sys
-from collections.abc import Awaitable, Callable
+from collections.abc import Awaitable, Callable, Mapping
 from datetime import timedelta
 from pathlib import Path
 from typing import TYPE_CHECKING, Any
@@ -23,7 +23,7 @@
 if TYPE_CHECKING:
     from temporalio.client import Client
 
-    from .config import Config
+    from .config import Config, Deployment
 
 OPS = {
     "up": ops.cmd_up,
@@ -37,7 +37,7 @@
 log = logging.getLogger("autodev")
 
 
-def _setup_logging(cfg: Config, name: str | None) -> None:
+def _setup_logging(cfg: Deployment, name: str | None) -> None:
     """Log to stderr, and also to state_dir/logs/<name>.log when `name` is given (the worker)."""
     handlers: list[logging.Handler] = [logging.StreamHandler()]
     if name:
@@ -51,13 +51,13 @@ def _setup_logging(cfg: Config, name: str | None) -> None:
     logging.getLogger("httpx").setLevel(logging.WARNING)
 
 
-def proxy_target(environ: dict) -> str | None:
+def proxy_target(environ: Mapping[str, str]) -> str | None:
     """The host:port of $https_proxy, if set."""
     url = environ.get("https_proxy") or environ.get("HTTPS_PROXY")
     return urlsplit(url).netloc if url else None
 
 
-async def _client(cfg: Config) -> Client:
+async def _client(cfg: Deployment) -> Client:
     """Connect to Temporal, through the HTTPS proxy when the config says the sandbox requires it."""
     from temporalio.client import Client
     from temporalio.service import HttpConnectProxyConfig
@@ -188,6 +188,7 @@ async def cmd_worker(cfg: Config, args: argparse.Namespace) -> int:
         client, task_queue=f"{task_queue}-await", activities=[activities.await_agent], max_concurrent_activities=20
     )
     task = asyncio.current_task()
+    assert task is not None  # we are inside the worker coroutine
     asyncio.get_running_loop().add_signal_handler(signal.SIGTERM, task.cancel)
     log.info(
         "worker up as %s: %s ns=%s queues %s, %s-await",

autodev/config.py

@@ -11,7 +11,7 @@
 import re
 import sys
 from pathlib import Path
-from typing import Literal
+from typing import Literal, Protocol
 
 import yaml
 from pydantic import BaseModel, ValidationError, model_validator
@@ -82,6 +82,45 @@ class AgentCfg(BaseModel):
     api_key_env: str | None = None  # bill the API with this env var's key instead of the broker account's login
 
 
+class GitHubAuth(Protocol):
+    """The GitHub auth fields both deployments' configs have (see auth.py)."""
+
+    @property
+    def auth(self) -> str: ...
+    @property
+    def token_env(self) -> str: ...
+    @property
+    def token_file(self) -> Path | None: ...
+    @property
+    def app_id(self) -> int | None: ...
+    @property
+    def installation_id(self) -> int | None: ...
+    @property
+    def private_key_path(self) -> Path | None: ...
+    @property
+    def api_url(self) -> str: ...
+
+
+class StatePaths(Protocol):
+    @property
+    def state_dir(self) -> Path: ...
+
+
+class Deployment(Protocol):
+    """What autodev's shared helpers (auth, the Temporal connection, logging, ops) need from a config.
+
+    Both autodev's and mia-data's Config satisfy it, so mia-data reuses those helpers with its own config."""
+
+    @property
+    def deployment(self) -> str: ...
+    @property
+    def github(self) -> GitHubAuth: ...
+    @property
+    def temporal(self) -> TemporalCfg: ...
+    @property
+    def paths(self) -> StatePaths: ...
+
+
 class Config(BaseModel):
     """The whole autodev.yaml, validated, plus the derived names (labels, branches, markers) built from it."""
 

autodev/github.py

@@ -12,8 +12,8 @@
 import collections
 import logging
 import time
-from collections.abc import AsyncIterator, Awaitable, Callable
-from typing import TYPE_CHECKING, Any
+from collections.abc import AsyncIterator, Awaitable, Callable, Sequence
+from typing import TYPE_CHECKING, Any, Literal, overload
 
 import httpx
 
@@ -108,6 +108,36 @@ def _rate_limit_delay(response: httpx.Response, attempt: int) -> float | None:
             return min(60.0 * 2**attempt, 900.0)  # GitHub: wait ≥1 min, then back off exponentially
         return None
 
+    @overload
+    async def request(
+        self,
+        method: str,
+        path: str,
+        *,
+        json: Any = None,
+        params: dict | None = None,
+        ok404: Literal[False] = False,
+        write: bool | None = None,
+        attempts: int = 8,
+        headers: dict | None = None,
+        conditional: bool = False,
+    ) -> httpx.Response: ...
+
+    @overload
+    async def request(
+        self,
+        method: str,
+        path: str,
+        *,
+        json: Any = None,
+        params: dict | None = None,
+        ok404: Literal[True],
+        write: bool | None = None,
+        attempts: int = 8,
+        headers: dict | None = None,
+        conditional: bool = False,
+    ) -> httpx.Response | None: ...
+
     async def request(
         self,
         method: str,
@@ -224,7 +254,7 @@ async def permission(self, login: str) -> str:
 
     async def label_event(self, issue: int, label: str) -> tuple[str | None, str | None, str | None]:
         """(actor login, event id, created_at) of the most recent `labeled` event for `label`."""
-        found = (None, None, None)
+        found: tuple[str | None, str | None, str | None] = (None, None, None)
         async for event in self._paged(f"/repos/{self.repo}/issues/{issue}/events"):
             if event.get("event") == "labeled" and (event.get("label") or {}).get("name") == label:
                 found = (
@@ -296,7 +326,7 @@ async def pr_review_comments(self, number: int) -> list[dict]:
             comment async for comment in self._paged(f"/repos/{self.repo}/pulls/{number}/comments", conditional=True)
         ]
 
-    async def check_state(self, sha: str, required: list[str] = ()) -> str:
+    async def check_state(self, sha: str, required: Sequence[str] = ()) -> str:
         """Overall CI state of a commit (success | pending | failure | none), combining check runs and statuses."""
         runs = (
             await self.request(

autodev/jobs.py

@@ -14,7 +14,7 @@
 from collections.abc import Callable
 from dataclasses import dataclass, field
 from pathlib import Path
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, Literal, overload
 
 from .models import JobOutcome
 
@@ -41,6 +41,12 @@ class JobSpec:
     env: dict[str, str] = field(default_factory=dict)
 
 
+@overload
+def read_untrusted(
+    job_dir: Path, rel: str, limit: int, tail: bool = ..., binary: Literal[False] = ...
+) -> str | None: ...
+@overload
+def read_untrusted(job_dir: Path, rel: str, limit: int, tail: bool = ..., *, binary: Literal[True]) -> bytes | None: ...
 def read_untrusted(job_dir: Path, rel: str, limit: int, tail: bool = False, binary: bool = False) -> str | bytes | None:
     """Read a file a sandboxed job could have tampered with. Returns None for anything suspicious.
 

autodev/ops.py

@@ -17,12 +17,12 @@
 from .config import PROJECT_DIR
 
 if TYPE_CHECKING:
-    from .config import Config
+    from .config import Deployment
 
 UP_SCRIPT = PROJECT_DIR / "scripts" / "autodev-up.sh"
 
 
-def log_files(cfg: Config) -> dict[str, Path]:
+def log_files(cfg: Deployment) -> dict[str, Path]:
     """Log name -> path. Worker logs are in the state dir; temporal/broker logs are host-only, beside it."""
     state = cfg.paths.state_dir
     host = state.with_name(state.name + "-host") / "logs"
@@ -36,17 +36,17 @@ def _tmux(*args: str, **kwargs) -> subprocess.CompletedProcess:
         sys.exit("tmux not found on PATH")
 
 
-def is_running(cfg: Config) -> bool:
+def is_running(cfg: Deployment) -> bool:
     return _tmux("has-session", "-t", f"={cfg.deployment}", capture_output=True).returncode == 0
 
 
-def cmd_up(cfg: Config, args: argparse.Namespace) -> int:
+def cmd_up(cfg: Deployment, args: argparse.Namespace) -> int:
     script = getattr(args, "up_script", UP_SCRIPT)
     config_env = getattr(args, "config_env", "AUTODEV_CONFIG")
     return subprocess.run([str(script)], env={**os.environ, config_env: str(args.config_path)}).returncode
 
 
-def cmd_down(cfg: Config, args: argparse.Namespace) -> int:
+def cmd_down(cfg: Deployment, args: argparse.Namespace) -> int:
     if not is_running(cfg):
         print(f"{cfg.deployment}: not running")
         return 0
@@ -56,11 +56,11 @@ def cmd_down(cfg: Config, args: argparse.Namespace) -> int:
     return returncode
 
 
-def cmd_restart(cfg: Config, args: argparse.Namespace) -> int:
+def cmd_restart(cfg: Deployment, args: argparse.Namespace) -> int:
     return cmd_down(cfg, args) or cmd_up(cfg, args)
 
 
-def cmd_ps(cfg: Config, args: argparse.Namespace) -> int:
+def cmd_ps(cfg: Deployment, args: argparse.Namespace) -> int:
     """Print whether the session is up, its windows, and where the logs are."""
     if not is_running(cfg):
         print(f"{cfg.deployment}: not running")
@@ -81,13 +81,13 @@ def cmd_ps(cfg: Config, args: argparse.Namespace) -> int:
     return 0
 
 
-def cmd_attach(cfg: Config, args: argparse.Namespace) -> int:
+def cmd_attach(cfg: Deployment, args: argparse.Namespace) -> int:
     if not is_running(cfg):
         sys.exit(f"{cfg.deployment} is not running (run `up` first)")
     return _tmux("attach", "-t", f"={cfg.deployment}").returncode
 
 
-def cmd_logs(cfg: Config, args: argparse.Namespace) -> int:
+def cmd_logs(cfg: Deployment, args: argparse.Namespace) -> int:
     path = log_files(cfg)[args.name]
     if not path.is_file():
         sys.exit(f"no log at {path}")