Bounded download retries, MCP 1.x/2.x support, geo acceptance

.github/workflows/ci.yml

@@ -24,7 +24,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        mcp-requirement: ["mcp==1.12.0", "mcp<2"]
+        mcp-requirement: ["mcp==1.12.0", "mcp>=2"]
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-python@v5

.github/workflows/upstream-smoke.yml

@@ -283,3 +283,8 @@ jobs:
         run: scripts/run-agent-evals.sh
         env:
           PITSTOP_EVAL_BIN: pitstop
+
+      - name: Run geo acceptance
+        run: scripts/geo-acceptance.sh
+        env:
+          PITSTOP_GEO_BIN: pitstop

README.md

@@ -76,11 +76,11 @@ pitstop chargers --comune Livo --provincia TN --json
 pitstop chargers --comune-id 021008 --radius 5 --fast --json
 ```
 
-`stations` flags: `--comune`, `--provincia`, `--brand`, `--near "lat,lon"`, `--radius`, `--fuel` (substring, case-insensitive), `--self`, `--served`, `--cheapest` (needs `--fuel`), `--min-price`, `--fresh-within-days`, `--max-deviation-pct`, `--no-comune-validate`, `--limit`, `--json`, `--geojson`. Choose one output format. `--limit 0` returns every match.
+`stations` flags: `--comune`, `--provincia`, `--brand`, `--near "lat,lon"`, `--radius`, `--fuel` (substring, case-insensitive), `--self`, `--served`, `--cheapest` (needs `--fuel`), `--min-price`, `--fresh-within-days`, `--max-deviation-pct`, `--drop-outliers`, `--no-comune-validate`, `--limit`, `--json`, `--geojson`. Choose one output format. `--limit 0` returns every match.
 
 Distances and radii are straight-line measurements, not driving distances or travel times.
 
-Loading flags (`--refresh`, `--max-age` in seconds, `--timeout` in seconds) apply to MIMIT commands and charger searches. The default caches last 24 hours for MIMIT, seven days for OSM, and 30 days for the municipality reference. `--refresh` also refreshes a charger's municipality lookup; `--max-age` controls its charger results. `--max-age 0` accepts cached files of any age. `places` supports `--refresh` and `--timeout`.
+Loading flags (`--refresh`, `--max-age` in seconds, `--timeout` in seconds) apply to MIMIT commands and charger searches. The default caches last 24 hours for MIMIT, seven days for OSM, and 30 days for the municipality reference. `--refresh` also refreshes a charger's municipality lookup; `--max-age` controls its charger results. `--max-age 0` accepts cached files of any age. `places` supports `--refresh` and `--timeout` (no `--max-age`).
 
 Every returned price carries a `median_basis`. A `screened` price also carries `regional_median` and `deviation_pct`, plus `outlier: true` when it is >15% below the local median **or** below the Tukey lower fence Q1−1.5·IQR (the Tukey rule catches misreports in tight markets that the percent rule alone misses). The `outlier` key is emitted **only when it is true**, so read it as optional. Pass `--drop-outliers` to remove flagged prices entirely.
 

pyproject.toml

@@ -38,10 +38,11 @@ Changelog     = "https://github.com/galjos/pitstop-cli/blob/main/CHANGELOG.md"
 Issues        = "https://github.com/galjos/pitstop-cli/issues"
 
 [project.optional-dependencies]
-# Upper bound is load-bearing: mcp 2.0.0 (2026-07-28) removed mcp.server.fastmcp,
-# which mcp_server.py imports. Lift it only together with that migration.
-mcp = ["mcp>=1.12,<2"]
-dev = ["pytest>=7", "mcp>=1.12,<2"]
+# mcp_server.py imports MCPServer/FastMCP and ToolAnnotations through a
+# version shim covering mcp 1.12+ and 2.x; keep the 1.12 floor (read-only
+# annotations, structured output) on both extras.
+mcp = ["mcp>=1.12"]
+dev = ["pytest>=7", "mcp>=1.12"]
 
 [project.scripts]
 pitstop = "pitstop.cli:main"

scripts/geo-acceptance.sh

@@ -0,0 +1,136 @@
+#!/bin/sh
+# Geographic acceptance: municipality resolution and charger locations.
+#
+# Live-only (needs the comuni reference + Overpass). Runs in the weekly
+# upstream-smoke workflow, not in PR CI.
+#
+# Exit 0 -> all location assertions passed, or the upstream was unreachable
+#   and that outage was reported honestly (GitHub ::warning, UPSTREAM-* lines).
+# Exit 1 -> pitstop resolved a place it should have gotten right. Act on it.
+#
+# Distance budgets are generous on purpose: they catch a wrong-city regression
+# (the 20 km Bolzano reference error this suite exists for) without paging on
+# ordinary reference-vs-OSM drift of a few kilometres.
+
+set -u
+
+BIN="${PITSTOP_GEO_BIN:-pitstop}"
+OUT="$(mktemp -d "${TMPDIR:-/tmp}/pitstop-geo-acceptance-XXXXXX")"
+trap 'rm -rf "$OUT"' EXIT INT TERM
+
+warn() {
+  echo "GEO-ACCEPTANCE: UPSTREAM-$1"
+}
+
+check_charger_center() {
+  name="$1"; comune="$2"; lat="$3"; lon="$4"; budget_km="$5"
+  if ! "$BIN" chargers --comune "$comune" --radius 5 --limit 1 --json > "$OUT/center.json" 2>"$OUT/center.err"; then
+    if grep -qi "no unique mapped center\|could not fetch\|failed" "$OUT/center.err"; then
+      warn "DEGRADED charger center $name unverifiable: $(cat "$OUT/center.err")"
+      return 2
+    fi
+    echo "GEO-ACCEPTANCE: FAIL $name: chargers exited non-zero: $(cat "$OUT/center.err")"
+    return 1
+  fi
+  python3 - "$OUT/center.json" "$name" "$lat" "$lon" "$budget_km" <<'PY'
+import json, math, sys
+
+path, name, lat, lon, budget = sys.argv[1], sys.argv[2], float(sys.argv[3]), float(sys.argv[4]), float(sys.argv[5])
+with open(path, encoding="utf-8") as fh:
+    payload = json.load(fh)
+if payload.get("error"):
+    print(f"GEO-ACCEPTANCE: UPSTREAM-DEGRADED charger center {name}: {payload['error']}")
+    sys.exit(2)
+loc = payload.get("location", {})
+try:
+    got_lat, got_lon = float(loc["lat"]), float(loc["lon"])
+except (KeyError, TypeError, ValueError):
+    print(f"GEO-ACCEPTANCE: FAIL {name}: no location in {payload}")
+    sys.exit(1)
+
+r = 6371.0
+p1, p2 = math.radians(lat), math.radians(got_lat)
+h = math.sin(math.radians(got_lat - lat) / 2) ** 2 + math.cos(p1) * math.cos(p2) * math.sin(math.radians(got_lon - lon) / 2) ** 2
+km = 2 * r * math.asin(math.sqrt(h))
+if km > budget:
+    print(f"GEO-ACCEPTANCE: FAIL {name}: center {got_lat},{got_lon} ({km:.1f} km from {lat},{lon}, budget {budget} km)")
+    sys.exit(1)
+print(f"GEO-ACCEPTANCE: ok {name} center -> {got_lat},{got_lon} ({km:.1f} km)")
+PY
+}
+
+failures=0
+unavailable=0
+
+check_charger_center "Bozen" "Bozen" "46.498" "11.354" "25"; rc=$?
+[ "$rc" -eq 1 ] && failures=$((failures + 1))
+[ "$rc" -eq 2 ] && unavailable=$((unavailable + 1))
+
+check_charger_center "Roma" "Roma" "41.903" "12.496" "30"; rc=$?
+[ "$rc" -eq 1 ] && failures=$((failures + 1))
+[ "$rc" -eq 2 ] && unavailable=$((unavailable + 1))
+
+if "$BIN" places Livo --json > "$OUT/livo.json" 2>"$OUT/livo.err"; then
+  python3 - "$OUT/livo.json" <<'PY'
+import json, sys
+
+with open(sys.argv[1], encoding="utf-8") as fh:
+    payload = json.load(fh)
+provs = {p.get("provincia") for p in payload.get("places", [])}
+if not {"CO", "TN"} <= provs:
+    print(f"GEO-ACCEPTANCE: FAIL Livo: CO+TN missing from {sorted(provs)}")
+    sys.exit(1)
+print("GEO-ACCEPTANCE: ok Livo -> CO+TN both listed")
+PY
+  [ "$?" -ne 0 ] && failures=$((failures + 1))
+  # Ambiguous exact names must refuse to guess (exit 2), not pick a province.
+  "$BIN" chargers --comune Livo --json > "$OUT/livo-amb.json" 2>"$OUT/livo-amb.err"
+  rc=$?
+  if [ "$rc" -eq 2 ] && grep -qi "ambiguous\|disambiguat" "$OUT/livo-amb.err"; then
+    echo "GEO-ACCEPTANCE: ok Livo -> ambiguous --comune exits 2"
+  else
+    echo "GEO-ACCEPTANCE: FAIL Livo: ambiguous --comune exited $rc, want 2 with a disambiguation error: $(cat "$OUT/livo-amb.err")"
+    failures=$((failures + 1))
+  fi
+else
+  warn "UNAVAILABLE places Livo exited non-zero: $(cat "$OUT/livo.err")"
+  unavailable=$((unavailable + 1))
+fi
+
+if "$BIN" chargers --near 46.498,11.354 --radius 5 --limit 5 --json > "$OUT/chargers.json" 2>"$OUT/chargers.err"; then
+  python3 - "$OUT/chargers.json" <<'PY'
+import json, math, sys
+
+with open(sys.argv[1], encoding="utf-8") as fh:
+    payload = json.load(fh)
+if payload.get("error"):
+    print(f"GEO-ACCEPTANCE: UPSTREAM-DEGRADED chargers: {payload['error']}")
+    sys.exit(2)
+qlat, qlon, radius = 46.498, 11.354, 5.0
+for st in payload.get("stations", []):
+    try:
+        d = float(st.get("distance_km", "nan"))
+    except (TypeError, ValueError):
+        print(f"GEO-ACCEPTANCE: FAIL charger without distance: {st.get('name')}")
+        sys.exit(1)
+    if not (d <= radius + 1.0):
+        print(f"GEO-ACCEPTANCE: FAIL charger {st.get('name')} at {d} km, outside radius {radius} km + 1 km tolerance")
+        sys.exit(1)
+print(f"GEO-ACCEPTANCE: ok chargers near 46.498,11.354 ({payload.get('count', 0)} returned)")
+PY
+  rc=$?
+  [ "$rc" -eq 1 ] && failures=$((failures + 1))
+  [ "$rc" -eq 2 ] && unavailable=$((unavailable + 1))
+else
+  warn "UNAVAILABLE chargers exited non-zero: $(cat "$OUT/chargers.err")"
+  unavailable=$((unavailable + 1))
+fi
+
+if [ "$failures" -gt 0 ]; then
+  echo "GEO-ACCEPTANCE: $failures failing check(s)"
+  exit 1
+fi
+if [ "$unavailable" -gt 0 ]; then
+  echo "::warning title=Geo acceptance skipped on unreachable upstream::$unavailable check(s) could not reach the comuni reference or Overpass; location assertions that ran passed."
+fi
+echo "GEO-ACCEPTANCE: passed"

scripts/smoke-installed.py

@@ -14,6 +14,22 @@ async def check_mcp(env):
     from mcp import ClientSession, StdioServerParameters
     from mcp.client.stdio import stdio_client
 
+    # CallToolResult.isError on mcp 1.x, is_error on 2.x.
+    def _result_error(result):
+        for name in ("isError", "is_error"):
+            value = getattr(result, name, None)
+            if value is not None:
+                return value
+        raise AssertionError("CallToolResult has neither isError nor is_error")
+
+    # structuredContent on mcp 1.x, structured_content on 2.x.
+    def _structured(result):
+        for name in ("structuredContent", "structured_content"):
+            value = getattr(result, name, None)
+            if value is not None:
+                return value
+        raise AssertionError("CallToolResult has neither structuredContent nor structured_content")
+
     params = StdioServerParameters(command=sys.executable, args=["-m", "pitstop.mcp_server"], env=env)
     async with stdio_client(params) as (read, write):
         async with ClientSession(read, write) as session:
@@ -22,13 +38,20 @@ async def check_mcp(env):
             assert {t.name for t in tools} == {
                 "find_stations", "find_cheapest", "find_chargers", "find_places", "list_fuels", "get_stats"
             }
-            assert all(t.annotations and t.annotations.readOnlyHint and
-                       t.annotations.destructiveHint is False for t in tools)
+            # ToolAnnotations field names are camelCase on mcp 1.x, snake_case on 2.x.
+            def _ann(tool, *names):
+                for name in names:
+                    value = getattr(tool.annotations, name, None)
+                    if value is not None:
+                        return value
+                return None
+            assert all(t.annotations and _ann(t, "readOnlyHint", "read_only_hint") and
+                       _ann(t, "destructiveHint", "destructive_hint") is False for t in tools)
             result = await session.call_tool("find_places", {"query": "Livo"})
-            assert not result.isError and result.structuredContent["matched_count"] == 2
-            assert {p["provincia"] for p in result.structuredContent["places"]} == {"CO", "TN"}
+            assert not _result_error(result) and _structured(result)["matched_count"] == 2
+            assert {p["provincia"] for p in _structured(result)["places"]} == {"CO", "TN"}
             result = await session.call_tool("find_chargers", {"near": "nan,11"})
-            assert result.isError
+            assert _result_error(result)
     print("Installed MCP: six read-only tools, structured discovery, invalid-input error passed")
 
 

skills/pitstop/SKILL.md

@@ -65,10 +65,12 @@ pitstop chargers --comune Bozen --radius 5 --socket ccs --fast --public --json
 
 Key flags:
 - `--comune`: Municipality name. Supports international names (**Rome, Milan, Venice, Florence, Bozen, Mailand, Venise**, etc.).
-- `--fuel`: Substring search. Supports **comma-separated lists** (e.g. `Benzina,Gasolio`).
+- `--fuel`: Substring search. Supports **comma-separated lists** (e.g. `Benzina,Gasolio`). With `--cheapest` and a comma list, each station ranks by its cheapest listed fuel.
 - `--geojson`: Emits a standard GeoJSON FeatureCollection with properties and geometry.
-- Other flags: `--provincia`, `--brand`, `--near`, `--radius`, `--self`/`--served`, `--cheapest`, `--min-price`, `--fresh-within-days`, `--limit`, `--json`.
+- Other station flags: `--provincia`, `--brand`, `--near`, `--radius`, `--self`/`--served`, `--cheapest` (needs `--fuel`), `--min-price`, `--fresh-within-days`, `--max-deviation-pct`, `--drop-outliers`, `--no-comune-validate`, `--limit`, `--json`, `--refresh`, `--max-age`, `--timeout`.
+- Charger flags: `--near`, `--comune`, `--provincia`, `--comune-id` (discover with `places`; use when names duplicate), `--radius`, `--operator`, `--socket`, `--min-power`, `--fast`, `--ultra-fast`, `--free` (only explicit `fee=no`; unknown fee excluded), `--public` (only explicit public access; unknown excluded), `--limit`, `--json`/`--geojson`, `--refresh`, `--timeout`, `--max-age`.
 - Charger power uses `--min-power` in kW; `--fast` means at least 50 kW and `--ultra-fast` at least 150 kW. Inspect the matching socket's power as well as the station maximum.
+- CLI↔MCP names differ: `--min-power` is `min_power_kw`, `--radius` is `radius_km`, `--free`/`--public` are `free_only`/`public_only`, `--fresh-within-days` is `max_age_days`, `--comune-id` is `comune_id`.
 
 ## JSON / GeoJSON contract
 

src/pitstop/cache.py

@@ -1,13 +1,18 @@
-"""Atomic cache writes and source-fetch timestamps."""
+"""Atomic cache writes, source-fetch timestamps, and bounded download retries."""
 
 from __future__ import annotations
 
 import os
 import tempfile
 import time
+import urllib.error
+import urllib.request
 from datetime import datetime, timezone
 from pathlib import Path
 
+RETRY_ATTEMPTS = 3
+RETRY_BASE_DELAY = 1.0  # seconds; doubled between attempts
+
 
 def write_atomic(path: Path, data: bytes) -> None:
     path.parent.mkdir(parents=True, exist_ok=True)
@@ -34,3 +39,25 @@ def fetch_metadata(fetched: float, status: str | None = None) -> dict:
     if status is not None:
         result["cache_status"] = status
     return result
+
+
+def _retryable(error: Exception) -> bool:
+    """Whether a failed download is worth retrying (transient network or server)."""
+    if isinstance(error, urllib.error.HTTPError):
+        return error.code == 429 or 500 <= error.code < 600
+    return isinstance(error, (urllib.error.URLError, OSError))
+
+
+def fetch_bytes(req: urllib.request.Request, timeout: int, attempts: int = RETRY_ATTEMPTS) -> bytes:
+    """Download a URL, retrying transient failures with backoff. Raises the last error."""
+    last: Exception | None = None
+    for attempt in range(max(1, attempts)):
+        try:
+            with urllib.request.urlopen(req, timeout=timeout) as resp:
+                return resp.read()
+        except (urllib.error.URLError, OSError) as e:
+            last = e
+            if not _retryable(e) or attempt + 1 >= max(1, attempts):
+                raise
+            time.sleep(RETRY_BASE_DELAY * (2 ** attempt))
+    raise last  # pragma: no cover - loop always raises first

src/pitstop/core.py

@@ -17,7 +17,7 @@
 from pathlib import Path
 
 from .validation import validate_search, validate_download, validate_nonnegative
-from .cache import fetch_metadata, write_atomic
+from .cache import fetch_bytes, fetch_metadata, write_atomic
 from .results import SearchResults, coverage_of
 
 ANAGRAFICA_URL = "https://www.mimit.gov.it/images/exportCSV/anagrafica_impianti_attivi.csv"
@@ -161,8 +161,7 @@ def _cached_file(
             return path
 
     req = urllib.request.Request(url, headers={"User-Agent": "pitstop"})
-    with urllib.request.urlopen(req, timeout=timeout) as resp:
-        data = resp.read()
+    data = fetch_bytes(req, timeout)
 
     # MIMIT serves its maintenance page with HTTP 200, so a successful request is
     # not evidence we got the CSV. Caching one blip poisons the whole max_age window.

src/pitstop/geocoding.py

@@ -20,7 +20,7 @@
 from dataclasses import dataclass
 from pathlib import Path
 
-from .cache import write_atomic
+from .cache import fetch_bytes, write_atomic
 from .validation import QueryError, parse_near, validate_download
 
 COMUNI_URL = (
@@ -110,8 +110,7 @@ def _cached_path(refresh: bool, max_age: int, timeout: int) -> Path | None:
             return path
     try:
         req = urllib.request.Request(COMUNI_URL, headers={"User-Agent": "pitstop"})
-        with urllib.request.urlopen(req, timeout=timeout) as resp:
-            data = resp.read()
+        data = fetch_bytes(req, timeout)
     except (urllib.error.URLError, OSError) as e:
         # Graceful fallback: if we cannot fetch, return any stale cache or None.
         print(f"pitstop: could not fetch comune coordinates ({e}); "

src/pitstop/mcp_server.py

@@ -5,15 +5,22 @@
 
 from typing import Any, Optional
 
-from mcp.server.fastmcp import FastMCP
-from mcp.types import ToolAnnotations
+try:  # mcp 2.x renamed FastMCP to MCPServer and moved ToolAnnotations out of mcp.types
+    from mcp.server.mcpserver import MCPServer as _Server
+    from mcp_types import ToolAnnotations
+    _ANNOTATIONS_KW = {"read_only_hint": True, "destructive_hint": False,
+                       "idempotent_hint": True, "open_world_hint": True}
+except ImportError:  # mcp 1.x
+    from mcp.server.fastmcp import FastMCP as _Server
+    from mcp.types import ToolAnnotations
+    _ANNOTATIONS_KW = {"readOnlyHint": True, "destructiveHint": False,
+                       "idempotentHint": True, "openWorldHint": True}
 
 from . import chargers as ev_chargers
 from . import core, geocoding, validation
 
-mcp = FastMCP("pitstop")
-_READ_ONLY = ToolAnnotations(readOnlyHint=True, destructiveHint=False,
-                             idempotentHint=True, openWorldHint=True)
+mcp = _Server("pitstop")
+_READ_ONLY = ToolAnnotations(**_ANNOTATIONS_KW)
 
 _CAVEATS = (
     " Data is daily (not real-time): prices are as of ~08:00 the day before "

src/pitstop/overpass.py

@@ -15,7 +15,7 @@
 import urllib.request
 from pathlib import Path
 
-from .cache import file_metadata, write_atomic
+from .cache import fetch_bytes, file_metadata, write_atomic
 from .validation import validate_download
 from .version import __version__
 
@@ -80,8 +80,7 @@ def describe(status: str, from_cache: bool = True) -> None:
     )
     error = None
     try:
-        with urllib.request.urlopen(req, timeout=timeout) as resp:
-            data = resp.read()
+        data = fetch_bytes(req, timeout)
     except (urllib.error.URLError, OSError) as e:
         error = str(e)
         print(f"pitstop: Overpass fetch failed ({error}); "