feat: embed videos inline in Slack responses#389
Conversation
When the content agent finishes generating a video, download it and
post it as a file upload so it appears inline in the Slack thread.
Falls back to the old URL-link behavior if the download fails.
- New downloadVideoBuffer utility to fetch video data
- Updated handleContentAgentCallback to use thread.post({ files })
- 14 new tests (4 downloadVideoBuffer + 10 callback handler)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds utilities to download video content and derive filenames, introduces Changes
Sequence DiagramsequenceDiagram
participant Agent as Content Agent Callback
participant Poster as postVideoResults()
participant Downloader as downloadVideoBuffer()
participant URL as External URL
participant Thread as Thread/Post API
Agent->>Poster: postVideoResults(thread, videos, failedCount)
Note over Poster,Downloader: Poster starts concurrent downloads
Poster->>Downloader: downloadVideoBuffer(url) [concurrent]
Downloader->>URL: fetch(url)
URL-->>Downloader: HTTP response
alt response.ok & buffer created
Downloader-->>Poster: Buffer
Poster->>Poster: getFilenameFromUrl(url)
Poster->>Thread: upload attachment (buffer, filename, mimeType: video/mp4)
else fetch or conversion failed
Downloader-->>Poster: null
Poster->>Thread: post markdown fallback (videoUrl + optional caption)
end
Poster->>Thread: if failedCount>0 post summary text
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| * @param url - The video URL | ||
| * @returns The extracted filename | ||
| */ | ||
| function getFilenameFromUrl(url: string): string { |
There was a problem hiding this comment.
SRP - new file for getFilenameFromUrl.
| for (let i = 0; i < videos.length; i++) { | ||
| const v = videos[i]; | ||
| const videoBuffer = await downloadVideoBuffer(v.videoUrl!); | ||
|
|
||
| if (videoBuffer) { | ||
| const filename = getFilenameFromUrl(v.videoUrl!); | ||
| const label = videos.length > 1 ? `**Video ${i + 1}**` : ""; | ||
| const caption = v.captionText ? `> ${v.captionText}` : ""; | ||
| const markdown = [label, caption].filter(Boolean).join("\n"); | ||
|
|
||
| await thread.post({ | ||
| markdown: markdown || filename, | ||
| files: [ | ||
| { | ||
| data: videoBuffer, | ||
| filename, | ||
| mimeType: "video/mp4", | ||
| }, | ||
| ], | ||
| }); | ||
| } else { | ||
| // Fallback to URL link if download fails | ||
| const label = videos.length > 1 ? `**Video ${i + 1}:** ` : ""; | ||
| const caption = v.captionText ? `\n> ${v.captionText}` : ""; | ||
| await thread.post(`${label}${v.videoUrl}${caption}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
OCP - new file for loop / embed video logic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/agents/content/handleContentAgentCallback.ts (1)
83-118:⚠️ Potential issue | 🔴 CriticalThe new per-video posts are not actually idempotent yet.
Lines 93-113 perform non-idempotent
thread.post()calls while the thread staysrunninguntil Line 118. If an upload fails mid-loop, or a duplicate delivery arrives before the final state write, the callback will replay videos that were already posted. Persist per-video progress (URL/index/result ID) and skip already-sent items on retry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agents/content/handleContentAgentCallback.ts` around lines 83 - 118, The loop that posts each video (using videos[], downloadVideoBuffer, getFilenameFromUrl and thread.post) is not idempotent: if the callback retries before thread.setState({ status: "completed" }) the same video posts can be duplicated; persist per-video progress (e.g., by recording each video's index or URL and final post result ID to the thread or a durable store as you successfully post) and on start of the handler skip any videos already recorded as posted, only attempting download/post for items not yet marked, and ensure you write per-video success before continuing to the next item so retries resume safely without duplicates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/agents/content/downloadVideoBuffer.ts`:
- Around line 11-13: The error log in downloadVideoBuffer.ts currently prints
the full signed URL (variable url) on failure; change the console.error in the
response.ok check to redact the query string by parsing url with the URL
constructor and logging only the origin and pathname (or host+pathname) plus the
HTTP status. Locate the failing-block that checks response.ok (references:
response, url, downloadVideoBuffer) and replace the message so it does not
include url.search/query parameters but still includes clear context and the
status code.
- Around line 9-17: The downloadVideoBuffer function currently uses fetch(url)
and response.arrayBuffer() without timeout or size limits and logs the full URL;
add an AbortController with a timeout (same pattern used in
lib/serpapi/searchGoogleImages.ts) and pass controller.signal into fetch(url, {
signal }), and enforce a MAX_BYTES limit (like
lib/youtube/getResizedImageBuffer.ts) by streaming response.body with a reader,
accumulating chunks until MAX_BYTES and aborting/returning null if exceeded;
replace the console.error that prints the full URL with a redacted message that
omits query params (log HTTP status and that the URL was redacted) and ensure
you clear the timeout on success or failure.
In `@lib/agents/content/handleContentAgentCallback.ts`:
- Around line 111-115: The failed-run summary (check of failed.length and
thread.post of `_${failed.length} run(s) failed._`) is currently inside the
"some videos succeeded" branch and gets skipped when there are zero successful
videos; move that check out of the success-video branch so it runs
unconditionally after handling the completed callback. In the
handleContentAgentCallback flow, ensure you still post either the success-video
message or the "Content generation finished but no videos were produced."
message, then always run the failed.length > 0 check and call thread.post with
the failure summary so failures are reported even when no videos were produced.
---
Outside diff comments:
In `@lib/agents/content/handleContentAgentCallback.ts`:
- Around line 83-118: The loop that posts each video (using videos[],
downloadVideoBuffer, getFilenameFromUrl and thread.post) is not idempotent: if
the callback retries before thread.setState({ status: "completed" }) the same
video posts can be duplicated; persist per-video progress (e.g., by recording
each video's index or URL and final post result ID to the thread or a durable
store as you successfully post) and on start of the handler skip any videos
already recorded as posted, only attempting download/post for items not yet
marked, and ensure you write per-video success before continuing to the next
item so retries resume safely without duplicates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5650f52d-55fe-4185-9632-317305beb5ef
⛔ Files ignored due to path filters (2)
lib/agents/content/__tests__/downloadVideoBuffer.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/agents/content/__tests__/handleContentAgentCallback.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
lib/agents/content/downloadVideoBuffer.tslib/agents/content/handleContentAgentCallback.ts
| const response = await fetch(url); | ||
|
|
||
| if (!response.ok) { | ||
| console.error(`Failed to download video: HTTP ${response.status} from ${url}`); | ||
| return null; | ||
| } | ||
|
|
||
| const arrayBuffer = await response.arrayBuffer(); | ||
| return Buffer.from(arrayBuffer); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the downloadVideoBuffer.ts file
fd downloadVideoBuffer.tsRepository: recoupable/api
Length of output: 99
🏁 Script executed:
# Also check for the file that uses it
fd handleContentAgentCallback.tsRepository: recoupable/api
Length of output: 106
🏁 Script executed:
# Get the file structure to understand the context better
git ls-files lib/agents/content/Repository: recoupable/api
Length of output: 1105
🏁 Script executed:
cat -n lib/agents/content/downloadVideoBuffer.tsRepository: recoupable/api
Length of output: 854
🏁 Script executed:
cat -n lib/agents/content/handleContentAgentCallback.tsRepository: recoupable/api
Length of output: 5572
🏁 Script executed:
# Check if there are any existing timeout or size guards elsewhere in the codebase
rg "timeout|maxSize|max-size|MAX_SIZE" lib/agents/content/ -A 2 -B 2Repository: recoupable/api
Length of output: 1861
🏁 Script executed:
# Check if there are any fetch configuration or timeout utilities elsewhere in the codebase
rg "AbortController|AbortSignal|timeout" lib/ -t ts -t tsx | head -20Repository: recoupable/api
Length of output: 85
🏁 Script executed:
# Check if there's a configured fetch wrapper or utility function
fd -e ts -e tsx lib/networking/ lib/utils/ | head -20Repository: recoupable/api
Length of output: 155
🏁 Script executed:
# Look for any environment variables or constants related to request timeouts
rg "TIMEOUT|timeout" lib/agents/content/ -t tsRepository: recoupable/api
Length of output: 523
🏁 Script executed:
# Look for AbortController or timeout patterns in the broader codebase
rg "AbortController|AbortSignal" lib/ -t ts | head -20Repository: recoupable/api
Length of output: 219
🏁 Script executed:
# Check if there's a networking utilities directory
fd -type d networkingRepository: recoupable/api
Length of output: 228
🏁 Script executed:
# Look for any fetch wrappers or similar patterns
rg "fetch\(" lib/ -t ts -B 2 -A 2 | head -40Repository: recoupable/api
Length of output: 2324
🏁 Script executed:
# Check the test file to understand expected behavior
cat -n lib/agents/content/__tests__/downloadVideoBuffer.test.tsRepository: recoupable/api
Length of output: 2072
🏁 Script executed:
# Check what URLs are being downloaded - look at the context that calls downloadVideoBuffer
rg "videoUrl" lib/agents/content/ -B 2 -A 2Repository: recoupable/api
Length of output: 4883
🏁 Script executed:
# Check if there are other similar download functions in the codebase and how they handle this
rg "arrayBuffer\(\)" lib/ -B 5 -A 2Repository: recoupable/api
Length of output: 3235
🏁 Script executed:
# Look at the examples of AbortController usage to see the pattern
cat -n lib/serpapi/searchGoogleImages.ts | grep -A 10 "AbortController"Repository: recoupable/api
Length of output: 385
🏁 Script executed:
# Check if there are any environment constants for max sizes
rg "MAX_.*SIZE|maxSize" lib/ -t tsRepository: recoupable/api
Length of output: 288
Add timeout and max-size guards to this download.
This runs on the callback request path and blocks the handler until complete. fetch() has no timeout and arrayBuffer() loads the entire response into memory without bounds. A slow or oversized video can stall the route and trigger Trigger.dev retries instead of gracefully falling back to the URL post. Add an AbortController timeout (following the pattern in lib/serpapi/searchGoogleImages.ts) and a max-size check (similar to lib/youtube/getResizedImageBuffer.ts).
Additionally, redact the URL from the error log at line 12 to avoid exposing signed query parameters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agents/content/downloadVideoBuffer.ts` around lines 9 - 17, The
downloadVideoBuffer function currently uses fetch(url) and
response.arrayBuffer() without timeout or size limits and logs the full URL; add
an AbortController with a timeout (same pattern used in
lib/serpapi/searchGoogleImages.ts) and pass controller.signal into fetch(url, {
signal }), and enforce a MAX_BYTES limit (like
lib/youtube/getResizedImageBuffer.ts) by streaming response.body with a reader,
accumulating chunks until MAX_BYTES and aborting/returning null if exceeded;
replace the console.error that prints the full URL with a redacted message that
omits query params (log HTTP status and that the URL was redacted) and ensure
you clear the timeout on success or failure.
| if (!response.ok) { | ||
| console.error(`Failed to download video: HTTP ${response.status} from ${url}`); | ||
| return null; |
There was a problem hiding this comment.
Redact the asset URL in the error log.
Line 12 logs the full download URL. These asset URLs are commonly signed, so writing the query string to logs can leak temporary credentials. Log the status plus a redacted host/path instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agents/content/downloadVideoBuffer.ts` around lines 11 - 13, The error
log in downloadVideoBuffer.ts currently prints the full signed URL (variable
url) on failure; change the console.error in the response.ok check to redact the
query string by parsing url with the URL constructor and logging only the origin
and pathname (or host+pathname) plus the HTTP status. Locate the failing-block
that checks response.ok (references: response, url, downloadVideoBuffer) and
replace the message so it does not include url.search/query parameters but still
includes clear context and the status code.
| if (failed.length > 0) { | ||
| lines.push(`\n_${failed.length} run(s) failed._`); | ||
| await thread.post(`_${failed.length} run(s) failed._`); | ||
| } | ||
|
|
||
| await thread.post(lines.join("\n\n")); | ||
| } else { | ||
| await thread.post("Content generation finished but no videos were produced."); |
There was a problem hiding this comment.
Keep the failed-run summary outside the success-video branch.
Right now the failure count is only posted when at least one video succeeds. A completed callback with zero successful videos falls back to "no videos were produced" and drops the more actionable failure summary.
💡 Suggested change
- if (failed.length > 0) {
- await thread.post(`_${failed.length} run(s) failed._`);
- }
} else {
await thread.post("Content generation finished but no videos were produced.");
}
+
+ if (failed.length > 0) {
+ await thread.post(`_${failed.length} run(s) failed._`);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agents/content/handleContentAgentCallback.ts` around lines 111 - 115, The
failed-run summary (check of failed.length and thread.post of `_${failed.length}
run(s) failed._`) is currently inside the "some videos succeeded" branch and
gets skipped when there are zero successful videos; move that check out of the
success-video branch so it runs unconditionally after handling the completed
callback. In the handleContentAgentCallback flow, ensure you still post either
the success-video message or the "Content generation finished but no videos were
produced." message, then always run the failed.length > 0 check and call
thread.post with the failure summary so failures are reported even when no
videos were produced.
…e downloads - Extract getFilenameFromUrl into its own file with tests for edge cases (encoded chars, no extension, fal.ai URLs, query params) - Extract video embed loop into postVideoResults (SRP/OCP) - Download all videos in parallel with Promise.all instead of sequentially, then post to Slack in order - Update handleContentAgentCallback tests to mock postVideoResults Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/agents/content/handleContentAgentCallback.ts (1)
65-69:⚠️ Potential issue | 🟡 MinorFailed-run summary is still skipped when no video succeeds.
When
videos.length === 0,postVideoResultsis not called, so_${failed.length} run(s) failed._is never posted in completed callbacks with only failures.Suggested fix
if (videos.length > 0) { await postVideoResults(thread, videos, failed.length); } else { await thread.post("Content generation finished but no videos were produced."); + if (failed.length > 0) { + await thread.post(`_${failed.length} run(s) failed._`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agents/content/handleContentAgentCallback.ts` around lines 65 - 69, The code skips posting the failed-run summary when videos.length === 0; always call postVideoResults so the completed callback includes the "_X run(s) failed._" message even if no videos succeeded. Replace the conditional that only calls postVideoResults when videos.length > 0 with a single unconditional await postVideoResults(thread, videos, failed.length) (or otherwise ensure thread.post is invoked to include failed.length) so the failure summary is always posted; locate this change around the videos, failed, postVideoResults, and thread.post usage in handleContentAgentCallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/agents/content/postVideoResults.ts`:
- Around line 11-13: The Thread interface's post signature is incorrect: update
the Thread interface so post accepts the SDK union (string | PostableMessage |
CardJSXElement) and returns Promise<SentMessage> (or mirror SentMessage shape),
make markdown optional, include optional ast/card/raw fields, and broaden file
data to Buffer | Blob | ArrayBuffer; either import PostableMessage/SentMessage
from '@vercel/chat' and use them in the type for post, or inline an accurate
type matching those fields and file data variants for Thread.post.
---
Duplicate comments:
In `@lib/agents/content/handleContentAgentCallback.ts`:
- Around line 65-69: The code skips posting the failed-run summary when
videos.length === 0; always call postVideoResults so the completed callback
includes the "_X run(s) failed._" message even if no videos succeeded. Replace
the conditional that only calls postVideoResults when videos.length > 0 with a
single unconditional await postVideoResults(thread, videos, failed.length) (or
otherwise ensure thread.post is invoked to include failed.length) so the failure
summary is always posted; locate this change around the videos, failed,
postVideoResults, and thread.post usage in handleContentAgentCallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9d5a62c4-3d45-4452-881e-0b3ffe04d348
⛔ Files ignored due to path filters (3)
lib/agents/content/__tests__/getFilenameFromUrl.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/agents/content/__tests__/handleContentAgentCallback.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/agents/content/__tests__/postVideoResults.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (3)
lib/agents/content/getFilenameFromUrl.tslib/agents/content/handleContentAgentCallback.tslib/agents/content/postVideoResults.ts
| interface Thread { | ||
| post: (message: string | { markdown: string; files: { data: Buffer; filename: string; mimeType: string }[] }) => Promise<void>; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For npm package "chat" (version 4.15.0), what is the exact TypeScript signature of ThreadImpl.post, especially for file uploads (field names/types for attachments)?
💡 Result:
The TypeScript signature for ThreadImpl.post in the "chat" npm package version 4.15.0 (Vercel Chat SDK) is: typescript post(message: string | PostableMessage | CardJSXElement): Promise; PostableMessage is a structured message type that supports file uploads via the files field: typescript type PostableMessage = { markdown?: string; ast?: Root; card?: CardElement; raw?: string; files?: FileUpload[]; // ... other fields }; For file uploads specifically, use the files array in any structured PostableMessage (e.g., { markdown: "...", files: [...] }). The FileUpload type is: typescript interface FileUpload { filename: string; data: Buffer | Blob | ArrayBuffer; mimeType?: string; } Example for file upload: typescript await thread.post({ markdown: "Here's the report:", files: [{ filename: "report.pdf", data: fileBuffer, mimeType: "application/pdf" }] }); ThreadImpl is the concrete implementation of the Thread interface, and post delegates to the adapter-specific posting logic after normalizing the message format. This applies to v4.15.0 as documented in the package files and official docs.
Citations:
- 1: https://www.chat-sdk.dev/docs/api/thread
- 2: https://mintlify.com/vercel/chat/threads-messages-channels
- 3: https://www.mintlify.com/vercel/chat/api/types
- 4: https://www.chat-sdk.dev/docs/api/postable-message
- 5: https://www.chat-sdk.dev/docs/posting-messages
- 6: https://mintlify.com/vercel/chat/files
- 7: https://www.chat-sdk.dev/docs/files
Correct the Thread.post interface to match the Vercel Chat SDK contract.
The local Thread interface (lines 11-13) diverges from the actual ThreadImpl.post signature in Chat SDK v4.15.0. The SDK accepts string | PostableMessage | CardJSXElement and returns Promise<SentMessage>, but the local interface:
- Makes
markdownrequired instead of optional - Omits
ast,card,rawfields in the message object - Ignores the
CardJSXElementbranch - Returns
Promise<void>instead ofPromise<SentMessage> - Restricts
FileUpload.datato onlyBuffer(SDK also acceptsBlob | ArrayBuffer)
This mismatch creates type drift that tests won't catch and reduces flexibility for future posting patterns.
Update interface to reflect SDK contract
// Replace lines 11-13 with the actual SDK type or import it
import { PostableMessage, SentMessage } from '@vercel/chat'; // if exported
interface Thread {
post: (message: string | PostableMessage | CardJSXElement) => Promise<SentMessage>;
}Or define it more accurately inline:
interface Thread {
post: (message: string | { markdown?: string; ast?: any; card?: any; raw?: string; files?: Array<{ filename: string; data: Buffer | Blob | ArrayBuffer; mimeType?: string }> }) => Promise<{ messageId: string }>;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agents/content/postVideoResults.ts` around lines 11 - 13, The Thread
interface's post signature is incorrect: update the Thread interface so post
accepts the SDK union (string | PostableMessage | CardJSXElement) and returns
Promise<SentMessage> (or mirror SentMessage shape), make markdown optional,
include optional ast/card/raw fields, and broaden file data to Buffer | Blob |
ArrayBuffer; either import PostableMessage/SentMessage from '@vercel/chat' and
use them in the type for post, or inline an accurate type matching those fields
and file data variants for Thread.post.
Run prettier on new files. Change Thread.post return type from Promise<void> to Promise<unknown> to match ThreadImpl signature. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
downloadVideoBufferutility fetches the video data from the callback URLthread.post()per video) so Slack renders them as separate embedded mediaTest plan
downloadVideoBuffer(success, 404, network error, URL parsing)handleContentAgentCallback(file upload, fallback, multi-video, no caption, idempotency, partial failures)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor