From 72e90acb475c36d3182010d55777868b89dc6baa Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Fri, 4 Sep 2026 16:20:05 +0200 Subject: [PATCH] Let the operator add aliases from the Commands tab A custom alias is data the command table is built from, so it cannot be a live switch like the ones already on the tab. Instead the management page writes aliases_custom.json in the bot's state directory and then asks the bot to reload, in one step: whenever the answer is anything but `reloaded` -- a word another command has, a name no command has, a bot still connecting -- the file is restored byte for byte and the bot's own reason is shown next to the field. What is on disk is therefore always what the bot serves, and a typo cannot sit in the file poisoning every later reload. The registry appends the operator's words to the shipped list at registration, which keeps `!pomoc` on the shipped spelling and routes the new words through the same clash check as aliases.json. An alias for a name no command has fails the whole build (ConfigError -> `kept`), because a silently ignored entry is how a file drifts. The tagger treats custom aliases as reserved words too, so a person cannot be given one. registry.py crossed the 250-line cap and now re-exports the record and the error types from command.py; the bot's state directory lookup, previously duplicated in pack.py and the suggestions export, is state_dir.py. Proven end to end: !e2ekot is unknown, the file is written and reload.sh runs, !e2ekot posts a cat, a file naming a command that does not exist is refused with 409 and the previous table still answers. Co-Authored-By: Claude Opus 4.8 --- DOCS-commands.md | 14 ++- DOCS-reload.md | 18 ++- DOCS-web-ui.md | 29 +++-- fake_signal/proofs/alias_after.yaml | 7 ++ fake_signal/proofs/alias_before.yaml | 8 ++ fake_signal/proofs/alias_kept.yaml | 8 ++ scripts/e2e.sh | 2 + scripts/e2e_proofs.sh | 28 +++++ signal_bot/command.py | 104 +++++++++++++++++ signal_bot/custom_aliases.py | 98 ++++++++++++++++ signal_bot/manage/app.py | 2 + signal_bot/manage/deps.py | 10 ++ signal_bot/manage/routes_aliases.py | 103 +++++++++++++++++ signal_bot/manage/routes_suggestions.py | 19 +--- signal_bot/pack.py | 14 +-- signal_bot/pack_people.py | 15 ++- signal_bot/registry.py | 145 ++++++++---------------- signal_bot/state_dir.py | 30 +++++ signal_bot/wiring.py | 11 +- tests/manage_fixture.py | 1 + tests/test_custom_aliases.py | 83 ++++++++++++++ tests/test_manage_aliases.py | 100 ++++++++++++++++ tests/test_pack_people.py | 13 +++ tests/test_registry.py | 15 +++ tests/test_wiring_aliases.py | 75 ++++++++++++ tool/web.py | 2 + web/src/api/client.ts | 5 + web/src/api/types.ts | 14 +++ web/src/app.test.tsx | 3 +- web/src/test/fixtures.ts | 3 + web/src/views/commands-aliases.test.tsx | 75 ++++++++++++ web/src/views/commands-aliases.tsx | 60 ++++++++++ web/src/views/commands.test.tsx | 42 +++++-- web/src/views/commands.tsx | 36 ++++-- 34 files changed, 1030 insertions(+), 162 deletions(-) create mode 100644 fake_signal/proofs/alias_after.yaml create mode 100644 fake_signal/proofs/alias_before.yaml create mode 100644 fake_signal/proofs/alias_kept.yaml create mode 100644 signal_bot/command.py create mode 100644 signal_bot/custom_aliases.py create mode 100644 signal_bot/manage/routes_aliases.py create mode 100644 signal_bot/state_dir.py create mode 100644 tests/test_custom_aliases.py create mode 100644 tests/test_manage_aliases.py create mode 100644 tests/test_wiring_aliases.py create mode 100644 web/src/views/commands-aliases.test.tsx create mode 100644 web/src/views/commands-aliases.tsx diff --git a/DOCS-commands.md b/DOCS-commands.md index ce19dda..6c56715 100644 --- a/DOCS-commands.md +++ b/DOCS-commands.md @@ -46,6 +46,17 @@ just legal: it sits two edits from `!pies`, and passes solely because `!poem` is one edit away and wins the tie. Deleting `!poem` would fail the suite at `!poems`, nowhere near where the change was made. +The operator can add words of their own without a commit: the Commands tab of +the management page writes `aliases_custom.json` in the bot's state directory +(`CUSTOM_ALIASES_FILE` names another path), keyed by command name, and the +bot appends them to the shipped list when it builds the table. They are +extra spellings only -- `!pomoc` keeps showing the shipped one -- and they go +through the same clash check as `aliases.json`, so a word another command +already answers to is refused, as is a name no command has. That is what +makes the page's save a reload: the bot proves the file before it is kept. +The tagger refuses to give a person a trigger that is already somebody's +custom alias, for the same reason it refuses the shipped ones. + ## A word nothing claims An unrecognised `!word` used to be dropped in silence, which made a typo @@ -93,7 +104,8 @@ outright or until a moment, and given its own cooldown. The bot reads the switch on every attempt, so it is live without a reload; a command that is off answers `BEEP BOOP !kot WYŁĄCZONE.` (or `… WYŁĄCZONE DO 14:01.`, Warsaw time) rather than going quiet, and the refusal neither starts the cooldown nor -counts as a use. `!pomoc` still lists it. `DOCS-web-ui.md` has the page. +counts as a use. `!pomoc` still lists it. `DOCS-web-ui.md` has the page, and +the same row is where a custom alias is typed. ## Why `!pomoc` shows the first alias and not a trigger diff --git a/DOCS-reload.md b/DOCS-reload.md index cfdf550..27af0cc 100644 --- a/DOCS-reload.md +++ b/DOCS-reload.md @@ -4,10 +4,10 @@ Two kinds of change reach the running bot, and they are handled differently. ## Data: `POST /_reload` -Aliases, an album's `people.json`/`tags.json`, and the `.env` and album -settings the management UI writes are *data*. The command table is built from -that data by one function (`signal_bot/wiring.py`), and `RegistryHolder` can -run it again: +Aliases (shipped, and the operator's own `aliases_custom.json`), an album's +`people.json`/`tags.json`, and the `.env` and album settings the management +UI writes are *data*. The command table is built from that data by one +function (`signal_bot/wiring.py`), and `RegistryHolder` can run it again: the new table is built whole, then swapped in with a single assignment. Nothing restarts, the websocket stays up, and no frame is lost or replayed. @@ -38,7 +38,9 @@ One thing the management UI writes needs neither: the per-command switches on its Commands tab (`/_commands`) live in the bot's own database and are read on every attempt, so they are live on the next message without a rebuild. They are also what a reload does *not* reset -- `command_settings` is a -permanent table. +permanent table. The aliases field on the same tab is the opposite case: it +is data, so the page performs the reload itself after writing the file, and +undoes the write if the answer is not `reloaded` (`DOCS-web-ui.md`). ## Code: `scripts/reload.sh --code` @@ -65,4 +67,8 @@ synthetic pack, the script runs, and the same process posts delta's photo. The route itself is covered in `tests/test_app.py`, the holder in `tests/test_registry_holder.py`, and the album case in `tests/test_bot.py`. The switches get the same bracket: `switch_off.yaml` and `switch_on.yaml` -around a `PUT /_commands/cat`, with no reload in between. +around a `PUT /_commands/cat`, with no reload in between. The operator's +aliases get a three-way one: `alias_before.yaml` (`!e2ekot` is nobody), the +file is written and `scripts/reload.sh` runs, `alias_after.yaml` (it posts a +cat), then a file naming a command that does not exist is refused with +`kept` and `alias_kept.yaml` shows the previous table still answering. diff --git a/DOCS-web-ui.md b/DOCS-web-ui.md index c076286..df3098e 100644 --- a/DOCS-web-ui.md +++ b/DOCS-web-ui.md @@ -140,19 +140,30 @@ right now, each with an on/off switch, an optional "off until" moment and a cooldown that overrides the bot's default for that one command. Every row has its own Save, because a switch is thrown for one command at a time. -Nothing here goes through `/_reload`. The bot keeps these in its own SQLite +No switch goes through `/_reload`. The bot keeps them in its own SQLite (`command_settings`) and reads the row on every attempt, so a switch is live -on the next message -- `DOCS-reload.md` is about the *table*, and this does -not touch the table. A command that is off answers `BEEP BOOP !kot WYŁĄCZONE` +on the next message -- `DOCS-reload.md` is about the *table*, and a switch +does not touch it. A command that is off answers `BEEP BOOP !kot WYŁĄCZONE` (with `DO HH:MM` for a timed switch) rather than staying silent, so nobody -reports it broken, and the refusal is filed under `refused_off` where the -Analytics tab counts it with the other refusals. A refusal does not start the -cooldown. +reports it broken; the refusal is filed under `refused_off`, counted by the +Analytics tab with the other refusals, and does not start the cooldown. The list is the bot's, fetched live, which is why the page says so when the bot is still connecting: there is no table to list until it has one. `!cisza` -and `!odcisz` have no cooldown to override and the field is disabled with -"none" in it, since a number typed there would be a lie the bot ignored. +and `!odcisz` have no cooldown to override, so the field is disabled with +"none" in it: a number typed there would be a lie the bot ignored. + +The one thing in a row that *does* go through `/_reload` is the aliases +field: the operator's own comma-separated trigger words for that command. +They are data the table is built from, so `PUT /api/aliases/{name}` writes +`aliases_custom.json` in the bot's state directory and then asks the bot to +rebuild. Write and reload are one step on purpose: whenever the answer is +anything but `reloaded` -- a word another command has, a name no command +has, a bot still connecting -- the file is put back byte for byte and the +bot's own sentence is shown next to the field, so what is on disk is always +what the bot serves and a typo cannot poison every later reload. A missing +`!` is added, a word with a space is refused before any write, and an empty +field removes the command's custom words. ## Ideas @@ -193,6 +204,8 @@ other path, so a reload on a sub-view works. | GET | `/api/suggestions` | Filed ideas and unrecognised-word tallies, bodies included. | | GET | `/api/commands` | Proxies the bot's `/_commands`: the live table with each command's switches. | | PUT | `/api/commands/{name}` | One command's switches, forwarded to the bot; its 404/503 come back unwrapped. | +| GET | `/api/aliases` | The operator's custom aliases, by command name; 409 if the file cannot be read. | +| PUT | `/api/aliases/{name}` | Writes the file and reloads the bot; anything but `reloaded` restores the file and passes the bot's reason on. | | GET | `/api/analytics/groups` | Every group with a command history, for the picker. | | GET | `/api/analytics/{overview,time,health,demand,behaviour,content}` | One per panel, all read-only, all taking an optional `?group=`. | | POST | `/api/suggestions/export` | Write those to a file **outside the repository**. | diff --git a/fake_signal/proofs/alias_after.yaml b/fake_signal/proofs/alias_after.yaml new file mode 100644 index 0000000..2fd56bf --- /dev/null +++ b/fake_signal/proofs/alias_after.yaml @@ -0,0 +1,7 @@ +# After the reload, the operator's spelling is !kot: the same process that +# just said "Nie znam" posts a cat. +name: "!e2ekot posts a cat once the alias file is live" +inject: + - { text: "!e2ekot", uuid: uuid-alias-after, name: AliasAfter } +expect: + - { kind: image, to: reply } diff --git a/fake_signal/proofs/alias_before.yaml b/fake_signal/proofs/alias_before.yaml new file mode 100644 index 0000000..6194a9d --- /dev/null +++ b/fake_signal/proofs/alias_before.yaml @@ -0,0 +1,8 @@ +# Not in scenarios/: the pair brackets the alias file that +# scripts/e2e_proofs.sh writes and reloads in between. Before it, !e2ekot +# is a word nobody registered. +name: "!e2ekot is unknown before the alias file exists" +inject: + - { text: "!e2ekot", uuid: uuid-alias, name: Alias } +expect: + - { kind: text, to: reply, contains: "Nie znam !e2ekot" } diff --git a/fake_signal/proofs/alias_kept.yaml b/fake_signal/proofs/alias_kept.yaml new file mode 100644 index 0000000..166ff6a --- /dev/null +++ b/fake_signal/proofs/alias_kept.yaml @@ -0,0 +1,8 @@ +# Replayed after a bad alias file is refused: the table that answered +# alias_after.yaml is the one still serving. Its own actor, because the +# cooldown lives in SQLite and alias_after's uuid is inside its window. +name: "!e2ekot still posts a cat while a bad alias file is refused" +inject: + - { text: "!e2ekot", uuid: uuid-alias-kept, name: AliasKept } +expect: + - { kind: image, to: reply } diff --git a/scripts/e2e.sh b/scripts/e2e.sh index d186115..80e6827 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -168,6 +168,7 @@ main() { POEM_TEXT_URL="${FAKE_URL}/_fixtures/poem-text?slug={slug}" \ PORTRAIT_CACHE_DIR="${TEMP_DIR}/portraits" \ PHOTO_PACK_DIR="${TEMP_DIR}/pack" \ + CUSTOM_ALIASES_FILE="${TEMP_DIR}/aliases_custom.json" \ BIND_PORT="$BOT_PORT" \ SILENCE_SECONDS=0 \ LIVENESS_TICK_SECONDS=1 \ @@ -184,6 +185,7 @@ main() { # prove_the_add_photo_invariants left with it; git has them at 64852ae. prove_the_table_reloads_without_a_restart prove_a_switch_is_live_without_a_reload + prove_an_alias_is_live_after_a_reload prove_the_deafness_gate enforce_the_time_budget "$STARTED_AT" } diff --git a/scripts/e2e_proofs.sh b/scripts/e2e_proofs.sh index b6fd3b7..9ddeb01 100755 --- a/scripts/e2e_proofs.sh +++ b/scripts/e2e_proofs.sh @@ -87,6 +87,34 @@ prove_a_switch_is_live_without_a_reload() { replay "${proofs}/switch_on.yaml" } +# The operator's alias file is data the table is built from, so it takes +# the same path as a tagged person: written, then scripts/reload.sh. The +# file is the one CUSTOM_ALIASES_FILE points the bot at, never the +# operator's own; a name the table does not hold is what the second half +# proves the bot refuses, with the old table still answering. +prove_an_alias_is_live_after_a_reload() { + local proofs="${REPO_ROOT}/fake_signal/proofs" + local file="${TEMP_DIR}/aliases_custom.json" + + echo "== before the alias, !e2ekot is nobody" + replay "${proofs}/alias_before.yaml" + + echo "== writing the operator's alias file, then scripts/reload.sh" + printf '{"cat": ["!e2ekot"]}\n' > "$file" + BIND_PORT="$BOT_PORT" "$REPO_ROOT/scripts/reload.sh" || + die "scripts/reload.sh did not get the alias live" + replay "${proofs}/alias_after.yaml" + + echo "== an alias for a command that does not exist keeps the old table" + printf '{"cat": ["!e2ekot"], "nosuch": ["!nic"]}\n' > "$file" + local answer + answer="$(curl -s -w '%{http_code}' -X POST "http://127.0.0.1:${BOT_PORT}/_reload")" + [[ "$answer" == *nosuch*409 ]] || + die "a bad alias file was not kept out with 409 and its name: ${answer}" + replay "${proofs}/alias_kept.yaml" + printf '{"cat": ["!e2ekot"]}\n' > "$file" +} + # A budget, not a benchmark. This run was four minutes until the liveness # tick became configurable, and nothing announced the cost -- slower is the # one regression a green suite never reports. Exceeding it fails the run; diff --git a/signal_bot/command.py b/signal_bot/command.py new file mode 100644 index 0000000..1e2cd25 --- /dev/null +++ b/signal_bot/command.py @@ -0,0 +1,104 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""What a command *is*: the record, its handler type, and the ways it says no. + +Split from ``registry.py`` so the lookup and the vocabulary each fit on one +screen; ``registry`` re-exports every name here, so callers see one module. +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable +from dataclasses import dataclass + +from signal_bot.context import CommandContext + +COMMAND_PREFIX = "!" + + +Handler = Callable[[CommandContext], Awaitable[None]] + +# Turns the words after a trigger into whatever the handler is allowed to see. +# For the pack that means pseudonym keys and nothing else: the raw remainder +# stops here, so no handler, log line or command_uses row ever holds a name +# somebody typed. A command without one gets the words unchanged, which is +# what every command before this feature did. +ArgResolver = Callable[[tuple[str, ...]], tuple[str, ...]] + + +@dataclass(frozen=True, slots=True) +class Command: + """A named command, every phrase that triggers it, and how to explain it.""" + + name: str + triggers: frozenset[str] + handler: Handler + # Group-facing Polish, one line, for !pomoc. Required at registration so + # that a command cannot exist without one -- a parallel help table would + # go stale exactly the way the README's command list did. + description: str + # The spelling !pomoc shows. It cannot come from `triggers`: those are + # folded skeletons in an unordered set, so "!papiez" is all that survives + # of "!papież". This is the first raw trigger instead, which is the + # canonical Polish one for every entry in aliases.json. + primary: str + # The only groups this command exists in, or None for everywhere. Not a + # gate inside the handler: the dispatcher treats a command that is not + # allowed here as no command at all, so it is never answered, never + # counted as a command and never logged by name in a group it does not + # belong to. A silent handler would still leak all three. + groups: frozenset[str] | None = None + # Whether words after the trigger are arguments rather than nonsense. + # Off by default, so "!cat blah" keeps resolving to nothing exactly as + # it does today. + takes_args: bool = False + # Applied to those words before they reach CommandContext.args. The + # scrubbing seam -- see ArgResolver. + arg_resolver: ArgResolver | None = None + + def allowed_in(self, group_id: str) -> bool: + """Whether this command exists in one group.""" + return self.groups is None or group_id in self.groups + + +class DuplicateTriggerError(ValueError): + """Raised when two commands that can both run claim the same trigger word. + + "Can both run" is the whole test. Two albums scoped to different groups + may each define "!alfa" -- they are different people, and no message can + ever mean both, because a command a group is not scoped to is not a + command there at all. A collision is only a collision where the group + sets overlap, and then it is fatal at startup rather than a coin toss. + """ + + +class CommandRefusedError(Exception): + """A gate declined the command, so nothing happened and no *use* 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. + + The refusal is now recorded in the command *history*, which is a different + table and a different question: how often a gate actually bites is what + says whether it is tuned right. ``gate`` names which one refused, because + "this command is refused 40% of the time" and "this command is popular" + are the two readings of the same raw count. + + It defaults to empty so that raising it bare stays legal -- every existing + raise site said only "no", and a required argument would turn a behaviour + that works into a signature change across three decorators. + """ + + def __init__(self, gate: str = "") -> None: + """Record which gate refused, if the caller says.""" + super().__init__(gate) + self.gate = gate + + +@dataclass(frozen=True, slots=True) +class Resolved: + """A command a message fired, and the words that followed it.""" + + command: Command + args: tuple[str, ...] diff --git a/signal_bot/custom_aliases.py b/signal_bot/custom_aliases.py new file mode 100644 index 0000000..e164b5a --- /dev/null +++ b/signal_bot/custom_aliases.py @@ -0,0 +1,98 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The operator's own trigger words, kept outside this repository. + +``aliases.json`` ships with the code and is public. The spellings one group +actually settles on -- a nickname for a person in the album, a pet name for +a command -- are not, and some of them are names. So they live in +``aliases_custom.json`` in the state directory, keyed by the command they +extend, and the registry appends them at build time: ``!pomoc`` keeps +printing the shipped spelling, and a reload is what makes an edit live. + +The file is written by the management page and read here; nothing in the +chat can add to it, because an alias is a command and a command is the +operator's to define. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +from typing import TYPE_CHECKING, Final + +from signal_bot.groups import ConfigError +from signal_bot.pack_store import write_json +from signal_bot.state_dir import state_dir + +if TYPE_CHECKING: + from collections.abc import Sequence + +FILE_NAME: Final = "aliases_custom.json" +# Where the e2e run points the bot, so a proof can write its own file +# without touching the operator's. +ENV_VAR: Final = "CUSTOM_ALIASES_FILE" + +Aliases = dict[str, tuple[str, ...]] + + +def default_path() -> Path: + """The file, from the environment or the state directory.""" + explicit = os.environ.get(ENV_VAR, "").strip() + return Path(explicit) if explicit else state_dir() / FILE_NAME + + +def normalise(triggers: Sequence[str]) -> tuple[str, ...]: + """Trimmed, bang-prefixed, deduplicated, in the order given. + + Raises: + ConfigError: A trigger has spaces in it. One word is all a message + can be matched by, so a longer one could never fire. + """ + seen: list[str] = [] + for raw in triggers: + word = raw.strip() + if not word: + continue + word = word if word.startswith("!") else f"!{word}" + if len(word.split()) != 1: + msg = f"a trigger is one word, not {raw.strip()!r}" + raise ConfigError(msg) + if word not in seen: + seen.append(word) + return tuple(seen) + + +def load(path: Path) -> Aliases: + """Every custom alias, by command name; an absent file is no aliases. + + Strict, unlike the album's audit files: a line here becomes a trigger, + and a trigger that half-loaded would be a command nobody can find. + + Raises: + ConfigError: The file cannot be read or is not the expected shape. + """ + if not path.exists(): + return {} + try: + document = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as error: + msg = f"{FILE_NAME} cannot be read: {error}" + raise ConfigError(msg) from error + if not isinstance(document, dict): + msg = f"{FILE_NAME} must be an object of command name to triggers" + raise ConfigError(msg) + aliases: Aliases = {} + for name, triggers in document.items(): + if not isinstance(triggers, list) or not all(isinstance(t, str) for t in triggers): + msg = f"{FILE_NAME}: {name!r} must map to a list of strings" + raise ConfigError(msg) + words = normalise(triggers) + if words: + aliases[str(name)] = words + return aliases + + +def store(path: Path, aliases: Aliases) -> None: + """Write the whole table, dropping commands left with no alias.""" + path.parent.mkdir(parents=True, exist_ok=True) + write_json(path, {name: list(words) for name, words in aliases.items() if words}) diff --git a/signal_bot/manage/app.py b/signal_bot/manage/app.py index 731ffe0..3e39004 100644 --- a/signal_bot/manage/app.py +++ b/signal_bot/manage/app.py @@ -23,6 +23,7 @@ from signal_bot.manage import ( guard, routes_actions, routes_album, + routes_aliases, routes_analytics, routes_commands, routes_env, @@ -55,5 +56,6 @@ def build_app(deps: ManageDeps, *, container: str = DEFAULT_CONTAINER) -> FastAP app.include_router(routes_suggestions.router(deps)) app.include_router(routes_analytics.router(deps)) app.include_router(routes_commands.router(deps)) + app.include_router(routes_aliases.router(deps)) assets.install(app, deps.dist_dir) return app diff --git a/signal_bot/manage/deps.py b/signal_bot/manage/deps.py index 1a9a532..ec3589a 100644 --- a/signal_bot/manage/deps.py +++ b/signal_bot/manage/deps.py @@ -55,6 +55,16 @@ class ManageDeps: misspelled names this particular file is full of. """ + aliases_path: Path + """The operator's own trigger words, ``custom_aliases.FILE_NAME``. + + Written here and read by the bot at every reload, so it has to be the + same path both processes resolve -- which is why tool/web.py takes it + from ``custom_aliases.default_path`` rather than naming a file of its + own. Required for the same reason database_path is: a default would + point the test suite at the operator's real file. + """ + client: httpx.AsyncClient """Shared, so the proxied /healthz and the group listing pool sockets.""" diff --git a/signal_bot/manage/routes_aliases.py b/signal_bot/manage/routes_aliases.py new file mode 100644 index 0000000..a735329 --- /dev/null +++ b/signal_bot/manage/routes_aliases.py @@ -0,0 +1,103 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The operator's aliases: one file, written here, proven by the bot. + +An alias is data the command table is built from, so unlike a switch it +goes through ``POST /_reload``. The write and the reload are one step on +purpose: the file is put back exactly as it was whenever the bot does not +come back ``reloaded``, so what is on disk is always what the bot serves. +Otherwise a typo'd name would sit in the file, refused at every later +reload of anything else, until somebody worked out which edit did it. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +import httpx +from fastapi import APIRouter, HTTPException, status +from pydantic import BaseModel + +from signal_bot.custom_aliases import load, normalise, store +from signal_bot.groups import ConfigError + +if TYPE_CHECKING: + from pathlib import Path + + from signal_bot.manage.deps import ManageDeps + + +class AliasRequest(BaseModel): + """Every custom trigger one command should have; empty removes them all.""" + + triggers: list[str] + + +def _restore(path: Path, previous: bytes | None) -> None: + """Put the file back as it was before the write, byte for byte.""" + if previous is None: + path.unlink(missing_ok=True) + else: + path.write_bytes(previous) + + +def router(deps: ManageDeps) -> APIRouter: + """The table and the per-command PUT. + + Args: + deps: The running app's configuration and shared HTTP client. + + Returns: + A router to include in the management app. + """ + api = APIRouter(prefix="/api") + + def current() -> dict[str, tuple[str, ...]]: + """The file as it stands, or the reason it cannot be read (409).""" + try: + return load(deps.aliases_path) + except ConfigError as error: + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(error)) from error + + async def reload() -> dict[str, Any]: + """Ask the bot to rebuild its table; anything but ``reloaded`` raises.""" + try: + response = await deps.client.post(f"{deps.bot_url}/_reload", timeout=30.0) + except httpx.HTTPError as error: + raise HTTPException( + status_code=status.HTTP_502_BAD_GATEWAY, + detail=f"the bot did not answer: {error}", + ) from error + body: dict[str, Any] = response.json() + if response.status_code != status.HTTP_200_OK: + # ``kept`` (409) carries the bot's own sentence about the data, + # ``connecting`` (503) why there is no table yet. Both are the + # bot's words, passed on. + raise HTTPException(status_code=response.status_code, detail=body.get("reason", body)) + return body + + @api.get("/aliases") + async def list_aliases() -> dict[str, dict[str, list[str]]]: + """Every custom alias, by command name.""" + return {"aliases": {name: list(words) for name, words in current().items()}} + + @api.put("/aliases/{name}") + async def put_aliases(name: str, request: AliasRequest) -> dict[str, Any]: + """Set one command's custom triggers and make them live, or change nothing.""" + try: + words = normalise(request.triggers) + except ConfigError as error: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, detail=str(error) + ) from error + aliases = current() + path = deps.aliases_path + previous = path.read_bytes() if path.exists() else None + store(path, {**aliases, name: words}) + try: + answer = await reload() + except HTTPException: + _restore(path, previous) + raise + return {"name": name, "triggers": list(words), **answer} + + return api diff --git a/signal_bot/manage/routes_suggestions.py b/signal_bot/manage/routes_suggestions.py index bd9b93f..58e5604 100644 --- a/signal_bot/manage/routes_suggestions.py +++ b/signal_bot/manage/routes_suggestions.py @@ -15,16 +15,17 @@ The export deliberately does **not** write into the repository. See from __future__ import annotations import contextlib -import os import sqlite3 -from pathlib import Path from typing import TYPE_CHECKING, Any, Final from fastapi import APIRouter +from signal_bot.state_dir import state_dir from signal_bot.suggestions import SuggestionStore if TYPE_CHECKING: + from pathlib import Path + from signal_bot.manage.deps import ManageDeps # Named for the repository's markdown convention, so that a copy pasted @@ -34,18 +35,8 @@ EXPORT_NAME: Final = "DOCS-suggestions.md" def default_export_path() -> Path: - """Where the export goes: the state directory, never the repository. - - Same order as pack.default_dir, and for the same reason -- systemd - creates STATE_DIRECTORY and exposes it regardless of ProtectHome, and - reading os.environ live is what lets one autouse fixture keep the whole - test suite out of the operator's real data. - """ - state = os.environ.get("STATE_DIRECTORY", "").strip() - if state: - return Path(state) / EXPORT_NAME - home = os.environ.get("XDG_DATA_HOME", "").strip() or str(Path.home() / ".local" / "share") - return Path(home) / "signal-bot" / EXPORT_NAME + """Where the export goes: the state directory, never the repository.""" + return state_dir() / EXPORT_NAME # What a body is truncated to in the export. Long enough for a sentence, diff --git a/signal_bot/pack.py b/signal_bot/pack.py index d7683f5..827fd90 100644 --- a/signal_bot/pack.py +++ b/signal_bot/pack.py @@ -32,6 +32,7 @@ from typing import TYPE_CHECKING, Final from signal_bot.folding import fold from signal_bot.pack_custom import CustomTags +from signal_bot.state_dir import state_dir if TYPE_CHECKING: from collections.abc import Mapping, Sequence @@ -136,19 +137,8 @@ def _implicit_dir() -> tuple[Path, bool]: Explicit means a missing directory is an error worth failing startup for. The implicit paths are the opposite: they are where a pack *would* live, so their absence is simply an installation with no album. - - Read straight off os.environ rather than from a passed mapping, exactly - as portraits.default_cache_dir is: .env has already been loaded into the - environment by then, and it is what lets one autouse fixture keep the - whole test suite away from a real album. STATE_DIRECTORY comes before - anything under $HOME because that is the directory systemd creates and - exposes regardless of ProtectHome. """ - state = os.environ.get("STATE_DIRECTORY", "").strip() - if state: - return Path(state) / "pack", False - home = os.environ.get("XDG_DATA_HOME", "").strip() or str(Path.home() / ".local" / "share") - return Path(home) / "signal-bot" / "pack", False + return state_dir() / "pack", False def options_for(pack: Pack, token: str) -> tuple[frozenset[str], ...] | None: diff --git a/signal_bot/pack_people.py b/signal_bot/pack_people.py index b4db4f2..e58585c 100644 --- a/signal_bot/pack_people.py +++ b/signal_bot/pack_people.py @@ -33,6 +33,8 @@ import json from pathlib import Path from typing import TYPE_CHECKING, Any, Final +from signal_bot.custom_aliases import default_path as custom_path +from signal_bot.custom_aliases import load as load_custom from signal_bot.folding import fold from signal_bot.groups import ConfigError from signal_bot.pack import PEOPLE_FILE @@ -49,14 +51,21 @@ KEY_PREFIX: Final = "p" def reserved() -> frozenset[str]: - """Every folded word that is already one of the bot's own commands.""" + """Every folded word that is already one of the bot's own commands. + + The shipped aliases and the operator's own, because both become + triggers at the same table -- and the registry would refuse the clash + at the next reload, which is later and less clear than refusing here. + """ document = json.loads(ALIAS_FILE.read_text(encoding="utf-8")) - return frozenset( - fold(trigger) + shipped = ( + trigger for name, triggers in document.items() if not name.startswith("_") for trigger in triggers ) + custom = (trigger for words in load_custom(custom_path()).values() for trigger in words) + return frozenset(fold(trigger) for trigger in (*shipped, *custom)) def next_key(people: Sequence[dict[str, Any]]) -> str: diff --git a/signal_bot/registry.py b/signal_bot/registry.py index 88bb319..49c4c5a 100644 --- a/signal_bot/registry.py +++ b/signal_bot/registry.py @@ -3,118 +3,60 @@ Reimplements the idea behind the unmerged ``rules.py`` branch: triggers are data, not a chain of if-statements, so aliases (including the Unicode-styled -ones people actually type) cost one tuple entry each. +ones people actually type) cost one tuple entry each. The record itself and +its error types live in ``command.py`` and are re-exported here. """ from __future__ import annotations -from collections.abc import Awaitable, Callable, Iterable -from dataclasses import dataclass +from typing import TYPE_CHECKING -from signal_bot.context import CommandContext +from signal_bot.command import ( + COMMAND_PREFIX, + ArgResolver, + Command, + CommandRefusedError, + DuplicateTriggerError, + Handler, + Resolved, +) from signal_bot.folding import fold -COMMAND_PREFIX = "!" +if TYPE_CHECKING: + from collections.abc import Iterable, Mapping, Sequence - -Handler = Callable[[CommandContext], Awaitable[None]] - -# Turns the words after a trigger into whatever the handler is allowed to see. -# For the pack that means pseudonym keys and nothing else: the raw remainder -# stops here, so no handler, log line or command_uses row ever holds a name -# somebody typed. A command without one gets the words unchanged, which is -# what every command before this feature did. -ArgResolver = Callable[[tuple[str, ...]], tuple[str, ...]] - - -@dataclass(frozen=True, slots=True) -class Command: - """A named command, every phrase that triggers it, and how to explain it.""" - - name: str - triggers: frozenset[str] - handler: Handler - # Group-facing Polish, one line, for !pomoc. Required at registration so - # that a command cannot exist without one -- a parallel help table would - # go stale exactly the way the README's command list did. - description: str - # The spelling !pomoc shows. It cannot come from `triggers`: those are - # folded skeletons in an unordered set, so "!papiez" is all that survives - # of "!papież". This is the first raw trigger instead, which is the - # canonical Polish one for every entry in aliases.json. - primary: str - # The only groups this command exists in, or None for everywhere. Not a - # gate inside the handler: the dispatcher treats a command that is not - # allowed here as no command at all, so it is never answered, never - # counted as a command and never logged by name in a group it does not - # belong to. A silent handler would still leak all three. - groups: frozenset[str] | None = None - # Whether words after the trigger are arguments rather than nonsense. - # Off by default, so "!cat blah" keeps resolving to nothing exactly as - # it does today. - takes_args: bool = False - # Applied to those words before they reach CommandContext.args. The - # scrubbing seam -- see ArgResolver. - arg_resolver: ArgResolver | None = None - - def allowed_in(self, group_id: str) -> bool: - """Whether this command exists in one group.""" - return self.groups is None or group_id in self.groups - - -class DuplicateTriggerError(ValueError): - """Raised when two commands that can both run claim the same trigger word. - - "Can both run" is the whole test. Two albums scoped to different groups - may each define "!alfa" -- they are different people, and no message can - ever mean both, because a command a group is not scoped to is not a - command there at all. A collision is only a collision where the group - sets overlap, and then it is fatal at startup rather than a coin toss. - """ - - -class CommandRefusedError(Exception): - """A gate declined the command, so nothing happened and no *use* 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. - - The refusal is now recorded in the command *history*, which is a different - table and a different question: how often a gate actually bites is what - says whether it is tuned right. ``gate`` names which one refused, because - "this command is refused 40% of the time" and "this command is popular" - are the two readings of the same raw count. - - It defaults to empty so that raising it bare stays legal -- every existing - raise site said only "no", and a required argument would turn a behaviour - that works into a signature change across three decorators. - """ - - def __init__(self, gate: str = "") -> None: - """Record which gate refused, if the caller says.""" - super().__init__(gate) - self.gate = gate - - -@dataclass(frozen=True, slots=True) -class Resolved: - """A command a message fired, and the words that followed it.""" - - command: Command - args: tuple[str, ...] +__all__ = [ + "COMMAND_PREFIX", + "ArgResolver", + "Command", + "CommandRefusedError", + "CommandRegistry", + "DuplicateTriggerError", + "Handler", + "Resolved", +] class CommandRegistry: """Maps trigger words to commands.""" - def __init__(self) -> None: - """Start with no commands registered.""" + def __init__(self, extra: Mapping[str, Sequence[str]] | None = None) -> None: + """Start with no commands registered. + + ``extra`` is the operator's own aliases, by command name: appended to + whatever a command registers with, so ``primary`` -- the spelling + !pomoc prints -- stays the shipped one. Applied here rather than by + each register_* call because a name is the one thing every command + has, and the one thing this table checks for clashes anyway. + """ # One trigger can be claimed by several commands, as long as no two # of them are ever live in the same group. See DuplicateTriggerError. self._by_trigger: dict[str, list[Command]] = {} self._commands: list[Command] = [] + self._extra: dict[str, tuple[str, ...]] = { + name: tuple(words) for name, words in (extra or {}).items() + } + self._unclaimed: set[str] = set(self._extra) def register( self, @@ -132,7 +74,8 @@ class CommandRegistry: Raises: DuplicateTriggerError: A trigger is already claimed. """ - raw = list(triggers) + raw = [*triggers, *self._extra.get(name, ())] + self._unclaimed.discard(name) normalised = frozenset(fold(t) for t in raw) for trigger in sorted(normalised): clash = next( @@ -219,6 +162,16 @@ class CommandRegistry: """Every registered command, in registration order.""" return list(self._commands) + @property + def unclaimed_aliases(self) -> frozenset[str]: + """Names in ``extra`` that nothing registered under. + + An alias for a command that does not exist is a typo, or a person + no longer in the album; either way it is the operator's mistake to + hear about, not a line to drop quietly. + """ + return frozenset(self._unclaimed) + def triggers_in(self, group_id: str) -> dict[str, Command]: """Every folded trigger live in one group, mapped to its command. diff --git a/signal_bot/state_dir.py b/signal_bot/state_dir.py new file mode 100644 index 0000000..a9c3c12 --- /dev/null +++ b/signal_bot/state_dir.py @@ -0,0 +1,30 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""Where the bot keeps what is not in this repository. + +One answer for every file that has to live outside the tree -- the album, +the suggestions export, the operator's own aliases -- so that the bot and +the management page, two processes reading the same environment, cannot +disagree about where a file is. +""" + +from __future__ import annotations + +import os +from pathlib import Path + + +def state_dir() -> Path: + """The directory for data this repository never sees. + + Read straight off os.environ rather than from a passed mapping, exactly + as portraits.default_cache_dir is: .env has already been loaded into the + environment by then, and it is what lets one autouse fixture keep the + whole test suite away from the operator's real data. STATE_DIRECTORY + comes before anything under $HOME because that is the directory systemd + creates and exposes regardless of ProtectHome. + """ + state = os.environ.get("STATE_DIRECTORY", "").strip() + if state: + return Path(state) + home = os.environ.get("XDG_DATA_HOME", "").strip() or str(Path.home() / ".local" / "share") + return Path(home) / "signal-bot" diff --git a/signal_bot/wiring.py b/signal_bot/wiring.py index e953897..9388cae 100644 --- a/signal_bot/wiring.py +++ b/signal_bot/wiring.py @@ -16,7 +16,10 @@ from typing import TYPE_CHECKING, Final from signal_bot.analytics_command import register_analytics_command from signal_bot.animals import load_aliases, register_animal_commands from signal_bot.art_command import register_art_command +from signal_bot.custom_aliases import default_path as custom_aliases_path +from signal_bot.custom_aliases import load as load_custom_aliases from signal_bot.decorators import cooldown +from signal_bot.groups import ConfigError from signal_bot.help_command import register_help_command from signal_bot.mute import UNMUTE_NAME from signal_bot.mute_command import MUTE_NAME, register_mute_commands @@ -68,10 +71,11 @@ def build_registry( """Read the data and register every command on a fresh table. Raises: - ConfigError: An album no longer loads. + ConfigError: An album no longer loads, the operator's alias file does + not, or it names a command that does not exist. DuplicateTriggerError: Two commands live in one group claim a word. """ - registry = CommandRegistry() + registry = CommandRegistry(load_custom_aliases(custom_aliases_path())) rate_limit = cooldown(config.cooldown_seconds, COOLDOWN_MESSAGE) register_animal_commands( registry, @@ -104,4 +108,7 @@ def build_registry( # Last, so !pomoc trails the list it prints; it reads the registry # when it runs, so the order is presentation only. register_help_command(registry, decorate=rate_limit) + if registry.unclaimed_aliases: + msg = f"aliases for commands that do not exist: {sorted(registry.unclaimed_aliases)}" + raise ConfigError(msg) return registry diff --git a/tests/manage_fixture.py b/tests/manage_fixture.py index eda035b..553598a 100644 --- a/tests/manage_fixture.py +++ b/tests/manage_fixture.py @@ -113,6 +113,7 @@ def deps_at( # then open and, on a version mismatch, partly drop. database_path=tmp_path / "signal_bot.sqlite3", export_path=tmp_path / "export" / "DOCS-suggestions.md", + aliases_path=tmp_path / "aliases_custom.json", client=httpx.AsyncClient(transport=httpx.MockTransport(handler)), port=PORT, ) diff --git a/tests/test_custom_aliases.py b/tests/test_custom_aliases.py new file mode 100644 index 0000000..12b1390 --- /dev/null +++ b/tests/test_custom_aliases.py @@ -0,0 +1,83 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The operator's alias file: where it is, what it accepts, what it refuses.""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import pytest + +from signal_bot.custom_aliases import ENV_VAR, FILE_NAME, default_path, load, normalise, store +from signal_bot.groups import ConfigError +from signal_bot.state_dir import state_dir + +if TYPE_CHECKING: + from pathlib import Path + + +def test_the_file_sits_in_the_state_directory_unless_named( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.delenv(ENV_VAR, raising=False) + assert default_path() == state_dir() / FILE_NAME + monkeypatch.setenv(ENV_VAR, str(tmp_path / "elsewhere.json")) + assert default_path() == tmp_path / "elsewhere.json" + + +def test_the_state_directory_is_systemds_first_then_xdg_then_home( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.setenv("STATE_DIRECTORY", str(tmp_path / "state")) + assert state_dir() == tmp_path / "state" + monkeypatch.delenv("STATE_DIRECTORY") + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "share")) + assert state_dir() == tmp_path / "share" / "signal-bot" + monkeypatch.delenv("XDG_DATA_HOME") + monkeypatch.setenv("HOME", str(tmp_path / "home")) + assert state_dir() == tmp_path / "home" / ".local" / "share" / "signal-bot" + + +def test_normalise_trims_bangs_and_deduplicates_in_order() -> None: + assert normalise([" kotek ", "!kotek", "", "!Kicia", "kicia"]) == ("!kotek", "!Kicia", "!kicia") + + +def test_normalise_refuses_a_trigger_of_more_than_one_word() -> None: + with pytest.raises(ConfigError, match="one word"): + normalise(["!kot ładny"]) + + +def test_an_absent_file_is_no_aliases(tmp_path: Path) -> None: + assert load(tmp_path / FILE_NAME) == {} + + +def test_store_and_load_round_trip_and_drop_emptied_commands(tmp_path: Path) -> None: + path = tmp_path / "deep" / FILE_NAME + store(path, {"cat": ("!kotek",), "dog": ()}) + assert load(path) == {"cat": ("!kotek",)} + assert path.stat().st_mode & 0o777 == 0o600 + + +@pytest.mark.parametrize( + ("text", "complaint"), + [ + ("{not json", "cannot be read"), + ('["!kotek"]', "must be an object"), + ('{"cat": "!kotek"}', "list of strings"), + ('{"cat": [1]}', "list of strings"), + ('{"cat": ["!kot ładny"]}', "one word"), + ], +) +def test_a_file_the_bot_could_not_honour_is_refused_whole( + tmp_path: Path, text: str, complaint: str +) -> None: + path = tmp_path / FILE_NAME + path.write_text(text, encoding="utf-8") + with pytest.raises(ConfigError, match=complaint): + load(path) + + +def test_a_command_left_with_no_words_is_not_an_alias(tmp_path: Path) -> None: + path = tmp_path / FILE_NAME + path.write_text(json.dumps({"cat": [" "], "dog": ["!piesek"]}), encoding="utf-8") + assert load(path) == {"dog": ("!piesek",)} diff --git a/tests/test_manage_aliases.py b/tests/test_manage_aliases.py new file mode 100644 index 0000000..fa3b83e --- /dev/null +++ b/tests/test_manage_aliases.py @@ -0,0 +1,100 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The alias editor: written to one file, made live by the bot, or undone.""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import httpx +import pytest + +from signal_bot.custom_aliases import load +from tests.manage_fixture import bot_is_down, client_for, deps_at + +if TYPE_CHECKING: + from pathlib import Path + +RELOADED = {"status": "reloaded", "commands": 24} + + +def _bot_that_reloads(request: httpx.Request) -> httpx.Response: + assert request.method == "POST" + assert request.url.path == "/_reload" + return httpx.Response(200, json=RELOADED) + + +def test_the_table_starts_empty_and_a_put_fills_it(tmp_path: Path) -> None: + deps = deps_at(tmp_path, _bot_that_reloads) + with client_for(deps) as client: + assert client.get("/api/aliases").json() == {"aliases": {}} + answer = client.put("/api/aliases/cat", json={"triggers": ["kotek", "!kicia", "kotek"]}) + listed = client.get("/api/aliases").json() + assert answer.status_code == 200 + assert answer.json() == {"name": "cat", "triggers": ["!kotek", "!kicia"], **RELOADED} + assert listed == {"aliases": {"cat": ["!kotek", "!kicia"]}} + assert load(deps.aliases_path) == {"cat": ("!kotek", "!kicia")} + + +def test_an_empty_list_removes_the_command_from_the_file(tmp_path: Path) -> None: + deps = deps_at(tmp_path, _bot_that_reloads) + with client_for(deps) as client: + client.put("/api/aliases/cat", json={"triggers": ["!kotek"]}) + client.put("/api/aliases/dog", json={"triggers": ["!piesek"]}) + answer = client.put("/api/aliases/cat", json={"triggers": []}) + assert answer.status_code == 200 + assert load(deps.aliases_path) == {"dog": ("!piesek",)} + + +def test_a_trigger_of_several_words_is_refused_before_anything_is_written( + tmp_path: Path, +) -> None: + deps = deps_at(tmp_path, _bot_that_reloads) + with client_for(deps) as client: + answer = client.put("/api/aliases/cat", json={"triggers": ["!kot ładny"]}) + assert answer.status_code == 400 + assert "one word" in answer.json()["detail"] + assert not deps.aliases_path.exists() + + +@pytest.mark.parametrize( + ("code", "body", "expected"), + [ + (409, {"status": "kept", "commands": 24, "reason": "trigger '!kot' is taken"}, "taken"), + (503, {"status": "connecting", "reason": "attempt 2: refused"}, "attempt 2"), + ], +) +def test_a_reload_the_bot_refuses_puts_the_file_back( + tmp_path: Path, code: int, body: dict[str, object], expected: str +) -> None: + def bot(_request: httpx.Request) -> httpx.Response: + return httpx.Response(code, json=body) + + deps = deps_at(tmp_path, bot) + before = json.dumps({"dog": ["!piesek"]}) + deps.aliases_path.write_text(before, encoding="utf-8") + with client_for(deps) as client: + answer = client.put("/api/aliases/cat", json={"triggers": ["!kot"]}) + assert answer.status_code == code + assert expected in answer.json()["detail"] + assert deps.aliases_path.read_text(encoding="utf-8") == before + + +def test_a_bot_that_does_not_answer_leaves_no_file_behind(tmp_path: Path) -> None: + deps = deps_at(tmp_path, bot_is_down) + with client_for(deps) as client: + answer = client.put("/api/aliases/cat", json={"triggers": ["!kotek"]}) + assert answer.status_code == 502 + assert not deps.aliases_path.exists() + + +def test_a_file_edited_by_hand_into_nonsense_is_reported_not_hidden(tmp_path: Path) -> None: + deps = deps_at(tmp_path, _bot_that_reloads) + deps.aliases_path.write_text("{oops", encoding="utf-8") + with client_for(deps) as client: + listed = client.get("/api/aliases") + put = client.put("/api/aliases/cat", json={"triggers": ["!kotek"]}) + assert listed.status_code == 409 + assert "cannot be read" in listed.json()["detail"] + assert put.status_code == 409 + assert deps.aliases_path.read_text(encoding="utf-8") == "{oops" diff --git a/tests/test_pack_people.py b/tests/test_pack_people.py index 4090326..7763cc5 100644 --- a/tests/test_pack_people.py +++ b/tests/test_pack_people.py @@ -8,6 +8,7 @@ from typing import TYPE_CHECKING, Any import pytest +from signal_bot.custom_aliases import ENV_VAR as CUSTOM_ALIASES_ENV from signal_bot.groups import ConfigError from signal_bot.pack import PEOPLE_FILE from signal_bot.pack_custom import CUSTOM_FILE @@ -50,6 +51,18 @@ def test_the_bot_s_own_commands_are_known() -> None: assert not any(word.startswith("_") for word in reserved()) +def test_the_operators_own_aliases_are_reserved_too( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + custom = tmp_path / "aliases_custom.json" + custom.write_text(json.dumps({"cat": ["!zeta"]}), encoding="utf-8") + monkeypatch.setenv(CUSTOM_ALIASES_ENV, str(custom)) + assert "!zeta" in reserved() + directory = _roster(tmp_path, [{"key": "p1", "triggers": ["!alfa"]}]) + with pytest.raises(ConfigError, match="already a command"): + add_person(directory, ["zeta"]) + + def test_a_person_is_appended_with_a_stable_key(tmp_path: Path) -> None: directory = _roster(tmp_path, [{"key": "p1", "triggers": ["!alfa"]}]) assert add_person(directory, ["delta"]) == "p2" diff --git a/tests/test_registry.py b/tests/test_registry.py index f028291..74caeba 100644 --- a/tests/test_registry.py +++ b/tests/test_registry.py @@ -54,6 +54,21 @@ def test_duplicate_trigger_is_rejected() -> None: registry.register("feline", ["!cat"], _noop, "feline description") +def test_the_operators_aliases_are_appended_and_the_shipped_spelling_stays_primary() -> None: + registry = CommandRegistry({"cat": ["!kotek"], "nosuch": ["!x"]}) + command = registry.register("cat", ["!kot", "!cat"], _noop, "cat description") + assert registry.resolve("!kotek") is command + assert command.primary == "!kot" + assert registry.unclaimed_aliases == frozenset({"nosuch"}) + + +def test_an_operators_alias_that_is_another_commands_word_is_a_clash() -> None: + registry = CommandRegistry({"dog": ["!kot"]}) + registry.register("cat", ["!kot"], _noop, "cat description") + with pytest.raises(DuplicateTriggerError, match="'cat'"): + registry.register("dog", ["!pies"], _noop, "dog description") + + def test_commands_lists_in_registration_order() -> None: registry = CommandRegistry() registry.register("cat", ["!cat"], _noop, "cat description") diff --git a/tests/test_wiring_aliases.py b/tests/test_wiring_aliases.py new file mode 100644 index 0000000..ea27c9c --- /dev/null +++ b/tests/test_wiring_aliases.py @@ -0,0 +1,75 @@ +# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence. +"""The operator's alias file, read by the real table builder.""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import pytest + +from signal_bot.config import DEFAULT_COMMONS_API_URL +from signal_bot.custom_aliases import ENV_VAR +from signal_bot.groups import ConfigError +from signal_bot.portraits import PortraitCache +from signal_bot.registry import DuplicateTriggerError +from signal_bot.wiring import build_registry +from tests.test_help_command import _offline_fetcher + +if TYPE_CHECKING: + from pathlib import Path + + from signal_bot.config import Config + from signal_bot.registry import CommandRegistry + + +def _build(config: Config) -> CommandRegistry: + fetcher = _offline_fetcher() + return build_registry(config, fetcher, PortraitCache(fetcher, DEFAULT_COMMONS_API_URL)) + + +@pytest.fixture +def aliases_file(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Path: + path = tmp_path / "aliases_custom.json" + monkeypatch.setenv(ENV_VAR, str(path)) + return path + + +def test_without_a_file_the_table_is_the_shipped_one(config: Config, aliases_file: Path) -> None: + assert not aliases_file.exists() + assert _build(config).resolve("!kotek") is None + + +def test_an_alias_in_the_file_resolves_and_pomoc_keeps_the_shipped_spelling( + config: Config, aliases_file: Path +) -> None: + aliases_file.write_text(json.dumps({"cat": ["!kotek"]}), encoding="utf-8") + registry = _build(config) + command = registry.resolve("!kotek") + assert command is not None + assert command.name == "cat" + assert command.primary == "!kot" + + +def test_an_alias_for_a_command_that_does_not_exist_refuses_the_whole_table( + config: Config, aliases_file: Path +) -> None: + aliases_file.write_text(json.dumps({"pack:p9": ["!nobody"]}), encoding="utf-8") + with pytest.raises(ConfigError, match="pack:p9"): + _build(config) + + +def test_an_alias_that_is_already_a_word_is_the_usual_clash( + config: Config, aliases_file: Path +) -> None: + aliases_file.write_text(json.dumps({"dog": ["!kot"]}), encoding="utf-8") + with pytest.raises(DuplicateTriggerError, match="'cat'"): + _build(config) + + +def test_a_file_that_does_not_parse_is_bad_data_not_a_crash( + config: Config, aliases_file: Path +) -> None: + aliases_file.write_text("{oops", encoding="utf-8") + with pytest.raises(ConfigError, match="cannot be read"): + _build(config) diff --git a/tool/web.py b/tool/web.py index 261497b..a28e117 100755 --- a/tool/web.py +++ b/tool/web.py @@ -41,6 +41,7 @@ import httpx import uvicorn from signal_bot.config_env import load_config +from signal_bot.custom_aliases import default_path as default_aliases_path from signal_bot.manage.app import build_app from signal_bot.manage.deps import ManageDeps from signal_bot.manage.routes_suggestions import default_export_path @@ -105,6 +106,7 @@ def main() -> int: # repo root; joining an absolute one leaves it alone. database_path=REPO_ROOT / settings.database_path, export_path=default_export_path(), + aliases_path=default_aliases_path(), client=httpx.AsyncClient(), port=arguments.port, ) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 69d2461..a87dad1 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -10,6 +10,8 @@ import type { import type { Roster } from "./setup.ts"; import type { AlbumState, + Aliases, + AliasesSaved, Commands, CommandSetting, EnvironmentView, @@ -79,6 +81,7 @@ function scoped(path: string, group: null | string): string { export const api = { album: async (): Promise => get("/api/album/state"), + aliases: async (): Promise => get("/api/aliases"), analyticsBehaviour: async (group: null | string): Promise => get(scoped("/api/analytics/behaviour", group)), analyticsContent: async (group: null | string): Promise => @@ -102,6 +105,8 @@ export const api = { send<{ key: string }>("/api/album/person", "POST", { triggers }), restart: async (): Promise => send("/api/restart-container", "POST", {}), + saveAliases: async (name: string, triggers: string[]): Promise => + send(`/api/aliases/${encodeURIComponent(name)}`, "PUT", { triggers }), saveCommand: async (name: string, body: SettingBody): Promise => send(`/api/commands/${encodeURIComponent(name)}`, "PUT", body), saveEnv: async (values: Record): Promise => diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 29333d9..b03aa62 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -115,3 +115,17 @@ export interface SettingBody { disabled_until: null | number; enabled: boolean; } + +// The operator's own trigger words, by command name. Kept in a file the +// bot rebuilds its table from, so a save goes through a reload -- unlike a +// switch, which the bot reads live. +export interface Aliases { + aliases: Record; +} + +export interface AliasesSaved { + commands: number; + name: string; + status: string; + triggers: string[]; +} diff --git a/web/src/app.test.tsx b/web/src/app.test.tsx index 9aff353..a782f59 100644 --- a/web/src/app.test.tsx +++ b/web/src/app.test.tsx @@ -3,7 +3,7 @@ import userEvent from "@testing-library/user-event"; import { afterEach, expect, it, vi } from "vitest"; import { App } from "./app.tsx"; -import { ALBUM, COMMANDS, ENV, HEALTHY } from "./test/fixtures.ts"; +import { ALBUM, ALIASES, COMMANDS, ENV, HEALTHY } from "./test/fixtures.ts"; import { serve } from "./test/http.ts"; afterEach(() => { @@ -13,6 +13,7 @@ afterEach(() => { function everythingAnswers(): void { serve({ "/api/album/state": { body: ALBUM }, + "/api/aliases": { body: ALIASES }, "/api/commands": { body: COMMANDS }, "/api/env": { body: ENV }, "/api/status": { body: HEALTHY }, diff --git a/web/src/test/fixtures.ts b/web/src/test/fixtures.ts index c1f3bf3..a19b1e7 100644 --- a/web/src/test/fixtures.ts +++ b/web/src/test/fixtures.ts @@ -5,6 +5,7 @@ // names here are letters of the NATO alphabet, as tests/pack_fixture.py does. import type { AlbumState, + Aliases, Commands, EnvironmentView as EnvironmentView, Status, @@ -90,6 +91,8 @@ export const NO_SUGGESTIONS: SuggestionsView = { suggestions: [], unknown: [] }; // Two rows: one ordinary command on its defaults, and the mute pair's // representative, which wiring registers without a cooldown. +export const ALIASES: Aliases = { aliases: { cat: ["!kotek"] } }; + export const COMMANDS: Commands = { commands: [ { diff --git a/web/src/views/commands-aliases.test.tsx b/web/src/views/commands-aliases.test.tsx new file mode 100644 index 0000000..a8e8282 --- /dev/null +++ b/web/src/views/commands-aliases.test.tsx @@ -0,0 +1,75 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, expect, it, vi } from "vitest"; + +import { serve } from "../test/http.ts"; +import { AliasField, wordsOf } from "./commands-aliases.tsx"; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +const RELOADED = { commands: 28, name: "cat", status: "reloaded", triggers: ["!kotek", "!kicia"] }; + +function field(initial: string[] = []): void { + render(); +} + +it("splits on commas and whitespace alike, and keeps no empty word", () => { + expect(wordsOf("!kotek, !kicia")).toEqual(["!kotek", "!kicia"]); + expect(wordsOf(" kotek kicia ,, ")).toEqual(["kotek", "kicia"]); + expect(wordsOf("")).toEqual([]); +}); + +it("shows the words the command already has, on one line", () => { + field(["!kotek", "!kicia"]); + expect(screen.getByLabelText("!kot aliases")).toHaveValue("!kotek, !kicia"); +}); + +it("sends the words typed and shows the line the bot took, with its count", async () => { + const fetcher = serve({ "/api/aliases/cat": { body: RELOADED } }); + field(); + await userEvent.type(screen.getByLabelText("!kot aliases"), "kotek kicia"); + await userEvent.click(screen.getByRole("button", { name: "Aliases !kot" })); + await waitFor(() => { + expect(screen.getByTestId("aliases-note-cat")).toHaveTextContent("reloaded, 28 commands"); + }); + const [url, init] = fetcher.mock.calls[0] as [string, RequestInit]; + expect(url).toBe("/api/aliases/cat"); + expect(init.method).toBe("PUT"); + expect(JSON.parse(init.body as string)).toEqual({ triggers: ["kotek", "kicia"] }); + // The bot's spelling wins: the "!" it added is what the field shows now. + expect(screen.getByLabelText("!kot aliases")).toHaveValue("!kotek, !kicia"); +}); + +it("escapes a command name with a colon in it, as the pack's have", async () => { + const fetcher = serve({ "/api/aliases/pack%3Ap1": { body: { ...RELOADED, name: "pack:p1" } } }); + render(); + await userEvent.click(screen.getByRole("button", { name: "Aliases !alfa" })); + await waitFor(() => { + expect(screen.getByTestId("aliases-note-pack:p1")).toHaveTextContent("reloaded"); + }); + const [url] = fetcher.mock.calls[0] as [string]; + expect(url).toBe("/api/aliases/pack%3Ap1"); +}); + +it("shows the bot's own sentence when the reload is refused", async () => { + serve({ "/api/aliases/cat": { body: { detail: "trigger '!pies' is taken" }, status: 409 } }); + field(); + await userEvent.type(screen.getByLabelText("!kot aliases"), "pies"); + await userEvent.click(screen.getByRole("button", { name: "Aliases !kot" })); + await waitFor(() => { + expect(screen.getByTestId("aliases-note-cat")).toHaveTextContent("trigger '!pies' is taken"); + }); + // Refused means not kept -- the field still says what was typed, so it can be fixed. + expect(screen.getByLabelText("!kot aliases")).toHaveValue("pies"); +}); + +it("says 'could not save' when the failure is not the server's", async () => { + vi.stubGlobal("fetch", vi.fn(() => Promise.reject(new TypeError("offline")))); + field(); + await userEvent.click(screen.getByRole("button", { name: "Aliases !kot" })); + await waitFor(() => { + expect(screen.getByTestId("aliases-note-cat")).toHaveTextContent("could not save"); + }); +}); diff --git a/web/src/views/commands-aliases.tsx b/web/src/views/commands-aliases.tsx new file mode 100644 index 0000000..1a2f790 --- /dev/null +++ b/web/src/views/commands-aliases.tsx @@ -0,0 +1,60 @@ +import { useCallback, useState } from "react"; + +import { api, ApiError } from "../api/client.ts"; + +// Typed as one line, "!kotek, !kicia"; the server adds a missing "!" and +// refuses a word with a space in it, so the split here only has to find the +// words. Commas and whitespace are both separators because both get typed. +export function wordsOf(text: string): string[] { + return text.split(/[\s,]+/u).filter((word) => word !== ""); +} + +// The operator's own words for one command. Unlike the switches in the same +// row this is data the bot's table is *built* from, so the save goes through +// a reload -- the page does it, and the note says whether the bot took it. +export function AliasField({ + initial, + name, + trigger, +}: Readonly<{ initial: string[]; name: string; trigger: string }>): React.JSX.Element { + const [text, setText] = useState(initial.join(", ")); + const [note, setNote] = useState(""); + const [isBusy, setIsBusy] = useState(false); + + const save = useCallback(() => { + setIsBusy(true); + void api + .saveAliases(name, wordsOf(text)) + .then((saved) => { + setText(saved.triggers.join(", ")); + setNote(`${saved.status}, ${String(saved.commands)} commands`); + }) + .catch((error: unknown) => { + setNote(error instanceof ApiError ? error.message : "could not save"); + }) + .finally(() => { + setIsBusy(false); + }); + }, [name, text]); + + return ( + <> + {" "} + {" "} + {note} + + ); +} diff --git a/web/src/views/commands.test.tsx b/web/src/views/commands.test.tsx index 0f118a0..73e821f 100644 --- a/web/src/views/commands.test.tsx +++ b/web/src/views/commands.test.tsx @@ -2,7 +2,7 @@ import { fireEvent, render, screen, waitFor, within } from "@testing-library/rea import userEvent from "@testing-library/user-event"; import { afterEach, expect, it, vi } from "vitest"; -import { COMMANDS } from "../test/fixtures.ts"; +import { ALIASES, COMMANDS } from "../test/fixtures.ts"; import { serve } from "../test/http.ts"; import { Commands } from "./commands.tsx"; @@ -12,6 +12,9 @@ afterEach(() => { const [CAT] = COMMANDS.commands; +// The view asks for the live table and the operator's aliases together. +const TABLE = { "/api/aliases": { body: ALIASES }, "/api/commands": { body: COMMANDS } }; + async function rendered(): Promise { render(); await waitFor(() => { @@ -20,13 +23,13 @@ async function rendered(): Promise { } it("says it is asking before the bot answers", () => { - serve({ "/api/commands": { body: COMMANDS } }); + serve({ ...TABLE }); render(); expect(screen.getByText("Asking the bot…")).toBeInTheDocument(); }); it("lists every live command, on, with the default as the placeholder", async () => { - serve({ "/api/commands": { body: COMMANDS } }); + serve({ ...TABLE }); await rendered(); expect(screen.getByTestId("command-cat")).toHaveTextContent("!kot"); expect(screen.getByTestId("command-cat")).toHaveAttribute("data-off", "false"); @@ -36,14 +39,33 @@ it("lists every live command, on, with the default as the placeholder", async () expect(screen.getByLabelText("!cisza cooldown")).toHaveAttribute("placeholder", "none"); }); +it("puts the operator's aliases in their command's row", async () => { + serve({ ...TABLE }); + await rendered(); + expect(screen.getByLabelText("!kot aliases")).toHaveValue("!kotek"); + expect(screen.getByLabelText("!cisza aliases")).toHaveValue(""); +}); + +it("reports an alias file that cannot be read instead of a half table", async () => { + serve({ + ...TABLE, + "/api/aliases": { body: { detail: "aliases_custom.json cannot be read: oops" }, status: 409 }, + }); + render(); + await waitFor(() => { + expect(screen.getByTestId("commands-problem")).toHaveTextContent("cannot be read"); + }); +}); + it("shows an override the bot already holds, as the value rather than the placeholder", async () => { - serve({ "/api/commands": { body: { commands: [{ ...CAT, cooldown_seconds: 5 }] } } }); + serve({ ...TABLE, "/api/commands": { body: { commands: [{ ...CAT, cooldown_seconds: 5 }] } } }); await rendered(); expect(screen.getByLabelText("!kot cooldown")).toHaveValue(5); }); it("names the bot's reason when there is no live table yet", async () => { serve({ + ...TABLE, "/api/commands": { body: { detail: { reason: "attempt 2: refused", status: "connecting" } }, status: 503, @@ -66,7 +88,7 @@ it("falls back to a plain sentence when the failure is not the server's", async it("saves one command's switches and shows the row the bot sent back", async () => { const saved = { ...CAT, cooldown_seconds: 5, enabled: false }; const fetcher = serve({ - "/api/commands": { body: COMMANDS }, + ...TABLE, "/api/commands/cat": { body: saved }, }); await rendered(); @@ -76,7 +98,7 @@ it("saves one command's switches and shows the row the bot sent back", async () await waitFor(() => { expect(screen.getByTestId("note-cat")).toHaveTextContent("saved"); }); - const [, init] = fetcher.mock.calls[1] as [string, RequestInit]; + const [, init] = fetcher.mock.calls[2] as [string, RequestInit]; expect(JSON.parse(init.body as string)).toEqual({ cooldown_seconds: 5, disabled_until: null, @@ -88,7 +110,7 @@ it("saves one command's switches and shows the row the bot sent back", async () it("sends the deadline as epoch seconds and never a cooldown for an ungated command", async () => { const later = Math.floor(Date.now() / 60_000) * 60 + 3600; const fetcher = serve({ - "/api/commands": { body: COMMANDS }, + ...TABLE, "/api/commands/mute": { body: { ...COMMANDS.commands[1], disabled_until: later } }, }); await rendered(); @@ -100,7 +122,7 @@ it("sends the deadline as epoch seconds and never a cooldown for an ungated comm await waitFor(() => { expect(screen.getByTestId("note-mute")).toHaveTextContent("saved"); }); - const [, init] = fetcher.mock.calls[1] as [string, RequestInit]; + const [, init] = fetcher.mock.calls[2] as [string, RequestInit]; expect(JSON.parse(init.body as string)).toEqual({ cooldown_seconds: null, disabled_until: later, @@ -112,7 +134,7 @@ it("sends the deadline as epoch seconds and never a cooldown for an ungated comm it("shows the bot's own sentence when a save is refused", async () => { serve({ - "/api/commands": { body: COMMANDS }, + ...TABLE, "/api/commands/cat": { body: { detail: "no command named 'cat'" }, status: 404 }, }); await rendered(); @@ -123,7 +145,7 @@ it("shows the bot's own sentence when a save is refused", async () => { }); it("says 'could not save' when the failure is not the server's", async () => { - serve({ "/api/commands": { body: COMMANDS } }); + serve({ ...TABLE }); await rendered(); vi.stubGlobal("fetch", vi.fn(() => Promise.reject(new TypeError("offline")))); await userEvent.click(screen.getByRole("button", { name: "Save !kot" })); diff --git a/web/src/views/commands.tsx b/web/src/views/commands.tsx index 996a4aa..7deee21 100644 --- a/web/src/views/commands.tsx +++ b/web/src/views/commands.tsx @@ -3,6 +3,7 @@ import { useCallback, useEffect, useState } from "react"; import type { CommandSetting } from "../api/types.ts"; import { api, ApiError } from "../api/client.ts"; +import { AliasField } from "./commands-aliases.tsx"; import { cooldownFrom, fromLocalInput, isOffAt, toLocalInput } from "./commands-time.ts"; // A row as shown: the bot's answer plus whether it is off *now*. Decided @@ -16,10 +17,20 @@ function shown(row: CommandSetting): Shown { return { ...row, isOff: isOffAt(row, Date.now() / 1000) }; } +// The live table and the operator's aliases arrive together: a row shows +// both, and neither is worth showing without the other. +interface Table { + aliases: Record; + rows: Shown[]; +} + // One row per live command, each with its own Save: a switch is thrown for // one command at a time, and the bot reads it on the very next attempt, so // there is nothing to batch and nothing to restart. -function Row({ initial }: Readonly<{ initial: Shown }>): React.JSX.Element { +function Row({ + aliases, + initial, +}: Readonly<{ aliases: string[]; initial: Shown }>): React.JSX.Element { const [row, setRow] = useState(initial); const [isEnabled, setIsEnabled] = useState(initial.enabled); const [until, setUntil] = useState(toLocalInput(initial.disabled_until)); @@ -106,19 +117,21 @@ function Row({ initial }: Readonly<{ initial: Shown }>): React.JSX.Element { {" "} {note} + + + ); } export function Commands(): React.JSX.Element { - const [rows, setRows] = useState(null); + const [table, setTable] = useState(null); const [problem, setProblem] = useState(""); useEffect(() => { - void api - .commands() - .then((listing) => { - setRows(listing.commands.map((row) => shown(row))); + void Promise.all([api.commands(), api.aliases()]) + .then(([listing, custom]) => { + setTable({ aliases: custom.aliases, rows: listing.commands.map((row) => shown(row)) }); }) .catch((error: unknown) => { // 503 while the bot is still connecting is the usual one: the list @@ -130,7 +143,7 @@ export function Commands(): React.JSX.Element { if (problem !== "") { return

The bot did not answer: {problem}

; } - if (rows === null) { + if (table === null) { return

Asking the bot…

; } return ( @@ -139,11 +152,16 @@ export function Commands(): React.JSX.Element {

Every switch here is live: the bot reads it on the next message, without a reload. A command that is off answers WYŁĄCZONE so nobody thinks it broke. An empty - cooldown means the bot's own default. + cooldown means the bot's own default. Aliases are the one exception: they go through a + reload, which the page performs, and a word the bot refuses is not kept.

- {rows.map((row) => )} + {table.rows.map((row) => { + return ( + + ); + })}