Review finding (low, doc-only): the rollback plan said each stage is one
independently-revertable commit, but each stage is actually two commits on its
branch (feature + review-round fix, the latter often fixing a real bug), so
reverting a single commit would orphan the review-round fix. Reword the unit of
rollback to the whole STAGE (branch/PR) and spell out the revert command per merge
strategy: squash-merge -> revert the one squash commit; merge-commit -> revert -m 1
the merge; linear history -> revert the full range of the stage's commits.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Caps the stacked stability work (stages 1-6). Verification only — ZERO production
code change (git diff against fix/stage-6-healthcheck touches only these two files).
- tests/test_stage7_integration.py (8 cross-stage integration tests via TestClient +
a mocked TelegramClient, no network): Range at the /media route level (206/206/416
through get_media -> prepare_file_response -> FileResponse); /ping stays prompt and
issues zero TG RPC while a slow op is parked; in-flight dedup shares one download and
drains _inflight after completion AND after request cancellation (no stuck key / hung
waiter); str(channel) access-time hit -> flush -> the bulk UPDATE lands on the seeded
row (mutation-verified: transposing the key columns reds it).
- docs/stability-verification.md: per-stage DoD -> evidence mapping (test or "operator
observation" for prod-only items), the exact manual curl/lsof scenarios for the
operator to run post-deploy, the diag-log signals to watch, and the per-stage
independent-commit rollback plan.
Full integrated suite: 260 passed (252 baseline + 8). The prod deploy + live diag-log
observation (plan items 3-4) are the operator's call — this stage does not deploy.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review finding (low, real): the comment claimed /ping degenerates to a pure
connectivity check when TG_WATCHDOG_ENABLED=false, but _wd_last_ok_monotonic is
also stamped by _restart_client (on the disconnect-flap path, which runs before
the watchdog-enabled gate), so with the watchdog off one flap sets age and nothing
ever refreshes it — age grows unbounded past the threshold and /ping returns 503 on
a live connection, spuriously failing the container healthcheck and triggering an
autoheal restart after every flap.
Fix: gate the staleness branch on the watchdog being enabled —
healthy = connected and (not Config["tg_watchdog_enabled"] or age is None or age < threshold)
so with the watchdog disabled /ping is a pure connectivity check (matching the
intent), and correct the comment to note a flap-restart can stamp age even when the
watchdog is off. New test test_ping_watchdog_disabled_stale_age_still_healthy:
watchdog off + connected + stale age => 200 ok. Adversarially validated — reverting
the gate reds the new test (503) while the watchdog-ON stale-probe test stays green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The container healthcheck hit /rss/...?limit=1 (5s timeout): on a cold cache RSS
generation exceeds 5s, or a hung TG RPC makes it hang, so docker/autoheal restarts
the container mid-download and corrupts temp files. Replace it with /ping, which
reflects process/loop liveness (answers instantly, always) plus TG liveness read
from the watchdog's last-probe data — issuing ZERO Telegram RPC.
- telegram_client: public watchdog_last_ok_age() — seconds since the last successful
watchdog probe (None if never). Pure read of the Stage-1 _wd_last_ok_monotonic
field; no RPC.
- api_server: /ping route (no token, no TG RPC, no SQLite, no fs scan). healthy =
connected and (age is None or age < threshold). age is None right after boot =>
healthy (don't kill before the first probe). connected coerced to bool so the JSON
"connected" field is always a bool (pre-start reports false, never null).
- config: TG_PING_UNHEALTHY_AFTER knob, default interval*(failures+1)+timeout = 250s
(how long until the watchdog itself gives up), env-overridable.
- dockercompose.yml: healthcheck -> curl -sf http://127.0.0.1:80/ping, interval 5m,
timeout 5s, retries 3, start_period 30s. Old /rss check removed, not left behind.
- tests: 10 (healthy/stale/disconnected/fresh-boot/pre-start-null/no-token +
the anti-regression zero-TG-RPC spy across all branches + the accessor).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 [security] The final feed-sanitize try/except was FAIL-OPEN: since 4.4 removed
the per-fragment passes, post['html'] / the concatenated feed html is now raw
channel-controlled HTML, and this is its ONLY sanitize. If bleach itself throws
(RecursionError/OOM on pathological nested HTML — a class already seen in this
project), the except branch returned the RAW payload = stored XSS. Both branches
(RSS per-post, HTML whole-feed) now fail CLOSED via html.escape, so the content
survives as inert text and no live tag ever reaches the client. Added a test that
forces bleach to raise and asserts no live <script>/<img>/<a> reaches the feed
(adversarially validated: reverting to fail-open makes it fail).
#2 [test] Direct test of the new upsert_media_file_ids_bulk_sync on a temp DB:
empty no-op, multi-row insert, and re-upsert-updates-added (the real executemany +
ON CONFLICT, previously only mocked).
#3 [test] Test that media ids collected before a render exception are still
flushed (the flush is in a finally) — removing the finally now turns a test red.
#4 [cleanup] Removed the dead in rss_generator (the
flush is delegated to post_parser._flush_pending_media_ids).
#5 [doc] Corrected the sanitize-map comments: RSS sanitizes per-post, only HTML is
the whole-feed pass.
233 passed (214 baseline + 19).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Feed generation blocked the loop (deepcopy of hundreds of Messages + per-message
bleach x4 + str(message) per post). This moves the CPU work off the loop and cuts
it down.
4.1 raw_message is lazy: process_message(include_raw=False) omits str(message)
entirely; only /json and debug-HTML compute it.
4.2 (done BEFORE 4.3) _save_media_file_ids no longer touches asyncio/DB — it
appends to the per-request PostParser._pending_media_ids, and the caller
flushes once via to_thread(upsert_media_file_ids_bulk_sync) (a new executemany
fn in file_io.py). Removed _persist_pending_count + the create_task machinery.
This had to precede 4.3 or get_running_loop() inside the render thread would
raise and silently kill media-id persistence.
4.3 The render pipeline (_create_time_based_media_groups, _create_messages_groups,
_trim_messages_groups, _render_messages_groups) is now plain-sync and runs in
ONE asyncio.to_thread(_render_pipeline) from both feed generators; deepcopy
moved into the thread. The render path is verified free of asyncio primitives.
4.4 Sanitize is now ONE pass per output boundary (removed the 4 internal
per-fragment bleach passes): RSS/HTML feeds sanitize once at the whole-feed
pass; /html and /json sanitize body+footer once in process_message; the debug
<pre> raw dump is html.escape'd. Media embeds in body, reactions in footer, so
both are covered by the boundary pass. XSS tests (<script>/onerror=/javascript:)
confirm all four outputs are clean.
Review round-1: documented that flags are now extracted from the pre-sanitize body
(a 4.4 consequence — non-security, legitimate links unaffected); added
media-caption XSS tests for the exact fragments whose internal passes were removed
(adversarially validated: neutering the sanitizer makes them fail); the media-id
flush is now in a finally so a partial render still persists what it collected.
230 passed (214 baseline + 16).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_vscode
agent_coder
claude code agent