mirror of
https://github.com/kuhyx/signal-bot.git
synced 2026-11-10 18:17:02 +01:00
Rebuild the command table on demand, and survive a tag whose file is gone
The registry used to be built once, inline in build_bot, so every change to the data behind it -- a person tagged in the web UI, an alias, soon a command switched off from the management page -- meant restarting the unit. That is cheap and lossless, but it is a deploy step somebody has to remember, and a 21:37 report can land in the middle of it. Now the registration sequence lives in wiring.build_registry, and RegistryHolder runs it again on POST /_reload: the new table is built whole and swapped in with one assignment, never patched in place, so a duplicate trigger is found before anything the dispatcher reads has changed. Bad data (ConfigError, DuplicateTriggerError) keeps the previous table and answers 409 with the reason; a programming error still surfaces as a 500. scripts/reload.sh wraps it, with --code for the restart a Python change needs, and an optional command to type in testbot so "reloaded" means "answered". No Python hot-reload on purpose. The loader change is what makes deploying this safe: two rows in the live tags.json name files that are no longer in media/, and load_pack refused them, so the next restart would have looped on ConfigError. A stale tag is drift in a file a page edits mid-session, not a broken album -- it is now skipped with a warning naming it. A tag that reaches outside the album is still refused. pack_load.py split at the 250-line cap; pack_tags.py is the half whose checks are not all fatal. Proven offline in scripts/e2e.sh: !delta is unknown, a fifth person is tagged into the synthetic pack, scripts/reload.sh runs, and the same process posts delta's photo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
3107a9e55f
commit
0717df646a
59
DOCS-reload.md
Normal file
59
DOCS-reload.md
Normal file
@ -0,0 +1,59 @@
|
||||
# Picking up a change without a restart
|
||||
|
||||
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 every setting a page in 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.
|
||||
|
||||
```sh
|
||||
scripts/reload.sh # rebuild the table, report how many commands are live
|
||||
scripts/reload.sh '!nowaosoba' # ...and then type that at the bot in testbot
|
||||
```
|
||||
|
||||
The answer is what the bot is serving **now**:
|
||||
|
||||
| status | code | meaning |
|
||||
|------------|------|--------------------------------------------------------------------|
|
||||
| `reloaded` | 200 | the new table is live; `commands` is its size |
|
||||
| `kept` | 409 | the data no longer builds; the previous table is still answering, and `reason` says what was wrong |
|
||||
| `connecting` | 503 | the bot has not reached Signal yet; there is no table to rebuild |
|
||||
|
||||
A rejected reload is deliberate: the builder reads files people edit by hand
|
||||
and pages write mid-session, and the only safe answer to a file that does not
|
||||
load is the table that was already answering. Only bad *data* is survived
|
||||
(`ConfigError`, `DuplicateTriggerError`); a programming error still surfaces
|
||||
as a 500 and a traceback in the journal, because a bug is not a file.
|
||||
|
||||
Startup is the one time a bad table is fatal: there is nothing to fall back
|
||||
on, and `Restart=always` looping on a `ConfigError` is a clearer signal than a
|
||||
bot that answers nothing.
|
||||
|
||||
## Code: `scripts/reload.sh --code`
|
||||
|
||||
A Python change needs the process replaced. The restart is about a second and
|
||||
lossless (signal-cli replays what arrived meanwhile), but it is a deploy step,
|
||||
so it is the flag rather than the default:
|
||||
|
||||
```sh
|
||||
scripts/reload.sh --code '!pomoc'
|
||||
```
|
||||
|
||||
restarts the unit, waits for `/healthz` to report `ok` again, prints the
|
||||
version it came back as, and -- with a command given -- types it in the test
|
||||
group so "restarted" means "answered". There is no hot-reload of Python on
|
||||
purpose: `importlib.reload` leaves every object the old module already
|
||||
created bound to the old classes, which is a bug that looks like a feature
|
||||
until the first `isinstance` check.
|
||||
|
||||
## Proven where
|
||||
|
||||
`scripts/e2e.sh` brackets a `scripts/reload.sh` with the two scenarios in
|
||||
`fake_signal/proofs/`: `!delta` is unknown, a fifth person is tagged into the
|
||||
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`.
|
||||
@ -227,9 +227,9 @@ things: `!staty` `!wczoraj` `!tydzień` `!miesiąc` `!rok` `!rekordy` `!korona`
|
||||
|
||||
## Managing it
|
||||
|
||||
`./web.sh` serves one page on `127.0.0.1:8140`: health, the configuration
|
||||
(editable) and the album's tagger. Started by hand and stopped again, because
|
||||
it can rewrite `.env`: [DOCS-web-ui.md](DOCS-web-ui.md).
|
||||
`./web.sh` serves one page on `127.0.0.1:8140`: health, the configuration (editable) and the
|
||||
album's tagger. Started by hand and stopped again, because it can rewrite `.env`: [DOCS-web-ui.md](DOCS-web-ui.md).
|
||||
A tagged person goes live with `scripts/reload.sh`, no restart; code with `--code`: [DOCS-reload.md](DOCS-reload.md).
|
||||
|
||||
## Develop
|
||||
|
||||
|
||||
7
fake_signal/proofs/reload_after.yaml
Normal file
7
fake_signal/proofs/reload_after.yaml
Normal file
@ -0,0 +1,7 @@
|
||||
# The other half of the bracket: the same process, one POST /_reload later,
|
||||
# and the person tagged into the album meanwhile answers with a photo.
|
||||
name: "!delta posts a photo after the reload"
|
||||
inject:
|
||||
- { text: "!delta", uuid: uuid-reload-after, name: ReloadAfter }
|
||||
expect:
|
||||
- { kind: image, to: reply }
|
||||
8
fake_signal/proofs/reload_before.yaml
Normal file
8
fake_signal/proofs/reload_before.yaml
Normal file
@ -0,0 +1,8 @@
|
||||
# Not in scenarios/: that glob replays against the pack as first written,
|
||||
# and this pair brackets the POST /_reload that scripts/e2e_proofs.sh makes
|
||||
# in between. Before it, !delta is a word the bot has never heard.
|
||||
name: "!delta is unknown before the reload"
|
||||
inject:
|
||||
- { text: "!delta", uuid: uuid-reload-before, name: ReloadBefore }
|
||||
expect:
|
||||
- { kind: text, to: reply, contains: "Nie znam !delta" }
|
||||
@ -95,6 +95,24 @@ wait_for_subscription() {
|
||||
done
|
||||
}
|
||||
|
||||
# Drive the fake through these scenario files, against the bot booted above.
|
||||
# One place for the group map, so a proof that injects after the main replay
|
||||
# resolves the same symbolic names the scenarios do.
|
||||
replay() {
|
||||
"$PYTHON" -m fake_signal.cli "$@" \
|
||||
--settle "${E2E_SETTLE:-0.15}" \
|
||||
--base-url "$FAKE_URL" \
|
||||
--group "watched=${WATCHED_GROUP_INTERNAL}" \
|
||||
--group "reply=${WATCHED_GROUP}" \
|
||||
--group "second=${SECOND_GROUP_INTERNAL}" \
|
||||
--group "second_reply=${SECOND_GROUP}" \
|
||||
--group "third=${THIRD_GROUP_INTERNAL}" \
|
||||
--group "third_reply=${THIRD_GROUP}" \
|
||||
--group "fourth=${FOURTH_GROUP_INTERNAL}" \
|
||||
--group "fourth_reply=${FOURTH_GROUP}" \
|
||||
--group "other=${OTHER_GROUP}"
|
||||
}
|
||||
|
||||
validate_requirements() {
|
||||
if [[ ! -x "$PYTHON" ]]; then
|
||||
echo "Error: no interpreter at $PYTHON. Run: uv venv .venv && uv pip install -e '.[dev]'" >&2
|
||||
@ -159,23 +177,12 @@ main() {
|
||||
wait_for_subscription
|
||||
|
||||
echo "== replaying scenarios"
|
||||
"$PYTHON" -m fake_signal.cli \
|
||||
"$REPO_ROOT"/fake_signal/scenarios/*.yaml \
|
||||
--settle "${E2E_SETTLE:-0.15}" \
|
||||
--base-url "$FAKE_URL" \
|
||||
--group "watched=${WATCHED_GROUP_INTERNAL}" \
|
||||
--group "reply=${WATCHED_GROUP}" \
|
||||
--group "second=${SECOND_GROUP_INTERNAL}" \
|
||||
--group "second_reply=${SECOND_GROUP}" \
|
||||
--group "third=${THIRD_GROUP_INTERNAL}" \
|
||||
--group "third_reply=${THIRD_GROUP}" \
|
||||
--group "fourth=${FOURTH_GROUP_INTERNAL}" \
|
||||
--group "fourth_reply=${FOURTH_GROUP}" \
|
||||
--group "other=${OTHER_GROUP}"
|
||||
replay "$REPO_ROOT"/fake_signal/scenarios/*.yaml
|
||||
|
||||
# !dodaj is unwired until photos land in an approval queue instead of
|
||||
# the album (2026-09-04). Its scenarios, the media generation step and
|
||||
# prove_the_add_photo_invariants left with it; git has them at 64852ae.
|
||||
prove_the_table_reloads_without_a_restart
|
||||
prove_the_deafness_gate
|
||||
enforce_the_time_budget "$STARTED_AT"
|
||||
}
|
||||
|
||||
@ -43,6 +43,28 @@ prove_the_deafness_gate() {
|
||||
echo "== /healthz recovered on its own after ${waited}s"
|
||||
}
|
||||
|
||||
# A person tagged while the bot is up becomes a command after POST /_reload,
|
||||
# with no restart: the same process that just answered "Nie znam !delta"
|
||||
# posts delta's photo a moment later. The scenarios live next to the main
|
||||
# set but outside its glob, because the second one is only true after the
|
||||
# reload that this proof performs between them.
|
||||
prove_the_table_reloads_without_a_restart() {
|
||||
local proofs="${REPO_ROOT}/fake_signal/proofs"
|
||||
echo "== before the reload, !delta is nobody"
|
||||
replay "${proofs}/reload_before.yaml"
|
||||
|
||||
echo "== tagging a fifth person into the pack, then scripts/reload.sh"
|
||||
"$PYTHON" "$REPO_ROOT/tool/e2e_pack.py" \
|
||||
--dir "${TEMP_DIR}/pack" --group "$WATCHED_GROUP" --extra delta >/dev/null
|
||||
# The operator's script, not a bare curl: this is the one place it runs
|
||||
# against a real process in CI, so its exit code is the proof.
|
||||
BIND_PORT="$BOT_PORT" "$REPO_ROOT/scripts/reload.sh" ||
|
||||
die "scripts/reload.sh did not get the new table live"
|
||||
|
||||
echo "== after the reload, !delta posts a photo"
|
||||
replay "${proofs}/reload_after.yaml"
|
||||
}
|
||||
|
||||
# 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;
|
||||
|
||||
121
scripts/reload.sh
Executable file
121
scripts/reload.sh
Executable file
@ -0,0 +1,121 @@
|
||||
#!/bin/bash
|
||||
|
||||
# ============================================================================
|
||||
# Make the running bot pick up a change, and prove that it did.
|
||||
#
|
||||
# Two kinds of change, two mechanisms:
|
||||
#
|
||||
# scripts/reload.sh [cmd] data changed (aliases, an album, a
|
||||
# command switched off): POST /_reload,
|
||||
# the table is rebuilt in place, nothing
|
||||
# restarts and no frame is lost.
|
||||
# scripts/reload.sh --code [cmd] code changed: restart the unit, wait for
|
||||
# /healthz to go green again. Lossless too
|
||||
# (signal-cli replays what arrived), but
|
||||
# a deploy step -- so it is the flag, not
|
||||
# the default.
|
||||
#
|
||||
# With [cmd], the bot then types that command at itself in the test group,
|
||||
# so "reloaded" means "answered", not "returned 200". E.g.
|
||||
#
|
||||
# scripts/reload.sh '!nowaosoba'
|
||||
# scripts/reload.sh --code '!pomoc'
|
||||
# ============================================================================
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_NAME="$(basename "$0")"
|
||||
readonly SCRIPT_NAME
|
||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
readonly SCRIPT_DIR
|
||||
readonly BOT="http://127.0.0.1:${BIND_PORT:-8137}"
|
||||
readonly UNIT="signal-bot"
|
||||
readonly TEST_GROUP="${RELOAD_GROUP:-testbot}"
|
||||
|
||||
RESTART=0
|
||||
PROBE=""
|
||||
|
||||
usage() {
|
||||
echo "Usage: $SCRIPT_NAME [--code] [command-to-selftest]"
|
||||
echo " --code restart the unit (code changed) instead of rebuilding the table"
|
||||
exit 0
|
||||
}
|
||||
|
||||
validate_requirements() {
|
||||
if ! command -v jq >/dev/null; then
|
||||
echo "Error: jq is required" >&2
|
||||
exit 1
|
||||
fi
|
||||
}
|
||||
|
||||
wait_healthy() {
|
||||
local attempt=0
|
||||
# 150 x 0.4 s: a restart is ~1 s, but signal-cli can take a while to
|
||||
# accept the websocket again after its own reconnect.
|
||||
until [[ "$(curl -s "${BOT}/healthz" | jq -r '.status // "down"')" == "ok" ]]; do
|
||||
attempt=$((attempt + 1))
|
||||
if [[ $attempt -ge 150 ]]; then
|
||||
echo "Error: ${BOT}/healthz never reported ok" >&2
|
||||
curl -s "${BOT}/healthz" >&2 || true
|
||||
exit 1
|
||||
fi
|
||||
sleep 0.4
|
||||
done
|
||||
}
|
||||
|
||||
reload_data() {
|
||||
local reply status
|
||||
reply="$(curl -s -X POST "${BOT}/_reload")"
|
||||
status="$(jq -r '.status // "?"' <<<"$reply")"
|
||||
case "$status" in
|
||||
reloaded)
|
||||
echo "reloaded: $(jq -r '.commands' <<<"$reply") commands live"
|
||||
;;
|
||||
kept)
|
||||
# The old table is still answering; the new data is what is broken.
|
||||
echo "Error: reload refused, previous table kept: $(jq -r '.reason' <<<"$reply")" >&2
|
||||
exit 1
|
||||
;;
|
||||
*)
|
||||
echo "Error: unexpected answer from ${BOT}/_reload: ${reply}" >&2
|
||||
exit 1
|
||||
;;
|
||||
esac
|
||||
}
|
||||
|
||||
restart_code() {
|
||||
systemctl --user restart "$UNIT"
|
||||
wait_healthy
|
||||
echo "restarted: $(curl -s "${BOT}/healthz" | jq -r '"\(.version) \(.pope)"')"
|
||||
}
|
||||
|
||||
main() {
|
||||
validate_requirements
|
||||
if [[ $RESTART -eq 1 ]]; then
|
||||
restart_code
|
||||
else
|
||||
wait_healthy
|
||||
reload_data
|
||||
fi
|
||||
if [[ -n "$PROBE" ]]; then
|
||||
"${SCRIPT_DIR}/selftest.sh" "$TEST_GROUP" "$PROBE"
|
||||
fi
|
||||
}
|
||||
|
||||
while [[ $# -gt 0 ]]; do
|
||||
case $1 in
|
||||
--code)
|
||||
RESTART=1
|
||||
shift
|
||||
;;
|
||||
-h | --help)
|
||||
usage
|
||||
;;
|
||||
*)
|
||||
PROBE="$1"
|
||||
shift
|
||||
;;
|
||||
esac
|
||||
done
|
||||
|
||||
main
|
||||
@ -143,3 +143,25 @@ async def selftest(request: SelfTestRequest, response: Response) -> dict[str, st
|
||||
response.status_code = status.HTTP_403_FORBIDDEN
|
||||
return {"status": "refused", "reason": str(refusal)}
|
||||
return {"status": "delivered", "group": group_id, "text": request.text}
|
||||
|
||||
|
||||
@app.post("/_reload")
|
||||
async def reload_commands(response: Response) -> dict[str, str | int]:
|
||||
"""Rebuild the command table from the data on disk, without a restart.
|
||||
|
||||
The answer is what the bot is serving *now*: after a rejected reload
|
||||
that is still the previous table, and ``reason`` says what was wrong
|
||||
with the new one. 409 rather than 500, because the data was bad and the
|
||||
bot is fine. Loopback only, like everything else on this port.
|
||||
"""
|
||||
starter: Starter = app.state.starter
|
||||
runtime = starter.runtime
|
||||
if runtime is None:
|
||||
response.status_code = status.HTTP_503_SERVICE_UNAVAILABLE
|
||||
return {"status": "connecting", "reason": starter.reason}
|
||||
bot = runtime.bot
|
||||
if bot.reload_commands():
|
||||
return {"status": "reloaded", "commands": bot.command_count}
|
||||
response.status_code = status.HTTP_409_CONFLICT
|
||||
holder = bot.dispatcher.holder
|
||||
return {"status": "kept", "commands": bot.command_count, "reason": holder.last_error}
|
||||
|
||||
@ -14,38 +14,22 @@ from collections.abc import Callable
|
||||
from dataclasses import dataclass
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from signal_bot.analytics_command import register_analytics_command
|
||||
from signal_bot.animals import load_aliases, register_animal_commands
|
||||
from signal_bot.announce import announce_all
|
||||
from signal_bot.art_command import register_art_command
|
||||
from signal_bot.changelog import load as load_changelog
|
||||
from signal_bot.decorators import cooldown
|
||||
from signal_bot.dispatcher import Dispatcher
|
||||
from signal_bot.help_command import register_help_command
|
||||
from signal_bot.http_client import ImageFetcher
|
||||
from signal_bot.pack_command import register_pack_commands
|
||||
from signal_bot.pack_holder import PackHolder
|
||||
from signal_bot.pack_load import load as load_pack
|
||||
from signal_bot.pack_random import register_random_command
|
||||
from signal_bot.poem_command import register_poem_command
|
||||
from signal_bot.popes_command import register_pope_command
|
||||
from signal_bot.portraits import PortraitCache
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
from signal_bot.reporting import compose
|
||||
from signal_bot.stats import periods
|
||||
from signal_bot.stats.commands import register_stats_commands
|
||||
from signal_bot.stats.nightly import days_to_report, mark_reported
|
||||
from signal_bot.suggest_command import register_suggest_command
|
||||
from signal_bot.traps_command import register_trap_command
|
||||
from signal_bot.versioning import POPE
|
||||
from signal_bot.wiring import COOLDOWN_MESSAGE, build_registry
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pathlib import Path
|
||||
|
||||
import httpx
|
||||
|
||||
from signal_bot.config import Config
|
||||
from signal_bot.decorators import Decorator
|
||||
from signal_bot.discovery import GroupDirectory
|
||||
from signal_bot.liveness import Liveness
|
||||
from signal_bot.store import Store
|
||||
@ -55,7 +39,9 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
Clock = Callable[[], dt.datetime]
|
||||
|
||||
COOLDOWN_MESSAGE = "BEEP BOOP POCZEKAJ {seconds} {unit}."
|
||||
# Re-exported: the message lives with the wiring now, and this is where
|
||||
# every test and the docs have always imported it from.
|
||||
__all__ = ["COOLDOWN_MESSAGE", "Bot", "Clock", "build_bot", "now_local"]
|
||||
|
||||
|
||||
def now_local() -> dt.datetime:
|
||||
@ -164,18 +150,14 @@ class Bot:
|
||||
self.liveness.note_envelope()
|
||||
await self.dispatcher.handle(frame)
|
||||
|
||||
def reload_commands(self) -> bool:
|
||||
"""Rebuild the command table from the data on disk, keeping the old one on failure."""
|
||||
return self.dispatcher.holder.reload()
|
||||
|
||||
def _register_album(registry: CommandRegistry, directory: Path, rate_limit: Decorator) -> None:
|
||||
"""Wire the album's commands, all of them reading one mutable holder.
|
||||
|
||||
!random takes the same cooldown as every other photo command; !add_photo
|
||||
takes no decorator at all, per "NO CAP, NO LIMITS" -- no cooldown, no
|
||||
size cap, no daily limit.
|
||||
"""
|
||||
aliases = load_aliases()
|
||||
holder = PackHolder(load_pack(directory))
|
||||
register_pack_commands(registry, holder, decorate=rate_limit)
|
||||
register_random_command(registry, holder, tuple(aliases["random"]), decorate=rate_limit)
|
||||
@property
|
||||
def command_count(self) -> int:
|
||||
"""How many commands the live table holds -- what /_reload reports back."""
|
||||
return len(self.dispatcher.registry.commands)
|
||||
|
||||
|
||||
def build_bot(
|
||||
@ -187,46 +169,20 @@ def build_bot(
|
||||
liveness: Liveness,
|
||||
clock: Clock = now_local,
|
||||
) -> Bot:
|
||||
"""Assemble the object graph from its injected dependencies."""
|
||||
registry = CommandRegistry()
|
||||
"""Assemble the object graph from its injected dependencies.
|
||||
|
||||
The registry is built through a holder so that the data behind it --
|
||||
aliases, albums -- can be re-read without a restart: see registry_holder.
|
||||
The fetcher and the portrait cache outlive any one table and are shared
|
||||
with every rebuild.
|
||||
"""
|
||||
fetcher = ImageFetcher(client)
|
||||
portraits = PortraitCache(fetcher, config.commons_api_url)
|
||||
rate_limit = cooldown(config.cooldown_seconds, COOLDOWN_MESSAGE)
|
||||
register_animal_commands(
|
||||
registry,
|
||||
fetcher,
|
||||
decorate=rate_limit,
|
||||
endpoints=config.image_api_urls,
|
||||
)
|
||||
register_pope_command(registry, portraits, decorate=rate_limit)
|
||||
register_trap_command(registry, fetcher, config.commons_api_url, decorate=rate_limit)
|
||||
register_art_command(registry, fetcher, config.commons_api_url, decorate=rate_limit)
|
||||
register_poem_command(registry, fetcher, config.poem_text_url, decorate=rate_limit)
|
||||
register_stats_commands(registry, decorate=rate_limit)
|
||||
# Reads the *command* history rather than the message history, and
|
||||
# reads the registry when it runs -- so the album's commands, which
|
||||
# are registered below this line, are still in the near-miss set.
|
||||
register_analytics_command(registry, decorate=rate_limit)
|
||||
# Rate-limited like every other command, and unlike !add_photo, whose
|
||||
# "NO CAP, NO LIMITS" is a deliberate exception rather than a default.
|
||||
# The cost is real and accepted: somebody told "napisz !zaproponuj ..."
|
||||
# who then files two ideas in a row is asked to wait for the second. The
|
||||
# alternative is one person being able to fill a table nothing prunes.
|
||||
register_suggest_command(registry, decorate=rate_limit)
|
||||
# Absent unless this installation has an album, and scoped to that
|
||||
# album's own groups when it does -- so on every other machine, and in
|
||||
# every other group, these commands simply do not exist.
|
||||
# "album", not "directory": build_bot's own `directory` is the group
|
||||
# directory, and shadowing it here silently passed the wrong object.
|
||||
for album in config.photo_pack_dirs:
|
||||
_register_album(registry, album, rate_limit)
|
||||
# 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)
|
||||
holder = RegistryHolder(lambda: build_registry(config, fetcher, portraits))
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=holder,
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=clock,
|
||||
|
||||
@ -22,6 +22,7 @@ if TYPE_CHECKING:
|
||||
from signal_bot.discovery import GroupDirectory
|
||||
from signal_bot.models import IncomingMessage
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
from signal_bot.store import Store
|
||||
from signal_bot.transport_base import Transport
|
||||
|
||||
@ -36,11 +37,19 @@ class Dispatcher:
|
||||
|
||||
config: Config
|
||||
directory: GroupDirectory
|
||||
registry: CommandRegistry
|
||||
# The box, not the table: the table is rebuilt and swapped when the
|
||||
# data behind it changes, and a dispatcher holding the table itself
|
||||
# would keep answering from the one it was built with.
|
||||
holder: RegistryHolder
|
||||
store: Store
|
||||
transport: Transport
|
||||
clock: Clock
|
||||
|
||||
@property
|
||||
def registry(self) -> CommandRegistry:
|
||||
"""The command table as it stands right now."""
|
||||
return self.holder.registry
|
||||
|
||||
async def _answer_unknown(self, message: IncomingMessage, group: str) -> None:
|
||||
"""Answer a "!" word nothing claims, and remember it.
|
||||
|
||||
|
||||
@ -4,34 +4,25 @@
|
||||
Every check here fails startup rather than a command, for the reason
|
||||
config_env.py gives: a manifest that names a person who does not exist, a
|
||||
trigger the folder can never match, or a file that has since been renamed is
|
||||
an answer, not an outage, and no amount of waiting fixes it. The one thing
|
||||
that is *not* an error is a pack that has not been tagged yet -- that is an
|
||||
album waiting for the tagger, and it says so in the log.
|
||||
an answer, not an outage, and no amount of waiting fixes it. What is *not*
|
||||
an error lives in pack_tags.py: a pack that has not been tagged yet, and a
|
||||
tag whose file has since gone.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
from typing import TYPE_CHECKING, Any, Final
|
||||
|
||||
from signal_bot.folding import fold
|
||||
from signal_bot.groups import ConfigError
|
||||
from signal_bot.pack import (
|
||||
MEDIA_DIR,
|
||||
MEDIA_TYPES,
|
||||
PEOPLE_FILE,
|
||||
TAGS_FILE,
|
||||
Entry,
|
||||
Pack,
|
||||
)
|
||||
from signal_bot.pack import PEOPLE_FILE, Entry, Pack
|
||||
from signal_bot.pack_custom import load as load_custom
|
||||
from signal_bot.pack_tags import document as _document
|
||||
from signal_bot.pack_tags import tags as _tags
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pathlib import Path
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# "!" plus at least one character to fold.
|
||||
MIN_TRIGGER_LENGTH: Final = 2
|
||||
|
||||
@ -66,24 +57,6 @@ def load(directory: Path) -> Pack:
|
||||
)
|
||||
|
||||
|
||||
def _document(path: Path) -> dict[str, Any]:
|
||||
"""One JSON manifest, read strictly."""
|
||||
try:
|
||||
text = path.read_text(encoding="utf-8")
|
||||
except OSError as error:
|
||||
msg = f"photo pack manifest {path} cannot be read: {error}"
|
||||
raise ConfigError(msg) from error
|
||||
try:
|
||||
document = json.loads(text)
|
||||
except json.JSONDecodeError as error:
|
||||
msg = f"photo pack manifest {path} is not valid JSON: {error}"
|
||||
raise ConfigError(msg) from error
|
||||
if not isinstance(document, dict):
|
||||
msg = f"photo pack manifest {path} must be an object"
|
||||
raise ConfigError(msg)
|
||||
return document
|
||||
|
||||
|
||||
def _groups(document: dict[str, Any]) -> frozenset[str]:
|
||||
"""The groups the album may be posted into, which may not be empty.
|
||||
|
||||
@ -213,38 +186,3 @@ def _unique_triggers(entries: list[Entry]) -> None:
|
||||
if owner != entry.key:
|
||||
msg = f"{PEOPLE_FILE} trigger {trigger!r} is claimed by {owner!r} and {entry.key!r}"
|
||||
raise ConfigError(msg)
|
||||
|
||||
|
||||
def _tags(directory: Path, people: set[str]) -> dict[str, frozenset[str]]:
|
||||
"""Which file shows whom, checked against the album and the manifest.
|
||||
|
||||
A missing tags.json is not an error: that is a pack the tagger has not
|
||||
been run on yet, and the commands answer "no such photo" until it has.
|
||||
A tag naming a file that is not there *is* an error -- a renamed file
|
||||
would otherwise drop out of the album silently.
|
||||
"""
|
||||
path = directory / TAGS_FILE
|
||||
if not path.exists():
|
||||
logger.warning("photo pack %s has no %s yet; run ./web.sh", directory, TAGS_FILE)
|
||||
return {}
|
||||
document = _document(path)
|
||||
tags: dict[str, frozenset[str]] = {}
|
||||
for filename, keys in document.items():
|
||||
_media_file(directory, str(filename))
|
||||
if not isinstance(keys, list) or any(str(key) not in people for key in keys):
|
||||
msg = f"{TAGS_FILE} entry {filename!r} names somebody who is not in {PEOPLE_FILE}"
|
||||
raise ConfigError(msg)
|
||||
if keys:
|
||||
tags[str(filename)] = frozenset(str(key) for key in keys)
|
||||
return tags
|
||||
|
||||
|
||||
def _media_file(directory: Path, filename: str) -> None:
|
||||
"""The tagged file has to exist, and be something Signal can render."""
|
||||
path = directory / MEDIA_DIR / filename
|
||||
if path.name != filename or not path.is_file():
|
||||
msg = f"{TAGS_FILE} names {filename!r}, which is not a file in {MEDIA_DIR}/"
|
||||
raise ConfigError(msg)
|
||||
if path.suffix.lower() not in MEDIA_TYPES:
|
||||
msg = f"{TAGS_FILE} names {filename!r}, whose type is not one Signal is told about"
|
||||
raise ConfigError(msg)
|
||||
|
||||
96
signal_bot/pack_tags.py
Normal file
96
signal_bot/pack_tags.py
Normal file
@ -0,0 +1,96 @@
|
||||
# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence.
|
||||
"""Reading tags.json: which file shows whom, checked against the album.
|
||||
|
||||
Split out of pack_load.py at the 250-line cap, and the half whose checks are
|
||||
*not* all fatal. The manifest half refuses anything it cannot honour, for
|
||||
the reason given there; this half has two conditions it survives, because
|
||||
both are states a healthy album passes through: no tags.json yet, and a tag
|
||||
whose file is no longer there.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from signal_bot.groups import ConfigError
|
||||
from signal_bot.pack import MEDIA_DIR, MEDIA_TYPES, PEOPLE_FILE, TAGS_FILE
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pathlib import Path
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def document(path: Path) -> dict[str, Any]:
|
||||
"""One JSON manifest, read strictly."""
|
||||
try:
|
||||
text = path.read_text(encoding="utf-8")
|
||||
except OSError as error:
|
||||
msg = f"photo pack manifest {path} cannot be read: {error}"
|
||||
raise ConfigError(msg) from error
|
||||
try:
|
||||
document = json.loads(text)
|
||||
except json.JSONDecodeError as error:
|
||||
msg = f"photo pack manifest {path} is not valid JSON: {error}"
|
||||
raise ConfigError(msg) from error
|
||||
if not isinstance(document, dict):
|
||||
msg = f"photo pack manifest {path} must be an object"
|
||||
raise ConfigError(msg)
|
||||
return document
|
||||
|
||||
|
||||
def tags(directory: Path, people: set[str]) -> dict[str, frozenset[str]]:
|
||||
"""Which file shows whom, checked against the album and the manifest.
|
||||
|
||||
A missing tags.json is not an error: that is a pack the tagger has not
|
||||
been run on yet, and the commands answer "no such photo" until it has.
|
||||
Nor is a tag naming a file that is no longer there: that entry is
|
||||
skipped, with a warning naming it. It used to be fatal, so a renamed
|
||||
file could not drop out of the album silently -- and on 2026-09-01 two
|
||||
such entries appeared in the live pack, which meant the next restart
|
||||
would have looped on ConfigError. One stale row is drift in a file a
|
||||
page edits mid-session, not a broken album, and now that the table is
|
||||
rebuilt from that file while the bot runs, the only safe reading of
|
||||
drift is "everything else still works".
|
||||
"""
|
||||
path = directory / TAGS_FILE
|
||||
if not path.exists():
|
||||
logger.warning("photo pack %s has no %s yet; run ./web.sh", directory, TAGS_FILE)
|
||||
return {}
|
||||
tags: dict[str, frozenset[str]] = {}
|
||||
for filename, keys in document(path).items():
|
||||
if not _media_file(directory, str(filename)):
|
||||
logger.warning(
|
||||
"%s names %r, which is not a file in %s/ -- skipped", TAGS_FILE, filename, MEDIA_DIR
|
||||
)
|
||||
continue
|
||||
if not isinstance(keys, list) or any(str(key) not in people for key in keys):
|
||||
msg = f"{TAGS_FILE} entry {filename!r} names somebody who is not in {PEOPLE_FILE}"
|
||||
raise ConfigError(msg)
|
||||
if keys:
|
||||
tags[str(filename)] = frozenset(str(key) for key in keys)
|
||||
return tags
|
||||
|
||||
|
||||
def _media_file(directory: Path, filename: str) -> bool:
|
||||
"""Whether the tagged file is there; if it is, it must be something Signal can render.
|
||||
|
||||
A name that reaches outside the album is refused outright rather than
|
||||
skipped: that is not a file that went missing, it is a tag that should
|
||||
never have been written.
|
||||
|
||||
Raises:
|
||||
ConfigError: The name is a path, or the type is one Signal cannot show.
|
||||
"""
|
||||
path = directory / MEDIA_DIR / filename
|
||||
if path.name != filename:
|
||||
msg = f"{TAGS_FILE} names {filename!r}, which is not a file in {MEDIA_DIR}/"
|
||||
raise ConfigError(msg)
|
||||
if not path.is_file():
|
||||
return False
|
||||
if path.suffix.lower() not in MEDIA_TYPES:
|
||||
msg = f"{TAGS_FILE} names {filename!r}, whose type is not one Signal is told about"
|
||||
raise ConfigError(msg)
|
||||
return True
|
||||
77
signal_bot/registry_holder.py
Normal file
77
signal_bot/registry_holder.py
Normal file
@ -0,0 +1,77 @@
|
||||
# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence.
|
||||
"""The one mutable box the command table lives in, so data changes need no restart.
|
||||
|
||||
``PackHolder`` made a *photo* visible the moment it was stored, and said out
|
||||
loud what it would never do: add a command. A new person in the tagger, a
|
||||
new alias, a command switched off from the management page -- every one of
|
||||
those changes the registry itself, and until now every one of them meant
|
||||
restarting the unit. Restarting is cheap and lossless (signal-cli replays
|
||||
what arrived meanwhile), but it is a deploy step somebody has to remember,
|
||||
and a 21:37 report can land in the middle of it.
|
||||
|
||||
So the registry is rebuilt, whole, from the same builder that made it at
|
||||
startup, and swapped in one assignment. Never patched in place: a table
|
||||
that is edited command by command can be observed half-edited, and a
|
||||
duplicate trigger found on the third command of five leaves the first two
|
||||
already live. Building a fresh table first means ``DuplicateTriggerError``
|
||||
and ``ConfigError`` are raised before anything the dispatcher reads has
|
||||
changed.
|
||||
|
||||
``reload`` never raises on bad *data*. The builder reads files people edit
|
||||
by hand and pages write mid-session, and the only safe answer to a file
|
||||
that does not load is the table that was already answering. A programming
|
||||
error is not data and is not swallowed: it reaches the caller, which for
|
||||
``POST /_reload`` means a 500 and a traceback in the journal.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from signal_bot.groups import ConfigError
|
||||
from signal_bot.registry import DuplicateTriggerError
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Callable
|
||||
|
||||
from signal_bot.registry import CommandRegistry
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# A `type` alias is evaluated lazily, so the TYPE_CHECKING-only names are fine.
|
||||
type Builder = Callable[[], CommandRegistry]
|
||||
|
||||
|
||||
class RegistryHolder:
|
||||
"""A mutable reference to the command table the dispatcher reads through."""
|
||||
|
||||
def __init__(self, build: Builder) -> None:
|
||||
"""Build the first table now, loudly.
|
||||
|
||||
Startup is the one time a bad table must *not* be survived: there is
|
||||
no previous one to fall back on, and ``Restart=always`` looping on a
|
||||
ConfigError is a clearer signal than a bot that answers nothing.
|
||||
"""
|
||||
self._build = build
|
||||
self.registry = build()
|
||||
self.last_error = ""
|
||||
|
||||
def reload(self) -> bool:
|
||||
"""Rebuild the table from whatever is on disk now and publish it.
|
||||
|
||||
Returns:
|
||||
True when the new table is live, False when the old one is still
|
||||
being served because the data no longer builds -- and then
|
||||
``last_error`` says why, for whoever asked.
|
||||
"""
|
||||
try:
|
||||
fresh = self._build()
|
||||
except (ConfigError, DuplicateTriggerError) as error:
|
||||
self.last_error = str(error)
|
||||
logger.exception("command registry no longer builds; keeping the previous one")
|
||||
return False
|
||||
self.registry = fresh
|
||||
self.last_error = ""
|
||||
logger.info("command registry rebuilt: %d commands", len(fresh.commands))
|
||||
return True
|
||||
95
signal_bot/wiring.py
Normal file
95
signal_bot/wiring.py
Normal file
@ -0,0 +1,95 @@
|
||||
# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence.
|
||||
"""Which commands exist: the one function that fills a CommandRegistry.
|
||||
|
||||
Split out of bot.py when the registry stopped being built once. ``build_bot``
|
||||
still assembles the object graph; this is the part of it that reads data --
|
||||
aliases.json, every album's manifests -- and so the part ``RegistryHolder``
|
||||
runs again when that data changes. Everything with a lifetime longer than
|
||||
one table (the HTTP client, the portrait cache, the store) is passed in, so
|
||||
a rebuild reuses them rather than opening a second of each.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
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.decorators import cooldown
|
||||
from signal_bot.help_command import register_help_command
|
||||
from signal_bot.pack_command import register_pack_commands
|
||||
from signal_bot.pack_holder import PackHolder
|
||||
from signal_bot.pack_load import load as load_pack
|
||||
from signal_bot.pack_random import register_random_command
|
||||
from signal_bot.poem_command import register_poem_command
|
||||
from signal_bot.popes_command import register_pope_command
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.stats.commands import register_stats_commands
|
||||
from signal_bot.suggest_command import register_suggest_command
|
||||
from signal_bot.traps_command import register_trap_command
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pathlib import Path
|
||||
|
||||
from signal_bot.config import Config
|
||||
from signal_bot.decorators import Decorator
|
||||
from signal_bot.http_client import ImageFetcher
|
||||
from signal_bot.portraits import PortraitCache
|
||||
|
||||
COOLDOWN_MESSAGE = "BEEP BOOP POCZEKAJ {seconds} {unit}."
|
||||
|
||||
|
||||
def _register_album(registry: CommandRegistry, directory: Path, rate_limit: Decorator) -> None:
|
||||
"""Wire the album's commands, all of them reading one mutable holder.
|
||||
|
||||
!random takes the same cooldown as every other photo command. !add_photo
|
||||
is not wired: a photo posted to a group must not reach the album until
|
||||
somebody has approved it, and no approval queue exists yet.
|
||||
"""
|
||||
aliases = load_aliases()
|
||||
holder = PackHolder(load_pack(directory))
|
||||
register_pack_commands(registry, holder, decorate=rate_limit)
|
||||
register_random_command(registry, holder, tuple(aliases["random"]), decorate=rate_limit)
|
||||
|
||||
|
||||
def build_registry(
|
||||
config: Config, fetcher: ImageFetcher, portraits: PortraitCache
|
||||
) -> CommandRegistry:
|
||||
"""Read the data and register every command on a fresh table.
|
||||
|
||||
Raises:
|
||||
ConfigError: An album no longer loads.
|
||||
DuplicateTriggerError: Two commands live in one group claim a word.
|
||||
"""
|
||||
registry = CommandRegistry()
|
||||
rate_limit = cooldown(config.cooldown_seconds, COOLDOWN_MESSAGE)
|
||||
register_animal_commands(
|
||||
registry,
|
||||
fetcher,
|
||||
decorate=rate_limit,
|
||||
endpoints=config.image_api_urls,
|
||||
)
|
||||
register_pope_command(registry, portraits, decorate=rate_limit)
|
||||
register_trap_command(registry, fetcher, config.commons_api_url, decorate=rate_limit)
|
||||
register_art_command(registry, fetcher, config.commons_api_url, decorate=rate_limit)
|
||||
register_poem_command(registry, fetcher, config.poem_text_url, decorate=rate_limit)
|
||||
register_stats_commands(registry, decorate=rate_limit)
|
||||
# Reads the *command* history rather than the message history, and
|
||||
# reads the registry when it runs -- so the album's commands, which
|
||||
# are registered below this line, are still in the near-miss set.
|
||||
register_analytics_command(registry, decorate=rate_limit)
|
||||
# Rate-limited like every other command. The cost is real and accepted:
|
||||
# somebody told "napisz !zaproponuj ..." who then files two ideas in a
|
||||
# row is asked to wait for the second. The alternative is one person
|
||||
# being able to fill a table nothing prunes.
|
||||
register_suggest_command(registry, decorate=rate_limit)
|
||||
# Absent unless this installation has an album, and scoped to that
|
||||
# album's own groups when it does -- so on every other machine, and in
|
||||
# every other group, these commands simply do not exist.
|
||||
for album in config.photo_pack_dirs:
|
||||
_register_album(registry, album, rate_limit)
|
||||
# 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)
|
||||
return registry
|
||||
@ -15,6 +15,7 @@ from signal_bot.analytics.usage import CommandUse, PersonUse
|
||||
from signal_bot.analytics_command import register_analytics_command, triggers_for
|
||||
from signal_bot.dispatcher import Dispatcher
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
from signal_bot.stats.events import MessageEvent
|
||||
from tests.analytics_seed import ALA, BASIA, Shape, ran
|
||||
from tests.conftest import WATCHED_GROUP, envelope
|
||||
@ -220,7 +221,7 @@ async def test_the_command_answers_in_the_group_that_asked(
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=a_registry(),
|
||||
holder=RegistryHolder(a_registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
|
||||
@ -10,6 +10,7 @@ from fastapi.testclient import TestClient
|
||||
|
||||
from signal_bot import app as app_module
|
||||
from signal_bot.app import lifespan
|
||||
from signal_bot.groups import ConfigError
|
||||
from signal_bot.versioning import __version__
|
||||
from tests.conftest import WATCHED_GROUP, mock_client, signal_client
|
||||
|
||||
@ -79,3 +80,53 @@ async def test_lifespan_is_an_async_context_manager(monkeypatch: pytest.MonkeyPa
|
||||
# Present immediately, before anything has reached Signal: /healthz
|
||||
# reads it, and an absent attribute would be a 500 rather than a 503.
|
||||
assert app_module.app.state.starter is not None
|
||||
|
||||
|
||||
def test_reload_is_refused_while_still_connecting(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
with _serving(monkeypatch, mock_client(_offline)) as client:
|
||||
response = client.post("/_reload")
|
||||
assert response.status_code == httpx.codes.SERVICE_UNAVAILABLE
|
||||
assert response.json()["status"] == "connecting"
|
||||
|
||||
|
||||
def _connected(client: TestClient) -> None:
|
||||
"""Spin the loop until the connecting task has produced a runtime."""
|
||||
for _ in range(50):
|
||||
if client.get("/healthz").json()["status"] == "ok":
|
||||
return
|
||||
msg = "the bot never connected"
|
||||
raise AssertionError(msg)
|
||||
|
||||
|
||||
def test_reload_rebuilds_the_command_table(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
with _serving(monkeypatch, signal_client(WATCHED_GROUP)) as client:
|
||||
_connected(client)
|
||||
response = client.post("/_reload")
|
||||
assert response.status_code == httpx.codes.OK
|
||||
body = response.json()
|
||||
assert body["status"] == "reloaded"
|
||||
assert body["commands"] > 0
|
||||
|
||||
|
||||
def test_reload_keeps_the_old_table_when_the_data_is_bad(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""409, not 500: the bot is fine, the file somebody edited is not."""
|
||||
with _serving(monkeypatch, signal_client(WATCHED_GROUP)) as client:
|
||||
_connected(client)
|
||||
before = client.post("/_reload").json()["commands"]
|
||||
|
||||
complaint = ConfigError("people.json: !alfa claimed twice")
|
||||
|
||||
def broken(*args: object) -> object:
|
||||
raise complaint
|
||||
|
||||
monkeypatch.setattr("signal_bot.bot.build_registry", broken)
|
||||
response = client.post("/_reload")
|
||||
assert response.status_code == httpx.codes.CONFLICT
|
||||
body = response.json()
|
||||
assert body["status"] == "kept"
|
||||
assert body["commands"] == before
|
||||
assert "claimed twice" in body["reason"]
|
||||
# And the old table still answers.
|
||||
assert client.get("/healthz").json()["status"] == "ok"
|
||||
|
||||
@ -8,8 +8,10 @@ from dataclasses import replace
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from signal_bot.bot import Bot, build_bot, now_local
|
||||
from signal_bot.groups import ConfigError
|
||||
from signal_bot.liveness import Liveness
|
||||
from signal_bot.stats.events import MessageEvent
|
||||
from signal_bot.stats.periods import WARSAW
|
||||
@ -125,3 +127,44 @@ def test_an_album_is_wired_up_only_where_there_is_one(
|
||||
assert command is not None
|
||||
assert command.name == "pack:p1"
|
||||
assert command.groups == frozenset({WATCHED_GROUP})
|
||||
|
||||
|
||||
def test_reload_commands_picks_up_a_person_added_to_the_album(
|
||||
tmp_path: Path,
|
||||
config: Config,
|
||||
store: Store,
|
||||
transport: FakeTransport,
|
||||
directory: GroupDirectory,
|
||||
) -> None:
|
||||
"""A person tagged after startup is a command after a reload, not a restart."""
|
||||
root = pack_fixture.build(tmp_path / "pack")
|
||||
bot = _bot(replace(config, photo_pack_dirs=(root,)), store, transport, directory)
|
||||
before = bot.command_count
|
||||
assert bot.dispatcher.registry.resolve("!delta") is None
|
||||
|
||||
people = [*pack_fixture.PEOPLE, {"key": "p4", "triggers": ["!delta"]}]
|
||||
pack_fixture.build(root, people=people, tags={**pack_fixture.TAGS, "delta.png": ["p4"]})
|
||||
|
||||
assert bot.reload_commands() is True
|
||||
assert bot.dispatcher.registry.resolve("!delta") is not None
|
||||
assert bot.command_count == before + 1
|
||||
|
||||
|
||||
def test_reload_commands_keeps_serving_when_the_album_breaks(
|
||||
tmp_path: Path,
|
||||
config: Config,
|
||||
store: Store,
|
||||
transport: FakeTransport,
|
||||
directory: GroupDirectory,
|
||||
) -> None:
|
||||
"""A half-edited manifest must not take the working commands down with it."""
|
||||
root = pack_fixture.build(tmp_path / "pack")
|
||||
bot = _bot(replace(config, photo_pack_dirs=(root,)), store, transport, directory)
|
||||
(root / "people.json").write_text("{not json", encoding="utf-8")
|
||||
|
||||
assert bot.reload_commands() is False
|
||||
assert bot.dispatcher.registry.resolve("!alfa") is not None
|
||||
assert bot.dispatcher.holder.last_error
|
||||
# And the real loader is what said no -- the same error a restart would die on.
|
||||
with pytest.raises(ConfigError):
|
||||
_bot(replace(config, photo_pack_dirs=(root,)), store, transport, directory)
|
||||
|
||||
@ -10,6 +10,7 @@ import pytest
|
||||
|
||||
from signal_bot.dispatcher import Dispatcher
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
from tests.conftest import OTHER_GROUP, SECOND_GROUP, WATCHED_GROUP, envelope
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@ -56,7 +57,7 @@ def dispatcher(
|
||||
return Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
@ -156,7 +157,7 @@ async def test_a_failing_command_does_not_kill_the_loop(
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
@ -184,7 +185,7 @@ async def test_a_reply_goes_back_to_the_group_that_asked(
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
|
||||
@ -9,6 +9,7 @@ from typing import TYPE_CHECKING
|
||||
from signal_bot.decorators import cooldown
|
||||
from signal_bot.dispatcher import Dispatcher
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
from signal_bot.stats.queries_day import day_rows
|
||||
from tests.conftest import SECOND_GROUP, WATCHED_GROUP, envelope
|
||||
|
||||
@ -40,7 +41,7 @@ async def test_a_refused_command_records_nothing(
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
@ -72,7 +73,7 @@ async def test_waiting_the_stated_time_lets_the_command_run(
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: clock[0],
|
||||
@ -110,7 +111,7 @@ async def test_a_scoped_command_in_another_group_is_not_a_command_at_all(
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
@ -141,7 +142,7 @@ async def test_the_words_after_a_trigger_reach_the_handler(
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
|
||||
@ -19,6 +19,7 @@ import pytest
|
||||
from signal_bot.analytics import probe
|
||||
from signal_bot.dispatcher import Dispatcher
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
from tests.conftest import WATCHED_GROUP, envelope
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@ -61,7 +62,7 @@ def build(
|
||||
return Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
|
||||
@ -17,6 +17,7 @@ from signal_bot.context import CommandContext
|
||||
from signal_bot.discovery import GroupDirectory
|
||||
from signal_bot.dispatcher import Dispatcher
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
from tests.conftest import WATCHED_GROUP, envelope
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@ -53,7 +54,7 @@ def dispatcher(
|
||||
return Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: NOW,
|
||||
|
||||
@ -27,6 +27,7 @@ from signal_bot.pack_load import load
|
||||
from signal_bot.pack_replies import FAILED
|
||||
from signal_bot.pack_store import ADDS_FILE
|
||||
from signal_bot.registry import CommandRegistry
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
from tests import pack_fixture
|
||||
from tests.conftest import WATCHED_GROUP, envelope
|
||||
from tests.pack_add_fixture import (
|
||||
@ -71,7 +72,7 @@ async def test_a_typed_label_reaches_no_log_line_no_row_and_no_reply(
|
||||
dispatcher = Dispatcher(
|
||||
config=config,
|
||||
directory=directory,
|
||||
registry=registry,
|
||||
holder=RegistryHolder(lambda: registry),
|
||||
store=store,
|
||||
transport=transport,
|
||||
clock=lambda: dt.datetime(2026, 8, 31, 12, 0, tzinfo=dt.UTC),
|
||||
|
||||
@ -38,9 +38,10 @@ def test_a_pack_that_no_longer_loads_keeps_the_old_one_and_does_not_raise(
|
||||
holder = PackHolder(load(root))
|
||||
before = holder.pack
|
||||
|
||||
# A tag naming a file that is not there: exactly what a half-rolled-back
|
||||
# write leaves behind.
|
||||
(root / "tags.json").write_text(json.dumps({"vanished.png": ["p1"]}), encoding="utf-8")
|
||||
# Half a write: exactly what a page interrupted mid-save leaves behind.
|
||||
# (A tag naming a vanished file used to be the example here; since
|
||||
# 2026-09-04 that is skipped with a warning rather than refused.)
|
||||
(root / "tags.json").write_text('{"alfa.png": ["p1"', encoding="utf-8")
|
||||
|
||||
assert holder.reload() is False
|
||||
assert holder.pack is before
|
||||
|
||||
@ -158,10 +158,21 @@ def test_an_untagged_pack_loads_and_warns(tmp_path: Path, caplog: pytest.LogCapt
|
||||
assert "./web.sh" in caplog.text
|
||||
|
||||
|
||||
def test_a_tag_naming_a_file_that_is_gone_is_refused(tmp_path: Path) -> None:
|
||||
"""A renamed file would otherwise drop out of the album in silence."""
|
||||
with pytest.raises(ConfigError, match="not a file"):
|
||||
_load(tmp_path, tags={"alfa.png": ["p1"]}, media={})
|
||||
def test_a_tag_naming_a_file_that_is_gone_is_skipped_out_loud(
|
||||
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""Drift in one row must not take the album down; it is named, then ignored.
|
||||
|
||||
Fatal until 2026-09-04, when two such rows in the live pack meant the
|
||||
next restart would have looped on ConfigError.
|
||||
"""
|
||||
pack = _load(
|
||||
tmp_path,
|
||||
tags={"alfa.png": ["p1"], "gone.jpg": ["p2"]},
|
||||
media={"alfa.png": pack_fixture.PIXEL},
|
||||
)
|
||||
assert set(pack.tags) == {"alfa.png"}
|
||||
assert "'gone.jpg', which is not a file in media/ -- skipped" in caplog.text
|
||||
|
||||
|
||||
def test_a_tag_may_not_reach_outside_the_album(tmp_path: Path) -> None:
|
||||
|
||||
109
tests/test_registry_holder.py
Normal file
109
tests/test_registry_holder.py
Normal file
@ -0,0 +1,109 @@
|
||||
# Copyright (c) 2026 Krzysztof Rudnicki. MIT licence.
|
||||
"""The registry holder: rebuilt whole, swapped once, never left half-built."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
|
||||
from signal_bot.groups import ConfigError
|
||||
from signal_bot.registry import CommandRegistry, DuplicateTriggerError
|
||||
from signal_bot.registry_holder import RegistryHolder
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from signal_bot.context import CommandContext
|
||||
|
||||
|
||||
async def _noop(ctx: CommandContext) -> None:
|
||||
"""A handler that does nothing; only the table's shape matters here."""
|
||||
|
||||
|
||||
def _table(*names: str) -> CommandRegistry:
|
||||
registry = CommandRegistry()
|
||||
for name in names:
|
||||
registry.register(name, [f"!{name}"], _noop, f"{name} description")
|
||||
return registry
|
||||
|
||||
|
||||
class _Builder:
|
||||
"""A builder whose answer can be changed between calls, like data on disk."""
|
||||
|
||||
def __init__(self, *names: str) -> None:
|
||||
self.names = names
|
||||
self.error: Exception | None = None
|
||||
self.calls = 0
|
||||
|
||||
def __call__(self) -> CommandRegistry:
|
||||
self.calls += 1
|
||||
if self.error is not None:
|
||||
raise self.error
|
||||
return _table(*self.names)
|
||||
|
||||
|
||||
def test_the_first_table_is_built_at_construction() -> None:
|
||||
builder = _Builder("cat")
|
||||
holder = RegistryHolder(builder)
|
||||
assert builder.calls == 1
|
||||
assert holder.registry.resolve("!cat") is not None
|
||||
assert holder.last_error == ""
|
||||
|
||||
|
||||
def test_a_bad_table_at_startup_is_fatal() -> None:
|
||||
"""No previous table to keep, so the unit must fail rather than answer nothing."""
|
||||
builder = _Builder("cat")
|
||||
builder.error = ConfigError("tags.json names a file that is not there")
|
||||
with pytest.raises(ConfigError):
|
||||
RegistryHolder(builder)
|
||||
|
||||
|
||||
def test_reload_publishes_the_new_table() -> None:
|
||||
builder = _Builder("cat")
|
||||
holder = RegistryHolder(builder)
|
||||
builder.names = ("cat", "dog")
|
||||
|
||||
assert holder.reload() is True
|
||||
assert holder.registry.resolve("!dog") is not None
|
||||
assert {command.name for command in holder.registry.commands} == {"cat", "dog"}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
ConfigError("people.json is not valid JSON"),
|
||||
DuplicateTriggerError("!cat is claimed twice"),
|
||||
],
|
||||
)
|
||||
def test_bad_data_keeps_the_previous_table(
|
||||
error: Exception, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""The table that was answering keeps answering; the caller learns why."""
|
||||
builder = _Builder("cat")
|
||||
holder = RegistryHolder(builder)
|
||||
before = holder.registry
|
||||
builder.error = error
|
||||
|
||||
assert holder.reload() is False
|
||||
assert holder.registry is before
|
||||
assert holder.last_error == str(error)
|
||||
assert "keeping the previous one" in caplog.text
|
||||
|
||||
|
||||
def test_a_later_good_reload_clears_the_error() -> None:
|
||||
builder = _Builder("cat")
|
||||
holder = RegistryHolder(builder)
|
||||
builder.error = ConfigError("broken")
|
||||
holder.reload()
|
||||
builder.error = None
|
||||
|
||||
assert holder.reload() is True
|
||||
assert holder.last_error == ""
|
||||
|
||||
|
||||
def test_a_programming_error_is_not_swallowed() -> None:
|
||||
"""Only bad *data* is survivable; a bug must surface, not become a 409."""
|
||||
builder = _Builder("cat")
|
||||
holder = RegistryHolder(builder)
|
||||
builder.error = TypeError("register() got an unexpected keyword")
|
||||
with pytest.raises(TypeError):
|
||||
holder.reload()
|
||||
@ -42,15 +42,25 @@ TAGS: Final = {
|
||||
}
|
||||
|
||||
|
||||
def write(directory: Path, group: str) -> None:
|
||||
"""Write the manifest, the tags and the files themselves."""
|
||||
def write(directory: Path, group: str, extra: str | None = None) -> None:
|
||||
"""Write the manifest, the tags and the files themselves.
|
||||
|
||||
``extra`` adds one more person, ``!<extra>`` with one photo, on top of
|
||||
the fixed four -- run a second time over the same directory, that is
|
||||
what "somebody tagged a new person while the bot was up" looks like,
|
||||
and what POST /_reload has to pick up without a restart.
|
||||
"""
|
||||
people, tags = list(PEOPLE), dict(TAGS)
|
||||
if extra is not None:
|
||||
people.append({"key": "p5", "triggers": [f"!{extra}"]})
|
||||
tags[f"{extra}.png"] = ["p5"]
|
||||
album = directory / "media"
|
||||
album.mkdir(parents=True, exist_ok=True)
|
||||
for name in TAGS:
|
||||
for name in tags:
|
||||
(album / name).write_bytes(CLIP if name.endswith(".mp4") else PIXEL)
|
||||
manifest = {"groups": [group], "people": PEOPLE, "combos": COMBOS}
|
||||
manifest = {"groups": [group], "people": people, "combos": COMBOS}
|
||||
(directory / "people.json").write_text(json.dumps(manifest, indent=2), encoding="utf-8")
|
||||
(directory / "tags.json").write_text(json.dumps(TAGS, indent=2), encoding="utf-8")
|
||||
(directory / "tags.json").write_text(json.dumps(tags, indent=2), encoding="utf-8")
|
||||
|
||||
|
||||
def main() -> int:
|
||||
@ -58,8 +68,9 @@ def main() -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--dir", required=True, type=Path, help="where to write the pack")
|
||||
parser.add_argument("--group", required=True, help="the group id the pack belongs to")
|
||||
parser.add_argument("--extra", help="one more person to add, by trigger word")
|
||||
arguments = parser.parse_args()
|
||||
write(arguments.dir, arguments.group)
|
||||
write(arguments.dir, arguments.group, arguments.extra)
|
||||
sys.stdout.write(f"== wrote a synthetic pack to {arguments.dir}\n")
|
||||
return 0
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user