From 3acc276d8de89bd72bf83abec3d158db42477daf Mon Sep 17 00:00:00 2001 From: Abhimanyu Saharan Date: Wed, 4 Mar 2026 23:26:31 +0530 Subject: [PATCH] fix(agent): address webhook payload read review feedback --- backend/app/api/agent.py | 89 ++++++++++++++++--- .../test_agent_webhook_payload_read_api.py | 64 ++++++++++++- ..._openapi_agent_webhook_payload_endpoint.py | 1 + 3 files changed, 139 insertions(+), 15 deletions(-) diff --git a/backend/app/api/agent.py b/backend/app/api/agent.py index 66787039..fbff168f 100644 --- a/backend/app/api/agent.py +++ b/backend/app/api/agent.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from enum import Enum from typing import TYPE_CHECKING, Any, cast from uuid import UUID @@ -20,8 +21,8 @@ from app.core.agent_auth import AgentAuthContext, get_agent_auth_context from app.db.pagination import paginate from app.db.session import get_session from app.models.agents import Agent -from app.models.boards import Board from app.models.board_webhook_payloads import BoardWebhookPayload +from app.models.boards import Board from app.models.tags import Tag from app.models.task_dependencies import TaskDependency from app.models.tasks import Task @@ -169,6 +170,53 @@ def _agent_board_openapi_hints( } +def _truncate_preview(raw: str, max_chars: int) -> str: + if len(raw) <= max_chars: + return raw + if max_chars <= 3: + return raw[:max_chars] + return f"{raw[: max_chars - 3]}..." + + +def _payload_preview_with_limit( + value: dict[str, object] | list[object] | str | int | float | bool | None, + *, + max_chars: int, +) -> tuple[str, bool]: + if isinstance(value, str): + return _truncate_preview(value, max_chars), len(value) > max_chars + + try: + # Stream JSON chunks so we can stop once we know truncation is required. + encoder = json.JSONEncoder(ensure_ascii=True) + parts: list[str] = [] + current_len = 0 + truncated = False + for chunk in encoder.iterencode(value): + remaining = (max_chars + 1) - current_len + if remaining <= 0: + truncated = True + break + if len(chunk) <= remaining: + parts.append(chunk) + current_len += len(chunk) + continue + parts.append(chunk[:remaining]) + current_len += remaining + truncated = True + break + raw = "".join(parts) + except TypeError: + raw = str(value) + return _truncate_preview(raw, max_chars), len(raw) > max_chars + + if len(raw) > max_chars: + truncated = True + if not truncated: + return raw, False + return _truncate_preview(raw, max_chars), True + + def _guard_board_access(agent_ctx: AgentAuthContext, board: Board) -> None: allowed = not (agent_ctx.agent.board_id and agent_ctx.agent.board_id != board.id) OpenClawAuthorizationPolicy.require_board_write_access(allowed=allowed) @@ -578,6 +626,29 @@ async def list_tags( "/boards/{board_id}/webhooks/{webhook_id}/payloads/{payload_id}", response_model=BoardWebhookPayloadRead, tags=AGENT_BOARD_TAGS, + openapi_extra=_agent_board_openapi_hints( + intent="agent_board_webhook_payload_read", + when_to_use=[ + "Agent needs to inspect a previously captured webhook payload for this board.", + "Agent is reconciling missed webhook events or deduping inbound processing.", + ], + routing_examples=[ + { + "input": { + "intent": "inspect stored webhook payload by id", + "required_privilege": "any_agent", + }, + "decision": "agent_board_webhook_payload_read", + }, + { + "input": { + "intent": "list tasks for planning", + "required_privilege": "any_agent", + }, + "decision": "agent_board_task_discovery", + }, + ], + ), ) async def get_webhook_payload( webhook_id: UUID, @@ -589,7 +660,7 @@ async def get_webhook_payload( ) -> BoardWebhookPayloadRead: """Fetch a stored webhook payload (agent-accessible, read-only). - This enables lead agents to backfill dropped webhook events and enforce + This enables board-scoped agents to backfill dropped webhook events and enforce idempotency by inspecting previously received payloads. If `max_chars` is provided and the serialized payload exceeds the limit, @@ -611,17 +682,9 @@ async def get_webhook_payload( response = BoardWebhookPayloadRead.model_validate(payload, from_attributes=True) if max_chars is not None and response.payload is not None: - import json - - try: - raw = json.dumps(response.payload, ensure_ascii=True) - except TypeError: - raw = str(response.payload) - if len(raw) > max_chars: - if max_chars <= 3: - response.payload = raw[:max_chars] - else: - response.payload = f"{raw[: max_chars - 3]}..." + preview, was_truncated = _payload_preview_with_limit(response.payload, max_chars=max_chars) + if was_truncated: + response.payload = preview return response diff --git a/backend/tests/test_agent_webhook_payload_read_api.py b/backend/tests/test_agent_webhook_payload_read_api.py index 65d6b4ac..a4c45ece 100644 --- a/backend/tests/test_agent_webhook_payload_read_api.py +++ b/backend/tests/test_agent_webhook_payload_read_api.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from uuid import UUID, uuid4 import pytest @@ -56,7 +57,11 @@ def _build_test_app(session_maker: async_sessionmaker[AsyncSession]) -> FastAPI: return app -async def _seed_payload(session: AsyncSession) -> tuple[str, Board, BoardWebhook, BoardWebhookPayload]: +async def _seed_payload( + session: AsyncSession, + *, + payload_value: dict[str, object] | list[object] | str | int | float | bool | None = None, +) -> tuple[str, Board, BoardWebhook, BoardWebhookPayload]: token = "test-agent-token-" + uuid4().hex token_hash = hash_agent_token(token) @@ -108,7 +113,7 @@ async def _seed_payload(session: AsyncSession) -> tuple[str, Board, BoardWebhook id=payload_id, board_id=board_id, webhook_id=webhook_id, - payload={"event": "push", "ref": "refs/heads/master"}, + payload=payload_value or {"event": "push", "ref": "refs/heads/master"}, headers={"x-github-event": "push"}, content_type="application/json", source_ip="127.0.0.1", @@ -168,6 +173,61 @@ async def test_agent_payload_read_rejects_invalid_token() -> None: await engine.dispose() +@pytest.mark.asyncio +async def test_agent_payload_read_truncates_json_preview_with_ellipsis() -> None: + engine = await _make_engine() + session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + app = _build_test_app(session_maker) + + async with session_maker() as session: + payload_value: dict[str, object] = {"event": "push", "ref": "refs/heads/master"} + token, board, webhook, payload = await _seed_payload(session, payload_value=payload_value) + + max_chars = 12 + raw = json.dumps(payload_value, ensure_ascii=True) + expected_preview = f"{raw[: max_chars - 3]}..." + + try: + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + response = await client.get( + f"/api/v1/agent/boards/{board.id}/webhooks/{webhook.id}/payloads/{payload.id}", + headers={"X-Agent-Token": token}, + params={"max_chars": max_chars}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["payload"] == expected_preview + + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_agent_payload_read_truncates_string_preview_without_json_quoting() -> None: + engine = await _make_engine() + session_maker = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) + app = _build_test_app(session_maker) + + async with session_maker() as session: + token, board, webhook, payload = await _seed_payload(session, payload_value="abcdef") + + try: + async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as client: + response = await client.get( + f"/api/v1/agent/boards/{board.id}/webhooks/{webhook.id}/payloads/{payload.id}", + headers={"X-Agent-Token": token}, + params={"max_chars": 4}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["payload"] == "a..." + + finally: + await engine.dispose() + + @pytest.mark.asyncio async def test_agent_payload_read_rejects_cross_board_access() -> None: engine = await _make_engine() diff --git a/backend/tests/test_openapi_agent_webhook_payload_endpoint.py b/backend/tests/test_openapi_agent_webhook_payload_endpoint.py index b5ff0d9a..f84fc95a 100644 --- a/backend/tests/test_openapi_agent_webhook_payload_endpoint.py +++ b/backend/tests/test_openapi_agent_webhook_payload_endpoint.py @@ -13,3 +13,4 @@ def test_openapi_includes_agent_webhook_payload_read_endpoint() -> None: op = schema["paths"][path]["get"] tags = set(op.get("tags", [])) assert "agent-worker" in tags + assert op.get("x-llm-intent") == "agent_board_webhook_payload_read"