Conversation
* feat: extract Slack message audio/image attachments for content pipeline When users attach audio or image files to their Slack message that triggers the Recoup Content Agent, those attachments are now extracted, uploaded to Vercel Blob storage, and passed through to the content creation pipeline. - Audio attachments replace the song selection from Git - Image attachments replace the face-guide from the artist's repo New files: - extractMessageAttachments.ts — extracts and uploads Slack attachments - extractMessageAttachments.test.ts — 9 tests for attachment extraction Modified files: - registerOnNewMention.ts — calls extractMessageAttachments, passes URLs - triggerCreateContent.ts — adds attachedAudioUrl/attachedImageUrl to payload - validateCreateContentBody.ts — accepts attached_audio_url/attached_image_url - createContentHandler.ts — passes attachment URLs through to trigger Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix: add data guard, error handling, and unique paths in extractMessageAttachments Address CodeRabbit review feedback: - Guard against undefined attachment data - Gracefully handle upload failures without crashing the handler - Add timestamp prefix to blob paths to prevent filename collisions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: align with docs/tasks — songs accepts URLs, images array replaces attachedAudioUrl/attachedImageUrl - extractMessageAttachments returns songUrl/imageUrl (not attachedAudioUrl/attachedImageUrl) - registerOnNewMention merges songUrl into songs array, imageUrl into images array - validateCreateContentBody accepts images array, removes attached_audio_url/attached_image_url - triggerCreateContent payload uses songs/images (matches tasks schema) - createContentHandler passes images array TDD: 1659/1659 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: detect Slack file uploads by mimeType, not just attachment type Slack classifies uploaded images/audio as type: "file", not "image"/"audio". Check mimeType (e.g. "image/jpeg", "audio/mpeg") to correctly detect media from Slack file uploads. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * debug: add logging to uploadAttachment to diagnose Blob corruption Log attachment metadata (type, name, mimeType, url, fetchData presence), fetched data size, and uploaded Blob URL to trace why files are corrupt. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add url to Attachment interface for debug logging Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use direct attachment URL instead of Blob re-upload Blob upload was corrupting both audio and image files (empty content). Now uses attachment.url directly when available, skipping the download+reupload round-trip. Falls back to Blob only when no URL exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: pass contentType to Blob put() to prevent file corruption Vercel Blob was serving files with wrong content type when contentType wasn't explicitly set. Now passes attachment.mimeType (or sensible default) to put(). Slack URLs are private so we must keep the Blob upload path but with correct content type headers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: download Slack files with bot token instead of broken fetchData Chat SDK fetchData() returns Slack HTML login page instead of file content. Now downloads directly from attachment.url using SLACK_CONTENT_BOT_TOKEN Bearer auth, then uploads to Blob with correct contentType. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: convert Slack thumbnail URLs to download URLs for file access Slack attachment.url is a files-tmb (thumbnail) URL that returns HTML. Convert to files-pri/download URL format with bot token Bearer auth to get actual file content. Verify response isn't HTML before uploading. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: correct Slack file URL conversion — use files-pri without /download/ The /download/ suffix caused 404s. Slack files-pri URLs work with just the team-fileID path + Bearer token auth. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use Slack files.info API to get url_private_download for file access The attachment.url from Chat SDK is a thumbnail URL (files-tmb) that doesn't serve actual file content. Now extracts the Slack file ID, calls files.info to get url_private_download, then downloads with Bearer token auth. This is the official Slack file download flow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: SRP — extract resolveAttachmentUrl, downloadSlackFile, extractSlackFileId Address sweetmantech review: - SRP: each function in its own file - KISS: images passed through same as songs in createContentHandler Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove dev logging, fix formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: move downloadSlackFile and extractSlackFileId to lib/slack/ Slack utilities belong with other Slack helpers, not in agents/content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: CTO Agent <cto@recoup.ai> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Sweets Sweetman <sweetmantech@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request adds support for extracting audio and image attachments from Slack messages, resolving them to publicly accessible URLs via Slack's API and Vercel Blob storage, and integrating them into the content creation pipeline with validation and payload extension. Changes
Sequence DiagramsequenceDiagram
participant Slack
participant registerOnNewMention as Mention Handler
participant extractAttach as extractMessageAttachments
participant resolveURL as resolveAttachmentUrl
participant SlackAPI as Slack API
participant VercelBlob as Vercel Blob
participant Pipeline as Content Pipeline
Slack->>registerOnNewMention: New mention with attachments
registerOnNewMention->>extractAttach: Extract songUrl & imageUrl
extractAttach->>resolveURL: Resolve audio attachment
resolveURL->>SlackAPI: Check for Slack URL
SlackAPI->>SlackAPI: files.info (get private URL)
SlackAPI->>SlackAPI: Download file content
SlackAPI-->>resolveURL: File Buffer
resolveURL->>VercelBlob: Upload to public blob storage
VercelBlob-->>resolveURL: Public URL
resolveURL-->>extractAttach: songUrl (or null)
extractAttach->>resolveURL: Resolve image attachment
resolveURL->>VercelBlob: Upload to public blob storage
VercelBlob-->>resolveURL: Public URL
resolveURL-->>extractAttach: imageUrl (or null)
extractAttach-->>registerOnNewMention: ExtractedAttachments
registerOnNewMention->>Pipeline: Pass songUrl, imageUrl in payload
Pipeline-->>Slack: Acknowledgment with attachment details
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
lib/agents/content/resolveAttachmentUrl.ts (2)
55-56: Missing error handling for Vercel Blobput()operation.The
put()call can throw on network errors or Blob storage issues, but this is not wrapped in try-catch. The caller (extractMessageAttachments) does catch exceptions, but adding explicit handling here would provide better error messages.♻️ Optional: Add explicit error handling
+ try { const blob = await put(blobPath, data, { access: "public", contentType }); return blob.url; + } catch (error) { + console.error(`[content-agent] Blob upload failed for "${filename}":`, error); + return null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agents/content/resolveAttachmentUrl.ts` around lines 55 - 56, The Vercel Blob put() call in resolveAttachmentUrl (the blob = await put(blobPath, data, { access: "public", contentType }) line) lacks explicit error handling; wrap the put() call in a try-catch inside the resolveAttachmentUrl function, catch and rethrow or return a clear, contextual error (include blobPath and contentType) so callers like extractMessageAttachments receive a descriptive message; ensure you preserve the original error stack when rethrowing (e.g., throw new Error(`Failed to upload blob ${blobPath}: ${err.message}`) or attach the original error) and keep returning blob.url on success.
3-12: Extract sharedAttachmentinterface to avoid duplication.The
Attachmentinterface is duplicated inextractMessageAttachments.ts. Per DRY principle, extract this to a shared types file.♻️ Suggested approach
Create
lib/agents/content/types.ts:export interface Attachment { type: "image" | "file" | "video" | "audio"; mimeType?: string; name?: string; url?: string; data?: Buffer | Blob; fetchData?: () => Promise<Buffer>; }Then import from both files:
-interface Attachment { - type: "image" | "file" | "video" | "audio"; - ... -} +import { Attachment } from "./types";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agents/content/resolveAttachmentUrl.ts` around lines 3 - 12, Duplicate Attachment interface across modules: extract the interface into a shared types module (export interface Attachment { ... }) and replace the local declarations by importing that shared Attachment type where used (e.g., in resolveAttachmentUrl.ts and extractMessageAttachments.ts). Update imports to reference the new exported Attachment and remove the duplicated interface declarations so both resolveAttachmentUrl and extractMessageAttachments use the single shared type.lib/agents/content/extractMessageAttachments.ts (1)
3-14: DuplicateAttachmentinterface - extract to shared module.As noted in
resolveAttachmentUrl.ts, this interface is duplicated. Extract to a shared types file to maintain DRY principle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agents/content/extractMessageAttachments.ts` around lines 3 - 14, The Attachment and MessageWithAttachments interfaces are duplicated; extract them into a shared types module and import there instead of redefining. Create a new shared type (e.g., export interface Attachment and export interface MessageWithAttachments) and replace the local definitions in extractMessageAttachments.ts and resolveAttachmentUrl.ts by importing those types; update any references to Attachment or MessageWithAttachments in functions like extractMessageAttachments and resolveAttachmentUrl to use the shared types.lib/slack/downloadSlackFile.ts (1)
9-11: Consider adding fetch timeouts to prevent indefinite hangs.Both fetch calls lack timeout configuration. If Slack's API or the download endpoint becomes unresponsive, this function will hang indefinitely, potentially blocking upstream callers and exhausting resources.
♻️ Proposed fix using AbortController
export async function downloadSlackFile(fileId: string, token: string): Promise<Buffer | null> { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 30000); + - const infoResponse = await fetch(`https://slack.com/api/files.info?file=${fileId}`, { - headers: { Authorization: `Bearer ${token}` }, - }); + try { + const infoResponse = await fetch(`https://slack.com/api/files.info?file=${fileId}`, { + headers: { Authorization: `Bearer ${token}` }, + signal: controller.signal, + });Apply similar pattern to the file download fetch as well.
Also applies to: 35-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/slack/downloadSlackFile.ts` around lines 9 - 11, The two fetch calls in downloadSlackFile (the info fetch that assigns infoResponse and the subsequent file download fetch) need timeouts to avoid hanging: create an AbortController, set a timeout (e.g. const timeoutId = setTimeout(() => controller.abort(), TIMEOUT_MS)), pass controller.signal to both fetch calls' options, and clearTimeout(timeoutId) after each successful response; also catch and handle the abort error (signal.aborted/DOMException name 'AbortError') to return/throw a clear timeout-specific error.lib/agents/content/handlers/registerOnNewMention.ts (1)
18-137: Consider extracting sub-functions to reduce handler complexity.The handler function spans ~118 lines, exceeding the 50-line guideline. While the logic is clear, extracting helper functions (e.g.,
buildAcknowledgmentDetails,resolveGithubRepo) would improve maintainability and testability.As per coding guidelines: "Keep functions under 50 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agents/content/handlers/registerOnNewMention.ts` around lines 18 - 137, The registerOnNewMention handler is too large; extract smaller helpers to reduce complexity and meet the <50-line> guideline by introducing functions such as resolveGithubRepo(accountId, artistAccountId, artistSlug) (wraps getArtistContentReadiness and fallback to selectAccountSnapshots), buildAcknowledgmentDetails({artistSlug, template, batch, lipsync, songs, songUrl, imageUrl}) (returns the details array used in the thread.post acknowledgment), and buildCreatePayload({accountId, artistSlug, template, lipsync, captionLength, upscale, githubRepo, songs, imageUrl}) (returns the payload object sent to triggerCreateContent); update registerOnNewMention to call parseContentPrompt, extractMessageAttachments, resolveArtistSlug, resolveGithubRepo, buildAcknowledgmentDetails, buildCreatePayload, and then use triggerCreateContent/triggerPollContentRun as before so the top-level handler becomes a short orchestration function.
🤖 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/resolveAttachmentUrl.ts`:
- Around line 38-42: In resolveAttachmentUrl.ts inside the fallback block where
you set data from attachment.fetchData()/attachment.data (variables: attachment,
raw, data), handle the case where raw is a Blob before calling Buffer.from;
detect Blob (e.g., raw instanceof Blob) and await raw.arrayBuffer() to get an
ArrayBuffer, then call Buffer.from on that ArrayBuffer (also accept raw already
a Buffer or ArrayBuffer/Uint8Array). Update the logic in the fallback that
currently does Buffer.from(raw as unknown as ArrayBuffer) so it safely converts
Blob -> ArrayBuffer -> Buffer and preserves existing Buffer/raw typed values.
---
Nitpick comments:
In `@lib/agents/content/extractMessageAttachments.ts`:
- Around line 3-14: The Attachment and MessageWithAttachments interfaces are
duplicated; extract them into a shared types module and import there instead of
redefining. Create a new shared type (e.g., export interface Attachment and
export interface MessageWithAttachments) and replace the local definitions in
extractMessageAttachments.ts and resolveAttachmentUrl.ts by importing those
types; update any references to Attachment or MessageWithAttachments in
functions like extractMessageAttachments and resolveAttachmentUrl to use the
shared types.
In `@lib/agents/content/handlers/registerOnNewMention.ts`:
- Around line 18-137: The registerOnNewMention handler is too large; extract
smaller helpers to reduce complexity and meet the <50-line> guideline by
introducing functions such as resolveGithubRepo(accountId, artistAccountId,
artistSlug) (wraps getArtistContentReadiness and fallback to
selectAccountSnapshots), buildAcknowledgmentDetails({artistSlug, template,
batch, lipsync, songs, songUrl, imageUrl}) (returns the details array used in
the thread.post acknowledgment), and buildCreatePayload({accountId, artistSlug,
template, lipsync, captionLength, upscale, githubRepo, songs, imageUrl})
(returns the payload object sent to triggerCreateContent); update
registerOnNewMention to call parseContentPrompt, extractMessageAttachments,
resolveArtistSlug, resolveGithubRepo, buildAcknowledgmentDetails,
buildCreatePayload, and then use triggerCreateContent/triggerPollContentRun as
before so the top-level handler becomes a short orchestration function.
In `@lib/agents/content/resolveAttachmentUrl.ts`:
- Around line 55-56: The Vercel Blob put() call in resolveAttachmentUrl (the
blob = await put(blobPath, data, { access: "public", contentType }) line) lacks
explicit error handling; wrap the put() call in a try-catch inside the
resolveAttachmentUrl function, catch and rethrow or return a clear, contextual
error (include blobPath and contentType) so callers like
extractMessageAttachments receive a descriptive message; ensure you preserve the
original error stack when rethrowing (e.g., throw new Error(`Failed to upload
blob ${blobPath}: ${err.message}`) or attach the original error) and keep
returning blob.url on success.
- Around line 3-12: Duplicate Attachment interface across modules: extract the
interface into a shared types module (export interface Attachment { ... }) and
replace the local declarations by importing that shared Attachment type where
used (e.g., in resolveAttachmentUrl.ts and extractMessageAttachments.ts). Update
imports to reference the new exported Attachment and remove the duplicated
interface declarations so both resolveAttachmentUrl and
extractMessageAttachments use the single shared type.
In `@lib/slack/downloadSlackFile.ts`:
- Around line 9-11: The two fetch calls in downloadSlackFile (the info fetch
that assigns infoResponse and the subsequent file download fetch) need timeouts
to avoid hanging: create an AbortController, set a timeout (e.g. const timeoutId
= setTimeout(() => controller.abort(), TIMEOUT_MS)), pass controller.signal to
both fetch calls' options, and clearTimeout(timeoutId) after each successful
response; also catch and handle the abort error (signal.aborted/DOMException
name 'AbortError') to return/throw a clear timeout-specific error.
🪄 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: 4046012d-056a-49d2-9774-c5fb310e6fc2
⛔ Files ignored due to path filters (2)
lib/agents/content/__tests__/extractMessageAttachments.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/agents/content/__tests__/registerOnNewMention.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (8)
lib/agents/content/extractMessageAttachments.tslib/agents/content/handlers/registerOnNewMention.tslib/agents/content/resolveAttachmentUrl.tslib/content/createContentHandler.tslib/content/validateCreateContentBody.tslib/slack/downloadSlackFile.tslib/slack/extractSlackFileId.tslib/trigger/triggerCreateContent.ts
| // Fallback to fetchData / data | ||
| if (!data) { | ||
| const raw = attachment.fetchData ? await attachment.fetchData() : attachment.data; | ||
| if (raw) data = Buffer.isBuffer(raw) ? raw : Buffer.from(raw as unknown as ArrayBuffer); | ||
| } |
There was a problem hiding this comment.
Unsafe Buffer.from cast when raw is a Blob.
When attachment.data is a Blob, Buffer.from(raw as unknown as ArrayBuffer) will not work correctly. Blob is not an ArrayBuffer; you need to call blob.arrayBuffer() first.
🐛 Proposed fix
// Fallback to fetchData / data
if (!data) {
const raw = attachment.fetchData ? await attachment.fetchData() : attachment.data;
- if (raw) data = Buffer.isBuffer(raw) ? raw : Buffer.from(raw as unknown as ArrayBuffer);
+ if (raw) {
+ if (Buffer.isBuffer(raw)) {
+ data = raw;
+ } else if (raw instanceof Blob) {
+ data = Buffer.from(await raw.arrayBuffer());
+ } else {
+ data = Buffer.from(raw as ArrayBuffer);
+ }
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agents/content/resolveAttachmentUrl.ts` around lines 38 - 42, In
resolveAttachmentUrl.ts inside the fallback block where you set data from
attachment.fetchData()/attachment.data (variables: attachment, raw, data),
handle the case where raw is a Blob before calling Buffer.from; detect Blob
(e.g., raw instanceof Blob) and await raw.arrayBuffer() to get an ArrayBuffer,
then call Buffer.from on that ArrayBuffer (also accept raw already a Buffer or
ArrayBuffer/Uint8Array). Update the logic in the fallback that currently does
Buffer.from(raw as unknown as ArrayBuffer) so it safely converts Blob ->
ArrayBuffer -> Buffer and preserves existing Buffer/raw typed values.
Summary
🤖 Generated with Claude Code
Summary by CodeRabbit