From 0f67ff68b861c4efc013d755d2c1271fa3f0ee6e Mon Sep 17 00:00:00 2001 From: Xuezhi Yan Date: Tue, 13 Oct 2026 23:26:56 -0400 Subject: [PATCH 1/8] refactor(harness)!: declare adapter config checks with matchers in descriptors Replace files.config..locked with checks: a key of the merged config, a GoogleTest-style matcher expression every value there must match, and an optional reason. The shared render runs them before any adapter step. Move the config rules of claude, codex, dsh, hermes, pi and terminus from quirks.py into their descriptors. Reference implementation of RFC #797; not to be merged before it is accepted. Co-Authored-By: Claude Opus 5.5 --- docs/developer-guide/harness-adapters.rst | 110 ++++- reef/harness/adapters/claude/descriptor.yaml | 83 +++- reef/harness/adapters/claude/quirks.py | 113 +---- reef/harness/adapters/codex/descriptor.yaml | 62 +++ reef/harness/adapters/codex/quirks.py | 125 +---- reef/harness/adapters/descriptor.py | 138 +++--- reef/harness/adapters/dsh/descriptor.yaml | 84 +++- reef/harness/adapters/dsh/quirks.py | 120 +---- reef/harness/adapters/hermes/descriptor.yaml | 94 +++- reef/harness/adapters/hermes/quirks.py | 157 +------ reef/harness/adapters/matchers.py | 443 ++++++++++++++++++ .../harness/adapters/opencode/descriptor.yaml | 23 +- reef/harness/adapters/pi/descriptor.yaml | 24 +- reef/harness/adapters/pi/quirks.py | 41 +- .../harness/adapters/terminus/descriptor.yaml | 55 +++ reef/harness/adapters/terminus/quirks.py | 93 +--- reef/harness/tree/render.py | 87 +--- .../test_harness_config_checks.py | 241 ++++++++++ .../reef_service/test_harness_model_route.py | 28 +- tests/reef_service/test_harness_render.py | 209 +++------ tests/reef_service/test_terminus_agent.py | 10 +- 21 files changed, 1364 insertions(+), 976 deletions(-) create mode 100644 reef/harness/adapters/matchers.py create mode 100644 tests/reef_service/test_harness_config_checks.py diff --git a/docs/developer-guide/harness-adapters.rst b/docs/developer-guide/harness-adapters.rst index 10719c6f0..cf24e43bb 100644 --- a/docs/developer-guide/harness-adapters.rst +++ b/docs/developer-guide/harness-adapters.rst @@ -79,11 +79,9 @@ whatever the adapter declares; for example: - An entry whose kind the descriptor gives no path, such as ``code_extension`` under ``native``. - A ``config`` entry naming a target the descriptor does not declare. -- A ``config`` entry that moves a key of the target's ``locked`` list off the - value its ``defaults`` set, or that sets a locked key the defaults never - name. A locked path is dotted key names with ``*`` for any key of an object - or any item of a list, and the check runs before the adapter's own steps, so - no adapter can skip it. +- A merged config that fails one of its target's ``checks`` (see + `Config checks`_). The checks run before the adapter's own steps, so no + adapter can skip them. - A ``native_graph`` or ``native_agent`` naming a tool, skill, agent, or graph that no entry defines, or agents that delegate in a cycle. - A second ``native_loop``: a tree holds at most one. @@ -93,8 +91,8 @@ What the adapter owns An adapter is configured mostly via declaration. ``descriptor.yaml`` supplies the path template for each tree node kind, the named config targets, the -defaults merged under every tree, and the keys a tree must keep (``locked``). -An adapter whose agent differs only in where +defaults merged under every tree, and the checks the merged config must pass +(``checks``). An adapter whose agent differs only in where files go needs no code at all, although they must comply with the following rules: @@ -102,6 +100,90 @@ rules: - ``files.rules`` and ``files.skill`` are required. - every named template must contain ``{name}``. +Config checks +^^^^^^^^^^^^^ + +A config target's ``checks`` state what its merged config must keep or must +not set. Each check has a ``key``, a ``match`` expression that every value at +the key must match, and an optional ``reason`` that a refusal gives: + +.. code-block:: yaml + + files: + config: + primary: + path: "claude/settings.json" + defaults: + includeCoAuthoredBy: false + checks: + - key: includeCoAuthoredBy + match: IsDefault() + - key: [model, fallbackModel] + match: IsAbsent() + reason: Reef's model binding chooses the model + - key: env.DISABLE_TELEMETRY + match: Not(Strip(Lowercase(IsIn(["0", "false", "off", "no", ""])))) + reason: benchmark episodes keep telemetry off + +The render runs the checks on the merged config, with the model binding +applied when there is one, before any adapter step. Reef owns the matchers; +a descriptor cannot add one. The render refuses a tree whose config fails a +check, for example:: + + claude composition fails the check on env key 'AWS_REGION' in + claude/settings.json: Reef's model binding chooses the endpoint, ... + +A refusal names the key, and for ``Each`` the failing key or item, but never +the value, which can be a credential the binding wrote. A check without a +``reason`` gives its match expression instead. + +**Keys.** ``key`` is required: dot-separated key names, or a list of them +(one check per key). ``.`` names the whole file, for a check on its top-level +key names; a check without a key fails to load. A ``*`` segment is every key of an +object and every item of a list. A ``**`` segment is any number of segments, +zero included. A key the config does not set is absent. A ``*`` or ``**`` +segment visits only the values the config has. + +**Matchers.** Write a match expression as nested calls with JSON literals +(``true``, ``false`` and ``null`` included). Use ``|-`` in YAML to wrap a long +one over several lines. + +.. list-table:: + :header-rows: 1 + + * - Matcher + - Matches + * - ``IsDefault()`` + - The value the target's ``defaults`` give the key, or an absent key when + they give none. Only for a key without ``*`` or ``**``. + * - ``IsAbsent()`` + - A key the config does not set. An empty value is a set one. + * - ``Eq(v)``, ``IsIn([v, ...])`` + - A value equal to ``v``, or to one of the list, with its type: ``false`` + matches neither ``0`` nor ``"false"``. + * - ``IsObject()``, ``IsInt()``, ``Ge(n)`` + - An object; an integer that is not a boolean; a number at least ``n``. + * - ``StartsWith(s)``, ``EndsWith(s)``, ``HasSubstr(s)``, ``MatchesRegex(s)`` + - Text with that prefix, suffix or substring, or text that the regular + expression matches as a whole. + * - ``Lowercase(m)``, ``Uppercase(m)``, ``Strip(m)`` + - Text that matches ``m`` after the change. + * - ``ParseJson(m)`` + - A string holding JSON whose value matches ``m``. + * - ``Each(m)``, ``Contains(m)`` + - Every entry of an object or item of a list matches ``m``, or at least + one does. ``Each`` also matches an absent key. + * - ``Key(m)``, ``Value(m)`` + - An object entry whose key, or whose value, matches ``m``. Use them + inside ``Each`` and ``Contains``. + * - ``AllOf(m, ...)``, ``AnyOf(m, ...)``, ``Not(m)`` + - Every matcher matches, at least one does, or ``m`` does not. + +Text matchers read a string as it is, and a number, a boolean or null as its +JSON text, so ``Lowercase(Eq("0"))`` matches ``0``. They match no object, list +or absent key. Only ``IsAbsent``, ``Not`` and ``Each`` match an absent key, so +"if it is set, it must be X" is ``AnyOf(IsAbsent(), X)``. + An adapter may also declare a ``quirks`` module to further customize the rendering process. The module defines one subclass of ``AdapterRenderer`` from ``reef.harness.adapters.descriptor``; the loader finds it in the module. @@ -116,7 +198,7 @@ files: - Use it to * - ``process_config`` - One merged config file, as a JSON object - - Refuse or adjust config, such as a shape the declaration cannot state + - Adjust config, or refuse a config a check cannot describe * - ``process_skill`` - The text of one ``skill`` file - Add or check skill frontmatter @@ -125,8 +207,8 @@ files: - Add or check command frontmatter * - ``check_model_route`` - The processed config files, skill files and command files - - Refuse the model routes the harness reads beyond the binding's keys, - such as a fallback model or a second provider + - Refuse a model route that no check can describe, such as a model in + the frontmatter of a skill or a command * - ``finalize_render`` - Every rendered file as text, each config file already written in its format @@ -146,6 +228,9 @@ gets the same values from ``skill_frontmatter(path, text)``. .. important:: + Write a rule on config values as a config check, not as a step: a check is + declared beside the defaults it protects and needs no code. + Prefer the narrow steps over ``finalize_render``. A ``process_*`` step sees one file of one kind, and ``check_model_route`` only reads, so a reader can tell what each one may change and a reviewer can check it in isolation. @@ -162,8 +247,9 @@ credential (the value holding ``{api_key}``) is set beside it. The binding is merged after the tree and wins every key it writes, so only a tree rendered without the binding, as admission renders a proposal, can set one; admission refuses an inline credential, so the credential tells the two apart. No -adapter can turn this check off. Each adapter's ``check_model_route`` refuses -the other routes its harness reads. +adapter can turn this check off. Each adapter refuses the other routes its +harness reads, such as a fallback model or a second provider, in its config +checks, and in ``check_model_route`` what the checks cannot describe. Every step is a static method, and defining one as an ordinary method fails when the class is created. The render uses the class without making an diff --git a/reef/harness/adapters/claude/descriptor.yaml b/reef/harness/adapters/claude/descriptor.yaml index 22bcd82e1..b3c17019f 100644 --- a/reef/harness/adapters/claude/descriptor.yaml +++ b/reef/harness/adapters/claude/descriptor.yaml @@ -43,7 +43,7 @@ env: # only with nothing else on the command line, stops at "unknown option" after # the --settings below. reef-claude update is reef's own install of the # served release. Claude Code copies settings.env over its environment, so -# the quirks refuse a tree whose settings.env sets either switch; a project +# the checks below refuse a tree whose settings.env sets either switch; a project # .claude/settings.json of the person's own can still set them. client_env: DISABLE_AUTOUPDATER: "1" @@ -85,29 +85,64 @@ files: # Covers a run where the person passes their own --settings, which # replaces the one in client_args. disableDeepLinkRegistration: "disable" - # Keys the merged config must keep at the defaults' value, and settings - # keys no tree may set at all: the shared render refuses a composition - # that moves them, before any quirks step runs. The locked settings - # choose the model, a credential helper or a login method, which Reef's - # model binding owns. - locked: - - includeCoAuthoredBy - - disableDeepLinkRegistration - - advisorModel - - apiKeyHelper - - availableModels - - awsAuthRefresh - - awsCredentialExport - - enforceAvailableModels - - fallbackModel - - forceLoginGatewayUrl - - forceLoginMethod - - gcpAuthRefresh - - model - - modelOverrides - - modelPicker - - proxyAuthHelper - - switchModelsOnFlag + # The shared render refuses a composition that fails one of these + # checks, before any quirks step runs. + checks: + - key: [includeCoAuthoredBy, disableDeepLinkRegistration] + match: IsDefault() + # Settings that choose the model, a credential helper or a login + # method, which Reef's model binding owns. + - key: [ + advisorModel, apiKeyHelper, availableModels, awsAuthRefresh, awsCredentialExport, + enforceAvailableModels, fallbackModel, forceLoginGatewayUrl, forceLoginMethod, gcpAuthRefresh, + model, modelOverrides, modelPicker, proxyAuthHelper, switchModelsOnFlag, + ] + match: IsAbsent() + reason: Reef's model binding chooses the model, the credential and the login method + # Claude Code copies settings.env over its environment. It reads the + # updater switches as on only for 1, true, yes or on, so a tree sets + # neither, under any name Windows matches without case. + - key: env + match: Each(Key(Not(Uppercase(IsIn(["DISABLE_AUTOUPDATER", "DISABLE_UPDATES"]))))) + reason: the episode env and reef-claude set it so Claude Code keeps the pinned version + - key: [env.DISABLE_TELEMETRY, env.CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC] + match: Not(Strip(Lowercase(IsIn(["0", "false", "off", "no", ""])))) + reason: benchmark episodes keep telemetry and non-essential traffic off + # The env names Claude Code 2.1.257 reads to choose the endpoint, the + # provider, the credential or the model of a model call: every + # ANTHROPIC_ name, the cloud providers and their credentials, the + # provider switches, proxies, endpoints, sockets, credential helpers + # and files, and model names. The binding writes the first three, and + # the render refuses them from a tree without the binding's token. + - key: env + match: |- + Each(Key(AnyOf( + IsIn(["ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_BASE_URL", "ANTHROPIC_MODEL"]), + Not(Uppercase(AnyOf( + StartsWith("ANTHROPIC_"), StartsWith("AWS_"), StartsWith("AZURE_"), StartsWith("GOOGLE_"), + StartsWith("GCLOUD_"), StartsWith("GCE_"), StartsWith("CLOUDSDK_"), StartsWith("CLOUD_ML_"), + StartsWith("VERTEX_"), + StartsWith("CLAUDE_CODE_USE_ANTHROPIC_"), StartsWith("CLAUDE_CODE_USE_BEDROCK"), + StartsWith("CLAUDE_CODE_USE_FOUNDRY"), StartsWith("CLAUDE_CODE_USE_GATEWAY"), + StartsWith("CLAUDE_CODE_USE_MANTLE"), StartsWith("CLAUDE_CODE_USE_VERTEX"), + StartsWith("CLAUDE_CODE_PROVIDER_"), AllOf(StartsWith("CLAUDE_CODE_SKIP_"), EndsWith("_AUTH")), + EndsWith("_MODEL"), HasSubstr("_MODEL_CATALOG"), HasSubstr("_MODEL_OPTION"), HasSubstr("_MODEL_FORCE"), + HasSubstr("PROXY"), HasSubstr("BASE_URL"), HasSubstr("ENDPOINT"), HasSubstr("SOCKET"), + HasSubstr("API_KEY"), HasSubstr("AUTH_"), HasSubstr("CREDS"), HasSubstr("CREDENTIAL"), + HasSubstr("HELPER"), HasSubstr("FILE_DESCRIPTOR") + ))) + ))) + reason: Reef's model binding chooses the endpoint, the credential and the model + # Claude Code merges the JSON object in CLAUDE_CODE_EXTRA_BODY into + # every request body, over the bound model, so the object names no + # model (model, or OpenRouter's fallback models). + - key: env + match: |- + Each(AnyOf( + Key(Not(Uppercase(Eq("CLAUDE_CODE_EXTRA_BODY")))), + Value(ParseJson(AllOf(IsObject(), Not(Contains(Key(IsIn(["model", "models"]))))))) + )) + reason: Reef's model binding chooses the model, so CLAUDE_CODE_EXTRA_BODY must be a JSON object without model or models rules: "claude/CLAUDE.md" agent_command: "claude/commands/{name}.md" skill: "claude/skills/{name}/SKILL.md" diff --git a/reef/harness/adapters/claude/quirks.py b/reef/harness/adapters/claude/quirks.py index 1e14ade08..b862ec696 100644 --- a/reef/harness/adapters/claude/quirks.py +++ b/reef/harness/adapters/claude/quirks.py @@ -1,4 +1,4 @@ -"""Claude Code adapter quirks: boot artifacts and enforced config invariants. +"""Claude Code adapter quirks: boot artifacts and the model a command or skill frontmatter names. Claude Code's boot writes local state beside the rendered config under CLAUDE_CONFIG_DIR: a top-level ``.claude.json`` state file, a ``statsig/`` @@ -6,31 +6,18 @@ The descriptor whitelists those so the episode inverse tolerates them and reports anything else as residue. -``process_config`` enforces what the declaration cannot state. Claude Code -copies ``settings.env`` over its own environment. The episode env turns -telemetry and non-essential traffic off, so ``settings.env`` must not set -either to ``0``, ``false``, ``off``, ``no`` or an empty value. The episode -env and ``reef-claude`` set ``DISABLE_AUTOUPDATER`` and ``DISABLE_UPDATES`` -so that Claude Code's updater and its own ``update`` and ``install`` keep -the pinned version; Claude Code reads them as on only for ``1``, ``true``, -``yes`` or ``on``, so ``settings.env`` must not set either one at all, under -any name Windows matches without case. The keys a settings.json must keep, -and the settings keys that choose the model, a credential helper or a login -method, are declared in the descriptor and refused by the shared render -before this step runs. - -It also keeps every model call on Reef's model binding. The binding writes -``ANTHROPIC_BASE_URL``, ``ANTHROPIC_AUTH_TOKEN`` and ``ANTHROPIC_MODEL`` into -``settings.env``, renders after the tree and wins every name it writes. Its -token is a credential, which a tree cannot hold because admission refuses an -inline credential, so the three names pass only beside the binding's token. -Any other env name Claude Code reads to choose the endpoint (another -provider such as Bedrock or Vertex, a proxy), the credential or the model, -and a ``model`` in the frontmatter of a command or a skill are the -tree choosing where calls go, and are refused. Claude Code merges the JSON -object in ``CLAUDE_CODE_EXTRA_BODY`` into every request body, over the bound -model, so that object may not name a model (``model``, or OpenRouter's -fallback ``models``). +The descriptor's checks keep the settings.json keys a tree must keep, refuse +the settings keys that choose the model, a credential helper or a login +method, and hold ``settings.env``: it must not turn the updater switches, +telemetry or non-essential traffic back on, and must not set an env name +Claude Code reads to choose the endpoint, the credential or the model, +beside the three names Reef's model binding writes. The shared render runs +them before this module's steps. + +Every model call stays on Reef's model binding, and a ``model`` in the +frontmatter of a command or a skill is the tree choosing where calls go. +``check_model_route`` refuses it, reading the frontmatter as Claude Code +reads it, which no config check can. These checks cover the config the tree renders. Reef's proxy forwards the ``model`` and ``models`` a request sends as they are, so a request that other @@ -40,7 +27,6 @@ from __future__ import annotations -import json import re from collections.abc import Mapping from typing import Any @@ -50,31 +36,6 @@ from reef.harness.adapters.descriptor import AdapterRenderer from reef.harness.tree.render import RenderError -_CONFIG_PATH = "claude/settings.json" - -# The env switches the descriptor relies on to keep episodes hermetic. A -# rendered settings.env that sets any of these to a falsey value would undo -# the descriptor's own env and let the binary phone home mid-campaign. -_HERMETIC_ENV = ("DISABLE_TELEMETRY", "CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC") -_FALSEY = {"0", "false", "off", "no", ""} - -#: The env names Reef's model binding writes. -BINDING_ENV = ("ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_BASE_URL", "ANTHROPIC_MODEL") -#: The env names Claude Code 2.1.257 reads to choose the endpoint, the provider, the credential or the model of a -#: model call: every ANTHROPIC_ name, the cloud providers and their credentials, the provider switches, proxies, -#: endpoints, sockets, credential helpers and files, and model names. Matched without case, as Windows reads env. -MODEL_ROUTE_ENV = re.compile( - r"^(?:ANTHROPIC_|AWS_|AZURE_|GOOGLE_|GCLOUD_|GCE_|CLOUDSDK_|CLOUD_ML_|VERTEX_)" - r"|^CLAUDE_CODE_(?:USE_(?:ANTHROPIC_|BEDROCK|FOUNDRY|GATEWAY|MANTLE|VERTEX)|SKIP_\w*_AUTH$|PROVIDER_)" - r"|_MODEL$|_MODEL_(?:CATALOG|OPTION|FORCE)" - r"|PROXY|BASE_URL|ENDPOINT|SOCKET|API_KEY|AUTH_|CREDS|CREDENTIAL|HELPER|FILE_DESCRIPTOR", - re.IGNORECASE, -) -#: The env name whose JSON object Claude Code merges into every request body, after the bound model. -EXTRA_BODY_ENV = "CLAUDE_CODE_EXTRA_BODY" -#: Request body fields that name the model: ``model`` replaces the bound one, and OpenRouter reads ``models`` as -#: fallback models. -BODY_MODEL_KEYS = ("model", "models") #: Claude Code's frontmatter block: after a byte order mark, an opening --- line, then the text up to the next ---. FRONTMATTER_BLOCK = re.compile(r"---\s*\n([\s\S]*?)---\s*\n?") #: A plain ``key: value`` line and the value characters Claude Code quotes when its YAML reader refuses the block. @@ -139,59 +100,11 @@ def frontmatter_chooses_model(text: str) -> bool: return "model" in block or "\\" in block -def extra_body_names_model(value: object) -> bool: - """Whether a ``CLAUDE_CODE_EXTRA_BODY`` value can name a model; a value render cannot read as an object counts. - - Claude Code strips a byte order mark and reads the value with JSON.parse. - """ - if not isinstance(value, str): - return True - try: - body = json.loads(value.removeprefix("\ufeff")) - except (ValueError, RecursionError): - return True - return not isinstance(body, dict) or any(key in body for key in BODY_MODEL_KEYS) - - class ClaudeAdapterRenderer(AdapterRenderer): - @staticmethod - def process_config(path: str, config: dict[str, Any]) -> dict[str, Any]: - env = config.get("env") - if isinstance(env, dict): - # The episode env and reef-claude set the updater switches, and a tree value other than 1, true, yes or - # on would turn the updater back on. Windows matches env names in any case. - for key in env: - if key.upper() in ("DISABLE_AUTOUPDATER", "DISABLE_UPDATES"): - raise RenderError( - f"claude composition must not set {key} in settings.env: the episode env and reef-claude " - "set it so Claude Code keeps the pinned version" - ) - for key in _HERMETIC_ENV: - if key in env and str(env[key]).strip().lower() in _FALSEY: - raise RenderError(f"claude composition must not re-enable {key} for benchmark episodes") - return config - @staticmethod def check_model_route( configs: Mapping[str, Mapping[str, Any]], skills: Mapping[str, str], commands: Mapping[str, str] ) -> None: - config = configs[_CONFIG_PATH] - env = config.get("env") - if isinstance(env, dict): - for name in env: - # The render refuses the binding's names from a tree without the binding's token. - if name in BINDING_ENV: - continue - if MODEL_ROUTE_ENV.search(name): - raise RenderError( - f"claude composition must not set env {name}: Reef's model binding chooses the endpoint, " - "the credential and the model" - ) - if name.upper() == EXTRA_BODY_ENV and extra_body_names_model(env[name]): - raise RenderError( - f"claude composition env {name} must be a JSON object without model or models: " - "Reef's model binding chooses the model" - ) for path, text in {**skills, **commands}.items(): if frontmatter_chooses_model(text): raise RenderError( diff --git a/reef/harness/adapters/codex/descriptor.yaml b/reef/harness/adapters/codex/descriptor.yaml index 04da901c5..b83bcfef3 100644 --- a/reef/harness/adapters/codex/descriptor.yaml +++ b/reef/harness/adapters/codex/descriptor.yaml @@ -64,8 +64,70 @@ files: trace_exporter: none sandbox_workspace_write: network_access: false + # The shared render refuses a composition that fails one of these + # checks, before any quirks step runs. web_search may take any value + # Codex reads, because the episode argv pins it disabled and only a + # person's reef-codex session reads the tree's value. + checks: + # The episode argv pins never, and a reef-codex session keeps Codex's + # on-request default, so the person approves each command that leaves + # the sandbox. + - key: approval_policy + match: IsAbsent() + reason: episodes pin never, and a reef-codex session keeps on-request + # The config.toml keys a tree may set, and the keys Reef's model + # binding writes, which the render refuses from a tree without it. + # The entire file is treated as a dict. + - key: "." + match: |- + Each(Key(IsIn([ + "analytics", "check_for_update_on_startup", "features", "feedback", "model_auto_compact_token_limit", + "model_auto_compact_token_limit_scope", "model_context_window", "model_reasoning_effort", + "model_reasoning_summary", "model_verbosity", "otel", "sandbox_workspace_write", + "tool_output_token_limit", "web_search", + "model", "model_provider", "model_providers" + ]))) + reason: the other codex config keys are not admitted for benchmark episodes + - key: web_search + match: IsIn(["disabled", "cached", "indexed", "live"]) + - key: [ + check_for_update_on_startup, analytics.enabled, feedback.enabled, otel, + sandbox_workspace_write.network_access, + features.apps, features.hooks, features.plugins, features.shell_snapshot, + features.skill_mcp_dependency_install, + ] + match: IsDefault() + - key: features + match: |- + Each(Key(IsIn([ + "apps", "enable_request_compression", "hooks", "plugins", "shell_snapshot", + "skill_mcp_dependency_install" + ]))) + - key: sandbox_workspace_write + match: Each(Key(IsIn(["network_access", "writable_roots"]))) + - key: sandbox_workspace_write.writable_roots + match: AnyOf(IsAbsent(), Eq([])) + # The provider Reef's model binding writes, with only its keys. + - key: model_provider + match: AnyOf(IsAbsent(), Eq("reef")) + reason: Reef's model binding chooses the model provider + - key: model_providers + match: Each(Key(Eq("reef"))) + reason: Reef's model binding chooses the model provider + - key: model_providers.* + match: |- + Each(Key(IsIn(["base_url", "experimental_bearer_token", "name", "supports_websockets", "wire_api"]))) + reason: Reef's model binding chooses the model provider models: path: "codex/models.json" + # Model metadata by model name, which the quirks module turns into + # Codex's model catalog. + checks: + # The entire file is treated as a dict. + - key: "." + match: Each(Key(Eq("models"))) + - key: models + match: AnyOf(IsAbsent(), IsObject()) rules: "codex/AGENTS.md" # Codex 0.153.4 loads no custom prompts (its TUI refuses /name), so an # agent_command renders as a skill the person types as $name. A skill and diff --git a/reef/harness/adapters/codex/quirks.py b/reef/harness/adapters/codex/quirks.py index cf7565c9d..316fe95b3 100644 --- a/reef/harness/adapters/codex/quirks.py +++ b/reef/harness/adapters/codex/quirks.py @@ -1,15 +1,13 @@ """Codex adapter quirks: TOML, skill metadata, and safety invariants. Reef config nodes are JSON objects for every adapter, while Codex reads user -configuration as TOML. ``process_config`` validates the benchmark invariants, -and the render writes ``config.toml`` as TOML by its suffix. Codex skills require ``name`` and -``description`` frontmatter. ``process_skill`` adds it when an evolved -skill omits it. An agent_command renders as a skill in the same root, -so it gets the same frontmatter. ``web_search`` may take any value Codex reads, because the -episode argv pins it disabled and only a person's reef-codex session reads -the tree's value. ``approval_policy`` is refused: the episode argv pins it -never, and a reef-codex session keeps Codex's on-request default, so the -person approves each command that leaves the sandbox. +configuration as TOML. The descriptor's checks hold the benchmark invariants +and the model provider, and the render writes ``config.toml`` as TOML by its +suffix. ``process_config`` turns the model metadata in ``models.json`` into +Codex's model catalog. Codex skills require ``name`` and ``description`` +frontmatter. ``process_skill`` adds it when an evolved skill omits it. An +agent_command renders as a skill in the same root, so it gets the same +frontmatter. Codex can run lifecycle hooks, but hook subprocesses do not share Codex's inner command sandbox. ``finalize_render`` therefore rejects ``code_extension`` @@ -35,89 +33,6 @@ CONFIG = "codex/config.toml" MODELS = "codex/models.json" EXTENSIONS = "codex/extensions/" -WEB_SEARCH_MODES = ("disabled", "cached", "indexed", "live") - -#: The config.toml keys a tree may set. -ALLOWED_CONFIG_KEYS = { - "analytics", - "check_for_update_on_startup", - "features", - "feedback", - "model_auto_compact_token_limit", - "model_auto_compact_token_limit_scope", - "model_context_window", - "model_reasoning_effort", - "model_reasoning_summary", - "model_verbosity", - "otel", - "sandbox_workspace_write", - "tool_output_token_limit", - "web_search", -} -#: The config.toml keys Reef's model binding writes; the render refuses each from a tree without the binding. -BINDING_CONFIG_KEYS = {"model", "model_provider", "model_providers"} -ALLOWED_FEATURES = { - "apps", - "enable_request_compression", - "hooks", - "plugins", - "shell_snapshot", - "skill_mcp_dependency_install", -} -DISABLED_FEATURES = {"apps", "hooks", "plugins", "shell_snapshot", "skill_mcp_dependency_install"} -ALLOWED_PROVIDER_KEYS = { - "base_url", - "experimental_bearer_token", - "name", - "supports_websockets", - "wire_api", -} -OTEL_DEFAULTS = { - "exporter": "none", - "log_user_prompt": False, - "metrics_exporter": "none", - "trace_exporter": "none", -} - - -def validate_config(config: dict[str, Any]) -> None: - if "approval_policy" in config: - raise RenderError( - "codex composition may not set approval_policy: episodes pin never, and a reef-codex session keeps" - " on-request so the person approves each command that leaves the sandbox" - ) - extra = sorted(set(config) - ALLOWED_CONFIG_KEYS - BINDING_CONFIG_KEYS) - if extra: - raise RenderError(f"codex config keys are not admitted for benchmark episodes: {', '.join(extra)}") - if config.get("web_search") not in WEB_SEARCH_MODES: - raise RenderError(f"codex web_search must be one of {', '.join(WEB_SEARCH_MODES)}") - if config.get("check_for_update_on_startup") is not False: - raise RenderError("codex composition must keep check_for_update_on_startup false") - for section in ("analytics", "feedback"): - settings = config.get(section) - if not isinstance(settings, dict) or settings.get("enabled") is not False: - raise RenderError(f"codex composition must keep {section}.enabled false") - features = config.get("features") - if not isinstance(features, dict): - raise RenderError("codex composition must keep features as an object") - extra_features = sorted(set(features) - ALLOWED_FEATURES) - if extra_features: - raise RenderError(f"codex feature keys are not admitted: {', '.join(extra_features)}") - for feature in DISABLED_FEATURES: - if features.get(feature) is not False: - raise RenderError(f"codex composition must keep features.{feature} disabled") - - otel = config.get("otel") - if otel != OTEL_DEFAULTS: - raise RenderError("codex composition must keep every OpenTelemetry exporter disabled") - - sandbox = config.get("sandbox_workspace_write") - if not isinstance(sandbox, dict) or sandbox.get("network_access") is not False: - raise RenderError("codex composition must keep sandbox_workspace_write.network_access false") - if set(sandbox) - {"network_access", "writable_roots"}: - raise RenderError("codex composition contains unadmitted sandbox_workspace_write fields") - if sandbox.get("writable_roots"): - raise RenderError("codex composition may not add sandbox_workspace_write.writable_roots") def bundled_model_catalog() -> dict[str, dict[str, object]]: @@ -200,38 +115,14 @@ class CodexAdapterRenderer(AdapterRenderer): @staticmethod def process_config(path: str, config: dict[str, Any]) -> dict[str, Any]: if path == CONFIG: - validate_config(config) return config - if set(config) - {"models"}: - raise RenderError("codex models config accepts only models") - metadata_models = config.get("models", {}) - if not isinstance(metadata_models, dict): - raise RenderError("codex models must map model names to metadata") - return {"models": list(get_model_catalog(metadata_models).values())} + return {"models": list(get_model_catalog(config.get("models", {})).values())} @staticmethod def process_skill(path: str, text: str) -> str: # An agent_command renders to the same path template, so it is processed here too. return with_skill_frontmatter(path, text) - @staticmethod - def check_model_route( - configs: Mapping[str, Mapping[str, Any]], skills: Mapping[str, str], commands: Mapping[str, str] - ) -> None: - config = configs[CONFIG] - providers = config.get("model_providers") - if isinstance(providers, dict): - extra_providers = sorted(set(providers) - {"reef"}) - if extra_providers: - raise RenderError( - f"codex composition may only configure the Reef model provider: {', '.join(extra_providers)}" - ) - for name, provider in providers.items(): - if not isinstance(provider, dict) or set(provider) - ALLOWED_PROVIDER_KEYS: - raise RenderError(f"codex model provider {name!r} contains unadmitted fields") - if config.get("model_provider") not in (None, "reef"): - raise RenderError("codex composition must use the Reef model provider") - @staticmethod def finalize_render(files: dict[str, str]) -> dict[str, str]: if any(path.startswith(EXTENSIONS) for path in files): diff --git a/reef/harness/adapters/descriptor.py b/reef/harness/adapters/descriptor.py index 5801fd173..ea5e6700f 100644 --- a/reef/harness/adapters/descriptor.py +++ b/reef/harness/adapters/descriptor.py @@ -5,8 +5,8 @@ - ``config_targets``: named JSON config files with their enforced defaults (a ``primary`` target is required; ``config`` nodes merge into targets) and - their locked keys (paths the merged config must keep at its defaults value, - or must not set when the defaults do not name it). + their checks (a key of the merged config and the matcher every value there + must match; see :mod:`reef.harness.adapters.matchers`). - ``node_paths``: root-relative render paths for the other node kinds; named kinds carry a ``{name}`` placeholder. A kind the adapter leaves out is refused at admission and at render, never dropped. @@ -45,9 +45,9 @@ A descriptor may name a ``quirks`` module: its ``cleanup_whitelist`` extends the declared one and the :class:`AdapterRenderer` subclass it defines adds the adapter's own render steps (the seam that enforces what the declaration cannot -state, such as opencode's agent and command shapes; the keys a tree must keep -are declared under ``files.config..locked`` and checked by the shared -render). External adapters register through the +state, such as opencode's agent and command shapes; what a config must keep or +must not set is declared under ``files.config..checks`` and checked by +the shared render). External adapters register through the ``reef.harness_adapters`` entry point group, each entry resolving to an ``AdapterDescriptor`` or a zero-argument callable returning one. """ @@ -67,6 +67,7 @@ import yaml from reef.core.errors import ReefError +from reef.harness.adapters.matchers import KeyDefault, Matcher, parse_matcher, select from reef.harness.episodes.executor import EpisodeExecutor @@ -83,8 +84,10 @@ class AdapterRenderer: The shared render runs them in a fixed order: ``process_config`` for each config file, ``process_skill`` and ``process_command`` for each skill and command file, ``check_model_route``, then ``finalize_render`` after the render has written each config file as text in the format its suffix names. Before any of them, the render - refuses a tree that sets a key Reef's model binding writes without the binding's credential; - ``check_model_route`` refuses the other model routes the harness reads. + refuses a tree that sets a key Reef's model binding writes without the binding's credential, and a tree whose + config fails a check its target declares. A rule on config values belongs in those checks; a step is for + changing files and for what a check cannot state, such as a model in a skill's frontmatter or a value that + must name an agent the config defines. Although ``finalize_render`` is an omnipower API that gives the users the maximum freedom for customization, it is a last resort since that method has a large impact surface and make it hard to maintain. So instead users @@ -209,16 +212,37 @@ class DescriptorError(ReefError): """The adapter descriptor is missing, malformed, or self-inconsistent.""" +#: The ``key`` of a check on the whole config file. +WHOLE_FILE_KEY = "." + + +@dataclass(frozen=True) +class ConfigCheck: + """One condition on a merged config file: every value at ``key`` matches ``matcher``. + + ``key`` is dot separated key names, or ``.`` for the whole file; ``source`` is the match expression as the + descriptor writes it, and ``reason`` says why the check exists. A refusal names both. + """ + + key: str + matcher: Matcher + source: str + reason: str = "" + + @property + def segments(self) -> tuple[str, ...]: + return () if self.key == WHOLE_FILE_KEY else tuple(self.key.split(".")) + + @dataclass(frozen=True) class ConfigTarget: - """One JSON config file: its render path, the defaults merged first, and the keys the tree must keep.""" + """One JSON config file: its render path, the defaults merged first, and the checks the merged file must pass.""" path: str defaults: Mapping[str, Any] = field(default_factory=dict) - #: Dotted key paths the merged config must keep at their ``defaults`` value; a path the defaults do not set - #: must not appear in the merged config at all. A ``*`` segment matches every key of an object and every item - #: of a list. The shared render checks them before any adapter step runs, so no adapter can skip the check. - locked: tuple[str, ...] = () + #: The shared render runs them on the merged config, the binding included, before any adapter step runs, so no + #: adapter can skip them. + checks: tuple[ConfigCheck, ...] = () #: The ``{api_key}`` an installed binding carries when the client has no Reef token (a Reef without auth): a @@ -448,7 +472,6 @@ def load_descriptor(path: Path) -> AdapterDescriptor: client_state = _parse_client_state(data.get("client_state"), where) renderer, quirk_whitelist, validate_execution = _load_quirks(data.get("quirks"), where) model_binding = _parse_model_binding(data.get("model_binding"), config_targets, where) - _check_locked_binding_overlap(config_targets, model_binding, where) descriptor = AdapterDescriptor( name=name, binary=_require_str(data, "binary", where), @@ -500,64 +523,55 @@ def _parse_config_targets(raw: Any, where: str) -> dict[str, ConfigTarget]: defaults = target.get("defaults", {}) if not isinstance(defaults, Mapping): raise DescriptorError(f"{where} config target {target_name!r} defaults must be an object") + if "locked" in target: + # A lock the loader skipped would leave the key unchecked, so the old field fails loudly. + raise DescriptorError( + f"{where} config target {target_name!r} 'locked' is replaced by 'checks': write a locked key as a " + "check whose match is IsDefault() when the defaults set it, and IsAbsent() when they do not" + ) targets[str(target_name)] = ConfigTarget( path=_require_str(target, "path", f"{where} config target {target_name!r}"), defaults=dict(defaults), - locked=_locked_paths(target.get("locked", []), f"{where} config target {target_name!r} 'locked'"), + checks=_parse_checks(target.get("checks", []), defaults, f"{where} config target {target_name!r} checks"), ) return targets -def _locked_paths(value: Any, where: str) -> tuple[str, ...]: - """``locked`` key paths: dot separated key names, a ``*`` segment matching any object key or list item.""" - paths = _str_list(value, where) - for path in paths: - if any(segment == "" for segment in path.split(".")): - raise DescriptorError(f"{where} entries must be dot separated key names, '*' naming a whole segment") - return paths +def _parse_checks(raw: Any, defaults: Mapping[str, Any], where: str) -> tuple[ConfigCheck, ...]: + """A target's ``checks``: each with ``key`` (one key or a list), ``match`` and an optional ``reason``. - -def _binding_key_paths(data: Mapping[str, Any], segments: tuple[str, ...]) -> list[tuple[str, ...]]: - """Each key path a binding template writes, as segments; a template segment in braces matches any key.""" - paths: list[tuple[str, ...]] = [] - for key, value in data.items(): - segment = str(key) - if isinstance(value, Mapping) and value: - paths.extend(_binding_key_paths(value, (*segments, segment))) - else: - paths.append((*segments, segment)) - return paths - - -def _paths_overlap(left: tuple[str, ...], right: tuple[str, ...]) -> bool: - """Whether two key paths can name one key: every segment pair matches, so one path holds the other. - - A ``*`` in a locked path matches any binding segment, and a binding segment in braces (a ``{model}`` - placeholder) matches any locked segment. + A check with a list of keys is one check per key, in order. A check without a key is refused, so a forgotten key + never checks the whole file; ``.`` names the whole file. """ - return all(locked == "*" or "{" in bound or locked == bound for locked, bound in zip(left, right, strict=False)) - - -def _check_locked_binding_overlap( - config_targets: Mapping[str, ConfigTarget], - model_binding: Mapping[str, tuple[Mapping[str, Any], ...]], - where: str, -) -> None: - """A locked path must not overlap a key the model binding writes: the shared binding check owns those keys. - - A lock beside the binding's keys could only refuse the binding's own render, or, one rename later, silently - stop the binding from writing what the adapter needs. - """ - for api, templates in model_binding.items(): - for node in templates: - target_name = str(node["target"]) - for bound in _binding_key_paths(node["data"], ()): - for locked in config_targets[target_name].locked: - if _paths_overlap(tuple(locked.split(".")), bound): - raise DescriptorError( - f"{where} config target {target_name!r} locks {locked!r}, which the {api} model " - f"binding writes ({'.'.join(bound)})" - ) + if not isinstance(raw, list): + raise DescriptorError(f"{where} must be a list") + checks: list[ConfigCheck] = [] + for index, entry in enumerate(raw): + place = f"{where}[{index}]" + if not isinstance(entry, Mapping) or set(entry) - {"key", "match", "reason"}: + raise DescriptorError(f"{place} must be an object with key and match, and optionally reason") + source = entry.get("match") + reason = entry.get("reason", "") + if not isinstance(source, str) or not source.strip() or not isinstance(reason, str): + raise DescriptorError(f"{place} requires a match expression, and its reason must be a string") + if "key" not in entry: + raise DescriptorError( + f"{place} requires a key: dot separated key names, or {WHOLE_FILE_KEY!r} for the whole file" + ) + keys = entry["key"] + for key in _str_list(keys, f"{place} key") if isinstance(keys, list) else [keys]: + if not isinstance(key, str) or (key != WHOLE_FILE_KEY and "" in key.split(".")): + raise DescriptorError( + f"{place} key must be dot separated key names, or {WHOLE_FILE_KEY!r} for the whole file" + ) + segments = () if key == WHOLE_FILE_KEY else tuple(key.split(".")) + default = None if {"*", "**"} & set(segments) else KeyDefault(select(dict(defaults), segments)[0][1]) + try: + matcher = parse_matcher(source, default) + except ValueError as error: + raise DescriptorError(f"{place} match: {error}") from error + checks.append(ConfigCheck(key=key, matcher=matcher, source=" ".join(source.split()), reason=reason)) + return tuple(checks) def _parse_model_binding( diff --git a/reef/harness/adapters/dsh/descriptor.yaml b/reef/harness/adapters/dsh/descriptor.yaml index 60fff0dc4..589b532fd 100644 --- a/reef/harness/adapters/dsh/descriptor.yaml +++ b/reef/harness/adapters/dsh/descriptor.yaml @@ -68,15 +68,71 @@ files: disabled: true session-telemetry-otel: disabled: true - # Keys the merged config must keep at the defaults' value: the session log - # stays where the bundle put it and uncompressed (the reader cannot parse - # zstd), and the LLM title call (one extra model request per episode) and - # the telemetry stay off. The shared render refuses a composition that - # moves them, in either profile, before any quirks step runs. - locked: - - session-persistence-jsonl.config.compression - - session-title-llm.disabled - - session-telemetry-otel.disabled + # The shared render refuses a composition that fails one of these + # checks, in either profile (the web target repeats them by the + # anchor), before any quirks step runs. + checks: &patch_checks + # The session log stays uncompressed (the reader cannot parse zstd), + # and the LLM title call (one extra model request per episode) and + # the telemetry stay off. + - key: [session-persistence-jsonl.config.compression, session-title-llm.disabled, session-telemetry-otel.disabled] + match: IsDefault() + - key: "." + match: Each(Value(IsObject())) + reason: a patch entry is an object holding config, disabled, or inject + - key: "*.name" + match: IsAbsent() + reason: a patch entry keeps its own package + # Every model call stays on Reef's model binding. The binding writes + # the llm-pi-ai route reef, puts its key in .env, and selects the + # route in agent-default-model; these hold only the keys it writes. + - key: [agent-default-model, llm-pi-ai] + match: Each(Key(Eq("config"))) + reason: Reef's model binding writes it + - key: agent-default-model.config + match: Each(Key(IsIn(["model", "provider"]))) + reason: Reef's model binding writes it + - key: agent-default-model.config.provider + match: AnyOf(IsAbsent(), Eq("reef")) + reason: Reef's model binding writes it + - key: llm-pi-ai.config + match: Each(Key(Eq("providers"))) + reason: Reef's model binding writes it + - key: llm-pi-ai.config.providers + match: Each(Key(Eq("reef"))) + reason: Reef's model binding writes the only route, reef + - key: llm-pi-ai.config.providers.reef + match: Each(Key(IsIn(["api", "apiKeyEnv", "baseURL", "displayName", "models"]))) + reason: Reef's model binding writes it + - key: llm-pi-ai.config.providers.reef.apiKeyEnv + match: AnyOf(IsAbsent(), Eq("REEF_API_KEY")) + reason: Reef's model binding writes it + # DeepSeek's own adapter, whose config is its endpoint: Reef never + # binds it. The plugins after it choose a call's provider, endpoint, + # credential or model: the title call, a subagent, a declared agent, + # the compaction summary and the web search model. ** reaches the key + # under an object or under each object of a list. + - key: llm-deepseek + match: Each(Key(Eq("disabled"))) + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model + - key: [ + agent-loop.config.agents.**.model, agent-loop.config.agents.**.provider, + compaction-basic.config.modelPolicies.**.summarizationModel, + compaction-basic.config.modelPolicies.**.summarizationProvider, + session-title-llm.config.model, session-title-llm.config.provider, + tool-subagent.config.agentOptions.**.model, tool-subagent.config.agentOptions.**.provider, + tool-subagent-fork.config.agentOptions.**.model, tool-subagent-fork.config.agentOptions.**.provider, + web-search-deepseek.config.apiKeyEnv, web-search-deepseek.config.baseURL, web-search-deepseek.config.model, + ] + match: IsAbsent() + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model + # A js expression in those plugins is a value no check can read. + - key: [ + agent-loop.**, compaction-basic.**, session-title-llm.**, tool-subagent.**, tool-subagent-fork.**, + web-search-deepseek.**, + ] + match: Not(StartsWith("!!js ")) + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model # The lowest trust layer of dsh's launch environment; the model binding # puts the key of its apiKeyEnv route here. env: @@ -97,10 +153,7 @@ files: disabled: true session-telemetry-otel: disabled: true - locked: - - session-persistence-jsonl.config.compression - - session-title-llm.disabled - - session-telemetry-otel.disabled + checks: *patch_checks # The web profile's manifest. dsh writes patchReload live into a new web # profile, and with it the pinned `dsh web` exits at start ("user # patch-layer watching requires the Cordis HMR service"); startup reads @@ -116,8 +169,9 @@ files: profile: bundles: ["@deepseek-ai/dsh-base", "@deepseek-ai/dsh-web-app"] patchReload: startup - locked: - - dsh.profile.patchReload + checks: + - key: dsh.profile.patchReload + match: IsDefault() rules: "dsh/AGENTS.md" agent_command: "dsh-agents/skills/{name}/SKILL.md" skill: "dsh/skills/{name}/SKILL.md" diff --git a/reef/harness/adapters/dsh/quirks.py b/reef/harness/adapters/dsh/quirks.py index d5bd01987..a8ce9eaa9 100644 --- a/reef/harness/adapters/dsh/quirks.py +++ b/reef/harness/adapters/dsh/quirks.py @@ -30,33 +30,17 @@ reads each value back with its type: a string that YAML 1.1 or 1.2 would read as a number, a boolean or a null is quoted. -The traps a mutated patch could reopen, in either profile: the session log -must stay plain JSONL (the reader cannot parse zstd, and a compressed -profile refuses a sessions root that holds plain logs), and the session -telemetry and the LLM title call stay disabled; the web profile's manifest -keeps ``patchReload: startup``. These keys are declared locks in the -descriptor, and the shared render refuses a composition that moves them -before any quirks step runs; the chained reads the check here needed would -have crashed on a section that is not an object. This step keeps the shape -check: a patch entry must be an object. - -Every model call stays on Reef's model binding, in either profile. The -binding writes the ``llm-pi-ai`` route ``reef`` and selects it in -``agent-default-model``, and puts the route's key in ``.env``; it renders -after the tree and wins every key it writes. The key is a credential, which -a tree cannot hold because admission refuses an inline credential, so the -two entries pass only beside it, and only with the keys the binding writes. -Another route, DeepSeek's own adapter (``llm-deepseek``), the web search -model and its endpoint, and a provider or a model set for the title call, a -subagent, a declared agent or the compaction summary are the tree choosing -where calls go, and are refused, as is a patch entry that names another -package or holds a js expression in those plugins, where the check cannot -read the value.""" +The descriptor's checks hold both profiles' patch layers before any step +here runs: the keys an episode relies on, a patch entry that is an object, +and every model call on Reef's model binding. The binding writes the +``llm-pi-ai`` route ``reef``, selects it in ``agent-default-model`` and puts +the route's key in ``.env``; another route, DeepSeek's own adapter, a +provider or a model set for another call, a patch entry that names another +package, and a js expression in those plugins are refused there.""" from __future__ import annotations import re -from collections.abc import Mapping from typing import Any, ClassVar import yaml @@ -70,8 +54,6 @@ "dsh/profiles/headless/cordis.patch.yml": "./extensions/", "dsh/profiles/web/cordis.patch.yml": "../headless/extensions/", } -#: The web profile's manifest: with dsh's default for a new web profile, live patch reload, ``dsh web`` exits at start. -WEB_MANIFEST = "dsh/profiles/web/package.json" _ENV = "dsh/.env" _EXTENSIONS = "dsh/profiles/headless/extensions/" _JS = "!!js " @@ -95,24 +77,6 @@ ), ) -#: The key Reef's binding puts in .env for its route, the entries it writes, and the keys of each. -BINDING_KEY_ENV = "REEF_API_KEY" -BINDING_ROUTE = "reef" -BINDING_ROUTE_KEYS = frozenset({"api", "apiKeyEnv", "baseURL", "displayName", "models"}) -BINDING_PLUGINS = ("agent-default-model", "llm-pi-ai") -#: Plugins whose config chooses a model call's provider, endpoint, credential or model, each with where that choice -#: sits: a key of the config (no parent), or a key of the object, or of each object in the list, under the parent. -MODEL_ROUTE_KEYS: dict[str, tuple[tuple[str | None, str], ...]] = { - "agent-loop": (("agents", "model"), ("agents", "provider")), - "compaction-basic": (("modelPolicies", "summarizationModel"), ("modelPolicies", "summarizationProvider")), - "session-title-llm": ((None, "model"), (None, "provider")), - "tool-subagent": (("agentOptions", "model"), ("agentOptions", "provider")), - "tool-subagent-fork": (("agentOptions", "model"), ("agentOptions", "provider")), - "web-search-deepseek": ((None, "apiKeyEnv"), (None, "baseURL"), (None, "model")), -} -#: DeepSeek's own adapter, whose config is its endpoint: Reef never binds it. -UNBOUND_ADAPTERS = ("llm-deepseek",) - # dsh's boot scaffolds each profile beside the rendered patch: the empty root # entry list, node_modules symlinks into the installation, and the module # fallback links, plus a package manifest and the pnpm workspace file for a @@ -240,70 +204,7 @@ def _with_frontmatter(path: str, text: str, user_only: bool) -> str: return "---\n" + dumped + "---\n" + body -def holds_js(value: object) -> bool: - if isinstance(value, str): - return value.startswith(_JS) - if isinstance(value, dict): - return any(holds_js(item) for item in value.values()) - if isinstance(value, list): - return any(holds_js(item) for item in value) - return False - - -def check_binding_entry(plugin: str, entry: dict[str, Any]) -> None: - """``llm-pi-ai`` holds only the binding's route and ``agent-default-model`` selects it.""" - refusal = f"dsh composition must not set {plugin}: Reef's model binding writes it" - config = entry.get("config") - if set(entry) != {"config"} or not isinstance(config, dict): - raise RenderError(refusal) - if plugin == "agent-default-model": - if set(config) != {"model", "provider"} or config["provider"] != BINDING_ROUTE: - raise RenderError(refusal) - return - providers = config.get("providers") - if set(config) != {"providers"} or not isinstance(providers, dict) or set(providers) != {BINDING_ROUTE}: - raise RenderError(f"{refusal}; its only route is {BINDING_ROUTE!r}") - route = providers[BINDING_ROUTE] - if not isinstance(route, dict) or set(route) != BINDING_ROUTE_KEYS or route["apiKeyEnv"] != BINDING_KEY_ENV: - raise RenderError(refusal) - - -def check_model_route(entries: dict[str, Any]) -> None: - """Refuse a model route, an endpoint, a credential or a model the binding did not write.""" - refusal = "Reef's model binding chooses the provider, the endpoint, the credential and the model" - for plugin, entry in entries.items(): - if not isinstance(entry, dict): - continue - if "name" in entry: - raise RenderError(f"dsh composition must not set {plugin}.name: a patch entry keeps its own package") - if plugin in BINDING_PLUGINS: - check_binding_entry(plugin, entry) - elif plugin in UNBOUND_ADAPTERS and set(entry) - {"disabled"}: - raise RenderError(f"dsh composition must not configure {plugin}: {refusal}") - elif plugin in MODEL_ROUTE_KEYS: - if holds_js(entry): - raise RenderError(f"dsh composition must not write {plugin} as a js expression: {refusal}") - config = entry.get("config") - if not isinstance(config, dict): - continue - for parent, key in MODEL_ROUTE_KEYS[plugin]: - value = config if parent is None else config.get(parent) - for item in value if isinstance(value, list) else [value]: - if isinstance(item, dict) and key in item: - where = key if parent is None else f"{parent}.{key}" - raise RenderError(f"dsh composition must not set {plugin} {where}: {refusal}") - - class DshAdapterRenderer(AdapterRenderer): - @staticmethod - def process_config(path: str, config: dict[str, Any]) -> dict[str, Any]: - if path not in PROFILE_PATCHES and path != WEB_MANIFEST: - return config - for plugin, entry in config.items() if path in PROFILE_PATCHES else (): - if not isinstance(entry, dict): - raise RenderError(f"dsh patch entry {plugin!r} must be an object holding config, disabled, or inject") - return config - @staticmethod def process_skill(path: str, text: str) -> str: return _with_frontmatter(path, text, False) @@ -313,13 +214,6 @@ def process_command(path: str, text: str) -> str: # A command is a skill in the second root that only the person invokes. return _with_frontmatter(path, text, True) - @staticmethod - def check_model_route( - configs: Mapping[str, Mapping[str, Any]], skills: Mapping[str, str], commands: Mapping[str, str] - ) -> None: - for patch_path in PROFILE_PATCHES: - check_model_route(dict(configs[patch_path])) - @staticmethod def finalize_render(files: dict[str, str]) -> dict[str, str]: # A patch layer is a list of plugin entries, not plain YAML, and inserts the rendered extensions. diff --git a/reef/harness/adapters/hermes/descriptor.yaml b/reef/harness/adapters/hermes/descriptor.yaml index f4cdb8794..884c0b13e 100644 --- a/reef/harness/adapters/hermes/descriptor.yaml +++ b/reef/harness/adapters/hermes/descriptor.yaml @@ -61,25 +61,81 @@ files: write_json_snapshots: true skills: creation_nudge_interval: 0 - # Keys the merged config must keep at the defaults' value, and keys no tree may set at all: the shared - # render refuses a composition that moves them, before any quirks step runs. The first six keep an episode - # hermetic and single request with its snapshot readable; the rest name another provider, endpoint or - # fallback chain, which Reef's model binding chooses. - locked: - - security.tirith_enabled - - auxiliary.title_generation.enabled - - memory.nudge_interval - - skills.creation_nudge_interval - - curator.enabled - - sessions.write_json_snapshots - - api_base - - base_url - - model_aliases - - provider - - providers - - custom_providers - - fallback_model - - fallback_providers + # The shared render refuses a composition that fails one of these + # checks, before any quirks step runs. + checks: + # Keep an episode hermetic and single request, with its snapshot readable. + - key: [ + security.tirith_enabled, auxiliary.title_generation.enabled, memory.nudge_interval, + skills.creation_nudge_interval, curator.enabled, sessions.write_json_snapshots, + ] + match: IsDefault() + # hermes reads these sections as objects, and a model that is not + # one names the model. + - key: model + match: AnyOf(IsAbsent(), Eq(null), IsObject()) + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model + - key: [moa, compression, auxiliary, delegation, cron, curator, curator.auxiliary] + match: AnyOf(IsAbsent(), Eq(null), IsObject()) + reason: hermes reads it as an object + # Every model call stays on Reef's model binding. The binding writes + # model.provider, model.default, model.base_url and model.api_key, + # renders after the tree and wins every key it writes. The keys below name another provider, endpoint, credential, + # transport, model or fallback chain. These top-level keys hermes + # moves into model. + - key: [ + api_base, base_url, model_aliases, provider, providers, custom_providers, fallback_model, + fallback_providers, + ] + match: IsAbsent() + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model + # hermes reads an empty string, list or object as unset. model.model + # and model.name are the model, model.api_base the endpoint, the key + # names the key, and aliases the models a switch resolves; api_mode + # and openai_runtime leave the bound endpoint. moa names the models + # of its presets, and the older compression keys move into + # auxiliary.compression. An auxiliary task, delegation, cron and the + # curator's older auxiliary section each choose their own route. + - key: [ + model.aliases, model.api_base, model.api_key_env, model.key_cmd, model.key_env, model.model, model.name, + model.api_mode, model.openai_runtime, + moa.aggregator, moa.presets, moa.reference_models, + compression.summary_base_url, compression.summary_model, + auxiliary.openrouter_model, + auxiliary.*.api_key, auxiliary.*.api_key_env, auxiliary.*.api_mode, auxiliary.*.base_url, + auxiliary.*.fallback_chain, auxiliary.*.key_cmd, auxiliary.*.key_env, auxiliary.*.model, + auxiliary.*.model_provider, + delegation.api_key, delegation.api_key_env, delegation.api_mode, delegation.base_url, + delegation.fallback_chain, delegation.key_cmd, delegation.key_env, delegation.model, + delegation.model_provider, delegation.provider, + cron.api_key, cron.api_key_env, cron.api_mode, cron.base_url, cron.fallback_chain, cron.key_cmd, + cron.key_env, cron.model, cron.model_provider, cron.provider, + curator.auxiliary.api_key, curator.auxiliary.api_key_env, curator.auxiliary.api_mode, + curator.auxiliary.base_url, curator.auxiliary.fallback_chain, curator.auxiliary.key_cmd, + curator.auxiliary.key_env, curator.auxiliary.model, curator.auxiliary.model_provider, + curator.auxiliary.provider, + ] + match: AnyOf(IsAbsent(), IsIn([null, "", [], {}])) + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model + # auto and main run an auxiliary task on the main model. + - key: [compression.summary_provider, auxiliary.*.provider] + match: AnyOf(IsAbsent(), IsIn([null, "", [], {}, "auto", "main"])) + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model + - key: [ + auxiliary.*.prefer_fast_model, delegation.prefer_fast_model, cron.prefer_fast_model, + curator.auxiliary.prefer_fast_model, + ] + match: Not(Eq(true)) + reason: Reef's model binding chooses the model + # A request body names no model: model replaces the bound one, and + # OpenRouter reads models as fallback models. Delegation sends + # request_overrides with each child call as call arguments. + - key: [ + auxiliary.*.extra_body, delegation.extra_body, cron.extra_body, curator.auxiliary.extra_body, + delegation.request_overrides, delegation.request_overrides.extra_body, + ] + match: Not(Contains(Key(IsIn(["model", "models"])))) + reason: Reef's model binding chooses the model # The env file hermes loads from its home; the model binding puts the # provider key here too, since for an OpenRouter host hermes reads the # key from the environment and not from config.yaml. diff --git a/reef/harness/adapters/hermes/quirks.py b/reef/harness/adapters/hermes/quirks.py index 1c3ff5585..61205e6e8 100644 --- a/reef/harness/adapters/hermes/quirks.py +++ b/reef/harness/adapters/hermes/quirks.py @@ -15,37 +15,20 @@ ``process_command`` give a SKILL.md the node text left bare the ``name`` and ``description`` frontmatter hermes requires, under both skill roots. -The traps a mutated config could reopen: the scanner download, the session -title call, the background review and the curator that write skills into -the tree, and the snapshot the reader parses, are declared in the -descriptor and refused by the shared render before any quirks step runs. -This step checks what the declaration cannot state: a section holding a -value that is not an object where one belongs, and a value that is not a -list of strings where Reef adds a name, are rejected at render, the same -check that rejects an invalid node. - -Every model call stays on Reef's model binding. The binding writes the -``model`` section (a ``custom`` provider, the endpoint, the model and a -literal key), renders after the tree and wins every key it writes. Its key -is a credential, which a tree cannot hold because admission refuses an -inline credential, so those keys pass only beside the binding's key. The -other names hermes reads for the model, the endpoint or the key (in -``model``, and at the top level, which hermes moves into ``model``), the -sections that name other providers, their endpoints and credential commands -(``providers``, ``custom_providers``) and the fallback models -(``fallback_model``, ``fallback_providers``) are declared locks beside the -descriptor's defaults. The model aliases, the mixture of agents presets, -and a provider, an endpoint, a credential, a model or a fallback chain set -for an auxiliary task, for delegation, for cron or for the curator, are the -tree choosing where calls go, and are refused here. An -auxiliary task may keep ``auto`` or ``main``, which run it on the main -model. The binding sets no transport, so a tree's ``api_mode`` (in -``model`` or in any of those sections) and ``model.openai_runtime`` are -refused too: for the ``custom`` provider two api modes leave the bound -endpoint, ``bedrock_converse`` for AWS Bedrock and ``codex_app_server`` for -a ``codex app-server`` subprocess. A request body a tree passes to the -bound endpoint (``extra_body``, ``delegation.request_overrides``) may not -name a model either. +The descriptor's checks hold the config before any step here runs: the +scanner download, the session title call, the background review and the +curator that write skills into the tree, and the snapshot the reader +parses stay as the defaults set them, and every model call stays on Reef's +model binding. The binding writes the ``model`` section (a ``custom`` +provider, the endpoint, the model and a literal key), and the checks refuse +the other names hermes reads for a provider, an endpoint, a credential, a +transport, a model or a fallback chain, in ``model``, at the top level, in +the mixture of agents presets, and for an auxiliary task, delegation, cron +or the curator; an auxiliary task may keep ``auto`` or ``main``, which run +it on the main model. A request body a tree passes to the bound endpoint +(``extra_body``, ``delegation.request_overrides``) may not name a model +either. ``process_config`` refuses a ``skills.external_dirs`` that is not a +list of strings, where it adds the commands root. These checks cover the config the tree renders. Reef's proxy forwards the ``model`` and ``models`` a request sends as they are, so a request that other @@ -55,7 +38,6 @@ from __future__ import annotations -from collections.abc import Mapping from typing import Any import yaml @@ -90,34 +72,6 @@ "default." ) -#: Other ``model`` keys hermes reads for the model, the endpoint or the key, and the model aliases a switch resolves. -MODEL_ALIAS_KEYS = ("aliases", "api_base", "api_key_env", "key_cmd", "key_env", "model", "name") -#: The ``model`` keys hermes reads for the transport; the binding writes neither. -TRANSPORT_KEYS = ("api_mode", "openai_runtime") -#: The mixture of agents keys that name its models: the presets, and the older flat form of one preset. -MOA_KEYS = ("aggregator", "presets", "reference_models") -#: The keys of an auxiliary task, of delegation, of cron and of the curator that choose its provider, endpoint, -#: credential, transport or model, or a fallback chain of those. -ROUTE_KEYS = ( - "api_key", - "api_key_env", - "api_mode", - "base_url", - "fallback_chain", - "key_cmd", - "key_env", - "model", - "model_provider", - "provider", -) -#: Request body fields that name the model: ``model`` replaces the bound one, and OpenRouter reads ``models`` as -#: fallback models. -BODY_MODEL_KEYS = ("model", "models") -#: The providers an auxiliary task may keep: hermes runs both on the main model. -MAIN_MODEL_PROVIDERS = ("auto", "main") -#: The older compression keys hermes moves into ``auxiliary.compression`` (provider, model, base_url). -COMPRESSION_KEYS = ("summary_base_url", "summary_model", "summary_provider") - # hermes's boot scaffolds the home on every start: state directories, the # runtime and cache files, lock files beside the state store, and the seed # skill it always writes. Episode state, not residue. @@ -177,87 +131,12 @@ def _granted(config: dict[str, Any], plugins: list[str]) -> dict[str, Any]: return {**config, "plugins": section} -def is_set(value: object) -> bool: - """Whether hermes reads ``value`` as set: an empty string, list or object is its default.""" - return value is not None and value != "" and value != [] and value != {} - - -def section_of(config: dict[str, Any], name: str, where: str = "") -> dict[str, Any]: - """The config section ``name`` (``where`` names it in a refusal), empty when absent; hermes reads it as an object.""" - section = config.get(name) - if section is None: - return {} - if not isinstance(section, dict): - raise RenderError(f"hermes {where or name} must be an object, got {type(section).__name__}") - return section - - -def check_model_route(config: dict[str, Any]) -> None: - """Refuse a provider, an endpoint, a credential or a model the binding did not write.""" - refusal = "Reef's model binding chooses the provider, the endpoint, the credential and the model" - model = config.get("model") - if model is not None and not isinstance(model, dict): - raise RenderError(f"hermes composition must not set model: {refusal}") - model = model or {} - # hermes reads model.model and model.name as the model (one reader prefers model.model to model.default), - # model.api_base as the endpoint and the key names as the key; the binding writes none of them. - for key in MODEL_ALIAS_KEYS: - if is_set(model.get(key)): - raise RenderError(f"hermes composition must not set model.{key}: {refusal}") - # The binding binds the chat completions API. For its custom provider hermes takes model.api_mode in any - # spelling it knows, and bedrock_converse (an AWS Bedrock client) and codex_app_server (a codex subprocess) - # leave the bound endpoint; openai_runtime picks the codex subprocess for the openai providers. - for key in TRANSPORT_KEYS: - if is_set(model.get(key)): - raise RenderError(f"hermes composition must not set model.{key}: {refusal}") - moa = section_of(config, "moa") - for key in MOA_KEYS: - if is_set(moa.get(key)): - raise RenderError(f"hermes composition must not set moa.{key}: {refusal}") - compression = section_of(config, "compression") - for key in COMPRESSION_KEYS: - value = compression.get(key) - if is_set(value) and not (key == "summary_provider" and value in MAIN_MODEL_PROVIDERS): - raise RenderError(f"hermes composition must not set compression.{key}: {refusal}") - auxiliary = section_of(config, "auxiliary") - if is_set(auxiliary.get("openrouter_model")): - raise RenderError(f"hermes composition must not set auxiliary.openrouter_model: {refusal}") - routed = [(f"auxiliary.{task}", settings) for task, settings in auxiliary.items() if isinstance(settings, dict)] - routed += [(name, section_of(config, name)) for name in ("delegation", "cron")] - # The curator still reads its older per task section, curator.auxiliary. - routed.append(("curator.auxiliary", section_of(section_of(config, "curator"), "auxiliary", "curator.auxiliary"))) - for where, settings in routed: - for key in ROUTE_KEYS: - value = settings.get(key) - on_main_model = where.startswith("auxiliary.") and key == "provider" and value in MAIN_MODEL_PROVIDERS - if is_set(value) and not on_main_model: - raise RenderError(f"hermes composition must not set {where}.{key}: {refusal}") - if settings.get("prefer_fast_model") is True: - raise RenderError(f"hermes composition must not set {where}.prefer_fast_model: {refusal}") - check_body(settings.get("extra_body"), f"{where}.extra_body", refusal) - # Delegation sends request_overrides with each child call: its keys are call arguments, and its extra_body - # joins the request body. - overrides = section_of(config, "delegation").get("request_overrides") - check_body(overrides, "delegation.request_overrides", refusal) - if isinstance(overrides, dict): - check_body(overrides.get("extra_body"), "delegation.request_overrides.extra_body", refusal) - - -def check_body(body: object, where: str, refusal: str) -> None: - """Refuse a request body that names a model; hermes skips a body that is not an object.""" - if not isinstance(body, dict): - return - for key in BODY_MODEL_KEYS: - if key in body: - raise RenderError(f"hermes composition must not set {where}.{key}: {refusal}") - - class HermesAdapterRenderer(AdapterRenderer): @staticmethod def process_config(path: str, config: dict[str, Any]) -> dict[str, Any]: if path != _CONFIG: return config - # skills is read as an object here; the locked keys above checked what a composition must keep. + # skills is an object here: the descriptor's check on skills.creation_nudge_interval keeps it one. tree_dirs = nested_setting(config, "skills", "external_dirs") external_dirs = string_list([tree_dirs] if isinstance(tree_dirs, str) else tree_dirs, "skills.external_dirs") config["skills"]["external_dirs"] = external_dirs + [ @@ -274,12 +153,6 @@ def process_command(path: str, text: str) -> str: # A command is a skill under the commands root, so it needs the same frontmatter. return with_skill_frontmatter(path, text) - @staticmethod - def check_model_route( - configs: Mapping[str, Mapping[str, Any]], skills: Mapping[str, str], commands: Mapping[str, str] - ) -> None: - check_model_route(dict(configs[_CONFIG])) - @staticmethod def finalize_render(files: dict[str, str]) -> dict[str, str]: soul = files.get(SOUL_PATH) diff --git a/reef/harness/adapters/matchers.py b/reef/harness/adapters/matchers.py new file mode 100644 index 000000000..31e48be63 --- /dev/null +++ b/reef/harness/adapters/matchers.py @@ -0,0 +1,443 @@ +"""Matchers for the config checks a descriptor declares, after GoogleTest's matchers. + +A config target in ``descriptor.yaml`` lists ``checks``. Each check names a +``key`` of the merged config file and a ``match`` expression that every value +at that key must match, such as ``Not(Lowercase(IsIn(["0", "false"])))``. The +render refuses a tree whose config fails a check, before any adapter step +runs. Reef owns the matcher vocabulary; a descriptor cannot add a matcher. + +A key is dot separated key names, or ``.`` for the whole file. A ``*`` segment is every key of an object +and every item of a list, and a ``**`` segment is any number of segments, +zero included. A key the config does not set has the value ``ABSENT``, which +only ``IsAbsent()``, ``Not(...)`` and ``Each(...)`` match. A ``*`` or ``**`` +segment visits only values the config has, so a check below one passes when +there is nothing to visit. + +Text matchers (``StartsWith``, ``EndsWith``, ``HasSubstr``, ``MatchesRegex`` +and the ``Lowercase``, ``Uppercase`` and ``Strip`` transforms) read a scalar +as text: a string as it is, and a number, a boolean or null as its JSON text. +They match no object, list or ``ABSENT``. ``Eq`` and ``IsIn`` compare values +with their types, so ``false`` matches neither ``0`` nor ``"false"``. + +A match expression is parsed with Python's expression grammar, restricted to +matcher calls and JSON literals (``true``, ``false`` and ``null`` included), so +the descriptor needs no grammar of its own and the expression is never run. +""" + +from __future__ import annotations + +import ast +import json +import re +from abc import ABC, abstractmethod +from dataclasses import dataclass + + +class Absent: + """The value of a key the config does not set.""" + + def __repr__(self) -> str: + return "ABSENT" + + +ABSENT = Absent() + + +@dataclass(frozen=True) +class Entry: + """One key of an object and its value, as ``Each`` and ``Contains`` visit an object.""" + + key: object + value: object + + +class Matcher(ABC): + """One condition on a config value.""" + + @abstractmethod + def matches(self, value: object) -> bool: ... + + +def text_of(value: object) -> str | None: + """A scalar as text matchers read it; None for an object, a list or ``ABSENT``.""" + if isinstance(value, str): + return value + if isinstance(value, bool | int | float) or value is None: + return json.dumps(value) + return None + + +def json_equal(left: object, right: object) -> bool: + """Whether two config values are equal with their types: a boolean equals only a boolean.""" + if isinstance(left, bool) or isinstance(right, bool): + return type(left) is type(right) and left == right + if isinstance(left, dict) and isinstance(right, dict): + return left.keys() == right.keys() and all(json_equal(left[key], right[key]) for key in left) + if isinstance(left, list) and isinstance(right, list): + return len(left) == len(right) and all(json_equal(a, b) for a, b in zip(left, right, strict=True)) + return left == right + + +def elements_of(value: object) -> list[object] | None: + """The entries of an object or the items of a list; None for any other value.""" + if isinstance(value, dict): + return [Entry(key, item) for key, item in value.items()] + if isinstance(value, list): + return list(value) + return None + + +@dataclass(frozen=True) +class Combining(Matcher): + """A matcher over the results of several others.""" + + matchers: tuple[Matcher, ...] + + +@dataclass(frozen=True) +class AllOf(Combining): + def matches(self, value: object) -> bool: + return all(matcher.matches(value) for matcher in self.matchers) + + +@dataclass(frozen=True) +class AnyOf(Combining): + def matches(self, value: object) -> bool: + return any(matcher.matches(value) for matcher in self.matchers) + + +@dataclass(frozen=True) +class Wrapping(Matcher): + """A matcher that applies another to the value, or to a part of it.""" + + matcher: Matcher + + +@dataclass(frozen=True) +class Not(Wrapping): + def matches(self, value: object) -> bool: + return not self.matcher.matches(value) + + +@dataclass(frozen=True) +class Each(Wrapping): + """Every entry of an object, or every item of a list, matches; an absent key has none to fail.""" + + def matches(self, value: object) -> bool: + if value is ABSENT: + return True + elements = elements_of(value) + return elements is not None and all(self.matcher.matches(element) for element in elements) + + def first_failure(self, value: object) -> str | None: + """The key or the item that fails, to name in a refusal; None when the value has neither.""" + if isinstance(value, dict): + return next( + (f"key {key!r}" for key, item in value.items() if not self.matcher.matches(Entry(key, item))), None + ) + if isinstance(value, list): + return next((f"item {index}" for index, item in enumerate(value) if not self.matcher.matches(item)), None) + return None + + +@dataclass(frozen=True) +class Contains(Wrapping): + """At least one entry of an object, or one item of a list, matches.""" + + def matches(self, value: object) -> bool: + elements = elements_of(value) + return elements is not None and any(self.matcher.matches(element) for element in elements) + + +@dataclass(frozen=True) +class Key(Wrapping): + """The key of an object entry matches.""" + + def matches(self, value: object) -> bool: + return isinstance(value, Entry) and self.matcher.matches(value.key) + + +@dataclass(frozen=True) +class Value(Wrapping): + """The value of an object entry matches.""" + + def matches(self, value: object) -> bool: + return isinstance(value, Entry) and self.matcher.matches(value.value) + + +@dataclass(frozen=True) +class ParseJson(Wrapping): + """A string holding JSON whose value matches; a string that is not JSON matches nothing.""" + + def matches(self, value: object) -> bool: + if not isinstance(value, str): + return False + try: + parsed = json.loads(value) + except (ValueError, RecursionError): + return False + return self.matcher.matches(parsed) + + +@dataclass(frozen=True) +class TextTransform(Wrapping): + """A matcher applied to the value's text after a change, such as lowercase.""" + + def matches(self, value: object) -> bool: + text = text_of(value) + return text is not None and self.matcher.matches(self.transform(text)) + + @abstractmethod + def transform(self, text: str) -> str: ... + + +@dataclass(frozen=True) +class Lowercase(TextTransform): + def transform(self, text: str) -> str: + return text.lower() + + +@dataclass(frozen=True) +class Uppercase(TextTransform): + def transform(self, text: str) -> str: + return text.upper() + + +@dataclass(frozen=True) +class Strip(TextTransform): + """The text without the whitespace at its start and end.""" + + def transform(self, text: str) -> str: + return text.strip() + + +@dataclass(frozen=True) +class TextMatcher(Matcher): + """A condition on the value's text and one string argument.""" + + argument: str + + def matches(self, value: object) -> bool: + text = text_of(value) + return text is not None and self.matches_text(text) + + @abstractmethod + def matches_text(self, text: str) -> bool: ... + + +@dataclass(frozen=True) +class StartsWith(TextMatcher): + def matches_text(self, text: str) -> bool: + return text.startswith(self.argument) + + +@dataclass(frozen=True) +class EndsWith(TextMatcher): + def matches_text(self, text: str) -> bool: + return text.endswith(self.argument) + + +@dataclass(frozen=True) +class HasSubstr(TextMatcher): + def matches_text(self, text: str) -> bool: + return self.argument in text + + +@dataclass(frozen=True) +class MatchesRegex(TextMatcher): + """Text the whole of which the regular expression matches.""" + + def matches_text(self, text: str) -> bool: + return re.fullmatch(self.argument, text) is not None + + +@dataclass(frozen=True) +class IsAbsent(Matcher): + def matches(self, value: object) -> bool: + return value is ABSENT + + +@dataclass(frozen=True) +class IsObject(Matcher): + def matches(self, value: object) -> bool: + return isinstance(value, dict) + + +@dataclass(frozen=True) +class IsInt(Matcher): + """An integer, not a boolean.""" + + def matches(self, value: object) -> bool: + return isinstance(value, int) and not isinstance(value, bool) + + +@dataclass(frozen=True) +class Ge(Matcher): + """A number, not a boolean, at least ``bound``.""" + + bound: int | float + + def matches(self, value: object) -> bool: + return isinstance(value, int | float) and not isinstance(value, bool) and value >= self.bound + + +@dataclass(frozen=True) +class Eq(Matcher): + """A value equal to ``expected`` with its type; ``IsDefault()`` is an ``Eq`` of the defaults' value.""" + + expected: object + + def matches(self, value: object) -> bool: + return json_equal(value, self.expected) + + +@dataclass(frozen=True) +class IsIn(Matcher): + """A value equal to one of ``choices`` with its type.""" + + choices: tuple[object, ...] + + def matches(self, value: object) -> bool: + return any(json_equal(value, choice) for choice in self.choices) + + +COMBINING: dict[str, type[Combining]] = {"AllOf": AllOf, "AnyOf": AnyOf} +WRAPPING: dict[str, type[Wrapping]] = { + "Not": Not, + "Each": Each, + "Contains": Contains, + "Key": Key, + "Value": Value, + "ParseJson": ParseJson, + "Lowercase": Lowercase, + "Uppercase": Uppercase, + "Strip": Strip, +} +TEXT: dict[str, type[TextMatcher]] = { + "StartsWith": StartsWith, + "EndsWith": EndsWith, + "HasSubstr": HasSubstr, + "MatchesRegex": MatchesRegex, +} +NO_ARGUMENT: dict[str, type[IsAbsent | IsObject | IsInt]] = { + "IsAbsent": IsAbsent, + "IsObject": IsObject, + "IsInt": IsInt, +} +#: The matchers that take literals, and ``IsDefault``, which the parser resolves to an ``Eq``. +LITERAL_MATCHERS = ("Eq", "IsIn", "Ge", "IsDefault") +JSON_NAMES = {"true": True, "false": False, "null": None} + + +@dataclass(frozen=True) +class KeyDefault: + """The value the target's defaults give a check's key, ``ABSENT`` when they do not set it.""" + + value: object + + +def parse_literal(node: ast.expr) -> object: + """A JSON value written in a match expression.""" + if isinstance(node, ast.Name) and node.id in JSON_NAMES: + return JSON_NAMES[node.id] + if isinstance(node, ast.Constant) and isinstance(node.value, bool): + raise ValueError(f"write {str(node.value).lower()} for a boolean, not {node.value}") + if isinstance(node, ast.Constant) and isinstance(node.value, str | int | float): + return node.value + if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.USub): + number = parse_literal(node.operand) + if isinstance(number, int | float) and not isinstance(number, bool): + return -number + if isinstance(node, ast.List): + return [parse_literal(item) for item in node.elts] + if isinstance(node, ast.Dict): + entries: dict[str, object] = {} + for key, item in zip(node.keys, node.values, strict=True): + # A None key is a ** unpacking. + if not isinstance(key, ast.Constant) or not isinstance(key.value, str): + raise ValueError(f"{ast.unparse(node)} is not a JSON literal: its keys must be strings") + entries[key.value] = parse_literal(item) + return entries + raise ValueError(f"{ast.unparse(node)} is not a JSON literal") + + +def parse_matcher(source: str, default: KeyDefault | None) -> Matcher: + """The matcher a ``match`` expression names. + + ``default`` is what ``IsDefault()`` matches; None for a key with ``*`` or ``**``, where it names no one value. + Raises ValueError for an expression that is not one matcher call. + """ + try: + expression = ast.parse(source.strip(), mode="eval").body + except SyntaxError as error: + raise ValueError( + f"cannot parse {source!r}: {error.msg} at line {error.lineno}, column {error.offset}" + ) from error + + def build(node: ast.expr) -> Matcher: + if not isinstance(node, ast.Call) or not isinstance(node.func, ast.Name) or node.keywords: + raise ValueError(f"{ast.unparse(node)} is not a matcher call such as Eq(1)") + name, args = node.func.id, node.args + known = [*COMBINING, *WRAPPING, *TEXT, *NO_ARGUMENT, *LITERAL_MATCHERS] + if name not in known: + raise ValueError(f"unknown matcher {name}; the matchers are {', '.join(known)}") + if name in COMBINING: + if not args: + raise ValueError(f"{name} takes one or more matchers") + return COMBINING[name](tuple(build(arg) for arg in args)) + if name in NO_ARGUMENT or name == "IsDefault": + if args: + raise ValueError(f"{name} takes no arguments: {ast.unparse(node)}") + elif len(args) != 1: + raise ValueError(f"{name} takes one argument: {ast.unparse(node)}") + if name in WRAPPING: + return WRAPPING[name](build(args[0])) + if name in NO_ARGUMENT: + return NO_ARGUMENT[name]() + if name == "IsDefault": + if default is None: + raise ValueError("IsDefault() needs a key without * or **, which names one value of the defaults") + return Eq(default.value) + value = parse_literal(args[0]) + if name in TEXT: + if not isinstance(value, str): + raise ValueError(f"{name} takes a string: {ast.unparse(node)}") + if name == "MatchesRegex": + try: + re.compile(value) + except re.error as error: + raise ValueError(f"MatchesRegex pattern {value!r} is not a regular expression: {error}") from error + return TEXT[name](value) + if name == "Ge": + if not isinstance(value, int | float) or isinstance(value, bool): + raise ValueError(f"Ge takes a number: {ast.unparse(node)}") + return Ge(value) + if name == "IsIn": + if not isinstance(value, list): + raise ValueError(f"IsIn takes a list: {ast.unparse(node)}") + return IsIn(tuple(value)) + return Eq(value) + + return build(expression) + + +def select( + data: object, segments: tuple[str, ...], path: tuple[str, ...] = () +) -> list[tuple[tuple[str, ...], object]]: + """Each concrete key path that ``segments`` names in ``data``, with its value there or ``ABSENT``.""" + if not segments: + return [(path, data)] + first, rest = segments[0], segments[1:] + children: list[tuple[str, object]] = [] + if isinstance(data, dict): + children = [(str(key), item) for key, item in data.items()] + elif isinstance(data, list): + children = [(str(index), item) for index, item in enumerate(data)] + if first == "**": + return select(data, rest, path) + [ + found for key, item in children for found in select(item, segments, (*path, key)) + ] + if first == "*": + return [found for key, item in children for found in select(item, rest, (*path, key))] + child = data.get(first, ABSENT) if isinstance(data, dict) else ABSENT + return select(child, rest, (*path, first)) diff --git a/reef/harness/adapters/opencode/descriptor.yaml b/reef/harness/adapters/opencode/descriptor.yaml index 9e7e2d248..a337f808a 100644 --- a/reef/harness/adapters/opencode/descriptor.yaml +++ b/reef/harness/adapters/opencode/descriptor.yaml @@ -45,19 +45,16 @@ files: enabled_providers: ["reef"] permission: "*": "allow" - # Keys the merged config must keep at the defaults' value, and keys no tree may set at all: the shared - # render refuses a composition that moves them, before any quirks step runs. autoupdate, share and - # enabled_providers keep a benchmark episode hermetic and on the binding's provider only; the rest choose - # a model beyond the binding, which Reef's model binding writes. - locked: - - autoupdate - - share - - enabled_providers - - small_model - - disabled_providers - - agent.*.model - - mode.*.model - - command.*.model + # The shared render refuses a composition that fails one of these + # checks, before any quirks step runs. + checks: + # Keep a benchmark episode hermetic and on the binding's provider only. + - key: [autoupdate, share, enabled_providers] + match: IsDefault() + # Each of these chooses a model beyond the binding. + - key: [small_model, disabled_providers, agent.*.model, mode.*.model, command.*.model] + match: IsAbsent() + reason: Reef's model binding chooses the model rules: "opencode/AGENTS.md" agent_command: "opencode/command/{name}.md" skill: "opencode/skill/{name}/SKILL.md" diff --git a/reef/harness/adapters/pi/descriptor.yaml b/reef/harness/adapters/pi/descriptor.yaml index 1ae365c4a..9eaa25c16 100644 --- a/reef/harness/adapters/pi/descriptor.yaml +++ b/reef/harness/adapters/pi/descriptor.yaml @@ -61,14 +61,28 @@ files: primary: path: "pi-agent/settings.json" defaults: {} - # Keys no tree may set at all: they choose the models a run may use or - # send every call through another host, which Reef's model binding owns. - locked: - - enabledModels - - httpProxy + # The shared render refuses a composition that fails one of these + # checks, before any quirks step runs. + checks: + # They choose the models a run may use or send every call through + # another host. + - key: [enabledModels, httpProxy] + match: IsAbsent() + reason: Reef's model binding chooses the endpoint and the model models: path: "pi-agent/models.json" defaults: {} + # The binding writes the provider reef and selects it with + # defaultProvider and defaultModel in settings.json. Another provider, + # with its own baseUrl or pi's built in one with a new endpoint, is the + # tree choosing where calls go. + checks: + - key: providers + match: Each(Key(Eq("reef"))) + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model + - key: providers.reef + match: Each(Key(IsIn(["api", "apiKey", "baseUrl", "models"]))) + reason: Reef's model binding chooses the provider, the endpoint, the credential and the model rules: "pi-agent/AGENTS.md" agent_command: "pi-agent/prompts/{name}.md" skill: "pi-agent/skills/{name}/SKILL.md" diff --git a/reef/harness/adapters/pi/quirks.py b/reef/harness/adapters/pi/quirks.py index a1735fe63..9a8a4c219 100644 --- a/reef/harness/adapters/pi/quirks.py +++ b/reef/harness/adapters/pi/quirks.py @@ -11,17 +11,12 @@ and never offered. ``process_skill`` gives a skill node whose text carries none both, the description being the text's first line. -Every model call stays on Reef's model binding. The binding writes the -provider ``reef`` in ``models.json`` and selects it with ``defaultProvider`` -and ``defaultModel`` in ``settings.json``; it renders after the tree and wins -every key it writes. The provider's ``apiKey`` is a credential, which a tree -cannot hold because admission refuses an inline credential, so those keys -pass only beside it, and ``reef`` only with the keys the binding writes. -Another provider (its own ``baseUrl``, or pi's built in one with a new -endpoint) is the tree choosing where calls go, and is refused; -``enabledModels`` and ``httpProxy``, which send every call through another -host or another set of models, are declared locks in the descriptor and -refused by the shared render. +The descriptor's checks keep every model call on Reef's model binding: the +binding writes the provider ``reef`` in ``models.json`` and selects it in +``settings.json``, and the checks refuse another provider, a ``reef`` +provider with keys the binding does not write, and ``enabledModels`` and +``httpProxy``, which send every call through another set of models or +another host. pi 0.84.2 rewrites a few old settings keys when it loads ``settings.json`` and writes the new form back on its next save: ``queueMode`` becomes @@ -34,12 +29,10 @@ from __future__ import annotations -from collections.abc import Mapping from typing import Any from reef.harness.adapters.common.frontmatter import with_skill_frontmatter from reef.harness.adapters.descriptor import AdapterRenderer -from reef.harness.tree.render import RenderError cleanup_whitelist = ( "pi-agent/trust.json", @@ -47,22 +40,6 @@ ) SETTINGS_PATH = "pi-agent/settings.json" -MODELS_PATH = "pi-agent/models.json" - -#: The provider Reef's binding writes and its keys. -BINDING_PROVIDER = "reef" -BINDING_PROVIDER_KEYS = frozenset({"api", "apiKey", "baseUrl", "models"}) - - -def check_model_route(models: dict[str, Any]) -> None: - """Refuse a provider, an endpoint, a credential or a model the binding did not write.""" - refusal = "Reef's model binding chooses the provider, the endpoint, the credential and the model" - providers = models.get("providers") - if providers is not None and (not isinstance(providers, dict) or set(providers) - {BINDING_PROVIDER}): - raise RenderError(f"pi composition must not configure a provider other than {BINDING_PROVIDER!r}: {refusal}") - provider = (providers or {}).get(BINDING_PROVIDER) - if provider is not None and (not isinstance(provider, dict) or set(provider) != BINDING_PROVIDER_KEYS): - raise RenderError(f"pi composition must not configure provider {BINDING_PROVIDER!r}: {refusal}") def migrated_settings(settings: dict[str, object]) -> dict[str, object]: @@ -93,9 +70,3 @@ def process_config(path: str, config: dict[str, Any]) -> dict[str, Any]: def process_skill(path: str, text: str) -> str: # Give every skill the frontmatter pi requires when its text has none. return with_skill_frontmatter(path, text) - - @staticmethod - def check_model_route( - configs: Mapping[str, Mapping[str, Any]], skills: Mapping[str, str], commands: Mapping[str, str] - ) -> None: - check_model_route(dict(configs[MODELS_PATH])) diff --git a/reef/harness/adapters/terminus/descriptor.yaml b/reef/harness/adapters/terminus/descriptor.yaml index b26cdcefe..b4360f4ed 100644 --- a/reef/harness/adapters/terminus/descriptor.yaml +++ b/reef/harness/adapters/terminus/descriptor.yaml @@ -50,6 +50,61 @@ files: primary: path: "terminus/config.json" defaults: {} + # The shared render refuses a composition that fails one of these + # checks, before any quirks step runs. + checks: + # Terminus 2 constructor arguments a tree may set, verified against + # harbor 0.20.0, which reef-eval 0.1.1 resolves; a bump should check + # the signature again. The last three are the ones Reef's model + # binding writes, which the render refuses from a tree without it. + # + # With ".", the entire file is matched as a dict. + - key: "." + match: |- + Each(Key(IsIn([ + "enable_summarize", "interleaved_thinking", "llm_call_kwargs", "max_thinking_tokens", "max_turns", + "parser_name", "proactive_summarization_threshold", "reasoning_effort", "temperature", + "api_base", "llm_kwargs", "model_name" + ]))) + reason: each key must be a Terminus 2 argument + - key: [llm_call_kwargs, llm_call_kwargs.extra_body] + match: AnyOf(IsAbsent(), Eq(null), IsObject()) + reason: litellm reads it as an object + - key: max_turns + match: AnyOf(IsAbsent(), Eq(null), AllOf(IsInt(), Ge(1))) + reason: max_turns must be a positive integer + # Every model call stays on Reef's model binding. llm_kwargs holds + # only the binding's key and provider. Terminus 2 passes + # llm_call_kwargs to litellm 1.102.1 on every call, over the + # binding's values, so no argument there may choose the endpoint, the + # provider, a credential, the model or a fallback model, or add a + # logging callback that sends the call to another host. litellm sends + # an argument it does not read, and extra_body, in the request body, + # so neither names a model (model, or OpenRouter's fallback models). + - key: llm_kwargs + match: Each(Key(IsIn(["api_key", "custom_llm_provider"]))) + reason: Reef's model binding chooses the endpoint, the credential and the model + - key: llm_call_kwargs + match: |- + Each(Key(Not(AnyOf( + IsIn([ + "api_base", "api_key", "api_version", "azure", "base_url", "callbacks", "client", "client_id", + "client_secret", "context_window_fallback_dict", "custom_llm_provider", "deployment_id", + "failure_callback", "fallbacks", "litellm_credential_name", "mock_response", "mock_timeout", "model", + "model_alias_map", "model_list", "models", "region_name", "success_callback", "tenant_id", + "use_litellm_proxy" + ]), + StartsWith("arize_"), StartsWith("aws_"), StartsWith("azure_"), StartsWith("dd_"), StartsWith("gcs_"), + StartsWith("humanloop_"), StartsWith("langfuse_"), StartsWith("langsmith_"), StartsWith("newrelic_"), + StartsWith("posthog_"), StartsWith("s3_"), StartsWith("vertex_"), StartsWith("wandb_"), + StartsWith("watsonx_"), StartsWith("weave_"), + StartsWith("adaptive_router_"), StartsWith("auto_router_"), StartsWith("complexity_router_"), + StartsWith("quality_router_") + )))) + reason: Reef's model binding chooses the endpoint, the credential and the model + - key: llm_call_kwargs.extra_body + match: Not(Contains(Key(IsIn(["model", "models"])))) + reason: Reef's model binding chooses the endpoint, the credential and the model rules: "terminus/AGENTS.md" # Terminus 2 has no slash-command surface, so an agent_command is a skill in # a second root. Harbor loads both roots and keeps its progressive skill diff --git a/reef/harness/adapters/terminus/quirks.py b/reef/harness/adapters/terminus/quirks.py index 5df045ede..421ee2395 100644 --- a/reef/harness/adapters/terminus/quirks.py +++ b/reef/harness/adapters/terminus/quirks.py @@ -2,9 +2,9 @@ Terminus 2 takes its behavior from constructor arguments rather than a config file it discovers, so ``terminus/config.json`` is a flat object whose keys must -each name a real Terminus 2 argument. An unknown key is a defect in the tree, -and failing at render is what keeps a gated change meaning what it says instead -of silently dropping a knob. +each name a real Terminus 2 argument. The descriptor's checks refuse an +unknown key, a defect in the tree: failing at render is what keeps a gated +change meaning what it says instead of silently dropping a knob. Skills carry the ``name`` and ``description`` frontmatter the instruction builder reads. ``process_skill`` and ``process_command`` add it @@ -12,17 +12,12 @@ renders under the second root, and Harbor lists those skills beside the others, with nothing that marks them as commands. -Every model call stays on Reef's model binding. The binding writes -``model_name``, ``api_base`` and ``llm_kwargs`` with the key, renders after -the tree and wins every key it writes. The key is a credential, which a tree -cannot hold because admission refuses an inline credential, so those knobs -pass only beside it, and ``llm_kwargs`` only with the keys the binding -writes. Terminus 2 passes ``llm_call_kwargs`` to litellm on every call, over -the binding's values, so a key there that litellm reads as the endpoint, the -provider, a credential, the model, a fallback model or a logging callback -that sends the call elsewhere is refused. litellm sends a key it does not -read, and ``extra_body``, in the request body to the bound endpoint, so -neither may name a model (``model``, or OpenRouter's fallback ``models``). +The descriptor's checks also keep every model call on Reef's model binding. +The binding writes ``model_name``, ``api_base`` and ``llm_kwargs`` with the +key; the checks refuse other ``llm_kwargs`` keys, a ``llm_call_kwargs`` +argument that litellm reads as the endpoint, the provider, a credential, the +model, a fallback model or a logging callback, and a model named in +``extra_body``. These checks cover the config the tree renders. Reef's proxy forwards the ``model`` and ``models`` a request sends as they are, so a request that other @@ -37,9 +32,7 @@ from __future__ import annotations -import re from collections.abc import Mapping -from typing import Any from reef.harness.adapters.common.frontmatter import with_skill_frontmatter from reef.harness.adapters.descriptor import AdapterRenderer, ExecutionValidator @@ -47,58 +40,8 @@ from reef.harness.runners.terminus.tree import ENVIRONMENT_ENV, TerminusTreeError, extension_source from reef.harness.tree.render import RenderError -_CONFIG = "terminus/config.json" - -#: Terminus 2 constructor arguments a tree may set. Verified against harbor -#: 0.20.0, which reef-eval 0.1.1 resolves; a bump should re-check the signature. -_ALLOWED_KNOBS = { - "enable_summarize", - "interleaved_thinking", - "llm_call_kwargs", - "max_thinking_tokens", - "max_turns", - "parser_name", - "proactive_summarization_threshold", - "reasoning_effort", - "temperature", -} -#: Set by Reef's model binding, which renders after the tree and therefore -#: wins the merge. Admitted only beside the binding's key, in llm_kwargs. -_BINDING_KNOBS = {"api_base", "llm_kwargs", "model_name"} -BINDING_LLM_KWARGS = frozenset({"api_key", "custom_llm_provider"}) -#: litellm call arguments (read from litellm 1.102.1) that choose the endpoint, the provider, a credential, the model or a -#: fallback model, or add a logging callback that sends the call to another host. litellm sends an argument it does -#: not read, such as OpenRouter's fallback ``models``, in the request body. -MODEL_ROUTE_KWARGS = re.compile( - r"^(?:api_base|api_key|api_version|azure|base_url|callbacks|client|client_id|client_secret" - r"|context_window_fallback_dict|custom_llm_provider|deployment_id|failure_callback|fallbacks" - r"|litellm_credential_name|mock_response|mock_timeout|model|model_alias_map|model_list|models|region_name" - r"|success_callback|tenant_id|use_litellm_proxy)$" - r"|^(?:arize|aws|azure|dd|gcs|humanloop|langfuse|langsmith|newrelic|posthog|s3|vertex|wandb|watsonx|weave)_" - r"|^(?:adaptive|auto|complexity|quality)_router_" -) -#: Request body fields that name the model: ``model`` replaces the bound one, and OpenRouter reads ``models`` as -#: fallback models. -BODY_MODEL_KEYS = frozenset({"model", "models"}) - class TerminusAdapterRenderer(AdapterRenderer): - @staticmethod - def process_config(path: str, config: dict[str, Any]) -> dict[str, Any]: - unknown = sorted(set(config) - _ALLOWED_KNOBS - _BINDING_KNOBS) - if unknown: - raise RenderError(f"terminus config sets keys that are not Terminus 2 arguments: {', '.join(unknown)}") - call_kwargs = config.get("llm_call_kwargs") - if call_kwargs is not None and not isinstance(call_kwargs, dict): - raise RenderError("terminus config llm_call_kwargs must be an object") - body = (call_kwargs or {}).get("extra_body") - if body is not None and not isinstance(body, dict): - raise RenderError("terminus config llm_call_kwargs extra_body must be an object") - turns = config.get("max_turns") - if turns is not None and (isinstance(turns, bool) or not isinstance(turns, int) or turns < 1): - raise RenderError("terminus config max_turns must be a positive integer") - return config - @staticmethod def process_skill(path: str, text: str) -> str: return with_skill_frontmatter(path, text) @@ -108,24 +51,6 @@ def process_command(path: str, text: str) -> str: # A command is a skill under the second root, so it needs the same frontmatter. return with_skill_frontmatter(path, text) - @staticmethod - def check_model_route( - configs: Mapping[str, Mapping[str, Any]], skills: Mapping[str, str], commands: Mapping[str, str] - ) -> None: - config = configs[_CONFIG] - llm_kwargs = config.get("llm_kwargs") - refusal = "Reef's model binding chooses the endpoint, the credential and the model" - if isinstance(llm_kwargs, dict) and set(llm_kwargs) - BINDING_LLM_KWARGS: - extra = ", ".join(sorted(set(llm_kwargs) - BINDING_LLM_KWARGS)) - raise RenderError(f"terminus config must not set llm_kwargs {extra}: {refusal}") - call_kwargs = config.get("llm_call_kwargs") or {} - routed = sorted(key for key in call_kwargs if MODEL_ROUTE_KWARGS.search(key)) - if routed: - raise RenderError(f"terminus config must not set llm_call_kwargs {', '.join(routed)}: {refusal}") - named = sorted(set(call_kwargs.get("extra_body") or {}) & BODY_MODEL_KEYS) - if named: - raise RenderError(f"terminus config must not set llm_call_kwargs extra_body {', '.join(named)}: {refusal}") - @staticmethod def finalize_render(files: dict[str, str]) -> dict[str, str]: try: diff --git a/reef/harness/tree/render.py b/reef/harness/tree/render.py index 21e82ae38..5665dc407 100644 --- a/reef/harness/tree/render.py +++ b/reef/harness/tree/render.py @@ -6,8 +6,8 @@ target in tree order over the descriptor's enforced defaults; rules nodes concatenate in tree order into the rules file; named kinds render one file per node through the descriptor's path templates. The shared render refuses -a tree that sets a locked key off its target's defaults (or sets one the -defaults never name), then the descriptor's ``renderer`` processes the +a tree whose merged config fails a check its target declares, then the +descriptor's ``renderer`` processes the config, skill and command files, checks the model route, writes each config file in its harness's format, and gets the last word in ``finalize_render``, so what the descriptor declares and what its adapter enforces hold on every @@ -18,7 +18,7 @@ import json import re -from collections.abc import Iterable, Mapping, Sequence +from collections.abc import Mapping, Sequence from pathlib import PurePosixPath from typing import Any @@ -27,6 +27,7 @@ from reef.core.errors import ReefError from reef.harness.adapters.descriptor import AdapterDescriptor +from reef.harness.adapters.matchers import Each, select from reef.harness.tree.nodes import NATIVE_LOOP_DEFAULT_MAX_STEPS @@ -221,7 +222,7 @@ def apply_adapter_renderer(descriptor: AdapterDescriptor, files: dict[str, str]) renderer = descriptor.renderer merged = {target.path: json.loads(files[target.path]) for target in descriptor.config_targets.values()} check_model_route(descriptor, merged) - check_locked_keys(descriptor, merged) + check_config(descriptor, merged) configs = {path: renderer.process_config(path, config) for path, config in merged.items()} skill_path = template_pattern(descriptor.node_paths["skill"]) command_template = descriptor.node_paths.get("agent_command") @@ -274,71 +275,25 @@ def check_model_route(descriptor: AdapterDescriptor, configs: Mapping[str, Mappi ) -def check_locked_keys(descriptor: AdapterDescriptor, configs: Mapping[str, Mapping[str, Any]]) -> None: - """Refuse a tree that sets a locked key off its target's defaults, or sets one the defaults never name. +def check_config(descriptor: AdapterDescriptor, configs: Mapping[str, Mapping[str, Any]]) -> None: + """Refuse a tree whose merged config fails one of the checks its target declares. - The descriptor's ``locked`` paths state what a composition must keep: after the merge, the value at each - locked path must equal the defaults' value there, and a path the defaults do not set must not appear at all - - an empty value is a set one. A section of the defaults the tree clobbers with a non object refuses too, since - it takes every locked key below it away. The check runs once for every adapter, after the binding check and - before the renderer's steps, so a declared lock holds on every rendered tree and no adapter can skip it. + The checks run once for every adapter, after the binding check and before the renderer's steps, so a declared + check holds on every rendered tree and no adapter can skip it. A refusal names the key, the entry or item of + an ``Each`` that fails, and the check's reason, or its match expression when it gives none. It never names the + value, which can be a credential the binding wrote. """ for target in descriptor.config_targets.values(): - config = configs[target.path] - for locked in target.locked: - segments = tuple(locked.split(".")) - for found, value in _locked_matches(config, segments, ()): - named, expected = _value_at(target.defaults, found) - if not named: - raise RenderError( - f"{descriptor.name} composition must not set {'.'.join(found)} in {target.path}: " - "the descriptor locks it, and its defaults do not set it" - ) - if isinstance(value, bool) != isinstance(expected, bool) or value != expected: - raise RenderError( - f"{descriptor.name} composition must keep {'.'.join(found)} {json.dumps(expected, default=repr)} " - f"in {target.path}: the descriptor locks it" - ) - for kept, expected in _locked_matches(target.defaults, segments, ()): - if not _locked_matches(config, kept, ()): - raise RenderError( - f"{descriptor.name} composition must keep {'.'.join(kept)} {json.dumps(expected, default=repr)} " - f"in {target.path}: the descriptor locks it, and the merged config does not have it" - ) - - -def _locked_matches(data: Any, segments: tuple[str, ...], path: tuple[str, ...]) -> list[tuple[tuple[str, ...], Any]]: - """Each concrete key path ``segments`` matches in ``data``, with the value there. - - A ``*`` segment matches every key of an object and every item of a list; a segment that would read through a - value that is neither matches nothing, so a pattern never crashes on a config of the wrong shape. - """ - if not segments: - return [(path, data)] - first, rest = segments[0], segments[1:] - if first == "*": - found: Iterable[tuple[Any, Any]] = ( - data.items() if isinstance(data, Mapping) else enumerate(data) if isinstance(data, list) else () - ) - elif isinstance(data, Mapping) and first in data: - found = ((first, data[first]),) - else: - found = () - return [match for key, value in found for match in _locked_matches(value, rest, (*path, str(key)))] - - -def _value_at(data: Any, segments: tuple[str, ...]) -> tuple[bool, Any]: - """Whether ``segments`` names a value in ``data``, and that value; a path through anything else is unnamed.""" - for segment in segments: - if isinstance(data, Mapping): - if segment not in data: - return False, None - data = data[segment] - elif isinstance(data, list) and segment.isdigit() and int(segment) < len(data): - data = data[int(segment)] - else: - return False, None - return True, data + for check in target.checks: + for path, value in select(configs[target.path], check.segments): + if check.matcher.matches(value): + continue + failure = check.matcher.first_failure(value) if isinstance(check.matcher, Each) else None + where = " ".join(part for part in (".".join(path), failure) if part) or "the whole file" + raise RenderError( + f"{descriptor.name} composition fails the check on {where} in {target.path}: " + f"{check.reason or f'it must match {check.source}'}" + ) def binding_leaves(data: Mapping[str, Any], segments: tuple[str, ...]) -> list[tuple[tuple[str, ...], Any]]: diff --git a/tests/reef_service/test_harness_config_checks.py b/tests/reef_service/test_harness_config_checks.py new file mode 100644 index 000000000..0e479173e --- /dev/null +++ b/tests/reef_service/test_harness_config_checks.py @@ -0,0 +1,241 @@ +"""Config checks: the matchers a descriptor's ``checks`` use, how a key selects values, and how render refuses.""" + +from __future__ import annotations + +import json +from dataclasses import replace +from pathlib import Path + +import pytest +import yaml + +import reef.harness.adapters +from reef.harness.adapters import get_adapter +from reef.harness.adapters.descriptor import AdapterDescriptor, AdapterRenderer, DescriptorError, load_descriptor +from reef.harness.adapters.matchers import ABSENT, parse_matcher +from reef.harness.tree.render import RenderError, render_composition + +OPENCODE = "opencode/opencode.json" + + +@pytest.mark.parametrize( + ("source", "value", "expected"), + [ + # Eq and IsIn compare with types: a boolean equals only a boolean, and a string never a number. + ("Eq(false)", False, True), + ("Eq(false)", 0, False), + ("Eq(false)", "false", False), + ("Eq(1)", 1.0, True), + ("Eq([])", [], True), + ("Eq({})", {}, True), + ('Eq({"a": [1, null]})', {"a": [1, None]}, True), + ("Eq(-1)", -1, True), + ('IsIn(["a", 1])', 1, True), + ('IsIn(["a", 1])', "1", False), + # Text matchers read a scalar's JSON text and match no object, list or absent key. + ('Strip(Lowercase(IsIn(["0", "false", ""])))', 0, True), + ('Strip(Lowercase(IsIn(["0", "false", ""])))', " FALSE ", True), + ('Strip(Lowercase(IsIn(["0", "false", ""])))', " ", True), + ('Strip(Lowercase(IsIn(["0", "false", ""])))', False, True), + ('Lowercase(Eq("x"))', {"x": 1}, False), + ('Uppercase(Eq("ABC"))', "abc", True), + ('StartsWith("!!js ")', "!!js 1", True), + ('StartsWith("!!js ")', ABSENT, False), + ('EndsWith("_MODEL")', "SUBAGENT_MODEL", True), + ('HasSubstr("PROXY")', "HTTPS_PROXY", True), + ('MatchesRegex("a+")', "aaa", True), + ('MatchesRegex("a+")', "aaab", False), + # Only IsAbsent, Not and Each match an absent key. + ("IsAbsent()", ABSENT, True), + ("IsAbsent()", None, False), + ('Not(Eq("x"))', ABSENT, True), + ('Each(Key(Eq("a")))', ABSENT, True), + ('Contains(Key(Eq("a")))', ABSENT, False), + ("IsObject()", ABSENT, False), + # Each and Contains visit an object's entries or a list's items, and nothing else. + ('Each(Key(IsIn(["a", "b"])))', {"a": 1, "b": 2}, True), + ('Each(Key(IsIn(["a", "b"])))', {"a": 1, "c": 2}, False), + ("Each(Value(IsObject()))", {"a": {}, "b": "x"}, False), + ("Each(Ge(1))", [1, 2], True), + ("Each(Ge(1))", "12", False), + ('Contains(Key(IsIn(["model", "models"])))', {"metadata": 1, "models": []}, True), + ('Contains(Eq("a"))', ["b", "a"], True), + ('Key(Eq("a"))', "a", False), + # ParseJson reads a string; a value that is not JSON matches nothing. + ('ParseJson(AllOf(IsObject(), Not(Contains(Key(Eq("model"))))))', '{"metadata": {}}', True), + ('ParseJson(AllOf(IsObject(), Not(Contains(Key(Eq("model"))))))', '{"model": "m"}', False), + ("ParseJson(IsObject())", "{model: m}", False), + ("ParseJson(IsObject())", '{"a": 1}', False), + ("ParseJson(IsObject())", {"a": 1}, False), + # Numbers, and the booleans that Python counts as integers. + ("AllOf(IsInt(), Ge(1))", 3, True), + ("AllOf(IsInt(), Ge(1))", 0, False), + ("AllOf(IsInt(), Ge(1))", True, False), + ("Ge(1)", "5", False), + ("AnyOf(IsAbsent(), Eq(null))", None, True), + ("AnyOf(IsAbsent(), Eq(null))", "", False), + ], +) +def test_a_matcher_matches_by_its_definition(source: str, value: object, expected: bool) -> None: + assert parse_matcher(source, None).matches(value) is expected + + +@pytest.mark.parametrize( + ("source", "message"), + [ + ("Eq(", "cannot parse"), + ("Foo(1)", "unknown matcher Foo"), + ("__import__('os')", "unknown matcher __import__"), + ("Eq(True)", "write true for a boolean"), + ("Eq(x)", "x is not a JSON literal"), + ("Not(1)", "1 is not a matcher call"), + ("Not(Eq(1)).matches", "is not a matcher call"), + ("Eq(1, 2)", "Eq takes one argument"), + ("Eq(value=1)", "is not a matcher call"), + ("IsAbsent(1)", "IsAbsent takes no arguments"), + ("AllOf()", "AllOf takes one or more matchers"), + ("IsIn(1)", "IsIn takes a list"), + ("StartsWith(1)", "StartsWith takes a string"), + ("Ge('1')", "Ge takes a number"), + ("MatchesRegex('(')", "is not a regular expression"), + ("IsDefault()", r"IsDefault\(\) needs a key without \* or \*\*"), + ], +) +def test_a_match_expression_that_is_not_one_matcher_call_is_refused(source: str, message: str) -> None: + with pytest.raises(ValueError, match=message): + parse_matcher(source, None) + + +def checked( + tmp_path: Path, checks: object, defaults: dict[str, object] | None = None, *, renderer: bool = False +) -> AdapterDescriptor: + """The opencode descriptor with these checks and defaults on its primary target, loaded from YAML; with the + base renderer unless ``renderer``, so only the checks act.""" + data = yaml.safe_load((Path(reef.harness.adapters.__file__).parent / "opencode" / "descriptor.yaml").read_text()) + primary = data["files"]["config"]["primary"] + primary["checks"] = checks + if defaults is not None: + primary["defaults"] = defaults + path = tmp_path / "descriptor.yaml" + path.write_text(yaml.safe_dump(data), encoding="utf-8") + descriptor = load_descriptor(path) + return descriptor if renderer else replace(descriptor, renderer=AdapterRenderer) + + +def render_config(descriptor: AdapterDescriptor, data: dict[str, object]) -> dict[str, object]: + return json.loads(render_composition([("config", {"data": data})], descriptor)[OPENCODE]) + + +def test_is_default_keeps_the_value_the_defaults_set(tmp_path: Path) -> None: + descriptor = checked( + tmp_path, + [{"key": ["autoupdate", "nested.key"], "match": "IsDefault()"}], + {"autoupdate": False, "nested": {"key": 1}}, + ) + assert render_config(descriptor, {"autoupdate": False, "nested": {"key": 1}})["nested"] == {"key": 1} + with pytest.raises(RenderError, match=r"on autoupdate in opencode/opencode\.json: it must match IsDefault\(\)"): + render_config(descriptor, {"autoupdate": True}) + # Booleans only match booleans: false refuses 0, which == false. + with pytest.raises(RenderError, match="on autoupdate in"): + render_config(descriptor, {"autoupdate": 0}) + # A section the tree replaces with a value that is not an object takes the checked key below it away. + with pytest.raises(RenderError, match=r"on nested\.key in"): + render_config(descriptor, {"nested": "flat"}) + + +def test_is_default_of_a_key_the_defaults_do_not_set_is_is_absent(tmp_path: Path) -> None: + descriptor = checked(tmp_path, [{"key": "small_model", "match": "IsDefault()"}], {}) + for value in ["reef/served", None, "", {}, []]: + # An empty value is a set one. + with pytest.raises(RenderError, match="on small_model in"): + render_config(descriptor, {"small_model": value}) + assert "small_model" not in render_config(descriptor, {}) + + +def test_a_star_visits_every_key_of_an_object_and_every_item_of_a_list(tmp_path: Path) -> None: + descriptor = checked(tmp_path, [{"key": ["agents.*.model", "tasks.*.model"], "match": "IsAbsent()"}], {}) + with pytest.raises(RenderError, match=r"on agents\.reviewer\.model in"): + render_config(descriptor, {"agents": {"reviewer": {"model": "m"}}}) + with pytest.raises(RenderError, match=r"on tasks\.0\.model in"): + render_config(descriptor, {"tasks": [{"model": "m"}]}) + assert render_config(descriptor, {"agents": {"reviewer": {"tools": ["read"]}}, "tasks": "none"})["tasks"] == "none" + + +def test_a_double_star_visits_every_value_below_the_key(tmp_path: Path) -> None: + descriptor = checked(tmp_path, [{"key": "plugin.**", "match": 'Not(StartsWith("!!js "))'}], {}) + with pytest.raises(RenderError, match=r"on plugin\.config\.agents\.1\.model in"): + render_config(descriptor, {"plugin": {"config": {"agents": [{}, {"model": "!!js pick()"}]}}}) + with pytest.raises(RenderError, match=r"on plugin in"): + render_config(descriptor, {"plugin": "!!js ({})"}) + render_config(descriptor, {"plugin": {"config": {"model": "!!jsx is text"}}, "other": "!!js 1"}) + + +def test_a_refusal_names_the_failing_key_and_the_reason_but_not_the_value(tmp_path: Path) -> None: + check = {"key": "env", "match": 'Each(Key(Not(HasSubstr("TOKEN"))))', "reason": "the binding owns credentials"} + descriptor = checked(tmp_path, [check], {}) + with pytest.raises(RenderError) as refusal: + render_config(descriptor, {"env": {"A": "1", "MY_TOKEN": "secret-value"}}) + assert str(refusal.value) == ( + "opencode composition fails the check on env key 'MY_TOKEN' in opencode/opencode.json: " + "the binding owns credentials" + ) + # Without a reason, the refusal gives the match expression, on one line however the descriptor wraps it. + descriptor = checked(tmp_path, [{"key": ".", "match": "Each(Key(\n IsIn(['a'])\n))"}], {}) + with pytest.raises(RenderError, match=r"on key 'b' in opencode/opencode\.json: it must match Each\(Key\( IsIn"): + render_config(descriptor, {"b": 1}) + + +def test_checks_run_before_the_adapter_steps(tmp_path: Path) -> None: + # opencode's own step would refuse this tree, which leaves no agent to start a run; the check refuses it first. + descriptor = checked(tmp_path, [{"key": "agent", "match": "IsAbsent()", "reason": "checked first"}], renderer=True) + with pytest.raises(RenderError, match="checked first"): + render_config(descriptor, {"agent": {"build": {"disable": True}, "plan": {"disable": True}}}) + + +@pytest.mark.parametrize( + ("checks", "message"), + [ + pytest.param({"key": "a"}, "must be a list", id="not-a-list"), + pytest.param([{"key": "a"}], "requires a match expression", id="no-match"), + pytest.param([{"match": "IsAbsent()"}], r"requires a key: .*'\.' for the whole file", id="no-key"), + pytest.param([{"key": "", "match": "IsAbsent()"}], "dot separated key names", id="empty-key"), + pytest.param( + [{"key": "a", "match": "IsAbsent()", "why": "x"}], "must be an object with key and match", id="field" + ), + pytest.param([{"key": "a", "match": "IsAbsent()", "reason": 1}], "reason must be a string", id="reason"), + pytest.param([{"key": "a..b", "match": "IsAbsent()"}], "dot separated key names", id="empty-segment"), + pytest.param([{"key": [".a"], "match": "IsAbsent()"}], "dot separated key names", id="leading-dot"), + pytest.param([{"key": ["a", 5], "match": "IsAbsent()"}], "list of non-empty strings", id="key-type"), + pytest.param([{"key": "a", "match": "Foo()"}], r"checks\[0\] match: unknown matcher Foo", id="matcher"), + pytest.param([{"key": "a.*", "match": "IsDefault()"}], r"IsDefault\(\) needs a key without", id="default"), + ], +) +def test_checks_are_validated_at_load(tmp_path: Path, checks: object, message: str) -> None: + with pytest.raises(DescriptorError, match=message): + checked(tmp_path, checks) + + +def test_a_descriptor_that_still_declares_locked_keys_is_refused(tmp_path: Path) -> None: + data = yaml.safe_load((Path(reef.harness.adapters.__file__).parent / "opencode" / "descriptor.yaml").read_text()) + data["files"]["config"]["primary"]["locked"] = ["autoupdate"] + path = tmp_path / "descriptor.yaml" + path.write_text(yaml.safe_dump(data), encoding="utf-8") + with pytest.raises(DescriptorError, match=r"'locked' is replaced by 'checks'.*IsDefault\(\).*IsAbsent\(\)"): + load_descriptor(path) + + +def test_a_check_with_a_list_of_keys_is_one_check_per_key(tmp_path: Path) -> None: + descriptor = checked(tmp_path, [{"key": ["a", "b.c"], "match": "IsAbsent()", "reason": "r"}], {}) + assert [(check.key, check.reason) for check in descriptor.config_targets["primary"].checks] == [ + ("a", "r"), + ("b.c", "r"), + ] + + +def test_checks_change_no_rendered_file() -> None: + descriptor = get_adapter("opencode") + unchecked = replace( + descriptor, + config_targets={name: replace(target, checks=()) for name, target in descriptor.config_targets.items()}, + ) + assert render_composition([], unchecked) == render_composition([], descriptor) diff --git a/tests/reef_service/test_harness_model_route.py b/tests/reef_service/test_harness_model_route.py index 0398bd257..bcace43fc 100644 --- a/tests/reef_service/test_harness_model_route.py +++ b/tests/reef_service/test_harness_model_route.py @@ -51,8 +51,7 @@ def config(data: dict[str, Any], target: str = "primary") -> tuple[str, dict[str pytest.param({"env": {"CLAUDE_CODE_EXTRA_BODY": "{model: m}"}}, id="body-render-cannot-read"), ] -#: settings.json keys the descriptor locks: fixed names a tree must not set, so the shared render refuses them -#: before the adapter's own route check runs. +#: settings.json keys whose descriptor check is IsAbsent(): fixed names a tree must not set. CLAUDE_LOCKED = [ pytest.param({"model": "m"}, id="settings-model"), pytest.param({"fallbackModel": ["m"]}, id="settings-fallback-model"), @@ -72,7 +71,7 @@ def test_claude_refuses_a_setting_that_chooses_the_route(data: dict[str, Any], b @pytest.mark.parametrize("data", CLAUDE_LOCKED) @pytest.mark.parametrize("bound", [False, True], ids=["tree", "bound"]) def test_claude_refuses_a_key_the_descriptor_locks(data: dict[str, Any], bound: bool) -> None: - with pytest.raises(RenderError, match="the descriptor locks it"): + with pytest.raises(RenderError, match=r"on \w+ in claude/settings\.json: Reef's model binding"): render("claude", [config(data)], bound=bound, api="anthropic") @@ -183,8 +182,8 @@ def test_hermes_refuses_a_setting_that_chooses_the_route(data: dict[str, Any], b render("hermes", [config(data)], bound=bound) -#: Top-level keys the descriptor locks: hermes moves them into ``model``, and a tree must not set them at all, -#: so the shared render refuses them before the adapter's own route check runs. +#: Top-level keys whose descriptor check is IsAbsent(): hermes moves them into ``model``, and a tree must not set +#: them at all. HERMES_LOCKED = [ pytest.param({"fallback_model": {"provider": "custom", "model": "m", "base_url": OTHER_URL}}, id="fallback-model"), pytest.param({"fallback_providers": [{"provider": "openrouter", "model": "m"}]}, id="fallback-providers"), @@ -201,7 +200,7 @@ def test_hermes_refuses_a_setting_that_chooses_the_route(data: dict[str, Any], b @pytest.mark.parametrize("data", HERMES_LOCKED) @pytest.mark.parametrize("bound", [False, True], ids=["tree", "bound"]) def test_hermes_refuses_a_key_the_descriptor_locks(data: dict[str, Any], bound: bool) -> None: - with pytest.raises(RenderError, match="the descriptor locks it"): + with pytest.raises(RenderError, match=r"on \w+ in hermes/config\.yaml: Reef's model binding"): render("hermes", [config(data)], bound=bound) @@ -225,7 +224,7 @@ def test_hermes_refuses_a_key_the_descriptor_locks(data: dict[str, Any], bound: @pytest.mark.parametrize("api_mode", HERMES_API_MODES) @pytest.mark.parametrize("bound", [False, True], ids=["tree", "bound"]) def test_hermes_refuses_an_api_mode_the_binding_does_not_set(api_mode: str, bound: bool) -> None: - with pytest.raises(RenderError, match=r"must not set model\.api_mode: Reef's model binding"): + with pytest.raises(RenderError, match=r"on model\.api_mode in hermes/config\.yaml: Reef's model binding"): render("hermes", [config({"model": {"api_mode": api_mode}})], bound=bound) @@ -323,8 +322,7 @@ def test_dsh_keeps_the_settings_a_tree_tunes(api: str) -> None: pytest.param(config({"defaultProvider": "evil", "defaultModel": "evil/m"}), False, id="default-model"), ] -#: settings.json keys the descriptor locks: fixed names a tree must not set, so the shared render refuses them -#: before the adapter's own route check runs. +#: settings.json keys whose descriptor check is IsAbsent(): fixed names a tree must not set. PI_LOCKED = [ pytest.param(config({"enabledModels": ["evil/m"]}), id="enabled-models"), pytest.param(config({"httpProxy": OTHER_URL}), id="proxy"), @@ -340,7 +338,7 @@ def test_pi_refuses_a_setting_that_chooses_the_route(node: tuple[str, dict[str, @pytest.mark.parametrize("node", PI_LOCKED) @pytest.mark.parametrize("bound", [False, True], ids=["tree", "bound"]) def test_pi_refuses_a_key_the_descriptor_locks(node: tuple[str, dict[str, Any]], bound: bool) -> None: - with pytest.raises(RenderError, match="the descriptor locks it"): + with pytest.raises(RenderError, match=r"on \w+ in pi-agent/settings\.json: Reef's model binding"): render("pi", [node], bound=bound) @@ -381,7 +379,9 @@ def test_terminus_refuses_a_setting_that_chooses_the_route(data: dict[str, Any], def test_terminus_refuses_a_request_body_that_is_not_an_object() -> None: - with pytest.raises(RenderError, match="extra_body must be an object"): + with pytest.raises( + RenderError, match=r"on llm_call_kwargs\.extra_body in terminus/config\.json: litellm reads it as an object" + ): render("terminus", [config({"llm_call_kwargs": {"extra_body": '{"model": "m"}'}})], bound=True) @@ -403,11 +403,11 @@ def test_terminus_keeps_the_arguments_a_tree_tunes() -> None: "Reef's model binding", ), ("claude", {"env": {"CLAUDE_CODE_EXTRA_BODY": '{"model": "m"}'}}, "Reef's model binding"), - ("claude", {"fallbackModel": ["m"]}, "the descriptor locks it"), + ("claude", {"fallbackModel": ["m"]}, "Reef's model binding"), ( "hermes", {"fallback_model": {"provider": "custom", "model": "m", "base_url": OTHER_URL}}, - "the descriptor locks it", + "Reef's model binding", ), ( "hermes", @@ -417,7 +417,7 @@ def test_terminus_keeps_the_arguments_a_tree_tunes() -> None: ("hermes", {"delegation": {"request_overrides": {"model": "m"}}}, "Reef's model binding"), ("hermes", {"model": {"api_mode": "bedrock_converse"}}, "Reef's model binding"), ("dsh", {"llm-pi-ai": {"config": {"providers": {"evil": DSH_ROUTE}}}}, "Reef's model binding"), - ("pi", {"enabledModels": ["evil/m"]}, "the descriptor locks it"), + ("pi", {"enabledModels": ["evil/m"]}, "Reef's model binding"), ("terminus", {"llm_call_kwargs": {"base_url": OTHER_URL}}, "Reef's model binding"), ("terminus", {"llm_call_kwargs": {"extra_body": {"model": "m"}}}, "Reef's model binding"), ], diff --git a/tests/reef_service/test_harness_render.py b/tests/reef_service/test_harness_render.py index 09452d55a..a3f82056b 100644 --- a/tests/reef_service/test_harness_render.py +++ b/tests/reef_service/test_harness_render.py @@ -15,14 +15,7 @@ import reef.harness.adapters from reef.harness.adapters import available_adapters, get_adapter from reef.harness.adapters.common.frontmatter import with_skill_frontmatter -from reef.harness.adapters.descriptor import ( - AdapterDescriptor, - AdapterRenderer, - ClientState, - ConfigTarget, - DescriptorError, - load_descriptor, -) +from reef.harness.adapters.descriptor import AdapterRenderer, ClientState, DescriptorError, load_descriptor from reef.harness.adapters.hermes.quirks import DEFAULT_IDENTITY from reef.harness.adapters.opencode.quirks import read_frontmatter from reef.harness.episodes.model_binding import ModelBinding, ModelBindingError @@ -112,25 +105,31 @@ def test_codex_rejects_code_extensions_until_hooks_have_separate_isolation() -> def test_codex_quirk_rejects_reopened_hermetic_switches() -> None: descriptor = get_adapter("codex") for approval_policy in ("on-request", "never"): - with pytest.raises(RenderError, match="may not set approval_policy"): + with pytest.raises(RenderError, match=r"on approval_policy in codex/config\.toml: episodes pin never"): render_composition([("config", {"data": {"approval_policy": approval_policy}})], descriptor) - with pytest.raises(RenderError, match="web_search must be one of disabled, cached, indexed, live"): + with pytest.raises(RenderError, match=r"on web_search in codex/config\.toml: it must match IsIn\("): render_composition([("config", {"data": {"web_search": "always"}})], descriptor) for data, message in ( - ({"features": {"apps": True}}, r"features\.apps disabled"), - ({"features": {"hooks": True}}, r"features\.hooks disabled"), - ({"features": {"plugins": True}}, r"features\.plugins disabled"), - ({"features": {"skill_mcp_dependency_install": True}}, r"skill_mcp_dependency_install disabled"), - ({"sandbox_workspace_write": {"network_access": True}}, r"network_access false"), - ({"sandbox_workspace_write": {"writable_roots": ["/tmp/outside"]}}, "may not add"), - ({"otel": {"exporter": "otlp-http"}}, "OpenTelemetry exporter disabled"), - ({"mcp_servers": {"untrusted": {"command": "tool"}}}, "config keys are not admitted"), - ({"hooks": {"SessionStart": []}}, "config keys are not admitted"), - ({"notify": ["untrusted-command"]}, "config keys are not admitted"), - ({"model_instructions_file": "/etc/passwd"}, "config keys are not admitted"), - ({"experimental_compact_prompt_file": "/etc/passwd"}, "config keys are not admitted"), - ({"model_catalog_json": "/etc/passwd"}, "config keys are not admitted"), - ({"agents": {"reviewer": {"config_file": "/etc/passwd"}}}, "config keys are not admitted"), + ({"features": {"apps": True}}, r"on features\.apps in"), + ({"features": {"hooks": True}}, r"on features\.hooks in"), + ({"features": {"plugins": True}}, r"on features\.plugins in"), + ({"features": {"skill_mcp_dependency_install": True}}, r"on features\.skill_mcp_dependency_install in"), + ({"sandbox_workspace_write": {"network_access": True}}, r"on sandbox_workspace_write\.network_access in"), + ( + {"sandbox_workspace_write": {"writable_roots": ["/tmp/outside"]}}, + r"on sandbox_workspace_write\.writable_roots in", + ), + ({"otel": {"exporter": "otlp-http"}}, "on otel in"), + ({"mcp_servers": {"untrusted": {"command": "tool"}}}, "config keys are not admitted for benchmark episodes"), + ({"hooks": {"SessionStart": []}}, "config keys are not admitted for benchmark episodes"), + ({"notify": ["untrusted-command"]}, "config keys are not admitted for benchmark episodes"), + ({"model_instructions_file": "/etc/passwd"}, "config keys are not admitted for benchmark episodes"), + ({"experimental_compact_prompt_file": "/etc/passwd"}, "config keys are not admitted for benchmark episodes"), + ({"model_catalog_json": "/etc/passwd"}, "config keys are not admitted for benchmark episodes"), + ( + {"agents": {"reviewer": {"config_file": "/etc/passwd"}}}, + "config keys are not admitted for benchmark episodes", + ), ): with pytest.raises(RenderError, match=message): render_composition([("config", {"data": data})], descriptor) @@ -202,7 +201,9 @@ def test_terminus_folds_agent_commands_into_a_second_skill_root() -> None: def test_terminus_rejects_config_keys_that_are_not_constructor_arguments() -> None: - with pytest.raises(RenderError, match="not Terminus 2 arguments"): + with pytest.raises( + RenderError, match=r"key 'tmux_pane_width' in terminus/config\.json: each key must be a Terminus 2 argument" + ): render_composition([("config", {"data": {"tmux_pane_width": 200}})], get_adapter("terminus")) @@ -414,22 +415,23 @@ def test_dsh_binds_both_profiles_in_every_dialect(api, route_api, base_url) -> N def test_dsh_locks_refuse_a_patch_that_breaks_the_episode() -> None: descriptor = get_adapter("dsh") - with pytest.raises(RenderError, match=r'must keep session-persistence-jsonl\.config\.compression "none"'): + with pytest.raises( + RenderError, + match=r"on session-persistence-jsonl\.config\.compression in dsh/profiles/headless/cordis\.patch\.yml", + ): render_composition( [("config", {"data": {"session-persistence-jsonl": {"config": {"compression": "zstd"}}}})], descriptor ) - with pytest.raises(RenderError, match=r"must keep session-telemetry-otel\.disabled true"): + with pytest.raises(RenderError, match=r"on session-telemetry-otel\.disabled in"): render_composition([("config", {"data": {"session-telemetry-otel": {"disabled": False}}})], descriptor) - with pytest.raises(RenderError, match="must be an object"): + with pytest.raises(RenderError, match=r"key 'agent-loop' in .*: a patch entry is an object"): render_composition([("config", {"data": {"agent-loop": "nope"}})], descriptor) - # A patch entry that is not an object takes the locked keys below it away; the lock refuses it as such, + # A patch entry that is not an object takes the checked keys below it away; the check refuses it as such, # where the chained reads a hand written check needs would crash on the shape. - with pytest.raises(RenderError, match=r'must keep session-persistence-jsonl\.config\.compression "none"'): + with pytest.raises(RenderError, match=r"on session-persistence-jsonl\.config\.compression in"): render_composition([("config", {"data": {"session-persistence-jsonl": "zstd"}})], descriptor) # The web profile's patch is held to the same checks: a compressed web profile refuses the shared sessions root. - with pytest.raises( - RenderError, match=rf'must keep session-persistence-jsonl\.config\.compression "none" in {DSH_WEB_PATCH}' - ): + with pytest.raises(RenderError, match=rf"on session-persistence-jsonl\.config\.compression in {DSH_WEB_PATCH}"): render_composition( [ ( @@ -439,13 +441,13 @@ def test_dsh_locks_refuse_a_patch_that_breaks_the_episode() -> None: ], descriptor, ) - with pytest.raises(RenderError, match=f"must keep session-title-llm.disabled true in {DSH_WEB_PATCH}"): + with pytest.raises(RenderError, match=f"on session-title-llm.disabled in {DSH_WEB_PATCH}"): render_composition( [("config", {"target": "web", "data": {"session-title-llm": {"disabled": False}}})], descriptor ) # And its manifest keeps the patch read once at start: with live reload dsh web exits at start. live = {"dsh": {"profile": {"patchReload": "live"}}} - with pytest.raises(RenderError, match=r"must keep dsh\.profile\.patchReload"): + with pytest.raises(RenderError, match=r"on dsh\.profile\.patchReload in"): render_composition([("config", {"target": "web_manifest", "data": live})], descriptor) @@ -520,17 +522,17 @@ def test_hermes_quirks_emit_the_config_the_plugin_grants_and_skill_frontmatter() def test_hermes_locks_refuse_a_config_that_breaks_the_episode() -> None: descriptor = get_adapter("hermes") - with pytest.raises(RenderError, match=r"must keep security\.tirith_enabled false"): + with pytest.raises(RenderError, match=r"on security\.tirith_enabled in"): render_composition([("config", {"data": {"security": {"tirith_enabled": True}}})], descriptor) - with pytest.raises(RenderError, match=r"must keep auxiliary\.title_generation\.enabled false"): + with pytest.raises(RenderError, match=r"on auxiliary\.title_generation\.enabled in"): render_composition([("config", {"data": {"auxiliary": {"title_generation": {"enabled": True}}}})], descriptor) - with pytest.raises(RenderError, match=r"must keep sessions\.write_json_snapshots true"): + with pytest.raises(RenderError, match=r"on sessions\.write_json_snapshots in"): render_composition([("config", {"data": {"sessions": {"write_json_snapshots": False}}})], descriptor) - with pytest.raises(RenderError, match=r"must keep memory\.nudge_interval 0"): + with pytest.raises(RenderError, match=r"on memory\.nudge_interval in"): render_composition([("config", {"data": {"memory": {"nudge_interval": 10}}})], descriptor) - with pytest.raises(RenderError, match=r"must keep skills\.creation_nudge_interval 0"): + with pytest.raises(RenderError, match=r"on skills\.creation_nudge_interval in"): render_composition([("config", {"data": {"skills": {"creation_nudge_interval": 10}}})], descriptor) - with pytest.raises(RenderError, match=r"must keep curator\.enabled false"): + with pytest.raises(RenderError, match=r"on curator\.enabled in"): render_composition([("config", {"data": {"curator": {"enabled": True}}})], descriptor) @@ -550,7 +552,9 @@ def test_hermes_admission_refuses_a_config_section_that_is_not_an_object() -> No entries, refusal = admit_mutations( [], [Mutation("create", "c1", {"name": "config", "config": {"data": data}})], descriptor ) - assert entries == [] and refusal is not None and refusal.startswith("hermes composition must keep"), data + assert ( + entries == [] and refusal is not None and refusal.startswith("hermes composition fails the check on") + ), data tracer = Mutation( "create", "e1", @@ -665,7 +669,7 @@ def test_two_nodes_cannot_render_to_the_same_path() -> None: def test_opencode_descriptor_lock_rejects_reopened_autoupdate() -> None: - with pytest.raises(RenderError, match=r"must keep autoupdate false in opencode/opencode\.json"): + with pytest.raises(RenderError, match=r"on autoupdate in opencode/opencode\.json: it must match IsDefault\(\)"): render_composition([("config", {"data": {"autoupdate": True}})], get_adapter("opencode")) @@ -690,13 +694,13 @@ def test_opencode_descriptor_lock_rejects_reopened_autoupdate() -> None: ({"provider": {"reef": {"options": {"baseURL": "http://127.0.0.1:9/v1"}}}}, "must not set provider"), (BINDING_COPY, "must not set provider"), ({"model": "reef/other"}, "must not set model"), - ({"small_model": "reef/served"}, "must not set small_model"), - ({"disabled_providers": ["reef"]}, "must not set disabled_providers"), - ({"enabled_providers": ["reef", "opencode"]}, r'must keep enabled_providers \["reef"\]'), - ({"enabled_providers": []}, r'must keep enabled_providers \["reef"\]'), - ({"agent": {"build": {"model": "evil/m"}}}, "must not set agent.build.model"), - ({"mode": {"chat": {"model": "evil/m"}}}, "must not set mode.chat.model"), - ({"command": {"hi": {"template": "Hi.", "model": "evil/m"}}}, "must not set command.hi.model"), + ({"small_model": "reef/served"}, r"on small_model in opencode/opencode\.json: Reef's model binding"), + ({"disabled_providers": ["reef"]}, "on disabled_providers in"), + ({"enabled_providers": ["reef", "opencode"]}, "on enabled_providers in"), + ({"enabled_providers": []}, "on enabled_providers in"), + ({"agent": {"build": {"model": "evil/m"}}}, r"on agent\.build\.model in"), + ({"mode": {"chat": {"model": "evil/m"}}}, r"on mode\.chat\.model in"), + ({"command": {"hi": {"template": "Hi.", "model": "evil/m"}}}, r"on command\.hi\.model in"), ], ) def test_opencode_quirk_refuses_a_composition_that_chooses_the_model(data: dict, message: str) -> None: @@ -1041,7 +1045,9 @@ def test_claude_quirk_refuses_a_tree_that_sets_the_updater_switches(key: str, va """Claude Code copies settings.env over its environment and reads the updater switches as on only for 1, true, yes or on, so a tree must not set them at all: the episode env and reef-claude's client_env own them. Windows matches env names in any case.""" - with pytest.raises(RenderError, match=f"must not set {key} in settings.env"): + with pytest.raises( + RenderError, match=f"on env key {key!r} in claude/settings.json: the episode env and reef-claude set it" + ): render_composition([("config", {"data": {"env": {key: value}}})], get_adapter("claude")) @@ -1061,107 +1067,6 @@ def test_claude_quirk_keeps_deep_link_registration_off(value: object) -> None: render_composition([("config", {"data": {"disableDeepLinkRegistration": value}})], get_adapter("claude")) -def _locked(locked: list[str], defaults: dict[str, object] | None = None) -> AdapterDescriptor: - """The opencode descriptor with its config target replaced and the base renderer, so only the locks act.""" - target = ConfigTarget( - path="opencode/opencode.json", defaults=defaults if defaults is not None else {}, locked=tuple(locked) - ) - return replace(get_adapter("opencode"), config_targets={"primary": target}, renderer=AdapterRenderer) - - -def test_a_locked_key_keeps_its_defaults_value() -> None: - descriptor = _locked(["autoupdate", "nested.key"], {"autoupdate": False, "nested": {"key": 1}}) - repeated = ("config", {"data": {"autoupdate": False, "nested": {"key": 1}}}) - assert json.loads(render_composition([repeated], descriptor)["opencode/opencode.json"])["nested"] == {"key": 1} - with pytest.raises( - RenderError, match=r"must keep autoupdate false in opencode/opencode\.json: the descriptor locks it" - ): - render_composition([("config", {"data": {"autoupdate": True}})], descriptor) - # Booleans only match booleans: a locked false refuses 0, which == false. - with pytest.raises(RenderError, match=r"must keep autoupdate false"): - render_composition([("config", {"data": {"autoupdate": 0}})], descriptor) - # A section the tree clobbers with a non object takes the locked keys below it away, and refuses too. - with pytest.raises(RenderError, match=r"must keep nested\.key 1 .*the merged config does not have it"): - render_composition([("config", {"data": {"nested": "flat"}})], descriptor) - - -def test_a_locked_key_without_a_default_must_not_be_set() -> None: - descriptor = _locked(["small_model"]) - for value in ["reef/served", None, "", {}, []]: - # An empty value is a set one: only the absence the defaults name keeps a lock without a default. - with pytest.raises(RenderError, match=r"must not set small_model in opencode/opencode\.json"): - render_composition([("config", {"data": {"small_model": value}})], descriptor) - assert "small_model" not in render_composition([], descriptor)["opencode/opencode.json"] - - -def test_a_locked_star_matches_every_key_of_an_object_and_every_item_of_a_list() -> None: - descriptor = _locked(["agents.*.model"], {"agents": {"build": {"model": "reef/served"}}}) - # An agent the tree adds under any name may not choose a model either. - with pytest.raises(RenderError, match=r"must not set agents\.reviewer\.model"): - render_composition([("config", {"data": {"agents": {"reviewer": {"model": "m"}}}})], descriptor) - with pytest.raises(RenderError, match=r"must keep agents\.build\.model .*the descriptor locks it"): - render_composition([("config", {"data": {"agents": {"build": {"model": "m"}}}})], descriptor) - add = render_composition([("config", {"data": {"agents": {"reviewer": {"tools": ["read"]}}}})], descriptor) - assert json.loads(add["opencode/opencode.json"])["agents"]["reviewer"] == {"tools": ["read"]} - listed = _locked(["tasks.*.model"]) - with pytest.raises(RenderError, match=r"must not set tasks\.0\.model"): - render_composition([("config", {"data": {"tasks": [{"model": "m"}]}})], listed) - keep = render_composition([("config", {"data": {"tasks": [{"name": "a"}]}})], listed) - assert json.loads(keep["opencode/opencode.json"])["tasks"] == [{"name": "a"}] - - -def test_a_locked_null_keeps_only_null() -> None: - descriptor = _locked(["share"], {"share": None}) - assert "share" in json.loads(render_composition([], descriptor)["opencode/opencode.json"]) - with pytest.raises(RenderError, match=r"must keep share null in opencode/opencode\.json"): - render_composition([("config", {"data": {"share": "disabled"}})], descriptor) - - -def _locked_descriptor_yaml(tmp_path: Path, locked: object) -> Path: - data = yaml.safe_load((Path(reef.harness.adapters.__file__).parent / "opencode" / "descriptor.yaml").read_text()) - if locked is None: - data["files"]["config"]["primary"].pop("locked", None) - else: - data["files"]["config"]["primary"]["locked"] = locked - target = tmp_path / "descriptor.yaml" - target.write_text(yaml.safe_dump(data), encoding="utf-8") - return target - - -@pytest.mark.parametrize( - ("locked", "message"), - [ - pytest.param(["autoupdate..x"], "dot separated key names", id="empty-segment"), - pytest.param([".autoupdate"], "dot separated key names", id="leading-dot"), - pytest.param(["autoupdate", 5], "list of non-empty strings", id="not-a-string"), - pytest.param(["model"], r"locks 'model', which the \w+ model binding writes \(model\)", id="binding-key"), - pytest.param( - ["provider.reef.options"], - r"locks 'provider\.reef\.options', which the \w+ model binding writes", - id="binding-parent", - ), - pytest.param(["provider.*"], r"locks 'provider\.\*'", id="binding-star"), - ], -) -def test_locked_paths_are_validated_at_load(tmp_path: Path, locked: object, message: str) -> None: - with pytest.raises(DescriptorError, match=message): - load_descriptor(_locked_descriptor_yaml(tmp_path, locked)) - - -def test_a_star_lock_below_the_binding_is_refused(tmp_path: Path) -> None: - # `agent.*.model` and the binding's `model` are different keys: siblings, no overlap, so it loads and holds. - descriptor = load_descriptor(_locked_descriptor_yaml(tmp_path, ["autoupdate", "agent.*.model"])) - assert descriptor.config_targets["primary"].locked == ("autoupdate", "agent.*.model") - with pytest.raises(RenderError, match="must keep autoupdate false"): - render_composition([("config", {"data": {"autoupdate": True}})], descriptor) - - -def test_a_descriptor_without_locked_keys_loads_and_renders_as_before(tmp_path: Path) -> None: - descriptor = load_descriptor(_locked_descriptor_yaml(tmp_path, None)) - assert descriptor.config_targets["primary"].locked == () - assert render_composition([], descriptor) == render_composition([], get_adapter("opencode")) - - def test_bundled_adapters_are_discoverable() -> None: assert set(available_adapters()) >= {"claude", "codex", "dsh", "opencode", "pi"} diff --git a/tests/reef_service/test_terminus_agent.py b/tests/reef_service/test_terminus_agent.py index 24ee37a60..df7e909f7 100644 --- a/tests/reef_service/test_terminus_agent.py +++ b/tests/reef_service/test_terminus_agent.py @@ -23,7 +23,7 @@ import pytest from reef.harness.adapters import get_adapter -from reef.harness.adapters.terminus.quirks import _ALLOWED_KNOBS, _BINDING_KNOBS +from reef.harness.adapters.matchers import Each, IsIn, Key from reef.harness.episodes.model_binding import ModelBinding from reef.harness.runners.terminus import instruction_paths, skill_roots from reef.harness.runners.terminus.runner import AGENT_NAME, agent_spec @@ -141,9 +141,13 @@ def test_every_admitted_knob_is_a_real_terminus_2_argument() -> None: from harbor.agents.terminus_2.terminus_2 import Terminus2 + # The descriptor's first check admits the config keys: Each(Key(IsIn([...]))). + admitted = get_adapter("terminus").config_targets["primary"].checks[0].matcher + assert isinstance(admitted, Each) and isinstance(admitted.matcher, Key) + assert isinstance(admitted.matcher.matcher, IsIn) parameters = set(inspect.signature(Terminus2.__init__).parameters) - unknown = sorted((_ALLOWED_KNOBS | _BINDING_KNOBS) - parameters) - assert unknown == [], f"the quirk admits arguments Terminus 2 does not take: {unknown}" + unknown = sorted(set(admitted.matcher.matcher.choices) - parameters) + assert unknown == [], f"the descriptor admits arguments Terminus 2 does not take: {unknown}" #: Provider fields a tree may pass through llm_call_kwargs, for example OpenRouter's routing and sampling fields; its From 34dec144bf474545311c052e0fa7f4fb4f2be585 Mon Sep 17 00:00:00 2001 From: Xuezhi Yan Date: Wed, 14 Oct 2026 13:19:22 -0400 Subject: [PATCH 2/8] docs(harness): describe descriptor checks as node checks Present checks as rules on tree nodes, with config as the node kind that takes them: config nodes merge into a target, so its checks match the merged target. Point the claude, hermes and codex text at the descriptor's checks instead of the quirks that no longer hold those defaults. Co-Authored-By: Claude Opus 5.5 --- docs/developer-guide/harness-adapters.rst | 66 +++++++++++++---------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/docs/developer-guide/harness-adapters.rst b/docs/developer-guide/harness-adapters.rst index cf24e43bb..7dbcd0b60 100644 --- a/docs/developer-guide/harness-adapters.rst +++ b/docs/developer-guide/harness-adapters.rst @@ -79,8 +79,8 @@ whatever the adapter declares; for example: - An entry whose kind the descriptor gives no path, such as ``code_extension`` under ``native``. - A ``config`` entry naming a target the descriptor does not declare. -- A merged config that fails one of its target's ``checks`` (see - `Config checks`_). The checks run before the adapter's own steps, so no +- A tree whose nodes fail one of the ``checks`` the descriptor declares (see + `Node checks`_). The checks run before the adapter's own steps, so no adapter can skip them. - A ``native_graph`` or ``native_agent`` naming a tool, skill, agent, or graph that no entry defines, or agents that delegate in a cycle. @@ -91,7 +91,7 @@ What the adapter owns An adapter is configured mostly via declaration. ``descriptor.yaml`` supplies the path template for each tree node kind, the named config targets, the -defaults merged under every tree, and the checks the merged config must pass +defaults merged under every tree, and the checks the tree's nodes must pass (``checks``). An adapter whose agent differs only in where files go needs no code at all, although they must comply with the following rules: @@ -100,12 +100,19 @@ rules: - ``files.rules`` and ``files.skill`` are required. - every named template must contain ``{name}``. -Config checks -^^^^^^^^^^^^^ +Node checks +^^^^^^^^^^^ -A config target's ``checks`` state what its merged config must keep or must +A descriptor's ``checks`` state what the nodes of a tree must keep or must not set. Each check has a ``key``, a ``match`` expression that every value at -the key must match, and an optional ``reason`` that a refusal gives: +the key must match, and an optional ``reason`` that a refusal gives. + +``config`` is the node kind that takes checks. Its nodes are fragments: each +one merges into a config target over the target's ``defaults``, and the model +binding merges last when one is applied. So the checks are declared on the +target, ``files.config..checks``, and match the merged target rather +than each node. This is what lets ``IsDefault()`` compare a key with the +target's default: .. code-block:: yaml @@ -125,10 +132,9 @@ the key must match, and an optional ``reason`` that a refusal gives: match: Not(Strip(Lowercase(IsIn(["0", "false", "off", "no", ""])))) reason: benchmark episodes keep telemetry off -The render runs the checks on the merged config, with the model binding -applied when there is one, before any adapter step. Reef owns the matchers; -a descriptor cannot add one. The render refuses a tree whose config fails a -check, for example:: +The render runs each target's checks on the merged target, before any +adapter step. Reef owns the matchers; a descriptor cannot add one. The render +refuses a tree whose nodes fail a check, for example:: claude composition fails the check on env key 'AWS_REGION' in claude/settings.json: Reef's model binding chooses the endpoint, ... @@ -138,11 +144,12 @@ the value, which can be a credential the binding wrote. A check without a ``reason`` gives its match expression instead. **Keys.** ``key`` is required: dot-separated key names, or a list of them -(one check per key). ``.`` names the whole file, for a check on its top-level -key names; a check without a key fails to load. A ``*`` segment is every key of an -object and every item of a list. A ``**`` segment is any number of segments, -zero included. A key the config does not set is absent. A ``*`` or ``**`` -segment visits only the values the config has. +(one check per key). ``.`` names the whole checked value, for a check on its +top-level key names; a check without a key fails to load. A ``*`` segment is +every key of an object and every item of a list. A ``**`` segment is any +number of segments, zero included. A key the checked value does not have is +absent. A ``*`` or ``**`` segment visits only the values the checked value +has. **Matchers.** Write a match expression as nested calls with JSON literals (``true``, ``false`` and ``null`` included). Use ``|-`` in YAML to wrap a long @@ -154,10 +161,10 @@ one over several lines. * - Matcher - Matches * - ``IsDefault()`` - - The value the target's ``defaults`` give the key, or an absent key when - they give none. Only for a key without ``*`` or ``**``. + - For a config target: the value its ``defaults`` give the key, or an + absent key when they give none. Only for a key without ``*`` or ``**``. * - ``IsAbsent()`` - - A key the config does not set. An empty value is a set one. + - A key the checked value does not have. An empty value is a set one. * - ``Eq(v)``, ``IsIn([v, ...])`` - A value equal to ``v``, or to one of the list, with its type: ``false`` matches neither ``0`` nor ``"false"``. @@ -714,9 +721,9 @@ Hermes Agent ~~~~~~~~~~~~ The ``hermes`` adapter runs ``hermes chat -Q --oneshot -q ""`` with -``HERMES_HOME`` relocated. Its ``primary`` target is ``config.yaml``; the -quirks write the merged configuration as YAML. They enforce these defaults -so an episode stays self-contained and makes one request: +``HERMES_HOME`` relocated. Its ``primary`` target is ``config.yaml``, which +the render writes as YAML. The descriptor's checks keep these defaults so an +episode stays self-contained and makes one request: - ``security.tirith_enabled`` disables the terminal scanner download. The scanner would also block a command it cannot resolve, such as @@ -801,11 +808,10 @@ Codex CLI The ``codex`` adapter runs ``codex exec --json`` headless with its user home relocated through ``CODEX_HOME``. Its ``primary`` target is -``config.toml``; the quirks write the merged configuration as TOML. They -enforce these defaults so an episode stays self-contained: the update -check, analytics, feedback, and telemetry are off, and the -``workspace-write`` sandbox has no network. Rendering rejects a tree that -changes them. +``config.toml``, which the render writes as TOML. The descriptor's checks +keep these defaults so an episode stays self-contained: the update check, +analytics, feedback, and telemetry are off, and the ``workspace-write`` +sandbox has no network. Rendering rejects a tree that changes them. Node paths and transformations are: @@ -1330,6 +1336,8 @@ Descriptor fields ``{prompt}`` in ``argv``. - ``files`` maps node kinds to output paths, such as ``skills/{name}/SKILL.md``. ``rules`` and ``skill`` paths are required. + Each ``files.config.`` gives a config file's ``path``, its + ``defaults`` and its ``checks`` (see `Node checks`_). Other kinds are optional; Reef rejects a mutation of a kind with no render path. ``files.tree`` is an optional entries-list path for agents that reconcile the live tree. @@ -1351,8 +1359,8 @@ Descriptor fields ``install`` commands, which would otherwise install the latest release over the person's own ``claude``). The ``claude`` episode ``env`` sets both too. A variable the shell sets wins. Claude Code applies the ``env`` block - of ``settings.json`` over the environment, so the ``claude`` quirks reject - a tree that sets either one there. + of ``settings.json`` over the environment, so the ``claude`` descriptor's + checks reject a tree that sets either one there. - ``client_args`` lists arguments the wrapper puts ahead of the person's own, for a setting the rendered tree must not undo. ``claude`` passes ``--settings '{"disableDeepLinkRegistration":"disable"}'``. Claude Code From 5d8cef536356f0976a0f84531c74ee8ebc457794 Mon Sep 17 00:00:00 2001 From: Xuezhi Yan Date: Wed, 14 Oct 2026 14:56:07 -0400 Subject: [PATCH 3/8] docs(harness): leave the quirk-owned defaults text for a follow-up Restore the claude client_env, hermes and codex text that says the quirks enforce the defaults; correcting it is tracked separately. Co-Authored-By: Claude Opus 5.5 --- docs/developer-guide/harness-adapters.rst | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/developer-guide/harness-adapters.rst b/docs/developer-guide/harness-adapters.rst index 7dbcd0b60..4482afb65 100644 --- a/docs/developer-guide/harness-adapters.rst +++ b/docs/developer-guide/harness-adapters.rst @@ -104,7 +104,7 @@ Node checks ^^^^^^^^^^^ A descriptor's ``checks`` state what the nodes of a tree must keep or must -not set. Each check has a ``key``, a ``match`` expression that every value at +not set during evolution. Each check has a ``key``, a ``match`` expression that every value at the key must match, and an optional ``reason`` that a refusal gives. ``config`` is the node kind that takes checks. Its nodes are fragments: each @@ -721,9 +721,9 @@ Hermes Agent ~~~~~~~~~~~~ The ``hermes`` adapter runs ``hermes chat -Q --oneshot -q ""`` with -``HERMES_HOME`` relocated. Its ``primary`` target is ``config.yaml``, which -the render writes as YAML. The descriptor's checks keep these defaults so an -episode stays self-contained and makes one request: +``HERMES_HOME`` relocated. Its ``primary`` target is ``config.yaml``; the +quirks write the merged configuration as YAML. They enforce these defaults +so an episode stays self-contained and makes one request: - ``security.tirith_enabled`` disables the terminal scanner download. The scanner would also block a command it cannot resolve, such as @@ -808,10 +808,11 @@ Codex CLI The ``codex`` adapter runs ``codex exec --json`` headless with its user home relocated through ``CODEX_HOME``. Its ``primary`` target is -``config.toml``, which the render writes as TOML. The descriptor's checks -keep these defaults so an episode stays self-contained: the update check, -analytics, feedback, and telemetry are off, and the ``workspace-write`` -sandbox has no network. Rendering rejects a tree that changes them. +``config.toml``; the quirks write the merged configuration as TOML. They +enforce these defaults so an episode stays self-contained: the update +check, analytics, feedback, and telemetry are off, and the +``workspace-write`` sandbox has no network. Rendering rejects a tree that +changes them. Node paths and transformations are: @@ -1359,8 +1360,8 @@ Descriptor fields ``install`` commands, which would otherwise install the latest release over the person's own ``claude``). The ``claude`` episode ``env`` sets both too. A variable the shell sets wins. Claude Code applies the ``env`` block - of ``settings.json`` over the environment, so the ``claude`` descriptor's - checks reject a tree that sets either one there. + of ``settings.json`` over the environment, so the ``claude`` quirks reject + a tree that sets either one there. - ``client_args`` lists arguments the wrapper puts ahead of the person's own, for a setting the rendered tree must not undo. ``claude`` passes ``--settings '{"disableDeepLinkRegistration":"disable"}'``. Claude Code From 9b6a1e9f5ee1f066ef0cd89c55ed65920643546a Mon Sep 17 00:00:00 2001 From: Xuezhi Yan Date: Wed, 14 Oct 2026 14:57:12 -0400 Subject: [PATCH 4/8] docs(harness): point the claude and codex defaults at the descriptor's checks This PR moved the claude updater-switch refusal and the codex defaults from the quirks into descriptor checks, so the guide now says so. Co-Authored-By: Claude Opus 5.5 --- docs/developer-guide/harness-adapters.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/developer-guide/harness-adapters.rst b/docs/developer-guide/harness-adapters.rst index 4482afb65..13bc276e5 100644 --- a/docs/developer-guide/harness-adapters.rst +++ b/docs/developer-guide/harness-adapters.rst @@ -808,9 +808,9 @@ Codex CLI The ``codex`` adapter runs ``codex exec --json`` headless with its user home relocated through ``CODEX_HOME``. Its ``primary`` target is -``config.toml``; the quirks write the merged configuration as TOML. They -enforce these defaults so an episode stays self-contained: the update -check, analytics, feedback, and telemetry are off, and the +``config.toml``; the quirks write the merged configuration as TOML. The +descriptor's checks keep these defaults so an episode stays self-contained: +the update check, analytics, feedback, and telemetry are off, and the ``workspace-write`` sandbox has no network. Rendering rejects a tree that changes them. @@ -1360,8 +1360,8 @@ Descriptor fields ``install`` commands, which would otherwise install the latest release over the person's own ``claude``). The ``claude`` episode ``env`` sets both too. A variable the shell sets wins. Claude Code applies the ``env`` block - of ``settings.json`` over the environment, so the ``claude`` quirks reject - a tree that sets either one there. + of ``settings.json`` over the environment, so the ``claude`` descriptor's + checks reject a tree that sets either one there. - ``client_args`` lists arguments the wrapper puts ahead of the person's own, for a setting the rendered tree must not undo. ``claude`` passes ``--settings '{"disableDeepLinkRegistration":"disable"}'``. Claude Code From 954d7099daecb6ff4a0d4fb05df72f2bece50d4d Mon Sep 17 00:00:00 2001 From: Xuezhi Yan Date: Wed, 14 Oct 2026 15:07:50 -0400 Subject: [PATCH 5/8] docs(harness): say the render writes hermes and codex config files The render writes each config file in the format its suffix names, and the hermes defaults are kept by the descriptor's checks, not the quirks. Co-Authored-By: Claude Opus 5.5 --- docs/developer-guide/harness-adapters.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/developer-guide/harness-adapters.rst b/docs/developer-guide/harness-adapters.rst index 13bc276e5..a16a89e78 100644 --- a/docs/developer-guide/harness-adapters.rst +++ b/docs/developer-guide/harness-adapters.rst @@ -721,9 +721,9 @@ Hermes Agent ~~~~~~~~~~~~ The ``hermes`` adapter runs ``hermes chat -Q --oneshot -q ""`` with -``HERMES_HOME`` relocated. Its ``primary`` target is ``config.yaml``; the -quirks write the merged configuration as YAML. They enforce these defaults -so an episode stays self-contained and makes one request: +``HERMES_HOME`` relocated. Its ``primary`` target is ``config.yaml``, which +the render writes as YAML. The descriptor's checks keep these defaults so an +episode stays self-contained and makes one request: - ``security.tirith_enabled`` disables the terminal scanner download. The scanner would also block a command it cannot resolve, such as @@ -808,8 +808,8 @@ Codex CLI The ``codex`` adapter runs ``codex exec --json`` headless with its user home relocated through ``CODEX_HOME``. Its ``primary`` target is -``config.toml``; the quirks write the merged configuration as TOML. The -descriptor's checks keep these defaults so an episode stays self-contained: +``config.toml``, which the render writes as TOML. The descriptor's checks +keep these defaults so an episode stays self-contained: the update check, analytics, feedback, and telemetry are off, and the ``workspace-write`` sandbox has no network. Rendering rejects a tree that changes them. From 366126c72fd6c64c41bf04e69f009537a077db0d Mon Sep 17 00:00:00 2001 From: Xuezhi Yan Date: Wed, 14 Oct 2026 15:52:49 -0400 Subject: [PATCH 6/8] docs(harness): separate node checks from the renderer steps Give the renderer steps their own section and move node checks after the engine and adapter seams, each with its part of the guidance on checks and steps. Wrap the page at 79 columns, turning the adapter grid table into a list-table; one link whose URL alone is longer stays as it is. Co-Authored-By: Claude Opus 5.5 --- docs/developer-guide/harness-adapters.rst | 493 ++++++++++++---------- 1 file changed, 261 insertions(+), 232 deletions(-) diff --git a/docs/developer-guide/harness-adapters.rst b/docs/developer-guide/harness-adapters.rst index a16a89e78..0a7965760 100644 --- a/docs/developer-guide/harness-adapters.rst +++ b/docs/developer-guide/harness-adapters.rst @@ -11,32 +11,40 @@ Terminus 2 agent through a Reef runner. With ``native``, the tree can change the agent's tools (``native_tool``) and its responses to loop events (``native_hook``). -+--------------+------------------------------------------------------------+-------------------------------------------+ -| Adapter | Config targets | Install pin | -+==============+============================================================+===========================================+ -| ``pi`` | ``primary`` -> ``pi-agent/settings.json``, | npm ``@earendil-works/pi-coding-agent`` | -| | ``models`` -> ``pi-agent/models.json`` | 0.84.2 | -+--------------+------------------------------------------------------------+-------------------------------------------+ -| ``opencode`` | ``primary`` -> ``opencode/opencode.json`` | npm ``opencode-ai`` 1.18.18 | -+--------------+------------------------------------------------------------+-------------------------------------------+ -| ``claude`` | ``primary`` -> ``claude/settings.json`` | npm ``@anthropic-ai/claude-code`` 2.1.257 | -+--------------+------------------------------------------------------------+-------------------------------------------+ -| ``codex`` | ``primary`` -> ``codex/config.toml`` | npm ``@openai/codex`` 0.153.4 | -+--------------+------------------------------------------------------------+-------------------------------------------+ -| ``dsh`` | ``primary`` -> ``dsh/profiles/headless/cordis.patch.yml``, | npm ``@deepseek-ai/dsh`` 0.1.2-alpha.5 | -| | ``env`` -> ``dsh/.env``, | | -| | ``web`` -> ``dsh/profiles/web/cordis.patch.yml``, | | -| | ``web_manifest`` -> ``dsh/profiles/web/package.json`` | | -+--------------+------------------------------------------------------------+-------------------------------------------+ -| ``hermes`` | ``primary`` -> ``hermes/config.yaml``, | git ``NousResearch/hermes-agent`` | -| | ``env`` -> ``hermes/.env`` | at ``v2026.8.31`` (0.21.0) | -+--------------+------------------------------------------------------------+-------------------------------------------+ -| ``native`` | ``primary`` -> ``native/config.json``, | none: ``reef-native`` ships with reef | -| | ``models`` -> ``native/models.json`` | | -+--------------+------------------------------------------------------------+-------------------------------------------+ -| ``terminus`` | ``primary`` -> ``terminus/config.json`` | none: ``reef-terminus`` ships with reef, | -| | | reef-eval ships with reef-infra | -+--------------+------------------------------------------------------------+-------------------------------------------+ +.. list-table:: + :header-rows: 1 + + * - Adapter + - Config targets + - Install pin + * - ``pi`` + - ``primary`` -> ``pi-agent/settings.json``, ``models`` -> + ``pi-agent/models.json`` + - npm ``@earendil-works/pi-coding-agent`` 0.84.2 + * - ``opencode`` + - ``primary`` -> ``opencode/opencode.json`` + - npm ``opencode-ai`` 1.18.18 + * - ``claude`` + - ``primary`` -> ``claude/settings.json`` + - npm ``@anthropic-ai/claude-code`` 2.1.257 + * - ``codex`` + - ``primary`` -> ``codex/config.toml`` + - npm ``@openai/codex`` 0.153.4 + * - ``dsh`` + - ``primary`` -> ``dsh/profiles/headless/cordis.patch.yml``, ``env`` -> + ``dsh/.env``, ``web`` -> ``dsh/profiles/web/cordis.patch.yml``, + ``web_manifest`` -> ``dsh/profiles/web/package.json`` + - npm ``@deepseek-ai/dsh`` 0.1.2-alpha.5 + * - ``hermes`` + - ``primary`` -> ``hermes/config.yaml``, ``env`` -> ``hermes/.env`` + - git ``NousResearch/hermes-agent`` at ``v2026.8.31`` (0.21.0) + * - ``native`` + - ``primary`` -> ``native/config.json``, ``models`` -> + ``native/models.json`` + - none: ``reef-native`` ships with reef + * - ``terminus`` + - ``primary`` -> ``terminus/config.json`` + - none: ``reef-terminus`` ships with reef, reef-eval ships with reef-infra Rendering ~~~~~~~~~ @@ -64,8 +72,9 @@ every adapter. No adapter replaces it, and an adapter registered through the bundled one. It turns entries into files by these rules: - Each config target is a JSON object: ``config`` entries deep-merge into it in - tree order, over the defaults the descriptor enforces. Every target is written - even when no entry touches it, so those defaults always reach the agent. + tree order, over the defaults the descriptor enforces. Every target is + written even when no entry touches it, so those defaults always reach the + agent. - All ``rules`` entries concatenate, in tree order, into the one rules file. A tree with no ``rules`` entry renders no rules file. - Each named kind renders one file per entry through the descriptor's path @@ -100,101 +109,15 @@ rules: - ``files.rules`` and ``files.skill`` are required. - every named template must contain ``{name}``. -Node checks -^^^^^^^^^^^ - -A descriptor's ``checks`` state what the nodes of a tree must keep or must -not set during evolution. Each check has a ``key``, a ``match`` expression that every value at -the key must match, and an optional ``reason`` that a refusal gives. - -``config`` is the node kind that takes checks. Its nodes are fragments: each -one merges into a config target over the target's ``defaults``, and the model -binding merges last when one is applied. So the checks are declared on the -target, ``files.config..checks``, and match the merged target rather -than each node. This is what lets ``IsDefault()`` compare a key with the -target's default: - -.. code-block:: yaml - - files: - config: - primary: - path: "claude/settings.json" - defaults: - includeCoAuthoredBy: false - checks: - - key: includeCoAuthoredBy - match: IsDefault() - - key: [model, fallbackModel] - match: IsAbsent() - reason: Reef's model binding chooses the model - - key: env.DISABLE_TELEMETRY - match: Not(Strip(Lowercase(IsIn(["0", "false", "off", "no", ""])))) - reason: benchmark episodes keep telemetry off - -The render runs each target's checks on the merged target, before any -adapter step. Reef owns the matchers; a descriptor cannot add one. The render -refuses a tree whose nodes fail a check, for example:: - - claude composition fails the check on env key 'AWS_REGION' in - claude/settings.json: Reef's model binding chooses the endpoint, ... - -A refusal names the key, and for ``Each`` the failing key or item, but never -the value, which can be a credential the binding wrote. A check without a -``reason`` gives its match expression instead. - -**Keys.** ``key`` is required: dot-separated key names, or a list of them -(one check per key). ``.`` names the whole checked value, for a check on its -top-level key names; a check without a key fails to load. A ``*`` segment is -every key of an object and every item of a list. A ``**`` segment is any -number of segments, zero included. A key the checked value does not have is -absent. A ``*`` or ``**`` segment visits only the values the checked value -has. - -**Matchers.** Write a match expression as nested calls with JSON literals -(``true``, ``false`` and ``null`` included). Use ``|-`` in YAML to wrap a long -one over several lines. - -.. list-table:: - :header-rows: 1 - - * - Matcher - - Matches - * - ``IsDefault()`` - - For a config target: the value its ``defaults`` give the key, or an - absent key when they give none. Only for a key without ``*`` or ``**``. - * - ``IsAbsent()`` - - A key the checked value does not have. An empty value is a set one. - * - ``Eq(v)``, ``IsIn([v, ...])`` - - A value equal to ``v``, or to one of the list, with its type: ``false`` - matches neither ``0`` nor ``"false"``. - * - ``IsObject()``, ``IsInt()``, ``Ge(n)`` - - An object; an integer that is not a boolean; a number at least ``n``. - * - ``StartsWith(s)``, ``EndsWith(s)``, ``HasSubstr(s)``, ``MatchesRegex(s)`` - - Text with that prefix, suffix or substring, or text that the regular - expression matches as a whole. - * - ``Lowercase(m)``, ``Uppercase(m)``, ``Strip(m)`` - - Text that matches ``m`` after the change. - * - ``ParseJson(m)`` - - A string holding JSON whose value matches ``m``. - * - ``Each(m)``, ``Contains(m)`` - - Every entry of an object or item of a list matches ``m``, or at least - one does. ``Each`` also matches an absent key. - * - ``Key(m)``, ``Value(m)`` - - An object entry whose key, or whose value, matches ``m``. Use them - inside ``Each`` and ``Contains``. - * - ``AllOf(m, ...)``, ``AnyOf(m, ...)``, ``Not(m)`` - - Every matcher matches, at least one does, or ``m`` does not. - -Text matchers read a string as it is, and a number, a boolean or null as its -JSON text, so ``Lowercase(Eq("0"))`` matches ``0``. They match no object, list -or absent key. Only ``IsAbsent``, ``Not`` and ``Each`` match an absent key, so -"if it is set, it must be X" is ``AnyOf(IsAbsent(), X)``. +Steps +^^^^^ An adapter may also declare a ``quirks`` module to further customize the -rendering process. The module defines one subclass of ``AdapterRenderer`` -from ``reef.harness.adapters.descriptor``; the loader finds it in the module. -The render calls its steps in this order, and each step is given only its own +rendering process. Before any of its steps run, the render runs the node +checks the descriptor declares, which `Node checks`_ explains below. The +module defines one subclass of ``AdapterRenderer`` from +``reef.harness.adapters.descriptor``; the loader finds it in the module. The +render calls its steps in this order, and each step is given only its own files: .. list-table:: @@ -235,10 +158,9 @@ gets the same values from ``skill_frontmatter(path, text)``. .. important:: - Write a rule on config values as a config check, not as a step: a check is - declared beside the defaults it protects and needs no code. - - Prefer the narrow steps over ``finalize_render``. A ``process_*`` step sees + Write a rule on config values as a node check, not as a step (see + `Node checks`_). When you need to customize the tree processing, prefer + the narrow steps over ``finalize_render``. A ``process_*`` step sees one file of one kind, and ``check_model_route`` only reads, so a reader can tell what each one may change and a reviewer can check it in isolation. ``finalize_render`` sees and may rewrite every file, including config files @@ -280,12 +202,12 @@ Examples: ``code_extension`` there too; refusals like this far outnumber the engine's own. -A quirks module written before ``AdapterRenderer`` may still define a -module-level ``finalize_render(files)`` instead. This is deprecated and loading -such a module warns: it runs in place of the ``finalize_render`` step. Its -config files arrive in the format their suffix names, so a ``.toml``, -``.yaml`` or ``.env`` file is no longer JSON. Define an ``AdapterRenderer`` -subclass instead. +*Backward compatibility*: A quirks module written before ``AdapterRenderer`` +may still define a module-level ``finalize_render(files)`` instead. This is +deprecated and loading such a module warns: it runs in place of the +``finalize_render`` step. Its config files arrive in the format their suffix +names, so a ``.toml``, ``.yaml`` or ``.env`` file is no longer JSON. Define an +``AdapterRenderer`` subclass instead. Note that ``native`` declares no ``quirks`` and runs on the engine's rules alone. @@ -301,6 +223,106 @@ Seaming engine and adapter together - One rules file is mandatory. An agent reading rules from several needs its quirks to split the concatenated text apart again. +Node checks +^^^^^^^^^^^ + +A descriptor's ``checks`` state what the nodes of a tree must keep or must +not set. This is particularly useful during harness evolution. Each check has a +``key``, a ``match`` expression that every value at the key must match, and +an optional ``reason`` that a refusal gives. + +Adapter authors should use ``checks`` rather than custom steps to enforce rules +on configuration values: a check is declared beside the defaults it protects, +and it is more discoverable and maintainable than a snippet of code in a corner +of ``quirks.py``. + +``config`` is the node kind that takes checks right now. Its nodes are +fragments: each one merges into a config target over the target's +``defaults``, and the model binding merges last when one is applied. So the +checks are declared on the target, ``files.config..checks``, and match +the merged target rather than each node. This is what lets ``IsDefault()`` +compare a key with the target's default: + +.. code-block:: yaml + + files: + config: + primary: + path: "claude/settings.json" + defaults: + includeCoAuthoredBy: false + checks: + - key: includeCoAuthoredBy + match: IsDefault() + - key: [model, fallbackModel] + match: IsAbsent() + reason: Reef's model binding chooses the model + - key: env.DISABLE_TELEMETRY + match: |- + Not(Strip(Lowercase(IsIn(["0", "false", "off", "no", ""])))) + reason: benchmark episodes keep telemetry off + +The render runs each target's checks on the merged target, before any adapter +step. Reef owns the matchers. If the existing ones are not enough, consider +contributing a matcher to Reef instead of overriding a step. The render refuses +a tree whose nodes fail a check, for example:: + + claude composition fails the check on env key 'AWS_REGION' in + claude/settings.json: Reef's model binding chooses the endpoint, ... + +A refusal names the key, and for ``Each`` the failing key or item, but never +the value, which can be a credential the binding wrote. A check without a +``reason`` gives its match expression instead. + +**Keys.** ``key`` is required: dot-separated key names, or a list of them +(one check per key). ``.`` names the whole checked value, for a check on its +top-level key names; a check without a key fails to load. A ``*`` segment is +every key of an object and every item of a list. A ``**`` segment is any +number of segments, zero included. A key the checked value does not have is +absent. A ``*`` or ``**`` segment visits only the values the checked value +has. + +**Matchers.** Write a match expression as nested calls with JSON literals +(``true``, ``false`` and ``null`` included). Use ``|-`` in YAML to wrap a long +one over several lines. + +.. list-table:: + :header-rows: 1 + + * - Matcher + - Matches + * - ``IsDefault()`` + - For a config target: the value its ``defaults`` give the key, or an + absent key when they give none. Only for a key without ``*`` or ``**``. + * - ``IsAbsent()`` + - A key the checked value does not have. An empty value is a set one. + * - ``Eq(v)``, ``IsIn([v, ...])`` + - A value equal to ``v``, or to one of the list, with its type: ``false`` + matches neither ``0`` nor ``"false"``. + * - ``IsObject()``, ``IsInt()``, ``Ge(n)`` + - An object; an integer that is not a boolean; a number at least ``n``. + * - ``StartsWith(s)``, ``EndsWith(s)``, ``HasSubstr(s)``, + ``MatchesRegex(s)`` + - Text with that prefix, suffix or substring, or text that the regular + expression matches as a whole. + * - ``Lowercase(m)``, ``Uppercase(m)``, ``Strip(m)`` + - Text that matches ``m`` after the change. + * - ``ParseJson(m)`` + - A string holding JSON whose value matches ``m``. + * - ``Each(m)``, ``Contains(m)`` + - Every entry of an object or item of a list matches ``m``, or at least + one does. ``Each`` also matches an absent key. + * - ``Key(m)``, ``Value(m)`` + - An object entry whose key, or whose value, matches ``m``. Use them + inside ``Each`` and ``Contains``. + * - ``AllOf(m, ...)``, ``AnyOf(m, ...)``, ``Not(m)`` + - Every matcher matches, at least one does, or ``m`` does not. + +Text matchers read a string as it is, and a number, a boolean or null as its +JSON text, so ``Lowercase(Eq("0"))`` matches ``0``. They match no object, list +or absent key. Only ``IsAbsent``, ``Not`` and ``Each`` match an absent key, so +"if it is set, it must be X" is ``AnyOf(IsAbsent(), X)``. + Misc ^^^^ @@ -311,8 +333,8 @@ one of the two becomes a release. Restrictions that depend on how an episode runs, rather than on what a tree contains, are checked at launch instead. Terminus's requirement for remote E2B -tasks is one: a tree carrying a ``code_extension`` renders, then fails to launch -under the wrong executor. +tasks is one: a tree carrying a ``code_extension`` renders, then fails to +launch under the wrong executor. Codex model metadata ~~~~~~~~~~~~~~~~~~~~ @@ -335,8 +357,8 @@ Endpoints without these fields can provide explicit values under context_window: 64000 reasoning: false -A Python ``ModelBinding`` accepts ``metadata=ModelMetadata(64000, False)``; -a named ``evolution.models`` binding accepts the same fields under ``metadata``. +A Python ``ModelBinding`` accepts ``metadata=ModelMetadata(64000, False)``; a +named ``evolution.models`` binding accepts the same fields under ``metadata``. Values in ``evolution.model_metadata`` take precedence over discovery. Missing or unavailable metadata leaves Codex's existing fallback behavior intact; Reef does not invent a model's context window. Restart the service to refresh cached @@ -348,30 +370,30 @@ capabilities after relocation. Codex retains its own context safety margin and compaction policy. Custom-model entries keep the pinned Codex unknown-model instructions, standard shell tools, and low/medium/high effort when reasoning is supported. The catalog includes all bundled models, so selecting another -model with ``--model`` or the interactive picker retains its native instructions -and tools, including ``gpt-6-astra``. Provider-prefixed names such as -``openai/gpt-6-astra`` retain the same native configuration. -Explicit and discovered capabilities override the selected model's -context window and reasoning support even when its name matches a bundled model; -other native fields and supported reasoning levels remain intact. +model with ``--model`` or the interactive picker retains its native +instructions and tools, including ``gpt-6-astra``. Provider-prefixed names such +as ``openai/gpt-6-astra`` retain the same native configuration. Explicit and +discovered capabilities override the selected model's context window and +reasoning support even when its name matches a bundled model; other native +fields and supported reasoning levels remain intact. ``codex/default_instructions.md`` records the effective unknown-model prompt -exported with this adapter configuration. It is derived from -`OpenAI Codex rust-v0.153.4 `__, +exported with this adapter configuration. It is derived from `OpenAI Codex +rust-v0.153.4 +`__, under the Apache-2.0 license, with SHA-256 -``3b08633fa672906666659d764864dfda1d7af5b5111ea5817c8f46e5de4e1a8d``. -Codex removes instructions for tools disabled by the adapter from the source -prompt. A real-binary regression compares the resulting request instructions -with and without metadata. ``codex/bundled_models.json`` is configuration data -exported from the same Apache-2.0-licensed CLI with -``codex debug models --bundled``. Its SHA-256 is -``661ed96cf0542e8ee117f1ddfd879f416cc05dc96dd1955249a13c04a313ba5e``. -Keep both resources synchronized with the Codex install pin; the real-binary -regression compares the complete bundled catalog, including instructions and -tools. Rendering reads these packaged resources without starting Codex or -making a network request. -Tree rules and skills are still added normally. Tree entries may supply the -same capability fields through the ``models`` config target, but may not set -an arbitrary catalog path or inject native Codex model fields. +``3b08633fa672906666659d764864dfda1d7af5b5111ea5817c8f46e5de4e1a8d``. Codex +removes instructions for tools disabled by the adapter from the source prompt. +A real-binary regression compares the resulting request instructions with and +without metadata. ``codex/bundled_models.json`` is configuration data exported +from the same Apache-2.0-licensed CLI with ``codex debug models --bundled``. +Its SHA-256 is +``661ed96cf0542e8ee117f1ddfd879f416cc05dc96dd1955249a13c04a313ba5e``. Keep both +resources synchronized with the Codex install pin; the real-binary regression +compares the complete bundled catalog, including instructions and tools. +Rendering reads these packaged resources without starting Codex or making a +network request. Tree rules and skills are still added normally. Tree entries +may supply the same capability fields through the ``models`` config target, but +may not set an arbitrary catalog path or inject native Codex model fields. Terminus 2 ~~~~~~~~~~ @@ -589,7 +611,8 @@ Command fields have these constraints: - Strings when present. * - ``agent`` - A string naming an enabled tree agent or a built-in agent: ``build``, - ``plan``, ``general``, ``explore``, ``title``, ``summary``, or ``compaction``. + ``plan``, ``general``, ``explore``, ``title``, ``summary``, or + ``compaction``. * - ``subtask`` - A boolean when present. * - ``model`` @@ -598,10 +621,10 @@ Command fields have these constraints: Tree agents are configured under ``agent`` (or the older ``mode``). Their ``disable`` and ``hidden`` fields are booleans, and ``mode`` is ``subagent``, ``primary``, or ``all``. An agent may not set a different ``name``. -``default_agent`` must name an enabled, non-hidden agent that is not a subagent. -Without ``default_agent``, keep at least one agent meeting those conditions. -Reef rejects invalid configurations before opencode can fail to start or load -a command. +``default_agent`` must name an enabled, non-hidden agent that is not a +subagent. Without ``default_agent``, keep at least one agent meeting those +conditions. Reef rejects invalid configurations before opencode can fail to +start or load a command. Configuration and model binding ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -635,8 +658,8 @@ After installing a dsh harness, start its browser interface: reef-dsh web Stop it with Ctrl-C. The wrapper waits for dsh to exit, removes the temporary -session copy, and exits with dsh's status (130 after Ctrl-C), without a traceback. -The headless profile is used for evaluation episodes. +session copy, and exits with dsh's status (130 after Ctrl-C), without a +traceback. The headless profile is used for evaluation episodes. Rendered files and profiles ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -666,13 +689,14 @@ those settings. Node paths and transformations are: The model binding uses an ``llm-pi-ai`` route. Its ``apiKeyEnv`` names the key supplied through the ``env`` config target, dsh's ``.env`` launch layer. -The ``web`` target is that profile's patch layer. It carries the same -defaults as the headless patch, is checked the same way, and gets the model -binding too, so the wrapper points it at its proxy. The upstream web template compresses session logs. Reef disables compression -in both profiles so they can share the sessions root; a compressed profile -would reject the headless profile's plain logs. The ``web_manifest`` target is the profile's -``package.json`` with ``patchReload: startup``: the manifest dsh writes for -a new web profile sets ``live``, and with it ``dsh web`` exits at start. +The ``web`` target is that profile's patch layer. It carries the same defaults +as the headless patch, is checked the same way, and gets the model binding too, +so the wrapper points it at its proxy. The upstream web template compresses +session logs. Reef disables compression in both profiles so they can share the +sessions root; a compressed profile would reject the headless profile's plain +logs. The ``web_manifest`` target is the profile's ``package.json`` with +``patchReload: startup``: the manifest dsh writes for a new web profile sets +``live``, and with it ``dsh web`` exits at start. A ``code_extension`` renders once; the web patch inserts it from the headless profile's directory (``../headless/extensions/.mjs``). A @@ -697,7 +721,8 @@ For an ``agent_command`` named ``review``, a minimal body is: Review the changes and report correctness problems. The adapter enforces ``disable-model-invocation: true``. The command remains -available to the user as ``/review`` and is not available for the model to invoke. +available to the user as ``/review`` and is not available for the model to +invoke. Parsing and normalization ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -1418,9 +1443,10 @@ Model binding checks ~~~~~~~~~~~~~~~~~~~~ These checks restrict the configuration rendered from a harness tree for -``claude``, ``dsh``, ``hermes``, ``pi``, and ``terminus``. They are not request-time -model authorization: the proxy forwards ``model`` and ``models`` as sent, so a -tool or plugin that constructs its own request can still name another model. +``claude``, ``dsh``, ``hermes``, ``pi``, and ``terminus``. They are not +request-time model authorization: the proxy forwards ``model`` and ``models`` +as sent, so a tool or plugin that constructs its own request can still name +another model. For rendered configuration, Reef's model binding supplies the endpoint, model, and credential after the tree and replaces every value it writes. A tree may @@ -1433,9 +1459,9 @@ Claude Code routing Rendering rejects these settings: -- In ``settings.json`` ``env``: ``ANTHROPIC_`` variables, cloud provider switches - and credentials (including Bedrock, Vertex, and Foundry), proxies, endpoints, - and model names. Names are matched without case, as on Windows. +- In ``settings.json`` ``env``: ``ANTHROPIC_`` variables, cloud provider + switches and credentials (including Bedrock, Vertex, and Foundry), proxies, + endpoints, and model names. Names are matched without case, as on Windows. - Model choices such as ``model``, ``fallbackModel``, ``availableModels``, ``modelOverrides``, and ``advisorModel``. - Credential helpers: ``apiKeyHelper``, ``awsAuthRefresh``, @@ -1451,13 +1477,14 @@ Hermes routing Rendering rejects alternative routes and model choices: -- ``providers``, ``custom_providers``, ``fallback_model``, ``fallback_providers``, - ``moa`` presets (``presets``, or the older ``reference_models`` and - ``aggregator``), and ``auxiliary.openrouter_model``. -- Aliases (``model_aliases``, ``model.aliases``) and endpoint, model, or credential - fields such as ``model.model``, ``model.name``, ``model.api_base``, and - ``model.key_env``. Top-level ``provider``, ``base_url``, and ``api_base`` - are included because Hermes moves them into ``model``. +- ``providers``, ``custom_providers``, ``fallback_model``, + ``fallback_providers``, ``moa`` presets (``presets``, or the older + ``reference_models`` and ``aggregator``), and ``auxiliary.openrouter_model``. +- Aliases (``model_aliases``, ``model.aliases``) and endpoint, model, or + credential fields such as ``model.model``, ``model.name``, + ``model.api_base``, and ``model.key_env``. Top-level ``provider``, + ``base_url``, and ``api_base`` are included because Hermes moves them into + ``model``. - ``model.api_mode`` and ``model.openai_runtime``. These can bypass the custom provider: ``bedrock_converse`` calls AWS Bedrock, while ``codex_app_server`` delegates to a ``codex app-server`` subprocess. @@ -1495,8 +1522,8 @@ The latter would forward all calls through another host. The binding writes Terminus routing ^^^^^^^^^^^^^^^^ -Rendering rejects ``llm_kwargs`` fields that the binding does not write. -It also rejects ``llm_call_kwargs`` arguments that select an endpoint, provider, +Rendering rejects ``llm_kwargs`` fields that the binding does not write. It +also rejects ``llm_call_kwargs`` arguments that select an endpoint, provider, credential, model, fallback, or logging callback, including ``base_url``, ``api_base``, ``custom_llm_provider``, ``model``, and ``fallbacks``. The Terminus quirk lists the recognized routing arguments. The binding writes @@ -1525,18 +1552,18 @@ For installation and recovery commands, see `Install the published tree Temporary session copies ^^^^^^^^^^^^^^^^^^^^^^^^ -Each session runs the agent on a temporary copy of the tree whose model binding points at the -wrapper's capture proxy and holds the token. The wrapper makes that copy in -``$XDG_CACHE_HOME/reef-harness/sessions`` (``~/.cache/reef-harness/sessions`` -by default on Linux and macOS; on Windows the wrapper runs under WSL, where -the same path applies), readable by you alone, and never in -``$TMPDIR`` or ``/tmp``: a command in the Codex or dsh sandbox can write -there, and Codex reads its config and rules from that copy again when you -start a new thread with ``/new``. When the terminal closes (SIGHUP) or the -wrapper gets SIGTERM, it passes the signal to the agent, waits for the -agent to exit, removes the temp copy, keeps the receipts, and exits with -128 plus the signal number. ``reef-pi update`` passes the install script to -``bash`` on its standard input, as ``curl ... | bash`` does, so no copy of +Each session runs the agent on a temporary copy of the tree whose model binding +points at the wrapper's capture proxy and holds the token. The wrapper makes +that copy in ``$XDG_CACHE_HOME/reef-harness/sessions`` +(``~/.cache/reef-harness/sessions`` by default on Linux and macOS; on Windows +the wrapper runs under WSL, where the same path applies), readable by you +alone, and never in ``$TMPDIR`` or ``/tmp``: a command in the Codex or dsh +sandbox can write there, and Codex reads its config and rules from that copy +again when you start a new thread with ``/new``. When the terminal closes +(SIGHUP) or the wrapper gets SIGTERM, it passes the signal to the agent, waits +for the agent to exit, removes the temp copy, keeps the receipts, and exits +with 128 plus the signal number. ``reef-pi update`` passes the install script +to ``bash`` on its standard input, as ``curl ... | bash`` does, so no copy of the script sits in ``$TMPDIR`` while it runs. Install records and validation @@ -1629,15 +1656,15 @@ state again in the tree; what the link pointed at is left as it was. Session inputs and older installations ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -A session gets the files the install wrote and that client state, so a -file added to the tree later is not used, and it gets only the env file -values the release's ``env`` items name. With a record, its PATH starts -with the agent binary's directory and the wrapper's, never the install -root, so a program added there does not run either. ``update``, ``setup``, ``doctor``, -``evolve`` and ``page`` run from your shell reach Reef at the recorded -address, not at the one in the model binding. An install made before Reef -kept this record has none, and neither has one whose record was removed: -its sessions start without the check, and each such start prints `` has no install record`` +A session gets the files the install wrote and that client state, so a file +added to the tree later is not used, and it gets only the env file values the +release's ``env`` items name. With a record, its PATH starts with the agent +binary's directory and the wrapper's, never the install root, so a program +added there does not run either. ``update``, ``setup``, ``doctor``, ``evolve`` +and ``page`` run from your shell reach Reef at the recorded address, not at the +one in the model binding. An install made before Reef kept this record has +none, and neither has one whose record was removed: its sessions start without +the check, and each such start prints `` has no install record`` and that ``reef-pi update`` records the files, until the update writes the record. @@ -1657,22 +1684,24 @@ To connect an agent that has no adapter yet: .. steps:: #. The file it reads configuration from becomes a ``files.config`` target. - #. The command line that runs one prompt headless becomes ``binary`` and ``argv``. - #. The path and format of its session log become ``trajectory``. A new format - subclasses ``TrajectoryReader`` - (`reef/harness/episodes/trajectory.py <../../reef/harness/episodes/trajectory.py>`__) and - registers with ``@register_trajectory_reader``. + #. The command line that runs one prompt headless becomes ``binary`` and + ``argv``. + #. The path and format of its session log become ``trajectory``. A new + format subclasses ``TrajectoryReader`` + (`reef/harness/episodes/trajectory.py + <../../reef/harness/episodes/trajectory.py>`__) and registers with + ``@register_trajectory_reader``. #. The files its first boot creates go in ``cleanup_whitelist``, so a fresh episode root is treated as clean. ``dir/**`` tolerates a whole subtree (session storage, ``node_modules``); any other entry is a glob against the root-relative path, so anchor a single file with its full path, like ``pi-agent/auth.json``. A bare directory name matches nothing under it. -`reef/harness/adapters/descriptor.py <../../reef/harness/adapters/descriptor.py>`__ -validates descriptors at load. The bundled adapters under -`reef/harness/adapters/ <../../reef/harness/adapters>`__ provide complete -references. External adapters register through the -``reef.harness_adapters`` entry-point group. +`reef/harness/adapters/descriptor.py +<../../reef/harness/adapters/descriptor.py>`__ validates descriptors at load. +The bundled adapters under `reef/harness/adapters/ +<../../reef/harness/adapters>`__ provide complete references. External adapters +register through the ``reef.harness_adapters`` entry-point group. ``evolution.client_models`` adds model names that an installed client may select. The install script repeats each ``model_binding`` template entry @@ -1696,12 +1725,13 @@ otherwise its ``name``. A prior successful setup check does not establish that the variable is set in the current shell. If a required item has not passed setup, an interactive session with a -``reef-pi`` wrapper asks ``Set up release now?`` and runs the setup -loop below before offering the update. It finds the wrapper through +``reef-pi`` wrapper asks ``Set up release now?`` and runs the setup loop +below before offering the update. It finds the wrapper through ``REEF_HARNESS_WRAPPER`` (exported by ``run_agent``), or, for a tree run -without ``reef-pi``, in ``~/.reef/installs``. Without a wrapper or UI, it prints the unmet items and -``Run reef-pi setup, then start reef-pi again.``. Items still unmet after -setup are reported, and the update offer waits until the next session. +without ``reef-pi``, in ``~/.reef/installs``. Without a wrapper or UI, it +prints the unmet items and ``Run reef-pi setup, then start reef-pi again.``. +Items still unmet after setup are reported, and the update offer waits until +the next session. The update runs ``reef-pi update`` through an available wrapper, or uses the install pipeline. It ends with ``Installed release . Type /reload @@ -1731,15 +1761,14 @@ adapter's command form. Pi harness requests ~~~~~~~~~~~~~~~~~~~ -``evolution.requests: true`` adds two Reef-owned entries to a pi tree: -the ``code_extension`` ``reef-requests`` -(`reef/harness/adapters/pi/requests.ts <../../reef/harness/adapters/pi/requests.ts>`__) -and the ``skill`` ``reef-pi-extension-api`` -(`reef/harness/adapters/pi/pi_extension_api.md -<../../reef/harness/adapters/pi/pi_extension_api.md>`__). The skill is the -pi extension API reference read by the service proposer before it writes -an extension. Under ``PI_OFFLINE`` the extension registers nothing. -Otherwise it registers two commands, two tools, and two event handlers. +``evolution.requests: true`` adds two Reef-owned entries to a pi tree: the +``code_extension`` ``reef-requests`` (`reef/harness/adapters/pi/requests.ts +<../../reef/harness/adapters/pi/requests.ts>`__) and the ``skill`` +``reef-pi-extension-api`` (`reef/harness/adapters/pi/pi_extension_api.md +<../../reef/harness/adapters/pi/pi_extension_api.md>`__). The skill is the pi +extension API reference read by the service proposer before it writes an +extension. Under ``PI_OFFLINE`` the extension registers nothing. Otherwise it +registers two commands, two tools, and two event handlers. ``/reefine `` files a harness-change request. With a UI, it first clarifies the request in the background. The command returns immediately; From 2bd819f77750796745f92918bdd45ff0311368e5 Mon Sep 17 00:00:00 2001 From: Xuezhi Yan Date: Wed, 14 Oct 2026 16:25:00 -0400 Subject: [PATCH 7/8] refactor(harness): keep check details in descriptors, drop locked handling Remove the quirks docstrings and descriptor comments that only restate what the shared checks do; adapter-specific reasons stay beside their checks. Treat a leftover locked field like any other unknown field, and describe the matchers as tree node checks that link to the adapter guide. Co-Authored-By: Claude Opus 5.5 --- reef/harness/adapters/claude/descriptor.yaml | 2 -- reef/harness/adapters/claude/quirks.py | 13 --------- reef/harness/adapters/codex/descriptor.yaml | 7 ++--- reef/harness/adapters/codex/quirks.py | 7 ++--- reef/harness/adapters/descriptor.py | 6 ---- reef/harness/adapters/dsh/descriptor.yaml | 5 ++-- reef/harness/adapters/dsh/quirks.py | 10 +------ reef/harness/adapters/hermes/descriptor.yaml | 2 -- reef/harness/adapters/hermes/quirks.py | 21 ++------------ reef/harness/adapters/matchers.py | 29 +++++-------------- .../harness/adapters/opencode/descriptor.yaml | 2 -- reef/harness/adapters/opencode/quirks.py | 11 ++----- reef/harness/adapters/pi/descriptor.yaml | 2 -- reef/harness/adapters/pi/quirks.py | 7 ----- .../harness/adapters/terminus/descriptor.yaml | 2 -- reef/harness/adapters/terminus/quirks.py | 18 ------------ .../test_harness_config_checks.py | 9 ------ 17 files changed, 21 insertions(+), 132 deletions(-) diff --git a/reef/harness/adapters/claude/descriptor.yaml b/reef/harness/adapters/claude/descriptor.yaml index b3c17019f..59b39e6b5 100644 --- a/reef/harness/adapters/claude/descriptor.yaml +++ b/reef/harness/adapters/claude/descriptor.yaml @@ -85,8 +85,6 @@ files: # Covers a run where the person passes their own --settings, which # replaces the one in client_args. disableDeepLinkRegistration: "disable" - # The shared render refuses a composition that fails one of these - # checks, before any quirks step runs. checks: - key: [includeCoAuthoredBy, disableDeepLinkRegistration] match: IsDefault() diff --git a/reef/harness/adapters/claude/quirks.py b/reef/harness/adapters/claude/quirks.py index b862ec696..57cc2e52d 100644 --- a/reef/harness/adapters/claude/quirks.py +++ b/reef/harness/adapters/claude/quirks.py @@ -6,23 +6,10 @@ The descriptor whitelists those so the episode inverse tolerates them and reports anything else as residue. -The descriptor's checks keep the settings.json keys a tree must keep, refuse -the settings keys that choose the model, a credential helper or a login -method, and hold ``settings.env``: it must not turn the updater switches, -telemetry or non-essential traffic back on, and must not set an env name -Claude Code reads to choose the endpoint, the credential or the model, -beside the three names Reef's model binding writes. The shared render runs -them before this module's steps. - Every model call stays on Reef's model binding, and a ``model`` in the frontmatter of a command or a skill is the tree choosing where calls go. ``check_model_route`` refuses it, reading the frontmatter as Claude Code reads it, which no config check can. - -These checks cover the config the tree renders. Reef's proxy forwards the -``model`` and ``models`` a request sends as they are, so a request that other -code builds with the rendered key, such as a tool the model runs, can still -name another model. """ from __future__ import annotations diff --git a/reef/harness/adapters/codex/descriptor.yaml b/reef/harness/adapters/codex/descriptor.yaml index b83bcfef3..7ed46e9a0 100644 --- a/reef/harness/adapters/codex/descriptor.yaml +++ b/reef/harness/adapters/codex/descriptor.yaml @@ -64,10 +64,9 @@ files: trace_exporter: none sandbox_workspace_write: network_access: false - # The shared render refuses a composition that fails one of these - # checks, before any quirks step runs. web_search may take any value - # Codex reads, because the episode argv pins it disabled and only a - # person's reef-codex session reads the tree's value. + # web_search may take any value Codex reads, because the episode argv + # pins it disabled and only a person's reef-codex session reads the + # tree's value. checks: # The episode argv pins never, and a reef-codex session keeps Codex's # on-request default, so the person approves each command that leaves diff --git a/reef/harness/adapters/codex/quirks.py b/reef/harness/adapters/codex/quirks.py index 316fe95b3..58fabe59e 100644 --- a/reef/harness/adapters/codex/quirks.py +++ b/reef/harness/adapters/codex/quirks.py @@ -1,9 +1,6 @@ -"""Codex adapter quirks: TOML, skill metadata, and safety invariants. +"""Codex adapter quirks: the model catalog, skill metadata, and code extensions. -Reef config nodes are JSON objects for every adapter, while Codex reads user -configuration as TOML. The descriptor's checks hold the benchmark invariants -and the model provider, and the render writes ``config.toml`` as TOML by its -suffix. ``process_config`` turns the model metadata in ``models.json`` into +``process_config`` turns the model metadata in ``models.json`` into Codex's model catalog. Codex skills require ``name`` and ``description`` frontmatter. ``process_skill`` adds it when an evolved skill omits it. An agent_command renders as a skill in the same root, so it gets the same diff --git a/reef/harness/adapters/descriptor.py b/reef/harness/adapters/descriptor.py index ea5e6700f..0c6529b5f 100644 --- a/reef/harness/adapters/descriptor.py +++ b/reef/harness/adapters/descriptor.py @@ -523,12 +523,6 @@ def _parse_config_targets(raw: Any, where: str) -> dict[str, ConfigTarget]: defaults = target.get("defaults", {}) if not isinstance(defaults, Mapping): raise DescriptorError(f"{where} config target {target_name!r} defaults must be an object") - if "locked" in target: - # A lock the loader skipped would leave the key unchecked, so the old field fails loudly. - raise DescriptorError( - f"{where} config target {target_name!r} 'locked' is replaced by 'checks': write a locked key as a " - "check whose match is IsDefault() when the defaults set it, and IsAbsent() when they do not" - ) targets[str(target_name)] = ConfigTarget( path=_require_str(target, "path", f"{where} config target {target_name!r}"), defaults=dict(defaults), diff --git a/reef/harness/adapters/dsh/descriptor.yaml b/reef/harness/adapters/dsh/descriptor.yaml index 589b532fd..e1147fc41 100644 --- a/reef/harness/adapters/dsh/descriptor.yaml +++ b/reef/harness/adapters/dsh/descriptor.yaml @@ -68,9 +68,8 @@ files: disabled: true session-telemetry-otel: disabled: true - # The shared render refuses a composition that fails one of these - # checks, in either profile (the web target repeats them by the - # anchor), before any quirks step runs. + # The web target repeats these checks through the anchor, so both + # profiles are held to them. checks: &patch_checks # The session log stays uncompressed (the reader cannot parse zstd), # and the LLM title call (one extra model request per episode) and diff --git a/reef/harness/adapters/dsh/quirks.py b/reef/harness/adapters/dsh/quirks.py index a8ce9eaa9..8a7955178 100644 --- a/reef/harness/adapters/dsh/quirks.py +++ b/reef/harness/adapters/dsh/quirks.py @@ -28,15 +28,7 @@ too deeply to read, holds any other tag, or is any other value than a mapping is refused at render. Every header is written so that YAML 1.2 reads each value back with its type: a string that YAML 1.1 or 1.2 would -read as a number, a boolean or a null is quoted. - -The descriptor's checks hold both profiles' patch layers before any step -here runs: the keys an episode relies on, a patch entry that is an object, -and every model call on Reef's model binding. The binding writes the -``llm-pi-ai`` route ``reef``, selects it in ``agent-default-model`` and puts -the route's key in ``.env``; another route, DeepSeek's own adapter, a -provider or a model set for another call, a patch entry that names another -package, and a js expression in those plugins are refused there.""" +read as a number, a boolean or a null is quoted.""" from __future__ import annotations diff --git a/reef/harness/adapters/hermes/descriptor.yaml b/reef/harness/adapters/hermes/descriptor.yaml index 884c0b13e..6f33c6518 100644 --- a/reef/harness/adapters/hermes/descriptor.yaml +++ b/reef/harness/adapters/hermes/descriptor.yaml @@ -61,8 +61,6 @@ files: write_json_snapshots: true skills: creation_nudge_interval: 0 - # The shared render refuses a composition that fails one of these - # checks, before any quirks step runs. checks: # Keep an episode hermetic and single request, with its snapshot readable. - key: [ diff --git a/reef/harness/adapters/hermes/quirks.py b/reef/harness/adapters/hermes/quirks.py index 61205e6e8..91572a32d 100644 --- a/reef/harness/adapters/hermes/quirks.py +++ b/reef/harness/adapters/hermes/quirks.py @@ -15,25 +15,8 @@ ``process_command`` give a SKILL.md the node text left bare the ``name`` and ``description`` frontmatter hermes requires, under both skill roots. -The descriptor's checks hold the config before any step here runs: the -scanner download, the session title call, the background review and the -curator that write skills into the tree, and the snapshot the reader -parses stay as the defaults set them, and every model call stays on Reef's -model binding. The binding writes the ``model`` section (a ``custom`` -provider, the endpoint, the model and a literal key), and the checks refuse -the other names hermes reads for a provider, an endpoint, a credential, a -transport, a model or a fallback chain, in ``model``, at the top level, in -the mixture of agents presets, and for an auxiliary task, delegation, cron -or the curator; an auxiliary task may keep ``auto`` or ``main``, which run -it on the main model. A request body a tree passes to the bound endpoint -(``extra_body``, ``delegation.request_overrides``) may not name a model -either. ``process_config`` refuses a ``skills.external_dirs`` that is not a -list of strings, where it adds the commands root. - -These checks cover the config the tree renders. Reef's proxy forwards the -``model`` and ``models`` a request sends as they are, so a request that other -code builds with the rendered key, such as a tool the model runs, can still -name another model. +``process_config`` refuses a ``skills.external_dirs`` that is not a list of +strings, where it adds the commands root. """ from __future__ import annotations diff --git a/reef/harness/adapters/matchers.py b/reef/harness/adapters/matchers.py index 31e48be63..d9bb29eca 100644 --- a/reef/harness/adapters/matchers.py +++ b/reef/harness/adapters/matchers.py @@ -1,23 +1,10 @@ -"""Matchers for the config checks a descriptor declares, after GoogleTest's matchers. - -A config target in ``descriptor.yaml`` lists ``checks``. Each check names a -``key`` of the merged config file and a ``match`` expression that every value -at that key must match, such as ``Not(Lowercase(IsIn(["0", "false"])))``. The -render refuses a tree whose config fails a check, before any adapter step -runs. Reef owns the matcher vocabulary; a descriptor cannot add a matcher. - -A key is dot separated key names, or ``.`` for the whole file. A ``*`` segment is every key of an object -and every item of a list, and a ``**`` segment is any number of segments, -zero included. A key the config does not set has the value ``ABSENT``, which -only ``IsAbsent()``, ``Not(...)`` and ``Each(...)`` match. A ``*`` or ``**`` -segment visits only values the config has, so a check below one passes when -there is nothing to visit. - -Text matchers (``StartsWith``, ``EndsWith``, ``HasSubstr``, ``MatchesRegex`` -and the ``Lowercase``, ``Uppercase`` and ``Strip`` transforms) read a scalar -as text: a string as it is, and a number, a boolean or null as its JSON text. -They match no object, list or ``ABSENT``. ``Eq`` and ``IsIn`` compare values -with their types, so ``false`` matches neither ``0`` nor ``"false"``. +"""Matchers for the tree node checks a descriptor declares, borrowed from GoogleTest's matchers. + +Each check names a ``key`` of the checked value and a ``match`` expression that +every value at that key must match, such as +``Not(Lowercase(IsIn(["0", "false"])))``. The key syntax and what each matcher +matches are the descriptor contract described in "Node checks" in +``docs/developer-guide/harness-adapters.rst``. A match expression is parsed with Python's expression grammar, restricted to matcher calls and JSON literals (``true``, ``false`` and ``null`` included), so @@ -331,7 +318,7 @@ def matches(self, value: object) -> bool: @dataclass(frozen=True) class KeyDefault: - """The value the target's defaults give a check's key, ``ABSENT`` when they do not set it.""" + """The value a check's key has in the defaults, ``ABSENT`` when they do not set it.""" value: object diff --git a/reef/harness/adapters/opencode/descriptor.yaml b/reef/harness/adapters/opencode/descriptor.yaml index a337f808a..8222435eb 100644 --- a/reef/harness/adapters/opencode/descriptor.yaml +++ b/reef/harness/adapters/opencode/descriptor.yaml @@ -45,8 +45,6 @@ files: enabled_providers: ["reef"] permission: "*": "allow" - # The shared render refuses a composition that fails one of these - # checks, before any quirks step runs. checks: # Keep a benchmark episode hermetic and on the binding's provider only. - key: [autoupdate, share, enabled_providers] diff --git a/reef/harness/adapters/opencode/quirks.py b/reef/harness/adapters/opencode/quirks.py index fd5e1ab06..15fe43a7f 100644 --- a/reef/harness/adapters/opencode/quirks.py +++ b/reef/harness/adapters/opencode/quirks.py @@ -1,4 +1,4 @@ -"""opencode adapter quirks: boot mutations and enforced config invariants. +"""opencode adapter quirks: boot mutations, agent and command shapes, and frontmatter. opencode's boot mutates its own composition directories: it injects a ``$schema`` key into config files that lack one (an in-place edit of a @@ -10,10 +10,7 @@ ``process_config`` checks what the declaration cannot state: an agent must keep its own name and carry the fields opencode's schema types, a command names an agent the run has, and a tree with no ``default_agent`` must keep -an agent that can start one. The keys a composition must keep are declared -in the descriptor (``autoupdate``, ``share``, ``enabled_providers``, and the -model choices the binding owns) and the shared render refuses them before -this step runs. +an agent that can start one. It also keeps the model Reef's. The descriptor sets ``enabled_providers`` to ``["reef"]``, so opencode offers no provider but the binding's, its own zen @@ -22,9 +19,7 @@ key it writes, and always writes a non-empty ``apiKey``, which a tree cannot hold because admission refuses an inline credential. So the two keys pass only in the binding's shape with that key, which a tree rendered alone, as -admission renders it, never has; the descriptor locks the model choices -(``small_model``, ``disabled_providers``, an agent's, a mode's or a -command's ``model``) beside them. +admission renders it, never has. opencode reads the frontmatter of a command or a skill with gray-matter, which strips a byte order mark, takes the text after the opening ``---`` as diff --git a/reef/harness/adapters/pi/descriptor.yaml b/reef/harness/adapters/pi/descriptor.yaml index 9eaa25c16..16bfdaa40 100644 --- a/reef/harness/adapters/pi/descriptor.yaml +++ b/reef/harness/adapters/pi/descriptor.yaml @@ -61,8 +61,6 @@ files: primary: path: "pi-agent/settings.json" defaults: {} - # The shared render refuses a composition that fails one of these - # checks, before any quirks step runs. checks: # They choose the models a run may use or send every call through # another host. diff --git a/reef/harness/adapters/pi/quirks.py b/reef/harness/adapters/pi/quirks.py index 9a8a4c219..d52ba75b1 100644 --- a/reef/harness/adapters/pi/quirks.py +++ b/reef/harness/adapters/pi/quirks.py @@ -11,13 +11,6 @@ and never offered. ``process_skill`` gives a skill node whose text carries none both, the description being the text's first line. -The descriptor's checks keep every model call on Reef's model binding: the -binding writes the provider ``reef`` in ``models.json`` and selects it in -``settings.json``, and the checks refuse another provider, a ``reef`` -provider with keys the binding does not write, and ``enabledModels`` and -``httpProxy``, which send every call through another set of models or -another host. - pi 0.84.2 rewrites a few old settings keys when it loads ``settings.json`` and writes the new form back on its next save: ``queueMode`` becomes ``steeringMode``, a boolean ``websockets`` becomes ``transport``, and a diff --git a/reef/harness/adapters/terminus/descriptor.yaml b/reef/harness/adapters/terminus/descriptor.yaml index b4360f4ed..4a0ec3ce4 100644 --- a/reef/harness/adapters/terminus/descriptor.yaml +++ b/reef/harness/adapters/terminus/descriptor.yaml @@ -50,8 +50,6 @@ files: primary: path: "terminus/config.json" defaults: {} - # The shared render refuses a composition that fails one of these - # checks, before any quirks step runs. checks: # Terminus 2 constructor arguments a tree may set, verified against # harbor 0.20.0, which reef-eval 0.1.1 resolves; a bump should check diff --git a/reef/harness/adapters/terminus/quirks.py b/reef/harness/adapters/terminus/quirks.py index 421ee2395..8a4960a02 100644 --- a/reef/harness/adapters/terminus/quirks.py +++ b/reef/harness/adapters/terminus/quirks.py @@ -1,29 +1,11 @@ """Terminus adapter quirks: constructor knobs, skill frontmatter, one context module. -Terminus 2 takes its behavior from constructor arguments rather than a config -file it discovers, so ``terminus/config.json`` is a flat object whose keys must -each name a real Terminus 2 argument. The descriptor's checks refuse an -unknown key, a defect in the tree: failing at render is what keeps a gated -change meaning what it says instead of silently dropping a knob. - Skills carry the ``name`` and ``description`` frontmatter the instruction builder reads. ``process_skill`` and ``process_command`` add it when an evolved node ships bare text, under both skill roots. Terminus 2 has no slash-command surface, so ``agent_command`` renders under the second root, and Harbor lists those skills beside the others, with nothing that marks them as commands. -The descriptor's checks also keep every model call on Reef's model binding. -The binding writes ``model_name``, ``api_base`` and ``llm_kwargs`` with the -key; the checks refuse other ``llm_kwargs`` keys, a ``llm_call_kwargs`` -argument that litellm reads as the endpoint, the provider, a credential, the -model, a fallback model or a logging callback, and a model named in -``extra_body``. - -These checks cover the config the tree renders. Reef's proxy forwards the -``model`` and ``models`` a request sends as they are, so a request that other -code builds with the rendered key, such as a tool the model runs, can still -name another model. - One ``code_extension`` can define an Agent subclass of Harbor's Terminus2. Rendering only checks its syntax. Execution requires Reef's sandbox around the runner and Harbor's remote E2B environment for the terminal task: the diff --git a/tests/reef_service/test_harness_config_checks.py b/tests/reef_service/test_harness_config_checks.py index 0e479173e..0bab3aaba 100644 --- a/tests/reef_service/test_harness_config_checks.py +++ b/tests/reef_service/test_harness_config_checks.py @@ -215,15 +215,6 @@ def test_checks_are_validated_at_load(tmp_path: Path, checks: object, message: s checked(tmp_path, checks) -def test_a_descriptor_that_still_declares_locked_keys_is_refused(tmp_path: Path) -> None: - data = yaml.safe_load((Path(reef.harness.adapters.__file__).parent / "opencode" / "descriptor.yaml").read_text()) - data["files"]["config"]["primary"]["locked"] = ["autoupdate"] - path = tmp_path / "descriptor.yaml" - path.write_text(yaml.safe_dump(data), encoding="utf-8") - with pytest.raises(DescriptorError, match=r"'locked' is replaced by 'checks'.*IsDefault\(\).*IsAbsent\(\)"): - load_descriptor(path) - - def test_a_check_with_a_list_of_keys_is_one_check_per_key(tmp_path: Path) -> None: descriptor = checked(tmp_path, [{"key": ["a", "b.c"], "match": "IsAbsent()", "reason": "r"}], {}) assert [(check.key, check.reason) for check in descriptor.config_targets["primary"].checks] == [ From 8d918848708f9f60879a19ffd6fdb03af5180763 Mon Sep 17 00:00:00 2001 From: Xuezhi Yan Date: Wed, 14 Oct 2026 16:36:02 -0400 Subject: [PATCH 8/8] test(harness): load every bundled descriptor so malformed checks fail in CI Co-Authored-By: Claude Opus 5.5 --- tests/reef_service/test_harness_config_checks.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/reef_service/test_harness_config_checks.py b/tests/reef_service/test_harness_config_checks.py index 0bab3aaba..bb755fbd5 100644 --- a/tests/reef_service/test_harness_config_checks.py +++ b/tests/reef_service/test_harness_config_checks.py @@ -10,7 +10,7 @@ import yaml import reef.harness.adapters -from reef.harness.adapters import get_adapter +from reef.harness.adapters import BUILTIN_ADAPTERS, builtin_descriptor_path, get_adapter from reef.harness.adapters.descriptor import AdapterDescriptor, AdapterRenderer, DescriptorError, load_descriptor from reef.harness.adapters.matchers import ABSENT, parse_matcher from reef.harness.tree.render import RenderError, render_composition @@ -18,6 +18,18 @@ OPENCODE = "opencode/opencode.json" +@pytest.mark.parametrize("adapter", BUILTIN_ADAPTERS) +def test_every_bundled_descriptor_loads(adapter: str) -> None: + """Loading a descriptor parses each of its match expressions, so a malformed check fails here, not at render.""" + descriptor = load_descriptor(builtin_descriptor_path(adapter)) + assert descriptor.name == adapter + + +def test_every_descriptor_file_is_a_bundled_adapter() -> None: + adapters_dir = Path(reef.harness.adapters.__file__).parent + assert {path.parent.name for path in adapters_dir.glob("*/descriptor.yaml")} == set(BUILTIN_ADAPTERS) + + @pytest.mark.parametrize( ("source", "value", "expected"), [