Add per-agent non-Claude model support via LiteLLM proxy

[jwbron]

Summary

Today every egg SDLC agent runs on Claude through the Claude Code
harness, with the gateway hard-wiring api.anthropic.com as the
only /v1/messages upstream. The orchestrator's consensus wrapper
hardcodes --model opus for every non-overseer role, so per-agent
model selection does not exist. We need to let any agent run on a
non-Claude backend (Qwen is the first target, primarily for cost)
while every Claude-bound agent stays byte-identically on the
existing path — and we need the integration to be safe to ship
before a live non-Claude endpoint is available.

This change lands the buildable, no-op-by-default seam in two
stacked PRs:

1. Gateway upstream router + LiteLLM Deployment (slice 1).
   Introduces a small UpstreamRegistry abstraction in the
   gateway that keys per-request httpx.Client + credential by
   upstream name, with proxy_anthropic_messages /
   proxy_count_tokens resolving the upstream per request via
   the existing IP-keyed session lookup that already drives
   session_mode. Adds Session.upstream (default "anthropic")
   and Session.upstream_model (default None), wired through
   /api/v1/sessions/create, SessionManager.register_session,
   and GatewayClient.register_session. The LiteLLM proxy itself
   ships as a separate Deployment + Service + ConfigMap in
   egg-system, reachable only by the gateway. The Claude path
   is structurally untouched.
2. Per-agent model config + spawn-side plumbing (slice 2).
   Adds PipelineConfig.agent_models: dict[str, str] and a
   default_agent_model repository-level setting, plus a
   resolution function that the orchestrator's spawner calls to
   (a) thread the right --model to build_consensus_wrapped_command
   and (b) tell the gateway the per-agent upstream and
   upstream_model at session-create time. The gateway, on a
   LiteLLM-routed request, rewrites the body's model field
   from the Claude alias presented to Claude Code to the
   upstream-side model name — keeping Claude Code's compaction
   math sane (cq-5).

With agent_models empty (the default everywhere), no LiteLLM
request fires. Every existing pipeline keeps running on Claude
with byte-identical gateway behavior. The empirical
Claude-Code-compaction smoke test (cq-4) is an operator-driven
follow-up once a live non-Claude endpoint is configured; it is
explicitly out of scope here.

Test plan

Automated:

• make test from the repo root catches both slices' reachable
  suites given the changeset.
• Slice 1: tests/gateway/test_upstream_registry.py (new),
  extensions to tests/gateway/test_anthropic_proxy.py,
  tests/gateway/test_anthropic_credentials.py,
  gateway/tests/test_session_manager.py,
  orchestrator/tests/test_gateway_client.py.
• Slice 2: orchestrator/tests/test_agent_model_resolution.py
  (new), extensions to the concurrent-executor and
  pipeline-spawn tests, extensions to the gateway proxy tests
  covering the body-rewrite branch.

Manual (reviewer):

• Confirm make test and make lint are green.
• kubectl apply --dry-run=client -k k8s/base/ succeeds with
  the new LiteLLM manifests included.
• Spot-check that with agent_models={} and no
  LITELLM_MASTER_KEY in secrets, gateway request flow is
  byte-identical to today's Claude path (no new headers, no
  upstream change, same SSE behavior).

Manual (operator, post-merge — not gating merge):

• Populate LITELLM_MASTER_KEY in secrets.env, configure
  LiteLLM model_list with a hosted Qwen provider (cq-6), set
  agent_models={"refiner": "qwen3-coder-30b"} on a pipeline,
  and exercise a tool-heavy multi-turn loop plus a long session
  crossing the auto-compaction boundary (the cq-4-deferred
  empirical compatibility check).

Manual steps

Pre-merge: none.

Post-merge (only required when operator wants to actually run an
agent on a non-Claude backend):

1. Add LITELLM_MASTER_KEY=<random> to ~/.config/egg/secrets.env.
2. Populate the LiteLLM ConfigMap model_list with at least one
   backend (hosted Qwen provider first, per cq-6). The
   provider-side API key goes in LiteLLM's standard env-var slot,
   not in secrets.env.
3. Either set default_agent_model in
   ~/.config/egg/repositories.yaml for the target repo, or
   pass agent_models={"<role>": "<model>"} on pipeline submit
   to override per pipeline.

---

Base / umbrella PR for issue #2769 — carries egg/issue-2769/work
into main. The implementation lands across two stacked slice PRs
that merge into this branch first:

