The URL workers bound only their own path (8 workers, bounded channel); inline queries, /debug and /test spawned fetches straight from handler tasks with no limit at all — and the expensive part runs inside the fetch (ugoira: up to 512 MiB plus ffmpeg; bsky: an HLS remux), so N users could mean N concurrent encodes. fetch_with_attempts now takes one permit from an 8-slot gate (the count matching URL_WORKERS, so the bot's own pipeline keeps its full width), which covers every entry at once: message links, queue-side repair, inline, /debug, /test, and retries. Unsupported and disabled links answer before the gate, and the shared-fetch dedup already waits outside x-media, so nothing double-counts.
download_media_limited had one hard-coded total (600s) for every caller, and its heaviest caller — the bot's upload fallback — holds a PREP slot (and its memory reservation) for the whole transfer: six slow-but-alive downloads (a byte every 29s satisfies the idle window) could stall the fallback chain for ten minutes, queue retries included. The budget is a parameter now: the fallback passes 300s of its own (50 MiB in 300s ≈ 1.4 Mbit/s; a slower link is better served by retrying toward the item's smaller URL than by pinning a slot), while bsky's in-fetch HLS segments keep the generous 600s DOWNLOAD_TOTAL_TIMEOUT, now pub(crate) and re-exported for them. download_too_slow reports whichever budget it got.
download_media_to_file took &mut std::fs::File and wrote every network chunk with a sync write_all on the executor thread — for a ugoira frame zip (up to 512 MiB) that is the whole download stalling a runtime worker, while bsky's remux had already been moved to tokio::fs for exactly this reason (its comment: a multi-megabyte std::fs::write blocks the executor thread). The signature takes &mut tokio::fs::File now; the single caller (pixiv's ugoira path) clones the NamedTempFile's handle — the clone shares the file offset, so the ZipArchive extraction in spawn_blocking reads what the download wrote — and drops it after the download hands the bytes to the OS.
download_to_temp wrote the whole buffered body — up to MAX_MEDIA_UPLOAD_BYTES, 50 MiB — with a sync write_all on whatever executor thread ran the prep task; six PREP slots could stall six runtime threads at once on a slow volume (Docker volume, SD card), pausing unrelated handlers and chat-action refreshes with them. The write now runs in spawn_blocking, the pattern the photo decode in the same file already uses; the failure classes are unchanged (write error = retryable resource exhaustion, panic = permanent, worker bugs must be loud).
debug_report wrapped every caption in <blockquote>, but preview_caption for a post over CAPTION_QUOTE_TEXT_CHARS already carries quote_long_caption's expandable blockquote — and the API rejects nested entities, so /debug on any long post answered 400. The wrapper now skips a caption that already carries a blockquote, exactly the rule quote_long_caption itself applies; the new test pins that the quoted caption passes through with a single blockquote while the existing test keeps pinning the wrap for ordinary captions.
status_error's catch-all called every unlisted status Transient, so a 400/405/418/451 got three retries per link before the same answer (twitter syndication's broken-token 400 being the live example), and download_status_error plus misskey's and bilibili's local fallbacks each carried their own copy of the table — bilibili and misskey classifying a 404 as Transient while the center classified it NotFound. The center now makes any client error except 408/429 a refusal (permanent), the media path delegates to it as status_error("media", ...) and its duplicated fn is deleted, and misskey/bilibili fall through to the center after their own special statuses (misskey's 400 body, bilibili's 412). A table test pins every class.
download_media_to_file's non-transport errors all folded into PixivError::Api, and Api is permanent in pixiv_error_is_retryable — so one 429/5xx, stalled transfer or temp-file write failure while fetching the ugoira frame zip permanently failed the whole post, while the bot's own upload downloads classify the very same classes as retryable (classify_download_error). A new PixivError::Transient carries those classes into the existing retry policy (and into startup validation's 'stays enabled' branch); Http keeps its arm and everything else stays permanent. The retryable table and the startup-validation loop pin both halves.
stop() flags the shutdown and fires notify_waiters, but a worker parked between its loop-top stop check and its notified() registration — i.e. inside earliest_run_after's DB await — was not registered when the notification fired, so it slept until the next enqueue that never comes; stop() then blocked until main's 30s shutdown timeout force-killed the drain. The sweep had the same window before recover_expired's await and would sit out a full 30s tick. Both loops now enable() the waiter first and re-check the stop flag: either the stop already happened (recheck returns) or the waiter is registered (notify_waiters reaches it) — no gap. The window itself is a scheduling race with no test seam, so this is pinned by reasoning rather than a regression test; the existing stop tests cover the ordinary path.
Three related races in ChatStore. A read that errored (busy/IO) was indistinguishable from an absent row, so the default got cached — and the next update would write that default back over the chat's real settings (forward channel, templates, formats). A cache-miss backfill inserted unconditionally, letting a stale DB snapshot overwrite the value a concurrent set had just written. And sweep eviction removed per-chat locks unconditionally, so a lock pulled out from under an in-flight update let a second writer create a fresh one and enter the critical section concurrently; eviction now keeps any lock with a holder (the same rule rate_limit's prune applies), which still bounds the map because an uncontended leftover is caught by a later sweep. Two regression tests pin the failed-read and contended-lock cases.
docker-compose injects PIXIV_REFRESH_TOKEN='' for a blank .env value and enabled() only checked that the variable exists, so a default deployment read pixiv as configured, sent an empty refresh token to OAuth and failed login validation on every boot (error line + admin DM) — and it disagreed with the empty-as-unset gating the tests already use. Filter empty strings in PIXIV_CLIENT and enabled(), like twitter and bilibili; a unit test pins empty == unset.
POOL_SIZE = 4 was the process-wide cap on concurrent DB operations while 4 queue workers, 8 URL workers, dispatcher handlers and the sweep all share the pool — WAL readers queued behind writers and every hot-path round trip (3-5 per message) contended for four permits. 16 covers every consumer at once; SQLite's single writer serializes writes regardless.
release_keep_alive retained every KEEP_ALIVE entry whose path matched the settling task, deleting the shared TempDir out from under a concurrent duplicate of the same post (a shared fetch pushes one Arc per pipeline): the duplicate's queued retry then dead-lettered on local media that no longer existed. Release now removes exactly one matching entry, which requires settle to run once per task — handle_task settled on Permanent right before the queue invoked dead_letter_notify, which settles the same payload again, so the redundant settle is dropped. A regression test pins one settle to one entry removed.
Webhook mode passed the secret token to the axum listener only when WEBHOOK_SECRET_TOKEN was set, and docker-compose defaults it to empty (an empty string counts as unset) — the default deployment therefore ran its listener on a public port with no check on X-Telegram-Bot-Api-Secret-Token, so anyone could POST forged updates and impersonate admins (/bot_dict, /clear_cache, /test). Startup now fails when webhook mode has no secret; .env.example and AGENTS.md spell out the requirement.
Inline results were URL-only: Telegram fetches an inline result's URL itself
and cannot send site headers, so every pixiv item (and every locally encoded
ugoira/bsky MP4) was skipped and such a query answered empty. A post that is
already in the link cache now answers with InlineQueryResultCached* built
from its Telegram file ids — no fetch, no upload, and the hotlink-protected
case simply works. A degraded entry (file ids gone) falls back to URLs, and
there a video with no poster is skipped (Telegram would try to render the
mp4 as its own thumbnail).
The answer call moves onto MediaSender (answer_inline_query, mirroring the
other user-flow methods), which is what makes the path testable at all: the
two new tests drive the cache answer and the degraded/empty answer through
TestStores + MockSender, which recorded nothing about inline before.
The result builders are shared by both paths now (url_result/cached_result +
inline_kind), so the fetch path's behaviour is unchanged.
A page whose `original` the API left out (the restricted ones) was dropped
whole by the multi-page branch's filter_map — a missing picture in the album
while `large` sat right there — and the single-page branch produced no media
at all when both places it looks for an original were empty. Both now share
one builder that falls back to `large` (the same picture, lower resolution).
The two tests that pinned the old behaviour now pin the fallback.
A post with no media was answered with "No media found or media type is not
supported.", throwing away text the fetch had already parsed, escaped and
built a caption for (the per-site format and the long-post quoting
included). It now goes out as a message through the same caption the media
path would attach — the senders' own quoting is applied here, since there is
no sender to do it. No queue entry: there is no Task shape for text and a
post with nothing to download is cheap to paste again, so a failure is
reported (send::send_text_post) rather than retried.
Proven live: live_a_text_only_link_is_sent_as_text fetches a real text-only
tweet through url_media and asserts one send_message carrying the post link
and no media send.
One MAX_UPLOAD_BYTES (10 MiB) bounded every upload, but that is the *photo*
limit: Telegram's own docs say sendVideo/sendAnimation/sendDocument take up
to 50 MB, and RequestEntityTooLarge is "larger than 50 MB". So a 10-50 MB
video that Telegram refused to fetch by URL was refused a download too, and
a video has no smaller variant — the post was lost. The non-photo cap is now
MAX_MEDIA_UPLOAD_BYTES, and such a body charges the process-wide budget for
the length of the preparation (one 64 MiB unit covers the cap), since
PREP_SLOTS alone no longer bounds their added RAM. A const test pins both
caps against Telegram's numbers.
MediaItemPayload carried one String field with two meanings and a file_id
bool beside it to say which, in all three variants; every reader re-checked
the flag (input_file had a three-arm pattern just to find the file-id case).
MediaRef::Source/FileId says it once, and the readers now match on it —
local_media_paths, item_url, input_file and the download path each shrank to
the one branch they care about.
Fixes a real failure that the flag was hiding: send_animation built its
InputFile with input_file_for(media_url), which read a cached file id as a
local path and answered "local media file missing" — permanent. So the
second request for a single-gif post always failed (the third worked, from
the degraded entry). It now uses the payload's own input_file, and
a_cached_animation_sends_by_file_id fails without that line.
Wire shape: a queued row from before this change no longer parses, and the
queue already handles that shape (handle_task dead-letters it as an invalid
payload, and dead_letter_notify still names the post and drops the stale
cache entry).
site/mod.rs held the registry, the Site trait, Fetched, the caption helpers,
FetchError and, at the end, the whole media-download stack: two HTTP clients
with different timeouts, the guard that refuses a URL inside the host's own
network (start URL and every redirect hop), and the two streaming entry
points. That stack is its own reason to change and moves whole, with its
tests; CLIENT and the download fns stay re-exported at site::… so no adapter
or bot call site moved. SITES becomes pub(crate) for the header rule.
classify_request_error, the marker tables it matches on, Classification and
SendError (with its fallback conversion) are one policy — which failures are
retried, which are permanent, which the reupload fallback owns — and were
interleaved with the payload types and the senders. They move whole into
send/error.rs and are re-exported, so every existing send::… path is
unchanged.
Two thirds of media_sender.rs was cfg(test)-only scaffolding (MockSender and
the fake_api), which made the trait's own surface hard to find. The module
becomes media_sender/mod.rs (trait + the Bot impl, 260 lines) plus
media_sender/test_support.rs (445); the path
crate::media_sender::test_support is unchanged, so no caller moved.
urls.rs carried five reasons to change: the job channel and its worker pool,
the single-flight fetch, URL parsing, the per-URL pipeline, and the startup
repair of queued retries. The two with their own lifecycle move out:
- url_workers.rs: the bounded channel, its supervised pool and
start/stop_url_workers (the pipeline stays in urls.rs, which the workers
call).
- repair.rs: needs_refetch/apply_refresh/refetch/repair_lost_local_media with
their tests, moved whole (the live one keeps its #[ignore]).
Also folds the two byte-identical render_fields -> CachedPost mappings
(urls.rs and repair.rs) into urls::cached_snapshot, and moves the shared
permanent_error test fixture into ctx::test_support.
execute_command reached for CHAT_STORE/LINK_CACHE/CONFIG in 16 places, which
is why the one handler body with no test could not have one: those statics
point at the real $DATA_DIR/task_queue.db, so any test would have written to
the developer's state. It now takes &AppContext (the shape handle_message
uses) and answers through the given sender; bot stays for what the
MediaSender surface does not carry (channel admin lookups, the HTML report).
Three tests cover what that unlocks: the settings/set_format round-trip
against the chat's own store (including the refused placeholder), the admin
gate on both admin-only commands, and /debug answering without sending.
download_media is gone (the bot uses download_media_limited), the per-site
modules no longer export enabled/is_retryable/media_headers for sites that
inherit the trait defaults, and the shared fetch is shared_fetch.
handlers::db_path already creates the DATA_DIR before it opens
$DATA_DIR/task_queue.db, and every other caller (tests) passes a path whose
parent exists, so the guard plus its one-caller rusqlite_error mapper were
duplicated work. The doc now says which caller owns the directory.
parse_media_url had a single caller (input_file_for) and read better as the
one expression it wrapped; photos_first rebound its argument only to gain
mut.
send_animation_inner only reshuffled its arguments into MediaSender's
send_animation; both call sites (URL send and the reupload fallback) now call
the sender themselves, which also drops the InputFile and MediaSender imports.
dptree::deps![""] inserted a &'static str no handler ever asked for;
Dispatcher::builder already starts from an empty DependencyMap, so the call
was the default written out.
Replacing is_group(kind) with teloxide's predicates dropped the parentheses:
&& binds tighter than ||, so the branch read as is_group() OR (is_supergroup()
AND has-link) -- a plain group got the hint for any link, supported or not.
The e2e test only covered a group with a supported link (which is true either
way); it now also covers an unsupported link in a group.
/bot_dict and /clear_cache each resolved the sender id and replied "Admin
only." on their own; the helper returns the id or answers the refusal, so
both arms read as one line.
/bot_dict's dump and /debug's report each cut a string at a byte boundary
and appended an ellipsis, with slightly different bookkeeping (one left a
byte for the ellipsis, one did not). cap_text is the version that keeps the
result within the cap, with a test covering the exact-fit, truncating and
multi-byte cases.
bilibili and misskey each had a byte-identical caption() (same escaping,
same empty-text early return, differing only in a named format argument).
site::caption holds it once; both adapters call it.
Four call sites (cache hit, fresh fetch, startup refetch, /debug preview)
spelled out the same message_format.get(...).cloned().unwrap_or_default();
the accessor names the lookup and keeps the empty-format contract in one
place.
is_group hand-rolled what Chat::is_group/is_supergroup/is_private already
answer (verified in teloxide-core's chat.rs), and the private check was a
ChatKind pattern match. The unit test that exercised the helper asserted
teloxide's semantics; the channel case it guarded (a channel must not get the
group hint) is now asserted through handle_message instead, next to the group
case.
Ten call sites across link_cache, state and queue repeated the same
match/if-let over a with_conn result with their own log line and default.
with_conn_or takes the level, the operation name and the default; each site
keeps its exact message and the same Ok/Err behaviour.
The one-photo MediaItemPayload literal was written out at six sites across
the urls and send test modules; ctx::test_support::photo_item holds it once.
permanent_error now delegates to api_error (it stays a fn pointer because
that is what MockSender::scripted takes), misskey's x_media_site_id wrapper
is gone in favour of the function it renamed, and PixivAPI is no longer
re-exported -- nothing outside pixiv/api.rs names it.
TypeModel's Illust and Manga variants existed only so serde would accept
those values -- both call sites ask a single question (is this ugoira?), so
the field is a String and the check is a comparison. Same for a type the API
adds later: it no longer fails the whole parse.
It existed only to pass u64::MAX to download_media_limited and was called
from this file's tests alone; the bot's own download path already calls
download_media_limited directly. The tests now exercise that function, with
the same uncapped bound.
site_name() only returned the public site_id field; the five callers (the
/debug report, the caption-format lookup on both fetch paths, the per-site
override in urls.rs) read the field now.
Every adapter set title (mostly None) on all three Media variants and
nothing ever read it: the bot's CachedMedia carries kind/file_id/url, and the
one read was misskey's own test. Removing it also drops misskey's
DriveFile.name, which existed only to feed it. Fetched::title (the post's
own headline, which captions do use) is untouched.
twitter, bsky, misskey and bilibili each carried enabled() -> true,
media_headers(url) -> None and is_retryable(err) -> the trait's own default,
with no caller outside their tests (the adapters never override those
methods, so the default was already the production policy). The tests that
only restated the default are gone; the two that pin site-specific classes
(bsky's MediaPrep, bilibili's risk-control codes) now ask the Site impl, and
site/mod.rs keeps one assertion of the shared retry policy. The live
verification notes (no Referer needed for hdslb/twimg) survive as comments.
twitter, twitter auth and bsky carried the same 404/410 -> NotFound,
401/403 -> Blocked, else Transient block (comments included).
site::status_error holds it once. bilibili and misskey keep their own
matches: neither maps 404/410 and each has a status the others do not
(412 risk control, 400 + NO_SUCH_NOTE), so routing them through the shared
block would have reclassified those statuses for the user.
caption_from_fields already returns truncate_caption(built_in) for an empty
format, which is exactly what the if-branch did; passing cached.caption as
built_in makes both paths one call. The format path is unmoved -- built_in is
read only when the format is empty.
The three webhook settings repeated the same env -> parse -> warn shape
(warn text unchanged); BOT_ADMIN partitioned and then re-parsed every entry,
building a throwaway vector of the bad ones. The ids an operator gets are
unchanged, pinned by a new test.