Commit Graph
13 Commits
Author SHA1 Message Date
YoursFunny 4e723e1657 test: drive a real Bot against a stand-in API
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.
2026-09-21 03:02:43 +08:00
YoursFunny 3b946b1eab fix: never eat a caption silently when the edit fails
Both caption-edit paths logged the error and carried on as if they had worked:
`edit_message_handler` consumed the user's reply (`let _ =`), and the
template button updated the prompt record and dismissed its toast with no
text. Since the edit surface is not rate limited, a 429 or a transient
failure meant the caption never changed and the user got no hint — the text
they sent was simply gone.

`apply_caption_edit` (handlers/mod.rs) is now the one place that applies a
caption edit and reports the outcome:

- A failure the API calls worth retrying (`classify_request_error` →
  `Retryable`) is retried once when the delay is at most 2 s — a reply or a
  button press has already been consumed by then, so a long flood-control
  wait must not stall the chat's update queue behind it.
- Otherwise the caller reports it: the reply path answers the user ("Could
  not update the caption (…). Send it again to retry."), the template path
  puts it in the callback toast and leaves the record alone — a swap that
  never happened must not be recorded as the prompt's template.

Tests: the swallowed-failure test now asserts the notice (it pinned the old
silent behaviour), plus a short `RetryAfter` that is retried and lands, a
60 s one that is not retried and is reported instead, and the template
button's failure toast with the record left unchanged.

`cargo fmt`, `cargo clippy --workspace --all-targets --locked -- -D
warnings`, `cargo test --workspace --locked` (190 passed, 15 ignored) clean.
One note: the first full run tripped `download_media_pixiv_original_with_
referer`, the token-gated pixiv CDN download test that AGENTS already
documents as a local-network flake; it passes in isolation and on the rerun.
2026-09-21 02:07:55 +08:00
YoursFunny 3828d5b483 test: share the handler/cache fixtures from ctx::test_support
The same fixtures were rebuilt in five test modules: a `CachedPost`
literal in `link_cache.rs`, `handlers/urls.rs` and twice in
`send/mod.rs`, the edit-before-forward prompt in `handlers/mod.rs` and
`handlers/callback.rs`, and a scripted API error in both handler
modules. They now live in `ctx::test_support` next to `TestStores`:

- `cached_photo()` — the canonical cached post (photo + file id at
  `https://x.com/u/status/1`, key `twitter:1`); tests mutate the fields
  they care about, as the caption-quote test already did.
- `seed_prompt(template, created_at)` + `PROMPT_ID`/`FORWARDED_ID` —
  the prompt record, the chat template and the bound forward channel.
  The two former copies differed only in which knob the caller set (the
  callback tests backdate it for the expiry cases, the reply tests pick
  the template), so the union is one helper.
- `api_error(message)` — construction only; each test module keeps its
  own message constant, because the wording is what that module's path
  answers with (`chat not found` vs `message not found`).

`send/mod.rs`'s `cached_sequence_cache_data()` (which re-extracted the
post out of the task it had just built) is gone: the two settle tests
seed the cache from the same builder the task uses.

No behaviour change: the values are the ones the tests used except
`file_id` (`AgAC-file-id` everywhere, asserted in the link-cache
round-trip) and `sensitive` (the unasserted `true` in the link-cache
fixture), and every test still passes unchanged.

Verified: `cargo fmt`, `cargo clippy --workspace --all-targets --locked
-- -D warnings` and `cargo test --workspace --locked` (180 passed, 14
ignored).
2026-09-21 00:25:01 +08:00
YoursFunny 4cf793cd7e fix(retry): stop losing posts to transient failures and broken promises
P0 of the retry audit. The main finding: a Telegram 5xx was classified
Permanent, so one Telegram-side blip dead-lettered the post.

- `classify_request_error`: a server error is retryable again. teloxide sleeps
  10s on a 5xx and then parses the body, so the HTTP status is gone by the
  time the error arrives; it is recognised by shape instead — a JSON
  server-error description, or an `InvalidJson` whose raw body is not JSON
  (a proxy/error page). A JSON body of the wrong shape stays permanent, since
  retrying a type mismatch cannot help. Reproduced end to end: with the old
  classification a fake 502 (HTML body) logged "failed permanently" and
  dead-lettered; now it logs "queued for retry" and the retry delivers.
- The same class of mistake elsewhere: `is_media_fetch_failure` was missing
  `failed to get HTTP url content`, the description single-media URL sends
  answer with, so hotlink-rejected media failed permanently instead of going
  through the reupload fallback.
- `enqueue_retry` now reports whether the row was written, and the callers
  only promise a retry when it was — a failed enqueue (DB write) used to tell
  the user "retrying in Ns" and then deliver nothing, ever.
- A forward that fails retryably now settles the prompt instead of leaving it
  live: the queued row carries the message ids itself, and a live prompt let
  a second Confirm copy the same messages to the channel twice and let Skip
  answer "nothing was forwarded" while the row still delivered.
- A prompt that could not be sent no longer swallows the gated forward
  silently: the chat is told, since nothing would ever forward.
