Thorsten [Sat, 8 Aug 2026 09:58:51 +0000 (11:58 +0200)]
Fix NATS silently dropping messages whose routing key has an empty token
bot.py joins shlex tokens with '.' and keeps any punctuation the body had,
so a message ending in a full stop ("<url> ... ich will das.") yields a
routing key ending in '.' - an empty trailing token. AMQP's topic exchange
routed that to '#' fine, but nats-server accepts the PUB without any error
while "classifier.>" never matches it, so the message vanished with nothing
logged on either side. Leading dots and a typed "..." (its own token) fail
the same way.
Collapse runs of dots and trim the ends in _sanitize_subject(), with a
fallback token for a key that reduces to nothing. Empty tokens carry no
information, and the binding keys plugins use still match the trimmed form.
Symptom that surfaced it: a posted YouTube URL followed by a sentence went
unanswered - and with URLResolver deliberately skipping youtube URLs, no
fallback title appeared either.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Thorsten [Wed, 5 Aug 2026 15:35:58 +0000 (17:35 +0200)]
Fix silently missing titles for pages with a late <title>
fetch_page() only ever read the first 8 KB of a response, so any page
whose <title> sits beyond that (github: byte 24571) produced no title at
all -- the log showed "fetching page ..." and then nothing, and the
plugin published an empty action. Read in chunks until </title> shows
up instead, capped at 512 KB, and skip the body entirely for non-text
responses.
Also along that path:
- decode with errors='replace'; the read boundary can land mid-character
- extract_title() passed user_agent=None, and requests treats a None
header value as "drop this header", so every non-YouTube fetch went
out with no User-Agent at all
- the non-text branch returned the tuple (1, content-type), which is
truthy, so URLResolver called .strip() on it and raised AttributeError
- log a warning on the remaining no-title paths, which were silent
- return None instead of Action(msg="") when nothing resolved
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fix NATS classifier publish dropping messages with quoted multi-word args
A shlex-quoted plugin argument (e.g. choose 'multi word option') produces
a routing key containing a literal space. AMQP's topic exchange tolerates
that, but NATS's whitespace-delimited wire protocol does not - the PUB
frame gets corrupted and the server silently drops the connection before
the message reaches any subscriber. Sanitize whitespace out of the
subject before publishing; the JSON payload plugins actually read from is
untouched.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix NATS provisioning for hosts where the bot user lacks passwordless sudo
nats_server.yml's tasks use become: true, which worked fine on munin (the
"pi" user has passwordless sudo) but hung waiting for a sudo password
prompt on aero2k.de, where "jabberbot" doesn't - discovered deploying NATS
there for the first time.
Moved "provision local nats server" into deploy.yml's first play, which
already connects as root directly (no become needed at all), instead of
the second play that connects as botuser. Also switched from a raw
include_tasks path to include_role(tasks_from: nats_server), since a bare
include_tasks doesn't pull in the role's defaults/main.yml (nats_version
was undefined otherwise) while include_role does regardless of which
tasks file is requested.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replace IsDown's third-party scraping with a direct reachability check
isup.me now serves a Cloudflare bot-challenge page to non-browser clients
(confirmed live), so scraping it can never work again regardless of the
earlier words[0]/words[1] fix. Checked two alternatives from munin:
downforeveryoneorjustme.com is also Cloudflare-gated, and
isitdownrightnow.com, while not gated, only exposes status via a CSS class
in a ping-history table with ambiguous boilerplate text ("is up"/"is down"
appear on every page regardless of actual status) - too fragile to scrape
reliably either.
Instead of depending on any third party's HTML, do the obvious thing: a
DNS lookup to distinguish "doesn't exist" from "unreachable", then a
direct request to the target with a timeout. Simpler, more reliable, and
checks the actual target instead of trusting a third party's cache.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix IsDown off-by-one bug; extend the smoke test to full plugin coverage
IsDown.parse_body() read words[0] (the command word "isdown" itself) as
the target URL instead of words[1] (the actual argument) - every other
plugin in the codebase treats words[0] as the command name and words[1:]
as arguments. This meant "bot: isdown <site>" always silently checked a
nonsense hostname and never replied, caught live via the extended smoke
test. Fixed, with regression test coverage mocking requests.get.
Extends tests/muc_smoke_check.py from the "complex edges" subset to
~40 checks covering nearly every registered plugin: all the deterministic
no-network ones (Pray, BOFH, Klammer, Terminate, Unicode, Slap, 8ball,
XChoose, Coin, Choose, Morse, Uptime, Info, SecurityTracker, URLBlacklist,
MentalDeficits), the network-dependent ones with generous timeouts
(Wikipedia, DuckDuckGo, Consumables/giphy, Translator, IsDown, Youtube,
URLResolver), and dedicated multi-step workflows for the stateful plugins
(Voting - ending the pre-seeded joke election and confirming a fresh vote
lands in the closing tally; VotePoll - full poll/vote/endpoll cycle with
timestamp-uniqued options and a check that skips rather than interrupts
someone else's already-active poll; Recorder - record-then-rejoin-under-
the-target-nick delivery). Deliberately excludes DidYouKnow (permanent
disk growth) and Searx (single hardcoded backend, worst-case ~17min retry
loop before failing).
Also fixes two harness bugs found while running this for real: leave_muc()
isn't a coroutine in the installed slixmpp version (was being incorrectly
awaited, crashing the Recorder workflow before it could finish), and each
stateful workflow is now individually try/excepted so one crashing doesn't
prevent the others from running and reporting.
Corrects an earlier, wrong claim (from before this was actually run live):
Recorder's delivery-on-join was assumed broken because its "userjoin.*"
binding-key entry can never match a real dotted-domain routing key - true,
but irrelevant, since Recorder also subscribes via Worker.CATCH_ALL and
receives the message regardless. Confirmed working end to end.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix cross-bot infinite reply loop: honor the existing other_bots config
muc_message() only ever ignored the bot's own nick. With multiple sibling
bot instances (urlbug, pibug, urlbrot) in the same room, each correctly
ignores itself but happily reacts to the others' replies - discovered
live when a smoke-test message containing a CVE id caused
SecurityTracker's URL reply to be picked up by the other bot's
URLResolver, whose title-scrape reply contained the same CVE id, causing
SecurityTracker to fire again, forever, across both bots, hammering
security-tracker.debian.org indefinitely until manually restarted.
persistent_config.ini.spec already had an `other_bots` key seemingly
designed for exactly this, but nothing in the codebase ever read it. Wired
it into muc_message()'s existing self-nick check. Populating the actual
other_bots list per host is a separate, per-deployment config change (not
committed here, since persistent_config.ini is gitignored per-host state).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix leading-'#' zero-match in routing.matches(); extend the smoke test
routing.matches() (used by the NATS classifier dispatch and the AMQP test
harness alike) only fixed up the trailing zero-match case ("nick.dice.#"
matching bare "nick.dice"). Real AMQP topic exchanges let '#' match zero
words at any position, so a binding key like "#.doctor.#" must also match
"doctor" as the literal first word with nothing before it - that shape
was silently broken under the NATS backend (confirmed live: Doctor and
DidYouKnow, the only two plugins using a bare leading '#', never fired for
that phrasing on munin). Fixed with a symmetric leading-anchor rewrite,
plus unit test coverage. A narrower gap remains for a '#' with zero words
strictly between two literals (documented in routing.py) - only affects
Selfreaction's degenerate "me.<nick>" phrasing, not worth a fully general
regex rewrite for.
Also extends muc_smoke_check.py with checks for the trickiest binding-key
shapes in the codebase: Doctor (leading '#', the bug above), Selfreaction
(mid-pattern '#', the shape with no NATS-subject equivalent at all), and
TeaTimer (a two-phase check - immediate confirmation, then the scheduled
event firing on its own via the JetStream-durable action_processing path,
with no new message sent to prompt it). Confirmed live: after this fix,
Doctor passes on munin (NATS) too, matching aero2k.de (AMQP) exactly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bind local NATS server to 127.0.0.1 instead of all interfaces
nats-server defaults to binding 0.0.0.0, and the systemd unit didn't
override that - meaning any host that can reach munin's port 4222 could
publish/subscribe to the bot's classifier exchange or JetStream streams
with zero authentication configured. It's only ever used by processes on
the same host (nats_uri=localhost), so restrict it to loopback.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Connects to a real room as a separate test account, addresses the bot by
nick with a few deterministic commands (ping/version/dice), and checks the
responses match expected patterns - a black-box way to confirm a deployed
instance is actually working end to end, not just that its systemd units
are active. Not part of the pytest suite; needs a real XMPP server/room/bot.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix NATS reconnect handling: retry forever instead of giving up after ~2min
nats-py's default max_reconnect_attempts (60 * 2s) meant a long-enough NATS
outage would leave worker.py's per-plugin consumer threads permanently
stuck with no recovery path - unlike the AsyncBroker path used by
action_worker.py, they had no closed_cb to notice and react to a permanent
disconnect. Setting max_reconnect_attempts=-1 makes nats-py retry
indefinitely instead; confirmed via a local test that a plugin thread
survives a NATS restart and resumes receiving messages afterward without
any restart of its own. Also bumps connect_timeout from nats-py's 2s
default to 10s, since munin (a Pi Zero-class host) observed transient
connect timeouts when ~40 plugin threads dialed in simultaneously at
startup under the tighter default.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add NATS/JetStream as an alternative broker to RabbitMQ
RabbitMQ's Erlang VM baseline costs ~300-500MB RES regardless of actual
traffic, which is wasteful for a small hobby-scale chatbot. Introduces a
runtime toggle (broker_backend = amqp | nats in local_config.ini) so a
deployment can opt into a NATS+JetStream backend instead, while defaulting
to amqp so existing deployments are unaffected.
- New common/broker.py abstraction (SyncBroker/AsyncBroker) with
broker_amqp.py (thin, behavior-preserving wrapper around the existing
pika calls) and broker_nats.py backends.
- Binding-key wildcard matching (some patterns, e.g. fun.py's mid-pattern
"#", have no native NATS subject-algebra equivalent) is handled by
subscribing broadly and filtering in-process via common/routing.py,
lifted from the AMQP-routing simulator already used in integration tests.
- action_processing and plugin_registry go through JetStream for
durability/late-subscriber buffering, matching today's AMQP guarantees.
- Ansible role: broker_backend/nats_uri toggle, optional local NATS server
provisioning (nats_local), wait-for-broker.sh picked conditionally.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fix Choose plugin crash when mampfdb.json is missing
open() raised an uncaught FileNotFoundError at class-definition time,
which is never caught by Worker.callback and crashed the whole worker
process on any deployment that hadn't manually placed the file (e.g.
munin, a fresh clone). Also ship mampfdb.json itself so the food-choice
feature works instead of silently falling back to an empty dict.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add XEP-0410 MUC self-ping to auto-rejoin after silent eviction
A c2s reconnect re-fires session_start and rejoins the rooms, but a
silent MUC eviction (e.g. an s2s flap when the account server restarts)
never touches the c2s stream: the bot stays online and JID-pingable yet
quietly drops out of the room with no way to notice.
Periodically ping our own occupant JID and rejoin on not-acceptable /
item-not-found; treat every other error (and timeouts) as inconclusive
so a flaky s2s link can't cause a rejoin storm.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>