diff --git a/phone_agent/actions/handler.py b/phone_agent/actions/handler.py index 0bef1c3a2..d7932b4cc 100644 --- a/phone_agent/actions/handler.py +++ b/phone_agent/actions/handler.py @@ -223,10 +223,22 @@ def _handle_long_press(self, action: dict, width: int, height: int) -> ActionRes def _handle_wait(self, action: dict, width: int, height: int) -> ActionResult: """Handle wait action.""" - duration_str = action.get("duration", "1 seconds") + duration_raw = action.get("duration", "1 seconds") try: - duration = float(duration_str.replace("seconds", "").strip()) - except ValueError: + # The model normally emits a string such as "3 seconds", but some + # models (or non-standard prompts) return a bare int/float instead. + # Calling ``.replace`` on a number raises ``AttributeError``, which + # the previous ``except ValueError`` did not catch, so the wait + # silently failed. Handle both forms explicitly. + if isinstance(duration_raw, (int, float)): + duration = float(duration_raw) + else: + duration = float(str(duration_raw).replace("seconds", "").strip()) + except (ValueError, TypeError): + duration = 1.0 + + # ``time.sleep`` raises ``ValueError`` for negative values, so clamp. + if duration < 0: duration = 1.0 time.sleep(duration) diff --git a/tests/test_wait_action.py b/tests/test_wait_action.py new file mode 100644 index 000000000..251acd2c6 --- /dev/null +++ b/tests/test_wait_action.py @@ -0,0 +1,77 @@ +"""Unit tests for the ``Wait`` action handler. + +These tests exercise ``ActionHandler._handle_wait`` via the public ``execute`` +entry point. They are pure and deterministic — ``time.sleep`` is patched so no +real time is spent and no device or network access is required. +""" + +from unittest.mock import patch + +from phone_agent.actions.handler import ActionHandler + + +def _wait_action(**kwargs): + action = {"_metadata": "do", "action": "Wait"} + action.update(kwargs) + return action + + +class TestHandleWait: + """Tests for ``ActionHandler._handle_wait`` robustness.""" + + def test_string_duration_with_unit(self): + handler = ActionHandler() + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(duration="3 seconds"), 1080, 1920) + assert result.success is True + assert result.should_finish is False + sleep.assert_called_once_with(3.0) + + def test_string_duration_without_unit(self): + handler = ActionHandler() + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(duration="2"), 1080, 1920) + assert result.success is True + sleep.assert_called_once_with(2.0) + + def test_default_duration_when_missing(self): + handler = ActionHandler() + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(), 1080, 1920) + assert result.success is True + sleep.assert_called_once_with(1.0) + + def test_integer_duration_does_not_crash(self): + """A bare int duration must not raise AttributeError. + + Previously ``int.replace`` raised ``AttributeError`` which the + ``except ValueError`` clause did not catch, so the wait silently + failed with ``success=False``. + """ + handler = ActionHandler() + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(duration=3), 1080, 1920) + assert result.success is True + sleep.assert_called_once_with(3.0) + + def test_float_duration(self): + handler = ActionHandler() + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(duration=1.5), 1080, 1920) + assert result.success is True + sleep.assert_called_once_with(1.5) + + def test_unparseable_duration_falls_back(self): + handler = ActionHandler() + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(duration="soon"), 1080, 1920) + assert result.success is True + sleep.assert_called_once_with(1.0) + + def test_negative_duration_is_clamped(self): + """A negative duration must be clamped so time.sleep does not raise.""" + handler = ActionHandler() + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(duration="-5 seconds"), 1080, 1920) + assert result.success is True + sleep.assert_called_once_with(1.0)