fix: LLM endpoint full base URLs (from #1189)

[cursor]

Summary

On-repo replacement of #1189 (surajsharan) so we can run repository CI and fold into the 0.6.0 cut per Ben’s ask.

--llm-endpoint / OpenAIClient now parse the endpoint as a URL:

• append /v1 when pathless; keep an existing path (gateway/proxy)
• append port only when the URL has none; reject conflicts
• reject non-http(s), missing host, credentials, query/fragment, and out-of-range ports

Ben APPROVED the fork tip (e421de47) including the documented removal of implicit port 8000.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation
• New environment
• Refactoring

Alignment Checklist

Before submitting, verify:

• I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
• I have checked .claude/docs/INVARIANTS.md and no invariants are violated
• Focused tests/lint run on changed files

RFC Status

• Not required (bug fix, docs, minor refactoring)
• RFC exists: #___
• RFC needed (will create before merge)

Test Plan

• PYTHONPATH=src:envs uv run pytest tests/core/test_llm_client.py tests/test_cli/test_collect.py -q — 105 passed
• ruff check / ruff format --check on changed Python — clean

Claude Code Review

N/A — release-automation on-repo replacement of Ben-approved #1189.

  

---

Note

Medium Risk
Changes how self-hosted LLM URLs are resolved and removes the implicit port-8000 default, which can break existing scripts; rollout collection depends on correct endpoint parsing but is heavily tested.

Overview
Self-hosted teacher URLs for openenv collect and OpenAIClient now treat --llm-endpoint as a full http(s) base URL instead of host + fixed port. /v1 is added only when the URL has no path; gateway paths (e.g. /openai/v1) are left unchanged. --llm-port is optional with no default (previously 8000), so http://localhost without a port uses 80.

Invalid endpoints fail early with clear CLI errors: wrong scheme, credentials in the URL, query/fragment, conflicting ports, and out-of-range ports. Secrets are redacted in error messages. openenv collect prints the resolved LLM endpoint and builds the teacher client before writing rollout metadata so bad URLs do not start a partial run.

Docs and examples (CLI reference, SFT warmup, harness README, ttt_collect_with_llm.py) are updated for vLLM/TGI/Ollama-style endpoints; tests cover URL resolution and request paths end-to-end.

^{Reviewed by Cursor Bugbot for commit 48581f5. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Alignment Review Report

Two-tier review of the --llm-endpoint / OpenAIClient full-base-URL change (diff c32c317...48581f5).

Automated Checks

• Lint: PASS (changed files) — usort check, ruff format --check, and ruff check are all clean on this PR's src/ and tests/ files. The repo-wide hook reports only pre-existing failures (envs/*, and the two known tests/envs/test_grid_world.py / test_julia_env.py usort files), none touched here. examples/ttt_collect_with_llm.py has a pre-existing format/import-sort nit at ~line 217 that is outside this PR's hunks and outside the hook's src/ tests/ envs/ scope. CI lint job: pass.
• Debug code: CLEAN — no bare print(), breakpoint(), pdb/ipdb, or new TODO/FIXME in the changed files. The one added console.print(...) is the established Rich logging pattern used throughout collect.py (a print(-substring false positive from the hook).

Open RFCs Context

All RFCs are currently In Review or Draft (none Accepted/Implemented). Two are thematically adjacent:

• RFC 005 – Agentic Harness Integration (In Review, @Darktex): collect drives the MCP harness and RFC 005 lists an "LLM Endpoint (vLLM/Claude API)" component. This PR does not touch the harness architecture or /harness endpoint → relevant context, no conflict.
• RFC 012 – Harbor Capture Providers (In Review, @adithya-s-k): formalizes an upstream provider descriptor (openai|anthropic|hf|vllm), endpoint reachability, and credential-scoped client caching. See RFC Conflicts below.

Tier 1: Fixes Required

None. The change is well-contained and covered by ~313 lines of new tests (URL forms, malformed / credential / conflicting-port cases, and an end-to-end SDK request-path test via httpx.MockTransport). CI is green across lint, test (3.11), test (3.12), package smoke-test, and Docker runtime validation. Verified statically:

• typer.BadParameter and console are already imported/available in collect.py.
• Moving the model_step build ahead of serializer.write_metadata(...) is safe (all referenced vars are defined at the new position) and correctly makes a bad endpoint fail before any output is written.
• Hosted providers are unaffected: _HOSTED_PROVIDERS still passes :443, so create_llm_client("openai"/"anthropic") resolves to the same base URLs as before.

Tier 2: Alignment Discussion

Principle Conflicts

None identified. Core llm_client.py imports no server/ code; rewards/Gymnasium API are untouched; scope is teacher-LLM configuration only.

Positive alignment worth calling out:

• Upholds INVARIANT “No credential exposure” — _redact_userinfo() strips user:password@ before any endpoint is echoed in an error, and credentials embedded in --llm-endpoint are rejected outright with a pointer to OPENAI_API_KEY. A dedicated test asserts the secret never leaks into the error string. This directly reinforces INVARIANTS.md §Security/3 (@Darktex).

RFC Conflicts

ALIGNMENT FLAG: Endpoint/provider handling overlaps RFC 012's upstream descriptor

• Principle/RFC at stake: RFC 012 – Harbor Capture Providers (In Review)
• The concern: This PR adds ad-hoc endpoint URL parsing/validation (_join_endpoint_port, _openai_base_url) plus a lightweight provider notion in the local collect path. RFC 012 proposes a formal upstream descriptor with provider ∈ {openai, anthropic, hf, vllm}, endpoint-reachability rules, and credential-identity-scoped client caching for the Harbor capture server. These are different subsystems (local rollout collection vs. Harbor capture), so this is not a direct conflict — but the team may want the two endpoint/provider models to converge rather than diverge (e.g. a future vllm provider, shared URL validation).
• Suggested reviewer: @adithya-s-k (RFC 012 author); @Darktex (RFC 005 / security invariant)

Behavior-Change Note (for the 0.6.0 changelog)

• --llm-port loses its implicit 8000 default and --llm-endpoint is now a full base URL, so --llm-endpoint http://localhost resolves to port 80, not 8000. This is a CLI breaking change, documented in help text, docs/, and the harness README. Per INVARIANTS “Breaking Change Policy” (pre-1.0, documented) this is acceptable; the PR body notes it was explicitly approved and folded into the 0.6.0 cut. Flagging only so it lands in the release notes.

Summary

• 0 mechanical issues to fix (Tier 1 clean; CI green)
• 1 alignment point for human review (RFC 012 convergence) + 1 release-note behavior change
• 1 RFC area to discuss (RFC 012); RFC 005 relevant as context only

Note: this PR is already merged; posting as an informational review.

  

_{Sent by Cursor Automation: Pre-review}