Repository navigation
Fix broken reply-history mapping; harden web dashboard - #1
Merged
Merged
Conversation
The highest-priority bug: MessageForwardService returned the source message as a placeholder for queued sends, and Forward used that placeholder to record history mappings — so every queued forward stored (source_chat, source_id) -> (source_chat, source_id) instead of the real destination, silently breaking reply-threading on every run. Fixed via an on_sent callback that fires with the real sent message once the queue worker actually sends it, with regression tests covering the queued path. Also: - requirements.txt was missing several deps web/app.py needs (fastapi, uvicorn, jinja2, pydantic, python-dotenv, loguru, aiofiles) and pinned an obsolete `typing` backport package. - web/app.py's /api/* routes had no authentication and defaulted to binding 0.0.0.0; added a required X-API-Key check (fails closed if API_KEY isn't set) and switched the default bind to 127.0.0.1. - web/app.py's startup handler checked env vars the app never populates and did nothing; it now actually connects using the session created by the CLI. - Removed source/utils/RateLimiter.py (unused, zero references) and the postgres/redis/prometheus docker-compose services (unused by any app code; two referenced config files that don't exist in the repo). - Removed unused env vars from .env.example/docker-compose.yml that were never read anywhere in the codebase. - Credentials.py now reuses Constants.CREDENTIALS_FILE_PATH instead of a separately hardcoded path, and writes the file 0600 since it holds a plaintext api_hash. - Cleaned up a no-op try/except in Bot.start() and silent exception swallowing in main.py's shutdown(). Co-Authored-By: Claude <noreply@anthropic.com>
CI was entirely broken (this is the repo's first PR, so it seems nobody had actually run it): - pyproject.toml had no `dev`/`docs` extras, so `pip install -e ".[dev]"` silently installed none of ruff/mypy/pytest-asyncio/pytest-cov and every test-matrix job failed at "ruff: command not found". Added both extras with the tools each CI step needs. - `security` and `docs` jobs failed before running any step because actions/upload-artifact@v3 is hard-blocked as deprecated. Bumped to v4, plus setup-python v4->v5, cache v3->v4, codecov-action v3->v4. - `docs` job's `make html` had no docs/ directory to build against; added a minimal Sphinx scaffold (conf.py, index.rst, Makefile). - `mypy source/` had 16 pre-existing type errors: implicit-Optional defaults and a return-type mismatch in Chat.py, a tzinfo|None attribute access in DateUtils.py, and false positives from InquirerPy's execute_async() being mistyped as returning None. Fixed the real issues and added [tool.mypy] config (ignore_missing_imports, skip-follow-imports for InquirerPy) so the check reflects actual bugs. - `ruff format --check` failed on 13 files that had only ever been run through black (via pre-commit), which disagrees with ruff format on some edge cases. Reformatted the tree so CI and pre-commit agree. Co-Authored-By: Claude <noreply@anthropic.com>
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full review found and fixed the following, highest priority first:
MessageForwardService.forward_message/forward_albumreturned the source message as a placeholder when queuing a send (the real send happens later, asynchronously).Forward._forward_message/_forward_albumthen used that placeholder to record history, storing(source_chat, source_id) -> (source_chat, source_id)instead of the real destination mapping — silently breaking reply-threading, sinceTelegramalways constructs a real queue. Fixed via anon_sentcallback that fires with the actual sent message once the queue worker sends it; added regression tests for the queued path (tests/test_forwarding_service.py).requirements.txtwas missing depsweb/app.pyneeds (fastapi,uvicorn,jinja2,pydantic,python-dotenv,loguru,aiofiles) and pinned an obsoletetypingbackport that can break stdlib typing on 3.10+./api/*routes had no authentication and bound to0.0.0.0by default — added a requiredX-API-Keycheck (fails closed ifAPI_KEYunset) and switched the default bind to127.0.0.1.pass) —telegram_clientwas never initialized. It now reuses the session created by the CLI (python main.py) viaCredentials.get_all()+Telegram.create().source/utils/RateLimiter.py(RateLimitedQueue) — zero references anywhere.postgres/redis/prometheusdocker-compose.ymlservices — unused by any app code, and two of them referenced./docker/postgres/init.sql/./docker/prometheus.yml, which don't exist in the repo (their profiles would fail to start).TELEGRAM_API_ID,TELEGRAM_API_HASH,TELEGRAM_PHONE,BOT_TOKEN,LOG_LEVEL,LOG_FILE,MAX_CONCURRENT_TASKS,MESSAGE_QUEUE_DELAY,SECRET_KEY) from.env.example/docker-compose.yml— none were ever read by the app.Credentials.pynow reusesConstants.CREDENTIALS_FILE_PATHinstead of a separately hardcoded path, and writes the file0600(contains a plaintextapi_hash).try/exceptinBot.start()and silent exception-swallowing inmain.py'sshutdown().README.md/CHANGELOG.mdto match.Test plan
pytest tests/— 16/16 pass (includes 2 new regression tests for the history-mapping fix)ruff check— cleanblack --checkon touched files (reformatted the 2 files with new code)web/app.pyimports and registers all routes with the new depsX-API-Keyauth dependency: 503 with noAPI_KEYset, 401 on missing/wrong key, 200 with the correct key