`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.
Three gaps the last audit list named, all in the "resource growth, background
timers and limits nobody watches" class.
**Idle inline-query entries are pruned.** `DebounceStates` had no eviction at
all: one entry per user who ever used inline mode, forever, while the rate
limiter's buckets and the chat store both prune in the 300s sweep. Entries
now carry a `last_seen` stamp and `prune_idle_states()` drops the ones idle
past 300s — the window Telegram caches an inline answer for
(`cache_time(300)`), after which a repeat reaches the bot again and has to be
answered fresh, so the entry would only suppress a fetch the user is waiting
for. The boundary is tested through `prune_idle_at(now, idle_for)` so it does
not depend on ageing a monotonic clock.
**The 300s sweep is a function, and tested.** It was an inline `tokio::spawn`
block: the expiry edit (the only part that talks to Telegram) had no test at
all. It is now `periodic_sweep(sender, chat_store, link_cache, task_queue,
config, stop)`, which also prunes the inline entries, driven in a test with
`start_paused` — the loop's own timer fires the tick, exactly one expired
prompt is rewritten in place, a live one keeps its record and buttons. The
interval is pinned as a constant because no assertion on the edits can see it
(a shorter one produces the same single edit; the paused clock can jump past
the boundary while a tick's DB work is in flight). To make the edit reachable
at all, `edit_message_text` joined the `MediaSender` trait (Bot impl + mock
recording), which is also what keeps `main.rs`'s remaining `Bot` calls
unambiguous. `main.rs` leaves the "untested modules" list except for
startup/shutdown and the dispatcher tree.
**The bot-wide send budget exists.** Telegram throttles a bot in total
(~30 msg/s) as well as per chat; only the per-chat bucket existed, so a batch
forward fanned out over many chats was unguarded and earned 429s the queue
then retried. `acquire_global` charges the same spend against a single shared
bucket at the three paced sites (`send_media_group`, `send_animation`,
`copy_messages`). The unpaced ones (`send_message`, the edits, the toasts) stay
unpaced on purpose: they are one call per action, far below the ceiling, and
pacing a user-visible reply would delay it. Not covered: that the send paths
call it (they need a real `Bot`), which is the same structural gap as the
dispatcher tree.
Also: the startup token-exchange decision is now `startup_validation(result)`
instead of living inside the `Site::validate` future, so "a 5xx while the
container comes up must not disable pixiv" is asserted as a decision — the
message the admin gets plus `enabled()` unchanged. The rejected-credential
half is deliberately not exercised: it calls `disable()`, a process-wide flag
with no reset, and a test touching it would order-couple every other pixiv
test.
Verified: `cargo fmt`, `cargo clippy --workspace --all-targets --locked -- -D
warnings`, `cargo test --workspace --locked` (184 passed, 14 ignored) — plus
mutations, each confirmed to fail the relevant test: the sweep not being
driven on its timer, the interval shortened to 60s, and (earlier) the queue
sweep's missing wake-up. Dropped an empty leftover `crates/x-media/tests/`
directory while there (never tracked by git).
Audit of all 205 tests (five read-only passes plus a line-by-line
re-check). Ten test functions were removed or merged and eight
subsumed assertion blocks trimmed; the suite is down to 180 tests with
no loss of mutation coverage, and four tests that were passing for
nothing now fail when the code they name is broken.
Redundant (deleted or merged):
- twitter: `syndication_text_only_has_no_media` (re-asserts its own
empty fixture), `..._keeps_multibyte_text` (both transforms are
no-ops for that text), `..._strips_trailing_short_link_without_entities`
(same branch as `..._media_short_link`, which now also covers the
real multibyte tweet), `..._regardless_of_index_units` (its
`display_text_range` rationale outlived the function it described).
- pixiv: `test_fetch` (a bare `is_ok()` on the illustration
`download_media_pixiv_original_with_referer` already asserts and
downloads, and the only network touch in a plain `cargo test`),
`startup_validation_only_disables_on_a_definitive_failure` (four rows
that are a subset of the retry-policy table; the `validate()` branch
it was named for is not asserted at all).
- bilibili: `from_item_legacy_draw_shape_still_parses` (its fixture is
the same legacy `draw` shape `from_item_maps_draw_images_and_topic`
builds, with a subset of its assertions).
- site/mod.rs: two `Ok(None)` cases merged into one test.
- urls.rs: `cache_hit_success_keeps_the_cache_entry` (the `/test` test
asserts the same two things under stricter settings), plus a
`assert_ne!` loop that re-states the mapping assertions above it.
- send/mod.rs: `media_group_success_and_forward_ok` (the forward half is
covered by `post_send_forwards_immediately_when_configured`; the
`is_ok()` half cannot see the returned file ids), and two boundary
rows implied by the constant they sit next to.
- commands.rs: the parse tail that `every_documented_invocation_parses`
already covers per README form, and three `debug_report` rows the
escaping test pins with stronger input.
Passing for nothing (now real):
- `truncate_caption_does_not_split_an_html_entity` — the cut lands
inside the entity, so `!contains("&")` never fired; it now asserts
the exact output in both directions and fails when the guard in
`truncate_caption` is deleted (verified).
- `pipeline_resizes_oversized_jpeg` — magic bytes and a non-empty buffer
pass for a copy-through; it now decodes the output's headers and
fails when the JPEG branch skips the resize (verified).
- `live_validate_with_bogus_token_fails` — expected `PixivError::Api`,
which the status check before the body read made unreachable; a bogus
token is a 4xx. Confirmed against the live endpoint: the old
assertion fails with `got Err(Status(400))`, the new one passes.
- bsky `live_fetch_with_photos` — its URL is a text-only post and it had
a byte-identical twin, so no live test pinned media; it now points at a
labelled post with photos and asserts media + the label (live-verified).
Also fixed, found by turning the runtime-sweep test into a real one:
the 30 s lease-expiry sweep recovered crashed rows but never woke a
worker, so a recovered task waited for the next unrelated enqueue (every
worker is parked on `notify` when no row is pending). `recover_update`
now reports its count, `recover_expired` wakes a worker when it changed
something, and `runtime_sweep_recovers_expired_lease` drives the spawned
loop with a paused clock instead of calling the recovery by hand — it
fails on both the missing wake-up and a sweep that recovers nothing.
Verified: `cargo fmt --check`, `cargo clippy --workspace --all-targets
--locked -- -D warnings`, `cargo test --workspace --locked` (180 passed,
14 ignored) and `cargo test -p x-media -- --ignored live` (13 passed).
P1 of the retry audit, from the report's "reliability and diagnosis" batch.
- Media downloads no longer share the 30s *total* timeout of metadata
fetches. The size caps allowed 10 MiB (reupload fallback) and 512 MiB
(ugoira frame zip) while the clock allowed 30s, so a slow link made those
posts impossible: `MEDIA_CLIENT` has no total timeout and instead bounds
the response head and every chunk with a 30s *idle* window, which keeps the
stalled-connection protection. Verified against a local probe: the old
policy aborts a 40s download at 30.0s, the new one completes it (2 MiB,
40.1s), and a body that stops delivering still fails after exactly 30s.
- A finished row's write-back is no longer best-effort. `delete_row` failing
left the row `in_progress` with a live lease, so the next sweep flipped it
back to `pending` and re-ran a completed task — a second album, a second
prompt, a second channel copy. Both terminal writes are now retried, and a
delete that still fails falls back to a `done` tombstone that neither the
lease query nor the sweep looks at; reschedule (no safe tombstone: marking
it done would drop the retry silently) logs what the sweep will do.
- bsky and pixiv no longer present a *failed* video conversion as a post with
no media: the remux/ugoira error propagates (pixiv keeps its retry class,
bsky reports Transient), so the user sees the real cause and `fetch` gets
its retries. bsky's "no ffmpeg" case stays a degradation — retrying a
deployment gap cannot help.
- pixiv's token exchange checks the HTTP status before parsing the body, so a
429/5xx from the OAuth endpoint stays retryable instead of becoming a
permanent Api/Json error (via the shared `pixiv_error_is_retryable`), and
startup validation only disables pixiv for a rejected credential — one 503
while the container came up used to turn every later pixiv link into
"pixiv support is disabled".
`Fetched.title` carried whatever text the platform had — a tweet's body,
a bilibili dynamic's body, a pixiv artwork's title — which was enough
while x/twitter (no title at all) set the shape. The platforms actually
disagree: pixiv has a title *and* a description, bilibili has an opus
headline *and* a body. Posts now carry both:
- `title`: the platform's title (a pixiv artwork title, a bilibili opus
headline or video card title), empty on text-only platforms;
- `content`: the body (tweet / bsky / misskey text, bilibili dynamic
body, and pixiv's description — fetched for the first time here and
flattened from the app API's HTML to plain text).
`{content}` joins the caption-format placeholders, so a custom
`/set_format` can include a pixiv description. The built-in captions keep
producing byte-identical output: `compose_text` joins the two fields the
same way the single field already was, and bilibili's forward marker
(`//@author:`) now lands in `content` behind the head line's `title`.
`CachedPost.content` is `#[serde(default)]`, so link-cache entries and
queued task payloads written before the split still parse, their text
living in `title`.
Post URLs and author URLs were interpolated raw into <a href> attributes
(and the raw user URL from empty_fetched into caption text), so crafted
links could break the HTML parse and fail the send with a 400. All
attribute interpolations now use encode_double_quoted_attribute; text
stays encode_text.
Both matched URL substrings anywhere in text, so a link like
https://evil.com/?u=pixiv.net/artworks/1 triggered a real fetch and
cache-key pollution. Prefix ^(?:https?://)? like the twitter pattern.
Apply rustfmt to the 11 files that had drifted (86 hunks): x-media
site modules (bsky/pixiv/twitter) and xmedia-bot (config/main/
photo/send). Formatting only - no semantic changes; full test suite
still green.
YoursFunny