From 692415a9ab742dfce675a3380c7acc1f239a02e2 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Tue, 30 Jun 2026 18:13:45 -0700 Subject: [PATCH 1/4] feat(config): typed Config model + validation for config.yaml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.vouch/config.yaml` was read as an untyped dict at each call site, with nested `.get()` guards and silent `except: {}` fallbacks — the schema was spread across the codebase and typos (`reveiw:`) were silently dropped. add a validated pydantic `Config` (with nested `ReviewConfig` and `RetrievalConfig`), parsed once and exposed as a cached `KBStore.config`. migrate the readers the issue names — proposals' self-approval / expire-window checks and the retrieval backend selector — onto it, and delete their ad-hoc parsing. a malformed value (e.g. `retrieval.default_limit: "ten"`) now fails fast with the offending key path instead of falling back, and `vouch doctor` surfaces unknown top-level keys as likely typos. sections owned by their own readers (serve, volunteer, mcp) stay loose so they neither break nor trip the unknown-key check, and the legacy `retrieval.backends` list still resolves as before, so an existing `.vouch/` loads with documented defaults and no on-disk change. closes #243 --- CHANGELOG.md | 9 +++ src/vouch/context.py | 28 +------- src/vouch/health.py | 24 ++++++- src/vouch/models.py | 105 +++++++++++++++++++++++++++- src/vouch/proposals.py | 39 +++-------- src/vouch/storage.py | 20 ++++++ tests/test_config_model.py | 138 +++++++++++++++++++++++++++++++++++++ 7 files changed, 305 insertions(+), 58 deletions(-) create mode 100644 tests/test_config_model.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f4d85de..5e242f58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,15 @@ All notable changes to vouch are documented here. Format follows committed SVGs stay reproducible (#286). ### Added +- typed `Config` model for `.vouch/config.yaml`: the file is parsed once into a + validated pydantic `Config` (with nested `ReviewConfig` / `RetrievalConfig`) + exposed as `KBStore.config`, replacing the per-call-site untyped `dict` reads + in `proposals.py` and the retrieval backend selector. a malformed value + (e.g. `retrieval.default_limit: "ten"`) now fails fast with the offending key + path instead of silently falling back, and `vouch doctor` surfaces unknown + top-level keys as likely typos instead of dropping them. existing KBs with + partial or absent `retrieval:` / `review:` blocks load with the documented + defaults — no on-disk change (#243). - GitHub PR auto-labeling: a pull-request metadata-only labeler workflow now applies vouch surface labels from `.github/labeler.yml`, keeps those labels in sync as files change, and adds OpenClaw-style `size: XS` through diff --git a/src/vouch/context.py b/src/vouch/context.py index 423c2fad..20be5039 100644 --- a/src/vouch/context.py +++ b/src/vouch/context.py @@ -16,8 +16,6 @@ import sqlite3 from typing import Any, Literal, cast -import yaml - from . import graph, index_db from .models import ClaimStatus, ContextItem, ContextPack, ContextQuality from .scoping import ( @@ -42,35 +40,15 @@ ContextItemKind = Literal["claim", "page", "entity", "relation", "source"] -_VALID_BACKENDS = ("auto", "embedding", "fts5", "substring") - - def _configured_backend(store: KBStore) -> str: """Resolve the retrieval backend from `config.yaml`, defaulting to "auto". Reads the singular `retrieval.backend` string. For KBs initialised before this knob existed, a legacy `retrieval.backends` list is honoured - by taking its first recognised entry. Anything unreadable or unrecognised - falls back to "auto". + by taking its first recognised entry. Anything unrecognised falls back to + "auto". Parsing + validation now lives in the typed `Config` model (#243). """ - try: - loaded = yaml.safe_load(store.config_path.read_text()) - except (OSError, yaml.YAMLError): - return "auto" - if not isinstance(loaded, dict): - return "auto" - retrieval = loaded.get("retrieval") - if not isinstance(retrieval, dict): - return "auto" - backend = retrieval.get("backend") - if isinstance(backend, str) and backend in _VALID_BACKENDS: - return backend - legacy = retrieval.get("backends") - if isinstance(legacy, list): - for entry in legacy: - if isinstance(entry, str) and entry in _VALID_BACKENDS: - return entry - return "auto" + return store.config.retrieval.resolved_backend() def _retrieve( diff --git a/src/vouch/health.py b/src/vouch/health.py index db2f1aa8..e18ab85c 100644 --- a/src/vouch/health.py +++ b/src/vouch/health.py @@ -17,7 +17,15 @@ from . import index_db from .audit import count_events, verify_chain -from .models import Claim, ClaimStatus, Entity, Page, ProposalKind, ProposalStatus +from .models import ( + Claim, + ClaimStatus, + ConfigError, + Entity, + Page, + ProposalKind, + ProposalStatus, +) from .storage import KBStore, _yaml_load, sha256_hex from .verify import verify_all @@ -211,6 +219,20 @@ def doctor(store: KBStore) -> HealthReport: f"audit chain broken at line {chain.line}{detail}", )) + # Config validity — a malformed value is an error; an unknown top-level + # key is a likely typo silently ignored before #243, now surfaced. + if store.config_path.exists(): + try: + cfg = store.config + except ConfigError as e: + report.findings.append(Finding("error", "config_invalid", str(e))) + else: + for key in cfg.unknown_keys(): + report.findings.append(Finding( + "warning", "config_unknown_key", + f"unknown config key {key!r} — possible typo, ignored", + )) + # Source integrity (content hash). for vr in verify_all(store): if not vr.stored_ok: diff --git a/src/vouch/models.py b/src/vouch/models.py index 23f94aa4..0e87b17d 100644 --- a/src/vouch/models.py +++ b/src/vouch/models.py @@ -12,7 +12,7 @@ from enum import StrEnum from typing import Any, Literal -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, ConfigDict, Field, ValidationError, field_validator def utcnow() -> datetime: @@ -456,3 +456,106 @@ class Capabilities(BaseModel): default_factory=list, description="OpenClaw context engines exposed (see openclaw.plugin.json)", ) + + +# --- config.yaml (issue #243) --------------------------------------------- +# +# `.vouch/config.yaml` used to be read as an untyped dict at each call site, +# with nested `.get()` guards and silent `except: {}` fallbacks. That spread +# the schema across the codebase and swallowed typos. These models are the +# single source of truth for a valid config; `KBStore.config` parses the file +# into a `Config` once, and malformed values fail fast with a per-field path. + + +class ConfigError(ValueError): + """`.vouch/config.yaml` could not be parsed into a valid Config.""" + + +class ReviewConfig(BaseModel): + """`review:` block — the write-gate policy.""" + + model_config = ConfigDict(extra="allow") + + require_human_approval: bool = True + expire_pending_after_days: int = Field(default=90, ge=0) + # "trusted-agent" lets an agent approve its own proposals; anything else + # (incl. unset) keeps the human-in-the-loop gate. + approver_role: str | None = None + + +class RetrievalConfig(BaseModel): + """`retrieval:` block — how kb.search / kb.context pick a backend.""" + + model_config = ConfigDict(extra="allow") + + # Unset (None) means "not pinned" — resolution then consults the legacy + # list and finally defaults to "auto". The starter config sets "auto" + # explicitly. Keeping this optional is what lets a legacy `backends`-only + # KB still resolve via the list. + backend: str | None = None + default_limit: int = Field(default=10, ge=1) + # Legacy plural list, honoured for KBs created before `backend` existed. + backends: list[str] | None = None + + def resolved_backend(self) -> str: + """Effective backend, preserving pre-#243 fallback behaviour. + + An explicit, recognised `backend` wins; else a legacy `backends` list + contributes its first recognised entry; else "auto". Unrecognised + values fall back to "auto". See `context._retrieve`. + """ + valid = ("auto", "embedding", "fts5", "substring") + if self.backend is not None and self.backend in valid: + return self.backend + for entry in self.backends or []: + if entry in valid: + return entry + return "auto" + + +class Config(BaseModel): + """Typed view of `.vouch/config.yaml`, parsed once at store-open. + + Known top-level sections are validated; unknown ones are preserved (not + dropped) so `vouch doctor` can flag them as likely typos — see + `unknown_keys()`. Sections owned by their own readers (`serve`, + `volunteer`, `mcp`) are kept loose here so they neither break nor trip + the unknown-key check. + """ + + model_config = ConfigDict(extra="allow") + + version: int = 1 + review: ReviewConfig = Field(default_factory=ReviewConfig) + retrieval: RetrievalConfig = Field(default_factory=RetrievalConfig) + agents: dict[str, Any] = Field(default_factory=dict) + page_kinds: dict[str, Any] = Field(default_factory=dict) + serve: dict[str, Any] | None = None + volunteer: dict[str, Any] | None = None + mcp: dict[str, Any] | None = None + + @classmethod + def load(cls, raw: Any) -> Config: + """Parse a `yaml.safe_load` result into a Config. + + `None` (empty file) and `{}` both yield all-defaults. A non-mapping + top level, or a per-field type error, raises `ConfigError` naming the + offending path. + """ + if raw is None: + raw = {} + if not isinstance(raw, dict): + raise ConfigError( + f"config.yaml must be a mapping at the top level, " + f"got {type(raw).__name__}" + ) + try: + return cls.model_validate(raw) + except ValidationError as e: + first = e.errors()[0] + loc = ".".join(str(p) for p in first["loc"]) or "" + raise ConfigError(f"config.yaml: {loc}: {first['msg']}") from e + + def unknown_keys(self) -> list[str]: + """Top-level keys outside the known schema (likely typos).""" + return sorted(self.__pydantic_extra__ or {}) diff --git a/src/vouch/proposals.py b/src/vouch/proposals.py index 22d71d4f..6dacd378 100644 --- a/src/vouch/proposals.py +++ b/src/vouch/proposals.py @@ -12,7 +12,6 @@ from datetime import UTC, datetime, timedelta from typing import Any -import yaml from pydantic import ValidationError from . import audit, index_db @@ -35,7 +34,6 @@ class ProposalError(RuntimeError): EXPIRE_REASON = "expired" EXPIRE_ACTOR = "vouch-expire" -_DEFAULT_EXPIRE_PENDING_DAYS = 90 @dataclass @@ -330,23 +328,14 @@ def _approval_block_reason( """ if proposal.status != ProposalStatus.PENDING: return f"proposal {proposal.id} is {proposal.status.value}, not pending" - if approved_by == proposal.proposed_by: - cfg: dict[str, Any] = {} - try: - loaded = yaml.safe_load((store.kb_dir / "config.yaml").read_text()) - if isinstance(loaded, dict): - cfg = loaded - except Exception: - pass - review_cfg = cfg.get("review") - approver_role = ( - review_cfg.get("approver_role") if isinstance(review_cfg, dict) else None + if ( + approved_by == proposal.proposed_by + and store.config.review.approver_role != "trusted-agent" + ): + return ( + f"forbidden_self_approval: {approved_by} cannot approve their own " + "proposal (set review.approver_role: trusted-agent in config.yaml to opt out)" ) - if approver_role != "trusted-agent": - return ( - f"forbidden_self_approval: {approved_by} cannot approve their own " - "proposal (set review.approver_role: trusted-agent in config.yaml to opt out)" - ) return None @@ -576,19 +565,7 @@ def expire_pending_after_days(store: KBStore, *, override: int | None = None) -> """Resolve GC threshold from config (`review.expire_pending_after_days`).""" if override is not None: return override - try: - loaded = yaml.safe_load(store.config_path.read_text()) - except Exception: - return _DEFAULT_EXPIRE_PENDING_DAYS - if not isinstance(loaded, dict): - return _DEFAULT_EXPIRE_PENDING_DAYS - review_cfg = loaded.get("review") - if not isinstance(review_cfg, dict): - return _DEFAULT_EXPIRE_PENDING_DAYS - days = review_cfg.get("expire_pending_after_days") - if isinstance(days, int) and days >= 0: - return days - return _DEFAULT_EXPIRE_PENDING_DAYS + return store.config.review.expire_pending_after_days def _utc(dt: datetime) -> datetime: diff --git a/src/vouch/storage.py b/src/vouch/storage.py index af21656c..99f0f90f 100644 --- a/src/vouch/storage.py +++ b/src/vouch/storage.py @@ -29,6 +29,7 @@ import re import sqlite3 import stat +from functools import cached_property from pathlib import Path from typing import Any @@ -36,6 +37,8 @@ from .models import ( Claim, + Config, + ConfigError, Entity, Evidence, Page, @@ -231,6 +234,23 @@ def init(cls, root: Path) -> KBStore: def config_path(self) -> Path: return self.kb_dir / CONFIG_FILENAME + @cached_property + def config(self) -> Config: + """Typed, validated view of config.yaml, parsed once. (#243) + + A missing file yields all-defaults (a KB may predate `config.yaml`); + a malformed file raises `ConfigError` naming the offending key. + Cached per store instance — construct a fresh `KBStore` to re-read + after rewriting config. + """ + try: + text = self.config_path.read_text(encoding="utf-8") + except FileNotFoundError: + return Config() + except OSError as e: + raise ConfigError(f"cannot read {self.config_path}: {e}") from e + return Config.load(_yaml_load(text)) + def _yaml(self, sub: str, obj_id: str) -> Path: return self.kb_dir / sub / f"{obj_id}.yaml" diff --git a/tests/test_config_model.py b/tests/test_config_model.py new file mode 100644 index 00000000..03927b4d --- /dev/null +++ b/tests/test_config_model.py @@ -0,0 +1,138 @@ +"""Typed Config model + validation for `.vouch/config.yaml` (issue #243).""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from vouch.models import Config, ConfigError +from vouch.storage import KBStore, _starter_config, _yaml_dump + + +def _init_kb(tmp_path: Path) -> KBStore: + return KBStore.init(tmp_path) + + +def _write_config(store: KBStore, raw: dict) -> None: + store.config_path.write_text(_yaml_dump(raw)) + + +# --- the model in isolation ------------------------------------------------ + + +def test_starter_config_parses_with_no_unknown_keys() -> None: + cfg = Config.load(_starter_config()) + assert cfg.review.require_human_approval is True + assert cfg.review.expire_pending_after_days == 90 + assert cfg.retrieval.backend == "auto" + assert cfg.retrieval.default_limit == 10 + assert cfg.unknown_keys() == [] + + +def test_empty_and_none_yield_defaults() -> None: + for raw in (None, {}): + cfg = Config.load(raw) + assert cfg.review.expire_pending_after_days == 90 + assert cfg.retrieval.resolved_backend() == "auto" + assert cfg.review.approver_role is None + + +def test_partial_config_fills_documented_defaults() -> None: + # An existing KB with only a `review` block still gets retrieval defaults. + cfg = Config.load({"version": 1, "review": {"approver_role": "trusted-agent"}}) + assert cfg.review.approver_role == "trusted-agent" + assert cfg.retrieval.resolved_backend() == "auto" + assert cfg.retrieval.default_limit == 10 + + +def test_malformed_value_fails_fast_with_field_path() -> None: + with pytest.raises(ConfigError) as ei: + Config.load({"retrieval": {"default_limit": "ten"}}) + assert "retrieval.default_limit" in str(ei.value) + + +def test_negative_expire_days_rejected() -> None: + with pytest.raises(ConfigError) as ei: + Config.load({"review": {"expire_pending_after_days": -1}}) + assert "review.expire_pending_after_days" in str(ei.value) + + +def test_non_mapping_top_level_rejected() -> None: + with pytest.raises(ConfigError): + Config.load(["not", "a", "mapping"]) + + +def test_unknown_top_level_key_preserved_not_dropped() -> None: + # `reveiw` is a typo — it must not silently vanish. + cfg = Config.load({"reveiw": {"require_human_approval": False}}) + assert cfg.unknown_keys() == ["reveiw"] + # The correctly-spelled default still applies. + assert cfg.review.require_human_approval is True + + +def test_known_sections_never_flagged_as_unknown() -> None: + cfg = Config.load( + { + "serve": {"bearer_tokens": []}, + "volunteer": {"enabled": False}, + "mcp": {"publish_skills": True}, + } + ) + assert cfg.unknown_keys() == [] + + +def test_resolved_backend_honours_legacy_list() -> None: + cfg = Config.load({"retrieval": {"backends": ["fts5", "substring"]}}) + assert cfg.retrieval.resolved_backend() == "fts5" + + +def test_resolved_backend_falls_back_on_unrecognised() -> None: + cfg = Config.load({"retrieval": {"backend": "nonsense"}}) + assert cfg.retrieval.resolved_backend() == "auto" + + +# --- KBStore integration --------------------------------------------------- + + +def test_store_config_defaults_when_file_missing(tmp_path: Path) -> None: + store = _init_kb(tmp_path) + store.config_path.unlink() + fresh = KBStore(tmp_path) + assert fresh.config.retrieval.resolved_backend() == "auto" + + +def test_store_config_round_trips_starter(tmp_path: Path) -> None: + store = _init_kb(tmp_path) + assert store.config.unknown_keys() == [] + assert store.config.review.expire_pending_after_days == 90 + + +def test_store_config_raises_on_malformed(tmp_path: Path) -> None: + store = _init_kb(tmp_path) + _write_config(store, {"retrieval": {"default_limit": "ten"}}) + with pytest.raises(ConfigError): + _ = KBStore(tmp_path).config + + +def test_doctor_warns_on_unknown_key(tmp_path: Path) -> None: + from vouch.health import doctor + + store = _init_kb(tmp_path) + _write_config(store, {**_starter_config(), "reveiw": {}}) + report = doctor(KBStore(tmp_path)) + codes = {(f.severity, f.code) for f in report.findings} + assert ("warning", "config_unknown_key") in codes + # An unknown key is only a warning — doctor stays ok. + assert report.ok is True + + +def test_doctor_errors_on_invalid_config(tmp_path: Path) -> None: + from vouch.health import doctor + + store = _init_kb(tmp_path) + _write_config(store, {"retrieval": {"default_limit": "ten"}}) + report = doctor(KBStore(tmp_path)) + codes = {(f.severity, f.code) for f in report.findings} + assert ("error", "config_invalid") in codes + assert report.ok is False From 930a86613293a12cf254c46859cdfd9e8c26dbd9 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Tue, 30 Jun 2026 18:25:05 -0700 Subject: [PATCH 2/4] fix(config): harden Config parsing and surface nested typos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit address review on #297: - `KBStore.config` now catches `yaml.YAMLError` from a syntactically broken config.yaml and re-raises it as `ConfigError`, so `vouch doctor` reports `config_invalid` instead of crashing on malformed YAML. - `Config.unknown_keys()` walks the `review` and `retrieval` blocks too and returns dotted paths, so a typo one level deep (`review.expier_pending_after_days`) is flagged rather than silently swallowed — the same problem this change closes at the top level. - consolidate the config diagnostics in `vouch doctor` into the single "config sanity" block. - rename the test module to `tests/test_models.py` per the mirror-the-module convention, and add cases for malformed-yaml handling, nested-typo surfacing, and the cached_property contract. --- src/vouch/health.py | 29 +++++------ src/vouch/models.py | 12 ++++- src/vouch/storage.py | 6 ++- .../{test_config_model.py => test_models.py} | 52 +++++++++++++++++++ 4 files changed, 81 insertions(+), 18 deletions(-) rename tests/{test_config_model.py => test_models.py} (70%) diff --git a/src/vouch/health.py b/src/vouch/health.py index e18ab85c..289bf4b5 100644 --- a/src/vouch/health.py +++ b/src/vouch/health.py @@ -219,20 +219,6 @@ def doctor(store: KBStore) -> HealthReport: f"audit chain broken at line {chain.line}{detail}", )) - # Config validity — a malformed value is an error; an unknown top-level - # key is a likely typo silently ignored before #243, now surfaced. - if store.config_path.exists(): - try: - cfg = store.config - except ConfigError as e: - report.findings.append(Finding("error", "config_invalid", str(e))) - else: - for key in cfg.unknown_keys(): - report.findings.append(Finding( - "warning", "config_unknown_key", - f"unknown config key {key!r} — possible typo, ignored", - )) - # Source integrity (content hash). for vr in verify_all(store): if not vr.stored_ok: @@ -254,7 +240,9 @@ def doctor(store: KBStore) -> HealthReport: ) ) - # Config sanity. + # Config sanity — a missing file is an error; otherwise a malformed value + # is an error and an unknown key is a likely typo (silently ignored before + # #243, now surfaced). if not store.config_path.exists(): report.findings.append( Finding( @@ -263,6 +251,17 @@ def doctor(store: KBStore) -> HealthReport: "config.yaml is missing", ) ) + else: + try: + cfg = store.config + except ConfigError as e: + report.findings.append(Finding("error", "config_invalid", str(e))) + else: + for key in cfg.unknown_keys(): + report.findings.append(Finding( + "warning", "config_unknown_key", + f"unknown config key {key!r} — possible typo, ignored", + )) # Index presence (warning only — the index is derivable). if not (store.kb_dir / index_db.DB_FILENAME).exists(): diff --git a/src/vouch/models.py b/src/vouch/models.py index 0e87b17d..4045018d 100644 --- a/src/vouch/models.py +++ b/src/vouch/models.py @@ -557,5 +557,13 @@ def load(cls, raw: Any) -> Config: raise ConfigError(f"config.yaml: {loc}: {first['msg']}") from e def unknown_keys(self) -> list[str]: - """Top-level keys outside the known schema (likely typos).""" - return sorted(self.__pydantic_extra__ or {}) + """Keys outside the known schema (likely typos), dotted for nesting. + + Walks the top level plus the two validated nested blocks, so a typo + one level deep (`review.expier_pending_after_days`) is surfaced the + same way a top-level one is, rather than silently swallowed. + """ + keys = set(self.__pydantic_extra__ or {}) + keys |= {f"review.{k}" for k in (self.review.__pydantic_extra__ or {})} + keys |= {f"retrieval.{k}" for k in (self.retrieval.__pydantic_extra__ or {})} + return sorted(keys) diff --git a/src/vouch/storage.py b/src/vouch/storage.py index 99f0f90f..6990513d 100644 --- a/src/vouch/storage.py +++ b/src/vouch/storage.py @@ -249,7 +249,11 @@ def config(self) -> Config: return Config() except OSError as e: raise ConfigError(f"cannot read {self.config_path}: {e}") from e - return Config.load(_yaml_load(text)) + try: + raw = _yaml_load(text) + except yaml.YAMLError as e: + raise ConfigError(f"{self.config_path}: invalid YAML: {e}") from e + return Config.load(raw) def _yaml(self, sub: str, obj_id: str) -> Path: return self.kb_dir / sub / f"{obj_id}.yaml" diff --git a/tests/test_config_model.py b/tests/test_models.py similarity index 70% rename from tests/test_config_model.py rename to tests/test_models.py index 03927b4d..40e3e03b 100644 --- a/tests/test_config_model.py +++ b/tests/test_models.py @@ -71,6 +71,23 @@ def test_unknown_top_level_key_preserved_not_dropped() -> None: assert cfg.review.require_human_approval is True +def test_unknown_nested_key_surfaced_with_dotted_path() -> None: + # A typo one level deep must be flagged too, not just top-level ones. + cfg = Config.load( + { + "review": {"expier_pending_after_days": 1}, + "retrieval": {"defualt_limit": 5}, + } + ) + assert cfg.unknown_keys() == [ + "retrieval.defualt_limit", + "review.expier_pending_after_days", + ] + # Correctly-spelled defaults are untouched. + assert cfg.review.expire_pending_after_days == 90 + assert cfg.retrieval.default_limit == 10 + + def test_known_sections_never_flagged_as_unknown() -> None: cfg = Config.load( { @@ -108,6 +125,20 @@ def test_store_config_round_trips_starter(tmp_path: Path) -> None: assert store.config.review.expire_pending_after_days == 90 +def test_store_config_is_cached_per_instance(tmp_path: Path) -> None: + store = _init_kb(tmp_path) + assert store.config is store.config + + +def test_store_config_raises_configerror_on_malformed_yaml(tmp_path: Path) -> None: + # Broken YAML *syntax* (not just a bad value) must surface as ConfigError, + # not an uncaught YAMLError that would crash callers like doctor(). + store = _init_kb(tmp_path) + store.config_path.write_text("retrieval: {backend: [unterminated\n") + with pytest.raises(ConfigError): + _ = KBStore(tmp_path).config + + def test_store_config_raises_on_malformed(tmp_path: Path) -> None: store = _init_kb(tmp_path) _write_config(store, {"retrieval": {"default_limit": "ten"}}) @@ -136,3 +167,24 @@ def test_doctor_errors_on_invalid_config(tmp_path: Path) -> None: codes = {(f.severity, f.code) for f in report.findings} assert ("error", "config_invalid") in codes assert report.ok is False + + +def test_doctor_reports_malformed_yaml_without_crashing(tmp_path: Path) -> None: + from vouch.health import doctor + + store = _init_kb(tmp_path) + store.config_path.write_text("retrieval: {backend: [unterminated\n") + report = doctor(KBStore(tmp_path)) # must not raise + codes = {(f.severity, f.code) for f in report.findings} + assert ("error", "config_invalid") in codes + assert report.ok is False + + +def test_doctor_surfaces_nested_unknown_key(tmp_path: Path) -> None: + from vouch.health import doctor + + store = _init_kb(tmp_path) + _write_config(store, {**_starter_config(), "review": {"expier_after": 1}}) + report = doctor(KBStore(tmp_path)) + warns = [f for f in report.findings if f.code == "config_unknown_key"] + assert any("review.expier_after" in f.message for f in warns) From bb7b0793bec663da503e3b53309d8a30934dee58 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Tue, 30 Jun 2026 18:44:12 -0700 Subject: [PATCH 3/4] test(config): split #243 tests to mirror the module under test address the re-review caution on #297: renaming to test_models.py left `KBStore.config` and `doctor()` cases in the wrong module's test file. - keep tests/test_models.py scoped to Config/ReviewConfig/RetrievalConfig unit tests - move the KBStore.config accessor cases into tests/test_storage.py - move the vouch doctor config-diagnostic cases into tests/test_health.py no production code change; all assertions preserved. --- tests/test_health.py | 39 +++++++++++++++- tests/test_models.py | 103 +++--------------------------------------- tests/test_storage.py | 34 ++++++++++++++ 3 files changed, 79 insertions(+), 97 deletions(-) diff --git a/tests/test_health.py b/tests/test_health.py index 32516a96..2ab9331a 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -8,7 +8,7 @@ from vouch import health, index_db from vouch.models import Claim, ClaimStatus, Proposal, ProposalKind, ProposalStatus -from vouch.storage import KBStore, _yaml_dump +from vouch.storage import KBStore, _starter_config, _yaml_dump @pytest.fixture @@ -333,3 +333,40 @@ def test_fsck_without_state_db_reports_info(store: KBStore) -> None: assert "index_missing" in codes # info finding alone shouldn't fail the report. assert report.ok is True + + +# --- config diagnostics (#243) -------------------------------------------- + + +def test_doctor_warns_on_unknown_config_key(store: KBStore) -> None: + store.config_path.write_text(_yaml_dump({**_starter_config(), "reveiw": {}})) + report = health.doctor(KBStore(store.root)) + codes = {(f.severity, f.code) for f in report.findings} + assert ("warning", "config_unknown_key") in codes + # An unknown key is only a warning — doctor stays ok. + assert report.ok is True + + +def test_doctor_errors_on_invalid_config_value(store: KBStore) -> None: + store.config_path.write_text(_yaml_dump({"retrieval": {"default_limit": "ten"}})) + report = health.doctor(KBStore(store.root)) + codes = {(f.severity, f.code) for f in report.findings} + assert ("error", "config_invalid") in codes + assert report.ok is False + + +def test_doctor_reports_malformed_yaml_without_crashing(store: KBStore) -> None: + store.config_path.write_text("retrieval: {backend: [unterminated\n") + report = health.doctor(KBStore(store.root)) # must not raise + codes = {(f.severity, f.code) for f in report.findings} + assert ("error", "config_invalid") in codes + assert report.ok is False + + +def test_doctor_surfaces_nested_unknown_config_key(store: KBStore) -> None: + store.config_path.write_text( + _yaml_dump({**_starter_config(), "review": {"expier_after": 1}}) + ) + report = health.doctor(KBStore(store.root)) + warns = [f for f in report.findings if f.code == "config_unknown_key"] + assert any("review.expier_after" in f.message for f in warns) diff --git a/tests/test_models.py b/tests/test_models.py index 40e3e03b..75b68fb3 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,24 +1,16 @@ -"""Typed Config model + validation for `.vouch/config.yaml` (issue #243).""" +"""Typed `Config` model + validation for `.vouch/config.yaml` (issue #243). -from __future__ import annotations +Pure model unit tests live here; `KBStore.config` integration is in +`test_storage.py` and the `vouch doctor` diagnostics in `test_health.py`, +per the mirror-the-module convention. +""" -from pathlib import Path +from __future__ import annotations import pytest from vouch.models import Config, ConfigError -from vouch.storage import KBStore, _starter_config, _yaml_dump - - -def _init_kb(tmp_path: Path) -> KBStore: - return KBStore.init(tmp_path) - - -def _write_config(store: KBStore, raw: dict) -> None: - store.config_path.write_text(_yaml_dump(raw)) - - -# --- the model in isolation ------------------------------------------------ +from vouch.storage import _starter_config def test_starter_config_parses_with_no_unknown_keys() -> None: @@ -107,84 +99,3 @@ def test_resolved_backend_honours_legacy_list() -> None: def test_resolved_backend_falls_back_on_unrecognised() -> None: cfg = Config.load({"retrieval": {"backend": "nonsense"}}) assert cfg.retrieval.resolved_backend() == "auto" - - -# --- KBStore integration --------------------------------------------------- - - -def test_store_config_defaults_when_file_missing(tmp_path: Path) -> None: - store = _init_kb(tmp_path) - store.config_path.unlink() - fresh = KBStore(tmp_path) - assert fresh.config.retrieval.resolved_backend() == "auto" - - -def test_store_config_round_trips_starter(tmp_path: Path) -> None: - store = _init_kb(tmp_path) - assert store.config.unknown_keys() == [] - assert store.config.review.expire_pending_after_days == 90 - - -def test_store_config_is_cached_per_instance(tmp_path: Path) -> None: - store = _init_kb(tmp_path) - assert store.config is store.config - - -def test_store_config_raises_configerror_on_malformed_yaml(tmp_path: Path) -> None: - # Broken YAML *syntax* (not just a bad value) must surface as ConfigError, - # not an uncaught YAMLError that would crash callers like doctor(). - store = _init_kb(tmp_path) - store.config_path.write_text("retrieval: {backend: [unterminated\n") - with pytest.raises(ConfigError): - _ = KBStore(tmp_path).config - - -def test_store_config_raises_on_malformed(tmp_path: Path) -> None: - store = _init_kb(tmp_path) - _write_config(store, {"retrieval": {"default_limit": "ten"}}) - with pytest.raises(ConfigError): - _ = KBStore(tmp_path).config - - -def test_doctor_warns_on_unknown_key(tmp_path: Path) -> None: - from vouch.health import doctor - - store = _init_kb(tmp_path) - _write_config(store, {**_starter_config(), "reveiw": {}}) - report = doctor(KBStore(tmp_path)) - codes = {(f.severity, f.code) for f in report.findings} - assert ("warning", "config_unknown_key") in codes - # An unknown key is only a warning — doctor stays ok. - assert report.ok is True - - -def test_doctor_errors_on_invalid_config(tmp_path: Path) -> None: - from vouch.health import doctor - - store = _init_kb(tmp_path) - _write_config(store, {"retrieval": {"default_limit": "ten"}}) - report = doctor(KBStore(tmp_path)) - codes = {(f.severity, f.code) for f in report.findings} - assert ("error", "config_invalid") in codes - assert report.ok is False - - -def test_doctor_reports_malformed_yaml_without_crashing(tmp_path: Path) -> None: - from vouch.health import doctor - - store = _init_kb(tmp_path) - store.config_path.write_text("retrieval: {backend: [unterminated\n") - report = doctor(KBStore(tmp_path)) # must not raise - codes = {(f.severity, f.code) for f in report.findings} - assert ("error", "config_invalid") in codes - assert report.ok is False - - -def test_doctor_surfaces_nested_unknown_key(tmp_path: Path) -> None: - from vouch.health import doctor - - store = _init_kb(tmp_path) - _write_config(store, {**_starter_config(), "review": {"expier_after": 1}}) - report = doctor(KBStore(tmp_path)) - warns = [f for f in report.findings if f.code == "config_unknown_key"] - assert any("review.expier_after" in f.message for f in warns) diff --git a/tests/test_storage.py b/tests/test_storage.py index ce1d643f..668bfdd4 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -11,6 +11,7 @@ from vouch.models import ( Claim, ClaimStatus, + ConfigError, Entity, EntityType, Evidence, @@ -831,3 +832,36 @@ def test_cite_resolves_source_and_evidence(store: KBStore) -> None: for c in citations } assert "source" in kinds and "evidence" in kinds + + +# --- typed config accessor (#243) ----------------------------------------- + + +def test_config_defaults_when_file_missing(store: KBStore) -> None: + store.config_path.unlink() + fresh = KBStore(store.root) + assert fresh.config.retrieval.resolved_backend() == "auto" + assert fresh.config.review.expire_pending_after_days == 90 + + +def test_config_round_trips_starter(store: KBStore) -> None: + assert store.config.unknown_keys() == [] + assert store.config.review.expire_pending_after_days == 90 + + +def test_config_is_cached_per_instance(store: KBStore) -> None: + assert store.config is store.config + + +def test_config_raises_on_malformed_value(store: KBStore) -> None: + store.config_path.write_text(_yaml_dump({"retrieval": {"default_limit": "ten"}})) + with pytest.raises(ConfigError): + _ = KBStore(store.root).config + + +def test_config_raises_on_malformed_yaml_syntax(store: KBStore) -> None: + # Broken YAML *syntax* must surface as ConfigError, not an uncaught + # yaml.YAMLError that would crash callers like doctor(). + store.config_path.write_text("retrieval: {backend: [unterminated\n") + with pytest.raises(ConfigError): + _ = KBStore(store.root).config From 73d2672c8d6e29329238dc2500c2c0c5df4d4df7 Mon Sep 17 00:00:00 2001 From: minion1227 Date: Mon, 6 Jul 2026 02:28:00 -0700 Subject: [PATCH 4/4] fix(capabilities): read host_compat from package.json after 2026.6 repackage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the #237 host-compat drift check read openclaw.compat.pluginApi from openclaw.plugin.json, but the 2026.6 loader repackage (47ac72f) moved that floor into the new loader-facing package.json — the current openclaw parser ignores openclaw.plugin.json's openclaw.* fields entirely. so _load_host_compat() read a key that no longer existed, host_compat silently degraded to {}, and the two drift tests failed on test. point _load_host_compat() and the drift tests at package.json, where the pluginApi floor now lives. the graceful-degradation and malformed-json tests move with it. no behaviour change beyond reading the right file. --- src/vouch/capabilities.py | 20 +++++++++++--------- tests/test_capabilities.py | 31 ++++++++++++++++--------------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/vouch/capabilities.py b/src/vouch/capabilities.py index 1393d978..677f5de6 100644 --- a/src/vouch/capabilities.py +++ b/src/vouch/capabilities.py @@ -17,10 +17,12 @@ _log = logging.getLogger(__name__) -# Path to the plugin manifest, relative to this module. capabilities.py lives -# at src/vouch/capabilities.py; openclaw.plugin.json lives at the repo root, -# three levels up (src/vouch/ -> src/ -> repo root). -_PLUGIN_MANIFEST_PATH = Path(__file__).resolve().parent.parent.parent / "openclaw.plugin.json" +# Path to the loader-facing package.json at the repo root, three levels up +# (src/vouch/ -> src/ -> repo root). The 2026.6 openclaw repackage moved the +# `openclaw.compat.pluginApi` floor out of openclaw.plugin.json (whose +# `openclaw.*` fields the current loader ignores) and into package.json, so +# that is now the single source of truth for host compat (#237). +_PACKAGE_JSON_PATH = Path(__file__).resolve().parent.parent.parent / "package.json" # The full method surface this implementation exposes. Keep this list in # sync with the MCP server + JSONL server registrations — `test_capabilities` @@ -90,19 +92,19 @@ def _load_host_compat() -> dict[str, dict[str, str]]: - """Read the `openclaw.compat` block from openclaw.plugin.json (#237). + """Read the `openclaw.compat` block from package.json (#237). Surfaced in `kb.capabilities` as `host_compat` so non-OpenClaw clients can detect compat without parsing the manifest themselves. Returns an - empty dict (rather than raising) if the manifest is missing or + empty dict (rather than raising) if package.json is missing or malformed — capabilities() must never fail to report basic info just because the manifest moved or this is installed as a standalone wheel - without the manifest packaged alongside it. + without package.json packaged alongside it. """ try: - manifest = json.loads(_PLUGIN_MANIFEST_PATH.read_text(encoding="utf-8")) + manifest = json.loads(_PACKAGE_JSON_PATH.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError) as e: - _log.debug("openclaw.plugin.json unreadable, host_compat will be empty: %s", e) + _log.debug("package.json unreadable, host_compat will be empty: %s", e) return {} compat = manifest.get("openclaw", {}).get("compat") if not isinstance(compat, dict): diff --git a/tests/test_capabilities.py b/tests/test_capabilities.py index 6908a3cb..cd133a8e 100644 --- a/tests/test_capabilities.py +++ b/tests/test_capabilities.py @@ -24,31 +24,32 @@ def test_capabilities_matches_jsonl_handlers() -> None: # --- host_compat drift detection (#237) ----------------------------------- # -# vouch declares openclaw.compat.pluginApi in openclaw.plugin.json. The same -# value must surface in kb.capabilities so non-OpenClaw clients can detect -# compat without parsing the manifest. These tests fail CI with a clear -# message if the two declarations drift apart. - -_MANIFEST_PATH = ( - Path(__file__).resolve().parent.parent / "openclaw.plugin.json" +# vouch declares openclaw.compat.pluginApi in package.json (the loader-facing +# manifest the 2026.6 repackage moved it to). The same value must surface in +# kb.capabilities so non-OpenClaw clients can detect compat without parsing +# the manifest. These tests fail CI with a clear message if the two +# declarations drift apart. + +_PACKAGE_JSON_PATH = ( + Path(__file__).resolve().parent.parent / "package.json" ) def _manifest_plugin_api() -> str: - manifest = json.loads(_MANIFEST_PATH.read_text(encoding="utf-8")) + manifest = json.loads(_PACKAGE_JSON_PATH.read_text(encoding="utf-8")) return manifest["openclaw"]["compat"]["pluginApi"] def test_capabilities_host_compat_matches_openclaw_manifest() -> None: """kb.capabilities.host_compat.openclaw.pluginApi must equal the - pluginApi range declared in openclaw.plugin.json. A bump in one file + pluginApi range declared in package.json. A bump in one file without the other is exactly the "host compat drift" #237 asks CI to catch.""" caps = capabilities.capabilities() manifest_range = _manifest_plugin_api() capabilities_range = caps.host_compat.get("openclaw", {}).get("pluginApi") assert capabilities_range == manifest_range, ( - f"host compat drift: openclaw.plugin.json declares pluginApi=" + f"host compat drift: package.json declares pluginApi=" f"{manifest_range!r} but kb.capabilities.host_compat reports " f"{capabilities_range!r}. Keep both in sync." ) @@ -68,10 +69,10 @@ def test_load_host_compat_returns_empty_on_missing_manifest( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, ) -> None: """_load_host_compat must degrade gracefully (empty dict, no raise) if - the manifest is absent -- e.g. installed as a standalone wheel without - openclaw.plugin.json packaged alongside it.""" + package.json is absent -- e.g. installed as a standalone wheel without + package.json packaged alongside it.""" monkeypatch.setattr( - capabilities, "_PLUGIN_MANIFEST_PATH", tmp_path / "does-not-exist.json" + capabilities, "_PACKAGE_JSON_PATH", tmp_path / "does-not-exist.json" ) assert capabilities._load_host_compat() == {} @@ -81,7 +82,7 @@ def test_load_host_compat_returns_empty_on_malformed_manifest( ) -> None: """A malformed manifest must not crash capabilities() -- it's reporting diagnostic info, not validating the install.""" - bad = tmp_path / "openclaw.plugin.json" + bad = tmp_path / "package.json" bad.write_text("{not valid json", encoding="utf-8") - monkeypatch.setattr(capabilities, "_PLUGIN_MANIFEST_PATH", bad) + monkeypatch.setattr(capabilities, "_PACKAGE_JSON_PATH", bad) assert capabilities._load_host_compat() == {}