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).
YoursFunny