fix(response_template,url_shortener): follow-ups to PR #254

Integration fixes on top of the merge, in three groups.

Regressions in the shared shortener, which is not opt-in — weather alerts
call it unconditionally whenever an alert carries a link:

- Restore the `response.ok` guard on the v.gd path. Without it a 503 whose
  body happens to start with `http` (a captive portal, a CDN error page) was
  returned as the short URL. The test that covered this still passed because
  it never set `.text`, so `MagicMock.startswith()` answered truthily; it now
  supplies a body that would be accepted on a 200, so only the status check
  can make it pass.
- Add the same guard to the shlink path, where a rejected API key is a 401
  with a problem-details body and was indistinguishable from an empty result.
- Restore debug-level logging for `Timeout`/`ConnectionError`. A mesh node's
  uplink drops out routinely and this had become `logger.error`.
- Drop the `shortUrlSlug` fallback. Shlink's create response carries
  `shortUrl` and `shortCode`; `shortUrlSlug` is not in its schema, and a bare
  slug is not a link — emitting one would have put `abc123` in a message.

The `shorten_url` filter did blocking HTTP inside the synchronous render
path (`process_message` → `check_keywords` → `format_response`), so a 5 s
shortener timeout stalled the radio transport along with everything else.
Making `check_keywords` async would touch 27 call sites, so instead the
network work moves ahead of the render:

- `resolve_template_async()` renders once with the filter in collection mode,
  which walks the real chain so a clause already suppressed by `hops_min`
  costs no request, then shortens what survived concurrently off-thread.
- The filter itself now only reads that mapping and never calls out. Asked to
  render without a pre-pass it warns and drops the clause rather than block.
- On failure the clause is dropped rather than sent unshortened. A v.gd link
  is ~19 bytes where the analyzer URL is ~59, against a ~158-byte budget the
  prefix is subtracted from, so falling back would have turned one
  transmission into two whenever the shortener was unreachable.

Smaller: rename `if_notempty` to `if_nonempty` to match the existing
`prefix_if_nonempty` (nothing ships using it yet, so there is no config to
migrate); `_build_create_shlink_url` no longer takes the long URL and API key
it never used; move the CHANGELOG entry from Fixed to Added and name the
filter it actually added; ungate the render debug log from `config`; document
that `shorten_url` is supported in `path`'s `reply_prefix` only.

Every template shipped in config.ini.example still renders identically to the
regex parser it replaced.

CHANGELOG.md

@@ -6,12 +6,27 @@ semantic versioning.
 
 ## [Unreleased]
 
-### Fixed
+### Added
 
