feat(channel): complete bidirectional file support#386
Open
xiami762 wants to merge 4 commits into
Open
Conversation
…ingtalk, telegram Apply PR #190 (wecom inbound + outbound file attachments) and extend the same pattern to dingtalk and telegram. Refactor the inbound dispatcher into a per-channel hook registry so new channels no longer need to modify dispatcher.py. Inbound (download to local FilePart): - wecom: AES-256-CBC decrypt via wecom_aibot_sdk, 30MB cap, nested mixed-message aeskey, Content-Disposition filename extraction - dingtalk: exchange download_code via OAPI /v1.0/robot/messageFiles/download, 20MB cap, separate exchange/download error classification - telegram: resolve telegram://<kind>/<file_id> via getFile + https://api.telegram.org/file/bot<token>/<file_path> download Outbound (send_media per channel): - wecom: SDK upload_media → send_media_message / reply_media; companion text sent as a follow-up markdown message - dingtalk: OAPI multipart upload → msgKey=file (downloadCode+fileName) for any type; msgKey=image (photoURL) for remote image URLs - telegram: route to sendPhoto / sendDocument / sendVideo / sendAudio / sendVoice / sendAnimation based on inferred kind; agent can force document via telegram:document:<url> prefix Dispatcher: - register_inbound_media_downloader() + _DOWNLOADERS table - dynamic per-channel lookup so test monkeypatches on the channel's inbound_media module still apply - SSE message.part.updated events for both FilePart and the rewritten text part; placeholder text replaced with 'Attached files: <path>' Tests: - wecom: +14 (send_media, inbound_media, content-disposition, mixed file) - dingtalk: +9 (download_code exchange, oversized guard, send_media routing, text-after-file, image URL inline) - telegram: +15 (file_id resolution, kind inference for image/pdf/gif/ogg, endpoint routing, kind override prefix, error path) - dispatcher: +9 (per-channel routing, placeholder detection, end-to-end per-channel pipeline) - test_e2e_file_roundtrip.py: 7 new tests covering real PNG byte round-trips for all five channels with in-process fake servers All 354 channel tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Documents the bidirectional file/image contract for all built-in channels, the dispatcher refactor, the per-channel downloader hook pattern, and the channel-specific outbound quirks reviewers need to know before approving changes to the media path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…escription) The channel file/image review notes are better kept in the PR description itself (where reviewers see them first) than in a root-level doc that the gitignore policy excludes from the docs/ folder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ge points, impact scope, and review focus Rewrite the "Pull Request Guidelines" section in CONTRIBUTING.md so the required PR description structure is explicit: - Key Changes (改动点) — concrete deltas grouped by area. - Impact Scope (影响范围) — user-visible behavior, compatibility, configuration, dependencies, performance, security. - Business Logic to Focus On During Review (需重点 Review 的业务逻辑) — the parts of the change that deserve extra reviewer attention. The previous section listed five reviewer-facing questions but did not constrain the order or depth of the description, which led to PRs that mentioned the impact but omitted the logic that needed a careful read. Also update the PR description template to match the new structure and add a Why-This-Approach section plus an explicit Compatibility, Migration & Rollback section. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FilePartand send outbound media (image / file / video / audio / animation) with correct kind routing. The inbound dispatcher is refactored into a per-channel hook registry so future channels can plug in without touchingdispatcher.py.Key Changes
Per-channel inbound (download →
FilePart)flocks/channel/builtin/wecom/inbound_media.py)wecom_aibot_sdk, 30 MB cap.aeskeyresolution.Content-Disposition.flocks/channel/builtin/dingtalk/inbound_media.py)download_codeexchange via OAPI/v1.0/robot/messageFiles/download, 20 MB cap.flocks/channel/builtin/telegram/inbound_media.py)telegram://<kind>/<file_id>viagetFile+ download fromhttps://api.telegram.org/file/bot<token>/<file_path>.Per-channel outbound (
send_media)upload_media→send_media_message/reply_media; companion text sent as a follow-up markdown message.msgKey=file(downloadCode + fileName) for any type,msgKey=image(photoURL) for remote imageURLs.
sendPhoto/sendDocument/sendVideo/sendAudio/sendVoice/sendAnimationby inferred kind; agentcan force document via
telegram:document:<url>prefix.Dispatcher refactor (
flocks/channel/inbound/dispatcher.py)register_inbound_media_downloader(channel_id, downloader)+ module-level_DOWNLOADERStable.inbound_mediamodule still apply.message.part.updatedevents emitted for both theFilePartand the rewritten text part; placeholder text ([图片消息]/[文件消息]/[文件消息: x]) replaced withAttached files: <path>.Brought along on the branch (precursor fixes required for the round-trip)
#369— assign session ownership and restrict ownerless access for channel-originated sessions.#356—/clearslash command in channel sessions.#350— clear history + surface Feishu websocket disconnects; per-account restart with backoff.#344— archive previous session on IM/new.28af9f9a—SessionCompaction.processreturn type nowLiteral["continue", "stop", "skipped"]; channel/compactreports the threestates distinctly.
Impact Scope
files. Inbound media is surfaced to the agent as a
FilePart; outbound media is rendered natively in each channel. Placeholder text isreplaced with
Attached files: <path>.feishualready registered a downloader; behavior is unchanged.#369: ownerless sessions are no longer shared across all users; only admins canmanage legacy ownerless ones. No data migration — existing sessions are covered by the new code path.
SessionCompaction.processreturn type widened to include"skipped"— callers that didresult == "continue"only are unaffected;callers that compared against the previous union are updated.
api.telegram.orgendpoints — requires network egress to Telegram from the server (same as before;the bot token path was already in use).
wecom_aibot_sdkwas already a dep (used by feat(wecom): support inbound and outbound file attachments #190).Content-Disposition(wecom) orfile_name(dingtalk) — passed through unchanged into the localFilePartpath. Filename sanitization for downstream consumers (TUI / WebUI) is unchanged from existing policy.
telegram:document:<url>prefix is a forced-route hint parsed at the channel layer; URLs are passed to the standardsendDocumentcall without additional validation.#369tightens who can manage channel-originated sessions; previously any authenticated local user could.Business Logic to Review
The following logic carries the most risk and warrants close review:
Dispatcher hook registry (
flocks/channel/inbound/dispatcher.py)register_inbound_media_downloaderis called at import time by each channel module. Verify the registration order is idempotent andthat monkeypatched downloaders (used in tests) are picked up by the dynamic lookup.
_is_placeholder_textmust match exactly the three placeholder forms emitted by upstream channel parsers. A drift here silentlyleaks placeholders to the user.
MessagePartthat gets streamed, not a duplicate.
wecom inbound (
wecom/inbound_media.py)aeskeyresolution: verify the nested path matches the upstream wecom mixed-message spec; the existing test(
test_mixed_file_aeskey) covers the happy path only.confidentiality boundary — a single offset error produces garbage that still passes the magic-byte check downstream.
dingtalk inbound (
dingtalk/inbound_media.py)download_codeexchange + actual download. Errors are classified separately so retry can target the right step. Verify theclassifier does not double-classify a 5xx from the download endpoint as an exchange error.
Content-Lengthpre-check) so we don't OOM ona multi-GB response with a missing/lying
Content-Length.telegram inbound (
telegram/inbound_media.py)getFilereturns afile_paththat is then fetched from a different host (api.telegram.org/file/bot<token>/...). Confirm thebot token is interpolated only into the URL and not logged.
sendDocumentrather than guessingsendPhoto.Outbound kind routing (telegram)
telegram:document:<url>prefix is parsed before the regular kind inference. Verify the prefix is not a valid URL that an agentcould be tricked into typing, and that it is consumed (not re-emitted) when calling
sendDocument.Session ownership change (
#369)— the prior behavior was "any authenticated user can do anything," which is the change.
resolver does not pick a random user.
SessionCompaction.process"skipped" state (PR feat/context compaction v2 #321 follow-up)processreturned a 2-state union must now handle"skipped". The channel/compacthandler isupdated; audit other call sites (
run_compactionis updated, but please grep forcompaction.process(to confirm).Why This Approach
dispatcher.pystopsgrowing.
telegram.
Attached files: <path>as the placeholder text keeps downstream prompts/UI stable — aFilePartis still emitted as a separatepart, the text is just user-facing fluff.
wecom_aibot_sdkis already in the lockfile; dingtalk and telegram useaiohttpalready pulled in by the channellayer.
Test Plan
uv run pytest tests/channel/ -q— all 354 channel tests pass.tests/channel/test_e2e_file_roundtrip.py— 7 in-process fake-server tests covering real PNG byte round-trips for all fivechannels (feishu + wecom + dingtalk + telegram).
uv run ruff check .— clean.CI.)