diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f8a0ff..ad45469 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to scolta-python are documented here. ## [Unreleased] +### Changed +- **Scolta ships with no AI provider selected, and `AutoProvisioner.ensure_ai_available()` no longer mints a trial (`src/scolta/config.py`, `src/scolta/health.py`, `src/scolta/ai/client.py`, `src/scolta/ai/service.py`, `src/scolta/ai/amazee/auto_provisioner.py`).** Two invariants, previously true in the PHP core and only half-true here. **No default provider:** `ScoltaConfig.ai_provider` defaulted to `"anthropic"`, `HealthChecker` coalesced an empty value back to `"anthropic"`, and `AiClient` defaulted its `provider` key the same way — so an install nobody had configured reported itself as an Anthropic install, and a key set in the environment before anyone chose a provider looked like a working one. The default is now `""`, nothing coalesces it, and `""` means AI is off: `AiServiceAdapter._get_client()` refuses to build a client and raises the `ApiKeyMissingException` the callers already degrade on (unexpanded query, no summary) rather than picking a vendor on the site's behalf; `AiClient` rejects an absent provider outright; health reports `ai_provider: ""` and a new `ai_provider_selected: False`, and `ai_usable` is false whatever else is present. Going-forward only — a provider already persisted by a site is read as-is and never rewritten. **No auto-enable:** `ensure_ai_available()` still constructed an `AmazeeTrialProvisioner` and minted a free trial whenever it found an empty credential store, so any request, cron or install path that called it enrolled a site that had never opted in. That branch is deleted. The method now matches the PHP core exactly: it self-heals credentials that are already stored but whose model names were never resolved, and does nothing else. A connection is established only by an explicit `AmazeeTrialProvisioner.provision()` call from an operator action. Covered by `tests/ai/amazee/test_manual_provider_and_opt_in.py`, whose Amazee transports fail the test if they are called at all. +- **Which action established an Amazee.ai connection is recorded when it happens (`src/scolta/ai/amazee/connection_source.py`, `storage.py`, `trial_provisioner.py`, `account_upgrader.py`).** Nothing recorded whether a stored token came from the demo or from an operator's own account — both write the same three fields through `ConfigStorage.store()` — so any surface naming one was guessing. `AmazeeConnectionSource` (`demo` / `account`) is now written by the class that establishes the connection, through the new `ProvenanceAwareConfigStorage` sub-interface. Kept as a sub-interface so every existing `ConfigStorage` implementation keeps working untouched and simply reports no provenance, which is the honest answer for credentials connected before this release. + +### Removed +- **`AutoProvisioner.ensure_ai_available()` no longer provisions anything, and its return value is always `False`.** It previously returned `True` when it had minted a fresh trial. There is no longer a success to report. The parameter list is unchanged, so callers keep compiling; a caller that branched on `True` should move that branch to its own explicit `provision()` call. Two tests that asserted the mint behaviour are replaced by tests asserting its absence. + ### Fixed - **Re-vendored the browser bundle (`src/scolta/assets/js/scolta.js`) from scolta-php: the AI summary's "Show more" control now follows the viewport width, and a summarize failure can no longer strand the loading skeleton** ([tag1consulting/scolta-php#269](https://github.com/tag1consulting/scolta-php/pull/269)). diff --git a/README.md b/README.md index af11985..09d2aa9 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,35 @@ binding. Platform integration for Django/Wagtail lives in the companion `scolta-django` package. +## Selecting an AI provider is always manual + +Scolta ships with **no AI provider selected**. `ai_provider` is empty until +somebody sets it, and while it is empty AI features are simply off: search +works, no provider is assumed, and Anthropic in particular is not silently +assumed. There is no default anywhere. + +A developer sets `ai_provider` in code or settings; in `scolta-django` an +operator picks one in the admin. Both are explicit acts. This is a +going-forward rule: a site that already persisted a provider keeps it, and +nothing rewrites an existing value. + +**Amazee.ai is never enabled on its own.** No credential is provisioned and no +outbound Amazee call is made on a request, cron, install or activation path for +a site that has not opted in. `AutoProvisioner.ensure_ai_available()` — whose +name predates the policy — establishes nothing: it only re-resolves gateway +model names against a key already on disk, which is reachable only for a site +that already connected. A connection is established solely by an explicit call +to `AmazeeTrialProvisioner.provision()` (the free demo, no email required) or +`AmazeeAccountUpgrader` (the email → verification code → region flow that +attaches an amazee.ai account). Amazee support is email-only, mirroring +amazee.ai's own `ai_provider_amazeeio` module; there is no paste-your-API-key +path. + +Which of those two established a connection is **recorded** at the time it +happens, through `ProvenanceAwareConfigStorage`, so a surface can report a demo +or an account from a stored fact instead of a guess. Credentials with no +recorded origin claim nothing. + ## Status Complete port of `scolta-php`, released as 1.0.x. See `CLAUDE.md` for the diff --git a/docs/CONFIG_REFERENCE.md b/docs/CONFIG_REFERENCE.md index ffa9ab4..0ea874e 100644 --- a/docs/CONFIG_REFERENCE.md +++ b/docs/CONFIG_REFERENCE.md @@ -9,7 +9,7 @@ out of sync with the code. | Property | Type | Default | Description | |---|---|---|---| -| `ai_provider` | string | `anthropic` | | +| `ai_provider` | string | `(empty)` | AI provider identifier (`anthropic`, `openai`). No default: empty means no provider has been selected and AI features are off. Selecting one is always explicit. | | `ai_api_key` | string | `(empty)` | | | `ai_model` | string | `claude-sonnet-4-5-20250929` | | | `ai_expansion_model` | string | `(empty)` | | diff --git a/src/scolta/ai/amazee/__init__.py b/src/scolta/ai/amazee/__init__.py index c4c7c6a..8e724cd 100644 --- a/src/scolta/ai/amazee/__init__.py +++ b/src/scolta/ai/amazee/__init__.py @@ -1,19 +1,23 @@ -"""Amazee.ai auto-provisioning subsystem (port of ``AiProvider\\Amazee``). +"""Amazee.ai managed-gateway subsystem (port of ``AiProvider\\Amazee``). -A managed LiteLLM gateway: provision a free trial (anonymous or by email), -resolve the best Claude models, and upgrade to a private key via an email-OTP -flow. The returned credentials configure the OpenAI-compatible AiClient path. +A managed LiteLLM gateway, connected only when an operator opts in. Two explicit +paths establish a connection: the free demo (anonymous, no email) and the +email-OTP account flow, which is also how an operator continues once the demo +credit runs out. Nothing here connects a site on its own — ``AutoProvisioner`` +only re-resolves model names against credentials that are already stored. The +returned credentials configure the OpenAI-compatible AiClient path. """ from .account_upgrader import AmazeeAccountUpgrader from .auto_provisioner import AutoProvisioner from .budget_decorator import BudgetAwareProviderDecorator from .client import AmazeeClient +from .connection_source import AmazeeConnectionSource from .exceptions import AmazeeApiException, AmazeeBudgetExceededException from .key_expiry_recovery import KeyExpiryRecovery from .model_resolver import AmazeeModelResolver from .results import ProvisioningResult, UpgradeResult -from .storage import ConfigStorage +from .storage import ConfigStorage, ProvenanceAwareConfigStorage from .trial_provisioner import AmazeeTrialProvisioner __all__ = [ @@ -21,12 +25,14 @@ "AmazeeApiException", "AmazeeBudgetExceededException", "AmazeeClient", + "AmazeeConnectionSource", "AmazeeModelResolver", "AmazeeTrialProvisioner", "AutoProvisioner", "BudgetAwareProviderDecorator", "ConfigStorage", "KeyExpiryRecovery", + "ProvenanceAwareConfigStorage", "ProvisioningResult", "UpgradeResult", ] diff --git a/src/scolta/ai/amazee/account_upgrader.py b/src/scolta/ai/amazee/account_upgrader.py index 30d0220..6eeef9c 100644 --- a/src/scolta/ai/amazee/account_upgrader.py +++ b/src/scolta/ai/amazee/account_upgrader.py @@ -3,11 +3,27 @@ from __future__ import annotations from .client import AmazeeClient +from .connection_source import AmazeeConnectionSource from .results import UpgradeResult -from .storage import ConfigStorage +from .storage import ConfigStorage, ProvenanceAwareConfigStorage class AmazeeAccountUpgrader: + """Connects a site to an amazee.ai account, by email. + + The only way to reach a real amazee.ai account, and email-only by design: it + mirrors amazee.ai's own ``ai_provider_amazeeio`` Drupal module, where an + operator never generates or pastes an API key. Signing in returns the + account's credentials and Scolta persists them. There is deliberately no + bring-your-own-key path — an operator who already holds an account attaches + it by signing in with that account's email, and the same flow creates the + account when it does not exist yet. + + It serves two operator journeys with the same steps: connecting an account + from a clean install, and continuing after the demo credit runs out, which + :class:`KeyExpiryRecovery` flags with its upgrade-needed marker. + """ + def __init__(self, client: AmazeeClient, storage: ConfigStorage) -> None: self.client = client self.storage = storage @@ -22,6 +38,14 @@ def list_regions(self, session_token: str) -> list: return self.client.list_regions(session_token) def upgrade(self, session_token: str, region_id: str) -> UpgradeResult: + """Provision a private AI key in the given region and store it. + + New credentials replace any existing stored credentials — including a + demo connection this account is replacing — and the connection source is + recorded as ``ACCOUNT`` when the store supports it. + """ result = self.client.create_private_key(session_token, region_id) self.storage.store(result.litellm_token, result.litellm_api_url, result.region) + if isinstance(self.storage, ProvenanceAwareConfigStorage): + self.storage.store_connection_source(AmazeeConnectionSource.ACCOUNT) return result diff --git a/src/scolta/ai/amazee/auto_provisioner.py b/src/scolta/ai/amazee/auto_provisioner.py index 66ef29b..aece669 100644 --- a/src/scolta/ai/amazee/auto_provisioner.py +++ b/src/scolta/ai/amazee/auto_provisioner.py @@ -1,18 +1,28 @@ -"""Idempotent auto-provisioning guard (port of AutoProvisioner).""" +"""Self-heal guard for stored managed-gateway credentials (port of AutoProvisioner).""" from __future__ import annotations from collections.abc import Callable from .client import AmazeeClient -from .exceptions import AmazeeApiException from .model_resolver import AmazeeModelResolver -from .results import ProvisioningResult from .storage import ConfigStorage -from .trial_provisioner import AmazeeTrialProvisioner class AutoProvisioner: + """Keeps already-stored managed-gateway credentials usable. + + This helper never establishes a managed gateway connection. Establishing one + is an explicit caller action: an operator-initiated enable path calls + :meth:`AmazeeTrialProvisioner.provision` directly. Nothing here does it on + the caller's behalf, from an install hook, from a request path, or behind a + flag. + + The name predates the policy and is kept for callers compiled against it. + What remains is :meth:`ensure_ai_available`: a self-heal for credentials that + are already stored but whose model names were never resolved. + """ + @staticmethod def ensure_ai_available( storage: ConfigStorage, @@ -21,65 +31,69 @@ def ensure_ai_available( client: AmazeeClient | None = None, has_resolved_models: Callable[[], bool] | None = None, ) -> bool: - """Provision a free trial unless AI is already configured. Idempotent; - no-op when an explicit key exists or credentials are already stored. - Returns True only when a fresh trial was provisioned. - - The stored-credentials no-op deliberately does NOT validate that the - stored key still works — trial keys are revoked server-side when the - trial ends, and that expiry is not announced at provisioning time, so a - cheap install-hook/lazy-init guard cannot know. Call-time auth failures - are the reliable signal: :class:`KeyExpiryRecovery` detects them, records - the failure for health, and flags the site for admin re-authentication - without requesting replacement credentials. - - Stored credentials are treated as a *complete* provision only once their - model names are resolved. A provision whose ``/model/info`` call failed - stores the token+url with no models, leaving the caller to fall back to - the dated config default — which the Amazee gateway rejects with HTTP - 400, breaking AI permanently because this guard kept no-opping on the - half-provisioned credentials. When the caller can confirm models are - still unresolved (via ``has_resolved_models``), model resolution is - re-attempted against the ALREADY-STORED key — never a fresh trial, which - would waste a server-side-limited allocation — so the incomplete-provision - state self-heals. Without that callback the historical no-op stands: the - caller cannot tell us, and we must not re-resolve blindly every request. + """Re-resolve model names for credentials that are already stored. + + This method never establishes a managed gateway connection, and it makes + no outbound call at all unless credentials are already stored. It is a + no-op when: + + - ``has_explicit_api_key`` is true (the caller has their own provider), + - no credentials are stored — nothing to heal, and nothing is + established here; that is :meth:`AmazeeTrialProvisioner.provision`, + reached only from an explicit operator action, or + - credentials are stored and ``has_resolved_models`` is absent or + reports that model names are already resolved. + + The stored-credentials path deliberately does NOT validate that the + stored key still works — credentials are revoked server-side when their + lifecycle ends, and that is not announced at issue time, so a cheap + lazy-init guard cannot know. Call-time auth failures are the reliable + signal: :class:`KeyExpiryRecovery` detects them, records the failure for + health, and flags the site for admin re-authentication without + requesting replacement credentials. + + Stored credentials are, however, usable only once their model names have + been resolved. Credentials stored while ``/model/info`` was unreachable + carry no resolved models, leaving the caller to fall back to the dated + config default — which the Amazee gateway rejects with HTTP 400, breaking + AI permanently because this guard kept no-opping on the half-configured + credentials. When the caller can confirm models are still unresolved (via + ``has_resolved_models``), model resolution is re-attempted against the + ALREADY-STORED key, so that state self-heals. Without that callback the + historical no-op stands: the caller cannot tell us, and we must not + re-resolve blindly on every request. + + Returns: + Always ``False``. The return value is retained for callers written + against the previous signature; nothing is established here, so + there is no success to report. """ if has_explicit_api_key: return False credentials = storage.load() - if credentials is not None: - # Already provisioned. Self-heal only an incomplete provision — one - # whose model resolution failed, leaving credentials with no models - # — and only when the caller can confirm that state. Re-resolve - # against the stored key (not a new trial) and persist the result. - if has_resolved_models is None or has_resolved_models(): - return False - - models = AmazeeModelResolver(client or AmazeeClient()).resolve( - credentials["litellm_api_url"], credentials["litellm_token"] - ) - if on_models_resolved is not None and ( - models["ai_model"] is not None or models["ai_expansion_model"] is not None - ): - on_models_resolved(models["ai_model"] or "", models["ai_expansion_model"] or "") - return False - - amazee_client = client or AmazeeClient() - provisioner = AmazeeTrialProvisioner( - amazee_client, storage, None, AmazeeModelResolver(amazee_client) - ) - try: - result = provisioner.provision() - except AmazeeApiException: + if credentials is None: + # POLICY: nothing is established here. Automatic enrollment was + # removed outright — there is no automatic path and no flag-gated + # one. A managed gateway connection is established only by an + # explicit operator action that calls + # AmazeeTrialProvisioner.provision(). With no stored credentials + # this is a no-op that makes no outbound call. return False - if not result.success or result.status != ProvisioningResult.STATUS_PROVISIONED: + # Credentials are stored. Self-heal only the incomplete case — model + # resolution never completed, leaving credentials with no models — and + # only when the caller can confirm that state. Re-resolve against the + # stored key and persist the result. + if has_resolved_models is None or has_resolved_models(): return False + models = AmazeeModelResolver(client or AmazeeClient()).resolve( + credentials["litellm_api_url"], credentials["litellm_token"] + ) if on_models_resolved is not None and ( - result.ai_model is not None or result.ai_expansion_model is not None + models["ai_model"] is not None or models["ai_expansion_model"] is not None ): - on_models_resolved(result.ai_model or "", result.ai_expansion_model or "") - return True + on_models_resolved(models["ai_model"] or "", models["ai_expansion_model"] or "") + + return False diff --git a/src/scolta/ai/amazee/connection_source.py b/src/scolta/ai/amazee/connection_source.py new file mode 100644 index 0000000..c432ab9 --- /dev/null +++ b/src/scolta/ai/amazee/connection_source.py @@ -0,0 +1,52 @@ +"""Which operator action produced the stored credentials (port of AmazeeConnectionSource).""" + +from __future__ import annotations + +from enum import Enum + + +class AmazeeConnectionSource(str, Enum): + """Which operator action produced the stored Amazee.ai credentials. + + Recorded at the moment a connection is established, never derived + afterwards. The distinction was previously guessed from whatever local fact + an adapter had to hand, which is why it was removed outright: both the trial + provisioner and the account upgrader persist the same three fields through + :meth:`ConfigStorage.store`, so nothing in the credential store could tell + them apart. Recording the fact at its source is what makes the distinction + reportable again. + + Neither case implies anything automatic. Both are reached only by an + explicit operator action in an admin UI, or by a developer who set + ``ai_provider`` to ``amazee`` in code and then ran the provisioning path. + + Storage backends opt in by implementing + :class:`ProvenanceAwareConfigStorage`. A store that does not — and every + credential persisted before this release — reports no connection source at + all, which callers must surface as unknown rather than as a guess. + """ + + #: The operator started the free demo, which needs no email and no account. + #: + #: One-time per site: the credit it ships with is not renewed. When it runs + #: out the operator continues by signing in to an account (:attr:`ACCOUNT`). + DEMO = "demo" + + #: The operator signed in to an amazee.ai account with their email address. + #: + #: The email → verification code → region flow creates or attaches the + #: account and returns its credentials, which are then persisted. Same flow + #: whether the account is new or already existed, matching amazee.ai's own + #: ``ai_provider_amazeeio`` module. + ACCOUNT = "account" + + def label(self) -> str: + """A short operator-facing name for this connection, in English. + + No label describes a connection as automatic or as provisioned on the + operator's behalf, because neither is. + """ + return { + AmazeeConnectionSource.DEMO: "Amazee.ai demo", + AmazeeConnectionSource.ACCOUNT: "Amazee.ai account", + }[self] diff --git a/src/scolta/ai/amazee/key_expiry_recovery.py b/src/scolta/ai/amazee/key_expiry_recovery.py index d901bcf..2dec663 100644 --- a/src/scolta/ai/amazee/key_expiry_recovery.py +++ b/src/scolta/ai/amazee/key_expiry_recovery.py @@ -131,7 +131,13 @@ def is_auth_failure(exc: BaseException) -> bool: return False def handle_auth_failure(self, exc: BaseException) -> bool: - """Handle an AI call failure on the auto-provisioned Amazee path. + """Handle an AI call failure on the Amazee path. + + "The Amazee path" means a site whose operator connected Amazee.ai — + either by starting the free demo or by signing in to an account. Nothing + reaches this on a site that did not opt in, and nothing here mints a + replacement connection: recovery is an operator action, prompted by the + upgrade-needed marker this sets. For an auth-class failure (the stored credentials are no longer accepted) this records the auth-failure marker so health reports AI as degraded, diff --git a/src/scolta/ai/amazee/storage.py b/src/scolta/ai/amazee/storage.py index a5b77be..b89cf05 100644 --- a/src/scolta/ai/amazee/storage.py +++ b/src/scolta/ai/amazee/storage.py @@ -4,6 +4,8 @@ from abc import ABC, abstractmethod +from .connection_source import AmazeeConnectionSource + class ConfigStorage(ABC): @abstractmethod @@ -15,3 +17,35 @@ def load(self) -> dict | None: @abstractmethod def clear(self) -> None: ... + + +class ProvenanceAwareConfigStorage(ConfigStorage): + """A credential store that can also record how the connection was made. + + Kept separate from :class:`ConfigStorage` so existing implementations keep + working untouched: adopting provenance is opting in to this sub-interface, + not a change to ``store()``'s signature. + + :class:`AmazeeTrialProvisioner` and :class:`AmazeeAccountUpgrader` record the + connection source through this interface when the store they were given + implements it, and skip the record when it does not. A store that does not + implement it reports no provenance, which is honest: nothing then knows how + the credentials were obtained. + + Implementations MUST drop the recorded source in ``clear()``, so + disconnecting does not leave a stale provenance to be paired with the next + connection. + """ + + @abstractmethod + def store_connection_source(self, source: AmazeeConnectionSource) -> None: + """Record which operator action produced the credentials just stored.""" + + @abstractmethod + def load_connection_source(self) -> AmazeeConnectionSource | None: + """The recorded connection source, or ``None`` when none was recorded. + + ``None`` is the correct answer for credentials stored before provenance + was recorded, and for a store that has been cleared. Callers must report + it as "not recorded" and must not substitute a guess. + """ diff --git a/src/scolta/ai/amazee/trial_provisioner.py b/src/scolta/ai/amazee/trial_provisioner.py index dda9946..fbb487b 100644 --- a/src/scolta/ai/amazee/trial_provisioner.py +++ b/src/scolta/ai/amazee/trial_provisioner.py @@ -5,12 +5,23 @@ from collections.abc import Callable from .client import AmazeeClient +from .connection_source import AmazeeConnectionSource from .model_resolver import AmazeeModelResolver from .results import ProvisioningResult -from .storage import ConfigStorage +from .storage import ConfigStorage, ProvenanceAwareConfigStorage class AmazeeTrialProvisioner: + """Establishes the free Amazee.ai demo connection, on an explicit request. + + **Nothing calls this on its own.** It is reached only from an operator + action — the "Try the demo" button in an admin UI, a provisioning management + command, or a first-use path in a headless framework where a developer set + ``ai_provider`` to ``amazee`` in code. :class:`AutoProvisioner` deliberately + does not call it: that class self-heals credentials that are already stored + and establishes nothing. + """ + def __init__( self, client: AmazeeClient, @@ -24,11 +35,19 @@ def __init__( self.model_resolver = model_resolver def provision(self, email: str = "") -> ProvisioningResult: + """Provision the free demo, optionally bound to an email address. + + ``email`` defaults to empty — anonymous provisioning — which is what the + "Try the demo" action in the admin UIs does, so that trying Scolta's AI + costs an operator no input at all. + """ if self.has_existing_provider is not None and self.has_existing_provider(): return ProvisioningResult.skipped_existing_provider() result = self.client.provision_trial(email) self.storage.store(result.litellm_token, result.litellm_api_url, result.region) + if isinstance(self.storage, ProvenanceAwareConfigStorage): + self.storage.store_connection_source(AmazeeConnectionSource.DEMO) if self.model_resolver is not None: models = self.model_resolver.resolve(result.litellm_api_url, result.litellm_token) diff --git a/src/scolta/ai/client.py b/src/scolta/ai/client.py index 298cf9a..cb8bcfe 100644 --- a/src/scolta/ai/client.py +++ b/src/scolta/ai/client.py @@ -25,7 +25,13 @@ class AiClient: def __init__(self, config: dict, http_client: httpx.Client | None = None) -> None: - self.provider = config.get("provider", "anthropic") + # No default. An absent or empty provider is "nobody selected one", and + # that is an error rather than Anthropic: a client must never be built + # on an assumption about which vendor the site meant. Callers are + # expected to keep AI off instead of constructing one. + self.provider = config.get("provider", "") + if not self.provider.strip(): + raise ValueError("No AI provider selected. Set one of: anthropic, openai.") self.api_key = config.get("api_key", "") self.model = config.get("model", "claude-sonnet-4-5-20250929") self.api_version = config.get("api_version", ANTHROPIC_API_VERSION) diff --git a/src/scolta/ai/service.py b/src/scolta/ai/service.py index 0899dd6..07eef42 100644 --- a/src/scolta/ai/service.py +++ b/src/scolta/ai/service.py @@ -9,6 +9,7 @@ from __future__ import annotations from ..config import ScoltaConfig +from ..exceptions import ApiKeyMissingException from . import prompts from .amazee.key_expiry_recovery import KeyExpiryRecovery from .client import AiClient @@ -108,6 +109,21 @@ def resolve_prompt(self, template: str) -> str: # -- overridable hooks -------------------------------------------------- def _get_client(self) -> AiClient: + """Get the built-in AiClient, refusing to build one with no provider. + + Scolta ships without a provider selected, and an unselected provider + means AI is off — not that it is Anthropic. Constructing a client here + would pick a vendor on the site's behalf, so instead this raises the + :class:`ApiKeyMissingException` the callers already degrade on: the + query goes out unexpanded and no summary is produced, which is what + "AI off" looks like from the outside. + """ + if not self._config.ai_provider.strip(): + raise ApiKeyMissingException( + "No AI provider is selected, so AI features are off. Select one in the " + "Scolta settings, or set the AI provider in configuration." + ) + if self._client is None: self._client = self._create_client() return self._client diff --git a/src/scolta/config.py b/src/scolta/config.py index a4d0193..efa5173 100644 --- a/src/scolta/config.py +++ b/src/scolta/config.py @@ -21,7 +21,11 @@ @dataclass class ScoltaConfig: # -- AI provider -- - ai_provider: str = "anthropic" + # No default. An install nobody has configured has AI off: search works, no + # provider is assumed, and Anthropic in particular is not silently assumed. + # Selecting a provider is always deliberate. Going-forward only: a value + # already persisted by a site is read as-is and never rewritten. + ai_provider: str = "" ai_api_key: str = "" ai_model: str = "claude-sonnet-4-5-20250929" ai_expansion_model: str = "" diff --git a/src/scolta/health.py b/src/scolta/health.py index 944294d..e54523d 100644 --- a/src/scolta/health.py +++ b/src/scolta/health.py @@ -69,7 +69,12 @@ def check(self) -> dict: self.cache.get(KeyExpiryRecovery.CACHE_KEY_AUTH_FAILURE), KeyExpiryRecovery.AUTH_FAILURE_TTL, ) - ai_usable = ai_configured and not ai_auth_failing + # No provider selected means AI is off, whatever else is present. A key + # can exist without a provider — an environment variable set before + # anybody chose one — and reporting that as usable would restore by the + # back door the assumption that an unselected provider is Anthropic. + provider_selected = self.config.ai_provider.strip() != "" + ai_usable = ai_configured and provider_selected and not ai_auth_failing status = "ok" if not index_exists or not ai_usable: @@ -93,7 +98,11 @@ def check(self) -> dict: return { "status": status, - "ai_provider": self.config.ai_provider or "anthropic", + # "" means no provider has been selected, which is what a fresh + # install reports. Never coalesced to "anthropic": claiming a + # provider nobody chose is the failure this field exists to expose. + "ai_provider": self.config.ai_provider, + "ai_provider_selected": provider_selected, "ai_configured": ai_configured, "ai_usable": ai_usable, "ai_auth_failing": ai_auth_failing, diff --git a/tests/ai/amazee/test_amazee.py b/tests/ai/amazee/test_amazee.py index fe37726..0244baa 100644 --- a/tests/ai/amazee/test_amazee.py +++ b/tests/ai/amazee/test_amazee.py @@ -334,35 +334,48 @@ def test_auto_provisioner_skips_when_already_provisioned(): assert AutoProvisioner.ensure_ai_available(storage) is False -def test_auto_provisioner_provisions_and_reports_models(): +def test_auto_provisioner_never_mints_and_never_calls_out(): + # Replaces a test that asserted the opposite — that a first pass with an + # empty store provisioned a trial. That behaviour is gone: a connection is + # established only by an explicit AmazeeTrialProvisioner.provision() call + # from an operator action. With nothing stored there is nothing to heal, so + # this guard makes no outbound call at all. The fail-on-call transport turns + # a regression into a named endpoint rather than a swallowed error. + attempted = [] + def handler(request): - if request.url.path == "/auth/generate-trial-access": - return httpx.Response( - 200, - json={"litellm_token": "tok", "litellm_api_url": "https://llm.x", "region": "us"}, - ) - if request.url.path == "/model/info": - return httpx.Response(200, json={"data": [{"model_name": "claude-sonnet-4-6"}]}) - return httpx.Response(404) + attempted.append(request.url.path) + raise AssertionError(f"no outbound Amazee call expected, got {request.url.path}") client = AmazeeClient(http_client=httpx.Client(transport=httpx.MockTransport(handler))) storage = MemoryStorage() - reported = {} - ok = AutoProvisioner.ensure_ai_available( - storage, on_models_resolved=lambda m, e: reported.update({"m": m, "e": e}), client=client + reported = [] + + result = AutoProvisioner.ensure_ai_available( + storage, + on_models_resolved=lambda m, e: reported.append((m, e)), + client=client, + has_resolved_models=lambda: False, ) - assert ok is True - assert storage.load()["litellm_token"] == "tok" - assert reported["m"] == "claude-sonnet-4-6" + assert result is False + assert storage.load() is None + assert reported == [] + assert attempted == [] + + +def test_auto_provisioner_with_explicit_key_touches_nothing(): + def handler(request): + raise AssertionError(f"no outbound Amazee call expected, got {request.url.path}") + + client = AmazeeClient(http_client=httpx.Client(transport=httpx.MockTransport(handler))) + storage = MemoryStorage() -def test_auto_provisioner_returns_false_on_api_error(): - client = AmazeeClient( - http_client=httpx.Client( - transport=httpx.MockTransport(lambda r: httpx.Response(500, json={})) - ) + assert ( + AutoProvisioner.ensure_ai_available(storage, has_explicit_api_key=True, client=client) + is False ) - assert AutoProvisioner.ensure_ai_available(MemoryStorage(), client=client) is False + assert storage.load() is None # -- auto provisioner: self-heal of an incomplete provision ------------------- @@ -399,16 +412,13 @@ def handler(request): storage = MemoryStorage() resolved = [] - # Pass 1: trial provisioning succeeds; /model/info returns no models. - provisioned = AutoProvisioner.ensure_ai_available( - storage, - on_models_resolved=lambda m, e: resolved.append((m, e)), - client=client, - has_resolved_models=lambda: False, - ) - assert provisioned is True # a fresh trial WAS provisioned + # Pass 1: the operator connects the demo explicitly, which is the only way + # credentials are ever established; /model/info returns no models, so the + # store is left half-provisioned. (This used to be driven through + # ensure_ai_available(), which no longer mints anything.) + AmazeeTrialProvisioner(client, storage, None, AmazeeModelResolver(client)).provision() assert storage.load()["litellm_token"] == "tok" - assert resolved == [] # but models stayed unresolved — the gap + assert resolved == [] # models stayed unresolved — the gap # Pass 2: credentials present, models still unresolved → self-heal by # re-resolving against the stored key. No second trial is provisioned. @@ -421,7 +431,7 @@ def handler(request): ) assert healed is False # a model-only heal, not a new provision assert resolved == [("claude-sonnet-4-6", "claude-haiku-4-5")] - assert state["trial_calls"] == 1 # never burned a second trial + assert state["trial_calls"] == 1 # never burned a second demo # The resolved model is a real undated alias, never the dated default the # gateway rejects. assert resolved[0][0] != "claude-sonnet-4-5-20250929" diff --git a/tests/ai/amazee/test_manual_provider_and_opt_in.py b/tests/ai/amazee/test_manual_provider_and_opt_in.py new file mode 100644 index 0000000..bd70cc4 --- /dev/null +++ b/tests/ai/amazee/test_manual_provider_and_opt_in.py @@ -0,0 +1,301 @@ +"""The two policy invariants, asserted where they are decided. + +**A — no default provider.** Nothing ships with an AI provider selected. +``ai_provider`` is empty until somebody chooses one, and while it is empty AI is +off: search still works, no provider is assumed, and Anthropic in particular is +not silently assumed. + +**B — Amazee is never auto-enabled.** No Amazee credential is provisioned and no +outbound Amazee call is made on any request, cron, install or activation path +for a site that did not opt in. The one automatic activity permitted is +re-resolving gateway model names against the key already on disk, which only a +site that already connected Amazee can reach. + +Every Amazee transport here fails the test if it is called, so an unexpected +outbound call is a hard failure naming the endpoint rather than a swallowed +transport error. + +Mirrors ``tests/AiProvider/Amazee/ManualProviderAndOptInTest.php`` in +scolta-php: the three cores share one contract, and this is where Python is held +to it. +""" + +from __future__ import annotations + +import httpx +import pytest + +from scolta.ai.amazee import ( + AmazeeAccountUpgrader, + AmazeeClient, + AmazeeConnectionSource, + AmazeeTrialProvisioner, + AutoProvisioner, + ConfigStorage, + ProvenanceAwareConfigStorage, +) +from scolta.ai.client import AiClient +from scolta.ai.service import AiServiceAdapter +from scolta.config import ScoltaConfig +from scolta.exceptions import ApiKeyMissingException +from scolta.health import HealthChecker + + +class _MemoryStorage(ConfigStorage): + """A store with nowhere to record provenance, like a pre-1.2.0 adapter.""" + + def __init__(self) -> None: + self.stored: dict | None = None + + def store(self, litellm_token: str, litellm_api_url: str, region: str) -> None: + self.stored = { + "litellm_token": litellm_token, + "litellm_api_url": litellm_api_url, + "region": region, + } + + def load(self) -> dict | None: + return self.stored + + def clear(self) -> None: + self.stored = None + + +class _ProvenanceStorage(ProvenanceAwareConfigStorage): + """An in-memory store that records provenance, like a real adapter's.""" + + def __init__(self) -> None: + self.stored: dict | None = None + self.source: AmazeeConnectionSource | None = None + + def store(self, litellm_token: str, litellm_api_url: str, region: str) -> None: + self.stored = { + "litellm_token": litellm_token, + "litellm_api_url": litellm_api_url, + "region": region, + } + + def load(self) -> dict | None: + return self.stored + + def clear(self) -> None: + self.stored = None + self.source = None + + def store_connection_source(self, source: AmazeeConnectionSource) -> None: + self.source = source + + def load_connection_source(self) -> AmazeeConnectionSource | None: + return self.source + + +def _fail_on_call_client(attempted: list[str]) -> AmazeeClient: + def handler(request: httpx.Request) -> httpx.Response: + attempted.append(request.url.path) + raise AssertionError(f"no outbound Amazee call expected, got {request.url.path}") + + return AmazeeClient(http_client=httpx.Client(transport=httpx.MockTransport(handler))) + + +# -- Invariant A: no default provider ----------------------------------------- + + +def test_config_ships_with_no_provider_selected(): + assert ScoltaConfig().ai_provider == "" + assert ScoltaConfig.from_dict({}).ai_provider == "" + + +def test_an_already_chosen_provider_is_preserved(): + # Going-forward only: a site that already picked a provider keeps it. + for chosen in ("anthropic", "openai", "amazee"): + assert ScoltaConfig.from_dict({"ai_provider": chosen}).ai_provider == chosen + + +def test_unconfigured_install_makes_no_ai_call_on_any_operation(): + builds = [] + + class _Adapter(AiServiceAdapter): + def _create_client(self): + builds.append(1) + raise AssertionError("an AI client was built with no provider selected") + + adapter = _Adapter(ScoltaConfig.from_dict({})) + + for operation in ("expand_query", "summarize", "follow_up"): + with pytest.raises(ApiKeyMissingException): + adapter.message_for_operation(operation, "sys", "user", 512) + + assert builds == [] + + +def test_ai_client_refuses_to_assume_a_provider(): + with pytest.raises(ValueError, match="No AI provider selected"): + AiClient({"api_key": "sk-test"}) + + +def test_health_reports_ai_off_rather_than_assuming_anthropic(tmp_path): + result = HealthChecker(ScoltaConfig.from_dict({}), str(tmp_path), None, None).check() + + assert result["ai_provider"] == "" + assert result["ai_provider_selected"] is False + assert result["ai_configured"] is False + assert result["ai_usable"] is False + + +def test_key_without_a_provider_is_still_ai_off(tmp_path): + # The case a coalescing default used to hide: a key set before anybody chose + # a provider looked like a working Anthropic install. + config = ScoltaConfig.from_dict({"ai_api_key": "sk-env"}) + result = HealthChecker(config, str(tmp_path), None, None).check() + + assert result["ai_provider"] == "" + assert result["ai_provider_selected"] is False + assert result["ai_usable"] is False + + +# -- Invariant B: Amazee is never auto-enabled -------------------------------- + + +def test_ensure_ai_available_never_mints_and_never_calls_out(): + attempted: list[str] = [] + storage = _MemoryStorage() + reported: list[tuple[str, str]] = [] + + result = AutoProvisioner.ensure_ai_available( + storage, + on_models_resolved=lambda m, e: reported.append((m, e)), + client=_fail_on_call_client(attempted), + has_resolved_models=lambda: False, + ) + + assert result is False + assert storage.load() is None + assert reported == [] + assert attempted == [] + + +def test_ensure_ai_available_with_an_explicit_key_touches_nothing(): + attempted: list[str] = [] + storage = _MemoryStorage() + + assert ( + AutoProvisioner.ensure_ai_available( + storage, has_explicit_api_key=True, client=_fail_on_call_client(attempted) + ) + is False + ) + assert attempted == [] + assert storage.load() is None + + +def test_self_heal_uses_the_stored_key_and_does_not_mint(): + # The only automatic Amazee activity the policy permits, reachable only for + # a site whose operator already connected. + paths: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + paths.append(request.url.path) + if request.url.path == "/model/info": + return httpx.Response(200, json={"data": [{"model_name": "claude-sonnet-4-6"}]}) + raise AssertionError(f"unexpected call to {request.url.path}") + + client = AmazeeClient(http_client=httpx.Client(transport=httpx.MockTransport(handler))) + storage = _MemoryStorage() + storage.store("stored-tok", "https://gateway.amazee.ai", "us-east") + + AutoProvisioner.ensure_ai_available(storage, client=client, has_resolved_models=lambda: False) + + assert paths == ["/model/info"] + assert storage.load()["litellm_token"] == "stored-tok" + + +# -- Provenance: recorded at connect time, never guessed ---------------------- + + +def _scripted_client(routes: dict[str, dict], recorded: list[bytes] | None = None) -> AmazeeClient: + def handler(request: httpx.Request) -> httpx.Response: + if recorded is not None: + recorded.append(request.content) + return httpx.Response(200, json=routes[request.url.path]) + + return AmazeeClient(http_client=httpx.Client(transport=httpx.MockTransport(handler))) + + +def test_demo_provision_records_demo_provenance_with_no_email(): + storage = _ProvenanceStorage() + bodies: list[bytes] = [] + client = _scripted_client( + { + "/auth/generate-trial-access": { + "litellm_token": "demo-tok", + "litellm_api_url": "https://gateway.amazee.ai", + "region": "us-east", + } + }, + bodies, + ) + + AmazeeTrialProvisioner(client, storage).provision() + + assert storage.load_connection_source() is AmazeeConnectionSource.DEMO + # No email is sent: trying the demo costs the operator no input. + assert b'"email": ""' in bodies[0] or b'"email":""' in bodies[0] + + +def test_account_sign_in_records_account_provenance(): + storage = _ProvenanceStorage() + storage.store("demo-tok", "https://gateway.amazee.ai", "us-east") + storage.store_connection_source(AmazeeConnectionSource.DEMO) + + client = _scripted_client( + { + "/private-ai-keys": { + "litellm_token": "account-tok", + "litellm_api_url": "https://ch.amazee.ai", + "region": "ch", + } + } + ) + + AmazeeAccountUpgrader(client, storage).upgrade("session", "ch") + + assert storage.load_connection_source() is AmazeeConnectionSource.ACCOUNT + assert storage.load()["litellm_token"] == "account-tok" + + +def test_provenance_unaware_store_connects_and_records_nothing(): + storage = _MemoryStorage() + client = _scripted_client( + { + "/auth/generate-trial-access": { + "litellm_token": "tok", + "litellm_api_url": "https://gateway.amazee.ai", + "region": "us-east", + } + } + ) + + AmazeeTrialProvisioner(client, storage).provision() + + assert storage.load()["litellm_token"] == "tok" + assert not isinstance(storage, ProvenanceAwareConfigStorage) + + +def test_clearing_credentials_also_clears_provenance(): + # A stale mark left behind would be paired with the next connection, which + # is a guess wearing a recorded fact's clothes. + storage = _ProvenanceStorage() + storage.store("tok", "https://gateway.amazee.ai", "us-east") + storage.store_connection_source(AmazeeConnectionSource.DEMO) + + storage.clear() + + assert storage.load_connection_source() is None + assert storage.load() is None + + +def test_no_connection_source_label_implies_automatic_provisioning(): + for source in AmazeeConnectionSource: + for banned in ("auto", "automatic", "free trial"): + assert banned not in source.value.lower() + assert banned not in source.label().lower() diff --git a/tests/ai/test_client.py b/tests/ai/test_client.py index ceedd13..bb8eefe 100644 --- a/tests/ai/test_client.py +++ b/tests/ai/test_client.py @@ -20,6 +20,10 @@ def _client(config, handler): transport = httpx.MockTransport(handler) + # A provider is required — there is no default. Tests about request shape, + # error mapping and timeouts are not about provider selection, so the helper + # supplies one unless the case sets its own. + config = {"provider": "anthropic", **config} return AiClient(config, http_client=httpx.Client(transport=transport)) diff --git a/tests/ai/test_service.py b/tests/ai/test_service.py index a9b197e..1d5b652 100644 --- a/tests/ai/test_service.py +++ b/tests/ai/test_service.py @@ -122,7 +122,7 @@ class _RecordingClient(AiClient): """Records the model and temperature passed to each message() call.""" def __init__(self): - super().__init__({}) + super().__init__({"provider": "anthropic"}) self.calls: list[dict] = [] def message(self, system_prompt, user_message, max_tokens=1024, model=None, temperature=None): @@ -145,7 +145,7 @@ def _get_client(self): def test_expand_query_reaches_client_with_temperature_zero(): # Expansion is a deterministic semantic mapping — it must run at # temperature 0 so the same query yields the same terms every call. - adapter = _make_recording_adapter(ScoltaConfig.from_dict({})) + adapter = _make_recording_adapter(ScoltaConfig.from_dict({"ai_provider": "anthropic"})) result = adapter.message_for_operation("expand_query", "sys", "user", 512) @@ -156,7 +156,7 @@ def test_expand_query_reaches_client_with_temperature_zero(): def test_non_expansion_operation_reaches_client_with_null_temperature(): # Summarize (and follow-up) are creative surfaces — they keep the provider # default, i.e. no temperature is sent (None). - adapter = _make_recording_adapter(ScoltaConfig.from_dict({})) + adapter = _make_recording_adapter(ScoltaConfig.from_dict({"ai_provider": "anthropic"})) adapter.message_for_operation("summarize", "sys", "user", 512) @@ -194,7 +194,7 @@ def test_ai_expansion_model_not_included_in_ai_client_config(): class _ThrowingClient(AiClient): def __init__(self, to_throw): self._to_throw = to_throw - super().__init__({}) + super().__init__({"provider": "anthropic"}) def message(self, system_prompt, user_message, max_tokens=1024, model=None, temperature=None): raise self._to_throw @@ -320,7 +320,9 @@ def _make_recovering_adapter(to_throw): """Adapter whose client always throws ``to_throw``, with recovery wired against a credential store seeded with stored credentials. Returns (adapter, storage, recovery).""" - cfg = ScoltaConfig.from_dict({}) + # A site with stored Amazee credentials has selected a provider: the Amazee + # gateway is LiteLLM, which speaks the OpenAI wire protocol. + cfg = ScoltaConfig.from_dict({"ai_provider": "openai"}) class _Adapter(AiServiceAdapter): def __init__(self, config, stub): diff --git a/tests/test_config.py b/tests/test_config.py index a358e84..6578057 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -11,7 +11,8 @@ def test_defaults(): c = ScoltaConfig() - assert c.ai_provider == "anthropic" + # No default provider. An untouched config has AI off, not Anthropic. + assert c.ai_provider == "" assert c.indexer == "auto" assert c.expand_subword_max_frequency == 0.05 assert c.expansion_combine_mode == "relevance_union" diff --git a/tests/test_health.py b/tests/test_health.py index 8631f36..f7159d1 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -9,7 +9,7 @@ def test_check_returns_expected_structure(tmp_path): - config = ScoltaConfig.from_dict({"ai_api_key": "sk-test"}) + config = ScoltaConfig.from_dict({"ai_provider": "anthropic", "ai_api_key": "sk-test"}) result = HealthChecker(config, str(tmp_path), None, None).check() for key in ( "status", @@ -28,7 +28,7 @@ def test_check_returns_expected_structure(tmp_path): def test_healthy_system_with_index(tmp_path): (tmp_path / "pagefind.js").write_text("// pagefind") - config = ScoltaConfig.from_dict({"ai_api_key": "sk-test-key"}) + config = ScoltaConfig.from_dict({"ai_provider": "anthropic", "ai_api_key": "sk-test-key"}) result = HealthChecker(config, str(tmp_path), None, None).check() assert result["status"] == "ok" assert result["ai_configured"] is True @@ -36,7 +36,7 @@ def test_healthy_system_with_index(tmp_path): def test_degraded_without_index(tmp_path): - config = ScoltaConfig.from_dict({"ai_api_key": "sk-test-key"}) + config = ScoltaConfig.from_dict({"ai_provider": "anthropic", "ai_api_key": "sk-test-key"}) result = HealthChecker(config, str(tmp_path), None, None).check() assert result["status"] == "degraded" assert result["index_exists"] is False @@ -52,12 +52,14 @@ def test_degraded_without_ai_key(tmp_path): def test_pagefind_subdir_index_detected(tmp_path): (tmp_path / "pagefind").mkdir() (tmp_path / "pagefind" / "pagefind.js").write_text("// pagefind") - config = ScoltaConfig.from_dict({"ai_api_key": "sk"}) + config = ScoltaConfig.from_dict({"ai_provider": "anthropic", "ai_api_key": "sk"}) assert HealthChecker(config, str(tmp_path), None, None).check()["index_exists"] is True def test_binary_indexer_upgrade_message_when_unavailable(tmp_path): - config = ScoltaConfig.from_dict({"ai_api_key": "sk", "indexer": "binary"}) + config = ScoltaConfig.from_dict( + {"ai_provider": "anthropic", "ai_api_key": "sk", "indexer": "binary"} + ) result = HealthChecker( config, str(tmp_path), "/nonexistent/pagefind-xyz", "/nonexistent" ).check() @@ -79,7 +81,9 @@ def test_stored_but_auth_failing_credentials_report_ai_not_usable(tmp_path): cache = InMemoryCacheDriver() cache.set(KeyExpiryRecovery.CACHE_KEY_AUTH_FAILURE, time.time(), 3600) - config = ScoltaConfig.from_dict({"ai_api_key": "sk-stored-but-expired"}) + config = ScoltaConfig.from_dict( + {"ai_provider": "anthropic", "ai_api_key": "sk-stored-but-expired"} + ) result = HealthChecker(config, str(tmp_path), None, None, cache).check() assert result["ai_configured"] is True, "Credentials ARE present — configured stays true" @@ -91,7 +95,7 @@ def test_stored_but_auth_failing_credentials_report_ai_not_usable(tmp_path): def test_configured_and_not_auth_failing_reports_usable(tmp_path): (tmp_path / "pagefind.js").write_text("// pagefind") - config = ScoltaConfig.from_dict({"ai_api_key": "sk-good"}) + config = ScoltaConfig.from_dict({"ai_provider": "anthropic", "ai_api_key": "sk-good"}) result = HealthChecker(config, str(tmp_path), None, None, InMemoryCacheDriver()).check() assert result["ai_usable"] is True @@ -104,7 +108,7 @@ def test_without_cache_ai_usable_mirrors_configured(tmp_path): # unchanged from before the ai_usable field existed. (tmp_path / "pagefind.js").write_text("// pagefind") - config = ScoltaConfig.from_dict({"ai_api_key": "sk-good"}) + config = ScoltaConfig.from_dict({"ai_provider": "anthropic", "ai_api_key": "sk-good"}) result = HealthChecker(config, str(tmp_path), None, None).check() assert result["ai_usable"] is True @@ -118,7 +122,7 @@ def test_cleared_auth_failure_marker_restores_usable(tmp_path): # KeyExpiryRecovery clears the marker by overwriting it with False. cache.set(KeyExpiryRecovery.CACHE_KEY_AUTH_FAILURE, False, 1) - config = ScoltaConfig.from_dict({"ai_api_key": "sk-recovered"}) + config = ScoltaConfig.from_dict({"ai_provider": "anthropic", "ai_api_key": "sk-recovered"}) result = HealthChecker(config, str(tmp_path), None, None, cache).check() assert result["ai_usable"] is True @@ -136,7 +140,7 @@ def test_stale_auth_failure_marker_ages_out(tmp_path): 3600, ) - config = ScoltaConfig.from_dict({"ai_api_key": "sk-good"}) + config = ScoltaConfig.from_dict({"ai_provider": "anthropic", "ai_api_key": "sk-good"}) result = HealthChecker(config, str(tmp_path), None, None, cache).check() assert result["ai_usable"] is True