From e86dd5edc58ad6280bad3d7987821e3254afc751 Mon Sep 17 00:00:00 2001 From: ZdenekSrotyr Date: Tue, 5 May 2026 08:00:57 +0200 Subject: [PATCH] fix(anthropic): strict json_schema (additionalProperties=false) + add /admin/scheduler-runs UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E2E test on a real BQ deploy showed every verification-extraction call fails with HTTP 400 invalid_request_error: "output_config.format.schema: For 'object' type, 'additionalProperties' must be explicitly set to false". The Anthropic structured-output API now requires the field on every object node in the json_schema. Fix: connectors/llm/anthropic_provider.py wraps the caller-supplied schema through a recursive _strict_json_schema() walker that adds the field where missing (preserving any explicit override), then passes the strict variant to the API. Six unit tests in TestStrictJsonSchema pin the recursion across nested objects, array items, and the no-mutation invariant. Adds /admin/scheduler-runs — a read-only admin page that surfaces the last 200 audit-log entries from scheduler-driven actions. New AuditRepository.query_actions(actions, limit) helper, new admin nav entry. Failed scheduler ticks (HTTP 401, network errors) don't reach the audit_log; the page calls that out with a hint to set SCHEDULER_API_TOKEN if no rows show up. --- CHANGELOG.md | 5 ++ app/web/router.py | 32 ++++++++ app/web/templates/_app_header.html | 3 +- app/web/templates/admin_scheduler_runs.html | 86 +++++++++++++++++++++ connectors/llm/anthropic_provider.py | 19 ++++- src/repositories/audit.py | 16 ++++ tests/test_llm_connector.py | 57 ++++++++++++++ tests/test_web_ui.py | 12 +++ 8 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 app/web/templates/admin_scheduler_runs.html diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c6a1a0..b01bdd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,11 @@ Five-defect fix for the silently-broken session pipeline on default Compose depl - **Defect 1 — `/corporate-memory` filtered `status IN ('approved','mandatory')` with no hint that pending items existed.** Admin banner added (Added above). - **#179 review — `/api/admin/run-session-collector` would SystemExit the worker.** The endpoint called `collector.main()`, whose `argparse.parse_args()` parsed uvicorn's `sys.argv` (`['app.main:app', '--host', …]`) and called `sys.exit(2)` on the unrecognised flags. `SystemExit` inherits from `BaseException`, escapes FastAPI's exception machinery, and propagates through the thread pool — every scheduler tick that fired the endpoint either 500-ed or risked killing the uvicorn worker. Fix: `services/session_collector/collector.py` now exposes an argv-free `run(dry_run, verbose) -> (rc, stats)` helper; `main()` is a thin CLI shim around it and the admin endpoint calls `run()` directly. Audit log now carries the per-run stats (`users_processed`, `files_copied`, `files_skipped`) instead of just the rc. Regression tests in `tests/test_session_collector.py::TestRunHelper`. - **#179 review — `python -m services.corporate_memory` crashed on missing LLM config instead of exiting cleanly.** The PR's fail-fast change made `collect_all()` raise `ValueError` when neither an `ai:` block nor `ANTHROPIC_API_KEY`/`LLM_API_KEY` was available. The `verification_detector` CLI was updated to catch it; the corporate-memory CLI was missed. Now also wrapped — operators get a one-line `Corporate Memory cannot run: ` on stderr and rc=1 instead of a raw traceback. Regression test in `tests/test_llm_connector.py::TestCorporateMemoryCollector::test_main_returns_1_on_no_ai_config_instead_of_traceback`. +- **E2E test — Anthropic API rejected every extraction request.** The structured-output API now requires `additionalProperties: false` on every `{"type": "object"}` node in the json_schema; without it the API returns 400 `invalid_request_error` ("output_config.format.schema: For 'object' type, 'additionalProperties' must be explicitly set to false"). Surfaced on a real BQ-backed deploy: every uploaded session jsonl failed verification-extraction in a tight retry loop. Fix: `connectors/llm/anthropic_provider.py` now wraps the caller-supplied schema through a recursive `_strict_json_schema()` walker that adds the field where missing (preserving any explicit operator override), then passes the strict variant to the API. Six unit tests in `tests/test_llm_connector.py::TestStrictJsonSchema` pin the recursion across nested objects, array items, and the no-mutation invariant. + +### Added + +- `/admin/scheduler-runs` — read-only admin page showing the last 200 audit-log entries from scheduler-driven actions (`run_session_collector`, `run_verification_detector`, `run_corporate_memory`, `marketplaces_sync_all`, `data_refresh`, `scripts_run_due`). New `AuditRepository.query_actions(actions, limit)` query helper, new admin nav entry under the Admin dropdown. Failed scheduler ticks (HTTP 401, network errors) don't reach the audit_log — those still live only in `docker logs agnes-scheduler-1`; the page calls that out with a hint to set `SCHEDULER_API_TOKEN` if no rows show up. ### Internal diff --git a/app/web/router.py b/app/web/router.py index 2b72f1e..6279a7f 100644 --- a/app/web/router.py +++ b/app/web/router.py @@ -946,6 +946,38 @@ async def admin_marketplaces_page( return templates.TemplateResponse(request, "admin_marketplaces.html", ctx) +# Scheduler-driven admin actions audited by app/api/admin.py and +# app/api/marketplaces.py. Keep in sync with the JOBS list in +# services/scheduler/__main__.py. +SCHEDULER_AUDIT_ACTIONS = [ + "run_session_collector", + "run_verification_detector", + "run_corporate_memory", + "marketplaces_sync_all", + "data_refresh", + "scripts_run_due", +] + + +@router.get("/admin/scheduler-runs", response_class=HTMLResponse) +async def admin_scheduler_runs_page( + request: Request, + user: dict = Depends(require_admin), + conn: duckdb.DuckDBPyConnection = Depends(_get_db), +): + """Read-only view of the audit_log filtered to scheduler-driven actions. + + Failed scheduler ticks (HTTP 401, network errors) don't reach this view — + they live only in the scheduler container's stdout. The audit_log shows + only what reached the admin endpoint and was processed. + """ + from src.repositories.audit import AuditRepository + + rows = AuditRepository(conn).query_actions(SCHEDULER_AUDIT_ACTIONS, limit=200) + ctx = _build_context(request, user=user, rows=rows, actions=SCHEDULER_AUDIT_ACTIONS) + return templates.TemplateResponse(request, "admin_scheduler_runs.html", ctx) + + @router.get("/admin/agent-prompt", response_class=HTMLResponse) async def admin_agent_prompt_page( request: Request, diff --git a/app/web/templates/_app_header.html b/app/web/templates/_app_header.html index 8d360fb..024f154 100644 --- a/app/web/templates/_app_header.html +++ b/app/web/templates/_app_header.html @@ -14,7 +14,7 @@ Setup local agent {% if session.user.is_admin %} Marketplaces - {% set _admin_active = _path.startswith('/admin/tables') or _path.startswith('/admin/tokens') or _path.startswith('/admin/users') or _path.startswith('/admin/groups') or _path.startswith('/admin/access') or _path.startswith('/admin/server-config') or _path.startswith('/admin/agent-prompt') or _path.startswith('/admin/workspace-prompt') %} + {% set _admin_active = _path.startswith('/admin/tables') or _path.startswith('/admin/tokens') or _path.startswith('/admin/users') or _path.startswith('/admin/groups') or _path.startswith('/admin/access') or _path.startswith('/admin/server-config') or _path.startswith('/admin/agent-prompt') or _path.startswith('/admin/workspace-prompt') or _path.startswith('/admin/scheduler-runs') %} {% endif %} diff --git a/app/web/templates/admin_scheduler_runs.html b/app/web/templates/admin_scheduler_runs.html new file mode 100644 index 0000000..7af2190 --- /dev/null +++ b/app/web/templates/admin_scheduler_runs.html @@ -0,0 +1,86 @@ +{% extends "base.html" %} +{% block title %}Scheduler runs — {{ config.INSTANCE_NAME }}{% endblock %} + +{% block content %} + + +
+

