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>
agent_vscode
agent_coder