Phase 20.3: route every shared portal client through the JSON decode guard

.planning/REQUIREMENTS.md

@@ -187,6 +187,7 @@ Backfilled 2026-07-25 alongside the phase, which was inserted as urgent work wit
 - [x] **ERR-02**: Reintroducing an uncovered tool fails the DEFAULT unit suite (`tests/test_tool_error_handling.py`), and the detector carries a self-test so it cannot pass vacuously
 - [x] **ERR-03**: A malformed upstream body is classified as an upstream failure, never as caller error — `shared/http.py:api_get` raises `httpx.DecodingError` (an `HTTPError`, not a `ValueError`) so it bypasses `except ValueError -> INVALID_INPUT` arms
 - [x] **ERR-04**: Genuine argument-validation `ValueError`s still return `INVALID_INPUT` — the decode fix does not swallow real caller errors
+- [x] **ERR-05**: Every shared client decodes JSON through `decode_json()`/`decode_json_bytes()`, never raw — ArcGIS Hub, OGC WFS, Socrata and the geojson parsers were missed by ERR-03, which guarded only `api_get` (Phase 20.3)
 
 ### MCP Prompts and Resources
 
@@ -472,6 +473,7 @@ Primary portal is **data.novascotia.ca** — a **Socrata** (Tyler Technologies)
 | ERR-02 | Phase 20.2 | Complete |
 | ERR-03 | Phase 20.2 | Complete |
 | ERR-04 | Phase 20.2 | Complete |
+| ERR-05 | Phase 20.3 | Complete |
 | PR-01 | Phase 40 | Complete |
 | PR-02 | Phase 40 | Complete |
 | PR-03 | Phase 40 | Complete |

.planning/ROADMAP.md

@@ -338,6 +338,17 @@ Plans:
 
 - [x] 20.2-01-PLAN.md — Catch-all coverage for every tool + decode guard in api_get
 
+### Phase 20.3: Route every shared portal client through the JSON decode guard (INSERTED)
+
+**Goal:** Finish what Phase 20.2 started. 20.2 added the decode guard to `shared/http.py:api_get` and stopped there, but three of the four portal technologies decode JSON themselves: ArcGIS Hub and Socrata called `response.json()` raw, OGC WFS called `json.loads(response.content)`, and the ArcGIS geojson hot path decoded via `arcgis_hub._parse_raw_json` + `parsers._parse_geojson`. Because `json.JSONDecodeError` subclasses `ValueError`, a malformed body from any of those still reached the `except ValueError -> INVALID_INPUT` arms in five tools (saskatchewan fire-bans / crop-yields / mineral-mines, manitoba livestock-prices / provincial-waterways) — reproduced before the fix, both probes returned `INVALID_INPUT` for an upstream outage. Adds `decode_json()` / `decode_json_bytes()` to `shared/http.py`, routes all 14 decode sites through them, and adds a structural test so a raw decode cannot be reintroduced.
+**Requirements**: ERR-05
+**Depends on:** Phase 20.2
+**Plans:** 1/1 plans complete
+
+Plans:
+
+- [x] 20.3-01 — decode_json helpers + route all 14 sites + structural guard
+
 ### Phase 21: New Brunswick Government Open Data
 
 **Goal:** [To be planned]

.planning/STATE.md

@@ -6,15 +6,15 @@ current_phase: 20.2
 current_phase_name: Normalize tool error handling and guard malformed upstream JSON
 status: awaiting_merge
 stopped_at: "Phase 20.2 merged to main (PR #3); next is Phase 21 (New Brunswick)"
-last_updated: "2026-07-26T04:19:16.802Z"
+last_updated: "2026-07-27T01:28:17.759Z"
 last_activity: 2026-07-26
 last_activity_desc: Phase 20.2 executed and verified; 317 live tests green
 progress:
-  total_phases: 36
+  total_phases: 37
   completed_phases: 16
-  total_plans: 77
+  total_plans: 78
   completed_plans: 77
-  percent: 44
+  percent: 43
 ---
 
 # Project State
