fix(frontend): gate paste and drag-to-attach on the file-uploads flag - #5604
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe attachment guard now uses ChangesAgent file upload gating
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
| */ | ||
| const composerDisabled = onboardingActive ? ideHandoffActive : modelBlocked | ||
| const attachmentsBlocked = () => voiceRecorder.active || composerDisabled | ||
| const attachmentsBlocked = () => !uploadsEnabled || voiceRecorder.active || composerDisabled |
There was a problem hiding this comment.
The whole functional change is this one added !uploadsEnabled. This guard was already consulted by all four entry points (drag-enter, drag-over, drop, paste), so gating here covers every path with the mechanism the feature already used for "recording in flight" and "composer disabled". The comments above this line explain why the handlers still preventDefault before the guard returns: releasing the drop would let the browser navigate to the file and unload the app.
| nowhere. Tradeoff: a clipboard carrying both a file and meaningful text loses the text. This | ||
| matches the pre-existing blocked-state behavior; the common case (a screenshot) would otherwise | ||
| paste junk markup. | ||
| 3. **The voice-recorder completion path is deliberately not gated by the uploads flag.** A finished |
There was a problem hiding this comment.
This is the one finding worth remembering past this PR: the voice recorder feeds the attachment tray below this guard, gated only by its own flag. Unreachable today (both flags default off), but Stage 2 turns the voice flag on, so Stage 2 must decide whether a finished recording respects the uploads flag.
|
@coderabbitai review |
dosubot
Bot
added
size:M
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/oss/src/components/AgentChatSlice/AgentConversation.tsx (1)
1778-1779: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the changed comments to one short line.
Both updated comment blocks span multiple lines. Condense each block to one short line.
Suggested rewrite
- // Every attach path — button, preview, drive uploads, paste, drag-and-drop — is gated on - // `NEXT_PUBLIC_AGENT_FILE_UPLOADS` until the agent service can deliver file parts. + // Gate every attachment path on NEXT_PUBLIC_AGENT_FILE_UPLOADS until file parts are supported. - {/* Attach button stays dead until the agent service is ready - for inline file parts (`NEXT_PUBLIC_AGENT_FILE_UPLOADS`). */} + {/* Gate the attach button until inline file parts are supported. */}As per coding guidelines, “Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints such as bugs, races, or ordering requirements.”
Also applies to: 2548-2548
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: faa9bfcc-5004-44c7-81fd-e5c0db9cdb78
📒 Files selected for processing (2)
docs/design/agent-workflows/projects/agent-multi-modality/protocols/stage-0.mdweb/oss/src/components/AgentChatSlice/AgentConversation.tsx
Railway Preview Environment
Updated at 2026-08-01T18:12:49.954Z |
a9bbf67 to
ebb22d8
Compare
ebb22d8 to
cb27a03
Compare
cb27a03 to
a652f2a
Compare
|
🤖 The AI agent says: The comment-shortening nitpick was applied in aa1cb47. That change lands one branch up the stack, since wp4 owns the later edits to that file. This PR's diff therefore still shows the original text, but the merged result carries the short form. |
5cd13b4 to
9528c48
Compare
a652f2a to
e424506
Compare
1 participant
Pasting a screenshot or dropping a file into the agent chat attaches it even though the file-uploads feature is dark: the attach button is gated on
NEXT_PUBLIC_AGENT_FILE_UPLOADS, but the paste and drag-and-drop paths predate the flag and are live ungated. Because the backend delivery does not exist yet, the runner flattens the file to a[image]placeholder and the model never sees it, with no warning to the user.Before: with the flag off, paste and drag-and-drop still attach files, which then silently fail at the runner.
After: with the flag off, no path attaches a file. A drop shows the "no drop" cursor and is swallowed (releasing it would let the browser navigate to the file and unload the app); a paste that carries a file inserts nothing, while plain-text paste is untouched. With the flag on, the guard expression is byte-for-byte the old behavior.
The change is one line: the shared
attachmentsBlockedguard, already consulted by every entry point (drag-enter, drag-over, drop, paste), now includes the flag. Two stale comments documenting the old ungated behavior are corrected.This is Stage 0 of the agent multi-modality plan (
docs/design/agent-workflows/projects/agent-multi-modality/plan.md), stacked on the design PR #5439. The stage protocol, including the audit of all other entry points to the attachment state, is in this PR atdocs/design/agent-workflows/projects/agent-multi-modality/protocols/stage-0.md.Implicit decisions and their tradeoffs
Full detail in the protocol file; the two worth your eyes:
Forced routes to double-check
None. Nothing here was a single-option path; the guard mechanism already existed and the change extends it.
QA
pnpm lint-fixand the fast typecheck (tsgo --noEmit) pass. Live QA ran on the dev stack in both flag states, with synthetic paste and drag events dispatched at the real composer (full record in the protocol file in this PR):https://claude.ai/code/session_01A1XQVjHPYJgVBHWSNUphtx