From 1671653e2bf8bd47ff18cd71b88cf39b34f3dfa8 Mon Sep 17 00:00:00 2001 From: steve Date: Thu, 30 Jul 2026 22:39:10 +0000 Subject: [PATCH 1/3] feat: add selectable URL shortener (da.gd, self-hosted Sink, or none) for !path and !topo map links --- Bot_Usage.md | 57 +++++++++++- commands/path.py | 30 ++----- commands/topo.py | 25 ++---- config.py | 35 +++++++- mcbot.conf.example | 18 ++++ shortener.py | 110 +++++++++++++++++++++++ tests/test_shortener.py | 194 ++++++++++++++++++++++++++++++++++++++++ tests/test_topo.py | 7 +- 8 files changed, 429 insertions(+), 47 deletions(-) create mode 100644 shortener.py create mode 100644 tests/test_shortener.py diff --git a/Bot_Usage.md b/Bot_Usage.md index 5c1b761..34d77e8 100644 --- a/Bot_Usage.md +++ b/Bot_Usage.md @@ -316,6 +316,9 @@ CLI: `--logs-dir`, `--log-level`, `--debug` (sets meshcore lib to DEBUG) | `dm_max_attempts` | 3 | `send_msg_with_retry` max attempts | | `dm_flood_after` | 2 | Switch to flood routing after N direct attempts | | `dm_max_flood_attempts` | 2 | Max flood-mode retries | +| `url_shortener` | `dagd` | `dagd`, `sink`, or `none` -- see [URL shortening](#url-shortening) | +| `sink_api_base` | (empty) | Sink instance base URL, e.g. `https://sht.nz`. Required when `url_shortener = sink` | +| `sink_link_ttl_days` | 30 | Days before a bot-created Sink link expires; `0` keeps them forever | CLI: `--commands-dir`, `--privkey-path`, `--disable-commands`, `--dm-max-attempts`, `--dm-flood-after`, `--dm-max-flood-attempts`, @@ -337,6 +340,47 @@ CLI: `--commands-dir`, `--privkey-path`, `--disable-commands`, To re-seed from conf: `sqlite3 mcbot.db "DELETE FROM channels;"` then restart. +### URL shortening + +`!path` and `!topo` reply with map links. A mesh message has ~180 characters +to spend and a geojson.io map payload alone runs to several KB, so the link +has to be shortened by a service. `[bot] url_shortener` picks which: + +| Value | Behaviour | +|-------|-----------| +| `dagd` | Public [da.gd](https://da.gd) service. No account, no configuration. Default. | +| `sink` | Your own [Sink](https://github.com/miantiao-me/Sink) instance. Needs `sink_api_base` and a `SINK_API_TOKEN`. | +| `none` | No shortening. `!topo` sends the raw OpenTopoMap URL; `!path` replies `(map err)`. | + +For `sink`, put the token in `[env]` rather than a command script -- the conf +file is gitignored, `commands/*.py` are tracked: + +```ini +[bot] +url_shortener = sink +sink_api_base = https://sht.nz +sink_link_ttl_days = 30 + +[env] +SINK_API_TOKEN = +``` + +The token is Sink's `NUXT_SITE_TOKEN` (its API bearer credential), *not* the +`NUXT_CF_API_TOKEN` analytics token. Notes on the `sink` provider: + +- Links are created with `POST /api/link/upsert` and a slug derived from a + hash of the target URL, so re-requesting the same route re-uses the + existing short link instead of adding a row per invocation. +- **It falls back to da.gd** when Sink is unreachable, misconfigured, or + rejects the URL. Sink caps target URLs (2048 characters by default), and a + route past ~8 hops exceeds that -- the fallback is what keeps long routes + from silently losing their map. Raise `UrlSchema` in your Sink instance's + `shared/schemas/link.ts` if you'd rather it accept them directly. +- Clicks on bot-generated links land in your Sink analytics. + +Changing any of these needs a bot restart -- `!adm reload` re-executes +command plugins but not the shared `shortener` module. + --- ## 4. Running the Bot @@ -934,8 +978,9 @@ in one reply. Arguments: (gaps are bridged, so route ≥ direct). **direct** = great-circle distance between the first and last located hop. 0.1 resolution. `mi` = miles, `km` = km. - The map is a [geojson.io](https://geojson.io) link drawing a LineString - through the located hops plus a labeled marker per hop, shortened via da.gd - so the reply fits a mesh message. Open it in any browser with internet. + through the located hops plus a labeled marker per hop, shortened so the + reply fits a mesh message (see [URL shortening](#url-shortening)). Open it + in any browser with internet. - If some hops can't be located, a `(located/total)` count is shown and the distances/map use only the located hops: ``` @@ -960,12 +1005,16 @@ in one reply. Arguments: own location on the radio helps but isn't required. - 10s per-user cooldown; no auth; works in DM and any allowed channel. -- Makes a da.gd shortener call per invocation (when ≥2 hops are located). +- Makes one shortener call per invocation (when ≥1 hop is located). The + geojson payload is several KB, so with shortening off or unreachable the + reply ends in `(map err)` -- there is no raw URL short enough to send. ### `!topo` / `!topo ` / `!topo help` Plots a contact's advertised location on [OpenTopoMap](https://opentopomap.org) -and replies with a da.gd-shortened marker link (zoom 16). +and replies with a shortened marker link (zoom 16). Unlike `!path`, the raw +OpenTopoMap URL is short enough to send, so it is used as-is if shortening +is off or unreachable. ``` !topo a1b2c3 → @[Alice] HillTop https://da.gd/abcd diff --git a/commands/path.py b/commands/path.py index 75ba3a8..ad162f1 100644 --- a/commands/path.py +++ b/commands/path.py @@ -9,7 +9,8 @@ # - h hop count; are per-hop pubkey-prefix hashes. # - route great-circle distance summed between consecutive located hops. # - direct great-circle distance between the first and last located hop. -# - da.gd shortened geojson.io map drawing the route. +# - shortened geojson.io map drawing the route. which shortener +# is used is set by [bot] url_shortener in mcbot.conf. # - distances are miles by default. pass 'k' for kilometers: !path k # - if some hops can't be located, a (located/total) count is shown and the # distances/map use only the located hops. @@ -24,7 +25,7 @@ import json import math -import requests +from shortener import shorten NAME = "path" TRIGGERS = ["!path"] @@ -201,19 +202,6 @@ def _geojson_io_url(located_named): return "https://geojson.io/#data=data:application/json;base64," + b64 -def _shorten_sync(long_url): - # shorten via da.gd - # - r = requests.get( - "https://da.gd/s", params={"url": long_url}, timeout=8, - ) - r.raise_for_status() - s = r.text.strip() - if not s.startswith("http"): - raise ValueError(f"shortener error: {s[:60]}") - return s - - def _parse_path_arg(path_arg): # parse provided path string argument into path_hex, path_len, # hash_mode. returns an error string on bad input. @@ -294,12 +282,12 @@ async def handle(ctx): if n_located == 0: return f"@[{name}] [{nh}h] {path_str} dist/map unavailable (0/{nh})" - # map link for any 1 or more located hops - try: - short = await asyncio.to_thread(_shorten_sync, _geojson_io_url(loc)) - except Exception: - ctx.bot.logger.exception("path map: shorten failed") - short = None + # map link for any 1 or more located hops. the geojson payload is far too + # long to send raw, so without a shortener there is no map to offer. + short = await asyncio.to_thread( + shorten, _geojson_io_url(loc), ctx.bot.cfg, + ctx.bot.logger, "path", + ) map_part = f", {short}" if short else " (map err)" # one located hop can't compute a distance, but still map the pin. diff --git a/commands/topo.py b/commands/topo.py index 1516654..ba63fc8 100644 --- a/commands/topo.py +++ b/commands/topo.py @@ -1,4 +1,4 @@ -# !topo — plot a contact's location on OpenTopoMap (with a da.gd short link) +# !topo — plot a contact's location on OpenTopoMap (with a short link) # # usage: # !topo plot that contact's location (prefix 4+ hex chars) @@ -12,7 +12,7 @@ import asyncio -import requests +from shortener import shorten NAME = "topo" TRIGGERS = ["!topo"] @@ -26,16 +26,6 @@ _USAGE = "!topo [pub-key prefix] (4+ chars)" -def _shorten_sync(long_url): - # shorten via da.gd (same service !path uses) - r = requests.get("https://da.gd/s", params={"url": long_url}, timeout=8) - r.raise_for_status() - s = r.text.strip() - if not s.startswith("http"): - raise ValueError(f"shortener error: {s[:60]}") - return s - - def _topo_url(lat, lon): # OpenTopoMap marker link at zoom 16 return f"https://opentopomap.org/#marker=16/{lat:.5f}/{lon:.5f}" @@ -49,13 +39,12 @@ def _has_geo(row): async def _map_reply(ctx, name, contact_name, lat, lon): - # build the OpenTopoMap link and shorten it; if da.gd is unreachable fall - # back to the full URL (short enough to send) rather than failing. + # build the OpenTopoMap link and shorten it; if no shortener is configured + # or reachable, fall back to the full URL (short enough to send). url = _topo_url(lat, lon) - try: - url = await asyncio.to_thread(_shorten_sync, url) - except Exception: - ctx.bot.logger.exception("topo: shorten failed") + url = await asyncio.to_thread( + shorten, url, ctx.bot.cfg, ctx.bot.logger, "topo", + ) or url label = (contact_name or "").strip() or "(no name)" return f"@[{name}] {label} {url}" diff --git a/config.py b/config.py index 152385b..a2a41de 100644 --- a/config.py +++ b/config.py @@ -11,6 +11,7 @@ from pathlib import Path from typing import Optional +import shortener from protocol import parse_contact_types # --------------------------------------------------------------------------- @@ -71,6 +72,17 @@ class Config: # anchor (any of the three above); the bot's own location helps but is not # required if a neighbouring hop resolves unambiguously. path_collision_radius_miles: float = 150.0 + # --- map-link shortening (!path, !topo) --- + # 'dagd' (public da.gd), 'sink' (self-hosted Sink instance), or 'none'. + # 'sink' needs sink_api_base plus a SINK_API_TOKEN env var (set it in + # [env]; this file is gitignored, command scripts are not) and falls back + # to da.gd when Sink is unreachable or rejects the URL as too long. + url_shortener: str = "dagd" + sink_api_base: str = "" + # days until a bot-created Sink link expires; 0 keeps links forever. + # Map links are single-use in practice, and the slug is derived from the + # URL, so an expired route is simply re-created on the next request. + sink_link_ttl_days: int = 30 debug: bool = False # idx -> (name, 16-byte secret) channels: dict[int, tuple[str, bytes]] = field(default_factory=dict) @@ -149,7 +161,8 @@ def target_desc(self) -> str: "radio_evict_headroom", "radio_evict_max_per_run", "radio_evict_min_interval", "radio_evict_protect_types", "advert_interval_hours", "command_delay", "watchdog_interval", - "owner_pubkeys"}, + "owner_pubkeys", "url_shortener", "sink_api_base", + "sink_link_ttl_days"}, "web": {"enabled", "host", "port", "admin_user", "admin_password_hash", "session_secret", "cors_origins", "api_tokens", "tls_cert", "tls_key"}, @@ -250,6 +263,26 @@ def load_config(args) -> Config: cfg.path_collision_radius_miles = max(0.0, parser["bot"].getfloat( "path_collision_radius_miles", cfg.path_collision_radius_miles )) + cfg.url_shortener = parser["bot"].get( + "url_shortener", cfg.url_shortener + ).strip().lower() + if cfg.url_shortener not in shortener.PROVIDERS: + sys.stderr.write( + f"ERROR: url_shortener must be one of " + f"{', '.join(shortener.PROVIDERS)}\n" + ) + sys.exit(2) + cfg.sink_api_base = parser["bot"].get( + "sink_api_base", cfg.sink_api_base + ).strip() + cfg.sink_link_ttl_days = max(0, parser["bot"].getint( + "sink_link_ttl_days", cfg.sink_link_ttl_days + )) + if cfg.url_shortener == "sink" and not cfg.sink_api_base: + sys.stderr.write( + "ERROR: url_shortener = sink needs sink_api_base\n" + ) + sys.exit(2) pk = parser["bot"].get("privkey_path", "") if pk: cfg.privkey_path = Path(pk) diff --git a/mcbot.conf.example b/mcbot.conf.example index 47303a2..05779e4 100644 --- a/mcbot.conf.example +++ b/mcbot.conf.example @@ -101,6 +101,21 @@ enabled = true ; Default: 150. ;path_collision_radius_miles = 150 +; Which service shortens the map links in !path and !topo replies. A mesh +; message has ~180 characters and a geojson.io map payload is several KB, so +; !path has no reply to give without one. +; dagd public da.gd service; no account or configuration (default) +; sink your own Sink instance; needs sink_api_base + SINK_API_TOKEN below. +; Links are upserted under a slug derived from the target URL, so the +; same route re-uses one short link. Falls back to da.gd when Sink is +; unreachable or rejects the URL (Sink caps target URLs at 2048 +; characters by default; a route past ~8 hops exceeds that). +; none no shortening: !topo sends the raw URL, !path replies '(map err)' +;url_shortener = dagd +;sink_api_base = https://sht.nz +; Days until a bot-created Sink link expires; 0 keeps them forever. +;sink_link_ttl_days = 30 + ; Path to the exported radio private key (0600). Default: .privkey ;privkey_path = ./mcbot.privkey ; Comma-separated full pubkeys bootstrapped into the 'owner' group on every @@ -174,6 +189,9 @@ owner_pubkeys = 0000000000000000000000000000000000000000000000000000000000000000 [env] ;PWS_API_KEY = your-weather-com-api-key ;PWS_STATION_ID = KXXYYYY1234 +; Sink API bearer token, used when [bot] url_shortener = sink. This is the +; instance's NUXT_SITE_TOKEN, not its NUXT_CF_API_TOKEN analytics token. +;SINK_API_TOKEN = your-sink-site-token [web] ; Optional web admin UI + REST/WebSocket API, run in-process. Disabled by diff --git a/shortener.py b/shortener.py new file mode 100644 index 0000000..e97f6dd --- /dev/null +++ b/shortener.py @@ -0,0 +1,110 @@ +"""URL shortening for command plugins. + +!path and !topo both have to fit a link into a mesh message of ~180 +characters -- a geojson.io map payload alone runs to several KB. Which +service does the shortening is an admin choice ([bot] url_shortener), +because the alternative to the public da.gd service is a self-hosted Sink +instance whose API token can't live in a git-tracked command script. + +Provider 'sink' falls back to da.gd rather than failing: a Sink instance +enforces its own maximum target-URL length (2048 characters by default), so +a long enough route would otherwise lose its map link entirely. +""" + +import hashlib +import os +import time + +import requests + +PROVIDERS = ("dagd", "sink", "none") + +_DAGD_URL = "https://da.gd/s" +_TIMEOUT = 8 +# Sink slugs match /^[a-z0-9]+(?:-[a-z0-9]+)*$/i, so hex needs no encoding. +_SINK_SLUG_PREFIX = "mc" +_SINK_SLUG_CHARS = 10 +_SINK_TOKEN_ENV = "SINK_API_TOKEN" + + +def sink_slug(long_url): + # Deterministic slug: re-shortening the same map upserts onto the same + # short link instead of minting a row per !path invocation. + digest = hashlib.sha256(long_url.encode("utf-8")).hexdigest() + return _SINK_SLUG_PREFIX + digest[:_SINK_SLUG_CHARS] + + +def _shorten_dagd(long_url): + r = requests.get(_DAGD_URL, params={"url": long_url}, timeout=_TIMEOUT) + r.raise_for_status() + s = r.text.strip() + if not s.startswith("http"): + raise ValueError(f"shortener error: {s[:60]}") + return s + + +def _shorten_sink(long_url, base, token, ttl_days, tag): + payload = {"url": long_url, "slug": sink_slug(long_url)} + if tag: + payload["tags"] = ["mcbot", tag] + payload["comment"] = f"mcbot !{tag}" + if ttl_days > 0: + payload["expiration"] = int(time.time()) + int(ttl_days) * 86400 + # upsert, not create: an identical map re-uses the existing link and + # returns status='existing'. An expired link is not considered existing, + # so the same route is re-created after its TTL lapses. + r = requests.post( + base.rstrip("/") + "/api/link/upsert", + json=payload, + headers={"Authorization": f"Bearer {token}"}, + timeout=_TIMEOUT, + ) + r.raise_for_status() + try: + short = (r.json() or {}).get("shortLink") + except ValueError: + short = None + if not short or not str(short).startswith("http"): + raise ValueError(f"sink: no shortLink in {r.status_code} response") + return str(short) + + +def shorten(long_url, cfg, logger=None, tag=""): + """Return a shortened URL, or None if none could be produced. + + Blocking (requests), so call it via asyncio.to_thread. Never raises: + every caller has its own idea of what an unshortened link means for its + reply, and none of them can send a multi-KB URL over the mesh. + """ + provider = (getattr(cfg, "url_shortener", "dagd") or "dagd").lower() + if provider == "none": + return None + + if provider == "sink": + base = getattr(cfg, "sink_api_base", "") or "" + token = os.environ.get(_SINK_TOKEN_ENV, "") + if base and token: + try: + return _shorten_sink( + long_url, base, token, + getattr(cfg, "sink_link_ttl_days", 30), tag, + ) + except Exception: + if logger: + logger.warning( + "shorten: sink failed, falling back to da.gd", + exc_info=True, + ) + elif logger: + missing = "sink_api_base" if not base else _SINK_TOKEN_ENV + logger.warning( + "shorten: url_shortener=sink but %s is unset; using da.gd", + missing, + ) + + try: + return _shorten_dagd(long_url) + except Exception: + if logger: + logger.exception("shorten: da.gd failed") + return None diff --git a/tests/test_shortener.py b/tests/test_shortener.py new file mode 100644 index 0000000..c2540c3 --- /dev/null +++ b/tests/test_shortener.py @@ -0,0 +1,194 @@ +"""Pluggable URL shortening for !path and !topo. + +Covers provider selection, the Sink upsert request the bot builds, and the +fallback chain. requests is stubbed throughout: no test touches the network. + +The fallback matters more than it looks — a Sink instance enforces its own +maximum target-URL length, and a geojson.io map for a long route is one of +the few payloads big enough to hit it.""" + +from types import SimpleNamespace + +import pytest + +import mcbot +import shortener + + +class FakeResponse: + def __init__(self, *, text="", payload=None, status=200, raise_for=None): + self.text = text + self._payload = payload + self.status_code = status + self._raise_for = raise_for + + def raise_for_status(self): + if self._raise_for: + raise self._raise_for + + def json(self): + if self._payload is None: + raise ValueError("no json") + return self._payload + + +@pytest.fixture +def net(monkeypatch): + """Records da.gd GETs and Sink POSTs; each can be scripted independently.""" + calls = {"get": [], "post": []} + scripted = {"get": FakeResponse(text="https://da.gd/short"), "post": None} + + def fake_get(url, params=None, timeout=None): + calls["get"].append({"url": url, "params": params, "timeout": timeout}) + r = scripted["get"] + if isinstance(r, Exception): + raise r + return r + + def fake_post(url, json=None, headers=None, timeout=None): + calls["post"].append({ + "url": url, "json": json, "headers": headers, "timeout": timeout, + }) + r = scripted["post"] + if isinstance(r, Exception): + raise r + return r + + monkeypatch.setattr(shortener.requests, "get", fake_get) + monkeypatch.setattr(shortener.requests, "post", fake_post) + return SimpleNamespace(calls=calls, scripted=scripted) + + +def cfg_for(provider="dagd", *, base="https://sht.nz", ttl=30): + return SimpleNamespace( + url_shortener=provider, sink_api_base=base, sink_link_ttl_days=ttl, + ) + + +@pytest.fixture +def sink_token(monkeypatch): + monkeypatch.setenv("SINK_API_TOKEN", "test-token") + + +LONG = "https://geojson.io/#data=data:application/json;base64,AAAA" + + +def test_dagd_is_the_default(net): + assert shortener.shorten(LONG, cfg_for("dagd")) == "https://da.gd/short" + assert net.calls["get"][0]["params"] == {"url": LONG}, "url passed through" + assert not net.calls["post"], "sink not called" + + +def test_none_disables_shortening(net): + assert shortener.shorten(LONG, cfg_for("none")) is None + assert not net.calls["get"] and not net.calls["post"], "no request at all" + + +def test_sink_upsert_request(net, sink_token): + net.scripted["post"] = FakeResponse( + payload={"shortLink": "https://sht.nz/mcabc", "status": "created"}, + ) + got = shortener.shorten(LONG, cfg_for("sink"), tag="path") + assert got == "https://sht.nz/mcabc", "returns Sink's shortLink" + + req = net.calls["post"][0] + assert req["url"] == "https://sht.nz/api/link/upsert", "upsert endpoint" + assert req["headers"]["Authorization"] == "Bearer test-token", "bearer auth" + body = req["json"] + assert body["url"] == LONG + assert body["slug"] == shortener.sink_slug(LONG), "deterministic slug" + assert body["tags"] == ["mcbot", "path"], "tagged for the dashboard" + assert body["expiration"] > 0, "expiring link" + assert not net.calls["get"], "no da.gd call when sink succeeds" + + +def test_sink_slug_is_stable_and_url_safe(): + a = shortener.sink_slug(LONG) + assert a == shortener.sink_slug(LONG), "same url -> same slug (upsert dedup)" + assert a != shortener.sink_slug(LONG + "B"), "different url -> different slug" + # Sink slugs must match /^[a-z0-9]+(?:-[a-z0-9]+)*$/i + assert a.isalnum() and a.islower(), f"slug not url-safe: {a}" + + +def test_zero_ttl_omits_expiration(net, sink_token): + net.scripted["post"] = FakeResponse(payload={"shortLink": "https://sht.nz/x"}) + shortener.shorten(LONG, cfg_for("sink", ttl=0)) + assert "expiration" not in net.calls["post"][0]["json"], "0 = never expires" + + +def test_sink_rejection_falls_back_to_dagd(net, sink_token): + # what a target URL over Sink's own length cap looks like from here + net.scripted["post"] = FakeResponse( + status=400, raise_for=RuntimeError("400 Validation Error"), + ) + assert shortener.shorten(LONG, cfg_for("sink")) == "https://da.gd/short", \ + "over-length/rejected url still gets a map link" + assert net.calls["post"] and net.calls["get"], "tried sink, then da.gd" + + +def test_sink_without_token_falls_back(net, monkeypatch): + monkeypatch.delenv("SINK_API_TOKEN", raising=False) + assert shortener.shorten(LONG, cfg_for("sink")) == "https://da.gd/short" + assert not net.calls["post"], "no unauthenticated sink call" + + +def test_sink_reply_without_shortlink_falls_back(net, sink_token): + net.scripted["post"] = FakeResponse(payload={"status": "created"}) + assert shortener.shorten(LONG, cfg_for("sink")) == "https://da.gd/short", \ + "malformed sink reply is not trusted" + + +def test_both_providers_down_returns_none(net, sink_token): + net.scripted["post"] = ConnectionError("sink down") + net.scripted["get"] = ConnectionError("da.gd down") + assert shortener.shorten(LONG, cfg_for("sink")) is None, "no link, no crash" + + +def test_dagd_error_body_is_not_a_link(net): + net.scripted["get"] = FakeResponse(text="Error: invalid URL") + assert shortener.shorten(LONG, cfg_for("dagd")) is None, \ + "non-http response body rejected" + + +def load_conf(tmp_path, bot_section): + p = tmp_path / "test.conf" + p.write_text(f"[radio]\nhost = 1.2.3.4\n\n[bot]\n{bot_section}\n") + return mcbot.load_config(mcbot.parse_args(["--config", str(p)])) + + +def test_config_defaults_to_dagd(tmp_path): + cfg = load_conf(tmp_path, "enabled = true") + assert cfg.url_shortener == "dagd", "unchanged behaviour without config" + + +def test_config_reads_sink_settings(tmp_path): + cfg = load_conf( + tmp_path, + "url_shortener = Sink\nsink_api_base = https://sht.nz/\n" + "sink_link_ttl_days = 7", + ) + assert cfg.url_shortener == "sink", "provider is case-insensitive" + assert cfg.sink_api_base == "https://sht.nz/" + assert cfg.sink_link_ttl_days == 7 + + +def test_config_rejects_unknown_provider(tmp_path): + # a typo must not silently fall back to a different service + with pytest.raises(SystemExit): + load_conf(tmp_path, "url_shortener = tinyurl") + + +def test_config_requires_sink_base(tmp_path): + with pytest.raises(SystemExit): + load_conf(tmp_path, "url_shortener = sink") + + +def test_failures_are_logged_not_raised(net, sink_token): + net.scripted["post"] = ConnectionError("sink down") + logged = [] + logger = SimpleNamespace( + warning=lambda *a, **k: logged.append(("warning", a)), + exception=lambda *a, **k: logged.append(("exception", a)), + ) + assert shortener.shorten(LONG, cfg_for("sink"), logger) == "https://da.gd/short" + assert any(lvl == "warning" for lvl, _ in logged), "fallback is logged" diff --git a/tests/test_topo.py b/tests/test_topo.py index 9998714..7c92d6a 100644 --- a/tests/test_topo.py +++ b/tests/test_topo.py @@ -2,7 +2,7 @@ Covers: help, short prefix, no match, single match (URL + name), single match without geo, ambiguous-prefix list, sender's own location, and the -no-location-for-sender case. The da.gd shortener is monkeypatched so the tests +no-location-for-sender case. The shortener is monkeypatched so the tests never touch the network.""" from types import SimpleNamespace @@ -14,12 +14,13 @@ _captured = {} -def _fake_shorten(url): +def _fake_shorten(url, cfg, logger=None, tag=""): _captured["url"] = url + _captured["tag"] = tag return "https://da.gd/test" -topo._shorten_sync = _fake_shorten # no network in tests +topo.shorten = _fake_shorten # no network in tests def pk(prefix): From 6cf36b3b3d3f8e9a19a04c322ff9e4ae6a32e64a Mon Sep 17 00:00:00 2001 From: steve Date: Thu, 30 Jul 2026 22:58:09 +0000 Subject: [PATCH 2/3] fix: let Sink assign its own 6-character slug instead of a 12-character deterministic one --- Bot_Usage.md | 9 ++++++--- mcbot.conf.example | 7 +++---- shortener.py | 26 ++++++++++---------------- tests/test_shortener.py | 23 +++++++++++------------ 4 files changed, 30 insertions(+), 35 deletions(-) diff --git a/Bot_Usage.md b/Bot_Usage.md index 34d77e8..b34dd9f 100644 --- a/Bot_Usage.md +++ b/Bot_Usage.md @@ -368,9 +368,12 @@ SINK_API_TOKEN = The token is Sink's `NUXT_SITE_TOKEN` (its API bearer credential), *not* the `NUXT_CF_API_TOKEN` analytics token. Notes on the `sink` provider: -- Links are created with `POST /api/link/upsert` and a slug derived from a - hash of the target URL, so re-requesting the same route re-uses the - existing short link instead of adding a row per invocation. +- Links are created with `POST /api/link/create` and no slug, so Sink + assigns its own -- 6 characters by default. Every character matters in a + mesh message, which is why the bot doesn't supply a longer deterministic + slug to make repeat routes share a link; `sink_link_ttl_days` keeps the + table from growing without bound instead. To go shorter still, set + `NUXT_PUBLIC_SLUG_DEFAULT_LENGTH` on the Sink instance. - **It falls back to da.gd** when Sink is unreachable, misconfigured, or rejects the URL. Sink caps target URLs (2048 characters by default), and a route past ~8 hops exceeds that -- the fallback is what keeps long routes diff --git a/mcbot.conf.example b/mcbot.conf.example index 05779e4..34ad88b 100644 --- a/mcbot.conf.example +++ b/mcbot.conf.example @@ -106,10 +106,9 @@ enabled = true ; !path has no reply to give without one. ; dagd public da.gd service; no account or configuration (default) ; sink your own Sink instance; needs sink_api_base + SINK_API_TOKEN below. -; Links are upserted under a slug derived from the target URL, so the -; same route re-uses one short link. Falls back to da.gd when Sink is -; unreachable or rejects the URL (Sink caps target URLs at 2048 -; characters by default; a route past ~8 hops exceeds that). +; Sink assigns each link its own 6-character slug. Falls back to da.gd +; when Sink is unreachable or rejects the URL (Sink caps target URLs +; at 2048 characters by default; a route past ~8 hops exceeds that). ; none no shortening: !topo sends the raw URL, !path replies '(map err)' ;url_shortener = dagd ;sink_api_base = https://sht.nz diff --git a/shortener.py b/shortener.py index e97f6dd..8a95e08 100644 --- a/shortener.py +++ b/shortener.py @@ -11,7 +11,6 @@ a long enough route would otherwise lose its map link entirely. """ -import hashlib import os import time @@ -21,19 +20,9 @@ _DAGD_URL = "https://da.gd/s" _TIMEOUT = 8 -# Sink slugs match /^[a-z0-9]+(?:-[a-z0-9]+)*$/i, so hex needs no encoding. -_SINK_SLUG_PREFIX = "mc" -_SINK_SLUG_CHARS = 10 _SINK_TOKEN_ENV = "SINK_API_TOKEN" -def sink_slug(long_url): - # Deterministic slug: re-shortening the same map upserts onto the same - # short link instead of minting a row per !path invocation. - digest = hashlib.sha256(long_url.encode("utf-8")).hexdigest() - return _SINK_SLUG_PREFIX + digest[:_SINK_SLUG_CHARS] - - def _shorten_dagd(long_url): r = requests.get(_DAGD_URL, params={"url": long_url}, timeout=_TIMEOUT) r.raise_for_status() @@ -44,17 +33,22 @@ def _shorten_dagd(long_url): def _shorten_sink(long_url, base, token, ttl_days, tag): - payload = {"url": long_url, "slug": sink_slug(long_url)} + # No slug: Sink generates its own, which is 6 characters by default + # (slugDefaultLength) against a 30-character alphabet. Every character + # counts in a ~180-character mesh message, so that beats a longer + # deterministic slug that would let repeat routes share one link -- + # sink_link_ttl_days does the housekeeping instead. + payload = {"url": long_url} if tag: payload["tags"] = ["mcbot", tag] payload["comment"] = f"mcbot !{tag}" if ttl_days > 0: payload["expiration"] = int(time.time()) + int(ttl_days) * 86400 - # upsert, not create: an identical map re-uses the existing link and - # returns status='existing'. An expired link is not considered existing, - # so the same route is re-created after its TTL lapses. + # create, not upsert: on the (remote) chance Sink's generated slug is + # already taken, create reports 409 rather than upsert's silent + # "here's the existing link", which would hand back someone else's map. r = requests.post( - base.rstrip("/") + "/api/link/upsert", + base.rstrip("/") + "/api/link/create", json=payload, headers={"Authorization": f"Bearer {token}"}, timeout=_TIMEOUT, diff --git a/tests/test_shortener.py b/tests/test_shortener.py index c2540c3..9d993e5 100644 --- a/tests/test_shortener.py +++ b/tests/test_shortener.py @@ -1,6 +1,6 @@ """Pluggable URL shortening for !path and !topo. -Covers provider selection, the Sink upsert request the bot builds, and the +Covers provider selection, the Sink create request the bot builds, and the fallback chain. requests is stubbed throughout: no test touches the network. The fallback matters more than it looks — a Sink instance enforces its own @@ -84,30 +84,29 @@ def test_none_disables_shortening(net): assert not net.calls["get"] and not net.calls["post"], "no request at all" -def test_sink_upsert_request(net, sink_token): +def test_sink_create_request(net, sink_token): net.scripted["post"] = FakeResponse( - payload={"shortLink": "https://sht.nz/mcabc", "status": "created"}, + payload={"shortLink": "https://sht.nz/abc123", "status": "created"}, ) got = shortener.shorten(LONG, cfg_for("sink"), tag="path") - assert got == "https://sht.nz/mcabc", "returns Sink's shortLink" + assert got == "https://sht.nz/abc123", "returns Sink's shortLink" req = net.calls["post"][0] - assert req["url"] == "https://sht.nz/api/link/upsert", "upsert endpoint" + assert req["url"] == "https://sht.nz/api/link/create", "create endpoint" assert req["headers"]["Authorization"] == "Bearer test-token", "bearer auth" body = req["json"] assert body["url"] == LONG - assert body["slug"] == shortener.sink_slug(LONG), "deterministic slug" assert body["tags"] == ["mcbot", "path"], "tagged for the dashboard" assert body["expiration"] > 0, "expiring link" assert not net.calls["get"], "no da.gd call when sink succeeds" -def test_sink_slug_is_stable_and_url_safe(): - a = shortener.sink_slug(LONG) - assert a == shortener.sink_slug(LONG), "same url -> same slug (upsert dedup)" - assert a != shortener.sink_slug(LONG + "B"), "different url -> different slug" - # Sink slugs must match /^[a-z0-9]+(?:-[a-z0-9]+)*$/i - assert a.isalnum() and a.islower(), f"slug not url-safe: {a}" +def test_no_slug_is_sent(net, sink_token): + # Sink's own generated slug is 6 characters; supplying one of our own + # would only make the reply longer, and a mesh message has ~180 to spend. + net.scripted["post"] = FakeResponse(payload={"shortLink": "https://sht.nz/x"}) + shortener.shorten(LONG, cfg_for("sink")) + assert "slug" not in net.calls["post"][0]["json"], "sink picks the slug" def test_zero_ttl_omits_expiration(net, sink_token): From 5b78953b9b6552fa846878c917295f9414623258 Mon Sep 17 00:00:00 2001 From: steve Date: Thu, 30 Jul 2026 23:00:42 +0000 Subject: [PATCH 3/3] docs: clarify that the Sink target-URL cap is per-instance, not fixed at 2048 --- Bot_Usage.md | 11 +++++++---- mcbot.conf.example | 6 ++++-- shortener.py | 7 ++++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Bot_Usage.md b/Bot_Usage.md index b34dd9f..f56b48e 100644 --- a/Bot_Usage.md +++ b/Bot_Usage.md @@ -375,10 +375,13 @@ The token is Sink's `NUXT_SITE_TOKEN` (its API bearer credential), *not* the table from growing without bound instead. To go shorter still, set `NUXT_PUBLIC_SLUG_DEFAULT_LENGTH` on the Sink instance. - **It falls back to da.gd** when Sink is unreachable, misconfigured, or - rejects the URL. Sink caps target URLs (2048 characters by default), and a - route past ~8 hops exceeds that -- the fallback is what keeps long routes - from silently losing their map. Raise `UrlSchema` in your Sink instance's - `shared/schemas/link.ts` if you'd rather it accept them directly. + rejects the URL, so a route never silently loses its map. The one limit + worth knowing is the target-URL cap each instance enforces: stock Sink + allows 2048 characters, which a route past ~8 hops exceeds. It is a + one-line change -- `UrlSchema` in `shared/schemas/link.ts` -- and at 8192 + a route would have to exceed ~30 hops to hit it, which no real mesh path + does. Check yours before assuming long routes reach Sink rather than the + fallback. - Clicks on bot-generated links land in your Sink analytics. Changing any of these needs a bot restart -- `!adm reload` re-executes diff --git a/mcbot.conf.example b/mcbot.conf.example index 34ad88b..b6e7837 100644 --- a/mcbot.conf.example +++ b/mcbot.conf.example @@ -107,8 +107,10 @@ enabled = true ; dagd public da.gd service; no account or configuration (default) ; sink your own Sink instance; needs sink_api_base + SINK_API_TOKEN below. ; Sink assigns each link its own 6-character slug. Falls back to da.gd -; when Sink is unreachable or rejects the URL (Sink caps target URLs -; at 2048 characters by default; a route past ~8 hops exceeds that). +; when Sink is unreachable or rejects the URL -- e.g. for exceeding +; the instance's target-URL cap (UrlSchema in shared/schemas/link.ts; +; stock Sink allows 2048 characters, which a route past ~8 hops +; exceeds; at 8192 no real path comes close). ; none no shortening: !topo sends the raw URL, !path replies '(map err)' ;url_shortener = dagd ;sink_api_base = https://sht.nz diff --git a/shortener.py b/shortener.py index 8a95e08..2de1e12 100644 --- a/shortener.py +++ b/shortener.py @@ -6,9 +6,10 @@ because the alternative to the public da.gd service is a self-hosted Sink instance whose API token can't live in a git-tracked command script. -Provider 'sink' falls back to da.gd rather than failing: a Sink instance -enforces its own maximum target-URL length (2048 characters by default), so -a long enough route would otherwise lose its map link entirely. +Provider 'sink' falls back to da.gd rather than failing, since an instance +can be down, misconfigured, or stricter than the bot expects -- a Sink +instance enforces its own maximum target-URL length, 2048 characters unless +raised, and a long enough route would otherwise lose its map link entirely. """ import os