@@ -413,6 +413,7 @@ Recent decisions affecting current work:
 - Phase 40 added: MCP Prompts and Resources — workflow prompts for guided data exploration, static resources for reference data across all modules
 - Phase 20.1 inserted after Phase 20: Remove UPSTREAM_ERROR escape-hatch pattern from all provincial integration tests (MB/SK/AB/QC/NS) and re-run live integration to surface masked upstream failures before pushing Phase 20 (URGENT)
 - Phase 20.2 inserted after Phase 20.1: Normalize tool error handling and guard malformed upstream JSON — root cause of a masking class surfaced by Codex review on PR #2; sequenced before Phase 21 so ~19 future modules inherit the correct handler shape
+- Phase 20.3 inserted after Phase 20.2: Route every shared portal client through the JSON decode guard — 20.2 fixed api_get only, leaving ArcGIS Hub, OGC WFS and Socrata exposed; 5 tools reproducibly returned INVALID_INPUT for an upstream outage
 
 ### Pending Todos
 

.planning/phases/20.3-route-every-shared-portal-client-through-the-json-decode-gua/.gitkeep

@@ -0,0 +1 @@
+

CLAUDE.md

@@ -98,7 +98,7 @@ Every module in `src/mcp_canada/modules/{name}/`:
 
 **Every `@tool` must have catch-all error coverage** — enforced by `tests/test_tool_error_handling.py` in the default unit suite. Satisfy it with `@upstream_guard(<api_name>)` beneath `@tool` (preferred — it is additive, so any handlers inside the function still run first), a broad `except Exception`/`httpx.HTTPError`, or delegation to a module helper that has one. **Catching only `httpx.HTTPStatusError` is not enough:** it covers a 500 but not a timeout, a connect error or a malformed body, each of which escapes as a raw `ToolError`. Phase 20.2 found 108 of 271 tools in that state.
 
-**Never let `json.JSONDecodeError` reach an `except ValueError` arm.** It subclasses `ValueError`, so an upstream answering HTTP 200 with an HTML error page gets reported as `INVALID_INPUT` — blaming the caller for someone else's outage, and failing live tests with a misleading code (`assert_live_or_transient` tolerates only `UPSTREAM_ERROR`/`RATE_LIMITED`/`UPSTREAM_UNAVAILABLE`). `shared/http.py:api_get` converts it to `httpx.DecodingError`, which is an `HTTPError` but not a `ValueError`. Any client calling `.json()` directly must do the same.
+**Never decode JSON outside `decode_json()` / `decode_json_bytes()`** (both in `shared/http.py`) — enforced by `tests/test_upstream_error_classification.py`. `json.JSONDecodeError` subclasses `ValueError`, so a raw `response.json()` on an upstream HTML error page gets reported as `INVALID_INPUT` — blaming the caller for someone else's outage, and failing live tests with a misleading code (`assert_live_or_transient` tolerates only `UPSTREAM_ERROR`/`RATE_LIMITED`/`UPSTREAM_UNAVAILABLE`). The helpers raise `httpx.DecodingError`, which is an `HTTPError` but not a `ValueError`, so it reaches the catch-all instead. Phase 20.2 guarded `api_get` alone and left ArcGIS Hub, OGC WFS and Socrata exposed; Phase 20.3 routed all 14 decode sites through the helpers. Use `decode_json(response, url)` for a `Response`, `decode_json_bytes(content, url)` when you hold raw bytes.
 
 **Every client function must:** return `(data, was_cached)`, use `cached_fetch()` + `get_limiter()`, flatten responses aggressively.
 

src/mcp_canada/shared/arcgis_hub.py

@@ -18,6 +18,8 @@
 
 import httpx
 
+from mcp_canada.shared.http import decode_json, decode_json_bytes
+
 from mcp_canada.shared.parsers import _parse_geojson
 
 # ---------------------------------------------------------------------------
