From fe341bc295f95e854c0587c4df3147994d57f841 Mon Sep 17 00:00:00 2001 From: Andrii Pasternak Date: Mon, 4 May 2026 14:04:54 +0100 Subject: [PATCH 1/2] fix(webhooks): delegate webhook token methods on DatabaseManager (#647) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WEBHOOK-001 (#291) added four methods to ScheduleOperations (generate_webhook_token, get_schedule_by_webhook_token, revoke_webhook_token, get_webhook_status) but never added the matching pass-throughs on the DatabaseManager facade. Because there is no __getattr__ proxy, every webhook endpoint blew up with AttributeError on a live stack. The regression went undetected because integration tests don't run in CI. Add the four pass-through methods (12 lines, no logic changes), plus an AST-based unit test that statically verifies every db.(...) call in routers/ and services/ resolves to a real method on DatabaseManager. The new test is split into a strict regression check for the four WEBHOOK-001 methods and a broader scan of all db.* call sites guarded by a KNOWN_FACADE_GAPS allowlist for eight unrelated pre-existing gaps — those are real but out of scope for this patch and should be cleaned up in a follow-up issue. Closes #647 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/backend/database.py | 13 ++ tests/unit/test_database_facade_delegation.py | 139 ++++++++++++++++++ 2 files changed, 152 insertions(+) create mode 100644 tests/unit/test_database_facade_delegation.py diff --git a/src/backend/database.py b/src/backend/database.py index a70ecb8a2..970f37f1e 100644 --- a/src/backend/database.py +++ b/src/backend/database.py @@ -697,6 +697,19 @@ def update_schedule(self, schedule_id: str, username: str, updates: dict): def delete_schedule(self, schedule_id: str, username: str): return self._schedule_ops.delete_schedule(schedule_id, username) + # Webhook token management (WEBHOOK-001, #291) + def generate_webhook_token(self, schedule_id: str): + return self._schedule_ops.generate_webhook_token(schedule_id) + + def get_schedule_by_webhook_token(self, token: str): + return self._schedule_ops.get_schedule_by_webhook_token(token) + + def revoke_webhook_token(self, schedule_id: str): + return self._schedule_ops.revoke_webhook_token(schedule_id) + + def get_webhook_status(self, schedule_id: str): + return self._schedule_ops.get_webhook_status(schedule_id) + def set_schedule_enabled(self, schedule_id: str, enabled: bool): return self._schedule_ops.set_schedule_enabled(schedule_id, enabled) diff --git a/tests/unit/test_database_facade_delegation.py b/tests/unit/test_database_facade_delegation.py new file mode 100644 index 000000000..bc4658ea1 --- /dev/null +++ b/tests/unit/test_database_facade_delegation.py @@ -0,0 +1,139 @@ +"""Lint-style guard: every `db.(...)` call site in routers/ and +services/ must resolve to a real method on `DatabaseManager`. + +Background: WEBHOOK-001 (#291) added `generate_webhook_token`, +`get_schedule_by_webhook_token`, `revoke_webhook_token`, and +`get_webhook_status` to `ScheduleOperations` in `src/backend/db/schedules.py` +but forgot to add the matching pass-through methods on the `DatabaseManager` +facade in `src/backend/database.py`. Because there is no `__getattr__` proxy, +every webhook endpoint blew up with `AttributeError` on a live stack — and +this went undetected because integration tests don't run in CI. + +This test statically scans every `db.(...)` call in routers and +services and asserts the attribute exists on `DatabaseManager`. AST-based, +no imports of backend modules required (so it runs without a venv). + +Issue: https://github.com/abilityai/trinity/issues/647 +""" + +from __future__ import annotations + +import ast +from pathlib import Path +from typing import Set + +PROJECT_ROOT = Path(__file__).resolve().parents[2] +BACKEND = PROJECT_ROOT / "src" / "backend" +DATABASE_PY = BACKEND / "database.py" +SCAN_DIRS = [BACKEND / "routers", BACKEND / "services"] + +# Pre-existing facade gaps discovered while writing this test for #647. +# Each entry is a real `AttributeError`-at-runtime bug, but fixing them is +# out of scope for the WEBHOOK-001 patch. Tracked separately so this lint +# test catches NEW regressions without forcing one giant cleanup PR. +# +# REMOVE entries from this set as the corresponding methods are added to +# DatabaseManager. Do NOT add new entries — fix the facade instead. +KNOWN_FACADE_GAPS: frozenset[str] = frozenset( + { + "create_validation_execution", + "get_agent_folder_config", + "get_agent_last_activity", + "get_agent_permissions", + "get_agent_schedules", + "get_full_capabilities", + "set_full_capabilities", + "update_business_status", + } +) + + +def _databasemanager_methods() -> Set[str]: + """Return the set of method names defined on the DatabaseManager class. + + Only includes methods declared directly in `database.py`. Methods + inherited via mixins or proxied through `__getattr__` would require + runtime imports and are out of scope for this static check. + """ + tree = ast.parse(DATABASE_PY.read_text()) + for node in ast.walk(tree): + if isinstance(node, ast.ClassDef) and node.name == "DatabaseManager": + return { + child.name + for child in node.body + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + raise AssertionError("DatabaseManager class not found in database.py") + + +def _db_attribute_calls(py_file: Path) -> Set[str]: + """Extract every `db.(...)` call attribute name in a Python file. + + Matches AST nodes shaped as `Call(func=Attribute(value=Name(id='db'), attr=...))`. + Ignores attribute access without a call (e.g., `db.X` standalone) and + keyword arguments named `db` (e.g., `redis.Redis(db=0)`). + """ + try: + tree = ast.parse(py_file.read_text()) + except SyntaxError: + return set() + + attrs: Set[str] = set() + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + func = node.func + if ( + isinstance(func, ast.Attribute) + and isinstance(func.value, ast.Name) + and func.value.id == "db" + ): + attrs.add(func.attr) + return attrs + + +def test_every_db_call_resolves_on_databasemanager(): + """Every `db.(...)` call in routers/ and services/ must exist + on DatabaseManager. Catches the WEBHOOK-001 facade-delegation regression. + """ + methods = _databasemanager_methods() + assert methods, "Failed to extract DatabaseManager methods" + + missing: dict[str, list[str]] = {} + for scan_dir in SCAN_DIRS: + for py in scan_dir.rglob("*.py"): + for attr in _db_attribute_calls(py): + if attr not in methods and attr not in KNOWN_FACADE_GAPS: + missing.setdefault(attr, []).append( + str(py.relative_to(PROJECT_ROOT)) + ) + + assert not missing, ( + "New db.(...) call sites found with no matching method on " + "DatabaseManager (facade gap — calls will fail with AttributeError " + "at runtime). Either add a pass-through method on DatabaseManager, " + "or — only if the gap pre-exists this PR — add the name to " + "KNOWN_FACADE_GAPS at the top of this file:\n" + + "\n".join( + f" - db.{name}() called from: {', '.join(sorted(set(files)))}" + for name, files in sorted(missing.items()) + ) + ) + + +def test_webhook_001_methods_delegated(): + """Explicit regression check for #647: the four WEBHOOK-001 methods must + be delegated on DatabaseManager. + """ + methods = _databasemanager_methods() + required = { + "generate_webhook_token", + "get_schedule_by_webhook_token", + "revoke_webhook_token", + "get_webhook_status", + } + missing = required - methods + assert not missing, ( + f"WEBHOOK-001 methods missing from DatabaseManager: {sorted(missing)}. " + "Add pass-through methods that delegate to self._schedule_ops." + ) From 9d7df6f0b472035e15b23c1303b37c71f996c51c Mon Sep 17 00:00:00 2001 From: Andrii Pasternak Date: Mon, 4 May 2026 14:14:48 +0100 Subject: [PATCH 2/2] test(catalog): register test_database_facade_delegation for #647 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the new lint-style facade-delegation test to tests/registry.json and .claude/agents/test-runner.md so future test-runner sweeps know it exists. Pure documentation/catalog — no test logic changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/agents/test-runner.md | 12 ++++++++++++ tests/registry.json | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/.claude/agents/test-runner.md b/.claude/agents/test-runner.md index 6900fb39b..37de9ab6a 100644 --- a/.claude/agents/test-runner.md +++ b/.claude/agents/test-runner.md @@ -265,6 +265,18 @@ Use these thresholds to assess test health (based on **executed** tests, not inc - **Warning**: 75-90% pass rate, <5 failures - **Critical**: <75% pass rate or >5 failures +## Recent Test Additions (2026-05-04) + +| Test File | Description | Tests Added | +|-----------|-------------|-------------| +| `unit/test_database_facade_delegation.py` | AST-based lint guard for the `DatabaseManager` facade (#647) — catches the WEBHOOK-001 regression class where methods exist on the underlying `*Operations` class but the pass-through delegation on `DatabaseManager` is missing, blowing up at runtime with `AttributeError`. Two tests: strict regression check for the four #647 methods (`generate_webhook_token`, `get_schedule_by_webhook_token`, `revoke_webhook_token`, `get_webhook_status`), and a broad scan of every `db.(...)` call site in `routers/` and `services/` guarded by a `KNOWN_FACADE_GAPS` allowlist for 8 unrelated pre-existing gaps. Pure `ast` static analysis — no backend deps required, runs in any Python with pytest. | 2 tests | + +**Webhook facade-delegation fix (#647)** — `src/backend/database.py`: + +Root cause of all `POST/GET/DELETE /api/agents/{name}/schedules/{id}/webhook` and `POST /api/webhooks/{token}` returning 500 on a live stack since #291 (Nov 2025). WEBHOOK-001 added the four methods to `ScheduleOperations` but never added the matching pass-throughs on the `DatabaseManager` facade — and there's no `__getattr__` proxy. The integration test from PR #643 would have caught it but doesn't run in CI. New unit test runs in any Python (no live stack), would have caught the regression at PR-time. + +--- + ## Recent Test Additions (2026-04-27) | Test File | Description | Tests Added | diff --git a/tests/registry.json b/tests/registry.json index 139e56380..a91163824 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -258,6 +258,13 @@ "added": "2026-05-03", "categories": ["backend", "api", "public-links", "proxy"], "description": "Tests for agent website proxy (#633): site link creation, link_type field, URL format, token validation (invalid/wrong-type/disabled), 502 when web server not running, redirect on missing trailing slash." + }, + { + "file": "unit/test_database_facade_delegation.py", + "feature": "Issue #647 / WEBHOOK-001 facade gap", + "added": "2026-05-04", + "categories": ["backend", "unit", "database", "webhooks", "lint"], + "description": "AST-based lint guard (no backend deps required) that asserts every db.(...) call in src/backend/routers/ and src/backend/services/ resolves to a real method on DatabaseManager. Two tests: strict regression check for the four WEBHOOK-001 methods (#647: generate_webhook_token, get_schedule_by_webhook_token, revoke_webhook_token, get_webhook_status) and a broad facade-resolution scan guarded by a KNOWN_FACADE_GAPS allowlist for eight unrelated pre-existing gaps. Catches AttributeError-at-runtime regressions that integration-only tests miss in CI — would have caught WEBHOOK-001 before #291 landed." } ] }