From 02c4b4fe68a6b43056645c80db5a60e5d532eaa7 Mon Sep 17 00:00:00 2001 From: Robert Date: Sat, 18 Jul 2026 03:40:55 -0300 Subject: [PATCH] Harden QuoteJudge managed persistence --- a2a.yaml | 2 +- agent.py | 42 +++++++++------- frontend/package.json | 2 +- requirements.txt | 5 -- tests/test_full_stack_contract.py | 84 ++++++++++++++++++++++++++++--- 5 files changed, 104 insertions(+), 31 deletions(-) diff --git a/a2a.yaml b/a2a.yaml index 6520e25..d705863 100644 --- a/a2a.yaml +++ b/a2a.yaml @@ -1,5 +1,5 @@ name: quote-judge-studio-v1 -version: 0.1.0 +version: 0.1.4 entrypoint: agent:QuoteJudgeStudioV1 expose: public: false diff --git a/agent.py b/agent.py index 78e03db..9f2bf2d 100644 --- a/agent.py +++ b/agent.py @@ -54,10 +54,10 @@ ALLOWED_UPLOAD_MEDIA_TYPES = { "application/vnd.ms-excel", "text/plain", } - -# Local/dev fallback only. Hosted production uses DATABASE_URL and managed Postgres. -_LOCAL_COMPARISONS: dict[tuple[str, str], dict[str, Any]] = {} -_LOCAL_RECEIPTS: list[dict[str, Any]] = [] +AGENT_VERSION = "0.1.4" +DB_CONNECT_TIMEOUT_SECONDS = 5 +DB_STATEMENT_TIMEOUT_MS = 10_000 +DB_LOCK_TIMEOUT_MS = 5_000 class QuoteJudgeStudioV1Config(BaseModel): @@ -128,7 +128,7 @@ class QuoteJudgeStudioV1(A2AAgent[QuoteJudgeStudioV1Config, PlatformUserAuth]): "scoring, persists per-user comparisons, supports browser uploads, and serves " "a polished packed product UI." ) - version = "0.1.3" + version = AGENT_VERSION config_model = QuoteJudgeStudioV1Config auth_model = PlatformUserAuth @@ -229,6 +229,12 @@ class QuoteJudgeStudioV1(A2AAgent[QuoteJudgeStudioV1Config, PlatformUserAuth]): record = await asyncio.to_thread(_load_comparison, tenant, comparison_id) if record is None: return {"ok": False, "code": "comparison_not_found", "comparison_id": comparison_id} + if record.get("ok") is False: + return { + "ok": False, + "code": record.get("code", "persistence_unavailable"), + "comparison_id": comparison_id, + } result = dict(record["result"]) result["comparison_id"] = comparison_id result["ok"] = True @@ -477,7 +483,15 @@ def _connect() -> Any: url = _database_url() if not url: raise RuntimeError("DATABASE_URL is not configured") - return psycopg.connect(url, row_factory=dict_row) + return psycopg.connect( + url, + row_factory=dict_row, + connect_timeout=DB_CONNECT_TIMEOUT_SECONDS, + options=( + f"-c statement_timeout={DB_STATEMENT_TIMEOUT_MS} " + f"-c lock_timeout={DB_LOCK_TIMEOUT_MS}" + ), + ) def _persist_comparison( @@ -487,13 +501,7 @@ def _persist_comparison( receipt: dict[str, Any], ) -> dict[str, Any]: if not _database_url() or psycopg is None: - _LOCAL_COMPARISONS[(tenant_key, comparison_id)] = { - "result": json.loads(json.dumps(result)), - "updated_at": receipt["created_at"], - "latest_receipt_id": receipt["receipt_id"], - } - _LOCAL_RECEIPTS.append(receipt) - return {"ok": True, "warning": "local_ephemeral_persistence"} + return {"ok": False, "code": "persistence_unavailable"} try: with _connect() as conn: with conn.cursor() as cur: @@ -550,12 +558,12 @@ def _persist_comparison( conn.commit() return {"ok": True} except Exception: - return {"ok": False, "warning": "persistence_unavailable"} + return {"ok": False, "code": "persistence_unavailable"} def _load_comparison(tenant_key: str, comparison_id: str) -> dict[str, Any] | None: if not _database_url() or psycopg is None: - return _LOCAL_COMPARISONS.get((tenant_key, comparison_id)) + return {"ok": False, "code": "persistence_unavailable"} try: with _connect() as conn: with conn.cursor() as cur: @@ -577,7 +585,7 @@ def _load_comparison(tenant_key: str, comparison_id: str) -> dict[str, Any] | No "latest_receipt_id": row.get("latest_receipt_id"), } except Exception: - return None + return {"ok": False, "code": "persistence_unavailable"} def _build_receipt( @@ -603,7 +611,7 @@ def _build_receipt( "schema": "quotejudge.production_receipt.v1", "receipt_id": receipt_id, "agent_name": "quote-judge-studio-v1", - "agent_version": "0.1.0", + "agent_version": AGENT_VERSION, "skill_name": skill_name, "comparison_id": comparison_id, "status": "ok" if result.get("ok") else "error", diff --git a/frontend/package.json b/frontend/package.json index 0a92df0..05f1d74 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "quote-judge-studio-v1-frontend", "private": true, - "version": "0.1.0", + "version": "0.1.4", "type": "module", "scripts": { "dev": "vite --host 0.0.0.0 --port 5173", diff --git a/requirements.txt b/requirements.txt index e6ccb60..fb6a7c8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,2 @@ # a2a-pack is auto-installed by the deploy build. -# These starter deps power the DeepAgents tool-calling example in agent.py. -deepagents>=0.5.0 -langchain>=0.3 -langchain-openai>=0.2 -langgraph>=0.6 psycopg[binary]>=3.2 diff --git a/tests/test_full_stack_contract.py b/tests/test_full_stack_contract.py index 20b441f..9229441 100644 --- a/tests/test_full_stack_contract.py +++ b/tests/test_full_stack_contract.py @@ -2,12 +2,14 @@ import asyncio import base64 import json from pathlib import Path +from unittest.mock import Mock from a2a_pack.auth import PlatformUserAuth from a2a_pack.cli.local import load_local_project from a2a_pack.context import LocalRunContext -from agent import QuoteJudgeStudioV1, _LOCAL_COMPARISONS, _LOCAL_RECEIPTS +import agent as agent_module +from agent import QuoteJudgeStudioV1 ROOT = Path(__file__).resolve().parents[1] @@ -54,11 +56,28 @@ def test_full_stack_product_contract(): assert databases[0].migrations is not None assert databases[0].migrations.path == "db/migrations" assert card.runtime.pricing.caller_pays_llm is False + assert card.version == "0.1.4" -def test_success_failure_reload_and_receipt_contract(): - _LOCAL_COMPARISONS.clear() - _LOCAL_RECEIPTS.clear() +def test_success_failure_reload_and_receipt_contract(monkeypatch): + comparisons = {} + receipts = [] + + def persist(tenant_key, comparison_id, result, receipt): + comparisons[(tenant_key, comparison_id)] = { + "result": json.loads(json.dumps(result)), + "updated_at": receipt["created_at"], + "latest_receipt_id": receipt["receipt_id"], + } + receipts.append(receipt) + return {"ok": True} + + monkeypatch.setattr(agent_module, "_persist_comparison", persist) + monkeypatch.setattr( + agent_module, + "_load_comparison", + lambda tenant_key, comparison_id: comparisons.get((tenant_key, comparison_id)), + ) agent = QuoteJudgeStudioV1() quotes = [ {"vendor": "Acme Bearings", "quantity": 500, "unit_price": 10.75, "delivery_days": 12, "warranty_months": 12}, @@ -72,7 +91,8 @@ def test_success_failure_reload_and_receipt_contract(): assert success["recommendation"]["vendor"] == "Beta Industrial" assert success["receipt"]["receipt_id"].startswith("qjr_") assert success["receipt"]["persisted"] is True - assert _LOCAL_RECEIPTS and _LOCAL_RECEIPTS[-1]["schema"] == "quotejudge.production_receipt.v1" + assert receipts and receipts[-1]["schema"] == "quotejudge.production_receipt.v1" + assert receipts[-1]["agent_version"] == "0.1.4" assert "tenant_key" not in json.dumps(success) reload = run(agent.local_invoke("get_comparison", auth=AUTH, comparison_id="studio-quote-v1")) @@ -91,8 +111,8 @@ def test_success_failure_reload_and_receipt_contract(): assert failure == {"ok": False, "code": "at_least_two_quotes_required", "comparison_id": "studio-quote-invalid"} -def test_browser_upload_bridge_success_and_invalid_base64(): - _LOCAL_COMPARISONS.clear() +def test_browser_upload_bridge_success_and_invalid_base64(monkeypatch): + monkeypatch.setattr(agent_module, "_persist_comparison", lambda *_args: {"ok": True}) agent = QuoteJudgeStudioV1() payload = base64.b64encode(json.dumps({ "quotes": [ @@ -124,6 +144,56 @@ def test_browser_upload_bridge_success_and_invalid_base64(): assert invalid["code"] == "invalid_base64" +def test_persistence_fails_closed_without_managed_database(monkeypatch): + monkeypatch.delenv("DATABASE_URL", raising=False) + receipt = {"receipt_id": "qjr_test", "created_at": "2026-01-01T00:00:00+00:00"} + + persisted = agent_module._persist_comparison("user:123", "quote-1", {}, receipt) + loaded = agent_module._load_comparison("user:123", "quote-1") + + assert persisted == {"ok": False, "code": "persistence_unavailable"} + assert loaded == {"ok": False, "code": "persistence_unavailable"} + + +def test_database_exception_returns_stable_failure_code(monkeypatch): + monkeypatch.setenv("DATABASE_URL", "postgresql://example.invalid/db") + monkeypatch.setattr(agent_module, "psycopg", Mock()) + + def unavailable(): + raise TimeoutError("database connection timed out") + + monkeypatch.setattr(agent_module, "_connect", unavailable) + persisted = agent_module._persist_comparison( + "user:123", + "quote-1", + {"weights": {}, "quote_count": 2, "recommendation": {"vendor": "Acme"}}, + { + "receipt_id": "qjr_test", + "created_at": "2026-01-01T00:00:00+00:00", + "skill_name": "compare_quotes", + "status": "ok", + "input_hash": "input", + "result_hash": "result", + }, + ) + + assert persisted == {"ok": False, "code": "persistence_unavailable"} + + +def test_database_connection_has_hard_timeouts(monkeypatch): + fake_psycopg = Mock() + fake_psycopg.connect.return_value = object() + monkeypatch.setattr(agent_module, "psycopg", fake_psycopg) + monkeypatch.setenv("DATABASE_URL", "postgresql://example.invalid/db") + + agent_module._connect() + + _, kwargs = fake_psycopg.connect.call_args + assert kwargs["connect_timeout"] == 5 + assert "statement_timeout=10000" in kwargs["options"] + assert "lock_timeout=5000" in kwargs["options"] + + def test_mcp_tool_schemas_are_bounded_and_typed(): card = load_local_project(ROOT).agent_cls().card() skills = {skill.name: skill for skill in card.skills}