@@ -77,12 +79,12 @@ async def search_hub_datasets(
     if httpx_client is not None:
         response = await httpx_client.get(url, params=params)
         response.raise_for_status()
-        return response.json()
+        return decode_json(response, url)
 
     async with httpx.AsyncClient(timeout=DEFAULT_TIMEOUT) as client:
         response = await client.get(url, params=params)
         response.raise_for_status()
-        return response.json()
+        return decode_json(response, url)
 
 
 async def query_feature_service(
@@ -194,12 +196,12 @@ async def get_layer_metadata(
     if httpx_client is not None:
         response = await httpx_client.get(url, params=params)
         response.raise_for_status()
-        data = response.json()
+        data = decode_json(response, url)
     else:
         async with httpx.AsyncClient(timeout=DEFAULT_TIMEOUT) as client:
             response = await client.get(url, params=params)
             response.raise_for_status()
-            data = response.json()
+            data = decode_json(response, url)
 
     return {
         "max_record_count": int(data.get("maxRecordCount", DEFAULT_PAGE_SIZE)),
@@ -244,12 +246,12 @@ async def get_count(
     if httpx_client is not None:
         response = await httpx_client.get(url, params=params)
         response.raise_for_status()
-        data = response.json()
+        data = decode_json(response, url)
     else:
         async with httpx.AsyncClient(timeout=DEFAULT_TIMEOUT) as client:
             response = await client.get(url, params=params)
             response.raise_for_status()
-            data = response.json()
+            data = decode_json(response, url)
 
     return int(data.get("count", 0))
 
@@ -289,6 +291,9 @@ def shape_hub_dataset(feature: dict[str, Any]) -> dict[str, Any]:
 
 
 def _parse_raw_json(content: bytes) -> dict[str, Any]:
-    """Parse raw bytes as JSON and return the dict (for checking vendor extensions)."""
-    import json
-    return json.loads(content)
+    """Parse raw bytes as JSON and return the dict (for checking vendor extensions).
+
+    Decodes via the shared helper so a malformed body raises httpx.DecodingError
+    rather than a ValueError subclass — see shared/http.py:decode_json.
+    """
+    return decode_json_bytes(content)

src/mcp_canada/shared/http.py

@@ -13,6 +13,40 @@
 )
 
 
+def decode_json(response: httpx.Response, url: str = "") -> Any:
+    """Parse a JSON body, raising ``httpx.DecodingError`` rather than a ValueError.
+
+    ``json.JSONDecodeError`` subclasses ``ValueError``, so an upstream that
+    answers HTTP 200 with an HTML error page would otherwise be caught by the
+    ``except ValueError -> INVALID_INPUT`` arms in saskatchewan and manitoba and
+    blamed on the caller. ``httpx.DecodingError`` is an ``httpx.HTTPError`` but
+    NOT a ``ValueError``, so it bypasses those arms and reaches the catch-all
+    every tool now has.
+
+    Every shared portal client must decode through this — Phase 20.2 guarded
+    only ``api_get``, which left the ArcGIS Hub, OGC WFS and Socrata paths
+    exposed (Phase 20.3).
+    """
+    try:
+        return response.json()
+    except json.JSONDecodeError as exc:
+        where = f" from {url}" if url else ""
+        raise httpx.DecodingError(
+            f"upstream returned a non-JSON body{where}: {exc}"
+        ) from exc
+
+
+def decode_json_bytes(content: bytes, url: str = "") -> Any:
+    """``decode_json`` for callers holding raw bytes (OGC WFS reads .content)."""
+    try:
+        return json.loads(content)
+    except json.JSONDecodeError as exc:
+        where = f" from {url}" if url else ""
+        raise httpx.DecodingError(
+            f"upstream returned a non-JSON body{where}: {exc}"
+        ) from exc
+
+
 def is_retryable(exc: BaseException) -> bool:
     """Return True if the exception warrants a retry."""
     if isinstance(exc, httpx.HTTPStatusError):
@@ -54,20 +88,6 @@ async def _fetch() -> Any:
         async with httpx.AsyncClient(timeout=timeout) as http:
             response = await http.get(url, params=params, headers=headers)
             response.raise_for_status()
-            try:
-                return response.json()
-            except json.JSONDecodeError as exc:
-                # An upstream that answers 200 with an HTML error page is an
-                # upstream failure, but json.JSONDecodeError subclasses
-                # ValueError and would be caught by the
-                # `except ValueError -> INVALID_INPUT` arms in statcan, ircc,
-                # manitoba, saskatchewan, nova_scotia, british_columbia and
-                # datastore — blaming the caller for someone else's outage.
-                # httpx.DecodingError is an httpx.HTTPError but NOT a
-                # ValueError, so it bypasses those arms and lands in the
-                # catch-all every module now has.
-                raise httpx.DecodingError(
-                    f"upstream returned a non-JSON body from {url}: {exc}"
-                ) from exc
+            return decode_json(response, url)
 
     return await _fetch()

src/mcp_canada/shared/ogc.py

@@ -21,6 +21,8 @@
 
 import httpx
 
+from mcp_canada.shared.http import decode_json_bytes
+
 # Imported at module level; defer inside function only if circular import arises.
 from mcp_canada.shared.parsers import _parse_geojson
 
@@ -129,9 +131,8 @@ async def wfs_get_features(
 
     # Use response.content (bytes) so _parse_geojson handles JSON parsing internally.
     # We also need numberReturned for the has_more flag — parse once via json module.
-    import json as _json  # noqa: PLC0415 — avoids shadowing top-level json if user imports this
 
-    body = _json.loads(response.content)
+    body = decode_json_bytes(response.content)
     features = _parse_geojson(response.content, include_geometry=include_geometry)
     has_more = body.get("numberReturned", 0) >= count
     return features, has_more
@@ -245,9 +246,8 @@ async def wfs_count(
 
     response.raise_for_status()
 
-    import json as _json  # noqa: PLC0415
 
-    body = _json.loads(response.content)
+    body = decode_json_bytes(response.content)
     return int(body.get("totalFeatures", 0))
 
 

src/mcp_canada/shared/parsers.py

@@ -9,7 +9,6 @@
 from __future__ import annotations
 
 import csv
-import json
 import re
 import ssl
 import unicodedata
@@ -19,6 +18,8 @@
 
 import httpx
 
+from mcp_canada.shared.http import decode_json_bytes
+
 from mcp_canada.shared.cache import cached_fetch
 
 # Regex to collapse non-alphanumeric runs to underscores
@@ -443,7 +444,7 @@ def _parse_geojson(
         List of dicts, one per Feature. Each dict contains the Feature's
         properties. If properties is None, returns an empty dict {}.
     """
-    data = json.loads(content)
+    data = decode_json_bytes(content)
     features = data.get("features", [])
     result: list[dict[str, Any]] = []
     for feature in features:
@@ -469,7 +470,7 @@ def _parse_json(content: bytes) -> list[dict[str, Any]]:
     Returns:
         List of dicts.
     """
-    data = json.loads(content)
+    data = decode_json_bytes(content)
     if isinstance(data, list):
         return data
     if isinstance(data, dict) and "features" in data:

src/mcp_canada/shared/socrata.py

@@ -29,6 +29,8 @@
 
 import httpx
 
+from mcp_canada.shared.http import decode_json
+
 # ---------------------------------------------------------------------------
 # Constants
 # ---------------------------------------------------------------------------
@@ -85,12 +87,12 @@ async def search_catalog(
     if httpx_client is not None:
         response = await httpx_client.get(url, params=params, headers=headers)
         response.raise_for_status()
-        return response.json()
+        return decode_json(response, url)
 
     async with httpx.AsyncClient(timeout=DEFAULT_TIMEOUT) as client:
         response = await client.get(url, params=params, headers=headers)
         response.raise_for_status()
-        return response.json()
+        return decode_json(response, url)
 
 
 async def get_dataset_metadata(
@@ -125,12 +127,12 @@ async def get_dataset_metadata(
     if httpx_client is not None:
         response = await httpx_client.get(url, headers=headers)
         response.raise_for_status()
-        data = response.json()
+        data = decode_json(response, url)
     else:
         async with httpx.AsyncClient(timeout=DEFAULT_TIMEOUT) as client:
             response = await client.get(url, headers=headers)
             response.raise_for_status()
-            data = response.json()
+            data = decode_json(response, url)
 
     return _flatten_metadata(data)
 
@@ -195,12 +197,12 @@ async def query_dataset(
     if httpx_client is not None:
         response = await httpx_client.get(url, params=params, headers=headers)
         response.raise_for_status()
-        return response.json()
+        return decode_json(response, url)
 
     async with httpx.AsyncClient(timeout=DEFAULT_TIMEOUT) as client:
         response = await client.get(url, params=params, headers=headers)
         response.raise_for_status()
-        return response.json()
+        return decode_json(response, url)
 
 
 def shape_catalog_result(result: dict[str, Any]) -> dict[str, Any]: