diff --git a/.env.example b/.env.example index 758fcc5a510..8413100bb39 100644 --- a/.env.example +++ b/.env.example @@ -127,9 +127,37 @@ LOG_LEVEL=INFO # COMPUTE_PERMITTED_EXECUTABLES='["/opt/conda/envs/tomopy/bin/tomopy"]' # CORA_ALLOW_RAW_CONDUCT=false -# LLM provider — Agent BC (Phase 8f-b) -# Required if the deployment ships the RunDebrief or CautionDrafter subscribers. -# Without it, the subscribers register but log-and-skip on every Run-terminal event. +# LLM provider: the switch and the credential +# Two settings, and BOTH are required before CORA calls an external model. +# LLM_ENABLED is the SWITCH and defaults to false; ANTHROPIC_API_KEY is the +# CREDENTIAL. Set only the key and nothing calls out: the switch decides. +# +# That split exists because a key is often present in an environment for an +# unrelated reason, and its mere presence used to be enough to register the +# RunDebriefer and CautionDrafter subscribers, which call an external API on +# EVERY terminal Run. That is experiment metadata leaving the facility and +# money being spent, switched on by a side effect. Every sibling subscriber +# already had its own default-off flag; this seam was the outlier. +# +# Leaving it off is graceful, never a crash: the two LLM-backed subscribers +# are skipped with a warning naming which of the two settings is missing, +# `regenerate_run_debrief` answers unavailable, and a conduct command that +# explicitly asks for the `llm` decide substrate gets a 422. +# +# UPGRADING: a deployment that ran the LLM subscribers on ANTHROPIC_API_KEY +# alone must now also set LLM_ENABLED=true, or those two stop registering. +# Nothing crashes; a boot warning names the switch and /readyz reports "off". +# +# NOTE this is a DIFFERENT axis from actuation (CONTROL_WRITES_ENABLED and +# COMPUTE_SUBSTRATE above). Those bound what CORA can MOVE; these bound the +# LLM path. A facility told "read-only" hears the first and would still +# object to the second, so neither promise covers the other. +# +# Read the scope narrowly: this bounds the LLM path ONLY, not egress in +# general. The HTTP checksum adapter is wired unconditionally for http/https +# Distributions, so CORA can still make outbound requests with the LLM off. +# Verify the effective state at `GET /readyz` -> `"llm": "off" | "live"`. +# LLM_ENABLED=true # ANTHROPIC_API_KEY=sk-ant-... # Observability — OpenTelemetry diff --git a/apps/api/openapi.json b/apps/api/openapi.json index 7651dbfca1f..28b37ef89e9 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -18383,7 +18383,7 @@ } } }, - "description": "RunDebriefer subscriber is not wired (ANTHROPIC_API_KEY unset)." + "description": "RunDebriefer is not wired (LLM_ENABLED off, the default, or no ANTHROPIC_API_KEY). The body names which." } }, "summary": "Re-invoke RunDebriefer on demand against a specific Run.", diff --git a/apps/api/src/cora/agent/_subscribers.py b/apps/api/src/cora/agent/_subscribers.py index 61ce933b687..3daa78ae6b4 100644 --- a/apps/api/src/cora/agent/_subscribers.py +++ b/apps/api/src/cora/agent/_subscribers.py @@ -17,9 +17,12 @@ The deterministic subscribers (AuthorityRevocationHolder, CautionPromoter) register independently of `kernel.llm`. The LLM-backed subscribers (RunDebriefer, CautionDrafter) register only -when `kernel.llm` is wired (`ANTHROPIC_API_KEY` set); if it is None -they are skipped with a warning rather than refusing to boot a -deployment that wants to defer Agent rollout. +when `kernel.llm` is wired, which needs BOTH `llm_enabled` (the +switch, default off) and `ANTHROPIC_API_KEY` (the credential); if it +is None they are skipped with a warning rather than refusing to boot a +deployment that wants to defer Agent rollout. These two are the seam +through which experiment metadata would leave the facility and tokens +would be spent, on every terminal Run, so the switch defaults off. ## Registered subscribers @@ -65,6 +68,7 @@ from typing import TYPE_CHECKING +from cora.agent.build_llm import llm_unwired_reason from cora.agent.subscribers.authority_revocation_holder import ( make_authority_revocation_holder_subscriber, ) @@ -136,7 +140,7 @@ def register_agent_subscribers(registry: ProjectionRegistry, deps: Kernel) -> No _log.warning( "agent_subscriber.skipped", subscribers=["run_debriefer", "caution_drafter"], - reason="kernel.llm is None (ANTHROPIC_API_KEY not configured)", + reason=llm_unwired_reason(deps.settings), ) return diff --git a/apps/api/src/cora/agent/build_llm.py b/apps/api/src/cora/agent/build_llm.py index 0e51a2d34eb..2a43a1520fc 100644 --- a/apps/api/src/cora/agent/build_llm.py +++ b/apps/api/src/cora/agent/build_llm.py @@ -7,12 +7,23 @@ owns `PostgresClearanceLookup`, Caution BC owns `PostgresCautionLookup`). -When `Settings.anthropic_api_key` is unset, returns `None` so the -Kernel ends up with `llm=None` and Agent subscribers fail-fast at -registration. This is intentional: a misconfigured prod deployment -should not silently downgrade to a no-LLM mode where RunDebriefer -goes silent. The subscriber-registration step fail-fasts on -`kernel.llm is None`. +Returns `None` unless BOTH `Settings.llm_enabled` (the switch) and +`Settings.anthropic_api_key` (the credential) are set, so the Kernel +carries `llm=None` and no external model is ever called. + +`llm=None` is a fully supported state, not a degraded one: every +consumer already handles it because the key-absent case always +produced it. The LLM-backed subscribers log-and-skip +(`cora.agent._subscribers`) and `regenerate_run_debrief` answers +unavailable. The `llm` decide substrate cannot be selected by a remote +caller at all (`WireDecideSubstrate` admits only `in_memory` and +`grid_walk`), so `build_decide_port`'s `llm is None` guard is an +internal-caller guard, not a request-reachable path. + +(An earlier version of this docstring claimed subscriber registration +"fail-fasts" on `kernel.llm is None`. It does not, and never did: +`register_agent_subscribers` logs a warning and skips, deliberately, +so a deployment may defer Agent rollout without refusing to boot.) """ from cora.agent.adapters.anthropic_llm import AnthropicLLM @@ -21,11 +32,20 @@ def build_llm(settings: Settings) -> LLM | None: - """Construct the production LLM or return `None` when unconfigured. + """Construct the production LLM, or `None` when off or unconfigured. + + Two independent reasons to return `None`, and the switch is checked + first so an operator who turns the LLM off gets that answer even + with a credential present in the environment: - Today this branches on `settings.anthropic_api_key`; a future - multi-provider deployment would branch on a `settings.llm_provider` - field with `anthropic` as one variant. + - `llm_enabled` is False (the default): this deployment does not + call an external model, so no experiment metadata leaves the + facility through this seam and no tokens are spent. + - no `anthropic_api_key`: nothing to call it with. + + Today the credential branch reads `settings.anthropic_api_key`; a + future multi-provider deployment would branch on a + `settings.llm_provider` field with `anthropic` as one variant. `SecretStr.get_secret_value()` is the ONLY place in the codebase that unwraps the API key; passing the raw string to the adapter @@ -33,9 +53,35 @@ def build_llm(settings: Settings) -> LLM | None: "live credential". Adapter scope is responsible for not re- exposing it (eg. via `repr(adapter)` or `str(adapter._client)`). """ + if not settings.llm_enabled: + return None if settings.anthropic_api_key is None: return None return AnthropicLLM(api_key=settings.anthropic_api_key.get_secret_value()) -__all__ = ["build_llm"] +def llm_unwired_reason(settings: Settings) -> str: + """Say WHICH of the two settings left `kernel.llm` unwired. + + One source of truth for every surface that has to explain the + unwired state: the subscriber-registration warning, the REST 503 + body, and the MCP tool's error. They must agree, and each must send + the operator to the remedy that actually applies. Naming the + credential when the switch is simply off is the more likely mistake + now, because off is the default. + + Callers are expected to have already established that the LLM is + unwired; this only explains why. + """ + if not settings.llm_enabled: + return ( + "LLM_ENABLED is false, so this deployment calls no external model. " + "Set LLM_ENABLED=true (and ANTHROPIC_API_KEY) to turn the LLM features on." + ) + return ( + "LLM_ENABLED is true but ANTHROPIC_API_KEY is not configured. " + "Supply the credential and restart." + ) + + +__all__ = ["build_llm", "llm_unwired_reason"] diff --git a/apps/api/src/cora/agent/features/regenerate_run_debrief/route.py b/apps/api/src/cora/agent/features/regenerate_run_debrief/route.py index f175a592dcd..2f28e3b4ee5 100644 --- a/apps/api/src/cora/agent/features/regenerate_run_debrief/route.py +++ b/apps/api/src/cora/agent/features/regenerate_run_debrief/route.py @@ -17,6 +17,7 @@ from fastapi import APIRouter, Depends, Header, HTTPException, Path, Request, status from pydantic import BaseModel, Field +from cora.agent.build_llm import llm_unwired_reason from cora.agent.features.regenerate_run_debrief.command import RegenerateRunDebrief from cora.agent.features.regenerate_run_debrief.handler import IdempotentHandler from cora.infrastructure.routing import ( @@ -53,9 +54,8 @@ def _get_handler(request: Request) -> IdempotentHandler: raise HTTPException( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=( - "regenerate_run_debrief is unavailable: kernel.llm is not wired " - "(ANTHROPIC_API_KEY not configured). Configure the API key " - "and restart, or use the test FakeLLM for non-prod." + f"regenerate_run_debrief is unavailable: kernel.llm is not wired. " + f"{llm_unwired_reason(request.app.state.deps.settings)}" ), ) return handler @@ -89,7 +89,10 @@ def _get_handler(request: Request) -> IdempotentHandler: }, status.HTTP_503_SERVICE_UNAVAILABLE: { "model": ErrorResponse, - "description": ("RunDebriefer subscriber is not wired (ANTHROPIC_API_KEY unset)."), + "description": ( + "RunDebriefer is not wired (LLM_ENABLED off, the default, or no " + "ANTHROPIC_API_KEY). The body names which." + ), }, }, summary="Re-invoke RunDebriefer on demand against a specific Run.", diff --git a/apps/api/src/cora/agent/tools.py b/apps/api/src/cora/agent/tools.py index ae53310b558..818060f9071 100644 --- a/apps/api/src/cora/agent/tools.py +++ b/apps/api/src/cora/agent/tools.py @@ -94,8 +94,8 @@ def register_agent_tools( # regenerate_run_debrief handler is Optional in the bundle # (None when kernel.llm is unwired). The tool's lambda dereferences # at call time and raises a RuntimeError to surface as MCP isError; - # this matches the REST 503 semantics. Production deploys with - # ANTHROPIC_API_KEY set never hit the None branch. + # this matches the REST 503 semantics. Deploys reach the None branch + # whenever LLM_ENABLED is off (the default) or the key is unset. regenerate_run_debrief_tool.register( mcp, get_handler=lambda: _resolve_regenerate_run_debrief(get_handlers()), @@ -134,9 +134,15 @@ def _resolve_regenerate_run_debrief(handlers: AgentHandlers) -> RegenerateRunDeb """Dereference the Optional regenerate_run_debrief handler with a loud failure when unwired. Mirrors the REST route's 503 path.""" if handlers.regenerate_run_debrief is None: + # Names BOTH settings rather than deriving which one applies: + # this resolver has no Settings in scope (the tool registry is + # wired from handlers alone), and threading one through to word a + # message would widen the registration signature. The REST 503, + # which does have Settings, names the specific one. msg = ( - "regenerate_run_debrief is unavailable: kernel.llm is not wired " - "(ANTHROPIC_API_KEY not configured)." + "regenerate_run_debrief is unavailable: kernel.llm is not wired. " + "It requires BOTH LLM_ENABLED=true (default false) and " + "ANTHROPIC_API_KEY." ) raise RuntimeError(msg) return handlers.regenerate_run_debrief diff --git a/apps/api/src/cora/api/_readiness.py b/apps/api/src/cora/api/_readiness.py index 1cbd9b810d9..1859d1fc395 100644 --- a/apps/api/src/cora/api/_readiness.py +++ b/apps/api/src/cora/api/_readiness.py @@ -57,6 +57,23 @@ deployment has no pool at all (the in-memory kernel), so there is nothing to report rather than nothing wrong.""" +LlmReach = Literal["off", "live"] +"""Whether this deployment calls an external language model. + +`live` means both the switch (`llm_enabled`) and the credential +(`anthropic_api_key`) are present, so the LLM-backed subscribers are +registered and will call out on every terminal Run. `off` means no +external model is called through this seam. + +Named `llm`, NOT `egress`, and the distinction is the honest part. This +reports ONE outbound path. It is not a claim that nothing leaves the +deployment: `HttpRangeChecksumAdapter` is wired unconditionally for +http/https Distributions (`cora.data.wire`), so CORA can make outbound +requests with the LLM entirely off. Calling this field `egress` would +promise a perimeter this code does not enforce, which is exactly the +overclaim `derive_actuation` is careful to avoid on its own axis. +""" + ActuationReach = Literal["inert", "reachable"] """Whether this deployment can drive a physical effect on the beamline. @@ -194,6 +211,22 @@ def derive_actuation(settings: Settings) -> ActuationReach: return "inert" +def derive_llm(settings: Settings) -> LlmReach: + """Report whether an external language model gets called. + + `live` requires BOTH the switch and the credential, mirroring + `build_llm`'s two guards, so this answers the question an operator + actually has ("is CORA phoning out and spending?") rather than + restating one flag. A deployment that sets `llm_enabled` and forgets + the key reads `off`, which is the truth: nothing is called. + + Like `derive_actuation` this is a REPORT, not a gate. It decides + nothing; `build_llm` is the thing that refuses. And it is scoped to + the LLM seam alone, not to egress in general (see `LlmReach`). + """ + return "live" if settings.llm_enabled and settings.anthropic_api_key is not None else "off" + + def readiness_body(database: DatabaseStatus, settings: Settings) -> dict[str, str]: """Render the probe result. Fixed vocabulary, no free text. @@ -217,13 +250,16 @@ def readiness_body(database: DatabaseStatus, settings: Settings) -> dict[str, st "database": database, "app_env": settings.app_env, "actuation": derive_actuation(settings), + "llm": derive_llm(settings), } __all__ = [ "ActuationReach", "DatabaseStatus", + "LlmReach", "derive_actuation", + "derive_llm", "probe_database", "readiness_body", ] diff --git a/apps/api/src/cora/api/main.py b/apps/api/src/cora/api/main.py index ea0779ef441..9dc7205f25b 100644 --- a/apps/api/src/cora/api/main.py +++ b/apps/api/src/cora/api/main.py @@ -82,7 +82,12 @@ from cora.api._enclosure_permit_observer import ControlPortEnclosureObserver from cora.api._inference_recorder import DelegatingInferenceRecorder from cora.api._procedure_watcher import procedure_watcher_lifespan -from cora.api._readiness import derive_actuation, probe_database, readiness_body +from cora.api._readiness import ( + derive_actuation, + derive_llm, + probe_database, + readiness_body, +) from cora.api._run_initiator import run_initiator_lifespan from cora.api._run_supervisor import run_supervisor_lifespan from cora.api.middleware import BodySizeLimitMiddleware @@ -776,6 +781,17 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None]: cora_allow_raw_conduct=settings.cora_allow_raw_conduct, ) + # The egress + spend axis, logged beside actuation because a + # facility hears both as one promise. `anthropic_api_key_present` + # is a BOOLEAN, never the key: SecretStr keeps the value out of + # every serialisation path and this line must not undo that. + _log.info( + "boot.llm_posture", + llm=derive_llm(settings), + llm_enabled=settings.llm_enabled, + anthropic_api_key_present=settings.anthropic_api_key is not None, + ) + app.state.safety = wire_safety(deps) app.state.caution = wire_caution(deps) app.state.calibration = wire_calibration(deps) @@ -882,7 +898,8 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None]: register_agent_projections(registry, deps) # side-effecting Agent BC subscribers # (RunDebriefer). Conditional: only registered when - # `kernel.llm` is wired (ANTHROPIC_API_KEY configured). + # `kernel.llm` is wired, which needs LLM_ENABLED (default + # off) AND ANTHROPIC_API_KEY. register_agent_subscribers(registry, deps) # budget BC's deterministic CampaignClosed -> seal reaction; # unconditional (bookkeeping must not depend on an LLM key). diff --git a/apps/api/src/cora/infrastructure/config.py b/apps/api/src/cora/infrastructure/config.py index e6723e12a44..812b3ba5d37 100644 --- a/apps/api/src/cora/infrastructure/config.py +++ b/apps/api/src/cora/infrastructure/config.py @@ -165,17 +165,44 @@ class Settings(BaseSettings): # 0.1s prevents accidental tight-loop misconfiguration. projection_poll_interval_seconds: float = 5.0 + # `llm_enabled` is the SWITCH; `anthropic_api_key` below is the + # CREDENTIAL. Both are required before CORA calls an external model, + # and this one defaults OFF. + # + # Why a switch at all, when a key is already needed: the key is often + # present in an environment for an unrelated reason, and its mere + # presence used to be enough to auto-register RunDebriefer + + # CautionDrafter, which call an external API on EVERY terminal Run. + # That is experiment metadata leaving the facility (EGRESS) and money + # being spent (SPEND), switched on by a side effect. Every one of the + # ~13 sibling subscribers carries its own `*_enabled` default-off + # flag; this seam was the outlier. + # + # Egress and spend are a DIFFERENT axis from actuation (the + # ControlPort write gate and the ComputePort exec allowlist, see + # `control_writes_enabled`). A facility told "read-only, CORA cannot + # touch my hardware" hears a claim about actuation, and would still + # be unhappy to discover CORA phoned its data out. Both promises + # belong to the observe-only posture; neither subsumes the other. + # + # Turning this off is graceful, never a crash: `build_llm` returns + # None, and every consumer already treats `llm=None` as a supported + # state because the key-absent case always produced it. The + # LLM-backed subscribers log-and-skip and `regenerate_run_debrief` + # answers unavailable. The `llm` decide substrate is not selectable + # over the wire at all, so its factory guard is an internal-caller + # guard rather than a request-reachable path. + llm_enabled: bool = False + # LLM provider — Agent BC wiring - # When None, `build_kernel` wires no LLM and the Kernel - # carries `llm=None`; subscribers that depend on the LLM (the - # RunDebriefer subscriber) raise / log-and-skip when they - # try to use it. The dev / test default of None matches the - # `AllowAllAuthorize` / `AlwaysCoveredClearanceLookup` test- - # bypass convention: tests don't need real API credentials. - # Production deployments that ship RunDebriefer MUST set this; - # the wire-up adds a startup gate that refuses - # to register `run_debriefer_subscriber` when this is unset (so - # the agent never runs blind). + # When None (or when `llm_enabled` is False above), `build_kernel` + # wires no LLM and the Kernel carries `llm=None`; the LLM-backed + # subscribers log-and-skip rather than refusing to boot, so a + # deployment may defer Agent rollout. The dev / test default of None + # matches the `AllowAllAuthorize` / `AlwaysCoveredClearanceLookup` + # test-bypass convention: tests don't need real API credentials. + # Production deployments that ship RunDebriefer MUST set BOTH this + # and `llm_enabled`. # # Read from `ANTHROPIC_API_KEY` env var (case-insensitive per # pydantic-settings; matches the bare-field-name convention diff --git a/apps/api/src/cora/infrastructure/deps.py b/apps/api/src/cora/infrastructure/deps.py index 6b04d1cf7df..bc7cd7a1e56 100644 --- a/apps/api/src/cora/infrastructure/deps.py +++ b/apps/api/src/cora/infrastructure/deps.py @@ -333,9 +333,9 @@ def make_postgres_kernel( `llm` defaults to `None` because most BCs and tests don't need an LLM; only Agent BC subscribers consume it. Production's - `build_kernel` injects `AnthropicLLM` when - `Settings.anthropic_api_key` is set; subscriber-level tests - inject `FakeLLM` explicitly. + `build_kernel` injects `AnthropicLLM` when both + `Settings.llm_enabled` and `Settings.anthropic_api_key` are + set; subscriber-level tests inject `FakeLLM` explicitly. `logbook_mirror` defaults to `None`; no production implementor exists yet. Subscribers short-circuit on `None`. @@ -1056,7 +1056,8 @@ class LLMFactory(Protocol): Agent BC's `cora.agent.adapters.AnthropicLLM` is the only production factory today; `cora.api.main` binds it when - `Settings.anthropic_api_key` is set. Same factory-injection + BOTH `Settings.llm_enabled` (the switch, default off) and + `Settings.anthropic_api_key` (the credential) are set. Same factory-injection shape as `AuthorizeFactory` / `ClearanceLookupFactory` / `CautionLookupFactory` so `cora.infrastructure.deps` doesn't import from any BC (tach module rule: @@ -1066,9 +1067,11 @@ class LLMFactory(Protocol): factory can read provider-specific options (timeouts, max_retries, base URL overrides) without growing the factory surface every time. The factory's `__call__` returns `None` - when settings indicate no LLM should be wired (eg. - `anthropic_api_key` unset) so the Kernel ends up with - `llm=None` and Agent subscribers fail-fast at registration. + when settings indicate no LLM should be wired (the switch off, + or no credential) so the Kernel ends up with `llm=None`. The + LLM-backed Agent subscribers then log a warning and SKIP + registration; they do not fail-fast, so a deployment may defer + Agent rollout without refusing to boot. """ def __call__( diff --git a/apps/api/src/cora/infrastructure/kernel.py b/apps/api/src/cora/infrastructure/kernel.py index e79f3e8a110..90a921f7541 100644 --- a/apps/api/src/cora/infrastructure/kernel.py +++ b/apps/api/src/cora/infrastructure/kernel.py @@ -333,9 +333,10 @@ class Kernel: `llm`: optional LLM-chat port consumed by Agent BC subscribers (RunDebriefer, CautionDrafter). Production wires - `AnthropicLLM` when `Settings.anthropic_api_key` is set; - otherwise this is `None` and subscribers that depend on it must - short-circuit or fail fast at registration time. Tests use + `AnthropicLLM` when both `Settings.llm_enabled` and + `Settings.anthropic_api_key` are set; otherwise this is `None` + and the subscribers that depend on it skip registration with a + warning. Tests use `FakeLLM` (zero network) when an LLM is needed and leave this `None` otherwise. diff --git a/apps/api/tests/architecture/test_egress_defaults_fail_closed.py b/apps/api/tests/architecture/test_egress_defaults_fail_closed.py new file mode 100644 index 00000000000..ab3774a7e6e --- /dev/null +++ b/apps/api/tests/architecture/test_egress_defaults_fail_closed.py @@ -0,0 +1,30 @@ +"""The egress + spend default ships SAFE: a blank deployment calls no model. + +Sibling to `test_actuation_defaults_fail_closed`, for the other axis. That +one pins the defaults that keep CORA from moving hardware; this one pins the +default that keeps experiment metadata inside the facility and the token bill +at zero. + +`llm_enabled` defaults False, so the LLM-backed subscribers (RunDebriefer, +CautionDrafter) do not register and nothing calls an external model. The flag +exists because the credential alone used to be enough: an `ANTHROPIC_API_KEY` +present in the environment for an unrelated reason silently switched on +outbound calls on EVERY terminal Run. Every one of the ~13 sibling +subscribers already carried its own default-off flag; this seam was the +outlier. + +## Why the DECLARED default, not an instantiated Settings + +Reading `model_fields[...].default` asserts what SHIPS, immune to whatever +the process environment happens to hold. `Settings()` would only tell us +about this shell. +""" + +import pytest + +from cora.infrastructure.config import Settings + + +@pytest.mark.architecture +def test_llm_default_is_off() -> None: + assert Settings.model_fields["llm_enabled"].default is False diff --git a/apps/api/tests/contract/test_readyz_endpoint.py b/apps/api/tests/contract/test_readyz_endpoint.py index 5d819e11fa3..1c95422b6f9 100644 --- a/apps/api/tests/contract/test_readyz_endpoint.py +++ b/apps/api/tests/contract/test_readyz_endpoint.py @@ -13,6 +13,7 @@ import pytest from fastapi.testclient import TestClient +from pydantic import SecretStr import cora.api.main as api_main from cora.api.main import create_app @@ -83,3 +84,29 @@ def test_readyz_is_not_in_the_openapi_schema() -> None: with TestClient(create_app()) as client: spec = client.get("/openapi.json").json() assert "/readyz" not in spec["paths"] + + +@pytest.mark.contract +def test_readyz_reports_llm_off_for_a_deployment_that_calls_no_model() -> None: + """The egress + spend claim, curl-able beside the actuation one. + + A credential is injected to prove the SWITCH is what decides: this + app has a key and still reports `off`. + """ + llm_off = Settings( + app_env="test", llm_enabled=False, anthropic_api_key=SecretStr("sk-test-fake") + ) + with TestClient(create_app(settings=llm_off)) as client: + body = client.get("/readyz").json() + assert body["llm"] == "off" + + +@pytest.mark.contract +def test_readyz_reports_llm_live_when_switched_on_with_a_credential() -> None: + """The other direction, so a hardcoded `off` cannot pass.""" + llm_live = Settings( + app_env="test", llm_enabled=True, anthropic_api_key=SecretStr("sk-test-fake") + ) + with TestClient(create_app(settings=llm_live)) as client: + body = client.get("/readyz").json() + assert body["llm"] == "live" diff --git a/apps/api/tests/unit/agent/test_agent_subscribers_registration.py b/apps/api/tests/unit/agent/test_agent_subscribers_registration.py index 9bfce86d943..facb4c0b188 100644 --- a/apps/api/tests/unit/agent/test_agent_subscribers_registration.py +++ b/apps/api/tests/unit/agent/test_agent_subscribers_registration.py @@ -5,6 +5,7 @@ from datetime import UTC, datetime import pytest +import structlog.testing from cora.agent import register_agent_subscribers from cora.infrastructure.config import Settings @@ -18,8 +19,16 @@ from cora.infrastructure.projection.registry import ProjectionRegistry -def _kernel(*, llm: object | None, caution_promoter_enabled: bool = False) -> object: - settings = Settings(caution_promoter_enabled=caution_promoter_enabled) # type: ignore[call-arg] +def _kernel( + *, + llm: object | None, + caution_promoter_enabled: bool = False, + llm_enabled: bool = False, +) -> object: + settings = Settings( # type: ignore[call-arg] + caution_promoter_enabled=caution_promoter_enabled, + llm_enabled=llm_enabled, + ) return make_inmemory_kernel( settings=settings, clock=FakeClock(datetime(2026, 5, 17, 14, 0, 0, tzinfo=UTC)), @@ -41,9 +50,9 @@ def test_registers_run_debrief_when_llm_configured() -> None: @pytest.mark.unit def test_skips_run_debrief_when_llm_is_none() -> None: - """If ANTHROPIC_API_KEY is unset, the subscriber would crash on - every apply() (no LLM to call). Skip registration cleanly with - a warning rather than crash at app boot.""" + """With no LLM wired (the switch off, or no credential), the + subscriber would crash on every apply(). Skip registration cleanly + with a warning rather than crash at app boot.""" registry = ProjectionRegistry() kernel = _kernel(llm=None) @@ -119,3 +128,35 @@ def test_registration_is_idempotent_safe_for_one_registry() -> None: with pytest.raises(DuplicateProjectionError): register_agent_subscribers(registry, kernel) # type: ignore[arg-type] + + +def _skip_reason(kernel: object) -> str: + """The `reason` field of the LLM-subscriber skip warning.""" + with structlog.testing.capture_logs() as captured: + register_agent_subscribers(ProjectionRegistry(), kernel) # type: ignore[arg-type] + skips = [entry for entry in captured if entry.get("event") == "agent_subscriber.skipped"] + assert len(skips) == 1, f"expected exactly one skip warning, got {skips}" + return str(skips[0]["reason"]) + + +@pytest.mark.unit +def test_skip_warning_names_the_switch_when_the_llm_is_turned_off() -> None: + """The two causes call for opposite remedies, so the warning must + distinguish them. A deployment that deliberately runs LLM-off should + not read a warning telling it a credential is missing.""" + reason = _skip_reason(_kernel(llm=None, llm_enabled=False)) + + # Names the switch as the CAUSE. It may still mention the key as part + # of the remedy (turning the LLM on needs both), but it must not tell + # a deliberately-off deployment that its credential is missing. + assert "LLM_ENABLED is false" in reason + assert "ANTHROPIC_API_KEY is not configured" not in reason + + +@pytest.mark.unit +def test_skip_warning_names_the_credential_when_the_switch_is_on() -> None: + """Switched on but unconfigured is a real misconfiguration; say so.""" + reason = _skip_reason(_kernel(llm=None, llm_enabled=True)) + + assert "ANTHROPIC_API_KEY is not configured" in reason + assert "LLM_ENABLED is true" in reason diff --git a/apps/api/tests/unit/api/test_readiness.py b/apps/api/tests/unit/api/test_readiness.py index f2f4ae8da8c..c0b43b5aa7a 100644 --- a/apps/api/tests/unit/api/test_readiness.py +++ b/apps/api/tests/unit/api/test_readiness.py @@ -17,8 +17,14 @@ import asyncpg import pytest - -from cora.api._readiness import derive_actuation, probe_database, readiness_body +from pydantic import SecretStr + +from cora.api._readiness import ( + derive_actuation, + derive_llm, + probe_database, + readiness_body, +) from cora.infrastructure.config import Settings @@ -171,7 +177,13 @@ def test_readiness_body_carries_app_env() -> None: @pytest.mark.unit def test_readiness_body_leaks_no_deployment_detail() -> None: """Unauthenticated endpoint: fixed vocabulary, no free text, no topology.""" - assert set(readiness_body("ok", _settings())) == {"status", "database", "app_env", "actuation"} + assert set(readiness_body("ok", _settings())) == { + "status", + "database", + "app_env", + "actuation", + "llm", + } def _actuation_settings(*, writes: bool, substrate: str) -> Settings: @@ -224,3 +236,50 @@ def test_readiness_body_actuation_is_independent_of_database_status() -> None: body = readiness_body("unreachable", _actuation_settings(writes=False, substrate="in_memory")) assert body["status"] == "not_ready" assert body["actuation"] == "inert" + + +def _llm_settings(*, enabled: bool, key: str | None) -> Settings: + """Settings with both LLM inputs pinned; init kwargs beat the env.""" + return Settings( + app_env="test", + llm_enabled=enabled, + anthropic_api_key=SecretStr(key) if key is not None else None, + ) + + +@pytest.mark.unit +def test_derive_llm_reports_off_when_the_switch_is_off() -> None: + """The default posture: no external model is called.""" + assert derive_llm(_llm_settings(enabled=False, key=None)) == "off" + + +@pytest.mark.unit +def test_derive_llm_reports_off_when_disabled_despite_a_credential() -> None: + """A key present for an unrelated reason must not read as live.""" + assert derive_llm(_llm_settings(enabled=False, key="sk-test-fake")) == "off" + + +@pytest.mark.unit +def test_derive_llm_reports_off_when_enabled_without_a_credential() -> None: + """Switched on but nothing to call with: the truth is still `off`.""" + assert derive_llm(_llm_settings(enabled=True, key=None)) == "off" + + +@pytest.mark.unit +def test_derive_llm_reports_live_when_switched_on_with_a_credential() -> None: + assert derive_llm(_llm_settings(enabled=True, key="sk-test-fake")) == "live" + + +@pytest.mark.unit +def test_readiness_body_reports_the_derived_llm_posture() -> None: + off = readiness_body("ok", _llm_settings(enabled=False, key="sk-test-fake")) + assert off["llm"] == "off" + live = readiness_body("ok", _llm_settings(enabled=True, key="sk-test-fake")) + assert live["llm"] == "live" + + +@pytest.mark.unit +def test_readiness_body_never_carries_the_api_key() -> None: + """The body is unauthenticated; a credential must not ride in it.""" + body = readiness_body("ok", _llm_settings(enabled=True, key="sk-ant-secret-VALUE")) + assert "sk-ant-secret-VALUE" not in str(body) diff --git a/apps/api/tests/unit/test_deps.py b/apps/api/tests/unit/test_deps.py index b15e6ccd01d..c43956970a0 100644 --- a/apps/api/tests/unit/test_deps.py +++ b/apps/api/tests/unit/test_deps.py @@ -138,25 +138,44 @@ def test_build_llm_returns_none_when_api_key_unset( monkeypatch: pytest.MonkeyPatch, ) -> None: """Agent BC's LLMFactory short-circuits cleanly when no - credential is configured; subscriber registration handles the - fail-fast at subscriber registration.""" + credential is configured; the LLM-backed subscribers then + log-and-skip rather than refusing to boot.""" monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.setenv("LLM_ENABLED", "true") settings = Settings() # type: ignore[call-arg] assert build_llm(settings) is None @pytest.mark.unit -def test_build_llm_returns_anthropic_adapter_when_api_key_set( +def test_build_llm_returns_anthropic_adapter_when_enabled_and_api_key_set( monkeypatch: pytest.MonkeyPatch, ) -> None: - """When the operator wires ANTHROPIC_API_KEY, the production - AnthropicLLM lands in the Kernel.""" + """The production AnthropicLLM lands in the Kernel only when the + operator wires BOTH the switch and the credential.""" + monkeypatch.setenv("LLM_ENABLED", "true") monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-test-fake") settings = Settings() # type: ignore[call-arg] llm = build_llm(settings) assert isinstance(llm, AnthropicLLM) +@pytest.mark.unit +def test_build_llm_returns_none_when_disabled_despite_api_key( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The switch beats the credential, which is the whole point. + + An API key is often present in an environment for an unrelated + reason. Before the switch existed its mere presence auto-registered + the LLM-backed subscribers, so experiment metadata left the facility + and tokens were spent on every terminal Run because of a side effect. + """ + monkeypatch.delenv("LLM_ENABLED", raising=False) + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-test-fake") + settings = Settings() # type: ignore[call-arg] + assert build_llm(settings) is None + + @pytest.mark.unit def test_anthropic_api_key_is_secret_str_and_redacted_in_repr( monkeypatch: pytest.MonkeyPatch, diff --git a/docs/architecture/modules/agent/index.md b/docs/architecture/modules/agent/index.md index 556e50bb921..e83eceb64f6 100644 --- a/docs/architecture/modules/agent/index.md +++ b/docs/architecture/modules/agent/index.md @@ -36,7 +36,7 @@ The runtime that drives an agent takes one of three host shapes: | ProcedureWatcher | deterministic | periodic loop | writes a `ProcedureProgress` (Stall) flag Decision (passive) | | CampaignWatcher | deterministic | periodic loop | writes a `CampaignProgress` (Stuck) flag Decision (passive) | -The active runtimes (RunSupervisor, CautionPromoter, ClearanceExpirer) ship off by default, gate every actuation through the Authorize port like any principal, and stand down the moment their Actor is deactivated. None of them reaches past the spine onto the real-time floor: an active agent only issues a command the spine already exposes. ClearanceWatcher, CalibrationWatcher, ProcedureWatcher, and CampaignWatcher are passive (each records a flag Decision and issues no command) and likewise ship off by default. RunSupervisor additionally carries shadow observe-only rules (run-liveness, plus signal-quality and signal-stall against a live Run's observation channels) that log a would-flag and take no further action; each is a separate opt-in above the agent's own enable, and advise / act promotions are deferred. +The active runtimes (RunSupervisor, CautionPromoter, ClearanceExpirer) ship off by default, gate every actuation through the Authorize port like any principal, and stand down the moment their Actor is deactivated. None of them reaches past the spine onto the real-time floor: an active agent only issues a command the spine already exposes. ClearanceWatcher, CalibrationWatcher, ProcedureWatcher, and CampaignWatcher are passive (each records a flag Decision and issues no command) and likewise ship off by default. The two LLM-backed reactions (RunDebriefer, CautionDrafter) also ship off by default, behind `LLM_ENABLED`: they are the seam that would send experiment metadata to an external model and spend on it, so the switch and a credential are both required before either registers. RunSupervisor additionally carries shadow observe-only rules (run-liveness, plus signal-quality and signal-stall against a live Run's observation channels) that log a would-flag and take no further action; each is a separate opt-in above the agent's own enable, and advise / act promotions are deferred.
@@ -297,7 +297,7 @@ The four examples below follow the canonical path for one Agent: define it (atom } ``` - Triggers a fresh RunDebriefer invocation against the named Run and returns `201 Created` with the new `decision_id`. When `parent_decision_id` is supplied, the new Decision links back to the prior debrief via PROV-O `wasInformedBy`. Returns `503 Service Unavailable` when the LLM adapter is not wired (development environment without an API key). + Triggers a fresh RunDebriefer invocation against the named Run and returns `201 Created` with the new `decision_id`. When `parent_decision_id` is supplied, the new Decision links back to the prior debrief via PROV-O `wasInformedBy`. Returns `503 Service Unavailable` when the LLM adapter is not wired (`LLM_ENABLED` off, the default, or no API key); the body names which one applies. === "MCP" diff --git a/docs/reference/layout.md b/docs/reference/layout.md index 4439bc34a02..7b8643800fd 100644 --- a/docs/reference/layout.md +++ b/docs/reference/layout.md @@ -86,7 +86,7 @@ Private `_*.py` modules stay flat at the BC root by default; the naming prefix ( Both re-export their public surface, so consumers import from the package (`from cora.equipment._pidinst import PidinstRecord`), not the submodules. The canonical shared-pattern files (`_bootstrap.py`, `_projections.py`, `__update_handler.py`) stay flat for cross-BC consistency. Grouping the DTOs by aggregate was rejected: the shared VOs have no single aggregate owner, so per-aggregate files would force false ownership. -**Capability-dependent handlers.** When a slice depends on an external capability that may be unwired in some deployments (today: `regenerate_run_debrief` needs `kernel.llm`, which is `None` when `ANTHROPIC_API_KEY` isn't configured), the handler bundle types the field as `Handler | None`. The route guards on `None` and raises `HTTPException(503)` inline; this is the only documented exception to the "command-slice routes don't wrap handler calls" rule. Pinned by `test_route_no_inline_http_exception.py`'s `GRANDFATHERED_COMMAND_ROUTES` allowlist. +**Capability-dependent handlers.** When a slice depends on an external capability that may be unwired in some deployments (today: `regenerate_run_debrief` needs `kernel.llm`, which is `None` unless both `LLM_ENABLED` and `ANTHROPIC_API_KEY` are set), the handler bundle types the field as `Handler | None`. The route guards on `None` and raises `HTTPException(503)` inline; this is the only documented exception to the "command-slice routes don't wrap handler calls" rule. Pinned by `test_route_no_inline_http_exception.py`'s `GRANDFATHERED_COMMAND_ROUTES` allowlist. ### Aggregate-internal shared modules diff --git a/docs/stack/deployment.md b/docs/stack/deployment.md index 77ed5b8027a..bbeee2a5999 100644 --- a/docs/stack/deployment.md +++ b/docs/stack/deployment.md @@ -13,7 +13,8 @@ The load-bearing auth vars (full list in `.env.example`): | `REQUIRE_AUTHENTICATED_PRINCIPAL` | `false` | Must be `true` whenever `TRUST_POLICY_ID` is set, and in any production-tier env (the boot gate refuses otherwise; see below) | | `ALLOW_PERMISSIVE_AUTHZ` | `false` | Production-tier escape hatch: set `true` to run the permit-everyone `AllowAllAuthorize` stub on purpose in a `prod`/`production`/`staging` env (airgapped / single-operator pilot) | | `IDENTITY_PROVIDERS` | unset → legacy `X-Principal-Id` header mode | JSON list of `IdentityProviderConfig` entries (see [Auth](auth.md)); enables bearer-token mode at the HTTP edge | -| `ANTHROPIC_API_KEY` | unset → AI subscribers log-and-skip | When you want RunDebriefer / CautionDrafter live | +| `LLM_ENABLED` | `false` → CORA calls no external model | The switch for the egress + spend axis. Required (with the key) for RunDebriefer / CautionDrafter | +| `ANTHROPIC_API_KEY` | unset → AI subscribers skipped | The credential. Setting it alone changes nothing; `LLM_ENABLED` decides | | `CONTROL_WRITES_ENABLED` | `false` → CORA never drives through the ControlPort | Set `true` only when this deployment is meant to actuate hardware (see below; it does not cover `COMPUTE_SUBSTRATE`) | | `CONTROL_PORT_ROUTES` | unset → `InMemoryControlPort` (no real substrate) | When CORA talks to a real control system. JSON list of `ControlPortRoute` | @@ -169,8 +170,8 @@ every pod into an outage the restart cannot mend, converting one database blip into a fleet-wide crash loop. `/readyz` returns 200 `{"status": "ready", ...}` or 503 -`{"status": "not_ready", "database": "..."}`, plus an `actuation` field -on both (see below). `database` is a fixed vocabulary: `ok`, +`{"status": "not_ready", "database": "..."}`, plus `actuation` and `llm` +fields on both (see below). `database` is a fixed vocabulary: `ok`, `unreachable`, `saturated`, `closing`, `skipped` (this deployment has no pool, the in-memory kernel), `error`. The body carries no URLs, no driver error text, and no projection or bounded context names: it is @@ -216,15 +217,45 @@ prints the raw inputs (`control_writes_enabled`, `compute_substrate`, count) beside the summary so it can be audited against its own sources. Scope, stated so it is not mistaken for more: `actuation` covers the -hardware axis only. Sending data OUT (the LLM seam, remote transfers) -and SPENDING money are separate promises with their own switches; when -they gain summaries they slot in beside `actuation` rather than widening -its meaning. Today the outbound transfer and remote-compute adapters -exist but are wired nowhere, and a fitness test -(`test_dormant_outbound_seams_unwired`) fails the build if one gets -constructed without the actuation derivation being revisited. The -trigger to build the spanning egress/spend summary is the first of those -paths that gets wired at the composition root. +hardware axis only. Sending data OUT and SPENDING money are separate +promises, and they now have a switch and a summary of their own. + +### The egress and spend axis + +`LLM_ENABLED` (default `false`) is the switch; `ANTHROPIC_API_KEY` is the +credential. Both are required before CORA calls an external model, and +`/readyz` reports the effective state as `llm`, either `off` or `live`, +with a matching `boot.llm_posture` line at startup. + +**Upgrading:** a deployment that ran the LLM subscribers on +`ANTHROPIC_API_KEY` alone must now also set `LLM_ENABLED=true`, or those +two subscribers stop registering. Nothing breaks and nothing crashes; a +boot warning names the switch, and `GET /readyz` reports `"llm": "off"`. + +The switch exists because the credential alone used to be enough. A key +present in the environment for an unrelated reason silently registered +the RunDebriefer and CautionDrafter subscribers, which call an external +API on every terminal Run: experiment metadata leaving the facility, and +money spent, because of a side effect. Every sibling subscriber already +carried its own default-off flag; this seam was the outlier. + +Turning it off is graceful. The two LLM-backed subscribers are skipped +with a warning naming which of the two settings is missing, +`regenerate_run_debrief` answers unavailable, and a conduct command that +explicitly asks for the `llm` decide substrate gets a 422 at +construction, before any FSM transition. + +Read `llm` narrowly: it reports ONE outbound path, not a perimeter. +CORA's HTTP checksum adapter is wired unconditionally for http/https +Distributions, so the process can still make outbound requests with the +LLM entirely off. That is why the field is named `llm` and not `egress`. + +Two adapter families remain dormant on this axis: the outbound transfer +adapters and the remote-compute adapter exist but are wired nowhere, and +a fitness test (`test_dormant_outbound_seams_unwired`) fails the build if +one gets constructed without the derivation being revisited. The trigger +to build a genuinely spanning egress summary is the first of those paths +that gets wired at the composition root. **Probe budget invariant.** The app bounds `/readyz` at 1.5s total. Set the orchestrator's own probe timeout ABOVE that (2s or more).