feat(platform): add file storage and attachment support for integration sandbox#462
Merged
Conversation
Add files API to the sandbox VM, allowing connectors to download and store files via an injected StorageProvider. Refactor execution logic out of internal_actions.ts into dedicated modules (execute_integration_impl, run_with_passes, validate_host, create_files_api, create_convex_storage_provider, execute_file_operation). Add binary HTTP body support and new tests.
…ersations Enable downloading, displaying, and sending file attachments through the integration sandbox. Inbound emails surface attachment metadata in the conversation UI with on-demand download via the sandbox file storage API. Outbound messages read attachments from Convex storage, base64-encode them, and pass them to the connector for inclusion in drafts.
📝 WalkthroughWalkthroughThis pull request adds comprehensive attachments support across the platform's email integration and conversation management layers. Changes include: updating the Outlook integration configuration and connector to support attachment operations (get_attachments, expand parameters, send with attachments); implementing attachment download and upload flows in the UI conversation panel and message components; extending conversation mutation handlers to accept and propagate attachments metadata; introducing a new multi-pass integration sandbox execution model (executeIntegrationImpl) with HTTP and file operation APIs to support asynchronous file handling in integration connectors; adding helper functions for file storage, host validation, and binary request handling; and updating conversation schemas and validators to include email attachment metadata. Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/convex/approvals/helpers.ts (1)
200-223:⚠️ Potential issue | 🟠 MajorAvoid early return; it can drop newer approvals from later statuses.
When the limit is reached while iterating the first status, the function returns before scanning remaining statuses. Since you sort by
_creationTimeacross statuses, this can exclude newer approvals in later statuses and return a stale set. Consider collecting across statuses, then sorting and slicing tolimit, or implement a merge/heap strategy.🔧 Suggested fix (collect → sort → slice)
for await (const approval of query) { if ( needsResourceTypeFilter && !resourceTypeSet.has(approval.resourceType) ) { continue; } if (searchLower && !matchesSearch(approval, searchLower)) { continue; } result.push(approval); - if (result.length >= limit) { - result.sort((a, b) => b._creationTime - a._creationTime); - return result; - } } } result.sort((a, b) => b._creationTime - a._creationTime); - return result; + return result.slice(0, limit);
🤖 Fix all issues with AI agents
In `@examples/integrations/outlook/connector.js`:
- Around line 789-840: The attachment download URL is incorrectly built using
params.messageId which will 404 when the original input was an
internetMessageId; update the downloadUrl construction inside the attachments
loop to use the resolved graphId variable instead of params.messageId (the code
paths around graphId, the attachments loop, and the downloadUrl string need to
reference graphId so the $value endpoint matches the message ID used to fetch
attachments).
- Around line 818-852: The getAttachments() function redeclares the variable
file in both the if and else branches causing scope confusion; rename the
variables to distinct names (e.g., storedFile for the files.store branch and
downloadedFile for the files.download branch) and update the attachments.push
references to use storedFile.fileId/storedFile.url and
downloadedFile.fileId/downloadedFile.url respectively so each branch uses its
own clearly named variable (keep all other fields like id, name, contentType,
size unchanged).
In
`@services/platform/app/features/conversations/components/conversation-panel.tsx`:
- Around line 366-376: The download error is only logged to console; update the
catch in the Message onDownloadAttachments handler to also show a user-facing
toast/notification (the same mechanism used for upload failure) so users see the
failure; modify the callback that calls downloadAttachments (and its .catch for
errors) to call the existing toast/notification utility (used in the upload
failure handling) with a clear message like "Failed to download attachments"
plus the error.message to provide context, referencing the Message component,
downloadAttachments call, and the toId<'conversationMessages'> conversion so you
update the correct handler.
In `@services/platform/app/features/conversations/components/message.tsx`:
- Around line 117-126: Replace the forbidden non-null assertion in the onClick
handler: inside the branch that checks hasUrl, stop using attachment.url! and
instead use the truthy value directly (e.g., read const url = attachment.url or
pass attachment.url to a.href) because hasUrl guarantees it exists; leave the
else branch calling onDownload?.() unchanged and ensure you reference hasUrl,
attachment.url, and onDownload in the updated handler.
- Around line 30-41: formatFileSize computes an index i that can exceed the
units array for very large bytes; in function formatFileSize guard against
out-of-bounds by capping i to units.length - 1 (or using Math.min(i,
units.length - 1)) before using units[i], so extremely large sizes fall back to
the largest unit instead of returning undefined; update the function (reference:
formatFileSize, variables bytes, k, i, units) to clamp i and then compute the
formatted value.
In `@services/platform/convex/conversations/internal_actions.ts`:
- Around line 393-408: The filename-only matching in the updatedAttachments
mapping can incorrectly reuse a single fileRef for multiple attachments with the
same a.filename; update the logic in the updatedAttachments/existingAttachments
mapping to avoid reusing the same fileRef: either match on an additional unique
property provided by the connector (e.g., attachment ID, size, or hash) if
available, or, if no extra property exists, ensure fileRefs is treated as a pool
(consume/remove the matched ref from fileRefs after matching) so each ref is
applied only once; refer to the variables updatedAttachments,
existingAttachments, fileRefs, and a.filename when making the change.
- Around line 38-47: The attachments schema uses storageId: v.string(), which
loses Convex ID validation; change it to storageId: v.id('_storage') so IDs are
validated and typed consistently when later used (e.g., passed to
ctx.storage.get()); update the attachments v.object in internal_actions.ts (the
storageId property) to v.id('_storage') to follow repository Convex ID
conventions for file-related APIs.
In `@services/platform/convex/conversations/internal_queries.ts`:
- Around line 111-118: getMessageById currently reads by messageId only; add an
organizationId arg to the internalQuery args (alongside messageId), fetch the
record via ctx.db.get(args.messageId), then validate the record’s organizationId
matches args.organizationId before returning; if no record or org IDs don’t
match return null (or same behavior as other queries). Update the handler to
perform this guard and keep the returns type using
internalMessageRecordValidator | null.
In `@services/platform/convex/conversations/mutations.ts`:
- Around line 60-67: The attachments schema uses v.object({... storageId:
v.string() ...}) but storageId must be an Id<'_storage'> for
ctx.storage.getUrl(); update the validator inside the attachments array object
to use storageId: v.id('_storage') instead of v.string() (keep other fields
unchanged) so the validator enforces the correct storage ID type for any handler
using attachments.
In
`@services/platform/convex/node_only/integration_sandbox/execute_integration_impl.ts`:
- Around line 79-151: The connector method calls (connectorObj.testConnection /
connectorObj.execute) run via runWithPasses after vm.runInContext and are not
covered by params.timeoutMs, so long-running connector code can bypass the
timeout; wrap the runWithPasses invocation in a timeout guard (or add a timeout
parameter into runWithPasses) that uses params.timeoutMs (fallback 30000) to
abort execution if exceeded (e.g., Promise.race with a timer or using an
AbortController passed into runWithPasses), and ensure the wrapper converts the
timeout to a clear error result and cleans up any resources; update callers for
both the __test_connection__ branch and the execute branch, and adjust
runWithPasses to accept and honor an optional timeout/abort signal if you choose
that approach (referencing vm.runInContext, runWithPasses,
connectorObj.testConnection, connectorObj.execute, params.timeoutMs, and
MAX_PASSES).
In
`@services/platform/convex/node_only/integration_sandbox/helpers/create_convex_storage_provider.ts`:
- Around line 19-38: The download function currently validates only the initial
URL via validateHost but uses globalThis.fetch which follows redirects and can
fetch disallowed hosts; change download to disable automatic redirects (pass
redirect: 'manual' to globalThis.fetch) and then handle 3xx responses by reading
the Location header, resolving it against the original URL, running validateHost
on each redirect target, and either follow validated redirects manually or
reject when Location is missing or the host is not allowed; ensure the
subsequent fetches before calling ctx.storage.store and ctx.storage.getUrl also
run validateHost on their URLs so no redirected fetch can bypass the allowlist.
In
`@services/platform/convex/node_only/integration_sandbox/helpers/run_with_passes.ts`:
- Around line 107-110: The loop in runWithPasses (involving MAX_PASSES,
httpRequests and fileRequests) can exit due to reaching MAX_PASSES without
clearing pending ops; update runWithPasses to signal that condition by detecting
when the loop ends with httpRequests.length>0 or fileRequests.length>0 and
MAX_PASSES reached, then either emit a warning (e.g., processLogger.warn or
console.warn) with counts of remaining ops or augment the return value with
metadata (e.g., a truncated:boolean and remainingCounts) so callers can detect
incomplete execution; modify callers accordingly to handle the new metadata or
rely on the warning.
… base64 Pass Convex storage URLs to connectors and let them download file content directly using the new `responseType: 'base64'` HTTP option, avoiding large base64 payloads in action arguments. The Convex storage host is dynamically whitelisted in allowedHosts for the sandbox.
…in attachment download
… pending operations
5 tasks
4 tasks
This was referenced Mar 14, 2026
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
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
1 participant
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
execute_integration_impl,run_with_passes,validate_host,create_files_api,create_convex_storage_provider,execute_file_operation) for better maintainabilityTest plan
binary_http_requests.test.ts)files_api.test.ts)http_methods.test.ts)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation