fix(sync): Handle timezone-naive datetimes in sync.toml (#8477) (#8487)

* fix(sync): Handle timezone-naive datetimes in sync.toml (#8477)

* address comment: add "Z" handler

* Address PR review comments

- Add error handling for invalid datetime strings in _parse_datetime_from_toml
- Add test case for invalid datetime format
- Remove PR/issue references from test class docstrings

* fix(sync): migrate to using epoch time for sync timestamp

* fix make pr

* Update samcli/commands/sync/sync_context.py

Author: vicheey

samcli/commands/sync/sync_context.py

@@ -7,7 +7,7 @@
 from dataclasses import dataclass
 from datetime import datetime, timezone
 from pathlib import Path
-from typing import Dict, Optional, cast
+from typing import Dict, Optional, Union, cast
 
 import tomlkit
 from tomlkit.items import Item
@@ -65,6 +65,45 @@ def update_infra_sync_time(self) -> None:
         self.latest_infra_sync_time = datetime.now(timezone.utc)
 
 
+def _parse_time_from_toml(time_value: Union[str, int, float, datetime]) -> Optional[datetime]:
+    """
+    Parse time from TOML file - supports both epoch and ISO format.
+
+    Handles legacy ISO format strings as sam migrates to using epoch timestamps for backward compatibility.
+    New writes always use epoch format.
+
+    Parameters
+    ----------
+    time_value: Union[str, int, float, datetime]
+        Either epoch timestamp (new format), ISO format string (legacy), or datetime object
+
+    Returns
+    -------
+    Optional[datetime]
+        Timezone-aware datetime in UTC, or None if parsing fails
+    """
+    try:
+        if isinstance(time_value, datetime):
+            return time_value if time_value.tzinfo else time_value.replace(tzinfo=timezone.utc)
+
+        if isinstance(time_value, (int, float)):
+            return datetime.fromtimestamp(time_value, tz=timezone.utc)
+
+        if isinstance(time_value, str):
+            if time_value.endswith("Z"):
+                time_value = time_value[:-1] + "+00:00"
+            parsed = datetime.fromisoformat(time_value)
+            return parsed if parsed.tzinfo else parsed.replace(tzinfo=timezone.utc)
+
+    except (ValueError, OSError) as e:
+        LOG.warning(
+            "Failed to parse timestamp from sync.toml: %s. Triggering full CloudFormation deployment. Error: %s",
+            time_value,
+            e,
+        )
+    return None
+
+
 def _sync_state_to_toml_document(sync_state: SyncState) -> TOMLDocument:
     """
     Writes the sync state information to the TOML file.
@@ -82,7 +121,7 @@ def _sync_state_to_toml_document(sync_state: SyncState) -> TOMLDocument:
     sync_state_toml_table = tomlkit.table()
     sync_state_toml_table[DEPENDENCY_LAYER] = sync_state.dependency_layer
     if sync_state.latest_infra_sync_time:
-        sync_state_toml_table[LATEST_INFRA_SYNC_TIME] = sync_state.latest_infra_sync_time.isoformat()
+        sync_state_toml_table[LATEST_INFRA_SYNC_TIME] = sync_state.latest_infra_sync_time.timestamp()
 
     resource_sync_states_toml_table = tomlkit.table()
     for resource_id in sync_state.resource_sync_states:
@@ -91,7 +130,7 @@ def _sync_state_to_toml_document(sync_state: SyncState) -> TOMLDocument:
         resource_sync_state_toml_table = tomlkit.table()
 
         resource_sync_state_toml_table[HASH] = resource_sync_state.hash_value
-        resource_sync_state_toml_table[SYNC_TIME] = resource_sync_state.sync_time.isoformat()
+        resource_sync_state_toml_table[SYNC_TIME] = resource_sync_state.sync_time.timestamp()
 
         # For Nested stack resources, replace "/" with "-"
         resource_id_toml = resource_id.replace("/", "-")
@@ -129,9 +168,15 @@ def _toml_document_to_sync_state(toml_document: Dict) -> Optional[SyncState]:
     if resource_sync_states_toml_table:
         for resource_id in resource_sync_states_toml_table:
             resource_sync_state_toml_table = resource_sync_states_toml_table.get(resource_id)
+            sync_time_str = resource_sync_state_toml_table.get(SYNC_TIME)
+            # Parse datetime and ensure it's timezone-aware UTC (consistent with how we write)
+            sync_time = _parse_time_from_toml(sync_time_str)
+            if sync_time is None:
+                # Skip this resource if timestamp is invalid - resource will be re-synced on next sam sync
+                continue
             resource_sync_state = ResourceSyncState(
                 resource_sync_state_toml_table.get(HASH),
-                datetime.fromisoformat(resource_sync_state_toml_table.get(SYNC_TIME)),
+                sync_time,
             )
 
             # For Nested stack resources, replace "-" with "/"
@@ -142,9 +187,9 @@ def _toml_document_to_sync_state(toml_document: Dict) -> Optional[SyncState]:
     latest_infra_sync_time = None
     if sync_state_toml_table:
         dependency_layer = sync_state_toml_table.get(DEPENDENCY_LAYER)
-        latest_infra_sync_time = sync_state_toml_table.get(LATEST_INFRA_SYNC_TIME)
-        if latest_infra_sync_time:
-            latest_infra_sync_time = datetime.fromisoformat(str(latest_infra_sync_time))
+        latest_infra_sync_time_str = sync_state_toml_table.get(LATEST_INFRA_SYNC_TIME)
+        if latest_infra_sync_time_str:
+            latest_infra_sync_time = _parse_time_from_toml(latest_infra_sync_time_str)
     sync_state = SyncState(dependency_layer, resource_sync_states, latest_infra_sync_time)
 
     return sync_state

tests/unit/commands/sync/test_sync_context.py

@@ -22,7 +22,7 @@
 )
 from samcli.lib.build.build_graph import DEFAULT_DEPENDENCIES_DIR
 
-MOCK_RESOURCE_SYNC_TIME = datetime(2023, 2, 8, 12, 12, 12)
+MOCK_RESOURCE_SYNC_TIME = datetime(2023, 2, 8, 12, 12, 12, tzinfo=timezone.utc)
 MOCK_INFRA_SYNC_TIME = datetime.now(timezone.utc)
 
 
@@ -141,7 +141,7 @@ def test_sync_state_to_toml(self, dependency_layer, latest_infra_sync_time, reso
         self.assertEqual(dependency_layer_toml_field, dependency_layer)
 
         latest_infra_sync_time_toml_field = sync_state_toml_table.get(LATEST_INFRA_SYNC_TIME)
-        self.assertEqual(latest_infra_sync_time_toml_field, latest_infra_sync_time.isoformat())
+        self.assertEqual(latest_infra_sync_time_toml_field, latest_infra_sync_time.timestamp())
 
         resource_sync_states_toml_field = toml_document.get(RESOURCE_SYNC_STATES)
         self.assertIsNotNone(resource_sync_states_toml_field)
@@ -155,7 +155,7 @@ def test_sync_state_to_toml(self, dependency_layer, latest_infra_sync_time, reso
                 resource_sync_state_toml_table.get(HASH),
             )
             self.assertEqual(
-                resource_sync_states[resource_sync_state_resource_id].sync_time.isoformat(),
+                resource_sync_states[resource_sync_state_resource_id].sync_time.timestamp(),
                 resource_sync_state_toml_table.get(SYNC_TIME),
             )
 
@@ -288,3 +288,203 @@ def test_sync_context_has_no_previous_state_if_file_doesnt_exist(self, patched_r
             self.assertIsNone(self.sync_context._previous_state)
             self.assertIsNotNone(self.sync_context._current_state)
             patched_rmtree_if_exists.assert_not_called()
+
+
+class TestTimestampParsing(TestCase):
+    """Tests for timestamp parsing with various formats and error handling"""
+
+    @parameterized.expand(
+        [
+            # Valid timestamp formats
+            (
+                "timezone_naive",
+                "2025-12-03T22:10:11.916279",
+                {"Resource1": ("hash1", "2025-12-03T22:10:35.345701")},
+                True,
+                ["Resource1"],
+                [],
+            ),
+            (
+                "timezone_aware",
+                "2025-12-03T22:10:11.916279+00:00",
+                {"Resource1": ("hash1", "2025-12-03T22:10:35.345701+00:00")},
+                True,
+                ["Resource1"],
+                [],
+            ),
+            (
+                "z_suffix",
+                "2024-05-08T15:16:43Z",
+                {"Resource1": ("hash1", "2024-05-08T15:16:43Z")},
+                True,
+                ["Resource1"],
+                [],
+            ),
+            ("epoch_format", 1733267411.916279, {"Resource1": ("hash1", 1733267435.345701)}, True, ["Resource1"], []),
+            (
+                "mixed_formats",
+                "2024-05-08T15:16:43Z",
+                {"Resource1": ("hash1", "2025-12-03T22:10:35+00:00"), "Resource2": ("hash2", 1733267440.123456)},
+                True,
+                ["Resource1", "Resource2"],
+                [],
+            ),
+            (
+                "nested_resource",
+                "2024-05-08T15:16:43Z",
+                {"Parent/Child/Resource": ("hash", "2024-05-08T15:16:43Z")},
+                True,
+                ["Parent/Child/Resource"],
+                [],
+            ),
+            # Invalid timestamp formats
+            (
+                "invalid_resource",
+                1733267411.916279,
+                {"Valid": ("hash1", 1733267435.345701), "Invalid": ("hash2", "bad-timestamp")},
+                True,
+                ["Valid"],
+                ["Invalid"],
+            ),
+            ("invalid_infra", "not-timestamp", {"Valid": ("hash1", 1733267435.345701)}, False, ["Valid"], []),
+            (
+                "multiple_invalid",
+                1733267411.916279,
+                {
+                    "Valid1": ("h1", 1733267435.345701),
+                    "Invalid1": ("h2", "bad1"),
+                    "Valid2": ("h3", 1733267440.123456),
+                    "Invalid2": ("h4", "bad2"),
+                },
+                True,
+                ["Valid1", "Valid2"],
+                ["Invalid1", "Invalid2"],
+            ),
+            ("all_invalid", "bad-infra", {"Invalid": ("hash", "bad-resource")}, False, [], ["Invalid"]),
+        ]
+    )
+    def test_timestamp_parsing(
+        self, test_name, infra_sync_time, resources, has_valid_infra, expected_resources, missing_resources
+    ):
+        """Test timestamp parsing for various formats and error handling"""
+        # Build TOML string
+        if isinstance(infra_sync_time, str):
+            infra_value = f'"{infra_sync_time}"'
+        else:
+            infra_value = str(infra_sync_time)
+
+        toml_str = f"""
+[sync_state]
+dependency_layer = true
+latest_infra_sync_time = {infra_value}
+
+[resource_sync_states]
+"""
+        for resource_id, (resource_hash, resource_sync_time) in resources.items():
+            if isinstance(resource_sync_time, str):
+                sync_time_value = f'"{resource_sync_time}"'
+            else:
+                sync_time_value = str(resource_sync_time)
+
+            resource_id_toml = resource_id.replace("/", "-")
+            toml_str += f"""
+[resource_sync_states.{resource_id_toml}]
+hash = "{resource_hash}"
+sync_time = {sync_time_value}
+"""
+
+        toml_doc = tomlkit.loads(toml_str)
+
+        # Mock logger if we expect invalid timestamps
+        if missing_resources or not has_valid_infra:
+            with patch("samcli.commands.sync.sync_context.LOG") as mock_log:
+                sync_state = _toml_document_to_sync_state(toml_doc)
+                mock_log.warning.assert_called()
+        else:
+            sync_state = _toml_document_to_sync_state(toml_doc)
+
+        # Verify infra sync time
+        if has_valid_infra:
+            self.assertIsNotNone(sync_state.latest_infra_sync_time)
+            self.assertEqual(sync_state.latest_infra_sync_time.tzinfo, timezone.utc)
+            # Verify datetime comparison works
+            time_diff = datetime.now(timezone.utc) - sync_state.latest_infra_sync_time
+            self.assertIsNotNone(time_diff)
+        else:
+            self.assertIsNone(sync_state.latest_infra_sync_time)
+
+        # Verify expected resources were loaded with correct timezone
+        for resource_id in expected_resources:
+            self.assertIn(resource_id, sync_state.resource_sync_states)
+            self.assertEqual(sync_state.resource_sync_states[resource_id].sync_time.tzinfo, timezone.utc)
+
+        # Verify missing resources were skipped
+        for resource_id in missing_resources:
+            self.assertNotIn(resource_id, sync_state.resource_sync_states)
+
+
+class TestEpochTimestampHandling(TestCase):
+    """Tests for epoch timestamp format (new format with backward compatibility)"""
+
+    def test_epoch_format_read(self):
+        """Test reading epoch timestamps (new format)"""
+        toml_str = """
+[sync_state]
+dependency_layer = true
+latest_infra_sync_time = 1733267411.916279
+
+[resource_sync_states]
+
+[resource_sync_states.MockResourceId]
+hash = "mock-hash"
+sync_time = 1733267435.345701
+"""
+        toml_doc = tomlkit.loads(toml_str)
+        sync_state = _toml_document_to_sync_state(toml_doc)
+
+        # Verify both are timezone-aware UTC
+        self.assertEqual(sync_state.latest_infra_sync_time.tzinfo, timezone.utc)
+        self.assertEqual(sync_state.resource_sync_states["MockResourceId"].sync_time.tzinfo, timezone.utc)
+
+    def test_epoch_format_write(self):
+        """Test that writing always produces epoch, not ISO strings"""
+        sync_state = SyncState(
+            dependency_layer=True,
+            resource_sync_states={"TestResource": ResourceSyncState("hash123", datetime.now(timezone.utc))},
+            latest_infra_sync_time=datetime.now(timezone.utc),
+        )
+
+        toml_doc = _sync_state_to_toml_document(sync_state)
+
+        # Verify written values are numeric (epoch), not strings (ISO)
+        self.assertIsInstance(toml_doc["sync_state"]["latest_infra_sync_time"], (int, float))
+        self.assertIsInstance(toml_doc["resource_sync_states"]["TestResource"]["sync_time"], (int, float))
+
+    def test_backward_compatibility_mixed_formats(self):
+        """Test that old ISO and new epoch can coexist during migration"""
+        toml_str = """
+[sync_state]
+dependency_layer = true
+latest_infra_sync_time = 1733267411.916279
+
+[resource_sync_states]
+
+[resource_sync_states.OldResource]
+hash = "old-hash"
+sync_time = "2025-12-03T22:10:35.345701"
+
+[resource_sync_states.NewResource]
+hash = "new-hash"
+sync_time = 1733267435.345701
+"""
+        toml_doc = tomlkit.loads(toml_str)
+        sync_state = _toml_document_to_sync_state(toml_doc)
+
+        # Both should work and be timezone-aware
+        self.assertEqual(sync_state.resource_sync_states["OldResource"].sync_time.tzinfo, timezone.utc)
+        self.assertEqual(sync_state.resource_sync_states["NewResource"].sync_time.tzinfo, timezone.utc)
+
+        # Verify datetime comparison works
+        current_time = datetime.now(timezone.utc)
+        time_diff = current_time - sync_state.latest_infra_sync_time
+        self.assertIsNotNone(time_diff)