diff --git a/CHANGELOG.md b/CHANGELOG.md index ba752cb..3837fcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,11 @@ semantic versioning. setting. The command runs for its text only and transmits nothing itself (`CommandManager.render_command_output`); unknown, disabled, admin-only, timing-out and silent commands expand to nothing rather than airing raw placeholder text. - Bounded by the new `[Bot] scheduled_command_timeout_seconds` (default 30). + Bounded by the new `[Bot] scheduled_command_timeout_seconds` (default 30). Two + non-configurable airtime guards apply: a schedule using `{cmd:...}` must not fire + more often than every 15 minutes (rejected at startup, measured by the tightest gap + so `0,1 * * * *` counts as 60 seconds), and the command's own `cooldown_seconds` is + still enforced. - `{path_distance}` is now available in the path command's `[Path_Command] reply_prefix`, reporting total distance travelled (sender → hops → bot, e.g. `12.4km`) and rendering empty when any node in the chain has no usable coordinates. The prefix now supports the diff --git a/config.ini.example b/config.ini.example index 07d865d..36307e6 100644 --- a/config.ini.example +++ b/config.ini.example @@ -547,7 +547,14 @@ category.funfact = fun # disabled, admin-only, times out, or produces no output — the raw {cmd:...} text is # never put on the air. If expansion leaves the message empty, nothing is sent. # Command output is not re-scanned, so a reply containing {cmd:...} cannot recurse. -# Mind the airtime: a scheduled command still costs a transmission every time it fires. +# +# Airtime guards (both deliberate, neither configurable): +# - A schedule containing {cmd:...} must not fire more often than every 15 minutes. +# An entry that does is rejected at startup with an error and is not scheduled. +# Measured by the tightest gap, so "0,1 * * * *" counts as every 60 seconds. +# Schedules without a command placeholder are unaffected. +# - The command's own cooldown still applies. If it is on cooldown when the schedule +# fires, the placeholder expands to nothing that round. # # Available placeholders for mesh network information: # diff --git a/docs/configuration.md b/docs/configuration.md index c63ef0c..1757e61 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -222,12 +222,19 @@ The trigger is matched against command **names and their keywords**, so `{cmd:we A placeholder expands to **nothing** (and logs a warning) when the command is unknown, disabled in config, admin-only, times out, or returns no output — the literal `{cmd:...}` is never transmitted. If a message is empty after expansion, nothing is sent at all. Command output is not re-scanned, so a reply that happens to contain `{cmd:...}` cannot recurse. -Two limits worth knowing: +#### Airtime guards + +Every firing is a transmission on a shared medium, and a command placeholder makes it easy to write a cron that airs several times an hour. Two guards apply, and neither is configurable: + +- **A 15-minute floor.** A schedule containing `{cmd:...}` may not fire more often than every 15 minutes. An entry that does is **rejected at startup** with an error and is not scheduled at all — it does not silently run at a slower rate. The check measures the *tightest* gap between firings, so `0,1 * * * *` is treated as a 60-second schedule rather than an hourly one. Schedules with no command placeholder are unaffected. +- **The command's own cooldown still applies.** `[_Command] cooldown_seconds` is not bypassed by scheduling. If the command is on cooldown when the schedule fires, the placeholder expands to nothing for that round and logs a warning. + +Other limits worth knowing: - Commands that transmit directly instead of returning text (currently only `announcements`) cannot be rendered and are refused, since running them would broadcast for real. - `[Bot] scheduled_command_timeout_seconds` (default `30`) bounds each render. Network-backed commands like `wx` need the headroom. -**This costs airtime every time it fires.** A `*/5 * * * *` forecast is 288 transmissions a day; pick an interval the mesh can afford. +Even within the floor, mind the cost: a `*/15 * * * *` forecast is 96 transmissions a day. ### Schedule keys (APScheduler cron, not Vixie) diff --git a/docs/service-installation.md b/docs/service-installation.md index 501759d..2fb2cde 100644 --- a/docs/service-installation.md +++ b/docs/service-installation.md @@ -58,6 +58,27 @@ attempt to restart that service while preserving the original failure status. Read the [upgrade guide](upgrade.md) before upgrading an existing installation. +### Optional feature packages + +Two feature sets are not installed by default because most bots do not need them: +the profanity filter (`better-profanity`, `unidecode`) and geocoding extras +(`pycountry`, `us`). A fresh install and a `--upgrade` both prompt for each one. + +For an unattended run, install both without prompting: + +```bash +sudo ./install-service.sh --upgrade --install-extras +``` + +`--install-extras` also works alongside `--update-venv`, which otherwise keeps +whatever is already in the virtual environment and does not prompt: + +```bash +sudo ./install-service.sh -u --update-venv --install-extras +``` + +A failed extras install is non-fatal; the bot installs and starts without them. + ## Manual Installation If you prefer to install manually: diff --git a/docs/upgrade.md b/docs/upgrade.md index 89b7293..57aab6c 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -114,6 +114,9 @@ database migration, copies relative-path databases coherently, rewrites the migr configuration, builds a fresh virtual environment, and restarts a service that was active before the upgrade. +Add `--install-extras` to install the optional profanity-filter and geocoding +packages without being prompted, which is what you want for an unattended upgrade. + Before upgrading, keep a separate backup of your configuration and database. The installer preserves: diff --git a/docs/weather-service.md b/docs/weather-service.md index 0115498..f8d0f95 100644 --- a/docs/weather-service.md +++ b/docs/weather-service.md @@ -111,6 +111,15 @@ Sends forecast to `weather_channel` at configured time: - Sunrise: `weather_alarm = sunrise` - Sunset: `weather_alarm = sunset` +`weather_alarm` fires **once a day**. For more than one forecast a day, or a forecast for a location other than the bot's own position, schedule the `wx` command instead with a [`{cmd:...}` placeholder](configuration.md#broadcasting-a-commands-output-cmd): + +```ini +[Scheduled_Messages] +0 6,12,18 * * * = Public:{cmd:wx Seattle} +``` + +That accepts any cron schedule and any location, and works the same way for `aqi` and other commands. `sunrise`/`sunset` are not expressible as cron, so keep using `weather_alarm` for those. + ### Rain Nowcast (Proactive) Watches your position and posts a heads-up to `rain_channel` (default: `weather_channel`) when precipitation is about to start, using Open-Meteo's 15-minutely forecast — the same engine as the [`rain`/`nowcast` command](command-reference.md#rain-location). diff --git a/modules/command_manager.py b/modules/command_manager.py index 18326bc..393900b 100644 --- a/modules/command_manager.py +++ b/modules/command_manager.py @@ -1920,6 +1920,19 @@ class CommandManager: ) return None + # The command's own cooldown still governs it. A schedule is not a licence to + # run something more often than the operator configured it to run. + allowed, remaining = command.check_cooldown() + if not allowed: + self.logger.warning( + "Scheduled {cmd:...} placeholder: %r is on cooldown for another %.0fs; skipped", + command_name, remaining, + ) + return None + # Recorded before execution, matching execute_commands, so a slow or failing + # render cannot be retried straight past the cooldown. + command.record_execution() + sink: list[str] = [] synthetic = MeshMessage( content=spec, diff --git a/modules/scheduler.py b/modules/scheduler.py index 3943001..1b0c24e 100644 --- a/modules/scheduler.py +++ b/modules/scheduler.py @@ -137,6 +137,18 @@ class MessageScheduler: channel, message, scope = parse_scheduled_message_value(message_info) message = decode_escape_sequences(message) + if self._has_command_placeholders(message): + interval = self._min_fire_interval_seconds(parsed.trigger, tz) + floor = self.MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS + if interval is not None and interval < floor: + self.logger.error( + "Scheduled message %r uses a {cmd:...} placeholder but fires " + "every %.0fs; the minimum is %ds because each firing spends " + "airtime. Not scheduled: %s", + schedule_key, interval, floor, message, + ) + continue + job_id = "schedmsg_" + hashlib.sha256( f"{schedule_key}\0{channel}\0{scope or ''}\0{message}".encode() ).hexdigest()[:24] @@ -495,6 +507,38 @@ class MessageScheduler: # Non-greedy and brace-free inside, matching the placeholder limits elsewhere. _COMMAND_PLACEHOLDER_RE = re.compile(r"\{cmd:([^{}]+)\}") + # Floor on how often a schedule containing {cmd:...} may fire. Every firing is a + # transmission on a shared medium, and a command placeholder makes it trivial to + # write a cron that airs several times an hour. Deliberately not configurable. + MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS = 900 + + @staticmethod + def _min_fire_interval_seconds(trigger, tz, samples: int = 12) -> Optional[float]: + """Smallest gap between consecutive firings of *trigger*, in seconds. + + Sampled rather than derived, so uneven crons are measured by their tightest + gap: ``0,1 * * * *`` is a 60-second schedule, not a half-hourly one. + + Returns None when the trigger has no future firings to compare. + """ + now = datetime.datetime.now(tz) + previous = trigger.get_next_fire_time(None, now) + if previous is None: + return None + + smallest = None + for _ in range(samples): + nxt = trigger.get_next_fire_time( + previous, previous + datetime.timedelta(microseconds=1) + ) + if nxt is None: + break + gap = (nxt - previous).total_seconds() + if gap > 0 and (smallest is None or gap < smallest): + smallest = gap + previous = nxt + return smallest + def _has_command_placeholders(self, message: str) -> bool: return bool(self._COMMAND_PLACEHOLDER_RE.search(message)) diff --git a/tests/unit/test_scheduled_command_placeholder.py b/tests/unit/test_scheduled_command_placeholder.py index eaaf698..b31803b 100644 --- a/tests/unit/test_scheduled_command_placeholder.py +++ b/tests/unit/test_scheduled_command_placeholder.py @@ -42,6 +42,8 @@ def _make_command(*, name, keywords, reply, enabled=True, admin=False, delay=0.0 cmd._derive_config_section_name.return_value = f"{name.title()}_Command" cmd.get_config_value.return_value = enabled cmd.requires_admin_access.return_value = admin + # Off cooldown unless a test says otherwise. + cmd.check_cooldown.return_value = (True, 0.0) async def execute(message): if delay: @@ -270,3 +272,146 @@ class TestRenderWithRealCommand: assert rendered, "a real ping should produce text" mgr.send_dm.assert_not_called() mgr.send_channel_message.assert_not_called() + + +@pytest.mark.unit +class TestCommandCooldownStillApplies: + """A schedule is not a licence to outrun a command's configured cooldown.""" + + @pytest.mark.asyncio + async def test_render_refused_while_on_cooldown(self, mock_bot): + wx = _make_command(name="wx", keywords=["wx"], reply="12C") + wx.check_cooldown.return_value = (False, 42.0) + mgr = _make_manager(mock_bot, {"wx": wx}) + assert await mgr.render_command_output("wx Seattle") is None + + @pytest.mark.asyncio + async def test_render_records_execution_so_cooldown_advances(self, mock_bot): + wx = _make_command(name="wx", keywords=["wx"], reply="12C") + wx.check_cooldown.return_value = (True, 0.0) + mgr = _make_manager(mock_bot, {"wx": wx}) + assert await mgr.render_command_output("wx Seattle") == "12C" + wx.record_execution.assert_called_once() + + @pytest.mark.asyncio + async def test_execution_recorded_even_if_command_then_fails(self, mock_bot): + """Recorded before execute, so a failing render cannot be retried immediately.""" + boom = _make_command(name="boom", keywords=["boom"], reply=None) + boom.check_cooldown.return_value = (True, 0.0) + + async def explode(message): + raise RuntimeError("kaboom") + + boom.execute = explode + mgr = _make_manager(mock_bot, {"boom": boom}) + assert await mgr.render_command_output("boom") is None + boom.record_execution.assert_called_once() + + +@pytest.mark.unit +class TestMinimumIntervalFloor: + """{cmd:...} schedules may not fire more often than every 15 minutes.""" + + @staticmethod + def _interval(cron): + import datetime + + from apscheduler.triggers.cron import CronTrigger + + from modules.scheduler import MessageScheduler + + tz = datetime.timezone.utc + trigger = CronTrigger.from_crontab(cron, timezone=tz) + return MessageScheduler._min_fire_interval_seconds(trigger, tz) + + def test_floor_is_fifteen_minutes(self): + from modules.scheduler import MessageScheduler + + assert MessageScheduler.MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS == 900 + + @pytest.mark.parametrize("cron,expected", [ + ("*/5 * * * *", 300), + ("*/15 * * * *", 900), + ("*/30 * * * *", 1800), + ("0 * * * *", 3600), + ("0 6,12,18 * * *", 21600), + ]) + def test_even_schedules_measured_correctly(self, cron, expected): + assert self._interval(cron) == expected + + def test_uneven_cron_measured_by_its_tightest_gap(self): + """0,1 * * * * is a 60-second schedule, not an hourly one.""" + assert self._interval("0,1 * * * *") == 60 + + def test_daily_schedule_is_well_above_the_floor(self): + from modules.scheduler import MessageScheduler + + assert self._interval("0 8 * * *") > MessageScheduler.MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS + + @pytest.mark.parametrize("cron,allowed", [ + ("* * * * *", False), + ("*/5 * * * *", False), + ("*/14 * * * *", False), + ("0,1 * * * *", False), + ("*/15 * * * *", True), + ("*/30 * * * *", True), + ("0 6,12,18 * * *", True), + ]) + def test_floor_admits_and_rejects_the_right_schedules(self, cron, allowed): + from modules.scheduler import MessageScheduler + + interval = self._interval(cron) + floor = MessageScheduler.MIN_COMMAND_PLACEHOLDER_INTERVAL_SECONDS + assert (interval >= floor) is allowed + + +@pytest.mark.unit +class TestFloorEnforcedDuringSetup: + """The floor has to actually stop the job being scheduled, not just compute a number.""" + + @staticmethod + def _run_setup(entries): + import configparser + from unittest.mock import patch + + from modules.scheduler import MessageScheduler + + config = configparser.ConfigParser() + config["Bot"] = {"timezone": "UTC"} + config["Scheduled_Messages"] = entries + + bot = MagicMock() + bot.config = config + + sched = object.__new__(MessageScheduler) + sched.bot = bot + sched.logger = MagicMock() + sched.scheduled_messages = {} + sched._shutdown_apscheduler_if_running = MagicMock() + sched._setup_device_mode_scheduler_jobs = MagicMock() + sched.setup_interval_advertising = MagicMock() + + fake_scheduler = MagicMock() + with patch("modules.scheduler.BackgroundScheduler", return_value=fake_scheduler): + sched.setup_scheduled_messages() + return sched, fake_scheduler + + def test_too_frequent_command_schedule_is_not_added(self): + sched, apsched = self._run_setup({"*/5 * * * *": "Public:{cmd:wx Seattle}"}) + assert apsched.add_job.call_count == 0 + assert sched.scheduled_messages == {} + assert sched.logger.error.called + + def test_uneven_cron_with_tight_gap_is_not_added(self): + _, apsched = self._run_setup({"0,1 * * * *": "Public:{cmd:wx Seattle}"}) + assert apsched.add_job.call_count == 0 + + def test_schedule_at_the_floor_is_added(self): + sched, apsched = self._run_setup({"*/15 * * * *": "Public:{cmd:wx Seattle}"}) + assert apsched.add_job.call_count == 1 + assert sched.scheduled_messages + + def test_frequent_schedule_without_placeholder_is_unaffected(self): + """The floor applies to command placeholders, not to plain scheduled text.""" + _, apsched = self._run_setup({"*/5 * * * *": "Public:static message"}) + assert apsched.add_job.call_count == 1