Skip to content

Commit 06a5ab9

Browse files
kingpanther13claude
andcommitted
fix: cover max-limit hint edge and invalid end_time validation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AgVydTVw2uzQqdnxZ7jfJv
1 parent 83e702c commit 06a5ab9

4 files changed

Lines changed: 62 additions & 6 deletions

File tree

src/ha_mcp/tools/error_log_parsing.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -551,8 +551,19 @@ def _attach_error_log_pagination(
551551
if unreturned_matches > 0:
552552
total = data.get("total_lines", unreturned_matches)
553553
suggested = min(total, MAX_LIMIT)
554-
data["pagination_hint"] = (
555-
f"{unreturned_matches} more matching lines remain inside this "
556-
f"window (no older history exists behind it). Repeat the call "
557-
f"with limit={suggested} to retrieve them in one response."
558-
)
554+
if limit is not None and limit >= suggested:
555+
# Raising the limit cannot help: it is already at (or past) the
556+
# ceiling, so recommending it would repeat the same request.
557+
data["pagination_hint"] = (
558+
f"{unreturned_matches} more matching lines remain inside this "
559+
f"window (no older history exists behind it), but 'limit' is "
560+
f"already at its maximum ({MAX_LIMIT}). Narrow the match set — "
561+
"a more specific 'search' or a 'level' filter — so the "
562+
"remainder fits in one response."
563+
)
564+
else:
565+
data["pagination_hint"] = (
566+
f"{unreturned_matches} more matching lines remain inside this "
567+
f"window (no older history exists behind it). Repeat the call "
568+
f"with limit={suggested} to retrieve them in one response."
569+
)

src/ha_mcp/tools/log_sources.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,20 @@ async def _get_logbook(
195195
)
196196

197197
if end_time:
198-
end_dt = datetime.fromisoformat(end_time.replace("Z", "+00:00"))
198+
try:
199+
end_dt = datetime.fromisoformat(end_time.replace("Z", "+00:00"))
200+
except ValueError:
201+
# Outside the fetch try-block below, so without this guard the
202+
# raw ValueError would bypass the structured ToolError shape.
203+
raise_tool_error(
204+
create_error_response(
205+
ErrorCode.VALIDATION_INVALID_PARAMETER,
206+
f"Invalid end_time '{end_time}': not an ISO 8601 timestamp",
207+
suggestions=[
208+
"Use ISO format, e.g. end_time='2026-08-28T01:00:00Z'"
209+
],
210+
)
211+
)
199212
else:
200213
end_dt = datetime.now(UTC)
201214

tests/src/unit/test_tools_utility_error_log_structured.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,19 @@ async def test_terminal_window_with_unreturned_matches_hints_a_larger_limit(self
11221122
assert "40 more matching lines remain" in result["pagination_hint"]
11231123
assert "limit=50" in result["pagination_hint"]
11241124

1125+
@pytest.mark.asyncio
1126+
async def test_terminal_window_at_max_limit_hints_narrowing_instead(self):
1127+
"""At limit=MAX_LIMIT the raise-the-limit hint would repeat the same
1128+
request verbatim; the hint switches to narrowing the match set."""
1129+
client = _make_client(_numbered_log(MAX_LIMIT + 100), has_more=False)
1130+
tools = _register_and_collect(client)
1131+
result = await tools["ha_get_logs"](
1132+
source="error_log", limit=MAX_LIMIT, search="issue"
1133+
)
1134+
assert result["has_more"] is False
1135+
assert "already at its maximum" in result["pagination_hint"]
1136+
assert f"limit={MAX_LIMIT} to retrieve" not in result["pagination_hint"]
1137+
11251138
@pytest.mark.asyncio
11261139
async def test_next_offset_advances_from_the_current_offset(self):
11271140
client = _make_client(_numbered_log(20), has_more=True)

tests/src/unit/test_tools_utility_log_order.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
newest-first fix (#1178).
77
"""
88

9+
import json
910
from unittest.mock import AsyncMock, patch
1011

1112
import pytest
13+
from fastmcp.exceptions import ToolError
1214

1315
from ha_mcp.client.rest_client import ErrorLogPage, HomeAssistantConnectionError
1416
from ha_mcp.tools.tools_logs import LogTools
@@ -261,6 +263,23 @@ async def test_tolerates_missing_none_and_non_dict_entries(self):
261263
assert result["entries"][0]["timestamp"] == 100.0
262264

263265

266+
class TestLogbookEndTimeValidation:
267+
"""source='logbook' — a bad end_time must fail with the structured shape."""
268+
269+
@pytest.mark.asyncio
270+
async def test_invalid_end_time_raises_structured_validation_error(self):
271+
"""`datetime.fromisoformat` runs before the fetch try-block; without
272+
the guard a raw ValueError escapes the ToolError contract."""
273+
client = AsyncMock()
274+
tools = LogTools(client)
275+
with pytest.raises(ToolError) as exc_info:
276+
await tools.get_logs(**_call_kwargs(source="logbook", end_time="invalid"))
277+
payload = json.loads(str(exc_info.value))
278+
assert payload["error"]["code"] == "VALIDATION_INVALID_PARAMETER"
279+
assert "end_time" in payload["error"]["message"]
280+
client.get_logbook.assert_not_called()
281+
282+
264283
class TestSystemMalformedEntries:
265284
"""source='system' — malformed records must not escape the error envelope."""
266285

0 commit comments

Comments
 (0)