Skip to content

feat(orch): api_delegate MCP tool for HTTP-only providers (xAI Grok) - #26

Open
cnighswonger wants to merge 2 commits into
ArkNill:mainfrom
cnighswonger:feat/api-delegate-grok-upstream
Open

feat(orch): api_delegate MCP tool for HTTP-only providers (xAI Grok)#26
cnighswonger wants to merge 2 commits into
ArkNill:mainfrom
cnighswonger:feat/api-delegate-grok-upstream

Conversation

@cnighswonger

Copy link
Copy Markdown
Contributor

Summary

Adds an api_delegate MCP tool for HTTP-only LLM providers (no local CLI binary), with xAI Grok as the initial provider. Sits alongside the existing cli_delegate so callers can choose either pathway without changing how results are consumed downstream.

cli_status and cli_probe extended to surface API providers alongside CLI providers; each row carries a new kind field ("cli-binary" vs "http-api") so callers can distinguish.

Motivation

Grok is useful as a sometimes-adversarial reviewer in writing workflows. Wrapping it as a fake CLI shim felt like more friction than value (the xAI API is one HTTP endpoint), so this adds a sibling path that treats HTTP-only providers natively. Future providers (OpenAI direct API, Anthropic direct API) drop into the same PROVIDERS dict — endpoint, default model, key resolver, response extractor.

Design

New module: src/llm_relay/orch/api_executor.py

