lekss361/gendesign

Fork 0

fix(tgbot): проверка темы при старте + общий rate limit на группу (#3471) #3494

Merged

lekss361 merged 4 commits from feat/3471-tg-topic-check-and-group-rate-limit into main

2026-09-12 13:37:10 +00:00

bot-backend commented

2026-09-12 12:00:36 +00:00

Collaborator

Summary

Startup-проверка (app.tgbot_main): verify_chat_and_topic дважды вызывается перед run_poll_loop (support + alerts) — getChat + sendChatAction (typing-индикатор с message_thread_id) без единого видимого сообщения; при отказе — logger.error, процесс не падает.
TelegramGroupRateLimiter (скользящее окно, ключ chat_id, не тема) встроен в TelegramClient._request перед _post — считает все исходящие независимо от relay/прямого пути, без изменений в support.py/glitchtip.py (они уже идут через общий shared.get_telegram_client()). Per-chat asyncio.Lock — без гонок при нескольких конкурентных отправителях. Порог — TELEGRAM_GROUP_RATE_LIMIT_PER_MINUTE (дефолт 18).

Test plan

ruff check — чисто на изменённых файлах
pytest tests/services/tgbot/ — 42 passed (12 новых + 30 существующих)
Откат 4 изменённых файлов к main подтвердил: новые тесты падают ImportError без правки

🤖 Generated with Claude Code

https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG

## Summary - Startup-проверка (`app.tgbot_main`): `verify_chat_and_topic` дважды вызывается перед `run_poll_loop` (support + alerts) — `getChat` + `sendChatAction` (typing-индикатор с `message_thread_id`) без единого видимого сообщения; при отказе — `logger.error`, процесс не падает. - `TelegramGroupRateLimiter` (скользящее окно, ключ `chat_id`, не тема) встроен в `TelegramClient._request` перед `_post` — считает все исходящие независимо от relay/прямого пути, без изменений в `support.py`/`glitchtip.py` (они уже идут через общий `shared.get_telegram_client()`). Per-chat `asyncio.Lock` — без гонок при нескольких конкурентных отправителях. Порог — `TELEGRAM_GROUP_RATE_LIMIT_PER_MINUTE` (дефолт 18). ## Test plan - [x] ruff check — чисто на изменённых файлах - [x] pytest tests/services/tgbot/ — 42 passed (12 новых + 30 существующих) - [x] Откат 4 изменённых файлов к main подтвердил: новые тесты падают ImportError без правки 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG

bot-backend added 1 commit 2026-09-12 12:00:37 +00:00

fix(tgbot): проверка темы при старте + общий rate limit на группу (#3471 )

CI Trade-In / changes (pull_request) Successful in 8s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI / changes (pull_request) Successful in 11s

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 8m55s

Details

6433477f7c

Бот падал молча на каждом сообщении, если тему форума удалили/переименовали:
узнавали об этом только по отсутствию сообщений у людей. tgbot_main теперь
один раз на старте проверяет getChat + typing-индикатор с message_thread_id
(единственный способ Bot API провалидировать message_thread_id без создания
видимого сообщения) и громко пишет error при отказе, не роняя процесс.

Второе: лимит Telegram (~20 msg/min) общий на всю группу, все темы делят
бюджет — всплеск GlitchTip-алертов вместе с потоком поддержки в ту же группу
уже давал 429 и терял сообщения. TelegramGroupRateLimiter — скользящее окно
per-chat_id (НЕ per-теме) с asyncio.Lock на чат, встроен прямо в
TelegramClient._request перед _post, поэтому считает все отправки независимо
от relay/прямого пути и без изменений в support.py/glitchtip.py (они уже идут
через общий клиент). Порог настраивается через
TELEGRAM_GROUP_RATE_LIMIT_PER_MINUTE (дефолт 18, чуть ниже потолка площадки).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG

bot-backend referenced this pull request from a commit

2026-09-12 12:28:20 +00:00

fix(tgbot): honest H1 rejection, per-role H2 budget, M1/M2/L1 cleanup (#3471 review)

bot-backend added 1 commit 2026-09-12 12:28:21 +00:00

fix(tgbot): honest H1 rejection, per-role H2 budget, M1/M2/L1 cleanup (#3471 review)

CI Trade-In / backend-tests (pull_request) Failing after 2m29s

Details

CI Trade-In / changes (pull_request) Successful in 9s

Details

CI / changes (pull_request) Successful in 11s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

8e7c65061b

Deep review of PR #3494 found the rate limiter unusable as designed:
- H1: acquire() waited unbounded even for interactive HTTP handlers (support.py,
  glitchtip.py already pass a narrow `timeout` — reuse it as the queue wait cap
  instead of editing those handlers, which are out of scope here). New
  TelegramRateLimitedError (subclass of TelegramError) gives a fast, honest
  502 instead of hanging past the caller's own budget.
- H2: the limiter is per-process (in-memory), but two processes write to the
  same group (uvicorn API + bot worker) — giving each the same 18/min doubled
  the platform ceiling. Split into telegram_group_rate_limit_api_per_minute
  (12) and _bot_per_minute (6), sum kept below ~20.
- M1: bridge.py sends without an explicit timeout inherited "wait forever",
  stalling the single-threaded poll loop (open DB session) past the SIGTERM
  drain window. Bounded via rate_limit_max_wait=20s at the six call sites.
- M2: _locks/_sent_at grew unbounded on every unique DM chat_id. Added
  opportunistic cleanup of fully-expired entries.
- L1: the "queue full" warning now logs once per acquire() call, not once
  per sleep iteration.
- Corrected a factual error in the docstring: TELEGRAM_SUPPORT_CHAT_ID and
  TELEGRAM_ALERTS_CHAT_ID are the SAME group on prod (topics differ only).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG

bot-backend added 1 commit 2026-09-12 12:34:42 +00:00

Merge remote-tracking branch 'forgejo/main' into feat/3471-tg-topic-check-and-group-rate-limit

CI / changes (pull_request) Successful in 13s

Details

CI Trade-In / changes (pull_request) Successful in 11s

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Failing after 2m27s

Details

acbfdf0a18

bot-backend referenced this pull request from a commit

2026-09-12 13:10:31 +00:00

fix(tgbot): stop CI-hanging busy-spin in rate-limit test, isolate shared client in tests

bot-backend added 1 commit 2026-09-12 13:10:32 +00:00

fix(tgbot): stop CI-hanging busy-spin in rate-limit test, isolate shared client in tests

CI Trade-In / changes (pull_request) Successful in 8s

Details

CI Trade-In / browser-tests (pull_request) Has been skipped

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / changes (pull_request) Successful in 11s

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 7m27s

Details

8142834555

Root cause of the red PR #3494 CI job (9% progress, 75s life, no error line):
test_send_message_rate_limits_across_different_topics_same_chat mocked
asyncio.sleep as a pure no-op without advancing time.monotonic. The 3rd send
(over the test's limit=2) entered TelegramGroupRateLimiter.acquire(), which
recomputes wait_s from the real, unmocked clock every iteration - since the
fake sleep never advances it, the window never expires and the while-loop
busy-spins forever instead of actually waiting, until pytest-timeout kills it.
Fixed by advancing a fake monotonic clock inside fake_sleep, matching the
already-correct pattern used by the other tests in this file.

Also added _reset_telegram_shared_client (tests/conftest.py, same pattern as
_reset_estimate_rate_limiter): app.services.tgbot.shared._client is a
module-level singleton whose rate limiter otherwise accumulates real
wall-clock timestamps across the whole pytest session, not per test.

Documented honestly in config.py: the API-role budget is shared between
support web-chat mirrors and GlitchTip alerts with no priority between them,
so a large alert burst can make the web-chat wait out its own timeout and
return 502 - flagged as a known follow-up, not fixed here.

NOTE: a full `pytest -q --timeout=60` run still hangs further into the suite,
at tests/test_glitchtip_webhook.py::test_telegram_failure_returns_502_not_500.
Not root-caused within this session's budget - the test's _fake_telegram_client
fixture correctly monkeypatches glitchtip_module.get_telegram_client, but the
anyio worker thread running the ASGI request is seen parked in a real
event-loop poll/select wait, consistent with an actual (non-mocked) sleep
somewhere in that path. Needs a follow-up session with a fresh time budget.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG