feat(desktop): support files and images in mastra chat#2058
Merged
Conversation
Replace images-only attachment API with files support for arbitrary file types (images, PDFs, text). Files are optimistically uploaded to Vercel Blob storage, then sent as file parts to the mastra harness. Key changes: - Add useOptimisticUpload hook and uploadFiles utility for blob uploads - Update ChatMastraInterface with MastraUploadFooter component - Add renderAttachment prop with loading spinners to ChatInputFooter - Rewrite UserMessage/AssistantMessage with unified AttachmentChip - Update zod schema from images/mimeType to files/mediaType - Add file-aware optimistic message building - Bump mastracode to v0.4.0-superset.12 with files harness support - Bump @mastra/core to 1.8.0-superset.1 and @mastra/memory to 1.5.2-superset.1 from superset-sh/mastra fork
Contributor
|
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:
📝 WalkthroughWalkthroughAdd file-attachment support with optimistic uploads: new upload utilities and hook, payload/schema changes from images→files, UI components for attachment rendering and upload state, optimistic message tracking updates, Electron externals update, and tabs file-viewer displayName support. Changes
Sequence DiagramsequenceDiagram
actor User
participant ChatUI as ChatMastraInterface
participant Hook as useOptimisticUpload
participant Uploader as uploadFiles
participant Server as ServerAPI
participant Viewer as MessageDisplay
User->>ChatUI: Attach file(s)
ChatUI->>Hook: provide attachmentFiles, sessionId
activate Hook
Hook->>Uploader: upload new attachment(s)
Uploader->>Server: POST /api/chat/{sessionId}/attachments (FormData)
Server-->>Uploader: uploaded FileUIPart(s)
Uploader-->>Hook: return uploaded file info
Hook-->>ChatUI: update uploaded state
deactivate Hook
User->>ChatUI: Submit message (text + files)
ChatUI->>Server: sendMessage payload (files + content)
Server-->>ChatUI: message created
ChatUI->>Viewer: render message with AttachmentChip(s)
Viewer-->>User: display attachments (open/download)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 19 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx:815">
P1: Unhandled promise rejection: `handleSend` re-throws errors from its catch block, but this `onSend` callback uses `void` without `.catch()`. In Electron, unhandled rejections can crash the renderer process. The slash command handler already does this correctly — apply the same pattern here.
(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts:93">
P2: Including `entries` in the `useEffect` dependency array creates an update loop: the effect calls `setEntries` → `entries` changes → effect re-triggers. While `inflightRef` prevents duplicate uploads, the effect still needlessly re-runs 2–3× per file.
Fix: stop removing IDs from `inflightRef` (so it becomes a "processed" set) and drop `entries` from the deps. This lets the `inflightRef.current.has(file.id)` guard prevent re-processing without needing to read `entries` inside the effect.
(Based on your team's feedback about narrowing React effect dependencies to required fields.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/components/ChatComposerControls/ChatComposerControls.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/components/ChatComposerControls/ChatComposerControls.tsx:74">
P2: The `disabled` prop is applied unconditionally, which would prevent the stop button from working if `canAbort` and `submitDisabled` are both true. Guard the `disabled` prop so it only takes effect when the button is in submit mode, not abort mode.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx:30">
P2: Duplicated `AttachmentChip` component: this is defined identically in both `AssistantMessage.tsx` and `UserMessage.tsx`. Extract it into a shared module (e.g., a `components/AttachmentChip.tsx` sibling) so both message types import the same component. This avoids drift and keeps changes centralized.
(Based on your team's feedback about avoiding duplicating business logic in multiple components.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx:178">
P2: `AttachmentChip` renders a `<button>` but no `onClick` is passed here, making it a non-functional interactive element. Either wire an `onClick` handler (e.g., to open or preview the file, like `UserMessage` does) or render a non-interactive element (`<div>`) when no click action is available.</violation>
</file>
<file name="packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts">
<violation number="1" location="packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts:152">
P1: Bug: File-only optimistic message is cleared prematurely when history already contains file/image messages. The fallback branch matches ANY historical user message with a file or image part, not specifically the newly sent one. If the user has previously sent a file/image in this session, `found` will immediately be `true` on the next poll, clearing the optimistic message before the server processes the new one.
Consider tracking the count of file-bearing user messages at send time and comparing against the current count, or matching on a more specific attribute (e.g., comparing exact file data/URLs or checking that the message count increased).</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx:111">
P1: `navigator.clipboard.writeText` returns a Promise that is neither awaited nor caught. If it rejects (e.g. permission denied), this produces an unhandled promise rejection—which can crash in Electron. Also, `setCopied(true)` fires optimistically even if the write fails, showing a false success indicator.
(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx:168">
P2: Unstable React key: using `segment.value.slice(0, 20)` as key material will produce duplicate keys if two text segments share the same prefix. Use the segment index instead for guaranteed uniqueness.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (segment.type === "text") { | ||
| return ( | ||
| <span | ||
| key={`${key}-text-${segment.value.slice(0, 20)}`} |
Contributor
There was a problem hiding this comment.
P2: Unstable React key: using segment.value.slice(0, 20) as key material will produce duplicate keys if two text segments share the same prefix. Use the segment index instead for guaranteed uniqueness.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx, line 168:
<comment>Unstable React key: using `segment.value.slice(0, 20)` as key material will produce duplicate keys if two text segments share the same prefix. Use the segment index instead for guaranteed uniqueness.</comment>
<file context>
@@ -1,100 +1,212 @@
+ if (segment.type === "text") {
+ return (
+ <span
+ key={`${key}-text-${segment.value.slice(0, 20)}`}
+ className="whitespace-pre-wrap break-words"
+ >
</file context>
When opening a chat attachment in a file viewer pane, pass the original filename as displayName so the tab shows the real name instead of the blob URL hash.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx (1)
30-62: ExtractAttachmentChipinto a shared component module.This component is duplicated with
UserMessage.tsx; extracting it avoids behavior drift and aligns with component-file boundaries.As per coding guidelines
**/{components,pages}/**/*.{tsx,ts}: No multi-component files - maintain one component per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx` around lines 30 - 62, Extract the duplicated AttachmentChip component into a new shared component file and import it from both AssistantMessage and UserMessage to avoid duplication: create a new component module that exports AttachmentChip (preserving its props signature: data, mediaType, filename?, onClick?) and its exact rendering/behavior (including isImage logic, label calculation, and the same className structure), then update the original files to remove the inline AttachmentChip implementation and import the shared AttachmentChip instead; ensure the new file lives under the components directory per guidelines and export default or named export consistent with your project imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`:
- Around line 77-107: handleSend currently swallows the async send
(fire-and-forget) causing PromptInput to treat submit as synchronous; update the
send chain to return the send promise so submit lifecycle waits for completion:
change the onSend type to return Promise<void> (or the actual Promise type
used), update handleSend (in ChatMastraInterface) to return the Promise from
onSend instead of void, and propagate that return through
MastraUploadFooter/ChatInputFooter so their onSend handlers also return the same
Promise; ensure PromptInput/PromptInputMessage submission receives and awaits
that Promise.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`:
- Around line 45-49: The AssistantMessage component currently always renders an
interactive <button> even when onClick is undefined; change the rendering so
that the element is only a clickable <button> with the provided onClick handler
when onClick exists, and otherwise render a non-interactive element (e.g., a
<div> or <span> with the same styling) to avoid a focusable no-op control;
update the conditional around the element in AssistantMessage to switch between
the interactive button (with onClick) and a semantic non-interactive element
when onClick is absent.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 109-114: The handleCopy callback currently calls
navigator.clipboard.writeText without handling rejection and sets
setCopied(true) unconditionally; update the handleCopy function to await the
writeText promise inside a try/catch (or use .then/.catch), only call
setCopied(true) and start the timeout when the clipboard write succeeds, and
handle failures by logging or showing an error state (e.g., do not set copied
and optionally set an error flag). Reference: handleCopy,
navigator.clipboard.writeText, setCopied.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts`:
- Around line 37-93: The effect can apply stale uploads when sessionId changes;
fix by resetting optimistic state at the start of the effect and guarding
promise handlers: clear inflightRef.current and reset setEntries to an empty Map
whenever sessionId changes, capture const currentSession = sessionId before
starting each upload, and in the uploadFiles .then/.catch handlers verify the
live sessionId === currentSession before mutating inflightRef, calling
setEntries, removeAttachment or onError; reference inflightRef, setEntries,
uploadFiles, removeAttachment and onError to locate the changes.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/optimisticUserMessage/optimisticUserMessage.ts`:
- Around line 18-25: The content array in the optimistic user message builder
currently places text before file parts, causing order-sensitive signature
mismatches with hasMatchingUserMessage; update the two content constructions in
optimisticUserMessage (the arrays around the spreads at the first occurrence and
the later block) to emit file entries before the optional text entry so
files.map(...) is spread first and then ...(text ? [{ type: "text", text }] :
[]), matching the ordering used by the display hook (use-mastra-chat-display) to
ensure reconciliation succeeds.
In
`@packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts`:
- Around line 141-160: The optimistic reconciliation currently treats any
historical user message that contains at least one attachment as a match,
clearing optimistics for file-only sends; update the logic around
optimisticText/historicalMessages used to compute found so that when
optimisticText is falsy you only consider historical user messages whose entire
content is attachments (i.e., all parts have type "file" or "image" and none are
text) — change the inner predicate on historicalMessages (used in the second
branch) to require message.content.every(...) for HistoryMessagePart types
instead of message.content.some(...), referencing the optimisticText variable,
the found constant, and the HistoryMessage/HistoryMessagePart types.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`:
- Around line 30-62: Extract the duplicated AttachmentChip component into a new
shared component file and import it from both AssistantMessage and UserMessage
to avoid duplication: create a new component module that exports AttachmentChip
(preserving its props signature: data, mediaType, filename?, onClick?) and its
exact rendering/behavior (including isImage logic, label calculation, and the
same className structure), then update the original files to remove the inline
AttachmentChip implementation and import the shared AttachmentChip instead;
ensure the new file lives under the components directory per guidelines and
export default or named export consistent with your project imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e372de1-888f-4f66-a58a-c229fc9b46b4
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
apps/desktop/electron.vite.config.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/optimisticUserMessage/optimisticUserMessage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/sendMessage/sendMessage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/toMastraImages/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/toMastraImages/toMastraImages.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/uploadFiles/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/uploadFiles/uploadFiles.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/components/ChatComposerControls/ChatComposerControls.tsxpackage.jsonpackages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.tspackages/chat-mastra/src/server/trpc/zod.tspackages/ui/src/components/ai-elements/prompt-input.tsx
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/toMastraImages/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/utils/toMastraImages/toMastraImages.ts
Comment on lines
+77
to
+107
| onSend: (payload: { content: string; files?: HarnessFilePayload[] }) => void; | ||
| } & Omit< | ||
| React.ComponentProps<typeof ChatInputFooter>, | ||
| "onSend" | "submitDisabled" | "renderAttachment" | ||
| >) { | ||
| const attachments = useProviderAttachments(); | ||
| const { getUploadedFiles, isUploading, entries } = useOptimisticUpload({ | ||
| sessionId, | ||
| attachmentFiles: attachments.files, | ||
| removeAttachment: attachments.remove, | ||
| onError, | ||
| }); | ||
|
|
||
| const handleSend = useCallback( | ||
| (message: PromptInputMessage) => { | ||
| const { ready, files: uploadedFiles } = getUploadedFiles(); | ||
| if (!ready) return; | ||
|
|
||
| const files: HarnessFilePayload[] = uploadedFiles.map((file) => ({ | ||
| data: file.url, | ||
| mediaType: file.mediaType, | ||
| filename: file.filename, | ||
| })); | ||
|
|
||
| onSend({ | ||
| content: message.text, | ||
| files: files.length > 0 ? files : undefined, | ||
| }); | ||
| }, | ||
| [getUploadedFiles, onSend], | ||
| ); |
Contributor
There was a problem hiding this comment.
Return the send promise through MastraUploadFooter instead of fire-and-forget.
The current void chain makes PromptInput treat submit as synchronous, so submit lifecycle callbacks can complete before the real send finishes.
✅ Suggested fix
function MastraUploadFooter({
sessionId,
onError,
onSend,
@@
}: {
sessionId: string | null;
onError: (message: string) => void;
- onSend: (payload: { content: string; files?: HarnessFilePayload[] }) => void;
+ onSend: (
+ payload: { content: string; files?: HarnessFilePayload[] },
+ ) => Promise<void> | void;
} & Omit<
@@
const handleSend = useCallback(
(message: PromptInputMessage) => {
@@
- onSend({
+ return onSend({
content: message.text,
files: files.length > 0 ? files : undefined,
});
},
[getUploadedFiles, onSend],
);
@@
- onSend={(sendPayload) => {
- void handleSend(sendPayload);
- }}
+ onSend={(sendPayload) => handleSend(sendPayload)}Also applies to: 814-816
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`
around lines 77 - 107, handleSend currently swallows the async send
(fire-and-forget) causing PromptInput to treat submit as synchronous; update the
send chain to return the send promise so submit lifecycle waits for completion:
change the onSend type to return Promise<void> (or the actual Promise type
used), update handleSend (in ChatMastraInterface) to return the Promise from
onSend instead of void, and propagate that return through
MastraUploadFooter/ChatInputFooter so their onSend handlers also return the same
Promise; ensure PromptInput/PromptInputMessage submission receives and awaits
that Promise.
Comment on lines
+37
to
+93
| useEffect(() => { | ||
| if (!sessionId) return; | ||
|
|
||
| for (const file of attachmentFiles) { | ||
| if (entries.has(file.id) || inflightRef.current.has(file.id)) continue; | ||
|
|
||
| inflightRef.current.add(file.id); | ||
| setEntries((prev) => { | ||
| const next = new Map(prev); | ||
| next.set(file.id, { uploaded: null, error: null, uploading: true }); | ||
| return next; | ||
| }); | ||
|
|
||
| uploadFiles(sessionId, [file]) | ||
| .then(([uploaded]) => { | ||
| inflightRef.current.delete(file.id); | ||
| setEntries((prev) => { | ||
| const next = new Map(prev); | ||
| next.set(file.id, { | ||
| uploaded: uploaded ?? null, | ||
| error: null, | ||
| uploading: false, | ||
| }); | ||
| return next; | ||
| }); | ||
| }) | ||
| .catch((err: unknown) => { | ||
| inflightRef.current.delete(file.id); | ||
| const message = err instanceof Error ? err.message : "Upload failed"; | ||
| setEntries((prev) => { | ||
| const next = new Map(prev); | ||
| next.set(file.id, { | ||
| uploaded: null, | ||
| error: message, | ||
| uploading: false, | ||
| }); | ||
| return next; | ||
| }); | ||
| removeAttachment(file.id); | ||
| onError?.(message); | ||
| }); | ||
| } | ||
|
|
||
| // Clean up entries for removed attachments | ||
| const currentIds = new Set(attachmentFiles.map((f) => f.id)); | ||
| setEntries((prev) => { | ||
| let changed = false; | ||
| const next = new Map(prev); | ||
| for (const id of next.keys()) { | ||
| if (!currentIds.has(id)) { | ||
| next.delete(id); | ||
| changed = true; | ||
| } | ||
| } | ||
| return changed ? next : prev; | ||
| }); | ||
| }, [attachmentFiles, sessionId, entries, removeAttachment, onError]); |
Contributor
There was a problem hiding this comment.
Reset/guard upload state on sessionId changes to prevent stale cross-session results.
Line 41 dedupes by file.id only. If the session changes, prior entries can suppress re-upload and old in-flight promises can still commit results into the new session state.
🔧 Proposed fix
import { useCallback, useEffect, useRef, useState } from "react";
@@
const inflightRef = useRef<Set<AttachmentId>>(new Set());
+ const sessionEpochRef = useRef(0);
+
+ useEffect(() => {
+ sessionEpochRef.current += 1;
+ inflightRef.current.clear();
+ setEntries(new Map());
+ }, [sessionId]);
useEffect(() => {
if (!sessionId) return;
+ const epoch = sessionEpochRef.current;
@@
uploadFiles(sessionId, [file])
.then(([uploaded]) => {
+ if (epoch !== sessionEpochRef.current) return;
inflightRef.current.delete(file.id);
@@
})
.catch((err: unknown) => {
+ if (epoch !== sessionEpochRef.current) return;
inflightRef.current.delete(file.id);
@@
- }, [attachmentFiles, sessionId, entries, removeAttachment, onError]);
+ }, [attachmentFiles, sessionId, entries, removeAttachment, onError]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts`
around lines 37 - 93, The effect can apply stale uploads when sessionId changes;
fix by resetting optimistic state at the start of the effect and guarding
promise handlers: clear inflightRef.current and reset setEntries to an empty Map
whenever sessionId changes, capture const currentSession = sessionId before
starting each upload, and in the uploadFiles .then/.catch handlers verify the
live sessionId === currentSession before mutating inflightRef, calling
setEntries, removeAttachment or onError; reference inflightRef, setEntries,
uploadFiles, removeAttachment and onError to locate the changes.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx (1)
113-118:⚠️ Potential issue | 🟡 MinorHandle clipboard write failures before setting copied state.
Line 115 uses
navigator.clipboard.writeTextwithout awaiting/catch, but Lines 116-117 always show success. This can show a false “copied” state when clipboard permission/context rejects.Proposed fix
- const handleCopy = useCallback(() => { + const handleCopy = useCallback(async () => { if (!fullText) return; - navigator.clipboard.writeText(fullText); - setCopied(true); - setTimeout(() => setCopied(false), 1500); + try { + await navigator.clipboard.writeText(fullText); + setCopied(true); + setTimeout(() => setCopied(false), 1500); + } catch { + setCopied(false); + } }, [fullText]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 113 - 118, The handleCopy callback uses navigator.clipboard.writeText(fullText) without awaiting or catching failures, causing setCopied(true) to run even on rejection; update the handleCopy function to await navigator.clipboard.writeText(fullText) inside a try/catch (or use .then/.catch) and only call setCopied(true) and the timeout on success, while handling errors in the catch (e.g., optional fallback copy method or logging) so the copied state reflects actual success.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/stores/tabs/utils.ts (1)
219-223: Consolidate display-name derivation into a shared helper.Line 219–223 duplicates logic also used in
apps/desktop/src/renderer/stores/tabs/store.ts(Line 744–746). Extracting one helper avoids future drift between creation vs. preview-replacement paths.♻️ Proposed refactor
--- a/apps/desktop/src/renderer/stores/tabs/utils.ts +++ b/apps/desktop/src/renderer/stores/tabs/utils.ts @@ +export const resolveFileViewerDisplayName = ({ + filePath, + displayName, +}: { + filePath: string; + displayName?: string; +}): string => displayName || filePath.split("/").pop() || filePath; @@ - const fileName = - options.displayName || - options.filePath.split("/").pop() || - options.filePath; + const fileName = resolveFileViewerDisplayName({ + filePath: options.filePath, + displayName: options.displayName, + });--- a/apps/desktop/src/renderer/stores/tabs/store.ts +++ b/apps/desktop/src/renderer/stores/tabs/store.ts @@ resolveActiveTabIdForWorkspace, + resolveFileViewerDisplayName, resolveFileViewerMode, } from "./utils"; @@ - const fileName = - options.displayName || - options.filePath.split("/").pop() || - options.filePath; + const fileName = resolveFileViewerDisplayName({ + filePath: options.filePath, + displayName: options.displayName, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/tabs/utils.ts` around lines 219 - 223, The display-name derivation (using options.displayName or falling back to the filename from options.filePath) is duplicated between utils.ts and store.ts; extract that logic into a single helper (e.g. deriveDisplayName or getDisplayNameForFile) and replace the inline expression that computes fileName in utils.ts (the variable assigned from options.displayName || options.filePath.split("/").pop() || options.filePath) and the matching logic in store.ts to call the new helper, ensuring both creation and preview-replacement paths use the same centralized function.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx (1)
87-107: Harden message-part parsing to avoid renderer crashes on malformed payloads.Line 87 force-casts
message.content, and Lines 91-104 assume object fields are present/typed. Add runtime narrowing (Array.isArray, object/type checks) before accessingpart.type,part.text,part.data, etc.Proposed defensive parsing refactor
- const parts = message.content as Record<string, unknown>[]; + const parts = Array.isArray(message.content) ? message.content : []; for (let i = 0; i < parts.length; i++) { - const part = parts[i]; + const part = parts[i]; + if (!part || typeof part !== "object") continue; + const safePart = part as Record<string, unknown>; const key = `${message.id}-${i}`; - if (part.type === "text") { - textParts.push({ key, text: part.text as string }); - } else if (part.type === "file" || part.type === "image") { + if (safePart.type === "text" && typeof safePart.text === "string") { + textParts.push({ key, text: safePart.text }); + } else if (safePart.type === "file" || safePart.type === "image") { const mime = - (part.mediaType as string) || - (part.mimeType as string) || + (typeof safePart.mediaType === "string" ? safePart.mediaType : "") || + (typeof safePart.mimeType === "string" ? safePart.mimeType : "") || "application/octet-stream"; - const data = (part.data as string) || (part.image as string) || ""; + const data = + (typeof safePart.data === "string" ? safePart.data : "") || + (typeof safePart.image === "string" ? safePart.image : "") || + ""; if (data) { attachments.push({ key, data, mediaType: mime, - filename: part.filename as string | undefined, + filename: + typeof safePart.filename === "string" + ? safePart.filename + : undefined, }); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 87 - 107, The code unsafely assumes message.content is an array and each part is a well-formed object; update the parsing in UserMessage.tsx (the parts variable and the loop that fills textParts and attachments) to first guard with Array.isArray(message.content) and bail or treat as single-text when not an array, then inside the loop check each part is a non-null object before accessing fields, narrow part.type with typeof or 'type' in part checks, safely read part.text/part.data/part.image/part.mediaType/part.mimeType/part.filename only after validating types (string) and only push to textParts or attachments when the validated values exist; ensure malformed parts are skipped without throwing to avoid renderer crashes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 19-52: Move the AttachmentChip JSX out of UserMessage.tsx into its
own component file (e.g., create AttachmentChip.tsx under a components folder),
export it (default or named) and keep the same props signature ({ data,
mediaType, filename?, onClick? }) and behavior (isImage check,
img/FileIcon/FileTextIcon rendering and classes). Replace the inline
AttachmentChip definition in UserMessage.tsx with an import of the new
AttachmentChip component and ensure the import/export names match; run a quick
typecheck to confirm props types are correctly exported/imported.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 113-118: The handleCopy callback uses
navigator.clipboard.writeText(fullText) without awaiting or catching failures,
causing setCopied(true) to run even on rejection; update the handleCopy function
to await navigator.clipboard.writeText(fullText) inside a try/catch (or use
.then/.catch) and only call setCopied(true) and the timeout on success, while
handling errors in the catch (e.g., optional fallback copy method or logging) so
the copied state reflects actual success.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 87-107: The code unsafely assumes message.content is an array and
each part is a well-formed object; update the parsing in UserMessage.tsx (the
parts variable and the loop that fills textParts and attachments) to first guard
with Array.isArray(message.content) and bail or treat as single-text when not an
array, then inside the loop check each part is a non-null object before
accessing fields, narrow part.type with typeof or 'type' in part checks, safely
read part.text/part.data/part.image/part.mediaType/part.mimeType/part.filename
only after validating types (string) and only push to textParts or attachments
when the validated values exist; ensure malformed parts are skipped without
throwing to avoid renderer crashes.
In `@apps/desktop/src/renderer/stores/tabs/utils.ts`:
- Around line 219-223: The display-name derivation (using options.displayName or
falling back to the filename from options.filePath) is duplicated between
utils.ts and store.ts; extract that logic into a single helper (e.g.
deriveDisplayName or getDisplayNameForFile) and replace the inline expression
that computes fileName in utils.ts (the variable assigned from
options.displayName || options.filePath.split("/").pop() || options.filePath)
and the matching logic in store.ts to call the new helper, ensuring both
creation and preview-replacement paths use the same centralized function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82c5760b-1592-49c6-9014-2ae780f2d191
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/utils.ts
This reverts commit b80b7e3.
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/stores/tabs/utils.ts">
<violation number="1">
P2: `filePath.split("/").pop()` won't extract the filename on Windows where paths use backslashes. Consider splitting on both separators (e.g. `/[/\\]/`) or using a cross-platform basename utility.
(Based on your team's feedback about using cross-platform path utilities instead of split.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
This reverts commit 358b115.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx (2)
19-51: 🛠️ Refactor suggestion | 🟠 MajorExtract
AttachmentChipinto its own component file.This file contains two components (
AttachmentChipandUserMessage), violating the single-component-per-file guideline. MoveAttachmentChipto its own folder/file (e.g.,AttachmentChip/AttachmentChip.tsxwith anindex.tsbarrel export) and import it here.As per coding guidelines: "
**/{components,pages}/**/*.{tsx,ts}: No multi-component files - maintain one component per file".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 19 - 51, Extract the AttachmentChip function into its own component file and update imports: create a new folder/component file (e.g., AttachmentChip/AttachmentChip.tsx) that exports the AttachmentChip component (and add an index.ts barrel export), move the AttachmentChip implementation there (preserve props {data, mediaType, filename?, onClick?} and the JSX/logic including isImage and label computation), then replace the inline AttachmentChip in UserMessage.tsx by importing AttachmentChip from its new module; ensure existing names (AttachmentChip, UserMessage) and props remain unchanged so usage in UserMessage continues to work.
109-114:⚠️ Potential issue | 🟡 MinorHandle clipboard errors before showing success state.
navigator.clipboard.writeTextreturns a Promise that can reject (e.g., if clipboard access is denied). The current code setscopied=trueunconditionally without awaiting or catching errors.🛠️ Suggested fix
- const handleCopy = useCallback(() => { + const handleCopy = useCallback(async () => { if (!fullText) return; - navigator.clipboard.writeText(fullText); - setCopied(true); - setTimeout(() => setCopied(false), 1500); + try { + await navigator.clipboard.writeText(fullText); + setCopied(true); + setTimeout(() => setCopied(false), 1500); + } catch { + // Clipboard write failed - don't show success state + } }, [fullText]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 109 - 114, The handleCopy callback currently calls navigator.clipboard.writeText(fullText) and immediately sets setCopied(true) without awaiting or handling failures; update the handleCopy function to await the Promise from navigator.clipboard.writeText(fullText) inside a try/catch (or use .then/.catch), only call setCopied(true) on successful resolution, handle rejection by optionally logging or setting an error state, and ensure the existing setTimeout to clear copied remains tied to successful copy; reference the handleCopy useCallback, fullText, navigator.clipboard.writeText, and setCopied when making the change.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx (2)
60-66: Consider renaminghandleImageClickfor clarity.The function
handleImageClickis now used for all attachment types (images, PDFs, text files), not just images. A more descriptive name likehandleAttachmentClickoropenAttachmentwould better reflect its purpose.♻️ Suggested rename
- const handleImageClick = useCallback( + const handleAttachmentClick = useCallback( (url: string) => { if (!workspaceId) return; addFileViewerPane(workspaceId, { filePath: url, isPinned: true }); }, [workspaceId, addFileViewerPane], );And update the usage:
- onClick={() => handleImageClick(att.data)} + onClick={() => handleAttachmentClick(att.data)}Also applies to: 154-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 60 - 66, The function name handleImageClick is misleading because it's used for all attachment types; rename it to handleAttachmentClick (or openAttachment) and update all references (including the usage at the other occurrence in this file) to the new name; specifically rename the callback declared as handleImageClick and replace its usages where addFileViewerPane(workspaceId, { filePath: url, isPinned: true }) is invoked so the hook dependencies ([workspaceId, addFileViewerPane]) remain correct and tests/consumers refer to handleAttachmentClick.
164-173: Use index-based keys instead of content-derived keys.The key
${key}-text-${segment.value.slice(0, 20)}can cause collisions if multiple text segments share the same first 20 characters, leading to React reconciliation issues.♻️ Suggested fix
- {mentionSegments.map((segment) => { + {mentionSegments.map((segment, segmentIndex) => { if (segment.type === "text") { return ( <span - key={`${key}-text-${segment.value.slice(0, 20)}`} + key={`${key}-text-${segmentIndex}`} className="whitespace-pre-wrap break-words" > {segment.value} </span> ); }And similarly for the mention button key at line 185:
- key={`${key}-mention-${segment.relativePath}`} + key={`${key}-mention-${segmentIndex}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 164 - 173, The keys for the mentionSegments.map items are derived from segment.value which can collide; change the mapping to use the array index (or a stable id) for keys instead of content: update the mentionSegments.map callback in UserMessage.tsx (the arrow function that checks segment.type === "text") to accept the index and set the span key to a stable index-based value (e.g., `${key}-text-${index}`), and apply the same change for the mention button key (the button rendered for mention segments) so both keys use the index or another stable identifier rather than segment.value.slice(0,20).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 83-84: The code in UserMessage (UserMessage.tsx) assumes
message.content is an array by casting to Record<string, unknown>[] and
iterating; add a defensive check using Array.isArray(message.content) (or coerce
with const parts = Array.isArray(message.content) ? message.content : []) before
the for-loop so the loop never runs on undefined/null/non-array values, and
adjust any downstream usage of parts accordingly; this ensures safe iteration
without changing the intended behavior.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 19-51: Extract the AttachmentChip function into its own component
file and update imports: create a new folder/component file (e.g.,
AttachmentChip/AttachmentChip.tsx) that exports the AttachmentChip component
(and add an index.ts barrel export), move the AttachmentChip implementation
there (preserve props {data, mediaType, filename?, onClick?} and the JSX/logic
including isImage and label computation), then replace the inline AttachmentChip
in UserMessage.tsx by importing AttachmentChip from its new module; ensure
existing names (AttachmentChip, UserMessage) and props remain unchanged so usage
in UserMessage continues to work.
- Around line 109-114: The handleCopy callback currently calls
navigator.clipboard.writeText(fullText) and immediately sets setCopied(true)
without awaiting or handling failures; update the handleCopy function to await
the Promise from navigator.clipboard.writeText(fullText) inside a try/catch (or
use .then/.catch), only call setCopied(true) on successful resolution, handle
rejection by optionally logging or setting an error state, and ensure the
existing setTimeout to clear copied remains tied to successful copy; reference
the handleCopy useCallback, fullText, navigator.clipboard.writeText, and
setCopied when making the change.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 60-66: The function name handleImageClick is misleading because
it's used for all attachment types; rename it to handleAttachmentClick (or
openAttachment) and update all references (including the usage at the other
occurrence in this file) to the new name; specifically rename the callback
declared as handleImageClick and replace its usages where
addFileViewerPane(workspaceId, { filePath: url, isPinned: true }) is invoked so
the hook dependencies ([workspaceId, addFileViewerPane]) remain correct and
tests/consumers refer to handleAttachmentClick.
- Around line 164-173: The keys for the mentionSegments.map items are derived
from segment.value which can collide; change the mapping to use the array index
(or a stable id) for keys instead of content: update the mentionSegments.map
callback in UserMessage.tsx (the arrow function that checks segment.type ===
"text") to accept the index and set the span key to a stable index-based value
(e.g., `${key}-text-${index}`), and apply the same change for the mention button
key (the button rendered for mention segments) so both keys use the index or
another stable identifier rather than segment.value.slice(0,20).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a898ad3f-3f37-44a3-9d21-dcc1257079f9
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx
Comment on lines
+83
to
+84
| const parts = message.content as Record<string, unknown>[]; | ||
| for (let i = 0; i < parts.length; i++) { |
Contributor
There was a problem hiding this comment.
Add defensive check for message.content before iteration.
The cast as Record<string, unknown>[] assumes message.content is always an array. If it's undefined, null, or a different shape, the loop will throw at runtime.
🛡️ Proposed fix
- const parts = message.content as Record<string, unknown>[];
- for (let i = 0; i < parts.length; i++) {
+ const parts = Array.isArray(message.content)
+ ? (message.content as Record<string, unknown>[])
+ : [];
+ for (let i = 0; i < parts.length; i++) {📝 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
| const parts = message.content as Record<string, unknown>[]; | |
| for (let i = 0; i < parts.length; i++) { | |
| const parts = Array.isArray(message.content) | |
| ? (message.content as Record<string, unknown>[]) | |
| : []; | |
| for (let i = 0; i < parts.length; i++) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`
around lines 83 - 84, The code in UserMessage (UserMessage.tsx) assumes
message.content is an array by casting to Record<string, unknown>[] and
iterating; add a defensive check using Array.isArray(message.content) (or coerce
with const parts = Array.isArray(message.content) ? message.content : []) before
the for-loop so the loop never runs on undefined/null/non-array values, and
adjust any downstream usage of parts accordingly; this ensures safe iteration
without changing the intended behavior.
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx:128">
P2: Non-image `AttachmentChip` renders as a `<button>` but never receives `onClick`, so clicking PDF/text attachments does nothing. Pass `handleAttachmentClick` here for consistency with the image click behavior.</violation>
</file>
<file name="apps/desktop/src/renderer/stores/tabs/store.ts">
<violation number="1" location="apps/desktop/src/renderer/stores/tabs/store.ts:745">
P2: The fallback `filePath.split("/").pop()` won't extract the correct basename on Windows paths that use backslashes. Consider using Node's `path.basename()` (or `path.posix`/`path.win32` as appropriate) for cross-platform correctness.
(Based on your team's feedback about using cross-platform path utilities instead of split.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const fileName = | ||
| options.filePath.split("/").pop() || options.filePath; | ||
| options.displayName || | ||
| options.filePath.split("/").pop() || |
Contributor
There was a problem hiding this comment.
P2: The fallback filePath.split("/").pop() won't extract the correct basename on Windows paths that use backslashes. Consider using Node's path.basename() (or path.posix/path.win32 as appropriate) for cross-platform correctness.
(Based on your team's feedback about using cross-platform path utilities instead of split.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/stores/tabs/store.ts, line 745:
<comment>The fallback `filePath.split("/").pop()` won't extract the correct basename on Windows paths that use backslashes. Consider using Node's `path.basename()` (or `path.posix`/`path.win32` as appropriate) for cross-platform correctness.
(Based on your team's feedback about using cross-platform path utilities instead of split.) </comment>
<file context>
@@ -741,7 +741,9 @@ export const useTabsStore = create<TabsStore>()(
const fileName =
- options.filePath.split("/").pop() || options.filePath;
+ options.displayName ||
+ options.filePath.split("/").pop() ||
+ options.filePath;
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx (3)
87-88:⚠️ Potential issue | 🟡 MinorAdd an array guard before iterating
message.content.Line 87 assumes an array shape; if it’s
null/non-array, the loop will throw.Proposed fix
-const parts = message.content as Record<string, unknown>[]; +const parts = Array.isArray(message.content) + ? (message.content as Record<string, unknown>[]) + : [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 87 - 88, The loop in UserMessage (where parts is assigned from message.content and iterated) assumes message.content is an array; add a guard to ensure message.content is an array before iterating: validate with Array.isArray(message.content) (or coerce to an empty array) when assigning parts, then iterate parts safely; update the variable assigned at "const parts = message.content as Record<string, unknown>[];" and the for-loop that follows in the UserMessage component so null/non-array content won't throw.
113-118:⚠️ Potential issue | 🟡 MinorHandle clipboard write failures before setting success state.
Line 115 can reject, but
copiedis set totrueregardless.Proposed fix
-const handleCopy = useCallback(() => { +const handleCopy = useCallback(async () => { if (!fullText) return; - navigator.clipboard.writeText(fullText); - setCopied(true); - setTimeout(() => setCopied(false), 1500); + try { + await navigator.clipboard.writeText(fullText); + setCopied(true); + setTimeout(() => setCopied(false), 1500); + } catch { + setCopied(false); + } }, [fullText]);#!/bin/bash rg -n -C2 'handleCopy|navigator\.clipboard\.writeText|setCopied\(' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 113 - 118, The handleCopy callback currently calls navigator.clipboard.writeText(fullText) without awaiting or catching failures and sets setCopied(true) unconditionally; change handleCopy (the function named handleCopy in UserMessage.tsx) to await the writeText promise (or use .then/.catch), only call setCopied(true) after a successful write, and handle rejections by not setting copied or by setting an error state/logging (and ensure the timeout to clear copied runs only on success). Keep fullText in the dependency array and preserve the existing timeout behavior but only trigger it after a confirmed successful clipboard write.
19-51: 🛠️ Refactor suggestion | 🟠 MajorExtract
AttachmentChipinto its own component file/folder.
UserMessage.tsxstill contains two components (AttachmentChipandUserMessage). Please moveAttachmentChipto its own component folder and import it here.Proposed refactor (minimal)
+import { AttachmentChip } from "./AttachmentChip"; ... -function AttachmentChip({ - data, - mediaType, - filename, - onClick, -}: { - data: string; - mediaType: string; - filename?: string; - onClick?: () => void; -}) { - ... -}As per coding guidelines "
**/{components,pages}/**/*.{tsx,ts}: No multi-component files - maintain one component per file".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx` around lines 19 - 51, The file contains two components; extract the AttachmentChip function into its own component file/folder (e.g., create a new AttachmentChip component that exports the component as default), move its props type signature with the mediaType/data/filename/onClick definitions into that new file, update UserMessage to import AttachmentChip and remove the local AttachmentChip definition, and ensure any used symbols (AttachmentChip, UserMessage, FileIcon, FileTextIcon) still have the necessary imports/exports and types updated so the project compiles.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
743-746: Avoid duplicating file-display-name resolution logic.This fallback chain is correct, but it now duplicates
createFileViewerPanebehavior. Extracting one shared helper will prevent drift.♻️ Proposed refactor
diff --git a/apps/desktop/src/renderer/stores/tabs/utils.ts b/apps/desktop/src/renderer/stores/tabs/utils.ts @@ export interface CreateFileViewerPaneOptions { @@ displayName?: string; } + +export const resolveFileViewerDisplayName = ({ + filePath, + displayName, +}: Pick<CreateFileViewerPaneOptions, "filePath" | "displayName">): string => + displayName || filePath.split("/").pop() || filePath; @@ - // Use displayName override, or fall back to filename from path - const fileName = - options.displayName || - options.filePath.split("/").pop() || - options.filePath; + const fileName = resolveFileViewerDisplayName(options); diff --git a/apps/desktop/src/renderer/stores/tabs/store.ts b/apps/desktop/src/renderer/stores/tabs/store.ts @@ resolveActiveTabIdForWorkspace, + resolveFileViewerDisplayName, resolveFileViewerMode, } from "./utils"; @@ - const fileName = - options.displayName || - options.filePath.split("/").pop() || - options.filePath; + const fileName = resolveFileViewerDisplayName(options);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/tabs/store.ts` around lines 743 - 746, The file-display-name fallback (options.displayName || options.filePath.split("/").pop() || options.filePath) is duplicated and should be extracted into a single helper; create a shared function (e.g., getDisplayNameForFile(options: {displayName?: string; filePath: string}) or getFileDisplayName) and replace the inline fallback in this store (store.ts) and the existing createFileViewerPane call site to both call that helper, preserving the current precedence (displayName, last path segment, full path) and handling undefined/null safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 168-173: The keys for mentionSegments in UserMessage are
collision-prone because they rely on text prefixes; update the map callback in
UserMessage (the mentionSegments.map) to generate collision-safe keys by
including the segment index or a unique segment identifier (e.g., use a stable
segment.id when available, or combine the parent key variable with the map index
like `${key}-text-${index}` / `${key}-mention-${index}-${segment.id ||
segment.value}`) so repeated prefixes or mentions cannot collide; make the same
change for the second map block referenced around the other mention rendering
(lines ~189-190).
---
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 87-88: The loop in UserMessage (where parts is assigned from
message.content and iterated) assumes message.content is an array; add a guard
to ensure message.content is an array before iterating: validate with
Array.isArray(message.content) (or coerce to an empty array) when assigning
parts, then iterate parts safely; update the variable assigned at "const parts =
message.content as Record<string, unknown>[];" and the for-loop that follows in
the UserMessage component so null/non-array content won't throw.
- Around line 113-118: The handleCopy callback currently calls
navigator.clipboard.writeText(fullText) without awaiting or catching failures
and sets setCopied(true) unconditionally; change handleCopy (the function named
handleCopy in UserMessage.tsx) to await the writeText promise (or use
.then/.catch), only call setCopied(true) after a successful write, and handle
rejections by not setting copied or by setting an error state/logging (and
ensure the timeout to clear copied runs only on success). Keep fullText in the
dependency array and preserve the existing timeout behavior but only trigger it
after a confirmed successful clipboard write.
- Around line 19-51: The file contains two components; extract the
AttachmentChip function into its own component file/folder (e.g., create a new
AttachmentChip component that exports the component as default), move its props
type signature with the mediaType/data/filename/onClick definitions into that
new file, update UserMessage to import AttachmentChip and remove the local
AttachmentChip definition, and ensure any used symbols (AttachmentChip,
UserMessage, FileIcon, FileTextIcon) still have the necessary imports/exports
and types updated so the project compiles.
---
Nitpick comments:
In `@apps/desktop/src/renderer/stores/tabs/store.ts`:
- Around line 743-746: The file-display-name fallback (options.displayName ||
options.filePath.split("/").pop() || options.filePath) is duplicated and should
be extracted into a single helper; create a shared function (e.g.,
getDisplayNameForFile(options: {displayName?: string; filePath: string}) or
getFileDisplayName) and replace the inline fallback in this store (store.ts) and
the existing createFileViewerPane call site to both call that helper, preserving
the current precedence (displayName, last path segment, full path) and handling
undefined/null safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e39a5f1-a945-4088-8e01-f43d446d8926
📒 Files selected for processing (5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx
Comment on lines
+168
to
+173
| {mentionSegments.map((segment) => { | ||
| if (segment.type === "text") { | ||
| return ( | ||
| <span | ||
| key={`${key}-text-${segment.value.slice(0, 20)}`} | ||
| className="whitespace-pre-wrap break-words" |
Contributor
There was a problem hiding this comment.
Use collision-safe keys for mention/text segments.
Current keys can collide for repeated text prefixes or repeated mentions in the same message.
Proposed fix
- {mentionSegments.map((segment) => {
+ {mentionSegments.map((segment, segmentIndex) => {
...
- key={`${key}-text-${segment.value.slice(0, 20)}`}
+ key={`${key}-text-${segmentIndex}`}
...
- key={`${key}-mention-${segment.relativePath}`}
+ key={`${key}-mention-${segmentIndex}-${segment.relativePath}`}Also applies to: 189-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsx`
around lines 168 - 173, The keys for mentionSegments in UserMessage are
collision-prone because they rely on text prefixes; update the map callback in
UserMessage (the mentionSegments.map) to generate collision-safe keys by
including the segment index or a unique segment identifier (e.g., use a stable
segment.id when available, or combine the parent key variable with the map index
like `${key}-text-${index}` / `${key}-mention-${index}-${segment.id ||
segment.value}`) so repeated prefixes or mentions cannot collide; make the same
change for the second map block referenced around the other mention rendering
(lines ~189-190).
- Add .catch() to void handleSend to prevent unhandled rejections - Fix file-only optimistic message reconciliation using send-time count - Handle clipboard promise rejection in UserMessage - Remove entries from useOptimisticUpload useEffect deps to prevent loops - Guard stop button disabled prop so it stays clickable during uploads - Extract shared AttachmentChip component with conditional button/div
saddlepaddle
force-pushed
the
saddlepaddle/support-files-and-images-in-ma
branch
from
8de2125 to
2361d0c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx (1)
137-178: Consider extracting the attachment handling block to a helper function.The bare
{ }block creates a scope forrawPart, which works but is an uncommon pattern that may confuse readers. Extracting this to a helper function would improve clarity.♻️ Optional refactor suggestion
function renderAttachmentNode( part: Record<string, unknown>, messageId: string, partIndex: number, onAttachmentClick: (url: string, filename?: string) => void, ): ReactNode | null { if (part.type !== "file" && part.type !== "image") return null; const mime = (part.mediaType as string) || (part.mimeType as string) || "application/octet-stream"; const data = (part.data as string) || (part.image as string) || ""; if (!data) return null; if (mime.startsWith("image/")) { return ( <button type="button" key={`${messageId}-${partIndex}`} className="max-w-[85%] cursor-pointer" onClick={() => onAttachmentClick(data, part.filename as string | undefined)} > <img src={data} alt="Generated" className="max-h-48 rounded-lg object-contain" /> </button> ); } return ( <AttachmentChip key={`${messageId}-${partIndex}`} data={data} filename={part.filename as string | undefined} mediaType={mime} onClick={() => onAttachmentClick(data, part.filename as string | undefined)} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx` around lines 137 - 178, The attachment handling is wrapped in a bare block using rawPart which is unclear; extract it into a helper (e.g., renderAttachmentNode) and replace the scoped block in AssistantMessage.tsx with a call to that helper. The helper should accept the part (Record<string, unknown>), message id and partIndex, and onAttachmentClick/handleAttachmentClick callback, compute mime and data as in the original logic, return a button with img for image/* or an AttachmentChip for non-image data, and preserve keys `${message.id}-${partIndex}` and props (filename, mediaType). Ensure AttachmentChip still receives data, filename and mediaType and that images call handleAttachmentClick with (data, filename).apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx (1)
69-133: Consider extractingMastraUploadFooterto its own file.
MastraUploadFooteris defined withinChatMastraInterface.tsx, which technically creates a multi-component file. While it's a small internal helper, the coding guidelines state "No multi-component files - maintain one component per file."Given its tight coupling to
ChatMastraInterfaceand small size, this is a minor concern that could be deferred.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`:
- Around line 166-175: In AssistantMessage, non-image AttachmentChip instances
are missing the click handler so PDFs/other files aren’t openable; update the
nodes.push call that renders AttachmentChip (inside AssistantMessage) to pass
the existing handleAttachmentClick callback as a prop (e.g., onClick or
onAttachmentClick depending on AttachmentChip’s API) along with data, filename
and mediaType so non-image attachments receive the same click behavior as images
handled earlier in the component.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`:
- Around line 137-178: The attachment handling is wrapped in a bare block using
rawPart which is unclear; extract it into a helper (e.g., renderAttachmentNode)
and replace the scoped block in AssistantMessage.tsx with a call to that helper.
The helper should accept the part (Record<string, unknown>), message id and
partIndex, and onAttachmentClick/handleAttachmentClick callback, compute mime
and data as in the original logic, return a button with img for image/* or an
AttachmentChip for non-image data, and preserve keys
`${message.id}-${partIndex}` and props (filename, mediaType). Ensure
AttachmentChip still receives data, filename and mediaType and that images call
handleAttachmentClick with (data, filename).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e4fbf52-cc7b-406e-8a7e-9259ea15a930
📒 Files selected for processing (8)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AttachmentChip/AttachmentChip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AttachmentChip/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/UserMessage/UserMessage.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/components/ChatComposerControls/ChatComposerControls.tsxpackages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/hooks/useOptimisticUpload/useOptimisticUpload.ts
- packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts
Comment on lines
+166
to
+175
| } else if (data) { | ||
| nodes.push( | ||
| <AttachmentChip | ||
| key={`${message.id}-${partIndex}`} | ||
| data={data} | ||
| filename={rawPart.filename as string | undefined} | ||
| mediaType={mime} | ||
| />, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Non-image attachments are not clickable in assistant messages.
The handleAttachmentClick callback is defined but not passed to AttachmentChip for non-image file attachments. This means PDFs and other files won't open in the file viewer when clicked, unlike images which have the click handler on lines 152-157.
🔧 Proposed fix to enable click handling
} else if (data) {
nodes.push(
<AttachmentChip
key={`${message.id}-${partIndex}`}
data={data}
filename={rawPart.filename as string | undefined}
mediaType={mime}
+ onClick={() =>
+ handleAttachmentClick(
+ data,
+ rawPart.filename as string | undefined,
+ )
+ }
/>,
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx`
around lines 166 - 175, In AssistantMessage, non-image AttachmentChip instances
are missing the click handler so PDFs/other files aren’t openable; update the
nodes.push call that renders AttachmentChip (inside AssistantMessage) to pass
the existing handleAttachmentClick callback as a prop (e.g., onClick or
onAttachmentClick depending on AttachmentChip’s API) along with data, filename
and mediaType so non-image attachments receive the same click behavior as images
handled earlier in the component.
Contributor
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts">
<violation number="1" location="packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts:327">
P2: Using the full `historicalMessages` array as a `useMemo` dependency makes `commands` unstable on frequent polling updates, causing avoidable callback/effect churn in consumers.
(Based on your team's feedback about narrowing React hook dependencies to required fields.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }, | ||
| }), | ||
| [cwd, sessionId, utils], | ||
| [cwd, sessionId, utils, historicalMessages], |
Contributor
There was a problem hiding this comment.
P2: Using the full historicalMessages array as a useMemo dependency makes commands unstable on frequent polling updates, causing avoidable callback/effect churn in consumers.
(Based on your team's feedback about narrowing React hook dependencies to required fields.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat-mastra/src/client/hooks/use-mastra-chat-display/use-mastra-chat-display.ts, line 327:
<comment>Using the full `historicalMessages` array as a `useMemo` dependency makes `commands` unstable on frequent polling updates, causing avoidable callback/effect churn in consumers.
(Based on your team's feedback about narrowing React hook dependencies to required fields.) </comment>
<file context>
@@ -304,7 +324,7 @@ export function useMastraChatDisplay(options: UseMastraChatDisplayOptions) {
},
}),
- [cwd, sessionId, utils],
+ [cwd, sessionId, utils, historicalMessages],
);
</file context>
…files-and-images-in-ma # Conflicts: # apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/components/ChatMastraMessageList/components/AssistantMessage/AssistantMessage.tsx
…mode - Export inferSubagentStatus/isSubagentRunning from toSubagentViewModels - Use inferSubagentStatus in toSubagentViewModels to infer status from result/error fields when explicit status is missing - Add inline prop to PendingPlanApprovalMessage to skip Message wrapper
This was referenced Mar 6, 2026
Kitenite
added a commit
that referenced
this pull request
Mar 6, 2026
…#2111) * Revert "feat(desktop): support files and images in mastra chat (#2058)" This reverts commit b528ad3. * feat(desktop): restore mastra chat file uploads * fix(desktop): remove stale resource badge severity props * fix(chat-mastra): restore optimistic file message reconciliation * fix(desktop): support file sends before session creation * fix(desktop): restore mastra user message copy action * fix(desktop): restore subagent activity and harden uploads
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
AttachmentChipcomponent (image thumbnails, file icons, copy button)useOptimisticUploadhook +uploadFilesutility for blob upload lifecycleMastraUploadFootercomponent withrenderAttachmentloading spinnersmastracodetov0.4.0-superset.12,@mastra/coreto1.8.0-superset.1,@mastra/memoryto1.5.2-superset.1(fork tarballs with file part support in harness)Depends on: superset-sh/mastra#6
Test plan
.txt/.json→ harness converts to inline textbun run typecheckpasses (20/20)🎉🎉🎉 MASTRACODE FILE SUPPORT IS ALIVE! 🎉🎉🎉Summary by cubic
Adds support for files and images in Mastra desktop chat with optimistic uploads to Vercel Blob. Replaces the image-only flow; attachments open in a viewer pane with the original filename. Also improves subagent status inference and allows inline plan approval rendering.
New Features
Migration
Written for commit a21f535. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements