From 275bcfb65866a25334909393ebeaa2ed32827055 Mon Sep 17 00:00:00 2001
From: Xing Ji <41811005+micsama@users.noreply.github.com>
Date: Sat, 14 Mar 2026 00:40:03 +0800
Subject: [PATCH] fix(time): treat empty timezone string as absent (#1127)

LLMs sometimes pass "" for optional parameters instead of omitting
them. Previously, passing timezone: "" or from_timezone: "" to the
time tool would trigger a parse error ("Unknown timezone ''") rather
than falling back to the context timezone or UTC.

Fix by adding .filter(|s| !s.is_empty()) after .as_str() in
resolve_timezone_for_output and optional_timezone, so empty strings
are treated the same as a missing field.

The same pattern exists in routine.rs (cron trigger timezone and
schedule fields), where "" produces "invalid IANA timezone: ''" or a
cron parse error. That will be addressed separately once routine.rs
has a test harness in place.

Regression tests added for the now and convert operations with
empty timezone strings.

Closes #1127
---
 src/tools/builtin/time.rs | 56 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)

diff --git a/src/tools/builtin/time.rs b/src/tools/builtin/time.rs
index bafbd4d7..5f037964 100644
--- a/src/tools/builtin/time.rs
+++ b/src/tools/builtin/time.rs
@@ -247,7 +247,11 @@ fn resolve_timezone_for_output(
     params: &serde_json::Value,
     ctx: &JobContext,
 ) -> Result<Option<(Tz, String)>, ToolError> {
-    if let Some(name) = params.get("timezone").and_then(|v| v.as_str()) {
+    if let Some(name) = params
+        .get("timezone")
+        .and_then(|v| v.as_str())
+        .filter(|s| !s.is_empty())
+    {
         let tz = parse_timezone(name)?;
         return Ok(Some((tz, tz.to_string())));
     }
@@ -286,7 +290,11 @@ fn context_timezone(ctx: &JobContext) -> Result<Option<(Tz, String)>, ToolError>
 
 fn optional_timezone(params: &serde_json::Value, keys: &[&str]) -> Result<Option<Tz>, ToolError> {
     for key in keys {
-        if let Some(value) = params.get(*key).and_then(|v| v.as_str()) {
+        if let Some(value) = params
+            .get(*key)
+            .and_then(|v| v.as_str())
+            .filter(|s| !s.is_empty())
+        {
             return parse_timezone(value).map(Some);
         }
     }
@@ -534,4 +542,48 @@ mod tests {
 
         assert_eq!(dt.to_rfc3339(), "2026-03-08T07:30:00+00:00");
     }
+
+    #[tokio::test]
+    async fn test_now_with_empty_timezone_string_does_not_error() {
+        // LLMs sometimes pass "" for optional fields instead of omitting them.
+        // Empty timezone should be treated as absent and fall back to UTC.
+        let tool = TimeTool;
+        let ctx = JobContext::with_user("test", "chat", "test");
+
+        let output = tool
+            .execute(
+                serde_json::json!({
+                    "operation": "now",
+                    "timezone": ""
+                }),
+                &ctx,
+            )
+            .await
+            .expect("empty timezone string should not error");
+
+        assert!(output.result.get("iso").is_some(), "should have iso");
+    }
+
+    #[tokio::test]
+    async fn test_convert_with_empty_from_timezone_string_does_not_error() {
+        // LLMs sometimes pass "" for optional fields instead of omitting them.
+        // Empty from_timezone should be treated as absent.
+        let tool = TimeTool;
+        let ctx = JobContext::with_user("test", "chat", "test");
+
+        let output = tool
+            .execute(
+                serde_json::json!({
+                    "operation": "convert",
+                    "timestamp": "2026-03-08T12:00:00Z",
+                    "to_timezone": "America/New_York",
+                    "from_timezone": ""
+                }),
+                &ctx,
+            )
+            .await
+            .expect("empty from_timezone string should not error");
+
+        assert!(output.result.get("output").is_some(), "should have output");
+    }
 }