-- Refactor response_template parsing to use finite state machine when processing
-  templates adding additional flexibility such as the use of nested fields. Added
-  support for shlink to External_Data configuration. Added url_shortener filter and
-  if_notempty filter to response_template.
+- Response templates now parse with a character-by-character state machine instead of
+  a regular expression, which lifts the restriction that a placeholder could contain
+  no `{}` of its own. A placeholder may now hold a double-quoted string literal with
+  further `{field}` placeholders nested inside it, so a whole URL can be assembled in
+  config rather than hard-coded: `{packet_hash|if_nonempty:"https://scope.example.net/#/packets/{packet_hash}"}`.
+  Every template shipped in `config.ini.example` renders identically to before.
+- `if_nonempty:LITERAL` renders `LITERAL` when the value survives the preceding
+  filters and nothing at all otherwise — the counterpart to `prefix_if_nonempty`,
+  for when the whole clause should be the literal rather than a label plus the value.
+- `shorten_url` shortens a value through the shortener configured under
+  `[External_Data]`, for putting a link in `path`'s `reply_prefix` without spending
+  a packet on a full URL. The request is made off the event loop before rendering
+  starts; if it fails the clause is dropped rather than sent unshortened, so a
+  shortener outage costs the link instead of a second transmission.
+- Shlink is supported as a URL shortener backend alongside v.gd / is.gd, selected
+  with `short_url_website_service` under `[External_Data]` (`gd`, the default, or
+  `shlink`). Shlink authenticates with `short_url_website_api_key`.
+
+### Fixed
 
 - `path` no longer answers "No path information available in current message" on a
   busy mesh (#255). Verifying a channel message against the RF cache only ever

config.ini.example

@@ -644,7 +644,9 @@ log_max_bytes = 5242880
 # log_backup_count: number of rotated backup files to keep (e.g. meshcore_bot.log.1 … .3)
 log_backup_count = 3
 [External_Data]
-# URL shortener Service (gd,shlink). Default: gd
+# URL shortener backend: gd (v.gd / is.gd-compatible create.php) or shlink.
+# shlink posts to <short_url_website>/rest/v3/short-urls and requires
+# short_url_website_api_key below. Default: gd
 short_url_website_service = gd
 # URL shortener API base (v.gd / is.gd-compatible create.php). Default: https://v.gd
 # See https://v.gd/apishorteningreference.php

docs/path-command-config.md

@@ -27,18 +27,22 @@ These options only affect the **path** command’s reply text and whether repeat
 reply_prefix = "{path_distance|prefix_if_nonempty:📏 }\n"
 ```
 - `hops_min:N` clears a field unless the message actually travelled at least N hops. `{path_distance}` renders `N/A` on a direct message, which `prefix_if_nonempty` treats as a value, so gate it first: `{path_distance|hops_min:1|prefix_if_nonempty:📏 }`. Unlike `pathbytes_min:N`, which asks how the path is *encoded*, this keeps a measurable one-byte multi-hop path.
-- `if_notempty:LITERAL` renders `LITERAL` when the value is non-empty after prior filters, and clears entirely otherwise — the opposite pairing of `prefix_if_nonempty`, useful when the whole output should be a fixed (or field-built) literal rather than the value with a label prepended. Since `{packet_hash}` is empty whenever RF correlation fails, gating on it hides the whole clause instead of printing a broken link:
+- `if_nonempty:LITERAL` renders `LITERAL` when the value is non-empty after prior filters, and clears entirely otherwise — the opposite pairing of `prefix_if_nonempty`, useful when the whole output should be a fixed (or field-built) literal rather than the value with a label prepended. Since `{packet_hash}` is empty whenever RF correlation fails, gating on it hides the whole clause instead of printing a broken link:
 
 ```ini
-reply_prefix = {packet_hash|if_notempty:"https://scope.example.net/#/packets/{packet_hash}"}
+reply_prefix = {packet_hash|if_nonempty:"https://scope.example.net/#/packets/{packet_hash}"}
 ```
   The `LITERAL` argument may itself be a double-quoted string containing nested `{field}` placeholders (expanded before the filter runs), so the link above still carries the packet hash even though the field being gated on (`packet_hash`) and the field inside the literal are the same one.
-- `shorten_url` runs the value through the shared shortener configured under `[External_Data]` (`short_url_website`, `short_url_website_service` — `gd` for v.gd/is.gd-compatible or `shlink`, and `short_url_website_api_key` where required). It falls back to the original, unshortened value if shortening fails or isn't configured, so a clause never silently disappears because of a network error. Chain it after building the link so only the final URL is sent over RF:
+- `shorten_url` replaces the value with a short link from the shortener configured under `[External_Data]` (`short_url_website`, `short_url_website_service` — `gd` for v.gd/is.gd-compatible or `shlink`, and `short_url_website_api_key`, which shlink requires). Chain it after building the link so only the final URL is sent over RF:
 
 ```ini
-reply_prefix = {packet_hash|if_notempty:"https://scope.example.net/#/packets/{packet_hash}"|shorten_url}
+reply_prefix = {packet_hash|if_nonempty:"https://scope.example.net/#/packets/{packet_hash}"|shorten_url}
 ```
 
+  If shortening fails or isn't configured, **the clause is dropped rather than sent unshortened**. A v.gd link costs about 19 bytes; the analyzer URL above is about 59, against a per-message budget of roughly 158–160 bytes that the reply prefix is subtracted from before the route list is packed. Falling back to the long URL would quietly turn one transmission into two every time the shortener was unreachable, so an outage costs you the link, not extra airtime.
+
+  `shorten_url` is currently supported in `path`'s `reply_prefix` only. Rendering is synchronous and happens on the event loop, so the HTTP request is made ahead of the render by `resolve_template_async()`, which the `path` command awaits. Used in a template that has no such pre-pass — the test command's `response_format`, for example — the filter logs a warning and drops the clause instead of blocking the bot for the length of the shortener's timeout.
+
 **`minimum_path_bytes`** (integer `0`–`3`, default `0`)
 
 - **`0` or `1`**: Always resolve repeater names when decoding a path (legacy behavior).

modules/commands/path_command.py

@@ -15,7 +15,7 @@ from ..path_inference import (
     select_node_repeater,
     select_repeater_by_graph,
 )
-from ..response_template import format_piped_template
+from ..response_template import format_piped_template, resolve_template_async
 from ..utils import (
     bytes_per_hop_from_routing_and_nodes,
     calculate_distance,
@@ -415,18 +415,30 @@ class PathCommand(BaseCommand):
             return ''
         return f"{distance:.1f}km"
 
-    def _format_path_reply_prefix(self, message: MeshMessage) -> str:
+    async def _format_path_reply_prefix(self, message: MeshMessage) -> str:
         if not self.path_reply_prefix:
             return ''
         fields = self.get_standard_placeholder_fields(message)
         fields['path_distance'] = self._format_path_distance(message)
-        formatted = format_piped_template(
+        str_fields = {k: str(v) for k, v in fields.items()}
+        # Any URL shortening happens here, off the event loop, before the
+        # synchronous render runs. See modules.response_template.
+        shortened = await resolve_template_async(
             self.path_reply_prefix,
-            {k: str(v) for k, v in fields.items()},
+            str_fields,
             message=message,
             logger=self.logger,
             config=self.bot.config,
             prefix_hex_chars=getattr(self.bot, 'prefix_hex_chars', 2),
+        )
+        formatted = format_piped_template(
+            self.path_reply_prefix,
+            str_fields,
+            message=message,
+            logger=self.logger,
+            config=self.bot.config,
+            shortened=shortened,
+            prefix_hex_chars=getattr(self.bot, 'prefix_hex_chars', 2),
         ).rstrip()
         if not formatted:
             return ''
@@ -1057,7 +1069,7 @@ class PathCommand(BaseCommand):
 
     async def _send_path_response(self, message: MeshMessage, response: str):
         """Send path response, splitting into multiple messages if necessary"""
-        prefix = self._format_path_reply_prefix(message)
+        prefix = await self._format_path_reply_prefix(message)
         self.last_response = prefix + response if prefix else response
 
         max_length = self.get_max_message_length(message)

modules/response_template.py

@@ -11,9 +11,10 @@ chain, evaluated left to right.
 
 from __future__ import annotations
 
+import asyncio
 from typing import Any, Callable
 
-from .url_shortener import shorten_url_sync
+from .url_shortener import shorten_url
 from .utils import message_hop_count, message_path_bytes_per_hop
 
 FilterFn = Callable[[str, dict[str, Any], str], str]
@@ -70,30 +71,55 @@ def _filter_prefix_if_nonempty(value: str, ctx: dict[str, Any], args: str) -> st
         return ''
     return args + value
 
-def _filter_shorten_url(value: str, ctx: dict[str, Any], args: str) -> str:
-    """Shorten *value* URL using configured URL shortener (v.gd / is.gd compatible)."""
-    logger = ctx.get('logger')
-    config = ctx.get('config')
-    if logger is not None:
-        logger.debug("Shortening URL %r", value)
-    if config is None or value == '':
-        if logger is not None:
-            logger.debug("Abandoning shorten url due to empty value or config")
-        return value
-    return shorten_url_sync(value, config=config, logger=logger) or value
 
-def _filter_if_notempty(value: str, ctx: dict[str, Any], args: str) -> str:
+def _filter_shorten_url(value: str, ctx: dict[str, Any], args: str) -> str:
+    """Swap *value* for its shortened form, resolved ahead of time.
+
+    Rendering is synchronous and runs on the event loop, so this filter never
+    performs the HTTP request itself: a 5 s shortener timeout here would stall the
+    radio transport along with everything else. :func:`resolve_template_async`
+    does the network work off-thread first and leaves the answers in ``ctx``.
+
+    On a miss the clause is dropped rather than falling back to the long URL. A
+    v.gd link is ~19 bytes against a 158-160 byte message budget where a real
+    analyzer URL is ~59, and ``_send_path_response`` subtracts the prefix from the
+    first segment's budget — so falling back would quietly turn one transmission
+    into two every time the shortener was unreachable.
+    """
+    if not value:
+        return ''
+    resolved = ctx.get('shortened')
+
+    # Collection pass: record what needs shortening, change nothing.
+    if isinstance(resolved, set):
+        resolved.add(value)
+        return value
+
+    if isinstance(resolved, dict):
+        return resolved.get(value, '')
+
+    logger = ctx.get('logger')
+    if logger is not None:
+        logger.warning(
+            "shorten_url used in a template rendered without resolve_template_async(); "
+            "dropping the clause rather than blocking the event loop"
+        )
+    return ''
+
+
+def _filter_if_nonempty(value: str, ctx: dict[str, Any], args: str) -> str:
     """Return *args* literal only when *value* is non-empty after prior filters."""
     if not value:
         return ''
     return args
 
+
 RESPONSE_TEMPLATE_FILTERS: dict[str, FilterFn] = {
     'pathbytes_min': _filter_pathbytes_min,
     'pathbytes': _filter_pathbytes_min,
     'hops_min': _filter_hops_min,
     'prefix_if_nonempty': _filter_prefix_if_nonempty,
-    'if_notempty': _filter_if_notempty,
+    'if_nonempty': _filter_if_nonempty,
     'shorten_url': _filter_shorten_url,
 }
 
@@ -257,6 +283,7 @@ def format_piped_template(
     message: Any = None,
     logger: Any = None,
     config: Any = None,
+    shortened: dict[str, str] | None = None,
     prefix_hex_chars: int = 2,
 ) -> str:
     """Replace ``{field}``, ``{"literal {field}"}``, and their piped filter chains.
@@ -268,6 +295,10 @@ def format_piped_template(
             lets ``prefix_if_nonempty`` drop its literal label too.
         message: Triggering mesh message; required for ``pathbytes`` / ``pathbytes_min`` filters.
         logger: Optional logger for unknown filter warnings.
+        config: Bot config, for filters that read ``[External_Data]``.
+        shortened: Long-URL to short-URL mapping from :func:`resolve_template_async`.
+            Required by the ``shorten_url`` filter, which will not make a network
+            call from this synchronous path.
         prefix_hex_chars: Bot prefix width for inferring bytes per hop from legacy path text.
 
     Returns:
@@ -278,7 +309,68 @@ def format_piped_template(
         'logger': logger,
         'prefix_hex_chars': prefix_hex_chars,
         'config': config,
+        'shortened': shortened,
     }
-    if (logger is not None) and (config is not None):
+    if logger is not None:
         logger.debug("Rendering response template %r with fields %r", template, fields)
     return _TemplateParser(template, fields, ctx, logger).render()
+
+
+def template_needs_resolution(template: str) -> bool:
+    """True if *template* uses a filter that needs :func:`resolve_template_async`.
+
+    A cheap substring test so the common template pays nothing for a feature it
+    does not use; the collection pass below is what actually decides.
+    """
+    return 'shorten_url' in template
+
+
+async def resolve_template_async(
+    template: str,
+    fields: dict[str, Any],
+    *,
+    message: Any = None,
+    logger: Any = None,
+    config: Any = None,
+    prefix_hex_chars: int = 2,
+) -> dict[str, str]:
+    """Resolve *template*'s network-backed filters off the event loop.
+
+    Renders the template once with ``shorten_url`` in collection mode, which walks
+    the real filter chain — so gating filters such as ``hops_min`` have already had
+    their say and a suppressed clause costs no request — then shortens whatever
+    survived, concurrently and in a worker thread. Pass the result to
+    :func:`format_piped_template` as ``shortened``.
+
+    Returns an empty mapping when there is nothing to do, which renders exactly as
+    an unresolved template would.
+    """
+    if config is None or not template_needs_resolution(template):
+        return {}
+
+    pending: set[str] = set()
+    ctx: dict[str, Any] = {
+        'message': message,
+        'logger': logger,
+        'prefix_hex_chars': prefix_hex_chars,
+        'config': config,
+        'shortened': pending,
+    }
+    _TemplateParser(template, fields, ctx, logger).render()
+    if not pending:
+        return {}
+
+    urls = sorted(pending)
+    results = await asyncio.gather(
+        *(shorten_url(u, config=config, logger=logger) for u in urls),
+        return_exceptions=True,
+    )
+    resolved: dict[str, str] = {}
+    for url, short in zip(urls, results, strict=True):
+        if isinstance(short, BaseException):
+            if logger is not None:
+                logger.debug("Shortening %r failed: %s", url, short)
+            continue
+        if short:
+            resolved[url] = short
+    return resolved

modules/url_shortener.py

@@ -9,6 +9,7 @@ Configure base URL and optional API key under [External_Data] in config.ini.
 from __future__ import annotations
 
 import asyncio
+import json
 import logging
 from typing import Any
 from urllib.parse import quote
@@ -104,7 +105,13 @@ def _build_create_gd_url(long_url: str, base: str, api_key: str) -> str:
     return rebuilt
 
 
-def _build_create_shlink_url(long_url: str, base: str, api_key: str) -> str:
+def _build_create_shlink_url(base: str) -> str:
+    """Build the Shlink create endpoint from *base*.
+
+    Shlink authenticates with an ``X-Api-Key`` header, so unlike the v.gd builder
+    this takes neither the long URL nor the key — nothing about them belongs in
+    the URL, and passing them in invited the assumption that they did.
+    """
     from urllib.parse import urlparse, urlunparse
 
     root = _normalize_base(base)
@@ -130,10 +137,8 @@ def _shorten_url_with_shlink(
     timeout: float = 5.0,
     logger: logging.Logger | None = None,
 ) -> str:
-    """Shorten a URL using Shlink API."""
-    import json
-
-    shortener_url = _build_create_shlink_url(long_url, base, api_key)
+    """Shorten a URL using the Shlink API."""
+    shortener_url = _build_create_shlink_url(base)
     headers = {
         "Content-Type": "application/json",
         "X-Api-Key": api_key,
@@ -142,16 +147,24 @@ def _shorten_url_with_shlink(
         {"longUrl": long_url, "findIfExists": True, "tags": ["meshcore-bot"]}
     )
 
-    get = session.post if session is not None else requests.post
-    response = get(shortener_url, headers=headers, data=payload, timeout=timeout)
-    if logger:
-        logger.debug("Shlink response: %s", response.text)
+    post = session.post if session is not None else requests.post
+    response = post(shortener_url, headers=headers, data=payload, timeout=timeout)
+    if not response.ok:
+        # A bad API key is a 401 with a JSON problem-details body; without this the
+        # misconfiguration is indistinguishable from "the shortener had nothing".
+        if logger:
+            logger.debug("Error shortening URL: HTTP %s", response.status_code)
+        return ""
+
     data = response.json()
-    short_url = data.get("shortUrl") or data.get("shortUrlSlug")
-
+    # Shlink's create response carries `shortUrl` (and `shortCode`, which is a bare
+    # slug, not a URL). Anything else means we did not get a usable link.
+    short_url = data.get("shortUrl")
     if short_url:
-        return short_url
+        return str(short_url)
 
+    if logger:
+        logger.debug("Shlink response had no shortUrl: %s", str(data)[:200])
     return ""
 
 
@@ -169,10 +182,17 @@ def _shorten_url_with_gd(
     get = session.get if session is not None else requests.get
 
     response = get(shortener_url, timeout=timeout)
+    if not response.ok:
+        if logger:
+            logger.debug("Error shortening URL: HTTP %s", response.status_code)
+        return ""
+
     short = _parse_simple_response(response.text)
     if short:
         return short
 
+    if logger:
+        logger.debug("URL shortener returned error: %s", response.text.strip()[:200])
     return ""
 
 
@@ -232,9 +252,15 @@ def shorten_url_sync(
             logger=logger,
         )
 
+    except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as e:
+        # A mesh node's uplink drops out routinely; that is not an error worth
+        # raising the log level for, and it used to be logged at debug.
+        if logger:
+            logger.debug("Error shortening URL: %s", e)
+        return ""
     except Exception as e:
         if logger:
-            logger.error("Unexpected error shortening URL: %s", e)
+            logger.debug("shorten_url_sync failed: %s", e)
         return ""
 
 

tests/test_url_shortener.py

@@ -43,22 +43,17 @@ class TestBuildCreateGdUrl:
 
 class TestBuildCreateShlinkUrl:
     def test_appends_rest_v3_short_urls_path(self):
-        u = _build_create_shlink_url("http://a.com", "https://short.example", "k1")
+        u = _build_create_shlink_url("https://short.example")
         assert u == "https://short.example/rest/v3/short-urls"
 
     def test_strips_trailing_slash_on_base(self):
-        u = _build_create_shlink_url("http://a.com", "https://short.example/", "k1")
+        u = _build_create_shlink_url("https://short.example/")
         assert u == "https://short.example/rest/v3/short-urls"
 
     def test_bare_hostname_gets_https_scheme(self):
-        u = _build_create_shlink_url("http://a.com", "short.example", "k1")
+        u = _build_create_shlink_url("short.example")
         assert u == "https://short.example/rest/v3/short-urls"
 
-    def test_api_key_never_appears_in_the_url(self):
-        """Shlink authenticates via the X-Api-Key header, not a query param."""
-        u = _build_create_shlink_url("http://a.com", "https://short.example", "super-secret")
-        assert "super-secret" not in u
-
 
 class TestCoerceUrlString:
     def test_dict_href(self):
@@ -185,10 +180,14 @@ class TestShortenUrlSync:
         assert call_url.startswith("https://v.gd/create.php")
 
     def test_http_error_returns_empty(self):
+        """Regression: the body is only trusted once the status says it is a real
+        answer. A captive portal or CDN error page can return 503 with a URL in the
+        body, and that URL is not a short link."""
         cfg = _minimal_config()
         mock_resp = MagicMock()
         mock_resp.ok = False
         mock_resp.status_code = 503
+        mock_resp.text = "http://portal.example/login?next=1"
         session = MagicMock()
         session.get.return_value = mock_resp
 
@@ -243,6 +242,7 @@ class TestShortenUrlSyncShlink:
     def test_success_returns_short_url(self):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.return_value = {"shortUrl": "https://short.example/abc123"}
         session = MagicMock()
         session.post.return_value = mock_resp
@@ -255,6 +255,7 @@ class TestShortenUrlSyncShlink:
     def test_posts_to_rest_v3_short_urls_with_api_key_header(self):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.return_value = {"shortUrl": "https://short.example/abc123"}
         session = MagicMock()
         session.post.return_value = mock_resp
@@ -269,19 +270,22 @@ class TestShortenUrlSyncShlink:
         assert payload["longUrl"] == "https://example.com/long/path"
         assert payload["findIfExists"] is True
 
-    def test_falls_back_to_short_url_slug(self):
+    def test_bare_short_code_is_not_treated_as_a_link(self):
+        """Shlink returns `shortCode` alongside `shortUrl`, but a slug on its own is
+        not a URL — emitting one would put `abc123` in a mesh message."""
         cfg = self._shlink_config()
         mock_resp = MagicMock()
-        mock_resp.json.return_value = {"shortUrlSlug": "abc123"}
+        mock_resp.ok = True
+        mock_resp.json.return_value = {"shortCode": "abc123"}
         session = MagicMock()
         session.post.return_value = mock_resp
 
-        out = shorten_url_sync("http://a.com", config=cfg, session=session)
-        assert out == "abc123"
+        assert shorten_url_sync("http://a.com", config=cfg, session=session) == ""
 
     def test_missing_short_url_in_response_returns_empty(self):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.return_value = {"unexpected": "shape"}
         session = MagicMock()
         session.post.return_value = mock_resp
@@ -299,6 +303,19 @@ class TestShortenUrlSyncShlink:
         assert out == ""
         session.post.assert_not_called()
 
+    def test_http_error_returns_empty(self):
+        """A rejected API key comes back as 401 with a problem-details body; without
+        a status check that is indistinguishable from an empty result."""
+        cfg = self._shlink_config()
+        mock_resp = MagicMock()
+        mock_resp.ok = False
+        mock_resp.status_code = 401
+        session = MagicMock()
+        session.post.return_value = mock_resp
+
+        assert shorten_url_sync("http://a.com", config=cfg, session=session) == ""
+        mock_resp.json.assert_not_called()
+
     def test_request_exception_returns_empty(self):
         cfg = self._shlink_config()
         session = MagicMock()
@@ -309,6 +326,7 @@ class TestShortenUrlSyncShlink:
     def test_malformed_json_response_returns_empty(self):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.side_effect = ValueError("not json")
         session = MagicMock()
         session.post.return_value = mock_resp
@@ -319,6 +337,7 @@ class TestShortenUrlSyncShlink:
     def test_no_session_uses_requests_post(self, mock_post):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.return_value = {"shortUrl": "https://short.example/xyz"}
         mock_post.return_value = mock_resp
 
@@ -352,6 +371,7 @@ async def test_shorten_url_async_shlink():
         short_url_website_api_key="test-api-key",
     )
     mock_resp = MagicMock()
+    mock_resp.ok = True
     mock_resp.json.return_value = {"shortUrl": "https://short.example/async1"}
     session = MagicMock()
     session.post.return_value = mock_resp

tests/unit/test_response_template.py

@@ -2,13 +2,17 @@
 """Unit tests for piped response templates and message_path_bytes_per_hop."""
 
 import configparser
-from unittest.mock import MagicMock, Mock
+from unittest.mock import MagicMock, Mock, patch
 
 import pytest
 
 from modules.commands.test_command import TestCommand as MeshTestCommand
 from modules.models import MeshMessage
-from modules.response_template import format_piped_template
+from modules.response_template import (
+    format_piped_template,
+    resolve_template_async,
+    template_needs_resolution,
+)
 from modules.utils import message_path_bytes_per_hop
 
 
@@ -389,16 +393,103 @@ def test_nested_placeholder_inside_a_quoted_literal_can_carry_its_own_filter():
     assert format_piped_template('{"Dist: {d|hops_min:5}"}', {"d": "12.4km"}, message=msg) == "Dist: "
 
 
+_LINK_TEMPLATE = (
+    '{packet_hash | if_nonempty: '
+    '"https://analyzer.example.net/#/packets/{packet_hash}?obs=1620457" '
+    '| shorten_url}'
+)
+_LONG_LINK = "https://analyzer.example.net/#/packets/ABCDEF12?obs=1620457"
+
+
 @pytest.mark.unit
 def test_quoted_filter_argument_with_a_nested_placeholder_does_not_close_early():
     """Regression: a quoted filter arg's own '}' (from a nested {field}) must not be
     mistaken for the placeholder's closing brace and truncate the rest of the chain."""
     template = (
-        '{packet_hash | if_notempty: '
-        '"https://analyzer.example.net/#/packets/{packet_hash}?obs=1620457" '
-        '| shorten_url}'
+        '{packet_hash | if_nonempty: '
+        '"https://analyzer.example.net/#/packets/{packet_hash}?obs=1620457"}'
     )
     assert format_piped_template(template, {"packet_hash": ""}) == ""
-    assert format_piped_template(template, {"packet_hash": "ABCDEF12"}) == (
-        "https://analyzer.example.net/#/packets/ABCDEF12?obs=1620457"
+    assert format_piped_template(template, {"packet_hash": "ABCDEF12"}) == _LONG_LINK
+
+
+@pytest.mark.unit
+def test_shorten_url_uses_the_preresolved_mapping():
+    out = format_piped_template(
+        _LINK_TEMPLATE,
+        {"packet_hash": "ABCDEF12"},
+        shortened={_LONG_LINK: "https://v.gd/abc"},
     )
+    assert out == "https://v.gd/abc"
+
+
+@pytest.mark.unit
+def test_shorten_url_never_calls_the_network_from_the_sync_render():
+    """The render path runs on the event loop; a blocking shortener call here would
+    stall the radio transport for the length of its timeout."""
+    with patch("modules.url_shortener.requests.get") as get, \
+         patch("modules.url_shortener.requests.post") as post:
+        format_piped_template(_LINK_TEMPLATE, {"packet_hash": "ABCDEF12"}, logger=Mock())
+    get.assert_not_called()
+    post.assert_not_called()
+
+
+@pytest.mark.unit
+def test_unresolved_shorten_url_drops_the_clause_and_warns():
+    """A 59-byte URL against a ~158-byte budget would push a path reply into a second
+    transmission, so an unresolved link is dropped rather than sent long."""
+    logger = Mock()
+    out = format_piped_template(_LINK_TEMPLATE, {"packet_hash": "ABCDEF12"}, logger=logger)
+    assert out == ""
+    logger.warning.assert_called_once()
+
+
+@pytest.mark.unit
+def test_template_needs_resolution_only_for_network_filters():
+    assert template_needs_resolution(_LINK_TEMPLATE)
+    assert not template_needs_resolution("{path_distance|prefix_if_nonempty: | Dist: }")
+
+
+@pytest.mark.unit
+@pytest.mark.asyncio
+async def test_resolve_template_async_shortens_the_built_link():
+    cfg = configparser.ConfigParser()
+    cfg.add_section("External_Data")
+    cfg.set("External_Data", "short_url_website", "https://v.gd")
+    resp = MagicMock()
+    resp.ok = True
+    resp.text = "https://v.gd/abc"
+    session = MagicMock()
+    session.get.return_value = resp
+
+    with patch("modules.url_shortener.requests.get", session.get):
+        resolved = await resolve_template_async(
+            _LINK_TEMPLATE, {"packet_hash": "ABCDEF12"}, config=cfg
+        )
+
+    assert resolved == {_LONG_LINK: "https://v.gd/abc"}
+    assert format_piped_template(
+        _LINK_TEMPLATE, {"packet_hash": "ABCDEF12"}, shortened=resolved
+    ) == "https://v.gd/abc"
+
+
+@pytest.mark.unit
+@pytest.mark.asyncio
+async def test_resolve_template_async_skips_a_gated_clause():
+    """hops_min has already suppressed the clause during the collection pass, so no
+    request is made for a link that would never have been sent."""
+    cfg = configparser.ConfigParser()
+    cfg.add_section("External_Data")
+    template = '{d|hops_min:5|if_nonempty:"https://x.example/{d}"|shorten_url}'
+    with patch("modules.url_shortener.requests.get") as get:
+        resolved = await resolve_template_async(
+            template, {"d": "12.4km"}, message=_msg(path="Direct", hops=0), config=cfg
+        )
+    assert resolved == {}
+    get.assert_not_called()
+
+
+@pytest.mark.unit
+@pytest.mark.asyncio
+async def test_resolve_template_async_is_a_noop_without_config():
+    assert await resolve_template_async(_LINK_TEMPLATE, {"packet_hash": "A"}) == {}