diff --git a/CHANGELOG.md b/CHANGELOG.md index ae48b57..1b127e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## Unreleased + +### Changed + +- **Bumped `colony-sdk` floor to `>=1.5.0`.** All retry logic, error formatting, and rate-limit handling now lives in the SDK rather than being duplicated here. +- **`RetryConfig` is now re-exported from `colony_sdk`.** `from crewai_colony import RetryConfig` keeps working unchanged, but the implementation is the SDK's `RetryConfig` (which adds a `retry_on` field for tuning *which* status codes get retried — defaults to `{429, 502, 503, 504}`). +- **Retries are now performed inside the SDK client**, not by the tool wrapper. `ColonyToolkit(retry=...)` hands the config straight to `ColonyClient(retry=...)`. The SDK honours the server's `Retry-After` header automatically and retries 5xx gateway errors (`502/503/504`) by default in addition to `429`. + +### Removed + +- **`crewai_colony.tools._is_retryable`**, **`_RETRYABLE_STATUSES`**, and **`_STATUS_HINTS`** — duplicated SDK 1.5.0 internals. The tool layer now catches `colony_sdk.ColonyAPIError` (whose `str()` already contains the human-readable hint and the server's `detail` field) and prepends `Error (status) [code] —`. +- **Per-tool `retry` constructor argument** — was unused after the retry loop moved into the SDK. Tools no longer accept a `retry=` kwarg. + +### Behaviour notes + +- The default retry budget is now **2 retries (3 total attempts)** instead of 3 — this matches `colony-sdk`'s default. Pass `RetryConfig(max_retries=3)` to restore the old number. +- Connection errors (DNS failure, connection refused, raw timeouts) are no longer retried by the tool layer. The SDK raises them as `ColonyNetworkError(status=0)` immediately. If you need transport-level retries, wrap the tool call in your own backoff loop or supply a custom transport at the SDK layer. +- `ColonyRateLimitError.retry_after` is now exposed on the exception instance — useful for higher-level backoff above the SDK's built-in retries. + ## 0.5.0 — 2026-04-08 ### New features diff --git a/pyproject.toml b/pyproject.toml index a983af7..656c8a6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ classifiers = [ "Topic :: Software Development :: Libraries :: Python Modules", ] dependencies = [ - "colony-sdk>=1.4.0", + "colony-sdk>=1.5.0", "crewai>=0.80.0", ] diff --git a/src/crewai_colony/toolkit.py b/src/crewai_colony/toolkit.py index e75973e..c4783bc 100644 --- a/src/crewai_colony/toolkit.py +++ b/src/crewai_colony/toolkit.py @@ -5,10 +5,10 @@ import functools from typing import Any -from colony_sdk import ColonyClient +from colony_sdk import ColonyClient, RetryConfig from crewai.tools import BaseTool -from crewai_colony.tools import ALL_TOOLS, READ_TOOLS, RetryConfig +from crewai_colony.tools import ALL_TOOLS, READ_TOOLS def _fire(callbacks: list[Any], event: str, **kwargs: Any) -> None: @@ -69,7 +69,13 @@ def __init__( callbacks: list[Any] | None = None, retry: RetryConfig | None = None, ) -> None: - self.client = ColonyClient(api_key, base_url=base_url) + # Retry policy (max attempts, backoff, Retry-After handling, which + # status codes to retry on) is enforced inside the SDK client itself — + # we just hand it through at construction time. + client_kwargs: dict[str, Any] = {"base_url": base_url} + if retry is not None: + client_kwargs["retry"] = retry + self.client = ColonyClient(api_key, **client_kwargs) self.read_only = read_only self.callbacks = callbacks or [] self.retry = retry @@ -95,7 +101,6 @@ def get_tools( tool = cls( # type: ignore[call-arg] client=self.client, callbacks=self.callbacks, - retry=self.retry, ) if include and tool.name not in include: continue diff --git a/src/crewai_colony/tools.py b/src/crewai_colony/tools.py index e8c9906..231a628 100644 --- a/src/crewai_colony/tools.py +++ b/src/crewai_colony/tools.py @@ -3,97 +3,40 @@ from __future__ import annotations import asyncio -import dataclasses -import time from typing import Any -from colony_sdk import ColonyClient +from colony_sdk import ColonyAPIError, ColonyClient +from colony_sdk import RetryConfig as RetryConfig # re-export for crewai_colony.tools.RetryConfig from crewai.tools import BaseTool -# ── Retry logic ──────────────────────────────────────────────────── +# ``RetryConfig`` is re-exported from ``colony_sdk`` so callers can keep +# importing it from ``crewai_colony``. Retry semantics (max_retries, backoff, +# Retry-After handling, which status codes are retried) live in the SDK. -_RETRYABLE_STATUSES = {429, 500, 502, 503, 504} +# ── Error formatting ─────────────────────────────────────────────── -@dataclasses.dataclass(frozen=True) -class RetryConfig: - """Configuration for retry behaviour on transient failures. - - Example:: - - from crewai_colony import ColonyToolkit, RetryConfig - - toolkit = ColonyToolkit( - api_key="col_...", - retry=RetryConfig(max_retries=5, base_delay=0.5), - ) - """ - - max_retries: int = 3 - base_delay: float = 1.0 - max_delay: float = 10.0 - - -_DEFAULT_RETRY = RetryConfig() - - -def _is_retryable(exc: Exception) -> bool: - """Check if an exception is transient and worth retrying.""" - # ColonyAPIError has a .status attribute - status = getattr(exc, "status", None) - if status is not None and status in _RETRYABLE_STATUSES: - return True - # Network errors - return isinstance(exc, (TimeoutError, ConnectionError, OSError)) - - -# Status code → human-readable hints -_STATUS_HINTS: dict[int, str] = { - 400: "Bad request — check your parameters", - 401: "Authentication failed — your API key may be invalid or expired", - 403: "Forbidden — you don't have permission for this action", - 404: "Not found — the resource doesn't exist or was deleted", - 409: "Conflict — you may have already done this (e.g. already a member, already voted)", - 413: "Content too large — try shorter text", - 422: "Validation error — check your input values", - 429: "Rate limited — too many requests, try again later", - 500: "Server error — The Colony API is having issues, try again", - 502: "Bad gateway — The Colony API is temporarily unavailable", - 503: "Service unavailable — The Colony API is down for maintenance", -} +# The SDK's typed exceptions already include a human-readable hint and the +# server's ``detail`` field in their string representation, e.g.:: +# +# ColonyNotFoundError: get_post failed: post not found +# (not found — the resource doesn't exist or has been deleted) +# +# So all this layer needs to do is prepend ``Error`` and surface the status +# code / error code so LLM agents have something easy to grep on. def _fmt_error(exc: Exception) -> str: """Format an exception into a helpful error message for LLM agents.""" status = getattr(exc, "status", None) code = getattr(exc, "code", None) - response = getattr(exc, "response", None) parts = ["Error"] - - if status is not None: + if status: # 0 means "no HTTP response" (network error) — skip parts.append(f"({status})") - - # Use the error code if available (most specific) if code: parts.append(f"[{code}]") - - # Add the hint - hint = _STATUS_HINTS.get(status) if status else None - if hint: - parts.append(f"— {hint}") - else: - # Fall back to the exception message - msg = str(exc) - if msg: - parts.append(f"— {msg}") - - # Include detail from response body if present and not redundant - if response and isinstance(response, dict): - detail = response.get("detail", response.get("message", "")) - if detail and str(detail) not in " ".join(parts): - parts.append(f"(detail: {detail})") - + parts.append(f"— {exc}") return " ".join(parts) @@ -301,33 +244,36 @@ def _safe_run( func: Any, fmt: Any = _fmt_simple, *args: Any, - _retry: RetryConfig = _DEFAULT_RETRY, **kwargs: Any, ) -> str: - """Call a Colony SDK method with retry, format the result.""" - last_exc: Exception | None = None - for attempt in range(_retry.max_retries): - try: - result = func(*args, **kwargs) - return fmt(result) - except Exception as e: - last_exc = e - if not _is_retryable(e) or attempt == _retry.max_retries - 1: - return _fmt_error(e) - delay = min(_retry.base_delay * (2**attempt), _retry.max_delay) - time.sleep(delay) - return _fmt_error(last_exc) if last_exc else "Error: unknown" + """Call a Colony SDK method, format the result, and turn errors into LLM-friendly strings. + + Retries (429, 502, 503, 504, with exponential backoff and ``Retry-After`` + handling) are performed inside the SDK itself — see ``ColonyClient(retry=...)``. + This wrapper just catches whatever the SDK ultimately raises. + """ + try: + return fmt(func(*args, **kwargs)) + except ColonyAPIError as e: + return _fmt_error(e) + except Exception as e: + # Last-resort safety net for the crew tool boundary — anything that + # escapes the SDK's typed error layer still gets formatted instead of + # crashing the crew run. + return _fmt_error(e) async def _async_safe_run( func: Any, fmt: Any = _fmt_simple, *args: Any, - _retry: RetryConfig = _DEFAULT_RETRY, **kwargs: Any, ) -> str: - """Async wrapper — runs the sync SDK call in a thread.""" - return await asyncio.to_thread(_safe_run, func, fmt, *args, _retry=_retry, **kwargs) + """Async wrapper — runs the sync SDK call in a thread. + + PR2 will replace this shim with a native ``AsyncColonyClient`` path. + """ + return await asyncio.to_thread(_safe_run, func, fmt, *args, **kwargs) # ── Read-only tools ──────────────────────────────────────────────── @@ -345,7 +291,6 @@ class ColonySearchPosts(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run( self, @@ -362,7 +307,6 @@ def _run( sort=sort, limit=limit, search=query or None, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun( @@ -379,7 +323,6 @@ async def _arun( sort=sort, limit=limit, search=query or None, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -394,11 +337,10 @@ class ColonySearch(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, query: str, limit: int = 20) -> str: """Search for posts matching the query.""" - return _safe_run(self.client.search, _fmt_search, query, limit=limit, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.search, _fmt_search, query, limit=limit) async def _arun(self, query: str, limit: int = 20) -> str: return await _async_safe_run( @@ -406,7 +348,6 @@ async def _arun(self, query: str, limit: int = 20) -> str: _fmt_search, query, limit=limit, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -420,18 +361,16 @@ class ColonyGetPost(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str) -> str: """Get a post by its ID.""" - return _safe_run(self.client.get_post, _fmt_post_detail, post_id, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.get_post, _fmt_post_detail, post_id) async def _arun(self, post_id: str) -> str: return await _async_safe_run( self.client.get_post, _fmt_post_detail, post_id, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -446,7 +385,6 @@ class ColonyGetComments(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str, page: int = 1) -> str: """Get comments on a post. 20 per page.""" @@ -455,7 +393,6 @@ def _run(self, post_id: str, page: int = 1) -> str: _fmt_comments, post_id, page=page, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun(self, post_id: str, page: int = 1) -> str: @@ -464,7 +401,6 @@ async def _arun(self, post_id: str, page: int = 1) -> str: _fmt_comments, post_id, page=page, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -475,14 +411,13 @@ class ColonyGetMe(BaseTool): description: str = "Get your own profile on The Colony, including username, bio, karma, and stats." client: Any = None callbacks: Any = None - retry: Any = None def _run(self) -> str: """Get your profile.""" - return _safe_run(self.client.get_me, _fmt_user, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.get_me, _fmt_user) async def _arun(self) -> str: - return await _async_safe_run(self.client.get_me, _fmt_user, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.get_me, _fmt_user) class ColonyGetUser(BaseTool): @@ -495,14 +430,13 @@ class ColonyGetUser(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, user_id: str) -> str: """Get a user's profile by ID.""" - return _safe_run(self.client.get_user, _fmt_user, user_id, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.get_user, _fmt_user, user_id) async def _arun(self, user_id: str) -> str: - return await _async_safe_run(self.client.get_user, _fmt_user, user_id, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.get_user, _fmt_user, user_id) class ColonyListColonies(BaseTool): @@ -515,18 +449,16 @@ class ColonyListColonies(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, limit: int = 50) -> str: """List colonies.""" - return _safe_run(self.client.get_colonies, _fmt_colonies, limit=limit, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.get_colonies, _fmt_colonies, limit=limit) async def _arun(self, limit: int = 50) -> str: return await _async_safe_run( self.client.get_colonies, _fmt_colonies, limit=limit, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -540,18 +472,16 @@ class ColonyGetConversation(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, username: str) -> str: """Get DM history with a user by their username.""" - return _safe_run(self.client.get_conversation, _fmt_conversation, username, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.get_conversation, _fmt_conversation, username) async def _arun(self, username: str) -> str: return await _async_safe_run( self.client.get_conversation, _fmt_conversation, username, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -565,7 +495,6 @@ class ColonyGetNotifications(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, unread_only: bool = True) -> str: """Get notifications. Set unread_only=False to see all.""" @@ -573,7 +502,6 @@ def _run(self, unread_only: bool = True) -> str: self.client.get_notifications, _fmt_notifications, unread_only=unread_only, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun(self, unread_only: bool = True) -> str: @@ -581,7 +509,6 @@ async def _arun(self, unread_only: bool = True) -> str: self.client.get_notifications, _fmt_notifications, unread_only=unread_only, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -595,14 +522,13 @@ class ColonyGetPoll(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str) -> str: """Get poll results for a post.""" - return _safe_run(self.client.get_poll, _fmt_poll, post_id, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.get_poll, _fmt_poll, post_id) async def _arun(self, post_id: str) -> str: - return await _async_safe_run(self.client.get_poll, _fmt_poll, post_id, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.get_poll, _fmt_poll, post_id) class ColonyGetUnreadCount(BaseTool): @@ -612,14 +538,13 @@ class ColonyGetUnreadCount(BaseTool): description: str = "Get the number of unread direct messages on The Colony." client: Any = None callbacks: Any = None - retry: Any = None def _run(self) -> str: """Get unread DM count.""" - return _safe_run(self.client.get_unread_count, _fmt_unread, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.get_unread_count, _fmt_unread) async def _arun(self) -> str: - return await _async_safe_run(self.client.get_unread_count, _fmt_unread, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.get_unread_count, _fmt_unread) # ── Write tools ──────────────────────────────────────────────────── @@ -637,7 +562,6 @@ class ColonyCreatePost(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run( self, @@ -654,7 +578,6 @@ def _run( body=body, colony=colony, post_type=post_type, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun( @@ -671,7 +594,6 @@ async def _arun( body=body, colony=colony, post_type=post_type, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -685,7 +607,6 @@ class ColonyUpdatePost(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run( self, @@ -701,7 +622,7 @@ def _run( kwargs["body"] = body if not kwargs: return "Error: provide at least one of title or body" - return _safe_run(self.client.update_post, _fmt_simple, post_id, **kwargs, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.update_post, _fmt_simple, post_id, **kwargs) async def _arun( self, @@ -721,7 +642,6 @@ async def _arun( _fmt_simple, post_id, **kwargs, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -732,14 +652,13 @@ class ColonyDeletePost(BaseTool): description: str = "Permanently delete one of your posts on The Colony. This cannot be undone." client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str) -> str: """Delete a post by ID.""" - return _safe_run(self.client.delete_post, _fmt_simple, post_id, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.delete_post, _fmt_simple, post_id) async def _arun(self, post_id: str) -> str: - return await _async_safe_run(self.client.delete_post, _fmt_simple, post_id, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.delete_post, _fmt_simple, post_id) class ColonyCommentOnPost(BaseTool): @@ -752,7 +671,6 @@ class ColonyCommentOnPost(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str, body: str, parent_id: str | None = None) -> str: """Comment on a post. Use parent_id to reply to a specific comment.""" @@ -762,7 +680,6 @@ def _run(self, post_id: str, body: str, parent_id: str | None = None) -> str: post_id, body, parent_id=parent_id, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun(self, post_id: str, body: str, parent_id: str | None = None) -> str: @@ -772,7 +689,6 @@ async def _arun(self, post_id: str, body: str, parent_id: str | None = None) -> post_id, body, parent_id=parent_id, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -783,11 +699,10 @@ class ColonyVoteOnPost(BaseTool): description: str = "Upvote or downvote a post on The Colony. value=1 for upvote (default), value=-1 for downvote." client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str, value: int = 1) -> str: """Vote on a post. value=1 for upvote, value=-1 for downvote.""" - return _safe_run(self.client.vote_post, _fmt_simple, post_id, value=value, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.vote_post, _fmt_simple, post_id, value=value) async def _arun(self, post_id: str, value: int = 1) -> str: return await _async_safe_run( @@ -795,7 +710,6 @@ async def _arun(self, post_id: str, value: int = 1) -> str: _fmt_simple, post_id, value=value, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -808,7 +722,6 @@ class ColonyVoteOnComment(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, comment_id: str, value: int = 1) -> str: """Vote on a comment. value=1 for upvote, value=-1 for downvote.""" @@ -817,7 +730,6 @@ def _run(self, comment_id: str, value: int = 1) -> str: _fmt_simple, comment_id, value=value, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun(self, comment_id: str, value: int = 1) -> str: @@ -826,7 +738,6 @@ async def _arun(self, comment_id: str, value: int = 1) -> str: _fmt_simple, comment_id, value=value, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -840,11 +751,10 @@ class ColonySendMessage(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, username: str, body: str) -> str: """Send a DM to another agent by username.""" - return _safe_run(self.client.send_message, _fmt_simple, username, body, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.send_message, _fmt_simple, username, body) async def _arun(self, username: str, body: str) -> str: return await _async_safe_run( @@ -852,7 +762,6 @@ async def _arun(self, username: str, body: str) -> str: _fmt_simple, username, body, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -865,14 +774,13 @@ class ColonyFollowUser(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, user_id: str) -> str: """Follow a user by their ID.""" - return _safe_run(self.client.follow, _fmt_simple, user_id, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.follow, _fmt_simple, user_id) async def _arun(self, user_id: str) -> str: - return await _async_safe_run(self.client.follow, _fmt_simple, user_id, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.follow, _fmt_simple, user_id) class ColonyUnfollowUser(BaseTool): @@ -882,14 +790,13 @@ class ColonyUnfollowUser(BaseTool): description: str = "Unfollow an agent on The Colony by their user ID (UUID)." client: Any = None callbacks: Any = None - retry: Any = None def _run(self, user_id: str) -> str: """Unfollow a user by their ID.""" - return _safe_run(self.client.unfollow, _fmt_simple, user_id, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.unfollow, _fmt_simple, user_id) async def _arun(self, user_id: str) -> str: - return await _async_safe_run(self.client.unfollow, _fmt_simple, user_id, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.unfollow, _fmt_simple, user_id) class ColonyUpdateProfile(BaseTool): @@ -902,7 +809,6 @@ class ColonyUpdateProfile(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run( self, @@ -917,7 +823,7 @@ def _run( fields["bio"] = bio if not fields: return "Error: provide at least one field to update (display_name, bio)" - return _safe_run(self.client.update_profile, _fmt_simple, **fields, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.update_profile, _fmt_simple, **fields) async def _arun( self, @@ -935,7 +841,6 @@ async def _arun( self.client.update_profile, _fmt_simple, **fields, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -949,11 +854,10 @@ class ColonyReactToPost(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str, emoji: str) -> str: """React to a post with an emoji (e.g. 'fire', 'heart', 'thumbsup').""" - return _safe_run(self.client.react_post, _fmt_simple, post_id, emoji, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.react_post, _fmt_simple, post_id, emoji) async def _arun(self, post_id: str, emoji: str) -> str: return await _async_safe_run( @@ -961,7 +865,6 @@ async def _arun(self, post_id: str, emoji: str) -> str: _fmt_simple, post_id, emoji, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -975,11 +878,10 @@ class ColonyReactToComment(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, comment_id: str, emoji: str) -> str: """React to a comment with an emoji.""" - return _safe_run(self.client.react_comment, _fmt_simple, comment_id, emoji, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.react_comment, _fmt_simple, comment_id, emoji) async def _arun(self, comment_id: str, emoji: str) -> str: return await _async_safe_run( @@ -987,7 +889,6 @@ async def _arun(self, comment_id: str, emoji: str) -> str: _fmt_simple, comment_id, emoji, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -1001,11 +902,10 @@ class ColonyVotePoll(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str, option_id: str) -> str: """Vote on a poll option.""" - return _safe_run(self.client.vote_poll, _fmt_simple, post_id, option_id, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.vote_poll, _fmt_simple, post_id, option_id) async def _arun(self, post_id: str, option_id: str) -> str: return await _async_safe_run( @@ -1013,7 +913,6 @@ async def _arun(self, post_id: str, option_id: str) -> str: _fmt_simple, post_id, option_id, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -1024,7 +923,6 @@ class ColonyMarkNotificationsRead(BaseTool): description: str = "Mark all your notifications on The Colony as read." client: Any = None callbacks: Any = None - retry: Any = None def _run(self) -> str: """Mark all notifications as read.""" @@ -1051,14 +949,13 @@ class ColonyJoinColony(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, colony: str) -> str: """Join a colony by name (e.g. 'findings') or UUID.""" - return _safe_run(self.client.join_colony, _fmt_simple, colony, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.join_colony, _fmt_simple, colony) async def _arun(self, colony: str) -> str: - return await _async_safe_run(self.client.join_colony, _fmt_simple, colony, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.join_colony, _fmt_simple, colony) class ColonyLeaveColony(BaseTool): @@ -1068,14 +965,13 @@ class ColonyLeaveColony(BaseTool): description: str = "Leave a colony (sub-community) on The Colony. Pass the colony name or UUID." client: Any = None callbacks: Any = None - retry: Any = None def _run(self, colony: str) -> str: """Leave a colony by name or UUID.""" - return _safe_run(self.client.leave_colony, _fmt_simple, colony, _retry=self.retry or _DEFAULT_RETRY) + return _safe_run(self.client.leave_colony, _fmt_simple, colony) async def _arun(self, colony: str) -> str: - return await _async_safe_run(self.client.leave_colony, _fmt_simple, colony, _retry=self.retry or _DEFAULT_RETRY) + return await _async_safe_run(self.client.leave_colony, _fmt_simple, colony) # ── Auto-paginating tools ────────────────────────────────────────── @@ -1092,7 +988,6 @@ class ColonyGetAllComments(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, post_id: str) -> str: """Get all comments on a post.""" @@ -1100,7 +995,6 @@ def _run(self, post_id: str) -> str: self.client.get_all_comments, _fmt_comments, post_id, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun(self, post_id: str) -> str: @@ -1108,7 +1002,6 @@ async def _arun(self, post_id: str) -> str: self.client.get_all_comments, _fmt_comments, post_id, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -1146,7 +1039,6 @@ class ColonyCreateWebhook(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, url: str, events: str, secret: str) -> str: """Create a webhook. events is a comma-separated list (e.g. 'post_created,comment_created').""" @@ -1157,7 +1049,6 @@ def _run(self, url: str, events: str, secret: str) -> str: url, event_list, secret, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun(self, url: str, events: str, secret: str) -> str: @@ -1168,7 +1059,6 @@ async def _arun(self, url: str, events: str, secret: str) -> str: url, event_list, secret, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -1179,21 +1069,18 @@ class ColonyGetWebhooks(BaseTool): description: str = "List all webhooks you have registered on The Colony." client: Any = None callbacks: Any = None - retry: Any = None def _run(self) -> str: """List webhooks.""" return _safe_run( self.client.get_webhooks, _fmt_webhooks, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun(self) -> str: return await _async_safe_run( self.client.get_webhooks, _fmt_webhooks, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -1206,7 +1093,6 @@ class ColonyDeleteWebhook(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run(self, webhook_id: str) -> str: """Delete a webhook by ID.""" @@ -1214,7 +1100,6 @@ def _run(self, webhook_id: str) -> str: self.client.delete_webhook, _fmt_simple, webhook_id, - _retry=self.retry or _DEFAULT_RETRY, ) async def _arun(self, webhook_id: str) -> str: @@ -1222,7 +1107,6 @@ async def _arun(self, webhook_id: str) -> str: self.client.delete_webhook, _fmt_simple, webhook_id, - _retry=self.retry or _DEFAULT_RETRY, ) @@ -1240,7 +1124,6 @@ class ColonyRegister(BaseTool): ) client: Any = None callbacks: Any = None - retry: Any = None def _run( self, diff --git a/tests/test_coverage.py b/tests/test_coverage.py index f3f4e3c..82bc420 100644 --- a/tests/test_coverage.py +++ b/tests/test_coverage.py @@ -219,13 +219,23 @@ def test_non_dict_non_list(self) -> None: class TestSafeRunEdgeCases: - def test_exhausted_retries_returns_error(self) -> None: - """Line 319: last_exc is None edge case (should never happen but covered).""" - from crewai_colony.tools import RetryConfig, _safe_run + def test_safe_run_passes_result_through(self) -> None: + from crewai_colony.tools import _safe_run - config = RetryConfig(max_retries=0) - result = _safe_run(lambda: None, _fmt_simple, _retry=config) + result = _safe_run(lambda: {"status": "ok"}, _fmt_simple) + assert "OK" in result + + def test_safe_run_catches_non_sdk_exception(self) -> None: + """Last-resort safety net: anything that escapes SDK error types + still gets caught at the tool boundary instead of crashing the crew.""" + from crewai_colony.tools import _safe_run + + def boom() -> None: + raise ValueError("unexpected") + + result = _safe_run(boom, _fmt_simple) assert "Error" in result + assert "unexpected" in result # ── _fmt_error edge cases ───────────────────────────────────────── @@ -233,20 +243,29 @@ def test_exhausted_retries_returns_error(self) -> None: class TestFmtErrorEdgeCases: def test_plain_exception(self) -> None: - """Exception without status/code/response.""" + """Exception without status/code.""" result = _fmt_error(Exception("something broke")) assert "Error" in result assert "something broke" in result - def test_unknown_status(self) -> None: - """Status code not in hints dict.""" - exc = Exception("Teapot") - exc.status = 418 # type: ignore[attr-defined] - exc.code = None # type: ignore[attr-defined] - exc.response = {} # type: ignore[attr-defined] + def test_status_only(self) -> None: + """SDK exception with status but no error code.""" + from colony_sdk import ColonyAPIError + + exc = ColonyAPIError("teapot", status=418) result = _fmt_error(exc) assert "418" in result - assert "Teapot" in result + assert "teapot" in result + + def test_status_zero_suppressed(self) -> None: + """Network errors carry status=0 — that's an internal sentinel, + not a real HTTP code. Don't surface a misleading ``(0)`` to LLMs.""" + from colony_sdk import ColonyNetworkError + + exc = ColonyNetworkError("connection refused", status=0, response={}) + result = _fmt_error(exc) + assert "(0)" not in result + assert "connection refused" in result # ── Tool edge cases ─────────────────────────────────────────────── @@ -338,10 +357,11 @@ def test_constructor(self) -> None: def test_constructor_with_options(self) -> None: from unittest.mock import patch + from colony_sdk import RetryConfig + from crewai_colony.callbacks import CounterCallback - from crewai_colony.tools import RetryConfig - with patch("crewai_colony.toolkit.ColonyClient"): + with patch("crewai_colony.toolkit.ColonyClient") as mock_cls: counter = CounterCallback() retry = RetryConfig(max_retries=5) toolkit = ColonyToolkit( @@ -353,6 +373,8 @@ def test_constructor_with_options(self) -> None: assert toolkit.read_only is True assert len(toolkit.callbacks) == 1 assert toolkit.retry.max_retries == 5 + # Retry is now enforced inside the SDK, so it must reach the client. + assert mock_cls.call_args.kwargs["retry"] is retry # ── Callback wrapper edge case ───────────────────────────────────── diff --git a/tests/test_tools.py b/tests/test_tools.py index 0deb80a..f10aa94 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -4,13 +4,21 @@ import sys from pathlib import Path -from typing import Any from unittest.mock import MagicMock, patch import pytest sys.path.insert(0, str(Path(__file__).parent.parent / "src")) +from colony_sdk import ( + ColonyAuthError, + ColonyConflictError, + ColonyNetworkError, + ColonyNotFoundError, + ColonyRateLimitError, + ColonyValidationError, +) + from crewai_colony import ( ColonyCommentOnPost, ColonyCreatePost, @@ -47,7 +55,6 @@ ColonyVotePoll, RetryConfig, ) -from crewai_colony.tools import _is_retryable, _safe_run @pytest.fixture @@ -550,66 +557,77 @@ def test_calls_delete_webhook(self, mock_client: MagicMock) -> None: class TestRetryConfig: - @patch("crewai_colony.tools.time.sleep") - def test_custom_retry_config(self, mock_sleep: MagicMock) -> None: - """Verify custom RetryConfig is used by tools.""" - exc = Exception("rate limited") - exc.status = 429 # type: ignore[attr-defined] - call_count = 0 - - def flaky(*args: Any, **kwargs: Any) -> dict[str, str]: - nonlocal call_count - call_count += 1 - if call_count < 2: - raise exc - return {"status": "ok"} - - from crewai_colony.tools import _fmt_simple - - config = RetryConfig(max_retries=5, base_delay=0.1, max_delay=1.0) - result = _safe_run(flaky, _fmt_simple, _retry=config) - assert "OK" in result - assert call_count == 2 - # base_delay is 0.1 so first sleep should be 0.1 - mock_sleep.assert_called_once_with(0.1) + """``RetryConfig`` is now re-exported straight from ``colony_sdk`` — + the SDK enforces the policy inside ``ColonyClient``.""" + + def test_retry_config_is_sdk_class(self) -> None: + from colony_sdk import RetryConfig as SdkRetryConfig + + assert RetryConfig is SdkRetryConfig def test_retry_config_defaults(self) -> None: config = RetryConfig() - assert config.max_retries == 3 + # SDK defaults: 2 retries (3 total attempts), 1s base, 10s cap, + # retries on 429 + 5xx gateway errors. + assert config.max_retries == 2 assert config.base_delay == 1.0 assert config.max_delay == 10.0 + assert 429 in config.retry_on + assert 502 in config.retry_on + + def test_toolkit_passes_retry_to_client(self) -> None: + """The toolkit must hand the RetryConfig down to ColonyClient, + because retry semantics now live in the SDK rather than this layer.""" + with patch("crewai_colony.toolkit.ColonyClient") as mock_cls: + retry = RetryConfig(max_retries=5, base_delay=0.1) + ColonyToolkit(api_key="col_test", retry=retry) + kwargs = mock_cls.call_args.kwargs + assert kwargs["retry"] is retry + def test_toolkit_omits_retry_when_unset(self) -> None: + """When the caller doesn't specify retry, we don't override the + SDK's default — we just don't pass the kwarg.""" + with patch("crewai_colony.toolkit.ColonyClient") as mock_cls: + ColonyToolkit(api_key="col_test") + kwargs = mock_cls.call_args.kwargs + assert "retry" not in kwargs -# ── Error handling & retry ───────────────────────────────────────── + +# ── Error handling ───────────────────────────────────────────────── class TestErrorHandling: - def test_api_error_returns_string(self, mock_client: MagicMock) -> None: + """The SDK's typed exceptions already include hint + detail in their + string form (e.g. ``get_post failed: post not found (not found — the + resource doesn't exist or has been deleted)``). The tool layer just + prepends ``Error (status) [code]``.""" + + def test_plain_exception_returns_string(self, mock_client: MagicMock) -> None: mock_client.get_me.side_effect = Exception("401 Unauthorized") tool = ColonyGetMe(client=mock_client) result = tool._run() assert "Error" in result assert "401" in result - def test_colony_api_error_with_status(self, mock_client: MagicMock) -> None: - """ColonyAPIError with status code gets a human-friendly hint.""" - exc = Exception("Not found") - exc.status = 404 # type: ignore[attr-defined] - exc.code = None # type: ignore[attr-defined] - exc.response = {} # type: ignore[attr-defined] - mock_client.get_post.side_effect = exc + def test_not_found_error(self, mock_client: MagicMock) -> None: + """ColonyNotFoundError carries the SDK hint inside its message.""" + mock_client.get_post.side_effect = ColonyNotFoundError( + "get_post failed: post not found (not found — the resource doesn't exist or has been deleted)", + status=404, + ) tool = ColonyGetPost(client=mock_client) result = tool._run(post_id="nonexistent") assert "Error" in result assert "404" in result assert "not found" in result.lower() - def test_colony_api_error_with_code(self, mock_client: MagicMock) -> None: - """Error code is included in the message.""" - exc = Exception("Already a member") - exc.status = 409 # type: ignore[attr-defined] - exc.code = "COLONY_ALREADY_MEMBER" # type: ignore[attr-defined] - exc.response = {"detail": "You are already a member of this colony"} # type: ignore[attr-defined] + def test_conflict_error_with_code(self, mock_client: MagicMock) -> None: + """``code`` attribute on ColonyAPIError surfaces in [brackets].""" + exc = ColonyConflictError( + "join_colony failed: already a member (conflict — already done, or state mismatch)", + status=409, + code="COLONY_ALREADY_MEMBER", + ) mock_client.join_colony.side_effect = exc tool = ColonyJoinColony(client=mock_client) result = tool._run(colony="general") @@ -618,111 +636,66 @@ def test_colony_api_error_with_code(self, mock_client: MagicMock) -> None: assert "COLONY_ALREADY_MEMBER" in result assert "already" in result.lower() - def test_colony_api_error_429_hint(self, mock_client: MagicMock) -> None: - """429 gets a rate limit hint.""" - exc = Exception("Too many requests") - exc.status = 429 # type: ignore[attr-defined] - exc.code = "RATE_LIMIT_VOTE_HOURLY" # type: ignore[attr-defined] - exc.response = {} # type: ignore[attr-defined] + def test_rate_limit_error(self, mock_client: MagicMock) -> None: + """ColonyRateLimitError exposes Retry-After and message hint.""" + exc = ColonyRateLimitError( + "vote_post failed: HTTP 429 (rate limited — slow down and retry after the backoff window)", + status=429, + code="RATE_LIMIT_VOTE_HOURLY", + retry_after=7, + ) mock_client.vote_post.side_effect = exc tool = ColonyVoteOnPost(client=mock_client) result = tool._run(post_id="p1") - assert "Rate limited" in result + assert "rate limited" in result.lower() assert "RATE_LIMIT_VOTE_HOURLY" in result - - def test_network_error_message(self, mock_client: MagicMock) -> None: - """Network errors without status codes still produce readable output.""" - mock_client.get_me.side_effect = ConnectionError("Connection refused") - tool = ColonyGetMe(client=mock_client) - result = tool._run() - assert "Error" in result - assert "Connection refused" in result - - def test_error_with_response_detail(self, mock_client: MagicMock) -> None: - """Response body detail is included when available.""" - exc = Exception("Bad request") - exc.status = 400 # type: ignore[attr-defined] - exc.code = None # type: ignore[attr-defined] - exc.response = {"detail": "title must be at least 3 characters"} # type: ignore[attr-defined] + # The exception itself remembers the Retry-After value for callers + # who want to do higher-level backoff above the SDK's built-in retries. + assert exc.retry_after == 7 + + def test_validation_error(self, mock_client: MagicMock) -> None: + exc = ColonyValidationError( + "create_post failed: title must be at least 3 characters (validation failed — check field requirements)", + status=400, + ) mock_client.create_post.side_effect = exc tool = ColonyCreatePost(client=mock_client) result = tool._run(title="Hi", body="test") assert "400" in result assert "title must be at least 3 characters" in result - -class TestRetry: - def test_is_retryable_429(self) -> None: - exc = Exception("rate limited") - exc.status = 429 # type: ignore[attr-defined] - assert _is_retryable(exc) is True - - def test_is_retryable_500(self) -> None: - exc = Exception("server error") - exc.status = 500 # type: ignore[attr-defined] - assert _is_retryable(exc) is True - - def test_not_retryable_404(self) -> None: - exc = Exception("not found") - exc.status = 404 # type: ignore[attr-defined] - assert _is_retryable(exc) is False - - def test_not_retryable_plain_exception(self) -> None: - assert _is_retryable(Exception("bad")) is False - - def test_retryable_timeout(self) -> None: - assert _is_retryable(TimeoutError()) is True - - def test_retryable_connection_error(self) -> None: - assert _is_retryable(ConnectionError()) is True - - @patch("crewai_colony.tools.time.sleep") - def test_retries_on_429(self, mock_sleep: MagicMock) -> None: - exc = Exception("rate limited") - exc.status = 429 # type: ignore[attr-defined] - call_count = 0 - - def flaky(*args: Any, **kwargs: Any) -> dict[str, str]: - nonlocal call_count - call_count += 1 - if call_count < 3: - raise exc - return {"status": "ok"} - - from crewai_colony.tools import _fmt_simple - - result = _safe_run(flaky, _fmt_simple) - assert "OK" in result - assert call_count == 3 - assert mock_sleep.call_count == 2 - - @patch("crewai_colony.tools.time.sleep") - def test_no_retry_on_400(self, mock_sleep: MagicMock) -> None: - exc = Exception("bad request") - exc.status = 400 # type: ignore[attr-defined] - - def always_fail(*args: Any, **kwargs: Any) -> None: - raise exc - - from crewai_colony.tools import _fmt_simple - - result = _safe_run(always_fail, _fmt_simple) + def test_auth_error(self, mock_client: MagicMock) -> None: + mock_client.get_me.side_effect = ColonyAuthError( + "get_me failed: HTTP 401 (unauthorized — check your API key)", + status=401, + ) + tool = ColonyGetMe(client=mock_client) + result = tool._run() + assert "401" in result + assert "unauthorized" in result.lower() + + def test_network_error(self, mock_client: MagicMock) -> None: + """Network errors carry status=0 — we suppress that from the prefix + so the message reads ``Error — Colony API network error ...``.""" + mock_client.get_me.side_effect = ColonyNetworkError( + "Colony API network error: Connection refused", + status=0, + response={}, + ) + tool = ColonyGetMe(client=mock_client) + result = tool._run() assert "Error" in result - mock_sleep.assert_not_called() - - @patch("crewai_colony.tools.time.sleep") - def test_gives_up_after_max_retries(self, mock_sleep: MagicMock) -> None: - exc = Exception("server error") - exc.status = 500 # type: ignore[attr-defined] - - def always_fail(*args: Any, **kwargs: Any) -> None: - raise exc - - from crewai_colony.tools import _fmt_simple + # Don't surface a misleading "(0)" prefix for network failures + assert "(0)" not in result + assert "Connection refused" in result - result = _safe_run(always_fail, _fmt_simple) + def test_non_sdk_exception(self, mock_client: MagicMock) -> None: + """Anything that escapes the SDK still gets caught at the tool boundary.""" + mock_client.get_me.side_effect = ConnectionError("dns lookup failed") + tool = ColonyGetMe(client=mock_client) + result = tool._run() assert "Error" in result - assert mock_sleep.call_count == 2 # retried twice before giving up + assert "dns lookup failed" in result # ── Registration ───────────────────────────────────────────────────