Scheduler runs

+

+ Last 200 audited scheduler-driven admin actions, newest first. + Tracked actions: {% for a in actions %}{{ a }}{% if not loop.last %} {% endif %}{% endfor %}. + Failed ticks (HTTP 401, network errors) live only in the scheduler container's + stdout — docker logs agnes-scheduler-1. Set SCHEDULER_API_TOKEN + in .env if you see no rows here. +

+ +
+ {% if rows %} + + + + + + + + + + + + {% for r in rows %} + + + + + + + + {% endfor %} + +
WhenActionResourceDurationResult / params
{{ r.timestamp.strftime("%Y-%m-%d %H:%M:%S") if r.timestamp else "" }}{{ r.action }}{{ r.resource or "" }}{% if r.duration_ms is not none %}{{ r.duration_ms }} ms{% endif %}{{ r.params or r.result or "" }}
+ {% else %} +
No scheduler runs in audit_log yet. The scheduler may not be authenticated — check SCHEDULER_API_TOKEN.
+ {% endif %} +
+
+{% endblock %} diff --git a/connectors/llm/anthropic_provider.py b/connectors/llm/anthropic_provider.py index df0e050..ee1539c 100644 --- a/connectors/llm/anthropic_provider.py +++ b/connectors/llm/anthropic_provider.py @@ -26,6 +26,23 @@ INITIAL_BACKOFF_SECONDS = 2 BACKOFF_MULTIPLIER = 2 +def _strict_json_schema(schema): + """Return a copy of the schema with additionalProperties=False on every object type. + + The Anthropic structured-output API rejects schemas where a `{"type": "object"}` node + omits `additionalProperties` (HTTP 400 invalid_request_error). We walk the schema + recursively and force the field where missing. + """ + if isinstance(schema, dict): + out = {k: _strict_json_schema(v) for k, v in schema.items()} + if out.get("type") == "object" and "additionalProperties" not in out: + out["additionalProperties"] = False + return out + if isinstance(schema, list): + return [_strict_json_schema(item) for item in schema] + return schema + + class AnthropicExtractor: """Structured JSON extractor using the Anthropic API. @@ -116,7 +133,7 @@ class AnthropicExtractor: output_config={ "format": { "type": "json_schema", - "schema": json_schema, + "schema": _strict_json_schema(json_schema), }, }, ) diff --git a/src/repositories/audit.py b/src/repositories/audit.py index 4473887..21bfc33 100644 --- a/src/repositories/audit.py +++ b/src/repositories/audit.py @@ -52,3 +52,19 @@ class AuditRepository: return [] columns = [desc[0] for desc in self.conn.description] return [dict(zip(columns, row)) for row in results] + + def query_actions( + self, + actions: List[str], + limit: int = 200, + ) -> List[Dict[str, Any]]: + """Return rows whose action is in the given list, newest first.""" + if not actions: + return [] + placeholders = ",".join("?" for _ in actions) + sql = f"SELECT * FROM audit_log WHERE action IN ({placeholders}) ORDER BY timestamp DESC LIMIT ?" + results = self.conn.execute(sql, list(actions) + [limit]).fetchall() + if not results: + return [] + columns = [desc[0] for desc in self.conn.description] + return [dict(zip(columns, row)) for row in results] diff --git a/tests/test_llm_connector.py b/tests/test_llm_connector.py index 75352a2..fd86fe6 100644 --- a/tests/test_llm_connector.py +++ b/tests/test_llm_connector.py @@ -166,6 +166,63 @@ class TestCreateExtractor: # =================================================================== +class TestStrictJsonSchema: + """The Anthropic API rejects object schemas without additionalProperties=False.""" + + def test_adds_to_top_level_object(self): + from connectors.llm.anthropic_provider import _strict_json_schema + + out = _strict_json_schema({"type": "object", "properties": {"a": {"type": "string"}}}) + assert out["additionalProperties"] is False + + def test_recurses_into_nested_objects(self): + from connectors.llm.anthropic_provider import _strict_json_schema + + schema = { + "type": "object", + "properties": { + "nested": { + "type": "object", + "properties": {"deep": {"type": "object", "properties": {}}}, + }, + }, + } + out = _strict_json_schema(schema) + assert out["additionalProperties"] is False + assert out["properties"]["nested"]["additionalProperties"] is False + assert out["properties"]["nested"]["properties"]["deep"]["additionalProperties"] is False + + def test_recurses_into_array_items(self): + from connectors.llm.anthropic_provider import _strict_json_schema + + schema = { + "type": "object", + "properties": {"items": {"type": "array", "items": {"type": "object", "properties": {}}}}, + } + out = _strict_json_schema(schema) + assert out["properties"]["items"]["items"]["additionalProperties"] is False + + def test_preserves_explicit_additional_properties(self): + from connectors.llm.anthropic_provider import _strict_json_schema + + schema = {"type": "object", "additionalProperties": True, "properties": {}} + out = _strict_json_schema(schema) + assert out["additionalProperties"] is True + + def test_does_not_mutate_input(self): + from connectors.llm.anthropic_provider import _strict_json_schema + + schema = {"type": "object", "properties": {}} + _strict_json_schema(schema) + assert "additionalProperties" not in schema + + def test_non_object_schemas_untouched(self): + from connectors.llm.anthropic_provider import _strict_json_schema + + out = _strict_json_schema({"type": "string"}) + assert "additionalProperties" not in out + + class TestAnthropicExtractor: """Tests for connectors.llm.anthropic_provider.AnthropicExtractor.""" diff --git a/tests/test_web_ui.py b/tests/test_web_ui.py index bbcc6eb..70b825e 100644 --- a/tests/test_web_ui.py +++ b/tests/test_web_ui.py @@ -327,6 +327,18 @@ class TestAdminRoleGuards: r = web_client.get("/admin/agent-prompt", cookies=admin_cookie, follow_redirects=False) assert r.status_code == 200 + def test_admin_scheduler_runs_page_admin_only(self, web_client, admin_cookie, analyst_cookie): + """The /admin/scheduler-runs read-only audit-log view is gated by require_admin.""" + r = web_client.get("/admin/scheduler-runs", follow_redirects=False) + assert r.status_code in (302, 401, 403) + r = web_client.get("/admin/scheduler-runs", cookies=analyst_cookie, follow_redirects=False) + assert r.status_code == 403 + r = web_client.get("/admin/scheduler-runs", cookies=admin_cookie, follow_redirects=False) + assert r.status_code == 200 + assert b"run_session_collector" in r.content + assert b"run_verification_detector" in r.content + assert b"run_corporate_memory" in r.content + class TestUnauthenticatedHtmlRedirects: def test_dashboard_unauthenticated_redirects_to_login(self, web_client):