diff --git a/apps/mcp-server/src/taskflow_mcp/tools/tasks.py b/apps/mcp-server/src/taskflow_mcp/tools/tasks.py index 3b70047..58c6c50 100644 --- a/apps/mcp-server/src/taskflow_mcp/tools/tasks.py +++ b/apps/mcp-server/src/taskflow_mcp/tools/tasks.py @@ -105,9 +105,9 @@ async def taskflow_add_task(params: AddTaskInput, ctx: Context) -> str: ) return _format_task_result(result, "created") except APIError as e: - return _format_error(e) - except Exception as e: - return json.dumps({"error": True, "message": str(e)}) + raise + except Exception: + raise @mcp.tool( @@ -170,9 +170,9 @@ async def taskflow_list_tasks(params: ListTasksInput, ctx: Context) -> str: ] return json.dumps(result, indent=2) except APIError as e: - return _format_error(e) - except Exception as e: - return json.dumps({"error": True, "message": str(e)}) + raise + except Exception: + raise @mcp.tool( @@ -247,9 +247,9 @@ async def taskflow_update_task(params: UpdateTaskInput, ctx: Context) -> str: ) return _format_task_result(result, "updated") except APIError as e: - return _format_error(e, params.task_id) - except Exception as e: - return json.dumps({"error": True, "message": str(e), "task_id": params.task_id}) + raise + except Exception: + raise @mcp.tool( @@ -292,9 +292,9 @@ async def taskflow_delete_task(params: TaskIdInput, ctx: Context) -> str: indent=2, ) except APIError as e: - return _format_error(e, params.task_id) - except Exception as e: - return json.dumps({"error": True, "message": str(e), "task_id": params.task_id}) + raise + except Exception: + raise # ============================================================================= @@ -338,9 +338,9 @@ async def taskflow_start_task(params: TaskIdInput, ctx: Context) -> str: ) return _format_task_result(result, "in_progress") except APIError as e: - return _format_error(e, params.task_id) - except Exception as e: - return json.dumps({"error": True, "message": str(e), "task_id": params.task_id}) + raise + except Exception: + raise @mcp.tool( @@ -379,9 +379,9 @@ async def taskflow_complete_task(params: TaskIdInput, ctx: Context) -> str: ) return _format_task_result(result, "completed") except APIError as e: - return _format_error(e, params.task_id) - except Exception as e: - return json.dumps({"error": True, "message": str(e), "task_id": params.task_id}) + raise + except Exception: + raise @mcp.tool( @@ -420,9 +420,9 @@ async def taskflow_request_review(params: TaskIdInput, ctx: Context) -> str: ) return _format_task_result(result, "review") except APIError as e: - return _format_error(e, params.task_id) - except Exception as e: - return json.dumps({"error": True, "message": str(e), "task_id": params.task_id}) + raise + except Exception: + raise @mcp.tool( @@ -463,9 +463,9 @@ async def taskflow_update_progress(params: ProgressInput, ctx: Context) -> str: ) return _format_task_result(result, result.get("status", "in_progress")) except APIError as e: - return _format_error(e, params.task_id) - except Exception as e: - return json.dumps({"error": True, "message": str(e), "task_id": params.task_id}) + raise + except Exception: + raise @mcp.tool( @@ -502,6 +502,6 @@ async def taskflow_assign_task(params: AssignInput, ctx: Context) -> str: ) return _format_task_result(result, "assigned") except APIError as e: - return _format_error(e, params.task_id) - except Exception as e: - return json.dumps({"error": True, "message": str(e), "task_id": params.task_id}) + raise + except Exception: + raise diff --git a/apps/mcp-server/tests/test_iserror_compliance.py b/apps/mcp-server/tests/test_iserror_compliance.py new file mode 100644 index 0000000..5354b3a --- /dev/null +++ b/apps/mcp-server/tests/test_iserror_compliance.py @@ -0,0 +1,55 @@ +"""Regression test for isError-compliance in taskflow_mcp task tools. + +9 `except APIError as e:` and 9 `except Exception as e:` handlers in +`apps/mcp-server/src/taskflow_mcp/tools/tasks.py` returned a JSON-encoded +`{"error": True, "message": ...}` string. FastMCP wraps the return +value as success content with `isError=false`, so MCP clients treat +the failure as data and the LLM often proceeds as if the call had +succeeded. + +The fix replaces each swallowed-error return with bare `raise` so the +original exception propagates and FastMCP sets `isError=true` on the +wire. + +Reference: https://composio.dev/blog/mcp-security-vulnerabilities (Dayna +Blackwell MCP security audit, June 2026). +""" +import pytest + +try: + from fastmcp.exceptions import ToolError +except ImportError: + ToolError = None # type: ignore + +pytestmark = pytest.mark.skipif( + ToolError is None, reason="fastmcp not installed; skip when unavailable" +) + + +@pytest.mark.asyncio +async def test_create_task_api_error_raises_tool_error(monkeypatch): + """An APIError during create_task must surface as ToolError + → isError=true on the wire, not as JSON success content.""" + from taskflow_mcp.tools import tasks + + async def _raise(*args, **kwargs): + raise tasks.APIError("upstream down") + + monkeypatch.setattr(tasks, "get_api_client", lambda: _make_client(_raise)) + + with pytest.raises(ToolError) as exc_info: + await tasks.create_task.fn( + user_id="u1", + project_id="p1", + title="t", + description="d", + is_recurring=False, + ) + assert "upstream down" in str(exc_info.value) or "Error" in str(exc_info.value) + + +def _make_client(_raise): + class _C: + async def create_task(self, *args, **kwargs): + return await _raise(*args, **kwargs) + return _C()