docs(20.2): record the catch-all rule and the JSONDecodeError trap

Both are non-obvious enough to reintroduce. Catching only HTTPStatusError
looks like error handling but covers a 500 and nothing else; JSONDecodeError
subclassing ValueError is easy to miss and silently blames the caller.

Adds the phase plan documenting why coverage had to land before the decode
guard, and why the first offender count of 136 was wrong.

Author: mariomeyer

.planning/phases/20.2-normalize-tool-error-handling-and-guard-malformed-upstream-j/20.2-01-PLAN.md

@@ -0,0 +1,72 @@
+---
+phase: "20.2"
+plan: "01"
+title: Catch-all coverage for every tool + decode guard in api_get
+status: complete
+---
+
+# Plan 20.2-01: Catch-all coverage for every tool + decode guard in api_get
+
+<objective>
+Make every module classify an upstream failure as an upstream failure. Two
+independent defects produce the same symptom — an outage that reaches the agent
+as caller error, or as no envelope at all.
+</objective>
+
+## Findings that shaped the plan
+
+Measured with an AST sweep over all 271 `@tool` functions, not by grep:
+
+| Handler shape | Tools | Consequence |
+|---|---|---|
+| `except httpx.HTTPStatusError` only | 68 | timeout / connect / decode escape raw |
+| no exception handling at all | 41 | everything escapes raw |
+| `except WfsError` only (british_columbia) | 15 | everything else escapes raw |
+| `ValueError` + `HTTPStatusError` | 12 | mislabels **and** leaks |
+| **total genuinely uncovered** | **108** | across 13 modules |
+
+A first pass counted 136. That was wrong: `york_region`'s 28 tools delegate to a
+`_call_client` helper that already has `except Exception -> UPSTREAM_ERROR`, so
+they were false positives. The detector now resolves module-level helper
+delegation before reporting.
+
+The second defect: `json.JSONDecodeError` subclasses `ValueError`. An upstream
+answering HTTP 200 with an HTML error page raises it out of
+`shared/http.py:api_get`, straight into the `except ValueError -> INVALID_INPUT`
+arms of statcan, ircc, manitoba, saskatchewan, nova_scotia, british_columbia and
+datastore.
+
+**Why the obvious one-line fix was rejected.** Raising `httpx.DecodingError`
+from `api_get` alone would have made things *worse*: it is an `httpx.HTTPError`
+but **not** an `httpx.HTTPStatusError`, and 5 modules caught only the latter — so
+a mislabelled error would have become an unhandled one, the exact failure Phase
+20.1 removed. Coverage had to land first. Order is load-bearing.
+
+## Tasks
+
+- [x] **RED** — `tests/test_tool_error_handling.py`: AST guard asserting every
+      `@tool` is covered by `@upstream_guard`, a broad `except`, or a safe helper.
+      Failed with 108 offenders. Includes a self-test proving the detector rejects
+      an `HTTPStatusError`-only shape, so it cannot pass vacuously.
+- [x] **GREEN** — applied `@upstream_guard(<api_name>)` beneath `@tool` on all 108.
+      `api_name` derived per-tool from each tool's own `make_response`/`make_error`
+      call (107 of 108); `wx_get_lightning` makes no network call and takes the
+      module constant so the rule has zero exceptions.
+- [x] **RED** — `tests/test_http.py`: `api_get` must raise `httpx.DecodingError`,
+      and explicitly *not* a `ValueError`, on a malformed body.
+- [x] **GREEN** — decode guard in `api_get`.
+- [x] **Behavioural cover** — `tests/test_upstream_error_classification.py` pins
+      the user-visible contract: timeout / connect-error / malformed-JSON each
+      return a transient envelope, malformed JSON is never `INVALID_INPUT`,
+      genuine bad input still is, and `lang` survives onto the envelope.
+- [x] **Non-vacuity check** — reverted `ircc/tools.py` + `shared/http.py` and
+      confirmed 7 of the 8 new behavioural tests fail. The one that stays green is
+      `genuine_bad_input_is_still_invalid_input`, which is behaviour to preserve.
+
+## Deliberately not done
+
+- **`upstream_guard` is additive, not a rewrite.** It wraps the function, so each
+  tool's existing handlers still run first and the guard only catches what
+  escapes. No working error path was removed.
+- **The 41 unhandled tools were not given bespoke handlers.** Uniform decoration
+  is verifiable by the AST guard; 41 hand-written blocks would not be.

CLAUDE.md

@@ -96,6 +96,10 @@ Every module in `src/mcp_canada/modules/{name}/`:
 
 **Every `@tool` must:** use standalone `@tool` from `fastmcp.tools`, include `lang: Literal["en", "fr"]`, return `make_response()`/`make_error()`, have `Use for:` + `Keywords:` in docstring, use module prefix (`boc_`, `parl_`, etc.).
 
+**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.
+
 **Every client function must:** return `(data, was_cached)`, use `cached_fetch()` + `get_limiter()`, flatten responses aggressively.
 
 **Don't:** add dependencies, modify `server.py` for new modules, put module tests in top-level `tests/`, skip rate limiting, mix refactoring with feature work.