Stdlib-only (urllib + ssl). Mirrors executor.py's shape so MCP/DB/history machinery treats both paths uniformly.

  • PROVIDERS dict keyed by short name; each entry has provider_id, endpoint, default_model, key_resolver callable, extract callable, auth_method, api_key_name.
  • execute_api(provider, prompt, model=None, system=None, timeout=120, max_tokens=4000, temperature=0.3) returns DelegationResult shaped identically to CLI execution — exit_code=0 on success per the CLI-executor convention (proxy/composition.py:814 checks exit_code == 0).
  • xAI/Grok key resolution: XAI_API_KEY_PATH (file) → ~/.llm-relay/grok.key~/grok.key (legacy) → XAI_API_KEY env var.
  • Key file permission gating: requires POSIX mode 0o600 or stricter; loose-permission files silently skipped with debug log. Windows exempt (st_mode bits don't carry equivalent meaning).
  • HTTPS scheme guard: refuses non-HTTPS endpoints before urlopen() is reached. PROVIDERS dict is in-tree, so this is defense against future misedits rather than untrusted input.
  • Provider-supplied hooks (key_resolver, extract) wrapped in try/except so a misbehaving provider returns a clean DelegationResult instead of bubbling.

New MCP tool: api_delegate

api_delegate(provider, prompt, model="", system="", timeout=120, max_tokens=4000)
  • Logs to delegation DB with strategy="api-direct".
  • Respects LLM_RELAY_HISTORY=1 for session-history capture.
  • Returns the same JSON envelope as cli_delegate plus model_used.

Extended: cli_status and cli_probe

Both surface API providers in addition to CLI providers. New kind field distinguishes:

{"cli_id": "claude-code", "kind": "cli-binary", ...}
{"cli_id": "xai-grok",    "kind": "http-api",   ...}

Tests

24 new in tests/test_orch/test_api_executor.py covering:

  • Provider listing + status (with and without a usable key)
  • All error paths: unknown provider, no key, HTTP error, transport error, non-JSON response, empty choices, key_resolver() raising, extract() raising
  • Success path with mocked urlopen — confirms model + system + auth header construction
  • Key resolution precedence (file path > legacy ~/grok.key > env var)
  • Permission gating (rejects 0o644 and 0o640, accepts 0o600)
  • HTTPS scheme guard fires before urlopen() is reached
  • Payload-failure exit codes use 502 sentinel, not the HTTP status code

TestCliStatus tests in tests/test_mcp/test_server.py updated to mock the API-provider surface so they stay focused on CLI behavior; added a new test_includes_api_providers asserting cross-surface integration.

Full suite: 618 pass + 1 pre-existing failure (test_codex_basic, env-sensitive, unrelated to this change).

Review history

This change went through Codex strict-technical review, an open-ended adversarial Grok review (via the new tool itself, dogfooding the round-trip), and a final Codex pass. Each round caught real bugs. Of note: the success-path exit_code was the one place Grok's review pointed at the right gap but suggested an incomplete fix (HTTP status code as exit code); Codex's second pass caught that the success path needed to return 0 per the existing CLI-executor convention. The disagreement is the reason this PR exists in its current shape.

Provider boundary

API keys are read at request time (not module load), so rotating ~/.llm-relay/grok.key requires no daemon restart. The PR does not introduce any always-on outbound calls — the new path is invoked only when a caller explicitly delegates to it.

Test plan

  • pytest tests/test_orch/test_api_executor.py — 24 pass
  • pytest tests/test_mcp/test_server.py — all green
  • pytest full suite — 618 pass (the one pre-existing failure is unrelated and reproduces on main without this PR)
  • End-to-end: with a valid xAI key at ~/.llm-relay/grok.key (mode 0600), restart daemon, call api_delegate(provider="grok", prompt="reply WORKING") via MCP — should round-trip in ~1-2s with success=true, cli_id="xai-grok", exit_code=0.

— AI Team Lead

cli_delegate covers providers with a local CLI binary (claude, codex,
gemini). Some useful providers — xAI Grok being the immediate case —
only expose an HTTP API; wrapping them as fake CLIs is more friction
than value. This change adds a sibling MCP tool, api_delegate, that
treats those providers natively.

New module orch/api_executor.py:
- stdlib-only HTTP delegation via urllib + ssl.create_default_context
- PROVIDERS dict keyed by short name; each entry declares endpoint,
  default model, key resolver, response-extraction function
- DelegationResult shape identical to executor.execute_cli so callers
  (MCP tool layer, DB logger, history capture) treat both paths
  uniformly
- xAI/Grok key resolution: XAI_API_KEY_PATH (file) → ~/.llm-relay/grok.key
  → ~/grok.key (legacy) → XAI_API_KEY (env var)
- Comprehensive error handling: unknown provider, no key, HTTP error,
  transport error, non-JSON response, empty choices — each returns a
  populated DelegationResult with explanatory error field

New MCP tool api_delegate:
- Signature: provider, prompt, model="", system="", timeout=120, max_tokens=4000
- Logs to delegation DB with strategy="api-direct"
- Respects LLM_RELAY_HISTORY=1 for session-history capture
- Returns same JSON envelope as cli_delegate

cli_status and cli_probe extended to surface API providers alongside
CLI providers. Each row now carries a "kind" field ("cli-binary" vs
"http-api") so callers can distinguish.

Tests: 16 new in tests/test_orch/test_api_executor.py covering provider
listing, status, all error paths, success path with mocked urlopen,
header construction, model/system pass-through, and key resolution
precedence. Existing TestCliStatus tests updated to mock the API-
provider surface so they stay focused on CLI behavior; added a new
test_includes_api_providers asserting the cross-surface integration.

Full suite: 610 pass + 1 pre-existing failure (test_codex_basic, env-
sensitive, unrelated to this change).
Round-trip review found three real bugs and three more on the second
pass. All addressed:

Grok review (3 fixes):
- HTTPS scheme guard: refuse non-HTTPS endpoints before sending the
  bearer token. The PROVIDERS dict is in-tree so this is defense
  against future misedits, not external input. Guard fires before
  urlopen() is reached.
- Key file permission gating: _read_key_file_if_safe() requires POSIX
  mode 0600 or stricter (no group / no other access). Loose-permission
  files are silently skipped with a debug log; resolver falls through
  to env var. Windows is exempt (st_mode bits don't carry equivalent
  meaning).
- Payload-failure exit codes: JSON-parse failures and empty-extract
  failures previously returned exit_code=status (200) on a 2xx HTTP
  response, looking like success to int-checking callers. Now both
  use PAYLOAD_FAILURE_EXIT (502 "bad gateway") sentinel.

Codex review (3 fixes):
- exit_code=0 on success: Grok's payload-failure fix tightened error
  paths but left the success path returning exit_code=status (200).
  proxy/composition.py:814 explicitly checks `exit_code == 0`; the
  CLI sister module executor.py returns proc.returncode (0 on clean
  exit). api_executor now matches the convention.
- Wrap key_resolver() in try/except: provider-supplied callable that
  raises previously bubbled. Now returns a clean DelegationResult.
- Wrap extract() in try/except: same fix for the response-extraction
  hook.
- Remove unused imports: os, tempfile, pytest, top-level PROVIDERS
  (each test that needs PROVIDERS imports it locally).

Plus: capture model_used into a local variable before the request so
exception handlers don't depend on body[] still being well-formed.

Tests: 16 → 24 (+ HTTPS-guard, perm gating ×3, payload-failure exit
×2, key_resolver-raises, extract-raises). Full suite 616 → 618 pass.

Live ~/grok.key chmod'd to 0600 (was 0664).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant