diff --git a/.env.sample b/.env.sample index c4f62b3..9fd2507 100644 --- a/.env.sample +++ b/.env.sample @@ -4,17 +4,19 @@ # REQUIRED. The account signal-cli is linked to, in +E.164 form. PHONE_NUMBER= -# REQUIRED. The group the bot listens to. +# REQUIRED. The groups the bot listens to, comma-separated. One entry is +# fine; several let a test group and the production group share one process. # # A linked Signal device receives every direct message and every group on the -# account. This value is the ONLY thing scoping the bot to one conversation, +# account. This value is the ONLY thing scoping the bot to its conversations, # which is why it has no default and startup fails without it. -# Find it with: curl -s localhost:9922/v1/groups/$PHONE_NUMBER | jq -GROUP_ID= - -# REQUIRED. Where replies and the daily report are posted. Set it to the same -# value as GROUP_ID to reply in the group that triggered the command. -GROUP_ID_SEND= +# Find the ids with: curl -s localhost:9922/v1/groups/$PHONE_NUMBER | jq +# +# Replies, rate limits and the daily tally are all per group: a command is +# answered in the group that asked and nowhere else, so there is no separate +# send target to configure. GROUP_ID is still accepted as the one-group +# spelling; GROUP_ID_SEND has been removed and now fails startup if set. +GROUP_IDS= # Optional. The signal-cli-rest-api container. Point this at the fake server # to run entirely offline. Default: http://localhost:9922 diff --git a/DOCS-signal-envelopes.md b/DOCS-signal-envelopes.md index 819e9e2..c93ff91 100644 --- a/DOCS-signal-envelopes.md +++ b/DOCS-signal-envelopes.md @@ -68,7 +68,7 @@ were counted as messages. Four consecutive commits (`f1931b8`, `b325126`, ## No `groupInfo` means a direct message A frame without `groupInfo` arrived in a one-to-one conversation. A linked -device receives every one of those on the account, so `counting.is_watched_group` +device receives every one of those on the account, so `counting.watched_group` rejects them explicitly rather than relying on a falsy comparison. See the `scoping` scenario in `fake_signal/scenarios/`. diff --git a/README.md b/README.md index 961b498..73b15af 100644 --- a/README.md +++ b/README.md @@ -8,10 +8,11 @@ Runs as a **linked device** on an existing Signal account via ## Scoping, first A linked device receives **every direct message and every group** on the -account it is linked to. `GROUP_ID` is the only thing that keeps this bot -inside one conversation, so: +account it is linked to. `GROUP_IDS` is the only thing that keeps this bot +inside its own conversations, so: -- the bot **refuses to start** without `GROUP_ID` and `GROUP_ID_SEND`; +- the bot **refuses to start** without `GROUP_IDS` (`GROUP_ID` is still + accepted as the one-group spelling); - direct messages are dropped explicitly, never by a falsy check; - the `scoping` scenario asserts that DMs and other groups produce **zero** outbound messages, and `scripts/e2e.sh` fails the build if that stops holding. @@ -135,7 +136,7 @@ journalctl --user -u signal-bot.service -f `Restart=always` brings it back from a crash; lingering (checked by the installer, because without it the promise is false) starts it at boot with nobody logged in. `StartLimitBurst=5` stops a config error — a missing -`GROUP_ID`, a taken port — from hiding inside a permanent restart loop. +`GROUP_IDS`, a taken port — from hiding inside a permanent restart loop. The health probe binds `127.0.0.1:8137` (`BIND_PORT`). Not 8000: that port attracts unrelated dev servers, and 8137 sits below the ephemeral range so @@ -155,7 +156,7 @@ cannot bind 127.0.0.1:8000 -- held by pid 82385: /usr/bin/python -m ... Starts the container, opens the QR code, waits for the link, then picks a group. **Caveat:** a linked device rides your account — it sends *as you* and -receives every DM and group you are in. `GROUP_ID` is the only thing scoping +receives every DM and group you are in. `GROUP_IDS` is the only thing scoping it, which is why that scoping is a tested, CI-enforced invariant rather than a convention. @@ -189,12 +190,32 @@ window across everybody. once N distinct people ask, within a window). Both are generic and tested; attach them to any command. +## Watching more than one group + +`GROUP_IDS` takes a comma-separated list, so one process can serve a test +group and the production group at once: + +```sh +GROUP_IDS=group.,group. +``` + +Everything downstream is keyed by group. A command is answered **in the group +that asked and nowhere else**; cooldowns, daily limits, votes and the tally +are per group, so a `!cat` in the test group neither throttles nor counts +against the production one. There is no reply-target setting to get wrong — +`GROUP_ID_SEND` was removed, and setting it now fails startup rather than +quietly posting the day's report into one group's chat. + +The `per_group_isolation` scenario is the gate: one person asks in both +groups inside a single cooldown window and must get two images, one in each. + ## Daily tally -Every non-sticker, non-reaction message in the watched group increments its -sender's count. At `REPORT_HOUR:REPORT_MINUTE` (default 21:37) the tally is -posted to `GROUP_ID_SEND` and reset. Counts live in SQLite and survive a -restart. +Every non-sticker, non-reaction message in a watched group increments its +sender's count **for that group**. At `REPORT_HOUR:REPORT_MINUTE` (default +21:37) each group is posted its own tally, in its own chat, and reset. A group +that refuses the report keeps its counts and does not cost the others theirs. +Counts live in SQLite and survive a restart. ## Develop diff --git a/SECURITY.md b/SECURITY.md index c952786..d536539 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -6,12 +6,15 @@ This bot runs as a **linked device** on an existing Signal account. That means it can read every direct message and every group on that account, and anything it sends is sent **as the account owner**. It is not a separate identity. -`GROUP_ID` is the only thing that narrows this. The bot therefore: +`GROUP_IDS` is the only thing that narrows this. The bot therefore: -- refuses to start when `GROUP_ID` or `GROUP_ID_SEND` is missing or blank, - rather than defaulting to an empty string that matches nothing predictably; +- refuses to start when `GROUP_IDS` is missing or blank, rather than + defaulting to an empty string that matches nothing predictably; - drops messages with no `groupInfo` (direct messages) explicitly, in - `counting.is_watched_group`, rather than relying on a falsy comparison; + `counting.watched_group`, rather than relying on a falsy comparison; +- resolves a message's group **once**, and uses that one answer as the reply + address and the key for all per-group state, so the scoping gate and the + reply router cannot disagree and answer in the wrong group; - ships a `scoping` scenario asserting that direct messages and other groups produce zero outbound messages, run by `scripts/e2e.sh` in CI. diff --git a/fake_signal/cli.py b/fake_signal/cli.py index 0bd6a8f..2d8c0a6 100644 --- a/fake_signal/cli.py +++ b/fake_signal/cli.py @@ -24,9 +24,31 @@ def _emit(line: str) -> None: sys.stdout.write(f"{line}\n") -def _groups(args: argparse.Namespace) -> dict[str, str]: - """Map the scenario's symbolic group names onto real ids.""" - return {"watched": args.group_id, "reply": args.group_id_send, "other": args.other_group} +def resolve_groups(args: argparse.Namespace) -> dict[str, str]: + """Map the scenario's symbolic group names onto real ids. + + Names are supplied as repeated ``--group NAME=ID`` pairs rather than one + flag per group, because the bot now watches an arbitrary number of them + and a scenario has to be able to say "this reply belongs in the *other* + watched group". + + Args: + args: Parsed command line. + + Returns: + Scenario alias to real group id. + + Raises: + SystemExit: A --group argument is not NAME=ID. + """ + groups: dict[str, str] = {} + for pair in args.group: + name, separator, value = pair.partition("=") + if not separator or not name or not value: + msg = f"--group expects NAME=ID, got {pair!r}" + raise SystemExit(msg) + groups[name] = value + return groups async def run_scenario( @@ -48,16 +70,14 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: parser = argparse.ArgumentParser(description="Replay a scenario against the bot.") parser.add_argument("scenarios", nargs="+", type=Path) parser.add_argument("--base-url", default="http://127.0.0.1:8099") - parser.add_argument("--group-id", required=True) - parser.add_argument("--group-id-send", required=True) - parser.add_argument("--other-group", default="group-other==") + parser.add_argument("--group", action="append", default=[], metavar="NAME=ID") parser.add_argument("--settle", type=float, default=SETTLE_SECONDS) return parser.parse_args(argv) async def _main_async(args: argparse.Namespace) -> int: """Run every scenario, reporting failures as they happen.""" - groups = _groups(args) + groups = resolve_groups(args) failed = 0 for path in args.scenarios: scenario = load_scenario(path) diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 860be61..84d2222 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -26,7 +26,10 @@ readonly FAKE_URL="http://127.0.0.1:${FAKE_PORT}" # message in its own group. readonly WATCHED_GROUP="group.WlRKbExYZGhkR05vWldRdFozSnZkWEF0YVdRdE16SXRZbmwwWlhNaElTRT0=" readonly WATCHED_GROUP_INTERNAL="ZTJlLXdhdGNoZWQtZ3JvdXAtaWQtMzItYnl0ZXMhISE=" -readonly REPLY_GROUP="group.e2e.reply==" +# A second watched group, so the suite proves replies and rate limits stay +# in the group that asked instead of leaking into the other one. +readonly SECOND_GROUP="group.WlRKbExYTmxZMjl1WkMxbmNtOTFjQzFwWkMwek1pMWllWFJsY3lFaElTRT0=" +readonly SECOND_GROUP_INTERNAL="ZTJlLXNlY29uZC1ncm91cC1pZC0zMi1ieXRlcyEhISE=" readonly OTHER_GROUP="group.e2e.other==" FAKE_PID="" @@ -99,13 +102,12 @@ main() { -d '["+15550000000"]' >/dev/null curl -sf -X POST "${FAKE_URL}/_control/groups" \ -H 'Content-Type: application/json' \ - -d "[{\"id\":\"${WATCHED_GROUP}\",\"internal_id\":\"${WATCHED_GROUP_INTERNAL}\",\"name\":\"e2e\",\"member\":true}]" >/dev/null + -d "[{\"id\":\"${WATCHED_GROUP}\",\"internal_id\":\"${WATCHED_GROUP_INTERNAL}\",\"name\":\"e2e\",\"member\":true},{\"id\":\"${SECOND_GROUP}\",\"internal_id\":\"${SECOND_GROUP_INTERNAL}\",\"name\":\"e2e-second\",\"member\":true}]" >/dev/null echo "== booting the bot on :${BOT_PORT}, pointed at the fake" env -u CAT_API \ PHONE_NUMBER="+15550000000" \ - GROUP_ID="$WATCHED_GROUP" \ - GROUP_ID_SEND="$REPLY_GROUP" \ + GROUP_IDS="${WATCHED_GROUP},${SECOND_GROUP}" \ API_BASE_URL="$FAKE_URL" \ DATABASE_PATH="${TEMP_DIR}/e2e.sqlite3" \ CAT_API_URL="${FAKE_URL}/_fixtures/cat-api" \ @@ -120,9 +122,11 @@ main() { "$PYTHON" -m fake_signal.cli \ "$REPO_ROOT"/fake_signal/scenarios/*.yaml \ --base-url "$FAKE_URL" \ - --group-id "$WATCHED_GROUP_INTERNAL" \ - --group-id-send "$REPLY_GROUP" \ - --other-group "$OTHER_GROUP" + --group "watched=${WATCHED_GROUP_INTERNAL}" \ + --group "reply=${WATCHED_GROUP}" \ + --group "second=${SECOND_GROUP_INTERNAL}" \ + --group "second_reply=${SECOND_GROUP}" \ + --group "other=${OTHER_GROUP}" } main "$@" diff --git a/scripts/lib_signal.sh b/scripts/lib_signal.sh index e1d7fd8..570cafa 100644 --- a/scripts/lib_signal.sh +++ b/scripts/lib_signal.sh @@ -135,15 +135,14 @@ signal_free_port() { return 1 } -# Write .env for the bot. GROUP_ID == GROUP_ID_SEND so replies land in the -# same group that triggered them. +# Write .env for the bot. GROUP_IDS is a comma-separated list; replies always +# go back to whichever of those groups the message came from. signal_write_env() { local repo_root="$1" number="$2" group="$3" port port="$(signal_free_port "${BOT_PORT_BASE:-8137}")" cat > "${repo_root}/.env" < None: encoded = await fetcher.fetch_base64(api_url, url_path) - await ctx.transport.send_image(encoded, ctx.recipient) + await ctx.transport.send_image(encoded, ctx.group_id) return handler diff --git a/signal_bot/app.py b/signal_bot/app.py index 7f94f47..b5c149a 100644 --- a/signal_bot/app.py +++ b/signal_bot/app.py @@ -50,10 +50,21 @@ class Bot: dispatcher: Dispatcher async def publish_report(self) -> None: - """Post the tally to the reply group and reset it.""" - rows = [(t.display_name or t.uuid, t.count) for t in self.store.tallies()] - await self.transport.send_text(format_tally(rows), self.config.group_id_send) - self.store.reset_tallies() + """Post each group's own tally into that group, and reset it. + + Each group is isolated on purpose. One group refusing the report -- + the bot removed from it, a transient send failure -- must not cost + every other group its report, and a tally is cleared only after its + report has actually gone out, so a failed send loses nothing. + """ + for group_id in self.config.group_ids: + rows = [(t.display_name or t.uuid, t.count) for t in self.store.tallies(group_id)] + try: + await self.transport.send_text(format_tally(rows), group_id) + except Exception: + logger.exception("daily report failed for group %s", group_id) + continue + self.store.reset_tallies(group_id) async def listen(self) -> None: """Consume frames forever, handing each to the dispatcher.""" @@ -136,9 +147,10 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: runtime = await create_runtime(config, client) runtime.start() logger.info( - "listening as %s, watching group ids %r", + "listening as %s, watching %d group(s): %r", config.phone_number, - sorted(config.watched_group_ids), + len(config.group_ids), + list(config.group_ids), ) app.state.runtime = runtime try: diff --git a/signal_bot/config.py b/signal_bot/config.py index 0585ff7..910146c 100644 --- a/signal_bot/config.py +++ b/signal_bot/config.py @@ -3,8 +3,6 @@ from __future__ import annotations -import base64 -import binascii import logging import os from dataclasses import dataclass @@ -13,6 +11,24 @@ from urllib.parse import urlsplit, urlunsplit from dotenv import load_dotenv +# Re-exported: every caller has always imported these from config, and the +# split is a filing change, not an interface change. +from signal_bot.groups import ( + ConfigError, + build_alias_map, + canonical_group_id, + group_id_aliases, + parse_group_ids, +) + +__all__ = [ + "Config", + "ConfigError", + "canonical_group_id", + "group_id_aliases", + "load_config", +] + DEFAULT_API_BASE_URL: Final = "http://localhost:9922" DEFAULT_DATABASE_PATH: Final = "signal_bot.sqlite3" DEFAULT_REPORT_HOUR: Final = 21 @@ -29,51 +45,17 @@ DEFAULT_BIND_PORT: Final = 8137 MAX_HOUR: Final = 23 MAX_MINUTE: Final = 59 MAX_PORT: Final = 65535 -GROUP_ID_PREFIX: Final = "group." DEFAULT_LOG_LEVEL: Final = "INFO" _WS_SCHEMES: Final = {"http": "ws", "https": "wss"} -def group_id_aliases(group_id: str) -> frozenset[str]: - """Every spelling of one group id that can arrive in an envelope. - - The REST API reports group ids two different ways and never says so: - ``/v1/groups`` returns ``group.`` (what the setup script writes - into .env), while an incoming envelope's ``groupInfo.groupId`` carries - the *internal* id -- which is exactly what that base64 decodes to. The - two never compare equal, so a bot configured from the API's own output - silently ignores every message in its own group. Verified live on - 2026-08-29: ``group.WTI5dVptbG...`` decodes to ``Y29uZmlnLWdy/...``, the id - the same endpoint reports as ``internal_id``. - - Args: - group_id: The configured group id, in either spelling. - - Returns: - The configured id plus its decoded internal form, when it has one. - """ - aliases = {group_id} - if group_id.startswith(GROUP_ID_PREFIX): - try: - decoded = base64.b64decode(group_id.removeprefix(GROUP_ID_PREFIX), validate=True) - except binascii.Error, ValueError: - return frozenset(aliases) - aliases.add(decoded.decode("utf-8", errors="replace")) - return frozenset(aliases) - - -class ConfigError(RuntimeError): - """Raised when the environment does not describe a runnable bot.""" - - @dataclass(frozen=True, slots=True) class Config: """Everything the bot needs to know, resolved and validated.""" phone_number: str - group_id: str - group_id_send: str + group_ids: tuple[str, ...] api_base_url: str = DEFAULT_API_BASE_URL database_path: str = DEFAULT_DATABASE_PATH report_hour: int = DEFAULT_REPORT_HOUR @@ -94,10 +76,35 @@ class Config: # into a bot that looks healthy and answers nothing. log_level: str = DEFAULT_LOG_LEVEL + @property + def group_alias_map(self) -> dict[str, str]: + """Every spelling of every watched group, mapped to its canonical id.""" + return build_alias_map(self.group_ids) + @property def watched_group_ids(self) -> frozenset[str]: - """Group ids that count as "the watched group" in an envelope.""" - return group_id_aliases(self.group_id) + """Group ids that count as "a watched group" in an envelope.""" + return frozenset(self.group_alias_map) + + def resolve_group(self, group_id: str | None) -> str | None: + """Return the canonical id of a watched group, or None. + + This is the single place an envelope's group id becomes a group the + bot acts on. The scoping gate, the reply address and the per-group + state key are all derived from this one answer, so the gate and the + router cannot disagree about which group a message belongs to. + + Args: + group_id: The group id as it arrived, or None for a direct + message. + + Returns: + The canonical ``group.`` id, or None when the message + came from a direct conversation or an unwatched group. + """ + if group_id is None: + return None + return self.group_alias_map.get(group_id) @property def receive_url(self) -> str: @@ -133,11 +140,7 @@ def _require(env: dict[str, str], key: str) -> str: """Return a non-empty environment value or raise ConfigError.""" value = env.get(key, "").strip() if not value: - msg = ( - f"{key} is required and must not be empty. " - "A linked Signal device sees every chat on the account, so the bot " - "refuses to start without an explicit group to scope itself to." - ) + msg = f"{key} is required and must not be empty." raise ConfigError(msg) return value @@ -191,8 +194,7 @@ def load_config(env: dict[str, str] | None = None, *, use_dotenv: bool = True) - return Config( phone_number=_require(env, "PHONE_NUMBER"), - group_id=_require(env, "GROUP_ID"), - group_id_send=_require(env, "GROUP_ID_SEND"), + group_ids=parse_group_ids(env), api_base_url=env.get("API_BASE_URL", "").strip() or DEFAULT_API_BASE_URL, database_path=env.get("DATABASE_PATH", "").strip() or DEFAULT_DATABASE_PATH, report_hour=hour, diff --git a/signal_bot/counting.py b/signal_bot/counting.py index 122ced8..9904ce6 100644 --- a/signal_bot/counting.py +++ b/signal_bot/counting.py @@ -10,20 +10,31 @@ if TYPE_CHECKING: from signal_bot.models import IncomingMessage -def is_watched_group(message: IncomingMessage, config: Config) -> bool: - """True only for messages from the one group the bot is scoped to. +def watched_group(message: IncomingMessage, config: Config) -> str | None: + """Return the canonical id of the watched group a message came from. A linked Signal device receives every direct conversation and every group - on the account. This predicate is the entire boundary between "a bot in - one group" and "a bot reading all of your chats", so it is deliberately - written as an explicit group-id membership test rather than a falsy - check: a direct message has no group id at all and must never reach a - handler. The set, rather than a single ==, is because the REST API - spells the same group two ways -- see Config.watched_group_ids. + on the account. This function is the entire boundary between "a bot in + its groups" and "a bot reading all of your chats", so it is deliberately + written as an explicit group-id lookup rather than a falsy check: a + direct message has no group id at all and must never reach a handler. + + It returns the id rather than a bool so that the gate and the reply + router are the same decision. Answering "may this run?" and "where does + the answer go?" separately is how a bot ends up replying to the wrong + group. + + Args: + message: The parsed inbound message. + config: Resolved runtime configuration. + + Returns: + The canonical group id, or None for a direct message or any group + the bot is not scoped to. """ if message.group_id is None: - return False - return message.group_id in config.watched_group_ids + return None + return config.resolve_group(message.group_id) def should_count(message: IncomingMessage) -> bool: diff --git a/signal_bot/decorators.py b/signal_bot/decorators.py index 20f3cf8..861cbeb 100644 --- a/signal_bot/decorators.py +++ b/signal_bot/decorators.py @@ -27,7 +27,7 @@ def cooldown(seconds: float, message: str) -> Decorator: def decorate(handler: Handler) -> Handler: @functools.wraps(handler) async def wrapper(ctx: CommandContext) -> None: - last = ctx.store.last_used(ctx.message.source_uuid, ctx.command_name) + 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)) @@ -45,7 +45,9 @@ def daily_limit(max_uses: int, message: str) -> Decorator: def decorate(handler: Handler) -> Handler: @functools.wraps(handler) async def wrapper(ctx: CommandContext) -> None: - used = ctx.store.uses_today(ctx.message.source_uuid, ctx.command_name, ctx.day) + used = ctx.store.uses_today( + ctx.group_id, ctx.message.source_uuid, ctx.command_name, ctx.day + ) if used >= max_uses: await ctx.reply(message.format(max_uses=max_uses, used=used)) return @@ -66,12 +68,12 @@ def vote_gated(required: int, window_seconds: float, message: str) -> Decorator: def decorate(handler: Handler) -> Handler: @functools.wraps(handler) async def wrapper(ctx: CommandContext) -> None: - ctx.store.add_vote(ctx.command_name, ctx.message.source_uuid, ctx.now) - votes = ctx.store.count_votes(ctx.command_name, ctx.now - window_seconds) + ctx.store.add_vote(ctx.group_id, ctx.command_name, ctx.message.source_uuid, ctx.now) + 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 - ctx.store.clear_votes(ctx.command_name) + ctx.store.clear_votes(ctx.group_id, ctx.command_name) await handler(ctx) return wrapper diff --git a/signal_bot/dispatcher.py b/signal_bot/dispatcher.py index a3eed95..44ed4c7 100644 --- a/signal_bot/dispatcher.py +++ b/signal_bot/dispatcher.py @@ -9,7 +9,7 @@ from collections.abc import Callable from dataclasses import dataclass from typing import TYPE_CHECKING -from signal_bot.counting import is_watched_group, should_count +from signal_bot.counting import should_count, watched_group from signal_bot.models import parse_envelope from signal_bot.registry import CommandContext, CommandRegistry @@ -41,16 +41,20 @@ class Dispatcher: # The scoping gate. Direct messages and every other group the account # belongs to stop here, before counting and before any handler runs. - if not is_watched_group(message, self.config): + # The canonical id it yields is the reply address and the key for + # every piece of per-group state, so a message can only ever be + # counted, rate-limited and answered in the group it came from. + group = watched_group(message, self.config) + if group is None: logger.debug( "ignoring message from group %r; watching %r", message.group_id, - sorted(self.config.watched_group_ids), + sorted(self.config.group_ids), ) return if should_count(message): - self.store.increment(message.source_uuid, message.source_name) + self.store.increment(group, message.source_uuid, message.source_name) command = self.registry.resolve(message.text) if command is None: @@ -60,7 +64,7 @@ class Dispatcher: now = self.clock() ctx = CommandContext( message=message, - recipient=self.config.group_id_send, + group_id=group, transport=self.transport, store=self.store, config=self.config, @@ -74,4 +78,4 @@ class Dispatcher: logger.exception("command %s failed", command.name) return # Recorded once, here, so stacked gates cannot double-count a use. - self.store.record_use(message.source_uuid, command.name, ctx.now, ctx.day) + self.store.record_use(group, message.source_uuid, command.name, ctx.now, ctx.day) diff --git a/signal_bot/registry.py b/signal_bot/registry.py index 7e3d74d..fc6b025 100644 --- a/signal_bot/registry.py +++ b/signal_bot/registry.py @@ -26,7 +26,7 @@ class CommandContext: """Everything a command handler is allowed to touch.""" message: IncomingMessage - recipient: str + group_id: str transport: Transport store: Store config: Config @@ -35,8 +35,13 @@ class CommandContext: day: str async def reply(self, text: str) -> None: - """Send a text reply to the configured reply target.""" - await self.transport.send_text(text, self.recipient) + """Send a text reply into the group the command came from. + + There is no configurable reply target. A command answers where it + was asked, which is the only routing that stays correct once the + bot watches a test group and a production group at once. + """ + await self.transport.send_text(text, self.group_id) Handler = Callable[[CommandContext], Awaitable[None]] diff --git a/signal_bot/store.py b/signal_bot/store.py index 34844d2..e82ff8d 100644 --- a/signal_bot/store.py +++ b/signal_bot/store.py @@ -16,33 +16,48 @@ if TYPE_CHECKING: from collections.abc import Iterator from types import TracebackType +# Bumped whenever the shape below changes. See Store._apply_schema for why +# a version number is the only safe way to notice. +SCHEMA_VERSION = 2 + +# Every table is keyed by group first. The bot watches several groups, and a +# tally, a cooldown or a vote that leaked between them would mean a test +# message throttling the production group -- so the scope is in the primary +# key rather than in the callers' discipline. _SCHEMA = """ CREATE TABLE IF NOT EXISTS message_counts ( - uuid TEXT PRIMARY KEY, + group_id TEXT NOT NULL, + uuid TEXT NOT NULL, display_name TEXT NOT NULL, - count INTEGER NOT NULL DEFAULT 0 + count INTEGER NOT NULL DEFAULT 0, + PRIMARY KEY (group_id, uuid) ); CREATE TABLE IF NOT EXISTS command_uses ( + group_id TEXT NOT NULL, user_uuid TEXT NOT NULL, command TEXT NOT NULL, used_at REAL NOT NULL, - PRIMARY KEY (user_uuid, command) + PRIMARY KEY (group_id, user_uuid, command) ); CREATE TABLE IF NOT EXISTS daily_uses ( + group_id TEXT NOT NULL, user_uuid TEXT NOT NULL, command TEXT NOT NULL, day TEXT NOT NULL, uses INTEGER NOT NULL DEFAULT 0, - PRIMARY KEY (user_uuid, command, day) + PRIMARY KEY (group_id, user_uuid, command, day) ); CREATE TABLE IF NOT EXISTS votes ( + group_id TEXT NOT NULL, command TEXT NOT NULL, voter_uuid TEXT NOT NULL, voted_at REAL NOT NULL, - PRIMARY KEY (command, voter_uuid) + PRIMARY KEY (group_id, command, voter_uuid) ); """ +_TABLES = ("message_counts", "command_uses", "daily_uses", "votes") + @dataclass(frozen=True, slots=True) class Tally: @@ -60,7 +75,28 @@ class Store: """Open (or create) the database at path and apply the schema.""" self._conn = sqlite3.connect(path) self._conn.row_factory = sqlite3.Row + self._apply_schema() + + def _apply_schema(self) -> None: + """Create the tables, rebuilding them when the file predates them. + + ``CREATE TABLE IF NOT EXISTS`` is a no-op against a table that + already exists with the wrong columns, so a database written before + the group_id key would survive startup untouched and then fail on + every single INSERT -- a bot that starts clean and answers nothing. + PRAGMA user_version is the only thing that notices, because it is + the only part of the file that records which shape it holds. + + The old rows are dropped rather than migrated: they carry no group, + and inventing one would silently attribute a whole day's counts to + the wrong conversation. + """ + version = int(self._conn.execute("PRAGMA user_version").fetchone()[0]) + if version < SCHEMA_VERSION: + for table in _TABLES: + self._conn.execute(f"DROP TABLE IF EXISTS {table}") self._conn.executescript(_SCHEMA) + self._conn.execute(f"PRAGMA user_version = {SCHEMA_VERSION}") self._conn.commit() def __enter__(self) -> Self: @@ -82,90 +118,95 @@ class Store: # -- message tallies --------------------------------------------------- - def increment(self, uuid: str, display_name: str) -> None: - """Add one to a person's tally, creating the row when new.""" + def increment(self, group_id: str, uuid: str, display_name: str) -> None: + """Add one to a person's tally in one group, creating the row when new.""" self._conn.execute( """ - INSERT INTO message_counts (uuid, display_name, count) - VALUES (?, ?, 1) - ON CONFLICT(uuid) DO UPDATE SET + INSERT INTO message_counts (group_id, uuid, display_name, count) + VALUES (?, ?, ?, 1) + ON CONFLICT(group_id, uuid) DO UPDATE SET count = count + 1, display_name = excluded.display_name """, - (uuid, display_name), + (group_id, uuid, display_name), ) self._conn.commit() - def tallies(self) -> list[Tally]: - """Return every tally, highest count first.""" + def tallies(self, group_id: str) -> list[Tally]: + """Return one group's tallies, highest count first.""" rows = self._conn.execute( "SELECT uuid, display_name, count FROM message_counts " - "ORDER BY count DESC, display_name ASC" + "WHERE group_id = ? ORDER BY count DESC, display_name ASC", + (group_id,), ).fetchall() return [Tally(r["uuid"], r["display_name"], r["count"]) for r in rows] - def reset_tallies(self) -> None: - """Clear every tally, as the daily report does after publishing.""" - self._conn.execute("DELETE FROM message_counts") + def reset_tallies(self, group_id: str) -> None: + """Clear one group's tallies, as its daily report does after publishing.""" + self._conn.execute("DELETE FROM message_counts WHERE group_id = ?", (group_id,)) self._conn.commit() - # -- per-user, per-command rate limiting ------------------------------- + # -- per-group, per-user, per-command rate limiting -------------------- - def record_use(self, user_uuid: str, command: str, now: float, day: str) -> None: + def record_use(self, group_id: str, user_uuid: str, command: str, now: float, day: str) -> None: """Record one successful use for both cooldown and daily-limit checks.""" self._conn.execute( - "INSERT INTO command_uses (user_uuid, command, used_at) VALUES (?, ?, ?) " - "ON CONFLICT(user_uuid, command) DO UPDATE SET used_at = excluded.used_at", - (user_uuid, command, now), + "INSERT INTO command_uses (group_id, user_uuid, command, used_at) " + "VALUES (?, ?, ?, ?) " + "ON CONFLICT(group_id, user_uuid, command) DO UPDATE SET used_at = excluded.used_at", + (group_id, user_uuid, command, now), ) self._conn.execute( """ - INSERT INTO daily_uses (user_uuid, command, day, uses) - VALUES (?, ?, ?, 1) - ON CONFLICT(user_uuid, command, day) DO UPDATE SET uses = uses + 1 + INSERT INTO daily_uses (group_id, user_uuid, command, day, uses) + VALUES (?, ?, ?, ?, 1) + ON CONFLICT(group_id, user_uuid, command, day) DO UPDATE SET uses = uses + 1 """, - (user_uuid, command, day), + (group_id, user_uuid, command, day), ) self._conn.commit() - def last_used(self, user_uuid: str, command: str) -> float | None: - """Return when this user last ran this command, or None.""" + def last_used(self, group_id: str, user_uuid: str, command: str) -> float | None: + """Return when this user last ran this command in this group, or None.""" row = self._conn.execute( - "SELECT used_at FROM command_uses WHERE user_uuid = ? AND command = ?", - (user_uuid, command), + "SELECT used_at FROM command_uses WHERE group_id = ? AND user_uuid = ? AND command = ?", + (group_id, user_uuid, command), ).fetchone() return None if row is None else float(row["used_at"]) - def uses_today(self, user_uuid: str, command: str, day: str) -> int: - """Return how many times this user ran this command on the given day.""" + def uses_today(self, group_id: str, user_uuid: str, command: str, day: str) -> int: + """Return how often this user ran this command in this group that day.""" row = self._conn.execute( - "SELECT uses FROM daily_uses WHERE user_uuid = ? AND command = ? AND day = ?", - (user_uuid, command, day), + "SELECT uses FROM daily_uses " + "WHERE group_id = ? AND user_uuid = ? AND command = ? AND day = ?", + (group_id, user_uuid, command, day), ).fetchone() return 0 if row is None else int(row["uses"]) # -- vote gating ------------------------------------------------------- - def add_vote(self, command: str, voter_uuid: str, now: float) -> None: + def add_vote(self, group_id: str, command: str, voter_uuid: str, now: float) -> None: """Record one vote, ignoring a repeat vote from the same person.""" self._conn.execute( - "INSERT INTO votes (command, voter_uuid, voted_at) VALUES (?, ?, ?) " - "ON CONFLICT(command, voter_uuid) DO NOTHING", - (command, voter_uuid, now), + "INSERT INTO votes (group_id, command, voter_uuid, voted_at) VALUES (?, ?, ?, ?) " + "ON CONFLICT(group_id, command, voter_uuid) DO NOTHING", + (group_id, command, voter_uuid, now), ) self._conn.commit() - def count_votes(self, command: str, not_before: float) -> int: - """Return how many votes for command are still inside the window.""" + def count_votes(self, group_id: str, command: str, not_before: float) -> int: + """Return how many votes for command in this group are still in the window.""" row = self._conn.execute( - "SELECT COUNT(*) AS n FROM votes WHERE command = ? AND voted_at >= ?", - (command, not_before), + "SELECT COUNT(*) AS n FROM votes WHERE group_id = ? AND command = ? AND voted_at >= ?", + (group_id, command, not_before), ).fetchone() return int(row["n"]) - def clear_votes(self, command: str) -> None: - """Drop every vote for a command, after it fires or expires.""" - self._conn.execute("DELETE FROM votes WHERE command = ?", (command,)) + def clear_votes(self, group_id: str, command: str) -> None: + """Drop every vote for a command in one group, after it fires or expires.""" + self._conn.execute( + "DELETE FROM votes WHERE group_id = ? AND command = ?", (group_id, command) + ) self._conn.commit() diff --git a/signal_bot/transport_rest.py b/signal_bot/transport_rest.py index d13fbc5..aa7d9c8 100644 --- a/signal_bot/transport_rest.py +++ b/signal_bot/transport_rest.py @@ -16,6 +16,8 @@ from typing import TYPE_CHECKING, Any, Final from websockets.asyncio.client import connect as ws_connect from websockets.exceptions import WebSocketException +from signal_bot.membership import MembershipError, MembershipVerifier + if TYPE_CHECKING: import httpx @@ -88,44 +90,20 @@ class RestTransport: return mode async def verify_membership(self) -> None: - """Raise unless the account is a full member of the watched group. + """Raise unless the account is a full member of every watched group. - A pending invitation is indistinguishable from membership from the - outside: /v1/groups lists the group, the websocket connects, - /healthz answers, and not one message ever arrives. Signal only - adds an account to a group outright when the inviter already holds - its profile key; otherwise it creates an invite, and signal-cli - cannot accept one addressed to a phone number ("Cannot find - service ID for self to accept invite"). Observed live on - 2026-08-29, where the bot looked perfectly healthy for hours while - being a non-member. Startup is the only honest place to catch it. + Delegated to signal_bot.membership, which owns the invite-versus-join + distinction; kept as a method here because it is a startup precondition + of this transport, not of the bot in general. Raises: - TransportError: The account is not a full member of the group. + TransportError: A watched group is missing, or the account is not + a full member of it and could not join. """ - response = await self._client.get(self._config.groups_url) - response.raise_for_status() - wanted = self._config.watched_group_ids - for group in response.json(): - if not isinstance(group, dict): - continue - if not {group.get("id"), group.get("internal_id")} & wanted: - continue - if group.get("member"): - return - msg = ( - f"the account is invited to {group.get('name')!r} but has not " - "joined it, so it will receive nothing. signal-cli cannot " - "accept an invite addressed to a phone number: have an admin " - "revoke the invite and add the bot again now that it has a " - "profile, or have the bot create the group itself." - ) - raise TransportError(msg) - msg = ( - f"the account is in no group matching {sorted(wanted)}. Check GROUP_ID " - "in .env against: curl -s $API_BASE_URL/v1/groups/$PHONE_NUMBER" - ) - raise TransportError(msg) + try: + await MembershipVerifier(self._config, self._client).verify() + except MembershipError as exc: + raise TransportError(str(exc)) from exc async def receive(self) -> AsyncIterator[str]: """Yield raw envelope frames, reconnecting with backoff when dropped.""" diff --git a/tests/conftest.py b/tests/conftest.py index 1ee0119..81ea7ee 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,7 +14,7 @@ from signal_bot.store import Store from signal_bot.transport_fake import FakeTransport WATCHED_GROUP = "group-watched==" -REPLY_GROUP = "group-reply==" +SECOND_GROUP = "group-second==" OTHER_GROUP = "group-other==" @@ -22,8 +22,7 @@ OTHER_GROUP = "group-other==" def config() -> Config: return Config( phone_number="+15550000000", - group_id=WATCHED_GROUP, - group_id_send=REPLY_GROUP, + group_ids=(WATCHED_GROUP, SECOND_GROUP), api_base_url="http://localhost:9922", database_path=":memory:", cooldown_seconds=10, diff --git a/tests/test_animals.py b/tests/test_animals.py index 9168c6e..ecf99f0 100644 --- a/tests/test_animals.py +++ b/tests/test_animals.py @@ -18,7 +18,7 @@ from signal_bot.decorators import cooldown from signal_bot.http_client import ImageFetcher from signal_bot.models import parse_envelope from signal_bot.registry import CommandContext, CommandRegistry -from tests.conftest import REPLY_GROUP, envelope +from tests.conftest import WATCHED_GROUP, envelope if TYPE_CHECKING: from pathlib import Path @@ -48,7 +48,7 @@ def _ctx( assert message is not None return CommandContext( message=message, - recipient=REPLY_GROUP, + group_id=WATCHED_GROUP, transport=transport, store=store, config=config, @@ -89,7 +89,7 @@ async def test_commands_post_an_image( assert command is not None await command.handler(_ctx(transport, store, config, name, trigger)) assert transport.sent[0].image_base64 is not None - assert transport.sent[0].recipient == REPLY_GROUP + assert transport.sent[0].recipient == WATCHED_GROUP async def test_decorator_is_applied_when_given( @@ -97,7 +97,7 @@ async def test_decorator_is_applied_when_given( ) -> None: registry = CommandRegistry() register_animal_commands(registry, _fetcher(), decorate=cooldown(10, "wait {seconds}")) - store.record_use("uuid-alice", "cat", 100.0, "2026-08-29") + 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")) diff --git a/tests/test_app.py b/tests/test_app.py index 282ef5a..7e8b342 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio +from dataclasses import dataclass from typing import TYPE_CHECKING import httpx @@ -12,19 +13,18 @@ from fastapi.testclient import TestClient from signal_bot import app as app_module from signal_bot.app import Bot, Runtime, build_bot, create_runtime, lifespan, now_local +from signal_bot.transport_fake import FakeTransport from signal_bot.transport_rest import TransportError -from tests.conftest import REPLY_GROUP, envelope +from tests.conftest import SECOND_GROUP, WATCHED_GROUP, envelope if TYPE_CHECKING: from signal_bot.config import Config from signal_bot.registry import Handler from signal_bot.store import Store - from signal_bot.transport_fake import FakeTransport BASE_ENV = { "PHONE_NUMBER": "+15550000000", - "GROUP_ID": "watched==", - "GROUP_ID_SEND": "reply==", + "GROUP_IDS": "group.watched==", "DATABASE_PATH": ":memory:", } @@ -38,7 +38,7 @@ def _client(handler: Handler) -> httpx.AsyncClient: return _REAL_ASYNC_CLIENT(transport=httpx.MockTransport(handler)) -def _signal(group_id: str, *, mode: str = "json-rpc", member: bool = True) -> Handler: +def _signal(*group_ids: str, mode: str = "json-rpc", member: bool = True) -> Handler: """Answer both startup probes: the mode check and the membership check.""" def handler(request: httpx.Request) -> httpx.Response: @@ -49,9 +49,10 @@ def _signal(group_id: str, *, mode: str = "json-rpc", member: bool = True) -> Ha { "id": group_id, "internal_id": group_id, - "name": "watched", + "name": f"group {index}", "member": member, } + for index, group_id in enumerate(group_ids) ], ) return httpx.Response(200, json={"mode": mode}) @@ -75,34 +76,40 @@ def test_build_bot_registers_the_animal_commands( assert bot.dispatcher.registry.resolve("!pies") is not None -async def test_publish_report_posts_and_resets( +async def test_publish_report_posts_each_group_its_own_tally( config: Config, store: Store, transport: FakeTransport ) -> None: - store.increment("u1", "Alice") + """Every group gets its own counts, in its own chat, and only its own.""" + store.increment(WATCHED_GROUP, "u1", "Alice") + store.increment(SECOND_GROUP, "u2", "Bob") + store.increment(SECOND_GROUP, "u2", "Bob") await _bot(config, store, transport).publish_report() - assert transport.texts == ["Alice 1"] - assert transport.sent[0].recipient == REPLY_GROUP - assert store.tallies() == [] + assert [(m.recipient, m.text) for m in transport.sent] == [ + (WATCHED_GROUP, "Alice 1"), + (SECOND_GROUP, "Bob 2"), + ] + assert store.tallies(WATCHED_GROUP) == [] + assert store.tallies(SECOND_GROUP) == [] async def test_publish_report_falls_back_to_uuid_when_unnamed( config: Config, store: Store, transport: FakeTransport ) -> None: - store.increment("u1", "") + store.increment(WATCHED_GROUP, "u1", "") await _bot(config, store, transport).publish_report() - assert transport.texts == ["u1 1"] + assert transport.texts[0] == "u1 1" async def test_listen_dispatches_every_frame( config: Config, store: Store, transport: FakeTransport ) -> None: - transport.queue([envelope(text="hello", group_id=config.group_id)]) + transport.queue([envelope(text="hello", group_id=WATCHED_GROUP)]) await _bot(config, store, transport).listen() - assert store.tallies()[0].count == 1 + assert store.tallies(WATCHED_GROUP)[0].count == 1 async def test_create_runtime_verifies_the_container_mode(config: Config) -> None: - runtime = await create_runtime(config, _client(_signal(config.group_id))) + runtime = await create_runtime(config, _client(_signal(*config.group_ids))) assert isinstance(runtime, Runtime) await runtime.stop() @@ -132,7 +139,7 @@ def test_healthz_serves_and_startup_completes(monkeypatch: pytest.MonkeyPatch) - monkeypatch.setenv(key, value) def fake_client(**kwargs: object) -> httpx.AsyncClient: - return _client(_signal(BASE_ENV["GROUP_ID"])) + return _client(_signal(BASE_ENV["GROUP_IDS"])) monkeypatch.setattr(app_module.httpx, "AsyncClient", fake_client) with TestClient(app_module.app) as client: @@ -147,7 +154,7 @@ async def test_lifespan_is_an_async_context_manager( monkeypatch.setattr( app_module.httpx, "AsyncClient", - lambda **kwargs: _client(_signal(BASE_ENV["GROUP_ID"])), + lambda **kwargs: _client(_signal(BASE_ENV["GROUP_IDS"])), ) async with lifespan(app_module.app): assert app_module.app.state.runtime is not None @@ -158,4 +165,34 @@ async def test_create_runtime_refuses_a_group_it_only_got_invited_to( ) -> None: """A pending invite looks healthy from outside; startup must not.""" with pytest.raises(TransportError, match="has not joined"): - await create_runtime(config, _client(_signal(config.group_id, member=False))) + await create_runtime(config, _client(_signal(*config.group_ids, member=False))) + + +@dataclass +class RefusingTransport(FakeTransport): + """A transport that rejects one group, the way a removal from it would.""" + + refuses: str = "" + + async def send_text(self, text: str, recipient: str) -> None: + """Record the send, unless this is the group that refuses them.""" + if recipient == self.refuses: + msg = "removed from the group" + raise RuntimeError(msg) + await super().send_text(text, recipient) + + +async def test_one_group_failing_its_report_does_not_cost_the_others( + config: Config, store: Store +) -> None: + """A failed send must not clear that group's day, nor skip the next group.""" + store.increment(WATCHED_GROUP, "u1", "Alice") + store.increment(SECOND_GROUP, "u2", "Bob") + transport = RefusingTransport(refuses=WATCHED_GROUP) + + await _bot(config, store, transport).publish_report() + + assert [(m.recipient, m.text) for m in transport.sent] == [(SECOND_GROUP, "Bob 1")] + # The group that failed keeps its counts; the one that succeeded is reset. + assert store.tallies(WATCHED_GROUP)[0].count == 1 + assert store.tallies(SECOND_GROUP) == [] diff --git a/tests/test_cli.py b/tests/test_cli.py index a1c2e68..c09612b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -61,15 +61,18 @@ async def test_run_scenario_reports_a_mismatch(monkeypatch: pytest.MonkeyPatch) assert await cli.run_scenario("http://x", scenario, GROUPS, settle=0) -def test_parse_args_requires_the_group_ids() -> None: - args = cli.parse_args(["a.yaml", "--group-id", "W==", "--group-id-send", "R=="]) +def test_parse_args_collects_named_groups() -> None: + args = cli.parse_args(["a.yaml", "--group", "watched=W==", "--group", "second=S=="]) assert args.scenarios == [Path("a.yaml")] - assert args.group_id == "W==" + assert cli.resolve_groups(args) == {"watched": "W==", "second": "S=="} -def test_parse_args_rejects_missing_groups() -> None: - with pytest.raises(SystemExit): - cli.parse_args(["a.yaml"]) +@pytest.mark.parametrize("pair", ["watched", "=W==", "watched="]) +def test_parse_args_rejects_a_malformed_group_pair(pair: str) -> None: + """A typo here would silently rename a group instead of failing.""" + args = cli.parse_args(["a.yaml", "--group", pair]) + with pytest.raises(SystemExit, match="NAME=ID"): + cli.resolve_groups(args) def _scenario_file(tmp_path: Path, expect: str) -> Path: @@ -83,7 +86,7 @@ def test_main_exits_zero_when_every_scenario_passes( ) -> None: _patch_client(monkeypatch, Recorder([{"base64_attachments": ["A"], "recipients": ["R=="]}])) path = _scenario_file(tmp_path, "expect:\n - {kind: image}\n") - code = cli.main([str(path), "--group-id", "W==", "--group-id-send", "R==", "--settle", "0"]) + code = cli.main([str(path), "--group", "watched=W==", "--group", "reply=R==", "--settle", "0"]) assert code == 0 assert "ok demo" in capsys.readouterr().out @@ -94,7 +97,7 @@ def test_main_exits_non_zero_on_a_mismatch( """The e2e script is a gate only because this returns 1.""" _patch_client(monkeypatch, Recorder([])) path = _scenario_file(tmp_path, "expect:\n - {kind: image}\n") - code = cli.main([str(path), "--group-id", "W==", "--group-id-send", "R==", "--settle", "0"]) + code = cli.main([str(path), "--group", "watched=W==", "--group", "reply=R==", "--settle", "0"]) assert code == 1 out = capsys.readouterr().out assert "FAIL demo" in out @@ -111,7 +114,7 @@ def test_module_runs_as_a_script(tmp_path: Path, monkeypatch: pytest.MonkeyPatch monkeypatch.setattr( sys, "argv", - ["cli", str(path), "--group-id", "W==", "--group-id-send", "R==", "--settle", "0"], + ["cli", str(path), "--group", "watched=W==", "--group", "reply=R==", "--settle", "0"], ) with pytest.raises(SystemExit) as exit_info: runpy.run_module("fake_signal.cli", run_name="__main__", alter_sys=True) diff --git a/tests/test_config.py b/tests/test_config.py index 8eaef62..8917e99 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -5,12 +5,21 @@ from __future__ import annotations import pytest -from signal_bot.config import Config, ConfigError, group_id_aliases, load_config +from signal_bot.config import ( + Config, + ConfigError, + canonical_group_id, + group_id_aliases, + load_config, +) + +# The real spellings of one group, as the API reports them. +PREFIXED = "group.WTI5dVptbG5MV2R5YjNWd0xXbGtMVE15TFdKNWRHVnpJU0VoSVNFaElRPT0=" +INTERNAL = "Y29uZmlnLWdyb3VwLWlkLTMyLWJ5dGVzISEhISEhIQ==" BASE_ENV = { "PHONE_NUMBER": "+15550000000", - "GROUP_ID": "watched==", - "GROUP_ID_SEND": "reply==", + "GROUP_IDS": "group.watched==", } @@ -22,7 +31,7 @@ def test_loads_minimal_environment() -> None: assert config.report_minute == 37 -@pytest.mark.parametrize("missing", ["PHONE_NUMBER", "GROUP_ID", "GROUP_ID_SEND"]) +@pytest.mark.parametrize("missing", ["PHONE_NUMBER", "GROUP_IDS"]) def test_refuses_to_start_without_required_value(missing: str) -> None: env = dict(BASE_ENV) del env[missing] @@ -30,7 +39,7 @@ def test_refuses_to_start_without_required_value(missing: str) -> None: load_config(env) -@pytest.mark.parametrize("missing", ["PHONE_NUMBER", "GROUP_ID", "GROUP_ID_SEND"]) +@pytest.mark.parametrize("missing", ["PHONE_NUMBER", "GROUP_IDS"]) def test_refuses_to_start_on_blank_value(missing: str) -> None: """A linked device sees every chat, so an unscoped bot must not start.""" env = dict(BASE_ENV) | {missing: " "} @@ -82,7 +91,7 @@ def test_reads_os_environ_when_no_mapping_given(monkeypatch: pytest.MonkeyPatch) for key, value in BASE_ENV.items(): monkeypatch.setenv(key, value) config = load_config(use_dotenv=False) - assert config.group_id == "watched==" + assert config.group_ids == ("group.watched==",) def test_derived_urls() -> None: @@ -98,8 +107,7 @@ def test_derived_urls() -> None: def test_https_base_url_yields_wss() -> None: config = Config( phone_number="+1", - group_id="g", - group_id_send="r", + group_ids=("group.g==",), api_base_url="https://signal.example", ) assert config.receive_url.startswith("wss://signal.example/") @@ -116,8 +124,10 @@ def test_group_id_aliases_adds_the_internal_form() -> None: } -def test_group_id_aliases_leaves_a_bare_internal_id_alone() -> None: - assert group_id_aliases("Y29uZmlnLWdy=") == {"Y29uZmlnLWdy="} +def test_group_id_aliases_offers_the_prefixed_form_of_an_internal_id() -> None: + """An internal id must still be addressable: /v2/send only takes group.*.""" + aliases = group_id_aliases(INTERNAL) + assert aliases == {INTERNAL, PREFIXED} def test_group_id_aliases_tolerates_undecodable_ids() -> None: @@ -126,22 +136,15 @@ def test_group_id_aliases_tolerates_undecodable_ids() -> None: def test_watched_group_ids_exposes_both_spellings() -> None: - config = load_config( - { - "PHONE_NUMBER": "+15550000000", - "GROUP_ID": "group.WTI5dVptbG5MV2R5YjNWd0xXbGtMVE15TFdKNWRHVnpJU0VoSVNFaElRPT0=", - "GROUP_ID_SEND": "reply==", - } - ) - assert "Y29uZmlnLWdyb3VwLWlkLTMyLWJ5dGVzISEhISEhIQ==" in config.watched_group_ids + config = load_config({"PHONE_NUMBER": "+15550000000", "GROUP_IDS": PREFIXED}) + assert INTERNAL in config.watched_group_ids def test_groups_url_is_scoped_to_the_account() -> None: config = load_config( { "PHONE_NUMBER": "+15550000000", - "GROUP_ID": "g==", - "GROUP_ID_SEND": "g==", + "GROUP_IDS": "group.g==", "API_BASE_URL": "http://localhost:9922", } ) @@ -153,8 +156,7 @@ def test_an_unknown_log_level_is_rejected() -> None: load_config( { "PHONE_NUMBER": "+15550000000", - "GROUP_ID": "g==", - "GROUP_ID_SEND": "g==", + "GROUP_IDS": "group.g==", "LOG_LEVEL": "chatty", } ) @@ -166,8 +168,67 @@ def test_a_port_outside_the_valid_range_is_rejected(port: str) -> None: load_config( { "PHONE_NUMBER": "+15550000000", - "GROUP_ID": "g==", - "GROUP_ID_SEND": "g==", + "GROUP_IDS": "group.g==", "BIND_PORT": port, } ) + + +def test_several_groups_are_parsed_in_order() -> None: + """The point of the change: one process, a test group and a real one.""" + config = load_config(dict(BASE_ENV) | {"GROUP_IDS": f"{PREFIXED}, group.other=="}) + assert config.group_ids == (PREFIXED, "group.other==") + assert config.resolve_group(INTERNAL) == PREFIXED + assert config.resolve_group("group.other==") == "group.other==" + + +def test_group_id_is_still_accepted_as_the_one_group_spelling() -> None: + """An .env written before the list form must keep working untouched.""" + env = {"PHONE_NUMBER": "+15550000000", "GROUP_ID": PREFIXED} + assert load_config(env).group_ids == (PREFIXED,) + + +def test_setting_both_spellings_is_refused() -> None: + env = dict(BASE_ENV) | {"GROUP_ID": PREFIXED} + with pytest.raises(ConfigError, match="not both"): + load_config(env) + + +def test_the_removed_send_target_fails_startup() -> None: + """Silently ignoring it would post the daily report into the void.""" + with pytest.raises(ConfigError, match="GROUP_ID_SEND has been removed"): + load_config(dict(BASE_ENV) | {"GROUP_ID_SEND": PREFIXED}) + + +def test_blank_entries_between_separators_are_skipped() -> None: + config = load_config(dict(BASE_ENV) | {"GROUP_IDS": f" {PREFIXED} , , "}) + assert config.group_ids == (PREFIXED,) + + +def test_a_list_of_only_separators_is_refused() -> None: + with pytest.raises(ConfigError, match="only separators"): + load_config(dict(BASE_ENV) | {"GROUP_IDS": " , , "}) + + +def test_one_group_listed_in_both_spellings_is_refused() -> None: + """Two scopes for one group would split its tally and its cooldowns.""" + with pytest.raises(ConfigError, match="listed twice"): + load_config(dict(BASE_ENV) | {"GROUP_IDS": f"{PREFIXED},{INTERNAL}"}) + + +def test_an_internal_id_is_stored_in_its_addressable_form() -> None: + """Replies are sent to the canonical id, and /v2/send rejects the other.""" + config = load_config(dict(BASE_ENV) | {"GROUP_IDS": INTERNAL}) + assert config.group_ids == (PREFIXED,) + assert config.resolve_group(INTERNAL) == PREFIXED + + +def test_canonical_group_id_is_idempotent() -> None: + assert canonical_group_id(PREFIXED) == PREFIXED + assert canonical_group_id(canonical_group_id(INTERNAL)) == PREFIXED + + +def test_resolve_group_rejects_direct_messages_and_strangers() -> None: + config = load_config(dict(BASE_ENV)) + assert config.resolve_group(None) is None + assert config.resolve_group("group.stranger==") is None diff --git a/tests/test_counting.py b/tests/test_counting.py index 4ca5ed7..a3ccafa 100644 --- a/tests/test_counting.py +++ b/tests/test_counting.py @@ -7,9 +7,9 @@ from typing import TYPE_CHECKING import pytest -from signal_bot.counting import format_tally, is_watched_group, should_count +from signal_bot.counting import format_tally, should_count, watched_group from signal_bot.models import parse_envelope -from tests.conftest import OTHER_GROUP, WATCHED_GROUP, envelope +from tests.conftest import OTHER_GROUP, SECOND_GROUP, WATCHED_GROUP, envelope if TYPE_CHECKING: from signal_bot.config import Config @@ -21,17 +21,19 @@ def _message(**kwargs: object) -> object: return parsed -def test_watched_group_is_accepted(config: Config) -> None: - assert is_watched_group(_message(group_id=WATCHED_GROUP), config) +@pytest.mark.parametrize("group", [WATCHED_GROUP, SECOND_GROUP]) +def test_every_watched_group_is_accepted_as_itself(config: Config, group: str) -> None: + """Each watched group resolves to itself, never to the first one listed.""" + assert watched_group(_message(group_id=group), config) == group def test_other_group_is_rejected(config: Config) -> None: - assert not is_watched_group(_message(group_id=OTHER_GROUP), config) + assert watched_group(_message(group_id=OTHER_GROUP), config) is None def test_direct_message_is_rejected(config: Config) -> None: """The dangerous case: a linked device receives every DM on the account.""" - assert not is_watched_group(_message(group_id=None), config) + assert watched_group(_message(group_id=None), config) is None def test_plain_text_counts() -> None: diff --git a/tests/test_decorators.py b/tests/test_decorators.py index e7a9dd7..90cc9a8 100644 --- a/tests/test_decorators.py +++ b/tests/test_decorators.py @@ -11,7 +11,7 @@ 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 REPLY_GROUP, envelope +from tests.conftest import SECOND_GROUP, WATCHED_GROUP, envelope if TYPE_CHECKING: from signal_bot.config import Config @@ -22,13 +22,19 @@ DAY = "2026-08-29" def _context( - store: Store, transport: FakeTransport, config: Config, *, uuid: str = "u1", now: float = 100.0 + store: Store, + transport: FakeTransport, + config: Config, + *, + uuid: str = "u1", + now: float = 100.0, + group: str = WATCHED_GROUP, ) -> CommandContext: - message = parse_envelope(envelope(text="!ping", uuid=uuid)) + message = parse_envelope(envelope(text="!ping", uuid=uuid, group_id=group)) assert message is not None return CommandContext( message=message, - recipient=REPLY_GROUP, + group_id=group, transport=transport, store=store, config=config, @@ -61,7 +67,7 @@ async def test_cooldown_allows_first_use( async def test_cooldown_blocks_and_reports_remaining( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: - store.record_use("u1", "ping", 100.0, DAY) + 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 == [] @@ -71,7 +77,7 @@ async def test_cooldown_blocks_and_reports_remaining( async def test_cooldown_expires( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: - store.record_use("u1", "ping", 100.0, DAY) + 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"] @@ -80,7 +86,7 @@ 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("u1", "ping", 100.0, DAY) + 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) ) @@ -97,7 +103,7 @@ async def test_daily_limit_allows_under_the_cap( async def test_daily_limit_blocks_at_the_cap( store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] ) -> None: - store.record_use("u1", "ping", 100.0, DAY) + 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) ) @@ -147,7 +153,7 @@ async def test_vote_tally_resets_after_firing( gated = vote_gated(2, 900, "{votes}/{required}")(handler) await gated(_context(store, transport, config, uuid="u1")) await gated(_context(store, transport, config, uuid="u2")) - assert store.count_votes("ping", 0.0) == 0 + assert store.count_votes(WATCHED_GROUP, "ping", 0.0) == 0 async def test_gates_stack( @@ -157,3 +163,34 @@ async def test_gates_stack( await stacked(_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)) + assert runs == ["u1"] + + +async def test_votes_do_not_carry_between_groups( + store: Store, transport: FakeTransport, config: Config, handler: Handler, runs: list[str] +) -> 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)) + assert runs == [] + assert transport.texts == ["1/2", "1/2"] diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index bb7825b..3f315a2 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -10,7 +10,7 @@ import pytest from signal_bot.dispatcher import Dispatcher from signal_bot.registry import CommandContext, CommandRegistry -from tests.conftest import OTHER_GROUP, envelope +from tests.conftest import OTHER_GROUP, SECOND_GROUP, WATCHED_GROUP, envelope if TYPE_CHECKING: from signal_bot.config import Config @@ -50,7 +50,7 @@ def dispatcher( async def test_counts_a_plain_group_message(dispatcher: Dispatcher, store: Store) -> None: await dispatcher.handle(envelope(text="hello")) - assert store.tallies()[0].count == 1 + assert store.tallies(WATCHED_GROUP)[0].count == 1 async def test_runs_a_command_and_records_the_use( @@ -58,8 +58,8 @@ async def test_runs_a_command_and_records_the_use( ) -> None: await dispatcher.handle(envelope(text="!cat")) assert runs == ["cat"] - assert store.last_used("uuid-alice", "cat") == NOW.timestamp() - assert store.uses_today("uuid-alice", "cat", "2026-08-29") == 1 + assert store.last_used(WATCHED_GROUP, "uuid-alice", "cat") == NOW.timestamp() + assert store.uses_today(WATCHED_GROUP, "uuid-alice", "cat", "2026-08-29") == 1 async def test_direct_messages_are_never_handled( @@ -68,7 +68,7 @@ async def test_direct_messages_are_never_handled( """A linked device sees every DM on the account; none may reach a handler.""" await dispatcher.handle(envelope(text="!cat", group_id=None)) assert runs == [] - assert store.tallies() == [] + assert store.tallies(WATCHED_GROUP) == [] assert transport.sent == [] @@ -77,22 +77,22 @@ async def test_other_groups_are_never_handled( ) -> None: await dispatcher.handle(envelope(text="!cat", group_id=OTHER_GROUP)) assert runs == [] - assert store.tallies() == [] + assert store.tallies(WATCHED_GROUP) == [] async def test_unparseable_frames_are_dropped(dispatcher: Dispatcher, store: Store) -> None: await dispatcher.handle("not json") - assert store.tallies() == [] + assert store.tallies(WATCHED_GROUP) == [] async def test_stickers_are_not_counted(dispatcher: Dispatcher, store: Store) -> None: await dispatcher.handle(envelope(text="hi", sticker=True, sync=True)) - assert store.tallies() == [] + assert store.tallies(WATCHED_GROUP) == [] async def test_reactions_are_not_counted(dispatcher: Dispatcher, store: Store) -> None: await dispatcher.handle(envelope(text="hi", reaction=True)) - assert store.tallies() == [] + assert store.tallies(WATCHED_GROUP) == [] async def test_non_command_text_runs_nothing(dispatcher: Dispatcher, runs: list[str]) -> None: @@ -118,9 +118,34 @@ async def test_a_failing_command_does_not_kill_the_loop( ) await dispatcher.handle(envelope(text="!cat")) # The use is not recorded, so the cooldown does not punish a failed run. - assert store.last_used("uuid-alice", "cat") is None + assert store.last_used(WATCHED_GROUP, "uuid-alice", "cat") is None async def test_command_message_is_also_counted(dispatcher: Dispatcher, store: Store) -> None: await dispatcher.handle(envelope(text="!cat")) - assert store.tallies()[0].count == 1 + assert store.tallies(WATCHED_GROUP)[0].count == 1 + + +async def test_a_reply_goes_back_to_the_group_that_asked( + config: Config, store: Store, transport: FakeTransport +) -> None: + """The multi-group invariant: never answer in the other watched group.""" + + async def answer(ctx: CommandContext) -> None: + await ctx.reply("pong") + + registry = CommandRegistry() + registry.register("cat", ["!cat"], answer) + dispatcher = Dispatcher( + config=config, registry=registry, store=store, transport=transport, clock=lambda: NOW + ) + await dispatcher.handle(envelope(text="!cat", group_id=SECOND_GROUP)) + assert [m.recipient for m in transport.sent] == [SECOND_GROUP] + + +async def test_counts_are_kept_per_group(dispatcher: Dispatcher, store: Store) -> None: + await dispatcher.handle(envelope(text="hello", group_id=WATCHED_GROUP)) + await dispatcher.handle(envelope(text="hello", group_id=SECOND_GROUP)) + await dispatcher.handle(envelope(text="hello", group_id=SECOND_GROUP)) + assert store.tallies(WATCHED_GROUP)[0].count == 1 + assert store.tallies(SECOND_GROUP)[0].count == 2 diff --git a/tests/test_main.py b/tests/test_main.py index 13978d7..148b957 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -17,8 +17,7 @@ from signal_bot.portcheck import PortInUseError BASE_ENV = { "PHONE_NUMBER": "+15550000000", - "GROUP_ID": "watched==", - "GROUP_ID_SEND": "reply==", + "GROUP_IDS": "group.watched==", } @@ -33,7 +32,7 @@ def _isolated_env(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("signal_bot.config.load_dotenv", lambda *a, **k: None) for key, value in BASE_ENV.items(): monkeypatch.setenv(key, value) - for key in ("BIND_HOST", "BIND_PORT"): + for key in ("BIND_HOST", "BIND_PORT", "GROUP_ID", "GROUP_ID_SEND"): monkeypatch.delenv(key, raising=False) diff --git a/tests/test_store.py b/tests/test_store.py index 2ec78e9..0107864 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -3,91 +3,161 @@ from __future__ import annotations +import sqlite3 from typing import TYPE_CHECKING -from signal_bot.store import Store, open_store +from signal_bot.store import SCHEMA_VERSION, Store, open_store if TYPE_CHECKING: from pathlib import Path +GROUP = "group-a==" +OTHER = "group-b==" + def test_increment_creates_and_accumulates(store: Store) -> None: - store.increment("u1", "Alice") - store.increment("u1", "Alice") - store.increment("u2", "Bob") - assert [(t.display_name, t.count) for t in store.tallies()] == [("Alice", 2), ("Bob", 1)] + store.increment(GROUP, "u1", "Alice") + store.increment(GROUP, "u1", "Alice") + store.increment(GROUP, "u2", "Bob") + assert [(t.display_name, t.count) for t in store.tallies(GROUP)] == [("Alice", 2), ("Bob", 1)] def test_display_name_is_refreshed(store: Store) -> None: - store.increment("u1", "Alice") - store.increment("u1", "Alicja") - assert store.tallies()[0].display_name == "Alicja" + store.increment(GROUP, "u1", "Alice") + store.increment(GROUP, "u1", "Alicja") + assert store.tallies(GROUP)[0].display_name == "Alicja" def test_tallies_sorted_by_count_then_name(store: Store) -> None: - store.increment("u1", "Zoe") - store.increment("u2", "Adam") - assert [t.display_name for t in store.tallies()] == ["Adam", "Zoe"] + store.increment(GROUP, "u1", "Zoe") + store.increment(GROUP, "u2", "Adam") + assert [t.display_name for t in store.tallies(GROUP)] == ["Adam", "Zoe"] def test_reset_clears_everything(store: Store) -> None: - store.increment("u1", "Alice") - store.reset_tallies() - assert store.tallies() == [] + store.increment(GROUP, "u1", "Alice") + store.reset_tallies(GROUP) + assert store.tallies(GROUP) == [] def test_tallies_survive_a_restart(tmp_path: Path) -> None: """The old bot kept the day's tally in a module global.""" db = str(tmp_path / "bot.sqlite3") with open_store(db) as first: - first.increment("u1", "Alice") + first.increment(GROUP, "u1", "Alice") with open_store(db) as second: - assert second.tallies()[0].count == 1 + assert second.tallies(GROUP)[0].count == 1 def test_last_used_is_none_before_first_use(store: Store) -> None: - assert store.last_used("u1", "cat") is None + assert store.last_used(GROUP, "u1", "cat") is None def test_record_use_tracks_cooldown_and_daily(store: Store) -> None: - store.record_use("u1", "cat", 100.0, "2026-08-29") - assert store.last_used("u1", "cat") == 100.0 - assert store.uses_today("u1", "cat", "2026-08-29") == 1 - store.record_use("u1", "cat", 200.0, "2026-08-29") - assert store.last_used("u1", "cat") == 200.0 - assert store.uses_today("u1", "cat", "2026-08-29") == 2 + store.record_use(GROUP, "u1", "cat", 100.0, "2026-08-29") + assert store.last_used(GROUP, "u1", "cat") == 100.0 + assert store.uses_today(GROUP, "u1", "cat", "2026-08-29") == 1 + store.record_use(GROUP, "u1", "cat", 200.0, "2026-08-29") + assert store.last_used(GROUP, "u1", "cat") == 200.0 + assert store.uses_today(GROUP, "u1", "cat", "2026-08-29") == 2 def test_daily_counter_is_per_day(store: Store) -> None: - store.record_use("u1", "cat", 100.0, "2026-08-29") - assert store.uses_today("u1", "cat", "2026-08-30") == 0 + store.record_use(GROUP, "u1", "cat", 100.0, "2026-08-29") + assert store.uses_today(GROUP, "u1", "cat", "2026-08-30") == 0 def test_uses_today_is_per_user_and_command(store: Store) -> None: - store.record_use("u1", "cat", 100.0, "2026-08-29") - assert store.uses_today("u2", "cat", "2026-08-29") == 0 - assert store.uses_today("u1", "dog", "2026-08-29") == 0 + store.record_use(GROUP, "u1", "cat", 100.0, "2026-08-29") + assert store.uses_today(GROUP, "u2", "cat", "2026-08-29") == 0 + assert store.uses_today(GROUP, "u1", "dog", "2026-08-29") == 0 def test_votes_are_deduplicated_per_person(store: Store) -> None: - store.add_vote("traps", "u1", 100.0) - store.add_vote("traps", "u1", 101.0) - store.add_vote("traps", "u2", 102.0) - assert store.count_votes("traps", 0.0) == 2 + store.add_vote(GROUP, "traps", "u1", 100.0) + store.add_vote(GROUP, "traps", "u1", 101.0) + store.add_vote(GROUP, "traps", "u2", 102.0) + assert store.count_votes(GROUP, "traps", 0.0) == 2 def test_votes_outside_the_window_are_ignored(store: Store) -> None: - store.add_vote("cmd", "u1", 100.0) - assert store.count_votes("cmd", 150.0) == 0 + store.add_vote(GROUP, "cmd", "u1", 100.0) + assert store.count_votes(GROUP, "cmd", 150.0) == 0 def test_clear_votes(store: Store) -> None: - store.add_vote("cmd", "u1", 100.0) - store.clear_votes("cmd") - assert store.count_votes("cmd", 0.0) == 0 + store.add_vote(GROUP, "cmd", "u1", 100.0) + store.clear_votes(GROUP, "cmd") + assert store.count_votes(GROUP, "cmd", 0.0) == 0 def test_store_is_a_context_manager(tmp_path: Path) -> None: with Store(str(tmp_path / "x.sqlite3")) as opened: - opened.increment("u1", "Alice") + opened.increment(GROUP, "u1", "Alice") assert opened.tallies.__self__ is opened + + +def test_state_is_scoped_per_group(store: Store) -> None: + """The whole point of the group key: two groups never share a counter.""" + store.increment(GROUP, "u1", "Alice") + store.increment(OTHER, "u1", "Alice") + store.increment(OTHER, "u1", "Alice") + assert store.tallies(GROUP)[0].count == 1 + assert store.tallies(OTHER)[0].count == 2 + + +def test_reset_clears_only_its_own_group(store: Store) -> None: + store.increment(GROUP, "u1", "Alice") + store.increment(OTHER, "u1", "Alice") + store.reset_tallies(GROUP) + assert store.tallies(GROUP) == [] + assert store.tallies(OTHER)[0].count == 1 + + +def test_rate_limits_are_scoped_per_group(store: Store) -> None: + store.record_use(GROUP, "u1", "cat", 100.0, "2026-08-29") + assert store.last_used(OTHER, "u1", "cat") is None + assert store.uses_today(OTHER, "u1", "cat", "2026-08-29") == 0 + + +def test_votes_are_scoped_per_group(store: Store) -> None: + store.add_vote(GROUP, "cmd", "u1", 100.0) + store.add_vote(OTHER, "cmd", "u2", 100.0) + store.clear_votes(GROUP, "cmd") + assert store.count_votes(GROUP, "cmd", 0.0) == 0 + assert store.count_votes(OTHER, "cmd", 0.0) == 1 + + +def test_a_pre_group_database_is_rebuilt(tmp_path: Path) -> None: + """CREATE TABLE IF NOT EXISTS is a no-op against the wrong columns. + + A database written before the group key would survive startup untouched + and then fail on every INSERT: a bot that boots clean and answers + nothing. PRAGMA user_version is the only part of the file that knows. + """ + db = str(tmp_path / "old.sqlite3") + legacy = sqlite3.connect(db) + legacy.execute( + "CREATE TABLE message_counts (uuid TEXT PRIMARY KEY, " + "display_name TEXT NOT NULL, count INTEGER NOT NULL DEFAULT 0)" + ) + legacy.execute("INSERT INTO message_counts VALUES ('u1', 'Alice', 7)") + legacy.commit() + legacy.close() + + with open_store(db) as store: + store.increment(GROUP, "u1", "Alice") + assert [(t.display_name, t.count) for t in store.tallies(GROUP)] == [("Alice", 1)] + + check = sqlite3.connect(db) + assert int(check.execute("PRAGMA user_version").fetchone()[0]) == SCHEMA_VERSION + check.close() + + +def test_a_current_database_is_left_alone(tmp_path: Path) -> None: + """Reopening must not wipe the day's tally on every restart.""" + db = str(tmp_path / "current.sqlite3") + with open_store(db) as first: + first.increment(GROUP, "u1", "Alice") + with open_store(db) as second: + assert second.tallies(GROUP)[0].count == 1 diff --git a/tests/test_transport_rest.py b/tests/test_transport_rest.py index 432f507..c1bd821 100644 --- a/tests/test_transport_rest.py +++ b/tests/test_transport_rest.py @@ -213,35 +213,3 @@ async def _no_sleep(seconds: float) -> None: def _groups(*entries: object) -> MockHandler: """Answer /v1/groups with an arbitrary payload.""" return lambda r: httpx.Response(200, json=list(entries)) - - -async def test_membership_accepts_a_group_matched_by_its_internal_id( - config: Config, -) -> None: - """Envelopes carry internal_id, so that spelling must satisfy the gate.""" - handler = _groups({"id": "group.other", "internal_id": config.group_id, "member": True}) - assert await _transport(config, handler).verify_membership() is None - - -async def test_membership_rejects_a_group_the_bot_was_only_invited_to( - config: Config, -) -> None: - handler = _groups({"id": config.group_id, "name": "testbot", "member": False}) - with pytest.raises(TransportError, match="has not joined"): - await _transport(config, handler).verify_membership() - - -async def test_membership_rejects_an_account_in_no_matching_group( - config: Config, -) -> None: - handler = _groups({"id": "group.someone-elses", "member": True}) - with pytest.raises(TransportError, match="in no group matching"): - await _transport(config, handler).verify_membership() - - -async def test_membership_ignores_entries_that_are_not_objects( - config: Config, -) -> None: - """A malformed row must not mask the real reason the bot is deaf.""" - handler = _groups("unexpected", {"id": config.group_id, "member": True}) - assert await _transport(config, handler).verify_membership() is None