- `scaled_retry_delay` only scales up, so a server-asked `retry_after` above
  the 300s cap is honoured instead of retried early (which earned another 429
  and then dead-lettered the post).
- Download classification: a 4xx media download is permanent (the media is
  gone or refused) while transport errors and 429/5xx retry — previously every
  download error counted as retryable and burned the whole budget. A temp-file
  *write* failure retries too (resource exhaustion clears; a temp dir that
  cannot be created stays permanent).
- Site status mapping: 401/403 are `Blocked` (permanent) rather than
  `Transient`, so a refusal is reported at once instead of after three
  wasted attempts; and a twitter 200 that is not a tweet is no longer
  reported as withheld content (the empty `{}` withheld shape keeps
  `Sensitive`, which is what triggers the auth fallback).
2026-09-20 20:46:18 +08:00
YoursFunny d6707133cc feat(ux): answer every link, name fetch failures, keep the chat action alive
Four ways a user could get silence are closed: a registered-but-disabled
site (pixiv without a token) now answers instead of being dropped as an
unsupported link, `/test` on such a link replies instead of doing nothing,
a supported link posted in a group gets a one-line hint (channels stay
silent), and fetch failures name their cause — gone / withheld / source
risk control / site disabled / source down — instead of one generic
sentence. `FetchError::Disabled` carries the "matched but switched off"
answer, which `find_site` used to fold into `Ok(None)`.

A withheld tweet no longer degrades to "no media": without
`TWITTER_AUTH_TOKEN` it stays `Sensitive` so the reply says the media is
age-restricted, and a failed authenticated fallback propagates its own
class instead of masquerading as an empty post (`empty_fetched` is gone).

Long jobs stop looking stalled: `run_with_chat_action` re-sends the chat
action every 4s while the pipeline is pending and the hint switches from
typing to send-photo/video once the media kinds are known. Media groups
go from 9 to Telegram's 10.

`/set_format` rejects unknown `{…}` placeholders (a typo used to be
published verbatim in every caption) and resets with `-`. The
edit-before-forward prompt states its TTL and that Confirm is required,
gains a Skip button, and is rewritten in place to "expired" by the sweep
— an edit, never a new message, so a background timer cannot wake a chat.
2026-09-20 15:48:46 +08:00
YoursFunny 0a9ff58a69 refactor: cover the edit/answer surface in MediaSender, test the button flows
docs/architecture-refactor.md §3 sketched the trait with "按需扩展:
edit_message_caption / delete_message / answer_callback_query …", but only the
five send methods landed, so `callback.rs` and the edit-before-forward caption
swap were stuck on the concrete `Bot` and remained untested (AGENTS.md still
lists callback.rs as untestable).

- `MediaSender` gains `answer_callback_query`, `edit_message_caption` (HTML
  parse mode baked in, every caller uses it) and `delete_message`; the mock
  records call order plus the texts, captions and answer toasts, so tests can
  assert what the user saw.
- `send_message` now returns the sent message id instead of the whole
  `Message`: the only consumer of the value is the edit-before-forward prompt
  (which keys its record by it), and returning a `Message` forced every mock
  to build a teloxide type. `reply`/`reply_html` follow.
- `callback.rs`: the dptree entry only unpacks the update; `handle_callback`
  takes plain values + `&AppContext`. `handlers/mod.rs::edit_message_handler`
  likewise takes the values the reply carries. Admin/setup APIs
  (`get_chat`, `get_chat_administrators`, `get_me`, `set_my_commands`) stay on
  the concrete `Bot`: they are not user flows worth a trait.
- The scripted mock moves to `parking_lot::Mutex` (no poisoning unwraps).

Tests: +11 (template button, forward ok/no-channel/retryable, expired+unknown
prompt, caption swap via template, escaping of user text into the caption,
failed swap still consuming the reply, prompt record written by post_send).
fmt/clippy clean, 70 + 69 tests pass.
2026-09-17 01:31:30 +08:00
YoursFunny c2d7c8406e refactor: finish the phase-B seam for the post-send path, funnel settlement
docs/architecture-refactor.md §3 stopped half-done: `url_media` got an injected
`AppContext`, but `send.rs`'s post-send half kept reaching for the process-wide
`CHAT_STORE`/`TASK_QUEUE`/`LINK_CACHE` statics, so the whole shell after a
successful send (edit-before-forward prompt, channel forward, retry enqueue,
cache write) had no test and no way to get one.

- `ctx.rs` now owns `AppContext` (sender + the three stores + config) with
  `from_statics` for production and a `CONTEXT` static for the spawned worker
  closures; `handlers/urls.rs` drops its private copy and the duplicated
  assembler, and the queue handler/dead-letter callbacks take the context
  (main wires them with `CONTEXT`).
- `send_media_sequence`/`send_animation`/`forward_messages`/`post_send_actions`
  take `&AppContext`; the cache write goes through the injected cache.
