diff --git a/docs/developer-guide/harness-adapters.rst b/docs/developer-guide/harness-adapters.rst index 7fb044fba..6170a558c 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 @@ -79,11 +88,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 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. - A second ``native_loop``: a tree holds at most one. @@ -93,8 +100,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 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: @@ -102,10 +109,15 @@ rules: - ``files.rules`` and ``files.skill`` are required. - every named template must contain ``{name}``. +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:: @@ -116,7 +128,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 +137,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,7 +158,9 @@ gets the same values from ``skill_frontmatter(path, text)``. .. important:: - 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 @@ -162,8 +176,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 @@ -201,6 +216,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 ^^^^ @@ -211,8 +326,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 ~~~~~~~~~~~~~~~~~~~~ @@ -235,8 +350,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 @@ -248,30 +363,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 ~~~~~~~~~~ @@ -489,7 +604,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`` @@ -498,10 +614,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 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -535,8 +651,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 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -566,13 +682,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 @@ -597,7 +714,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 ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -621,9 +739,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 @@ -708,9 +826,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``, 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. @@ -1237,6 +1355,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. @@ -1258,8 +1378,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 @@ -1316,9 +1436,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 @@ -1331,9 +1452,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``, @@ -1349,13 +1470,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. @@ -1393,8 +1515,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 @@ -1423,18 +1545,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 @@ -1527,15 +1649,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. @@ -1555,22 +1677,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 @@ -1594,12 +1718,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 @@ -1629,15 +1754,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; diff --git a/reef/harness/adapters/claude/descriptor.yaml b/reef/harness/adapters/claude/descriptor.yaml index 22bcd82e1..59b39e6b5 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,62 @@ 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 + 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..57cc2e52d 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,41 +6,14 @@ 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``). - -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. +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. """ from __future__ import annotations -import json import re from collections.abc import Mapping from typing import Any @@ -50,31 +23,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 +87,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..7ed46e9a0 100644 --- a/reef/harness/adapters/codex/descriptor.yaml +++ b/reef/harness/adapters/codex/descriptor.yaml @@ -64,8 +64,69 @@ files: trace_exporter: none sandbox_workspace_write: network_access: false + # 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..58fabe59e 100644 --- a/reef/harness/adapters/codex/quirks.py +++ b/reef/harness/adapters/codex/quirks.py @@ -1,15 +1,10 @@ -"""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. +"""Codex adapter quirks: the model catalog, skill metadata, and code extensions. + +``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 +30,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 +112,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 695da2937..da99a6f77 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. """ @@ -65,6 +65,7 @@ import yaml from reef.core.errors import ReefError +from reef.harness.adapters.matchers import KeyDefault, Matcher, parse_matcher, select from reef.sandbox.process import EpisodeExecutor @@ -81,8 +82,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 @@ -192,16 +195,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 @@ -431,7 +455,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), @@ -486,61 +509,46 @@ def _parse_config_targets(raw: Any, where: str) -> dict[str, ConfigTarget]: 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. - """ - 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. + 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. """ - 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..e1147fc41 100644 --- a/reef/harness/adapters/dsh/descriptor.yaml +++ b/reef/harness/adapters/dsh/descriptor.yaml @@ -68,15 +68,70 @@ 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 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 + # 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 +152,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 +168,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..8a7955178 100644 --- a/reef/harness/adapters/dsh/quirks.py +++ b/reef/harness/adapters/dsh/quirks.py @@ -28,35 +28,11 @@ 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 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.""" +read as a number, a boolean or a null is quoted.""" from __future__ import annotations import re -from collections.abc import Mapping from typing import Any, ClassVar import yaml @@ -70,8 +46,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 +69,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 +196,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 +206,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..6f33c6518 100644 --- a/reef/harness/adapters/hermes/descriptor.yaml +++ b/reef/harness/adapters/hermes/descriptor.yaml @@ -61,25 +61,79 @@ 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 + 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..91572a32d 100644 --- a/reef/harness/adapters/hermes/quirks.py +++ b/reef/harness/adapters/hermes/quirks.py @@ -15,47 +15,12 @@ ``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. - -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 -from collections.abc import Mapping from typing import Any import yaml @@ -90,34 +55,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 +114,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 +136,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..d9bb29eca --- /dev/null +++ b/reef/harness/adapters/matchers.py @@ -0,0 +1,430 @@ +"""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 +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 a check's key has in the defaults, ``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..8222435eb 100644 --- a/reef/harness/adapters/opencode/descriptor.yaml +++ b/reef/harness/adapters/opencode/descriptor.yaml @@ -45,19 +45,14 @@ 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 + 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/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 1ae365c4a..16bfdaa40 100644 --- a/reef/harness/adapters/pi/descriptor.yaml +++ b/reef/harness/adapters/pi/descriptor.yaml @@ -61,14 +61,26 @@ 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 + 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..d52ba75b1 100644 --- a/reef/harness/adapters/pi/quirks.py +++ b/reef/harness/adapters/pi/quirks.py @@ -11,18 +11,6 @@ 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. - 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 @@ -34,12 +22,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 +33,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 +63,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..4a0ec3ce4 100644 --- a/reef/harness/adapters/terminus/descriptor.yaml +++ b/reef/harness/adapters/terminus/descriptor.yaml @@ -50,6 +50,59 @@ files: primary: path: "terminus/config.json" defaults: {} + 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 dc4b84a45..c0eb494e1 100644 --- a/reef/harness/adapters/terminus/quirks.py +++ b/reef/harness/adapters/terminus/quirks.py @@ -1,34 +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. 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. - 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. -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``). - -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 @@ -37,9 +14,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 +22,8 @@ from reef.harness.tree.render import RenderError from reef.sandbox.process import EpisodeExecutor, EpisodeLaunchError, SandboxExecutor -_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 +33,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/harness/test_harness_config_checks.py b/tests/harness/test_harness_config_checks.py new file mode 100644 index 000000000..bb755fbd5 --- /dev/null +++ b/tests/harness/test_harness_config_checks.py @@ -0,0 +1,244 @@ +"""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 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 + +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"), + [ + # 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_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/harness/test_harness_model_route.py b/tests/harness/test_harness_model_route.py index 0398bd257..bcace43fc 100644 --- a/tests/harness/test_harness_model_route.py +++ b/tests/harness/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/harness/test_harness_render.py b/tests/harness/test_harness_render.py index ef1278e50..c5fbe90e0 100644 --- a/tests/harness/test_harness_render.py +++ b/tests/harness/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/harness/test_terminus_agent.py b/tests/harness/test_terminus_agent.py index d733c7c18..7f6f8f944 100644 --- a/tests/harness/test_terminus_agent.py +++ b/tests/harness/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