Code quality: readable names, docs, type hints, ruff, CI

.github/workflows/ci.yml

@@ -0,0 +1,17 @@
+name: CI
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: prefix-dev/setup-pixi@v0.8.10
+        with:
+          cache: true
+      - run: pixi run lint
+      - run: pixi run test

.gitignore

@@ -3,6 +3,6 @@ __pycache__/
 *.egg-info/
 .pytest_cache/
 .env
-autodev.yaml
-autodev-snippets.yaml
+autodev*.yaml
+!autodev.yaml.example
 mia_data.yaml

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 |
+| `tests/` | unit tests (`pixi run test`); no GitHub, LSF or Temporal server needed. `pixi run lint` runs ruff |

autodev/agent.py

@@ -1,18 +1,27 @@
-"""One agent round = one csub job running Claude Code in a sandbox, in a workspace that holds no credentials."""
+"""One agent round: the prompt Claude Code gets, the script that runs it in the sandbox, and how its
+output is read back.
+
+The workspace the job sees holds no credentials. The agent commits locally; the script bundles the
+commits and the worker pushes them (see gitops.py).
+"""
 
 from __future__ import annotations
 
 import asyncio
 import json
 import os
 import shlex
+from collections.abc import Callable
 from pathlib import Path
-from typing import Callable
+from typing import TYPE_CHECKING, Any
 
 from . import jobs
 from .jobs import read_untrusted
 from .models import JobOutcome, JobRef, Workspace, check_key
 
+if TYPE_CHECKING:
+    from .config import Config
+
 PROMPT = """You are implementing GitHub issue #{number} in {repo}. The repository is checked out in the current
 directory on branch `{branch}` at commit {base_sha}.
 
@@ -32,39 +41,68 @@
 """
 
 
-def render_prompt(cfg, issue: dict, comments: list[dict], branch: str, base_sha: str, pr: int | None,
-                  feedback: str | None) -> str:
-    human = [c for c in comments if cfg.marker_prefix not in (c.get("body") or "")]
-    discussion = ("\n# Discussion on the issue\n\n" + "\n\n".join(
-        f"**{(c.get('user') or {}).get('login')}**: {c.get('body')}" for c in human) + "\n") if human else ""
-    revision = (f"\n# Requested changes\n\nPull request #{pr} has your earlier work on this branch. "
-                f"A reviewer asked for:\n\n{feedback}\n") if feedback else ""
-    return PROMPT.format(number=issue["number"], repo=cfg.github.repo, branch=branch, base_sha=base_sha,
-                         title=issue.get("title") or "", body=issue.get("body") or "(no description)",
-                         discussion=discussion, revision=revision,
-                         protected=", ".join(cfg.github.protected_paths) or "(nothing)")
-
-
-def job_script(cfg, base_sha: str) -> str:
-    """Runs inside the csub sandbox with cwd = the round directory."""
-    q = shlex.quote
-    path = f'export PATH={":".join(q(p) for p in cfg.agent.path_prepend)}:"$PATH"\n' if cfg.agent.path_prepend else ""
+def render_prompt(
+    cfg: Config, issue: dict, comments: list[dict], branch: str, base_sha: str, pr: int | None, feedback: str | None
+) -> str:
+    """Fill the prompt template with the issue, the human discussion on it, and any reviewer feedback.
+
+    `issue` and `comments` are GitHub API dicts; the bot's own comments (carrying our marker) are left out."""
+    human = [comment for comment in comments if cfg.marker_prefix not in (comment.get("body") or "")]
+    discussion = (
+        "\n# Discussion on the issue\n\n"
+        + "\n\n".join(f"**{(comment.get('user') or {}).get('login')}**: {comment.get('body')}" for comment in human)
+        + "\n"
+        if human
+        else ""
+    )
+    revision = (
+        (
+            f"\n# Requested changes\n\nPull request #{pr} has your earlier work on this branch. "
+            f"A reviewer asked for:\n\n{feedback}\n"
+        )
+        if feedback
+        else ""
+    )
+    return PROMPT.format(
+        number=issue["number"],
+        repo=cfg.github.repo,
+        branch=branch,
+        base_sha=base_sha,
+        title=issue.get("title") or "",
+        body=issue.get("body") or "(no description)",
+        discussion=discussion,
+        revision=revision,
+        protected=", ".join(cfg.github.protected_paths) or "(nothing)",
+    )
+
+
+def job_script(cfg: Config, base_sha: str) -> str:
+    """The shell script the csub job runs, with cwd = the round directory.
+
+    It runs Claude Code on the prompt, bundles whatever was committed since `base_sha`, and records the
+    exit code for the worker to read."""
+    quote = shlex.quote
+    path = (
+        f'export PATH={":".join(quote(entry) for entry in cfg.agent.path_prepend)}:"$PATH"\n'
+        if cfg.agent.path_prepend
+        else ""
+    )
     budget = f" --max-budget-usd {cfg.agent.max_budget_usd}" if cfg.agent.max_budget_usd is not None else ""
     return f"""set -u
 {path}cd repo || exit 1
-claude -p "$(cat ../prompt.md)" --model {q(cfg.agent.model)} --output-format json{budget} \\
+claude -p "$(cat ../prompt.md)" --model {quote(cfg.agent.model)} --output-format json{budget} \\
   --allowedTools "Bash,Read,Edit,Write,Glob,Grep" > ../result.json 2> ../claude.err
 rc=$?
-git bundle create -q ../result.bundle {q(base_sha)}..HEAD 2>> ../claude.err
+git bundle create -q ../result.bundle {quote(base_sha)}..HEAD 2>> ../claude.err
 echo "$rc" > ../exit_code
 exit "$rc"
 """
 
 
