A one-line saturating_sub wrapper exported for one production line (the prompt text) and two assertions — the expression is shorter than its name at every use.
All three callers passed item.thumbnail_url() as the fourth argument — a parameter that could never vary without defeating its own purpose, and only the video arm ever reads it. The function takes the item already; it asks the item. Three call sites lose an argument.
photo_media, video_media and animation_media each wrapped the same five lines (build the kind, attach the caption, attach the spoiler) for exactly one caller — media_from's own match arms. The arms carry those lines now; three pub(super) functions and their call indirection are gone, and the dispatch the doc already described as 'the one place' actually is the one place.
Tweet::caption duplicated site::caption byte for byte for non-empty text (and dropped the shared empty-text rule, leaving a dangling ': ' behind the author link when a tweet's text comes out empty after link expansion). It delegates now, like bsky's Post::caption a commit earlier and bilibili/misskey before them; encode_double_quoted_attribute loses its last twitter user.
Post::caption hand-formatted exactly the string site::caption produces — same template, same three escapes (the shared fn even handles the empty-text case these two lines did not: no dangling ': ' after the author link). One line now, matching what bilibili and misskey already do; the encode_double_quoted_attribute import loses its last bsky user.
InlineKind mirrored CachedMediaKind variant for variant (Photo/Video/Gif) and existed only to feed url_result, with two conversions kept in step: Media → InlineKind at the fetch call and CachedMediaKind → InlineKind at the cache call (plus the enum and the mapper). url_result takes CachedMediaKind now — the type the cache path already carries and the fetch path maps to with the same three-arm match — and the degraded-video skip collapses to a guard before the call. One enum fewer between a media kind and its Telegram result.
periodic_sweep listed sender, chat_store, link_cache, task_queue and config as separate parameters — exactly the five fields AppContext already carries, and exactly what handle_message and the workers pass around as one value. It takes &AppContext now (production via from_statics(&bot), the test via stores.ctx(&sender), which that test already had); CHAT_STORE and LINK_CACHE lose their last direct use in main and leave the import. The collaborator bundle is unchanged, so the paused-clock test drives the same loop.
ffmpeg_available() and log_once_ffmpeg_missing() only ever appeared together (both encoders, three lines each: check, log, return), and calling one without the other was a bug in waiting — a probe that never logged, or a log that never gated. ffmpeg_missing() is the single gate: false when the binary is there, otherwise log once per process and say yes. Both call sites shrink to one condition.
find_site walked the registry with "enabled && matches" and disabled_site walked it again with "!enabled && matches" — two passes re-running the same regexes to answer one question. matching_site finds the first match and the fetch decides what a disabled site means; the patterns are disjoint (one domain each), so "first enabled match" and "first match, then check" never disagree.
site_id_from_key split the prefix out and then walked the registry to confirm the prefix was a registered site id — a round trip over a value cache_key itself produced from that registry: the unknown and no-colon branches were unreachable for any key the bot makes. The single caller (the link-cache hit path) splits the prefix directly; the registry-echo test and the misskey assertion of it go with it.
photo_plan wrapped the header dispatch for exactly one consumer (decode_budget_bytes); plan_photo itself has three callers and stays. The dispatch is inlined into the budget function with its doc merged, saving the wrapper's signature, doc duplication and call indirection.
parse_png_header hand-decoded the IHDR — byte offsets, the depth byte's five legal values, the color byte's five — roughly thirty lines the png crate already implements (and validates properly: CRC included). It is now Decoder::new + read_info, which is all the header a plan needs; no pixels are decoded. The synthetic test fixture gained the IHDR CRC and an IDAT header (read_info stops at the first IDAT; the hand parser stopped four bytes earlier and checked no CRC), with a nine-line reflected CRC-32 alongside it; the assertions on width/height/depth/color are unchanged, as are the real-file cases.
Two template landmines for anyone running docker compose from the tracked files. (1) LOCAL_USER_ID defaulted to 1000 in compose and .env.example while the entrypoint and both READMEs say 9001 — the ./data owner on the host silently depended on which doc you read; all four now say 9001. (2) The healthcheck probed WEBHOOK_PORT unconditionally, so the template's own WEBHOOK=false (polling, no listener) shipped a permanently unhealthy container — the probe is now conditional on the interpolated WEBHOOK value (test '<v>' != true || exec 3<>/dev/tcp/…): polling deployments answer healthy without a port, webhook deployments still surface a dead listener to the orchestrator. Both expressions verified locally; compose YAML parses.
Config::load had exactly one test (BOT_ADMIN parsing) while carrying the branches an operator is most likely to mistype: the WEBHOOK truth table (case-insensitive true|yes|1 — 'on' must not enable it), an unparseable EDIT_MESSAGE_TTL_SECONDS (warn + default, not a silent 0 that expires prompts instantly), and empty WEBHOOK_CERT/WEBHOOK_SECRET_TOKEN (compose injects ${VAR:-} as an empty string, which must read as unset, not as a one-character secret). Three table-style tests reuse the existing env save/restore pattern; none of them asserts a field another test reads, so they are safe under parallel execution.
The pull_request paths filter listed only Dockerfile/entrypoint/.dockerignore/Cargo.{toml,lock}/workflow/crate manifests, so a PR changing only Rust sources never triggered the build-only check — and the Dockerfile's stub-plus-touch layering is exactly the thing that breaks against source structure it has never seen (a new crate directory, a build script). Such PRs now get the check before master/tag reveals the problem; crates/**/Cargo.toml is subsumed by crates/**.
Three holes in the live job. (1) It ran -p x-media only, on the comment that everything network-gated lives there — false: xmedia-bot carries two #[ignore]d live tests (repair refetch, text-only link) that CI never executed; both crates run now, still behind the 'live' name filter, and the bot crate's tests need no token (their sends go through MockSender). (2) BILIBILI_COOKIE was never passed, so the bilibili live tests always hit the risk-control early return. (3) That early return — like the pixiv download test's token gate — printed an indistinguishable 'skipping:' line that --show-output never showed, so with continue-on-error a fully-skipped weekly run read exactly like full coverage; skips now print a 'SKIP ' prefix, the steps tee with --show-output, and a summary step lists every skip in the run summary plus a workflow warning. The redundant 'Run token-gated tests' step (the whole non-ignored suite, a rerun of the test job) goes away with the comment that justified it.
The pixiv download test was token-gated but not #[ignore]d, so a shell with PIXIV_REFRESH_TOKEN exported made every local cargo test --workspace hit i.pximg.net — where it demonstrably flaked (twice in one session: TLS EOF mid-body) — and CI's non-ignored step ran it as a side effect of having a secret. It carries the convention's #[ignore = "live network: …"] now and is renamed with the live_ prefix so the live job's --ignored live name filter actually picks it up (the old name did not contain 'live' and would have been filtered out). Its skip prints the SKIP prefix the next commit's live job greps, and AGENTS.md stops claiming a local workspace run is not fully offline.
The default build fetched /redirect/latest/ — a floating URL that changes under every build — and skipped the sha256 check whenever FFMPEG_SHA256 was empty, which it always was: neither the Dockerfile nor docker.yml carried a hash, so any binary the CDN served reached the image unverified and then processed untrusted media bytes. Both now pin the 9.0.2 release build with the hash the mirror publishes beside it (verified here by direct digest of the downloaded zip: fa8ecf4a…909d7f matches the sidecar), docker.yml's build-args fall back to the same pair, and the check is unconditional — an FFMPEG_URL override without its matching hash fails the build at the download step. The ffmpeg layer also moves above the dependency layer so a manifest/lock edit no longer re-downloads it (the old comment claimed caching 'unless FFMPEG_URL changes', which was never true), and the stale step reference in the sources comment goes with it. Local note: Git-Bash's sha256sum -c reads files in text mode here and cannot roundtrip anything binary — the -c line itself is standard and is exercised by the PR build in CI.
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.
decode_budget_bytes mirrored the within-limits early return and the
MAX_DECODE_BYTES guard that prepare_png and prepare_jpeg each spelled out
inline (three copies of the same arithmetic, which the reservation and the
branches had to keep in sync by hand). PhotoPlan/plan_photo now hold that
decision once and all three call it; the log order and the exact bounds are
unchanged. decode_budget_follows_the_processing_decision still pins the
reservation against the branches.
pending_backlog, earliest_run_after, ChatStore::get and LinkCache::get each
hand-rolled prepare + query + rows.next() for what is a single-row read.
query_row + OptionalExtension::optional is the same statement and the same
error mapping with less scaffolding; the backlog's NULL-on-empty MIN still
goes through the count check, so a pending row with a NULL run_after is not
misread. Also fixes the rustfmt drift from the previous commit.
media_from_file, media_from_url and build_media_group's closure each wrote
the same per-kind match plus the same video-thumbnail attach. media_from
takes the already-selected InputFile; the two builders that differ only in
how that file is chosen are now two-line calls to it. The local-file branch
keeps InputFile::file (no existence probe) and every caller still passes the
item's own has_spoiler / thumbnail_url, so what reaches Telegram is the same.
build_send_task and apply_refresh each wrote the same animation-vs-sequence
branch and the same two 13-field literals; Task::from_items takes the
delivery envelope (chat, reply, forward/edit settings, notify targets) once.
The two callers keep computing that envelope from their own source -- chat
settings vs. the queued row being replaced -- so the repair path's delivery
semantics are unchanged.
Every push and PR paid the full four-minute job — fmt, clippy, the offline
suite, a release-profile build and the dependency audit — even when the diff
was a README or AGENTS edit, and every master push built and published an
image for a commit that cannot have changed it.
`ci.yml` gains a `changes` gate job: a push or PR whose *entire* diff is
markdown skips `test`, which then reports as *skipped* instead of missing —
the reason this is a gate job and not a workflow-level `paths` filter, which
leaves a required status check waiting for a check run that will never appear.
Anything non-markdown (and an empty diff, e.g. a re-run of the same commit)
runs the full job, so a new directory of code cannot slip through a stale
allowlist. `schedule`/`workflow_dispatch`, which have no `before` commit, also
run it.
`docker.yml`'s `should-build` gate now also skips a branch push that touched
none of the image's inputs (`Dockerfile`, `docker-entrypoint.sh`,
`.dockerignore`, the manifests, `Cargo.lock`, this workflow, anything under
`crates/`); tag pushes always build.
Checked against this repo's real ranges: the docs-only commit 667f523
(AGENTS.md) → `code=false` (test skipped) and skip, a workflow commit →
`code=true` and build, the h2 bump (Cargo.lock) → build.
The previous commit blamed `actions/checkout` for not fetching tags. It does
(with `fetch-depth: 0`), and the run logs show it: the v1.9.1 master run's
checkout fetched every tag up to v1.9.0 and nothing newer, because v1.9.1 did
not exist on the remote yet — the branch push came first, the tag push eight
seconds later. The duplicate is a race with the tag push, not a missing
fetch. Comments corrected; the re-fetch before the decision stays, and it is
what makes the gap between the checkout and the decision irrelevant (the tag
only has to exist by the time *this* step runs).
Pushing master and a release tag fires two workflow runs, and `should-build`
exists to keep only one of them building: a branch run skips when its commit
is already tagged. It never worked — `actions/checkout` does not fetch tags
(`fetch-tags` defaults to false, and `fetch-depth` does not imply it), so
`git tag --points-at "$GITHUB_SHA"` came up empty and the master run built the
same commit the tag run was building: two ~6 minute docker builds pushing the
same image, for v1.9.0 and again for v1.9.1.
The check step now fetches the tags itself, immediately before deciding, so
the view is as fresh as it can be. Reproduced and fixed against this repo: a
clone made the way the action makes it (`--no-tags`) reports "NO TAG ->
build=true (duplicate build!)" for the tagged v1.9.1 commit, and the same
clone after the step's `git fetch --tags --force origin` reports
"tag(s): v1.9.1 -> build=false (skip)".
A tag pushed *after* the branch run started cannot be anticipated, so the
release flow is documented as one push (`git push origin master vX.Y.Z`) in
both the workflow and AGENTS.md; pushing master first is exactly what made
today's pair build twice.
`cargo fmt --check`, `clippy`, the test suite and the workflow's YAML parse
are all clean (workflow/docs only, no Rust changes).
Enabling reqwest's `http2` feature pulled in h2 0.4.15, which accepts
unbounded empty DATA frames — a remote peer could make the bot queue them
without limit (memory growth, or a panic on length overflow). Low severity,
but the CI dependency-audit gate fails on it, and the fix is a patch bump:
`cargo update -p h2` → 0.4.19.
`cargo audit` against the advisory database is now clean of vulnerabilities
(the two remaining entries are pre-existing `unmaintained` warnings for
`dotenv` and `proc-macro-error2`, which the gate allows), and the full suite,
the live suite and a release build pass on the new lock.
Degrading a link-cache entry (previous commit) closed the "user retries right
after a failure" case, but left a new one open: the entry could never regain
file ids. `cache_sent_task` skipped *every* cached send — its rule was "the
entry already holds the ids the next repeat wants" — which is true for a
healthy entry and false for a degraded one. So a degraded entry (pixiv's
hotlink-protected media, say) kept sending by URL forever, and every repeat
paid a download and an upload through the fallback that the file ids would
have avoided. Worse than the re-fetch it replaced.
The rule is now the one it always meant: a send whose cache snapshot still
carries file ids leaves the entry alone, and a send served from a degraded
entry writes back the ids it produced (the URLs in the rewritten entry come
from that send's own items, so they stay correct).
Verified by a test that drives `cache_sent_task` directly with both task
shapes: the degraded one updates the entry to the fresh id, the healthy one
leaves it untouched. 125 bot tests + 91 x-media tests, live suite (14) pass.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
`link_cache` exists so a repeat link costs nothing: no source request, no
download, no upload. It was written only on a *successful* send, and a send
that failed permanently deleted the entry — so the user's immediate retry, the
one case where they are most likely to try again, re-fetched everything:
site requests, a download, and for a ugoira or a bsky video a full ffmpeg
encode. Invalidation is right about the cause (the cached Telegram file id is
what went stale) and wrong about the cure (the media and its URLs are usually
fine).
Cached media now carries the source URL it was sent from, and a permanent
failure *degrades* the entry: the file ids are cleared, the URLs and the
caption fields stay, and the next request sends from those URLs — Telegram
fetches the media (or the upload fallback does) with no source round trip.
That is the same media a fresh fetch would have produced (site CDN URLs are
stable per post), and it is bounded: an entry that is already degraded, or one
from before this field existed, is removed instead, so a dead post still ends
up re-fetched and reported rather than retried forever.
Verified: a cached send that fails permanently leaves the entry with its URL
and no file id, a second failure drops it, and a degraded entry sends the
media with no fetch at all (the mock records no reply, which is what the
fetch-error path would have produced). 124 bot tests + 91 x-media tests pass,
including a direct test of the two payload shapes.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
`PREP_SLOTS` caps how many items are prepared at once (6) but says nothing
about what they hold: one photo's decode buffer can be up to
`MAX_DECODE_BYTES` (512 MiB) and that guard is *per photo*, so six of them —
an album of large scans, two chats at once — could peak near 3 GiB on a host
sized for a fraction of it. The upload fallback is the only path that
allocates like this; nothing downstream notices until the kernel does.
Photo preparation now charges a process-wide memory budget
(`MEMORY_UNITS` × 64 MiB = 512 MiB) for what it actually holds: the
downloaded bytes plus the decode buffer the *header* predicts — the same
prediction the per-photo guards apply, now shared (`decode_bytes`,
`decode_budget_bytes`) so the reservation and the guard cannot drift. A photo
that is already within Telegram's limits is billed only its download, so an
ordinary 10-image album still runs several at a time; two photos near the
per-photo cap serialize (each takes the whole budget). The request is clamped
to the budget so a single huge photo runs alone instead of waiting for
permits that cannot exist.
Verified with a throwaway harness against a locally served 9999x9999 PNG
(126 KB on the wire, ~100 MB decoded) driven through the real
`prepare_upload_item`: with the budget held the preparation waits — "after
1516ms the prep is still waiting on the budget" — and finishes in 11.8s the
moment it is released, so the accounting binds in the real path and not just
in the semaphore.
Kept as permanent tests instead: the unit math (rounding, clamp, and that a
max-size photo still gets the whole budget rather than waiting forever), the
budget sharing (huge decodes cannot overlap, ordinary ones do not queue), and
the prediction agreeing with the processing decision (over-sized PNG/JPEG
charged, within-limits and unknown formats free).
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean (122 bot + 91 x-media).
Two things the 300 s sweep did the hard way:
- The link cache is pruned by `created_at` (`DELETE FROM link_cache WHERE
created_at < ?`) and had no index on it, so every sweep scanned the whole
table — every post sent inside the TTL window, which is up to a week of
them — while the `url` primary key served none of it. A new migration
(appended; migration 1 is frozen and already shipped) creates the index, and
the upgrade test now asserts it exists after an upgrade.
- `ChatStore::prune_expired` only ever *looked* at chats that had an expired
edit-before-forward record, so a chat with no prompt at all — the common
case: every chat that ever sent a message or ran a command — stayed in the
cache and in the per-chat lock map for the process lifetime. The candidate
set now includes chats holding no records, which is what the eviction below
was written for; the DB keeps the row, so the next use costs one SELECT
(pinned by a new test that also shows the durable settings come back).
Deliberately *not* done: skipping the write in `ChatStore::set` when the state
is unchanged. Comparing against the cached copy would skip a serialize plus a
blocking DB round trip for a no-op update — but every one of the 13 `update`
callers mutates something, so the no-op case is a user repeating an identical
command, and the same comparison would also skip the write that repairs a row
whose earlier write failed. A rare saving against a rare repair, and the write
is what makes the cache a cache rather than a source of truth.
Verified: the new eviction test fails without the candidate change (checked by
reverting it) and passes with it; 118 bot tests and 91 x-media tests pass.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
Every item was skipped — `needs_media_headers` for pixiv's pximg.net, or a
local ugoira/bsky MP4 that does not parse as a URL — and the function fell
through to `Ok(false)`, which the debounce reads as "retry this query". The
client got no answer at all (a spinner with nothing behind it) and every
keystroke re-ran the fetch, while an unsatisfiable link is the *permanent*
truth for that query: Telegram fetches inline result URLs itself and never
sends a Referer.
It now answers the empty result set with a 300 s window, so the client stops
spinning and the same query is served from Telegram's cache and from the
debounce (which only releases on a failure). A *failed* fetch still releases,
so a retry is not answered from a stale empty answer.
Verified with a real pixiv link through a real `Bot` against the stand-in
API: `AnswerInlineQuery` with `results: []` and `cache_time: 300` (and
`Ok(true)`, so no release).
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
`MEDIA_CLIENT` deliberately has no reqwest total timeout (a 30s cap made a
hundreds-of-MB ugoira zip impossible to deliver), and the idle window only
covers *silence*: a server that drips a chunk every 29 s keeps the download
alive indefinitely. On the bot's side each such download holds one of the
process-wide upload-prep slots (`send::upload`'s `PREP_SLOTS`, 6), so a
handful of trickling sources can take the whole fallback path out of service
without ever looking broken.
`DOWNLOAD_TOTAL_TIMEOUT` (600s) bounds the whole transfer, checked between
chunks — a transfer that completes just over the budget is kept rather than
thrown away, and a genuinely slow link (the case the cap was removed for)
stays far inside it. Reported as `Transient` like the idle-window stall: the
transfer may simply have been unlucky, and a retry restarts it.
Verified with a local trickling server (a 1 KiB chunk every 1.2s, chunked so
the client cannot see the total up front): with the budget temporarily
lowered to 2s the download aborted after 2425ms with
`transient: download exceeded 2s` — two chunks in, the server seeing the
client go away — proving the budget and not the 30s idle window ended it.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
`link_cache` is only written *after* a send succeeds, so two chats posting
the same link at the same moment each ran a full fetch: two sets of source
requests, and for an ugoira or a bsky video two ffmpeg encodes of the same
post — minutes of CPU for the second one. The same applies to a batch
forward racing a queued retry. Within one message `dedupe_urls` already
handled the duplicates; across calls nothing did.
`fetch_shared` keys an in-flight fetch by the post's cache key: the first
caller runs it, the rest subscribe and take its result. The entry is removed
by a guard when the fetch settles (cancellation included), so this dedupes
what is *concurrent* and never answers from an old result — a repeat later
fetches again, and a failure is deliberately not cached: the user is told to
try again, and a cached failure would answer that retry from a stale state.
Two hazards that shape the code, each with a test: a broadcast channel lives
while *any* sender does, so the waiter drops its own clone of the sender
before waiting — otherwise a cancelled sharer would leave it waiting forever
— and a waiter whose sharer vanished fetches for itself instead of failing a
link that is perfectly fetchable.
One fetched post can now serve several sends, which the temp files behind
`Fetched::keep_alive` had to support: the field is `Arc<TempDir>` and the
accessor hands out references (the bot's `KEEP_ALIVE` registry holds the
same), so the ugoira/bsky MP4 stays on disk until the *last* task settles
rather than the first. `take_keep_alive` is gone — a `take` could only ever
serve one of the senders.
Verified: 116 bot tests (three new sharing tests, including the cancelled
sharer) and 91 x-media tests (a new one pinning the keep-alive refcount)
pass, plus a live check that two concurrent `fetch_shared` calls for one
tweet return the very same `Arc` after one fetch's worth of wall time.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
x-media's reqwest had exactly `["json", "rustls-tls"]` — no decompression, no
HTTP/2 — so every adapter fetched its JSON as identity over HTTP/1.1. The
site APIs compress: twitter's syndication body measures 4469 bytes identity
against 1066 gzipped (4.2x) for a single-tweet response, and the fetch is on
every twitter link. The CDNs all negotiate h2 (the shared client reports
HTTP/2.0 against cdn.syndication.twimg.com, api.bilibili.com and
public.api.bsky.app), which also multiplexes the concurrent media downloads
that used to open a connection each.
Both features are one crate-level switch, so the pages' compression is what
the APIs answer with and requires no adapter change: reqwest adds
`accept-encoding: gzip` and decompresses transparently. `download_media_limited`
keeps a correct size cap either way — it checks the accumulated *body* while
streaming, not the declared Content-Length, which for a compressed response
is the compressed size.
Verified: a local echo server recorded `accept-encoding: gzip` on a request
from the shared client (both clients come from `build_client`), and a probe
over the shared `CLIENT` reported `HTTP/2.0` for the three site hosts above.
`cargo test -p x-media -- --ignored live` (14 passed) covers the real
endpoints with the new transport.
Cargo.lock gains async-compression (+codecs/core), fnv and h2.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
Two calls per link went out that could not affect anything:
- `run_with_chat_action` awaited the opening `send_chat_action` to completion
before the pipeline was polled at all, and again inside the loop on every
`ACTION_REFRESH`. Telegram round trips are hundreds of ms: the first delay
came out of the user's wait for every link, and each refresh suspended the
fetch (an ugoira encode or HLS remux runs for seconds) by the same amount.
- `handle_message` enqueued *every* URL a private chat posted, including
links no site adapter claims. Those cost a queue slot, a worker wake-up,
a `Message` clone and (through the action above) one Telegram call, only
for `url_media_inner` to conclude there was nothing to send. The group
branch has always made the `cache_key(url).is_some()` test before it acts;
the private branch now makes it before it enqueues.
The in-flight action is held (`Option<BoxFuture>` — the sender surface is
already type-erased, so it is `Unpin`) and polled as its own `select!`
branch: still polled *before* the pipeline, so the indicator is on screen
before the first send, but a slow Telegram response can no longer delay the
pipeline, and none of the branch bodies ever awaits one. One action is in
flight at a time; a refresh while one is unanswered is skipped rather than
dropping the request mid-flight. Note that `select!` evaluates every branch's
future expression eagerly, so the `None` case is an `async` block whose
`unwrap` only runs when the branch is polled (the eager form panicked).
Behavior pinned by the existing tests, unchanged: the opening action precedes
the first send, a 12s pipeline still sees exactly three actions
(`a_long_pipeline_keeps_the_chat_action_alive`), and an unsupported URL
reaching `url_media` still gets the one indicator before the pipeline settles
— in production it no longer reaches `url_media` at all.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
`cookie()` cached the device cookies only on success: the `Option<String>`
it held could not tell "not fetched yet" from "the fetch failed", so a failed
fingerprint meant *every* later post re-asked the SPI endpoint — one extra
round trip per post, forever, exactly the case the fingerprint exists for (a
flagged IP, where that endpoint is the thing answering `-352`/412).
The cache is now `Option<Option<String>>`: the outer level is "an attempt has
been made", the inner one is the cookie it produced, so a failure is
remembered as no-cookie and reaches the request path unchanged. The guard is
also dropped before the request instead of being held across it, which had
serialized every concurrent bilibili fetch behind that one round trip.
A racing first pair still costs a duplicate fingerprint call
(`get_or_insert`, first writer wins) — never a wrong cookie.
Verified live: the 5 bilibili tests (`--ignored`, including
`live_fingerprint_yields_device_cookies` and the four dynamic fetches that
send the cookie) pass. The failure path itself has no unit test: `SPI_URL` is
a const, so there is no seam to make the endpoint fail on demand.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
A long bluesky video can be 500 segments, and the remux downloaded them
strictly one at a time: the user waited for every round trip in turn, which
is the dominant cost of the whole remux (the ffmpeg concat itself is local).
Each segment's multi-megabyte body also went to disk through a blocking
`std::fs::write` on an executor thread.
Segments now download and write under a small bound (`SEGMENT_CONCURRENCY`,
4 — a segment can be 20 MiB and the playlist is capped at 256 MiB, so this is
also what bounds the remux's peak memory) and the write goes through
`tokio::fs`. Concurrent downloads complete in completion order, and ffmpeg
concatenates the list in whatever order it holds — an out-of-order list is a
*silently* scrambled video, not an error — so `concat_list` sorts by segment
index and carries its own test.
x-media's own tokio features gain `rt` (JoinSet) and `fs`: the library
already used `spawn_blocking` on the strength of the bot crate's features.
Verified against a local HLS fixture — 12 one-second segments of solid
red/green/blue, each served with a 150 ms delay, reached through a name that
resolves to loopback (the guard refuses a literal 127.0.0.1) — with a
temporary in-module test: all 12 sampled frames come back in the right colour
order, and the server recorded a peak of 4 requests in flight, where the
serial version showed 1.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
`fetch` classified the response with `parse_syndication_body` — which parses
the whole body into a `serde_json::Value` — and then threw that value away
and re-parsed the same text into a `SyndicationTweet`. Two full JSON scans
and two allocations of every string in the body (text, entities, media
details) per tweet.
`Tweet::from_syndication_value` takes the value classification already
built and deserializes from that: `from_value` moves the strings out of the
tree instead of allocating copies, so the response text is scanned once.
The auth fallback had the mirror image of the same waste — it built the
syndication shape as a `Value` and then serialized it back to a string for
a parse — and the 8 test call sites lose their `.to_string()` with it.
Verified live: `cargo test -p x-media -- --ignored live` (14 passed),
including the 5 twitter fetches that exercise this path.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
A failed bsky video remux was reported as `FetchError::Transient`, so the
fetch loop retried the *whole* adapter — master playlist, variant playlist
and up to 500 segments again (256 MiB of cap each time), for a failure that
happened near the end of the work the retry was about to redo. The message
had to stay honest (returning `Ok` with no media reads as "this post has no
media"), so it needed a class of its own.
`FetchError::MediaPrep` is that class: post fetched, media could not be
prepared locally, not retryable (the per-site `is_retryable` whitelist
excludes it by construction), with its own user-facing text — the generic
"failed to fetch" would have hidden that the download or encode was what
broke. The retry itself is not lost, it moved: `fetch_hls` retries the
request that actually failed, once, for the classes a retry can change
(transport, 429/5xx).
Checked the sibling sites before widening the change: pixiv already degrades
to no media when the ugoira encode fails (`Ok(None)`), and its frame-zip
download failure is the last step so a re-fetch re-does only that; twitter's
auth leg replays two cheap metadata GETs and a GraphQL 5xx *is* worth
retrying. Neither needed the new class.
Verified with a throwaway proxy harness: a 503 answered twice-in-a-row path
costs 2 requests and succeeds, a 404 costs exactly 1 and fails (no pointless
retry). `site::bsky::interface::tests::media_prep_failure_is_not_retried`
pins the classification.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
Each batch's items were prepared under their own `Semaphore::new(3)`, which
is not a memory bound: 8 URL workers and 4 queue workers can each be inside
a batch, so a burst could have two dozen downloads in flight at once, each
buffering a whole photo before it is processed. Nothing else on the media
path bounds them — the send itself is paced by the rate limiter, but the
download and the decode happen before it is charged.
One process-wide `PREP_SLOTS` (6) replaces the per-batch semaphore, and the
photo download gets its own cap: `MAX_PHOTO_DOWNLOAD_BYTES` (32 MiB) for the
transfer, with `MAX_DECODE_BYTES` (512 MiB) left as the pre-allocation guard
on a single decoded buffer. A photo over the download cap degrades to its
smaller URL exactly as one over the decode budget does
(`FallbackError::MediaTooLarge` → `fallback_url`) — never an error.
Verified with the same throwaway proxy harness: 4 concurrent 10-item batches
against a server that holds every response 150 ms peak at exactly 6
concurrent downloads (the per-batch three allowed 12) with all 40 items
prepared.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
The upload fallback asked `x_media::site::media_size` for every remote item
before downloading it. That call is a real GET (not a HEAD) on the *un-
guarded* `CLIENT` — so every fallback item cost two requests where one would
do, the response body was never consumed (the connection cannot return to
the pool), and for photos the answer was discarded outright
(`too_large && !matches!(item, Photo { .. })` still fired the request). It
bypassed `media_request`'s private-network guard as well, the one choke
point every other egress goes through.
For videos the probe was redundant twice over: `download_media_limited`
reads the declared Content-Length before any body byte and aborts with
`FetchError::TooLarge`, which the call site already turns into the item's
smaller URL (`FallbackError::MediaTooLarge` → `fallback_url`).
`media_size` is deleted (no other caller) and the download's own cap is the
only size gate. The video cap is now exactly `MAX_UPLOAD_BYTES` instead of
`MAX_UPLOAD_BYTES + 1`, so the boundary the probe drew survives byte for
byte: a file of exactly the cap is admitted (`len > max_bytes` is false),
one byte over degrades to the smaller URL.
Verified with a throwaway harness (a local HTTP server reached through
`TELOXIDE_PROXY`, the one LAN egress the guard allows): a small video, a
photo and an oversized video each cost 1 request where the probe made it 2,
and the oversized one still lands on `/fallback.mp4` without fetching it.
`cargo fmt --check`, `cargo clippy --workspace --all-targets --locked -- -D
warnings` and `cargo test --workspace --locked` clean.
The repo runs the ignored network tests with `cargo test -- --ignored
live`, which matches on the test name; the new redirect test had no
`live_` prefix and would have been skipped by it.
Every test went through `MockSender`, so `media_sender`'s `Bot`
implementation — the URL it builds, the multipart it sends, the per-chat
limiter and the bot-wide budget it charges — was never exercised, and neither
was any handler reached from a real update. The two things that made that hard
are gone:
- `media_sender::test_support::fake_api::FakeApi` is a stand-in for
`api.telegram.org`: a `tokio` TCP listener that reads one HTTP/1.1 request
(JSON or multipart), records it and answers the smallest result the method
needs. No new dependency, and `Bot::new(token).set_api_url(api.url())`
points a real `Bot` at it. Note for future tests: teloxide keys methods by
payload type, so the path is `SendMediaGroup`, not `sendMediaGroup`.
- `message_handler` built its own `AppContext::from_statics` internally, so no
test could reach its branches; its body is now `handle_message(ctx, bot,
message)` with `message_handler` as the thin `dptree` entry.
Tests: a media group through the real `Bot` (asserting the multipart fields —
chat, media URL, caption — and that the send charged the chat's limiter), the
forward button through the real callback path (`CopyMessages`,
`DeleteMessage`, `AnswerCallbackQuery` with the prompt's ids and the toast
text), and `handle_message` twice (a prompt reply becoming an
`EditMessageCaption`, and a supported link in a group producing the one
explanatory `SendMessage`).
Also closes the redirect-hop gap left open by the download guard: the live
`a_redirect_into_the_hosts_network_is_refused` follows a public redirector to
`169.254.169.254` and asserts the policy refuses the hop (verified against
httpbin.org here, and by mutation — disabling the hop check fails it).
Docs: AGENTS.md's testing conventions and untested-modules list (the Bot
implementation and the handler branches are covered now; `main.rs`'s
startup/shutdown and its `dptree` tree still are not).
`cargo fmt`, `cargo clippy --workspace --all-targets --locked -- -D
warnings`, `cargo test --workspace --locked` (201 passed, 16 ignored) clean.
The media URLs the bot fetches come from a site's own API response (media
URLs, `fallback_url`, thumbnails), `build_client` left reqwest's default
redirect policy in place (up to 10 hops, any host), and the downloaded bytes
are uploaded to Telegram — so a response pointing at a cloud metadata
endpoint would read it back into a chat.
`media_request` is now the one choke point both download paths go through:
http(s) only, and a host that is no address or name of the host's own network
(`blocked_ip` covers loopback, private, link-local, unspecified, broadcast,
documentation, multicast, IPv6 unique-local/link-local, IPv4-mapped, plus
CGA-NAT and benchmarking ranges; `is_local_name` covers `localhost` and
`*.local`). A refusal is `FetchError::Blocked` — permanent, so the send path
does not retry a URL that would be refused again (a connection error used to
be retryable and burned attempts). The same guard runs on every redirect hop
through a custom redirect policy, keeping reqwest's 10-hop cap.
Deliberate gap, documented at the function: DNS rebinding (a name the site
controls resolving to a private address) needs a `reqwest::dns::Resolve`
wrapper, which would also resolve the operator's own proxy host — and
`TELOXIDE_PROXY` is routinely a LAN address — so it would take down working
deployments to block a much less likely attack.
Verified: three offline tests (the address table, the URL table, and a
refusal that holds with nothing listening at the metadata endpoint), both
mutations confirmed to fail them (guard disabled → the download test fails;
link-local dropped from `blocked_ip` → 169.254.169.254 is accepted), and the
live `download_media_pixiv_original_with_referer` still fetches from
i.pximg.net through the guarded client, so real media downloads are
unaffected. `cargo fmt`, `cargo clippy --workspace --all-targets --locked --
-D warnings`, `cargo test --workspace --locked` (198 passed, 15 ignored)
clean.
`extract_urls` decides whether a pasted link is seen at all — a bug there is
silence for the user, which is the complaint this bot's UX work keeps coming
back to — and it had no test: it takes a teloxide `Message`, which is not
worth building by hand.
Split it into the two decisions that are ours and keep the offset work
(teloxide's `parse_entities` turning entities into slices of the message
text) in the thin `extract_urls` composition:
- `url_of(kind, text)`: a bare `Url` entity is its own text, a `TextLink`
keeps its target (not the words the user sees), everything else carries
none.
- `dedupe_urls`: first occurrence wins, deduped by normalized post id — so
`/status/1`, `/status/1/photo/1` and a text link to the same post are one
entry — and by exact text for URLs no site claims.
Verified by mutation, each confirmed to fail the new tests: dropping the
`TextLink` branch, and deduping by raw text (`left: [.../status/1,
.../status/1/photo/1]`).
`cargo fmt`, `cargo clippy --workspace --all-targets --locked -- -D
warnings`, `cargo test --workspace --locked` clean.
`db.rs`'s two rules — `schema_init` is the version-0 baseline and never gains
a column, `MIGRATIONS` is append-only and never edited — were enforced by
comments only. Both have a silent failure mode across releases, and the worst
one (a baseline edit) makes `open_store` fail with `duplicate column name`,
i.e. a fresh deployment that will not start.
Three tests, no production change:
- A pre-migration database (the historical DDL written out literally, so an
edit to the baseline shows up here instead of being followed) upgrades
through `open_store`: version at the latest, exactly the migrated column
set, rows intact.
- The shipped migration text is frozen and compared entry by entry; the
assertion names the rule when it fires. Appending still passes — that is
the one allowed change.
- A fresh database lands at the latest version, so a deployment that only
ever saw fresh databases is on the same schema as an upgraded one, and
re-opening the same file is a no-op.
Verified by mutation, both confirmed to fail the new tests: editing the
shipped migration (`shipped_migrations_are_frozen`, with the rule in the
message) and adding the column to `schema_init` instead of a migration
(`a_fresh_database_lands_at_the_latest_version`, `duplicate column name:
lease_token`).
Docs synced for this and the previous two items: `main.rs` (startup repair),
`queue.rs` (`runnable_rows`/`replace_payload`), `handlers/` (`urls.rs`'s
repair, `mod.rs`'s `apply_caption_edit`), `db.rs` (the migration tests) and
the untested-modules list (`db.rs` now covered for migrations; the bot-side
live test renamed to match the repo's `--ignored live` filter).
`cargo fmt`, `cargo clippy --workspace --all-targets --locked -- -D
warnings`, `cargo test --workspace --locked` (193 passed, 15 ignored) and
`cargo test -p x-media -- --ignored live` (13) clean.
YoursFunny