• [issue-2769][slice-1/2] Gateway upstream router + LiteLLM Deploymen... #2773 — slice 1: Gateway upstream router + LiteLLM Deployment (no-op by default)
• [issue-2769][merge-gate] Add per-agent non-Claude model support via... #2774 — slice 2: Per-agent model config + spawn plumbing

Closes #2769.

[james-in-a-box]

Contract Verification — PR #2775 (issue #2769)

Verdict: Comment. The implementation fully and correctly satisfies the
contract; every per-task acceptance criterion I could check statically is
met, with comprehensive tests and docs. However, there are contract-metadata
inconsistencies and a PR-scoping caveat the human reviewer should be
aware of. No code defects found.

---

Scoping caveat — PR #2775 carries no implementation code

PR #2775 (egg/issue-2769/work, the umbrella PR) changes only .egg-state/
files — pipeline state, contract JSON, drafts, BRC history. The actual
implementation landed via the stacked slice PRs, both already merged:

• #2773 — egg/issue-2769/slice-1 → main (carries the full stack: both slices)
• #2774 — egg/issue-2769/slice-2 → egg/issue-2769/slice-1

Verifying PR #2775 in isolation would find zero implementation. This review
verifies the delivered code now on origin/main (HEAD 8f0c980), which is
the complete issue-#2769 implementation.

---

Contract metadata inconsistency (RED FLAG per contract rules)

Both slices are marked status: complete, but most of their tasks are not:

Slice	Slice status	Tasks complete	Tasks still pending (commit null)
slice-1	complete	task-1-1…1-9, 1-12	task-1-10, task-1-11 (tester)
slice-2	complete	task-2-7, 2-8 (tester)	task-2-1, 2-2, 2-3, 2-4, 2-5, 2-6 (coder), 2-9 (documenter)

This is the contract-rules red flag "Phase marked complete but tasks are
pending." The reality is the inverse of a real problem — the code for all
9 pending tasks is fully merged and correct on main (verified
file-by-file below). The contract bookkeeping simply lagged the delivered work:
task status/commit fields were never updated. Also current_phase is stuck
at "refine" though the work is merged.

Net: the contract under-reports completion. Not a code defect, but the
contract no longer reflects reality and should be reconciled.

---

Acceptance-criteria marking

The contract's top-level acceptance_criteria array is empty — there are
no enumerable ac-N criteria, only free-text acceptance_criteria strings on
each task. egg-contract verify-criterion --criterion ac-N therefore has
nothing to mark. (The egg-contract CLI / orchestrator was also unreachable
throughout this review — "Orchestrator unreachable".) I verified the
per-task criteria directly against the code instead.

---

Per-task verification (code on origin/main)

Slice 1 — Gateway upstream router + LiteLLM Deployment

