Repository navigation
control-plane: per-agent model providers, OpenRouter support, failure-injection audit - #47
Open
OfficialAbhinavSingh wants to merge 26 commits into
Open
OfficialAbhinavSingh wants to merge 26 commits into
OfficialAbhinavSingh wants to merge 26 commits into
Conversation
…OVIDER/GROQ_MODEL concept
…e step, planner error handling, llm_check coverage, dead references) - deploy/server/README.md + update.sh: document the manual PLANNER_PROVIDER/Anthropic-credential step an operator must take on the existing hosted deployment before/during this merge, and make update.sh fail loudly if the api container doesn't come back up healthy, instead of reporting a broken deploy as a success. - mergit_cp/llm.py: the planner's OpenAI-compatible client now surfaces a 200-status error object the same way the worker's does, instead of a generic "no choices" message; a misspelled PLANNER_PROVIDER now raises a clear ConfigError instead of a bare KeyError; the no-key stand-in names the actual provider instead of assuming it's always the planner's. - mergit_cp/llm_check.py: checks every distinct (provider, model) pair actually in use, not just the first model found per provider, so both OpenRouter agent models get their own check. - deploy/compose.yaml: gives the local api container an optional OPENROUTER_API_KEY so llm_check can run inside it, per control-plane/README.md's own instructions. - mergit_worker/providers.py: a malformed [providers.*] table now raises ConfigError instead of a raw AttributeError. - Removed the dead GROQ_BASE_URL constant and rewrote the stale keyrelay docs, README env-var lists, and prices.toml's Groq comment to match the current per-provider-table model. - deploy/make-controlplane-env.sh: dropped a named person from a comment and tightened the model- key guard to require an Anthropic credential, matching what this deployment's agents actually need.
Member
Author
|
PR #47 (per-agent model providers + OpenRouter) needs one thing before it merges it adds a new required setting (PLANNER_PROVIDER) that the hosted server's current api.env doesn't have. without it the api container will restart-loop after this merges and the deploy won't catch it. steps are written out in deploy/server/README.md under " |
OfficialAbhinavSingh
marked this pull request as ready for review
November 2, 2026 04:28
OfficialAbhinavSingh
changed the title
Member
Author
|
Ready for review. Since the last push: merged Two things before merge:
The auto-dispatch routing work is parked on |
# Conflicts: # control-plane/mergit_cp/config.py # control-plane/tests/conftest.py
Member
Author
|
Rebased on main's latest ( One real conflict, in All gates green post-merge: |
Member
Author
|
@Viscous106 one manual step needed on the server at/before merge, or On the server: cd ~/mergit/mergit
grep -q '^PLANNER_PROVIDER=' deploy/secrets/api.env || echo 'PLANNER_PROVIDER=anthropic' >> deploy/secrets/api.env
grep -qE '^(ANTHROPIC_API_KEY|ANTHROPIC_AUTH_TOKEN)=' deploy/secrets/api.env \ || echo 'api.env has no Anthropic credential: add ANTHROPIC_API_KEY before recreating api' >&2
grep -qE '^(ANTHROPIC_API_KEY|ANTHROPIC_AUTH_TOKEN)=' deploy/secrets/worker.env \ || echo 'worker.env has no Anthropic credential: add it before recreating worker' >&2
Full writeup: |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
…or name a Mergit secret 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 could send MERGIT_AGENT_KEY_1, STORE_WRITE_TOKEN, GITHUB_TOKEN or DATABASE_URL to any host. load_providers now refuses that: - base_url is https on the default port to a host in _PROVIDER_HOSTS (api.groq.com, openrouter.ai), or exactly http://keyrelay:8787, the compose-internal rotator. No credentials in the URL. - api_key_env is <PROVIDER>_API_KEY in capitals and does not start with MERGIT_. Every model provider key has that shape and none of Mergit's own secrets does (*_TOKEN, *_SECRET, *_KEY, DATABASE_URL, MERGIT_AGENT_KEY_<id>), so no denylist has to be kept in step. - an anthropic provider may not set base_url, which its client ignored. - a non-string base_url or api_key_env is a ConfigError, not an AttributeError. Every caller (worker, API planner, llm_check, agent loading) goes through load_providers, and the repo's own agents.toml passes. A new provider host is a code change to _PROVIDER_HOSTS.
A server set up before PLANNER_PROVIDER existed has an api.env the new api cannot start from, and a worker.env with only GROQ_API_KEY now that mergit-coder-0 is provider = "anthropic" makes the new worker refuse to start. Pushes to main deploy on their own, so either would have shipped as an outage. update.sh now builds first, then runs each service's own start-up checks in the new image against the real env file (python -m mergit_cp.preflight, python -m mergit_worker.preflight), and only then runs compose up. A bad setting fails the deploy with one line naming it, and the old containers keep serving. create_app's lifespan and build_worker call the same check_config / check_models, so the deploy checks exactly what start-up checks. The api preflight also fails on a planner with no key, which the api itself tolerates (planning answers 502) but a deploy should not ship by accident. Errors name the setting and the reason only. Pydantic's message carries the input, which is the environment, and the preflight's output goes to the public GitHub Actions log; load_config had the same leak into the worker's own logs. GROQ_BASE_URL, GROQ_MODEL and LLM_PROVIDER are no longer read: start-up warns about any still set. GROQ_BASE_URL is refused instead when it disagrees with agents.toml for a provider in use that reads GROQ_API_KEY, since that env file was written for keyrelay and its proxy token would now go to Groq itself. The post-deploy api check retries for about 90 s before calling a just-recreated api down. The README's upgrade section now describes this. Checked: full suite (390 passed), ruff and pyright clean, and both preflights run in locally built api and worker images: an old api.env, a planner with no key and a Groq-only worker.env each fail with one line; good files pass.
It ran in the api container and also checked every deployed agent's provider, whose keys are only in worker.env, so it reported FAILED for keys that were fine where they are used. It now takes a scope: planner (the default, in the api container), agents (in the worker container, read through the worker's own config) or all (a shell that holds every key). The api service in deploy/compose.yaml had OPENROUTER_API_KEY only for the old all-in-one check; it no longer gets it.
…ver carry the key
Both OpenAI-compatible clients (the planner's and the worker's) now read an
HTTP 200 body through one shared body_error in mergit_worker/openai_chat.py:
- OpenRouter reports an upstream 429 or 5xx as HTTP 200 with
{"error": {"code": 429}}. That is retried like the bare status was;
other codes still fail at once.
- An error object without a message says its code instead of "None".
- A body that is JSON but not an object is a clear error, not an
AttributeError from data.get.
Error text from a provider is scrubbed of the key before it is truncated:
some providers echo the key they rejected in a 401 body, and that text goes to
logs and to llm_check's output.
The llm_check test that used to assert the token never appears had lost that
line; it is back as an end-to-end test with the real client and a provider
that echoes the key. 14 of the 16 new client cases fail on the old code (the
other two pin that a non-retryable error is still tried once).
…k probes validate_agents_have_keys, validate_agents_are_priced, load_agent_spec and llm_check each read agents.json and agents.toml their own way and called .get on whatever they found, so a valid-TOML but wrong-shaped entry (coder = "x" under [agents]) crashed all four with AttributeError. They now share deployed_agents, which checks the shape once and raises a ConfigError naming the entry. The new tests fail on all four old readers. llm_check's probes no longer inherit the planner's max_tokens=16000: OpenRouter checks the account's credit against max_tokens, so a low balance failed a probe it would pass for real. build_planner_llm takes max_tokens and the probes ask for 1024.
It pointed at a plan file outside the repo and at file:line ranges that go stale with any edit. Evidence is now the file and the test name, which pytest can run directly; every cited test was checked to exist.
…er, scope llm_check
…n't match its own print) Two mechanical bugs in that commit, both caught by CI: - llm_check.py's module docstring lost a line break when the OpenRouter example was removed, over ruff's 110-column limit; the new test's own docstring was also one line too long. - test_a_scoped_run_says_which_scope_it_checked asserted the literal substring "checking the planner only", but _say_scope's actual text is "checking the planner (run `agents` in the worker container for the agents' keys) only, ..." — the parenthetical breaks that substring. Loosened the assertion to check what the test's own docstring says it is for. Full suite green (469 passed, 1 pre-existing skip), ruff/format/pyright clean.
…de-haiku-4.5 for the OpenRouter planner
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Each agent (and the planner) names its own model provider in config, instead of one
LLM_PROVIDERchoice for the whole process. OpenRouter is added as a first-class OpenAI-compatible provider alongside Anthropic and Groq, so different agents can run on different models/keys in the same deployment — the prerequisite for routing between them.Part of #11 (router needs more than one real arm to route between). The final whole-branch review is done (fixes in
398ecca). Rebased onmain's latest inab90460, see "What I verified" for the real-key check and the rebase conflict.Also carries the failure-injection audit from #28 (test and docs only, no runtime change), see the section below. Brought up to date with
mainin982b98e, then again inab90460.Plan:
docs/superpowers/plans/2026-11-01-per-agent-model-providers.md(4 tasks, all complete):[providers.<name>]config,ProviderSpec,load_providers,provider_key, per-agentproviderfield (+ pricing for the two local OpenRouter agents)OpenAIChatLLM),ModelClientsbuilds one client per agent's provider, worker uses the agent's own client per jobSettings.planner_provider),llm_checkcovers every provider in use, deadLLM_PROVIDER/require_provider/has_anthropic_credentialsingle-process-provider code removedcompose.yaml,make-controlplane-env.sh), README rewritten for the new per-provider model, repo-wideLLM_PROVIDER/GROQ_MODELcleanup verified emptyFailure-injection audit (#28)
Checked the HLD's failure-injection list (fabricated success, wrong file, empty diff, budget overrun, step tampering, cache replay) and #28's "no lost or double-run tasks" against existing coverage. Five were already guarded and tested, empty diff is accepted behaviour by design, and cache replay can't be built until a receipt carries cache provenance. The full table with file:line evidence is in
control-plane/docs/failure-injection-audit.md.b1d9795: budget exhausted with no completed work now has a test asserting Φ's realfalse. It passed first run, no harness change was needed.5fe8ad2: a sealed receipt with a flipped step digest, reordered steps or a dropped step is rejected bymergit_receipt.verify()called from Python. Previously only the Rust vector tests covered this.4a04561: the audit document, with every citation re-checked, and the README "Not done here" line pointing at it.Type of change
feat— new functionalityWhat I verified
ruff check,ruff format --check,pyrightstrict,pytest,pytest -m postgres) green at each commit, reviewed task-by-task with a fix round where needed; both OpenRouter model ids checked live against/api/v1/modelsfor tool support; pricing verified against the live OpenRouter pricing endpoint; the deploy wiring was run end-to-end inside the builtmergit-api:localimage with placeholder keys and reached real endpoints (api.anthropic.com,openrouter.ai).llm_check's own per-pair check (_check_one) against the live OpenRouter API with a real key for both configured agents —openrouter/anthropic/claude-sonnet-5andopenrouter/deepseek/deepseek-v4-flash. Both passed both steps (plain answer, forced tool call), e.g.claude-sonnet-5:plain answer: ok in=16 out=4,tool call: ok in=446 out=47;deepseek-v4-flash:plain answer: ok in=11 out=2,tool call: ok in=289 out=43. This closes the one open item from the last review pass. The key was never written to a file or committed, only exported for that one process.anthropicprovider path (planner's default,planner_provider="anthropic") with a real Anthropic key — no Anthropic key on hand this round. The adapter itself (AnthropicLLM) is unchanged by this PR and already in production use onmaintoday, so this is a pre-existing gap, not a new one.mainmerge (ab90460, resolving447f521/0ec2c79):ruff,ruff format,pyright(0 errors, was 34), unit suite (352) andpytest -m postgres(352) all pass clean — see the rebase comment below for the one real conflict and how it was resolved.Checklist
.envfiles, mnemonics)If this touches the runtime
modelfield now just carries the real per-agent model id (unchanged mechanism, wider range of values)Evidence
Commit-by-commit: Task 1 =
9b901fe(+ pricing fixbbd4431), Task 2 =1528af7(+ fix round8377885), Task 3 =4a293cb, Task 4 =574ac92, rebase on main's second drift =ab90460(see comment below). Per-task gate output and TDD evidence are in each task's report under.superpowers/sdd/2026-11-01-per-agent-model-providers/locally (not committed — internal working notes).Breaking changes / follow-ups
LLM_PROVIDERenv var,GROQ_MODEL, and the old single-process-provider model-swap behavior are fully removed — every agent and the planner now name their provider explicitly inagents.toml.deploy/compose.yaml's localworkerservice no longer requiresANTHROPIC_API_KEY(verified: its two local agents are bothopenrouter-provider now) but does requireOPENROUTER_API_KEY;make-controlplane-env.shnow also prompts for it.prices.toml's Groq comment header still says "the fallback provider when there is no Anthropic key" — stale now that there's no implicit fallback concept, left for whoever next touches that file.Deploy step still needed before/at merge (see the comment above): hosted
secrets/api.envneedsPLANNER_PROVIDER=anthropicadded, or the API container restart-loops after this merges.