-def read_result(d: Path, limit: int = 20000) -> tuple[str, float | None]:
+def read_result(round_dir: Path, limit: int = 20000) -> tuple[str, float | None]:
     """The agent's summary and cost from result.json. Untrusted: any shape may be there."""
     try:
-        data = json.loads(read_untrusted(d, "result.json", 1 << 20) or "")
+        data = json.loads(read_untrusted(round_dir, "result.json", 1 << 20) or "")
         text, cost = str(data.get("result") or ""), data.get("total_cost_usd")
     except (ValueError, AttributeError, RecursionError):
         return "(the agent left no readable result.json)", None
@@ -73,36 +111,56 @@ def read_result(d: Path, limit: int = 20000) -> tuple[str, float | None]:
     return text or "(empty summary)", cost if isinstance(cost, (int, float)) else None
 
 
-def log_tail(d: Path, n: int = 40) -> str:
-    d = Path(d)
+def log_tail(round_dir: Path, lines: int = 40) -> str:
+    """The last `lines` of claude.err and of the newest few csub job stderr files, for failure reports."""
+    round_dir = Path(round_dir)
     parts = []
-    for rel in ["claude.err", *(str(p.relative_to(d)) for p in sorted((d / ".csub" / "jobs").glob("*/stderr"))[-3:])]:  # newest few: bounds memory
-        if text := (read_untrusted(d, rel, 1 << 16, tail=True) or "").strip():
-            parts.append(f"== {rel} ==\n" + "\n".join(text.splitlines()[-n:]))
+    job_logs = sorted((round_dir / ".csub" / "jobs").glob("*/stderr"))[-3:]  # newest few: bounds memory
+    for rel in ["claude.err", *(str(path.relative_to(round_dir)) for path in job_logs)]:
+        if text := (read_untrusted(round_dir, rel, 1 << 16, tail=True) or "").strip():
+            parts.append(f"== {rel} ==\n" + "\n".join(text.splitlines()[-lines:]))
     return "\n".join(parts)[-5000:]
 
 
-def csub_client(cfg):
+def csub_client(cfg: Config) -> Any:
     return jobs.csub_client(cfg.deployment)
 
 
 class AgentJobs:
-    def __init__(self, cfg, client_factory=None, sleep=asyncio.sleep, poll_s: float = 60.0):
+    """Submits and waits on agent jobs for one deployment, keyed by (issue, key, round)."""
+
+    def __init__(
+        self,
+        cfg: Config,
+        client_factory: Callable[[], Any] | None = None,
+        sleep: Callable[[float], Any] = asyncio.sleep,
+        poll_s: float = 60.0,
+    ):
         self.cfg = cfg
-        self.jobs = jobs.CsubJobs(cfg.deployment, cfg.paths.state_dir / "jobs",
-                                  client_factory or (lambda: csub_client(cfg)), sleep, poll_s)
+        self.jobs = jobs.CsubJobs(
+            cfg.deployment, cfg.paths.state_dir / "jobs", client_factory or (lambda: csub_client(cfg)), sleep, poll_s
+        )
 
