From 74ab3df148c119b82010eebb38ce9552ec60fe9a Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 08:33:40 +0100 Subject: [PATCH 1/4] fix(forge): --agent reports a contract written to a --target-dir outside the cwd The receipt and the forge.contract_written event come from a before/after snapshot that scanned only the current directory, two levels deep. A --target-dir outside it, or deeper, got neither: an AI-mode run wrote the contract and the JSONL stream never said so. The stated target is now snapshotted too, with ~ expanded as the writer expands it, and a file outside the current directory is reported by its absolute path. Found by the forge_docs review of #850. --- CHANGELOG.md | 5 ++ fluid_build/cli/forge.py | 31 +++++++++--- tests/cli/test_forge_agent_mode.py | 81 ++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1272622..dd07aaf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`fluid doctor` reports an installed coding agent as ready.** With `FLUID_LLM_PROVIDER` set to `claude-code`, `codex`, `cursor` or `kiro`, readiness said "No API key found"; it now checks that the agent's CLI is on PATH, and says so when it is not. Each CLI keeps its own sign-in. +- **`fluid forge --agent` reports a contract written to a `--target-dir` outside the current + directory.** The receipt and the `forge.contract_written` event came from a scan of the current + directory, two levels deep, so a target outside it, or deeper, got neither: the run wrote the + contract and the stream never said so. The stated target is scanned too, and a file outside the + current directory is reported by its absolute path. ### Changed diff --git a/fluid_build/cli/forge.py b/fluid_build/cli/forge.py index c2ab4624..fd30309b 100644 --- a/fluid_build/cli/forge.py +++ b/fluid_build/cli/forge.py @@ -28,7 +28,7 @@ from fluid_build.cli.artifact_envelope import dump_json_with_envelope from fluid_build.cli.artifact_paths import product_forge_receipt_path from fluid_build.cli.artifact_receipts import ReceiptBuilder -from fluid_build.cli.artifact_scan import diff_snapshots, snapshot_workspace +from fluid_build.cli.artifact_scan import ArtifactSnapshot, diff_snapshots, snapshot_workspace from fluid_build.cli.console import cprint, cprint_literal from fluid_build.cli.console import error as console_error from fluid_build.cli.forge_banner import print_v2_banner @@ -1657,12 +1657,12 @@ def _run_main(args, logger: logging.Logger) -> int: return _run_watch(args, logger) # --- Snapshot for the forge receipt --- - # Scan cwd before any mode runs so the diff catches every file the - # mode handler wrote. find_workspace_root is cheap and localised; - # if this isn't a workspace yet, we fall back to cwd and still - # produce a receipt. + # Scan cwd (and an explicit --target-dir) before any mode runs so the + # diff catches every file the mode handler wrote. find_workspace_root + # is cheap and localised; if this isn't a workspace yet, we fall back + # to cwd and still produce a receipt. scan_root = Path.cwd() - before_snapshot = snapshot_workspace(scan_root) + before_snapshot = _forge_snapshot(scan_root, args) import fluid_build.cli.forge as _self _self.reset_token_usage() @@ -2049,6 +2049,23 @@ def load_context( _AGENT_EVENT_ACTIONS = {"create": "created", "update": "modified"} +def _forge_snapshot(scan_root: Path, args: Any) -> ArtifactSnapshot: + """Snapshot *scan_root*, plus the explicit ``--target-dir`` when one is given. + + The cwd scan reaches contracts two directory levels down. A target outside + cwd, or deeper than that, needs its own scan, or the receipt and the + ``--agent`` ``forge.contract_written`` event miss the contract written + there. A file under both roots is keyed by its resolved path, so it is + counted once; one outside cwd is reported by its absolute path. + """ + snapshot = snapshot_workspace(scan_root) + target_dir = get_cli_arg(args, "target_dir", None) + if isinstance(target_dir, (str, os.PathLike)) and str(target_dir): + # ``~`` as the writer reads it (``forge_modes`` expands the target too). + snapshot.files.update(snapshot_workspace(Path(target_dir).expanduser()).files) + return snapshot + + def _write_forge_receipt( *, flow: str, @@ -2070,7 +2087,7 @@ def _write_forge_receipt( recently modified one. """ try: - after_snapshot = snapshot_workspace(scan_root) + after_snapshot = _forge_snapshot(scan_root, args) entries = diff_snapshots(before_snapshot, after_snapshot) # Ignore no-op rows — the receipt only records what changed. changed = [e for e in entries if e.action != "unchanged"] diff --git a/tests/cli/test_forge_agent_mode.py b/tests/cli/test_forge_agent_mode.py index 026b1d92..0a5c8c58 100644 --- a/tests/cli/test_forge_agent_mode.py +++ b/tests/cli/test_forge_agent_mode.py @@ -454,3 +454,84 @@ def _fake_ai(args, logger): assert written, [e["event"] for e in events] assert written[0]["action"] == "created" assert written[0]["path"].endswith("contract.fluid.yaml") + + +def _run_ai_writing_to(product: Path, args, capsys, monkeypatch, *, contract=None): + """Run forge.run under --agent with an AI mode that writes *product*'s contract.""" + from fluid_build.cli import forge as forge_mod + + def _fake_ai(args, logger): + product.mkdir(parents=True, exist_ok=True) + (product / "contract.fluid.yaml").write_text(contract or "fluidVersion: 0.7.6\n") + return 0 + + monkeypatch.setattr(forge_mod, "run_ai_copilot_mode", _fake_ai) + monkeypatch.setattr( + "builtins.input", lambda *a, **k: pytest.fail("input() called under --agent") + ) + forge_mod.run(args, logging.getLogger("test")) + events = _parse_jsonl_events(capsys.readouterr().out) + return [e for e in events if e["event"] == "forge.contract_written"] + + +def test_agent_ai_mode_sees_a_target_dir_outside_cwd(tmp_path, capsys, monkeypatch): + """A ``--target-dir`` outside cwd is scanned too: the event and receipt name it.""" + work = tmp_path / "work" + work.mkdir() + monkeypatch.chdir(work) + product = tmp_path / "product" # where _agent_args points -d: outside cwd + args = _agent_args(["--llm-provider", "claude-code"], tmp_path) + + written = _run_ai_writing_to(product, args, capsys, monkeypatch) + + contract = (product / "contract.fluid.yaml").resolve() + assert [(e["path"], e["action"]) for e in written] == [(str(contract), "created")] + assert (product / ".fluid" / "forge-receipt.json").is_file() + + +def test_agent_ai_mode_sees_a_target_dir_deeper_than_the_cwd_scan(tmp_path, capsys, monkeypatch): + """The cwd scan stops two levels down; a deeper --target-dir still reports.""" + monkeypatch.chdir(tmp_path) + product = tmp_path / "domains" / "sales" / "orders" + args = _agent_args(["--llm-provider", "claude-code"], tmp_path) + args.target_dir = str(product) + + written = _run_ai_writing_to(product, args, capsys, monkeypatch) + + assert [(e["path"], e["action"]) for e in written] == [ + ("domains/sales/orders/contract.fluid.yaml", "created") + ] + + +def test_agent_ai_mode_reports_an_existing_contract_rewritten_in_place( + tmp_path, capsys, monkeypatch +): + """The target is scanned before the run as well, so a rewrite is "modified".""" + work = tmp_path / "work" + work.mkdir() + monkeypatch.chdir(work) + product = tmp_path / "product" + product.mkdir() + (product / "contract.fluid.yaml").write_text("fluidVersion: 0.7.5\n") + args = _agent_args(["--llm-provider", "claude-code"], tmp_path) + + written = _run_ai_writing_to(product, args, capsys, monkeypatch) + + assert [e["action"] for e in written] == ["modified"] + + +def test_agent_ai_mode_expands_a_tilde_target_dir(tmp_path, capsys, monkeypatch): + """A literal ``~`` (quoted, or from a program) is read as the writer reads it.""" + home = tmp_path / "home" + home.mkdir() + monkeypatch.setenv("HOME", str(home)) + work = tmp_path / "work" + work.mkdir() + monkeypatch.chdir(work) + args = _agent_args(["--llm-provider", "claude-code"], tmp_path) + args.target_dir = "~/prod" + + written = _run_ai_writing_to(home / "prod", args, capsys, monkeypatch) + + contract = (home / "prod" / "contract.fluid.yaml").resolve() + assert [(e["path"], e["action"]) for e in written] == [(str(contract), "created")] From 32b11ad19d9e96aabc2f9bb9aeaef3ff8c3dcf72 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 08:33:40 +0100 Subject: [PATCH 2/4] fix(ai): one Ollama address for every read, any host, from the stated endpoint OLLAMA_HOST was read four ways. The chat call dialled any host, but detection, forge's default-model pick and fluid ai setup ignored a non-local host with a warning and asked localhost, so the model came from one daemon and the call went to another. fluid ai test --endpoint refused a non-local Ollama outright. Owner decision: any host, as in ollama-python. OLLAMA_HOST is already a protected name that a project .env cannot set. - fluid_build/llm/ollama_host.py is the one reader, following ollama-python's _parse_host convention (MIT). It keeps user:password@, sent as basic auth, and reads the scheme case-insensitively. - A stated endpoint (--llm-endpoint, FLUID_LLM_ENDPOINT, ai test --endpoint) now drives the default-model pick, the model preflight and the ai test model check, which asked OLLAMA_HOST or localhost. A --tiered stage keeps a stated endpoint, as LlmConfig.for_routing already did. - LlmProvider.list_available_models takes an endpoint keyword, passed only when one is stated. The Ollama model list moved from urllib to httpx, so basic auth works and no redirect is followed; a non-http(s) URL is refused. - Credentials: fluid doctor and fluid ai setup show the address masked (providers.redact_endpoint), ai_config.json stores it without them unless FLUID_ALLOW_PLAINTEXT_AI_SECRETS is set, and the environment's OLLAMA_HOST wins over the saved host. --- CHANGELOG.md | 18 ++ fluid_build/cli/_ai_setup_ollama.py | 19 +- fluid_build/cli/_ai_setup_storage.py | 27 ++- fluid_build/cli/_ai_setup_test.py | 16 +- fluid_build/cli/ai_setup.py | 32 ++- fluid_build/cli/doctor.py | 5 +- fluid_build/copilot/agents/base.py | 8 +- fluid_build/llm/litellm_backend.py | 88 +++---- fluid_build/llm/ollama_host.py | 81 +++++++ fluid_build/llm/providers.py | 90 +++---- tests/test_llm_endpoint_sentinel.py | 22 +- tests/test_ollama_host.py | 341 +++++++++++++++++++++++++++ 12 files changed, 602 insertions(+), 145 deletions(-) create mode 100644 fluid_build/llm/ollama_host.py create mode 100644 tests/test_ollama_host.py diff --git a/CHANGELOG.md b/CHANGELOG.md index dd07aaf8..5cb7c787 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 directory, two levels deep, so a target outside it, or deeper, got neither: the run wrote the contract and the stream never said so. The stated target is scanned too, and a file outside the current directory is reported by its absolute path. +- **Forge asks the Ollama it calls.** With `--llm-endpoint` or `FLUID_LLM_ENDPOINT` set, forge + picked its default Ollama model from `OLLAMA_HOST` or localhost, and so did its model preflight + and `fluid ai test --endpoint`, which failed with "model ... was not returned by the provider" + for a model the endpoint has. All three now ask the stated endpoint. A `--tiered` stage now + calls a stated endpoint too, as the routing model already did; it called the provider's + default address instead. ### Changed @@ -71,6 +77,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `claude --help` lists it. Claude Code authentication that lives in a setting (an `apiKeyHelper`, or Bedrock or Vertex variables in `settings.json`) must come from the shell environment instead. +- **`OLLAMA_HOST` may name any machine, as in Ollama's own client.** The chat call already dialled + it, but detection, forge's default-model pick and `fluid ai setup` ignored a non-local host with + a warning and asked localhost, so the model came from one daemon and the call went to another. + Every Ollama read now goes through one helper, `fluid_build/llm/ollama_host.py`. A project + `.env` still cannot set `OLLAMA_HOST`. `fluid ai test --endpoint` accepts any Ollama host, over + http or https. A stated endpoint is read the same way (`--llm-endpoint gpu-box` means + `http://gpu-box:11434`) and keeps a `user:password@`, sent as basic auth. `fluid doctor` and + `fluid ai setup` show it masked, and `~/.fluid/ai_config.json` stores a saved address without + it unless `FLUID_ALLOW_PLAINTEXT_AI_SECRETS` is set, as for API keys. +- **`LlmProvider.list_available_models` takes an `endpoint` keyword**, the address the call will + go to, passed only when the operator states one. A provider plugin that overrides the method + with the two-argument signature keeps working until an endpoint is stated. ## [0.28.0] — 2026-11-06 diff --git a/fluid_build/cli/_ai_setup_ollama.py b/fluid_build/cli/_ai_setup_ollama.py index 88fe4b17..3a342bd7 100644 --- a/fluid_build/cli/_ai_setup_ollama.py +++ b/fluid_build/cli/_ai_setup_ollama.py @@ -11,7 +11,7 @@ Resolves host-module dependencies via the ``module-attribute-access`` indirection pattern so test patches on ``fluid_build.cli.ai_setup.`` (e.g. ``_save_ai_config``, -``_sanitize_ollama_host``) flow through to this module. +``_normalize_ollama_host``) flow through to this module. """ from __future__ import annotations @@ -33,8 +33,15 @@ def _host(): return _as -def _sanitize_ollama_host(host: str) -> str: - return _host()._sanitize_ollama_host(host) +def _normalize_ollama_host(host: str) -> str: + return _host()._normalize_ollama_host(host) + + +def _shown(host: str) -> str: + """*host* as printed to the terminal: credentials in the address masked.""" + from fluid_build.llm.providers import redact_endpoint + + return redact_endpoint(host) def _save_ai_config(*args, **kwargs): @@ -70,7 +77,7 @@ def _poll_for_ollama( char = spinner_chars[tick % len(spinner_chars)] try: console.print( - f"[dim]{char} Waiting for Ollama at {host}… " + f"[dim]{char} Waiting for Ollama at {_shown(host)}… " f"({int(deadline - time.time())}s left, Ctrl-C to give up)[/dim]" ) except Exception: # noqa: BLE001 @@ -129,14 +136,14 @@ def _setup_ollama(console: Any) -> Optional[Any]: LlmConfig = host_mod.LlmConfig provider = BUILTIN_LLM_PROVIDERS["ollama"] - host = _sanitize_ollama_host(os.environ.get("OLLAMA_HOST", "http://localhost:11434")) + host = _normalize_ollama_host(os.environ.get("OLLAMA_HOST") or "") os.environ["OLLAMA_HOST"] = host available_models = _query_ollama_models(host) if not available_models: console.print( "[yellow]Couldn't reach Ollama at " - f"[bold]{host}[/bold] (no models installed or daemon not running)." + f"[bold]{_shown(host)}[/bold] (no models installed or daemon not running)." "[/yellow]\n" "[dim]Install: [bold cyan]https://ollama.com[/bold cyan] · " "Then: [bold]ollama pull llama3.1[/bold][/dim]" diff --git a/fluid_build/cli/_ai_setup_storage.py b/fluid_build/cli/_ai_setup_storage.py index d58d93f9..5d1ccc6a 100644 --- a/fluid_build/cli/_ai_setup_storage.py +++ b/fluid_build/cli/_ai_setup_storage.py @@ -117,6 +117,26 @@ def _allow_plaintext_ai_secrets() -> bool: } +def _without_credentials(url: str) -> str: + """*url* minus any ``user:password@``, unless plaintext AI secrets are allowed.""" + if _allow_plaintext_ai_secrets(): + return url + scheme, sep, rest = url.partition("://") + if not sep: + scheme, rest = "", url + # The authority ends at the first "/", "?" or "#" (RFC 3986 section 3.2). + cut = min((i for i in (rest.find(c) for c in "/?#") if i != -1), default=len(rest)) + authority, tail = rest[:cut], rest[cut:] + if "@" not in authority: + return url + LOG.warning( + "Credentials in a saved AI address were not written to %s; keep them in the " + "environment (for example OLLAMA_HOST).", + _config_file(), + ) + return f"{scheme}{sep}{authority.rpartition('@')[2]}{tail}" + + def _read_config_file() -> Optional[dict]: """Read + JSON-parse ``~/.fluid/ai_config.json`` (delegates to the leaf). @@ -226,10 +246,13 @@ def _save_ai_config( LOG.warning("Plaintext local AI credential fallback is enabled.") else: LOG.debug("Keyring unavailable; sensitive AI value was not persisted.") + # A URL can carry ``user:password@``. Like an API key, it is written to the + # file only behind the plaintext opt-in; otherwise the address is saved + # without it and the credentials stay in the environment (OLLAMA_HOST, ...). if endpoint: - entry["endpoint"] = endpoint + entry["endpoint"] = _without_credentials(endpoint) if ollama_host: - entry["ollama_host"] = ollama_host + entry["ollama_host"] = _without_credentials(ollama_host) providers[provider] = entry return _write_config_map(active=provider, providers=providers) diff --git a/fluid_build/cli/_ai_setup_test.py b/fluid_build/cli/_ai_setup_test.py index 7386e932..6226587c 100644 --- a/fluid_build/cli/_ai_setup_test.py +++ b/fluid_build/cli/_ai_setup_test.py @@ -64,8 +64,8 @@ def detect_ollama_available(*args, **kwargs): return _host().detect_ollama_available(*args, **kwargs) -def _sanitize_ollama_host(*args, **kwargs): - return _host()._sanitize_ollama_host(*args, **kwargs) +def _normalize_ollama_host(*args, **kwargs): + return _host()._normalize_ollama_host(*args, **kwargs) def _resolve_cloud_api_key(*args, **kwargs): @@ -193,10 +193,9 @@ def _validate_ai_test_endpoint(provider_name: str, endpoint: str) -> Optional[st return "AI test endpoint URLs must not embed credentials." if parsed.scheme not in {"http", "https"} or not parsed.netloc: return "AI test endpoint must be an absolute HTTP(S) URL." - host = (parsed.hostname or "").lower() if provider_name == "ollama": - if parsed.scheme != "http" or host not in {"localhost", "127.0.0.1", "::1"}: - return "Ollama AI test endpoints must stay on localhost." + # Any host, over http or https: the same as every other Ollama read + # (fluid_build/llm/ollama_host.py). Ollama sends no API key. return None if parsed.scheme != "https": return "Cloud AI test endpoints must use HTTPS to avoid sending API keys over plaintext." @@ -247,7 +246,7 @@ def _build_for_provider( if pname == "ollama": host = saved_for_provider.get("ollama_host") if host and not os.environ.get("OLLAMA_HOST"): - os.environ["OLLAMA_HOST"] = _sanitize_ollama_host(str(host)) + os.environ["OLLAMA_HOST"] = _normalize_ollama_host(str(host)) # Only an operator-supplied endpoint is validated; with none, the # call goes through litellm, which owns the address. if endpoint_override: @@ -354,7 +353,10 @@ def _check_ai_test_model_availability( provider: Any, config: LlmConfig ) -> tuple[str, Optional[list[str]]]: try: - available = provider.list_available_models(config.api_key, dict(os.environ)) + # The list comes from where the test call goes: the --endpoint, if any. + # The keyword goes only with an endpoint (two-argument plugins keep working). + stated = {"endpoint": config.endpoint} if config.endpoint else {} + available = provider.list_available_models(config.api_key, dict(os.environ), **stated) except httpx.HTTPStatusError as exc: raise CopilotGenerationError( "ai_test_model_preflight_failed", diff --git a/fluid_build/cli/ai_setup.py b/fluid_build/cli/ai_setup.py index a389c679..4fb09e42 100644 --- a/fluid_build/cli/ai_setup.py +++ b/fluid_build/cli/ai_setup.py @@ -137,22 +137,16 @@ def __getattr__(name: str): LOG = logging.getLogger("fluid.cli.ai_setup") -# SSRF guard: restrict Ollama host to localhost addresses only. -_LOCALHOST_PREFIXES = ( - "http://localhost", - "http://127.0.0.1", - "http://[::1]", - "http://0.0.0.0", -) +def _normalize_ollama_host(host: str) -> str: + """*host* as a URL to dial, or the default local address when blank. + + Any machine is allowed, the same as for every other Ollama read + (``fluid_build/llm/ollama_host.py``). + """ + from fluid_build.llm.ollama_host import ollama_daemon_url -def _sanitize_ollama_host(host: str) -> str: - """Return *host* if it points to localhost, otherwise fall back to the default.""" - clean = (host or "http://localhost:11434").rstrip("/") - if not any(clean.lower().startswith(p) for p in _LOCALHOST_PREFIXES): - LOG.warning("OLLAMA_HOST points to a non-localhost address (%s), ignoring.", clean) - return "http://localhost:11434" - return clean + return ollama_daemon_url({}, host) # Keyring key prefix used when persisting API keys. @@ -1388,10 +1382,12 @@ def run_ai_setup_inline(console: Any) -> Optional[LlmConfig]: LOG.info("Inline AI setup: loaded coding agent %s from saved config", pname) return config if pname == "ollama": - ollama_host = _sanitize_ollama_host( - saved.get("ollama_host", "http://localhost:11434") - ) - os.environ["OLLAMA_HOST"] = ollama_host + # The environment wins over the saved address: it may hold the + # ``user:password@`` that ai_config.json leaves out. + if not os.environ.get("OLLAMA_HOST"): + os.environ["OLLAMA_HOST"] = _normalize_ollama_host( + saved.get("ollama_host") or "" + ) config = _make_ollama_config(model=model) if console and RICH_AVAILABLE: console.print(f"[dim]Using Ollama ({config.model}).[/dim]") diff --git a/fluid_build/cli/doctor.py b/fluid_build/cli/doctor.py index f9871ee8..7d7b8972 100644 --- a/fluid_build/cli/doctor.py +++ b/fluid_build/cli/doctor.py @@ -1412,7 +1412,10 @@ def _print_copilot_readiness(readiness: LlmReadinessCheck, verbose: bool = False status_text = "✅ Ready" if readiness.ready else "⚠️ Setup needed" auth_text = "Configured" if readiness.auth_available else "Missing" # No endpoint of ours is not "not configured": litellm picks the provider's own. - endpoint_text = readiness.endpoint or ( + # An endpoint can carry ``user:password@``, so it is shown masked. + from fluid_build.llm.providers import redact_endpoint + + endpoint_text = redact_endpoint(readiness.endpoint) or ( "Provider default" if readiness.provider else "Not configured" ) diff --git a/fluid_build/copilot/agents/base.py b/fluid_build/copilot/agents/base.py index aaada9b3..a6771372 100644 --- a/fluid_build/copilot/agents/base.py +++ b/fluid_build/copilot/agents/base.py @@ -272,8 +272,12 @@ def resolve_model(self, session: StageSession) -> Optional[LlmConfig]: or get_catalog_tier_model(provider_name, self.tier) or session.llm_config.model ) - provider = BUILTIN_LLM_PROVIDERS[provider_name] - endpoint = provider.default_endpoint(tier_model, dict(os.environ)) + if session.llm_config.endpoint_explicit: + # A stated endpoint is where every call goes, as in ``LlmConfig.for_routing``. + endpoint = session.llm_config.endpoint + else: + provider = BUILTIN_LLM_PROVIDERS[provider_name] + endpoint = provider.default_endpoint(tier_model, dict(os.environ)) return dataclasses.replace(session.llm_config, model=tier_model, endpoint=endpoint) def cache_namespace(self) -> str: diff --git a/fluid_build/llm/litellm_backend.py b/fluid_build/llm/litellm_backend.py index d4a8da48..e04bac6d 100644 --- a/fluid_build/llm/litellm_backend.py +++ b/fluid_build/llm/litellm_backend.py @@ -18,16 +18,15 @@ from __future__ import annotations -import ipaddress import json import logging import os import sys import threading import time -import urllib.parse from typing import Any, Dict, Iterator, List, Mapping, Optional, Tuple +from fluid_build.llm.ollama_host import ollama_base_url, ollama_daemon_url from fluid_build.llm.providers import ( CopilotGenerationError, LlmConfig, @@ -257,43 +256,6 @@ def _is_anthropic_model(model_id: str) -> bool: return "claude" in lower -def _ollama_base_url(env: Mapping[str, str]) -> str: - """``OLLAMA_HOST`` as a URL, or ``""`` when unset. - - Ollama's own variable accepts forms litellm and ``urllib`` cannot dial as - they are (a bare ``host``, ``host:port``, ``:port``), so it is normalised - with the convention of ollama-python's ``_parse_host`` (MIT, - github.com/ollama/ollama-python, ``ollama/_client.py``): no scheme means - ``http`` on the default port 11434; an explicit ``http://`` or ``https://`` - without a port means port 80 or 443; a path is kept. Both reads of - ``OLLAMA_HOST`` in this module go through here, so they agree. - """ - host = (env.get("OLLAMA_HOST") or "").strip() - if not host: - return "" - port = 11434 - scheme, _, hostport = host.partition("://") - if not hostport: - scheme, hostport = "http", host - elif scheme == "http": - port = 80 - elif scheme == "https": - port = 443 - try: - split = urllib.parse.urlsplit(f"{scheme}://{hostport}") - name = split.hostname or "127.0.0.1" - port = split.port or port - except ValueError: # e.g. a non-numeric port: leave it to fail visibly at the dial - return f"{scheme}://{hostport}".rstrip("/") - try: - if isinstance(ipaddress.ip_address(name), ipaddress.IPv6Address): - name = f"[{name}]" # urlsplit strips the brackets - except ValueError: - pass # a hostname, not an IP literal - path = split.path.strip("/") - return f"{scheme}://{name}:{port}" + (f"/{path}" if path else "") - - # --------------------------------------------------------------------------- # Adapter # --------------------------------------------------------------------------- @@ -330,7 +292,7 @@ def default_endpoint(self, model: str, env: Mapping[str, str]) -> str: reported. Never a placeholder URL: every consumer of ``config.endpoint`` treats a non-empty value as an address to dial. """ - return _ollama_base_url(env) if self.name == "ollama" else "" + return ollama_base_url(env) if self.name == "ollama" else "" def _apply_ollama_api_base(self, payload: Dict[str, Any], config: LlmConfig) -> None: """Point an Ollama call at an explicit endpoint, else ``OLLAMA_HOST``. @@ -340,7 +302,7 @@ def _apply_ollama_api_base(self, payload: Dict[str, Any], config: LlmConfig) -> """ if self.name != "ollama": return - api_base = config.endpoint or _ollama_base_url(os.environ) + api_base = ollama_base_url(os.environ, config.endpoint) if api_base: payload["api_base"] = api_base @@ -478,7 +440,10 @@ def choose_available_model( return available_models[0] def list_available_models( - self, api_key: Optional[str], env: Mapping[str, str] + self, + api_key: Optional[str], + env: Mapping[str, str], + endpoint: Optional[str] = None, ) -> Optional[List[str]]: """Return the model ids litellm knows for this provider. @@ -487,16 +452,17 @@ def list_available_models( kept current upstream — so a stale catalog default in fluid_build can be auto-corrected to a live name. Falls back to ``None`` (preflight unavailable) when litellm is missing or - the provider isn't in its registry. + the provider isn't in its registry. For Ollama the list is the + live one of the daemon at *endpoint*, else ``OLLAMA_HOST``. """ try: litellm = _get_litellm() except CopilotGenerationError: return None - # Ollama: hit the local daemon for live model list. Everything + # Ollama: ask the daemon for its live model list. Everything # else: read litellm's static models_by_provider map. if self.name == "ollama": - return _list_ollama_models(env) + return _list_ollama_models(env, endpoint) provider_key = _LITELLM_PREFIX_BY_PROVIDER.get(self.name, self.name) registry = getattr(litellm, "models_by_provider", None) or {} ids = registry.get(provider_key) @@ -776,23 +742,29 @@ def invoke_streaming( # --------------------------------------------------------------------------- -def _list_ollama_models(env: Mapping[str, str]) -> Optional[List[str]]: - """Hit the local Ollama daemon for its installed model list. +def _list_ollama_models( + env: Mapping[str, str], endpoint: Optional[str] = None +) -> Optional[List[str]]: + """Ask the Ollama daemon for its installed model list. - litellm doesn't ship a static Ollama catalog (model ids depend on - what the user has pulled). The daemon's ``/api/tags`` endpoint - returns the live list. Returns ``None`` if the daemon isn't - reachable so preflight degrades to "unavailable" rather than - falsely rejecting models. + The daemon is the one the chat call dials: *endpoint* when given, else + ``OLLAMA_HOST``, else the default local address. litellm doesn't ship a + static Ollama catalog (model ids depend on what the user has pulled). The + daemon's ``/api/tags`` endpoint returns the live list. Returns ``None`` if + the daemon isn't reachable so preflight degrades to "unavailable" rather + than falsely rejecting models. """ - base = _ollama_base_url(env) or "http://localhost:11434" + base = ollama_daemon_url(env, endpoint) + if not base.startswith(("http://", "https://")): + return None # only an HTTP daemon has a model list; never a file:// read try: - import urllib.error - import urllib.request + # httpx, as the chat call's transport: it sends ``user:password@`` as + # basic auth, which urllib cannot, and follows no redirect. + import httpx - req = urllib.request.Request(f"{base}/api/tags") - with urllib.request.urlopen(req, timeout=2.0) as resp: # nosec B310 — local daemon - payload = json.loads(resp.read().decode("utf-8")) + resp = httpx.get(f"{base}/api/tags", timeout=2.0) + resp.raise_for_status() + payload = resp.json() except Exception: # noqa: BLE001 — daemon down → preflight unavailable return None models = payload.get("models") if isinstance(payload, dict) else None diff --git a/fluid_build/llm/ollama_host.py b/fluid_build/llm/ollama_host.py new file mode 100644 index 00000000..27883b8a --- /dev/null +++ b/fluid_build/llm/ollama_host.py @@ -0,0 +1,81 @@ +# Copyright 2025 Agentics Transformation Limited +# SPDX-License-Identifier: Apache-2.0 + +"""Where the Ollama daemon is, read one way everywhere in fluid. + +Every Ollama call goes through here: the chat call, the model list, the +default-model pick, local detection, ``fluid ai setup`` and ``fluid ai test``. +``OLLAMA_HOST`` and an explicit endpoint (``--llm-endpoint``, +``FLUID_LLM_ENDPOINT``, ``fluid ai test --endpoint``) may name any machine, as +the ollama-python client allows. A project ``.env`` file cannot set +``OLLAMA_HOST``: it is a protected name +(``fluid_build/credentials/protected_env.py``). + +Standard library only, so any module can import it without cost. +""" + +from __future__ import annotations + +import ipaddress +import urllib.parse +from typing import Mapping, Optional + +__all__ = ["DEFAULT_OLLAMA_URL", "normalize_ollama_host", "ollama_base_url", "ollama_daemon_url"] + +#: The address an Ollama daemon listens on by default. +DEFAULT_OLLAMA_URL = "http://localhost:11434" + + +def normalize_ollama_host(host: str) -> str: + """*host* as a URL that ``litellm`` and ``urllib`` can dial; ``""`` when blank. + + Ollama accepts forms neither can dial as they are (a bare ``host``, + ``host:port``, ``:port``), so *host* is normalised with the convention of + ollama-python's ``_parse_host`` (MIT, github.com/ollama/ollama-python, + ``ollama/_client.py``): no scheme means ``http`` on the default port + 11434; an explicit ``http://`` or ``https://`` without a port means port + 80 or 443; a path is kept. Unlike ``_parse_host``, ``user:password@`` is + kept too, so an Ollama behind a basic-auth proxy stays reachable, and the + scheme is read case-insensitively, as RFC 3986 says. + """ + host = (host or "").strip() + if not host: + return "" + port = 11434 + scheme, _, hostport = host.partition("://") + scheme = scheme.lower() # a URL scheme is case-insensitive (RFC 3986) + if not hostport: + scheme, hostport = "http", host + elif scheme == "http": + port = 80 + elif scheme == "https": + port = 443 + try: + split = urllib.parse.urlsplit(f"{scheme}://{hostport}") + name = split.hostname or "127.0.0.1" + port = split.port or port + except ValueError: # e.g. a non-numeric port: leave it to fail visibly at the dial + return f"{scheme}://{hostport}".rstrip("/") + try: + if isinstance(ipaddress.ip_address(name), ipaddress.IPv6Address): + name = f"[{name}]" # urlsplit strips the brackets + except ValueError: + pass # a hostname, not an IP literal + userinfo = split.netloc.rpartition("@")[0] + path = split.path.strip("/") + return f"{scheme}://{userinfo + '@' if userinfo else ''}{name}:{port}" + ( + f"/{path}" if path else "" + ) + + +def ollama_base_url(env: Mapping[str, str], endpoint: Optional[str] = None) -> str: + """The explicit *endpoint*, else ``OLLAMA_HOST``, normalised; ``""`` when neither is set. + + ``""`` lets litellm use its own default address for the chat call. + """ + return normalize_ollama_host(endpoint or env.get("OLLAMA_HOST") or "") + + +def ollama_daemon_url(env: Mapping[str, str], endpoint: Optional[str] = None) -> str: + """Like :func:`ollama_base_url`, but the daemon's default address when neither is set.""" + return ollama_base_url(env, endpoint) or DEFAULT_OLLAMA_URL diff --git a/fluid_build/llm/providers.py b/fluid_build/llm/providers.py index 7cbe98d6..2d5e8f1d 100644 --- a/fluid_build/llm/providers.py +++ b/fluid_build/llm/providers.py @@ -56,6 +56,7 @@ import httpx from fluid_build._contract_loader import CLIError +from fluid_build.llm.ollama_host import ollama_daemon_url # Per-provider response_format helpers (``openai_response_format``, # ``gemini_response_schema_config``, ``anthropic_tool_definition``, @@ -271,17 +272,25 @@ def for_routing(self) -> "LlmConfig": @property def redacted_endpoint(self) -> str: - endpoint = self.endpoint - # Redact common credential query parameters - endpoint = re.sub( - r"([?&](?:key|token|api_key|auth|secret|credential|password)=)[^&]+", - r"\1***", - endpoint, - flags=re.I, - ) - # Redact userinfo in URLs (user:pass@host) - endpoint = re.sub(r"(https?://)([^@/]+)@", r"\1***:***@", endpoint) - return endpoint + return redact_endpoint(self.endpoint) + + +def redact_endpoint(endpoint: str) -> str: + """*endpoint* fit to show a person: credential query values and all userinfo masked. + + The whole ``user:password@`` goes, not only the password: a token is often + passed as a bare username (``https://TOKEN@host``). + """ + endpoint = endpoint or "" + # Redact common credential query parameters + endpoint = re.sub( + r"([?&](?:key|token|api_key|auth|secret|credential|password)=)[^&]+", + r"\1***", + endpoint, + flags=re.I, + ) + # Redact userinfo in URLs (user:pass@host); a URL scheme is case-insensitive. + return re.sub(r"(https?://)([^@/]+)@", r"\1***:***@", endpoint, flags=re.I) # --------------------------------------------------------------------------- @@ -323,10 +332,15 @@ def extract_prompt_cache(self, response_json: Dict[str, Any]) -> Dict[str, Any]: return _prompt_cache_metrics(0, 0) def list_available_models( - self, api_key: Optional[str], env: Mapping[str, str] + self, + api_key: Optional[str], + env: Mapping[str, str], + endpoint: Optional[str] = None, ) -> Optional[List[str]]: """Return live provider model ids when a model-list API exists. + *endpoint* is the address the operator stated for the provider, when + there is one: the list must come from where the call will go. Providers without a lightweight list endpoint return ``None``. Callers should treat that as "preflight unavailable", not as "this provider has no models". @@ -900,6 +914,11 @@ def resolve_llm_config(args: Any, environ: Optional[Mapping[str, str]] = None) - provider_name = _provider_name_from(args, env) provider = get_llm_provider(provider_name) + # A stated endpoint is where the call goes, so the Ollama model pick and the + # model preflight below ask that daemon, not OLLAMA_HOST or localhost. + stated_endpoint = getattr(args, "llm_endpoint", None) or env.get("FLUID_LLM_ENDPOINT") + models_endpoint = stated_endpoint if isinstance(stated_endpoint, str) else None + # Resolve model: explicit flag → env var → catalog default → class default. catalog_default = get_catalog_default(provider.name) explicit_model = getattr(args, "llm_model", None) or env.get("FLUID_LLM_MODEL") @@ -907,7 +926,7 @@ def resolve_llm_config(args: Any, environ: Optional[Mapping[str, str]] = None) - model = resolve_model_name(provider.name, explicit_model) model_source = "explicit" elif provider.name == "ollama": - model = resolve_ollama_model(env) + model = resolve_ollama_model(env, models_endpoint) model_source = "ollama" else: model = catalog_default or provider.default_model @@ -963,7 +982,10 @@ def _available_models() -> Optional[List[str]]: if available_models is not None: return available_models try: - available_models = provider.list_available_models(api_key, env) + # The keyword goes only with a stated endpoint, so a provider plugin + # written against the two-argument signature keeps working without one. + stated = {"endpoint": models_endpoint} if models_endpoint else {} + available_models = provider.list_available_models(api_key, env, **stated) except Exception as exc: # noqa: BLE001 raise CopilotGenerationError( "copilot_llm_model_preflight_failed", @@ -1023,7 +1045,7 @@ def _available_models() -> Optional[List[str]]: model = replacement model_source = "live_preflight" - endpoint = getattr(args, "llm_endpoint", None) or env.get("FLUID_LLM_ENDPOINT") + endpoint = stated_endpoint endpoint_explicit = bool(endpoint) if not endpoint: endpoint = provider.default_endpoint(model, env) @@ -1802,32 +1824,16 @@ def build_llm_run_plan(config: "LlmConfig", *, tiered: bool = False) -> Dict[str # --------------------------------------------------------------------------- -_LOCALHOST_PREFIXES = ( - "http://localhost", - "http://127.0.0.1", - "http://[::1]", - "http://0.0.0.0", -) - _ollama_available_cache: Optional[bool] = None -def _ollama_host(env: Mapping[str, str]) -> str: - host = (env.get("OLLAMA_HOST") or "http://localhost:11434").rstrip("/") - # SSRF guard: only allow localhost targets for Ollama. - if not any(host.lower().startswith(prefix) for prefix in _LOCALHOST_PREFIXES): - LOG.warning("OLLAMA_HOST points to a non-localhost address (%s), ignoring.", host) - return "http://localhost:11434" - return host - - def detect_ollama_available(env: Mapping[str, str]) -> bool: """Return ``True`` if a local Ollama instance is reachable (cached per-process).""" global _ollama_available_cache # noqa: PLW0603 if _ollama_available_cache is not None: return _ollama_available_cache try: - resp = httpx.get(f"{_ollama_host(env)}/api/version", timeout=1.0) + resp = httpx.get(f"{ollama_daemon_url(env)}/api/version", timeout=1.0) _ollama_available_cache = resp.status_code == 200 except Exception: # noqa: BLE001 _ollama_available_cache = False @@ -1847,14 +1853,18 @@ def _parse_param_size(value: str) -> float: return number -def query_ollama_models(env: Mapping[str, str]) -> List[Dict[str, Any]]: - """Query the local Ollama instance for downloaded models. +def query_ollama_models( + env: Mapping[str, str], endpoint: Optional[str] = None +) -> List[Dict[str, Any]]: + """Query the Ollama daemon for downloaded models. - Returns a list of model dicts sorted by parameter size (largest first), - or an empty list if Ollama is unreachable. + The daemon is *endpoint* when given, else ``OLLAMA_HOST``, else the + default local address: the one the chat call dials. Returns a list of + model dicts sorted by parameter size (largest first), or an empty list + if Ollama is unreachable. """ try: - resp = httpx.get(f"{_ollama_host(env)}/api/tags", timeout=2.0) + resp = httpx.get(f"{ollama_daemon_url(env, endpoint)}/api/tags", timeout=2.0) resp.raise_for_status() models = resp.json().get("models") or [] for m in models: @@ -1866,9 +1876,9 @@ def query_ollama_models(env: Mapping[str, str]) -> List[Dict[str, Any]]: return [] -def resolve_ollama_model(env: Mapping[str, str]) -> str: - """Return the best locally-available Ollama model, or the static fallback.""" - models = query_ollama_models(env) +def resolve_ollama_model(env: Mapping[str, str], endpoint: Optional[str] = None) -> str: + """Return the best model the Ollama daemon has, or the static fallback.""" + models = query_ollama_models(env, endpoint) if models: return models[0]["name"] return get_catalog_default("ollama") or OllamaProvider.default_model diff --git a/tests/test_llm_endpoint_sentinel.py b/tests/test_llm_endpoint_sentinel.py index fb745115..7a67815e 100644 --- a/tests/test_llm_endpoint_sentinel.py +++ b/tests/test_llm_endpoint_sentinel.py @@ -196,7 +196,7 @@ def test_copying_the_endpoint_into_args_changes_nothing(self, host): assert _payload(second).get("api_base") == _payload(first).get("api_base") -# Forms of OLLAMA_HOST -> the URL both reads of it use. Same rules as +# Forms of OLLAMA_HOST -> the URL every read of it uses. Same rules as # ollama-python's ``_parse_host``: no scheme means http on 11434; an explicit # scheme without a port means that scheme's default port; a path is kept. _OLLAMA_HOST_FORMS = [ @@ -221,35 +221,35 @@ def test_copying_the_endpoint_into_args_changes_nothing(self, host): class TestOllamaHostForms: @pytest.mark.parametrize("host, expected", _OLLAMA_HOST_FORMS) def test_base_url(self, host, expected): - from fluid_build.llm.litellm_backend import _ollama_base_url + from fluid_build.llm.ollama_host import ollama_base_url - assert _ollama_base_url({"OLLAMA_HOST": host}) == expected + assert ollama_base_url({"OLLAMA_HOST": host}) == expected @pytest.mark.parametrize("env", [{}, {"OLLAMA_HOST": ""}, {"OLLAMA_HOST": " "}]) def test_unset_stays_empty_for_litellm_to_choose(self, env): - from fluid_build.llm.litellm_backend import _ollama_base_url + from fluid_build.llm.ollama_host import ollama_base_url - assert _ollama_base_url(env) == "" + assert ollama_base_url(env) == "" def test_an_unparseable_port_is_left_to_fail_at_the_dial(self): - from fluid_build.llm.litellm_backend import _ollama_base_url + from fluid_build.llm.ollama_host import ollama_base_url - assert _ollama_base_url({"OLLAMA_HOST": "myhost:abc"}) == "http://myhost:abc" + assert ollama_base_url({"OLLAMA_HOST": "myhost:abc"}) == "http://myhost:abc" @pytest.mark.parametrize("host, expected", _OLLAMA_HOST_FORMS) def test_the_model_list_dials_the_same_url(self, host, expected): from fluid_build.llm.litellm_backend import _list_ollama_models - with mock.patch("urllib.request.urlopen", side_effect=OSError("down")) as urlopen: + with mock.patch("httpx.get", side_effect=OSError("down")) as get: assert _list_ollama_models({"OLLAMA_HOST": host}) is None - assert urlopen.call_args.args[0].full_url == f"{expected}/api/tags" + assert get.call_args.args[0] == f"{expected}/api/tags" def test_the_model_list_defaults_to_localhost(self): from fluid_build.llm.litellm_backend import _list_ollama_models - with mock.patch("urllib.request.urlopen", side_effect=OSError("down")) as urlopen: + with mock.patch("httpx.get", side_effect=OSError("down")) as get: _list_ollama_models({}) - assert urlopen.call_args.args[0].full_url == "http://localhost:11434/api/tags" + assert get.call_args.args[0] == "http://localhost:11434/api/tags" def test_resolved_endpoint_and_api_base_use_the_bare_host_port(self): config = resolve_llm_config(_args(llm_provider="ollama"), {"OLLAMA_HOST": "myhost"}) diff --git a/tests/test_ollama_host.py b/tests/test_ollama_host.py new file mode 100644 index 00000000..497d20b7 --- /dev/null +++ b/tests/test_ollama_host.py @@ -0,0 +1,341 @@ +# Copyright 2025 Agentics Transformation Limited +# SPDX-License-Identifier: Apache-2.0 + +"""Every Ollama read agrees on which daemon, and any host is honoured. + +Founder decision (2026-11-12): ``OLLAMA_HOST`` may name any machine, as the +ollama-python client allows; a project ``.env`` cannot set it. Before, the chat +call dialled a remote ``OLLAMA_HOST`` while detection, the default-model pick +and ``fluid ai setup`` ignored it with a warning and asked localhost instead. + +A stated endpoint is where the chat call goes, so the model list, the +default-model pick and ``fluid ai test --endpoint`` ask that daemon too. + +No test here touches the network: ``httpx.get`` and ``urlopen`` are patched. +""" + +from __future__ import annotations + +import logging +from types import SimpleNamespace +from unittest import mock + +import pytest + +from fluid_build.llm import providers +from fluid_build.llm.ollama_host import DEFAULT_OLLAMA_URL, ollama_base_url, ollama_daemon_url +from fluid_build.llm.providers import BUILTIN_LLM_PROVIDERS, LlmConfig, resolve_llm_config + +pytestmark = pytest.mark.unit + +_REMOTE = "http://gpu-box.lan:11434" +_LOCAL = "http://127.0.0.1:11434" +_TAGS = {"models": [{"name": "big:70b", "details": {"parameter_size": "70B"}}]} + + +@pytest.fixture(autouse=True) +def _clean(monkeypatch): + for var in ("OLLAMA_HOST", "FLUID_LLM_ENDPOINT", "FLUID_LLM_PROVIDER", "FLUID_LLM_MODEL"): + monkeypatch.delenv(var, raising=False) + providers.reset_llm_caches() + yield + providers.reset_llm_caches() + + +def _args(**kw): + base = {"llm_provider": "ollama", "llm_model": None, "llm_endpoint": None} + base.update(kw) + return SimpleNamespace(**base) + + +def _httpx_tags(): + response = mock.MagicMock(status_code=200) + response.json.return_value = _TAGS + return mock.patch.object(providers.httpx, "get", return_value=response) + + +def _payload(config: LlmConfig) -> dict: + _, payload = BUILTIN_LLM_PROVIDERS["ollama"].build_request(config, "sys", "usr") + return payload + + +class TestOneReading: + def test_a_stated_endpoint_beats_ollama_host(self): + assert ollama_base_url({"OLLAMA_HOST": _LOCAL}, "gpu-box.lan") == _REMOTE + + def test_a_stated_endpoint_is_normalised_like_ollama_host(self): + assert ollama_base_url({}, "gpu-box.lan:11434/") == _REMOTE + + def test_the_daemon_defaults_to_localhost(self): + assert ollama_daemon_url({}) == DEFAULT_OLLAMA_URL + assert ollama_daemon_url({"OLLAMA_HOST": " "}) == DEFAULT_OLLAMA_URL + + +class TestAnyHostIsHonoured: + def test_detection_dials_a_remote_ollama_host(self): + with _httpx_tags() as get: + assert providers.detect_ollama_available({"OLLAMA_HOST": _REMOTE}) is True + assert get.call_args.args[0] == f"{_REMOTE}/api/version" + + def test_the_default_model_comes_from_the_remote_daemon(self): + with _httpx_tags() as get: + assert providers.resolve_ollama_model({"OLLAMA_HOST": _REMOTE}) == "big:70b" + assert get.call_args.args[0] == f"{_REMOTE}/api/tags" + + def test_ai_setup_keeps_a_remote_host_without_a_warning(self, caplog): + from fluid_build.cli.ai_setup import _normalize_ollama_host + + with caplog.at_level(logging.WARNING): + assert _normalize_ollama_host(_REMOTE) == _REMOTE + assert not caplog.records + + def test_ai_setup_blank_host_is_the_local_default(self): + from fluid_build.cli.ai_setup import _normalize_ollama_host + + assert _normalize_ollama_host("") == DEFAULT_OLLAMA_URL + + def test_the_chat_call_and_the_model_pick_ask_the_same_daemon(self): + with _httpx_tags() as get: + config = resolve_llm_config(_args(), {"OLLAMA_HOST": "gpu-box.lan"}) + assert get.call_args.args[0] == f"{_REMOTE}/api/tags" + assert config.model == "big:70b" + assert _payload(config)["api_base"] == _REMOTE + + +class TestAStatedEndpointDrivesTheModelLookups: + @pytest.mark.parametrize("how", ["flag", "env"]) + def test_the_default_model_is_picked_from_the_stated_endpoint(self, how): + args = _args(llm_endpoint=_REMOTE) if how == "flag" else _args() + env = {"OLLAMA_HOST": _LOCAL} + if how == "env": + env["FLUID_LLM_ENDPOINT"] = _REMOTE + with _httpx_tags() as get: + config = resolve_llm_config(args, env) + assert get.call_args.args[0] == f"{_REMOTE}/api/tags" + assert config.model == "big:70b" + assert config.endpoint == _REMOTE + + def test_the_model_preflight_lists_the_stated_endpoint(self): + env = {"OLLAMA_HOST": _LOCAL, "FLUID_LLM_MODEL_PREFLIGHT": "1"} + args = _args(llm_endpoint=_REMOTE, llm_model="big:70b") + with _httpx_tags() as get: + config = resolve_llm_config(args, env) + assert get.call_args.args[0] == f"{_REMOTE}/api/tags" + assert not get.call_args.kwargs.get("follow_redirects", False) + assert config.model == "big:70b" + + def test_a_bare_stated_endpoint_reaches_the_chat_call_as_a_url(self): + config = resolve_llm_config(_args(llm_endpoint="gpu-box.lan", llm_model="m"), {"X": "1"}) + assert _payload(config)["api_base"] == _REMOTE + + def test_ai_test_lists_the_models_of_its_endpoint(self): + from fluid_build.cli._ai_setup_test import _check_ai_test_model_availability + + config = LlmConfig(provider="ollama", model="big:70b", endpoint=_REMOTE, api_key=None) + with _httpx_tags() as get: + status, available = _check_ai_test_model_availability( + BUILTIN_LLM_PROVIDERS["ollama"], config + ) + assert get.call_args.args[0] == f"{_REMOTE}/api/tags" + assert (status, available) == ("available", ["big:70b"]) + + +def test_the_model_list_never_opens_a_non_http_url(): + from fluid_build.llm.litellm_backend import _list_ollama_models + + with mock.patch("httpx.get") as get: + assert _list_ollama_models({}, "file:///etc/passwd") is None + get.assert_not_called() + + +class TestCredentialsInTheAddress: + def test_the_normaliser_keeps_user_and_password(self): + # An Ollama behind a basic-auth proxy: ollama-python's _parse_host drops + # the userinfo, fluid keeps it so the stated address still works. + assert ollama_base_url({}, "https://u:p@proxy.lan/ollama/") == ( + "https://u:p@proxy.lan:443/ollama" # pragma: allowlist secret + ) + + @pytest.mark.parametrize( + "endpoint", + [ + "http://u:hunter2@proxy.lan:11434", # pragma: allowlist secret + "http://hunter2@proxy.lan:11434", # a token passed as a bare username + ], + ) + def test_doctor_masks_the_credentials(self, capsys, endpoint): + from fluid_build.cli.doctor import _print_copilot_readiness + from fluid_build.llm.providers import LlmReadinessCheck + + readiness = LlmReadinessCheck( + ready=True, provider="ollama", model="m", endpoint=endpoint, auth_available=True + ) + _print_copilot_readiness(readiness) + out = capsys.readouterr().out + assert "hunter2" not in out + assert "proxy.lan:11434" in out + + def test_the_model_list_sends_them_as_basic_auth(self): + # httpx turns userinfo into an Authorization header; urllib could not dial it. + from fluid_build.llm.litellm_backend import _list_ollama_models + + with _httpx_tags() as get: + assert _list_ollama_models({}, "https://u:p@proxy.lan/ollama") == ["big:70b"] + assert get.call_args.args[0] == ( + "https://u:p@proxy.lan:443/ollama/api/tags" # pragma: allowlist secret + ) + + def test_ai_setup_never_prints_them(self, monkeypatch): + from fluid_build.cli import _ai_setup_ollama + + secret_host = "https://alice:s3cret@ollama.corp" # pragma: allowlist secret + monkeypatch.setenv("OLLAMA_HOST", secret_host) + monkeypatch.setattr(_ai_setup_ollama, "_query_ollama_models", lambda host: []) + monkeypatch.setattr("fluid_build.cli.forge_ui.ask_numbered_choice", lambda *a, **k: "skip") + console = mock.MagicMock() + assert _ai_setup_ollama._setup_ollama(console) is None + printed = " ".join(str(c.args[0]) for c in console.print.call_args_list if c.args) + assert "Couldn't reach Ollama" in printed + assert "s3cret" not in printed and "alice" not in printed + + @pytest.mark.parametrize("plaintext_opt_in", [False, True]) + def test_ai_config_json_keeps_them_only_behind_the_plaintext_opt_in( + self, tmp_path, monkeypatch, plaintext_opt_in + ): + from fluid_build.cli.ai_setup import _load_ai_config, _save_ai_config + + if plaintext_opt_in: + monkeypatch.setenv("FLUID_ALLOW_PLAINTEXT_AI_SECRETS", "1") + else: + monkeypatch.delenv("FLUID_ALLOW_PLAINTEXT_AI_SECRETS", raising=False) + secret_host = "https://alice:s3cret@ollama.corp:443" # pragma: allowlist secret + config_file = tmp_path / "ai_config.json" + with ( + mock.patch("fluid_build.cli.ai_setup._CONFIG_FILE", config_file), + mock.patch("fluid_build.cli.ai_setup._CONFIG_DIR", tmp_path), + ): + _save_ai_config("ollama", "m", ollama_host=secret_host, endpoint=secret_host) + loaded = _load_ai_config() + expected = secret_host if plaintext_opt_in else "https://ollama.corp:443" + assert (loaded["ollama_host"], loaded["endpoint"]) == (expected, expected) + assert ("s3cret" in config_file.read_text()) is plaintext_opt_in + + +class TestAiTestAcceptsAnyOllamaHost: + @pytest.mark.parametrize("endpoint", [_REMOTE, "https://ollama.example.com"]) + def test_a_remote_ollama_endpoint_is_accepted(self, endpoint): + from fluid_build.cli._ai_setup_test import _validate_ai_test_endpoint + + assert _validate_ai_test_endpoint("ollama", endpoint) is None + + @pytest.mark.parametrize( + "endpoint", + ["http://u:p@gpu-box.lan:11434", "gpu-box.lan:11434", "ftp://gpu-box.lan"], + ) + def test_the_other_endpoint_rules_still_hold(self, endpoint): + from fluid_build.cli._ai_setup_test import _validate_ai_test_endpoint + + assert _validate_ai_test_endpoint("ollama", endpoint) is not None + + +def test_an_upper_case_scheme_is_read_as_lower_case(): + assert ollama_base_url({}, "HTTP://gpu-box.lan") == "http://gpu-box.lan:80" + + +def test_redaction_ignores_the_scheme_case(): + from fluid_build.llm.providers import redact_endpoint + + assert redact_endpoint("HTTP://tok@gpu-box.lan:1") == "HTTP://***:***@gpu-box.lan:1" + + +class TestTieredStagesCallTheStatedEndpoint: + def _resolve(self, config): + from fluid_build.copilot.agents.base import BaseStageAgent, StageSession + + session = StageSession(store=mock.MagicMock(), llm_config=config, tiered=True) + return BaseStageAgent(stage="s", tier="deep").resolve_model(session) + + def test_a_stated_endpoint_is_kept(self, monkeypatch): + monkeypatch.setenv("OLLAMA_HOST", _LOCAL) + config = LlmConfig( + provider="ollama", model="m", endpoint=_REMOTE, api_key=None, endpoint_explicit=True + ) + assert self._resolve(config).endpoint == _REMOTE + + def test_an_unstated_endpoint_is_derived_again(self, monkeypatch): + monkeypatch.setenv("OLLAMA_HOST", "gpu-box.lan") + config = LlmConfig(provider="ollama", model="m", endpoint="", api_key=None) + assert self._resolve(config).endpoint == _REMOTE + + +def test_a_two_argument_plugin_still_preflights_without_an_endpoint(monkeypatch): + # The ``endpoint`` keyword goes only with a stated endpoint. + from fluid_build.llm.litellm_backend import LiteLLMProvider + + class _OldPlugin(LiteLLMProvider): + def list_available_models(self, api_key, env): # the pre-endpoint signature + return ["gpt-x"] + + monkeypatch.setattr(providers, "get_llm_provider", lambda name: _OldPlugin("openai")) + env = {"OPENAI_API_KEY": "k", "FLUID_LLM_MODEL_PREFLIGHT": "1"} # pragma: allowlist secret + config = resolve_llm_config(_args(llm_provider="openai", llm_model="gpt-x"), env) + assert config.model == "gpt-x" + + +class TestSavedAddressesHoldNoCredentials: + @pytest.mark.parametrize( + "url, saved", + [ + ("http://u:p@host:9", "http://host:9"), # pragma: allowlist secret + ("u:p@host:9", "host:9"), # pragma: allowlist secret + ("https://u:p@[::1]:9/x", "https://[::1]:9/x"), # pragma: allowlist secret + ("http://u:p@h?x=1", "http://h?x=1"), # pragma: allowlist secret + ("http://h?x=1@y", "http://h?x=1@y"), # an "@" in the query is no userinfo + ("http://host/path@x", "http://host/path@x"), # nor one in the path + ("http://h#frag@x", "http://h#frag@x"), + ("", ""), + ], + ) + def test_the_address_is_saved_without_them(self, monkeypatch, caplog, url, saved): + from fluid_build.cli._ai_setup_storage import _without_credentials + + monkeypatch.delenv("FLUID_ALLOW_PLAINTEXT_AI_SECRETS", raising=False) + with caplog.at_level(logging.WARNING): + assert _without_credentials(url) == saved + assert all(":p@" not in record.getMessage() for record in caplog.records) + + def test_inline_setup_keeps_the_credentials_in_ollama_host(self, tmp_path, monkeypatch): + # ai_config.json holds the address without them; the environment keeps them. + from fluid_build.cli.ai_setup import _save_ai_config, run_ai_setup_inline + + monkeypatch.delenv("FLUID_ALLOW_PLAINTEXT_AI_SECRETS", raising=False) + secret_host = "http://alice:s3cret@127.0.0.1:9" # pragma: allowlist secret + with ( + mock.patch("fluid_build.cli.ai_setup._CONFIG_FILE", tmp_path / "ai_config.json"), + mock.patch("fluid_build.cli.ai_setup._CONFIG_DIR", tmp_path), + ): + _save_ai_config("ollama", "m", ollama_host=secret_host) + monkeypatch.setenv("OLLAMA_HOST", secret_host) + assert run_ai_setup_inline(None).provider == "ollama" + assert providers.os.environ["OLLAMA_HOST"] == secret_host + + monkeypatch.delenv("OLLAMA_HOST") + run_ai_setup_inline(None) + assert providers.os.environ["OLLAMA_HOST"] == "http://127.0.0.1:9" + + +@pytest.mark.parametrize("failure", ["redirect", "not json"]) +def test_the_model_list_is_unavailable_on_a_redirect_or_a_bad_body(failure): + import httpx + + from fluid_build.llm.litellm_backend import _list_ollama_models + + response = mock.MagicMock(status_code=302 if failure == "redirect" else 200) + if failure == "redirect": + response.raise_for_status.side_effect = httpx.HTTPStatusError( + "302", request=mock.MagicMock(), response=response + ) + else: + response.json.side_effect = ValueError("not json") + with mock.patch("httpx.get", return_value=response): + assert _list_ollama_models({}, _REMOTE) is None From cbf1fc61777004d30495683da7cd4002db7b7736 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 08:33:40 +0100 Subject: [PATCH 3/4] fix(ai): stop with copilot_no_llm_provider instead of guessing gemini With nothing selecting a provider, forge used gemini. It sent FLUID_LLM_API_KEY to Google whatever key it was, and with two provider keys set it asked for a gemini key, while fluid doctor said "Not selected". Owner decision: no silent fallback. resolve_llm_config now raises copilot_no_llm_provider before any provider is built, with no_provider_message(env), shared with check_llm_readiness so forge and doctor say the same thing: nothing configured, several keys set, a key that names no provider, or a saved fluid ai setup choice the command does not read. The error catalog carries the suggestions. --- .secrets.baseline | 10 +- CHANGELOG.md | 6 ++ fluid_build/_error_catalog.py | 9 ++ fluid_build/llm/provider_resolve.py | 73 ++++++++++++--- fluid_build/llm/providers.py | 31 ++++--- tests/test_ai_setup.py | 31 ++++--- tests/test_llm_keyless_autopick.py | 35 +++---- tests/test_llm_no_provider.py | 139 ++++++++++++++++++++++++++++ 8 files changed, 273 insertions(+), 61 deletions(-) create mode 100644 tests/test_llm_no_provider.py diff --git a/.secrets.baseline b/.secrets.baseline index 358260d5..dc6b206c 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -270,7 +270,7 @@ "filename": "fluid_build/cli/ai_setup.py", "hashed_secret": "fedd98aeb509344663485744497d778515a12158", "is_verified": false, - "line_number": 160 + "line_number": 154 } ], "fluid_build/cli/ai_source_setup.py": [ @@ -1537,21 +1537,21 @@ "filename": "tests/test_ai_setup.py", "hashed_secret": "06c3796a342a1f1172aa7a907d3c0a5da02a053a", "is_verified": false, - "line_number": 1098 + "line_number": 1101 }, { "type": "Secret Keyword", "filename": "tests/test_ai_setup.py", "hashed_secret": "6b1196369d5cd92be55dbdd72ef619957b6e6624", "is_verified": false, - "line_number": 1102 + "line_number": 1105 }, { "type": "Secret Keyword", "filename": "tests/test_ai_setup.py", "hashed_secret": "9ea9d5acda505555f09a5d2203f7078f1a3b6363", "is_verified": false, - "line_number": 1107 + "line_number": 1110 } ], "tests/test_ai_setup_import_keys.py": [ @@ -2317,5 +2317,5 @@ } ] }, - "generated_at": "2026-11-11T22:30:08Z" + "generated_at": "2026-11-12T07:03:08Z" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cb7c787..066646dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `http://gpu-box:11434`) and keeps a `user:password@`, sent as basic auth. `fluid doctor` and `fluid ai setup` show it masked, and `~/.fluid/ai_config.json` stores a saved address without it unless `FLUID_ALLOW_PLAINTEXT_AI_SECRETS` is set, as for API keys. +- **With no provider selected, forge stops instead of using gemini.** It used to send + `FLUID_LLM_API_KEY` to Google whatever key it was, and with two provider keys set it asked for + a gemini key. It now fails with `copilot_no_llm_provider` and says why (nothing configured, + several keys, a key that names no provider, or a `fluid ai setup` choice that the command does + not read), in the words `fluid doctor` shows. Pass `--llm-provider`, or set + `FLUID_LLM_PROVIDER` or the key of exactly one provider. - **`LlmProvider.list_available_models` takes an `endpoint` keyword**, the address the call will go to, passed only when the operator states one. A provider plugin that overrides the method with the two-argument signature keeps working until an endpoint is stated. diff --git a/fluid_build/_error_catalog.py b/fluid_build/_error_catalog.py index 005c75b3..a9db72ed 100644 --- a/fluid_build/_error_catalog.py +++ b/fluid_build/_error_catalog.py @@ -338,6 +338,15 @@ def slug_for(event: str) -> str: ], "secrets", ), + "copilot_no_llm_provider": ( + [ + "Pass --llm-provider, for example anthropic, openai, gemini or ollama", + "Or set FLUID_LLM_PROVIDER, or the API key of exactly one provider", + "Or run 'fluid ai setup' to choose and save a provider", + "Run 'fluid doctor' to see what forge would select", + ], + None, + ), # Optional-dependency gate (fluid_build/_optional.py). The message already # carries the exact `pip install` line; these only give the stable slug a # curated follow-up and a docs anchor. diff --git a/fluid_build/llm/provider_resolve.py b/fluid_build/llm/provider_resolve.py index 04691d7d..d7c43e65 100644 --- a/fluid_build/llm/provider_resolve.py +++ b/fluid_build/llm/provider_resolve.py @@ -30,7 +30,7 @@ import logging import shutil import sys -from typing import Mapping, Optional +from typing import Dict, List, Mapping, Optional, Tuple LOG = logging.getLogger("fluid.cli.llm_providers.resolve") @@ -65,6 +65,63 @@ def _bind_constants_from_host() -> None: _bind_constants_from_host() +# Each built-in provider and the explicit variables that name it. Exactly one +# provider named this way selects it; several select none, and +# :func:`no_provider_message` says so. +_EXPLICIT_KEY_VARS: Tuple[Tuple[str, Tuple[str, ...]], ...] = ( + ("openai", ("OPENAI_API_KEY",)), + ("anthropic", ("ANTHROPIC_API_KEY",)), + ("gemini", ("GEMINI_API_KEY", "GOOGLE_API_KEY")), + ("ollama", ("OLLAMA_HOST",)), +) + + +def _explicit_key_signals(env: Mapping[str, str]) -> Dict[str, List[str]]: + """Each provider an explicit variable names, with the names of the ones set.""" + signals: Dict[str, List[str]] = {} + for provider, names in _EXPLICIT_KEY_VARS: + present = [name for name in names if env.get(name)] + if present: + signals[provider] = present + return signals + + +def _saved_provider_name() -> Optional[str]: + """The provider ``fluid ai setup`` saved, if any (``~/.fluid``).""" + try: + from fluid_build._ai_config_shared import load_ai_config + + name = (load_ai_config() or {}).get("provider") + except Exception: # noqa: BLE001 — an unreadable config names nothing + return None + return name if isinstance(name, str) and name else None + + +def no_provider_message(env: Mapping[str, str]) -> str: + """Why no LLM provider is selected. ``fluid doctor`` and forge both say this.""" + saved = _saved_provider_name() + if saved: + # Only a command that does not read the saved choice gets here: the + # readiness ladder (``fluid doctor``) uses it. + return ( + f"No LLM provider is selected for this run: 'fluid ai setup' saved {saved}, " + f"but this command does not read the saved choice. Pass --llm-provider {saved}." + ) + signals = _explicit_key_signals(env) + if len(signals) > 1: + names = ", ".join(name for present in signals.values() for name in present) + return ( + f"No LLM provider is selected: {names} are all set, so none of them is " + "chosen. Pick one with --llm-provider or FLUID_LLM_PROVIDER." + ) + if env.get("FLUID_LLM_API_KEY"): + return ( + "No LLM provider is selected: FLUID_LLM_API_KEY does not say which provider " + "the key is for. Pick one with --llm-provider or FLUID_LLM_PROVIDER." + ) + return "No LLM provider configured. Run 'fluid ai setup' or set an API key env var." + + def _infer_provider_from_explicit_keys(env: Mapping[str, str]) -> Optional[str]: """Return the provider when the operator supplied exactly one explicit API-key env var. @@ -79,17 +136,9 @@ def _infer_provider_from_explicit_keys(env: Mapping[str, str]) -> Optional[str]: which only fires once every other resolution step has failed. """ - detected = [] - if env.get("OPENAI_API_KEY"): - detected.append("openai") - if env.get("ANTHROPIC_API_KEY"): - detected.append("anthropic") - if env.get("GEMINI_API_KEY") or env.get("GOOGLE_API_KEY"): - detected.append("gemini") - if env.get("OLLAMA_HOST"): - detected.append("ollama") - if len(detected) == 1: - return detected[0] + signals = _explicit_key_signals(env) + if len(signals) == 1: + return next(iter(signals)) return None diff --git a/fluid_build/llm/providers.py b/fluid_build/llm/providers.py index 2d5e8f1d..fccf1617 100644 --- a/fluid_build/llm/providers.py +++ b/fluid_build/llm/providers.py @@ -796,16 +796,17 @@ def has_llm_api_key( _NON_LITELLM_PROVIDERS: frozenset = _KEYLESS_PROVIDERS - {"ollama"} -def _provider_name_from(args: Any, env: Mapping[str, str]) -> str: - """The provider name the flags and environment select (flag, env, key sniff, default).""" - return ( - _explicit_provider_name(args, env) - or _infer_provider_from_env( - env, - non_interactive=bool(getattr(args, "non_interactive", False)), - allow_agents=_agents_allowed(args, env), - ) - or "gemini" +def _provider_name_from(args: Any, env: Mapping[str, str]) -> Optional[str]: + """The provider the flags and environment select (flag, env, key sniff, ambient). + + ``None`` when nothing selects one. There is no default provider: guessing + one would send ``FLUID_LLM_API_KEY`` to a vendor it may not belong to, and + ask for that vendor's key when other keys are set. + """ + return _explicit_provider_name(args, env) or _infer_provider_from_env( + env, + non_interactive=bool(getattr(args, "non_interactive", False)), + allow_agents=_agents_allowed(args, env), ) @@ -875,8 +876,8 @@ def preflight_llm_provider(args: Any, environ: Optional[Mapping[str, str]] = Non agent_loop = agent_loop_requested(getattr(args, "agent_loop", False), env) if not _explicit_provider_name(args, env): # Nothing chosen: inference reads the keyring and probes Ollama, and every - # provider it can pick (or the gemini default) needs litellm, except the - # coding agent a non-interactive run auto-picks. Without litellm, that + # provider it can pick needs litellm, except the coding agent a + # non-interactive run auto-picks. Without litellm, that # agent is the only provider that can run; otherwise fail first. from fluid_build._optional import is_available @@ -912,6 +913,9 @@ def resolve_llm_config(args: Any, environ: Optional[Mapping[str, str]] = None) - """Resolve provider, model, endpoint, and API key from flags and env vars.""" env = dict(environ or os.environ) provider_name = _provider_name_from(args, env) + if provider_name is None: + # The suggestions come from the error catalog (``_error_catalog``). + raise CopilotGenerationError("copilot_no_llm_provider", no_provider_message(env)) provider = get_llm_provider(provider_name) # A stated endpoint is where the call goes, so the Ollama model pick and the @@ -1500,6 +1504,7 @@ def call_llm_streaming( _lean_keyless_autopick, _resolve_api_key, clear_api_key_from_keyring, + no_provider_message, save_api_key_to_keyring, ) @@ -1648,7 +1653,7 @@ def check_llm_readiness(environ: Optional[Mapping[str, str]] = None) -> LlmReadi LOG.debug("LLM readiness: no provider detected from env or config") return LlmReadinessCheck( ready=False, - error="No LLM provider configured. Run 'fluid ai setup' or set an API key env var.", + error=no_provider_message(env), ) provider = BUILTIN_LLM_PROVIDERS.get(provider_name) diff --git a/tests/test_ai_setup.py b/tests/test_ai_setup.py index 0f775361..cfa14f1e 100644 --- a/tests/test_ai_setup.py +++ b/tests/test_ai_setup.py @@ -979,7 +979,9 @@ class TestGeminiIsDefaultProvider: 2. ``_prompt_for_api_key`` numbered menu — Gemini's free path is the first option and ``default=1`` selects it. 3. ``resolve_llm_config`` resolution chain — when no flag, env var, or - env-var-inferred provider is present, the fallback is ``"gemini"``. + env-var-inferred provider is present, there is no silent fallback to + any provider: forge stops with ``copilot_no_llm_provider`` (founder + decision, 2026-11-12). The card was filed when OpenAI was the default everywhere; this suite prevents accidental regression to that state. @@ -1021,11 +1023,11 @@ def test_prompt_for_api_key_lists_gemini_first_with_default_one(self): connect_snippet = src[connect_idx : connect_idx + 400] assert "default=1" in connect_snippet, "ask_numbered_choice default= regressed" - def test_resolve_llm_config_falls_back_to_gemini_when_unconfigured(self, monkeypatch): + def test_resolve_llm_config_stops_when_no_provider_is_selected(self, monkeypatch): """With no flags, no FLUID_LLM_PROVIDER, no inferable provider env, and no API key already saved in the keyring, resolve_llm_config - should pick gemini and surface the missing-API-key error rather - than silently dropping to OpenAI.""" + stops with ``copilot_no_llm_provider``: it neither guesses gemini + nor drops to OpenAI.""" from fluid_build.cli.forge_copilot_llm_providers import ( CopilotGenerationError, resolve_llm_config, @@ -1065,24 +1067,25 @@ def test_resolve_llm_config_falls_back_to_gemini_when_unconfigured(self, monkeyp "fluid_build.cli.forge_copilot_llm_providers._resolve_api_key", return_value=None, ), + # No saved ``fluid ai setup`` choice either (never the real ~/.fluid). + patch("fluid_build._ai_config_shared.load_ai_config", return_value=None), ): with pytest.raises(CopilotGenerationError) as exc_info: resolve_llm_config(args, environ={}) - # The error message — not just the event slug — must specifically - # mention the gemini adapter, proving the fallback resolved to - # gemini before the API-key check fired. ``CopilotGenerationError`` - # carries the user-facing detail string in ``message``. + # No provider was guessed: the error says so, names no vendor, and + # carries the catalog's how-to-pick suggestions. + assert exc_info.value.event == "copilot_no_llm_provider" message = getattr(exc_info.value, "message", "") or "" - assert "gemini" in message.lower(), message + assert "no llm provider" in message.lower(), message + assert "gemini" not in message.lower(), message assert "openai" not in message.lower(), message + assert any("--llm-provider" in item for item in exc_info.value.suggestions) def test_resolve_llm_config_explicit_keys_still_win_over_gemini_default(self, monkeypatch): - """The Gemini-first fallback is the *no-signal* default. Operators - who set OPENAI_API_KEY / ANTHROPIC_API_KEY / explicit - ``--llm-provider openai`` must still get the provider they asked - for. Otherwise the fallback would be a regression for everyone - who explicitly opts into a different provider.""" + """Operators who set OPENAI_API_KEY / ANTHROPIC_API_KEY / explicit + ``--llm-provider openai`` get the provider they asked for; Gemini + is only the first choice the interactive setup offers.""" from fluid_build.cli.forge_copilot_llm_providers import _infer_provider_from_env for key in ( diff --git a/tests/test_llm_keyless_autopick.py b/tests/test_llm_keyless_autopick.py index 4e7252c5..ef9c3b26 100644 --- a/tests/test_llm_keyless_autopick.py +++ b/tests/test_llm_keyless_autopick.py @@ -4,8 +4,9 @@ """Keyless auto-pick: a non-interactive run with no provider configured. Order: a running local Ollama (data stays local), then the first coding-agent CLI -on PATH in ``CODING_AGENT_NAMES`` order, then the hardcoded ``gemini`` last -resort. Interactive runs keep "offer, don't force" (the ``fluid ai setup`` +on PATH in ``CODING_AGENT_NAMES`` order, and otherwise no provider at all: +forge then stops with ``copilot_no_llm_provider``, the same "Not selected" +``fluid doctor`` shows. Interactive runs keep "offer, don't force" (the ``fluid ai setup`` inline offer). ``check_llm_readiness`` and the real selection share one helper, so ``fluid doctor`` never disagrees with what forge then does. @@ -115,8 +116,8 @@ def test_running_ollama_beats_an_agent(self, world): world.monkeypatch.setattr(providers, "detect_ollama_available", lambda env: True) assert _provider_name_from(_args(), dict(world.env)) == "ollama" - def test_nothing_detected_keeps_the_gemini_last_resort(self, world): - assert _provider_name_from(_args(), dict(world.env)) == "gemini" + def test_nothing_detected_selects_no_provider(self, world): + assert _provider_name_from(_args(), dict(world.env)) is None def test_non_interactive_flag_overrides_a_tty(self, world): world.install("claude-code") @@ -134,7 +135,7 @@ class TestInteractiveKeepsOfferDontForce: def test_tty_does_not_auto_pick_an_agent(self, world): world.install("claude-code") _tty(world, True) - assert _provider_name_from(_args(), dict(world.env)) == "gemini" + assert _provider_name_from(_args(), dict(world.env)) is None def test_tty_still_picks_a_running_ollama(self, world): world.install("claude-code") @@ -168,10 +169,10 @@ def test_env_selector_wins(self, world, var): assert _provider_name_from(_args(), {**world.env, var: "codex"}) == "codex" def test_two_explicit_keys_are_still_configured(self, world): - # Ambiguous keys fall through to the default, never to an agent. + # Ambiguous keys select no provider, and never an agent. world.install("claude-code") env = {**world.env, "OPENAI_API_KEY": "k", "ANTHROPIC_API_KEY": "k"} - assert _provider_name_from(_args(), env) == "gemini" + assert _provider_name_from(_args(), env) is None def test_stated_ollama_host_is_never_swapped_for_an_agent(self, world): # Selection treats OLLAMA_HOST as an explicit Ollama choice; the agent @@ -183,7 +184,7 @@ def test_stated_ollama_host_is_never_swapped_for_an_agent(self, world): def test_generic_api_key_is_configured(self, world): world.install("claude-code") - assert _provider_name_from(_args(), {**world.env, "FLUID_LLM_API_KEY": "k"}) == "gemini" + assert _provider_name_from(_args(), {**world.env, "FLUID_LLM_API_KEY": "k"}) is None def test_keyring_key_wins(self, world): world.install("claude-code") @@ -195,7 +196,7 @@ def test_keyring_key_wins(self, world): def test_ambiguous_keyring_is_still_configured(self, world): world.install("claude-code") world.monkeypatch.setattr(providers, "_get_api_key_from_keyring", lambda n: "k") - assert _provider_name_from(_args(), dict(world.env)) == "gemini" + assert _provider_name_from(_args(), dict(world.env)) is None def test_saved_config_is_still_configured(self, world): world.install("claude-code") @@ -203,7 +204,7 @@ def test_saved_config_is_still_configured(self, world): "fluid_build._ai_config_shared.load_ai_config", lambda *a, **k: {"provider": "openai", "model": "gpt-4.1-mini"}, ) - assert _provider_name_from(_args(), dict(world.env)) == "gemini" + assert _provider_name_from(_args(), dict(world.env)) is None class TestSaysWhichWasPicked: @@ -278,12 +279,12 @@ def test_llm_model_flag(self, world): assert _provider_name_from(_args(llm_model="gpt-4o"), dict(world.env)) != "claude-code" def test_resolve_keeps_the_stated_endpoint_off_the_agent(self, world): - # The security repro: the agent is not picked, so with no key for the - # default provider the run stops before any call leaves the machine. + # The security repro: the agent is not picked, so with no provider + # selected the run stops before any call leaves the machine. world.install("claude-code") with pytest.raises(providers.CopilotGenerationError) as info: resolve_llm_config(_args(llm_endpoint=self._ONPREM, llm_model="my-model"), {}) - assert "copilot_missing_llm_api_key" in repr(vars(info.value)) + str(info.value) + assert info.value.event == "copilot_no_llm_provider" @pytest.mark.parametrize( "var, value", @@ -331,18 +332,18 @@ def test_the_options_carry_the_env_route(self, world, value): world.monkeypatch.setenv("FLUID_COPILOT_AGENT_LOOP", value) assert self._options().agent_loop is True - def test_flag_route_ends_at_the_missing_key_error(self, world): + def test_flag_route_ends_at_the_no_provider_error(self, world): world.install("claude-code") with pytest.raises(providers.CopilotGenerationError) as info: resolve_llm_config(self._options(agent_loop=True), {}) - assert info.value.event == "copilot_missing_llm_api_key" + assert info.value.event == "copilot_no_llm_provider" - def test_env_route_ends_at_the_missing_key_error(self, world): + def test_env_route_ends_at_the_no_provider_error(self, world): world.install("claude-code") world.monkeypatch.setenv("FLUID_COPILOT_AGENT_LOOP", "1") with pytest.raises(providers.CopilotGenerationError) as info: resolve_llm_config(self._options(), {}) - assert info.value.event == "copilot_missing_llm_api_key" + assert info.value.event == "copilot_no_llm_provider" def test_env_route_blocks_the_pick_in_selection(self, world): world.install("claude-code") diff --git a/tests/test_llm_no_provider.py b/tests/test_llm_no_provider.py new file mode 100644 index 00000000..c9779ee0 --- /dev/null +++ b/tests/test_llm_no_provider.py @@ -0,0 +1,139 @@ +# Copyright 2025 Agentics Transformation Limited +# SPDX-License-Identifier: Apache-2.0 + +"""No provider selected: forge stops and says why, the same words as ``fluid doctor``. + +Founder decision (2026-11-12): there is no silent fallback provider. The old +``gemini`` default sent ``FLUID_LLM_API_KEY`` to Google whatever key it was, and +with two keys set it asked for a third, gemini's, while ``fluid doctor`` said +"Not selected". Now forge raises ``copilot_no_llm_provider`` before any +provider is built, and readiness reports the same message. + +Hermetic: no keyring, no saved ``~/.fluid`` config, no Ollama, no agent on PATH. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest + +from fluid_build._error_catalog import suggestions_for +from fluid_build.llm import providers +from fluid_build.llm.provider_resolve import _infer_provider_from_explicit_keys +from fluid_build.llm.providers import ( + CopilotGenerationError, + check_llm_readiness, + no_provider_message, + resolve_llm_config, +) + +pytestmark = pytest.mark.unit + +_PLAIN = "No LLM provider configured. Run 'fluid ai setup' or set an API key env var." + + +@pytest.fixture(autouse=True) +def _nothing_configured(monkeypatch): + for var in ( + "FLUID_LLM_PROVIDER", + "FLUID_LLM_API_KEY", + "FLUID_LLM_MODEL", + "FLUID_LLM_ENDPOINT", + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "GEMINI_API_KEY", + "GOOGLE_API_KEY", + "OLLAMA_HOST", + "FLUID_COPILOT_AGENT_LOOP", + ): + monkeypatch.delenv(var, raising=False) + monkeypatch.setattr(providers, "_get_api_key_from_keyring", lambda name: None) + monkeypatch.setattr(providers, "detect_ollama_available", lambda env: False) + monkeypatch.setattr( + "fluid_build.llm.provider_resolve._first_coding_agent_on_path", lambda env: None + ) + monkeypatch.setattr("fluid_build._ai_config_shared.load_ai_config", lambda *a, **k: None) + + +def _args(): + return SimpleNamespace(llm_provider=None, llm_model=None, llm_endpoint=None) + + +# The env always holds something, so ``resolve_llm_config`` never swaps an +# empty mapping for ``os.environ``. +_NOTHING = {"PATH": "/nonexistent"} +_TWO_KEYS = { + **_NOTHING, + "OPENAI_API_KEY": "k1", # pragma: allowlist secret + "ANTHROPIC_API_KEY": "k2", # pragma: allowlist secret +} +_GENERIC_KEY = {**_NOTHING, "FLUID_LLM_API_KEY": "sk-x"} # pragma: allowlist secret + + +class TestForgeStops: + def test_nothing_configured(self): + with pytest.raises(CopilotGenerationError) as info: + resolve_llm_config(_args(), _NOTHING) + assert info.value.event == "copilot_no_llm_provider" + assert info.value.message == _PLAIN + assert any("--llm-provider" in item for item in info.value.suggestions) + + def test_two_keys_are_named_instead_of_asking_for_a_third(self): + with pytest.raises(CopilotGenerationError) as info: + resolve_llm_config(_args(), _TWO_KEYS) + assert info.value.event == "copilot_no_llm_provider" + assert "OPENAI_API_KEY, ANTHROPIC_API_KEY" in info.value.message + assert "gemini" not in info.value.message.lower() + + def test_a_generic_key_is_never_sent_to_a_guessed_vendor(self, monkeypatch): + # Stopping happens before any provider is built, so nothing can dial. + monkeypatch.setattr( + providers, "get_llm_provider", lambda name: pytest.fail(f"built {name}") + ) + with pytest.raises(CopilotGenerationError) as info: + resolve_llm_config(_args(), _GENERIC_KEY) + assert info.value.event == "copilot_no_llm_provider" + assert "FLUID_LLM_API_KEY" in info.value.message + + def test_a_named_provider_still_resolves(self): + config = resolve_llm_config( + SimpleNamespace(llm_provider="anthropic", llm_model=None, llm_endpoint=None), + {**_TWO_KEYS}, + ) + assert config.provider == "anthropic" + + +class TestDoctorSaysTheSame: + @pytest.mark.parametrize("env", [_NOTHING, _TWO_KEYS, _GENERIC_KEY]) + def test_readiness_error_is_the_forge_message(self, env): + readiness = check_llm_readiness(env) + assert readiness.provider is None + assert readiness.error == no_provider_message(env) + + def test_readiness_names_the_two_keys(self): + assert "OPENAI_API_KEY, ANTHROPIC_API_KEY" in check_llm_readiness(_TWO_KEYS).error + + +def test_gemini_and_google_keys_are_one_provider(): + env = {"GEMINI_API_KEY": "k", "GOOGLE_API_KEY": "k"} + assert _infer_provider_from_explicit_keys(env) == "gemini" + assert no_provider_message({**env, "OPENAI_API_KEY": "k"}).startswith( + "No LLM provider is selected: OPENAI_API_KEY, GEMINI_API_KEY, GOOGLE_API_KEY" + ) + + +def test_the_error_has_catalog_suggestions(): + assert suggestions_for("copilot_no_llm_provider") + + +def test_a_saved_setup_choice_is_named_instead_of_asking_to_run_setup(monkeypatch): + # Commands that do not read the saved choice (non-interactive forge) say so. + monkeypatch.setattr( + "fluid_build._ai_config_shared.load_ai_config", lambda *a, **k: {"provider": "anthropic"} + ) + with pytest.raises(CopilotGenerationError) as info: + resolve_llm_config(_args(), _NOTHING) + assert info.value.event == "copilot_no_llm_provider" + assert "'fluid ai setup' saved anthropic" in info.value.message + assert "--llm-provider anthropic" in info.value.message From 3166bde06b21af2f7da2840a4d68f303253932db Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Thu, 12 Nov 2026 09:02:27 +0100 Subject: [PATCH 4/4] test(ai): reset the setup-skip flag in the inline-setup credentials test Coverage & Audit (Python 3.14) failed on test_inline_setup_keeps_the_credentials_in_ollama_host with "'NoneType' object has no attribute 'provider'": run_ai_setup_inline returns None when an earlier test on the same worker left the module-level _ai_setup_skipped set, and random test order put one first. The test now resets the flag, as the other inline-setup tests do, points HOME at its tmp dir and pins the litellm check, so it reads nothing of the machine it runs on. --- tests/test_ollama_host.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_ollama_host.py b/tests/test_ollama_host.py index 497d20b7..9848f425 100644 --- a/tests/test_ollama_host.py +++ b/tests/test_ollama_host.py @@ -306,8 +306,14 @@ def test_the_address_is_saved_without_them(self, monkeypatch, caplog, url, saved def test_inline_setup_keeps_the_credentials_in_ollama_host(self, tmp_path, monkeypatch): # ai_config.json holds the address without them; the environment keeps them. + from fluid_build.cli import ai_setup from fluid_build.cli.ai_setup import _save_ai_config, run_ai_setup_inline + # Hermetic: another test may have set the session-level skip, the real + # ~/.fluid must not be read, and the run must not depend on litellm. + monkeypatch.setattr(ai_setup, "_ai_setup_skipped", False) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setattr("fluid_build._optional.is_available", lambda name: True) monkeypatch.delenv("FLUID_ALLOW_PLAINTEXT_AI_SECRETS", raising=False) secret_host = "http://alice:s3cret@127.0.0.1:9" # pragma: allowlist secret with (