Skip to content

Commit ff94734

Browse files
joaomdmouraclaude
andcommitted
fix(tools): reject NaN waits and pluralize single-second results
_resolve_duration now rejects NaN with its own message instead of letting time.sleep raise "Invalid value NaN (not a number)" from a positional call. Infinity keeps clamping to the cap like any other oversized wait. Result and description text no longer says "1 seconds". Tests use the public WaitTool().description as the baseline rather than reaching for module-private helpers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f343936 commit ff94734

2 files changed

Lines changed: 52 additions & 14 deletions

File tree

lib/crewai-tools/src/crewai_tools/tools/wait_tool/wait_tool.py

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Tool that pauses execution for a given amount of time."""
22

33
import asyncio
4+
import math
45
import time
56
from typing import Any
67

@@ -25,13 +26,25 @@
2526
"Do not use this to pace a conversation, to pretend to work, or when the "
2627
"information needed is already available. Waiting only lets clock time pass, it "
2728
"does not advance or check the job.\n"
28-
"A single call waits at most {max_seconds} seconds. If more time is needed, "
29+
"A single call waits at most {cap}. If more time is needed, "
2930
"call this tool again."
3031
)
3132

3233
# Everything before the cap figure; used to tell a generated description from one
3334
# the caller wrote, so only generated text is kept in sync with ``max_seconds``.
34-
_GENERATED_DESCRIPTION_PREFIX = _DESCRIPTION_TEMPLATE.split("{max_seconds}")[0]
35+
_GENERATED_DESCRIPTION_PREFIX = _DESCRIPTION_TEMPLATE.split("{cap}")[0]
36+
37+
38+
def _format_seconds(value: float) -> str:
39+
"""Render a duration with a correctly pluralized unit.
40+
41+
Args:
42+
value: The duration in seconds.
43+
44+
Returns:
45+
The duration and its unit, e.g. ``"1 second"`` or ``"300 seconds"``.
46+
"""
47+
return f"{value:g} second" if value == 1 else f"{value:g} seconds"
3548

3649

3750
def _build_description(max_seconds: float) -> str:
@@ -43,7 +56,7 @@ def _build_description(max_seconds: float) -> str:
4356
Returns:
4457
The tool description shown to the model.
4558
"""
46-
return _DESCRIPTION_TEMPLATE.format(max_seconds=f"{max_seconds:g}")
59+
return _DESCRIPTION_TEMPLATE.format(cap=_format_seconds(max_seconds))
4760

4861

4962
def _is_generated_description(description: str) -> bool:
@@ -145,8 +158,9 @@ def _resolve_duration(self, seconds: float) -> tuple[float, bool]:
145158
"""Validate and clamp the requested duration to ``max_seconds``.
146159
147160
``BaseTool.run`` skips ``args_schema`` validation when called with
148-
positional arguments, so the non-negative bound is enforced here too
149-
rather than left to ``time.sleep`` to reject.
161+
positional arguments, so the bounds are enforced here too rather than
162+
left to ``time.sleep`` to reject. Infinity is a valid request: it clamps
163+
to the cap like any other oversized wait.
150164
151165
Args:
152166
seconds: The requested wait duration.
@@ -155,8 +169,10 @@ def _resolve_duration(self, seconds: float) -> tuple[float, bool]:
155169
A tuple of the duration to actually wait and whether it was capped.
156170
157171
Raises:
158-
ValueError: If ``seconds`` is negative.
172+
ValueError: If ``seconds`` is negative or not a number.
159173
"""
174+
if math.isnan(seconds):
175+
raise ValueError("seconds must be a number, got NaN.")
160176
if seconds < 0:
161177
raise ValueError(f"seconds must be zero or greater, got {seconds:g}.")
162178
if seconds > self.max_seconds:
@@ -176,10 +192,11 @@ def _format_result(
176192
Returns:
177193
A summary of how long was waited and whether the request was capped.
178194
"""
179-
parts = [f"Waited {waited:g} seconds."]
195+
parts = [f"Waited {_format_seconds(waited)}."]
180196
if waited < requested:
181197
parts.append(
182-
f"Requested {requested:g} seconds, capped at {self.max_seconds:g} seconds per call - "
198+
f"Requested {_format_seconds(requested)}, capped at "
199+
f"{_format_seconds(self.max_seconds)} per call - "
183200
"call this tool again if more waiting is needed."
184201
)
185202
if reason:

lib/crewai-tools/tests/tools/wait_tool_test.py

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
from unittest.mock import AsyncMock, patch
22

33
from crewai_tools.tools.wait_tool import WaitTool
4-
from crewai_tools.tools.wait_tool.wait_tool import (
5-
DEFAULT_MAX_SECONDS,
6-
_build_description,
7-
)
84
from pydantic import ValidationError
95
import pytest
106

@@ -80,6 +76,31 @@ async def test_async_negative_seconds_is_rejected_when_passed_positionally(tool)
8076
mock_sleep.assert_not_awaited()
8177

8278

79+
@patch("crewai_tools.tools.wait_tool.wait_tool.time.sleep")
80+
def test_nan_seconds_is_rejected_when_passed_positionally(mock_sleep, tool):
81+
with pytest.raises(ValueError, match="seconds must be a number"):
82+
tool.run(float("nan"))
83+
84+
mock_sleep.assert_not_called()
85+
86+
87+
@patch("crewai_tools.tools.wait_tool.wait_tool.time.sleep")
88+
def test_infinite_seconds_is_capped_like_any_other_long_wait(mock_sleep, tool):
89+
result = tool.run(float("inf"))
90+
91+
mock_sleep.assert_called_once_with(300)
92+
assert "Waited 300 seconds." in result
93+
94+
95+
@patch("crewai_tools.tools.wait_tool.wait_tool.time.sleep")
96+
def test_singular_second_is_not_pluralized(mock_sleep):
97+
tool = WaitTool(max_seconds=1)
98+
99+
assert "Waited 1 second." in tool.run(seconds=1)
100+
assert "capped at 1 second per call" in tool.run(seconds=5)
101+
assert "at most 1 second." in tool.description
102+
103+
83104
def test_invalid_max_seconds_is_rejected():
84105
with pytest.raises(ValidationError):
85106
WaitTool(max_seconds=0)
@@ -114,7 +135,7 @@ async def test_async_wait_caps_long_waits(tool):
114135
],
115136
)
116137
def test_advertised_cap_matches_enforced_cap(build):
117-
tool = build(_build_description(DEFAULT_MAX_SECONDS))
138+
tool = build(WaitTool().description)
118139

119140
assert tool.max_seconds == 10
120141
assert "at most 10 seconds" in tool.description
@@ -139,7 +160,7 @@ def test_waits_are_never_cached():
139160
# turning a poll-wait-check loop into a busy loop.
140161
tool = WaitTool()
141162

142-
assert tool.cache_function("{}", "Waited 1 seconds.") is False
163+
assert tool.cache_function("{}", "Waited 1 second.") is False
143164
assert WaitTool.model_validate(tool.model_dump()).cache_function() is False
144165

145166

0 commit comments

Comments
 (0)