-    async def submit(self, issue: int, key: str, rnd: int, ws: Workspace) -> JobRef:
+    async def submit(self, issue: int, key: str, round_number: int, workspace: Workspace) -> JobRef:
         """Submit once per (issue, key, round); a retry returns the same job."""
-        a = self.cfg.agent
-        job_key = f"issue-{issue}-{check_key(key)}-r{rnd}"
-        spec = lambda: jobs.JobSpec(  # noqa: E731  built lazily: env is read only when no id file exists
-            name=f"autodev-{issue}-r{rnd}",  # csub rejects names containing the username (autodev-<you>); session = deployment
-            command=job_script(self.cfg, ws.base_sha), workspace=ws.dir, walltime_min=a.walltime_min, cpus=a.cpus,
-            mem_mb=a.mem_mb, allow_hosts=list(a.allow_hosts), agent="claude",
-            ro_mounts=[m.removesuffix(":ro") for m in a.extra_mounts],  # Config guarantees ":ro"
-            env={"ANTHROPIC_API_KEY": os.environ[a.api_key_env]} if a.api_key_env else {})
-        return JobRef(await self.jobs.submit(job_key, spec), a.walltime_min)
-
-    async def wait(self, job_id: str, ws: Workspace, heartbeat: Callable[[str], None]) -> JobOutcome:
-        return await self.jobs.wait(job_id, ws.dir, heartbeat)
+        agent_cfg = self.cfg.agent
+        job_key = f"issue-{issue}-{check_key(key)}-r{round_number}"
+
+        def build_spec() -> jobs.JobSpec:  # built lazily: the API key env var is read only when no id file exists
+            return jobs.JobSpec(
+                name=f"autodev-{issue}-r{round_number}",  # csub rejects names containing the username; session = deployment
+                command=job_script(self.cfg, workspace.base_sha),
+                workspace=workspace.dir,
+                walltime_min=agent_cfg.walltime_min,
+                cpus=agent_cfg.cpus,
+                mem_mb=agent_cfg.mem_mb,
+                allow_hosts=list(agent_cfg.allow_hosts),
+                agent="claude",
+                ro_mounts=[mount.removesuffix(":ro") for mount in agent_cfg.extra_mounts],  # Config guarantees ":ro"
+                env={"ANTHROPIC_API_KEY": os.environ[agent_cfg.api_key_env]} if agent_cfg.api_key_env else {},
+            )
+
+        return JobRef(await self.jobs.submit(job_key, build_spec), agent_cfg.walltime_min)
+
+    async def wait(self, job_id: str, workspace: Workspace, heartbeat: Callable[[str], None]) -> JobOutcome:
+        return await self.jobs.wait(job_id, workspace.dir, heartbeat)

autodev/auth.py

@@ -1,4 +1,8 @@
-"""Who autodev is on GitHub. token: a personal token (run as yourself). app: a GitHub App installation (service)."""
+"""Who autodev is on GitHub, and how it gets a token.
+
+Two modes: `token` runs as you, with a personal access token from the environment or a file; `app` runs
+as a GitHub App installation, minting short-lived installation tokens from the app's private key.
+"""
 
 from __future__ import annotations
 
@@ -7,38 +11,56 @@
 import os
 import threading
 import time
+from collections.abc import Callable
 from datetime import datetime
 from pathlib import Path
-from typing import Callable
+from typing import TYPE_CHECKING
 from urllib.parse import quote
 
 import httpx
 from cryptography.hazmat.primitives import hashes, serialization
 from cryptography.hazmat.primitives.asymmetric import padding
 
+if TYPE_CHECKING:
+    from .config import Config
+
 _HEADERS = {"Accept": "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28"}
 
 
-def _b64(b: bytes) -> str:
-    return base64.urlsafe_b64encode(b).rstrip(b"=").decode()
+def _b64url(data: bytes) -> str:
+    """Base64url without padding, as JWTs want it."""
+    return base64.urlsafe_b64encode(data).rstrip(b"=").decode()
 
 
 def app_jwt(app_id: int, pem: bytes, now: float | None = None) -> str:
     """RS256 JWT for GitHub App auth: valid 9 minutes, iat backdated 60 s for clock drift."""
     now = int(time.time() if now is None else now)
-    header = _b64(json.dumps({"alg": "RS256", "typ": "JWT"}).encode())
-    payload = _b64(json.dumps({"iat": now - 60, "exp": now + 540, "iss": str(app_id)}).encode())
+    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)
-    sig = key.sign(f"{header}.{payload}".encode(), padding.PKCS1v15(), hashes.SHA256())
-    return f"{header}.{payload}.{_b64(sig)}"
+    signature = key.sign(f"{header}.{payload}".encode(), padding.PKCS1v15(), hashes.SHA256())
+    return f"{header}.{payload}.{_b64url(signature)}"
 
 
 class AppTokens:
