diff --git a/docs/source/reference/cli.md b/docs/source/reference/cli.md index a6a968ab11..c6aae05995 100644 --- a/docs/source/reference/cli.md +++ b/docs/source/reference/cli.md @@ -105,6 +105,35 @@ openenv skills add --dest /path/to/my-agent/skills/ [[autodoc]] openenv.cli.commands.skills.skills_preview +## `openenv collect` + +Collect rollouts from a running environment with a teacher model and write +them as an SFT-ready `results.jsonl`. The teacher can be a hosted provider +(`--provider openai|anthropic`) or any self-hosted OpenAI-compatible server +such as vLLM, TGI or Ollama via `--llm-endpoint`. + +```bash +# Scripted teacher, no API key needed +openenv collect openspiel:tic_tac_toe --base-url http://localhost:8001 \ + --output-dir ./rollouts -n 10 --provider scripted + +# Self-hosted model (vLLM serving on port 8000) +openenv collect reasoning_gym:chain_sum --base-url http://localhost:8001 \ + --output-dir ./rollouts -n 50 \ + --llm-endpoint http://localhost:8000 --model Qwen/Qwen3-1.7B +``` + +`--llm-endpoint` takes a full base URL. `/v1` is appended when the URL has no +path, so `http://localhost:8000` and `http://localhost:8000/v1` are equivalent; +a URL with a path (for example a gateway prefix) is used as-is. `--llm-port` is +only needed when the URL does not include a port; it has no default, so +`--llm-endpoint http://localhost` means port 80 (earlier releases assumed 8000). +The resolved endpoint is printed when the run starts. Only `http(s)` URLs are +accepted, and credentials, query strings and fragments in the URL are rejected: +pass the key through `OPENAI_API_KEY` instead. + +[[autodoc]] openenv.cli.commands.collect.collect + # API Reference ## Entry point diff --git a/docs/source/tutorials/sft-warmup.md b/docs/source/tutorials/sft-warmup.md index 1683fc93c5..279837bb23 100644 --- a/docs/source/tutorials/sft-warmup.md +++ b/docs/source/tutorials/sft-warmup.md @@ -146,6 +146,21 @@ hub_repo_arg = shlex.quote(f"{YOUR_HF_USERNAME}/chain-sum-rollouts") --output-dir ./rollouts ``` +To use a self-hosted teacher instead of a hosted provider, point `--llm-endpoint` at any +OpenAI-compatible server (vLLM, TGI, Ollama) and pass the model id it serves: + +```bash +openenv collect reasoning_gym:chain_sum \ + --base-url https://sergiopaniego-reasoning-gym.hf.space \ + --llm-endpoint http://localhost:8000 \ + --model Qwen/Qwen3-1.7B \ + --num-episodes 300 \ + --output-dir ./rollouts +``` + +The teacher drives the environment through tool calls, so the server must have tool calling +enabled (for vLLM: `--enable-auto-tool-choice --tool-call-parser `). + The command prints a live progress summary and pushes the collected episodes to the Hub as `{YOUR_HF_USERNAME}/chain-sum-rollouts`. Pull them back to start filtering: diff --git a/examples/ttt_collect_with_llm.py b/examples/ttt_collect_with_llm.py index cf24bb1ff7..a0138953ec 100644 --- a/examples/ttt_collect_with_llm.py +++ b/examples/ttt_collect_with_llm.py @@ -21,7 +21,7 @@ # Local vLLM (OpenAI-compatible) with Qwen: python examples/ttt_collect_with_llm.py \\ --base-url http://localhost:8000 \\ - --llm-endpoint http://localhost --llm-port 8001 \\ + --llm-endpoint http://localhost:8001 \\ --model Qwen/Qwen2.5-7B-Instruct \\ --num-episodes 20 @@ -136,9 +136,20 @@ def parse_args() -> argparse.Namespace: parser.add_argument( "--llm-endpoint", default=None, - help="OpenAI-compatible endpoint (e.g. http://localhost for vLLM).", + help=( + "Base URL of an OpenAI-compatible server (e.g. http://localhost:8001 " + "for vLLM). /v1 is appended when the URL has no path." + ), + ) + parser.add_argument( + "--llm-port", + type=int, + default=None, + help=( + "Port appended to --llm-endpoint when the URL does not include one. " + "No default: earlier versions assumed 8000." + ), ) - parser.add_argument("--llm-port", type=int, default=8000) parser.add_argument("--temperature", type=float, default=0.2) parser.add_argument("--max-tokens", type=int, default=200) @@ -156,7 +167,7 @@ def main() -> None: llm_client = build_llm_client(args) teacher_label = ( - f"{args.llm_endpoint}:{args.llm_port}/{args.model}" + f"{llm_client.base_url}/{args.model}" if args.llm_endpoint else f"{args.provider}/{args.model}" ) diff --git a/src/openenv/cli/commands/collect.py b/src/openenv/cli/commands/collect.py index a749ad27b0..36ec52a3be 100644 --- a/src/openenv/cli/commands/collect.py +++ b/src/openenv/cli/commands/collect.py @@ -141,7 +141,7 @@ def _build_llm_model_step( model: str, *, llm_endpoint: str | None, - llm_port: int, + llm_port: int | None, temperature: float, max_tokens: int, system_prompt: str | None = None, @@ -152,15 +152,19 @@ def _build_llm_model_step( # Self-hosted OpenAI-compatible endpoint (vLLM, TGI, Ollama, ...). from openenv.core.llm_client import OpenAIClient - client: LLMClient = OpenAIClient( - endpoint=llm_endpoint, - port=llm_port, - model=model, - api_key=os.getenv("OPENAI_API_KEY") or "not-needed", - system_prompt=effective_system_prompt, - temperature=temperature, - max_tokens=max_tokens, - ) + try: + client: LLMClient = OpenAIClient( + endpoint=llm_endpoint, + port=llm_port, + model=model, + api_key=os.getenv("OPENAI_API_KEY") or "not-needed", + system_prompt=effective_system_prompt, + temperature=temperature, + max_tokens=max_tokens, + ) + except ValueError as exc: + raise typer.BadParameter(str(exc), param_hint="--llm-endpoint") from exc + console.print(f"[cyan]LLM endpoint:[/cyan] {client.base_url}") else: client = create_llm_client( provider=provider, @@ -280,13 +284,23 @@ def collect( str | None, typer.Option( "--llm-endpoint", - help="OpenAI-compatible endpoint URL (for self-hosted vLLM/TGI/Ollama).", + help=( + "Base URL of a self-hosted OpenAI-compatible server " + "(vLLM/TGI/Ollama), e.g. http://localhost:8000. /v1 is appended " + "when the URL has no path; a URL with a path is used as-is." + ), ), ] = None, llm_port: Annotated[ - int, - typer.Option("--llm-port", help="Port for self-hosted LLM endpoint."), - ] = 8000, + int | None, + typer.Option( + "--llm-port", + help=( + "Port appended to --llm-endpoint when the URL does not include one. " + "No default: earlier releases assumed 8000." + ), + ), + ] = None, temperature: Annotated[ float, typer.Option("--temperature", help="Sampling temperature.") ] = 0.2, @@ -364,6 +378,20 @@ def collect( factory = _build_session_factory( env, base_url, dataset_config=parsed_dataset_config ) + + if uses_llm_teacher: + model_step = _build_llm_model_step( + provider=provider, + model=model, # type: ignore[arg-type] # validated above + llm_endpoint=llm_endpoint, + llm_port=llm_port, + temperature=temperature, + max_tokens=max_tokens, + system_prompt=system_prompt, + ) + else: + model_step = _build_scripted_model_step() + serializer = RolloutSerializer(output_dir) serializer.write_metadata( { @@ -379,19 +407,6 @@ def collect( } ) - if uses_llm_teacher: - model_step = _build_llm_model_step( - provider=provider, - model=model, # type: ignore[arg-type] # validated above - llm_endpoint=llm_endpoint, - llm_port=llm_port, - temperature=temperature, - max_tokens=max_tokens, - system_prompt=system_prompt, - ) - else: - model_step = _build_scripted_model_step() - should_keep = None if keep_losses else (lambda record: record.reward >= 0.0) collect_runner = CollectRunner( diff --git a/src/openenv/core/harness/README.md b/src/openenv/core/harness/README.md index d45199e81a..1acdbd50a7 100644 --- a/src/openenv/core/harness/README.md +++ b/src/openenv/core/harness/README.md @@ -28,6 +28,12 @@ OPENAI_API_KEY=... openenv collect openspiel:tic_tac_toe \ --output-dir /tmp/ttt-sft-v1 \ -n 200 --provider openai --model gpt-5-mini \ --push-to-hub /ttt-sft-v1 + +# Self-hosted teacher (vLLM/TGI/Ollama) via any OpenAI-compatible base URL +openenv collect openspiel:tic_tac_toe \ + --base-url https://-.hf.space \ + --output-dir /tmp/ttt-sft-local \ + -n 200 --llm-endpoint http://localhost:8000 --model Qwen/Qwen3-1.7B ``` Programmatic use: diff --git a/src/openenv/core/llm_client.py b/src/openenv/core/llm_client.py index c37ad1bccc..783a06ed57 100644 --- a/src/openenv/core/llm_client.py +++ b/src/openenv/core/llm_client.py @@ -13,6 +13,9 @@ client = OpenAIClient("http://localhost", 8000, model="meta-llama/...") response = await client.complete("What is 2+2?") + # The endpoint may carry its own port and path prefix: + client = OpenAIClient("http://localhost:8000/v1", port=None, model="meta-llama/...") + # Or use the factory for hosted APIs: client = create_llm_client("openai", model="gpt-4", api_key="sk-...") response = await client.complete_with_tools(messages, tools) @@ -25,9 +28,79 @@ from abc import ABC, abstractmethod from dataclasses import dataclass, field from typing import Any +from urllib.parse import urlsplit, urlunsplit from openai import AsyncOpenAI +_OPENAI_API_PREFIX = "/v1" +_MIN_PORT, _MAX_PORT = 1, 65535 + + +def _redact_userinfo(endpoint: str) -> str: + """Drop `user:password@` from a URL so it is safe to echo in an error.""" + scheme, sep, rest = endpoint.partition("://") + authority, slash, tail = rest.partition("/") + if not sep or "@" not in authority: + return endpoint + return f"{scheme}{sep}***@{authority.rsplit('@', 1)[-1]}{slash}{tail}" + + +def _join_endpoint_port(endpoint: str, port: int | None) -> str: + """Validate an endpoint URL and combine it with an optional port. + + Only `http`/`https` URLs with a host are accepted. Credentials, query + strings and fragments are rejected: the endpoint is logged and persisted in + rollout metadata, and the SDKs build request URLs by appending to the path, + which corrupts a query string. `port` is appended only when the URL does + not name one; an explicit port that differs from the one in the URL is + rejected rather than silently overridden. A trailing slash on the path is + dropped. Invalid endpoints raise `ValueError`. + """ + safe = _redact_userinfo(endpoint) + try: + parts = urlsplit(endpoint) + if parts.scheme not in ("http", "https"): + raise ValueError("expected an http:// or https:// URL") + if not parts.hostname: + raise ValueError("missing host") + if parts.username is not None or parts.password is not None: + raise ValueError("credentials in the URL are not supported, use api_key") + if parts.query or parts.fragment: + raise ValueError("query strings and fragments are not supported") + if parts.netloc.endswith(":"): + raise ValueError("empty port") + url_port = parts.port + for candidate in (url_port, port): + if candidate is not None and not _MIN_PORT <= candidate <= _MAX_PORT: + raise ValueError( + f"port {candidate} is out of range {_MIN_PORT}-{_MAX_PORT}" + ) + except ValueError as exc: + raise ValueError(f"Invalid endpoint URL {safe!r}: {exc}") from exc + + if url_port is None: + if port is not None: + parts = parts._replace(netloc=f"{parts.netloc}:{port}") + elif port is not None and port != url_port: + raise ValueError( + f"Endpoint URL {safe!r} already specifies port {url_port}, " + f"which conflicts with port={port}" + ) + return urlunsplit(parts._replace(path=parts.path.rstrip("/"))) + + +def _openai_base_url(base_url: str) -> str: + """Append the OpenAI `/v1` prefix when `base_url` has no path. + + A URL that already names a path (`/v1`, or a gateway prefix such as + `/openai/v1`) is used as-is, matching the OpenAI SDK convention that + `base_url` includes the API prefix. + """ + parts = urlsplit(base_url) + if parts.path in ("", "/"): + return urlunsplit(parts._replace(path=_OPENAI_API_PREFIX)) + return base_url + @dataclass class ToolCall: @@ -70,12 +143,15 @@ class LLMClient(ABC): Args: endpoint (`str`): - The base URL of the LLM service (e.g. "http://localhost"). - port (`int`): - The port the service listens on. + The `http(s)` base URL of the LLM service (e.g. "http://localhost"). + May include a port and a path (e.g. "http://localhost:8000/v1"). + Credentials, query strings and fragments are rejected. + port (`int` or `None`): + The port the service listens on. Appended to `endpoint` when the + URL does not name one; must match the URL's port when both are given. """ - def __init__(self, endpoint: str, port: int): + def __init__(self, endpoint: str, port: int | None): self.endpoint = endpoint self.port = port @@ -122,8 +198,8 @@ async def complete_with_tools( @property def base_url(self) -> str: - """Construct base URL from endpoint and port.""" - return f"{self.endpoint}:{self.port}" + """Base URL of the service: `endpoint` plus `port` when the URL names none.""" + return _join_endpoint_port(self.endpoint, self.port) class OpenAIClient(LLMClient): @@ -134,9 +210,12 @@ class OpenAIClient(LLMClient): Args: endpoint (`str`): - The base URL (e.g. "http://localhost"). - port (`int`): - The port number. + The base URL (e.g. "http://localhost"). May include a port and a + path (e.g. "http://localhost:8000/v1"). The `/v1` API prefix is + appended when the URL has no path; a URL with a path is used as-is. + port (`int` or `None`): + The port number, appended when `endpoint` does not name one; must + match the URL's port when both are given. model (`str`): Model name to pass to the API. api_key (`str`, *optional*): @@ -155,7 +234,7 @@ class OpenAIClient(LLMClient): def __init__( self, endpoint: str, - port: int, + port: int | None, model: str, api_key: str | None = None, system_prompt: str | None = None, @@ -174,7 +253,7 @@ def __init__( self._omit_temperature = use_max_completion_tokens self._client = AsyncOpenAI( - base_url=f"{self.base_url}/v1", + base_url=_openai_base_url(self.base_url), api_key=api_key if api_key is not None else "not-needed", ) @@ -246,9 +325,10 @@ class AnthropicClient(LLMClient): Args: endpoint (`str`): - The base URL (e.g. `https://api.anthropic.com`). - port (`int`): - The port number. + The base URL (e.g. `https://api.anthropic.com`). May include a port. + port (`int` or `None`): + The port number, appended when `endpoint` does not name one; must + match the URL's port when both are given. model (`str`): Model name (e.g. "claude-sonnet-4-20250514"). api_key (`str`, *optional*): @@ -264,7 +344,7 @@ class AnthropicClient(LLMClient): def __init__( self, endpoint: str, - port: int, + port: int | None, model: str, api_key: str | None = None, system_prompt: str | None = None, diff --git a/tests/core/test_llm_client.py b/tests/core/test_llm_client.py index 8b8e556ff6..6d48cc5121 100644 --- a/tests/core/test_llm_client.py +++ b/tests/core/test_llm_client.py @@ -5,7 +5,9 @@ import json from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest +from openai import AsyncOpenAI from openenv.core.llm_client import ( _clean_mcp_schema, _mcp_tools_to_anthropic, @@ -59,6 +61,91 @@ async def complete(self, prompt: str, **kwargs) -> str: client = StubClient("https://api.example.com", 443) assert client.base_url == "https://api.example.com:443" + @pytest.mark.parametrize( + ("endpoint", "port", "expected"), + [ + ("http://localhost:8000", None, "http://localhost:8000"), + ("http://localhost:8000", 8000, "http://localhost:8000"), + ("http://localhost:8000/", 8000, "http://localhost:8000"), + ("http://proxy/litellm", 4000, "http://proxy:4000/litellm"), + ("http://[::1]", 8000, "http://[::1]:8000"), + ( + "https://gw.example.com/openai/v1", + None, + "https://gw.example.com/openai/v1", + ), + ("http://localhost", None, "http://localhost"), + ], + ) + def test_base_url_endpoint_forms(self, endpoint, port, expected): + """Port is appended only when the URL names none; a path survives.""" + + class StubClient(LLMClient): + async def complete(self, prompt: str, **kwargs) -> str: + return "stub" + + assert StubClient(endpoint, port).base_url == expected + + @pytest.mark.parametrize( + "endpoint", + [ + "http://localhost:8000:8000", + "http://localhost:", + "localhost:8000", + "http://[::1", + "ftp://localhost:8000", + "http:///v1", + "http://user:token@localhost:8000", + "http://localhost:8000/v1?api-version=1", + "http://localhost:8000#frag", + "http://localhost:0", + "http://localhost:99999", + ], + ) + def test_base_url_malformed_endpoint_raises(self, endpoint): + """Invalid endpoints fail with a clear error instead of a bad URL.""" + + class StubClient(LLMClient): + async def complete(self, prompt: str, **kwargs) -> str: + return "stub" + + with pytest.raises(ValueError, match="Invalid endpoint URL"): + StubClient(endpoint, 8000).base_url + + @pytest.mark.parametrize("port", [0, -1, 65536, 99999]) + def test_base_url_port_out_of_range_raises(self, port): + """The appended port is range-checked like a port inside the URL.""" + + class StubClient(LLMClient): + async def complete(self, prompt: str, **kwargs) -> str: + return "stub" + + with pytest.raises(ValueError, match="out of range 1-65535"): + StubClient("http://localhost", port).base_url + + def test_base_url_error_does_not_echo_credentials(self): + """A credential-bearing endpoint is rejected without leaking the secret.""" + + class StubClient(LLMClient): + async def complete(self, prompt: str, **kwargs) -> str: + return "stub" + + with pytest.raises(ValueError, match="credentials") as excinfo: + StubClient("http://user:s3cret@localhost:8000/v1", None).base_url + + assert "s3cret" not in str(excinfo.value) + assert "***@localhost:8000/v1" in str(excinfo.value) + + def test_base_url_conflicting_port_raises(self): + """An explicit port that differs from the URL's port is rejected.""" + + class StubClient(LLMClient): + async def complete(self, prompt: str, **kwargs) -> str: + return "stub" + + with pytest.raises(ValueError, match="conflicts with port=8000"): + StubClient("http://localhost:11434", 8000).base_url + @pytest.mark.asyncio async def test_complete_with_tools_not_implemented(self): """Default complete_with_tools raises NotImplementedError.""" @@ -136,6 +223,109 @@ def test_custom_temperature_and_max_tokens(self, mock_openai_cls): assert client.temperature == 0.7 assert client.max_tokens == 512 + @pytest.mark.parametrize( + ("endpoint", "port", "expected"), + [ + ("http://localhost:8000", None, "http://localhost:8000/v1"), + ("http://localhost:8000", 8000, "http://localhost:8000/v1"), + ("http://localhost:8000/", None, "http://localhost:8000/v1"), + ("http://localhost:8000/v1", None, "http://localhost:8000/v1"), + ("http://localhost:8000/v1/", None, "http://localhost:8000/v1"), + ("http://localhost", 8001, "http://localhost:8001/v1"), + ("http://proxy:4000/litellm", None, "http://proxy:4000/litellm"), + ( + "https://api.groq.com/openai/v1", + None, + "https://api.groq.com/openai/v1", + ), + ( + "https://generativelanguage.googleapis.com/v1beta/openai", + None, + "https://generativelanguage.googleapis.com/v1beta/openai", + ), + ], + ) + def test_endpoint_url_forms(self, endpoint, port, expected): + """/v1 is appended only when the URL has no path; a path is used as-is.""" + with patch("openenv.core.llm_client.AsyncOpenAI") as mock_openai_cls: + OpenAIClient(endpoint, port, model="gpt-4") + + mock_openai_cls.assert_called_once_with( + base_url=expected, + api_key="not-needed", + ) + + def test_endpoint_with_malformed_port_raises(self): + """The double-port form fails before any client is created.""" + with pytest.raises(ValueError, match="Invalid endpoint URL"): + OpenAIClient("http://localhost:8000:8000", 8000, model="gpt-4") + + def test_endpoint_with_conflicting_port_raises(self): + """A port argument that contradicts the URL's port is rejected.""" + with pytest.raises(ValueError, match="conflicts with port=8000"): + OpenAIClient("http://localhost:11434", 8000, model="gpt-4") + + +class TestOpenAIClientRequestUrl: + """The URL the SDK actually requests, not only what is handed to it.""" + + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("endpoint", "port", "expected"), + [ + ( + "http://localhost:8000", + None, + "http://localhost:8000/v1/chat/completions", + ), + ( + "http://localhost:8000/v1", + None, + "http://localhost:8000/v1/chat/completions", + ), + ("http://localhost", 8001, "http://localhost:8001/v1/chat/completions"), + ( + "https://gw.example.com/openai/v1", + None, + "https://gw.example.com/openai/v1/chat/completions", + ), + ], + ) + async def test_chat_completion_request_url(self, endpoint, port, expected): + """complete() reaches /chat/completions through the real SDK.""" + requested: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + requested.append(str(request.url)) + return httpx.Response( + 200, + json={ + "id": "cmpl-1", + "object": "chat.completion", + "created": 0, + "model": "m", + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": "ok"}, + "finish_reason": "stop", + } + ], + }, + ) + + def build_sdk_client(**kwargs): + transport = httpx.MockTransport(handler) + return AsyncOpenAI( + http_client=httpx.AsyncClient(transport=transport), **kwargs + ) + + with patch("openenv.core.llm_client.AsyncOpenAI", side_effect=build_sdk_client): + client = OpenAIClient(endpoint, port, model="m") + + assert await client.complete("hi") == "ok" + assert requested == [expected] + class TestOpenAIClientComplete: """Test the complete() method.""" diff --git a/tests/test_cli/test_collect.py b/tests/test_cli/test_collect.py index a2b11c20ef..8c6da25ee6 100644 --- a/tests/test_cli/test_collect.py +++ b/tests/test_cli/test_collect.py @@ -4,6 +4,7 @@ from __future__ import annotations +import re from pathlib import Path from unittest.mock import MagicMock, patch @@ -14,6 +15,8 @@ runner = CliRunner() +_ANSI_ESCAPE = re.compile(r"\x1b\[[0-9;]*m") + @pytest.fixture def mock_pipeline(): @@ -289,3 +292,123 @@ def test_default_filters_losing_rollouts(tmp_path: Path, mock_pipeline): losing = MagicMock(reward=-1.0) assert should_keep(winning) is True assert should_keep(losing) is False + + +@pytest.mark.parametrize( + ("llm_args", "expected_base_url"), + [ + (["--llm-endpoint", "http://localhost:8000"], "http://localhost:8000/v1"), + (["--llm-endpoint", "http://localhost:8000/v1"], "http://localhost:8000/v1"), + ( + ["--llm-endpoint", "http://localhost", "--llm-port", "8001"], + "http://localhost:8001/v1", + ), + (["--llm-endpoint", "http://localhost:11434"], "http://localhost:11434/v1"), + (["--llm-endpoint", "http://localhost"], "http://localhost/v1"), + (["--llm-endpoint", "http://gw/openai/v1"], "http://gw/openai/v1"), + ], +) +def test_llm_endpoint_url_forms_reach_openai_client( + tmp_path: Path, mock_pipeline, llm_args, expected_base_url +): + with patch("openenv.core.llm_client.AsyncOpenAI") as openai_cls: + result = runner.invoke( + app, + [ + "collect", + "openspiel:tic_tac_toe", + "--base-url", + "https://example.hf.space", + "--output-dir", + str(tmp_path), + "--model", + "Qwen/Qwen3-1.7B", + *llm_args, + ], + ) + + assert result.exit_code == 0, result.output + openai_cls.assert_called_once() + assert openai_cls.call_args.kwargs["base_url"] == expected_base_url + + +@pytest.mark.parametrize( + ("llm_args", "expected_message"), + [ + ( + ["--llm-endpoint", "http://localhost:8000:8000"], + "Invalid endpoint URL", + ), + ( + ["--llm-endpoint", "http://localhost:11434", "--llm-port", "8000"], + "conflicts with port=8000", + ), + (["--llm-endpoint", "ftp://localhost:8000"], "expected an http"), + ( + ["--llm-endpoint", "http://localhost", "--llm-port", "99999"], + "out of range 1-65535", + ), + ( + ["--llm-endpoint", "http://localhost:8000/v1?api-version=1"], + "query strings and fragments are not supported", + ), + ( + ["--llm-endpoint", "http://user:s3cret@localhost:8000"], + "credentials in the URL are not supported", + ), + ], +) +def test_bad_llm_endpoint_is_usage_error_before_output_is_written( + tmp_path: Path, mock_pipeline, llm_args, expected_message +): + result = runner.invoke( + app, + [ + "collect", + "openspiel:tic_tac_toe", + "--base-url", + "https://example.hf.space", + "--output-dir", + str(tmp_path), + "--model", + "Qwen/Qwen3-1.7B", + *llm_args, + ], + ) + + # Typer renders usage errors in a wrapped box, coloured when GITHUB_ACTIONS + # or FORCE_COLOR is set; strip the colour codes and flatten before matching. + plain = _ANSI_ESCAPE.sub("", result.output) + output = " ".join(plain.replace("\u2502", " ").split()) + assert result.exit_code == 2, result.output + assert "--llm-endpoint" in output + assert expected_message in output + assert "s3cret" not in result.output + mock_pipeline["serializer_cls"].return_value.write_metadata.assert_not_called() + mock_pipeline["runner_instance"].run.assert_not_called() + + +def test_resolved_llm_endpoint_is_printed(tmp_path: Path, mock_pipeline): + with ( + patch("openenv.core.llm_client.AsyncOpenAI"), + patch("openenv.cli.commands.collect.console") as console, + ): + result = runner.invoke( + app, + [ + "collect", + "openspiel:tic_tac_toe", + "--base-url", + "https://example.hf.space", + "--output-dir", + str(tmp_path), + "--model", + "Qwen/Qwen3-1.7B", + "--llm-endpoint", + "http://localhost", + ], + ) + + assert result.exit_code == 0, result.output + printed = [str(call.args[0]) for call in console.print.call_args_list] + assert "[cyan]LLM endpoint:[/cyan] http://localhost" in printed