fix(attachments): model egress attachments - #6312
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Secret provenance filtering now classifies workspace attachments by canonical storage key and provenance state only—it no longer rejects attachments when the caller’s Chat payload building now surfaces prepared uploads in model context when tracking succeeds, and fails the request with a clear error (instead of silently skipping) when an authorized attachment cannot be prepared (e.g. provenance sidecar issues). Shared DB loading for model-safe row lookups is consolidated for attachment filtering and batch key checks. Tests cover key-only attachments, successful upload context, and preparation failures. Reviewed by Cursor Bugbot for commit e698b10. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e698b10. Configure here.
| throw new Error( | ||
| `Failed to prepare attached file "${filename}" for Copilot. Please try again.`, | ||
| { cause } | ||
| ) |
There was a problem hiding this comment.
Prepare failure leaves chat incomplete
Medium Severity
When trackChatUpload fails, the new rethrow aborts after persistUserMessage has already saved the user turn. For workspace chats that also publish started, the request returns 500 without terminal status or assistant finalization, so the chat can keep an attachment-bearing user message and no completed turn.
Reviewed by Cursor Bugbot for commit e698b10. Configure here.
Greptile SummaryThis PR changes model-egress attachment provenance checks to classify files by their canonical storage key rather than runtime attachment IDs, and makes Copilot attachment-preparation failures abort the request.
Confidence Score: 4/5The attachment failure path should be fixed before merging because it can return an error after leaving an incomplete user turn in chat history. Attachment preparation now throws after user-message persistence has started, while the outer error handler returns a failure without reconciling that persisted message. Files Needing Attention: apps/sim/lib/copilot/chat/payload.ts and apps/sim/app/api/copilot/chat/post.ts
|
| Filename | Overview |
|---|---|
| apps/sim/lib/copilot/chat/payload.ts | Makes attachment tracking fail closed, but the exception occurs after message persistence has begun and can leave an incomplete chat turn. |
| apps/sim/lib/copilot/chat/payload.test.ts | Adds model-context and attachment-preparation failure coverage, but does not exercise request-level persistence cleanup. |
| apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.ts | Replaces attachment-ID matching with canonical, globally unique active storage-key classification and extracts the shared row query. |
| apps/sim/lib/uploads/contexts/workspace/workspace-file-secret-provenance.test.ts | Covers missing, synthesized, mismatched, and tainted attachment identities under key-based provenance classification. |
Sequence Diagram
sequenceDiagram
participant Client
participant API as Copilot API
participant DB
participant Payload as Payload Builder
participant Upload as Attachment Tracker
Client->>API: Submit message with attachment
API->>DB: Begin persisting user message
API->>Payload: Build model payload
Payload->>Upload: trackChatUpload
Upload-->>Payload: Error
Payload-->>API: Throw preparation error
DB-->>API: User message persisted
API-->>Client: HTTP 500
Note over DB,Client: Persisted user turn has no assistant response
Reviews (1): Last reviewed commit: "fix(attachments): model egress attachmen..." | Re-trigger Greptile
| chatId, | ||
| error: toError(err).message, | ||
| error: cause.message, | ||
| }) | ||
| throw new Error( |
There was a problem hiding this comment.
Attachment failure leaves persisted turn
When trackChatUpload rejects, this new exception aborts the request after persistence of the user message has already begun, causing the client to receive an HTTP 500 while the chat retains a user turn with no assistant response; retrying can then create duplicate user turns.


Summary
Fix attachments to correctly work when model egress is involved.
Type of Change
Testing
Tested manually
Checklist