Skip to content

Design: one harness-config reader, one recorded rule

Phase 2 of 3 (requirements → design → tasks). Derived from the approved requirements.md. MUST be reviewed/approved before the tasks breakdown.

Architecture

Three changes that reinforce each other: a module that is the only place the harness config is read, a decision record that says when reading it is allowed, and docs that stop contradicting both.

A1. the_loop.harness_config — the single reader

A new module beside cli_config.py, deliberately its mirror image: cli_config.py is "the operator's file, resolved from four places, never repo-scoped"; harness_config.py is "the repository's file, resolved from one place, always repo-scoped".

text
cli/the_loop/
  cli_config.py       # the operator's machine  — webhooks / polling / eventLog / integrations
  harness_config.py   # a repository's policy   — workflow / reviews / testing / notifications

Surface:

NamePurpose
FILENAMES("harness-config.yaml", "config.yaml") — the rename fallback, expressed once (issue-82, decision-035)
config_path(root)The first of FILENAMES that exists under root/.the-loop, else None
load(root)Best-effort parse → dict; {} for absent / unparseable / non-mapping
load_strict(root)Same, but raises HarnessConfigError for unparseable / non-mapping
READSThe declared read surface: a tuple of HarnessConfigRead(key, command, why)

Two load functions rather than a flag, because the two behaviours are genuinely different policies and each caller has already chosen one:

  • check / graph / scenarios are best-effort — a repository that has never seen the harness config still has to gate, and a broken file must not stop a CI job from reporting the phase it can compute (bootstrap.py's existing contract).
  • critic is strictload_critics raises CriticConfigError on an unparseable file today, and must keep doing so: a review round that silently reviews nothing is a false green.

READS is the point of the module. It is what turns "which keys does the CLI read?" from a grep into a lookup, and it is what the test in A3 asserts against:

python
READS = (
    HarnessConfigRead("workflow.phaseLabelPrefix", "check, graph, daemon", "..."),
    HarnessConfigRead("workflow.specDir",          "check, graph, daemon", "..."),
    HarnessConfigRead("notifications",             "check, graph, daemon", "..."),
    HarnessConfigRead("reviews.critics",           "critic",              "..."),
    HarnessConfigRead("testing.integrationTestGlobs", "scenarios",        "..."),
)

Call sites collapse onto it, keeping their existing public names as thin aliases so no importer or test outside the package changes:

Call siteBeforeAfter
graph/bootstrap.pyown load_harness_config, own 2-name loopload_harness_config = harness_config.load (re-exported, __all__ unchanged)
critics.pyown config_path, own yaml.safe_load + error wrappingconfig_path = harness_config.config_path; load_critics calls load_strict, re-raising as CriticConfigError
commands/scenarios.pyown _load_config_globs with its own candidate list_load_config_globs reads harness_config.load(root)

Why not one function with a strict= flag, like cli_config._load_cli_config_raw? Because cli_config's two modes are the same policy at two moments (start-up vs hot-reload), while these are two different callers' contracts. A flag would let a future caller pick the wrong one silently; two named functions make the choice legible at the call site.

Why is notifications read whole rather than by leaf? Because build_runtime passes the block through to the graph runtime's hooks unexamined. READS declares the block, and the schema assertion resolves it as a block — the honest description of what is read.

A2. Decision-044 — the invariant

The rule the code has always followed, stated in one line:

A repository's harness config may configure work done on that repository. It may never configure the daemon itself.

Not "daemons read cli-config.yaml, one-shot commands read harness-config.yaml". That framing was never quite right and stopped being true when issue-113 wired graphlink into the shared dispatcher: the daemon reads a work item's own checkout for phaseLabelPrefix / specDir / notifications, on purpose, after_checkout_belongs_to proves the checkout is that repository's.

The record refines decision-032 rather than reversing it. Decision-032's real content was never "the daemon must not open that file" — it was "the daemon's own settings must not come from a checkout", which is the ⟵ direction and stays absolutely true. The record states both directions explicitly, with the four rejection reasons from requirements.md, and lists graph/model.py's .the-loop/graph.yaml / pdlc.yaml override as the other read that already obeys the same rule.

A3. cli/tests/test_harness_config.py — the pin

Four assertions, in the idiom of test_docs_parity.py (pure filesystem reads, no network, no fixtures, skipped on a source distribution for the doc halves):

#AssertionCatches
H1every READS key resolves in .the-loop/harness-config.schema.jsona key declared under a name the schema does not have
H2no module outside harness_config.py mentions a harness-config filename in a path expressiona fourth reader added quietly
H3every READS key appears in docs/config/harness-config.md's CLI-read tablereading a key nobody documented
H4every key in that table appears in READSdocumenting a read that no longer happens

H2 is the one that does the work, and it is deliberately a source assertion rather than a behavioural one: there is no runtime signal for "somebody opened this file", and the failure mode this pins is a contributor adding a fourth reader — which is visible in the diff, and only in the diff. It matches on the filename constants appearing next to .the-loop, so a docstring that merely mentions harness-config.yaml (there are several, and they are good) does not trip it.

A4. Documentation

PageChange
docs/config/index.mdReplace the ::: warning The daemon never reads… block with the directional rule; keep the fail-closed authorizedUsers / repos warning, which is the part that was load-bearing. Rewrite the "three exceptions worth memorising" list, which is now four and is not a list of exceptions.
docs/cli/concepts.mdSame correction, one paragraph.
docs/cli/commands/index.mdThe daemon/repo-scoped split stays (it is a useful reading of what the commands are), but the parenthetical "not any repository's harness config" goes.
docs/cli/index.mdMermaid: keep the two halves, add the daemon → work item's checkout edge, so the diagram shows the thing the prose now says.
docs/config/harness-config.mdNew "What the CLI reads from it" section — the table H3/H4 assert against, plus the direction rule and a link to decision-044.
docs/cli/extending.mdThe "do not mix them" guidance gains the direction rule, since that page is where a new command's author decides which file to read.
docs/capabilities/cli.mdThe invariant + a history row for issue-121.

Alternatives considered

  • Move the keys to cli-config.yaml — the issue's own third question. Rejected for the four reasons in requirements.md § Analysis: wrong cardinality, a second source of truth against the skill, breaking check/scenarios in a bare CI checkout, and inverting a trust argument that only runs one way.
  • Split the difference: keep policy in the harness config but let cli-config.yaml override per repository. Rejected — it buys one real use case (an operator who wants a different critic than the repository declares) at the cost of making every value's provenance ambiguous, and the harness config already has a per-work-item override mechanism (spec front-matter overrides) that lives next to the work it changes.
  • Fix only the docs. Tempting — the code is correct as-is, and R5 says nothing about behaviour changes. Rejected because the three duplicated readers are why the rule was never stated: with no single place that reads the file, there was nowhere for the rule to live and nothing for a test to pin. The refactor is small (net negative lines) and it is what makes R4 possible.
  • Delete the pre-rename config.yaml fallback while consolidating. Rejected — out of scope, and it is a real compatibility promise (decision-035). Consolidating it into one constant makes the eventual removal a one-line change.
  • Assert H2 with an import hook or a runtime open() shim instead of a source scan. Rejected: it would only catch a read on a path the test happens to execute, which is precisely not the failure mode (a new command with no test yet).

Security design

No trust boundary moves. Restating the requirements' threat model at the level of this design:

  • The ⟵ direction stays closed. Nothing in harness_config.py is reachable from cli_config.py, and no ingress/routing key gains a harness-config fallback. authorizedUsers and polling.sources[].repos remain CLI-config-only and fail closed — the tests that pin that (test_trust.py, test_poller.py) are untouched and must stay green.
  • The ⟶ direction stays gated where it matters. The daemon's read is still behind graphlink._checkout_belongs_to (origin-remote match, fails closed). This design adds no new caller of harness_config.load on the ingress path.
  • reviews.critics[] is unchanged. The one executable key keeps its strict load, its _validate fail-closed path, its explicit-by-name invocation and its shell=False spawn (decision-043). load_strict exists specifically so consolidation does not quietly downgrade it to best-effort — that downgrade would be the one way this refactor could become a security regression, and H2 plus the existing test_critics.py cover it.
  • Parsing is unchanged. Still yaml.safe_load, never yaml.load. Consolidation reduces the number of parse call sites from three to one, which shrinks the surface on which that could ever be got wrong.
  • READS discloses nothing. Every key is already in the published schema and the shipped template.

Testing strategy

tdd.mode: standard — the pin (A3) is written first and goes red before the module exists, then the module and the docs turn each assertion green.

LayerWhat
New unittest_harness_config.py: config_path prefers harness-config.yaml, falls back to config.yaml, returns None for neither; load returns {} for absent / unparseable / non-mapping / empty; load_strict raises for unparseable and non-mapping but returns {} for absent.
New parityH1–H4 above.
Regression (untouched)test_critics.py, test_critics_integration.py (unparseable still raises CriticConfigError; duplicate names still raise), test_cli.py's scenarios coverage (config globs, pre-rename fallback), test_graph_*.py (build_runtime defaults), test_docs_parity.py. No pre-existing test may be edited — R5.2 makes that the evidence that behaviour did not change.
Gatesmake checkruff check, ruff format --check, pyright, markdownlint, validate_config.py, the full pytest suite.
Process gateuv run the-loop check issue-121 --recompute --fail-on block — this repository's own graph gate on its own spec.

Gherkin (testing.requireGherkinDocstrings) applies to integration tests; H1–H4 and the loader unit tests are unit-scoped, matching test_docs_parity.py, which is the closest precedent in the tree.

Released under the MIT License.