- New `settle_task(ctx, task, Sent|Failed)` is the single place that ends a
  task: release its keep-alive temp media, and drop the link-cache entry only
  on failure. All five former call sites funnel through it — the earlier
  keep-alive leak existed precisely because one of them had to remember.
  `invalidate_cache`/`invalidate_cache_with` (static + injected pair, the
  latter only existing because of the former) collapse into one private fn.
- `ctx::test_support::TestStores` gives tests a tempdir store set + context;
  `handlers/urls.rs` tests use it instead of hand-rolled setup.

Tests: +5 (post-send forward ok / queued / notified, settle Sent/Failed); the
post-send and settle paths were previously untested. fmt/clippy clean,
60 + 69 tests pass.
2026-09-17 01:27:05 +08:00
YoursFunny abdc27ed5e docs: resync AGENTS.md and the doc comments with the code
AGENTS.md:
- db.rs row claimed a per-store connection pool; there is one shared pool for
  all three tables (statics.rs builds it once).
- retry enqueue moved to send.rs, noted in both handlers rows.
- queue row now names both notifies (workers' + the sweep's).
- Retries bullet documents fetch vs fetch_once.
- test count ~125 -> ~135, the untested-files list no longer claims state.rs
  and handlers.rs are untested, and the live-test inventory mentions the
  token-gated, not-#[ignore]d pixiv download test that makes a local
  `cargo test --workspace` hit the network.
- /bot_dict is admin-only now.

Code docs:
- site/mod.rs: the module doc pointed new sites at `fetch_once` (a name that
  did not exist then and now means a single-attempt fetch) -> `SITES`; the
  cache_key/SITES/Site docs still said "twitter -> bsky -> pixiv" (misskey
  is registered third); RenderData now documents which fields are escaped
  and why url/author_url are not.
- state.rs, callback.rs: drop the pre-misskey site list and the `<name>`
  that rustdoc read as an HTML tag.
- Fixed the remaining rustdoc links/warnings: `cargo doc --workspace
  --no-deps` is now warning-free (was 8).
- docs/site-registry-refactor.md: §1 describes the pre-refactor state; said so.

No behavior change. fmt/clippy clean, 55 + 68 tests pass (the live pixiv
download test flaked on a CDN body timeout, as before).
2026-09-16 23:28:46 +08:00
YoursFunny 0a577600fd fix: repair four correctness defects in the send/state/handler paths
- send: the PHOTO_INVALID_DIMENSIONS marker never matched (the description is
  lower-cased, the marker was not), so oversized photos sent by URL were
  classified Permanent instead of taking the download-and-downscale fallback.
- inline: the debounce state was one global slot, so a second user's query
  cancelled the first user's pending answer entirely; it is now per user.
- state: prune_expired wrote back a stale snapshot without the per-chat lock,
  clobbering a concurrent update() (lost edit-message record -> "Expired");
  it now re-reads and prunes under the same lock update() uses.
- send/queue: a task dead-lettered on retry exhaustion kept its keep-alive
  temp media (ugoira MP4) alive until process exit; dead_letter_notify now
  releases it, and enqueue_retry releases when the enqueue itself fails.

Also folds the duplicated retry enqueue in post_send_actions into
send::enqueue_retry (single clock source, single place that releases).

Tests: +6 (marker, per-user debounce x3, keep-alive release, prune contract
x2); the marker and keep-alive cases were verified to fail before the fix.
cargo fmt/clippy clean, 52 + 69 tests pass.
2026-09-16 21:11:39 +08:00
YoursFunny 2f741e5f4b refactor(send): apply ponytail audit cuts 2, 4, 6
- updated_sequence_task: clone the Task and mutate the two fields
  instead of rebuilding all 12 by hand (-22 lines; new fields no
  longer need a sync here)
- unify unix_now with db::now_f64 (unix_now() = now_f64() as i64),
  moved to db.rs next to its clock source
- classify_to_send_error takes the MediaFetchFailure label, folding
  the duplicated inline match in send_batch_via_upload (-8 lines)
2026-09-07 19:19:56 +08:00
YoursFunny 89c4642e1c fix(lint): resolve clippy warnings from rust 1.98
- photo.rs: chunks_exact(4)/(2) -> as_chunks::<N>().0
  (chunks_exact_to_as_chunks, the new lint prefers the
  compile-time-checked slice split)
- send.rs: box the Task inside SendError so the error fits the
  result_large_err limit (Task is ~400 bytes; the error now moves
  through Result as a pointer); unbox with *task at the two
  enqueue_retry call sites (handlers/urls.rs, handlers/callback.rs)

cargo clippy --workspace --all-targets is now warning-free; the
remaining proc-macro-error2 future-incompat note is upstream
(teloxide -> aquamarine) and unfixable locally. Full test suite passes.
2026-09-07 17:00:38 +08:00
YoursFunny ae69d72930 refactor(handlers): inject AppContext into url_media; cover the full URL pipeline 2026-08-14 23:38:41 +08:00
YoursFunny c9e72fda70 refactor(handlers): split monolithic handlers.rs into modules 2026-08-14 21:50:40 +08:00