diff --git a/src/time/README.md b/src/time/README.md index b4dfa76613..81c801e113 100644 --- a/src/time/README.md +++ b/src/time/README.md @@ -31,6 +31,7 @@ The server speaks both eras of the Model Context Protocol over stdio, with nothi The server is built on the SDK's high-level `MCPServer`. Each tool declares an `outputSchema` and returns its result twice: as `structuredContent`, and as the same object in indented JSON in a text block, for a client that does not read structured output. - **A tool failure is an `isError` result** whose text the model can read and act on, prefixed `Error executing tool : `. That covers an unknown or non-exact timezone (`Invalid timezone: ...`), an empty timezone (`Missing required argument: `), a time not in `HH:MM` 24-hour form, a wall-clock time skipped by a clock change, and an argument missing or of the wrong type (the SDK's validation message). +- **An ambiguous time is reported, not guessed.** On the day a clock change repeats a wall-clock time (a DST fall-back, such as 01:30 on 2024-11-03 in `America/New_York`), `convert_time` returns its first occurrence in `source` and `target`, and the second in `later_occurrence`: a `note` saying the time occurs twice, with that occurrence's own `source`, `target` and `time_difference`. For any other time `later_occurrence` is `null`. - **An unknown tool name is a JSON-RPC `-32602` error** (`Unknown tool: `), not an `isError` result, as both protocol versions specify. - The server registers no prompts or resources. Their list methods answer with empty lists. @@ -267,7 +268,8 @@ Response (`structuredContent`, and the same JSON as text): "day_of_week": "Tuesday", "is_dst": false }, - "time_difference": "+14.0h" + "time_difference": "+14.0h", + "later_occurrence": null } ``` diff --git a/src/time/src/mcp_server_time/server.py b/src/time/src/mcp_server_time/server.py index e0b15c23c2..3440e27777 100644 --- a/src/time/src/mcp_server_time/server.py +++ b/src/time/src/mcp_server_time/server.py @@ -53,6 +53,26 @@ class TimeConversionResult(BaseModel): source: TimeResult target: TimeResult time_difference: str + later_occurrence: "LaterOccurrence | None" = Field( + default=None, + description=( + "Set only when the source time occurs twice on this date because " + "a clock change repeats it (a DST fall-back): the same conversion " + "at its second occurrence. source and target are the first." + ), + ) + + +class LaterOccurrence(BaseModel): + """The second reading of an ambiguous source time (#5196).""" + + note: str + source: TimeResult + target: TimeResult + time_difference: str + + +TimeConversionResult.model_rebuild() class TimeConversionInput(BaseModel): @@ -146,31 +166,63 @@ def convert_time( f"{source_time.date().isoformat()} (skipped by a clock change)" ) - target_time = source_time.astimezone(target_timezone) - source_offset = source_time.utcoffset() or timedelta() - target_offset = target_time.utcoffset() or timedelta() - hours_difference = (target_offset - source_offset).total_seconds() / 3600 + def convert(at: datetime) -> tuple[TimeResult, TimeResult, str]: + """`at` as a source and target `TimeResult`, and their difference.""" + target_time = at.astimezone(target_timezone) + source_offset = at.utcoffset() or timedelta() + target_offset = target_time.utcoffset() or timedelta() + hours_difference = (target_offset - source_offset).total_seconds() / 3600 + + if hours_difference.is_integer(): + time_diff_str = f"{hours_difference:+.1f}h" + else: + # For fractional hours like Nepal's UTC+5:45 + time_diff_str = f"{hours_difference:+.2f}".rstrip("0").rstrip(".") + "h" + + return ( + TimeResult( + timezone=source_tz, + datetime=at.isoformat(timespec="seconds"), + day_of_week=at.strftime("%A"), + is_dst=bool(at.dst()), + ), + TimeResult( + timezone=target_tz, + datetime=target_time.isoformat(timespec="seconds"), + day_of_week=target_time.strftime("%A"), + is_dst=bool(target_time.dst()), + ), + time_diff_str, + ) - if hours_difference.is_integer(): - time_diff_str = f"{hours_difference:+.1f}h" - else: - # For fractional hours like Nepal's UTC+5:45 - time_diff_str = f"{hours_difference:+.2f}".rstrip("0").rstrip(".") + "h" + source, target, time_difference = convert(source_time) + + # A wall-clock time a clock change repeats (e.g. 01:30 on a DST + # fall-back day) has two instants. The datetime above is the first + # (fold=0); fold=1 is the second, at a different UTC offset. Report + # both rather than presenting the first as the only answer (#5196). + later_occurrence = None + later_time = source_time.replace(fold=1) + if later_time.utcoffset() != source_time.utcoffset(): + later_source, later_target, later_difference = convert(later_time) + later_occurrence = LaterOccurrence( + note=( + f"{time_str} occurs twice in {source_tz} on " + f"{source_time.date().isoformat()} because a clock change " + f"repeats it: first at {source.datetime}, then at " + f"{later_source.datetime}. source and target are the first " + "occurrence; this is the second." + ), + source=later_source, + target=later_target, + time_difference=later_difference, + ) return TimeConversionResult( - source=TimeResult( - timezone=source_tz, - datetime=source_time.isoformat(timespec="seconds"), - day_of_week=source_time.strftime("%A"), - is_dst=bool(source_time.dst()), - ), - target=TimeResult( - timezone=target_tz, - datetime=target_time.isoformat(timespec="seconds"), - day_of_week=target_time.strftime("%A"), - is_dst=bool(target_time.dst()), - ), - time_difference=time_diff_str, + source=source, + target=target, + time_difference=time_difference, + later_occurrence=later_occurrence, ) diff --git a/src/time/tests/test_protocol.py b/src/time/tests/test_protocol.py index ff17d34895..cf9e46d68e 100644 --- a/src/time/tests/test_protocol.py +++ b/src/time/tests/test_protocol.py @@ -255,9 +255,41 @@ def expected_tools(local_tz: str) -> list[dict[str, Any]]: "source": {"$ref": "#/$defs/TimeResult"}, "target": {"$ref": "#/$defs/TimeResult"}, "time_difference": {"title": "Time Difference", "type": "string"}, + "later_occurrence": { + "anyOf": [ + {"$ref": "#/$defs/LaterOccurrence"}, + {"type": "null"}, + ], + "default": None, + "description": ( + "Set only when the source time occurs twice on this " + "date because a clock change repeats it (a DST " + "fall-back): the same conversion at its second " + "occurrence. source and target are the first." + ), + }, }, "required": ["source", "target", "time_difference"], - "$defs": {"TimeResult": TIME_RESULT_SCHEMA}, + "$defs": { + "LaterOccurrence": { + "type": "object", + "title": "LaterOccurrence", + "description": ( + "The second reading of an ambiguous source time (#5196)." + ), + "properties": { + "note": {"title": "Note", "type": "string"}, + "source": {"$ref": "#/$defs/TimeResult"}, + "target": {"$ref": "#/$defs/TimeResult"}, + "time_difference": { + "title": "Time Difference", + "type": "string", + }, + }, + "required": ["note", "source", "target", "time_difference"], + }, + "TimeResult": TIME_RESULT_SCHEMA, + }, }, "annotations": READ_ONLY_ANNOTATIONS, }, @@ -599,6 +631,8 @@ async def test_convert_time_wire_result() -> None: "is_dst": False, }, "time_difference": "+8.0h", + # Only an ambiguous source time sets it (#5196). + "later_occurrence": None, } assert result == { "content": [{"type": "text", "text": json.dumps(expected, indent=2)}], @@ -795,8 +829,10 @@ async def test_convert_time_into_a_nonexistent_local_time() -> None: async def test_convert_time_at_an_ambiguous_local_time() -> None: - # 01:30 on 2024-11-03 happens twice in New York. fold=0 picks the first - # occurrence, still on EDT. + # 01:30 on 2024-11-03 happens twice in New York: first on EDT, then, after + # the clocks go back at 02:00, on EST. source and target are the first + # occurrence; later_occurrence carries the second, with a note saying the + # time is ambiguous rather than presenting one answer as certain (#5196). async with connected() as session: with frozen_at("2024-11-03 12:00:00+00:00"): result = await call_json( @@ -816,6 +852,122 @@ async def test_convert_time_at_an_ambiguous_local_time() -> None: } assert result["target"]["datetime"] == "2024-11-03T05:30:00+00:00" assert result["time_difference"] == "+4.0h" + assert result["later_occurrence"] == { + "note": ( + "01:30 occurs twice in America/New_York on 2024-11-03 because a " + "clock change repeats it: first at 2024-11-03T01:30:00-04:00, then " + "at 2024-11-03T01:30:00-05:00. source and target are the first " + "occurrence; this is the second." + ), + "source": { + "timezone": "America/New_York", + "datetime": "2024-11-03T01:30:00-05:00", + "day_of_week": "Sunday", + "is_dst": False, + }, + "target": { + "timezone": "UTC", + "datetime": "2024-11-03T06:30:00+00:00", + "day_of_week": "Sunday", + "is_dst": False, + }, + "time_difference": "+5.0h", + } + + +async def test_convert_time_ambiguity_is_in_the_structured_result() -> None: + # The structured content carries the same later_occurrence as the text. + async with connected() as session: + with frozen_at("2024-10-27 12:00:00+00:00"): + result = await call( + session, + "convert_time", + { + "source_timezone": "Europe/Warsaw", + "time": "02:30", + "target_timezone": "UTC", + }, + ) + structured = result["structuredContent"] + assert structured == json.loads(result["content"][0]["text"]) + assert structured["source"]["datetime"] == "2024-10-27T02:30:00+02:00" + assert structured["target"]["datetime"] == "2024-10-27T00:30:00+00:00" + later = structured["later_occurrence"] + assert later["source"]["datetime"] == "2024-10-27T02:30:00+01:00" + assert later["target"]["datetime"] == "2024-10-27T01:30:00+00:00" + assert later["time_difference"] == "-1.0h" + + +@pytest.mark.parametrize( + "instant,source,time,target,later_target,later_difference", + [ + # Lord Howe Island moves its clocks back by only 30 minutes (02:00 -> + # 01:30), so 01:45 repeats with a fractional difference. + ( + "2024-04-07 00:00:00+00:00", + "Australia/Lord_Howe", + "01:45", + "UTC", + "2024-04-06T15:15:00+00:00", + "-10.5h", + ), + # The target's own zone does not matter: only the source is ambiguous. + ( + "2024-11-03 12:00:00+00:00", + "America/New_York", + "01:00", + "Asia/Kolkata", + "2024-11-03T11:30:00+05:30", + "+10.5h", + ), + ], +) +async def test_convert_time_later_occurrence_in_other_zones( + instant: str, + source: str, + time: str, + target: str, + later_target: str, + later_difference: str, +) -> None: + async with connected() as session: + with frozen_at(instant): + result = await call_json( + session, + "convert_time", + {"source_timezone": source, "time": time, "target_timezone": target}, + ) + later = result["later_occurrence"] + assert later["note"].startswith(f"{time} occurs twice in {source} on ") + assert later["source"]["timezone"] == source + assert later["target"]["datetime"] == later_target + assert later["time_difference"] == later_difference + + +@pytest.mark.parametrize( + "instant,source,time", + [ + # The hour after the repeated one, on the fall-back day itself. + ("2024-11-03 12:00:00+00:00", "America/New_York", "02:00"), + # The hour before it, also on the fall-back day. + ("2024-11-03 12:00:00+00:00", "America/New_York", "00:59"), + # The same wall time on an ordinary day. + ("2024-11-10 12:00:00+00:00", "America/New_York", "01:30"), + # The same date in a zone with no clock change. + ("2024-11-03 12:00:00+00:00", "UTC", "01:30"), + ], +) +async def test_convert_time_unambiguous_time_has_no_later_occurrence( + instant: str, source: str, time: str +) -> None: + async with connected() as session: + with frozen_at(instant): + result = await call_json( + session, + "convert_time", + {"source_timezone": source, "time": time, "target_timezone": "UTC"}, + ) + assert result["later_occurrence"] is None async def test_convert_time_difference_uses_offsets_at_the_converted_instant() -> None: