Support text file attachments in chat#3920
Conversation
- Store supported text files in the project workspace - Insert attachment links into the chat prompt
- Base user-message collapse thresholds on visible link text - Add regression coverage for markdown attachment links
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. New feature adding text file attachment support with 7 unresolved review comments identifying substantive bugs including a high-severity storage path issue, race conditions, and platform-specific path handling problems. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f044e403be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : `thread:${target.environmentId}:${target.threadId}`; | ||
| } | ||
|
|
||
| function textAttachmentStorageCwd(projectCwd: string): string { |
There was a problem hiding this comment.
🟡 Medium chat/ChatComposer.tsx:139
textAttachmentStorageCwd infers the separator by checking whether projectCwd contains a backslash. On POSIX, \ is a valid filename character, so a workspace path like /tmp/foo\bar produces /tmp/foo\bar\..\.t3\attachments, treating the literal backslashes in the dirname as separators. The computed directory is not the intended sibling .t3/attachments folder, so text attachment writes fail. Consider deriving the separator from the platform (process.platform or a configured runtime value) rather than from the path string.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 139:
`textAttachmentStorageCwd` infers the separator by checking whether `projectCwd` contains a backslash. On POSIX, `\` is a valid filename character, so a workspace path like `/tmp/foo\bar` produces `/tmp/foo\bar\..\.t3\attachments`, treating the literal backslashes in the dirname as separators. The computed directory is not the intended sibling `.t3/attachments` folder, so text attachment writes fail. Consider deriving the separator from the platform (`process.platform` or a configured runtime value) rather than from the path string.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e38c0b4. Configure here.
|
|
||
| function textAttachmentStorageCwd(projectCwd: string): string { | ||
| const separator = projectCwd.includes("\\") ? "\\" : "/"; | ||
| return `${projectCwd.replace(/[\\/]+$/, "")}${separator}..${separator}.t3${separator}attachments`; |
There was a problem hiding this comment.
Attachment storage resolves outside project
High Severity
textAttachmentStorageCwd builds the write directory as the parent of gitCwd plus .t3/attachments, instead of under the project workspace. Text files are written and linked from that external path, so attachments no longer live at gitCwd/.t3/attachments as described. Sibling repos can share the same folder, worktree sessions get nested paths like .t3/worktrees/.t3/attachments, and markdown file links often fall outside workspaceRoot, breaking in-app preview.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e38c0b4. Configure here.
| composerDraftTarget, | ||
| `${currentPrompt}${separator}${serializeComposerFileLink( | ||
| resolvePathLinkTarget(relativePath, attachmentCwd), | ||
| )} `, |
There was a problem hiding this comment.
Send uses stale prompt ref
Medium Severity
After a text attachment succeeds, the draft store is updated with the new file link, but promptRef is not updated in the same turn. ChatView’s onSend still reads promptRef.current, so a send right after the composer shows the chip can omit the attachment from the outgoing message.
Reviewed by Cursor Bugbot for commit e38c0b4. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e38c0b47fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const TEXT_ATTACHMENT_PATH_PATTERN = /(?:^|[\\/])\.t3[\\/]attachments[\\/]/; | ||
|
|
||
| export function inferFileTagEntryKind(path: string): "file" | "directory" { | ||
| return TEXT_ATTACHMENT_PATH_PATTERN.test(path) ? "file" : inferEntryKindFromPath(path); |
There was a problem hiding this comment.
Recognize attachment links before rendering chips
This .t3/attachments check only runs after a markdown link has already been accepted as a file link. In ChatMarkdown, resolveMarkdownFileLinkMeta still rejects extensionless absolute paths under roots outside its POSIX allowlist, so an attachment like /workspace/project/../.t3/attachments/<id>/extensionless-test renders as a normal markdown link instead of the file chip/openable file this change is trying to support. Move the attachment-path special case into the markdown file-link detection path as well, not just the icon-kind inference.
Useful? React with 👍 / 👎.
|
|
||
| function textAttachmentStorageCwd(projectCwd: string): string { | ||
| const separator = projectCwd.includes("\\") ? "\\" : "/"; | ||
| return `${projectCwd.replace(/[\\/]+$/, "")}${separator}..${separator}.t3${separator}attachments`; |
There was a problem hiding this comment.
Normalize attachment paths before linking them
Because this returns a path containing the literal project segment followed by .., an attachment from /repo/app is linked as /repo/app/../.t3/attachments/.... The markdown link resolver's workspaceRelativePath check is a string-prefix check, so extension-bearing attachment links are misclassified as in-workspace ../.t3/...; ChatMarkdown then opens the right-panel file preview, whose project read rejects traversal outside the workspace, instead of opening the generated attachment. Normalize the storage cwd before serializing the link, or ensure these links are treated as external to the project preview path.
Useful? React with 👍 / 👎.
| setComposerDraftPrompt( | ||
| composerDraftTarget, | ||
| `${currentPrompt}${separator}${serializeComposerFileLink( | ||
| resolvePathLinkTarget(relativePath, attachmentCwd), | ||
| )} `, |
There was a problem hiding this comment.
Synchronize promptRef when appending attachment links
After a text attachment is written, this updates only the draft store; promptRef.current is not updated until the later passive sync effect. The fresh evidence versus the earlier pending-attachment race is that isAttachingFiles is cleared as soon as this promise settles, while onSend still reads promptRef.current, so an immediate send after the button re-enables can submit the old prompt without the newly visible file link. Assign the computed next prompt to promptRef.current before calling setComposerDraftPrompt.
Useful? React with 👍 / 👎.


Verification
Summary
.t3/attachmentsand insert standard file-link chips into the promptWhy
The composer previously rejected every dropped file that was not an image. This made it impossible to attach Markdown, source, configuration, or other text files as agent context.
Text attachments now reuse the existing project file-writing and Markdown file-link paths, so they render and behave like other file references. Files are limited to 1 MB and rejected when they contain NUL bytes or invalid UTF-8.
The message collapse heuristic also now measures displayed link labels instead of their backing absolute paths. Without this, prompts containing several attachments could appear hidden even though their visible content was short.
Validation
pnpm exec vp test apps/web/src/components/chat/MessagesTimeline.test.tsxpnpm exec vp checkpnpm exec vp run typecheckNote
Support text file attachments in chat via paste, drag-drop, and file links
ChatComposerto accept text files (up to 1 MB, valid UTF-8) alongside images via paste and drag-drop; non-image files are written to.t3/attachments/and linked into the prompt as markdown file links.enqueueComposerAttachments), disabling the send button while attachments are in-flight.shouldCollapseUserMessageto strip markdown file links before evaluating collapse thresholds, preventing messages with many file links from being incorrectly collapsed..t3/attachments/by treating them as files ininferFileTagEntryKind.📊 Macroscope summarized e38c0b4. 3 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.
Note
Medium Risk
Writes user-provided text into the project workspace and mutates the composer prompt asynchronously; mistakes could affect agent context or attachment limits, but scope is limited to the composer UI and existing write-file path.
Overview
The chat composer now accepts non-image files from paste and drag-and-drop, not only images. UTF-8 text up to 1 MB is validated (no NUL bytes), written under
.t3/attachments/<uuid>/, and appended to the prompt as the same file-link chips used for@path picks.Attachment handling is queued per draft/thread so mixed batches stay ordered; send stays disabled while uploads run, and image limits use the live draft count so rapid drops don’t bypass caps. Partial failures surface via toast/thread error instead of stopping the whole batch.
Timeline collapse now measures visible link labels, not long absolute paths in markdown, so short prompts with many attachments aren’t hidden. File tag chips under
.t3/attachments/always show as files, including extensionless names.Reviewed by Cursor Bugbot for commit e38c0b4. Bugbot is set up for automated code reviews on this repo. Configure here.