Review findings (both low, no bugs; accumulator design adversarially confirmed):
- test-coverage: the DoD's hottest changed site — get_media's pre-semaphore
cache-hit — had no direct zero-SQLite guard (the spy test only exercised
download_media_file), so a regression re-introducing a per-hit write into
the get_media branch would pass green. Add a mirror spy test through
get_media asserting the accumulator is written and update_media_file_access_sync
is NOT called. Verified site-specific: neutering only the get_media write
reds the new test while the download_media_file test stays green.
- documentation: the _access_updates comment claimed a str/int key mix "would
make the WHERE silently never match" — empirically false: channel is a TEXT
column, so a bound int is affinity-coerced and DOES match. Reword to say we
key str(channel) to stay consistent with the stored form rather than lean on
SQLite's implicit coercion (the code was already correct on both sites).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stage 5. A /media cache hit no longer touches SQLite: it records the access
timestamp into a module-level accumulator (a dict write on the event loop,
cheap and atomic), and a supervised 60s background task flushes the whole
batch in one executemany UPDATE. This removes both per-hit write sites — the
awaited to_thread in download_media_file and the fire-and-forget create_task
in the pre-semaphore fast path — so under active RSS polling the threadpool is
no longer starved by per-request access-time UPDATEs.
- file_io: add update_media_file_access_bulk_sync (one connection, executemany;
empty batch is a no-op).
- api_server: _access_updates accumulator + _flush_access_updates (snapshot-
then-clear atomically before the await so writes during the flush land in the
fresh dict; re-queue the batch with setdefault on write failure so a fresher
concurrent write is never clobbered and no access-time is lost) +
_access_flush_loop under _supervised + a final flush on shutdown, ordered
after the loop task is cancelled and before the io threadpool is shut down.
- Keys use str(channel) to match the TEXT channel column (a str/int mix would
make the UPDATE WHERE silently never match, evicting still-used files).
- tests/test_stage5_sqlite.py: 7 tests (no-sync-write hot path, str-key
discipline, hit->flush->DB, empty no-op, snapshot-then-clear race, re-queue-
without-clobbering-fresh, bulk SQL).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#1 [medium] FileResponse with stat_result=None re-stats at send-time and raises a
RuntimeError (-> 500 + stacktrace, escaping get_media's try/except) if the file was
swept between the early exists() check and the send. The old streaming code caught
FileNotFoundError on getsize -> 404. Now prepare_file_response takes ONE
authoritative os.stat (in try/except FileNotFoundError -> 404) after the temp_*
touch and passes it as FileResponse(stat_result=...): restores the 404 semantics,
collapses the double stat into one, and makes the ETag reflect the observed mtime.
The remaining narrow deleted-between-our-stat-and-open window pre-existed.
#2/#3 [doc] Corrected the Content-Disposition comment (FileResponse emits
filename="x" for ASCII, filename*=UTF-8''x only for non-ASCII, via setdefault — a
manual header would override, not double) and the ETag-stability comment (stable
within a 300s window; a longer view costs at most one safe 200 If-Range restart
per interval).
#4 [simplification] Removed the now-dead StreamingResponse import and corrected
get_media's return annotation to -> Response (nothing returns StreamingResponse
after the FileResponse migration).
#5 [test] Added a test for the middleware's non-http (lifespan/websocket) branch —
it only runs in a real deploy, never through TestClient — asserting it delegates
to the inner app and logs nothing.
214 passed (195 baseline + 19).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces the hand-rolled media streaming with Starlette FileResponse, drops the
BaseHTTPMiddleware, and enlarges the default threadpool.
3.1 prepare_file_response now returns FileResponse (handles Range/If-Range/206/
416/multipart, sets Accept-Ranges/ETag/Last-Modified, reads efficiently — no
per-64KB to_thread hop that starved the pool). Kept: the early 404 pre-check,
the MIME logic (python-magic + SQLite cache), and every stage-2 behavior — the
temp_* mtime touch (now DEBOUNCED, see below), delete_after -> BackgroundTask
(passed as FileResponse background=), the media_key MIME cache. Removed the
manual Range parsing, file_chunk_generator, and hand-built headers;
Content-Disposition is formed by FileResponse from filename= (no double-set).
206 slices are byte-identical to the old code; accepted RFC-7233 deltas
documented in the tests.
3.2 RequestLoggingMiddleware rewritten as a pure-ASGI class (wraps only send to
observe the status line, never buffers the body, passes non-http scopes
through) — the streaming body flows untouched.
3.3 lifespan sets a larger default executor (ThreadPoolExecutor, IO_THREAD_POOL_SIZE
default 32) and shuts it down on exit.
Review round-1 fixes folded in: the temp_* mtime touch is DEBOUNCED
(TEMP_MTIME_REFRESH_INTERVAL=300s) so FileResponse's mtime-derived ETag stays
stable across a resume/seek session (an every-serve touch broke If-Range resume);
starlette pinned to 0.45.3; the io executor is shut down on lifespan exit; the
ASGI logger includes the query string.
Tests (tests/test_stage3_fileresponse.py, 18): the Range matrix vs FileResponse
with every delta documented; temp_* mtime refreshed when stale AND stable when
fresh (ETag identical); delete_after background runs and removes the file;
media_key MIME cache hit/miss; the ASGI middleware passes the body and logs.
213 passed (195 baseline + 18).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F1 [WARNING] The HTTP_DOWNLOAD_SEMAPHORE admission-control balance was untested,
and it is a plain asyncio.Semaphore — so an over-release would SILENTLY inflate
the permit count and disable the limiter (no ValueError), a green suite hiding the
exact bug stage 2 fixes. Added two tests (mirroring the stage-1 tg_throttle
balance test): the permit is released exactly once when the download errors (count
back to baseline), and a timed-out acquire (503) releases NOTHING (count
unchanged). Adversarially validated: neutralizing the finally release makes the
first test fail, so it genuinely catches a leak.
F2 [low] Corrected the _download_deduped docstring: the detached task sets the
Future in try/except, and its finally ALWAYS pops the key — the previous wording
attributed the Future-set to the finally, which an auditor reading the finally
block would find contradicted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes flaky static media serving: partial files served as 'ready', truncated
stubs living for an hour, and app-wide hangs under a saturated download path.
2.1 _download_atomic(file_id, final_path, timeout): downloads to a unique
{final}.part.{hex}, validates size>0, and publishes solely via os.rename
(atomic on POSIX); a finally ALWAYS removes the partial (timeout/cancel/
zero-size/race-loser). Every downloader call routes through it, so a file at a
FINAL name ({fid} / temp_{fid}) is GUARANTEED complete (grep-proven: the only
safe_download_media caller passes a .part. path). Big-video timeout scales with
size (min/max/min-speed knobs, documented in dockercompose.yml). The sweeper
regex now matches both .part. and legacy .tmp. stubs.
2.2 In-flight dedup registry: the first request for a key runs the download in a
DETACHED task sharing a Future; the task's finally sets the Future AND pops the
key. Waiters await asyncio.shield(fut) so a client disconnect / waiter timeout
cancels only the waiter, never the download — no hung waiters, no stuck key, a
failed download frees the key for retry.
2.3 FloodWait->429: handler before except RPCError (FloodWait subclasses it),
Retry-After = min(value + rand(1,30), 300); propagates from the detached task
through the Future.
2.4 Serving a temp_* file touches its mtime so the 1h sweeper can't delete a
video out from under a viewer.
2.5 The HTTP download semaphore acquire is bounded (wait_for 30 -> 503 +
Retry-After); the permit is released only if the acquire succeeded. The
request-scoped-permit trade-off (a disconnect can transiently exceed the
download count) is documented inline for stage-7 prod observation.
Tests (tests/test_stage2_static.py, 13): atomic publish/clean on every exit incl.
FloodWait-through-the-finally; concurrent big-video serves no partial; dedup runs
one download, a cancelled waiter doesn't hang others, a failed download frees the
key; FloodWait -> 429; mtime touch; sweeper cleans .part./.tmp./stale temp_ but
keeps fresh files. 193 passed (180 baseline + 13).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F1 [WARNING] pytest-asyncio was not declared, so the 6 new async stage-1 tests
ERROR on a clean checkout (async def not natively supported) — zero regression
protection, masked by a globally-installed plugin. Added pytest-asyncio to
requirements.txt + asyncio_mode=auto to tests/pytest.ini. Verified on a fresh venv
from requirements alone: the tests collect and run (180 passed).
F2 [WARNING] The /raw_json endpoint's get_messages was the one remaining live RPC
without a timeout, violating the stage-1 DoD ('every Telegram RPC is bounded').
Wrapped it in asyncio.wait_for(..., 30) mirroring PostParser.get_post. (It is not
under the tg_rpc gate, so its blast radius was one request, not the app.)
F3 [WARNING] The worker test globally no-op'd asyncio.sleep, so the dedicated
except FloodWait branch was indistinguishable from the generic handler — deleting
it kept the test green. The sleep stub now records delays and the test asserts the
FloodWait backoff of 6 (=min(1+5,900)), distinct from the success path's 2.
F5 [low] The tricky 'gate outside, timeout inside' nesting was open-coded at 3
sites (each re-deriving the invariant). Extracted tg_rpc_bounded(timeout) into
tg_throttle (using asyncio.timeout()); the 3 sites now use it, so a future call
site cannot silently wrap the gate entry and reopen the hang-under-backpressure.
F4 [low] Documented TG_RPC_TIMEOUT in the dockercompose.yml environment block
next to the other TG_RPC_* knobs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Eliminates the app-wide hangs where one stuck Telegram RPC holds the single
tg_rpc gate permit forever (freezing every RSS/HTML request) and where the
background download worker dies silently.
1.1 Every RPC under the global gate is now bounded by asyncio.wait_for
(config tg_rpc_timeout, env TG_RPC_TIMEOUT, default 60). The wait_for wraps
ONLY the RPC body, never the gate acquire, so queue backpressure stays
legitimate; the paginated get_chat_history is collected in an inner coroutine
so the async-for can be bounded. A timeout propagates out of the gate so its
permit is released (no leak).
1.2 The other live RPC paths get the same treatment: _reply_enrichment's
get_messages now runs under the gate + timeout; PostParser.get_post is bounded
at 30s (and a stray print removed); /health's get_me at 10s.
1.3 background_download_worker: queue.get() moved out of the try so task_done()
in finally balances exactly one get (no ValueError that killed the worker);
FloodWait is caught BEFORE the generic Exception (sleeps, does not flood);
download_new_files uses put_nowait so a full queue no longer blocks the
sweeper forever.
1.4 Both background tasks run under a _supervised() wrapper: a crash or an
unexpected return is logged CRITICAL and restarted (restarts rate-limited to
once per 60s so a hard-failing task can't spin), while CancelledError is
propagated to the child for a clean shutdown.
Tests (tests/test_stage1_hangs.py): gate timeout releases the permit + a second
call succeeds; gate cancel mid-spacing releases the permit; the worker survives
Exception and FloodWait with balanced task_done; and _supervised restarts a
crashing task (rate-limited) and an unexpected return, and propagates cancellation
to its child. 180 passed (174 baseline + 6).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert delayed_delete_file to async and use asyncio.sleep.
Add pre‑semaphore cache hit to serve cached files without acquiring the download semaphore.
Set SQLite busy_timeout to reduce lock errors.
Introduce a diagnostics counter for pending _persist_media_file_id_async tasks.
Refactor flag extraction to accept pre‑generated HTML and compute html body once.
Batch reply enrichment in RSS generator to minimize API calls.
Store cached messages directly in tg_cache and add fallback for legacy double‑pickle format.
BREAKING CHANGE: delayed_delete_file is now async and must be awaited.
- Use unique temporary file paths with UUID to avoid race conditions during concurrent downloads.
- Perform atomic rename of the temp file to the final cache location and handle existing concurrent downloads.
- Clean up zero‑size and stale temporary files, including new “.tmp.<hex>” pattern.
- Extend cleanup logic to detect and remove race‑condition temp files.
- Validate and convert environment variables (TG_API_ID, TG_PROXY_PORT, API_PORT) with clear error messages and proper exit handling.
- Flush prints and replace `os._exit` with `sys.exit` for graceful termination.
- Fix URL regex in `PostParser` and guard against missing channel usernames when generating media URLs.
- Deep‑copy message lists in RSS generator to prevent mutation across calls.
- Propagate `FloodWait` exceptions to be handled by the API server instead of retrying internally.
- Enhance `TelegramClient` disconnect handling with a brief pause, shutdown check, and reconnection reset.
- Refine auth retry logic to check for actual `KeyError` instances.
Added a guard in `api_server.download_media_file` to detect `None` or empty messages and return a 404 error, preventing downstream failures.
Enhanced `PostParser._extract_reactions` to handle paid reactions, custom emojis, and unknown types, aggregating counts correctly. Updated `_extract_flags` to skip paid and custom emoji reactions, ensuring flag detection works reliably.
The `is_local_request` function now supports trusted reverse proxies by:
- Reading `TRUSTED_PROXIES` from configuration.
- Resolving the real client IP from `X-Real-IP` or `X-Forwarded-For` headers when the request comes through a trusted proxy.
- Adding IPv6 loopback support and detailed logging for misconfigurations.
- Updating configuration to expose `trusted_proxies` via the `TRUSTED_PROXIES` environment variable.
Introduce a root GET route that serves a minimal HTML landing page describing the Pyrogram Bridge service and linking to its GitHub repository. This provides a friendly entry point for users accessing the service via a browser.
Replace string comparisons with the MessageMediaType.POLL enum and add the
necessary import. This ensures poll messages are correctly identified and
skipped during media processing.
Add a new `mime_type` column to the media_file_ids table and provide
functions to get and set cached MIME types. The API now attempts to
retrieve the MIME type from the database before falling back to
python-magic, persisting the detected type for future requests. This
reduces I/O overhead and improves response performance. Also adjust
logging to debug level to reduce noise.
Implement HTTP Range request support by replacing FileResponse with StreamingResponse.
Added async chunked generator, proper Content-Range, Accept-Ranges, and filename encoding headers.
Updated imports, endpoint signature, and background cleanup handling.
Switch persistence of media file identifiers from a JSON file to a
SQLite database. Introduce DB_PATH, init_db_sync, upsert, update, and
removal functions. Remove json-repair dependency and related locking
logic. Update api_server and post_parser to use the new database
helpers.
BREAKING CHANGE: media_file_ids.json is no longer used; existing
data must be migrated to the new SQLite database.
Add a `_shutting_down` flag to `TelegramClient` to suppress disconnect handling during intentional shutdown. The flag is set before sending SIGTERM in `_restart_app` and checked in `_on_disconnect` to ignore those events. Minor formatting adjustments were also applied to `api_server.py` (try block and `uvicorn.run` arguments).
Add monotonic timing instrumentation around media download, semaphore
wait, HTML sanitization, and media file persistence. Log warnings when
operations exceed defined thresholds and provide diagnostic info for
task queue size. This aids performance monitoring without altering
behaviour.
Separate live HTTP media requests and background cache workers by introducing `HTTP_DOWNLOAD_SEMAPHORE` and `BACKGROUND_DOWNLOAD_SEMAPHORE`. Remove the previous global `DOWNLOAD_SEMAPHORE`, diagnostic event‑loop monitor, and related logging. Refactor `download_media_file` to handle large videos with temporary files, clean up zero‑size cache entries, and update access timestamps without semaphore tracking. Adjust background worker and request handling to use the new semaphores, simplifying concurrency control and eliminating unnecessary diagnostics.
Introduce a periodic event‑loop monitor that logs active tasks, semaphore
state, queue size and thread‑pool activity. Track the number of coroutine
waiters for the download semaphore and emit detailed diagnostic logs in
download handling and the background worker. This aids debugging of
concurrency and resource‑usage issues.
Add queue-based background download system to improve performance and reliability:
- Introduce DOWNLOAD_SEMAPHORE to limit concurrent downloads to 3
- Add asyncio.Queue (maxsize 100) with dedicated worker for background processing
- Implement safe_get_messages and safe_download_media wrappers with timeout protection (30s and 120s)
- Add retry logic for KeyError auth failures with 5s backoff
- Replace synchronous sequential downloads with asynchronous queued processing
- Prevent event loop blocking by queuing files instead of immediate download
agent_coder
claude code agent
vvzvlad