feat: Slack media attachments for content pipeline#381
Merged
sweetmantech merged 15 commits intotestfrom Mar 31, 2026
Merged
Conversation
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>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExtracts the first audio and first image from a Slack message's attachments, uploads resolved files to Vercel Blob to obtain public URLs, and passes those URLs (songUrl, imageUrl) into validation and the create-content trigger payload (images optional; songs may be URLs). Changes
Sequence DiagramsequenceDiagram
participant Slack as Slack Message
participant Handler as registerOnNewMention
participant Extractor as extractMessageAttachments
participant Blob as Vercel Blob
participant Validator as validateCreateContentBody
participant Trigger as triggerCreateContent
participant Task as Content Creation Task
Slack->>Handler: onNewMention(message with attachments)
Handler->>Extractor: extractMessageAttachments(message)
Extractor->>Blob: upload audio (if present)
Blob-->>Extractor: audio URL
Extractor->>Blob: upload image (if present)
Blob-->>Extractor: image URL
Extractor-->>Handler: { songUrl, imageUrl }
Handler->>Validator: validateCreateContentBody(payload with songUrl/imageUrl)
Validator-->>Handler: validated payload
Handler->>Trigger: triggerCreateContent(payload with songs/images)
Trigger->>Task: tasks.trigger(CREATE_CONTENT_TASK_ID, payload)
Task-->>Trigger: triggered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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 |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/agents/content/extractMessageAttachments.ts (2)
62-63: Filename collisions may overwrite existing attachments.Using only
attachment.name(or the fallback"attachment") for the blob path could cause collisions when multiple users upload files with the same name. Consider adding a unique identifier (e.g., timestamp, UUID) to prevent overwrites.♻️ Proposed fix to add uniqueness
+import { randomUUID } from "crypto"; + async function uploadAttachment(attachment: Attachment, prefix: string): Promise<string> { const data = attachment.fetchData ? await attachment.fetchData() : (attachment.data as Buffer); const filename = attachment.name ?? "attachment"; - const blobPath = `content-attachments/${prefix}/${filename}`; + const uniqueId = randomUUID().slice(0, 8); + const blobPath = `content-attachments/${prefix}/${uniqueId}-${filename}`; const blob = await put(blobPath, data, { access: "public" }); return blob.url; }🤖 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 62 - 63, The blob path uses a non-unique filename (const filename and blobPath) which can cause collisions; modify extractMessageAttachments to append a unique identifier (e.g., timestamp or UUID) to the filename before building blobPath (for example derive a safe base name from attachment.name and add `-${uuid}` or `-${Date.now()}`) so blobPath becomes unique per upload and prevents overwriting existing attachments.
26-51: Consider handling partial upload failures gracefully.If the audio upload succeeds but the image upload fails, the function throws without returning the successful audio URL. Depending on requirements, you may want to return partial results or handle each upload independently.
♻️ Optional: Independent error handling for each upload
export async function extractMessageAttachments( message: MessageWithAttachments, ): Promise<ExtractedAttachments> { const result: ExtractedAttachments = { attachedAudioUrl: null, attachedImageUrl: null, }; const attachments = message.attachments; if (!attachments || attachments.length === 0) { return result; } const audioAttachment = attachments.find(a => a.type === "audio"); const imageAttachment = attachments.find(a => a.type === "image"); if (audioAttachment) { - result.attachedAudioUrl = await uploadAttachment(audioAttachment, "audio"); + try { + result.attachedAudioUrl = await uploadAttachment(audioAttachment, "audio"); + } catch (error) { + console.error("[extractMessageAttachments] Failed to upload audio:", error); + } } if (imageAttachment) { - result.attachedImageUrl = await uploadAttachment(imageAttachment, "image"); + try { + result.attachedImageUrl = await uploadAttachment(imageAttachment, "image"); + } catch (error) { + console.error("[extractMessageAttachments] Failed to upload image:", error); + } } return result; }🤖 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 26 - 51, The extractMessageAttachments function currently lets one failing upload prevent returning any successful uploads; modify extractMessageAttachments so each call to uploadAttachment (for audio and image) is wrapped in its own try/catch, set result.attachedAudioUrl and result.attachedImageUrl only on success, and log or collect the individual upload errors instead of rethrowing so the function can return partial results; keep the existing return shape (ExtractedAttachments) and ensure you reference uploadAttachment, attachedAudioUrl, attachedImageUrl, and the extractMessageAttachments function when making the change.lib/agents/content/handlers/registerOnNewMention.ts (1)
18-135: Consider extracting sub-handlers to reduce function length.The
onNewMentioncallback is approximately 115 lines, exceeding the 50-line guideline. While the logic is well-organized with clear sections, extracting some responsibilities (e.g.,resolveGitHubRepo,buildAcknowledgmentText,triggerAndPoll) into separate helper functions would improve maintainability and testability.This is an optional refactor that could be addressed in a follow-up PR.
🤖 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 - 135, The onNewMention callback in registerOnNewMention is too long; extract responsibilities into helpers to reduce length: implement a resolveGitHubRepo helper used in the block that calls getArtistContentReadiness/selectAccountSnapshots to return githubRepo (keep current error handling but move it), a buildAcknowledgmentText helper that accepts {artistSlug, template, batch, lipsync, songs, attachedAudioUrl, attachedImageUrl} and returns the posted string, and a triggerAndPoll helper that accepts the payload and thread (or runIds + callbackThreadId) to run Promise.allSettled, collect runIds, set thread state, and call triggerPollContentRun with proper error handling; then replace the corresponding inline sections in registerOnNewMention/onNewMention with calls to these helpers to shrink the callback and improve testability.
🤖 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/extractMessageAttachments.ts`:
- Around line 59-67: In uploadAttachment, validate that attachment.fetchData()
or attachment.data yields non-null Buffer before calling put(): check
(attachment.fetchData ? await attachment.fetchData() : attachment.data) is
defined and of expected type and throw or return a clear error if not; also wrap
the put(blobPath, data, { access: "public" }) call in a try/catch to surface a
helpful error (including the attachment.name and blobPath) or perform
cleanup/rollback as appropriate so upload failures don’t leave partial state;
reference symbols: uploadAttachment, attachment.fetchData, attachment.data, put,
blobPath, blob.url.
---
Nitpick comments:
In `@lib/agents/content/extractMessageAttachments.ts`:
- Around line 62-63: The blob path uses a non-unique filename (const filename
and blobPath) which can cause collisions; modify extractMessageAttachments to
append a unique identifier (e.g., timestamp or UUID) to the filename before
building blobPath (for example derive a safe base name from attachment.name and
add `-${uuid}` or `-${Date.now()}`) so blobPath becomes unique per upload and
prevents overwriting existing attachments.
- Around line 26-51: The extractMessageAttachments function currently lets one
failing upload prevent returning any successful uploads; modify
extractMessageAttachments so each call to uploadAttachment (for audio and image)
is wrapped in its own try/catch, set result.attachedAudioUrl and
result.attachedImageUrl only on success, and log or collect the individual
upload errors instead of rethrowing so the function can return partial results;
keep the existing return shape (ExtractedAttachments) and ensure you reference
uploadAttachment, attachedAudioUrl, attachedImageUrl, and the
extractMessageAttachments function when making the change.
In `@lib/agents/content/handlers/registerOnNewMention.ts`:
- Around line 18-135: The onNewMention callback in registerOnNewMention is too
long; extract responsibilities into helpers to reduce length: implement a
resolveGitHubRepo helper used in the block that calls
getArtistContentReadiness/selectAccountSnapshots to return githubRepo (keep
current error handling but move it), a buildAcknowledgmentText helper that
accepts {artistSlug, template, batch, lipsync, songs, attachedAudioUrl,
attachedImageUrl} and returns the posted string, and a triggerAndPoll helper
that accepts the payload and thread (or runIds + callbackThreadId) to run
Promise.allSettled, collect runIds, set thread state, and call
triggerPollContentRun with proper error handling; then replace the corresponding
inline sections in registerOnNewMention/onNewMention with calls to these helpers
to shrink the callback and improve testability.
🪄 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: ff05264a-d281-4c40-a313-2f007b81795f
⛔ 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 (5)
lib/agents/content/extractMessageAttachments.tslib/agents/content/handlers/registerOnNewMention.tslib/content/createContentHandler.tslib/content/validateCreateContentBody.tslib/trigger/triggerCreateContent.ts
sweetmantech
added a commit
to recoupable/docs
that referenced
this pull request
Mar 31, 2026
Document the new optional attachment parameters for POST /api/content/create. These allow callers to provide custom audio and face-guide images that override the defaults from the artist's GitHub repo. Companion to recoupable/api#381 and recoupable/tasks#116. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 task
…geAttachments 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>
sweetmantech
added a commit
to recoupable/docs
that referenced
this pull request
Mar 31, 2026
* docs: add attached_audio_url and attached_image_url to content/create Document the new optional attachment parameters for POST /api/content/create. These allow callers to provide custom audio and face-guide images that override the defaults from the artist's GitHub repo. Companion to recoupable/api#381 and recoupable/tasks#116. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: implement design feedback — songs accepts URLs, images array replaces attached_image_url Address sweetmantech's review: - Extend songs param to accept both slugs and public URLs (no separate attached_audio_url) - Replace attached_image_url with images array to match songs pattern (KISS) - Add Create Content to Guides navbar above Create Sandbox Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…places 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>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/agents/content/handlers/registerOnNewMention.ts (1)
82-84: Deduplicate merged songs to avoid duplicate downstream requests.If a prompt already includes the same URL as
songUrl,allSongscan contain duplicates.♻️ Proposed fix
- const allSongs = [...(songs ?? []), ...(songUrl ? [songUrl] : [])]; + const allSongs = Array.from(new Set([...(songs ?? []), ...(songUrl ? [songUrl] : [])]));🤖 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 82 - 84, The merged songs array (allSongs) can contain duplicates when songs already includes songUrl; update the assignment in registerOnNewMention to deduplicate the combined list: build the merged array from songs and songUrl (using the existing variables songs and songUrl) and then remove duplicates (e.g., via Array.from(new Set(...)) or a filter by indexOf) so downstream processing only receives unique URLs.
🤖 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/handlers/registerOnNewMention.ts`:
- Around line 29-31: In registerOnNewMention, avoid performing external uploads
via extractMessageAttachments before validating required prerequisites
(artist/repo checks) and make uploads optional: move the call to
extractMessageAttachments to after the validation logic that checks artist and
repository existence, and wrap the call in a try/catch so any errors from
extractMessageAttachments do not throw but instead log a warning and set
songUrl/imageUrl to undefined (soft-fail) so the handler can continue; reference
extractMessageAttachments and the registerOnNewMention handler when applying
this change.
---
Nitpick comments:
In `@lib/agents/content/handlers/registerOnNewMention.ts`:
- Around line 82-84: The merged songs array (allSongs) can contain duplicates
when songs already includes songUrl; update the assignment in
registerOnNewMention to deduplicate the combined list: build the merged array
from songs and songUrl (using the existing variables songs and songUrl) and then
remove duplicates (e.g., via Array.from(new Set(...)) or a filter by indexOf) so
downstream processing only receives unique URLs.
🪄 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: 7221b450-35b4-4fe9-9c95-8aaf9d435912
⛔ 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 (5)
lib/agents/content/extractMessageAttachments.tslib/agents/content/handlers/registerOnNewMention.tslib/content/createContentHandler.tslib/content/validateCreateContentBody.tslib/trigger/triggerCreateContent.ts
✅ Files skipped from review due to trivial changes (1)
- lib/content/createContentHandler.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/trigger/triggerCreateContent.ts
- lib/content/validateCreateContentBody.ts
- lib/agents/content/extractMessageAttachments.ts
Comment on lines
+29
to
+31
| // Extract audio/image attachments from the Slack message | ||
| const { songUrl, imageUrl } = await extractMessageAttachments(message); | ||
|
|
There was a problem hiding this comment.
Move attachment extraction after prerequisite validation and soft-fail on upload errors.
Line 30 triggers external upload before artist/repo checks. If validation fails later, the handler exits but files may already be publicly uploaded. Also, any extraction/upload error currently aborts the whole request, even though attachments are optional.
💡 Proposed fix
- // Extract audio/image attachments from the Slack message
- const { songUrl, imageUrl } = await extractMessageAttachments(message);
+ let songUrl: string | undefined;
+ let imageUrl: string | undefined;
...
// Resolve GitHub repo
let githubRepo: string;
try {
const readiness = await getArtistContentReadiness({
accountId,
artistAccountId,
artistSlug,
});
githubRepo = readiness.githubRepo;
} catch {
const snapshots = await selectAccountSnapshots(artistAccountId);
const repo = snapshots?.[0]?.github_repo;
if (!repo) {
await thread.post(
`No GitHub repository found for artist \`${artistSlug}\`. Content creation requires a configured repo.`,
);
return;
}
githubRepo = repo;
}
+
+ // Extract audio/image attachments only after request prerequisites pass.
+ try {
+ ({ songUrl, imageUrl } = await extractMessageAttachments(message));
+ } catch (attachmentError) {
+ console.warn("[content-agent] attachment extraction failed:", attachmentError);
+ }🤖 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 29 - 31, In
registerOnNewMention, avoid performing external uploads via
extractMessageAttachments before validating required prerequisites (artist/repo
checks) and make uploads optional: move the call to extractMessageAttachments to
after the validation logic that checks artist and repository existence, and wrap
the call in a try/catch so any errors from extractMessageAttachments do not
throw but instead log a warning and set songUrl/imageUrl to undefined
(soft-fail) so the handler can continue; reference extractMessageAttachments and
the registerOnNewMention handler when applying this change.
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>
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>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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>
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>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/agents/content/extractMessageAttachments.ts (1)
46-60: Deduplicate audio/image resolution branches.The two try/catch blocks are structurally identical and should be consolidated to reduce branching duplication and maintenance overhead.
♻️ DRY refactor sketch
- if (audioAttachment) { - try { - result.songUrl = await resolveAttachmentUrl(audioAttachment, "audio"); - } catch (error) { - console.error("[content-agent] Failed to resolve audio attachment:", error); - } - } - - if (imageAttachment) { - try { - result.imageUrl = await resolveAttachmentUrl(imageAttachment, "image"); - } catch (error) { - console.error("[content-agent] Failed to resolve image attachment:", error); - } - } + const resolutions: Array<{ + attachment: Attachment | undefined; + prefix: "audio" | "image"; + key: keyof ExtractedAttachments; + }> = [ + { attachment: audioAttachment, prefix: "audio", key: "songUrl" }, + { attachment: imageAttachment, prefix: "image", key: "imageUrl" }, + ]; + + for (const { attachment, prefix, key } of resolutions) { + if (!attachment) continue; + try { + result[key] = await resolveAttachmentUrl(attachment, prefix); + } catch (error) { + console.error(`[content-agent] Failed to resolve ${prefix} attachment:`, error); + } + }As per coding guidelines, "Extract shared logic into reusable utilities following Don't Repeat Yourself (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 46 - 60, The audio and image resolution try/catch blocks are duplicated; refactor by extracting the shared logic into a small helper (e.g., resolveAndAssignAttachment) that accepts an attachment object, a type string ("audio" | "image"), and a target key (e.g., "songUrl" or "imageUrl"), calls resolveAttachmentUrl(attachment, type) inside a single try/catch, and assigns the result to result[targetKey], logging errors the same way; then replace the two blocks that reference audioAttachment/imageAttachment with calls to this helper to remove duplication.
🤖 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/extractMessageAttachments.ts`:
- Around line 73-87: The logs in extractMessageAttachments.ts currently print
full URLs (attachment.url and blob.url) which can leak signed query params;
replace those console.log calls in the branch that returns attachment.url and
after put(...) so they do not emit the full URLs—log only non-sensitive metadata
(e.g., attachment.name, filename, blobPath, or a redacted indicator) or a
success message, and if necessary implement a redactUrl helper to strip query
params before logging; update references to attachment.url and blob.url
accordingly so raw URLs are never written to logs.
---
Nitpick comments:
In `@lib/agents/content/extractMessageAttachments.ts`:
- Around line 46-60: The audio and image resolution try/catch blocks are
duplicated; refactor by extracting the shared logic into a small helper (e.g.,
resolveAndAssignAttachment) that accepts an attachment object, a type string
("audio" | "image"), and a target key (e.g., "songUrl" or "imageUrl"), calls
resolveAttachmentUrl(attachment, type) inside a single try/catch, and assigns
the result to result[targetKey], logging errors the same way; then replace the
two blocks that reference audioAttachment/imageAttachment with calls to this
helper to remove duplication.
🪄 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: 375b5cd6-6fd5-4d37-922e-24cf0ca013df
⛔ Files ignored due to path filters (1)
lib/agents/content/__tests__/extractMessageAttachments.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (1)
lib/agents/content/extractMessageAttachments.ts
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>
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>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/agents/content/extractMessageAttachments.ts (1)
113-113: Default content-type assumptions may be incorrect for certain file formats.Defaulting to
audio/mpegfor audio andimage/pngfor images may cause issues when the actual file is a.wav,.ogg,.webp, or.gif. Consider deriving the content type from the filename extension as a fallback, or logging a warning when falling back to defaults.♻️ Proposed enhancement with extension-based fallback
+function inferContentType(filename: string, prefix: string): string { + const ext = filename.split(".").pop()?.toLowerCase(); + const mimeMap: Record<string, string> = { + mp3: "audio/mpeg", + wav: "audio/wav", + ogg: "audio/ogg", + m4a: "audio/mp4", + png: "image/png", + jpg: "image/jpeg", + jpeg: "image/jpeg", + gif: "image/gif", + webp: "image/webp", + }; + return mimeMap[ext ?? ""] ?? (prefix === "audio" ? "audio/mpeg" : "image/png"); +} + const filename = attachment.name ?? "attachment"; const blobPath = `content-attachments/${prefix}/${Date.now()}-${filename}`; - const contentType = attachment.mimeType ?? (prefix === "audio" ? "audio/mpeg" : "image/png"); + const contentType = attachment.mimeType ?? inferContentType(filename, prefix);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/agents/content/extractMessageAttachments.ts` at line 113, The current fallback for contentType in extractMessageAttachments.ts uses a fixed audio/mpeg or image/png which mislabels .wav/.ogg/.webp/.gif files; update the logic in the extractMessageAttachments function where contentType is set (currently using attachment.mimeType ?? (prefix === "audio" ? "audio/mpeg" : "image/png")) to instead attempt to derive the MIME type from attachment.filename extension (e.g., using a small extension-to-mime map or a mime lookup utility) before falling back to the generic audio/image defaults, and add a processLogger.warn or similar when you must use the generic fallback so unexpected types are visible.
🤖 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/extractMessageAttachments.ts`:
- Around line 101-104: The current logic in extractMessageAttachments that sets
`data` from `raw` (using `attachment.fetchData` or `attachment.data`)
incorrectly casts a `Blob` to an `ArrayBuffer`; update the branch that
constructs `data` so it detects a Blob and calls `await raw.arrayBuffer()`
before converting to a Buffer: i.e., if `raw` is already a Buffer keep it, if it
is a Blob await raw.arrayBuffer() then Buffer.from(...) , otherwise handle other
ArrayBuffer-like types via Buffer.from; adjust the code paths around
`attachment.fetchData`, `attachment.data`, and the `data` variable so Blob
inputs are correctly awaited and converted.
- Line 1: Run Prettier to fix the formatting issue in
lib/agents/content/extractMessageAttachments.ts; specifically reformat the file
(including the import line "import { put } from \"@vercel/blob\";") by executing
prettier --write so the file conforms to project style and clears the CI
Prettier warning.
---
Nitpick comments:
In `@lib/agents/content/extractMessageAttachments.ts`:
- Line 113: The current fallback for contentType in extractMessageAttachments.ts
uses a fixed audio/mpeg or image/png which mislabels .wav/.ogg/.webp/.gif files;
update the logic in the extractMessageAttachments function where contentType is
set (currently using attachment.mimeType ?? (prefix === "audio" ? "audio/mpeg" :
"image/png")) to instead attempt to derive the MIME type from
attachment.filename extension (e.g., using a small extension-to-mime map or a
mime lookup utility) before falling back to the generic audio/image defaults,
and add a processLogger.warn or similar when you must use the generic fallback
so unexpected types are visible.
🪄 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: 8eac377f-4009-427c-8346-73b97303d9c0
⛔ Files ignored due to path filters (1)
lib/agents/content/__tests__/extractMessageAttachments.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (1)
lib/agents/content/extractMessageAttachments.ts
| @@ -0,0 +1,141 @@ | |||
| import { put } from "@vercel/blob"; | |||
There was a problem hiding this comment.
Address Prettier formatting warning.
The CI pipeline flagged a formatting issue on this file. Run prettier --write to fix code style inconsistencies before merging.
🧰 Tools
🪛 GitHub Actions: Format Check
[warning] 1-1: Prettier formatting check ([warn]) reported code style issues in this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/agents/content/extractMessageAttachments.ts` at line 1, Run Prettier to
fix the formatting issue in lib/agents/content/extractMessageAttachments.ts;
specifically reformat the file (including the import line "import { put } from
\"@vercel/blob\";") by executing prettier --write so the file conforms to
project style and clears the CI Prettier warning.
Comment on lines
+101
to
+104
| 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.
Incorrect handling when attachment.data is a Blob.
The interface declares data?: Buffer | Blob, but if raw is a Blob, the cast raw as unknown as ArrayBuffer won't produce valid data—Blob is not directly convertible to ArrayBuffer without calling await blob.arrayBuffer() first.
🔧 Proposed fix to handle Blob correctly
// Fallback to fetchData / data if Slack download didn't work
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());
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 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 (!data) { | |
| const raw = attachment.fetchData ? await attachment.fetchData() : attachment.data; | |
| if (raw) { | |
| if (Buffer.isBuffer(raw)) { | |
| data = raw; | |
| } else if (raw instanceof Blob) { | |
| data = Buffer.from(await raw.arrayBuffer()); | |
| } | |
| } | |
| } |
🤖 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 101 - 104, The
current logic in extractMessageAttachments that sets `data` from `raw` (using
`attachment.fetchData` or `attachment.data`) incorrectly casts a `Blob` to an
`ArrayBuffer`; update the branch that constructs `data` so it detects a Blob and
calls `await raw.arrayBuffer()` before converting to a Buffer: i.e., if `raw` is
already a Buffer keep it, if it is a Blob await raw.arrayBuffer() then
Buffer.from(...) , otherwise handle other ArrayBuffer-like types via
Buffer.from; adjust the code paths around `attachment.fetchData`,
`attachment.data`, and the `data` variable so Blob inputs are correctly awaited
and converted.
…oad/ 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>
…cess 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>
| * then downloads with Bearer token auth. The attachment.url from the | ||
| * Chat SDK is a thumbnail URL that doesn't serve actual file content. | ||
| */ | ||
| async function resolveAttachmentUrl(attachment: Attachment, prefix: string): Promise<string | null> { |
Contributor
There was a problem hiding this comment.
SRP - new lib file for resolveAttachmentUrl and any function with a name different from the file name.
| * Extracts the Slack file ID (e.g. F0APMKTKG9M) from a Slack file URL. | ||
| * URL format: files-tmb/TEAMID-FILEID-HASH/name or files-pri/TEAMID-FILEID/name | ||
| */ | ||
| function extractSlackFileId(url: string): string | null { |
Contributor
There was a problem hiding this comment.
SRP - new lib file for extractSlackFileId and any function with a name different from the file name.
lib/content/createContentHandler.ts
Outdated
| upscale: validated.upscale, | ||
| githubRepo, | ||
| songs: validated.songs, | ||
| ...(validated.images && validated.images.length > 0 && { images: validated.images }), |
Contributor
There was a problem hiding this comment.
KISS - Why is images logic more complex than the songs logic?
…actSlackFileId 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>
sweetmantech
force-pushed
the
feature/slack-media-attachments
branch
from
08d2f26 to
3af8ed4
Compare
| * @param token | ||
| */ | ||
| export async function downloadSlackFile(fileId: string, token: string): Promise<Buffer | null> { | ||
| console.log(`[content-agent] Fetching file info for ${fileId}`); |
| const contentType = attachment.mimeType ?? (prefix === "audio" ? "audio/mpeg" : "image/png"); | ||
|
|
||
| const blob = await put(blobPath, data, { access: "public", contentType }); | ||
| console.log(`[content-agent] Uploaded to Blob: ${blob.url}, size=${data.byteLength}`); |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Slack utilities belong with other Slack helpers, not in agents/content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merged
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
2 participants
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
attached_audio_urlandattached_image_urlto the REST API endpoint for parityChanges
extractMessageAttachments.ts— extracts first audio/image from Slack message, uploads to Vercel BlobextractMessageAttachments.test.ts— 9 unit testsregisterOnNewMention.ts— calls extraction, passes URLs in payload, notes in ack messageregisterOnNewMention.test.ts— 4 new tests for attachment pass-throughtriggerCreateContent.ts— addsattachedAudioUrl/attachedImageUrlto payload interfacevalidateCreateContentBody.ts— acceptsattached_audio_url/attached_image_urlvia REST APIcreateContentHandler.ts— passes attachment URLs through to triggerTest plan
extractMessageAttachments(all pass)registerOnNewMentionincluding 4 new attachment tests (all pass)🤖 Generated with Claude Code
Summary by CodeRabbit