Skip to content

refactor(harness): declare adapter config checks with matchers in descriptors - #798

Merged
BobbyZhouZijian merged 10 commits into
mainfrom
yanxz/config_check_matchers
Oct 15, 2026
Merged

BobbyZhouZijian merged 10 commits into
mainfrom
yanxz/config_check_matchers

Conversation

@yanxz

@yanxz yanxz commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

First stage of RFC #797: config checks, which supersede locked (#737). Checks for the other node kinds are not implemented here. Part of #681.

Changes

  • Add reef/harness/adapters/matchers.py, a GoogleTest-style matcher subset and a parser for match expressions.
  • Replace files.config.<target>.locked with checks. The shared render runs them before any adapter step.
  • Move the config rules of claude, codex, dsh, hermes, pi and terminus from quirks.py into their descriptors. Convert opencode's locked list; its cross-reference checks stay in Python.
  • Also add tests for all the descriptor.yaml files of bundled harnesses, to make sure they are legit. This is especially important considering the matcher string is non-trivial to get right.

Documentation impact

  • Reader question: how does an adapter declare what its config must keep or must not set?
  • Updated page: "Config checks" in docs/developer-guide/harness-adapters.rst.
  • Verification: the documentation build was not run.

Compatibility and operational impact

The loader no longer reads locked; a descriptor must declare checks instead. Refusal messages change format. The RFC lists the refusals that are stricter or looser than before.

The install route renders a release again with the model binding, and it swallows a RenderError and installs the tree without a binding. A release published before this change that fails one of the stricter checks therefore installs with no model binding and no error, until the install route stops dropping render errors (a separate follow-up).

Verification

  • pytest tests/harness/test_harness_config_checks.py: 91 passed. It loads every bundled descriptor, so a malformed match expression fails here; checked by misspelling one matcher.
  • pytest tests/harness after merging main: 1146 passed, 15 skipped, 1 failed. The failure, test_native_plugins.py::test_every_kind_installs_into_the_host_and_leaves_nothing_when_the_entry_leaves, also fails on main.
  • python .github/scripts/check_test_layout.py: passed.
  • pre-commit run --files <changed files>: passed.
  • mypy: no new errors. The existing reef/observability/open_telemetry.py errors remain.
  • Not run: the full tests/ suite, which needs the training environment, and real-binary smoke tests.

AI assistance

Claude Code implemented the change and drafted this description under the author's direction. The author is still reviewing it.

Checklist

  • The change is focused and contains no unrelated cleanup.
  • Changed code satisfies the style, code structure, and naming rules in AGENTS.md. Author review is pending.
  • Tests cover behavior changes, or this pull request does not change behavior.
  • Public interface changes include contract tests, or no public interface changes are present.
  • Affected documentation is updated and reviewed against the Documentation review checklist in AGENTS.md. Author review is pending.
  • Root README changes update both languages and README.i18n.yaml, or do not affect README content.
  • An accepted RFC is linked, or this change does not require an RFC. The RFC is not accepted yet.
  • Relevant pre-commit and test checks pass.
  • Non-trivial AI assistance is disclosed above, or none was used.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

…scriptors

Replace files.config.<target>.locked with checks: a key of the merged
config, a GoogleTest-style matcher expression every value there must
match, and an optional reason. The shared render runs them before any
adapter step. Move the config rules of claude, codex, dsh, hermes, pi
and terminus from quirks.py into their descriptors.

Reference implementation of RFC #797; not to be merged before it is
accepted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added area: docs User and developer documentation area: harness Harness integration, recipes, and evolution surfaces area: service Service APIs, deployment, and command-line interfaces labels Oct 14, 2026
@BobbyZhouZijian BobbyZhouZijian changed the title refactor(harness)!: declare adapter config checks with matchers in descriptors [DO NOT MERGE] refactor(harness)!: declare adapter config checks with matchers in descriptors Oct 14, 2026
@BobbyZhouZijian

Copy link
Copy Markdown
Contributor

@hanfeiyu it seems the doc rule might be relevant to this PR. just fyi

yanxz and others added 5 commits October 14, 2026 13:19
Present checks as rules on tree nodes, with config as the node kind that
takes them: config nodes merge into a target, so its checks match the
merged target. Point the claude, hermes and codex text at the descriptor's
checks instead of the quirks that no longer hold those defaults.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restore the claude client_env, hermes and codex text that says the quirks
enforce the defaults; correcting it is tracked separately.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s checks

This PR moved the claude updater-switch refusal and the codex defaults
from the quirks into descriptor checks, so the guide now says so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The render writes each config file in the format its suffix names, and
the hermes defaults are kept by the descriptor's checks, not the quirks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Give the renderer steps their own section and move node checks after the
engine and adapter seams, each with its part of the guidance on checks and
steps. Wrap the page at 79 columns, turning the adapter grid table into a
list-table; one link whose URL alone is longer stays as it is.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread reef/harness/adapters/claude/quirks.py Outdated
Comment thread reef/harness/adapters/claude/quirks.py
Comment thread reef/harness/adapters/claude/descriptor.yaml Outdated
Comment thread reef/harness/adapters/claude/descriptor.yaml
Comment thread reef/harness/adapters/descriptor.py Outdated
Comment thread reef/harness/adapters/matchers.py Outdated
Comment thread reef/harness/adapters/matchers.py Outdated
Comment thread reef/harness/adapters/matchers.py Outdated
Comment thread reef/harness/adapters/matchers.py Outdated
yanxz and others added 3 commits October 14, 2026 16:25
…dling

Remove the quirks docstrings and descriptor comments that only restate
what the shared checks do; adapter-specific reasons stay beside their
checks. Treat a leftover locked field like any other unknown field, and
describe the matchers as tree node checks that link to the adapter guide.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… in CI

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve two conflicts: keep the matchers import beside the moved
EpisodeExecutor import, and drop the guide's paragraph on the module-level
finalize_render that main removed. Move the config checks tests into
tests/harness/ with the other harness suites.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the area: service Service APIs, deployment, and command-line interfaces label Oct 14, 2026
@yanxz
yanxz marked this pull request as ready for review October 14, 2026 20:43
@blacksmith-sh

This comment has been minimized.

@yanxz yanxz changed the title [DO NOT MERGE] refactor(harness)!: declare adapter config checks with matchers in descriptors refactor(harness): declare adapter config checks with matchers in descriptors Oct 15, 2026
@BobbyZhouZijian
BobbyZhouZijian merged commit 0e5ff84 into main Oct 15, 2026
24 of 25 checks passed
@yanxz
yanxz deleted the yanxz/config_check_matchers branch October 15, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs User and developer documentation area: harness Harness integration, recipes, and evolution surfaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants