From 15d69799bf68510fbdaa2b41f68f91cc38808052 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Wed, 23 Sep 2026 23:25:58 +0000 Subject: [PATCH] Make the config file the only home for settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With no production instance yet, there is no reason to carry two ways of configuring the agent. Every environment variable that duplicated a config key is gone; the environment now supplies only the secrets the file references by ${…}, plus NISSE_CONFIG and NISSE_ENV to find the files. - AgentConfig loads from a file (load, from_file) or a parsed tree (from_tree, for tests); a missing file is an error naming the example - The from_env constructors on LiteLLMClient, AnthropicPassthroughClient, GiteaClient, MatrixConfig and RoleMap are removed, with the env reading for max output tokens; clients take their settings from config - The live integration tests share a committed agent config, tests/integration/nisse-test.yaml, aimed at test resources, and one conftest that loads it and builds clients. NISSE_LIVE_PLANNER_MODEL becomes its planner role; NISSE_CAPABILITY_MODELS stays as a per-run switch - A missing secret now fails the live tests instead of skipping them - CI passes only the three tokens - .env.example is secrets only; spec A5 says config comes from the file Co-Authored-By: Claude Opus 5.5 (1M context) --- .env.example | 70 +--- .gitea/workflows/ci.yml | 14 +- docs/Architecture-Build-Spec.md | 6 +- docs/Tuning-Guide.md | 1 - nisse-dev.example.yaml | 38 +- nisse-dev/src/nisse_dev/catalog.py | 4 +- nisse-dev/src/nisse_dev/config.py | 105 ++--- nisse-dev/src/nisse_dev/gitea.py | 10 - nisse-dev/src/nisse_dev/llm.py | 55 +-- nisse-dev/src/nisse_dev/matrix.py | 27 +- nisse-dev/tests/test_agent_build.py | 25 +- nisse-dev/tests/test_config.py | 460 ++++++++++----------- nisse-dev/tests/test_dev_main.py | 16 +- nisse-dev/tests/test_gitea.py | 5 - nisse-dev/tests/test_llm.py | 34 +- nisse-dev/tests/test_matrix.py | 17 - tests/integration/conftest.py | 62 +++ tests/integration/nisse-test.yaml | 40 ++ tests/integration/test_delegation_live.py | 38 +- tests/integration/test_entry_points.py | 17 +- tests/integration/test_expert_live.py | 38 +- tests/integration/test_gitea_live.py | 33 +- tests/integration/test_llm_live.py | 40 +- tests/integration/test_matrix_live.py | 39 +- tests/integration/test_milestone_live.py | 30 +- tests/integration/test_model_capability.py | 44 +- tests/integration/test_passthrough_live.py | 45 +- tests/integration/test_planning_live.py | 35 +- 28 files changed, 571 insertions(+), 777 deletions(-) create mode 100644 tests/integration/conftest.py create mode 100644 tests/integration/nisse-test.yaml diff --git a/.env.example b/.env.example index 97cc184..4052e03 100644 --- a/.env.example +++ b/.env.example @@ -1,64 +1,18 @@ -# Copy to .env (chmod 600). Real environment variables win over this file. -# .env is gitignored; never commit real values. +# Secrets only. Copy to .env (chmod 600); it is gitignored, so never commit +# real values. Real environment variables win over this file. # -# Settings can also live in a YAML file (nisse-dev.example.yaml), which keeps -# structure and the tuning knobs; secrets stay here and are referenced from -# the file as ${NISSE_LLM_TOKEN} and so on. Every variable below overrides the -# matching key in that file. +# Everything else lives in the YAML config (see nisse-dev.example.yaml), +# which refers to these by name, e.g. `token: ${NISSE_LLM_TOKEN}`. The live +# integration tests use the same names, from tests/integration/nisse-test.yaml. -# --- LLM access (A5): the LiteLLM proxy is the single LLM destination --- -NISSE_LLM_BASE_URL=http://litellm.example:4000 NISSE_LLM_TOKEN=sk-your-litellm-virtual-key - -# --- Role mapping (A11): roles resolve to concrete model strings --- -NISSE_ROLE_EXECUTOR=anthropic/claude-haiku-4-5-20251001 -NISSE_ROLE_PLANNER=anthropic/claude-opus-5 -NISSE_ROLE_DELEGATE=anthropic/claude-opus-5 -NISSE_ROLE_CONSULTANT=anthropic/claude-opus-5 - -# Opt-in for the live planning test: a model the virtual key can reach that -# holds the tool-call contract. Planning escalates unconditionally (A12), and -# small local models do not hold the contract reliably — llama3.2 returns -# truncated tool arguments, Haiku-class and up are solid. -NISSE_LIVE_PLANNER_MODEL=claude-haiku-4-5 - -# --- Matrix transport (A1), used by nisse-dev's MatrixTransport --- -NISSE_MATRIX_HOMESERVER=https://matrix.example -NISSE_MATRIX_USER_ID=@nisse-dev:example NISSE_MATRIX_TOKEN=syt_your_matrix_access_token -NISSE_MATRIX_ROOM_ID=!roomid:example -# Dedicated room for the live integration test; never the production room. -NISSE_MATRIX_TEST_ROOM_ID=!testroomid:example -NISSE_MATRIX_OPERATORS=@you:example - -# --- Gitea (B2): service account for branches, PRs, and comment intake --- -NISSE_GITEA_URL=https://gitea.example NISSE_GITEA_TOKEN=your-scoped-service-account-token -# Sandbox repo for live tests; never the real project repo. -NISSE_GITEA_TEST_REPO=owner/sandbox-repo -# Gitea logins allowed to drive the agent via PR comments (A4 per-channel -# allowlist; a Matrix MXID and a Gitea login are different names). -NISSE_GITEA_OPERATORS=your-gitea-login -# --- Running the agent (the demo, not the tests) --- -# The tests use NISSE_GITEA_TEST_REPO; a running agent uses these. -# -# Layout. NISSE_WORKSPACE is the *clone the agent works in*, not the directory -# the agent runs from — they must not be the same place: -# -# ~/nisse-demo-workspace/ deployment root, not a git repo -# ├── .env this file -# ├── state.db the A2 state store -# └── project/ NISSE_WORKSPACE, the clone -# -# The agent commits its workspace with `git add -A` and pushes it, so anything -# inside the clone reaches the project repository. A .env or state store placed -# there would publish credentials as a side effect of the agent working -# correctly; startup refuses to proceed if either is inside the workspace. -NISSE_STATE_DB=~/nisse-demo-workspace/state.db -NISSE_WORKSPACE=~/nisse-demo-workspace/project -NISSE_PROJECT=demo -NISSE_GITEA_REPO=owner/demo-repo -# NISSE_BASE_BRANCH=main -# Path to this file, if not ./.env -# NISSE_ENV=~/nisse-demo-workspace/.env +# Where the config file is, if not ./nisse-dev.yaml. Where this file is, if +# not ./.env, is NISSE_ENV, which has to be set in the real environment. +# NISSE_CONFIG=~/nisse-demo-workspace/nisse-dev.yaml + +# Opt-in for the capability matrix: models to measure, comma separated. +# Spends real tokens across every model listed. See docs/Tuning-Guide.md. +# NISSE_CAPABILITY_MODELS=local-fast,local-coder diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 6f7d82c..07030d1 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -34,19 +34,11 @@ jobs: run: uv run pytest -m "not integration" - name: Integration tests - # Live-path credentials come from repo Actions secrets; the tests - # skip-guard on these, so a missing secret or unreachable service - # degrades to "skipped", never a failure on unrelated work. + # Everything but the secrets is in tests/integration/nisse-test.yaml, + # which refers to these by name. A missing secret fails the live + # tests that need it; it is never quietly turned into a skip. env: - NISSE_LLM_BASE_URL: ${{ secrets.NISSE_LLM_BASE_URL }} NISSE_LLM_TOKEN: ${{ secrets.NISSE_LLM_TOKEN }} - NISSE_ROLE_EXECUTOR: ${{ secrets.NISSE_ROLE_EXECUTOR }} - NISSE_LIVE_PLANNER_MODEL: ${{ secrets.NISSE_LIVE_PLANNER_MODEL }} - NISSE_MATRIX_HOMESERVER: ${{ secrets.NISSE_MATRIX_HOMESERVER }} - NISSE_MATRIX_USER_ID: ${{ secrets.NISSE_MATRIX_USER_ID }} NISSE_MATRIX_TOKEN: ${{ secrets.NISSE_MATRIX_TOKEN }} - NISSE_MATRIX_TEST_ROOM_ID: ${{ secrets.NISSE_MATRIX_TEST_ROOM_ID }} - NISSE_GITEA_URL: ${{ secrets.NISSE_GITEA_URL }} NISSE_GITEA_TOKEN: ${{ secrets.NISSE_GITEA_TOKEN }} - NISSE_GITEA_TEST_REPO: ${{ secrets.NISSE_GITEA_TEST_REPO }} run: uv run pytest -m integration -rs diff --git a/docs/Architecture-Build-Spec.md b/docs/Architecture-Build-Spec.md index 405ba34..d7ce157 100644 --- a/docs/Architecture-Build-Spec.md +++ b/docs/Architecture-Build-Spec.md @@ -389,8 +389,10 @@ Requirements: - **No vendor credentials on the agent VM.** LiteLLM holds every key, sourced from Bitwarden machine accounts (A7); the agent holds one LiteLLM token. Adding a vendor does not touch the agent VM, and a compromised agent leaks no provider keys. -- Config from env/`.env`, **nothing hardcoded**: base URL, auth token, and the role - mapping (A11). Changing any of it is an env change + restart, nothing more. +- Config from the agent's config file, **nothing hardcoded**: base URL, auth token, + and the role mapping (A11). Secrets in it are references to environment + variables, never values. Changing any of it is a config edit + restart, + nothing more. - **Budget caps and spend alerts live on the LiteLLM virtual key**, not in agent code. ### A6. Browser Tool diff --git a/docs/Tuning-Guide.md b/docs/Tuning-Guide.md index d91a22d..ef49dea 100644 --- a/docs/Tuning-Guide.md +++ b/docs/Tuning-Guide.md @@ -131,7 +131,6 @@ role. executor. Frontier planning wants the large default. This one knob covers every role for now; a per-model setting is coming with the context budget (see *Coming next*). -- Environment override: `NISSE_MAX_OUTPUT_TOKENS`. ### `tuning.executor.max_turns` — default 24 diff --git a/nisse-dev.example.yaml b/nisse-dev.example.yaml index 856b8ba..8f61011 100644 --- a/nisse-dev.example.yaml +++ b/nisse-dev.example.yaml @@ -2,44 +2,42 @@ # NISSE_CONFIG at your copy) and edit. # # - Structure lives here. Secrets do not: write them as ${VARIABLE} references, -# resolved from the environment (or .env) at startup. -# - Environment variables win. Every NISSE_* variable in .env.example -# overrides the key noted beside it here. +# resolved from the environment (or .env, see .env.example) at startup. # - Keep this file outside the workspace clone. Startup refuses otherwise, # because the agent commits and pushes everything in the clone. # - Unknown keys are errors, so a misspelt knob fails at startup instead of # silently doing nothing. -state_db: ~/nisse-demo-workspace/state.db # NISSE_STATE_DB -workspace: ~/nisse-demo-workspace/project # NISSE_WORKSPACE, the clone -project: demo # NISSE_PROJECT +state_db: ~/nisse-demo-workspace/state.db +workspace: ~/nisse-demo-workspace/project # the clone +project: demo llm: # A5: LiteLLM is the only LLM destination - base_url: http://litellm.example:4000 # NISSE_LLM_BASE_URL + base_url: http://litellm.example:4000 token: ${NISSE_LLM_TOKEN} # Ceiling on each response. Must fit inside a local model's context window # alongside the prompt; see docs/Tuning-Guide.md. - max_output_tokens: 16384 # NISSE_MAX_OUTPUT_TOKENS + max_output_tokens: 16384 roles: # A11: role -> LiteLLM model alias - executor: local-fast # NISSE_ROLE_EXECUTOR - planner: claude-opus-5 # NISSE_ROLE_PLANNER - delegate: claude-opus-5 # NISSE_ROLE_DELEGATE - consultant: claude-opus-5 # NISSE_ROLE_CONSULTANT + executor: local-fast + planner: claude-opus-5 + delegate: claude-opus-5 + consultant: claude-opus-5 matrix: # A1 - homeserver: https://matrix.example # NISSE_MATRIX_HOMESERVER - user_id: "@nisse-dev:example" # NISSE_MATRIX_USER_ID + homeserver: https://matrix.example + user_id: "@nisse-dev:example" token: ${NISSE_MATRIX_TOKEN} - room_id: "!roomid:example" # NISSE_MATRIX_ROOM_ID - operators: ["@you:example"] # NISSE_MATRIX_OPERATORS (comma-separated) + room_id: "!roomid:example" + operators: ["@you:example"] gitea: # B2 - url: https://gitea.example # NISSE_GITEA_URL + url: https://gitea.example token: ${NISSE_GITEA_TOKEN} - repo: owner/demo-repo # NISSE_GITEA_REPO - operators: [your-gitea-login] # NISSE_GITEA_OPERATORS (comma-separated) - base_branch: main # NISSE_BASE_BRANCH + repo: owner/demo-repo + operators: [your-gitea-login] + base_branch: main # What each knob does, and how to choose a value: docs/Tuning-Guide.md. # These are the defaults; the section can be left out entirely. The file is diff --git a/nisse-dev/src/nisse_dev/catalog.py b/nisse-dev/src/nisse_dev/catalog.py index ce22d9d..175684d 100644 --- a/nisse-dev/src/nisse_dev/catalog.py +++ b/nisse-dev/src/nisse_dev/catalog.py @@ -121,8 +121,8 @@ class ModelCatalog: Reported together rather than one per boot: an operator fixing a config wants the whole list, not to rediscover the next typo on the - next start (the same reason ``AgentConfig.from_env`` batches its - missing variables). + next start (the same reason ``AgentConfig`` reports every missing + setting at once). """ unknown = { role: alias for role, alias in roles.items() if alias not in self._facts diff --git a/nisse-dev/src/nisse_dev/config.py b/nisse-dev/src/nisse_dev/config.py index 68dbb88..03829f0 100644 --- a/nisse-dev/src/nisse_dev/config.py +++ b/nisse-dev/src/nisse_dev/config.py @@ -1,14 +1,11 @@ """Agent configuration (spec A5: nothing hardcoded). -Configuration comes from a YAML file plus the environment: - -- **The file carries structure.** Roles, connections, and the tuning knobs - (see docs/Tuning-Guide.md). It holds no secrets: a credential is written as - a reference, ``token: ${NISSE_LLM_TOKEN}``, and resolved from the - environment, so the file can be reviewed or shared without leaking. -- **The environment wins.** Every setting that existed as an environment - variable still does, and overrides the file. The file is optional, so a - deployment or CI run configured purely from the environment works as before. +Configuration is a YAML file: roles, connections, and the tuning knobs (see +docs/Tuning-Guide.md). It holds no secrets. A credential is written as a +reference, ``token: ${NISSE_LLM_TOKEN}``, and resolved from the environment, +so the file can be reviewed, shared, or committed without leaking. The +environment supplies those references and nothing else: every setting has +exactly one home. Every value is collected and validated **before** anything is built, and a single error names every problem at once. The alternative — failing at first @@ -31,32 +28,6 @@ from nisse_dev.matrix import MatrixConfig DEFAULT_CONFIG_FILE = "nisse-dev.yaml" -# Each environment variable and the key it overrides. The names are the ones -# the agent has always used, so existing .env files and CI keep working. -ENV_KEYS: dict[str, str] = { - "NISSE_STATE_DB": "state_db", - "NISSE_WORKSPACE": "workspace", - "NISSE_PROJECT": "project", - "NISSE_LLM_BASE_URL": "llm.base_url", - "NISSE_LLM_TOKEN": "llm.token", - "NISSE_MAX_OUTPUT_TOKENS": "llm.max_output_tokens", - "NISSE_ROLE_EXECUTOR": "roles.executor", - "NISSE_ROLE_PLANNER": "roles.planner", - "NISSE_ROLE_DELEGATE": "roles.delegate", - "NISSE_ROLE_CONSULTANT": "roles.consultant", - "NISSE_MATRIX_HOMESERVER": "matrix.homeserver", - "NISSE_MATRIX_USER_ID": "matrix.user_id", - "NISSE_MATRIX_TOKEN": "matrix.token", - "NISSE_MATRIX_ROOM_ID": "matrix.room_id", - "NISSE_MATRIX_OPERATORS": "matrix.operators", - "NISSE_GITEA_URL": "gitea.url", - "NISSE_GITEA_TOKEN": "gitea.token", - "NISSE_GITEA_REPO": "gitea.repo", - "NISSE_GITEA_OPERATORS": "gitea.operators", - "NISSE_BASE_BRANCH": "gitea.base_branch", -} -KEY_ENV = {key: env for env, key in ENV_KEYS.items()} - REQUIRED_KEYS = ( "state_db", "workspace", @@ -78,8 +49,6 @@ REQUIRED_KEYS = ( "gitea.operators", ) OPTIONAL_KEYS = ("llm.max_output_tokens", "gitea.base_branch") -# Kept for callers that list what a pure-environment deployment must set. -REQUIRED = tuple(KEY_ENV[key] for key in REQUIRED_KEYS) _REFERENCE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") @@ -226,11 +195,6 @@ def _read_file(path: Path, errors: list[str]) -> dict[str, Any]: return loaded -def _describe(key: str) -> str: - env = KEY_ENV.get(key) - return f"{key} ({env})" if env else key - - @dataclass(frozen=True) class AgentConfig: state_db: Path @@ -252,28 +216,39 @@ class AgentConfig: @classmethod def load(cls, env: Mapping[str, str] = os.environ) -> "AgentConfig": - """The file named by NISSE_CONFIG, or ./nisse-dev.yaml if present, - overlaid by the environment. Naming a file that does not exist is an - error; the default file is simply optional.""" - named = env.get("NISSE_CONFIG") - path = Path(named or DEFAULT_CONFIG_FILE).expanduser() - if named and not path.is_file(): + """The file named by NISSE_CONFIG, or ./nisse-dev.yaml.""" + path = Path(env.get("NISSE_CONFIG") or DEFAULT_CONFIG_FILE).expanduser() + if not path.is_file(): raise ConfigError( - f"cannot start: NISSE_CONFIG names {path}, which is missing" + f"cannot start: no config file at {path}; create it from " + "nisse-dev.example.yaml, or point NISSE_CONFIG at one" ) - return cls.from_sources(env, path if path.is_file() else None) + return cls.from_file(path, env) @classmethod - def from_env(cls, env: Mapping[str, str] = os.environ) -> "AgentConfig": - """Environment only, with no file.""" - return cls.from_sources(env, None) - - @classmethod - def from_sources( - cls, env: Mapping[str, str], config_file: Path | None + def from_file( + cls, path: Path, env: Mapping[str, str] = os.environ ) -> "AgentConfig": errors: list[str] = [] - tree = _read_file(config_file, errors) if config_file else {} + tree = _read_file(path, errors) + return cls._build(tree, env, errors, config_file=path) + + @classmethod + def from_tree( + cls, tree: Mapping[str, Any], env: Mapping[str, str] = os.environ + ) -> "AgentConfig": + """From an already-parsed config, as the file would give it.""" + return cls._build(tree, env, [], config_file=None) + + @classmethod + def _build( + cls, + tree: Mapping[str, Any], + env: Mapping[str, str], + errors: list[str], + *, + config_file: Path | None, + ) -> "AgentConfig": tuning = _parse_tuning(tree.get("tuning"), errors) flat = _flatten(tree) flat.pop("tuning", None) @@ -281,20 +256,16 @@ class AgentConfig: known = set(REQUIRED_KEYS) | set(OPTIONAL_KEYS) for key in sorted(set(flat) - known): errors.append(f"{key}: unknown setting") - overrides = {key: env[var] for var, key in ENV_KEYS.items() if var in env} - # A key the environment overrides is not resolved from the file, so a - # reference there to an unset variable is not an error. values = { key: _resolve(value, key, env, errors) for key, value in flat.items() - if key in known and key not in overrides + if key in known } - values.update(overrides) # Whitespace-only counts as missing: a token of spaces would pass a # truthiness check and fail at the first request instead. missing = [ - _describe(key) + key for key in REQUIRED_KEYS if values.get(key) is None or (isinstance(values[key], str) and not values[key].strip()) @@ -309,9 +280,7 @@ class AgentConfig: if max_output <= 0: raise ValueError except (TypeError, ValueError): - errors.append( - f"{_describe('llm.max_output_tokens')}: must be a positive integer" - ) + errors.append("llm.max_output_tokens: must be a positive integer") if errors: raise ConfigError("cannot start: " + "; ".join(errors)) @@ -357,7 +326,7 @@ class AgentConfig: """ workspace = self.workspace.expanduser().resolve() candidates = [ - ("NISSE_STATE_DB", self.state_db), + ("state_db", self.state_db), ("NISSE_ENV", Path(env.get("NISSE_ENV", ".env"))), ] if self.config_file is not None: diff --git a/nisse-dev/src/nisse_dev/gitea.py b/nisse-dev/src/nisse_dev/gitea.py index d5fb62b..6a0678f 100644 --- a/nisse-dev/src/nisse_dev/gitea.py +++ b/nisse-dev/src/nisse_dev/gitea.py @@ -10,8 +10,6 @@ idempotent across a crash: boot reconciliation (A3) asks Gitea whether the effect already happened rather than repeating it. """ -import os -from collections.abc import Mapping from dataclasses import dataclass from typing import Any @@ -60,14 +58,6 @@ class GiteaClient: transport=transport, ) - @classmethod - def from_env(cls, env: Mapping[str, str] = os.environ) -> "GiteaClient": - base_url = env.get("NISSE_GITEA_URL") - token = env.get("NISSE_GITEA_TOKEN") - if not base_url or not token: - raise GiteaError("set NISSE_GITEA_URL and NISSE_GITEA_TOKEN") - return cls(base_url, token) - def _request(self, method: str, path: str, **kwargs: Any) -> Any: try: response = self._http.request(method, path, **kwargs) diff --git a/nisse-dev/src/nisse_dev/llm.py b/nisse-dev/src/nisse_dev/llm.py index 77c4841..7f1542d 100644 --- a/nisse-dev/src/nisse_dev/llm.py +++ b/nisse-dev/src/nisse_dev/llm.py @@ -2,18 +2,16 @@ One client interface, present from the first real LLM call. All traffic goes to the LiteLLM proxy — the VM's single LLM destination; the agent holds one -LiteLLM token and no vendor credentials. Config comes from the environment -(``NISSE_LLM_BASE_URL``, ``NISSE_LLM_TOKEN``), nothing hardcoded. +LiteLLM token and no vendor credentials. The base URL and token come from the +agent's config (nisse_dev.config), nothing hardcoded. Model identity is expressed as roles (A11), resolved to concrete model strings -from env config; the resolved mapping is pinned at task start so a mid-task +from config; the resolved mapping is pinned at task start so a mid-task config change cannot split a task across two planners. """ import json -import os import time -from collections.abc import Mapping from dataclasses import asdict, dataclass from typing import Any, ClassVar, Protocol @@ -49,11 +47,6 @@ RATE_LIMIT_RETRY_S = 20.0 DEFAULT_MAX_OUTPUT_TOKENS = 16384 -def max_output_tokens(env: Mapping[str, str] = os.environ) -> int: - raw = env.get("NISSE_MAX_OUTPUT_TOKENS") - return int(raw) if raw else DEFAULT_MAX_OUTPUT_TOKENS - - class UpstreamUnavailable(LLMError): """The proxy or provider could not be reached, or refused for reasons unrelated to what was asked: a network blip, a timeout, a 5xx, a rate @@ -113,22 +106,10 @@ class RoleMap: delegate: str consultant: str - _ENV_VARS: ClassVar[dict[str, str]] = { - "executor": "NISSE_ROLE_EXECUTOR", - "planner": "NISSE_ROLE_PLANNER", - "delegate": "NISSE_ROLE_DELEGATE", - "consultant": "NISSE_ROLE_CONSULTANT", - } - - @classmethod - def from_env(cls, env: Mapping[str, str] = os.environ) -> "RoleMap": - missing = [var for var in cls._ENV_VARS.values() if not env.get(var)] - if missing: - raise LLMError(f"role mapping incomplete; set: {', '.join(missing)}") - return cls(**{role: env[var] for role, var in cls._ENV_VARS.items()}) + ROLES: ClassVar[tuple[str, ...]] = ("executor", "planner", "delegate", "consultant") def resolve(self, role: str) -> str: - if role not in self._ENV_VARS: + if role not in self.ROLES: raise LLMError(f"unknown role: {role!r}") return str(getattr(self, role)) @@ -156,20 +137,12 @@ class LiteLLMClient: ) self._renderer = OpenAIChatRenderer() self._sleep = sleep - self._max_tokens = max_tokens or max_output_tokens() + self._max_tokens = max_tokens or DEFAULT_MAX_OUTPUT_TOKENS # Set after each call so a caller can meter itself (A13 caps); # a side channel rather than a return value, so the LLMClient # protocol stays the same for every implementation. self.last_usage: dict[str, int] = {} - @classmethod - def from_env(cls, env: Mapping[str, str] = os.environ) -> "LiteLLMClient": - base_url = env.get("NISSE_LLM_BASE_URL") - token = env.get("NISSE_LLM_TOKEN") - if not base_url or not token: - raise LLMError("set NISSE_LLM_BASE_URL and NISSE_LLM_TOKEN") - return cls(base_url, token) - def complete( self, events: list[StoredEvent], @@ -203,7 +176,7 @@ class LiteLLMClient: if choice.get("finish_reason") == "length": raise TruncatedResponse( f"{requested_model} hit its output limit mid-answer; " - "raise NISSE_MAX_OUTPUT_TOKENS" + "raise llm.max_output_tokens" ) produced_by = data.get("model") or requested_model usage = data.get("usage") or {} @@ -275,20 +248,10 @@ class AnthropicPassthroughClient: transport=transport, ) self._renderer = AnthropicMessagesRenderer() - self._max_tokens = max_tokens or max_output_tokens() + self._max_tokens = max_tokens or DEFAULT_MAX_OUTPUT_TOKENS self._sleep = sleep self.last_usage: dict[str, int] = {} - @classmethod - def from_env( - cls, env: Mapping[str, str] = os.environ - ) -> "AnthropicPassthroughClient": - base_url = env.get("NISSE_LLM_BASE_URL") - token = env.get("NISSE_LLM_TOKEN") - if not base_url or not token: - raise LLMError("set NISSE_LLM_BASE_URL and NISSE_LLM_TOKEN") - return cls(base_url, token) - def complete( self, events: list[StoredEvent], @@ -315,7 +278,7 @@ class AnthropicPassthroughClient: if data.get("stop_reason") == "max_tokens": raise TruncatedResponse( f"{requested_model} hit the output limit of {self._max_tokens} " - f"tokens mid-answer; raise NISSE_MAX_OUTPUT_TOKENS" + f"tokens mid-answer; raise llm.max_output_tokens" ) content = data.get("content") if not isinstance(content, list): diff --git a/nisse-dev/src/nisse_dev/matrix.py b/nisse-dev/src/nisse_dev/matrix.py index 19366b5..dde3e2b 100644 --- a/nisse-dev/src/nisse_dev/matrix.py +++ b/nisse-dev/src/nisse_dev/matrix.py @@ -11,12 +11,10 @@ loop backs off (capped) and retries indefinitely — an outage delays reactions but never fails anything. """ -import os import threading import uuid -from collections.abc import Callable, Mapping +from collections.abc import Callable from dataclasses import dataclass -from typing import ClassVar import httpx @@ -34,29 +32,6 @@ class MatrixConfig: token: str room_id: str - _ENV: ClassVar[tuple[str, ...]] = ( - "NISSE_MATRIX_HOMESERVER", - "NISSE_MATRIX_USER_ID", - "NISSE_MATRIX_TOKEN", - ) - - @classmethod - def from_env( - cls, - env: Mapping[str, str] = os.environ, - room_var: str = "NISSE_MATRIX_ROOM_ID", - ) -> "MatrixConfig": - needed = (*cls._ENV, room_var) - missing = [var for var in needed if not env.get(var)] - if missing: - raise TransportError(f"matrix config incomplete; set: {', '.join(missing)}") - return cls( - homeserver=env["NISSE_MATRIX_HOMESERVER"].rstrip("/"), - user_id=env["NISSE_MATRIX_USER_ID"], - token=env["NISSE_MATRIX_TOKEN"], - room_id=env[room_var], - ) - class MatrixTransport: def __init__( diff --git a/nisse-dev/tests/test_agent_build.py b/nisse-dev/tests/test_agent_build.py index 085ec13..655309e 100644 --- a/nisse-dev/tests/test_agent_build.py +++ b/nisse-dev/tests/test_agent_build.py @@ -8,7 +8,7 @@ from typing import Any import httpx import pytest -from test_config import COMPLETE +from test_config import SECRETS, tree from nisse_dev.agent import build_agent, discover_models, prepare_workspace from nisse_dev.catalog import ModelCatalog, UnknownModel @@ -34,14 +34,15 @@ TEST_CATALOG = ModelCatalog.from_payload( ) -def config_for(tmp_path: Path, **overrides: str) -> AgentConfig: - env = { - **COMPLETE, - "NISSE_STATE_DB": str(tmp_path / "state.db"), - "NISSE_WORKSPACE": str(tmp_path / "work"), - **overrides, - } - return AgentConfig.from_env(env) +def config_for(tmp_path: Path, **changes: str) -> AgentConfig: + return AgentConfig.from_tree( + tree( + state_db=str(tmp_path / "state.db"), + workspace=str(tmp_path / "work"), + **changes, + ), + SECRETS, + ) @pytest.fixture @@ -59,7 +60,7 @@ def origin(tmp_path: Path) -> Path: def test_workspace_is_cloned_when_absent(tmp_path: Path, origin: Path) -> None: - config = config_for(tmp_path, NISSE_GITEA_URL=f"file://{origin}") + config = config_for(tmp_path, gitea__url=f"file://{origin}") # authenticated_url would mangle a file:// remote, so clone directly. GitRepo(tmp_path).clone(str(origin), config.workspace) repo = prepare_workspace(config, "nisse-dev-testing") @@ -206,7 +207,7 @@ def test_httpx_clients_are_not_shared_between_routes( def test_a_typod_role_refuses_to_boot(tmp_path: Path, monkeypatch) -> None: # Misconfiguration should cost a second at startup, not be discovered ten # minutes into a milestone. - config = config_for(tmp_path, NISSE_ROLE_PLANNER="clause-opus-5") + config = config_for(tmp_path, roles__planner="clause-opus-5") monkeypatch.setattr( "nisse_dev.agent.ModelCatalog.discover", classmethod(lambda cls, *a, **kw: TEST_CATALOG), @@ -220,7 +221,7 @@ def test_a_role_this_key_cannot_reach_refuses_to_boot( ) -> None: # /model/info is scoped per key, so the same check catches a role pointed # at a real model that this token has no access to. - config = config_for(tmp_path, NISSE_ROLE_EXECUTOR="local-fast") + config = config_for(tmp_path, roles__executor="local-fast") monkeypatch.setattr( "nisse_dev.agent.ModelCatalog.discover", classmethod(lambda cls, *a, **kw: TEST_CATALOG), diff --git a/nisse-dev/tests/test_config.py b/nisse-dev/tests/test_config.py index e6aba55..03c27ad 100644 --- a/nisse-dev/tests/test_config.py +++ b/nisse-dev/tests/test_config.py @@ -1,163 +1,37 @@ +import copy from pathlib import Path +from typing import Any import pytest from nisse_dev.config import AgentConfig, ConfigError -COMPLETE = { - "NISSE_STATE_DB": "~/demo/state.db", - "NISSE_WORKSPACE": "~/demo/work", - "NISSE_PROJECT": "greet", - "NISSE_LLM_BASE_URL": "https://litellm.test", - "NISSE_LLM_TOKEN": "sk-x", - "NISSE_ROLE_EXECUTOR": "claude-haiku-4-5", - "NISSE_ROLE_PLANNER": "claude-opus-5", - "NISSE_ROLE_DELEGATE": "claude-opus-5", - "NISSE_ROLE_CONSULTANT": "claude-haiku-4-5", - "NISSE_MATRIX_HOMESERVER": "https://matrix.test", - "NISSE_MATRIX_USER_ID": "@nisse-dev-testing:test", - "NISSE_MATRIX_TOKEN": "syt_x", - "NISSE_MATRIX_ROOM_ID": "!room:test", - "NISSE_MATRIX_OPERATORS": "@don:test", - "NISSE_GITEA_URL": "https://gitea.test", - "NISSE_GITEA_TOKEN": "gt", - "NISSE_GITEA_REPO": "don/demo", - "NISSE_GITEA_OPERATORS": "don", +# A complete config as the YAML file would give it, with secrets by reference. +COMPLETE: dict[str, Any] = { + "state_db": "~/demo/state.db", + "workspace": "~/demo/work", + "project": "greet", + "llm": {"base_url": "https://litellm.test", "token": "${TEST_LLM_TOKEN}"}, + "roles": { + "executor": "claude-haiku-4-5", + "planner": "claude-opus-5", + "delegate": "claude-opus-5", + "consultant": "claude-haiku-4-5", + }, + "matrix": { + "homeserver": "https://matrix.test/", + "user_id": "@nisse-dev-testing:test", + "token": "${TEST_MATRIX_TOKEN}", + "room_id": "!room:test", + "operators": ["@don:test"], + }, + "gitea": { + "url": "https://gitea.test", + "token": "${TEST_GITEA_TOKEN}", + "repo": "don/demo", + "operators": ["don"], + }, } - - -def test_complete_environment_builds() -> None: - config = AgentConfig.from_env(COMPLETE) - assert config.project == "greet" - assert config.roles.planner == "claude-opus-5" - assert config.matrix.room_id == "!room:test" - assert config.gitea_repo == "don/demo" - assert config.base_branch == "main" - - -def test_paths_expand_the_home_directory() -> None: - config = AgentConfig.from_env(COMPLETE) - assert config.workspace.is_absolute() - assert "~" not in str(config.state_db) - - -def test_every_missing_variable_is_reported_at_once() -> None: - # Failing at first use would surface a missing token twenty minutes into - # a milestone, on the call that mattered. - partial = {k: v for k, v in COMPLETE.items() if k != "NISSE_MATRIX_TOKEN"} - partial.pop("NISSE_GITEA_REPO") - with pytest.raises(ConfigError) as excinfo: - AgentConfig.from_env(partial) - message = str(excinfo.value) - assert "NISSE_MATRIX_TOKEN" in message - assert "NISSE_GITEA_REPO" in message - - -def test_empty_is_treated_as_missing() -> None: - with pytest.raises(ConfigError, match="NISSE_GITEA_TOKEN"): - AgentConfig.from_env({**COMPLETE, "NISSE_GITEA_TOKEN": " "}) - - -def test_operator_lists_are_split_and_trimmed() -> None: - config = AgentConfig.from_env( - { - **COMPLETE, - "NISSE_MATRIX_OPERATORS": " @don:test , @other:test ,", - "NISSE_GITEA_OPERATORS": "don,", - } - ) - assert config.operators == {"@don:test", "@other:test"} - # A trailing comma must not admit an empty identity into an allowlist. - assert "" not in config.operators - assert config.gitea_operators == {"don"} - - -def test_base_branch_is_overridable() -> None: - config = AgentConfig.from_env({**COMPLETE, "NISSE_BASE_BRANCH": "trunk"}) - assert config.base_branch == "trunk" - - -def test_redacted_summary_carries_no_secrets() -> None: - summary = AgentConfig.from_env(COMPLETE).redacted() - flat = " ".join(summary.values()) - for secret in ("sk-x", "syt_x", "gt"): - assert secret not in flat - assert summary["project"] == "greet" - - -def test_state_db_and_workspace_are_separate(tmp_path: Path) -> None: - # A2: build artefacts must not share a volume with agent state. - config = AgentConfig.from_env(COMPLETE) - assert config.state_db.parent != config.workspace - - -def test_state_db_inside_the_clone_is_refused(tmp_path: Path) -> None: - # git add -A would commit it, and the push would publish the agent's - # own state alongside the project. - work = tmp_path / "project" - with pytest.raises(ConfigError, match="inside the workspace"): - AgentConfig.from_env( - { - **COMPLETE, - "NISSE_WORKSPACE": str(work), - "NISSE_STATE_DB": str(work / "state.db"), - } - ) - - -def test_env_file_inside_the_clone_is_refused(tmp_path: Path) -> None: - # The severe case: the agent would commit and push its own credentials - # as a side effect of working correctly. - work = tmp_path / "project" - with pytest.raises(ConfigError, match="NISSE_ENV"): - AgentConfig.from_env( - { - **COMPLETE, - "NISSE_WORKSPACE": str(work), - "NISSE_STATE_DB": str(tmp_path / "state.db"), - "NISSE_ENV": str(work / ".env"), - } - ) - - -def test_sibling_paths_are_fine(tmp_path: Path) -> None: - config = AgentConfig.from_env( - { - **COMPLETE, - "NISSE_WORKSPACE": str(tmp_path / "project"), - "NISSE_STATE_DB": str(tmp_path / "state.db"), - "NISSE_ENV": str(tmp_path / ".env"), - } - ) - assert config.workspace.name == "project" - - -# --- the YAML file (#32) --------------------------------------------------- - -FILE = """ -state_db: ~/demo/state.db -workspace: ~/demo/work -project: greet -llm: - base_url: https://litellm.test - token: ${TEST_LLM_TOKEN} -roles: - executor: local-fast - planner: claude-opus-5 - delegate: claude-opus-5 - consultant: claude-haiku-4-5 -matrix: - homeserver: https://matrix.test/ - user_id: "@nisse-dev-testing:test" - token: ${TEST_MATRIX_TOKEN} - room_id: "!room:test" - operators: ["@don:test", " "] -gitea: - url: https://gitea.test - token: ${TEST_GITEA_TOKEN} - repo: don/demo - operators: [don] -""" SECRETS = { "TEST_LLM_TOKEN": "sk-x", "TEST_MATRIX_TOKEN": "syt_x", @@ -165,86 +39,133 @@ SECRETS = { } -def write(tmp_path: Path, text: str) -> Path: - path = tmp_path / "nisse-dev.yaml" - path.write_text(text) - return path +def tree(**changes: Any) -> dict[str, Any]: + """COMPLETE with dotted keys changed; a value of None removes the key.""" + result = copy.deepcopy(COMPLETE) + for dotted, value in changes.items(): + *parents, leaf = dotted.split("__") + node = result + for part in parents: + node = node.setdefault(part, {}) + if value is None: + node.pop(leaf, None) + else: + node[leaf] = value + return result -def test_a_file_configures_everything_with_secrets_by_reference( - tmp_path: Path, -) -> None: - config = AgentConfig.from_sources(SECRETS, write(tmp_path, FILE)) +def build(**changes: Any) -> AgentConfig: + return AgentConfig.from_tree(tree(**changes), SECRETS) + + +def test_complete_config_builds_with_secrets_by_reference() -> None: + config = build() + assert config.project == "greet" assert config.llm_token == "sk-x" assert config.gitea_token == "gt" - assert config.roles.executor == "local-fast" - assert config.operators == {"@don:test"} # a blank entry admits no one + assert config.roles.planner == "claude-opus-5" + assert config.matrix.room_id == "!room:test" assert config.matrix.homeserver == "https://matrix.test" + assert config.gitea_repo == "don/demo" + assert config.base_branch == "main" assert config.tuning.executor.max_turns == 24 # absent section: defaults -def test_the_environment_wins_over_the_file(tmp_path: Path) -> None: - config = AgentConfig.from_sources( - {**SECRETS, "NISSE_ROLE_EXECUTOR": "claude-haiku-4-5"}, write(tmp_path, FILE) - ) - assert config.roles.executor == "claude-haiku-4-5" +def test_paths_expand_the_home_directory() -> None: + config = build() + assert config.workspace.is_absolute() + assert "~" not in str(config.state_db) -def test_an_overridden_key_needs_no_resolvable_reference(tmp_path: Path) -> None: - env = {k: v for k, v in SECRETS.items() if k != "TEST_LLM_TOKEN"} - config = AgentConfig.from_sources( - {**env, "NISSE_LLM_TOKEN": "sk-env"}, write(tmp_path, FILE) - ) - assert config.llm_token == "sk-env" +def test_every_missing_setting_is_reported_at_once() -> None: + # Failing at first use would surface a missing token twenty minutes into + # a milestone, on the call that mattered. + with pytest.raises(ConfigError) as excinfo: + build(matrix__token=None, gitea__repo=None) + message = str(excinfo.value) + assert "matrix.token" in message + assert "gitea.repo" in message -def test_a_reference_to_an_unset_variable_is_an_error(tmp_path: Path) -> None: +def test_empty_is_treated_as_missing() -> None: + with pytest.raises(ConfigError, match="gitea.repo"): + build(gitea__repo=" ") + + +def test_a_reference_to_an_unset_variable_is_an_error() -> None: + # Never an empty string: an empty token would pass for a value and fail + # at the first request. env = {k: v for k, v in SECRETS.items() if k != "TEST_GITEA_TOKEN"} with pytest.raises(ConfigError, match=r"gitea.token: references \$\{TEST_GITEA"): - AgentConfig.from_sources(env, write(tmp_path, FILE)) + AgentConfig.from_tree(tree(), env) -def test_tuning_is_read_from_the_file(tmp_path: Path) -> None: - text = ( - FILE - + """ -tuning: - executor: {max_turns: 40, failure_limit: 3} - expert: {consult_budget: 5} - delegation: {max_turns: 20, max_tokens: 100000, wall_clock_s: 600} - tools: {command_timeout_s: 300, max_output_chars: 2000} -""" +def test_operator_lists_drop_blanks() -> None: + config = build(matrix__operators=[" @don:test ", "@other:test", " "]) + assert config.operators == {"@don:test", "@other:test"} + # A blank entry must not admit an empty identity into an allowlist. + assert "" not in config.operators + + +def test_an_operator_list_of_only_blanks_is_missing() -> None: + with pytest.raises(ConfigError, match="gitea.operators"): + build(gitea__operators=[" "]) + + +def test_base_branch_is_configurable() -> None: + assert build(gitea__base_branch="trunk").base_branch == "trunk" + + +def test_max_output_tokens_is_configurable_and_checked() -> None: + assert build(llm__max_output_tokens=8192).max_output_tokens == 8192 + with pytest.raises(ConfigError, match="llm.max_output_tokens"): + build(llm__max_output_tokens=0) + + +def test_redacted_summary_carries_no_secrets() -> None: + summary = build().redacted() + flat = " ".join(summary.values()) + for secret in SECRETS.values(): + assert secret not in flat + assert summary["project"] == "greet" + + +def test_tuning_is_read() -> None: + config = build( + tuning={ + "executor": {"max_turns": 40, "failure_limit": 3}, + "expert": {"consult_budget": 5}, + "delegation": {"max_turns": 20, "max_tokens": 100000, "wall_clock_s": 600}, + "tools": {"command_timeout_s": 300, "max_output_chars": 2000}, + } ) - tuning = AgentConfig.from_sources(SECRETS, write(tmp_path, text)).tuning + tuning = config.tuning assert (tuning.executor.max_turns, tuning.executor.failure_limit) == (40, 3) assert tuning.expert.consult_budget == 5 assert tuning.delegation.max_tokens == 100_000 assert tuning.delegation.wall_clock_s == 600.0 assert tuning.tools.max_output_chars == 2000 - # A section that is only partly given keeps defaults for the rest. - partial = FILE + "\ntuning:\n executor: {max_turns: 40}\n" - tuning = AgentConfig.from_sources(SECRETS, write(tmp_path, partial)).tuning + + +def test_a_partly_given_tuning_section_keeps_the_other_defaults() -> None: + tuning = build(tuning={"executor": {"max_turns": 40}}).tuning assert tuning.executor.failure_limit == 2 -def test_every_problem_in_a_file_is_reported_at_once(tmp_path: Path) -> None: +def test_every_problem_is_reported_at_once() -> None: # A misspelt knob that was silently ignored would look exactly like a knob # with no effect, so unknown keys are errors. - text = ( - FILE.replace("project: greet\n", "") - + """ -llm_extra: 1 -tuning: - executor: {max_turn: 40, failure_limit: 0} - expert: {consult_budget: true} - bogus: {x: 1} -""" - ) + bad = tree(project=None, llm_extra=1) + bad["tuning"] = { + "executor": {"max_turn": 40, "failure_limit": 0}, + "expert": {"consult_budget": True}, + "bogus": {"x": 1}, + } with pytest.raises(ConfigError) as excinfo: - AgentConfig.from_sources(SECRETS, write(tmp_path, text)) + AgentConfig.from_tree(bad, SECRETS) message = str(excinfo.value) for expected in ( - "project (NISSE_PROJECT)", + "project", "llm_extra: unknown setting", "tuning.executor.max_turn: unknown setting", "tuning.executor.failure_limit: must be a positive number", @@ -254,50 +175,76 @@ tuning: assert expected in message -def test_a_whole_number_is_required_where_one_is_meant(tmp_path: Path) -> None: - text = FILE + "\ntuning:\n executor: {max_turns: 2.5}\n" +def test_a_whole_number_is_required_where_one_is_meant() -> None: with pytest.raises(ConfigError, match="tuning.executor.max_turns"): - AgentConfig.from_sources(SECRETS, write(tmp_path, text)) + build(tuning={"executor": {"max_turns": 2.5}}) -def test_max_output_tokens_comes_from_the_file_or_the_environment( - tmp_path: Path, -) -> None: - text = FILE.replace( - " base_url: https://litellm.test\n", - " base_url: https://litellm.test\n max_output_tokens: 8192\n", - ) - path = write(tmp_path, text) - assert AgentConfig.from_sources(SECRETS, path).max_output_tokens == 8192 - env = {**SECRETS, "NISSE_MAX_OUTPUT_TOKENS": "4096"} - assert AgentConfig.from_sources(env, path).max_output_tokens == 4096 +# --- the file -------------------------------------------------------------- + +FILE = """ +state_db: ~/demo/state.db +workspace: ~/demo/work +project: greet +llm: {base_url: "https://litellm.test", token: "${TEST_LLM_TOKEN}"} +roles: + executor: local-fast + planner: claude-opus-5 + delegate: claude-opus-5 + consultant: claude-haiku-4-5 +matrix: + homeserver: https://matrix.test + user_id: "@nisse-dev-testing:test" + token: ${TEST_MATRIX_TOKEN} + room_id: "!room:test" + operators: ["@don:test"] +gitea: + url: https://gitea.test + token: ${TEST_GITEA_TOKEN} + repo: don/demo + operators: [don] +""" -def test_load_finds_the_named_file_and_refuses_a_missing_one(tmp_path: Path) -> None: - path = write(tmp_path, FILE) +def write(directory: Path, text: str = FILE) -> Path: + path = directory / "nisse-dev.yaml" + path.write_text(text) + return path + + +def test_a_file_is_read(tmp_path: Path) -> None: + config = AgentConfig.from_file(write(tmp_path), SECRETS) + assert config.roles.executor == "local-fast" + assert config.config_file == tmp_path / "nisse-dev.yaml" + + +def test_invalid_yaml_is_reported(tmp_path: Path) -> None: + with pytest.raises(ConfigError, match="not valid YAML"): + AgentConfig.from_file(write(tmp_path, "roles: [unclosed"), SECRETS) + + +def test_load_uses_nisse_config(tmp_path: Path) -> None: + path = write(tmp_path) assert AgentConfig.load({**SECRETS, "NISSE_CONFIG": str(path)}).project == "greet" - with pytest.raises(ConfigError, match="missing"): - AgentConfig.load({**SECRETS, "NISSE_CONFIG": str(tmp_path / "absent.yaml")}) -def test_load_without_a_file_uses_the_environment( +def test_load_defaults_to_the_working_directory( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - monkeypatch.chdir(tmp_path) # no ./nisse-dev.yaml here - assert AgentConfig.load(COMPLETE).config_file is None + write(tmp_path) + monkeypatch.chdir(tmp_path) + assert AgentConfig.load(SECRETS).project == "greet" -def test_a_config_file_inside_the_clone_is_refused(tmp_path: Path) -> None: - work = tmp_path / "project" - work.mkdir() - text = FILE.replace("workspace: ~/demo/work", f"workspace: {work}") - path = work / "nisse-dev.yaml" - path.write_text(text) - with pytest.raises(ConfigError, match="NISSE_CONFIG"): - AgentConfig.from_sources(SECRETS, path) +def test_load_without_a_file_says_how_to_make_one( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + with pytest.raises(ConfigError, match="nisse-dev.example.yaml"): + AgentConfig.load(SECRETS) -def test_the_example_file_is_valid(tmp_path: Path) -> None: +def test_the_example_file_is_valid() -> None: # The example documents every knob; it must not drift out of loading. example = Path(__file__).resolve().parents[2] / "nisse-dev.example.yaml" env = { @@ -305,5 +252,48 @@ def test_the_example_file_is_valid(tmp_path: Path) -> None: "NISSE_MATRIX_TOKEN": "syt_x", "NISSE_GITEA_TOKEN": "gt", } - config = AgentConfig.from_sources(env, example) - assert config.tuning.executor.max_turns == 24 + assert AgentConfig.from_file(example, env).tuning.executor.max_turns == 24 + + +# --- nothing that carries secrets may sit inside the clone ----------------- + + +def test_state_db_and_workspace_are_separate() -> None: + # A2: build artefacts must not share a volume with agent state. + config = build() + assert config.state_db.parent != config.workspace + + +def test_state_db_inside_the_clone_is_refused(tmp_path: Path) -> None: + # git add -A would commit it, and the push would publish the agent's + # own state alongside the project. + work = tmp_path / "project" + with pytest.raises(ConfigError, match="inside the workspace"): + build(workspace=str(work), state_db=str(work / "state.db")) + + +def test_env_file_inside_the_clone_is_refused(tmp_path: Path) -> None: + # The severe case: the agent would commit and push its own credentials + # as a side effect of working correctly. + work = tmp_path / "project" + with pytest.raises(ConfigError, match="NISSE_ENV"): + AgentConfig.from_tree( + tree(workspace=str(work), state_db=str(tmp_path / "state.db")), + {**SECRETS, "NISSE_ENV": str(work / ".env")}, + ) + + +def test_a_config_file_inside_the_clone_is_refused(tmp_path: Path) -> None: + work = tmp_path / "project" + work.mkdir() + text = FILE.replace("workspace: ~/demo/work", f"workspace: {work}") + with pytest.raises(ConfigError, match="NISSE_CONFIG"): + AgentConfig.from_file(write(work, text), SECRETS) + + +def test_sibling_paths_are_fine(tmp_path: Path) -> None: + config = AgentConfig.from_tree( + tree(workspace=str(tmp_path / "project"), state_db=str(tmp_path / "state.db")), + {**SECRETS, "NISSE_ENV": str(tmp_path / ".env")}, + ) + assert config.workspace.name == "project" diff --git a/nisse-dev/tests/test_dev_main.py b/nisse-dev/tests/test_dev_main.py index 2304939..600252f 100644 --- a/nisse-dev/tests/test_dev_main.py +++ b/nisse-dev/tests/test_dev_main.py @@ -2,25 +2,23 @@ from pathlib import Path import pytest -from nisse_dev.config import REQUIRED from nisse_dev.main import main -def test_main_refuses_to_start_without_configuration( +def test_main_refuses_to_start_without_a_config_file( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path, ) -> None: - # Clear the real variables rather than swapping os.environ: from_env binds - # the environment object as a default argument, so a replacement would be - # ignored. And point NISSE_ENV at a file that does not exist, because the - # working directory's .env holds live credentials. - for name in REQUIRED: - monkeypatch.delenv(name, raising=False) + # Run from an empty directory, and point NISSE_ENV at a file that does not + # exist: the repository's .env holds live credentials, and a test must + # never be one cwd away from starting a real agent. + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("NISSE_CONFIG", raising=False) monkeypatch.setenv("NISSE_ENV", str(tmp_path / "absent.env")) assert main() == 2 err = capsys.readouterr().err assert "cannot start" in err - assert "NISSE_LLM_TOKEN" in err + assert "nisse-dev.example.yaml" in err diff --git a/nisse-dev/tests/test_gitea.py b/nisse-dev/tests/test_gitea.py index 2f2190c..17a4d20 100644 --- a/nisse-dev/tests/test_gitea.py +++ b/nisse-dev/tests/test_gitea.py @@ -162,11 +162,6 @@ def test_list_comments_passes_since() -> None: assert seen["since"] == "2026-09-07T00:00:00Z" -def test_from_env_requires_config() -> None: - with pytest.raises(GiteaError, match="NISSE_GITEA_URL"): - GiteaClient.from_env({}) - - # --- reconciler (A3 external truth) -------------------------------------- diff --git a/nisse-dev/tests/test_llm.py b/nisse-dev/tests/test_llm.py index 746ad53..5b1238e 100644 --- a/nisse-dev/tests/test_llm.py +++ b/nisse-dev/tests/test_llm.py @@ -21,7 +21,6 @@ from nisse_dev.llm import ( RoleMap, TruncatedResponse, UpstreamUnavailable, - max_output_tokens, ) EVENTS = [StoredEvent(id=1, ts="t", event=UserMessage(text="hi", source="m:1"))] @@ -165,16 +164,16 @@ def test_malformed_response_raises_llm_error() -> None: client.complete(EVENTS, tools=None, model="local/qwen") -ROLE_ENV = { - "NISSE_ROLE_EXECUTOR": "local/qwen", - "NISSE_ROLE_PLANNER": "anthropic/claude-opus-5", - "NISSE_ROLE_DELEGATE": "anthropic/claude-opus-5", - "NISSE_ROLE_CONSULTANT": "anthropic/claude-opus-5", -} +ROLES = RoleMap( + executor="local/qwen", + planner="anthropic/claude-opus-5", + delegate="anthropic/claude-opus-5", + consultant="anthropic/claude-opus-5", +) def test_role_map_resolves_and_pins() -> None: - roles = RoleMap.from_env(ROLE_ENV) + roles = ROLES assert roles.resolve("executor") == "local/qwen" assert roles.pinned() == { "executor": "local/qwen", @@ -184,19 +183,9 @@ def test_role_map_resolves_and_pins() -> None: } -def test_role_map_missing_env_raises() -> None: - with pytest.raises(LLMError, match="NISSE_ROLE_PLANNER"): - RoleMap.from_env({"NISSE_ROLE_EXECUTOR": "local/qwen"}) - - def test_unknown_role_raises() -> None: with pytest.raises(LLMError, match="unknown role"): - RoleMap.from_env(ROLE_ENV).resolve("boss") - - -def test_client_from_env_requires_config() -> None: - with pytest.raises(LLMError, match="NISSE_LLM_BASE_URL"): - LiteLLMClient.from_env({}) + ROLES.resolve("boss") def test_load_env_file(tmp_path: Path) -> None: @@ -306,12 +295,11 @@ def test_a_complete_answer_is_not_mistaken_for_truncation() -> None: assert call.arguments["content"] == "a real plan" -def test_output_ceiling_is_configurable_and_generous_by_default() -> None: +def test_output_ceiling_is_generous_by_default() -> None: # Output is billed by what is produced, so a low ceiling buys nothing and - # costs silent truncation. + # costs silent truncation. The configured value is tested in test_config. assert DEFAULT_MAX_OUTPUT_TOKENS >= 8192 - assert max_output_tokens({}) == DEFAULT_MAX_OUTPUT_TOKENS - assert max_output_tokens({"NISSE_MAX_OUTPUT_TOKENS": "2048"}) == 2048 + assert LiteLLMClient("http://x", "t")._max_tokens == DEFAULT_MAX_OUTPUT_TOKENS def test_truncation_is_an_llm_error_so_callers_already_handle_it() -> None: diff --git a/nisse-dev/tests/test_matrix.py b/nisse-dev/tests/test_matrix.py index 2e1fd69..049651b 100644 --- a/nisse-dev/tests/test_matrix.py +++ b/nisse-dev/tests/test_matrix.py @@ -143,20 +143,3 @@ def test_start_without_handler_raises() -> None: ) with pytest.raises(TransportError, match="no message handler"): transport.start() - - -def test_config_from_env_reports_missing() -> None: - with pytest.raises(TransportError, match="NISSE_MATRIX_TOKEN"): - MatrixConfig.from_env({"NISSE_MATRIX_HOMESERVER": "https://m"}) - - -def test_config_from_env_custom_room_var() -> None: - env = { - "NISSE_MATRIX_HOMESERVER": "https://m/", - "NISSE_MATRIX_USER_ID": "@nisse-dev:test", - "NISSE_MATRIX_TOKEN": "t", - "NISSE_MATRIX_TEST_ROOM_ID": "!t:test", - } - config = MatrixConfig.from_env(env, room_var="NISSE_MATRIX_TEST_ROOM_ID") - assert config.room_id == "!t:test" - assert config.homeserver == "https://m" # trailing slash stripped diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py new file mode 100644 index 0000000..7a983fa --- /dev/null +++ b/tests/integration/conftest.py @@ -0,0 +1,62 @@ +"""Shared setup for the live integration tests. + +They run against real services, configured by nisse-test.yaml beside this +file: an ordinary agent config aimed at test resources, committed to the +repository. Its secrets are ${…} references, supplied by CI as environment +variables and locally by .env at the repository root. + +A missing secret is a failure, not a skip. A half-configured environment must +not pass quietly as a skip: that is how a broken CI credential hides. +""" + +from pathlib import Path + +import pytest + +from nisse_dev.config import AgentConfig +from nisse_dev.envfile import load_env_file +from nisse_dev.gitea import GiteaClient +from nisse_dev.llm import AnthropicPassthroughClient, LiteLLMClient + +REPO = Path(__file__).resolve().parents[2] +TEST_CONFIG = Path(__file__).parent / "nisse-test.yaml" + +# At import, so per-run switches such as NISSE_CAPABILITY_MODELS can be set +# in .env too. Real environment variables win over the file. +load_env_file(REPO / ".env") + + +@pytest.fixture(scope="session") +def live_config() -> AgentConfig: + return AgentConfig.from_file(TEST_CONFIG) + + +@pytest.fixture +def live_model(live_config: AgentConfig) -> str: + """The model the live tests plan, execute and delegate with.""" + return live_config.roles.planner + + +@pytest.fixture +def litellm(live_config: AgentConfig) -> LiteLLMClient: + return LiteLLMClient( + live_config.llm_base_url, + live_config.llm_token, + max_tokens=live_config.max_output_tokens, + ) + + +@pytest.fixture +def passthrough(live_config: AgentConfig) -> AnthropicPassthroughClient: + return AnthropicPassthroughClient( + live_config.llm_base_url, + live_config.llm_token, + max_tokens=live_config.max_output_tokens, + ) + + +@pytest.fixture +def gitea(live_config: AgentConfig) -> GiteaClient: + client = GiteaClient(live_config.gitea_url, live_config.gitea_token) + yield client + client.close() diff --git a/tests/integration/nisse-test.yaml b/tests/integration/nisse-test.yaml new file mode 100644 index 0000000..2d4451f --- /dev/null +++ b/tests/integration/nisse-test.yaml @@ -0,0 +1,40 @@ +# Agent config for the live integration tests, aimed at test resources: the +# testing identities, the dedicated test room, and the sandbox repo. Never +# point it at production. +# +# Secrets are ${…} references. CI passes them as environment variables; +# locally they come from .env at the repository root. A missing one fails +# the live tests that need it, rather than skipping them, so a broken +# credential cannot hide as a skip. + +# Unused by the live tests, which work in pytest's temporary directories; +# present because every agent config has them. +state_db: /tmp/nisse-test/state.db +workspace: /tmp/nisse-test/work +project: nisse-dev-testing + +llm: + base_url: https://litellm.dongar.net + token: ${NISSE_LLM_TOKEN} + +# The live tests exercise the gated loop and the routes over a real proxy, +# not local-model competence (measured separately by test_model_capability), +# so every role uses a model that holds the tool-call contract. +roles: + executor: claude-haiku-4-5 + planner: claude-haiku-4-5 + delegate: claude-haiku-4-5 + consultant: claude-haiku-4-5 + +matrix: + homeserver: https://matrix.dongar.net + user_id: "@nisse-dev-testing:dongar.net" + token: ${NISSE_MATRIX_TOKEN} + room_id: "!UxpXhTlQsFIeNZwkgv:dongar.net" # the test room, never production + operators: ["@dongar:dongar.net"] + +gitea: + url: https://gitea.dongar.net + token: ${NISSE_GITEA_TOKEN} + repo: dongar/nisse-dev-testing-sandbox # the sandbox, never a real project + operators: [dongar] diff --git a/tests/integration/test_delegation_live.py b/tests/integration/test_delegation_live.py index 3ccc9d8..4c73f6e 100644 --- a/tests/integration/test_delegation_live.py +++ b/tests/integration/test_delegation_live.py @@ -2,30 +2,18 @@ working code and a structured handback out.""" import io -import os from pathlib import Path import pytest from nisse_dev.delegation import Caps, Delegation -from nisse_dev.envfile import load_env_file from nisse_dev.git import GitRepo from nisse_dev.llm import AnthropicPassthroughClient from nisse_dev.oplog import OpLog from nisse_dev.state import StateStore from nisse_dev.tools import Workspace -load_env_file(Path(__file__).resolve().parents[2] / ".env") - -SENTINEL = "NISSE_LIVE_PLANNER_MODEL" - -pytestmark = [ - pytest.mark.integration, - pytest.mark.skipif( - not os.environ.get(SENTINEL), - reason=f"no live frontier route configured ({SENTINEL} unset)", - ), -] +pytestmark = pytest.mark.integration BRIEF = """Create `slugify.py` in the workspace root containing a function `slugify(text)` that lowercases the text and replaces every run of @@ -34,7 +22,9 @@ hyphen. Then create `test_slugify.py` with at least two assertions covering it, and run `python -m pytest test_slugify.py -q` until it passes.""" -def build(tmp_path: Path, caps: Caps) -> tuple[Delegation, GitRepo, StateStore]: +def build( + tmp_path: Path, caps: Caps, client: AnthropicPassthroughClient, model: str +) -> tuple[Delegation, GitRepo, StateStore]: repo = GitRepo(tmp_path / "work") repo.init() repo.configure_identity("nisse-dev-testing", "n@t.test") @@ -42,18 +32,22 @@ def build(tmp_path: Path, caps: Caps) -> tuple[Delegation, GitRepo, StateStore]: repo.commit_all("seed") store = StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO())) delegation = Delegation( - AnthropicPassthroughClient.from_env(), + client, store, Workspace(repo.root, timeout_s=60), repo, - model=os.environ[SENTINEL], + model=model, caps=caps, ) return delegation, repo, store -def test_delegate_completes_a_scoped_subtask(tmp_path: Path) -> None: - delegation, repo, _store = build(tmp_path, Caps(turns=12, wall_clock_s=300.0)) +def test_delegate_completes_a_scoped_subtask( + tmp_path: Path, passthrough: AnthropicPassthroughClient, live_model: str +) -> None: + delegation, repo, _store = build( + tmp_path, Caps(turns=12, wall_clock_s=300.0), passthrough, live_model + ) result = delegation.run(brief=BRIEF, branch="live/delegation-1") @@ -76,10 +70,14 @@ def test_delegate_completes_a_scoped_subtask(tmp_path: Path) -> None: assert check.success, f"delegate's own tests do not pass: {check.result[:400]}" -def test_a_cap_breach_discards_the_work(tmp_path: Path) -> None: +def test_a_cap_breach_discards_the_work( + tmp_path: Path, passthrough: AnthropicPassthroughClient, live_model: str +) -> None: # One turn cannot finish the brief, so this exercises the abort path # against a real model rather than a scripted one. - delegation, repo, _ = build(tmp_path, Caps(turns=1, wall_clock_s=120.0)) + delegation, repo, _ = build( + tmp_path, Caps(turns=1, wall_clock_s=120.0), passthrough, live_model + ) result = delegation.run(brief=BRIEF, branch="live/delegation-2") diff --git a/tests/integration/test_entry_points.py b/tests/integration/test_entry_points.py index dc6d951..5a75521 100644 --- a/tests/integration/test_entry_points.py +++ b/tests/integration/test_entry_points.py @@ -2,6 +2,7 @@ import os import subprocess +from pathlib import Path import pytest @@ -16,27 +17,31 @@ def test_ops_entry_point_runs_and_greets() -> None: assert result.stdout == "Hello from nisse-ops, the house spirit is awake.\n" -def test_dev_entry_point_refuses_to_start_unconfigured() -> None: - """Missing configuration must fail at startup, naming everything absent. +def test_dev_entry_point_refuses_to_start_unconfigured(tmp_path: Path) -> None: + """An incomplete config must fail at startup, naming everything absent. - Run with a deliberately empty environment and a .env path that does not - exist: the real .env in the working directory holds live credentials, and - a test must never be one cwd away from starting a real agent. + Run from an empty directory, with an empty config file and a .env path + that does not exist: the repository's .env holds live credentials, and a + test must never be one cwd away from starting a real agent. """ + config = tmp_path / "nisse-dev.yaml" + config.write_text("") result = subprocess.run( ["nisse-dev"], capture_output=True, text=True, timeout=30, check=False, + cwd=tmp_path, env={ "PATH": os.environ["PATH"], "HOME": os.environ["HOME"], "NISSE_ENV": "/nonexistent/.env", + "NISSE_CONFIG": str(config), }, ) assert result.returncode == 2 assert "cannot start" in result.stderr # All of them at once, not the first one it happened to reach. - for expected in ("NISSE_STATE_DB", "NISSE_MATRIX_TOKEN", "NISSE_GITEA_REPO"): + for expected in ("state_db", "matrix.token", "gitea.repo"): assert expected in result.stderr diff --git a/tests/integration/test_expert_live.py b/tests/integration/test_expert_live.py index 8197efd..0205152 100644 --- a/tests/integration/test_expert_live.py +++ b/tests/integration/test_expert_live.py @@ -5,40 +5,34 @@ that parses against our local schema, since no API-level enforcement exists. """ import io -import os from pathlib import Path import pytest -from nisse_dev.envfile import load_env_file from nisse_dev.evals import collect from nisse_dev.expert import BudgetExhausted, ExpertDesk from nisse_dev.llm import LiteLLMClient, RoleMap from nisse_dev.oplog import OpLog from nisse_dev.state import StateStore -load_env_file(Path(__file__).resolve().parents[2] / ".env") - -SENTINEL = "NISSE_LIVE_PLANNER_MODEL" - -pytestmark = [ - pytest.mark.integration, - pytest.mark.skipif( - not os.environ.get(SENTINEL), - reason=f"no live frontier route configured ({SENTINEL} unset)", - ), -] +pytestmark = pytest.mark.integration -def desk(tmp_path: Path, budget: int = 2) -> tuple[ExpertDesk, StateStore]: - model = os.environ[SENTINEL] - roles = RoleMap(executor=model, planner=model, delegate=model, consultant=model) - store = StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO())) - return ExpertDesk(LiteLLMClient.from_env(), store, roles, budget=budget), store +@pytest.fixture +def desk(tmp_path: Path, litellm: LiteLLMClient, live_model: str): + def make(budget: int = 2) -> tuple[ExpertDesk, StateStore]: + model = live_model + roles = RoleMap(executor=model, planner=model, delegate=model, consultant=model) + store = StateStore( + tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO()) + ) + return ExpertDesk(litellm, store, roles, budget=budget), store + + return make -def test_live_consult_parses_against_the_local_schema(tmp_path: Path) -> None: - expert, store = desk(tmp_path) +def test_live_consult_parses_against_the_local_schema(desk) -> None: + expert, store = desk() answer = expert.ask( "For a single-process agent storing a few thousand rows of task state, " @@ -53,8 +47,8 @@ def test_live_consult_parses_against_the_local_schema(tmp_path: Path) -> None: assert record.exit_status == "answered" -def test_live_budget_stops_a_circling_caller(tmp_path: Path) -> None: - expert, _ = desk(tmp_path, budget=1) +def test_live_budget_stops_a_circling_caller(desk) -> None: + expert, _ = desk(budget=1) expert.ask("Name one advantage of WAL mode in SQLite. Be brief.") with pytest.raises(BudgetExhausted): expert.ask("And another?") diff --git a/tests/integration/test_gitea_live.py b/tests/integration/test_gitea_live.py index c1f3d0f..27270ab 100644 --- a/tests/integration/test_gitea_live.py +++ b/tests/integration/test_gitea_live.py @@ -5,29 +5,17 @@ Cleans up after itself (closes the PR, deletes the branch) so repeated runs do not silt up the sandbox. """ -import os import uuid from pathlib import Path import pytest -from nisse_dev.envfile import load_env_file +from nisse_dev.config import AgentConfig from nisse_dev.git import GitError, GitRepo, authenticated_url from nisse_dev.gitea import GiteaClient, pull_request_reconciler from nisse_dev.state import Intent -load_env_file(Path(__file__).resolve().parents[2] / ".env") - -SENTINEL = "NISSE_GITEA_URL" -ALSO_REQUIRED = ("NISSE_GITEA_TOKEN", "NISSE_GITEA_TEST_REPO") - -pytestmark = [ - pytest.mark.integration, - pytest.mark.skipif( - not os.environ.get(SENTINEL), - reason=f"no live Gitea configured ({SENTINEL} unset)", - ), -] +pytestmark = pytest.mark.integration def intent_for(branch: str) -> Intent: @@ -41,16 +29,13 @@ def intent_for(branch: str) -> Intent: ) -def test_push_open_pr_and_reconcile(tmp_path: Path) -> None: - missing = [var for var in ALSO_REQUIRED if not os.environ.get(var)] - assert not missing, ( - f"{SENTINEL} is set, so the live Gitea path is expected to work, " - f"but these are missing: {', '.join(missing)}" - ) - base_url = os.environ[SENTINEL] - token = os.environ["NISSE_GITEA_TOKEN"] - repo_path = os.environ["NISSE_GITEA_TEST_REPO"] - client = GiteaClient.from_env() +def test_push_open_pr_and_reconcile( + tmp_path: Path, live_config: AgentConfig, gitea: GiteaClient +) -> None: + base_url = live_config.gitea_url + token = live_config.gitea_token + repo_path = live_config.gitea_repo # the sandbox + client = gitea account = client.whoami() assert "testing" in account, ( diff --git a/tests/integration/test_llm_live.py b/tests/integration/test_llm_live.py index 196487e..db0ef5c 100644 --- a/tests/integration/test_llm_live.py +++ b/tests/integration/test_llm_live.py @@ -1,41 +1,17 @@ -"""Live smoke test against the real LiteLLM proxy. - -NISSE_LLM_BASE_URL alone decides whether this runs: if it is set, the live -path is expected to work, and anything else missing is a *failure*, not a -skip. A half-configured environment must not pass quietly as a skip — that is -how a broken CI credential hides. -""" - -import os -from pathlib import Path +"""Live smoke test against the real LiteLLM proxy.""" import pytest from nisse_dev.canonical import AssistantText, StoredEvent, UserMessage -from nisse_dev.envfile import load_env_file +from nisse_dev.config import AgentConfig from nisse_dev.llm import LiteLLMClient -load_env_file(Path(__file__).resolve().parents[2] / ".env") - -SENTINEL = "NISSE_LLM_BASE_URL" -ALSO_REQUIRED = ("NISSE_LLM_TOKEN", "NISSE_ROLE_EXECUTOR") - -pytestmark = [ - pytest.mark.integration, - pytest.mark.skipif( - not os.environ.get(SENTINEL), - reason=f"no live LLM configured ({SENTINEL} unset)", - ), -] +pytestmark = pytest.mark.integration -def test_executor_model_completes_via_litellm() -> None: - missing = [var for var in ALSO_REQUIRED if not os.environ.get(var)] - assert not missing, ( - f"{SENTINEL} is set, so the live LLM path is expected to work, " - f"but these are missing: {', '.join(missing)}" - ) - client = LiteLLMClient.from_env() +def test_executor_model_completes_via_litellm( + litellm: LiteLLMClient, live_config: AgentConfig +) -> None: events = [ StoredEvent( id=1, @@ -45,9 +21,7 @@ def test_executor_model_completes_via_litellm() -> None: ), ) ] - result = client.complete( - events, tools=None, model=os.environ["NISSE_ROLE_EXECUTOR"] - ) + result = litellm.complete(events, tools=None, model=live_config.roles.executor) texts = [e.text for e in result if isinstance(e, AssistantText)] assert texts, f"no assistant text in {result!r}" assert "pong" in texts[0].lower() diff --git a/tests/integration/test_matrix_live.py b/tests/integration/test_matrix_live.py index 1bf8922..018cf55 100644 --- a/tests/integration/test_matrix_live.py +++ b/tests/integration/test_matrix_live.py @@ -1,36 +1,14 @@ """Live smoke test against the real homeserver, bound strictly to the -dedicated test room (never the production room). +dedicated test room that nisse-test.yaml names (never the production room).""" -NISSE_MATRIX_HOMESERVER alone decides whether this runs: if it is set, the -live path is expected to work, and anything else missing is a *failure*, not -a skip. A half-configured environment must not pass quietly as a skip — that -is how a broken CI credential hides.""" - -import os import uuid -from pathlib import Path import pytest -from nisse_dev.envfile import load_env_file +from nisse_dev.config import AgentConfig from nisse_dev.matrix import MatrixConfig, MatrixTransport -load_env_file(Path(__file__).resolve().parents[2] / ".env") - -SENTINEL = "NISSE_MATRIX_HOMESERVER" -ALSO_REQUIRED = ( - "NISSE_MATRIX_USER_ID", - "NISSE_MATRIX_TOKEN", - "NISSE_MATRIX_TEST_ROOM_ID", -) - -pytestmark = [ - pytest.mark.integration, - pytest.mark.skipif( - not os.environ.get(SENTINEL), - reason=f"no live homeserver configured ({SENTINEL} unset)", - ), -] +pytestmark = pytest.mark.integration def _assert_testing_identity(transport: MatrixTransport, config: MatrixConfig) -> None: @@ -46,7 +24,7 @@ def _assert_testing_identity(transport: MatrixTransport, config: MatrixConfig) - whoami.raise_for_status() actual = whoami.json()["user_id"] assert actual == config.user_id, ( - f"NISSE_MATRIX_USER_ID is {config.user_id} but the token belongs to " + f"matrix.user_id is {config.user_id} but the token belongs to " f"{actual}; the self-message filter would not match" ) assert "testing" in actual, ( @@ -54,13 +32,8 @@ def _assert_testing_identity(transport: MatrixTransport, config: MatrixConfig) - ) -def test_send_lands_in_test_room() -> None: - missing = [var for var in ALSO_REQUIRED if not os.environ.get(var)] - assert not missing, ( - f"{SENTINEL} is set, so the live Matrix path is expected to work, " - f"but these are missing: {', '.join(missing)}" - ) - config = MatrixConfig.from_env(room_var="NISSE_MATRIX_TEST_ROOM_ID") +def test_send_lands_in_test_room(live_config: AgentConfig) -> None: + config = live_config.matrix marker = f"transport smoke test {uuid.uuid4().hex}" transport = MatrixTransport(config) try: diff --git a/tests/integration/test_milestone_live.py b/tests/integration/test_milestone_live.py index 8a8ea62..a10e63c 100644 --- a/tests/integration/test_milestone_live.py +++ b/tests/integration/test_milestone_live.py @@ -2,12 +2,11 @@ approve through the gate, then let the executor role actually create and test a file on disk. -Gated on NISSE_LIVE_PLANNER_MODEL like the planning test — it needs a model -that holds the tool-call contract on both ends. +Uses the test config's planner model for every role: it needs a model that +holds the tool-call contract on both ends. """ import io -import os from pathlib import Path import pytest @@ -19,7 +18,6 @@ from nisse_dev.control import ( EXECUTING, Orchestrator, ) -from nisse_dev.envfile import load_env_file from nisse_dev.executor import LLMExecutor from nisse_dev.llm import LiteLLMClient, RoleMap from nisse_dev.oplog import OpLog @@ -28,17 +26,7 @@ from nisse_dev.state import StateStore from nisse_dev.tools import Workspace from nisse_dev.transport import InMemoryTransport -load_env_file(Path(__file__).resolve().parents[2] / ".env") - -SENTINEL = "NISSE_LIVE_PLANNER_MODEL" - -pytestmark = [ - pytest.mark.integration, - pytest.mark.skipif( - not os.environ.get(SENTINEL), - reason=f"no live planner route configured ({SENTINEL} unset)", - ), -] +pytestmark = pytest.mark.integration def _drive_to_execution( @@ -67,16 +55,14 @@ def _drive_to_execution( ) -def test_plan_approve_execute_produces_a_real_file(tmp_path: Path) -> None: - assert os.environ.get("NISSE_LLM_TOKEN"), ( - f"{SENTINEL} is set, so the live path is expected to work, " - "but NISSE_LLM_TOKEN is missing" - ) +def test_plan_approve_execute_produces_a_real_file( + tmp_path: Path, litellm: LiteLLMClient, live_model: str +) -> None: # Both roles use the capable model here: the point is the whole gated # loop over a real proxy, not local-model competence (measured separately). - model = os.environ[SENTINEL] + model = live_model roles = RoleMap(executor=model, planner=model, delegate=model, consultant=model) - client = LiteLLMClient.from_env() + client = litellm store = StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO())) workspace = Workspace(tmp_path / "work", timeout_s=60) transport = InMemoryTransport() diff --git a/tests/integration/test_model_capability.py b/tests/integration/test_model_capability.py index f5f3ff4..149e352 100644 --- a/tests/integration/test_model_capability.py +++ b/tests/integration/test_model_capability.py @@ -14,7 +14,6 @@ because it spends real tokens across several models. """ import os -from pathlib import Path import pytest @@ -26,14 +25,12 @@ from nisse_dev.canonical import ( UserMessage, ) from nisse_dev.catalog import ModelCatalog, select_client -from nisse_dev.envfile import load_env_file +from nisse_dev.config import AgentConfig from nisse_dev.expert import parse_answer from nisse_dev.llm import AnthropicPassthroughClient, LiteLLMClient, LLMError from nisse_dev.planner import PLANNER_TOOLS from nisse_dev.tools import EXECUTOR_TOOLS -load_env_file(Path(__file__).resolve().parents[2] / ".env") - SENTINEL = "NISSE_CAPABILITY_MODELS" MODELS = [m.strip() for m in os.environ.get(SENTINEL, "").split(",") if m.strip()] @@ -45,17 +42,18 @@ pytestmark = [ ] -def clients() -> tuple[LiteLLMClient, AnthropicPassthroughClient]: - return LiteLLMClient.from_env(), AnthropicPassthroughClient.from_env() +@pytest.fixture +def routes( + litellm: LiteLLMClient, passthrough: AnthropicPassthroughClient +) -> tuple[LiteLLMClient, AnthropicPassthroughClient]: + return litellm, passthrough @pytest.fixture(scope="module") -def catalog() -> ModelCatalog: +def catalog(live_config: AgentConfig) -> ModelCatalog: """The live catalog this key can see. /model/info is scoped per key, so the probe list and the key have to agree.""" - return ModelCatalog.discover( - os.environ["NISSE_LLM_BASE_URL"], os.environ["NISSE_LLM_TOKEN"] - ) + return ModelCatalog.discover(live_config.llm_base_url, live_config.llm_token) def user(text: str) -> list[StoredEvent]: @@ -63,8 +61,8 @@ def user(text: str) -> list[StoredEvent]: @pytest.mark.parametrize("model", MODELS) -def test_model_answers_at_all(model: str, catalog: ModelCatalog) -> None: - normalized, passthrough = clients() +def test_model_answers_at_all(model: str, catalog: ModelCatalog, routes) -> None: + normalized, passthrough = routes client = select_client(model, normalized, passthrough, catalog) produced = client.complete( user("Reply with exactly one word: pong"), None, model, system="Be terse." @@ -74,10 +72,12 @@ def test_model_answers_at_all(model: str, catalog: ModelCatalog) -> None: @pytest.mark.parametrize("model", MODELS) -def test_model_holds_the_planner_contract(model: str, catalog: ModelCatalog) -> None: +def test_model_holds_the_planner_contract( + model: str, catalog: ModelCatalog, routes +) -> None: """A4's gating is structural at the tool layer: a model that answers in prose cannot be gated, because there is no plan to approve.""" - normalized, passthrough = clients() + normalized, passthrough = routes client = select_client(model, normalized, passthrough, catalog) produced = client.complete( user("Draft an architecture plan for a command-line slug generator."), @@ -101,10 +101,12 @@ def test_model_holds_the_planner_contract(model: str, catalog: ModelCatalog) -> @pytest.mark.parametrize("model", MODELS) -def test_model_round_trips_a_tool_result(model: str, catalog: ModelCatalog) -> None: +def test_model_round_trips_a_tool_result( + model: str, catalog: ModelCatalog, routes +) -> None: """The executor loop feeds results back; a model that cannot consume its own tool result cannot run a milestone.""" - normalized, passthrough = clients() + normalized, passthrough = routes client = select_client(model, normalized, passthrough, catalog) events = user("List the files in the current directory using your tools.") produced = client.complete(events, EXECUTOR_TOOLS, model, system="Use a tool.") @@ -130,11 +132,11 @@ def test_model_round_trips_a_tool_result(model: str, catalog: ModelCatalog) -> N @pytest.mark.parametrize("model", MODELS) def test_model_holds_the_prompt_level_json_contract( - model: str, catalog: ModelCatalog + model: str, catalog: ModelCatalog, routes ) -> None: """A14 contracts structured output at the prompt level because vendors differ on JSON enforcement. This measures whether that holds per model.""" - normalized, passthrough = clients() + normalized, passthrough = routes client = select_client(model, normalized, passthrough, catalog) produced = client.complete( user("Is SQLite sufficient for a single-process agent's task state?"), @@ -153,11 +155,13 @@ def test_model_holds_the_prompt_level_json_contract( @pytest.mark.parametrize("model", MODELS) -def test_route_selection_matches_the_model(model: str, catalog: ModelCatalog) -> None: +def test_route_selection_matches_the_model( + model: str, catalog: ModelCatalog, routes +) -> None: """A non-Anthropic model must not be sent Anthropic-shaped bodies. This was hardcoded until gpt-6-astra made it observable, and name-sniffed until semantic aliases made the name stop meaning anything.""" - normalized, passthrough = clients() + normalized, passthrough = routes facts = catalog.facts(model) chosen = select_client(model, normalized, passthrough, catalog) if facts.provider == "anthropic": diff --git a/tests/integration/test_passthrough_live.py b/tests/integration/test_passthrough_live.py index f5c9191..e5d0812 100644 --- a/tests/integration/test_passthrough_live.py +++ b/tests/integration/test_passthrough_live.py @@ -6,30 +6,19 @@ passthrough rather than being flattened by the proxy. """ import io -import os from pathlib import Path import httpx import pytest from nisse_dev.canonical import AssistantText, ToolCall, ToolResult, UserMessage -from nisse_dev.envfile import load_env_file +from nisse_dev.config import AgentConfig from nisse_dev.llm import AnthropicPassthroughClient from nisse_dev.oplog import OpLog from nisse_dev.state import StateStore from nisse_dev.tools import EXECUTOR_TOOLS -load_env_file(Path(__file__).resolve().parents[2] / ".env") - -SENTINEL = "NISSE_LIVE_PLANNER_MODEL" - -pytestmark = [ - pytest.mark.integration, - pytest.mark.skipif( - not os.environ.get(SENTINEL), - reason=f"no live frontier route configured ({SENTINEL} unset)", - ), -] +pytestmark = pytest.mark.integration def store_with(tmp_path: Path, *events) -> StateStore: @@ -39,18 +28,22 @@ def store_with(tmp_path: Path, *events) -> StateStore: return store -def test_passthrough_completes_a_plain_turn(tmp_path: Path) -> None: +def test_passthrough_completes_a_plain_turn( + tmp_path: Path, passthrough: AnthropicPassthroughClient, live_model: str +) -> None: store = store_with( tmp_path, UserMessage(text="Reply with exactly: pong", source="live") ) - result = AnthropicPassthroughClient.from_env().complete( - store.read_events(), None, os.environ[SENTINEL], system="Be terse." + result = passthrough.complete( + store.read_events(), None, live_model, system="Be terse." ) texts = [e.text for e in result if isinstance(e, AssistantText)] assert texts and "pong" in texts[0].lower() -def test_passthrough_round_trips_a_tool_call(tmp_path: Path) -> None: +def test_passthrough_round_trips_a_tool_call( + tmp_path: Path, passthrough: AnthropicPassthroughClient, live_model: str +) -> None: store = store_with( tmp_path, UserMessage( @@ -58,11 +51,11 @@ def test_passthrough_round_trips_a_tool_call(tmp_path: Path) -> None: source="live", ), ) - client = AnthropicPassthroughClient.from_env() + client = passthrough produced = client.complete( store.read_events(), EXECUTOR_TOOLS, - os.environ[SENTINEL], + live_model, system="Use a tool. Do not answer in prose.", ) calls = [e for e in produced if isinstance(e, ToolCall)] @@ -75,18 +68,18 @@ def test_passthrough_round_trips_a_tool_call(tmp_path: Path) -> None: store.append_event( ToolResult(call_id=calls[0].call_id, result="app.py\ntests/", success=True) ) - follow_up = client.complete( - store.read_events(), EXECUTOR_TOOLS, os.environ[SENTINEL] - ) + follow_up = client.complete(store.read_events(), EXECUTOR_TOOLS, live_model) assert follow_up, "the vendor rejected or ignored the tool_result turn" -def test_prompt_caching_survives_the_passthrough() -> None: +def test_prompt_caching_survives_the_passthrough( + live_config: AgentConfig, live_model: str +) -> None: # A5 asks for this to be verified rather than assumed: a proxy that # flattened the request would silently drop cache control and turn a # cost optimisation into a per-call expense. - base = os.environ["NISSE_LLM_BASE_URL"].rstrip("/") - token = os.environ["NISSE_LLM_TOKEN"] + base = live_config.llm_base_url.rstrip("/") + token = live_config.llm_token response = httpx.post( f"{base}/v1/messages", headers={ @@ -95,7 +88,7 @@ def test_prompt_caching_survives_the_passthrough() -> None: "anthropic-version": "2023-06-01", }, json={ - "model": os.environ[SENTINEL], + "model": live_model, "max_tokens": 16, "messages": [{"role": "user", "content": "Say pong."}], }, diff --git a/tests/integration/test_planning_live.py b/tests/integration/test_planning_live.py index e49044f..821ccec 100644 --- a/tests/integration/test_planning_live.py +++ b/tests/integration/test_planning_live.py @@ -1,47 +1,30 @@ """Live planning turn: a real model must answer with a submit_plan or ask_human tool call, not prose. -Deliberately opt-in via NISSE_LIVE_PLANNER_MODEL rather than the usual -NISSE_LLM_BASE_URL sentinel. Planning escalates to a frontier model -unconditionally (A12), and the small local model does not hold the tool-call -contract reliably — measured, see the PR — so this test needs a frontier -route that does not exist yet. Set the variable to the model to plan with -once one is configured; until then this skips on purpose, not by accident. +Plans with the test config's planner role. Planning escalates to a frontier +model unconditionally (A12), and small local models do not hold the tool-call +contract reliably, so that role must be one that does. """ import io -import os from pathlib import Path import pytest from nisse_dev.canonical import UserMessage from nisse_dev.control import AskHuman, SubmitPlan -from nisse_dev.envfile import load_env_file from nisse_dev.llm import LiteLLMClient, RoleMap from nisse_dev.oplog import OpLog from nisse_dev.planner import LLMPlanningAgent from nisse_dev.state import StateStore -load_env_file(Path(__file__).resolve().parents[2] / ".env") - -SENTINEL = "NISSE_LIVE_PLANNER_MODEL" - -pytestmark = [ - pytest.mark.integration, - pytest.mark.skipif( - not os.environ.get(SENTINEL), - reason=f"no frontier planner route configured ({SENTINEL} unset)", - ), -] +pytestmark = pytest.mark.integration -def test_live_planning_turn_returns_a_tool_action(tmp_path: Path) -> None: - assert os.environ.get("NISSE_LLM_TOKEN"), ( - f"{SENTINEL} is set, so the live planning path is expected to work, " - "but NISSE_LLM_TOKEN is missing" - ) - model = os.environ[SENTINEL] +def test_live_planning_turn_returns_a_tool_action( + tmp_path: Path, litellm: LiteLLMClient, live_model: str +) -> None: + model = live_model roles = RoleMap(executor=model, planner=model, delegate=model, consultant=model) store = StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO())) store.append_event( @@ -53,7 +36,7 @@ def test_live_planning_turn_returns_a_tool_action(tmp_path: Path) -> None: source="live-test", ) ) - agent = LLMPlanningAgent(LiteLLMClient.from_env(), store, roles) + agent = LLMPlanningAgent(litellm, store, roles) action = agent.plan("arch", "hello-cli")