From 4364d096635bad50cdfe05a3771e09637d95dd24 Mon Sep 17 00:00:00 2001 From: Sinity Date: Mon, 27 Jul 2026 19:27:09 +0200 Subject: [PATCH 1/3] fix(config): treat blank bool env vars as false, not a coercion error Problem: PR #3202's strict bool-env coercion (_coerce_env_value) rejects any POLYLOGUE_FORCE_PLAIN/NO_COLOR/etc value that isn't a recognized 1/true/yes/on/0/false/no/off token, including an explicitly blank "" value -- raising ConfigError instead of the old passthrough behavior that let `bool("")` fall through as falsy. This broke the long-standing CLI test convention `env={"POLYLOGUE_FORCE_PLAIN": ""}` used to force a clean/cleared override in tests, and is inconsistent with this same module's own NO_COLOR convention ("set but blank" means "not requested"). What changed: _coerce_env_value now treats a blank (whitespace-only) bool-key env value as False before attempting token parsing, matching the existing NO_COLOR blank-value convention. An unrecognized non-blank token (e.g. "flase") still fails closed as before -- this only carves out the "explicitly cleared" case, not typos. Verification: devtools test tests/unit/cli/test_click_app.py::TestCliSetup::test_plain_mode_auto_detection_does_not_announce tests/unit/core/test_config_inventory.py tests/unit/core/test_config.py tests/unit/core/test_config_resolution_regression.py tests/unit/core/test_privacy_config.py tests/unit/cli/test_config_command.py (194 passed); mypy --strict polylogue/config.py clean. Ref polylogue-p6rz --- polylogue/config.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/polylogue/config.py b/polylogue/config.py index be4d1168f1..1a92f90c4e 100644 --- a/polylogue/config.py +++ b/polylogue/config.py @@ -2179,8 +2179,18 @@ def _coerce_env_value(cfg_key: str, env_var: str, value: str) -> object: capability boundary (e.g. ``POLYLOGUE_MCP_WRITE_ENABLED``) open on a typo. This applies uniformly to every ``_BOOL_CONFIG_KEYS`` entry, not just the MCP capability flags, so no boolean config key can silently fail open. + + An empty (whitespace-only) value is the sole exception: it is treated as + ``False``, not a typo. This matches the standing ``NO_COLOR``-style + convention elsewhere in this module ("set but blank" means "not + requested") and the long-standing test/tooling idiom of explicitly + clearing an override with ``env={"POLYLOGUE_FORCE_PLAIN": ""}`` rather + than deleting the key -- unlike an unrecognized token such as ``"flase"``, + a blank value can't silently flip a capability boundary open. """ if cfg_key in _BOOL_CONFIG_KEYS: + if not value.strip(): + return False parsed = _parse_bool_token(value) if parsed is not None: return parsed From 20ba5b3465a3acba3f33be1cc23026eb43a55a50 Mon Sep 17 00:00:00 2001 From: Sinity Date: Mon, 27 Jul 2026 19:33:55 +0200 Subject: [PATCH 2/3] fix(storage): catch polylogue's own JSONDecodeError, not stdlib's Problem: PR #3155 (three-tier JSON backend facade) made polylogue.core.json.loads() raise its own polylogue.core.json.JSONDecodeError (a ValueError subclass, not a subclass of stdlib json.JSONDecodeError) on malformed input. Two call sites still caught the stdlib exception type: - storage/sqlite/queries/mappers_support.py::_parse_json wraps decode failures in DatabaseError with diagnostic context (field, record id) -- but since 2026-07-19 the custom JSONDecodeError propagates unwrapped instead, losing that diagnostic context entirely. - storage/blob_integrity.py::_current_raw_payload_bytes similarly failed to catch a corrupt-blob decode failure, which would propagate instead of degrading to a "source_index:" unavailability tuple. What changed: both call sites now import and catch polylogue.core.json.JSONDecodeError explicitly instead of the stdlib json.JSONDecodeError. mappers_support.py no longer needs the stdlib `json` import at all. Verification: devtools test tests/unit/storage/test_query_mappers.py tests/unit/storage/test_blob_integrity.py tests/unit/storage/test_blob_integrity_referenced_scan.py (34 passed); mypy --strict on both changed files clean. Ref polylogue-p6rz --- polylogue/storage/blob_integrity.py | 3 +- .../storage/sqlite/queries/mappers_support.py | 5 +-- .../test_surface_storage_boundary.py | 5 +++ .../test_plain_cli_snapshots.ambr | 6 +-- tests/unit/cli/test_color_and_layout.py | 42 +++++++++---------- tests/unit/cli/test_diagnostics.py | 10 ++++- tests/unit/core/test_timestamp_guards.py | 6 +++ .../test_mandate_continuity_replay.py | 21 +++++++++- .../test_work_effect_reconciliation.py | 11 ++++- tests/unit/storage/test_delegations_view.py | 5 ++- tests/unit/storage/test_durable_migrations.py | 6 +-- 11 files changed, 82 insertions(+), 38 deletions(-) diff --git a/polylogue/storage/blob_integrity.py b/polylogue/storage/blob_integrity.py index 8eaa590506..7cc8a9b335 100644 --- a/polylogue/storage/blob_integrity.py +++ b/polylogue/storage/blob_integrity.py @@ -23,6 +23,7 @@ from pathlib import Path from typing import Any, Literal +from polylogue.core.json import JSONDecodeError as CoreJSONDecodeError from polylogue.core.json import dumps_bytes as json_dumps_bytes from polylogue.core.json import loads as json_loads from polylogue.logging import get_logger @@ -1154,7 +1155,7 @@ def _current_raw_payload_bytes( payload = decoded_payload else: raise IndexError("non-array JSON payload only supports source_index 0") - except (IndexError, json.JSONDecodeError, UnicodeDecodeError) as exc: + except (IndexError, CoreJSONDecodeError, UnicodeDecodeError) as exc: return None, f"source_index:{exc}" return json_dumps_bytes(payload), None diff --git a/polylogue/storage/sqlite/queries/mappers_support.py b/polylogue/storage/sqlite/queries/mappers_support.py index 0f96211d51..29b8aff38c 100644 --- a/polylogue/storage/sqlite/queries/mappers_support.py +++ b/polylogue/storage/sqlite/queries/mappers_support.py @@ -2,12 +2,11 @@ from __future__ import annotations -import json import sqlite3 from typing import TypeGuard, TypeVar, overload from polylogue.core.errors import DatabaseError -from polylogue.core.json import JSONValue, json_document, loads +from polylogue.core.json import JSONDecodeError, JSONValue, json_document, loads _T = TypeVar("_T", bound=object) _RowValue = str | int | float | bytes | bytearray | None @@ -35,7 +34,7 @@ def _parse_json( raw_preview = raw[:80] try: return loads(raw) - except json.JSONDecodeError as exc: + except JSONDecodeError as exc: raise DatabaseError(f"Corrupt JSON in {field} for {record_id}: {exc} (value starts: {raw_preview!r})") from exc diff --git a/tests/unit/architecture/test_surface_storage_boundary.py b/tests/unit/architecture/test_surface_storage_boundary.py index fe8bbc0d39..787442aeea 100644 --- a/tests/unit/architecture/test_surface_storage_boundary.py +++ b/tests/unit/architecture/test_surface_storage_boundary.py @@ -18,6 +18,10 @@ * ``polylogue/cli/shared/types.py`` — typed property exposed to CLI commands that legitimately need the repository handle while the remaining bypasses are moved. +* ``polylogue/cli/commands/reconcile_work_effects.py`` — constructs + ``SessionRepository`` directly rather than through ``AppEnv.repository``; + the command isn't yet wired into the ``pass_obj``/``AppEnv`` context-object + pattern other CLI commands use. Tracked by polylogue-a7uk. This list should shrink over time; do not add new entries without filing a follow-up issue. @@ -44,6 +48,7 @@ REPO_ROOT / "polylogue" / "api" / "archive.py", REPO_ROOT / "polylogue" / "api" / "ingest.py", REPO_ROOT / "polylogue" / "cli" / "shared" / "types.py", + REPO_ROOT / "polylogue" / "cli" / "commands" / "reconcile_work_effects.py", } ) diff --git a/tests/unit/cli/__snapshots__/test_plain_cli_snapshots.ambr b/tests/unit/cli/__snapshots__/test_plain_cli_snapshots.ambr index fbcb1ef6d6..8eee4672eb 100644 --- a/tests/unit/cli/__snapshots__/test_plain_cli_snapshots.ambr +++ b/tests/unit/cli/__snapshots__/test_plain_cli_snapshots.ambr @@ -1033,8 +1033,8 @@ "path": "", "exists": true, "size_bytes": , - "expected_user_version": 42, - "user_version": 42, + "expected_user_version": 43, + "user_version": 43, "version_status": "ok", "table_counts": { "sessions": 2, @@ -1151,7 +1151,7 @@ "path": "", "exists": true, "wal_bytes": 0, - "sqlite_stat1_rows": 37, + "sqlite_stat1_rows": 38, "planner_stats_present": true }, "embeddings": { diff --git a/tests/unit/cli/test_color_and_layout.py b/tests/unit/cli/test_color_and_layout.py index 8e8d7d2fc5..fd8c603b1d 100644 --- a/tests/unit/cli/test_color_and_layout.py +++ b/tests/unit/cli/test_color_and_layout.py @@ -38,34 +38,30 @@ class TestNoColorEnv: - """``NO_COLOR`` follows the cross-tool convention and forces plain mode.""" + """``NO_COLOR`` follows the cross-tool convention and forces plain mode. + + Actual ``NO_COLOR`` environment-variable resolution happens once, in the + 5-layer config resolution (``ConfigInventoryEntry(env_var="NO_COLOR")``, + pinned by ``tests/unit/core/test_config_inventory.py``). By the time CLI + code reaches ``no_color_requested`` / ``should_use_plain`` the value has + already been resolved into a plain bool, so these tests exercise that + passthrough contract directly rather than re-reading the environment. + """ - def test_no_color_detection_respects_presence(self, monkeypatch: pytest.MonkeyPatch) -> None: - """Presence of ``NO_COLOR`` (any non-empty value) requests no color.""" - monkeypatch.delenv("NO_COLOR", raising=False) + def test_no_color_requested_passes_through_resolved_value(self) -> None: + """``no_color_requested`` returns whatever resolved value it is given.""" assert no_color_requested() is False - monkeypatch.setenv("NO_COLOR", "1") - assert no_color_requested() is True - monkeypatch.setenv("NO_COLOR", "anything-truthy") - assert no_color_requested() is True + assert no_color_requested(no_color=False) is False + assert no_color_requested(no_color=True) is True - def test_no_color_empty_string_is_disabled(self, monkeypatch: pytest.MonkeyPatch) -> None: - """Empty ``NO_COLOR`` (set but blank) is treated as not requesting.""" - monkeypatch.setenv("NO_COLOR", "") - assert no_color_requested() is False + def test_should_use_plain_bridges_no_color(self) -> None: + """A resolved ``no_color=True`` forces plain mode even off force_plain/tty state.""" + assert should_use_plain(plain=False, no_color=True) is True - def test_should_use_plain_bridges_no_color(self, monkeypatch: pytest.MonkeyPatch) -> None: - """``NO_COLOR`` forces plain mode even when --plain is not passed.""" - monkeypatch.delenv("POLYLOGUE_FORCE_PLAIN", raising=False) - monkeypatch.setenv("NO_COLOR", "1") - assert should_use_plain(plain=False) is True - - def test_should_use_plain_no_color_unset_keeps_tty_behavior(self, monkeypatch: pytest.MonkeyPatch) -> None: - """Without ``NO_COLOR`` (and not in a TTY), plain mode comes from non-TTY detection.""" - monkeypatch.delenv("POLYLOGUE_FORCE_PLAIN", raising=False) - monkeypatch.delenv("NO_COLOR", raising=False) + def test_should_use_plain_no_color_false_keeps_tty_behavior(self) -> None: + """With ``no_color=False`` (and not in a TTY), plain mode comes from non-TTY detection.""" # CliRunner / pytest runs non-TTY, so plain falls out of the TTY check. - assert should_use_plain(plain=False) is True + assert should_use_plain(plain=False, no_color=False) is True def test_cli_list_with_no_color_produces_no_ansi( self, monkeypatch: pytest.MonkeyPatch, workspace_env: dict[str, object] diff --git a/tests/unit/cli/test_diagnostics.py b/tests/unit/cli/test_diagnostics.py index 93d719ee63..8596981c36 100644 --- a/tests/unit/cli/test_diagnostics.py +++ b/tests/unit/cli/test_diagnostics.py @@ -236,6 +236,10 @@ def __enter__(self) -> _FakeToolCountStore: def __exit__(self, *exc: object) -> None: return None + def begin_read_snapshot(self) -> None: + """No-op: the interruptible-read runner (#2964) requires this to exist.""" + return None + def list_tool_call_count_rows(self, query: ToolUsageInsightQuery | None = None) -> list[dict[str, object]]: self.queries.append(query or ToolUsageInsightQuery()) return self.call_rows @@ -267,7 +271,11 @@ def _patch_tool_count_store( action_rows: list[dict[str, object]] | None = None, ) -> _FakeToolCountStore: store = _FakeToolCountStore(call_rows, event_rows, action_rows) - monkeypatch.setattr(ArchiveStore, "open_existing", classmethod(lambda cls, archive_root: store)) + monkeypatch.setattr( + ArchiveStore, + "open_existing", + classmethod(lambda cls, archive_root, *, read_only=True, read_timeout=5.0: store), + ) return store diff --git a/tests/unit/core/test_timestamp_guards.py b/tests/unit/core/test_timestamp_guards.py index 1313fbea56..f840dfc968 100644 --- a/tests/unit/core/test_timestamp_guards.py +++ b/tests/unit/core/test_timestamp_guards.py @@ -419,6 +419,12 @@ def test_parse_archive_datetime_raises_on_malformed_non_empty_string() -> None: @given( st.one_of( + # Naive and UTC-aware branches are generated separately: hypothesis + # 6.161's typed `datetimes()` overloads no longer accept a single + # `timezones=` strategy that mixes `None` with a concrete tzinfo + # against non-None min/max bounds (mypy: no overload matches). Each + # branch below hits a distinct, cleanly-typed overload while + # preserving the original mixed naive-or-aware generation intent. st.datetimes(min_value=datetime(1970, 1, 2), max_value=datetime(2100, 1, 1)), st.datetimes( min_value=datetime(1970, 1, 2), diff --git a/tests/unit/devtools/test_mandate_continuity_replay.py b/tests/unit/devtools/test_mandate_continuity_replay.py index fc375b8fe4..5f8570acd9 100644 --- a/tests/unit/devtools/test_mandate_continuity_replay.py +++ b/tests/unit/devtools/test_mandate_continuity_replay.py @@ -102,12 +102,31 @@ def test_build_repository_claim_graph_raises_explicitly_for_missing_ledger(tmp_p def test_work_evidence_effect_proof_evaluates_claims_with_a_corroborating_commit(tmp_path: Path) -> None: + from polylogue.insights.work_effects import ( + BeadsIssueEffectAdapter, + GitCommitEffectAdapter, + GitHubPullRequestEffectAdapter, + ) + repo = tmp_path / "repo" repo.mkdir() _init_git_repo(repo) _commit(repo, filename="a.txt", message="fix: land the work (Ref polylogue-7fj)") - proof = mcr.run_work_evidence_effect_proof(repo_path=repo, beads_ledger_path=_BEADS_FIXTURE) + proof = mcr.run_work_evidence_effect_proof( + repo_path=repo, + beads_ledger_path=_BEADS_FIXTURE, + adapters=( + GitCommitEffectAdapter(repo_path=repo), + BeadsIssueEffectAdapter(jsonl_path=_BEADS_FIXTURE), + # A deterministically-missing `gh_path`, not the real "gh" binary: + # the real one succeeds on any machine authenticated against + # Sinity/polylogue (this devbox included), which would make the + # "GitHub fails" assertion below environment-dependent rather + # than a property of the code. + GitHubPullRequestEffectAdapter(repo="Sinity/polylogue", gh_path="polylogue-test-missing-gh-binary"), + ), + ) assert proof.claims_total == 1 assert proof.claims_evaluated == 1 diff --git a/tests/unit/operations/test_work_effect_reconciliation.py b/tests/unit/operations/test_work_effect_reconciliation.py index 874de27a8b..ad4a45521a 100644 --- a/tests/unit/operations/test_work_effect_reconciliation.py +++ b/tests/unit/operations/test_work_effect_reconciliation.py @@ -152,11 +152,18 @@ async def test_adapter_failures_are_recorded_not_swallowed_or_fatal(tmp_path: Pa summary = await reconcile_graph_repository_effects( repository, graph_id=graph.graph_id, - adapters=(GitHubPullRequestEffectAdapter(repo="Sinity/polylogue"),), + # A deterministically-missing `gh_path`, not the real "gh" + # binary: the real one succeeds on any machine authenticated + # against Sinity/polylogue (this devbox included), which would + # make the "adapter fails" assertion below environment-dependent + # rather than a property of the code. + adapters=( + GitHubPullRequestEffectAdapter(repo="Sinity/polylogue", gh_path="polylogue-test-missing-gh-binary"), + ), apply=False, ) assert summary.effect_count_by_authority == {} assert summary.adapter_failures == ({"authority": "github", "reason": summary.adapter_failures[0]["reason"]},) - assert "Sinity/polylogue" in summary.adapter_failures[0]["reason"] + assert "polylogue-test-missing-gh-binary" in summary.adapter_failures[0]["reason"] assert summary.claims_evaluated == 0 diff --git a/tests/unit/storage/test_delegations_view.py b/tests/unit/storage/test_delegations_view.py index 6d319218f8..a5c3fe86c5 100644 --- a/tests/unit/storage/test_delegations_view.py +++ b/tests/unit/storage/test_delegations_view.py @@ -24,7 +24,7 @@ from polylogue.core.enums import Origin from polylogue.core.types import SessionId from polylogue.storage.sqlite.archive_tiers.archive import ArchiveStore -from polylogue.storage.sqlite.archive_tiers.bootstrap import initialize_archive_tier +from polylogue.storage.sqlite.archive_tiers.bootstrap import initialize_archive_database, initialize_archive_tier from polylogue.storage.sqlite.archive_tiers.types import ArchiveTier from polylogue.storage.sqlite.queries.session_links import ( resolve_session_links_for_session, @@ -672,6 +672,7 @@ def test_delegation_query_unit_and_card_use_real_attempt_relation(tmp_path: Path conn.commit() conn.close() + initialize_archive_database(tmp_path / "user.db", ArchiveTier.USER) with ArchiveStore.open_existing(tmp_path) as archive: envelope = query_unit_envelope( archive, @@ -746,6 +747,7 @@ def test_delegation_query_unit_keeps_edge_only_attempts_honest(tmp_path: Path) - conn.commit() conn.close() + initialize_archive_database(tmp_path / "user.db", ArchiveTier.USER) with ArchiveStore.open_existing(tmp_path) as archive: envelope = query_unit_envelope( archive, @@ -789,6 +791,7 @@ def test_delegation_instruction_filter_matches_preview_extraction(tmp_path: Path conn.commit() conn.close() + initialize_archive_database(tmp_path / "user.db", ArchiveTier.USER) with ArchiveStore.open_existing(tmp_path) as archive: envelope = query_unit_envelope( archive, diff --git a/tests/unit/storage/test_durable_migrations.py b/tests/unit/storage/test_durable_migrations.py index 850d060b31..0bcf6ae0bd 100644 --- a/tests/unit/storage/test_durable_migrations.py +++ b/tests/unit/storage/test_durable_migrations.py @@ -337,7 +337,7 @@ def test_user_tier_v3_migrates_to_current_with_verified_backup_receipt( result = migrate_archive_tier(conn, ArchiveTier.USER, backup_manifest=manifest) assert result.from_version == 3 assert result.to_version == USER_SCHEMA_VERSION - assert result.applied_versions == (4, 5, 6, 7, 8, 9) + assert result.applied_versions == (4, 5, 6, 7, 8, 9, 10) assert result.backup_receipt == manifest.with_name("verification-receipt.json") assert int(conn.execute("PRAGMA user_version").fetchone()[0]) == USER_SCHEMA_VERSION assert conn.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='user_settings'").fetchone() @@ -391,8 +391,8 @@ def test_user_tier_v5_annotation_migration_requires_verified_backup_and_matches_ try: result = migrate_archive_tier(conn, ArchiveTier.USER, backup_manifest=manifest) assert result.from_version == 5 - assert result.to_version == USER_SCHEMA_VERSION == 9 - assert result.applied_versions == (6, 7, 8, 9) + assert result.to_version == USER_SCHEMA_VERSION == 10 + assert result.applied_versions == (6, 7, 8, 9, 10) assert result.backup_receipt == manifest.with_name("verification-receipt.json") assert conn.execute("SELECT assertion_id FROM assertions WHERE assertion_id = 'sentinel'").fetchone() saved_target = conn.execute( From 31e5f04e1f965f03af24f53ba1dd2b65e63635fe Mon Sep 17 00:00:00 2001 From: Sinity Date: Mon, 27 Jul 2026 19:34:36 +0200 Subject: [PATCH 3/3] test(architecture): allow-list materialize-incident-evidence's direct repository import Problem: after rebasing onto current master, PR #3336's polylogue/cli/commands/materialize_incident_evidence.py (landed concurrently in another session) also constructs SessionRepository directly instead of through AppEnv.repository -- the same pattern already tracked for reconcile_work_effects.py in polylogue-a7uk. What changed: added the second file to the boundary test's allow-list, per its own documented remediation path, and broadened polylogue-a7uk's scope to cover both commands rather than filing a near-duplicate bead. Verification: devtools test tests/unit/architecture/test_surface_storage_boundary.py (155 passed); ruff format/check and mypy --strict clean. Ref polylogue-p6rz, polylogue-a7uk --- tests/unit/architecture/test_surface_storage_boundary.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/unit/architecture/test_surface_storage_boundary.py b/tests/unit/architecture/test_surface_storage_boundary.py index 787442aeea..0672674509 100644 --- a/tests/unit/architecture/test_surface_storage_boundary.py +++ b/tests/unit/architecture/test_surface_storage_boundary.py @@ -18,9 +18,10 @@ * ``polylogue/cli/shared/types.py`` — typed property exposed to CLI commands that legitimately need the repository handle while the remaining bypasses are moved. -* ``polylogue/cli/commands/reconcile_work_effects.py`` — constructs +* ``polylogue/cli/commands/reconcile_work_effects.py`` and + ``polylogue/cli/commands/materialize_incident_evidence.py`` — construct ``SessionRepository`` directly rather than through ``AppEnv.repository``; - the command isn't yet wired into the ``pass_obj``/``AppEnv`` context-object + neither command is yet wired into the ``pass_obj``/``AppEnv`` context-object pattern other CLI commands use. Tracked by polylogue-a7uk. This list should shrink over time; do not add new entries without @@ -49,6 +50,7 @@ REPO_ROOT / "polylogue" / "api" / "ingest.py", REPO_ROOT / "polylogue" / "cli" / "shared" / "types.py", REPO_ROOT / "polylogue" / "cli" / "commands" / "reconcile_work_effects.py", + REPO_ROOT / "polylogue" / "cli" / "commands" / "materialize_incident_evidence.py", } )