Phase 38: Dual-Path Index Rebuild + Stale Detection

custom_components/asp_parking/const.py

@@ -88,6 +88,10 @@
 MAX_CSCL_PAGES = 30
 CSCL_BATCH_SIZE = 10000
 SIGNS_BATCH_SIZE = 50000
+# WR-02 (38-REVIEW.md): SODA ASP-signs pagination DoS guard, mirroring
+# MAX_CSCL_PAGES -- without this, a misbehaving/compromised SODA endpoint
+# that keeps returning exactly SIGNS_BATCH_SIZE records loops forever.
+MAX_SIGNS_PAGES = 30
 
 # Vehicular street filter (CSCL RW_TYPE codes)
 VEHICULAR_RW_TYPES = frozenset({1, 2, 3, 4, 5})

custom_components/asp_parking/coordinator.py

@@ -120,6 +120,7 @@
     _sync_cleanup_stale,
     _sync_download_and_extract,
     _sync_read_build_timestamp,
+    _sync_verify_index,
 )
 
 if TYPE_CHECKING:
@@ -685,7 +686,15 @@ async def async_request_rebuild(
         # write cannot bypass the IDX-02 concurrent-press guard (CR-01).
         self._is_rebuilding = True
 
+        # CR-01: snapshot the PREVIOUS press before overwriting so the
+        # 24h double-press check in ``_async_decide_rebuild_path`` compares
+        # "now" against the prior press, not against itself. Threading this
+        # through as an argument (rather than re-reading the mutated
+        # ``self._last_button_press`` instance attribute later) is what
+        # makes the check correct -- see 38-REVIEW.md CR-01.
+        previous_button_press: datetime | None = None
         if triggered_by == "button":
+            previous_button_press = self._last_button_press
             self._last_button_press = dt_util.utcnow()
             if self._index_stale_store is not None:
                 try:
@@ -714,12 +723,19 @@ async def async_request_rebuild(
         # self._async_do_rebuild() since `self` is an ASPParkingCoordinator.
         self._rebuild_task = self.entry.async_create_background_task(
             self.hass,
-            ASPParkingCoordinator._async_do_rebuild(self, triggered_by=triggered_by),
+            ASPParkingCoordinator._async_do_rebuild(
+                self,
+                triggered_by=triggered_by,
+                previous_button_press=previous_button_press,
+            ),
             name="asp_parking_index_rebuild",
         )
 
     async def _async_do_rebuild(
-        self, *, triggered_by: Literal["button", "stale_check"] = "button"
+        self,
+        *,
+        triggered_by: Literal["button", "stale_check"] = "button",
+        previous_button_press: datetime | None = None,
     ) -> None:
         """Background task body — performs the full rebuild lifecycle.
 
@@ -760,7 +776,9 @@ async def _async_do_rebuild(
                 # Phase 38 (IDX-05): decide which executor strategy to run
                 # BEFORE doing any work so the INFO log records intent even
                 # if the chosen path fails.
-                path, reason = await self._async_decide_rebuild_path(triggered_by)
+                path, reason = await self._async_decide_rebuild_path(
+                    triggered_by, previous_button_press
+                )
                 logger.info(
                     "asp_parking: index rebuild path=%s reason=%s",
                     path.value,
@@ -783,6 +801,15 @@ async def _async_do_rebuild(
 
                 await self.hass.async_add_executor_job(_sync_atomic_swap, INDEX_DIR)
 
+                # WR-01: verify the newly-swapped-in index BEFORE trusting it
+                # and posting a success notification. A partial/corrupt write
+                # (disk full mid-write, SD-card corruption, truncated
+                # extraction) must be caught here -- not later as an opaque
+                # IndexIntegrityError/rtree crash from the resolve pipeline.
+                # A raised IndexIntegrityError is caught by the ``except``
+                # block below and reported as a normal rebuild failure.
+                await self.hass.async_add_executor_job(_sync_verify_index, INDEX_DIR)
+
                 # RESEARCH Pitfall 2: reset MUST happen AFTER atomic_swap so the
                 # next SpatialIndex.get() re-opens the new files. reset() just
                 # closes the rtree handle and nulls the singleton — safe to
@@ -818,6 +845,13 @@ async def _async_do_rebuild(
 
             except Exception as err:  # noqa: BLE001
                 pn_dismiss(self.hass, "asp_parking_index_rebuild")
+                # WR-04: dismiss the "index is stale, auto-rebuilding" banner
+                # on failure too -- otherwise it lingers alongside the new
+                # "Rebuild Failed" notification until the next stale-check
+                # cycle happens to re-post it. Idempotent to dismiss a
+                # notification that was never posted (button-triggered
+                # rebuilds never create this one).
+                pn_dismiss(self.hass, "asp_parking_index_stale")
                 if isinstance(err, OSError):
                     if err.strerror and err.filename:
                         _err_summary = f"{err.strerror} ({err.filename})"
@@ -864,7 +898,7 @@ async def _async_do_rebuild(
     # ------------------------------------------------------------------
 
     async def _async_decide_rebuild_path(
-        self, triggered_by: str
+        self, triggered_by: str, previous_button_press: datetime | None = None
     ) -> tuple[RebuildPath, str]:
         """Return ``(path, reason_log_tag)`` for the rebuild router.
 
@@ -876,9 +910,17 @@ async def _async_decide_rebuild_path(
 
         D-03: ``triggered_by="stale_check"`` SKIPS the 24h double-press
         override entirely — that rule is button-only.
+
+        CR-01: ``previous_button_press`` is the value of
+        ``self._last_button_press`` from BEFORE the current press
+        overwrote it (snapshotted by the caller in
+        ``async_request_rebuild``). Re-reading the mutated instance
+        attribute here would always compare "now" against itself, making
+        every button press look like a double-press — see 38-REVIEW.md
+        CR-01.
         """
-        if triggered_by == "button" and self._last_button_press is not None:
-            window = dt_util.utcnow() - self._last_button_press
+        if triggered_by == "button" and previous_button_press is not None:
+            window = dt_util.utcnow() - previous_button_press
             if window < timedelta(hours=BUTTON_DOUBLE_PRESS_WINDOW_HOURS):
                 return RebuildPath.FROM_SOURCE, "double_press"
 
@@ -1098,6 +1140,16 @@ async def _async_check_stale_and_rebuild(self, now: datetime | None = None) -> N
                 "ASP Parking: stale-check/rebuild encountered unexpected error",
                 exc_info=True,
             )
+            # Re-import locally: the lazy import above lives inside the try
+            # block, so `pn_create` is a function-local name that is still
+            # UNBOUND whenever the exception was raised before that import ran
+            # (e.g. a TypeError from the `_last_rebuilt` subtraction).  Without
+            # this, the handler itself dies with UnboundLocalError and the
+            # error notification is never posted.
+            from homeassistant.components.persistent_notification import (
+                async_create as pn_create,
+            )
+
             pn_create(
                 self.hass,
                 "The automatic stale-index check failed unexpectedly. "

custom_components/asp_parking/index_io.py

@@ -53,6 +53,7 @@
     CSCL_BATCH_SIZE,
     CSCL_GEOJSON_URL,
     MAX_CSCL_PAGES,
+    MAX_SIGNS_PAGES,
     SIGNS_BATCH_SIZE,
     SODA_PARKING_SIGNS_URL,
     VEHICULAR_RW_TYPES,
@@ -182,13 +183,33 @@ def _sync_cleanup_stale(index_dir: Path) -> None:
             except OSError as exc:
                 logger.error(
                     "cleanup_stale: could not restore backup index from %s to %s (%s) — "
-                    "destroying backup; the index will need to be rebuilt",
+                    "attempting copy-based fallback recovery",
                     bak,
                     index_dir,
                     exc,
                     exc_info=True,
                 )
-                shutil.rmtree(bak, ignore_errors=True)
+                # WR-05 (38-REVIEW.md): os.replace can fail for reasons that
+                # don't affect a plain copy (e.g. EXDEV when _bak and the
+                # parent dir unexpectedly end up on different filesystems,
+                # or a transient EBUSY). Before permanently discarding the
+                # LAST viable copy of the index, try shutil.copytree as a
+                # fallback — it trades a recoverable state (a valid index,
+                # copied instead of renamed) for an avoidable one (forcing
+                # a full rebuild). _bak is wiped either way once we're done
+                # with it: on copy success it is now redundant; on copy
+                # failure it is unusable and there is nothing left to keep.
+                try:
+                    shutil.copytree(bak, index_dir)
+                except OSError as copy_exc:
+                    logger.error(
+                        "cleanup_stale: copytree fallback also failed (%s) — "
+                        "destroying backup; the index will need to be rebuilt",
+                        copy_exc,
+                        exc_info=True,
+                    )
+                finally:
+                    shutil.rmtree(bak, ignore_errors=True)
 
     try:
         download_zip.unlink(missing_ok=True)
@@ -780,12 +801,29 @@ def _sync_fetch_asp_signs(
     """Fetch ASP sign block-faces. Fail-soft on httpx.HTTPError (T-38-01-02)."""
     asp_tuples: set[tuple[str, str, str, str]] = set()
     offset = 0
+    page_count = 0
 
     try:
         with httpx.Client(
             timeout=300, headers=headers, follow_redirects=True
         ) as client:
             while True:
+                # WR-02 (38-REVIEW.md): mirror the CSCL fetcher's
+                # MAX_CSCL_PAGES DoS guard. Without a cap, a misbehaving or
+                # compromised SODA endpoint that keeps returning exactly
+                # SIGNS_BATCH_SIZE records loops forever in the executor
+                # thread -- unlike CSCL (fail-hard RuntimeError), the
+                # signs fetch is fail-soft, so this breaks with partial
+                # results instead of raising.
+                if page_count >= MAX_SIGNS_PAGES:
+                    logger.warning(
+                        "SODA ASP signs pagination exceeded MAX_SIGNS_PAGES=%d "
+                        "(offset=%d); using partial results",
+                        MAX_SIGNS_PAGES,
+                        offset,
+                    )
+                    break
+
                 params = {
                     "$where": (
                         "sign_description LIKE '%SANITATION BROOM%'"
@@ -799,6 +837,23 @@ def _sync_fetch_asp_signs(
                 resp = client.get(SODA_PARKING_SIGNS_URL, params=params)
                 resp.raise_for_status()
                 records = resp.json()
+                # WR-03 (38-REVIEW.md): mirror the CSCL fetcher's body-shape
+                # guard. SODA can return a truthy non-list JSON body (e.g.
+                # ``{"error": "..."}`` on an HTTP-200 soft error). Without
+                # this guard, `not records` is False for such a body, so
+                # the loop falls into `for record in records:` -- which
+                # iterates a dict's string KEYS, and `record.get(...)` then
+                # raises AttributeError (not in the caught exception tuple
+                # below), turning a fail-soft SODA outage into a hard
+                # failure of the entire from-source rebuild (T-38-01-02).
+                if not isinstance(records, list):
+                    logger.warning(
+                        "SODA ASP signs response was not a list (offset=%d): "
+                        "%r -- treating as no more data",
+                        offset,
+                        type(records).__name__,
+                    )
+                    break
                 if not records:
                     break
                 for record in records:
@@ -810,6 +865,7 @@ def _sync_fetch_asp_signs(
                     side = (record.get("side_of_street") or "").upper().strip()
                     if on_street and side:
                         asp_tuples.add((on_street, from_street, to_street, side))
+                page_count += 1
                 if len(records) < SIGNS_BATCH_SIZE:
                     break
                 offset += SIGNS_BATCH_SIZE