mirror of
https://github.com/kuhyx/signal-bot.git
synced 2026-11-11 15:43:49 +01:00
fix: a refused command no longer re-arms its own cooldown
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ed51aaf3eb
commit
a925928f12
@ -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
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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)
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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."""
|
||||
|
||||
|
||||
53
tests/gate_helpers.py
Normal file
53
tests/gate_helpers.py
Normal file
@ -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)
|
||||
@ -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:
|
||||
|
||||
121
tests/test_cooldown_gate.py
Normal file
121
tests/test_cooldown_gate.py
Normal file
@ -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"]
|
||||
@ -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"]
|
||||
|
||||
87
tests/test_dispatcher_gates.py
Normal file
87
tests/test_dispatcher_gates.py
Normal file
@ -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"]
|
||||
@ -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:
|
||||
|
||||
Loading…
Reference in New Issue
Block a user