From 82e40f363ab75992f3667dc5f2624569f287cf23 Mon Sep 17 00:00:00 2001 From: Hongsheng Liu Date: Sun, 4 Oct 2026 11:31:13 +0800 Subject: [PATCH] Provider malformed-response corpus + era-jump migration; fix usage accounting on non-dict payloads (#105) Corpus: 11 protocol-agnostic dirty bodies (truncated JSON, empty, HTML error pages, null/array top level, wrong inner shapes, non-string text, empty output) parametrized over BOTH adapters, each row asserting ProviderError (fail closed), no crash, and no credential material in the error text; plus parse_intent never returns garbage, and the ideal shapes keep passing (no overfit). The corpus caught a real regression from #77 on its first run: the usage accounting block ran payload.get("usage") before any dict check, so a null or array body raised a bare AttributeError instead of ProviderError. Both adapters now degrade accounting on non-dict payloads (isinstance guard); the content parse below owns the ProviderError. Migration: a database created at the v0.1-era schema (no digest/stale/ flaky columns; permissions without content_hash or capabilities) opens under current TaskStore + PermissionCenter with every newer dimension off, round-trips an update, and the additive migrations are idempotent on reopen. 745 tests pass (+27); demo 8/8. --- src/nanodot/native/providers/anthropic.py | 6 +- src/nanodot/native/providers/openai_compat.py | 7 +- tests/test_provider_corpus.py | 198 ++++++++++++++++++ 3 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 tests/test_provider_corpus.py diff --git a/src/nanodot/native/providers/anthropic.py b/src/nanodot/native/providers/anthropic.py index 6546731..9e6f88a 100644 --- a/src/nanodot/native/providers/anthropic.py +++ b/src/nanodot/native/providers/anthropic.py @@ -101,7 +101,11 @@ def _messages(self, system: str, user: str) -> str: ) with authenticated_urlopen(request, timeout=self._timeout) as response: payload = json.loads(response.read().decode()) - usage_block = payload.get("usage") or {} + # Same degradation as openai_compat: a non-dict payload skips + # accounting; the content parse below raises the ProviderError. + usage_block = ( + payload.get("usage") or {} if isinstance(payload, dict) else {} + ) if self._usage is not None and usage_block: from nanodot.native.providers.openai_compat import _tokens_of diff --git a/src/nanodot/native/providers/openai_compat.py b/src/nanodot/native/providers/openai_compat.py index bc47bb0..e4b1e61 100644 --- a/src/nanodot/native/providers/openai_compat.py +++ b/src/nanodot/native/providers/openai_compat.py @@ -129,7 +129,12 @@ def _chat(self, system: str, user: str) -> str: ) with authenticated_urlopen(request, timeout=self._timeout) as response: payload = json.loads(response.read().decode()) - _record_usage(self._name, self._usage, payload.get("usage") or {}) + # Usage accounting degrades on non-dict payloads (null, arrays, + # error pages): the content parse below owns the ProviderError. + usage_block = ( + payload.get("usage") or {} if isinstance(payload, dict) else {} + ) + _record_usage(self._name, self._usage, usage_block) except urllib.error.HTTPError as error: detail = self._scrub(error.read().decode(errors="replace"))[:200] retry_after = _retry_after_seconds(error.headers) diff --git a/tests/test_provider_corpus.py b/tests/test_provider_corpus.py new file mode 100644 index 0000000..f493eb8 --- /dev/null +++ b/tests/test_provider_corpus.py @@ -0,0 +1,198 @@ +"""Provider malformed-response corpus + era-jump migration (issue #105). + +Real APIs send dirty data; the contract tests use ideal shapes. Every +corpus row asserts the same three things per adapter: ProviderError +(fail closed), no crash, and no credential material in the error text. +The migration test proves a v0.1-era database opens under current code +with every newer dimension off. +""" + +from __future__ import annotations + +import io +import json +import sqlite3 +from pathlib import Path + +import pytest +from fakes import FakeClock, FakeGitHub, FakeSink + +from nanodot.core.activity import ActivityLog +from nanodot.core.runner import TaskLoop +from nanodot.core.tasks import PRTarget, Task, TaskStore +from nanodot.native.providers.anthropic import AnthropicProvider +from nanodot.native.providers.openai_compat import APIInferenceProvider +from nanodot.ports.inference import ProviderError, StateChange + +CHANGE = StateChange(kind="checks-failed", summary="ci failing", head_sha="s") +API_KEY = "sk-corpus-secret-123" + + +class _Response(io.BytesIO): + def __init__(self, body: bytes | str, status: int = 200) -> None: + raw = body if isinstance(body, bytes) else body.encode() + super().__init__(raw) + self.status = status + + def __enter__(self): + return self + + def __exit__(self, *args): + self.close() + return False + + +OPENAI_GOOD = {"choices": [{"message": {"content": "ok"}}]} +ANTHROPIC_GOOD = {"content": [{"type": "text", "text": "ok"}]} + +# (label, raw body, status) — protocol-agnostic dirt. +CORPUS = [ + ("truncated-json", b'{"choices": [{"message"'), + ("empty-body", b""), + ("html-error-page", b"502 Bad Gateway"), + ("null-instead-of-object", b"null"), + ("array-instead-of-object", b"[1, 2, 3]"), + ("success-status-error-shape", json.dumps({"error": {"message": "nope"}})), + ("wrong-inner-shape", json.dumps({"choices": []})), + ("null-content-field", json.dumps({"choices": [{"message": {"content": None}}]})), + ( + "anthropic-text-not-string", + json.dumps({"content": [{"type": "text", "text": 42}]}), + ), + ("anthropic-no-text-block", json.dumps({"content": [{"type": "image"}]})), + ("empty-text", json.dumps({"choices": [{"message": {"content": " "}}]})), +] + + +def _patch_transport(monkeypatch, module, body, status=200): + def fake_urlopen(request, timeout=None): + return _Response(body, status=status) + + monkeypatch.setattr(module, "authenticated_urlopen", fake_urlopen) + + +@pytest.mark.parametrize("label,body", [(c[0], c[1]) for c in CORPUS]) +def test_openai_compat_corpus_fails_closed(monkeypatch, label, body) -> None: + import nanodot.native.providers.openai_compat as module + + _patch_transport(monkeypatch, module, body) + provider = APIInferenceProvider(api_key=API_KEY, base_url="https://api.test/v1") + with pytest.raises(ProviderError) as caught: + provider.summarize(CHANGE) + assert API_KEY not in str(caught.value) + + +@pytest.mark.parametrize("label,body", [(c[0], c[1]) for c in CORPUS]) +def test_anthropic_corpus_fails_closed(monkeypatch, label, body) -> None: + import nanodot.native.providers.anthropic as module + + _patch_transport(monkeypatch, module, body) + provider = AnthropicProvider(api_key=API_KEY, base_url="https://api.test") + with pytest.raises(ProviderError) as caught: + provider.summarize(CHANGE) + assert API_KEY not in str(caught.value) + + +@pytest.mark.parametrize("body", [b'{"choices":[{}]}', b"", b"upstream"]) +def test_parse_intent_corpus_never_returns_garbage(monkeypatch, body) -> None: + import nanodot.native.providers.openai_compat as module + + _patch_transport(monkeypatch, module, body) + provider = APIInferenceProvider(api_key=API_KEY, base_url="https://api.test/v1") + with pytest.raises(ProviderError): + provider.parse_intent("watch owner/repo#1") + + +def test_good_shapes_still_pass_through_both_adapters(monkeypatch) -> None: + """The corpus must not overfit: the ideal shapes keep working.""" + import nanodot.native.providers.anthropic as amod + import nanodot.native.providers.openai_compat as omod + + _patch_transport(monkeypatch, omod, json.dumps(OPENAI_GOOD).encode()) + assert APIInferenceProvider( + api_key=API_KEY, base_url="https://api.test/v1" + ).summarize(CHANGE) == "ok" + + _patch_transport(monkeypatch, amod, json.dumps(ANTHROPIC_GOOD).encode()) + assert AnthropicProvider( + api_key=API_KEY, base_url="https://api.test" + ).summarize(CHANGE) == "ok" + + +# -- era-jump migration ----------------------------------------------------------- + + +_V01_TASKS = """ +CREATE TABLE IF NOT EXISTS tasks ( + id TEXT PRIMARY KEY, + target TEXT NOT NULL, + purpose TEXT NOT NULL, + cadence_seconds INTEGER NOT NULL, + allowed_actions TEXT NOT NULL, + notification_conditions TEXT NOT NULL, + stop_conditions TEXT NOT NULL, + state TEXT NOT NULL, + blocker TEXT, + scope_version INTEGER NOT NULL DEFAULT 1, + created_at REAL NOT NULL, + updated_at REAL NOT NULL, + next_check_at REAL, + watch_state TEXT NOT NULL DEFAULT '{}' +); +""" +_V01_PERMISSIONS = """ +CREATE TABLE IF NOT EXISTS grants ( + id TEXT PRIMARY KEY, action TEXT NOT NULL, target TEXT NOT NULL, + scope TEXT NOT NULL, task_id TEXT NOT NULL, created_at REAL NOT NULL, + expires_at REAL, revoked_at REAL +); +CREATE TABLE IF NOT EXISTS requests ( + id TEXT PRIMARY KEY, action TEXT NOT NULL, target TEXT NOT NULL, + scope TEXT NOT NULL, task_id TEXT NOT NULL, created_at REAL NOT NULL, + expires_at REAL NOT NULL, state TEXT NOT NULL +); +""" + + +def test_v01_database_jumps_to_current_schema(home: Path) -> None: + db = home / "nanodot.db" + db.parent.mkdir(parents=True, exist_ok=True) + conn = sqlite3.connect(db) + conn.executescript(_V01_TASKS + _V01_PERMISSIONS) + from nanodot.core.tasks import ( + DEFAULT_NOTIFICATION_CONDITIONS, + DEFAULT_STOP_CONDITIONS, + ) + + conn.execute( + "INSERT INTO tasks VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?)", + ( + "legacy1", "o/r#1", "legacy", 300, '["read"]', + DEFAULT_NOTIFICATION_CONDITIONS, DEFAULT_STOP_CONDITIONS, + "active", None, 1, 1.0, 1.0, 1.0, "{}", + ), + ) + conn.commit() + conn.close() + + store = TaskStore(path=db) + task = store.get("legacy1") + assert task is not None + # Every newer dimension is off, and the legacy row round-trips. + assert task.digest_interval_seconds is None + assert task.stale_after_seconds is None + assert task.flaky_alerts is False + task.purpose = "still works" + store.update(task) + assert store.get("legacy1").purpose == "still works" + + from nanodot.core.permissions import PermissionCenter + + center = PermissionCenter(path=db) + assert center.grants() == [] # old tables migrated, readable + # Reopen: the additive migrations are idempotent. + store.close() + center.close() + store2 = TaskStore(path=db) + assert store2.get("legacy1").digest_interval_seconds is None + store2.close()