-    """Installation tokens, minted on demand and reused until 5 minutes before they expire."""
-
-    def __init__(self, app_id: int, installation_id: int, key_path: Path, api_url: str = "https://api.github.com",
-                 clock: Callable[[], float] = time.time, http: httpx.Client | None = None):
-        self._app_id, self._inst, self._key_path, self._api = app_id, installation_id, Path(key_path), api_url
+    """A callable that returns a valid installation token, minting a new one when the current one is within
+    5 minutes of expiry. Thread-safe, since git runs in worker threads."""
+
+    def __init__(
+        self,
+        app_id: int,
+        installation_id: int,
+        key_path: Path,
+        api_url: str = "https://api.github.com",
+        clock: Callable[[], float] = time.time,
+        http: httpx.Client | None = None,
+    ):
+        self._app_id, self._installation_id, self._key_path, self._api_url = (
+            app_id,
+            installation_id,
+            Path(key_path),
+            api_url,
+        )
         self._clock = clock
         self._http = http or httpx.Client(timeout=30)
         self._token: str | None = None
@@ -50,47 +72,50 @@ def __call__(self) -> str:
             if self._token is None or self._clock() > self._expires - 300:
                 # ponytail: a sync HTTP call about once an hour from inside the event loop; fine at this rate.
                 jwt = app_jwt(self._app_id, self._key_path.read_bytes(), now=self._clock())
-                r = self._http.post(f"{self._api}/app/installations/{self._inst}/access_tokens",
-                                    headers={**_HEADERS, "Authorization": f"Bearer {jwt}"})
-                r.raise_for_status()
-                body = r.json()
+                response = self._http.post(
+                    f"{self._api_url}/app/installations/{self._installation_id}/access_tokens",
+                    headers={**_HEADERS, "Authorization": f"Bearer {jwt}"},
+                )
+                response.raise_for_status()
+                body = response.json()
                 self._token = body["token"]
                 self._expires = datetime.fromisoformat(body["expires_at"]).timestamp()
             return self._token
 
 
-def token_fn(cfg) -> Callable[[], str]:
-    g = cfg.github
-    if g.auth == "app":
-        return AppTokens(g.app_id, g.installation_id, g.private_key_path, g.api_url)
+def token_fn(cfg: Config) -> 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)
 
     def from_env_or_file() -> str:
-        if tok := os.environ.get(g.token_env):
-            return tok
-        if g.token_file and g.token_file.is_file():
-            return g.token_file.read_text().strip()
-        raise RuntimeError(f"no GitHub token: set ${g.token_env} (or put it in .env), or github.token_file")
+        if token := os.environ.get(github.token_env):
+            return token
+        if github.token_file and github.token_file.is_file():
+            return github.token_file.read_text().strip()
+        raise RuntimeError(f"no GitHub token: set ${github.token_env} (or put it in .env), or github.token_file")
 
     return from_env_or_file
 
 
-async def bot_login(cfg) -> str:
-    """The login autodev's own comments, commits and PRs appear under."""
-    g = cfg.github
-    async with httpx.AsyncClient(base_url=g.api_url, timeout=30, headers=_HEADERS) as c:
-        if g.auth == "token":
-            r = await c.get("/user", headers={"Authorization": f"Bearer {token_fn(cfg)()}"})
-            r.raise_for_status()
-            return r.json()["login"]
-        jwt = app_jwt(g.app_id, g.private_key_path.read_bytes())
-        r = await c.get("/app", headers={"Authorization": f"Bearer {jwt}"})
-        r.raise_for_status()
-        return f"{r.json()['slug']}[bot]"
+async def bot_login(cfg: Config) -> 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())
+        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, login: str) -> str:
+async def noreply_email(cfg: Config, 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 c:
-        r = await c.get(f"/users/{quote(login)}")
-        r.raise_for_status()
-        return f"{r.json()['id']}+{login}@users.noreply.github.com"
+    async with httpx.AsyncClient(base_url=cfg.github.api_url, timeout=30, headers=_HEADERS) as http:
+        response = await http.get(f"/users/{quote(login)}")
+        response.raise_for_status()
+        return f"{response.json()['id']}+{login}@users.noreply.github.com"