From c17d50959bbd9d76e72f0dd1fe109cb2e61db87f Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sun, 30 Aug 2026 20:39:14 +0200 Subject: [PATCH] fix: a refused command no longer re-arms its own cooldown Co-Authored-By: Claude Opus 5 (1M context) --- DOCS-changelog.md | 8 +++ signal_bot/bot.py | 2 +- signal_bot/decorators.py | 45 ++++++++++-- signal_bot/dispatcher.py | 8 ++- signal_bot/registry.py | 10 +++ tests/gate_helpers.py | 53 +++++++++++++++ tests/test_animals.py | 9 +-- tests/test_cooldown_gate.py | 121 +++++++++++++++++++++++++++++++++ tests/test_decorators.py | 116 +++++++------------------------ tests/test_dispatcher_gates.py | 87 ++++++++++++++++++++++++ tests/test_popes_command.py | 9 +-- 11 files changed, 359 insertions(+), 109 deletions(-) create mode 100644 tests/gate_helpers.py create mode 100644 tests/test_cooldown_gate.py create mode 100644 tests/test_dispatcher_gates.py diff --git a/DOCS-changelog.md b/DOCS-changelog.md index 37d8f75..4f3ac5b 100644 --- a/DOCS-changelog.md +++ b/DOCS-changelog.md @@ -38,6 +38,14 @@ this repo: describe what changes for them. 0.0.1 shipped a bullet about `/healthz`, an endpoint bound to 127.0.0.1 that nobody in a Signal group can reach, and it was rightly called out. +## 0.1.3 — Soter + +- "POCZEKAJ 1 SEKUND" odmienia się wreszcie po polsku — teraz jest + 1 SEKUNDĘ, 2 SEKUNDY, 5 SEKUND +- odczekanie tego, co bot każe, wreszcie działa: dotąd każda nieudana próba + po cichu przedłużała blokadę, więc kto grzecznie czekał, ten dostawał + odmowę w kółko + ## 0.1.2 — Anicet - `!ja` pokazywał wszystkim tę samą godzinę (12:00) — to był artefakt diff --git a/signal_bot/bot.py b/signal_bot/bot.py index d0fc3dd..d3af5c4 100644 --- a/signal_bot/bot.py +++ b/signal_bot/bot.py @@ -48,7 +48,7 @@ logger = logging.getLogger(__name__) Clock = Callable[[], dt.datetime] -COOLDOWN_MESSAGE = "BEEP BOOP POCZEKAJ {seconds} SEKUND." +COOLDOWN_MESSAGE = "BEEP BOOP POCZEKAJ {seconds} {unit}." def now_local() -> dt.datetime: diff --git a/signal_bot/decorators.py b/signal_bot/decorators.py index e99fd10..49434b4 100644 --- a/signal_bot/decorators.py +++ b/signal_bot/decorators.py @@ -5,17 +5,41 @@ These are the mechanisms salvaged from the unmerged vote and rate-limit branches, detached from the command they were originally written for. They only decide whether a handler may run; recording a successful use is the dispatcher's job, so stacking two gates cannot double-count one invocation. + +A gate that refuses raises ``CommandRefusedError`` rather than returning, which +is what tells the dispatcher no use took place. Returning quietly used to look +identical to a successful run. """ from __future__ import annotations import functools +import logging +import math from collections.abc import Callable -from signal_bot.registry import CommandContext, Handler +from signal_bot.registry import CommandContext, CommandRefusedError, Handler +from signal_bot.stats.text import plural + +logger = logging.getLogger(__name__) Decorator = Callable[[Handler], Handler] +# Padding on the *displayed* wait only, never on the gate itself. A number that +# is a second short sends somebody back for another refusal, which reads as the +# bot lying; a number that is a second long costs nothing. +DISPLAY_MARGIN_SECONDS = 2 + + +def wait_display(remaining: float) -> tuple[int, str]: + """Round a remaining wait up (plus margin) and pick its Polish noun form. + + Three forms, not two: 1 sekundę, 2-4 sekundy, 5+ sekund -- and the rule + reads the last two digits, so 12-14 take the many-form and 22-24 do not. + """ + seconds = math.ceil(remaining) + DISPLAY_MARGIN_SECONDS + return seconds, plural(seconds, "SEKUNDĘ", "SEKUNDY", "SEKUND") + def cooldown(seconds: float, message: str) -> Decorator: """Refuse a command until `seconds` have passed since that user's last use. @@ -29,9 +53,18 @@ def cooldown(seconds: float, message: str) -> Decorator: async def wrapper(ctx: CommandContext) -> None: last = ctx.store.last_used(ctx.group_id, ctx.message.source_uuid, ctx.command_name) if last is not None and ctx.now - last < seconds: - remaining = seconds - (ctx.now - last) - await ctx.reply(message.format(seconds=int(remaining) + 1)) - return + shown, unit = wait_display(seconds - (ctx.now - last)) + # Logged with the number the group was given: when this said + # one thing and the gate did another, nothing recorded either. + logger.info( + "cooldown refused %s for %s: told them %d %s", + ctx.command_name, + ctx.message.source_name, + shown, + unit, + ) + await ctx.reply(message.format(seconds=shown, unit=unit)) + raise CommandRefusedError await handler(ctx) return wrapper @@ -50,7 +83,7 @@ def daily_limit(max_uses: int, message: str) -> Decorator: ) if used >= max_uses: await ctx.reply(message.format(max_uses=max_uses, used=used)) - return + raise CommandRefusedError await handler(ctx) return wrapper @@ -72,7 +105,7 @@ def vote_gated(required: int, window_seconds: float, message: str) -> Decorator: votes = ctx.store.count_votes(ctx.group_id, ctx.command_name, ctx.now - window_seconds) if votes < required: await ctx.reply(message.format(votes=votes, required=required)) - return + raise CommandRefusedError ctx.store.clear_votes(ctx.group_id, ctx.command_name) await handler(ctx) diff --git a/signal_bot/dispatcher.py b/signal_bot/dispatcher.py index 71e9ffc..512ca16 100644 --- a/signal_bot/dispatcher.py +++ b/signal_bot/dispatcher.py @@ -12,7 +12,7 @@ from typing import TYPE_CHECKING from signal_bot.counting import should_count, watched_group from signal_bot.envelope_shape import shape_of from signal_bot.models import parse_envelope -from signal_bot.registry import CommandContext, CommandRegistry +from signal_bot.registry import CommandContext, CommandRefusedError, CommandRegistry from signal_bot.stats.recorder import record_message if TYPE_CHECKING: @@ -101,6 +101,12 @@ class Dispatcher: ) try: await command.handler(ctx) + except CommandRefusedError: + # A gate said no and already answered. Nothing ran, so nothing is + # recorded -- otherwise every "wait N seconds" reply would push the + # cooldown window N seconds further away and the wait never ended. + logger.debug("command %s refused by a gate", command.name) + return except Exception: logger.exception("command %s failed", command.name) return diff --git a/signal_bot/registry.py b/signal_bot/registry.py index 6f6a437..2e030d7 100644 --- a/signal_bot/registry.py +++ b/signal_bot/registry.py @@ -70,6 +70,16 @@ class DuplicateTriggerError(ValueError): """Raised when two commands claim the same trigger word.""" +class CommandRefusedError(Exception): + """A gate declined the command, so nothing happened and nothing is recorded. + + The dispatcher records one use per invocation, and it cannot see inside a + handler. Without this signal a *refused* run looked exactly like a + successful one, which slid the cooldown window forward every time somebody + was told to wait -- so waiting the stated time never helped. + """ + + class CommandRegistry: """Maps trigger words to commands.""" diff --git a/tests/gate_helpers.py b/tests/gate_helpers.py new file mode 100644 index 0000000..96f7200 --- /dev/null +++ b/tests/gate_helpers.py @@ -0,0 +1,53 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""Shared scaffolding for the gate tests, split across two files by the 250-line cap.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +from signal_bot.models import parse_envelope +from signal_bot.registry import CommandContext, CommandRefusedError, Handler +from tests.conftest import WATCHED_GROUP, envelope + +if TYPE_CHECKING: + from signal_bot.config import Config + from signal_bot.store import Store + from signal_bot.transport_fake import FakeTransport + +DAY = "2026-08-29" + + +def gate_context( + store: Store, + transport: FakeTransport, + config: Config, + *, + uuid: str = "u1", + now: float = 100.0, + group: str = WATCHED_GROUP, +) -> CommandContext: + """A context for a harmless "!ping", which every gate test wraps.""" + message = parse_envelope(envelope(text="!ping", sender=(uuid, "Alice"), group_id=group)) + assert message is not None + return CommandContext( + message=message, + group_id=group, + transport=transport, + store=store, + config=config, + command_name="ping", + now=now, + day=DAY, + ) + + +async def refused(gate: Handler, ctx: CommandContext) -> None: + """Await a gated handler that is expected to decline. + + A gate signals a refusal by raising, which is what keeps the dispatcher + from recording a use for a run that never happened. + """ + with pytest.raises(CommandRefusedError): + await gate(ctx) diff --git a/tests/test_animals.py b/tests/test_animals.py index d63cd01..70b0996 100644 --- a/tests/test_animals.py +++ b/tests/test_animals.py @@ -16,10 +16,10 @@ from signal_bot.animals import ( paged_url, register_animal_commands, ) -from signal_bot.decorators import cooldown +from signal_bot.decorators import DISPLAY_MARGIN_SECONDS, cooldown from signal_bot.http_client import ImageFetcher from signal_bot.models import parse_envelope -from signal_bot.registry import CommandContext, CommandRegistry +from signal_bot.registry import CommandContext, CommandRefusedError, CommandRegistry from tests.conftest import WATCHED_GROUP, envelope if TYPE_CHECKING: @@ -114,8 +114,9 @@ async def test_decorator_is_applied_when_given( store.record_use(WATCHED_GROUP, "uuid-alice", "cat", 100.0, "2026-08-29") command = registry.resolve("!cat") assert command is not None - await command.handler(_ctx(transport, store, config, "cat", "!cat")) - assert transport.texts == ["wait 11"] + with pytest.raises(CommandRefusedError): + await command.handler(_ctx(transport, store, config, "cat", "!cat")) + assert transport.texts == [f"wait {10 + DISPLAY_MARGIN_SECONDS}"] def test_custom_alias_table_is_honoured() -> None: diff --git a/tests/test_cooldown_gate.py b/tests/test_cooldown_gate.py new file mode 100644 index 0000000..4e02f42 --- /dev/null +++ b/tests/test_cooldown_gate.py @@ -0,0 +1,121 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The cooldown gate: when it refuses, and what it says when it does.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +from signal_bot.decorators import DISPLAY_MARGIN_SECONDS, cooldown, wait_display +from tests.conftest import SECOND_GROUP, WATCHED_GROUP +from tests.gate_helpers import DAY, gate_context, refused + +if TYPE_CHECKING: + from signal_bot.config import Config + from signal_bot.registry import CommandContext, Handler + from signal_bot.store import Store + from signal_bot.transport_fake import FakeTransport + + +@pytest.fixture +def runs() -> list[str]: + return [] + + +@pytest.fixture +def handler(runs: list[str]) -> Handler: + async def _handler(ctx: CommandContext) -> None: + runs.append(ctx.message.source_uuid) + + return _handler + + +async def test_cooldown_allows_first_use( + store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] +) -> None: + await cooldown(10, "wait {seconds}")(handler)(gate_context(store, transport, config)) + assert runs == ["u1"] + + +async def test_cooldown_blocks_and_reports_remaining( + store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] +) -> None: + store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) + ctx = gate_context(store, transport, config, now=104.0) + await refused(cooldown(10, "wait {seconds}")(handler), ctx) + assert runs == [] + # 6s left, rounded up, plus the deliberate margin. + assert transport.texts == [f"wait {6 + DISPLAY_MARGIN_SECONDS}"] + + +async def test_cooldown_expires( + store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] +) -> None: + store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) + await cooldown(10, "wait {seconds}")(handler)(gate_context(store, transport, config, now=111.0)) + assert runs == ["u1"] + + +async def test_cooldown_is_per_user( + store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] +) -> None: + """One person's cat picture used to silence everybody.""" + store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) + await cooldown(10, "wait {seconds}")(handler)( + gate_context(store, transport, config, uuid="u2", now=101.0) + ) + assert runs == ["u2"] + + +async def test_cooldown_is_per_group( + store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] +) -> None: + """A test-group command must not throttle the production group.""" + store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) + await cooldown(10, "wait {seconds}")(handler)( + gate_context(store, transport, config, now=101.0, group=SECOND_GROUP) + ) + assert runs == ["u1"] + assert transport.texts == [] + + +@pytest.mark.parametrize( + ("shown", "unit"), + [ + (1, "SEKUNDĘ"), + (2, "SEKUNDY"), + (3, "SEKUNDY"), + (4, "SEKUNDY"), + (5, "SEKUND"), + (11, "SEKUND"), + (12, "SEKUND"), + (13, "SEKUND"), + (14, "SEKUND"), + (21, "SEKUND"), + (22, "SEKUNDY"), + (25, "SEKUND"), + (112, "SEKUND"), + ], +) +def test_the_wait_agrees_in_polish(shown: int, unit: str) -> None: + """One SEKUND for every number is what the group noticed first. + + Driven through the display helper rather than the gate: the gate can only + ever show 3 or more, because a live refusal always has some fraction of a + second left and the margin sits on top of that. + """ + assert wait_display(shown - DISPLAY_MARGIN_SECONDS) == (shown, unit) + + +async def test_the_shown_wait_rounds_up_and_carries_the_margin( + store: Store, transport: FakeTransport, config: Config, handler: Handler +) -> None: + """Never below the real wait: a short number sends somebody back refused.""" + store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) + await refused( + cooldown(10, "POCZEKAJ {seconds} {unit}")(handler), + gate_context(store, transport, config, now=103.5), + ) + # 6.5s really left -> 7 rounded up, plus the margin. + assert transport.texts == [f"POCZEKAJ {7 + DISPLAY_MARGIN_SECONDS} SEKUND"] diff --git a/tests/test_decorators.py b/tests/test_decorators.py index 69b7d6a..8470f8f 100644 --- a/tests/test_decorators.py +++ b/tests/test_decorators.py @@ -1,5 +1,5 @@ # Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. -"""The reusable gates, tested against a harmless command.""" +"""The daily-limit and vote gates, tested against a harmless command.""" from __future__ import annotations @@ -9,40 +9,15 @@ from typing import TYPE_CHECKING import pytest from signal_bot.decorators import cooldown, daily_limit, vote_gated -from signal_bot.models import parse_envelope -from signal_bot.registry import CommandContext, Handler -from tests.conftest import SECOND_GROUP, WATCHED_GROUP, envelope +from tests.conftest import SECOND_GROUP, WATCHED_GROUP +from tests.gate_helpers import DAY, gate_context, refused if TYPE_CHECKING: from signal_bot.config import Config + from signal_bot.registry import CommandContext, Handler from signal_bot.store import Store from signal_bot.transport_fake import FakeTransport -DAY = "2026-08-29" - - -def _context( - store: Store, - transport: FakeTransport, - config: Config, - *, - uuid: str = "u1", - now: float = 100.0, - group: str = WATCHED_GROUP, -) -> CommandContext: - message = parse_envelope(envelope(text="!ping", sender=(uuid, "Alice"), group_id=group)) - assert message is not None - return CommandContext( - message=message, - group_id=group, - transport=transport, - store=store, - config=config, - command_name="ping", - now=now, - day=DAY, - ) - @pytest.fixture def runs() -> list[str]: @@ -57,46 +32,10 @@ def handler(runs: list[str]) -> Handler: return _handler -async def test_cooldown_allows_first_use( - store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] -) -> None: - await cooldown(10, "wait {seconds}")(handler)(_context(store, transport, config)) - assert runs == ["u1"] - - -async def test_cooldown_blocks_and_reports_remaining( - store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] -) -> None: - store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) - ctx = _context(store, transport, config, now=104.0) - await cooldown(10, "wait {seconds}")(handler)(ctx) - assert runs == [] - assert transport.texts == ["wait 7"] - - -async def test_cooldown_expires( - store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] -) -> None: - store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) - await cooldown(10, "wait {seconds}")(handler)(_context(store, transport, config, now=111.0)) - assert runs == ["u1"] - - -async def test_cooldown_is_per_user( - store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] -) -> None: - """One person's cat picture used to silence everybody.""" - store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) - await cooldown(10, "wait {seconds}")(handler)( - _context(store, transport, config, uuid="u2", now=101.0) - ) - assert runs == ["u2"] - - async def test_daily_limit_allows_under_the_cap( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: - await daily_limit(1, "no more")(handler)(_context(store, transport, config)) + await daily_limit(1, "no more")(handler)(gate_context(store, transport, config)) assert runs == ["u1"] @@ -104,8 +43,9 @@ async def test_daily_limit_blocks_at_the_cap( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) - await daily_limit(1, "no more, used {used}/{max_uses}")(handler)( - _context(store, transport, config) + await refused( + daily_limit(1, "no more, used {used}/{max_uses}")(handler), + gate_context(store, transport, config), ) assert runs == [] assert transport.texts == ["no more, used 1/1"] @@ -115,7 +55,7 @@ async def test_vote_gate_waits_for_quorum( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: gated = vote_gated(2, 900, "{votes}/{required}")(handler) - await gated(_context(store, transport, config, uuid="u1")) + await refused(gated, gate_context(store, transport, config, uuid="u1")) assert runs == [] assert transport.texts == ["1/2"] @@ -124,8 +64,8 @@ async def test_vote_gate_fires_on_quorum( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: gated = vote_gated(2, 900, "{votes}/{required}")(handler) - await gated(_context(store, transport, config, uuid="u1")) - await gated(_context(store, transport, config, uuid="u2")) + await refused(gated, gate_context(store, transport, config, uuid="u1")) + await gated(gate_context(store, transport, config, uuid="u2")) assert runs == ["u2"] @@ -133,8 +73,8 @@ async def test_repeat_vote_does_not_reach_quorum( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: gated = vote_gated(2, 900, "{votes}/{required}")(handler) - await gated(_context(store, transport, config, uuid="u1")) - await gated(_context(store, transport, config, uuid="u1", now=101.0)) + await refused(gated, gate_context(store, transport, config, uuid="u1")) + await refused(gated, gate_context(store, transport, config, uuid="u1", now=101.0)) assert runs == [] @@ -142,8 +82,8 @@ async def test_votes_expire_outside_the_window( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: gated = vote_gated(2, 60, "{votes}/{required}")(handler) - await gated(_context(store, transport, config, uuid="u1", now=100.0)) - await gated(_context(store, transport, config, uuid="u2", now=1000.0)) + await refused(gated, gate_context(store, transport, config, uuid="u1", now=100.0)) + await refused(gated, gate_context(store, transport, config, uuid="u2", now=1000.0)) assert runs == [] @@ -151,8 +91,8 @@ async def test_vote_tally_resets_after_firing( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: gated = vote_gated(2, 900, "{votes}/{required}")(handler) - await gated(_context(store, transport, config, uuid="u1")) - await gated(_context(store, transport, config, uuid="u2")) + await refused(gated, gate_context(store, transport, config, uuid="u1")) + await gated(gate_context(store, transport, config, uuid="u2")) assert store.count_votes(WATCHED_GROUP, "ping", 0.0) == 0 @@ -160,28 +100,18 @@ async def test_gates_stack( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: stacked = cooldown(10, "wait {seconds}")(daily_limit(2, "no more")(handler)) - await stacked(_context(store, transport, config)) + await stacked(gate_context(store, transport, config)) assert runs == ["u1"] assert replace(config, cooldown_seconds=10).cooldown_seconds == 10 -async def test_cooldown_is_per_group( - store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] -) -> None: - """A test-group command must not throttle the production group.""" - store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) - await cooldown(10, "wait {seconds}")(handler)( - _context(store, transport, config, now=101.0, group=SECOND_GROUP) - ) - assert runs == ["u1"] - assert transport.texts == [] - - async def test_daily_limit_is_per_group( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: store.record_use(WATCHED_GROUP, "u1", "ping", 100.0, DAY) - await daily_limit(1, "no more")(handler)(_context(store, transport, config, group=SECOND_GROUP)) + await daily_limit(1, "no more")(handler)( + gate_context(store, transport, config, group=SECOND_GROUP) + ) assert runs == ["u1"] @@ -190,7 +120,7 @@ async def test_votes_do_not_carry_between_groups( ) -> None: """Quorum in one group must not fire the command in another.""" gated = vote_gated(2, 900, "{votes}/{required}")(handler) - await gated(_context(store, transport, config, uuid="u1")) - await gated(_context(store, transport, config, uuid="u2", group=SECOND_GROUP)) + await refused(gated, gate_context(store, transport, config, uuid="u1")) + await refused(gated, gate_context(store, transport, config, uuid="u2", group=SECOND_GROUP)) assert runs == [] assert transport.texts == ["1/2", "1/2"] diff --git a/tests/test_dispatcher_gates.py b/tests/test_dispatcher_gates.py new file mode 100644 index 0000000..8d50256 --- /dev/null +++ b/tests/test_dispatcher_gates.py @@ -0,0 +1,87 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""What the dispatcher records when a gate refuses -- which is nothing.""" + +from __future__ import annotations + +import datetime as dt +from typing import TYPE_CHECKING + +from signal_bot.decorators import cooldown +from signal_bot.dispatcher import Dispatcher +from signal_bot.registry import CommandContext, CommandRegistry +from tests.conftest import WATCHED_GROUP, envelope + +if TYPE_CHECKING: + from signal_bot.config import Config + from signal_bot.discovery import GroupDirectory + from signal_bot.store import Store + from signal_bot.transport_fake import FakeTransport + +NOW = dt.datetime(2026, 8, 29, 12, 0, tzinfo=dt.UTC) + + +async def test_a_refused_command_records_nothing( + config: Config, directory: GroupDirectory, store: Store, transport: FakeTransport +) -> None: + """A gate's refusal must not slide the cooldown window forward. + + This is what made "BEEP BOOP POCZEKAJ n SEKUND" a lie: the refusal itself + was recorded as a use, so waiting the stated n only bought another refusal. + """ + runs: list[str] = [] + + async def handler(ctx: CommandContext) -> None: + runs.append(ctx.command_name) + + registry = CommandRegistry() + registry.register("cat", ["!cat"], cooldown(10, "wait {seconds}")(handler)) + dispatcher = Dispatcher( + config=config, + directory=directory, + registry=registry, + store=store, + transport=transport, + clock=lambda: NOW, + ) + earlier = NOW.timestamp() - 4.0 + store.record_use(WATCHED_GROUP, "uuid-alice", "cat", earlier, NOW.date().isoformat()) + + await dispatcher.handle(envelope(text="!cat")) + + assert runs == [] + assert store.last_used(WATCHED_GROUP, "uuid-alice", "cat") == earlier + # The seeded run is the only one counted; a refusal must not eat an + # allowance either, which is the same bug in daily_limit. + assert store.uses_today(WATCHED_GROUP, "uuid-alice", "cat", NOW.date().isoformat()) == 1 + + +async def test_waiting_the_stated_time_lets_the_command_run( + config: Config, directory: GroupDirectory, store: Store, transport: FakeTransport +) -> None: + """The user's own loop: obey the reply, retry, and it must work.""" + runs: list[str] = [] + clock = [NOW] + + async def handler(ctx: CommandContext) -> None: + runs.append(ctx.command_name) + + registry = CommandRegistry() + registry.register("cat", ["!cat"], cooldown(10, "{seconds}")(handler)) + dispatcher = Dispatcher( + config=config, + directory=directory, + registry=registry, + store=store, + transport=transport, + clock=lambda: clock[0], + ) + + await dispatcher.handle(envelope(text="!cat")) + clock[0] = NOW + dt.timedelta(seconds=3) + await dispatcher.handle(envelope(text="!cat")) + assert runs == ["cat"] + + waited = int(transport.texts[-1]) + clock[0] += dt.timedelta(seconds=waited) + await dispatcher.handle(envelope(text="!cat")) + assert runs == ["cat", "cat"] diff --git a/tests/test_popes_command.py b/tests/test_popes_command.py index c78bf9c..44d8b08 100644 --- a/tests/test_popes_command.py +++ b/tests/test_popes_command.py @@ -10,7 +10,7 @@ from typing import TYPE_CHECKING import httpx import pytest -from signal_bot.decorators import cooldown +from signal_bot.decorators import DISPLAY_MARGIN_SECONDS, cooldown from signal_bot.http_client import ImageFetcher from signal_bot.models import parse_envelope from signal_bot.popes_command import ( @@ -21,7 +21,7 @@ from signal_bot.popes_command import ( register_pope_command, ) from signal_bot.portraits import PortraitCache -from signal_bot.registry import CommandContext, CommandRegistry +from signal_bot.registry import CommandContext, CommandRefusedError, CommandRegistry from signal_bot.versioning import POPES from tests.conftest import WATCHED_GROUP, envelope @@ -109,9 +109,10 @@ async def test_the_cooldown_decorator_applies( command = registry.resolve("!pope") assert command is not None - await command.handler(_ctx(transport, store, config, "!pope")) + with pytest.raises(CommandRefusedError): + await command.handler(_ctx(transport, store, config, "!pope")) - assert transport.texts == ["WAIT 11"] + assert transport.texts == [f"WAIT {10 + DISPLAY_MARGIN_SECONDS}"] def test_the_caption_names_the_pope_in_both_languages() -> None: