diff --git a/CHANGELOG.md b/CHANGELOG.md index 75dc9a85..8a826190 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **`fluid ai models --provider` works, and takes the names `fluid forge --llm-provider` + takes.** The flag stored its value where the global infrastructure `--provider` keeps its + own, so the global check read `gemini` as an infrastructure provider: every + `fluid ai models --provider ` exited 2 with "Unknown provider 'gemini' — installed + providers: aws, databricks, gcp, local, snowflake (see `fluid providers`)". The flag now has + its own slot, and its names come from the LLM provider registry: the built-ins, the keyless + `mcp-sampling` and coding agents (`claude-code`, `codex`, `cursor`, `kiro`), and installed + `fluid_build.llm_providers` plugins, each shown with the model plan forge would use. An + unknown name is refused with the built-in names, as `fluid forge --llm-provider` refuses it; + the flag used to list five hard-coded names. The docs noted the exit in 0.18.1; it was still + there in 0.29.0. +- **Receipt run ids no longer repeat in one process.** `generate_run_id`, the `run_id` of + `.fluid/init-receipt.json` and `forge-receipt.json`, paired a millisecond timestamp with 32 + random bits, so ids made in the same millisecond could collide by the birthday bound: 1000 ids + in a tight loop repeated one in roughly 1 run of 10,000, which made `test_run_ids_are_unique` + flaky on CI. The suffix is now monotonic, as in a ULID or a UUIDv7 (RFC 9562, section 6.2): a + random 31-bit start in each new millisecond, plus one for each later id in it. Ids from one + process never repeat and always increase, also when the clock steps back, and a forked child + starts its own sequence. The format is unchanged (`<11 hex digits>-<8 hex digits>`), and + nothing in forge parses it. + ## [0.29.0] — 2026-11-12 ### Added diff --git a/fluid_build/cli/ai_setup.py b/fluid_build/cli/ai_setup.py index 0e57f80d..1bd28f9f 100644 --- a/fluid_build/cli/ai_setup.py +++ b/fluid_build/cli/ai_setup.py @@ -1681,7 +1681,12 @@ def _provider_model_plan(provider_name: str) -> dict: get_catalog_tier_models, ) - provider = BUILTIN_LLM_PROVIDERS[provider_name] + provider = BUILTIN_LLM_PROVIDERS.get(provider_name) + if provider is None: + # ``mcp-sampling`` and plugin providers are not in the built-in dict. + from fluid_build.llm.providers import get_llm_provider + + provider = get_llm_provider(provider_name) model = get_catalog_default(provider_name) or provider.default_model routing_model = get_catalog_routing_model(provider_name, model) tier_models = get_catalog_tier_models(provider_name) @@ -1714,10 +1719,13 @@ def show_ai_models( if name in BUILTIN_LLM_PROVIDERS ] if provider_filter: + from fluid_build.cli._llm_provider_plugins import LlmProviderChoices + requested = provider_filter.strip().lower() if requested == "claude": requested = "anthropic" - provider_names = [name for name in provider_names if name == requested] + # Any name ``fluid forge --llm-provider`` takes, not only the four above. + provider_names = [requested] if requested in LlmProviderChoices() else [] plans = {name: _provider_model_plan(name) for name in provider_names} if as_json: @@ -1843,10 +1851,17 @@ def register(subparsers) -> None: "models", help="Show provider defaults, routing model, and tiered model plan", ) + # The registry ``fluid forge --llm-provider`` reads: built-ins (listed in + # --help and in an invalid-choice error) and pip-installed plugins. The + # dest keeps the value off ``args.provider``: the global checks read that + # as an infrastructure provider, which made every value here exit 2. + from fluid_build.cli._llm_provider_plugins import LlmProviderChoices + models_parser.add_argument( "--provider", - choices=["gemini", "openai", "anthropic", "claude", "ollama"], - help="Limit output to one provider", + dest="llm_provider", + choices=LlmProviderChoices(), + help="Limit output to one LLM provider (any `fluid forge --llm-provider` name)", ) models_parser.add_argument("--json", action="store_true", help="Print machine-readable JSON") test_parser = ai_sub.add_parser( @@ -2001,7 +2016,7 @@ def _run_ai_command(args, logger: logging.Logger) -> int: if action == "models": show_ai_models( console, - provider_filter=getattr(args, "provider", None), + provider_filter=getattr(args, "llm_provider", None), as_json=bool(getattr(args, "json", False)), ) return 0 diff --git a/fluid_build/cli/artifact_receipts.py b/fluid_build/cli/artifact_receipts.py index ed60676b..d6e37395 100644 --- a/fluid_build/cli/artifact_receipts.py +++ b/fluid_build/cli/artifact_receipts.py @@ -46,7 +46,9 @@ "generate_run_id", ] +import os import secrets +import threading import time from dataclasses import dataclass, field from pathlib import Path @@ -107,22 +109,66 @@ def to_payload(self) -> Dict[str, Any]: } -def generate_run_id() -> str: - """Return a compact, time-sortable, machine-unique run identifier. +#: The largest suffix 8 hex digits hold. +_SUFFIX_MAX = 0xFFFFFFFF + +# What the last run id used: its millisecond and its suffix. One process keeps +# one sequence; ``_reset_run_id_state`` starts a forked child on its own. +_run_id_lock = threading.Lock() +_last_ms = -1 +_last_suffix = 0 + + +def _reset_run_id_state() -> None: + """Forget the last run id. Runs in a forked child. + + A child that kept the parent's state would make the parent's next id in + the same millisecond. The new lock replaces one that another thread may + have held at the fork. + """ + global _run_id_lock, _last_ms, _last_suffix + _run_id_lock = threading.Lock() + _last_ms = -1 + _last_suffix = 0 + + +if hasattr(os, "register_at_fork"): # POSIX only + os.register_at_fork(after_in_child=_reset_run_id_state) - Format: ``-`` — e.g. - ``018f7c1d3a42-9a2f1d03``. Sortable lexicographically because the - timestamp comes first. Uses 4 bytes (8 hex chars) of randomness so - rapid-fire generation (e.g. tests in a tight loop) does not produce - collisions even when the ms-resolution timestamp is identical. - No dependencies on ``uuid`` / ``ulid`` so the module stays - stdlib-only. +def generate_run_id() -> str: + """Return a compact, time-sortable run identifier, unique in this process. + + Format: ``-``, e.g. + ``1a1d06ce9d5-3f9c0a17``. The timestamp comes first, so ids sort by time. + 11 hex digits last until about the year 2527. + + The suffix is monotonic, as in a ULID or a UUIDv7 (RFC 9562, section + 6.2): the first id in a millisecond starts it at a random 31-bit value, + and each later id in that millisecond adds one. So ids from one process + never repeat and always increase, also in a tight loop and when the + clock steps back: the last millisecond is kept until the clock passes + it. A random suffix alone repeated by the birthday bound (1000 ids in + one millisecond: about 1 in 10,000). Across processes the random start + makes a repeat unlikely. + + Stdlib only: ``uuid.uuid7`` needs Python 3.14 and is 36 characters long. """ - # Milliseconds since epoch, hex-encoded (fits in 11 chars until ~year 2527) - ts_hex = f"{int(time.time() * 1000):011x}" - rand_hex = secrets.token_hex(4) # 8 hex chars ~ 4B values - return f"{ts_hex}-{rand_hex}" + global _last_ms, _last_suffix + now_ms = int(time.time() * 1000) + with _run_id_lock: + if now_ms > _last_ms: + # A new millisecond. The top bit starts clear, so at least 2**31 + # ids fit in it. + _last_ms, _last_suffix = now_ms, secrets.randbits(31) + elif _last_suffix < _SUFFIX_MAX: + _last_suffix += 1 + else: + # The suffix is used up. Move to the next millisecond, as RFC 9562 + # allows, and start a new suffix. + _last_ms, _last_suffix = _last_ms + 1, secrets.randbits(31) + ms, suffix = _last_ms, _last_suffix + return f"{ms:011x}-{suffix:08x}" class ReceiptBuilder: diff --git a/tests/test_ai_models_provider.py b/tests/test_ai_models_provider.py new file mode 100644 index 00000000..6e512952 --- /dev/null +++ b/tests/test_ai_models_provider.py @@ -0,0 +1,131 @@ +# Copyright 2025 Agentics Transformation Limited +# SPDX-License-Identifier: Apache-2.0 + +"""``fluid ai models --provider`` takes the LLM provider registry. + +The flag wrote ``args.provider``, the slot of the global infrastructure +``--provider``. The global check then read every value as an infrastructure +provider, so ``fluid ai models --provider gemini`` exited 2 with "Unknown +provider 'gemini' — installed providers: aws, databricks, gcp, local, +snowflake". The flag's own ``choices`` were five hard-coded names, behind the +names ``fluid forge --llm-provider`` takes. +""" + +from __future__ import annotations + +import importlib.metadata as md +import json +import re +from typing import Any, Dict, Mapping + +import pytest + +from fluid_build.cli import _validate_global_args, build_parser, main +from fluid_build.cli._llm_provider_plugins import ( + BUILTIN_PROVIDER_CHOICES, + LLM_PROVIDER_GROUP, + reset_plugin_registry, +) +from fluid_build.cli.ai_setup import show_ai_models +from fluid_build.cli.forge_copilot_llm_providers import LlmProvider + + +class _PluginProvider(LlmProvider): + """A third-party provider, as a ``fluid_build.llm_providers`` entry point loads it.""" + + name = "acme-llm" + default_model = "acme-large" + + def default_endpoint(self, model: str, env: Mapping[str, str]) -> str: + return "https://acme.invalid/v1" + + def build_request(self, config, system_prompt, user_prompt): + return ({}, {}) + + def extract_text(self, response_json: Dict[str, Any]) -> str: + return "" + + +class _EntryPoint: + name = "acme-llm" + + def load(self) -> Any: + return _PluginProvider + + +@pytest.fixture(autouse=True) +def _isolate_plugin_registry(): + reset_plugin_registry() + yield + reset_plugin_registry() + + +@pytest.fixture +def acme_plugin(monkeypatch): + def entry_points(group=None, **_kw): + return [_EntryPoint()] if group == LLM_PROVIDER_GROUP else [] + + monkeypatch.setattr(md, "entry_points", entry_points) + + +@pytest.fixture +def parser(monkeypatch): + # The global --provider defaults to $FLUID_PROVIDER; keep the shell's value out. + monkeypatch.delenv("FLUID_PROVIDER", raising=False) + return build_parser() + + +class _Log: + def __init__(self): + self.errors = [] + + def log_safe(self, level, message, **_kw): + if level == "error": + self.errors.append(message) + + +def test_every_registry_name_stays_off_the_infrastructure_slot(parser): + for name in BUILTIN_PROVIDER_CHOICES: + args = parser.parse_args(["ai", "models", "--provider", name]) + assert args.llm_provider == name + assert getattr(args, "provider", None) is None + log = _Log() + _validate_global_args(args, log, known_providers=["aws", "gcp", "local"]) + assert log.errors == [] + + +def test_an_unknown_name_is_refused_with_exactly_the_registry(parser, capsys): + with pytest.raises(SystemExit) as exc: + parser.parse_args(["ai", "models", "--provider", "not-a-provider"]) + assert exc.value.code == 2 + # Python 3.12.8+ prints the names bare, older versions quote them. + listed = re.search(r"choose from (.+)\)", capsys.readouterr().err).group(1) + assert [name.strip("'\" ") for name in listed.split(",")] == list(BUILTIN_PROVIDER_CHOICES) + + +def test_the_command_prints_the_plan_it_was_asked_for(capsys, monkeypatch): + monkeypatch.delenv("FLUID_PROVIDER", raising=False) + assert main(["ai", "models", "--provider", "gemini", "--json"]) == 0 + assert list(json.loads(capsys.readouterr().out)) == ["gemini"] + + +@pytest.mark.parametrize("name", BUILTIN_PROVIDER_CHOICES) +def test_every_registry_name_has_a_plan(name, capsys): + show_ai_models(None, provider_filter=name, as_json=True) + plans = json.loads(capsys.readouterr().out) + shown = "anthropic" if name == "claude" else name + assert list(plans) == [shown] + assert plans[shown]["primary_model"] + + +def test_a_plugin_provider_is_accepted_and_shown(acme_plugin, capsys): + args = build_parser().parse_args(["ai", "models", "--provider", "acme-llm"]) + assert args.llm_provider == "acme-llm" + show_ai_models(None, provider_filter="acme-llm", as_json=True) + plans = json.loads(capsys.readouterr().out) + assert plans["acme-llm"]["primary_model"] == "acme-large" + + +def test_a_name_outside_the_registry_shows_nothing(capsys): + show_ai_models(None, provider_filter="not-a-provider", as_json=True) + assert json.loads(capsys.readouterr().out) == {} diff --git a/tests/test_artifact_receipts.py b/tests/test_artifact_receipts.py index a534544d..fe78a806 100644 --- a/tests/test_artifact_receipts.py +++ b/tests/test_artifact_receipts.py @@ -5,8 +5,16 @@ from __future__ import annotations +import os +import subprocess +import sys +import time from pathlib import Path +from types import SimpleNamespace +import pytest + +from fluid_build.cli import artifact_receipts from fluid_build.cli.artifact_receipts import ( ReceiptBuilder, ReceiptDocument, @@ -15,6 +23,26 @@ ) from fluid_build.cli.artifact_writer import ArtifactWriter +#: Run by the fork test in a fresh interpreter: an id in the parent, one in a +#: forked child, then the parent's next id, all in one millisecond. +_FORK_SCRIPT = """ +import os, types +from fluid_build.cli import artifact_receipts as ar +ar.time = types.SimpleNamespace(time=lambda: 1_800_000_000.0005) +seeds = iter([5, 9]) +ar.secrets = types.SimpleNamespace(randbits=lambda k: next(seeds)) +print(ar.generate_run_id(), flush=True) +read_end, write_end = os.pipe() +pid = os.fork() +if pid == 0: + os.write(write_end, ar.generate_run_id().encode()) + os._exit(0) +os.close(write_end) +os.waitpid(pid, 0) +print(os.read(read_end, 64).decode()) +print(ar.generate_run_id()) +""" + class TestGenerateRunId: def test_format_has_timestamp_and_random(self): @@ -29,6 +57,74 @@ def test_run_ids_are_unique(self): ids = {generate_run_id() for _ in range(1000)} assert len(ids) == 1000 + def test_ids_in_one_millisecond_never_repeat(self, monkeypatch): + """The collision behind the flake above (``999 == 1000``), made certain. + + With the clock stopped and the random source fixed, a random suffix + repeats at once. Uniqueness in one process must not rest on chance. + """ + frozen = time.time() + monkeypatch.setattr(artifact_receipts, "time", SimpleNamespace(time=lambda: frozen)) + monkeypatch.setattr( + artifact_receipts, + "secrets", + SimpleNamespace(token_hex=lambda n=32: "ab" * n, randbits=lambda k: 0x1234), + ) + ids = [generate_run_id() for _ in range(1000)] + assert len(set(ids)) == 1000 + assert ids == sorted(ids) + + +class TestRunIdSequence: + """Order and uniqueness at the edges: the clock, a used-up suffix, a fork.""" + + def test_ids_keep_increasing_when_the_clock_steps_back(self, monkeypatch): + start = time.time() + clock = iter([start, start - 1.0, start - 2.0]) + monkeypatch.setattr(artifact_receipts, "time", SimpleNamespace(time=lambda: next(clock))) + ids = [generate_run_id() for _ in range(3)] + assert len(set(ids)) == 3 + assert ids == sorted(ids) + + def test_a_used_up_suffix_moves_to_the_next_millisecond(self, monkeypatch): + ms = int(time.time() * 1000) + monkeypatch.setattr(artifact_receipts, "_last_ms", -1) + monkeypatch.setattr(artifact_receipts, "_last_suffix", 0) + # +0.5 ms so the float round trip cannot land on the millisecond before. + monkeypatch.setattr( + artifact_receipts, "time", SimpleNamespace(time=lambda: (ms + 0.5) / 1000) + ) + monkeypatch.setattr( + artifact_receipts, "secrets", SimpleNamespace(randbits=lambda k: 0xFFFFFFFF) + ) + assert generate_run_id() == f"{ms:011x}-ffffffff" + assert generate_run_id() == f"{ms + 1:011x}-ffffffff" + + @pytest.mark.skipif(not hasattr(os, "fork"), reason="os.fork is POSIX only") + def test_a_forked_child_starts_its_own_sequence(self): + """A real ``os.fork()``, in a fresh interpreter with no other threads. + + Without the at-fork reset, the child continues the parent's sequence + and makes the same id as the parent's next one (``...-00000006``). + """ + root = Path(artifact_receipts.__file__).resolve().parents[2] + paths = [str(root), os.environ.get("PYTHONPATH", "")] + env = {**os.environ, "PYTHONPATH": os.pathsep.join(p for p in paths if p)} + result = subprocess.run( + [sys.executable, "-c", _FORK_SCRIPT], + capture_output=True, + text=True, + timeout=120, + cwd=root, + env=env, + check=True, + ) + ms = 1_800_000_000_000 + parent_first, child, parent_next = result.stdout.split() + assert parent_first == f"{ms:011x}-00000005" + assert child == f"{ms:011x}-00000009" + assert parent_next == f"{ms:011x}-00000006" + class TestReceiptEntryToDict: def test_minimal_entry_drops_empty_fields(self): diff --git a/tests/test_cli_extension_commands.py b/tests/test_cli_extension_commands.py index b0d6b768..e584d5f5 100644 --- a/tests/test_cli_extension_commands.py +++ b/tests/test_cli_extension_commands.py @@ -20,7 +20,8 @@ def test_parser_exposes_ai_models_and_forge_tier_controls(): ai_args = parser.parse_args(["ai", "models", "--provider", "openai", "--json"]) assert ai_args.cmd == "ai" assert ai_args.ai_action == "models" - assert ai_args.provider == "openai" + # Not ``provider``: the global checks read that as an infrastructure provider. + assert ai_args.llm_provider == "openai" assert ai_args.json is True forge_args = parser.parse_args(