diff --git a/control-plane/README.md b/control-plane/README.md index 5b69b72..0970b3e 100644 --- a/control-plane/README.md +++ b/control-plane/README.md @@ -188,9 +188,10 @@ instance the worker happened to reach. Fine for one instance (`O10` as currently instance would need Postgres `LISTEN`/`NOTIFY` or a shared queue, which is out of scope here. **Configuration** (`mergit_cp/config.py`). Required: `DATABASE_URL`, `APP_DOMAIN`, `SESSION_SECRET` (32+ -characters), `GATEWAY_URL`, `STORE_URL`, `AGENTS_FILE`, `PLANNER_MODEL`, `EVENTS_TOKEN`, `CHAIN_EVENTS_TOKEN`, -`WORKER_TOKEN`, plus the model provider key. Defaults: `CHAIN_ID` (84532), `PLANS_PER_HOUR` (10), -`SUGGESTIONS_PER_HOUR` (60, exploring suggestions only), `PLANNER_MAX_TASKS` (8), `MAX_ATTEMPTS` (3), `REPO_HOSTS` (github.com), `CORS_ORIGINS`, `SSE_POLL_S` (1), +characters), `GATEWAY_URL`, `STORE_URL`, `AGENTS_FILE`, `PLANNER_MODEL`, `PLANNER_PROVIDER`, `EVENTS_TOKEN`, +`CHAIN_EVENTS_TOKEN`, `WORKER_TOKEN`, plus whichever model key `PLANNER_PROVIDER` needs. Defaults: `CHAIN_ID` +(84532), `PLANS_PER_HOUR` (10), `SUGGESTIONS_PER_HOUR` (60, exploring suggestions only), `PLANNER_MAX_TASKS` (8), +`MAX_ATTEMPTS` (3), `REPO_HOSTS` (github.com), `CORS_ORIGINS`, `SSE_POLL_S` (1), `OUTCOME_POLL_S` (1). `WORKER_TOKEN` is this deployment's own choice, like `CHAIN_EVENTS_TOKEN`: spec/events-v0.md does not name a token for this leg either, since the whole endpoint is new in O7. The planner model must be priced in @@ -261,7 +262,9 @@ depends on it. `NullProgressReporter` (the default) sends nothing, for a worker Typed, from the environment (`mergit_worker/config.py`). Required: `DATABASE_URL`, `GATEWAY_URL`, `STORE_URL`, `EVENTS_TOKEN`, `STORE_WRITE_TOKEN`, `RECEIPT_REGISTRY`, `AGENTS_FILE`, and `MERGIT_AGENT_KEY_` per agent, -plus the model provider key (`ANTHROPIC_API_KEY`). Everything else has a default, including `CHAIN_ID` +plus whichever model key(s) its deployed agents' providers need (`ModelClients.build` only builds a client +for a provider a *deployed* agent actually names, so e.g. `ANTHROPIC_API_KEY` is not required when every +deployed agent is `openai_chat`). Everything else has a default, including `CHAIN_ID` (84532), `SANDBOX_IMAGE`, `SANDBOX_RUNTIME`, `SANDBOX_MEMORY` (2g), `SANDBOX_NANO_CPUS` (2 CPUs), `MAX_STEPS` (60), `MAX_OUTPUT_BYTES` (1 MiB), `COMMAND_TIMEOUT_S`, `JOB_DEADLINE_S`, `MODEL_MAX_TOKENS` (16000; Opus 5.5's thinking tokens count against it, so this is also a per-call ceiling the harness clamps @@ -367,23 +370,53 @@ uv run pytest -m "docker or postgres" # real sandbox and real Postgres ## Not done here Tracked by other plan tasks and issue #12 bullets: egress allowlist, receipt body encryption (K4), data -availability checks, additional agents, evals, failure injection, and hosting (O10, in progress). gVisor is -wired through +availability checks, additional agents, evals, failure injection: audited, see +`docs/failure-injection-audit.md`, and hosting (O10, in progress). gVisor is wired through `SANDBOX_RUNTIME` but has not been exercised in CI. -## Model provider (Anthropic, with Groq as the fallback) +## Model provider -The planner and the agents call one model provider, chosen once at start-up from the environment -(`mergit_worker/providers.py`): +Each agent and the planner name their own model provider; there is no process-wide default and no +fallback. `agents.toml` carries both: -| Environment | Provider | -| --- | --- | -| `ANTHROPIC_API_KEY`, or `ANTHROPIC_AUTH_TOKEN` (with `ANTHROPIC_BASE_URL`, for a gateway such as Claude Lend) set | Anthropic (the models in `agents.toml` and `PLANNER_MODEL`) | -| only `GROQ_API_KEY` set | Groq, through its OpenAI-compatible API; every agent and the planner run `GROQ_MODEL` (default `openai/gpt-oss-120b`) | -| `LLM_PROVIDER=anthropic` or `groq` | forces one | - -The Claude model ids are swapped for the Groq model when an agent is loaded, before anything is priced, so a -receipt names the model that really ran and prices it with the Groq entry in `prices.toml` (list prices from Groq's -own pages). Before a real run, check the credential: `python -m mergit_cp.llm_check` (inside the api -container) makes one plain and one tool-calling request and prints the result, never the key. The worker refuses to start with no key; the API starts and a planning request answers `502`. -There is no per-call fallback: a receipt belongs to one model, so a run never changes model halfway. +```toml +[providers.openrouter] +kind = "openai_chat" +base_url = "https://openrouter.ai/api/v1" +api_key_env = "OPENROUTER_API_KEY" + +[agents.claude-worker] +provider = "openrouter" +model = "anthropic/claude-sonnet-5" +... +``` + +A `[providers.]` table names that provider's implementation: `kind = "anthropic"` talks to the +Anthropic SDK directly and needs nothing else (it falls back to the SDK's own `ANTHROPIC_API_KEY` / +`ANTHROPIC_AUTH_TOKEN`); `kind = "openai_chat"` speaks an OpenAI-compatible chat API (Groq, OpenRouter, or +anything else shaped like one) and needs `base_url` plus `api_key_env`, the environment variable holding +its key. Each `[agents.]` table then names which provider it uses by `provider`, and the planner +names its own by `Settings.planner_provider` (`PLANNER_PROVIDER`) — a deployment choice, same as +`PLANNER_MODEL`, not derived from which keys happen to be set. + +The provider key goes to `base_url` as a Bearer token, and `agents.toml` ships with every push to main, so +`load_providers` refuses a provider that could send a key somewhere new. `base_url` must be https on the +default port to a host in `_PROVIDER_HOSTS` (`api.groq.com`, `openrouter.ai`), or exactly +`http://keyrelay:8787` (the compose-internal key rotator, deploy/server/README.md); a URL with credentials +in it is refused. `api_key_env` must be `_API_KEY` in capitals and not start with `MERGIT_`, so it +cannot name one of Mergit's own secrets (`*_TOKEN`, `*_SECRET`, `*_KEY`, `DATABASE_URL`). An `anthropic` +provider may not set `base_url`. Adding a provider host is a change to `_PROVIDER_HOSTS`, reviewed as code. + +`load_providers` (`mergit_worker/providers.py`) reads every `[providers.*]` table once at start-up; the +worker builds one client per provider actually used by a *deployed* agent (`ModelClients.build`), so an +unused provider's key is never required. A run never changes provider or model halfway: the choice is made +once, at start-up, and a receipt names the model that produced it, priced from the matching entry in +`prices.toml`. + +Before a real run, check the credentials where they live. Each container holds only its own keys, so each +checks its own: `python -m mergit_cp.llm_check planner` in the api container (the planner's provider and +model) and `python -m mergit_cp.llm_check agents` in the worker container (every deployed agent's), or +`all` from a shell that has every key. Each makes one plain and one tool-calling request per distinct +provider and model, printing the result and never the key. The worker refuses to start if a deployed +agent's provider has no key; the API starts and a planning request answers `502`. A deploy checks the keys +are present (not that they work) before it swaps containers: deploy/server/README.md, "Upgrading". diff --git a/control-plane/agents.toml b/control-plane/agents.toml index 66127e2..8adf4f1 100644 --- a/control-plane/agents.toml +++ b/control-plane/agents.toml @@ -1,8 +1,25 @@ -# Model and system prompt per agent, keyed by the `name` in deployments/agents.json. +# Which model provider implementation each agent below names by `provider`. `kind = "anthropic"` uses the +# Anthropic SDK directly; `kind = "openai_chat"` speaks Groq/OpenRouter's OpenAI-compatible chat API, read +# from `base_url` with the key in the environment variable named by `api_key_env`. +[providers.anthropic] +kind = "anthropic" + +[providers.groq] +kind = "openai_chat" +base_url = "https://api.groq.com/openai/v1" +api_key_env = "GROQ_API_KEY" + +[providers.openrouter] +kind = "openai_chat" +base_url = "https://openrouter.ai/api/v1" +api_key_env = "OPENROUTER_API_KEY" + +# Provider, model and system prompt per agent, keyed by the `name` in deployments/agents.json. # Models are provider model ids; the worker reads this file, nothing is hardcoded in code. [agents.claude-worker] -model = "claude-sonnet-5" +provider = "openrouter" +model = "anthropic/claude-sonnet-5" system_prompt = """\ You are a software engineer working in a sandboxed checkout of a repository. Use the tools to inspect files, edit them and run commands. Make the smallest change that @@ -10,16 +27,20 @@ satisfies the task instructions and keep to the paths the task allows. Call `finish` when you believe the work is complete. A separate check decides whether it is.""" [agents.second-worker] -model = "claude-opus-5-5" +provider = "openrouter" +model = "deepseek/deepseek-v4-flash" system_prompt = """\ You are a careful software engineer working in a sandboxed checkout of a repository. Read the relevant code before changing it, run the tests you can, and keep changes minimal. Call `finish` when you believe the work is complete. A separate check decides whether it is.""" -# The agent seeded on Base Sepolia by mergit-contracts' SeedAgents (AGENT_NAME_0). With no Anthropic key the model -# below is replaced by GROQ_MODEL when the agent is loaded (mergit_worker/providers.py). +# The agent seeded on Base Sepolia by mergit-contracts' SeedAgents (AGENT_NAME_0). [agents.mergit-coder-0] -model = "claude-sonnet-5" +# On the hosted deployment this agent runs Claude Sonnet 5 through OpenRouter. To run the same model on +# Anthropic's API directly instead (an ANTHROPIC_API_KEY on the worker), use provider = "anthropic" and +# model = "claude-sonnet-5"; both ids are priced in prices.toml. +provider = "openrouter" +model = "anthropic/claude-sonnet-5" system_prompt = """\ You are a software engineer working in a sandboxed checkout of a repository. Use the tools to inspect files, edit them and run commands. Make the smallest change that diff --git a/control-plane/docs/failure-injection-audit.md b/control-plane/docs/failure-injection-audit.md new file mode 100644 index 0000000..817ec58 --- /dev/null +++ b/control-plane/docs/failure-injection-audit.md @@ -0,0 +1,26 @@ +# Failure-injection audit + +**Source:** HLD decisions doc, "M5 · wk 8-9 · Hardening for production": "Failure-injection suite: +fabricated success, empty diff, wrong file, budget overrun, step tampering, cache replay. Each must +be caught." Plus issue #28's "multi-worker load test with no lost or double-run tasks." + +This records what the code and tests already guarantee for each named scenario. Evidence is cited by +file and test name, not line numbers, so it stays true as the files change: `pytest +'tests/test_harness.py::test_dropped_step_is_rejected'` runs a cited test on its own. Paths without a +`control-plane/` prefix are relative to the repo root (the Rust crates and the `mergit_receipt` type +stub live outside this subtree); all other paths are relative to `control-plane/`. + +| HLD scenario | Status | Evidence | +|---|---|---| +| Fabricated success (agent claims done, nothing actually passed) | **Covered, structurally and by test** | The `finish` tool in `mergit_worker/tools.py` takes only a `summary` string: there is no field an agent could set to claim success, and only Φ decides. `tests/test_harness.py::test_finish_alone_does_not_complete_the_task` drives this through the real pipeline: the agent calls `finish` and `outcome.holds is False`. | +| Wrong file (agent edits outside the task's paths, the check still exits 0) | **Covered** | `tests/test_phi.py::test_change_outside_allowed_paths_fails_even_when_check_passes` writes the allowed fix plus an out-of-scope file; the command exits 0 and `holds is False`. | +| Empty diff passing | **Accepted by design, not a defect** | The pull-request gate in `README.md` (plan task O9) does not treat an empty patch, a task that passed without changing a tracked file, as a blocker, and skips it when applying a plan's combined diff because `git apply` refuses an empty patch. A task can pass Φ without a change (a check that was already satisfied), and failing it would make the gate stricter than the predicate it trusts. | +| Budget overrun | **Covered, including the one gap this audit found** | `tests/test_harness.py::test_budget_exhausted_still_seals_with_phi` shows sealing still happens and partial, useful work still passes Φ. `tests/test_harness.py::test_budget_too_small_for_one_token_stops_before_calling` stops the loop with no model call. Nothing had checked that running out of budget with no completed work ends in a recorded failure rather than a silent pass, so `tests/test_harness.py::test_budget_exhausted_with_no_completed_work_fails` was added: with `budget_usdc_units=1` it asserts no model call, `outcome.notes == ["cost_budget"]` and `outcome.holds is False`. It passed against the unmodified harness, so this is new coverage of an unexercised branch, not a bug fix. | +| Step tampering (a receipt's hash chain or signature altered after sealing) | **Covered in the Rust crates, and from Python** | The vectors in `spec/vectors/` (`bad_signature.json`, `chain_mismatch.json`, `body_hash_mismatch.json`, `commitment_mismatch.json` and others) are consumed by `verifier/tests/vectors.rs` and `spec/tests/attestation.rs`. `mergit_receipt.verify()` is exposed to Python in `runtime/py/mergit_receipt.pyi`. Three tests in `tests/test_harness.py` mutate a copy of a real sealed bundle the way an attacker would and confirm the control plane's own `verify()` call rejects it: `test_tampered_step_output_is_rejected` (one character of a step's `output_digest`), `test_reordered_steps_are_rejected` (two steps swapped) and `test_dropped_step_is_rejected` (a step removed; CLAUDE.md invariant 1, the complete trajectory). Each asserts the status is not `"unattested_valid"` and `reasons` is non-empty, and `test_fake_agent_fixes_repo_and_phi_holds` first shows the untampered bundle verifies. | +| Cache replay (a stale cached tool or model response presented as fresh) | **Not buildable yet: the feature it would guard does not exist** | The only cache fields are `cache_read_tokens` and `cache_write_tokens`, for billing (`mergit_worker/meter.py`, `LLMResult` in `mergit_cp/llm.py`): they count tokens a provider served from its own cache, with no notion of freshness or replay. There is no cache-provenance field on a receipt step, no cache store, and nothing a step could replay. A guard needs a receipt-schema decision first (mergit-research), like TEE attestation. | +| No lost or double-run tasks under concurrent workers (issue #28) | **Covered** | `tests/test_jobs.py::test_concurrent_workers_never_claim_the_same_job` runs 6 worker threads against `SELECT ... FOR UPDATE SKIP LOCKED` on a real Postgres and checks that 30 jobs are each claimed exactly once. | + +**Summary:** five of seven items were already guarded and tested. Empty diff is accepted by design, and +cache replay cannot be built without a schema decision. The one real gap, a budget overrun with no +completed work, is closed by a new test, and step tampering now has Python-side tests as well as the +Rust vector suite. diff --git a/control-plane/mergit_cp/agents.py b/control-plane/mergit_cp/agents.py index be1ff5f..78f203a 100644 --- a/control-plane/mergit_cp/agents.py +++ b/control-plane/mergit_cp/agents.py @@ -7,7 +7,7 @@ from pydantic import BaseModel, ConfigDict, Field -from mergit_worker.providers import effective_model +from mergit_worker.providers import load_providers _ADDRESS = r"^0x[0-9a-fA-F]{40}$" @@ -18,6 +18,7 @@ class Agent: name: str operator: str runtime_key: str + provider: str model: str system_prompt: str @@ -38,6 +39,7 @@ class _Deployment(BaseModel): class _ModelConfig(BaseModel): model_config = ConfigDict(extra="forbid") + provider: str = Field(min_length=1) model: str = Field(min_length=1) system_prompt: str = Field(min_length=1) @@ -46,7 +48,8 @@ def load_agents(path: str, config_path: str = "agents.toml") -> list[Agent]: """Join the deployed agents (``agents.json``) with their model config (``[agents.]`` in ``agents.toml``, the same file the worker reads). - Raises ValueError when an agent has no config or an id appears twice, so a bad deploy fails at start. + Raises ValueError when an agent has no config, names a provider ``agents.toml`` does not define, or + an id appears twice, so a bad deploy fails at start. """ deployed = _Deployment.model_validate(json.loads(Path(path).read_text())).agents tables = tomllib.loads(Path(config_path).read_text()).get("agents") @@ -56,6 +59,7 @@ def load_agents(path: str, config_path: str = "agents.toml") -> list[Agent]: str(name): _ModelConfig.model_validate(table) # pyright: ignore[reportUnknownArgumentType] for name, table in tables.items() # pyright: ignore[reportUnknownVariableType] } + providers = load_providers(Path(config_path)) seen: set[str] = set() agents: list[Agent] = [] for chain_agent in deployed: @@ -65,13 +69,19 @@ def load_agents(path: str, config_path: str = "agents.toml") -> list[Agent]: config = configs.get(chain_agent.name) if config is None: raise ValueError(f"agent {chain_agent.name!r} has no entry in {config_path}") + if config.provider not in providers: + raise ValueError( + f"agent {chain_agent.name!r} names provider {config.provider!r}, which has no " + f"[providers.{config.provider}] table in {config_path}" + ) agents.append( Agent( agent_id=chain_agent.agent_id, name=chain_agent.name, operator=chain_agent.operator.lower(), runtime_key=chain_agent.runtime_key.lower(), - model=effective_model(config.model), + provider=config.provider, + model=config.model, system_prompt=config.system_prompt, ) ) diff --git a/control-plane/mergit_cp/app.py b/control-plane/mergit_cp/app.py index 9232c12..211b447 100644 --- a/control-plane/mergit_cp/app.py +++ b/control-plane/mergit_cp/app.py @@ -2,15 +2,13 @@ hosts build it the same way.""" import asyncio +import logging from collections.abc import AsyncGenerator from contextlib import asynccontextmanager, suppress -from pathlib import Path from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware -from mergit_worker.meter import PriceTable - from . import ( auth, chain_events, @@ -25,18 +23,19 @@ routes_runs, wallet_interest, ) -from .agents import load_agents from .config import Settings from .db import migrate, open_pool from .events import EventDeliverer -from .llm import LLM, build_llm +from .llm import LLM from .outcomes import OutcomeProcessor from .planner import PgCostSink from .pr_store import PgPrStore +from .preflight import check_config, planner_llm from .reputation import GatewayCosts, ReputationUpdater -from .spec import require_schemas from .stream import PlanBroadcaster +log = logging.getLogger(__name__) + def create_app( settings: Settings | None = None, @@ -53,19 +52,19 @@ def create_app( @asynccontextmanager async def lifespan(app: FastAPI) -> AsyncGenerator[None]: - # Fail at start, not on the first request, when configuration is wrong. - app.state.agents = load_agents(settings.agents_file, settings.agents_config) - require_schemas() # the plan checks and events are validated against spec/schemas - prices = PriceTable.load(Path(settings.prices_file)) - for model in settings.model_choices: # every planner model a user may choose must have a price - prices.model_call_cost(model, 0, 0, 0) + # Fail at start, not on the first request, when configuration is wrong. The deploy runs the same + # checks first (mergit_cp/preflight.py), before it swaps the running container. + checked = check_config(settings) + app.state.agents, prices = checked.agents, checked.prices + if checked.stale: + log.warning("no longer read, remove from the env file: %s", ", ".join(checked.stale)) pool = await open_pool(settings.database_url, settings.db_pool_max) async with pool.connection() as conn: await migrate(conn) app.state.pool = pool # The pull-request routes share the worker's queue table; the API holds no GitHub token (see README). app.state.pr_store = PgPrStore.connect(settings.database_url, "api") - app.state.llm = llm or build_llm() + app.state.llm = llm or planner_llm(settings) app.state.costs = PgCostSink(pool, prices) app.state.broadcaster = PlanBroadcaster() app.state.mailer = mailer or (otp.SmtpMailer(settings) if settings.email_enabled else None) diff --git a/control-plane/mergit_cp/config.py b/control-plane/mergit_cp/config.py index 3bc0c8a..7bc1054 100644 --- a/control-plane/mergit_cp/config.py +++ b/control-plane/mergit_cp/config.py @@ -5,8 +5,6 @@ from pydantic import Field, SecretStr, field_validator, model_validator from pydantic_settings import BaseSettings, NoDecode, SettingsConfigDict -from mergit_worker.providers import effective_model - BASE_SEPOLIA_CHAIN_ID = 84532 @@ -33,9 +31,12 @@ class Settings(BaseSettings): planner_model: str """The planner's model (HLD D18 is open, so this is a deployment choice, not a code default).""" + planner_provider: str + """Which `[providers.]` table in `agents_config` the planner uses (HLD D18 is open, so this is a + deployment choice, not a code default).""" planner_models: Annotated[list[str], NoDecode] = [] - """Other planner models a user may choose in their settings (comma-separated). Each must be priced, like - ``planner_model``. Empty: everyone gets ``planner_model``. See ``model_choices``.""" + """Other planner models a user may choose in their settings (comma-separated), all served by + ``planner_provider``. Empty: everyone gets ``planner_model``. See ``model_choices``.""" prices_file: str = "prices.toml" planner_max_tasks: int = Field(default=8, ge=1, le=32) plans_per_hour: int = Field(default=10, ge=1) @@ -142,16 +143,6 @@ class Settings(BaseSettings): """How often GET /v0/plans/{id}/events re-reads plan and task state from the database. A step the worker posts to /v0/internal/progress is relayed to subscribers immediately, not on this timer.""" - @model_validator(mode="after") - def _planner_model_follows_the_provider(self) -> "Settings": - # On Groq the planner runs the Groq model, and the start-up price check and the recorded cost must - # name - # the model that really answers (mergit_worker/providers.py). Unchanged whenever Anthropic is the - # provider. - self.planner_model = effective_model(self.planner_model) - self.planner_models = [effective_model(m) for m in self.planner_models] - return self - @model_validator(mode="before") @classmethod def _blank_is_unset(cls, data: object) -> object: diff --git a/control-plane/mergit_cp/llm.py b/control-plane/mergit_cp/llm.py index 46e2b35..03fc6bc 100644 --- a/control-plane/mergit_cp/llm.py +++ b/control-plane/mergit_cp/llm.py @@ -1,11 +1,10 @@ -"""Model gateway: one small interface the planner and the agents call, with an Anthropic adapter and a Groq +"""Model gateway: one small interface the planner calls, with an Anthropic adapter and an OpenAI-compatible one. -Nothing outside this module imports a provider SDK, so adding a provider is one more class here. The provider -is -chosen once at start-up by `build_llm` (mergit_worker/providers.py): Anthropic when its key is set, Groq when -only -Groq's is. Credentials come from the environment and never pass through a prompt. +Nothing outside this module imports a provider SDK, so adding a provider is one more class here. The planner +names its provider in `Settings.planner_provider`, exactly as an agent names its own in `agents.toml` +(`mergit_worker/providers.py`); `build_planner_llm` resolves that name to a client once at start-up. +Credentials come from the environment and never pass through a prompt. """ import asyncio @@ -20,9 +19,17 @@ from anthropic import APIError, AsyncAnthropic, omit from anthropic.types import Message, MessageParam, ToolParam -from mergit_worker.llm import GROQ_BASE_URL, retry_wait -from mergit_worker.openai_chat import BadReply, parse_reply, to_openai_messages, to_openai_tools -from mergit_worker.providers import has_anthropic_credential, llm_provider +from mergit_worker.config import ConfigError +from mergit_worker.llm import retry_wait +from mergit_worker.openai_chat import ( + BadReply, + body_error, + parse_reply, + scrub, + to_openai_messages, + to_openai_tools, +) +from mergit_worker.providers import ProviderSpec, provider_key @dataclass(frozen=True) @@ -130,28 +137,25 @@ async def _create( _ATTEMPTS = 3 -class GroqLLM: - """Groq's OpenAI-compatible chat API, the fallback when there is no Anthropic key (same translation as the - worker's - `GroqLLM`, in `mergit_worker/openai_chat.py`). A structured answer is requested as best-effort JSON-schema - output - (`strict: false`, which every Groq model supports) because the planner validates every answer itself in - `check_tasks`, exactly as it does for Anthropic.""" +class OpenAIChatLLM: + """Any OpenAI-compatible chat API (Groq, OpenRouter, ...), async for the planner (the worker's sync twin + is `mergit_worker.llm.OpenAIChatLLM`). A structured answer is requested as best-effort JSON-schema output + (`strict: false`, which every such provider accepts) because the planner validates every answer itself + in `check_tasks`, exactly as it does for Anthropic. The key is only ever sent in the `Authorization` + header: errors never include it.""" def __init__( self, - api_key: str | None = None, + api_key: str, *, + base_url: str, client: httpx.AsyncClient | None = None, timeout_s: float = 120.0, - base_url: str = GROQ_BASE_URL, sleep: Callable[[float], Any] = asyncio.sleep, max_tokens: int = 16000, ) -> None: - key = api_key or os.environ.get("GROQ_API_KEY") - if not key: - raise LLMUnavailable("GROQ_API_KEY is not set") - self._headers = {"Authorization": f"Bearer {key}"} + self._key = api_key + self._headers = {"Authorization": f"Bearer {api_key}"} self._url = f"{base_url.rstrip('/')}/chat/completions" self._http = client or httpx.AsyncClient(timeout=timeout_s) self._sleep = sleep @@ -173,11 +177,18 @@ async def _post(self, payload: dict[str, Any]) -> dict[str, Any]: continue if response.status_code == 200: try: - return cast("dict[str, Any]", response.json()) + data: object = response.json() except ValueError as exc: raise LLMUnavailable("the answer is not JSON") from exc + problem = body_error(data) # shared with the worker's client (mergit_worker/openai_chat.py) + if problem is None: + return cast("dict[str, Any]", data) + if not problem.retryable or attempt == _ATTEMPTS: + raise LLMUnavailable(scrub(problem.message, self._key)[:300]) + await self._wait(float(attempt)) + continue if response.status_code not in _RETRY_STATUSES or attempt == _ATTEMPTS: - raise LLMUnavailable(f"HTTP {response.status_code}: {response.text[:200]}") + raise LLMUnavailable(f"HTTP {response.status_code}: {scrub(response.text, self._key)[:200]}") await self._wait(retry_wait(response, attempt)) raise AssertionError("unreachable") # pragma: no cover @@ -222,19 +233,65 @@ async def complete( return result -def build_llm(environ: Mapping[str, str] | None = None) -> LLM: - """The planner's model client: Anthropic when its key is set, Groq when only Groq's is. +class _UnavailableLLM: + """Stands in for a provider with no key configured, so start-up never fails for it: a planning request + then fails clearly with `LLMUnavailable`, which the API turns into a 502.""" + + def __init__(self, name: str) -> None: + self._name = name + + async def complete( + self, + messages: list[dict[str, Any]], + tools: list[dict[str, Any]] | None, + model: str, + *, + system: str | None = None, + output_schema: dict[str, Any] | None = None, + ) -> LLMResult: + # Named generically, not assumed to always be the planner's own: `build_planner_llm` is also called + # by `mergit_cp.llm_check` to resolve an agent's provider, not only `Settings.planner_provider`. + raise LLMUnavailable(f"no model key is configured for provider {self._name!r}") + + +def planner_unavailable(llm: LLM) -> bool: + """True for the stand-in `build_planner_llm` returns when its provider has no key.""" + return isinstance(llm, _UnavailableLLM) + + +def build_planner_llm( + providers: dict[str, ProviderSpec], + name: str, + timeout_s: float = 120.0, + environ: Mapping[str, str] | None = None, + max_tokens: int = 16000, +) -> LLM: + """The planner's model client for its configured provider, named in `Settings.planner_provider` exactly + as an agent names its own in `agents.toml`. - With no key at all the API still starts, as it always has, so wallet sign-in, credits and the agent list + With no key for it, the API still starts, as it always has, so wallet sign-in, credits and the agent list work; a planning request then fails with 502. (The worker, which can do nothing without a model, refuses to start.)""" env = os.environ if environ is None else environ - provider = llm_provider(env) - if provider == "groq" and env.get("GROQ_API_KEY"): - return GroqLLM(env["GROQ_API_KEY"]) - if not has_anthropic_credential(env): + try: + spec = providers[name] + except KeyError: + raise ConfigError( + f"provider {name!r} has no [providers.{name}] table; valid providers are " + f"{', '.join(sorted(providers.keys())) or '(none configured)'}" + ) from None + key = provider_key(spec, env) + if not key: + env_var = spec.api_key_env or "ANTHROPIC_API_KEY or ANTHROPIC_AUTH_TOKEN" log.warning( - "no model key set (ANTHROPIC_API_KEY, ANTHROPIC_AUTH_TOKEN or GROQ_API_KEY): planning requests " - "will fail until one is" + "no model key set for provider %r (%s): planning requests will fail until one is", name, env_var ) - return AnthropicLLM() + return _UnavailableLLM(name) + if spec.kind == "anthropic": + # With its own `api_key_env`, this provider's key may not be the SDK's own env-var default (another + # `[providers.*]` table could be anthropic too, with a different key); pass it explicitly so the + # right one is actually used, not just checked for presence above. + client = AsyncAnthropic(api_key=key) if spec.api_key_env else None + return AnthropicLLM(client=client, max_tokens=max_tokens) + assert spec.base_url is not None # load_providers requires base_url for kind == "openai_chat" + return OpenAIChatLLM(key, base_url=spec.base_url, timeout_s=timeout_s, max_tokens=max_tokens) diff --git a/control-plane/mergit_cp/llm_check.py b/control-plane/mergit_cp/llm_check.py index b66744c..54d5dc1 100644 --- a/control-plane/mergit_cp/llm_check.py +++ b/control-plane/mergit_cp/llm_check.py @@ -1,22 +1,35 @@ -"""Checks the model credential the way the planner and the agents use it, before a real run depends on it. +"""Checks the model credentials the way the planner and the agents use them, before a real run depends on +them. Each container holds only its own keys, so each checks only its own: - python -m mergit_cp.llm_check [model] (run inside the api container, or with the same env) + python -m mergit_cp.llm_check planner in the api container: the planner's provider and model + python -m mergit_cp.llm_check agents in the worker container: every deployed agent's + python -m mergit_cp.llm_check all both, from a shell that holds every key -Makes two tiny calls with the configured provider (``ANTHROPIC_API_KEY``, or ``ANTHROPIC_AUTH_TOKEN`` with -``ANTHROPIC_BASE_URL`` for a gateway such as Claude Lend, or Groq): a plain answer, then one that must call a -tool, because the agents work by tool calls. Prints the provider, the model, whether each call worked and its -token counts; never a key or a token. Exit status 0 only if both worked. +``planner`` is the default. Run in the api container, ``agents`` would report every agent key as missing, +because ``api.env`` rightly does not have them. + +For each distinct (provider, model) pair actually in use, makes two tiny calls (a plain answer, then one +that must call a tool, because the agents work by tool calls): the planner's own provider and model, plus +every deployed agent's provider and model. Two agents sharing a provider but naming different models (the +shipped `claude-worker` and `second-worker`, both `openrouter` but one `anthropic/claude-sonnet-5` and the +other `deepseek/deepseek-v4-flash`) each get their own check; two agents naming the identical pair are +checked once, not twice. Prints the provider, the endpoint, the model, whether each call worked and its +token counts; never a key or a token. Exit status 0 only if every check worked. """ import asyncio import os import sys +from pathlib import Path from typing import Any from urllib.parse import urlsplit -from mergit_worker.providers import effective_model, llm_provider +from mergit_worker.agents import deployed_agents +from mergit_worker.config import load_config +from mergit_worker.providers import ProviderSpec, load_providers -from .llm import LLMError, LLMUnavailable, build_llm +from .config import Settings +from .llm import LLMError, LLMUnavailable, build_planner_llm _TOOL = { "name": "report", @@ -29,15 +42,46 @@ } -def _endpoint() -> str: - if llm_provider() == "groq": - return urlsplit(os.environ.get("GROQ_BASE_URL") or "https://api.groq.com").hostname or "groq" +# A probe needs a word or one tool call; reasoning models spend some tokens before either. The planner's own +# 16000 is a cap, not usage, but OpenRouter checks the account's credit against it, so a low balance would +# fail a probe it would pass for real. +_PROBE_MAX_TOKENS = 1024 + + +def _endpoint(spec: ProviderSpec) -> str: + if spec.kind == "openai_chat": + return urlsplit(spec.base_url or "").hostname or spec.base_url or spec.name return urlsplit(os.environ.get("ANTHROPIC_BASE_URL", "")).hostname or "api.anthropic.com" -async def check(model: str) -> bool: - llm = build_llm() - print(f"provider={llm_provider()} endpoint={_endpoint()} model={model}") +def _agent_pairs_in_use(agents_file: Path, agents_config: Path) -> set[tuple[str, str]]: + """Every distinct (provider, model) pair actually used by a deployed agent, read the same way + `mergit_worker.agents.validate_agents_have_keys` reads the deploy.""" + pairs: set[tuple[str, str]] = set() + for _entry, cfg in deployed_agents(agents_file, agents_config): + provider = cfg.get("provider") if cfg else None + model = cfg.get("model") if cfg else None + if provider and model: # a missing provider/model here is load_agent_spec's own clear error to raise + pairs.add((str(provider), str(model))) + return pairs + + +def _providers_in_use(settings: Settings) -> set[tuple[str, str]]: + """Every distinct (provider, model) pair actually in use: the planner's own, plus every deployed + agent's. Two agents sharing a provider but naming different models each keep their own pair; two naming + the identical pair collapse to one (it's a `set`).""" + pairs = {(settings.planner_provider, settings.planner_model)} + pairs |= _agent_pairs_in_use(Path(settings.agents_file), Path(settings.agents_config)) + return pairs + + +async def _check_one(providers: dict[str, ProviderSpec], name: str, model: str) -> bool: + spec = providers.get(name) + if spec is None: + print(f"provider={name} model={model} FAILED no [providers.{name}] table in agents.toml") + return False + llm = build_planner_llm(providers, name, max_tokens=_PROBE_MAX_TOKENS) + print(f"provider={name} endpoint={_endpoint(spec)} model={model}") ok = True steps: list[tuple[str, list[dict[str, Any]], list[dict[str, Any]] | None]] = [ ("plain answer", [{"role": "user", "content": "Reply with the single word: ready"}], None), @@ -52,26 +96,62 @@ async def check(model: str) -> bool: [_TOOL], ), ] - for name, messages, tools in steps: + for step_name, messages, tools in steps: try: result = await llm.complete(messages, tools, model) except (LLMUnavailable, LLMError) as problem: - print(f" {name}: FAILED {str(problem)[:300]}") + print(f" {step_name}: FAILED {str(problem)[:300]}") ok = False continue worked = bool(result.tool_calls) if tools else bool(result.text.strip()) ok = ok and worked print( - f" {name}: {'ok' if worked else 'FAILED (the reply did not do what was asked)'}" + f" {step_name}: {'ok' if worked else 'FAILED (the reply did not do what was asked)'}" f" in={result.input_tokens} out={result.output_tokens} served_by={result.model}" ) return ok +async def _check_pairs(agents_config: Path, pairs: set[tuple[str, str]]) -> bool: + providers = load_providers(agents_config) + ok = True + for name, model in sorted(pairs): + ok = await _check_one(providers, name, model) and ok + return ok + + +async def check(settings: Settings) -> bool: + """The planner's pair and every deployed agent's: `all`, for a shell that holds every key.""" + return await _check_pairs(Path(settings.agents_config), _providers_in_use(settings)) + + +async def check_planner(settings: Settings) -> bool: + """The planner's pair only, with the api container's own keys.""" + pairs = {(settings.planner_provider, settings.planner_model)} + return await _check_pairs(Path(settings.agents_config), pairs) + + +async def check_agents(agents_file: Path, agents_config: Path) -> bool: + """Every deployed agent's pair, with the worker container's own keys.""" + return await _check_pairs(agents_config, _agent_pairs_in_use(agents_file, agents_config)) + + +_USAGE = "usage: python -m mergit_cp.llm_check [planner|agents|all]" + + def main(argv: list[str] | None = None) -> int: args = sys.argv[1:] if argv is None else argv - model = effective_model(args[0] if args else os.environ.get("PLANNER_MODEL", "claude-haiku-4-5")) - return 0 if asyncio.run(check(model)) else 1 + scope = args[0] if args else "planner" + if len(args) > 1 or scope not in ("planner", "agents", "all"): + print(_USAGE, file=sys.stderr) + return 2 + if scope == "agents": + cfg = load_config() # the worker's own settings: the api's are not in the worker container + ok = asyncio.run(check_agents(cfg.agents_file, cfg.agents_config)) + else: + settings = Settings() # type: ignore[call-arg] # read from the environment, as the API does + ok = asyncio.run(check_planner(settings) if scope == "planner" else check(settings)) + return 0 if ok else 1 if __name__ == "__main__": diff --git a/control-plane/mergit_cp/preflight.py b/control-plane/mergit_cp/preflight.py new file mode 100644 index 0000000..aba866c --- /dev/null +++ b/control-plane/mergit_cp/preflight.py @@ -0,0 +1,87 @@ +"""`python -m mergit_cp.preflight`: the API's start-up configuration checks, and nothing else. + +deploy/server/update.sh runs this in the freshly built image, with the real env file, before it swaps the +running container. A configuration the new code cannot start from (a missing setting such as +`PLANNER_PROVIDER`, an unknown planner provider, an unpriced planner model) then fails the deploy while the +old API keeps running, instead of leaving a new one restart-looping. It touches no database and makes no +network call. + +`create_app`'s start-up calls `check_config` too, so the deploy checks exactly what start-up checks. The +preflight is stricter in one way: a planner with no model key fails it. The API itself still starts without +one (sign-in, credits and the agent list work and planning answers 502), but a deploy should not ship that +by accident. +""" + +from __future__ import annotations + +import os +import sys +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path + +from pydantic import ValidationError + +from mergit_worker.config import ConfigError, describe_invalid_settings +from mergit_worker.meter import PriceTable +from mergit_worker.providers import check_model_id, load_providers, stale_settings + +from .agents import Agent, load_agents +from .config import Settings +from .llm import LLM, build_planner_llm, planner_unavailable +from .spec import require_schemas + + +@dataclass(frozen=True) +class Checked: + agents: list[Agent] + prices: PriceTable + stale: list[str] + """Removed settings still set in the environment, to warn about (`providers.REMOVED_SETTINGS`).""" + + +def check_config(settings: Settings, environ: Mapping[str, str] | None = None) -> Checked: + """The agents load, the schemas are present, every planner model a user may choose is priced, and no + removed setting still steers the planner's key somewhere it should not go.""" + env = os.environ if environ is None else environ + agents = load_agents(settings.agents_file, settings.agents_config) + require_schemas() # the plan checks and events are validated against spec/schemas + prices = PriceTable.load(Path(settings.prices_file)) + for model in settings.model_choices: # every planner model a user may choose must have a price + prices.model_call_cost(model, 0, 0, 0) + providers = load_providers(Path(settings.agents_config)) + planner = providers.get(settings.planner_provider) + if planner is not None: # an undefined provider is build_planner_llm's own clear error + for model in settings.model_choices: + check_model_id(planner, model) + stale = stale_settings(providers, {settings.planner_provider}, env) + return Checked(agents, prices, stale) + + +def planner_llm(settings: Settings, environ: Mapping[str, str] | None = None) -> LLM: + """The planner's client. Raises `ConfigError` for a provider that `agents.toml` does not define.""" + return build_planner_llm( + load_providers(Path(settings.agents_config)), settings.planner_provider, environ=environ + ) + + +def main() -> int: + try: + try: + settings = Settings() # pyright: ignore[reportCallIssue] # fields come from the environment + except ValidationError as exc: + raise ConfigError(describe_invalid_settings(exc)) from None + checked = check_config(settings) + if planner_unavailable(planner_llm(settings)): + raise ConfigError(f"the planner's provider {settings.planner_provider!r} has no model key") + except (ConfigError, ValueError, OSError) as exc: + print(f"api preflight failed: {' '.join(str(exc).split())}", file=sys.stderr) + return 1 + if checked.stale: + print(f"api preflight warning: no longer read, remove from the env file: {', '.join(checked.stale)}") + print(f"api preflight ok: planner {settings.planner_provider} / {settings.planner_model}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/control-plane/mergit_worker/__main__.py b/control-plane/mergit_worker/__main__.py index c471ead..8b95ecd 100644 --- a/control-plane/mergit_worker/__main__.py +++ b/control-plane/mergit_worker/__main__.py @@ -10,17 +10,14 @@ from mergit_cp.pr_store import PgPrStore -from .agents import validate_agents_are_priced from .config import ConfigError, WorkerConfig, load_config from .github import GitHubConfig, GitHubPrOpener from .harness import HarnessConfig from .jobs import PgJobStore -from .llm import build_llm -from .meter import PriceTable from .models import Job from .pr_worker import PrHandler +from .preflight import check_models from .progress import HttpProgressReporter, NullProgressReporter, ProgressReporter -from .providers import llm_provider from .publish import ReceiptPublisher from .sandbox import Sandbox, SandboxConfig, reap_stale from .tools import ToolLimits @@ -46,6 +43,8 @@ class Built: worker: Worker jobs: PgJobStore prs: PgPrStore | None + providers: set[str] + stale: list[str] def close(self) -> None: self.jobs.close() @@ -91,13 +90,13 @@ def build_worker(cfg: WorkerConfig, worker_id: str) -> Built: def make_sandbox(job: Job) -> Sandbox: return Sandbox.create(job.repo_url, job.ref, sandbox_cfg) - prices = PriceTable.load(cfg.prices_file) - validate_agents_are_priced(cfg.agents_file, cfg.agents_config, prices) + checked = check_models(cfg) + prices, models, needed = checked.prices, checked.clients, checked.providers store = PgJobStore.connect(cfg.database_url.get_secret_value(), worker_id) pr = build_pr_handler(cfg, worker_id) deps = WorkerDeps( store=store, - llm=build_llm(cfg.http_timeout_s * 20), + models=models, prices=prices, publisher=ReceiptPublisher( store_url=cfg.store_url, @@ -128,7 +127,7 @@ def make_sandbox(job: Job) -> Sandbox: ), ), ) - return Built(Worker(cfg, deps), store, pr[1] if pr else None) + return Built(Worker(cfg, deps), store, pr[1] if pr else None, needed, checked.stale) def main() -> int: @@ -141,8 +140,10 @@ def main() -> int: return 2 worker_id = cfg.worker_id or socket.gethostname() - log.info("model provider: %s", llm_provider()) built = build_worker(cfg, worker_id) + log.info("model providers: %s", ", ".join(sorted(built.providers)) or "(no deployed agent needs one)") + if built.stale: + log.warning("no longer read, remove from the env file: %s", ", ".join(built.stale)) log.info("failed %d jobs this worker left running before a restart", built.jobs.fail_own_running()) log.info( "reclaimed %d jobs left running past their lease by any worker", diff --git a/control-plane/mergit_worker/agents.py b/control-plane/mergit_worker/agents.py index cee8cc4..42e1b6c 100644 --- a/control-plane/mergit_worker/agents.py +++ b/control-plane/mergit_worker/agents.py @@ -8,57 +8,102 @@ import tomllib from collections.abc import Mapping from pathlib import Path -from typing import Any +from typing import Any, cast from .config import ConfigError, agent_signing_key from .harness import AgentSpec from .meter import PriceTable -from .providers import effective_model +from .providers import load_providers _ADDRESS = re.compile(r"^0x[0-9a-fA-F]{40}$") -def load_agent_spec( - agent_id: str, agents_file: Path, agents_config: Path, environ: Mapping[str, str] | None = None -) -> AgentSpec: +def deployed_agents( + agents_file: Path, agents_config: Path +) -> list[tuple[dict[str, Any], dict[str, Any] | None]]: + """Each agent in `agents_file` with its `[agents.]` table from `agents_config` (None when it has + none), the one way every check here reads the deploy. Both files are checked for shape on the way: a + registry entry or an agent table that is not a table is a `ConfigError` naming it, never a bare + AttributeError from calling `.get` on a string.""" try: - registry: list[dict[str, Any]] = json.loads(agents_file.read_text())["agents"] - models: dict[str, Any] = tomllib.loads(agents_config.read_text())["agents"] - except (OSError, KeyError, ValueError) as exc: + registry: object = json.loads(agents_file.read_text())["agents"] + models: object = tomllib.loads(agents_config.read_text())["agents"] + except (OSError, KeyError, TypeError, ValueError) as exc: raise ConfigError(f"cannot read agent configuration: {exc}") from exc + if not isinstance(registry, list): + raise ConfigError(f"{agents_file}: `agents` must be a list") + if not isinstance(models, dict): + raise ConfigError(f"{agents_config}: [agents] must be a table") + tables = cast("dict[str, object]", models) + pairs: list[tuple[dict[str, Any], dict[str, Any] | None]] = [] + for entry in cast("list[object]", registry): + if not isinstance(entry, dict): + raise ConfigError(f"{agents_file}: every entry in `agents` must be an object, got {entry!r}") + entry = cast("dict[str, Any]", entry) + name = str(entry.get("name")) + cfg = tables.get(name) + if cfg is not None and not isinstance(cfg, dict): + raise ConfigError(f"agent {name!r} must be an [agents.{name}] table in {agents_config}") + pairs.append((entry, cast("dict[str, Any] | None", cfg))) + return pairs - entry = next((a for a in registry if str(a.get("agent_id")) == agent_id), None) - if entry is None: + +def load_agent_spec( + agent_id: str, agents_file: Path, agents_config: Path, environ: Mapping[str, str] | None = None +) -> AgentSpec: + found = next( + ( + (e, c) + for e, c in deployed_agents(agents_file, agents_config) + if str(e.get("agent_id")) == agent_id + ), + None, + ) + if found is None: raise ConfigError(f"agent {agent_id!r} is not in {agents_file}") + entry, cfg = found runtime_key = str(entry.get("runtime_key", "")) if not _ADDRESS.match(runtime_key): raise ConfigError(f"agent {agent_id!r} has no valid runtime_key address") - cfg: dict[str, Any] | None = models.get(str(entry.get("name"))) - if not cfg or not cfg.get("model") or not cfg.get("system_prompt"): - raise ConfigError(f"agent {entry.get('name')!r} needs a model and system_prompt in {agents_config}") + name = str(entry.get("name")) + if not cfg or not cfg.get("model") or not cfg.get("system_prompt") or not cfg.get("provider"): + raise ConfigError(f"agent {name!r} needs a provider, model and system_prompt in {agents_config}") + provider = str(cfg["provider"]) + if provider not in load_providers(agents_config): + raise ConfigError( + f"agent {name!r} names provider {provider!r}, which has no [providers.{provider}] " + f"table in {agents_config}" + ) return AgentSpec( agent_id=agent_id, - model=effective_model(str(cfg["model"]), environ), + provider=provider, + model=str(cfg["model"]), system_prompt=str(cfg["system_prompt"]), runtime_key_address=runtime_key, signing_key=agent_signing_key(agent_id, environ).get_secret_value(), ) -def validate_agents_are_priced( - agents_file: Path, agents_config: Path, prices: PriceTable, environ: Mapping[str, str] | None = None -) -> None: +def validate_agents_have_keys( + agents_file: Path, agents_config: Path, environ: Mapping[str, str] | None = None +) -> set[str]: + """Every provider named by a deployed agent, read exactly as `validate_agents_are_priced` reads the + deploy. This does not itself check that a key is present for each one: `ModelClients.build` does that, + against the set this returns, so the two stay in lockstep by construction.""" + needed: set[str] = set() + for _entry, cfg in deployed_agents(agents_file, agents_config): + provider = cfg.get("provider") if cfg else None + if provider: # a missing provider here is load_agent_spec's own clear error to raise + needed.add(str(provider)) + return needed + + +def validate_agents_are_priced(agents_file: Path, agents_config: Path, prices: PriceTable) -> None: """Every agent with a model in `agents_config` must have that model in the price table, checked once at start-up. Otherwise a bad deploy is discovered only after a job has already paid the provider for a call whose receipt could never carry a real cost (`PriceTable.price` would raise mid-run).""" - try: - registry: list[dict[str, Any]] = json.loads(agents_file.read_text())["agents"] - models: dict[str, Any] = tomllib.loads(agents_config.read_text())["agents"] - except (OSError, KeyError, ValueError) as exc: - raise ConfigError(f"cannot read agent configuration: {exc}") from exc - for entry in registry: - cfg: dict[str, Any] | None = models.get(str(entry.get("name"))) + for _entry, cfg in deployed_agents(agents_file, agents_config): model = cfg.get("model") if cfg else None if model: # a missing model/system_prompt here is load_agent_spec's own clear error to raise - prices.price(effective_model(str(model), environ)) + prices.price(str(model)) diff --git a/control-plane/mergit_worker/config.py b/control-plane/mergit_worker/config.py index 8380e11..37a02ca 100644 --- a/control-plane/mergit_worker/config.py +++ b/control-plane/mergit_worker/config.py @@ -7,7 +7,7 @@ from collections.abc import Mapping from pathlib import Path -from pydantic import Field, SecretStr, field_validator +from pydantic import Field, SecretStr, ValidationError, field_validator from pydantic_settings import BaseSettings, SettingsConfigDict _ADDRESS = re.compile(r"^0x[0-9a-fA-F]{40}$") @@ -18,6 +18,16 @@ class ConfigError(Exception): """Configuration is missing or malformed. The worker refuses to start.""" +def describe_invalid_settings(exc: ValidationError) -> str: + """Which settings are missing or malformed, by name and reason only. Pydantic's own message carries the + input values, and the input is the environment: secrets, partly shown, in a log (the deploy preflight's + goes to a public GitHub Actions log).""" + return "; ".join( + f"{'.'.join(str(p) for p in e['loc']).upper() or 'settings'}: {e['msg']}" + for e in exc.errors(include_url=False, include_input=False, include_context=False) + ) + + class WorkerConfig(BaseSettings): model_config = SettingsConfigDict(extra="ignore", case_sensitive=False) @@ -97,6 +107,8 @@ def _url_without_trailing_slash(cls, v: str) -> str: def load_config() -> WorkerConfig: try: return WorkerConfig() # type: ignore[call-arg] # fields come from the environment + except ValidationError as exc: + raise ConfigError(describe_invalid_settings(exc)) from None except ValueError as exc: raise ConfigError(str(exc)) from exc diff --git a/control-plane/mergit_worker/harness.py b/control-plane/mergit_worker/harness.py index 209e2eb..853d985 100644 --- a/control-plane/mergit_worker/harness.py +++ b/control-plane/mergit_worker/harness.py @@ -49,6 +49,7 @@ def publish(self, sealed: mergit_receipt.SealedBundle) -> None: ... @dataclass(frozen=True, slots=True) class AgentSpec: agent_id: str + provider: str model: str system_prompt: str runtime_key_address: str # the registered runtime key; the header's attestation.dev_key diff --git a/control-plane/mergit_worker/llm.py b/control-plane/mergit_worker/llm.py index aac45e4..6345a83 100644 --- a/control-plane/mergit_worker/llm.py +++ b/control-plane/mergit_worker/llm.py @@ -13,8 +13,8 @@ import httpx from .config import ConfigError -from .openai_chat import BadReply, parse_reply, to_openai_messages, to_openai_tools -from .providers import require_provider +from .openai_chat import BadReply, body_error, parse_reply, scrub, to_openai_messages, to_openai_tools +from .providers import ProviderSpec, provider_key class LLMError(Exception): @@ -96,7 +96,6 @@ def complete( ) -GROQ_BASE_URL = "https://api.groq.com/openai/v1" _RETRY_STATUSES = frozenset({429, 500, 502, 503, 504}) _ATTEMPTS = 3 @@ -110,12 +109,10 @@ def retry_wait(response: httpx.Response, attempt: int) -> float: return float(attempt) -class GroqLLM: - """Groq's OpenAI-compatible chat API, the fallback when there is no Anthropic key. It takes and returns - the same +class OpenAIChatLLM: + """Any OpenAI-compatible chat API (Groq, OpenRouter, ...). It takes and returns the same Anthropic-shaped data as `AnthropicLLM`, so the harness and the receipts do not know which one answered. - The key - is read from `GROQ_API_KEY` and is only ever in the `Authorization` header: errors never include it. + The key is only ever sent in the `Authorization` header: errors never include it. Cached prompt tokens are counted as ordinary input here, because the worker's meter has no cached-token field: @@ -123,17 +120,15 @@ class GroqLLM: def __init__( self, - api_key: str | None = None, + api_key: str, *, + base_url: str, client: httpx.Client | None = None, timeout_s: float = 600.0, - base_url: str = GROQ_BASE_URL, sleep: Callable[[float], None] = time.sleep, ) -> None: - key = api_key or os.environ.get("GROQ_API_KEY") - if not key: - raise ConfigError("GROQ_API_KEY is not set") - self._headers = {"Authorization": f"Bearer {key}"} + self._key = api_key + self._headers = {"Authorization": f"Bearer {api_key}"} self._url = f"{base_url.rstrip('/')}/chat/completions" self._http = client or httpx.Client(timeout=timeout_s) self._sleep = sleep @@ -149,11 +144,19 @@ def _post(self, payload: dict[str, Any]) -> dict[str, Any]: continue if response.status_code == 200: try: - return cast("dict[str, Any]", response.json()) + data: object = response.json() except ValueError as exc: raise LLMError("model call failed: the answer is not JSON") from exc + problem = body_error(data) # shared with the planner's client (mergit_cp/llm.py) + if problem is None: + return cast("dict[str, Any]", data) + if not problem.retryable or attempt == _ATTEMPTS: + raise LLMError(f"model call failed: {scrub(problem.message, self._key)[:300]}") + self._sleep(float(attempt)) + continue if response.status_code not in _RETRY_STATUSES or attempt == _ATTEMPTS: - raise LLMError(f"model call failed: HTTP {response.status_code}: {response.text[:200]}") + detail = scrub(response.text, self._key)[:200] + raise LLMError(f"model call failed: HTTP {response.status_code}: {detail}") self._sleep(retry_wait(response, attempt)) raise AssertionError("unreachable") # pragma: no cover @@ -191,10 +194,52 @@ def complete( ) -def build_llm(timeout_s: float, environ: Mapping[str, str] | None = None) -> LLM: - """The model client for this deploy: Anthropic when its key is set, Groq when only Groq's is - (providers.py).""" - env = os.environ if environ is None else environ - if require_provider(env) == "groq": - return GroqLLM(env["GROQ_API_KEY"], timeout_s=timeout_s) - return AnthropicLLM(timeout_s=timeout_s) +@dataclass(frozen=True, slots=True) +class ModelClients: + """One model client per provider actually used by a deployed agent, built once at start-up so a missing + key is a start-up error, never a surprise on a job's first model call.""" + + _clients: dict[str, LLM] + + @classmethod + def build( + cls, + providers: dict[str, ProviderSpec], + needed: set[str], + timeout_s: float, + environ: Mapping[str, str] | None = None, + ) -> ModelClients: + env = os.environ if environ is None else environ + clients: dict[str, LLM] = {} + for name in needed: + spec = providers.get(name) + if spec is None: + raise ConfigError( + f"provider {name!r} is used by an agent but has no [providers.{name}] table" + ) + key = provider_key(spec, env) + if not key: + env_var = spec.api_key_env or "ANTHROPIC_API_KEY or ANTHROPIC_AUTH_TOKEN" + raise ConfigError(f"{name} needs {env_var}") + if spec.kind == "anthropic": + # With its own `api_key_env`, this provider's key may not be the SDK's own env-var default + # (another `[providers.*]` table could be anthropic too, with a different key); pass it + # explicitly so the right one is actually used, not just checked for presence above. + clients[name] = ( + AnthropicLLM(client=anthropic.Anthropic(api_key=key, timeout=timeout_s)) + if spec.api_key_env + else AnthropicLLM(timeout_s=timeout_s) + ) + else: + if not spec.base_url: + raise ConfigError(f"provider {name!r} is openai_chat but has no base_url") + clients[name] = OpenAIChatLLM(key, base_url=spec.base_url, timeout_s=timeout_s) + return cls(clients) + + def for_provider(self, name: str) -> LLM: + """The client built for `name`. Raising here (rather than returning `None`) means a mismatch + between `validate_agents_have_keys` and `build` fails loudly instead of as a `None` call later.""" + try: + return self._clients[name] + except KeyError: + raise ConfigError(f"no model client was built for provider {name!r}") from None diff --git a/control-plane/mergit_worker/openai_chat.py b/control-plane/mergit_worker/openai_chat.py index 751dba3..62a0620 100644 --- a/control-plane/mergit_worker/openai_chat.py +++ b/control-plane/mergit_worker/openai_chat.py @@ -144,3 +144,47 @@ def parse_reply(data: dict[str, Any]) -> ParsedReply: cached_tokens=int(details.get("cached_tokens") or 0), model=data.get("model"), ) + + +# What a provider's own status codes mean here: worth another attempt, or not. +RETRY_STATUSES = frozenset({429, 500, 502, 503, 504}) + + +@dataclass(frozen=True) +class BodyError: + message: str + retryable: bool + + +def body_error(data: object) -> BodyError | None: + """Why an HTTP 200 body is not an answer, or None when it is one. The body is untrusted input. + + Some providers, OpenRouter among them, report a failure with HTTP 200 and an `error` object carrying the + status they would have sent (`{"error": {"code": 429, "message": ...}}`), so a rate limit or an upstream + outage there is retried like the same status on its own. Both planner and worker clients use this.""" + if not isinstance(data, dict): + return BodyError("the answer is not a JSON object", retryable=False) + error = cast("dict[str, Any]", data).get("error") + if not error: + return None + if not isinstance(error, dict): + return BodyError(str(error), retryable=False) + fields = cast("dict[str, Any]", error) + message, code = fields.get("message"), fields.get("code") + try: + status = int(code) if code is not None and not isinstance(code, bool) else None + except (TypeError, ValueError): + status = None + if isinstance(message, str) and message.strip(): + text = message.strip() + elif code is not None: + text = f"error code {code}" + else: + text = "an error with no message" + return BodyError(text, retryable=status in RETRY_STATUSES) + + +def scrub(text: str, secret: str) -> str: + """`text` with `secret` cut out. Error text from a provider goes into logs and `llm_check`'s output, and + some providers echo the key they rejected back in the body of a 401.""" + return text.replace(secret, "[redacted]") if secret else text diff --git a/control-plane/mergit_worker/preflight.py b/control-plane/mergit_worker/preflight.py new file mode 100644 index 0000000..2838fe5 --- /dev/null +++ b/control-plane/mergit_worker/preflight.py @@ -0,0 +1,66 @@ +"""`python -m mergit_worker.preflight`: the worker's start-up configuration checks, and nothing else. + +deploy/server/update.sh runs this in the freshly built image, with the real env file, before it swaps the +running container. A configuration the new code cannot start from (a deployed agent whose provider has no +key, an unpriced model, a missing setting) then fails the deploy while the old worker keeps running, instead +of leaving a new one that refuses to start. It touches no database and makes no network call. + +`build_worker` calls `check_models` too, so the deploy checks exactly what start-up checks. +""" + +from __future__ import annotations + +import os +import sys +from collections.abc import Mapping +from dataclasses import dataclass + +from .agents import deployed_agents, validate_agents_are_priced, validate_agents_have_keys +from .config import ConfigError, WorkerConfig, load_config +from .llm import ModelClients +from .meter import PriceTable +from .providers import check_model_id, load_providers, stale_settings + + +@dataclass(frozen=True) +class Models: + prices: PriceTable + clients: ModelClients + providers: set[str] + """The providers a deployed agent uses.""" + stale: list[str] + """Removed settings still set in the environment, to warn about (`providers.REMOVED_SETTINGS`).""" + + +def check_models(cfg: WorkerConfig, environ: Mapping[str, str] | None = None) -> Models: + """Every deployed agent is priced, and its provider is configured and has a key.""" + env = os.environ if environ is None else environ + prices = PriceTable.load(cfg.prices_file) + validate_agents_are_priced(cfg.agents_file, cfg.agents_config, prices) + needed = validate_agents_have_keys(cfg.agents_file, cfg.agents_config) + providers = load_providers(cfg.agents_config) + for _entry, agent in deployed_agents(cfg.agents_file, cfg.agents_config): + spec = providers.get(str(agent.get("provider"))) if agent else None + if agent and spec is not None: + check_model_id(spec, str(agent.get("model"))) + stale = stale_settings(providers, needed, env) + clients = ModelClients.build(providers, needed, cfg.http_timeout_s * 20, env) + return Models(prices, clients, needed, stale) + + +def main() -> int: + try: + checked = check_models(load_config()) + except (ConfigError, ValueError, OSError) as exc: + print(f"worker preflight failed: {' '.join(str(exc).split())}", file=sys.stderr) + return 1 + if checked.stale: + print( + f"worker preflight warning: no longer read, remove from the env file: {', '.join(checked.stale)}" + ) + print(f"worker preflight ok: providers {', '.join(sorted(checked.providers)) or '(none)'}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/control-plane/mergit_worker/providers.py b/control-plane/mergit_worker/providers.py index 6b960d7..2373d0f 100644 --- a/control-plane/mergit_worker/providers.py +++ b/control-plane/mergit_worker/providers.py @@ -1,70 +1,162 @@ -"""Which model provider runs, decided once at start-up from the environment. - -Anthropic is the provider whenever `ANTHROPIC_API_KEY` is set. Groq is the fallback when only `GROQ_API_KEY` -is set. -`LLM_PROVIDER=anthropic|groq` forces one. The choice is made once and logged, never per call: a receipt names -the model -that produced it, so a run must not change model halfway. - -On Groq every agent and the planner run `GROQ_MODEL` (default `openai/gpt-oss-120b`) in place of the Claude -ids in -`agents.toml` / `PLANNER_MODEL`. The swap happens when the agent is loaded, before anything is priced, so the -receipt -carries the Groq model's name and the Groq price for what actually ran. +"""The provider each agent and the planner use, each named once in `agents.toml` (or, for the planner, in +the control-plane's own `Settings.planner_provider`). + +`[providers.]` tables in `agents.toml` name a provider's implementation (`kind`) and, for an +OpenAI-compatible one, its `base_url` and the environment variable holding its key. `load_providers` reads +every one of them once at start-up; each agent in `[agents.]` names one by `provider`, and the planner +names one by `planner_provider`. A run never changes model or provider halfway: the choice is made once, at +start-up, and a receipt names the model that produced it. """ from __future__ import annotations -import os +import re +import tomllib from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Literal, cast +from urllib.parse import urlsplit from .config import ConfigError -DEFAULT_GROQ_MODEL = "openai/gpt-oss-120b" -_CHOICES = ("auto", "anthropic", "groq") - - -def has_anthropic_credential(env: Mapping[str, str]) -> bool: - """A real key, or the token a gateway such as Claude Lend hands out (used with ANTHROPIC_BASE_URL; the SDK - reads both from the environment).""" - return bool(env.get("ANTHROPIC_API_KEY") or env.get("ANTHROPIC_AUTH_TOKEN")) - - -def llm_provider(environ: Mapping[str, str] | None = None) -> str: - """`anthropic` or `groq`. With no key at all it answers `anthropic`: nothing changes for a deploy that has - a key or - for tests, and `require_provider` is where a missing key becomes an error.""" - env = os.environ if environ is None else environ - choice = env.get("LLM_PROVIDER", "auto").strip().lower() or "auto" - if choice not in _CHOICES: - raise ConfigError(f"LLM_PROVIDER must be one of {', '.join(_CHOICES)}, not {choice!r}") - if choice != "auto": - return choice - if has_anthropic_credential(env): - return "anthropic" - return "groq" if env.get("GROQ_API_KEY") else "anthropic" - - -def require_provider(environ: Mapping[str, str] | None = None) -> str: - """The provider, or a clear error when the key it needs is missing.""" - env = os.environ if environ is None else environ - provider = llm_provider(env) - if provider == "groq": - key, present = "GROQ_API_KEY", bool(env.get("GROQ_API_KEY")) - else: - key, present = "ANTHROPIC_API_KEY or ANTHROPIC_AUTH_TOKEN", has_anthropic_credential(env) - if present: - return provider - if env.get("LLM_PROVIDER", "auto").strip().lower() in ("", "auto"): +_KINDS = ("anthropic", "openai_chat") + +# The provider key is sent to `base_url` as a Bearer token, and `agents.toml` ships with every push to main, +# so an edit to that file alone must not be able to send a key, or any other secret in the worker's or the +# API's environment, somewhere new. A provider endpoint is therefore one of these hosts over https on the +# default port, or keyrelay (deploy/server/README.md), which is plain http on the compose network and +# publishes no port. A new provider host is a change to this list, reviewed as code. +_PROVIDER_HOSTS = frozenset({"api.groq.com", "openrouter.ai"}) +_KEYRELAY = ("http", "keyrelay", 8787) +# Every model provider's key is named `_API_KEY`, and none of Mergit's own secrets is: they are +# `*_TOKEN`, `*_SECRET`, `*_KEY`, `DATABASE_URL` or `MERGIT_AGENT_KEY_`. `MERGIT_` is refused outright +# so a future Mergit secret cannot match by accident. +_API_KEY_ENV = re.compile(r"[A-Z][A-Z0-9_]*_API_KEY") + + +def _check_base_url(name: str, base_url: str) -> None: + try: + url = urlsplit(base_url) + port = url.port + except ValueError as exc: + raise ConfigError(f"provider {name!r} has an unreadable base_url {base_url!r}: {exc}") from exc + if url.username is not None or url.password is not None: + raise ConfigError(f"provider {name!r} base_url must not carry credentials") + host = (url.hostname or "").lower() + if (url.scheme, host, port) == _KEYRELAY: + return + if url.scheme != "https" or host not in _PROVIDER_HOSTS or port not in (None, 443): + raise ConfigError( + f"provider {name!r} base_url {base_url!r} is not an allowed provider endpoint: it must be https " + f"on one of {', '.join(sorted(_PROVIDER_HOSTS))}, or http://keyrelay:8787. A new provider host " + "is added to _PROVIDER_HOSTS in mergit_worker/providers.py" + ) + + +def _check_api_key_env(name: str, api_key_env: str) -> None: + if not _API_KEY_ENV.fullmatch(api_key_env) or api_key_env.startswith("MERGIT_"): + raise ConfigError( + f"provider {name!r} api_key_env {api_key_env!r} must name a model provider's key, " + "_API_KEY in capitals, and never one of Mergit's own secrets" + ) + + +@dataclass(frozen=True) +class ProviderSpec: + name: str + kind: Literal["anthropic", "openai_chat"] + base_url: str | None + api_key_env: str | None + + +def load_providers(agents_config: Path) -> dict[str, ProviderSpec]: + """Every `[providers.]` table in `agents_config`. An `openai_chat` provider must give `base_url` + and `api_key_env`; `anthropic` needs neither (`provider_key` falls back to the SDK's own environment + variables) and may not give a `base_url`, which its client would ignore. Both fields are checked + against the rules above, so the file cannot point a key at a new host or name a Mergit secret.""" + tables: dict[str, Any] = tomllib.loads(agents_config.read_text()).get("providers", {}) + providers: dict[str, ProviderSpec] = {} + for name, table in tables.items(): + if not isinstance(table, dict): + raise ConfigError(f"provider {name!r} must be a [providers.{name}] table, got {table!r}") + table = cast("dict[str, Any]", table) + kind = table.get("kind") + if kind not in _KINDS: + raise ConfigError(f"provider {name!r} has kind {kind!r}, must be one of {', '.join(_KINDS)}") + base_url = table.get("base_url") + api_key_env = table.get("api_key_env") + for field, value in (("base_url", base_url), ("api_key_env", api_key_env)): + if value is not None and not isinstance(value, str): + raise ConfigError(f"provider {name!r} {field} must be a string, got {value!r}") + if kind == "openai_chat" and (not base_url or not api_key_env): + raise ConfigError(f"provider {name!r} needs base_url and api_key_env for kind openai_chat") + if kind == "anthropic" and base_url is not None: + raise ConfigError(f"provider {name!r} of kind anthropic does not use base_url; remove it") + if base_url is not None: + _check_base_url(str(name), base_url) + if api_key_env is not None: + _check_api_key_env(str(name), api_key_env) + providers[str(name)] = ProviderSpec( + name=str(name), kind=kind, base_url=base_url, api_key_env=api_key_env + ) + return providers + + +def check_model_id(spec: ProviderSpec, model: str) -> None: + """Refuse a model id the provider cannot have, before anything is called with it. A priced id with a + key present passes every other start-up check, so a native id sent to OpenRouter (``claude-haiku-4-5``) + or an OpenRouter id sent to Anthropic (``anthropic/claude-haiku-4.5``) would otherwise surface only as a + failed call on every plan or job. OpenRouter ids are always ``vendor/model``; Anthropic's never have a + slash.""" + host = (urlsplit(spec.base_url).hostname or "").lower() if spec.base_url else "" + if spec.kind == "anthropic" and "/" in model: + raise ConfigError( + f"model {model!r} looks like an OpenRouter id, but provider {spec.name!r} is Anthropic's own " + "API, which names models without a vendor prefix (claude-...)" + ) + if host == "openrouter.ai" and "/" not in model: raise ConfigError( - "set ANTHROPIC_API_KEY, ANTHROPIC_AUTH_TOKEN or GROQ_API_KEY: " - "the planner and the agents need a model" + f"model {model!r} is not an OpenRouter id: provider {spec.name!r} is OpenRouter, whose ids are " + "vendor/model (anthropic/claude-haiku-4.5, ...)" ) - raise ConfigError(f"LLM_PROVIDER={provider} needs {key}") -def effective_model(configured: str, environ: Mapping[str, str] | None = None) -> str: - env = os.environ if environ is None else environ - if llm_provider(env) != "groq": - return configured - return env.get("GROQ_MODEL") or DEFAULT_GROQ_MODEL +def provider_key(spec: ProviderSpec, environ: Mapping[str, str]) -> str | None: + """The credential this provider's client should send. `anthropic` with no `api_key_env` falls back to + `ANTHROPIC_API_KEY` or `ANTHROPIC_AUTH_TOKEN`, which the SDK also reads itself.""" + if spec.api_key_env: + return environ.get(spec.api_key_env) + if spec.kind == "anthropic": + return environ.get("ANTHROPIC_API_KEY") or environ.get("ANTHROPIC_AUTH_TOKEN") + return None + + +# Read by earlier versions, ignored now: a provider's endpoint and model are in `agents.toml`, and each agent +# and the planner name their provider there (or in `PLANNER_PROVIDER`). Still set, they mislead whoever +# reads the env file, so start-up names them. +REMOVED_SETTINGS = ("GROQ_BASE_URL", "GROQ_MODEL", "LLM_PROVIDER") + + +def stale_settings( + providers: Mapping[str, ProviderSpec], in_use: set[str], environ: Mapping[str, str] +) -> list[str]: + """The removed settings still set in `environ`, for start-up to warn about. + + One of them is refused instead: `GROQ_BASE_URL` naming another endpoint than the one `agents.toml` gives + a provider in use that reads `GROQ_API_KEY`. That env file was written for keyrelay, so `GROQ_API_KEY` + holds keyrelay's proxy token, and with `GROQ_BASE_URL` ignored the token would now go to the endpoint in + `agents.toml` instead.""" + stale = [name for name in REMOVED_SETTINGS if environ.get(name)] + old_url = (environ.get("GROQ_BASE_URL") or "").rstrip("/") + for name in sorted(in_use): + spec = providers.get(name) + if spec is None or spec.api_key_env != "GROQ_API_KEY" or not old_url or not spec.base_url: + continue + if old_url != spec.base_url.rstrip("/"): + raise ConfigError( + f"GROQ_BASE_URL is set but no longer read, and provider {name!r} now sends GROQ_API_KEY to " + f"{spec.base_url} (agents.toml) instead. Set that provider's base_url in agents.toml to the " + "endpoint you meant (http://keyrelay:8787/v1 for keyrelay), then remove GROQ_BASE_URL" + ) + return stale diff --git a/control-plane/mergit_worker/worker.py b/control-plane/mergit_worker/worker.py index 6a7469f..044b30a 100644 --- a/control-plane/mergit_worker/worker.py +++ b/control-plane/mergit_worker/worker.py @@ -11,7 +11,7 @@ from .config import WorkerConfig from .harness import HarnessConfig, Publisher, run_task from .jobs import JobStore -from .llm import LLM +from .llm import ModelClients from .meter import PriceTable from .models import Job from .pr_worker import PrHandler @@ -27,7 +27,7 @@ @dataclass(frozen=True, slots=True) class WorkerDeps: store: JobStore - llm: LLM + models: ModelClients prices: PriceTable publisher: Publisher make_sandbox: SandboxFactory @@ -78,7 +78,7 @@ def _run(self, job: Job) -> None: job, agent, sandbox, - self._deps.llm, + self._deps.models.for_provider(agent.provider), self._deps.prices, self._deps.publisher, self._deps.harness, diff --git a/control-plane/prices.toml b/control-plane/prices.toml index b6c84c2..a2a9dce 100644 --- a/control-plane/prices.toml +++ b/control-plane/prices.toml @@ -37,7 +37,8 @@ output_per_mtok = 5.00 cache_read_per_mtok = 0.10 cache_write_per_mtok = 1.25 -# Groq, the fallback provider when there is no Anthropic key (mergit_worker/providers.py). Checked against +# Groq, one more `[providers.*]` option in agents.toml (mergit_worker/providers.py), not a fallback: each +# agent and the planner name their own provider explicitly. Checked against # https://console.groq.com/docs/models (list prices) and https://console.groq.com/docs/prompt-caching (cached input is # "50% less" than normal input, and caching has no extra fee, so cache_write equals input) on 2026-10-30. # The llama models Groq lists as "contact sales" have no public price and are deliberately absent: the meter refuses an @@ -54,3 +55,25 @@ input_per_mtok = 0.075 output_per_mtok = 0.30 cache_read_per_mtok = 0.0375 cache_write_per_mtok = 0.075 + +# OpenRouter list prices, checked against https://openrouter.ai/api/v1/models on 2026-11-01 (claude-haiku-4.5 on +# 2026-11-04, same as Anthropic's own). The receipt +# prices the exact OpenRouter id called. + +[models."anthropic/claude-sonnet-5"] +input_per_mtok = 2.00 +output_per_mtok = 10.00 +cache_read_per_mtok = 0.20 +cache_write_per_mtok = 2.50 + +[models."anthropic/claude-haiku-4.5"] +input_per_mtok = 1.00 +output_per_mtok = 5.00 +cache_read_per_mtok = 0.10 +cache_write_per_mtok = 1.25 + +[models."deepseek/deepseek-v4-flash"] +input_per_mtok = 0.091 +output_per_mtok = 0.182 +cache_read_per_mtok = 0.0182 +cache_write_per_mtok = 0.091 diff --git a/control-plane/tests/conftest.py b/control-plane/tests/conftest.py index ba28506..bb9950f 100644 --- a/control-plane/tests/conftest.py +++ b/control-plane/tests/conftest.py @@ -52,7 +52,9 @@ def agents_files(tmp_path: Path) -> tuple[Path, Path]: ) config = tmp_path / "agents.toml" config.write_text( - '[agents.coder-a]\nmodel = "claude-sonnet-5"\nsystem_prompt = "You fix failing tests."\n' + '[providers.anthropic]\nkind = "anthropic"\n\n' + '[agents.coder-a]\nprovider = "anthropic"\nmodel = "claude-sonnet-5"\n' + 'system_prompt = "You fix failing tests."\n' ) return chain, config @@ -70,6 +72,7 @@ def settings(database_url: str, agents_files: tuple[Path, Path]) -> Settings: agents_config=str(config), cors_origins=["http://localhost:5173"], planner_model="claude-opus-5", + planner_provider="anthropic", verify_repos=False, # tests do not reach GitHub; the ones about the check turn it on events_token=SecretStr("test-events-token"), chain_events_token=SecretStr("test-chain-events-token"), diff --git a/control-plane/tests/support.py b/control-plane/tests/support.py index 9272deb..43fc326 100644 --- a/control-plane/tests/support.py +++ b/control-plane/tests/support.py @@ -90,6 +90,7 @@ def make_job(repo_url: str, **over: Any) -> Job: def make_agent() -> AgentSpec: return AgentSpec( agent_id="1", + provider="anthropic", model="test-model", system_prompt="You fix bugs.", runtime_key_address=KEY_ADDRESS, diff --git a/control-plane/tests/test_account_settings.py b/control-plane/tests/test_account_settings.py index 3a10d63..d69dcb9 100644 --- a/control-plane/tests/test_account_settings.py +++ b/control-plane/tests/test_account_settings.py @@ -14,6 +14,7 @@ "STORE_URL": "http://s", "AGENTS_FILE": "a.json", "PLANNER_MODEL": "m", + "PLANNER_PROVIDER": "anthropic", "EVENTS_TOKEN": "e", "CHAIN_EVENTS_TOKEN": "c", "WORKER_TOKEN": "w", diff --git a/control-plane/tests/test_agents.py b/control-plane/tests/test_agents.py index 9fd8339..aa553e9 100644 --- a/control-plane/tests/test_agents.py +++ b/control-plane/tests/test_agents.py @@ -3,6 +3,9 @@ import pytest from mergit_cp.agents import Agent, load_agents +from mergit_worker.agents import load_agent_spec +from mergit_worker.config import ConfigError +from tests.support import KEY def test_load_agents_joins_chain_ids_with_model_config(agents_files: tuple[Path, Path]) -> None: @@ -13,6 +16,7 @@ def test_load_agents_joins_chain_ids_with_model_config(agents_files: tuple[Path, name="coder-a", operator="0x1111111111111111111111111111111111111111", runtime_key="0x2222222222222222222222222222222222222222", + provider="anthropic", model="claude-sonnet-5", system_prompt="You fix failing tests.", ) @@ -21,11 +25,27 @@ def test_load_agents_joins_chain_ids_with_model_config(agents_files: tuple[Path, def test_agent_without_model_config_fails_loudly(agents_files: tuple[Path, Path]) -> None: chain, config = agents_files - config.write_text('[agents.someone-else]\nmodel = "m"\nsystem_prompt = "p"\n') + config.write_text('[agents.someone-else]\nprovider = "anthropic"\nmodel = "m"\nsystem_prompt = "p"\n') with pytest.raises(ValueError, match="coder-a"): load_agents(str(chain), str(config)) +def test_agent_naming_missing_provider_is_refused(agents_files: tuple[Path, Path]) -> None: + chain, config = agents_files + config.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + '[agents.coder-a]\nprovider = "nope"\nmodel = "claude-sonnet-5"\nsystem_prompt = "p"\n' + ) + with pytest.raises(ValueError, match="coder-a") as exc: + load_agents(str(chain), str(config)) + assert "nope" in str(exc.value) + + env = {"MERGIT_AGENT_KEY_1": KEY} + with pytest.raises(ConfigError, match="coder-a") as cexc: + load_agent_spec("1", chain, config, env) + assert "nope" in str(cexc.value) + + def test_rejects_non_decimal_agent_id(agents_files: tuple[Path, Path]) -> None: chain, config = agents_files chain.write_text(chain.read_text().replace('"agent_id": "1"', '"agent_id": "0x1"')) diff --git a/control-plane/tests/test_chain_events.py b/control-plane/tests/test_chain_events.py index 609f43c..2708038 100644 --- a/control-plane/tests/test_chain_events.py +++ b/control-plane/tests/test_chain_events.py @@ -108,6 +108,7 @@ def escrowed(task_id: str) -> dict[str, Any]: store_url="http://store.invalid", agents_file="unused", planner_model="claude-opus-5", + planner_provider="anthropic", events_token=SecretStr("test-events-token"), chain_events_token=SecretStr("test-chain-events-token"), worker_token=SecretStr("test-worker-token"), diff --git a/control-plane/tests/test_eligibility.py b/control-plane/tests/test_eligibility.py index d9ac0e4..c013f78 100644 --- a/control-plane/tests/test_eligibility.py +++ b/control-plane/tests/test_eligibility.py @@ -38,6 +38,7 @@ def agent(operator: str) -> Agent: name=f"agent-{agent_id}", operator=operator, runtime_key="0x" + "2" * 40, + provider="anthropic", model="claude-sonnet-5", system_prompt="You fix failing tests.", ) diff --git a/control-plane/tests/test_groq_llm.py b/control-plane/tests/test_groq_llm.py deleted file mode 100644 index 5614f1d..0000000 --- a/control-plane/tests/test_groq_llm.py +++ /dev/null @@ -1,225 +0,0 @@ -"""The Groq adapters (worker: sync, planner: async) against Groq's documented HTTP shape, faked with respx.""" - -from __future__ import annotations - -import asyncio -import json -from typing import Any - -import httpx -import pytest -import respx - -from mergit_cp import llm as cp_llm -from mergit_worker import llm as worker_llm - -URL = "https://api.groq.com/openai/v1/chat/completions" -TOOLS = [{"name": "finish", "description": "done", "input_schema": {"type": "object", "properties": {}}}] - - -def reply( - message: dict[str, Any], finish: str = "stop", usage: dict[str, Any] | None = None -) -> dict[str, Any]: - return { - "model": "openai/gpt-oss-120b", - "choices": [{"message": {"role": "assistant", **message}, "finish_reason": finish}], - "usage": usage or {"prompt_tokens": 12, "completion_tokens": 3}, - } - - -# -- worker (sync) ---------------------------------------------------------------------------- - - -@respx.mock -def test_worker_sends_the_documented_request_and_returns_anthropic_shaped_blocks(): - route = respx.post(URL).respond( - 200, - json=reply( - { - "content": "ok", - "tool_calls": [ - {"id": "c1", "type": "function", "function": {"name": "finish", "arguments": "{}"}} - ], - }, - finish="tool_calls", - ), - ) - llm = worker_llm.GroqLLM("test-key", sleep=lambda _s: None) - result = llm.complete( - [{"role": "user", "content": "go"}], TOOLS, "openai/gpt-oss-120b", system="sys", max_tokens=99 - ) - - request = route.calls.last.request - assert request.headers["authorization"] == "Bearer test-key" - body = json.loads(request.content) - assert body["model"] == "openai/gpt-oss-120b" - assert body["max_completion_tokens"] == 99 and "max_tokens" not in body - assert body["messages"][0] == {"role": "system", "content": "sys"} - assert body["tools"][0]["function"]["name"] == "finish" and body["tool_choice"] == "auto" - - assert result.text == "ok" and result.stop_reason == "tool_use" - assert [(c.id, c.name, c.input) for c in result.tool_calls] == [("c1", "finish", {})] - assert result.assistant_content == [ - {"type": "text", "text": "ok"}, - {"type": "tool_use", "id": "c1", "name": "finish", "input": {}}, - ] - assert (result.input_tokens, result.output_tokens, result.model) == (12, 3, "openai/gpt-oss-120b") - - -@respx.mock -def test_worker_counts_cached_prompt_tokens_in_full_so_a_receipt_never_understates_cost(): - respx.post(URL).respond( - 200, - json=reply( - {"content": "x"}, - usage={ - "prompt_tokens": 100, - "completion_tokens": 1, - "prompt_tokens_details": {"cached_tokens": 60}, - }, - ), - ) - result = worker_llm.GroqLLM("k", sleep=lambda _s: None).complete( - [{"role": "user", "content": "x"}], None, "m" - ) - assert result.input_tokens == 100 - - -@respx.mock -def test_worker_retries_rate_limits_then_succeeds(): - route = respx.post(URL).mock( - side_effect=[ - httpx.Response(429, headers={"retry-after": "1"}, json={}), - httpx.Response(200, json=reply({"content": "hi"})), - ] - ) - waits: list[float] = [] - result = worker_llm.GroqLLM("k", sleep=waits.append).complete( - [{"role": "user", "content": "x"}], None, "m" - ) - assert result.text == "hi" and route.call_count == 2 and waits == [1.0] - - -@respx.mock -def test_worker_gives_up_after_three_attempts(): - route = respx.post(URL).respond(503, json={}) - with pytest.raises(worker_llm.LLMError, match="503"): - worker_llm.GroqLLM("k", sleep=lambda _s: None).complete([{"role": "user", "content": "x"}], None, "m") - assert route.call_count == 3 - - -@respx.mock -def test_worker_does_not_retry_a_rejected_request_and_never_echoes_the_key(): - route = respx.post(URL).respond(400, json={"error": {"message": "bad field"}}) - with pytest.raises(worker_llm.LLMError) as caught: - worker_llm.GroqLLM("secret-key-123", sleep=lambda _s: None).complete( - [{"role": "user", "content": "x"}], None, "m" - ) - assert route.call_count == 1 and "secret-key-123" not in str(caught.value) and "400" in str(caught.value) - - -@respx.mock -def test_worker_turns_a_connection_failure_into_an_llm_error(): - respx.post(URL).mock(side_effect=httpx.ConnectError("down")) - with pytest.raises(worker_llm.LLMError): - worker_llm.GroqLLM("k", sleep=lambda _s: None).complete([{"role": "user", "content": "x"}], None, "m") - - -def test_the_worker_factory_picks_the_provider_from_the_environment(): - assert isinstance(worker_llm.build_llm(10.0, {"GROQ_API_KEY": "g"}), worker_llm.GroqLLM) - assert isinstance(worker_llm.build_llm(10.0, {"ANTHROPIC_API_KEY": "a"}), worker_llm.AnthropicLLM) - from mergit_worker.config import ConfigError - - with pytest.raises(ConfigError): - worker_llm.build_llm(10.0, {}) - - -# -- planner (async) --------------------------------------------------------------------------- - - -def run(coro: Any) -> Any: - return asyncio.run(coro) - - -@respx.mock -def test_planner_asks_for_a_json_schema_answer_best_effort_and_splits_cached_tokens(): - route = respx.post(URL).respond( - 200, - json=reply( - {"content": '{"tasks": []}'}, - usage={ - "prompt_tokens": 100, - "completion_tokens": 8, - "prompt_tokens_details": {"cached_tokens": 40}, - }, - ), - ) - schema = {"type": "object", "properties": {"tasks": {"type": "array"}}} - result = run( - cp_llm.GroqLLM("k", sleep=lambda _s: None).complete( - [{"role": "user", "content": "plan"}], - None, - "openai/gpt-oss-120b", - system="s", - output_schema=schema, - ) - ) - body = json.loads(route.calls.last.request.content) - assert body["response_format"] == { - "type": "json_schema", - "json_schema": {"name": "answer", "strict": False, "schema": schema}, - } - assert result.text == '{"tasks": []}' - # input is the uncached part and the cached part is priced separately at its own rate - assert ( - result.input_tokens, - result.cache_read_tokens, - result.cache_write_tokens, - result.output_tokens, - ) == ( - 60, - 40, - 0, - 8, - ) - - -@respx.mock -def test_planner_reports_an_outage_as_unavailable_so_the_api_answers_502(): - respx.post(URL).respond(500, json={}) - with pytest.raises(cp_llm.LLMUnavailable): - run( - cp_llm.GroqLLM("k", sleep=lambda _s: None).complete([{"role": "user", "content": "x"}], None, "m") - ) - - -@respx.mock -def test_planner_reports_a_truncated_answer_as_an_error_that_still_carries_its_usage(): - respx.post(URL).respond( - 200, - json=reply({"content": '{"ta'}, finish="length", usage={"prompt_tokens": 5, "completion_tokens": 9}), - ) - with pytest.raises(cp_llm.LLMError) as caught: - run( - cp_llm.GroqLLM("k", sleep=lambda _s: None).complete([{"role": "user", "content": "x"}], None, "m") - ) - assert caught.value.reason == "max_tokens" and caught.value.usage.output_tokens == 9 - - -def test_the_planner_factory_picks_the_provider_from_the_environment(): - assert isinstance(cp_llm.build_llm({"GROQ_API_KEY": "g"}), cp_llm.GroqLLM) - assert isinstance(cp_llm.build_llm({"ANTHROPIC_API_KEY": "a"}), cp_llm.AnthropicLLM) - - -def test_the_api_still_starts_with_no_model_key_so_sign_in_and_credits_work(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setenv( - "ANTHROPIC_API_KEY", "placeholder" - ) # the SDK builds its client without contacting anything - assert isinstance(cp_llm.build_llm({}), cp_llm.AnthropicLLM) - - -def test_the_planner_accepts_a_gateway_token_without_warning(caplog: pytest.LogCaptureFixture): - env = {"ANTHROPIC_AUTH_TOKEN": "lend-token", "ANTHROPIC_BASE_URL": "https://lend.example"} - with caplog.at_level("WARNING", logger="mergit_cp.llm"): - assert isinstance(cp_llm.build_llm(env), cp_llm.AnthropicLLM) - assert "no model key" not in caplog.text diff --git a/control-plane/tests/test_harness.py b/control-plane/tests/test_harness.py index 05d5b77..a8455b2 100644 --- a/control-plane/tests/test_harness.py +++ b/control-plane/tests/test_harness.py @@ -68,6 +68,48 @@ def report(self, task_id: str, seq: int, step_type: str, cost: Any) -> None: self.reports.append((task_id, seq, step_type, cost.usd_micros)) +def test_tampered_step_output_is_rejected(sandbox, publisher, repo_url): + """Flip one character of a non-final step's `output_digest` in a copy of the sealed bundle: the + chain no longer commits to what it attests, so the control plane's own `verify()` call must flag + it, not just the Rust crates' internal vector tests.""" + run(sandbox, publisher, [turn(FIX), turn(("finish", {"summary": "done"}))], repo_url) + bundle = publisher.bundle() + assert bundle["receipt"]["steps"][0]["type"] == "model_call" # the step we're about to tamper + digest = bundle["receipt"]["steps"][0]["output_digest"] + flipped = "1" if digest[-1] != "1" else "2" + bundle["receipt"]["steps"][0]["output_digest"] = digest[:-1] + flipped + result = json.loads(mergit_receipt.verify(json.dumps(bundle), CHAIN_ID, REGISTRY, None)) + assert result["status"] != "unattested_valid" + assert result["reasons"] + + +def test_reordered_steps_are_rejected(sandbox, publisher, repo_url): + """Swap two non-terminal steps' positions in a copy of the sealed bundle (their `seq` fields are + left untouched, so the list order no longer matches `seq` order): the control plane's own + `verify()` call must flag it.""" + run(sandbox, publisher, [turn(FIX), turn(("finish", {"summary": "done"}))], repo_url) + bundle = publisher.bundle() + steps = bundle["receipt"]["steps"] + assert steps[0]["type"] == "model_call" and steps[2]["type"] == "model_call" # the steps we're swapping + steps[0], steps[2] = steps[2], steps[0] + result = json.loads(mergit_receipt.verify(json.dumps(bundle), CHAIN_ID, REGISTRY, None)) + assert result["status"] != "unattested_valid" + assert result["reasons"] + + +def test_dropped_step_is_rejected(sandbox, publisher, repo_url): + """Remove one non-terminal step entirely from a copy of the sealed bundle's `steps` array: this is + CLAUDE.md invariant 1's "complete trajectory" check, proven from the Python side.""" + run(sandbox, publisher, [turn(FIX), turn(("finish", {"summary": "done"}))], repo_url) + bundle = publisher.bundle() + steps = bundle["receipt"]["steps"] + assert steps[1]["type"] == "artifact_write" # the step we're about to drop + del steps[1] + result = json.loads(mergit_receipt.verify(json.dumps(bundle), CHAIN_ID, REGISTRY, None)) + assert result["status"] != "unattested_valid" + assert result["reasons"] + + def test_steps_are_reported_live_as_they_are_recorded(sandbox, publisher, repo_url): """Every agent-loop step goes to the progress reporter the instant it is recorded, with the same seq the receipt gives it; the Φ command and phi_eval steps, added after the loop ends, are not @@ -208,6 +250,18 @@ def test_budget_too_small_for_one_token_stops_before_calling(sandbox, publisher, assert outcome.notes == ["cost_budget"] +def test_budget_exhausted_with_no_completed_work_fails(sandbox, publisher, repo_url): + """The budget runs out before the agent is even called once, and no work was done, so the sandbox + is still in its unfixed starting state: Φ's predicate is unsatisfied and `holds` must say so, not + come back `None` or get skipped because the agent never got a turn.""" + job = make_job(repo_url, budget_usdc_units=1) # a fraction of a cent: cannot afford one output token + llm = _RecordingLLM([turn(("finish", {}))]) + outcome = run_task(job, make_agent(), sandbox, llm, make_prices(), publisher, make_harness_config()) + assert llm.calls == 0 + assert outcome.notes == ["cost_budget"] + assert outcome.holds is False + + def test_step_budget_stops_the_loop(sandbox, publisher, repo_url): spin = [turn(("list_files", {})) for _ in range(5)] outcome, receipt, llm = run(sandbox, publisher, spin, repo_url, max_steps=3) diff --git a/control-plane/tests/test_llm.py b/control-plane/tests/test_llm.py index e75c387..d6ee76a 100644 --- a/control-plane/tests/test_llm.py +++ b/control-plane/tests/test_llm.py @@ -1,12 +1,25 @@ -"""The Anthropic adapter, against a stand-in for the SDK client (no network, no key).""" +"""The Anthropic adapter (against a stand-in for the SDK client, no network, no key), and the planner's own +provider choice (`build_planner_llm`): it names a provider from `[providers.]`, exactly as an agent +does.""" import asyncio +from pathlib import Path from types import SimpleNamespace from typing import Any +import psycopg import pytest +import respx +from eth_account import Account +from eth_account.messages import encode_defunct +from fastapi.testclient import TestClient +from siwe import SiweMessage -from mergit_cp.llm import AnthropicLLM, LLMError, LLMUnavailable +from mergit_cp.app import create_app +from mergit_cp.config import Settings +from mergit_cp.llm import AnthropicLLM, LLMError, LLMUnavailable, build_planner_llm +from mergit_worker.providers import ProviderSpec +from tests.conftest import APP_DOMAIN class StubMessages: @@ -82,3 +95,87 @@ async def create(self, **kwargs: Any) -> Any: llm = AnthropicLLM(SimpleNamespace(messages=Down())) # pyright: ignore[reportArgumentType] with pytest.raises(LLMUnavailable): asyncio.run(llm.complete([{"role": "user", "content": "x"}], None, "claude-opus-5")) + + +@respx.mock +def test_planner_uses_named_provider() -> None: + """`build_planner_llm` sends to the provider's own `base_url`, with its own key, not Anthropic's or + Groq's: an OpenRouter deploy sends to OpenRouter.""" + providers = { + "openrouter": ProviderSpec( + name="openrouter", + kind="openai_chat", + base_url="https://openrouter.ai/api/v1", + api_key_env="OPENROUTER_API_KEY", + ) + } + route = respx.post("https://openrouter.ai/api/v1/chat/completions").respond( + 200, + json={ + "model": "anthropic/claude-sonnet-5", + "choices": [{"message": {"role": "assistant", "content": "ok"}, "finish_reason": "stop"}], + "usage": {"prompt_tokens": 5, "completion_tokens": 2}, + }, + ) + llm = build_planner_llm(providers, "openrouter", environ={"OPENROUTER_API_KEY": "k"}) + asyncio.run(llm.complete([{"role": "user", "content": "x"}], None, "anthropic/claude-sonnet-5")) + request = route.calls.last.request + assert str(request.url) == "https://openrouter.ai/api/v1/chat/completions" + assert request.headers["authorization"] == "Bearer k" + + +_PLAN_BODY = { + "goal": "Fix the failing refund test", + "repo_url": "https://github.com/mergit-io/demo-repo", + "ref": "main", + "budget_usdc_units": "2000000", + "category": "test-repair", +} + + +def _sign_in(client: TestClient) -> None: + account = Account.create() + nonce = client.get("/v0/auth/nonce").json()["nonce"] + message = SiweMessage.model_validate( + { + "domain": APP_DOMAIN, + "address": account.address, + "uri": f"https://{APP_DOMAIN}", + "version": "1", + "chain_id": 84532, + "issued_at": "2026-10-28T10:00:00.000Z", + "nonce": nonce, + } + ).prepare_message() + signature = account.sign_message(encode_defunct(text=message)).signature.to_0x_hex() + assert ( + client.post("/v0/auth/verify", json={"message": message, "signature": signature}).status_code == 200 + ) + + +@pytest.mark.postgres +def test_missing_planner_key_answers_502( + settings: Settings, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """With no OPENROUTER_API_KEY in the environment, the API still starts — sign-in, credits and the agent + list all work — exactly as it always has with no Anthropic key; only a planning request fails, with a + clear 502, because `build_planner_llm` falls back to an always-unavailable client.""" + monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) + config = tmp_path / "agents.toml" + config.write_text( + Path(settings.agents_config).read_text() + '\n[providers.openrouter]\nkind = "openai_chat"\n' + 'base_url = "https://openrouter.ai/api/v1"\napi_key_env = "OPENROUTER_API_KEY"\n' + ) + missing_key = settings.model_copy( + update={ + "planner_provider": "openrouter", + "planner_model": "anthropic/claude-sonnet-5", # an OpenRouter id: the native default is refused + "agents_config": str(config), + } + ) + with TestClient(create_app(missing_key, deliver_events=False), base_url="https://testserver") as c: + assert c.get("/v0/agents").status_code == 200 + _sign_in(c) + assert c.post("/v0/plans", json=_PLAN_BODY).status_code == 502 + with psycopg.connect(settings.database_url) as conn: + conn.execute("TRUNCATE planning_attempts, sessions, auth_nonces") diff --git a/control-plane/tests/test_llm_check.py b/control-plane/tests/test_llm_check.py index b8f26ee..7d921d2 100644 --- a/control-plane/tests/test_llm_check.py +++ b/control-plane/tests/test_llm_check.py @@ -1,11 +1,19 @@ -"""`python -m mergit_cp.llm_check` passes only when a plain answer and a tool call both work, and never prints -the credential.""" +"""`python -m mergit_cp.llm_check` passes only when every provider in use answers a plain question and a +tool call, and never prints the credential.""" +from __future__ import annotations + +import asyncio +import json +from pathlib import Path from typing import Any import pytest +import respx +from pydantic import SecretStr from mergit_cp import llm_check +from mergit_cp.config import Settings from mergit_cp.llm import LLMResult, LLMUnavailable @@ -22,31 +30,273 @@ async def complete(self, messages: Any, tools: Any, model: str, **_: Any) -> LLM return LLMResult("ready" if not tools else "", calls, 5, 3, model) -@pytest.fixture(autouse=True) -def _env(monkeypatch: pytest.MonkeyPatch) -> None: - for name in ("ANTHROPIC_API_KEY", "GROQ_API_KEY", "LLM_PROVIDER"): - monkeypatch.delenv(name, raising=False) - monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "super-secret-token") - monkeypatch.setenv("ANTHROPIC_BASE_URL", "https://lend.example/v1") +def _agent(name: str, provider: str, model: str) -> dict[str, str]: + return {"name": name, "provider": provider, "model": model} + + +def _write_agents( + tmp_path: Path, providers: dict[str, dict[str, str]], agents: list[dict[str, str]] +) -> tuple[Path, Path]: + """A minimal `agents.json` + `agents.toml` pair, built directly for the test rather than touching the + repo's real deploy files.""" + chain = tmp_path / f"agents-{len(agents)}-{len(providers)}.json" + chain.write_text( + json.dumps( + { + "agents": [ + { + "agent_id": str(i), + "name": a["name"], + "operator": "0x" + "1" * 40, + "runtime_key": "0x" + "2" * 40, + } + for i, a in enumerate(agents) + ] + } + ) + ) + provider_tables = "\n\n".join( + f"[providers.{name}]\n" + "\n".join(f'{k} = "{v}"' for k, v in table.items()) + for name, table in providers.items() + ) + agent_tables = ( + "\n".join( + f'[agents.{a["name"]}]\nprovider = "{a["provider"]}"\nmodel = "{a["model"]}"\n' + 'system_prompt = "p"\n' + for a in agents + ) + or "[agents]\n" # an explicit empty table, so `models["agents"]` still resolves with no agents + ) + config = tmp_path / f"agents-{len(agents)}-{len(providers)}.toml" + config.write_text(f"{provider_tables}\n\n{agent_tables}") + return chain, config -def test_it_passes_when_both_calls_work(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]): - monkeypatch.setattr(llm_check, "build_llm", lambda: Fake()) - assert llm_check.main(["claude-haiku-4-5"]) == 0 +def _settings( + agents_files: tuple[Path, Path], planner_provider: str, planner_model: str = "claude-haiku-4-5" +) -> Settings: + chain, config = agents_files + return Settings( + database_url="postgresql://unused/db", # never connected to: llm_check touches no database + app_domain="app.mergit.test", + session_secret=SecretStr("test-secret-at-least-32-bytes-long!!"), + gateway_url="http://gateway.invalid", + store_url="http://store.invalid", + agents_file=str(chain), + agents_config=str(config), + planner_model=planner_model, + planner_provider=planner_provider, + events_token=SecretStr("test-events-token"), + chain_events_token=SecretStr("test-chain-events-token"), + worker_token=SecretStr("test-worker-token"), + ) + + +def test_it_passes_when_both_calls_work( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + agents_files = _write_agents(tmp_path, {"anthropic": {"kind": "anthropic"}}, []) + settings = _settings(agents_files, "anthropic") + monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "super-secret-token") + monkeypatch.setattr(llm_check, "build_planner_llm", lambda *a, **k: Fake()) + + assert asyncio.run(llm_check.check(settings)) is True out = capsys.readouterr().out - assert "lend.example" in out and "tool call: ok" in out + assert "provider=anthropic" in out and "tool call: ok" in out assert "super-secret-token" not in out -def test_it_fails_when_the_model_does_not_call_the_tool(monkeypatch: pytest.MonkeyPatch): - monkeypatch.setattr(llm_check, "build_llm", lambda: Fake(tool_works=False)) - assert llm_check.main(["m"]) == 1 +def test_it_fails_when_the_model_does_not_call_the_tool(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + agents_files = _write_agents(tmp_path, {"anthropic": {"kind": "anthropic"}}, []) + settings = _settings(agents_files, "anthropic") + monkeypatch.setattr(llm_check, "build_planner_llm", lambda *a, **k: Fake(tool_works=False)) + + assert asyncio.run(llm_check.check(settings)) is False def test_it_fails_and_says_why_when_the_provider_refuses( - monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + agents_files = _write_agents(tmp_path, {"anthropic": {"kind": "anthropic"}}, []) + settings = _settings(agents_files, "anthropic") + monkeypatch.setattr(llm_check, "build_planner_llm", lambda *a, **k: Fake(down=True)) + + assert asyncio.run(llm_check.check(settings)) is False + assert "HTTP 401" in capsys.readouterr().out + + +def test_distinct_models_on_the_same_provider_each_get_checked( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + """Both shipped agents are on `openrouter` but name different models (`claude-sonnet-5` via OpenRouter and + `deepseek-v4-flash`): each distinct (provider, model) pair actually in use gets its own check line, not + just the first one found for the provider.""" + agents_files = _write_agents( + tmp_path, + { + "anthropic": {"kind": "anthropic"}, + "openrouter": { + "kind": "openai_chat", + "base_url": "https://openrouter.ai/api/v1", + "api_key_env": "OPENROUTER_API_KEY", + }, + }, + [_agent("coder-a", "openrouter", "model-a"), _agent("coder-b", "openrouter", "model-b")], + ) + settings = _settings(agents_files, "anthropic") + + calls: list[str] = [] + + def fake_build(providers: Any, name: str, *a: Any, **k: Any) -> Fake: + calls.append(name) + return Fake() + + monkeypatch.setattr(llm_check, "build_planner_llm", fake_build) + + assert asyncio.run(llm_check.check(settings)) is True + assert sorted(calls) == ["anthropic", "openrouter", "openrouter"] + lines = [line for line in capsys.readouterr().out.splitlines() if line.startswith("provider=")] + assert len(lines) == 3 + assert any(line.startswith("provider=anthropic") for line in lines) + assert any("provider=openrouter" in line and "model=model-a" in line for line in lines) + assert any("provider=openrouter" in line and "model=model-b" in line for line in lines) + + +def test_same_provider_and_model_is_checked_only_once( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ): - monkeypatch.setattr(llm_check, "build_llm", lambda: Fake(down=True)) - assert llm_check.main(["m"]) == 1 + """Two agents on the same provider naming the SAME model produce only one check line: no redundant call + for an identical (provider, model) pair.""" + agents_files = _write_agents( + tmp_path, + { + "anthropic": {"kind": "anthropic"}, + "openrouter": { + "kind": "openai_chat", + "base_url": "https://openrouter.ai/api/v1", + "api_key_env": "OPENROUTER_API_KEY", + }, + }, + [_agent("coder-a", "openrouter", "model-a"), _agent("coder-b", "openrouter", "model-a")], + ) + settings = _settings(agents_files, "anthropic") + + calls: list[str] = [] + + def fake_build(providers: Any, name: str, *a: Any, **k: Any) -> Fake: + calls.append(name) + return Fake() + + monkeypatch.setattr(llm_check, "build_planner_llm", fake_build) + + assert asyncio.run(llm_check.check(settings)) is True + assert sorted(calls) == ["anthropic", "openrouter"] + lines = [line for line in capsys.readouterr().out.splitlines() if line.startswith("provider=")] + assert len(lines) == 2 + assert any(line.startswith("provider=anthropic") for line in lines) + assert any("provider=openrouter" in line and "model=model-a" in line for line in lines) + + +def _two_providers(tmp_path: Path) -> tuple[Path, Path]: + return _write_agents( + tmp_path, + { + "anthropic": {"kind": "anthropic"}, + "openrouter": { + "kind": "openai_chat", + "base_url": "https://openrouter.ai/api/v1", + "api_key_env": "OPENROUTER_API_KEY", + }, + }, + [_agent("coder-a", "openrouter", "model-a")], + ) + + +def _record_builds(monkeypatch: pytest.MonkeyPatch) -> list[str]: + calls: list[str] = [] + + def fake_build(providers: Any, name: str, *a: Any, **k: Any) -> Fake: + calls.append(name) + return Fake() + + monkeypatch.setattr(llm_check, "build_planner_llm", fake_build) + return calls + + +def test_the_planner_scope_checks_only_the_planner_so_the_api_container_needs_no_agent_key( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + """Run in the api container, which holds no agent keys, the default check must not report the + agents' providers as failed.""" + calls = _record_builds(monkeypatch) + assert asyncio.run(llm_check.check_planner(_settings(_two_providers(tmp_path), "anthropic"))) is True + assert calls == ["anthropic"] + + +def test_the_agents_scope_checks_only_the_deployed_agents(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + calls = _record_builds(monkeypatch) + chain, config = _two_providers(tmp_path) + assert asyncio.run(llm_check.check_agents(chain, config)) is True + assert calls == ["openrouter"] + + +@pytest.mark.parametrize("argv", [["everything"], ["planner", "agents"]]) +def test_an_unknown_scope_prints_the_usage(argv: list[str], capsys: pytest.CaptureFixture[str]): + assert llm_check.main(argv) == 2 + assert "planner|agents|all" in capsys.readouterr().err + + +@respx.mock +def test_a_refused_key_is_reported_and_never_printed_even_when_the_provider_echoes_it( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +): + """The real client, not a fake: llm_check's output is read on a terminal and pasted into issues.""" + key = "sk-or-v1-feedfacefeedfacefeedfacefeedface" + monkeypatch.setenv("OPENROUTER_API_KEY", key) + respx.post("https://openrouter.ai/api/v1/chat/completions").respond(401, text=f"invalid key {key}") + agents_files = _write_agents( + tmp_path, + { + "openrouter": { + "kind": "openai_chat", + "base_url": "https://openrouter.ai/api/v1", + "api_key_env": "OPENROUTER_API_KEY", + } + }, + [], + ) + settings = _settings(agents_files, "openrouter", planner_model="model-a") + assert asyncio.run(llm_check.check_planner(settings)) is False out = capsys.readouterr().out - assert "HTTP 401" in out and "super-secret-token" not in out + assert "HTTP 401" in out and key not in out + + +@respx.mock +def test_a_probe_asks_for_a_small_answer_not_the_planners_16000_tokens( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + """OpenRouter checks the account's credit against max_tokens, so a probe asking for 16000 could fail + on a balance that real work would not.""" + monkeypatch.setenv("OPENROUTER_API_KEY", "k") + route = respx.post("https://openrouter.ai/api/v1/chat/completions").respond( + 200, + json={ + "model": "model-a", + "choices": [{"message": {"role": "assistant", "content": "ready"}, "finish_reason": "stop"}], + "usage": {"prompt_tokens": 5, "completion_tokens": 1}, + }, + ) + agents_files = _write_agents( + tmp_path, + { + "openrouter": { + "kind": "openai_chat", + "base_url": "https://openrouter.ai/api/v1", + "api_key_env": "OPENROUTER_API_KEY", + } + }, + [], + ) + asyncio.run(llm_check.check_planner(_settings(agents_files, "openrouter", planner_model="model-a"))) + sent = [json.loads(c.request.content) for c in route.calls] + assert sent and all(body["max_completion_tokens"] == 1024 for body in sent) diff --git a/control-plane/tests/test_meter_config.py b/control-plane/tests/test_meter_config.py index 37bd004..5294b8b 100644 --- a/control-plane/tests/test_meter_config.py +++ b/control-plane/tests/test_meter_config.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import tomllib from pathlib import Path import pytest @@ -73,7 +74,10 @@ def test_load_agent_spec_joins_chain_identity_with_model_config(tmp_path: Path): agents = tmp_path / "agents.json" agents.write_text(json.dumps({"agents": [{"agent_id": "1", "name": "w", "runtime_key": KEY_ADDRESS}]})) cfg = tmp_path / "agents.toml" - cfg.write_text('[agents.w]\nmodel = "m"\nsystem_prompt = "p"\n') + cfg.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + '[agents.w]\nprovider = "anthropic"\nmodel = "m"\nsystem_prompt = "p"\n' + ) spec = load_agent_spec("1", agents, cfg, {"MERGIT_AGENT_KEY_1": KEY}) assert (spec.model, spec.system_prompt, spec.runtime_key_address) == ("m", "p", KEY_ADDRESS) @@ -82,7 +86,10 @@ def test_load_agent_spec_errors(tmp_path: Path): agents = tmp_path / "agents.json" agents.write_text(json.dumps({"agents": [{"agent_id": "1", "name": "w", "runtime_key": "bad"}]})) cfg = tmp_path / "agents.toml" - cfg.write_text('[agents.w]\nmodel = "m"\nsystem_prompt = "p"\n') + cfg.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + '[agents.w]\nprovider = "anthropic"\nmodel = "m"\nsystem_prompt = "p"\n' + ) env = {"MERGIT_AGENT_KEY_1": KEY, "MERGIT_AGENT_KEY_2": KEY} with pytest.raises(ConfigError, match="runtime_key"): load_agent_spec("1", agents, cfg, env) @@ -102,6 +109,15 @@ def test_shipped_agents_toml_covers_the_example_agents(): assert all(f"[agents.{a['name']}]" in toml for a in registry) +def test_shipped_agents_are_priced(): + """Every model named in the repo's own `agents.toml` must be in the repo's own `prices.toml`, or the + worker refuses to start (`validate_agents_are_priced`).""" + prices = PriceTable.load(ROOT / "prices.toml") + agents = tomllib.loads((ROOT / "agents.toml").read_text())["agents"] + for cfg in agents.values(): + prices.price(cfg["model"]) + + BASE_ENV = { "DATABASE_URL": "postgres://u:p@h/db", "GATEWAY_URL": "http://gateway:8082/", diff --git a/control-plane/tests/test_openai_chat_llm.py b/control-plane/tests/test_openai_chat_llm.py new file mode 100644 index 0000000..0c6f769 --- /dev/null +++ b/control-plane/tests/test_openai_chat_llm.py @@ -0,0 +1,381 @@ +"""The worker's `OpenAIChatLLM` and the planner's async twin (any OpenAI-compatible chat API: Groq, +OpenRouter, ...), against the documented HTTP shape, faked with respx.""" + +from __future__ import annotations + +import asyncio +import json +from typing import Any + +import httpx +import pytest +import respx + +from mergit_cp import llm as cp_llm +from mergit_worker import llm as worker_llm +from mergit_worker.providers import ProviderSpec + +BASE_URL = "https://api.groq.com/openai/v1" +URL = f"{BASE_URL}/chat/completions" +TOOLS = [{"name": "finish", "description": "done", "input_schema": {"type": "object", "properties": {}}}] + + +def reply( + message: dict[str, Any], finish: str = "stop", usage: dict[str, Any] | None = None +) -> dict[str, Any]: + return { + "model": "openai/gpt-oss-120b", + "choices": [{"message": {"role": "assistant", **message}, "finish_reason": finish}], + "usage": usage or {"prompt_tokens": 12, "completion_tokens": 3}, + } + + +# -- worker (sync) ---------------------------------------------------------------------------- + + +@respx.mock +def test_worker_sends_the_documented_request_and_returns_anthropic_shaped_blocks(): + route = respx.post(URL).respond( + 200, + json=reply( + { + "content": "ok", + "tool_calls": [ + {"id": "c1", "type": "function", "function": {"name": "finish", "arguments": "{}"}} + ], + }, + finish="tool_calls", + ), + ) + llm = worker_llm.OpenAIChatLLM("test-key", base_url=BASE_URL, sleep=lambda _s: None) + result = llm.complete( + [{"role": "user", "content": "go"}], TOOLS, "openai/gpt-oss-120b", system="sys", max_tokens=99 + ) + + request = route.calls.last.request + assert request.headers["authorization"] == "Bearer test-key" + body = json.loads(request.content) + assert body["model"] == "openai/gpt-oss-120b" + assert body["max_completion_tokens"] == 99 and "max_tokens" not in body + assert body["messages"][0] == {"role": "system", "content": "sys"} + assert body["tools"][0]["function"]["name"] == "finish" and body["tool_choice"] == "auto" + + assert result.text == "ok" and result.stop_reason == "tool_use" + assert [(c.id, c.name, c.input) for c in result.tool_calls] == [("c1", "finish", {})] + assert result.assistant_content == [ + {"type": "text", "text": "ok"}, + {"type": "tool_use", "id": "c1", "name": "finish", "input": {}}, + ] + assert (result.input_tokens, result.output_tokens, result.model) == (12, 3, "openai/gpt-oss-120b") + + +@respx.mock +def test_worker_counts_cached_prompt_tokens_in_full_so_a_receipt_never_understates_cost(): + respx.post(URL).respond( + 200, + json=reply( + {"content": "x"}, + usage={ + "prompt_tokens": 100, + "completion_tokens": 1, + "prompt_tokens_details": {"cached_tokens": 60}, + }, + ), + ) + result = worker_llm.OpenAIChatLLM("k", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + assert result.input_tokens == 100 + + +@respx.mock +def test_worker_retries_rate_limits_then_succeeds(): + route = respx.post(URL).mock( + side_effect=[ + httpx.Response(429, headers={"retry-after": "1"}, json={}), + httpx.Response(200, json=reply({"content": "hi"})), + ] + ) + waits: list[float] = [] + result = worker_llm.OpenAIChatLLM("k", base_url=BASE_URL, sleep=waits.append).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + assert result.text == "hi" and route.call_count == 2 and waits == [1.0] + + +@respx.mock +def test_worker_gives_up_after_three_attempts(): + route = respx.post(URL).respond(503, json={}) + with pytest.raises(worker_llm.LLMError, match="503"): + worker_llm.OpenAIChatLLM("k", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + assert route.call_count == 3 + + +@respx.mock +def test_worker_does_not_retry_a_rejected_request_and_never_echoes_the_key(): + route = respx.post(URL).respond(400, json={"error": {"message": "bad field"}}) + with pytest.raises(worker_llm.LLMError) as caught: + worker_llm.OpenAIChatLLM("secret-key-123", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + assert route.call_count == 1 and "secret-key-123" not in str(caught.value) and "400" in str(caught.value) + + +@respx.mock +def test_worker_turns_a_connection_failure_into_an_llm_error(): + respx.post(URL).mock(side_effect=httpx.ConnectError("down")) + with pytest.raises(worker_llm.LLMError): + worker_llm.OpenAIChatLLM("k", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + + +@respx.mock +def test_error_object_in_200_is_llm_error(): + """Some OpenAI-compatible providers (OpenRouter among them) answer a routing failure with HTTP 200 and + an `error` object in the body instead of a non-200 status; that must still surface as `LLMError`, with + the provider's own message and never the key.""" + respx.post(URL).respond(200, json={"error": {"message": "No endpoints found", "code": 404}}) + with pytest.raises(worker_llm.LLMError) as caught: + worker_llm.OpenAIChatLLM("secret-key-123", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + assert "No endpoints found" in str(caught.value) + assert "secret-key-123" not in str(caught.value) + + +@respx.mock +def test_posts_to_configured_base_url(): + """`base_url` is a required constructor argument: this class is provider-agnostic, so a deploy pointed + at OpenRouter (or any other OpenAI-compatible endpoint) sends its request there, not to Groq.""" + openrouter_url = "https://openrouter.ai/api/v1/chat/completions" + route = respx.post(openrouter_url).respond(200, json=reply({"content": "ok"})) + worker_llm.OpenAIChatLLM("k", base_url="https://openrouter.ai/api/v1", sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + request = route.calls.last.request + assert str(request.url) == openrouter_url + assert request.headers["authorization"] == "Bearer k" + + +# -- planner (async) --------------------------------------------------------------------------- + + +def run(coro: Any) -> Any: + return asyncio.run(coro) + + +@respx.mock +def test_planner_asks_for_a_json_schema_answer_best_effort_and_splits_cached_tokens(): + route = respx.post(URL).respond( + 200, + json=reply( + {"content": '{"tasks": []}'}, + usage={ + "prompt_tokens": 100, + "completion_tokens": 8, + "prompt_tokens_details": {"cached_tokens": 40}, + }, + ), + ) + schema = {"type": "object", "properties": {"tasks": {"type": "array"}}} + result = run( + cp_llm.OpenAIChatLLM("k", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "plan"}], + None, + "openai/gpt-oss-120b", + system="s", + output_schema=schema, + ) + ) + body = json.loads(route.calls.last.request.content) + assert body["response_format"] == { + "type": "json_schema", + "json_schema": {"name": "answer", "strict": False, "schema": schema}, + } + assert result.text == '{"tasks": []}' + # input is the uncached part and the cached part is priced separately at its own rate + assert ( + result.input_tokens, + result.cache_read_tokens, + result.cache_write_tokens, + result.output_tokens, + ) == ( + 60, + 40, + 0, + 8, + ) + + +@respx.mock +def test_planner_reports_an_outage_as_unavailable_so_the_api_answers_502(): + respx.post(URL).respond(500, json={}) + with pytest.raises(cp_llm.LLMUnavailable): + run( + cp_llm.OpenAIChatLLM("k", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + ) + + +@respx.mock +def test_planner_error_object_in_200_is_llm_unavailable(): + """Mirrors the worker's `test_error_object_in_200_is_llm_error`: some OpenAI-compatible providers + (OpenRouter among them) answer a routing failure with HTTP 200 and an `error` object in the body instead + of a non-200 status; that must still surface as `LLMUnavailable`, with the provider's own message and + never the key.""" + respx.post(URL).respond(200, json={"error": {"message": "No endpoints found", "code": 404}}) + with pytest.raises(cp_llm.LLMUnavailable) as caught: + run( + cp_llm.OpenAIChatLLM("secret-key-123", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + ) + assert "No endpoints found" in str(caught.value) + assert "secret-key-123" not in str(caught.value) + + +@respx.mock +def test_planner_reports_a_truncated_answer_as_an_error_that_still_carries_its_usage(): + respx.post(URL).respond( + 200, + json=reply({"content": '{"ta'}, finish="length", usage={"prompt_tokens": 5, "completion_tokens": 9}), + ) + with pytest.raises(cp_llm.LLMError) as caught: + run( + cp_llm.OpenAIChatLLM("k", base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m" + ) + ) + assert caught.value.reason == "max_tokens" and caught.value.usage.output_tokens == 9 + + +_ANTHROPIC = {"anthropic": ProviderSpec(name="anthropic", kind="anthropic", base_url=None, api_key_env=None)} +_OPENROUTER = { + "openrouter": ProviderSpec( + name="openrouter", kind="openai_chat", base_url=BASE_URL, api_key_env="OPENROUTER_API_KEY" + ) +} + + +def test_the_planner_factory_picks_the_clients_kind_from_the_named_provider(): + assert isinstance( + cp_llm.build_planner_llm(_OPENROUTER, "openrouter", environ={"OPENROUTER_API_KEY": "k"}), + cp_llm.OpenAIChatLLM, + ) + assert isinstance( + cp_llm.build_planner_llm(_ANTHROPIC, "anthropic", environ={"ANTHROPIC_API_KEY": "a"}), + cp_llm.AnthropicLLM, + ) + + +def test_the_api_still_starts_with_no_model_key_so_sign_in_and_credits_work(): + # The API still starts and `build_planner_llm` still returns a client; a planning request then fails + # clearly with `LLMUnavailable`, which the API turns into a 502 (see tests/test_llm.py). + llm = cp_llm.build_planner_llm(_ANTHROPIC, "anthropic", environ={}) + with pytest.raises(cp_llm.LLMUnavailable): + run(llm.complete([{"role": "user", "content": "x"}], None, "m")) + + +def test_the_planner_accepts_a_gateway_token_without_warning(caplog: pytest.LogCaptureFixture): + env = {"ANTHROPIC_AUTH_TOKEN": "lend-token", "ANTHROPIC_BASE_URL": "https://lend.example"} + with caplog.at_level("WARNING", logger="mergit_cp.llm"): + assert isinstance(cp_llm.build_planner_llm(_ANTHROPIC, "anthropic", environ=env), cp_llm.AnthropicLLM) + assert "no model key" not in caplog.text + + +def test_an_unknown_planner_provider_is_a_clear_config_error_not_a_bare_key_error(): + """A misspelled `PLANNER_PROVIDER` on a real deployment must fail start-up with a message naming the + bad value and the valid choices, not a bare `KeyError`.""" + from mergit_worker.config import ConfigError + + with pytest.raises(ConfigError) as caught: + cp_llm.build_planner_llm(_ANTHROPIC, "antropic", environ={"ANTHROPIC_API_KEY": "a"}) + assert "antropic" in str(caught.value) + assert "anthropic" in str(caught.value) + + +# -- both clients: what a 200 body or an error body may contain ------------------------------------ + +KEY = "sk-or-v1-0123456789abcdef0123456789abcdef" + + +def _worker(**k: Any) -> Any: + return worker_llm.OpenAIChatLLM(KEY, base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m", **k + ) + + +def _planner(**k: Any) -> Any: + return run( + cp_llm.OpenAIChatLLM(KEY, base_url=BASE_URL, sleep=lambda _s: None).complete( + [{"role": "user", "content": "x"}], None, "m", **k + ) + ) + + +CLIENTS = [(_worker, worker_llm.LLMError), (_planner, cp_llm.LLMUnavailable)] + + +@pytest.mark.parametrize(("call", "_error"), CLIENTS) +@respx.mock +def test_a_rate_limit_reported_inside_a_200_is_retried(call: Any, _error: type[Exception]): + """OpenRouter reports an upstream 429 or 5xx as HTTP 200 with `{"error": {"code": 429}}`. That is the + same condition as a bare 429 and gets the same retries.""" + route = respx.post(URL).mock( + side_effect=[ + httpx.Response(200, json={"error": {"code": 429, "message": "Rate limit exceeded"}}), + httpx.Response(200, json=reply({"content": "ok"})), + ] + ) + assert call().text == "ok" + assert route.call_count == 2 + + +@pytest.mark.parametrize(("call", "error"), CLIENTS) +@respx.mock +def test_a_non_retryable_error_inside_a_200_is_not_retried(call: Any, error: type[Exception]): + route = respx.post(URL).respond(200, json={"error": {"code": 400, "message": "bad model"}}) + with pytest.raises(error, match="bad model"): + call() + assert route.call_count == 1 + + +@pytest.mark.parametrize(("call", "error"), CLIENTS) +@respx.mock +def test_an_error_with_no_message_says_so_and_never_none(call: Any, error: type[Exception]): + respx.post(URL).respond(200, json={"error": {"code": 418}}) + with pytest.raises(error) as caught: + call() + assert "None" not in str(caught.value) and "418" in str(caught.value) + + +@pytest.mark.parametrize("body", [[1, 2], "text", 7]) +@pytest.mark.parametrize(("call", "error"), CLIENTS) +@respx.mock +def test_a_body_that_is_json_but_not_an_object_is_a_clear_error(call: Any, error: type[Exception], body: Any): + """Before, `data.get("error")` on a list or a string raised a bare AttributeError.""" + respx.post(URL).respond(200, json=body) + with pytest.raises(error, match="not a JSON object"): + call() + + +@pytest.mark.parametrize(("call", "error"), CLIENTS) +@respx.mock +def test_a_provider_that_echoes_the_rejected_key_does_not_get_it_logged(call: Any, error: type[Exception]): + respx.post(URL).respond(401, text=f'{{"error": "invalid key {KEY}"}}') + with pytest.raises(error) as caught: + call() + assert "401" in str(caught.value) and KEY not in str(caught.value) and KEY[:20] not in str(caught.value) + + +@pytest.mark.parametrize(("call", "error"), CLIENTS) +@respx.mock +def test_an_echoed_key_inside_a_200_error_is_scrubbed_too(call: Any, error: type[Exception]): + respx.post(URL).respond(200, json={"error": {"code": 401, "message": f"No auth credentials {KEY}"}}) + with pytest.raises(error) as caught: + call() + assert KEY not in str(caught.value) diff --git a/control-plane/tests/test_outcomes.py b/control-plane/tests/test_outcomes.py index 21d70c4..a89fc71 100644 --- a/control-plane/tests/test_outcomes.py +++ b/control-plane/tests/test_outcomes.py @@ -32,6 +32,7 @@ store_url="http://store.invalid", agents_file="unused", planner_model="claude-opus-5", + planner_provider="anthropic", events_token=SecretStr("test-events-token"), chain_events_token=SecretStr("test-chain-events-token"), worker_token=SecretStr("test-worker-token"), diff --git a/control-plane/tests/test_plans.py b/control-plane/tests/test_plans.py index 827c94f..a2c6a1a 100644 --- a/control-plane/tests/test_plans.py +++ b/control-plane/tests/test_plans.py @@ -107,7 +107,13 @@ def settings(settings: Settings, tmp_path: Path) -> Settings: ) ) config = tmp_path / "two-agents.toml" - config.write_text("".join(f'[agents.agent-{i}]\nmodel = "m"\nsystem_prompt = "p"\n' for i in ("1", "2"))) + config.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + + "".join( + f'[agents.agent-{i}]\nprovider = "anthropic"\nmodel = "m"\nsystem_prompt = "p"\n' + for i in ("1", "2") + ) + ) return settings.model_copy(update={"agents_file": str(chain), "agents_config": str(config)}) diff --git a/control-plane/tests/test_preflight.py b/control-plane/tests/test_preflight.py new file mode 100644 index 0000000..1e0fa66 --- /dev/null +++ b/control-plane/tests/test_preflight.py @@ -0,0 +1,218 @@ +"""The deploy preflights (`python -m mergit_cp.preflight`, `python -m mergit_worker.preflight`), which +deploy/server/update.sh runs in the new image before it swaps a running container. Each must fail on a +configuration its service cannot start from, and pass on one it can.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from mergit_cp import preflight as api_preflight +from mergit_worker import preflight as worker_preflight + +ROOT = Path(__file__).resolve().parents[1] +MODEL_KEYS = ("ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "OPENROUTER_API_KEY", "GROQ_API_KEY") + + +@pytest.fixture +def agent_files(tmp_path: Path) -> tuple[Path, Path]: + chain = tmp_path / "agents.json" + chain.write_text( + '{"agents": [{"agent_id": "1", "name": "coder-a",' + ' "operator": "0x1111111111111111111111111111111111111111",' + ' "runtime_key": "0x2222222222222222222222222222222222222222",' + ' "metadata_uri": "ipfs://example"}]}' + ) + config = tmp_path / "agents.toml" + config.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + '[agents.coder-a]\nprovider = "anthropic"\nmodel = "claude-sonnet-5"\nsystem_prompt = "p"\n' + ) + return chain, config + + +@pytest.fixture +def clean_env(monkeypatch: pytest.MonkeyPatch) -> pytest.MonkeyPatch: + for k in MODEL_KEYS: + monkeypatch.delenv(k, raising=False) + return monkeypatch + + +def _worker_env(env: pytest.MonkeyPatch, files: tuple[Path, Path]) -> None: + for k, v in { + "DATABASE_URL": "postgres://u:p@h/db", # never connected to + "GATEWAY_URL": "http://gateway:8082", + "STORE_URL": "http://store:8081", + "EVENTS_TOKEN": "e", + "STORE_WRITE_TOKEN": "s", + "RECEIPT_REGISTRY": "0x0000000000000000000000000000000000000001", + "AGENTS_FILE": str(files[0]), + "AGENTS_CONFIG": str(files[1]), + "PRICES_FILE": str(ROOT / "prices.toml"), + }.items(): + env.setenv(k, v) + + +def _api_env(env: pytest.MonkeyPatch, files: tuple[Path, Path]) -> None: + for k, v in { + "DATABASE_URL": "postgres://u:p@h/db", # never connected to + "APP_DOMAIN": "app.mergit.test", + "SESSION_SECRET": "test-secret-at-least-32-bytes-long!!", + "GATEWAY_URL": "http://gateway.invalid", + "STORE_URL": "http://store.invalid", + "AGENTS_FILE": str(files[0]), + "AGENTS_CONFIG": str(files[1]), + "PRICES_FILE": str(ROOT / "prices.toml"), + "PLANNER_MODEL": "claude-haiku-4-5", + "PLANNER_PROVIDER": "anthropic", + "EVENTS_TOKEN": "e", + "CHAIN_EVENTS_TOKEN": "c", + "WORKER_TOKEN": "w", + }.items(): + env.setenv(k, v) + + +def test_worker_preflight_passes_when_every_deployed_agent_has_its_key( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + _worker_env(clean_env, agent_files) + clean_env.setenv("ANTHROPIC_API_KEY", "k") + assert worker_preflight.main() == 0 + assert "providers anthropic" in capsys.readouterr().out + + +def test_worker_preflight_fails_when_a_deployed_agents_provider_has_no_key( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + """The upgrade hazard: a server whose worker.env only has a Groq key, after the deployed agent moved + to provider = "anthropic". The new worker would refuse to start; the deploy must stop first.""" + _worker_env(clean_env, agent_files) + clean_env.setenv("GROQ_API_KEY", "g") + assert worker_preflight.main() == 1 + assert "ANTHROPIC_API_KEY" in capsys.readouterr().err + + +def test_worker_preflight_fails_on_a_missing_setting( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path] +): + _worker_env(clean_env, agent_files) + clean_env.setenv("ANTHROPIC_API_KEY", "k") + clean_env.delenv("STORE_WRITE_TOKEN") + assert worker_preflight.main() == 1 + + +def test_api_preflight_passes_on_a_configuration_the_api_starts_from( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + _api_env(clean_env, agent_files) + clean_env.setenv("ANTHROPIC_API_KEY", "k") + assert api_preflight.main() == 0 + assert "planner anthropic / claude-haiku-4-5" in capsys.readouterr().out + + +def test_api_preflight_fails_without_planner_provider( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + """An api.env written before PLANNER_PROVIDER existed: the new API would restart-loop.""" + _api_env(clean_env, agent_files) + clean_env.setenv("ANTHROPIC_API_KEY", "k") + clean_env.delenv("PLANNER_PROVIDER") + assert api_preflight.main() == 1 + assert "planner_provider" in capsys.readouterr().err.lower() + + +def test_api_preflight_fails_on_a_provider_agents_toml_does_not_define( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path] +): + _api_env(clean_env, agent_files) + clean_env.setenv("ANTHROPIC_API_KEY", "k") + clean_env.setenv("PLANNER_PROVIDER", "groq") + assert api_preflight.main() == 1 + + +def test_api_preflight_fails_when_the_planner_has_no_key( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + """The API itself starts without a planner key (planning answers 502), but a deploy should not ship + that by accident.""" + _api_env(clean_env, agent_files) + assert api_preflight.main() == 1 + assert "no model key" in capsys.readouterr().err + + +def test_api_preflight_fails_on_an_unpriced_planner_model( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path] +): + _api_env(clean_env, agent_files) + clean_env.setenv("ANTHROPIC_API_KEY", "k") + clean_env.setenv("PLANNER_MODEL", "no-such-model") + assert api_preflight.main() == 1 + + +SECRET = "s3cr3t-value-that-must-never-be-logged-0123456789" + + +def test_a_failed_api_preflight_names_the_setting_and_prints_no_value( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + """The preflight's output goes to the deploy's GitHub Actions log, which is public. Pydantic's own + message dumps the input, and the input is the environment.""" + _api_env(clean_env, agent_files) + clean_env.setenv("SESSION_SECRET", SECRET) + clean_env.setenv("DATABASE_URL", f"postgres://u:{SECRET}@h/db") + clean_env.setenv("ANTHROPIC_API_KEY", SECRET) + clean_env.delenv("PLANNER_PROVIDER") + assert api_preflight.main() == 1 + out = capsys.readouterr() + assert "PLANNER_PROVIDER" in out.err + assert SECRET[:12] not in out.err + out.out + + +def test_a_failed_worker_preflight_names_the_setting_and_prints_no_value( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + _worker_env(clean_env, agent_files) + clean_env.setenv("STORE_WRITE_TOKEN", SECRET) + clean_env.setenv("DATABASE_URL", f"postgres://u:{SECRET}@h/db") + clean_env.setenv("GATEWAY_URL", "ftp://gateway") + assert worker_preflight.main() == 1 + out = capsys.readouterr() + assert "GATEWAY_URL" in out.err + assert SECRET[:12] not in out.err + out.out + + +def _with_openrouter(files: tuple[Path, Path], agent_model: str) -> tuple[Path, Path]: + """The same roster with the agent on OpenRouter and both providers defined, as on the hosted site.""" + chain, config = files + config.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + '[providers.openrouter]\nkind = "openai_chat"\nbase_url = "https://openrouter.ai/api/v1"\n' + 'api_key_env = "OPENROUTER_API_KEY"\n\n' + f'[agents.coder-a]\nprovider = "openrouter"\nmodel = "{agent_model}"\nsystem_prompt = "p"\n' + ) + return chain, config + + +def test_api_preflight_refuses_a_native_planner_model_on_openrouter( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + """The upgrade trap: PLANNER_PROVIDER switched to openrouter while PLANNER_MODEL keeps the native id. + Both are priced and the key is there, so only this check stops every plan from failing at OpenRouter.""" + _api_env(clean_env, _with_openrouter(agent_files, "anthropic/claude-sonnet-5")) + clean_env.setenv("OPENROUTER_API_KEY", "k") + clean_env.setenv("PLANNER_PROVIDER", "openrouter") + clean_env.setenv("PLANNER_MODEL", "claude-haiku-4-5") + assert api_preflight.main() == 1 + assert "not an OpenRouter id" in capsys.readouterr().err + clean_env.setenv("PLANNER_MODEL", "anthropic/claude-haiku-4.5") + assert api_preflight.main() == 0 + + +def test_worker_preflight_refuses_a_native_agent_model_on_openrouter( + clean_env: pytest.MonkeyPatch, agent_files: tuple[Path, Path], capsys: pytest.CaptureFixture[str] +): + _worker_env(clean_env, _with_openrouter(agent_files, "claude-sonnet-5")) + clean_env.setenv("OPENROUTER_API_KEY", "k") + assert worker_preflight.main() == 1 + assert "not an OpenRouter id" in capsys.readouterr().err diff --git a/control-plane/tests/test_providers.py b/control-plane/tests/test_providers.py index be73aa1..43b293c 100644 --- a/control-plane/tests/test_providers.py +++ b/control-plane/tests/test_providers.py @@ -1,4 +1,5 @@ -"""Which model provider runs, decided once at start-up, and the model swapped in before anything is priced.""" +"""The provider each agent and the planner use, configured once in `agents.toml` (or, for the planner, in +`Settings.planner_provider`).""" from __future__ import annotations @@ -7,76 +8,195 @@ import pytest -from mergit_worker.agents import load_agent_spec, validate_agents_are_priced +from mergit_worker.agents import ( + load_agent_spec, + validate_agents_are_priced, + validate_agents_have_keys, +) from mergit_worker.config import ConfigError from mergit_worker.meter import PriceTable -from mergit_worker.providers import DEFAULT_GROQ_MODEL, effective_model, llm_provider, require_provider +from mergit_worker.providers import ProviderSpec, check_model_id, load_providers, provider_key, stale_settings from tests.support import KEY, KEY_ADDRESS ROOT = Path(__file__).resolve().parents[1] -def test_anthropic_is_the_provider_whenever_its_key_is_set(): - assert llm_provider({"ANTHROPIC_API_KEY": "a", "GROQ_API_KEY": "g"}) == "anthropic" +def test_provider_key_falls_back_to_anthropics_own_env_vars_when_no_api_key_env_is_named(): + # A provider with no `api_key_env` (the common single-Anthropic-provider deploy) lets the credential + # come from whichever of ANTHROPIC_API_KEY/ANTHROPIC_AUTH_TOKEN is set, exactly as the SDK itself would + # read it; a gateway such as Claude Lend hands out ANTHROPIC_AUTH_TOKEN (+ ANTHROPIC_BASE_URL) instead + # of a real key, and it must still be picked up. + spec = ProviderSpec(name="anthropic", kind="anthropic", base_url=None, api_key_env=None) + assert provider_key(spec, {"ANTHROPIC_AUTH_TOKEN": "t"}) == "t" + assert provider_key(spec, {"ANTHROPIC_API_KEY": "a", "ANTHROPIC_AUTH_TOKEN": "t"}) == "a" + assert provider_key(spec, {}) is None -def test_an_anthropic_auth_token_counts_as_the_anthropic_credential(): - # A gateway such as Claude Lend takes ANTHROPIC_AUTH_TOKEN (+ ANTHROPIC_BASE_URL) instead of a real key. - # It must win over a Groq key left in the same file, or the deploy would silently keep using Groq. - assert llm_provider({"ANTHROPIC_AUTH_TOKEN": "t", "GROQ_API_KEY": "g"}) == "anthropic" - assert require_provider({"ANTHROPIC_AUTH_TOKEN": "t"}) == "anthropic" - assert require_provider({"ANTHROPIC_AUTH_TOKEN": "t", "GROQ_API_KEY": "g"}) == "anthropic" - # A blank value is not a credential. - assert llm_provider({"ANTHROPIC_AUTH_TOKEN": "", "GROQ_API_KEY": "g"}) == "groq" - with pytest.raises(ConfigError, match="ANTHROPIC_API_KEY or ANTHROPIC_AUTH_TOKEN"): - require_provider({"LLM_PROVIDER": "anthropic", "ANTHROPIC_AUTH_TOKEN": ""}) +def test_provider_key_uses_the_named_env_var_when_one_is_given(): + spec = ProviderSpec( + name="openrouter", + kind="openai_chat", + base_url="https://openrouter.ai/api/v1", + api_key_env="OPENROUTER_API_KEY", + ) + assert provider_key(spec, {"OPENROUTER_API_KEY": "k", "ANTHROPIC_API_KEY": "a"}) == "k" + assert provider_key(spec, {}) is None -def test_groq_is_the_fallback_when_only_its_key_is_set(): - assert llm_provider({"GROQ_API_KEY": "g"}) == "groq" +def test_the_repo_price_table_prices_the_groq_models_from_groqs_published_prices(): + table = PriceTable.load(ROOT / "prices.toml") + # $0.15 in + $0.60 out per Mtok, openai/gpt-oss-120b (console.groq.com/docs/models, 2026-10-30) + assert table.model_call_cost("openai/gpt-oss-120b", 1_000_000, 1_000_000, 5).usd_micros == 750_000 + assert table.model_call_cost("openai/gpt-oss-20b", 1_000_000, 1_000_000, 5).usd_micros == 375_000 + # cached input costs 50% less (console.groq.com/docs/prompt-caching) + cached = table.model_call_cost("openai/gpt-oss-120b", 0, 0, 5, cache_read_tokens=1_000_000) + assert cached.usd_micros == 75_000 -def test_with_no_keys_the_default_is_anthropic_so_nothing_changes_for_tests_or_a_keyed_deploy(): - assert llm_provider({}) == "anthropic" +def test_loads_openai_chat_and_anthropic_providers(tmp_path: Path): + config = tmp_path / "agents.toml" + config.write_text( + "[providers.openrouter]\n" + 'kind = "openai_chat"\n' + 'base_url = "https://openrouter.ai/api/v1"\n' + 'api_key_env = "OPENROUTER_API_KEY"\n' + "\n" + "[providers.anthropic]\n" + 'kind = "anthropic"\n' + ) + assert load_providers(config) == { + "openrouter": ProviderSpec( + name="openrouter", + kind="openai_chat", + base_url="https://openrouter.ai/api/v1", + api_key_env="OPENROUTER_API_KEY", + ), + "anthropic": ProviderSpec(name="anthropic", kind="anthropic", base_url=None, api_key_env=None), + } + + +def test_openai_chat_requires_base_url_and_key_env(tmp_path: Path): + config = tmp_path / "agents.toml" + config.write_text('[providers.openrouter]\nkind = "openai_chat"\napi_key_env = "OPENROUTER_API_KEY"\n') + with pytest.raises(ConfigError, match="openrouter"): + load_providers(config) + config.write_text( + '[providers.openrouter]\nkind = "openai_chat"\nbase_url = "https://openrouter.ai/api/v1"\n' + ) + with pytest.raises(ConfigError, match="openrouter"): + load_providers(config) -def test_an_explicit_choice_wins_over_the_keys_present(): - assert llm_provider({"LLM_PROVIDER": "groq", "ANTHROPIC_API_KEY": "a", "GROQ_API_KEY": "g"}) == "groq" - assert llm_provider({"LLM_PROVIDER": "anthropic", "GROQ_API_KEY": "g"}) == "anthropic" +def test_unknown_kind_is_refused(tmp_path: Path): + config = tmp_path / "agents.toml" + config.write_text('[providers.aws]\nkind = "bedrock"\n') + with pytest.raises(ConfigError, match="bedrock"): + load_providers(config) -def test_an_unknown_provider_name_is_an_error(): - with pytest.raises(ConfigError, match="LLM_PROVIDER"): - llm_provider({"LLM_PROVIDER": "openia"}) +def test_a_malformed_provider_entry_is_a_clear_config_error_not_a_bare_attribute_error(tmp_path: Path): + """`openrouter = "not-a-table"` under `[providers]` is valid TOML but not a provider table; this must + raise `ConfigError` naming the malformed provider, not a raw `AttributeError` from calling `.get` on a + plain string.""" + config = tmp_path / "agents.toml" + config.write_text('[providers]\nopenrouter = "not-a-table"\n') + with pytest.raises(ConfigError, match="openrouter"): + load_providers(config) -def test_require_provider_refuses_to_start_without_the_key_it_needs(): - with pytest.raises(ConfigError, match="ANTHROPIC_API_KEY, ANTHROPIC_AUTH_TOKEN or GROQ_API_KEY"): - require_provider({}) - with pytest.raises(ConfigError, match="GROQ_API_KEY"): - require_provider({"LLM_PROVIDER": "groq", "ANTHROPIC_API_KEY": "a"}) - assert require_provider({"GROQ_API_KEY": "g"}) == "groq" +def _openai_chat(tmp_path: Path, base_url: object, api_key_env: object = "OPENROUTER_API_KEY") -> Path: + def toml(v: object) -> str: + return f'"{v}"' if isinstance(v, str) else str(v).lower() + config = tmp_path / "agents.toml" + config.write_text( + '[providers.p]\nkind = "openai_chat"\n' + f"base_url = {toml(base_url)}\napi_key_env = {toml(api_key_env)}\n" + ) + return config + + +def test_the_repo_agents_toml_passes_its_own_checks(): + assert set(load_providers(ROOT / "agents.toml")) >= {"anthropic", "groq", "openrouter"} + + +@pytest.mark.parametrize( + "base_url", + ["https://openrouter.ai/api/v1", "https://api.groq.com/openai/v1", "http://keyrelay:8787/v1"], +) +def test_a_known_provider_endpoint_is_accepted(tmp_path: Path, base_url: str): + assert load_providers(_openai_chat(tmp_path, base_url))["p"].base_url == base_url + + +@pytest.mark.parametrize( + "base_url", + [ + "https://attacker.example/v1", # any host: the key goes out as a Bearer token + "https://openrouter.ai.attacker.example/v1", # a suffix is not the host + "https://attacker.example@openrouter.ai/v1", # credentials in the URL + "http://openrouter.ai/api/v1", # cleartext to a real provider + "https://openrouter.ai:8443/api/v1", # a non-default port + "http://keyrelay:9999/v1", # keyrelay on another port + "https://keyrelay:8787/v1", # keyrelay is http inside compose, not https + "file:///etc/passwd", + "openrouter.ai/api/v1", # no scheme + ], +) +def test_a_base_url_outside_the_allowlist_is_refused(tmp_path: Path, base_url: str): + """`OpenAIChatLLM` sends the provider key to `base_url` as a Bearer token, and `agents.toml` ships with + every push to main, so an edit to it must not be able to point a key at an arbitrary host.""" + with pytest.raises(ConfigError, match="base_url"): + load_providers(_openai_chat(tmp_path, base_url)) + + +@pytest.mark.parametrize( + "api_key_env", + [ + "STORE_WRITE_TOKEN", + "GITHUB_TOKEN", + "WORKER_TOKEN", + "SESSION_SECRET", + "DATABASE_URL", + "TREASURY_KEY", + "MERGIT_AGENT_KEY_1", + "MERGIT_EXTRA_API_KEY", # the MERGIT_ namespace is never a model provider's key + "openrouter_api_key", + "API_KEY", + "", + ], +) +def test_api_key_env_must_name_a_model_provider_key_not_a_mergit_secret(tmp_path: Path, api_key_env: str): + """Whatever `api_key_env` names is read from the worker's or the API's environment and sent to the + provider, so it must not be able to name one of Mergit's own secrets.""" + with pytest.raises(ConfigError, match="api_key_env"): + load_providers(_openai_chat(tmp_path, "https://openrouter.ai/api/v1", api_key_env)) + + +def test_an_anthropic_provider_may_not_set_a_base_url_it_would_ignore(tmp_path: Path): + config = tmp_path / "agents.toml" + config.write_text('[providers.a]\nkind = "anthropic"\nbase_url = "https://attacker.example"\n') + with pytest.raises(ConfigError, match="base_url"): + load_providers(config) -def test_the_configured_model_is_kept_on_anthropic(): - assert effective_model("claude-sonnet-5", {"ANTHROPIC_API_KEY": "a"}) == "claude-sonnet-5" +def test_an_anthropic_provider_key_env_follows_the_same_rule(tmp_path: Path): + config = tmp_path / "agents.toml" + config.write_text('[providers.a]\nkind = "anthropic"\napi_key_env = "GITHUB_TOKEN"\n') + with pytest.raises(ConfigError, match="api_key_env"): + load_providers(config) + config.write_text('[providers.a]\nkind = "anthropic"\napi_key_env = "SECOND_ANTHROPIC_API_KEY"\n') + assert load_providers(config)["a"].api_key_env == "SECOND_ANTHROPIC_API_KEY" -def test_on_groq_every_model_becomes_the_groq_model_and_it_can_be_chosen(): - assert effective_model("claude-sonnet-5", {"GROQ_API_KEY": "g"}) == DEFAULT_GROQ_MODEL - assert effective_model("claude-opus-5", {"GROQ_API_KEY": "g", "GROQ_MODEL": "openai/gpt-oss-20b"}) == ( - "openai/gpt-oss-20b" - ) - -def test_the_repo_price_table_prices_the_groq_models_from_groqs_published_prices(): - table = PriceTable.load(ROOT / "prices.toml") - # $0.15 in + $0.60 out per Mtok, openai/gpt-oss-120b (console.groq.com/docs/models, 2026-10-30) - assert table.model_call_cost("openai/gpt-oss-120b", 1_000_000, 1_000_000, 5).usd_micros == 750_000 - assert table.model_call_cost("openai/gpt-oss-20b", 1_000_000, 1_000_000, 5).usd_micros == 375_000 - # cached input costs 50% less (console.groq.com/docs/prompt-caching) - cached = table.model_call_cost("openai/gpt-oss-120b", 0, 0, 5, cache_read_tokens=1_000_000) - assert cached.usd_micros == 75_000 +@pytest.mark.parametrize("field", ["base_url", "api_key_env"]) +def test_a_non_string_field_is_a_config_error_not_an_attribute_error(tmp_path: Path, field: str): + values: dict[str, object] = { + "base_url": "https://openrouter.ai/api/v1", + "api_key_env": "OPENROUTER_API_KEY", + } + values[field] = 42 + with pytest.raises(ConfigError, match=field): + load_providers(_openai_chat(tmp_path, values["base_url"], values["api_key_env"])) def _agent_files(tmp_path: Path) -> tuple[Path, Path]: @@ -96,74 +216,124 @@ def _agent_files(tmp_path: Path) -> tuple[Path, Path]: ) ) config = tmp_path / "agents.toml" - config.write_text('[agents.coder]\nmodel = "claude-sonnet-5"\nsystem_prompt = "p"\n') + config.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + '[agents.coder]\nprovider = "anthropic"\nmodel = "claude-sonnet-5"\nsystem_prompt = "p"\n' + ) return chain, config -def test_the_agent_is_loaded_with_the_groq_model_so_the_receipt_prices_and_names_what_really_ran( - tmp_path: Path, -): +def test_model_is_never_swapped(tmp_path: Path): chain, config = _agent_files(tmp_path) env = {"GROQ_API_KEY": "g", "MERGIT_AGENT_KEY_1": KEY} - assert load_agent_spec("1", chain, config, env).model == DEFAULT_GROQ_MODEL - env_a = {"ANTHROPIC_API_KEY": "a", "MERGIT_AGENT_KEY_1": KEY} - assert load_agent_spec("1", chain, config, env_a).model == "claude-sonnet-5" + assert load_agent_spec("1", chain, config, env).model == "claude-sonnet-5" -def test_start_up_price_check_uses_the_model_that_will_run(tmp_path: Path): +def test_start_up_price_check_uses_the_configured_model(tmp_path: Path): chain, config = _agent_files(tmp_path) - only_groq = tmp_path / "p.toml" - only_groq.write_text(f'[models."{DEFAULT_GROQ_MODEL}"]\ninput_per_mtok = 0.15\noutput_per_mtok = 0.6\n') - table = PriceTable.load(only_groq) - validate_agents_are_priced(chain, config, table, {"GROQ_API_KEY": "g"}) + priced = tmp_path / "p.toml" + priced.write_text('[models."claude-sonnet-5"]\ninput_per_mtok = 1\noutput_per_mtok = 1\n') + validate_agents_are_priced(chain, config, PriceTable.load(priced)) with pytest.raises(ConfigError, match="no price"): - validate_agents_are_priced(chain, config, table, {"ANTHROPIC_API_KEY": "a"}) + validate_agents_are_priced(chain, config, PriceTable({})) -def _only(monkeypatch: pytest.MonkeyPatch, **keys: str) -> None: - for name in ("ANTHROPIC_API_KEY", "GROQ_API_KEY", "LLM_PROVIDER", "GROQ_MODEL"): - monkeypatch.delenv(name, raising=False) - for name, value in keys.items(): - monkeypatch.setenv(name, value) +GROQ = ProviderSpec("groq", "openai_chat", "https://api.groq.com/openai/v1", "GROQ_API_KEY") -def test_the_api_lists_the_model_that_will_really_run( - agents_files: tuple[Path, Path], monkeypatch: pytest.MonkeyPatch -): - from mergit_cp.agents import load_agents +def test_removed_settings_still_set_are_named_for_a_warning(): + env = {"GROQ_MODEL": "x", "LLM_PROVIDER": "groq", "GROQ_API_KEY": "k"} + assert stale_settings({"groq": GROQ}, {"groq"}, env) == ["GROQ_MODEL", "LLM_PROVIDER"] + assert stale_settings({"groq": GROQ}, {"groq"}, {}) == [] - chain, config = agents_files - _only(monkeypatch, GROQ_API_KEY="g") - assert load_agents(str(chain), str(config))[0].model == DEFAULT_GROQ_MODEL - _only(monkeypatch, ANTHROPIC_API_KEY="a") - assert load_agents(str(chain), str(config))[0].model == "claude-sonnet-5" +def test_a_keyrelay_env_file_cannot_send_its_proxy_token_to_groq_itself(): + """An env file written for keyrelay holds its proxy token in GROQ_API_KEY and keyrelay's address in + GROQ_BASE_URL. That variable is ignored now, so a provider in use would send the token to agents.toml's + endpoint instead: refused, not warned about.""" + env = {"GROQ_BASE_URL": "http://keyrelay:8787/v1", "GROQ_API_KEY": "proxy-token"} + with pytest.raises(ConfigError, match="GROQ_BASE_URL"): + stale_settings({"groq": GROQ}, {"groq"}, env) + + +def test_a_stale_groq_base_url_only_warns_when_no_provider_in_use_reads_groq_api_key(): + env = {"GROQ_BASE_URL": "http://keyrelay:8787/v1"} + assert stale_settings({"groq": GROQ}, {"anthropic"}, env) == ["GROQ_BASE_URL"] -def test_the_planner_model_follows_the_provider( - agents_files: tuple[Path, Path], monkeypatch: pytest.MonkeyPatch -): - from pydantic import SecretStr - - from mergit_cp.config import Settings - - chain, config = agents_files - - def settings() -> Settings: - return Settings( - database_url="postgresql://x", - app_domain="app.mergit.test", - session_secret=SecretStr("test-secret-at-least-32-bytes-long!!"), - gateway_url="http://gateway.invalid", - store_url="http://store.invalid", - agents_file=str(chain), - agents_config=str(config), - planner_model="claude-opus-5", - events_token=SecretStr("e"), - chain_events_token=SecretStr("c"), - worker_token=SecretStr("w"), - ) - _only(monkeypatch, ANTHROPIC_API_KEY="a") - assert settings().planner_model == "claude-opus-5" - _only(monkeypatch, GROQ_API_KEY="g", GROQ_MODEL="openai/gpt-oss-20b") - assert settings().planner_model == "openai/gpt-oss-20b" +def test_a_stale_groq_base_url_that_agrees_with_agents_toml_only_warns(): + relay = ProviderSpec("groq", "openai_chat", "http://keyrelay:8787/v1", "GROQ_API_KEY") + env = {"GROQ_BASE_URL": "http://keyrelay:8787/v1/"} + assert stale_settings({"groq": relay}, {"groq"}, env) == ["GROQ_BASE_URL"] + + +def _deploy(tmp_path: Path, registry: object, toml: str) -> tuple[Path, Path]: + chain = tmp_path / "agents.json" + chain.write_text(json.dumps({"agents": registry})) + config = tmp_path / "agents.toml" + config.write_text(toml) + return chain, config + + +_GOOD_ENTRY = {"agent_id": "1", "name": "coder", "operator": "0x" + "1" * 40, "runtime_key": KEY_ADDRESS} + + +def _every_reader(chain: Path, config: Path) -> list[object]: + from mergit_cp.llm_check import _agent_pairs_in_use # pyright: ignore[reportPrivateUsage] + + return [ + lambda: validate_agents_have_keys(chain, config), + lambda: validate_agents_are_priced(chain, config, PriceTable.load(ROOT / "prices.toml")), + lambda: load_agent_spec("1", chain, config, {"MERGIT_AGENT_KEY_1": KEY}), + lambda: _agent_pairs_in_use(chain, config), + ] + + +@pytest.mark.parametrize("index", range(4)) +def test_an_agent_entry_that_is_not_a_table_is_a_config_error_for_every_reader(tmp_path: Path, index: int): + """`coder = "claude"` under [agents] is valid TOML. Every reader of the deploy used to call `.get` on + it and crash with AttributeError; they now share one reader that names the bad entry.""" + chain, config = _deploy(tmp_path, [_GOOD_ENTRY], '[agents]\ncoder = "claude"\n') + with pytest.raises(ConfigError, match="coder"): + _every_reader(chain, config)[index]() # type: ignore[operator] + + +@pytest.mark.parametrize("index", range(4)) +def test_a_registry_entry_that_is_not_an_object_is_a_config_error_for_every_reader( + tmp_path: Path, index: int +): + chain, config = _deploy(tmp_path, ["coder"], '[agents.coder]\nprovider = "anthropic"\n') + with pytest.raises(ConfigError, match="object"): + _every_reader(chain, config)[index]() # type: ignore[operator] + + +OPENROUTER = ProviderSpec("openrouter", "openai_chat", "https://openrouter.ai/api/v1", "OPENROUTER_API_KEY") +ANTHROPIC = ProviderSpec("anthropic", "anthropic", None, None) + + +@pytest.mark.parametrize( + ("spec", "model"), + [ + (OPENROUTER, "anthropic/claude-haiku-4.5"), + (OPENROUTER, "deepseek/deepseek-v4-flash"), + (ANTHROPIC, "claude-haiku-4-5"), + (GROQ, "openai/gpt-oss-120b"), # Groq ids may or may not have a slash: no rule + (GROQ, "llama-3.3-70b-versatile"), + ], +) +def test_a_model_id_the_provider_serves_passes(spec: ProviderSpec, model: str): + check_model_id(spec, model) + + +@pytest.mark.parametrize( + ("spec", "model"), + [ + (OPENROUTER, "claude-haiku-4-5"), # a native id left over after switching the planner to OpenRouter + (ANTHROPIC, "anthropic/claude-haiku-4.5"), # the other way round + ], +) +def test_a_model_id_from_the_other_provider_is_refused(spec: ProviderSpec, model: str): + """Both ids are priced and a key would be present, so nothing else at start-up catches this: every plan or + job would fail at the provider instead.""" + with pytest.raises(ConfigError, match=model.replace(".", r"\.")): + check_model_id(spec, model) diff --git a/control-plane/tests/test_routes_routing.py b/control-plane/tests/test_routes_routing.py index 5400dfc..0495f2b 100644 --- a/control-plane/tests/test_routes_routing.py +++ b/control-plane/tests/test_routes_routing.py @@ -237,6 +237,7 @@ def roster_agent(agent_id: str) -> Agent: name=f"agent-{agent_id}", operator=OPERATORS[agent_id], runtime_key="0x" + "2" * 40, + provider="anthropic", model="m", system_prompt="p", ) @@ -257,7 +258,13 @@ def roster(settings: Settings, tmp_path: Path) -> Settings: ) ) config = tmp_path / "five-agents.toml" - config.write_text("".join(f'[agents.agent-{i}]\nmodel = "m"\nsystem_prompt = "p"\n' for i in OPERATORS)) + config.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + + "".join( + f'[agents.agent-{i}]\nprovider = "anthropic"\nmodel = "m"\nsystem_prompt = "p"\n' + for i in OPERATORS + ) + ) return settings.model_copy(update={"agents_file": str(chain), "agents_config": str(config)}) diff --git a/control-plane/tests/test_worker.py b/control-plane/tests/test_worker.py index 15af789..8013180 100644 --- a/control-plane/tests/test_worker.py +++ b/control-plane/tests/test_worker.py @@ -2,13 +2,16 @@ import json import threading +from dataclasses import dataclass from pathlib import Path from typing import Any import pytest -from mergit_worker.config import WorkerConfig +from mergit_worker.config import ConfigError, WorkerConfig +from mergit_worker.llm import ModelClients from mergit_worker.models import Job, RunOutcome +from mergit_worker.providers import ProviderSpec from mergit_worker.worker import Worker, WorkerDeps from tests.support import ( FIX, @@ -51,7 +54,10 @@ def cfg(tmp_path: Path) -> WorkerConfig: agents = tmp_path / "agents.json" agents.write_text(json.dumps({"agents": [{"agent_id": "1", "name": "w", "runtime_key": KEY_ADDRESS}]})) toml = tmp_path / "agents.toml" - toml.write_text('[agents.w]\nmodel = "test-model"\nsystem_prompt = "p"\n') + toml.write_text( + '[providers.anthropic]\nkind = "anthropic"\n\n' + '[agents.w]\nprovider = "anthropic"\nmodel = "test-model"\nsystem_prompt = "p"\n' + ) return WorkerConfig( database_url="postgres://x", gateway_url="http://g", @@ -65,6 +71,28 @@ def cfg(tmp_path: Path) -> WorkerConfig: ) +@dataclass +class _SingleModel: + """A `WorkerDeps.models` stand-in that answers every provider with the same fake `LLM`, for tests that + don't care which provider name was asked for.""" + + llm: Any + + def for_provider(self, _name: str) -> Any: + return self.llm + + +@dataclass +class _RecordingModels: + """A `WorkerDeps.models` stand-in backed by one fake `LLM` per provider name, so a test can see which + one a job actually used.""" + + clients: dict[str, Any] + + def for_provider(self, name: str) -> Any: + return self.clients[name] + + def make_worker(cfg: WorkerConfig, store: FakeStore, llm: Any, tmp_path: Path, made: list[ProcessSandbox]): def factory(job: Job) -> ProcessSandbox: sbx = ProcessSandbox.create(job.repo_url, job.ref, tmp_path / "ws") @@ -73,7 +101,7 @@ def factory(job: Job) -> ProcessSandbox: deps = WorkerDeps( store=store, - llm=llm, + models=_SingleModel(llm), prices=make_prices(), publisher=RecordingPublisher(sealed=[]), make_sandbox=factory, @@ -111,7 +139,7 @@ def test_a_failed_claim_does_not_crash_the_worker(cfg: WorkerConfig): process down with it; the poll loop backs off and tries again.""" deps = WorkerDeps( store=_RaisingStore(), - llm=ScriptedLLM([]), + models=_SingleModel(ScriptedLLM([])), prices=make_prices(), publisher=RecordingPublisher(sealed=[]), make_sandbox=_unused_sandbox, @@ -125,7 +153,7 @@ def test_a_crashing_pr_queue_does_not_stop_job_processing(cfg: WorkerConfig): its next turn.""" deps = WorkerDeps( store=FakeStore([]), - llm=ScriptedLLM([]), + models=_SingleModel(ScriptedLLM([])), prices=make_prices(), publisher=RecordingPublisher(sealed=[]), make_sandbox=_unused_sandbox, @@ -237,3 +265,115 @@ def test_run_forever_stops_when_asked(cfg: WorkerConfig, tmp_path: Path): stop.set() t.join(timeout=5) assert not t.is_alive() + + +def test_each_job_uses_its_agents_provider(tmp_path: Path, repo_url: str, monkeypatch: pytest.MonkeyPatch): + """Two agents naming different providers must each reach their own model client: a job for agent 1 + never calls provider 2's client, and vice versa.""" + monkeypatch.setenv("MERGIT_AGENT_KEY_1", KEY) + monkeypatch.setenv("MERGIT_AGENT_KEY_2", KEY) + agents = tmp_path / "agents.json" + agents.write_text( + json.dumps( + { + "agents": [ + {"agent_id": "1", "name": "a1", "runtime_key": KEY_ADDRESS}, + {"agent_id": "2", "name": "a2", "runtime_key": KEY_ADDRESS}, + ] + } + ) + ) + toml = tmp_path / "agents.toml" + toml.write_text( + '[providers.p1]\nkind = "anthropic"\n\n' + '[providers.p2]\nkind = "anthropic"\n\n' + '[agents.a1]\nprovider = "p1"\nmodel = "test-model"\nsystem_prompt = "p"\n' + '[agents.a2]\nprovider = "p2"\nmodel = "test-model"\nsystem_prompt = "p"\n' + ) + cfg = WorkerConfig( + database_url="postgres://x", + gateway_url="http://g", + store_url="http://s", # type: ignore[arg-type] + events_token="e", + store_write_token="s", + receipt_registry=REGISTRY, # type: ignore[arg-type] + agents_file=agents, + agents_config=toml, + poll_interval_s=0.01, + ) + + llm1, llm2 = ScriptedLLM([turn(("finish", {}))]), ScriptedLLM([turn(("finish", {}))]) + models = _RecordingModels({"p1": llm1, "p2": llm2}) + made: list[ProcessSandbox] = [] + + def factory(job: Job) -> ProcessSandbox: + sbx = ProcessSandbox.create(job.repo_url, job.ref, tmp_path / f"ws-{job.agent_id}") + made.append(sbx) + return sbx + + store = FakeStore([make_job(repo_url, agent_id="1"), make_job(repo_url, agent_id="2")]) + deps = WorkerDeps( + store=store, + models=models, + prices=make_prices(), + publisher=RecordingPublisher(sealed=[]), + make_sandbox=factory, + harness=make_harness_config(), + ) + worker = Worker(cfg, deps) + + assert worker.run_once() is True + assert (llm1.calls, llm2.calls) == (1, 0) + assert worker.run_once() is True + assert (llm1.calls, llm2.calls) == (1, 1) + assert store.failures == [] + + +def test_missing_key_for_deployed_agent_refuses_start(): + """A provider every deployed agent needs, with no key for it, is caught once at start-up: the error + names the provider and the environment variable it needs, never leaving that discovery for a job's + first model call.""" + providers = { + "openrouter": ProviderSpec( + name="openrouter", + kind="openai_chat", + base_url="https://openrouter.ai/api/v1", + api_key_env="OPENROUTER_API_KEY", + ) + } + with pytest.raises(ConfigError, match="OPENROUTER_API_KEY"): + ModelClients.build(providers, {"openrouter"}, timeout_s=1.0, environ={}) + ModelClients.build(providers, set(), timeout_s=1.0, environ={}) # no agent needs it: does not raise + + +def test_two_anthropic_providers_each_use_their_own_key(): + """Two `[providers.*]` tables can both be `kind = "anthropic"` with different `api_key_env` values. + Start-up checks that each one's own key is present (`provider_key`); the client built for each must then + actually send that provider's key, not whichever of `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN` the SDK + would pick for itself if neither were passed explicitly.""" + providers = { + "claude-a": ProviderSpec( + name="claude-a", kind="anthropic", base_url=None, api_key_env="CLAUDE_A_KEY" + ), + "claude-b": ProviderSpec( + name="claude-b", kind="anthropic", base_url=None, api_key_env="CLAUDE_B_KEY" + ), + } + env = {"CLAUDE_A_KEY": "key-a", "CLAUDE_B_KEY": "key-b"} + models = ModelClients.build(providers, {"claude-a", "claude-b"}, timeout_s=1.0, environ=env) + assert models.for_provider("claude-a")._client.api_key == "key-a" # type: ignore[attr-defined] + assert models.for_provider("claude-b")._client.api_key == "key-b" # type: ignore[attr-defined] + + +def test_the_default_anthropic_provider_still_lets_the_sdk_read_its_own_env_vars( + monkeypatch: pytest.MonkeyPatch, +): + """A provider with no `api_key_env` (the common single-Anthropic-provider deploy) keeps today's + behavior: the SDK reads `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN` itself, so a gateway token works + exactly as it always has.""" + monkeypatch.setenv("ANTHROPIC_API_KEY", "sdk-default-key") + providers = { + "anthropic": ProviderSpec(name="anthropic", kind="anthropic", base_url=None, api_key_env=None) + } + models = ModelClients.build(providers, {"anthropic"}, timeout_s=1.0, environ={"ANTHROPIC_API_KEY": "x"}) + assert models.for_provider("anthropic")._client.api_key == "sdk-default-key" # type: ignore[attr-defined] diff --git a/deploy/compose.controlplane.yaml b/deploy/compose.controlplane.yaml index da4e76d..dda6eb6 100644 --- a/deploy/compose.controlplane.yaml +++ b/deploy/compose.controlplane.yaml @@ -22,10 +22,12 @@ services: environment: ORCHESTRATOR_URL: http://api:8000 - # The Groq key rotator (tools/keyrelay). The planner and the agents call Groq through it when GROQ_BASE_URL in - # secrets/api.env and secrets/worker.env is http://keyrelay:8787/v1 and their GROQ_API_KEY is its proxy token. - # It starts only when deploy/secrets/keyrelay/keyrelay.toml exists (update.sh adds the profile), so a checkout - # without it is unaffected. No port is published: only api and worker reach it, by name. + # The Groq key rotator (tools/keyrelay). The planner and the agents call Groq through it when the groq + # provider's `base_url` in control-plane/agents.toml's `[providers.groq]` table is http://keyrelay:8787/v1 + # (no env var wires this any more: a provider's base_url is read only from agents.toml, per-provider) and + # their GROQ_API_KEY (named by that table's api_key_env) is its proxy token. It starts only when + # deploy/secrets/keyrelay/keyrelay.toml exists (update.sh adds the profile), so a checkout without it is + # unaffected. No port is published: only api and worker reach it, by name. keyrelay: profiles: ["keyrelay"] build: { context: ../tools/keyrelay } diff --git a/deploy/compose.yaml b/deploy/compose.yaml index 613e57e..9e778b5 100644 --- a/deploy/compose.yaml +++ b/deploy/compose.yaml @@ -107,6 +107,7 @@ services: STORE_URL: http://store:8081 AGENTS_FILE: /app/deploy/agents.local.json PLANNER_MODEL: claude-haiku-4-5 + PLANNER_PROVIDER: anthropic EVENTS_TOKEN: local-events-token CHAIN_EVENTS_TOKEN: local-chain-events-token WORKER_TOKEN: local-worker-progress-token @@ -142,4 +143,8 @@ services: # addresses in deploy/agents.local.json. Not the shared-example deploy/agents.example.json. MERGIT_AGENT_KEY_1: "0x5d494b945348034ee48abb4651b80cc519f857de563b0df0fbe7cc1b2c28389a" MERGIT_AGENT_KEY_2: "0xcb78d92ca5e24d6c4bbe912b25ee362c982f591ecba1afcac7f3b18feb3165d7" - ANTHROPIC_API_KEY: ${ANTHROPIC_API_KEY:?set ANTHROPIC_API_KEY before starting the worker} + # claude-worker and second-worker (deploy/agents.local.json) are both provider = "openrouter" in + # agents.toml now; ANTHROPIC_API_KEY is not read, since ModelClients.build only builds a client for + # a provider a *deployed* agent actually names (mergit_worker/agents.py validate_agents_have_keys). + # mergit-coder-0 (OpenRouter on the hosted deployment) is not deployed in this local stack. + OPENROUTER_API_KEY: ${OPENROUTER_API_KEY:?set OPENROUTER_API_KEY before starting the worker} diff --git a/deploy/make-controlplane-env.sh b/deploy/make-controlplane-env.sh index 933dc83..9541f93 100755 --- a/deploy/make-controlplane-env.sh +++ b/deploy/make-controlplane-env.sh @@ -3,10 +3,14 @@ # deploy/make-controlplane-env.sh [app-domain] default mergit.site. On a laptop pass the page's host WITH its port, # e.g. localhost:5173: sign-in checks it against window.location.host. # The service tokens are copied from deploy/secrets/testnet.env (run make-testnet-env.sh first) so the gateway and the -# control plane agree. MERGIT_AGENT_KEY_1 comes from ../../mergit-contracts/.env.services. The model keys are read from -# $ANTHROPIC_API_KEY / $GROQ_API_KEY or asked for without echo; give either or both (Enter skips one). Anthropic is used -# when its key is present, Groq when only Groq's is (control-plane/mergit_worker/providers.py). Optional: GROQ_MODEL -# (default openai/gpt-oss-120b) and LLM_PROVIDER=anthropic|groq to force one. Prints names only, never values. +# control plane agree. MERGIT_AGENT_KEY_1 comes from ../../mergit-contracts/.env.services. Each agent and the planner +# name their own provider: the agents in control-plane/agents.toml (the deployed one, mergit-coder-0, is +# provider = "openrouter"), the planner in PLANNER_PROVIDER, which this script writes. Choose it with +# PLANNER_PROVIDER=openrouter|anthropic|groq in the environment (default openrouter); PLANNER_MODEL defaults to that +# provider's id for Claude Haiku 4.5 (Groq: openai/gpt-oss-120b). The model keys are read from $OPENROUTER_API_KEY / +# $ANTHROPIC_API_KEY / $GROQ_API_KEY or asked for without echo (Enter skips one); every key given is written, so a +# deployment can switch provider later without regenerating. The planner's provider must have its key. Prints names +# only, never values. set -euo pipefail cd "$(dirname "$0")" domain=${1:-mergit.site} @@ -25,14 +29,24 @@ if [ -z "${ANTHROPIC_API_KEY:-}" ]; then read -rsp "ANTHROPIC_API_KEY (input hid if [ -z "${ANTHROPIC_API_KEY:-}${ANTHROPIC_AUTH_TOKEN:-}" ]; then read -rsp "ANTHROPIC_AUTH_TOKEN, for a gateway such as Claude Lend (input hidden, Enter to skip): " ANTHROPIC_AUTH_TOKEN; echo; fi if [ -n "${ANTHROPIC_AUTH_TOKEN:-}" ] && [ -z "${ANTHROPIC_BASE_URL:-}" ]; then read -rp "ANTHROPIC_BASE_URL for that token: " ANTHROPIC_BASE_URL; fi if [ -z "${GROQ_API_KEY:-}" ]; then read -rsp "GROQ_API_KEY (input hidden, Enter to skip): " GROQ_API_KEY; echo; fi -[ -n "${ANTHROPIC_API_KEY:-}${ANTHROPIC_AUTH_TOKEN:-}${GROQ_API_KEY:-}" ] || { echo "a model key is required (planner and agents): ANTHROPIC_API_KEY, ANTHROPIC_AUTH_TOKEN or GROQ_API_KEY" >&2; exit 1; } -model_keys() { # only the keys that were given, so the provider is chosen from what is actually there +if [ -z "${OPENROUTER_API_KEY:-}" ]; then read -rsp "OPENROUTER_API_KEY (input hidden, Enter to skip): " OPENROUTER_API_KEY; echo; fi +planner_provider=${PLANNER_PROVIDER:-openrouter} +case "$planner_provider" in + openrouter) planner_model=${PLANNER_MODEL:-anthropic/claude-haiku-4.5}; planner_key=${OPENROUTER_API_KEY:-} ;; + anthropic) planner_model=${PLANNER_MODEL:-claude-haiku-4-5}; planner_key=${ANTHROPIC_API_KEY:-}${ANTHROPIC_AUTH_TOKEN:-} ;; + groq) planner_model=${PLANNER_MODEL:-openai/gpt-oss-120b}; planner_key=${GROQ_API_KEY:-} ;; + *) echo "PLANNER_PROVIDER must be openrouter, anthropic or groq, not $planner_provider" >&2; exit 1 ;; +esac +[ -n "$planner_key" ] || { echo "the planner's provider ($planner_provider) needs its key; none was given" >&2; exit 1; } +# The deployed agent's provider is in agents.toml, not here; the worker's start-up check (and update.sh's preflight) +# refuses a missing key exactly. This only warns early for the shipped default. +[ -n "${OPENROUTER_API_KEY:-}" ] || echo "note: no OPENROUTER_API_KEY, but mergit-coder-0 is provider = \"openrouter\" in agents.toml: the worker will not start unless agents.toml points it elsewhere" >&2 +model_keys() { # only the keys that were given, so each configured provider gets the key it needs [ -z "${ANTHROPIC_API_KEY:-}" ] || echo "ANTHROPIC_API_KEY=$ANTHROPIC_API_KEY" [ -z "${ANTHROPIC_AUTH_TOKEN:-}" ] || echo "ANTHROPIC_AUTH_TOKEN=$ANTHROPIC_AUTH_TOKEN" [ -z "${ANTHROPIC_BASE_URL:-}" ] || echo "ANTHROPIC_BASE_URL=$ANTHROPIC_BASE_URL" [ -z "${GROQ_API_KEY:-}" ] || echo "GROQ_API_KEY=$GROQ_API_KEY" - [ -z "${GROQ_MODEL:-}" ] || echo "GROQ_MODEL=$GROQ_MODEL" - [ -z "${LLM_PROVIDER:-}" ] || echo "LLM_PROVIDER=$LLM_PROVIDER" + [ -z "${OPENROUTER_API_KEY:-}" ] || echo "OPENROUTER_API_KEY=$OPENROUTER_API_KEY" } session=$(head -c 32 /dev/urandom | od -An -tx1 | tr -d ' \n') [ ${#session} -eq 64 ] || { echo "could not generate SESSION_SECRET" >&2; exit 1; } @@ -43,7 +57,8 @@ umask 077 echo "APP_DOMAIN=$domain" echo "CORS_ORIGINS=$scheme://$domain" echo "SESSION_SECRET=$session" - echo "PLANNER_MODEL=claude-haiku-4-5" # on Groq the planner runs GROQ_MODEL instead (providers.py) + echo "PLANNER_PROVIDER=$planner_provider" + echo "PLANNER_MODEL=$planner_model" model_keys for t in EVENTS_TOKEN CHAIN_EVENTS_TOKEN WORKER_TOKEN; do echo "$t=$(token $t)"; done } > secrets/api.env @@ -54,5 +69,5 @@ umask 077 for t in EVENTS_TOKEN STORE_WRITE_TOKEN WORKER_TOKEN; do echo "$t=$(token $t)"; done # GITHUB_TOKEN=... (optional: only the pull-request gate needs it; add it here by hand) } > secrets/worker.env -echo "wrote deploy/secrets/api.env (APP_DOMAIN CORS_ORIGINS SESSION_SECRET PLANNER_MODEL, the model key(s) you gave, EVENTS_TOKEN CHAIN_EVENTS_TOKEN WORKER_TOKEN)" +echo "wrote deploy/secrets/api.env (APP_DOMAIN CORS_ORIGINS SESSION_SECRET PLANNER_MODEL PLANNER_PROVIDER, the model key(s) you gave, EVENTS_TOKEN CHAIN_EVENTS_TOKEN WORKER_TOKEN)" echo "wrote deploy/secrets/worker.env (the model key(s) you gave, MERGIT_AGENT_KEY_1 APP_BASE_URL EVENTS_TOKEN STORE_WRITE_TOKEN WORKER_TOKEN)" diff --git a/deploy/server/README.md b/deploy/server/README.md index 1df3fa9..14cec2e 100644 --- a/deploy/server/README.md +++ b/deploy/server/README.md @@ -84,6 +84,46 @@ deploy/server/update.sh app # only the frontend: web or app Render deployed on every push. The server does not by itself, so `.github/workflows/deploy.yml` runs `update.sh` over SSH on every push to `main` (and by hand from the Actions tab, where you can pick `services`, `web` or `app`). +### Upgrading an existing deployment past per-agent providers (`PLANNER_PROVIDER`) + +The hosted deployment calls models through OpenRouter: `mergit-coder-0` is `provider = "openrouter"`, +`anthropic/claude-sonnet-5` in `control-plane/agents.toml`, and the planner uses whatever `PLANNER_PROVIDER` in +`api.env` names. `PLANNER_PROVIDER` has no default, and `deploy/make-controlplane-env.sh` will not touch +`deploy/secrets/api.env` or `worker.env` once they exist, so a server set up before this change has files the new +code cannot start from. Nothing goes down because of it: `update.sh` builds the new images, then runs each +service's own start-up checks in them against the real env files (`python -m mergit_cp.preflight` and +`python -m mergit_worker.preflight`), and only then replaces the running containers. A missing +`PLANNER_PROVIDER`, or a provider with no key (a `worker.env` with only `GROQ_API_KEY`, say), fails the deploy +with one line naming the setting, never its value, and the old containers keep serving. The automatic deploy +shows red until the files are fixed. + +To move the server to OpenRouter, on the server, once (the key is read without echo and never printed): + +```bash +cd ~/mergit/mergit +read -rsp "OPENROUTER_API_KEY: " k; echo +for f in api worker; do grep -q '^OPENROUTER_API_KEY=' deploy/secrets/$f.env || echo "OPENROUTER_API_KEY=$k" >> deploy/secrets/$f.env; done; unset k +sed -i '/^PLANNER_PROVIDER=/d; /^PLANNER_MODEL=/d' deploy/secrets/api.env +printf 'PLANNER_PROVIDER=openrouter\nPLANNER_MODEL=anthropic/claude-haiku-4.5\n' >> deploy/secrets/api.env +grep '^PLANNER_MODELS=' deploy/secrets/api.env # if set, every id in it must be an OpenRouter id (see below) +deploy/server/update.sh services # runs the preflight again, then deploys +``` + +`PLANNER_MODELS` (the extra planner models a user may pick in Settings) are all sent to the planner's provider, +so with OpenRouter they must be OpenRouter ids priced in `prices.toml` (`anthropic/claude-sonnet-5`, ...). A +native id such as `claude-sonnet-5` is priced too, so the preflight passes it, but OpenRouter does not know it +and a plan using it would answer 502. Leave `PLANNER_MODELS` unset to offer only `PLANNER_MODEL`. + +**Anthropic stays an option.** To call Anthropic's API directly instead, put `ANTHROPIC_API_KEY` (or +`ANTHROPIC_AUTH_TOKEN` with `ANTHROPIC_BASE_URL`) in the env files and set `PLANNER_PROVIDER=anthropic`, +`PLANNER_MODEL=claude-haiku-4-5` for the planner; for the agent, change `mergit-coder-0` in `agents.toml` to +`provider = "anthropic"`, `model = "claude-sonnet-5"` and commit it. Keeping both keys in the env files means +either can be switched to without touching secrets again. + +`GROQ_BASE_URL`, `GROQ_MODEL` and `LLM_PROVIDER` are no longer read; a provider's endpoint and model live in +`control-plane/agents.toml`. Leaving them in the env files is harmless, but the API and the worker log a +warning at start-up naming any that are still set, so delete them. + ### Automatic deploys: one-time setup 1. On your machine, make a key used for nothing else: `ssh-keygen -t ed25519 -N '' -f deploy_key -C github-deploy`. @@ -179,15 +219,23 @@ $C build keyrelay $C run --rm keyrelay check --config /etc/keyrelay/keyrelay.toml # valid / dead / limited per key, by index TOKEN=$(cat deploy/secrets/keyrelay/proxy-token.txt) sed -i "s|^GROQ_API_KEY=.*|GROQ_API_KEY=$TOKEN|" deploy/secrets/api.env deploy/secrets/worker.env -grep -q '^GROQ_BASE_URL=' deploy/secrets/api.env || echo 'GROQ_BASE_URL=http://keyrelay:8787/v1' >> deploy/secrets/api.env -grep -q '^GROQ_BASE_URL=' deploy/secrets/worker.env || echo 'GROQ_BASE_URL=http://keyrelay:8787/v1' >> deploy/secrets/worker.env -$C up -d --force-recreate keyrelay api worker +``` + +A provider's endpoint is no longer an env var read at container start: each provider's `base_url` lives in +`control-plane/agents.toml`'s `[providers.]` table (control-plane/README.md, "Model provider"), baked into +the `api` and `worker` images at build time. To route Groq traffic through the rotator, set `[providers.groq]`'s +`base_url` to `http://keyrelay:8787/v1` in `agents.toml` and commit it — an edit made only on the server would be +reverted by the next deploy, since `update.sh` mirrors `origin/main` — then rebuild and recreate: + +```bash +$C up -d --force-recreate --build keyrelay api worker $C exec keyrelay keyrelay status --config /etc/keyrelay/keyrelay.toml # which keys are ok / cooling / dead $C logs --tail 20 keyrelay # indexes only, never a key ``` If keyrelay is down, planning fails with a 502 and the API logs `planner model unavailable: ...`. To go back, restore -a real `GROQ_API_KEY` in `api.env` and `worker.env`, remove `GROQ_BASE_URL`, and recreate `api` and `worker`. +a real `GROQ_API_KEY` in `api.env` and `worker.env`, set `[providers.groq]`'s `base_url` in `agents.toml` back to +Groq's own endpoint (`https://api.groq.com/openai/v1`), commit, and rebuild and recreate `api` and `worker`. ## Free credits and redeem codes diff --git a/deploy/server/update.sh b/deploy/server/update.sh index a01fc49..53bdfd9 100755 --- a/deploy/server/update.sh +++ b/deploy/server/update.sh @@ -18,7 +18,16 @@ sync_repo ../mergit-contracts # deployments/base-sepolia.json lives there case "$what" in all|services) - compose up -d --build + compose build + # Before anything running is replaced: run the API's and the worker's own start-up configuration checks + # in the images just built, against the real env files (mergit_cp/preflight.py, mergit_worker/preflight.py). + # A setting the new code needs and deploy/secrets/*.env lacks (PLANNER_PROVIDER, a deployed agent's model + # key) then fails the deploy here, with `set -e`, and the old containers keep serving, instead of `up` + # leaving a new api restart-looping or a new worker that refuses to start. Neither check touches the + # database or the network. + compose run --rm --no-deps -T api python -m mergit_cp.preflight + compose run --rm --no-deps -T worker python -m mergit_worker.preflight + compose up -d # `git pull` replaces the Caddyfile with a new file, and a single-file bind mount keeps pointing at the old one, # so neither `up` nor `caddy reload` (which re-reads the stale copy and still succeeds) applies a changed # Caddyfile. Compare what the container sees with the file on disk; if they differ, or the container cannot be @@ -34,3 +43,10 @@ case "$what" in all) deploy/server/build-sites.sh all ;; web|app) deploy/server/ compose ps for h in gateway store; do printf '%s: ' "$h"; curl -fsS "https://$h.mergit.site/healthz"; echo; done +# The control-plane API has no /healthz; GET /v0/agents is public and answers 200 once the app has started. +# A just-recreated api takes a while (migrations, the healthcheck's start_period), so retry for up to about +# 90 s before calling it down. `-f` then makes curl, and with `set -e` this script, fail on a non-2xx or an +# unreachable api, instead of exiting 0 over one that never came up. +printf 'api: ' +curl -fsS --retry 30 --retry-delay 3 --retry-max-time 90 --retry-all-errors -o /dev/null -w '%{http_code}\n' \ + "https://app.mergit.site/v0/agents"