From 2371fbbb2fecc30c97b913975596a38397b0f7cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BA=91=E9=BE=99?= <76432572+nankingjing@users.noreply.github.com> Date: Sat, 11 Jul 2026 00:27:51 +0800 Subject: [PATCH 1/3] fix: handle bare numeric and negative durations in Wait action handler --- phone_agent/actions/handler.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) 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) From b221704c06d2bfe4b92c0f4894d524ec9509abf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BA=91=E9=BE=99?= <76432572+nankingjing@users.noreply.github.com> Date: Sat, 11 Jul 2026 00:28:02 +0800 Subject: [PATCH 2/3] test: add unit tests for Wait action duration handling --- tests/test_wait_action.py | 77 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/test_wait_action.py 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) From 46f1012d3b99781a77cf135820370463e40563a6 Mon Sep 17 00:00:00 2001 From: nankingjing <76432572+nankingjing@users.noreply.github.com> Date: Tue, 14 Jul 2026 01:29:54 +0000 Subject: [PATCH 3/3] fix: clamp non-finite Wait durations NaN and infinite durations would be accepted by time.sleep but produce unexpected behaviour (no wait or an indefinite hang). Clamp them to the default 1.0 second alongside negative values, and add regression tests. --- phone_agent/actions/handler.py | 7 +++++-- tests/test_wait_action.py | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/phone_agent/actions/handler.py b/phone_agent/actions/handler.py index d7932b4cc..2ebbe9f97 100644 --- a/phone_agent/actions/handler.py +++ b/phone_agent/actions/handler.py @@ -1,6 +1,7 @@ """Action handler for processing AI model outputs.""" import ast +import math import re import subprocess import time @@ -237,8 +238,10 @@ def _handle_wait(self, action: dict, width: int, height: int) -> ActionResult: except (ValueError, TypeError): duration = 1.0 - # ``time.sleep`` raises ``ValueError`` for negative values, so clamp. - if duration < 0: + # ``time.sleep`` accepts ``float('nan')`` and ``float('inf')`` as valid + # arguments, but neither is useful here: NaN produces no wait and inf + # would hang the agent. Clamp non-finite values to the default. + if duration < 0 or not math.isfinite(duration): duration = 1.0 time.sleep(duration) diff --git a/tests/test_wait_action.py b/tests/test_wait_action.py index 251acd2c6..c269f7359 100644 --- a/tests/test_wait_action.py +++ b/tests/test_wait_action.py @@ -75,3 +75,21 @@ def test_negative_duration_is_clamped(self): result = handler.execute(_wait_action(duration="-5 seconds"), 1080, 1920) assert result.success is True sleep.assert_called_once_with(1.0) + + def test_non_finite_duration_falls_back(self): + """NaN and infinite durations must fall back to the default.""" + handler = ActionHandler() + for raw in ("nan", "inf", "NaN", "+Infinity"): + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(duration=raw), 1080, 1920) + assert result.success is True, f"duration={raw!r} should not fail" + sleep.assert_called_once_with(1.0) + sleep.reset_mock() + + def test_zero_duration_is_allowed(self): + """Zero is a legitimate finite duration and should not be clamped.""" + handler = ActionHandler() + with patch("phone_agent.actions.handler.time.sleep") as sleep: + result = handler.execute(_wait_action(duration="0 seconds"), 1080, 1920) + assert result.success is True + sleep.assert_called_once_with(0.0)