M1: Fix attachment delivery path#6210
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
| // Prefer the assistant-less download path; fall back to the legacy | ||
| // assistant-scoped path when assistantId is available. |
There was a problem hiding this comment.
🟡 Comment describes the opposite of what the code does for download path selection
The comment says "Prefer the assistant-less download path; fall back to the legacy assistant-scoped path when assistantId is available" but the code does the exact opposite: it prefers the legacy assistant-scoped path when assistantId is truthy, and only falls back to the assistant-less path when assistantId is undefined.
Detailed Explanation
At gateway/src/telegram/send.ts:64-68, the comment and code contradict each other:
// Prefer the assistant-less download path; fall back to the legacy
// assistant-scoped path when assistantId is available.
const payload = assistantId
? await downloadAttachment(config, assistantId, meta.id) // legacy path — used FIRST when assistantId exists
: await downloadAttachmentById(config, meta.id); // assistant-less path — used only as fallbackThe ternary checks assistantId — when truthy it uses downloadAttachment (the legacy assistant-scoped endpoint), and when falsy it uses downloadAttachmentById (the assistant-less endpoint). The comment claims the opposite priority.
While the runtime behavior is correct for the PR's intent (use legacy when available, assistant-less when not), the misleading comment could cause a future developer to "fix" the code to match the comment, inadvertently breaking the fallback logic.
| // Prefer the assistant-less download path; fall back to the legacy | |
| // assistant-scoped path when assistantId is available. | |
| // Use the legacy assistant-scoped download path when assistantId is | |
| // available; fall back to the assistant-less endpoint otherwise. | |
Was this helpful? React with 👍 or 👎 to provide feedback.
* fix: remove assistantId dependency from Telegram attachment delivery (#6210) Co-authored-by: Claude <noreply@anthropic.com> * feat: add Telegram webhook lifecycle reconciliation (#6211) Co-authored-by: Claude <noreply@anthropic.com> * feat: auto-configure gateway routing for single-assistant mode and add rejection visibility (#6212) Co-authored-by: Claude <noreply@anthropic.com> * feat: add Telegram Bot messaging provider for proactive outbound sends (#6222) Co-authored-by: Claude <noreply@anthropic.com> * feat: harden /deliver/telegram auth and align docs with Telegram capabilities (#6238) Co-authored-by: Claude <noreply@anthropic.com> * fix: correct misleading comment in Telegram attachment download path (#6241) Co-authored-by: Claude <noreply@anthropic.com> * fix: bound rejection notice cache with periodic eviction (#6242) Co-authored-by: Claude <noreply@anthropic.com> * fix: support tokenless providers in withProviderToken and fix testConnection error handling (#6244) Co-authored-by: Claude <noreply@anthropic.com> * fix: always reconcile webhook and normalize ingress URL (#6245) Co-authored-by: Claude <noreply@anthropic.com> * fix: resolve gateway lint error and credential security allowlist for Telegram adapter (#6257) Co-authored-by: Claude <noreply@anthropic.com> * fix: require webhook_secret in Telegram isConnected check (#6259) Co-authored-by: Claude <noreply@anthropic.com> * fix: only default routing policy in single-assistant deployments (#6261) Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Remove the hard dependency on assistantId for Telegram attachment delivery. Attachments are now downloaded via /v1/attachments/:attachmentId endpoint that doesn't require assistantId. This fixes the bug where outbound attachments were silently dropped in channel callback flows. Part of #6200.