From fe6038e945a2cd04ad3d7965ed1a93576a3400cb Mon Sep 17 00:00:00 2001 From: Kaiohz Date: Thu, 21 May 2026 17:55:18 +0200 Subject: [PATCH] fix: use camelCase payload keys for Bricks API and add detailed logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix publish_section_version payload keys: snake_case → camelCase (projectUniqueId, sectionKey, workflowId, workflowName, workflowMetadata) to match Bricks API requirements (was getting HTTP 400 MISSING_FIELDS) - Add workflowMetadata with execution_id, executed_at, version - Add detailed logging to BricksApiAdapter (GET/POST requests, responses, error bodies) for debugging - Propagate original error messages in ToolError instead of generic messages --- src/application/api/mcp_bricks_tools.py | 47 ++++++++++++++----- .../publish_section_version_use_case.py | 10 ++-- .../bricks/bricks_api_adapter.py | 43 +++++++++++++++-- tests/unit/test_mcp_bricks_tools.py | 38 ++++++++------- .../test_publish_section_version_use_case.py | 24 +++++----- 5 files changed, 111 insertions(+), 51 deletions(-) diff --git a/src/application/api/mcp_bricks_tools.py b/src/application/api/mcp_bricks_tools.py index fe4df9f..17db143 100644 --- a/src/application/api/mcp_bricks_tools.py +++ b/src/application/api/mcp_bricks_tools.py @@ -1,4 +1,5 @@ import logging +from datetime import datetime, timezone from fastmcp import FastMCP from fastmcp.exceptions import ToolError @@ -40,11 +41,15 @@ async def list_bricks_documents(project_unique_id: str) -> list[DocumentSummary] use_case = get_list_bricks_documents_use_case() try: documents = await use_case.execute(project_id=project_unique_id) - except Exception: - logger.exception( - "Failed to list bricks documents for project %s", project_unique_id - ) - raise ToolError("Failed to list bricks documents") from None + except PermissionError as e: + logger.error("List documents auth failed for project %s: %s", project_unique_id, e) + raise ToolError(str(e)) from e + except (ConnectionError, TimeoutError) as e: + logger.error("List documents network error for project %s: %s", project_unique_id, e) + raise ToolError(str(e)) from e + except Exception as e: + logger.exception("Failed to list bricks documents for project %s: %s", project_unique_id, e) + raise ToolError(f"Failed to list bricks documents: {e}") from e return [ DocumentSummary( id=doc.id, @@ -79,9 +84,15 @@ async def read_bricks_document( use_case = get_read_bricks_document_use_case() try: result = await use_case.execute(document_id=document_id, project_id=project_unique_id) - except Exception: - logger.exception("Failed to read bricks document: %s in project %s", document_id, project_unique_id) - raise ToolError("Failed to read bricks document") from None + except PermissionError as e: + logger.error("Read document auth failed for %s: %s", document_id, e) + raise ToolError(str(e)) from e + except (ConnectionError, TimeoutError) as e: + logger.error("Read document network error for %s: %s", document_id, e) + raise ToolError(str(e)) from e + except Exception as e: + logger.exception("Failed to read bricks document: %s in project %s: %s", document_id, project_unique_id, e) + raise ToolError(f"Failed to read bricks document: {e}") from e return FileContentResponse( content=result.content, metadata=result.metadata, @@ -107,6 +118,12 @@ async def publish_section_version( Résultat de la publication avec statut et aperçu du payload """ use_case = get_publish_section_version_use_case() + now = datetime.now(timezone.utc) + workflow_metadata = { + "execution_id": f"run-{now.strftime('%Y-%m-%d')}-001", + "executed_at": now.isoformat(), + "version": "1.0.0", + } try: return await use_case.execute( project_unique_id=project_unique_id, @@ -114,10 +131,16 @@ async def publish_section_version( content=content, workflow_id="agent-haiku-files-v1", workflow_name="agent-haiku-files-v1", - workflow_metadata=None, + workflow_metadata=workflow_metadata, ) - except Exception: + except PermissionError as e: + logger.error("Publish auth failed for project %s: %s", project_unique_id, e) + raise ToolError(str(e)) from e + except (ConnectionError, TimeoutError) as e: + logger.error("Publish network error for project %s: %s", project_unique_id, e) + raise ToolError(str(e)) from e + except Exception as e: logger.exception( - "Failed to publish section version for project %s", project_unique_id + "Failed to publish section version for project %s: %s", project_unique_id, e ) - raise ToolError("Failed to publish section version") from None + raise ToolError(f"Failed to publish section version: {e}") from e diff --git a/src/application/use_cases/publish_section_version_use_case.py b/src/application/use_cases/publish_section_version_use_case.py index fb6cfb1..8dc37f3 100644 --- a/src/application/use_cases/publish_section_version_use_case.py +++ b/src/application/use_cases/publish_section_version_use_case.py @@ -20,12 +20,12 @@ async def execute( workflow_metadata: dict | None = None, ) -> SectionVersionResult: payload = { - "project_unique_id": project_unique_id, - "section_key": section_key, + "projectUniqueId": project_unique_id, + "sectionKey": section_key, "content": content, - "workflow_id": workflow_id, - "workflow_name": workflow_name, - "workflow_metadata": workflow_metadata + "workflowId": workflow_id, + "workflowName": workflow_name, + "workflowMetadata": workflow_metadata if workflow_metadata is not None else {}, } diff --git a/src/infrastructure/bricks/bricks_api_adapter.py b/src/infrastructure/bricks/bricks_api_adapter.py index 7d18f60..a4ac139 100644 --- a/src/infrastructure/bricks/bricks_api_adapter.py +++ b/src/infrastructure/bricks/bricks_api_adapter.py @@ -28,18 +28,42 @@ async def close(self) -> None: pass def _get(self, url: str, headers: dict | None = None) -> tuple[bytes, dict]: + logger.debug("GET %s", url) req = urllib.request.Request(url, headers=headers or {}) - with urllib.request.urlopen(req, timeout=_DEFAULT_TIMEOUT) as resp: - return resp.read(), dict(resp.headers) + try: + with urllib.request.urlopen(req, timeout=_DEFAULT_TIMEOUT) as resp: + body = resp.read() + resp_headers = dict(resp.headers) + logger.debug("GET %s -> %d bytes (status=%s)", url, len(body), resp.status) + return body, resp_headers + except urllib.error.HTTPError as e: + error_body = e.read().decode("utf-8", errors="replace") if hasattr(e, "read") else "" + logger.error("GET %s -> HTTP %d: %s", url, e.code, error_body[:500]) + raise + except Exception as e: + logger.error("GET %s -> error: %s", url, e) + raise def _post(self, url: str, payload: dict, headers: dict) -> bytes: data = json.dumps(payload).encode("utf-8") + logger.debug("POST %s (body=%d bytes)", url, len(data)) req = urllib.request.Request(url, data=data, headers=headers, method="POST") - with urllib.request.urlopen(req, timeout=_DEFAULT_TIMEOUT) as resp: - return resp.read() + try: + with urllib.request.urlopen(req, timeout=_DEFAULT_TIMEOUT) as resp: + body = resp.read() + logger.debug("POST %s -> %d bytes (status=%s)", url, len(body), resp.status) + return body + except urllib.error.HTTPError as e: + error_body = e.read().decode("utf-8", errors="replace") if hasattr(e, "read") else "" + logger.error("POST %s -> HTTP %d: %s", url, e.code, error_body[:500]) + raise + except Exception as e: + logger.error("POST %s -> error: %s", url, e) + raise async def list_project_documents(self, project_id: str) -> list[BricksDocumentInfo]: url = f"{self._base_url}/api/projects/{project_id}/documents/ai" + logger.info("Listing Bricks documents for project %s", project_id) try: body, _ = await asyncio.to_thread( self._get, url, {"Authorization": f"Bearer {self._bearer_token}"} @@ -59,6 +83,7 @@ async def list_project_documents(self, project_id: str) -> list[BricksDocumentIn except TimeoutError as e: raise TimeoutError(f"Bricks API request timed out: {e}") from e items = json.loads(body).get("items", []) + logger.info("Found %d Bricks documents for project %s", len(items), project_id) documents = [BricksDocumentInfo(**item) for item in items] return documents @@ -67,6 +92,7 @@ async def download_document( document_id: str, project_id: str, ) -> tuple[bytes, str, str]: + logger.info("Downloading Bricks document %s from project %s", document_id, project_id) documents = await self.list_project_documents(project_id) url = None mime_type = "" @@ -99,6 +125,7 @@ async def download_document( raise TimeoutError(f"Document download timed out: {e}") from e filename = _extract_filename(resp_headers.get("Content-Disposition", ""), url) + logger.info("Downloaded Bricks document %s (%d bytes, mime=%s, filename=%s)", document_id, len(body), mime_type, filename) return body, filename, mime_type async def publish_section_version(self, payload: dict) -> SectionVersionResult: @@ -107,6 +134,13 @@ async def publish_section_version(self, payload: dict) -> SectionVersionResult: "X-API-Key": self._api_key, "Content-Type": "application/json", } + logger.info( + "Publishing section version: project=%s section=%s workflow=%s", + payload.get("project_unique_id"), + payload.get("section_key"), + payload.get("workflow_id"), + ) + logger.info("Publish payload: %s", json.dumps(payload, ensure_ascii=False, default=str)) try: body = await asyncio.to_thread(self._post, url, payload, headers) except urllib.error.HTTPError as e: @@ -120,6 +154,7 @@ async def publish_section_version(self, payload: dict) -> SectionVersionResult: except TimeoutError as e: raise TimeoutError(f"Publish request timed out: {e}") from e data = json.loads(body) + logger.info("Published section version successfully: %s", data) return SectionVersionResult(success=True, message="Published", data=data) diff --git a/tests/unit/test_mcp_bricks_tools.py b/tests/unit/test_mcp_bricks_tools.py index 68e154f..813e999 100644 --- a/tests/unit/test_mcp_bricks_tools.py +++ b/tests/unit/test_mcp_bricks_tools.py @@ -106,7 +106,7 @@ async def test_raises_tool_error_for_api_failure(self) -> None: "application.api.mcp_bricks_tools.get_list_bricks_documents_use_case", return_value=mock_use_case, ), - pytest.raises(ToolError, match="Failed to list bricks documents"), + pytest.raises(ToolError, match="API unreachable"), ): await list_bricks_documents(project_unique_id="proj-123") @@ -150,7 +150,7 @@ async def test_raises_tool_error_for_download_failure(self) -> None: "application.api.mcp_bricks_tools.get_read_bricks_document_use_case", return_value=mock_use_case, ), - pytest.raises(ToolError, match="Failed to read bricks document"), + pytest.raises(ToolError, match="S3 download failed"), ): await read_bricks_document( document_id="doc-expired", project_unique_id="proj-1" @@ -232,14 +232,16 @@ async def test_forward_all_params_to_use_case(self) -> None: content="Summary content", ) - mock_use_case.execute.assert_called_once_with( - project_unique_id="proj-abc", - section_key="consolidated_data", - content="Summary content", - workflow_id="agent-haiku-files-v1", - workflow_name="agent-haiku-files-v1", - workflow_metadata=None, - ) + call_args = mock_use_case.execute.call_args + assert call_args.kwargs["project_unique_id"] == "proj-abc" + assert call_args.kwargs["section_key"] == "consolidated_data" + assert call_args.kwargs["content"] == "Summary content" + assert call_args.kwargs["workflow_id"] == "agent-haiku-files-v1" + assert call_args.kwargs["workflow_name"] == "agent-haiku-files-v1" + assert call_args.kwargs["workflow_metadata"] is not None + assert "execution_id" in call_args.kwargs["workflow_metadata"] + assert "executed_at" in call_args.kwargs["workflow_metadata"] + assert call_args.kwargs["workflow_metadata"]["version"] == "1.0.0" async def test_raises_tool_error_for_api_failure(self) -> None: """Should convert API errors to ToolError.""" @@ -251,7 +253,7 @@ async def test_raises_tool_error_for_api_failure(self) -> None: "application.api.mcp_bricks_tools.get_publish_section_version_use_case", return_value=mock_use_case, ), - pytest.raises(ToolError, match="Failed to publish section version"), + pytest.raises(ToolError, match="API connection failed"), ): await publish_section_version( project_unique_id="proj-123", @@ -268,7 +270,7 @@ async def test_raises_tool_error_for_generic_failure(self) -> None: "application.api.mcp_bricks_tools.get_publish_section_version_use_case", return_value=mock_use_case, ), - pytest.raises(ToolError, match="Failed to publish section version"), + pytest.raises(ToolError, match="Failed to publish section version: Unexpected error"), ): await publish_section_version( project_unique_id="proj-123", @@ -283,12 +285,12 @@ async def test_returns_dry_run_result_with_preview(self) -> None: message="Dry run", dry_run=True, payload_preview={ - "project_unique_id": "proj-123", - "section_key": "consolidated_data", + "projectUniqueId": "proj-123", + "sectionKey": "consolidated_data", "content": "Hello", - "workflow_id": "agent-haiku-files-v1", - "workflow_name": "agent-haiku-files-v1", - "workflow_metadata": None, + "workflowId": "agent-haiku-files-v1", + "workflowName": "agent-haiku-files-v1", + "workflowMetadata": {}, }, ) @@ -303,4 +305,4 @@ async def test_returns_dry_run_result_with_preview(self) -> None: assert result.dry_run is True assert result.payload_preview is not None - assert result.payload_preview["section_key"] == "consolidated_data" + assert result.payload_preview["sectionKey"] == "consolidated_data" diff --git a/tests/unit/test_publish_section_version_use_case.py b/tests/unit/test_publish_section_version_use_case.py index 363ab3a..64d27fc 100644 --- a/tests/unit/test_publish_section_version_use_case.py +++ b/tests/unit/test_publish_section_version_use_case.py @@ -66,12 +66,12 @@ async def test_dry_run_returns_payload_preview( assert result.success is True assert result.dry_run is True assert result.payload_preview is not None - assert result.payload_preview["project_unique_id"] == "proj-123" - assert result.payload_preview["section_key"] == "summary" + assert result.payload_preview["projectUniqueId"] == "proj-123" + assert result.payload_preview["sectionKey"] == "summary" assert result.payload_preview["content"] == "Summary text" - assert result.payload_preview["workflow_id"] == "wf-2" - assert result.payload_preview["workflow_name"] == "review" - assert result.payload_preview["workflow_metadata"] == {"version": 1} + assert result.payload_preview["workflowId"] == "wf-2" + assert result.payload_preview["workflowName"] == "review" + assert result.payload_preview["workflowMetadata"] == {"version": 1} async def test_live_calls_bricks_api( self, @@ -122,12 +122,12 @@ async def test_live_passes_correct_payload( payload = call_args[1].get("payload", call_args[0][0] if call_args[0] else None) if payload is None: payload = call_args.kwargs.get("payload") - assert payload["project_unique_id"] == "proj-abc" - assert payload["section_key"] == "results" + assert payload["projectUniqueId"] == "proj-abc" + assert payload["sectionKey"] == "results" assert payload["content"] == "Final results" - assert payload["workflow_id"] == "wf-final" - assert payload["workflow_name"] == "final_review" - assert payload["workflow_metadata"] == {"approved_by": "admin"} + assert payload["workflowId"] == "wf-final" + assert payload["workflowName"] == "final_review" + assert payload["workflowMetadata"] == {"approved_by": "admin"} async def test_workflow_metadata_defaults_to_empty_dict( self, @@ -142,7 +142,7 @@ async def test_workflow_metadata_defaults_to_empty_dict( workflow_name="draft", ) - assert result.payload_preview["workflow_metadata"] == {} + assert result.payload_preview["workflowMetadata"] == {} async def test_workflow_metadata_none_in_live_payload( self, @@ -166,7 +166,7 @@ async def test_workflow_metadata_none_in_live_payload( payload = call_args[1].get("payload", call_args[0][0] if call_args[0] else None) if payload is None: payload = call_args.kwargs.get("payload") - assert payload["workflow_metadata"] == {} + assert payload["workflowMetadata"] == {} async def test_live_propagates_api_errors( self,