[issue-2769][slice-1/2] Gateway upstream router + LiteLLM Deploymen...

[james-in-a-box]

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.

This slice

Gateway upstream router + LiteLLM Deployment (no-op by default)

Files affected:

• gateway/upstream_registry.py
• gateway/anthropic_credentials.py
• gateway/gateway.py
• gateway/session_manager.py
• orchestrator/gateway_client.py
• k8s/base/litellm-deployment.yaml
• k8s/base/litellm-service.yaml
• k8s/base/litellm-configmap.yaml
• k8s/base/kustomization.yaml
• config/secrets.template.env
• tests/gateway/test_upstream_registry.py
• tests/gateway/test_anthropic_proxy.py
• tests/gateway/test_anthropic_credentials.py
• gateway/tests/test_session_manager.py
• orchestrator/tests/test_gateway_client.py
• docs/architecture/upstream-routing.md
• gateway/CLAUDE.md
• docs/architecture/orchestrator.md

Tasks:

• task-1-1: Introduce gateway/upstream_registry.py (NEW) containing an UpstreamRegistry class keyed by upstream name ("anthropic", "litellm"). Each registry entry pairs a singleton httpx.Client (base_url, timeout, connection limits) with a credential resolver returning an UpstreamCredential (the union of today's AnthropicCredential shape and the new LiteLLM x-api-key shape). Provide get(upstream: str) returning (client, credential_resolver), raising a typed UnknownUpstreamError on miss. Wire it into a get_upstream_registry() accessor that mirrors today's get_anthropic_client() lifetime semantics.
  • Acceptance criteria: - UpstreamRegistry.get("anthropic") returns a client with base_url == "https://api.anthropic.com" and the existing Anthropic credential resolver (preserves the # noqa: EGG200 annotation pattern at gateway/gateway.py:9325). - UpstreamRegistry.get("litellm") returns a client whose base_url is sourced from a new LITELLM_BASE_URL env var (default http://litellm.egg-system.svc.cluster.local:4000) and the LiteLLM credential resolver. - UpstreamRegistry.get("unknown") raises UnknownUpstreamError. - Both clients share the same timeout / pooling characteristics as today's _anthropic_client.
• task-1-2: Add a LiteLLM credential resolver to gateway/anthropic_credentials.py (or a sibling module if file-size discipline requires it). The resolver reads LITELLM_MASTER_KEY from secrets.env using the existing parse_env_file helper at gateway/anthropic_credentials.py:52, caches with the same mtime-invalidated pattern as AnthropicCredentialsManager, and returns a credential shaped header_name="x-api-key", header_value="<key>". Returns None when the key is absent (no-op default — matches today's behavior when Anthropic credentials are absent).
  • Acceptance criteria: - With LITELLM_MASTER_KEY unset, the resolver returns None and does not warn at startup. - With LITELLM_MASTER_KEY=foo, the resolver returns a credential with header_name == "x-api-key" and header_value == "foo". - secrets.env mtime change invalidates the cache the same way AnthropicCredentialsManager does.
• task-1-3: Make _inject_anthropic_credentials upstream-aware (rename to _inject_upstream_credentials(headers, upstream) and keep the old symbol as a back-compat alias calling through with upstream="anthropic"). Dispatch to the LiteLLM credential resolver when upstream == "litellm". Preserve the 401 / "no credential" error path verbatim for both.
  • Acceptance criteria: - _inject_upstream_credentials(headers, "anthropic") behaves byte-identically to today's _inject_anthropic_credentials(headers). - _inject_upstream_credentials(headers, "litellm") adds x-api-key: <LITELLM_MASTER_KEY> when the key is set. - Missing credentials for either upstream return a 401 with the same JSON body shape as today.
• task-1-4: Add upstream: str = "anthropic" and upstream_model: str | None = None to the Session dataclass at gateway/session_manager.py:288. Plumb them through Session.to_dict_for_persistence / Session.from_persistence so existing persisted sessions without the fields still load (defaults apply). Extend SessionManager.register_session (gateway/session_manager.py:548) to accept the two new optional parameters.
  • Acceptance criteria: - A Session created without the new fields keeps upstream == "anthropic" and upstream_model is None. - Session.to_dict_for_persistence / Session.from_persistence round-trip both fields losslessly and tolerate persisted dicts where the fields are absent. - SessionManager.register_session(upstream="litellm", upstream_model="qwen3-coder-30b") stores both on the returned Session.
• task-1-5: Wire upstream and upstream_model through the /api/v1/sessions/create route handler at gateway/gateway.py:8507 (parse from request body with their defaults, validate that upstream is one of the registered names from UpstreamRegistry, pass through to SessionManager.register_session). Log them in the existing audit_log("session_created", ...) call so the per-session upstream is auditable.
  • Acceptance criteria: - POSTing to /api/v1/sessions/create without the new fields creates a session with upstream="anthropic" and upstream_model is None. - POSTing with upstream="litellm", upstream_model="qwen3-coder-30b" creates a session with those values. - POSTing with upstream="bogus" returns a 400 with a descriptive error. - session_created audit log includes the upstream and upstream_model.
• task-1-6: Refactor proxy_anthropic_messages (gateway/gateway.py:9753) and proxy_count_tokens (gateway/gateway.py:10020) to resolve the upstream per request: replace client = get_anthropic_client() with the registry lookup using session.upstream (defaulting to "anthropic" when there is no session — preserves today's behavior). Replace _inject_anthropic_credentials(headers) calls with _inject_upstream_credentials(headers, session.upstream). Keep the SSE accumulator, tool-filter, and stream-resilience retry loop unchanged.
  • Acceptance criteria: - A request whose session has upstream == "anthropic" (or no session) hits the Anthropic httpx client and injects the Anthropic credential — byte-identical to today. - A request whose session has upstream == "litellm" hits the LiteLLM client and injects the LiteLLM credential. - _filter_blocked_tools, the _SSEAccumulator parse, and the connection-reset retry loop are unchanged in behavior and code shape (no new branches inside any of them). - proxy_count_tokens mirrors the same routing change.
• task-1-7: Extend GatewayClient.register_session at orchestrator/gateway_client.py:602 with optional upstream: str | None = None and upstream_model: str | None = None parameters. Include them in request_data only when set (matches the existing optional-field pattern at gateway_client.py:653-690). No caller in slice 1 passes them; this is purely the wire-shape.
  • Acceptance criteria: - GatewayClient.register_session(...) without the new args produces the same request body as today. - GatewayClient.register_session(upstream="litellm", upstream_model="qwen3-coder-30b") includes both keys in the POSTed JSON.
• task-1-8: Add k8s manifests for the LiteLLM proxy: k8s/base/litellm-deployment.yaml, k8s/base/litellm-service.yaml, and k8s/base/litellm-configmap.yaml. Deployment runs a pinned LiteLLM image in egg-system, mounts the ConfigMap at /app/config.yaml, exposes port 4000 (LiteLLM's default). Service is ClusterIP named litellm. ConfigMap ships with an EMPTY model_list so the deployment comes up healthy but serves nothing until operators populate it. Add all three to k8s/base/kustomization.yaml.
  • Acceptance criteria: - kubectl apply --dry-run=client -k k8s/base/ succeeds with the new resources included. - The LiteLLM Service resolves to litellm.egg-system.svc.cluster.local:4000, which matches the default LITELLM_BASE_URL baked into UpstreamRegistry. - No NetworkPolicy change to egg-agents egress — agents do not talk to LiteLLM directly.
• task-1-9: Document LITELLM_MASTER_KEY in config/secrets.template.env (one block below the ANTHROPIC_API_KEY block, with an explicit "leave empty to disable LiteLLM routing — no agent will be routed to LiteLLM with this unset" comment).
  • Acceptance criteria: - config/secrets.template.env contains a documented LITELLM_MASTER_KEY="" entry with the disable-when-empty note.
• task-1-10: Write unit tests covering the slice 1 gateway-side changes: tests/gateway/test_upstream_registry.py (new, covering the three registry cases — anthropic, litellm, unknown), extensions to tests/gateway/test_anthropic_credentials.py (LiteLLM resolver path), and extensions to tests/gateway/test_anthropic_proxy.py (the two routing branches for both proxy routes).
  • Acceptance criteria: - make test reaches and passes the new + extended tests. - Coverage includes the unknown-upstream error path, the "no credential" 401 path for both upstreams, and a byte-identity check for the Anthropic-routed request shape vs today.
• task-1-11: Write unit tests covering the slice 1 session-manager and orchestrator-client changes: extensions to gateway/tests/test_session_manager.py (round-trip the two new fields, register_session with defaults, register with explicit LiteLLM values) and to orchestrator/tests/test_gateway_client.py (omitted args → no new keys in body, explicit args → keys present).
  • Acceptance criteria: - All tests pass under make test. - The session-persistence test verifies a dict missing the new keys still rehydrates cleanly (back-compat guard).
• task-1-12: Author a new architecture doc docs/architecture/upstream-routing.md describing the UpstreamRegistry seam, the LiteLLM topology, the per-session routing decision, the credential layout, and the cq-1 / cq-2 / cq-5 / cq-7 / cq-8 resolutions that shape it. Cross-link from gateway/CLAUDE.md and docs/architecture/orchestrator.md.
  • Acceptance criteria: - The doc names every primitive added in slice 1 with a file:line cite, explains the no-op-by-default invariant, and walks through the request lifecycle for both upstreams. - gateway/CLAUDE.md and docs/architecture/orchestrator.md link to it.

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.

Stack

• Position: slice 1 of 2 in pipeline issue-2769
• Stacked on top of egg/issue-2769/work

Slice slice-1 of pipeline issue-2769. Stacked on top of egg/issue-2769/work.

[james-in-a-box]

Autofix tracking

{"Lint/Custom Checks": 1, "Lint/Python": 1}

[egg-reviewer]

No agent-mode design concerns.

This is gateway/infrastructure plumbing (upstream router, session fields, k8s manifests) — it doesn't construct agent prompts, pre-fetch data, define agent workflows, or post-process agent output, so most of the agent-mode anti-patterns don't apply. I checked the ones that could:

• Constraint enforcement (guideline: "the sandbox is the constraint"). The upstream-routing decision is gateway-enforced per-session metadata declared by the orchestrator at session-create, not a prompt-level instruction. The cq-2 decision explicitly rejected a sandbox-supplied HTTP header in favor of session metadata — the right call: routing stays a trust-boundary decision the agent cannot influence. Credentials remain gateway-held and never reach the sandbox. This strengthens the gateway-as-constraint model.

• Agent SDK / Claude Code harness path. Preserved. The sandbox still talks to a single URL (ANTHROPIC_BASE_URL=http://egg-gateway:9848); the router is invisible to it. cq-5 keeps the Claude Code harness even for non-Claude backends, so the Agent SDK invocation path is unchanged.

• EGG200 (direct LLM calls in infra code). upstream_registry.py is a proxy client, not a decision-making LLM call — consistent with the existing get_anthropic_client() exception. The # noqa: EGG200 annotations on the api.anthropic.com literal and client construction follow the established pattern.

• EGG201 (pinned model identifiers). No pinned Claude identifiers introduced; the opus alias is used for the Claude-Code-facing model. upstream_model values like qwen3-coder-30b are non-Claude on-the-wire names that have no alias concept — EGG201 doesn't govern them.

The internal orchestrator↔gateway JSON fields (upstream/upstream_model) are a genuine machine-to-machine API, not human-facing structured output.

— Authored by egg

[egg-reviewer]

Review — PR #2773: Gateway upstream router + LiteLLM Deployment (slice 1/2)

I traced the full changeset against the existing gateway code, the k8s
manifests, and the deploy tooling. The core seam is well-built and the
no-op-by-default invariant for the request path genuinely holds —
verified end-to-end:

• Session.upstream defaults to "anthropic"; from_persistence
  tolerates pre-#2769 dicts (gateway/session_manager.py:419-420).
• proxy_anthropic_messages / proxy_count_tokens keep calling
  get_anthropic_client() on the "anthropic" branch, so existing
  mocks and behavior are byte-identical.
• No slice-1 caller sets upstream="litellm".
• The NetworkPolicy claim checks out: all policies are scoped to
  egg-agents; egg-system has none, so gateway→litellm works and
  agents still cannot reach litellm:4000. No policy change needed.

One blocking issue and several non-blocking items below.

---

Blocking

1. make deploy now hard-depends on an external image pull, and a failed pull aborts the whole deploy with a misleading error.

The LiteLLM Deployment is added to k8s/base/kustomization.yaml, and
k8s/overlays/local/ includes ../../base, so every make deploy
unconditionally schedules the litellm pod. Its image
(ghcr.io/berriai/litellm:main-v1.55.10) is pulled from an external
registry — it is not in the k3s-import list and not rewritten by
the deploy sed.

make deploy runs scripts/await-egg-deploy.sh, which at lines 63-71
scans all egg-system pods for ImagePullBackOff|ErrImagePull:

kubectl -n "$NS" get pods -o jsonpath='{range .items[*]}...' | grep -qE 'ImagePullBackOff|ErrImagePull'

If the litellm image is unpullable — wrong tag, ghcr.io outage,
air-gapped cluster — the litellm pod enters ImagePullBackOff, this
grep matches, and the entire deploy aborts with:

ERROR: egg-system pods cannot pull image tag '<TAG>' — it is not in k3s.
A commit, pull, or rebase since your last build moved EGG_IMAGE_TAG.

That message blames egg's own image tag drift, but the real culprit
is the third-party litellm image. This breaks make deploy for
everyone — including operators who never opt into LiteLLM — and
points them at the wrong fix. The no-op-by-default invariant holds for
the request path but not for the deploy path.

Please address one of:

• Narrow await-egg-deploy.sh's ImagePullBackOff scan to the
  orchestrator/gateway pods (by label selector) so unrelated pods
  don't fail the deploy; or
• Pre-import the litellm image (extend k3s-import) so it isn't
  pulled from ghcr at apply time; or
• Move the litellm resources out of k8s/base/ into an opt-in
  overlay so they're only deployed when an operator wants LiteLLM.

Also please confirm the pinned tag main-v1.55.10 actually exists and
pulls — if it doesn't, this breaks every deploy immediately, not just
in degraded environments.

---

Non-blocking

2. _inject_upstream_credentials silently treats unknown upstreams as Anthropic.
gateway/gateway.py:9404 only special-cases "litellm"; every other
value (including a bogus upstream) falls through to the Anthropic
branch and injects the Anthropic credential. In the current proxy flow
this is masked — the subsequent get_upstream_registry().get(...)
raises UnknownUpstreamError → 502 before any upstream call, so no
credential leaves the gateway. But it produces an observable
inconsistency: for a session with an unknown upstream, the request
returns 401 when no Anthropic credential is configured (inject
fails first) and 502 when one is (inject succeeds, client lookup
fails). Same invalid input, two different error codes depending on
unrelated state. test_unknown_upstream_on_session_returns_5xx only
passes because it mocks an Anthropic credential present; with None
it would get a 401 and fail the 5xx assertion. Consider having
_inject_upstream_credentials reject unknown upstreams explicitly
rather than defaulting them to Anthropic.

3. register_session does not validate upstream.
Only the /api/v1/sessions/create route validates against
UpstreamRegistry.is_known(). Both SessionManager.register_session
(gateway/session_manager.py:579) and
GatewayClient.register_session accept any string. Slice 2's spawner
will call these directly — a resolution bug there would create a
bogus-upstream session that bypasses validation entirely and lands in
the inconsistent path from item 2. Worth a validation guard in
register_session itself, or at least a note for slice 2.

4. litellm pod health is asserted but unverified.
The Test Plan only runs kubectl apply --dry-run=client. The claim
that the pod "comes up healthy" with an empty model_list is
untested, and readOnlyRootFilesystem: true with only /tmp writable
is a real risk if LiteLLM writes a cache/log to $HOME or its install
dir at startup. Since await-egg-deploy.sh only waits on
orchestrator/gateway, a crashlooping (not image-pull) litellm pod
won't break the deploy — but it will sit broken in every cluster.
Please actually start the pod once and confirm
/health/readiness + /health/liveliness pass before relying on the
"healthy idle" claim in the ConfigMap comment.

5. Error messages hardcode "Anthropic API" on every upstream.
proxy_anthropic_messages / proxy_count_tokens return
"Failed to connect to Anthropic API" / "Anthropic API request timed out" even for a litellm-routed request (gateway/gateway.py:10101,
10112, 10186, 10197). Not slice-1-reachable, but it will mislead
operators debugging a LiteLLM outage in slice 2. Cheap to make
upstream-aware now.

6. Test skip-guards mask regressions now that the implementation has landed.
Many of the new tests still carry scaffold-era guards —
try: import upstream_registry except ImportError: pytest.skip(...)
and except TypeError: pytest.skip(...). With the module merged these
no longer skip, but they now mean a future rename/break would make the
suite skip silently instead of failing. Remove them so the suite
fails loudly. Relatedly, TestSessionCreateUpstreamValidation skips
entirely when EGG_LAUNCHER_SECRET is unset — if CI doesn't set it,
the session-create upstream validation branch (TASK-1-5) has no
automated coverage. Consider a unit test that exercises the validation
without needing launcher auth.

7. The registry's credential-resolver half is unused by the proxy routes.
UpstreamRegistry.get() returns (client, resolver), but the proxy
routes discard the resolver (client, _ = ...) and inject credentials
via the separate _inject_upstream_credentials. Both ultimately call
the same managers so they stay consistent, but the paired-resolver
abstraction is effectively dead in production (exercised only by
tests). Not wrong — just worth noting the redundancy.

8. Minor:

• ("anthropic", "litellm") is hardcoded in three places in
  upstream_registry.py (get, is_known, known_upstreams) plus
  the litellm-only branch in _inject_upstream_credentials. A 4th
  upstream means editing all four in sync.
• docs/architecture/upstream-routing.md references cq-9 in prose
  but the HITL table only lists cq-1/2/5/7/8.
• docs/architecture/orchestrator.md links to
  ../guides/per-agent-models.md, which doesn't exist until slice 2 —
  a dangling link in merged docs meanwhile.
• gateway/upstream_registry.py:1 puts # noqa: EGG200 - ... as the
  first line of the module docstring; it works for the line-based lint
  but becomes the literal first line of __doc__. A normal trailing
  # noqa comment on the import/constant lines reads cleaner.

---

What's good

• The no-op-by-default request path is genuinely verified, not just
  asserted — defaults, persistence back-compat, and the unchanged
  Anthropic branch all hold.
• upstream_model input validation in session_create (type,
  non-empty, ≤256 chars) is solid defensive validation.
• The LiteLLM credential resolver correctly refuses to fall back to
  ANTHROPIC_API_KEY, and the test suite explicitly guards that
  cross-upstream leak — good adversarial coverage.
• The k3s-secrets change correctly closes the gap where the LiteLLM
  pod and gateway would otherwise disagree on the master key.
• Routing tests drive the real Flask routes end-to-end rather than
  hand-building fixtures.

The blocking item is the deploy coupling (#1); the rest are
non-blocking. Happy to re-review once #1 is addressed.

— Authored by egg

[james-in-a-box]

Review feedback addressed — commit b914607

Thanks for the thorough trace. Every item is fixed in this PR; no deferrals.

Blocking

1. make deploy aborts on the litellm image pull / misleading error — fixed-in-PR (commit b914607)
scripts/await-egg-deploy.sh's ImagePullBackOff fast-fail now scans only egg's own deployments (orchestrator/gateway) via the app.kubernetes.io/component label selector, looping the existing DEPLOYMENTS array. A third-party pod (the LiteLLM proxy, image pulled from an external registry) failing to pull no longer aborts make deploy or mis-blames EGG_IMAGE_TAG — the tag-drift fast-fail only ever applies to images egg actually builds and tag-rewrites. I picked this option (vs. pre-importing the image or moving the manifests to an overlay) because it is the root-cause fix: the script's premise is egg tag drift, so it should only ever blame egg's own pods. Confirmed ghcr.io/berriai/litellm:main-v1.55.10 resolves — ghcr manifest request returns HTTP 200.

Non-blocking

2. _inject_upstream_credentials silently treats unknown upstreams as Anthropic — fixed-in-PR (commit b914607)
Added an explicit guard at the top: an upstream not in UpstreamRegistry fails closed with a deterministic 502 before the Anthropic branch, so the same invalid input no longer yields 401-vs-502 depending on unrelated Anthropic-credential state. New regression test test_unknown_upstream_returns_502_without_anthropic_fallthrough exercises exactly the None-credential case you flagged.

3. register_session does not validate upstream — fixed-in-PR (commit b914607)
SessionManager.register_session now validates upstream against UpstreamRegistry and raises ValueError on an unknown value — a direct caller (the slice-2 spawner, tests) can no longer create a bogus-upstream session. New test test_register_with_unknown_upstream_raises. GatewayClient.register_session (orchestrator side) gets a docstring note that the gateway is authoritative and 400s an unknown value — I kept the orchestrator a thin client rather than duplicating the known-upstreams list across the trust boundary.

4. litellm pod health asserted but unverified — fixed-in-PR (commit b914607) (hardening) + verification note
Addressed the concrete readOnlyRootFilesystem risk: the Deployment now sets HOME=/tmp so any home-relative startup write LiteLLM performs (cache/config dirs, and the XDG_* defaults that derive from $HOME) lands in the writable tmpfs instead of failing against the read-only root. I cannot start a live pod in this sandbox (no cluster for an arbitrary external image), so a real /health/readiness + /health/liveliness run still needs an operator with a cluster — but the HOME=/tmp change removes the specific failure mode that made the "healthy idle" claim risky.

5. Error messages hardcode "Anthropic API" on every upstream — fixed-in-PR (commit b914607)
proxy_anthropic_messages / proxy_count_tokens ConnectError / TimeoutException / Exception handlers (and their log lines) are now upstream-aware — f"Failed to connect to {upstream_name} upstream" etc., with upstream=upstream_name structured-log context.

6. Scaffold skip-guards mask regressions — fixed-in-PR (commit b914607)
Removed every scaffold-era guard: the try: import upstream_registry except ImportError: skip, except TypeError: skip, and except AttributeError: skip blocks across test_anthropic_proxy.py, test_anthropic_credentials.py, test_upstream_registry.py, test_session_manager.py, and test_gateway_client.py, plus the two-shape _import_litellm_resolver indirection. A future rename now fails the suite loudly. TestSessionCreateUpstreamValidation no longer depends on EGG_LAUNCHER_SECRET — it patches get_launcher_secret and sends the matching bearer, so the TASK-1-5 validation branch has deterministic coverage (test_bogus_upstream_returns_400 + test_known_upstreams_pass_validation).

7. Registry credential-resolver half is unused by the proxy routes — disagree (deliberate design, not redundancy)
The paired (client, resolver) is intentional: the registry is the single source of truth for the upstream→credential mapping and stays self-describing and test-covered. The proxy routes use the dedicated _inject_upstream_credentials because the Anthropic branch carries the OAuth-passthrough / client-supplied-auth fall-through, which is upstream-specific behavior that does not belong inside a generic registry resolver. Routing injection through registry.get(...)[1] would also break the test isolation that patches gateway.gateway.get_litellm_credentials_manager directly. Since you noted this is "not wrong," I've kept the pairing as-is.

8. Minor — all fixed-in-PR (commit b914607)

• KNOWN_UPSTREAMS module constant in upstream_registry.py replaces the three hardcoded ("anthropic", "litellm") tuples in get / is_known / known_upstreams.
• Added the cq-9 row to the upstream-routing HITL table (and updated "five" → "six") so the table matches the prose.
• De-linked the per-agent-models.md references in orchestrator.md and upstream-routing.md — they are now plain text noting the file ships in slice 2, so merged docs carry no dangling link.
• Dropped the # noqa: EGG200 from the module docstring's first line by rewording the docstring to not name api.anthropic.com (the ANTHROPIC_BASE_URL constant keeps its trailing noqa, which reads cleanly).

Verification

ruff check + ruff format --check clean; check-llm-api-calls.py (EGG200) clean; shellcheck clean on the deploy script; mypy clean for the changed files. Test suites green: test_upstream_registry.py (10), test_anthropic_credentials.py (23), test_anthropic_proxy.py (66), test_session_manager.py + test_gateway_client.py (236), and the session-create/proxy slice of test_gateway.py (25).

— Authored by egg

[egg-reviewer]

Agent-mode design re-review — no concerns.

The delta since my last review (commit b9146074) is purely gateway/infra plumbing in response to review feedback:

• _inject_upstream_credentials / register_session now fail closed on an unknown upstream.
• Proxy error messages and logs are made upstream-aware.
• KNOWN_UPSTREAMS constant deduplicates the upstream name list.
• HOME=/tmp added to the LiteLLM Deployment; await-egg-deploy.sh scopes its image-pull fast-fail to egg's own deployments.
• Scaffold-era test skip-guards removed; docs corrected.

None of this touches agent-mode design surfaces — no prompt construction, no pre-fetching, no structured-output-for-humans, no post-processing pipelines, no prompt-level security, no direct LLM API calls, and no hardcoded model identifiers. The gateway proxies whatever model the client sends; it does not pin one.

Worth noting the delta improves agent-mode hygiene: it removes a # noqa: EGG200 from upstream_registry.py's module docstring by rewording the prose instead of suppressing the linter. The remaining # noqa: EGG200 on the ANTHROPIC_BASE_URL constant is appropriate — that is a proxy target URL, not a direct LLM call.

— Authored by egg

[egg-reviewer]

Re-review — PR #2773: Gateway upstream router + LiteLLM Deployment (slice 1/2)

Re-reviewed at commit b914607. The previous-review commit was no longer
reachable (branch force-pushed), so I re-traced the full changeset against
the base branch and verified each prior item against the current tree.

Verdict: the blocking issue is genuinely fixed and every non-blocking
item is addressed. Approving. Three cosmetic notes below — none block.

Blocking issue #1 — make deploy aborting on the litellm image pull — RESOLVED

scripts/await-egg-deploy.sh now scopes the ImagePullBackOff fast-fail
to egg's own deployments: it loops DEPLOYMENTS=(orchestrator gateway)
and queries pods by -l app.kubernetes.io/component=$d. I verified the
selector is correct — both orchestrator-deployment.yaml and
gateway-deployment.yaml carry app.kubernetes.io/component on the pod
template labels (gateway-deployment.yaml:20,
orchestrator-deployment.yaml:25), so the pod-level scan matches. A
third-party litellm ImagePullBackOff no longer aborts the deploy or
mis-blames EGG_IMAGE_TAG. The success-path check (lines 39-58) also only
waits on orchestrator/gateway, so a crashlooping litellm pod won't
cause a timeout either.

I also independently confirmed the pinned tag resolves:
GET ghcr.io/v2/berriai/litellm/manifests/main-v1.55.10 returns HTTP
200.

Non-blocking items from prior review — all addressed

• #2 _inject_upstream_credentials now rejects an unknown upstream
  with a deterministic 502 via is_known() before the Anthropic
  branch — no more 401-vs-502 ambiguity. Regression test
  test_unknown_upstream_returns_502_without_anthropic_fallthrough
  exercises the real injector with None Anthropic credential.
• #3 SessionManager.register_session validates upstream against
  the registry and raises ValueError. Test
  test_register_with_unknown_upstream_raises covers it; the lazy
  from upstream_registry import ... correctly avoids a circular import.
• #4 HOME=/tmp added to the litellm Deployment, removing the
  concrete readOnlyRootFilesystem startup-write failure mode. Live
  /health/readiness + /health/liveliness verification still needs an
  operator with a cluster — acceptable for a non-blocking item, but worth
  flagging to whoever first deploys this.
• #5 Proxy error messages / log lines are upstream-aware
  (f"Failed to connect to {upstream_name} upstream").
• #6 Every scaffold-era skip-guard removed — verified no
  pytest.skip / except ImportError / except TypeError remains in
  the five touched test files. TestSessionCreateUpstreamValidation
  patches get_launcher_secret directly, so TASK-1-5 has deterministic
  coverage.
• #7 Disagreement on the unused resolver half is reasonable — the
  prior review explicitly said "not wrong."
• #8 KNOWN_UPSTREAMS constant replaces the three duplicated tuples;
  cq-9 row added to the HITL table ("six" matches); per-agent-models.md
  references de-linked to plain text; docstring # noqa removed.

I ran the suites locally: test_upstream_registry.py (10),
test_anthropic_proxy.py + test_anthropic_credentials.py (89),
test_session_manager.py + test_gateway_client.py (236) — 335
passed. The new tests drive the real Flask routes / real
SessionManager / real UpstreamRegistry rather than hand-built
fixtures, and the credential-leak guards (TestLiteLLMNoFallbackToClientAuth)
are solid adversarial coverage.

Non-blocking notes

1. register_session validates upstream but not upstream_model.
   The session-create HTTP route validates upstream_model (type,
   non-empty, ≤256 chars — gateway.py:8682-8688), but the direct
   register_session path (session_manager.py:580) stores it
   unchecked. The same defense-in-depth argument that justified adding
   upstream validation here (a slice-2 spawner calling register_session
   directly) applies equally to upstream_model. Low impact in slice 1
   since it's just a stored string, but worth a guard or a slice-2 note.

2. Stale docstring in test_unknown_upstream_on_session_returns_5xx.
   The docstring describes a 500-via-dual-UnknownUpstreamError-identity
   path. Now that _inject_upstream_credentials rejects unknown upstreams
   first via the boolean is_known() check, the route returns a
   deterministic 502 and the except UnknownUpstreamError in the
   client-resolution block is effectively unreachable (harmless defensive
   code). The test still passes (502 ∈ 5xx), but the docstring no longer
   matches the code path.

3. test_anthropic_default_omitted_from_persistence docstring says
   "Marked xfail-strict-false" but the test is a plain test with
   conditional asserts, not an xfail. Cosmetic.

The slice-1 no-op-by-default invariant now holds for both the request
path (defaults, persistence back-compat, unchanged Anthropic branch) and
the deploy path. Good work on the trace-through.

— Authored by egg

[james-in-a-box]

Review feedback addressed — commit 020569d

Thanks for the re-review. All three non-blocking notes are fixed in this PR; no deferrals.

1. register_session validates upstream but not upstream_model — fixed-in-PR (commit 020569d)
SessionManager.register_session now applies the same upstream_model checks the /api/v1/sessions/create route does (type is str, non-empty, ≤256 chars), raising ValueError on a violation. This closes the defense-in-depth gap you flagged: a slice-2 spawner calling register_session directly can no longer store a malformed model name that would only surface later in the body-rewrite path. The docstring Raises: section is updated to cover it, and two regression tests were added — test_register_with_empty_upstream_model_raises and test_register_with_oversized_upstream_model_raises.

2. Stale docstring in test_unknown_upstream_on_session_returns_5xx — fixed-in-PR (commit 020569d)
Rewrote the docstring to describe the current code path: _inject_upstream_credentials runs the UpstreamRegistry.is_known() pre-check before any per-upstream branch, so an unregistered upstream gets a deterministic 502 and the except UnknownUpstreamError in the client-resolution block is now unreachable defensive code. The docstring no longer references the obsolete dual-import 500 scenario. The assertion still ranges over 5xx — the fail-closed contract, not the exact code, is what the test guards.

3. test_anthropic_default_omitted_from_persistence docstring claims "xfail-strict-false" — fixed-in-PR (commit 020569d)
Reworded the docstring — the test is a lenient conditional-assert test (accepts an omitted field or one present at its default; fails only on an unexpected non-default value), not an xfail. The inaccurate "Marked xfail-strict-false" sentence is removed.

Verification

ruff check + ruff format --check clean on all three changed files. Test suites green: test_session_manager.py (134) and test_anthropic_proxy.py (66) — 200 passed, including the two new register_session validation tests.

— Authored by egg

[egg-reviewer]

Re-review — PR #2773: agent-mode design

Re-reviewed the delta since my last approval at b914607. The only new
commit is 020569dc ("Address re-review suggestions"), a 3-file change:

• gateway/session_manager.py — register_session now validates
  upstream_model (type, non-empty, ≤256 chars), mirroring the
  /api/v1/sessions/create route.
• gateway/tests/test_session_manager.py — docstring reword + two new
  validation regression tests.
• tests/gateway/test_anthropic_proxy.py — docstring reword.

No agent-mode design concerns. The delta is gateway-side input
validation and test/docstring hygiene — it touches no prompt templates,
no spawn-side plumbing, and no agent code. The upstream_model field it
guards is an upstream-side model name destined for LiteLLM (e.g.
qwen3-coder-30b), not a Claude model alias, so EGG201 (hardcoded model
identifiers) does not apply. The slice-1 no-op-by-default invariant is
unaffected.

The prior review's verdict stands.

— Authored by egg

[egg-reviewer]

Re-review — PR #2773: Gateway upstream router + LiteLLM Deployment (slice 1/2)

Re-reviewed at commit 020569d. The prior-review commit b9146074 was no
longer reachable (the single PR commit was amended), so I traced the full
changeset against base egg/issue-2769/work and verified each of the three
non-blocking notes from the previous re-review against the current tree.

Verdict: all three notes are genuinely addressed, not superficially.
Approving — no blocking issues, no new issues introduced.

Note #1 — register_session validates upstream but not upstream_model — RESOLVED

SessionManager.register_session (session_manager.py:614-635) now mirrors
the session-create route's upstream_model checks exactly: type is str,
non-empty, ≤256 chars. I diffed the two validation blocks — the
register_session checks are byte-equivalent in semantics to
gateway.py:8683-8689 (only the error surface differs, ValueError vs
make_error, which is correct for the two call contexts). The validation
runs before token generation and Session construction, so a rejected
input leaves no partial state. The Raises: docstring section is updated
to cover both upstream and upstream_model.

Two regression tests added and verified passing:
test_register_with_empty_upstream_model_raises and
test_register_with_oversized_upstream_model_raises. Both drive the real
SessionManager.register_session (no hand-built fixture, no mock of the
validation path) with upstream="litellm" so the upstream pre-check
passes and the upstream_model branch is actually reached — the production
code path is exercised.

Note #2 — stale docstring in test_unknown_upstream_on_session_returns_5xx — RESOLVED

The docstring (test_anthropic_proxy.py:1853-1865) now accurately
describes the current path: _inject_upstream_credentials runs the
UpstreamRegistry.is_known() boolean pre-check before any per-upstream
branch, so an unregistered upstream gets a deterministic 502 and the
except UnknownUpstreamError in the client-resolution block is now
unreachable defensive code. The obsolete dual-import-500 scenario is gone.
The assertion still ranges over 5xx — guarding the fail-closed contract
rather than the exact code — which the docstring now explains.

Note #3 — test_anthropic_default_omitted_from_persistence "xfail-strict-false" claim — RESOLVED

The docstring is reworded to accurately describe the test as a lenient
conditional-assert guard (accepts an omitted field or one present at its
default; fails only on an unexpected non-default value). The inaccurate
"Marked xfail-strict-false" sentence is removed — the test is a plain test
with conditional asserts, never an xfail.

Verification

• python -m pytest gateway/tests/test_session_manager.py::TestSessionManagerRegisterUpstream — 6 passed, including the two new validation tests.
• test_anthropic_default_omitted_from_persistence + test_unknown_upstream_on_session_returns_5xx — 2 passed.
• ruff check clean on session_manager.py, test_session_manager.py, test_anthropic_proxy.py.

The slice-1 no-op-by-default invariant is unchanged by this delta — it
touches only validation hardening and test docstrings. The blocking issue
and all prior non-blocking items remain resolved. Good close-out.

— Authored by egg

[egg-reviewer]

Agent-mode design re-review — no concerns

Re-reviewed the delta since my last approval at 020569dc. The only new PR-authored commit is 1ff4803 (slice-2: per-agent non-Claude model support); f17948251 is a no-op merge of origin/main.

Slice-2 adds the per-agent model resolver, config plumbing, and the gateway body-rewrite half of the cq-5 mitigation. I checked the agent-mode anti-patterns that could apply:

• Constraint enforcement (sandbox, not prompt). Upstream routing stays gateway-enforced per-session metadata (Session.upstream / upstream_model threaded through spawn/restart), not a prompt-level instruction. _rewrite_upstream_model runs in the gateway proxy path.
• Direct LLM API calls / Agent SDK bypass. None. Agents still spawn via build_consensus_wrapped_command → egg_agent. LiteLLM is an in-cluster proxy upstream; the gateway remains a proxy and makes no direct Anthropic API calls.
• Hardcoded model IDs (EGG201). The resolver uses short aliases (opus/sonnet/haiku). The ^claude- regex exists to recognize versioned IDs an operator might pass and route them correctly — that's the intended classifier behavior, not a violation. The single # noqa: EGG201 sits on a docstring example, which is appropriate.
• cq-5 mitigation. Handing Claude Code --model opus on LiteLLM-routed agents while the gateway swaps the body's model name is a technical workaround for compaction heuristics — it enables non-Claude models rather than constraining agent flexibility.

No pre-fetching, structured-output-for-humans, post-processing, or rigid-procedure concerns. The new docs/guides/per-agent-models.md is an operator how-to, not an agent prompt.

— Authored by egg

[egg-reviewer]

Re-review — PR #2773 (slice-2 delta: per-agent non-Claude model support)

Re-reviewed at HEAD f1794825. Since my last review at 020569dc the only PR-authored
commit is 1ff4803 ("[issue-2769][merge-gate]", the slice-2 content: per-agent model
resolver, config plumbing, gateway body-rewrite); f1794825 is a no-op merge of
origin/main.

Verdict: no blocking issues. The feature wires end-to-end correctly. One non-blocking
inconsistency and a couple of doc nits, detailed below. (Self-authored merged PR — posting
as a comment.)

Verification performed

• Traced the full data flow: agent_models → PipelineConfig validator → _run_concurrent_phase
  → ConcurrentPhaseExecutor._spawn_agent → resolve_agent_model → spawn_fn/spawn_agent_job
  → register_session → Session.upstream/upstream_model → proxy_anthropic_messages/
  proxy_count_tokens → _rewrite_upstream_model. The refiner (the operator-guide's primary
  example) does reach the resolver — all phases run through _run_concurrent_phase per
  _run_pipeline's docstring, confirmed.
• build_consensus_wrapped_command already accepts model: str = "opus", so the new
  model=decision.claude_code_alias call is byte-identical on the default path.
• restart_agent_container is an alias of restart_agent_job (kubernetes_spawner.py:1753),
  which the PR extended with upstream/upstream_model — no signature mismatch.
• Content-Length: content-length is in ANTHROPIC_BLOCKED_HEADERS, so the body-length
  change from _rewrite_upstream_model (and from the pre-existing _filter_blocked_tools)
  is safe — httpx recomputes it.
• Ran the slice-2 suites: test_agent_model_resolution.py (33 passed),
  test_concurrent_executor.py slice-2 tests (15 passed), test_anthropic_proxy.py
  rewrite tests (13 passed). All exercise the real production code path (real resolver,
  real _rewrite_upstream_model, real Flask proxy route) — no hand-built-fixture bypass,
  no self-seeding goldens.

Non-blocking — restart path resolves the model for every role, including the resolver-bypassing ones

restart_agent (routes/pipelines.py:2255) accepts any valid AgentRole (only
AgentRole(agent_role) is validated, no phase-role restriction) and calls
resolve_agent_model(role=role, …) unconditionally for whatever role is restarted.

This contradicts the design invariant the PR itself documents. MODEL_OVERRIDE_ROLES and
the PipelineConfig.agent_models validator exist because — per the validator's own comment
(models.py) and the agent_roles.py comment — "Utility roles (AUTOFIXER,
CONFLICT_RESOLVER) and interface roles (OVERSEER, INSPECTOR) spawn through dedicated paths
that never call the resolver." That is true for the initial spawn but false for the
restart path.

Concrete consequence: with a repo-level default_agent_model: qwen3-coder-30b set, the
initial overseer/autofixer/conflict_resolver/inspector spawn stays on
anthropic/opus (it bypasses the resolver), but an operator restart of that same role
via the restart_agent HTTP route resolves tier-2 → a LiteLLM decision → the restarted
agent registers its gateway session with upstream=litellm. Same role, two code paths,
two different upstreams — and these roles were deliberately carved out of the override
surface precisely because they were never validated for a non-Claude backend.

The per-pipeline agent_models knob is protected (the validator rejects these role keys),
but the repo-level default_agent_model has no role filtering and leaks through the
restart path. Blast radius is narrow (needs a non-Claude repo default and a restart of a
non-phase role, and it is a misroute rather than a crash or state corruption), so this is
non-blocking — but it is a genuine "same input, different result across code paths"
inconsistency in code this PR introduces. Suggested fix: in restart_agent, skip
resolution (use the built-in classify_model("opus") default) when
role not in MODEL_OVERRIDE_ROLES, mirroring the initial-spawn behavior.

Non-blocking — stale file:line cites in docs/guides/per-agent-models.md

The guide cites proxy_anthropic_messages at gateway/gateway.py:9839 (actual 9922),
proxy_count_tokens at :10135 (actual 10227), and _filter_blocked_tools at :9496
(actual 9522) — off by ~25–90 lines. The merge of origin/main shifted gateway.py
after the doc was authored. The cites still land in the right neighborhood, so this is a
minor accuracy nit, but given the PR explicitly chased cite accuracy elsewhere
(models.py:405 → :757), worth a refresh.

Non-blocking — default_agent_model "every role" wording

The guide (step 3a) describes default_agent_model as applying to "every role". On the
initial spawn it only reaches roles routed through the resolver (phase producers +
reviewers); utility/interface roles are unaffected. Minor imprecision — the validator
section later clarifies the distinction, so a careful reader recovers, but the step-3a
prose overstates the scope.

Good close-out otherwise: the no-op-by-default invariant holds (default config →
byte-identical build_consensus_wrapped_command args, no register_session kwargs), the
_rewrite_upstream_model helper handles every edge case the tests probe (None /
invalid-JSON / non-dict / missing-model-key), and the defensive try/except around
resolve_agent_model at both spawn sites prevents a resolver regression from crashing
pipelines.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

16 previous review(s) hidden.