• task-1-1 gateway/upstream_registry.py — UpstreamRegistry with
  get()/is_known()/known_upstreams(); anthropic → https://api.anthropic.com,
  litellm → LITELLM_BASE_URL env (default http://litellm.egg-system.svc.cluster.local:4000),
  unknown → UnknownUpstreamError; both clients via shared _make_client
  (timeout 120s, 100 conns). All ACs met.
• task-1-2 LiteLLMCredentialsManager in anthropic_credentials.py —
  mtime-invalidated cache, x-api-key credential, None when key absent, no
  startup warning. Met.
• task-1-3 _inject_upstream_credentials(headers, upstream) + back-compat
  _inject_anthropic_credentials alias; per-upstream dispatch; 401 on missing
  cred. Adds a fail-closed 502 on unknown upstream (documented "#2769
  review" addition). Met.
• task-1-4 Session.upstream/upstream_model with defaults; persistence
  round-trips and tolerates absent keys; only persisted when non-default
  (byte-identical disk layout for Claude path). Met.
• task-1-5 /api/v1/sessions/create parses, validates (is_known, 400 on
  bogus), forwards to register_session, logs both in audit_log. Met.
• task-1-6 proxy_anthropic_messages/proxy_count_tokens resolve upstream
  per request; Anthropic/no-session still uses get_anthropic_client()
  (byte-identical, preserves test mocks); SSE accumulator / _filter_blocked_tools
  / retry loop untouched. Met.
• task-1-7 GatewayClient.register_session optional upstream/upstream_model,
  included in body only when set. Met.
• task-1-8 litellm-{deployment,service,configmap}.yaml + kustomization.yaml;
  pinned image litellm:main-v1.55.10, ClusterIP litellm:4000, empty
  model_list: [], no egg-agents egress change. Manifests well-formed
  (reviewer should still run kubectl apply --dry-run=client -k k8s/base/).
  Met.
• task-1-9 LITELLM_MASTER_KEY="" documented in config/secrets.template.env
  with disable-when-empty note. Met.
• task-1-10 / task-1-11 tester tasks marked pending, but the test files
  exist and are merged: tests/gateway/test_upstream_registry.py (12 tests
  covering anthropic/litellm/unknown + client semantics), and extensions to
  test_anthropic_credentials.py, test_anthropic_proxy.py,
  gateway/tests/test_session_manager.py, orchestrator/tests/test_gateway_client.py.
  Coverage matches the ACs.
• task-1-12 docs/architecture/upstream-routing.md (431 lines) + cross-links
  from gateway/CLAUDE.md and docs/architecture/orchestrator.md. Met.

Slice 2 — Per-agent model config + spawn plumbing + body rewrite

• task-2-1 PipelineConfig.agent_models + field_validator. Note: the
  validator checks against MODEL_OVERRIDE_ROLES (phase producers + reviewers)
  rather than the full AgentRole enum the task description named — a
  deliberate, documented narrowing (utility/interface roles never reach the
  resolver, so an override naming them would silently no-op). All three task-2-1
  ACs still hold (refiner constructs, bogus_role raises, default is {}).
  Reasonable improvement, not a violation.
• task-2-2 get_default_agent_model(repo) in config/repo_config.py;
  repositories.yaml.example documents the field + precedence. Adds a
  ValueError on a non-string YAML value (defensible hardening). Met.
• task-2-3 orchestrator/agent_model_resolution.py — resolve_agent_model
  • classify_model + frozen AgentModelDecision; precedence pipeline → repo →
    built-in opus; classifier routes Claude aliases to anthropic and
    everything else to litellm with the cq-5 opus alias. All ACs met.
• task-2-4 concurrent_executor._spawn_agent resolves the decision, passes
  model= to build_consensus_wrapped_command (whose default is already
  "opus", so the empty-agent_models path is byte-identical), and forwards
  upstream/upstream_model to the spawner only on non-default decisions;
  kubernetes_spawner threads them through spawn_agent_job/restart_agent_job/_spawn
  to register_session. Met.
• task-2-5 routes/pipelines.restart_agent resolves + passes model=;
  forwards upstream kwargs only on non-default decisions. The task description
  assumed restart "reuses the existing session" — the implementation correctly
  recognizes restart rebuilds the session and forwards upstream so the
  rebuilt session matches (more correct than the description). ACs met.
• task-2-6 _rewrite_upstream_model next to _filter_blocked_tools;
  rewrites body model on the LiteLLM path, returns body unchanged on
  None/parse-error, called after _filter_blocked_tools and guarded by
  upstream_name != "anthropic" (Anthropic path byte-identical). Met.
• task-2-7 / task-2-8 complete, commit a7658bf1.
  test_agent_model_resolution.py (24 tests: precedence, classifier,
  validation, dual-import, default-path regression, explicit cq-5 alias-pin
  assertion) + test_anthropic_proxy.py/test_concurrent_executor.py
  extensions. Coverage matches the ACs.
• task-2-9 documenter task marked pending, but docs/guides/per-agent-models.md
  (469 lines) exists and is merged, cross-linked from docs/index.md and
  docs/architecture/upstream-routing.md.

The no-op-by-default invariant holds: with agent_models={} and no
LITELLM_MASTER_KEY, every resolver call returns the built-in opus/anthropic
decision, no register_session upstream kwargs are sent, _rewrite_upstream_model
is never invoked, and the gateway forwards bytes verbatim to Anthropic.

---

Minor — files_affected incomplete (orphaned-but-necessary changes)

Two files changed by the implementation are not listed in any task's
files_affected:

• Makefile (k3s-secrets) — extracts LITELLM_MASTER_KEY into the
  gateway-secrets Secret as litellm-master-key. Required for
  litellm-deployment.yaml's secretKeyRef: key: litellm-master-key to
  resolve — logically part of task-1-8.
• scripts/await-egg-deploy.sh — scopes the ImagePullBackOff fast-fail to
  egg-owned deployments so the third-party LiteLLM image doesn't abort
  make deploy. Also a necessary consequence of task-1-8.

Both are correct and in-scope-by-necessity; the gap is contract fidelity
(task-1-8's files_affected should have listed them), not code quality.

---

Summary

The issue-#2769 implementation is complete, correct, and well-tested as
delivered on main. No blocking issues. The items above — contract task
statuses not reconciled with the merged work, current_phase stale at
refine, and two unlisted files_affected — are contract-bookkeeping notes
for the human reviewer, not code defects. PR #2775 itself is already merged.

— Authored by egg

[egg-reviewer]

Review — PR #2775 (per-agent non-Claude model support via LiteLLM proxy)

Scope note. PR #2775 is the umbrella PR for issue #2769. Its own diff against main contains only .egg-state/ orchestrator artifacts (skipped per review policy) — the implementation landed via the stacked slice PRs #2773 (slice‑1) and #2774 (slice‑2), which carry the feature into main. This review covers the combined feature code those slices delivered (gateway upstream router, UpstreamRegistry, per-agent model resolution, spawn plumbing, k8s manifests, tests).

Overall this is a careful, well-structured change: the no-op-by-default invariant is genuinely preserved, credential isolation is sound, and test coverage is broad. I traced the feature end-to-end (config → resolver → spawn → gateway session → proxy routing → body rewrite) and the happy path works. One blocking issue and several non-blocking items below.

---

Blocking

1. default_agent_model misconfiguration is silently swallowed at spawn.

config/repo_config.py::get_default_agent_model deliberately raises a loud ValueError for a non-string default_agent_model (e.g. default_agent_model: 4 in repositories.yaml) — its docstring explicitly says it surfaces the misconfiguration loudly. But that loud failure is dead-lettered:

• orchestrator/agent_model_resolution.py::resolve_agent_model calls get_default_agent_model(repo) un-guarded (tier-2 lookup).
• Both spawn callers wrap the entire resolver in a catch-all:
  • orchestrator/concurrent_executor.py::_spawn_agent — except Exception as resolve_err: ... decision = classify_model(DEFAULT_AGENT_MODEL)
  • orchestrator/routes/pipelines.py::restart_agent — same pattern.

Net effect: an operator who misconfigures default_agent_model gets a buried logger.warning and every agent silently runs on opus. The pipeline reports success; the operator's deliberately-set config was ignored with no pipeline-level signal. This is exactly the "operator-facing misconfiguration produces no signal / silent except Exception fallback" pattern the review rules call out as blocking — and it's asymmetric with agent_models, whose bad keys are rejected loudly at PipelineConfig construction so submission fails.

The catch-all is legitimate as a guard against resolver bugs; the problem is it also eats operator config errors. Fix options: validate default_agent_model eagerly when the repo config is loaded / at pipeline submit (so it fails before the pipeline starts), or have the spawn-path except distinguish a config ValueError (re-raise / register a HITL decision) from an unexpected resolver bug (fall back). The PR even ships a test (test_non_string_repo_default_raises_value_error) whose own comment acknowledges "the spawn-path except Exception then swallows [it] into the opus fallback" — yet no test asserts the operator actually sees a failure, and the swallow was left in place.

---

Non-blocking

2. Two vacuous tests in tests/gateway/test_upstream_registry.py.
test_anthropic_credential_resolver_is_anthropic and test_litellm_credential_resolver_is_litellm do manager = getattr(credential_resolver, "__self__", None) then assert manager is None or isinstance(...). The resolvers are plain lambdas (lambda: get_credentials_manager().get_credential() in upstream_registry.py), which have no __self__, so manager is always None and both assertions pass unconditionally. They would pass even if the anthropic/litellm resolvers were swapped — i.e. they verify nothing. Assert on observable behavior instead (e.g. patch the two managers to return distinguishable credentials and check which one the resolver returns).

3. Skip-on-ImportError scaffolding should be removed now that the code has landed.
_resolver(), _decision_cls(), _agent_models_field_exists(), _slice_2_available(), _rewrite_helper_exists(), and the _rewrite_fn fixture all pytest.skip (or return False) on ImportError. That was reasonable tester scaffolding while slice-2 was in flight, but post-merge it's a hazard: a future change that breaks an import in agent_model_resolution.py, or removes _rewrite_upstream_model, would make these suites silently skip instead of fail, masking the regression. Convert them to hard imports now that the symbols exist.

4. Streaming body-rewrite is untested. Every _rewrite_upstream_model proxy-route test (TestRewriteUpstreamModelOnProxyRoute) posts non-streaming requests; _build_full_registry_patch even notes "Slice-2 tests target the non-streaming path." The streaming path (_send_and_prime → client.build_request(..., content=request_body)) does forward the rewritten body, but it's only covered transitively. Add a stream: true LiteLLM test asserting the rewritten model reaches build_request.

5. Classifier sharp edges (orchestrator/agent_model_resolution.py).

• It's case-sensitive: _CLAUDE_EXACT_ALIASES is lowercase-only and _CLAUDE_VERSIONED_RE is ^claude-, so default_agent_model: Opus or a typo like sonnett is classified as a LiteLLM model name, routed to the LiteLLM upstream, and fails only at request time when LiteLLM's model_list has no such entry. The operator intended Anthropic. It fails loudly enough at runtime not to be blocking, but consider case-normalizing and/or warning on near-miss aliases.
• _CLAUDE_VERSIONED_RE = ^claude- accepts any claude-* string including hallucinated names (claude-clone-12b), which then route to Anthropic with an invalid --model. The adversarial test test_litellm_alias_pin_is_opus_not_upstream_model actually feeds claude-clone-12b and only conditionally asserts — fine, but it documents the gap.

6. Empty-string agent_models value silently no-ops. Pydantic enforces dict[str, str], so values must be strings, but {"refiner": ""} passes validation and resolve_agent_model's if override: treats "" as unset → silent fallthrough to the default. Minor, but a deliberately-set (if nonsensical) value is dropped without signal.

7. LiteLLM Service has no dedicated NetworkPolicy. In practice this is mitigated — the existing egg-agents default-deny-egress plus the explicit gateway/orchestrator allow-rules mean untrusted agent pods cannot reach litellm.egg-system:4000. But k8s/base/ ships the Service cluster-reachable within egg-system with no ingress restriction, and the operator-supplied NetworkPolicy overlay is deferred. A defense-in-depth policy in k8s/base/ restricting LiteLLM ingress to component: gateway would close the gap before any operator populates model_list. Worth calling out prominently in docs/guides/per-agent-models.md as a required operator step.

8. LiteLLM ConfigMap master_key with an empty key. general_settings.master_key: os.environ/LITELLM_MASTER_KEY resolves to "" when the operator hasn't set LITELLM_MASTER_KEY (the Makefile always emits --from-literal=litellm-master-key=""). With an empty model_list this is an inert no-op so it doesn't block, but document that an operator enabling LiteLLM must set a real master key — an empty master_key plus a populated model_list would stand up an effectively unauthenticated proxy.

---

Things verified as correct (worth noting)

• The except json.JSONDecodeError, TypeError: lines in gateway.py and except AttributeError, ValueError: in test_concurrent_executor.py are valid — Python 3.14 (PEP 758) allows unparenthesized except tuples, and the repo runs 3.14. Visually confusable with Python 2 syntax, but not a bug.
• Credential isolation is sound: _get_forwarded_headers strips client authorization/x-api-key via ANTHROPIC_BLOCKED_HEADERS, and _inject_upstream_credentials injects only the per-upstream credential — no client OAuth token leaks to LiteLLM.
• Unknown-upstream handling fails closed with a deterministic 502 in _inject_upstream_credentials before any per-upstream branch.
• Spawn/restart plumbing is k8s-only (create_concurrent_spawn_fn / restart_agent_container exist only on KubernetesSpawner; restart_agent_container is an alias of the updated restart_agent_job), so the extra upstream/upstream_model kwargs cannot TypeError an un-updated spawner. The default-Anthropic path omits the new kwargs entirely, preserving the wire shape.
• AgentRole is the same class across egg_contracts.agent_roles, orchestrator.models, and egg_orchestrator.types (all re-export the canonical enum), so the resolver's isinstance(role, AgentRole) dict-lookup works regardless of caller import path.
• The body rewrite runs after _filter_blocked_tools and only on the non-Anthropic path; the Anthropic path stays byte-identical.

One observation outside the scope of this PR: per-agent model overrides are honored only on the concurrent-executor and restart_agent paths. With concurrent_phases defaulting to all three SDLC phases this covers the normal case, but a pipeline that narrows concurrent_phases would silently ignore agent_models for its sequential phases. Worth a follow-up note.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

1 previous review(s) hidden.