Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/vouch/admission.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@

import yaml

from .config_coerce import coerce_bool

if TYPE_CHECKING:
from .storage import KBStore

Expand Down Expand Up @@ -103,10 +105,11 @@ def load_config(store: KBStore) -> AdmissionConfig:
if not isinstance(raw, dict):
return AdmissionConfig()
return AdmissionConfig(
enabled=bool(raw.get("enabled", DEFAULT_ENABLED)),
enabled=coerce_bool(raw.get("enabled", DEFAULT_ENABLED), DEFAULT_ENABLED),
min_confidence=_as_float(raw.get("min_confidence")) or DEFAULT_MIN_CONFIDENCE,
reject_uncited_session_pages=bool(
raw.get("reject_uncited_session_pages", DEFAULT_REJECT_UNCITED_SESSION_PAGES)
reject_uncited_session_pages=coerce_bool(
raw.get("reject_uncited_session_pages", DEFAULT_REJECT_UNCITED_SESSION_PAGES),
DEFAULT_REJECT_UNCITED_SESSION_PAGES,
),
)

Expand Down
3 changes: 2 additions & 1 deletion src/vouch/capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import yaml

from .config_coerce import coerce_bool
from .enrich import Enrichment
from .models import ProposalStatus
from .secrets import mask_secrets
Expand Down Expand Up @@ -70,7 +71,7 @@ def load_config(store: KBStore) -> CaptureConfig:
if answer_mode not in _ANSWER_MODES:
answer_mode = DEFAULT_ANSWER_MODE
return CaptureConfig(
enabled=bool(raw.get("enabled", DEFAULT_ENABLED)),
enabled=coerce_bool(raw.get("enabled", DEFAULT_ENABLED), DEFAULT_ENABLED),
min_observations=int(raw.get("min_observations", DEFAULT_MIN_OBSERVATIONS)),
dedup_window_seconds=float(
raw.get("dedup_window_seconds", DEFAULT_DEDUP_WINDOW_SECONDS)
Expand Down
3 changes: 2 additions & 1 deletion src/vouch/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

from . import audit as audit_mod
from . import llm_draft
from .config_coerce import coerce_bool
from .context import _RETRACTED_CLAIM_STATUSES
from .models import ProposalStatus
from .proposals import ProposalError, _slugify, propose_page
Expand Down Expand Up @@ -169,7 +170,7 @@ def load_config(store: KBStore) -> CompileConfig:
raw.get("timeout_seconds", DEFAULT_TIMEOUT_SECONDS),
DEFAULT_TIMEOUT_SECONDS, float,
),
two_phase=bool(raw.get("two_phase", False)),
two_phase=coerce_bool(raw.get("two_phase", False), False),
)


Expand Down
39 changes: 39 additions & 0 deletions src/vouch/config_coerce.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"""shared config-boolean coercion.

`bool(x)` on an arbitrary config value is a trap: `yaml.safe_load` resolves
an *unquoted* `true`/`false` to a real Python bool, but a mistakenly-quoted
`"false"` in config.yaml stays a non-empty string, and `bool("false")` is
True. A dozen `load_config()` functions across this codebase each read a
boolean flag out of raw YAML; before this module they either repeated the
same `bool(raw.get(...))` bug (compile.py's `two_phase`, and the `enabled`
flag in session_split/admission/inbox/recall/capture/volunteer_context) or
raised hard on a bad value (install_adapter.py's install.yaml flags, which
is the right call for a one-time CLI install but not for config read on
every session/render).

`coerce_bool()` is the fail-soft half of that spectrum: recognized string
spellings are parsed case-insensitively, anything else (wrong type, typo,
unrecognized spelling) degrades to the caller's default rather than taking
down every caller on a config mistake -- the same fail-soft posture
compile.py's own `_coerce()` already documents for its numeric fields.
"""

from __future__ import annotations

from typing import Any

_TRUE_STRINGS = frozenset({"true", "yes", "on", "1"})
_FALSE_STRINGS = frozenset({"false", "no", "off", "0"})


def coerce_bool(value: Any, default: bool) -> bool:
"""parse a YAML-sourced config boolean, defaulting on anything unrecognized."""
if isinstance(value, bool):
return value
if isinstance(value, str):
lowered = value.strip().lower()
if lowered in _TRUE_STRINGS:
return True
if lowered in _FALSE_STRINGS:
return False
return default
3 changes: 2 additions & 1 deletion src/vouch/inbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import yaml

from .config_coerce import coerce_bool
from .proposals import propose_page
from .storage import KBStore

Expand Down Expand Up @@ -62,7 +63,7 @@ def load_config(store: KBStore) -> InboxConfig:
return InboxConfig()
extensions = raw.get("extensions")
return InboxConfig(
enabled=bool(raw.get("enabled", True)),
enabled=coerce_bool(raw.get("enabled", True), True),
min_chars=int(raw.get("min_chars", DEFAULT_MIN_CHARS)),
extensions=(
tuple(str(e) for e in extensions)
Expand Down
18 changes: 16 additions & 2 deletions src/vouch/proposals.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from pydantic import ValidationError

from . import admission, audit, index_db
from .config_coerce import coerce_bool
from .models import (
ArtifactScope,
Claim,
Expand Down Expand Up @@ -513,15 +514,28 @@ def propose_delete(


def _review_config(store: KBStore) -> dict[str, Any]:
"""The ``review:`` section of config.yaml, or {} if absent/unreadable."""
"""the ``review:`` section of config.yaml, or {} if absent/unreadable.

``auto_approve_on_receipt`` is normalized to a real bool here -- the
single source of truth every caller below reads from -- so a
mistakenly-quoted ``auto_approve_on_receipt: "false"`` in config.yaml
can never be silently treated as enabled by one call site while another
(correctly) treats the same value as disabled. callers that still wrap
this in their own ``bool(...)`` are unaffected: bool() on an already-real
bool is a no-op.
"""
try:
loaded = yaml.safe_load(
(store.kb_dir / "config.yaml").read_text(encoding="utf-8")
)
except Exception:
return {}
if isinstance(loaded, dict) and isinstance(loaded.get("review"), dict):
return loaded["review"]
review = dict(loaded["review"])
review["auto_approve_on_receipt"] = coerce_bool(
review.get("auto_approve_on_receipt"), False
)
return review
return {}


Expand Down
3 changes: 2 additions & 1 deletion src/vouch/recall.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import yaml

from .config_coerce import coerce_bool
from .context import _RETRACTED_CLAIM_STATUSES
from .scoping import ViewerContext, is_visible, viewer_from
from .storage import KBStore
Expand Down Expand Up @@ -45,7 +46,7 @@ def load_config(store: KBStore) -> RecallConfig:
if not isinstance(raw, dict):
return RecallConfig()
return RecallConfig(
enabled=bool(raw.get("enabled", DEFAULT_ENABLED)),
enabled=coerce_bool(raw.get("enabled", DEFAULT_ENABLED), DEFAULT_ENABLED),
max_chars=int(raw.get("max_chars", DEFAULT_MAX_CHARS)),
)

Expand Down
3 changes: 2 additions & 1 deletion src/vouch/session_split.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from . import audit as audit_mod
from . import capture, enrich, llm_draft
from . import compile as compile_mod
from .config_coerce import coerce_bool
from .llm_draft import LLMDraftError
from .models import ProposalStatus
from .proposals import _slugify, propose_page, reject
Expand Down Expand Up @@ -71,7 +72,7 @@ def load_split_config(store: KBStore) -> SplitConfig:
return SplitConfig()
llm_cmd = raw.get("llm_cmd")
return SplitConfig(
enabled=bool(raw.get("enabled", True)),
enabled=coerce_bool(raw.get("enabled", True), True),
llm_cmd=str(llm_cmd) if llm_cmd else None,
threshold_observations=_coerce(
raw.get("threshold_observations", DEFAULT_THRESHOLD_OBSERVATIONS),
Expand Down
3 changes: 2 additions & 1 deletion src/vouch/volunteer_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import yaml

from . import hot_memory
from .config_coerce import coerce_bool
from .context import _RETRACTED_CLAIM_STATUSES
from .models import Session
from .scoping import ViewerContext, viewer_from
Expand Down Expand Up @@ -74,7 +75,7 @@ def load_config(store: KBStore) -> VolunteerConfig:
raw = loaded.get("volunteer")
if not isinstance(raw, dict):
return VolunteerConfig()
enabled = bool(raw.get("enabled", True))
enabled = coerce_bool(raw.get("enabled", True), True)
threshold = float(raw.get("threshold", DEFAULT_THRESHOLD))
throttle = float(raw.get("throttle_seconds", DEFAULT_THROTTLE_SECONDS))
poll = float(raw.get("poll_interval_seconds", DEFAULT_POLL_INTERVAL))
Expand Down
15 changes: 15 additions & 0 deletions tests/test_admission.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@ def store(tmp_path, monkeypatch) -> KBStore:
return s


def test_load_config_quoted_false_does_not_enable(store: KBStore) -> None:
"""Regression: bool("false") is True in plain Python, so a mistakenly
quoted `enabled: "false"` previously silently kept admission gating on,
and `reject_uncited_session_pages: "false"` kept that rule on too."""
store.config_path.write_text(
store.config_path.read_text(encoding="utf-8")
+ '\nadmission:\n enabled: "false"\n'
' reject_uncited_session_pages: "false"\n',
encoding="utf-8",
)
cfg = admission.load_config(store)
assert cfg.enabled is False
assert cfg.reject_uncited_session_pages is False


# ---------------------------------------------------------------- pure predicate
@pytest.mark.parametrize("text", FRAGMENT_CLAIMS)
def test_assess_claim_rejects_structural_fragments(text: str) -> None:
Expand Down
7 changes: 7 additions & 0 deletions tests/test_capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ def test_load_config_reads_override(store: KBStore) -> None:
assert cfg.min_observations == 5


def test_load_config_quoted_false_does_not_enable(store: KBStore) -> None:
"""Regression: bool("false") is True in plain Python, so a mistakenly
quoted `enabled: "false"` previously silently kept capture enabled."""
store.config_path.write_text('capture:\n enabled: "false"\n')
assert cap.load_config(store).enabled is False


def test_buffer_path_under_captures_dir(store: KBStore) -> None:
p = cap.buffer_path(store, "sess-123")
assert p == store.kb_dir / "captures" / "sess-123.jsonl"
Expand Down
12 changes: 12 additions & 0 deletions tests/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,18 @@ def test_two_phase_config_flag_parsed(store: KBStore) -> None:
assert cfg.two_phase is True


def test_two_phase_quoted_false_string_does_not_enable(store: KBStore) -> None:
"""Regression: bool("false") is True in plain Python, so a mistakenly
quoted `two_phase: "false"` previously silently enabled two-phase mode."""
store.config_path.write_text(
store.config_path.read_text(encoding="utf-8")
+ '\ncompile:\n llm_cmd: "cat /dev/null"\n two_phase: "false"\n',
encoding="utf-8",
)
cfg = compile_mod.load_config(store)
assert cfg.two_phase is False


def test_parse_topics_reads_strings_and_objects() -> None:
assert compile_mod.parse_topics('["Alpha", "Beta"]') == ["Alpha", "Beta"]
assert compile_mod.parse_topics('[{"title": "Gamma"}]') == ["Gamma"]
Expand Down
40 changes: 40 additions & 0 deletions tests/test_config_coerce.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
"""Tests for the shared config-boolean coercer (used by compile/session_split/
admission/inbox/recall/capture/volunteer_context/proposals config loaders)."""

from __future__ import annotations

import pytest

from vouch.config_coerce import coerce_bool


class TestCoerceBool:
def test_real_true_passes_through(self):
assert coerce_bool(True, False) is True

def test_real_false_passes_through(self):
assert coerce_bool(False, True) is False

@pytest.mark.parametrize("value", ["true", "True", "TRUE", "yes", "on", "1"])
def test_recognized_true_strings(self, value):
assert coerce_bool(value, False) is True

@pytest.mark.parametrize("value", ["false", "False", "FALSE", "no", "off", "0"])
def test_recognized_false_strings(self, value):
"""Regression: bool("false") is True in plain Python -- the exact bug
this module exists to close off for every config loader that uses it."""
assert coerce_bool(value, True) is False

def test_whitespace_is_stripped(self):
assert coerce_bool(" false ", True) is False
assert coerce_bool(" true ", False) is True

def test_unrecognized_string_falls_back_to_default(self):
assert coerce_bool("maybe", True) is True
assert coerce_bool("maybe", False) is False

def test_non_bool_non_string_falls_back_to_default(self):
assert coerce_bool(None, True) is True
assert coerce_bool(1, False) is False # int, not bool -- not the same type
assert coerce_bool([], True) is True
assert coerce_bool({}, False) is False
10 changes: 10 additions & 0 deletions tests/test_inbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@ def test_scan_disabled_via_config_is_noop(store: KBStore) -> None:
assert store.list_proposals(ProposalStatus.PENDING) == []


def test_load_config_quoted_false_does_not_enable(store: KBStore) -> None:
"""Regression: bool("false") is True in plain Python, so a mistakenly
quoted `enabled: "false"` previously silently kept inbox scanning on."""
store.config_path.write_text(
store.config_path.read_text(encoding="utf-8") + '\ninbox:\n enabled: "false"\n',
encoding="utf-8",
)
assert inbox.load_config(store).enabled is False


def test_watch_is_bounded_by_iterations(store: KBStore) -> None:
_drop(store, "notes.md")
results: list[inbox.ScanResult] = []
Expand Down
7 changes: 7 additions & 0 deletions tests/test_recall.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ def test_load_config_override(store: KBStore) -> None:
assert cfg.max_chars == 500


def test_load_config_quoted_false_does_not_enable(store: KBStore) -> None:
"""Regression: bool("false") is True in plain Python, so a mistakenly
quoted `enabled: "false"` previously silently kept recall enabled."""
store.config_path.write_text('recall:\n enabled: "false"\n', encoding="utf-8")
assert recall.load_config(store).enabled is False


def test_starter_config_has_recall_namespace() -> None:
assert _starter_config()["recall"]["enabled"] is True

Expand Down
49 changes: 49 additions & 0 deletions tests/test_receipt_auto_approve.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,52 @@ def test_auto_approve_receipts_leaves_id_conflict_pending(store: KBStore) -> Non
assert other is not None
assert auto_approve_receipts(store) == []
assert store.get_proposal(other.id).status is ProposalStatus.PENDING


def test_receipt_claim_blocked_when_gate_quoted_false_string(store: KBStore) -> None:
"""Regression: bool("false") is True in plain Python. Before
_review_config() normalized this field, a mistakenly-quoted
`auto_approve_on_receipt: "false"` in config.yaml was read as *enabled*
by the raw truthy checks in resolve_pending_receipt_claim/approve,
silently granting self-approval an operator explicitly meant to disable."""
store.config_path.write_text(
"review:\n auto_approve_on_receipt: \"false\"\n", encoding="utf-8"
)
src = store.put_source(b"the sky is blue")
res = propose_quoted_claim(
store, text="the sky is blue", source_id=src.id,
quote="the sky is blue", proposed_by="agent-a",
)
assert res is not None
with pytest.raises(ProposalError, match="forbidden_self_approval"):
approve(store, res.id, approved_by="agent-a")


def test_auto_approve_receipts_noop_when_gate_quoted_false_string(store: KBStore) -> None:
"""Same regression as above, exercised through the batch drain path."""
store.config_path.write_text(
"review:\n auto_approve_on_receipt: \"false\"\n", encoding="utf-8"
)
src = store.put_source(b"alpha beta")
propose_quoted_claim(
store, text="mentions beta", source_id=src.id, quote="beta",
proposed_by="agent-a",
)
assert auto_approve_receipts(store) == []


def test_receipt_verified_claim_self_approves_when_gate_quoted_true_string(
store: KBStore,
) -> None:
"""The quoted-string fix must not break the legitimate quoted-"true" case."""
store.config_path.write_text(
"review:\n auto_approve_on_receipt: \"true\"\n", encoding="utf-8"
)
src = store.put_source(b"the sky is blue today")
res = propose_quoted_claim(
store, text="the sky is blue", source_id=src.id,
quote="the sky is blue", proposed_by="agent-a",
)
assert res is not None
claim = approve(store, res.id, approved_by="agent-a")
assert store.get_claim(claim.id).text == "the sky is blue"
Loading
Loading