From 147d7116574d64887828a5413ab92c8e909771f9 Mon Sep 17 00:00:00 2001 From: busya Date: Sun, 5 Jul 2026 14:14:42 +0300 Subject: [PATCH] feat(agent-chat): complete context guardrail event coverage --- backend/src/agent/app.py | 39 +++++- backend/src/agent/tools.py | 40 ++++++ backend/tests/test_agent_confirmation_v2.py | 70 ++++++++++ backend/tests/test_agent_context.py | 61 +++++++++ backend/tests/test_agent_retry.py | 46 ++++--- backend/tests/test_agent_tool_filter.py | 78 +++++++++++ .../src/lib/components/agent/AgentChat.svelte | 15 +++ .../assistant/ConfirmationCard.svelte | 21 ++- .../__tests__/ConfirmationCard.ux.test.ts | 124 ++++++++++++++++++ .../AgentChat.StreamProcessor.svelte.ts | 9 ++ .../src/lib/models/AgentChatModel.svelte.ts | 13 ++ frontend/src/lib/models/AgentChatTypes.ts | 14 +- .../__tests__/AgentChatModel.context.test.ts | 81 ++++++++++++ 13 files changed, 587 insertions(+), 24 deletions(-) create mode 100644 backend/tests/test_agent_confirmation_v2.py create mode 100644 backend/tests/test_agent_context.py create mode 100644 backend/tests/test_agent_tool_filter.py create mode 100644 frontend/src/lib/components/assistant/__tests__/ConfirmationCard.ux.test.ts create mode 100644 frontend/src/lib/models/__tests__/AgentChatModel.context.test.ts diff --git a/backend/src/agent/app.py b/backend/src/agent/app.py index b70a092b..7a2222f2 100644 --- a/backend/src/agent/app.py +++ b/backend/src/agent/app.py @@ -52,7 +52,12 @@ from src.agent.context import set_user_jwt, set_user_role from src.agent.document_parser import parse_upload from src.agent.langgraph_setup import create_agent from src.agent.middleware import log_tool_event -from src.agent.tools import _redact_sensitive_fields, get_all_tools +from src.agent.tools import ( + _redact_sensitive_fields, + drain_tool_retry_events, + get_all_tools, + start_tool_retry_event_buffer, +) from src.core.auth.jwt import decode_token from src.core.cot_logger import seed_trace_id from src.core.logger import logger @@ -517,6 +522,15 @@ async def agent_handler( # noqa: C901 — intentionally complex C4 orchestratio user_role, uicontext.get("objectType") if uicontext else None, ) + yield json.dumps({ + "content": "", + "metadata": { + "type": "pipeline_result", + "tools": [tool.name for tool in agent_tools], + "object_type": uicontext.get("objectType") if uicontext else None, + "user_role": user_role, + }, + }) # Auto-inject environment_id into tool calls when LLM omits it agent_tools = _inject_env_id_into_tools(agent_tools, env_id) agent = await create_agent(agent_tools, env_id) @@ -524,6 +538,7 @@ async def agent_handler( # noqa: C901 — intentionally complex C4 orchestratio assistant_parts: list[str] = [] max_attempts = 2 + start_tool_retry_event_buffer() try: for attempt in range(max_attempts): @@ -534,6 +549,9 @@ async def agent_handler( # noqa: C901 — intentionally complex C4 orchestratio config=config, version="v2", ): + for retry_event in drain_tool_retry_events(): + emitted_any = True + yield json.dumps(retry_event) kind = event.get("event") if kind in ("on_tool_start", "on_tool_end", "on_tool_error"): await log_tool_event(event, conv_id) @@ -589,12 +607,31 @@ async def agent_handler( # noqa: C901 — intentionally complex C4 orchestratio user_role=denied_role, ) continue + if "timed out" in err.lower() or "timeout" in err.lower(): + is_write_tool = "operation status unknown" in err.lower() or tool_name in { + "deploy_dashboard", "commit_changes", "create_branch", "run_backup", + "execute_migration", "start_maintenance", "end_maintenance", + } + yield json.dumps({ + "content": f"⏱️ {tool_name} — timeout", + "metadata": { + "type": "tool_timeout", + "tool": tool_name, + "timeout_seconds": 30, + "is_write_tool": is_write_tool, + "retryable": not is_write_tool, + }, + }) + continue yield json.dumps({ "content": f"❌ {tool_name} — {err}", "metadata": {"type": "tool_error", "tool": tool_name, "error": err}, }) state = await agent.aget_state(config) + for retry_event in drain_tool_retry_events(): + emitted_any = True + yield json.dumps(retry_event) if getattr(state, "next", None): emitted_any = True yield confirmation_payload(conv_id, state, visible_user_text, user_role, env_id) diff --git a/backend/src/agent/tools.py b/backend/src/agent/tools.py index d362b717..79f02878 100644 --- a/backend/src/agent/tools.py +++ b/backend/src/agent/tools.py @@ -6,6 +6,7 @@ # @INVARIANT Every @tool function MUST have a Python docstring (triple-quoted string immediately after the signature). LangChain raises ValueError("Function must have a docstring if description not provided.") when args_schema is provided without a description param AND the function lacks a docstring. This is a runtime blocker — the entire Gradio agent fails to import when ANY single tool violates this invariant. The GRACE @BRIEF comment is NOT a substitute — it's structural metadata invisible to the Python runtime. Rule: BEFORE decorating with @tool(args_schema=...), ensure `"""One-line description."""` is present on the next line. import asyncio +from contextvars import ContextVar import json import os from typing import Any @@ -20,9 +21,47 @@ from src.core.logger import logger TOOL_RESPONSE_LIMIT = 4000 TOOL_TIMEOUT_SECONDS = 30 +_TOOL_RETRY_EVENTS: ContextVar[list[dict[str, Any]] | None] = ContextVar( + "agent_tool_retry_events", + default=None, +) # ── Internal helpers ───────────────────────────────────────────── +# #region AgentChat.Tools.RetryEvents [C:2] [TYPE Function] [SEMANTICS agent-chat,tools,retry,sse] +# @ingroup AgentChat +# @BRIEF Context-local retry event buffer used by Gradio stream to emit tool_retry metadata. +def start_tool_retry_event_buffer() -> None: + """Initialise an empty retry-event buffer for the current agent request.""" + _TOOL_RETRY_EVENTS.set([]) + + +def drain_tool_retry_events() -> list[dict[str, Any]]: + """Return and clear pending tool_retry events for the current request.""" + events = _TOOL_RETRY_EVENTS.get() + if not events: + return [] + drained = list(events) + events.clear() + return drained + + +def _queue_tool_retry_event(tool_name: str, attempt: int, max_attempts: int) -> None: + """Queue a tool_retry event if a request-local buffer is active.""" + events = _TOOL_RETRY_EVENTS.get() + if events is None: + return + events.append({ + "content": f"🔁 {tool_name} retry {attempt}/{max_attempts}", + "metadata": { + "type": "tool_retry", + "tool": tool_name, + "attempt": attempt, + "max_attempts": max_attempts, + }, + }) +# #endregion AgentChat.Tools.RetryEvents + # #region AgentChat.Tools.DualAuthHeaders [C:2] [TYPE Function] [SEMANTICS agent-chat,tools,auth,helper] # @ingroup AgentChat # @BRIEF Build dual-identity auth headers for tool→FastAPI calls per FR-007/FR-019. @@ -132,6 +171,7 @@ async def _retry_read_tool( except (httpx.HTTPStatusError, httpx.ConnectError, httpx.ReadTimeout) as e: last_error = e if attempt < max_attempts: + _queue_tool_retry_event(tool_name, attempt, max_attempts) logger.reason("Tool retry", payload={"tool": tool_name, "attempt": attempt, "error": str(e)[:100]}, extra={"src": "AgentChat.Tools.Retry"}) await asyncio.sleep(1) continue diff --git a/backend/tests/test_agent_confirmation_v2.py b/backend/tests/test_agent_confirmation_v2.py new file mode 100644 index 00000000..5a17720a --- /dev/null +++ b/backend/tests/test_agent_confirmation_v2.py @@ -0,0 +1,70 @@ +# #region Test.AgentChat.ConfirmationV2 [C:3] [TYPE Module] [SEMANTICS test,agent-chat,confirmation,guardrails] +# @BRIEF Contract tests for build_confirmation_contract_v2 and permission_denied payloads. +# @RELATION BINDS_TO -> [AgentChat.Confirmation.GuardV2] +# @RELATION BINDS_TO -> [AgentChat.Confirmation.PermissionDenied] +# @TEST_EDGE: deploy_prod -> guarded write with prod env_context. +# @TEST_EDGE: delete_any -> dangerous operation. +# @TEST_EDGE: analyst_deploy -> permission_denied SSE metadata. + +import json + +from src.agent._confirmation import build_confirmation_contract_v2, permission_denied_payload + + +# #region test_deploy_prod_guarded [C:2] [TYPE Function] +# @BRIEF deploy_dashboard targeting prod resolves guarded write + env_context prod. +def test_deploy_prod_guarded(): + contract = build_confirmation_contract_v2( + "deploy_dashboard", + {"env_id": "prod-eu"}, + user_role="admin", + target_env="staging", + ) + + assert contract["risk"] == "write" + assert contract["risk_level"] == "guarded" + assert contract["env_context"] == "prod" + assert contract["permission_granted"] is True +# #endregion test_deploy_prod_guarded + + +# #region test_delete_any_is_dangerous [C:2] [TYPE Function] +# @BRIEF delete-prefixed operations are classified as dangerous. +def test_delete_any_is_dangerous(): + contract = build_confirmation_contract_v2("delete_dashboard", {}, user_role="admin") + + assert contract["risk"] == "write" + assert contract["risk_level"] == "dangerous" + assert contract["dangerous"] is True +# #endregion test_delete_any_is_dangerous + + +# #region test_read_only_safe [C:2] [TYPE Function] +# @BRIEF search_dashboards remains safe read. +def test_read_only_safe(): + contract = build_confirmation_contract_v2("search_dashboards", {}, user_role="analyst") + + assert contract["risk"] == "read" + assert contract["risk_level"] == "safe" + assert contract["permission_granted"] is True +# #endregion test_read_only_safe + + +# #region test_analyst_deploy_permission_denied_payload [C:2] [TYPE Function] +# @BRIEF Analyst deploy produces permission_denied metadata, not confirm_required metadata. +def test_analyst_deploy_permission_denied_payload(): + contract = build_confirmation_contract_v2("deploy_dashboard", {}, user_role="analyst") + payload = json.loads(permission_denied_payload( + "deploy_dashboard", + required_role=contract["required_role"], + user_role="analyst", + alternatives=contract["alternatives"], + )) + + assert contract["permission_granted"] is False + assert payload["metadata"]["type"] == "permission_denied" + assert payload["metadata"]["required_role"] == "admin" + assert payload["metadata"]["user_role"] == "analyst" +# #endregion test_analyst_deploy_permission_denied_payload + +# #endregion Test.AgentChat.ConfirmationV2 diff --git a/backend/tests/test_agent_context.py b/backend/tests/test_agent_context.py new file mode 100644 index 00000000..b6f960d6 --- /dev/null +++ b/backend/tests/test_agent_context.py @@ -0,0 +1,61 @@ +# #region Test.AgentChat.Context [C:3] [TYPE Module] [SEMANTICS test,agent-chat,context,validation] +# @BRIEF Contract tests for UIContext validation at the backend trust boundary. +# @RELATION BINDS_TO -> [AgentChat.Context.Validate] +# @TEST_EDGE: valid -> returns unchanged payload. +# @TEST_EDGE: invalid_objectType -> raises validation error. +# @TEST_EDGE: oversized -> raises validation error. +# @TEST_EDGE: objectName_too_long -> raises validation error. +# ruff: noqa: I001 + +import pytest + +from src.agent._context import UIContextValidationError, validate_uicontext + + +VALID_CONTEXT = { + "objectType": "dashboard", + "objectId": "42", + "objectName": "Energy", + "envId": "ss-dev", + "route": "/dashboards/42", + "contextVersion": 1, +} + + +# #region test_valid_uicontext_returns_payload [C:2] [TYPE Function] +# @BRIEF Valid UIContext payload returns unchanged. +def test_valid_uicontext_returns_payload(): + assert validate_uicontext(dict(VALID_CONTEXT)) == VALID_CONTEXT +# #endregion test_valid_uicontext_returns_payload + + +# #region test_invalid_object_type_rejected [C:2] [TYPE Function] +# @BRIEF Unknown objectType is rejected by backend validation. +def test_invalid_object_type_rejected(): + payload = dict(VALID_CONTEXT, objectType="chart") + + with pytest.raises(UIContextValidationError, match="objectType"): + validate_uicontext(payload) +# #endregion test_invalid_object_type_rejected + + +# #region test_oversized_payload_rejected [C:2] [TYPE Function] +# @BRIEF Payloads over 4KB are rejected before prompt injection. +def test_oversized_payload_rejected(): + payload = dict(VALID_CONTEXT, debug_padding="x" * 5000) + + with pytest.raises(UIContextValidationError, match="4 KB"): + validate_uicontext(payload) +# #endregion test_oversized_payload_rejected + + +# #region test_object_name_too_long_rejected [C:2] [TYPE Function] +# @BRIEF objectName longer than 256 chars is rejected. +def test_object_name_too_long_rejected(): + payload = dict(VALID_CONTEXT, objectName="x" * 257) + + with pytest.raises(UIContextValidationError, match="objectName"): + validate_uicontext(payload) +# #endregion test_object_name_too_long_rejected + +# #endregion Test.AgentChat.Context diff --git a/backend/tests/test_agent_retry.py b/backend/tests/test_agent_retry.py index acff1c00..d36a5393 100644 --- a/backend/tests/test_agent_retry.py +++ b/backend/tests/test_agent_retry.py @@ -7,14 +7,17 @@ # @TEST_EDGE: connect_error -> Retries on ConnectError too. # @TEST_EDGE: read_timeout -> Retries on ReadTimeout too. -import asyncio +import pytest from unittest.mock import AsyncMock, patch import httpx -import pytest - -from src.agent.tools import _execute_with_timeout, _retry_read_tool +from src.agent.tools import ( + _execute_with_timeout, + _retry_read_tool, + drain_tool_retry_events, + start_tool_retry_event_buffer, +) # ── Shared fixtures ────────────────────────────────────────────────── @@ -48,6 +51,7 @@ class TestRetryReadTool: expected = {"status": "ok", "data": [1, 2, 3]} error_502 = _make_502_error() mock_fn = AsyncMock(side_effect=[error_502, expected]) + start_tool_retry_event_buffer() with patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: result = await _retry_read_tool("read_dashboards", mock_fn) @@ -55,6 +59,15 @@ class TestRetryReadTool: assert result == expected assert mock_fn.call_count == 2 mock_sleep.assert_awaited_once_with(1) + assert drain_tool_retry_events() == [{ + "content": "🔁 read_dashboards retry 1/2", + "metadata": { + "type": "tool_retry", + "tool": "read_dashboards", + "attempt": 1, + "max_attempts": 2, + }, + }] # #endregion test_first_attempt_502_retries_once # #region test_both_attempts_502_raises [C:2] [TYPE Function] @@ -64,9 +77,8 @@ class TestRetryReadTool: error_502 = _make_502_error() mock_fn = AsyncMock(side_effect=[error_502, error_502]) - with patch("asyncio.sleep", new_callable=AsyncMock): - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await _retry_read_tool("read_dashboards", mock_fn) + with patch("asyncio.sleep", new_callable=AsyncMock), pytest.raises(httpx.HTTPStatusError) as exc_info: + await _retry_read_tool("read_dashboards", mock_fn) assert exc_info.value is error_502 assert mock_fn.call_count == 2 @@ -124,9 +136,8 @@ class TestRetryReadTool: non_http_err = ValueError("something broken in business logic") mock_fn = AsyncMock(side_effect=non_http_err) - with patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: - with pytest.raises(ValueError) as exc_info: - await _retry_read_tool("broken_tool", mock_fn) + with patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep, pytest.raises(ValueError) as exc_info: + await _retry_read_tool("broken_tool", mock_fn) assert exc_info.value is non_http_err assert mock_fn.call_count == 1 @@ -149,14 +160,13 @@ class TestWriteToolNoRetry: error_502 = _make_502_error() write_op = AsyncMock(side_effect=error_502) - with patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep: - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await _execute_with_timeout( - "create_dashboard", - write_op, - is_write=True, - timeout_s=5, - ) + with patch("asyncio.sleep", new_callable=AsyncMock) as mock_sleep, pytest.raises(httpx.HTTPStatusError) as exc_info: + await _execute_with_timeout( + "create_dashboard", + write_op, + is_write=True, + timeout_s=5, + ) assert exc_info.value is error_502 assert write_op.call_count == 1 diff --git a/backend/tests/test_agent_tool_filter.py b/backend/tests/test_agent_tool_filter.py new file mode 100644 index 00000000..1ce0403d --- /dev/null +++ b/backend/tests/test_agent_tool_filter.py @@ -0,0 +1,78 @@ +# #region Test.AgentChat.ToolFilter [C:3] [TYPE Module] [SEMANTICS test,agent-chat,tools,filter] +# @BRIEF Contract tests for AgentChat.ToolFilter — RBAC first, then context affinity. +# @RELATION BINDS_TO -> [AgentChat.ToolFilter.Pipeline] +# @TEST_EDGE: null_context -> all RBAC-allowed tools are returned. +# @TEST_EDGE: dashboard_analyst -> dashboard tools minus admin-only tools. +# @TEST_EDGE: unknown_context -> graceful fallback to RBAC-only filtering. + +from types import SimpleNamespace + +from src.agent._tool_filter import build_tool_pipeline, enforce_tool_permission + + +def _tool(name: str) -> SimpleNamespace: + return SimpleNamespace(name=name) + + +TOOLS = [ + _tool("show_capabilities"), + _tool("search_dashboards"), + _tool("get_health_summary"), + _tool("deploy_dashboard"), + _tool("create_branch"), + _tool("commit_changes"), + _tool("superset_explore_database"), + _tool("run_backup"), +] + + +# #region test_null_context_filters_by_rbac_only [C:2] [TYPE Function] +# @BRIEF Null objectType returns every tool the role may invoke. +def test_null_context_filters_by_rbac_only(): + result = build_tool_pipeline(TOOLS, user_role="analyst", object_type=None) + + assert [tool.name for tool in result] == [ + "show_capabilities", + "search_dashboards", + "get_health_summary", + "superset_explore_database", + ] +# #endregion test_null_context_filters_by_rbac_only + + +# #region test_dashboard_analyst_filters_context_after_rbac [C:2] [TYPE Function] +# @BRIEF Dashboard context excludes non-dashboard tools after RBAC removes admin-only tools. +def test_dashboard_analyst_filters_context_after_rbac(): + result = build_tool_pipeline(TOOLS, user_role="analyst", object_type="dashboard") + + assert [tool.name for tool in result] == [ + "show_capabilities", + "search_dashboards", + "get_health_summary", + ] +# #endregion test_dashboard_analyst_filters_context_after_rbac + + +# #region test_unknown_context_falls_back_to_rbac_only [C:2] [TYPE Function] +# @BRIEF Unknown objectType does not drop RBAC-allowed tools. +def test_unknown_context_falls_back_to_rbac_only(): + result = build_tool_pipeline(TOOLS, user_role="analyst", object_type="unknown") + + assert [tool.name for tool in result] == [ + "show_capabilities", + "search_dashboards", + "get_health_summary", + "superset_explore_database", + ] +# #endregion test_unknown_context_falls_back_to_rbac_only + + +# #region test_enforce_tool_permission_rejects_admin_only [C:2] [TYPE Function] +# @BRIEF Invocation guard rejects analyst access to admin-only tools. +def test_enforce_tool_permission_rejects_admin_only(): + assert enforce_tool_permission("deploy_dashboard", "analyst") is False + assert enforce_tool_permission("deploy_dashboard", "admin") is True + assert enforce_tool_permission("search_dashboards", "analyst") is True +# #endregion test_enforce_tool_permission_rejects_admin_only + +# #endregion Test.AgentChat.ToolFilter diff --git a/frontend/src/lib/components/agent/AgentChat.svelte b/frontend/src/lib/components/agent/AgentChat.svelte index 8e883ee8..f72f3205 100644 --- a/frontend/src/lib/components/agent/AgentChat.svelte +++ b/frontend/src/lib/components/agent/AgentChat.svelte @@ -338,6 +338,7 @@ // ── Tool calls (full objects) ── active_tool_calls: model.activeToolCalls.length, active_tool_calls_full: model.activeToolCalls, + effective_tool_pipeline: model.effectiveToolPipeline, // ── HITL / Confirmation ── confirmation_message: model.confirmationMessage, pending_tool_name: model.pendingToolName, @@ -642,6 +643,20 @@

Контекст не передан

{/if} + + +
+

Pipeline

+ {#if model.effectiveToolPipeline.length > 0} +
+ {#each model.effectiveToolPipeline as tool (tool)} + {tool} + {/each} +
+ {:else} +

Pipeline ещё не получен от backend

+ {/if} +
{/if} diff --git a/frontend/src/lib/components/assistant/ConfirmationCard.svelte b/frontend/src/lib/components/assistant/ConfirmationCard.svelte index b7503ba1..36ce612f 100644 --- a/frontend/src/lib/components/assistant/ConfirmationCard.svelte +++ b/frontend/src/lib/components/assistant/ConfirmationCard.svelte @@ -40,6 +40,11 @@ "Требуется подтверждение", ); let isPermissionDenied = $derived(model.pendingRiskLevel === "permission_denied"); + let envBadge = $derived( + model.pendingEnvContext === "prod" ? "⚠️ PROD" : + model.pendingEnvContext === "staging" ? "STAGING" : + model.pendingEnvContext === "dev" ? "DEV" : "", + ); let toneClasses = $derived( model.confirmationTone === "success" ? { @@ -176,6 +181,15 @@ {model.pendingRiskLevel === 'dangerous' ? '💀 dangerous' : model.pendingRiskLevel === 'guarded' ? '⚠️ guarded' : model.pendingRiskLevel === 'permission_denied' ? '🔒 permission_denied' : ''} {/if} + {#if envBadge} + + {envBadge} + + {/if}

{fallbackDescription}

@@ -194,7 +208,7 @@ {#if argEntries.length > 0}
- {#each argEntries as [key, value]} + {#each argEntries as [key, value] (key)}
{key}: {String(value).slice(0, 120)} @@ -221,11 +235,14 @@ onclick={handleConfirm} > {model.dangerousCountdownActive && model.dangerousCountdown > 0 - ? `Подтвердить (${model.dangerousCountdown})` + ? `💀 Удалить (${model.dangerousCountdown})` : loading === "confirming" ? ($t.assistant?.confirming || "Подтверждение…") : ($t.assistant?.confirm || "Подтвердить")} + {#if model.dangerousCountdownActive} + Осталось {model.dangerousCountdown} секунд + {/if} {/if}