From c64c2d374a3cf83bd4c66ef3d93189ec87a61936 Mon Sep 17 00:00:00 2001 From: CaliBrain Date: Sun, 14 Jun 2026 01:40:09 -0400 Subject: [PATCH] Make IRC less spammy and require a bot name for conversatons (#1065) First step towards fixing the friction created by shelfmark in #997 --- docs/environment-variables.md | 5 +- shelfmark/release_sources/irc/settings.py | 6 +- shelfmark/release_sources/irc/source.py | 79 +++++++++++++++-- tests/irc/test_source.py | 103 +++++++++++++++++++++- 4 files changed, 180 insertions(+), 13 deletions(-) diff --git a/docs/environment-variables.md b/docs/environment-variables.md index d3206040..54cf04da 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -1293,7 +1293,7 @@ Delay between requests in seconds to avoid rate limiting (0-10). | `IRC_USE_TLS` | Enable TLS/SSL encryption for the IRC connection. Disable for servers that don't support TLS. | boolean | `true` | | `IRC_CHANNEL` | Channel name without the # prefix | string | _none_ | | `IRC_NICK` | Your IRC nickname (required). Must be unique on the IRC network. | string | _none_ | -| `IRC_SEARCH_BOT` | The search bot to query for results | string | _none_ | +| `IRC_SEARCH_BOT` | The search bot to address queries to (required). | string | _none_ | | `IRC_CACHE_TTL` | How long to keep cached search results before they expire. | string (choice) | `2592000` |
@@ -1351,10 +1351,11 @@ Your IRC nickname (required). Must be unique on the IRC network. **Search bot** -The search bot to query for results +The search bot to address queries to (required). Searches are sent as "@ ". Without it, queries would be posted unaddressed to the channel. - **Type:** string - **Default:** _none_ +- **Required:** Yes #### `IRC_CACHE_TTL` diff --git a/shelfmark/release_sources/irc/settings.py b/shelfmark/release_sources/irc/settings.py index 2203c7b6..71ca354a 100644 --- a/shelfmark/release_sources/irc/settings.py +++ b/shelfmark/release_sources/irc/settings.py @@ -88,7 +88,11 @@ def irc_settings() -> list[SettingsField]: key="IRC_SEARCH_BOT", label="Search bot", placeholder="e.g. search", - description="The search bot to query for results", + description=( + "The search bot to address queries to (required). Searches are sent as " + '"@ ".' + ), + required=True, env_supported=True, ), HeadingField( diff --git a/shelfmark/release_sources/irc/source.py b/shelfmark/release_sources/irc/source.py index 5cac0deb..d3b20575 100644 --- a/shelfmark/release_sources/irc/source.py +++ b/shelfmark/release_sources/irc/source.py @@ -88,6 +88,13 @@ def _emit_status(message: str, phase: str = "searching") -> None: MIN_SEARCH_INTERVAL = 15.0 _last_search_time: float = 0 +# Per-query cooldown: never re-post an identical search to the channel within this +# window, even when a user hits "Refresh" or retries a book that returned nothing. +# This stops retry loops from flooding the channel with the same message over and over. +# One day: results don't change minute-to-minute, so there's no reason to re-ask sooner. +QUERY_COOLDOWN_SECONDS = 24 * 60 * 60 # 1 day +_recent_query_times: dict[str, float] = {} + def _enforce_rate_limit() -> None: """Ensure minimum time between searches.""" @@ -102,6 +109,31 @@ def _enforce_rate_limit() -> None: _last_search_time = time.time() +def _query_cooldown_key(channel: str, query: str) -> str: + """Build a normalized key identifying a search posted to a channel.""" + return f"{channel.casefold()}:{query.casefold().strip()}" + + +def _query_on_cooldown(key: str) -> bool: + """Return True if an identical query was posted to the channel recently.""" + last = _recent_query_times.get(key) + if last is None: + return False + return (time.time() - last) < QUERY_COOLDOWN_SECONDS + + +def _record_query_sent(key: str) -> None: + """Record that a query was just posted to the channel (for cooldown).""" + now = time.time() + _recent_query_times[key] = now + # Opportunistically prune stale entries so the dict can't grow unbounded. + stale = [ + k for k, sent_at in _recent_query_times.items() if now - sent_at > QUERY_COOLDOWN_SECONDS + ] + for k in stale: + _recent_query_times.pop(k, None) + + @register_source("irc") class IRCReleaseSource(ReleaseSource): """Search IRC channels for ebook and audiobook releases.""" @@ -117,11 +149,16 @@ class IRCReleaseSource(ReleaseSource): self._online_servers: set[str] | None = None def is_available(self) -> bool: - """Check if IRC is configured (server, channel, and nick are set).""" + """Check if IRC is configured (server, channel, nick, and search bot are set). + + The search bot is required: without it we would post bare queries straight + to the channel, which reads as spam and gets the nick banned. + """ server = _config_text("IRC_SERVER") channel = _config_text("IRC_CHANNEL") nick = _config_text("IRC_NICK") - return bool(server and channel and nick) + search_bot = _config_text("IRC_SEARCH_BOT") + return bool(server and channel and nick and search_bot) def get_column_config(self) -> ReleaseColumnConfig: """Configure UI columns for IRC results.""" @@ -193,11 +230,6 @@ class IRCReleaseSource(ReleaseSource): logger.warning("No search query could be built") return [] - logger.info("IRC search: %s", query) - - # Enforce rate limit - _enforce_rate_limit() - # Get IRC settings server = _config_text("IRC_SERVER") port = _config_port("IRC_PORT", 6697) @@ -206,6 +238,34 @@ class IRCReleaseSource(ReleaseSource): nick = _config_text("IRC_NICK") search_bot = _config_text("IRC_SEARCH_BOT") + # Never post an unaddressed query to the channel. A bare book title looks like + # spam to everyone else in the channel and gets the nick banned. Searches must + # be addressed to a search bot ("@ "). + if not search_bot: + logger.warning( + "IRC search bot not configured; refusing to post unaddressed query to channel" + ) + _emit_status("IRC search bot not configured", phase="error") + return [] + + # Don't re-post an identical query to the channel within the cooldown window, + # even on refresh. This is what stops a frustrated user (or a retry loop) from + # spamming the same title over and over. Fall back to whatever is cached. + cooldown_key = _query_cooldown_key(channel, query) + if _query_on_cooldown(cooldown_key): + logger.info("IRC query on cooldown, not re-posting to channel: %s", query) + _emit_status("Search sent recently — showing latest results", phase="complete") + cached = get_cached_results(book.provider, book.provider_id, content_type=content_type) + if cached: + self._online_servers = set(cached.get("online_servers", [])) + return cached["releases"] + return [] + + logger.info("IRC search: %s", query) + + # Enforce rate limit + _enforce_rate_limit() + client = None try: # Get or reuse IRC connection @@ -221,9 +281,10 @@ class IRCReleaseSource(ReleaseSource): # Capture online servers (elevated users in channel) self._online_servers = client.online_servers - # Send search request - search_msg = f"@{search_bot} {query}" if search_bot else query + # Send search request (always addressed to the search bot, never bare) + search_msg = f"@{search_bot} {query}" client.send_message(f"#{channel}", search_msg) + _record_query_sent(cooldown_key) # Wait for results DCC - this is the long wait _emit_status(f"Connected to #{channel} - Waiting for results...", phase="searching") diff --git a/tests/irc/test_source.py b/tests/irc/test_source.py index 2184256b..80fa1bf1 100644 --- a/tests/irc/test_source.py +++ b/tests/irc/test_source.py @@ -84,11 +84,27 @@ def test_search_no_dcc_offer_releases_connection_and_caches_empty_result(monkeyp self.channel = channel self.message = message - def wait_for_dcc(self, *, timeout: float, result_type: bool) -> None: + def wait_for_dcc( + self, *, timeout: float, result_type: bool, expected_senders: object = None + ) -> None: return None client = FakeClient() + # A search bot is required; make config report one so the channel send path runs. + monkeypatch.setattr( + irc_source, + "_config_text", + lambda key: { + "IRC_SERVER": "irc.example.net", + "IRC_CHANNEL": "ebooks", + "IRC_NICK": "tester", + "IRC_SEARCH_BOT": "search", + }.get(key, ""), + ) + # Ensure no leftover cooldown entry from a previous test blocks the send. + irc_source._recent_query_times.clear() + monkeypatch.setattr(source, "is_available", lambda: True) monkeypatch.setattr(irc_source, "_enforce_rate_limit", lambda: None) monkeypatch.setattr(irc_source, "_emit_status", lambda *_args, **_kwargs: None) @@ -137,3 +153,88 @@ def test_search_no_dcc_offer_releases_connection_and_caches_empty_result(monkeyp "online_servers": ["AudioBot"], } ] + + +def test_search_without_search_bot_never_posts_to_channel(monkeypatch): + """A bare (unaddressed) query must never reach the channel; refuse to connect.""" + import shelfmark.release_sources.irc.source as irc_source + + source = IRCReleaseSource() + + # Force is_available True so we exercise the in-search guard (defense in depth). + monkeypatch.setattr(source, "is_available", lambda: True) + monkeypatch.setattr(irc_source, "_emit_status", lambda *_args, **_kwargs: None) + monkeypatch.setattr(irc_source, "_enforce_rate_limit", lambda: None) + monkeypatch.setattr( + "shelfmark.release_sources.irc.cache.get_cached_results", + lambda provider, provider_id, *, content_type: None, + ) + monkeypatch.setattr( + irc_source, + "_config_text", + lambda key: { + "IRC_SERVER": "irc.example.net", + "IRC_CHANNEL": "ebooks", + "IRC_NICK": "tester", + "IRC_SEARCH_BOT": "", # not configured + }.get(key, ""), + ) + monkeypatch.setattr( + "shelfmark.release_sources.irc.connection_manager.connection_manager.get_connection", + lambda **_kwargs: (_ for _ in ()).throw( + AssertionError("must not connect/post without a search bot") + ), + ) + + book = BookMetadata(provider="hardcover", provider_id="nobot", title="No Bot") + plan = SimpleNamespace(primary_query="No Bot") + + assert source.search(book, plan) == [] + + +def test_search_on_cooldown_returns_cache_without_reposting(monkeypatch): + """An identical query within the cooldown window must not be re-posted to the channel.""" + import shelfmark.release_sources.irc.source as irc_source + + source = IRCReleaseSource() + cached_release = Release(source="irc", source_id="cached-line", title="Cached Result") + + monkeypatch.setattr(source, "is_available", lambda: True) + monkeypatch.setattr(irc_source, "_emit_status", lambda *_args, **_kwargs: None) + monkeypatch.setattr(irc_source, "_enforce_rate_limit", lambda: None) + monkeypatch.setattr( + irc_source, + "_config_text", + lambda key: { + "IRC_SERVER": "irc.example.net", + "IRC_CHANNEL": "ebooks", + "IRC_NICK": "tester", + "IRC_SEARCH_BOT": "search", + }.get(key, ""), + ) + monkeypatch.setattr( + "shelfmark.release_sources.irc.cache.get_cached_results", + lambda provider, provider_id, *, content_type: { + "releases": [cached_release], + "online_servers": ["AudioBot"], + }, + ) + monkeypatch.setattr( + "shelfmark.release_sources.irc.connection_manager.connection_manager.get_connection", + lambda **_kwargs: (_ for _ in ()).throw( + AssertionError("cooldown should skip IRC connection") + ), + ) + + # Pretend the same query was just posted to the channel. + irc_source._recent_query_times.clear() + irc_source._record_query_sent(irc_source._query_cooldown_key("ebooks", "Cooldown Book")) + + book = BookMetadata(provider="hardcover", provider_id="cd", title="Cooldown Book") + plan = SimpleNamespace(primary_query="Cooldown Book") + + # expand_search=True bypasses the normal top-level cache, forcing the cooldown path. + releases = source.search(book, plan, expand_search=True) + + assert releases == [cached_release] + assert source._online_servers == {"AudioBot"}